Skip to content

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Oct 24, 2022

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

The PR introduces a waiting for token invalidation similar to the fix
for elastic#67347 and elastic#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: elastic#90509
@ywangd ywangd added >test Issues or PRs that are addressing/adding tests :Security/Security Security issues without another label v7.17.0 v8.6.0 labels Oct 24, 2022
@ywangd ywangd requested a review from n1v0lg October 24, 2022 04:11
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Oct 24, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ywangd ywangd merged commit 6e1e807 into elastic:main Oct 24, 2022
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Oct 25, 2022
)

The PR introduces a waiting for token invalidation similar to the fix
for elastic#67347 and elastic#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: elastic#90509
elasticsearchmachine pushed a commit that referenced this pull request Oct 25, 2022
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Security/Security Security issues without another label Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v7.17.0 v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] TokenAuthIntegTests testExpiredTokensDeletedAfterExpiration failing

3 participants