From 109282cfd933ba8f7e26024a4bd71d6ce403edb7 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Tue, 13 Oct 2026 16:35:14 +0200 Subject: [PATCH 1/5] feat(keyring-eth-money-mpc): skip MFA for whitelisted signatures Ask the whitelist before prompting, and retry once with a 2FA token when the backend still returns mfa_required. Co-authored-by: Cursor --- packages/keyring-eth-money-mpc/CHANGELOG.md | 4 + packages/keyring-eth-money-mpc/README.md | 6 + packages/keyring-eth-money-mpc/package.json | 1 + .../keyring-eth-money-mpc/src/cloud.test.ts | 100 +++++++- packages/keyring-eth-money-mpc/src/cloud.ts | 70 +++++- packages/keyring-eth-money-mpc/src/index.ts | 3 +- .../src/mpc-money-keyring.test.ts | 235 +++++++++++++++++- .../src/mpc-money-keyring.ts | 92 ++++++- packages/keyring-eth-money-mpc/src/types.ts | 18 ++ yarn.config.cjs | 5 +- yarn.lock | 56 ++++- 11 files changed, 559 insertions(+), 31 deletions(-) diff --git a/packages/keyring-eth-money-mpc/CHANGELOG.md b/packages/keyring-eth-money-mpc/CHANGELOG.md index 2d6168930..15412cf71 100644 --- a/packages/keyring-eth-money-mpc/CHANGELOG.md +++ b/packages/keyring-eth-money-mpc/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Skip MFA for whitelisted signature requests, and retry with a 2FA token when the backend returns `mfa_required` ([#TODO](https://github.com/MetaMask/accounts/pull/TODO)) + ## [0.1.0] ### Added diff --git a/packages/keyring-eth-money-mpc/README.md b/packages/keyring-eth-money-mpc/README.md index 3fa2232b8..d30d4f9b2 100644 --- a/packages/keyring-eth-money-mpc/README.md +++ b/packages/keyring-eth-money-mpc/README.md @@ -4,6 +4,12 @@ A Keyring for Ethereum money accounts that uses Multi-Party Computation (MPC) fo The default export is the V1 `MpcMoneyKeyring` implementation. A V2 adapter that wraps it is available from `@metamask/eth-money-mpc-keyring/v2`. +## MFA + +Pass `getMfaWhitelistConfig` to let signature requests skip MFA when they match [`@metamask/money-account-mfa-whitelist`](https://github.com/MetaMask/core/tree/main/packages/money-account-mfa-whitelist). The keyring sends the original request with the hash. The backend repeats the check with its own account, hash, and pinned configuration. When that still requires MFA, `/sign` responds with `{ "code": "mfa_required" }`. The keyring then requests a 2FA profile token and calls `/sign` once more. + +Key generation, import, share rotation, share sync, and transaction signing always use a 2FA token. Omitting `getMfaWhitelistConfig` does too. + ## Installation `yarn add @metamask/eth-money-mpc-keyring` diff --git a/packages/keyring-eth-money-mpc/package.json b/packages/keyring-eth-money-mpc/package.json index d4c1e3da4..a9c4e2cdb 100644 --- a/packages/keyring-eth-money-mpc/package.json +++ b/packages/keyring-eth-money-mpc/package.json @@ -60,6 +60,7 @@ "@metamask/mfa-wallet-cl24": "^0.1.0", "@metamask/mfa-wallet-dkls23": "^0.1.0", "@metamask/mfa-wallet-network": "^0.1.0", + "@metamask/money-account-mfa-whitelist": "file:../../../../core/packages/money-account-mfa-whitelist", "@metamask/superstruct": "^3.4.1", "@metamask/utils": "^11.11.0" }, diff --git a/packages/keyring-eth-money-mpc/src/cloud.test.ts b/packages/keyring-eth-money-mpc/src/cloud.test.ts index d0c54f859..f8181f042 100644 --- a/packages/keyring-eth-money-mpc/src/cloud.test.ts +++ b/packages/keyring-eth-money-mpc/src/cloud.test.ts @@ -198,14 +198,16 @@ describe('cloud helpers', () => { okEmpty(); const data = new Uint8Array([104, 105]); // "hi" - await sign({ - baseURL: 'https://cloud.example', - token: 'token-1', - clientNetId: 'local-1', - nonce: '0xnonce', - data, - shareEpoch: 1, - }); + expect( + await sign({ + baseURL: 'https://cloud.example', + token: 'token-1', + clientNetId: 'local-1', + nonce: '0xnonce', + data, + shareEpoch: 1, + }), + ).toBe('ok'); const init = fetchSpy.mock.calls[0]?.[1] as { body: string; @@ -220,6 +222,88 @@ describe('cloud helpers', () => { expect(body.data).toBe('aGk='); expect(body.shareEpoch).toBe(1); expect(body.token).toBeUndefined(); + expect(body).not.toHaveProperty('request'); + }); + + it('sends the signature request with the hash', async () => { + okEmpty(); + const request = { + method: 'signPersonalMessage' as const, + address: '0x1111111111111111111111111111111111111111' as const, + message: '0x68656c6c6f' as const, + }; + + await sign({ + baseURL: 'https://cloud.example', + token: 'token-1', + clientNetId: 'local-1', + nonce: '0xnonce', + data: new Uint8Array([1]), + shareEpoch: 1, + request, + }); + + const body = JSON.parse( + (fetchSpy.mock.calls[0]?.[1] as { body: string }).body, + ) as { request: unknown }; + expect(body.request).toStrictEqual(request); + }); + + it('returns mfa_required when the backend requires MFA', async () => { + fetchSpy.mockResolvedValue({ + ok: false, + statusText: 'Forbidden', + text: jest + .fn() + .mockResolvedValue( + JSON.stringify({ code: 'mfa_required', reason: 'stale config' }), + ), + } as never); + + expect( + await sign({ + baseURL: 'https://cloud.example', + token: 'token-1', + clientNetId: 'local-1', + nonce: '0xnonce', + data: new Uint8Array([1]), + shareEpoch: 1, + }), + ).toBe('mfa_required'); + }); + + it('returns mfa_required from a successful response body', async () => { + okJson({ code: 'mfa_required' }); + + expect( + await sign({ + baseURL: 'https://cloud.example', + token: 'token-1', + clientNetId: 'local-1', + nonce: '0xnonce', + data: new Uint8Array([1]), + shareEpoch: 1, + }), + ).toBe('mfa_required'); + }); + + it('throws when a sign error body is not mfa_required', async () => { + fetchSpy.mockResolvedValue({ + ok: false, + statusText: 'Forbidden', + text: jest.fn().mockResolvedValue(JSON.stringify({ code: 'forbidden' })), + } as never); + + await expect( + sign({ + baseURL: 'https://cloud.example', + token: 'token-1', + clientNetId: 'local-1', + nonce: '0xnonce', + data: new Uint8Array([1]), + shareEpoch: 1, + }), + ).rejects.toThrow('Failed to initialize cloud sign session: Forbidden'); }); it('throws when cloud sign initialization fails', async () => { diff --git a/packages/keyring-eth-money-mpc/src/cloud.ts b/packages/keyring-eth-money-mpc/src/cloud.ts index b042f8273..7eb743f5f 100644 --- a/packages/keyring-eth-money-mpc/src/cloud.ts +++ b/packages/keyring-eth-money-mpc/src/cloud.ts @@ -1,9 +1,11 @@ import type { PartyId } from '@metamask/mfa-wallet-interface'; +import type { MoneyAccountSignatureRequest } from '@metamask/money-account-mfa-whitelist'; import type { Infer, Struct } from '@metamask/superstruct'; import { exactOptional, integer, is, + literal, min, nonempty, sensitive, @@ -12,6 +14,8 @@ import { } from '@metamask/superstruct'; import { base64ToBytes, bytesToBase64 } from '@metamask/utils'; +import { MFA_REQUIRED_CODE } from './types.js'; + export type LoadKeyShareBackupResult = { encryptedKeyShare: Uint8Array; epoch: number; @@ -252,16 +256,47 @@ export async function registerClient(opts: { ); } +const signErrorPrefix = 'Failed to initialize cloud sign session'; + +const MfaRequiredBodyStruct = type({ + code: literal(MFA_REQUIRED_CODE), +}); + +export type SignResult = 'ok' | typeof MFA_REQUIRED_CODE; + +/** + * Read a JSON body, or `undefined` when the body is empty or not JSON. + * + * @param response - The fetch response. + * @returns The parsed body, or `undefined`. + */ +async function readJsonBody(response: Response): Promise { + try { + const text = await response.text(); + if (text.length === 0) { + return undefined; + } + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +} + /** * Start a backend signing session for the active share epoch. * + * A response body of `{ "code": "mfa_required" }` means the backend still + * requires MFA. Every other non-OK response is an error. + * * @param opts - Request options. * @param opts.baseURL - MPC backend base URL. - * @param opts.token - Profile token with 2FA and challenge. + * @param opts.token - Profile token. 2FA is included when the client requires MFA. * @param opts.data - Message hash to sign. * @param opts.clientNetId - Client network id. * @param opts.nonce - Client session nonce. * @param opts.shareEpoch - Local share epoch (must equal backend activeEpoch). + * @param opts.request - Signature request the hash was computed from. + * @returns `ok` when the session starts, or `mfa_required` when the backend requires MFA. */ export async function sign(opts: { baseURL: string; @@ -270,18 +305,35 @@ export async function sign(opts: { clientNetId: PartyId; nonce: string; shareEpoch: number; -}): Promise { - await postJson( + request?: MoneyAccountSignatureRequest; +}): Promise { + const response = await fetchWithTimeout( `${opts.baseURL}/sign`, - opts.token, { - data: bytesToBase64(opts.data), - clientNetId: opts.clientNetId, - nonce: opts.nonce, - shareEpoch: opts.shareEpoch, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${opts.token}`, + }, + body: JSON.stringify({ + data: bytesToBase64(opts.data), + clientNetId: opts.clientNetId, + nonce: opts.nonce, + shareEpoch: opts.shareEpoch, + ...(opts.request === undefined ? {} : { request: opts.request }), + }), }, - 'Failed to initialize cloud sign session', + signErrorPrefix, ); + + const body = await readJsonBody(response); + if (is(body, MfaRequiredBodyStruct)) { + return MFA_REQUIRED_CODE; + } + if (!response.ok) { + throw new Error(`${signErrorPrefix}: ${response.statusText}`); + } + return 'ok'; } /** diff --git a/packages/keyring-eth-money-mpc/src/index.ts b/packages/keyring-eth-money-mpc/src/index.ts index 473fdb6ef..3e5f37766 100644 --- a/packages/keyring-eth-money-mpc/src/index.ts +++ b/packages/keyring-eth-money-mpc/src/index.ts @@ -1,6 +1,7 @@ export { MpcMoneyKeyring } from './mpc-money-keyring.js'; -export { MpcMoneyKeyringSetupMode } from './types.js'; +export { MFA_REQUIRED_CODE, MpcMoneyKeyringSetupMode } from './types.js'; +export type { MfaWhitelistConfig } from '@metamask/money-account-mfa-whitelist'; export type { MpcMoneyKeyringOpts, MpcMoneyKeyringSerializer, diff --git a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.test.ts b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.test.ts index 1dbe10f8c..1632574bf 100644 --- a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.test.ts +++ b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.test.ts @@ -1,9 +1,11 @@ /* eslint-disable @typescript-eslint/explicit-function-return-type */ import { hashPersonalMessage } from '@ethereumjs/util'; +import type { MfaWhitelistConfig } from '@metamask/money-account-mfa-whitelist'; import { bytesToHex } from '@metamask/utils'; import type { Hex, Json } from '@metamask/utils'; import { MpcMoneyKeyring } from './mpc-money-keyring.js'; +import { MFA_REQUIRED_CODE } from './types.js'; const mockCreateKey = jest.fn(); const mockRotateKeyShares = jest.fn(); @@ -25,6 +27,7 @@ const mockStoreKeyShareBackup = jest.fn(); const mockCheckKeyShare = jest.fn(); const mockSetActiveEpoch = jest.fn(); const mockLoadKeyShareBackup = jest.fn(); +const mockGetMfaRequirement = jest.fn(); let lastNetworkManagerOptions: Record | undefined; const mockDerivedAddress = '0x1111111111111111111111111111111111111111' as Hex; @@ -110,6 +113,10 @@ jest.mock('@metamask/mfa-wallet-dkls23', () => { return { Dkls23TssLib: MockDkls23TssLib }; }); +jest.mock('@metamask/money-account-mfa-whitelist', () => ({ + getMfaRequirement: (...args: unknown[]) => mockGetMfaRequirement(...args), +})); + jest.mock('./cloud', () => ({ getNetId: (...args: unknown[]) => mockGetNetId(...args), createKey: (...args: unknown[]) => mockStartCreateKey(...args), @@ -185,9 +192,16 @@ const makeRootSession = (statusPayload = 'data persisted') => { return session; }; +const mockWhitelistConfig = { + chainId: '0x8f', +} as unknown as MfaWhitelistConfig; + const makeKeyring = ( getProfileToken = jest.fn().mockResolvedValue('token'), getBackupEncryptionKey = jest.fn().mockResolvedValue(mockBackupKey), + getMfaWhitelistConfig?: () => + | MfaWhitelistConfig + | Promise, ) => new MpcMoneyKeyring({ cl24Lib: {} as never, @@ -196,6 +210,7 @@ const makeKeyring = ( relayerURL: 'https://relayer.example', getProfileToken, getBackupEncryptionKey, + ...(getMfaWhitelistConfig === undefined ? {} : { getMfaWhitelistConfig }), }); const deserializeState = async ( @@ -233,7 +248,11 @@ describe('MpcMoneyKeyring', () => { mockGetNetId.mockResolvedValue('cloud-user'); mockStartCreateKey.mockResolvedValue(undefined); mockRegisterClient.mockResolvedValue(undefined); - mockStartSign.mockResolvedValue(undefined); + mockStartSign.mockResolvedValue('ok'); + mockGetMfaRequirement.mockReturnValue({ + mfaRequired: true, + reason: 'test', + }); mockStartRotateKeyShares.mockResolvedValue(undefined); mockStoreKeyShareBackup.mockResolvedValue(undefined); mockCheckKeyShare.mockResolvedValue({ @@ -709,16 +728,23 @@ describe('MpcMoneyKeyring', () => { messageHex, ); expect(signatureHex).toBe(bytesToHex(mockEthSignature)); + const messageHash = hashPersonalMessage(new TextEncoder().encode('hello')); expect(getProfileToken).toHaveBeenCalledWith({ twoFactor: true, - challenge: hashPersonalMessage(new TextEncoder().encode('hello')), + challenge: messageHash, }); + expect(mockGetMfaRequirement).not.toHaveBeenCalled(); expect(mockStartSign).toHaveBeenCalledWith( expect.objectContaining({ clientNetId: 'local-user', token: 'token', - data: hashPersonalMessage(new TextEncoder().encode('hello')), + data: messageHash, shareEpoch: 1, + request: { + method: 'signPersonalMessage', + address: mockDerivedAddress, + message: messageHex, + }, }), ); expect(mockDklsSetup).not.toHaveBeenCalled(); @@ -798,6 +824,11 @@ describe('MpcMoneyKeyring', () => { token: 'token', data: expectedHash, shareEpoch: 1, + request: { + method: 'signEip7702Authorization', + address: mockDerivedAddress, + authorization, + }, }), ); expect(mockDklsSign).toHaveBeenCalledWith( @@ -808,6 +839,204 @@ describe('MpcMoneyKeyring', () => { ); }); + it('skips MFA for a whitelisted signature request', async () => { + const getProfileToken = jest.fn().mockResolvedValue('token'); + const getMfaWhitelistConfig = jest + .fn() + .mockResolvedValue(mockWhitelistConfig); + const keyring = makeKeyring( + getProfileToken, + undefined, + getMfaWhitelistConfig, + ); + await deserializeState(keyring); + mockCreateSession.mockResolvedValue(makeRootSession('signing completed')); + mockGetMfaRequirement.mockReturnValue({ + mfaRequired: false, + rule: 'chomp-authentication', + }); + + const messageHex = '0x68656c6c6f'; + const messageHash = hashPersonalMessage(new TextEncoder().encode('hello')); + expect( + await keyring.signPersonalMessage(mockDerivedAddress, messageHex), + ).toBe(bytesToHex(mockEthSignature)); + + expect(mockGetMfaRequirement).toHaveBeenCalledWith( + { + method: 'signPersonalMessage', + address: mockDerivedAddress, + message: messageHex, + }, + { + address: mockDerivedAddress, + hash: bytesToHex(messageHash), + config: mockWhitelistConfig, + }, + ); + expect(getProfileToken).toHaveBeenCalledTimes(1); + expect(getProfileToken).toHaveBeenCalledWith({ + twoFactor: false, + challenge: messageHash, + }); + expect(mockStartSign).toHaveBeenCalledTimes(1); + expect(mockStartSign).toHaveBeenCalledWith( + expect.objectContaining({ + token: 'token', + request: { + method: 'signPersonalMessage', + address: mockDerivedAddress, + message: messageHex, + }, + }), + ); + }); + + it('prompts for MFA when the backend returns mfa_required', async () => { + const getProfileToken = jest + .fn() + .mockResolvedValueOnce('session-token') + .mockResolvedValueOnce('mfa-token'); + const keyring = makeKeyring( + getProfileToken, + undefined, + jest.fn().mockReturnValue(mockWhitelistConfig), + ); + await deserializeState(keyring); + mockCreateSession.mockResolvedValue(makeRootSession('signing completed')); + mockGetMfaRequirement.mockReturnValue({ + mfaRequired: false, + rule: 'chomp-authentication', + }); + mockStartSign + .mockResolvedValueOnce(MFA_REQUIRED_CODE) + .mockResolvedValueOnce('ok'); + const mockedUtil = jest.requireMock('./util'); + mockedUtil.generateSessionNonce + .mockReturnValueOnce('0x1111') + .mockReturnValueOnce('0x2222'); + + const messageHash = hashPersonalMessage(new TextEncoder().encode('hello')); + expect( + await keyring.signPersonalMessage(mockDerivedAddress, '0x68656c6c6f'), + ).toBe(bytesToHex(mockEthSignature)); + + expect(getProfileToken).toHaveBeenNthCalledWith(1, { + twoFactor: false, + challenge: messageHash, + }); + expect(getProfileToken).toHaveBeenNthCalledWith(2, { + twoFactor: true, + challenge: messageHash, + }); + expect(mockStartSign).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + token: 'session-token', + nonce: '0x1111', + request: expect.objectContaining({ method: 'signPersonalMessage' }), + }), + ); + expect(mockStartSign).toHaveBeenNthCalledWith( + 2, + expect.not.objectContaining({ + request: expect.anything(), + }), + ); + expect(mockStartSign).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + token: 'mfa-token', + nonce: '0x2222', + }), + ); + expect(mockCreateSession).toHaveBeenCalledWith( + expect.anything(), + 'session:cloud-user|local-user:0x2222', + ); + }); + + it('requires MFA when whitelist configuration throws', async () => { + const getProfileToken = jest.fn().mockResolvedValue('token'); + const keyring = makeKeyring( + getProfileToken, + undefined, + jest.fn().mockRejectedValue(new Error('config unavailable')), + ); + await deserializeState(keyring); + mockCreateSession.mockResolvedValue(makeRootSession('signing completed')); + + await keyring.signPersonalMessage(mockDerivedAddress, '0x68656c6c6f'); + + expect(mockGetMfaRequirement).not.toHaveBeenCalled(); + expect(getProfileToken).toHaveBeenCalledWith({ + twoFactor: true, + challenge: hashPersonalMessage(new TextEncoder().encode('hello')), + }); + }); + + it('requires MFA when the whitelist check throws', async () => { + const getProfileToken = jest.fn().mockResolvedValue('token'); + const keyring = makeKeyring( + getProfileToken, + undefined, + jest.fn().mockReturnValue(mockWhitelistConfig), + ); + await deserializeState(keyring); + mockCreateSession.mockResolvedValue(makeRootSession('signing completed')); + mockGetMfaRequirement.mockImplementation(() => { + throw new Error('whitelist failed'); + }); + + await keyring.signPersonalMessage(mockDerivedAddress, '0x68656c6c6f'); + + expect(getProfileToken).toHaveBeenCalledWith({ + twoFactor: true, + challenge: hashPersonalMessage(new TextEncoder().encode('hello')), + }); + }); + + it('throws when the backend requires MFA for a 2FA token', async () => { + const getProfileToken = jest.fn().mockResolvedValue('token'); + const keyring = makeKeyring(getProfileToken); + await deserializeState(keyring); + mockStartSign.mockResolvedValue(MFA_REQUIRED_CODE); + + await expect( + keyring.signPersonalMessage(mockDerivedAddress, '0x68656c6c6f'), + ).rejects.toThrow('MFA required'); + + expect(getProfileToken).toHaveBeenCalledTimes(1); + expect(mockStartSign).toHaveBeenCalledTimes(1); + expect(mockCreateSession).not.toHaveBeenCalled(); + }); + + it('throws when the MFA retry is still rejected', async () => { + const getProfileToken = jest.fn().mockResolvedValue('token'); + const keyring = makeKeyring( + getProfileToken, + undefined, + jest.fn().mockReturnValue(mockWhitelistConfig), + ); + await deserializeState(keyring); + mockGetMfaRequirement.mockReturnValue({ + mfaRequired: false, + rule: 'chomp-authentication', + }); + mockStartSign.mockResolvedValue(MFA_REQUIRED_CODE); + + await expect( + keyring.signPersonalMessage(mockDerivedAddress, '0x68656c6c6f'), + ).rejects.toThrow('MFA required'); + + expect(getProfileToken).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ twoFactor: true }), + ); + expect(mockStartSign).toHaveBeenCalledTimes(2); + expect(mockCreateSession).not.toHaveBeenCalled(); + }); + it('runs TSS setup when the peer has discarded setup', async () => { const keyring = makeKeyring(); await deserializeState(keyring); diff --git a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts index 9d43cf6fe..5eb6d08a0 100644 --- a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts +++ b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts @@ -21,6 +21,8 @@ import { CentrifugeNetworkManager, createScopedSessionId, } from '@metamask/mfa-wallet-network'; +import { getMfaRequirement } from '@metamask/money-account-mfa-whitelist'; +import type { MoneyAccountSignatureRequest } from '@metamask/money-account-mfa-whitelist'; import { enums, integer, @@ -46,6 +48,7 @@ import { sign as startSign, storeKeyShareBackup, } from './cloud.js'; +import { MFA_REQUIRED_CODE } from './types.js'; import type { MpcMoneyKeyring as MpcMoneyKeyringContract, MpcMoneyKeyringOpts, @@ -238,6 +241,8 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { readonly #getProfileToken: (opts?: ProfileTokenOpts) => Promise; + readonly #getMfaWhitelistConfig: MpcMoneyKeyringOpts['getMfaWhitelistConfig']; + readonly #getBackupEncryptionKey: () => Promise; #opQueue: Promise = Promise.resolve(); @@ -266,6 +271,7 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { ...(opts.webSocket === undefined ? {} : { websocket: opts.webSocket }), }); this.#getProfileToken = opts.getProfileToken; + this.#getMfaWhitelistConfig = opts.getMfaWhitelistConfig; this.#getBackupEncryptionKey = opts.getBackupEncryptionKey; } @@ -554,7 +560,11 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { const rawMsg = hexToBytes(msgHex); const msgHash = hashPersonalMessage(rawMsg); - const signature = await this.#signHash(address, msgHash); + const signature = await this.#signHash(address, msgHash, { + method: 'signPersonalMessage', + address, + message: msgHex as Hex, + }); return bytesToHex(signature); } @@ -580,7 +590,12 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { const messageHash = getSignedTypedDataHash(data, version); - const signature = await this.#signHash(address, messageHash); + const signature = await this.#signHash(address, messageHash, { + method: 'signTypedData', + address, + version, + data, + }); return bytesToHex(signature); } @@ -598,7 +613,12 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { _opts?: Record, ): Promise { const messageHash = new Uint8Array(hashEIP7702Authorization(authorization)); - const signature = await this.#signHash(address, messageHash); + const [chainId, contractAddress, nonce] = authorization; + const signature = await this.#signHash(address, messageHash, { + method: 'signEip7702Authorization', + address, + authorization: [chainId, contractAddress as Hex, nonce], + }); return bytesToHex(signature); } @@ -713,7 +733,40 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { }); } - async #signHash(address: Hex, hash: Uint8Array): Promise { + /** + * Whether signing `request` requires MFA. + * + * A missing request, missing configuration, or a thrown check requires MFA. + * + * @param request - The signature request, when this operation has one. + * @param hash - The hash the keyring will sign. + * @returns Whether the profile token must include 2FA. + */ + async #mfaRequired( + request: MoneyAccountSignatureRequest | undefined, + hash: Uint8Array, + ): Promise { + if (request === undefined || this.#getMfaWhitelistConfig === undefined) { + return true; + } + + try { + const config = await this.#getMfaWhitelistConfig(); + return getMfaRequirement(request, { + address: this.#address(), + hash: bytesToHex(hash), + config, + }).mfaRequired; + } catch { + return true; + } + } + + async #signHash( + address: Hex, + hash: Uint8Array, + request: MoneyAccountSignatureRequest | undefined, + ): Promise { return this.#serializeOp(async () => { const state = this.#assertState(); const { keyShare, netCreds, serverNetId, shareEpoch } = state; @@ -724,19 +777,42 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { throw new Error(`account ${address} not found`); } - const token = await this.#getProfileToken({ - twoFactor: true, + const mfaRequired = await this.#mfaRequired(request, hash); + let token = await this.#getProfileToken({ + twoFactor: mfaRequired, challenge: hash, }); - const nonce = generateSessionNonce(); - await startSign({ + let nonce = generateSessionNonce(); + let signResult = await startSign({ baseURL: this.#cloudURL, token, data: hash, clientNetId: netCreds.partyId, nonce, shareEpoch, + ...(request === undefined ? {} : { request }), }); + if (signResult === MFA_REQUIRED_CODE) { + if (mfaRequired) { + throw new Error('MFA required'); + } + token = await this.#getProfileToken({ + twoFactor: true, + challenge: hash, + }); + nonce = generateSessionNonce(); + signResult = await startSign({ + baseURL: this.#cloudURL, + token, + data: hash, + clientNetId: netCreds.partyId, + nonce, + shareEpoch, + }); + if (signResult === MFA_REQUIRED_CODE) { + throw new Error('MFA required'); + } + } const netSession = await this.#createNetworkSession( netCreds, diff --git a/packages/keyring-eth-money-mpc/src/types.ts b/packages/keyring-eth-money-mpc/src/types.ts index 10e959a1a..2ac67f5e1 100644 --- a/packages/keyring-eth-money-mpc/src/types.ts +++ b/packages/keyring-eth-money-mpc/src/types.ts @@ -2,6 +2,7 @@ import type { EthKeyring } from '@metamask/keyring-utils'; import type { CL24DKM, CL24ThresholdKey } from '@metamask/mfa-wallet-cl24'; import type { Dkls23TssLib } from '@metamask/mfa-wallet-dkls23'; import type { CentrifugeIdentity } from '@metamask/mfa-wallet-network'; +import type { MfaWhitelistConfig } from '@metamask/money-account-mfa-whitelist'; import type { Json } from '@metamask/utils'; export type ProfileTokenOpts = { @@ -9,6 +10,15 @@ export type ProfileTokenOpts = { challenge?: Uint8Array; }; +/** + * `/sign` response code when the backend still requires MFA. + * + * The keyring prompts for a 2FA token and retries once when it receives this + * after trying to sign without MFA. A second response, or this response to a + * token that already has 2FA, is an error. + */ +export const MFA_REQUIRED_CODE = 'mfa_required' as const; + export type Cl24Lib = ConstructorParameters[0]; export type Dkls23Lib = ConstructorParameters[0]; @@ -19,6 +29,14 @@ export type MpcMoneyKeyringOpts = { relayerURL: string; getTransportToken?: () => Promise; getProfileToken: (opts?: ProfileTokenOpts) => Promise; + /** + * Pinned whitelist configuration for the current signature. + * + * When this is omitted or throws, every signature requires MFA. + */ + getMfaWhitelistConfig?: () => + | MfaWhitelistConfig + | Promise; getBackupEncryptionKey: () => Promise; webSocket?: unknown; }; diff --git a/yarn.config.cjs b/yarn.config.cjs index 875c250d6..f77c43d82 100644 --- a/yarn.config.cjs +++ b/yarn.config.cjs @@ -33,7 +33,10 @@ const ALLOWED_INCONSISTENT_DEPENDENCIES = {}; * * @type {string[]} */ -const PINNED_VERSION_EXCEPTIONS = []; +const PINNED_VERSION_EXCEPTIONS = [ + // File link to the local core checkout until the package is published. + '@metamask/money-account-mfa-whitelist', +]; /** * FIXME: These packages are ESM-only, which is not yet the standard package diff --git a/yarn.lock b/yarn.lock index 219884905..7f7b2b1e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1603,6 +1603,16 @@ __metadata: languageName: node linkType: hard +"@metamask/abi-utils@npm:^2.0.4": + version: 2.0.4 + resolution: "@metamask/abi-utils@npm:2.0.4" + dependencies: + "@metamask/superstruct": "npm:^3.1.0" + "@metamask/utils": "npm:^9.0.0" + checksum: 10/3d32d42c6e98fc4719b2b53597e573764b80936c7cc31d884c87729c4c4f74a30e93096db87aaa7cbcec9d3bb7d22b1adfc98a8bcb4c7c2f17bfbddaa4367d34 + languageName: node + linkType: hard + "@metamask/abi-utils@npm:^3.0.0": version: 3.0.0 resolution: "@metamask/abi-utils@npm:3.0.0" @@ -1839,6 +1849,17 @@ __metadata: languageName: node linkType: hard +"@metamask/delegation-core@npm:^2.2.1": + version: 2.2.1 + resolution: "@metamask/delegation-core@npm:2.2.1" + dependencies: + "@metamask/abi-utils": "npm:^3.0.0" + "@metamask/utils": "npm:^11.4.0" + "@noble/hashes": "npm:^1.8.0" + checksum: 10/8cc532e4a5fdc83eddb36fada6283d51629272fcce8b7c7c2d3b4addde5aff7c741365a3bcd701364460fbbcbbe94fccc0460ee3ef9af284a82f84ca708f0e06 + languageName: node + linkType: hard + "@metamask/eslint-config-jest@npm:^15.0.0": version: 15.0.0 resolution: "@metamask/eslint-config-jest@npm:15.0.0" @@ -2020,6 +2041,7 @@ __metadata: "@metamask/mfa-wallet-dkls23": "npm:^0.1.0" "@metamask/mfa-wallet-interface": "npm:^0.1.0" "@metamask/mfa-wallet-network": "npm:^0.1.0" + "@metamask/money-account-mfa-whitelist": "file:../../../../core/packages/money-account-mfa-whitelist" "@metamask/superstruct": "npm:^3.4.1" "@metamask/utils": "npm:^11.11.0" "@ts-bridge/cli": "npm:^0.6.3" @@ -2569,6 +2591,18 @@ __metadata: languageName: node linkType: hard +"@metamask/money-account-mfa-whitelist@file:../../../../core/packages/money-account-mfa-whitelist::locator=%40metamask%2Feth-money-mpc-keyring%40workspace%3Apackages%2Fkeyring-eth-money-mpc": + version: 0.0.0 + resolution: "@metamask/money-account-mfa-whitelist@file:../../../../core/packages/money-account-mfa-whitelist#../../../../core/packages/money-account-mfa-whitelist::hash=cc59ba&locator=%40metamask%2Feth-money-mpc-keyring%40workspace%3Apackages%2Fkeyring-eth-money-mpc" + dependencies: + "@metamask/abi-utils": "npm:^2.0.4" + "@metamask/delegation-core": "npm:^2.2.1" + "@metamask/utils": "npm:^12.0.0" + "@noble/hashes": "npm:^1.8.0" + checksum: 10/d3e4e71aa9afca4b9e43981cee35e5cba0c54099e7e76b53201cd4cca81f7c13bbc54f743b838f605a8d61f5533d1b702fd17c7b13b2a337d3566f3b6e8ee4aa + languageName: node + linkType: hard + "@metamask/mpc-interface@npm:^0.2.0": version: 0.2.0 resolution: "@metamask/mpc-interface@npm:0.2.0" @@ -2897,6 +2931,26 @@ __metadata: languageName: node linkType: hard +"@metamask/utils@npm:^12.0.0": + version: 12.0.0 + resolution: "@metamask/utils@npm:12.0.0" + dependencies: + "@ethereumjs/tx": "npm:^5.4.0" + "@metamask/scure-bip39": "npm:^2.1.1" + "@metamask/superstruct": "npm:^3.4.1" + "@noble/hashes": "npm:^1.8.0" + "@scure/base": "npm:^1.2.6" + "@types/debug": "npm:^4.1.7" + "@types/lodash": "npm:^4.17.20" + debug: "npm:^4.3.4" + lodash: "npm:^4.17.21" + pony-cause: "npm:^2.1.10" + semver: "npm:^7.6.3" + uuid: "npm:^9.0.1" + checksum: 10/3a2c592283b081ac627262d903bbac84d7ce81896c86e5839ffee4c5cbc48b161cc1599c4b57f74bcc576d8b27bb517ced4c9e4e7b31e69e3113873acfde5f03 + languageName: node + linkType: hard + "@metamask/utils@npm:^9.0.0": version: 9.2.1 resolution: "@metamask/utils@npm:9.2.1" @@ -3510,7 +3564,7 @@ __metadata: languageName: node linkType: hard -"@scure/base@npm:^1.0.0, @scure/base@npm:^1.1.1, @scure/base@npm:^1.1.3, @scure/base@npm:~1.2.5": +"@scure/base@npm:^1.0.0, @scure/base@npm:^1.1.1, @scure/base@npm:^1.1.3, @scure/base@npm:^1.2.6, @scure/base@npm:~1.2.5": version: 1.2.6 resolution: "@scure/base@npm:1.2.6" checksum: 10/c1a7bd5e0b0c8f94c36fbc220f4a67cc832b00e2d2065c7d8a404ed81ab1c94c5443def6d361a70fc382db3496e9487fb9941728f0584782b274c18a4bed4187 From e879f79ff962c11b09b6e3afbab124ef781ce596 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Tue, 13 Oct 2026 21:11:34 +0200 Subject: [PATCH 2/5] chore(keyring-eth-money-mpc): use the MFA whitelist preview build Co-authored-by: Cursor --- packages/keyring-eth-money-mpc/package.json | 2 +- yarn.config.cjs | 5 +---- yarn.lock | 10 +++++----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/keyring-eth-money-mpc/package.json b/packages/keyring-eth-money-mpc/package.json index a9c4e2cdb..8589109db 100644 --- a/packages/keyring-eth-money-mpc/package.json +++ b/packages/keyring-eth-money-mpc/package.json @@ -60,7 +60,7 @@ "@metamask/mfa-wallet-cl24": "^0.1.0", "@metamask/mfa-wallet-dkls23": "^0.1.0", "@metamask/mfa-wallet-network": "^0.1.0", - "@metamask/money-account-mfa-whitelist": "file:../../../../core/packages/money-account-mfa-whitelist", + "@metamask/money-account-mfa-whitelist": "npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055", "@metamask/superstruct": "^3.4.1", "@metamask/utils": "^11.11.0" }, diff --git a/yarn.config.cjs b/yarn.config.cjs index f77c43d82..875c250d6 100644 --- a/yarn.config.cjs +++ b/yarn.config.cjs @@ -33,10 +33,7 @@ const ALLOWED_INCONSISTENT_DEPENDENCIES = {}; * * @type {string[]} */ -const PINNED_VERSION_EXCEPTIONS = [ - // File link to the local core checkout until the package is published. - '@metamask/money-account-mfa-whitelist', -]; +const PINNED_VERSION_EXCEPTIONS = []; /** * FIXME: These packages are ESM-only, which is not yet the standard package diff --git a/yarn.lock b/yarn.lock index 7f7b2b1e8..0e6bc2ee7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2041,7 +2041,7 @@ __metadata: "@metamask/mfa-wallet-dkls23": "npm:^0.1.0" "@metamask/mfa-wallet-interface": "npm:^0.1.0" "@metamask/mfa-wallet-network": "npm:^0.1.0" - "@metamask/money-account-mfa-whitelist": "file:../../../../core/packages/money-account-mfa-whitelist" + "@metamask/money-account-mfa-whitelist": "npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055" "@metamask/superstruct": "npm:^3.4.1" "@metamask/utils": "npm:^11.11.0" "@ts-bridge/cli": "npm:^0.6.3" @@ -2591,15 +2591,15 @@ __metadata: languageName: node linkType: hard -"@metamask/money-account-mfa-whitelist@file:../../../../core/packages/money-account-mfa-whitelist::locator=%40metamask%2Feth-money-mpc-keyring%40workspace%3Apackages%2Fkeyring-eth-money-mpc": - version: 0.0.0 - resolution: "@metamask/money-account-mfa-whitelist@file:../../../../core/packages/money-account-mfa-whitelist#../../../../core/packages/money-account-mfa-whitelist::hash=cc59ba&locator=%40metamask%2Feth-money-mpc-keyring%40workspace%3Apackages%2Fkeyring-eth-money-mpc" +"@metamask/money-account-mfa-whitelist@npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055": + version: 0.0.0-preview-6ccf7a055 + resolution: "@metamask-previews/money-account-mfa-whitelist@npm:0.0.0-preview-6ccf7a055" dependencies: "@metamask/abi-utils": "npm:^2.0.4" "@metamask/delegation-core": "npm:^2.2.1" "@metamask/utils": "npm:^12.0.0" "@noble/hashes": "npm:^1.8.0" - checksum: 10/d3e4e71aa9afca4b9e43981cee35e5cba0c54099e7e76b53201cd4cca81f7c13bbc54f743b838f605a8d61f5533d1b702fd17c7b13b2a337d3566f3b6e8ee4aa + checksum: 10/f9d944284313c4f0d384abf1a3f6bdab022d1e8cbf4d5b4b3c96fde02527c3e219b401b66ae0247bce64a624a4fc6d350467b3daa7785b61e055a4148e165378 languageName: node linkType: hard From cb6d1ff6da4e1788d493ca37141471ad3d8d9d00 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 15 Oct 2026 23:02:44 +0200 Subject: [PATCH 3/5] replace preview package --- packages/keyring-eth-money-mpc/package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/keyring-eth-money-mpc/package.json b/packages/keyring-eth-money-mpc/package.json index 8589109db..b0700fe66 100644 --- a/packages/keyring-eth-money-mpc/package.json +++ b/packages/keyring-eth-money-mpc/package.json @@ -60,7 +60,7 @@ "@metamask/mfa-wallet-cl24": "^0.1.0", "@metamask/mfa-wallet-dkls23": "^0.1.0", "@metamask/mfa-wallet-network": "^0.1.0", - "@metamask/money-account-mfa-whitelist": "npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055", + "@metamask/money-account-mfa-whitelist": "^0.1.0", "@metamask/superstruct": "^3.4.1", "@metamask/utils": "^11.11.0" }, diff --git a/yarn.lock b/yarn.lock index 0e6bc2ee7..18856dcf6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2041,7 +2041,7 @@ __metadata: "@metamask/mfa-wallet-dkls23": "npm:^0.1.0" "@metamask/mfa-wallet-interface": "npm:^0.1.0" "@metamask/mfa-wallet-network": "npm:^0.1.0" - "@metamask/money-account-mfa-whitelist": "npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055" + "@metamask/money-account-mfa-whitelist": "npm:^0.1.0" "@metamask/superstruct": "npm:^3.4.1" "@metamask/utils": "npm:^11.11.0" "@ts-bridge/cli": "npm:^0.6.3" @@ -2591,15 +2591,15 @@ __metadata: languageName: node linkType: hard -"@metamask/money-account-mfa-whitelist@npm:@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055": - version: 0.0.0-preview-6ccf7a055 - resolution: "@metamask-previews/money-account-mfa-whitelist@npm:0.0.0-preview-6ccf7a055" +"@metamask/money-account-mfa-whitelist@npm:^0.1.0": + version: 0.1.0 + resolution: "@metamask/money-account-mfa-whitelist@npm:0.1.0" dependencies: "@metamask/abi-utils": "npm:^2.0.4" "@metamask/delegation-core": "npm:^2.2.1" "@metamask/utils": "npm:^12.0.0" "@noble/hashes": "npm:^1.8.0" - checksum: 10/f9d944284313c4f0d384abf1a3f6bdab022d1e8cbf4d5b4b3c96fde02527c3e219b401b66ae0247bce64a624a4fc6d350467b3daa7785b61e055a4148e165378 + checksum: 10/73cbe3e723b0aff74b649b693fd95fd81da053f930ab882fc92a889d73faf876808bf84732ff989a23589f1495c1ab90da38084028b3ebd9d271e467a90e23cb languageName: node linkType: hard From 605c30c2fd5c7eb17e7be62617d4e32645192cd7 Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Thu, 15 Oct 2026 23:03:47 +0200 Subject: [PATCH 4/5] fill pr number --- packages/keyring-eth-money-mpc/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/keyring-eth-money-mpc/CHANGELOG.md b/packages/keyring-eth-money-mpc/CHANGELOG.md index 15412cf71..cea36d8f0 100644 --- a/packages/keyring-eth-money-mpc/CHANGELOG.md +++ b/packages/keyring-eth-money-mpc/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Skip MFA for whitelisted signature requests, and retry with a 2FA token when the backend returns `mfa_required` ([#TODO](https://github.com/MetaMask/accounts/pull/TODO)) +- Skip MFA for whitelisted signature requests, and retry with a 2FA token when the backend returns `mfa_required` ([#670](https://github.com/MetaMask/accounts/pull/670)) ## [0.1.0] From 662cf19537effd2fc2349fe2e63de95f167d608d Mon Sep 17 00:00:00 2001 From: Matthias Geihs Date: Fri, 16 Oct 2026 15:45:25 +0200 Subject: [PATCH 5/5] refactor(keyring-eth-money-mpc): always send the signature request Every sign method already has a request, so the omit path was unreachable and dropped branch coverage below 100%. Co-authored-by: Cursor --- .../keyring-eth-money-mpc/src/mpc-money-keyring.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts index 5eb6d08a0..d62bafa3b 100644 --- a/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts +++ b/packages/keyring-eth-money-mpc/src/mpc-money-keyring.ts @@ -736,17 +736,17 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { /** * Whether signing `request` requires MFA. * - * A missing request, missing configuration, or a thrown check requires MFA. + * Missing configuration, or a thrown check, requires MFA. * - * @param request - The signature request, when this operation has one. + * @param request - The signature request the hash was computed from. * @param hash - The hash the keyring will sign. * @returns Whether the profile token must include 2FA. */ async #mfaRequired( - request: MoneyAccountSignatureRequest | undefined, + request: MoneyAccountSignatureRequest, hash: Uint8Array, ): Promise { - if (request === undefined || this.#getMfaWhitelistConfig === undefined) { + if (this.#getMfaWhitelistConfig === undefined) { return true; } @@ -765,7 +765,7 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { async #signHash( address: Hex, hash: Uint8Array, - request: MoneyAccountSignatureRequest | undefined, + request: MoneyAccountSignatureRequest, ): Promise { return this.#serializeOp(async () => { const state = this.#assertState(); @@ -790,7 +790,7 @@ export class MpcMoneyKeyring implements MpcMoneyKeyringContract { clientNetId: netCreds.partyId, nonce, shareEpoch, - ...(request === undefined ? {} : { request }), + request, }); if (signResult === MFA_REQUIRED_CODE) { if (mfaRequired) {