chore!: Improve permissions check on oauth-apps endpoints (#32338)

Co-authored-by: Marcos Spessatto Defendi <[email protected]>
RocketChat · Sep 16, 2024 · 1b1695f · 1b1695f
1 parent fcd8105
commit 1b1695f
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 47 deletions.
diff --git a/apps/meteor/app/api/server/v1/oauthapps.ts b/apps/meteor/app/api/server/v1/oauthapps.ts
@@ -1,7 +1,6 @@
 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';
@@ -20,15 +19,10 @@ API.v1.addRoute(
 
 API.v1.addRoute(
 	'oauth-apps.get',
-	{ authRequired: true, validateParams: isOauthAppsGetParams },
+	{ authRequired: true, validateParams: isOauthAppsGetParams, permissionsRequired: ['manage-oauth-apps'] },
 	{
 		async get() {
-			const isOAuthAppsManager = await hasPermissionAsync(this.userId, 'manage-oauth-apps');
-
-			const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(
-				this.queryParams,
-				!isOAuthAppsManager ? { projection: { clientSecret: 0 } } : {},
-			);
+			const oauthApp = await OAuthApps.findOneAuthAppByIdOrClientId(this.queryParams);
 
 			if (!oauthApp) {
 				return API.v1.failure('OAuth app not found.');

diff --git a/apps/meteor/tests/end-to-end/api/oauthapps.ts b/apps/meteor/tests/end-to-end/api/oauthapps.ts
@@ -80,46 +80,33 @@ describe('[OAuthApps]', () => {
 					expect(res.body.oauthApp).to.have.property('clientSecret');
 				});
 		});
-		it('should return only non sensitive information if user does not have the permission to manage oauth apps when searching by clientId', async () => {
-			await updatePermission('manage-oauth-apps', []);
-			await request
-				.get(api('oauth-apps.get'))
-				.query({ clientId: 'zapier' })
-				.set(credentials)
-				.expect(200)
-				.expect((res) => {
-					expect(res.body).to.have.property('success', true);
-					expect(res.body).to.have.property('oauthApp');
-					expect(res.body.oauthApp._id).to.be.equal('zapier');
-					expect(res.body.oauthApp.clientId).to.be.equal('zapier');
-					expect(res.body.oauthApp).to.not.have.property('clientSecret');
-				});
-		});
-		it('should return only non sensitive information if user does not have the permission to manage oauth apps when searching by appId', async () => {
-			await updatePermission('manage-oauth-apps', []);
-			await request
-				.get(api('oauth-apps.get'))
-				.query({ appId: 'zapier' })
-				.set(credentials)
-				.expect(200)
-				.expect((res) => {
-					expect(res.body).to.have.property('success', true);
-					expect(res.body).to.have.property('oauthApp');
-					expect(res.body.oauthApp._id).to.be.equal('zapier');
-					expect(res.body.oauthApp.clientId).to.be.equal('zapier');
-					expect(res.body.oauthApp).to.not.have.property('clientSecret');
-				});
+		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 fail returning an oauth app when an invalid id is provided (avoid NoSQL injections)', () => {
-			return request
-				.get(api('oauth-apps.get'))
-				.query({ _id: '{ "$ne": "" }' })
-				.set(credentials)
-				.expect(400)
-				.expect((res) => {
-					expect(res.body).to.have.property('success', false);
-					expect(res.body).to.have.property('error', 'OAuth app not found.');
-				});
+		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);
+			});
 		});
 	});