From a63a0bdfdc47f826bea157a737eae9692648b05d Mon Sep 17 00:00:00 2001 From: Gcolon021 <34667267+Gcolon021@users.noreply.github.com> Date: Mon, 9 Sep 2024 14:11:16 -0400 Subject: [PATCH] [ALS-7197] BDC Auth access: User able to access data (#203) * - The clinical query template has been updated to include the `parentAccessField` in the fields section. The `parentAccessionField` value is now set to `\\\\_Parent Study Accession with Subject ID\\\\`. - The `TOPMED+PARENT` access rule, associated with clinical privileges, has been fixed. The rule previously had misconfigured `subAccessRules` and `gates`: - The `TOPMED_CONSENT` gate consent rule was incorrectly set to `IS_EMPTY` instead of `IS_NOT_EMPTY`, causing queries to be erroneously rejected, even when the `"categoryFilters": "\\_topmed_consents\\"` section was correctly included in the query. - A missing `subAccessRule` named `ALLOW_TOPMED_CONSENT` was added. This rule ensures that all required consents are present in the `\\_topmed_consents\\` section of the query template. - A new method, `configureClinicalAccessRuleWithPhenoSubRule`, was added to `AccessRuleService`. This method contains the configuration for this access rule. - Previously, there were many unassociated access rules. All access rules are now correctly linked to other rules and privileges. In Spring Boot, any entity property defining a parent-child relationship must use the `cascade = CascadeType.ALL` option in the `@ManyToOne` or `@ManyToMany` annotations to ensure updates to the parent or child entities on save. - A new role, `MANUAL_ROLE_NAMED_DATASET`, has been assigned to all BDC users. This role grants users access to the `/named/dataset/`, `/named/dataset/{uuid}`, and `/query/{uuid}/metadata` endpoints. The role, along with its privilege and rule definition, is now part of the BDC infrastructure. - The `createPhoneTypeSubRule` method now correctly removes double-escaped characters. For example, a concept path formatted as `\\\\phs000xxx\\\\` is now converted to `\\phs000xxx\\`. - The `upsertConsentGate` method now properly escapes backslashes in rules. - The `extractAndCheckRule` method now correctly evaluates access rules that need to check map nodes. When using JsonPath to retrieve a node value, it is naturally converted into a list. This list is now converted into a Map for key checking. - When evaluating `subAccessRules` for a given rule, the rules must first be merged before evaluation. Since sub access rules may overlap with other rules, the method `preProcessARBySortedKeys` is used to handle the merging process. - The `pic-sure-auth-micro-app` was originally designed with an ephemeral database. With each deployment, all roles, privileges, and access rules were re-created. This provided an opportunity to add new allowed query types and standard access rules. Now that our database is persisted, there was no mechanism to add new allowed query types or standard access rules. A new method has been added to `PrivilegeService` named `updateAllPrivilegesOnStartup`. - This method updates all privileges to include all current standard access rules. Any standard access rules that need to be removed from a privilege must be handled via a migration script. - The method also updates all allowed query types. All existing allowed query types are removed from each `access_rule` that currently has allowed query types as `subAccessRules`. Once these are removed, all currently defined allowed query types are added to the `access_rule`, enabling the addition or removal of allowed query types without needing migration scripts. * Add transactional annotation to PrivilegeRepository * Change event listener and improve privilege update logic * Refactor privilege update method to avoid transactional query Replaced the custom query with direct calls to add access rules and save privileges. This simplifies the code and utilizes existing save method, ensuring transactional consistency. * Fix sub-access rule addition in PrivilegeService * Refactor role name constants to uppercase * Set logging level to DEBUG for auth service implementation Changed the logging level for `edu.harvard.hms.dbmi.avillach.auth.service.impl` from INFO to DEBUG to facilitate detailed debugging. This will help in tracking down issues more effectively during development and troubleshooting. * Switch logging level from DEBUG to TRACE in AccessRuleService Changed the logging level from DEBUG to TRACE for specific methods in AccessRuleService to reduce the verbosity of the application logs. This will make debugging more efficient by including detailed logs specific to fine-grain debugging levels without cluttering higher-level log outputs. * Restrict logging to denied access attempts Refactor log statement in AuthorizationService to log only when access is denied. This change reduces unnecessary log entries for granted access, improving log readability and performance. * Fix double backslash handling in Topmed access rule generation Address an issue where double backslashes in the concept path were not appropriately converted, causing potential mismatches in rule text generation. This ensures consistency by replacing all instances of `\\\\` with `\\` before forming the rule text. * Lower logging level in evaluateAccessRule method Changed log level from debug to trace for initial access rule evaluation steps. This change reduces verbosity in the logs, helping to focus on more critical debug information. * Update pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java --- .../auth/repository/AccessRuleRepository.java | 9 + .../auth/repository/PrivilegeRepository.java | 6 + .../auth/service/impl/AccessRuleService.java | 180 +++++++++++------ .../auth/service/impl/PrivilegeService.java | 181 ++++++++++++------ .../auth/service/impl/RoleService.java | 10 +- .../auth/service/impl/UserService.java | 26 +-- .../AimAheadAuthenticationService.java | 4 +- .../FENCEAuthenticationService.java | 5 - .../OpenAuthenticationService.java | 4 +- .../authorization/AuthorizationService.java | 18 +- .../src/main/resources/application.properties | 9 +- .../RASAuthenticationServiceTest.java | 2 +- .../authorization/AccessRuleServiceTest.java | 129 ------------- 13 files changed, 297 insertions(+), 286 deletions(-) delete mode 100644 pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AccessRuleServiceTest.java diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/AccessRuleRepository.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/AccessRuleRepository.java index c668dcd57..a97dce8c3 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/AccessRuleRepository.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/AccessRuleRepository.java @@ -2,8 +2,12 @@ import edu.harvard.hms.dbmi.avillach.auth.entity.AccessRule; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; +import java.util.List; +import java.util.Set; import java.util.UUID; /** @@ -16,4 +20,9 @@ public interface AccessRuleRepository extends JpaRepository { AccessRule findByName(String name); + @Query("SELECT p.accessRules FROM privilege p WHERE p.uuid = :uuid") + Set findAccessRulesByPrivilegeId(@Param("uuid") UUID uuid); + + @Query("SELECT p.accessRules FROM privilege p WHERE p.uuid IN :privilegeIds") + List getAccessRulesByPrivilegeIds(@Param("privilegeIds") List privilegeIds); } diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/PrivilegeRepository.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/PrivilegeRepository.java index c7af74330..4a1a2dcb2 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/PrivilegeRepository.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/PrivilegeRepository.java @@ -1,9 +1,15 @@ package edu.harvard.hms.dbmi.avillach.auth.repository; +import edu.harvard.hms.dbmi.avillach.auth.entity.AccessRule; import edu.harvard.hms.dbmi.avillach.auth.entity.Privilege; +import jakarta.transaction.Transactional; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; +import java.util.Set; import java.util.UUID; /** diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AccessRuleService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AccessRuleService.java index 1e71fd602..e032821aa 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AccessRuleService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AccessRuleService.java @@ -33,8 +33,9 @@ public class AccessRuleService { private final ConcurrentHashMap accessRuleCache = new ConcurrentHashMap<>(); private Set allowQueryTypeRules; + private Set standardAccessRules; - private static final String parentAccessionField = "\\\\_Parent Study Accession with Subject ID\\\\"; + public static final String parentAccessionField = "\\\\_Parent Study Accession with Subject ID\\\\"; private static final String topmedAccessionField = "\\\\_Topmed Study Accession with Subject ID\\\\"; private final String fence_harmonized_consent_group_concept_path; private final String fence_parent_consent_group_concept_path; @@ -76,7 +77,7 @@ public void init() { "\\\\_studies\\\\", "\\\\_studies_consents\\\\", //used to provide consent-level counts for open access "\\\\_parent_consents\\\\", //parent consents not used for auth (use combined _consents) - "\\\\_Consents\\\\" ///old _Consents\Short Study... path no longer used, but still present in examples. + "\\\\_Consents\\\\" }; logger.info("fence_standard_access_rules: {}", fence_standard_access_rules); @@ -125,6 +126,10 @@ public List removeAccessRuleById(String accessRuleId) { } public AccessRule save(AccessRule accessRule) { + // if the access rule exists in the AccessRule cache, update it + if (accessRuleCache.containsKey(accessRule.getName())) { + accessRuleCache.put(accessRule.getName(), accessRule); + } return this.accessRuleRepo.save(accessRule); } @@ -287,8 +292,8 @@ private AccessRule mergeAccessRules(AccessRule baseAccessRule, AccessRule access } public boolean evaluateAccessRule(Object parsedRequestBody, AccessRule accessRule) { - logger.debug("evaluateAccessRule() starting with: {}", parsedRequestBody); - logger.debug("evaluateAccessRule() access rule: {}", accessRule.getName()); + logger.trace("evaluateAccessRule() starting with: {}", parsedRequestBody); + logger.trace("evaluateAccessRule() access rule: {}", accessRule.getName()); Set gates = accessRule.getGates(); boolean gatesPassed = true; @@ -340,7 +345,9 @@ public boolean evaluateAccessRule(Object parsedRequestBody, AccessRule accessRul return false; } else { if (accessRule.getSubAccessRule() != null) { - for (AccessRule subAccessRule : accessRule.getSubAccessRule()) { + // We need to check all the sub rules as merged rules; they can overlap + Set mergedSubRules = preProcessARBySortedKeys(accessRule.getSubAccessRule()); + for (AccessRule subAccessRule : mergedSubRules) { if (!extractAndCheckRule(subAccessRule, parsedRequestBody)) { logger.debug("Query Rejected by rule(2) {} :: {} :: {}", subAccessRule.getRule(), subAccessRule.getType(), subAccessRule.getValue()); return false; @@ -369,17 +376,19 @@ public boolean extractAndCheckRule(AccessRule accessRule, Object parsedRequestBo int accessRuleType = accessRule.getType(); try { + logger.debug("extractAndCheckRule() -> JsonPath.parse().read() with parsedRequestBody - {} - {}", parsedRequestBody, rule); requestBodyValue = JsonPath.parse(parsedRequestBody).read(rule); - // Json parse will always return a list even when we want a map (to check keys) - if (requestBodyValue instanceof JsonArray && ((JsonArray) requestBodyValue).size() == 1) { - requestBodyValue = ((JsonArray) requestBodyValue).get(0); + if (accessRule.getCheckMapNode() != null && accessRule.getCheckMapNode()) { + // Json parse will always return a list even when we want a map (to check keys) + if (requestBodyValue instanceof JsonArray && ((JsonArray) requestBodyValue).size() == 1) { + requestBodyValue = ((JsonArray) requestBodyValue).get(0); + } } - } catch (PathNotFoundException ex) { if (accessRuleType == AccessRule.TypeNaming.IS_EMPTY) { // We could return accessRuleType == AccessRule.TypeNaming.IS_EMPTY directly, but we want to log the reason - logger.debug("extractAndCheckRule() -> JsonPath.parse().read() PathNotFound; passing IS_EMPTY rule"); + logger.debug("extractAndCheckRule() -> JsonPath.parse().read() PathNotFound; passing IS_EMPTY rule {}", rule); return true; } logger.debug("extractAndCheckRule() -> JsonPath.parse().read() throws exception with parsedRequestBody - {} : {} - {}", parsedRequestBody, ex.getClass().getSimpleName(), ex.getMessage()); @@ -417,6 +426,9 @@ private boolean evaluateNode(Object requestBodyValue, AccessRule accessRule) { } private boolean evaluateMap(Object requestBodyValue, AccessRule accessRule) { + logger.trace("evaluateMap() access rule:{}", accessRule.getName()); + logger.trace("evaluateMap() request body value:{}", requestBodyValue); + switch (accessRule.getType()) { case (AccessRule.TypeNaming.ANY_EQUALS): case (AccessRule.TypeNaming.ANY_CONTAINS): @@ -459,6 +471,9 @@ && evaluateNode(entry.getValue(), accessRule)) } private Boolean evaluateCollection(Collection requestBodyValue, AccessRule accessRule) { + logger.trace("evaluateCollection() access rule:{}", accessRule.getName()); + logger.trace("evaluateCollection() request body value:{}", requestBodyValue); + switch (accessRule.getType()) { case (AccessRule.TypeNaming.ANY_EQUALS): case (AccessRule.TypeNaming.ANY_CONTAINS): @@ -543,10 +558,11 @@ public boolean decisionMaker(AccessRule accessRule, String requestBodyValue) { } private boolean _decisionMaker(AccessRule accessRule, String requestBodyValue, String value) { - logger.debug("_decisionMaker() starting"); - logger.debug("_decisionMaker() access rule:{}", accessRule.getName()); - logger.debug(requestBodyValue); - logger.debug(value); + logger.trace("_decisionMaker() starting"); + logger.trace("_decisionMaker() access rule:{}", accessRule.getName()); + logger.trace("_decisionMaker() request body value:{}", requestBodyValue); + logger.trace("_decisionMaker() value:{}", value); + logger.trace("_decisionMaker() access rule type:{}", accessRule.getType()); return switch (accessRule.getType()) { case AccessRule.TypeNaming.NOT_CONTAINS -> !requestBodyValue.contains(value); @@ -573,14 +589,11 @@ private boolean _decisionMaker(AccessRule accessRule, String requestBodyValue, S * @param consent_group The consent group. * @param conceptPath The concept path. * @param projectAlias The project alias. - * @param parent Whether to include parent gates. - * @param harmonized Whether to include harmonized gates. - * @param topmed Whether to include Topmed gates. */ - protected void configureAccessRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias, boolean parent, boolean harmonized, boolean topmed) { + protected void configureAccessRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) { if (ar.getGates() == null) { ar.setGates(new HashSet<>()); - ar.getGates().addAll(getGates(parent, harmonized, topmed)); + ar.getGates().addAll(getGates(true, false, false)); if (ar.getSubAccessRule() == null) { ar.setSubAccessRule(new HashSet<>()); @@ -588,7 +601,6 @@ protected void configureAccessRule(AccessRule ar, String studyIdentifier, String ar.getSubAccessRule().addAll(getAllowedQueryTypeRules()); ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias)); ar.getSubAccessRule().addAll(getTopmedRestrictedSubRules()); - this.save(ar); } } @@ -597,11 +609,11 @@ protected void configureAccessRule(AccessRule ar, String studyIdentifier, String * * @param ar The AccessRule to configure. * @param studyIdentifier The study identifier. - * @param consent_group The consent group. * @param conceptPath The concept path. * @param projectAlias The project alias. + * @return */ - protected void configureHarmonizedAccessRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) { + protected void configureHarmonizedAccessRule(AccessRule ar, String studyIdentifier, String conceptPath, String projectAlias) { if (ar.getGates() == null) { ar.setGates(new HashSet<>()); ar.getGates().add(upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data")); @@ -612,11 +624,26 @@ protected void configureHarmonizedAccessRule(AccessRule ar, String studyIdentifi ar.getSubAccessRule().addAll(getAllowedQueryTypeRules()); ar.getSubAccessRule().addAll(getHarmonizedSubRules()); ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias)); - this.save(ar); } } - private Set getAllowedQueryTypeRules() { + protected AccessRule configureClinicalAccessRuleWithPhenoSubRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) { + if (ar.getGates() == null) { + ar.setGates(new HashSet<>()); + ar.getGates().addAll(getGates(true, false, true)); + + if (ar.getSubAccessRule() == null) { + ar.setSubAccessRule(new HashSet<>()); + } + ar.getSubAccessRule().addAll(getAllowedQueryTypeRules()); + ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias)); + ar.getSubAccessRule().add(createPhenotypeSubRule(fence_topmed_consent_group_concept_path, "ALLOW_TOPMED_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true)); + } + + return ar; + } + + protected Set getAllowedQueryTypeRules() { if (allowQueryTypeRules == null) { allowQueryTypeRules = loadAllowedQueryTypeRules(); } @@ -660,6 +687,8 @@ private Set loadAllowedQueryTypeRules() { return rules; } + + private Collection getTopmedRestrictedSubRules() { Set rules = new HashSet(); rules.add(upsertTopmedRestrictedSubRule("CATEGORICAL", "$.query.query.variantInfoFilters[*].categoryVariantInfoFilters.*")); @@ -791,19 +820,23 @@ private Collection getGates(boolean parent, boolean harmon return gates; } - protected void populateAccessRule(AccessRule rule, boolean includeParent, boolean includeHarmonized, boolean includeTopmed) { + protected AccessRule populateTopmedAccessRule(AccessRule rule, boolean includeParent) { if (rule.getGates() == null) { - rule.setGates(new HashSet<>(getGates(includeParent, includeHarmonized, includeTopmed))); + rule.setGates(new HashSet<>(getGates(includeParent, false, true))); + } else { + rule.getGates().addAll(getGates(includeParent, false, true)); } if (rule.getSubAccessRule() == null) { rule.setSubAccessRule(new HashSet<>(getAllowedQueryTypeRules())); + } else { + rule.getSubAccessRule().addAll(getAllowedQueryTypeRules()); } - this.save(rule); + return rule; } - protected void populateHarmonizedAccessRule(AccessRule rule, String parentConceptPath, String studyIdentifier, String projectAlias) { + protected AccessRule populateHarmonizedAccessRule(AccessRule rule, String parentConceptPath, String studyIdentifier, String projectAlias) { if (rule.getGates() == null) { rule.setGates(new HashSet<>(Collections.singletonList( upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data") @@ -816,32 +849,37 @@ protected void populateHarmonizedAccessRule(AccessRule rule, String parentConcep rule.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias)); } - this.save(rule); + return rule; } - // A set of standard access rules that are added to all privileges - // to cache the standard access rules - private Set standardAccessRules; - protected void addStandardAccessRules(Set accessRules) { + /** + * The set of standard access rules that are added to all privileges. + * This set is cached to avoid loading the rules multiple times. + * + * @return The set of standard access rules. + */ + protected Set addStandardAccessRules() { if (standardAccessRules != null && !standardAccessRules.isEmpty()) { - accessRules.addAll(standardAccessRules); - } else { - standardAccessRules = new HashSet<>(); - for (String arName : fence_standard_access_rules.split(",")) { - if (arName.startsWith("AR_")) { - logger.info("Adding AccessRule {} to privilege", arName); - AccessRule ar = this.getAccessRuleByName(arName); - if (ar != null) { - standardAccessRules.add(ar); - } else { - logger.warn("Unable to find an access rule with name {}", arName); - } + return standardAccessRules; + } + + standardAccessRules = new HashSet<>(); + for (String arName : fence_standard_access_rules.split(",")) { + if (arName.startsWith("AR_")) { + AccessRule ar = this.getAccessRuleByName(arName); + if (ar != null) { + standardAccessRules.add(ar); + } else { + logger.warn("Unable to find an access rule with name {}", arName); } + } else { + logger.info("Skipping AccessRule {} as it does not start with AR_", arName); } - - accessRules.addAll(standardAccessRules); } + + logger.info("Added {} standard access rules to privilege", standardAccessRules.size()); + return standardAccessRules; } @@ -887,7 +925,15 @@ protected AccessRule createConsentAccessRule(String studyIdentifier, String cons protected AccessRule upsertTopmedAccessRule(String project_name, String consent_group, String label) { String ar_name = (consent_group != null && !consent_group.isEmpty()) ? "AR_TOPMED_" + project_name + "_" + consent_group + "_" + label : "AR_TOPMED_" + project_name + "_" + label; String description = "MANAGED AR for " + project_name + "." + consent_group + " Topmed data"; - String ruleText = "$.query.query.categoryFilters." + fence_topmed_consent_group_concept_path + "[*]"; + + String conceptPath = fence_topmed_consent_group_concept_path; + // Check if the conceptPath has `\\\\` present. This technically represents `\\`. + if(conceptPath != null && conceptPath.contains("\\\\")) { + // This will convert all `\\\\` to `\\`. + conceptPath = conceptPath.replaceAll("\\\\\\\\", "\\\\"); + } + + String ruleText = "$.query.query.categoryFilters." + conceptPath + "[*]"; String arValue = (consent_group != null && !consent_group.isEmpty()) ? project_name + "." + consent_group : project_name; return getOrCreateAccessRule( @@ -909,12 +955,11 @@ protected AccessRule upsertTopmedAccessRule(String project_name, String consent_ * * @param project_name The name of the project. * @param consent_group The consent group. - * @param label The label for the rule. * @return The created AccessRule. */ - protected AccessRule upsertHarmonizedAccessRule(String project_name, String consent_group, String label) { - String ar_name = "AR_TOPMED_" + project_name + "_" + consent_group + "_" + label; - logger.info("upsertHarmonizedAccessRule() Creating new access rule {}", ar_name); + protected AccessRule upsertHarmonizedAccessRule(String project_name, String consent_group) { + String ar_name = "AR_TOPMED_" + project_name + "_" + consent_group + "_" + "HARMONIZED"; + logger.debug("upsertHarmonizedAccessRule() Creating new access rule {}", ar_name); String description = "MANAGED AR for " + project_name + "." + consent_group + " Topmed data"; String ruleText = "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]"; String arValue = project_name + "." + consent_group; @@ -945,6 +990,9 @@ protected AccessRule upsertHarmonizedAccessRule(String project_name, String cons */ private AccessRule upsertConsentGate(String gateName, String rule, boolean is_present, String description) { gateName = "GATE_" + gateName + "_" + (is_present ? "PRESENT" : "MISSING"); + + escapePath(rule); + return getOrCreateAccessRule( gateName, "MANAGED GATE for " + description + " consent " + (is_present ? "present" : "missing"), @@ -958,9 +1006,16 @@ private AccessRule upsertConsentGate(String gateName, String rule, boolean is_pr ); } - private AccessRule createPhenotypeSubRule(String conceptPath, String alias, String rule, int ruleType, String label, boolean useMapKey) { + protected AccessRule createPhenotypeSubRule(String conceptPath, String alias, String rule, int ruleType, String label, boolean useMapKey) { String ar_name = "AR_PHENO_" + alias + "_" + label; - logger.info("createPhenotypeSubRule() Creating new access rule {}", ar_name); + logger.debug("createPhenotypeSubRule() Creating new access rule {}", ar_name); + + // Check if the conceptPath has `\\\\` present. This technically represents `\\`. + if(conceptPath != null && conceptPath.contains("\\\\")) { + // This will convert all `\\\\` to `\\`. + conceptPath = conceptPath.replaceAll("\\\\\\\\", "\\\\"); + } + return getOrCreateAccessRule( ar_name, "MANAGED SUB AR for " + alias + " " + label + " clinical concepts", @@ -974,11 +1029,11 @@ private AccessRule createPhenotypeSubRule(String conceptPath, String alias, Stri ); } - private AccessRule getOrCreateAccessRule(String name, String description, String rule, int type, String value, boolean checkMapKeyOnly, boolean checkMapNode, boolean evaluateOnlyByGates, boolean gateAnyRelation) { + protected AccessRule getOrCreateAccessRule(String name, String description, String rule, int type, String value, boolean checkMapKeyOnly, boolean checkMapNode, boolean evaluateOnlyByGates, boolean gateAnyRelation) { return accessRuleCache.computeIfAbsent(name, key -> { AccessRule ar = this.getAccessRuleByName(key); if (ar == null) { - logger.info("Creating new access rule {}", key); + logger.debug("Creating new access rule {}", key); ar = new AccessRule(); ar.setName(name); ar.setDescription(description); @@ -989,10 +1044,21 @@ private AccessRule getOrCreateAccessRule(String name, String description, String ar.setCheckMapNode(checkMapNode); ar.setEvaluateOnlyByGates(evaluateOnlyByGates); ar.setGateAnyRelation(gateAnyRelation); - this.save(ar); + ar = this.save(ar); } return ar; }); } + + private String escapePath(String path) { + if (path != null && !path.contains("\\\\")) { + return path.replaceAll("\\\\", "\\\\\\\\"); + } + return path; + } + + public List getAccessRulesByPrivilegeIds(List privilegeIds) { + return this.accessRuleRepo.getAccessRulesByPrivilegeIds(privilegeIds); + } } diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/PrivilegeService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/PrivilegeService.java index c28d40bee..cc02f9cdc 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/PrivilegeService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/PrivilegeService.java @@ -11,6 +11,9 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.event.ApplicationContextEvent; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.context.event.EventListener; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; @@ -65,6 +68,12 @@ private void init() { logger.info("variantAnnotationColumns: {}", variantAnnotationColumns); } + @Transactional + @EventListener(ApplicationContextEvent.class) + protected void onContextRefreshedEvent() { + updateAllPrivilegesOnStartup(); + } + @Transactional public List deletePrivilegeByPrivilegeId(String privilegeId) { Optional privilege = this.privilegeRepository.findById(UUID.fromString(privilegeId)); @@ -208,56 +217,25 @@ private Privilege upsertClinicalPrivilege(String studyIdentifier, String project priv.setApplication(picSureApp); priv.setName(privilegeName); - // Set consent concept path + // In BioData Catalyst this is either \\_harmonized_consent\\_ or \\_consents\\ we need to escape the slashes String consent_concept_path = isHarmonized ? fence_harmonized_consent_group_concept_path : fence_parent_consent_group_concept_path; - if (!consent_concept_path.contains("\\\\")) { - consent_concept_path = consent_concept_path.replaceAll("\\\\", "\\\\\\\\"); - logger.debug("Escaped consent concept path: {}", consent_concept_path); - } - - if (fence_harmonized_concept_path != null && !fence_harmonized_concept_path.contains("\\\\")) { - //these have to be escaped again so that jaxson can convert it correctly - fence_harmonized_concept_path = fence_harmonized_concept_path.replaceAll("\\\\", "\\\\\\\\"); - logger.debug("upsertTopmedPrivilege(): escaped harmonized consent path" + fence_harmonized_concept_path); - } - - - String studyIdentifierField = (consent_group != null && !consent_group.isEmpty()) ? studyIdentifier + "." + consent_group : studyIdentifier; - String queryTemplateText = String.format( - "{\"categoryFilters\": {\"%s\":[\"%s\"]},\"numericFilters\":{},\"requiredFields\":[],\"fields\":[],\"variantInfoFilters\":[{\"categoryVariantInfoFilters\":{},\"numericVariantInfoFilters\":{}}],\"expectedResultType\": \"COUNT\"}", - consent_concept_path, studyIdentifierField - ); + consent_concept_path = escapePath(consent_concept_path); + fence_harmonized_concept_path = escapePath(fence_harmonized_concept_path); - priv.setQueryTemplate(queryTemplateText); + priv.setQueryTemplate(createClinicalQueryTemplate(studyIdentifier, consent_group, consent_concept_path)); priv.setQueryScope(isHarmonized ? String.format("[\"%s\",\"_\",\"%s\"]", conceptPath, fence_harmonized_concept_path) : String.format("[\"%s\",\"_\"]", conceptPath)); - // Initialize the set of AccessRules - Set accessrules = new HashSet<>(); - - // Create and add the parent consent access rule - AccessRule ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consent_group, "PARENT", fence_parent_consent_group_concept_path); - this.accessRuleService.configureAccessRule(ar, studyIdentifier, consent_group, conceptPath, projectAlias, true, false, false); - accessrules.add(ar); - - // Create and add the Topmed+Parent access rule - ar = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consent_group, "TOPMED+PARENT"); - this.accessRuleService.configureAccessRule(ar, studyIdentifier, consent_group, conceptPath, projectAlias, true, false, true); - accessrules.add(ar); - - // If harmonized, create and add the harmonized access rule + Set accessRules = new HashSet<>(); + accessRules.add(createClinicalParentAccessRule(studyIdentifier, consent_group, conceptPath, projectAlias)); + accessRules.add(createClinicalTopmedParentAccessRule(studyIdentifier, consent_group, conceptPath, projectAlias)); if (isHarmonized) { - ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consent_group, "HARMONIZED", fence_harmonized_consent_group_concept_path); - this.accessRuleService.configureHarmonizedAccessRule(ar, studyIdentifier, consent_group, conceptPath, projectAlias); - accessrules.add(ar); + accessRules.add(createClinicalHarmonizedAccessRule(studyIdentifier, consent_group, conceptPath, projectAlias)); } + accessRules.addAll(this.accessRuleService.addStandardAccessRules()); + priv.setAccessRules(accessRules); + logger.info("Added {} access_rules to privilege", accessRules.size()); - // Add standard access rules - this.accessRuleService.addStandardAccessRules(accessrules); - - priv.setAccessRules(accessrules); - logger.info("Added {} access_rules to privilege", accessrules.size()); - - this.save(priv); + priv = this.save(priv); logger.info("Added new privilege {} to DB", priv.getName()); } catch (Exception ex) { logger.error("Could not save privilege", ex); @@ -265,15 +243,41 @@ private Privilege upsertClinicalPrivilege(String studyIdentifier, String project return priv; } + private static String createClinicalQueryTemplate(String studyIdentifier, String consent_group, String consent_concept_path) { + String studyIdentifierField = (consent_group != null && !consent_group.isEmpty()) ? studyIdentifier + "." + consent_group : studyIdentifier; + return String.format( + "{\"categoryFilters\": {\"%s\":[\"%s\"]},\"numericFilters\":{},\"requiredFields\":[],\"fields\":[\"%s\"],\"variantInfoFilters\":[{\"categoryVariantInfoFilters\":{},\"numericVariantInfoFilters\":{}}],\"expectedResultType\": \"COUNT\"}", + consent_concept_path, studyIdentifierField, AccessRuleService.parentAccessionField + ); + } + + private AccessRule createClinicalHarmonizedAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) { + AccessRule ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consentGroup, "HARMONIZED", fence_harmonized_consent_group_concept_path); + this.accessRuleService.configureHarmonizedAccessRule(ar, studyIdentifier, conceptPath, projectAlias); + return this.accessRuleService.save(ar); + } + + private AccessRule createClinicalTopmedParentAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) { + AccessRule ar = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED+PARENT"); + ar = this.accessRuleService.configureClinicalAccessRuleWithPhenoSubRule(ar, studyIdentifier, consentGroup, conceptPath, projectAlias); + return this.accessRuleService.save(ar); + } + + private AccessRule createClinicalParentAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) { + AccessRule ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consentGroup, "PARENT", fence_parent_consent_group_concept_path); + this.accessRuleService.configureAccessRule(ar, studyIdentifier, consentGroup, conceptPath, projectAlias); + return this.accessRuleService.save(ar); + } + /** * Creates a privilege for Topmed access. This has (up to) three access rules: * 1) topmed only 2) topmed + parent 3) topmed + harmonized. * - * @param studyIdentifier - * @param projectAlias - * @param consentGroup - * @param parentConceptPath - * @param isHarmonized + * @param studyIdentifier The study identifier + * @param projectAlias The project alias + * @param consentGroup The consent group + * @param parentConceptPath The parent concept path + * @param isHarmonized Whether the study is harmonized * @return Privilege */ private Privilege upsertTopmedPrivilege(String studyIdentifier, String projectAlias, String consentGroup, String parentConceptPath, boolean isHarmonized) { @@ -291,31 +295,22 @@ private Privilege upsertTopmedPrivilege(String studyIdentifier, String projectAl buildPrivilegeObject(priv, privilegeName, studyIdentifier, consentGroup); Set accessRules = new HashSet<>(); - AccessRule topmedRule = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED"); - - this.accessRuleService.populateAccessRule(topmedRule, false, false, true); - topmedRule.getSubAccessRule().addAll(this.accessRuleService.getPhenotypeRestrictedSubRules(studyIdentifier, consentGroup, projectAlias)); - accessRules.add(topmedRule); + accessRules.add(createTopmedAccessRules(studyIdentifier, projectAlias, consentGroup)); if (parentConceptPath != null) { - AccessRule topmedParentRule = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED+PARENT"); - this.accessRuleService.populateAccessRule(topmedParentRule, true, false, true); - topmedParentRule.getSubAccessRule().addAll(this.accessRuleService.getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias)); - accessRules.add(topmedParentRule); + accessRules.add(createTopmedParentAccessRule(studyIdentifier, consentGroup, parentConceptPath, projectAlias)); if (isHarmonized) { - AccessRule harmonizedRule = this.accessRuleService.upsertHarmonizedAccessRule(studyIdentifier, consentGroup, "HARMONIZED"); - this.accessRuleService.populateHarmonizedAccessRule(harmonizedRule, parentConceptPath, studyIdentifier, projectAlias); - accessRules.add(harmonizedRule); + accessRules.add(createHarmonizedTopmedAccessRule(studyIdentifier, projectAlias, consentGroup, parentConceptPath)); } } - this.accessRuleService.addStandardAccessRules(accessRules); + accessRules.addAll(this.accessRuleService.addStandardAccessRules()); priv.setAccessRules(accessRules); logger.info("upsertTopmedPrivilege() Added {} access_rules to privilege", accessRules.size()); - this.save(priv); + priv = this.save(priv); logger.info("upsertTopmedPrivilege() Added new privilege {} to DB", priv.getName()); } catch (Exception ex) { logger.error("upsertTopmedPrivilege() could not save privilege", ex); @@ -324,6 +319,28 @@ private Privilege upsertTopmedPrivilege(String studyIdentifier, String projectAl return priv; } + private AccessRule createHarmonizedTopmedAccessRule(String studyIdentifier, String projectAlias, String consentGroup, String parentConceptPath) { + AccessRule harmonizedRule = this.accessRuleService.upsertHarmonizedAccessRule(studyIdentifier, consentGroup); + harmonizedRule = this.accessRuleService.populateHarmonizedAccessRule(harmonizedRule, parentConceptPath, studyIdentifier, projectAlias); + this.accessRuleService.save(harmonizedRule); + return harmonizedRule; + } + + private AccessRule createTopmedParentAccessRule(String studyIdentifier, String consentGroup, String parentConceptPath, String projectAlias) { + AccessRule topmedParentRule = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED+PARENT"); + this.accessRuleService.populateTopmedAccessRule(topmedParentRule, true); + topmedParentRule.getSubAccessRule().addAll(this.accessRuleService.getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias)); + topmedParentRule.getSubAccessRule().add(this.accessRuleService.createPhenotypeSubRule(fence_topmed_consent_group_concept_path, "ALLOW_TOPMED_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true)); + return this.accessRuleService.save(topmedParentRule); + } + + private AccessRule createTopmedAccessRules(String studyIdentifier, String projectAlias, String consentGroup) { + AccessRule topmedRule = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED"); + topmedRule = this.accessRuleService.populateTopmedAccessRule(topmedRule, false); + topmedRule.getSubAccessRule().addAll(this.accessRuleService.getPhenotypeRestrictedSubRules(studyIdentifier, consentGroup, projectAlias)); + return this.accessRuleService.save(topmedRule); + } + private void buildPrivilegeObject(Privilege priv, String privilegeName, String studyIdentifier, String consentGroup) { priv.setApplication(picSureApp); priv.setName(privilegeName); @@ -401,4 +418,44 @@ private StudyMetaData getStudyMappingForProjectAndConsent(String projectId, Stri return fenceMapping.get(consentVal); } + + public Optional findById(UUID uuid) { + return privilegeRepository.findById(uuid); + } + + /** + * This method will update all existing privileges with the standard access rules and allowed query types. + * This method will not remove any existing standard access rules If you need to remove a standard access rule, + * you will need to create a migration script. + */ + protected void updateAllPrivilegesOnStartup() { + List privileges = this.getPrivilegesAll(); + Set standardAccessRules = this.accessRuleService.addStandardAccessRules(); + if (standardAccessRules.isEmpty()) { + logger.error("No standard access rules found."); + return; + } else { + privileges.forEach(privilege -> + { + privilege.getAccessRules().addAll(standardAccessRules); + this.save(privilege); + }); + } + + List privilegeIds = privileges.stream().map(Privilege::getUuid).toList(); + List accessRules = this.accessRuleService.getAccessRulesByPrivilegeIds(privilegeIds); + + // find each access rule that has sub access rules that allow query types an update them + accessRules.parallelStream() + .filter(accessRule -> accessRule.getSubAccessRule().stream().anyMatch(subAccessRule -> subAccessRule.getName().startsWith("AR_ALLOW_"))) + .forEach(accessRule -> { + Set subAccessRules = accessRule.getSubAccessRule(); + subAccessRules.removeIf(subAccessRule -> subAccessRule.getName().startsWith("AR_ALLOW_")); + + // Add the currently allowed query types + subAccessRules.addAll(this.accessRuleService.getAllowedQueryTypeRules()); + accessRule.setSubAccessRule(subAccessRules); + this.accessRuleService.save(accessRule); + }); + } } diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleService.java index c8796d8ec..3c81fd2b6 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/RoleService.java @@ -7,7 +7,6 @@ import edu.harvard.hms.dbmi.avillach.auth.model.CustomUserDetails; import edu.harvard.hms.dbmi.avillach.auth.model.fenceMapping.StudyMetaData; import edu.harvard.hms.dbmi.avillach.auth.model.ras.RasDbgapPermission; -import edu.harvard.hms.dbmi.avillach.auth.repository.PrivilegeRepository; import edu.harvard.hms.dbmi.avillach.auth.repository.RoleRepository; import edu.harvard.hms.dbmi.avillach.auth.utils.FenceMappingUtility; import org.apache.commons.lang3.StringUtils; @@ -30,16 +29,15 @@ public class RoleService { private final Logger logger = LoggerFactory.getLogger(RoleService.class); private final RoleRepository roleRepository; - private final PrivilegeRepository privilegeRepo; private final PrivilegeService privilegeService; private final FenceMappingUtility fenceMappingUtility; - public static final String managed_open_access_role_name = "MANAGED_ROLE_OPEN_ACCESS"; + public static final String MANAGED_OPEN_ACCESS_ROLE_NAME = "MANUAL_ROLE_OPEN_ACCESS"; + public static final String MANAGED_ROLE_NAMED_DATASET = "MANUAL_ROLE_NAMED_DATASET"; private final Set publicAccessRoles = new HashSet<>(); @Autowired - public RoleService(RoleRepository roleRepository, PrivilegeRepository privilegeRepo, PrivilegeService privilegeService, FenceMappingUtility fenceMappingUtility) { + public RoleService(RoleRepository roleRepository, PrivilegeService privilegeService, FenceMappingUtility fenceMappingUtility) { this.roleRepository = roleRepository; - this.privilegeRepo = privilegeRepo; this.privilegeService = privilegeService; this.fenceMappingUtility = fenceMappingUtility; } @@ -120,7 +118,7 @@ private void checkPrivilegeAssociation(List roles) throws RuntimeException if (role.getPrivileges() != null) { Set privileges = new HashSet<>(); for (Privilege p : role.getPrivileges()) { - Optional privilege = privilegeRepo.findById(p.getUuid()); + Optional privilege = this.privilegeService.findById(p.getUuid()); if (privilege.isEmpty()) { throw new RuntimeException("Privilege not found - uuid: " + p.getUuid().toString()); } diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java index 6a1d9a696..31c9f5ffb 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/UserService.java @@ -19,7 +19,6 @@ import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jws; import jakarta.mail.MessagingException; -import jakarta.persistence.NoResultException; import jakarta.validation.constraints.NotNull; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -40,7 +39,8 @@ import java.util.*; import java.util.stream.Collectors; -import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.managed_open_access_role_name; +import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.MANAGED_OPEN_ACCESS_ROLE_NAME; +import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.MANAGED_ROLE_NAMED_DATASET; @Service public class UserService { @@ -649,7 +649,7 @@ public User updateUserRoles(User current_user, Set roleNames) { .collect(Collectors.toSet()); Set rolesToRemove = current_user.getRoles().stream() - .filter(role -> !roleNames.contains(role.getName()) && !role.getName().equals(managed_open_access_role_name) + .filter(role -> !roleNames.contains(role.getName()) && !role.getName().equals(MANAGED_OPEN_ACCESS_ROLE_NAME) && !role.getName().startsWith("MANUAL_") && !role.getName().equals("PIC-SURE Top Admin") && !role.getName().equals("Admin")) .collect(Collectors.toSet()); @@ -673,14 +673,18 @@ public User updateUserRoles(User current_user, Set roleNames) { current_user.getRoles().addAll(newRoles); } - final String idp = extractIdp(current_user); - if (!current_user.getRoles().isEmpty() || openAccessIdpValues.contains(idp.toLowerCase())) { - Role openAccessRole = roleService.findByName(managed_open_access_role_name); - if (openAccessRole != null) { - current_user.getRoles().add(openAccessRole); - } else { - logger.warn("Unable to find fence OPEN ACCESS role"); - } + Role openAccessRole = roleService.findByName(MANAGED_OPEN_ACCESS_ROLE_NAME); + if (openAccessRole != null) { + current_user.getRoles().add(openAccessRole); + } else { + logger.warn("Unable to find fence OPEN ACCESS role"); + } + + Role role = roleService.findByName(MANAGED_ROLE_NAMED_DATASET); + if (role != null) { + current_user.getRoles().add(role); + } else { + logger.warn("upsertRole() Unable to find role named {}", MANAGED_ROLE_NAMED_DATASET); } // Every user has access to public datasets by default. diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/AimAheadAuthenticationService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/AimAheadAuthenticationService.java index bd3c23639..736198f08 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/AimAheadAuthenticationService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/AimAheadAuthenticationService.java @@ -20,7 +20,7 @@ import java.util.*; -import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.managed_open_access_role_name; +import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.MANAGED_OPEN_ACCESS_ROLE_NAME; @Service public class AimAheadAuthenticationService extends OktaAuthenticationService implements AuthenticationService { @@ -171,7 +171,7 @@ private User loadUser(JsonNode introspectResponse) { } // All users that login through OKTA should have the fence_open_access role, or they will not be able to interact with the UI - Role fenceOpenAccessRole = roleService.getRoleByName(managed_open_access_role_name); + Role fenceOpenAccessRole = roleService.getRoleByName(MANAGED_OPEN_ACCESS_ROLE_NAME); if (!user.get().getRoles().contains(fenceOpenAccessRole)) { logger.info("Adding fence_open_access role to user: {}", user.get().getUuid()); Set roles = user.get().getRoles(); diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/FENCEAuthenticationService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/FENCEAuthenticationService.java index 95b5b0ef2..8c0542f19 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/FENCEAuthenticationService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/FENCEAuthenticationService.java @@ -3,9 +3,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; import edu.harvard.hms.dbmi.avillach.auth.entity.Connection; -import edu.harvard.hms.dbmi.avillach.auth.entity.Role; import edu.harvard.hms.dbmi.avillach.auth.entity.User; import edu.harvard.hms.dbmi.avillach.auth.exceptions.NotAuthorizedException; import edu.harvard.hms.dbmi.avillach.auth.model.fenceMapping.StudyMetaData; @@ -28,9 +26,6 @@ import org.springframework.util.MultiValueMap; import java.util.*; -import java.util.stream.Collectors; - -import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.managed_open_access_role_name; @Service public class FENCEAuthenticationService implements AuthenticationService { diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/OpenAuthenticationService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/OpenAuthenticationService.java index 6564541cf..dccd63516 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/OpenAuthenticationService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/OpenAuthenticationService.java @@ -16,7 +16,7 @@ import java.util.HashMap; import java.util.Map; -import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.managed_open_access_role_name; +import static edu.harvard.hms.dbmi.avillach.auth.service.impl.RoleService.MANAGED_OPEN_ACCESS_ROLE_NAME; @Service public class OpenAuthenticationService implements AuthenticationService { @@ -53,7 +53,7 @@ public HashMap authenticate(Map authRequest, Str // If we can't find the user by UUID, create a new one if (currentUser == null) { - Role openAccessRole = roleService.getRoleByName(managed_open_access_role_name); + Role openAccessRole = roleService.getRoleByName(MANAGED_OPEN_ACCESS_ROLE_NAME); currentUser = userService.createOpenAccessUser(openAccessRole); //clear some cache entries if we register a new login diff --git a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java index eec92e2ea..d81d4359c 100644 --- a/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java +++ b/pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AuthorizationService.java @@ -147,24 +147,30 @@ public boolean isAuthorized(Application application, Object requestBody, User us if (!this.strictConnections.contains(label)) { Set privileges = user.getPrivilegesByApplication(application); + // List all privileges of the user + logger.info("ACCESS_LOG ___ {},{},{} ___ has the following privileges: {}", user.getUuid().toString(), user.getEmail(), user.getName(), privileges.stream().map(Privilege::getName).collect(Collectors.joining(", "))); if (privileges == null || privileges.isEmpty()) { logger.info("ACCESS_LOG ___ {},{},{} ___ has been denied access to execute query ___ {} ___ in application ___ {} __ USER HAS NO PRIVILEGES ASSOCIATED TO THE APPLICATION, BUT APPLICATION HAS PRIVILEGES", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName); return false; } accessRules = this.accessRuleService.cachedPreProcessAccessRules(user, privileges); - if (accessRules == null || accessRules.isEmpty()) { + logger.info("ACCESS_LOG ___ {},{},{} ___ has the following access rules: {}", user.getUuid().toString(), user.getEmail(), user.getName(), accessRules.stream().map(AccessRule::getName).collect(Collectors.joining(", "))); + if (accessRules.isEmpty()) { logger.info("ACCESS_LOG ___ {},{},{} ___ has been granted access to execute query ___ {} ___ in application ___ {} ___ NO ACCESS RULES EVALUATED", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName); return true; } } else { accessRules = this.accessRuleService.getAccessRulesForUserAndApp(user, application); - if (accessRules == null || accessRules.isEmpty()) { + + logger.info("ACCESS_LOG ___ {},{},{} ___ has the following access rules: {}", user.getUuid().toString(), user.getEmail(), user.getName(), accessRules.stream().map(AccessRule::toString).collect(Collectors.joining(", "))); + if (accessRules.isEmpty()) { logger.info("ACCESS_LOG ___ {},{},{} ___ has been denied access to execute query ___ {} ___ in application ___ {} ___ NO ACCESS RULES EVALUATED", user.getUuid().toString(), user.getEmail(), user.getName(), formattedQuery, applicationName); return false; } } + logger.debug("Request: {}", requestBody); // Current logic here is: among all accessRules, they are OR relationship Set failedRules = new HashSet<>(); AccessRule passByRule = null; @@ -187,10 +193,10 @@ public boolean isAuthorized(Application application, Object requestBody, User us passRuleName = passByRule.getMergedName(); } - logger.debug("ACCESS_LOG ___ {},{},{} ___ has been {} access to execute query ___ {} ___ in application ___ {} ___ {}", user.getUuid().toString(), user.getEmail(), user.getName(), result ? "granted" : "denied", formattedQuery, applicationName, result ? "passed by " + passRuleName : "failed by rules: [" - + failedRules.stream() - .map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName())) - .collect(Collectors.joining(", ")) + "]"); + logger.info("ACCESS_LOG ___ {},{},{} ___ has been {} access to execute query ___ {} ___ in application ___ {} ___ {}", user.getUuid().toString(), user.getEmail(), user.getName(), result ? "granted" : "denied", formattedQuery, applicationName, result ? "passed by " + passRuleName : "failed by rules: [" + + failedRules.stream() + .map(ar -> (ar.getMergedName().isEmpty() ? ar.getName() : ar.getMergedName())) + .collect(Collectors.joining(", ")) + "]"); return result; } diff --git a/pic-sure-auth-services/src/main/resources/application.properties b/pic-sure-auth-services/src/main/resources/application.properties index af0c159ee..60648570e 100644 --- a/pic-sure-auth-services/src/main/resources/application.properties +++ b/pic-sure-auth-services/src/main/resources/application.properties @@ -22,6 +22,7 @@ spring.jpa.hibernate.naming.physical-strategy=org.hibernate.boot.model.naming.Ph # Logging #logging.level.org.springframework.security=info #logging.level.root=DEBUG +logging.level.edu.harvard.hms.dbmi.avillach.auth.service.impl=DEBUG # Mail session configuration (Assuming Gmail SMTP for example) spring.mail.host=smtp.gmail.com @@ -44,7 +45,9 @@ application.admin.users=${ADMIN_USERS:__ADMIN_USERS__} # If you intend to use a fence_mapping.json file you will need to set the following properties. fence.consent.group.concept.path=\\DCC Harmonized data set\\ fence.standard.access.rules=AR_ONLY_INFO,AR_ONLY_SEARCH,AR_INFO_COLUMN_LISTING -fence.allowed.query.types=COUNT,CROSS_COUNT,CATEGORICAL_CROSS_COUNT,CONTINUOUS_CROSS_COUNT,DATAFRAME +fence.allowed.query.types=${FENCE_ALLOWED_QUERY_TYPES:COUNT,CROSS_COUNT,CATEGORICAL_CROSS_COUNT,CONTINUOUS_CROSS_COUNT,DATAFRAME} +#COUNT,CROSS_COUNT,CATEGORICAL_CROSS_COUNT,CONTINUOUS_CROSS_COUNT,DATAFRAME,DATAFRAME_PFB + fence.harmonized.consent.group.concept.path=\\_harmonized_consent\\ fence.parent.consent.group.concept.path=\\_consents\\ fence.topmed.consent.group.concept.path=\\_topmed_consents\\ @@ -60,10 +63,6 @@ application.client.secret=${APPLICATION_CLIENT_SECRET} application.client.secret.base64=${APPLICATION_CLIENT_SECRET_IS_BASE_64:false} application.user.id.claim=${USER_ID_CLAIM:sub} -# Token Expiration -#application.token.expiration.time=${TOKEN_EXPIRATION_TIME:300000} -#application.max.session.length=${MAX_SESSION_TIME:600000} - # IDLE Timeout 15 minutes by default application.token.expiration.time=${TOKEN_EXPIRATION_TIME:900000} # Max session length 8 hours by default diff --git a/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/RASAuthenticationServiceTest.java b/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/RASAuthenticationServiceTest.java index 7a82bbb54..cdb05250b 100644 --- a/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/RASAuthenticationServiceTest.java +++ b/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authentication/RASAuthenticationServiceTest.java @@ -45,7 +45,7 @@ public class RASAuthenticationServiceTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - RoleService roleService = new RoleService(mock(RoleRepository.class), mock(PrivilegeRepository.class), mock(PrivilegeService.class), mock(FenceMappingUtility.class)); + RoleService roleService = new RoleService(mock(RoleRepository.class), mock(PrivilegeService.class), mock(FenceMappingUtility.class)); this.rasPassPortService = spy(new RASPassPortService(restClientUtil, userService, "")); doReturn(false).when(rasPassPortService).isExpired(any()); diff --git a/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AccessRuleServiceTest.java b/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AccessRuleServiceTest.java deleted file mode 100644 index 5cd22f2b4..000000000 --- a/pic-sure-auth-services/src/test/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/authorization/AccessRuleServiceTest.java +++ /dev/null @@ -1,129 +0,0 @@ -package edu.harvard.hms.dbmi.avillach.auth.service.impl.authorization; - -import edu.harvard.hms.dbmi.avillach.auth.entity.AccessRule; -import edu.harvard.hms.dbmi.avillach.auth.repository.AccessRuleRepository; -import edu.harvard.hms.dbmi.avillach.auth.service.impl.AccessRuleService; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import java.util.*; - -import static org.junit.Assert.*; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.when; - -public class AccessRuleServiceTest { - - @Mock - private AccessRuleRepository accessRuleRepo; - - @InjectMocks - private AccessRuleService accessRuleService; - - @Before - public void setUp() { - MockitoAnnotations.initMocks(this); - } - - @After - public void tearDown() throws Exception { - } - - @Test - public void testGetAccessRuleById_found() { - UUID id = UUID.randomUUID(); - AccessRule accessRule = new AccessRule(); - when(accessRuleRepo.findById(id)).thenReturn(Optional.of(accessRule)); - - Optional result = accessRuleService.getAccessRuleById(id.toString()); - assertTrue(result.isPresent()); - Assert.assertSame(accessRule, result.get()); - } - - @Test - public void testGetAccessRuleById_notFound() { - UUID id = UUID.randomUUID(); - when(accessRuleRepo.findById(id)).thenReturn(Optional.empty()); - - Optional result = accessRuleService.getAccessRuleById(id.toString()); - assertFalse(result.isPresent()); - } - - @Test - public void testGetAllAccessRules_empty() { - when(accessRuleRepo.findAll()).thenReturn(Collections.emptyList()); - - List result = accessRuleService.getAllAccessRules(); - assertTrue(result.isEmpty()); - } - - @Test - public void testGetAllAccessRules_nonEmpty() { - List rules = Arrays.asList(new AccessRule(), new AccessRule()); - when(accessRuleRepo.findAll()).thenReturn(rules); - - List result = accessRuleService.getAllAccessRules(); - assertEquals(2, result.size()); - } - - - @Test - public void testAddAccessRule_withNullFields() { - AccessRule rule = new AccessRule(); // fields are null - List rules = Collections.singletonList(rule); - when(accessRuleRepo.saveAll(anyList())).thenAnswer(invocation -> invocation.getArgument(0)); - - List result = accessRuleService.addAccessRule(rules); - assertFalse(result.getFirst().getEvaluateOnlyByGates()); - assertFalse(result.getFirst().getCheckMapKeyOnly()); - assertFalse(result.getFirst().getCheckMapNode()); - assertFalse(result.getFirst().getGateAnyRelation()); - } - - @Test - public void testAddAccessRule_noNullFields() { - AccessRule rule = new AccessRule(); - rule.setEvaluateOnlyByGates(true); - rule.setCheckMapKeyOnly(true); - rule.setCheckMapNode(true); - rule.setGateAnyRelation(true); - List rules = Collections.singletonList(rule); - when(accessRuleRepo.saveAll(anyList())).thenReturn(rules); - - List result = accessRuleService.addAccessRule(rules); - assertTrue(result.getFirst().getEvaluateOnlyByGates()); - assertTrue(result.getFirst().getCheckMapKeyOnly()); - assertTrue(result.getFirst().getCheckMapNode()); - assertTrue(result.getFirst().getGateAnyRelation()); - } - - - @Test - public void testUpdateAccessRules() { - AccessRule rule = new AccessRule(); - List rules = Collections.singletonList(rule); - when(accessRuleRepo.saveAll(anyList())).thenReturn(rules); - - List result = accessRuleService.updateAccessRules(rules); - assertSame(rules, result); - } - - - @Test - public void testRemoveAccessRuleById() { - UUID id = UUID.randomUUID(); - List remainingRules = List.of(new AccessRule()); - doNothing().when(accessRuleRepo).deleteById(id); - when(accessRuleRepo.findAll()).thenReturn(remainingRules); - - List result = accessRuleService.removeAccessRuleById(id.toString()); - assertEquals(1, result.size()); - } - -} \ No newline at end of file