From c3e01d1b076abae4df4d1ed4f012096f13d20361 Mon Sep 17 00:00:00 2001 From: rkboyce Date: Fri, 2 Feb 2024 18:26:38 +0000 Subject: [PATCH 1/4] fix to the Atlas resd-restricted role to add permissions for all approved sources --- ...s_read_restricted_role_and_permissions.sql | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql diff --git a/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql b/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql new file mode 100644 index 0000000000..2cbfcc9b23 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql @@ -0,0 +1,226 @@ +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 +) +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 + ( + '*:cohortresults:*:breakdown:get', + '*:person:*:get:dates', + '*:vocabulary:lookup:identifiers:post', + 'cdmresults:*:get', + 'cohort-characterization:*:download:get', + 'cohort-characterization:*:exists:get', + 'cohort-characterization:*:export:conceptset:get', + 'cohort-characterization:*:export:get', + 'cohort-characterization:*:post', + 'cohort-characterization:*:tag:*:delete', + 'cohort-characterization:*:tag:post', + 'cohort-characterization:*:version:*:createAsset:put', + 'cohort-characterization:*:version:*:delete', + 'cohort-characterization:*:version:*:put', + 'cohort-characterization:byTags:post', + 'cohort-characterization:check:post', + 'cohort-characterization:generation:*:delete', + 'cohort-characterization:generation:*:design:get', + 'cohort-characterization:generation:*:explore:prevalence:*:*:*:get', + 'cohort-characterization:generation:*:result:count:get', + 'cohort-characterization:generation:*:result:export:post', + 'cohort-characterization:generation:*:result:get', + 'cohort-characterization:generation:*:result:post', + 'cohort-characterization:get', + 'cohort-characterization:import:post', + 'cohort-characterization:post', + 'cohortanalysis:*:get', + 'cohortanalysis:get', + 'cohortanalysis:post', + 'cohortdefinition:*:check:get', + 'cohortdefinition:*:check:post', + 'cohortdefinition:*:copy:get', + 'cohortdefinition:*:exists:get', + 'cohortdefinition:*:export:conceptset:get', + 'cohortdefinition:*:tag:*:delete', + 'cohortdefinition:*:tag:post', + 'cohortdefinition:*:version:*:createAsset:put', + 'cohortdefinition:*:version:*:delete', + 'cohortdefinition:*:version:*:put', + 'cohortdefinition:byTags:post', + 'cohortdefinition:check:post', + 'cohortdefinition:checkv2:post', + 'cohortdefinition:get', + 'cohortdefinition:post', + 'cohortdefinition:printfriendly:cohort:post', + 'cohortdefinition:printfriendly:conceptsets:post', + 'cohortdefinition:sql:post', + 'cohortresults:*:get', + 'cohortsample:*:*:*:delete', + 'cohortsample:*:*:*:get', + 'cohortsample:*:*:*:refresh:post', + 'cohortsample:*:*:delete', + 'cohortsample:*:*:get', + 'cohortsample:*:*:post', + 'comparativecohortanalysis:*:copy:get', + 'comparativecohortanalysis:*:delete', + 'comparativecohortanalysis:*:put', + 'comparativecohortanalysis:get', + 'comparativecohortanalysis:post', + 'conceptset:*:copy-name:get', + 'conceptset:*:exists:get', + 'conceptset:*:export:get', + 'conceptset:*:expression:*:get', + 'conceptset:*:generationinfo:get', + 'conceptset:*:tag:*:delete', + 'conceptset:*:tag:post', + 'conceptset:*:version:*:createAsset:put', + 'conceptset:*:version:*:delete', + 'conceptset:*:version:*:expression:*:get', + 'conceptset:*:version:*:get', + 'conceptset:*:version:*:put', + 'conceptset:*:version:get', + 'conceptset:byTags:post', + 'conceptset:check:post', + 'conceptset:get', + 'conceptset:post', + 'configuration:edit:ui', + 'estimation:*:exists:get', + 'estimation:*:generation:*:post', + 'estimation:check:post', + 'estimation:generation:*:result:get', + 'estimation:get', + 'estimation:import:post', + 'estimation:post', + 'evidence:*:drugconditionpairs:post', + 'evidence:*:druglabel:post', + 'evidence:*:get', + 'evidence:*:negativecontrols:*:get', + 'evidence:*:negativecontrols:post', + 'executionservice:*:get', + 'executionservice:execution:run:post', + 'feasibility:*:delete', + 'feasibility:*:get', + 'feasibility:*:put', + 'feasibility:get', + 'feature-analysis:*:copy:get', + 'feature-analysis:*:exists:get', + 'feature-analysis:*:export:conceptset:get', + 'feature-analysis:*:get', + 'feature-analysis:aggregates:get', + 'feature-analysis:get', + 'feature-analysis:post', + 'featureextraction:*:get', + 'gis:cohort:*:bounds:*:get', + 'gis:cohort:*:clusters:*:get', + 'gis:cohort:*:density:*:get', + 'gis:person:*:bounds:*:get', + 'gis:source:check:*:get', + 'ir:*:exists:get', + 'ir:*:tag:*:delete', + 'ir:*:tag:post', + 'ir:*:version:*:createAsset:put', + 'ir:*:version:*:delete', + 'ir:*:version:*:put', + 'ir:byTags:post', + 'ir:check:post', + 'ir:design:post', + 'ir:get', + 'ir:post', + 'ir:sql:post', + 'job:execution:get', + 'job:get', + 'job:type:*:name:*:get', + 'notifications:get', + 'notifications:viewed:get', + 'notifications:viewed:post', + 'pathway-analysis:*:exists:get', + 'pathway-analysis:*:export:get', + 'pathway-analysis:*:post', + 'pathway-analysis:*:tag:*:delete', + 'pathway-analysis:*:tag:post', + 'pathway-analysis:*:version:*:createAsset:put', + 'pathway-analysis:*:version:*:delete', + 'pathway-analysis:*:version:*:put', + 'pathway-analysis:byTags:post', + 'pathway-analysis:check:post', + 'pathway-analysis:get', + 'pathway-analysis:import:post', + 'pathway-analysis:post', + 'plp:*:copy:get', + 'plp:*:delete', + 'plp:*:put', + 'plp:get', + 'plp:post', + 'prediction:*:generation:*:post', + 'prediction:check:post', + 'prediction:generation:*:result:get', + 'prediction:get', + 'prediction:import:post', + 'prediction:post', + 'reusable:*:exists:get', + 'reusable:*:get', + 'reusable:*:post', + 'reusable:*:tag:*:delete', + 'reusable:*:tag:post', + 'reusable:*:version:*:createAsset:put', + 'reusable:*:version:*:delete', + 'reusable:*:version:*:get', + 'reusable:*:version:*:put', + 'reusable:*:version:get', + 'reusable:byTags:post', + 'reusable:get', + 'reusable:post', + 'source:*:get', + 'source:daimon:priority:get', + 'source:priorityVocabulary:get', + 'sqlrender:translate:post', + 'tag:*:delete', + 'tag:*:get', + 'tag:*:put', + 'tag:get', + 'tag:multiAssign:post', + 'tag:multiUnassign:post', + 'tag:post', + 'tag:search:get', + 'vocabulary:*:compare-arbitrary', + 'vocabulary:*:post' + ) +; From 700ee82982ec0350cefa418a3ae14729cb0f97d4 Mon Sep 17 00:00:00 2001 From: rkboyce Date: Fri, 2 Feb 2024 18:27:26 +0000 Subject: [PATCH 2/4] file path name fix --- ...130082100__fix_atlas_read_restricted_role_and_permissions.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/postgresql/{V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql => V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql} (100%) diff --git a/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql b/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql similarity index 100% rename from src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_altas_read_restricted_role_and_permissions.sql rename to src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql From 939ef5798ca47222f45cb6700ae21338d552db6f Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 23 Aug 2024 17:54:19 +0200 Subject: [PATCH 3/4] fix: remove all * permissions from role 15 This risks being a bit too blunt and some functionality might stop working for some of the removed parts. In these cases, we'll have to add these parts back one by one without a * permission, and test. So far, this has been mainly tested for cohortdefinition and conceptsets. Anyway, I judged this better than having too broad permissions and resulting security incidents. --- ...s_read_restricted_role_and_permissions.sql | 157 +++++------------- 1 file changed, 43 insertions(+), 114 deletions(-) diff --git a/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql b/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql index 2cbfcc9b23..162037c4e2 100644 --- a/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql +++ b/src/main/resources/db/migration/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql @@ -1,3 +1,6 @@ +-- When using role 15, remember to remove the role "source user" from all users. +-- See also https://github.com/OHDSI/WebAPI/wiki/Read-restricted-Configuration + delete from ${ohdsiSchema}.sec_role_permission where role_id = 15; INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) @@ -36,54 +39,60 @@ with vocab_source as ( (':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 ( - '*:cohortresults:*:breakdown:get', - '*:person:*:get:dates', - '*:vocabulary:lookup:identifiers:post', - 'cdmresults:*:get', - 'cohort-characterization:*:download:get', - 'cohort-characterization:*:exists:get', - 'cohort-characterization:*:export:conceptset:get', - 'cohort-characterization:*:export:get', - 'cohort-characterization:*:post', - 'cohort-characterization:*:tag:*:delete', - 'cohort-characterization:*:tag:post', - 'cohort-characterization:*:version:*:createAsset:put', - 'cohort-characterization:*:version:*:delete', - 'cohort-characterization:*:version:*:put', 'cohort-characterization:byTags:post', 'cohort-characterization:check:post', - 'cohort-characterization:generation:*:delete', - 'cohort-characterization:generation:*:design:get', - 'cohort-characterization:generation:*:explore:prevalence:*:*:*:get', - 'cohort-characterization:generation:*:result:count:get', - 'cohort-characterization:generation:*:result:export:post', - 'cohort-characterization:generation:*:result:get', - 'cohort-characterization:generation:*:result:post', 'cohort-characterization:get', 'cohort-characterization:import:post', 'cohort-characterization:post', - 'cohortanalysis:*:get', 'cohortanalysis:get', 'cohortanalysis:post', - 'cohortdefinition:*:check:get', - 'cohortdefinition:*:check:post', - 'cohortdefinition:*:copy:get', - 'cohortdefinition:*:exists:get', - 'cohortdefinition:*:export:conceptset:get', - 'cohortdefinition:*:tag:*:delete', - 'cohortdefinition:*:tag:post', - 'cohortdefinition:*:version:*:createAsset:put', - 'cohortdefinition:*:version:*:delete', - 'cohortdefinition:*:version:*:put', 'cohortdefinition:byTags:post', 'cohortdefinition:check:post', 'cohortdefinition:checkv2:post', @@ -92,73 +101,22 @@ SELECT DISTINCT 15 role_id, permission_id 'cohortdefinition:printfriendly:cohort:post', 'cohortdefinition:printfriendly:conceptsets:post', 'cohortdefinition:sql:post', - 'cohortresults:*:get', - 'cohortsample:*:*:*:delete', - 'cohortsample:*:*:*:get', - 'cohortsample:*:*:*:refresh:post', - 'cohortsample:*:*:delete', - 'cohortsample:*:*:get', - 'cohortsample:*:*:post', - 'comparativecohortanalysis:*:copy:get', - 'comparativecohortanalysis:*:delete', - 'comparativecohortanalysis:*:put', 'comparativecohortanalysis:get', 'comparativecohortanalysis:post', - 'conceptset:*:copy-name:get', - 'conceptset:*:exists:get', - 'conceptset:*:export:get', - 'conceptset:*:expression:*:get', - 'conceptset:*:generationinfo:get', - 'conceptset:*:tag:*:delete', - 'conceptset:*:tag:post', - 'conceptset:*:version:*:createAsset:put', - 'conceptset:*:version:*:delete', - 'conceptset:*:version:*:expression:*:get', - 'conceptset:*:version:*:get', - 'conceptset:*:version:*:put', - 'conceptset:*:version:get', 'conceptset:byTags:post', 'conceptset:check:post', 'conceptset:get', 'conceptset:post', 'configuration:edit:ui', - 'estimation:*:exists:get', - 'estimation:*:generation:*:post', 'estimation:check:post', - 'estimation:generation:*:result:get', 'estimation:get', 'estimation:import:post', 'estimation:post', - 'evidence:*:drugconditionpairs:post', - 'evidence:*:druglabel:post', - 'evidence:*:get', - 'evidence:*:negativecontrols:*:get', - 'evidence:*:negativecontrols:post', - 'executionservice:*:get', 'executionservice:execution:run:post', - 'feasibility:*:delete', - 'feasibility:*:get', - 'feasibility:*:put', 'feasibility:get', - 'feature-analysis:*:copy:get', - 'feature-analysis:*:exists:get', - 'feature-analysis:*:export:conceptset:get', - 'feature-analysis:*:get', 'feature-analysis:aggregates:get', 'feature-analysis:get', 'feature-analysis:post', - 'featureextraction:*:get', - 'gis:cohort:*:bounds:*:get', - 'gis:cohort:*:clusters:*:get', - 'gis:cohort:*:density:*:get', - 'gis:person:*:bounds:*:get', - 'gis:source:check:*:get', - 'ir:*:exists:get', - 'ir:*:tag:*:delete', - 'ir:*:tag:post', - 'ir:*:version:*:createAsset:put', - 'ir:*:version:*:delete', - 'ir:*:version:*:put', 'ir:byTags:post', 'ir:check:post', 'ir:design:post', @@ -167,60 +125,31 @@ SELECT DISTINCT 15 role_id, permission_id 'ir:sql:post', 'job:execution:get', 'job:get', - 'job:type:*:name:*:get', 'notifications:get', 'notifications:viewed:get', 'notifications:viewed:post', - 'pathway-analysis:*:exists:get', - 'pathway-analysis:*:export:get', - 'pathway-analysis:*:post', - 'pathway-analysis:*:tag:*:delete', - 'pathway-analysis:*:tag:post', - 'pathway-analysis:*:version:*:createAsset:put', - 'pathway-analysis:*:version:*:delete', - 'pathway-analysis:*:version:*:put', 'pathway-analysis:byTags:post', 'pathway-analysis:check:post', 'pathway-analysis:get', 'pathway-analysis:import:post', 'pathway-analysis:post', - 'plp:*:copy:get', - 'plp:*:delete', - 'plp:*:put', 'plp:get', 'plp:post', - 'prediction:*:generation:*:post', - 'prediction:check:post', - 'prediction:generation:*:result:get', 'prediction:get', 'prediction:import:post', 'prediction:post', - 'reusable:*:exists:get', - 'reusable:*:get', - 'reusable:*:post', - 'reusable:*:tag:*:delete', - 'reusable:*:tag:post', - 'reusable:*:version:*:createAsset:put', - 'reusable:*:version:*:delete', - 'reusable:*:version:*:get', - 'reusable:*:version:*:put', - 'reusable:*:version:get', 'reusable:byTags:post', 'reusable:get', 'reusable:post', - 'source:*:get', 'source:daimon:priority:get', 'source:priorityVocabulary:get', 'sqlrender:translate:post', - 'tag:*:delete', - 'tag:*:get', - 'tag:*:put', 'tag:get', 'tag:multiAssign:post', 'tag:multiUnassign:post', 'tag:post', 'tag:search:get', - 'vocabulary:*:compare-arbitrary', - 'vocabulary:*:post' + '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.... ) ; From fc7b2961df0d69a14ef8f3b2dcab0329e92c4f78 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 23 Aug 2024 17:55:40 +0200 Subject: [PATCH 4/4] fix: add missing read permissions to PermissionSchema classes --- .../model/CohortDefinitionPermissionSchema.java | 14 +++++++------- .../security/model/ConceptSetPermissionSchema.java | 3 +++ 2 files changed, 10 insertions(+), 7 deletions(-) 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 bb6781ae0a..e5632c823e 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/security/model/ConceptSetPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java index 66b4b1a4b2..73b8ed89ce 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java @@ -18,6 +18,9 @@ public class ConceptSetPermissionSchema extends EntityPermissionSchema { put("conceptset:%s:get", "view conceptset definition with id %s"); put("conceptset:%s:expression:get", "Resolve concept set %s expression"); put("conceptset:%s:version:*:expression:get", "Get expression for concept set %s items for default source"); + put("conceptset:%s:expression:*:get", "expression:*:get permission, specific to this conceptset with id %s"); + put("conceptset:%s:version:get", "version:get permission, specific to this conceptset with id %s"); + put("conceptset:%s:copy-name:get", "copy-name:get permission, specific to this conceptset with id %s"); }}; public ConceptSetPermissionSchema() {