Skip to content

Commit df8fcd2

Browse files
dschomvpomerleau
authored andcommitted
task(settings) Wrap add recovery phone and replace recovery with MFA guard
1 parent d1259a3 commit df8fcd2

13 files changed

Lines changed: 155 additions & 42 deletions

File tree

packages/functional-tests/tests/resetPassword/resetPassword2FA.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,8 @@ test.describe('reset password with recovery phone', () => {
418418
await settings.totp.addRecoveryPhoneButton.click();
419419
await page.waitForURL(/recovery_phone\/setup/);
420420

421+
await settings.confirmMfaGuard(credentials.email);
422+
421423
await expect(recoveryPhone.addHeader()).toBeVisible();
422424

423425
await recoveryPhone.enterPhoneNumber(testNumber);
@@ -504,6 +506,8 @@ test.describe('reset password with recovery phone', () => {
504506
await settings.totp.addRecoveryPhoneButton.click();
505507
await page.waitForURL(/recovery_phone\/setup/);
506508

509+
await settings.confirmMfaGuard(credentials.email);
510+
507511
await expect(recoveryPhone.addHeader()).toBeVisible();
508512

509513
await recoveryPhone.enterPhoneNumber(testNumber);

packages/functional-tests/tests/settings/recoveryPhone.spec.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,12 @@ test.describe('severity-1 #smoke', () => {
3737
await signInAccount(target, page, settings, signin, credentials);
3838

3939
await settings.goto();
40-
const totpCredentials = await setup2faWithBackupCodeChoice(
41-
settings,
42-
totp
43-
);
40+
await setup2faWithBackupCodeChoice(settings, totp);
4441
await expect(settings.totp.status).toHaveText('Enabled');
4542
await settings.totp.addRecoveryPhoneButton.click();
4643

44+
await settings.confirmMfaGuard(credentials.email);
45+
4746
await expect(recoveryPhone.addHeader()).toBeVisible();
4847

4948
await recoveryPhone.enterPhoneNumber('1234567890');
@@ -65,13 +64,12 @@ test.describe('severity-1 #smoke', () => {
6564
await signInAccount(target, page, settings, signin, credentials);
6665

6766
await settings.goto();
68-
const totpCredentials = await setup2faWithBackupCodeChoice(
69-
settings,
70-
totp
71-
);
67+
await setup2faWithBackupCodeChoice(settings, totp);
7268
await expect(settings.totp.status).toHaveText('Enabled');
7369
await settings.totp.addRecoveryPhoneButton.click();
7470

71+
await settings.confirmMfaGuard(credentials.email);
72+
7573
await expect(recoveryPhone.addHeader()).toBeVisible();
7674

7775
await recoveryPhone.enterPhoneNumber(testNumber);
@@ -119,10 +117,7 @@ test.describe('severity-1 #smoke', () => {
119117
await signInAccount(target, page, settings, signin, credentials);
120118

121119
await settings.goto();
122-
const totpCredentials = await setup2faWithBackupCodeChoice(
123-
settings,
124-
totp
125-
);
120+
await setup2faWithBackupCodeChoice(settings, totp);
126121
await expect(settings.totp.status).toHaveText('Enabled');
127122
await addRecoveryPhone(
128123
settings,
@@ -744,6 +739,8 @@ async function addRecoveryPhone(
744739
await settings.totp.addRecoveryPhoneButton.click();
745740
await page.waitForURL(/recovery_phone\/setup/);
746741

742+
await settings.confirmMfaGuard(credentials.email);
743+
747744
await expect(recoveryPhone.addHeader()).toBeVisible();
748745

749746
await recoveryPhone.enterPhoneNumber(target.smsClient.getPhoneNumber());

packages/fxa-auth-client/lib/client.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2392,7 +2392,7 @@ export default class AuthClient {
23922392
}
23932393

23942394
/**
2395-
* Tries to register a recovery phone number
2395+
* Tries to register a recovery phone number. Important, this must be used for inline_recovery flow!
23962396
*
23972397
* @param sessionToken The user's current session token
23982398
* @param phoneNumber The phone number to register. Should be E.164 format
@@ -2411,6 +2411,26 @@ export default class AuthClient {
24112411
);
24122412
}
24132413

2414+
/**
2415+
* Tries to register a recovery phone number
2416+
*
2417+
* @param jwt The users MFA jwt
2418+
* @param phoneNumber The phone number to register. Should be E.164 format
2419+
* @param headers
2420+
*/
2421+
async recoveryPhoneCreateWithJwt(
2422+
jwt: string,
2423+
phoneNumber: string,
2424+
headers?: Headers
2425+
): Promise<{ nationalFormat?: string; success: boolean }> {
2426+
return this.jwtPost(
2427+
'/mfa/recovery_phone/create',
2428+
jwt,
2429+
{ phoneNumber },
2430+
headers
2431+
);
2432+
}
2433+
24142434
/**
24152435
* Checks to see if a recovery phone is available in the user's region.
24162436
* @param sessionToken The user's current session token

packages/fxa-auth-server/lib/routes/recovery-phone.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,32 @@ export const recoveryPhoneRoutes = (
10371037
return recoveryPhoneHandler.setupPhoneNumber(request);
10381038
},
10391039
},
1040+
{
1041+
method: 'POST',
1042+
path: '/mfa/recovery_phone/create',
1043+
options: {
1044+
pre: [{ method: featureEnabledCheck }],
1045+
auth: {
1046+
strategy: 'mfa',
1047+
scope: ['mfa:2fa'],
1048+
payload: false,
1049+
},
1050+
validate: {
1051+
payload: isA.object({
1052+
phoneNumber: isA.string().regex(E164_NUMBER).required(),
1053+
}),
1054+
},
1055+
},
1056+
handler: function (request: AuthRequest) {
1057+
return routes
1058+
.find(
1059+
(route) =>
1060+
route.path === '/v1/recovery_phone/create' &&
1061+
route.method === 'POST'
1062+
)
1063+
?.handler(request);
1064+
},
1065+
},
10401066
{
10411067
method: 'POST',
10421068
path: '/recovery_phone/available',

packages/fxa-settings/src/components/Settings/Page2faReplaceBackupCodes/index.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import VerifiedSessionGuard from '../VerifiedSessionGuard';
2121
import FlowSetup2faBackupCodeDownload from '../FlowSetup2faBackupCodeDownload';
2222
import FlowSetup2faBackupCodeConfirm from '../FlowSetup2faBackupCodeConfirm';
2323
import { MfaGuard } from '../MfaGuard';
24+
import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
2425

2526
export const PageMfaGuard2faReplaceBackupCodes = (
2627
props: RouteComponentProps
@@ -96,7 +97,7 @@ export const Page2faReplaceBackupCodes = (_: RouteComponentProps) => {
9697

9798
alertSuccessAndGoHome();
9899
} catch (e) {
99-
if (e && e.code === 401 && e.errno === 110) {
100+
if (isInvalidJwtError(e)) {
100101
// JWT invalid/expired
101102
errorHandler(e);
102103
return;

packages/fxa-settings/src/components/Settings/Page2faSetup/index.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ const Page2faSetup = (_: RouteComponentProps) => {
186186
};
187187

188188
const handleVerifyPhone = async (phoneNumberInput: string) => {
189+
// TODO: Switch to addRecoveryPhoneJwt once page is wrapped with MfaGuard
189190
const { nationalFormat } = await account.addRecoveryPhone(phoneNumberInput);
190191
setPhoneData({
191192
phoneNumber: phoneNumberInput,
@@ -194,6 +195,7 @@ const Page2faSetup = (_: RouteComponentProps) => {
194195
};
195196

196197
const handleResendSms = async () => {
198+
// TODO: Switch to addRecoveryPhoneWithJwt once page is wrapped with MfaGuard
197199
await account.addRecoveryPhone(phoneData.phoneNumber);
198200
};
199201

packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,20 @@ describe('PageRecoveryPhoneSetup', () => {
4848

4949
it('add recovery phone with successful submission renders step 2', async () => {
5050
const user = userEvent.setup();
51-
const addRecoveryPhone = jest
51+
const addRecoveryPhoneWithJwt = jest
5252
.fn()
5353
.mockResolvedValueOnce(mockSuccessResponse);
54-
renderWithRouter(<Subject account={{ addRecoveryPhone }} />);
54+
55+
renderWithRouter(<Subject account={{ addRecoveryPhoneWithJwt }} />);
5556

5657
await completeStepOne(user);
57-
await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(1));
5858
await waitFor(() =>
59-
expect(addRecoveryPhone).toHaveBeenCalledWith(MOCK_FULL_PHONE_NUMBER)
59+
expect(addRecoveryPhoneWithJwt).toHaveBeenCalledTimes(1)
60+
);
61+
await waitFor(() =>
62+
expect(addRecoveryPhoneWithJwt).toHaveBeenCalledWith(
63+
MOCK_FULL_PHONE_NUMBER
64+
)
6065
);
6166
expect(
6267
screen.queryByText(/Youll get a text message from Mozilla/i)
@@ -69,12 +74,12 @@ describe('PageRecoveryPhoneSetup', () => {
6974
const confirmRecoveryPhone = jest
7075
.fn()
7176
.mockResolvedValueOnce(mockSuccessResponse);
72-
const addRecoveryPhone = jest
77+
const addRecoveryPhoneWithJwt = jest
7378
.fn()
7479
.mockResolvedValueOnce(mockSuccessResponse)
7580
.mockResolvedValueOnce(mockSuccessResponse);
7681
renderWithRouter(
77-
<Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
82+
<Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
7883
);
7984

8085
await completeStepOne(user);
@@ -88,21 +93,29 @@ describe('PageRecoveryPhoneSetup', () => {
8893
})
8994
);
9095

91-
await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(2));
92-
expect(addRecoveryPhone).toHaveBeenNthCalledWith(1, MOCK_FULL_PHONE_NUMBER);
93-
expect(addRecoveryPhone).toHaveBeenNthCalledWith(2, MOCK_FULL_PHONE_NUMBER);
96+
await waitFor(() =>
97+
expect(addRecoveryPhoneWithJwt).toHaveBeenCalledTimes(2)
98+
);
99+
expect(addRecoveryPhoneWithJwt).toHaveBeenNthCalledWith(
100+
1,
101+
MOCK_FULL_PHONE_NUMBER
102+
);
103+
expect(addRecoveryPhoneWithJwt).toHaveBeenNthCalledWith(
104+
2,
105+
MOCK_FULL_PHONE_NUMBER
106+
);
94107
});
95108

96109
it('at step 2, allows code confirm', async () => {
97110
const user = userEvent.setup();
98111
const confirmRecoveryPhone = jest
99112
.fn()
100113
.mockResolvedValueOnce(mockSuccessResponse);
101-
const addRecoveryPhone = jest
114+
const addRecoveryPhoneWithJwt = jest
102115
.fn()
103116
.mockResolvedValueOnce(mockSuccessResponse);
104117
renderWithRouter(
105-
<Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
118+
<Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
106119
);
107120

108121
await completeStepOne(user);
@@ -127,11 +140,11 @@ describe('PageRecoveryPhoneSetup', () => {
127140
const confirmRecoveryPhone = jest
128141
.fn()
129142
.mockResolvedValueOnce(mockSuccessResponse);
130-
const addRecoveryPhone = jest
143+
const addRecoveryPhoneWithJwt = jest
131144
.fn()
132145
.mockResolvedValueOnce(mockSuccessResponse);
133146
renderWithRouter(
134-
<Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
147+
<Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
135148
);
136149

137150
await waitFor(() =>
@@ -152,11 +165,11 @@ describe('PageRecoveryPhoneSetup', () => {
152165
const confirmRecoveryPhone = jest
153166
.fn()
154167
.mockResolvedValueOnce(mockSuccessResponse);
155-
const addRecoveryPhone = jest
168+
const addRecoveryPhoneWithJwt = jest
156169
.fn()
157170
.mockResolvedValueOnce(mockSuccessResponse);
158171
renderWithRouter(
159-
<Subject account={{ confirmRecoveryPhone, addRecoveryPhone }} />
172+
<Subject account={{ confirmRecoveryPhone, addRecoveryPhoneWithJwt }} />
160173
);
161174

162175
await completeStepOne(user);

packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import React, { useState } from 'react';
6+
import { useErrorHandler } from 'react-error-boundary';
67
import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
78
import { SETTINGS_PATH } from '../../../constants';
89
import { useAccount, useFtlMsgResolver } from '../../../models';
@@ -11,14 +12,25 @@ import FlowSetupRecoveryPhoneConfirmCode from '../FlowSetupRecoveryPhoneConfirmC
1112
import FlowSetupRecoveryPhoneSubmitNumber from '../FlowSetupRecoveryPhoneSubmitNumber';
1213
import { RouteComponentProps, useLocation } from '@reach/router';
1314
import { RecoveryPhoneSetupReason } from '../../../lib/types';
15+
import { MfaGuard } from '../MfaGuard';
16+
import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
1417

1518
const numberOfSteps = 2;
1619

20+
export const MfaGuardPageRecoveryPhoneSetup = (props: RouteComponentProps) => {
21+
return (
22+
<MfaGuard requiredScope="2fa">
23+
<PageRecoveryPhoneSetup {...props} />
24+
</MfaGuard>
25+
);
26+
};
27+
1728
export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
1829
const ftlMsgResolver = useFtlMsgResolver();
1930
const navigateWithQuery = useNavigateWithQuery();
2031
const account = useAccount();
2132
const location = useLocation();
33+
const errorHandler = useErrorHandler();
2234
const reason: RecoveryPhoneSetupReason =
2335
(location as any)?.state?.reason ?? RecoveryPhoneSetupReason.setup;
2436

@@ -82,7 +94,17 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
8294
// one code can be valid at the same time if the user clicks “resend code” to
8395
// account for SMS transmission delay. (This will change in FXA-11039)
8496
// try/catch is in the component that calls this function
85-
await account.addRecoveryPhone(phoneData.phoneNumber);
97+
try {
98+
await account.addRecoveryPhoneWithJwt(phoneData.phoneNumber);
99+
} catch (e) {
100+
if (isInvalidJwtError(e)) {
101+
// JWT invalid/expired
102+
errorHandler(e);
103+
return;
104+
}
105+
106+
throw e;
107+
}
86108
};
87109

88110
const verifyRecoveryCode = async (code: string) => {
@@ -97,11 +119,22 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
97119

98120
const verifyPhoneNumber = async (phoneNumberInput: string) => {
99121
// try/catch is in the component that calls this function
100-
const { nationalFormat } = await account.addRecoveryPhone(phoneNumberInput);
101-
setPhoneData({
102-
phoneNumber: phoneNumberInput,
103-
nationalFormat,
104-
});
122+
try {
123+
const { nationalFormat } =
124+
await account.addRecoveryPhoneWithJwt(phoneNumberInput);
125+
setPhoneData({
126+
phoneNumber: phoneNumberInput,
127+
nationalFormat,
128+
});
129+
} catch (e) {
130+
if (isInvalidJwtError(e)) {
131+
// JWT invalid/expired
132+
errorHandler(e);
133+
return;
134+
}
135+
136+
throw e;
137+
}
105138
};
106139

107140
return (
@@ -118,7 +151,7 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
118151
verifyPhoneNumber,
119152
currentStep,
120153
numberOfSteps,
121-
reason
154+
reason,
122155
}}
123156
/>
124157
)}
@@ -139,7 +172,7 @@ export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => {
139172
verifyRecoveryCode,
140173
currentStep,
141174
numberOfSteps,
142-
reason
175+
reason,
143176
}}
144177
/>
145178
)}

packages/fxa-settings/src/components/Settings/PageSecondaryEmailAdd/index.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { getErrorFtlId } from '../../../lib/error-utils';
1717
import { MfaGuard } from '../MfaGuard';
1818
import { useErrorHandler } from 'react-error-boundary';
1919
import VerifiedSessionGuard from '../VerifiedSessionGuard';
20+
import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
2021

2122
export const PageSecondaryEmailAdd = (_: RouteComponentProps) => {
2223
usePageViewEvent('settings.emails');
@@ -45,7 +46,7 @@ export const PageSecondaryEmailAdd = (_: RouteComponentProps) => {
4546
await account.createSecondaryEmail(email);
4647
navigateWithQuery('emails/verify', { state: { email }, replace: true });
4748
} catch (e) {
48-
if (e && e.code === 401 && e.errno === 110) {
49+
if (isInvalidJwtError(e)) {
4950
// JWT invalid/expired
5051
errorHandler(e);
5152
return;

0 commit comments

Comments
 (0)