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

Idea : filter items shown to users in summary lists (e.g., concept sets, cohort definitions, etc) based on permissions #2222

Closed
rkboyce opened this issue Feb 28, 2023 · 6 comments
Milestone

Comments

@rkboyce
Copy link
Contributor

rkboyce commented Feb 28, 2023

Use case:
Dozens of users in a data commons are using Atlas to create concept sets and cohort definitions. Many of them are just learning how build these artifacts and some have obtained mastery. Users do want to view/browse concept sets and cohort definitions that could be useful for their analyses (reusability) but often other users would prefer that their artifacts not be listed until they give permission (e.g., because they are in draft mode or private to a project or team).

Current behavior:
All users will see up to hundreds of concept sets and cohort definitions when they click on the #/conceptsets and #/cohortdefinitions endpoints in Atlas. This can be very confusing to the users as they will see numerous things that are described only by title and often with little context to help them know if its relevant to their project. They might not have a way of knowing know if the artifacts are in draft mode or have been tested. Also, many users would prefer the ability to give permission to share the viewing/listing of their artifacts with either specific individuals, groups, or the community of users as a whole. Reasons for wanting to control access include a desire to wait until the artifact is solidly tested prior to contributing it to the community, wanting the the project to be complete and published before they share the work, or other reasons.

Proposal:
Modify the WebAPI to use a new property setting that will cause the WebAPI instance to return only concept sets, cohort definitions, or other artifacts (e.g., incidence rate analysis, characterizations, etc) in the response provided by endpoints such as #/conceptsets, #/cohortdefinitions, etc. that a user has explicit permission to view based on the sec_role_permission and sec_permission tables in the security schema.

Justification for doing this in WebAPI:
It seems more appropriate from an authorization standpoint for the service to filter artifact lists based on the authenticated user's permissions rather than having the client application (Atlas) do so. In the latter case, the client app still receives a listing that someone could sniff using developer tools within the browser. Note that if this idea is accepted, it will likely make sense to add the ability of an author to edit read permissions using Atlas "Configure access". This will add a new ability given that the current Configure Access focuses only on whom can edit an artifact.

Additional considerations:

  • If a user creates X number of concept sets (or cohort definitions etc.), will they have to go into all X and specify the invited permissions?
  • Is there a way to create user groups so that a user can invite a group of users (perhaps through group roles)?
  • (Add more in comments so that we can design this well)
@anthonysena anthonysena added this to the v2.14 milestone Mar 7, 2023
@anthonysena
Copy link
Collaborator

