refactor(client): use SDK storage helper in AuthenticationProvider.getLoginToken (#40482)

Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>

Author: ggazzo

apps/meteor/client/hooks/useReactiveValue.ts

@@ -1,4 +1,5 @@
 import type { IWithManageableCache } from './CachedStore';
+import { getDdpSdk } from '../sdk/ddpSdk';
 
 class CachedStoresManager {
 	private items = new Set<IWithManageableCache>();
@@ -16,6 +17,8 @@ class CachedStoresManager {
 
 const instance = new CachedStoresManager();
 
+getDdpSdk().account.onLogout(() => instance.clearAllCachesOnLogout());
+
 export {
 	/** @deprecated */
 	instance as CachedStoresManager,

apps/meteor/client/lib/cachedStores/CachedStoresManager.ts

@@ -5,8 +5,8 @@ import { Meteor } from 'meteor/meteor';
 
 import { createMeteorBackedSdk } from './meteorBackedSdk';
 import { isSdkTransportEnabled } from './sdkTransportEnabled';
-import { STORAGE_KEYS, getStoredItem } from './storage';
 import { getRootUrl } from '../meteorRuntimeConfig';
+import { STORAGE_KEYS, getStoredItem, removeStoredItem } from './storage';
 import { userIdStore } from '../user';
 
 const sdkTransportEnabled = isSdkTransportEnabled();
@@ -143,7 +143,9 @@ 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.
-			Accounts._unstoreLoginToken();
+			removeStoredItem(STORAGE_KEYS.USER_ID);
+			removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN);
+			removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN_EXPIRES);
 			Meteor.connection.setUserId(null);
 			return;
 		}

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

@@ -6,6 +6,7 @@
 export const STORAGE_KEYS = {
 	USER_ID: 'Meteor.userId',
 	LOGIN_TOKEN: 'Meteor.loginToken',
+	LOGIN_TOKEN_EXPIRES: 'Meteor.loginTokenExpires',
 	E2EE_PUBLIC_KEY: 'public_key',
 	E2EE_PRIVATE_KEY: 'private_key',
 	E2EE_RANDOM_PASSWORD: 'e2e.randomPassword',

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

@@ -7,5 +7,4 @@ import './desktopInjection';
 import './oauthRedirectUri';
 import './settings';
 import './totpOnCall';
-import './unstoreLoginToken';
 import './userAndUsers';

apps/meteor/client/meteor/overrides/index.ts

@@ -4,12 +4,13 @@ import { AuthenticationContext, useSetting } from '@rocket.chat/ui-contexts';
 import { Accounts } from 'meteor/accounts-base';
 import { Meteor } from 'meteor/meteor';
 import type { ContextType, ReactElement, ReactNode } from 'react';
-import { useMemo } from 'react';
+import { useMemo, useSyncExternalStore } from 'react';
 
 import { useLDAPAndCrowdCollisionWarning } from './hooks/useLDAPAndCrowdCollisionWarning';
 import { capitalize as capitalizeService } from '../../../lib/utils/stringUtils';
-import { useReactiveValue } from '../../hooks/useReactiveValue';
 import { loginServices } from '../../lib/loginServices';
+import { getDdpSdk } from '../../lib/sdk/ddpSdk';
+import { STORAGE_KEYS, getStoredItem, removeStoredItem } from '../../lib/sdk/storage';
 
 export type LoginMethods = keyof typeof Meteor extends infer T ? (T extends `loginWith${string}` ? T : never) : never;
 
@@ -27,7 +28,34 @@ const callLoginMethod = (
 	});
 };
 
