Skip to content

Commit

Permalink
Feat: restrict the read restricted role (#142)
Browse files Browse the repository at this point in the history
* feat: remove the * permissions

* fix: remove extra item from concat(l,m,r)

* tmp: temporarily disable conflicting check

* fix: put back the regular vocabulary: permissions

* tmp: disable "source user" role assignment

* tmp: rename flyway script

* fix: ensure source:omop:access becomes part of role 15

* tmp: rename sql migration script

* fix: make sure copy permission is part of the default permission schema for cohortdefinition

* fix: add cohortdefinition:*:exists:get permission to role 15

* fix: revert copy permission part

...as this would cause the current code to filter out all
cohorts. Current code requires the user to have ALL read permissions listed
in the schema to see a cohort definition...

* fix: add cohortdefinition:*:copy:get permisstion to role 15

* Revert "fix: revert copy permission part"

This reverts commit 8c9caf9.

* feat: migration script to add copy:get permission to teamproject cohorts

* fix: set permissionEntity to use right sequence

* fix: fix the migration script / schema name part for setval

* feat: migration script to add generate:SOURCE:get permission to role 15

* fix: added extra conceptset permissions to role 15, some of which will need review

...and fixing in ConceptSetPermissionSchema.java

* fix: support two authorization rules, where one should match the method and service expected

* fix: remove temporary solution for "Source user"

... as we have now moved the most relevant permissions into role 15

* fix: format in CohortDefinitionPermissionSchema.java
  • Loading branch information
pieterlukasse authored Jun 19, 2024
1 parent fbebf9a commit 6261dff
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit 6261dff

Please sign in to comment.