From 933aa3e7b3f6eefc51dbbfd3aa2a2b2c46cd1de1 Mon Sep 17 00:00:00 2001 From: Julio Araujo Date: Wed, 8 Jul 2026 08:00:44 +0200 Subject: [PATCH 1/2] fix: team conversion permissions checked incorrectly (#41206) --- .changeset/quiet-teams-guard.md | 5 ++ apps/meteor/app/api/server/v1/channels.ts | 10 ++-- apps/meteor/app/api/server/v1/teams.ts | 7 ++- apps/meteor/tests/end-to-end/api/channels.ts | 30 +++++++++++ apps/meteor/tests/end-to-end/api/teams.ts | 57 ++++++++++++++++++++ 5 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 .changeset/quiet-teams-guard.md diff --git a/.changeset/quiet-teams-guard.md b/.changeset/quiet-teams-guard.md new file mode 100644 index 0000000000000..ac9a8af178b0a --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/api/server/v1/channels.ts b/apps/meteor/app/api/server/v1/channels.ts index 44be9f89b54cc..5cd2e327dc17f 100644 --- a/apps/meteor/app/api/server/v1/channels.ts +++ b/apps/meteor/app/api/server/v1/channels.ts @@ -46,7 +46,7 @@ import { removeRoomModerator } from '../../../../server/methods/removeRoomModera import { removeRoomOwner } from '../../../../server/methods/removeRoomOwner'; import { removeUserFromRoomMethod } from '../../../../server/methods/removeUserFromRoom'; import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import { hasAllPermissionAsync, hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { saveRoomSettings } from '../../../channel-settings/server/methods/saveRoomSettings'; import { mountIntegrationQueryBasedOnPermissions } from '../../../integrations/server/lib/mountQueriesBasedOnPermission'; import { addUsersToRoomMethod } from '../../../lib/server/methods/addUsersToRoom'; @@ -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 }, }); diff --git a/apps/meteor/app/api/server/v1/teams.ts b/apps/meteor/app/api/server/v1/teams.ts index fe3de29cf22a4..303a60d8d8a35 100644 --- a/apps/meteor/app/api/server/v1/teams.ts +++ b/apps/meteor/app/api/server/v1/teams.ts @@ -30,7 +30,7 @@ import { escapeRegExp } from '@rocket.chat/string-helpers'; import { eraseRoom } from '../../../../server/lib/eraseRoom'; import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync, hasAtLeastOnePermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import { hasPermissionAsync, hasAtLeastOnePermissionAsync, hasAllPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { removeUserFromRoom } from '../../../lib/server/functions/removeUserFromRoom'; import { settings } from '../../../settings/server'; 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, diff --git a/apps/meteor/tests/end-to-end/api/channels.ts b/apps/meteor/tests/end-to-end/api/channels.ts index 16dbbd836f2b4..363f18886aa61 100644 --- a/apps/meteor/tests/end-to-end/api/channels.ts +++ b/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; + 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')) diff --git a/apps/meteor/tests/end-to-end/api/teams.ts b/apps/meteor/tests/end-to-end/api/teams.ts index 35ddbaae0b5d9..d99f2817d28e6 100644 --- a/apps/meteor/tests/end-to-end/api/teams.ts +++ b/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; + let roomOwnerCredentials: Credentials; + let attacker: TestUser; + 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', () => { From 0b96c2805322c95d4e184d4c4f8323028bcb2cb3 Mon Sep 17 00:00:00 2001 From: Julio Araujo Date: Thu, 9 Jul 2026 16:13:25 +0200 Subject: [PATCH 2/2] Fix lint --- apps/meteor/app/api/server/v1/teams.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/meteor/app/api/server/v1/teams.ts b/apps/meteor/app/api/server/v1/teams.ts index 303a60d8d8a35..e53615e44a4f3 100644 --- a/apps/meteor/app/api/server/v1/teams.ts +++ b/apps/meteor/app/api/server/v1/teams.ts @@ -30,7 +30,11 @@ import { escapeRegExp } from '@rocket.chat/string-helpers'; import { eraseRoom } from '../../../../server/lib/eraseRoom'; import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync, hasAtLeastOnePermissionAsync, hasAllPermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import { + hasPermissionAsync, + hasAtLeastOnePermissionAsync, + hasAllPermissionAsync, +} from '../../../authorization/server/functions/hasPermission'; import { removeUserFromRoom } from '../../../lib/server/functions/removeUserFromRoom'; import { settings } from '../../../settings/server'; import type { ExtractRoutesFromAPI } from '../ApiClass';