From ee52d6fb31580549fea0defeb92da4150972b2a7 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 9 Jan 2019 20:20:46 -0600 Subject: [PATCH] Add SearchNarrowingInterceptor --- .../example/AuthorizationInterceptors.java | 61 +++- .../rest/gclient/ReferenceClientParam.java | 16 +- .../uhn/fhir/rest/param/BaseAndListParam.java | 11 +- .../ca/uhn/fhir/rest/param/ParameterUtil.java | 9 + .../fhir/instance/model/api/IAnyResource.java | 10 +- .../client/method/BaseQueryParameter.java | 2 +- hapi-fhir-server/pom.xml | 4 + .../auth/AuthorizationInterceptor.java | 2 + .../interceptor/auth/AuthorizedList.java | 84 +++++ .../auth/SearchNarrowingInterceptor.java | 198 ++++++++++++ .../fhir/rest/client/SearchClientTest.java | 225 +++++++------ .../auth/SearchNarrowingInterceptorTest.java | 300 ++++++++++++++++++ pom.xml | 5 +- src/changes/changes.xml | 6 + src/site/xdoc/doc_rest_server_security.xml | 40 ++- 15 files changed, 850 insertions(+), 123 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizedList.java create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptor.java create mode 100644 hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java diff --git a/examples/src/main/java/example/AuthorizationInterceptors.java b/examples/src/main/java/example/AuthorizationInterceptors.java index 3ef5822b6d1..28d508a5b38 100644 --- a/examples/src/main/java/example/AuthorizationInterceptors.java +++ b/examples/src/main/java/example/AuthorizationInterceptors.java @@ -1,15 +1,11 @@ package example; -import static org.apache.commons.lang3.StringUtils.isNotBlank; - -import java.util.List; - -import org.hl7.fhir.dstu3.model.IdType; -import org.hl7.fhir.instance.model.api.IBaseResource; - import ca.uhn.fhir.model.dstu2.resource.Patient; import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.annotation.*; +import ca.uhn.fhir.rest.annotation.ConditionalUrlParam; +import ca.uhn.fhir.rest.annotation.IdParam; +import ca.uhn.fhir.rest.annotation.ResourceParam; +import ca.uhn.fhir.rest.annotation.Update; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; @@ -17,6 +13,12 @@ import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.interceptor.auth.*; +import org.hl7.fhir.dstu3.model.IdType; +import org.hl7.fhir.instance.model.api.IBaseResource; + +import java.util.List; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; @SuppressWarnings("unused") public class AuthorizationInterceptors { @@ -158,4 +160,47 @@ public class AuthorizationInterceptors { //END SNIPPET: patchAll } + + + //START SNIPPET: narrowing + public class MyPatientSearchNarrowingInterceptor extends SearchNarrowingInterceptor { + + /** + * This method must be overridden to provide the list of compartments + * and/or resources that the current user should have access to + */ + @Override + protected AuthorizedList buildAuthorizedList(RequestDetails theRequestDetails) { + // Process authorization header - The following is a fake + // implementation. Obviously we'd want something more real + // for a production scenario. + // + // In this basic example we have two hardcoded bearer tokens, + // one which is for a user that has access to one patient, and + // another that has full access. + String authHeader = theRequestDetails.getHeader("Authorization"); + if ("Bearer dfw98h38r".equals(authHeader)) { + + // This user will have access to two compartments + return new AuthorizedList() + .addCompartment("Patient/123") + .addCompartment("Patient/456"); + + } else if ("Bearer 39ff939jgg".equals(authHeader)) { + + // This user has access to everything + return new AuthorizedList(); + + } else { + + throw new AuthenticationException("Unknown bearer token"); + + } + + } + + } + //END SNIPPET: narrowing + + } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java index 9a8cdcda269..7b2aba70b6b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/gclient/ReferenceClientParam.java @@ -1,8 +1,10 @@ package ca.uhn.fhir.rest.gclient; import ca.uhn.fhir.context.FhirContext; +import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IIdType; +import java.util.Arrays; import java.util.Collection; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -97,7 +99,19 @@ public class ReferenceClientParam extends BaseClientParam implements IParam { public ICriterion hasAnyOfIds(Collection theIds) { return new StringCriterion<>(getParamName(), theIds); } - + + /** + * Match the referenced resource if the resource has ANY of the given IDs + * (this is an OR search, not an AND search), (this can be the logical ID or + * the absolute URL of the resource). Note that to specify an AND search, + * simply add a subsequent {@link IQuery#where(ICriterion) where} criteria + * with the same parameter. + */ + public ICriterion hasAnyOfIds(String... theIds) { + Validate.notNull(theIds, "theIds must not be null"); + return hasAnyOfIds(Arrays.asList(theIds)); + } + private static class ReferenceChainCriterion implements ICriterion, ICriterionInternal { private final String myResourceTypeQualifier; diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseAndListParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseAndListParam.java index b6533e892dc..fcb50e4c461 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseAndListParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseAndListParam.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.param; * 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. @@ -62,5 +62,10 @@ public abstract class BaseAndListParam> implement return myValues.toString(); } - + /** + * Returns the number of AND parameters + */ + public int size() { + return myValues.size(); + } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java index 7c74ece0228..c33bc9ed6b2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java @@ -20,8 +20,10 @@ import org.hl7.fhir.instance.model.api.IPrimitiveType; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; /* * #%L @@ -208,6 +210,13 @@ public class ParameterUtil { || IPrimitiveType.class.isAssignableFrom(theClass); } + public static String escapeAndJoinOrList(Collection theValues) { + return theValues + .stream() + .map(ParameterUtil::escape) + .collect(Collectors.joining(",")); + } + public static int nonEscapedIndexOf(String theString, char theCharacter) { for (int i = 0; i < theString.length(); i++) { if (theString.charAt(i) == theCharacter) { diff --git a/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IAnyResource.java b/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IAnyResource.java index 7dd655693b4..13ce67e35f5 100644 --- a/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IAnyResource.java +++ b/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IAnyResource.java @@ -29,14 +29,14 @@ public interface IAnyResource extends IBaseResource { * Search parameter constant for _language */ @SearchParamDefinition(name="_language", path="", description="The language of the resource", type="string" ) - public static final String SP_RES_LANGUAGE = "_language"; + String SP_RES_LANGUAGE = "_language"; /** * Search parameter constant for _id */ @SearchParamDefinition(name="_id", path="", description="The ID of the resource", type="token" ) - public static final String SP_RES_ID = "_id"; + String SP_RES_ID = "_id"; /** * Fluent Client search parameter constant for _id @@ -46,7 +46,7 @@ public interface IAnyResource extends IBaseResource { * Path: Resource._id
*

*/ - public static final TokenClientParam RES_ID = new TokenClientParam(IAnyResource.SP_RES_ID); + TokenClientParam RES_ID = new TokenClientParam(IAnyResource.SP_RES_ID); String getId(); @@ -55,11 +55,11 @@ public interface IAnyResource extends IBaseResource { IPrimitiveType getLanguageElement(); - public Object getUserData(String name); + Object getUserData(String name); @Override IAnyResource setId(String theId); - public void setUserData(String name, Object value); + void setUserData(String name, Object value); } diff --git a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/method/BaseQueryParameter.java b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/method/BaseQueryParameter.java index 2f056184b13..9e260e56755 100644 --- a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/method/BaseQueryParameter.java +++ b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/method/BaseQueryParameter.java @@ -81,7 +81,7 @@ public abstract class BaseQueryParameter implements IParameter { String paramName = isNotBlank(qualifier) ? getName() + qualifier : getName(); List paramValues = theTargetQueryArguments.get(paramName); if (paramValues == null) { - paramValues = new ArrayList(value.size()); + paramValues = new ArrayList<>(value.size()); theTargetQueryArguments.put(paramName, paramValues); } diff --git a/hapi-fhir-server/pom.xml b/hapi-fhir-server/pom.xml index 3ea4bbff59f..570c20552f2 100644 --- a/hapi-fhir-server/pom.xml +++ b/hapi-fhir-server/pom.xml @@ -59,6 +59,10 @@ test + + org.apache.commons + commons-collections4 + diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index 13679f9e4aa..4f0c9bdd7ed 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -54,6 +54,8 @@ import static org.apache.commons.lang3.StringUtils.defaultString; * Documentation on Server Security * for information on how to use this interceptor. *

+ * + * @see SearchNarrowingInterceptor */ public class AuthorizationInterceptor extends ServerOperationInterceptorAdapter implements IRuleApplier { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizedList.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizedList.java new file mode 100644 index 00000000000..f99918e3274 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizedList.java @@ -0,0 +1,84 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.rest.api.server.RequestDetails; +import org.apache.commons.lang3.Validate; + +import java.util.ArrayList; +import java.util.List; + +/** + * Return type for {@link SearchNarrowingInterceptor#buildAuthorizedList(RequestDetails)} + */ +public class AuthorizedList { + + private List myCompartments; + private List myResources; + + List getCompartments() { + return myCompartments; + } + + List getResources() { + return myResources; + } + + /** + * Adds a compartment that the user should be allowed to access + * + * @param theCompartment The compartment name, e.g. "Patient/123" (in this example the user would be allowed to access Patient/123 as well as Observations where Observation.subject="Patient/123"m, etc. + * @return Returns this for easy method chaining + */ + public AuthorizedList addCompartment(String theCompartment) { + Validate.notNull(theCompartment, "theCompartment must not be null"); + if (myCompartments == null) { + myCompartments = new ArrayList<>(); + } + myCompartments.add(theCompartment); + + return this; + } + + /** + * Adds a compartment that the user should be allowed to access + * + * @param theCompartments The compartment names, e.g. "Patient/123" (in this example the user would be allowed to access Patient/123 as well as Observations where Observation.subject="Patient/123"m, etc. + * @return Returns this for easy method chaining + */ + public AuthorizedList addCompartments(String... theCompartments) { + Validate.notNull(theCompartments, "theCompartments must not be null"); + for (String next : theCompartments) { + addCompartment(next); + } + return this; + } + + /** + * Adds a resource that the user should be allowed to access + * + * @param theResource The resource name, e.g. "Patient/123" (in this example the user would be allowed to access Patient/123 but not Observations where Observation.subject="Patient/123"m, etc. + * @return Returns this for easy method chaining + */ + public AuthorizedList addResource(String theResource) { + Validate.notNull(theResource, "theResource must not be null"); + if (myResources == null) { + myResources = new ArrayList<>(); + } + myResources.add(theResource); + + return this; + } + + /** + * Adds a resource that the user should be allowed to access + * + * @param theResources The resource names, e.g. "Patient/123" (in this example the user would be allowed to access Patient/123 but not Observations where Observation.subject="Patient/123"m, etc. + * @return Returns this for easy method chaining + */ + public AuthorizedList addResources(String... theResources) { + Validate.notNull(theResources, "theResources must not be null"); + for (String next : theResources) { + addResource(next); + } + return this; + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptor.java new file mode 100644 index 00000000000..92bbb719633 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptor.java @@ -0,0 +1,198 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.rest.api.QualifiedParamList; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.param.ParameterUtil; +import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; +import ca.uhn.fhir.rest.server.interceptor.InterceptorAdapter; +import org.apache.commons.collections4.ListUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.*; + +/** + * This interceptor can be used to automatically narrow the scope of searches in order to + * automatically restrict the searches to specific compartments. + *

+ * For example, this interceptor + * could be used to restrict a user to only viewing data belonging to Patient/123 (i.e. data + * in the Patient/123 compartment). In this case, a user performing a search + * for
+ * http://baseurl/Observation?category=laboratory
+ * would receive results as though they had requested
+ * http://baseurl/Observation?subject=Patient/123&category=laboratory + *

+ *

+ * Note that this interceptor should be used in combination with {@link AuthorizationInterceptor} + * if you are restricting results because of a security restriction. This interceptor is not + * intended to be a failsafe way of preventing users from seeing the wrong data (that is the + * purpose of AuthorizationInterceptor). This interceptor is simply intended as a convenience to + * help users simplify their queries while not receiving security errors for to trying to access + * data they do not have access to see. + *

+ * + * @see AuthorizationInterceptor + */ +public abstract class SearchNarrowingInterceptor extends InterceptorAdapter { + + /** + * Subclasses should override this method to supply the set of compartments that + * the user making the request should actually have access to. + *

+ * Typically this is done by examining theRequestDetails to find + * out who the current user is and then building a list of Strings. + *

+ * + * @param theRequestDetails The individual request currently being applied + */ + protected AuthorizedList buildAuthorizedList(@SuppressWarnings("unused") RequestDetails theRequestDetails) { + return new AuthorizedList(); + } + + + @Override + public boolean incomingRequestPostProcessed(RequestDetails theRequestDetails, HttpServletRequest theRequest, HttpServletResponse theResponse) throws AuthenticationException { + + // We don't support this operation type yet + Validate.isTrue(theRequestDetails.getRestOperationType() != RestOperationTypeEnum.SEARCH_SYSTEM); + + if (theRequestDetails.getRestOperationType() != RestOperationTypeEnum.SEARCH_TYPE) { + return true; + } + + FhirContext ctx = theRequestDetails.getServer().getFhirContext(); + RuntimeResourceDefinition resDef = ctx.getResourceDefinition(theRequestDetails.getResourceName()); + HashMap> parameterToOrValues = new HashMap<>(); + AuthorizedList authorizedList = buildAuthorizedList(theRequestDetails); + + /* + * Create a map of search parameter values that need to be added to the + * given request + */ + Collection compartments = authorizedList.getCompartments(); + if (compartments != null) { + processResourcesOrCompartments(theRequestDetails, resDef, parameterToOrValues, compartments, true); + } + Collection resources = authorizedList.getResources(); + if (resources != null) { + processResourcesOrCompartments(theRequestDetails, resDef, parameterToOrValues, resources, false); + } + + /* + * Add any param values to the actual request + */ + if (parameterToOrValues.size() > 0) { + Map newParameters = new HashMap<>(theRequestDetails.getParameters()); + for (Map.Entry> nextEntry : parameterToOrValues.entrySet()) { + String nextParamName = nextEntry.getKey(); + List nextAllowedValues = nextEntry.getValue(); + + if (!newParameters.containsKey(nextParamName)) { + + /* + * If we don't already have a parameter of the given type, add one + */ + String nextValuesJoined = ParameterUtil.escapeAndJoinOrList(nextAllowedValues); + String[] paramValues = {nextValuesJoined}; + newParameters.put(nextParamName, paramValues); + + } else { + + /* + * If the client explicitly requested the given parameter already, we'll + * just update the request to have the intersection of the values that the client + * requested, and the values that the user is allowed to see + */ + String[] existingValues = newParameters.get(nextParamName); + boolean restrictedExistingList = false; + for (int i = 0; i < existingValues.length; i++) { + + String nextExistingValue = existingValues[i]; + List nextRequestedValues = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(null, nextExistingValue); + List nextPermittedValues = ListUtils.intersection(nextRequestedValues, nextAllowedValues); + if (nextPermittedValues.size() > 0) { + restrictedExistingList = true; + existingValues[i] = ParameterUtil.escapeAndJoinOrList(nextPermittedValues); + } + + } + + /* + * If none of the values that were requested by the client overlap at all + * with the values that the user is allowed to see, we'll just add the permitted + * list as a new list. Ultimately this scenario actually means that the client + * shouldn't get *any* results back, and adding a new AND parameter (that doesn't + * overlap at all with the others) is one way of ensuring that. + */ + if (!restrictedExistingList) { + String[] newValues = Arrays.copyOf(existingValues, existingValues.length + 1); + newValues[existingValues.length] = ParameterUtil.escapeAndJoinOrList(nextAllowedValues); + newParameters.put(nextParamName, newValues); + } + } + + } + theRequestDetails.setParameters(newParameters); + } + + return true; + } + + private void processResourcesOrCompartments(RequestDetails theRequestDetails, RuntimeResourceDefinition theResDef, HashMap> theParameterToOrValues, Collection theResourcesOrCompartments, boolean theAreCompartments) { + String lastCompartmentName = null; + String lastSearchParamName=null; + for (String nextCompartment : theResourcesOrCompartments) { + Validate.isTrue(StringUtils.countMatches(nextCompartment, '/') == 1, "Invalid compartment name (must be in form \"ResourceType/xxx\": %s", nextCompartment); + String compartmentName = nextCompartment.substring(0, nextCompartment.indexOf('/')); + + String searchParamName = null; + if (compartmentName.equalsIgnoreCase(lastCompartmentName)) { + + // Avoid doing a lookup for the same thing repeatedly + searchParamName = lastSearchParamName; + + } else { + + if (compartmentName.equalsIgnoreCase(theRequestDetails.getResourceName())) { + + searchParamName = "_id"; + + } else if (theAreCompartments) { + + List searchParams = theResDef.getSearchParamsForCompartmentName(compartmentName); + if (searchParams.size() > 0) { + + // Resources like Observation have several fields that add the resource to + // the compartment. In the case of Observation, it's subject, patient and performer. + // For this kind of thing, we'll prefer the one called "patient". + RuntimeSearchParam searchParam = + searchParams + .stream() + .filter(t -> t.getName().equalsIgnoreCase(compartmentName)) + .findFirst() + .orElse(searchParams.get(0)); + searchParamName = searchParam.getName(); + + } + } + + lastCompartmentName = compartmentName; + lastSearchParamName = searchParamName; + + } + + if (searchParamName != null) { + List orValues = theParameterToOrValues.computeIfAbsent(searchParamName, t -> new ArrayList<>()); + orValues.add(nextCompartment); + } + } + } + +} diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/client/SearchClientTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/client/SearchClientTest.java index 37375c04f9c..4c2428d7530 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/client/SearchClientTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/client/SearchClientTest.java @@ -1,16 +1,19 @@ package ca.uhn.fhir.rest.client; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.io.StringReader; -import java.nio.charset.Charset; -import java.util.*; - +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.rest.annotation.IncludeParam; +import ca.uhn.fhir.rest.annotation.RequiredParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.client.api.IBasicClient; +import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; +import ca.uhn.fhir.rest.param.TokenAndListParam; +import ca.uhn.fhir.rest.param.TokenOrListParam; +import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; +import ca.uhn.fhir.util.TestUtil; +import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; import org.apache.commons.io.input.ReaderInputStream; import org.apache.http.HttpResponse; @@ -23,118 +26,144 @@ import org.apache.http.message.BasicStatusLine; import org.hamcrest.Matchers; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Encounter; -import org.junit.*; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.internal.stubbing.defaultanswers.ReturnsDeepStubs; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.rest.annotation.*; -import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.client.api.IBasicClient; -import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; -import ca.uhn.fhir.rest.param.TokenOrListParam; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; -import ca.uhn.fhir.util.TestUtil; +import java.io.StringReader; +import java.nio.charset.Charset; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class SearchClientTest { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchClientTest.class); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchClientTest.class); - private FhirContext ourCtx; - private HttpClient ourHttpClient; - private HttpResponse ourHttpResponse; + private FhirContext ourCtx; + private HttpClient ourHttpClient; + private HttpResponse ourHttpResponse; - @Before - public void before() { - ourCtx = FhirContext.forR4(); + @Before + public void before() { + ourCtx = FhirContext.forR4(); - ourHttpClient = mock(HttpClient.class, new ReturnsDeepStubs()); - ourCtx.getRestfulClientFactory().setHttpClient(ourHttpClient); - ourCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); + ourHttpClient = mock(HttpClient.class, new ReturnsDeepStubs()); + ourCtx.getRestfulClientFactory().setHttpClient(ourHttpClient); + ourCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); - ourHttpResponse = mock(HttpResponse.class, new ReturnsDeepStubs()); - } + ourHttpResponse = mock(HttpResponse.class, new ReturnsDeepStubs()); + } - @Test - public void testPostOnLongParamsList() throws Exception { - String resp = createBundle(); + @Test + public void testPostOnLongParamsList() throws Exception { + String resp = createBundle(); - ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); - when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); - when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); - when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML_NEW + "; charset=UTF-8")); - when(ourHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(resp), Charset.forName("UTF-8"))); + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); + when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML_NEW + "; charset=UTF-8")); + when(ourHttpResponse.getEntity().getContent()).thenAnswer(t->new ReaderInputStream(new StringReader(resp), Charset.forName("UTF-8"))); - ITestClient client = ourCtx.newRestfulClient(ITestClient.class, "http://foo"); - Set includes = new HashSet(); - includes.add(new Include("one")); - includes.add(new Include("two")); - TokenOrListParam params = new TokenOrListParam(); - for (int i = 0; i < 1000; i++) { - params.add(new TokenParam("system", "value")); - } - List found = client.searchByList(params, includes); + ITestClient client = ourCtx.newRestfulClient(ITestClient.class, "http://foo"); + Set includes = new HashSet(); + includes.add(new Include("one")); + includes.add(new Include("two")); + TokenOrListParam params = new TokenOrListParam(); + for (int i = 0; i < 1000; i++) { + params.add(new TokenParam("system", "value")); + } - assertEquals(1, found.size()); + // With OR list - Encounter encounter = found.get(0); - assertNotNull(encounter.getSubject().getReference()); - HttpUriRequest value = capt.getValue(); + List found = client.searchByList(params, includes); - assertTrue("Expected request of type POST on long params list", value instanceof HttpPost); - HttpPost post = (HttpPost) value; - String body = IOUtils.toString(post.getEntity().getContent()); - ourLog.info(body); - assertThat(body, Matchers.containsString("_include=one")); - assertThat(body, Matchers.containsString("_include=two")); - } + assertEquals(1, found.size()); - @Test - public void testReturnTypedList() throws Exception { - - String resp = createBundle(); + Encounter encounter = found.get(0); + assertNotNull(encounter.getSubject().getReference()); + HttpUriRequest value = capt.getValue(); - ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); - when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); - when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); - when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML_NEW + "; charset=UTF-8")); - when(ourHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(resp), Charset.forName("UTF-8"))); + assertTrue("Expected request of type POST on long params list", value instanceof HttpPost); + HttpPost post = (HttpPost) value; + String body = IOUtils.toString(post.getEntity().getContent(), Charsets.UTF_8); + ourLog.info(body); + assertThat(body, Matchers.containsString("_include=one")); + assertThat(body, Matchers.containsString("_include=two")); - ITestClient client = ourCtx.newRestfulClient(ITestClient.class, "http://foo"); - List found = client.search(); - assertEquals(1, found.size()); + // With AND list - Encounter encounter = found.get(0); - assertNotNull(encounter.getSubject().getReference()); - } + TokenAndListParam paramsAndList = new TokenAndListParam(); + paramsAndList.addAnd(params); + found = client.searchByList(paramsAndList, includes); - private String createBundle() { - Bundle bundle = new Bundle(); - - Encounter enc = new Encounter(); - enc.getSubject().setReference("Patient/1"); - - bundle.addEntry().setResource(enc); - - String retVal = ourCtx.newXmlParser().encodeResourceToString(bundle); - return retVal; - } + assertEquals(1, found.size()); - private interface ITestClient extends IBasicClient { + encounter = found.get(0); + assertNotNull(encounter.getSubject().getReference()); + value = capt.getAllValues().get(1); - @Search - List search(); + assertTrue("Expected request of type POST on long params list", value instanceof HttpPost); + post = (HttpPost) value; + body = IOUtils.toString(post.getEntity().getContent(), Charsets.UTF_8); + ourLog.info(body); + assertThat(body, Matchers.containsString("_include=one")); + assertThat(body, Matchers.containsString("_include=two")); + } - @Search - List searchByList(@RequiredParam(name = Encounter.SP_IDENTIFIER) TokenOrListParam tokenOrListParam, @IncludeParam Set theIncludes) throws BaseServerResponseException; + @Test + public void testReturnTypedList() throws Exception { - } + String resp = createBundle(); - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(ourHttpClient.execute(capt.capture())).thenReturn(ourHttpResponse); + when(ourHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(ourHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML_NEW + "; charset=UTF-8")); + when(ourHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(resp), Charset.forName("UTF-8"))); + + ITestClient client = ourCtx.newRestfulClient(ITestClient.class, "http://foo"); + List found = client.search(); + assertEquals(1, found.size()); + + Encounter encounter = found.get(0); + assertNotNull(encounter.getSubject().getReference()); + } + + private String createBundle() { + Bundle bundle = new Bundle(); + + Encounter enc = new Encounter(); + enc.getSubject().setReference("Patient/1"); + + bundle.addEntry().setResource(enc); + + String retVal = ourCtx.newXmlParser().encodeResourceToString(bundle); + return retVal; + } + + private interface ITestClient extends IBasicClient { + + @Search + List search(); + + @Search + List searchByList(@RequiredParam(name = Encounter.SP_IDENTIFIER) TokenOrListParam tokenOrListParam, @IncludeParam Set theIncludes) throws BaseServerResponseException; + + @Search + List searchByList(@RequiredParam(name = Encounter.SP_IDENTIFIER) TokenAndListParam tokenOrListParam, @IncludeParam Set theIncludes) throws BaseServerResponseException; + + } + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java new file mode 100644 index 00000000000..f0ce3456333 --- /dev/null +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java @@ -0,0 +1,300 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.IQueryParameterOr; +import ca.uhn.fhir.model.api.IQueryParameterType; +import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; +import ca.uhn.fhir.rest.param.BaseAndListParam; +import ca.uhn.fhir.rest.param.ReferenceAndListParam; +import ca.uhn.fhir.rest.param.StringAndListParam; +import ca.uhn.fhir.rest.param.TokenAndListParam; +import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider; +import ca.uhn.fhir.rest.server.IResourceProvider; +import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.util.PortUtil; +import ca.uhn.fhir.util.TestUtil; +import org.apache.commons.lang3.Validate; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.hamcrest.Matchers; +import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Resource; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class SearchNarrowingInterceptorTest { + + private static String ourLastHitMethod; + private static FhirContext ourCtx; + private static TokenAndListParam ourLastIdParam; + private static TokenAndListParam ourLastCodeParam; + private static ReferenceAndListParam ourLastSubjectParam; + private static ReferenceAndListParam ourLastPatientParam; + private static ReferenceAndListParam ourLastPerformerParam; + private static StringAndListParam ourLastNameParam; + private static List ourReturn; + private static Server ourServer; + private static IGenericClient ourClient; + private static AuthorizedList ourNextCompartmentList; + + @Before + public void before() { + ourLastHitMethod = null; + ourReturn = Collections.emptyList(); + ourLastIdParam = null; + ourLastNameParam = null; + ourLastSubjectParam = null; + ourLastPatientParam = null; + ourLastPerformerParam = null; + ourLastCodeParam = null; + ourNextCompartmentList = null; + } + + @Test + public void testNarrowObservationsByPatientContext_ClientRequestedNoParams() { + + ourNextCompartmentList = new AuthorizedList().addCompartments("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Observation") + .execute(); + + assertEquals("Observation.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertThat(toStrings(ourLastPatientParam), Matchers.contains("Patient/123,Patient/456")); + } + + /** + * Should not make any changes + */ + @Test + public void testNarrowObservationsByPatientResources_ClientRequestedNoParams() { + + ourNextCompartmentList = new AuthorizedList().addResources("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Observation") + .execute(); + + assertEquals("Observation.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertNull(ourLastPatientParam); + } + + @Test + public void testNarrowPatientByPatientResources_ClientRequestedNoParams() { + + ourNextCompartmentList = new AuthorizedList().addResources("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Patient") + .execute(); + + assertEquals("Patient.search", ourLastHitMethod); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertNull(ourLastPatientParam); + assertThat(toStrings(ourLastIdParam), Matchers.contains("Patient/123,Patient/456")); + } + + @Test + public void testNarrowPatientByPatientContext_ClientRequestedNoParams() { + + ourNextCompartmentList = new AuthorizedList().addCompartments("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Patient") + .execute(); + + assertEquals("Patient.search", ourLastHitMethod); + assertNull(ourLastNameParam); + assertThat(toStrings(ourLastIdParam), Matchers.contains("Patient/123,Patient/456")); + } + + @Test + public void testNarrowPatientByPatientContext_ClientRequestedSomeOverlap() { + + ourNextCompartmentList = new AuthorizedList().addCompartments("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Patient") + .where(IAnyResource.RES_ID.exactly().codes("Patient/123", "Patient/999")) + .execute(); + + assertEquals("Patient.search", ourLastHitMethod); + assertNull(ourLastNameParam); + assertThat(toStrings(ourLastIdParam), Matchers.contains("Patient/123")); + } + + @Test + public void testNarrowObservationsByPatientContext_ClientRequestedSomeOverlap() { + + ourNextCompartmentList = new AuthorizedList().addCompartments("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Observation") + .where(Observation.PATIENT.hasAnyOfIds("Patient/456", "Patient/777")) + .and(Observation.PATIENT.hasAnyOfIds("Patient/456", "Patient/888")) + .execute(); + + assertEquals("Observation.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertThat(toStrings(ourLastPatientParam), Matchers.contains("Patient/456", "Patient/456")); + } + + @Test + public void testNarrowObservationsByPatientContext_ClientRequestedNoOverlap() { + + ourNextCompartmentList = new AuthorizedList().addCompartments("Patient/123", "Patient/456"); + + ourClient + .search() + .forResource("Observation") + .where(Observation.PATIENT.hasAnyOfIds("Patient/111", "Patient/777")) + .and(Observation.PATIENT.hasAnyOfIds("Patient/111", "Patient/888")) + .execute(); + + assertEquals("Observation.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertThat(toStrings(ourLastPatientParam), Matchers.contains("Patient/111,Patient/777", "Patient/111,Patient/888", "Patient/123,Patient/456")); + } + + private List toStrings(BaseAndListParam> theParams) { + List> valuesAsQueryTokens = theParams.getValuesAsQueryTokens(); + + return valuesAsQueryTokens + .stream() + .map(IQueryParameterOr::getValuesAsQueryTokens) + .map(t -> t + .stream() + .map(j -> j.getValueAsQueryToken(ourCtx)) + .collect(Collectors.joining(","))) + .collect(Collectors.toList()); + } + + public static class DummyPatientResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + @Search() + public List search( + @OptionalParam(name = "_id") TokenAndListParam theIdParam, + @OptionalParam(name = "name") StringAndListParam theNameParam + ) { + ourLastHitMethod = "Patient.search"; + ourLastIdParam = theIdParam; + ourLastNameParam = theNameParam; + return ourReturn; + } + + } + + public static class DummyObservationResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Observation.class; + } + + + @Search() + public List search( + @OptionalParam(name = "_id") TokenAndListParam theIdParam, + @OptionalParam(name = Observation.SP_SUBJECT) ReferenceAndListParam theSubjectParam, + @OptionalParam(name = Observation.SP_PATIENT) ReferenceAndListParam thePatientParam, + @OptionalParam(name = Observation.SP_PERFORMER) ReferenceAndListParam thePerformerParam, + @OptionalParam(name = "code") TokenAndListParam theCodeParam + ) { + ourLastHitMethod = "Observation.search"; + ourLastIdParam = theIdParam; + ourLastSubjectParam = theSubjectParam; + ourLastPatientParam = thePatientParam; + ourLastPerformerParam = thePerformerParam; + ourLastCodeParam = theCodeParam; + return ourReturn; + } + + } + + private static class MySearchNarrowingInterceptor extends SearchNarrowingInterceptor { + @Override + protected AuthorizedList buildAuthorizedList(RequestDetails theRequestDetails) { + Validate.notNull(ourNextCompartmentList); + return ourNextCompartmentList; + } + } + + @AfterClass + public static void afterClassClearContext() throws Exception { + ourServer.stop(); + TestUtil.clearAllStaticFieldsForUnitTest(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + ourCtx = FhirContext.forR4(); + + int ourPort = PortUtil.findFreePort(); + ourServer = new Server(ourPort); + + DummyPatientResourceProvider patProvider = new DummyPatientResourceProvider(); + DummyObservationResourceProvider obsProv = new DummyObservationResourceProvider(); + + ServletHandler proxyHandler = new ServletHandler(); + RestfulServer ourServlet = new RestfulServer(ourCtx); + ourServlet.setFhirContext(ourCtx); + ourServlet.setResourceProviders(patProvider, obsProv); + ourServlet.setPagingProvider(new FifoMemoryPagingProvider(100)); + ourServlet.registerInterceptor(new MySearchNarrowingInterceptor()); + ServletHolder servletHolder = new ServletHolder(ourServlet); + proxyHandler.addServletWithMapping(servletHolder, "/*"); + ourServer.setHandler(proxyHandler); + ourServer.start(); + + ourCtx.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); + ourCtx.getRestfulClientFactory().setSocketTimeout(1000000); + ourClient = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort); + } + + +} diff --git a/pom.xml b/pom.xml index 91b544b88fb..3754403f1f0 100644 --- a/pom.xml +++ b/pom.xml @@ -519,6 +519,7 @@ 3.8.1 10.14.2.0 2.0.18 + 2.3.2 25.0-jre 2.8.5 2.2.11_1 @@ -639,7 +640,7 @@ com.google.errorprone error_prone_core - 2.3.2 + ${error_prone_core_version} com.google.guava @@ -2308,7 +2309,7 @@ com.google.errorprone error_prone_core - 2.3.2 + ${error_prone_core_version} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7f77730f380..d68cb4989ec 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -264,6 +264,12 @@ OperationDefinitions are now created for named queries in server module. Thanks to Stig Døssing for the pull request! + + A new server interceptor has been added called "SearchNarrowingInterceptor". + This interceptor can be used to automatically narrow the scope of searches + performed by the user to limit them to specific resources or compartments + that the user should have access to. + diff --git a/src/site/xdoc/doc_rest_server_security.xml b/src/site/xdoc/doc_rest_server_security.xml index 111ba9186ea..1264ac5c504 100644 --- a/src/site/xdoc/doc_rest_server_security.xml +++ b/src/site/xdoc/doc_rest_server_security.xml @@ -96,10 +96,10 @@

- AuthorizationInterceptor is a new feature in HAPI FHIR, and has not yet - been heavily tested. Use with caution, and do lots of testing! We welcome - feedback and suggestions on this feature. In addition, this documentation is - not yet complete. More examples and details will be added soon! Please get in + AuthorizationInterceptor has been well tested, but it is impossible to + predeict every scenario and environment in which HAPI FHIR will be used. + Use with caution, and do lots of testing! We welcome + feedback and suggestions on this feature. Please get in touch if you'd like to help test, have suggestions, etc.

@@ -253,7 +253,37 @@ - + +
+ +

+ HAPI FHIR 3.7.0 introduced a new interceptor, the + SearchNarrowingInterceptor. +

+

+ This interceptor is designed to be used in conjunction with AuthorizationInterceptor. It + uses a similar strategy where a dynamic list is built up for each request, but the + purpose of this interceptor is to modify client searches that are received (after + HAPI FHIR received the HTTP request, but before the search is actually performed) + to restrict the search to only search for specific resources or compartments that the + user has access to. +

+

+ This could be used, for example, to allow the user to perform a search for
+ http://baseurl/Observation?category=laboratory
+ and then receive results as though they had requested
+ http://baseurl/Observation?subject=Patient/123&category=laboratory. +

+

+ An example of this interceptor follows: +

+ + + + + +
+