Skip to content

Commit be9d2d9

Browse files
Gcolon021Gcolon021
authored
[ALS-9219] Clean up roles on startup (#258)
* Refactor role cleanup and privilege logic Implemented cleanupRolesNotInMapping in RoleService to remove roles not present in the given mapping and delete associated user-role relationships. Enhanced privilege configuration with better rule initialization and optimized gate-setting logic. Updated logging levels and replaced redundant code blocks for clarity. * Clear privileges before deleting roles in cleanupRolesNotInMapping * Update role cleanup logic to use managedRoles and improve logging
1 parent 6ea0a75 commit be9d2d9

File tree

8 files changed

+114
-50
lines changed

8 files changed

+114
-50
lines changed

‎pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/RoleRepository.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,6 @@ public interface RoleRepository extends JpaRepository<Role, UUID> {
2121
Set<Role> findByUuidIn(Set<UUID> uuids);
2222

2323
Set<Role> findByNameIn(Set<String> names);
24+
25+
Set<Role> findByNameNotIn(Set<String> names);
2426
}

‎pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/repository/UserRepository.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@
44
import edu.harvard.hms.dbmi.avillach.auth.entity.User;
55
import org.springframework.data.jpa.repository.EntityGraph;
66
import org.springframework.data.jpa.repository.JpaRepository;
7+
import org.springframework.data.jpa.repository.Modifying;
8+
import org.springframework.data.jpa.repository.Query;
79
import org.springframework.data.repository.PagingAndSortingRepository;
10+
import org.springframework.data.repository.query.Param;
811
import org.springframework.stereotype.Repository;
12+
import org.springframework.transaction.annotation.Transactional;
913

1014
import java.util.List;
1115
import java.util.Optional;
@@ -39,4 +43,9 @@ public interface UserRepository extends JpaRepository<User, UUID> {
3943

4044
Set<User> findByPassportIsNotNull();
4145

46+
@Modifying
47+
@Query(value = "DELETE FROM user_role where role_id in (:roleIDs)", nativeQuery = true)
48+
@Transactional
49+
void deleteUserRoles(@Param("roleIDs") List<UUID> roleIDs);
50+
4251
}

‎pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/AccessRuleService.java

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ public boolean extractAndCheckRule(AccessRule accessRule, Object parsedRequestBo
383383
return true;
384384
}
385385

386-
if(accessRuleType == AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY ||
387-
accessRuleType == AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY_IGNORE_CASE) {
386+
if (accessRuleType == AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY ||
387+
accessRuleType == AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY_IGNORE_CASE) {
388388
logger.debug("extractAndCheckRule() -> JsonPath.parse().read() PathNotFound; passing rule {} for type {}", rule, accessRuleType);
389389
return true;
390390
}
@@ -564,14 +564,20 @@ private boolean _decisionMaker(AccessRule accessRule, String requestBodyValue, S
564564

565565
return switch (accessRule.getType()) {
566566
case AccessRule.TypeNaming.NOT_CONTAINS -> !requestBodyValue.contains(value);
567-
case AccessRule.TypeNaming.NOT_CONTAINS_IGNORE_CASE -> !requestBodyValue.toLowerCase().contains(value.toLowerCase());
567+
case AccessRule.TypeNaming.NOT_CONTAINS_IGNORE_CASE ->
568+
!requestBodyValue.toLowerCase().contains(value.toLowerCase());
568569
case (AccessRule.TypeNaming.NOT_EQUALS) -> !value.equals(requestBodyValue);
569-
case (AccessRule.TypeNaming.ANY_EQUALS), (AccessRule.TypeNaming.ALL_EQUALS) -> value.equals(requestBodyValue);
570-
case (AccessRule.TypeNaming.ALL_CONTAINS), (AccessRule.TypeNaming.ANY_CONTAINS), (AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY) -> requestBodyValue.contains(value);
571-
case (AccessRule.TypeNaming.ALL_CONTAINS_IGNORE_CASE), (AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY_IGNORE_CASE) -> requestBodyValue.toLowerCase().contains(value.toLowerCase());
570+
case (AccessRule.TypeNaming.ANY_EQUALS), (AccessRule.TypeNaming.ALL_EQUALS) ->
571+
value.equals(requestBodyValue);
572+
case (AccessRule.TypeNaming.ALL_CONTAINS), (AccessRule.TypeNaming.ANY_CONTAINS),
573+
(AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY) -> requestBodyValue.contains(value);
574+
case (AccessRule.TypeNaming.ALL_CONTAINS_IGNORE_CASE),
575+
(AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY_IGNORE_CASE) ->
576+
requestBodyValue.toLowerCase().contains(value.toLowerCase());
572577
case (AccessRule.TypeNaming.NOT_EQUALS_IGNORE_CASE) -> !value.equalsIgnoreCase(requestBodyValue);
573578
case (AccessRule.TypeNaming.ALL_EQUALS_IGNORE_CASE) -> value.equalsIgnoreCase(requestBodyValue);
574-
case (AccessRule.TypeNaming.ALL_REG_MATCH), (AccessRule.TypeNaming.ANY_REG_MATCH) -> requestBodyValue.matches(value);
579+
case (AccessRule.TypeNaming.ALL_REG_MATCH), (AccessRule.TypeNaming.ANY_REG_MATCH) ->
580+
requestBodyValue.matches(value);
575581
default -> {
576582
logger.warn("evaluateAccessRule() incoming accessRule type is out of scope. Just return true.");
577583
yield true;
@@ -589,12 +595,8 @@ private boolean _decisionMaker(AccessRule accessRule, String requestBodyValue, S
589595
* @param projectAlias The project alias.
590596
*/
591597
protected void configureAccessRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) {
592-
ar.setGates(new HashSet<>());
593-
ar.getGates().addAll(getGates(true, false, false));
598+
ar.setGates(new HashSet<>(getGates(true, false, false)));
594599

595-
if (ar.getSubAccessRule() == null) {
596-
ar.setSubAccessRule(new HashSet<>());
597-
}
598600
addUniqueSubRules(ar, getAllowedQueryTypeRules());
599601
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
600602
addUniqueSubRules(ar, getTopmedRestrictedSubRules());
@@ -610,25 +612,15 @@ protected void configureAccessRule(AccessRule ar, String studyIdentifier, String
610612
* @param projectAlias The project alias.
611613
*/
612614
protected void configureHarmonizedAccessRule(AccessRule ar, String studyIdentifier, String conceptPath, String projectAlias) {
613-
ar.setGates(new HashSet<>());
614-
ar.getGates().add(upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data"));
615-
616-
if (ar.getSubAccessRule() == null) {
617-
ar.setSubAccessRule(new HashSet<>());
618-
}
615+
ar.setGates(new HashSet<>(Collections.singleton(upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data"))));
619616

620617
addUniqueSubRules(ar, getAllowedQueryTypeRules());
621618
addUniqueSubRules(ar, getHarmonizedSubRules());
622619
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
623620
}
624621

625622
protected AccessRule configureClinicalAccessRuleWithPhenoSubRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) {
626-
ar.setGates(new HashSet<>());
627-
ar.getGates().addAll(getGates(true, false, true));
628-
629-
if (ar.getSubAccessRule() == null) {
630-
ar.setSubAccessRule(new HashSet<>());
631-
}
623+
ar.setGates(new HashSet<>(getGates(true, false, true)));
632624

633625
addUniqueSubRules(ar, getAllowedQueryTypeRules());
634626
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
@@ -705,7 +697,7 @@ private AccessRule upsertTopmedRestrictedSubRule(String type, String rule) {
705697
AccessRule ar = this.getAccessRuleByName(ar_name);
706698
if (ar != null) {
707699
// Log and return the existing rule
708-
logger.debug("Found existing rule: {}", ar.getName());
700+
logger.trace("Found existing rule: {}", ar.getName());
709701
return ar;
710702
}
711703

@@ -823,12 +815,7 @@ private Collection<? extends AccessRule> getGates(boolean parent, boolean harmon
823815
}
824816

825817
protected AccessRule populateTopmedAccessRule(AccessRule rule, boolean includeParent) {
826-
if (rule.getGates() == null) {
827-
rule.setGates(new HashSet<>(getGates(includeParent, false, true)));
828-
} else {
829-
rule.getGates().addAll(getGates(includeParent, false, true));
830-
}
831-
818+
rule.setGates(new HashSet<>(getGates(includeParent, false, true)));
832819
addUniqueSubRules(rule, getAllowedQueryTypeRules());
833820

834821
return rule;
@@ -955,7 +942,7 @@ protected AccessRule upsertTopmedAccessRule(String project_name, String consent_
955942
*/
956943
protected AccessRule upsertHarmonizedAccessRule(String project_name, String consent_group) {
957944
String ar_name = "AR_TOPMED_" + project_name + "_" + consent_group + "_" + "HARMONIZED";
958-
logger.debug("upsertHarmonizedAccessRule() Creating new access rule {}", ar_name);
945+
logger.trace("upsertHarmonizedAccessRule() Creating new access rule {}", ar_name);
959946
String description = "MANAGED AR for " + project_name + "." + consent_group + " Topmed data";
960947
String ruleText = "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]";
961948
String arValue = project_name + "." + consent_group;
@@ -1004,7 +991,7 @@ private AccessRule upsertConsentGate(String gateName, String rule, boolean is_pr
1004991

1005992
protected AccessRule createPhenotypeSubRule(String conceptPath, String alias, String rule, int ruleType, String label, boolean useMapKey) {
1006993
String ar_name = "AR_PHENO_" + alias + "_" + label;
1007-
logger.debug("createPhenotypeSubRule() Creating new access rule {}", ar_name);
994+
logger.trace("createPhenotypeSubRule() Creating new access rule {}", ar_name);
1008995

1009996
// Check if the conceptPath has `\\\\` present. This technically represents `\\`.
1010997
if (conceptPath != null && conceptPath.contains("\\\\")) {
@@ -1029,8 +1016,9 @@ protected AccessRule getOrCreateAccessRule(String name, String description, Stri
10291016
return accessRuleCache.computeIfAbsent(name, key -> {
10301017
AccessRule ar = this.getAccessRuleByName(key);
10311018
if (ar == null) {
1032-
logger.debug("Creating new access rule {}", key);
1019+
logger.trace("Creating new access rule {}", key);
10331020
ar = new AccessRule();
1021+
10341022
ar.setName(name);
10351023
ar.setDescription(description);
10361024
ar.setRule(rule);
@@ -1062,7 +1050,7 @@ public List<AccessRule> getAccessRulesByPrivilegeIds(List<UUID> privilegeIds) {
10621050
* Adds unique sub-rules to the provided parent access rule. This method ensures that duplicate sub-rules,
10631051
* based on their names, are not added to the parent access rule.
10641052
*
1065-
* @param accessRule the parent access rule to which the sub-rules are added
1053+
* @param accessRule the parent access rule to which the sub-rules are added
10661054
* @param subRulesToAdd the collection of sub-rules to be added to the parent access rule
10671055
*/
10681056
private void addUniqueSubRules(AccessRule accessRule, Collection<? extends AccessRule> subRulesToAdd) {

‎pic-sure-auth-services/src/main/java/edu/harvard/hms/dbmi/avillach/auth/service/impl/PrivilegeService.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public Privilege save(Privilege privilege) {
122122

123123
public Set<Privilege> addPrivileges(Role r, Map<String, StudyMetaData> fenceMapping) {
124124
String roleName = r.getName();
125-
logger.info("addFENCEPrivileges() starting, adding privilege(s) to role {}", roleName);
125+
logger.debug("addPrivilege() starting, adding privilege(s) to role {}", roleName);
126126

127127
//each project can have up to three privileges: Parent | Harmonized | Topmed
128128
//harmonized has 2 ARs for parent + harmonized and harmonized only
@@ -135,13 +135,13 @@ public Set<Privilege> addPrivileges(Role r, Map<String, StudyMetaData> fenceMapp
135135
//e.g. MANAGED_phs0000xx_c2 or MANAGED_tutorial-biolinc_camp
136136
String project_name = extractProject(roleName);
137137
if (project_name.isEmpty()) {
138-
logger.warn("addFENCEPrivileges() role name: {} returned an empty project name", roleName);
138+
logger.warn("addPrivileges() role name: {} returned an empty project name", roleName);
139139
}
140140
String consent_group = extractConsentGroup(roleName);
141141
if (!consent_group.isEmpty()) {
142-
logger.warn("addFENCEPrivileges() role name: {} returned an empty consent group", roleName);
142+
logger.warn("addPrivileges() role name: {} returned an empty consent group", roleName);
143143
}
144-
logger.info("addFENCEPrivileges() project name: {} consent group: {}", project_name, consent_group);
144+
logger.debug("addPrivileges() project name: {} consent group: {}", project_name, consent_group);
145145

146146
// Look up the metadata by consent group.
147147
StudyMetaData projectMetadata = getStudyMappingForProjectAndConsent(project_name, consent_group, fenceMapping);
@@ -170,7 +170,7 @@ public Set<Privilege> addPrivileges(Role r, Map<String, StudyMetaData> fenceMapp
170170

171171
if (dataType != null && dataType.contains("P")) {
172172
//insert clinical privs
173-
logger.info("addPrivileges() project:{} consent_group:{} concept_path:{}", project_name, consent_group, concept_path);
173+
logger.debug("addPrivileges() project:{} consent_group:{} concept_path:{}", project_name, consent_group, concept_path);
174174
privs.add(upsertClinicalPrivilege(project_name, projectAlias, consent_group, concept_path, false));
175175

176176
//if harmonized study, also create harmonized privileges
@@ -231,7 +231,6 @@ private Privilege upsertClinicalPrivilege(String studyIdentifier, String project
231231
}
232232
accessRules.addAll(this.accessRuleService.addStandardAccessRules());
233233
priv.setAccessRules(accessRules);
234-
logger.info("Added {} access_rules to privilege", accessRules.size());
235234

236235
priv = this.save(priv);
237236
logger.info("Added new privilege {} to DB", priv.getName());
@@ -251,18 +250,21 @@ private static String createClinicalQueryTemplate(String studyIdentifier, String
251250

252251
private AccessRule createClinicalHarmonizedAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) {
253252
AccessRule ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consentGroup, "HARMONIZED", fence_harmonized_consent_group_concept_path);
253+
ar.setSubAccessRule(new HashSet<>());
254254
this.accessRuleService.configureHarmonizedAccessRule(ar, studyIdentifier, conceptPath, projectAlias);
255255
return this.accessRuleService.save(ar);
256256
}
257257

258258
private AccessRule createClinicalTopmedParentAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) {
259259
AccessRule ar = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED+PARENT");
260+
ar.setSubAccessRule(new HashSet<>());
260261
ar = this.accessRuleService.configureClinicalAccessRuleWithPhenoSubRule(ar, studyIdentifier, consentGroup, conceptPath, projectAlias);
261262
return this.accessRuleService.save(ar);
262263
}
263264

264265
private AccessRule createClinicalParentAccessRule(String studyIdentifier, String consentGroup, String conceptPath, String projectAlias) {
265266
AccessRule ar = this.accessRuleService.createConsentAccessRule(studyIdentifier, consentGroup, "PARENT", fence_parent_consent_group_concept_path);
267+
ar.setSubAccessRule(new HashSet<>());
266268
this.accessRuleService.configureAccessRule(ar, studyIdentifier, consentGroup, conceptPath, projectAlias);
267269
return this.accessRuleService.save(ar);
268270
}
@@ -316,13 +318,15 @@ private Privilege upsertTopmedPrivilege(String studyIdentifier, String projectAl
316318

317319
private AccessRule createHarmonizedTopmedAccessRule(String studyIdentifier, String projectAlias, String consentGroup, String parentConceptPath) {
318320
AccessRule harmonizedRule = this.accessRuleService.upsertHarmonizedAccessRule(studyIdentifier, consentGroup);
321+
harmonizedRule.setSubAccessRule(new HashSet<>());
319322
harmonizedRule = this.accessRuleService.populateHarmonizedAccessRule(harmonizedRule, parentConceptPath, studyIdentifier, projectAlias);
320323
this.accessRuleService.save(harmonizedRule);
321324
return harmonizedRule;
322325
}
323326

324327
private AccessRule createTopmedParentAccessRule(String studyIdentifier, String consentGroup, String parentConceptPath, String projectAlias) {
325328
AccessRule topmedParentRule = this.accessRuleService.upsertTopmedAccessRule(studyIdentifier, consentGroup, "TOPMED+PARENT");
329+
topmedParentRule.setSubAccessRule(new HashSet<>());
326330
this.accessRuleService.populateTopmedAccessRule(topmedParentRule, true);
327331
topmedParentRule.getSubAccessRule().addAll(this.accessRuleService.getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias));
328332
topmedParentRule.getSubAccessRule().add(this.accessRuleService.createPhenotypeSubRule(fence_topmed_consent_group_concept_path, "ALLOW_TOPMED_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true));
@@ -411,7 +415,7 @@ private static String extractConsentGroup(String roleName) {
411415

412416
private StudyMetaData getStudyMappingForProjectAndConsent(String projectId, String consent_group, Map<String, StudyMetaData> fenceMapping) {
413417
String consentVal = (consent_group != null && !consent_group.isEmpty()) ? projectId + "." + consent_group : projectId;
414-
logger.info("getFENCEMappingforProjectAndConsent() looking up {}", consentVal);
418+
logger.debug("getStudyMappingForProjectAndConsent() looking up {}", consentVal);
415419

416420
return fenceMapping.get(consentVal);
417421
}

0 commit comments

Comments
 (0)