Simplify CompositeRolesStore#getRoleDescriptors (#103126)

This method has only one production caller, which asserts that the
response always has length 1 and (if assertions are disabled) discards
all but the first set of role descriptors in the response. We can push
that behaviour down into the implementation simplifying things quite
considerably and eliminating the allocation and copying needed to
support other sizes of response.

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolver.java

@@ -15,7 +15,6 @@
 import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 
-import java.util.Collection;
 import java.util.Set;
 
 public class ApiKeyUserRoleDescriptorResolver {
@@ -37,15 +36,10 @@ public void resolveUserRoleDescriptors(final Authentication authentication, fina
             return;
         }
 
-        rolesStore.getRoleDescriptorsList(effectiveSubject, listener.delegateFailureAndWrap(this::handleRoleDescriptorsList));
+        rolesStore.getRoleDescriptors(effectiveSubject, listener.delegateFailureAndWrap(this::handleRoleDescriptors));
     }
 
-    private void handleRoleDescriptorsList(
-        ActionListener<Set<RoleDescriptor>> listener,
-        Collection<Set<RoleDescriptor>> roleDescriptorsList
-    ) {
-        assert roleDescriptorsList.size() == 1;
-        final var roleDescriptors = roleDescriptorsList.iterator().next();
+    private void handleRoleDescriptors(ActionListener<Set<RoleDescriptor>> listener, Set<RoleDescriptor> roleDescriptors) {
         for (RoleDescriptor rd : roleDescriptors) {
             DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry);
         }

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

@@ -10,7 +10,6 @@
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.support.GroupedActionListener;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.cache.Cache;
@@ -349,30 +348,18 @@ private void buildThenMaybeCacheRole(
         );
     }
 
-    public void getRoleDescriptorsList(Subject subject, ActionListener<Collection<Set<RoleDescriptor>>> listener) {
-        tryGetRoleDescriptorForInternalUser(subject).ifPresentOrElse(
-            roleDescriptor -> listener.onResponse(List.of(Set.of(roleDescriptor))),
-            () -> {
-                final List<RoleReference> roleReferences = subject.getRoleReferenceIntersection(anonymousUser).getRoleReferences();
-                final GroupedActionListener<Set<RoleDescriptor>> groupedActionListener = new GroupedActionListener<>(
-                    roleReferences.size(),
-                    listener
-                );
+    public void getRoleDescriptors(Subject subject, ActionListener<Set<RoleDescriptor>> listener) {
+        tryGetRoleDescriptorForInternalUser(subject).ifPresentOrElse(roleDescriptor -> listener.onResponse(Set.of(roleDescriptor)), () -> {
+            final List<RoleReference> roleReferences = subject.getRoleReferenceIntersection(anonymousUser).getRoleReferences();
+            assert roleReferences.size() == 1; // we only handle the singleton case today, but that may change with derived API keys
 
-                roleReferences.forEach(
-                    roleReference -> roleReference.resolve(
-                        roleReferenceResolver,
-                        groupedActionListener.delegateFailureAndWrap((delegate, rolesRetrievalResult) -> {
-                            if (rolesRetrievalResult.isSuccess()) {
-                                delegate.onResponse(rolesRetrievalResult.getRoleDescriptors());
-                            } else {
-                                delegate.onFailure(new ElasticsearchException("role retrieval had one or more failures"));
-                            }
-                        })
-                    )
-                );
-            }
-        );
+            ActionListener.run(listener.<RolesRetrievalResult>map(rolesRetrievalResult -> {
+                if (rolesRetrievalResult.isSuccess() == false) {
+                    throw new ElasticsearchException("role retrieval had one or more failures");
+                }
+                return rolesRetrievalResult.getRoleDescriptors();
+            }), l -> roleReferences.iterator().next().resolve(roleReferenceResolver, l));
+        });
     }
 
     // Package private for testing

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolverTests.java

@@ -19,8 +19,6 @@
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 
-import java.util.Collection;
-import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -56,11 +54,10 @@ public void testGetRoleDescriptors() {
             assertThat(subject.getType(), is(Subject.Type.USER));
             assertThat(Set.of(subject.getUser().roles()), equalTo(userRoleNames));
 
-            ActionListener<Collection<Set<RoleDescriptor>>> listener = (ActionListener<Collection<Set<RoleDescriptor>>>) args[args.length
-                - 1];
-            listener.onResponse(List.of(roleDescriptors));
+            ActionListener<Set<RoleDescriptor>> listener = (ActionListener<Set<RoleDescriptor>>) args[args.length - 1];
+            listener.onResponse(roleDescriptors);
             return null;
-        }).when(rolesStore).getRoleDescriptorsList(any(Subject.class), any(ActionListener.class));
+        }).when(rolesStore).getRoleDescriptors(any(Subject.class), any(ActionListener.class));
 
         final PlainActionFuture<Set<RoleDescriptor>> future = new PlainActionFuture<>();
         resolver.resolveUserRoleDescriptors(authentication, future);
