From f17e5fbb4e7b69b5224242a3a4cb3d369de1c666 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 1 Nov 2024 07:41:08 -0400 Subject: [PATCH] Second attempt at improving auth handling (#6382) * Revert "Revert "Improve auth interceptor operation handling (#6278)" (#6381)" This reverts commit 422d4f0741502aec0a2a617fbbc1fda6a85d77d5. * Move changelog --- ...e-auth-interceptor-operation-handling.yaml | 10 + .../r4/AuthorizationInterceptorJpaR4Test.java | 216 +++++++++++++----- .../auth/AuthorizationInterceptor.java | 14 +- ...uthRuleBuilderOperationNamedAndScoped.java | 6 +- .../interceptor/auth/OperationRule.java | 89 +++----- .../server/interceptor/auth/RuleImplOp.java | 57 +++-- .../auth/AuthorizationInterceptorR4Test.java | 4 +- 7 files changed, 256 insertions(+), 140 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6258-improve-auth-interceptor-operation-handling.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6258-improve-auth-interceptor-operation-handling.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6258-improve-auth-interceptor-operation-handling.yaml new file mode 100644 index 00000000000..54123b6f36b --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6258-improve-auth-interceptor-operation-handling.yaml @@ -0,0 +1,10 @@ +--- +type: fix +issue: 6258 +title: "The AuthorizationInterceptor handling for operations has been improved + so that operation rules now directly test the contents of response Bundle + or Parameters objects returned by the operation when configure to require + explicit response authorization. This fixes a regression in 7.4.0 where + operation responses could sometimes be denied even if appropriate + permissions were granted to view resources in a response bundle. Thanks to + Gijsbert van den Brink for reporting the issue with a sample test!" diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java index dc1d3546a13..b1154e9621b 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java @@ -20,11 +20,15 @@ import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor; +import ca.uhn.fhir.rest.gclient.IOperation; +import ca.uhn.fhir.rest.gclient.IOperationUnnamed; +import ca.uhn.fhir.rest.gclient.IOperationUntypedWithInput; import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor; import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRule; +import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRuleBuilder; import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRuleTester; import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum; import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder; @@ -68,6 +72,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; @@ -897,8 +902,17 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes } - @Test - public void testDiffOperation_AllowedByType_Instance() { + @ParameterizedTest + @CsvSource({ + // ResourceId , RequireExplicitResponseAuthorization , ShouldSucceed + "Patient/A , true , true", + "Patient/A/_history/2 , true , true", + "Observation/B , true , false", + "Patient/A , false , true", + "Patient/A/_history/2 , false , true", + "Observation/B , false , true" + }) + public void testDiffOperation_AllowedByType_Instance(String theResourceId, boolean theRequireExplicitResponseAuthorization, boolean theShouldSucceed) { createPatient(withId("A"), withActiveTrue()); createPatient(withId("A"), withActiveFalse()); createObservation(withId("B"), withStatus("final")); @@ -906,30 +920,42 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes myServer.getRestfulServer().registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { - return new RuleBuilder() - .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen() - .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen() - .allow().read().resourcesOfType(Patient.class).withAnyId().andThen() - .denyAll() - .build(); + RuleBuilder ruleBuilder = new RuleBuilder(); + if (theRequireExplicitResponseAuthorization) { + ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andRequireExplicitResponseAuthorization().andThen(); + ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andRequireExplicitResponseAuthorization().andThen(); + } else { + ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen(); + ruleBuilder.allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen(); + } + ruleBuilder.allow().read().resourcesOfType(Patient.class).withAnyId().andThen(); + ruleBuilder.denyAll(); + return ruleBuilder.build(); } }); Parameters diff; - diff = myClient.operation().onInstance("Patient/A").named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute(); - assertThat(diff.getParameter()).hasSize(1); - - diff = myClient.operation().onInstanceVersion(new IdType("Patient/A/_history/2")).named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute(); - assertThat(diff.getParameter()).hasSize(1); - - try { - myClient.operation().onInstance("Observation/B").named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class).execute(); - fail(); - } catch (ForbiddenOperationException e) { - // good + IOperation operation = myClient.operation(); + IOperationUnnamed target; + if (theResourceId.contains("_history")) { + target = operation.onInstanceVersion(new IdType(theResourceId)); + } else { + target = operation.onInstance(theResourceId); } + IOperationUntypedWithInput executable = target.named(ProviderConstants.DIFF_OPERATION_NAME).withNoParameters(Parameters.class); + if (theShouldSucceed) { + diff = executable.execute(); + assertThat(diff.getParameter()).hasSize(1); + } else { + try { + executable.execute(); + fail(); + } catch (ForbiddenOperationException e) { + // good + } + } } @Test @@ -943,8 +969,8 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andAllowAllResponses().andThen() - .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andAllowAllResponses().andThen() + .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onAnyInstance().andRequireExplicitResponseAuthorization().andThen() + .allow().operation().named(ProviderConstants.DIFF_OPERATION_NAME).onServer().andRequireExplicitResponseAuthorization().andThen() .allow().read().resourcesOfType(Patient.class).withAnyId().andThen() .denyAll() .build(); @@ -977,6 +1003,72 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes } + @Test + public void testDocumentOperation_withExplicitAuthorization() { + IIdType patientId = createPatient(); + IIdType compositionId = createResource("Composition", withSubject(patientId)); + + AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow().read().instance(patientId).andThen() + .allow().operation().named("$document").onInstance(compositionId).andRequireExplicitResponseAuthorization().andThen() + .allow().read().instance(compositionId).andThen() + .denyAll() + .build(); + } + }; + myServer.getRestfulServer().registerInterceptor(interceptor); + + Bundle bundle = myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute(); + assertEquals(2, bundle.getEntry().size()); + } + + @Test + public void testDocumentOperation_explicitAuthorizationNotNeeded() { + IIdType patientId = createPatient(); + IIdType compositionId = createResource("Composition", withSubject(patientId)); + + AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow().operation().named("$document").onInstance(compositionId).andAllowAllResponses().andThen() + .denyAll() + .build(); + } + }; + myServer.getRestfulServer().registerInterceptor(interceptor); + + Bundle bundle = myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute(); + assertEquals(2, bundle.getEntry().size()); + } + + @Test + public void testDocumentOperation_withoutExplicitAuthorization() { + IIdType patientId = createPatient(); + IIdType compositionId = createResource("Composition", withSubject(patientId)); + + AuthorizationInterceptor interceptor = new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow().operation().named("$document").onInstance(compositionId).andRequireExplicitResponseAuthorization().andThen() + .allow().read().instance(compositionId).andThen() + .denyAll() + .build(); + } + }; + myServer.getRestfulServer().registerInterceptor(interceptor); + + try { + myClient.operation().onInstanceVersion(compositionId).named("$document").withNoParameters(Parameters.class).returnResourceType(Bundle.class).execute(); + fail(); + } catch (ForbiddenOperationException e) { + // good + } + } @Test public void testGraphQL_AllowedByType_Instance() throws IOException { @@ -1205,8 +1297,17 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(resp)); } - @Test - public void testOperationEverything_SomeIncludedResourcesNotAuthorized() { + @ParameterizedTest + @CsvSource({ + // RequireExplicitResponseAuthorization , AddNotExplicitlyAuthorizedResources , ShouldSucceed + "true , false , true", + "true , true , false", + "false , false , true", + "false , true , true", + }) + public void testOperationEverything_SomeIncludedResourcesNotAuthorized(boolean theRequireExplicitResponseAuthorization, boolean theAddNotExplicitlyAuthorizedResources, boolean theShouldSucceed) { + int expectedCount = 2; + Patient pt1 = new Patient(); pt1.setActive(true); final IIdType pid1 = myClient.create().resource(pt1).execute().getId().toUnqualifiedVersionless(); @@ -1216,45 +1317,57 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes obs1.setSubject(new Reference(pid1)); myClient.create().resource(obs1).execute(); + if (theAddNotExplicitlyAuthorizedResources) { + // Add an Encounter, which will be returned by $everything but that hasn't been + // explicitly authorized + Encounter enc = new Encounter(); + enc.setSubject(new Reference(pid1)); + myClient.create().resource(enc).execute(); + + Organization org = new Organization(); + org.setName("Hello"); + IIdType orgId = myClient.create().resource(org).execute().getId().toUnqualifiedVersionless(); + expectedCount++; + + pt1.setId(pid1); + pt1.setManagingOrganization(new Reference(orgId)); + myClient.update().resource(pt1).execute(); + expectedCount++; + } + myServer.getRestfulServer().registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @SuppressWarnings("deprecation") @Override public List buildRuleList(RequestDetails theRequestDetails) { - return new RuleBuilder() - .allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andRequireExplicitResponseAuthorization().andThen() - .allow().read().resourcesOfType(Patient.class).inCompartment("Patient", pid1).andThen() - .allow().read().resourcesOfType(Observation.class).inCompartment("Patient", pid1).andThen() - .allow().create().resourcesOfType(Encounter.class).withAnyId().andThen() - .build(); + IAuthRuleBuilder ruleBuilder = new RuleBuilder(); + if (theRequireExplicitResponseAuthorization) { + ruleBuilder.allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andRequireExplicitResponseAuthorization().andThen(); + } else { + ruleBuilder.allow().operation().named(JpaConstants.OPERATION_EVERYTHING).onInstance(pid1).andAllowAllResponsesWithAllResourcesAccess().andThen(); + } + ruleBuilder.allow().read().resourcesOfType(Patient.class).inCompartment("Patient", pid1).andThen(); + ruleBuilder.allow().read().resourcesOfType(Observation.class).inCompartment("Patient", pid1).andThen(); + return ruleBuilder.build(); } }); - Bundle outcome = myClient + IOperationUntypedWithInput executable = myClient .operation() .onInstance(pid1) .named(JpaConstants.OPERATION_EVERYTHING) .withNoParameters(Parameters.class) - .returnResourceType(Bundle.class) - .execute(); - assertThat(outcome.getEntry()).hasSize(2); + .returnResourceType(Bundle.class); - // Add an Encounter, which will be returned by $everything but that hasn't been - // explicitly authorized - - Encounter enc = new Encounter(); - enc.setSubject(new Reference(pid1)); - myClient.create().resource(enc).execute(); - - try { - myClient - .operation() - .onInstance(pid1) - .named(JpaConstants.OPERATION_EVERYTHING) - .withNoParameters(Parameters.class) - .returnResourceType(Bundle.class) - .execute(); - fail(); - } catch (ForbiddenOperationException e) { - assertThat(e.getMessage()).contains("Access denied by default policy"); + if (theShouldSucceed) { + Bundle outcome = executable.execute(); + assertThat(outcome.getEntry()).hasSize(expectedCount); + } else { + try { + executable.execute(); + fail(); + } catch (ForbiddenOperationException e) { + assertThat(e.getMessage()).contains("Access denied by default policy"); + } } } @@ -2029,6 +2142,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes super(theResourceTypes); } + @Override public List buildRuleList(RequestDetails theRequestDetails) { List rules = new ArrayList<>(super.buildRuleList(theRequestDetails)); List rulesToAdd = new RuleBuilder().allow().transaction().withAnyOperation().andApplyNormalRules().build(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index e5da51b63a6..500a19d11a6 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -164,7 +164,8 @@ public class AuthorizationInterceptor implements IRuleApplier { rules.size(), getPointcutNameOrEmpty(thePointcut), getResourceTypeOrEmpty(theInputResource), - getResourceTypeOrEmpty(theOutputResource)); + getResourceTypeOrEmpty(theOutputResource), + thePointcut); Verdict verdict = null; for (IAuthRule nextRule : rules) { @@ -528,7 +529,7 @@ public class AuthorizationInterceptor implements IRuleApplier { case EXTENDED_OPERATION_TYPE: case EXTENDED_OPERATION_INSTANCE: { if (theResponseObject != null) { - resources = toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext); + resources = toListOfResourcesAndExcludeContainerUnlessStandalone(theResponseObject, fhirContext); } break; } @@ -575,7 +576,7 @@ public class AuthorizationInterceptor implements IRuleApplier { OUT, } - protected static List toListOfResourcesAndExcludeContainer( + protected static List toListOfResourcesAndExcludeContainerUnlessStandalone( IBaseResource theResponseObject, FhirContext fhirContext) { if (theResponseObject == null) { return Collections.emptyList(); @@ -588,6 +589,13 @@ public class AuthorizationInterceptor implements IRuleApplier { return Collections.singletonList(theResponseObject); } + return toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext); + } + + @Nonnull + public static List toListOfResourcesAndExcludeContainer( + IBaseResource theResponseObject, FhirContext fhirContext) { + List retVal; retVal = fhirContext.newTerser().getAllPopulatedChildElementsOfType(theResponseObject, IBaseResource.class); // Exclude the container diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java index 05b580c11fe..b641ee40bd0 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamedAndScoped.java @@ -27,11 +27,9 @@ public interface IAuthRuleBuilderOperationNamedAndScoped { IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses(); /** - * Responses for this operation will not be checked and access to all resources is allowed. This - * is intended for operations which are known to fetch a graph of resources that is known to be - * safe, such as `$everything` which may access and fetch resources outside the patient's compartment - * but enforces safety in what it fetches via strict SQL queries. + * @deprecated This is a synonym for {@link #andAllowAllResponses()}, use that method instead */ + @Deprecated(since = "7.6.0") IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponsesWithAllResourcesAccess(); /** diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java index f5c2f06634c..e45185fb4ec 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java @@ -85,6 +85,7 @@ class OperationRule extends BaseRule implements IAuthRule { myAppliesToTypes = theAppliesToTypes; } + @SuppressWarnings("EnumSwitchStatementWhichMissesCases") @Override public Verdict applyRule( RestOperationTypeEnum theOperation, @@ -97,23 +98,8 @@ class OperationRule extends BaseRule implements IAuthRule { Pointcut thePointcut) { FhirContext ctx = theRequestDetails.getServer().getFhirContext(); - // Operation rules apply to the execution of the operation itself, not to side effects like - // loading resources (that will presumably be reflected in the response). Those loads need - // to be explicitly authorized - if (!myAllowAllResourcesAccess && isResourceAccess(thePointcut)) { - return null; - } - boolean applies = false; switch (theOperation) { - case ADD_TAGS: - case DELETE_TAGS: - case GET_TAGS: - case GET_PAGE: - case GRAPHQL_REQUEST: - // These things can't be tracked by the AuthorizationInterceptor - // at this time - return null; case EXTENDED_OPERATION_SERVER: if (myAppliesToServer || myAppliesAtAnyLevel) { applies = true; @@ -138,10 +124,7 @@ class OperationRule extends BaseRule implements IAuthRule { applies = true; } else { IIdType requestResourceId = null; - if (theInputResourceId != null) { - requestResourceId = theInputResourceId; - } - if (requestResourceId == null && myAllowAllResponses) { + if (theRequestDetails.getId() != null) { requestResourceId = theRequestDetails.getId(); } if (requestResourceId != null) { @@ -168,40 +151,6 @@ class OperationRule extends BaseRule implements IAuthRule { } } break; - case CREATE: - break; - case DELETE: - break; - case HISTORY_INSTANCE: - break; - case HISTORY_SYSTEM: - break; - case HISTORY_TYPE: - break; - case READ: - break; - case SEARCH_SYSTEM: - break; - case SEARCH_TYPE: - break; - case TRANSACTION: - break; - case UPDATE: - break; - case VALIDATE: - break; - case VREAD: - break; - case METADATA: - break; - case META_ADD: - break; - case META: - break; - case META_DELETE: - break; - case PATCH: - break; default: return null; } @@ -214,13 +163,33 @@ class OperationRule extends BaseRule implements IAuthRule { return null; } - return newVerdict( - theOperation, - theRequestDetails, - theInputResource, - theInputResourceId, - theOutputResource, - theRuleApplier); + if (theOutputResource == null) { + // This is the request part + return newVerdict( + theOperation, + theRequestDetails, + theInputResource, + theInputResourceId, + theOutputResource, + theRuleApplier); + } else { + // This is the response part, so we might want to check all of the + // resources in the response + if (myAllowAllResponses) { + return newVerdict( + theOperation, + theRequestDetails, + theInputResource, + theInputResourceId, + theOutputResource, + theRuleApplier); + } else { + List outputResources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer( + theOutputResource, theRequestDetails.getFhirContext()); + return RuleImplOp.applyRulesToResponseResources( + theRequestDetails, theRuleApplier, thePointcut, outputResources); + } + } } /** 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 57cf7cbf615..8716208018a 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 @@ -828,31 +828,48 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { return verdict; } else if (theOutputResource != null) { - - List outputResources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer( - theOutputResource, theRequestDetails.getFhirContext()); - - Verdict verdict = null; - for (IBaseResource nextResource : outputResources) { - if (nextResource == null) { - continue; - } - Verdict newVerdict = theRuleApplier.applyRulesAndReturnDecision( - RestOperationTypeEnum.READ, theRequestDetails, null, null, nextResource, thePointcut); - if (newVerdict == null) { - continue; - } else if (verdict == null) { - verdict = newVerdict; - } else if (verdict.getDecision() == PolicyEnum.ALLOW && newVerdict.getDecision() == PolicyEnum.DENY) { - verdict = newVerdict; - } - } - return verdict; + return applyRulesToResponseBundle(theRequestDetails, theOutputResource, theRuleApplier, thePointcut); } else { return null; } } + @Nullable + private static Verdict applyRulesToResponseBundle( + RequestDetails theRequestDetails, + IBaseResource theOutputResource, + IRuleApplier theRuleApplier, + Pointcut thePointcut) { + List outputResources = + AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone( + theOutputResource, theRequestDetails.getFhirContext()); + return applyRulesToResponseResources(theRequestDetails, theRuleApplier, thePointcut, outputResources); + } + + @Nullable + public static Verdict applyRulesToResponseResources( + RequestDetails theRequestDetails, + IRuleApplier theRuleApplier, + Pointcut thePointcut, + List outputResources) { + Verdict verdict = null; + for (IBaseResource nextResource : outputResources) { + if (nextResource == null) { + continue; + } + Verdict newVerdict = theRuleApplier.applyRulesAndReturnDecision( + RestOperationTypeEnum.READ, theRequestDetails, null, null, nextResource, thePointcut); + if (newVerdict == null) { + continue; + } else if (verdict == null) { + verdict = newVerdict; + } else if (verdict.getDecision() == PolicyEnum.ALLOW && newVerdict.getDecision() == PolicyEnum.DENY) { + verdict = newVerdict; + } + } + return verdict; + } + private boolean isInvalidNestedBundleRequest(BundleEntryParts theEntry) { IBaseResource resource = theEntry.getResource(); if (!(resource instanceof IBaseBundle)) { diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java index d64ff934a1a..ed4e8718913 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java @@ -4230,7 +4230,7 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline RequestDetails requestDetails = new SystemRequestDetails(); requestDetails.setResourceName("Bundle"); - List resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(searchSet, ourCtx); + List resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx); assertEquals(1, resources.size()); assertTrue(resources.contains(bundle)); } @@ -4247,7 +4247,7 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline RequestDetails requestDetails = new SystemRequestDetails(); requestDetails.setResourceName("Patient"); - List resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(searchSet, ourCtx); + List resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx); assertEquals(2, resources.size()); assertTrue(resources.contains(patient1)); assertTrue(resources.contains(patient2));