From 184e8c8038dce7bd0dbfa67f4d387e55fb8a00dc Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 16 Sep 2026 22:53:56 +0200 Subject: [PATCH 1/2] feat(profile-sync): add MFA step-up sessions to AuthenticationController --- packages/profile-sync-controller/CHANGELOG.md | 1 + packages/profile-sync-controller/README.md | 17 + ...nticationController-method-action-types.ts | 50 ++ .../AuthenticationController.test.ts | 583 +++++++++++++++++- .../AuthenticationController.ts | 245 +++++++- .../src/controllers/authentication/index.ts | 4 + 6 files changed, 888 insertions(+), 12 deletions(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 8a097e0d66c..cf95f99e568 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `beginStepUp`, `completeStepUp`, `getElevatedProfileToken` and `clearStepUpSession` to `AuthenticationController`, holding the elevated token in memory only behind a hard-expiring `stepUpSessionExpiresAt` state and a `stepUpSessionTtlMs` config ([#10267](https://github.com/MetaMask/core/pull/10267)) - Add `refreshEnrolledCredentials`, `beginCredentialEnrollment` and `completeCredentialEnrollment` to `AuthenticationController`, backed by a memory-only `enrolledCredentials` state and an optional `trace` callback ([#10266](https://github.com/MetaMask/core/pull/10266)) - Add passkey and email OTP enrollment, verification, credential-list, and elevated-token exchange SDK methods ([#10265](https://github.com/MetaMask/core/pull/10265)) - Add validated MFA domain types and structured `MfaError` classes with a serialization-safe `mfaCode` ([#10264](https://github.com/MetaMask/core/pull/10264)) diff --git a/packages/profile-sync-controller/README.md b/packages/profile-sync-controller/README.md index ff8a3d40487..86320a78183 100644 --- a/packages/profile-sync-controller/README.md +++ b/packages/profile-sync-controller/README.md @@ -42,6 +42,23 @@ import { ... } from '@metamask/profile-sync-controller/auth/mocks' import { ... } from '@metamask/profile-sync-controller/user-storage/mocks' ``` +## Multi-factor authentication + +`AuthenticationController` exposes UI-independent primitives for passkey and +email OTP enrollment and step-up verification: + +- `refreshEnrolledCredentials()` refreshes the in-memory credential list. +- `beginCredentialEnrollment()` and `completeCredentialEnrollment()` surround + a client-owned passkey ceremony or email-code screen. +- `beginStepUp()` and `completeStepUp()` verify an enrolled credential and + return an elevated profile token. +- `getElevatedProfileToken()` reuses a live elevated session when it satisfies + the caller's freshness requirement; `clearStepUpSession()` clears it. + +Clients must retain the challenge `flowId`, perform the platform ceremony, and +send the resulting proof to the matching completion method. OTP codes, +passkey results, and elevated tokens are never persisted in controller state. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController-method-action-types.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController-method-action-types.ts index 824c2a5ed7a..ae6cbbb8489 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController-method-action-types.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController-method-action-types.ts @@ -60,6 +60,52 @@ export type AuthenticationControllerCompleteCredentialEnrollmentAction = { handler: AuthenticationController['completeCredentialEnrollment']; }; +/** + * Begins step-up verification with an enrolled credential. + * + * @param request - Credential type and trace reason. + * @returns A challenge for the client-owned ceremony. + */ +export type AuthenticationControllerBeginStepUpAction = { + type: `AuthenticationController:beginStepUp`; + handler: AuthenticationController['beginStepUp']; +}; + +/** + * Completes step-up verification and opens a short-lived elevated session. + * + * The AAL2 assertion returned by the MFA service is exchanged at Hydra for + * an elevated access token, whose claims are checked before the session + * opens. The token itself never enters controller state. + * + * @param request - Flow identifier, platform or email proof, and trace reason. + * @returns The elevated profile access token. + */ +export type AuthenticationControllerCompleteStepUpAction = { + type: `AuthenticationController:completeStepUp`; + handler: AuthenticationController['completeStepUp']; +}; + +/** + * Returns the active elevated token when it meets the requested freshness. + * + * @param request - Optional maximum session age in milliseconds, measured + * from when the token was obtained. Zero always requires a new ceremony. + * @returns A live elevated token, or null when no reusable session exists. + */ +export type AuthenticationControllerGetElevatedProfileTokenAction = { + type: `AuthenticationController:getElevatedProfileToken`; + handler: AuthenticationController['getElevatedProfileToken']; +}; + +/** + * Clears the in-memory elevated session and its expiration timer. + */ +export type AuthenticationControllerClearStepUpSessionAction = { + type: `AuthenticationController:clearStepUpSession`; + handler: AuthenticationController['clearStepUpSession']; +}; + export type AuthenticationControllerPerformSignOutAction = { type: `AuthenticationController:performSignOut`; handler: AuthenticationController['performSignOut']; @@ -182,6 +228,10 @@ export type AuthenticationControllerMethodActions = | AuthenticationControllerRefreshEnrolledCredentialsAction | AuthenticationControllerBeginCredentialEnrollmentAction | AuthenticationControllerCompleteCredentialEnrollmentAction + | AuthenticationControllerBeginStepUpAction + | AuthenticationControllerCompleteStepUpAction + | AuthenticationControllerGetElevatedProfileTokenAction + | AuthenticationControllerClearStepUpSessionAction | AuthenticationControllerPerformSignOutAction | AuthenticationControllerClearStateAction | AuthenticationControllerGetBearerTokenAction diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index fa6737243d8..c6630b1a9f0 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -14,7 +14,9 @@ import { EmailRequiredError, Platform } from '../../sdk/index.js'; import { MOCK_ACCESS_JWT, MOCK_MFA_CREDENTIALS_RESPONSE, + MOCK_ELEVATED_ACCESS_TOKEN_RESPONSE, MOCK_MFA_ENROLL_EMAIL_RESPONSE, + MOCK_MFA_VERIFY_COMPLETE_RESPONSE, MOCK_USER_PROFILE_LINEAGE_RESPONSE, } from '../../sdk/mocks/auth.js'; import { @@ -25,10 +27,14 @@ import { mockEndpointMfaCredentials, mockEndpointMfaEnroll, mockEndpointMfaEnrollComplete, + mockEndpointMfaVerify, + mockEndpointMfaVerifyComplete, + mockEndpointAccessToken, } from './__fixtures__/mockServices.js'; import { AuthenticationController, defaultState, + STEP_UP_SESSION_TTL_MS, } from './AuthenticationController.js'; import type { AuthenticationControllerMessenger, @@ -2318,6 +2324,7 @@ describe('AuthenticationController', () => { describe('MFA credential enrollment', () => { function createController(options?: { state?: AuthenticationControllerState; + stepUpSessionTtlMs?: number; trace?: TraceCallback; }): { controller: AuthenticationController; @@ -2329,6 +2336,11 @@ describe('MFA credential enrollment', () => { messenger, metametrics: createMockAuthMetaMetrics(), state: options?.state ?? mockSignedInState(), + config: { + ...(options?.stepUpSessionTtlMs === undefined + ? {} + : { stepUpSessionTtlMs: options.stepUpSessionTtlMs }), + }, trace: options?.trace, }), baseMessenger, @@ -2356,11 +2368,30 @@ describe('MFA credential enrollment', () => { expect(listener).toHaveBeenCalledTimes(1); }); + it('invalidates the cached SRP session when authentication is rejected', async () => { + mockEndpointMfaCredentials({ + status: 401, + body: { message: 'Access token expired' }, + }); + const { controller } = createController(); + + await expect(controller.refreshEnrolledCredentials()).rejects.toMatchObject( + { mfaCode: 'authentication_required' }, + ); + expect( + controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]].profile + .canonicalProfileId, + ).toBe(''); + }); + it('begins passkey enrollment with validated tracing tags', async () => { mockEndpointMfaEnroll(); + const setAttribute = jest.fn(); const trace = jest.fn( - (_request: unknown, fn?: () => unknown): Promise => - Promise.resolve(fn?.()), + ( + _request: unknown, + fn?: (context?: unknown) => unknown, + ): Promise => Promise.resolve(fn?.({ setAttribute })), ) as unknown as TraceCallback; const { controller } = createController({ trace }); @@ -2381,10 +2412,10 @@ describe('MFA credential enrollment', () => { operation: 'settings.addPasskey', credentialType: 'passkey', }, - data: { outcome: 'success' }, }), expect.any(Function), ); + expect(setAttribute).toHaveBeenCalledWith('outcome', 'success'); }); it('begins email enrollment and rejects invalid boundary input', async () => { @@ -2677,6 +2708,46 @@ describe('MFA credential enrollment', () => { expect(controller.state.enrolledCredentials).toStrictEqual([]); }); + it('does not update state if the wallet locks during enrollment completion', async () => { + let resolveCompletion: + | ((response: Awaited>) => void) + | undefined; + let requestStartedResolve: (() => void) | undefined; + const requestStarted = new Promise((resolve) => { + requestStartedResolve = resolve; + }); + const fetchSpy = jest.spyOn(globalThis, 'fetch').mockImplementationOnce( + async (): ReturnType => + await new Promise((resolve) => { + resolveCompletion = resolve; + requestStartedResolve?.(); + }), + ); + const { controller, baseMessenger } = createController(); + try { + const completion = controller.completeCredentialEnrollment({ + flowId: 'flow-id', + proof: { type: 'email_otp', code: '123456' }, + reason: { operation: 'settings.addEmail' }, + }); + await requestStarted; + baseMessenger.publish('KeyringController:lock'); + resolveCompletion?.( + new globalThis.Response(JSON.stringify({ status: 'enrolled' }), { + status: 200, + }), + ); + + await expect(completion).rejects.toThrow( + 'the authenticated session ended', + ); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(controller.state.enrolledCredentials).toStrictEqual([]); + } finally { + fetchSpy.mockRestore(); + } + }); + it('rejects MFA calls while the wallet is locked', async () => { const { controller, baseMessenger } = createController(); baseMessenger.publish('KeyringController:lock'); @@ -2780,6 +2851,512 @@ describe('MFA credential enrollment', () => { }); }); +describe('MFA step-up verification', () => { + const passkeyProof = { + type: 'passkey', + assertion: { + id: 'credential-id', + rawId: 'credential-id', + type: 'public-key', + response: { + authenticatorData: 'authenticator-data', + clientDataJSON: 'client-data', + signature: 'signature', + }, + }, + } as const; + + function createController(options?: { + stepUpSessionTtlMs?: number; + trace?: TraceCallback; + }): { + controller: AuthenticationController; + baseMessenger: RootMessenger; + } { + const { messenger, baseMessenger } = createMockAuthenticationMessenger(); + return { + controller: new AuthenticationController({ + messenger, + metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), + config: { + ...(options?.stepUpSessionTtlMs === undefined + ? {} + : { stepUpSessionTtlMs: options.stepUpSessionTtlMs }), + }, + trace: options?.trace, + }), + baseMessenger, + }; + } + + function mockSuccessfulCompletion(): void { + mockEndpointMfaVerifyComplete(); + mockEndpointAccessToken({ + status: 200, + body: MOCK_ELEVATED_ACCESS_TOKEN_RESPONSE, + }); + } + + async function completeStepUp( + controller: AuthenticationController, + ): Promise { + await controller.completeStepUp({ + flowId: 'flow-id', + proof: passkeyProof, + reason: { operation: 'money.signTransaction' }, + }); + } + + it('begins passkey and email verification without consulting the cache', async () => { + mockEndpointMfaVerify(); + const { controller } = createController(); + + expect( + await controller.beginStepUp({ + type: 'passkey', + reason: { operation: 'money.signTransaction' }, + }), + ).toMatchObject({ + type: 'passkey', + flowId: 'verify-passkey-flow-id', + publicKey: expect.objectContaining({ challenge: expect.any(String) }), + }); + + cleanAllNock(); + mockEndpointMfaVerify({ + status: 200, + body: { + flow_id: 'verify-email-flow-id', + expires_at: '2099-09-07T14:30:00Z', + }, + }); + expect( + await controller.beginStepUp({ + type: 'email_otp', + reason: { operation: 'kalshi.deposit' }, + }), + ).toStrictEqual({ + type: 'email_otp', + flowId: 'verify-email-flow-id', + expiresAt: Date.parse('2099-09-07T14:30:00Z'), + }); + }); + + it('maps a missing server credential to a stable error code', async () => { + mockEndpointMfaVerify({ + status: 409, + body: { + code: 'credential_not_enrolled', + message: 'Credential is not enrolled', + }, + }); + const { controller } = createController(); + + await expect( + controller.beginStepUp({ + type: 'passkey', + reason: { operation: 'money.signTransaction' }, + }), + ).rejects.toMatchObject({ mfaCode: 'credential_not_enrolled' }); + }); + + it('opens and exposes an elevated session after AAL2 exchange', async () => { + mockSuccessfulCompletion(); + const { controller, baseMessenger } = createController(); + const listener = jest.fn(); + baseMessenger.subscribe('AuthenticationController:stateChange', listener); + + const token = await controller.completeStepUp({ + flowId: 'flow-id', + proof: passkeyProof, + reason: { operation: 'money.signTransaction' }, + }); + + expect(token.accessToken).toBe( + MOCK_ELEVATED_ACCESS_TOKEN_RESPONSE.access_token, + ); + expect(token.claims).toStrictEqual({ + sub: 'f88227bd-b615-41a3-b0be-467dd781a4ad', + aal: 2, + amr: ['passkey'], + exp: 4102444800, + }); + expect(controller.getElevatedProfileToken()).toBe(token); + expect(controller.state.stepUpSessionExpiresAt).toBe( + token.obtainedAt + 60_000, + ); + // Exactly one state write, and the token itself never enters state. + expect(listener).toHaveBeenCalledTimes(1); + expect(JSON.stringify(controller.state)).not.toContain(token.accessToken); + }); + + it('clears the session idempotently without spurious state writes', async () => { + mockSuccessfulCompletion(); + const { controller, baseMessenger } = createController(); + await completeStepUp(controller); + const listener = jest.fn(); + baseMessenger.subscribe('AuthenticationController:stateChange', listener); + + controller.clearStepUpSession(); + controller.clearStepUpSession(); + + expect(controller.state.stepUpSessionExpiresAt).toBeUndefined(); + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('does not write state when clearing a session that does not exist', () => { + const { controller, baseMessenger } = createController(); + const listener = jest.fn(); + baseMessenger.subscribe('AuthenticationController:stateChange', listener); + + controller.clearStepUpSession(); + baseMessenger.publish('KeyringController:lock'); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('keeps the default TTL when the config key is passed as undefined', async () => { + mockSuccessfulCompletion(); + const { messenger } = createMockAuthenticationMessenger(); + const controller = new AuthenticationController({ + messenger, + metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), + config: { stepUpSessionTtlMs: undefined }, + }); + + const token = await controller.completeStepUp({ + flowId: 'flow-id', + proof: passkeyProof, + reason: { operation: 'money.signTransaction' }, + }); + + expect(controller.state.stepUpSessionExpiresAt).toBe( + token.obtainedAt + STEP_UP_SESSION_TTL_MS, + ); + }); + + it('traces every verification network step with final outcomes', async () => { + mockEndpointMfaVerify(); + mockSuccessfulCompletion(); + const requests: { name?: string }[] = []; + const setAttribute = jest.fn(); + const trace = jest.fn( + ( + request: { name?: string }, + fn?: (context?: unknown) => unknown, + ): Promise => { + requests.push(request); + return Promise.resolve(fn?.({ setAttribute })); + }, + ) as unknown as TraceCallback; + const { controller } = createController({ trace }); + + await controller.beginStepUp({ + type: 'passkey', + reason: { operation: 'money.signTransaction' }, + }); + await completeStepUp(controller); + + expect(requests.map(({ name }) => name)).toStrictEqual([ + 'MFA Step-Up Begin', + 'MFA Step-Up Complete', + 'MFA Token Exchange', + ]); + expect(requests).toStrictEqual( + requests.map(() => + expect.objectContaining({ + tags: { + operation: 'money.signTransaction', + credentialType: 'passkey', + }, + }), + ), + ); + expect(setAttribute).toHaveBeenCalledTimes(3); + expect(setAttribute).toHaveBeenNthCalledWith(1, 'outcome', 'success'); + expect(setAttribute).toHaveBeenNthCalledWith(2, 'outcome', 'success'); + expect(setAttribute).toHaveBeenNthCalledWith(3, 'outcome', 'success'); + }); + + it('honors caller freshness requirements without clearing the session', async () => { + mockSuccessfulCompletion(); + const { controller } = createController(); + await completeStepUp(controller); + + expect( + controller.getElevatedProfileToken({ maxSessionAgeMs: 0 }), + ).toBeNull(); + expect(controller.getElevatedProfileToken()).not.toBeNull(); + expect( + controller.getElevatedProfileToken({ maxSessionAgeMs: 600_000 }), + ).not.toBeNull(); + expect(() => + controller.getElevatedProfileToken({ maxSessionAgeMs: -1 }), + ).toThrow(/MFA\[invalid_request\]/u); + }); + + it('hard-clears the session at the configured TTL', async () => { + const fetchSpy = jest + .spyOn(globalThis, 'fetch') + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify(MOCK_MFA_VERIFY_COMPLETE_RESPONSE), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify(MOCK_ELEVATED_ACCESS_TOKEN_RESPONSE), + { status: 200 }, + ), + ); + jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] }); + jest.setSystemTime(new Date('2026-09-16T10:00:00Z')); + try { + const { controller } = createController({ + stepUpSessionTtlMs: 1_000, + }); + await completeStepUp(controller); + + expect(controller.getElevatedProfileToken()).not.toBeNull(); + jest.advanceTimersByTime(1_000); + expect(controller.getElevatedProfileToken()).toBeNull(); + expect(controller.state.stepUpSessionExpiresAt).toBeUndefined(); + } finally { + jest.useRealTimers(); + fetchSpy.mockRestore(); + } + }); + + it('hard-clears the session when the elevated token expires first', async () => { + const now = new Date('2026-09-16T10:00:00Z'); + const header = btoa(JSON.stringify({ alg: 'none', typ: 'JWT' })); + const payload = btoa( + JSON.stringify({ + sub: 'profile-id', + aal: 2, + amr: 'passkey', + exp: Math.floor(now.getTime() / 1000) + 1, + }), + ); + const fetchSpy = jest + .spyOn(globalThis, 'fetch') + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify(MOCK_MFA_VERIFY_COMPLETE_RESPONSE), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify({ + access_token: `${header}.${payload}.signature`, + expires_in: 900, + }), + { status: 200 }, + ), + ); + jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] }); + jest.setSystemTime(now); + try { + const { controller } = createController({ + stepUpSessionTtlMs: 60_000, + }); + await completeStepUp(controller); + + expect(controller.state.stepUpSessionExpiresAt).toBe( + now.getTime() + 1_000, + ); + jest.advanceTimersByTime(1_000); + expect(controller.getElevatedProfileToken()).toBeNull(); + } finally { + jest.useRealTimers(); + fetchSpy.mockRestore(); + } + }); + + it('clears stale state when the clock passes expiration before the timer runs', async () => { + const now = new Date('2026-09-16T10:00:00Z'); + const fetchSpy = jest + .spyOn(globalThis, 'fetch') + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify(MOCK_MFA_VERIFY_COMPLETE_RESPONSE), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new globalThis.Response( + JSON.stringify(MOCK_ELEVATED_ACCESS_TOKEN_RESPONSE), + { status: 200 }, + ), + ); + jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] }); + jest.setSystemTime(now); + try { + const { controller } = createController({ + stepUpSessionTtlMs: 1_000, + }); + await completeStepUp(controller); + + jest.setSystemTime(now.getTime() + 2_000); + expect(controller.getElevatedProfileToken()).toBeNull(); + expect(controller.state.stepUpSessionExpiresAt).toBeUndefined(); + } finally { + jest.useRealTimers(); + fetchSpy.mockRestore(); + } + }); + + /** + * Starts `completeStepUp`, ends the authenticated session while the + * verification request is still in flight, then lets the request succeed. + * + * @param endSession - Ends the session mid-flight. + * @returns The controller, once the completion has been rejected. + */ + async function arrangeSessionEndDuringCompletion( + endSession: ( + controller: AuthenticationController, + baseMessenger: RootMessenger, + ) => void, + ): Promise { + let resolveVerification: + | ((response: Awaited>) => void) + | undefined; + let requestStartedResolve: (() => void) | undefined; + const requestStarted = new Promise((resolve) => { + requestStartedResolve = resolve; + }); + const fetchSpy = jest.spyOn(globalThis, 'fetch').mockImplementationOnce( + async (): ReturnType => + await new Promise((resolve) => { + resolveVerification = resolve; + requestStartedResolve?.(); + }), + ); + const { controller, baseMessenger } = createController(); + try { + const completion = controller.completeStepUp({ + flowId: 'flow-id', + proof: passkeyProof, + reason: { operation: 'money.signTransaction' }, + }); + await requestStarted; + endSession(controller, baseMessenger); + resolveVerification?.( + new globalThis.Response( + JSON.stringify(MOCK_MFA_VERIFY_COMPLETE_RESPONSE), + { status: 200 }, + ), + ); + + await expect(completion).rejects.toThrow( + 'the authenticated session ended', + ); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(controller.state.stepUpSessionExpiresAt).toBeUndefined(); + return controller; + } finally { + fetchSpy.mockRestore(); + } + } + + it('cannot reopen an elevated session if the wallet locks during completion', async () => { + const controller = await arrangeSessionEndDuringCompletion( + (_controller, baseMessenger) => + baseMessenger.publish('KeyringController:lock'), + ); + + expect(() => controller.getElevatedProfileToken()).toThrow( + 'wallet is locked', + ); + }); + + it.each([ + [ + 'sign-out', + (controller: AuthenticationController): void => + controller.performSignOut(), + ], + [ + 'wallet reset', + (controller: AuthenticationController): void => controller.clearState(), + ], + ])( + 'leaves no elevated token retrievable when %s happens during completion', + async (_name, endSession) => { + const controller = await arrangeSessionEndDuringCompletion(endSession); + + expect(controller.getElevatedProfileToken()).toBeNull(); + }, + ); + + it.each([MOCK_ACCESS_JWT, 'not-a-jwt'])( + 'rejects an exchanged token without valid AAL2 claims', + async (accessToken) => { + mockEndpointMfaVerifyComplete(); + mockEndpointAccessToken({ + status: 200, + body: { + access_token: accessToken, + expires_in: 900, + }, + }); + const { controller } = createController(); + + await expect( + controller.completeStepUp({ + flowId: 'flow-id', + proof: passkeyProof, + reason: { operation: 'money.signTransaction' }, + }), + ).rejects.toMatchObject({ mfaCode: 'elevated_token_invalid' }); + expect(controller.state.stepUpSessionExpiresAt).toBeUndefined(); + }, + ); + + it('clears the elevated session on lock, sign-out, and wallet reset', async () => { + mockSuccessfulCompletion(); + const first = createController(); + await completeStepUp(first.controller); + first.baseMessenger.publish('KeyringController:lock'); + expect(first.controller.state.stepUpSessionExpiresAt).toBeUndefined(); + + cleanAllNock(); + mockSuccessfulCompletion(); + const second = createController(); + await completeStepUp(second.controller); + second.controller.performSignOut(); + expect(second.controller.getElevatedProfileToken()).toBeNull(); + + cleanAllNock(); + mockSuccessfulCompletion(); + const third = createController(); + await completeStepUp(third.controller); + third.controller.clearState(); + expect(third.controller.getElevatedProfileToken()).toBeNull(); + }); + + it('clears an elevated session after successful enrollment', async () => { + mockSuccessfulCompletion(); + mockEndpointMfaEnrollComplete(); + mockEndpointMfaCredentials(); + const { controller } = createController(); + await completeStepUp(controller); + + await controller.completeCredentialEnrollment({ + flowId: 'enrollment-flow', + proof: { type: 'email_otp', code: '123456' }, + reason: { operation: 'settings.addEmail' }, + }); + + expect(controller.getElevatedProfileToken()).toBeNull(); + }); +}); + describe('metadata', () => { it('includes expected state in debug snapshots', () => { const controller = new AuthenticationController({ diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 90dd33b4551..abdde7364fc 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -4,7 +4,7 @@ import type { ControllerStateChangeEvent, StateMetadata, } from '@metamask/base-controller'; -import type { TraceCallback } from '@metamask/controller-utils'; +import type { TraceCallback, TraceContext } from '@metamask/controller-utils'; import { selectHdKeyringEntropySourceIds } from '@metamask/keyring-controller'; import type { KeyringControllerGetStateAction, @@ -23,6 +23,10 @@ import { BeginEnrollmentRequestStruct, CompleteEnrollmentRequestStruct, assertValidMfaRequest, + BeginStepUpRequestStruct, + CompleteStepUpRequestStruct, + GetElevatedTokenRequestStruct, + parseElevatedTokenClaims, } from '../../sdk/authentication-jwt-bearer/mfa/schemas.js'; import type { LoginIdentifierType, @@ -38,6 +42,11 @@ import type { CompleteEnrollmentRequest, EnrolledCredential, EnrollmentChallenge, + BeginStepUpRequest, + CompleteStepUpRequest, + ElevatedProfileToken, + GetElevatedTokenRequest, + StepUpChallenge, } from '../../sdk/index.js'; import { assertMessageStartsWithMetamask, @@ -46,7 +55,9 @@ import { JwtBearerAuth, PairConflictError, getMfaErrorCode, + ElevatedTokenInvalidError, } from '../../sdk/index.js'; +import { decodeJwtPayload } from '../../sdk/utils/jwt.js'; import type { MetaMetricsAuth } from '../../shared/types/services.js'; import { getPrimaryHdKeyringEntropySourceId } from '../../shared/utils/entropy-source.js'; import { getHdKeyringSeed } from '../../shared/utils/hd-keyring-seed.js'; @@ -68,6 +79,11 @@ export type AuthenticationControllerState = { * assignable to the controller state type. */ enrolledCredentials?: EnrolledCredential[]; + /** + * Epoch-ms hard expiry of the in-memory elevated session, or undefined when + * none is open. Lets UI show "verified" state without holding the token. + */ + stepUpSessionExpiresAt?: number; /** * Client gate for profile pairing. Defaults to `true` (fresh install / * upgrade), set to `false` after a successful `performSignIn` pair, set @@ -159,8 +175,21 @@ const metadata: StateMetadata = { includeInDebugSnapshot: false, usedInUi: true, }, + stepUpSessionExpiresAt: { + includeInStateLogs: false, + persist: false, + includeInDebugSnapshot: false, + usedInUi: true, + }, }; +/** + * Default lifetime of an elevated session. Deliberately shorter than the + * elevated token's own `exp` so a fresh ceremony is required per sensitive + * action window, per the MFA phase-1 specification. + */ +export const STEP_UP_SESSION_TTL_MS = 60_000; + type ControllerConfig = { env: Env; /** @@ -169,6 +198,11 @@ type ControllerConfig = { * `() => false`. */ isSocialPairingEnabled: () => boolean; + /** + * Lifetime of an elevated session opened by `completeStepUp`, clamped to the + * elevated token's `exp`. Defaults to `STEP_UP_SESSION_TTL_MS`. + */ + stepUpSessionTtlMs: number; }; const MESSENGER_EXPOSED_METHODS = [ @@ -186,6 +220,10 @@ const MESSENGER_EXPOSED_METHODS = [ 'refreshEnrolledCredentials', 'beginCredentialEnrollment', 'completeCredentialEnrollment', + 'beginStepUp', + 'completeStepUp', + 'getElevatedProfileToken', + 'clearStepUpSession', ] as const; export type Actions = @@ -252,6 +290,7 @@ export class AuthenticationController extends BaseController< readonly #config: ControllerConfig = { env: Env.PRD, isSocialPairingEnabled: () => false, + stepUpSessionTtlMs: STEP_UP_SESSION_TTL_MS, }; #isUnlocked = false; @@ -262,6 +301,13 @@ export class AuthenticationController extends BaseController< */ #authSessionEpoch = 0; + #stepUpSession: { + token: ElevatedProfileToken; + expiresAt: number; + } | null = null; + + #stepUpTimer: ReturnType | undefined; + /** * Sequence number of the most recently started credentials refresh. Only * that refresh may write the cache, so a slower, earlier request cannot @@ -288,6 +334,7 @@ export class AuthenticationController extends BaseController< this.messenger.subscribe('KeyringController:lock', () => { this.#authSessionEpoch += 1; this.#isUnlocked = false; + this.clearStepUpSession(); }); }, }; @@ -320,11 +367,14 @@ export class AuthenticationController extends BaseController< throw new Error('`metametrics` field is required'); } + // `??` per key so an explicit `undefined` keeps the default rather than + // clobbering it (a `NaN` TTL would open a session that never expires). this.#config = { - ...this.#config, - ...config, + env: config?.env ?? this.#config.env, isSocialPairingEnabled: config?.isSocialPairingEnabled ?? this.#config.isSocialPairingEnabled, + stepUpSessionTtlMs: + config?.stepUpSessionTtlMs ?? this.#config.stepUpSessionTtlMs, }; this.#metametrics = metametrics; @@ -805,23 +855,24 @@ export class AuthenticationController extends BaseController< credentialType: string, fn: () => Promise, ): Promise { - const data: Record = { outcome: 'pending' }; return await this.#trace( { name, tags: { operation, credentialType }, - data, }, - async () => { + async (context) => { try { const result = await fn(); - data.outcome = 'success'; + this.#setTraceAttribute(context, 'outcome', 'success'); return result; } catch (error) { - data.outcome = 'error'; + this.#setTraceAttribute(context, 'outcome', 'error'); const mfaCode = getMfaErrorCode(error); if (mfaCode) { - data.mfaErrorCode = mfaCode; + this.#setTraceAttribute(context, 'mfaErrorCode', mfaCode); + if (mfaCode === 'authentication_required' && this.#isUnlocked) { + this.#invalidateSrpSession(this.#getPrimaryEntropySourceId()); + } } throw error; } @@ -829,6 +880,26 @@ export class AuthenticationController extends BaseController< ); } + /** + * `TraceContext` is opaque in `@metamask/controller-utils`; both clients + * hand back a Sentry span, so attributes are set via duck typing and a + * non-Sentry context is a silent no-op. + * + * @param context - Span handed to the trace callback, if any. + * @param key - Attribute name. + * @param value - Attribute value. + */ + #setTraceAttribute( + context: TraceContext | undefined, + key: string, + value: string, + ): void { + const traceSpan = context as + | { setAttribute?: (attribute: string, data: string) => void } + | undefined; + traceSpan?.setAttribute?.(key, value); + } + /** * Refreshes credentials enrolled on the canonical profile. * @@ -939,6 +1010,7 @@ export class AuthenticationController extends BaseController< } throw error; } + this.clearStepUpSession(); try { return await this.refreshEnrolledCredentials(); @@ -951,6 +1023,159 @@ export class AuthenticationController extends BaseController< } } + /** + * Begins step-up verification with an enrolled credential. + * + * @param request - Credential type and trace reason. + * @returns A challenge for the client-owned ceremony. + */ + public async beginStepUp( + request: BeginStepUpRequest, + ): Promise { + this.#assertIsUnlocked('beginStepUp'); + const sessionEpoch = this.#authSessionEpoch; + assertValidMfaRequest(request, BeginStepUpRequestStruct); + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); + const challenge = await this.#runMfaRequest( + 'MFA Step-Up Begin', + request.reason.operation, + request.type, + async () => + await this.#auth.beginMfaVerification( + request.type, + primaryEntropySourceId, + ), + ); + this.#assertAuthSessionEpoch(sessionEpoch, 'beginStepUp'); + return challenge; + } + + /** + * Completes step-up verification and opens a short-lived elevated session. + * + * The AAL2 assertion returned by the MFA service is exchanged at Hydra for + * an elevated access token, whose claims are checked before the session + * opens. The token itself never enters controller state. + * + * @param request - Flow identifier, platform or email proof, and trace reason. + * @returns The elevated profile access token. + */ + public async completeStepUp( + request: CompleteStepUpRequest, + ): Promise { + this.#assertIsUnlocked('completeStepUp'); + const sessionEpoch = this.#authSessionEpoch; + assertValidMfaRequest(request, CompleteStepUpRequestStruct); + const { type } = request.proof; + const { operation } = request.reason; + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); + const assertion = await this.#runMfaRequest( + 'MFA Step-Up Complete', + operation, + type, + async () => + await this.#auth.completeMfaVerification( + request.flowId, + request.proof, + primaryEntropySourceId, + ), + ); + this.#assertAuthSessionEpoch(sessionEpoch, 'completeStepUp'); + const accessToken = await this.#runMfaRequest( + 'MFA Token Exchange', + operation, + type, + async () => await this.#auth.exchangeMfaAssertion(assertion.token), + ); + this.#assertAuthSessionEpoch(sessionEpoch, 'completeStepUp'); + + let decodedClaims: unknown; + try { + decodedClaims = decodeJwtPayload(accessToken.accessToken); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new ElevatedTokenInvalidError(message); + } + const claims = parseElevatedTokenClaims(decodedClaims); + if (claims.exp * 1000 <= Date.now()) { + throw new ElevatedTokenInvalidError('Elevated token is expired'); + } + + const token: ElevatedProfileToken = { ...accessToken, claims }; + this.#openStepUpSession(token); + return token; + } + + /** + * Returns the active elevated token when it meets the requested freshness. + * + * @param request - Optional maximum session age in milliseconds, measured + * from when the token was obtained. Zero always requires a new ceremony. + * @returns A live elevated token, or null when no reusable session exists. + */ + public getElevatedProfileToken( + request: GetElevatedTokenRequest = {}, + ): ElevatedProfileToken | null { + this.#assertIsUnlocked('getElevatedProfileToken'); + assertValidMfaRequest(request, GetElevatedTokenRequestStruct); + const session = this.#stepUpSession; + if (!session) { + return null; + } + const now = Date.now(); + if (now >= session.expiresAt) { + // The hard-expiry timer has not fired yet (e.g. a suspended tab). + this.clearStepUpSession(); + return null; + } + // `>=` so a zero max age always forces a fresh ceremony. + const maxAge = request.maxSessionAgeMs; + if (maxAge !== undefined && now - session.token.obtainedAt >= maxAge) { + return null; + } + return session.token; + } + + /** + * Opens the elevated session. Its lifetime is the configured TTL clamped to + * the token's own `exp`, so the session never outlives the token. + * + * @param token - The freshly exchanged elevated token. + */ + #openStepUpSession(token: ElevatedProfileToken): void { + this.clearStepUpSession(); + const expiresAt = Math.min( + token.obtainedAt + this.#config.stepUpSessionTtlMs, + token.claims.exp * 1000, + ); + this.#stepUpSession = { token, expiresAt }; + this.update((state) => { + state.stepUpSessionExpiresAt = expiresAt; + }); + this.#stepUpTimer = setTimeout( + () => this.clearStepUpSession(), + Math.max(0, expiresAt - Date.now()), + ); + // Never keep a Node process alive for the expiry timer (tests, tooling). + (this.#stepUpTimer as { unref?: () => void }).unref?.(); + } + + /** + * Clears the in-memory elevated session and its expiration timer. + */ + public clearStepUpSession(): void { + if (this.#stepUpTimer !== undefined) { + clearTimeout(this.#stepUpTimer); + this.#stepUpTimer = undefined; + } + this.#stepUpSession = null; + if (this.state.stepUpSessionExpiresAt !== undefined) { + this.update((state) => { + state.stepUpSessionExpiresAt = undefined; + }); + } + } + /** * Drops the cached credential list. Callers that wipe profile state must go * through this so subscribers never keep credentials belonging to a profile @@ -971,6 +1196,7 @@ export class AuthenticationController extends BaseController< public performSignOut(): void { this.#authSessionEpoch += 1; + this.clearStepUpSession(); this.#clearEnrolledCredentials(); this.update((state) => { state.isSignedIn = false; @@ -985,6 +1211,7 @@ export class AuthenticationController extends BaseController< public clearState(): void { this.#profilePairingRequestEpoch += 1; this.#authSessionEpoch += 1; + this.clearStepUpSession(); this.#clearEnrolledCredentials(); this.update(() => ({ ...defaultState })); } diff --git a/packages/profile-sync-controller/src/controllers/authentication/index.ts b/packages/profile-sync-controller/src/controllers/authentication/index.ts index 8887e1c3d69..f504baec99e 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/index.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/index.ts @@ -19,4 +19,8 @@ export type { AuthenticationControllerRefreshEnrolledCredentialsAction, AuthenticationControllerBeginCredentialEnrollmentAction, AuthenticationControllerCompleteCredentialEnrollmentAction, + AuthenticationControllerBeginStepUpAction, + AuthenticationControllerCompleteStepUpAction, + AuthenticationControllerGetElevatedProfileTokenAction, + AuthenticationControllerClearStepUpSessionAction, } from './AuthenticationController-method-action-types.js'; From 744dee0a3053cb79234f42feb8f780385192f76d Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 18 Sep 2026 23:10:00 +0200 Subject: [PATCH 2/2] fix: add back cleanAllNock --- .../controllers/authentication/AuthenticationController.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index c6630b1a9f0..2d8f69c9862 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -7,6 +7,7 @@ import type { MessengerEvents, MockAnyNamespace, } from '@metamask/messenger'; +import { cleanAll as cleanAllNock } from 'nock'; import { arrangeAuthAPIs } from '../../sdk/__fixtures__/auth.js'; import type { LoginResponse } from '../../sdk/index.js';