OHDSI / WebAPI

OHDSI WebAPI contains all OHDSI services that can be called from OHDSI applications
Apache License 2.0
126 stars 156 forks source link

There is no proper messages for users with 'read restricted Atlas Users' role if an object is unavailable #2324

Open ttemnikova opened 7 months ago

ttemnikova commented 7 months ago

Expected behavior

If a user with 'read restricted Atlas Users' role tries accessing an object they do not have read or write permissions, the user should get a proper message on the screen (e.g. "You do not have permissions to access this object")

Actual behavior

No messages at all (empty screen) or message about unavailable feature (You do not have access to this feature. For more information on how to get access please contact the System Administrator) which is not true. For PLP the page looks broken (there are design blocks but they are empty, no labels, no icons etc.)

Steps to reproduce behavior

  1. user1 creates an object (for example, a cohort pathways analysis)
  2. user2 has 'read restricted Atlas Users' role
  3. user2 does not have read or write access to the object from the step 1, but does have permissions to this type of objects (e.g. cohort pathways)
  4. user2 tries opening the object from the step 1
ttemnikova commented 7 months ago

2316

chrisknoll commented 7 months ago

@ttemnikova , I'm looking at the permissions the referenced PR, and for pathway permissions, i see these:

        'pathway-analysis:*:exists:get',
        'pathway-analysis:*:export:get',
        'pathway-analysis:*:post',
        'pathway-analysis:*:tag:*:delete',
        'pathway-analysis:*:tag:post',
        'pathway-analysis:*:version:*:createAsset:put',
        'pathway-analysis:*:version:*:delete',
        'pathway-analysis:*:version:*:put',
        'pathway-analysis:byTags:post',
        'pathway-analysis:check:post',
        'pathway-analysis:get',
        'pathway-analysis:import:post',
        'pathway-analysis:post',

I don't see one that has pathway-analysis:*:get (which would allow you to open up any ID) so my question is: why did SHIRO let the rest call go through? Can you check your Chrome console to see if you got a error in the GET call, and if so, the issue is probably handling the fetch of the pathway analysis, and not some permission check problem in the UI.

But from here, it doesn't look like you should be able to fetch something like pathway-analysis/14 GET via HTTP without an error.

ttemnikova commented 7 months ago

Yes, you are right @chrisknoll , when I am accessing an analysis (e.g. pathways) I get a 403 error without any other information in the response of the request GET /WebAPI/pathway-analysis/<ID>. I am able to get other users' pathways with READ permissions granted for my user. Also, I can create new pathways and open them from the list of pathways.

chrisknoll commented 7 months ago

Ok, so maybe the issue is that we're not handling the 403 response (that is not authorized) in the Cohort Pathway editor page.

ttemnikova commented 7 months ago

I just would like to notice that I observe issues with unexpected behavior not only in pathways, but for all types of objects:

This behavior can be caused by different reasons, but I combined them into one task because from the user perspective they look very similar: "I do not have permissions to view this object, I would like to get an appropriate message about it"

chrisknoll commented 7 months ago

In this case, the error message is generic: you get an 'unauthorized' response, it could be because of a feature (ie: denied to any cohort definition) or it could be denied to a specific element (which is a new context with the introduction to read permissions, where before everyone was assumed to have at least read, now it is possible to have none). This message could be made more generic to not talk about 'to the feature' but simply 'access is denied'.

We could introduce functionality to distinguish 'access to feature' vs. 'access to entity', but (if I recall correctly) the 403 error is handled at a very high level, and any 403 results in the given message.

But, it is definitely a broken case for those pages where you get no message or a broken UI.

rkboyce commented 7 months ago

I went through and confirmed these issues in my dev environment:

Set up:

Similar to @ttemnikova, when I copied the URL for each of the following artifact types from the browser user 1 was logged into over to browser that user 2 was logged into I see the following in Atlas and dev tools:

So, based on this, it seems that the WebAPI is working as it is supposed to wrt to restricting READ permissions. At not time was user 2 able to view or see even a glimpse of the artifacts that the user had no access to. I think the Atlas client needs a bit of work to better and consistently handle the 403 situations. This would include us agreeing about the message to show.

I suggest that it indeed needs to get fixed but I move that we do it for 2.15 rather than now. This is because sites can the configuration that enables READ restrictions if they plan to have all non-admin users have the read-restricted Atlas role from the start. That way, users never have links to artifacts that they do not have READ access to because of historical interactions with the system prior to the read-restricted role existing. Also, if user 1 shares a link to user 2 for a resource that they have READ access to, user 2 will not be able to read it unless given permission. This is expected behavior for the use case.