fix(auth): add timeout to CAS popup login

.changeset/fix-recorder-interval-memory-leak.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes memory leak in `VideoMessageRecorder` and `AudioMessageRecorder` caused by `setInterval` not being cleared on component unmount.

apps/meteor/client/lib/openCASLoginPopup.spec.ts

@@ -0,0 +1,78 @@
+import { Meteor } from 'meteor/meteor';
+
+import { openCASLoginPopup } from './openCASLoginPopup';
+
+const mockPeek = jest.fn();
+
+jest.mock('./settings', () => ({
+	settings: {
+		peek: (...args: unknown[]) => mockPeek(...args),
+	},
+}));
+
+jest.mock('meteor/meteor', () => ({
+	Meteor: {
+		absoluteUrl: jest.fn(),
+	},
+}));
+
+describe('openCASLoginPopup', () => {
+	const originalRuntimeConfig = (global as any).__meteor_runtime_config__;
+	let openSpy: jest.SpyInstance;
+
+	beforeEach(() => {
+		jest.useFakeTimers();
+		jest.clearAllMocks();
+		mockPeek.mockImplementation((key: string) => {
+			switch (key) {
+				case 'CAS_login_url':
+					return 'https://cas.example.com/login';
+				case 'CAS_popup_width':
+					return 800;
+				case 'CAS_popup_height':
+					return 600;
+				default:
+					return undefined;
+			}
+		});
+
+		(Meteor.absoluteUrl as jest.Mock).mockReturnValue('https://chat.example.com/');
+		(global as any).__meteor_runtime_config__ = { ROOT_URL_PATH_PREFIX: '' };
+		openSpy = jest.spyOn(window, 'open').mockReturnValue({ focus: jest.fn(), closed: false } as unknown as Window);
+	});
+
+	afterEach(() => {
+		jest.useRealTimers();
+		openSpy.mockRestore();
+		(global as any).__meteor_runtime_config__ = originalRuntimeConfig;
+	});
+
+	it('resolves when the popup closes', async () => {
+		const popup = { focus: jest.fn(), closed: false } as unknown as Window;
+		(window.open as jest.Mock).mockReturnValue(popup);
+
+		const promise = openCASLoginPopup('credential-token');
+
+		await jest.advanceTimersByTimeAsync(100);
+		(popup as { closed: boolean }).closed = true;
+		await jest.advanceTimersByTimeAsync(100);
+
+		await expect(promise).resolves.toBeUndefined();
+		expect(window.open).toHaveBeenCalledWith(
+			'https://cas.example.com/login?service=https%3A%2F%2Fchat.example.com%2F_cas%2Fcredential-token',
+			'Login',
+			expect.stringContaining('width=800'),
+		);
+	});
+
+	it('rejects if the popup stays open too long', async () => {
+		const popup = { focus: jest.fn(), closed: false } as unknown as Window;
+		(window.open as jest.Mock).mockReturnValue(popup);
+
+		const promise = openCASLoginPopup('credential-token');
+
+		await jest.advanceTimersByTimeAsync(5 * 60 * 1000);
+
+		await expect(promise).rejects.toThrow('CAS login popup did not close in time');
+	});
+});

apps/meteor/client/lib/openCASLoginPopup.ts

@@ -2,6 +2,8 @@ import { Meteor } from 'meteor/meteor';
 
 import { settings } from './settings';
 
