From 01ad27d14a6adfe952cfe70d5defd03bf229e852 Mon Sep 17 00:00:00 2001 From: sumo_slonik Date: Tue, 23 Jun 2026 14:36:50 +0200 Subject: [PATCH 1/4] Fix logged-out sign-in page URL to stay at the root PublicScreens registered the SignInPage under the top-level SCREENS.HOME, which has no root-level path mapping (it's only mapped as "home" nested in TAB_NAVIGATOR), so React Navigation derived "/Home" from the screen name. Logging out from the Home tab also surfaced "/home" because the post-logout reset carried over the nested tab focus. Register the SignInPage under NAVIGATORS.TAB_NAVIGATOR instead, mirroring the authenticated top-level navigator (as it used to be registered under REPORTS_SPLIT_NAVIGATOR). TAB_NAVIGATOR has no path of its own, so the unauthenticated URL stays at "/". Update the post-logout and validate-login resets to target TAB_NAVIGATOR and drop any carried-over nested tab state so logout always lands on "/". --- .../Navigation/AppNavigator/PublicScreens.tsx | 6 ++- src/libs/Navigation/NavigationRoot.tsx | 5 +-- .../helpers/getAdaptedStateFromPath.ts | 14 +++---- .../helpers/getStateToResetAfterLogout.ts | 13 +++++-- src/libs/Navigation/types.ts | 3 ++ src/pages/ValidateLoginPage/index.web.tsx | 11 +++--- .../getPathFromStatePublicHomeTest.ts | 39 +++++++++++++++++++ tests/ui/ValidateLoginPageTest.tsx | 5 ++- tests/unit/getStateToResetAfterLogoutTest.ts | 30 +++++++++----- 9 files changed, 93 insertions(+), 33 deletions(-) create mode 100644 tests/navigation/getPathFromStatePublicHomeTest.ts diff --git a/src/libs/Navigation/AppNavigator/PublicScreens.tsx b/src/libs/Navigation/AppNavigator/PublicScreens.tsx index e20ab13730bc..324da63b257e 100644 --- a/src/libs/Navigation/AppNavigator/PublicScreens.tsx +++ b/src/libs/Navigation/AppNavigator/PublicScreens.tsx @@ -24,9 +24,11 @@ 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. */} + {/* The structure for the root route has to be the same in public and auth screens, so the SignInPage is registered under */} + {/* NAVIGATORS.TAB_NAVIGATOR (the authenticated top-level navigator). TAB_NAVIGATOR has no path of its own, so the */} + {/* unauthenticated URL stays at the root ("/") instead of resolving to a tab path like "/home". */} ({name})); /** - * 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, @@ -413,8 +412,9 @@ 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. + // Legacy/external `/Home` URLs (e.g. cached last-visited paths from when the unauthenticated SignInPage + // was registered as the top-level SCREENS.HOME) must resolve to the root. The authenticated config maps + // SCREENS.HOME to lowercase 'home', so the case-sensitive `/Home` would otherwise fall through to NOT_FOUND. if (normalizedPath === `/${SCREENS.HOME}`) { normalizedPath = '/'; } diff --git a/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts b/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts index b7f0e7c2035a..ef0d7c7d5ff6 100644 --- a/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts +++ b/src/libs/Navigation/helpers/getStateToResetAfterLogout.ts @@ -14,18 +14,23 @@ 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; + + // The public SignInPage is hosted by TAB_NAVIGATOR, which has no path of its own. Drop any nested + // tab focus carried over from the authenticated session (e.g. the Home tab) so the post-logout URL + // lands on the root "/" 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 70c9c2f4bea4..914b94c1833c 100644 --- a/src/libs/Navigation/types.ts +++ b/src/libs/Navigation/types.ts @@ -3022,6 +3022,9 @@ type ShareNavigatorParamList = { }; type PublicScreensParamList = SharedScreensParamList & { + // The SignInPage is registered under TAB_NAVIGATOR so the unauthenticated navigator mirrors the authenticated + // top-level structure (TAB_NAVIGATOR) and the root URL stays "/". It renders SignInPage, not the tab navigator. + [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..5b4001ffcc2f 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'; @@ -112,16 +112,17 @@ function ValidateLoginPage({ 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. + // to it no-op from the public /v/ route; reset the stack to TAB_NAVIGATOR instead (the public + // SignInPage is registered under that name) — 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}]}); + navigationRef.reset({index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}); }); } diff --git a/tests/navigation/getPathFromStatePublicHomeTest.ts b/tests/navigation/getPathFromStatePublicHomeTest.ts new file mode 100644 index 000000000000..e4c30f470fcb --- /dev/null +++ b/tests/navigation/getPathFromStatePublicHomeTest.ts @@ -0,0 +1,39 @@ +import type {NavigationState, PartialState} from '@react-navigation/routers'; +import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; +import NAVIGATORS from '@src/NAVIGATORS'; +import SCREENS from '@src/SCREENS'; + +/** + * Regression test for the unauthenticated sign-in page URL. + * + * PublicScreens registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated + * top-level navigator) instead of SCREENS.HOME. TAB_NAVIGATOR has no path of its own, so the + * logged-out URL resolves to the root "/" instead of "/Home". The authenticated Home tab is the HOME + * route *nested inside* TAB_NAVIGATOR and still resolves to "/home". + * + * Before the fix, SignInPage was registered as the top-level SCREENS.HOME with no root-level path + * mapping, so React Navigation derived "/Home" from the screen name and the logged-out address bar + * showed "/Home". + */ +describe('getPathFromState - sign-in page (public TAB_NAVIGATOR)', () => { + it('returns "/" for the unauthenticated TAB_NAVIGATOR root (SignInPage, no nested tab state)', () => { + const state: PartialState = {index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}; + expect(getPathFromState(state)).toBe('/'); + }); + + it('returns "/" when "/" synthesizes TAB_NAVIGATOR with ReportsSplit focused (empty path)', () => { + const state: PartialState = { + index: 0, + routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 1, routes: [{name: SCREENS.HOME}, {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]}}], + }; + expect(getPathFromState(state)).toBe('/'); + }); + + it('keeps "/home" for the authenticated Home tab nested in TAB_NAVIGATOR', () => { + const state: PartialState = { + index: 0, + routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 0, routes: [{name: SCREENS.HOME}]}}], + }; + expect(getPathFromState(state)).toBe('/home'); + }); +}); 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(); + }); }); From 3a9e15d7fbb13ea8abad19bae10da23a5c03f655 Mon Sep 17 00:00:00 2001 From: sumo_slonik Date: Wed, 24 Jun 2026 10:46:18 +0200 Subject: [PATCH 2/4] Strengthen sign-in root URL tests with a PublicScreens render guard - Add PublicSignInPageRootUrlTest: mounts the real PublicScreens navigator and asserts the root route renders SignInPage under TAB_NAVIGATOR and resolves to "/" (verified to fail if reverted to SCREENS.HOME). - Rename getPathFromStatePublicHomeTest -> publicSignInPageUrlResolutionTest; drive the real linking config round-trip (getAdaptedStateFromPath -> getPathFromState), cover the legacy /Home -> / redirect, and fix the mislabeled authenticated-root case. --- .../PublicSignInPageRootUrlTest.tsx | 67 +++++++++++++++++++ .../getPathFromStatePublicHomeTest.ts | 39 ----------- .../publicSignInPageUrlResolutionTest.ts | 63 +++++++++++++++++ 3 files changed, 130 insertions(+), 39 deletions(-) create mode 100644 tests/navigation/PublicSignInPageRootUrlTest.tsx delete mode 100644 tests/navigation/getPathFromStatePublicHomeTest.ts create mode 100644 tests/navigation/publicSignInPageUrlResolutionTest.ts diff --git a/tests/navigation/PublicSignInPageRootUrlTest.tsx b/tests/navigation/PublicSignInPageRootUrlTest.tsx new file mode 100644 index 000000000000..002bac4513e2 --- /dev/null +++ b/tests/navigation/PublicSignInPageRootUrlTest.tsx @@ -0,0 +1,67 @@ +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 getPathFromState from '@libs/Navigation/helpers/getPathFromState'; +import navigationRef from '@libs/Navigation/navigationRef'; +import NAVIGATORS from '@src/NAVIGATORS'; +import SCREENS from '@src/SCREENS'; + +/** + * Stub every public page so the test mounts the real `PublicScreens` navigator (the thing under test) + * without dragging in each page's provider tree. Only the route-name → component wiring matters here. + */ +const makeStub = (testID: string) => () => ; + +jest.mock('@pages/signin/SignInPage', () => makeStub('sign-in-page')); +jest.mock('@pages/ConnectionCompletePage', () => makeStub('connection-complete-page')); +jest.mock('@pages/LogInWithShortLivedAuthTokenPage', () => makeStub('login-with-short-lived-token-page')); +jest.mock('@pages/signin/SAMLSignInPage', () => makeStub('saml-sign-in-page')); +jest.mock('@pages/UnlinkLoginPage', () => makeStub('unlink-login-page')); +jest.mock('@pages/ValidateLoginPage', () => makeStub('validate-login-page')); +jest.mock('@libs/Navigation/AppNavigator/Navigators/TestToolsModalNavigator', () => makeStub('test-tools-modal-navigator')); + +// The navigator-level hooks reach for theme/style context; stub them so we don't need the provider tree. +jest.mock('@libs/Navigation/AppNavigator/useRootNavigatorScreenOptions', () => () => ({basicModalNavigator: {}})); +jest.mock('@hooks/useTheme', () => () => ({overlay: '#000000'})); +jest.mock('@hooks/useStyleUtils', () => () => ({getBackgroundColorWithOpacityStyle: () => ({})})); + +/** + * Regression guard for the logged-out sign-in page URL. + * + * `PublicScreens` registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated + * top-level navigator) instead of SCREENS.HOME. Because TAB_NAVIGATOR has no path of its own, the + * logged-out URL resolves to the root "/" instead of "/Home". + * + * Before the fix the SignInPage was registered as the top-level SCREENS.HOME, which has no root-level + * path mapping, so React Navigation derived "/Home" from the screen name. + */ +describe('PublicScreens - sign-in page root URL', () => { + it('renders the SignInPage for the TAB_NAVIGATOR root route and keeps the URL at "/"', () => { + render( + + + , + ); + + // The public root route is named TAB_NAVIGATOR and renders the SignInPage component. + expect(screen.getByTestId('sign-in-page')).toBeOnTheScreen(); + + const rootState = navigationRef.getRootState(); + expect(rootState.routes.at(0)?.name).toBe(NAVIGATORS.TAB_NAVIGATOR); + + // TAB_NAVIGATOR has no path of its own, so the logged-out address bar stays at the root. + expect(getPathFromState(rootState)).toBe('/'); + }); + + it('does NOT register the SignInPage under SCREENS.HOME (the source of the old "/Home" URL)', () => { + render( + + + , + ); + + expect(navigationRef.getRootState().routeNames).not.toContain(SCREENS.HOME); + }); +}); diff --git a/tests/navigation/getPathFromStatePublicHomeTest.ts b/tests/navigation/getPathFromStatePublicHomeTest.ts deleted file mode 100644 index e4c30f470fcb..000000000000 --- a/tests/navigation/getPathFromStatePublicHomeTest.ts +++ /dev/null @@ -1,39 +0,0 @@ -import type {NavigationState, PartialState} from '@react-navigation/routers'; -import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; -import NAVIGATORS from '@src/NAVIGATORS'; -import SCREENS from '@src/SCREENS'; - -/** - * Regression test for the unauthenticated sign-in page URL. - * - * PublicScreens registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated - * top-level navigator) instead of SCREENS.HOME. TAB_NAVIGATOR has no path of its own, so the - * logged-out URL resolves to the root "/" instead of "/Home". The authenticated Home tab is the HOME - * route *nested inside* TAB_NAVIGATOR and still resolves to "/home". - * - * Before the fix, SignInPage was registered as the top-level SCREENS.HOME with no root-level path - * mapping, so React Navigation derived "/Home" from the screen name and the logged-out address bar - * showed "/Home". - */ -describe('getPathFromState - sign-in page (public TAB_NAVIGATOR)', () => { - it('returns "/" for the unauthenticated TAB_NAVIGATOR root (SignInPage, no nested tab state)', () => { - const state: PartialState = {index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}; - expect(getPathFromState(state)).toBe('/'); - }); - - it('returns "/" when "/" synthesizes TAB_NAVIGATOR with ReportsSplit focused (empty path)', () => { - const state: PartialState = { - index: 0, - routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 1, routes: [{name: SCREENS.HOME}, {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]}}], - }; - expect(getPathFromState(state)).toBe('/'); - }); - - it('keeps "/home" for the authenticated Home tab nested in TAB_NAVIGATOR', () => { - const state: PartialState = { - index: 0, - routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 0, routes: [{name: SCREENS.HOME}]}}], - }; - expect(getPathFromState(state)).toBe('/home'); - }); -}); diff --git a/tests/navigation/publicSignInPageUrlResolutionTest.ts b/tests/navigation/publicSignInPageUrlResolutionTest.ts new file mode 100644 index 000000000000..6ef50278052c --- /dev/null +++ b/tests/navigation/publicSignInPageUrlResolutionTest.ts @@ -0,0 +1,63 @@ +import type {NavigationState, PartialState} from '@react-navigation/routers'; +import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath'; +import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; +import NAVIGATORS from '@src/NAVIGATORS'; +import SCREENS from '@src/SCREENS'; + +/** + * URL ↔ state resolution for the logged-out sign-in page. + * + * PublicScreens registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated + * top-level navigator) instead of SCREENS.HOME. TAB_NAVIGATOR has no path of its own, so the + * logged-out URL resolves to the root "/" instead of "/Home". The authenticated Home tab is the HOME + * route *nested inside* TAB_NAVIGATOR and still resolves to "/home". + * + * Before the fix, SignInPage was registered as the top-level SCREENS.HOME with no root-level path + * mapping, so React Navigation derived "/Home" from the screen name and the logged-out address bar + * showed "/Home". + * + * The companion render test (PublicSignInPageRootUrlTest) pins that PublicScreens actually registers + * the SignInPage under TAB_NAVIGATOR; this file pins that the shared linking config maps that name to + * "/" (and back). + */ +describe('Public sign-in page URL resolution', () => { + describe('through the real linking config (getAdaptedStateFromPath → getPathFromState)', () => { + it('maps "/" to a TAB_NAVIGATOR root and back to "/" (the SignInPage host the public navigator renders)', () => { + const state = getAdaptedStateFromPath('/', undefined); + + // The top-level route the linking config produces for "/" must be the same name PublicScreens + // registers the SignInPage under, otherwise the logged-out navigator can't render it. + expect(state?.routes.at(-1)?.name).toBe(NAVIGATORS.TAB_NAVIGATOR); + expect(getPathFromState(state)).toBe('/'); + }); + + it('redirects the legacy "/Home" URL (cached last-visited path) to the root "/"', () => { + const state = getAdaptedStateFromPath('/Home', undefined); + + expect(getPathFromState(state)).toBe('/'); + }); + }); + + describe('getPathFromState', () => { + it('returns "/" for the unauthenticated TAB_NAVIGATOR root (SignInPage, no nested tab state)', () => { + const state: PartialState = {index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}; + expect(getPathFromState(state)).toBe('/'); + }); + + it('returns "/" for the authenticated root (TAB_NAVIGATOR with ReportsSplit focused — the empty-path inbox)', () => { + const state: PartialState = { + index: 0, + routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 1, routes: [{name: SCREENS.HOME}, {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]}}], + }; + expect(getPathFromState(state)).toBe('/'); + }); + + it('keeps "/home" for the authenticated Home tab nested in TAB_NAVIGATOR', () => { + const state: PartialState = { + index: 0, + routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 0, routes: [{name: SCREENS.HOME}]}}], + }; + expect(getPathFromState(state)).toBe('/home'); + }); + }); +}); From 0ec6b728ffc27572166983162820722a8de307c2 Mon Sep 17 00:00:00 2001 From: sumo_slonik Date: Wed, 24 Jun 2026 12:22:47 +0200 Subject: [PATCH 3/4] Tighten sign-in root URL change: minimal comments, leaner tests - Trim comments to behavior-only (drop change/history narration) across PublicScreens, getAdaptedStateFromPath, getStateToResetAfterLogout, types, and the ValidateLoginPage 2FA reset. - Consolidate navigation tests into one PublicScreens render guard (renders SignInPage under TAB_NAVIGATOR at '/', plus the legacy /Home -> / redirect); remove the redundant getPathFromState resolution file. - Make the test's jest.mock factory robust (mock-prefixed hoisted function declaration) instead of relying on module-load ordering. --- .../Navigation/AppNavigator/PublicScreens.tsx | 5 +- .../helpers/getAdaptedStateFromPath.ts | 5 +- .../helpers/getStateToResetAfterLogout.ts | 5 +- src/libs/Navigation/types.ts | 3 +- src/pages/ValidateLoginPage/index.web.tsx | 9 +-- .../PublicSignInPageRootUrlTest.tsx | 59 ++++++----------- .../publicSignInPageUrlResolutionTest.ts | 63 ------------------- 7 files changed, 30 insertions(+), 119 deletions(-) delete mode 100644 tests/navigation/publicSignInPageUrlResolutionTest.ts diff --git a/src/libs/Navigation/AppNavigator/PublicScreens.tsx b/src/libs/Navigation/AppNavigator/PublicScreens.tsx index 324da63b257e..1df1671d7b2a 100644 --- a/src/libs/Navigation/AppNavigator/PublicScreens.tsx +++ b/src/libs/Navigation/AppNavigator/PublicScreens.tsx @@ -24,9 +24,8 @@ function PublicScreens() { const StyleUtils = useStyleUtils(); return ( - {/* The structure for the root route has to be the same in public and auth screens, so the SignInPage is registered under */} - {/* NAVIGATORS.TAB_NAVIGATOR (the authenticated top-level navigator). TAB_NAVIGATOR has no path of its own, so the */} - {/* unauthenticated URL stays at the root ("/") instead of resolving to a tab path like "/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 "/". */} ; [SCREENS.UNLINK_LOGIN]: { accountID?: string; diff --git a/src/pages/ValidateLoginPage/index.web.tsx b/src/pages/ValidateLoginPage/index.web.tsx index 5b4001ffcc2f..ff0df633174e 100644 --- a/src/pages/ValidateLoginPage/index.web.tsx +++ b/src/pages/ValidateLoginPage/index.web.tsx @@ -110,12 +110,9 @@ 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 TAB_NAVIGATOR instead (the public - // SignInPage is registered under that name) — 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. diff --git a/tests/navigation/PublicSignInPageRootUrlTest.tsx b/tests/navigation/PublicSignInPageRootUrlTest.tsx index 002bac4513e2..5c6fe098df69 100644 --- a/tests/navigation/PublicSignInPageRootUrlTest.tsx +++ b/tests/navigation/PublicSignInPageRootUrlTest.tsx @@ -3,65 +3,46 @@ 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'; -import SCREENS from '@src/SCREENS'; -/** - * Stub every public page so the test mounts the real `PublicScreens` navigator (the thing under test) - * without dragging in each page's provider tree. Only the route-name → component wiring matters here. - */ -const makeStub = (testID: string) => () => ; - -jest.mock('@pages/signin/SignInPage', () => makeStub('sign-in-page')); -jest.mock('@pages/ConnectionCompletePage', () => makeStub('connection-complete-page')); -jest.mock('@pages/LogInWithShortLivedAuthTokenPage', () => makeStub('login-with-short-lived-token-page')); -jest.mock('@pages/signin/SAMLSignInPage', () => makeStub('saml-sign-in-page')); -jest.mock('@pages/UnlinkLoginPage', () => makeStub('unlink-login-page')); -jest.mock('@pages/ValidateLoginPage', () => makeStub('validate-login-page')); -jest.mock('@libs/Navigation/AppNavigator/Navigators/TestToolsModalNavigator', () => makeStub('test-tools-modal-navigator')); - -// The navigator-level hooks reach for theme/style context; stub them so we don't need the provider tree. +// 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: () => ({})})); -/** - * Regression guard for the logged-out sign-in page URL. - * - * `PublicScreens` registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated - * top-level navigator) instead of SCREENS.HOME. Because TAB_NAVIGATOR has no path of its own, the - * logged-out URL resolves to the root "/" instead of "/Home". - * - * Before the fix the SignInPage was registered as the top-level SCREENS.HOME, which has no root-level - * path mapping, so React Navigation derived "/Home" from the screen name. - */ -describe('PublicScreens - sign-in page root URL', () => { - it('renders the SignInPage for the TAB_NAVIGATOR root route and keeps the URL at "/"', () => { +describe('Logged-out sign-in page URL', () => { + it('renders the SignInPage at the root "/" (registered under TAB_NAVIGATOR, which has no path)', () => { render( , ); - // The public root route is named TAB_NAVIGATOR and renders the SignInPage component. expect(screen.getByTestId('sign-in-page')).toBeOnTheScreen(); const rootState = navigationRef.getRootState(); expect(rootState.routes.at(0)?.name).toBe(NAVIGATORS.TAB_NAVIGATOR); - - // TAB_NAVIGATOR has no path of its own, so the logged-out address bar stays at the root. expect(getPathFromState(rootState)).toBe('/'); }); - it('does NOT register the SignInPage under SCREENS.HOME (the source of the old "/Home" URL)', () => { - render( - - - , - ); - - expect(navigationRef.getRootState().routeNames).not.toContain(SCREENS.HOME); + it('redirects a legacy "/Home" URL to the root "/"', () => { + expect(getPathFromState(getAdaptedStateFromPath('/Home', undefined))).toBe('/'); }); }); diff --git a/tests/navigation/publicSignInPageUrlResolutionTest.ts b/tests/navigation/publicSignInPageUrlResolutionTest.ts deleted file mode 100644 index 6ef50278052c..000000000000 --- a/tests/navigation/publicSignInPageUrlResolutionTest.ts +++ /dev/null @@ -1,63 +0,0 @@ -import type {NavigationState, PartialState} from '@react-navigation/routers'; -import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath'; -import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; -import NAVIGATORS from '@src/NAVIGATORS'; -import SCREENS from '@src/SCREENS'; - -/** - * URL ↔ state resolution for the logged-out sign-in page. - * - * PublicScreens registers the SignInPage under NAVIGATORS.TAB_NAVIGATOR (mirroring the authenticated - * top-level navigator) instead of SCREENS.HOME. TAB_NAVIGATOR has no path of its own, so the - * logged-out URL resolves to the root "/" instead of "/Home". The authenticated Home tab is the HOME - * route *nested inside* TAB_NAVIGATOR and still resolves to "/home". - * - * Before the fix, SignInPage was registered as the top-level SCREENS.HOME with no root-level path - * mapping, so React Navigation derived "/Home" from the screen name and the logged-out address bar - * showed "/Home". - * - * The companion render test (PublicSignInPageRootUrlTest) pins that PublicScreens actually registers - * the SignInPage under TAB_NAVIGATOR; this file pins that the shared linking config maps that name to - * "/" (and back). - */ -describe('Public sign-in page URL resolution', () => { - describe('through the real linking config (getAdaptedStateFromPath → getPathFromState)', () => { - it('maps "/" to a TAB_NAVIGATOR root and back to "/" (the SignInPage host the public navigator renders)', () => { - const state = getAdaptedStateFromPath('/', undefined); - - // The top-level route the linking config produces for "/" must be the same name PublicScreens - // registers the SignInPage under, otherwise the logged-out navigator can't render it. - expect(state?.routes.at(-1)?.name).toBe(NAVIGATORS.TAB_NAVIGATOR); - expect(getPathFromState(state)).toBe('/'); - }); - - it('redirects the legacy "/Home" URL (cached last-visited path) to the root "/"', () => { - const state = getAdaptedStateFromPath('/Home', undefined); - - expect(getPathFromState(state)).toBe('/'); - }); - }); - - describe('getPathFromState', () => { - it('returns "/" for the unauthenticated TAB_NAVIGATOR root (SignInPage, no nested tab state)', () => { - const state: PartialState = {index: 0, routes: [{name: NAVIGATORS.TAB_NAVIGATOR}]}; - expect(getPathFromState(state)).toBe('/'); - }); - - it('returns "/" for the authenticated root (TAB_NAVIGATOR with ReportsSplit focused — the empty-path inbox)', () => { - const state: PartialState = { - index: 0, - routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 1, routes: [{name: SCREENS.HOME}, {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]}}], - }; - expect(getPathFromState(state)).toBe('/'); - }); - - it('keeps "/home" for the authenticated Home tab nested in TAB_NAVIGATOR', () => { - const state: PartialState = { - index: 0, - routes: [{name: NAVIGATORS.TAB_NAVIGATOR, state: {index: 0, routes: [{name: SCREENS.HOME}]}}], - }; - expect(getPathFromState(state)).toBe('/home'); - }); - }); -}); From e15cfeaa6a358106cf1ca2de6a3d04f3a6038cb0 Mon Sep 17 00:00:00 2001 From: sumo_slonik Date: Wed, 24 Jun 2026 16:35:33 +0200 Subject: [PATCH 4/4] Clear preserved navigator states on logout restoreTabNavigatorRoutes reattaches the prior session's TAB_NAVIGATOR subtree to the public sign-in route (which now shares that name), so the logged-out reset failed to drop a previous deep-linked report. Evict the in-memory preserved navigator states on logout so the next session starts clean. --- .../createSplitNavigator/usePreserveNavigatorState.ts | 11 ++++++++++- src/libs/Navigation/NavigationRoot.tsx | 6 +++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState.ts b/src/libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState.ts index 5baf118651d3..ce3fb18d710d 100644 --- a/src/libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState.ts +++ b/src/libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState.ts @@ -35,6 +35,15 @@ const clearPreservedSearchNavigatorStates = () => { } }; +// 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 15a36c6dfaaa..d711251053ad 100644 --- a/src/libs/Navigation/NavigationRoot.tsx +++ b/src/libs/Navigation/NavigationRoot.tsx @@ -26,7 +26,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; 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'; @@ -224,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;