Fixed an issue where bundles containing an operation outcome from a search get blocked from access. (#6800)

* Merged master into branch

* Fixed an issue where bundles containing an operation outcome from a search get blocked from access.

* addressed comments

* ran 'mvn spotless apply'

* added a javadoc to AuthorizationInterceptor.toListOfResourcesAndExcludeOperationOutcomeBasedOnRestOperationType()

---------

Co-authored-by: Abayomi Adebowale <abayomi.adebowale@smiledigitalhealth.com>

Author: AbayomiE-T

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6799-bundles-containing-an-operation-outcome-resource-from-a-search-get-blocked-from-access.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6799
+title: "Fixed an issue where bundles containing an operation outcome from a search cannot be accessed without explicit
+ read permissions given for the OperationOutcome resource."

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

@@ -91,6 +91,8 @@ public class AuthorizationInterceptor implements IRuleApplier {
 			AuthorizationInterceptor.class.getName() + "_" + myInstanceIndex + "_RULELIST";
 	private PolicyEnum myDefaultPolicy = PolicyEnum.DENY;
 	private Set<AuthorizationFlagsEnum> myFlags = Collections.emptySet();
+	public static final List<RestOperationTypeEnum> REST_OPERATIONS_TO_EXCLUDE_SECURITY_FOR_OPERATION_OUTCOME = List.of(
+			RestOperationTypeEnum.SEARCH_TYPE, RestOperationTypeEnum.SEARCH_SYSTEM, RestOperationTypeEnum.GET_PAGE);
 	private IValidationSupport myValidationSupport;
 
 	private IAuthorizationSearchParamMatcher myAuthorizationSearchParamMatcher;
@@ -512,7 +514,6 @@ private void checkOutgoingResourceAndFailIfDeny(
 		if (alreadySeenMap.putIfAbsent(theResponseObject, Boolean.TRUE) != null) {
 			return;
 		}
-
 		FhirContext fhirContext = theRequestDetails.getServer().getFhirContext();
 		List<IBaseResource> resources = Collections.emptyList();
 
@@ -529,7 +530,8 @@ private void checkOutgoingResourceAndFailIfDeny(
 			case EXTENDED_OPERATION_TYPE:
 			case EXTENDED_OPERATION_INSTANCE: {
 				if (theResponseObject != null) {
-					resources = toListOfResourcesAndExcludeContainerUnlessStandalone(theResponseObject, fhirContext);
+					resources = toListOfResourcesAndExcludeContainerUnlessStandalone(
+							theResponseObject, fhirContext, theRequestDetails);
 				}
 				break;
 			}
@@ -577,21 +579,46 @@ private enum OperationExamineDirection {
 	}
 
 	protected static List<IBaseResource> toListOfResourcesAndExcludeContainerUnlessStandalone(
-			IBaseResource theResponseObject, FhirContext fhirContext) {
+			IBaseResource theResponseObject, FhirContext fhirContext, RequestDetails theRequestDetails) {
+
 		if (theResponseObject == null) {
 			return Collections.emptyList();
 		}
 
-		List<IBaseResource> retVal;
-
 		boolean shouldExamineChildResources = shouldExamineChildResources(theResponseObject, fhirContext);
 		if (!shouldExamineChildResources) {
-			return Collections.singletonList(theResponseObject);
+			return toListOfResourcesAndExcludeOperationOutcomeBasedOnRestOperationType(
+					theResponseObject, theRequestDetails);
 		}
 
 		return toListOfResourcesAndExcludeContainer(theResponseObject, fhirContext);
 	}
 
+	/**
+	 *
+	 * @param theResponseObject The resource to convert to a list.
+	 * @param theRequestDetails The request details.
+	 * @return The response object (a resource) as a list. If the REST operation type in the request details is a
+	 * search, and the search is for resources that aren't the OperationOutcome, any OperationOutcome resource is removed from the list.
+	 * e.g. A GET [base]/Patient?parameter(s) search may return a bundle containing an OperationOutcome. The OperationOutcome will be removed from the
+	 * list to exclude from security.
+	 */
+	private static List<IBaseResource> toListOfResourcesAndExcludeOperationOutcomeBasedOnRestOperationType(
+			IBaseResource theResponseObject, RequestDetails theRequestDetails) {
+		List<IBaseResource> resources = new ArrayList<>();
+		RestOperationTypeEnum restOperationType = theRequestDetails.getRestOperationType();
+		String resourceName = theRequestDetails.getResourceName();
+		resources.add(theResponseObject);
+
+		if (resourceName != null
+				&& !resourceName.equals("OperationOutcome")
+				&& REST_OPERATIONS_TO_EXCLUDE_SECURITY_FOR_OPERATION_OUTCOME.contains(restOperationType)) {
+			resources.removeIf(t -> t instanceof IBaseOperationOutcome);
+		}
+
+		return resources;
+	}
+
 	@Nonnull
 	public static List<IBaseResource> toListOfResourcesAndExcludeContainer(
 			IBaseResource theResponseObject, FhirContext fhirContext) {

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

@@ -842,7 +842,7 @@ private static Verdict applyRulesToResponseBundle(
 			Pointcut thePointcut) {
 		List<IBaseResource> outputResources =
 				AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(
-						theOutputResource, theRequestDetails.getFhirContext());
+						theOutputResource, theRequestDetails.getFhirContext(), theRequestDetails);
 		return applyRulesToResponseResources(theRequestDetails, theRuleApplier, thePointcut, outputResources);
 	}
 

hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java

@@ -92,7 +92,9 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -101,6 +103,7 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
+import java.util.stream.Stream;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -4225,7 +4228,7 @@ public void testToListOfResourcesAndExcludeContainer_withSearchSetContainingDocu
 		RequestDetails requestDetails = new SystemRequestDetails();
 		requestDetails.setResourceName("Bundle");
 
-		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx);
+		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx, requestDetails);
 		assertEquals(1, resources.size());
 		assertTrue(resources.contains(bundle));
 	}
@@ -4242,12 +4245,46 @@ public void testToListOfResourcesAndExcludeContainer_withSearchSetContainingPati
 		RequestDetails requestDetails = new SystemRequestDetails();
 		requestDetails.setResourceName("Patient");
 
-		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx);
+		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(searchSet, ourCtx, requestDetails);
 		assertEquals(2, resources.size());
 		assertTrue(resources.contains(patient1));
 		assertTrue(resources.contains(patient2));
 	}
 
