Configurable retention period for inactive API keys (#92219)

Inactive API keys, either expired or actively invalidated, are removed
from the security index eventually. However expired and invalidated keys
have different retention periods. It is hard-coded to be 7 days for
expired keys and Zero for invalidated keys.

This PR makes the retention period consistent for both expired and
invalidated keys. It also exposes a new setting so that the retention
period is dyanmically configurable (defaults to 7 days).

Relates: #91738

docs/changelog/92219.yaml

@@ -0,0 +1,5 @@
+pr: 92219
+summary: Configurable retention period for invalidated or expired API keys
+area: Security
+type: enhancement
+issues: []

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

@@ -96,6 +96,7 @@ import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.transport.filter.IPFilter;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -149,6 +150,7 @@ import static org.hamcrest.Matchers.in;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
 public class ApiKeyIntegTests extends SecurityIntegTestCase {
@@ -162,6 +164,13 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         null
     );
 
+    private static long deleteRetentionPeriodDays;
+
+    @BeforeClass
+    public static void randomDeleteRetentionPeriod() {
+        deleteRetentionPeriodDays = randomLongBetween(0, 7);
+    }
+
     @Override
     public Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
         return Settings.builder()
@@ -170,6 +179,7 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
             .put(XPackSettings.TOKEN_SERVICE_ENABLED_SETTING.getKey(), true)
             .put(ApiKeyService.DELETE_INTERVAL.getKey(), TimeValue.timeValueMillis(DELETE_INTERVAL_MILLIS))
             .put(ApiKeyService.DELETE_TIMEOUT.getKey(), TimeValue.timeValueSeconds(5L))
+            .put(ApiKeyService.DELETE_RETENTION_PERIOD.getKey(), TimeValue.timeValueDays(deleteRetentionPeriodDays))
             .put("xpack.security.crypto.thread_pool.queue_size", CRYPTO_THREAD_POOL_QUEUE_SIZE)
             .build();
     }
@@ -459,12 +469,48 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         assertThat(invalidateResponse.getErrors().size(), equalTo(0));
     }
 
