Implement query filter auth via tester (#3758)

Use tester instead of new subclass to implement fhir filtering.

The subclass was too complicated to be trusted for authorization.
This commit is contained in:
michaelabuckley 2022-07-07 12:15:17 -04:00 committed by GitHub
parent 835ec1b870
commit 7cf447cda1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 284 additions and 252 deletions

View File

@ -38,9 +38,9 @@ public class AuthorizationSearchParamMatcher implements IAuthorizationSearchPara
} }
@Override @Override
public MatchResult match(String theCriteria, IBaseResource theResource) { public MatchResult match(String theQueryParameters, IBaseResource theResource) {
try { try {
InMemoryMatchResult inMemoryMatchResult = mySearchParamMatcher.match(theCriteria, theResource, null); InMemoryMatchResult inMemoryMatchResult = mySearchParamMatcher.match(theQueryParameters, theResource, null);
if (!inMemoryMatchResult.supported()) { if (!inMemoryMatchResult.supported()) {
return MatchResult.buildUnsupported(inMemoryMatchResult.getUnsupportedReason()); return MatchResult.buildUnsupported(inMemoryMatchResult.getUnsupportedReason());
} }
@ -54,7 +54,7 @@ public class AuthorizationSearchParamMatcher implements IAuthorizationSearchPara
// it assumes it is during SearchParameter storage. // it assumes it is during SearchParameter storage.
// Instead, we adapt this to UNSUPPORTED during authorization. // Instead, we adapt this to UNSUPPORTED during authorization.
// We may be applying to all types, and this filter won't match. // We may be applying to all types, and this filter won't match.
ourLog.info("Unsupported filter {} applied to resource: {}", theCriteria, e.getMessage()); ourLog.info("Unsupported filter {} applied to resource: {}", theQueryParameters, e.getMessage());
return MatchResult.buildUnsupported(e.getMessage()); return MatchResult.buildUnsupported(e.getMessage());
} }
} }

View File

