From b9e2235bad3e7eeb7411223ad69c46abba90145a Mon Sep 17 00:00:00 2001
From: Yang Wang <yang.wang@elastic.co>
Date: Tue, 25 Oct 2022 10:31:59 +1100
Subject: [PATCH] [Test] Guard against "about-to-be-deleted" token document
 (#91071)

The PR introduces a waiting for token invalidation similar to the fix
for #67347 and #64757. It targets access token instead of the refresh
token. But the underlying cause is the same because invalidation of both
token types are at least a 2-step process: (1) find the document and (2)
delete the document. In the first step, a document can be found by
either search or get when it is about to be deleted and this results to
missingDocument exception in step 2. The intention of the particular
test does not really care about this situation. Its main purpose is to
ensure nothing gets invalidated when the requested token does not exist
(deleted). Hence the PR just adds a wait to ensure temporary
inconsistent condition eventually goes away.

Resolves: #90509
---
 .../security/authc/TokenAuthIntegTests.java   | 27 +++++++++++++------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
index 9f0d4f67461b9..5deaf88a725dd 100644
--- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
+++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
@@ -219,6 +219,14 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
             assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L));
         }, 30, TimeUnit.SECONDS);
 
+        // Weird testing behaviour ahead...
+        // In a multi node cluster, invalidating by access token (get) or refresh token (search) can both,
+        // in a small % of cases, find a document that has been deleted but not yet refreshed
+        // in that node's shard.
+        // Our assertion, therefore, is that an attempt to invalidate the token must not actually invalidate
+        // anything (concurrency controls must prevent that), nor may return any errors,
+        // but it might _temporarily_ find an "already deleted" token.
+
         // Now the documents are deleted, try to invalidate the access token and refresh token again
         InvalidateTokenResponse invalidateAccessTokenResponse = securityClient.prepareInvalidateToken(accessToken)
             .setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
@@ -226,15 +234,18 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
             .actionGet();
         assertThat(invalidateAccessTokenResponse.getResult().getInvalidatedTokens(), empty());
         assertThat(invalidateAccessTokenResponse.getResult().getPreviouslyInvalidatedTokens(), empty());
-        assertThat(invalidateAccessTokenResponse.getResult().getErrors(), empty());
 
-        // Weird testing behaviour ahead...
-        // invalidating by access token (above) is a Get, but invalidating by refresh token (below) is a Search
-        // In a multi node cluster, in a small % of cases, the search might find a document that has been deleted but not yet refreshed
-        // in that node's shard.
-        // Our assertion, therefore, is that an attempt to invalidate the refresh token must not actually invalidate
-        // anything (concurrency controls must prevent that), nor may return any errors,
-        // but it might _temporarily_ find an "already deleted" token.
+        // 99% of the time, this will already be empty, but if not ensure it goes to empty within the allowed timeframe
+        if (false == invalidateAccessTokenResponse.getResult().getErrors().isEmpty()) {
+            assertBusy(() -> {
+                InvalidateTokenResponse newResponse = securityClient.prepareInvalidateToken(accessToken)
+                    .setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
+                    .execute()
+                    .actionGet();
+                assertThat(newResponse.getResult().getErrors(), empty());
+            });
+        }
+
         InvalidateTokenResponse invalidateRefreshTokenResponse = securityClient.prepareInvalidateToken(refreshToken)
             .setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
             .execute()