Allow request mutation by Access checkers

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;

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])));
+  }
 }

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;

server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java

@@ -16,13 +16,28 @@
 package com.google.fhir.gateway.interfaces;
 
 import java.io.IOException;
+import javax.annotation.Nullable;
 import org.apache.http.HttpResponse;
 
 public interface AccessDecision {
 
   /** @return true iff access was granted. */
   boolean canAccess();
 
+  /**
+   * Allows the incoming request mutation based on the access decision.
+   *
+   * <p>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

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;

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<String, List<String>> queryParams = new HashMap<>();
+}

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<String, List<String>> 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"));
+  }
 }