diff --git a/apps/meteor/app/api/server/v1/oauthapps.ts b/apps/meteor/app/api/server/v1/oauthapps.ts index 034a73f54104..d338eb6ee33f 100644 --- a/apps/meteor/app/api/server/v1/oauthapps.ts +++ b/apps/meteor/app/api/server/v1/oauthapps.ts @@ -1,20 +1,15 @@ import { OAuthApps } from '@rocket.chat/models'; import { isUpdateOAuthAppParams, isOauthAppsGetParams, isOauthAppsAddParams, isDeleteOAuthAppParams } from '@rocket.chat/rest-typings'; -import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger'; import { addOAuthApp } from '../../../oauth2-server-config/server/admin/functions/addOAuthApp'; 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(), }); @@ -24,13 +19,9 @@ API.v1.addRoute( API.v1.addRoute( 'oauth-apps.get', - { authRequired: true, validateParams: isOauthAppsGetParams }, + { authRequired: true, validateParams: isOauthAppsGetParams, permissionsRequired: ['manage-oauth-apps'] }, { async get() { - if (!(await hasPermissionAsync(this.userId, 'manage-oauth-apps'))) { - return API.v1.unauthorized(); - } - const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(this.queryParams); if (!oauthApp) { @@ -53,13 +44,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); @@ -74,13 +62,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); @@ -95,13 +80,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/18-oauthapps.js b/apps/meteor/tests/end-to-end/api/18-oauthapps.js index 4566dab47967..dc12c39b0369 100644 --- a/apps/meteor/tests/end-to-end/api/18-oauthapps.js +++ b/apps/meteor/tests/end-to-end/api/18-oauthapps.js @@ -27,10 +27,10 @@ describe('[OAuthApps]', function () { 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); }); @@ -75,31 +75,27 @@ describe('[OAuthApps]', function () { }) .end(done); }); - it('should return a 403 Forbidden error when the user does not have the necessary permission by client id', (done) => { - updatePermission('manage-oauth-apps', []).then(() => { - request - .get(api('oauth-apps.get?clientId=zapier')) - .set(credentials) - .expect(403) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body.error).to.be.equal('unauthorized'); - }) - .end(done); - }); + it('should return a 403 Forbidden error when the user does not have the necessary permission by client id', async () => { + await updatePermission('manage-oauth-apps', []); + await request + .get(api('oauth-apps.get?clientId=zapier')) + .set(credentials) + .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]'); + }); }); - it('should return a 403 Forbidden error when the user does not have the necessary permission by app id', (done) => { - updatePermission('manage-oauth-apps', []).then(() => { - request - .get(api('oauth-apps.get?appId=zapier')) - .set(credentials) - .expect(403) - .expect((res) => { - expect(res.body).to.have.property('success', false); - expect(res.body.error).to.be.equal('unauthorized'); - }) - .end(done); - }); + it('should return a 403 Forbidden error when the user does not have the necessary permission by app id', async () => { + await updatePermission('manage-oauth-apps', []); + await request + .get(api('oauth-apps.get?appId=zapier')) + .set(credentials) + .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]'); + }); }); }); @@ -116,7 +112,11 @@ describe('[OAuthApps]', function () { 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']); }); @@ -199,11 +199,12 @@ describe('[OAuthApps]', function () { describe('[/oauth-apps.update]', () => { let appId; - before((done) => { + before(async () => { + await updatePermission('manage-oauth-apps', ['admin']); const name = 'test-oauth-app'; const redirectUri = 'https://test.com'; const active = true; - request + const res = await request .post(api('oauth-apps.create')) .set(credentials) .send({ @@ -212,12 +213,13 @@ describe('[OAuthApps]', function () { 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 () => { @@ -243,16 +245,40 @@ describe('[OAuthApps]', function () { 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; - before((done) => { + before(async () => { + await updatePermission('manage-oauth-apps', ['admin']); const name = 'test-oauth-app'; const redirectUri = 'https://test.com'; const active = true; - request + const res = await request .post(api('oauth-apps.create')) .set(credentials) .send({ @@ -261,11 +287,12 @@ describe('[OAuthApps]', function () { 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 () => { @@ -281,5 +308,21 @@ describe('[OAuthApps]', function () { 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]'); + }); + }); }); });