Fix NPE on disabled API key auth cache (elastic#120483)

Currently, when `xpack.security.authc.api_key.cache.ttl` is set to `0d` API key creation (and invalidation) fail with NPEs. This PR adds null checks to fix this.
carlosdelest · Jan 28, 2025 · 5e953dc · 5e953dc
1 parent 7322015
commit 5e953dc
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 5 deletions.
diff --git a/docs/changelog/120483.yaml b/docs/changelog/120483.yaml
@@ -0,0 +1,5 @@
+pr: 120483
+summary: Fix NPE on disabled API auth key cache
+area: Authentication
+type: bug
+issues: []
diff --git a/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
@@ -266,15 +266,19 @@ public void invalidate(Collection<String> keys) {
                     if (apiKeyDocCache != null) {
                         apiKeyDocCache.invalidate(keys);
                     }
-                    keys.forEach(apiKeyAuthCache::invalidate);
+                    if (apiKeyAuthCache != null) {
+                        keys.forEach(apiKeyAuthCache::invalidate);
+                    }
                 }
 
                 @Override
                 public void invalidateAll() {
                     if (apiKeyDocCache != null) {
                         apiKeyDocCache.invalidateAll();
                     }
-                    apiKeyAuthCache.invalidateAll();
+                    if (apiKeyAuthCache != null) {
+                        apiKeyAuthCache.invalidateAll();
+                    }
                 }
             });
             cacheInvalidatorRegistry.registerCacheInvalidator("api_key_doc", new CacheInvalidatorRegistry.CacheInvalidator() {
@@ -589,9 +593,11 @@ private void createApiKeyAndIndexIt(
                                     + "])";
                             assert indexResponse.getResult() == DocWriteResponse.Result.CREATED
                                 : "Index response was [" + indexResponse.getResult() + "]";
-                            final ListenableFuture<CachedApiKeyHashResult> listenableFuture = new ListenableFuture<>();
-                            listenableFuture.onResponse(new CachedApiKeyHashResult(true, apiKey));
-                            apiKeyAuthCache.put(request.getId(), listenableFuture);
+                            if (apiKeyAuthCache != null) {
+                                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))
                     )

diff --git a/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java
@@ -2367,6 +2367,50 @@ public void testWillInvalidateAuthCacheWhenDocNotFound() {
         assertNull(service.getApiKeyAuthCache().get(docId));
     }
 
+    public void testCanCreateApiKeyWithAuthCacheDisabled() {
+        final ApiKeyService service = createApiKeyService(
+            Settings.builder()
+                .put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true)
+                .put("xpack.security.authc.api_key.cache.ttl", "0s")
+                .build()
+        );
+        final Authentication authentication = AuthenticationTestHelper.builder()
+            .user(new User(randomAlphaOfLengthBetween(8, 16), "superuser"))
+            .realmRef(new RealmRef(randomAlphaOfLengthBetween(3, 8), randomAlphaOfLengthBetween(3, 8), randomAlphaOfLengthBetween(3, 8)))
+            .build(false);
+        final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest(randomAlphaOfLengthBetween(3, 8), null, null);
+        when(client.prepareIndex(anyString())).thenReturn(new IndexRequestBuilder(client));
+        when(client.prepareBulk()).thenReturn(new BulkRequestBuilder(client));
+        when(client.threadPool()).thenReturn(threadPool);
+        doAnswer(inv -> {
+            final Object[] args = inv.getArguments();
+            @SuppressWarnings("unchecked")
+            final ActionListener<BulkResponse> listener = (ActionListener<BulkResponse>) args[2];
+            final IndexResponse indexResponse = new IndexResponse(
+                new ShardId(INTERNAL_SECURITY_MAIN_INDEX_7, randomAlphaOfLength(22), randomIntBetween(0, 1)),
+                createApiKeyRequest.getId(),
+                randomLongBetween(1, 99),
+                randomLongBetween(1, 99),
+                randomIntBetween(1, 99),
+                true
+            );
+            listener.onResponse(
+                new BulkResponse(
+                    new BulkItemResponse[] { BulkItemResponse.success(randomInt(), DocWriteRequest.OpType.INDEX, indexResponse) },
+                    randomLongBetween(0, 100)
+                )
+            );
+            return null;
+        }).when(client).execute(eq(TransportBulkAction.TYPE), any(BulkRequest.class), any());
+
+        assertThat(service.getFromCache(createApiKeyRequest.getId()), is(nullValue()));
+        final PlainActionFuture<CreateApiKeyResponse> listener = new PlainActionFuture<>();
+        service.createApiKey(authentication, createApiKeyRequest, Set.of(), listener);
+        final CreateApiKeyResponse createApiKeyResponse = listener.actionGet();
+        assertThat(createApiKeyResponse.getId(), equalTo(createApiKeyRequest.getId()));
+        assertThat(service.getFromCache(createApiKeyResponse.getId()), is(nullValue()));
+    }
+
     public void testGetCreatorRealm() {
         final User user = AuthenticationTests.randomUser();