fix: Prevent audit messages from reading abac rooms

apps/meteor/ee/server/hooks/abac/beforeAddUserToRoom.ts

@@ -18,5 +18,5 @@ beforeAddUserToRoom.patch(async (prev, users, room, actor) => {
 		throw new Error('error-room-is-abac-managed');
 	}
 
-	await Abac.checkUsernamesMatchAttributes(validUsers as string[], room.abacAttributes);
+	await Abac.checkUsernamesMatchAttributes(validUsers as string[], room.abacAttributes, room._id);
 });

apps/meteor/ee/server/lib/audit/methods.ts

@@ -37,7 +37,8 @@ const getRoomInfoByAuditParams = async ({
 	userId: string;
 }) => {
 	if (rid) {
-		return getValue(await Rooms.findOne({ _id: rid }));
+		// When ABAC is enabled, only rooms without ABAC attributes are considered for auditing by room ID.
+		return getValue(await Rooms.findOne({ _id: rid, abacAttributes: { $exists: false } }));
 	}
 
 	if (type === 'd') {
@@ -165,7 +166,7 @@ Meteor.methods<ServerMethods>({
 		} else {
 			const roomInfo = await getRoomInfoByAuditParams({ type, roomId: rid, users: usernames, visitor, agent, userId: user._id });
 			if (!roomInfo) {
-				throw new Meteor.Error('Room doesn`t exist');
+				throw new Meteor.Error(`Room doesn't exist`);
 			}
 
 			rids = roomInfo.rids;

apps/meteor/tests/end-to-end/api/abac.ts

@@ -4,7 +4,7 @@ import { expect } from 'chai';
 import { before, after, describe, it } from 'mocha';
 import { MongoClient } from 'mongodb';
 
-import { getCredentials, request, credentials } from '../../data/api-data';
+import { getCredentials, request, credentials, methodCall } from '../../data/api-data';
 import { sleep } from '../../data/livechat/utils';
 import { updatePermission, updateSetting } from '../../data/permissions.helper';
 import { createRoom, deleteRoom } from '../../data/rooms.helper';
@@ -387,6 +387,35 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 				});
 		});
 
+		it('should throw an error when trying to audit the messages of an abac managed room', async () => {
+			await request
+				.post(methodCall('auditGetMessages'))
+				.set(credentials)
+				.send({
+					message: JSON.stringify({
+						method: 'auditGetMessages',
+						params: [
+							{
+								type: '',
+								msg: 'test1234',
+								startDate: { $date: new Date() },
+								endDate: { $date: new Date() },
+								rid: testRoom._id,
+								users: [],
+							},
+						],
+						id: '14',
+						msg: 'method',
+					}),
+				})
+				.expect(200)
+				.expect((res) => {
+					const result = JSON.parse(res.body.message);
+					expect(result).to.have.property('error');
+					expect(result.error).to.have.property('error', `Room doesn't exist`);
+				});
+		});
+
 		it('PUT room attribute should replace values and keep inUse=true', async () => {
 			await request
 				.put(`${v1}/abac/rooms/${testRoom._id}/attributes/${updatedKey}`)

ee/packages/abac/src/audit.ts

@@ -9,6 +9,7 @@ import type {
 	AbacAuditReason,
 	MinimalRoom,
 	MinimalUser,
+	AbacActionPerformed,
 } from '@rocket.chat/core-typings';
 import { ServerEvents } from '@rocket.chat/models';
 
@@ -116,11 +117,16 @@ export const Audit = {
 			{ type: 'user', _id: actor._id, username: actor.username!, ip: '0.0.0.0', useragent: '' },
 		);
 	},
-	actionPerformed: async (subject: MinimalUser, object: MinimalRoom, reason: AbacAuditReason = 'room-attributes-change') => {
+	actionPerformed: async (
+		subject: MinimalUser,
+		object: MinimalRoom,
+		reason: AbacAuditReason = 'room-attributes-change',
+		actionPerformed: AbacActionPerformed = 'revoked-object-access',
+	) => {
 		return audit(
 			'abac.action.performed',
 			{
-				action: 'revoked-object-access',
+				action: actionPerformed,
 				reason,
 				subject,
 				object,

ee/packages/abac/src/index.ts

@@ -461,7 +461,7 @@ export class AbacService extends ServiceClass implements IAbacService {
 		await this.onRoomAttributesChanged(room, updated?.abacAttributes || []);
 	}
 
-	async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[]): Promise<void> {
+	async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], objectId: string): Promise<void> {
 		if (!usernames.length || !attributes.length) {
 			return;
 		}
@@ -486,9 +486,8 @@ export class AbacService extends ServiceClass implements IAbacService {
 			);
 		}
 
-		this.logger.debug({
-			msg: 'User list complied with ABAC attributes for room',
-			usernames,
+		usernames.forEach((username) => {
+			void Audit.actionPerformed({ username }, { _id: objectId }, 'system', 'granted-object-access');
 		});
 	}
 

ee/packages/abac/src/service.spec.ts

@@ -21,6 +21,7 @@ const mockUsersUpdateOne = jest.fn();
 const mockUsersSetAbacAttributesById = jest.fn();
 const mockUsersUnsetAbacAttributesById = jest.fn();
 const mockAbacFindOneAndUpdate = jest.fn();
+const mockCreateAuditServerEvent = jest.fn();
 
 jest.mock('@rocket.chat/models', () => ({
 	Rooms: {
@@ -53,7 +54,7 @@ jest.mock('@rocket.chat/models', () => ({
 		updateOne: (...args: any[]) => mockUsersUpdateOne(...args),
 	},
 	ServerEvents: {
-		createAuditServerEvent: jest.fn(),
+		createAuditServerEvent: (...args: any[]) => mockCreateAuditServerEvent(...args),
 	},
 }));
 
@@ -1030,17 +1031,18 @@ describe('AbacService (unit)', () => {
 	describe('checkUsernamesMatchAttributes', () => {
 		beforeEach(() => {
 			mockUsersFind.mockReset();
+			mockCreateAuditServerEvent.mockReset();
 		});
 
 		const attributes = [{ key: 'dept', values: ['eng'] }];
 
 		it('returns early (no query) when usernames array is empty', async () => {
-			await expect(service.checkUsernamesMatchAttributes([], attributes as any)).resolves.toBeUndefined();
+			await expect(service.checkUsernamesMatchAttributes([], attributes as any, 'objectId')).resolves.toBeUndefined();
 			expect(mockUsersFind).not.toHaveBeenCalled();
 		});
 
 		it('returns early (no query) when attributes array is empty', async () => {
-			await expect(service.checkUsernamesMatchAttributes(['alice'], [])).resolves.toBeUndefined();
+			await expect(service.checkUsernamesMatchAttributes(['alice'], [], 'objectId')).resolves.toBeUndefined();
 			expect(mockUsersFind).not.toHaveBeenCalled();
 		});
 
@@ -1052,7 +1054,7 @@ describe('AbacService (unit)', () => {
 				}),
 			}));
 
-			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any)).resolves.toBeUndefined();
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).resolves.toBeUndefined();
 
 			expect(mockUsersFind).toHaveBeenCalledWith(
 				{
@@ -1083,11 +1085,44 @@ describe('AbacService (unit)', () => {
 				}),
 			}));
 
-			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any)).rejects.toMatchObject({
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).rejects.toMatchObject({
 				error: 'error-usernames-not-matching-abac-attributes',
 				message: expect.stringContaining('[error-usernames-not-matching-abac-attributes]'),
 				details: expect.arrayContaining(['bob', 'charlie']),
 			});
 		});
+
+		it('generates an audit log for every compliant username', async () => {
+			const usernames = ['alice', 'bob'];
+
+			mockUsersFind.mockImplementationOnce(() => ({
+				map: () => ({
+					toArray: async () => [],
+				}),
+			}));
+
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).resolves.toBeUndefined();
+
+			expect(mockCreateAuditServerEvent).toHaveBeenCalledTimes(usernames.length);
+			const calledUsernames = mockCreateAuditServerEvent.mock.calls.map(([, payload]: any[]) => payload?.subject?.username).filter(Boolean);
+			expect(calledUsernames.sort()).toEqual(usernames.sort());
+		});
+
+		it('does not generate audit logs when usernames do not match attributes', async () => {
+			const usernames = ['alice', 'bob', 'charlie'];
+			const nonCompliantDocs = [{ username: 'alice' }, { username: 'bob' }, { username: 'charlie' }];
+
+			mockUsersFind.mockImplementationOnce(() => ({
+				map: (fn: (u: any) => string) => ({
+					toArray: async () => nonCompliantDocs.map(fn),
+				}),
+			}));
+
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any, 'objectId')).rejects.toMatchObject({
+				error: 'error-usernames-not-matching-abac-attributes',
+			});
+
+			expect(mockCreateAuditServerEvent).not.toHaveBeenCalled();
+		});
 	});
 });

