From 5e953dc7c3881690c61ef39bbef0ae88af35647b Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Tue, 28 Jan 2025 12:00:30 +0100 Subject: [PATCH] Fix NPE on disabled API key auth cache (#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. --- docs/changelog/120483.yaml | 5 +++ .../xpack/security/authc/ApiKeyService.java | 16 ++++--- .../security/authc/ApiKeyServiceTests.java | 44 +++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 docs/changelog/120483.yaml diff --git a/docs/changelog/120483.yaml b/docs/changelog/120483.yaml new file mode 100644 index 0000000000000..20da3b9ab4e8d --- /dev/null +++ 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/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index c1be25b27c51e..c2d1370c2cbf3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -266,7 +266,9 @@ public void invalidate(Collection keys) { if (apiKeyDocCache != null) { apiKeyDocCache.invalidate(keys); } - keys.forEach(apiKeyAuthCache::invalidate); + if (apiKeyAuthCache != null) { + keys.forEach(apiKeyAuthCache::invalidate); + } } @Override @@ -274,7 +276,9 @@ 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 listenableFuture = new ListenableFuture<>(); - listenableFuture.onResponse(new CachedApiKeyHashResult(true, apiKey)); - apiKeyAuthCache.put(request.getId(), listenableFuture); + if (apiKeyAuthCache != null) { + final ListenableFuture 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/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 996291c52c71f..185669a6a203b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/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 listener = (ActionListener) 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 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();