From afb682dfe9af5e0675259151dfe5c298b9fe4a23 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 15 Aug 2019 09:16:45 -0400 Subject: [PATCH] Handle AuthorizationInterceptor rejection of by-type reads on the wrong type earlier in the process --- .../server/interceptor/auth/RuleImplOp.java | 19 ++--- .../auth/AuthorizationInterceptorR4Test.java | 76 +++++++++++++++++-- src/changes/changes.xml | 5 ++ 3 files changed, 86 insertions(+), 14 deletions(-) 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 3042e4f1521..396250e0d24 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 @@ -412,15 +412,16 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } } if (appliesToResourceType != null) { - if (myAppliesToTypes.contains(appliesToResourceType)) { - if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource)) { - return null; - } - if (myClassifierType == ClassifierTypeEnum.ANY_ID) { - return newVerdict(); - } else if (myClassifierType == ClassifierTypeEnum.IN_COMPARTMENT) { - // ok we'll check below - } + if (!myAppliesToTypes.contains(appliesToResourceType)) { + return null; + } + if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource)) { + return null; + } + if (myClassifierType == ClassifierTypeEnum.ANY_ID) { + return newVerdict(); + } else if (myClassifierType == ClassifierTypeEnum.IN_COMPARTMENT) { + // ok we'll check below } } break; diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java index f67b8a41114..049fd746134 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java @@ -43,10 +43,7 @@ import org.junit.*; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; +import java.util.*; import java.util.concurrent.TimeUnit; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -2136,6 +2133,41 @@ public class AuthorizationInterceptorR4Test { } + + @Test + public void testReadByTypeWithAnyId() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("Rule 1").read().resourcesOfType(ServiceRequest.class).withAnyId().andThen() + .build(); + } + }); + + HttpGet httpGet; + HttpResponse status; + String response; + + ourReturn = Collections.singletonList(new Consent().setDateTime(new Date()).setId("Consent/123")); + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Consent"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + ourReturn = Collections.singletonList(new ServiceRequest().setAuthoredOn(new Date()).setId("ServiceRequest/123")); + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/ServiceRequest"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertTrue(ourHitMethod); + assertEquals(200, status.getStatusLine().getStatusCode()); + + } + + @Test public void testReadByCompartmentReadByIdParam() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @@ -3607,6 +3639,38 @@ public class AuthorizationInterceptorR4Test { } + public static class DummyServiceRequestResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return ServiceRequest.class; + } + + @Search + public List search() { + assert ourReturn != null; + ourHitMethod = true; + return ourReturn; + } + + } + + public static class DummyConsentResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Consent.class; + } + + @Search + public List search() { + assert ourReturn != null; + ourHitMethod = true; + return ourReturn; + } + + } + @SuppressWarnings("unused") public static class DummyPatientResourceProvider implements IResourceProvider { @@ -3825,7 +3889,9 @@ public class AuthorizationInterceptorR4Test { ServletHandler proxyHandler = new ServletHandler(); ourServlet = new RestfulServer(ourCtx); ourServlet.setFhirContext(ourCtx); - ourServlet.setResourceProviders(patProvider, obsProv, encProv, cpProv, orgProv, drProv); + ourServlet.registerProviders(patProvider, obsProv, encProv, cpProv, orgProv, drProv); + ourServlet.registerProvider(new DummyServiceRequestResourceProvider()); + ourServlet.registerProvider(new DummyConsentResourceProvider()); ourServlet.setPlainProviders(plainProvider); ourServlet.setPagingProvider(new FifoMemoryPagingProvider(100)); ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 261ca2b399d..af2f6203f01 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -24,6 +24,11 @@ were incorrectly performing a partial match. This has been corrected. Thanks to Marc Sandberg for pointing this out! + + When using the AuthorizationInterceptor with a rule to allow all reads by resource type, + the server will now reject requests for other resource types earlier in the processing + cycle. Thanks to Anders Havn for the suggestion! +