@@ -77,6 +74,6 @@ public void testGetRoleDescriptorsEmptyForApiKey() {
         resolver.resolveUserRoleDescriptors(authentication, future);
 
         assertThat(future.actionGet(), equalTo(Set.of()));
-        verify(rolesStore, never()).getRoleDescriptorsList(any(), any());
+        verify(rolesStore, never()).getRoleDescriptors(any(), any());
     }
 }

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

@@ -2760,15 +2760,15 @@ public void testGetRoleDescriptorsListForInternalUsers() {
         when(subject.getUser()).thenReturn(InternalUsers.SYSTEM_USER);
         final IllegalArgumentException e1 = expectThrows(
             IllegalArgumentException.class,
-            () -> compositeRolesStore.getRoleDescriptorsList(subject, new PlainActionFuture<>())
+            () -> compositeRolesStore.getRoleDescriptors(subject, ActionListener.noop())
         );
         assertThat(e1.getMessage(), equalTo("should never try to get the roles for internal user [" + SystemUser.NAME + "]"));
 
         for (var internalUser : AuthenticationTestHelper.internalUsersWithLocalRoleDescriptor()) {
             when(subject.getUser()).thenReturn(internalUser);
-            final PlainActionFuture<Collection<Set<RoleDescriptor>>> future = new PlainActionFuture<>();
-            compositeRolesStore.getRoleDescriptorsList(subject, future);
-            assertThat(future.actionGet(), equalTo(List.of(Set.of(internalUser.getLocalClusterRoleDescriptor().get()))));
+            final PlainActionFuture<Set<RoleDescriptor>> future = new PlainActionFuture<>();
+            compositeRolesStore.getRoleDescriptors(subject, future);
+            assertThat(future.actionGet(), equalTo(Set.of(internalUser.getLocalClusterRoleDescriptor().get())));
         }
     }
 
@@ -2787,9 +2787,9 @@ public void testGetRoleDescriptorsListUsesRoleStoreToResolveRoleWithInternalRole
         when(subject.getRoleReferenceIntersection(any())).thenReturn(
             new RoleReferenceIntersection(new RoleReference.NamedRoleReference(new String[] { roleName }))
         );
-        final PlainActionFuture<Collection<Set<RoleDescriptor>>> future = new PlainActionFuture<>();
-        compositeRolesStore.getRoleDescriptorsList(subject, future);
-        assertThat(future.actionGet(), equalTo(List.of(Set.of(expectedRoleDescriptor))));
+        final PlainActionFuture<Set<RoleDescriptor>> future = new PlainActionFuture<>();
+        compositeRolesStore.getRoleDescriptors(subject, future);
+        assertThat(future.actionGet(), equalTo(Set.of(expectedRoleDescriptor)));
     }
 
     private void getRoleForRoleNames(CompositeRolesStore rolesStore, Collection<String> roleNames, ActionListener<Role> listener) {