From discussion on the Atlas WG today, some notes from discussion:

  • At the moment, WebAPI does not have the notion of a "read" permission for entities - they are assumed to be read-only for all users in the system. This is problematic in the scenario described above as it creates a mess of "work in progress" entities. It also does not provide the fine-grained controls required by data commons such that work can be done in private until such a time that it can be shared.
  • This would require a change such that the global "get" for an entity (i.e. GET /cohortdefinitions/*) is moved to a specialized role for persons that need to see all cohort definitions. Then, when new entities are created, an explicit read/write permissions are granted to the creator of the entity. In this way, a user can have a private entity until they are ready to share it with other users.
  • We also discussed the use of tags to mark the status of an entity (draft, in review, approved, etc). This would not be tied to security and instead is just a mechanism for moving things through a workflow.

@rkboyce
Copy link
Contributor Author

rkboyce commented Mar 7, 2023

Adding to the previous post a couple of small items:

  • Shiro security does have a READ access construct in addition to a WRITE access construct. One of the technical details on the WebAPI would be to add code to handle READ access more specifically but all the pieces to make this happen are present with no change needed to the core security model
  • The tagging approach is great for folks who are ready to share draft artifacts publicly (e.g., to build collaboration and seek broad input). Managing READ permissions more granularly would be necessary to support folks keeping things private to themselves and selected users until they are ready to share them publicly.

@chrisknoll
Copy link
Collaborator

chrisknoll commented Mar 7, 2023

Just some clarifications:

Re: read permission for entities: all WebAPI endpoints must have a permission defined, including the get:*: permission in order to fetch the asset from the repository. In the case of 'read' permissions, we assume that there is a get:* permission for every endpoint such that SHIRO will allow access to it. So, what @anthonysena says as a 'notion of 'read' I think is referring to that WebAPI/Atlas doesn't have a notion of controlling read access to users, but rather everyone can at least view an entity, but the 'real' permission comes down to being able to write.

What @anthonysena suggests to move a 'global get' (ie: get:*) to another role would be so that you can remove the global get from Atlas Users (so no one can view anything) and make a special role called 'global reader' that does have the get:* permission. That would work fine for @rkboyce context where he will want to be able to 'see' everything without everyone having to grant him specific permissions to 'read'.

Because Atlas assumes everyone has 'global read' (that's the default configuration for Atlas Users), we never thought that we would have to provide a UI to grant read permissions but now that we have this use-case, it's something that we should think about.

Side note: we are seeing that the permission payload from the serer is getting larger as more assets are defined and more permissions granted. We'd like to look at (in the 2.14 or 2.15 timeline) about optimizing or even re-thinking the permission structure so that it's faster to download and easier to process.

Edit: @rkboyce beat me to it!

@rkboyce
Copy link
Contributor Author

rkboyce commented Mar 27, 2023

Below are a few diagrams and some screen shots to help add to the specification of this feature enhancement:

Permission Requirements - WebAPI

Below if a requirements diagram that has taken the current implementation and extended it to add more refined read permissions. The feature enhancement would require the addition of two new elements to the requirements:

  • global_read (upper left) : a permission template and role that enables a given user to read all instances of a certain artifact type (concept set, cohort definition, etc.) Once the permission template is created, an admin could use the existing Atlas interface to assign the role to any other user role. The system should enable this by default for the admin role.
  • author_read (top row, third from the left) : management of an user role (created for every user) that enables a given user to read specific instances of a certain artifact type

Also, a design constraint (shown bottom right) of this feature enhancement is that global read permissions would be enabled by default but a site could enable refined read permission functionality using a configuration in the settings.xml that affects application properties.

NOTE: currently, PermissionController.java PermissionService.java works in conjunction with PermissionService.java to fulfill the role of providing services to generally manage both global and individual permissions (permission_implementation in the diagram). The abstract methods for changing the read and write permissions to any entity can be found in CommonEntityDTO.java.

NOTE: The specific entity (concept set, cohort definition, etc.) read and write permissions are maintained in model and service classes specific to those entities (e.g., for concept sets see ConceptSetPermissionSchema.java and ConceptSetService.java). These entity specific classes make calls to

requirementDiagram

    requirement permission_defined {
    id: 1
    text: endpoints must have a permission defined
    risk: high
    verifymethod: demonstration
    }

    functionalRequirement manage_global_permissions {
    id: 1.1
    text: permissions that apply to all users
    risk: high
    verifymethod: demonstration
    }

    functionalRequirement manage_individual_permissions {
    id: 1.2
    text: permissions that apply to any single user
    risk: high
    verifymethod: demonstration
    }

    functionalRequirement permission_service {
    id: 1.3
    text: permissions that apply to any single user
    risk: high
    verifymethod: demonstration
    }

    designConstraint configure_read_permissions {
    id: 1.4
    text: refined read permissions is optional based on configuration
    risk: high
    verifymethod: demonstration
    }

    element app_property_conf {
    type: configuration element that affects application properties
    docref: settings.xml
    }
    element global_read {
    type: role with shiro template
    docref: PermissionService.java 
    } 

    element global_write {
    type: role with shiro template
    docref: PermissionService.java 
    }

    element author_read {
    type: role with shiro template
    docref: PermissionService.java 
    }

    element author_write {
    type: role with shiro template
    docref: PermissionService.java 
    }

element permission_implementation {
    type: code
    docref: PermissionController.java
}

manage_global_permissions - refines -> permission_service
global_read - refines -> manage_global_permissions
global_write - refines -> manage_global_permissions

manage_individual_permissions - refines -> permission_service 
author_read - refines -> manage_individual_permissions
author_write - refines -> manage_individual_permissions 

permission_service - satisfies -> permission_defined
permission_implementation - contains -> permission_service  

permission_service - derives -> app_property_conf
app_property_conf - satisfies -> configure_read_permissions
Loading

User experience and interface requirements

The current (default) approach to permissions management by non-admin users

flowchart LR
    A[design artifact] -->|edit| B(save)
    B --> |public| D[global read]
    B --> |edit| C{write access}
    C -->|user| F[single user]
Loading

NOTE: In Atlas, the user can edit write access using the lock icon present in the concept set expression view:

image

Clicking on the lock icon brings up a simple form:

image

The revised (optional) approach to permission management by non-admin users

flowchart LR
    A[design artifact] -->|edit| B(save)
    B --> |edit| C{write access}
    C -->|group| E[all users in a  group]
    C -->|user| F[single user]
    B --> |edit| D{read access}
    D -->|public| G[global read]
    D -->|group| H[all users in a  group]
    D -->|user| I[single user]
Loading

NOTE: In Atlas, this could be implemented by change the modal form presented after clicking the the lock icon present in the concept set expression view to allow the user to edit both read and write access:

image

This change could be accomplished by editing the following Atlas components and files:

  • Permission.js - adding a READ accessType to the grantEntityAccess function
  • configure-access-modal.js and configure-access-modal.html -- these files provide the configure access modal shown above. The change would require editing these files to add a form where the user can select users to give read access to and a table to show who has read access. The change would add several new variables in the view-model to distinguish read and write permissions (the current implementation assumes that edits are only to write permissions)

@rkboyce
Copy link
Contributor Author

rkboyce commented Mar 28, 2023

In discussion, it was recognized that, once artifacts are filtered based on specific read permissions, users might want to make their artifacts public to all users. That would be distinct use case and will be treated in a separate issue.

Also, the modal in Atlas for configuring who has read/write permissions to an artifact might be more usable if it used tabs.

@anthonysena
Copy link
Collaborator

Linking to #2300 and closing out since it is complete and merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Review Complete
Status: Done
Development

No branches or pull requests

3 participants