diff --git a/src/pages/ValidateLoginPage/index.web.tsx b/src/pages/ValidateLoginPage/index.web.tsx index a3f32494c567..da5cdeb7938e 100644 --- a/src/pages/ValidateLoginPage/index.web.tsx +++ b/src/pages/ValidateLoginPage/index.web.tsx @@ -7,12 +7,13 @@ import JustSignedInModal from '@components/ValidateCode/JustSignedInModal'; import ValidateCodeModal from '@components/ValidateCode/ValidateCodeModal'; import useOnyx from '@hooks/useOnyx'; import Log from '@libs/Log'; -import Navigation from '@libs/Navigation/Navigation'; +import Navigation, {navigationRef} from '@libs/Navigation/Navigation'; import {isValidValidateCode} from '@libs/ValidationUtils'; import {handleExitToNavigation, initAutoAuthState, signInWithValidateCode} from '@userActions/Session'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; +import SCREENS from '@src/SCREENS'; import type {Session as SessionType} from '@src/types/onyx'; import type ValidateLoginPageProps from './types'; @@ -40,7 +41,20 @@ function ValidateLoginPage({ const effectiveAutoAuthState = hasInitialized ? autoAuthState : undefined; const autoAuthStateWithDefault = effectiveAutoAuthState ?? CONST.AUTO_AUTH_STATE.NOT_STARTED; const is2FARequired = !!account?.requiresTwoFactorAuth; - const cachedAccountID = credentials?.accountID; + // A magic-link sign-in that needs 2FA completes on the sign-in page: it reuses the stored + // `credentials.validateCode`, and SignInPage renders the authenticator-code stage once + // `requiresTwoFactorAuth` + that code are present. Send the user there to enter their code instead + // of the informational "2FA required" modal, which is a dead end. Require the cached credentials to + // match THIS link — `signInWithValidateCode` only caches the code on a successful 2FA-required + // response, so a stale code left over from an earlier attempt can't redirect a later failed/expired + // link. `exitTo` deep links route here too; the effect keeps the deferred `exitTo` navigation + // pending so the user reaches their destination after 2FA completes. + const canCompleteTwoFactorOnSignIn = + is2FARequired && + !isSignedIn && + credentials?.validateCode === validateCode && + credentials?.accountID === Number(accountID) && + (autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN || autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.FAILED); const isUserClickedSignIn = !login && isSignedIn && (autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.SIGNING_IN || autoAuthStateWithDefault === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN); const shouldStartSignInWithValidateCode = !isUserClickedSignIn && !isSignedIn && (!!login || !!exitTo) && isValidValidateCode(validateCode); const isNavigatingToExitTo = isSignedIn && !!exitTo; @@ -94,18 +108,37 @@ function ValidateLoginPage({ ); useEffect(() => { - if (!!login || !cachedAccountID || !is2FARequired) { - if (exitTo) { - handleExitToNavigation(exitTo); - } - return; + let ignore = false; + if (canCompleteTwoFactorOnSignIn) { + // Show the sign-in page so its ValidateCodeForm renders the authenticator-code stage. + // ROUTES.HOME ('home') is nested under the authenticated TAB_NAVIGATOR, so navigate/goBack + // to it no-op from the public /v/ route; reset the stack to SCREENS.HOME instead — the same + // mechanism logout uses to surface the public SignInPage. The "2FA required" modal stays + // rendered as the fallback so a failed hand-off shows it, not a blank or an endless loader. + Navigation.isNavigationReady().then(() => { + // Bail if the effect re-ran (e.g. `isSignedIn` flipped true) before this resolved, so a + // stale callback can't reset the stack out from under the new state. + if (ignore) { + return; + } + navigationRef.reset({index: 0, routes: [{name: SCREENS.HOME}]}); + }); + } + + // Register the deferred `exitTo` destination navigation while still unauthenticated. + // `handleExitToNavigation` waits for the authToken, so for a 2FA account it fires only after the + // user enters their code on the sign-in page above (the token lands and resolves the shared + // sign-in promise) — landing them on their destination instead of Home. For a non-2FA account + // it's the normal post-sign-in handoff. The `!isSignedIn` guard registers it once and never + // re-fires after sign-in, so the effect re-running can't queue a duplicate navigation. + if (exitTo && !isSignedIn) { + handleExitToNavigation(exitTo); } - // The user clicked the option to sign in the current tab - Navigation.isNavigationReady().then(() => { - Navigation.goBack(); - }); - }, [login, cachedAccountID, is2FARequired, exitTo]); + return () => { + ignore = true; + }; + }, [canCompleteTwoFactorOnSignIn, exitTo, isSignedIn]); // waitForProtectedRoutes()/authToken can hang (lazy AuthScreens chunk fails, token // never lands). We can't recover the consumed code here, but surface a stuck sign-in to diff --git a/tests/ui/ValidateLoginPageTest.tsx b/tests/ui/ValidateLoginPageTest.tsx index ccb925ceb5e4..0f0cb6ce56fa 100644 --- a/tests/ui/ValidateLoginPageTest.tsx +++ b/tests/ui/ValidateLoginPageTest.tsx @@ -6,6 +6,7 @@ import Navigation from '@libs/Navigation/Navigation'; import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator'; import type {PublicScreensParamList} from '@libs/Navigation/types'; import ValidateLoginPage from '@pages/ValidateLoginPage/index.web'; +import {handleExitToNavigation} from '@userActions/Session'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -16,10 +17,22 @@ import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct' // protected routes are available) — not just the final args. const mockWaitForProtectedRoutes = {resolve: () => {}}; +// Controllable deferred for isNavigationReady() so tests can resolve it on demand — and, for the +// stale-callback guard, resolve it *after* the page unmounts to prove the reset is skipped. +const mockIsNavigationReady = {resolve: () => {}}; + +// Standalone fn so assertions don't access `navigationRef.reset` unbound (unbound-method lint rule). +const mockNavigationReset = jest.fn(); + jest.mock('@libs/Navigation/Navigation', () => ({ navigate: jest.fn(), goBack: jest.fn(), - isNavigationReady: jest.fn(() => Promise.resolve()), + isNavigationReady: jest.fn( + () => + new Promise((resolve) => { + mockIsNavigationReady.resolve = resolve; + }), + ), waitForProtectedRoutes: jest.fn( () => new Promise((resolve) => { @@ -29,6 +42,22 @@ jest.mock('@libs/Navigation/Navigation', () => ({ getActiveRoute: jest.fn(() => ''), getActiveRouteWithoutParams: jest.fn(() => ''), isActiveRoute: jest.fn(() => false), + // Dereference inside the closure (not at factory time) — the factory runs before the const below + // is initialized, so capturing `mockNavigationReset` directly would freeze `undefined`. + navigationRef: { + reset: (...args: unknown[]) => { + mockNavigationReset(...args); + }, + isReady: () => true, + }, +})); + +// Mock the session actions the page calls so `signInWithValidateCode` doesn't hit the API and the +// deferred `handleExitToNavigation` handoff can be asserted. +jest.mock('@userActions/Session', () => ({ + initAutoAuthState: jest.fn(), + signInWithValidateCode: jest.fn(), + handleExitToNavigation: jest.fn(), })); const RootStack = createPlatformStackNavigator(); @@ -57,6 +86,7 @@ describe('ValidateLoginPage', () => { beforeEach(async () => { jest.clearAllMocks(); mockWaitForProtectedRoutes.resolve = () => {}; + mockIsNavigationReady.resolve = () => {}; await act(async () => { await Onyx.clear(); }); @@ -164,10 +194,10 @@ describe('ValidateLoginPage', () => { expect(Navigation.navigate).not.toHaveBeenCalledWith(ROUTES.HOME, {forceReplace: true}); }); - it('Should show the 2FA-required prompt (not an infinite loader) for a separate-session sign-in needing 2FA', async () => { - // Separate session: no cached `login`, signed in via the link but 2FA is required so authToken - // never lands. Without the fix isCompletingDirectSignIn keeps the loader up forever; instead we - // must surface the "2FA required" prompt and never redirect Home. + it('Should show the 2FA-required prompt (not an infinite loader) when 2FA is needed and no validate code is cached', async () => { + // Genuinely-stuck fallback: 2FA is required but there's no cached `credentials.validateCode`, so + // the sign-in page can't render the authenticator stage and there's nowhere to send the user. + // Surface the informational "2FA required" prompt and never redirect Home or loop on a loader. await act(async () => { await Onyx.set(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true}); await Onyx.set(ONYXKEYS.SESSION, { @@ -179,6 +209,120 @@ describe('ValidateLoginPage', () => { await waitForBatchedUpdatesWithAct(); expect(screen.queryByTestId('validate-login-loading')).toBeNull(); + expect(mockNavigationReset).not.toHaveBeenCalled(); expect(Navigation.navigate).not.toHaveBeenCalledWith(ROUTES.HOME, {forceReplace: true}); }); + + it('Should redirect to the sign-in page to enter the 2FA code when a validate code is cached', async () => { + // Initiating browser: the magic-link attempt stored `credentials.validateCode` and 2FA is + // required. SignInPage reuses that code to render the authenticator-code stage, so we replace + // the consumed /v/ route with the sign-in page instead of the dead-end "2FA required" modal. + await act(async () => { + await Onyx.set(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true}); + await Onyx.set(ONYXKEYS.SESSION, { + autoAuthState: CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN, + }); + await Onyx.set(ONYXKEYS.CREDENTIALS, { + accountID: 1, + validateCode: '123456', + }); + }); + + renderPage({accountID: '1', validateCode: '123456'}); + await waitForBatchedUpdatesWithAct(); + + // Resolve the navigation-ready gate, then the effect resets the public stack to the SignInPage. + await act(async () => { + mockIsNavigationReady.resolve(); + await Promise.resolve(); + }); + await waitForBatchedUpdatesWithAct(); + + expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: SCREENS.HOME}]}); + }); + + it('Should route an exitTo 2FA magic link to the sign-in page AND keep the deferred exitTo navigation', async () => { + // exitTo deep link on a 2FA account: the user must still reach the authenticator form (not the + // dead-end "2FA required" modal), so we reset to the sign-in page. We also register + // handleExitToNavigation so that once 2FA completes and the authToken lands, the user is taken + // to their deep-link destination instead of being dropped on Home. + await act(async () => { + await Onyx.set(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true}); + await Onyx.set(ONYXKEYS.SESSION, { + autoAuthState: CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN, + }); + await Onyx.set(ONYXKEYS.CREDENTIALS, { + accountID: 1, + validateCode: '123456', + }); + }); + + renderPage({accountID: '1', validateCode: '123456', exitTo: 'concierge'}); + await waitForBatchedUpdatesWithAct(); + + await act(async () => { + mockIsNavigationReady.resolve(); + await Promise.resolve(); + }); + await waitForBatchedUpdatesWithAct(); + + expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(handleExitToNavigation).toHaveBeenCalledWith('concierge'); + }); + + it('Should NOT redirect to the sign-in page when the cached validate code is for a different link (stale code on a failed attempt)', async () => { + // A 2FA account can keep an old `credentials.validateCode` from an earlier attempt. A later /v/ + // link that fails leaves that stale code in Onyx (failureData doesn't touch it) while + // `requiresTwoFactorAuth` lingers — so the redirect must NOT fire for a code that isn't this + // link's, otherwise the user lands on the 2FA stage for the stale code instead of the failed UI. + await act(async () => { + await Onyx.set(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true}); + await Onyx.set(ONYXKEYS.SESSION, { + autoAuthState: CONST.AUTO_AUTH_STATE.FAILED, + }); + await Onyx.set(ONYXKEYS.CREDENTIALS, { + accountID: 1, + validateCode: '111111', // stale code cached by an earlier attempt + }); + }); + + // The link the user just opened carries a different code. + renderPage({accountID: '1', validateCode: '222222'}); + await waitForBatchedUpdatesWithAct(); + await waitForBatchedUpdatesWithAct(); + + expect(mockNavigationReset).not.toHaveBeenCalled(); + }); + + it('Should not reset to the sign-in page when the page unmounts before navigation is ready (stale-callback guard)', async () => { + // PERF-15 cleanup: if the effect tears down (deps change / unmount) before isNavigationReady() + // resolves, the pending callback must bail instead of resetting the stack out from under the + // screen that replaced it. + await act(async () => { + await Onyx.set(ONYXKEYS.ACCOUNT, {requiresTwoFactorAuth: true}); + await Onyx.set(ONYXKEYS.SESSION, { + autoAuthState: CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN, + }); + await Onyx.set(ONYXKEYS.CREDENTIALS, { + accountID: 1, + validateCode: '123456', + }); + }); + + const {unmount} = renderPage({accountID: '1', validateCode: '123456'}); + await waitForBatchedUpdatesWithAct(); + + // isNavigationReady() is still pending. Unmounting runs the effect cleanup (sets `ignore = true`). + await act(async () => { + unmount(); + }); + + // Resolving now fires the stale callback, which must skip the reset. + await act(async () => { + mockIsNavigationReady.resolve(); + await Promise.resolve(); + }); + + expect(mockNavigationReset).not.toHaveBeenCalled(); + }); });