From 8fcebdc29b6e82f22a7d7c162a474ee07fedd905 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 29 May 2024 20:00:01 +0200 Subject: [PATCH 01/36] ctds integration changes feat: add CTDS CI build and push Co-authored-by: Andrew Prokhorenkov --- .github/workflows/image_build_push.yaml | 16 + .pre-commit-config.yaml | 11 + .secrets.baseline | 501 ++++++++++++++++++++++++ Dockerfile | 2 +- 4 files changed, 529 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/image_build_push.yaml create mode 100644 .pre-commit-config.yaml create mode 100644 .secrets.baseline diff --git a/.github/workflows/image_build_push.yaml b/.github/workflows/image_build_push.yaml new file mode 100644 index 0000000000..7598078802 --- /dev/null +++ b/.github/workflows/image_build_push.yaml @@ -0,0 +1,16 @@ +name: Build Image and Push to Quay + +on: push + +jobs: + ci: + name: Build Image and Push to Quay + uses: uc-cdis/.github/.github/workflows/image_build_push.yaml@master + with: + OVERRIDE_REPO_NAME: "ohdsi-webapi" + BUILD_PLATFORMS: "linux/amd64" + secrets: + ECR_AWS_ACCESS_KEY_ID: ${{ secrets.ECR_AWS_ACCESS_KEY_ID }} + ECR_AWS_SECRET_ACCESS_KEY: ${{ secrets.ECR_AWS_SECRET_ACCESS_KEY }} + QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }} + QUAY_ROBOT_TOKEN: ${{ secrets.QUAY_ROBOT_TOKEN }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000000..8412d22a48 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,11 @@ +repos: +- repo: git@github.com:Yelp/detect-secrets + rev: v1.3.0 + hooks: + - id: detect-secrets + args: ['--baseline', '.secrets.baseline'] +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.3.0 + hooks: + - id: no-commit-to-branch + args: [--branch, develop, --branch, master, --pattern, release/.*] diff --git a/.secrets.baseline b/.secrets.baseline new file mode 100644 index 0000000000..748b130a27 --- /dev/null +++ b/.secrets.baseline @@ -0,0 +1,501 @@ +{ + "version": "1.3.0", + "plugins_used": [ + { + "name": "ArtifactoryDetector" + }, + { + "name": "AWSKeyDetector" + }, + { + "name": "AzureStorageKeyDetector" + }, + { + "name": "Base64HighEntropyString", + "limit": 4.5 + }, + { + "name": "BasicAuthDetector" + }, + { + "name": "CloudantDetector" + }, + { + "name": "GitHubTokenDetector" + }, + { + "name": "HexHighEntropyString", + "limit": 3.0 + }, + { + "name": "IbmCloudIamDetector" + }, + { + "name": "IbmCosHmacDetector" + }, + { + "name": "JwtTokenDetector" + }, + { + "name": "KeywordDetector", + "keyword_exclude": "" + }, + { + "name": "MailchimpDetector" + }, + { + "name": "NpmDetector" + }, + { + "name": "PrivateKeyDetector" + }, + { + "name": "SendGridDetector" + }, + { + "name": "SlackDetector" + }, + { + "name": "SoftlayerDetector" + }, + { + "name": "SquareOAuthDetector" + }, + { + "name": "StripeDetector" + }, + { + "name": "TwilioKeyDetector" + } + ], + "filters_used": [ + { + "path": "detect_secrets.filters.allowlist.is_line_allowlisted" + }, + { + "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", + "min_level": 2 + }, + { + "path": "detect_secrets.filters.heuristic.is_indirect_reference" + }, + { + "path": "detect_secrets.filters.heuristic.is_likely_id_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_lock_file" + }, + { + "path": "detect_secrets.filters.heuristic.is_not_alphanumeric_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_potential_uuid" + }, + { + "path": "detect_secrets.filters.heuristic.is_prefixed_with_dollar_sign" + }, + { + "path": "detect_secrets.filters.heuristic.is_sequential_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_swagger_file" + }, + { + "path": "detect_secrets.filters.heuristic.is_templated_secret" + } + ], + "results": { + "src/main/java/org/ohdsi/webapi/Constants.java": [ + { + "type": "Secret Keyword", + "filename": "src/main/java/org/ohdsi/webapi/Constants.java", + "hashed_secret": "36bf8bf280c025ae1b21209f025c5ab89f5a5fde", + "is_verified": false, + "line_number": 61 + } + ], + "src/main/java/org/ohdsi/webapi/db/migartion/V2_7_2_20190515164044__hideSensitiveInfo.java": [ + { + "type": "Base64 High Entropy String", + "filename": "src/main/java/org/ohdsi/webapi/db/migartion/V2_7_2_20190515164044__hideSensitiveInfo.java", + "hashed_secret": "beff15da86201b1f12daa42c4d4daa5d2bd1a81d", + "is_verified": false, + "line_number": 28 + } + ], + "src/main/java/org/ohdsi/webapi/db/migartion/V2_8_0_20190410103000__migratePathwayResults.java": [ + { + "type": "Base64 High Entropy String", + "filename": "src/main/java/org/ohdsi/webapi/db/migartion/V2_8_0_20190410103000__migratePathwayResults.java", + "hashed_secret": "bdd2eec10cbb782c92c4275938f8fdd9861519d7", + "is_verified": false, + "line_number": 35 + } + ], + "src/main/java/org/ohdsi/webapi/source/SourceService.java": [ + { + "type": "Secret Keyword", + "filename": "src/main/java/org/ohdsi/webapi/source/SourceService.java", + "hashed_secret": "f3286de6fd32c59d435a497a503b0d617cfc1957", + "is_verified": false, + "line_number": 71 + } + ], + "src/main/resources/i18n/messages_en.json": [ + { + "type": "Secret Keyword", + "filename": "src/main/resources/i18n/messages_en.json", + "hashed_secret": "8be3c943b1609fffbfc51aad666d0a04adf83c9d", + "is_verified": false, + "line_number": 1102 + } + ], + "src/main/resources/saml/dev/idp-metadata-okta.xml": [ + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "cb6a50097700afce86d92676d2f147ef94be927d", + "is_verified": false, + "line_number": 9 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "cb030c9aa0bdd77941abdf05ed80af5824602763", + "is_verified": false, + "line_number": 10 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "a9ffb06909b4119f5bb62fa018853d24363ab03f", + "is_verified": false, + "line_number": 11 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "eb2af7e771d17d4edf1bd8b19999d748741417f9", + "is_verified": false, + "line_number": 12 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "d01de86f3203b1f2eee42403117abe9f4cceda45", + "is_verified": false, + "line_number": 13 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "0d4cb13af14741d0f1391987830a6da7ee64de31", + "is_verified": false, + "line_number": 14 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "9ca2c90ee357b96b1c1877786979d9b4de229992", + "is_verified": false, + "line_number": 15 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "d6801c07c94ea7a90b3a27ba5ad336562797b6c2", + "is_verified": false, + "line_number": 16 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "8074752ac6cb13dc5b320612b1908bce40126dc2", + "is_verified": false, + "line_number": 17 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "fa4a269b937eeeb2e899ae06f566c03d922901a5", + "is_verified": false, + "line_number": 18 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "353be20fb07c72ac88309652dbbe105555929047", + "is_verified": false, + "line_number": 19 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "219a007a9b87f8eb7e992a4ff586476b9a1a11d3", + "is_verified": false, + "line_number": 20 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "60730ef29a971c3cfc4df9d093f8a5abf95859ed", + "is_verified": false, + "line_number": 21 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "8d1951aea82f86f3b6cd0470a51706bf7fec3716", + "is_verified": false, + "line_number": 22 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "fe223100b2b6a1c1f14e5ce57de00ed6010d0a8b", + "is_verified": false, + "line_number": 23 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/idp-metadata-okta.xml", + "hashed_secret": "ce19604bbbc57bfa19822dec9da5477ceb6a9ae2", + "is_verified": false, + "line_number": 24 + } + ], + "src/main/resources/saml/dev/sp-metadata-okta.xml": [ + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "53792807b60da72427adc2a5d8888c3b4eed806d", + "is_verified": false, + "line_number": 10 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "db1d320d3af18c2cee924f080611f8d2371adac3", + "is_verified": false, + "line_number": 11 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "33a99cc58ae79b3883abb9b599e09de31869dbff", + "is_verified": false, + "line_number": 12 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "7aa5001262f0de15c6e3b594ec5e49260b8f3285", + "is_verified": false, + "line_number": 13 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "fb2607f22827bc5016c0632cf97f0249403e8d9f", + "is_verified": false, + "line_number": 14 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "ade963b9c8b82d3f39e2a3c732578068e2e354ed", + "is_verified": false, + "line_number": 15 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "56bb3d8139532beb1223b596da87e2f6bb5cd705", + "is_verified": false, + "line_number": 16 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "dbf09395fc349a7608ce71bf849a3b01c6a41ee2", + "is_verified": false, + "line_number": 17 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "c7c5126e4259a5fb1ac2c05c966f61623ce7a96e", + "is_verified": false, + "line_number": 18 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "f36e69c2d71f07cdd769e3fc160737fd680d581e", + "is_verified": false, + "line_number": 19 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "e43adde2b6012380ccd00f63dbf4318e69133fd0", + "is_verified": false, + "line_number": 20 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "37bf6f5836588898ab8c9edf312241300ffbd320", + "is_verified": false, + "line_number": 21 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "edc037dbef474ecbd02bea6e1884b8d39accb104", + "is_verified": false, + "line_number": 22 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata-okta.xml", + "hashed_secret": "aa647b01661ad734d6218021f205dcb9e84befae", + "is_verified": false, + "line_number": 23 + } + ], + "src/main/resources/saml/dev/sp-metadata.xml": [ + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "53792807b60da72427adc2a5d8888c3b4eed806d", + "is_verified": false, + "line_number": 10 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "db1d320d3af18c2cee924f080611f8d2371adac3", + "is_verified": false, + "line_number": 11 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "33a99cc58ae79b3883abb9b599e09de31869dbff", + "is_verified": false, + "line_number": 12 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "7aa5001262f0de15c6e3b594ec5e49260b8f3285", + "is_verified": false, + "line_number": 13 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "fb2607f22827bc5016c0632cf97f0249403e8d9f", + "is_verified": false, + "line_number": 14 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "ade963b9c8b82d3f39e2a3c732578068e2e354ed", + "is_verified": false, + "line_number": 15 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "56bb3d8139532beb1223b596da87e2f6bb5cd705", + "is_verified": false, + "line_number": 16 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "dbf09395fc349a7608ce71bf849a3b01c6a41ee2", + "is_verified": false, + "line_number": 17 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "c7c5126e4259a5fb1ac2c05c966f61623ce7a96e", + "is_verified": false, + "line_number": 18 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "f36e69c2d71f07cdd769e3fc160737fd680d581e", + "is_verified": false, + "line_number": 19 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "e43adde2b6012380ccd00f63dbf4318e69133fd0", + "is_verified": false, + "line_number": 20 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "37bf6f5836588898ab8c9edf312241300ffbd320", + "is_verified": false, + "line_number": 21 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "edc037dbef474ecbd02bea6e1884b8d39accb104", + "is_verified": false, + "line_number": 22 + }, + { + "type": "Base64 High Entropy String", + "filename": "src/main/resources/saml/dev/sp-metadata.xml", + "hashed_secret": "aa647b01661ad734d6218021f205dcb9e84befae", + "is_verified": false, + "line_number": 23 + } + ], + "src/test/java/org/ohdsi/webapi/util/DataSourceDTOParserTest.java": [ + { + "type": "Secret Keyword", + "filename": "src/test/java/org/ohdsi/webapi/util/DataSourceDTOParserTest.java", + "hashed_secret": "f1c71f96792fdd4a57765618dfd0487aa8ac5d71", + "is_verified": false, + "line_number": 15 + }, + { + "type": "Secret Keyword", + "filename": "src/test/java/org/ohdsi/webapi/util/DataSourceDTOParserTest.java", + "hashed_secret": "4656aa812a80c7fc0c6dc70912e040025a9b72e9", + "is_verified": false, + "line_number": 29 + }, + { + "type": "Secret Keyword", + "filename": "src/test/java/org/ohdsi/webapi/util/DataSourceDTOParserTest.java", + "hashed_secret": "1982359f5876305fd807997058a4ef11ec493dee", + "is_verified": false, + "line_number": 30 + } + ], + "src/test/resources/application-test.properties": [ + { + "type": "Secret Keyword", + "filename": "src/test/resources/application-test.properties", + "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", + "is_verified": false, + "line_number": 27 + } + ] + }, + "generated_at": "2022-07-30T01:42:07Z" +} diff --git a/Dockerfile b/Dockerfile index 5667da9724..f14d0a56c4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM maven:3.6-jdk-11 as builder +FROM quay.io/cdis/maven:3.6-jdk-11 as builder WORKDIR /code From 628681d45fd420d29e685ce17efa474ade5f9a2a Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 30 Aug 2023 19:59:37 +0200 Subject: [PATCH 02/36] feat: prototype for teamProject solution - feat: introduce custom configuration option Update pom.xml with a better default authorization url - feat: improve logging of jwt - fix: add "Atlas users" as default system role - feat: add more log statements for PermissionManager - feat: ensure /user/me endpoint also triggers the UPDATE_TOKEN filter - feat: ensure the teamproject is stored per user ...and allow reading current teamproject from cache in case of a request to /user/refresh endpoint - feat: main logic in new filter class TeamProjectBasedAuthorizingFilter - fix: ensure reset of roles always happens - feat: remove unnecessary method from PermissionManager - fix: use lower() in SQL query itself for finding login - fix: take login from shiro-parsed principal instead of DB ... to avoid the issue where the login is all lowercase in db - feat: move the defaultRoles definition into AtlasSecurity - fix: move authorizationMode check to PostConstruct ...to avoid NullPointerException as attributes are not yet wired when in constructor - fix: remove session.stop() call from UpdateAccessTokenFilter ...and therefore from the flow of endpoints like /user/refresh. Not sure why this was added there, as the /user/logout should be the place to remove a session. This solves a org.apache.shiro.subject.support.DisabledSessionException. If the worry is that logout won`t be called, then the expiry time should just be set to a short period. The adjustment in JwtAuthRealm.java was to deal with the side effect that occurred after the removal of the .stop in the UpdateAccessTokenFilter filter: java.lang.ClassCastException: io.buji.pac4j.subject.Pac4jPrincipal cannot be cast to java.lang.String - fix: do not create a new session when requesting current session --- pom.xml | 6 +- .../webapi/security/PermissionService.java | 6 +- .../model/EntityPermissionSchema.java | 25 +- .../org/ohdsi/webapi/service/UserService.java | 2 +- .../webapi/shiro/Entities/UserRepository.java | 4 +- .../webapi/shiro/Entities/UserRoleEntity.java | 5 + .../ohdsi/webapi/shiro/PermissionManager.java | 163 ++++++++++-- .../TeamProjectBasedAuthorizingFilter.java | 250 ++++++++++++++++++ .../filters/UpdateAccessTokenFilter.java | 7 +- .../management/AtlasRegularSecurity.java | 11 +- .../shiro/management/AtlasSecurity.java | 15 +- .../shiro/management/FilterChainBuilder.java | 11 +- .../shiro/management/FilterTemplates.java | 1 + .../datasource/BaseDataSourceAccessor.java | 10 +- .../webapi/shiro/realms/JwtAuthRealm.java | 11 +- .../ohdsi/webapi/source/SourceService.java | 15 +- src/main/resources/application.properties | 4 + 17 files changed, 509 insertions(+), 37 deletions(-) create mode 100644 src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java diff --git a/pom.xml b/pom.xml index c3778b5502..ad77074a10 100644 --- a/pom.xml +++ b/pom.xml @@ -78,6 +78,8 @@ ISOLATION_READ_COMMITTED default + teamproject + http://arborist-service/auth/mapping DisabledSecurity 43200 http://localhost @@ -191,7 +193,7 @@ - true + false 8080 @@ -229,7 +231,7 @@ 200 true info - info + debug info info info diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionService.java b/src/main/java/org/ohdsi/webapi/security/PermissionService.java index cc510579dd..a10e6004ce 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionService.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionService.java @@ -62,6 +62,9 @@ public class PermissionService { @Value("#{!'${security.provider}'.equals('DisabledSecurity')}") private boolean securityEnabled; + @Value("${security.ohdsi.custom.authorization.mode}") + private String authorizationMode; + private final EntityGraph PERMISSION_ENTITY_GRAPH = EntityGraphUtils.fromAttributePaths("rolePermissions", "rolePermissions.role"); public PermissionService( @@ -189,7 +192,8 @@ public boolean hasAccess(CommonEntity entity, AccessType accessType) { Subject subject = SecurityUtils.getSubject(); String login = this.permissionManager.getSubjectName(); UserSimpleAuthorizationInfo authorizationInfo = this.permissionManager.getAuthorizationInfo(login); - if (Objects.equals(authorizationInfo.getUserId(), entity.getCreatedBy().getId())) { + if (!this.authorizationMode.equals("teamproject") && + Objects.equals(authorizationInfo.getUserId(), entity.getCreatedBy().getId())) { hasAccess = true; // the role is the one that created the artifact } else { EntityType entityType = entityPermissionSchemaResolver.getEntityType(entity.getClass()); diff --git a/src/main/java/org/ohdsi/webapi/security/model/EntityPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/EntityPermissionSchema.java index aab9017933..2469143421 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/EntityPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/EntityPermissionSchema.java @@ -2,19 +2,27 @@ import org.ohdsi.webapi.model.CommonEntity; import org.ohdsi.webapi.shiro.Entities.RoleEntity; +import org.ohdsi.webapi.shiro.filters.UpdateAccessTokenFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.ohdsi.webapi.shiro.PermissionManager; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import java.util.Collections; import java.util.HashMap; import java.util.Map; public abstract class EntityPermissionSchema { + private final Logger logger = LoggerFactory.getLogger(UpdateAccessTokenFilter.class); private final EntityType entityType; private final Map readPermissions; private final Map writePermissions; + @Value("${security.ohdsi.custom.authorization.mode}") + private String authorizationMode; + @Autowired protected PermissionManager permissionManager; @@ -49,8 +57,12 @@ public Map getAllPermissions() { } public void onInsert(CommonEntity commonEntity) { - - addPermissionsToCurrentUserFromTemplate(commonEntity, getAllPermissions()); + logger.debug("AUTHORIZATION_MODE in EntityPermissionSchema == '{}'", this.authorizationMode); + if (this.authorizationMode.equals("teamproject")) { + addPermissionsToCurrentTeamProjectFromTemplate(commonEntity, getAllPermissions()); + } else { + addPermissionsToCurrentUserFromTemplate(commonEntity, getAllPermissions()); + } } public void onDelete(CommonEntity commonEntity) { @@ -65,4 +77,13 @@ protected void addPermissionsToCurrentUserFromTemplate(CommonEntity commonEntity RoleEntity role = permissionManager.getUserPersonalRole(login); permissionManager.addPermissionsFromTemplate(role, template, commonEntity.getId().toString()); } + + protected void addPermissionsToCurrentTeamProjectFromTemplate(CommonEntity commonEntity, Map template) { + + RoleEntity role = permissionManager.getCurrentTeamProjectRoleForCurrentUser(); + if (role == null) { + throw new RuntimeException("Expected a teamproject role but found none!"); + } + permissionManager.addPermissionsFromTemplate(role, template, commonEntity.getId().toString()); + } } diff --git a/src/main/java/org/ohdsi/webapi/service/UserService.java b/src/main/java/org/ohdsi/webapi/service/UserService.java index 5dd56bcc18..4388972db1 100644 --- a/src/main/java/org/ohdsi/webapi/service/UserService.java +++ b/src/main/java/org/ohdsi/webapi/service/UserService.java @@ -165,7 +165,7 @@ public Role createRole(Role role) throws Exception { public Role updateRole(@PathParam("roleId") Long id, Role role) throws Exception { RoleEntity roleEntity = this.authorizer.getRole(id); if (roleEntity == null) { - throw new Exception("Role doesn't exist"); + throw new Exception("Role doesn't exist: " + id); } roleEntity.setName(role.role); roleEntity = this.authorizer.updateRole(roleEntity); diff --git a/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRepository.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRepository.java index 7c8112c5c0..007f871636 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRepository.java +++ b/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRepository.java @@ -4,13 +4,15 @@ import java.util.Set; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.CrudRepository; +import org.springframework.data.repository.query.Param; /** * Created by GMalikov on 24.08.2015. */ public interface UserRepository extends CrudRepository { - public UserEntity findByLogin(String login); + @Query("SELECT u FROM UserEntity u WHERE lower(u.login) = lower(:login)") + public UserEntity findByLogin(@Param("login") String login); @Query("SELECT u.login FROM UserEntity u") public Set getUserLogins(); diff --git a/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRoleEntity.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRoleEntity.java index 2b3e2ba9f2..544e0ab5bd 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRoleEntity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/Entities/UserRoleEntity.java @@ -87,4 +87,9 @@ public UserOrigin getOrigin() { public void setOrigin(UserOrigin origin) { this.origin = origin; } + + public String toString() { + role = this.getRole(); + return (role != null ? role.getName() : ""); + } } diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 43ff4ace64..239ba58a1f 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -3,6 +3,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.odysseusinc.logging.event.AddUserEvent; import com.odysseusinc.logging.event.DeleteRoleEvent; + +import io.buji.pac4j.subject.Pac4jPrincipal; + import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.UnknownAccountException; import org.apache.shiro.subject.Subject; @@ -20,14 +23,17 @@ import org.ohdsi.webapi.shiro.Entities.UserRepository; import org.ohdsi.webapi.shiro.Entities.UserRoleEntity; import org.ohdsi.webapi.shiro.Entities.UserRoleRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; import java.security.Principal; -import java.util.ArrayList; +import java.util.AbstractMap; import java.util.HashMap; +import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -49,6 +55,7 @@ @Component @Transactional public class PermissionManager { + private final Logger logger = LoggerFactory.getLogger(PermissionManager.class); @Value("${datasource.ohdsi.schema}") private String ohdsiSchema; @@ -76,12 +83,16 @@ public class PermissionManager { private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); + private Map, String> teamProjectRoles = new HashMap<>(); + public static class PermissionsDTO { public Map> permissions = null; } public RoleEntity addRole(String roleName, boolean isSystem) { + logger.debug("Called addRole: {}", roleName); + Guard.checkNotEmpty(roleName); checkRoleIsAbsent(roleName, isSystem, "Can't create role - it already exists"); @@ -119,8 +130,32 @@ public void removeUserFromRole(String roleName, String login, UserOrigin origin) UserEntity user = this.getUserByLogin(login); UserRoleEntity userRole = this.userRoleRepository.findByUserAndRole(user, role); - if (userRole != null && (origin == null || origin.equals(userRole.getOrigin()))) + if (userRole != null && (origin == null || origin.equals(userRole.getOrigin()))) { + logger.debug("Removing user from role: {}, {}, {}", user.getLogin(), role.getName(), userRole.getOrigin()); this.userRoleRepository.delete(userRole); + } + } + + public void removeUserFromUserRole(String roleName, String login) { + Guard.checkNotEmpty(roleName); + Guard.checkNotEmpty(login); + + if (roleName.equalsIgnoreCase(login)) + throw new RuntimeException("Can't remove user from personal role"); + + logger.debug("Checking if role exists: {}", roleName); + RoleEntity role = this.roleRepository.findByNameAndSystemRole(roleName, false); + if (role != null) { + UserEntity user = this.getUserByLogin(login); + + UserRoleEntity userRole = this.userRoleRepository.findByUserAndRole(user, role); + if (userRole != null) { + logger.debug("Removing user from USER role: {}, {}", user.getLogin(), roleName); + this.userRoleRepository.delete(userRole); + } + } else { + logger.debug("Role {} not found", roleName); + } } public Iterable getRoles(boolean includePersonalRoles) { @@ -177,14 +212,28 @@ public void clearAuthorizationInfoCache() { this.authorizationInfoCache.set(new ConcurrentHashMap<>()); } + @Transactional + public void updateUser(String login, Set defaultRoles, Set newUserRoles, + boolean resetRoles) { + registerUser(login, null, UserOrigin.SYSTEM, defaultRoles, newUserRoles, resetRoles); + } + @Transactional public UserEntity registerUser(final String login, final String name, final Set defaultRoles) { - return registerUser(login, name, UserOrigin.SYSTEM, defaultRoles); + return registerUser(login, name, UserOrigin.SYSTEM, defaultRoles, null, false); } @Transactional public UserEntity registerUser(final String login, final String name, final UserOrigin userOrigin, final Set defaultRoles) { + return registerUser(login, name, userOrigin, defaultRoles, null, false); + } + + @Transactional + private UserEntity registerUser(final String login, final String name, final UserOrigin userOrigin, + final Set defaultRoles, final Set newUserRoles, boolean resetRoles) { + logger.debug("Called registerUser with resetRoles: login={}, reset roles={}, default roles={}, new user roles={}", + login, resetRoles, defaultRoles, newUserRoles); Guard.checkNotEmpty(login); UserEntity user = userRepository.findByLogin(login); @@ -198,7 +247,17 @@ public UserEntity registerUser(final String login, final String name, final User user.setOrigin(userOrigin); user = userRepository.save(user); } - return user; + if (resetRoles) { + // remove all user roles: + removeAllUserRolesFromUser(login, user, newUserRoles); + // add back just the given newUserRoles: + addRolesForUser(login, userOrigin, user, newUserRoles, false); + // make sure the default roles are there: TODO - discuss if really necessary.... + addDefaultRolesForUser(login, userOrigin, user, defaultRoles); + } + // get user again, fresh from db with all new roles: + user = userRepository.findOne(user.getId()); + return user; // >>>>>>>>>> RETURN! Add else for readability??? } checkRoleIsAbsent(login, false, "User with such login has been improperly removed from the database. " + @@ -212,18 +271,49 @@ public UserEntity registerUser(final String login, final String name, final User RoleEntity personalRole = this.addRole(login, false); this.addUser(user, personalRole, userOrigin, null); + addRolesForUser(login, userOrigin, user, newUserRoles, false); + addDefaultRolesForUser(login, userOrigin, user, defaultRoles); + // get user again, fresh from db with all new roles: + user = userRepository.findOne(user.getId()); + return user; + } - if (defaultRoles != null) { - for (String roleName: defaultRoles) { - RoleEntity defaultRole = this.getSystemRoleByName(roleName); - if (defaultRole != null) { - this.addUser(user, defaultRole, userOrigin, null); + private void addRolesForUser(String login, UserOrigin userOrigin, UserEntity user, Set roles, boolean isSystemRole) { + if (roles != null) { + for (String roleName: roles) { + // Temporary patch/workaround (in reality the role should have been added by sysadmin?): + if (!isSystemRole) { + pocAddUserRole(roleName); + } + // end temporary patch + RoleEntity role = this.getRoleByName(roleName, isSystemRole); + if (role != null) { + this.addUser(user, role, userOrigin, null); } } } + } - user = userRepository.findOne(user.getId()); - return user; + private void addDefaultRolesForUser(String login, UserOrigin userOrigin, UserEntity user, Set roles) { + logger.debug("Adding the following system roles for the user: roles={}", roles); + addRolesForUser(login, userOrigin, user, roles,true); + } + + private void removeAllUserRolesFromUser(String login, UserEntity user, final Set rolesToSkip) { + Set userRoles = this.getUserRoles(user); + // remove all roles except the personal role and any role in the rolesToSkip: + userRoles.stream().filter(role -> !role.getName().equalsIgnoreCase(login) && !rolesToSkip.contains(role.getName())).forEach(role -> { + this.removeUserFromUserRole(role.getName(), login); + }); + } + + private RoleEntity pocAddUserRole(String roleName) { + RoleEntity role = this.roleRepository.findByNameAndSystemRole(roleName, false); + if (role != null) { + return role; + } else { + return addRole(roleName, false); + } } public Iterable getUsers() { @@ -402,6 +492,7 @@ private Set getRolePermissions(RoleEntity role) { private Set getUserRoles(UserEntity user) { Set userRoles = user.getUserRoles(); + logger.debug("Called getUserRoles. Found: {}", userRoles); Set roles = new LinkedHashSet<>(); for (UserRoleEntity userRole : userRoles) { if (isRelationAllowed(userRole.getStatus())) { @@ -438,17 +529,19 @@ public UserEntity getUserById(Long userId) { } private UserEntity getUserByLogin(final String login) { + logger.debug("Looking for user login={}...", login); final UserEntity user = this.userRepository.findByLogin(login); - if (user == null) + if (user == null) { + logger.error("User does NOT exist for login={}...", login); throw new RuntimeException("User doesn't exist"); - + } return user; } private RoleEntity getRoleByName(String roleName, Boolean isSystemRole) { final RoleEntity roleEntity = this.roleRepository.findByNameAndSystemRole(roleName, isSystemRole); if (roleEntity == null) - throw new RuntimeException("Role doesn't exist"); + throw new RuntimeException("Role doesn't exist:" + roleName); return roleEntity; } @@ -460,7 +553,7 @@ public RoleEntity getSystemRoleByName(String roleName) { private RoleEntity getRoleById(Long roleId) { final RoleEntity roleEntity = this.roleRepository.findById(roleId); if (roleEntity == null) - throw new RuntimeException("Role doesn't exist"); + throw new RuntimeException("Role doesn't exist:" + roleId); return roleEntity; } @@ -494,12 +587,15 @@ private UserRoleEntity addUser(final UserEntity user, final RoleEntity role, final UserOrigin userOrigin, final String status) { UserRoleEntity relation = this.userRoleRepository.findByUserAndRole(user, role); if (relation == null) { + logger.debug("The role={} is new for this user. Adding...", role.getName()); relation = new UserRoleEntity(); relation.setUser(user); relation.setRole(role); relation.setStatus(status); relation.setOrigin(userOrigin); relation = this.userRoleRepository.save(relation); + } else { + logger.debug("The user already had the role={}", role.getName()); } return relation; @@ -509,11 +605,20 @@ public String getSubjectName() { Subject subject = SecurityUtils.getSubject(); Object principalObject = subject.getPrincipals().getPrimaryPrincipal(); - if (principalObject instanceof String) + if (principalObject instanceof String) { + logger.debug("principal IS STRING: " + principalObject); return (String)principalObject; + } + + if (principalObject instanceof Pac4jPrincipal) { + String login = ((Pac4jPrincipal)principalObject).getProfile().getEmail(); + logger.debug("principal IS Pac4jPrincipal: " + login); + return login; + } if (principalObject instanceof Principal) { Principal principal = (Principal)principalObject; + logger.debug("principal IS Principal: " + principal.getName()); return principal.getName(); } @@ -552,4 +657,30 @@ public void removePermissionsFromTemplate(Map template, String v public boolean roleExists(String roleName) { return this.roleRepository.existsByName(roleName); } + + private String getCurrentUserSessionId() { + Subject subject = SecurityUtils.getSubject(); + return subject.getSession(false).getId().toString(); + } + + private AbstractMap.SimpleEntry getCurrentUserAndSessionTuple() { + AbstractMap.SimpleEntry userAndSessionTuple = new AbstractMap.SimpleEntry<> + (getCurrentUser().getLogin(), getCurrentUserSessionId()); + return userAndSessionTuple; + } + + public void setCurrentTeamProjectRoleForCurrentUser(String teamProjectRole, String login) { + logger.debug("Current user in setCurrentTeamProjectRoleForCurrentUser() {}", login); + this.teamProjectRoles.put(getCurrentUserAndSessionTuple(), teamProjectRole); + } + + public RoleEntity getCurrentTeamProjectRoleForCurrentUser() { + logger.debug("Current user in getCurrentTeamProjectRoleForCurrentUser(): {}", getCurrentUser().getLogin()); + String teamProjectRole = this.teamProjectRoles.get(getCurrentUserAndSessionTuple()); + if (teamProjectRole == null) { + return null; + } else { + return this.getRoleByName(teamProjectRole, false); + } + } } diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java new file mode 100644 index 0000000000..3c3776af6b --- /dev/null +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java @@ -0,0 +1,250 @@ +package org.ohdsi.webapi.shiro.filters; + +import java.io.IOException; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; + +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.web.servlet.AdviceFilter; +import org.apache.shiro.web.util.WebUtils; +import org.json.JSONArray; +import org.json.JSONObject; +import org.ohdsi.webapi.shiro.PermissionManager; +import org.ohdsi.webapi.shiro.Entities.RoleEntity; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.web.client.RestTemplate; + +/** + * Filter class to dynamically assign a "team project" role to the + * user, after validating with an external authorization service + * whether the user has been granted access to this "team project". + * + * @author Pieter Lukasse + */ +public class TeamProjectBasedAuthorizingFilter extends AdviceFilter { + + private final Logger logger = LoggerFactory.getLogger(TeamProjectBasedAuthorizingFilter.class); + + private final PermissionManager authorizer; + private final Set defaultRoles; + private final String authorizationMode; + private final String authorizationUrl; + + public TeamProjectBasedAuthorizingFilter( + PermissionManager authorizer, + Set defaultRoles, + String authorizationMode, + String authorizationUrl) { + this.authorizer = authorizer; + this.defaultRoles = defaultRoles; + this.authorizationMode = authorizationMode; + this.authorizationUrl = authorizationUrl; + logger.debug("AUTHORIZATION_MODE in TeamProjectBasedAuthorizingFilter constructor == '{}'", this.authorizationMode); + logger.debug("AUTHORIZATION_URL in TeamProjectBasedAuthorizingFilter constructor == '{}'", this.authorizationUrl); + } + + @Override + protected boolean preHandle(ServletRequest request, ServletResponse response) throws Exception { + + try { + logger.debug("preHandle in TeamProjectBasedAuthorizingFilter == '{}'", this.authorizationMode); + if (this.authorizationMode.equals("teamproject") && SecurityUtils.getSubject().isAuthenticated()) { + // in case of "teamproject" mode, we want all roles to be reset always, and + // set to only the one requested/found in the request parameters (following lines below): + String login = this.authorizer.getSubjectName(); + boolean foundValidTeamProject = extractAndValidateTeamProjectRoleAndUpdateUserIfNecessary(this, login, request, response); + if (!foundValidTeamProject) { + return false; + } + } + + } catch (Exception e) { + WebUtils.toHttp(response).setHeader("x-auth-error", e.getMessage()); + throw new Exception(e); + } + + return true; + } + + /** + * Tries to extract "team project" role found in the request. + * If necessary, assigns the newly found "team project" role to the user, + * after validating with a Gen3 authorization service if the user + * has been granted access to this team. + * + * @param self: reference to "this". Just a workaround to allow the method to be + * globally synchronized by making it static. + * @param login: the user login + * @param request: the request passing through this filter + * @param response: response object that can be used to write error status and + * error message if needed. + * + * @return: Returns true if the "team project" was found and passed validation + * and DB update to user roles did not fail. + * + * @throws IOException + * @throws Exception + */ + private static boolean extractAndValidateTeamProjectRoleAndUpdateUserIfNecessary( + TeamProjectBasedAuthorizingFilter self, + String login, ServletRequest request, + ServletResponse response) throws IOException, Exception { + + // synchronize on login to avoid race conditions (especially on DB updates) if + // this filter receives parallel requests for any reason: + synchronized (login.intern()) { + // check if a teamproject parameter is found in the request: + String teamProjectRole = self.extractTeamProjectFromRequestParameters(request); + Set newUserRoles = new HashSet(); + + // if found, add teamproject as a role in the newUserRoles list: + if (teamProjectRole != null && !teamProjectRole.trim().isEmpty()) { + // double check if this role has really been granted to the user: + if (self.checkGen3Authorization(teamProjectRole, login) == false) { + String errorMessage = "User is not authorized to access this team project's data"; + self.logger.error(errorMessage); + WebUtils.toHttp(response).sendError(HttpServletResponse.SC_FORBIDDEN, + errorMessage); + return false; + } + // add teamproject role: + newUserRoles.add(teamProjectRole); + self.authorizer.setCurrentTeamProjectRoleForCurrentUser(teamProjectRole, login); + self.authorizer.updateUser(login, self.defaultRoles, newUserRoles, true); + return true; + } else { + String errorMessage = "The teamproject is compulsory when on authorizationMode==teamproject configuration"; + self.logger.error(errorMessage); + WebUtils.toHttp(response).sendError(HttpServletResponse.SC_FORBIDDEN, + errorMessage); + return false; + } + } + } + + + private boolean checkGen3Authorization(String teamProjectRole, String login) throws Exception { + logger.debug("Checking Gen3 Authorization for 'team project'={} and user={} using service={}", teamProjectRole, login, this.authorizationUrl); + RestTemplate restTemplate = new RestTemplate(); + String arboristAuthorizationURL = this.authorizationUrl; + String requestBody = String.format("{\"username\": \"%s\"}", login); + String jsonResponseString = restTemplate.postForObject(arboristAuthorizationURL, requestBody, String.class); + + JSONObject jsonObject = new JSONObject(jsonResponseString); + + if (!jsonObject.keySet().contains(teamProjectRole)) { + logger.warn("User is not authorized to access this team project's data"); + return false; + } 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, + 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; + } + logger.debug("Parsed service is as expected"); + return true; + } + } + + /** + * Tries to find the team project information in the given request or as part of the + * stored this.authorizer.getCurrentTeamProjectRoleForCurrentUser(). Returns null + * if nothing can be found. + */ + private String extractTeamProjectFromRequestParameters(ServletRequest request) throws Exception { + // Get the url + HttpServletRequest httpRequest = (HttpServletRequest) request; + String url = httpRequest.getRequestURL().toString(); + + RoleEntity currentTeamProjectRole = this.authorizer.getCurrentTeamProjectRoleForCurrentUser(); + String currentTeamProjectName = null; + if (currentTeamProjectRole != null) { + currentTeamProjectName = this.authorizer.getCurrentTeamProjectRoleForCurrentUser().getName(); + if (currentTeamProjectName == null || currentTeamProjectName.trim().isEmpty()) { + throw new Exception("The teamproject role was found but name was unexpectedly empty"); + } + } + logger.debug("Current teamproject: {}...", currentTeamProjectName); + logger.debug("Checking if a teamproject has been specified in the request..."); + + // try to find it in the redirectUrl parameter: + logger.debug("Looking for redirectUrl in request: {}....", url); + String[] redirectUrlParams = getParameterValues(request, "redirectUrl"); + if (redirectUrlParams != null) { + logger.debug("Parameter redirectUrl found. Checking if it contains teamproject...."); + // teamProject will be in first one in this case...as only parameter: + String firstParameter = redirectUrlParams[0].toLowerCase(); + if (firstParameter.contains("teamproject=")) { + String teamProject = firstParameter.split("teamproject=")[1]; + logger.debug("Found teamproject: {}", teamProject); + return teamProject; + } + } + + // try to find "teamproject" param in url itself (there will be no redirectUrl if user session is still valid): + logger.debug("Fallback1: Looking for teamproject in request: {}....", url); + String[] teamProjectParams = getParameterValues(request, "teamproject"); + if (teamProjectParams != null) { + logger.debug("Parameter teamproject found. Parsing...."); + String teamProject = teamProjectParams[0].toLowerCase(); + logger.debug("Found teamproject: {}", teamProject); + return teamProject; + } + + logger.debug("Fallback2: Looking for teamproject in Action-Location header of request: {}....", url); + String actionLocationUrl = httpRequest.getHeader("Action-Location"); + if (actionLocationUrl != null && actionLocationUrl.contains("teamproject=")) { + String teamProject = actionLocationUrl.split("teamproject=")[1]; + logger.debug("Found teamproject: {}", teamProject); + return teamProject; + } + + logger.debug("Found NO teamproject explicitly set in the request, so keeping team project: {}.", + currentTeamProjectName); + return currentTeamProjectName; + } + + private String[] getParameterValues(ServletRequest request, String parameterName) { + // Get the parameters + logger.debug("Looking for parameter with name: {} ...", parameterName); + Enumeration paramNames = request.getParameterNames(); + while(paramNames.hasMoreElements()) { + String paramName = paramNames.nextElement(); + logger.debug("Parameter name: {}", paramName); + if (paramName.equals(parameterName)) { + String[] paramValues = request.getParameterValues(paramName); + return paramValues; + } + } + logger.debug("Found NO parameter with name: {}", parameterName); + return null; + } + +} diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java index f5597058e8..5d55530132 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java @@ -115,11 +115,6 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th login = UserUtils.toLowerCase(login); - // stop session to make logout of OAuth users possible - Session session = SecurityUtils.getSubject().getSession(false); - if (session != null) { - session.stop(); - } if (jwt == null) { if (name == null) { @@ -174,4 +169,4 @@ private Date getExpirationDate(final int expirationIntervalInSeconds) { calendar.add(Calendar.SECOND, expirationIntervalInSeconds); return calendar.getTime(); } -} +} \ No newline at end of file diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java index e19a1ebc8b..b5375b72f6 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasRegularSecurity.java @@ -256,6 +256,12 @@ public class AtlasRegularSecurity extends AtlasSecurity { @Value("${security.auth.google.enabled}") private boolean googleAuthEnabled; + @Value("${security.ohdsi.custom.authorization.mode}") + private String authorizationMode; + + @Value("${security.ohdsi.custom.authorization.url}") + private String authorizationUrl; + private RestTemplate restTemplate = new RestTemplate(); @Autowired @@ -275,6 +281,8 @@ public Map getFilters() { Map filters = super.getFilters(); filters.put(LOGOUT, new LogoutFilter(eventPublisher)); + logger.debug("Initializing UpdateAccessTokenFilter with AUTHORIZATION_MODE === '{}'", this.authorizationMode); + logger.debug("Initializing UpdateAccessTokenFilter with AUTHORIZATION_URL === '{}'", this.authorizationUrl); filters.put(UPDATE_TOKEN, new UpdateAccessTokenFilter(this.authorizer, this.defaultRoles, this.tokenExpirationIntervalInSeconds, this.redirectUrl)); @@ -413,6 +421,7 @@ protected FilterChainBuilder getFilterChainBuilder() { .setRestFilters(SSL, NO_SESSION_CREATION, CORS, NO_CACHE) .setAuthcFilter(authcFilters.toArray(new FilterTemplates[0])) .setAuthzFilter(AUTHZ) + .setTeamProjectAuthzFilter(TEAM_PROJECT_AUTHZ) // login/logout .addRestPath("/user/refresh", JWT_AUTHC, UPDATE_TOKEN, SEND_TOKEN_IN_HEADER) .addProtectedRestPath("/user/runas", RUN_AS, UPDATE_TOKEN, SEND_TOKEN_IN_HEADER) @@ -429,7 +438,7 @@ protected FilterChainBuilder getFilterChainBuilder() { if (this.openidAuthEnabled) { filterChainBuilder - .addRestPath("/user/login/openid", FORCE_SESSION_CREATION, OIDC_AUTH, UPDATE_TOKEN, SEND_TOKEN_IN_URL) + .addRestPath("/user/login/openid", FORCE_SESSION_CREATION, OIDC_AUTH, UPDATE_TOKEN, TEAM_PROJECT_AUTHZ, SEND_TOKEN_IN_URL) .addRestPath("/user/login/openidDirect", FORCE_SESSION_CREATION, OIDC_DIRECT_AUTH, UPDATE_TOKEN, SEND_TOKEN_IN_HEADER); } 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 47e086a572..7aa81f4f74 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java @@ -21,6 +21,7 @@ import org.ohdsi.webapi.shiro.filters.CorsFilter; import org.ohdsi.webapi.shiro.filters.ForceSessionCreationFilter; import org.ohdsi.webapi.shiro.filters.ResponseNoCacheFilter; +import org.ohdsi.webapi.shiro.filters.TeamProjectBasedAuthorizingFilter; import org.ohdsi.webapi.shiro.filters.UrlBasedAuthorizingFilter; import org.ohdsi.webapi.source.SourceRepository; import org.slf4j.Logger; @@ -37,6 +38,7 @@ import static org.ohdsi.webapi.shiro.management.FilterTemplates.NO_CACHE; import static org.ohdsi.webapi.shiro.management.FilterTemplates.NO_SESSION_CREATION; import static org.ohdsi.webapi.shiro.management.FilterTemplates.SSL; +import static org.ohdsi.webapi.shiro.management.FilterTemplates.TEAM_PROJECT_AUTHZ; /** * @@ -68,6 +70,12 @@ public abstract class AtlasSecurity extends Security { @Value("${security.ssl.enabled}") private boolean sslEnabled; + @Value("${security.ohdsi.custom.authorization.mode}") + private String authorizationMode; + + @Value("${security.ohdsi.custom.authorization.url}") + private String authorizationUrl; + private final EntityPermissionSchemaResolver permissionSchemaResolver; protected final Set defaultRoles = new LinkedHashSet<>(); @@ -76,13 +84,17 @@ public abstract class AtlasSecurity extends Security { private final Map filters = new HashMap<>(); public AtlasSecurity(EntityPermissionSchemaResolver permissionSchemaResolver) { - this.defaultRoles.add("public"); this.permissionSchemaResolver = permissionSchemaResolver; featureAnalysisPermissionTemplates = permissionSchemaResolver.getForType(EntityType.FE_ANALYSIS).getAllPermissions(); } @PostConstruct private void init() { + this.defaultRoles.add("public"); + 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 + } fillFilters(); } @@ -130,6 +142,7 @@ private void fillFilters() { filters.put(NO_SESSION_CREATION, new NoSessionCreationFilter()); filters.put(FORCE_SESSION_CREATION, new ForceSessionCreationFilter()); filters.put(AUTHZ, new UrlBasedAuthorizingFilter()); + filters.put(TEAM_PROJECT_AUTHZ, new TeamProjectBasedAuthorizingFilter(this.authorizer, this.defaultRoles, this.authorizationMode, this.authorizationUrl)); filters.put(CORS, new CorsFilter()); filters.put(SSL, this.getSslFilter()); filters.put(NO_CACHE, this.getNoCacheFilter()); diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/FilterChainBuilder.java b/src/main/java/org/ohdsi/webapi/shiro/management/FilterChainBuilder.java index 6bb2c96a19..30ae556f0d 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/FilterChainBuilder.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/FilterChainBuilder.java @@ -11,6 +11,7 @@ public class FilterChainBuilder { private String restFilters; private String authcFilter; private String authzFilter; + private String teamProjectAuthzFilter; private String filtersBeforeOAuth; private String filtersAfterOAuth; @@ -39,6 +40,12 @@ public FilterChainBuilder setAuthzFilter(FilterTemplates... authzFilters) { return this; } + public FilterChainBuilder setTeamProjectAuthzFilter(FilterTemplates... authzFilters) { + this.teamProjectAuthzFilter = convertArrayToString(authzFilters); + return this; + } + + public FilterChainBuilder addRestPath(String path, String filters) { return this.addPath(path, this.restFilters + ", " + filters); } @@ -56,13 +63,13 @@ public FilterChainBuilder addOAuthPath(String path, FilterTemplates... oauthFilt } public FilterChainBuilder addProtectedRestPath(String path) { - return this.addRestPath(path, this.authcFilter + ", " + this.authzFilter); + return this.addRestPath(path, this.authcFilter + ", " + this.authzFilter + ", " + this.teamProjectAuthzFilter); } public FilterChainBuilder addProtectedRestPath(String path, FilterTemplates... filters) { String filtersStr = convertArrayToString(filters); - return this.addRestPath(path, authcFilter + ", " + authzFilter + ", " + filtersStr); + return this.addRestPath(path, authcFilter + ", " + authzFilter + ", " + this.teamProjectAuthzFilter + ", " + filtersStr); } public FilterChainBuilder addPath(String path, FilterTemplates... filters) { diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/FilterTemplates.java b/src/main/java/org/ohdsi/webapi/shiro/management/FilterTemplates.java index cc3e578fde..88903ae78b 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/FilterTemplates.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/FilterTemplates.java @@ -20,6 +20,7 @@ public enum FilterTemplates { NO_SESSION_CREATION("noSessionCreation"), FORCE_SESSION_CREATION("forceSessionCreation"), AUTHZ("authz"), + TEAM_PROJECT_AUTHZ("teamProjectAuthz"), CORS("cors"), SSL("ssl"), NO_CACHE("noCache"), diff --git a/src/main/java/org/ohdsi/webapi/shiro/management/datasource/BaseDataSourceAccessor.java b/src/main/java/org/ohdsi/webapi/shiro/management/datasource/BaseDataSourceAccessor.java index 8cf2b80bf1..bdea3958b7 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/datasource/BaseDataSourceAccessor.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/datasource/BaseDataSourceAccessor.java @@ -4,12 +4,16 @@ import org.ohdsi.webapi.shiro.management.DisabledSecurity; import org.ohdsi.webapi.shiro.management.Security; import org.ohdsi.webapi.source.Source; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import javax.ws.rs.ForbiddenException; public abstract class BaseDataSourceAccessor implements DataSourceAccessor { + private final Logger logger = LoggerFactory.getLogger(BaseDataSourceAccessor.class); + @Autowired(required = false) private DisabledSecurity disabledSecurity; @@ -26,9 +30,13 @@ public boolean hasAccess(T s) { Source source = extractSource(s); if (source == null) { + logger.debug("Found extractSource() to return null!"); return false; } - return SecurityUtils.getSubject().isPermitted(String.format(Security.SOURCE_ACCESS_PERMISSION, source.getSourceKey())); + + boolean isPermitted = SecurityUtils.getSubject().isPermitted(String.format(Security.SOURCE_ACCESS_PERMISSION, source.getSourceKey())); + logger.debug("Returning isPermitted={} for {}", isPermitted, String.format(Security.SOURCE_ACCESS_PERMISSION, source.getSourceKey())); + return isPermitted; } protected abstract Source extractSource(T source); diff --git a/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java b/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java index e7245b1b49..7729436818 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java +++ b/src/main/java/org/ohdsi/webapi/shiro/realms/JwtAuthRealm.java @@ -15,6 +15,8 @@ import org.ohdsi.webapi.shiro.PermissionManager; import org.ohdsi.webapi.shiro.tokens.JwtAuthToken; +import io.buji.pac4j.subject.Pac4jPrincipal; + /** * * @author gennadiy.anisimov @@ -35,7 +37,14 @@ public boolean supports(AuthenticationToken token) { @Override protected AuthorizationInfo doGetAuthorizationInfo(PrincipalCollection principals) { - final String login = (String) principals.getPrimaryPrincipal(); + String login; + Object principal = principals.getPrimaryPrincipal(); + if (principal instanceof Pac4jPrincipal) { + login = ((Pac4jPrincipal)principal).getProfile().getEmail(); + } + else { + login = (String) principals.getPrimaryPrincipal(); + } return authorizer.getAuthorizationInfo(login); } diff --git a/src/main/java/org/ohdsi/webapi/source/SourceService.java b/src/main/java/org/ohdsi/webapi/source/SourceService.java index 65551781a2..0e5e4ae4e0 100644 --- a/src/main/java/org/ohdsi/webapi/source/SourceService.java +++ b/src/main/java/org/ohdsi/webapi/source/SourceService.java @@ -7,6 +7,8 @@ import org.ohdsi.webapi.common.SourceMapKey; import org.ohdsi.webapi.service.AbstractDaoService; import org.ohdsi.webapi.shiro.management.datasource.SourceAccessor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.jdbc.CannotGetJdbcConnectionException; import org.springframework.jdbc.core.JdbcTemplate; @@ -21,6 +23,8 @@ @Service public class SourceService extends AbstractDaoService { + private final Logger logger = LoggerFactory.getLogger(SourceService.class); + private static Collection cachedSources = null; @Value("${jasypt.encryptor.enabled}") @@ -113,6 +117,7 @@ public void checkConnection(Source source) { public Source getPrioritySourceForDaimon(SourceDaimon.DaimonType daimonType) { List sourcesByDaimonPriority = sourceRepository.findAllSortedByDiamonPrioirty(daimonType); + logger.debug("Found {} sources for daimon type {}.", sourcesByDaimonPriority.size(), daimonType.name()); for (Source source : sourcesByDaimonPriority) { if (!(sourceAccessor.hasAccess(source) && connectionAvailability.computeIfAbsent(source, this::checkConnectionSafe))) { @@ -120,6 +125,7 @@ public Source getPrioritySourceForDaimon(SourceDaimon.DaimonType daimonType) { } return source; } + logger.debug("Found NO sources that have access."); return null; } @@ -129,7 +135,8 @@ public Map getPriorityDaimons() { class SourceValidator { private Map checkedSources = new HashMap<>(); - private boolean isSourceAvaialble(Source source) { + private boolean isSourceAvailable(Source source) { + logger.debug("Checking source access via isSourceAvailable..."); return checkedSources.computeIfAbsent(source.getSourceId(), v -> sourceAccessor.hasAccess(source) && connectionAvailability.computeIfAbsent(source, SourceService.this::checkConnectionSafe)); } @@ -140,7 +147,8 @@ private boolean isSourceAvaialble(Source source) { Arrays.asList(SourceDaimon.DaimonType.values()).forEach(d -> { List sources = sourceRepository.findAllSortedByDiamonPrioirty(d); - Optional source = sources.stream().filter(sourceValidator::isSourceAvaialble) + logger.debug("Found {} sources.", sources.size()); + Optional source = sources.stream().filter(sourceValidator::isSourceAvailable) .findFirst(); source.ifPresent(s -> priorityDaimons.put(d, s)); }); @@ -169,8 +177,11 @@ private boolean checkConnectionSafe(Source source) { try { checkConnection(source); + logger.debug("Connection worked."); return true; } catch (CannotGetJdbcConnectionException ex) { + logger.debug("Connection failed: {}", ex.getMessage()); + ex.printStackTrace(); return false; } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index cd1afb2013..2b7a811ee5 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -201,6 +201,10 @@ security.auth.ldap.enabled=${security.auth.ldap.enabled} security.auth.ad.enabled=${security.auth.ad.enabled} security.auth.cas.enabled=${security.auth.cas.enabled} +#Authorization config +security.ohdsi.custom.authorization.mode=${security.ohdsi.custom.authorization.mode} +security.ohdsi.custom.authorization.url=${security.ohdsi.custom.authorization.url} + #Execution engine executionengine.updateStatusCallback=${executionengine.updateStatusCallback} executionengine.resultCallback=${executionengine.resultCallback} From 02426d8bf8cf6f848b16b9c6c426b9171feedcee Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 25 Apr 2024 11:56:48 -0500 Subject: [PATCH 03/36] feat: updating dependencies (#128) downgrade pac4j --- pom.xml | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index ad77074a10..5e2ce52462 100644 --- a/pom.xml +++ b/pom.xml @@ -17,10 +17,10 @@ 4.2.0 2.2.1 5.5.0 - 5.4.2.Final - 42.3.7 + 5.4.24.Final + 42.3.9 1.69 - 1.12.0 + 1.13.0 2.1.3 0.4.0 3.2.0 @@ -575,13 +575,88 @@ redshift-jdbc42-no-awssdk 1.2.10.1009 + + xalan + xalan + 2.7.3 + + + com.fasterxml.woodstox + woodstox-core + 5.4.0 + + + org.apache.activemq + activemq-client + 5.15.16 + + + org.apache.activemq + activemq-openwire-legacy + 5.15.16 + + + org.apache.activemq + activemq-broker + 5.15.16 + + + commons-beanutils + commons-beanutils + 1.9.4 + + + net.minidev + json-smart + 2.4.9 + + + org.apache.commons + commons-compress + 1.26.1 + + + commons-codec + commons-codec + 1.13 + + + org.codehaus.jettison + jettison + 1.5.4 + + + com.beust + jcommander + 1.75 + + + io.springfox + springfox-swagger-ui + 2.10.5 + + + org.apache.tika + tika-core + 1.22 + + + org.apache.santuario + xmlsec + 2.1.7 + + + net.lingala.zip4j + zip4j + 2.11.3 + com.fasterxml.jackson.core jackson-databind - ${jackson.version} + 2.12.7.1 com.fasterxml.jackson.core @@ -664,7 +739,7 @@ com.thoughtworks.xstream xstream - 1.4.19 + 1.4.20 org.springframework.boot @@ -835,7 +910,7 @@ com.microsoft.azure msal4j - 1.9.0 + 1.10.0 com.opencsv @@ -987,7 +1062,7 @@ org.json json - 20230227 + 20231013 org.ohdsi From 8fa7db5226b0c1fe81c23fb4a6d36ddf5ffa9199 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 15 May 2024 22:09:01 +0200 Subject: [PATCH 04/36] feat: new flyway script to add the new global share permission to sec_permission Update src/main/resources/db/migration/postgresql/V2.15.0.20240515220400_atlas_global_share_permission.sql --- ...V2.15.0.20240515220400__atlas_global_share_permission.sql | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20240515220400__atlas_global_share_permission.sql diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20240515220400__atlas_global_share_permission.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20240515220400__atlas_global_share_permission.sql new file mode 100644 index 0000000000..5650b504bb --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20240515220400__atlas_global_share_permission.sql @@ -0,0 +1,5 @@ +INSERT INTO ${ohdsiSchema}.sec_permission +(value, description) +values +('artifact:global:share:put', 'special permission (intended for admins) that allows the user to share any artifact with a "global reader role"') +; From d77ce3269bf9130b3e6c7c2257f483bb0c551ca4 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 23 May 2024 15:33:02 +0200 Subject: [PATCH 05/36] feat: adjust checkCommonEntityOwnership to allow for teamproject ownership --- .../ohdsi/webapi/security/PermissionService.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionService.java b/src/main/java/org/ohdsi/webapi/security/PermissionService.java index a10e6004ce..d1a9df54e6 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionService.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionService.java @@ -124,8 +124,18 @@ public void checkCommonEntityOwnership(EntityType entityType, Integer entityId) .getReturnType(); CommonEntity entity = (CommonEntity) entityRepository.getOne((Serializable) conversionService.convert(entityId, idClazz)); - if (!isCurrentUserOwnerOf(entity)) { - throw new UnauthorizedException(); + if (this.authorizationMode.equals("teamproject")) { + // in teamproject mode, it is sufficient if the user has write permission to this entity, + // as entity ownership and maintenance is a shared responsibility within a team: + boolean hasAccess = hasWriteAccess(entity); + if (!hasAccess) { + throw new UnauthorizedException(); + } + } else { + // default validation: current **user** should be owner: + if (!isCurrentUserOwnerOf(entity)) { + throw new UnauthorizedException(); + } } } From 012dd606bb4f2d6d7a7d2db5f1bba96adef08518 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 18 Jan 2024 17:20:33 +0100 Subject: [PATCH 06/36] feat: amazoncorreto alternative to deprecated openJDK 8 --- Dockerfile | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index f14d0a56c4..c4c6daa8ca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -29,10 +29,8 @@ RUN mvn package ${MAVEN_PARAMS} \ && jar -xf WebAPI.war \ && rm WebAPI.war -# OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 11 -FROM openjdk:8-jre-slim - -MAINTAINER Lee Evans - www.ltscomputingllc.com +# OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 +FROM amazoncorretto:8u402-al2023 # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" @@ -59,9 +57,19 @@ COPY --from=builder /code/war/META-INF META-INF EXPOSE 8080 -USER 101 +RUN echo $JAVA_HOME && mkdir /usr/local/share/aws-certs \ + && curl https://truststore.pki.rds.amazonaws.com/us-east-1/us-east-1-bundle.pem -o /usr/local/share/aws-certs/us-east-1-bundle.pem \ + && cd $JAVA_HOME/jre/lib/security \ + && keytool -import -trustcacerts -storepass changeit -noprompt -alias aws -file /usr/local/share/aws-certs/us-east-1-bundle.pem + +# Copy your custom CA certificates to the container +RUN cp /usr/local/share/aws-certs/us-east-1-bundle.pem /etc/pki/ca-trust/source/anchors/ + +# Update the CA certificate store +RUN update-ca-trust + # Directly run the code as a WAR. CMD exec java ${DEFAULT_JAVA_OPTS} ${JAVA_OPTS} \ -cp ".:WebAPI.jar:WEB-INF/lib/*.jar${CLASSPATH}" \ - org.springframework.boot.loader.WarLauncher + org.springframework.boot.loader.WarLauncher \ No newline at end of file From c904887989ec1e11f5fd0f0693e86a72d0411f3e Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 23 May 2024 17:21:12 +0200 Subject: [PATCH 07/36] tmp: temporary solution to simplify onboarding i.e. also add the "Source user (omop)" role to the list of defaultRoles for each user. TODO - replace with final solution from https://ctds-planx.atlassian.net/browse/VADC-1086 --- .../java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java | 2 ++ 1 file changed, 2 insertions(+) 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 7aa81f4f74..e7145d5a7f 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/management/AtlasSecurity.java @@ -94,6 +94,8 @@ 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(); } From 06067d50296044f7373aee6ca1bf3e3430436c2b Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 25 Apr 2024 11:56:27 -0500 Subject: [PATCH 08/36] [Snyk] Fix for 14 vulnerabilities (#129) * fix: pom.xml to reduce vulnerabilities The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JAVA-IONETTY-2812456 - https://snyk.io/vuln/SNYK-JAVA-IONETTY-5725787 - https://snyk.io/vuln/SNYK-JAVA-ORGAPACHESOLR-6241853 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTY-2945452 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTY-2945453 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTY-5426161 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTY-5902998 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTY-5958847 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTYHTTP2-5958845 - https://snyk.io/vuln/SNYK-JAVA-ORGECLIPSEJETTYHTTP2-5958918 - https://snyk.io/vuln/SNYK-JAVA-ORGSPRINGFRAMEWORKSECURITY-570203 - https://snyk.io/vuln/SNYK-JAVA-ORGXERIALSNAPPY-5710959 - https://snyk.io/vuln/SNYK-JAVA-ORGXERIALSNAPPY-5710960 - https://snyk.io/vuln/SNYK-JAVA-ORGXERIALSNAPPY-5710961 * some reverts --------- Co-authored-by: snyk-bot --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 5e2ce52462..55d428b15e 100644 --- a/pom.xml +++ b/pom.xml @@ -1082,7 +1082,7 @@ org.springframework.security spring-security-crypto - 4.2.3.RELEASE + 4.2.16.RELEASE com.zaxxer @@ -1929,7 +1929,7 @@ {!complexphrase inOrder=true} - 8.11.2 + 8.11.3 From 67811ef4c5b1c57c053f4cff0d22f03df8288f74 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 6 Jun 2024 21:12:34 +0200 Subject: [PATCH 09/36] fix: add extra check to ensure user has global sharing permission --- .../webapi/security/PermissionController.java | 15 +++++++++++++-- .../ohdsi/webapi/shiro/Entities/RoleEntity.java | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionController.java b/src/main/java/org/ohdsi/webapi/security/PermissionController.java index 6471b75bbf..625f5a3f6b 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionController.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionController.java @@ -20,13 +20,15 @@ import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.StreamSupport; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.authz.UnauthorizedException; +import org.apache.shiro.authz.permission.WildcardPermission; +import org.apache.shiro.subject.Subject; /** * REST Services related to working with security permissions @@ -159,7 +161,16 @@ public void grantEntityPermissionsForRole( AccessRequestDTO accessRequestDTO ) throws Exception { + // check if user, or his "teamProject" own the entity being given access: permissionService.checkCommonEntityOwnership(entityType, entityId); + // furthermore, check if the entity is being shared with "public" role (roleId==1). If yes, then + // check if user has the necessary global/public sharing permission ("artifact:global:share:put") to do so: + if (roleId == RoleEntity.PUBLIC_ROLE_ID) { + Subject subject = SecurityUtils.getSubject(); + if (!subject.isPermitted(new WildcardPermission("artifact:global:share:put"))) { + throw new UnauthorizedException(); + } + } Map permissionTemplates = permissionService.getTemplatesForType(entityType, accessRequestDTO.getAccessType()); diff --git a/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java index f958d4fd9a..06d05b808d 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java @@ -23,6 +23,7 @@ public class RoleEntity implements Serializable{ private static final long serialVersionUID = 6257846375334314942L; + public static final int PUBLIC_ROLE_ID = 1; @Id @Column(name = "ID") From a3d29ce14ea996e0d1e1b209be624f3b005d7e3f Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 7 Jun 2024 18:50:02 +0200 Subject: [PATCH 10/36] fix: stricter check in checkCommonEntityOwnership for teamProject ownership check --- .../webapi/security/PermissionController.java | 7 +++++- .../webapi/security/PermissionService.java | 24 ++++++++++++++++--- .../webapi/shiro/Entities/RoleEntity.java | 13 ++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionController.java b/src/main/java/org/ohdsi/webapi/security/PermissionController.java index 625f5a3f6b..e3fc428fb8 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionController.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionController.java @@ -6,6 +6,8 @@ import org.ohdsi.webapi.service.UserService; import org.ohdsi.webapi.shiro.Entities.PermissionEntity; import org.ohdsi.webapi.shiro.Entities.RoleEntity; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.ohdsi.webapi.shiro.PermissionManager; import org.springframework.core.convert.ConversionService; import org.springframework.stereotype.Controller; @@ -39,6 +41,7 @@ @Path(value = "/permission") @Transactional public class PermissionController { + private final Logger logger = LoggerFactory.getLogger(PermissionController.class); private final PermissionService permissionService; private final PermissionManager permissionManager; @@ -163,11 +166,13 @@ public void grantEntityPermissionsForRole( // check if user, or his "teamProject" own the entity being given access: permissionService.checkCommonEntityOwnership(entityType, entityId); - // furthermore, check if the entity is being shared with "public" role (roleId==1). If yes, then + // furthermore, check if the entity is being shared to the "public" role (roleId==1). If yes, then // check if user has the necessary global/public sharing permission ("artifact:global:share:put") to do so: if (roleId == RoleEntity.PUBLIC_ROLE_ID) { Subject subject = SecurityUtils.getSubject(); if (!subject.isPermitted(new WildcardPermission("artifact:global:share:put"))) { + logger.error("Permission denied: user {} has no permission for sharing entities globally (making them visible to all with a 'public' role)", + this.permissionManager.getSubjectName()); throw new UnauthorizedException(); } } diff --git a/src/main/java/org/ohdsi/webapi/security/PermissionService.java b/src/main/java/org/ohdsi/webapi/security/PermissionService.java index d1a9df54e6..7c00d5e99e 100644 --- a/src/main/java/org/ohdsi/webapi/security/PermissionService.java +++ b/src/main/java/org/ohdsi/webapi/security/PermissionService.java @@ -125,10 +125,13 @@ public void checkCommonEntityOwnership(EntityType entityType, Integer entityId) CommonEntity entity = (CommonEntity) entityRepository.getOne((Serializable) conversionService.convert(entityId, idClazz)); if (this.authorizationMode.equals("teamproject")) { - // in teamproject mode, it is sufficient if the user has write permission to this entity, + // in teamproject mode, it is sufficient if the team has ALL write permission to this entity, // as entity ownership and maintenance is a shared responsibility within a team: - boolean hasAccess = hasWriteAccess(entity); - if (!hasAccess) { + RoleEntity teamProjectRole = this.permissionManager.getCurrentTeamProjectRoleForCurrentUser(); + boolean teamHasWriteAccess = roleHasAccess(teamProjectRole, entity, AccessType.WRITE); + if (!teamHasWriteAccess) { + logger.error("Permission denied: teamProject {} does not have write access to the entity {}", + teamProjectRole.getName(), entity.getId()); throw new UnauthorizedException(); } } else { @@ -217,6 +220,21 @@ public boolean hasAccess(CommonEntity entity, AccessType accessType) { } return hasAccess; } + + public boolean roleHasAccess(RoleEntity role, CommonEntity entity, AccessType accessType) { + boolean hasAccess = false; + if (securityEnabled) { + try { + EntityType entityType = entityPermissionSchemaResolver.getEntityType(entity.getClass()); + List permsToCheck = getEntityPermissions(entityType, entity.getId(), accessType); + hasAccess = permsToCheck.stream().allMatch(p -> role.isPermitted(p)); + } catch (Exception e) { + logger.error("Error getting and verifying role's permissions", e); + throw new RuntimeException(e); + } + } + return hasAccess; + } public boolean hasWriteAccess(CommonEntity entity) { return hasAccess(entity, AccessType.WRITE); diff --git a/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java index 06d05b808d..713cd77cb1 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java +++ b/src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java @@ -11,6 +11,8 @@ import javax.persistence.Id; import javax.persistence.OneToMany; import javax.persistence.Table; +import org.apache.shiro.authz.permission.WildcardPermission; +import org.apache.shiro.authz.Permission; import org.hibernate.annotations.GenericGenerator; import org.hibernate.annotations.Parameter; @@ -90,4 +92,15 @@ public Boolean isSystemRole() { public void setSystemRole(Boolean system) { systemRole = system; } + + public boolean isPermitted(Permission p) { + for (RolePermissionEntity rolePermissionEntity : this.getRolePermissions()) { + PermissionEntity permissionEntity = rolePermissionEntity.getPermission(); + WildcardPermission rolePermission = new WildcardPermission(permissionEntity.getValue()); + if (rolePermission.implies(p)) { + return true; + } + } + return false; + } } From 913be862827fc6965bd530506b8f5b5a9e1a2ef7 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 11 Jun 2024 21:45:44 +0200 Subject: [PATCH 11/36] feat: enable CI for ctds local fork`s default branch --- .github/workflows/ci_custom.yaml | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/ci_custom.yaml diff --git a/.github/workflows/ci_custom.yaml b/.github/workflows/ci_custom.yaml new file mode 100644 index 0000000000..09664bfd6e --- /dev/null +++ b/.github/workflows/ci_custom.yaml @@ -0,0 +1,38 @@ +# Continuous integration +name: CI CTDS + +# Run customized version of CI in our local fork's main (default) branch and pull requests to +# this branch (note: this ci_custom.yaml file is a simplified version of the ci.yaml file in this same directory) +on: + push: + branches: [ 2.15.0-DEV ] + pull_request: + branches: [ 2.15.0-DEV ] + +jobs: + # Build and test the code + build: + runs-on: ubuntu-latest + + env: + MAVEN_PROFILE: webapi-postgresql + + steps: + # Checks-out repository under $GITHUB_WORKSPACE, so the job can access it + - uses: actions/checkout@v2 + + - uses: actions/setup-java@v1 + with: + java-version: 8 + + - name: Maven cache + uses: actions/cache@v2 + with: + path: ~/.m2 + # Key for restoring and saving the cache + key: ${{ runner.os }}-maven-${{ hashFiles('pom.xml') }} + restore-keys: | + ${{ runner.os }}-maven- + + - name: Test + run: mvn -B -P${{ env.MAVEN_PROFILE }} test From 1498717c76c5c20e86e946890967c89ffa2134c4 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 12 Jun 2024 11:55:56 +0200 Subject: [PATCH 12/36] feat: upgrade commons-io to 2.14.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 55d428b15e..954366c267 100644 --- a/pom.xml +++ b/pom.xml @@ -831,7 +831,7 @@ commons-io commons-io - 2.7 + 2.14.0 com.sun.xml.security From a43248aab12805610d2353ea2303e4c40a21f3f3 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 12 Jun 2024 15:20:38 +0200 Subject: [PATCH 13/36] fix: add transactional annotation to failing test --- .../org/ohdsi/webapi/generationcache/GenerationCacheTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/ohdsi/webapi/generationcache/GenerationCacheTest.java b/src/test/java/org/ohdsi/webapi/generationcache/GenerationCacheTest.java index a7ed924ac9..6330380bb6 100644 --- a/src/test/java/org/ohdsi/webapi/generationcache/GenerationCacheTest.java +++ b/src/test/java/org/ohdsi/webapi/generationcache/GenerationCacheTest.java @@ -4,6 +4,7 @@ import com.odysseusinc.arachne.execution_engine_common.api.v1.dto.KerberosAuthMechanism; import net.lingala.zip4j.ZipFile; import net.lingala.zip4j.exception.ZipException; +import org.springframework.transaction.annotation.Transactional; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -170,6 +171,7 @@ public void generateCohort() { } @Test + @Transactional public void checkCachingWithEmptyResultSet() { CacheableGenerationType type = CacheableGenerationType.COHORT; From 6261dffa66b6be10e6516c71a017d1500fe5cc0e Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 19 Jun 2024 16:32:09 +0200 Subject: [PATCH 14/36] Feat: restrict the read restricted role (#142) * 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 8c9caf95543cd9eb5046891ab444026cd0890e99. * 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 --- .../CohortDefinitionPermissionSchema.java | 14 +- .../shiro/Entities/PermissionEntity.java | 2 +- .../TeamProjectBasedAuthorizingFilter.java | 36 ++- .../shiro/management/AtlasSecurity.java | 2 - ...s_more_restricted_read_restricted_role.sql | 206 ++++++++++++++++++ 5 files changed, 231 insertions(+), 29 deletions(-) create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql 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/shiro/Entities/PermissionEntity.java b/src/main/java/org/ohdsi/webapi/shiro/Entities/PermissionEntity.java index f54160b6bf..61867f7e12 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 3c3776af6b..91d23b11d9 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 e7145d5a7f..7aa81f4f74 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 0000000000..a15f4b911f --- /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; From ae704c9f2e19b68ca3dd01326a9f422225bde3c2 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 21 Jun 2024 21:19:29 +0200 Subject: [PATCH 15/36] fix: add conceptset:*:copy-name:get permission to role 15 (#144) --- ...0500__custom_ctds_more_restricted_read_restricted_role.sql} | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename src/main/resources/db/migration/postgresql/{V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql => V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql} (96%) 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.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql similarity index 96% rename from src/main/resources/db/migration/postgresql/V2.15.0.20240617204600__custom_ctds_more_restricted_read_restricted_role.sql rename to src/main/resources/db/migration/postgresql/V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql index a15f4b911f..89262be3e9 100644 --- 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.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql @@ -151,7 +151,8 @@ SELECT DISTINCT 15 role_id, permission_id '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. + 'conceptset:*:version:get', -- TODO - taken over from role 10...This one is probably too broad and will need further fixes. + 'conceptset:*:copy-name:get' -- TODO - taken over from role 10...This one is probably too broad and will need further fixes. ) ; From 5213ef60bb167f40a1dcefa2e1b7caf07f8b80e5 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 25 Jun 2024 21:14:08 +0200 Subject: [PATCH 16/36] fix: remove .toLowerCase() in teamproject parsing (#146) --- .../shiro/filters/TeamProjectBasedAuthorizingFilter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 91d23b11d9..3b4503cae9 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java @@ -198,7 +198,7 @@ private String extractTeamProjectFromRequestParameters(ServletRequest request) t if (redirectUrlParams != null) { logger.debug("Parameter redirectUrl found. Checking if it contains teamproject...."); // teamProject will be in first one in this case...as only parameter: - String firstParameter = redirectUrlParams[0].toLowerCase(); + String firstParameter = redirectUrlParams[0]; if (firstParameter.contains("teamproject=")) { String teamProject = firstParameter.split("teamproject=")[1]; logger.debug("Found teamproject: {}", teamProject); @@ -211,7 +211,7 @@ private String extractTeamProjectFromRequestParameters(ServletRequest request) t String[] teamProjectParams = getParameterValues(request, "teamproject"); if (teamProjectParams != null) { logger.debug("Parameter teamproject found. Parsing...."); - String teamProject = teamProjectParams[0].toLowerCase(); + String teamProject = teamProjectParams[0]; logger.debug("Found teamproject: {}", teamProject); return teamProject; } From 21f68db7c7ad28c044cea851bd121312d30fd541 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 27 Jun 2024 14:18:07 -0500 Subject: [PATCH 17/36] Update AmazonCorretto to 8u412-al2023 (#147) * dep: update runtime image to AmazonCorretto 8u412-al2023 * feat: adding newline * feat: JSONArgsRecommended CMD statement * feat: back to shell form --- Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index c4c6daa8ca..e95a90c753 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM quay.io/cdis/maven:3.6-jdk-11 as builder +FROM quay.io/cdis/maven:3.6-jdk-11 AS builder WORKDIR /code @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM amazoncorretto:8u402-al2023 +FROM amazoncorretto:8u412-al2023 # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" @@ -72,4 +72,4 @@ RUN update-ca-trust # Directly run the code as a WAR. CMD exec java ${DEFAULT_JAVA_OPTS} ${JAVA_OPTS} \ -cp ".:WebAPI.jar:WEB-INF/lib/*.jar${CLASSPATH}" \ - org.springframework.boot.loader.WarLauncher \ No newline at end of file + org.springframework.boot.loader.WarLauncher From 6fc2d7206e6db50222e12a8a58ddd847410c0375 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Wed, 31 Jul 2024 14:59:43 -0500 Subject: [PATCH 18/36] dep: update jackson --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 954366c267..839ac73c7e 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ 1.16.1 3.1.2 4.0.0 - 2.12.7 + 2.17.0 org.ohdsi.webapi.WebApi false false From a0a4d774d3c2ecf6d8b1d18fd7964b20be3dbedc Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Wed, 31 Jul 2024 15:00:37 -0500 Subject: [PATCH 19/36] dep: update postgresql --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 839ac73c7e..9da4fab16b 100644 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ 2.2.1 5.5.0 5.4.24.Final - 42.3.9 + 42.7.3 1.69 1.13.0 2.1.3 From b2351053609ef15dfb37e4fd376fcc14bc6f0826 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Wed, 31 Jul 2024 15:36:33 -0500 Subject: [PATCH 20/36] Update base images and dependencies (#152) * dep: update base Docker images to Amazon Corretto images * dep: update to jackson * dep: update tika-core * dep: update msal4j --- Dockerfile | 4 ++-- pom.xml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index e95a90c753..961fa265cd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM quay.io/cdis/maven:3.6-jdk-11 AS builder +FROM maven:3.9.8-amazoncorretto-11-al2023 AS builder WORKDIR /code @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM amazoncorretto:8u412-al2023 +FROM amazoncorretto:8u422-al2023-jre # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" diff --git a/pom.xml b/pom.xml index 9da4fab16b..d8419ef06c 100644 --- a/pom.xml +++ b/pom.xml @@ -638,7 +638,7 @@ org.apache.tika tika-core - 1.22 + 1.28.4 org.apache.santuario @@ -910,7 +910,7 @@ com.microsoft.azure msal4j - 1.10.0 + 1.15.1 com.opencsv From 58f2f59792f339819cbf9e9de4ca8814697c67e3 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 1 Aug 2024 09:43:37 +0200 Subject: [PATCH 21/36] Fix/session vs token issue (#151) * fix: revert back original session.stop() code from upstream * fix: do not use session for teamproject role management --- .../ohdsi/webapi/shiro/PermissionManager.java | 17 +++-------------- .../shiro/filters/UpdateAccessTokenFilter.java | 7 ++++++- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index 239ba58a1f..ab8a5909f3 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -83,7 +83,7 @@ public class PermissionManager { private ThreadLocal> authorizationInfoCache = ThreadLocal.withInitial(ConcurrentHashMap::new); - private Map, String> teamProjectRoles = new HashMap<>(); + private Map teamProjectRoles = new HashMap<>(); public static class PermissionsDTO { @@ -658,25 +658,14 @@ public boolean roleExists(String roleName) { return this.roleRepository.existsByName(roleName); } - private String getCurrentUserSessionId() { - Subject subject = SecurityUtils.getSubject(); - return subject.getSession(false).getId().toString(); - } - - private AbstractMap.SimpleEntry getCurrentUserAndSessionTuple() { - AbstractMap.SimpleEntry userAndSessionTuple = new AbstractMap.SimpleEntry<> - (getCurrentUser().getLogin(), getCurrentUserSessionId()); - return userAndSessionTuple; - } - public void setCurrentTeamProjectRoleForCurrentUser(String teamProjectRole, String login) { logger.debug("Current user in setCurrentTeamProjectRoleForCurrentUser() {}", login); - this.teamProjectRoles.put(getCurrentUserAndSessionTuple(), teamProjectRole); + this.teamProjectRoles.put(getCurrentUser().getLogin(), teamProjectRole); } public RoleEntity getCurrentTeamProjectRoleForCurrentUser() { logger.debug("Current user in getCurrentTeamProjectRoleForCurrentUser(): {}", getCurrentUser().getLogin()); - String teamProjectRole = this.teamProjectRoles.get(getCurrentUserAndSessionTuple()); + String teamProjectRole = this.teamProjectRoles.get(getCurrentUser().getLogin()); if (teamProjectRole == null) { return null; } else { diff --git a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java index 5d55530132..f5597058e8 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/UpdateAccessTokenFilter.java @@ -115,6 +115,11 @@ protected boolean preHandle(ServletRequest request, ServletResponse response) th login = UserUtils.toLowerCase(login); + // stop session to make logout of OAuth users possible + Session session = SecurityUtils.getSubject().getSession(false); + if (session != null) { + session.stop(); + } if (jwt == null) { if (name == null) { @@ -169,4 +174,4 @@ private Date getExpirationDate(final int expirationIntervalInSeconds) { calendar.add(Calendar.SECOND, expirationIntervalInSeconds); return calendar.getTime(); } -} \ No newline at end of file +} From 866167834475e457d77bd4afd364430d20996940 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Mon, 12 Aug 2024 10:04:06 -0500 Subject: [PATCH 22/36] dep/fix (#154) * dep: fix msal4j * dep: add oauth2-oidc-sdk * dep: go back to 1.10.0 for msal4j * dep: msal4j to 1.15.1 * dep: update pac4j to 4.5.7 (last minor) * dep: pac4j to 4.4.0 * dep: pac4j to 4.0.0 * dep: fix, going back, need to find better option --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d8419ef06c..ec143a1ac2 100644 --- a/pom.xml +++ b/pom.xml @@ -910,7 +910,7 @@ com.microsoft.azure msal4j - 1.15.1 + 1.10.0 com.opencsv From 506e34742d6ba9b53a45fb8cffeb4e612cb4d357 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Mon, 12 Aug 2024 19:48:10 +0200 Subject: [PATCH 23/36] Fix: restrict concept permissions in role 15 (#153) * fix: added missing migration line to prev migration script Adding this for completeness... * fix: move too broad conceptset:* permissions to narrow ones ...linked to specific individual conceptsets * fix: added missing readPermissions for conceptsets These changes remove the need for having these permissions granted as * permission. Instead, users now get a conceptset specific permission. --- .../model/ConceptSetPermissionSchema.java | 3 + ...s_more_restricted_read_restricted_role.sql | 5 + ..._restricted_read_restricted_role_part2.sql | 100 ++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20240801170500__custom_ctds_more_restricted_read_restricted_role_part2.sql 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() { diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql index 89262be3e9..240ebbf007 100644 --- a/src/main/resources/db/migration/postgresql/V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20240621210500__custom_ctds_more_restricted_read_restricted_role.sql @@ -205,3 +205,8 @@ from 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; + + +-- CTDS/"team project" feature specific - keep only "admin" role assignment... i.e. remove all other +-- role assignments for all users: +DELETE from ${ohdsiSchema}.sec_user_role where role_id != 2; -- role 2 is the standard "admin" role diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20240801170500__custom_ctds_more_restricted_read_restricted_role_part2.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20240801170500__custom_ctds_more_restricted_read_restricted_role_part2.sql new file mode 100644 index 0000000000..b562fd57d8 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20240801170500__custom_ctds_more_restricted_read_restricted_role_part2.sql @@ -0,0 +1,100 @@ +-- Delete set of too broad conceptset permissions from role 15: +DELETE from ${ohdsiSchema}.sec_role_permission srp +where srp.role_id = 15 AND srp.permission_id in +( + Select sp.id from ${ohdsiSchema}.sec_permission sp + where sp.value IN + ( + 'conceptset:*:expression:*:get', -- taken over from role 10...This one was too broad + 'conceptset:*:version:get', -- taken over from role 10...This one was too broad + 'conceptset:*:copy-name:get' -- taken over from role 10...This one was too broad + ) +) +; + + +-- CONCEPT_SET_SEC_ROLE is our custom view that returns a list of conceptset ids per role +-- as long as that role has a permission starting with "conceptset:"" for that id. E.g. : +-- concept_set_id | sec_role_name +-- ----------------------+------------------------- +-- 1 | /gwas_projects/project2 +-- 2 | /gwas_projects/project2 +-- 30 | /gwas_projects/project1 + +DROP VIEW IF EXISTS ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE; +CREATE VIEW ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE AS + select + distinct cast(regexp_replace(sec_permission.value, + '^conceptset:([0-9]+):.*','\1') as integer) as concept_set_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 ~ 'conceptset:[0-9]+' +; + +-- Below we create new "expression:*:get", "version:get", "copy-name:get" permissions specific to each conceptset (step 1), and +-- then tie these new permissions to the right role, according to the conceptset id vs role name +-- mapping found in CONCEPT_SET_SEC_ROLE (step 2). + +-- step 1. create the sec_permission records: +INSERT INTO ${ohdsiSchema}.sec_permission (value, description) +select + concat('conceptset:', concept_set_id, ':expression:*:get'), + 'expression:*:get permission, specific to this conceptset' +from ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE +ON CONFLICT (value) +DO NOTHING; + +INSERT INTO ${ohdsiSchema}.sec_permission (value, description) +select + concat('conceptset:', concept_set_id, ':version:get'), + 'version:get permission, specific to this conceptset' +from ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE +ON CONFLICT (value) +DO NOTHING; + +INSERT INTO ${ohdsiSchema}.sec_permission (value, description) +select + concat('conceptset:', concept_set_id, ':copy-name:get'), + 'copy-name:get permission, specific to this conceptset' +from ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE +ON CONFLICT (value) +DO NOTHING; + + +-- step 2. insert sec_role_permissions: +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +Select + sec_role.id, + sec_permission.id +from + ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE + join ${ohdsiSchema}.sec_role on CONCEPT_SET_SEC_ROLE.sec_role_name = sec_role.name + join ${ohdsiSchema}.sec_permission on concat('conceptset:', CONCEPT_SET_SEC_ROLE.concept_set_id, ':expression:*:get') = sec_permission.value +ON CONFLICT (role_id, permission_id) +DO NOTHING; + +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +Select + sec_role.id, + sec_permission.id +from + ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE + join ${ohdsiSchema}.sec_role on CONCEPT_SET_SEC_ROLE.sec_role_name = sec_role.name + join ${ohdsiSchema}.sec_permission on concat('conceptset:', CONCEPT_SET_SEC_ROLE.concept_set_id, ':version:get') = sec_permission.value +ON CONFLICT (role_id, permission_id) +DO NOTHING; + +INSERT INTO ${ohdsiSchema}.sec_role_permission (role_id, permission_id) +Select + sec_role.id, + sec_permission.id +from + ${ohdsiSchema}.CONCEPT_SET_SEC_ROLE + join ${ohdsiSchema}.sec_role on CONCEPT_SET_SEC_ROLE.sec_role_name = sec_role.name + join ${ohdsiSchema}.sec_permission on concat('conceptset:', CONCEPT_SET_SEC_ROLE.concept_set_id, ':copy-name:get') = sec_permission.value +ON CONFLICT (role_id, permission_id) +DO NOTHING; From 0692cbb10b2d8030e19fe671fe1fda8efc16e236 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 15 Oct 2024 18:49:10 +0200 Subject: [PATCH 24/36] Fix: add Transactional annotation to method to fix "no session" error (#157) * fix: add Transactional annotation to method to fix "no session" error * fix: try to initialize session --- .../java/org/ohdsi/webapi/feanalysis/FeAnalysisController.java | 1 + .../converter/FeAnalysisEntityToFeAnalysisDTOConverter.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/main/java/org/ohdsi/webapi/feanalysis/FeAnalysisController.java b/src/main/java/org/ohdsi/webapi/feanalysis/FeAnalysisController.java index e290875295..a7456abfd6 100644 --- a/src/main/java/org/ohdsi/webapi/feanalysis/FeAnalysisController.java +++ b/src/main/java/org/ohdsi/webapi/feanalysis/FeAnalysisController.java @@ -153,6 +153,7 @@ public void deleteAnalysis(@PathParam("id") final Integer feAnalysisId) { * @return ID, type, name domain, description, etc of feature analysis */ @GET + @Transactional @Path("/{id}") @Produces(MediaType.APPLICATION_JSON) public FeAnalysisDTO getFeAnalysis(@PathParam("id") final Integer feAnalysisId) { diff --git a/src/main/java/org/ohdsi/webapi/feanalysis/converter/FeAnalysisEntityToFeAnalysisDTOConverter.java b/src/main/java/org/ohdsi/webapi/feanalysis/converter/FeAnalysisEntityToFeAnalysisDTOConverter.java index f39f12cd2f..f215237149 100644 --- a/src/main/java/org/ohdsi/webapi/feanalysis/converter/FeAnalysisEntityToFeAnalysisDTOConverter.java +++ b/src/main/java/org/ohdsi/webapi/feanalysis/converter/FeAnalysisEntityToFeAnalysisDTOConverter.java @@ -7,6 +7,7 @@ import org.ohdsi.webapi.feanalysis.dto.*; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import org.hibernate.Hibernate; import java.util.List; import java.util.Objects; @@ -46,6 +47,7 @@ private Object convertDesignToJson(final FeAnalysisEntity source) { switch (source.getType()) { case CRITERIA_SET: FeAnalysisWithCriteriaEntity sourceWithCriteria = (FeAnalysisWithCriteriaEntity) source; + Hibernate.initialize(sourceWithCriteria.getDesign()); // Explicitly initialize the collection return sourceWithCriteria.getDesign() .stream() .map(this::convertCriteria) From d7b51f25a96d45562c0c01a8117a2988cd16fd9a Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Tue, 12 Nov 2024 12:27:15 -0600 Subject: [PATCH 25/36] dep: update base runtime image to amazoncorretto:8u432-al2023-jre (#164) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 961fa265cd..ec09998dc4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM amazoncorretto:8u422-al2023-jre +FROM amazoncorretto:8u432-al2023-jre # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From c1b017a0fbafbb44a51b8629954b76c288ce87cf Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Mon, 25 Nov 2024 21:44:27 +0100 Subject: [PATCH 26/36] fix: use /auth/request instead of /auth/mapping Arborist endpoint (#166) --- pom.xml | 2 +- .../TeamProjectBasedAuthorizingFilter.java | 65 +++++++++---------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/pom.xml b/pom.xml index ec143a1ac2..f9b599cef9 100644 --- a/pom.xml +++ b/pom.xml @@ -79,7 +79,7 @@ default teamproject - http://arborist-service/auth/mapping + http://arborist-service/auth/request DisabledSecurity 43200 http://localhost 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 3b4503cae9..089956d17e 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java +++ b/src/main/java/org/ohdsi/webapi/shiro/filters/TeamProjectBasedAuthorizingFilter.java @@ -13,12 +13,12 @@ import org.apache.shiro.SecurityUtils; import org.apache.shiro.web.servlet.AdviceFilter; import org.apache.shiro.web.util.WebUtils; -import org.json.JSONArray; import org.json.JSONObject; import org.ohdsi.webapi.shiro.PermissionManager; import org.ohdsi.webapi.shiro.Entities.RoleEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.ResponseEntity; import org.springframework.web.client.RestTemplate; /** @@ -134,40 +134,39 @@ private boolean checkGen3Authorization(String teamProjectRole, String login) thr logger.debug("Checking Gen3 Authorization for 'team project'={} and user={} using service={}", teamProjectRole, login, this.authorizationUrl); RestTemplate restTemplate = new RestTemplate(); String arboristAuthorizationURL = this.authorizationUrl; - String requestBody = String.format("{\"username\": \"%s\"}", login); - String jsonResponseString = restTemplate.postForObject(arboristAuthorizationURL, requestBody, String.class); - - JSONObject jsonObject = new JSONObject(jsonResponseString); - - if (!jsonObject.keySet().contains(teamProjectRole)) { - logger.warn("User is not authorized to access this team project's data"); - return false; - } else { - JSONArray teamProjectAuthorizations = jsonObject.getJSONArray(teamProjectRole); - logger.debug("Found authorizations={}", teamProjectAuthorizations); - // 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; - } - String expectedMethod = "access"; - String expectedService = "atlas-argo-wrapper-and-cohort-middleware"; // TODO - make the service name configurable? - 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"); + String expectedMethod = "access"; + String expectedService = "atlas-argo-wrapper-and-cohort-middleware"; // TODO - make the service name configurable? + String requestBody = String.format( + "{\"user\": {" + + " \"user_id\": \"%s\"" + + " }," + + " \"request\": {\n" + + " \"resource\": \"%s\"," + + " \"action\": {" + + " \"service\": \"%s\"," + + " \"method\": \"%s\"" + + " }" + + " }" + + "}", + login, teamProjectRole, expectedService, expectedMethod + ); + ResponseEntity responseEntity = restTemplate.postForEntity(arboristAuthorizationURL, requestBody, String.class); + if (responseEntity.getStatusCode().value() == 200) { + String responseBody = responseEntity.getBody(); + JSONObject jsonObject = new JSONObject(responseBody); + if (jsonObject.optBoolean("auth", false)) { + // auth is true, handle success + logger.debug("Authorization successful!"); return true; - } + } else { + // auth response is missing or false + logger.error("Authorization failed."); + return false; } - 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; + } else { + // HTTP response status is not 200 + logger.error("Request failed with status: {} ", responseEntity.getStatusCode()); + return false; } } From 9e40134bee94e408ab0b2527a59cac3a22e1c531 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Mon, 13 Jan 2025 21:24:59 -0600 Subject: [PATCH 27/36] empty commit: trigger rebuild From 636b4e86d7b659469ef7291ae55b5d6b81d03590 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Wed, 15 Jan 2025 14:55:06 -0600 Subject: [PATCH 28/36] Security updates (#169) * dep: update xstream to 1.4.21 * dep: update commons-codes to 1.14 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index f9b599cef9..c99162927f 100644 --- a/pom.xml +++ b/pom.xml @@ -618,7 +618,7 @@ commons-codec commons-codec - 1.13 + 1.14 org.codehaus.jettison @@ -739,7 +739,7 @@ com.thoughtworks.xstream xstream - 1.4.20 + 1.4.21 org.springframework.boot From 0c4c38b29e4486f44222db0381db37a5bccc9ddb Mon Sep 17 00:00:00 2001 From: cmlsn <100160785+cmlsn@users.noreply.github.com> Date: Thu, 16 Jan 2025 09:29:24 -0800 Subject: [PATCH 29/36] moved from dockerhub to try and fix Snyk scan issues. (#170) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index ec09998dc4..fe21d74a0b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM amazoncorretto:8u432-al2023-jre +FROM public.ecr.aws/docker/library/amazoncorretto:8u432-al2023-jre # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From ada5713f29d172d6c458490dd6f94e92be399137 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 16 Jan 2025 11:47:01 -0600 Subject: [PATCH 30/36] Update Dockerfile (#171) * Update Dockerfile * Fix Dockerfile syntax error --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index fe21d74a0b..cf6e67fe75 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM public.ecr.aws/docker/library/amazoncorretto:8u432-al2023-jre +FROM docker.io/library/amazoncorretto:8u432-al2023-jre@sha256:aacb886ec3d61bf9db6091f0fa0904a4cea235f65fbe5c0de3e28a666fd6de0b # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From 2af515a3561826ccac4de6ce34b7f88795b4e0d6 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 16 Jan 2025 11:53:53 -0600 Subject: [PATCH 31/36] Update Dockerfile (#172) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index cf6e67fe75..277b96f70e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM docker.io/library/amazoncorretto:8u432-al2023-jre@sha256:aacb886ec3d61bf9db6091f0fa0904a4cea235f65fbe5c0de3e28a666fd6de0b +FROM amazoncorretto:8u432-al2023-jre@sha256:aacb886ec3d61bf9db6091f0fa0904a4cea235f65fbe5c0de3e28a666fd6de0b # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From 43904a5d24f5b6cf9318b39d8ecf8e7dcb8e8769 Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Thu, 16 Jan 2025 12:04:43 -0600 Subject: [PATCH 32/36] Update Dockerfile (#173) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 277b96f70e..d4bc6cfc13 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM amazoncorretto:8u432-al2023-jre@sha256:aacb886ec3d61bf9db6091f0fa0904a4cea235f65fbe5c0de3e28a666fd6de0b +FROM --platform=linux/amd64 amazoncorretto:8u432-al2023-jre # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From 11773919b0defcd54420b58114b758bdcf253b2a Mon Sep 17 00:00:00 2001 From: Andrew Prokhorenkov Date: Fri, 17 Jan 2025 12:50:00 -0600 Subject: [PATCH 33/36] Update Dockerfile (#174) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index d4bc6cfc13..b99d2cf776 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ RUN mvn package ${MAVEN_PARAMS} \ && rm WebAPI.war # OHDSI WebAPI and ATLAS web application running as a Spring Boot application with Java 8 -FROM --platform=linux/amd64 amazoncorretto:8u432-al2023-jre +FROM public.ecr.aws/amazoncorretto/amazoncorretto:8u432-al2023-jre # Any Java options to pass along, e.g. memory, garbage collection, etc. ENV JAVA_OPTS="" From 92683f77767243d866afa3e8ce01c6ea23407186 Mon Sep 17 00:00:00 2001 From: Beatrix McBride <95314232+beamcb@users.noreply.github.com> Date: Mon, 3 Feb 2025 11:33:34 -0600 Subject: [PATCH 34/36] Create NOTICE --- NOTICE | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 NOTICE diff --git a/NOTICE b/NOTICE new file mode 100644 index 0000000000..bc84ec441f --- /dev/null +++ b/NOTICE @@ -0,0 +1,7 @@ +Copyright 2014-2025 University of Chicago + +Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. From d409c3b328a2f12499a59edcb137924b8ad0f0f2 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 14 Mar 2025 21:26:54 +0100 Subject: [PATCH 35/36] Feat: add permissions needed for cohort characterization and feature analysis to role 15 (#176) * feat: add the extra permissions needed for cohort characterization and feature analysis to role 15 fix: disable two unecessary global cohort-characterization permissions, incompatible with "teamprojects" ...and improve debug logs fix: add cohort-characterization id to various methods/api calls - This allows for the correct (more strict) permission check to find place based on the user's permissions on the given cohort-characterization - Also expanded the new api patterns into CohortCharacterizationPermissionSchema and removed cohort-characterization permissions that were too broad from role 15 migration script. * fix: remove wrong/nonsensical(?) role cohort-characterization:design:%s:get --- .../cohortcharacterization/CcController.java | 60 +++++++++-------- .../cohortcharacterization/CcServiceImpl.java | 6 +- ...ohortCharacterizationPermissionSchema.java | 16 ++++- .../ohdsi/webapi/shiro/PermissionManager.java | 2 +- ..._restricted_read_restricted_role_part3.sql | 65 +++++++++++++++++++ 5 files changed, 116 insertions(+), 33 deletions(-) create mode 100644 src/main/resources/db/migration/postgresql/V2.15.0.20250307190500__custom_ctds_more_restricted_read_restricted_role_part3.sql diff --git a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java index d32f478a8d..a95281590a 100644 --- a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java +++ b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcController.java @@ -375,14 +375,15 @@ public List getGenerationList(@PathParam("id") final Long i /** * Get generation information by generation id + * @param id The id for an existing cohort characterization * @param generationId The generation id to look up * @return Data about the generation including the generation id, sourceKey, hashcode, start and end times */ @GET - @Path("/generation/{generationId}") + @Path("/{id}/generation/{generationId}") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public CommonGenerationDTO getGeneration(@PathParam("generationId") final Long generationId) { + public CommonGenerationDTO getGeneration(@PathParam("id") final Long id, @PathParam("generationId") final Long generationId) { CcGenerationEntity generationEntity = service.findGenerationById(generationId); return sensitiveInfoService.filterSensitiveInfo(conversionService.convert(generationEntity, CommonGenerationDTO.class), @@ -391,76 +392,80 @@ public CommonGenerationDTO getGeneration(@PathParam("generationId") final Long g /** * Delete a cohort characterization generation + * @param id The id for an existing cohort characterization * @param generationId */ - @DELETE - @Path("/generation/{generationId}") - @Produces(MediaType.APPLICATION_JSON) - @Consumes(MediaType.APPLICATION_JSON) - public void deleteGeneration(@PathParam("generationId") final Long generationId) { - service.deleteCcGeneration(generationId); - } + // @DELETE //not used + // @Path("/generation/{generationId}") + // @Produces(MediaType.APPLICATION_JSON) + // @Consumes(MediaType.APPLICATION_JSON) + // public void deleteGeneration(@PathParam("id") final Long id, @PathParam("generationId") final Long generationId) { + // service.deleteCcGeneration(generationId); + // } /** * Get the definition of a cohort characterization for a given generation id + * @param id The id for an existing cohort characterization * @param generationId * @return A cohort characterization definition */ @GET - @Path("/generation/{generationId}/design") + @Path("/{id}/generation/{generationId}/design") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public CcExportDTO getGenerationDesign( - @PathParam("generationId") final Long generationId) { + @PathParam("id") final Long id, @PathParam("generationId") final Long generationId) { return conversionService.convert(service.findDesignByGenerationId(generationId), CcExportDTO.class); } /** * Get the total number of analyses in a cohort characterization * + * @param id The id for an existing cohort characterization * @param generationId * @return The total number of analyses in the given cohort characterization */ @GET - @Path("/generation/{generationId}/result/count") + @Path("/{id}/generation/{generationId}/result/count") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) - public Long getGenerationsResultsCount( @PathParam("generationId") final Long generationId) { + public Long getGenerationsResultsCount(@PathParam("id") final Long id, @PathParam("generationId") final Long generationId) { return service.getCCResultsTotalCount(generationId); } /** * Get cohort characterization results + * @param id The id for an existing cohort characterization * @param generationId id for generation * @param thresholdLevel The max prevelance for a covariate. Covariates that occur in less than {threholdLevel}% * of the cohort will not be returned. Default is 0.01 = 1% * @return The complete set of characterization analyses filtered by the thresholdLevel parameter */ - @GET - @Path("/generation/{generationId}/result") - @Produces(MediaType.APPLICATION_JSON) - @Consumes(MediaType.APPLICATION_JSON) - public List getGenerationsResults( - @PathParam("generationId") final Long generationId, @DefaultValue("0.01") @QueryParam("thresholdLevel") final float thresholdLevel) { - return service.findResultAsList(generationId, thresholdLevel); - } + // @GET //NOT USED + // @Path("/{id}/generation/{generationId}/result") + // @Produces(MediaType.APPLICATION_JSON) + // @Consumes(MediaType.APPLICATION_JSON) + // public List getGenerationsResults( + // @PathParam("id") final Long id, @PathParam("generationId") final Long generationId, @DefaultValue("0.01") @QueryParam("thresholdLevel") final float thresholdLevel) { + // return service.findResultAsList(generationId, thresholdLevel); + // } @POST - @Path("/generation/{generationId}/result") + @Path("/{id}/generation/{generationId}/result") @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) @ReturnType("java.lang.Object") public GenerationResults getGenerationsResults( - @PathParam("generationId") final Long generationId, @RequestBody ExportExecutionResultRequest params) { + @PathParam("id") final Long id, @PathParam("generationId") final Long generationId, @RequestBody ExportExecutionResultRequest params) { return service.findData(generationId, params); } @POST - @Path("/generation/{generationId}/result/export") + @Path("/{id}/generation/{generationId}/result/export") @Produces(MediaType.APPLICATION_OCTET_STREAM) @Consumes(MediaType.APPLICATION_JSON) public Response exportGenerationsResults( - @PathParam("generationId") final Long generationId, ExportExecutionResultRequest params) { + @PathParam("id") final Long id, @PathParam("generationId") final Long generationId, ExportExecutionResultRequest params) { GenerationResults res = service.exportExecutionResult(generationId, params); return prepareExecutionResultResponse(res.getReports()); } @@ -511,9 +516,10 @@ private void createZipEntry(ZipOutputStream zos, Report report) throws IOExcepti } @GET - @Path("/generation/{generationId}/explore/prevalence/{analysisId}/{cohortId}/{covariateId}") + @Path("/{id}/generation/{generationId}/explore/prevalence/{analysisId}/{cohortId}/{covariateId}") @Produces(MediaType.APPLICATION_JSON) - public List getPrevalenceStat(@PathParam("generationId") Long generationId, + public List getPrevalenceStat(@PathParam("id") final Long id, + @PathParam("generationId") Long generationId, @PathParam("analysisId") Long analysisId, @PathParam("cohortId") Long cohortId, @PathParam("covariateId") Long covariateId) { diff --git a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcServiceImpl.java b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcServiceImpl.java index a10b3a8ff9..b32a1c49ce 100644 --- a/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcServiceImpl.java +++ b/src/main/java/org/ohdsi/webapi/cohortcharacterization/CcServiceImpl.java @@ -547,9 +547,11 @@ public Page getPageWithLinkedEntities(final Pageab @Override public Page getPage(final Pageable pageable) { - List ccList = repository.findAll() - .stream().filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true) + List ccList = repository.findAll(); + log.debug("UNFILTERED CC list size {}", ccList.size()); + ccList = ccList.stream().filter(!defaultGlobalReadPermissions ? entity -> permissionService.hasReadAccess(entity) : entity -> true) .collect(Collectors.toList()); + log.debug("FILTERED CC list size {}", ccList.size()); return getPageFromResults(pageable, ccList); } diff --git a/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java b/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java index f6ea10012a..920d02e691 100644 --- a/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java +++ b/src/main/java/org/ohdsi/webapi/security/model/CohortCharacterizationPermissionSchema.java @@ -11,17 +11,27 @@ public class CohortCharacterizationPermissionSchema extends EntityPermissionSche private static Map writePermissions = new HashMap() {{ put("cohort-characterization:%s:put", "Update Cohort Characterization with ID = %s"); put("cohort-characterization:%s:delete", "Delete Cohort Characterization with ID = %s"); + // new generation permissions based on previous ones that were too broad: + put("cohort-characterization:%s:generation:*:result:post", ""); + put("cohort-characterization:%s:generation:*:result:export:post", ""); + }}; private static Map readPermissions = new HashMap() {{ put("cohort-characterization:%s:get", "Get cohort characterization"); put("cohort-characterization:%s:generation:get", "Get cohort characterization generations"); - put("cohort-characterization:generation:*:get", "Get cohort characterization generation"); - put("cohort-characterization:design:get", "cohort-characterization:design:get"); + // Disable these two unecessary global cohort-characterization permissions, and incompatible with "teamprojects": + // put("cohort-characterization:generation:*:get", "Get cohort characterization generation"); + // put("cohort-characterization:design:get", "cohort-characterization:design:get"); + // Have specific ones instead: + put("cohort-characterization:%s:generation:*:get", "Get any generation for a cohort characterization record"); put("cohort-characterization:%s:design:get", "Get cohort characterization design"); - put("cohort-characterization:design:%s:get", "view cohort characterization with id %s"); put("cohort-characterization:%s:version:get", "Get list of characterization versions"); put("cohort-characterization:%s:version:*:get", "Get list of characterization version"); + // new generation permissions based on previous ones that were too broad: + put("cohort-characterization:%s:generation:*:design:get", ""); + put("cohort-characterization:%s:generation:*:result:count:get", ""); + put("cohort-characterization:%s:generation:*:explore:prevalence:*:get", ""); }}; public CohortCharacterizationPermissionSchema() { diff --git a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java index ab8a5909f3..4ea3d0bb65 100644 --- a/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java +++ b/src/main/java/org/ohdsi/webapi/shiro/PermissionManager.java @@ -154,7 +154,7 @@ public void removeUserFromUserRole(String roleName, String login) { this.userRoleRepository.delete(userRole); } } else { - logger.debug("Role {} not found", roleName); + logger.debug("Role {} not found as a user role - likely a system role", roleName); } } diff --git a/src/main/resources/db/migration/postgresql/V2.15.0.20250307190500__custom_ctds_more_restricted_read_restricted_role_part3.sql b/src/main/resources/db/migration/postgresql/V2.15.0.20250307190500__custom_ctds_more_restricted_read_restricted_role_part3.sql new file mode 100644 index 0000000000..2fba298f19 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V2.15.0.20250307190500__custom_ctds_more_restricted_read_restricted_role_part3.sql @@ -0,0 +1,65 @@ +-- This script will add cohort-characterization permissions to 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 +), + char_generate_perms as ( + select distinct concat(lcg,mcg,rcg) perm + from ( + select * + from (values + ('cohort-characterization:*:generation:') + ) t1111 (lcg) + cross join + ( select source_key + from vocab_source + ) t2222 (mcg) + cross join + (values + (':post') + ) t3333 (rcg) + ) combined +) -- TODO - consider also adding "cdmresults:EUNOMIA:conceptRecordCount:post" (where EUNOMIA is a vocab_source.source_key example) +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 char_generate_perms) + or + sp.value IN + ( + 'cohort-characterization:*:exists:get', -- weird one...but is needed / used by UI before saving a new cohort-characterization.... + 'feature-analysis:*:exists:get', -- weird one...but is needed / used by UI + 'feature-analysis:*:get' -- TODO - currently needed for accessing standard feature records (and features created by others) - will need extra work to isolate + ) +; + +-- COHORT_CHARACTERIZATION_SEC_ROLE is our custom view that returns a list of cohort characterization ids per role +-- when the role has a permission starting with "cohort-characterization:". E.g. how view output looks like: +-- +-- cohort_characterization_id | sec_role_name +-- ----------------------------+------------------------- +-- 8 | /gwas_projects/project2 +-- 9 | /gwas_projects/project2 +-- 300 | /gwas_projects/project1 + +DROP VIEW IF EXISTS ${ohdsiSchema}.COHORT_CHARACTERIZATION_SEC_ROLE; +CREATE VIEW ${ohdsiSchema}.COHORT_CHARACTERIZATION_SEC_ROLE AS + select + distinct cast(regexp_replace(sec_permission.value, + '^cohort-characterization:([0-9]+):.*','\1') as integer) as cohort_characterization_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 ~ 'cohort-characterization:[0-9]+' +; + +-- TODO - solve all todo's above regarding too broad permissions +-- >> see also how this was done for conceptsets in the java code and in the migration script V2.15.0.20240801170500__custom_ctds_more_restricted_read_restricted_role_part2.sql From ede37b974628207725f06e9797dfb3907291b7cd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 12 Sep 2025 17:47:28 +0000 Subject: [PATCH 36/36] Bump the maven group across 1 directory with 11 updates Bumps the maven group with 11 updates in the / directory: | Package | From | To | | --- | --- | --- | | [org.apache.activemq:activemq-client](https://github.com/apache/activemq) | `5.15.16` | `6.1.7` | | [org.apache.activemq:activemq-openwire-legacy](https://github.com/apache/activemq) | `5.15.16` | `5.16.8` | | commons-beanutils:commons-beanutils | `1.9.4` | `1.11.0` | | org.apache.santuario:xmlsec | `2.1.7` | `2.2.6` | | org.hibernate:hibernate-validator | `5.4.2.Final` | `6.2.0.Final` | | org.apache.commons:commons-lang3 | `3.12.0` | `3.18.0` | | commons-fileupload:commons-fileupload | `1.5` | `1.6.0` | | [org.pac4j:pac4j-oidc](https://github.com/pac4j/pac4j) | `4.0.0` | `4.5.5` | | [org.springframework.security:spring-security-crypto](https://github.com/spring-projects/spring-security) | `4.2.16.RELEASE` | `6.3.8` | | [org.springframework.ldap:spring-ldap-core](https://github.com/spring-projects/spring-ldap) | `2.3.2.RELEASE` | `2.4.4` | | com.databricks:databricks-jdbc | `2.6.34` | `2.6.40` | Updates `org.apache.activemq:activemq-client` from 5.15.16 to 6.1.7 - [Commits](https://github.com/apache/activemq/compare/activemq-5.15.16...activemq-6.1.7) Updates `org.apache.activemq:activemq-openwire-legacy` from 5.15.16 to 5.16.8 - [Commits](https://github.com/apache/activemq/compare/activemq-5.15.16...activemq-5.16.8) Updates `commons-beanutils:commons-beanutils` from 1.9.4 to 1.11.0 Updates `org.apache.santuario:xmlsec` from 2.1.7 to 2.2.6 Updates `org.hibernate:hibernate-validator` from 5.4.2.Final to 6.2.0.Final Updates `org.apache.commons:commons-lang3` from 3.12.0 to 3.18.0 Updates `commons-fileupload:commons-fileupload` from 1.5 to 1.6.0 Updates `org.pac4j:pac4j-oidc` from 4.0.0 to 4.5.5 - [Commits](https://github.com/pac4j/pac4j/compare/pac4j-4.0.0...pac4j-4.5.5) Updates `org.springframework.security:spring-security-crypto` from 4.2.16.RELEASE to 6.3.8 - [Release notes](https://github.com/spring-projects/spring-security/releases) - [Changelog](https://github.com/spring-projects/spring-security/blob/main/RELEASE.adoc) - [Commits](https://github.com/spring-projects/spring-security/compare/4.2.16.RELEASE...6.3.8) Updates `org.springframework.ldap:spring-ldap-core` from 2.3.2.RELEASE to 2.4.4 - [Release notes](https://github.com/spring-projects/spring-ldap/releases) - [Changelog](https://github.com/spring-projects/spring-ldap/blob/main/changelog.txt) - [Commits](https://github.com/spring-projects/spring-ldap/compare/2.3.2.RELEASE...2.4.4) Updates `com.databricks:databricks-jdbc` from 2.6.34 to 2.6.40 --- updated-dependencies: - dependency-name: org.apache.activemq:activemq-client dependency-version: 6.1.7 dependency-type: direct:production dependency-group: maven - dependency-name: org.apache.activemq:activemq-openwire-legacy dependency-version: 5.16.8 dependency-type: direct:production dependency-group: maven - dependency-name: commons-beanutils:commons-beanutils dependency-version: 1.11.0 dependency-type: direct:production dependency-group: maven - dependency-name: org.apache.santuario:xmlsec dependency-version: 2.2.6 dependency-type: direct:production dependency-group: maven - dependency-name: org.hibernate:hibernate-validator dependency-version: 6.2.0.Final dependency-type: direct:production dependency-group: maven - dependency-name: org.apache.commons:commons-lang3 dependency-version: 3.18.0 dependency-type: direct:production dependency-group: maven - dependency-name: commons-fileupload:commons-fileupload dependency-version: 1.6.0 dependency-type: direct:production dependency-group: maven - dependency-name: org.pac4j:pac4j-oidc dependency-version: 4.5.5 dependency-type: direct:production dependency-group: maven - dependency-name: org.springframework.security:spring-security-crypto dependency-version: 6.3.8 dependency-type: direct:production dependency-group: maven - dependency-name: org.springframework.ldap:spring-ldap-core dependency-version: 2.4.4 dependency-type: direct:production dependency-group: maven - dependency-name: com.databricks:databricks-jdbc dependency-version: 2.6.40 dependency-type: direct:production dependency-group: maven ... Signed-off-by: dependabot[bot] --- pom.xml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pom.xml b/pom.xml index c99162927f..01becf8143 100644 --- a/pom.xml +++ b/pom.xml @@ -26,13 +26,13 @@ 3.2.0 8.5.87 - 1.5 + 1.6.0 1.11.2 2.14 1.16.1 3.1.2 - 4.0.0 + 4.5.5 2.17.0 org.ohdsi.webapi.WebApi false @@ -588,12 +588,12 @@ org.apache.activemq activemq-client - 5.15.16 + 6.1.7 org.apache.activemq activemq-openwire-legacy - 5.15.16 + 5.16.8 org.apache.activemq @@ -603,7 +603,7 @@ commons-beanutils commons-beanutils - 1.9.4 + 1.11.0 net.minidev @@ -643,7 +643,7 @@ org.apache.santuario xmlsec - 2.1.7 + 2.2.6 net.lingala.zip4j @@ -799,7 +799,7 @@ org.hibernate hibernate-validator - 5.4.2.Final + 6.2.0.Final org.hibernate @@ -849,7 +849,7 @@ org.apache.commons commons-lang3 - 3.12.0 + 3.18.0 org.flywaydb @@ -1082,7 +1082,7 @@ org.springframework.security spring-security-crypto - 4.2.16.RELEASE + 6.3.8 com.zaxxer @@ -1153,7 +1153,7 @@ org.springframework.ldap spring-ldap-core - 2.3.2.RELEASE + 2.4.4 com.odysseusinc @@ -1564,7 +1564,7 @@ com.databricks databricks-jdbc - 2.6.34 + 2.6.40 runtime