From f3942e1a707eb31a8b382094d552381f31aaa659 Mon Sep 17 00:00:00 2001 From: Vijay Budhram Date: Thu, 14 May 2026 17:17:57 +0000 Subject: [PATCH] fix(settings): skip OAuth keys check for passwordless 2FA recovery flows When a user with 2FA enabled follows a passwordless (OTP email code) sign-in flow and clicks "Trouble entering code?" on the TOTP page, the recovery code and recovery phone containers were calling useOAuthKeysCheck without the isPasswordlessFlow skip flag. Passwordless users never derive keys (no password was entered), so the check incorrectly produced a TRY_AGAIN error that rendered the "Bad Request" page instead of the backup authentication code or recovery phone page. The fix mirrors what SigninTotpCodeContainer already does: pass isSignInWithThirdPartyAuth || isPasswordlessFlow as the skip flag. --- .../SigninRecoveryCode/container.test.tsx | 53 ++++++++++++++++++- .../Signin/SigninRecoveryCode/container.tsx | 2 +- .../SigninRecoveryPhone/container.test.tsx | 28 ++++++++++ .../Signin/SigninRecoveryPhone/container.tsx | 2 +- 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx index 15892403354..a0bbf6af172 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx @@ -10,7 +10,11 @@ import { LocationProvider } from '@reach/router'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import SigninRecoveryCodeContainer from './container'; import { createMockWebIntegration } from '../../../lib/integrations/mocks'; -import { Integration, useAuthClient, useSensitiveDataClient } from '../../../models'; +import { + Integration, + useAuthClient, + useSensitiveDataClient, +} from '../../../models'; import { mockSensitiveDataClient as createMockSensitiveDataClient } from '../../../models/mocks'; import { MOCK_STORED_ACCOUNT, @@ -24,6 +28,16 @@ import { mockSigninLocationState } from '../mocks'; import { waitFor } from '@testing-library/react'; import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors'; import { SensitiveData } from '../../../lib/sensitive-data-client'; +import { + useFinishOAuthFlowHandler, + useOAuthKeysCheck, +} from '../../../lib/oauth/hooks'; + +jest.mock('../../../lib/oauth/hooks', () => ({ + __esModule: true, + useFinishOAuthFlowHandler: jest.fn(), + useOAuthKeysCheck: jest.fn(), +})); let integration: Integration; function mockWebIntegration() { @@ -122,6 +136,13 @@ function applyDefaultMocks() { mockWebIntegration(); resetMockSensitiveDataClient(); resetMockAuthClient(); + (useFinishOAuthFlowHandler as jest.Mock).mockImplementation(() => ({ + finishOAuthFlowHandler: jest.fn(), + oAuthDataError: null, + })); + (useOAuthKeysCheck as jest.Mock).mockImplementation(() => ({ + oAuthKeysCheckError: null, + })); } function render() { @@ -170,6 +191,36 @@ describe('SigninRecoveryCode container', () => { }); }); + describe('useOAuthKeysCheck', () => { + it('passes isPasswordlessFlow to skip the keys check', () => { + mockReachRouter('signin_recovery_code', { + signinState: { ...mockSigninLocationState, isPasswordlessFlow: true }, + }); + render(); + expect(useOAuthKeysCheck).toHaveBeenCalledWith( + integration, + MOCK_KEY_FETCH_TOKEN, + MOCK_UNWRAP_BKEY, + true + ); + }); + + it('renders the recovery code component when keys check is skipped for passwordless flow', () => { + (useOAuthKeysCheck as jest.Mock).mockImplementationOnce( + (_integration: any, _kft: any, _ubk: any, skipKeysCheck: boolean) => ({ + oAuthKeysCheckError: skipKeysCheck + ? null + : { errno: 1, message: 'TRY_AGAIN' }, + }) + ); + mockReachRouter('signin_recovery_code', { + signinState: { ...mockSigninLocationState, isPasswordlessFlow: true }, + }); + render(); + expect(currentSigninRecoveryCodeProps).toBeDefined(); + }); + }); + describe('submitRecoveryCode', () => { it('successful', async () => { mockAuthClient.consumeRecoveryCode.mockResolvedValue({ remaining: 3 }); diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx index cb2ac8d64e3..d98fda852e6 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx @@ -58,7 +58,7 @@ export const SigninRecoveryCodeContainer = ({ integration, keyFetchToken, unwrapBKey, - signinState?.isSignInWithThirdPartyAuth + signinState?.isSignInWithThirdPartyAuth || signinState?.isPasswordlessFlow ); const submitRecoveryCode: SubmitRecoveryCode = useCallback( diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx index d87809d9b6f..46f963f78da 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.test.tsx @@ -248,4 +248,32 @@ describe('SigninRecoveryPhoneContainer', () => { ); }); }); + + describe('useOAuthKeysCheck', () => { + it('passes isPasswordlessFlow to skip the keys check', () => { + mockReachRouter('/signin_recovery_phone', { + signinState: { ...mockSigninLocationState, isPasswordlessFlow: true }, + lastFourPhoneDigits: '1234', + }); + renderSigninRecoveryPhoneContainer(); + const lastArg = (useOAuthKeysCheck as jest.Mock).mock.calls[0]?.[3]; + expect(lastArg).toBe(true); + }); + + it('renders the recovery phone component when keys check is skipped for passwordless flow', () => { + (useOAuthKeysCheck as jest.Mock).mockImplementationOnce( + (_integration: any, _kft: any, _ubk: any, skipKeysCheck: boolean) => ({ + oAuthKeysCheckError: skipKeysCheck + ? null + : { errno: 1, message: 'TRY_AGAIN' }, + }) + ); + mockReachRouter('/signin_recovery_phone', { + signinState: { ...mockSigninLocationState, isPasswordlessFlow: true }, + lastFourPhoneDigits: '1234', + }); + renderSigninRecoveryPhoneContainer(); + expect(currentPageProps).toBeDefined(); + }); + }); }); diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx index 7cefbe8ae98..65d813a4982 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx @@ -70,7 +70,7 @@ const SigninRecoveryPhoneContainer = ({ integration, keyFetchToken, unwrapBKey, - signinState?.isSignInWithThirdPartyAuth + signinState?.isSignInWithThirdPartyAuth || signinState?.isPasswordlessFlow ); const webRedirectCheck = useWebRedirect(integration.data.redirectTo);