chore: improve the way we send DDP connection data to hooks #38268 #38294

coderabbitai · 2026-01-27T19:19:53Z

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Check if DeviceManagementService listens to accounts.login and emits device-login echo "=== Checking DeviceManagementService for device-login emission ===" cat -n apps/meteor/server/services/device-management/service.ts echo "" echo "=== All device-login emissions in the codebase ===" rg -n "emit.*device-login" --type ts -B 2 -A 2

Repository: RocketChat/Rocket.Chat

Length of output: 1956

🏁 Script executed:

#!/bin/bash # Check the context of the sauMonitorHooks.ts emission - what event triggers it echo "=== Full context of sauMonitorHooks.ts ===" cat -n apps/meteor/server/hooks/sauMonitorHooks.ts | head -50

Repository: RocketChat/Rocket.Chat

Length of output: 1988

🏁 Script executed:

#!/bin/bash # Find where accounts.login event is emitted echo "=== Where is accounts.login event emitted ===" rg -n "emit.*accounts\.login|services\.emit.*accounts\.login" --type ts -B 3 -A 3 echo "" echo "=== Check if sauMonitor events configuration or accounts emission ===" rg -n "accounts\.login" --type ts -B 2 -A 2 | head -100

Repository: RocketChat/Rocket.Chat

Length of output: 4361

🏁 Script executed:

#!/bin/bash # Check if sauMonitorHooks.ts or any Meteor hook emits accounts.login event echo "=== Does sauMonitorHooks.ts emit accounts.login event ===" rg -n "broadcast.*accounts\.login|api\.emit.*accounts\.login" --type ts -B 2 -A 2 echo "" echo "=== Check Accounts.onLogin relationship to accounts.login event ===" rg -n "Accounts\.onLogin" --type ts | head -20 echo "" echo "=== Check where accounts.login event is broadcast from ===" rg -n "broadcast\('accounts\.login" --type ts -B 5 -A 5

Repository: RocketChat/Rocket.Chat

Length of output: 2405

device-login is emitted twice per login in EE deployments.

The concern is valid. In Enterprise Edition, two code paths emit the same event for a single login:

DeviceManagementService listens to the accounts.login event broadcast from DDPStreamer and emits device-login (line 14 in apps/meteor/server/services/device-management/service.ts)

sauMonitorHooks.ts uses Meteor's Accounts.onLogin hook and also emits device-login (line 38)

Both are triggered during the same login flow, causing the handler in apps/meteor/ee/server/lib/deviceManagement/session.ts to run twice. Consider removing one of these duplicate emissions to avoid processing the same login event twice.

🤖 Prompt for AI Agents

In `@apps/meteor/server/hooks/sauMonitorHooks.ts` at line 38, The same 'device-login' event is emitted twice (once from DeviceManagementService reacting to the DDPStreamer accounts.login broadcast and once from sauMonitorHooks.ts via Accounts.onLogin), causing duplicate handling; remove the redundant emission in sauMonitorHooks.ts (the deviceManagementEvents.emit('device-login', ...) call) or add a guard there so it only emits when DeviceManagementService/EE path is not active (e.g., detect presence of the DDPStreamer/DeviceManagementService or an EE flag) to ensure a single emission per login.

coderabbitai · 2026-01-27T19:26:45Z

⚠️ Potential issue | 🟡 Minor

Remove inline implementation comment.

Line 56 introduces a code comment in implementation; prefer documenting this in the events type file or docs instead. As per coding guidelines, ...

🧹 Proposed clean-up

- // in case of implementing a listener of this event, define the parameters type in services/sauMonitor/events.ts sauEvents.emit('sau.socket.connected', { instanceId: InstanceStatus.id(), connectionId: connection.id });

🤖 Prompt for AI Agents

In `@apps/meteor/server/hooks/sauMonitorHooks.ts` around lines 56 - 58, Remove the inline implementation comment before the sauEvents.emit call and instead document the expected event parameter types in the events type definition file (services/sauMonitor/events.ts); update the type declaration or add a comment in that file describing the parameters for the 'sau.socket.connected' event so the sauEvents.emit({ instanceId: InstanceStatus.id(), connectionId: connection.id }) call remains clean and implementation-only.

