Skip to content

Commit 075997b

Browse files
matheusbsilva137ggazzo
authored andcommitted
chore: Improve permissions check on users endpoints (#32353)
1 parent 111b8a5 commit 075997b

File tree

2 files changed

+18
-31
lines changed

2 files changed

+18
-31
lines changed

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

+11-27
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,9 @@ API.v1.addRoute(
324324

325325
API.v1.addRoute(
326326
'users.delete',
327-
{ authRequired: true },
327+
{ authRequired: true, permissionsRequired: ['delete-user'] },
328328
{
329329
async post() {
330-
if (!(await hasPermissionAsync(this.userId, 'delete-user'))) {
331-
return API.v1.unauthorized();
332-
}
333-
334330
const user = await getUserFromParams(this.bodyParams);
335331
const { confirmRelinquish = false } = this.bodyParams;
336332

@@ -365,16 +361,15 @@ API.v1.addRoute(
365361

366362
API.v1.addRoute(
367363
'users.setActiveStatus',
368-
{ authRequired: true, validateParams: isUserSetActiveStatusParamsPOST },
364+
{
365+
authRequired: true,
366+
validateParams: isUserSetActiveStatusParamsPOST,
367+
permissionsRequired: {
368+
POST: { permissions: ['edit-other-user-active-status', 'manage-moderation-actions'], operation: 'hasAny' },
369+
},
370+
},
369371
{
370372
async post() {
371-
if (
372-
!(await hasPermissionAsync(this.userId, 'edit-other-user-active-status')) &&
373-
!(await hasPermissionAsync(this.userId, 'manage-moderation-actions'))
374-
) {
375-
return API.v1.unauthorized();
376-
}
377-
378373
const { userId, activeStatus, confirmRelinquish = false } = this.bodyParams;
379374
await Meteor.callAsync('setUserActiveStatus', userId, activeStatus, confirmRelinquish);
380375

@@ -391,13 +386,9 @@ API.v1.addRoute(
391386

392387
API.v1.addRoute(
393388
'users.deactivateIdle',
394-
{ authRequired: true, validateParams: isUserDeactivateIdleParamsPOST },
389+
{ authRequired: true, validateParams: isUserDeactivateIdleParamsPOST, permissionsRequired: ['edit-other-user-active-status'] },
395390
{
396391
async post() {
397-
if (!(await hasPermissionAsync(this.userId, 'edit-other-user-active-status'))) {
398-
return API.v1.unauthorized();
399-
}
400-
401392
const { daysIdle, role = 'user' } = this.bodyParams;
402393

403394
const lastLoggedIn = new Date();
@@ -469,13 +460,10 @@ API.v1.addRoute(
469460
{
470461
authRequired: true,
471462
queryOperations: ['$or', '$and'],
463+
permissionsRequired: ['view-d-room'],
472464
},
473465
{
474466
async get() {
475-
if (!(await hasPermissionAsync(this.userId, 'view-d-room'))) {
476-
return API.v1.unauthorized();
477-
}
478-
479467
if (
480468
settings.get('API_Apply_permission_view-outside-room_on_users-list') &&
481469
!(await hasPermissionAsync(this.userId, 'view-outside-room'))
@@ -835,13 +823,9 @@ API.v1.addRoute(
835823

836824
API.v1.addRoute(
837825
'users.getPersonalAccessTokens',
838-
{ authRequired: true },
826+
{ authRequired: true, permissionsRequired: ['create-personal-access-tokens'] },
839827
{
840828
async get() {
841-
if (!(await hasPermissionAsync(this.userId, 'create-personal-access-tokens'))) {
842-
throw new Meteor.Error('not-authorized', 'Not Authorized');
843-
}
844-
845829
const user = (await Users.getLoginTokensByUserId(this.userId).toArray())[0] as unknown as IUser | undefined;
846830

847831
const isPersonalAccessToken = (loginToken: ILoginToken | IPersonalAccessToken): loginToken is IPersonalAccessToken =>

apps/meteor/tests/end-to-end/api/users.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -2982,7 +2982,7 @@ describe('[Users]', () => {
29822982
.expect(403)
29832983
.expect((res) => {
29842984
expect(res.body).to.have.property('success', false);
2985-
expect(res.body).to.have.property('error', 'unauthorized');
2985+
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
29862986
});
29872987
});
29882988

@@ -3245,10 +3245,10 @@ describe('[Users]', () => {
32453245
.get(api('users.getPersonalAccessTokens'))
32463246
.set(credentials)
32473247
.expect('Content-Type', 'application/json')
3248-
.expect(400)
3248+
.expect(403)
32493249
.expect((res) => {
32503250
expect(res.body).to.have.property('success', false);
3251-
expect(res.body.errorType).to.be.equal('not-authorized');
3251+
expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]');
32523252
})
32533253
.end(done);
32543254
});
@@ -3369,6 +3369,7 @@ describe('[Users]', () => {
33693369
.expect(403)
33703370
.expect((res) => {
33713371
expect(res.body).to.have.property('success', false);
3372+
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
33723373
})
33733374
.end(done);
33743375
});
@@ -3385,6 +3386,7 @@ describe('[Users]', () => {
33853386
.expect(403)
33863387
.expect((res) => {
33873388
expect(res.body).to.have.property('success', false);
3389+
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
33883390
})
33893391
.end(done);
33903392
});
@@ -3401,6 +3403,7 @@ describe('[Users]', () => {
34013403
.expect(403)
34023404
.expect((res) => {
34033405
expect(res.body).to.have.property('success', false);
3406+
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
34043407
})
34053408
.end(done);
34063409
});
@@ -3615,7 +3618,7 @@ describe('[Users]', () => {
36153618
.expect(403)
36163619
.expect((res) => {
36173620
expect(res.body).to.have.property('success', false);
3618-
expect(res.body).to.have.property('error', 'unauthorized');
3621+
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
36193622
})
36203623
.end(done);
36213624
});

0 commit comments

Comments
 (0)