fix: sign out user when session expires

src/libs/Middleware/Reauthentication.ts

@@ -1,5 +1,7 @@
+import type {OnyxKey} from 'react-native-onyx';
 import {reconnect} from '@libs/actions/Reconnect';
 import redirectToSignIn from '@libs/actions/SignInRedirect';
+import HttpsError from '@libs/Errors/HttpsError';
 import Log from '@libs/Log';
 import {replay as replayMainQueue} from '@libs/Network/MainQueue';
 import {isAuthenticating as isAuthenticatingNetworkStore, setIsAuthenticating} from '@libs/Network/NetworkStore';
@@ -9,6 +11,9 @@ import reauthenticateLibs from '@libs/Reauthentication';
 import {processWithMiddleware} from '@libs/Request';
 import RequestThrottle from '@libs/RequestThrottle';
 import CONST from '@src/CONST';
+import type Request from '@src/types/onyx/Request';
+import type {PaginatedRequest} from '@src/types/onyx/Request';
+import type Response from '@src/types/onyx/Response';
 import type Middleware from './types';
 
 // We store a reference to the active authentication request so that we are only ever making one request to authenticate at a time.
@@ -52,6 +57,86 @@ function resetReauthentication(): void {
     reauthThrottle.clear();
 }
 
+function isExpiredSessionError(error: unknown): error is HttpsError {
+    return error instanceof HttpsError && Number(error.status) === CONST.JSON_CODE.NOT_AUTHENTICATED;
+}
+
+// Preserve hard HTTP failures from Authenticate as response-shaped data so the
+// auth flow can map them to the right sign-in error instead of retrying them
+// like transient transport failures.
+function shouldResolveAuthenticateHTTPError(error: unknown, request: {command: string}): error is HttpsError {
+    return request.command === 'Authenticate' && error instanceof HttpsError && !!error.status && error.message !== CONST.ERROR.EXPENSIFY_SERVICE_INTERRUPTED;
+}
+
+function handleExpiredSession<TKey extends OnyxKey>(
+    request: Request<TKey> | PaginatedRequest<TKey>,
+    isFromSequentialQueue: boolean,
+    data: Response<TKey> = {jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED} as Response<TKey>,
+): Promise<Response<TKey> | void> {
+    if (getIsOffline()) {
+        // Both body-level and HTTP-level 407 responses should honor the existing
+        // offline pause so flaky connectivity does not trigger sign-out retries.
+        throw new Error('Unable to reauthenticate because we are offline');
+    }
+
+    // There are some API requests that should not be retried when there is an auth failure like
+    // creating and deleting logins. In those cases, they should handle the original response instead
+    // of the new response created by handleExpiredAuthToken.
+    const shouldRetry = request?.data?.shouldRetry;
+    const apiRequestType = request?.data?.apiRequestType;
+
+    // For the SignInWithShortLivedAuthToken command, if the short token expires, the server returns a 407 error,
+    // and credentials are still empty at this time, which causes reauthenticate to throw an error (requireParameters),
+    // and the subsequent SaveResponseInOnyx also cannot be executed, so we need this parameter to skip the reauthentication logic.
+    const skipReauthentication = request?.data?.skipReauthentication;
+    if ((!shouldRetry && !apiRequestType) || skipReauthentication) {
+        if (isFromSequentialQueue) {
+            return Promise.resolve(data);
+        }
+
+        if (request.resolve) {
+            request.resolve(data);
+        }
+        return Promise.resolve(data);
+    }
+
+    // We are already authenticating and using the DeprecatedAPI so we will replay the request
+    if (!apiRequestType && isAuthenticatingNetworkStore()) {
+        replayMainQueue(request);
+        return Promise.resolve(data);
+    }
+
+    return reauthenticate(request?.commandName)
+        .then((wasSuccessful) => {
+            if (!wasSuccessful) {
+                // Reauth already handled the sign-in redirect, so do not briefly show the failed request UI before sign-in.
+                request.failureData = undefined;
+                request.finallyData = undefined;
+                return data;
+            }
+
+            if (isFromSequentialQueue || apiRequestType === CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {
+                return processWithMiddleware(request, isFromSequentialQueue);
+            }
+
+            if (apiRequestType === CONST.API_REQUEST_TYPE.READ) {
+                // Re-sync app data after successful re-authentication
+                reconnect();
+                return Promise.resolve();
+            }
+
+            replayMainQueue(request);
+        })
+        .catch(() => {
+            if (isFromSequentialQueue || apiRequestType) {
+                throw new Error('Failed to reauthenticate');
+            }
+
+            // If we make it here, then our reauthenticate request could not be made due to a networking issue. The original request can be retried safely.
+            replayMainQueue(request);
+        });
+}
+
 const Reauthentication: Middleware = (response, request, isFromSequentialQueue) =>
     response
         .then((data) => {
@@ -62,64 +147,7 @@ const Reauthentication: Middleware = (response, request, isFromSequentialQueue)
             }
 
             if (data.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) {
-                if (getIsOffline()) {
-                    // If we are offline and somehow handling this response we do not want to reauthenticate
-                    throw new Error('Unable to reauthenticate because we are offline');
-                }
-
-                // There are some API requests that should not be retried when there is an auth failure like
-                // creating and deleting logins. In those cases, they should handle the original response instead
-                // of the new response created by handleExpiredAuthToken.
-                const shouldRetry = request?.data?.shouldRetry;
-                const apiRequestType = request?.data?.apiRequestType;
-
-                // For the SignInWithShortLivedAuthToken command, if the short token expires, the server returns a 407 error,
-                // and credentials are still empty at this time, which causes reauthenticate to throw an error (requireParameters),
-                // and the subsequent SaveResponseInOnyx also cannot be executed, so we need this parameter to skip the reauthentication logic.
-                const skipReauthentication = request?.data?.skipReauthentication;
-                if ((!shouldRetry && !apiRequestType) || skipReauthentication) {
-                    if (isFromSequentialQueue) {
-                        return data;
-                    }
-
-                    if (request.resolve) {
-                        request.resolve(data);
-                    }
-                    return data;
-                }
-
-                // We are already authenticating and using the DeprecatedAPI so we will replay the request
-                if (!apiRequestType && isAuthenticatingNetworkStore()) {
-                    replayMainQueue(request);
-                    return data;
-                }
-
-                return reauthenticate(request?.commandName)
-                    .then((wasSuccessful) => {
-                        if (!wasSuccessful) {
-                            return;
-                        }
-
-                        if (isFromSequentialQueue || apiRequestType === CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {
-                            return processWithMiddleware(request, isFromSequentialQueue);
-                        }
-
-                        if (apiRequestType === CONST.API_REQUEST_TYPE.READ) {
-                            // Re-sync app data after successful re-authentication
-                            reconnect();
-                            return Promise.resolve();
-                        }
-
-                        replayMainQueue(request);
-                    })
-                    .catch(() => {
-                        if (isFromSequentialQueue || apiRequestType) {
-                            throw new Error('Failed to reauthenticate');
-                        }
-
-                        // If we make it here, then our reauthenticate request could not be made due to a networking issue. The original request can be retried safely.
-                        replayMainQueue(request);
-                    });
+                return handleExpiredSession(request, isFromSequentialQueue, data);
             }
 
             if (isFromSequentialQueue) {
@@ -134,6 +162,31 @@ const Reauthentication: Middleware = (response, request, isFromSequentialQueue)
             return data;
         })
         .catch((error) => {
+            if (isExpiredSessionError(error)) {
+                return Promise.resolve()
+                    .then(() => handleExpiredSession(request, isFromSequentialQueue))
+                    .catch((reauthenticationError) => {
+                        if (isFromSequentialQueue) {
+                            throw reauthenticationError;
+                        }
+
+                        if (request.resolve) {
+                            request.resolve({jsonCode: CONST.JSON_CODE.UNABLE_TO_RETRY});
+                        }
+                    });
+            }
+
+            if (shouldResolveAuthenticateHTTPError(error, request)) {
+                if (request.resolve) {
+                    request.resolve({
+                        jsonCode: Number(error.status),
+                        message: error.message,
+                        title: error.title,
+                    });
+                }
+                return;
+            }
+
             // If the request is on the sequential queue, re-throw the error so we can decide to retry or not
             if (isFromSequentialQueue) {
                 throw error;

src/libs/Reauthentication.ts

@@ -7,9 +7,11 @@ import ONYXKEYS from '@src/ONYXKEYS';
 import type {Account} from '@src/types/onyx';
 import type Response from '@src/types/onyx/Response';
 import {isConnectedAsDelegate, restoreDelegateSession} from './actions/Delegate';
+import clearShortLivedAuthState from './actions/Session/clearShortLivedAuthState';
 import updateSessionAuthTokens from './actions/Session/updateSessionAuthTokens';
 import redirectToSignIn from './actions/SignInRedirect';
-import {getAuthenticateErrorMessage} from './ErrorUtils';
+import HttpsError from './Errors/HttpsError';
+import {getAuthenticateErrorMessage, getErrorMessage} from './ErrorUtils';
 import Log from './Log';
 import {post} from './Network';
 import {getCredentials, hasReadRequiredDataFromStorage, setAuthToken, setIsAuthenticating} from './Network/NetworkStore';
@@ -69,8 +71,9 @@ function Authenticate<TKey extends OnyxKey>(parameters: Parameters): Promise<Res
     try {
         requireParameters(['partnerName', 'partnerPassword', 'partnerUserID', 'partnerUserSecret'], parameters, commandName);
     } catch (error) {
-        const errorMessage = (error as Error).message;
-        trackAuthenticationError(error as Error, {
+        const errorMessage = getErrorMessage(error);
+        // Caught values can be non-Error objects, but telemetry expects an Error instance.
+        trackAuthenticationError(error instanceof Error ? error : new Error(errorMessage), {
             errorType: 'missing_params',
             functionName: 'Authenticate',
             commandName,
@@ -104,6 +107,24 @@ function Authenticate<TKey extends OnyxKey>(parameters: Parameters): Promise<Res
     });
 }
 
+function shouldRetryAuthenticateError(error: unknown): boolean {
+    if (!(error instanceof HttpsError)) {
+        return true;
+    }
+
+    // Only retry transient connectivity/service issues. Real HTTP auth failures,
+    // and auth throttling, should fall through to the normal sign-out path so we
+    // do not spin on Authenticate before redirecting to sign in.
+    return error.message === CONST.ERROR.FAILED_TO_FETCH || error.message === CONST.ERROR.EXPENSIFY_SERVICE_INTERRUPTED;
+}
+
+function getAuthenticationErrorResponse(error: HttpsError): Response<OnyxKey> {
+    return {
+        jsonCode: Number(error.status),
+        message: error.message,
+    };
+}
+
 /**
  * Reauthenticate using the stored credentials and redirect to the sign in page if unable to do so.
  * @param [command] command name for logging purposes
@@ -116,11 +137,25 @@ function reauthenticate(command = ''): Promise<boolean> {
 
     // Prevent re-authentication if authentication with shortLiveToken is in progress
     if (isAuthenticatingWithShortLivedToken) {
-        Log.hmmm('[Reauthenticate] Authentication with shortLivedToken is in progress. Re-authentication aborted.', {
+        // Only treat the short-lived auth state as stale once account loading has
+        // explicitly finished. Until then, keep aborting reauth to avoid interrupting
+        // a valid short-lived-token login during startup hydration races.
+        if (account?.isLoading !== false) {
+            Log.hmmm('[Reauthenticate] Authentication with shortLivedToken is in progress. Re-authentication aborted.', {
+                command,
+                isSupportAuthTokenUsed,
+                accountIsLoading: account?.isLoading,
+            });
+            return Promise.resolve(false);
+        }
+
+        Log.alert('[Reauthenticate] Found stale shortLivedToken authentication state. Clearing it before re-authenticating.', {
             command,
             isSupportAuthTokenUsed,
         });
-        return Promise.resolve(false);
+        isAuthenticatingWithShortLivedToken = false;
+        isSupportAuthTokenUsed = false;
+        clearShortLivedAuthState();
     }
 
     // Prevent any more requests from being processed while authentication happens
@@ -233,14 +268,38 @@ function reauthenticate(command = ''): Promise<boolean> {
                 return true;
             })
             .catch((error) => {
-                trackAuthenticationError(error as Error, {
+                if (error instanceof HttpsError && !shouldRetryAuthenticateError(error)) {
+                    // Reuse the standard Authenticate response handling so HTTP
+                    // failures and body-level failures produce the same UX.
+                    const authenticationErrorResponse = getAuthenticationErrorResponse(error);
+                    const errorMessage = getAuthenticateErrorMessage(authenticationErrorResponse);
+
+                    trackAuthenticationError(error, {
+                        errorType: 'auth_failure',
+                        functionName: 'reauthenticate',
+                        jsonCode: Number(error.status),
+                        command,
+                        errorMessage,
+                    });
+                    setIsAuthenticating(false);
+                    Log.hmmm('[Reauthenticate] Redirecting to Sign In because Authenticate returned a non-retryable HTTP error', {
+                        command,
+                        error: error.message,
+                        status: error.status,
+                    });
+                    redirectToSignIn(errorMessage);
+                    return false;
+                }
+
+                // Caught values can be non-Error objects, but telemetry expects an Error instance.
+                trackAuthenticationError(error instanceof Error ? error : new Error(getErrorMessage(error)), {
                     errorType: 'unexpected_error',
                     functionName: 'reauthenticate',
                     command,
                 });
 
                 Log.alert('[Reauthenticate] Unexpected error during authentication', {
-                    error: (error as Error).message,
+                    error: getErrorMessage(error),
                     command,
                 });
                 throw error;

src/libs/actions/Session/clearShortLivedAuthState.ts

@@ -0,0 +1,12 @@
+import Onyx from 'react-native-onyx';
+import ONYXKEYS from '@src/ONYXKEYS';
+
+export default function clearShortLivedAuthState() {
+    return Promise.all([
+        Onyx.merge(ONYXKEYS.SESSION, {
+            signedInWithShortLivedAuthToken: null,
+            isSupportAuthTokenUsed: null,
+        }),
+        Onyx.set(ONYXKEYS.RAM_ONLY_IS_AUTHENTICATING_WITH_SHORT_LIVED_TOKEN, false),
+    ]).then(() => undefined);
+}