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.

Sign up for GitHub

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

Jump to bottom

Fix/flyway atlas read only user #2342

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix/flyway atlas read only user #2342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c3e01d1
fix to the Atlas resd-restricted role to add permissions for all appr…
rkboyce Feb 2, 2024
700ee82
file path name fix
rkboyce Feb 2, 2024
939ef57
fix: remove all * permissions from role 15
pieterlukasse Aug 23, 2024
fc7b296
fix: add missing read permissions to PermissionSchema classes
pieterlukasse Aug 23, 2024
80c9780
Merge pull request #8 from pieterlukasse/patch-4
rkboyce Sep 2, 2024
5b91043
Merge pull request #7 from pieterlukasse/patch-3
rkboyce Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/main/java/org/ohdsi/webapi/security/model/CohortDefinitionPermissionSchema.java
View file
Open in desktop
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
3 changes: 3 additions & 0 deletions src/main/java/org/ohdsi/webapi/security/model/ConceptSetPermissionSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
155 changes: 155 additions & 0 deletions ...ion/postgresql/V2.14.1.20240130082100__fix_atlas_read_restricted_role_and_permissions.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
-- 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)
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
Comment on lines +16 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from (values
('vocabulary:')
) t1 (l)
cross join
( select source_key
from vocab_source
) t2 (m)
cross join
from (values
('vocabulary:*')
) t1 (l)
cross join

feedback from Chris ^ ?

Copy link
Contributor

@pieterlukasse pieterlukasse Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one issue with this is that permissions like vocabulary:*:search:*:get do not yet exist in the sec_permission table, so these would first have to be added as well (maybe as a first step of this migration script).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, and that would be needed otherwise it won't find any permission IDs to assign to the role.

(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....
)
;
Loading