From 93bf2788ec42982a5f84a5aae93bd4064fbfca58 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sat, 12 Jan 2019 14:23:26 -0600 Subject: [PATCH] Add subscription narrowing interceptor and refactor RuleBuilder to be a bit cleaner --- .../uhn/fhir/rest/param/BaseAndListParam.java | 4 +- .../cache/SubscriptionCanonicalizer.java | 4 +- .../interceptor/auth/AuthorizedList.java | 44 +++-- .../auth/IAuthRuleBuilderRuleOp.java | 3 + .../server/interceptor/auth/RuleBuilder.java | 52 +++--- .../server/interceptor/auth/RuleImplOp.java | 107 ++++++++---- .../auth/SearchNarrowingInterceptor.java | 34 +++- .../interceptor/auth/RuleBuilderTest.java | 14 ++ .../AuthorizationInterceptorDstu3Test.java | 162 +++++++++++++----- .../auth/SearchNarrowingInterceptorTest.java | 22 ++- 10 files changed, 326 insertions(+), 120 deletions(-) create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java 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 fcb50e4c461..39e23a5bd68 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. diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java index d8661af2127..b7a3f09ce7c 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.module.cache; * 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. 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 index f99918e3274..441e0fc4e06 100644 --- 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 @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.server.interceptor.auth; +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2019 University Health Network + * %% + * 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. + * #L% + */ + import ca.uhn.fhir.rest.api.server.RequestDetails; import org.apache.commons.lang3.Validate; @@ -11,15 +31,15 @@ import java.util.List; */ public class AuthorizedList { - private List myCompartments; - private List myResources; + private List myAllowedCompartments; + private List myAllowedInstances; - List getCompartments() { - return myCompartments; + List getAllowedCompartments() { + return myAllowedCompartments; } - List getResources() { - return myResources; + List getAllowedInstances() { + return myAllowedInstances; } /** @@ -30,10 +50,10 @@ public class AuthorizedList { */ public AuthorizedList addCompartment(String theCompartment) { Validate.notNull(theCompartment, "theCompartment must not be null"); - if (myCompartments == null) { - myCompartments = new ArrayList<>(); + if (myAllowedCompartments == null) { + myAllowedCompartments = new ArrayList<>(); } - myCompartments.add(theCompartment); + myAllowedCompartments.add(theCompartment); return this; } @@ -60,10 +80,10 @@ public class AuthorizedList { */ public AuthorizedList addResource(String theResource) { Validate.notNull(theResource, "theResource must not be null"); - if (myResources == null) { - myResources = new ArrayList<>(); + if (myAllowedInstances == null) { + myAllowedInstances = new ArrayList<>(); } - myResources.add(theResource); + myAllowedInstances.add(theResource); return this; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java index e9521ad5860..ce1b1702fdf 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.rest.server.interceptor.auth; import org.hl7.fhir.instance.model.api.IIdType; +import java.util.Collection; + /* * #%L * HAPI FHIR - Server Framework @@ -58,4 +60,5 @@ public interface IAuthRuleBuilderRuleOp extends IAuthRuleBuilderAppliesTo theInstances); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index dc8d258392e..0557989e55f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -172,7 +172,6 @@ public class RuleBuilder implements IAuthRuleBuilder { private PolicyEnum myRuleMode; private String myRuleName; - private RuleOpEnum myRuleOp; RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) { myRuleMode = theRuleMode; @@ -186,8 +185,7 @@ public class RuleBuilder implements IAuthRuleBuilder { @Override public IAuthRuleBuilderRuleOp delete() { - myRuleOp = RuleOpEnum.DELETE; - return new RuleBuilderRuleOp(); + return new RuleBuilderRuleOp(RuleOpEnum.DELETE); } @Override @@ -211,14 +209,12 @@ public class RuleBuilder implements IAuthRuleBuilder { @Override public IAuthRuleBuilderPatch patch() { - myRuleOp = RuleOpEnum.PATCH; return new PatchBuilder(); } @Override public IAuthRuleBuilderRuleOp read() { - myRuleOp = RuleOpEnum.READ; - return new RuleBuilderRuleOp(); + return new RuleBuilderRuleOp(RuleOpEnum.READ); } @Override @@ -233,8 +229,7 @@ public class RuleBuilder implements IAuthRuleBuilder { @Override public IAuthRuleBuilderRuleOp write() { - myRuleOp = RuleOpEnum.WRITE; - return new RuleBuilderRuleOp(); + return new RuleBuilderRuleOp(RuleOpEnum.WRITE); } @Override @@ -245,7 +240,6 @@ public class RuleBuilder implements IAuthRuleBuilder { private class RuleBuilderRuleConditional implements IAuthRuleBuilderRuleConditional { private AppliesTypeEnum myAppliesTo; - private Set myAppliesToTypes; private RestOperationTypeEnum myOperationType; @@ -291,13 +285,15 @@ public class RuleBuilder implements IAuthRuleBuilder { private class RuleBuilderRuleOp implements IAuthRuleBuilderRuleOp { - private AppliesTypeEnum myAppliesTo; - private Set myAppliesToTypes; + private final RuleOpEnum myRuleOp; + + public RuleBuilderRuleOp(RuleOpEnum theRuleOp) { + myRuleOp = theRuleOp; + } @Override public IAuthRuleBuilderRuleOpClassifier allResources() { - myAppliesTo = AppliesTypeEnum.ALL_RESOURCES; - return new RuleBuilderRuleOpClassifier(); + return new RuleBuilderRuleOpClassifier(AppliesTypeEnum.ALL_RESOURCES, null); } @Override @@ -312,15 +308,22 @@ public class RuleBuilder implements IAuthRuleBuilder { Validate.notBlank(theId.getValue(), "theId.getValue() must not be null or empty"); Validate.notBlank(theId.getIdPart(), "theId must contain an ID part"); - return new RuleBuilderRuleOpClassifier(Collections.singletonList(theId)).finished(); + List instances = Collections.singletonList(theId); + return instances(instances); + } + + @Override + public IAuthRuleFinished instances(Collection theInstances) { + Validate.notNull(theInstances, "theInstances must not be null"); + Validate.notEmpty(theInstances, "theInstances must not be empty"); + + return new RuleBuilderRuleOpClassifier(theInstances).finished(); } @Override public IAuthRuleBuilderRuleOpClassifier resourcesOfType(Class theType) { Validate.notNull(theType, "theType must not be null"); - myAppliesTo = AppliesTypeEnum.TYPES; - myAppliesToTypes = Collections.singleton(theType); - return new RuleBuilderRuleOpClassifier(); + return new RuleBuilderRuleOpClassifier(AppliesTypeEnum.TYPES, Collections.singleton(theType)); } private class RuleBuilderRuleOpClassifier implements IAuthRuleBuilderRuleOpClassifier { @@ -328,21 +331,26 @@ public class RuleBuilder implements IAuthRuleBuilder { private ClassifierTypeEnum myClassifierType; private String myInCompartmentName; private Collection myInCompartmentOwners; - private List myAppliesToInstances; + private Collection myAppliesToInstances; + private final AppliesTypeEnum myAppliesTo; + private final Set myAppliesToTypes; /** * Constructor */ - RuleBuilderRuleOpClassifier() { + RuleBuilderRuleOpClassifier(AppliesTypeEnum theAppliesTo, Set> theAppliesToTypes) { super(); + myAppliesTo = theAppliesTo; + myAppliesToTypes=theAppliesToTypes; } /** * Constructor */ - RuleBuilderRuleOpClassifier(List theAppliesToInstances) { + RuleBuilderRuleOpClassifier(Collection theAppliesToInstances) { myAppliesToInstances = theAppliesToInstances; myAppliesTo = AppliesTypeEnum.INSTANCES; + myAppliesToTypes = null; } private IAuthRuleBuilderRuleOpClassifierFinished finished() { @@ -554,6 +562,10 @@ public class RuleBuilder implements IAuthRuleBuilder { private class PatchBuilder implements IAuthRuleBuilderPatch { + public PatchBuilder() { + super(); + } + @Override public IAuthRuleFinished allRequests() { BaseRule rule = new RuleImplPatch(myRuleName) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index 7ed4209830d..c818140ba3d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java @@ -20,10 +20,7 @@ import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -37,9 +34,9 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * 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. @@ -48,6 +45,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * #L% */ +@SuppressWarnings("EnumSwitchStatementWhichMissesCases") class RuleImplOp extends BaseRule /* implements IAuthRule */ { private AppliesTypeEnum myAppliesTo; @@ -57,7 +55,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { private ClassifierTypeEnum myClassifierType; private RuleOpEnum myOp; private TransactionAppliesToEnum myTransactionAppliesToOp; - private List myAppliesToInstances; + private Collection myAppliesToInstances; /** * Constructor @@ -77,7 +75,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { FhirContext ctx = theRequestDetails.getServer().getFhirContext(); IBaseResource appliesToResource; - IIdType appliesToResourceId = null; + Collection appliesToResourceId = null; String appliesToResourceType = null; Map appliesToSearchParams = null; switch (myOp) { @@ -90,7 +88,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { switch (theOperation) { case READ: case VREAD: - appliesToResourceId = theInputResourceId; + appliesToResourceId = Collections.singleton(theInputResourceId); appliesToResourceType = theInputResourceId.getResourceType(); break; case SEARCH_SYSTEM: @@ -105,6 +103,29 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } appliesToResourceType = theRequestDetails.getResourceName(); appliesToSearchParams = theRequestDetails.getParameters(); + + /* + * If this is a search with an "_id" parameter, we can treat this + * as a read for the given resource ID(s) + */ + if (theRequestDetails.getParameters().containsKey("_id")) { + String[] idValues = theRequestDetails.getParameters().get("_id"); + appliesToResourceId = new ArrayList<>(); + for (String next : idValues) { + IIdType nextId = ctx.getVersion().newIdType().setValue(next); + if (nextId.hasIdPart()){ + if (!nextId.hasResourceType()) { + nextId = nextId.withResourceType(appliesToResourceType); + } + if (nextId.getResourceType().equals(appliesToResourceType)) { + appliesToResourceId.add(nextId); + } + } + } + if (appliesToResourceId.isEmpty()) { + appliesToResourceId = null; + } + } break; case HISTORY_TYPE: if (theFlags.contains(AuthorizationFlagsEnum.NO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { @@ -116,7 +137,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { if (theFlags.contains(AuthorizationFlagsEnum.NO_NOT_PROACTIVELY_BLOCK_COMPARTMENT_READ_ACCESS)) { return new Verdict(PolicyEnum.ALLOW, this); } - appliesToResourceId = theInputResourceId; + appliesToResourceId = Collections.singleton(theInputResourceId); break; case GET_PAGE: return new Verdict(PolicyEnum.ALLOW, this); @@ -145,7 +166,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } appliesToResource = theOutputResource; if (theOutputResource != null) { - appliesToResourceId = theOutputResource.getIdElement(); + appliesToResourceId = Collections.singleton(theOutputResource.getIdElement()); } break; case WRITE: @@ -160,7 +181,9 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { case META_ADD: case META_DELETE: appliesToResource = theInputResource; - appliesToResourceId = theInputResourceId; + if (theInputResourceId != null) { + appliesToResourceId = Collections.singletonList(theInputResourceId); + } break; default: return null; @@ -291,19 +314,29 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { switch (myAppliesTo) { case INSTANCES: - if (appliesToResourceId != null) { - for (IIdType next : myAppliesToInstances) { - if (isNotBlank(next.getResourceType())) { - if (!next.getResourceType().equals(appliesToResourceId.getResourceType())) { + if (appliesToResourceId != null && appliesToResourceId.size() > 0) { + int haveMatches = 0; + for (IIdType requestAppliesToResource : appliesToResourceId) { + + for (IIdType next : myAppliesToInstances) { + if (isNotBlank(next.getResourceType())) { + if (!next.getResourceType().equals(requestAppliesToResource.getResourceType())) { + continue; + } + } + if (!next.getIdPart().equals(requestAppliesToResource.getIdPart())) { continue; } + if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource)) { + return null; + } + haveMatches++; + break; } - if (!next.getIdPart().equals(appliesToResourceId.getIdPart())) { - continue; - } - if (!applyTesters(theOperation, theRequestDetails, theInputResourceId, theInputResource, theOutputResource)) { - return null; - } + + } + + if (haveMatches == appliesToResourceId.size()) { return newVerdict(); } } @@ -326,10 +359,14 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { } } } - if (appliesToResourceId != null && appliesToResourceId.hasResourceType()) { - Class type = theRequestDetails.getServer().getFhirContext().getResourceDefinition(appliesToResourceId.getResourceType()).getImplementingClass(); - if (myAppliesToTypes.contains(type) == false) { - return null; + if (appliesToResourceId != null) { + for (IIdType nextRequestAppliesToResourceId : appliesToResourceId) { + if (nextRequestAppliesToResourceId.hasResourceType()) { + Class type = theRequestDetails.getServer().getFhirContext().getResourceDefinition(nextRequestAppliesToResourceId.getResourceType()).getImplementingClass(); + if (myAppliesToTypes.contains(type) == false) { + return null; + } + } } } if (appliesToResourceType != null) { @@ -356,6 +393,16 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { case IN_COMPARTMENT: FhirTerser t = ctx.newTerser(); boolean foundMatch = false; + + if (appliesToResourceId != null && appliesToResourceId.size() > 0) { + boolean haveOwnersForAll = appliesToResourceId + .stream() + .allMatch(n -> myClassifierCompartmentOwners.contains(n.toUnqualifiedVersionless())); + if (haveOwnersForAll) { + foundMatch = true; + } + } + for (IIdType next : myClassifierCompartmentOwners) { if (appliesToResource != null) { if (t.isSourceInCompartmentForTarget(myClassifierCompartmentName, appliesToResource, next)) { @@ -363,12 +410,6 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { break; } } - if (appliesToResourceId != null && appliesToResourceId.hasResourceType() && appliesToResourceId.hasIdPart()) { - if (appliesToResourceId.toUnqualifiedVersionless().getValue().equals(next.toUnqualifiedVersionless().getValue())) { - foundMatch = true; - break; - } - } /* * If the client has permission to read compartment @@ -490,7 +531,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { myAppliesTo = theAppliesTo; } - public void setAppliesToInstances(List theAppliesToInstances) { + public void setAppliesToInstances(Collection theAppliesToInstances) { myAppliesToInstances = theAppliesToInstances; } 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 index 92bbb719633..9317cfcdef7 100644 --- 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 @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.server.interceptor.auth; +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2019 University Health Network + * %% + * 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. + * #L% + */ + import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; @@ -51,9 +71,12 @@ public abstract class SearchNarrowingInterceptor extends InterceptorAdapter { *

