diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2760-collapse-shared-compartments.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2760-collapse-shared-compartments.yaml new file mode 100644 index 00000000000..8bc7b10694e --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2760-collapse-shared-compartments.yaml @@ -0,0 +1,6 @@ +--- +type: change +issue: 2760 +title: "If two authorization compartments apply to the same targets and share the same compartment name, +then instead of creating a new compartment, the rule builder now adds the new owner to the list of owners in the +existing compartment." diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java index 5e9a121e813..a8982b3a7c7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java @@ -1457,6 +1457,52 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes } } + @Test + public void testReadCompartmentTwoPatientIds() { + Patient patient1 = new Patient(); + patient1.setActive(true); + IIdType p1id = myPatientDao.create(patient1).getId().toUnqualifiedVersionless(); + + Patient patient2 = new Patient(); + patient2.setActive(true); + IIdType p2id = myPatientDao.create(patient2).getId().toUnqualifiedVersionless(); + + ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow() + .read() + .resourcesOfType(Patient.class) + .inCompartment("Patient", p1id) + .andThen() + .allow() + .read() + .resourcesOfType(Patient.class) + .inCompartment("Patient", p2id) + .andThen() + .denyAll() + .build(); + } + }); + + { + String url = "/Patient?_id=" + p1id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(1, result.getTotal()); + } + { + String url = "/Patient?_id=" + p2id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(1, result.getTotal()); + } + { + String url = "/Patient?_id=" + p1id.getIdPart() + "," + p2id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(2, result.getTotal()); + } + } + @Test public void testDeleteExpungeBlocked() { // Create Patient, and Observation that refers to it diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 5b0154a7eb3..2785bfe9a08 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -38,6 +38,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -46,7 +47,7 @@ import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; public class RuleBuilder implements IAuthRuleBuilder { private static final ConcurrentHashMap, String> ourTypeToName = new ConcurrentHashMap<>(); - private ArrayList myRules; + private final ArrayList myRules; private IAuthRuleBuilderRule myAllow; private IAuthRuleBuilderRule myDeny; @@ -229,8 +230,8 @@ public class RuleBuilder implements IAuthRuleBuilder { private class RuleBuilderRule implements IAuthRuleBuilderRule { - private PolicyEnum myRuleMode; - private String myRuleName; + private final PolicyEnum myRuleMode; + private final String myRuleName; private RuleBuilderRuleOp myReadRuleBuilder; private RuleBuilderRuleOp myWriteRuleBuilder; @@ -321,7 +322,7 @@ public class RuleBuilder implements IAuthRuleBuilder { private AppliesTypeEnum myAppliesTo; private Set myAppliesToTypes; - private RestOperationTypeEnum myOperationType; + private final RestOperationTypeEnum myOperationType; RuleBuilderRuleConditional(RestOperationTypeEnum theOperationType) { myOperationType = theOperationType; @@ -506,12 +507,26 @@ public class RuleBuilder implements IAuthRuleBuilder { Validate.notBlank(theCompartmentName, "theCompartmentName must not be null"); Validate.notNull(theOwner, "theOwner must not be null"); validateOwner(theOwner); + Optional oRule = findRuleByAppliesToAndCompartmentName(theCompartmentName); + if (oRule.isPresent()) { + RuleImplOp rule = oRule.get(); + rule.addClassifierCompartmentOwner(theOwner); + return new RuleBuilderFinished(rule); + } myInCompartmentName = theCompartmentName; myInCompartmentOwners = Collections.singletonList(theOwner); myClassifierType = ClassifierTypeEnum.IN_COMPARTMENT; return finished(); } + private Optional findRuleByAppliesToAndCompartmentName(String theCompartmentName) { + return myRules.stream() + .filter(RuleImplOp.class::isInstance) + .map(RuleImplOp.class::cast) + .filter(rule -> rule.matches(myAppliesTo, myAppliesToInstances, myAppliesToTypes, theCompartmentName)) + .findFirst(); + } + private void validateOwner(IIdType theOwner) { Validate.notBlank(theOwner.getIdPart(), "owner.getIdPart() must not be null or empty"); Validate.notBlank(theOwner.getIdPart(), "owner.getResourceType() must not be null or empty"); @@ -546,7 +561,7 @@ public class RuleBuilder implements IAuthRuleBuilder { private class RuleBuilderRuleOperationNamed implements IAuthRuleBuilderOperationNamed { - private String myOperationName; + private final String myOperationName; RuleBuilderRuleOperationNamed(String theOperationName) { if (theOperationName != null && !theOperationName.startsWith("$")) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index cfe5097b9ab..07cb7b6f0a5 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java @@ -630,4 +630,24 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { void setAppliesToDeleteExpunge(boolean theAppliesToDeleteExpunge) { myAppliesToDeleteExpunge = theAppliesToDeleteExpunge; } + + public void addClassifierCompartmentOwner(IIdType theOwner) { + List newList = new ArrayList<>(myClassifierCompartmentOwners); + newList.add(theOwner); + myClassifierCompartmentOwners = newList; + } + + public boolean matches(AppliesTypeEnum theAppliesTo, Collection theAppliesToInstances, Set theAppliesToTypes, String theCompartmentName) { + switch (theAppliesTo) { + case TYPES: + return theAppliesToTypes.equals(myAppliesToTypes) && theCompartmentName.equals(myClassifierCompartmentName); + case INSTANCES: + return theAppliesToInstances.equals(myAppliesToInstances) && theCompartmentName.equals(myClassifierCompartmentName); + case ALL_RESOURCES: + return theCompartmentName.equals(myClassifierCompartmentName); + default: + // no more cases + return false; + } + } }