Skip to content

Commit 6e1e807

Browse files
authored
[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
1 parent 83694c3 commit 6e1e807

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -170,19 +170,27 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
170170
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L));
171171
}, 30, TimeUnit.SECONDS);
172172

173+
// Weird testing behaviour ahead...
174+
// In a multi node cluster, invalidating by access token (get) or refresh token (search) can both,
175+
// in a small % of cases, find a document that has been deleted but not yet refreshed
176+
// in that node's shard.
177+
// Our assertion, therefore, is that an attempt to invalidate the token must not actually invalidate
178+
// anything (concurrency controls must prevent that), nor may return any errors,
179+
// but it might _temporarily_ find an "already deleted" token.
180+
173181
// Now the documents are deleted, try to invalidate the access token and refresh token again
174182
TokenInvalidation invalidateAccessTokenResponse = invalidateAccessToken(accessToken);
175183
assertThat(invalidateAccessTokenResponse.invalidated(), equalTo(0));
176184
assertThat(invalidateAccessTokenResponse.previouslyInvalidated(), equalTo(0));
177-
assertThat(invalidateAccessTokenResponse.errors(), empty());
178185

179-
// Weird testing behaviour ahead...
180-
// invalidating by access token (above) is a Get, but invalidating by refresh token (below) is a Search
181-
// In a multi node cluster, in a small % of cases, the search might find a document that has been deleted but not yet refreshed
182-
// in that node's shard.
183-
// Our assertion, therefore, is that an attempt to invalidate the refresh token must not actually invalidate
184-
// anything (concurrency controls must prevent that), nor may return any errors,
185-
// but it might _temporarily_ find an "already deleted" token.
186+
// 99% of the time, this will already be empty, but if not ensure it goes to empty within the allowed timeframe
187+
if (false == invalidateAccessTokenResponse.errors().isEmpty()) {
188+
assertBusy(() -> {
189+
var newResponse = invalidateAccessToken(accessToken);
190+
assertThat(newResponse.errors(), empty());
191+
});
192+
}
193+
186194
TokenInvalidation invalidateRefreshTokenResponse = invalidateRefreshToken(refreshToken);
187195
assertThat(invalidateRefreshTokenResponse.invalidated(), equalTo(0));
188196
assertThat(invalidateRefreshTokenResponse.previouslyInvalidated(), equalTo(0));

0 commit comments

Comments
 (0)