Merge branch 'master' of github.com:jamesagnew/hapi-fhir

hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties

@@ -11,6 +11,7 @@ ca.uhn.fhir.context.RuntimeResourceDefinition.typeWrongVersion=This context is f
 ca.uhn.fhir.rest.client.impl.BaseClient.ioExceptionDuringOperation=Encountered IOException when performing {0} to URL {1} - {2}
 ca.uhn.fhir.rest.client.impl.BaseClient.failedToParseResponse=Failed to parse response from server when performing {0} to URL {1} - {2}
 
+
 ca.uhn.fhir.rest.client.impl.GenericClient.cantDetermineRequestType=Unable to determing encoding of request (body does not appear to be valid XML or JSON)
 ca.uhn.fhir.rest.client.impl.GenericClient.noPagingLinkFoundInBundle=Can not perform paging operation because no link was found in Bundle with relation "{0}"
 ca.uhn.fhir.rest.client.impl.GenericClient.noVersionIdForVread=No version specified in URL for 'vread' operation: {0}
@@ -19,6 +20,8 @@ ca.uhn.fhir.rest.client.impl.GenericClient.cannotDetermineResourceTypeFromUri=Un
 ca.uhn.fhir.rest.client.impl.RestfulClientFactory.failedToRetrieveConformance=Failed to retrieve the server metadata statement during client initialization. URL used was {0} 
 ca.uhn.fhir.rest.client.impl.RestfulClientFactory.wrongVersionInConformance=The server at base URL "{0}" returned a conformance statement indicating that it supports FHIR version "{1}" which corresponds to {2}, but this client is configured to use {3} (via the FhirContext). 
 
+ca.uhn.fhir.rest.server.interceptor.auth.RuleImplOp.invalidRequestBundleTypeForTransaction=Invalid request Bundle.type value for transaction: {0}
+
 ca.uhn.fhir.rest.server.method.BaseOutcomeReturningMethodBindingWithResourceParam.incorrectIdForUpdate=Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of "{0}" does not match URL ID of "{1}"
 ca.uhn.fhir.rest.server.method.BaseOutcomeReturningMethodBindingWithResourceParam.noIdInBodyForUpdate=Can not update resource, resource body must contain an ID element for update (PUT) operation
 ca.uhn.fhir.rest.server.method.BaseOutcomeReturningMethodBindingWithResourceParam.noIdInUrlForUpdate=Can not update resource, request URL must contain an ID element for update (PUT) operation (it must be of the form [base]/[resource type]/[id])

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

@@ -407,7 +407,13 @@ public class AuthorizationInterceptor extends ServerOperationInterceptorAdapter
 		@Override
 		public String toString() {
 			ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE);
-			b.append("rule", myDecidingRule.getName());
+			String ruleName;
+			if (myDecidingRule != null) {
+				ruleName = myDecidingRule.getName();
+			} else {
+				ruleName = "(none)";
+			}
+			b.append("rule", ruleName);
 			b.append("decision", myDecision.name());
 			return b.build();
 		}

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

@@ -7,6 +7,8 @@ import ca.uhn.fhir.rest.api.RequestTypeEnum;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
+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;
@@ -24,6 +26,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static org.apache.commons.lang3.StringUtils.defaultString;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 /*
@@ -466,17 +469,19 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
 		}
 
 		IBaseBundle request = (IBaseBundle) theInputResource;
-		String bundleType = BundleUtil.getBundleType(theContext, request);
+		String bundleType = defaultString(BundleUtil.getBundleType(theContext, request));
 
 		//noinspection EnumSwitchStatementWhichMissesCases
-		switch (theOp) {
+		if (theOp == RuleOpEnum.TRANSACTION) {
-			case TRANSACTION:
+			if ("transaction".equals(bundleType) || "batch".equals(bundleType)) {
-				return "transaction".equals(bundleType)
+				return true;
-					|| "batch".equals(bundleType);
+			} else {
-			default:
+				String msg = theContext.getLocalizer().getMessage(RuleImplOp.class, "invalidRequestBundleTypeForTransaction", '"' + bundleType + '"');
-				return false;
+				throw new UnprocessableEntityException(msg);
 			}
 		}
+		return false;
+	}
 
 	public void setAppliesTo(AppliesTypeEnum theAppliesTo) {
 		myAppliesTo = theAppliesTo;

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorDstu3Test.java

@@ -46,6 +46,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.hamcrest.Matchers.containsString;
@@ -645,6 +646,13 @@ public class AuthorizationInterceptorDstu3Test {
 					.denyAll("Default Rule")
 					.build();
 			}
+
+			@Override
+			protected void handleDeny(Verdict decision) {
+				// Make sure the toString() method on Verdict never fails
+				ourLog.info("Denying with decision: {}", decision);
+				super.handleDeny(decision);
+			}
 		});
 
 		HttpGet httpGet;
@@ -688,6 +696,65 @@ public class AuthorizationInterceptorDstu3Test {
 		assertFalse(ourHitMethod);
 	}
 
+	@Test
+	public void testDenyAllByDefault() throws Exception {
+		ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
+			@Override
+			public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
+				return new RuleBuilder()
+					.allow().read().resourcesOfType(Patient.class).withAnyId().andThen()
+					.build();
+			}
+
+			@Override
+			protected void handleDeny(Verdict decision) {
+				// Make sure the toString() method on Verdict never fails
+				ourLog.info("Denying with decision: {}", decision);
+				super.handleDeny(decision);
+			}
+		});
+
+		HttpGet httpGet;
+		HttpResponse status;
+		String response;
+
+		ourHitMethod = false;
+		ourReturn = Collections.singletonList(createPatient(2));
+		httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1");
+		status = ourClient.execute(httpGet);
+		extractResponseAndClose(status);
+		assertEquals(200, status.getStatusLine().getStatusCode());
+		assertTrue(ourHitMethod);
+
+		ourHitMethod = false;
+		ourReturn = Collections.singletonList(createObservation(10, "Patient/2"));
+		httpGet = new HttpGet("http://localhost:" + ourPort + "/Observation/10");
+		status = ourClient.execute(httpGet);
+		response = extractResponseAndClose(status);
+		ourLog.info(response);
+		assertThat(response, containsString("Access denied by default policy"));
+		assertEquals(403, status.getStatusLine().getStatusCode());
+		assertFalse(ourHitMethod);
+
+		ourHitMethod = false;
+		httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$validate");
+		status = ourClient.execute(httpGet);
+		response = extractResponseAndClose(status);
+		ourLog.info(response);
+		assertThat(response, containsString("Access denied by default policy"));
+		assertEquals(403, status.getStatusLine().getStatusCode());
+		assertFalse(ourHitMethod);
+
+		ourHitMethod = false;
+		httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$opName");
+		status = ourClient.execute(httpGet);
+		response = extractResponseAndClose(status);
+		ourLog.info(response);
+		assertThat(response, containsString("Access denied by default policy"));
+		assertEquals(403, status.getStatusLine().getStatusCode());
+		assertFalse(ourHitMethod);
+	}
+
 	/**
 	 * #528
 	 */
