Credit and tests for #762

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorResourceProviderR4Test.java

@@ -376,6 +376,93 @@ public class AuthorizationInterceptorResourceProviderR4Test extends BaseResource
 
 	}
 
+	/**
+	 * See #762
+	 */
+	@Test
+	public void testInstanceRuleOkForResourceWithNoId2() {
+
+		ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
+			@Override
+			public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
+				return new RuleBuilder()
+					.allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen()
+					.allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen()
+					.allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen()
+					.allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen()
+					.denyAll("deny all")
+					.build();
+			}
+		});
+
+
+
+		// Create a bundle that will be used as a transaction
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.TRANSACTION);
+
+
+
+		String encounterId = "123-123";
+		String encounterSystem = "http://our.internal.code.system/encounter";
+		Encounter encounter = new Encounter();
+
+		encounter.addIdentifier(new Identifier().setValue(encounterId)
+			.setSystem(encounterSystem));
+
+		encounter.setStatus(Encounter.EncounterStatus.FINISHED);
+
+		Patient p = new Patient()
+			.addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient"));
+		p.setId(IdDt.newRandomUuid());
+
+		// add patient to bundle so its created
+		bundle.addEntry()
+			.setFullUrl(p.getId())
+			.setResource(p)
+			.getRequest()
+			.setUrl("Patient")
+			.setMethod(Bundle.HTTPVerb.POST);
+
+		Reference patientRef = new Reference(p.getId());
+
+		encounter.setSubject(patientRef);
+		Condition condition = new Condition()
+			.setCode(new CodeableConcept().addCoding(
+				new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN")))
+			.setSubject(patientRef);
+
+		condition.setId(IdDt.newRandomUuid());
+
+		// add condition to bundle so its created
+		bundle.addEntry()
+			.setFullUrl(condition.getId())
+			.setResource(condition)
+			.getRequest()
+			.setUrl("Condition")
+			.setMethod(Bundle.HTTPVerb.POST);
+
+		Encounter.DiagnosisComponent dc = new Encounter.DiagnosisComponent();
+
+		dc.setCondition(new Reference(condition.getId()));
+		encounter.addDiagnosis(dc);
+		CodeableConcept reason = new CodeableConcept();
+		reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW");
+		encounter.addReason(reason);
+
+		// add encounter to bundle so its created
+		bundle.addEntry()
+			.setResource(encounter)
+			.getRequest()
+			.setUrl("Encounter")
+			.setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId)
+			.setMethod(Bundle.HTTPVerb.POST);
+
+
+		Bundle resp = myClient.transaction().withBundle(bundle).execute();
+		assertEquals(3, resp.getEntry().size());
+
+	}
 
 	private void unregisterInterceptors() {
 		for (IServerInterceptor next : new ArrayList<IServerInterceptor>(ourRestServer.getInterceptors())) {

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/IServerInterceptor.java

@@ -41,6 +41,8 @@ import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
 import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 /**
  * Provides methods to intercept requests and responses. Note that implementations of this interface may wish to use
  * {@link InterceptorAdapter} in order to not need to implement every method.
@@ -329,9 +331,9 @@ public interface IServerInterceptor {
 	public static class ActionRequestDetails {
 		private final FhirContext myContext;
 		private final IIdType myId;
+		private final String myResourceType;
 		private RequestDetails myRequestDetails;
 		private IBaseResource myResource;
-		private final String myResourceType;
 
 		public ActionRequestDetails(RequestDetails theRequestDetails) {
 			myId = theRequestDetails.getId();
@@ -346,7 +348,11 @@ public interface IServerInterceptor {
 		}
 
 		public ActionRequestDetails(RequestDetails theRequestDetails, FhirContext theContext, String theResourceType, IIdType theId) {
-			myId = theId;
+			if (theId != null && isBlank(theId.getValue())) {
+				myId = null;
+			} else {
+				myId = theId;
+			}
 			myResourceType = theResourceType;
 			myContext = theContext;
 			myRequestDetails = theRequestDetails;
@@ -409,6 +415,13 @@ public interface IServerInterceptor {
 			return myResource;
 		}
 
+		/**
+		 * This method should not be called by client code
+		 */
+		public void setResource(IBaseResource theObject) {
+			myResource = theObject;
+		}
+
 		/**
 		 * Returns the resource type this request pertains to, or <code>null</code> if this request is not type specific
 		 * (e.g. server-history)
@@ -450,13 +463,6 @@ public interface IServerInterceptor {
 			}
 		}
 
-		/**
-		 * This method should not be called by client code
-		 */
-		public void setResource(IBaseResource theObject) {
-			myResource = theObject;
-		}
-
 	}
 
 }

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java

