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

SMART interceptor should enforce scope permissions up front #2669

Closed
lmsurpre opened this issue Aug 9, 2021 · 2 comments
Closed

SMART interceptor should enforce scope permissions up front #2669

lmsurpre opened this issue Aug 9, 2021 · 2 comments
Assignees
Labels

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Aug 9, 2021

Describe the bug
Because SMART scopes are only enforced on resources on their way out of the FHIR server, it is possible for an application with a valid access token to obtain information about the presence (or absence) of resource types for which they have not been granted read access.

Environment
Which version of IBM FHIR Server?

To Reproduce
Steps to reproduce the behavior:

  1. load a server with some AllergyIntolerance for a patient
  2. configure that server with the fhir-smart jar
  3. obtain a token with a patient_id claim for that patient but no scope permitting read access to AllergyIntolerancce
  4. perform search
  5. obtain a token with a patient_id claim for some other patient with no AllergyIntolerance
  6. perform search

Not that a search that returns the AllergyIntolerance comes back with a 403 whereas the other comes back with a 200 (with no results).

Expected behavior
A client app without read permission for a given resource type should not be able to infer whether or not any such resources exist.

Additional context
Add any other context about the problem here.

@lmsurpre lmsurpre self-assigned this Aug 9, 2021
lmsurpre added a commit that referenced this issue Aug 9, 2021
search

In addition to checking the scopes for each resource on the way out (via
the afterX methods), we now check that the requested interaction is
permitted by the passed scopes on the way in (the beforeX methods).
Two reasons:
1. We prevent some unnecessary work for invalid requests
2. We prevent leaking info about the presence/absence of resourceTypes
for which a given application has not been granted access

The request scopes are already checked before create, update, and
delete.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Aug 10, 2021
…d search (#2671)

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member Author

One of the questions I had while implementing this is whether it should apply only to patient compartment resources (e.g. Condition, Observation) or whether it should also apply to non-compartment resources like Practitioner and Organization.

It wasn't super clear to me whether the existing afterX methods checked the scope string for these non-patient-compartment resources or not, but after reviewing https://chat.fhir.org/#narrow/stream/179170-smart/topic/Authorization.20for.20transactions/near/213305185 I felt that it would make the most sense to check the scope string for non-patient-compartment resources in the beforeX methods as well.

Therefore, to invoke read/vread/search/history against ANY resource type's endpoint, it is now definitively required to include the corresponding resourceType (or *) in the list of granted resource types. For example, to read an Organization resource, the user must have granted an application one of patient/Organization.read, patient/*.read, user/Organization.read, or user/*.read. (however, use of user/ permissions is still not implemented for some cases.

@kmbarton423
Copy link
Contributor

Walked thru the test of this one with Lee using his Alvearie demo environment. Confirmed appropriate behaviors with and without limited scope when attempting to access (read/search) non-Patient compartment resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants