fix: team conversion permissions checked incorrectly

.changeset/quiet-teams-guard.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Ensures room permission checks are applied consistently regardless of how the room is identified when converting a channel to a team or creating a team from an existing room

apps/meteor/server/api/v1/channels.ts

@@ -46,7 +46,7 @@ import { executeUnarchiveRoom } from '../../../app/lib/server/methods/unarchiveR
 import { getUserMentionsByChannel } from '../../../app/mentions/server/methods/getUserMentionsByChannel';
 import { settings } from '../../../app/settings/server';
 import { normalizeMessagesForUser } from '../../../app/utils/server/lib/normalizeMessagesForUser';
-import { hasPermissionAsync } from '../../lib/authorization/hasPermission';
+import { hasAllPermissionAsync, hasPermissionAsync } from '../../lib/authorization/hasPermission';
 import { eraseRoom } from '../../lib/eraseRoom';
 import { findUsersOfRoom } from '../../lib/findUsersOfRoom';
 import { openRoom } from '../../lib/openRoom';
@@ -522,10 +522,6 @@ API.v1.addRoute(
 				return API.v1.failure('The parameter "channelId" or "channelName" is required');
 			}
 
-			if (channelId && !(await hasPermissionAsync(this.userId, 'edit-room', channelId))) {
-				return API.v1.forbidden();
-			}
-
 			const room = await findChannelByIdOrName({
 				params: channelId !== undefined ? { roomId: channelId } : { roomName: channelName },
 				userId: this.userId,
@@ -535,6 +531,10 @@ API.v1.addRoute(
 				return API.v1.failure('Channel not found');
 			}
 
+			if (!(await hasAllPermissionAsync(this.userId, ['create-team', 'edit-room'], room._id))) {
+				return API.v1.forbidden();
+			}
+
 			const subscriptions = await Subscriptions.findByRoomId(room._id, {
 				projection: { 'u._id': 1 },
 			});

apps/meteor/server/api/v1/teams.ts

@@ -30,7 +30,7 @@ import { escapeRegExp } from '@rocket.chat/string-helpers';
 
 import { canAccessRoomAsync } from '../../../app/authorization/server';
 import { settings } from '../../../app/settings/server';
-import { hasPermissionAsync, hasAtLeastOnePermissionAsync } from '../../lib/authorization/hasPermission';
+import { hasPermissionAsync, hasAtLeastOnePermissionAsync, hasAllPermissionAsync } from '../../lib/authorization/hasPermission';
 import { eraseRoom } from '../../lib/eraseRoom';
 import { removeUserFromRoom } from '../../lib/rooms/removeUserFromRoom';
 import type { ExtractRoutesFromAPI } from '../ApiClass';
@@ -125,11 +125,16 @@ const teamsEndpoints = API.v1
 				}),
 				400: validateBadRequestErrorResponse,
 				401: validateUnauthorizedErrorResponse,
+				403: validateForbiddenErrorResponse,
 			},
 		},
 		async function action() {
 			const { name, type, members, room, owner } = this.bodyParams;
 
+			if (room?.id && !(await hasAllPermissionAsync(this.userId, ['create-team', 'edit-room'], room.id))) {
+				return API.v1.forbidden();
+			}
+
 			const team = await Team.create(this.userId, {
 				team: {
 					name,

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

@@ -3991,6 +3991,36 @@ describe('[Channels]', () => {
 				});
 		});
 
+		describe('when a user without edit-room permission on the channel tries to convert it to a team', () => {
+			let outsiderChannel: IRoom;
+			let outsiderUser: TestUser<IUser>;
+			let outsiderCredentials: Credentials;
+
+			before(async () => {
+				await updatePermission('create-team', ['admin', 'user']);
+				await updatePermission('edit-room', ['admin', 'owner', 'moderator']);
+
+				outsiderChannel = (await createRoom({ type: 'c', name: `channel.convertToTeam.outsider.test.${Date.now()}` })).body.channel;
+				outsiderUser = await createUser();
+				outsiderCredentials = await login(outsiderUser.username, password);
+			});
+
+			after(async () => {
+				await Promise.all([deleteRoom({ type: 'c', roomId: outsiderChannel._id }), deleteUser(outsiderUser)]);
+			});
+
+			it('should return 403 when using channelName', async () => {
+				await request
+					.post(api('channels.convertToTeam'))
+					.set(outsiderCredentials)
+					.send({ channelName: outsiderChannel.name })
+					.expect(403)
+					.expect((res) => {
+						expect(res.body).to.have.a.property('success', false);
+					});
+			});
+		});
+
 		it(`should return an error when the channel's name and id are sent as parameter`, (done) => {
 			void request
 				.post(api('channels.convertToTeam'))

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

@@ -249,6 +249,63 @@ describe('[Teams]', () => {
 				});
 		});
 	});
+
+	describe('/teams.create - existing room ownership check', () => {
+		let roomOwner: TestUser<IUser>;
+		let roomOwnerCredentials: Credentials;
+		let attacker: TestUser<IUser>;
+		let attackerCredentials: Credentials;
+		let targetRoom: IRoom;
+		const teamName = `test-team-hijack-${Date.now()}`;
+
+		before(async () => {
+			[roomOwner, attacker] = await Promise.all([createUser(), createUser()]);
+			[roomOwnerCredentials, attackerCredentials] = await Promise.all([
+				login(roomOwner.username, password),
+				login(attacker.username, password),
+			]);
+			targetRoom = (await createRoom({ type: 'c', name: `test-room-hijack-${Date.now()}`, credentials: roomOwnerCredentials })).body
+				.channel;
+		});
+
+		before(() => updatePermission('create-team', ['admin', 'user']));
+
+		after(async () => {
+			await Promise.all([
+				deleteRoom({ type: 'c', roomId: targetRoom._id }),
+				deleteUser(roomOwner),
+				deleteUser(attacker),
+				updatePermission('create-team', ['admin', 'user']),
+			]);
+		});
+
+		it('should not allow a user with no ownership/moderation of a room to hijack it into a new team by passing room.id', async () => {
+			await request
+				.post(api('teams.create'))
+				.set(attackerCredentials)
+				.send({
+					name: teamName,
+					type: 0,
+					room: { id: targetRoom._id },
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(403)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', false);
+				});
+
+			await request
+				.get(api('channels.info'))
+				.set(credentials)
+				.query({ roomId: targetRoom._id })
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body.channel).to.not.have.property('teamId');
+					expect(res.body.channel).to.not.have.property('teamMain');
+				});
+		});
+	});
 });
 
 describe('/teams.convertToChannel', () => {