Skip to content

Commit 026461e

Browse files
Gcolon021Gcolon021
authored
[ALS-9152] BDC: anyRecordOf and anyRecordOfMulti Access Rules (#253)
1 parent 81b52da commit 026461e

File tree

6 files changed

+240
-76
lines changed

6 files changed

+240
-76
lines changed

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

Lines changed: 73 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.util.*;
2424
import java.util.concurrent.ConcurrentHashMap;
25+
import java.util.stream.Collectors;
2526

2627
@Service
2728
public class AccessRuleService {
@@ -386,11 +387,11 @@ public boolean extractAndCheckRule(AccessRule accessRule, Object parsedRequestBo
386387
}
387388

388389
if (accessRuleType == AccessRule.TypeNaming.IS_EMPTY
389-
|| accessRuleType == AccessRule.TypeNaming.IS_NOT_EMPTY) {
390+
|| accessRuleType == AccessRule.TypeNaming.IS_NOT_EMPTY) {
390391
if (requestBodyValue == null
391-
|| (requestBodyValue instanceof String && ((String) requestBodyValue).isEmpty())
392-
|| (requestBodyValue instanceof Collection && ((Collection) requestBodyValue).isEmpty())
393-
|| (requestBodyValue instanceof Map && ((Map) requestBodyValue).isEmpty())) {
392+
|| (requestBodyValue instanceof String && ((String) requestBodyValue).isEmpty())
393+
|| (requestBodyValue instanceof Collection && ((Collection) requestBodyValue).isEmpty())
394+
|| (requestBodyValue instanceof Map && ((Map) requestBodyValue).isEmpty())) {
394395
return accessRuleType == AccessRule.TypeNaming.IS_EMPTY;
395396
} else {
396397
return accessRuleType == AccessRule.TypeNaming.IS_NOT_EMPTY;
@@ -428,7 +429,7 @@ private boolean evaluateMap(Object requestBodyValue, AccessRule accessRule) {
428429
return true;
429430

430431
if ((accessRule.getCheckMapKeyOnly() == null || !accessRule.getCheckMapKeyOnly())
431-
&& evaluateNode(entry.getValue(), accessRule))
432+
&& evaluateNode(entry.getValue(), accessRule))
432433
return true;
433434
}
434435
return false;
@@ -451,7 +452,7 @@ && evaluateNode(entry.getValue(), accessRule))
451452
return false;
452453

453454
if ((accessRule.getCheckMapKeyOnly() == null || !accessRule.getCheckMapKeyOnly())
454-
&& !evaluateNode(entry.getValue(), accessRule))
455+
&& !evaluateNode(entry.getValue(), accessRule))
455456
return false;
456457
}
457458

