Ensure authz role for API key is named after owner role (#59041)

The composite role that is used for authz, following the authn with an API key,
is an intersection of the privileges from the owner role and the key privileges defined
when the key has been created.
This change ensures that the `#names` property of such a role equals the `#names`
property of the key owner role, thereby rectifying the value for the `user.roles`
audit event field.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java

@@ -29,17 +29,12 @@ import java.util.function.Predicate;
 public final class LimitedRole extends Role {
     private final Role limitedBy;
 
-    LimitedRole(String[] names, ClusterPermission cluster, IndicesPermission indices, ApplicationPermission application,
+    LimitedRole(ClusterPermission cluster, IndicesPermission indices, ApplicationPermission application, RunAsPermission runAs,
-            RunAsPermission runAs, Role limitedBy) {
+                Role limitedBy) {
-        super(names, cluster, indices, application, runAs);
+        super(Objects.requireNonNull(limitedBy, "limiting role is required").names(), cluster, indices, application, runAs);
-        assert limitedBy != null : "limiting role is required";
         this.limitedBy = limitedBy;
     }
 
-    public Role limitedBy() {
-        return limitedBy;
-    }
-
     @Override
     public ClusterPermission cluster() {
         throw new UnsupportedOperationException("cannot retrieve cluster permission on limited role");
@@ -86,7 +81,7 @@ public final class LimitedRole extends Role {
     @Override
     public Automaton allowedActionsMatcher(String index) {
         final Automaton allowedMatcher = super.allowedActionsMatcher(index);
-        final Automaton limitedByMatcher = super.allowedActionsMatcher(index);
+        final Automaton limitedByMatcher = limitedBy.allowedActionsMatcher(index);
         return Automatons.intersectAndMinimize(allowedMatcher, limitedByMatcher);
     }
 
@@ -187,7 +182,6 @@ public final class LimitedRole extends Role {
      */
     public static LimitedRole createLimitedRole(Role fromRole, Role limitedByRole) {
         Objects.requireNonNull(limitedByRole, "limited by role is required to create limited role");
-        return new LimitedRole(fromRole.names(), fromRole.cluster(), fromRole.indices(), fromRole.application(), fromRole.runAs(),
+        return new LimitedRole(fromRole.cluster(), fromRole.indices(), fromRole.application(), fromRole.runAs(), limitedByRole);
-                limitedByRole);
     }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java

@@ -6,9 +6,11 @@
 
 package org.elasticsearch.xpack.core.security.authz.permission;
 
+import org.apache.lucene.util.automaton.Automaton;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
 import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction;
+import org.elasticsearch.action.bulk.BulkAction;
 import org.elasticsearch.action.search.SearchAction;
 import org.elasticsearch.cluster.metadata.AliasMetadata;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -24,12 +26,14 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivileg
 import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
 import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
 import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
+import org.elasticsearch.xpack.core.security.support.Automatons;
 import org.junit.Before;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Predicate;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -50,6 +54,7 @@ public class LimitedRoleTests extends ESTestCase {
         Role limitedByRole = Role.builder("limited-role").build();
         Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole);
         assertNotNull(role);
+        assertThat(role.names(), is(limitedByRole.names()));
 
         NullPointerException npe = expectThrows(NullPointerException.class, () -> LimitedRole.createLimitedRole(fromRole, null));
         assertThat(npe.getMessage(), containsString("limited by role is required to create limited role"));
@@ -200,6 +205,42 @@ public class LimitedRoleTests extends ESTestCase {
         }
     }
 
+    public void testAllowedActionsMatcher() {
+        Role fromRole = Role.builder("fromRole")
+                .add(IndexPrivilege.WRITE, "ind*")
+                .add(IndexPrivilege.READ, "ind*")
+                .add(IndexPrivilege.READ, "other*")
+                .build();
+        Automaton fromRoleAutomaton = fromRole.allowedActionsMatcher("index1");
+        Predicate<String> fromRolePredicate = Automatons.predicate(fromRoleAutomaton);
+        assertThat(fromRolePredicate.test(SearchAction.NAME), is(true));
+        assertThat(fromRolePredicate.test(BulkAction.NAME), is(true));
+
+        Role limitedByRole = Role.builder("limitedRole")
+                .add(IndexPrivilege.READ, "index1", "index2")
+                .build();
+        Automaton limitedByRoleAutomaton = limitedByRole.allowedActionsMatcher("index1");
+        Predicate<String> limitedByRolePredicated = Automatons.predicate(limitedByRoleAutomaton);
+        assertThat(limitedByRolePredicated.test(SearchAction.NAME), is(true));
+        assertThat(limitedByRolePredicated.test(BulkAction.NAME), is(false));
+        Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole);
+
+        Automaton roleAutomaton = role.allowedActionsMatcher("index1");
+        Predicate<String> rolePredicate = Automatons.predicate(roleAutomaton);
+        assertThat(rolePredicate.test(SearchAction.NAME), is(true));
+        assertThat(rolePredicate.test(BulkAction.NAME), is(false));
+
+        roleAutomaton = role.allowedActionsMatcher("index2");
+        rolePredicate = Automatons.predicate(roleAutomaton);
+        assertThat(rolePredicate.test(SearchAction.NAME), is(true));
+        assertThat(rolePredicate.test(BulkAction.NAME), is(false));
+
+        roleAutomaton = role.allowedActionsMatcher("other");
+        rolePredicate = Automatons.predicate(roleAutomaton);
+        assertThat(rolePredicate.test(SearchAction.NAME), is(false));
+        assertThat(rolePredicate.test(BulkAction.NAME), is(false));
+    }
+
     public void testCheckClusterPrivilege() {
         Role fromRole = Role.builder("a-role").cluster(Collections.singleton("manage_security"), Collections.emptyList())
                 .build();

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

@@ -71,6 +71,7 @@ import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
 import org.elasticsearch.xpack.core.security.action.GetApiKeyResponse;
 import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
+import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
@@ -246,7 +247,7 @@ public class ApiKeyService {
     }
 
     /**
-     * package protected for testing
+     * package-private for testing
      */
     XContentBuilder newDocument(SecureString apiKey, String name, Authentication authentication, Set<RoleDescriptor> userRoles,
                                         Instant created, Instant expiration, List<RoleDescriptor> keyRoles,
@@ -335,6 +336,16 @@ public class ApiKeyService {
         }
     }
 
+    public Authentication createApiKeyAuthentication(AuthenticationResult authResult, String nodeName) {
+        if (false == authResult.isAuthenticated()) {
+            throw new IllegalArgumentException("API Key authn result must be successful");
+        }
+        final User user = authResult.getUser();
+        final RealmRef authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName);
+        return new Authentication(user, authenticatedBy, null, Version.CURRENT, Authentication.AuthenticationType.API_KEY,
+                authResult.getMetadata());
+    }
+
     private void loadApiKeyAndValidateCredentials(ThreadContext ctx, ApiKeyCredentials credentials,
                                                   ActionListener<AuthenticationResult> listener) {
         final String docId = credentials.getId();
@@ -531,7 +542,8 @@ public class ApiKeyService {
         return apiKeyAuthCache == null ? null : FutureUtils.get(apiKeyAuthCache.get(id), 0L, TimeUnit.MILLISECONDS);
     }
 
-    private void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
+    // package-private for testing
+    void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
                                   ActionListener<AuthenticationResult> listener) {
         final Long expirationEpochMilli = (Long) source.get("expiration_time");
         if (expirationEpochMilli == null || Instant.ofEpochMilli(expirationEpochMilli).isAfter(clock.instant())) {
@@ -624,12 +636,12 @@ public class ApiKeyService {
         }
     }
 
-    // package private class for testing
+    // public class for testing
-    static final class ApiKeyCredentials implements Closeable {
+    public static final class ApiKeyCredentials implements Closeable {
         private final String id;
         private final SecureString key;
 
-        ApiKeyCredentials(String id, SecureString key) {
+        public ApiKeyCredentials(String id, SecureString key) {
             this.id = id;
             this.key = key;
         }

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

@@ -346,10 +346,9 @@ public class AuthenticationService {
         private void checkForApiKey() {
             apiKeyService.authenticateWithApiKeyIfPresent(threadContext, ActionListener.wrap(authResult -> {
                     if (authResult.isAuthenticated()) {
-                        final User user = authResult.getUser();
+                        final Authentication authentication = apiKeyService.createApiKeyAuthentication(authResult, nodeName);
-                        authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName);
+                        this.authenticatedBy = authentication.getAuthenticatedBy();
-                        writeAuthToContext(new Authentication(user, authenticatedBy, null, Version.CURRENT,
+                        writeAuthToContext(authentication);
-                            Authentication.AuthenticationType.API_KEY, authResult.getMetadata()));
                     } else if (authResult.getStatus() == AuthenticationResult.Status.TERMINATE) {
                         Exception e = (authResult.getException() != null) ? authResult.getException()
                             : Exceptions.authenticationError(authResult.getMessage());

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

@@ -66,7 +66,9 @@ import java.util.Base64;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Semaphore;
@@ -719,6 +721,23 @@ public class ApiKeyServiceTests extends ESTestCase {
         assertEquals(AuthenticationResult.Status.SUCCESS, authenticationResult3.getStatus());
     }
 
+    public static class Utils {
+
+        public static Authentication createApiKeyAuthentication(ApiKeyService apiKeyService,
+                                                                Authentication authentication,
+                                                                Set<RoleDescriptor> userRoles,
+                                                                List<RoleDescriptor> keyRoles) throws Exception {
+            XContentBuilder keyDocSource = apiKeyService.newDocument(new SecureString("secret".toCharArray()), "test", authentication,
+                    userRoles, Instant.now(), Instant.now().plus(Duration.ofSeconds(3600)), keyRoles, Version.CURRENT);
+            Map<String, Object> keyDocMap = XContentHelper.convertToMap(BytesReference.bytes(keyDocSource), true, XContentType.JSON).v2();
+            PlainActionFuture<AuthenticationResult> authenticationResultFuture = PlainActionFuture.newFuture();
+            apiKeyService.validateApiKeyExpiration(keyDocMap, new ApiKeyService.ApiKeyCredentials("id",
+                            new SecureString("pass".toCharArray())),
+                    Clock.systemUTC(), authenticationResultFuture);
+            return apiKeyService.createApiKeyAuthentication(authenticationResultFuture.get(), "node01");
+        }
+    }
+
     private ApiKeyService createApiKeyService(Settings baseSettings) {
         final Settings settings = Settings.builder()
             .put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true)

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

@@ -12,9 +12,11 @@ import org.elasticsearch.action.admin.cluster.state.ClusterStateAction;
 import org.elasticsearch.action.get.GetAction;
 import org.elasticsearch.action.index.IndexAction;
 import org.elasticsearch.action.support.PlainActionFuture;
+import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -63,10 +65,10 @@ import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.core.security.user.XPackUser;
 import org.elasticsearch.xpack.security.audit.AuditUtil;
 import org.elasticsearch.xpack.security.authc.ApiKeyService;
-import org.elasticsearch.xpack.security.authc.ApiKeyService.ApiKeyRoleDescriptors;
 import org.elasticsearch.xpack.security.support.SecurityIndexManager;
 
 import java.io.IOException;
+import java.time.Clock;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -88,8 +90,10 @@ import java.util.function.Predicate;
 
 import static org.elasticsearch.mock.orig.Mockito.times;
 import static org.elasticsearch.mock.orig.Mockito.verifyNoMoreInteractions;
+import static org.elasticsearch.xpack.security.authc.ApiKeyServiceTests.Utils.createApiKeyAuthentication;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.hasSize;
@@ -1009,7 +1013,7 @@ public class CompositeRolesStoreTests extends ESTestCase {
         assertEquals("the user [_system] is the system user and we should never try to get its roles", iae.getMessage());
     }
 
-    public void testApiKeyAuthUsesApiKeyService() throws IOException {
+    public void testApiKeyAuthUsesApiKeyService() throws Exception {
         final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
         doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class));
         final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class);
@@ -1022,7 +1026,9 @@ public class CompositeRolesStoreTests extends ESTestCase {
         }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class));
         final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore());
         ThreadContext threadContext = new ThreadContext(SECURITY_ENABLED_SETTINGS);
-        ApiKeyService apiKeyService = mock(ApiKeyService.class);
+        ApiKeyService apiKeyService = new ApiKeyService(SECURITY_ENABLED_SETTINGS, Clock.systemUTC(), mock(Client.class),
+                new XPackLicenseState(SECURITY_ENABLED_SETTINGS), mock(SecurityIndexManager.class), mock(ClusterService.class),
+                mock(ThreadPool.class));
         NativePrivilegeStore nativePrivStore = mock(NativePrivilegeStore.class);
         doAnswer(invocationOnMock -> {
             ActionListener<Collection<ApplicationPrivilegeDescriptor>> listener =
@@ -1039,23 +1045,19 @@ public class CompositeRolesStoreTests extends ESTestCase {
                 new XPackLicenseState(SECURITY_ENABLED_SETTINGS), cache, apiKeyService, documentSubsetBitsetCache,
                 rds -> effectiveRoleDescriptors.set(rds));
         AuditUtil.getOrGenerateRequestId(threadContext);
-        final Authentication authentication = new Authentication(new User("test api key user", "superuser"),
+
-            new RealmRef("_es_api_key", "_es_api_key", "node"), null, Version.CURRENT, AuthenticationType.API_KEY, Collections.emptyMap());
+        final Authentication authentication = createApiKeyAuthentication(apiKeyService, createAuthentication(),
-        doAnswer(invocationOnMock -> {
+                Collections.singleton(new RoleDescriptor("user_role_" + randomAlphaOfLength(4), new String[]{"manage"}, null, null)), null);
-            ActionListener<ApiKeyRoleDescriptors> listener = (ActionListener<ApiKeyRoleDescriptors>) invocationOnMock.getArguments()[1];
-            listener.onResponse(new ApiKeyRoleDescriptors("keyId",
-                Collections.singletonList(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR), null));
-            return Void.TYPE;
-        }).when(apiKeyService).getRoleForApiKey(eq(authentication), any(ActionListener.class));
 
         PlainActionFuture<Role> roleFuture = new PlainActionFuture<>();
         compositeRolesStore.getRoles(authentication.getUser(), authentication, roleFuture);
-        roleFuture.actionGet();
+        Role role = roleFuture.actionGet();
         assertThat(effectiveRoleDescriptors.get(), is(nullValue()));
-        verify(apiKeyService).getRoleForApiKey(eq(authentication), any(ActionListener.class));
+        assertThat(role.names().length, is(1));
+        assertThat(role.names()[0], containsString("user_role_"));
     }
 
-    public void testApiKeyAuthUsesApiKeyServiceWithScopedRole() throws IOException {
+    public void testApiKeyAuthUsesApiKeyServiceWithScopedRole() throws Exception {
         final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
         doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class));
         final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class);
@@ -1068,7 +1070,10 @@ public class CompositeRolesStoreTests extends ESTestCase {
         }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class));
         final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore());
         ThreadContext threadContext = new ThreadContext(SECURITY_ENABLED_SETTINGS);
-        ApiKeyService apiKeyService = mock(ApiKeyService.class);
+
+        ApiKeyService apiKeyService = new ApiKeyService(SECURITY_ENABLED_SETTINGS, Clock.systemUTC(), mock(Client.class),
+                new XPackLicenseState(SECURITY_ENABLED_SETTINGS), mock(SecurityIndexManager.class), mock(ClusterService.class),
+                mock(ThreadPool.class));
         NativePrivilegeStore nativePrivStore = mock(NativePrivilegeStore.class);
         doAnswer(invocationOnMock -> {
             ActionListener<Collection<ApplicationPrivilegeDescriptor>> listener =
@@ -1085,23 +1090,18 @@ public class CompositeRolesStoreTests extends ESTestCase {
                 new XPackLicenseState(SECURITY_ENABLED_SETTINGS), cache, apiKeyService, documentSubsetBitsetCache,
                 rds -> effectiveRoleDescriptors.set(rds));
         AuditUtil.getOrGenerateRequestId(threadContext);
-        final Authentication authentication = new Authentication(new User("test api key user", "api_key"),
+
-            new RealmRef("_es_api_key", "_es_api_key", "node"), null, Version.CURRENT, AuthenticationType.API_KEY, Collections.emptyMap());
+        final Authentication authentication = createApiKeyAuthentication(apiKeyService, createAuthentication(),
-        doAnswer(invocationOnMock -> {
+                Collections.singleton(new RoleDescriptor("user_role_" + randomAlphaOfLength(4), new String[]{"manage"}, null, null)),
-            ActionListener<ApiKeyRoleDescriptors> listener = (ActionListener<ApiKeyRoleDescriptors>) invocationOnMock.getArguments()[1];
+                Collections.singletonList(new RoleDescriptor("key_role_" + randomAlphaOfLength(8), new String[]{"monitor"}, null, null)));
-            listener.onResponse(new ApiKeyRoleDescriptors("keyId",
-                Collections.singletonList(new RoleDescriptor("a-role", new String[] {"all"}, null, null)),
-                Collections.singletonList(
-                    new RoleDescriptor("scoped-role", new String[] {"manage_security"}, null, null))));
-            return Void.TYPE;
-        }).when(apiKeyService).getRoleForApiKey(eq(authentication), any(ActionListener.class));
 
         PlainActionFuture<Role> roleFuture = new PlainActionFuture<>();
         compositeRolesStore.getRoles(authentication.getUser(), authentication, roleFuture);
         Role role = roleFuture.actionGet();
         assertThat(role.checkClusterAction("cluster:admin/foo", Empty.INSTANCE, mock(Authentication.class)), is(false));
         assertThat(effectiveRoleDescriptors.get(), is(nullValue()));
-        verify(apiKeyService).getRoleForApiKey(eq(authentication), any(ActionListener.class));
+        assertThat(role.names().length, is(1));
+        assertThat(role.names()[0], containsString("user_role_"));
     }
 
     public void testUsageStats() {
@@ -1184,6 +1184,24 @@ public class CompositeRolesStoreTests extends ESTestCase {
         );
     }
 
+    private Authentication createAuthentication() {
+        final RealmRef lookedUpBy;
+        final User user;
+        if (randomBoolean()) {
+            user = new User("_username", randomBoolean() ? new String[]{"r1"} :
+                    new String[]{ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()},
+                    new User("authenticated_username", new String[]{"r2"}));
+            lookedUpBy = new RealmRef("lookRealm", "up", "by");
+        } else {
+            user = new User("_username", randomBoolean() ? new String[]{"r1"} :
+                    new String[]{ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()});
+            lookedUpBy = null;
+        }
+        return new Authentication(user, new RealmRef("authRealm", "test", "foo"), lookedUpBy,
+                Version.CURRENT, randomFrom(AuthenticationType.REALM, AuthenticationType.TOKEN, AuthenticationType.INTERNAL,
+                AuthenticationType.ANONYMOUS), Collections.emptyMap());
+    }
+
     private CompositeRolesStore buildCompositeRolesStore(Settings settings,
                                                          @Nullable FileRolesStore fileRolesStore,
                                                          @Nullable NativeRolesStore nativeRolesStore,