Skip to content

Commit

Permalink
chore!: Improve permissions check on oauth-apps endpoints (#32338)
Browse files Browse the repository at this point in the history 
Co-authored-by: Marcos Spessatto Defendi <marcos.defendi@rocket.chat>
  • Loading branch information
@matheusbsilva137 @MarcosSpessatto @ggazzo
2 people authored and ggazzo committed Oct 11, 2024
1 parent 20e37de commit a0114fa
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 35 deletions.
21 changes: 4 additions & 17 deletions apps/meteor/app/api/server/v1/oauthapps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
});
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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 });
Expand Down
83 changes: 65 additions & 18 deletions apps/meteor/tests/end-to-end/api/oauthapps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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']);
});

Expand Down Expand Up @@ -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({
Expand All @@ -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 () => {
Expand All @@ -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({
Expand All @@ -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 () => {
Expand All @@ -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]');
});
});
});
});

0 comments on commit a0114fa

Please sign in to comment.