-    public void testInvalidatedApiKeysDeletedByRemover() throws Exception {
+    public void testApiKeyRemover() throws Exception {
+        final String namePrefix = randomAlphaOfLength(10);
+        try {
+            if (deleteRetentionPeriodDays == 0) {
+                doTestInvalidKeysImmediatelyDeletedByRemover(namePrefix);
+                // Change the setting dynamically and test the other behaviour
+                deleteRetentionPeriodDays = randomIntBetween(1, 7);
+                setRetentionPeriod(false);
+                doTestDeletionBehaviorWhenKeysBecomeInvalidBeforeAndAfterRetentionPeriod("not-" + namePrefix);
+            } else {
+                doTestDeletionBehaviorWhenKeysBecomeInvalidBeforeAndAfterRetentionPeriod(namePrefix);
+                // Change the setting dynamically and test the other behaviour
+                deleteRetentionPeriodDays = 0;
+                setRetentionPeriod(false);
+                doTestInvalidKeysImmediatelyDeletedByRemover("not-" + namePrefix);
+            }
+        } finally {
+            setRetentionPeriod(true);
+        }
+    }
+
+    private void setRetentionPeriod(boolean clear) {
+        final Settings.Builder builder = Settings.builder();
+        if (clear) {
+            builder.putNull(ApiKeyService.DELETE_RETENTION_PERIOD.getKey());
+        } else {
+            builder.put(ApiKeyService.DELETE_RETENTION_PERIOD.getKey(), TimeValue.timeValueDays(deleteRetentionPeriodDays));
+        }
+        client().admin().cluster().prepareUpdateSettings().setPersistentSettings(builder).get();
+    }
+
+    private void doTestInvalidKeysImmediatelyDeletedByRemover(String namePrefix) throws Exception {
+        assertThat(deleteRetentionPeriodDays, equalTo(0L));
         Client client = waitForExpiredApiKeysRemoverTriggerReadyAndGetClient().filterWithHeader(
             Collections.singletonMap("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING))
         );
 
-        List<CreateApiKeyResponse> createdApiKeys = createApiKeys(2, null).v1();
+        // Create a very short-lived key (1ms expiration)
+        createApiKeys(1, TimeValue.timeValueMillis(1));
+        // Create keys that will not expire during this test
+        final CreateApiKeyResponse nonExpiringKey = createApiKeys(1, namePrefix, TimeValue.timeValueDays(1)).v1().get(0);
+        List<CreateApiKeyResponse> createdApiKeys = createApiKeys(2, namePrefix, randomBoolean() ? TimeValue.timeValueDays(1) : null).v1();
 
         PlainActionFuture<InvalidateApiKeyResponse> listener = new PlainActionFuture<>();
         client.execute(
@@ -481,13 +527,21 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         refreshSecurityIndex();
 
         PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
-        client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
-        Set<String> expectedKeyIds = Sets.newHashSet(createdApiKeys.get(0).getId(), createdApiKeys.get(1).getId());
+        client.execute(
+            GetApiKeyAction.INSTANCE,
+            GetApiKeyRequest.builder().apiKeyName(namePrefix + "*").build(),
+            getApiKeyResponseListener
+        );
+        // The first API key with 1ms expiration should already be deleted
+        Set<String> expectedKeyIds = Sets.newHashSet(nonExpiringKey.getId(), createdApiKeys.get(0).getId(), createdApiKeys.get(1).getId());
         boolean apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
         for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
             assertThat(apiKey.getId(), is(in(expectedKeyIds)));
-            if (apiKey.getId().equals(createdApiKeys.get(0).getId())) {
-                // has been invalidated but not yet deleted by ExpiredApiKeysRemover
+            if (apiKey.getId().equals(nonExpiringKey.getId())) {
+                assertThat(apiKey.isInvalidated(), is(false));
+                assertThat(apiKey.getExpiration(), notNullValue());
+            } else if (apiKey.getId().equals(createdApiKeys.get(0).getId())) {
+                // has been invalidated but not yet deleted by InactiveApiKeysRemover
                 assertThat(apiKey.isInvalidated(), is(true));
                 apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
             } else if (apiKey.getId().equals(createdApiKeys.get(1).getId())) {
@@ -497,7 +551,7 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         }
         assertThat(
             getApiKeyResponseListener.get().getApiKeyInfos().length,
-            is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 2 : 1)
+            is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 3 : 2)
         );
 
         client = waitForExpiredApiKeysRemoverTriggerReadyAndGetClient().filterWithHeader(
@@ -517,22 +571,29 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         refreshSecurityIndex();
 
         // Verify that 1st invalidated API key is deleted whereas the next one may be or may not be as it depends on whether update was
-        // indexed before ExpiredApiKeysRemover ran
+        // indexed before InactiveApiKeysRemover ran
         getApiKeyResponseListener = new PlainActionFuture<>();
-        client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
-        expectedKeyIds = Sets.newHashSet(createdApiKeys.get(1).getId());
+        client.execute(
+            GetApiKeyAction.INSTANCE,
+            GetApiKeyRequest.builder().apiKeyName(namePrefix + "*").build(),
+            getApiKeyResponseListener
+        );
+        expectedKeyIds = Sets.newHashSet(nonExpiringKey.getId(), createdApiKeys.get(1).getId());
         apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
         for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
             assertThat(apiKey.getId(), is(in(expectedKeyIds)));
-            if (apiKey.getId().equals(createdApiKeys.get(1).getId())) {
-                // has been invalidated but not yet deleted by ExpiredApiKeysRemover
+            if (apiKey.getId().equals(nonExpiringKey.getId())) {
+                assertThat(apiKey.isInvalidated(), is(false));
+                assertThat(apiKey.getExpiration(), notNullValue());
+            } else if (apiKey.getId().equals(createdApiKeys.get(1).getId())) {
+                // has been invalidated but not yet deleted by InactiveApiKeysRemover
                 assertThat(apiKey.isInvalidated(), is(true));
                 apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
             }
         }
         assertThat(
             getApiKeyResponseListener.get().getApiKeyInfos().length,
-            is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 1 : 0)
+            is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 2 : 1)
         );
     }
 
@@ -554,44 +615,99 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         return internalCluster().client(nodeWithMostRecentRun);
     }
 
