From dbe6f057abb06d99b56704bd95c9eb0f43ee4da1 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 23 Sep 2026 13:14:17 +0200 Subject: [PATCH 1/3] feat(authentication-controller): Add pairedIdentifierIds to UserProfile in srpSessionData --- packages/profile-sync-controller/CHANGELOG.md | 1 + .../AuthenticationController.test.ts | 150 ++++++++++++++++++ .../AuthenticationController.ts | 31 +++- .../sdk/authentication-jwt-bearer/flow-srp.ts | 5 +- .../services.test.ts | 80 ++++++++-- .../sdk/authentication-jwt-bearer/services.ts | 15 +- .../sdk/authentication-jwt-bearer/types.ts | 11 +- .../src/sdk/authentication.test.ts | 2 +- .../src/sdk/authentication.ts | 5 +- .../src/sdk/utils/validate-pair-response.ts | 1 + 10 files changed, 275 insertions(+), 26 deletions(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 5ee18f0c2ec..0e7ce3bc6bb 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `beginMfaEnrollment` accepts an `accessToken` option so enrollment can begin with an elevated token - `AuthenticationController.beginCredentialEnrollment` sends the elevated token while a step-up session is live, since enrolling additional credentials requires AAL2 - Add the `email_socially_verified`, `multi_primary_srp` and `aal2_required` MFA error codes, a `StepUpRequiredError` class, and support for the `retry_after_seconds` error field when computing `retryAfterMs` +- Add `pairedIdentifierIds` to `UserProfile` in `srpSessionData`, set from the login response and, on the primary SRP session, from the SRP and social pairing responses, so clients can tell whether a profile has been socially paired ([#XXXXX](https://github.com/MetaMask/core/pull/XXXXX)) ### Changed 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 6c7117a57f1..92f0876d1c6 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -45,6 +45,7 @@ import type { import { MOCK_LOGIN_RESPONSE, MOCK_OATH_TOKEN_RESPONSE, + MOCK_PAIR_SOCIAL_IDENTIFIER_RESPONSE, } from './mocks/mockResponses.js'; jest.mock('../../shared/utils/message-signing.js', () => ({ @@ -823,6 +824,79 @@ describe('AuthenticationController', () => { } }); + it('stores paired identifiers from the pair response on the primary SRP session only', async () => { + const pairedIdentifierIds = [ + { id: 'h1', type: 'SRP' }, + { id: 'id-1', type: 'SRP' }, + ]; + arrangeAuthAPIs({ + mockPairProfiles: { + status: 200, + body: { + profile: { + identifier_id: 'id-1', + metametrics_id: 'mm-1', + profile_id: MOCK_LOGIN_RESPONSE.profile.profile_id, + paired_identifier_ids: pairedIdentifierIds, + }, + profile_aliases: [], + }, + }, + }); + const controller = new AuthenticationController({ + messenger: createMockAuthenticationMessenger().messenger, + state: mockSignedInState({ needsProfilePairing: true }), + metametrics: createMockAuthMetaMetrics(), + }); + + await controller.performSignIn(); + + const [primaryId, secondaryId] = MOCK_ENTROPY_SOURCE_IDS; + expect( + controller.state.srpSessionData?.[primaryId]?.profile + .pairedIdentifierIds, + ).toStrictEqual(pairedIdentifierIds); + expect( + controller.state.srpSessionData?.[secondaryId]?.profile, + ).not.toHaveProperty('pairedIdentifierIds'); + }); + + it('stores paired identifiers returned by login on the SRP session', async () => { + const pairedIdentifierIds = [ + { id: 'id-google', type: 'GOOGLE' }, + { id: MOCK_LOGIN_RESPONSE.profile.identifier_id, type: 'SRP' }, + ]; + arrangeAuthAPIs({ + mockSrpLoginUrl: { + status: 200, + body: { + ...MOCK_LOGIN_RESPONSE, + profile: { + ...MOCK_LOGIN_RESPONSE.profile, + paired_identifier_ids: pairedIdentifierIds, + }, + }, + }, + }); + const { messenger, mockKeyringControllerGetState } = + createMockAuthenticationMessenger(); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); + const controller = new AuthenticationController({ + messenger, + metametrics: createMockAuthMetaMetrics(), + }); + + await controller.performSignIn(); + + expect( + controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]]?.profile + .pairedIdentifierIds, + ).toStrictEqual(pairedIdentifierIds); + }); + it('epoch check: a concurrent requestProfilePairing during multi-SRP performSignIn keeps the gate set', async () => { const metametrics = createMockAuthMetaMetrics(); arrangeAuthAPIs(); @@ -1015,6 +1089,54 @@ describe('AuthenticationController', () => { expect(accessTokens[0]).toBe(MOCK_OATH_TOKEN_RESPONSE.access_token); }); + it('stores paired identifiers from the pair/identifier response on the primary SRP session', async () => { + const pairedIdentifierIds = [ + { id: 'id-google', type: 'GOOGLE' }, + { id: MOCK_LOGIN_RESPONSE.profile.identifier_id, type: 'SRP' }, + ]; + arrangeAuthAPIs({ + mockPairSocialIdentifier: { + status: 200, + body: { + ...MOCK_PAIR_SOCIAL_IDENTIFIER_RESPONSE, + profile: { + ...MOCK_PAIR_SOCIAL_IDENTIFIER_RESPONSE.profile, + paired_identifier_ids: pairedIdentifierIds, + }, + }, + }, + }); + const { + messenger, + mockKeyringControllerGetState, + mockSeedlessOnboardingGetState, + mockSeedlessOnboardingGetAccessToken, + } = createMockAuthenticationMessenger(); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings: mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + }); + mockSeedlessOnboardingGetState.mockReturnValue({ + vault: 'encrypted', + authConnection: 'google', + socialLoginEmail: MOCK_SOCIAL_EMAIL, + }); + mockSeedlessOnboardingGetAccessToken.mockResolvedValue(MOCK_SOCIAL_JWT); + + const controller = new AuthenticationController({ + messenger, + metametrics: createMockAuthMetaMetrics(), + config: SOCIAL_PAIRING_ENABLED, + }); + + await controller.performSignIn(); + + expect( + controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]]?.profile + .pairedIdentifierIds, + ).toStrictEqual(pairedIdentifierIds); + }); + it('treats undefined needsSocialPairing as still needed', async () => { const metametrics = createMockAuthMetaMetrics(); const mockEndpoints = arrangeAuthAPIs(); @@ -3470,6 +3592,34 @@ describe('metadata', () => { ]); }); + it('strips paired identifiers out of state logs', () => { + const state = mockSignedInState(); + const primaryEntry = state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]]; + if (primaryEntry) { + primaryEntry.profile.pairedIdentifierIds = [ + { id: 'hashed-google-sub', type: 'GOOGLE' }, + ]; + } + const controller = new AuthenticationController({ + messenger: createMockAuthenticationMessenger().messenger, + metametrics: createMockAuthMetaMetrics(), + state, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).not.toHaveProperty([ + 'srpSessionData', + MOCK_ENTROPY_SOURCE_IDS[0], + 'profile', + 'pairedIdentifierIds', + ]); + }); + it('includes expected state in state logs, with access token stripped out', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 9b66b0b60f0..28ef2b9b156 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -32,6 +32,7 @@ import type { LoginIdentifierType, LoginResponse, ProfileAlias, + ProfileIdentifier, SRPInterface, SrpLoginTag, UserProfile, @@ -137,7 +138,7 @@ const metadata: StateMetadata = { usedInUi: true, }, srpSessionData: { - // Remove access token from state logs + // Remove access token and paired identifiers from state logs includeInStateLogs: (srpSessionData) => { // Unreachable branch, included just to fix a type error for the case where this property is // unset. The type gets collapsed to include `| undefined` even though `undefined` is never @@ -151,9 +152,12 @@ const metadata: StateMetadata = { (sanitizedSrpSessionData, [key, value]) => { const { accessToken: _unused, ...tokenWithoutAccessToken } = value.token; + const { pairedIdentifierIds: _unusedPairedIdentifierIds, ...profileWithoutPairedIdentifierIds } = + value.profile; sanitizedSrpSessionData[key] = { ...value, token: tokenWithoutAccessToken, + profile: profileWithoutPairedIdentifierIds, }; return sanitizedSrpSessionData; }, @@ -190,7 +194,6 @@ const metadata: StateMetadata = { * action window, per the MFA phase-1 specification. */ export const STEP_UP_SESSION_TTL_MS = 60_000; - type ControllerConfig = { env: Env; /** @@ -690,7 +693,7 @@ export class AuthenticationController extends BaseController< } try { - await this.#auth.pairSocialIdentifier( + const pairedIdentifierIds = await this.#auth.pairSocialIdentifier( { identifierType, socialJwt, @@ -698,6 +701,7 @@ export class AuthenticationController extends BaseController< }, primaryAccessToken, ); + this.#setPrimaryPairedIdentifierIds(pairedIdentifierIds); this.#clearNeedsSocialPairing(); } catch (error) { if (error instanceof PairConflictError) { @@ -793,12 +797,31 @@ export class AuthenticationController extends BaseController< const primaryAccessToken = accessTokens[0]; // Associated with primary SRP. const { profileAliases, - profile: { canonicalProfileId }, + profile: { canonicalProfileId, pairedIdentifierIds }, } = await this.#auth.pairSrpProfiles(accessTokens, primaryAccessToken); this.#propagateCanonical(canonicalProfileId); + this.#setPrimaryPairedIdentifierIds(pairedIdentifierIds); return profileAliases; } + /** + * Pair calls use the primary SRP's token, so their response only describes + * the primary's profile: secondaries skipped by the server are not in it. + * + * @param pairedIdentifierIds - Identifiers returned by the pair call. + */ + #setPrimaryPairedIdentifierIds( + pairedIdentifierIds: ProfileIdentifier[] = [], + ): void { + const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); + this.update((state) => { + const entry = state.srpSessionData?.[primaryEntropySourceId]; + if (entry?.profile) { + entry.profile.pairedIdentifierIds = pairedIdentifierIds; + } + }); + } + #propagateCanonical(canonicalProfileId: string): void { const { srpSessionData } = this.state; if (!srpSessionData) { diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts index 5ab7eb5e185..6f39e862cae 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts @@ -49,6 +49,7 @@ import type { OidcTokenAudience, OidcTokenClaims, PairSocialIdentifierParams, + ProfileIdentifier, SrpLoginTag, UserProfile, UserProfileLineage, @@ -390,8 +391,8 @@ export class SRPJwtBearerAuth implements IBaseAuth { async pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, - ): Promise { - await pairSocialIdentifier(params, authAccessToken, this.#config.env); + ): Promise { + return await pairSocialIdentifier(params, authAccessToken, this.#config.env); } async pairSrpProfiles( diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts index c99fb3e5a3f..6958be705a8 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts @@ -384,6 +384,7 @@ describe('services', () => { metaMetricsId: 'mm-1', profileId: 'profile-1', canonicalProfileId: 'profile-1', + pairedIdentifierIds: [], }, profileAliases: [], }); @@ -399,6 +400,33 @@ describe('services', () => { ); }); + it('should map paired_identifier_ids onto the profile', async () => { + mockFetch.mockResolvedValue( + createMockResponse({ + ...mockAuthResponse, + profile: { + ...mockAuthResponse.profile, + paired_identifier_ids: [ + { id: 'id-google', type: 'GOOGLE' }, + { id: 'id-1', type: 'SRP' }, + ], + }, + }), + ); + + const result = await authenticate( + 'raw-message', + 'signature', + AuthType.SRP, + Env.DEV, + ); + + expect(result.profile.pairedIdentifierIds).toStrictEqual([ + { id: 'id-google', type: 'GOOGLE' }, + { id: 'id-1', type: 'SRP' }, + ]); + }); + it('should send X-MetaMask-Profile-Pairing header for SRP', async () => { const mockResponse = createMockResponse(mockAuthResponse); mockFetch.mockResolvedValue(mockResponse); @@ -937,6 +965,31 @@ describe('services', () => { }); }); + it('should map paired_identifier_ids onto the profile', async () => { + mockFetch.mockResolvedValue( + createMockResponse({ + ...mockPairApiResponse, + profile: { + ...mockPairApiResponse.profile, + paired_identifier_ids: [ + { id: 'h1', type: 'SRP' }, + { id: 'id-canonical', type: 'SRP' }, + ], + }, + }), + ); + + const result = await pairProfiles( + ['token-1', 'token-2'], + 'auth-access-token', + Env.DEV, + ); + + expect(result.profile.pairedIdentifierIds).toStrictEqual([ + { id: 'h1', type: 'SRP' }, + { id: 'id-canonical', type: 'SRP' }, + ]); + }); it('should throw PairError on network failure', async () => { mockFetch.mockRejectedValue(new Error('Connection refused')); @@ -1082,23 +1135,28 @@ describe('services', () => { ); }); - it('should resolve without reading the response body on 2xx', async () => { - const mockResponse = createMockResponse(mockSocialPairResponse); - const jsonSpy = jest.spyOn(mockResponse, 'json'); - mockFetch.mockResolvedValue(mockResponse); + it('should return the paired identifiers from the response', async () => { + const pairedIdentifierIds = [ + { id: 'id-social', type: 'GOOGLE' }, + { id: 'id-srp', type: 'SRP' }, + ]; + mockFetch.mockResolvedValue( + createMockResponse({ + ...mockSocialPairResponse, + profile: { + ...mockSocialPairResponse.profile, + paired_identifier_ids: pairedIdentifierIds, + }, + }), + ); expect( await pairSocialIdentifier( - { - identifierType: 'GOOGLE', - socialJwt: 'social-jwt', - email: 'user@example.com', - }, + { identifierType: 'APPLE', socialJwt: 'social-jwt' }, 'primary-srp-token', Env.DEV, ), - ).toBeUndefined(); - expect(jsonSpy).not.toHaveBeenCalled(); + ).toStrictEqual(pairedIdentifierIds); }); it('should throw PairError on network failure', async () => { diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 38e6e87fa20..977fa890363 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -20,6 +20,7 @@ import type { OidcTokenClaims, PairSocialIdentifierParams, ProfileAlias, + ProfileIdentifier, UserProfile, UserProfileLineage, } from './types.js'; @@ -317,6 +318,7 @@ export async function pairProfiles( metaMetricsId: pairResponse.profile.metametrics_id ?? '', profileId: pairResponse.profile.profile_id, canonicalProfileId: pairResponse.profile.profile_id, + pairedIdentifierIds: pairResponse.profile.paired_identifier_ids ?? [], }, profileAliases: parseProfileAliases(pairResponse.profile_aliases ?? []), }; @@ -330,19 +332,18 @@ export async function pairProfiles( * owns `authAccessToken` (`POST /api/v2/profile/pair/identifier`). * * `email` is omitted from the body when undefined. 409 Conflict throws - * {@link PairConflictError} so callers can treat it as terminal. The - * response body (new token + profile) is not consumed: a 2xx status is the - * only success signal. + * {@link PairConflictError} so callers can treat it as terminal. * * @param params - Social identifier type, social JWT, and optional email * @param authAccessToken - Bearer token of the canonical (primary SRP) profile * @param env - server environment + * @returns The profile's paired identifiers, including the new one */ export async function pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, env: Env, -): Promise { +): Promise { const pairUrl = new URL(PAIR_SOCIAL_IDENTIFIER_URL(env)); const errorPrefix = 'Failed to pair social identifier'; @@ -366,8 +367,11 @@ export async function pairSocialIdentifier( } await throwServiceError(response, errorPrefix, PairError); } + + const pairResponse = await response.json(); + return pairResponse.profile.paired_identifier_ids ?? []; } catch (error) { - await throwServiceError(error, errorPrefix, PairError); + return await throwServiceError(error, errorPrefix, PairError); } } @@ -531,6 +535,7 @@ export async function authenticate( metaMetricsId: loginResponse.profile.metametrics_id, profileId: loginResponse.profile.profile_id, canonicalProfileId: loginResponse.profile.profile_id, + pairedIdentifierIds: loginResponse.profile.paired_identifier_ids ?? [], }, profileAliases: parseProfileAliases(loginResponse.profile_aliases ?? []), }; diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts index ec1cada5ab9..ef6ab9b7bd1 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts @@ -38,7 +38,6 @@ export type PairSocialIdentifierParams = { */ email?: string; }; - /** * Claim names accepted by `POST /api/v2/oidc/token`. Only `email` is * supported; `email_verified` is set by the server when email is present. @@ -86,6 +85,11 @@ export type AccessToken = { obtainedAt: number; }; +export type ProfileIdentifier = { + id: string; + type: string; +}; + export type UserProfile = { /** * The "Identifier" used to log in with. @@ -106,6 +110,11 @@ export type UserProfile = { * Server MetaMetrics ID. Allows grouping of user events cross platform. */ metaMetricsId: string; + /** + * All identifiers attached to this profile (SRP, social, MFA). + * Absent on sessions stored before this field existed. + */ + pairedIdentifierIds?: ProfileIdentifier[]; }; /** diff --git a/packages/profile-sync-controller/src/sdk/authentication.test.ts b/packages/profile-sync-controller/src/sdk/authentication.test.ts index 2da8e022ad0..5ab739afafb 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.test.ts @@ -674,7 +674,7 @@ describe('Authentication - pairSocialIdentifier()', () => { }, 'primary-srp-token', ), - ).toBeUndefined(); + ).toStrictEqual([]); expect(mockPairSocialIdentifierUrl.isDone()).toBe(true); }); diff --git a/packages/profile-sync-controller/src/sdk/authentication.ts b/packages/profile-sync-controller/src/sdk/authentication.ts index 6f6c2fbb315..5e2e97a34fd 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.ts @@ -23,6 +23,7 @@ import type { UserProfile, Pair, PairSocialIdentifierParams, + ProfileIdentifier, OidcTokenAudience, OidcTokenClaims, UserProfileLineage, @@ -177,9 +178,9 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface { async pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, - ): Promise { + ): Promise { this.#assertSRP(this.#type, this.#sdk); - await this.#sdk.pairSocialIdentifier(params, authAccessToken); + return await this.#sdk.pairSocialIdentifier(params, authAccessToken); } async signMessage( diff --git a/packages/profile-sync-controller/src/sdk/utils/validate-pair-response.ts b/packages/profile-sync-controller/src/sdk/utils/validate-pair-response.ts index 563c6cfeb93..2a96e9e32b2 100644 --- a/packages/profile-sync-controller/src/sdk/utils/validate-pair-response.ts +++ b/packages/profile-sync-controller/src/sdk/utils/validate-pair-response.ts @@ -11,6 +11,7 @@ export type RawPairResponse = { profile_id: string; identifier_id: string; metametrics_id?: string; + paired_identifier_ids?: { id: string; type: string }[]; }; profile_aliases?: RawProfileAlias[]; }; From baf2c82032f7fd059170ef336e2daaf05b9553b5 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 23 Sep 2026 13:15:31 +0200 Subject: [PATCH 2/3] fix: update CHANGELOG --- packages/profile-sync-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 0e7ce3bc6bb..60129bf9172 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `beginMfaEnrollment` accepts an `accessToken` option so enrollment can begin with an elevated token - `AuthenticationController.beginCredentialEnrollment` sends the elevated token while a step-up session is live, since enrolling additional credentials requires AAL2 - Add the `email_socially_verified`, `multi_primary_srp` and `aal2_required` MFA error codes, a `StepUpRequiredError` class, and support for the `retry_after_seconds` error field when computing `retryAfterMs` -- Add `pairedIdentifierIds` to `UserProfile` in `srpSessionData`, set from the login response and, on the primary SRP session, from the SRP and social pairing responses, so clients can tell whether a profile has been socially paired ([#XXXXX](https://github.com/MetaMask/core/pull/XXXXX)) +- Add `pairedIdentifierIds` to `UserProfile` in `srpSessionData`, set from the login response and, on the primary SRP session, from the SRP and social pairing responses, so clients can tell whether a profile has been socially paired ([#10394](https://github.com/MetaMask/core/pull/10394)) ### Changed From 8f2c14bcd142dff6712ed5d2e11477469c2ce3e8 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 23 Sep 2026 13:26:29 +0200 Subject: [PATCH 3/3] fix: address cursor feedback --- .../AuthenticationController.test.ts | 38 +++++++++++++++++++ .../AuthenticationController.ts | 19 ++++++++-- .../sdk/authentication-jwt-bearer/flow-srp.ts | 8 +++- .../services.test.ts | 2 +- .../sdk/authentication-jwt-bearer/services.ts | 10 ++--- .../src/sdk/authentication.test.ts | 2 +- .../src/sdk/authentication.ts | 2 +- 7 files changed, 67 insertions(+), 14 deletions(-) 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 92f0876d1c6..7b7ed8eeedc 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -861,6 +861,44 @@ describe('AuthenticationController', () => { ).not.toHaveProperty('pairedIdentifierIds'); }); + it.each([ + ['a pair response', { needsProfilePairing: true }, MOCK_HD_KEYRINGS], + [ + 'a login response', + { expiresIn: 0 }, + mockHdKeyrings(MOCK_ENTROPY_SOURCE_IDS[0]), + ], + ])( + 'keeps stored paired identifiers when %s omits them', + async (_, stateOptions, keyrings) => { + arrangeAuthAPIs(); + const state = mockSignedInState(stateOptions); + const storedIds = [{ id: 'id-google', type: 'GOOGLE' }]; + const primaryEntry = state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]]; + if (primaryEntry) { + primaryEntry.profile.pairedIdentifierIds = storedIds; + } + const { messenger, mockKeyringControllerGetState } = + createMockAuthenticationMessenger(); + mockKeyringControllerGetState.mockReturnValue({ + isUnlocked: true, + keyrings, + }); + const controller = new AuthenticationController({ + messenger, + state, + metametrics: createMockAuthMetaMetrics(), + }); + + await controller.performSignIn(); + + expect( + controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]]?.profile + .pairedIdentifierIds, + ).toStrictEqual(storedIds); + }, + ); + it('stores paired identifiers returned by login on the SRP session', async () => { const pairedIdentifierIds = [ { id: 'id-google', type: 'GOOGLE' }, diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 28ef2b9b156..77ca0a1bf5d 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -152,8 +152,10 @@ const metadata: StateMetadata = { (sanitizedSrpSessionData, [key, value]) => { const { accessToken: _unused, ...tokenWithoutAccessToken } = value.token; - const { pairedIdentifierIds: _unusedPairedIdentifierIds, ...profileWithoutPairedIdentifierIds } = - value.profile; + const { + pairedIdentifierIds: _unusedPairedIdentifierIds, + ...profileWithoutPairedIdentifierIds + } = value.profile; sanitizedSrpSessionData[key] = { ...value, token: tokenWithoutAccessToken, @@ -426,11 +428,17 @@ export class AuthenticationController extends BaseController< if (!state.srpSessionData) { state.srpSessionData = {}; } + // The API omits `paired_identifier_ids` when it fails to load them, so + // keep the last known value rather than wiping it. + const pairedIdentifierIds = + loginResponse.profile.pairedIdentifierIds ?? + state.srpSessionData[resolvedId]?.profile.pairedIdentifierIds; state.srpSessionData[resolvedId] = { ...loginResponse, profile: { ...loginResponse.profile, metaMetricsId, + ...(pairedIdentifierIds ? { pairedIdentifierIds } : {}), }, }; }); @@ -808,11 +816,14 @@ export class AuthenticationController extends BaseController< * Pair calls use the primary SRP's token, so their response only describes * the primary's profile: secondaries skipped by the server are not in it. * - * @param pairedIdentifierIds - Identifiers returned by the pair call. + * @param pairedIdentifierIds - Identifiers returned by the pair call, if any. */ #setPrimaryPairedIdentifierIds( - pairedIdentifierIds: ProfileIdentifier[] = [], + pairedIdentifierIds: ProfileIdentifier[] | undefined, ): void { + if (!pairedIdentifierIds) { + return; + } const primaryEntropySourceId = this.#getPrimaryEntropySourceId(); this.update((state) => { const entry = state.srpSessionData?.[primaryEntropySourceId]; diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts index 6f39e862cae..58f96938056 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts @@ -391,8 +391,12 @@ export class SRPJwtBearerAuth implements IBaseAuth { async pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, - ): Promise { - return await pairSocialIdentifier(params, authAccessToken, this.#config.env); + ): Promise { + return await pairSocialIdentifier( + params, + authAccessToken, + this.#config.env, + ); } async pairSrpProfiles( diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts index 6958be705a8..f2c5efcdf5f 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.test.ts @@ -384,7 +384,7 @@ describe('services', () => { metaMetricsId: 'mm-1', profileId: 'profile-1', canonicalProfileId: 'profile-1', - pairedIdentifierIds: [], + pairedIdentifierIds: undefined, }, profileAliases: [], }); diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 977fa890363..b4ee1d8df0b 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -318,7 +318,7 @@ export async function pairProfiles( metaMetricsId: pairResponse.profile.metametrics_id ?? '', profileId: pairResponse.profile.profile_id, canonicalProfileId: pairResponse.profile.profile_id, - pairedIdentifierIds: pairResponse.profile.paired_identifier_ids ?? [], + pairedIdentifierIds: pairResponse.profile.paired_identifier_ids, }, profileAliases: parseProfileAliases(pairResponse.profile_aliases ?? []), }; @@ -337,13 +337,13 @@ export async function pairProfiles( * @param params - Social identifier type, social JWT, and optional email * @param authAccessToken - Bearer token of the canonical (primary SRP) profile * @param env - server environment - * @returns The profile's paired identifiers, including the new one + * @returns The profile's paired identifiers, if returned by the API */ export async function pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, env: Env, -): Promise { +): Promise { const pairUrl = new URL(PAIR_SOCIAL_IDENTIFIER_URL(env)); const errorPrefix = 'Failed to pair social identifier'; @@ -369,7 +369,7 @@ export async function pairSocialIdentifier( } const pairResponse = await response.json(); - return pairResponse.profile.paired_identifier_ids ?? []; + return pairResponse.profile.paired_identifier_ids; } catch (error) { return await throwServiceError(error, errorPrefix, PairError); } @@ -535,7 +535,7 @@ export async function authenticate( metaMetricsId: loginResponse.profile.metametrics_id, profileId: loginResponse.profile.profile_id, canonicalProfileId: loginResponse.profile.profile_id, - pairedIdentifierIds: loginResponse.profile.paired_identifier_ids ?? [], + pairedIdentifierIds: loginResponse.profile.paired_identifier_ids, }, profileAliases: parseProfileAliases(loginResponse.profile_aliases ?? []), }; diff --git a/packages/profile-sync-controller/src/sdk/authentication.test.ts b/packages/profile-sync-controller/src/sdk/authentication.test.ts index 5ab739afafb..2da8e022ad0 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.test.ts @@ -674,7 +674,7 @@ describe('Authentication - pairSocialIdentifier()', () => { }, 'primary-srp-token', ), - ).toStrictEqual([]); + ).toBeUndefined(); expect(mockPairSocialIdentifierUrl.isDone()).toBe(true); }); diff --git a/packages/profile-sync-controller/src/sdk/authentication.ts b/packages/profile-sync-controller/src/sdk/authentication.ts index 5e2e97a34fd..43e04ea1d99 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.ts @@ -178,7 +178,7 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface { async pairSocialIdentifier( params: PairSocialIdentifierParams, authAccessToken: string, - ): Promise { + ): Promise { this.#assertSRP(this.#type, this.#sdk); return await this.#sdk.pairSocialIdentifier(params, authAccessToken); }