Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthzPolicyEnforcement should prevent access to the whole-system history api #2300

Closed
lmsurpre opened this issue Apr 28, 2021 · 3 comments
Closed
Assignees
Labels
enhancement New feature or request P2 Priority 2 - Should Have security

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Apr 28, 2021

Is your feature request related to a problem? Please describe.
The fhir-smart AuthzPolicyEnforcementPersistenceInterceptor can be used to provide user-scoped authorization to the the FHIR Server's APIs.

For example, it scopes all searches to the patient compartment that matches the patient context for a given session.

However, one thing it doesn't prevent access to is the whole-system history endpoint.
This endpoint provides an overview of all the create/update/delete interactions serviced on the server.
Therefor, a use with access to the api would be able to determine all of the resource ids on the server.

Describe the solution you'd like
Currently the whole-system-history implementation doesn't execute any beforeX or afterX hooks in our PersistenceInterceptor framework. Since this is the mechanism used by AuthzPolicyEnforcementPersistenceInterceptor to restrict access, we should first add these hooks (or use the existing beforeHistory and afterHistory ones).

Next, we should either filter the list of results somehow or, more likely, just prohibit this interaction altogether.

Describe alternatives you've considered
We already have support for restricting access to the whole-system history endpoint and that is to filter the "history" interaction on the "Resource" resource in the fhir-server-config.json (as described at https://ibm.github.io/FHIR/guides/FHIRServerUsersGuide#412-fhir-rest-api ).
This might be sufficient, but it requires the operator to override this setting on each configured resource type.

Example:

"resources": {
    "open": false,
    "Condition": {
        "interactions": ["read", "vread", "history", "search"]
    },
    "Observation": {
        "interactions": ["read", "vread", "history", "search"]
    },
    "Patient": {
        "interactions": ["read", "vread", "search"]
    }
}

Acceptance Criteria
1.
GIVEN a deployment with fhir-smart installed
AND a request scoped to a particular patient
WHEN they invoke the whole-system history endpoint
THEN they should not see the resource ids for any resources to which they do not have access

Additional context
Relates to #2026 which rounds out our support for proper "whole-system history" behavior.

QA suggestions

  1. Test system-level history: Requires user/*.read or system/*.read authority
    Example: https://localhost:9443/fhir-server/api/v4/_history
  2. Test system-level history (with _type parameter): Requires user/<resource_type>.read or system/<resource_type>.read authority to each resource type in the _type parameter.
    Example: https://localhost:9443/fhir-server/api/v4/_history?_type=Patient,Observation
  3. Test type-level history: Requires user/<resource_type>.read or system/<resource_type>.read authority to the resource type.
    Example: https://localhost:9443/fhir-server/api/v4/Patient/_history
@prb112 prb112 added the enhancement New feature or request label Apr 29, 2021
@lmsurpre
Copy link
Member Author

the whole-system history endpoint impl should call the beforeHistory and afterHistory hooks in the interceptor with a resource type of "Resource"

@lmsurpre
Copy link
Member Author

two separate tasks:

  1. call the interceptor hooks
  2. update fhir-smart to properly controll access to this

@lmsurpre lmsurpre added P2 Priority 2 - Should Have security labels Jun 14, 2021
@tbieste tbieste self-assigned this Feb 28, 2022
tbieste added a commit that referenced this issue Feb 28, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Feb 28, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 1, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 1, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 2, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 2, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 2, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 4, 2022
Issue #2300 - Add calls to beforeHistory and afterHistory
tbieste added a commit that referenced this issue Mar 15, 2022
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Mar 15, 2022
Issue #2300 - Fix buildPersistenceEventProperties
@kmbarton423
Copy link
Contributor

Confirmed behavior is as expected. Below is a sample of the OperationOutcome received when attempting without the appropriate scope:

403 Forbidden
{
"resourceType": "OperationOutcome",
"id": "ac-11-ae-6c-485c69c3-21c8-4243-bef5-62151b93b7bb",
"issue": [
{
"severity": "fatal",
"code": "forbidden",
"details": {
"text": "Read permission for system history of '[Condition, AllergyIntolerance]' is not granted by any of the provided scopes: [patient/*.read, user/Condition.read]"
}
}
]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Priority 2 - Should Have security
Projects
None yet
Development

No branches or pull requests

4 participants