address review comments

Author: richard-dennehy

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -534,16 +534,19 @@ private void authorizeAction(
                         .addListener(
                             wrapPreservingContext(
                                 new AuthorizationResultListener<>(
-                                    result -> handleIndexActionAuthorizationResult(
-                                        result,
-                                        requestInfo,
-                                        requestId,
-                                        authzInfo,
-                                        authzEngine,
-                                        resolvedIndicesAsyncSupplier,
-                                        projectMetadata,
-                                        l
-                                    ),
+                                    result -> {
+                                        setIndexResolutionException(authzInfo, request, authentication, action);
+                                        handleIndexActionAuthorizationResult(
+                                            result,
+                                            requestInfo,
+                                            requestId,
+                                            authzInfo,
+                                            authzEngine,
+                                            resolvedIndicesAsyncSupplier,
+                                            projectMetadata,
+                                            l
+                                        );
+                                    },
                                     l::onFailure,
                                     requestInfo,
                                     requestId,
@@ -712,31 +715,38 @@ private void handleIndexActionAuthorizationResult(
                 )
             );
         } else {
-            if (request instanceof IndicesRequest.Replaceable replaceable) {
-                var indexExpressions = replaceable.getResolvedIndexExpressions();
-                if (indexExpressions != null) {
-                    indexExpressions.expressions().forEach(resolved -> {
-                        if (resolved.localExpressions().localIndexResolutionResult() == CONCRETE_RESOURCE_UNAUTHORIZED) {
-                            resolved.localExpressions()
-                                .setException(
-                                    actionDenied(
-                                        authentication,
-                                        authzInfo,
-                                        action,
-                                        request,
-                                        IndexAuthorizationResult.getFailureDescription(List.of(resolved.original()), restrictedIndices),
-                                        null
-                                    )
-                                );
-                        }
-                    });
-                }
-            }
-
             runRequestInterceptors(requestInfo, authzInfo, authorizationEngine, listener);
         }
     }
 
+    private void setIndexResolutionException(
+        AuthorizationInfo authzInfo,
+        TransportRequest request,
+        Authentication authentication,
+        String action
+    ) {
+        if (request instanceof IndicesRequest.Replaceable replaceable) {
+            var indexExpressions = replaceable.getResolvedIndexExpressions();
+            if (indexExpressions != null) {
+                indexExpressions.expressions().forEach(resolved -> {
+                    if (resolved.localExpressions().localIndexResolutionResult() == CONCRETE_RESOURCE_UNAUTHORIZED) {
+                        resolved.localExpressions()
+                            .setException(
+                                actionDenied(
+                                    authentication,
+                                    authzInfo,
+                                    action,
+                                    request,
+                                    IndexAuthorizationResult.getFailureDescription(List.of(resolved.original()), restrictedIndices),
+                                    null
+                                )
+                            );
+                    }
+                });
+            }
+        }
+    }
+
     private void runRequestInterceptors(
         RequestInfo requestInfo,
         AuthorizationInfo authorizationInfo,

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

@@ -3869,38 +3869,53 @@ public void testSetExceptionOnMissingIndexWhenIgnoreUnavailable() {
         when(crossProjectModeDecider.crossProjectEnabled()).thenReturn(true);
         when(crossProjectModeDecider.resolvesCrossProject(any())).thenReturn(true);
 
-        mockMetadataWithIndex("available-index");
+        final var metadataBuilder = ProjectMetadata.builder(projectId).put(createIndexMetadata("accessible-index"), true);
+        if (randomBoolean()) {
+            metadataBuilder.put(createIndexMetadata("not-accessible-index"), true);
+        }
+
+        mockClusterState(metadataBuilder.build());
+
         var authentication = createAuthentication(new User("user", "partial-access-role"));
         roleMap.put(
             "partial-access-role",
             new RoleDescriptor(
                 "partial-access-role",
                 null,
-                new IndicesPrivileges[] { IndicesPrivileges.builder().indices("available-index").privileges("read").build() },
+                new IndicesPrivileges[] { IndicesPrivileges.builder().indices("accessible-index").privileges("read").build() },
                 null
             )
         );
-        final var request = new SearchRequest("available-index", "not-available-index").indicesOptions(
-            IndicesOptions.fromOptions(true, false, true, false)
-        );
+        var requestAccessibleIndex = randomBoolean();
+
+        var request = requestAccessibleIndex
+            ? new SearchRequest("accessible-index", "not-accessible-index")
+            : new SearchRequest("not-accessible-index");
+
+        request.indicesOptions(IndicesOptions.fromOptions(true, false, true, false));
         AuditUtil.getOrGenerateRequestId(threadContext);
         authorize(authentication, TransportSearchAction.TYPE.name(), request);
 
         final var authorizationInfo = mock(AuthorizationInfo.class);
         when(authorizationInfo.asMap()).thenReturn(Map.of("user.info", new String[] { "partial-access-role" }));
 
         final var expressions = request.getResolvedIndexExpressions().expressions();
-        assertThat(expressions, hasSize(2));
-        assertThat(expressions.getFirst(), equalTo(resolvedIndexExpression("available-index", Set.of("available-index"), SUCCESS)));
+        if (requestAccessibleIndex) {
+            assertThat(expressions, hasSize(2));
+            assertThat(expressions.getFirst(), equalTo(resolvedIndexExpression("accessible-index", Set.of("accessible-index"), SUCCESS)));
+        } else {
+            assertThat(expressions, hasSize(1));
+        }
 
-        assertThat(expressions.get(1).original(), equalTo("not-available-index"));
-        assertThat(expressions.get(1).localExpressions().indices(), empty());
-        assertThat(expressions.get(1).localExpressions().localIndexResolutionResult(), equalTo(CONCRETE_RESOURCE_UNAUTHORIZED));
+        var notAccessibleIndexExpression = requestAccessibleIndex ? expressions.get(1) : expressions.getFirst();
+        assertThat(notAccessibleIndexExpression.original(), equalTo("not-accessible-index"));
+        assertThat(notAccessibleIndexExpression.localExpressions().indices(), empty());
+        assertThat(notAccessibleIndexExpression.localExpressions().localIndexResolutionResult(), equalTo(CONCRETE_RESOURCE_UNAUTHORIZED));
         assertThat(
-            expressions.get(1).localExpressions().exception().getMessage(),
+            notAccessibleIndexExpression.localExpressions().exception().getMessage(),
             equalTo(
                 "action [indices:data/read/search] is unauthorized for user [user]"
-                    + " with effective roles [partial-access-role] on indices [not-available-index], "
+                    + " with effective roles [partial-access-role] on indices [not-accessible-index], "
                     + "this action is granted by the index privileges [read,all]"
             )
         );