+const POPUP_CLOSE_TIMEOUT_MS = 5 * 60 * 1000;
+
 const openCenteredPopup = (url: string, width: number, height: number) => {
 	const screenX = window.screenX ?? window.screenLeft;
 	const screenY = window.screenY ?? window.screenTop;
@@ -41,13 +43,19 @@ const getPopupUrl = (credentialToken: string): string => {
 };
 
 const waitForPopupClose = (popup: Window) => {
-	return new Promise<void>((resolve) => {
+	return new Promise<void>((resolve, reject) => {
 		const checkPopupOpen = setInterval(() => {
 			if (popup.closed || popup.closed === undefined) {
 				clearInterval(checkPopupOpen);
+				clearTimeout(timeoutId);
 				resolve();
 			}
 		}, 100);
+
+		const timeoutId = setTimeout(() => {
+			clearInterval(checkPopupOpen);
+			reject(new Error('CAS login popup did not close in time'));
+		}, POPUP_CLOSE_TIMEOUT_MS);
 	});
 };
 

apps/meteor/client/views/composer/AudioMessageRecorder/AudioMessageRecorder.tsx

@@ -3,7 +3,7 @@ import { Box, Icon, Throbber } from '@rocket.chat/fuselage';
 import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
 import { MessageComposerAction } from '@rocket.chat/ui-composer';
 import type { ReactElement } from 'react';
-import { useEffect, useState } from 'react';
+import { useEffect, useState, useRef } from 'react';
 import { useTranslation } from 'react-i18next';
 
 import { AudioRecorder } from '../../../../app/ui/client/lib/recorderjs/AudioRecorder';
@@ -21,14 +21,18 @@ const AudioMessageRecorder = ({ rid, isMicrophoneDenied }: AudioMessageRecorderP
 
 	const [state, setState] = useState<'loading' | 'recording'>('recording');
 	const [time, setTime] = useState('00:00');
-	const [recordingInterval, setRecordingInterval] = useState<ReturnType<typeof setInterval> | null>(null);
+	const recordingIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
 	const [recordingRoomId, setRecordingRoomId] = useState<IRoom['_id'] | null>(null);
 
-	const stopRecording = useEffectEvent(async () => {
-		if (recordingInterval) {
-			clearInterval(recordingInterval);
+	const clearRecordingInterval = () => {
+		if (recordingIntervalRef.current) {
+			clearInterval(recordingIntervalRef.current);
+			recordingIntervalRef.current = null;
 		}
-		setRecordingInterval(null);
+	};
+
+	const stopRecording = useEffectEvent(async () => {
+		clearRecordingInterval();
 		setRecordingRoomId(null);
 
 		setTime('00:00');
@@ -42,9 +46,12 @@ const AudioMessageRecorder = ({ rid, isMicrophoneDenied }: AudioMessageRecorderP
 		return blob;
 	});
 
-	const handleUnmount = useEffectEvent(async () => {
+	const handleUnmount = useEffectEvent(() => {
+		clearRecordingInterval();
 		if (state === 'recording') {
-			await stopRecording();
+			audioRecorder.stop(() => undefined);
+			chat?.action.stop('recording');
+			chat?.composer?.setRecordingMode(false);
 		}
 	});
 
@@ -59,18 +66,16 @@ const AudioMessageRecorder = ({ rid, isMicrophoneDenied }: AudioMessageRecorderP
 			await audioRecorder.start();
 			chat?.action.performContinuously('recording');
 			const startTime = new Date();
-			setRecordingInterval(
-				setInterval(() => {
-					const now = new Date();
-					const distance = (now.getTime() - startTime.getTime()) / 1000;
-					const minutes = Math.floor(distance / 60);
-					const seconds = Math.floor(distance % 60);
-					setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
-				}, 1000),
-			);
+			recordingIntervalRef.current = setInterval(() => {
+				const now = new Date();
+				const distance = (now.getTime() - startTime.getTime()) / 1000;
+				const minutes = Math.floor(distance / 60);
+				const seconds = Math.floor(distance % 60);
+				setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
+			}, 1000);
 			setRecordingRoomId(rid);
 		} catch (error) {
-			console.log(error);
+			console.error(error);
 			chat?.composer?.setRecordingMode(false);
 		}
 	});
@@ -100,6 +105,14 @@ const AudioMessageRecorder = ({ rid, isMicrophoneDenied }: AudioMessageRecorderP
 		};
 	}, [handleUnmount, handleRecord]);
 
+	// Ensure interval is cleared on unmount as a safety net
+	useEffect(() => {
+		return () => {
+			clearRecordingInterval();
+		};
+	// eslint-disable-next-line react-hooks/exhaustive-deps
+	}, []);
+
 	if (isMicrophoneDenied) {
 		return null;
 	}

apps/meteor/client/views/composer/VideoMessageRecorder/VideoMessageRecorder.tsx

@@ -4,7 +4,7 @@ import { Box, ButtonGroup, Button, Icon, PositionAnimated } from '@rocket.chat/f
 import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
 import { useTranslation, useToastMessageDispatch } from '@rocket.chat/ui-contexts';
 import type { AllHTMLAttributes, RefObject } from 'react';
-import { useRef, useEffect, useState } from 'react';
+import { useRef, useEffect, useState, useCallback } from 'react';
 
 import { UserAction, USER_ACTIVITIES } from '../../../../app/ui/client/lib/UserAction';
 import { VideoRecorder } from '../../../../app/ui/client/lib/recorderjs/videoRecorder';
@@ -43,23 +43,30 @@ const VideoMessageRecorder = ({ rid, tmid, reference }: VideoMessageRecorderProp
 
 	const [time, setTime] = useState<string | undefined>('00:00');
 	const [recordingState, setRecordingState] = useState<'idle' | 'loading' | 'recording'>('idle');
-	const [recordingInterval, setRecordingInterval] = useState<ReturnType<typeof setInterval> | null>(null);
+	const recordingIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
 	const isRecording = recordingState === 'recording';
 	const sendButtonDisabled = !(VideoRecorder.cameraStarted.get() && !(recordingState === 'recording'));
 
 	const chat = useChat();
 
-	const stopVideoRecording = async (rid: IRoom['_id'], tmid?: IMessage['_id']) => {
-		if (recordingInterval) {
-			clearInterval(recordingInterval);
+	const clearRecordingInterval = useCallback(() => {
+		if (recordingIntervalRef.current) {
+			clearInterval(recordingIntervalRef.current);
+			recordingIntervalRef.current = null;
 		}
-		setRecordingInterval(null);
-		VideoRecorder.stopRecording();
-		UserAction.stop(rid, USER_ACTIVITIES.USER_RECORDING, { tmid });
-		setRecordingState('idle');
-	};
+	}, []);
+
+	const stopVideoRecording = useCallback(
+		(rid: IRoom['_id'], tmid?: IMessage['_id']) => {
+			clearRecordingInterval();
+			VideoRecorder.stopRecording();
+			UserAction.stop(rid, USER_ACTIVITIES.USER_RECORDING, { tmid });
+			setRecordingState('idle');
+		},
+		[clearRecordingInterval],
+	);
 
-	const handleRecord = async () => {
+	const handleRecord = useCallback(async () => {
 		if (recordingState === 'recording') {
 			stopVideoRecording(rid, tmid);
 		} else {
@@ -68,17 +75,15 @@ const VideoMessageRecorder = ({ rid, tmid, reference }: VideoMessageRecorderProp
 			UserAction.performContinuously(rid, USER_ACTIVITIES.USER_RECORDING, { tmid });
 			setTime('00:00');
 			const startTime = new Date();
-			setRecordingInterval(
-				setInterval(() => {
-					const now = new Date();
-					const distance = (now.getTime() - startTime.getTime()) / 1000;
-					const minutes = Math.floor(distance / 60);
-					const seconds = Math.floor(distance % 60);
-					setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
-				}, 1000),
-			);
+			recordingIntervalRef.current = setInterval(() => {
+				const now = new Date();
+				const distance = (now.getTime() - startTime.getTime()) / 1000;
+				const minutes = Math.floor(distance / 60);
+				const seconds = Math.floor(distance % 60);
+				setTime(`${String(minutes).padStart(2, '0')}:${String(seconds).padStart(2, '0')}`);
+			}, 1000);
 		}
-	};
+	}, [recordingState, rid, tmid, stopVideoRecording]);
 
 	const handleSendRecord = async () => {
 		const cb = async (blob: Blob) => {
@@ -108,9 +113,11 @@ const VideoMessageRecorder = ({ rid, tmid, reference }: VideoMessageRecorderProp
 		VideoRecorder.start(videoRef.current ?? undefined, (success) => (!success ? handleCancel() : undefined));
 
 		return () => {
+			clearRecordingInterval();
 			VideoRecorder.stop();
+			UserAction.stop(rid, USER_ACTIVITIES.USER_RECORDING, { tmid });
 		};
-	}, [dispatchToastMessage, handleCancel, t]);
+	}, [dispatchToastMessage, handleCancel, clearRecordingInterval, rid, tmid, t]);
 
 	return (
 		<PositionAnimated visible='visible' anchor={reference} placement='top-end'>