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

Filter cohorts and concepts by permission (#2245) #2301

Merged
merged 3 commits into from
Aug 14, 2023
Merged

Conversation

chrisknoll
Copy link
Collaborator

Enable WebAPI to do filtering based on READ permissions. The new property is called security.defaultGlobalReadPermissions.

rkboyce and others added 2 commits July 28, 2023 14:42
* This is the first working filtration of the conceptset lists so that a user only sees what their role has permission to read. This initial commit has a big issue in that a person who authors a conceptset cannot see the concept set unless a new permission is added. To be fixed.

* changed the name, data type, and location for the configuration option used to tell the WebAPI to do filtering based on READ permissions. The new property is called security.defaultGlobalReadPermissions

* Fixed the default value of defaultGlobalReadPermissions to be true in pom.xm and to be camel case formatted throughout the code
Handle read-only filtering for Pageable.
Remove redundant code using conditional filtering.
Maintain backwards comparability in PermissionService.
@chrisknoll
Copy link
Collaborator Author

@rkboyce : please pull this branch and test on your local environment.

@rkboyce
Copy link
Contributor

rkboyce commented Aug 8, 2023

All of the following tests passed:

Configuration 1:

  • WebAPI: <security.defaultGlobalReadPermissions>false</security.defaultGlobalReadPermissions>
  • Atlas: configLocal.enablePermissionManagement = true;
  • A test user with the following roles:
Atlas users
Moderator
Source user (<some source>)
admin
cohort creator
cohort reader
concept set creator
public
<user specific role>
  • A test user with the following roles but who does not have the permissions that follow the listed roles:
-- Roles
Source user (<some source>)
cohort creator
concept set creator
public
<user specific role>

-- The user does not have these permissions 

    	      'conceptset:*:get',
    	      'conceptset:*:expression:get',
    	      'conceptset:*:version:*:expression:get',    	       
    	--
              'cohortdefinition:*:get',
              'cohortdefinition:*:info:get',
              'cohortdefinition:*:version:get',
              'cohortdefinition:*:version:*:get',        
        --        
	      'cohort-characterization:*:get',
               'cohort-characterization:*:generation:get',
	 	'cohort-characterization:generation:*:get',
		'cohort-characterization:design:get',
		'cohort-characterization:*:design:get',
		'cohort-characterization:design:*:get',
		'cohort-characterization:*:version:get',
		'cohort-characterization:*:version:*:get',
		--
		'pathway-analysis:*:get',
		'pathway-analysis:*:generation:get',
		'pathway-analysis:generation:*:get',
		'pathway-analysis:generation:*:result:get',
		'pathway-analysis:generation:*:design:get',
		'pathway-analysis:*:version:get',
		'pathway-analysis:*:version:*:get'
		--
		'ir:*:get',
		'ir:*:copy:get',
		'ir:*:info:get',
		'ir:*:design:get',
		'ir:*:version:get',
		'ir:*:version:*:get'
		--		
		'estimation:*:get',
		'estimation:*:copy:get',
		'estimation:*:download:get',
		'estimation:*:export:get',
		'estimation:*:generation:get',
		'comparativecohortanalysis:*:get', -- might only apply to older versions of Atlas/WebAPI
		--
		'prediction:*:get',
		'prediction:*:copy:get',
		'prediction:*:download:get',
		'prediction:*:export:get',
		'prediction:*:generation:get',
		'prediction:*:exists:get',
		'plp:*:get' -- might only apply to older versions of Atlas/WebAPI

Test 1: expected behavior - filtering of listed entities based on READ permissions by WebAPI

User logs in and can view all of the entities that the user has READ permissions to (concept sets, cohort definitions, characterizations, cohort pathways, incidence rates, estimation, prediction) - Passed

Test 2: ability to add READ/WRITE permissions to any entity that the user has WRITE permissions to

User creates an entity or opens an existing entity that they have WRITE permissions for and can add READ/WRITE permissions to that entity for another user. The other user will be able to view (if given READ permissions) and edit (if given WRITE permissions) - Passed

Test 3: ability to remove READ/WRITE permissions to any entity that the user has WRITE permissions to

User opens an existing entity that they have WRITE permissions for and can remove the READ/WRITE permissions to that entity for another user. The other user will not be able to view (if READ permissions are removed) nor edit (if WRITE permissions are removed) that entity - Passed

@chrisknoll chrisknoll merged commit 5393ed2 into master Aug 14, 2023
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the issue-2300 branch August 14, 2023 14:30
@anthonysena anthonysena linked an issue Sep 19, 2023 that may be closed by this pull request
return definitions.stream()
.filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true)
Copy link
Contributor

@pieterlukasse pieterlukasse Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisknoll @rkboyce I just came across this change and was wondering why the permissionService is only checked here. Should it not also be used in getCohortDefinitionRaw(https://github.com/OHDSI/WebAPI/pull/2301/files#diff-f0d513a6643cd34ac8cee922ef4e560fe9cbc270929d9054520ccc7055fbad10R478-R487) for example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, the purpose of this filter is to remove non-permissioned entities (like cohort definitions) from the calls to get the list of cohort definitions (or concept sets) from the server. The getCohortDefinitionRaw (and other method) fetches a single cohort definition from the service, which would already be 'permission denied' by the security layer...so we don't need to filter in this method as this method will be rejected if you don't have permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter assets by read-only permission
4 participants