-Original file line number
+Diff line change
@@ -1,4 +1,4 @@
-    import type { ISession, ISessionDevice, ISocketConnectionLogged, IUser } from '@rocket.chat/core-typings';
+    import type { ISession, ISessionDevice, IUser } from '@rocket.chat/core-typings';
     import { cronJobs } from '@rocket.chat/cron';
     import { Logger } from '@rocket.chat/logger';
     import { Sessions, Users, aggregates } from '@rocket.chat/models';
@@ Expand All / @@ -8,7 +8,6 @@ import UAParser from 'ua-parser-js'; @@
     import { UAParserMobile, UAParserDesktop } from './UAParserCustom';
     import { getMostImportantRole } from '../../../../lib/roles/getMostImportantRole';
-    import { getClientAddress } from '../../../../server/lib/getClientAddress';
     import { sauEvents } from '../../../../server/services/sauMonitor/events';
     type DateObj = { day: number; month: number; year: number };
@@ Expand All / @@ -32,6 +31,16 @@ const getUserRoles = mem( @@
     const isProdEnv = process.env.NODE_ENV === 'production';
+    type HandleSessionArgs = {
+    	userId: string;
+    	instanceId: string;
+    	userAgent: string;
+    	loginToken?: string;
+    	connectionId: string;
+    	clientAddress: string;
+    	host: string;
+    };
     /**
      * Server Session Monitor for SAU(Simultaneously Active Users) based on Meteor server sessions
      */
@@ Expand Down Expand Up / @@ -97,12 +106,12 @@ export class SAUMonitorClass { @@
     			return;
     		}
-    		sauEvents.on('socket.disconnected', async ({ id, instanceId }) => {
+    		sauEvents.on('sau.socket.disconnected', async ({ connectionId, instanceId }) => {
     			if (!this.isRunning()) {
     				return;
     			}
-    			await Sessions.closeByInstanceIdAndSessionId(instanceId, id);
+    			await Sessions.closeByInstanceIdAndSessionId(instanceId, connectionId);
     		});
     	}
@@ Expand All / @@ -111,7 +120,7 @@ export class SAUMonitorClass { @@
     			return;
     		}
-    		sauEvents.on('accounts.login', async ({ userId, connection }) => {
+    		sauEvents.on('sau.accounts.login', async ({ userId, instanceId, userAgent, loginToken, connectionId, clientAddress, host }) => {
     			if (!this.isRunning()) {
     				return;
     			}
@@ Expand All / @@ -121,23 +130,22 @@ export class SAUMonitorClass { @@
     			const mostImportantRole = getMostImportantRole(roles);
     			const loginAt = new Date();
-    			const params = { userId, roles, mostImportantRole, loginAt, ...getDateObj() };
-    			await this._handleSession(connection, params);
+    			const params = { roles, mostImportantRole, loginAt, ...getDateObj() };
+    			await this._handleSession({ userId, instanceId, userAgent, loginToken, connectionId, clientAddress, host }, params);
     		});
-    		sauEvents.on('accounts.logout', async ({ userId, connection }) => {
+    		sauEvents.on('sau.accounts.logout', async ({ userId, sessionId }) => {
     			if (!this.isRunning()) {
     				return;
     			}
     			if (!userId) {
-    				logger.warn({ msg: "Received 'accounts.logout' event without 'userId'" });
+    				logger.warn({ msg: "Received 'sau.accounts.logout' event without 'userId'" });
     				return;
     			}
-    			const { id: sessionId } = connection;
     			if (!sessionId) {
-    				logger.warn({ msg: "Received 'accounts.logout' event without 'sessionId'" });
+    				logger.warn({ msg: "Received 'sau.accounts.logout' event without 'sessionId'" });
     				return;
     			}
@@ Expand All / @@ -157,14 +165,20 @@ export class SAUMonitorClass { @@
     	}
     	private async _handleSession(
-    		connection: ISocketConnectionLogged,
-    		params: Pick<ISession, 'userId' | 'mostImportantRole' | 'loginAt' | 'day' | 'month' | 'year' | 'roles'>,
+    		{ userId, instanceId, userAgent, loginToken, connectionId, clientAddress, host }: HandleSessionArgs,
+    		params: Pick<ISession, 'mostImportantRole' | 'loginAt' | 'day' | 'month' | 'year' | 'roles'>,
     	): Promise<void> {
-    		const data = this._getConnectionInfo(connection, params);
-    		if (!data) {
-    			return;
-    		}
+    		const data: Omit<ISession, '_id' | '_updatedAt' | 'createdAt' | 'searchTerm'> = {
+    			userId,
+    			...(loginToken && { loginToken }),
+    			ip: clientAddress,
+    			host,
+    			sessionId: connectionId,
+    			instanceId,
+    			type: 'session',
+    			...(loginToken && this._getUserAgentInfo(userAgent)),
+    			...params,
+    		};
     		const searchTerm = this._getSearchTerm(data);
@@ Expand Down Expand Up / @@ -221,37 +235,7 @@ export class SAUMonitorClass { @@
     			.join('');
     	}
-    	private _getConnectionInfo(
-    		connection: ISocketConnectionLogged,
-    		params: Pick<ISession, 'userId' | 'mostImportantRole' | 'loginAt' | 'day' | 'month' | 'year' | 'roles'>,
-    	): Omit<ISession, '_id' | '_updatedAt' | 'createdAt' | 'searchTerm'> | undefined {
-    		if (!connection) {
-    			return;
-    		}
-    		const ip = getClientAddress(connection);
-    		const host = connection.httpHeaders?.host ?? '';
-    		return {
-    			type: 'session',
-    			sessionId: connection.id,
-    			instanceId: connection.instanceId,
-    			...(connection.loginToken && { loginToken: connection.loginToken }),
-    			ip,
-    			host,
-    			...this._getUserAgentInfo(connection),
-    			...params,
-    		};
-    	}
-    	private _getUserAgentInfo(connection: ISocketConnectionLogged): { device: ISessionDevice } | undefined {
-    		if (!connection?.httpHeaders?.['user-agent']) {
-    			return;
-    		}
-    		const uaString = connection.httpHeaders['user-agent'];
+    	private _getUserAgentInfo(uaString: string): { device: ISessionDevice } | undefined {
     		// TODO define a type for "result" below
     		// | UAParser.IResult
     		// | { device: { type: string; model?: string }; browser: undefined; os: undefined; app: { name: string; version: string } }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -32,14 +32,14 @@ const uaParser = async ( @@
     };
     export const listenSessionLogin = () => {
-    	return deviceManagementEvents.on('device-login', async ({ userId, connection }) => {
+    	return deviceManagementEvents.on('device-login', async ({ userId, userAgent, loginToken, clientAddress }) => {
     		const deviceEnabled = settings.get('Device_Management_Enable_Login_Emails');
     		if (!deviceEnabled) {
     			return;
     		}
-    		if (connection.loginToken) {
+    		if (loginToken) {
     			return;
     		}
@@ Expand Down Expand Up / @@ -67,11 +67,7 @@ export const listenSessionLogin = () => { @@
     			emails: [{ address: email }],
     		} = user;
-    		const userAgentString =
-    			connection.httpHeaders instanceof Headers
-    				? (connection.httpHeaders.get('user-agent') ?? '')
-    				: (connection.httpHeaders['user-agent'] ?? '');
-    		const { browser, os, device, cpu, app } = await uaParser(userAgentString);
+    		const { browser, os, device, cpu, app } = await uaParser(userAgent);
     		const mailData = {
     			name,
@@ Expand All / @@ -81,7 +77,7 @@ export const listenSessionLogin = () => { @@
     			deviceInfo: `${device.type || t('Device_Management_Device_Unknown')} ${device.vendor || ''} ${device.model || ''} ${
     				cpu.architecture || ''
     			}`,
-    			ipInfo: connection.clientAddress,
+    			ipInfo: clientAddress,
     			userAgent: '',
     			date: moment().format(String(dateFormat)),
     		};
@@ Expand All / @@ -105,7 +101,7 @@ export const listenSessionLogin = () => { @@
     				mailData.deviceInfo = `Desktop App ${cpu.architecture || ''}`;
     				break;
     			default:
-    				mailData.userAgent = connection.httpHeaders['user-agent'] || '';
+    				mailData.userAgent = userAgent || '';
     				break;
     		}
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,6 +1,6 @@
-    import type { IncomingHttpHeaders } from 'http';
+    import { hashLoginToken } from '@rocket.chat/account-utils';
     import { InstanceStatus } from '@rocket.chat/instance-status';
+    import { getHeader } from '@rocket.chat/tools';
     import { Accounts } from 'meteor/accounts-base';
     import { Meteor } from 'meteor/meteor';
@@ Expand All / @@ -19,45 +19,40 @@ Accounts.onLogin((info: ILoginAttempt) => { @@
     	}
     	const { resume } = methodArguments.find((arg) => 'resume' in arg) ?? {};
+    	const loginToken = resume ? hashLoginToken(resume) : '';
+    	const instanceId = InstanceStatus.id();
+    	const clientAddress = info.connection.clientAddress || getHeader(httpHeaders, 'x-real-ip');
+    	const userAgent = getHeader(httpHeaders, 'user-agent');
+    	const host = getHeader(httpHeaders, 'host');
-    	const eventObject = {
+    	sauEvents.emit('sau.accounts.login', {
     		userId: info.user._id,
-    		connection: {
-    			...info.connection,
-    			...(resume && { loginToken: Accounts._hashLoginToken(resume) }),
-    			instanceId: InstanceStatus.id(),
-    			httpHeaders: httpHeaders as IncomingHttpHeaders,
-    		},
-    	};
-    	sauEvents.emit('accounts.login', eventObject);
-    	deviceManagementEvents.emit('device-login', eventObject);
+    		instanceId,
+    		userAgent,
+    		loginToken,
+    		connectionId: info.connection.id,
+    		clientAddress,
+    		host,
+    	});
+    	deviceManagementEvents.emit('device-login', { userId: info.user._id, userAgent, loginToken, clientAddress });
     });
     Accounts.onLogout((info) => {
-    	const { httpHeaders } = info.connection;
     	if (!info.user) {
     		return;
     	}
-    	sauEvents.emit('accounts.logout', {
-    		userId: info.user._id,
-    		connection: { instanceId: InstanceStatus.id(), ...info.connection, httpHeaders: httpHeaders as IncomingHttpHeaders },
-    	});
+    	sauEvents.emit('sau.accounts.logout', { userId: info.user._id, sessionId: info.connection.id });
     });
     Meteor.onConnection((connection) => {
     	connection.onClose(async () => {
-    		const { httpHeaders } = connection;
-    		sauEvents.emit('socket.disconnected', {
-    			instanceId: InstanceStatus.id(),
-    			...connection,
-    			httpHeaders: httpHeaders as IncomingHttpHeaders,
-    		});
+    		sauEvents.emit('sau.socket.disconnected', { connectionId: connection.id, instanceId: InstanceStatus.id() });
     	});
     });
     Meteor.onConnection((connection) => {
-    	const { httpHeaders } = connection;
-    	sauEvents.emit('socket.connected', { instanceId: InstanceStatus.id(), ...connection, httpHeaders: httpHeaders as IncomingHttpHeaders });
+    	// in case of implementing a listener of this event, define the parameters type in services/sauMonitor/events.ts
+    	sauEvents.emit('sau.socket.connected', { instanceId: InstanceStatus.id(), connectionId: connection.id });
     });

-Original file line number
+Diff line change
@@ -1,6 +1,5 @@
-    import type { ISocketConnectionLogged } from '@rocket.chat/core-typings';
     import { Emitter } from '@rocket.chat/emitter';
     export const deviceManagementEvents = new Emitter<{
-    	'device-login': { userId: string; connection: ISocketConnectionLogged };
+    	'device-login': { userId: string; userAgent: string; loginToken?: string; clientAddress: string };
     }>();

-Original file line number
+Diff line change
@@ -1,5 +1,6 @@
     import type { IDeviceManagementService } from '@rocket.chat/core-services';
     import { ServiceClassInternal } from '@rocket.chat/core-services';
+    import { getHeader } from '@rocket.chat/tools';
     import { deviceManagementEvents } from './events';
@@ Expand All @@
     	constructor() {
     		super();
-    		this.onEvent('accounts.login', async (data) => {
-    			// TODO need to add loginToken to data
-    			deviceManagementEvents.emit('device-login', data);
+    		this.onEvent('accounts.login', async ({ userId, connection }) => {
+    			deviceManagementEvents.emit('device-login', {
+    				userId,
+    				userAgent: getHeader(connection.httpHeaders, 'user-agent'),
+    				clientAddress: connection.clientAddress || getHeader(connection.httpHeaders, 'x-real-ip'),
+    				loginToken: connection.loginToken,
+    			});
     		});
     	}
     }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: improve the way we send DDP connection data to hooks #38268 #38294

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

coderabbitai bot Jan 27, 2026

Uh oh!

coderabbitai bot Jan 27, 2026

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -1,9 +1,16 @@
-    import type { ISocketConnection, ISocketConnectionLogged } from '@rocket.chat/core-typings';
     import { Emitter } from '@rocket.chat/emitter';
     export const sauEvents = new Emitter<{
-    	'accounts.login': { userId: string; connection: ISocketConnectionLogged };
-    	'accounts.logout': { userId: string; connection: ISocketConnection };
-    	'socket.connected': ISocketConnection;
-    	'socket.disconnected': ISocketConnection;
+    	'sau.accounts.login': {
+    		userId: string;
+    		instanceId: string;
+    		connectionId: string;
+    		loginToken?: string;
+    		clientAddress: string;
+    		userAgent: string;
+    		host: string;
+    	};
+    	'sau.accounts.logout': { userId: string; sessionId: string };
+    	'sau.socket.connected': { instanceId: string; connectionId: string };
+    	'sau.socket.disconnected': { instanceId: string; connectionId: string };
     }>();

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,6 +2,7 @@ @@
     import { ServiceClassInternal } from '@rocket.chat/core-services';
     import type { ISAUMonitorService } from '@rocket.chat/core-services';
+    import { getHeader } from '@rocket.chat/tools';
     import { sauEvents } from './events';
@@ Expand All @@
     	constructor() {
     		super();
-    		this.onEvent('accounts.login', async (data) => {
-    			sauEvents.emit('accounts.login', data);
+    		this.onEvent('accounts.login', async ({ userId, connection }) => {
+    			sauEvents.emit('sau.accounts.login', {
+    				userId,
+    				instanceId: connection.instanceId,
+    				connectionId: connection.id,
+    				loginToken: connection.loginToken,
+    				clientAddress: connection.clientAddress || getHeader(connection.httpHeaders, 'x-real-ip'),
+    				userAgent: getHeader(connection.httpHeaders, 'user-agent'),
+    				host: getHeader(connection.httpHeaders, 'host'),
+    			});
     		});
-    		this.onEvent('accounts.logout', async (data) => {
-    			sauEvents.emit('accounts.logout', data);
+    		this.onEvent('accounts.logout', async ({ userId, connection }) => {
+    			sauEvents.emit('sau.accounts.logout', { userId, sessionId: connection.id });
     		});
     		this.onEvent('socket.disconnected', async (data) => {
-    			// console.log('socket.disconnected', data);
-    			sauEvents.emit('socket.disconnected', data);
+    			sauEvents.emit('sau.socket.disconnected', { instanceId: data.instanceId, connectionId: data.id });
     		});
     		this.onEvent('socket.connected', async (data) => {
-    			// console.log('socket.connected', data);
-    			sauEvents.emit('socket.connected', data);
+    			sauEvents.emit('sau.socket.connected', { instanceId: data.instanceId, connectionId: data.id });
     		});
     	}
     }

chore: improve the way we send DDP connection data to hooks #38268 #38294

Are you sure you want to change the base?

chore: improve the way we send DDP connection data to hooks #38268 #38294

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

coderabbitai bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!