Skip to content

Commit

Permalink
chore!: Improve permissions check on permissions endpoints (#32343)
Browse files Browse the repository at this point in the history
  • Loading branch information
matheusbsilva137 authored and ggazzo committed May 15, 2024
1 parent 9c4ca83 commit 1ec86c0
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
7 changes: 1 addition & 6 deletions apps/meteor/app/api/server/v1/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { Permissions, Roles } from '@rocket.chat/models';
import { isBodyParamsValidPermissionUpdate } from '@rocket.chat/rest-typings';
import { Meteor } from 'meteor/meteor';

import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { notifyOnPermissionChangedById } from '../../../lib/server/lib/notifyListener';
import { API } from '../api';

Expand Down Expand Up @@ -41,13 +40,9 @@ API.v1.addRoute(

API.v1.addRoute(
'permissions.update',
{ authRequired: true },
{ authRequired: true, permissionsRequired: ['access-permissions'] },
{
async post() {
if (!(await hasPermissionAsync(this.userId, 'access-permissions'))) {
return API.v1.failure('Editing permissions is not allowed', 'error-edit-permissions-not-allowed');
}

const { bodyParams } = this;

if (!isBodyParamsValidPermissionUpdate(bodyParams)) {
Expand Down
33 changes: 33 additions & 0 deletions apps/meteor/tests/end-to-end/api/11-permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { before, describe, it, after } from 'mocha';

import { getCredentials, api, request, credentials } from '../../data/api-data.js';
import { updatePermission } from '../../data/permissions.helper';
import { password } from '../../data/user';
import { createUser, deleteUser, login } from '../../data/users.helper.js';

describe('[Permissions]', function () {
this.retries(0);
Expand Down Expand Up @@ -54,6 +56,19 @@ describe('[Permissions]', function () {
});

describe('[/permissions.update]', () => {
let testUser;
let testUserCredentials;
before(async () => {
testUser = await createUser();
testUserCredentials = await login(testUser.username, password);
await updatePermission('access-permissions', ['admin']);
});

after(async () => {
await updatePermission('access-permissions', ['admin']);
await deleteUser(testUser);
});

it('should change the permissions on the server', (done) => {
const permissions = [
{
Expand Down Expand Up @@ -127,5 +142,23 @@ describe('[Permissions]', function () {
})
.end(done);
});
it('should fail updating permission if user does NOT have the access-permissions permission', async () => {
const permissions = [
{
_id: 'add-oauth-service',
roles: ['admin', 'user'],
},
];
await request
.post(api('permissions.update'))
.set(testUserCredentials)
.send({ permissions })
.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]');
});
});
});
});

0 comments on commit 1ec86c0

Please sign in to comment.