@@ -581,55 +582,51 @@ private boolean _decisionMaker(AccessRule accessRule, String requestBodyValue, S
581582
* @param projectAlias The project alias.
582583
*/
583584
protected void configureAccessRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) {
584-
if (ar.getGates() == null) {
585-
ar.setGates(new HashSet<>());
586-
ar.getGates().addAll(getGates(true, false, false));
585+
ar.setGates(new HashSet<>());
586+
ar.getGates().addAll(getGates(true, false, false));
587587

588-
if (ar.getSubAccessRule() == null) {
589-
ar.setSubAccessRule(new HashSet<>());
590-
}
591-
ar.getSubAccessRule().addAll(getAllowedQueryTypeRules());
592-
ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
593-
ar.getSubAccessRule().addAll(getTopmedRestrictedSubRules());
588+
if (ar.getSubAccessRule() == null) {
589+
ar.setSubAccessRule(new HashSet<>());
594590
}
591+
addUniqueSubRules(ar, getAllowedQueryTypeRules());
592+
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
593+
addUniqueSubRules(ar, getTopmedRestrictedSubRules());
595594
}
596595

596+
597597
/**
598598
* Configures the harmonized AccessRule with gates and sub-rules.
599599
*
600600
* @param ar The AccessRule to configure.
601601
* @param studyIdentifier The study identifier.
602602
* @param conceptPath The concept path.
603603
* @param projectAlias The project alias.
604-
* @return
605604
*/
606605
protected void configureHarmonizedAccessRule(AccessRule ar, String studyIdentifier, String conceptPath, String projectAlias) {
607-
if (ar.getGates() == null) {
608-
ar.setGates(new HashSet<>());
609-
ar.getGates().add(upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data"));
606+
ar.setGates(new HashSet<>());
607+
ar.getGates().add(upsertConsentGate("HARMONIZED_CONSENT", "$.query.query.categoryFilters." + fence_harmonized_consent_group_concept_path + "[*]", true, "harmonized data"));
610608

611-
if (ar.getSubAccessRule() == null) {
612-
ar.setSubAccessRule(new HashSet<>());
613-
}
614-
ar.getSubAccessRule().addAll(getAllowedQueryTypeRules());
615-
ar.getSubAccessRule().addAll(getHarmonizedSubRules());
616-
ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
609+
if (ar.getSubAccessRule() == null) {
610+
ar.setSubAccessRule(new HashSet<>());
617611
}
612+
613+
addUniqueSubRules(ar, getAllowedQueryTypeRules());
614+
addUniqueSubRules(ar, getHarmonizedSubRules());
615+
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
618616
}
619617

620618
protected AccessRule configureClinicalAccessRuleWithPhenoSubRule(AccessRule ar, String studyIdentifier, String consent_group, String conceptPath, String projectAlias) {
621-
if (ar.getGates() == null) {
622-
ar.setGates(new HashSet<>());
623-
ar.getGates().addAll(getGates(true, false, true));
619+
ar.setGates(new HashSet<>());
620+
ar.getGates().addAll(getGates(true, false, true));
624621

625-
if (ar.getSubAccessRule() == null) {
626-
ar.setSubAccessRule(new HashSet<>());
627-
}
628-
ar.getSubAccessRule().addAll(getAllowedQueryTypeRules());
629-
ar.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
630-
ar.getSubAccessRule().add(createPhenotypeSubRule(fence_topmed_consent_group_concept_path, "ALLOW_TOPMED_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true));
622+
if (ar.getSubAccessRule() == null) {
623+
ar.setSubAccessRule(new HashSet<>());
631624
}
632625

626+
addUniqueSubRules(ar, getAllowedQueryTypeRules());
627+
addUniqueSubRules(ar, getPhenotypeSubRules(studyIdentifier, conceptPath, projectAlias));
628+
addUniqueSubRules(ar, Collections.singleton(createPhenotypeSubRule(fence_topmed_consent_group_concept_path, "ALLOW_TOPMED_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true)));
629+
633630
return ar;
634631
}
635632

@@ -678,7 +675,6 @@ private Set<AccessRule> loadAllowedQueryTypeRules() {
678675
}
679676

680677

681-
682678
private Collection<? extends AccessRule> getTopmedRestrictedSubRules() {
683679
Set<AccessRule> rules = new HashSet<AccessRule>();
684680
rules.add(upsertTopmedRestrictedSubRule("CATEGORICAL", "$.query.query.variantInfoFilters[*].categoryVariantInfoFilters.*"));
@@ -722,7 +718,6 @@ private AccessRule upsertTopmedRestrictedSubRule(String type, String rule) {
722718
}
723719

724720
protected Collection<? extends AccessRule> getPhenotypeSubRules(String studyIdentifier, String conceptPath, String alias) {
725-
726721
Set<AccessRule> rules = new HashSet<AccessRule>();
727722
//categorical filters will always contain at least one entry (for the consent groups); it will never be empty
728723
rules.add(createPhenotypeSubRule(fence_parent_consent_group_concept_path, "ALLOW_PARENT_CONSENT", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "", true));
@@ -731,12 +726,16 @@ protected Collection<? extends AccessRule> getPhenotypeSubRules(String studyIden
731726
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.fields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "FIELDS", false));
732727
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "CATEGORICAL", true));
733728
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.requiredFields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "REQ_FIELDS", false));
729+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOf.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF", false));
730+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOfMulti.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF_MULTI", false));
734731
}
735732

736733
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "CATEGORICAL", true));
737734
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.numericFilters", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "NUMERIC", true));
738735
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.fields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "FIELDS", false));
739736
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.requiredFields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "REQUIRED_FIELDS", false));
737+
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.anyRecordOf.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF", false));
738+
rules.add(createPhenotypeSubRule(conceptPath, alias + "_" + studyIdentifier, "$.query.query.anyRecordOfMulti.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF_MULTI", false));
740739

