From ac3596949814c8bc4112b6d874efefa61171a068 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Thu, 9 May 2024 17:48:28 -0300 Subject: [PATCH] chore!: Improve permissions check on oauth-apps endpoints (#32338) Co-authored-by: Marcos Spessatto Defendi --- apps/meteor/app/api/server/v1/oauthapps.ts | 21 +---- apps/meteor/tests/end-to-end/api/oauthapps.ts | 83 +++++++++++++++---- 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/apps/meteor/app/api/server/v1/oauthapps.ts b/apps/meteor/app/api/server/v1/oauthapps.ts index 4113b945a4db..97d489295d42 100644 --- a/apps/meteor/app/api/server/v1/oauthapps.ts +++ b/apps/meteor/app/api/server/v1/oauthapps.ts @@ -8,13 +8,9 @@ import { API } from '../api'; API.v1.addRoute( 'oauth-apps.list', - { authRequired: true }, + { authRequired: true, permissionsRequired: ['manage-oauth-apps'] }, { async get() { - if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) { - throw new Error('error-not-allowed'); - } - return API.v1.success({ oauthApps: await OAuthApps.find().toArray(), }); @@ -54,13 +50,10 @@ API.v1.addRoute( { authRequired: true, validateParams: isUpdateOAuthAppParams, + permissionsRequired: ['manage-oauth-apps'], }, { async post() { - if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) { - return API.v1.unauthorized(); - } - const { appId } = this.bodyParams; const result = await Meteor.callAsync('updateOAuthApp', appId, this.bodyParams); @@ -75,13 +68,10 @@ API.v1.addRoute( { authRequired: true, validateParams: isDeleteOAuthAppParams, + permissionsRequired: ['manage-oauth-apps'], }, { async post() { - if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) { - return API.v1.unauthorized(); - } - const { appId } = this.bodyParams; const result = await Meteor.callAsync('deleteOAuthApp', appId); @@ -96,13 +86,10 @@ API.v1.addRoute( { authRequired: true, validateParams: isOauthAppsAddParams, + permissionsRequired: ['manage-oauth-apps'], }, { async post() { - if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) { - return API.v1.unauthorized(); - } - const application = await addOAuthApp(this.bodyParams, this.userId); return API.v1.success({ application }); diff --git a/apps/meteor/tests/end-to-end/api/oauthapps.ts b/apps/meteor/tests/end-to-end/api/oauthapps.ts index db714d1107bd..5e42069d9934 100644 --- a/apps/meteor/tests/end-to-end/api/oauthapps.ts +++ b/apps/meteor/tests/end-to-end/api/oauthapps.ts @@ -27,10 +27,10 @@ describe('[OAuthApps]', () => { void request .get(api('oauth-apps.list')) .set(credentials) - .expect(400) + .expect(403) .expect((res) => { expect(res.body).to.have.property('success', false); - expect(res.body.error).to.be.equal('error-not-allowed'); + expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]'); }) .end(done); }); @@ -136,7 +136,11 @@ describe('[OAuthApps]', () => { active: false, }) .expect('Content-Type', 'application/json') - .expect(403); + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]'); + }); await updatePermission('manage-oauth-apps', ['admin']); }); @@ -219,11 +223,12 @@ describe('[OAuthApps]', () => { describe('[/oauth-apps.update]', () => { let appId: IOAuthApps['_id']; - before((done) => { + before(async () => { + await updatePermission('manage-oauth-apps', ['admin']); const name = 'test-oauth-app'; const redirectUri = 'https://test.com'; const active = true; - void request + const res = await request .post(api('oauth-apps.create')) .set(credentials) .send({ @@ -232,12 +237,13 @@ describe('[OAuthApps]', () => { active, }) .expect('Content-Type', 'application/json') - .expect(200) - .end((_err, res) => { - appId = res.body.application._id; - createdAppsIds.push(appId); - done(); - }); + .expect(200); + appId = res.body.application._id; + createdAppsIds.push(appId); + }); + + after(async () => { + await updatePermission('manage-oauth-apps', ['admin']); }); it("should update an app's name, its Active and Redirect URI fields correctly by its id", async () => { @@ -263,16 +269,40 @@ describe('[OAuthApps]', () => { expect(res.body).to.have.property('name', name); }); }); + + it('should fail updating an app if user does NOT have the manage-oauth-apps permission', async () => { + const name = `new app ${Date.now()}`; + const redirectUri = 'http://localhost:3000'; + const active = false; + + await updatePermission('manage-oauth-apps', []); + await request + .post(api(`oauth-apps.update`)) + .set(credentials) + .send({ + appId, + name, + redirectUri, + active, + }) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]'); + }); + }); }); describe('[/oauth-apps.delete]', () => { let appId: IOAuthApps['_id']; - before((done) => { + before(async () => { + await updatePermission('manage-oauth-apps', ['admin']); const name = 'test-oauth-app'; const redirectUri = 'https://test.com'; const active = true; - void request + const res = await request .post(api('oauth-apps.create')) .set(credentials) .send({ @@ -281,11 +311,12 @@ describe('[OAuthApps]', () => { active, }) .expect('Content-Type', 'application/json') - .expect(200) - .end((_err, res) => { - appId = res.body.application._id; - done(); - }); + .expect(200); + appId = res.body.application._id; + }); + + after(async () => { + await updatePermission('manage-oauth-apps', ['admin']); }); it('should delete an app by its id', async () => { @@ -301,5 +332,21 @@ describe('[OAuthApps]', () => { expect(res.body).to.equals(true); }); }); + + it('should fail deleting an app by its id if user does NOT have the manage-oauth-apps permission', async () => { + await updatePermission('manage-oauth-apps', []); + await request + .post(api(`oauth-apps.delete`)) + .set(credentials) + .send({ + appId, + }) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]'); + }); + }); }); });