diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolver.java index 17c35ecca5f13..9c54f6decb0c8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolver.java +++ b/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> listener, - Collection> roleDescriptorsList - ) { - assert roleDescriptorsList.size() == 1; - final var roleDescriptors = roleDescriptorsList.iterator().next(); + private void handleRoleDescriptors(ActionListener> listener, Set roleDescriptors) { for (RoleDescriptor rd : roleDescriptors) { DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java index ab9ea8f772054..631a988b0f325 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java +++ b/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>> listener) { - tryGetRoleDescriptorForInternalUser(subject).ifPresentOrElse( - roleDescriptor -> listener.onResponse(List.of(Set.of(roleDescriptor))), - () -> { - final List roleReferences = subject.getRoleReferenceIntersection(anonymousUser).getRoleReferences(); - final GroupedActionListener> groupedActionListener = new GroupedActionListener<>( - roleReferences.size(), - listener - ); + public void getRoleDescriptors(Subject subject, ActionListener> listener) { + tryGetRoleDescriptorForInternalUser(subject).ifPresentOrElse(roleDescriptor -> listener.onResponse(Set.of(roleDescriptor)), () -> { + final List 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.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 diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolverTests.java index 2af18f5dca7dc..502d2b7ce0dda 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/ApiKeyUserRoleDescriptorResolverTests.java +++ b/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>> listener = (ActionListener>>) args[args.length - - 1]; - listener.onResponse(List.of(roleDescriptors)); + ActionListener> listener = (ActionListener>) 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> 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()); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index c0df3a0947c71..4ad7c61d45d63 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/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>> future = new PlainActionFuture<>(); - compositeRolesStore.getRoleDescriptorsList(subject, future); - assertThat(future.actionGet(), equalTo(List.of(Set.of(internalUser.getLocalClusterRoleDescriptor().get())))); + final PlainActionFuture> 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>> future = new PlainActionFuture<>(); - compositeRolesStore.getRoleDescriptorsList(subject, future); - assertThat(future.actionGet(), equalTo(List.of(Set.of(expectedRoleDescriptor)))); + final PlainActionFuture> future = new PlainActionFuture<>(); + compositeRolesStore.getRoleDescriptors(subject, future); + assertThat(future.actionGet(), equalTo(Set.of(expectedRoleDescriptor))); } private void getRoleForRoleNames(CompositeRolesStore rolesStore, Collection roleNames, ActionListener listener) {