From 1028c746bc4d54111bbb3e5577cf7fe1b2ab9d94 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Fri, 18 Apr 2025 14:52:19 -0700 Subject: [PATCH 1/6] WIP restrict navigating away from require 2fa page --- src/libs/Navigation/Navigation.ts | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index b82f64e037bb..9fad0c22dfa5 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -17,7 +17,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {HybridAppRoute, Route} from '@src/ROUTES'; import ROUTES, {HYBRID_APP_ROUTES} from '@src/ROUTES'; import SCREENS, {PROTECTED_SCREENS} from '@src/SCREENS'; -import type {Report} from '@src/types/onyx'; +import type {Account, Report} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import getInitialSplitNavigatorState from './AppNavigator/createSplitNavigator/getInitialSplitNavigatorState'; import originalCloseRHPFlow from './helpers/closeRHPFlow'; @@ -46,6 +46,18 @@ Onyx.connect({ }, }); +let account: OnyxEntry; +Onyx.connect({ + key: ONYXKEYS.ACCOUNT, + callback: (value) => { + account = value; + }, +}); + +function shouldShowRequire2FAPage() { + return !!account?.needsTwoFactorAuthSetup && !account?.requiresTwoFactorAuth; +} + let resolveNavigationIsReadyPromise: () => void; const navigationIsReadyPromise = new Promise((resolve) => { resolveNavigationIsReadyPromise = resolve; @@ -63,9 +75,16 @@ function setShouldPopAllStateOnUP(shouldPopAllStateFlag: boolean) { } /** - * Checks if the navigationRef is ready to perform a method. + * Checks if the navigationRef is ready to perform a method, and blocks navigation if the user is required to enable 2FA. + * If a navigation method is called while 2FA is required, and the target route is not the 2FA page, navigation is blocked. */ function canNavigate(methodName: string, params: Record = {}): boolean { + // Block navigation if 2FA is required and the target is not the 2FA page + const targetRoute = params.route || params.backToRoute; + if (shouldShowRequire2FAPage() && targetRoute !== ROUTES.REQUIRE_TWO_FACTOR_AUTH) { + Log.info(`[Navigation] Blocked navigation due to active 2FA requirement:`, targetRoute); + return false; + } if (navigationRef.isReady()) { return true; } @@ -307,7 +326,7 @@ function goUp(backToRoute: Route, options?: GoBackOptions) { * @param options - Optional configuration that affects navigation logic */ function goBack(backToRoute?: Route, options?: GoBackOptions) { - if (!canNavigate('goBack')) { + if (!canNavigate('goBack', {backToRoute})) { return; } From b2c4a1965144f866c15fc42450a720ed75ab4f87 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 21 Apr 2025 06:58:13 -0700 Subject: [PATCH 2/6] Allow nav to select routes for enabling 2fa --- src/libs/Navigation/Navigation.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index 9fad0c22dfa5..06606046557c 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -81,8 +81,17 @@ function setShouldPopAllStateOnUP(shouldPopAllStateFlag: boolean) { function canNavigate(methodName: string, params: Record = {}): boolean { // Block navigation if 2FA is required and the target is not the 2FA page const targetRoute = params.route || params.backToRoute; - if (shouldShowRequire2FAPage() && targetRoute !== ROUTES.REQUIRE_TWO_FACTOR_AUTH) { - Log.info(`[Navigation] Blocked navigation due to active 2FA requirement:`, targetRoute); + + // Allow navigation to only these routes to enable 2fa + const allowed2FARoutes = [ + ROUTES.REQUIRE_TWO_FACTOR_AUTH, + ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), + ROUTES.SETTINGS_2FA_VERIFY.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), + ROUTES.SETTINGS_2FA_SUCCESS.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), + ]; + + if (shouldShowRequire2FAPage() && !allowed2FARoutes.includes(targetRoute)) { + Log.info(`[Navigation] Blocked navigation due to active 2FA requirement for route ${targetRoute}`); return false; } if (navigationRef.isReady()) { From 06264b47ebfacbbe06ff9318c8edab5e90aa3c7c Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 21 Apr 2025 07:06:36 -0700 Subject: [PATCH 3/6] Fix type for can navigate params --- src/libs/Navigation/Navigation.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index 06606046557c..708ae139c133 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -74,23 +74,26 @@ function setShouldPopAllStateOnUP(shouldPopAllStateFlag: boolean) { shouldPopAllStateOnUP = shouldPopAllStateFlag; } +type CanNavigateParams = { + route?: Route; + backToRoute?: Route; +}; + /** * Checks if the navigationRef is ready to perform a method, and blocks navigation if the user is required to enable 2FA. * If a navigation method is called while 2FA is required, and the target route is not the 2FA page, navigation is blocked. */ -function canNavigate(methodName: string, params: Record = {}): boolean { - // Block navigation if 2FA is required and the target is not the 2FA page - const targetRoute = params.route || params.backToRoute; - - // Allow navigation to only these routes to enable 2fa - const allowed2FARoutes = [ +function canNavigate(methodName: string, params: CanNavigateParams = {}): boolean { + // Block navigation if 2FA is required and the targetRoute is not part of the flow to enable 2FA + const targetRoute = params.route ?? params.backToRoute; + const allowed2FARoutes: Route[] = [ ROUTES.REQUIRE_TWO_FACTOR_AUTH, ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), ROUTES.SETTINGS_2FA_VERIFY.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), ROUTES.SETTINGS_2FA_SUCCESS.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), ]; - if (shouldShowRequire2FAPage() && !allowed2FARoutes.includes(targetRoute)) { + if (shouldShowRequire2FAPage() && targetRoute && !allowed2FARoutes.includes(targetRoute)) { Log.info(`[Navigation] Blocked navigation due to active 2FA requirement for route ${targetRoute}`); return false; } From f4efda3496e5e52a68dba2aaca99d93dad2a31bf Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 21 Apr 2025 07:08:56 -0700 Subject: [PATCH 4/6] Only store pending route when nav isn't ready --- src/libs/Navigation/Navigation.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index 708ae139c133..ad5d7ba1519a 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -190,10 +190,12 @@ function isActiveRoute(routePath: Route): boolean { */ function navigate(route: Route, options?: LinkToOptions) { if (!canNavigate('navigate', {route})) { - // Store intended route if the navigator is not yet available, - // we will try again after the NavigationContainer is ready - Log.hmmm(`[Navigation] Container not yet ready, storing route as pending: ${route}`); - pendingRoute = route; + if (!navigationRef.isReady()) { + // Store intended route if the navigator is not yet available, + // we will try again after the NavigationContainer is ready + Log.hmmm(`[Navigation] Container not yet ready, storing route as pending: ${route}`); + pendingRoute = route; + } return; } From 5255040e58446dffed9826f9b4ef54041c356564 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 21 Apr 2025 07:15:54 -0700 Subject: [PATCH 5/6] Clean up diff --- src/libs/Navigation/Navigation.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index ad5d7ba1519a..b3b4a2841009 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -80,8 +80,7 @@ type CanNavigateParams = { }; /** - * Checks if the navigationRef is ready to perform a method, and blocks navigation if the user is required to enable 2FA. - * If a navigation method is called while 2FA is required, and the target route is not the 2FA page, navigation is blocked. + * Checks if the route can be navigated to based on whether the navigation ref is ready and if 2FA is required to be set up. */ function canNavigate(methodName: string, params: CanNavigateParams = {}): boolean { // Block navigation if 2FA is required and the targetRoute is not part of the flow to enable 2FA @@ -94,7 +93,7 @@ function canNavigate(methodName: string, params: CanNavigateParams = {}): boolea ]; if (shouldShowRequire2FAPage() && targetRoute && !allowed2FARoutes.includes(targetRoute)) { - Log.info(`[Navigation] Blocked navigation due to active 2FA requirement for route ${targetRoute}`); + Log.info(`[Navigation] Blocked navigation because 2FA is required to be set up to access route: ${targetRoute}`); return false; } if (navigationRef.isReady()) { @@ -289,7 +288,7 @@ const defaultGoBackOptions: Required = { * @param options - Optional configuration that affects navigation logic, such as parameter comparison. */ function goUp(backToRoute: Route, options?: GoBackOptions) { - if (!canNavigate('goUp') || !navigationRef.current) { + if (!canNavigate('goUp', {backToRoute}) || !navigationRef.current) { Log.hmmm(`[Navigation] Unable to go up. Can't navigate.`); return; } From 53fd6ab60719823287c13701919af9ba00d1d18a Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 21 Apr 2025 08:09:44 -0700 Subject: [PATCH 6/6] Add const in file --- src/libs/Navigation/Navigation.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libs/Navigation/Navigation.ts b/src/libs/Navigation/Navigation.ts index b3b4a2841009..05f747e72c8b 100644 --- a/src/libs/Navigation/Navigation.ts +++ b/src/libs/Navigation/Navigation.ts @@ -37,6 +37,14 @@ import {linkingConfig} from './linkingConfig'; import navigationRef from './navigationRef'; import type {NavigationPartialRoute, NavigationRoute, NavigationStateRoute, RootNavigatorParamList, State} from './types'; +// Routes which are part of the flow to set up 2FA +const SET_UP_2FA_ROUTES: Route[] = [ + ROUTES.REQUIRE_TWO_FACTOR_AUTH, + ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), + ROUTES.SETTINGS_2FA_VERIFY.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), + ROUTES.SETTINGS_2FA_SUCCESS.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), +]; + let allReports: OnyxCollection; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, @@ -85,14 +93,7 @@ type CanNavigateParams = { function canNavigate(methodName: string, params: CanNavigateParams = {}): boolean { // Block navigation if 2FA is required and the targetRoute is not part of the flow to enable 2FA const targetRoute = params.route ?? params.backToRoute; - const allowed2FARoutes: Route[] = [ - ROUTES.REQUIRE_TWO_FACTOR_AUTH, - ROUTES.SETTINGS_2FA_ROOT.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), - ROUTES.SETTINGS_2FA_VERIFY.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), - ROUTES.SETTINGS_2FA_SUCCESS.getRoute(ROUTES.REQUIRE_TWO_FACTOR_AUTH), - ]; - - if (shouldShowRequire2FAPage() && targetRoute && !allowed2FARoutes.includes(targetRoute)) { + if (shouldShowRequire2FAPage() && targetRoute && !SET_UP_2FA_ROUTES.includes(targetRoute)) { Log.info(`[Navigation] Blocked navigation because 2FA is required to be set up to access route: ${targetRoute}`); return false; }