Improve processing of deletes in AuthorizationInterceptor

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:

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())) {
+					if (myOperationType == RestOperationTypeEnum.DELETE) {
-						return null;
+						String resourceName = theRequestDetails.getResourceName();
+						Class<? extends IBaseResource> 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;

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;
 				}

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<IAuthRule> 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

src/changes/changes.xml

@@ -210,6 +210,11 @@
 				RuleBuilder, in order to ensure that cascading deletes are only available
 				to users with sufficient permission.
 			</action>
+			<action type="add">
+				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)
+			</action>
 		</release>
 		<release version="3.8.0" date="2019-05-30" description="Hippo">
 			<action type="fix">