@ -2,16 +2,20 @@ package ca.uhn.fhir.jpa.auth;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor; import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor;
import ca.uhn.fhir.rest.server.interceptor.auth.FhirQueryRuleImpl; import ca.uhn.fhir.rest.server.interceptor.auth.FhirQueryRuleTester;
import ca.uhn.fhir.rest.server.interceptor.auth.IAuthRule;
import ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher; import ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher;
import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum; import ca.uhn.fhir.rest.server.interceptor.auth.PolicyEnum;
import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder; import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder;
import ca.uhn.fhir.test.utilities.ITestDataBuilder; import ca.uhn.fhir.test.utilities.ITestDataBuilder;
import ca.uhn.test.util.LogbackCaptureTestExtension; import ca.uhn.test.util.LogbackCaptureTestExtension;
import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Level;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
@ -25,15 +29,16 @@ import org.mockito.junit.jupiter.MockitoSettings;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import java.util.HashSet; import java.util.HashSet;
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.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
// wipjv where should this test live? - // wipjv where should this test live? - It can't live in hapi-fhir-server since we need a real FhirContext for the compartment checks.
// wipjv can we mock the resource? We just use it for stubbing here. If so, move this back to hapi-fhir-server ca.uhn.fhir.rest.server.interceptor.auth
@MockitoSettings @MockitoSettings
class FhirQueryRuleImplTest implements ITestDataBuilder { class FhirQueryRuleImplTest implements ITestDataBuilder {
@ -47,57 +52,41 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
@RegisterExtension @RegisterExtension
LogbackCaptureTestExtension myLogCapture = new LogbackCaptureTestExtension(myMockRuleApplier.getTroubleshootingLog().getName()); LogbackCaptureTestExtension myLogCapture = new LogbackCaptureTestExtension(myMockRuleApplier.getTroubleshootingLog().getName());
private FhirQueryRuleImpl myRule; private IAuthRule myRule;
private IBaseResource myResource; IIdType myPatientId = new IdDt("Patient/1");
private IBaseResource myResource2; private IBaseResource myPatient;
//@Mock private IBaseResource myObservation;
private final SystemRequestDetails mySrd = new SystemRequestDetails(); @Mock
private RequestDetails myRequestDetails;
private final FhirContext myFhirContext = FhirContext.forR4Cached(); private final FhirContext myFhirContext = FhirContext.forR4Cached();
@Mock @Mock
private IAuthorizationSearchParamMatcher myMatcher; private IAuthorizationSearchParamMatcher myMatcher;
@BeforeEach @BeforeEach
public void setUp() { public void setUp() {
mySrd.setFhirContext(myFhirContext); when(myRequestDetails.getFhirContext()).thenReturn(myFhirContext);
} }
// // our IRuleApplierStubs
// @Override
// public Logger getTroubleshootingLog() {
// return ourTargetLog;
// }
//
// public IAuthorizationSearchParamMatcher getSearchParamMatcher() {
// return myMatcher;
// }
//
// @Override
// public AuthorizationInterceptor.Verdict applyRulesAndReturnDecision(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Pointcut thePointcut) {
// return null;
// }
@Nested @Nested
public class MatchingLogic { public class MatchingLogic {
@Test @Test
public void simpleStringSearch_whenMatchResource_allow() { public void typeWithFilter_whenMatch_allow() {
// given // given
withPatientWithNameAndId(); withPatientWithNameAndId();
RuleBuilder b = new RuleBuilder(); RuleBuilder b = new RuleBuilder();
myRule = (FhirQueryRuleImpl) b.allow() myRule = b.allow()
.read() .read()
.resourcesOfType("Patient") .resourcesOfType("Patient")
.withFilter( "family=Smith") .withFilter( "family=Smith")
.andThen().build().get(0); .andThen().build().get(0);
when(myMatcher.match(ArgumentMatchers.eq("Patient?family=Smith"), ArgumentMatchers.same(myResource))) stubMatcherCall("Patient?family=Smith", myPatient, buildMatched());
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildMatched());
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then // then
assertThat(verdict, notNullValue()); assertThat(verdict, notNullValue());
@ -105,38 +94,74 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
} }
@Test @Test
public void simpleStringSearch_noMatch_noVerdict() { public void anyTypewithQueryFilter_whenMatch_allow() {
// given // given
withPatientWithNameAndId(); withPatientWithNameAndId();
myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Patient")
.inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi") RuleBuilder b = new RuleBuilder();
myRule = b.allow()
.read()
.allResources()
.withFilter( "family=Smith")
.andThen().build().get(0); .andThen().build().get(0);
when(myMatcher.match(ArgumentMatchers.eq("Patient?family=smi"), ArgumentMatchers.same(myResource)))
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched()); stubMatcherCall("Patient?family=Smith", myPatient, buildMatched());
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then
assertThat(verdict, notNullValue());
assertThat(verdict.getDecision(), equalTo(PolicyEnum.ALLOW));
}
@Test
public void typeWithQuery_noQueryMatch_noVerdict() {
// given
withPatientWithNameAndId();
myRule = new RuleBuilder().allow().read().resourcesOfType("Patient")
.withFilter( "family=smi")
.andThen().build().get(0);
stubMatcherCall("Patient?family=smi", myPatient, buildUnmatched());
// when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then // then
assertThat(verdict, nullValue()); assertThat(verdict, nullValue());
} }
@Test @Test
public void observation_notInCompartmentMatchFilter_noVerdict() { public void typeWithQuery_wrongType_noVerdict() {
// given
withPatientWithNameAndId();
withObservationWithSubjectAndCode(myPatientId);
myRule = new RuleBuilder().allow().read().resourcesOfType("Patient")
.withFilter( "family=smi")
.andThen().build().get(0);
//stubMatcherCall("Patient?family=smi", myPatient, buildUnmatched());
// when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myObservation);
// then
assertThat(verdict, nullValue());
}
@Test
public void inCompartmentwithQueryFilter_resourceNotInCompartmentButMatchFilter_noVerdict() {
// given // given
withPatientWithNameAndId(); withPatientWithNameAndId();
// create patient for observation to point to so that the observation isn't in our main patient compartment // create patient for observation to point to so that the observation isn't in our main patient compartment
IBaseResource patient = buildResource("Patient", withFamily("Jones"), withId("bad-id")); withObservationWithSubjectAndCode(myPatient.getIdElement());
withObservationWithSubjectAndCode(patient.getIdElement());
myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Observation") myRule = new RuleBuilder().allow().read().resourcesOfType("Observation")
.inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "code=28521000087105") .inCompartmentWithFilter("patient", myPatient.getIdElement().withResourceType("Patient"), "code=28521000087105")
.andThen().build().get(0); .andThen().build().get(0);
when(myMatcher.match("Observation?code=28521000087105", myResource2)) // matcher won't be called since not in compartment
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched());
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myObservation);
// then // then
assertThat(verdict, nullValue()); assertThat(verdict, nullValue());
@ -146,41 +171,88 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
public void observation_noMatchFilter_noVerdict() { public void observation_noMatchFilter_noVerdict() {
// given // given
withPatientWithNameAndId(); withPatientWithNameAndId();
withObservationWithSubjectAndCode(myResource.getIdElement()); withObservationWithSubjectAndCode(myPatient.getIdElement());
myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Observation") myRule = new RuleBuilder().allow().read().resourcesOfType("Observation")
.withFilter("code=12") .withFilter("code=12")
.andThen().build().get(0); .andThen().build().get(0);
when(myMatcher.match("Observation?code=12", myResource2))
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnmatched()); stubMatcherCall("Observation?code=12", myObservation, buildUnmatched());
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myObservation);
// then // then
assertThat(verdict, nullValue()); assertThat(verdict, nullValue());
} }
@Test @Test
public void observation_denyWithFilter_deny() { public void denyTypeWithQueryFilter_match_deny() {
// given // given
withPatientWithNameAndId(); withPatientWithNameAndId();
withObservationWithSubjectAndCode(myResource.getIdElement()); withObservationWithSubjectAndCode(myPatient.getIdElement());
myRule = (FhirQueryRuleImpl) new RuleBuilder().deny().read().resourcesOfType("Observation") myRule = new RuleBuilder().deny().read().resourcesOfType("Observation")
.withFilter("code=28521000087105") .withFilter("code=28521000087105")
.andThen().build().get(0); .andThen().build().get(0);
when(myMatcher.match("Observation?code=28521000087105", myResource2)) stubMatcherCall("Observation?code=28521000087105", myObservation, buildMatched());
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildMatched());
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource2); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myObservation);
// then // then
assertThat(verdict, notNullValue()); assertThat(verdict, notNullValue());
assertThat(verdict.getDecision(), equalTo(PolicyEnum.DENY)); assertThat(verdict.getDecision(), equalTo(PolicyEnum.DENY));
} }
@Test
public void allowIdwithQueryFilter_matchesIdAndFilter_allow() {
// given
withPatientWithNameAndId();
myRule = new RuleBuilder()
.allow()
.read().instance(myPatient.getIdElement())
.withTester(new FhirQueryRuleTester("name=smith"))
.andThen().build().get(0);
stubMatcherCall("Patient?name=smith", myPatient, buildMatched());
// when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then
assertThat(verdict, notNullValue());
assertThat(verdict.getDecision(), equalTo(PolicyEnum.ALLOW));
}
@Test
public void allowIdwithQueryFilter_matchesJustIdNotFilter_abstain() {
// given
withPatientWithNameAndId();
myRule = new RuleBuilder()
.allow()
.read().instance(myPatient.getIdElement())
.withTester(new FhirQueryRuleTester("name=smith"))
.andThen().build().get(0);
stubMatcherCall("Patient?name=smith", myPatient, buildUnmatched());
// when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then
assertThat(verdict, nullValue());
}
}
private void stubMatcherCall(String expectedQuery, IBaseResource theTargetResource, IAuthorizationSearchParamMatcher.MatchResult theStubResult) {
when(myMatcher.match(ArgumentMatchers.eq(expectedQuery), ArgumentMatchers.same(theTargetResource)))
.thenReturn(theStubResult);
} }
@ -197,35 +269,33 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
@Test @Test
public void givenAllowRule_whenUnsupportedQuery_noVerdict() { public void givenAllowRule_whenUnsupportedQuery_noVerdict() {
withPatientWithNameAndId(); withPatientWithNameAndId();
myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Patient") myRule = new RuleBuilder().allow().read().resourcesOfType("Patient")
.inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi").andThen().build().get(0); .inCompartmentWithFilter("patient", myPatient.getIdElement().withResourceType("Patient"), "unsupported.chain=smi").andThen().build().get(0);
when(myMatcher.match("Patient?family=smi", myResource)) stubMatcherCall("Patient?unsupported.chain=smi", myPatient, buildUnsupported("I'm broken unsupported chain XXX"));
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported("I'm broken unsupported chain XXX"));
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then // then
assertThat(verdict, nullValue()); assertThat(verdict, nullValue());
assertThat(myLogCapture.getLogEvents(), MatcherAssert.assertThat(myLogCapture.getLogEvents(),
hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "unsupported chain XXX"))); Matchers.hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "unsupported chain XXX")));
} }
@Test @Test
public void givenDenyRule_whenUnsupportedQuery_reject() { public void givenDenyRule_whenUnsupportedQuery_reject() {
withPatientWithNameAndId(); withPatientWithNameAndId();
myRule = (FhirQueryRuleImpl) new RuleBuilder().deny().read().resourcesOfType("Patient") myRule = new RuleBuilder().deny().read().resourcesOfType("Patient")
.inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Patient"), "family=smi").andThen().build().get(0); .inCompartmentWithFilter("patient", myPatientId, "unsupported.chain=smi").andThen().build().get(0);
when(myMatcher.match("Patient?family=smi", myResource)) stubMatcherCall("Patient?unsupported.chain=smi", myPatient, buildUnsupported("I'm broken unsupported chain XXX"));
.thenReturn(IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported("I'm broken unsupported chain XXX"));
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then // then
assertThat(verdict.getDecision(), equalTo(PolicyEnum.DENY)); assertThat(verdict.getDecision(), equalTo(PolicyEnum.DENY));
assertThat(myLogCapture.getLogEvents(), MatcherAssert.assertThat(myLogCapture.getLogEvents(),
hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "unsupported chain XXX"))); Matchers.hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "unsupported chain XXX")));
} }
/** /**
@ -236,16 +306,16 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
public void noMatcherService_unsupportedPerm_noVerdict() { public void noMatcherService_unsupportedPerm_noVerdict() {
withPatientWithNameAndId(); withPatientWithNameAndId();
myMatcher = null; myMatcher = null;
myRule = (FhirQueryRuleImpl) new RuleBuilder().allow().read().resourcesOfType("Patient") myRule = new RuleBuilder().allow().read().resourcesOfType("Patient")
.inCompartmentWithFilter("patient", myResource.getIdElement().withResourceType("Observation"), "code:in=foo").andThen().build().get(0); .inCompartmentWithFilter("patient", myPatient.getIdElement().withResourceType("Patient"), "code:in=foo").andThen().build().get(0);
// when // when
AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myResource); AuthorizationInterceptor.Verdict verdict = applyRuleToResource(myPatient);
// then // then
assertThat(verdict, nullValue()); assertThat(verdict, nullValue());
assertThat(myLogCapture.getLogEvents(), MatcherAssert.assertThat(myLogCapture.getLogEvents(),
hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "No matcher provided"))); Matchers.hasItem(myLogCapture.eventWithLevelAndMessageContains(Level.WARN, "No matcher provided")));
} }
} }
@ -253,20 +323,21 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
// We need the builder to set AppliesTypeEnum, and the use that to build the matcher expression. // We need the builder to set AppliesTypeEnum, and the use that to build the matcher expression.
private AuthorizationInterceptor.Verdict applyRuleToResource(IBaseResource theResource) { private AuthorizationInterceptor.Verdict applyRuleToResource(IBaseResource theResource) {
return myRule.applyRule(RestOperationTypeEnum.SEARCH_TYPE, mySrd, null, null, theResource, myMockRuleApplier, new HashSet<>(), Pointcut.STORAGE_PRESHOW_RESOURCES); return myRule.applyRule(RestOperationTypeEnum.SEARCH_TYPE, myRequestDetails, null, null, theResource, myMockRuleApplier, new HashSet<>(), Pointcut.STORAGE_PRESHOW_RESOURCES);
} }
private void withPatientWithNameAndId() { private void withPatientWithNameAndId() {
myResource = buildResource("Patient", withFamily("Smith"), withId("some-id")); myPatient = buildPatient(withId(myPatientId));
} }
// Use in sequence with above // Use in sequence with above
private void withObservationWithSubjectAndCode(IIdType theIdElement) { private void withObservationWithSubjectAndCode(IIdType theIdElement) {
String snomedUriString = "http://snomed.info/sct"; String snomedUriString = "https://snomed.info/sct";
String insulin2hCode = "28521000087105"; String insulin2hCode = "28521000087105";
myResource2 = buildResource("Observation", withObservationCode(snomedUriString, insulin2hCode), withSubject(theIdElement)); myObservation = buildResource("Observation", withObservationCode(snomedUriString, insulin2hCode), withSubject(theIdElement));
} }
@Override @Override
public IIdType doCreateResource(IBaseResource theResource) { public IIdType doCreateResource(IBaseResource theResource) {
return null; return null;
@ -279,6 +350,7 @@ class FhirQueryRuleImplTest implements ITestDataBuilder {
@Override @Override
public FhirContext getFhirContext() { public FhirContext getFhirContext() {
return FhirContext.forR4Cached(); return myFhirContext;
} }
} }

View File

@ -1765,8 +1765,9 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
@Override @Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) { public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder() return new RuleBuilder()
.allow("filter rule").read().allResources().withFilter("code=" + FhirResourceDaoR4TerminologyTest.URL_MY_CODE_SYSTEM + "|").andThen() .allow("filter rule").read().allResources().withAnyId()
.build(); .withFilterTester("code=" + FhirResourceDaoR4TerminologyTest.URL_MY_CODE_SYSTEM + "|")
.andThen().build();
} }
}; };
interceptor.setAuthorizationSearchParamMatcher(new AuthorizationSearchParamMatcher(mySearchParamMatcher)); interceptor.setAuthorizationSearchParamMatcher(new AuthorizationSearchParamMatcher(mySearchParamMatcher));

View File

@ -25,6 +25,8 @@ import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict; import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IIdType;
@ -112,4 +114,16 @@ abstract class BaseRule implements IAuthRule {
return thePointcut.equals(Pointcut.STORAGE_PREACCESS_RESOURCES) || thePointcut.equals(Pointcut.STORAGE_PRESHOW_RESOURCES); return thePointcut.equals(Pointcut.STORAGE_PREACCESS_RESOURCES) || thePointcut.equals(Pointcut.STORAGE_PRESHOW_RESOURCES);
} }
@Override
public String toString() {
ToStringBuilder builder = toStringBuilder();
return builder.toString();
}
ToStringBuilder toStringBuilder() {
ToStringBuilder builder = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
builder.append("testers", myTesters);
return builder;
}
} }

View File

@ -1,110 +0,0 @@
package ca.uhn.fhir.rest.server.interceptor.auth;
/*-
* #%L
* HAPI FHIR - Server Framework
* %%
* Copyright (C) 2014 - 2022 Smile CDR, Inc.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.annotation.Nonnull;
import java.util.Set;
/**
* Extension to rules that also requires matching a query filter, e.g. code=foo
*/
public class FhirQueryRuleImpl extends RuleImplOp {
private static final Logger ourLog = LoggerFactory.getLogger(FhirQueryRuleImpl.class);
private String myFilter;
/**
* Constructor
*/
public FhirQueryRuleImpl(String theRuleName) {
super(theRuleName);
}
public void setFilter(String theFilter) {
myFilter = theFilter;
}
public String getFilter() {
return myFilter;
}
/**
* Our override that first checks our filter against the resource if present.
*/
@Override
protected AuthorizationInterceptor.Verdict applyRuleLogic(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource, Set<AuthorizationFlagsEnum> theFlags, FhirContext theFhirContext, RuleTarget theRuleTarget, IRuleApplier theRuleApplier) {
ourLog.trace("applyRuleLogic {} {}", theOperation, theRuleTarget);
// Note - theOutputResource == null means we're in some other pointcut and don't have a result yet.
if (theOutputResource == null) {
return super.applyRuleLogic(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theFlags, theFhirContext, theRuleTarget, theRuleApplier);
}
// look for a matcher
IAuthorizationSearchParamMatcher matcher = theRuleApplier.getSearchParamMatcher();
if (matcher == null) {
theRuleApplier.getTroubleshootingLog().warn("No matcher provided. Can't apply filter permission.");
if ( PolicyEnum.DENY.equals(getMode())) {
return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this);
}
return null;
}
// wipjv check in vs out resource - write will need to check write.
// wipjv what about the id case - does that path doesn't call applyRuleLogic()
IAuthorizationSearchParamMatcher.MatchResult mr = matcher.match(theOutputResource.fhirType() + "?" + myFilter, theOutputResource);
AuthorizationInterceptor.Verdict result;
switch (mr.getMatch()) {
case MATCH:
result = super.applyRuleLogic(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theFlags, theFhirContext, theRuleTarget, theRuleApplier);
break;
case UNSUPPORTED:
theRuleApplier.getTroubleshootingLog().warn("Unsupported matcher expression {}: {}. Abstaining.", myFilter, mr.getUnsupportedReason());
if ( PolicyEnum.DENY.equals(getMode())) {
result = new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this);
} else {
result = null;
}
break;
case NO_MATCH:
default:
result = null;
break;
}
return result;
}
@Nonnull
@Override
protected ToStringBuilder toStringBuilder() {
return super.toStringBuilder()
.append("filter", myFilter);
}
}

