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

User with 'read restricted Atlas Users' role cannot open their own objects created before receiving this role #2323

Open ttemnikova opened 7 months ago

ttemnikova commented 7 months ago

Expected behavior

The objects created by a user are available for this user in spite of their role

Actual behavior

With the role 'read restricted Atlas Users', the user is not able to open or edit the objects they created before receiving this role

Steps to reproduce behavior

  1. user1 has created some objects (cohorts, concept sets, analyses of different types...)
  2. Administrator excludes user1 from their role and includes user1 to the 'read restricted Atlas Users' role
  3. user1 tries opening the objects created on the step 1
ttemnikova commented 7 months ago

2316

chrisknoll commented 7 months ago

I think this would be addressed if we added the 'hasWrite' check inside the 'hasRead' check.

What's strange about your steps is: the individual entity permissons (ie: at the ID level) is not part of any particular role (ie: atlas_users, or the read_only group), so I'm unclear why a person would loose permission to an entity, UNLESS the issue is that we don't depend on a SHIRO permission to allow access to an entity directly (ie: a perm directly to the ID) and instead depend on the * permission to let the REST request go through, but then do the check for 'author' to see if they have write permission.

Is that the case?

ttemnikova commented 7 months ago

@chrisknoll thank you for the clarification. Yes, probably that is the case.

  1. user1 has the role 'Atlas users'
  2. user1 creates an object
  3. user1 grants themself READ access to the object
  4. Administrator deletes user1 from 'Atlas users' and adds to 'read restricted Atlas Users' role
  5. user1 tries accessing the object from the step 2

If I omit the step 3, then my test user cannot access their objects created on the step 2

chrisknoll commented 7 months ago

Right, so I'm not sure how we can solve this: the original security implementation assumed everyone had read access, so we didn't have to assign read permissions to anything, just set up a SHIRO permission to let anything through, and then do additional permission checks (such as if you are author) to either throw an auth exception or not.

Now that we're trying to remove the read permission from the user (by removing the AtlasUser role which grants read to everything), you won't be able to call the GET route on the entity to even figure out if you're able to might have the write permission to it.

This leaves me to the feeling that the entire security model needs to be reconsidered.

rkboyce commented 7 months ago

This is not an issue that will affect organizations who want to use the read-restricted role because we have now documented on the wiki (which we will point to in the release note) the use case and preparation required for using the new feature. However, the issue is noted in the wiki with discussion.