diff --git a/Dockerfile b/Dockerfile old mode 100644 new mode 100755 diff --git a/exec/pom.xml b/exec/pom.xml old mode 100644 new mode 100755 index 5035307e..2bee4c42 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.18-beta + 0.1.19-beta exec diff --git a/exec/src/main/java/com/google/fhir/gateway/MainApp.java b/exec/src/main/java/com/google/fhir/gateway/MainApp.java index c0b3d61e..815e3976 100644 --- a/exec/src/main/java/com/google/fhir/gateway/MainApp.java +++ b/exec/src/main/java/com/google/fhir/gateway/MainApp.java @@ -19,8 +19,8 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.web.servlet.ServletComponentScan; -@SpringBootApplication -@ServletComponentScan +@SpringBootApplication(scanBasePackages = {"com.google.fhir.gateway.plugin"}) +@ServletComponentScan(basePackages = "com.google.fhir.gateway") public class MainApp { public static void main(String[] args) { diff --git a/plugins/pom.xml b/plugins/pom.xml old mode 100644 new mode 100755 index 1723bcb9..44b33dbf --- 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.18-beta + 0.1.19-beta 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 99cc9fa4..ef2a4fd3 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 @@ -27,6 +27,8 @@ import com.google.fhir.gateway.HttpFhirClient; import com.google.fhir.gateway.HttpUtil; import com.google.fhir.gateway.interfaces.AccessDecision; +import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import java.io.IOException; import java.util.Set; import org.apache.http.HttpResponse; @@ -63,6 +65,11 @@ private AccessGrantedAndUpdateList( this.resourceTypeExpected = resourceTypeExpected; } + @Override + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + return null; + } + @Override public boolean canAccess() { return true; 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 6cbab3c4..f8b84b22 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 @@ -20,6 +20,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.fhir.gateway.ProxyConstants; import com.google.fhir.gateway.interfaces.AccessDecision; +import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import com.google.gson.Gson; import java.io.FileReader; import java.io.IOException; @@ -98,6 +100,11 @@ public void preProcess(ServletRequestDetails servletRequestDetails) { } } + @Override + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + return null; + } + /** * Adds filters to the {@link ServletRequestDetails} for the _tag property to allow filtering by * specific code-url-values that match specific locations, teams or organisations diff --git a/pom.xml b/pom.xml index f38cfd01..7ffeb93b 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.18-beta + 0.1.19-beta pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml old mode 100644 new mode 100755 index 68356a40..cd00d2d9 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.18-beta + 0.1.19-beta server @@ -71,6 +71,13 @@ ${spring.version} + + org.springframework + spring-test + ${spring.version} + test + + com.google.http-client 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 51ed8c87..e07f607e 100755 --- a/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java +++ b/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java @@ -39,6 +39,7 @@ import com.google.fhir.gateway.interfaces.AccessCheckerFactory; import com.google.fhir.gateway.interfaces.AccessDecision; import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import com.google.gson.JsonObject; import com.google.gson.JsonParser; import java.io.IOException; @@ -62,6 +63,7 @@ import org.apache.http.util.EntityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.CollectionUtils; @Interceptor public class BearerAuthorizationInterceptor { @@ -72,6 +74,10 @@ public class BearerAuthorizationInterceptor { private static final String DEFAULT_CONTENT_TYPE = "application/json; charset=UTF-8"; private static final String BEARER_PREFIX = "Bearer "; + private static final String ACCEPT_ENCODING_HEADER = "Accept-Encoding"; + + private static final String GZIP_ENCODING_VALUE = "gzip"; + // See https://hl7.org/fhir/smart-app-launch/conformance.html#using-well-known @VisibleForTesting static final String WELL_KNOWN_CONF_PATH = ".well-known/smart-configuration"; @@ -271,6 +277,7 @@ public boolean authorizeRequest(RequestDetails requestDetails) { return false; } AccessDecision outcome = checkAuthorization(requestDetails); + mutateRequest(requestDetails, outcome); outcome.preProcess(servletDetails); logger.debug("Authorized request path " + requestPath); try { @@ -302,7 +309,6 @@ public boolean authorizeRequest(RequestDetails requestDetails) { proxyResponse.addHeader(header.getName(), header.getValue()); } // This should be called after adding headers. - // TODO handle non-text responses, e.g., gzip. // TODO verify DEFAULT_CONTENT_TYPE/CHARSET are compatible with `entity.getContentType()`. Writer writer = proxyResponse.getResponseWriter( @@ -310,7 +316,7 @@ public boolean authorizeRequest(RequestDetails requestDetails) { response.getStatusLine().toString(), DEFAULT_CONTENT_TYPE, Constants.CHARSET_NAME_UTF8, - false); + sendGzippedResponse(servletDetails)); Reader reader; if (content != null) { // We can read the entity body stream only once; in this case we have already done that. @@ -331,6 +337,15 @@ public boolean authorizeRequest(RequestDetails requestDetails) { return false; } + private boolean sendGzippedResponse(ServletRequestDetails requestDetails) { + // we send gzipped encoded response to client only if they requested so + String acceptEncodingValue = requestDetails.getHeader(ACCEPT_ENCODING_HEADER.toLowerCase()); + if (acceptEncodingValue == null) { + return false; + } + return GZIP_ENCODING_VALUE.equalsIgnoreCase(acceptEncodingValue); + } + /** * Reads the content from the FHIR store response `entity`, replaces any FHIR store URLs by the * corresponding proxy URLs, and write the modified response to the proxy response `writer`. @@ -367,6 +382,7 @@ private void replaceAndCopyResponse(Reader entityContentReader, Writer writer, S // Handle any remaining characters that partially matched. writer.write(fhirStoreUrl.substring(0, numMatched)); } + writer.close(); } private void serveWellKnown(ServletRequestDetails request) { @@ -386,10 +402,24 @@ private void serveWellKnown(ServletRequestDetails request) { Constants.CHARSET_NAME_UTF8, false); writer.write(configJson); + writer.close(); } catch (IOException e) { logger.error( String.format("Exception serving %s with error %s", request.getRequestPath(), e)); ExceptionUtil.throwRuntimeExceptionAndLog(logger, e.getMessage(), e); } } + + @VisibleForTesting + void mutateRequest(RequestDetails requestDetails, AccessDecision accessDecision) { + RequestMutation mutation = + accessDecision.getRequestMutation(new RequestDetailsToReader(requestDetails)); + if (mutation == null || CollectionUtils.isEmpty(mutation.getQueryParams())) { + return; + } + + mutation + .getQueryParams() + .forEach((key, value) -> requestDetails.addParameter(key, value.toArray(new String[0]))); + } } 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 f7aeafb3..718a66e9 100644 --- a/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java +++ b/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java @@ -21,6 +21,8 @@ import com.google.common.base.Preconditions; import com.google.common.io.CharStreams; import com.google.fhir.gateway.interfaces.AccessDecision; +import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import java.io.IOException; import org.apache.http.HttpResponse; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -53,6 +55,11 @@ static synchronized CapabilityPostProcessor getInstance(FhirContext fhirContext) return instance; } + @Override + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + return null; + } + @Override public boolean canAccess() { return true; diff --git a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java index 4891079f..dbf6ebc5 100644 --- a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java +++ b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java @@ -51,6 +51,7 @@ public abstract class HttpFhirClient { "date", "expires", "content-location", + "content-encoding", "etag", "location", "x-progress", @@ -67,6 +68,7 @@ public abstract class HttpFhirClient { static final Set REQUEST_HEADERS_TO_KEEP = Sets.newHashSet( "content-type", + "accept-encoding", "last-modified", "etag", "prefer", 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 abc269ee..a9c9e748 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 @@ -17,6 +17,7 @@ import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import java.io.IOException; +import javax.annotation.Nullable; import org.apache.http.HttpResponse; public interface AccessDecision { @@ -28,6 +29,20 @@ public interface AccessDecision { void preProcess(ServletRequestDetails servletRequestDetails); + /** + * Allows the incoming request mutation based on the access decision. + * + *

Response is used to mutate the incoming request before executing the FHIR operation. We + * currently only support query parameters update for GET Http method. This is expected to be + * called after checking the access using @canAccess method. Mutating the request before checking + * access can have side effect of wrong access check. + * + * @param requestDetailsReader details about the resource and operation requested + * @return mutation to be applied on the incoming request or null if no mutation required + */ + @Nullable + RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader); + /** * Depending on the outcome of the FHIR operations, this does any post-processing operations that * are related to access policies. This is expected to be called only if the actual FHIR operation 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 b04a07f1..135e1059 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 @@ -26,6 +26,11 @@ public NoOpAccessDecision(boolean accessGranted) { this.accessGranted = accessGranted; } + @Override + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + return null; + } + @Override public boolean canAccess() { return accessGranted; diff --git a/server/src/main/java/com/google/fhir/gateway/interfaces/RequestMutation.java b/server/src/main/java/com/google/fhir/gateway/interfaces/RequestMutation.java new file mode 100644 index 00000000..fdcac3dd --- /dev/null +++ b/server/src/main/java/com/google/fhir/gateway/interfaces/RequestMutation.java @@ -0,0 +1,35 @@ +/* + * 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.interfaces; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import lombok.Builder; +import lombok.Getter; + +/** Defines mutations that can be applied to the incoming request by an {@link AccessChecker}. */ +@Builder +@Getter +public class RequestMutation { + + // Additional query parameters and list of values for a parameter that should be added to the + // outgoing FHIR request. + // New values overwrites the old one if there is a conflict for a request param (i.e. a returned + // parameter in RequestMutation is already present in the original request). + // Old parameter values should be explicitly retained while mutating values for that parameter. + @Builder.Default Map> queryParams = new HashMap<>(); +} diff --git a/server/src/test/java/com/google/fhir/gateway/BearerAuthorizationInterceptorTest.java b/server/src/test/java/com/google/fhir/gateway/BearerAuthorizationInterceptorTest.java index c481fce3..8327a159 100644 --- a/server/src/test/java/com/google/fhir/gateway/BearerAuthorizationInterceptorTest.java +++ b/server/src/test/java/com/google/fhir/gateway/BearerAuthorizationInterceptorTest.java @@ -16,6 +16,7 @@ package com.google.fhir.gateway; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.ArgumentMatchers.any; @@ -31,6 +32,7 @@ import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.rest.server.servlet.ServletRestfulResponse; import com.auth0.jwt.JWT; import com.auth0.jwt.JWTCreator; import com.auth0.jwt.algorithms.Algorithm; @@ -42,6 +44,7 @@ import com.google.fhir.gateway.interfaces.AccessDecision; import com.google.fhir.gateway.interfaces.NoOpAccessDecision; import com.google.fhir.gateway.interfaces.RequestDetailsReader; +import com.google.fhir.gateway.interfaces.RequestMutation; import com.google.gson.Gson; import java.io.IOException; import java.io.StringWriter; @@ -56,8 +59,11 @@ import java.security.interfaces.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; import java.util.Base64; +import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -71,6 +77,7 @@ import org.mockito.junit.MockitoJUnitRunner; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.mock.web.MockHttpServletResponse; @RunWith(MockitoJUnitRunner.class) public class BearerAuthorizationInterceptorTest { @@ -352,4 +359,73 @@ public void deniedRequest() throws IOException { testInstance.authorizeRequest(requestMock); } + + @Test + public void mutateRequest() { + ServletRequestDetails requestDetails = new ServletRequestDetails(); + requestDetails.addParameter("param1", new String[] {"param1-value1"}); + requestDetails.addParameter("param2", new String[] {"param2-value1"}); + + HashMap> paramMutations = new HashMap<>(); + paramMutations.put("param1", List.of("param1-value2")); + paramMutations.put("param3", List.of("param3-value1", "param3-value2")); + AccessDecision mutableAccessDecision = + new AccessDecision() { + public boolean canAccess() { + return true; + } + + public void preProcess(ServletRequestDetails servletRequestDetails) {} + + public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) { + return RequestMutation.builder().queryParams(paramMutations).build(); + } + + public String postProcess(HttpResponse response) throws IOException { + return null; + } + }; + + testInstance.mutateRequest(requestDetails, mutableAccessDecision); + + assertThat( + requestDetails.getParameters().get("param1"), arrayContainingInAnyOrder("param1-value2")); + assertThat( + requestDetails.getParameters().get("param2"), arrayContainingInAnyOrder("param2-value1")); + assertThat( + requestDetails.getParameters().get("param3"), + arrayContainingInAnyOrder("param3-value2", "param3-value1")); + } + + @Test + public void shouldSendGzippedResponseWhenRequested() throws IOException { + testInstance = createTestInstance(true, null); + String responseJson = "{\"resourceType\": \"Bundle\"}"; + JWTCreator.Builder jwtBuilder = JWT.create().withIssuer(TOKEN_ISSUER); + when(requestMock.getHeader("Authorization")).thenReturn("Bearer " + signJwt(jwtBuilder)); + when(requestMock.getHeader("Accept-Encoding".toLowerCase())).thenReturn("gzip"); + + // requestMock.getResponse() {@link ServletRequestDetails#getResponse()} is an abstraction HAPI + // provides to access the response object which is of type ServletRestfulResponse {@link + // ServletRestfulResponse}. Internally HAPI uses the HttpServletResponse {@link + // HttpServletResponse} object to perform any response related operations for this wrapper class + // ServletRestfulResponse. We have to perform mocking at two levels: one with + // requestMock.getResponse() because this is how we access the wrapper response object and write + // to it. We also need to perform a deeper level mock using requestMock.getServletResponse() + // {@link ServletRequestDetails#getServletResponse()} for the internal HAPI operations to be + // performed successfully. This complication arises from us mocking the request object. Had the + // object been not mocked, and set by a server we would not have needed to do this levels of + // mocks. + when(requestMock.getServer()).thenReturn(serverMock); + ServletRestfulResponse proxyResponseMock = new ServletRestfulResponse(requestMock); + when(requestMock.getResponse()).thenReturn(proxyResponseMock); + HttpServletResponse proxyServletResponseMock = new MockHttpServletResponse(); + when(requestMock.getServletResponse()).thenReturn(proxyServletResponseMock); + TestUtil.setUpFhirResponseMock(fhirResponseMock, responseJson); + + testInstance.authorizeRequest(requestMock); + + assertThat( + proxyServletResponseMock.getHeader("Content-Encoding".toLowerCase()), equalTo("gzip")); + } }