-    public void testExpiredApiKeysBehaviorWhenKeysExpired1WeekBeforeAnd1DayBefore() throws Exception {
+    private void doTestDeletionBehaviorWhenKeysBecomeInvalidBeforeAndAfterRetentionPeriod(String namePrefix) throws Exception {
+        assertThat(deleteRetentionPeriodDays, greaterThan(0L));
         Client client = waitForExpiredApiKeysRemoverTriggerReadyAndGetClient().filterWithHeader(
             Collections.singletonMap("Authorization", basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING))
         );
 
-        int noOfKeys = 4;
-        List<CreateApiKeyResponse> createdApiKeys = createApiKeys(noOfKeys, null).v1();
+        int noOfKeys = 9;
+        List<CreateApiKeyResponse> createdApiKeys = createApiKeys(noOfKeys, namePrefix, null).v1();
         Instant created = Instant.now();
 
         PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
-        client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
+        client.execute(
+            GetApiKeyAction.INSTANCE,
+            GetApiKeyRequest.builder().apiKeyName(namePrefix + "*").build(),
+            getApiKeyResponseListener
+        );
         assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(noOfKeys));
 
         // Expire the 1st key such that it cannot be deleted by the remover
-        // hack doc to modify the expiration time to a day before
-        Instant dayBefore = created.minus(1L, ChronoUnit.DAYS);
-        assertTrue(Instant.now().isAfter(dayBefore));
+        // hack doc to modify the expiration time
+        Instant withinRetention = created.minus(deleteRetentionPeriodDays - 1, ChronoUnit.DAYS);
+        assertFalse(created.isBefore(withinRetention));
         UpdateResponse expirationDateUpdatedResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(0).getId())
-            .setDoc("expiration_time", dayBefore.toEpochMilli())
+            .setDoc("expiration_time", withinRetention.toEpochMilli())
             .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
             .get();
         assertThat(expirationDateUpdatedResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
 
         // Expire the 2nd key such that it can be deleted by the remover
-        // hack doc to modify the expiration time to the week before
-        Instant weekBefore = created.minus(8L, ChronoUnit.DAYS);
-        assertTrue(Instant.now().isAfter(weekBefore));
+        // hack doc to modify the expiration time
+        Instant outsideRetention = created.minus(deleteRetentionPeriodDays + 1, ChronoUnit.DAYS);
+        assertTrue(Instant.now().isAfter(outsideRetention));
         expirationDateUpdatedResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(1).getId())
