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 46d87ba4..fdd36907 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 @@ -26,6 +26,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; @@ -62,6 +64,11 @@ private AccessGrantedAndUpdateList( this.resourceTypeExpected = resourceTypeExpected; } + @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/BearerAuthorizationInterceptor.java b/server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java index fb847aa6..4aebd203 100644 --- 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 { @@ -270,6 +272,7 @@ public boolean authorizeRequest(RequestDetails requestDetails) { return false; } AccessDecision outcome = checkAuthorization(requestDetails); + mutateRequest(requestDetails, outcome); logger.debug("Authorized request path " + requestPath); try { HttpResponse response = fhirClient.handleRequest(servletDetails); @@ -390,4 +393,17 @@ private void serveWellKnown(ServletRequestDetails request) { 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 1ebd7c23..399fffe0 100644 --- a/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java +++ b/server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java @@ -20,6 +20,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; @@ -52,6 +54,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/interfaces/AccessDecision.java b/server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java index c32f00bf..cc49026f 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 @@ -16,6 +16,7 @@ package com.google.fhir.gateway.interfaces; import java.io.IOException; +import javax.annotation.Nullable; import org.apache.http.HttpResponse; public interface AccessDecision { @@ -23,6 +24,20 @@ public interface AccessDecision { /** @return true iff access was granted. */ boolean canAccess(); + /** + * 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 84e65a4a..d0394811 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 @@ -25,6 +25,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..d1daaee6 --- /dev/null +++ b/server/src/main/java/com/google/fhir/gateway/interfaces/RequestMutation.java @@ -0,0 +1,20 @@ +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..b08ecc11 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; @@ -42,6 +43,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,6 +58,8 @@ 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 org.apache.http.HttpResponse; @@ -352,4 +356,39 @@ 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 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")); + } }