chore!: Improve permissions check on settings endpoints (#32350)

apps/meteor/app/api/server/v1/settings.ts

@@ -149,12 +149,15 @@ API.v1.addRoute(
 
 API.v1.addRoute(
 	'settings/:_id',
-	{ authRequired: true },
+	{
+		authRequired: true,
+		permissionsRequired: {
+			GET: { permissions: ['view-privileged-setting'], operation: 'hasAll' },
+			POST: { permissions: ['edit-privileged-setting'], operation: 'hasAll' },
+		},
+	},
 	{
 		async get() {
-			if (!(await hasPermissionAsync(this.userId, 'view-privileged-setting'))) {
-				return API.v1.unauthorized();
-			}
 			const setting = await Settings.findOneNotHiddenById(this.urlParams._id);
 			if (!setting) {
 				return API.v1.failure();
@@ -164,10 +167,6 @@ API.v1.addRoute(
 		post: {
 			twoFactorRequired: true,
 			async action(): Promise<ResultFor<'POST', '/v1/settings/:_id'>> {
-				if (!(await hasPermissionAsync(this.userId, 'edit-privileged-setting'))) {
-					return API.v1.unauthorized();
-				}
-
 				if (typeof this.urlParams._id !== 'string') {
 					throw new Meteor.Error('error-id-param-not-provided', 'The parameter "id" is required');
 				}

apps/meteor/tests/end-to-end/api/08-settings.js

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import { before, describe, it, after } from 'mocha';
 
 import { getCredentials, api, request, credentials } from '../../data/api-data.js';
-import { updateSetting } from '../../data/permissions.helper';
+import { updatePermission, updateSetting } from '../../data/permissions.helper';
 
 describe('[Settings]', function () {
 	this.retries(0);
@@ -57,8 +57,18 @@ describe('[Settings]', function () {
 	});
 
 	describe('[/settings/:_id]', () => {
-		it('should return one setting', (done) => {
-			request
+		before(async () => {
+			await updatePermission('view-privileged-setting', ['admin']);
+			await updatePermission('edit-privileged-setting', ['admin']);
+		});
+
+		after(async () => {
+			await updatePermission('view-privileged-setting', ['admin']);
+			await updatePermission('edit-privileged-setting', ['admin']);
+		});
+
+		it('should succesfully return one setting (GET)', async () => {
+			return request
 				.get(api('settings/Site_Url'))
 				.set(credentials)
 				.expect('Content-Type', 'application/json')
@@ -67,8 +77,50 @@ describe('[Settings]', function () {
 					expect(res.body).to.have.property('success', true);
 					expect(res.body).to.have.property('_id', 'Site_Url');
 					expect(res.body).to.have.property('value');
+				});
+		});
+
+		it('should fail returning a setting if user does NOT have the view-privileged-setting permission (GET)', async () => {
+			await updatePermission('view-privileged-setting', []);
+			return request
+				.get(api('settings/Site_Url'))
+				.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 succesfully set the value of a setting (POST)', async () => {
+			return request
+				.post(api('settings/LDAP_Enable'))
+				.set(credentials)
+				.send({
+					value: false,
 				})
-				.end(done);
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+				});
+		});
+
+		it('should fail updating the value of a setting if user does NOT have the edit-privileged-setting permission (POST)', async () => {
+			await updatePermission('edit-privileged-setting', []);
+			return request
+				.post(api('settings/LDAP_Enable'))
+				.set(credentials)
+				.send({
+					value: false,
+				})
+				.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]');
+				});
 		});
 	});