feat(wallet): Add SeedlessOnboardingController and PasskeyController#9533
Conversation
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
| * Encryptor used to protect the seedless onboarding vault. Defaults to a PBKDF2 encryptor | ||
| * configured with 600,000 iterations. | ||
| */ | ||
| encryptor?: GenericEncryptor; |
There was a problem hiding this comment.
Why do we need to override this option just for initialization? (edit: oh, perhaps this is related to #9533 (comment))
There was a problem hiding this comment.
Hi, we wanna follow the KeyringControllerInstanceOptions.
We intentionally make encryptor field optional (default with GenericEncryptor) so that extension side controller init is align with KeyringController initialization.
For the mobile side, we have to provide the mobile compatible version.
There was a problem hiding this comment.
Hmm, we should think of a different way to accomplish this so we don't have to override anything in the init code. Ideally initialization code should be as simple as possible — it should only construct the controller, and the controller should do the rest. For instance in this case since the encryptor is supplied on the client side, the type of the encryptor in the controller should be generic enough to allow for that.
But if this is already a problem with KeyringController then it's a known problem. It would just be nice to resolve this at some point.
| describe('seedlessOnboardingController', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| (SeedlessOnboardingController as jest.Mock).mockImplementation( |
There was a problem hiding this comment.
See note above about mocking PasskeyController. I feel like we shouldn't have to do this.
|
|
||
| describe('seedlessOnboardingController', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); |
There was a problem hiding this comment.
You shouldn't have to do this, mocks are cleared and restored automatically in this repo.
| jest.clearAllMocks(); |
| afterEach(() => { | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
This shouldn't be necessary either.
| afterEach(() => { | |
| jest.restoreAllMocks(); | |
| }); |
| * Encryptor used to protect the seedless onboarding vault. Defaults to a PBKDF2 encryptor | ||
| * configured with 600,000 iterations. | ||
| */ | ||
| encryptor?: GenericEncryptor; |
There was a problem hiding this comment.
Hmm, we should think of a different way to accomplish this so we don't have to override anything in the init code. Ideally initialization code should be as simple as possible — it should only construct the controller, and the controller should do the rest. For instance in this case since the encryptor is supplied on the client side, the type of the encryptor in the controller should be generic enough to allow for that.
But if this is already a problem with KeyringController then it's a known problem. It would just be nice to resolve this at some point.
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| (PasskeyController as jest.Mock).mockImplementation( | ||
| (...args: unknown[]) => new ActualPasskeyController(...args), |
There was a problem hiding this comment.
Why are we mocking the controller only to effectively unmock it? I realize that you may have copied this from another test but I don't think we need to do this. See the tests for ApprovalController and RemoteFeatureFlagController.
| it('is registered as a default initialization configuration', () => { | ||
| expect(Object.values(defaultConfigurations)).toContain(passkeyController); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Do we need this test? I realize we do this for other controllers, but we use import * as defaultConfigurations from './instances'; in defaults.ts, so we can probably make a safe assumption this is included. (Besides, the purpose of this file is to test passkey-controller.ts — not defaults.ts.)
| it('is registered as a default initialization configuration', () => { | ||
| expect(Object.values(defaultConfigurations)).toContain( | ||
| seedlessOnboardingController, | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
See note about this test in passkey-controller.test.ts.
|
@metamaskbot publish-preview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 30b4935. Configure here.
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
mcmire
left a comment
There was a problem hiding this comment.
One nit, but looks good otherwise.
| messenger, | ||
| state, | ||
| }), | ||
| getMessenger: (parent: RootMessenger<DefaultActions, DefaultEvents>) => { |
There was a problem hiding this comment.
Nit: You don't need this type assertion, TypeScript can infer it:
| getMessenger: (parent: RootMessenger<DefaultActions, DefaultEvents>) => { | |
| getMessenger: (parent) => { |

Explanation
Adds
PasskeyControllerandSeedlessOnboardingControlleras default initialized controllers in thewalletpackage, following theInitializationConfigurationpattern established byKeyringController,AccountsController, and other wallet instances. Each controller gets its own per-directory init module (passkey-controller/,seedless-onboarding-controller/) with colocated types and tests.@metamask/walletis the shared controller-integration layer for extension, mobile, and other clients, so values that differ between clients are injected viainstanceOptionsrather than hardcoded.PasskeyController — WebAuthn relying-party configuration is platform-specific, so
passkeyController.expectedRPID,passkeyController.expectedOrigin, andpasskeyController.rpNamemust be supplied by each client (see the options table below). The wallet init forwardsinstanceOptionsto the controller as-is and does not inject any MetaMask-specific defaults;rpNameis required, anduserName/userDisplayNamefall back torpNameinsidePasskeyControllerwhen omitted. Persisted passkey state is forwarded from wallet state when present.SeedlessOnboardingController — JWT lifecycle callbacks (
refreshJWTToken,revokeRefreshToken,renewRefreshToken) are platform-specific and must be injected by each client (typically delegating toOAuthServiceon extension orAuthTokenHandleron mobile). A default PBKDF2 encryptor (600,000 iterations, shared withKeyringController) is applied when no customencryptoris passed. All otherSeedlessOnboardingControllerconstructor options (e.g.network,passwordOutdatedCacheTTL,keyDerivationInterface) are forwarded frominstanceOptionsas-is.Neither controller requires messenger delegation beyond the standard
getState/stateChangedwiring — both are self-contained at the messenger level.References
Per-environment options
passkeyController.expectedRPIDextension.runtime.getURL('')(stripped trailing slash)'extension-id')passkeyController.expectedOriginexpectedRPID'https://extension.origin')passkeyController.rpName'MetaMask')'MetaMask'); requiredpasskeyController.userName/userDisplayNamerpNameis desiredrpNameinside the controller when omittedseedlessOnboardingController.refreshJWTTokenOAuthService:getNewRefreshTokenvia init messengerAuthTokenHandlerjest.fn()in unit testsseedlessOnboardingController.revokeRefreshTokenOAuthService:revokeRefreshTokenAuthTokenHandlerjest.fn()in unit testsseedlessOnboardingController.renewRefreshTokenOAuthService:renewRefreshTokenAuthTokenHandlerjest.fn()in unit testsseedlessOnboardingController.encryptorencryptorFactory(600_000)(extension-local factory)Encryptoradapter (cipher↔datanormalization)encryptorFactory(600_000)seedlessOnboardingController.networkWeb3AuthNetwork.Devnet(dev/test) orMainnet(prod)web3AuthNetworkconstantPasskeyControllerhas no messenger-level dependencies on other controllers.SeedlessOnboardingControllercurrently receives JWT callbacks out-of-band (not via messenger); a future refactor may route these throughOAuthServiceactions directly.Client construction sites
These are the current
mainconstruction sites the compatibility audit was based on (the adoption PRs below will replace them):PasskeyController
SeedlessOnboardingController
Client adoption PRs
Checklist
Note
Medium Risk
Breaking wallet initialization touches passkey vault protection and seedless auth/key material paths via Keyring delegation and encryptor defaults; misconfigured client options could break onboarding or auth at runtime.
Overview
Breaking:
@metamask/walletnow includes PasskeyController and SeedlessOnboardingController in the defaultinitialize()ensemble, so consumers must supply the new optionalinstanceOptions(passkeyController,seedlessOnboardingController) where those flows are used.New initialization modules follow the existing
InitializationConfigurationpattern: Passkey forwards client WebAuthn settings (expectedRPID,expectedOrigin,rpName, etc.) and delegates KeyringController password/export actions on the messenger; seedless onboarding forwards JWT lifecycle callbacks from the client and applies the shared 600k-iteration encryptor when none is provided.Dependencies, TypeScript project references, README dependency graph, CODEOWNERS /
codeowners.tsinit paths, and unit tests for init, state, and messenger exposure are updated accordingly.Reviewed by Cursor Bugbot for commit b5b7ac6. Bugbot is set up for automated code reviews on this repo. Configure here.