diff --git a/example-projects/hapi-fhir-jpaserver-dynamic/src/main/java/ca/uhn/fhir/jpa/demo/FhirServerConfigCommon.java b/example-projects/hapi-fhir-jpaserver-dynamic/src/main/java/ca/uhn/fhir/jpa/demo/FhirServerConfigCommon.java index 76cd73dc002..5aa2bb96a1c 100644 --- a/example-projects/hapi-fhir-jpaserver-dynamic/src/main/java/ca/uhn/fhir/jpa/demo/FhirServerConfigCommon.java +++ b/example-projects/hapi-fhir-jpaserver-dynamic/src/main/java/ca/uhn/fhir/jpa/demo/FhirServerConfigCommon.java @@ -82,6 +82,10 @@ public class FhirServerConfigCommon { logger.error("----FhiServerConfigCommon: getDataSource: setting driver error: " + e.getMessage()); } dataSource.setUrl(dbUrl); + + // A check for WS-2020-0287 + assert dataSource.getJmxName() == null; + return dataSource; } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2648-consent-on-metadata.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2648-consent-on-metadata.yaml new file mode 100644 index 00000000000..4896f5333f7 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2648-consent-on-metadata.yaml @@ -0,0 +1,6 @@ +--- +type: change +issue: 2648 +title: "The ConsentInterceptor no longer fully runs on calls to `/metadata` or during the `$meta` operation." + + diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java index 3c71aa69964..e3ffc87b834 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java @@ -122,6 +122,9 @@ public enum DriverTypeEnum { dataSource.setUsername(theUsername); dataSource.setPassword(thePassword); + // A check for WS-2020-0287 + assert dataSource.getJmxName() == null; + return newConnectionProperties(dataSource); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/consent/ConsentInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/consent/ConsentInterceptor.java index cce264c81a6..410679e6ab2 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/consent/ConsentInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/consent/ConsentInterceptor.java @@ -44,6 +44,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import static ca.uhn.fhir.rest.api.Constants.URL_TOKEN_METADATA; + @Interceptor public class ConsentInterceptor { private static final AtomicInteger ourInstanceCount = new AtomicInteger(0); @@ -51,6 +53,7 @@ public class ConsentInterceptor { private final String myRequestAuthorizedKey = ConsentInterceptor.class.getName() + "_" + myInstanceIndex + "_AUTHORIZED"; private final String myRequestCompletedKey = ConsentInterceptor.class.getName() + "_" + myInstanceIndex + "_COMPLETED"; private final String myRequestSeenResourcesKey = ConsentInterceptor.class.getName() + "_" + myInstanceIndex + "_SEENRESOURCES"; + public static final String META_OPERATION_NAME = "$meta"; private IConsentService myConsentService; private IConsentContextServices myContextConsentServices; @@ -94,6 +97,9 @@ public class ConsentInterceptor { @Hook(value = Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED) public void interceptPreHandled(RequestDetails theRequestDetails) { + if (isAllowListedRequest(theRequestDetails)) { + return; + } ConsentOutcome outcome = myConsentService.startOperation(theRequestDetails, myContextConsentServices); Validate.notNull(outcome, "Consent service returned null outcome"); @@ -129,6 +135,9 @@ public class ConsentInterceptor { if (isRequestAuthorized(theRequestDetails)) { return; } + if (isAllowListedRequest(theRequestDetails)) { + return; + } for (int i = 0; i < thePreResourceAccessDetails.size(); i++) { IBaseResource nextResource = thePreResourceAccessDetails.getResource(i); @@ -150,6 +159,9 @@ public class ConsentInterceptor { if (isRequestAuthorized(theRequestDetails)) { return; } + if (isAllowListedRequest(theRequestDetails)) { + return; + } IdentityHashMap alreadySeenResources = getAlreadySeenResourcesMap(theRequestDetails); for (int i = 0; i < thePreResourceShowDetails.size(); i++) { @@ -198,6 +210,9 @@ public class ConsentInterceptor { if (isRequestAuthorized(theRequestDetails)) { return; } + if (isAllowListedRequest(theRequestDetails)) { + return; + } IdentityHashMap alreadySeenResources = getAlreadySeenResourcesMap(theRequestDetails); @@ -330,4 +345,16 @@ public class ConsentInterceptor { } return new ForbiddenOperationException("Rejected by consent service", operationOutcome); } + + private boolean isAllowListedRequest(RequestDetails theRequestDetails) { + return isMetadataPath(theRequestDetails) || isMetaOperation(theRequestDetails); + } + + private boolean isMetaOperation(RequestDetails theRequestDetails) { + return META_OPERATION_NAME.equals(theRequestDetails.getOperation()); + } + + private boolean isMetadataPath(RequestDetails theRequestDetails) { + return URL_TOKEN_METADATA.equals(theRequestDetails.getRequestPath()); + } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java index 077eddc20ec..6c880e0f2df 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.rest.server.interceptor; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.api.BundleInclusionRule; +import ca.uhn.fhir.rest.annotation.Operation; +import ca.uhn.fhir.rest.annotation.OperationParam; import ca.uhn.fhir.rest.annotation.RequiredParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.Constants; @@ -11,11 +13,13 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.rest.server.interceptor.auth.SearchNarrowingInterceptorTest; import ca.uhn.fhir.rest.server.interceptor.consent.ConsentInterceptor; import ca.uhn.fhir.rest.server.interceptor.consent.ConsentOperationStatusEnum; import ca.uhn.fhir.rest.server.interceptor.consent.ConsentOutcome; import ca.uhn.fhir.rest.server.interceptor.consent.IConsentService; import ca.uhn.fhir.rest.server.provider.HashMapResourceProvider; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.test.utilities.JettyUtil; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; @@ -27,9 +31,11 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.hl7.fhir.instance.model.api.IBaseParameters; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.OperationOutcome; +import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.AfterAll; @@ -70,6 +76,7 @@ public class ConsentInterceptorTest { private static Server ourServer; private static DummyPatientResourceProvider ourPatientProvider; private static IGenericClient ourFhirClient; + private static DummySystemProvider ourSystemProvider; @Mock private IConsentService myConsentSvc; @@ -163,6 +170,28 @@ public class ConsentInterceptorTest { } + @Test + public void testMetadataCallHasChecksSkipped() throws IOException{ + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/metadata"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(200, status.getStatusLine().getStatusCode()); + String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8); + ourLog.info("Response: {}", responseContent); + } + + httpGet = new HttpGet("http://localhost:" + ourPort + "/$meta"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(200, status.getStatusLine().getStatusCode()); + String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8); + ourLog.info("Response: {}", responseContent); + } + + verify(myConsentSvc, times(0)).canSeeResource(any(), any(), any()); + verify(myConsentSvc, times(0)).willSeeResource(any(), any(), any()); + verify(myConsentSvc, times(0)).startOperation(any(), any()); + verify(myConsentSvc, times(2)).completeOperationSuccess(any(), any()); + } + @Test public void testSearch_SeeResourceAuthorizesOuterBundle() throws IOException { ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA")); @@ -457,11 +486,13 @@ public class ConsentInterceptorTest { ourServer = new Server(0); ourPatientProvider = new DummyPatientResourceProvider(ourCtx); + ourSystemProvider = new DummySystemProvider(); ServletHandler servletHandler = new ServletHandler(); ourServlet = new RestfulServer(ourCtx); ourServlet.setDefaultPrettyPrint(true); ourServlet.setResourceProviders(ourPatientProvider); + ourServlet.registerProvider(ourSystemProvider); ourServlet.setBundleInclusionRule(BundleInclusionRule.BASED_ON_RESOURCE_PRESENCE); ServletHolder servletHolder = new ServletHolder(ourServlet); servletHandler.addServletWithMapping(servletHolder, "/*"); @@ -478,4 +509,15 @@ public class ConsentInterceptorTest { ourFhirClient = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort); } + private static class DummySystemProvider{ + + @Operation(name = "$meta", idempotent = true, returnParameters = { + @OperationParam(name = "return", typeName = "Meta") + }) + public IBaseParameters meta(ServletRequestDetails theRequestDetails) { + Parameters retval = new Parameters(); + retval.addParameter("Meta", "Yes"); + return retval; + } + } }