Fixes + Enhancements

exec/pom.xml

@@ -21,7 +21,7 @@
   <parent>
     <groupId>com.google.fhir.gateway</groupId>
     <artifactId>fhir-gateway</artifactId>
-    <version>0.1.24</version>
+    <version>0.1.25</version>
   </parent>
 
   <artifactId>exec</artifactId>

plugins/pom.xml

@@ -23,7 +23,7 @@
     implementations do not have to do this; they can redeclare those deps. -->
     <groupId>com.google.fhir.gateway</groupId>
     <artifactId>fhir-gateway</artifactId>
-    <version>0.1.24</version>
+    <version>0.1.25</version>
   </parent>
 
   <artifactId>plugins</artifactId>

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) {}
 }

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,41 +91,47 @@ 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(
             "CR1bAeGgaYqIpsNkG0iidfE5WVb5BJV1yltmL4YFp3o6mxj3iJPhKh4k9ROhlyZveFC8298lYzft8SIy8yMNLl5GVWQXNRr1sSeBkP2McfFZjbMYyrxlNFOJgqvtccDKKYSwBiLHq2By5tRupHcmpIIghV7Hp39KgF4iBDNqIGMKhgOIieQwt5BRih5FgnwdHrdlK9ix");
       }
 
       // Skip app-wide global resource requests
-      if (!shouldSkipDataFiltering(servletRequestDetails)) {
-
-        addSyncFilters(
-            servletRequestDetails, getSyncTags(locationIds, careTeamIds, organizationIds));
+      if (!shouldSkipDataFiltering(requestDetailsReader)) {
+
+        List<String> 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<String, Map<String, String[]>> syncTags) {
+  private List<String> addSyncFilters(
+      RequestDetailsReader requestDetailsReader, Pair<String, Map<String, String[]>> syncTags) {
     List<String> 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<String, Object> 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<IgnoredResourcesConfig> 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";

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();
-      }
-    }
   }
 }