Enhance RuleBuilder code to support multiple instances (#5852)

* Overhaul bulk export permissions.

* Overhaul bulk export permissions.

* Small tweak to rule builder.

* Cleanup validation.

* Cleanup validation.

* Code review feedback.
This commit is contained in:
Luke deGruchy 2024-04-22 09:39:12 -04:00 committed by GitHub
parent d05009e526
commit 4322b83257
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 143 additions and 2 deletions

View File

@ -0,0 +1,4 @@
---
type: add
issue: 5861
title: "Enhance RuleBuilder code to support multiple instance IDs."

View File

@ -22,6 +22,8 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import java.util.Collection;
public interface IAuthRuleBuilderOperationNamed {
/**
@ -44,6 +46,8 @@ public interface IAuthRuleBuilderOperationNamed {
*/
IAuthRuleBuilderOperationNamedAndScoped onInstance(IIdType theInstanceId);
IAuthRuleBuilderOperationNamedAndScoped onInstances(Collection<IIdType> theInstanceIds);
/**
* Rule applies to invocations of this operation at the <code>instance</code> level on any instance of the given type
*/

View File

@ -22,6 +22,9 @@ package ca.uhn.fhir.rest.server.interceptor.auth;
import jakarta.annotation.Nonnull;
import org.hl7.fhir.instance.model.api.IIdType;
import java.util.Collection;
import java.util.stream.Collectors;
/**
* @since 5.5.0
*/
@ -58,6 +61,14 @@ public interface IAuthRuleBuilderRuleBulkExport {
return patientExportOnPatient(theFocusResourceId.getValue());
}
default IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatients(
@Nonnull Collection<IIdType> theFocusResourceIds) {
return patientExportOnPatientStrings(
theFocusResourceIds.stream().map(IIdType::getValue).collect(Collectors.toList()));
}
IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatientStrings(Collection<String> theFocusResourceIds);
/**
* Allow/deny <b>patient-level</b> export rule applies to the Group with the given resource ID, e.g. <code>Group/123</code>
*

View File

@ -736,6 +736,20 @@ public class RuleBuilder implements IAuthRuleBuilder {
return new RuleBuilderOperationNamedAndScoped(rule);
}
@Override
public IAuthRuleBuilderOperationNamedAndScoped onInstances(Collection<IIdType> theInstanceIds) {
Validate.notNull(theInstanceIds, "theInstanceIds must not be null");
theInstanceIds.forEach(instanceId -> Validate.notBlank(
instanceId.getResourceType(),
"at least one of theInstanceIds does not have a resource type"));
theInstanceIds.forEach(instanceId -> Validate.notBlank(
instanceId.getIdPart(), "at least one of theInstanceIds does not have an ID part"));
final OperationRule rule = createRule();
rule.appliesToInstances(new ArrayList<>(theInstanceIds));
return new RuleBuilderOperationNamedAndScoped(rule);
}
@Override
public IAuthRuleBuilderOperationNamedAndScoped onInstancesOfType(
Class<? extends IBaseResource> theType) {
@ -900,6 +914,26 @@ public class RuleBuilder implements IAuthRuleBuilder {
return new RuleBuilderBulkExportWithTarget(ruleBulkExport);
}
@Override
public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnPatientStrings(
@Nonnull Collection<String> theFocusResourceIds) {
if (ruleBulkExport == null) {
RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName);
rule.setAppliesToPatientExport(theFocusResourceIds);
rule.setMode(myRuleMode);
ruleBulkExport = rule;
} else {
ruleBulkExport.setAppliesToPatientExport(theFocusResourceIds);
}
// prevent duplicate rules being added
if (!myRules.contains(ruleBulkExport)) {
myRules.add(ruleBulkExport);
}
return new RuleBuilderBulkExportWithTarget(ruleBulkExport);
}
@Override
public IAuthRuleBuilderRuleBulkExportWithTarget patientExportOnGroup(@Nonnull String theFocusResourceId) {
RuleBulkExportImpl rule = new RuleBulkExportImpl(myRuleName);

View File

@ -24,6 +24,7 @@ import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters;
import com.google.common.annotations.VisibleForTesting;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
@ -193,6 +194,11 @@ public class RuleBulkExportImpl extends BaseRule {
myPatientIds.add(thePatientId);
}
public void setAppliesToPatientExport(Collection<String> thePatientIds) {
myWantExportStyle = BulkExportJobParameters.ExportStyle.PATIENT;
myPatientIds.addAll(thePatientIds);
}
public void setAppliesToSystem() {
myWantExportStyle = BulkExportJobParameters.ExportStyle.SYSTEM;
}
@ -212,4 +218,14 @@ public class RuleBulkExportImpl extends BaseRule {
BulkExportJobParameters.ExportStyle getWantExportStyle() {
return myWantExportStyle;
}
@VisibleForTesting
Collection<String> getPatientIds() {
return myPatientIds;
}
@VisibleForTesting
Collection<String> getResourceTypes() {
return myResourceTypes;
}
}

View File

@ -1,16 +1,23 @@
package ca.uhn.fhir.rest.server.interceptor.auth;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.server.provider.ProviderConstants;
import com.google.common.collect.Lists;
import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Stream;
import static org.hamcrest.Matchers.contains;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.mockito.Mockito.mock;
public class RuleBuilderTest {
@ -98,7 +105,72 @@ public class RuleBuilderTest {
builder.allow().bulkExport().patientExportOnPatient("Patient/pat2").withResourceTypes(resourceTypes);
List<IAuthRule> rules = builder.build();
assertEquals(rules.size(),1);
assertTrue(rules.get(0) instanceof RuleBulkExportImpl);
assertInstanceOf(RuleBulkExportImpl.class, rules.get(0));
}
public static Stream<Arguments> multipleInstancesParams() {
return Stream.of(
Arguments.of(List.of("Patient/pat1"), List.of("Patient"), PolicyEnum.ALLOW),
Arguments.of(List.of("Patient/pat1", "Patient/pat2"), List.of("Patient"), PolicyEnum.ALLOW),
Arguments.of(List.of("Patient/pat1", "Patient/pat2"), List.of("Patient", "Observation"), PolicyEnum.ALLOW),
Arguments.of(List.of("Patient/pat1"), List.of("Patient"), PolicyEnum.DENY),
Arguments.of(List.of("Patient/pat1", "Patient/pat2"), List.of("Patient"), PolicyEnum.DENY),
Arguments.of(List.of("Patient/pat1", "Patient/pat2"), List.of("Patient", "Observation"), PolicyEnum.DENY)
);
}
@ParameterizedTest
@MethodSource("multipleInstancesParams")
public void testBulkExport_PatientExportOnPatients_MultiplePatientsSingleRule(Collection<String> theExpectedPatientIds, Collection<String> theExpectedResourceTypes, PolicyEnum thePolicyEnum) {
final RuleBuilder builder = new RuleBuilder();
final IAuthRuleBuilderRule rule = switch (thePolicyEnum) {
case ALLOW -> builder.allow();
case DENY -> builder.deny();
};
rule.bulkExport().patientExportOnPatientStrings(theExpectedPatientIds).withResourceTypes(theExpectedResourceTypes);
final List<IAuthRule> rules = builder.build();
assertEquals(rules.size(),1);
final IAuthRule authRule = rules.get(0);
assertInstanceOf(RuleBulkExportImpl.class, authRule);
final RuleBulkExportImpl ruleBulkExport = (RuleBulkExportImpl) authRule;
assertEquals(theExpectedPatientIds, ruleBulkExport.getPatientIds());
assertEquals(theExpectedResourceTypes, ruleBulkExport.getResourceTypes());
assertEquals(thePolicyEnum, ruleBulkExport.getMode());
}
public static Stream<Arguments> owners() {
return Stream.of(
Arguments.of(List.of(new IdDt("Patient/pat1")), PolicyEnum.ALLOW),
Arguments.of(List.of(new IdDt("Patient/pat1")), PolicyEnum.DENY),
Arguments.of(List.of(new IdDt("Patient/pat1"), new IdDt("Patient/pat2")), PolicyEnum.ALLOW),
Arguments.of(List.of(new IdDt("Patient/pat1"), new IdDt("Patient/pat2")), PolicyEnum.DENY)
);
}
@ParameterizedTest
@MethodSource("owners")
public void testBulkExport_PatientExportOnPatients_onInstances(List<IIdType> theExpectedOwners, PolicyEnum thePolicyEnum) {
final RuleBuilder builder = new RuleBuilder();
final IAuthRuleBuilderRule rule = switch (thePolicyEnum) {
case ALLOW -> builder.allow();
case DENY -> builder.deny();
};
final List<IAuthRule> rules = rule
.operation()
.named(ProviderConstants.OPERATION_EXPORT)
.onInstances(theExpectedOwners)
.andAllowAllResponses()
.andThen()
.build();
assertEquals(rules.size(),1);
final IAuthRule authRule = rules.get(0);
assertInstanceOf(OperationRule.class, authRule);
final OperationRule operationRule = (OperationRule) authRule;
assertEquals(theExpectedOwners, operationRule.getAppliesToIds());
assertEquals(ProviderConstants.OPERATION_EXPORT, operationRule.getOperationName());
assertEquals(thePolicyEnum, operationRule.getMode());
}
@Test