redirect 2FA in incognito while 2FA is enabled by mukhrr · Pull Request #93656 · Expensify/App

mukhrr · 2026-06-16T04:09:18Z

Explanation of Change

Follow-up of #90488

Fixed Issues

$ #93753
PROPOSAL: #90488 (comment)

Tests

Log in with existing 2FA enabled account and request magic code
Navigate to email and locate the magic link - change to staging if needed
Open an new INCOGNITO window and navigate to the staging link
Verify the "Here's your magic code" page is displayed
Verify there's a link to "Just sign In here"
Click the link

Verify you are in 2FA code inserting page
Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

REC-20260616091050.mp4

melvin-bot · 2026-06-16T04:09:24Z

@mkhutornyi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 049d508b5d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

codecov · 2026-06-16T05:04:14Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/pages/ValidateLoginPage/index.web.tsx	`100.00% <100.00%> (ø)`
... and 300 files with indirect coverage changes

mkhutornyi · 2026-06-16T16:48:32Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

Android: mWeb Chrome

iOS: HybridApp

iOS: mWeb Safari

MacOS: Chrome / Safari

web.mov

chatgpt-codex-connector · 2026-06-16T16:55:10Z

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: b6cc47cbba

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

MelvinBot · 2026-06-16T16:57:34Z

Code Review

Reviewed the rewrite of the redirect useEffect in ValidateLoginPage/index.web.tsx. The core 2FA-redirect logic looks sound — SCREENS.HOME does resolve to the public SignInPage while unauthenticated, and the ignore guard correctly protects the stack reset. Two issues worth a look:

1. `handleExitToNavigation(exitTo)` can fire twice for a 2FA + `exitTo` magic link (medium)

The new effect calls handleExitToNavigation(exitTo) on every run, and its deps [canCompleteTwoFactorOnSignIn, exitTo] cause it to re-run when canCompleteTwoFactorOnSignIn flips false (which happens the moment isSignedIn flips true after 2FA completes).

Failure scenario: 2FA account opens a magic link with exitTo (e.g. concierge).

Effect runs on mount → canCompleteTwoFactorOnSignIn = true → registers handleExitToNavigation('concierge'), which awaits the authToken.
User enters their 2FA code → authToken lands → the awaited callback fires goBack(HOME) + navigate('concierge').
Simultaneously isSignedIn flips true → canCompleteTwoFactorOnSignIn flips false → the effect re-runs and calls handleExitToNavigation('concierge') a second time. Since authToken is now present, waitForUserSignIn() resolves immediately → a second goBack(HOME) + navigate('concierge').

