diff --git a/eslint-suppressions.json b/eslint-suppressions.json index ef11872af77..beb5d4a9377 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1641,9 +1641,6 @@ } }, "packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 5 - }, "id-length": { "count": 4 }, diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 3665d3c5e1b..9b27e131240 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 ### Changed +- Resolve HD entropy source IDs from `KeyringController` instead of the message-signing snap (`getBearerToken` primary ID, `performSignIn` SRP enumeration) ([#9794](https://github.com/MetaMask/core/pull/9794)) - Bump `@metamask/keyring-controller` from `^27.1.0` to `^27.1.1` ([#9791](https://github.com/MetaMask/core/pull/9791)) ## [29.0.0] 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 3a9c3e87471..77f44cef40d 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -1,4 +1,5 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; +import { KeyringTypes } from '@metamask/keyring-controller'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MessengerActions, @@ -29,6 +30,25 @@ const MOCK_ENTROPY_SOURCE_IDS = [ 'MOCK_ENTROPY_SOURCE_ID2', ]; +const MOCK_HD_KEYRINGS = MOCK_ENTROPY_SOURCE_IDS.map((id) => ({ + type: KeyringTypes.hd, + accounts: [] as string[], + metadata: { id, name: '' }, +})); + +const mockHdKeyrings = ( + ...ids: string[] +): { + type: typeof KeyringTypes.hd; + accounts: string[]; + metadata: { id: string; name: string }; +}[] => + ids.map((id) => ({ + type: KeyringTypes.hd, + accounts: [], + metadata: { id, name: '' }, + })); + type SrpLoginRequestBody = { metametrics?: { // eslint-disable-next-line @typescript-eslint/naming-convention -- API field @@ -118,12 +138,8 @@ describe('AuthenticationController', () => { it('should create access token(s) and update state', async () => { const metametrics = createMockAuthMetaMetrics(); const mockEndpoints = arrangeAuthAPIs(); - const { - messenger, - mockSnapGetPublicKey, - mockSnapGetAllPublicKeys, - mockSnapSignMessage, - } = createMockAuthenticationMessenger(); + const { messenger, mockSnapGetPublicKey, mockSnapSignMessage } = + createMockAuthenticationMessenger(); const controller = new AuthenticationController({ messenger, @@ -131,9 +147,8 @@ describe('AuthenticationController', () => { }); const result = await controller.performSignIn(); - // 1 from `performSignIn` itself + 1 from `#doPair` → - // `#getCanonicalProfileId` → `#getPrimaryEntropySourceId` (cold cache). - expect(mockSnapGetAllPublicKeys).toHaveBeenCalledTimes(2); + // SRP enumeration uses KeyringController; snap is only needed for + // getPublicKey / signMessage during cold login. expect(mockSnapGetPublicKey).toHaveBeenCalledTimes(2); // Primary and secondary tags produce distinct messages, so both are signed. expect(mockSnapSignMessage).toHaveBeenCalledTimes(2); @@ -206,12 +221,13 @@ describe('AuthenticationController', () => { const { messenger, mockSnapSignMessage, - mockSnapGetAllPublicKeys, + mockKeyringControllerGetState, mockSeedlessOnboardingGetState, } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - [MOCK_ENTROPY_SOURCE_IDS[0], 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); mockSeedlessOnboardingGetState.mockReturnValue({ vault: 'encrypted', authConnection: 'google', @@ -245,12 +261,13 @@ describe('AuthenticationController', () => { }); const { messenger, - mockSnapGetAllPublicKeys, + mockKeyringControllerGetState, mockSeedlessOnboardingGetState, } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - [MOCK_ENTROPY_SOURCE_IDS[0], 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); mockSeedlessOnboardingGetState.mockReturnValue({ vault: 'encrypted', authConnection: 'google', @@ -339,12 +356,13 @@ describe('AuthenticationController', () => { }); const { messenger, - mockSnapGetAllPublicKeys, + mockKeyringControllerGetState, mockSeedlessOnboardingGetState, } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - [MOCK_ENTROPY_SOURCE_IDS[0], 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); mockSeedlessOnboardingGetState.mockReturnValue(seedlessState); const controller = new AuthenticationController({ @@ -374,12 +392,13 @@ describe('AuthenticationController', () => { }); const { messenger, - mockSnapGetAllPublicKeys, + mockKeyringControllerGetState, mockSeedlessOnboardingGetState, } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - [MOCK_ENTROPY_SOURCE_IDS[0], 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); mockSeedlessOnboardingGetState.mockImplementation(() => { throw new Error('SeedlessOnboardingController unavailable'); }); @@ -417,7 +436,10 @@ describe('AuthenticationController', () => { arrangeAuthAPIs(); const metametrics = createMockAuthMetaMetrics(); - mockKeyringControllerGetState.mockReturnValue({ isUnlocked: true }); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: MOCK_HD_KEYRINGS, + }); const controller = new AuthenticationController({ messenger, @@ -443,7 +465,7 @@ describe('AuthenticationController', () => { */ async function testAndAssertFailingEndpoints( endpointFail: 'nonce' | 'login' | 'token', - ) { + ): Promise { const mockEndpoints = mockAuthenticationFlowEndpoints({ endpointFail, }); @@ -482,12 +504,13 @@ describe('AuthenticationController', () => { it('does NOT call pairProfiles when only 1 SRP exists, but clears needsProfilePairing', async () => { const metametrics = createMockAuthMetaMetrics(); const mockEndpoints = arrangeAuthAPIs(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - ['SINGLE_ENTROPY_SOURCE_ID', 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings('SINGLE_ENTROPY_SOURCE_ID'), + }); const controller = new AuthenticationController({ messenger, @@ -771,12 +794,13 @@ describe('AuthenticationController', () => { it('epoch check: a concurrent requestProfilePairing during single-SRP performSignIn keeps the gate set', async () => { const metametrics = createMockAuthMetaMetrics(); arrangeAuthAPIs(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - ['SINGLE_ENTROPY_SOURCE_ID', 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings('SINGLE_ENTROPY_SOURCE_ID'), + }); const controller = new AuthenticationController({ messenger, @@ -791,10 +815,10 @@ describe('AuthenticationController', () => { expect(controller.state.needsProfilePairing).toBe(true); }); - it('epoch check: a requestProfilePairing fired from inside #doPair (between snap call and pair API completion) keeps the gate set', async () => { + it('epoch check: a requestProfilePairing fired from inside #doPair (during primary entropy ID resolve) keeps the gate set', async () => { const metametrics = createMockAuthMetaMetrics(); arrangeAuthAPIs(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); const controller = new AuthenticationController({ @@ -803,24 +827,23 @@ describe('AuthenticationController', () => { metametrics, }); - // `#snapGetAllPublicKeys` is called twice per `performSignIn`: - // 1. directly inside `performSignIn` (to enumerate SRPs) - // 2. indirectly inside `#doPair` → `#getCanonicalProfileId` → - // `#getPrimaryEntropySourceId` (cold cache) - // We hook the second call to fire the rearm — this is the realistic - // production race window (e.g. user adds an SRP while the pair API - // request is in flight). - mockSnapGetAllPublicKeys - .mockResolvedValueOnce( - MOCK_ENTROPY_SOURCE_IDS.map((id) => [id, 'MOCK_PUBLIC_KEY']), - ) - .mockImplementationOnce(async () => { + // Keyring reads during this performSignIn: + // 1. SRP enumeration at the start of `performSignIn` + // 2. `#doPair` → `#getCanonicalProfileId` → `#getPrimaryEntropySourceId` + // Fire the rearm on (2) so we cover the in-#doPair race window + // (e.g. user adds an SRP while pairing is in flight), not enumeration. + let keyringReads = 0; + mockKeyringControllerGetState.mockImplementation(() => { + keyringReads += 1; + if (keyringReads === 2) { controller.requestProfilePairing(); - return MOCK_ENTROPY_SOURCE_IDS.map((id) => [id, 'MOCK_PUBLIC_KEY']); - }); + } + return { isUnlocked: true, keyrings: MOCK_HD_KEYRINGS }; + }); await controller.performSignIn(); + expect(keyringReads).toBeGreaterThanOrEqual(2); expect(controller.state.needsProfilePairing).toBe(true); }); }); @@ -993,7 +1016,7 @@ describe('AuthenticationController', () => { it('resolves undefined entropySourceId to primary and stores token', async () => { const metametrics = createMockAuthMetaMetrics(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); arrangeAuthAPIs(); @@ -1005,7 +1028,7 @@ describe('AuthenticationController', () => { const result = await controller.getBearerToken(); expect(result).toBe(MOCK_OATH_TOKEN_RESPONSE.access_token); - expect(mockSnapGetAllPublicKeys).toHaveBeenCalled(); + expect(mockKeyringControllerGetState).toHaveBeenCalled(); expect(controller.state.isSignedIn).toBe(true); expect( controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]], @@ -1029,9 +1052,9 @@ describe('AuthenticationController', () => { expect(resultUndefined).toBe(resultExplicit); }); - it('caches the primary entropySourceId resolution across calls', async () => { + it('resolves primary entropySourceId from the HD keyring without the snap', async () => { const metametrics = createMockAuthMetaMetrics(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockSnapGetPublicKey, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); const originalState = mockSignedInState(); const controller = new AuthenticationController({ @@ -1044,39 +1067,27 @@ describe('AuthenticationController', () => { await controller.getBearerToken(); await controller.getBearerToken(); - // Only the first call hits the snap; subsequent calls hit the cache. - expect(mockSnapGetAllPublicKeys).toHaveBeenCalledTimes(1); + // Cached session: no snap identify/sign; only keyring for primary ID. + expect(mockSnapGetPublicKey).not.toHaveBeenCalled(); + expect(mockKeyringControllerGetState).toHaveBeenCalled(); }); - it('throws when snap returns no entropy sources', async () => { + it('throws when no HD keyring is available', async () => { const metametrics = createMockAuthMetaMetrics(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([]); - - const controller = new AuthenticationController({ - messenger, - metametrics, + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: [], }); - await expect(controller.getBearerToken()).rejects.toThrow( - 'No entropy sources found from snap', - ); - }); - - it('throws when primary entropy source ID is undefined', async () => { - const metametrics = createMockAuthMetaMetrics(); - const { messenger, mockSnapGetAllPublicKeys } = - createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([[undefined, 'MOCK_KEY']]); - const controller = new AuthenticationController({ messenger, metametrics, }); await expect(controller.getBearerToken()).rejects.toThrow( - 'Primary entropy source ID is undefined', + 'no HD keyring available', ); }); }); @@ -1208,12 +1219,13 @@ describe('AuthenticationController', () => { it('should re-login primary SRP and return fresh canonical regardless of SRP count', async () => { const metametrics = createMockAuthMetaMetrics(); arrangeAuthAPIs(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([ - ['SINGLE_ENTROPY_SOURCE_ID', 'MOCK_PUBLIC_KEY'], - ]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings('SINGLE_ENTROPY_SOURCE_ID'), + }); const originalState = mockSignedInState(); const controller = new AuthenticationController({ @@ -1229,12 +1241,15 @@ describe('AuthenticationController', () => { ).toBeDefined(); }); - it('should throw if snap returns no entropy sources', async () => { + it('should throw if no HD keyring is available', async () => { const metametrics = createMockAuthMetaMetrics(); - const { messenger, mockSnapGetAllPublicKeys } = + const { messenger, mockKeyringControllerGetState } = createMockAuthenticationMessenger(); - mockSnapGetAllPublicKeys.mockResolvedValue([]); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: [], + }); const controller = new AuthenticationController({ messenger, @@ -1242,7 +1257,7 @@ describe('AuthenticationController', () => { }); await expect(controller.refreshCanonicalProfileId()).rejects.toThrow( - expect.any(Error), + 'no HD keyring available', ); }); }); @@ -1616,7 +1631,10 @@ const controllerName = 'AuthenticationController'; * * @returns Auth Messenger */ -function createAuthenticationMessenger() { +function createAuthenticationMessenger(): { + messenger: AuthenticationControllerMessenger; + baseMessenger: RootMessenger; +} { const rootMessenger = getRootMessenger(); const messenger = new Messenger< typeof controllerName, @@ -1645,23 +1663,26 @@ function createAuthenticationMessenger() { * * @returns Mock Auth Messenger */ -function createMockAuthenticationMessenger() { +function createMockAuthenticationMessenger(): { + messenger: AuthenticationControllerMessenger; + baseMessenger: RootMessenger; + mockSnapGetPublicKey: jest.Mock; + mockSnapSignMessage: jest.Mock; + mockKeyringControllerGetState: jest.Mock; + mockSeedlessOnboardingGetState: jest.Mock; +} { const { baseMessenger, messenger } = createAuthenticationMessenger(); const mockCall = jest.spyOn(messenger, 'call'); const mockSnapGetPublicKey = jest.fn().mockResolvedValue('MOCK_PUBLIC_KEY'); - const mockSnapGetAllPublicKeys = jest - .fn() - .mockResolvedValue( - MOCK_ENTROPY_SOURCE_IDS.map((id) => [id, 'MOCK_PUBLIC_KEY']), - ); const mockSnapSignMessage = jest .fn() .mockResolvedValue('MOCK_SIGNED_MESSAGE'); - const mockKeyringControllerGetState = jest - .fn() - .mockReturnValue({ isUnlocked: true }); + const mockKeyringControllerGetState = jest.fn().mockReturnValue({ + isUnlocked: true, + keyrings: MOCK_HD_KEYRINGS, + }); const mockSeedlessOnboardingGetState = jest .fn() @@ -1680,10 +1701,6 @@ function createMockAuthenticationMessenger() { return mockSnapGetPublicKey(); } - if (params?.request.method === 'getAllPublicKeys') { - return mockSnapGetAllPublicKeys(); - } - if (params?.request.method === 'signMessage') { return mockSnapSignMessage(params.request.params); } @@ -1712,7 +1729,6 @@ function createMockAuthenticationMessenger() { messenger, baseMessenger, mockSnapGetPublicKey, - mockSnapGetAllPublicKeys, mockSnapSignMessage, mockKeyringControllerGetState, mockSeedlessOnboardingGetState, @@ -1728,7 +1744,7 @@ function createMockAuthenticationMessenger() { */ function mockAuthenticationFlowEndpoints(params?: { endpointFail: 'nonce' | 'login' | 'token' | 'lineage' | 'customerService'; -}) { +}): ReturnType { const { mockNonceUrl, mockOAuth2TokenUrl, @@ -1762,7 +1778,10 @@ function mockAuthenticationFlowEndpoints(params?: { * * @returns mock metametrics method */ -function createMockAuthMetaMetrics() { +function createMockAuthMetaMetrics(): { + getMetaMetricsId: jest.Mock; + agent: typeof Platform.EXTENSION; +} { const getMetaMetricsId = jest .fn() .mockReturnValue(MOCK_LOGIN_RESPONSE.profile.metametrics_id); diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 4d793413f59..c9e3d4d1276 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -30,9 +30,12 @@ import { JwtBearerAuth, } from '../../sdk/index.js'; import type { MetaMetricsAuth } from '../../shared/types/services.js'; +import { + getHdKeyringEntropySourceIds, + getPrimaryHdKeyringEntropySourceId, +} from '../../shared/utils/entropy-source.js'; import { createSnapPublicKeyRequest, - createSnapAllPublicKeysRequest, createSnapSignMessageRequest, } from './auth-snap-requests.js'; import { AuthenticationControllerMethodActions } from './AuthenticationController-method-action-types.js'; @@ -183,8 +186,6 @@ export class AuthenticationController extends BaseController< #isUnlocked = false; - #cachedPrimaryEntropySourceId?: string; - // Bumped by `requestProfilePairing`. `performSignIn` snapshots this // before its first await; if it changes mid-flight we must NOT clear // `needsProfilePairing` (the rearm signal wins). @@ -270,8 +271,7 @@ export class AuthenticationController extends BaseController< async #getLoginResponseFromState( entropySourceId?: string, ): Promise { - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); if (!this.state.srpSessionData?.[resolvedId]) { return null; } @@ -282,8 +282,7 @@ export class AuthenticationController extends BaseController< loginResponse: LoginResponse, entropySourceId?: string, ) { - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); const metaMetricsId = await this.#metametrics.getMetaMetricsId(); this.update((state) => { state.isSignedIn = true; @@ -306,27 +305,28 @@ export class AuthenticationController extends BaseController< } } - async #getPrimaryEntropySourceId(): Promise { - if (this.#cachedPrimaryEntropySourceId) { - return this.#cachedPrimaryEntropySourceId; - } - const allPublicKeys = await this.#snapGetAllPublicKeys(); - - if (allPublicKeys.length === 0) { - throw new Error( - '#getPrimaryEntropySourceId - No entropy sources found from snap', - ); - } - - const primaryId = allPublicKeys[0][0]; - if (!primaryId) { - throw new Error( - '#getPrimaryEntropySourceId - Primary entropy source ID is undefined', - ); - } + /** + * Reads the HD keyring entropy source IDs from KeyringController. + * + * @returns The HD keyring metadata IDs, primary first. + */ + #getHdKeyringEntropySourceIds(): string[] { + const { keyrings } = this.messenger.call('KeyringController:getState'); + return getHdKeyringEntropySourceIds(keyrings); + } - this.#cachedPrimaryEntropySourceId = primaryId; - return this.#cachedPrimaryEntropySourceId; + /** + * Resolves the primary SRP's entropy source ID from KeyringController rather + * than the message-signing snap, so callers like `getBearerToken()` are not + * blocked on snap boot. + * + * @returns The primary HD keyring metadata ID. + * @throws If no HD keyring is available; callers must only resolve while + * the wallet is unlocked. + */ + #getPrimaryEntropySourceId(): string { + const { keyrings } = this.messenger.call('KeyringController:getState'); + return getPrimaryHdKeyringEntropySourceId(keyrings); } /** @@ -339,7 +339,7 @@ export class AuthenticationController extends BaseController< * @returns The login tag to append to the signed message. */ async #getLoginTag(entropySourceId?: string): Promise { - const primaryEntropySourceId = await this.#getPrimaryEntropySourceId(); + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); const resolvedId = entropySourceId ?? primaryEntropySourceId; return resolvedId === primaryEntropySourceId ? 'primary' : 'secondary'; @@ -366,7 +366,7 @@ export class AuthenticationController extends BaseController< async #getLoginIdentifierType( entropySourceId?: string, ): Promise { - const primaryEntropySourceId = await this.#getPrimaryEntropySourceId(); + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); const resolvedId = entropySourceId ?? primaryEntropySourceId; // Social vault authConnection is only associated with the primary source. @@ -417,17 +417,17 @@ export class AuthenticationController extends BaseController< this.#assertIsUnlocked('performSignIn'); const epochAtStart = this.#profilePairingRequestEpoch; - const allPublicKeys = await this.#snapGetAllPublicKeys(); + const entropySourceIds = this.#getHdKeyringEntropySourceIds(); const accessTokens: string[] = []; // We iterate sequentially in order to be sure that the first entry // is the primary SRP LoginResponse. - for (const [entropySourceId] of allPublicKeys) { + for (const entropySourceId of entropySourceIds) { const accessToken = await this.#auth.getAccessToken(entropySourceId); accessTokens.push(accessToken); } - if (allPublicKeys.length < 2) { + if (entropySourceIds.length < 2) { // Single-SRP wallet: nothing to pair. this.#tryClearNeedsProfilePairing(epochAtStart); } else { @@ -486,10 +486,10 @@ export class AuthenticationController extends BaseController< * pair API call was in-flight. */ async #doPair(accessTokens: string[], epochAtStart: number): Promise { - const previousCanonical = await this.#getCanonicalProfileId(); + const previousCanonical = this.#getCanonicalProfileId(); const profileAliases = await this.#pairSrpProfiles(accessTokens); - const newCanonical = await this.#getCanonicalProfileId(); + const newCanonical = this.#getCanonicalProfileId(); // If somehow we cannot compute the new canonical profile ID after pairing, // we just return now and do not update the `needsProfilePairing` flag. @@ -549,8 +549,8 @@ export class AuthenticationController extends BaseController< * * @returns The canonical profile id, or `null` if unavailable. */ - async #getCanonicalProfileId(): Promise { - const primaryEntropySourceId = await this.#getPrimaryEntropySourceId(); + #getCanonicalProfileId(): string | null { + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); return ( this.state.srpSessionData?.[primaryEntropySourceId]?.profile ?.canonicalProfileId ?? null @@ -558,7 +558,6 @@ export class AuthenticationController extends BaseController< } public performSignOut(): void { - this.#cachedPrimaryEntropySourceId = undefined; this.update((state) => { state.isSignedIn = false; state.srpSessionData = undefined; @@ -578,8 +577,7 @@ export class AuthenticationController extends BaseController< */ public async getBearerToken(entropySourceId?: string): Promise { this.#assertIsUnlocked('getBearerToken'); - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); return await this.#auth.getAccessToken(resolvedId); } @@ -600,8 +598,7 @@ export class AuthenticationController extends BaseController< entropySourceId?: string, ): Promise { this.#assertIsUnlocked('getSessionProfile'); - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); return await this.#auth.getUserProfile(resolvedId); } @@ -624,11 +621,11 @@ export class AuthenticationController extends BaseController< public async refreshCanonicalProfileId(): Promise { this.#assertIsUnlocked('refreshCanonicalProfileId'); - const primaryEntropySourceId = await this.#getPrimaryEntropySourceId(); + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); this.#invalidateSrpSession(primaryEntropySourceId); await this.#auth.getAccessToken(primaryEntropySourceId); - const canonical = await this.#getCanonicalProfileId(); + const canonical = this.#getCanonicalProfileId(); if (!canonical) { throw new Error( 'refreshCanonicalProfileId - Unable to resolve canonical profile ID', @@ -655,8 +652,7 @@ export class AuthenticationController extends BaseController< entropySourceId?: string, ): Promise { this.#assertIsUnlocked('getUserProfileLineage'); - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); return await this.#auth.getUserProfileLineage(resolvedId); } @@ -675,8 +671,7 @@ export class AuthenticationController extends BaseController< entropySourceId?: string, ): Promise { this.#assertIsUnlocked('getCustomerServiceToken'); - const resolvedId = - entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + const resolvedId = entropySourceId ?? this.#getPrimaryEntropySourceId(); return await this.#auth.getCustomerServiceToken(resolvedId); } @@ -702,22 +697,6 @@ export class AuthenticationController extends BaseController< return result; } - /** - * Returns a mapping of entropy source IDs to auth snap public keys. - * - * @returns A mapping of entropy source IDs to public keys. - */ - async #snapGetAllPublicKeys(): Promise<[string, string][]> { - this.#assertIsUnlocked('#snapGetAllPublicKeys'); - - const result = (await this.messenger.call( - 'SnapController:handleRequest', - createSnapAllPublicKeysRequest(), - )) as [string, string][]; - - return result; - } - #_snapSignMessageCache: Record<`metamask:${string}`, string> = {}; /** diff --git a/packages/profile-sync-controller/src/controllers/authentication/auth-snap-requests.ts b/packages/profile-sync-controller/src/controllers/authentication/auth-snap-requests.ts index 1de9cd1ee9a..325669fa75b 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/auth-snap-requests.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/auth-snap-requests.ts @@ -29,22 +29,6 @@ export function createSnapPublicKeyRequest( }; } -/** - * Constructs Request to Message Signing Snap to get [EntropySourceId, PublicKey][] - * - * @returns Snap getAllPublicKeys Request - */ -export function createSnapAllPublicKeysRequest(): SnapRPCRequest { - return { - snapId, - origin: 'metamask', - handler: 'onRpcRequest' as any, - request: { - method: 'getAllPublicKeys', - }, - }; -} - /** * Constructs Request to get Message Signing Snap to sign a message. * diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index ae4d54ac654..8271ae76469 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -17,7 +17,6 @@ import type { TraceContext, TraceRequest, } from '@metamask/controller-utils'; -import { KeyringTypes } from '@metamask/keyring-controller'; import type { KeyringControllerGetStateAction, KeyringControllerLockEvent, @@ -33,6 +32,10 @@ import type { } from '../../sdk/index.js'; import { Env, UserStorage } from '../../sdk/index.js'; import type { NativeScrypt } from '../../shared/types/encryption.js'; +import { + getHdKeyringEntropySourceIds, + getPrimaryHdKeyringEntropySourceId, +} from '../../shared/utils/entropy-source.js'; import { EventQueue } from '../../shared/utils/event-queue.js'; import { createSnapSignMessageRequest } from '../authentication/auth-snap-requests.js'; import type { @@ -533,35 +536,28 @@ export class UserStorageController extends BaseController< } /** - * Reads the HD keyring entropy source IDs (metadata IDs) from the - * KeyringController, primary first. Returns an empty array when none are - * available (e.g. the wallet is locked, where `keyrings` is cleared). + * Reads the HD keyring entropy source IDs from KeyringController. * * @returns The HD keyring metadata IDs, primary first. */ #getHdKeyringEntropySourceIds(): string[] { const { keyrings } = this.messenger.call('KeyringController:getState'); - return (keyrings ?? []) - .filter((keyring) => keyring.type === KeyringTypes.hd.toString()) - .map((keyring) => keyring.metadata.id); + return getHdKeyringEntropySourceIds(keyrings); } /** - * Resolves the primary SRP's entropy source ID (the first HD keyring's - * metadata ID), used to scope the primary's cache entries. The ID is randomly - * regenerated whenever the vault is recreated (e.g. on restore), so a new - * primary can never inherit a previous vault's cached key. + * Resolves the primary SRP's entropy source ID, used to scope the primary's + * cache entries. The ID is randomly regenerated whenever the vault is + * recreated (e.g. on restore), so a new primary can never inherit a previous + * vault's cached key. * * @returns The primary HD keyring metadata ID. * @throws If no HD keyring is available; callers must only resolve the scope * while the wallet is unlocked. */ #getPrimaryEntropySourceId(): string { - const [primaryEntropySourceId] = this.#getHdKeyringEntropySourceIds(); - if (!primaryEntropySourceId) { - throw new Error('#getPrimaryEntropySourceId - no HD keyring available'); - } - return primaryEntropySourceId; + const { keyrings } = this.messenger.call('KeyringController:getState'); + return getPrimaryHdKeyringEntropySourceId(keyrings); } /** diff --git a/packages/profile-sync-controller/src/shared/utils/entropy-source.test.ts b/packages/profile-sync-controller/src/shared/utils/entropy-source.test.ts new file mode 100644 index 00000000000..9e8069c611a --- /dev/null +++ b/packages/profile-sync-controller/src/shared/utils/entropy-source.test.ts @@ -0,0 +1,54 @@ +import { KeyringTypes } from '@metamask/keyring-controller'; +import type { KeyringObject } from '@metamask/keyring-controller'; + +import { + getHdKeyringEntropySourceIds, + getPrimaryHdKeyringEntropySourceId, +} from './entropy-source.js'; + +const hd = (id: string): KeyringObject => ({ + type: KeyringTypes.hd, + accounts: [], + metadata: { id, name: '' }, +}); + +describe('entropy-source utils', () => { + describe('getHdKeyringEntropySourceIds', () => { + it('returns HD keyring metadata IDs, primary first', () => { + const keyrings: KeyringObject[] = [ + hd('primary'), + { + type: 'Simple Key Pair', + accounts: [], + metadata: { id: 'simple', name: '' }, + }, + hd('secondary'), + ]; + + expect(getHdKeyringEntropySourceIds(keyrings)).toStrictEqual([ + 'primary', + 'secondary', + ]); + }); + + it('returns an empty array when keyrings are missing or empty', () => { + expect(getHdKeyringEntropySourceIds(undefined)).toStrictEqual([]); + expect(getHdKeyringEntropySourceIds(null)).toStrictEqual([]); + expect(getHdKeyringEntropySourceIds([])).toStrictEqual([]); + }); + }); + + describe('getPrimaryHdKeyringEntropySourceId', () => { + it('returns the first HD keyring metadata ID', () => { + expect( + getPrimaryHdKeyringEntropySourceId([hd('primary'), hd('secondary')]), + ).toBe('primary'); + }); + + it('throws when no HD keyring is available', () => { + expect(() => getPrimaryHdKeyringEntropySourceId([])).toThrow( + 'no HD keyring available', + ); + }); + }); +}); diff --git a/packages/profile-sync-controller/src/shared/utils/entropy-source.ts b/packages/profile-sync-controller/src/shared/utils/entropy-source.ts new file mode 100644 index 00000000000..9de8c974e5f --- /dev/null +++ b/packages/profile-sync-controller/src/shared/utils/entropy-source.ts @@ -0,0 +1,40 @@ +import { KeyringTypes } from '@metamask/keyring-controller'; +import type { KeyringObject } from '@metamask/keyring-controller'; + +/** + * Reads HD keyring entropy source IDs (metadata IDs) from KeyringController + * keyrings, primary first. Returns an empty array when none are available + * (e.g. the wallet is locked, where `keyrings` is cleared). + * + * @param keyrings - Keyrings from `KeyringController:getState`. + * @returns The HD keyring metadata IDs, primary first. + */ +export function getHdKeyringEntropySourceIds( + keyrings: KeyringObject[] | null | undefined, +): string[] { + return (keyrings ?? []) + .filter((keyring) => keyring.type === KeyringTypes.hd.toString()) + .map((keyring) => keyring.metadata.id); +} + +/** + * Resolves the primary SRP's entropy source ID (the first HD keyring's + * metadata ID). The ID is randomly regenerated whenever the vault is + * recreated (e.g. on restore). + * + * @param keyrings - Keyrings from `KeyringController:getState`. + * @returns The primary HD keyring metadata ID. + * @throws If no HD keyring is available; callers must only resolve while + * the wallet is unlocked. + */ +export function getPrimaryHdKeyringEntropySourceId( + keyrings: KeyringObject[] | null | undefined, +): string { + const [primaryEntropySourceId] = getHdKeyringEntropySourceIds(keyrings); + if (!primaryEntropySourceId) { + throw new Error( + 'getPrimaryHdKeyringEntropySourceId - no HD keyring available', + ); + } + return primaryEntropySourceId; +}