Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/profile-sync-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ([#10394](https://github.com/MetaMask/core/pull/10394))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -823,6 +824,117 @@ 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.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' },
{ 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();
Expand Down Expand Up @@ -1015,6 +1127,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();
Expand Down Expand Up @@ -3470,6 +3630,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import type {
LoginIdentifierType,
LoginResponse,
ProfileAlias,
ProfileIdentifier,
SRPInterface,
SrpLoginTag,
UserProfile,
Expand Down Expand Up @@ -137,7 +138,7 @@ const metadata: StateMetadata<AuthenticationControllerState> = {
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
Expand All @@ -151,9 +152,14 @@ const metadata: StateMetadata<AuthenticationControllerState> = {
(sanitizedSrpSessionData, [key, value]) => {
const { accessToken: _unused, ...tokenWithoutAccessToken } =
value.token;
const {
pairedIdentifierIds: _unusedPairedIdentifierIds,
...profileWithoutPairedIdentifierIds
} = value.profile;
sanitizedSrpSessionData[key] = {
...value,
token: tokenWithoutAccessToken,
profile: profileWithoutPairedIdentifierIds,
};
return sanitizedSrpSessionData;
},
Expand Down Expand Up @@ -190,7 +196,6 @@ const metadata: StateMetadata<AuthenticationControllerState> = {
* action window, per the MFA phase-1 specification.
*/
export const STEP_UP_SESSION_TTL_MS = 60_000;

type ControllerConfig = {
env: Env;
/**
Expand Down Expand Up @@ -423,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 } : {}),
},
};
});
Expand Down Expand Up @@ -690,14 +701,15 @@ export class AuthenticationController extends BaseController<
}

try {
await this.#auth.pairSocialIdentifier(
const pairedIdentifierIds = await this.#auth.pairSocialIdentifier(
{
identifierType,
socialJwt,
...(email ? { email } : {}),
},
primaryAccessToken,
);
this.#setPrimaryPairedIdentifierIds(pairedIdentifierIds);
this.#clearNeedsSocialPairing();
} catch (error) {
if (error instanceof PairConflictError) {
Expand Down Expand Up @@ -793,12 +805,34 @@ 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, if any.
*/
#setPrimaryPairedIdentifierIds(
pairedIdentifierIds: ProfileIdentifier[] | undefined,
): void {
if (!pairedIdentifierIds) {
return;
}
const primaryEntropySourceId = this.#getPrimaryEntropySourceId();
this.update((state) => {
const entry = state.srpSessionData?.[primaryEntropySourceId];
if (entry?.profile) {
entry.profile.pairedIdentifierIds = pairedIdentifierIds;
}
Comment thread
cursor[bot] marked this conversation as resolved.
});
}

#propagateCanonical(canonicalProfileId: string): void {
const { srpSessionData } = this.state;
if (!srpSessionData) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import type {
OidcTokenAudience,
OidcTokenClaims,
PairSocialIdentifierParams,
ProfileIdentifier,
SrpLoginTag,
UserProfile,
UserProfileLineage,
Expand Down Expand Up @@ -390,8 +391,12 @@ export class SRPJwtBearerAuth implements IBaseAuth {
async pairSocialIdentifier(
params: PairSocialIdentifierParams,
authAccessToken: string,
): Promise<void> {
await pairSocialIdentifier(params, authAccessToken, this.#config.env);
): Promise<ProfileIdentifier[] | undefined> {
return await pairSocialIdentifier(
params,
authAccessToken,
this.#config.env,
);
}

async pairSrpProfiles(
Expand Down
Loading
Loading