fix: [ENTERPRISE] Guest users can join more than maxRoomsPerGuest rooms (#26400)

.changeset/shiny-donkeys-prove.md

@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/model-typings": patch
+---
+
+fix: [ENTERPRISE] Guest users can join more than maxRoomsPerGuest rooms

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

@@ -26,6 +26,11 @@ export const addUserToRoom = async function (
 
 	const userToBeAdded = typeof user !== 'string' ? user : await Users.findOneByUsername(user.replace('@', ''));
 	const roomDirectives = roomCoordinator.getRoomDirectives(room.t);
+
+	if (!userToBeAdded) {
+		throw new Meteor.Error('user-not-found');
+	}
+
 	if (
 		!(await roomDirectives.allowMemberAction(room, RoomMemberActions.JOIN, userToBeAdded._id)) &&
 		!(await roomDirectives.allowMemberAction(room, RoomMemberActions.INVITE, userToBeAdded._id))
@@ -39,6 +44,8 @@ export const addUserToRoom = async function (
 		throw new Meteor.Error((error as any)?.message);
 	}
 
+	await callbacks.run('beforeAddedToRoom', { user: userToBeAdded, inviter: userToBeAdded });
+
 	// Check if user is already in room
 	const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, userToBeAdded._id);
 	if (subscription || !userToBeAdded) {

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

@@ -144,7 +144,7 @@ export const createRoom = async <T extends RoomType>(
 	} else {
 		for await (const username of [...new Set(members)]) {
 			const member = await Users.findOneByUsername(username, {
-				projection: { 'username': 1, 'settings.preferences': 1, 'federated': 1 },
+				projection: { 'username': 1, 'settings.preferences': 1, 'federated': 1, 'roles': 1 },
 			});
 			if (!member) {
 				continue;
@@ -152,6 +152,7 @@ export const createRoom = async <T extends RoomType>(
 
 			try {
 				await callbacks.run('federation.beforeAddUserToARoom', { user: member, inviter: owner }, room);
+				await callbacks.run('beforeAddedToRoom', { user: member, inviter: owner });
 			} catch (error) {
 				continue;
 			}

apps/meteor/ee/app/license/server/license.ts

@@ -21,6 +21,7 @@ interface IValidLicense {
 }
 
 let maxGuestUsers = 0;
+let maxRoomsPerGuest = 0;
 let maxActiveUsers = 0;
 
 class LicenseClass {
@@ -192,6 +193,10 @@ class LicenseClass {
 				maxGuestUsers = license.maxGuestUsers;
 			}
 
+			if (license.maxRoomsPerGuest > maxRoomsPerGuest) {
+				maxRoomsPerGuest = license.maxRoomsPerGuest;
+			}
+
 			if (license.maxActiveUsers > maxActiveUsers) {
 				maxActiveUsers = license.maxActiveUsers;
 			}
@@ -323,6 +328,10 @@ export function getMaxGuestUsers(): number {
 	return maxGuestUsers;
 }
 
+export function getMaxRoomsPerGuest(): number {
+	return maxRoomsPerGuest;
+}
+
 export function getMaxActiveUsers(): number {
 	return maxActiveUsers;
 }

apps/meteor/ee/server/startup/index.ts

@@ -3,6 +3,7 @@ import './apps';
 import './audit';
 import './deviceManagement';
 import './engagementDashboard';
+import './maxRoomsPerGuest';
 import './seatsCap';
 import './services';
 import './upsell';

apps/meteor/ee/server/startup/maxRoomsPerGuest.ts

@@ -0,0 +1,21 @@
+import { Meteor } from 'meteor/meteor';
+import { Subscriptions } from '@rocket.chat/models';
+
+import { callbacks } from '../../../lib/callbacks';
+import { getMaxRoomsPerGuest } from '../../app/license/server/license';
+import { i18n } from '../../../server/lib/i18n';
+
+callbacks.add(
+	'beforeAddedToRoom',
+	async ({ user }) => {
+		if (user.roles?.includes('guest')) {
+			const totalSubscriptions = await Subscriptions.countByUserId(user._id);
+
+			if (totalSubscriptions >= getMaxRoomsPerGuest()) {
+				throw new Meteor.Error('error-max-rooms-per-guest-reached', i18n.t('error-max-rooms-per-guest-reached'));
+			}
+		}
+	},
+	callbacks.priority.MEDIUM,
+	'check-max-rooms-per-guest',
+);

apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json

@@ -1993,6 +1993,7 @@
   "error-max-departments-number-reached": "You reached the maximum number of departments allowed by your license. Contact sale@rocket.chat for a new license.",
   "error-max-guests-number-reached": "You reached the maximum number of guest users allowed by your license. Contact sale@rocket.chat for a new license.",
   "error-max-number-simultaneous-chats-reached": "The maximum number of simultaneous chats per agent has been reached.",
+  "error-max-rooms-per-guest-reached": "The maximum number of rooms per guest has been reached.",
   "error-message-deleting-blocked": "Message deleting is blocked",
   "error-message-editing-blocked": "Message editing is blocked",
   "error-message-size-exceeded": "Message size exceeds Message_MaxAllowedSize",

apps/meteor/server/models/raw/Subscriptions.ts

@@ -953,6 +953,12 @@ export class SubscriptionsRaw extends BaseRaw<ISubscription> implements ISubscri
 		return this.col.countDocuments(query);
 	}
 
+	countByUserId(userId: string): Promise<number> {
+		const query = { 'u._id': userId };
+
+		return this.col.countDocuments(query);
+	}
+
 	countByRoomId(roomId: string): Promise<number> {
 		const query = {
 			rid: roomId,

apps/meteor/tests/data/constants.ts

@@ -0,0 +1 @@
+export const CI_MAX_ROOMS_PER_GUEST = 10;

apps/meteor/tests/end-to-end/api/02-channels.js

@@ -2,11 +2,12 @@ import { expect } from 'chai';
 
 import { getCredentials, api, request, credentials, apiPublicChannelName, channel, reservedWords } from '../../data/api-data.js';
 import { adminUsername, password } from '../../data/user';
-import { createUser, login } from '../../data/users.helper';
+import { createUser, login, deleteUser } from '../../data/users.helper';
 import { updatePermission, updateSetting } from '../../data/permissions.helper';
 import { createRoom } from '../../data/rooms.helper';
 import { createIntegration, removeIntegration } from '../../data/integration.helper';
 import { testFileUploads } from '../../data/uploads.helper';
+import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants';
 
 function getRoomInfo(roomId) {
 	return new Promise((resolve /* , reject*/) => {
@@ -48,6 +49,66 @@ describe('[Channels]', function () {
 			.end(done);
 	});
 
+	describe('[/channels.create]', () => {
+		let guestUser;
+		let room;
+
+		before(async () => {
+			guestUser = await createUser({ roles: ['guest'] });
+		});
+		after(async () => {
+			await deleteUser(guestUser);
+		});
+
+		it('should not add guest users to more rooms than defined in the license', async function () {
+			// TODO this is not the right way to do it. We're doing this way for now just because we have separate CI jobs for EE and CE,
+			// ideally we should have a single CI job that adds a license and runs both CE and EE tests.
+			if (!process.env.IS_EE) {
+				this.skip();
+			}
+
+			const promises = [];
+			for (let i = 0; i < maxRoomsPerGuest; i++) {
+				promises.push(
+					createRoom({
+						type: 'c',
+						name: `channel.test.${Date.now()}-${Math.random()}`,
+						members: [guestUser.username],
+					}),
+				);
+			}
+			await Promise.all(promises);
+
+			request
+				.post(api('channels.create'))
+				.set(credentials)
+				.send({
+					name: `channel.test.${Date.now()}-${Math.random()}`,
+					members: [guestUser.username],
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					room = res.body.group;
+				})
+				.then(() => {
+					request
+						.get(api('channels.members'))
+						.set(credentials)
+						.query({
+							roomId: room._id,
+						})
+						.expect('Content-Type', 'application/json')
+						.expect(200)
+						.expect((res) => {
+							expect(res.body).to.have.property('success', true);
+							expect(res.body).to.have.property('members').and.to.be.an('array');
+							expect(res.body.members).to.have.lengthOf(1);
+						});
+				});
+		});
+	});
 	describe('[/channels.info]', () => {
 		let testChannel = {};
 		let channelMessage = {};

apps/meteor/tests/end-to-end/api/03-groups.js

@@ -2,11 +2,12 @@ import { expect } from 'chai';
 
 import { getCredentials, api, request, credentials, group, apiPrivateChannelName } from '../../data/api-data.js';
 import { adminUsername, password } from '../../data/user';
-import { createUser, login } from '../../data/users.helper';
+import { createUser, login, deleteUser } from '../../data/users.helper';
 import { updatePermission, updateSetting } from '../../data/permissions.helper';
 import { createRoom } from '../../data/rooms.helper';
 import { createIntegration, removeIntegration } from '../../data/integration.helper';
 import { testFileUploads } from '../../data/uploads.helper';
+import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants';
 
 function getRoomInfo(roomId) {
 	return new Promise((resolve /* , reject*/) => {
@@ -47,6 +48,66 @@ describe('[Groups]', function () {
 			})
 			.end(done);
 	});
+	describe('[/groups.create]', () => {
+		let guestUser;
+		let room;
+
+		before(async () => {
+			guestUser = await createUser({ roles: ['guest'] });
+		});
+		after(async () => {
+			await deleteUser(guestUser);
+		});
+
+		it('should not add guest users to more rooms than defined in the license', async function () {
+			// TODO this is not the right way to do it. We're doing this way for now just because we have separate CI jobs for EE and CE,
+			// ideally we should have a single CI job that adds a license and runs both CE and EE tests.
+			if (!process.env.IS_EE) {
+				this.skip();
+			}
+			const promises = [];
+
+			for (let i = 0; i < maxRoomsPerGuest; i++) {
+				promises.push(
+					createRoom({
+						type: 'p',
+						name: `channel.test.${Date.now()}-${Math.random()}`,
+						members: [guestUser.username],
+					}),
+				);
+			}
+			await Promise.all(promises);
+
+			request
+				.post(api('groups.create'))
+				.set(credentials)
+				.send({
+					name: `channel.test.${Date.now()}-${Math.random()}`,
+					members: [guestUser.username],
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					room = res.body.group;
+				})
+				.then(() => {
+					request
+						.get(api('groups.members'))
+						.set(credentials)
+						.query({
+							roomId: room._id,
+						})
+						.expect('Content-Type', 'application/json')
+						.expect(200)
+						.expect((res) => {
+							expect(res.body).to.have.property('success', true);
+							expect(res.body).to.have.property('members').and.to.be.an('array');
+							expect(res.body.members).to.have.lengthOf(1);
+						});
+				});
+		});
+	});
 	describe('/groups.create (encrypted)', () => {
 		it('should create a new encrypted group', async () => {
 			await request

apps/meteor/tests/end-to-end/api/24-methods.js

@@ -1,7 +1,10 @@
 import { expect } from 'chai';
 
 import { getCredentials, request, methodCall, api, credentials } from '../../data/api-data.js';
+import { createUser, deleteUser } from '../../data/users.helper.js';
+import { createRoom } from '../../data/rooms.helper';
 import { updatePermission, updateSetting } from '../../data/permissions.helper';
+import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants';
 
 describe('Meteor.methods', function () {
 	this.retries(0);
@@ -2284,4 +2287,113 @@ describe('Meteor.methods', function () {
 				});
 		});
 	});
+
+	describe('[@addUsersToRoom]', () => {
+		let guestUser;
+		let user;
+		let room;
+
+		before(async () => {
+			guestUser = await createUser({ roles: ['guest'] });
+			user = await createUser();
+			room = (
+				await createRoom({
+					type: 'c',
+					name: `channel.test.${Date.now()}-${Math.random()}`,
+				})
+			).body.channel;
+		});
+		after(async () => {
+			await deleteUser(user);
+			await deleteUser(guestUser);
+			user = undefined;
+		});
+
+		it('should fail if not logged in', (done) => {
+			request
+				.post(methodCall('addUsersToRoom'))
+				.expect('Content-Type', 'application/json')
+				.expect(401)
+				.expect((res) => {
+					expect(res.body).to.have.property('status', 'error');
+					expect(res.body).to.have.property('message');
+				})
+				.end(done);
+		});
+
+		it('should add a single user to a room', (done) => {
+			request
+				.post(methodCall('addUsersToRoom'))
+				.set(credentials)
+				.send({
+					message: JSON.stringify({
+						method: 'addUsersToRoom',
+						params: [{ rid: room._id, users: [user.username] }],
+						id: 'id',
+						msg: 'method',
+					}),
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+				})
+				.then(() => {
+					request
+						.get(api('channels.members'))
+						.set(credentials)
+						.query({
+							roomId: room._id,
+						})
+						.expect('Content-Type', 'application/json')
+						.expect(200)
+						.expect((res) => {
+							expect(res.body).to.have.property('success', true);
+							expect(res.body).to.have.property('members').and.to.be.an('array');
+							expect(res.body.members).to.have.lengthOf(2);
+						})
+						.end(done);
+				})
+				.catch(done);
+		});
+
+		it('should not add guest users to more rooms than defined in the license', async function () {
+			// TODO this is not the right way to do it. We're doing this way for now just because we have separate CI jobs for EE and CE,
+			// ideally we should have a single CI job that adds a license and runs both CE and EE tests.
+			if (!process.env.IS_EE) {
+				this.skip();
+			}
+			const promises = [];
+			for (let i = 0; i < maxRoomsPerGuest; i++) {
+				promises.push(
+					createRoom({
+						type: 'c',
+						name: `channel.test.${Date.now()}-${Math.random()}`,
+						members: [guestUser.username],
+					}),
+				);
+			}
+			await Promise.all(promises);
+
+			request
+				.post(methodCall('addUsersToRoom'))
+				.set(credentials)
+				.send({
+					message: JSON.stringify({
+						method: 'addUsersToRoom',
+						params: [{ rid: room._id, users: [guestUser.username] }],
+						id: 'id',
+						msg: 'method',
+					}),
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					const parsedBody = JSON.parse(res.body.message);
+					expect(parsedBody).to.have.property('error');
+					expect(parsedBody.error).to.have.property('error', 'error-max-rooms-per-guest-reached');
+				});
+		});
+	});
 });

packages/model-typings/src/models/ISubscriptionsModel.ts

@@ -232,6 +232,7 @@ export interface ISubscriptionsModel extends IBaseModel<ISubscription> {
 	removeUnreadThreadsByRoomId(rid: string, tunread: string[]): Promise<UpdateResult | Document>;
 	countByRoomIdAndRoles(roomId: string, roles: string[]): Promise<number>;
 	countByRoomId(roomId: string): Promise<number>;
+	countByUserId(userId: string): Promise<number>;
 	openByRoomIdAndUserId(roomId: string, userId: string): Promise<UpdateResult>;
 	countByRoomIdAndNotUserId(rid: string, uid: string): Promise<number>;
 	countByRoomIdWhenUsernameExists(rid: string): Promise<number>;