Fix mobile SAML/Okta double login on session resume by MelvinBot · Pull Request #94082 · Expensify/App

MelvinBot · 2026-06-19T18:05:59Z

Explanation of Change

On mobile, when a SAML/Okta SSO user returns to the app after their IdP session has gone idle and completes re-authentication + MFA in the in-app browser, the app briefly freezes, logs them out, and forces a second login. The second login always succeeds.

Root cause — a race on app resume. Opening the in-app browser (openAuthSessionAsync) backgrounds the app. When the browser closes and the app resumes, two flows fire concurrently:

App-resume reconnection: reconnectApp() runs with the now-expired authToken, gets a 407 NOT_AUTHENTICATED, and triggers reauthenticate(). For SAML users it takes the account.isSAMLRequired branch and calls redirectToSignIn(undefined, true) → Onyx.clear(), wiping the session and cancelling in-flight requests.
SAML callback: the browser returns the new shortLivedAuthToken and calls signInWithShortLivedAuthToken().

reauthenticate() already has a guard that aborts when isAuthenticatingWithShortLivedToken is set — but that flag was only set optimistically inside signInWithShortLivedAuthToken(), which runs after the browser returns. So on resume the guard is still false, reauth wins the race, and the session is cleared before the SAML callback can land the user in. The second login works because the app is then in a clean state with no competing reconnection flow.

Fix. Set the isAuthenticatingWithShortLivedToken guard (the RAM-only key RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN) to true before opening the in-app browser, so the guard is active for the entire SAML browser session and reauthenticate() aborts instead of redirecting to sign-in. The guard is reset to false on the cancel, error, and callback-failure paths; on success, signInWithShortLivedAuthToken()'s optimistic/finally data takes over flag management as before.

The guard reliably aborts during the SAML flow because the page is only reached while account.isLoading is true (SignInPage only navigates to the SAML page when shouldInitiateSAMLLogin, which requires !!account.isLoading), and reconnectApp()/openApp() operate on the separate IS_LOADING_APP key, so resume does not clear account.isLoading. This satisfies the guard's account?.isLoading !== false condition in reauthenticate(). The new flag is a separate key from account.isLoading, so the SAML success check in SAMLSignInPage is unaffected.

Changes are native-only (SAMLSignInPage/index.native.tsx), matching where the bug occurs; desktop uses a different SAML implementation.

AI tests run locally by MelvinBot (informational — does not replace human Tests/QA)

npx prettier on changed files — pass (unchanged)
npm run lint-changed — pass
npm run typecheck (tsc) — pass
npm test -- tests/actions/SessionTest.ts — pass (32 tests, incl. new test asserting setIsAuthenticatingWithShortLivedToken(true) makes reauthenticate() abort)
npm test -- tests/ui/SessionTest.tsx — pass (2 tests)
npm run react-compiler-compliance-check check src/pages/signin/SAMLSignInPage/index.native.tsx — COMPILED

This change can only be fully validated on a device with a real Okta SAML session, which MelvinBot cannot operate.

Fixed Issues

$ #86705
PROPOSAL: #86705 (comment)

Tests

n/a - we'll have customer retest on staging.

Verify that no errors appear in the JS console

Offline tests

None.

QA Steps

None.

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

…owser Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com>

github-actions · 2026-06-19T19:00:10Z

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

github-actions · 2026-06-19T19:05:32Z

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

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

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

👀 View the workflow run that generated this build 👀

roryabraham · 2026-06-19T19:20:20Z

@johncschuster do you want to have your customer retest the saml bug on this AdHoc build? The only thing is they won't be able to install the AdHoc build on iOS, so if that's the only device they have they won't be able to test.

roryabraham · 2026-06-19T19:23:25Z

Only asking because you said "They would gladly QA this if we need someone to ensure it's working"

johncschuster · 2026-06-22T21:27:19Z

That would be great! Just so I'm clear, do I just need to send this URL to the end user?

roryabraham · 2026-06-26T06:45:21Z

ok, the customer was unsuccessful in installing the AdHoc build - no big deal. I think we should move this forward since we can't repro locally. Then we can have the customer retest on staging.