* * @param theRequestDetails The individual request currently being applied + * @return The list of allowed compartments and instances that should be used + * for search narrowing. If this method returns null, no narrowing will + * be performed */ protected AuthorizedList buildAuthorizedList(@SuppressWarnings("unused") RequestDetails theRequestDetails) { - return new AuthorizedList(); + return null; } @@ -71,16 +94,19 @@ public abstract class SearchNarrowingInterceptor extends InterceptorAdapter { RuntimeResourceDefinition resDef = ctx.getResourceDefinition(theRequestDetails.getResourceName()); HashMap> parameterToOrValues = new HashMap<>(); AuthorizedList authorizedList = buildAuthorizedList(theRequestDetails); + if (authorizedList == null) { + return true; + } /* * Create a map of search parameter values that need to be added to the * given request */ - Collection compartments = authorizedList.getCompartments(); + Collection compartments = authorizedList.getAllowedCompartments(); if (compartments != null) { processResourcesOrCompartments(theRequestDetails, resDef, parameterToOrValues, compartments, true); } - Collection resources = authorizedList.getResources(); + Collection resources = authorizedList.getAllowedInstances(); if (resources != null) { processResourcesOrCompartments(theRequestDetails, resDef, parameterToOrValues, resources, false); } @@ -147,7 +173,7 @@ public abstract class SearchNarrowingInterceptor extends InterceptorAdapter { private void processResourcesOrCompartments(RequestDetails theRequestDetails, RuntimeResourceDefinition theResDef, HashMap> theParameterToOrValues, Collection theResourcesOrCompartments, boolean theAreCompartments) { String lastCompartmentName = null; - String lastSearchParamName=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('/')); diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java new file mode 100644 index 00000000000..70f9afabd61 --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -0,0 +1,14 @@ +package ca.uhn.fhir.rest.server.interceptor.auth; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class RuleBuilderTest { + + @Test + public void testCollapseReadInstancesIntoSingleRule() { + + } + +} diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java index 603194a0e35..5fb6b005d8b 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java @@ -9,6 +9,7 @@ import ca.uhn.fhir.rest.api.*; import ca.uhn.fhir.rest.api.server.IRequestOperationCallback; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.ReferenceParam; +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; @@ -629,7 +630,7 @@ public class AuthorizationInterceptorDstu3Test { httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8))); status = ourClient.execute(httpPost); responseString = extractResponseAndClose(status); - assertEquals(responseString,403, status.getStatusLine().getStatusCode()); + assertEquals(responseString, 403, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); bundle.getEntry().clear(); @@ -640,7 +641,7 @@ public class AuthorizationInterceptorDstu3Test { httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8))); status = ourClient.execute(httpPost); responseString = extractResponseAndClose(status); - assertEquals(responseString,200, status.getStatusLine().getStatusCode()); + assertEquals(responseString, 200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); ourHitMethod = false; @@ -652,7 +653,7 @@ public class AuthorizationInterceptorDstu3Test { httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8))); status = ourClient.execute(httpPost); responseString = extractResponseAndClose(status); - assertEquals(responseString,403, status.getStatusLine().getStatusCode()); + assertEquals(responseString, 403, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); ourHitMethod = false; @@ -664,7 +665,7 @@ public class AuthorizationInterceptorDstu3Test { httpPost.setEntity(new StringEntity(ourCtx.newJsonParser().encodeResourceToString(bundle), ContentType.create(Constants.CT_FHIR_JSON_NEW, Charsets.UTF_8))); status = ourClient.execute(httpPost); responseString = extractResponseAndClose(status); - assertEquals(responseString,200, status.getStatusLine().getStatusCode()); + assertEquals(responseString, 200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); } @@ -2499,6 +2500,73 @@ public class AuthorizationInterceptorDstu3Test { } + @Test + public void testReadByInstanceAllowsTargetedSearch() throws Exception { + ourConditionalCreateId = "1"; + + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("Rule 1").read().instance("Patient/900").andThen() + .allow("Rule 1").read().instance("Patient/700").andThen() + .build(); + } + }); + + HttpResponse status; + String response; + HttpGet httpGet; + ourReturn = Collections.singletonList(createPatient(900)); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=900"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=Patient/900"); + status = ourClient.execute(httpGet); + extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=901"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertEquals(ERR403, response); + assertFalse(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=Patient/901"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertEquals(ERR403, response); + assertFalse(ourHitMethod); + + ourHitMethod = false; + // technically this is invalid, but just in case.. + httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation?_id=Patient/901"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertEquals(ERR403, response); + assertFalse(ourHitMethod); + + ourHitMethod = false; + httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation?_id=901"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertEquals(ERR403, response); + assertFalse(ourHitMethod); + } + @Test public void testReadPageRight() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @@ -3205,45 +3273,6 @@ public class AuthorizationInterceptorDstu3Test { assertTrue(ourHitMethod); } - @AfterClass - public static void afterClassClearContext() throws Exception { - ourServer.stop(); - TestUtil.clearAllStaticFieldsForUnitTest(); - } - - @BeforeClass - public static void beforeClass() throws Exception { - - ourPort = PortUtil.findFreePort(); - ourServer = new Server(ourPort); - - DummyPatientResourceProvider patProvider = new DummyPatientResourceProvider(); - DummyObservationResourceProvider obsProv = new DummyObservationResourceProvider(); - DummyOrganizationResourceProvider orgProv = new DummyOrganizationResourceProvider(); - DummyEncounterResourceProvider encProv = new DummyEncounterResourceProvider(); - DummyCarePlanResourceProvider cpProv = new DummyCarePlanResourceProvider(); - DummyDiagnosticReportResourceProvider drProv = new DummyDiagnosticReportResourceProvider(); - DummyMessageHeaderResourceProvider mshProv = new DummyMessageHeaderResourceProvider(); - PlainProvider plainProvider = new PlainProvider(); - - ServletHandler proxyHandler = new ServletHandler(); - ourServlet = new RestfulServer(ourCtx); - ourServlet.setFhirContext(ourCtx); - ourServlet.setResourceProviders(patProvider, obsProv, encProv, cpProv, orgProv, drProv, mshProv); - ourServlet.setPlainProviders(plainProvider); - ourServlet.setPagingProvider(new FifoMemoryPagingProvider(100)); - ServletHolder servletHolder = new ServletHolder(ourServlet); - proxyHandler.addServletWithMapping(servletHolder, "/*"); - ourServer.setHandler(proxyHandler); - ourServer.start(); - - PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); - HttpClientBuilder builder = HttpClientBuilder.create(); - builder.setConnectionManager(connectionManager); - ourClient = builder.build(); - - } - public static class DummyCarePlanResourceProvider implements IResourceProvider { @Override @@ -3314,7 +3343,7 @@ public class AuthorizationInterceptorDstu3Test { } @Operation(name = "process-message", idempotent = true) - public Parameters operation0(@OperationParam(name="content") Bundle theInput) { + public Parameters operation0(@OperationParam(name = "content") Bundle theInput) { ourHitMethod = true; return (Parameters) new Parameters().setId("1"); } @@ -3397,7 +3426,9 @@ public class AuthorizationInterceptorDstu3Test { } @Search() - public List search(@OptionalParam(name = "subject") ReferenceParam theSubject) { + public List search( + @OptionalParam(name = "_id") TokenAndListParam theIds, + @OptionalParam(name = "subject") ReferenceParam theSubject) { ourHitMethod = true; return ourReturn; } @@ -3591,7 +3622,7 @@ public class AuthorizationInterceptorDstu3Test { @Transaction() public Bundle search(IRequestOperationCallback theRequestOperationCallback, @TransactionParam Bundle theInput) { ourHitMethod = true; - if (ourDeleted != null){ + if (ourDeleted != null) { for (IBaseResource next : ourDeleted) { theRequestOperationCallback.resourceDeleted(next); } @@ -3601,6 +3632,45 @@ public class AuthorizationInterceptorDstu3Test { } + @AfterClass + public static void afterClassClearContext() throws Exception { + ourServer.stop(); + TestUtil.clearAllStaticFieldsForUnitTest(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + + ourPort = PortUtil.findFreePort(); + ourServer = new Server(ourPort); + + DummyPatientResourceProvider patProvider = new DummyPatientResourceProvider(); + DummyObservationResourceProvider obsProv = new DummyObservationResourceProvider(); + DummyOrganizationResourceProvider orgProv = new DummyOrganizationResourceProvider(); + DummyEncounterResourceProvider encProv = new DummyEncounterResourceProvider(); + DummyCarePlanResourceProvider cpProv = new DummyCarePlanResourceProvider(); + DummyDiagnosticReportResourceProvider drProv = new DummyDiagnosticReportResourceProvider(); + DummyMessageHeaderResourceProvider mshProv = new DummyMessageHeaderResourceProvider(); + PlainProvider plainProvider = new PlainProvider(); + + ServletHandler proxyHandler = new ServletHandler(); + ourServlet = new RestfulServer(ourCtx); + ourServlet.setFhirContext(ourCtx); + ourServlet.setResourceProviders(patProvider, obsProv, encProv, cpProv, orgProv, drProv, mshProv); + ourServlet.setPlainProviders(plainProvider); + ourServlet.setPagingProvider(new FifoMemoryPagingProvider(100)); + ourServlet.setDefaultResponseEncoding(EncodingEnum.JSON); + ServletHolder servletHolder = new ServletHolder(ourServlet); + proxyHandler.addServletWithMapping(servletHolder, "/*"); + ourServer.setHandler(proxyHandler); + ourServer.start(); + + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); + HttpClientBuilder builder = HttpClientBuilder.create(); + builder.setConnectionManager(connectionManager); + ourClient = builder.build(); + + } } 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 index f0ce3456333..ccd6c11a2c2 100644 --- 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 @@ -68,6 +68,24 @@ public class SearchNarrowingInterceptorTest { ourNextCompartmentList = null; } + @Test + public void testReturnNull() { + + ourNextCompartmentList = null; + + ourClient + .search() + .forResource("Patient") + .execute(); + + assertEquals("Patient.search", ourLastHitMethod); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertNull(ourLastPatientParam); + assertNull(ourLastIdParam); + } + @Test public void testNarrowObservationsByPatientContext_ClientRequestedNoParams() { @@ -259,7 +277,9 @@ public class SearchNarrowingInterceptorTest { private static class MySearchNarrowingInterceptor extends SearchNarrowingInterceptor { @Override protected AuthorizedList buildAuthorizedList(RequestDetails theRequestDetails) { - Validate.notNull(ourNextCompartmentList); + if (ourNextCompartmentList == null) { + return null; + } return ourNextCompartmentList; } }