Searching for Bundles with read all Bundles permission returns 403

hapi-fhir-base/src/main/java/ca/uhn/fhir/model/valueset/BundleTypeEnum.java

@@ -91,7 +91,7 @@ public String getSystem() {
 	/**
 	 * Returns the enumerated value associated with this code
 	 */
-	public BundleTypeEnum forCode(String theCode) {
+	public static BundleTypeEnum forCode(String theCode) {
 		BundleTypeEnum retVal = CODE_TO_ENUM.get(theCode);
 		return retVal;
 	}

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java

@@ -26,6 +26,7 @@
 import ca.uhn.fhir.context.RuntimeResourceDefinition;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.model.primitive.IdDt;
+import ca.uhn.fhir.model.valueset.BundleTypeEnum;
 import ca.uhn.fhir.rest.api.PatchTypeEnum;
 import ca.uhn.fhir.rest.api.RequestTypeEnum;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
@@ -235,6 +236,14 @@ public static String getBundleType(FhirContext theContext, IBaseBundle theBundle
 		return null;
 	}
 
+	public static BundleTypeEnum getBundleTypeEnum(FhirContext theContext, IBaseBundle theBundle) {
+		String bundleTypeCode = BundleUtil.getBundleType(theContext, theBundle);
+		if (isBlank(bundleTypeCode)) {
+			return null;
+		}
+		return BundleTypeEnum.forCode(bundleTypeCode);
+	}
+
 	public static void setBundleType(FhirContext theContext, IBaseBundle theBundle, String theType) {
 		RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
 		BaseRuntimeChildDefinition entryChild = def.getChildByName("type");

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5644-searching-for-bundles-with-read-all-bundles-permissions-returns-403.yaml

@@ -0,0 +1,8 @@
+---
+type: fix
+issue: 5644
+title: "Previously, searching for `Bundle` resources with read all `Bundle` resources permissions, returned an 
+HTTP 403 Forbidden error. This was because the `AuthorizationInterceptor` applied permissions to the resources inside 
+the `Bundle`, instead of the `Bundle` itself. This has been fixed and permissions are no longer applied to the resources 
+inside a `Bundle` of type `document`, `message`, or `collection` for `Bundle` requests."
+

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

@@ -1,7 +1,6 @@
 package ca.uhn.fhir.jpa.provider.r4;
 
 import ca.uhn.fhir.i18n.Msg;
-import ca.uhn.fhir.batch2.jobs.export.BulkDataExportProvider;
 import ca.uhn.fhir.jpa.delete.ThreadSafeResourceDeleterSvc;
 import ca.uhn.fhir.jpa.interceptor.CascadingDeleteInterceptor;
 import ca.uhn.fhir.jpa.model.util.JpaConstants;
@@ -14,6 +13,7 @@
 import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor;
 import ca.uhn.fhir.rest.server.exceptions.AuthenticationException;
 import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
@@ -37,23 +37,28 @@
 import org.hl7.fhir.r4.model.Bundle;
 import org.hl7.fhir.r4.model.CodeableConcept;
 import org.hl7.fhir.r4.model.Coding;
+import org.hl7.fhir.r4.model.Composition;
 import org.hl7.fhir.r4.model.Condition;
 import org.hl7.fhir.r4.model.Encounter;
 import org.hl7.fhir.r4.model.ExplanationOfBenefit;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Identifier;
+import org.hl7.fhir.r4.model.MessageHeader;
 import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Observation.ObservationStatus;
 import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Parameters;
 import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Practitioner;
 import org.hl7.fhir.r4.model.Reference;
+import org.hl7.fhir.r4.model.Resource;
 import org.hl7.fhir.r4.model.StringType;
 import org.hl7.fhir.r4.model.ValueSet;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -68,7 +73,9 @@
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Test {
@@ -79,6 +86,8 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes
 	private SearchParamMatcher mySearchParamMatcher;
 	@Autowired
 	private ThreadSafeResourceDeleterSvc myThreadSafeResourceDeleterSvc;
+	private AuthorizationInterceptor myReadAllBundleInterceptor;
+	private AuthorizationInterceptor myReadAllPatientInterceptor;
 
 	@BeforeEach
 	@Override
@@ -87,7 +96,8 @@ public void before() throws Exception {
 		myStorageSettings.setAllowMultipleDelete(true);
 		myStorageSettings.setExpungeEnabled(true);
 		myStorageSettings.setDeleteExpungeEnabled(true);
-		myServer.getRestfulServer().registerInterceptor(new BulkDataExportProvider());
+		myReadAllBundleInterceptor = new ReadAllAuthorizationInterceptor("Bundle");
+		myReadAllPatientInterceptor = new ReadAllAuthorizationInterceptor("Patient");
 	}
 
 	@Override
@@ -1506,4 +1516,250 @@ public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
 		}
 
 	}
+
+	@Test
+	public void testSearchBundles_withPermissionToSearchAllBundles_doesNotReturn403ForbiddenForDocumentBundles(){
+		myServer.getRestfulServer().registerInterceptor(myReadAllBundleInterceptor);
+
+		Bundle bundle1 = createDocumentBundle(createPatient("John", "Smith"));
+		Bundle bundle2 = createDocumentBundle(createPatient("Jane", "Doe"));
+		assertSearchContainsResources("/Bundle", bundle1, bundle2);
+	}
+
+	@Test
+	public void testSearchBundles_withPermissionToSearchAllBundles_doesNotReturn403ForbiddenForCollectionBundles(){
+		myServer.getRestfulServer().registerInterceptor(myReadAllBundleInterceptor);
+
+		Bundle bundle1 = createCollectionBundle(createPatient("John", "Smith"));
+		Bundle bundle2 = createCollectionBundle(createPatient("Jane", "Doe"));
+		assertSearchContainsResources("/Bundle", bundle1, bundle2);
+	}
+
+	@Test
+	public void testSearchBundles_withPermissionToSearchAllBundles_doesNotReturn403ForbiddenForMessageBundles(){
+		myServer.getRestfulServer().registerInterceptor(myReadAllBundleInterceptor);
+
+		Bundle bundle1 = createMessageHeaderBundle(createPatient("John", "Smith"));
+		Bundle bundle2 = createMessageHeaderBundle(createPatient("Jane", "Doe"));
+		assertSearchContainsResources("/Bundle", bundle1, bundle2);
+	}
+
+	@Test
+	public void testSearchBundles_withPermissionToViewOneBundle_onlyAllowsViewingOneBundle(){
+		Bundle bundle1 = createMessageHeaderBundle(createPatient("John", "Smith"));
+		Bundle bundle2 = createMessageHeaderBundle(createPatient("Jane", "Doe"));
+
+		myServer.getRestfulServer().getInterceptorService().registerInterceptor(
+			new ReadInCompartmentAuthorizationInterceptor("Bundle", bundle1.getIdElement())
+		);
+
+		assertSearchContainsResources("/Bundle?_id=" + bundle1.getIdPart(), bundle1);
+		assertSearchFailsWith403Forbidden("/Bundle?_id=" + bundle2.getIdPart());
+		assertSearchFailsWith403Forbidden("/Bundle");
+	}
+
+	@Test
+	public void testSearchPatients_withPermissionToSearchAllBundles_returns403Forbidden(){
+		myServer.getRestfulServer().registerInterceptor(myReadAllBundleInterceptor);
+
+		createPatient("John", "Smith");
+		createPatient("Jane", "Doe");
+		assertSearchFailsWith403Forbidden("/Patient");
+	}
+
+	@Test
+	public void testSearchPatients_withPermissionToSearchAllPatients_returnsAllPatients(){
+		myServer.getRestfulServer().registerInterceptor(myReadAllPatientInterceptor);
+
+		Patient patient1 = createPatient("John", "Smith");
+		Patient patient2 = createPatient("Jane", "Doe");
+		assertSearchContainsResources("/Patient", patient1, patient2);
+	}
+
+	@Test
+	public void testSearchPatients_withPermissionToViewOnePatient_onlyAllowsViewingOnePatient(){
+		Patient patient1 = createPatient("John", "Smith");
+		Patient patient2 = createPatient("Jane", "Doe");
+
+		myServer.getRestfulServer().getInterceptorService().registerInterceptor(
+			new ReadInCompartmentAuthorizationInterceptor("Patient", patient1.getIdElement())
+		);
+
+		assertSearchContainsResources("/Patient?_id=" + patient1.getIdPart(), patient1);
+		assertSearchFailsWith403Forbidden("/Patient?_id=" + patient2.getIdPart());
+		assertSearchFailsWith403Forbidden("/Patient");
+	}
+
+	@Test
+	public void testToListOfResourcesAndExcludeContainer_withSearchSetContainingDocumentBundles_onlyRecursesOneLevelDeep() {
+		Bundle bundle1 = createDocumentBundle(createPatient("John", "Smith"));
+		Bundle bundle2 = createDocumentBundle(createPatient("John", "Smith"));
+		Bundle searchSet = createSearchSet(bundle1, bundle2);
+
+		RequestDetails requestDetails = new SystemRequestDetails();
+		requestDetails.setResourceName("Bundle");
+
+		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(requestDetails, searchSet, myFhirContext);
+		assertEquals(2, resources.size());
+		assertTrue(resources.contains(bundle1));
+		assertTrue(resources.contains(bundle2));
+	}
+
+	@Test
+	public void testToListOfResourcesAndExcludeContainer_withSearchSetContainingPatients_returnsPatients() {
+		Patient patient1 = createPatient("John", "Smith");
+		Patient patient2 = createPatient("Jane", "Doe");
+		Bundle searchSet = createSearchSet(patient1, patient2);
+
+		RequestDetails requestDetails = new SystemRequestDetails();
+		requestDetails.setResourceName("Patient");
+
+		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainer(requestDetails, searchSet, myFhirContext);
+		assertEquals(2, resources.size());
+		assertTrue(resources.contains(patient1));
+		assertTrue(resources.contains(patient2));
+	}
+
+	@ParameterizedTest
+	@EnumSource(value = Bundle.BundleType.class, names = {"DOCUMENT", "COLLECTION", "MESSAGE"})
+	public void testShouldExamineBundleResources_withBundleRequestAndStandAloneBundleType_returnsFalse(Bundle.BundleType theBundleType){
+		RequestDetails requestDetails = new SystemRequestDetails();
+		requestDetails.setResourceName("Bundle");
+		Bundle bundle = new Bundle();
+		bundle.setType(theBundleType);
+
+		assertFalse(AuthorizationInterceptor.shouldExamineBundleChildResources(requestDetails, myFhirContext, bundle));
+	}
+
+	@ParameterizedTest
+	@EnumSource(value = Bundle.BundleType.class, names = {"DOCUMENT", "COLLECTION", "MESSAGE"}, mode= EnumSource.Mode.EXCLUDE)
+	public void testShouldExamineBundleResources_withBundleRequestAndNonStandAloneBundleType_returnsTrue(Bundle.BundleType theBundleType){
+		RequestDetails requestDetails = new SystemRequestDetails();
+		requestDetails.setResourceName("Bundle");
+		Bundle bundle = new Bundle();
+		bundle.setType(theBundleType);
+
+		assertTrue(AuthorizationInterceptor.shouldExamineBundleChildResources(requestDetails, myFhirContext, bundle));
+	}
+
+	@ParameterizedTest
+	@EnumSource(value = Bundle.BundleType.class)
+	public void testShouldExamineBundleResources_withNonBundleRequests_returnsTrue(Bundle.BundleType theBundleType){
+		RequestDetails requestDetails = new SystemRequestDetails();
+		requestDetails.setResourceName("Patient");
+		Bundle bundle = new Bundle();
+		bundle.setType(theBundleType);
+
+		assertTrue(AuthorizationInterceptor.shouldExamineBundleChildResources(requestDetails, myFhirContext, bundle));
+	}
+
+	private Patient createPatient(String theFirstName, String theLastName){
+		Patient patient = new Patient();
+		patient.addName().addGiven(theFirstName).setFamily(theLastName);
+		return (Patient) myPatientDao.create(patient, mySrd).getResource();
+	}
+
+	private Bundle createDocumentBundle(Patient thePatient){
+		Composition composition = new Composition();
+		composition.setType(new CodeableConcept().addCoding(new Coding().setSystem("http://example.org").setCode("some-type")));
+		composition.getSubject().setReference(thePatient.getIdElement().getValue());
+
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.DOCUMENT);
+		bundle.addEntry().setResource(composition);
+		bundle.addEntry().setResource(thePatient);
+		return (Bundle) myBundleDao.create(bundle, mySrd).getResource();
+	}
+
+	private Bundle createCollectionBundle(Patient thePatient) {
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.COLLECTION);
+		bundle.addEntry().setResource(thePatient);
+		return (Bundle) myBundleDao.create(bundle, mySrd).getResource();
+	}
+
+	private Bundle createMessageHeaderBundle(Patient thePatient) {
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.MESSAGE);
+
+		MessageHeader messageHeader = new MessageHeader();
+		Coding event = new Coding().setSystem("http://acme.com").setCode("some-event");
+		messageHeader.setEvent(event);
+		messageHeader.getFocusFirstRep().setReference(thePatient.getIdElement().getValue());
+		bundle.addEntry().setResource(messageHeader);
+		bundle.addEntry().setResource(thePatient);
+
+		return (Bundle) myBundleDao.create(bundle, mySrd).getResource();
+	}
+
+	private void assertSearchContainsResources(String theUrl, Resource... theExpectedResources){
+		List<String> expectedIds = Arrays.stream(theExpectedResources).toList().stream()
+			.map(resource -> resource.getIdPart())
+			.toList();
+
+		Bundle searchResult = myClient
+			.search()
+			.byUrl(theUrl)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		List<String> actualIds = searchResult.getEntry().stream()
+			.map(entry -> entry.getResource().getIdPart())
+			.toList();
+
+		assertEquals(expectedIds.size(), actualIds.size());
+		assertTrue(expectedIds.containsAll(actualIds));
+	}
+
+	private void assertSearchFailsWith403Forbidden(String theUrl){
+		try {
+			myClient.search().byUrl(theUrl).execute();
+			fail();
+		} catch (Exception e){
+			assertTrue(e.getMessage().contains("HTTP 403 Forbidden"));
+		}
+	}
+
+	private Bundle createSearchSet(Resource... theResources){
+		Bundle bundle = new Bundle();
+		bundle.setType(Bundle.BundleType.SEARCHSET);
+		Arrays.stream(theResources).forEach(resource -> bundle.addEntry().setResource(resource));
+		return bundle;
+	}
+
+	static class ReadAllAuthorizationInterceptor extends AuthorizationInterceptor {
+
+		private final String myResourceType;
+
+		public ReadAllAuthorizationInterceptor(String theResourceType){
+			super(PolicyEnum.DENY);
+			myResourceType = theResourceType;
+		}
+
+		@Override
+		public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
+			return new RuleBuilder()
+				.allow().read().resourcesOfType(myResourceType).withAnyId().andThen()
+				.build();
+		}
+	}
+
+	static class ReadInCompartmentAuthorizationInterceptor extends AuthorizationInterceptor {
+
+		private final String myResourceType;
+		private final IIdType myId;
+
+		public ReadInCompartmentAuthorizationInterceptor(String theResourceType, IIdType theId){
+			super(PolicyEnum.DENY);
+			myResourceType = theResourceType;
+			myId = theId;
+		}
+
+		@Override
+		public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
+			return new RuleBuilder()
+				.allow().read().allResources().inCompartment(myResourceType, myId).andThen()
+				.build();
+		}
+	}
 }