Skip to content

Commit ef4f3c6

Browse files
authored
[Test] Guard against "about-to-be-deleted" token document (#91071) (#91103)
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
1 parent 5f7968f commit ef4f3c6

File tree

1 file changed

+19
-8
lines changed

1 file changed

+19
-8
lines changed

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -219,22 +219,33 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
219219
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L));
220220
}, 30, TimeUnit.SECONDS);
221221

222+
// Weird testing behaviour ahead...
223+
// In a multi node cluster, invalidating by access token (get) or refresh token (search) can both,
224+
// in a small % of cases, find a document that has been deleted but not yet refreshed
225+
// in that node's shard.
226+
// Our assertion, therefore, is that an attempt to invalidate the token must not actually invalidate
227+
// anything (concurrency controls must prevent that), nor may return any errors,
228+
// but it might _temporarily_ find an "already deleted" token.
229+
222230
// Now the documents are deleted, try to invalidate the access token and refresh token again
223231
InvalidateTokenResponse invalidateAccessTokenResponse = securityClient.prepareInvalidateToken(accessToken)
224232
.setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
225233
.execute()
226234
.actionGet();
227235
assertThat(invalidateAccessTokenResponse.getResult().getInvalidatedTokens(), empty());
228236
assertThat(invalidateAccessTokenResponse.getResult().getPreviouslyInvalidatedTokens(), empty());
229-
assertThat(invalidateAccessTokenResponse.getResult().getErrors(), empty());
230237

231-
// Weird testing behaviour ahead...
232-
// invalidating by access token (above) is a Get, but invalidating by refresh token (below) is a Search
233-
// In a multi node cluster, in a small % of cases, the search might find a document that has been deleted but not yet refreshed
234-
// in that node's shard.
235-
// Our assertion, therefore, is that an attempt to invalidate the refresh token must not actually invalidate
236-
// anything (concurrency controls must prevent that), nor may return any errors,
237-
// but it might _temporarily_ find an "already deleted" token.
238+
// 99% of the time, this will already be empty, but if not ensure it goes to empty within the allowed timeframe
239+
if (false == invalidateAccessTokenResponse.getResult().getErrors().isEmpty()) {
240+
assertBusy(() -> {
241+
InvalidateTokenResponse newResponse = securityClient.prepareInvalidateToken(accessToken)
242+
.setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
243+
.execute()
244+
.actionGet();
245+
assertThat(newResponse.getResult().getErrors(), empty());
246+
});
247+
}
248+
238249
InvalidateTokenResponse invalidateRefreshTokenResponse = securityClient.prepareInvalidateToken(refreshToken)
239250
.setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
240251
.execute()

0 commit comments

Comments
 (0)