Skip to content

Commit 746329d

Browse files
committed
task(settings): Wrap disable 2FA with MFA guard
Because: - We want to require MFA when disabling 2FA This Commit: - Wraps the 2FA row's disable button with an MfaGuard component - Updates functional tests to supply MfaGuard's OTP code when required
1 parent ccc259d commit 746329d

8 files changed

Lines changed: 140 additions & 12 deletions

File tree

packages/functional-tests/pages/settings/index.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,38 @@ export class SettingsPage extends SettingsLayout {
101101
}, creds.uid);
102102
}
103103

104+
/**
105+
* Removes 2FA from the account by clicking the 'disable' button on the 2FA row.
106+
*/
104107
async disconnectTotp() {
105108
await this.totp.disableButton.click();
109+
110+
// Obtain the MFA JWT if necessary
111+
if (await this.isMfaGuardVisible()) {
112+
const email = await this.primaryEmail.status.textContent();
113+
if (!email) {
114+
throw new Error('Could not determine primary email!');
115+
}
116+
await this.confirmMfaGuard(email);
117+
}
118+
106119
await this.modalConfirmButton.click();
107120

108121
await expect(this.settingsHeading).toBeVisible();
109122
await expect(this.alertBar).toHaveText('Two-step authentication disabled');
110123
await expect(this.totp.status).toHaveText('Disabled');
111124
}
112125

126+
/**
127+
* Indicates that the MFA guard's modal dialog is currently displayed.
128+
* @returns true if the MFA modal is open
129+
*/
130+
async isMfaGuardVisible() {
131+
return await this.page
132+
.getByRole('heading', { name: 'Enter confirmation code' })
133+
.isVisible();
134+
}
135+
113136
/**
114137
* Confirms the MFA modal (MfaGuard) by retrieving the code from the inbox
115138
* and submitting it. Use when an action triggers the protected modal.

packages/functional-tests/tests/oauth/totp.spec.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,7 @@ test.describe('severity-1 #smoke', () => {
5757
await expect(settings.alertBar).toHaveText(
5858
'Two-step authentication has been enabled'
5959
);
60-
await settings.totp.disableButton.click();
61-
await settings.clickModalConfirm();
62-
63-
await expect(settings.modalConfirmButton).toBeHidden();
64-
await expect(settings.totp.status).toHaveText('Disabled');
65-
await expect(settings.alertBar).toHaveText(
66-
'Two-step authentication disabled'
67-
);
60+
await settings.disconnectTotp();
6861

6962
await relier.goto();
7063
await relier.clickEmailFirst();

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1871,10 +1871,30 @@ export default class AuthClient {
18711871
return this.jwtPost('/mfa/totp/replace/confirm', jwt, { code }, headers);
18721872
}
18731873

1874+
/**
1875+
* @deprecated Use deleteTotpTokenWithJwt instead
1876+
*
1877+
* Disables 2FA Protection on the account.
1878+
*
1879+
* @param sessionToken - required, must be a verified session token
1880+
* @param headers - Optional additional headers for the request
1881+
* @returns A promise that resolves when the 2FA has been removed
1882+
*/
18741883
async deleteTotpToken(sessionToken: hexstring, headers?: Headers) {
18751884
return this.sessionPost('/totp/destroy', sessionToken, {}, headers);
18761885
}
18771886

