Skip to content

Commit 875b4b3

Browse files
StaberindeZAStaberindeZA
authored
Merge pull request #18909 from mozilla/fxa-11700-replace-oauth
feat(react): navigate to RPs without adding to browser history
2 parents 72df06b + 0ceac54 commit 875b4b3

8 files changed

Lines changed: 71 additions & 47 deletions

File tree

‎packages/fxa-react/lib/utils.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import React from 'react';
1313
export function hardNavigate(
1414
href: string,
1515
additionalQueryParams = {},
16-
includeCurrentQueryParams = false
16+
includeCurrentQueryParams = false,
17+
replace: boolean = false,
1718
) {
1819
// If there are any query params in the href, we automatically include them in the new url.
1920
const url = new URL(href, window.location.origin);
@@ -32,7 +33,11 @@ export function hardNavigate(
3233
url.searchParams.append(key, value);
3334
});
3435

35-
window.location.href = url.href;
36+
if (replace) {
37+
window.location.replace(url.href);
38+
} else {
39+
window.location.href = url.href;
40+
}
3641
}
3742

3843
export enum LocalizedDateOptions {
@@ -98,7 +103,7 @@ export class FtlMsgResolver {
98103
constructor(
99104
public readonly l10n: ReactLocalization,
100105
public readonly strict: boolean
101-
) {}
106+
) { }
102107

103108
/**
104109
* A wrapper around l10n.getString that provides more safety. When strict is true, using this function ensures:

‎packages/fxa-settings/src/pages/Signin/SigninPushCode/container.test.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ function mockSigninPushCodeModule() {
9898
}
9999

100100
function mockReactUtilsModule() {
101-
jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => {});
101+
jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => { });
102102
}
103103

104104
// Set this when testing local storage
@@ -224,7 +224,10 @@ describe('SigninPushCode container', () => {
224224

225225
await waitFor(() =>
226226
expect(ReactUtils.hardNavigate).toBeCalledWith(
227-
'/pair?showSuccessMessage=true'
227+
'/pair?showSuccessMessage=true',
228+
undefined,
229+
undefined,
230+
false
228231
)
229232
);
230233
});

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jest.mock('@reach/router', () => {
5151
__esModule: true,
5252
...jest.requireActual('@reach/router'),
5353
navigate: jest.fn(),
54-
useLocation: () => () => {},
54+
useLocation: () => () => { },
5555
};
5656
});
5757

@@ -69,7 +69,7 @@ function render(
6969
}
7070

7171
function mockReactUtilsModule() {
72-
jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => {});
72+
jest.spyOn(ReactUtils, 'hardNavigate').mockImplementation(() => { });
7373
}
7474

7575
const serviceRelayText =
@@ -107,7 +107,7 @@ describe('SigninTokenCode page', () => {
107107
(_, element) =>
108108
element?.tagName === 'P' &&
109109
element?.textContent ===
110-
`Enter the code that was sent to ${MOCK_EMAIL} within 5 minutes.`
110+
`Enter the code that was sent to ${MOCK_EMAIL} within 5 minutes.`
111111
);
112112
screen.getByLabelText('Enter 6-digit code');
113113
screen.getByRole('button', { name: 'Confirm' });
@@ -237,7 +237,7 @@ describe('SigninTokenCode page', () => {
237237
beforeEach(() => {
238238
hardNavigateSpy = jest
239239
.spyOn(ReactUtils, 'hardNavigate')
240-
.mockImplementation(() => {});
240+
.mockImplementation(() => { });
241241
});
242242
afterEach(() => {
243243
hardNavigateSpy.mockRestore();
@@ -260,7 +260,7 @@ describe('SigninTokenCode page', () => {
260260

261261
await expectSuccessGleanEvents();
262262
expect(mockOnSessionVerified).toHaveBeenCalledTimes(1);
263-
expect(navigate).toHaveBeenCalledWith('/settings');
263+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
264264
});
265265
it('when verificationReason is a force password change', async () => {
266266
session = mockSession();
@@ -286,13 +286,13 @@ describe('SigninTokenCode page', () => {
286286
const integration = createMockSigninOAuthIntegration();
287287
const hardNavigate = jest
288288
.spyOn(ReactUtils, 'hardNavigate')
289-
.mockImplementation(() => {});
289+
.mockImplementation(() => { });
290290

291291
render({ finishOAuthFlowHandler, integration });
292292
submitCode();
293293
await expectSuccessGleanEvents();
294294
await waitFor(() => {
295-
expect(hardNavigate).toHaveBeenCalledWith('someUri');
295+
expect(hardNavigate).toHaveBeenCalledWith('someUri', undefined, undefined, true);
296296
});
297297
});
298298
});

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ describe('Sign in with TOTP code page', () => {
154154
expect(GleanMetrics.totpForm.view).toHaveBeenCalledTimes(1);
155155
expect(GleanMetrics.totpForm.submit).toHaveBeenCalledTimes(1);
156156
expect(GleanMetrics.totpForm.success).toHaveBeenCalledTimes(1);
157-
expect(navigate).toHaveBeenCalledWith('/settings');
157+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
158158
});
159159

160160
describe('fxaLogin webchannel message', () => {
@@ -164,7 +164,7 @@ describe('Sign in with TOTP code page', () => {
164164
fxaLoginSpy = jest.spyOn(firefox, 'fxaLogin');
165165
hardNavigateSpy = jest
166166
.spyOn(utils, 'hardNavigate')
167-
.mockImplementation(() => {});
167+
.mockImplementation(() => { });
168168
});
169169
it('is sent if Sync integration and navigates to pair', async () => {
170170
const integration = createMockSigninOAuthNativeSyncIntegration();
@@ -173,7 +173,10 @@ describe('Sign in with TOTP code page', () => {
173173
);
174174
expect(fxaLoginSpy).toHaveBeenCalled();
175175
expect(hardNavigateSpy).toHaveBeenCalledWith(
176-
'/pair?showSuccessMessage=true'
176+
'/pair?showSuccessMessage=true',
177+
undefined,
178+
undefined,
179+
true
177180
);
178181
});
179182
it('is not sent otherwise', async () => {
@@ -215,7 +218,7 @@ describe('Sign in with TOTP code page', () => {
215218
.mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE);
216219
const hardNavigate = jest
217220
.spyOn(ReactUtils, 'hardNavigate')
218-
.mockImplementationOnce(() => {});
221+
.mockImplementationOnce(() => { });
219222

220223
await waitFor(() =>
221224
renderAndSubmitTotpCode({}, finishOAuthFlowHandler, integration)
@@ -224,7 +227,7 @@ describe('Sign in with TOTP code page', () => {
224227
expect(GleanMetrics.totpForm.submit).toHaveBeenCalledTimes(1);
225228
expect(GleanMetrics.totpForm.success).toHaveBeenCalledTimes(1);
226229
await waitFor(() =>
227-
expect(hardNavigate).toHaveBeenCalledWith('someUri')
230+
expect(hardNavigate).toHaveBeenCalledWith('someUri', undefined, undefined, true)
228231
);
229232
});
230233

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ describe('SigninUnblock', () => {
245245
await waitFor(() => {
246246
expect(signinWithUnblockCode).toHaveBeenCalledTimes(1);
247247
});
248-
expect(navigate).toHaveBeenCalledWith('/settings');
248+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
249249
});
250250

251251
it('emits expected metrics events', async () => {
@@ -286,7 +286,7 @@ describe('SigninUnblock', () => {
286286
beforeEach(() => {
287287
hardNavigateSpy = jest
288288
.spyOn(utils, 'hardNavigate')
289-
.mockImplementation(() => {});
289+
.mockImplementation(() => { });
290290
});
291291

292292
afterEach(() => {
@@ -304,7 +304,7 @@ describe('SigninUnblock', () => {
304304
submitButton.click();
305305

306306
await waitFor(() => {
307-
expect(hardNavigateSpy).toHaveBeenCalledWith(redirectTo);
307+
expect(hardNavigateSpy).toHaveBeenCalledWith(redirectTo, undefined, undefined, false);
308308
});
309309
});
310310

@@ -321,7 +321,7 @@ describe('SigninUnblock', () => {
321321
submitButton.click();
322322

323323
await waitFor(() => {
324-
expect(navigate).toHaveBeenCalledWith('/settings');
324+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
325325
});
326326
});
327327

@@ -334,7 +334,7 @@ describe('SigninUnblock', () => {
334334
submitButton.click();
335335

336336
await waitFor(() => {
337-
expect(navigate).toHaveBeenCalledWith('/settings');
337+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
338338
});
339339
});
340340
});

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ describe('Signin component', () => {
310310
enterPasswordAndSubmit();
311311
await waitFor(() => {
312312
expect(navigate).toHaveBeenCalledWith('/signin_totp_code', {
313+
replace: false,
313314
state: {
314315
email: MOCK_EMAIL,
315316
uid: MOCK_UID,
@@ -334,6 +335,7 @@ describe('Signin component', () => {
334335
enterPasswordAndSubmit();
335336
await waitFor(() => {
336337
expect(navigate).toHaveBeenCalledWith('/confirm_signup_code', {
338+
replace: false,
337339
state: {
338340
email: MOCK_EMAIL,
339341
uid: MOCK_UID,
@@ -357,6 +359,7 @@ describe('Signin component', () => {
357359
enterPasswordAndSubmit();
358360
await waitFor(() => {
359361
expect(navigate).toHaveBeenCalledWith('/signin_token_code', {
362+
replace: false,
360363
state: {
361364
email: MOCK_EMAIL,
362365
uid: MOCK_UID,
@@ -385,6 +388,7 @@ describe('Signin component', () => {
385388
expect(navigate).toHaveBeenCalledWith(
386389
'/inline_recovery_key_setup?',
387390
{
391+
replace: true,
388392
state: {
389393
email: MOCK_EMAIL,
390394
uid: MOCK_UID,
@@ -407,7 +411,7 @@ describe('Signin component', () => {
407411

408412
enterPasswordAndSubmit();
409413
await waitFor(() => {
410-
expect(navigate).toHaveBeenCalledWith('/settings');
414+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: false });
411415
});
412416
});
413417

@@ -419,7 +423,7 @@ describe('Signin component', () => {
419423
fxaLoginSpy = jest.spyOn(firefox, 'fxaLogin');
420424
hardNavigateSpy = jest
421425
.spyOn(utils, 'hardNavigate')
422-
.mockImplementation(() => {});
426+
.mockImplementation(() => { });
423427
});
424428
it('is sent if Sync integration and navigates to CAD', async () => {
425429
const beginSigninHandler = jest.fn().mockReturnValueOnce(
@@ -446,7 +450,7 @@ describe('Signin component', () => {
446450
});
447451
});
448452
expect(hardNavigateSpy).toHaveBeenCalledWith(
449-
'/pair?showSuccessMessage=true'
453+
'/pair?showSuccessMessage=true', undefined, undefined, false
450454
);
451455
});
452456
it('is not sent if user has 2FA enabled', async () => {
@@ -479,7 +483,7 @@ describe('Signin component', () => {
479483
fxaLoginSpy = jest.spyOn(firefox, 'fxaLogin');
480484
hardNavigateSpy = jest
481485
.spyOn(utils, 'hardNavigate')
482-
.mockImplementation(() => {});
486+
.mockImplementation(() => { });
483487
finishOAuthFlowHandler = jest
484488
.fn()
485489
.mockReturnValueOnce(MOCK_OAUTH_FLOW_HANDLER_RESPONSE);
@@ -503,6 +507,7 @@ describe('Signin component', () => {
503507
enterPasswordAndSubmit();
504508
await waitFor(() => {
505509
expect(navigate).toHaveBeenCalledWith('/confirm_signup_code', {
510+
replace: false,
506511
state: {
507512
email: MOCK_EMAIL,
508513
uid: MOCK_UID,
@@ -531,6 +536,7 @@ describe('Signin component', () => {
531536
enterPasswordAndSubmit();
532537
await waitFor(() => {
533538
expect(navigate).toHaveBeenCalledWith('/confirm_signup_code', {
539+
replace: false,
534540
state: {
535541
email: MOCK_EMAIL,
536542
uid: MOCK_UID,
@@ -559,7 +565,10 @@ describe('Signin component', () => {
559565
await waitFor(() => {
560566
expect(fxaOAuthLoginSpy).not.toHaveBeenCalled();
561567
expect(hardNavigateSpy).toHaveBeenCalledWith(
562-
MOCK_OAUTH_FLOW_HANDLER_RESPONSE.redirect
568+
MOCK_OAUTH_FLOW_HANDLER_RESPONSE.redirect,
569+
undefined,
570+
undefined,
571+
true
563572
);
564573
});
565574
});
@@ -599,7 +608,7 @@ describe('Signin component', () => {
599608
expect(fxaLoginCallOrder).toBeLessThan(fxaOAuthLoginCallOrder);
600609

601610
expect(hardNavigateSpy).toHaveBeenCalledWith(
602-
'/pair?showSuccessMessage=true'
611+
'/pair?showSuccessMessage=true', undefined, undefined, true
603612
);
604613
});
605614
});
@@ -642,7 +651,7 @@ describe('Signin component', () => {
642651
// Ensure fxaLogin is called first
643652
expect(fxaLoginCallOrder).toBeLessThan(fxaOAuthLoginCallOrder);
644653

645-
expect(navigate).toHaveBeenCalledWith('/settings');
654+
expect(navigate).toHaveBeenCalledWith('/settings', { replace: true });
646655
});
647656
});
648657
});
@@ -852,7 +861,7 @@ describe('Signin component', () => {
852861
beforeEach(() => {
853862
hardNavigateSpy = jest
854863
.spyOn(utils, 'hardNavigate')
855-
.mockImplementation(() => {});
864+
.mockImplementation(() => { });
856865
});
857866

858867
afterEach(() => {
@@ -884,7 +893,7 @@ describe('Signin component', () => {
884893

885894
enterPasswordAndSubmit();
886895
await waitFor(() => {
887-
expect(hardNavigateSpy).toHaveBeenCalledWith('someUri');
896+
expect(hardNavigateSpy).toHaveBeenCalledWith('someUri', undefined, undefined, true);
888897
});
889898
});
890899
it('handles error due to TOTP required or insufficent ARC value', async () => {
@@ -910,6 +919,7 @@ describe('Signin component', () => {
910919
expect(GleanMetrics.login.error).toHaveBeenCalledTimes(1);
911920

912921
expect(navigate).toHaveBeenCalledWith('/inline_totp_setup', {
922+
replace: true,
913923
state: {
914924
email: MOCK_EMAIL,
915925
keyFetchToken: signinResponse.data.signIn.keyFetchToken,
@@ -1015,7 +1025,7 @@ describe('Signin component', () => {
10151025
beforeEach(() => {
10161026
hardNavigateSpy = jest
10171027
.spyOn(utils, 'hardNavigate')
1018-
.mockImplementation(() => {});
1028+
.mockImplementation(() => { });
10191029
});
10201030
afterEach(() => {
10211031
hardNavigateSpy.mockRestore();

‎packages/fxa-settings/src/pages/Signin/interfaces.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,16 @@ export type SigninUnblockIntegration = Pick<
3535

3636
export type SigninIntegration =
3737
| Pick<
38-
Integration,
39-
| 'type'
40-
| 'isSync'
41-
| 'getService'
42-
| 'getClientId'
43-
| 'wantsKeys'
44-
| 'data'
45-
| 'isDesktopSync'
46-
| 'isDesktopRelay'
47-
>
38+
Integration,
39+
| 'type'
40+
| 'isSync'
41+
| 'getService'
42+
| 'getClientId'
43+
| 'wantsKeys'
44+
| 'data'
45+
| 'isDesktopSync'
46+
| 'isDesktopRelay'
47+
>
4848
| SigninOAuthIntegration;
4949

5050
export type SigninOAuthIntegration = Pick<
@@ -203,6 +203,7 @@ export interface NavigationOptions {
203203
integration: SigninIntegration;
204204
finishOAuthFlowHandler: FinishOAuthFlowHandler;
205205
redirectTo?: string;
206+
replace?: boolean;
206207
queryParams: string;
207208
showInlineRecoveryKeySetup?: boolean;
208209
isSignInWithThirdPartyAuth?: boolean;

0 commit comments

Comments
 (0)