diff --git a/src/libs/Navigation/AppNavigator/PublicScreens.tsx b/src/libs/Navigation/AppNavigator/PublicScreens.tsx index e20ab13730bc..1df1671d7b2a 100644 --- a/src/libs/Navigation/AppNavigator/PublicScreens.tsx +++ b/src/libs/Navigation/AppNavigator/PublicScreens.tsx @@ -24,9 +24,10 @@ function PublicScreens() { const StyleUtils = useStyleUtils(); return ( - {/* The structure for the HOME route has to be the same in public and auth screens. That's why the name for SignInPage is SCREENS.HOME. */} + {/* SignInPage is registered under TAB_NAVIGATOR (the authenticated top-level navigator name) so the public and auth */} + {/* root structures match. TAB_NAVIGATOR has no path of its own, so the logged-out URL stays at the root "/". */} { } }; +// Preserved states belong to the authenticated session. They must be dropped on logout so the next +// session can't restore them — e.g. restoreTabNavigatorRoutes reattaching a previous user's tab +// subtree to the public sign-in route (which shares the TAB_NAVIGATOR name). +const clearPreservedNavigatorStates = () => { + for (const key of Object.keys(preservedNavigatorStates)) { + delete preservedNavigatorStates[key]; + } +}; + const getPreservedNavigatorState = >(key: string): T | undefined => preservedNavigatorStates[key] as T | undefined; const setPreservedNavigatorState = (key: string, state: NavigationState) => { @@ -52,4 +61,4 @@ function usePreserveNavigatorState(state: StackNavigationState, r export default usePreserveNavigatorState; -export {getPreservedNavigatorState, setPreservedNavigatorState, cleanPreservedNavigatorStates, clearPreservedSearchNavigatorStates}; +export {getPreservedNavigatorState, setPreservedNavigatorState, cleanPreservedNavigatorStates, clearPreservedSearchNavigatorStates, clearPreservedNavigatorStates}; diff --git a/src/libs/Navigation/NavigationRoot.tsx b/src/libs/Navigation/NavigationRoot.tsx index abaa2f8170e5..d711251053ad 100644 --- a/src/libs/Navigation/NavigationRoot.tsx +++ b/src/libs/Navigation/NavigationRoot.tsx @@ -25,9 +25,8 @@ import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; -import SCREENS from '@src/SCREENS'; import AppNavigator from './AppNavigator'; -import {cleanPreservedNavigatorStates} from './AppNavigator/createSplitNavigator/usePreserveNavigatorState'; +import {cleanPreservedNavigatorStates, clearPreservedNavigatorStates} from './AppNavigator/createSplitNavigator/usePreserveNavigatorState'; import getNavigationBaseTheme from './getNavigationBaseTheme'; import createDynamicRoute from './helpers/dynamicRoutesUtils/createDynamicRoute'; import getActiveTabName from './helpers/getActiveTabName'; @@ -225,6 +224,10 @@ function NavigationRoot({authenticated, lastVisitedPath, initialUrl, onReady}: N return; } + // Drop the previous session's preserved navigator states. Otherwise restoreTabNavigatorRoutes + // reattaches the prior TAB_NAVIGATOR subtree to the public sign-in route (which shares that name). + clearPreservedNavigatorStates(); + const stateToReset = getStateToResetAfterLogout(navigationRef.getRootState()); if (!stateToReset) { return; @@ -234,9 +237,9 @@ function NavigationRoot({authenticated, lastVisitedPath, initialUrl, onReady}: N navigationRef.reset(stateToReset); } catch (error) { // A synthesized reset state can be rejected by RN; fall back to a known-valid - // SCREENS.HOME (PublicScreens maps "/" → SignInPage) instead of leaving a blank screen. + // TAB_NAVIGATOR (PublicScreens maps "/" → SignInPage) instead of leaving a blank screen. Log.alert('[NavigationRoot] Post-logout navigation reset failed', {error: String(error)}); - navigationRef.reset({index: 0, routes: [{name: SCREENS.HOME}]}); + navigationRef.reset({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); } }, [authenticated, previousAuthenticated]); diff --git a/src/libs/Navigation/helpers/getAdaptedStateFromPath.ts b/src/libs/Navigation/helpers/getAdaptedStateFromPath.ts index 98968a76b6e8..7189c34a9cdf 100644 --- a/src/libs/Navigation/helpers/getAdaptedStateFromPath.ts +++ b/src/libs/Navigation/helpers/getAdaptedStateFromPath.ts @@ -33,12 +33,11 @@ type GetAdaptedStateFromPath = (...args: [...Parameters => ({routes, index: routes.length - 1}); /** - * Screens that are registered in PublicScreens (unauthenticated navigator) and should not - * have TabNavigator prepended, because when the user is unauthenticated TabNavigator does - * not exist in the navigator tree and the RESET action would fail. + * Standalone full-screen public pages registered in PublicScreens (unauthenticated navigator) that + * should NOT have TabNavigator prepended — they render on their own, with no tab navigator underneath. * - * Keep in sync with the screens registered in PublicScreens.tsx (excluding SCREENS.HOME, - * which doubles as the authenticated home tab, and navigator entries). + * Keep in sync with the screens registered in PublicScreens.tsx (excluding TAB_NAVIGATOR, which hosts + * the SignInPage at the root, and the other navigator entries). */ const PUBLIC_SCREENS = new Set([ SCREENS.VALIDATE_LOGIN, @@ -414,8 +413,8 @@ const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options, shouldR normalizedPath = '/'; } - // PublicScreens registers SCREENS.HOME ('Home') without a path mapping, so React Navigation derives `/Home` as the URL. - // The authenticated config maps SCREENS.HOME to lowercase 'home', and the case-sensitive mismatch falls to NOT_FOUND. + // `/Home` (capital H) has no route mapping — the config maps SCREENS.HOME to lowercase 'home' — so it would + // fall through to NOT_FOUND. Redirect legacy/cached `/Home` paths to the root instead. if (normalizedPath === `/${SCREENS.HOME}`) { normalizedPath = '/'; } diff --git a/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts b/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts index b7f0e7c2035a..ae14c951d416 100644 --- a/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts +++ b/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts @@ -14,18 +14,22 @@ function getStateToResetAfterLogout(rootState: NavigationState | undefined): Nav } // ValidateLogin's /v/ code is spent by logout; keeping it strands the user. Reset to - // SCREENS.HOME — this runs only post-logout (NavigationRoot gates on !authenticated), when - // PublicScreens is mounted and HOME (SignInPage) is its top-level route. + // TAB_NAVIGATOR — this runs only post-logout (NavigationRoot gates on !authenticated), when + // PublicScreens is mounted and TAB_NAVIGATOR (SignInPage) is its top-level route. if (lastRoute.name === SCREENS.VALIDATE_LOGIN) { - return {index: 0, routes: [{name: SCREENS.HOME}]}; + return {index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}; } // ReportsSplit is shared between logged-in & logged-out; its params can carry stale auth. const shouldClearParams = lastRoute.name === NAVIGATORS.REPORTS_SPLIT_NAVIGATOR; + + // TAB_NAVIGATOR hosts the public SignInPage at the root. Drop nested tab focus carried over from the + // authenticated session so the post-logout URL lands on "/" instead of a tab path like "/home". + const shouldClearNestedState = lastRoute.name === NAVIGATORS.TAB_NAVIGATOR; return { ...rootState, index: 0, - routes: [{...lastRoute, params: shouldClearParams ? undefined : lastRoute.params}], + routes: [{...lastRoute, params: shouldClearParams ? undefined : lastRoute.params, state: shouldClearNestedState ? undefined : lastRoute.state}], }; } diff --git a/src/libs/Navigation/types.ts b/src/libs/Navigation/types.ts index 9af445615220..c35185009634 100644 --- a/src/libs/Navigation/types.ts +++ b/src/libs/Navigation/types.ts @@ -2993,6 +2993,8 @@ type ShareNavigatorParamList = { }; type PublicScreensParamList = SharedScreensParamList & { + // Hosts the public SignInPage (see PublicScreens), mirroring the authenticated top-level navigator name. + [NAVIGATORS.TAB_NAVIGATOR]: NavigatorScreenParams; [SCREENS.UNLINK_LOGIN]: { accountID?: string; validateCode?: string; diff --git a/src/pages/ValidateLoginPage/index.web.tsx b/src/pages/ValidateLoginPage/index.web.tsx index da5cdeb7938e..ff0df633174e 100644 --- a/src/pages/ValidateLoginPage/index.web.tsx +++ b/src/pages/ValidateLoginPage/index.web.tsx @@ -11,9 +11,9 @@ 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 NAVIGATORS from '@src/NAVIGATORS'; 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'; @@ -110,18 +110,16 @@ function ValidateLoginPage({ useEffect(() => { 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. + // Surface the sign-in page so its ValidateCodeForm renders the authenticator-code stage. navigate/goBack + // no-op from the public /v/ route, so reset the stack to TAB_NAVIGATOR (which hosts the public SignInPage). + // The "2FA required" modal stays rendered as the fallback so a failed hand-off shows it, not a blank screen. 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}]}); + navigationRef.reset({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); } diff --git a/tests/navigation/PublicSignInPageRootUrlTest.tsx b/tests/navigation/PublicSignInPageRootUrlTest.tsx new file mode 100644 index 000000000000..5c6fe098df69 --- /dev/null +++ b/tests/navigation/PublicSignInPageRootUrlTest.tsx @@ -0,0 +1,48 @@ +import {NavigationContainer} from '@react-navigation/native'; +import {render, screen} from '@testing-library/react-native'; +import React from 'react'; +import {View} from 'react-native'; +import PublicScreens from '@libs/Navigation/AppNavigator/PublicScreens'; +import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath'; +import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; +import navigationRef from '@libs/Navigation/navigationRef'; +import NAVIGATORS from '@src/NAVIGATORS'; + +// SignInPage is the initial (and only rendered) public route, so it gets a findable testID. The `mock` +// prefix is required for the hoisted jest.mock factory to reference it; a function declaration hoists too, +// so there's no temporal-dead-zone risk when the factory runs. +function mockSignInPage() { + return ; +} + +// Stub the public pages so the test can mount the real PublicScreens navigator without their provider trees. +jest.mock('@pages/signin/SignInPage', () => mockSignInPage); +jest.mock('@pages/ConnectionCompletePage', () => () => null); +jest.mock('@pages/LogInWithShortLivedAuthTokenPage', () => () => null); +jest.mock('@pages/signin/SAMLSignInPage', () => () => null); +jest.mock('@pages/UnlinkLoginPage', () => () => null); +jest.mock('@pages/ValidateLoginPage', () => () => null); +jest.mock('@libs/Navigation/AppNavigator/Navigators/TestToolsModalNavigator', () => () => null); +jest.mock('@libs/Navigation/AppNavigator/useRootNavigatorScreenOptions', () => () => ({basicModalNavigator: {}})); +jest.mock('@hooks/useTheme', () => () => ({overlay: '#000000'})); +jest.mock('@hooks/useStyleUtils', () => () => ({getBackgroundColorWithOpacityStyle: () => ({})})); + +describe('Logged-out sign-in page URL', () => { + it('renders the SignInPage at the root "/" (registered under TAB_NAVIGATOR, which has no path)', () => { + render( + + + , + ); + + expect(screen.getByTestId('sign-in-page')).toBeOnTheScreen(); + + const rootState = navigationRef.getRootState(); + expect(rootState.routes.at(0)?.name).toBe(NAVIGATORS.TAB_NAVIGATOR); + expect(getPathFromState(rootState)).toBe('/'); + }); + + it('redirects a legacy "/Home" URL to the root "/"', () => { + expect(getPathFromState(getAdaptedStateFromPath('/Home', undefined))).toBe('/'); + }); +}); diff --git a/tests/ui/ValidateLoginPageTest.tsx b/tests/ui/ValidateLoginPageTest.tsx index 0f0cb6ce56fa..c1bbb32da8f2 100644 --- a/tests/ui/ValidateLoginPageTest.tsx +++ b/tests/ui/ValidateLoginPageTest.tsx @@ -8,6 +8,7 @@ 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 NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import SCREENS from '@src/SCREENS'; @@ -238,7 +239,7 @@ describe('ValidateLoginPage', () => { }); await waitForBatchedUpdatesWithAct(); - expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); it('Should route an exitTo 2FA magic link to the sign-in page AND keep the deferred exitTo navigation', async () => { @@ -266,7 +267,7 @@ describe('ValidateLoginPage', () => { }); await waitForBatchedUpdatesWithAct(); - expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(mockNavigationReset).toHaveBeenCalledWith({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); expect(handleExitToNavigation).toHaveBeenCalledWith('concierge'); }); diff --git a/tests/unit/getStateToResetAfterLogoutTest.ts b/tests/unit/getStateToResetAfterLogoutTest.ts index 48a083280c75..ce23cffa33e5 100644 --- a/tests/unit/getStateToResetAfterLogoutTest.ts +++ b/tests/unit/getStateToResetAfterLogoutTest.ts @@ -3,12 +3,14 @@ import getStateToResetAfterLogout from '@libs/Navigation/helpers/getStateToReset import NAVIGATORS from '@src/NAVIGATORS'; import SCREENS from '@src/SCREENS'; -/** Build a minimal root NavigationState from a list of `{name, params?}` routes. */ -const buildRootState = (routes: Array<{name: string; params?: Record}>): NavigationState => ({ +type RouteState = NavigationState['routes'][number]['state']; + +/** Build a minimal root NavigationState from a list of `{name, params?, state?}` routes. */ +const buildRootState = (routes: Array<{name: string; params?: Record; state?: RouteState}>): NavigationState => ({ key: 'stack-root', index: routes.length - 1, routeNames: routes.map((route) => route.name), - routes: routes.map((route, i) => ({key: `${route.name}-${i}`, name: route.name, params: route.params})), + routes: routes.map((route, i) => ({key: `${route.name}-${i}`, name: route.name, params: route.params, state: route.state})), type: 'stack', stale: false, }); @@ -22,25 +24,25 @@ describe('getStateToResetAfterLogout', () => { expect(getStateToResetAfterLogout(buildRootState([]))).toBeUndefined(); }); - it('resets to SCREENS.HOME when a consumed magic-link is the last route, even if a host is mounted (post-logout reset always lands on the public HOME/SignInPage route)', () => { + it('resets to TAB_NAVIGATOR when a consumed magic-link is the last route, even if a host is mounted (post-logout reset always lands on the public TAB_NAVIGATOR/SignInPage route)', () => { const result = getStateToResetAfterLogout(buildRootState([{name: NAVIGATORS.TAB_NAVIGATOR, params: {deep: 'link'}}, {name: SCREENS.VALIDATE_LOGIN}])); - expect(result).toEqual({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(result).toEqual({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); - it('resets to SCREENS.HOME for a consumed magic-link with a ReportsSplit host too', () => { + it('resets to TAB_NAVIGATOR for a consumed magic-link with a ReportsSplit host too', () => { const result = getStateToResetAfterLogout(buildRootState([{name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}, {name: SCREENS.VALIDATE_LOGIN}])); - expect(result).toEqual({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(result).toEqual({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); - it('resets to SCREENS.HOME when a consumed magic-link is the only route (fresh session, no host mounted)', () => { + it('resets to TAB_NAVIGATOR when a consumed magic-link is the only route (fresh session, no host mounted)', () => { const result = getStateToResetAfterLogout(buildRootState([{name: SCREENS.VALIDATE_LOGIN, params: {accountID: '1'}}])); - expect(result).toEqual({index: 0, routes: [{name: SCREENS.HOME}]}); + expect(result).toEqual({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); - it('PRESERVES params and does NOT redirect to HOME for a non-magic-link login/logout route (TransitionBetweenApps — a29d2e24c6a)', () => { + it('PRESERVES params and does NOT redirect to TAB_NAVIGATOR for a non-magic-link login/logout route (TransitionBetweenApps — a29d2e24c6a)', () => { const transitionParams = {shortLivedAuthToken: 'tok', exitTo: 'settings'}; const result = getStateToResetAfterLogout(buildRootState([{name: SCREENS.TRANSITION_BETWEEN_APPS, params: transitionParams}])); @@ -63,4 +65,12 @@ describe('getStateToResetAfterLogout', () => { expect(result?.routes[0].name).toBe(NAVIGATORS.TAB_NAVIGATOR); expect(result?.routes[0].params).toEqual(params); }); + + it('drops the nested tab state when logging out from a TAB_NAVIGATOR tab, so the SignInPage lands on the root "/" (not "/home")', () => { + // Simulate logging out while the Home tab is focused inside TAB_NAVIGATOR. + const result = getStateToResetAfterLogout(buildRootState([{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 0, routes: [{key: 'home', name: SCREENS.HOME}]}}])); + + expect(result?.routes[0].name).toBe(NAVIGATORS.TAB_NAVIGATOR); + expect(result?.routes[0].state).toBeUndefined(); + }); });