1887+
/**
1888+
* Disables 2FA Protection on the account.
1889+
*
1890+
* @param jwt - required, must be a verified session token
1891+
* @param headers - Optional additional headers for the request
1892+
* @returns A promise that resolves when the 2FA has been removed
1893+
*/
1894+
async deleteTotpTokenWithJwt(jwt: string, headers?: Headers) {
1895+
return this.jwtPost('/mfa/totp/destroy', jwt, {}, headers);
1896+
}
1897+
18781898
async checkTotpTokenExists(
18791899
sessionToken: hexstring,
18801900
headers?: Headers

packages/fxa-auth-server/docs/swagger/totp-api.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,17 @@ const TOTP_DESTROY_POST = {
3232
`,
3333
],
3434
};
35+
const MFA_TOTP_DESTROY_POST = {
36+
...TAGS_TOTP,
37+
description: '/mfa/totp/destroy',
38+
notes: [
39+
dedent`
40+
🔒 Authenticated with MFA JWT (scope: mfa:2fa)
41+
42+
Deletes the current TOTP token for the user. The underlying session needs to have been verified by TOTP to remove it. It does not bypass that requirement.
43+
`,
44+
],
45+
};
3546

3647
const TOTP_EXISTS_GET = {
3748
...TAGS_TOTP,
@@ -158,6 +169,7 @@ const API_DOCS = {
158169
SESSION_VERIFY_TOTP_POST,
159170
TOTP_CREATE_POST,
160171
TOTP_DESTROY_POST,
172+
MFA_TOTP_DESTROY_POST,
161173
TOTP_EXISTS_GET,
162174
TOTP_VERIFY_POST,
163175
TOTP_VERIFY_RECOVERY_CODE_POST,

packages/fxa-auth-server/lib/routes/totp.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ module.exports = (
262262
}
263263
}
264264

265-
return [
265+
const routes = [
266266
{
267267
method: 'POST',
268268
path: '/totp/create',
@@ -588,6 +588,27 @@ module.exports = (
588588
}
589589
},
590590
},
591+
{
592+
method: 'POST',
593+
path: '/mfa/totp/destroy',
594+
options: {
595+
...TOTP_DOCS.MFA_TOTP_DESTROY_POST,
596+
auth: {
597+
strategy: 'mfa',
598+
scope: ['mfa:2fa'],
599+
payload: false,
600+
},
601+
response: {},
602+
},
603+
handler: async function (request) {
604+
return routes
605+
.find(
606+
(route) =>
607+
route.path === '/v1/totp/destroy' && route.method === 'POST'
608+
)
609+
.handler(request);
610+
},
611+
},
591612
{
592613
method: 'POST',
593614
path: '/totp/destroy',
@@ -1163,4 +1184,6 @@ module.exports = (
11631184
handler: handleTotpReplaceConfirm,
11641185
},
11651186
];
1187+
1188+
return routes;
11661189
};

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
MOCK_NATIONAL_FORMAT_PHONE_NUMBER,
1313
} from '../../../pages/mocks';
1414
import GleanMetrics from '../../../lib/glean';
15+
import { JwtTokenCache } from '../../../lib/cache';
1516

1617
jest.mock('../../../models/AlertBarInfo');
1718

@@ -21,7 +22,32 @@ jest.mock('../../../lib/glean', () => ({
2122
},
2223
}));
2324

25+
jest.mock('../../../models', () => ({
26+
...jest.requireActual('../../../models'),
27+
useAuthClient: () => ({
28+
mfaRequestOtp: jest.fn(),
29+
}),
30+
}));
31+
32+
// Mocks to suppress MFA Guard
33+
jest.mock('../../../lib/cache', () => ({
34+
...jest.requireActual('../../../lib/cache'),
35+
JwtTokenCache: {
36+
hasToken: jest.fn(),
37+
getToken: jest.fn(),
38+
getSnapshot: jest.fn(),
39+
subscribe: jest.fn(),
40+
},
41+
sessionToken: () => 'session-123',
42+
}));
43+
2444
describe('UnitRowTwoStepAuth', () => {
45+
beforeEach(() => {
46+
(JwtTokenCache.hasToken as jest.Mock).mockReturnValue(true);
47+
(JwtTokenCache.getToken as jest.Mock).mockReturnValue('jwt-123');
48+
(JwtTokenCache.getSnapshot as jest.Mock).mockReturnValue(true);
49+
});
50+
2551
it('renders when two-step authentication is enabled', async () => {
2652
renderWithRouter(createSubject());
2753

@@ -129,6 +155,17 @@ describe('UnitRowTwoStepAuth', () => {
129155
);
130156
});
131157

158+
it('renders MFAGuard when the disable totp modal is rendered and jwt is missing', async () => {
159+
(JwtTokenCache.hasToken as jest.Mock).mockReturnValue(false);
160+
const user = userEvent.setup();
161+
162+
renderWithRouter(createSubject());
163+
164+
await user.click(screen.getByRole('button', { name: 'Disable' }));
165+
166+
expect(screen.getByText('Enter confirmation code')).toBeInTheDocument();
167+
});
168+
132169
it('renders with no backup codes and no recovery phone', () => {
133170
renderWithRouter(
134171
createSubject({

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

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

55
import React, { useCallback, useEffect } from 'react';
6+
import * as Sentry from '@sentry/browser';
7+
import { useErrorHandler } from 'react-error-boundary';
68
import LinkExternal from 'fxa-react/components/LinkExternal';
79
import { useBooleanState } from 'fxa-react/lib/hooks';
810
import Modal from '../Modal';
@@ -22,6 +24,8 @@ import { useNavigateWithQuery } from '../../../lib/hooks/useNavigateWithQuery';
2224
import { formatPhoneNumber } from '../../../lib/recovery-phone-utils';
2325
import { RecoveryPhoneSetupReason } from '../../../lib/types';
2426
import ModalVerifySession from '../ModalVerifySession';
27+
import { MfaGuard } from '../MfaGuard';
28+
import { isInvalidJwtError } from '../../../lib/mfa-guard-utils';
2529

2630
const route = `${SETTINGS_PATH}/two_step_authentication`;
2731
const replaceCodesRoute = `${route}/replace_codes`;
@@ -209,7 +213,14 @@ export const UnitRowTwoStepAuth = () => {
209213
/>
210214
)}
211215
{disable2FAModalRevealed && (
212-
<DisableTwoStepAuthModal {...{ hideDisable2FAModal }} />
216+
<MfaGuard
217+
requiredScope={'2fa'}
218+
onDismissCallback={async () => {
219+
hideDisable2FAModal();
220+
}}
221+
>
222+
<DisableTwoStepAuthModal {...{ hideDisable2FAModal }} />
223+
</MfaGuard>
213224
)}
214225
</UnitRow>
215226
</>
@@ -221,6 +232,7 @@ const DisableTwoStepAuthModal = ({
221232
}: {
222233
hideDisable2FAModal: () => void;
223234
}) => {
235+
const errorHandler = useErrorHandler();
224236
const alertBar = useAlertBar();
225237
const account = useAccount();
226238
const ftlMsgResolver = useFtlMsgResolver();
@@ -243,15 +255,23 @@ const DisableTwoStepAuthModal = ({
243255
() => GleanMetrics.accountPref.twoStepAuthDisableSuccessView()
244256
);
245257
} catch (e) {
258+
if (isInvalidJwtError(e)) {
259+
// JWT invalid/expired.
260+
errorHandler(e);
261+
return;
262+
}
263+
246264
hideDisable2FAModal();
247265
alertBar.error(
248266
ftlMsgResolver.getMsg(
249267
'tfa-row-cannot-disable-2',
250268
'Two-step authentication could not be disabled'
251269
)
252270
);
271+
272+
Sentry.captureException(e);
253273
}
254-
}, [account, hideDisable2FAModal, alertBar, ftlMsgResolver]);
274+
}, [account, hideDisable2FAModal, alertBar, ftlMsgResolver, errorHandler]);
255275

256276
return (
257277
<VerifiedSessionGuard

packages/fxa-settings/src/models/Account.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1126,7 +1126,7 @@ export class Account implements AccountData {
11261126

11271127
async disableTwoStepAuth() {
11281128
await this.withLoadingStatus(
1129-
this.authClient.deleteTotpToken(sessionToken()!)
1129+
this.authClient.deleteTotpTokenWithJwt(this.getCachedJwtByScope('2fa'))
11301130
);
11311131

11321132
const cache = this.apolloClient.cache;

0 commit comments

Comments
 (0)