melvin-bot · 2026-06-26T06:46:54Z

@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: feb15e0ae1

ℹ️ 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 · 2026-06-26T06:50:20Z

+        // wiping the session before the SAML callback can sign the user back in. Setting this guard up front makes
+        // reauthenticate() abort while the SAML sign-in is in progress. signInWithShortLivedAuthToken() resets it on
+        // success; the cancel/error/failure paths reset it via handleExitSAMLFlow and handleNavigationStateChange.
+        setIsAuthenticatingWithShortLivedToken(true);


Clear SAML guard for malformed callbacks

When the in-app browser returns a success URL without the expected json parameter, handleNavigationStateChange() returns before reaching the new cleanup path, so the guard set here remains true. This function already treats missing json as a possible callback shape, and for loginCallback URLs the back button is hidden; while account.isLoading remains true, future reauthenticate() calls will keep aborting instead of recovering or redirecting. Please clear the guard on that early-return path as well.

Useful? React with 👍 / 👎.

@MelvinBot Please check this

MelvinBot · 2026-06-26T06:55:19Z

Review: the fix is sound ✅

I traced the full data flow and the core reasoning in the PR holds. Setting the RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN guard before openAuthSessionAsync() correctly makes reauthenticate() abort on app-resume instead of wiping the session via redirectToSignIn().

Verified claims:

The guard aborts and skips redirect. The check is in Reauthentication.ts:138-159 (not Session/index.ts as the PR body implies). It returns Promise.resolve(false) before the isSAMLRequired → redirectToSignIn() branch, so the session is preserved. ✅
account.isLoading stays truthy throughout. SignInPage.tsx:103 only navigates to the SAML page when !!account.isLoading, and reconnectApp()/openApp() flip the separate IS_LOADING_APP key — so the guard's inner account?.isLoading !== false condition is satisfied during resume. ✅
Flag auto-resets on success. On success the page calls signInWithShortLivedAuthToken(), whose getShortLivedLoginParams resets the flag via finallyData (Session/index.ts:203-207) — note this is finallyData, not "successData", so it covers the API-failure path too. ✅
RAM-only key, won't persist. Registered in ramOnlyKeys in setup/index.ts:68, and there's a self-healing stale-state path in reauthenticate() that clears it if it's ever stuck while account.isLoading === false. ✅

One minor edge case worth a one-line fix (non-blocking):

In handleNavigationStateChange, the "no json parameter" early return at SAMLSignInPage/index.native.tsx:50-53 logs and returns without resetting the guard. In practice openAuthSessionAsync only resolves success with the final redirect URL (which carries json), so this is unlikely to fire on a real callback — and the stale-state path in reauthenticate() would eventually self-heal it, so it's not a permanent lock. But it's the one post-openAuthSessionAsync branch that doesn't explicitly reset the flag. Adding setIsAuthenticatingWithShortLivedToken(false) there would close the gap for completeness.

The unit test in tests/actions/SessionTest.ts correctly asserts the guard makes reauthenticate() abort without redirecting. The change is appropriately scoped to native-only.

Net: the approach is correct and safe to move forward; the edge case above is optional polish.

thelullabyy · 2026-06-26T14:52:14Z

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

thelullabyy · 2026-06-27T04:33:29Z

+        // wiping the session before the SAML callback can sign the user back in. Setting this guard up front makes
+        // reauthenticate() abort while the SAML sign-in is in progress. signInWithShortLivedAuthToken() resets it on
+        // success; the cancel/error/failure paths reset it via handleExitSAMLFlow and handleNavigationStateChange.
+        setIsAuthenticatingWithShortLivedToken(true);


When the in-app browser returns a success URL without the expected json parameter, handleNavigationStateChange() returns before reaching the new cleanup path, so the guard set here remains true. This function already treats missing json as a possible callback shape, and for loginCallback URLs the back button is hidden; while account.isLoading remains true, future reauthenticate() calls will keep aborting instead of recovering or redirecting. Please clear the guard on that early-return path as well.

