diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/ListAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/ListAccessChecker.java index 308b5cdd..d42f3337 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/ListAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/ListAccessChecker.java @@ -315,7 +315,8 @@ private AccessDecision processBundle(RequestDetailsReader requestDetails) throws @Nullable private BundlePatients createBundlePatients(RequestDetailsReader requestDetails) throws IOException { - BundlePatients patientsInBundleUnfiltered = patientFinder.findPatientsInBundle(requestDetails); + Bundle requestBundle = FhirUtil.parseRequestToBundle(fhirContext, requestDetails); + BundlePatients patientsInBundleUnfiltered = patientFinder.findPatientsInBundle(requestBundle); if (patientsInBundleUnfiltered == null) { return null; diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PatientAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PatientAccessChecker.java index f216569b..9db77003 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PatientAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PatientAccessChecker.java @@ -16,7 +16,6 @@ package com.google.fhir.gateway.plugin; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.RequestTypeEnum; import com.auth0.jwt.interfaces.DecodedJWT; import com.google.common.annotations.VisibleForTesting; @@ -37,12 +36,9 @@ import com.google.fhir.gateway.plugin.SmartFhirScope.Principal; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Set; import javax.inject.Named; -import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent; @@ -208,7 +204,8 @@ private AccessDecision checkPatientAccessInUpdate(RequestDetailsReader requestDe } private AccessDecision processBundle(RequestDetailsReader requestDetails) { - BundlePatients patientsInBundle = patientFinder.findPatientsInBundle(requestDetails); + Bundle requestBundle = FhirUtil.parseRequestToBundle(fhirContext, requestDetails); + BundlePatients patientsInBundle = patientFinder.findPatientsInBundle(requestBundle); if (patientsInBundle == null || patientsInBundle.areTherePatientToCreate() @@ -226,13 +223,7 @@ private AccessDecision processBundle(RequestDetailsReader requestDetails) { return NoOpAccessDecision.accessDenied(); } } - // TODO: there is duplication in processing the request Bundle here. It is already processed in - // PatientFinder - // Will follow up with a PR to solve this :https://github.com/google/fhir-gateway/issues/104 - Bundle requestBundle = createBundleFromRequest(requestDetails); - if (requestBundle == null) { - return NoOpAccessDecision.accessDenied(); - } + for (BundleEntryComponent entryComponent : requestBundle.getEntry()) { if (!doesBundleElementHavePermission(entryComponent)) { return NoOpAccessDecision.accessDenied(); @@ -241,21 +232,6 @@ private AccessDecision processBundle(RequestDetailsReader requestDetails) { return NoOpAccessDecision.accessGranted(); } - private Bundle createBundleFromRequest(RequestDetailsReader request) { - byte[] requestContentBytes = request.loadRequestContents(); - Charset charset = request.getCharset(); - if (charset == null) { - charset = StandardCharsets.UTF_8; - } - String requestContent = new String(requestContentBytes, charset); - IParser jsonParser = fhirContext.newJsonParser(); - IBaseResource resource = jsonParser.parseResource(requestContent); - if (!(resource instanceof Bundle)) { - return null; - } - return (Bundle) resource; - } - private boolean doesReferenceElementHavePermission( IIdType referenceElement, Permission permission) { if (referenceElement.getResourceType() != null && referenceElement.hasIdPart()) { diff --git a/server/src/main/java/com/google/fhir/gateway/FhirUtil.java b/server/src/main/java/com/google/fhir/gateway/FhirUtil.java index 3d4accdd..9c053d2a 100644 --- a/server/src/main/java/com/google/fhir/gateway/FhirUtil.java +++ b/server/src/main/java/com/google/fhir/gateway/FhirUtil.java @@ -21,6 +21,8 @@ import com.google.common.base.Preconditions; import com.google.fhir.gateway.interfaces.RequestDetailsReader; import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.regex.Pattern; import javax.annotation.Nullable; import org.apache.http.HttpResponse; @@ -59,6 +61,25 @@ public static Bundle parseResponseToBundle(FhirContext fhirContext, HttpResponse return (Bundle) resource; } + public static IBaseResource createResourceFromRequest( + FhirContext fhirContext, RequestDetailsReader request) { + byte[] requestContentBytes = request.loadRequestContents(); + Charset charset = request.getCharset(); + if (charset == null) { + charset = StandardCharsets.UTF_8; + } + String requestContent = new String(requestContentBytes, charset); + IParser jsonParser = fhirContext.newJsonParser(); + return jsonParser.parseResource(requestContent); + } + + public static Bundle parseRequestToBundle(FhirContext fhirContext, RequestDetailsReader request) { + IBaseResource resource = createResourceFromRequest(fhirContext, request); + Preconditions.checkArgument( + FhirUtil.isSameResourceType(resource.fhirType(), ResourceType.Bundle)); + return (Bundle) resource; + } + public static boolean isValidId(String id) { return ID_PATTERN.matcher(id).matches(); } diff --git a/server/src/main/java/com/google/fhir/gateway/PatientFinderImp.java b/server/src/main/java/com/google/fhir/gateway/PatientFinderImp.java index 1575111e..64423844 100644 --- a/server/src/main/java/com/google/fhir/gateway/PatientFinderImp.java +++ b/server/src/main/java/com/google/fhir/gateway/PatientFinderImp.java @@ -211,17 +211,6 @@ public String findPatientFromParams(RequestDetailsReader requestDetails) { return patientId; } - private IBaseResource createResourceFromRequest(RequestDetailsReader request) { - byte[] requestContentBytes = request.loadRequestContents(); - Charset charset = request.getCharset(); - if (charset == null) { - charset = StandardCharsets.UTF_8; - } - String requestContent = new String(requestContentBytes, charset); - IParser jsonParser = fhirContext.newJsonParser(); - return jsonParser.parseResource(requestContent); - } - private JsonArray createJsonArrayFromRequest(RequestDetailsReader request) { byte[] requestContentBytes = request.loadRequestContents(); Charset charset = request.getCharset(); @@ -253,14 +242,7 @@ private Set parseReferencesForPatientIds(IBaseResource resource) { } @Override - public BundlePatients findPatientsInBundle(RequestDetailsReader request) { - IBaseResource resource = createResourceFromRequest(request); - if (!(resource instanceof Bundle)) { - ExceptionUtil.throwRuntimeExceptionAndLog( - logger, "The provided resource is not a Bundle!", InvalidRequestException.class); - } - Bundle bundle = (Bundle) resource; - + public BundlePatients findPatientsInBundle(Bundle bundle) { if (bundle.getType() != BundleType.TRANSACTION) { // Currently, support only for transaction bundles; see: // https://github.com/google/fhir-access-proxy/issues/67 @@ -460,7 +442,7 @@ private void addPatientReference(Resource resource, BundlePatientsBuilder builde @Override public Set findPatientsInResource(RequestDetailsReader request) { - IBaseResource resource = createResourceFromRequest(request); + IBaseResource resource = FhirUtil.createResourceFromRequest(fhirContext, request); if (!resource.fhirType().equals(request.getResourceName())) { ExceptionUtil.throwRuntimeExceptionAndLog( logger, diff --git a/server/src/main/java/com/google/fhir/gateway/interfaces/PatientFinder.java b/server/src/main/java/com/google/fhir/gateway/interfaces/PatientFinder.java index bf0ca35e..701f483c 100644 --- a/server/src/main/java/com/google/fhir/gateway/interfaces/PatientFinder.java +++ b/server/src/main/java/com/google/fhir/gateway/interfaces/PatientFinder.java @@ -18,6 +18,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import com.google.fhir.gateway.BundlePatients; import java.util.Set; +import org.hl7.fhir.r4.model.Bundle; public interface PatientFinder { /** @@ -34,12 +35,12 @@ public interface PatientFinder { /** * Find all patients referenced or updated in a Bundle. * - * @param request that is expected to have a Bundle content. + * @param bundle bundle request to find patient references in. * @return the {@link BundlePatients} that wraps all found patients. * @throws InvalidRequestException for various reasons when unexpected content is encountered. * Callers are expected to deny access when this happens. */ - BundlePatients findPatientsInBundle(RequestDetailsReader request); + BundlePatients findPatientsInBundle(Bundle bundle); /** * Finds all patients in the content of a request. diff --git a/server/src/test/java/com/google/fhir/gateway/FhirUtilTest.java b/server/src/test/java/com/google/fhir/gateway/FhirUtilTest.java index 97e2e2a3..4cfd6636 100644 --- a/server/src/test/java/com/google/fhir/gateway/FhirUtilTest.java +++ b/server/src/test/java/com/google/fhir/gateway/FhirUtilTest.java @@ -17,7 +17,14 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import ca.uhn.fhir.context.FhirContext; +import com.google.common.io.Resources; +import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import java.io.IOException; +import java.net.URL; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; @@ -25,6 +32,8 @@ @RunWith(MockitoJUnitRunner.class) public class FhirUtilTest { + private static final FhirContext fhirContext = FhirContext.forR4(); + @Test public void isValidIdPass() { assertThat(FhirUtil.isValidId("simple-id"), equalTo(true)); @@ -72,4 +81,23 @@ public void isValidIdTooLong() { public void isValidIdNull() { assertThat(FhirUtil.isValidId(""), equalTo(false)); } + + @Test + public void canParseValidBundle() throws IOException { + URL bundleUrl = Resources.getResource("patient_id_search.json"); + byte[] bundleBytes = Resources.toByteArray(bundleUrl); + RequestDetailsReader requestMock = mock(RequestDetailsReader.class); + when(requestMock.loadRequestContents()).thenReturn(bundleBytes); + assertThat( + FhirUtil.parseRequestToBundle(fhirContext, requestMock).getEntry().size(), equalTo(10)); + } + + @Test(expected = IllegalArgumentException.class) + public void throwExceptionOnNonBundleResource() throws IOException { + URL bundleUrl = Resources.getResource("test_patient.json"); + byte[] bundleBytes = Resources.toByteArray(bundleUrl); + RequestDetailsReader requestMock = mock(RequestDetailsReader.class); + when(requestMock.loadRequestContents()).thenReturn(bundleBytes); + FhirUtil.parseRequestToBundle(fhirContext, requestMock).getEntry().size(); + } }