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/services/team/service.ts

@@ -40,6 +40,10 @@ export class TeamService extends ServiceClassInternal implements ITeamService {
 	protected name = 'team';
 
 	async create(uid: string, { team, room = { name: team.name, extraData: {} }, members, owner }: ITeamCreateParams): Promise<ITeam> {
+		if (room.id && !(await Authorization.hasAllPermission(uid, ['create-team', 'edit-room'], room.id))) {
+			throw new Error('error-no-owner-channel');
+		}
+
 		if (!(await checkUsernameAvailability(team.name))) {
 			throw new Error('team-name-already-exists');
 		}

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

@@ -3991,6 +3991,29 @@ describe('[Channels]', () => {
 				});
 		});
 
+		it('should return 403 when a user without edit-room permission on the channel tries to convert it to a team using channelName', async () => {
+			await updatePermission('create-team', ['admin', 'user']);
+			await updatePermission('edit-room', ['admin', 'owner', 'moderator']);
+
+			const outsiderChannel = (await createRoom({ type: 'c', name: `channel.convertToTeam.outsider.test.${Date.now()}` })).body.channel;
+			const outsiderUser = await createUser();
+			const outsiderCredentials = await login(outsiderUser.username, password);
+
+			try {
+				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);
+					});
+			} finally {
+				await deleteRoom({ type: 'c', roomId: outsiderChannel._id });
+				await deleteUser(outsiderUser);
+			}
+		});
+
 		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,64 @@ 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(400)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', false);
+					expect(res.body).to.have.property('error', 'error-no-owner-channel');
+				});
+
+			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', () => {