View File

@ -1,37 +1,52 @@
package ca.uhn.fhir.rest.server.interceptor.auth; package ca.uhn.fhir.rest.server.interceptor.auth;
public class FhirQueryRuleTester implements IAuthRuleTester { import org.apache.commons.lang3.builder.ToStringBuilder;
private final String myType; import org.apache.commons.lang3.builder.ToStringStyle;
private final String myFilter;
public FhirQueryRuleTester(String theType, String theFilter) { /**
myType = theType; * Tester that a resource matches a provided query filter.
myFilter = theFilter; */
public class FhirQueryRuleTester implements IAuthRuleTester {
private final String myQueryParameters;
public FhirQueryRuleTester(String theQueryParameters) {
myQueryParameters = theQueryParameters;
} }
@Override @Override
public boolean matches(RuleTestRequest theRuleTestRequest) { public boolean matches(RuleTestRequest theRuleTestRequest) {
// wipmb placeholder until we get to writes return checkMatch(theRuleTestRequest);
return true;
} }
@Override @Override
public boolean matchesOutput(RuleTestRequest theRuleTestRequest) { public boolean matchesOutput(RuleTestRequest theRuleTestRequest) {
return checkMatch(theRuleTestRequest);
}
private boolean checkMatch(RuleTestRequest theRuleTestRequest) {
// look for a matcher // look for a matcher
IAuthorizationSearchParamMatcher matcher = theRuleTestRequest.ruleApplier.getSearchParamMatcher(); IAuthorizationSearchParamMatcher matcher = theRuleTestRequest.ruleApplier.getSearchParamMatcher();
if (matcher == null) { if (matcher == null) {
theRuleTestRequest.ruleApplier.getTroubleshootingLog().warn("No matcher provided. Can't apply filter permission.");
return false; return false;
} }
// wipmb myType, or target type? Is this just query filter, or do we bring * into it? // this is a bit weird.
IAuthorizationSearchParamMatcher.MatchResult mr = matcher.match(myType + "?" + myFilter, theRuleTestRequest.resource); // A tester narrows a rule -- i.e. a rule only applies if the main logic matches AND the testers all match
// But this rule would have matched without this tester, so true means abstain.
if (theRuleTestRequest.resource == null) {
// we aren't looking at a resource yet. treat as no-op
return true;
}
switch (mr.getMatch()) { // we use the target type since the rule might apply to all types, a type set, or instances, and that has already been checked.
IAuthorizationSearchParamMatcher.MatchResult mr = matcher.match(theRuleTestRequest.resource.fhirType() + "?" + myQueryParameters, theRuleTestRequest.resource);
switch (mr.match) {
case MATCH: case MATCH:
return true; return true;
case UNSUPPORTED: case UNSUPPORTED:
theRuleTestRequest.ruleApplier.getTroubleshootingLog().warn("Unsupported matcher expression {}: {}.", myFilter, mr.getUnsupportedReason()); theRuleTestRequest.ruleApplier.getTroubleshootingLog().warn("Unsupported matcher expression {}: {}.", myQueryParameters, mr.unsupportedReason);
// unsupported doesn't match unless this is a deny request, and we need to be safe! // unsupported doesn't match unless this is a deny request, and we need to be safe!
return (theRuleTestRequest.mode == PolicyEnum.DENY); return (theRuleTestRequest.mode == PolicyEnum.DENY);
case NO_MATCH: case NO_MATCH:
@ -39,4 +54,11 @@ public class FhirQueryRuleTester implements IAuthRuleTester {
return false; return false;
} }
} }
@Override
public String toString() {
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
.append("filter", myQueryParameters)
.toString();
}
} }

View File

@ -51,4 +51,10 @@ public interface IAuthRuleFinished {
*/ */
IAuthRuleFinished withTester(@Nullable IAuthRuleTester theTester); IAuthRuleFinished withTester(@Nullable IAuthRuleTester theTester);
/**
* Narrow this rule to resources matching the given FHIR query.
* @param theQueryParameters a FHIR query parameter string. E.g. category=laboratory&date=ge2021
*/
IAuthRuleFinished withFilterTester(String theQueryParameters);
} }

