diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3756-tester-api.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3756-tester-api.yaml new file mode 100644 index 00000000000..2511bdd6ba1 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3756-tester-api.yaml @@ -0,0 +1,4 @@ +--- +type: change +issue: 3756 +title: "The IAuthRuleTester api has changed and now uses a parameter object for flexibility." diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/AuthorizationSearchParamMatcher.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/AuthorizationSearchParamMatcher.java index bca89bbb920..331775c6c76 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/AuthorizationSearchParamMatcher.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/AuthorizationSearchParamMatcher.java @@ -23,8 +23,14 @@ package ca.uhn.fhir.jpa.searchparam.matcher; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +/** + * Adapter from {@link SearchParamMatcher} to our authorization version. + */ public class AuthorizationSearchParamMatcher implements IAuthorizationSearchParamMatcher { + private static final Logger ourLog = LoggerFactory.getLogger(AuthorizationSearchParamMatcher.class); private final SearchParamMatcher mySearchParamMatcher; public AuthorizationSearchParamMatcher(SearchParamMatcher mySearchParamMatcher) { @@ -36,18 +42,20 @@ public class AuthorizationSearchParamMatcher implements IAuthorizationSearchPara try { InMemoryMatchResult inMemoryMatchResult = mySearchParamMatcher.match(theCriteria, theResource, null); if (!inMemoryMatchResult.supported()) { - return new MatchResult(Match.UNSUPPORTED, inMemoryMatchResult.getUnsupportedReason()); + return MatchResult.buildUnsupported(inMemoryMatchResult.getUnsupportedReason()); } if (inMemoryMatchResult.matched()) { - return new MatchResult(Match.MATCH, null); + return MatchResult.buildMatched(); } else { - return new MatchResult(Match.NO_MATCH, null); + return MatchResult.buildUnmatched(); } } catch (MatchUrlService.UnrecognizedSearchParameterException e) { - // wipmb revisit this design - // The matcher treats a bad expression as InvalidRequestException because it assumes it is during SearchParameter storage. - // Instead, we adapt this to UNSUPPORTED during authorization. We may be applying to all types, and this filter won't match. - return new MatchResult(Match.UNSUPPORTED, e.getMessage()); + // The matcher treats a bad expression as InvalidRequestException because + // it assumes it is during SearchParameter storage. + // Instead, we adapt this to UNSUPPORTED during authorization. + // We may be applying to all types, and this filter won't match. + ourLog.info("Unsupported filter {} applied to resource: {}", theCriteria, e.getMessage()); + return MatchResult.buildUnsupported(e.getMessage()); } } } diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/auth/FhirQueryRuleImplTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/auth/FhirQueryRuleImplTest.java index ef9157f3b65..61e827a07d1 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/auth/FhirQueryRuleImplTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/auth/FhirQueryRuleImplTest.java @@ -94,7 +94,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { .andThen().build().get(0); when(myMatcher.match(ArgumentMatchers.eq("Patient?family=Smith"), ArgumentMatchers.same(myResource))) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeMatched()); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildMatched()); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); @@ -112,7 +112,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { .inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi") .andThen().build().get(0); when(myMatcher.match(ArgumentMatchers.eq("Patient?family=smi"), ArgumentMatchers.same(myResource))) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeUnmatched()); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched()); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); @@ -133,7 +133,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { .inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "code=28521000087105") .andThen().build().get(0); when(myMatcher.match("Observation?code=28521000087105", myResource2)) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeUnmatched()); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched()); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); @@ -152,7 +152,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { .withFilter("code=12") .andThen().build().get(0); when(myMatcher.match("Observation?code=12", myResource2)) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeUnmatched()); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched()); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); @@ -171,7 +171,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { .withFilter("code=28521000087105") .andThen().build().get(0); when(myMatcher.match("Observation?code=28521000087105", myResource2)) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeMatched()); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildMatched()); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); @@ -200,7 +200,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Patient") .inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi").andThen().build().get(0); when(myMatcher.match("Patient?family=smi", myResource)) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeUnsupported("I'm broken unsupported chain XXX")); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported("I'm broken unsupported chain XXX")); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); @@ -217,7 +217,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder { myRule = (FhirQueryRuleImpl) new RuleBuilder().deny().read().resourcesOfType("Patient") .inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi").andThen().build().get(0); when(myMatcher.match("Patient?family=smi", myResource)) - .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.makeUnsupported("I'm broken unsupported chain XXX")); + .thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported("I'm broken unsupported chain XXX")); // when AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java index ace323139f2..22e4d39a1ac 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/BaseRule.java @@ -29,7 +29,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; @@ -54,20 +53,24 @@ abstract class BaseRule implements IAuthRule { theTesters.forEach(this::addTester); } - private boolean applyTesters(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource, IBaseResource theOutputResource) { + private boolean applyTesters(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource, IBaseResource theOutputResource, IRuleApplier theRuleApplier) { assert !(theInputResource != null && theOutputResource != null); boolean retVal = true; if (theOutputResource == null) { + IAuthRuleTester.RuleTestRequest inputRequest = new IAuthRuleTester.RuleTestRequest(myMode, theOperation, theRequestDetails, theInputResourceId, theInputResource, theRuleApplier); + + for (IAuthRuleTester next : getTesters()) { - if (!next.matches(theOperation, theRequestDetails, theInputResourceId, theInputResource)) { + if (!next.matches(inputRequest)) { retVal = false; break; } } } else { + IAuthRuleTester.RuleTestRequest outputRequest = new IAuthRuleTester.RuleTestRequest(myMode, theOperation, theRequestDetails, theOutputResource.getIdElement(), theOutputResource, theRuleApplier); for (IAuthRuleTester next : getTesters()) { - if (!next.matchesOutput(theOperation, theRequestDetails, theOutputResource)) { + if (!next.matchesOutput(outputRequest)) { retVal = false; break; } @@ -98,8 +101,8 @@ abstract class BaseRule implements IAuthRule { return Collections.unmodifiableList(myTesters); } - Verdict newVerdict(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource) { - if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource)) { + Verdict newVerdict(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, IRuleApplier theRuleApplier) { + if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource, theRuleApplier)) { return null; } return new Verdict(myMode, this); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java new file mode 100644 index 00000000000..ca2f615788e --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTester.java @@ -0,0 +1,42 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +public class FhirQueryRuleTester implements IAuthRuleTester { + private final String myType; + private final String myFilter; + + public FhirQueryRuleTester(String theType, String theFilter) { + myType = theType; + myFilter = theFilter; + } + + @Override + public boolean matches(RuleTestRequest theRuleTestRequest) { + // wipmb placeholder until we get to writes + return true; + } + + @Override + public boolean matchesOutput(RuleTestRequest theRuleTestRequest) { + + // look for a matcher + IAuthorizationSearchParamMatcher matcher = theRuleTestRequest.ruleApplier.getSearchParamMatcher(); + if (matcher == null) { + return false; + } + + // wipmb myType, or target type? Is this just query filter, or do we bring * into it? + IAuthorizationSearchParamMatcher.MatchResult mr = matcher.match(myType + "?" + myFilter, theRuleTestRequest.resource); + + switch (mr.getMatch()) { + case MATCH: + return true; + case UNSUPPORTED: + theRuleTestRequest.ruleApplier.getTroubleshootingLog().warn("Unsupported matcher expression {}: {}.", myFilter, mr.getUnsupportedReason()); + // unsupported doesn't match unless this is a deny request, and we need to be safe! + return (theRuleTestRequest.mode == PolicyEnum.DENY); + case NO_MATCH: + default: + return false; + } + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleTester.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleTester.java index a2f60e0a96c..17f28e1bad7 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleTester.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleTester.java @@ -22,9 +22,13 @@ package ca.uhn.fhir.rest.server.interceptor.auth; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; +import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + /** * Allows user-supplied logic for authorization rules. *

@@ -34,6 +38,44 @@ import org.hl7.fhir.instance.model.api.IIdType; * @since 3.4.0 */ public interface IAuthRuleTester { + /** + * A request object to make this easier to extend. + */ + class RuleTestRequest { + // fake record pattern + /** the mode of the calling rule context */ + @Nonnull public final PolicyEnum mode; + /** + * The FHIR operation being performed. + * Note that this is not necessarily the same as the value obtained from invoking + * {@link RequestDetails#getRestOperationType()} on {@literal requestDetails} + * because multiple operations can be nested within + * an HTTP request using FHIR transaction and batch operations + */ + @Nonnull public final RestOperationTypeEnum operation; + @Nonnull public final RequestDetails requestDetails; + @Nullable public final IIdType resourceId; + @Nullable public final IBaseResource resource; + /** supplier for support services */ + @Nonnull public final IRuleApplier ruleApplier; + + public RuleTestRequest(PolicyEnum theMode, @Nonnull RestOperationTypeEnum theOperation, @Nonnull RequestDetails theRequestDetails, @Nullable IIdType theResourceId, @Nullable IBaseResource theResource, @Nonnull IRuleApplier theRuleApplier) { + Validate.notNull(theMode); + Validate.notNull(theOperation); + Validate.notNull(theRequestDetails); + Validate.notNull(theRuleApplier); + mode = theMode; + operation = theOperation; + requestDetails = theRequestDetails; + resource = theResource; + if (theResourceId == null && resource != null) { + resourceId = resource.getIdElement(); + } else { + resourceId = theResourceId; + } + ruleApplier = theRuleApplier; + } + } /** * Allows user-supplied logic for authorization rules. @@ -41,11 +83,19 @@ public interface IAuthRuleTester { * THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API * may change. * - * @param theOperation The FHIR operation being performed - Note that this is not necessarily the same as the value obtained from invoking - * {@link RequestDetails#getRestOperationType()} on {@literal theRequestDetails} because multiple operations can be nested within - * an HTTP request using FHIR transaction and batch operations - * @since 3.4.0 + * @param theRequest The details to evaluate + * @since 6.1.0 */ + default boolean matches(RuleTestRequest theRequest) { + return this.matches(theRequest.operation, theRequest.requestDetails, theRequest.resourceId, theRequest.resource); + } + + + /** + * DO NOT IMPLEMENT - Old api. {@link #matches(RuleTestRequest)} instead. + * @deprecated + */ + @Deprecated(since = "6.1.0") default boolean matches(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource) { return true; } @@ -56,11 +106,18 @@ public interface IAuthRuleTester { * THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API * may change. * - * @param theOperation The FHIR operation being performed - Note that this is not necessarily the same as the value obtained from invoking - * {@link RequestDetails#getRestOperationType()} on {@literal theRequestDetails} because multiple operations can be nested within - * an HTTP request using FHIR transaction and batch operations - * @since 5.0.0 + * @param theRequest The details to evaluate + * @since 6.1.0 */ + default boolean matchesOutput(RuleTestRequest theRequest) { + return this.matchesOutput(theRequest.operation, theRequest.requestDetails, theRequest.resource); + } + + /** + * DO NOT IMPLEMENT - Old api. {@link #matches(RuleTestRequest)} instead. + * @deprecated + */ + @Deprecated(since = "6.1.0") default boolean matchesOutput(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theOutputResource) { return true; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthorizationSearchParamMatcher.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthorizationSearchParamMatcher.java index ca3e4da5c9b..d126ac8f071 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthorizationSearchParamMatcher.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthorizationSearchParamMatcher.java @@ -23,36 +23,42 @@ package ca.uhn.fhir.rest.server.interceptor.auth; import org.hl7.fhir.instance.model.api.IBaseResource; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** - * wipjv Ken do we like this name, or this package? + * Adapt the InMemoryMatcher to support authorization filters in {@link FhirQueryRuleImpl}. */ public interface IAuthorizationSearchParamMatcher { MatchResult match(String theCriteria, IBaseResource theResource); + /** + * Match outcomes. + */ enum Match { MATCH, NO_MATCH, + /** Used for contexts without matcher infrastructure like hybrid providers */ UNSUPPORTED } - public static class MatchResult { - private final Match myMatch; - private final String myUnsupportedReason; + class MatchResult { + // wipmb consider a record pattern - public and drop the accessors. + @Nonnull private final Match myMatch; + @Nullable private final String myUnsupportedReason; - public static MatchResult makeMatched() { + public static MatchResult buildMatched() { return new MatchResult(Match.MATCH, null); } - public static MatchResult makeUnmatched() { + public static MatchResult buildUnmatched() { return new MatchResult(Match.NO_MATCH, null); } - public static MatchResult makeUnsupported(@Nonnull String theReason) { + public static MatchResult buildUnsupported(@Nonnull String theReason) { return new MatchResult(Match.UNSUPPORTED, theReason); } - public MatchResult(Match myMatch, String myUnsupportedReason) { + private MatchResult(Match myMatch, String myUnsupportedReason) { this.myMatch = myMatch; this.myUnsupportedReason = myUnsupportedReason; } 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 b08446aa530..2a4098aff96 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 @@ -199,7 +199,7 @@ class OperationRule extends BaseRule implements IAuthRule { return null; } - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } /** 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 584e5b23402..9e23de12b58 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 @@ -583,7 +583,6 @@ public class RuleBuilder implements IAuthRuleBuilder { @Override public IAuthRuleFinished inCompartmentWithFilter(String theCompartmentName, IIdType theIdElement, String theFilter) { - // wipjv (resolved?) implemented Validate.notBlank(theCompartmentName, "theCompartmentName must not be null"); Validate.notNull(theIdElement, "theOwner must not be null"); validateOwner(theIdElement); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 092b066e951..9effd068e7f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -72,14 +72,14 @@ public class RuleBulkExportImpl extends BaseRule { } if (myWantAnyStyle || myWantExportStyle == BulkDataExportOptions.ExportStyle.SYSTEM) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } if (isNotBlank(myGroupId) && options.getGroupId() != null) { String expectedGroupId = new IdDt(myGroupId).toUnqualifiedVersionless().getValue(); String actualGroupId = options.getGroupId().toUnqualifiedVersionless().getValue(); if (Objects.equals(expectedGroupId, actualGroupId)) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } return null; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java index ccb1b33219f..86cb3f0974c 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java @@ -72,7 +72,7 @@ public class RuleImplConditional extends BaseRule implements IAuthRule { break; } - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } return null; 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 374018c407d..11da0e3a4c7 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 @@ -115,12 +115,12 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { case SEARCH_SYSTEM: case HISTORY_SYSTEM: if (theFlags.contains(AuthorizationFlagsEnum.DO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } break; case SEARCH_TYPE: if (theFlags.contains(AuthorizationFlagsEnum.DO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } target.resourceType = theRequestDetails.getResourceName(); target.setSearchParams(theRequestDetails); @@ -135,18 +135,18 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { break; case HISTORY_TYPE: if (theFlags.contains(AuthorizationFlagsEnum.DO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } target.resourceType = theRequestDetails.getResourceName(); break; case HISTORY_INSTANCE: if (theFlags.contains(AuthorizationFlagsEnum.DO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } target.resourceIds = Collections.singleton(theInputResourceId); break; case GET_PAGE: - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); // None of the following are checked on the way in case ADD_TAGS: @@ -222,7 +222,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { case DELETE: if (theOperation == RestOperationTypeEnum.DELETE) { if (thePointcut == Pointcut.STORAGE_PRE_DELETE_EXPUNGE && myAppliesToDeleteExpunge) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } if (myAppliesToDeleteCascade != (thePointcut == Pointcut.STORAGE_CASCADE_DELETE)) { return null; @@ -233,10 +233,10 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { if (theInputResourceId.hasIdPart() == false) { // This is a conditional DELETE, so we'll authorize it using STORAGE events instead // so just let it through for now.. - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } if (theInputResource == null && myClassifierCompartmentOwners != null && myClassifierCompartmentOwners.size() > 0) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } target.resource = theInputResource; @@ -246,14 +246,14 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } break; case GRAPHQL: - return applyRuleToGraphQl(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, thePointcut); + return applyRuleToGraphQl(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, thePointcut, theRuleApplier); case TRANSACTION: return applyRuleToTransaction(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier, thePointcut, ctx); case ALL: - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); case METADATA: if (theOperation == RestOperationTypeEnum.METADATA) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } return null; default: @@ -263,11 +263,11 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { switch (myAppliesTo) { case INSTANCES: - return applyRuleToInstances(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, target); + return applyRuleToInstances(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, target, theRuleApplier); case ALL_RESOURCES: if (target.resourceType != null) { if (myClassifierType == ClassifierTypeEnum.ANY_ID) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } break; @@ -295,7 +295,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { return null; } if (myClassifierType == ClassifierTypeEnum.ANY_ID) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } else if (myClassifierType == ClassifierTypeEnum.IN_COMPARTMENT) { // ok we'll check below } @@ -320,16 +320,16 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { case ANY_ID: break; case IN_COMPARTMENT: - return applyRuleToCompartment(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theFlags, theFhirContext, theRuleTarget); + return applyRuleToCompartment(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theFlags, theFhirContext, theRuleTarget, theRuleApplier); default: throw new IllegalStateException(Msg.code(337) + "Unable to apply security to event of applies to type " + myAppliesTo); } - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } @Nullable - private Verdict applyRuleToGraphQl(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Pointcut thePointcut) { + private Verdict applyRuleToGraphQl(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Pointcut thePointcut, IRuleApplier theRuleApplier) { if (theOperation == RestOperationTypeEnum.GRAPHQL_REQUEST) { // Make sure that the requestor actually has sufficient access to see the given resource @@ -337,14 +337,14 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { return null; } - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } else { return null; } } @Nullable - private Verdict applyRuleToCompartment(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Set theFlags, FhirContext ctx, RuleTarget target) { + private Verdict applyRuleToCompartment(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Set theFlags, FhirContext ctx, RuleTarget target, IRuleApplier theRuleApplier) { FhirTerser t = ctx.newTerser(); boolean foundMatch = false; @@ -377,7 +377,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { * it makes sense. */ if (next.getResourceType().equals(target.resourceType)) { - Verdict verdict = checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(target.getSearchParams(), next, SP_RES_ID, theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + Verdict verdict = checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(target.getSearchParams(), next, SP_RES_ID, theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); if (verdict != null) { return verdict; } @@ -423,13 +423,13 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { if (target.getSearchParams() != null && !theFlags.contains(AuthorizationFlagsEnum.DO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { for (RuntimeSearchParam nextRuntimeSearchParam : params) { String name = nextRuntimeSearchParam.getName(); - Verdict verdict = checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(target.getSearchParams(), next, name, theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + Verdict verdict = checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(target.getSearchParams(), next, name, theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); if (verdict != null) { return verdict; } } } else if (getMode() == PolicyEnum.ALLOW) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } } @@ -438,11 +438,11 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { if (!foundMatch) { return null; } - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } @Nullable - private Verdict applyRuleToInstances(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, RuleTarget target) { + private Verdict applyRuleToInstances(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, RuleTarget target, IRuleApplier theRuleApplier) { if (target.resourceIds != null && target.resourceIds.size() > 0) { int haveMatches = 0; for (IIdType requestAppliesToResource : target.resourceIds) { @@ -463,7 +463,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } if (haveMatches == target.resourceIds.size()) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } @@ -477,7 +477,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } if (theInputResource != null && requestAppliesToTransaction(ctx, myOp, theInputResource)) { if (getMode() == PolicyEnum.DENY) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } List inputResources = BundleUtil.toListOfEntries(ctx, (IBaseBundle) theInputResource); Verdict verdict = null; @@ -551,7 +551,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { * be applying security on the way out */ if (allComponentsAreGets) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } return verdict; @@ -602,7 +602,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } } - private Verdict checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(Map theSearchParams, IIdType theCompartmentOwner, String theSearchParamName, RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource) { + private Verdict checkForSearchParameterMatchingCompartmentAndReturnSuccessfulVerdictOrNull(Map theSearchParams, IIdType theCompartmentOwner, String theSearchParamName, RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, IRuleApplier theRuleApplier) { Verdict verdict = null; if (theSearchParams != null) { String[] values = theSearchParams.get(theSearchParamName); @@ -611,11 +611,11 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { QualifiedParamList orParamList = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(null, nextParameterValue); for (String next : orParamList) { if (next.equals(theCompartmentOwner.getValue())) { - verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); break; } if (next.equals(theCompartmentOwner.getIdPart())) { - verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); break; } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java index 3e333637990..8f3680a3090 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplPatch.java @@ -41,7 +41,7 @@ class RuleImplPatch extends BaseRule { if (myAllRequests) { if (theOperation == RestOperationTypeEnum.PATCH) { if (theInputResource == null && theOutputResource == null) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplUpdateHistoryRewrite.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplUpdateHistoryRewrite.java index bd54f819b8b..78e37fc329d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplUpdateHistoryRewrite.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplUpdateHistoryRewrite.java @@ -41,7 +41,7 @@ public class RuleImplUpdateHistoryRewrite extends BaseRule { IRuleApplier theRuleApplier, Set theFlags, Pointcut thePointcut) { if (myAllRequests) { if (theRequestDetails.getId() != null && theRequestDetails.getId().hasVersionIdPart() && theOperation == RestOperationTypeEnum.UPDATE) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchParameterAndValueSetRuleImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchParameterAndValueSetRuleImpl.java index 03017043d11..f337c9c1005 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchParameterAndValueSetRuleImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchParameterAndValueSetRuleImpl.java @@ -113,12 +113,12 @@ class SearchParameterAndValueSetRuleImpl extends RuleImplOp { } if (myWantCode && codeMatchCount.getMatchingCodeCount() > 0) { - return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); } else if (!myWantCode) { boolean notFound = getMode() == PolicyEnum.ALLOW && codeMatchCount.getMatchingCodeCount() == 0; boolean othersFound = getMode() == PolicyEnum.DENY && codeMatchCount.getMatchingCodeCount() < codeMatchCount.getOverallCodeCount(); if (notFound || othersFound) { - AuthorizationInterceptor.Verdict verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + AuthorizationInterceptor.Verdict verdict = newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier); if (notFound) { troubleshootingLog .debug("Code was not found in VS - Verdict: {}", verdict); diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTesterTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTesterTest.java new file mode 100644 index 00000000000..1e045c9878b --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/FhirQueryRuleTesterTest.java @@ -0,0 +1,103 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoSettings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher.MatchResult.buildMatched; +import static ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched; +import static ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; + +@MockitoSettings +class FhirQueryRuleTesterTest { + private static final Logger ourLog = LoggerFactory.getLogger(FhirQueryRuleTesterTest.class); + FhirQueryRuleTester myTester = new FhirQueryRuleTester("Observation", "code=foo"); + + IAuthRuleTester.RuleTestRequest myTestRequest; + @Mock + IBaseResource myResource; + @Mock + RequestDetails myRequestDetails; + @Mock + IRuleApplier myRuleApplier; + @Mock + IAuthorizationSearchParamMatcher mySearchParamMatcher; + + @BeforeEach + void stubConfig() { + when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); + } + + void stubMatchResult(IAuthorizationSearchParamMatcher.MatchResult result) { + when(mySearchParamMatcher.match("Observation?code=foo", myResource)).thenReturn(result); + } + + private void stubLogForWarning() { + when(myRuleApplier.getTroubleshootingLog()).thenReturn(ourLog); + } + + + @Test + public void matchesFilter_true() { + + myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, + myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); + stubMatchResult(buildMatched()); + + boolean matches = myTester.matchesOutput(myTestRequest); + + assertTrue(matches); + } + + + @Test + public void notMatchesFilter_false() { + //when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); + + myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, + myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); + stubMatchResult(buildUnmatched()); + + boolean matches = myTester.matchesOutput(myTestRequest); + + assertFalse(matches); + } + + @Test + public void unsupportedAllow_false() { + + myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, + myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); + stubMatchResult(buildUnsupported("a message")); + stubLogForWarning(); + + boolean matches = myTester.matchesOutput(myTestRequest); + + assertFalse(matches); + } + + @Test + public void unsupportedDeny_true() { + + myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.DENY, RestOperationTypeEnum.SEARCH_TYPE, + myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); + stubMatchResult(buildUnsupported("a message")); + stubLogForWarning(); + + boolean matches = myTester.matchesOutput(myTestRequest); + + assertTrue(matches); + } + + +}