Skip to content

Commit

Permalink
[ALS-7197] BDC Auth access: User able to access data (#203)
Browse files Browse the repository at this point in the history
* - 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
  • Loading branch information
Gcolon021 committed Sep 9, 2024
1 parent 9838b55 commit a63a0bd
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 286 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -16,4 +20,9 @@ public interface AccessRuleRepository extends JpaRepository<AccessRule, UUID> {

AccessRule findByName(String name);

@Query("SELECT p.accessRules FROM privilege p WHERE p.uuid = :uuid")
Set<AccessRule> findAccessRulesByPrivilegeId(@Param("uuid") UUID uuid);

@Query("SELECT p.accessRules FROM privilege p WHERE p.uuid IN :privilegeIds")
List<AccessRule> getAccessRulesByPrivilegeIds(@Param("privilegeIds") List<UUID> privilegeIds);
}
Original file line number Diff line number Diff line change
@@ -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;

/**
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,6 +68,12 @@ private void init() {
logger.info("variantAnnotationColumns: {}", variantAnnotationColumns);
}

@Transactional
@EventListener(ApplicationContextEvent.class)
protected void onContextRefreshedEvent() {
updateAllPrivilegesOnStartup();
}

@Transactional
public List<Privilege> deletePrivilegeByPrivilegeId(String privilegeId) {
Optional<Privilege> privilege = this.privilegeRepository.findById(UUID.fromString(privilegeId));
Expand Down Expand Up @@ -208,72 +217,67 @@ 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<AccessRule> 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<AccessRule> 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);
}
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) {
Expand All @@ -291,31 +295,22 @@ private Privilege upsertTopmedPrivilege(String studyIdentifier, String projectAl
buildPrivilegeObject(priv, privilegeName, studyIdentifier, consentGroup);

Set<AccessRule> 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);
Expand All @@ -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);
Expand Down Expand Up @@ -401,4 +418,44 @@ private StudyMetaData getStudyMappingForProjectAndConsent(String projectId, Stri

return fenceMapping.get(consentVal);
}

public Optional<Privilege> 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<Privilege> privileges = this.getPrivilegesAll();
Set<AccessRule> 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<UUID> privilegeIds = privileges.stream().map(Privilege::getUuid).toList();
List<AccessRule> 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<AccessRule> 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);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Role> 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;
}
Expand Down Expand Up @@ -120,7 +118,7 @@ private void checkPrivilegeAssociation(List<Role> roles) throws RuntimeException
if (role.getPrivileges() != null) {
Set<Privilege> privileges = new HashSet<>();
for (Privilege p : role.getPrivileges()) {
Optional<Privilege> privilege = privilegeRepo.findById(p.getUuid());
Optional<Privilege> privilege = this.privilegeService.findById(p.getUuid());
if (privilege.isEmpty()) {
throw new RuntimeException("Privilege not found - uuid: " + p.getUuid().toString());
}
Expand Down
Loading

0 comments on commit a63a0bd

Please sign in to comment.