-const getLoggingIn = () => Accounts.loggingIn();
+// Bridge Accounts.loggingIn() — Meteor's Tracker-reactive flag — into a
+// non-reactive subscribe/getSnapshot pair for useSyncExternalStore. We hook
+// `_setLoggingIn` (Meteor's internal flip, also accessed in
+// apps/meteor/client/meteor/overrides/killMeteorStream.ts) to fan out
+// transitions without entering a Tracker computation.
+const loggingInListeners = new Set<() => void>();
+let loggingInBridgeInstalled = false;
+const installLoggingInBridge = (): void => {
+	if (loggingInBridgeInstalled) return;
+	loggingInBridgeInstalled = true;
+	const wrap = Accounts as unknown as { _setLoggingIn?: (v: boolean) => void };
+	const original = wrap._setLoggingIn;
+	if (typeof original !== 'function') return;
+	wrap._setLoggingIn = function (this: typeof Accounts, v: boolean) {
+		original.call(this, v);
+		loggingInListeners.forEach((cb) => cb());
+	};
+};
+
+const subscribeLoggingIn = (cb: () => void): (() => void) => {
+	installLoggingInBridge();
+	loggingInListeners.add(cb);
+	return () => {
+		loggingInListeners.delete(cb);
+	};
+};
+
+const getLoggingInSnapshot = (): boolean => Accounts.loggingIn();
 
 const AuthenticationProvider = ({ children }: AuthenticationProviderProps): ReactElement => {
 	const isLdapEnabled = useSetting('LDAP_Enable', false);
@@ -37,7 +65,7 @@ const AuthenticationProvider = ({ children }: AuthenticationProviderProps): Reac
 
 	useLDAPAndCrowdCollisionWarning();
 
-	const isLoggingIn = useReactiveValue(getLoggingIn);
+	const isLoggingIn = useSyncExternalStore(subscribeLoggingIn, getLoggingInSnapshot);
 
 	const contextValue = useMemo(
 		(): ContextType<typeof AuthenticationContext> => ({
@@ -123,29 +151,18 @@ const AuthenticationProvider = ({ children }: AuthenticationProviderProps): Reac
 						resolve();
 					});
 				}),
-			getLoginToken: () => Accounts.storageLocation.getItem(Accounts.LOGIN_TOKEN_KEY) ?? null,
+			getLoginToken: () => getStoredItem(STORAGE_KEYS.LOGIN_TOKEN),
 			wipeLocalAuth: () => {
-				try {
-					Accounts._unstoreLoginToken();
-				} catch {
-					// ignore
-				}
+				removeStoredItem(STORAGE_KEYS.USER_ID);
+				removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN);
+				removeStoredItem(STORAGE_KEYS.LOGIN_TOKEN_EXPIRES);
 				try {
 					Meteor.connection.setUserId(null);
 				} catch {
 					// ignore
 				}
 			},
-			unstoreLoginToken: (callback) => {
-				const { _unstoreLoginToken } = Accounts;
-				Accounts._unstoreLoginToken = function (...args) {
-					callback();
-					_unstoreLoginToken.apply(Accounts, args);
-				};
-				return () => {
-					Accounts._unstoreLoginToken = _unstoreLoginToken;
-				};
-			},
+			unstoreLoginToken: (callback) => getDdpSdk().account.onLogout(callback),
 			queryLoginServices: {
 				getCurrentValue: () => loginServices.getLoginServiceButtons(),
 				subscribe: (onStoreChange: () => void) => loginServices.on('changed', onStoreChange),

apps/meteor/client/meteor/overrides/unstoreLoginToken.ts

@@ -11,13 +11,11 @@ import type { Method, PathFor, OperationParams, OperationResult, UrlParams, Path
 import type { UploadResult, ServerContextValue } from '@rocket.chat/ui-contexts';
 import { ServerContext } from '@rocket.chat/ui-contexts';
 import { Meteor } from 'meteor/meteor';
-import { Tracker } from 'meteor/tracker';
 import { compile } from 'path-to-regexp';
-import { useMemo, type ReactNode } from 'react';
+import { useMemo, useSyncExternalStore, type ReactNode } from 'react';
 
 import { sdk } from '../../app/utils/client/lib/SDKClient';
 import { Info as info } from '../../app/utils/rocketchat.info';
-import { useReactiveValue } from '../hooks/useReactiveValue';
 import { absoluteUrl } from '../lib/absoluteUrl';
 import { ensureConnectedAndAuthenticated, getDdpSdk } from '../lib/sdk/ddpSdk';
 import { isSdkTransportEnabled } from '../lib/sdk/sdkTransportEnabled';
@@ -99,16 +97,6 @@ const reconnect = sdkTransportEnabled
 		}
 	: () => Meteor.reconnect();
 
-// With SDK transport on, combine Meteor's DDP status with DDPSDK's so the
-// ConnectionStatusBar / idle-connection hooks reflect the worst-case of both
-// transports. With the flag off, route status straight through Meteor —
-// Meteor.status() is already Tracker-reactive, so adding a second dependency
-// via the meteor-backed proxy's emitter would just double-fire the autorun.
-const ddpSdkStatusDep = sdkTransportEnabled ? new Tracker.Dependency() : undefined;
-if (sdkTransportEnabled) {
-	getDdpSdk().connection.on('connection', () => ddpSdkStatusDep!.changed());
-}
-
 type CombinedStatus = ReturnType<typeof Meteor.status>;
 
 const sdkStatusToMeteor = (sdkStatus: string, meteor: CombinedStatus): CombinedStatus => {
@@ -132,21 +120,48 @@ const sdkStatusToMeteor = (sdkStatus: string, meteor: CombinedStatus): CombinedS
 	}
 };
 
