-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Use PUSH instead of REPLACE for 2FA Download codes navigation on web #94761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import {useIsFocused} from '@react-navigation/native'; | ||
| import {findFocusedRoute, useIsFocused} from '@react-navigation/native'; | ||
| import React, {useEffect, useState} from 'react'; | ||
| import {View} from 'react-native'; | ||
| import ActivityIndicator from '@components/ActivityIndicator'; | ||
|
|
@@ -20,13 +20,15 @@ import Clipboard from '@libs/Clipboard'; | |
| import getPlatform from '@libs/getPlatform'; | ||
| import localFileDownload from '@libs/localFileDownload'; | ||
| import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; | ||
| import getStateFromPath from '@libs/Navigation/helpers/getStateFromPath'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan'; | ||
| import {toggleTwoFactorAuth} from '@userActions/Session'; | ||
| import {quitAndNavigateBack, setCodesAreCopied} from '@userActions/TwoFactorAuthActions'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; | ||
| import SCREENS from '@src/SCREENS'; | ||
| import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue'; | ||
| import TwoFactorAuthWrapper from './TwoFactorAuthWrapper'; | ||
|
|
||
|
|
@@ -45,6 +47,12 @@ function DynamicTwoFactorAuthPage() { | |
|
|
||
| const backPath = useDynamicBackPath(DYNAMIC_ROUTES.TWO_FACTOR_AUTH_ROOT.path); | ||
|
|
||
| // Determine whether the 2FA flow was entered from Settings > Security or from a non-settings flow | ||
| // (e.g. the bank account or Xero 2FA requirement), so we can return the user to the right place once 2FA is enabled. | ||
| const baseState = getStateFromPath(backPath); | ||
| const focusedRoute = baseState ? findFocusedRoute(baseState) : undefined; | ||
| const isSecuritySettingsFlow = focusedRoute?.name === SCREENS.SETTINGS.SECURITY; | ||
|
|
||
| const isWeb = getPlatform() === CONST.PLATFORM.WEB; | ||
|
|
||
| const announceStatus = (message: string) => { | ||
|
|
@@ -72,7 +80,14 @@ function DynamicTwoFactorAuthPage() { | |
|
|
||
| if (isFocused && is2FAEnabled) { | ||
| Navigation.isNavigationReady().then(() => { | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_ENABLED, {forceReplace: true}); | ||
| // For the Settings > Security entry, land on the Enabled page. For non-settings entries return to the | ||
| // originating flow instead, so on web the browser Back button from the success page doesn't divert the | ||
| // user to Settings (the recovery-codes page now stays in history because Download codes uses PUSH). | ||
| if (isSecuritySettingsFlow) { | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_ENABLED, {forceReplace: true}); | ||
| return; | ||
| } | ||
| Navigation.navigate(backPath, {forceReplace: true}); | ||
| }); | ||
| return; | ||
| } | ||
|
|
@@ -185,7 +200,7 @@ function DynamicTwoFactorAuthPage() { | |
| setError(''); | ||
| setCodesAreCopied(); | ||
| announceStatus(translate('fileDownload.success.title')); | ||
| Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.TWO_FACTOR_AUTH_VERIFY.path, backPath), {forceReplace: true}); | ||
| Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.TWO_FACTOR_AUTH_VERIFY.path, backPath), {forceReplace: !isWeb}); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this runs on web from a dynamic entry point such as the bank-account 2FA prompt or the required-Xero overlay, using PUSH leaves the recovery-codes screen underneath the later success screen. After the user enables 2FA, browser Back from the success page lands on that root screen; the root page redirects any enabled account to Useful? React with 👍 / 👎. |
||
| }} | ||
| /> | ||
| )} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?