Skip to content

Commit

Permalink
chore!: Improve permissions check on settings endpoints (#32350)
Browse files Browse the repository at this point in the history
  • Loading branch information
matheusbsilva137 authored and ggazzo committed Aug 15, 2024
1 parent 77cfe8c commit 38cb0fe
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 12 deletions.
15 changes: 7 additions & 8 deletions apps/meteor/app/api/server/v1/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,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();
Expand All @@ -165,10 +168,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');
}
Expand Down
60 changes: 56 additions & 4 deletions apps/meteor/tests/end-to-end/api/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { expect } from 'chai';
import { before, describe, it, after } from 'mocha';

import { getCredentials, api, request, credentials } from '../../data/api-data';
import { updateSetting } from '../../data/permissions.helper';
import { updatePermission, updateSetting } from '../../data/permissions.helper';

describe('[Settings]', () => {
before((done) => getCredentials(done));
Expand Down Expand Up @@ -56,8 +56,18 @@ describe('[Settings]', () => {
});

describe('[/settings/:_id]', () => {
it('should return one setting', (done) => {
void 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')
Expand All @@ -66,8 +76,50 @@ describe('[Settings]', () => {
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]');
});
});
});

Expand Down

0 comments on commit 38cb0fe

Please sign in to comment.