diff --git a/src/libs/actions/Session/index.ts b/src/libs/actions/Session/index.ts index 845ab1bfa1d2..f25c3eeea5af 100644 --- a/src/libs/actions/Session/index.ts +++ b/src/libs/actions/Session/index.ts @@ -847,6 +847,21 @@ function signInWithShortLivedAuthToken(authToken: string, isSAML = false) { NetworkStore.setLastShortAuthToken(authToken); } +/** + * Marks (or clears) that a short-lived-token sign-in is in progress. + * + * This should be set to `true` before opening the in-app browser for native SAML sign-in so the + * reauthentication middleware won't race against the SAML callback when the app resumes from the + * browser. On resume, `reconnectApp()` fires with the now-expired authToken and gets a 407; without + * this flag set, `reauthenticate()` takes the `isSAMLRequired` branch and calls `redirectToSignIn()`, + * wiping the session before the SAML callback can sign the user back in. `signInWithShortLivedAuthToken` + * resets this flag automatically once the sign-in succeeds, so the caller only needs to reset it if the + * browser is cancelled or fails. + */ +function setIsAuthenticatingWithShortLivedToken(isAuthenticating: boolean) { + Onyx.set(ONYXKEYS.RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN, isAuthenticating); +} + /** * Sign the user into the application. This will first authenticate their account * then it will create a temporary login for them which is used when re-authenticating @@ -1696,6 +1711,7 @@ export { signInWithValidateCodeAndNavigate, initAutoAuthState, signInWithShortLivedAuthToken, + setIsAuthenticatingWithShortLivedToken, cleanupSession, signOut, signOutAndRedirectToSignIn, diff --git a/src/pages/signin/SAMLSignInPage/index.native.tsx b/src/pages/signin/SAMLSignInPage/index.native.tsx index 695b11967311..aff91eebb9a0 100644 --- a/src/pages/signin/SAMLSignInPage/index.native.tsx +++ b/src/pages/signin/SAMLSignInPage/index.native.tsx @@ -11,7 +11,7 @@ import Log from '@libs/Log'; import {handleSAMLLoginError, postSAMLLogin} from '@libs/LoginUtils'; import Navigation from '@libs/Navigation/Navigation'; -import {clearSignInData, setAccountError, signInWithShortLivedAuthToken} from '@userActions/Session'; +import {clearSignInData, setAccountError, setIsAuthenticatingWithShortLivedToken, signInWithShortLivedAuthToken} from '@userActions/Session'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; @@ -32,6 +32,8 @@ function SAMLSignInPage() { const hasOpenedAuthSession = useRef(false); const handleExitSAMLFlow = useCallback(() => { + // Clear the guard we set before opening the in-app browser so we don't block future reauthentication + setIsAuthenticatingWithShortLivedToken(false); Navigation.isNavigationReady().then(() => { Navigation.goBack(); clearSignInData(); @@ -51,20 +53,19 @@ function SAMLSignInPage() { const searchParams = new URLSearchParams(new URL(url).search); const jsonParam = searchParams.get('json'); - if (!jsonParam) { - Log.hmmm('SAMLSignInPage - No JSON parameter found in callback URL'); - return; - } - let shortLivedAuthToken: string | null = null; - try { - const decodedData = JSON.parse(jsonParam) as Record; - shortLivedAuthToken = decodedData.shortLivedAuthToken ?? null; - if (decodedData.error) { - Log.hmmm('SAMLSignInPage - SAML login returned error', {error: decodedData.error}); + if (jsonParam) { + try { + const decodedData = JSON.parse(jsonParam) as Record; + shortLivedAuthToken = decodedData.shortLivedAuthToken ?? null; + if (decodedData.error) { + Log.hmmm('SAMLSignInPage - SAML login returned error', {error: decodedData.error}); + } + } catch (parseError) { + Log.hmmm('SAMLSignInPage - Failed to parse JSON parameter', {error: parseError}); } - } catch (parseError) { - Log.hmmm('SAMLSignInPage - Failed to parse JSON parameter', {error: parseError}); + } else { + Log.hmmm('SAMLSignInPage - No JSON parameter found in callback URL'); } if (!account?.isLoading && credentials?.login && shortLivedAuthToken) { @@ -73,6 +74,11 @@ function SAMLSignInPage() { return; } + // The browser returned but we couldn't sign in (no JSON parameter, a parse failure, or no token), so clear + // the guard we set before opening it and send the user back to a clean state. Otherwise the guard stays + // true, and since loginCallback URLs hide the back button and leave account.isLoading true, the user gets + // stuck on the loading screen with future reauthenticate() calls aborting. + setIsAuthenticatingWithShortLivedToken(false); clearSignInData(); setAccountError(translate('common.error.login')); Navigation.isNavigationReady().then(() => { @@ -90,6 +96,12 @@ function SAMLSignInPage() { return; } hasOpenedAuthSession.current = true; + // Opening the in-app browser backgrounds the app. When it returns, the app resumes and fires + // reconnectApp() with the expired authToken, which 407s and triggers reauthenticate() -> redirectToSignIn(), + // 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); openAuthSessionAsync(SAMLUrl, CONST.SAML_REDIRECT_URL) .then((response: WebBrowserAuthSessionResult) => { if (response.type !== 'success') { diff --git a/tests/actions/SessionTest.ts b/tests/actions/SessionTest.ts index 984a2ba9c7a2..8d1e3be4b7a5 100644 --- a/tests/actions/SessionTest.ts +++ b/tests/actions/SessionTest.ts @@ -105,6 +105,36 @@ describe('Session', () => { redirectToSignInSpy.mockRestore(); }); + test('setIsAuthenticatingWithShortLivedToken(true) makes reauthenticate abort (blocks the SAML resume race)', async () => { + let isAuthenticatingWithShortLivedToken: OnyxEntry; + Onyx.connect({ + key: ONYXKEYS.RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN, + callback: (val) => (isAuthenticatingWithShortLivedToken = val), + }); + + // Given the SAML sign-in flow set the guard before opening the in-app browser + SessionUtil.setIsAuthenticatingWithShortLivedToken(true); + await waitForBatchedUpdates(); + expect(isAuthenticatingWithShortLivedToken).toBe(true); + + const redirectToSignInSpy = jest.spyOn(SignInRedirect, 'default').mockImplementation(() => Promise.resolve()); + + // When the app resumes and reconnectApp's 407 triggers reauthenticate + const result = await reauthenticate('TestCommand'); + await waitForBatchedUpdates(); + + // Then reauthenticate aborts without redirecting to sign in, so the SAML callback can complete + expect(result).toBe(false); + expect(redirectToSignInSpy).not.toHaveBeenCalled(); + + // When the browser is cancelled/fails, the guard is cleared so future reauthentication isn't blocked + SessionUtil.setIsAuthenticatingWithShortLivedToken(false); + await waitForBatchedUpdates(); + expect(isAuthenticatingWithShortLivedToken).toBe(false); + + redirectToSignInSpy.mockRestore(); + }); + test('reauthenticate proceeds even when a legacy session.isAuthenticatingWithShortLivedToken=true is persisted (recovers stuck users)', async () => { // Given a session in Onyx that still carries the legacy stuck flag from before the RAM-only migration. // The Session type no longer declares the field, so cast to write the legacy shape.