Skip to content

Commit 36eb8b9

Browse files
authored
[Test] More resilience for token invalidation test (#75319) (#76737)
This PR add more resilience to testExpiredTokensDeletedAfterExpiration by wrapping the empty error assertion into an assertBusy block. In rare cases, it is possible for search to return a deleted but not yet refreshed document. Trying to invalidate this document then result into MissDocumentException. This state is transient and should go away once all shards refresh. Resolves: #67347
1 parent 09030c4 commit 36eb8b9

File tree

1 file changed

+53
-3
lines changed

1 file changed

+53
-3
lines changed

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

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import static org.hamcrest.Matchers.empty;
6666
import static org.hamcrest.Matchers.equalTo;
6767
import static org.hamcrest.Matchers.hasItem;
68+
import static org.hamcrest.Matchers.hasSize;
6869

6970
public class TokenAuthIntegTests extends SecurityIntegTestCase {
7071

@@ -217,22 +218,71 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
217218

218219
// Now the documents are deleted, try to invalidate the access token and refresh token again
219220
InvalidateTokenResponse invalidateAccessTokenResponse = securityClient.prepareInvalidateToken(accessToken)
220-
.setType(randomFrom(InvalidateTokenRequest.Type.values()))
221+
.setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
221222
.execute()
222223
.actionGet();
223224
assertThat(invalidateAccessTokenResponse.getResult().getInvalidatedTokens(), empty());
224225
assertThat(invalidateAccessTokenResponse.getResult().getPreviouslyInvalidatedTokens(), empty());
225226
assertThat(invalidateAccessTokenResponse.getResult().getErrors(), empty());
227+
228+
// Weird testing behaviour ahead...
229+
// invalidating by access token (above) is a Get, but invalidating by refresh token (below) is a Search
230+
// In a multi node cluster, in a small % of cases, the search might find a document that has been deleted but not yet refreshed
231+
// in that node's shard.
232+
// Our assertion, therefore, is that an attempt to invalidate the refresh token must not actually invalidate
233+
// anything (concurrency controls must prevent that), nor may return any errors,
234+
// but it might _temporarily_ find an "already deleted" token.
226235
InvalidateTokenResponse invalidateRefreshTokenResponse = securityClient.prepareInvalidateToken(refreshToken)
227236
.setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
228237
.execute()
229238
.actionGet();
230239
assertThat(invalidateRefreshTokenResponse.getResult().getInvalidatedTokens(), empty());
231240
assertThat(invalidateRefreshTokenResponse.getResult().getPreviouslyInvalidatedTokens(), empty());
232-
assertThat(invalidateRefreshTokenResponse.getResult().getErrors(), empty());
241+
242+
// 99% of the time, this will already be empty, but if not ensure it goes to empty within the allowed timeframe
243+
if (false == invalidateRefreshTokenResponse.getResult().getErrors().isEmpty()) {
244+
assertBusy(() -> {
245+
InvalidateTokenResponse newResponse = securityClient.prepareInvalidateToken(refreshToken)
246+
.setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
247+
.execute()
248+
.actionGet();
249+
assertThat(newResponse.getResult().getErrors(), empty());
250+
});
251+
}
252+
}
253+
254+
public void testAccessTokenAndRefreshTokenCanBeInvalidatedIndependently() {
255+
CreateTokenResponse response = securityClient().prepareCreateToken().setGrantType("password")
256+
.setUsername(SecuritySettingsSource.TEST_USER_NAME)
257+
.setPassword(new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray()))
258+
.get();
259+
final InvalidateTokenResponse response1, response2;
260+
if (randomBoolean()) {
261+
response1 = securityClient().prepareInvalidateToken(response.getTokenString())
262+
.setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
263+
.execute().actionGet();
264+
response2 = securityClient().prepareInvalidateToken(response.getRefreshToken())
265+
.setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
266+
.execute().actionGet();
267+
} else {
268+
response1 = securityClient().prepareInvalidateToken(response.getRefreshToken())
269+
.setType(InvalidateTokenRequest.Type.REFRESH_TOKEN)
270+
.execute().actionGet();
271+
response2 = securityClient().prepareInvalidateToken(response.getTokenString())
272+
.setType(InvalidateTokenRequest.Type.ACCESS_TOKEN)
273+
.execute().actionGet();
274+
}
275+
276+
assertThat(response1.getResult().getInvalidatedTokens(), hasSize(1));
277+
assertThat(response1.getResult().getPreviouslyInvalidatedTokens(), empty());
278+
assertThat(response1.getResult().getErrors(), empty());
279+
280+
assertThat(response2.getResult().getInvalidatedTokens(), hasSize(1));
281+
assertThat(response2.getResult().getPreviouslyInvalidatedTokens(), empty());
282+
assertThat(response2.getResult().getErrors(), empty());
233283
}
234284

235-
public void testInvalidateAllTokensForUser() throws Exception{
285+
public void testInvalidateAllTokensForUser() throws Exception {
236286
final int numOfRequests = randomIntBetween(5, 10);
237287
for (int i = 0; i < numOfRequests; i++) {
238288
securityClient().prepareCreateToken()

0 commit comments

Comments
 (0)