-const getStatus = sdkTransportEnabled
-	? () => {
-			ddpSdkStatusDep!.depend();
-			return sdkStatusToMeteor(getDdpSdk().connection.status, Meteor.status());
-		}
-	: // useReactiveValue stores the snapshot in useSyncExternalStore, which
-		// compares by identity. Meteor.status() reuses the same internal object
-		// (mutated in place), so without spreading the snapshot reference never
-		// changes and ConnectionStatusBar stops re-rendering on connect/drop.
-		() => ({ ...Meteor.status() });
+// With SDK transport on, combine Meteor's DDP status with DDPSDK's so the
+// ConnectionStatusBar / idle-connection hooks reflect the worst-case of both
+// transports. With the flag off, route status straight through Meteor —
+// `meteorBackedSdk` already bridges Meteor's `_stream` events into
+// `sdk.connection.on('connection')`, so the same subscription works in both
+// modes.
+const computeStatus: () => CombinedStatus = sdkTransportEnabled
+	? () => sdkStatusToMeteor(getDdpSdk().connection.status, Meteor.status())
+	: () => ({ ...Meteor.status() });
+
+const isStatusEqual = (a: CombinedStatus, b: CombinedStatus): boolean =>
+	a.status === b.status && a.connected === b.connected && a.retryCount === b.retryCount && a.retryTime === b.retryTime;
+
+let cachedStatus: CombinedStatus = computeStatus();
+const statusListeners = new Set<() => void>();
+let statusBridgeStarted = false;
+
+const ensureStatusBridge = (): void => {
+	if (statusBridgeStarted) return;
+	statusBridgeStarted = true;
+	getDdpSdk().connection.on('connection', () => {
+		const next = computeStatus();
+		if (isStatusEqual(cachedStatus, next)) return;
+		cachedStatus = next;
+		statusListeners.forEach((cb) => cb());
+	});
+};
+
+const subscribeStatus = (cb: () => void): (() => void) => {
+	ensureStatusBridge();
+	statusListeners.add(cb);
+	return () => {
+		statusListeners.delete(cb);
+	};
+};
+
+const getStatusSnapshot = (): CombinedStatus => cachedStatus;
 
 type ServerProviderProps = { children?: ReactNode };
 
 const ServerProvider = ({ children }: ServerProviderProps) => {
-	const { connected, status, retryCount, retryTime } = useReactiveValue(getStatus);
+	const { connected, status, retryCount, retryTime } = useSyncExternalStore(subscribeStatus, getStatusSnapshot);
 
 	const value = useMemo(
 		(): ServerContextValue => ({