diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java index b78f2f87d9c..fa2fcca3d5f 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptor.java @@ -159,7 +159,7 @@ public class AuthorizationInterceptor implements IRuleApplier { case DELETE: // Delete is a special case - return OperationExamineDirection.NONE; + return OperationExamineDirection.IN; case CREATE: case UPDATE: diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java index 570bc78fe93..72c86ceae71 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplConditional.java @@ -52,21 +52,29 @@ public class RuleImplConditional extends BaseRule implements IAuthRule { } if (theOperation == myOperationType) { + if (theRequestDetails.getConditionalUrl(myOperationType) == null) { + return null; + } + switch (myAppliesTo) { case ALL_RESOURCES: case INSTANCES: break; case TYPES: - if (theInputResource == null || !myAppliesToTypes.contains(theInputResource.getClass())) { - return null; + if (myOperationType == RestOperationTypeEnum.DELETE) { + String resourceName = theRequestDetails.getResourceName(); + Class resourceType = theRequestDetails.getFhirContext().getResourceDefinition(resourceName).getImplementingClass(); + if (!myAppliesToTypes.contains(resourceType)) { + return null; + } + } else { + if (theInputResource == null || !myAppliesToTypes.contains(theInputResource.getClass())) { + return null; + } } break; } - if (theRequestDetails.getConditionalUrl(myOperationType) == null) { - return null; - } - if (getTenantApplicabilityChecker() != null) { if (!getTenantApplicabilityChecker().applies(theRequestDetails)) { return null; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java index c29c557d0f4..94316132aa6 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.interceptor.auth.AuthorizationInterceptor.Verdict; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.BundleUtil.BundleEntryParts; +import ca.uhn.fhir.util.CollectionUtil; import ca.uhn.fhir.util.FhirTerser; import com.google.common.annotations.VisibleForTesting; import org.apache.commons.lang3.StringUtils; @@ -215,6 +216,7 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ { return newVerdict(); } appliesToResource = theInputResource; + appliesToResourceId = Collections.singleton(theInputResourceId); } else { return null; } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java index 18f4de0904b..6812a623858 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java @@ -3033,22 +3033,34 @@ public class AuthorizationInterceptorR4Test { @Override public List buildRuleList(RequestDetails theRequestDetails) { return new RuleBuilder() - .allow("Rule 2").deleteConditional().resourcesOfType(Patient.class) + .allow("Rule 2").deleteConditional().resourcesOfType(Patient.class).andThen() + .allow().delete().instance(new IdType("Patient/2")).andThen() .build(); } }); HttpDelete httpDelete; HttpResponse status; + String response; - ourReturn = Collections.singletonList(createPatient(1)); +// // Wrong resource +// ourReturn = Collections.singletonList(createPatient(1)); +// ourHitMethod = false; +// httpDelete = new HttpDelete("http://localhost:" + ourPort + "/Patient?foo=bar"); +// status = ourClient.execute(httpDelete); +// response = extractResponseAndClose(status); +// ourLog.info(response); +// assertEquals(403, status.getStatusLine().getStatusCode()); +// assertTrue(ourHitMethod); + // Right resource + ourReturn = Collections.singletonList(createPatient(2)); ourHitMethod = false; httpDelete = new HttpDelete("http://localhost:" + ourPort + "/Patient?foo=bar"); status = ourClient.execute(httpDelete); - String response = extractResponseAndClose(status); + response = extractResponseAndClose(status); ourLog.info(response); - assertEquals(403, status.getStatusLine().getStatusCode()); + assertEquals(204, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); } @@ -3074,7 +3086,7 @@ public class AuthorizationInterceptorR4Test { status = ourClient.execute(httpDelete); extractResponseAndClose(status); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); ourHitMethod = false; ourReturn = Collections.singletonList(createPatient(1)); @@ -3082,7 +3094,7 @@ public class AuthorizationInterceptorR4Test { status = ourClient.execute(httpDelete); extractResponseAndClose(status); assertEquals(403, status.getStatusLine().getStatusCode()); - assertTrue(ourHitMethod); + assertFalse(ourHitMethod); } @Test diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 04db1e2028c..d2892bafac0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -210,6 +210,11 @@ RuleBuilder, in order to ensure that cascading deletes are only available to users with sufficient permission. + + AuthorizationInterceptor will now try to block delete operations sooner + in the processing lifecycle if there is no chance they will be permitted + later (i.e. because the type is not authorized at all) +