diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3400-bulk-export-rules-incorrectly-applied-to-group-and-patient-exports.yml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3400-bulk-export-rules-incorrectly-applied-to-group-and-patient-exports.yml new file mode 100644 index 00000000000..e7b4162ad2b --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3400-bulk-export-rules-incorrectly-applied-to-group-and-patient-exports.yml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 3400 +title: "User was permitted to bulk export all groups/patients when they were unauthorized. This issue has been fixed." diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java index 81ac1d3ff39..cefe4847885 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImpl.java @@ -80,6 +80,8 @@ public class RuleBulkExportImpl extends BaseRule { String actualGroupId = options.getGroupId().toUnqualifiedVersionless().getValue(); if (Objects.equals(expectedGroupId, actualGroupId)) { return newVerdict(theOperation, theRequestDetails, theInputResource, theInputResourceId, theOutputResource); + } else { + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY,this); } } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java index db3c57194ed..609f2eaa8b7 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBulkExportImplTest.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.rest.server.interceptor.auth; import ca.uhn.fhir.interceptor.api.Pointcut; +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.BulkDataExportOptions; @@ -19,7 +20,8 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class RuleBulkExportImplTest { - + private RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; + private Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; @Mock private RequestDetails myRequestDetails; @Mock @@ -30,8 +32,6 @@ public class RuleBulkExportImplTest { @Test public void testDenyBulkRequestWithInvalidResourcesTypes() { RuleBulkExportImpl myRule = new RuleBulkExportImpl("a"); - RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; - Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; Set myTypes = new HashSet<>(); myTypes.add("Patient"); @@ -43,6 +43,7 @@ public class RuleBulkExportImplTest { BulkDataExportOptions options = new BulkDataExportOptions(); options.setResourceTypes(myWantTypes); + when(myRequestDetails.getAttribute(any())).thenReturn(options); AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); @@ -52,8 +53,6 @@ public class RuleBulkExportImplTest { @Test public void testBulkRequestWithValidResourcesTypes() { RuleBulkExportImpl myRule = new RuleBulkExportImpl("a"); - RestOperationTypeEnum myOperation = RestOperationTypeEnum.EXTENDED_OPERATION_SERVER; - Pointcut myPointcut = Pointcut.STORAGE_INITIATE_BULK_EXPORT; Set myTypes = new HashSet<>(); myTypes.add("Patient"); @@ -66,10 +65,43 @@ public class RuleBulkExportImplTest { BulkDataExportOptions options = new BulkDataExportOptions(); options.setResourceTypes(myWantTypes); + when(myRequestDetails.getAttribute(any())).thenReturn(options); AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); assertNull(verdict); } -} + @Test + public void testDenyBulkRequestWithInvalidGroupId() { + RuleBulkExportImpl myRule = new RuleBulkExportImpl("a"); + myRule.setAppliesToGroupExportOnGroup("invalid group"); + myRule.setMode(PolicyEnum.ALLOW); + + BulkDataExportOptions options = new BulkDataExportOptions(); + options.setExportStyle(BulkDataExportOptions.ExportStyle.GROUP); + options.setGroupId(new IdDt("Group/123")); + + when(myRequestDetails.getAttribute(any())).thenReturn(options); + + AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + assertEquals(PolicyEnum.DENY, verdict.getDecision()); + } + + @Test + public void testAllowBulkRequestWithValidGroupId() { + RuleBulkExportImpl myRule = new RuleBulkExportImpl("a"); + myRule.setAppliesToGroupExportOnGroup("Group/1"); + myRule.setMode(PolicyEnum.ALLOW); + + BulkDataExportOptions options = new BulkDataExportOptions(); + options.setExportStyle(BulkDataExportOptions.ExportStyle.GROUP); + options.setGroupId(new IdDt("Group/1")); + + when(myRequestDetails.getAttribute(any())).thenReturn(options); + + AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); + assertEquals(PolicyEnum.ALLOW, verdict.getDecision()); + } + +} \ No newline at end of file diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java index 7d8ae7429bf..fe7529d87af 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/bulk/export/provider/BulkDataExportProvider.java @@ -128,7 +128,7 @@ public class BulkDataExportProvider { validatePreferAsyncHeader(theRequestDetails); BulkDataExportOptions bulkDataExportOptions = buildGroupBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter, theIdParam, theMdm); validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes()); - IBulkDataExportSvc.JobInfo outcome = myBulkDataExportSvc.submitJob(bulkDataExportOptions, shouldUseCache(theRequestDetails), null); + IBulkDataExportSvc.JobInfo outcome = myBulkDataExportSvc.submitJob(bulkDataExportOptions, shouldUseCache(theRequestDetails), theRequestDetails); writePollingLocationToResponseHeaders(theRequestDetails, outcome); } @@ -158,7 +158,7 @@ public class BulkDataExportProvider { validatePreferAsyncHeader(theRequestDetails); BulkDataExportOptions bulkDataExportOptions = buildPatientBulkExportOptions(theOutputFormat, theType, theSince, theTypeFilter); validateResourceTypesAllContainPatientSearchParams(bulkDataExportOptions.getResourceTypes()); - IBulkDataExportSvc.JobInfo outcome = myBulkDataExportSvc.submitJob(bulkDataExportOptions, shouldUseCache(theRequestDetails), null); + IBulkDataExportSvc.JobInfo outcome = myBulkDataExportSvc.submitJob(bulkDataExportOptions, shouldUseCache(theRequestDetails), theRequestDetails); writePollingLocationToResponseHeaders(theRequestDetails, outcome); }