fix: persist new accessToken in SeedlessOnboarding Vault after tokens refresh
#7800
Conversation
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
Contributor
Author
|
@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. |
Contributor
Author
|
@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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Show resolved
Hide resolved
Contributor
Author
|
@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. |
…t Encryptor interface
…he wallet is unlocked
Contributor
Author
|
@metamaskbot publish-preview |
lwin-kyaw
changed the title
accessToken in SeedlessOnboarding Vault after tokens refresh
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Show resolved
Hide resolved
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@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. |
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Show resolved
Hide resolved
matthiasgeihs
requested changes
Feb 3, 2026
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Outdated
Show resolved
Hide resolved
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@metamaskbot publish-preview |
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@metamaskbot publish-preview |
1 similar comment
Contributor
Author
|
@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. |
tuna1207
reviewed
Feb 5, 2026
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Show resolved
Hide resolved
packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts
Show resolved
Hide resolved
Member
chaitanyapotti
left a comment
chaitanyapotti
left a comment
There was a problem hiding this comment.
Do we also refresh on server 401?
Contributor
Author
No, we don't refresh on 401 responses from Anyway, we validate the tokens (expiry) before we make requests (in |
matthiasgeihs
approved these changes
Feb 10, 2026
| // if the password is provided (not undefined), encrypt the vault with the password | ||
| // We gonna prioritize the password encryption here, in case of the operation is `Change Password`. | ||
| // We don't wanna re-use the old encryption key from the state. | ||
| if (password !== undefined) { |
Member
There was a problem hiding this comment.
best practice is to use typeof password !== "undefined" when checking for existence of a variable especially globals.
This is fine for this use case.
| // so skip the check if the vault is locked | ||
| let isAccessTokenExpired = false; | ||
| if (this.#isUnlocked) { | ||
| isAccessTokenExpired = this.checkAccessTokenExpired(); |
Member
There was a problem hiding this comment.
Ideally, we must check expiry with server as part of oauth2 spec
since, client time can be incorrect or session might have been invalidated by server.
this particular implementation is non-standard and can be revisited at a later date
chaitanyapotti
approved these changes
Feb 11, 2026
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Contributor
Author
|
@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. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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
This PR address
accessTokenisn't being persisted to vault after the tokens refresh (refreshAuthToken()).Issue:
Previously, clients could directly access state.accessToken from the controller state, which could potentially return an expired token. This could lead to failed API calls and poor user experience when the token had expired but wasn't refreshed.
Solution:
getAccessToken()that automatically refreshes expired tokens: Uses the #executeWithTokenRefresh wrapper to check token expiration and refresh if needed before returningaccessTokeninto Encrypted vault after token refresh.Persist
newAccessTokenin the encrypted vaultThe persist flow can be different based on the wallet (vault) status when
refreshAuthTokens()is called.encryptionKeyis available in the state).When wallet is unlocked, vault encryption key is temporarily cached in the controller state and the values inside the encrypted vault are also flooded into the controller state, for performance and to avoid the password dependency
We can use these cache values and create a new encrypted vault (with updated accessToken).
While the wallet is locked,
refreshAuthTokens()can be called under one condition, i.e, check for password sync (getAuthPubKeyRPC call) when user tries to unlock the wallet.In this case, we can't immediately update the encrypted vault as the wallet is locked. So, we will temporarily store the new
accessTokenin the controller state, then persist the stored value later in the vault unlock (only when user provides correct password) and update the vault.Based on the password sync status, the persist flow can be a little bit different.
2.1. Password is in sync
When user's current device password is in sync, we will update the vault when user unlocks via
submitPassword.Vault is decrypted using the submitted password, we will compare the access token value from decrypted vault (
accessToken1) and recently set new access token (accessToken2). If the accessToken is different and detected as new, we will update the encrypted vault value in the state (using password).Please note that this requires an additional encryption step to update the latestToken in encrypted vault.
2.2. Password is not in sync (
OutdatedPassword).When user's current device password is out of sync and user submits the latest
globalPassword, we do thepwEncKeyrecovery first so that we can unlock the current vault. Similar to the flow above (2.1), we decrypt the current vault and compare the two tokens (accessToken1andaccessToken2). If the accessToken is different and detected as new, we will update the controller state and encrypted vault.Unlike the flow (2.1.), this doesn't require additional encryption, as we can include the latest accessToken when we sync the latest global enc keys into the controller.
References
Fixes #7805, MetaMask/metamask-extension#39566
Checklist
Note
Medium Risk
Touches vault encryption/update paths and token refresh behavior, so bugs could lead to incorrect vault contents or token state during unlock/refresh flows, though changes are localized and well-covered by tests.
Overview
Fixes token persistence by updating the seedless onboarding vault when
refreshAuthTokens()obtains a newaccessToken, usingencryptWithKeywhen the wallet is already unlocked and reconciling state vs vault tokens duringsubmitPassword/submitGlobalPasswordvia a newcompareAndGetLatestTokenhelper.Adds a new public
getAccessToken()action/messenger handler that returns the current access token and triggers the existing refresh flow when tokens are expired, and introducesassertIsValidPasswordplus test refactors/new cases (including a sharedcreateMockJWTToken) to cover these behaviors. Breaking: the injected vaultencryptormust now implementencryptWithKey.Written by Cursor Bugbot for commit 09a0ddf. This will update automatically on new commits. Configure here.