diff --git a/src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java index bb6781ae0..e5632c823 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java @@ -14,14 +14,14 @@ public class CohortDefinitionPermissionSchema extends EntityPermissionSchema { put("cohortdefinition:%s:check:post", "Fix Cohort Definition with ID = %s"); }}; - private static Map readPermissions = new HashMap() {{ - put("cohortdefinition:%s:get", "Get Cohort Definition by ID"); - put("cohortdefinition:%s:info:get",""); + private static Map readPermissions = new HashMap() {{ + put("cohortdefinition:%s:get", "Get Cohort Definition by ID"); + put("cohortdefinition:%s:info:get",""); - put("cohortdefinition:%s:version:get", "Get list of cohort versions"); - put("cohortdefinition:%s:version:*:get", "Get cohort version"); - } - }; + put("cohortdefinition:%s:version:get", "Get list of cohort versions"); + put("cohortdefinition:%s:version:*:get", "Get cohort version"); + put("cohortdefinition:%s:copy:get", "Copy the specified cohort definition"); + }}; public CohortDefinitionPermissionSchema() { diff --git a/src/main/java/org/ohdsi/webapi/shiro/Entities/PermissionEntity.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/PermissionEntity.java index f54160b6b..61867f7e1 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/Entities/PermissionEntity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/Entities/PermissionEntity.java @@ -30,7 +30,7 @@ public class PermissionEntity implements Serializable { name = "sec_permission_generator", strategy = "org.hibernate.id.enhanced.SequenceStyleGenerator", parameters = { - @Parameter(name = "sequence_name", value = "sec_permission_id_seq"), + @Parameter(name = "sequence_name", value = "sec_permission_sequence"), @Parameter(name = "initial_value", value = "1000"), @Parameter(name = "increment_size", value = "1") } diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java index 3c3776af6..91d23b11d 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java @@ -145,31 +145,29 @@ private boolean checkGen3Authorization(String teamProjectRole, String login) thr } else { JSONArray teamProjectAuthorizations = jsonObject.getJSONArray(teamProjectRole); logger.debug("Found authorizations={}", teamProjectAuthorizations); - // We expect only one authorization rule per teamproject: - if (teamProjectAuthorizations.length() != 1) { - logger.error("Only one authorization rule expected for 'teamproject'={}, found={}", teamProjectRole, + // We expect two authorization rules per teamproject: + if (teamProjectAuthorizations.length() != 2) { + logger.error("Two authorization rules expected for 'teamproject'={}, found={}", teamProjectRole, teamProjectAuthorizations.length()); return false; } - JSONObject teamProjectAuthorization = teamProjectAuthorizations.getJSONObject(0); - - // check if the authorization contains the right "service" and "method" values: String expectedMethod = "access"; String expectedService = "atlas-argo-wrapper-and-cohort-middleware"; // TODO - make the service name configurable? - String service = teamProjectAuthorization.getString("service"); - String method = teamProjectAuthorization.getString("method"); - logger.debug("Parsed service={} and method={}", service, method); - if (!method.equalsIgnoreCase(expectedMethod)) { - logger.error("The 'teamproject' authorization method should be '{}', but found '{}'", expectedMethod, method); - return false; - } - logger.debug("Parsed method is as expected"); - if (!service.equalsIgnoreCase(expectedService)) { - logger.error("The 'teamproject' authorization service should be '{}', but found '{}'", expectedService, service); - return false; + for(int i = 0; i < teamProjectAuthorizations.length(); i++) { + JSONObject teamProjectAuthorization = teamProjectAuthorizations.getJSONObject(i); + // check if the authorization contains the right "service" and "method" values: + String service = teamProjectAuthorization.getString("service"); + String method = teamProjectAuthorization.getString("method"); + logger.debug("Parsed service={} and method={}", service, method); + if (method.equalsIgnoreCase(expectedMethod) && service.equalsIgnoreCase(expectedService)) { + logger.debug("Parsed method is as expected"); + logger.debug("Parsed service is as expected"); + return true; + } } - logger.debug("Parsed service is as expected"); - return true; + logger.error("The 'teamproject' authorization method should be '{}', but was not found", expectedMethod); + logger.error("The 'teamproject' authorization service should be '{}', but was not found", expectedService); + return false; } } diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java index e7145d5a7..7aa81f4f7 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java @@ -94,8 +94,6 @@ private void init() { if (this.authorizationMode.equals("teamproject")){ // add system role that enables read restrictions/permissions based read access configurations: this.defaultRoles.add("read restricted Atlas Users"); // aka reserved system role 15 - // temporary solution to simplify onboarding: also add the "Source user (omop)" role to the list of defaultRoles for each user: - this.defaultRoles.add("Source user (omop)"); // TODO - replace with final solution from https://ctds-planx.atlassian.net/browse/VADC-1086 } fillFilters(); } diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql new file mode 100644 index 000000000..a15f4b911 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql @@ -0,0 +1,206 @@ +-- todo remove the role "source user" from all users or transform that role in non-system + +delete from ${ohdsiSchema}.sec_role_permission where role_id = 15; + +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +with vocab_source as ( + select source_key + from ${ohdsiSchema}.source s + inner join ${ohdsiSchema}.source_daimon sd on s.source_id = sd.source_id + where sd.daimon_type = 1 +), vocab_perms as ( + select distinct concat(l,m,r) perm + from ( + select * + from (values + ('vocabulary:') + ) t1 (l) + cross join + ( select source_key + from vocab_source + ) t2 (m) + cross join + (values + (':*:get'), + (':compare:post'), + (':concept:*:ancestorAndDescendant:get'), + (':concept:*:get'), + (':concept:*:related:get'), + (':included-concepts:count:post'), + (':lookup:identifiers:ancestors:post'), + (':lookup:identifiers:post'), + (':lookup:mapped:post'), + (':lookup:recommended:post'), + (':lookup:sourcecodes:post'), + (':optimize:post'), + (':resolveConceptSetExpression:post'), + (':search:*:get'), + (':search:post') + ) t3 (r) + ) combined +), + source_perms as ( + select distinct concat(ls,ms,rs) perm + from ( + select * + from (values + ('source:') + ) t11 (ls) + cross join + ( select source_key + from vocab_source + ) t22 (ms) + cross join + (values + (':access') + ) t33 (rs) + ) combined +), + generate_perms as ( + select distinct concat(lg,mg,rg) perm + from ( + select * + from (values + ('cohortdefinition:*:generate:') + ) t111 (lg) + cross join + ( select source_key + from vocab_source + ) t222 (mg) + cross join + (values + (':get') + ) t333 (rg) + ) combined +) +SELECT DISTINCT 15 role_id, permission_id + FROM ${ohdsiSchema}.sec_role_permission srp + INNER JOIN ${ohdsiSchema}.sec_permission sp ON srp.permission_id = sp.id + WHERE + sp.value IN (select perm from vocab_perms) + or + sp.value IN (select perm from source_perms) + or + sp.value IN (select perm from generate_perms) + or + sp.value IN + ( + 'cohort-characterization:byTags:post', + 'cohort-characterization:check:post', + 'cohort-characterization:get', + 'cohort-characterization:import:post', + 'cohort-characterization:post', + 'cohortanalysis:get', + 'cohortanalysis:post', + 'cohortdefinition:byTags:post', + 'cohortdefinition:check:post', + 'cohortdefinition:checkv2:post', + 'cohortdefinition:get', + 'cohortdefinition:post', + 'cohortdefinition:printfriendly:cohort:post', + 'cohortdefinition:printfriendly:conceptsets:post', + 'cohortdefinition:sql:post', + 'comparativecohortanalysis:get', + 'comparativecohortanalysis:post', + 'conceptset:byTags:post', + 'conceptset:check:post', + 'conceptset:get', + 'conceptset:post', + 'configuration:edit:ui', + 'estimation:check:post', + 'estimation:get', + 'estimation:import:post', + 'estimation:post', + 'executionservice:execution:run:post', + 'feasibility:get', + 'feature-analysis:aggregates:get', + 'feature-analysis:get', + 'feature-analysis:post', + 'ir:byTags:post', + 'ir:check:post', + 'ir:design:post', + 'ir:get', + 'ir:post', + 'ir:sql:post', + 'job:execution:get', + 'job:get', + 'notifications:get', + 'notifications:viewed:get', + 'notifications:viewed:post', + 'pathway-analysis:byTags:post', + 'pathway-analysis:check:post', + 'pathway-analysis:get', + 'pathway-analysis:import:post', + 'pathway-analysis:post', + 'plp:get', + 'plp:post', + 'prediction:get', + 'prediction:import:post', + 'prediction:post', + 'reusable:byTags:post', + 'reusable:get', + 'reusable:post', + 'source:daimon:priority:get', + 'source:priorityVocabulary:get', + 'sqlrender:translate:post', + 'tag:get', + 'tag:multiAssign:post', + 'tag:multiUnassign:post', + 'tag:post', + 'tag:search:get', + 'cohortdefinition:*:exists:get', -- weird one...but is needed / used by UI before saving a new cohortdefinition.... + 'conceptset:*:exists:get', -- weird one...but is needed / used by UI before saving a new conceptset.... + 'conceptset:*:expression:*:get', -- TODO - taken over from role 10...This one is probably too broad and will need further fixes. + 'conceptset:*:version:get' -- TODO - taken over from role 10...This one is probably too broad and will need further fixes. + ) +; + + +-- COHORT_DEFINITION_SEC_ROLE is our custom view that returns a list of cohort definition ids per role +-- as long as that role has a permission starting with "cohortdefinition:"" for that id. E.g. : +-- cohort_definition_id | sec_role_name +-- ----------------------+------------------------- +-- 8 | /gwas_projects/project2 +-- 9 | /gwas_projects/project2 +-- 300 | /gwas_projects/project1 + +DROP VIEW IF EXISTS ${ohdsiSchema}.COHORT_DEFINITION_SEC_ROLE; +CREATE VIEW ${ohdsiSchema}.COHORT_DEFINITION_SEC_ROLE AS + select + distinct cast(regexp_replace(sec_permission.value, + '^cohortdefinition:([0-9]+):.*','\1') as integer) as cohort_definition_id, + sec_role.name as sec_role_name + from + ${ohdsiSchema}.sec_role + inner join ${ohdsiSchema}.sec_role_permission on sec_role.id = sec_role_permission.role_id + inner join ${ohdsiSchema}.sec_permission on sec_role_permission.permission_id = sec_permission.id + where + sec_permission.value ~ 'cohortdefinition:[0-9]+' +; + +-- Below we create new "copy:get" permissions specific to each cohort definition (step 1), and +-- then tie these new permissions to the right role, according to the cohort definition id vs role name +-- mapping found in COHORT_DEFINITION_SEC_ROLE (step 2). + +SELECT setval('${ohdsiSchema}.sec_permission_sequence', (select max(id)+1 from ${ohdsiSchema}.sec_permission), false); + +-- 1. create the sec_permission records: +INSERT INTO ${ohdsiSchema}.sec_permission (value, description) +select + concat('cohortdefinition:', cohort_definition_id, ':copy:get'), + 'Copy the specified cohort definition' +from ${ohdsiSchema}.COHORT_DEFINITION_SEC_ROLE +ON CONFLICT (value) +DO NOTHING; + +-- 2. insert sec_role_permissions: +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +Select + sec_role.id, + sec_permission.id +from + ${ohdsiSchema}.COHORT_DEFINITION_SEC_ROLE + join ${ohdsiSchema}.sec_role on COHORT_DEFINITION_SEC_ROLE.sec_role_name = sec_role.name + join ${ohdsiSchema}.sec_permission on concat('cohortdefinition:', COHORT_DEFINITION_SEC_ROLE.cohort_definition_id, ':copy:get') = sec_permission.value +ON CONFLICT (role_id, permission_id) +DO NOTHING;