feat(passkey): reveal SRP and private keys with passkey verification#8996
Open
tanguyenvn wants to merge 9 commits into
Open
feat(passkey): reveal SRP and private keys with passkey verification#8996tanguyenvn wants to merge 9 commits into
tanguyenvn wants to merge 9 commits into
Conversation
Contributor
Author
|
I'll need to support exporting private keys of imported accounts using passkeys |
Contributor
I was about to review, but IMO we should apply the new logic with the credentials in The idea being, we now have several "credential kind" to unlock sensitive operations in this controller, passkeys knows the "vault key" (that they use with I would also add this support in this PR! |
ccharly
reviewed
Jun 5, 2026
Comment on lines
+1092
to
+1098
| if (typeof credentials === 'string') { | ||
| await this.verifyPassword(credentials); | ||
| } else if (hasProperty(credentials, 'password')) { | ||
| await this.verifyPassword(credentials.password as string); | ||
| } else { | ||
| await this.verifyEncryptionKey(credentials.encryptionKey); | ||
| } |
Contributor
There was a problem hiding this comment.
Let's add a new #verifyCredentials method for this, so we can use it in exportSeedPhrase and in exportAccount too.
|
|
||
| if (typeof credentials === 'string') { | ||
| await this.verifyPassword(credentials); | ||
| } else if (hasProperty(credentials, 'password')) { |
Contributor
There was a problem hiding this comment.
Let's use in here, that's the pattern we use in this controller already:
Suggested change
| } else if (hasProperty(credentials, 'password')) { | |
| } else if ('password' in credentials) { |
| if (typeof credentials === 'string') { | ||
| await this.verifyPassword(credentials); | ||
| } else if (hasProperty(credentials, 'password')) { | ||
| await this.verifyPassword(credentials.password as string); |
Contributor
There was a problem hiding this comment.
We don't need to type-cast here (at least, not when using if ('password' in credentials)
Suggested change
| await this.verifyPassword(credentials.password as string); | |
| await this.verifyPassword(credentials.password); |
tanguyenvn
changed the title
tanguyenvn
changed the title
ccharly
reviewed
Jun 5, 2026
| * | ||
| * @param encryptionKey - Serialized vault encryption key. | ||
| */ | ||
| async verifyEncryptionKey(encryptionKey: string): Promise<void> { |
Contributor
There was a problem hiding this comment.
Maybe we don't need to export it publicly for now, unlike verifyPassword that could be used to write "end-user logic" (e.g. to test the password), we should not really need to call verifyEncryptionKey explicitely 🤔
We could make this private (e.g. #verifyEncryptionKey).
WDYT @danroc?
ccharly
reviewed
Jun 5, 2026
| * Credentials for re-authenticating the keyring during sensitive operations | ||
| * such as `exportSeedPhrase` and `exportAccount`. | ||
| */ | ||
| export type VerificationCredentials = |
Contributor
There was a problem hiding this comment.
I would go for Credentials here!
ccharly
reviewed
Jun 5, 2026
| }); | ||
| }); | ||
|
|
||
| describe('verifyEncryptionKey', () => { |
Contributor
There was a problem hiding this comment.
Since we want to make this private, we can still test those using any methods that uses Credentials as parameter, so that should still be fine, maybe we can rename that to:
Suggested change
| describe('verifyEncryptionKey', () => { | |
| describe('#verifyEncryptionKey', () => { |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
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.
Description
Passkey-enrolled wallets currently require the wallet password to reveal sensitive key material (Secret Recovery Phrase and account private keys), even when the user has already unlocked MetaMask with a passkey. That creates friction and inconsistency with other passkey-gated flows (e.g. change password).
This PR adds password-less reveal paths for non–social-login wallets with an enrolled passkey:
revealSeedWordsWithPasskeyverifies the WebAuthn assertion, retrieves the passkey-wrapped vault key viaretrieveVaultKeyWithPasskey, and callskeyringController.exportSeedPhrase({ encryptionKey: vaultKey }).exportAccountsWithPasskeyuses the same vault-key retrieval, then callskeyringController.exportAccount({ encryptionKey: vaultKey }, address)for each address in a multichain account group. The multichain private key list UI uses passkey verification as the initial credential step, with password fallback.PasskeyVerification(andrunPasskeyVerificationCeremony) from change-password, reducing duplicated WebAuthn ceremony logic across settings and reveal flows.SrpRevealWithPasskeyMetaMetrics events (started / completed / failed).In both flows, the passkey assertion is not only a step-up UI gate — the passkey-derived vault key must successfully decrypt the vault via the keyring controller export APIs, cryptographically binding the passkey to this vault.
Not supported:
Dependency: Requires the breaking
@metamask/keyring-controllerchange that acceptsVerificationCredentials({ password }|{ encryptionKey }) onexportSeedPhraseandexportAccount(mm-core PR for TO-737).Changelog
CHANGELOG entry: Added the ability to reveal your Secret Recovery Phrase and account private keys using passkey verification instead of your wallet password.
Related issues
Fixes: TO-737
Manual testing steps
SRP reveal
Private key reveal (multichain account group)
General
yarn test:unit ui/pages/keychains/reveal-seed.test.tsx ui/components/app/passkey-verification/passkey-verification.test.tsx ui/store/actions.test.js app/scripts/metamask-controller.actions.test.jsScreenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
High Risk
Breaking API on sensitive export methods plus new encryption-key verification path; incorrect credential handling could affect SRP/private-key export security for all consumers.
Overview
BREAKING:
exportSeedPhraseandexportAccountnow accept aCredentialsobject ({ password }or{ encryptionKey }) instead of a plain password string, with changelog and messenger action docs updated accordingly.Re-authentication is centralized in
#verifyCredentials, which validates either the wallet password or the serialized vault encryption key (via new#verifyEncryptionKeyanddecryptWithKey). Password-based export behavior is preserved; callers can pass a passkey-derived vault key so export is cryptographically tied to vault decryption, not only a UI gate.exportAccountnow enforces the unlocked controller check (#assertIsUnlocked) likeexportSeedPhrase. Unit tests cover encryption-key success/failure, missing vault, and locked-state errors for both export paths.Reviewed by Cursor Bugbot for commit 834f9ac. Bugbot is set up for automated code reviews on this repo. Configure here.