packages/core-services/src/types/IAbacService.ts

@@ -38,7 +38,7 @@ export interface IAbacService {
 	removeRoomAbacAttribute(rid: string, key: string, actor: AbacActor | undefined): Promise<void>;
 	addRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor | undefined): Promise<void>;
 	replaceRoomAbacAttributeByKey(rid: string, key: string, values: string[], actor: AbacActor | undefined): Promise<void>;
-	checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[]): Promise<void>;
+	checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[], objectId: string): Promise<void>;
 	canAccessObject(
 		room: Pick<IRoom, '_id' | 't' | 'teamId' | 'prid' | 'abacAttributes'>,
 		user: Pick<IUser, '_id'>,

packages/core-typings/src/ServerAudit/IAuditServerAbacAction.ts

@@ -1,10 +1,12 @@
-import type { IUser, IRoom, IAuditServerEventType, IAbacAttributeDefinition, IServerEvents } from '..';
+import type { IUser, IRoom, IAuditServerEventType, IAbacAttributeDefinition, IServerEvents, Optional } from '..';
 
-export type MinimalUser = Pick<IUser, '_id' | 'username'>;
+export type MinimalUser = Pick<IUser, 'username'> & Optional<Pick<IUser, '_id'>, '_id'>;
 export type MinimalRoom = Pick<IRoom, '_id' | 'name'>;
 
 export type AbacAuditReason = 'ldap-sync' | 'room-attributes-change' | 'system' | 'api' | 'realtime-policy-eval';
 
+export type AbacActionPerformed = 'revoked-object-access' | 'granted-object-access';
+
 export type AbacAttributeDefinitionChangeType =
 	| 'created'
 	| 'updated'
@@ -54,7 +56,7 @@ interface IServerEventAbacAttributeChanged
 
 interface IServerEventAbacActionPerformed
 	extends IAuditServerEventType<
-		| { key: 'action'; value: 'revoked-object-access' }
+		| { key: 'action'; value: AbacActionPerformed }
 		| { key: 'reason'; value: AbacAuditReason }
 		| { key: 'subject'; value: MinimalUser | undefined }
 		| { key: 'object'; value: MinimalRoom | undefined }