View File

@ -78,12 +78,18 @@ public interface IAuthRuleTester {
} }
/** /**
* Allows user-supplied logic for authorization rules. * User supplied logic called just before the parent rule renders a verdict on the operation
* or input resource.
*
* Returning true will allow the verdict continue.
* Returning false will block the verdict and cause the rule to abstain (i.e. return null).
*
* <p> * <p>
* THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API * THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API
* may change. * may change.
* *
* @param theRequest The details to evaluate * @param theRequest The details of the operation or an INPUT resource to evaluate
* @return true if the verdict should continue
* @since 6.1.0 * @since 6.1.0
*/ */
default boolean matches(RuleTestRequest theRequest) { default boolean matches(RuleTestRequest theRequest) {
@ -101,14 +107,18 @@ public interface IAuthRuleTester {
} }
/** /**
* Allows user-supplied logic for authorization rules. * User supplied logic called just before the parent rule renders a verdict on an output resource.
*
* Returning true will allow the verdict continue.
* Returning false will block the verdict and cause the rule to abstain (i.e. return null).
*
* <p> * <p>
* THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API * THIS IS AN EXPERIMENTAL API! Feedback is welcome, and this API
* may change. * may change.
* *
* @param theRequest The details to evaluate * @param theRequest The details of the operation or an INPUT resource to evaluate
* @since 6.1.0 * @return true if the verdict should continue
*/ * @since 6.1.0 */
default boolean matchesOutput(RuleTestRequest theRequest) { default boolean matchesOutput(RuleTestRequest theRequest) {
return this.matchesOutput(theRequest.operation, theRequest.requestDetails, theRequest.resource); return this.matchesOutput(theRequest.operation, theRequest.requestDetails, theRequest.resource);
} }

View File

@ -26,10 +26,17 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
/** /**
* Adapt the InMemoryMatcher to support authorization filters in {@link FhirQueryRuleImpl}. * Adapt the InMemoryMatcher to support authorization filters in {@link FhirQueryRuleTester}.
* Exists because filters may be applied to resources that don't support all paramters, and UNSUPPORTED
* has a different meaning during authorization.
*/ */
public interface IAuthorizationSearchParamMatcher { public interface IAuthorizationSearchParamMatcher {
MatchResult match(String theCriteria, IBaseResource theResource); /**
* Calculate if the resource would match the fhir query parameters.
* @param theQueryParameters e.g. "category=laboratory"
* @param theResource the target of the comparison
*/
MatchResult match(String theQueryParameters, IBaseResource theResource);
/** /**
* Match outcomes. * Match outcomes.
@ -42,9 +49,11 @@ public interface IAuthorizationSearchParamMatcher {
} }
class MatchResult { class MatchResult {
// wipmb consider a record pattern - public and drop the accessors. // fake record pattern
@Nonnull private final Match myMatch; /** match result */
@Nullable private final String myUnsupportedReason; @Nonnull public final Match match;
/** the reason for the UNSUPPORTED result */
@Nullable public final String unsupportedReason;
public static MatchResult buildMatched() { public static MatchResult buildMatched() {
return new MatchResult(Match.MATCH, null); return new MatchResult(Match.MATCH, null);
@ -59,16 +68,8 @@ public interface IAuthorizationSearchParamMatcher {
} }
private MatchResult(Match myMatch, String myUnsupportedReason) { private MatchResult(Match myMatch, String myUnsupportedReason) {
this.myMatch = myMatch; this.match = myMatch;
this.myUnsupportedReason = myUnsupportedReason; this.unsupportedReason = myUnsupportedReason;
}
public Match getMatch() {
return myMatch;
}
public String getUnsupportedReason() {
return myUnsupportedReason;
} }
} }

View File

@ -185,6 +185,11 @@ public class RuleBuilder implements IAuthRuleBuilder {
return this; return this;
} }
@Override
public IAuthRuleFinished withFilterTester(String theQueryParameters) {
return withTester(new FhirQueryRuleTester(theQueryParameters));
}
private class TenantCheckingTester implements IAuthRuleTester { private class TenantCheckingTester implements IAuthRuleTester {
private final Collection<String> myTenantIds; private final Collection<String> myTenantIds;
private final boolean myOutcome; private final boolean myOutcome;
@ -600,17 +605,19 @@ public class RuleBuilder implements IAuthRuleBuilder {
} }
myInCompartmentOwners = Collections.singletonList(theIdElement); myInCompartmentOwners = Collections.singletonList(theIdElement);
FhirQueryRuleImpl rule = new FhirQueryRuleImpl(myRuleName); RuleBuilderFinished result = finished();
rule.setFilter(theFilter); result.withTester(new FhirQueryRuleTester(theFilter));
return finished(rule); return result;
} }
@Override @Override
public IAuthRuleFinished withFilter(String theFilter) { public IAuthRuleFinished withFilter(String theFilter) {
myClassifierType = ClassifierTypeEnum.ANY_ID; myClassifierType = ClassifierTypeEnum.ANY_ID;
FhirQueryRuleImpl rule = new FhirQueryRuleImpl(myRuleName);
rule.setFilter(theFilter); RuleBuilderFinished result = finished();
return finished(rule); result.withTester(new FhirQueryRuleTester(theFilter));
return result;
} }
RuleBuilderFinished addInstances(Collection<IIdType> theInstances) { RuleBuilderFinished addInstances(Collection<IIdType> theInstances) {

View File

@ -675,15 +675,9 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
} }
@Override
public String toString() {
ToStringBuilder builder = toStringBuilder();
return builder.toString();
}
@Nonnull @Nonnull
protected ToStringBuilder toStringBuilder() { protected ToStringBuilder toStringBuilder() {
ToStringBuilder builder = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE); ToStringBuilder builder = super.toStringBuilder();
builder.append("op", myOp); builder.append("op", myOp);
builder.append("transactionAppliesToOp", myTransactionAppliesToOp); builder.append("transactionAppliesToOp", myTransactionAppliesToOp);
builder.append("appliesTo", myAppliesTo); builder.append("appliesTo", myAppliesTo);

View File

@ -16,16 +16,18 @@ import static ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParam
import static ca.uhn.fhir.rest.server.interceptor.auth.IAuthorizationSearchParamMatcher.MatchResult.buildUnsupported; 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.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@MockitoSettings @MockitoSettings
class FhirQueryRuleTesterTest { class FhirQueryRuleTesterTest {
private static final Logger ourLog = LoggerFactory.getLogger(FhirQueryRuleTesterTest.class); private static final Logger ourLog = LoggerFactory.getLogger(FhirQueryRuleTesterTest.class);
FhirQueryRuleTester myTester = new FhirQueryRuleTester("Observation", "code=foo"); FhirQueryRuleTester myTester = new FhirQueryRuleTester("code=foo");
IAuthRuleTester.RuleTestRequest myTestRequest; IAuthRuleTester.RuleTestRequest myTestRequest;
@Mock @Mock
IBaseResource myResource; IBaseResource myObservation;
@Mock @Mock
RequestDetails myRequestDetails; RequestDetails myRequestDetails;
@Mock @Mock
@ -35,11 +37,13 @@ class FhirQueryRuleTesterTest {
@BeforeEach @BeforeEach
void stubConfig() { void stubConfig() {
when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher); lenient().when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher);
lenient().when(myObservation.fhirType()).thenReturn("Observation");
} }
void stubMatchResult(IAuthorizationSearchParamMatcher.MatchResult result) { void stubMatchResult(IAuthorizationSearchParamMatcher.MatchResult result) {
when(mySearchParamMatcher.match("Observation?code=foo", myResource)).thenReturn(result); when(mySearchParamMatcher.match("Observation?code=foo", myObservation)).thenReturn(result);
} }
private void stubLogForWarning() { private void stubLogForWarning() {
@ -51,7 +55,7 @@ class FhirQueryRuleTesterTest {
public void matchesFilter_true() { public void matchesFilter_true() {
myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE,
myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); myRequestDetails, new IdDt("Observation/1"), myObservation, myRuleApplier);
stubMatchResult(buildMatched()); stubMatchResult(buildMatched());
boolean matches = myTester.matchesOutput(myTestRequest); boolean matches = myTester.matchesOutput(myTestRequest);
@ -62,10 +66,9 @@ class FhirQueryRuleTesterTest {
@Test @Test
public void notMatchesFilter_false() { public void notMatchesFilter_false() {
//when(myRuleApplier.getSearchParamMatcher()).thenReturn(mySearchParamMatcher);
myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE,
myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); myRequestDetails, new IdDt("Observation/1"), myObservation, myRuleApplier);
stubMatchResult(buildUnmatched()); stubMatchResult(buildUnmatched());
boolean matches = myTester.matchesOutput(myTestRequest); boolean matches = myTester.matchesOutput(myTestRequest);
@ -77,7 +80,7 @@ class FhirQueryRuleTesterTest {
public void unsupportedAllow_false() { public void unsupportedAllow_false() {
myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE, myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.ALLOW, RestOperationTypeEnum.SEARCH_TYPE,
myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); myRequestDetails, new IdDt("Observation/1"), myObservation, myRuleApplier);
stubMatchResult(buildUnsupported("a message")); stubMatchResult(buildUnsupported("a message"));
stubLogForWarning(); stubLogForWarning();
@ -90,7 +93,7 @@ class FhirQueryRuleTesterTest {
public void unsupportedDeny_true() { public void unsupportedDeny_true() {
myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.DENY, RestOperationTypeEnum.SEARCH_TYPE, myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.DENY, RestOperationTypeEnum.SEARCH_TYPE,
myRequestDetails, new IdDt("Observation/1"), myResource, myRuleApplier); myRequestDetails, new IdDt("Observation/1"), myObservation, myRuleApplier);
stubMatchResult(buildUnsupported("a message")); stubMatchResult(buildUnsupported("a message"));
stubLogForWarning(); stubLogForWarning();
@ -99,5 +102,17 @@ class FhirQueryRuleTesterTest {
assertTrue(matches); assertTrue(matches);
} }
@Test
public void preHandledCheckHasNoResource_true() {
myTestRequest = new IAuthRuleTester.RuleTestRequest(PolicyEnum.DENY, RestOperationTypeEnum.READ,
myRequestDetails, null, null, myRuleApplier);
// no stubs needed since we don't have a resource
boolean matches = myTester.matchesOutput(myTestRequest);
assertTrue(matches);
}
} }

View File

@ -21,7 +21,7 @@ public class RuleImplOpTest {
@Test @Test
public void testToString() { public void testToString() {
assertEquals("RuleImplOp[op=<null>,transactionAppliesToOp=<null>,appliesTo=<null>,appliesToTypes=<null>,classifierCompartmentName=<null>,classifierCompartmentOwners=<null>,classifierType=<null>]", new RuleImplOp("").toString()); assertEquals("RuleImplOp[testers=<null>,op=<null>,transactionAppliesToOp=<null>,appliesTo=<null>,appliesToTypes=<null>,classifierCompartmentName=<null>,classifierCompartmentOwners=<null>,classifierType=<null>]", new RuleImplOp("").toString());
} }
@Test @Test