feat(wallet): Add SeedlessOnboardingController and PasskeyController#9533
feat(wallet): Add SeedlessOnboardingController and PasskeyController#9533lwin-kyaw wants to merge 13 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@metamaskbot publish-preview |
lwin-kyaw
changed the title
|
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.
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 ded1aa1. Configure here.
| 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.
4 participants
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
High Risk
Breaking default wallet bootstrap touches passkey/WebAuthn and seedless vault encryption paths; clients must supply platform-specific options and adopt coordinated extension/mobile PRs.
Overview
Breaking:
@metamask/walletnow initializesPasskeyControllerandSeedlessOnboardingControllerby default, alongside existing wallet instances. NewInitializationConfigurationmodules forward client-suppliedinstanceOptions(WebAuthn RP settings for passkeys; JWT callbacks and optional encryptor for seedless onboarding, with a 600k-iteration PBKDF2 default when encryptor is omitted).WalletOptions/InstanceSpecificOptionsgain optionalpasskeyControllerandseedlessOnboardingControllertyping; package deps, TS project refs, README dependency graph, and CODEOWNERS initialization paths are updated. Unit tests cover registration indefaultConfigurations, state/options forwarding, and messengergetStatewiring.Reviewed by Cursor Bugbot for commit 12bb367. Bugbot is set up for automated code reviews on this repo. Configure here.