From 5afa10b5cd35f989ef75224caefc27b1af4b988b Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 3 May 2024 10:30:41 -0300 Subject: [PATCH] chore!: Improve permissions check on channels endpoints (#32330) * chore: Improve permission check on channels endpoints --- apps/meteor/app/api/server/v1/channels.ts | 41 ++- .../tests/end-to-end/api/02-channels.js | 279 +++++++++++++----- 2 files changed, 217 insertions(+), 103 deletions(-) diff --git a/apps/meteor/app/api/server/v1/channels.ts b/apps/meteor/app/api/server/v1/channels.ts index 931cf4be2019..a713bfd29ac6 100644 --- a/apps/meteor/app/api/server/v1/channels.ts +++ b/apps/meteor/app/api/server/v1/channels.ts @@ -27,7 +27,7 @@ import { findUsersOfRoom } from '../../../../server/lib/findUsersOfRoom'; import { hideRoomMethod } from '../../../../server/methods/hideRoom'; import { removeUserFromRoomMethod } from '../../../../server/methods/removeUserFromRoom'; import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync, hasAtLeastOnePermissionAsync } from '../../../authorization/server/functions/hasPermission'; +import { 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'; @@ -272,6 +272,7 @@ API.v1.addRoute( { authRequired: true, validateParams: isChannelsMessagesProps, + permissionsRequired: ['view-c-room'], }, { async get() { @@ -292,9 +293,6 @@ API.v1.addRoute( ) { return API.v1.unauthorized(); } - if (!(await hasPermissionAsync(this.userId, 'view-c-room'))) { - return API.v1.unauthorized(); - } const { cursor, totalCount } = await Messages.findPaginated(ourQuery, { sort: sort || { ts: -1 }, @@ -477,13 +475,10 @@ API.v1.addRoute( { authRequired: true, validateParams: isChannelsConvertToTeamProps, + permissionsRequired: ['create-team'], }, { async post() { - if (!(await hasPermissionAsync(this.userId, 'create-team'))) { - return API.v1.unauthorized(); - } - const { channelId, channelName } = this.bodyParams; if (!channelId && !channelName) { @@ -855,20 +850,22 @@ API.v1.addRoute( API.v1.addRoute( 'channels.getIntegrations', - { authRequired: true }, { - async get() { - if ( - !(await hasAtLeastOnePermissionAsync(this.userId, [ + authRequired: true, + permissionsRequired: { + GET: { + permissions: [ 'manage-outgoing-integrations', 'manage-own-outgoing-integrations', 'manage-incoming-integrations', 'manage-own-incoming-integrations', - ])) - ) { - return API.v1.unauthorized(); - } - + ], + operation: 'hasAny', + }, + }, + }, + { + async get() { const findResult = await findChannelByIdOrName({ params: this.queryParams, checkedArchived: false, @@ -954,7 +951,12 @@ API.v1.addRoute( API.v1.addRoute( 'channels.list', - { authRequired: true }, + { + authRequired: true, + permissionsRequired: { + GET: { permissions: ['view-c-room', 'view-joined-room'], operation: 'hasAny' }, + }, + }, { async get() { const { offset, count } = await getPaginationItems(this.queryParams); @@ -964,9 +966,6 @@ API.v1.addRoute( const ourQuery: Record = { ...query, t: 'c' }; if (!hasPermissionToSeeAllPublicChannels) { - if (!(await hasPermissionAsync(this.userId, 'view-joined-room'))) { - return API.v1.unauthorized(); - } const roomIds = ( await Subscriptions.findByUserIdAndType(this.userId, 'c', { projection: { rid: 1 }, diff --git a/apps/meteor/tests/end-to-end/api/02-channels.js b/apps/meteor/tests/end-to-end/api/02-channels.js index ded521f52509..64b9d2e14c4d 100644 --- a/apps/meteor/tests/end-to-end/api/02-channels.js +++ b/apps/meteor/tests/end-to-end/api/02-channels.js @@ -338,21 +338,98 @@ describe('[Channels]', function () { .end(done); }); - it('/channels.list', (done) => { - request - .get(api('channels.list')) - .set(credentials) - .query({ - roomId: channel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('count'); - expect(res.body).to.have.property('total'); - }) - .end(done); + describe('/channels.list', () => { + let testChannel; + before(async () => { + await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']); + await updatePermission('view-joined-room', ['guest', 'bot', 'app', 'anonymous']); + testChannel = (await createRoom({ type: 'c', name: `channels.messages.test.${Date.now()}` })).body.channel; + }); + + after(async () => { + await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']); + await updatePermission('view-joined-room', ['guest', 'bot', 'app', 'anonymous']); + await deleteRoom({ type: 'c', roomId: testChannel._id }); + }); + + it('should succesfully return a list of channels', async () => { + await request + .get(api('channels.list')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('channels').that.is.an('array'); + expect(res.body).to.have.property('count'); + expect(res.body).to.have.property('total'); + }); + }); + + it('should correctly filter channel by id', async () => { + await request + .get(api('channels.list')) + .set(credentials) + .query({ + query: JSON.stringify({ + _id: testChannel._id, + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('channels').that.is.an('array').of.length(1); + expect(res.body).to.have.property('count', 1); + expect(res.body).to.have.property('total'); + }); + }); + + it('should not be succesful when user does NOT have the permission to view channels or joined rooms', async () => { + await updatePermission('view-c-room', []); + await updatePermission('view-joined-room', []); + await request + .get(api('channels.list')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]'); + }); + }); + + it('should be succesful when user does NOT have the permission to view channels, but can view joined rooms', async () => { + await updatePermission('view-c-room', []); + await updatePermission('view-joined-room', ['admin']); + await request + .get(api('channels.list')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('channels').that.is.an('array'); + expect(res.body).to.have.property('count'); + expect(res.body).to.have.property('total'); + }); + }); + + it('should be succesful when user does NOT have the permission to view joined rooms, but can view channels', async () => { + await updatePermission('view-c-room', ['admin']); + await updatePermission('view-joined-room', []); + await request + .get(api('channels.list')) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('channels').that.is.an('array'); + expect(res.body).to.have.property('count'); + expect(res.body).to.have.property('total'); + }); + }); }); it('/channels.list.joined', (done) => { @@ -1396,77 +1473,69 @@ describe('[Channels]', function () { ]); }); - it('should return the list of integrations of created channel and it should contain the integration created by user when the admin DOES have the permission', (done) => { - updatePermission('manage-incoming-integrations', ['admin']).then(() => { - request - .get(api('channels.getIntegrations')) - .set(credentials) - .query({ - roomId: createdChannel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - const integrationCreated = res.body.integrations.find( - (createdIntegration) => createdIntegration._id === integrationCreatedByAnUser._id, - ); - expect(integrationCreated).to.be.an('object'); - expect(integrationCreated._id).to.be.equal(integrationCreatedByAnUser._id); - expect(res.body).to.have.property('offset'); - expect(res.body).to.have.property('total'); - }) - .end(done); - }); + it('should return the list of integrations of created channel and it should contain the integration created by user when the admin DOES have the permission', async () => { + await updatePermission('manage-incoming-integrations', ['admin']); + await request + .get(api('channels.getIntegrations')) + .set(credentials) + .query({ + roomId: createdChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const integrationCreated = res.body.integrations.find( + (createdIntegration) => createdIntegration._id === integrationCreatedByAnUser._id, + ); + expect(integrationCreated).to.be.an('object'); + expect(integrationCreated._id).to.be.equal(integrationCreatedByAnUser._id); + expect(res.body).to.have.property('offset'); + expect(res.body).to.have.property('total'); + }); }); - it('should return the list of integrations created by the user only', (done) => { - updatePermission('manage-own-incoming-integrations', ['admin']).then(() => { - updatePermission('manage-incoming-integrations', []).then(() => { - request - .get(api('channels.getIntegrations')) - .set(credentials) - .query({ - roomId: createdChannel._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - const integrationCreated = res.body.integrations.find( - (createdIntegration) => createdIntegration._id === integrationCreatedByAnUser._id, - ); - expect(integrationCreated).to.be.equal(undefined); - expect(res.body).to.have.property('offset'); - expect(res.body).to.have.property('total'); - }) - .end(done); + it('should return the list of integrations created by the user only', async () => { + await updatePermission('manage-own-incoming-integrations', ['admin']); + await updatePermission('manage-incoming-integrations', []); + await request + .get(api('channels.getIntegrations')) + .set(credentials) + .query({ + roomId: createdChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const integrationCreated = res.body.integrations.find( + (createdIntegration) => createdIntegration._id === integrationCreatedByAnUser._id, + ); + expect(integrationCreated).to.be.equal(undefined); + expect(res.body).to.have.property('offset'); + expect(res.body).to.have.property('total'); }); - }); }); - it('should return unauthorized error when the user does not have any integrations permissions', (done) => { - updatePermission('manage-incoming-integrations', []).then(() => { - updatePermission('manage-own-incoming-integrations', []).then(() => { - updatePermission('manage-outgoing-integrations', []).then(() => { - updatePermission('manage-own-outgoing-integrations', []).then(() => { - request - .get(api('channels.getIntegrations')) - .set(credentials) - .query({ - roomId: createdChannel._id, - }) - .expect('Content-Type', 'application/json') - .expect(403) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'unauthorized'); - }) - .end(done); - }); - }); + it('should return unauthorized error when the user does not have any integrations permissions', async () => { + await Promise.all([ + updatePermission('manage-incoming-integrations', []), + updatePermission('manage-own-incoming-integrations', []), + updatePermission('manage-outgoing-integrations', []), + updatePermission('manage-own-outgoing-integrations', []), + ]); + await request + .get(api('channels.getIntegrations')) + .set(credentials) + .query({ + roomId: createdChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]'); }); - }); }); }); @@ -2041,7 +2110,7 @@ describe('[Channels]', function () { ]); }); - it('should fail to convert channel if lacking edit-room permission', async () => { + it('should fail to convert channel if lacking create-team permission', async () => { await updatePermission('create-team', []); await updatePermission('edit-room', ['admin']); @@ -2055,7 +2124,7 @@ describe('[Channels]', function () { }); }); - it('should fail to convert channel if lacking create-team permission', async () => { + it('should fail to convert channel if lacking edit-room permission', async () => { await updatePermission('create-team', ['admin']); await updatePermission('edit-room', []); @@ -2236,4 +2305,50 @@ describe('[Channels]', function () { }); }); }); + + describe('[/channels.messages]', () => { + let testChannel; + before(async () => { + await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']); + testChannel = (await createRoom({ type: 'c', name: `channels.messages.test.${Date.now()}` })).body.channel; + }); + + after(async () => { + await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']); + await deleteRoom({ type: 'c', roomId: testChannel._id }); + }); + + it('should return an empty array of messages when inspecting a new room', async () => { + await request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: testChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages').and.to.be.an('array').that.is.empty; + expect(res.body).to.have.property('count', 0); + expect(res.body).to.have.property('total', 0); + }); + }); + + it('should not return message when the user does NOT have the necessary permission', async () => { + await updatePermission('view-c-room', []); + await request + .get(api('channels.messages')) + .set(credentials) + .query({ + roomId: testChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]'); + }); + }); + }); });