Result is a duplicate navigation (extra browser-history entry / transient flash). The ignore cleanup only guards the navigationRef.reset, not this call. Note this is genuinely new — the old effect did not call handleExitToNavigation for the 2FA path (it goBack()'d), and its deps didn't react to the isSignedIn flip.

Suggested fix: only register the deferred exitTo handler once (e.g. guard it so it doesn't re-fire once already signed in / on the re-run), so the second navigate is avoided.

2. `navigationRef.reset` isn't re-checked for readiness inside the `.then` (low / optional)

Line 115-122 calls navigationRef.reset(...) after isNavigationReady() resolves, without re-checking navigationRef.isReady(). isNavigationReady() is a one-shot promise that stays resolved forever, so the container could theoretically be mid-remount when the microtask runs.

This degrades gracefully (React Navigation's reset logs a console error and no-ops rather than throwing, and the "2FA required" modal fallback stays rendered), and it matches the existing convention in Navigation.ts where helpers also don't re-check after isNavigationReady(). So this is informational — flagging only because the !isSignedIn gate makes the window narrow but not provably impossible.

This is an automated review — treat findings as starting points. Reply with @MelvinBot <feedback> to refine.

mukhrr · 2026-06-16T17:13:03Z

I think neither needs a fix here @mkhutornyi

handleExitToNavigation(exitTo) can fire twice for a 2FA + exitTo magic link (medium)

The second handleExitToNavigation can’t happen. The re-run it describes requires ValidateLoginPage to still be mounted when isSignedIn flips — but it isn’t. The moment canCompleteTwoFactorOnSignIn becomes true, isNavigationReady().then(...) resets the stack to SCREENS.HOME, and SCREENS.HOME (SignInPage) and SCREENS.VALIDATE_LOGIN are sibling routes in the same public RootStack, so the reset removes VALIDATE_LOGIN and unmounts this page — within a microtask of canComplete becoming true.

mukhrr · 2026-06-16T17:14:00Z

navigationRef.reset isn't re-checked for readiness inside the .then (low / optional)

navigationRef.isReady() re-check. Agreed this is informational. reset() no-ops gracefully if the container is mid-remount, the “2FA required” modal stays rendered as the fallback, and not re-checking after isNavigationReady() matches the existing convention in Navigation.ts. Combined with the !isSignedIn gate and the unmount above, I’ll leave it as-is to match convention .

mkhutornyi · 2026-06-17T08:24:34Z

+        // 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.
+        if (exitTo) {
+            handleExitToNavigation(exitTo);


This is now also called when canCompleteTwoFactorOnSignIn = true. Is it safe?

Good question — it's safe (the reset unmounts this page before 2FA completes, and the only double-call is at mount where both share the single authPromiseResolver, so one navigate fires). But I'll make it explicit by guarding the registration with !isSignedIn so it can never re-register post-sign-in — no reliance on timing.

mkhutornyi · 2026-06-17T08:26:28Z

+                // 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;
+                }


I think ignore will be reset to false when this effect re-runs. Am I wrong?

Each effect run closes over its own ignore. On re-run, React runs the previous run's cleanup (ignore = true on the binding the pending .then captured) before the new body runs with a fresh ignore = false and a new .then — so the stale callback bails and the new one proceeds. It's the standard React "ignore" cleanup pattern; the stale-callback guard test exercises exactly this.

MelvinBot · 2026-06-17T08:45:28Z

Regression review

No code-level regressions found. The diff is scoped to ValidateLoginPage/index.web.tsx (web/mWeb only) plus its test. CI is green, patch coverage is 100%, and the new mechanism matches existing, proven patterns. The main gap is test/QA coverage, not code — see below.

What I verified

Reset-to-HOME is the same mechanism logout uses. index.web.tsx:121 (navigationRef.reset({index: 0, routes: [{name: SCREENS.HOME}]})) is byte-identical to the logout reset at NavigationRoot.tsx:238 / getStateToResetAfterLogout.ts:20, so an unauthenticated user lands on the public SignInPage as intended.
The gating condition matches SignInPage's 2FA stage exactly. canCompleteTwoFactorOnSignIn requires requiresTwoFactorAuth && !!credentials?.validateCode, which is the same condition SignInPage uses to render the authenticator stage: SignInPage.tsx:241 (isTwoFactorStage = account?.requiresTwoFactorAuth && !!credentials?.validateCode). So the hand-off can't land on the wrong stage.
credentials.validateCode survives the sign-in attempt. For the exitTo+2FA path the mount effect calls signInWithValidateCode, but that action never clears validateCode — the success path re-merges it (Session/index.ts:941-948) and the optimistic/failure paths don't touch it. So SignInPage still finds the code after the reset.
The removed Navigation.goBack() was already a no-op from the public /v/ route (the reason for this follow-up), so narrowing/replacing it loses no real navigation behavior.
Multiple handleExitToNavigation calls are benign. Each call just overwrites authPromiseResolver in waitForUserSignIn (Session/index.ts:1418-1435) — last writer wins, no double navigation.

Worth confirming before merge (coverage gap, not a code bug)

mWeb evidence is missing. This is index.web.tsx, which also runs on mobile web (Android Chrome / iOS Safari). The PR body's Android: mWeb Chrome and iOS: mWeb Safari video sections are empty — only a macOS Chrome video is attached. The reset-to-HOME → SignInPage → authenticator-stage handoff should be confirmed on mWeb since that's where the original blank-page bug was reported (Windows Chrome).
The unit tests mock signInWithValidateCode and handleExitToNavigation, so they assert the page's intent (reset called, handleExitToNavigation called) but not the real end-to-end outcome — that after the reset the deferred exitTo navigation actually survives ValidateLoginPage unmounting and lands the user on their destination. That path is only covered by manual testing.

Next Steps: Reply with @MelvinBot <feedback> to refine this review.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6cc47cbba

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

mukhrr · 2026-06-19T02:19:40Z

@mkhutornyi ready to proceed

mkhutornyi

LGTM

OSBotify · 2026-06-19T16:40:15Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-06-19T16:43:15Z

🚧 @blimpich has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-06-19T16:48:48Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #93656.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-813dbed3097-aa5daf5-806f0e924e721966bc2c3bb2c57070c52ccd721a/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-813dbed-aa5daf5-aa6dcb30a31407d93b957c1ad2eeea994c7321b6/index.html

Web 🕸️
https://93656.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-06-22T23:27:41Z

🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.17-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-06-22T23:29:29Z

🤖 No help site changes required.

This PR is an internal fix to the sign-in navigation logic in src/pages/ValidateLoginPage/index.web.tsx. When a 2FA magic link is opened in a separate session (e.g. an incognito window), the user is now redirected to the sign-in page's authenticator-code stage to finish signing in, instead of landing on the dead-end "2FA required" informational modal. exitTo deep links are also preserved through 2FA so the user reaches their destination.

This is a behavior/UX fix with no new user-facing feature, setting, tab, or button to document. The existing help articles already describe the correct expected behavior, and this change brings the product in line with what they say:

Two-Factor Authentication (2FA) — "After setup, login requires both: 1. Your magic code (sent via email). 2. The 6-digit verification code from your authenticator app."
Login Troubleshooting — "After entering your Magic Code, you'll be prompted to enter another code — this one comes from your authenticator app."

Both articles already state that a 2FA prompt follows the magic code, which is exactly the flow this PR now delivers in a separate session. No draft docs PR was created.

@mukhrr, no help site PR was needed for this change — the existing articles already reflect the current behavior. If you believe a docs update is warranted, let me know and I'll open a draft PR.

OSBotify · 2026-06-23T21:59:55Z

🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

OSBotify · 2026-06-23T23:38:25Z

🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

redirect 2FA in incognito while 2FA is enabled

049d508

mukhrr requested review from a team as code owners June 16, 2026 04:09

melvin-bot Bot assigned mukhrr Jun 16, 2026

melvin-bot Bot requested review from flaviadefaria and mkhutornyi June 16, 2026 04:09

melvin-bot Bot removed request for a team June 16, 2026 04:09

mukhrr mentioned this pull request Jun 16, 2026

auto-sign in with sign in link in separate browser #90488

Merged

53 tasks

github-actions Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread src/pages/ValidateLoginPage/index.web.tsx

chatgpt-codex-connector Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread src/pages/ValidateLoginPage/index.web.tsx Outdated

mukhrr added 2 commits June 16, 2026 09:54

removed !exitTo unnecessary gate

703a615

fixed spell check fail

20e3941

added 2FA enabled account tests

b6cc47c

mkhutornyi reviewed Jun 17, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread src/pages/ValidateLoginPage/index.web.tsx Outdated

fix: scope the 2FA sign-in redirect to the current magic link

65638bc

mukhrr requested a review from mkhutornyi June 19, 2026 02:10

mkhutornyi approved these changes Jun 19, 2026

View reviewed changes

melvin-bot Bot requested a review from blimpich June 19, 2026 05:20

mkhutornyi reviewed Jun 19, 2026

View reviewed changes

blimpich approved these changes Jun 19, 2026

View reviewed changes

blimpich merged commit 813dbed into Expensify:main Jun 19, 2026
34 of 35 checks passed

OSBotify mentioned this pull request Jun 22, 2026

Deploy Checklist: New Expensify 2026-06-22 #94222

Closed

melvin-bot Bot mentioned this pull request Jun 23, 2026

[Due for payment 2026-06-30] Login - User is asked to go back to original session to enter 2FA after clicking sign in here #93753

Closed

7 tasks

MelvinBot mentioned this pull request Jun 26, 2026

[Due for payment 2026-06-24] [$250] Login - Blank page opens when login via magic link different session #89545

Closed

7 tasks

Uh oh!

Conversation

mukhrr commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Jun 16, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

codecov Bot commented Jun 16, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mkhutornyi commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

chatgpt-codex-connector Bot commented Jun 16, 2026

Uh oh!

MelvinBot commented Jun 16, 2026

Code Review

1. handleExitToNavigation(exitTo) can fire twice for a 2FA + exitTo magic link (medium)

2. navigationRef.reset isn't re-checked for readiness inside the .then (low / optional)

Uh oh!

mukhrr commented Jun 16, 2026

Uh oh!

mukhrr commented Jun 16, 2026

Uh oh!

mkhutornyi Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

mukhrr Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

mkhutornyi Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

mukhrr Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Jun 17, 2026

Regression review

What I verified

Worth confirming before merge (coverage gap, not a code bug)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

mukhrr commented Jun 19, 2026

Uh oh!

mkhutornyi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OSBotify commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

OSBotify commented Jun 22, 2026

Uh oh!

MelvinBot commented Jun 22, 2026

Uh oh!

OSBotify commented Jun 23, 2026

Uh oh!

mukhrr commented Jun 16, 2026 •

edited

Loading

codecov Bot commented Jun 16, 2026 •

edited by github-actions Bot

Loading

mkhutornyi commented Jun 16, 2026 •

edited

Loading

1. `handleExitToNavigation(exitTo)` can fire twice for a 2FA + `exitTo` magic link (medium)

2. `navigationRef.reset` isn't re-checked for readiness inside the `.then` (low / optional)