diff --git a/exec/pom.xml b/exec/pom.xml index 68516818..ca80ce3b 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.24 + 0.1.25 exec diff --git a/plugins/pom.xml b/plugins/pom.xml index a8a10d28..68819695 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.24 + 0.1.25 plugins diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/AccessGrantedAndUpdateList.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/AccessGrantedAndUpdateList.java index 3cba9a0a..84c9395e 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/AccessGrantedAndUpdateList.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/AccessGrantedAndUpdateList.java @@ -17,7 +17,6 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.parser.IParser; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import com.google.common.escape.Escaper; @@ -157,7 +156,4 @@ public static AccessGrantedAndUpdateList forBundle( return new AccessGrantedAndUpdateList( patientListId, httpFhirClient, fhirContext, existPutPatients, ResourceType.Bundle); } - - @Override - public void preProcess(ServletRequestDetails servletRequestDetails) {} } diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java index 5f373f82..f4705809 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java @@ -21,7 +21,6 @@ import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.client.api.IGenericClient; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.annotations.VisibleForTesting; import com.google.fhir.gateway.ProxyConstants; import com.google.fhir.gateway.interfaces.AccessDecision; @@ -92,12 +91,15 @@ public boolean canAccess() { } @Override - public void preProcess(ServletRequestDetails servletRequestDetails) { + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + + RequestMutation requestMutation = null; + // TODO: Disable access for a user who adds tags to organisations, locations or care teams that // they do not have access to // This does not bar access to anyone who uses their own sync tags to circumvent // the filter. The aim of this feature based on scoping was to pre-filter the data for the user - if (isSyncUrl(servletRequestDetails)) { + if (isSyncUrl(requestDetailsReader)) { // This prevents access to a user who has no location/organisation/team assigned to them if (locationIds.size() == 0 && careTeamIds.size() == 0 && organizationIds.size() == 0) { locationIds.add( @@ -105,28 +107,31 @@ public void preProcess(ServletRequestDetails servletRequestDetails) { } // Skip app-wide global resource requests - if (!shouldSkipDataFiltering(servletRequestDetails)) { - - addSyncFilters( - servletRequestDetails, getSyncTags(locationIds, careTeamIds, organizationIds)); + if (!shouldSkipDataFiltering(requestDetailsReader)) { + + List syncFilterParameterValues = + addSyncFilters( + requestDetailsReader, getSyncTags(locationIds, careTeamIds, organizationIds)); + requestMutation = + RequestMutation.builder() + .queryParams(Map.of(ProxyConstants.TAG_SEARCH_PARAM, syncFilterParameterValues)) + .build(); } } - } - @Override - public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { - return null; + return requestMutation; } /** - * Adds filters to the {@link ServletRequestDetails} for the _tag property to allow filtering by + * Adds filters to the {@link RequestDetailsReader} for the _tag property to allow filtering by * specific code-url-values that match specific locations, teams or organisations * - * @param servletRequestDetails + * @param requestDetailsReader * @param syncTags + * @return the extra query Parameter values */ - private void addSyncFilters( - ServletRequestDetails servletRequestDetails, Pair> syncTags) { + private List addSyncFilters( + RequestDetailsReader requestDetailsReader, Pair> syncTags) { List paramValues = new ArrayList<>(); Collections.addAll( paramValues, @@ -136,13 +141,12 @@ private void addSyncFilters( .split(ProxyConstants.PARAM_VALUES_SEPARATOR)); String[] prevTagFilters = - servletRequestDetails.getParameters().get(ProxyConstants.TAG_SEARCH_PARAM); + requestDetailsReader.getParameters().get(ProxyConstants.TAG_SEARCH_PARAM); if (prevTagFilters != null && prevTagFilters.length > 0) { Collections.addAll(paramValues, prevTagFilters); } - servletRequestDetails.addParameter( - ProxyConstants.TAG_SEARCH_PARAM, paramValues.toArray(new String[0])); + return paramValues; } @Override @@ -170,9 +174,8 @@ public String postProcess(RequestDetailsReader request, HttpResponse response) */ private String postProcessModeListEntries(HttpResponse response) throws IOException { - String resultContent = null; - IBaseResource responseResource = - fhirR4JsonParser.parseResource((new BasicResponseHandler().handleResponse(response))); + String resultContent = new BasicResponseHandler().handleResponse(response); + IBaseResource responseResource = fhirR4JsonParser.parseResource(resultContent); if (responseResource instanceof ListResource && ((ListResource) responseResource).hasEntry()) { @@ -252,12 +255,12 @@ private void addTags( } } - private boolean isSyncUrl(ServletRequestDetails servletRequestDetails) { - if (servletRequestDetails.getRequestType() == RequestTypeEnum.GET - && !TextUtils.isEmpty(servletRequestDetails.getResourceName())) { - String requestPath = servletRequestDetails.getRequestPath(); + private boolean isSyncUrl(RequestDetailsReader requestDetailsReader) { + if (requestDetailsReader.getRequestType() == RequestTypeEnum.GET + && !TextUtils.isEmpty(requestDetailsReader.getResourceName())) { + String requestPath = requestDetailsReader.getRequestPath(); return isResourceTypeRequest( - requestPath.replace(servletRequestDetails.getFhirServerBase(), "")); + requestPath.replace(requestDetailsReader.getFhirServerBase(), "")); } return false; @@ -305,23 +308,23 @@ protected IgnoredResourcesConfig getSkippedResourcesConfigs() { * This method checks the request to ensure the path, request type and parameters match values in * the hapi_sync_filter_ignored_queries configuration */ - private boolean shouldSkipDataFiltering(ServletRequestDetails servletRequestDetails) { + private boolean shouldSkipDataFiltering(RequestDetailsReader requestDetailsReader) { if (config == null) return false; for (IgnoredResourcesConfig entry : config.entries) { - if (!entry.getPath().equals(servletRequestDetails.getRequestPath())) { + if (!entry.getPath().equals(requestDetailsReader.getRequestPath())) { continue; } if (entry.getMethodType() != null - && !entry.getMethodType().equals(servletRequestDetails.getRequestType().name())) { + && !entry.getMethodType().equals(requestDetailsReader.getRequestType().name())) { continue; } for (Map.Entry expectedParam : entry.getQueryParams().entrySet()) { String[] actualQueryValue = - servletRequestDetails.getParameters().get(expectedParam.getKey()); + requestDetailsReader.getParameters().get(expectedParam.getKey()); if (actualQueryValue == null) { return true; @@ -357,6 +360,11 @@ protected void setSkippedResourcesConfig(IgnoredResourcesConfig config) { this.config = config; } + @VisibleForTesting + protected void setFhirR4Context(FhirContext fhirR4Context) { + this.fhirR4Context = fhirR4Context; + } + class IgnoredResourcesConfig { @Getter List entries; @Getter private String path; @@ -375,16 +383,6 @@ public String toString() { } } - @VisibleForTesting - protected void setFhirR4Context(FhirContext fhirR4Context) { - this.fhirR4Context = fhirR4Context; - } - - @VisibleForTesting - protected void setFhirR4JsonParser(IParser fhirR4JsonParser) { - this.fhirR4JsonParser = fhirR4JsonParser; - } - public static final class Constants { public static final String FHIR_GATEWAY_MODE = "fhir-gateway-mode"; public static final String LIST_ENTRIES = "list-entries"; diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java index a82141cd..435bc8da 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java @@ -339,7 +339,7 @@ public AccessChecker create( : Collections.singletonList(new CareTeam()); for (CareTeam careTeam : careTeams) { if (careTeam.getIdElement() != null) { - careTeamIds.add(getResourceId(careTeam.getIdElement())); + careTeamIds.add(careTeam.getIdElement().getIdPart()); } } } else if (syncStrategy.contains(ORGANIZATION)) { @@ -350,7 +350,7 @@ public AccessChecker create( : Collections.singletonList(new Organization()); for (Organization organization : organizations) { if (organization.getIdElement() != null) { - organizationIds.add(getResourceId(organization.getIdElement())); + organizationIds.add(organization.getIdElement().getIdPart()); } } } else if (syncStrategy.contains(LOCATION)) { @@ -361,7 +361,7 @@ public AccessChecker create( : Collections.singletonList(new Location()); for (Location location : locations) { if (location.getIdElement() != null) { - locationIds.add(getResourceId(location.getIdElement())); + locationIds.add(location.getIdElement().getIdPart()); } } } @@ -375,13 +375,5 @@ public AccessChecker create( organizationIds, syncStrategy); } - - public String getResourceId(IdType idType) { - if (idType.isIdPartValidLong() && idType.getIdPartAsLong() != null) { - return idType.getIdPartAsLong().toString(); - } else { - return idType.getIdPart(); - } - } } } diff --git a/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java b/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java index 1ef5da66..074abad2 100755 --- a/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java +++ b/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java @@ -22,15 +22,16 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.gclient.ITransaction; import ca.uhn.fhir.rest.gclient.ITransactionTyped; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.collect.Maps; import com.google.common.io.Resources; -import com.google.fhir.gateway.HttpFhirClient; import com.google.fhir.gateway.ProxyConstants; import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import java.io.IOException; import java.net.URL; import java.nio.charset.StandardCharsets; @@ -46,7 +47,6 @@ import org.junit.runner.RunWith; import org.mockito.Answers; import org.mockito.ArgumentCaptor; -import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -61,15 +61,13 @@ public class OpenSRPSyncAccessDecisionTest { private OpenSRPSyncAccessDecision testInstance; - @Mock private HttpFhirClient httpFhirClientMock; - @Test public void preprocessShouldAddAllFiltersWhenIdsForLocationsOrganisationsAndCareTeamsAreProvided() throws IOException { testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Patient"); @@ -78,7 +76,8 @@ public void preprocessShouldAddAllFiltersWhenIdsForLocationsOrganisationsAndCare requestDetails.setRequestPath("Patient"); // Call the method under testing - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); List allIds = new ArrayList<>(); allIds.addAll(locationIds); @@ -89,7 +88,9 @@ public void preprocessShouldAddAllFiltersWhenIdsForLocationsOrganisationsAndCare Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(locationId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.LOCATION_TAG_URL + "|" + locationId)); } @@ -97,7 +98,9 @@ public void preprocessShouldAddAllFiltersWhenIdsForLocationsOrganisationsAndCare Assert.assertFalse(requestDetails.getCompleteUrl().contains(careTeamId)); Assert.assertFalse(requestDetails.getRequestPath().contains(careTeamId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.CARE_TEAM_TAG_URL + "|" + careTeamId)); } @@ -105,7 +108,9 @@ public void preprocessShouldAddAllFiltersWhenIdsForLocationsOrganisationsAndCare Assert.assertFalse(requestDetails.getCompleteUrl().contains(organisationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(organisationId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.ORGANISATION_TAG_URL + "|" + organisationId)); } } @@ -117,7 +122,7 @@ public void preProcessShouldAddLocationIdFiltersWhenUserIsAssignedToLocationsOnl locationIds.add("locationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Patient"); @@ -125,17 +130,20 @@ public void preProcessShouldAddLocationIdFiltersWhenUserIsAssignedToLocationsOnl requestDetails.setCompleteUrl("https://smartregister.org/fhir/Patient"); requestDetails.setRequestPath("Patient"); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); for (String locationId : locationIds) { Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(locationId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.LOCATION_TAG_URL + "|" + locationId)); } - for (String param : requestDetails.getParameters().get("_tag")) { + for (String param : mutatedRequest.getQueryParams().get("_tag")) { Assert.assertFalse(param.contains(ProxyConstants.CARE_TEAM_TAG_URL)); Assert.assertFalse(param.contains(ProxyConstants.ORGANISATION_TAG_URL)); } @@ -148,7 +156,7 @@ public void preProcessShouldAddCareTeamIdFiltersWhenUserIsAssignedToCareTeamsOnl careTeamIds.add("careteamid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Patient"); @@ -156,17 +164,20 @@ public void preProcessShouldAddCareTeamIdFiltersWhenUserIsAssignedToCareTeamsOnl requestDetails.setCompleteUrl("https://smartregister.org/fhir/Patient"); requestDetails.setRequestPath("Patient"); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); for (String locationId : careTeamIds) { Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(locationId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.CARE_TEAM_TAG_URL + "|" + locationId)); } - for (String param : requestDetails.getParameters().get("_tag")) { + for (String param : mutatedRequest.getQueryParams().get("_tag")) { Assert.assertFalse(param.contains(ProxyConstants.LOCATION_TAG_URL)); Assert.assertFalse(param.contains(ProxyConstants.ORGANISATION_TAG_URL)); } @@ -179,7 +190,7 @@ public void preProcessShouldAddOrganisationIdFiltersWhenUserIsAssignedToOrganisa organisationIds.add("organizationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Patient"); @@ -187,17 +198,20 @@ public void preProcessShouldAddOrganisationIdFiltersWhenUserIsAssignedToOrganisa requestDetails.setCompleteUrl("https://smartregister.org/fhir/Patient"); requestDetails.setRequestPath("Patient"); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); for (String locationId : careTeamIds) { Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(locationId)); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.ORGANISATION_TAG_URL + "|" + locationId)); } - for (String param : requestDetails.getParameters().get("_tag")) { + for (String param : mutatedRequest.getQueryParams().get("_tag")) { Assert.assertFalse(param.contains(ProxyConstants.LOCATION_TAG_URL)); Assert.assertFalse(param.contains(ProxyConstants.CARE_TEAM_TAG_URL)); } @@ -209,7 +223,7 @@ public void preProcessShouldAddFiltersWhenResourceNotInSyncFilterIgnoredResource organisationIds.add("organizationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Patient"); @@ -217,14 +231,17 @@ public void preProcessShouldAddFiltersWhenResourceNotInSyncFilterIgnoredResource requestDetails.setCompleteUrl("https://smartregister.org/fhir/Patient"); requestDetails.setRequestPath("Patient"); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); for (String locationId : organisationIds) { Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); Assert.assertFalse(requestDetails.getRequestPath().contains(locationId)); - Assert.assertEquals(1, requestDetails.getParameters().size()); + Assert.assertEquals(1, mutatedRequest.getQueryParams().size()); Assert.assertTrue( - Arrays.asList(requestDetails.getParameters().get("_tag")) + mutatedRequest + .getQueryParams() + .get("_tag") .contains(ProxyConstants.ORGANISATION_TAG_URL + "|" + locationId)); } } @@ -235,7 +252,7 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso organisationIds.add("organizationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("Questionnaire"); @@ -243,7 +260,8 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso requestDetails.setCompleteUrl("https://smartregister.org/fhir/Questionnaire"); requestDetails.setRequestPath("Questionnaire"); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); for (String locationId : organisationIds) { Assert.assertFalse(requestDetails.getCompleteUrl().contains(locationId)); @@ -259,7 +277,7 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso organisationIds.add("organizationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("StructureMap"); @@ -277,7 +295,8 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso params.put("_id", new String[] {StringUtils.join(queryStringParamValues, ",")}); requestDetails.setParameters(params); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); Assert.assertNull(requestDetails.getParameters().get(ProxyConstants.TAG_SEARCH_PARAM)); } @@ -289,7 +308,7 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso organisationIds.add("organizationid2"); testInstance = createOpenSRPSyncAccessDecisionTestInstance(); - ServletRequestDetails requestDetails = new ServletRequestDetails(); + RequestDetails requestDetails = new ServletRequestDetails(); requestDetails.setRequestType(RequestTypeEnum.GET); requestDetails.setRestOperationType(RestOperationTypeEnum.SEARCH_TYPE); requestDetails.setResourceName("StructureMap"); @@ -307,15 +326,16 @@ public void preProcessShouldSkipAddingFiltersWhenResourceInSyncFilterIgnoredReso params.put("_id", new String[] {StringUtils.join(queryStringParamValues, ",")}); requestDetails.setParameters(params); - testInstance.preProcess(requestDetails); + RequestMutation mutatedRequest = + testInstance.getRequestMutation(new TestRequestDetailsToReader(requestDetails)); - String[] searchParamArrays = - requestDetails.getParameters().get(ProxyConstants.TAG_SEARCH_PARAM); + List searchParamArrays = + mutatedRequest.getQueryParams().get(ProxyConstants.TAG_SEARCH_PARAM); Assert.assertNotNull(searchParamArrays); - for (int i = 0; i < searchParamArrays.length; i++) { + for (int i = 0; i < mutatedRequest.getQueryParams().size(); i++) { Assert.assertTrue( organisationIds.contains( - searchParamArrays[i].replace(ProxyConstants.ORGANISATION_TAG_URL + "|", ""))); + searchParamArrays.get(i).replace(ProxyConstants.ORGANISATION_TAG_URL + "|", ""))); } } diff --git a/plugins/src/test/java/com/google/fhir/gateway/plugin/TestRequestDetailsToReader.java b/plugins/src/test/java/com/google/fhir/gateway/plugin/TestRequestDetailsToReader.java new file mode 100644 index 00000000..9f7e8b25 --- /dev/null +++ b/plugins/src/test/java/com/google/fhir/gateway/plugin/TestRequestDetailsToReader.java @@ -0,0 +1,106 @@ +/* + * Copyright 2021-2023 Google LLC + * + * 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. + */ +package com.google.fhir.gateway.plugin; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.api.RequestTypeEnum; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import java.nio.charset.Charset; +import java.util.List; +import java.util.Map; +import org.hl7.fhir.instance.model.api.IIdType; + +// Note instances of this class are expected to be one per thread and this class is not thread-safe +// the same way the underlying `requestDetails` is not. +public class TestRequestDetailsToReader implements RequestDetailsReader { + private final RequestDetails requestDetails; + + TestRequestDetailsToReader(RequestDetails requestDetails) { + this.requestDetails = requestDetails; + } + + public String getRequestId() { + return requestDetails.getRequestId(); + } + + public Charset getCharset() { + return requestDetails.getCharset(); + } + + public String getCompleteUrl() { + return requestDetails.getCompleteUrl(); + } + + public FhirContext getFhirContext() { + // TODO: There might be a race condition in the underlying `getFhirContext`; check if this is + // true. Note the `myServer` object is shared between threads. + return requestDetails.getFhirContext(); + } + + public String getFhirServerBase() { + return requestDetails.getFhirServerBase(); + } + + public String getHeader(String name) { + return requestDetails.getHeader(name); + } + + public List getHeaders(String name) { + return requestDetails.getHeaders(name); + } + + public IIdType getId() { + return requestDetails.getId(); + } + + public String getOperation() { + return requestDetails.getOperation(); + } + + public Map getParameters() { + return requestDetails.getParameters(); + } + + public String getRequestPath() { + return requestDetails.getRequestPath(); + } + + public RequestTypeEnum getRequestType() { + return requestDetails.getRequestType(); + } + + public String getResourceName() { + return requestDetails.getResourceName(); + } + + public RestOperationTypeEnum getRestOperationType() { + return requestDetails.getRestOperationType(); + } + + public String getSecondaryOperation() { + return requestDetails.getSecondaryOperation(); + } + + public boolean isRespondGzip() { + return requestDetails.isRespondGzip(); + } + + public byte[] loadRequestContents() { + return requestDetails.loadRequestContents(); + } +} diff --git a/pom.xml b/pom.xml index f2798cea..1637affd 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.24 + 0.1.25 pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml index 7c4cb378..b7ca1c80 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.24 + 0.1.25 server diff --git a/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java b/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java index bb5f869e..7cf0be64 100755 --- a/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java +++ b/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java @@ -278,7 +278,6 @@ public boolean authorizeRequest(RequestDetails requestDetails) { } AccessDecision outcome = checkAuthorization(requestDetails); mutateRequest(requestDetails, outcome); - outcome.preProcess(servletDetails); logger.debug("Authorized request path " + requestPath); try { HttpResponse response = fhirClient.handleRequest(servletDetails); diff --git a/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java b/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java index 1cefc1de..0f27b89d 100644 --- a/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java +++ b/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java @@ -17,7 +17,6 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.parser.IParser; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import com.google.common.base.Preconditions; import com.google.common.io.CharStreams; import com.google.fhir.gateway.interfaces.AccessDecision; @@ -101,7 +100,4 @@ private void addCors(CapabilityStatementRestSecurityComponent security) { .setCode(RestfulSecurityService.OAUTH.toCode()); security.setDescription(SECURITY_DESCRIPTION); } - - @Override - public void preProcess(ServletRequestDetails servletRequestDetails) {} } diff --git a/server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java b/server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java index 708a99ee..6559e427 100644 --- a/server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java +++ b/server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java @@ -15,7 +15,6 @@ */ package com.google.fhir.gateway.interfaces; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import java.io.IOException; import javax.annotation.Nullable; import org.apache.http.HttpResponse; @@ -27,8 +26,6 @@ public interface AccessDecision { */ boolean canAccess(); - void preProcess(ServletRequestDetails servletRequestDetails); - /** * Allows the incoming request mutation based on the access decision. * diff --git a/server/src/main/java/com/google/fhir/gateway/interfaces/NoOpAccessDecision.java b/server/src/main/java/com/google/fhir/gateway/interfaces/NoOpAccessDecision.java index 90e9f42a..7921c983 100644 --- a/server/src/main/java/com/google/fhir/gateway/interfaces/NoOpAccessDecision.java +++ b/server/src/main/java/com/google/fhir/gateway/interfaces/NoOpAccessDecision.java @@ -15,7 +15,6 @@ */ package com.google.fhir.gateway.interfaces; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import org.apache.http.HttpResponse; public final class NoOpAccessDecision implements AccessDecision { @@ -41,9 +40,6 @@ public String postProcess(RequestDetailsReader requestDetailsReader, HttpRespons return null; } - @Override - public void preProcess(ServletRequestDetails servletRequestDetails) {} - public static AccessDecision accessGranted() { return new NoOpAccessDecision(true); }