Feat: restrict the read restricted role

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<String, String> readPermissions = new HashMap<String, String>() {{                                     
-	put("cohortdefinition:%s:get", "Get Cohort Definition by ID");
-	put("cohortdefinition:%s:info:get","");
+    private static Map<String, String> readPermissions = new HashMap<String, String>() {{                                     
+        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() {
 

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")
       }

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;
     }
   }
 

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();
   }

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;