741740
return rules;
742741
}
@@ -759,12 +758,16 @@ private Collection<? extends AccessRule> getHarmonizedSubRules() {
759758
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.fields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "FIELDS", false));
760759
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "CATEGORICAL", true));
761760
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.requiredFields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "REQ_FIELDS", false));
761+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOf.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF", false));
762+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOfMulti.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF_MULTI", false));
762763
}
763764

764765
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "CATEGORICAL", true));
765766
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.numericFilters", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "NUMERIC", true));
766767
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.fields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "FIELDS", false));
767768
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.requiredFields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "REQUIRED_FIELDS", false));
769+
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.anyRecordOf.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF", false));
770+
rules.add(createPhenotypeSubRule(fence_harmonized_concept_path, "HARMONIZED", "$.query.query.anyRecordOfMulti.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF_MULTI", false));
768771

769772
return rules;
770773
}
@@ -784,6 +787,8 @@ protected Collection<? extends AccessRule> getPhenotypeRestrictedSubRules(String
784787
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.fields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "FIELDS", false));
785788
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.categoryFilters", AccessRule.TypeNaming.ALL_CONTAINS, "CATEGORICAL", true));
786789
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.requiredFields.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "REQ_FIELDS", false));
790+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOf.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF", false));
791+
rules.add(createPhenotypeSubRule(underscorePath, "ALLOW " + underscorePath, "$.query.query.anyRecordOfMulti.[*]", AccessRule.TypeNaming.ALL_CONTAINS_OR_EMPTY, "ANY_RECORD_OF_MULTI", false));
787792
}
788793

789794
rules.add(createPhenotypeSubRule(null, alias + "_" + studyIdentifier + "_" + consentCode, "$.query.query.numericFilters.[*]", AccessRule.TypeNaming.IS_EMPTY, "DISALLOW_NUMERIC", false));
@@ -817,11 +822,7 @@ protected AccessRule populateTopmedAccessRule(AccessRule rule, boolean includePa
817822
rule.getGates().addAll(getGates(includeParent, false, true));
818823
}
819824

820-
if (rule.getSubAccessRule() == null) {
821-
rule.setSubAccessRule(new HashSet<>(getAllowedQueryTypeRules()));
822-
} else {
823-
rule.getSubAccessRule().addAll(getAllowedQueryTypeRules());
824-
}
825+
addUniqueSubRules(rule, getAllowedQueryTypeRules());
825826

826827
return rule;
827828
}
@@ -833,11 +834,9 @@ protected AccessRule populateHarmonizedAccessRule(AccessRule rule, String parent
833834
)));
834835
}
835836

836-
if (rule.getSubAccessRule() == null) {
837-
rule.setSubAccessRule(new HashSet<>(getAllowedQueryTypeRules()));
838-
rule.getSubAccessRule().addAll(getHarmonizedSubRules());
839-
rule.getSubAccessRule().addAll(getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias));
840-
}
837+
addUniqueSubRules(rule, getAllowedQueryTypeRules());
838+
addUniqueSubRules(rule, getHarmonizedSubRules());
839+
addUniqueSubRules(rule, getPhenotypeSubRules(studyIdentifier, parentConceptPath, projectAlias));
841840

