Remove deprecated code from `ApiKeyService` (#89380)

Small refactor to remove use of deprecated functionality in
ApiKeyService.

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

@@ -23,11 +23,9 @@ import org.elasticsearch.action.bulk.BulkItemResponse;
 import org.elasticsearch.action.bulk.BulkRequest;
 import org.elasticsearch.action.bulk.BulkRequestBuilder;
 import org.elasticsearch.action.bulk.BulkResponse;
-import org.elasticsearch.action.bulk.TransportSingleItemBulkWriteAction;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.action.index.IndexRequest;
-import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.search.SearchAction;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
@@ -69,7 +67,6 @@ import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.xcontent.DeprecationHandler;
 import org.elasticsearch.xcontent.InstantiatingObjectParser;
-import org.elasticsearch.xcontent.NamedXContentRegistry;
 import org.elasticsearch.xcontent.ParseField;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentFactory;
@@ -95,6 +92,7 @@ import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
+import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.RealmDomain;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
@@ -134,7 +132,6 @@ import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-import static org.elasticsearch.action.bulk.TransportSingleItemBulkWriteAction.toSingleItemBulkRequest;
 import static org.elasticsearch.core.Strings.format;
 import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
 import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
@@ -322,14 +319,16 @@ public class ApiKeyService {
                     request.getMetadata()
                 )
             ) {
-
-                final IndexRequest indexRequest = client.prepareIndex(SECURITY_MAIN_ALIAS)
-                    .setSource(builder)
-                    .setId(request.getId())
-                    .setOpType(DocWriteRequest.OpType.CREATE)
-                    .setRefreshPolicy(request.getRefreshPolicy())
-                    .request();
-                final BulkRequest bulkRequest = toSingleItemBulkRequest(indexRequest);
+                final BulkRequestBuilder bulkRequestBuilder = client.prepareBulk();
+                bulkRequestBuilder.add(
+                    client.prepareIndex(SECURITY_MAIN_ALIAS)
+                        .setSource(builder)
+                        .setId(request.getId())
+                        .setOpType(DocWriteRequest.OpType.CREATE)
+                        .request()
+                );
+                bulkRequestBuilder.setRefreshPolicy(request.getRefreshPolicy());
+                final BulkRequest bulkRequest = bulkRequestBuilder.request();
 
                 securityIndex.prepareIndexIfNeededThenExecute(
                     listener::onFailure,
@@ -338,14 +337,20 @@ public class ApiKeyService {
                         SECURITY_ORIGIN,
                         BulkAction.INSTANCE,
                         bulkRequest,
-                        TransportSingleItemBulkWriteAction.<IndexResponse>wrapBulkResponse(ActionListener.wrap(indexResponse -> {
-                            assert request.getId().equals(indexResponse.getId());
-                            assert indexResponse.getResult() == DocWriteResponse.Result.CREATED;
-                            final ListenableFuture<CachedApiKeyHashResult> listenableFuture = new ListenableFuture<>();
-                            listenableFuture.onResponse(new CachedApiKeyHashResult(true, apiKey));
-                            apiKeyAuthCache.put(request.getId(), listenableFuture);
-                            listener.onResponse(new CreateApiKeyResponse(request.getName(), request.getId(), apiKey, expiration));
-                        }, listener::onFailure))
+                        ActionListener.wrap(bulkResponse -> {
+                            assert bulkResponse.getItems().length == 1;
+                            final BulkItemResponse indexResponse = bulkResponse.getItems()[0];
+                            if (indexResponse.isFailed()) {
+                                listener.onFailure(indexResponse.getFailure().getCause());
+                            } else {
+                                assert request.getId().equals(indexResponse.getResponse().getId());
+                                assert indexResponse.getResponse().getResult() == DocWriteResponse.Result.CREATED;
+                                final ListenableFuture<CachedApiKeyHashResult> listenableFuture = new ListenableFuture<>();
+                                listenableFuture.onResponse(new CachedApiKeyHashResult(true, apiKey));
+                                apiKeyAuthCache.put(request.getId(), listenableFuture);
+                                listener.onResponse(new CreateApiKeyResponse(request.getName(), request.getId(), apiKey, expiration));
+                            }
+                        }, listener::onFailure)
                     )
                 );
             } catch (IOException e) {
@@ -683,8 +688,7 @@ public class ApiKeyService {
                 final ApiKeyDoc apiKeyDoc;
                 try (
                     XContentParser parser = XContentHelper.createParser(
-                        NamedXContentRegistry.EMPTY,
-                        LoggingDeprecationHandler.INSTANCE,
+                        XContentParserConfiguration.EMPTY.withDeprecationHandler(LoggingDeprecationHandler.INSTANCE),
                         response.getSourceAsBytesRef(),
                         XContentType.JSON
                     )
@@ -729,8 +733,9 @@ public class ApiKeyService {
                 try (
                     XContentParser parser = XContentType.JSON.xContent()
                         .createParser(
-                            NamedXContentRegistry.EMPTY,
-                            new ApiKeyLoggingDeprecationHandler(deprecationLogger, apiKeyId),
+                            XContentParserConfiguration.EMPTY.withDeprecationHandler(
+                                new ApiKeyLoggingDeprecationHandler(deprecationLogger, apiKeyId)
+                            ),
                             BytesReference.bytes(builder).streamInput()
                         )
                 ) {
@@ -765,8 +770,7 @@ public class ApiKeyService {
         List<RoleDescriptor> roleDescriptors = new ArrayList<>();
         try (
             XContentParser parser = XContentHelper.createParser(
-                NamedXContentRegistry.EMPTY,
-                new ApiKeyLoggingDeprecationHandler(deprecationLogger, apiKeyId),
+                XContentParserConfiguration.EMPTY.withDeprecationHandler(new ApiKeyLoggingDeprecationHandler(deprecationLogger, apiKeyId)),
                 bytesReference,
                 XContentType.JSON
             )
@@ -1225,17 +1229,13 @@ public class ApiKeyService {
                         );
                         invalidateListener.onResponse(InvalidateApiKeyResponse.emptyResponse());
                     } else {
-                        invalidateAllApiKeys(apiKeys.stream().map(ApiKey::getId).collect(Collectors.toSet()), invalidateListener);
+                        indexInvalidation(apiKeys.stream().map(ApiKey::getId).collect(Collectors.toSet()), invalidateListener);
                     }
                 }, invalidateListener::onFailure)
             );
         }
     }
 
-    private void invalidateAllApiKeys(Collection<String> apiKeyIds, ActionListener<InvalidateApiKeyResponse> invalidateListener) {
-        indexInvalidation(apiKeyIds, invalidateListener, null);
-    }
-
     private <T> void findApiKeys(
         final BoolQueryBuilder boolQuery,
         boolean filterOutInvalidatedKeys,
@@ -1344,16 +1344,10 @@ public class ApiKeyService {
     /**
      * Performs the actual invalidation of a collection of api keys
      *
-     * @param apiKeyIds       the api keys to invalidate
-     * @param listener        the listener to notify upon completion
-     * @param previousResult  if this not the initial attempt for invalidation, it contains the result of invalidating
-     *                        api keys up to the point of the retry. This result is added to the result of the current attempt
+     * @param apiKeyIds the api keys to invalidate
+     * @param listener  the listener to notify upon completion
      */
-    private void indexInvalidation(
-        Collection<String> apiKeyIds,
-        ActionListener<InvalidateApiKeyResponse> listener,
-        @Nullable InvalidateApiKeyResponse previousResult
-    ) {
+    private void indexInvalidation(Collection<String> apiKeyIds, ActionListener<InvalidateApiKeyResponse> listener) {
         maybeStartApiKeyRemover();
         if (apiKeyIds.isEmpty()) {
             listener.onFailure(new ElasticsearchSecurityException("No api key ids provided for invalidation"));
@@ -1376,11 +1370,6 @@ public class ApiKeyService {
                         ArrayList<ElasticsearchException> failedRequestResponses = new ArrayList<>();
                         ArrayList<String> previouslyInvalidated = new ArrayList<>();
                         ArrayList<String> invalidated = new ArrayList<>();
-                        if (null != previousResult) {
-                            failedRequestResponses.addAll((previousResult.getErrors()));
-                            previouslyInvalidated.addAll(previousResult.getPreviouslyInvalidatedApiKeys());
-                            invalidated.addAll(previousResult.getInvalidatedApiKeys());
-                        }
                         for (BulkItemResponse bulkItemResponse : bulkResponse.getItems()) {
                             if (bulkItemResponse.isFailed()) {
                                 Throwable cause = bulkItemResponse.getFailure().getCause();
@@ -1744,23 +1733,36 @@ public class ApiKeyService {
      */
     public static String getCreatorRealmName(final Authentication authentication) {
         if (authentication.isApiKey()) {
-            return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
+            return (String) authentication.getEffectiveSubject().getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME);
         } else {
+            // TODO we should use the effective subject realm here but need to handle the failed lookup scenario, in which the realm may be
+            // `null`. Since this method is used in audit logging, this requires some care.
             return authentication.getSourceRealm().getName();
         }
     }
 
-    /** Returns the realm names that the username can access resources across.
+    /**
+     * Returns the realm names that the username can access resources across.
      */
-    public static String[] getOwnersRealmNames(Authentication authentication) {
+    public static String[] getOwnersRealmNames(final Authentication authentication) {
         if (authentication.isApiKey()) {
-            return new String[] { (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME) };
+            return new String[] {
+                (String) authentication.getEffectiveSubject().getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_NAME) };
         } else {
-            RealmDomain domain = authentication.getSourceRealm().getDomain();
+            final Authentication.RealmRef effectiveSubjectRealm = authentication.getEffectiveSubject().getRealm();
+            // The effective subject realm can only be `null` when run-as lookup fails. The owner is always the effective subject, so there
+            // is no owner information to return here
+            if (effectiveSubjectRealm == null) {
+                final var message =
+                    "Cannot determine owner realms without an effective subject realm for non-API key authentication object";
+                assert false : message;
+                throw new IllegalArgumentException(message);
+            }
+            final RealmDomain domain = effectiveSubjectRealm.getDomain();
             if (domain != null) {
-                return domain.realms().stream().map(realmIdentifier -> realmIdentifier.getName()).toArray(String[]::new);
+                return domain.realms().stream().map(RealmConfig.RealmIdentifier::getName).toArray(String[]::new);
             } else {
-                return new String[] { authentication.getSourceRealm().getName() };
+                return new String[] { effectiveSubjectRealm.getName() };
             }
         }
     }
@@ -1774,8 +1776,10 @@ public class ApiKeyService {
      */
     public static String getCreatorRealmType(final Authentication authentication) {
         if (authentication.isApiKey()) {
-            return (String) authentication.getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
+            return (String) authentication.getEffectiveSubject().getMetadata().get(AuthenticationField.API_KEY_CREATOR_REALM_TYPE);
         } else {
+            // TODO we should use the effective subject realm here but need to handle the failed lookup scenario, in which the realm may be
+            // `null`. Since this method is used in audit logging, this requires some care.
             return authentication.getSourceRealm().getType();
         }
     }
@@ -1792,11 +1796,11 @@ public class ApiKeyService {
                 "authentication realm must be ["
                     + AuthenticationField.API_KEY_REALM_TYPE
                     + "], got ["
-                    + authentication.getAuthenticatedBy().getType()
+                    + authentication.getEffectiveSubject().getRealm().getType()
                     + "]"
             );
         }
-        final Object apiKeyMetadata = authentication.getMetadata().get(AuthenticationField.API_KEY_METADATA_KEY);
+        final Object apiKeyMetadata = authentication.getEffectiveSubject().getMetadata().get(AuthenticationField.API_KEY_METADATA_KEY);
         if (apiKeyMetadata != null) {
             final Tuple<XContentType, Map<String, Object>> tuple = XContentHelper.convertToMap(
                 (BytesReference) apiKeyMetadata,

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

@@ -17,6 +17,7 @@ import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.bulk.BulkAction;
 import org.elasticsearch.action.bulk.BulkItemResponse;
 import org.elasticsearch.action.bulk.BulkRequest;
+import org.elasticsearch.action.bulk.BulkRequestBuilder;
 import org.elasticsearch.action.bulk.BulkResponse;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.action.index.IndexAction;
@@ -205,6 +206,7 @@ public class ApiKeyServiceTests extends ESTestCase {
             .build(false);
         final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest("key-1", null, null);
         when(client.prepareIndex(anyString())).thenReturn(new IndexRequestBuilder(client, IndexAction.INSTANCE));
+        when(client.prepareBulk()).thenReturn(new BulkRequestBuilder(client, BulkAction.INSTANCE));
         when(client.threadPool()).thenReturn(threadPool);
         final AtomicBoolean bulkActionInvoked = new AtomicBoolean(false);
         doAnswer(inv -> {
@@ -356,6 +358,7 @@ public class ApiKeyServiceTests extends ESTestCase {
             .build(false);
         final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest(randomAlphaOfLengthBetween(3, 8), null, null);
         when(client.prepareIndex(anyString())).thenReturn(new IndexRequestBuilder(client, IndexAction.INSTANCE));
+        when(client.prepareBulk()).thenReturn(new BulkRequestBuilder(client, BulkAction.INSTANCE));
         when(client.threadPool()).thenReturn(threadPool);
         doAnswer(inv -> {
             final Object[] args = inv.getArguments();