@MelvinBot What do you think?

thelullabyy · 2026-06-30T05:55:08Z

@roryabraham he still doesnt listen to me, could you please try to ask him?

roryabraham · 2026-06-30T16:32:13Z

When the in-app browser returns a success URL without the expected json parameter, handleNavigationStateChange() returns before reaching the new cleanup path, so the guard set here remains true. This function already treats missing json as a possible callback shape, and for loginCallback URLs the back button is hidden; while account.isLoading remains true, future reauthenticate() calls will keep aborting instead of recovering or redirecting. Please clear the guard on that early-return path as well.

@MelvinBot What do you think?

thelullabyy · 2026-07-01T00:58:31Z

@roryabraham I think I will ask Melvin to open another PR. It should fix the problem

thelullabyy · 2026-07-01T01:43:53Z

I think we can close this PR as favor of #95017

roryabraham · 2026-07-01T02:00:52Z

no problem

…arGuard Clear SAML short-lived-token guard on the no-JSON early-return path (based on #94082)

Fix mobile SAML/Okta double login by guarding reauth during in-app br…

feb15e0

…owser Co-authored-by: Rory Abraham <roryabraham@users.noreply.github.com>

MelvinBot assigned roryabraham Jun 19, 2026

MelvinBot requested a review from a team June 19, 2026 18:06

MelvinBot mentioned this pull request Jun 19, 2026

[Due for payment 2026-07-21] [$500] [Mobile] Okta SSO requires double login after idle timeout — app freezes on SAML session handoff #86705

Open

6 tasks

thelullabyy mentioned this pull request Jun 26, 2026

Fix Okta SAML double login on native session resume #94396

Closed

43 tasks

roryabraham marked this pull request as ready for review June 26, 2026 06:46

roryabraham requested a review from a team as a code owner June 26, 2026 06:46

melvin-bot Bot requested review from heyjennahay and thelullabyy and removed request for a team June 26, 2026 06:46

melvin-bot Bot removed the request for review from a team June 26, 2026 06:46

chatgpt-codex-connector Bot reviewed Jun 26, 2026

View reviewed changes

thelullabyy reviewed Jun 27, 2026

View reviewed changes

This comment was marked as off-topic.

Sign in to view

roryabraham assigned thelullabyy Jun 30, 2026

This comment was marked as outdated.

Sign in to view

This comment was marked as duplicate.

Sign in to view

roryabraham unassigned thelullabyy Jun 30, 2026

MelvinBot mentioned this pull request Jul 1, 2026

Clear SAML short-lived-token guard on the no-JSON early-return path (based on #94082) #95017

Merged

43 tasks

roryabraham closed this Jul 1, 2026

grgia added a commit that referenced this pull request Jul 10, 2026

Merge pull request #95017 from Expensify/claude-samlSsoDoubleLoginCle…

b980be6

…arGuard Clear SAML short-lived-token guard on the no-JSON early-return path (based on #94082)

Uh oh!

Conversation

MelvinBot commented Jun 19, 2026 • edited by roryabraham 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!

github-actions Bot commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

roryabraham commented Jun 19, 2026

Uh oh!

roryabraham commented Jun 19, 2026

Uh oh!

johncschuster commented Jun 22, 2026

Uh oh!

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

Uh oh!

Uh oh!

melvin-bot Bot commented Jun 26, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

thelullabyy Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Jun 26, 2026

Review: the fix is sound ✅

Uh oh!

thelullabyy commented Jun 26, 2026

Reviewer Checklist

Screenshots/Videos

Uh oh!

thelullabyy Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as outdated.

This comment was marked as duplicate.

thelullabyy commented Jun 30, 2026

Uh oh!

This comment was marked as duplicate.

roryabraham commented Jun 30, 2026

Uh oh!

thelullabyy commented Jul 1, 2026

Uh oh!

thelullabyy commented Jul 1, 2026

Uh oh!

roryabraham commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MelvinBot commented Jun 19, 2026 •

edited by roryabraham

Loading

roryabraham commented Jun 26, 2026 •

edited

Loading