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/oauthapps.ts b/apps/meteor/tests/end-to-end/api/oauthapps.ts index 7bffa3297bfc..4f9bd8aa1418 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); }); @@ -77,33 +77,29 @@ describe('[OAuthApps]', () => { }) .end(done); }); - it('should return a 403 Forbidden error when the user does not have the necessary permission by client id', (done) => { - void updatePermission('manage-oauth-apps', []).then(() => { - void request - .get(api('oauth-apps.get')) - .query({ 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')) + .query({ 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) => { - void updatePermission('manage-oauth-apps', []).then(() => { - void request - .get(api('oauth-apps.get')) - .query({ 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')) + .query({ 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]'); + }); }); }); @@ -120,7 +116,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']); }); @@ -203,11 +203,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({ @@ -216,12 +217,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 () => { @@ -247,16 +249,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({ @@ -265,11 +291,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 () => { @@ -285,5 +312,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]'); + }); + }); }); });