@@ -2459,6 +2526,85 @@ public class AuthorizationInterceptorDstu3Test {
 
 	}
 
+	@Test
+	public void testTransactionWithSearch() 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("read patient").read().resourcesOfType(Patient.class).withAnyId().andThen()
+					.denyAll("deny all")
+					.build();
+			}
+		});
+
+		// Request is a transaction with 1 search
+		Bundle requestBundle = new Bundle();
+		requestBundle.setType(Bundle.BundleType.TRANSACTION);
+		String patientId = "10000003857";
+		Bundle.BundleEntryComponent bundleEntryComponent = requestBundle.addEntry();
+		Bundle.BundleEntryRequestComponent bundleEntryRequestComponent = new Bundle.BundleEntryRequestComponent();
+		bundleEntryRequestComponent.setMethod(Bundle.HTTPVerb.GET);
+		bundleEntryRequestComponent.setUrl(ResourceType.Patient + "?identifier=" + patientId);
+		bundleEntryComponent.setRequest(bundleEntryRequestComponent);
+
+		/*
+		 * Response is a transaction response containing the search results
+		 */
+		Bundle searchResponseBundle = new Bundle();
+		Patient patent = new Patient();
+		patent.setActive(true);
+		patent.setId("Patient/123");
+		searchResponseBundle.addEntry().setResource(patent);
+
+		Bundle responseBundle = new Bundle();
+		responseBundle
+			.addEntry()
+			.setResource(searchResponseBundle);
+		ourReturn = Collections.singletonList(responseBundle);
+
+		HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/");
+		httpPost.setEntity(createFhirResourceEntity(requestBundle));
+		CloseableHttpResponse status = ourClient.execute(httpPost);
+		String resp = extractResponseAndClose(status);
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
+	}
+
+	@Test
+	public void testTransactionWithNoBundleType() 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("read patient").read().resourcesOfType(Patient.class).withAnyId().andThen()
+					.denyAll("deny all")
+					.build();
+			}
+		});
+
+		// Request is a transaction with 1 search
+		Bundle requestBundle = new Bundle();
+		String patientId = "10000003857";
+		Bundle.BundleEntryComponent bundleEntryComponent = requestBundle.addEntry();
+		Bundle.BundleEntryRequestComponent bundleEntryRequestComponent = new Bundle.BundleEntryRequestComponent();
+		bundleEntryRequestComponent.setMethod(Bundle.HTTPVerb.GET);
+		bundleEntryRequestComponent.setUrl(ResourceType.Patient + "?identifier=" + patientId);
+		bundleEntryComponent.setRequest(bundleEntryRequestComponent);
+
+		HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/");
+		httpPost.setEntity(createFhirResourceEntity(requestBundle));
+		CloseableHttpResponse status = ourClient.execute(httpPost);
+		String resp = extractResponseAndClose(status);
+		assertEquals(422, status.getStatusLine().getStatusCode());
+		assertThat(resp, containsString("Invalid request Bundle.type value for transaction: \\\"\\\""));
+
+	}
+
 	/**
 	 * See #762
 	 */

src/changes/changes.xml

@@ -243,6 +243,10 @@
 				new setting on the DaoConfig. This can be used to force a total to
 				always be calculated for searches, including large ones.
 			</action>
+			<action type="add">
+				AuthorizationInterceptor now rejects transactions with an invalid or unset request
+				using an HTTP 422 response	Bundle type instead of silently refusing to authorize them.
+			</action>
 		</release>
 		<release version="3.6.0" date="2018-11-12" description="Food">
 			<action type="add">