honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow AuthorizationInterceptor to read patients if the user has read 

access to the individual instance
This commit is contained in:
James Agnew 2019-01-14 13:51:52 -06:00
parent 4f5640e541
commit b8755615b2
6 changed files with 224 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java
View File

@ -60,5 +60,20 @@ public interface IAuthRuleBuilderRuleOp extends IAuthRuleBuilderAppliesTo<IAuthR
*/
IAuthRuleFinished instance(IIdType theId);
IAuthRuleFinished instances(Collection<IIdType> theInstances);
/**
* Rule applies to the resource with the given ID (e.g. <code>Patient/123</code>)
* <p>
* See the following examples which show how theId is interpreted:
* </p>
* <ul>
* <li><b><code>http://example.com/Patient/123</code></b> - Any Patient resource with the ID "123" will be matched (note: the base URL part is ignored)</li>
* <li><b><code>Patient/123</code></b> - Any Patient resource with the ID "123" will be matched</li>
* <li><b><code>123</code></b> - Any resource of any type with the ID "123" will be matched</li>
* </ul>
*
* @param theIds The IDs of the resource to apply (e.g. <code>Patient/123</code>)
* @throws IllegalArgumentException If theId does not contain an ID with at least an ID part
* @throws NullPointerException If theId is null
*/
IAuthRuleFinished instances(Collection<IIdType> theIds);
}

117
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java
View File

