From 9021e7e765b3f9d26b3e8357eb59d745b7e95d63 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 8 May 2024 20:03:46 -0700 Subject: [PATCH] Convert a few nulls to aggressive denies --- .../interceptor/auth/RuleBulkExportImpl.java | 7 ++++--- .../auth/RuleBulkExportImplTest.java | 17 +++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) 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 0e3e303050d..eadb0ee4c99 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,10 +80,10 @@ public class RuleBulkExportImpl extends BaseRule { // Do we only authorize some types? If so, make sure requested types are a subset if (isNotEmpty(myResourceTypes)) { if (isEmpty(inboundBulkExportRequestOptions.getResourceTypes())) { - return null; + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } if (!myResourceTypes.containsAll(inboundBulkExportRequestOptions.getResourceTypes())) { - return null; + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } } @@ -136,8 +136,9 @@ public class RuleBulkExportImpl extends BaseRule { Set permittedPatientIds = sanitizeIds(myPatientIds); if (permittedPatientIds.containsAll(requestedPatientIds)) { return allowVerdict; + } else { + return new AuthorizationInterceptor.Verdict(PolicyEnum.DENY, this); } - return null; } } return null; 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 de8d319e28b..1dc8eb5b8e8 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 @@ -49,7 +49,7 @@ public class RuleBulkExportImplTest { when(myRequestDetails.getAttribute(any())).thenReturn(options); AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); - assertAbstain(verdict); + assertDeny(verdict); } @@ -68,7 +68,7 @@ public class RuleBulkExportImplTest { when(myRequestDetails.getAttribute(any())).thenReturn(options); AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); - assertAbstain(verdict); + assertDeny(verdict); } @Test @@ -106,7 +106,7 @@ public class RuleBulkExportImplTest { AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); - assertAbstain(verdict); + assertDeny(verdict); } @Nested class StyleChecks { @@ -316,7 +316,7 @@ public class RuleBulkExportImplTest { AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); //Then: abstain - assertAbstain(verdict); + assertDeny(verdict); } @Test @@ -426,7 +426,7 @@ public class RuleBulkExportImplTest { final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); //Then: We do not have permissions on the requested patient so we abstain - assertAbstain(verdict); + assertDeny(verdict); } @Test @@ -484,7 +484,7 @@ public class RuleBulkExportImplTest { final AuthorizationInterceptor.Verdict verdict = myRule.applyRule(myOperation, myRequestDetails, null, null, null, myRuleApplier, myFlags, myPointcut); //Then: There are unpermitted patients in the request so this is not permitted. - assertAbstain(verdict); + assertDeny(verdict); } // @Test @@ -567,4 +567,9 @@ public class RuleBulkExportImplTest { Assertions.assertNotNull(verdict, "Expect ALLOW, got abstain"); Assertions.assertEquals(PolicyEnum.ALLOW, verdict.getDecision(), "Expect ALLOW"); } + + private static void assertDeny(AuthorizationInterceptor.Verdict verdict) { + Assertions.assertNotNull(verdict, "Expect DENY, got abstain"); + Assertions.assertEquals(PolicyEnum.DENY, verdict.getDecision(), "Expect DENY"); + } }