diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3642-group-bulk-export-failure.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3642-group-bulk-export-failure.yaml new file mode 100644 index 00000000000..fcae08f5138 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3642-group-bulk-export-failure.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 3642 +jira: SMILE-4383 +title: "Previously, the RuleBuilder's rules surrounding Group Bulk Export would return failures too early in the case of multiple permissions. This has been corrected, and the rule will no longer prematurely +return a DENY verdict, instead opting to delegate to future rules." diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java index d1d11e6fc24..3e267713350 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java @@ -105,6 +105,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() .allow().bulkExport().groupExportOnGroup(new IdType("Group/123")).andThen() + .allow().bulkExport().groupExportOnGroup(new IdType("Group/789")).andThen() .build(); } }; @@ -115,7 +116,7 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes */ { BulkDataExportOptions bulkDataExportOptions = new BulkDataExportOptions(); - bulkDataExportOptions.setGroupId(new IdType("Group/123")); + bulkDataExportOptions.setGroupId(new IdType("Group/789")); bulkDataExportOptions.setExportStyle(BulkDataExportOptions.ExportStyle.GROUP); ServletRequestDetails requestDetails = new ServletRequestDetails().setServletRequest(new MockHttpServletRequest()); @@ -123,6 +124,20 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes assertEquals(BulkExportJobStatusEnum.SUBMITTED, jobDetails.getStatus()); } + /* + * Second matching group ID + */ + { + BulkDataExportOptions bulkDataExportOptions = new BulkDataExportOptions(); + bulkDataExportOptions.setGroupId(new IdType("Group/789")); + bulkDataExportOptions.setExportStyle(BulkDataExportOptions.ExportStyle.GROUP); + + ServletRequestDetails requestDetails = new ServletRequestDetails().setServletRequest(new MockHttpServletRequest()); + IBulkDataExportSvc.JobInfo jobDetails = myBulkDataExportSvc.submitJob(bulkDataExportOptions, true, requestDetails); + assertEquals(BulkExportJobStatusEnum.SUBMITTED, jobDetails.getStatus()); + + } + /* * Non matching group ID */ 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 cefe4847885..092b066e951 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,11 +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); } } - return null; } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java index e6cfe63004e..1a349eea3cd 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilderTest.java @@ -4,6 +4,7 @@ import ca.uhn.fhir.model.primitive.IdDt; import com.google.common.collect.Lists; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.List; import static org.hamcrest.Matchers.contains; @@ -49,6 +50,19 @@ public class RuleBuilderTest { )); } + @Test + public void testBulkExportPermitsIfASingleGroupMatches() { + RuleBuilder builder = new RuleBuilder(); + List resourceTypes = new ArrayList<>(); + resourceTypes.add("Patient"); + resourceTypes.add("Organization"); + + builder.allow().bulkExport().groupExportOnGroup("group1").withResourceTypes(resourceTypes); + builder.allow().bulkExport().groupExportOnGroup("group2").withResourceTypes(resourceTypes); + List build = builder.build(); + + } + @Test public void testNullConditional() { IAuthRuleBuilder ruleBuilder = new RuleBuilder().allow().metadata().andThen(); 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 609f2eaa8b7..cf808e834ab 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 @@ -73,7 +73,7 @@ public class RuleBulkExportImplTest { } @Test - public void testDenyBulkRequestWithInvalidGroupId() { + public void testWrongGroupIdDelegatesToNextRule() { RuleBulkExportImpl myRule = new RuleBulkExportImpl("a"); myRule.setAppliesToGroupExportOnGroup("invalid group"); myRule.setMode(PolicyEnum.ALLOW); @@ -85,7 +85,7 @@ public class RuleBulkExportImplTest { when(myRequestDetails.getAttribute(any())).thenReturn(options); AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); - assertEquals(PolicyEnum.DENY, verdict.getDecision()); + assertEquals(null, verdict); } @Test @@ -104,4 +104,4 @@ public class RuleBulkExportImplTest { assertEquals(PolicyEnum.ALLOW, verdict.getDecision()); } -} \ No newline at end of file +}