@ -23,6 +23,7 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import com.google.common.collect.Lists;
import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
@ -35,6 +36,8 @@ public class RuleBuilder implements IAuthRuleBuilder {
private static final String[] EMPTY_STRING_ARRAY = new String[0];
private ArrayList<IAuthRule> myRules;
private IAuthRuleBuilderRule myAllow;
private IAuthRuleBuilderRule myDeny;
public RuleBuilder() {
myRules = new ArrayList<>();
@ -42,7 +45,10 @@ public class RuleBuilder implements IAuthRuleBuilder {
@Override
public IAuthRuleBuilderRule allow() {
return allow(null);
if (myAllow == null) {
myAllow = allow(null);
}
return myAllow;
}
@Override
@ -69,7 +75,10 @@ public class RuleBuilder implements IAuthRuleBuilder {
@Override
public IAuthRuleBuilderRule deny() {
return deny(null);
if (myDeny == null) {
myDeny = deny(null);
}
return myDeny;
}
@Override
@ -172,6 +181,8 @@ public class RuleBuilder implements IAuthRuleBuilder {
private PolicyEnum myRuleMode;
private String myRuleName;
private RuleBuilderRuleOp myReadRuleBuilder;
private RuleBuilderRuleOp myWriteRuleBuilder;
RuleBuilderRule(PolicyEnum theRuleMode, String theRuleName) {
myRuleMode = theRuleMode;
@ -214,7 +225,10 @@ public class RuleBuilder implements IAuthRuleBuilder {
@Override
public IAuthRuleBuilderRuleOp read() {
return new RuleBuilderRuleOp(RuleOpEnum.READ);
if (myReadRuleBuilder == null) {
myReadRuleBuilder = new RuleBuilderRuleOp(RuleOpEnum.READ);
}
return myReadRuleBuilder;
}
@Override
@ -229,7 +243,10 @@ public class RuleBuilder implements IAuthRuleBuilder {
@Override
public IAuthRuleBuilderRuleOp write() {
return new RuleBuilderRuleOp(RuleOpEnum.WRITE);
if (myWriteRuleBuilder == null) {
myWriteRuleBuilder = new RuleBuilderRuleOp(RuleOpEnum.WRITE);
}
return myWriteRuleBuilder;
}
@Override
@ -286,6 +303,7 @@ public class RuleBuilder implements IAuthRuleBuilder {
private class RuleBuilderRuleOp implements IAuthRuleBuilderRuleOp {
private final RuleOpEnum myRuleOp;
private RuleBuilderRuleOpClassifier myInstancesBuilder;
public RuleBuilderRuleOp(RuleOpEnum theRuleOp) {
myRuleOp = theRuleOp;
@ -308,16 +326,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");
List<IIdType> instances = Collections.singletonList(theId);
List<IIdType> instances = Lists.newArrayList(theId);
return instances(instances);
}
@Override
public IAuthRuleFinished instances(Collection<IIdType> theInstances) {
public RuleBuilderFinished instances(Collection<IIdType> theInstances) {
Validate.notNull(theInstances, "theInstances must not be null");
Validate.notEmpty(theInstances, "theInstances must not be empty");
return new RuleBuilderRuleOpClassifier(theInstances).finished();
if (myInstancesBuilder == null) {
RuleBuilderRuleOpClassifier instancesBuilder = new RuleBuilderRuleOpClassifier(theInstances);
myInstancesBuilder = instancesBuilder;
return instancesBuilder.finished();
} else {
return myInstancesBuilder.addInstances(theInstances);
}
}
@Override
@ -328,12 +352,13 @@ public class RuleBuilder implements IAuthRuleBuilder {
private class RuleBuilderRuleOpClassifier implements IAuthRuleBuilderRuleOpClassifier {
private final AppliesTypeEnum myAppliesTo;
private final Set<?> myAppliesToTypes;
private ClassifierTypeEnum myClassifierType;
private String myInCompartmentName;
private Collection<? extends IIdType> myInCompartmentOwners;
private Collection<IIdType> myAppliesToInstances;
private final AppliesTypeEnum myAppliesTo;
private final Set<?> myAppliesToTypes;
private RuleImplOp myRule;
/**
* Constructor
@ -353,20 +378,20 @@ public class RuleBuilder implements IAuthRuleBuilder {
myAppliesToTypes = null;
}
private IAuthRuleBuilderRuleOpClassifierFinished finished() {
private RuleBuilderFinished finished() {
Validate.isTrue(myRule == null, "Can not call finished() twice");
myRule = new RuleImplOp(myRuleName);
myRule.setMode(myRuleMode);
myRule.setOp(myRuleOp);
myRule.setAppliesTo(myAppliesTo);
myRule.setAppliesToTypes(myAppliesToTypes);
myRule.setAppliesToInstances(myAppliesToInstances);
myRule.setClassifierType(myClassifierType);
myRule.setClassifierCompartmentName(myInCompartmentName);
myRule.setClassifierCompartmentOwners(myInCompartmentOwners);
myRules.add(myRule);
RuleImplOp rule = new RuleImplOp(myRuleName);
rule.setMode(myRuleMode);
rule.setOp(myRuleOp);
rule.setAppliesTo(myAppliesTo);
rule.setAppliesToTypes(myAppliesToTypes);
rule.setAppliesToInstances(myAppliesToInstances);
rule.setClassifierType(myClassifierType);
rule.setClassifierCompartmentName(myInCompartmentName);
rule.setClassifierCompartmentOwners(myInCompartmentOwners);
myRules.add(rule);
return new RuleBuilderFinished(rule);
return new RuleBuilderFinished(myRule);
}
@Override
@ -405,6 +430,10 @@ public class RuleBuilder implements IAuthRuleBuilder {
return finished();
}
RuleBuilderFinished addInstances(Collection<IIdType> theInstances) {
myAppliesToInstances.addAll(theInstances);
return new RuleBuilderFinished(myRule);
}
}
}
@ -424,28 +453,6 @@ public class RuleBuilder implements IAuthRuleBuilder {
private class RuleBuilderRuleOperationNamed implements IAuthRuleBuilderOperationNamed {
private class RuleBuilderOperationNamedAndScoped implements IAuthRuleBuilderOperationNamedAndScoped {
private final OperationRule myRule;
public RuleBuilderOperationNamedAndScoped(OperationRule theRule) {
myRule = theRule;
}
@Override
public IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses() {
myRule.allowAllResponses();
myRules.add(myRule);
return new RuleBuilderFinished(myRule);
}
@Override
public IAuthRuleBuilderRuleOpClassifierFinished andRequireExplicitResponseAuthorization() {
myRules.add(myRule);
return new RuleBuilderFinished(myRule);
}
}
private String myOperationName;
RuleBuilderRuleOperationNamed(String theOperationName) {
@ -532,6 +539,28 @@ public class RuleBuilder implements IAuthRuleBuilder {
Validate.notNull(theType, "theType must not be null");
}
private class RuleBuilderOperationNamedAndScoped implements IAuthRuleBuilderOperationNamedAndScoped {
private final OperationRule myRule;
public RuleBuilderOperationNamedAndScoped(OperationRule theRule) {
myRule = theRule;
}
@Override
public IAuthRuleBuilderRuleOpClassifierFinished andAllowAllResponses() {
myRule.allowAllResponses();
myRules.add(myRule);
return new RuleBuilderFinished(myRule);
}
@Override
public IAuthRuleBuilderRuleOpClassifierFinished andRequireExplicitResponseAuthorization() {
myRules.add(myRule);
return new RuleBuilderFinished(myRule);
}
}
}
}

23
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java
View File

@ -3,15 +3,18 @@ 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.RequestTypeEnum;
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.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict;
import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.BundleUtil.BundleEntryParts;
import ca.uhn.fhir.util.FhirTerser;
import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
@ -64,6 +67,15 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
super(theRuleName);
}
@VisibleForTesting
Collection<IIdType> getAppliesToInstances() {
return myAppliesToInstances;
}
public void setAppliesToInstances(Collection<IIdType> theAppliesToInstances) {
myAppliesToInstances = theAppliesToInstances;
}
@Override
public Verdict applyRule(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IBaseResource theInputResource, IIdType theInputResourceId, IBaseResource theOutputResource,
IRuleApplier theRuleApplier, Set<AuthorizationFlagsEnum> theFlags) {
@ -111,7 +123,10 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
if (theRequestDetails.getParameters().containsKey("_id")) {
String[] idValues = theRequestDetails.getParameters().get("_id");
appliesToResourceId = new ArrayList<>();
for (String next : idValues) {
for (String nextIdValue : idValues) {
QualifiedParamList orParamList = QualifiedParamList.splitQueryStringByCommasIgnoreEscape(null, nextIdValue);
for (String next : orParamList) {
IIdType nextId = ctx.getVersion().newIdType().setValue(next);
if (nextId.hasIdPart()) {
if (!nextId.hasResourceType()) {
@ -122,6 +137,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
}
}
}
}
if (appliesToResourceId.isEmpty()) {
appliesToResourceId = null;
}
@ -340,6 +356,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
return newVerdict();
}
}
return null;
case ALL_RESOURCES:
if (appliesToResourceType != null) {
@ -531,10 +548,6 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
myAppliesTo = theAppliesTo;
}
public void setAppliesToInstances(Collection<IIdType> theAppliesToInstances) {
myAppliesToInstances = theAppliesToInstances;
}
public void setAppliesToTypes(Set<?> theAppliesToTypes) {
myAppliesToTypes = theAppliesToTypes;
}

39
hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java
View File

@ -1,14 +1,51 @@
package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.model.primitive.IdDt;
import com.google.common.collect.Lists;
import org.junit.Test;
import static org.junit.Assert.*;
import java.util.List;
import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
public class RuleBuilderTest {
/**
* If the user creates multiple rules that allow read/write of individual
* instances, we will collapse these into a single rule for performance
*/
@Test
public void testCollapseReadInstancesIntoSingleRule() {
RuleBuilder builder = new RuleBuilder();
builder.allow().read().instance(new IdDt("Patient/READ-1"));
builder.allow().write().instance(new IdDt("Patient/WRITE-1"));
builder.allow().read().instance(new IdDt("Patient/READ-2"));
builder.allow().write().instance(new IdDt("Patient/WRITE-2"));
builder.allow().read().instances(Lists.newArrayList(new IdDt("Patient/READ-3"), new IdDt("Patient/READ-4")));
builder.allow().write().instances(Lists.newArrayList(new IdDt("Patient/WRITE-3"), new IdDt("Patient/WRITE-4")));
List<IAuthRule> list = builder.build();
assertEquals(2, list.size());
assertEquals(RuleImplOp.class, list.get(0).getClass());
RuleImplOp allowRead = (RuleImplOp) list.get(0);
assertThat(allowRead.getAppliesToInstances(), contains(
new IdDt("Patient/READ-1"),
new IdDt("Patient/READ-2"),
new IdDt("Patient/READ-3"),
new IdDt("Patient/READ-4")
));
assertEquals(RuleImplOp.class, list.get(1).getClass());
RuleImplOp allowWrite = (RuleImplOp) list.get(1);
assertThat(allowWrite.getAppliesToInstances(), contains(
new IdDt("Patient/WRITE-1"),
new IdDt("Patient/WRITE-2"),
new IdDt("Patient/WRITE-3"),
new IdDt("Patient/WRITE-4")
));
}
}

94
hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java
View File

@ -2507,10 +2507,10 @@ public class AuthorizationInterceptorDstu3Test {
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder()
.allow("Rule 1").read().instance("Patient/900").andThen()
.allow("Rule 1").read().instance("Patient/700").andThen()
.build();
RuleBuilder ruleBuilder = new RuleBuilder();
ruleBuilder.allow().read().instance("Patient/900").andThen();
ruleBuilder.allow().read().instance("Patient/700").andThen();
return ruleBuilder.build();
}
});
@ -2519,51 +2519,65 @@ public class AuthorizationInterceptorDstu3Test {
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);
ourHitMethod = false;
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=900");
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=Patient/900,Patient/700");
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");
httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id=900,777");
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);
}
@ -3560,7 +3574,7 @@ public class AuthorizationInterceptorDstu3Test {
}
@Search()
public List<Resource> search(@OptionalParam(name = "_id") IdType theIdParam) {
public List<Resource> search(@OptionalParam(name = "_id") TokenAndListParam theIdParam) {
ourHitMethod = true;
return ourReturn;
}

4
src/changes/changes.xml
View File

@ -299,6 +299,10 @@
A wrapper script for Maven has been added, enabling new users to use Maven without having
to install it beforehand. Thanks to Ari Ruotsalainen for the Pull Request!
</action>
<action type="add">
AuthorizationInterceptor can now allow a user to perform a search that is scoped to a particular
resource (e.g. Patient?_id=123) if the user has read access for that specific instance.
</action>
</release>
<release version="3.6.0" date="2018-11-12" description="Food">
<action type="add">