Fix incorrect assumptions for api_key authentication type (elastic#81564

)

This is essentially a follow-up PR for elastic#79809 which enables run-as for API keys.
The code still has a few places where it assumes API keys cannot perform run-as. 
This PR fixes the incorrect assumptions and adds relevant tests.

Resolves: elastic#81425

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

@@ -186,7 +186,8 @@ public void executeAfterRewritingAuthentication(Consumer<StoredContext> consumer
     @SuppressWarnings("unchecked")
     private Map<String, Object> rewriteMetadataForApiKeyRoleDescriptors(Version streamVersion, Authentication authentication) {
         Map<String, Object> metadata = authentication.getMetadata();
-        if (authentication.getAuthenticationType() == AuthenticationType.API_KEY) {
+        // If authentication type is API key, regardless whether it has run-as, the metadata must contain API key role descriptors
+        if (authentication.isAuthenticatedWithApiKey()) {
             if (authentication.getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)
                 && streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) {
                 metadata = new HashMap<>(metadata);

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

@@ -125,6 +125,20 @@ public boolean isAuthenticatedWithApiKey() {
         return AuthenticationType.API_KEY.equals(getAuthenticationType());
     }
 
+    /**
+     * Authenticate with a service account and no run-as
+     */
+    public boolean isServiceAccount() {
+        return isAuthenticatedWithServiceAccount() && false == getUser().isRunAs();
+    }
+
+    /**
+     * Authenticated with an API key and no run-as
+     */
+    public boolean isApiKey() {
+        return isAuthenticatedWithApiKey() && false == getUser().isRunAs();
+    }
+
     /**
      * Writes the authentication to the context. There must not be an existing authentication in the context and if there is an
      * {@link IllegalStateException} will be thrown
@@ -170,11 +184,11 @@ public void writeTo(StreamOutput out) throws IOException {
      *      security limitations</a>
      */
     public boolean canAccessResourcesOf(Authentication other) {
-        if (AuthenticationType.API_KEY == getAuthenticationType() && AuthenticationType.API_KEY == other.getAuthenticationType()) {
+        if (isApiKey() && other.isApiKey()) {
             final boolean sameKeyId = getMetadata().get(AuthenticationField.API_KEY_ID_KEY)
                 .equals(other.getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
             if (sameKeyId) {
-                assert getUser().principal().equals(other.getUser().principal())
+                assert getUser().principal().equals(getUser().principal())
                     : "The same API key ID cannot be attributed to two different usernames";
             }
             return sameKeyId;
@@ -278,7 +292,7 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
     }
 
     private void assertApiKeyMetadata() {
-        assert (AuthenticationType.API_KEY.equals(this.type) == false) || (this.metadata.get(AuthenticationField.API_KEY_ID_KEY) != null)
+        assert (false == isAuthenticatedWithApiKey()) || (this.metadata.get(AuthenticationField.API_KEY_ID_KEY) != null)
             : "API KEY authentication requires metadata to contain API KEY id, and the value must be non-null.";
     }
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java

@@ -14,7 +14,6 @@
 import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyRequest;
 import org.elasticsearch.xpack.core.security.action.apikey.QueryApiKeyRequest;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
-import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
 import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission;
 import org.elasticsearch.xpack.core.security.support.Automatons;
@@ -120,7 +119,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(
                  * TODO bizybot we need to think on how we can propagate appropriate error message to the end user when username, realm name
                  *   is missing. This is similar to the problem of propagating right error messages in case of access denied.
                  */
-                if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) {
+                if (authentication.isApiKey()) {
                     // API key cannot own any other API key so deny access
                     return false;
                 } else if (ownedByAuthenticatedUser) {
@@ -135,7 +134,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(
         }
 
         private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) {
-            if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) {
+            if (authentication.isApiKey()) {
                 // API key id from authentication must match the id from request
                 final String authenticatedApiKeyId = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY);
                 if (Strings.hasText(apiKeyId)) {

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

@@ -96,6 +96,27 @@ public void testCanAccessResourcesOf() {
             randomApiKeyAuthentication(randomFrom(user1, user2), apiKeyId1),
             randomApiKeyAuthentication(randomFrom(user1, user2), apiKeyId2)
         );
+
+        final User user3 = randomValueOtherThanMany(
+            u -> u.principal().equals(user1.principal()) || u.principal().equals(user2.principal()),
+            AuthenticationTests::randomUser
+        );
+
+        // Same API key but run-as different user are not the same owner
+        assertCannotAccessResources(
+            randomApiKeyAuthentication(new User(user2, user1), apiKeyId1),
+            randomApiKeyAuthentication(new User(user3, user1), apiKeyId1)
+        );
+
+        // Same or different API key run-as the same user are the same owner
+        checkCanAccessResources(
+            randomApiKeyAuthentication(new User(user3, user1), apiKeyId1),
+            randomApiKeyAuthentication(new User(user3, user1), apiKeyId1)
+        );
+        checkCanAccessResources(
+            randomApiKeyAuthentication(new User(user3, user1), apiKeyId1),
+            randomApiKeyAuthentication(new User(user3, user2), apiKeyId2)
+        );
     }
 
     public void testIsServiceAccount() {
@@ -213,7 +234,7 @@ public static Authentication randomApiKeyAuthentication(User user, String apiKey
         return new Authentication(
             user,
             apiKeyRealm,
-            null,
+            user.isRunAs() ? new RealmRef("lookup_realm", "lookup_realm", randomAlphaOfLength(5)) : null,
             VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT),
             AuthenticationType.API_KEY,
             metadata

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java

@@ -38,7 +38,8 @@ public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner
         final HashMap<String, Object> metadata = new HashMap<>();
         metadata.put(AuthenticationField.API_KEY_ID_KEY, apiKeyId);
         metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomAlphaOfLengthBetween(1, 16));
-        final Authentication authentication = createMockAuthentication("joe", "_es_api_key", AuthenticationType.API_KEY, metadata);
+        final User userJoe = new User("joe");
+        final Authentication authentication = createMockAuthentication(userJoe, "_es_api_key", AuthenticationType.API_KEY, metadata);
         final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
         final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
 
@@ -55,7 +56,8 @@ public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOw
         final HashMap<String, Object> metadata = new HashMap<>();
         metadata.put(AuthenticationField.API_KEY_ID_KEY, randomAlphaOfLength(7));
         metadata.put(AuthenticationField.API_KEY_NAME_KEY, randomBoolean() ? null : randomAlphaOfLengthBetween(1, 16));
-        final Authentication authentication = createMockAuthentication("joe", "_es_api_key", AuthenticationType.API_KEY, metadata);
+        final User userJoe = new User("joe");
+        final Authentication authentication = createMockAuthentication(userJoe, "_es_api_key", AuthenticationType.API_KEY, metadata);
         final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
         final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
 
@@ -67,7 +69,14 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner()
         final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder())
             .build();
 
-        final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of());
+        final boolean isRunAs = randomBoolean();
+        final User userJoe = new User("joe");
+        final Authentication authentication = createMockAuthentication(
+            isRunAs ? new User(userJoe, new User("not-joe")) : userJoe,
+            "realm1",
+            isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM,
+            Map.of()
+        );
         final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe");
         final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe");
 
@@ -80,7 +89,14 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner_W
         final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder())
             .build();
 
-        final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of());
+        final boolean isRunAs = randomBoolean();
+        final User userJoe = new User("joe");
+        final Authentication authentication = createMockAuthentication(
+            isRunAs ? new User(userJoe, new User("not-joe")) : userJoe,
+            "realm1",
+            isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM,
+            Map.of()
+        );
         final TransportRequest getApiKeyRequest = GetApiKeyRequest.forOwnedApiKeys();
         final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.forOwnedApiKeys();
 
@@ -93,7 +109,14 @@ public void testAuthenticationWithUserDeniesAccessToApiKeyActionsWhenItIsNotOwne
         final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder())
             .build();
 
-        final Authentication authentication = createMockAuthentication("joe", "realm1", AuthenticationType.REALM, Map.of());
+        final boolean isRunAs = randomBoolean();
+        final User userJoe = new User("joe");
+        final Authentication authentication = createMockAuthentication(
+            isRunAs ? new User(userJoe, new User("not-joe")) : userJoe,
+            "realm1",
+            isRunAs ? randomFrom(AuthenticationType.REALM, AuthenticationType.API_KEY) : AuthenticationType.REALM,
+            Map.of()
+        );
         final TransportRequest getApiKeyRequest = randomFrom(
             GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)),
             GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"),
@@ -153,19 +176,20 @@ public void testCheckQueryApiKeyRequest() {
     }
 
     private Authentication createMockAuthentication(
-        String username,
+        User user,
         String realmName,
         AuthenticationType authenticationType,
         Map<String, Object> metadata
     ) {
-        final User user = new User(username);
         final Authentication authentication = mock(Authentication.class);
         final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
         when(authentication.getUser()).thenReturn(user);
         when(authentication.getSourceRealm()).thenReturn(authenticatedBy);
         when(authentication.getAuthenticationType()).thenReturn(authenticationType);
         when(authenticatedBy.getName()).thenReturn(realmName);
         when(authentication.getMetadata()).thenReturn(metadata);
+        when(authentication.isAuthenticatedWithApiKey()).thenCallRealMethod();
+        when(authentication.isApiKey()).thenCallRealMethod();
         return authentication;
     }
 

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

@@ -89,6 +89,7 @@
 import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_7;
 import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS;
 import static org.elasticsearch.xpack.security.Security.SECURITY_CRYPTO_THREAD_POOL_NAME;
+import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -1179,6 +1180,36 @@ public void testDerivedKeys() throws ExecutionException, InterruptedException {
         assertApiKeyNotCreated(client, "key-5");
     }
 
+    public void testApiKeyRunAsAnotherUserCanCreateApiKey() {
+        final RoleDescriptor descriptor = new RoleDescriptor("role", Strings.EMPTY_ARRAY, null, new String[] { ES_TEST_ROOT_USER });
+        Client client = client().filterWithHeader(
+            Map.of("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING))
+        );
+        final CreateApiKeyResponse response1 = new CreateApiKeyRequestBuilder(client).setName("run-as-key")
+            .setRoleDescriptors(List.of(descriptor))
+            .setMetadata(ApiKeyTests.randomMetadata())
+            .get();
+
+        final String base64ApiKeyKeyValue = Base64.getEncoder()
+            .encodeToString((response1.getId() + ":" + response1.getKey()).getBytes(StandardCharsets.UTF_8));
+
+        final CreateApiKeyResponse response2 = new CreateApiKeyRequestBuilder(
+            client().filterWithHeader(
+                Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue, "es-security-runas-user", ES_TEST_ROOT_USER)
+            )
+        ).setName("create-by run-as user").setRoleDescriptors(List.of(new RoleDescriptor("a", new String[] { "all" }, null, null))).get();
+
+        final GetApiKeyResponse getApiKeyResponse = client.execute(
+            GetApiKeyAction.INSTANCE,
+            GetApiKeyRequest.usingApiKeyId(response2.getId(), true)
+        ).actionGet();
+        assertThat(getApiKeyResponse.getApiKeyInfos(), arrayWithSize(1));
+        final ApiKey apiKeyInfo = getApiKeyResponse.getApiKeyInfos()[0];
+        assertThat(apiKeyInfo.getId(), equalTo(response2.getId()));
+        assertThat(apiKeyInfo.getUsername(), equalTo(ES_TEST_ROOT_USER));
+        assertThat(apiKeyInfo.getRealm(), equalTo("file"));
+    }
+
     public void testCreationAndAuthenticationReturns429WhenThreadPoolIsSaturated() throws Exception {
         final String nodeName = randomFrom(internalCluster().getNodeNames());
         final Settings settings = internalCluster().getInstance(Settings.class, nodeName);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportCreateApiKeyAction.java

@@ -52,7 +52,7 @@ protected void doExecute(Task task, CreateApiKeyRequest request, ActionListener<
         if (authentication == null) {
             listener.onFailure(new IllegalStateException("authentication is required"));
         } else {
-            if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType() && grantsAnyPrivileges(request)) {
+            if (authentication.isApiKey() && grantsAnyPrivileges(request)) {
                 listener.onFailure(
                     new IllegalArgumentException(
                         "creating derived api keys requires an explicit role descriptor that is empty (has no privileges)"

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java

@@ -72,7 +72,7 @@ protected void doExecute(Task task, CreateTokenRequest request, ActionListener<C
             case PASSWORD, KERBEROS -> authenticateAndCreateToken(type, request, listener);
             case CLIENT_CREDENTIALS -> {
                 Authentication authentication = securityContext.getAuthentication();
-                if (authentication.isAuthenticatedWithServiceAccount() && false == authentication.getUser().isRunAs()) {
+                if (authentication.isServiceAccount()) {
                     // Service account itself cannot create OAuth2 tokens.
                     // But it is possible to create an oauth2 token if the service account run-as a different user.
                     // In this case, the token will be created for the run-as user (not the service account).

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateAction.java

@@ -55,7 +55,8 @@ protected void doExecute(Task task, AuthenticateRequest request, ActionListener<
             final User user = authentication.getUser();
             final boolean shouldAddAnonymousRoleNames = anonymousUser.enabled()
                 && false == anonymousUser.equals(user)
-                && authentication.getAuthenticationType() != Authentication.AuthenticationType.API_KEY;
+                && false == authentication.isApiKey()
+                && false == authentication.isServiceAccount();
             if (shouldAddAnonymousRoleNames) {
                 final String[] allRoleNames = Stream.concat(Stream.of(user.roles()), Stream.of(anonymousUser.roles()))
                     .toArray(String[]::new);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java

@@ -1492,7 +1492,7 @@ LogEntryBuilder withXForwardedFor(ThreadContext threadContext) {
         LogEntryBuilder withAuthentication(Authentication authentication) {
             logEntry.with(PRINCIPAL_FIELD_NAME, authentication.getUser().principal());
             logEntry.with(AUTHENTICATION_TYPE_FIELD_NAME, authentication.getAuthenticationType().toString());
-            if (Authentication.AuthenticationType.API_KEY == authentication.getAuthenticationType()) {
+            if (authentication.isAuthenticatedWithApiKey()) {
                 logEntry.with(API_KEY_ID_FIELD_NAME, (String) authentication.getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
                 String apiKeyName = (String) authentication.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY);
                 if (apiKeyName != null) {