-            .setDoc("expiration_time", weekBefore.toEpochMilli())
+            .setDoc("expiration_time", outsideRetention.toEpochMilli())
             .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
             .get();
         assertThat(expirationDateUpdatedResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
 
+        // Invalidate the 3rd key such that it cannot be deleted by the remover
+        UpdateResponse invalidateUpdateResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(2).getId())
+            .setDoc("invalidation_time", withinRetention.toEpochMilli(), "api_key_invalidated", true)
+            .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+            .get();
+        assertThat(invalidateUpdateResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
+
+        // Invalidate the 4th key such that it will be deleted by the remover
+        invalidateUpdateResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(3).getId())
+            .setDoc("invalidation_time", outsideRetention.toEpochMilli(), "api_key_invalidated", true)
+            .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+            .get();
+        assertThat(invalidateUpdateResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
+
+        // 5th key will be deleted because its expiration is outside of retention even though its invalidation time is not
+        UpdateResponse updateResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(4).getId())
+            .setDoc(
+                "expiration_time",
+                outsideRetention.toEpochMilli(),
+                "invalidation_time",
+                withinRetention.toEpochMilli(),
+                "api_key_invalidated",
+                true
+            )
+            .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+            .get();
+        assertThat(updateResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
+
+        // 6th key will be deleted because its invalidation time is outside of retention even though its expiration is not
+        updateResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(5).getId())
+            .setDoc(
+                "expiration_time",
+                withinRetention.toEpochMilli(),
+                "invalidation_time",
+                outsideRetention.toEpochMilli(),
+                "api_key_invalidated",
+                true
+            )
+            .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+            .get();
+        assertThat(updateResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
+
+        // 7th key will be deleted because it has old style invalidation (no invalidation time)
+        // It does not matter whether it has an expiration time or whether the expiration time is still within retention period
+        updateResponse = client.prepareUpdate(SECURITY_MAIN_ALIAS, createdApiKeys.get(6).getId())
+            .setDoc("api_key_invalidated", true, "expiration_time", randomBoolean() ? withinRetention.toEpochMilli() : null)
+            .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+            .get();
+        assertThat(updateResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
+
         // Invalidate to trigger the remover
         PlainActionFuture<InvalidateApiKeyResponse> listener = new PlainActionFuture<>();
         client.execute(
             InvalidateApiKeyAction.INSTANCE,
-            InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(2).getId(), false),
+            InvalidateApiKeyRequest.usingApiKeyId(createdApiKeys.get(7).getId(), false),
             listener
         );
         assertThat(listener.get().getInvalidatedApiKeys().size(), is(1));
@@ -600,16 +716,20 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
 
         refreshSecurityIndex();
 
-        // Verify get API keys does not return api keys deleted by ExpiredApiKeysRemover
+        // Verify get API keys does not return api keys deleted by InactiveApiKeysRemover
         getApiKeyResponseListener = new PlainActionFuture<>();
-        client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
+        client.execute(
+            GetApiKeyAction.INSTANCE,
+            GetApiKeyRequest.builder().apiKeyName(namePrefix + "*").build(),
+            getApiKeyResponseListener
+        );
 
         Set<String> expectedKeyIds = Sets.newHashSet(
             createdApiKeys.get(0).getId(),
             createdApiKeys.get(2).getId(),
-            createdApiKeys.get(3).getId()
+            createdApiKeys.get(7).getId(),
+            createdApiKeys.get(8).getId()
         );
-        boolean apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
         for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
             assertThat(apiKey.getId(), is(in(expectedKeyIds)));
             if (apiKey.getId().equals(createdApiKeys.get(0).getId())) {
@@ -617,11 +737,14 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
                 assertTrue(apiKey.getExpiration().isBefore(Instant.now()));
                 assertThat(apiKey.isInvalidated(), is(false));
             } else if (apiKey.getId().equals(createdApiKeys.get(2).getId())) {
-                // has not been expired as no expiration, is invalidated but not yet deleted by ExpiredApiKeysRemover
+                // has been invalidated, not expired
+                assertThat(apiKey.getExpiration(), nullValue());
+                assertThat(apiKey.isInvalidated(), is(true));
+            } else if (apiKey.getId().equals(createdApiKeys.get(7).getId())) {
+                // has not been expired as no expiration, is invalidated but not yet deleted by InactiveApiKeysRemover
                 assertThat(apiKey.getExpiration(), is(nullValue()));
                 assertThat(apiKey.isInvalidated(), is(true));
-                apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
-            } else if (apiKey.getId().equals(createdApiKeys.get(3).getId())) {
+            } else if (apiKey.getId().equals(createdApiKeys.get(8).getId())) {
                 // has not been expired as no expiration, not invalidated
                 assertThat(apiKey.getExpiration(), is(nullValue()));
                 assertThat(apiKey.isInvalidated(), is(false));
@@ -629,10 +752,7 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
                 fail("unexpected API key " + apiKey);
             }
         }
-        assertThat(
-            getApiKeyResponseListener.get().getApiKeyInfos().length,
-            is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 3 : 2)
-        );
+        assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(4));
     }
 
     private void refreshSecurityIndex() throws Exception {
@@ -2894,6 +3014,18 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
         return createApiKeys(ES_TEST_ROOT_USER, noOfApiKeys, expiration, DEFAULT_API_KEY_ROLE_DESCRIPTOR.getClusterPrivileges());
     }
 
