fix: make sdk.current nullable and guard its call sites

app/definitions/rest/v1/push.ts

@@ -14,6 +14,7 @@ export type PushEndpoints = {
 				userId: string;
 			};
 		};
+		DELETE: (params: { token: string }) => { success: boolean };
 	};
 	'push.info': {
 		GET: () => TPushInfo;

app/lib/methods/actions.test.ts

@@ -21,15 +21,11 @@ jest.mock('../navigation/appNavigation', () => ({
 jest.mock('../services/sdk', () => ({
 	__esModule: true,
 	default: {
-		current: {
-			currentLogin: {
-				userId: 'user-id',
-				authToken: 'auth-token'
-			},
-			client: {
-				host: 'https://chat.example.com'
-			}
-		}
+		currentLogin: {
+			userId: 'user-id',
+			authToken: 'auth-token'
+		},
+		host: 'https://chat.example.com'
 	}
 }));
 

app/lib/methods/actions.ts

@@ -108,12 +108,11 @@ export async function triggerAction({
 	const payload = rest.payload ?? rest.value;
 
 	try {
-		const { currentLogin } = sdk.current;
-		if (!currentLogin) {
-			throw new Error('triggerAction requires an authenticated session');
+		const { host, currentLogin } = sdk;
+		if (!host || !currentLogin) {
+			throw new Error('triggerAction requires an initialized, authenticated session');
 		}
 		const { userId, authToken } = currentLogin;
-		const { host } = sdk.current.client;
 		const interaction = toUserInteraction({
 			type,
 			actionId,

app/lib/methods/getSettings.ts

@@ -149,7 +149,7 @@ export async function subscribeSettings(): Promise<void> {
 
 type IData = ISettingsIcon | IPreparedSettings;
 
-export async function getSettings(): Promise<void> {
+export async function getSettings(server: string): Promise<void> {
 	try {
 		const db = database.active;
 		const settingsParams = Object.keys(defaultSettings).filter(key => !loginSettings.includes(key));
@@ -159,8 +159,8 @@ export async function getSettings(): Promise<void> {
 		let settings: IData[] = [];
 		const serverVersion = reduxStore.getState().server.version;
 		const url = compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.0.0')
-			? `${sdk.current.client.host}/api/v1/settings.public?_id=${settingsParams.join(',')}`
-			: `${sdk.current.client.host}/api/v1/settings.public?query={"_id":{"$in":${JSON.stringify(settingsParams)}}}`;
+			? `${server}/api/v1/settings.public?_id=${settingsParams.join(',')}`
+			: `${server}/api/v1/settings.public?query={"_id":{"$in":${JSON.stringify(settingsParams)}}}`;
 		// Iterate over paginated results to retrieve all settings
 		do {
 			// TODO: why is no-await-in-loop enforced in the first place?

app/lib/methods/logout.test.ts

@@ -1,3 +1,5 @@
+import type * as SdkIntegration from '../testUtils/sdkIntegration';
+
 jest.mock('../database', () => ({
 	__esModule: true,
 	default: {
@@ -28,7 +30,16 @@ jest.mock('../services/restApi', () => ({
 	removePushToken: jest.fn()
 }));
 
-import { removeServerData } from './logout';
+const mockSdkLogout = jest.fn();
+
+jest.mock('../services/sdk', () => {
+	const { makeSdkMock } = jest.requireActual<typeof SdkIntegration>('../testUtils/sdkIntegration');
+	return { __esModule: true, default: makeSdkMock({ logout: () => mockSdkLogout() }) };
+});
+
+import { logout, removeServerData } from './logout';
+import sdk from '../services/sdk';
+import { disconnect } from '../services/connect';
 import database from '../database';
 import UserPreferences from './userPreferences';
 import { BASIC_AUTH_KEY } from './helpers/fetch';
@@ -41,6 +52,8 @@ import {
 	TOKEN_KEY
 } from '../constants/keys';
 
+const mockSdk = sdk as unknown as SdkIntegration.IMockSdk;
+
 const SERVER = 'https://a.rocket.chat';
 const OTHER_SERVER = 'https://b.rocket.chat';
 const USER_ID = 'user-a';
@@ -136,3 +149,42 @@ describe('removeServerData', () => {
 		serverKeys(SERVER).forEach(key => expect(UserPreferences.getString(key)).toBeNull());
 	});
 });
+
+describe('logout', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+		keysToClear.forEach(key => UserPreferences.removeItem(key));
+		mockDestroyableServerRecord();
+		mockSdk.setClient(null);
+	});
+
+	it('skips the server-side logout when there is no client', async () => {
+		seedServer(SERVER, USER_ID);
+
+		await logout({ server: SERVER });
+
+		expect(mockSdkLogout).not.toHaveBeenCalled();
+		expect(disconnect).not.toHaveBeenCalled();
+	});
+
+	it('clears the local logout state when there is no client', async () => {
+		seedServer(SERVER, USER_ID);
+		UserPreferences.setString(CURRENT_SERVER, SERVER);
+
+		await logout({ server: SERVER });
+
+		expect(UserPreferences.getString(CURRENT_SERVER)).toBeNull();
+		expect(UserPreferences.getString(tokenKey(SERVER))).toBeNull();
+		serverKeys(SERVER).forEach(key => expect(UserPreferences.getString(key)).toBeNull());
+	});
+
+	it('calls the server-side logout when a client exists', async () => {
+		seedServer(SERVER, USER_ID);
+		mockSdk.setClient({ host: SERVER });
+
+		await logout({ server: SERVER });
+
+		expect(mockSdkLogout).toHaveBeenCalled();
+		expect(disconnect).toHaveBeenCalled();
+	});
+});

app/lib/methods/logout.ts

@@ -106,14 +106,13 @@ export async function logout({ server }: { server: string }): Promise<void> {
 		log(e);
 	}
 
-	try {
-		// RC 0.60.0
-		await sdk.current.logout();
-	} catch (e) {
-		log(e);
-	}
-
-	if (sdk.current) {
+	if (sdk.isInitialized) {
+		try {
+			// RC 0.60.0
+			await sdk.logout();
+		} catch (e) {
+			log(e);
+		}
 		disconnect();
 	}
 

app/lib/methods/subscriptions/__tests__/roomSubscription.integration.test.ts

@@ -130,7 +130,7 @@ afterEach(() => {
 
 async function connectDriver() {
 	sdk.initialize('https://example.com');
-	const connectPromise = (sdk.current as unknown as { connect(): Promise<unknown> }).connect();
+	const connectPromise = sdk.connect();
 	await flush();
 	mockConnections[0].onopen();
 	await flush();

app/lib/methods/subscriptions/__tests__/rooms.hostGuard.test.ts

@@ -0,0 +1,92 @@
+const mockOnStreamData = jest.fn(async (_event: string, _callback: (message: IDDPMessage) => void) => ({ stop: jest.fn() }));
+const mockSubscribeNotifyUser = jest.fn(async () => undefined);
+
+jest.mock('../../../services/sdk', () => {
+	const { makeSdkMock } = jest.requireActual<typeof SdkIntegration>('../../../testUtils/sdkIntegration');
+	return {
+		__esModule: true,
+		default: makeSdkMock({
+			onStreamData: (...args: Parameters<typeof mockOnStreamData>) => mockOnStreamData(...args),
+			subscribeNotifyUser: () => mockSubscribeNotifyUser()
+		})
+	};
+});
+
+jest.mock('../../../database', () => ({
+	__esModule: true,
+	default: { active: { get: jest.fn(), write: jest.fn(), batch: jest.fn() } }
+}));
+
+jest.mock('../../../store/auxStore', () => ({
+	store: { dispatch: jest.fn(), getState: jest.fn(() => ({ settings: {}, login: { user: {} } })) }
+}));
+
+jest.mock('../../helpers/log', () => ({ __esModule: true, default: jest.fn() }));
+
+import subscribeRooms, { roomsSubscription } from '../rooms';
+import sdk from '../../../services/sdk';
+import database from '../../../database';
+import type { IDDPMessage } from '../../../../definitions/IDDPMessage';
+import type * as SdkIntegration from '../../../testUtils/sdkIntegration';
+
+const mockedSdk = sdk as unknown as SdkIntegration.IMockSdk;
+const mockedDatabase = database as unknown as { active: { get: jest.Mock } };
+
+const HOST = 'https://open.rocket.chat';
+
+const removedSubscriptionFrame = (): IDDPMessage =>
+	({
+		msg: 'changed',
+		collection: 'stream-notify-user',
+		id: 'id',
+		fields: {
+			eventName: 'userId/subscriptions-changed',
+			args: ['removed', { rid: 'rid' }]
+		}
+	}) as unknown as IDDPMessage;
+
+describe('subscribeRooms host guard', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+		mockedSdk.setClient(null);
+	});
+
+	it('does not open the stream when there is no client', () => {
+		subscribeRooms();
+
+		expect(mockOnStreamData).not.toHaveBeenCalled();
+		expect(mockSubscribeNotifyUser).not.toHaveBeenCalled();
+	});
+
+	it('drops a frame that arrives after the client is gone', async () => {
+		mockedSdk.setClient({ host: HOST });
+		subscribeRooms();
+
+		const [, handleStreamMessageReceived] = mockOnStreamData.mock.calls[0];
+		mockedSdk.setClient(null);
+		await handleStreamMessageReceived(removedSubscriptionFrame());
+
+		expect(mockedDatabase.active.get).not.toHaveBeenCalled();
+	});
+
+	it('drops a frame that arrives after the subscription stopped', async () => {
+		mockedSdk.setClient({ host: HOST });
+		subscribeRooms();
+
+		const [, handleStreamMessageReceived] = mockOnStreamData.mock.calls[0];
+		roomsSubscription?.stop();
+		await handleStreamMessageReceived(removedSubscriptionFrame());
+
+		expect(mockedDatabase.active.get).not.toHaveBeenCalled();
+	});
+
+	it('processes a frame whose host matches the subscribed server', async () => {
+		mockedSdk.setClient({ host: HOST });
+		subscribeRooms();
+
+		const [, handleStreamMessageReceived] = mockOnStreamData.mock.calls[0];
+		await handleStreamMessageReceived(removedSubscriptionFrame());
+
+		expect(mockedDatabase.active.get).toHaveBeenCalledWith('subscriptions');
+	});
+});

app/lib/methods/subscriptions/rooms.ts

@@ -39,7 +39,7 @@ import { handleVideoConfIncomingWebsocketMessages } from '../../../actions/video
 const removeListener = (listener: { stop: () => void }) => listener.stop();
 
 let streamListener: Promise<any> | false;
-let subServer: string;
+let subscribedHost: string | null = null;
 let queue: { [key: string]: ISubscription | IRoom } = {};
 let subTimer: ReturnType<typeof setTimeout> | null | false = null;
 const WINDOW_TIME = 500;
@@ -301,8 +301,7 @@ export default function subscribeRooms() {
 	const handleStreamMessageReceived = protectedFunction(async (ddpMessage: IDDPMessage) => {
 		const db = database.active;
 
-		// check if the server from variable is the same as the js sdk client
-		if (sdk && sdk.current.client && sdk.current.client.host !== subServer) {
+		if (!subscribedHost || sdk.host !== subscribedHost) {
 			return;
 		}
 		if (ddpMessage.msg === 'added') {
@@ -433,14 +432,20 @@ export default function subscribeRooms() {
 			subTimer = false;
 		}
 		roomsSubscription = null;
+		subscribedHost = null;
 	};
 
+	const host = sdk.host;
+	if (!host) {
+		return null;
+	}
+
 	streamListener = sdk.onStreamData('stream-notify-user', handleStreamMessageReceived);
 
 	try {
 		// set the server that started this task
-		subServer = sdk.current.client.host;
-		sdk.current.subscribeNotifyUser().catch((e: unknown) => console.log(e));
+		subscribedHost = host;
+		sdk.subscribeNotifyUser().catch((e: unknown) => console.log(e));
 		roomsSubscription = { stop: () => stop() };
 		return null;
 	} catch (e) {

app/lib/services/__tests__/socketHealth.integration.test.ts

@@ -7,7 +7,7 @@ import {
 	framesOn,
 	stopAnsweringFrames
 } from '../../testUtils/sdkIntegration';
-import type { MockConnection, ISdkDriver } from '../../testUtils/sdkIntegration';
+import type { IMockSdk, MockConnection, IMockSdkDriver } from '../../testUtils/sdkIntegration';
 import type * as SdkIntegration from '../../testUtils/sdkIntegration';
 
 const mockConnections: MockConnection[] = [];
@@ -19,24 +19,24 @@ jest.mock('universal-websocket-client', () =>
 	})
 );
 
-jest.mock('../sdk', () => ({
-	__esModule: true,
-	default: { current: undefined }
-}));
+jest.mock('../sdk', () => {
+	const sdkIntegration = jest.requireActual<typeof SdkIntegration>('../../testUtils/sdkIntegration');
+	return { __esModule: true, default: sdkIntegration.makeSdkMock() };
+});
 
 const USER_ID = 'user-id';
 const PING_INTERVAL = 10000;
 const CLOSED = 3;
 
 describe('recoverSocket against the real SDK socket', () => {
-	let driver: ISdkDriver;
+	let driver: IMockSdkDriver;
 
 	beforeEach(async () => {
 		jest.clearAllMocks();
 		jest.useFakeTimers();
 		mockConnections.length = 0;
 		driver = await buildConnectedDriver(mockConnections, USER_ID);
-		(sdk as unknown as { current: { driver: ISdkDriver } }).current = { driver };
+		(sdk as unknown as IMockSdk).setClient({ driver });
 	});
 
 	afterEach(() => {
@@ -154,7 +154,7 @@ describe('recoverSocket against the real SDK socket', () => {
 		await jest.advanceTimersByTimeAsync(0);
 		await expect(recovery).resolves.toBe('reopened');
 
-		const resubscribed = driver.waitForNotifyUserMediaSubs!();
+		const resubscribed = driver.waitForNotifyUserMediaSubs();
 		await jest.advanceTimersByTimeAsync(200);
 		await expect(resubscribed).resolves.toBe(true);
 
@@ -189,7 +189,7 @@ describe('recoverSocket against the real SDK socket', () => {
 		await jest.advanceTimersByTimeAsync(0);
 		await expect(recovery).resolves.toBe('reopened');
 
-		const resubscribed = driver.waitForNotifyUserMediaSubs!(1000);
+		const resubscribed = driver.waitForNotifyUserMediaSubs(1000);
 		await jest.advanceTimersByTimeAsync(100);
 		expect(framesOn(mockConnections[1], 'sub')).toHaveLength(0);
 
@@ -215,7 +215,7 @@ describe('recoverSocket against the real SDK socket', () => {
 
 		stopAnsweringFrames(mockConnections[1]);
 
-		const resubscribed = driver.waitForNotifyUserMediaSubs!(500);
+		const resubscribed = driver.waitForNotifyUserMediaSubs(500);
 		await jest.advanceTimersByTimeAsync(500);
 
 		await expect(resubscribed).resolves.toBe(false);