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

Add support for Team level authorization in Atlas/WebAPI #2369

Open pieterlukasse opened 1 month ago

pieterlukasse commented 1 month ago

Expected behavior

New feature request: Following-up on the recent "read restricted" feature introduced in https://github.com/OHDSI/WebAPI/issues/2222 and documented at https://github.com/OHDSI/WebAPI/wiki/Read-restricted-Configuration, it would be nice to have a way to assign permissions on artifacts (like concept sets and cohort definitions) to a group/team of users, so members of the same team can see and use each others artifacts, while at the same time hiding them/restricting their access for other teams.

Actual behavior

Currently any user sees all artifacts created by all other users. With the recent "read restricted" feature referenced above, it is also possible to let the user see only the artifacts he has personally authored.

Background info

Below are some figures and diagrams clarifying the requirements (taken from slides recently presented in one of our dev calls).

Screenshot 2024-05-07 at 17 55 25

Screenshot 2024-05-07 at 17 58 29

Screenshot 2024-05-07 at 17 59 20 Diagram steps explained:

  1. user X selects "team A" (during or after login)
  2. authorization service is queried to check if user X is really part of "team A"
  3. if authorization is successful, the sec_user_role table is updated for user X, assigning the corresponding "team A" role to user X and removing any other user roles assigned to this user (e.g. "team B" from a previous session by this same user)
  4. and 5. subsequent requests done by user X, like a call to retrieve the list of cohort definitions, will automatically result in a filter being applied in the backend, narrowing down the result set to just the items owned by "team A" role
chrisknoll commented 1 month ago

Thanks for posting this here for discussion.

As we spoke on the team call, I wanted to propose an alternative way to isolate 'team assets' that doesn't depend on permission arrangement.

My concern is that 'weaving' team functionality into the permission layer will lead to complexity in the codebase insofar that if you want to manage functionality realted to teams, you have to manipulate the permission subsystem. I'd rather see Team functionality a full-fledged 'thing' in the system. So to that end, I propose the following:

We have a common entity for all of our assets in teams called a CommonEntity. Using the inheritance explorer in my IDE, this is what this shows:

CommonEntity

So if we add a new entity Team and introduce a @ManyToOne relationship between Team and the subclass of CommonEntity, I believe that would easily associate a Team to the asset. This means we can treat a Team as a first-class citizen, by defining attributes on it, create and delete them freely etc. This also means that we can switch between Team-enabled and disabled which would just mean any new assets created while teams is disabled get a NULL association to a Team, and when Team functionality is enabled, you will be able to see both those belonging to the user's team and anything that is not assigned to a team (maybe we think about a function that lets certain users 'claim team ownership' for an asset that is not associated to a team).

This is what the data model would look like:

EntityDiagram

We can use the existing entity definitions as a guide, but I belive by adding the new Enity type (Team) and defining a field of TeamId as a @ManyToOne relationship to a Team, all that would be required is to create migration scripts to introduce the new Team table and the TeamID column on any entity that derives from a CommonEntity. That will enable persistence of the field in the database, and then we can apply result-filters on queries that fetch entities from the database. Where we filter out those things that are not read-visible to users, we can add an additional layer to filter out those things that do not belong to the user's team.

I believe this will give a nice separation of concerns between what permissions do vs. what team assignment to assets does, and may simplify our design. It may also have some drawbacks, and I'm eager to hear thoughts or concerns on this.

pieterlukasse commented 1 month ago

Thanks @chrisknoll ! Here are some extra thoughts in favor of the original proposal:

Let me know what you think!