chore!: Improve permissions check on teams endpoints (#32351)

RocketChat · May 9, 2024 · a3e9e9b · a3e9e9b
1 parent 47ba37b
commit a3e9e9b
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 15 deletions.
diff --git a/apps/meteor/app/api/server/v1/teams.ts b/apps/meteor/app/api/server/v1/teams.ts
@@ -44,13 +44,9 @@ API.v1.addRoute(
 
 API.v1.addRoute(
 	'teams.listAll',
-	{ authRequired: true },
+	{ authRequired: true, permissionsRequired: ['view-all-teams'] },
 	{
 		async get() {
-			if (!(await hasPermissionAsync(this.userId, 'view-all-teams'))) {
-				return API.v1.unauthorized();
-			}
-
 			const { offset, count } = await getPaginationItems(this.queryParams);
 
 			const { records, total } = await Team.listAll({ offset, count });
@@ -67,13 +63,9 @@ API.v1.addRoute(
 
 API.v1.addRoute(
 	'teams.create',
-	{ authRequired: true },
+	{ authRequired: true, permissionsRequired: ['create-team'] },
 	{
 		async post() {
-			if (!(await hasPermissionAsync(this.userId, 'create-team'))) {
-				return API.v1.unauthorized();
-			}
-
 			check(
 				this.bodyParams,
 				Match.ObjectIncluding({
@@ -285,10 +277,7 @@ API.v1.addRoute(
 
 			const allowPrivateTeam: boolean = await hasPermissionAsync(this.userId, 'view-all-teams', team.roomId);
 
-			let getAllRooms = false;
-			if (await hasPermissionAsync(this.userId, 'view-all-team-channels', team.roomId)) {
-				getAllRooms = true;
-			}
+			const getAllRooms = await hasPermissionAsync(this.userId, 'view-all-team-channels', team.roomId);
 
 			const listFilter = {
 				name: filter ?? undefined,

diff --git a/apps/meteor/tests/end-to-end/api/25-teams.js b/apps/meteor/tests/end-to-end/api/25-teams.js
@@ -1,5 +1,5 @@
 import { expect } from 'chai';
-import { before, describe, it } from 'mocha';
+import { before, after, describe, it } from 'mocha';
 
 import { getCredentials, api, request, credentials, methodCall } from '../../data/api-data';
 import { updatePermission } from '../../data/permissions.helper';
@@ -60,6 +60,14 @@ describe('[Teams]', () => {
 	});
 
 	describe('/teams.create', () => {
+		before(async () => {
+			return updatePermission('create-team', ['admin', 'user']);
+		});
+
+		after(async () => {
+			return updatePermission('create-team', ['admin', 'user']);
+		});
+
 		it('should create a public team', (done) => {
 			request
 				.post(api('teams.create'))
@@ -181,6 +189,23 @@ describe('[Teams]', () => {
 				})
 				.end(done);
 		});
+
+		it('should not allow creating a team when the user does NOT have the create-team permission', async () => {
+			await updatePermission('create-team', []);
+			await request
+				.post(api('teams.create'))
+				.set(credentials)
+				.send({
+					name: `test-team-${Date.now()}`,
+					type: 0,
+				})
+				.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]');
+				});
+		});
 	});
 
 	describe('/teams.convertToChannel', () => {
@@ -538,6 +563,61 @@ describe('[Teams]', () => {
 		});
 	});
 
+	describe('/teams.listAll', () => {
+		before(async () => {
+			await updatePermission('view-all-teams', ['admin']);
+			const teamName = `test-team-${Date.now()}`;
+			await request.post(api('teams.create')).set(credentials).send({
+				name: teamName,
+				type: 0,
+			});
+		});
+
+		after(async () => {
+			return updatePermission('view-all-teams', ['admin']);
+		});
+
+		it('should list all teams', async () => {
+			await request
+				.get(api('teams.listAll'))
+				.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('count');
+					expect(res.body).to.have.property('offset', 0);
+					expect(res.body).to.have.property('total');
+					expect(res.body).to.have.property('teams');
+					expect(res.body.teams).to.have.length.greaterThan(1);
+					expect(res.body.teams[0]).to.include.property('_id');
+					expect(res.body.teams[0]).to.include.property('_updatedAt');
+					expect(res.body.teams[0]).to.include.property('name');
+					expect(res.body.teams[0]).to.include.property('type');
+					expect(res.body.teams[0]).to.include.property('roomId');
+					expect(res.body.teams[0]).to.include.property('createdBy');
+					expect(res.body.teams[0].createdBy).to.include.property('_id');
+					expect(res.body.teams[0].createdBy).to.include.property('username');
+					expect(res.body.teams[0]).to.include.property('createdAt');
+					expect(res.body.teams[0]).to.include.property('rooms');
+					expect(res.body.teams[0]).to.include.property('numberOfUsers');
+				});
+		});
+
+		it('should return an error when the user does NOT have the view-all-teams permission', async () => {
+			await updatePermission('view-all-teams', []);
+			await request
+				.get(api('teams.listAll'))
+				.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]');
+				});
+		});
+	});
+
 	describe('/teams.updateMember', () => {
 		let testTeam;
 		before('Create test team', (done) => {