842841
return rule;
843842
}
@@ -918,7 +917,7 @@ protected AccessRule upsertTopmedAccessRule(String project_name, String consent_
918917

919918
String conceptPath = fence_topmed_consent_group_concept_path;
920919
// Check if the conceptPath has `\\\\` present. This technically represents `\\`.
921-
if(conceptPath != null && conceptPath.contains("\\\\")) {
920+
if (conceptPath != null && conceptPath.contains("\\\\")) {
922921
// This will convert all `\\\\` to `\\`.
923922
conceptPath = conceptPath.replaceAll("\\\\\\\\", "\\\\");
924923
}
@@ -1001,8 +1000,8 @@ protected AccessRule createPhenotypeSubRule(String conceptPath, String alias, St
10011000
logger.debug("createPhenotypeSubRule() Creating new access rule {}", ar_name);
10021001

10031002
// Check if the conceptPath has `\\\\` present. This technically represents `\\`.
1004-
if(conceptPath != null && conceptPath.contains("\\\\")) {
1005-
// This will convert all `\\\\` to `\\`.
1003+
if (conceptPath != null && conceptPath.contains("\\\\")) {
1004+
// This will convert all `\\\\` to `\\`.
10061005
conceptPath = conceptPath.replaceAll("\\\\\\\\", "\\\\");
10071006
}
10081007

@@ -1051,4 +1050,29 @@ private String escapePath(String path) {
10511050
public List<AccessRule> getAccessRulesByPrivilegeIds(List<UUID> privilegeIds) {
10521051
return this.accessRuleRepo.getAccessRulesByPrivilegeIds(privilegeIds);
10531052
}
1053+
1054+
/**
1055+
* Adds unique sub-rules to the provided parent access rule. This method ensures that duplicate sub-rules,
1056+
* based on their names, are not added to the parent access rule.
1057+
*
1058+
* @param accessRule the parent access rule to which the sub-rules are added
1059+
* @param subRulesToAdd the collection of sub-rules to be added to the parent access rule
1060+
*/
1061+
private void addUniqueSubRules(AccessRule accessRule, Collection<? extends AccessRule> subRulesToAdd) {
1062+
if (accessRule.getSubAccessRule() == null) {
1063+
accessRule.setSubAccessRule(new HashSet<>());
1064+
}
1065+
1066+
Set<String> existingRuleNames = accessRule.getSubAccessRule().stream()
1067+
.map(AccessRule::getName)
1068+
.collect(Collectors.toSet());
1069+
1070+
for (AccessRule subRule : subRulesToAdd) {
1071+
if (!existingRuleNames.contains(subRule.getName())) {
1072+
accessRule.getSubAccessRule().add(subRule);
1073+
existingRuleNames.add(subRule.getName());
1074+
}
1075+
}
1076+
}
1077+
10541078
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,10 @@ private Privilege upsertClinicalPrivilege(String studyIdentifier, String project
207207

208208
// Check if the Privilege already exists
209209
Privilege priv = this.findByName(privilegeName);
210-
if (priv != null) {
211-
logger.info("{} already exists", privilegeName);
212-
return priv;
210+
if (priv == null) {
211+
priv = new Privilege();
213212
}
214213

215-
priv = new Privilege();
216214
try {
217215
priv.setApplication(picSureApp);
218216
priv.setName(privilegeName);

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

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,15 @@ public Role findByName(String roleName) {
177177
return this.roleRepository.findByName(roleName);
178178
}
179179

180+
public Role getOrCreateRole(String roleName, String roleDescription) {
181+
Role existingRole = this.roleRepository.findByName(roleName);
182+
if (existingRole != null) {
183+
return existingRole;
184+
}
185+
186+
return createRole(roleName, roleDescription);
187+
}
188+
180189
public Role createRole(String roleName, String roleDescription) {
181190
if (roleName.isEmpty()) {
182191
logger.info("createRole() roleName is empty");
@@ -205,28 +214,6 @@ public Role createRole(String roleName, String roleDescription) {
205214
return role;
206215
}
207216

208-
/**
209-
* Only use this service if you know this is a new role. Otherwise, use createRole().
210-
* @param roleName The name of the role to be created.
211-
* @param roleDescription Description of the role.
212-
* @return
213-
*/
214-
public Role createNewRole(String roleName, String roleDescription) {
215-
if(roleName.isEmpty()){
216-
logger.info("createNewRole() roleName is empty");
217-
return null;
218-
}
219-
220-
logger.info("createNewRole() New PSAMA role name:{}", roleName);
221-
Role role = new Role();
222-
role.setName(roleName);
223-
role.setDescription(roleDescription);
224-
// Since this is a new Role, we need to ensure that the
225-
// corresponding Privilege (with gates) and AccessRule is added.
226-
role.setPrivileges(privilegeService.addPrivileges(role, this.fenceMappingUtility.getFENCEMapping()));
227-
return role;
228-
}
229-
230217
/**
231218
* Insert or Update the User object's list of Roles in the database.
232219
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ public User updateUserRoles(User current_user, Set<String> roleNames) {
694694
Map<String, Role> existingRoles = roleService.findByNames(roleNames);
695695
List<Role> newRoles = roleNames.stream()
696696
.filter(roleName -> !currentRoleNames.contains(roleName))
697-
.map(roleName -> existingRoles.getOrDefault(roleName, this.roleService.createNewRole(roleName, "MANAGED role " + roleName)))
697+
.map(roleName -> existingRoles.getOrDefault(roleName, this.roleService.getOrCreateRole(roleName, "MANAGED role " + roleName)))
698698
.filter(Objects::nonNull)
699699
.collect(Collectors.toList());
700700

0 commit comments

Comments
 (0)