fix: JWT token refresh race conditions in seedless-onboarding-controller.#7989
Merged
himanshuchawla009 merged 4 commits intomainfrom Feb 23, 2026
Merged
fix: JWT token refresh race conditions in seedless-onboarding-controller.#7989himanshuchawla009 merged 4 commits intomainfrom
himanshuchawla009 merged 4 commits intomainfrom
Conversation
himanshuchawla009
changed the title
himanshuchawla009
changed the title
himanshuchawla009
changed the title
himanshuchawla009
force-pushed
the
fix/refresh-token-concurrency
branch
from
b2e076c to
27d5ba4
Compare
Contributor
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
himanshuchawla009
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Feb 19, 2026
…MaxKeyChainLength recovery Companion to MetaMask/core#7989. - Add `RefreshTokenHttpError` class to `AuthTokenHandler` that carries the HTTP `statusCode`, enabling callers to distinguish permanent (401) from transient failures. The core controller duck-types this error shape to map 401 responses to `InvalidRefreshToken` instead of the generic `FailedToRefreshJWTTokens`. - Wrap `SeedlessOnboardingController.refreshAuthTokens()` in a try/catch in the `MaxKeyChainLengthExceeded` recovery path. A stale or revoked refresh token should not abort rehydration — the error is logged and execution continues to `rehydrateSeedPhrase`.
3 tasks
lwin-kyaw
reviewed
Feb 19, 2026
| pendingToBeRevokedTokens?: { | ||
| refreshToken: string; | ||
| revokeToken: string; | ||
| queuedAt?: number; |
Contributor
There was a problem hiding this comment.
Hi, we add this new property queuedAt so that the pendingToBeRevokedTokens will stay in the state for at most 15 seconds.
Since this state value is persisted so we will accidentally expose the revokeToken in the plain text?
Contributor
There was a problem hiding this comment.
I've noticed that the way we use revokePendingRefreshTokens method in extension and mobile is abit different too.
We call this method immediately after renewRefreshToken is successful in the extension.
But in mobile, we add the 15s delay between calls.
Contributor
Author
There was a problem hiding this comment.
have reverted the queuedAt property. We will remove it from mobile as well.
…s and fix 401 log
- Add test: concurrent refreshAuthTokens rejection propagates to all callers
- Add test: #pendingRefreshPromise is cleared after failure (recovery path)
- Add test: live state.refreshToken is used in authenticate after concurrent token rotation
- Add test: old token not queued for revocation when vault creation fails
- Move coalescing test inside describe('refreshAuthTokens') (was at wrong nesting level)
- Log httpStatusCode alongside error in #doRefreshAuthTokens catch block so
401 misclassification is observable in production logs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d from token revocation The 15 s delay before revoking pending tokens was intended to cover in-flight refreshAuthTokens calls on the old token, but the mobile client plans to remove the corresponding delay on its side. Remove the PENDING_REVOKE_TOKEN_DELAY_MS constant, the queuedAt timestamp field, the filtering logic in revokePendingRefreshTokens, and the associated tests and changelog entry.
himanshuchawla009
force-pushed
the
fix/refresh-token-concurrency
branch
from
af61130 to
0e35a86
Compare
lwin-kyaw
approved these changes
Feb 20, 2026
github-merge-queue bot
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Feb 25, 2026
…MaxKeyChainLength recovery (#26258) ## Explanation Companion to MetaMask/core#7989. ### 1. `RefreshTokenHttpError` in `AuthTokenHandler` The core `SeedlessOnboardingController` now distinguishes permanent token failures (401) from transient ones to apply the right recovery strategy. It does this by checking whether the error thrown by `refreshJWTToken` is a `RefreshTokenHttpError` with `statusCode === 401`. Previously `AuthTokenHandler` threw a plain `Error('Failed to refresh JWT token')` for all non-2xx responses, making this distinction impossible. This PR adds a `RefreshTokenHttpError` class that carries the HTTP `statusCode`, giving the controller the information it needs ### 2. try/catch around `refreshAuthTokens` in the `MaxKeyChainLengthExceeded` recovery path `refreshAuthTokens` is called fire-and-forget in the `MaxKeyChainLengthExceeded` recovery path before `rehydrateSeedPhrase`. If the refresh token is already stale or revoked (which is precisely the scenario that triggers this path), the call would throw and abort rehydration entirely — leaving the user stuck. Wrapping in try/catch logs the error and lets rehydration proceed regardless. ### 3. encryptorAdapter for SeedlessOnboardingController KeyringController has a generic EncryptionResult type that accepts our mobile Encryptor's cipher property. However, SeedlessOnboardingController uses a fixed VaultEncryptor type (from ExportableKeyEncryptor) that strictly requires data instead of cipher. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## References - Companion PR: MetaMask/core#7989 — fixes JWT token refresh race conditions in `SeedlessOnboardingController` ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've manually tested the PR (required) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches OAuth token refresh and seedless recovery flows; incorrect error typing/handling could change recovery behavior for revoked or transient refresh-token failures. > > **Overview** > Improves seedless OAuth token-refresh error handling so callers can distinguish auth-server HTTP failures by status code. > > `AuthTokenHandler.refreshJWTToken` now throws `RefreshTokenHttpError` (includes `statusCode`) on non-2xx responses, and the `MaxKeyChainLengthExceeded` recovery path in `Authentication.syncPasswordAndUnlockWallet` wraps `SeedlessOnboardingController.refreshAuthTokens()` in a `try/catch` to log and continue with `rehydrateSeedPhrase` instead of aborting recovery. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 413b327. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Gaurav Goel <grvgoel19@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Explanation
The token refresh/rotation lifecycle has several race conditions that produce stale or already-revoked tokens being sent to the auth server. In production this manifests as mass 401 responses from
POST /api/v1/oauth/token, surfacing to users asSeedlessOnboardingController - Failed to refresh JWT tokensThree bugs are fixed:
1. Concurrent
refreshAuthTokensstormrefreshAuthTokenshad no guard against concurrent invocations. Multiple simultaneous callers (e.g.fetchMetadataAccessCreds,#executeWithTokenRefresh, a background reconnect) each issued an independent HTTP refresh request with the same token. The server honours the first one and may reject the rest.Fix: a
#pendingRefreshPromisefield coalesces concurrent calls — if a refresh is already in-flight, new callers receive the same promise rather than firing a duplicate request.2. Stale
refreshTokencapture bugInside
#doRefreshAuthTokens, after the HTTP call returns, the code was callingauthenticate(…, refreshToken: refreshToken)whererefreshTokenwas captured at the start of the method. IfrenewRefreshTokenran concurrently during the HTTP round-trip,state.refreshTokenwould already have been updated to the new token. Passing the captured (now-stale) value toauthenticatewould silently overwritestate.refreshTokenwith the old token, causing all subsequent refreshes to 401 once that old token is revoked 15 s later.Fix: use
this.state.refreshToken(the live value) rather than the captured variable when callingauthenticate.3. Opaque error taxonomy + premature revocation queuing
All HTTP failures from the refresh endpoint were wrapped in the same
FailedToRefreshJWTTokenserror, making it impossible to distinguish a permanently-revoked token (401) from a transient failure (503, network timeout). The controller also called#addRefreshTokenToRevokeListbefore#createNewVaultWithAuthData, meaning a token was queued for revocation even if vault creation subsequently failed.Fix: the catch block in
#doRefreshAuthTokenschecks whether the underlying error is aRefreshTokenHttpErrorwithstatusCode === 401and maps that toInvalidRefreshTokeninstead ofFailedToRefreshJWTTokens, giving callers a distinct permanent vs transient signal. The#addRefreshTokenToRevokeListcall is moved to after#createNewVaultWithAuthDatasucceeds.References
METAMASK-MOBILE-4FKN—SeedlessOnboardingController - Failed to refresh JWT tokens(metamask-mobile: try/catch aroundrefreshAuthTokensin theMaxKeyChainLengthExceededrecovery path +RefreshTokenHttpErrortyped error class inAuthTokenHandlerChecklist
Note
Medium Risk
Touches authentication/token refresh and rotation flows; while scoped and well-tested, regressions could impact login/session continuity and token revocation behavior.
Overview
Prevents refresh-token race conditions in
SeedlessOnboardingControllerby coalescing concurrentrefreshAuthTokenscalls behind a shared in-flight promise and splitting the implementation intorefreshAuthTokens+#doRefreshAuthTokens.Improves correctness and observability by using the live
state.refreshTokenwhen re-callingauthenticateafter an HTTP refresh, mapping 401RefreshTokenHttpErrortoInvalidRefreshToken(vs genericFailedToRefreshJWTTokens), and only queueing the old token for revocation after#createNewVaultWithAuthDatasucceeds duringrenewRefreshToken.Adds targeted unit tests for the new error behavior, concurrency coalescing/cleanup, and the vault-failure revocation-queueing fix, and documents the changes in the package changelog.
Written by Cursor Bugbot for commit 0e35a86. This will update automatically on new commits. Configure here.