Skip to content

Commit 79839a5

Browse files
StaberindeZAStaberindeZA
authored
Merge pull request #20242 from mozilla/fxa-13297-sync-merge-otp
fix(settings): add sync merge check for otp accounts
2 parents 419f5b7 + ff63b80 commit 79839a5

4 files changed

Lines changed: 398 additions & 21 deletions

File tree

‎packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.test.tsx‎

Lines changed: 129 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ import {
2323
import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
2424
import {
2525
createMockSigninOAuthIntegration,
26+
createMockSigninOAuthNativeIntegration,
2627
createMockSigninWebIntegration,
2728
} from '../mocks';
2829
import { SigninOAuthIntegration } from '../interfaces';
2930
import { MozServices } from '../../../lib/types';
3031
import GleanMetrics from '../../../lib/glean';
32+
import { OAuthNativeServices } from '@fxa/accounts/oauth';
3133

3234
jest.mock('../../../lib/metrics', () => ({
3335
usePageViewEvent: jest.fn(),
@@ -70,6 +72,8 @@ function applyDefaultMocks() {
7072

7173
let mockNavigate = jest.fn();
7274
let mockNavigateWithQuery = jest.fn().mockResolvedValue(undefined);
75+
let mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(undefined);
76+
7377
jest.mock('@reach/router', () => {
7478
return {
7579
__esModule: true,
@@ -84,6 +88,14 @@ jest.mock('../../../lib/hooks/useNavigateWithQuery', () => ({
8488
useNavigateWithQuery: () => mockNavigateWithQuery,
8589
}));
8690

91+
let mockHandleNavigation = jest.fn().mockResolvedValue({ error: undefined });
92+
93+
jest.mock('../utils', () => ({
94+
getSyncNavigate: jest.fn((search, options) => ({ to: '/connect_another_device' })),
95+
handleNavigation: (...args: any[]) => mockHandleNavigation(...args),
96+
ensureCanLinkAcountOrRedirect: (...args: any[]) => mockEnsureCanLinkAcountOrRedirect(...args),
97+
}));
98+
8799
jest.mock('../../../lib/hooks/useWebRedirect', () => ({
88100
useWebRedirect: () => ({ isValid: true }),
89101
}));
@@ -135,6 +147,26 @@ describe('SigninPasswordlessCode page', () => {
135147
resetMockAuthClient();
136148
mockNavigate = jest.fn();
137149
mockNavigateWithQuery = jest.fn().mockResolvedValue(undefined);
150+
mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(true);
151+
mockHandleNavigation = jest.fn().mockImplementation(async (navigationOptions) => {
152+
// Simulate handleNavigation behavior for unverified sessions with TOTP
153+
const { signinData } = navigationOptions;
154+
if (signinData.verificationMethod === 'totp-2fa' && !signinData.sessionVerified) {
155+
const mockNavigateModule = jest.requireMock('@reach/router');
156+
mockNavigateModule.navigate(`/signin_totp_code${navigationOptions.queryParams || ''}`, {
157+
state: {
158+
email: navigationOptions.email,
159+
uid: signinData.uid,
160+
sessionToken: signinData.sessionToken,
161+
emailVerified: signinData.emailVerified,
162+
sessionVerified: signinData.sessionVerified,
163+
verificationMethod: signinData.verificationMethod,
164+
verificationReason: signinData.verificationReason,
165+
},
166+
});
167+
}
168+
return { error: undefined };
169+
});
138170
});
139171

140172
afterEach(() => {
@@ -403,7 +435,6 @@ describe('SigninPasswordlessCode page', () => {
403435
});
404436

405437
it('navigates to TOTP code page when account has 2FA', async () => {
406-
const mockNavigateModule = jest.requireMock('@reach/router');
407438
mockAuthClient.passwordlessConfirmCode = jest.fn().mockResolvedValue({
408439
uid: MOCK_UID,
409440
sessionToken: MOCK_SESSION_TOKEN,
@@ -413,20 +444,25 @@ describe('SigninPasswordlessCode page', () => {
413444
verificationReason: 'login',
414445
});
415446

416-
render({ isSignup: false });
447+
const integration = createMockSigninWebIntegration();
448+
integration.isSync = jest.fn().mockReturnValue(true);
449+
450+
render({ integration, isSignup: false });
417451
await submitCode();
418452

419453
await waitFor(() => {
420-
expect(mockNavigateModule.navigate).toHaveBeenCalledWith(
454+
expect(mockNavigateWithQuery).toHaveBeenCalledWith(
421455
'/signin_totp_code',
422456
expect.objectContaining({
457+
replace: true,
423458
state: expect.objectContaining({
424459
email: MOCK_EMAIL,
425460
sessionToken: MOCK_SESSION_TOKEN,
426461
uid: MOCK_UID,
427462
sessionVerified: false,
428463
verificationMethod: 'totp-2fa',
429464
verificationReason: 'login',
465+
isPasswordlessFlow: true,
430466
}),
431467
})
432468
);
@@ -443,7 +479,10 @@ describe('SigninPasswordlessCode page', () => {
443479
verificationReason: 'login',
444480
});
445481

446-
render({ isSignup: false });
482+
const integration = createMockSigninWebIntegration();
483+
integration.isSync = jest.fn().mockReturnValue(true);
484+
485+
render({ integration, isSignup: false });
447486
await submitCode();
448487

449488
await waitFor(() => {
@@ -471,7 +510,6 @@ describe('SigninPasswordlessCode page', () => {
471510
});
472511

473512
const integration = createMockSigninOAuthIntegration();
474-
integration.wantsKeys = jest.fn().mockReturnValue(false);
475513

476514
render({ integration, isSignup: false });
477515
await submitCode();
@@ -506,13 +544,26 @@ describe('SigninPasswordlessCode page', () => {
506544
hardNavigateSpy.mockRestore();
507545
});
508546

509-
it('redirects to set password page when integration wantsKeys', async () => {
547+
it('redirects to set password page for Sync signin flow when user accepts merge', async () => {
548+
// Mock ensureCanLinkAcountOrRedirect to return true (user can link account)
549+
mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(true);
550+
510551
const integration = createMockSigninWebIntegration();
511552
integration.isSync = jest.fn().mockReturnValue(true);
512553

513-
render({ integration });
554+
render({ integration, isSignup: false });
514555
await submitCode();
515556

557+
await waitFor(() => {
558+
expect(mockEnsureCanLinkAcountOrRedirect).toHaveBeenCalledWith(expect.objectContaining(
559+
{
560+
email: MOCK_EMAIL,
561+
uid: MOCK_UID,
562+
navigateWithQuery: mockNavigateWithQuery,
563+
}
564+
))
565+
});
566+
516567
await waitFor(() => {
517568
expect(mockNavigateWithQuery).toHaveBeenCalledWith(
518569
'/post_verify/third_party_auth/set_password',
@@ -526,12 +577,82 @@ describe('SigninPasswordlessCode page', () => {
526577
});
527578
});
528579

580+
it('does not navigate to set password when user rejects Sync merge', async () => {
581+
// Mock ensureCanLinkAcountOrRedirect to return false (user rejected merge)
582+
mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(false);
583+
584+
const integration = createMockSigninWebIntegration();
585+
integration.isSync = jest.fn().mockReturnValue(true);
586+
587+
render({ integration, isSignup: false });
588+
await submitCode();
589+
590+
await waitFor(() => {
591+
expect(mockEnsureCanLinkAcountOrRedirect).toHaveBeenCalled();
592+
});
593+
594+
// Should not navigate to set password page when user rejects merge
595+
expect(mockNavigateWithQuery).not.toHaveBeenCalledWith(
596+
'/post_verify/third_party_auth/set_password',
597+
expect.anything()
598+
);
599+
});
600+
601+
it('skips merge check for Sync signup flow', async () => {
602+
mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(true);
603+
604+
const integration = createMockSigninWebIntegration();
605+
integration.isSync = jest.fn().mockReturnValue(true);
606+
607+
render({ integration, isSignup: true });
608+
await submitCode();
609+
610+
// ensureCanLinkAcountOrRedirect should NOT be called for signup flows
611+
await waitFor(() => {
612+
expect(mockNavigateWithQuery).toHaveBeenCalledWith(
613+
'/post_verify/third_party_auth/set_password',
614+
expect.objectContaining({
615+
replace: true,
616+
state: {
617+
isPasswordlessFlow: true,
618+
},
619+
})
620+
);
621+
});
622+
623+
expect(mockEnsureCanLinkAcountOrRedirect).not.toHaveBeenCalled();
624+
});
625+
626+
it('redirects to set password page for Firefox non-Sync (Relay) signin flow when user accepts merge', async () => {
627+
mockEnsureCanLinkAcountOrRedirect = jest.fn().mockResolvedValue(true);
628+
629+
const integration = createMockSigninOAuthNativeIntegration({
630+
service: OAuthNativeServices.Relay,
631+
isSync: false,
632+
});
633+
634+
render({ integration, isSignup: false });
635+
await submitCode();
636+
637+
await waitFor(() => {
638+
expect(mockEnsureCanLinkAcountOrRedirect).toHaveBeenCalledWith(
639+
expect.objectContaining({
640+
email: MOCK_EMAIL,
641+
uid: MOCK_UID,
642+
navigateWithQuery: mockNavigateWithQuery,
643+
})
644+
);
645+
});
646+
await waitFor(() => {
647+
expect(mockHandleNavigation).toHaveBeenCalled();
648+
});
649+
});
650+
529651
it('with OAuth integration', async () => {
530652
const finishOAuthFlowHandler = jest
531653
.fn()
532654
.mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE);
533655
const integration = createMockSigninOAuthIntegration();
534-
integration.wantsKeys = jest.fn().mockReturnValue(false);
535656

536657
render({ finishOAuthFlowHandler, integration, isSignup: true });
537658
await submitCode();
@@ -546,7 +667,6 @@ describe('SigninPasswordlessCode page', () => {
546667

547668
it('redirects to TOTP setup when integration wantsTwoStepAuthentication', async () => {
548669
const integration = createMockSigninOAuthIntegration();
549-
integration.wantsKeys = jest.fn().mockReturnValue(false);
550670
integration.wantsTwoStepAuthentication = jest.fn().mockReturnValue(true);
551671

552672
render({ integration, isSignup: true });
@@ -568,7 +688,6 @@ describe('SigninPasswordlessCode page', () => {
568688
it('with web integration and valid redirect', async () => {
569689
const integration = createMockSigninWebIntegration();
570690
integration.data.redirectTo = 'https://mozilla.org';
571-
integration.wantsKeys = jest.fn().mockReturnValue(false);
572691

573692
render({ integration, isSignup: true });
574693
await submitCode();
@@ -582,7 +701,6 @@ describe('SigninPasswordlessCode page', () => {
582701

583702
it('navigates to settings when web integration without redirectTo', async () => {
584703
const integration = createMockSigninWebIntegration();
585-
integration.wantsKeys = jest.fn().mockReturnValue(false);
586704

587705
render({ integration, isSignup: true });
588706
await submitCode();

‎packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.tsx‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
isSyncDesktopV3Integration,
3333
isWebIntegration,
3434
} from '../../../models';
35-
import { getSyncNavigate, handleNavigation } from '../utils';
35+
import { getSyncNavigate, handleNavigation, ensureCanLinkAcountOrRedirect } from '../utils';
3636
import firefox from '../../../lib/channels/firefox';
3737
import { useWebRedirect } from '../../../lib/hooks/useWebRedirect';
3838
import VerificationMethods from '../../../constants/verification-methods';
@@ -208,6 +208,21 @@ const SigninPasswordlessCode = ({
208208
sessionVerified: isSessionVerified,
209209
});
210210

211+
// For existing users signing into Sync (signin flow), show merge warning
212+
// before navigating to set password. For signup flows, the warning was
213+
// already shown on the email-first page.
214+
if ((integration.isSync() || integration.isFirefoxNonSync()) && !isSignup) {
215+
const canLink = await ensureCanLinkAcountOrRedirect({
216+
email,
217+
uid: result.uid,
218+
ftlMsgResolver,
219+
navigateWithQuery,
220+
});
221+
if (!canLink) {
222+
return;
223+
}
224+
}
225+
211226
// Sync flows need a password to derive encryption keys (unwrapBKey).
212227
// TOTP accounts must verify first since /password/create requires
213228
// a verifiedSessionToken.

0 commit comments

Comments
 (0)