Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Scopes are not filtered on search-type #83

Closed
liquid36 opened this issue Apr 6, 2022 · 12 comments
Closed

Scopes are not filtered on search-type #83

liquid36 opened this issue Apr 6, 2022 · 12 comments
Labels
bug Something isn't working

Comments

@liquid36
Copy link

liquid36 commented Apr 6, 2022

Hi! I'm having an issue in this lines which changes on this #79

if (resourceType === '*' || resourceType === reqResourceType || reqOperation === 'search-type') {

This is my use case:

        const clonedScopeRule = emptyScopeRule();
        clonedScopeRule.system.read = ['search-type'];
        expect(
            filterOutUnusableScope(
                ['system/DocumentReference.read', 'system/Patient.read'],
                clonedScopeRule,
                'search-type',
                false,
                'Procedure',
            ),
        ).toEqual([]);

If the user claims for this scope system/DocumentReference.read system/Patient.read but ask for a Procedure resource, the filterOutUnusableScope mark both scope as usable and then the operation is allowed.

@ssvegaraju
Copy link
Contributor

Hey @liquid36, thanks for catching this! I've added it to our team's backlog.

@ssvegaraju ssvegaraju added the bug Something isn't working label Apr 7, 2022
@medhost-bfindeisen
Copy link

We are also experiencing this bug with user/ scopes.

It allows a user access to ANY resource type as long as there is at least one user/ resource in the scope list.

Example: A scope list with user/Procedure.read
"scp": [
"openid",
"fhirUser",
"offline_access",
"user/Procedure.read"
],

allows access to read Observations

@FernandoAranda
Copy link

Hi @medhost-bfindeisen , thank you for bringing this to our attention,
we are working on this issue and is already in our backlog.

-Fernando

@rsmayda
Copy link
Contributor

rsmayda commented Sep 8, 2022

@medhost-bfindeisen is this specifically for search-type? or for read as well

@medhost-bfindeisen
Copy link

@rsmayda , this appears specific to search-type, read would result in the last condition here to be false:

https://github.com/awslabs/fhir-works-on-aws-authz-smart/blob/mainline/src/smartScopeHelper.ts#L58

@rsmayda
Copy link
Contributor

rsmayda commented Sep 8, 2022

@medhost-bfindeisen we weren't able to replicate with the user scope could you send over the jwt scp claims, the fhirUser claim and your request? I wonder if it is related to this input param: https://github.com/awslabs/fhir-works-on-aws-authz-smart/blob/mainline/src/smartHandler.ts#L74

@medhost-bfindeisen
Copy link

Hi @rsmayda

Our scp only has access to Procedure.read:

  "scp": [
    "openid",
    "fhirUser",
    "offline_access",
    "user/Procedure.read"
  ],

Our request is for an Observation: /Observation?_id=<observation_id>

This observation is returned.

If we change the request to be a read: /Observation/<observation_id>
We get a 401 with diagnostics of "access_token does not have permission for requested operation"

We were expecting the search to also return a 401 due to the Observation not being in the scope list.

Our fhiruser claim is the path to our practitioner resource.

@rsmayda
Copy link
Contributor

rsmayda commented Sep 8, 2022

@medhost-bfindeisen just published to NPM let us know if this helps! The version is v3.1.2

@rsmayda
Copy link
Contributor

rsmayda commented Sep 12, 2022

@medhost-bfindeisen we just published version 3.1.3 see above PR for more context

@liquid36
Copy link
Author

I just upgrade the package version on my project and i can confirm that the issues was resolved. Thanks!

@medhost-bfindeisen
Copy link

The issue is resolved for system, patient, and user scopes. Thanks!

@ssvegaraju
Copy link
Contributor

That's great to hear! I'll go ahead and close this issue for now, but feel free to reopen if there's anything else.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants