(Task AB#1268089) | Defer token store unlock until token refresh response action occurs #168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…after new token stored or old token deleted Currently the token store unlock occurs directly after the token refresh call completes, but prior to the new token being stored or the old token being deleted. In situations with a high volume of asynchronous calls occurring leading up to the token refresh flow initiating, a race condition can occur upon token store unlock whereby a subsequent token refresh call that was queued then immediately retrieves the previous refresh token from the unlocked token store before the new refresh token has had a chance to be written to the token store. The resulting token refresh API call attempt using a burnt/no longer valid refresh token results in a 400 level error (e.g. 'invalid_grant' ) from the OAuth2 identity server.
…shared call - Removed some `self` references - Added comment
plflanagan
reviewed
Jan 31, 2024
plflanagan
approved these changes
Feb 2, 2024
brettwellmanmbo
deleted the
task/AB#1268089-delay-tokenstore-unlock-until-refresh-response-action-occurs
branch
brettwellmanmbo
changed the title
brettwellmanmbo
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes (pick all that apply):
Summary
The goal of this pull request is to ensure that during a token refresh, the token store lock stays engaged until after any token store I/O resulting from the outcome of the token refresh have occurred.
Presently the token store lock disengages immediately after a token refresh response is received but before any actions are taken on the token store (such as saving the new token to the token store). This, theoretically, could allow premature token store access before the outcome of the token refresh call has had a chance to propagate its result to the token store. If this were to occur, the premature access would result in the token store returning a token that was just burnt by the token refresh call, resulting in call failure due to either an access token that's no longer valid, or a refresh token that was just used.
Implementation
Move & split the disengagement of the token store lock from where it is now (executing immediately after receiving a refresh response), into two places:
Test Plan
Induce token refreshes in any client apps that consume this framework & ensure regular functionality is observed. I have been testing these changes in MBApp while hunting down a refresh token race condition and all