Fix Okta SAML double login on native session resume

[trasnake87]

Explanation of Change

On native, SAML sign-in opens an in-app browser (openAuthSessionAsync), which backgrounds the app. When the browser returns and the app resumes, reconnectApp() fires with the now-expired authToken and gets a 407. At that moment reauthenticate() runs, and because account.isSAMLRequired is true it takes the SAML branch and calls redirectToSignIn(undefined, true) — tearing the session down before the SAML callback (signInWithShortLivedAuthToken) can sign the user back in. That is the reported "double login": after returning from the IdP the user is bounced back to the SSO login screen.

The isAuthenticatingWithShortLivedToken guard already exists to make reauthenticate() abort while a short-lived-token sign-in is in flight — but it was only set once the browser returned with the token, which is too late for the resume race. This PR sets the guard before opening the in-app browser, so it is already in place when the app resumes and reauthenticate() aborts instead of wiping the session. The guard is cleared by signInWithShortLivedAuthToken() on success and explicitly on the cancel/error paths, so it can never get stuck.

• Session.setIsAuthenticatingWithShortLivedToken(value) — small setter for the RAM-only guard key.
• SAMLSignInPage (native) — set the guard true before openAuthSessionAsync; clear it on cancel/error and on the no-token callback path.

Fixed Issues

$ #86705
PROPOSAL: #86705 (comment)

Tests

Validated on a real Okta → Expensify SAML environment (the resume-after-idle race needs a real IdP):

1. Configure an Okta SAML domain and sign into the native app via SSO.
2. Leave the app idle until the authToken expires (or shorten the Okta session/idle timeout to force it quickly), then background and resume the app.
3. Before this change: on resume the app drops the session and returns to the SSO sign-in screen (double login). Logs show [Reauthenticate] Redirecting to Sign In because SAML is required.
4. After this change: the session is preserved — the app stays signed in and reauthenticate() aborts ([Reauthenticate] Authentication with shortLivedToken is in progress); there is no redirect to sign-in.

Automated coverage (tests/unit/SAMLSignInPageTest.tsx):

• SAMLSignInPage sets the guard to true before openAuthSessionAsync is called.
• The guard is cleared when the browser is cancelled, so future reauthentication is not blocked.

The complementary contract — that reauthenticate() aborts while the guard is set — is already covered by tests/actions/SessionTest.ts.

• Verify that no errors appear in the JS console

Offline tests

N/A — this is a session-resume / re-authentication change that only manifests with connectivity (the resume reconnectApp 407). Offline, the request queue is paused and reauthenticate() does not run.

QA Steps

