fix false rule matches (#2762)

* fixed an issue with matching compartments to lists of ids

* change log

* Added extra guards against false rule matches.

* changelog

* fix inCompartment

* cleanup
This commit is contained in:
Ken Stevens 2021-06-27 10:34:32 -04:00 committed by GitHub
parent 5565bf9930
commit 9500a1a7de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 100 additions and 6 deletions

View File

@ -0,0 +1,4 @@
---
type: fix
issue: 2762
title: "A regression was introduced in 2760 where a READ compartment could get collapsed into a WRITE compartment. This has been corrected."

View File

@ -507,23 +507,23 @@ public class RuleBuilder implements IAuthRuleBuilder {
Validate.notBlank(theCompartmentName, "theCompartmentName must not be null"); Validate.notBlank(theCompartmentName, "theCompartmentName must not be null");
Validate.notNull(theOwner, "theOwner must not be null"); Validate.notNull(theOwner, "theOwner must not be null");
validateOwner(theOwner); validateOwner(theOwner);
Optional<RuleImplOp> oRule = findRuleByAppliesToAndCompartmentName(theCompartmentName); myClassifierType = ClassifierTypeEnum.IN_COMPARTMENT;
myInCompartmentName = theCompartmentName;
Optional<RuleImplOp> oRule = findMatchingRule();
if (oRule.isPresent()) { if (oRule.isPresent()) {
RuleImplOp rule = oRule.get(); RuleImplOp rule = oRule.get();
rule.addClassifierCompartmentOwner(theOwner); rule.addClassifierCompartmentOwner(theOwner);
return new RuleBuilderFinished(rule); return new RuleBuilderFinished(rule);
} }
myInCompartmentName = theCompartmentName;
myInCompartmentOwners = Collections.singletonList(theOwner); myInCompartmentOwners = Collections.singletonList(theOwner);
myClassifierType = ClassifierTypeEnum.IN_COMPARTMENT;
return finished(); return finished();
} }
private Optional<RuleImplOp> findRuleByAppliesToAndCompartmentName(String theCompartmentName) { private Optional<RuleImplOp> findMatchingRule() {
return myRules.stream() return myRules.stream()
.filter(RuleImplOp.class::isInstance) .filter(RuleImplOp.class::isInstance)
.map(RuleImplOp.class::cast) .map(RuleImplOp.class::cast)
.filter(rule -> rule.matches(myAppliesTo, myAppliesToInstances, myAppliesToTypes, theCompartmentName)) .filter(rule -> rule.matches(myRuleOp, myAppliesTo, myAppliesToInstances, myAppliesToTypes, myClassifierType, myInCompartmentName))
.findFirst(); .findFirst();
} }

View File

@ -639,7 +639,13 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
myClassifierCompartmentOwners = newList; myClassifierCompartmentOwners = newList;
} }
public boolean matches(AppliesTypeEnum theAppliesTo, Collection<IIdType> theAppliesToInstances, Set<String> theAppliesToTypes, String theCompartmentName) { public boolean matches(RuleOpEnum theRuleOp, AppliesTypeEnum theAppliesTo, Collection<IIdType> theAppliesToInstances, Set<String> theAppliesToTypes, ClassifierTypeEnum theClassifierType, String theCompartmentName) {
if (theRuleOp != myOp ||
theAppliesTo != myAppliesTo ||
theClassifierType != myClassifierType) {
return false;
}
switch (theAppliesTo) { switch (theAppliesTo) {
case TYPES: case TYPES:
return theAppliesToTypes.equals(myAppliesToTypes) && theCompartmentName.equals(myClassifierCompartmentName); return theAppliesToTypes.equals(myAppliesToTypes) && theCompartmentName.equals(myClassifierCompartmentName);

View File

@ -1,11 +1,95 @@
package ca.uhn.fhir.rest.server.interceptor.auth; package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.model.primitive.IdDt;
import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class RuleImplOpTest { public class RuleImplOpTest {
public static final String COMPARTMENT_NAME = "Patient";
private static final ClassifierTypeEnum CLASSIFIER_TYPE = ClassifierTypeEnum.IN_COMPARTMENT;
@Test @Test
public void testToString() { public void testToString() {
new RuleImplOp("").toString(); new RuleImplOp("").toString();
} }
@Test
public void testMatchesTypes() {
RuleImplOp aRuleOp = new RuleImplOp("a");
aRuleOp.setOp(RuleOpEnum.READ);
aRuleOp.setAppliesTo(AppliesTypeEnum.TYPES);
aRuleOp.setClassifierType(CLASSIFIER_TYPE);
Set<String> types = new HashSet<>();
types.add("ABC");
types.add("DEF");
aRuleOp.setAppliesToTypes(types);
aRuleOp.setClassifierCompartmentName(COMPARTMENT_NAME);
Set<String> matchTypes = new HashSet<>();
matchTypes.add("ABC");
matchTypes.add("DEF");
Set<String> noMatchTypes = new HashSet<>();
noMatchTypes.add("ABC");
noMatchTypes.add("XYZ");
assertTrue(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, Collections.emptyList(), matchTypes, CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, Collections.emptyList(), noMatchTypes, CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.WRITE, AppliesTypeEnum.TYPES, Collections.emptyList(), matchTypes, CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.INSTANCES, Collections.emptyList(), matchTypes, CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, Collections.emptyList(), matchTypes, CLASSIFIER_TYPE, "Observation"));
}
@Test
public void testMatchesInstances() {
RuleImplOp aRuleOp = new RuleImplOp("a");
aRuleOp.setOp(RuleOpEnum.READ);
aRuleOp.setAppliesTo(AppliesTypeEnum.INSTANCES);
aRuleOp.setClassifierType(CLASSIFIER_TYPE);
List<IIdType> instances = new ArrayList<>();
instances.add(new IdDt("ABC"));
instances.add(new IdDt("DEF"));
aRuleOp.setAppliesToInstances(instances);
aRuleOp.setClassifierCompartmentName(COMPARTMENT_NAME);
List<IIdType> matchInstances = new ArrayList<>();
matchInstances.add(new IdDt("ABC"));
matchInstances.add(new IdDt("DEF"));
List<IIdType> noMatchInstances = new ArrayList<>();
noMatchInstances.add(new IdDt("ABC"));
noMatchInstances.add(new IdDt("XYZ"));
assertTrue(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.INSTANCES, matchInstances, Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.INSTANCES, noMatchInstances, Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.INSTANCES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.WRITE, AppliesTypeEnum.INSTANCES, matchInstances, Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, matchInstances, Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.INSTANCES, matchInstances, Collections.emptySet(), CLASSIFIER_TYPE, "Observation"));
}
@Test
public void testMatchesAllResources() {
RuleImplOp aRuleOp = new RuleImplOp("a");
aRuleOp.setOp(RuleOpEnum.READ);
aRuleOp.setAppliesTo(AppliesTypeEnum.ALL_RESOURCES);
aRuleOp.setClassifierType(CLASSIFIER_TYPE);
aRuleOp.setClassifierCompartmentName("Patient");
assertTrue(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.ALL_RESOURCES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.WRITE, AppliesTypeEnum.ALL_RESOURCES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.TYPES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Patient"));
assertFalse(aRuleOp.matches(RuleOpEnum.READ, AppliesTypeEnum.ALL_RESOURCES, Collections.emptyList(), Collections.emptySet(), CLASSIFIER_TYPE, "Observation"));
}
} }