+	@ParameterizedTest
+	@MethodSource("provideArgumentsForToListOfResourcesAndExcludeContainerUnlessStandalone")
+	public void givenAsearchRequestWithOperationOutcomeIntheResponse_whenToListOfResourcesAndExcludeContainerUnlessStandalone_thenReturnNoOperationOutcome(
+		IBaseResource theResource, RequestDetails theRequestDetails, int theExpectedListSize
+	) {
+		List<IBaseResource> resources = AuthorizationInterceptor.toListOfResourcesAndExcludeContainerUnlessStandalone(theResource, ourCtx, theRequestDetails);
+		assertEquals(theExpectedListSize, resources.size());
+	}
+
+	private static Stream<Arguments> provideArgumentsForToListOfResourcesAndExcludeContainerUnlessStandalone() {
+		Stream.Builder<Arguments> retVal = Stream.builder();
+	AuthorizationInterceptor.REST_OPERATIONS_TO_EXCLUDE_SECURITY_FOR_OPERATION_OUTCOME.forEach((restOperationType)->{
+		RequestDetails firstRequestDetails = new SystemRequestDetails();
+		RequestDetails secondRequestDetails = new SystemRequestDetails();
+		firstRequestDetails.setResourceName("Patient");
+		firstRequestDetails.setRestOperationType(restOperationType);
+		secondRequestDetails.setResourceName("OperationOutcome");
+		secondRequestDetails.setRestOperationType(restOperationType);
+
+		OperationOutcome firstResponse = new OperationOutcome();
+		OperationOutcome secondResponse = new OperationOutcome();
+		firstResponse.addIssue().
+			setSeverity(OperationOutcome.IssueSeverity.INFORMATION).
+			setCode(OperationOutcome.IssueType.INFORMATIONAL);
+		secondResponse.addIssue().
+			setSeverity(OperationOutcome.IssueSeverity.ERROR)
+			.setCode(OperationOutcome.IssueType.CODEINVALID);
+
+		retVal.add(Arguments.of(firstResponse, firstRequestDetails, 0));
+		retVal.add(Arguments.of(secondResponse, secondRequestDetails, 1));
+	});
+		return retVal.build();
+	}
+
 	@ParameterizedTest
 	@EnumSource(value = Bundle.BundleType.class, names = {"DOCUMENT", "MESSAGE"})
 	public void testShouldExamineBundleResources_withBundleRequestAndStandAloneBundleType_returnsFalse(Bundle.BundleType theBundleType) {