diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2761-compartment-match-id-list.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2761-compartment-match-id-list.yaml new file mode 100644 index 00000000000..590137f488a --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2761-compartment-match-id-list.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 2761 +title: "When searching for ExplanationOfBenefit?patient=123,456, the compartment authorization interceptor was treating +'123,456' as a single id rather than as a list of ids. This has been corrected." 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 a8982b3a7c7..c67b198eaeb 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 @@ -38,6 +38,7 @@ import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Condition; import org.hl7.fhir.r4.model.Encounter; +import org.hl7.fhir.r4.model.ExplanationOfBenefit; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Observation; @@ -1473,12 +1474,12 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes return new RuleBuilder() .allow() .read() - .resourcesOfType(Patient.class) + .allResources() .inCompartment("Patient", p1id) .andThen() .allow() .read() - .resourcesOfType(Patient.class) + .allResources() .inCompartment("Patient", p2id) .andThen() .denyAll() @@ -1503,6 +1504,80 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes } } + + @Test + public void testReadCompartmentTwoPatientIdsTwoEOBs() { + Patient patient1 = new Patient(); + patient1.setActive(true); + IIdType p1id = myPatientDao.create(patient1).getId().toUnqualifiedVersionless(); + + ExplanationOfBenefit eob1 = new ExplanationOfBenefit(); + eob1.setPatient(new Reference(p1id)); + myExplanationOfBenefitDao.create(eob1); + + Patient patient2 = new Patient(); + patient2.setActive(true); + IIdType p2id = myPatientDao.create(patient2).getId().toUnqualifiedVersionless(); + + ExplanationOfBenefit eob2 = new ExplanationOfBenefit(); + eob2.setPatient(new Reference(p2id)); + myExplanationOfBenefitDao.create(eob2); + + Patient patient3 = new Patient(); + patient3.setActive(true); + IIdType p3id = myPatientDao.create(patient3).getId().toUnqualifiedVersionless(); + + ExplanationOfBenefit eob3 = new ExplanationOfBenefit(); + eob3.setPatient(new Reference(p3id)); + myExplanationOfBenefitDao.create(eob3); + + ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow() + .read() + .allResources() + .inCompartment("Patient", p1id) + .andThen() + .allow() + .read() + .allResources() + .inCompartment("Patient", p2id) + .andThen() + .denyAll() + .build(); + } + }); + + { + String url = "/ExplanationOfBenefit?patient=" + p1id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(1, result.getTotal()); + } + { + String url = "/ExplanationOfBenefit?patient=" + p2id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(1, result.getTotal()); + } + { + String url = "/ExplanationOfBenefit?patient=" + p1id.getIdPart() + "," + p2id.getIdPart(); + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + assertEquals(2, result.getTotal()); + } + { + String url = "/ExplanationOfBenefit?patient=" + p1id.getIdPart() + "," + p3id.getIdPart(); + try { + Bundle result = myClient.search().byUrl(url).returnBundle(Bundle.class).execute(); + fail(); + } catch (ForbiddenOperationException e) { + assertThat(e.getMessage(), startsWith("HTTP 403 Forbidden: Access denied by rule")); + } + } + + } + + @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/RuleImplOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index 07cb7b6f0a5..44c98e047f4 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 @@ -524,7 +524,6 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } else if (getMode() == PolicyEnum.ALLOW) { return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); } - break; } } } @@ -546,13 +545,16 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { String[] values = theSearchParams.get(theSearchParamName); if (values != null) { for (String nextParameterValue : values) { - if (nextParameterValue.equals(theCompartmentOwner.getValue())) { - verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); - break; - } - if (nextParameterValue.equals(theCompartmentOwner.getIdPart())) { - verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); - break; + QualifiedParamList orParamList = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(null, nextParameterValue); + for (String next : orParamList) { + if (next.equals(theCompartmentOwner.getValue())) { + verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + break; + } + if (next.equals(theCompartmentOwner.getIdPart())) { + verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + break; + } } } }