feat: Update add/join methods to use abac rules

apps/meteor/app/lib/server/functions/addUserToRoom.ts

@@ -12,6 +12,7 @@ import { getSubscriptionAutotranslateDefaultConfig } from '../../../../server/li
 import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator';
 import { settings } from '../../../settings/server';
 import { getDefaultSubscriptionPref } from '../../../utils/lib/getDefaultSubscriptionPref';
+import { beforeAddUserToRoom as beforeAddUserToRoomPatch } from '../lib/beforeAddUserToRoom';
 import { notifyOnRoomChangedById, notifyOnSubscriptionChangedById } from '../lib/notifyListener';
 
 /**
@@ -57,7 +58,10 @@ export const addUserToRoom = async (
 	}
 
 	try {
-		await beforeAddUserToRoom.run({ user: userToBeAdded, inviter: (inviter && (await Users.findOneById(inviter._id))) || undefined }, room);
+		const inviterUser = inviter && ((await Users.findOneById(inviter._id)) || undefined);
+		// Not "duplicated": we're moving away from callbacks so this is a patch function. We should migrate the next one to be a patch or use this same patch, instead of calling both
+		await beforeAddUserToRoomPatch([userToBeAdded.username!], room, inviterUser);
+		await beforeAddUserToRoom.run({ user: userToBeAdded, inviter: inviterUser }, room);
 	} catch (error) {
 		throw new Meteor.Error((error as any)?.message);
 	}

apps/meteor/app/lib/server/lib/beforeAddUserToRoom.ts

@@ -0,0 +1,6 @@
+import type { IRoom, IUser } from '@rocket.chat/core-typings';
+import { makeFunction } from '@rocket.chat/patch-injection';
+
+export const beforeAddUserToRoom = makeFunction(async (_users: IUser['username'][], _room: IRoom, _actor?: IUser) => {
+	// no op on CE
+});

apps/meteor/app/slashcommands-invite/server/server.ts

@@ -8,6 +8,16 @@ import { addUsersToRoomMethod, sanitizeUsername } from '../../lib/server/methods
 import { settings } from '../../settings/server';
 import { slashCommands } from '../../utils/server/slashCommand';
 
+// Type guards for the error
+
+function isErrorWithDetails(error: unknown): error is Error & { details: string[] } {
+	return error instanceof Error && Array.isArray((error as any).details);
+}
+
+function isStringError(error: unknown): error is { error: string } {
+	return typeof (error as any)?.error === 'string';
+}
+
 /*
  * Invite is a named function that will replace /invite commands
  * @param {Object} message - The message object
@@ -73,23 +83,31 @@ slashCommands.add({
 						},
 						inviter,
 					);
-				} catch ({ error }: any) {
-					if (typeof error !== 'string') {
-						return;
-					}
+				} catch (e: unknown) {
+					if (isErrorWithDetails(e)) {
+						const details = Array.isArray(e.details) ? e.details.join(', ') : '';
 
-					if (error === 'error-federated-users-in-non-federated-rooms') {
-						void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
-							msg: i18n.t('You_cannot_add_external_users_to_non_federated_room', { lng: settings.get('Language') || 'en' }),
-						});
-					} else if (error === 'cant-invite-for-direct-room') {
-						void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
-							msg: i18n.t('Cannot_invite_users_to_direct_rooms', { lng: settings.get('Language') || 'en' }),
-						});
-					} else {
 						void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
-							msg: i18n.t(error, { lng: settings.get('Language') || 'en' }),
+							msg: i18n.t(e.message, { lng: settings.get('Language') || 'en', details: `\`${details}\`` }),
 						});
+						return;
+					}
+
+					if (isStringError(e)) {
+						const { error } = e;
+						if (error === 'error-federated-users-in-non-federated-rooms') {
+							void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
+								msg: i18n.t('You_cannot_add_external_users_to_non_federated_room', { lng: settings.get('Language') || 'en' }),
+							});
+						} else if (error === 'cant-invite-for-direct-room') {
+							void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
+								msg: i18n.t('Cannot_invite_users_to_direct_rooms', { lng: settings.get('Language') || 'en' }),
+							});
+						} else {
+							void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
+								msg: i18n.t(error, { lng: settings.get('Language') || 'en' }),
+							});
+						}
 					}
 				}
 			}),

apps/meteor/ee/server/configuration/abac.ts

@@ -7,5 +7,7 @@ Meteor.startup(async () => {
 
 		await addSettings();
 		await createPermissions();
+
+		await import('../hooks/abac');
 	});
 });

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

@@ -0,0 +1,22 @@
+import { Abac } from '@rocket.chat/core-services';
+import { License } from '@rocket.chat/license';
+
+import { beforeAddUserToRoom } from '../../../../app/lib/server/lib/beforeAddUserToRoom';
+import { settings } from '../../../../app/settings/server';
+
+beforeAddUserToRoom.patch(async (prev, users, room, actor) => {
+	await prev(users, room, actor);
+
+	const validUsers = users.filter(Boolean);
+	if (
+		!room?.abacAttributes?.length ||
+		!validUsers.length ||
+		!License.hasModule('abac') ||
+		room.t !== 'p' ||
+		!settings.get('ABAC_Enabled')
+	) {
+		return;
+	}
+
+	await Abac.checkUsernamesMatchAttributes(validUsers as string[], room.abacAttributes);
+});

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

@@ -0,0 +1 @@
+import './beforeAddUserToRoom';

apps/meteor/server/methods/addAllUserToRoom.ts

@@ -6,6 +6,7 @@ import { check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 
 import { hasPermissionAsync } from '../../app/authorization/server/functions/hasPermission';
+import { beforeAddUserToRoom } from '../../app/lib/server/lib/beforeAddUserToRoom';
 import { notifyOnSubscriptionChangedById } from '../../app/lib/server/lib/notifyListener';
 import { settings } from '../../app/settings/server';
 import { getDefaultSubscriptionPref } from '../../app/utils/lib/getDefaultSubscriptionPref';
@@ -50,6 +51,11 @@ export const addAllUserToRoomFn = async (userId: string, rid: IRoom['_id'], acti
 		});
 	}
 
+	await beforeAddUserToRoom(
+		users.map((u) => u.username!),
+		room,
+	);
+
 	const now = new Date();
 	for await (const user of users) {
 		const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, user._id);

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

@@ -1411,4 +1411,76 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 			});
 		});
 	});
+
+	describe('Room access (invite, addition)', () => {
+		let roomWithoutAttr: IRoom;
+		let roomWithAttr: IRoom;
+		const accessAttrKey = `access_attr_${Date.now()}`;
+
+		before(async () => {
+			await updateSetting('ABAC_Enabled', true);
+
+			await request
+				.post(`${v1}/abac/attributes`)
+				.set(credentials)
+				.send({ key: accessAttrKey, values: ['v1'] })
+				.expect(200);
+
+			// We have to add them directly cause otherwise the abac engine would kick the user from the room after the attribute is added
+			await addAbacAttributesToUserDirectly(credentials['X-User-Id'], [{ key: accessAttrKey, values: ['v1'] }]);
+
+			// Create two private rooms: one will stay without attributes, the other will get the attribute
+			roomWithoutAttr = (await createRoom({ type: 'p', name: `abac-access-noattr-${Date.now()}` })).body.group;
+			roomWithAttr = (await createRoom({ type: 'p', name: `abac-access-withattr-${Date.now()}` })).body.group;
+
+			// Assign the attribute to the second room
+			await request
+				.post(`${v1}/abac/rooms/${roomWithAttr._id}/attributes/${accessAttrKey}`)
+				.set(credentials)
+				.send({ values: ['v1'] })
+				.expect(200);
+		});
+
+		after(async () => {
+			await deleteRoom({ type: 'p', roomId: roomWithoutAttr._id });
+			await deleteRoom({ type: 'p', roomId: roomWithAttr._id });
+		});
+
+		it('INVITE: user without attributes invited to room without attributes succeeds', async () => {
+			await request
+				.post(`${v1}/groups.invite`)
+				.set(credentials)
+				.send({ roomId: roomWithoutAttr._id, usernames: [unauthorizedUser.username] })
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+				});
+		});
+
+		it('INVITE: user without attributes invited to room with attributes should fail', async () => {
+			await request
+				.post(`${v1}/groups.invite`)
+				.set(credentials)
+				.send({ roomId: roomWithAttr._id, usernames: [unauthorizedUser.username] })
+				.expect(400)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', false);
+					expect(res.body).to.have.property('error').that.includes('error-usernames-not-matching-abac-attributes');
+				});
+		});
+
+		it('INVITE: after room loses attributes user without attributes can be invited', async () => {
+			await request.delete(`${v1}/abac/rooms/${roomWithAttr._id}/attributes/${accessAttrKey}`).set(credentials).expect(200);
+
+			// Try inviting again - should now succeed
+			await request
+				.post(`${v1}/groups.invite`)
+				.set(credentials)
+				.send({ roomId: roomWithAttr._id, usernames: [unauthorizedUser.username] })
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+				});
+		});
+	});
 });

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

@@ -14,6 +14,7 @@ const mockUpdateSingleAbacAttributeValuesById = jest.fn();
 const mockUpdateAbacAttributeValuesArrayFilteredById = jest.fn();
 const mockRemoveAbacAttributeByRoomIdAndKey = jest.fn();
 const mockInsertAbacAttributeIfNotExistsById = jest.fn();
+const mockUsersFind = jest.fn();
 
 jest.mock('@rocket.chat/models', () => ({
 	Rooms: {
@@ -37,6 +38,9 @@ jest.mock('@rocket.chat/models', () => ({
 		removeById: (...args: any[]) => mockAbacDeleteOne(...args),
 		find: (...args: any[]) => mockAbacFind(...args),
 	},
+	Users: {
+		find: (...args: any[]) => mockUsersFind(...args),
+	},
 }));
 
 // Minimal mock for ServiceClass (we don't need its real behavior in unit scope)
@@ -815,4 +819,120 @@ describe('AbacService (unit)', () => {
 			});
 		});
 	});
+
+	describe('checkUsernamesMatchAttributes', () => {
+		beforeEach(() => {
+			mockUsersFind.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();
+			expect(mockUsersFind).not.toHaveBeenCalled();
+		});
+
+		it('returns early (no query) when attributes array is empty', async () => {
+			await expect(service.checkUsernamesMatchAttributes(['alice'], [])).resolves.toBeUndefined();
+			expect(mockUsersFind).not.toHaveBeenCalled();
+		});
+
+		it('resolves when all provided usernames are compliant (query returns empty)', async () => {
+			const usernames = ['alice', 'bob'];
+			mockUsersFind.mockImplementationOnce(() => ({
+				map: () => ({
+					toArray: async () => [],
+				}),
+			}));
+
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any)).resolves.toBeUndefined();
+
+			expect(mockUsersFind).toHaveBeenCalledWith(
+				{
+					username: { $in: usernames },
+					$or: [
+						{
+							abacAttributes: {
+								$not: {
+									$elemMatch: {
+										key: 'dept',
+										values: { $all: ['eng'] },
+									},
+								},
+							},
+						},
+					],
+				},
+				{ projection: { username: 1 } },
+			);
+		});
+
+		it('rejects with error-usernames-not-matching-abac-attributes and details for non-compliant users', async () => {
+			const usernames = ['alice', 'bob', 'charlie'];
+			const nonCompliantDocs = [{ username: 'bob' }, { username: 'charlie' }];
+			mockUsersFind.mockImplementationOnce(() => ({
+				map: (fn: (u: any) => string) => ({
+					toArray: async () => nonCompliantDocs.map(fn),
+				}),
+			}));
+
+			await expect(service.checkUsernamesMatchAttributes(usernames, attributes as any)).rejects.toMatchObject({
+				message: 'error-usernames-not-matching-abac-attributes',
+				details: expect.arrayContaining(['bob', 'charlie']),
+			});
+		});
+	});
+	describe('buildNonCompliantConditions (private)', () => {
+		it('returns empty array for empty attributes list', () => {
+			const result = (service as any).buildNonCompliantConditions([]);
+			expect(result).toEqual([]);
+		});
+
+		it('maps single attribute to $not $elemMatch query', () => {
+			const attrs = [{ key: 'dept', values: ['eng', 'sales'] }];
+			const result = (service as any).buildNonCompliantConditions(attrs);
+			expect(result).toEqual([
+				{
+					abacAttributes: {
+						$not: {
+							$elemMatch: {
+								key: 'dept',
+								values: { $all: ['eng', 'sales'] },
+							},
+						},
+					},
+				},
+			]);
+		});
+
+		it('maps multiple attributes preserving order', () => {
+			const attrs = [
+				{ key: 'dept', values: ['eng'] },
+				{ key: 'region', values: ['emea', 'apac'] },
+			];
+			const result = (service as any).buildNonCompliantConditions(attrs);
+			expect(result).toEqual([
+				{
+					abacAttributes: {
+						$not: {
+							$elemMatch: {
+								key: 'dept',
+								values: { $all: ['eng'] },
+							},
+						},
+					},
+				},
+				{
+					abacAttributes: {
+						$not: {
+							$elemMatch: {
+								key: 'region',
+								values: { $all: ['emea', 'apac'] },
+							},
+						},
+					},
+				},
+			]);
+		});
+	});
 });

ee/packages/abac/src/index.ts

@@ -442,6 +442,38 @@ export class AbacService extends ServiceClass implements IAbacService {
 		}));
 	}
 
+	async checkUsernamesMatchAttributes(usernames: string[], attributes: IAbacAttributeDefinition[]): Promise<void> {
+		if (!usernames.length || !attributes.length) {
+			return;
+		}
+
+		const nonComplianceConditions = this.buildNonCompliantConditions(attributes);
+		const nonCompliantUsersFromList = await Users.find(
+			{
+				username: { $in: usernames },
+				$or: nonComplianceConditions,
+			},
+			{ projection: { username: 1 } },
+		)
+			.map((u) => u.username as string)
+			.toArray();
+
+		const nonCompliantSet = new Set<string>(nonCompliantUsersFromList);
+
+		if (nonCompliantSet.size) {
+			// Note: open to suggestions, or if it's actually needed. My idea is to return the list of non compliant users, but our current errors dont' allow that
+			// Maybe we should just throw a generic error? idk. I may create a custom error just for this
+			const err = new Error('error-usernames-not-matching-abac-attributes');
+			(err as any).details = Array.from(nonCompliantSet);
+			throw err;
+		}
+
+		this.logger.debug({
+			msg: 'User list complied with ABAC attributes for room',
+			usernames,
+		});
+	}
+
 	protected async onRoomAttributesChanged(
 		room: AtLeast<IRoom, '_id' | 't' | 'teamMain' | 'abacAttributes'>,
 		newAttributes: IAbacAttributeDefinition[],