@@ -2,6 +2,7 @@ package ca.uhn.fhir.rest.server.interceptor;
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.api.AddProfileTagEnum;
+import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.rest.annotation.*;
 import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.*;
@@ -263,6 +264,108 @@ public class AuthorizationInterceptorR4Test {
 		assertEquals(403, status.getStatusLine().getStatusCode());
 	}
 
+
+	/**
+	 * See #762
+	 */
+	@Test
+	public void testInstanceRuleOkForResourceWithNoId2() throws IOException {
+
+		ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
+			@Override
+			public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
+				return new RuleBuilder()
+					.allow("transactions").transaction().withAnyOperation().andApplyNormalRules().andThen()
+					.allow("write patient").write().resourcesOfType(Patient.class).withAnyId().andThen()
+					.allow("write encounter").write().resourcesOfType(Encounter.class).withAnyId().andThen()
+					.allow("write condition").write().resourcesOfType(Condition.class).withAnyId().andThen()
+					.denyAll("deny all")
+					.build();
+			}
+		});
+
+
+
+		// Create a bundle that will be used as a transaction
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.TRANSACTION);
+
+
+
+		String encounterId = "123-123";
+		String encounterSystem = "http://our.internal.code.system/encounter";
+		Encounter encounter = new Encounter();
+
+		encounter.addIdentifier(new Identifier().setValue(encounterId)
+			.setSystem(encounterSystem));
+
+		encounter.setStatus(Encounter.EncounterStatus.FINISHED);
+
+		Patient p = new Patient()
+			.addIdentifier(new Identifier().setValue("321-321").setSystem("http://our.internal.code.system/patient"));
+		p.setId(IdDt.newRandomUuid());
+
+		// add patient to bundle so its created
+		bundle.addEntry()
+			.setFullUrl(p.getId())
+			.setResource(p)
+			.getRequest()
+			.setUrl("Patient")
+			.setMethod(Bundle.HTTPVerb.POST);
+
+		Reference patientRef = new Reference(p.getId());
+
+		encounter.setSubject(patientRef);
+		Condition condition = new Condition()
+			.setCode(new CodeableConcept().addCoding(
+				new Coding("http://hl7.org/fhir/icd-10", "S53.40", "FOREARM SPRAIN / STRAIN")))
+			.setSubject(patientRef);
+
+		condition.setId(IdDt.newRandomUuid());
+
+		// add condition to bundle so its created
+		bundle.addEntry()
+			.setFullUrl(condition.getId())
+			.setResource(condition)
+			.getRequest()
+			.setUrl("Condition")
+			.setMethod(Bundle.HTTPVerb.POST);
+
+		Encounter.DiagnosisComponent dc = new Encounter.DiagnosisComponent();
+
+		dc.setCondition(new Reference(condition.getId()));
+		encounter.addDiagnosis(dc);
+		CodeableConcept reason = new CodeableConcept();
+		reason.setText("SLIPPED ON FLOOR,PAIN L) ELBOW");
+		encounter.addReason(reason);
+
+		// add encounter to bundle so its created
+		bundle.addEntry()
+			.setResource(encounter)
+			.getRequest()
+			.setUrl("Encounter")
+			.setIfNoneExist("identifier=" + encounterSystem + "|" + encounterId)
+			.setMethod(Bundle.HTTPVerb.POST);
+
+		Bundle output = new Bundle();
+		output.setType(Bundle.BundleType.TRANSACTIONRESPONSE);
+		output.addEntry()
+			.setResource(new Patient().setActive(true)) // don't give this an ID
+			.getResponse().setLocation("/Patient/1");
+
+
+		ourReturn = Collections.singletonList((Resource) output);
+		HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/");
+		httpPost.setEntity(createFhirResourceEntity(bundle));
+		CloseableHttpResponse status = ourClient.execute(httpPost);
+		String resp = extractResponseAndClose(status);
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
+		ourLog.info(resp);
+
+	}
+
+
 	@Test
 	public void testBatchWhenTransactionReadDenied() throws Exception {
 		ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {

src/changes/changes.xml

@@ -240,6 +240,12 @@
 				The learn more links on the website home page had broken links. Thanks to
 				James Daily for the pull request to fix this!
 			</action>
+			<action type="add" issue="762">
+				Prevent a crash in AuthorizationInterceptor when processing transactions
+				if the interceptor has rules declared which allow resources to be read/written
+				by "any ID of a given type". Thanks to GitHub user @dconlan for the pull
+				request!
+			</action>
 		</release>
 		<release version="3.0.0" date="2017-09-27">
 			<action type="add">