diff --git a/packages/functional-tests/pages/inlineTotpSetup.tsx b/packages/functional-tests/pages/inlineTotpSetup.tsx index e8fda3bae33..27fde31a755 100644 --- a/packages/functional-tests/pages/inlineTotpSetup.tsx +++ b/packages/functional-tests/pages/inlineTotpSetup.tsx @@ -16,4 +16,34 @@ export class InlineTotpSetupPage extends BaseLayout { get continueButton() { return this.page.getByRole('button', { name: 'Continue' }); } + + /** + * The MFA email-OTP guard modal now protects inline TOTP enrolment + * (FXA-14311). Its heading matches the Settings MfaGuard modal. + */ + get mfaGuardHeading() { + return this.page.getByRole('heading', { name: 'Enter confirmation code' }); + } + + /** + * Satisfies the MFA email-OTP guard that gates inline TOTP enrolment: + * fetches the code from the inbox and submits it. On entry to + * `/inline_totp_setup` the guard has no cached JWT, so the modal is shown. + */ + async confirmMfaGuard(email: string) { + await this.mfaGuardHeading.waitFor(); + const code = + await this.target.emailClient.getVerifyAccountChangeCode(email); + await this.page + .getByRole('textbox', { name: 'Enter 6-digit code' }) + .fill(code); + await this.page.getByRole('button', { name: 'Confirm' }).click(); + } + + /** Confirms the MFA guard only if its modal is currently displayed. */ + async confirmMfaGuardIfVisible(email: string) { + if (await this.mfaGuardHeading.isVisible()) { + await this.confirmMfaGuard(email); + } + } } diff --git a/packages/functional-tests/pages/settings/totp.ts b/packages/functional-tests/pages/settings/totp.ts index ea3a3ce938c..a9d24c60ff9 100644 --- a/packages/functional-tests/pages/settings/totp.ts +++ b/packages/functional-tests/pages/settings/totp.ts @@ -248,6 +248,8 @@ export class TotpPage extends SettingsLayout { recoveryPhoneAvailable: boolean ): Promise { await this.page.waitForURL(/inline_totp_setup/); + // Inline TOTP enrolment is now gated by an MFA email-OTP (FXA-14311). + await inlineTotpSetup.confirmMfaGuard(credentials.email); await expect(inlineTotpSetup.introHeading).toBeVisible(); await inlineTotpSetup.continueButton.click(); const secret = await this.setUp2faAppWithManualCode(credentials); diff --git a/packages/functional-tests/tests/oauth/totp.spec.ts b/packages/functional-tests/tests/oauth/totp.spec.ts index d6a99614e04..2ed60a5b0ea 100644 --- a/packages/functional-tests/tests/oauth/totp.spec.ts +++ b/packages/functional-tests/tests/oauth/totp.spec.ts @@ -81,6 +81,8 @@ test.describe('severity-1 #smoke', () => { await page.waitForURL(/inline_totp_setup/); + // Inline TOTP enrolment is gated by an MFA email-OTP (FXA-14311). + await inlineTotpSetup.confirmMfaGuard(credentials.email); await expect(inlineTotpSetup.introHeading).toBeVisible(); await inlineTotpSetup.continueButton.click(); await expect(totp.setup2faAppHeading).toBeVisible(); @@ -118,6 +120,8 @@ test.describe('OAuth totp with recovery phone (local only)', () => { await page.waitForURL(/inline_totp_setup/); + // Inline TOTP enrolment is gated by an MFA email-OTP (FXA-14311). + await inlineTotpSetup.confirmMfaGuard(credentials.email); await expect(inlineTotpSetup.introHeading).toBeVisible(); await inlineTotpSetup.continueButton.click(); await expect(totp.setup2faAppHeading).toBeVisible(); diff --git a/packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts b/packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts index 71fdd9b2bff..1b9c0086b1a 100644 --- a/packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts +++ b/packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts @@ -555,7 +555,7 @@ test.describe('severity-1 #smoke', () => { test('forces /inline_totp_setup when password sign-in on an account with a passkey enrolled hits an AAL2 RP', async ({ target, - pages: { page, relier, settings, settingsPasskeyAdd, signin }, + pages: { page, relier, settings, settingsPasskeyAdd, signin, inlineTotpSetup }, testAccountTracker, }) => { // Passkey enrolment alone doesn't satisfy AAL2 for a password session, @@ -576,6 +576,9 @@ test.describe('severity-1 #smoke', () => { await signin.fillOutPasswordForm(credentials.password); await page.waitForURL(/inline_totp_setup/); + // Inline TOTP enrolment is gated by an MFA email-OTP (FXA-14311); the + // setup UI only renders once the guard is satisfied. + await inlineTotpSetup.confirmMfaGuard(credentials.email); await expect( page.getByRole('heading', { name: /Set up two-step authentication/i }) ).toBeVisible(); diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts index 496dcec936a..532e95d6ff4 100644 --- a/packages/fxa-auth-client/lib/client.ts +++ b/packages/fxa-auth-client/lib/client.ts @@ -2668,7 +2668,7 @@ export default class AuthClient { */ async completeTotpSetupWithJwt( jwt: string, - options: { metricsContext?: MetricsContext } = {}, + options: { service?: string; metricsContext?: MetricsContext } = {}, headers?: Headers ): Promise<{ success: boolean }> { return this.jwtPost( diff --git a/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.test.tsx b/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.test.tsx new file mode 100644 index 00000000000..780afad7622 --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.test.tsx @@ -0,0 +1,76 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { screen } from '@testing-library/react'; +import { mockAppContext, renderWithRouter } from '../../../models/mocks'; +import { MfaGuardCore } from './MfaGuardCore'; +import { JwtTokenCache } from '../../../lib/cache'; +import { AppContext } from '../../../models'; +import { MfaReason } from '../../../lib/types'; + +const mockSessionToken = 'session-core'; +const mockScope = 'test'; +const mockAuthClient = { + mfaRequestOtp: jest.fn().mockResolvedValue(undefined), + mfaOtpVerify: jest.fn(), +}; + +jest.mock('../../../models', () => ({ + ...jest.requireActual('../../../models'), + useAuthClient: () => mockAuthClient, +})); + +const noop = () => {}; + +function renderCore() { + renderWithRouter( + + +
secured content
+ + + ); +} + +describe('MfaGuardCore', () => { + beforeEach(() => { + JwtTokenCache.removeToken(mockSessionToken, mockScope); + jest.clearAllMocks(); + }); + + it('blocks children and requests an OTP when no JWT is cached', async () => { + renderCore(); + + // The security invariant: children (the sensitive action) must not render + // until an email OTP has produced a JWT. + expect(screen.queryByText('secured content')).not.toBeInTheDocument(); + expect( + await screen.findByText('Enter confirmation code') + ).toBeInTheDocument(); + expect(mockAuthClient.mfaRequestOtp).toHaveBeenCalledWith( + mockSessionToken, + mockScope + ); + }); + + it('renders children (no OTP request) when a JWT is already cached', () => { + JwtTokenCache.setToken(mockSessionToken, mockScope, 'jwt-present'); + + renderCore(); + + expect(screen.getByText('secured content')).toBeInTheDocument(); + expect( + screen.queryByText('Enter confirmation code') + ).not.toBeInTheDocument(); + expect(mockAuthClient.mfaRequestOtp).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.tsx b/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.tsx new file mode 100644 index 00000000000..8be6c166075 --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/MfaGuard/MfaGuardCore.tsx @@ -0,0 +1,224 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React, { + ReactNode, + useCallback, + useEffect, + useState, + useSyncExternalStore, +} from 'react'; + +import { useAuthClient, useConfig, useFtlMsgResolver } from '../../../models'; +import Modal from '../ModalMfaProtected'; +import { JwtTokenCache, MfaOtpRequestCache } from '../../../lib/cache'; +import { MfaReason, MfaScope } from '../../../lib/types'; +import { ERRNO } from '@fxa/accounts/errors'; +import * as Sentry from '@sentry/react'; +import { getLocalizedErrorMessage } from '../../../lib/error-utils'; +import GleanMetrics from '../../../lib/glean'; +import { MfaContext } from './context'; + +/** + * Host-agnostic core of the MFA email-OTP guard. It blocks its children behind + * an email-OTP → JWT exchange (scope `mfa:`), caching the JWT in + * `JwtTokenCache`, and provides the scope via `MfaContext` so children can use + * `useMfaErrorHandler`. + * + * Everything host-specific is injected via props so this can be used both inside + * Settings (see the `MfaGuard` wrapper in ./index) and in the Signin inline + * enrolment flow (FXA-14311). It deliberately does NOT read `useAccount`, + * `useAlertBar`, the global cached session token, or navigate directly. + */ +export const MfaGuardCore = ({ + children, + requiredScope, + reason, + email, + sessionToken, + onDismiss, + onSessionInvalid, + onFatalError, + debounceIntervalMs = 3000, +}: { + children: ReactNode; + requiredScope: MfaScope; + reason: MfaReason; + /** Address shown in the OTP modal (host supplies it; not read from a model). */ + email: string; + /** Session token used to request/verify the OTP and key the JWT cache. */ + sessionToken: string; + /** Called when the user dismisses the modal or after a fatal error — host decides where to go. */ + onDismiss: () => void; + /** Called when the session token is rejected as invalid/expired — host redirects to sign-in. */ + onSessionInvalid: () => void; + /** Called with an already-localized message for an unrecoverable OTP-request error. */ + onFatalError: (localizedMessage: string) => void; + debounceIntervalMs?: number; +}) => { + const config = useConfig(); + const authClient = useAuthClient(); + const ftlMsgResolver = useFtlMsgResolver(); + + const [localizedErrorBannerMessage, setLocalizedErrorBannerMessage] = + useState(undefined); + const [resendCodeLoading, setResendCodeLoading] = useState(false); + const [showResendSuccessBanner, setShowResendSuccessBanner] = useState(false); + + const resetStates = useCallback(() => { + setLocalizedErrorBannerMessage(undefined); + setShowResendSuccessBanner(false); + }, []); + + // Reactive state: if the store state changes, a re-render is triggered + const jwtState = useSyncExternalStore( + JwtTokenCache.subscribe, + JwtTokenCache.getSnapshot + ); + + const dismiss = useCallback(() => { + resetStates(); + onDismiss(); + }, [resetStates, onDismiss]); + + // jwtState should always return + if (!jwtState) { + throw new Error('Invalid state. Missing jwt cache.'); + } + + const debounce = useCallback( + (limitInMs: number) => { + const lastRequest = MfaOtpRequestCache.get(sessionToken, requiredScope); + return lastRequest != null && Date.now() - lastRequest < limitInMs; + }, + [sessionToken, requiredScope] + ); + + // Modal Setup + useEffect(() => { + (async () => { + // To avoid requesting multiple OTPs on mount + if (JwtTokenCache.hasToken(sessionToken, requiredScope)) { + return; + } + + // Avoid bombarding the user with emails just because they open + // the dialog + const limitInMs = config.mfa.otp.expiresInMinutes * 60 * 1000; + if (debounce(limitInMs)) { + return; + } + + try { + MfaOtpRequestCache.set(sessionToken, requiredScope); + await authClient.mfaRequestOtp(sessionToken, requiredScope); + } catch (err) { + MfaOtpRequestCache.remove(sessionToken, requiredScope); + + // If session token is invalid (destroyed/expired), redirect to signin + if (err?.errno === ERRNO.INVALID_TOKEN) { + onSessionInvalid(); + return; + } + + if (err.code === 429) { + setShowResendSuccessBanner(false); + setLocalizedErrorBannerMessage( + getLocalizedErrorMessage(ftlMsgResolver, err) + ); + return; + } + + Sentry.captureException(err); + onFatalError(getLocalizedErrorMessage(ftlMsgResolver, err)); + dismiss(); + } + })(); + }, [ + jwtState, + sessionToken, + requiredScope, + authClient, + ftlMsgResolver, + dismiss, + onSessionInvalid, + onFatalError, + config.mfa.otp.expiresInMinutes, + debounce, + ]); + + const onSubmitOtp = async (code: string) => { + try { + const result = await authClient.mfaOtpVerify( + sessionToken, + code, + requiredScope + ); + GleanMetrics.accountPref.mfaGuardSubmitSuccess({ + event: { reason }, + }); + JwtTokenCache.setToken(sessionToken, requiredScope, result.accessToken); + resetStates(); + } catch (err) { + setShowResendSuccessBanner(false); + setLocalizedErrorBannerMessage( + getLocalizedErrorMessage(ftlMsgResolver, err) + ); + } + }; + + const handleResendCode = async () => { + // Stop users from hammering the resend button... + if (debounce(debounceIntervalMs)) { + return; + } + + setResendCodeLoading(true); + try { + MfaOtpRequestCache.set(sessionToken, requiredScope); + await authClient.mfaRequestOtp(sessionToken, requiredScope); + setLocalizedErrorBannerMessage(undefined); + setShowResendSuccessBanner(true); + } catch (err) { + MfaOtpRequestCache.remove(sessionToken, requiredScope); + setShowResendSuccessBanner(false); + setLocalizedErrorBannerMessage( + getLocalizedErrorMessage(ftlMsgResolver, err) + ); + } finally { + setResendCodeLoading(false); + } + }; + + const expirationTime = config.mfa.otp.expiresInMinutes; + + const getModal = () => ( + setLocalizedErrorBannerMessage(undefined), + resendCodeLoading, + showResendSuccessBanner, + localizedErrorBannerMessage, + reason, + }} + > +

Re-verify Account!

+ + ); + + // If we don't have a JWT, we need to open the modal to prompt for it. + if (!JwtTokenCache.hasToken(sessionToken, requiredScope)) { + return getModal(); + } + + // Provide the scope via context so child components can use useMfaErrorHandler + return ( + {children} + ); +}; diff --git a/packages/fxa-settings/src/components/Settings/MfaGuard/context.ts b/packages/fxa-settings/src/components/Settings/MfaGuard/context.ts new file mode 100644 index 00000000000..be95439e4d3 --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/MfaGuard/context.ts @@ -0,0 +1,32 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { createContext, useCallback, useContext } from 'react'; +import { MfaScope } from '../../../lib/types'; +import { clearMfaAndJwtCacheOnInvalidJwt } from '../../../lib/mfa-guard-utils'; + +// Extracted from index.tsx so both the host-agnostic `MfaGuardCore` and the +// Settings `MfaGuard` wrapper can share the same context instance without a +// circular import. +export const MfaContext = createContext(undefined); + +/** + * Hook to handle MFA-related errors in child components. + * The returned function returns true if the error was handled, false otherwise. + */ +export const useMfaErrorHandler = () => { + const scope = useContext(MfaContext); + + if (!scope) { + throw new Error('useMfaErrorHandler must be used within an MfaGuard'); + } + + // Memoize to prevent unnecessary re-renders + return useCallback( + (error: unknown) => { + return clearMfaAndJwtCacheOnInvalidJwt(error, scope); + }, + [scope] + ); +}; diff --git a/packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx b/packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx index 9d26d5190dd..1fb9161837e 100644 --- a/packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx +++ b/packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx @@ -2,63 +2,25 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import React, { - ReactNode, - createContext, - useCallback, - useContext, - useEffect, - useState, - useSyncExternalStore, -} from 'react'; +import React, { ReactNode, useCallback } from 'react'; -import { - useAccount, - useAlertBar, - useAuthClient, - useConfig, - useFtlMsgResolver, -} from '../../../models'; -import Modal from '../ModalMfaProtected'; -import { - JwtTokenCache, - MfaOtpRequestCache, - sessionToken as getSessionToken, -} from '../../../lib/cache'; +import { useAccount, useAlertBar } from '../../../models'; +import { sessionToken as getSessionToken } from '../../../lib/cache'; import { MfaReason, MfaScope } from '../../../lib/types'; -import { ERRNO } from '@fxa/accounts/errors'; import { useNavigate } from 'react-router'; -import * as Sentry from '@sentry/react'; -import { getLocalizedErrorMessage } from '../../../lib/error-utils'; -import GleanMetrics from '../../../lib/glean'; -import { clearMfaAndJwtCacheOnInvalidJwt } from '../../../lib/mfa-guard-utils'; +import { MfaGuardCore } from './MfaGuardCore'; -export const MfaContext = createContext(undefined); +// Re-export the shared context API so existing imports +// (`import { useMfaErrorHandler, MfaContext } from '../MfaGuard'`) keep working. +export { MfaContext, useMfaErrorHandler } from './context'; +export { MfaGuardCore } from './MfaGuardCore'; /** - * Hook to handle MFA-related errors in child components. - * The returned function returns true if the error was handled, false otherwise. - */ -export const useMfaErrorHandler = () => { - const scope = useContext(MfaContext); - - if (!scope) { - throw new Error('useMfaErrorHandler must be used within an MfaGuard'); - } - - // Memoize to prevent unnecessary re-renders - return useCallback( - (error: unknown) => { - return clearMfaAndJwtCacheOnInvalidJwt(error, scope); - }, - [scope] - ); -}; - -/** - * This is a guard component designed to wrap around components that perform - * security-sensitive actions. It blocks access to the child components until - * a JWT is obtained through an OTP code exchange. + * Settings-flavored MFA guard. A thin wrapper over {@link MfaGuardCore} that + * supplies the Settings-specific bits — the account email, the alert bar for + * fatal errors, the globally-cached session token, and navigation back to + * `/settings` / `/signin`. Behavior and public API are unchanged; the reusable + * logic now lives in `MfaGuardCore` (see FXA-14311). */ export const MfaGuard = ({ children, @@ -73,184 +35,34 @@ export const MfaGuard = ({ debounceIntervalMs?: number; reason: MfaReason; }) => { - const config = useConfig(); - const [localizedErrorBannerMessage, setLocalizedErrorBannerMessage] = - useState(undefined); - - const [resendCodeLoading, setResendCodeLoading] = useState(false); - const [showResendSuccessBanner, setShowResendSuccessBanner] = useState(false); - - const resetStates = useCallback(() => { - setLocalizedErrorBannerMessage(undefined); - setShowResendSuccessBanner(false); - }, []); - - // Reactive state: if the store state changes, a re-render is triggered - const jwtState = useSyncExternalStore( - JwtTokenCache.subscribe, - JwtTokenCache.getSnapshot - ); const account = useAccount(); - const authClient = useAuthClient(); + const alertBar = useAlertBar(); const navigate = useNavigate(); const sessionToken = getSessionToken(); - const ftlMsgResolver = useFtlMsgResolver(); - - const alertBar = useAlertBar(); - - const onDismiss = useCallback(() => { - onDismissCallback().then(() => { - resetStates(); - navigate('/settings'); - }); - }, [navigate, resetStates, onDismissCallback]); - // If no session token exists, kick them to sign-in if (!sessionToken) { throw new Error('Invalid state. Missing sessionToken'); } - // jwtState should always return - if (!jwtState) { - throw new Error('Invalid state. Missing jwt cache.'); - } - - const debounce = useCallback( - (limitInMs: number) => { - const lastRequest = MfaOtpRequestCache.get(sessionToken, requiredScope); - return lastRequest != null && Date.now() - lastRequest < limitInMs; - }, - [sessionToken, requiredScope] - ); - - // Modal Setup - useEffect(() => { - (async () => { - // To avoid requesting multiple OTPs on mount - if (JwtTokenCache.hasToken(sessionToken, requiredScope)) { - return; - } - - // Avoid bombarding the user with emails just because they open - // the dialog - const limitInMs = config.mfa.otp.expiresInMinutes * 60 * 1000; - if (debounce(limitInMs)) { - return; - } - - try { - MfaOtpRequestCache.set(sessionToken, requiredScope); - await authClient.mfaRequestOtp(sessionToken, requiredScope); - } catch (err) { - MfaOtpRequestCache.remove(sessionToken, requiredScope); - - // If session token is invalid (destroyed/expired), redirect to signin - if (err?.errno === ERRNO.INVALID_TOKEN) { - navigate('/signin'); - return; - } - - if (err.code === 429) { - setShowResendSuccessBanner(false); - setLocalizedErrorBannerMessage( - getLocalizedErrorMessage(ftlMsgResolver, err) - ); - return; - } - - Sentry.captureException(err); - alertBar.error(getLocalizedErrorMessage(ftlMsgResolver, err)); - onDismiss(); - } - })(); - }, [ - jwtState, - sessionToken, - requiredScope, - authClient, - alertBar, - ftlMsgResolver, - onDismiss, - config.mfa.otp.expiresInMinutes, - debounce, - navigate, - ]); - - const onSubmitOtp = async (code: string) => { - try { - const result = await authClient.mfaOtpVerify( - sessionToken, - code, - requiredScope - ); - GleanMetrics.accountPref.mfaGuardSubmitSuccess({ - event: { reason }, - }); - JwtTokenCache.setToken(sessionToken, requiredScope, result.accessToken); - resetStates(); - } catch (err) { - setShowResendSuccessBanner(false); - setLocalizedErrorBannerMessage( - getLocalizedErrorMessage(ftlMsgResolver, err) - ); - } - }; - - const handleResendCode = async () => { - // Stop users from hammering the resend button... - if (debounce(debounceIntervalMs)) { - return; - } - - setResendCodeLoading(true); - try { - MfaOtpRequestCache.set(sessionToken, requiredScope); - await authClient.mfaRequestOtp(sessionToken, requiredScope); - setLocalizedErrorBannerMessage(undefined); - setShowResendSuccessBanner(true); - } catch (err) { - MfaOtpRequestCache.remove(sessionToken, requiredScope); - setShowResendSuccessBanner(false); - setLocalizedErrorBannerMessage( - getLocalizedErrorMessage(ftlMsgResolver, err) - ); - } finally { - setResendCodeLoading(false); - } - }; - - const email = account.email; - const expirationTime = config.mfa.otp.expiresInMinutes; - - const getModal = () => ( - setLocalizedErrorBannerMessage(undefined), - resendCodeLoading, - showResendSuccessBanner, - localizedErrorBannerMessage, - reason, - }} - > -

Re-verify Account!

- - ); - - // If we don't have a JWT, we need to open the modal to prompt for it. - if (!JwtTokenCache.hasToken(sessionToken, requiredScope)) { - return getModal(); - } + const onDismiss = useCallback(() => { + onDismissCallback().then(() => { + navigate('/settings'); + }); + }, [navigate, onDismissCallback]); - // Provide the scope via context so child components can use useMfaErrorHandler return ( - + navigate('/signin')} + onFatalError={(localizedMessage) => alertBar.error(localizedMessage)} + > {children} - + ); }; diff --git a/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.test.tsx b/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.test.tsx index 693a123f0b9..a41f4d49461 100644 --- a/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.test.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.test.tsx @@ -33,6 +33,16 @@ import { } from '../../lib/oauth/hooks'; import { SensitiveData } from '../../lib/sensitive-data-client'; import { mockWindowLocation } from 'fxa-react/lib/test-utils/mockWindowLocation'; +import { ReactNode } from 'react'; +import { JwtTokenCache } from '../../lib/cache'; + +// The MFA email-OTP guard is unit-tested separately (MfaGuardCore); here it is a +// pass-through so the recovery setup (its children) renders directly. A mfa:2fa +// JWT is seeded in setMocks so the JWT-guarded completion call resolves. +jest.mock('../../components/Settings/MfaGuard', () => ({ + __esModule: true, + MfaGuardCore: ({ children }: { children: ReactNode }) => children, +})); let mockLocationState = {}; const search = '?' + new URLSearchParams(MOCK_QUERY_PARAMS); @@ -131,7 +141,7 @@ jest.mock('./index', () => { }; }); -let mockCompleteTotpSetup = jest.fn().mockResolvedValue({ success: true }); +let mockCompleteTotpSetupWithJwt = jest.fn().mockResolvedValue({ success: true }); let mockCheckTotpTokenExists = jest.fn(); function setMocks() { @@ -144,10 +154,17 @@ function setMocks() { mockCheckTotpTokenExists.mockResolvedValue({ exists: false, verified: false }); (InlineRecoverySetupModule.default as jest.Mock).mockReset(); mockNavigateHook.mockReset(); - mockCompleteTotpSetup.mockClear(); + mockCompleteTotpSetupWithJwt.mockClear(); mockCheckTotpTokenExists.mockClear(); - (mockAuthClient as any).completeTotpSetup = mockCompleteTotpSetup; + (mockAuthClient as any).completeTotpSetupWithJwt = mockCompleteTotpSetupWithJwt; (mockAuthClient as any).checkTotpTokenExists = mockCheckTotpTokenExists; + // Seed the mfa:2fa JWT the completion reads (obtained by the guard via email + // OTP; pre-seeded here so the guard pass-through renders the flow). + JwtTokenCache.setToken( + MOCK_SIGNIN_RECOVERY_LOCATION_STATE.sessionToken, + '2fa', + 'test-jwt' + ); (useFinishOAuthFlowHandler as jest.Mock).mockImplementation(() => ({ finishOAuthFlowHandler: jest .fn() @@ -347,7 +364,7 @@ describe('InlineRecoverySetupContainer', () => { '010431', '12345678900' ); - expect(mockCompleteTotpSetup).toHaveBeenCalledTimes(1); + expect(mockCompleteTotpSetupWithJwt).toHaveBeenCalledTimes(1); }); }); @@ -364,7 +381,7 @@ describe('InlineRecoverySetupContainer', () => { await args.completeBackupCodeSetup('wibble'); }); expect(setRecoveryCodesFn).toHaveBeenCalledWith(['wibble', 'quux']); - expect(mockCompleteTotpSetup).toHaveBeenCalledTimes(1); + expect(mockCompleteTotpSetupWithJwt).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.tsx b/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.tsx index ac19203bfa1..b8779ae3855 100644 --- a/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.tsx +++ b/packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.tsx @@ -10,7 +10,7 @@ import { useOAuthKeysCheck, } from '../../lib/oauth/hooks'; import AppLayout from '../../components/AppLayout'; -import { MozServices } from '../../lib/types'; +import { MfaReason, MozServices } from '../../lib/types'; import { Integration, useAccount, @@ -19,6 +19,8 @@ import { useFtlMsgResolver, useSensitiveDataClient, } from '../../models'; +import { MfaGuardCore } from '../../components/Settings/MfaGuard'; +import { JwtTokenCache } from '../../lib/cache'; import InlineRecoverySetup from './index'; import { hardNavigate } from 'fxa-react/lib/utils'; import { SigninRecoveryLocationState } from './interfaces'; @@ -150,10 +152,15 @@ export const InlineRecoverySetupContainer = ({ const verifyTotpHandler = useCallback(async () => { try { - await authClient.completeTotpSetup( + // Uses the JWT-guarded /mfa/totp/* route so completing TOTP setup (which + // elevates the session to AAL2) requires the MFA email-OTP proof, not just + // the session token (FXA-14311). The JWT was obtained by the guard below + // (or carried over from the inline TOTP-setup step). + const jwt = JwtTokenCache.getToken( signinRecoveryLocationState!.sessionToken, - { service: serviceName } + '2fa' ); + await authClient.completeTotpSetupWithJwt(jwt, { service: serviceName }); return true; } catch (err) { // todo handle this error better @@ -292,29 +299,43 @@ export const InlineRecoverySetupContainer = ({ return ; } + // Require an MFA email-OTP (→ mfa:2fa JWT) before completing TOTP setup so a + // hijacked session cannot finalize a second factor (FXA-14311). The JWT is + // normally carried over from the inline TOTP-setup step; the guard only + // re-prompts if it is missing or expired. return ( - + navigateWithQuery('/signin')} + onSessionInvalid={() => navigateWithQuery('/signin')} + onFatalError={() => navigateWithQuery('/signin')} + > + + ); }; diff --git a/packages/fxa-settings/src/pages/InlineTotpSetup/container.test.tsx b/packages/fxa-settings/src/pages/InlineTotpSetup/container.test.tsx index 14c245fe856..1bfb4debe35 100644 --- a/packages/fxa-settings/src/pages/InlineTotpSetup/container.test.tsx +++ b/packages/fxa-settings/src/pages/InlineTotpSetup/container.test.tsx @@ -20,6 +20,8 @@ import { import { screen, waitFor } from '@testing-library/react'; import { AuthUiError, AuthUiErrors } from '../../lib/auth-errors/auth-errors'; import { MOCK_FLOW_ID } from '../Signin/mocks'; +import { ReactNode } from 'react'; +import { JwtTokenCache } from '../../lib/cache'; const mockLocationHook = jest.fn(); const mockNavigateHook = jest.fn(); @@ -32,9 +34,9 @@ jest.mock('react-router', () => { }); const mockSessionHook = jest.fn(); -const mockVerifyTotpSetupCode = jest.fn(); +const mockVerifyTotpSetupCodeWithJwt = jest.fn(); const mockSendVerificationCode = jest.fn(); -const mockCreateTotpToken = jest.fn(); +const mockCreateTotpTokenWithJwt = jest.fn(); const mockCheckTotpTokenExists = jest.fn(); jest.mock('../../models', () => { @@ -42,13 +44,21 @@ jest.mock('../../models', () => { ...jest.requireActual('../../models'), useSession: () => mockSessionHook(), useAuthClient: () => ({ - verifyTotpSetupCode: mockVerifyTotpSetupCode, - createTotpToken: mockCreateTotpToken, + verifyTotpSetupCodeWithJwt: mockVerifyTotpSetupCodeWithJwt, + createTotpTokenWithJwt: mockCreateTotpTokenWithJwt, checkTotpTokenExists: mockCheckTotpTokenExists, }), }; }); +// The MFA email-OTP guard is unit-tested separately (MfaGuardCore); here it is a +// pass-through so the enrolment (its children) renders directly. A mfa:2fa JWT +// is seeded in setMocks so the JWT-guarded enrolment calls resolve. +jest.mock('../../components/Settings/MfaGuard', () => ({ + __esModule: true, + MfaGuardCore: ({ children }: { children: ReactNode }) => children, +})); + jest.mock('../../lib/glean', () => ({ __esModule: true, default: { @@ -71,9 +81,9 @@ function setMocks() { search, state: MOCK_SIGNIN_LOCATION_STATE, }); - mockVerifyTotpSetupCode.mockReset(); + mockVerifyTotpSetupCodeWithJwt.mockReset(); mockSendVerificationCode.mockReset(); - mockCreateTotpToken.mockReset(); + mockCreateTotpTokenWithJwt.mockReset(); mockCheckTotpTokenExists.mockReset(); mockSessionHook.mockReturnValue({ isSessionVerified: async () => true, @@ -81,7 +91,14 @@ function setMocks() { }); // Default: TOTP doesn't exist, so we need to create one mockCheckTotpTokenExists.mockResolvedValue({ exists: false, verified: false }); - mockCreateTotpToken.mockResolvedValue(MOCK_TOTP_TOKEN); + mockCreateTotpTokenWithJwt.mockResolvedValue(MOCK_TOTP_TOKEN); + // Seed the mfa:2fa JWT the enrolment reads (the guard would have obtained it + // via email OTP; here it's pre-seeded so the guard pass-through renders). + JwtTokenCache.setToken( + MOCK_SIGNIN_LOCATION_STATE.sessionToken, + '2fa', + 'test-jwt' + ); jest.spyOn(InlineTotpSetupModule, 'default'); (InlineTotpSetupModule.default as jest.Mock).mockReset(); mockNavigateHook.mockReset(); @@ -216,7 +233,7 @@ describe('InlineTotpSetupContainer', () => { // Wait a bit to ensure the component has mounted await new Promise((resolve) => setTimeout(resolve, 100)); - expect(mockCreateTotpToken).not.toHaveBeenCalled(); + expect(mockCreateTotpTokenWithJwt).not.toHaveBeenCalled(); }); it('does not call createTotpToken when TOTP is already verified', async () => { @@ -230,7 +247,7 @@ describe('InlineTotpSetupContainer', () => { await waitFor(() => { expect(mockNavigateHook).toHaveBeenCalled(); }); - expect(mockCreateTotpToken).not.toHaveBeenCalled(); + expect(mockCreateTotpTokenWithJwt).not.toHaveBeenCalled(); }); }); @@ -258,7 +275,7 @@ describe('InlineTotpSetupContainer', () => { describe('callbacks', () => { describe('verifyCodeHandler', () => { it('throws an error when the server rejects the code', async () => { - mockVerifyTotpSetupCode.mockRejectedValue(new Error('bad')); + mockVerifyTotpSetupCodeWithJwt.mockRejectedValue(new Error('bad')); render(); await waitFor(() => { expect(InlineTotpSetupModule.default).toHaveBeenCalled(); @@ -279,7 +296,7 @@ describe('InlineTotpSetupContainer', () => { }); it('throws an error when checking the code errors', async () => { - mockVerifyTotpSetupCode.mockRejectedValue(new Error('err')); + mockVerifyTotpSetupCodeWithJwt.mockRejectedValue(new Error('err')); render(); await waitFor(() => { expect(InlineTotpSetupModule.default).toHaveBeenCalled(); @@ -300,7 +317,7 @@ describe('InlineTotpSetupContainer', () => { }); it('redirects to inline_recovery_setup when the code is valid', async () => { - mockVerifyTotpSetupCode.mockResolvedValue({ success: true }); + mockVerifyTotpSetupCodeWithJwt.mockResolvedValue({ success: true }); render(); await waitFor(() => { expect(InlineTotpSetupModule.default).toHaveBeenCalled(); diff --git a/packages/fxa-settings/src/pages/InlineTotpSetup/container.tsx b/packages/fxa-settings/src/pages/InlineTotpSetup/container.tsx index 2c3f83d187c..14dab4eea69 100644 --- a/packages/fxa-settings/src/pages/InlineTotpSetup/container.tsx +++ b/packages/fxa-settings/src/pages/InlineTotpSetup/container.tsx @@ -6,7 +6,7 @@ import { useLocation } from 'react-router'; import { useNavigateWithQuery } from '../../lib/hooks/useNavigateWithQuery'; import { useCallback, useEffect, useState, useRef } from 'react'; import InlineTotpSetup from '.'; -import { MozServices, TotpInfo } from '../../lib/types'; +import { MfaReason, MozServices, TotpInfo } from '../../lib/types'; import AppLayout from '../../components/AppLayout'; import { Integration, useSession, useAuthClient } from '../../models'; import { AuthUiErrors } from '../../lib/auth-errors/auth-errors'; @@ -17,6 +17,101 @@ import { QueryParams } from '../..'; import { queryParamsToMetricsContext } from '../../lib/metrics'; import GleanMetrics from '../../lib/glean'; import * as Sentry from '@sentry/browser'; +import { MfaGuardCore } from '../../components/Settings/MfaGuard'; +import { JwtTokenCache } from '../../lib/cache'; + +type MetricsContext = ReturnType; + +type NavTo = ( + uri: + | '/' + | '/signin_token_code' + | '/signin_totp_code' + | '/inline_recovery_setup', + state?: SigninLocationState | SigninRecoveryLocationState +) => void; + +/** + * Runs the actual TOTP enrolment. Rendered as a child of `MfaGuardCore`, so an + * MFA email-OTP has already been confirmed and a `mfa:2fa` JWT is cached — the + * enrolment calls use the JWT-guarded `/mfa/totp/*` routes (FXA-14311), so a + * bare session token can never enrol a second factor. + */ +const InlineTotpEnrolment = ({ + sessionToken, + signinState, + serviceName, + integration, + metricsContext, + navTo, +}: { + sessionToken: string; + signinState: SigninLocationState; + serviceName: MozServices; + integration: Integration; + metricsContext: MetricsContext; + navTo: NavTo; +}) => { + const authClient = useAuthClient(); + const [totp, setTotp] = useState(); + const isTotpCreating = useRef(false); + + // Trigger TOTP setup once the guard has provided the JWT. + useEffect(() => { + if (totp !== undefined || isTotpCreating.current) { + return; + } + (async () => { + isTotpCreating.current = true; + try { + const jwt = JwtTokenCache.getToken(sessionToken, '2fa'); + const totpResp = await authClient.createTotpTokenWithJwt(jwt, { + metricsContext, + }); + setTotp(totpResp); + } catch (error) { + Sentry.captureException(error); + navTo('/'); + } + })(); + }, [authClient, metricsContext, navTo, totp, sessionToken]); + + const verifyCodeHandler = useCallback( + async (code: string) => { + try { + const jwt = JwtTokenCache.getToken(sessionToken, '2fa'); + await authClient.verifyTotpSetupCodeWithJwt(jwt, code, { + metricsContext, + }); + + const state = { + ...Object.assign({}, signinState), + ...(totp ? { totp } : {}), + }; + GleanMetrics.accountPref.twoStepAuthQrCodeSuccess(); + navTo( + '/inline_recovery_setup', + Object.keys(state).length > 0 ? state : undefined + ); + } catch (error) { + // TODO: handle this error better + // auth-server may return more specific errors (including throttling) + throw AuthUiErrors.INVALID_TOTP_CODE; + } + }, + [authClient, navTo, totp, signinState, sessionToken, metricsContext] + ); + + if (totp === undefined) { + return ; + } + + return ( + + ); +}; export const InlineTotpSetupContainer = ({ isSignedIn, @@ -29,7 +124,6 @@ export const InlineTotpSetupContainer = ({ serviceName: MozServices; flowQueryParams: QueryParams; }) => { - const [totp, setTotp] = useState(); const [sessionVerified, setSessionVerified] = useState( undefined ); @@ -47,20 +141,12 @@ export const InlineTotpSetupContainer = ({ const metricsContext = queryParamsToMetricsContext( flowQueryParams as unknown as Record ); - const isTotpCreating = useRef(false); const isTotpStatusChecked = useRef(false); const signinState = getSigninState(location.state); - const navTo = useCallback( - ( - uri: - | '/' - | '/signin_token_code' - | '/signin_totp_code' - | '/inline_recovery_setup', - state?: SigninLocationState | SigninRecoveryLocationState - ) => { + const navTo: NavTo = useCallback( + (uri, state) => { navigateWithQuery(uri, { state }); }, [navigateWithQuery] @@ -100,40 +186,6 @@ export const InlineTotpSetupContainer = ({ })(); }, [session, sessionVerified, setSessionVerified]); - // Determine if a totp needs to be setup, and if so trigger setup. - useEffect(() => { - if ( - totp !== undefined || - totpStatus?.verified || - isTotpCreating.current || - totpStatusLoading || - !signinState?.sessionToken - ) { - return; - } - (async () => { - isTotpCreating.current = true; - try { - const totpResp = await authClient.createTotpToken( - signinState.sessionToken, - { metricsContext } - ); - setTotp(totpResp); - } catch (error) { - Sentry.captureException(error); - navTo('/'); - } - })(); - }, [ - authClient, - metricsContext, - navTo, - totpStatus, - totpStatusLoading, - totp, - signinState?.sessionToken, - ]); - // Once state has settled, determine if user should be directed to another page useEffect(() => { if (!isSignedIn || !signinState) { @@ -164,49 +216,42 @@ export const InlineTotpSetupContainer = ({ navigateWithQuery, ]); - const verifyCodeHandler = useCallback( - async (code: string) => { - try { - await authClient.verifyTotpSetupCode(signinState!.sessionToken, code, { - metricsContext, - }); - - const state = { - ...Object.assign({}, signinState), - ...(totp ? { totp } : {}), - }; - GleanMetrics.accountPref.twoStepAuthQrCodeSuccess(); - navTo( - '/inline_recovery_setup', - Object.keys(state).length > 0 ? state : undefined - ); - } catch (error) { - // TODO: handle this error better - // auth-server may return more specific errors (including throttling) - throw AuthUiErrors.INVALID_TOTP_CODE; - } - }, - [authClient, navTo, totp, signinState, metricsContext] - ); - if (!isSignedIn || !signinState) { return ; } + // Still resolving sanity checks, or a redirect effect above is about to fire. if ( - !isSignedIn || - !signinState || totpStatusLoading || - totp === undefined || - sessionVerified === undefined + sessionVerified === undefined || + totpStatus?.verified || + sessionVerified === false ) { return ; } + // Ready to enrol: require an MFA email-OTP (→ mfa:2fa JWT) before enrolment so + // a hijacked session cannot silently add a second factor (FXA-14311). The + // guard renders the enrolment only once the JWT is obtained. return ( - + navTo('/')} + onSessionInvalid={() => navigateWithQuery('/signin')} + onFatalError={() => navTo('/')} + > + + ); };