+    private Tuple<List<CreateApiKeyResponse>, List<Map<String, Object>>> createApiKeys(
+        int noOfApiKeys,
+        String namePrefix,
+        TimeValue expiration
+    ) {
+        final Map<String, String> headers = Collections.singletonMap(
+            "Authorization",
+            basicAuthHeaderValue(ES_TEST_ROOT_USER, TEST_PASSWORD_SECURE_STRING)
+        );
+        return createApiKeys(headers, noOfApiKeys, namePrefix, expiration, DEFAULT_API_KEY_ROLE_DESCRIPTOR.getClusterPrivileges());
+    }
+
     private Tuple<List<CreateApiKeyResponse>, List<Map<String, Object>>> createApiKeys(
         String user,
         int noOfApiKeys,

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

@@ -1108,6 +1108,7 @@ public class Security extends Plugin
         settingsList.add(ApiKeyService.PASSWORD_HASHING_ALGORITHM);
         settingsList.add(ApiKeyService.DELETE_TIMEOUT);
         settingsList.add(ApiKeyService.DELETE_INTERVAL);
+        settingsList.add(ApiKeyService.DELETE_RETENTION_PERIOD);
         settingsList.add(ApiKeyService.CACHE_HASH_ALGO_SETTING);
         settingsList.add(ApiKeyService.CACHE_MAX_KEYS_SETTING);
         settingsList.add(ApiKeyService.CACHE_TTL_SETTING);

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

@@ -163,6 +163,12 @@ public class ApiKeyService {
         TimeValue.timeValueHours(24L),
         Property.NodeScope
     );
+    public static final Setting<TimeValue> DELETE_RETENTION_PERIOD = Setting.positiveTimeSetting(
+        "xpack.security.authc.api_key.delete.retention_period",
+        TimeValue.timeValueDays(7),
+        Property.NodeScope,
+        Property.Dynamic
+    );
     public static final Setting<String> CACHE_HASH_ALGO_SETTING = Setting.simpleString(
         "xpack.security.authc.api_key.cache.hash_algo",
         "ssha256",
@@ -193,7 +199,7 @@ public class ApiKeyService {
     private final Hasher hasher;
     private final boolean enabled;
     private final Settings settings;
-    private final ExpiredApiKeysRemover expiredApiKeysRemover;
+    private final InactiveApiKeysRemover inactiveApiKeysRemover;
     private final TimeValue deleteInterval;
     private final Cache<String, ListenableFuture<CachedApiKeyHashResult>> apiKeyAuthCache;
     private final Hasher cacheHasher;
@@ -225,7 +231,7 @@ public class ApiKeyService {
         this.hasher = Hasher.resolve(PASSWORD_HASHING_ALGORITHM.get(settings));
         this.settings = settings;
         this.deleteInterval = DELETE_INTERVAL.get(settings);
-        this.expiredApiKeysRemover = new ExpiredApiKeysRemover(settings, client);
+        this.inactiveApiKeysRemover = new InactiveApiKeysRemover(settings, client, clusterService);
         this.threadPool = threadPool;
         this.cacheHasher = Hasher.resolve(CACHE_HASH_ALGO_SETTING.get(settings));
         final TimeValue ttl = CACHE_TTL_SETTING.get(settings);
@@ -1554,7 +1560,7 @@ public class ApiKeyService {
 
     // pkg scoped for testing
     boolean isExpirationInProgress() {
-        return expiredApiKeysRemover.isExpirationInProgress();
+        return inactiveApiKeysRemover.isExpirationInProgress();
     }
 
     // pkg scoped for testing
@@ -1565,7 +1571,7 @@ public class ApiKeyService {
     private void maybeStartApiKeyRemover() {
         if (securityIndex.isAvailable()) {
             if (client.threadPool().relativeTimeInMillis() - lastExpirationRunMs > deleteInterval.getMillis()) {
-                expiredApiKeysRemover.submit(client.threadPool());
+                inactiveApiKeysRemover.submit(client.threadPool());
                 lastExpirationRunMs = client.threadPool().relativeTimeInMillis();
             }
         }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredApiKeysRemover.java → x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/InactiveApiKeysRemover.java

@@ -12,6 +12,7 @@ import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.bulk.BulkItemResponse;
 import org.elasticsearch.client.internal.Client;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.core.TimeValue;
@@ -24,9 +25,9 @@ import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.threadpool.ThreadPool.Names;
 import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
 
-import java.time.Duration;
 import java.time.Instant;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
 
 import static org.elasticsearch.action.support.TransportActions.isShardNotAvailableException;
 import static org.elasticsearch.core.Strings.format;
@@ -36,18 +37,23 @@ import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
 /**
  * Responsible for cleaning the invalidated and expired API keys from the security index.
  */
-public final class ExpiredApiKeysRemover extends AbstractRunnable {
-    public static final Duration EXPIRED_API_KEYS_RETENTION_PERIOD = Duration.ofDays(7L);
-
-    private static final Logger logger = LogManager.getLogger(ExpiredApiKeysRemover.class);
+public final class InactiveApiKeysRemover extends AbstractRunnable {
+    private static final Logger logger = LogManager.getLogger(InactiveApiKeysRemover.class);
 
     private final Client client;
     private final AtomicBoolean inProgress = new AtomicBoolean(false);
     private final TimeValue timeout;
+    private final AtomicLong retentionPeriodInMs;
 
-    ExpiredApiKeysRemover(Settings settings, Client client) {
+    InactiveApiKeysRemover(Settings settings, Client client, ClusterService clusterService) {
         this.client = client;
         this.timeout = ApiKeyService.DELETE_TIMEOUT.get(settings);
+        this.retentionPeriodInMs = new AtomicLong(ApiKeyService.DELETE_RETENTION_PERIOD.get(settings).getMillis());
+        clusterService.getClusterSettings()
+            .addSettingsUpdateConsumer(
+                ApiKeyService.DELETE_RETENTION_PERIOD,
+                newRetentionPeriod -> this.retentionPeriodInMs.set(newRetentionPeriod.getMillis())
+            );
     }
 
     @Override
@@ -58,11 +64,18 @@ public final class ExpiredApiKeysRemover extends AbstractRunnable {
             expiredDbq.getSearchRequest().source().timeout(timeout);
         }
         final Instant now = Instant.now();
+        final long cutoffTimestamp = now.minusMillis(retentionPeriodInMs.get()).toEpochMilli();
         expiredDbq.setQuery(
             QueryBuilders.boolQuery()
                 .filter(QueryBuilders.termsQuery("doc_type", "api_key"))
-                .should(QueryBuilders.termsQuery("api_key_invalidated", true))
-                .should(QueryBuilders.rangeQuery("expiration_time").lte(now.minus(EXPIRED_API_KEYS_RETENTION_PERIOD).toEpochMilli()))
+                .should(QueryBuilders.rangeQuery("expiration_time").lte(cutoffTimestamp))
+                .should(
+                    QueryBuilders.boolQuery()
+                        .filter(QueryBuilders.termsQuery("api_key_invalidated", true))
+                        .should(QueryBuilders.rangeQuery("invalidation_time").lte(cutoffTimestamp))
+                        .should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("invalidation_time")))
+                        .minimumShouldMatch(1)
+                )
                 .minimumShouldMatch(1)
         );
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailFilterTests.java

@@ -2869,6 +2869,7 @@ public class LoggingAuditTrailFilterTests extends ESTestCase {
         final List<Setting<?>> settingsList = new ArrayList<>();
         LoggingAuditTrail.registerSettings(settingsList);
         settingsList.addAll(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
+        settingsList.add(ApiKeyService.DELETE_RETENTION_PERIOD);
         return new ClusterSettings(settings, new HashSet<>(settingsList));
     }
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java

@@ -320,7 +320,8 @@ public class LoggingAuditTrailTests extends ESTestCase {
                 LoggingAuditTrail.FILTER_POLICY_IGNORE_ROLES,
                 LoggingAuditTrail.FILTER_POLICY_IGNORE_INDICES,
                 LoggingAuditTrail.FILTER_POLICY_IGNORE_ACTIONS,
-                Loggers.LOG_LEVEL_SETTING
+                Loggers.LOG_LEVEL_SETTING,
+                ApiKeyService.DELETE_RETENTION_PERIOD
             )
         );
         when(clusterService.getClusterSettings()).thenReturn(clusterSettings);

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

@@ -40,12 +40,14 @@ import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.cache.Cache;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
 import org.elasticsearch.common.util.concurrent.ListenableFuture;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
@@ -2119,12 +2121,16 @@ public class ApiKeyServiceTests extends ESTestCase {
             .put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true)
             .put(baseSettings)
             .build();
+        final ClusterSettings clusterSettings = new ClusterSettings(
+            settings,
+            Sets.union(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS, Set.of(ApiKeyService.DELETE_RETENTION_PERIOD))
+        );
         final ApiKeyService service = new ApiKeyService(
             settings,
             clock,
             client,
             securityIndex,
-            ClusterServiceUtils.createClusterService(threadPool),
+            ClusterServiceUtils.createClusterService(threadPool, clusterSettings),
             cacheInvalidatorRegistry,
             threadPool
         );

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

@@ -37,9 +37,11 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.env.Environment;
@@ -304,7 +306,11 @@ public class AuthenticationServiceTests extends ESTestCase {
             runnable.run();
             return null;
         }).when(securityIndex).checkIndexVersionThenExecute(anyConsumer(), any(Runnable.class));
-        ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool);
+        final ClusterSettings clusterSettings = new ClusterSettings(
+            settings,
+            Sets.union(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS, Set.of(ApiKeyService.DELETE_RETENTION_PERIOD))
+        );
+        ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool, clusterSettings);
         final SecurityContext securityContext = new SecurityContext(settings, threadContext);
         apiKeyService = new ApiKeyService(
             settings,

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

@@ -14,6 +14,7 @@ import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -64,6 +65,7 @@ import java.util.Base64;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -122,6 +124,7 @@ public class SecondaryAuthenticatorTests extends ESTestCase {
         final ClusterService clusterService = mock(ClusterService.class);
         final ClusterState clusterState = ClusterState.EMPTY_STATE;
         when(clusterService.state()).thenReturn(clusterState);
+        when(clusterService.getClusterSettings()).thenReturn(new ClusterSettings(settings, Set.of(ApiKeyService.DELETE_RETENTION_PERIOD)));
 
         securityContext = new SecurityContext(settings, threadContext);
 

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

@@ -33,6 +33,7 @@ import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.set.Sets;
@@ -1806,13 +1807,17 @@ public class CompositeRolesStoreTests extends ESTestCase {
         }).when(nativeRolesStore).getRoleDescriptors(isASet(), anyActionListener());
         final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore());
         ThreadContext threadContext = new ThreadContext(SECURITY_ENABLED_SETTINGS);
+        final ClusterService clusterService = mock(ClusterService.class);
+        when(clusterService.getClusterSettings()).thenReturn(
+            new ClusterSettings(SECURITY_ENABLED_SETTINGS, Set.of(ApiKeyService.DELETE_RETENTION_PERIOD))
+        );
         ApiKeyService apiKeyService = spy(
             new ApiKeyService(
                 SECURITY_ENABLED_SETTINGS,
                 Clock.systemUTC(),
                 mock(Client.class),
                 mock(SecurityIndexManager.class),
-                mock(ClusterService.class),
+                clusterService,
                 mock(CacheInvalidatorRegistry.class),
                 mock(ThreadPool.class)
             )
@@ -1882,13 +1887,17 @@ public class CompositeRolesStoreTests extends ESTestCase {
         final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore());
         ThreadContext threadContext = new ThreadContext(SECURITY_ENABLED_SETTINGS);
 
+        final ClusterService clusterService = mock(ClusterService.class);
+        when(clusterService.getClusterSettings()).thenReturn(
+            new ClusterSettings(SECURITY_ENABLED_SETTINGS, Set.of(ApiKeyService.DELETE_RETENTION_PERIOD))
+        );
         ApiKeyService apiKeyService = spy(
             new ApiKeyService(
                 SECURITY_ENABLED_SETTINGS,
                 Clock.systemUTC(),
                 mock(Client.class),
                 mock(SecurityIndexManager.class),
-                mock(ClusterService.class),
+                clusterService,
                 mock(CacheInvalidatorRegistry.class),
                 mock(ThreadPool.class)
             )