regression: redirect to login when stored token is expired (#40870)

Author: ggazzo

apps/meteor/client/lib/sdk/ddpSdk.ts

@@ -151,22 +151,41 @@ export const ensureConnectedAndAuthenticated = async (): Promise<void> => {
 			// latter dispatches a `logout` method which itself races against
 			// parallel re-auth flows in CI's parallel-shard environment and
 			// kicked otherwise-healthy tests out.
-			removeStoredItem(STORAGE_KEYS.USER_ID);
-			removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN);
-			removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN_EXPIRES);
-			Meteor.connection.setUserId(null);
+			clearStoredCredentials();
 			return;
 		}
 		console.warn('[ddpSdk] loginWithToken failed', error);
 	}
 };
 
-const isAuthError = (error: unknown): boolean => {
+/**
+ * Drop the local session credentials without dispatching Meteor's `logout`
+ * method. Nulling the connection userId propagates through the
+ * Accounts.connection.userId() Tracker.autorun (see overrides/userAndUsers.ts)
+ * into the userIdStore, so `useUserId()` becomes undefined and the router falls
+ * through to LoginPage. We avoid `Meteor.logout()` on purpose: it dispatches a
+ * `logout` method that races parallel re-auth flows (fresh registration,
+ * Meteor's own resume) and has kicked otherwise-healthy sessions/tests out.
+ */
+export const clearStoredCredentials = (): void => {
+	removeStoredItem(STORAGE_KEYS.USER_ID);
+	removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN);
+	removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN_EXPIRES);
+	Meteor.connection.setUserId(null);
+};
+
+export const isAuthError = (error: unknown): boolean => {
 	if (!error || typeof error !== 'object') return false;
-	const e = error as { error?: unknown; reason?: unknown };
+	const e = error as { error?: unknown; reason?: unknown; status?: unknown; statusCode?: unknown };
 	return (
 		e.error === 401 ||
 		e.error === 403 ||
+		// REST-shaped failures (e.g. sdk.rest.get('/v1/me'), userData stream
+		// `nosub`) surface the HTTP status instead of a DDP `error` code.
+		e.status === 401 ||
+		e.status === 403 ||
+		e.statusCode === 401 ||
+		e.statusCode === 403 ||
 		e.reason === 'User not found' ||
 		e.reason === 'Login token expired' ||
 		e.reason === 'You are not allowed to use this token'

apps/meteor/client/meteor/overrides/ddpOverREST.ts

@@ -2,6 +2,7 @@ import { Meteor } from 'meteor/meteor';
 
 import { sdk } from '../../../app/utils/client/lib/SDKClient';
 import { parseDDP, stringifyDDP } from '../../lib/sdk/ddpProtocol';
+import { clearStoredCredentials } from '../../lib/sdk/ddpSdk';
 import { getUserId } from '../../lib/user';
 
 const bypassMethods: string[] = ['setUserStatus', 'logout'];
@@ -102,6 +103,21 @@ const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor
 			.catch((error: unknown) => {
 				const e = (error ?? {}) as { error?: unknown; reason?: unknown; message?: unknown };
 
+				// Check if it's a session expiration error and clear credentials.
+				const isExpiredSessionError =
+					(typeof e.error === 'string' && e.error === 'You must be logged in to do this.') ||
+					(typeof e.message === 'string' && e.message === 'You must be logged in to do this.') ||
+					(typeof e.reason === 'string' && e.reason === 'You must be logged in to do this.');
+
+				if (isExpiredSessionError) {
+					console.warn('[ddpOverREST] Expired session detected, clearing credentials', { method: message.method, error });
+					try {
+						clearStoredCredentials();
+					} catch (cleanupError) {
+						console.warn('[ddpOverREST] Failed to clean up expired session', cleanupError);
+					}
+				}
+
 				// method.call / method.callAnon encode the original Meteor error
 				// inside `body.message` as a DDP `result` frame (mountResult in
 				// app/api/server/v1/misc.ts). Forward it untouched so the original

apps/meteor/client/startup/startup.ts

@@ -3,8 +3,9 @@ import type { UserStatus } from '@rocket.chat/core-typings';
 import 'highlight.js/styles/github.css';
 import { sdk } from '../../app/utils/client/lib/SDKClient';
 import { onLoggedIn } from '../lib/loggedIn';
-import { ensureConnectedAndAuthenticated, getDdpSdk } from '../lib/sdk/ddpSdk';
+import { clearStoredCredentials, ensureConnectedAndAuthenticated, getDdpSdk, isAuthError } from '../lib/sdk/ddpSdk';
 import { isSdkTransportEnabled } from '../lib/sdk/sdkTransportEnabled';
+import { STORAGE_KEYS, getStoredItem } from '../lib/sdk/storage';
 import { userIdStore } from '../lib/user';
 import { removeLocalUserData, synchronizeUserData } from '../lib/userData';
 import { fireGlobalEvent } from '../lib/utils/fireGlobalEvent';
@@ -47,12 +48,33 @@ if (!sdkTransportEnabled) {
 		// of userIdStore, so without sequencing the sub races the auth and
 		// hits the rejection on every re-login. Await the SDK auth here so
 		// the sub fires authenticated.
+		const tokenBeforeSync = getStoredItem(STORAGE_KEYS.LOGIN_TOKEN);
 		try {
 			await ensureConnectedAndAuthenticated();
 		} catch {
 			// non-fatal: sdk.stream queues until DDPSDK eventually auths
 		}
-		const user = await synchronizeUserData(uid);
+
+		let user: Awaited<ReturnType<typeof synchronizeUserData>>;
+		try {
+			user = await synchronizeUserData(uid);
+		} catch (error) {
+			// When the stored token is expired/revoked server-side, the userData
+			// stream sub + /v1/me come back 401/403, so synchronizeUserData throws
+			// and useUserDataSyncReady never flips true. useMainReady then stays
+			// false forever (uid is still truthy from the stale credentials), so
+			// Preload sits on PageLoading — and because Preload WRAPS
+			// AuthenticationCheck, the user never reaches LoginPage and the
+			// workspace looks stuck "loading"/grayed-out. Escalate the dead-session
+			// signal to a credential wipe so userId drops to null and the router
+			// falls through to the login screen. Token-stable guard: only clear
+			// when localStorage still holds the token we synced with, so a parallel
+			// re-auth that already rotated the token isn't kicked out.
+			if (isAuthError(error) && getStoredItem(STORAGE_KEYS.LOGIN_TOKEN) === tokenBeforeSync) {
+				clearStoredCredentials();
+			}
+			throw error;
+		}
 		if (!user) return;
 
 		const utcOffset = -new Date().getTimezoneOffset() / 60;