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

First commit: Patch bad implementation of RuleBulkExportImpl to fix … (#4986) 

* First commit:  Patch bad implementation of RuleBulkExportImpl to fix this use case.

* Fix changelog.  Fix comments on unit tests.

* Fix TODO with correct issue number.
This commit is contained in:
Luke deGruchy 2023-06-15 11:34:00 -04:00 committed by GitHub
parent 048c1f99bb
commit f79ac1992e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 129 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4989-fhir-op-initiate-bulk-export-patient-incorrect-permissions.yaml Normal file
View File

@ -0,0 +1,5 @@
---
type: fix
issue: 4989
title: "Bulk export with patient ID specific user rules/permissions will fail with Parameters that include that patient ID in a _typeFilter.
This has been fixed."

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

@ -37,6 +37,7 @@ import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
public class RuleBulkExportImpl extends BaseRule {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RuleBulkExportImpl.class);
private String myGroupId;
private String myPatientId;
private BulkDataExportOptions.ExportStyle myWantExportStyle;
@ -89,14 +90,37 @@ public class RuleBulkExportImpl extends BaseRule {
// TODO This is a _bad bad bad implementation_ but we are out of time.
// 1. If a claimed resource ID is present in the parameters, and the permission contains one, check for membership
// 2. If not a member, Deny.
if (myWantExportStyle == BulkDataExportOptions.ExportStyle.PATIENT && isNotBlank(myPatientId) && options.getPatientIds() != null) {
String expectedPatientId = new IdDt(myPatientId).toUnqualifiedVersionless().getValue();
String actualPatientIds = options.getPatientIds().stream().map(t -> t.toUnqualifiedVersionless().getValue()).collect(Collectors.joining(","));
if (actualPatientIds.contains(expectedPatientId)) {
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier);
} else {
if (myWantExportStyle == BulkDataExportOptions.ExportStyle.PATIENT && isNotBlank(myPatientId)) {
final String expectedPatientId = new IdDt(myPatientId).toUnqualifiedVersionless().getValue();
if (options.getPatientIds() != null) {
ourLog.debug("options.getPatientIds() != null");
final String actualPatientIds = options.getPatientIds().stream()
.map(t -> t.toUnqualifiedVersionless().getValue())
.collect(Collectors.joining(","));
if (actualPatientIds.contains(expectedPatientId)) {
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier);
}
return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY,this);
}
final Set<String> filters = options.getFilters();
// TODO: LD: This admittedly adds more to the tech debt above, and should really be addressed by https://github.com/hapifhir/hapi-fhir/issues/4990
if (! filters.isEmpty()) {
ourLog.debug("filters not empty");
final Set<String> patientIdsInFilters = filters.stream()
.filter(filter -> filter.startsWith("Patient?_id="))
.map(filter -> filter.replace("?_id=", "/"))
.collect(Collectors.toUnmodifiableSet());
if (patientIdsInFilters.contains(expectedPatientId)) {
return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource, theRuleApplier);
}
return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY,this);
}
ourLog.debug("patientIds and filters both empty");
}
return null;
}

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

@ -156,4 +156,98 @@ public class RuleBulkExportImplTest {
//Then: We make no claims about type-level export on Patient.
assertEquals(null, verdict);
}
@Test
public void testPatientExportRulesOnTypeLevelExportWithTypeFilterResourceTypePatient() {
//Given
final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b");
myRule.setAppliesToPatientExport("Patient/123");
myRule.setMode(PolicyEnum.ALLOW);
final BulkDataExportOptions options = new BulkDataExportOptions();
options.setExportStyle(BulkDataExportOptions.ExportStyle.PATIENT);
options.setFilters(Set.of("Patient?_id=123"));
options.setResourceTypes(Set.of("Patient"));
when(myRequestDetails.getAttribute(any())).thenReturn(options);
//When
final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
//Then: The patient IDs match so this is permitted
assertEquals(PolicyEnum.ALLOW, verdict.getDecision());
}
@Test
public void testPatientExportRulesOnTypeLevelExportWithTypeFilterResourceTypePatientAndFilterHasResources() {
//Given
final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b");
myRule.setAppliesToPatientExport("Patient/123");
myRule.setMode(PolicyEnum.ALLOW);
final BulkDataExportOptions options = new BulkDataExportOptions();
options.setExportStyle(BulkDataExportOptions.ExportStyle.PATIENT);
options.setFilters(Set.of("Patient?_id=123"));
options.setResourceTypes(Set.of("Patient", "Condition", "Immunization"));
when(myRequestDetails.getAttribute(any())).thenReturn(options);
//When
final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
//Then: The patient IDs match so this is permitted
assertEquals(PolicyEnum.ALLOW, verdict.getDecision());
}
@Test
public void testPatientExportRulesOnTypeLevelExportWithTypeFilterResourceTypeObservation() {
//Given
final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b");
myRule.setAppliesToPatientExport("Patient/123");
myRule.setMode(PolicyEnum.ALLOW);
final BulkDataExportOptions options = new BulkDataExportOptions();
options.setExportStyle(BulkDataExportOptions.ExportStyle.PATIENT);
options.setFilters(Set.of("Patient?_id=123"));
options.setResourceTypes(Set.of("Observation"));
when(myRequestDetails.getAttribute(any())).thenReturn(options);
//When
final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
//Then: The patient IDs match so this is permitted
assertEquals(PolicyEnum.ALLOW, verdict.getDecision());
}
@Test
public void testPatientExportRulesOnTypeLevelExportWithTypeFilterNoResourceType() {
//Given
final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b");
myRule.setAppliesToPatientExport("Patient/123");
myRule.setMode(PolicyEnum.ALLOW);
final BulkDataExportOptions options = new BulkDataExportOptions();
options.setExportStyle(BulkDataExportOptions.ExportStyle.PATIENT);
options.setFilters(Set.of("Patient?_id=123"));
when(myRequestDetails.getAttribute(any())).thenReturn(options);
//When
final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
//Then: The patient IDs match so this is permitted
assertEquals(PolicyEnum.ALLOW, verdict.getDecision());
}
@Test
public void testPatientExportRulesOnTypeLevelExportWithTypeFilterMismatch() {
//Given
final RuleBulkExportImpl myRule = new RuleBulkExportImpl("b");
myRule.setAppliesToPatientExport("Patient/123");
myRule.setMode(PolicyEnum.ALLOW);
final BulkDataExportOptions options = new BulkDataExportOptions();
options.setExportStyle(BulkDataExportOptions.ExportStyle.PATIENT);
options.setFilters(Set.of("Patient?_id=456"));
options.setResourceTypes(Set.of("Patient"));
when(myRequestDetails.getAttribute(any())).thenReturn(options);
//When
final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut);
//Then: The patient IDs do NOT match so this is not permitted.
assertEquals(PolicyEnum.DENY, verdict.getDecision());
}
}