1. On a native build, sign in to an account that uses Okta/SAML SSO.
2. Background the app and leave it idle long enough for the auth token to expire, then reopen it.
3. Verify the app stays signed in and lands on the user's inbox — it should not bounce back to the SSO login screen.

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (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 followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• 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 new assets were added or existing ones were modified, I verified that:
  • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
  • The assets load correctly across all supported platforms.
• 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.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user 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.

Screenshots/Videos

Android: Native

Bug reproduction on a real Okta SAML session — the app boots with a SAML session, then on the resume-time reconnectApp 407 it tears the session down and bounces to the Okta re-login (the "double login"):

Bug reproduction recording

Android: mWeb Chrome

N/A — this change only touches SAMLSignInPage/index.native.tsx; mWeb uses a separate SAML path and is unaffected.

iOS: Native

iOS: mWeb Safari

N/A — native-only change; mWeb is unaffected.

MacOS: Chrome / Safari

N/A — native-only change; desktop is unaffected.

[melvin-bot]

@thelullabyy 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: 6a34506def

ℹ️ 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".

[chatgpt-codex-connector]

Keep the guard valid while SAML browser is open

When this SAML page is opened from a state where account.isLoading is already false (for example the SSO choice path after BeginSignin has completed), setting only RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN is not enough: reauthenticate() treats that combination as stale (account?.isLoading === false), clears the guard, and then still follows the isSAMLRequired redirect path. In that resume/407 case the new guard is removed before the SAML callback can run, so the double-login race remains for those flows unless this path also marks the account as actively loading or reauthenticate() can distinguish an open native SAML browser from a stale short-lived-token request.

Useful? React with 👍 / 👎.

[trasnake87]

A note on validation here, since #86705 is a timing race: the teardown fires when a reconnectApp 407 lands while the in-app SAML re-auth is mid-flight. For that kind of bug a before/after video isn't a reliable proof — when the fix works, the symptom is a teardown that simply doesn't happen, which on screen is indistinguishable from a normal resume. So the deterministic coverage is the stronger proof:

• tests/unit/SAMLSignInPageTest.tsx asserts the guard is set before openAuthSessionAsync, and is cleared on cancel.
• tests/actions/SessionTest.ts already asserts reauthenticate() aborts while that guard is set.

Together those pin the exact behavior the fix relies on. The recording in the PR description shows the bug's symptom (teardown → Okta re-login) reproduced on a real Okta SAML session.

[thelullabyy]

@trasnake87 Can you add a recording to prove that it works correctly after the change?

[joekaufmanexpensify]

Good for product

[trasnake87]

Thanks @thelullabyy — fair ask. The honest picture: the fix sets the isAuthenticatingWithShortLivedToken guard before the in-app SAML browser opens, and the bug only bites when a reconnectApp 407 fires during the seconds that browser is up — a genuine timing race (which is why it was hard to repro originally). On a dev/emulator build I can't trigger that mid-browser reconnect on demand (the JS is paused behind the Custom Tab), and the fixed outcome is a teardown that simply doesn't happen — on screen indistinguishable from a normal resume.

So the reliable proof is the deterministic test, which exercises exactly the change: SAMLSignInPageTest.tsx asserts the page sets the guard before openAuthSessionAsync (and clears it on cancel), and SessionTest.ts asserts reauthenticate() aborts while that guard is set. The recording in the PR shows the bug itself on a real SAML session.

If a live device repro would help, I'm set up to run the AdHoc build against my Okta SAML env — happy to pair on triggering the idle-timeout there. Whatever's most convincing for you.

[thelullabyy]

Please add the recording to prove that it's working. We have to make sure that it doesn't break the current flow @trasnake87

Besides, how did you handle the Okta SAML login. Let's have a full recording for Okta SAML login in PR description not just the screenshot or short recording

[trasnake87]

@thelullabyy — quick update + a question so I capture the right thing.

I got the full Okta SAML login working end-to-end on my test env: native app → SAML → Okta auth → shortLivedAuthToken issued → reaches the inbox. I also confirmed the fix's guard (setIsAuthenticatingWithShortLivedToken) fires as SAMLSignInPage opens the browser, without breaking the login.

One snag on capturing a clean native-shell recording: on my test-env domain the SAML callback redirects to the web transition URL (new.expensify.com/transition?...&shortLivedAuthToken=…) instead of the native deep link (expensify://open) the app listens for — so it completes in the in-app browser rather than handing back to the native shell. Looks like a test-env RelayState/redirect-target thing on my side, not the PR.

Before I sink time into it: is there a preferred way to get the native deep-link return on a SAML test domain (a domain SAML config setting I might be missing), or would a recording of the login authenticating + completing to the inbox be enough for your check? Happy to do whichever gets you what you need.

[thelullabyy]

I am unable to do Okta SAML login. How did you do it before as you said you are able to reproduce the bug and login with Okta SAML @trasnake87 ?

[trasnake87]

@thelullabyy — right, that's the heart of this issue: there's no Okta SAML login available out of the box, which is why it's been hard to validate.

How I set mine up: I stood up my own Okta org (a free Okta trial) and wired it to an Expensify SAML domain I control (trasnakeexpensify.dev) — claimed the domain in Expensify, enabled SSO, pasted my Okta app's IdP metadata, and created a test user (tester@trasnakeexpensify.dev) in Okta. That lets me drive a real Okta SAML login against Expensify on demand and reproduce the resume-after-idle race.

Since you don't have an Okta env to test against, I'm happy to be the one to capture it — I'll record the native app doing the full Okta SAML login. One thing to confirm so I grab exactly what you need: on my test domain the login authenticates and reaches the inbox, but the callback completes in the in-app browser rather than handing back to the native shell (a RelayState/redirect quirk on my test setup, not the PR). Is a recording of the login authenticating + completing enough for your check, or is there a config tweak you'd recommend to get the clean native-shell return?

Also glad to walk you through the Okta setup if it'd help for validating future SAML issues.

[thelullabyy]

It looks like you submit the wrong evidences

Here is the reproduction steps:

Action Performed:
1. Log into Expensify mobile app via Okta SSO
2. Leave app idle for ~3–4 hours (aligned with Okta global session idle timeout)
3. Return to app
4. Complete Okta re-authentication + MFA in the in-app browser
5. Tap "Done" on in-app browser to return to the app

Expected Result:
User is successfully authenticated and returned to the app with a persisted session — no additional login required.

Actual Result:
App freezes briefly after returning from the in-app browser, then logs the user out and redirects to the Expensify login screen. User must log in a second time (including Okta MFA). The second login works as expected.

Workaround:
User can log in a second time — the second attempt always succeeds.

Based on your explanation here and evidence you submitted on PR, I see that it requires you to login again which is exact the bug. If it is the after changes result, it looks like your PR doesn't fix the issue. After the fix, it should not require second login - this is the correct behavior.

Please update recording accordingly @trasnake87

[trasnake87]

@thelullabyy — ah, I see the mix-up, and it's an important one: that recording is the bug reproduction (before the fix) — it deliberately shows the teardown and the forced second login to demonstrate that the bug exists. It's the "before," not the "after."

The whole point of the fix is exactly your expected result: after re-authenticating and tapping Done, the session persists with no second login. The fix sets the isAuthenticatingWithShortLivedToken guard before the in-app browser opens, so the resume-time reconnectApp 407 aborts re-auth instead of tearing the session down.

For an "after" recording: the catch is that driving your exact steps needs the native re-auth to hand back to the app shell, and on my SAML test domain the Okta callback completes in the in-app browser (the web transition URL) rather than returning via the deep link — so my env can't reach the "tap Done → back in the app" step to film it directly. The cleanest validation here is an AdHoc build run on a real production Okta env — the customer @johncschuster mentioned (who reported this) has that exact setup, and I'm glad to get the AdHoc build to them to confirm there's no second login. Alternatively, if there's a config tweak to get the native deep-link return working on a test domain, point me at it and I'll film it directly.

[thelullabyy]

@trasnake87 Sorry but if you can't recording, I think we will need to continue with Melvin's PR here, it already had the adhoc link and looks like @johncschuster can forward it to customer

@roryabraham just an update with you, we don't have proper way to verify the fix due to limitation of domain/deep link config, @trasnake87 cannot actually login with Okta SAML...

[roryabraham]

ok, let's close this and continue with Melvin's PR then

[trasnake87]

@roryabraham @thelullabyy — totally fair to go with whatever unblocks the customer fastest, and I'm glad it's getting tested on a real env.

One thing worth flagging though: the recording was blocked only by my SAML test env (the deep-link return doesn't fire on my test domain) — not by anything in this PR. #94396 is product-approved, CI-green, and uses the same isAuthenticatingWithShortLivedToken guard approach as #94082.

Since I'm the assigned contributor here, and the customer-AdHoc retest you're using applies equally to either PR, would you be open to reopening #94396 and pointing the customer's retest at its AdHoc build instead? Happy to do whatever's needed to get that build generated. If it's simpler to keep going with #94082, no worries at all — the priority is the customer getting unblocked.

[thelullabyy]

@trasnake87 Sorry but your fix is identical with Melvin. I think if you cannot run the test, Melvin's proposal which is the first proposal introduce the solution should be put in priority