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 but read permissions - penultimate draft pull request #2297

Closed

Conversation

rkboyce
Copy link
Contributor

@rkboyce rkboyce commented Jul 10, 2023

Now filtering works for concept sets and cohort definitions. Wanted to check if this is going ok and then discuss if we should keep going for all of the other applications now or do the rest for 2.15 release. Also, need to discuss where revised system permission should go (i.e., in the release code or just instructions).

rkboyce and others added 20 commits February 13, 2023 14:44
…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.
…n used to tell the WebAPI to do filtering based on READ permissions. The new property is called security.defaultGlobalReadPermissions
…n used to tell the WebAPI to do filtering based on READ permissions. The new property is called security.defaultGlobalReadPermissions
… on getting the cohortdefinition filtering to work. Still need to fix the case where another user grants read permission to a single cohort definition to another user and then that definition is visible. Currently, somehow, all definitions authored by the granting user are showing in the grantee's Atlas. Once worked out, the task will be to repeat the steps for every one of the following files: src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/EstimationPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/FeatureAnalysisPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/IncidenceRatePermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/PathwayAnalysisPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/PredictionPermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/ReusablePermissionSchema.java src/main/java/org/ohdsi/webapi/security/model/TagPermissionSchema.java
… apply the concept filtering to other artifact types
…D permissions - tested and not working completely. Strange permissions issue that appears to be entirely client-side.
@rkboyce
Copy link
Contributor Author

rkboyce commented Jul 16, 2023

This updated draft pull request implements the READ permissions filtering across all the following applications in Atlas:

  • concept sets
  • Cohort definition
  • Cohort characterization
  • Incident Rates
  • Estimation
  • Prediction

Of the above, Prediction is still having issues in testing but it seems to be on the Atlas client side. I am submitting this draft so that code review can occur while I work out the issue and do a bit more testing.

NOTE: These changes will need to come with a new system role that the admins could select which I am calling the 'Read Restricted Atlas User' role. I think the following query could be the basis for adding SQL to flyway to create this role:

select distinct sp.*
from ohdsi.sec_role_permission srp 
  inner join ohdsi.sec_permission sp on srp.permission_id = sp.id 
where srp.role_id in (6,10) -- 'cohort reader', 'Atlas Users'
 and sp.value not in 
    (  
    	'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'
		--
		'prediction:*:get',
		'prediction:*:copy:get',
		'prediction:*:download:get',
		'prediction:*:export:get',
		'prediction:*:generation:get',
		'prediction:*:exists:get'
    )
order by sp.value 
;

@rkboyce rkboyce marked this pull request as ready for review July 16, 2023 04:48
@rkboyce
Copy link
Contributor Author

rkboyce commented Jul 19, 2023

This is closed because the update should have happened on draft pull request 2245

@rkboyce rkboyce closed this Jul 19, 2023
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.

1 participant