Skip to content

Commit

Permalink
chore!: Improve permissions check on integrations endpoints (#32355)
Browse files Browse the repository at this point in the history
  • Loading branch information
@matheusbsilva137 @ggazzo
matheusbsilva137 authored and ggazzo committed Sep 27, 2024
1 parent 7308252 commit 5e8a45d
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 83 deletions.
54 changes: 30 additions & 24 deletions apps/meteor/app/api/server/v1/integrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { Match, check } from 'meteor/check';
import { Meteor } from 'meteor/meteor';
import type { Filter } from 'mongodb';

import { hasAtLeastOnePermissionAsync } from '../../../authorization/server/functions/hasPermission';
import {
mountIntegrationHistoryQueryBasedOnPermissions,
mountIntegrationQueryBasedOnPermissions,
Expand Down Expand Up @@ -43,15 +42,17 @@ API.v1.addRoute(

API.v1.addRoute(
'integrations.history',
{ authRequired: true, validateParams: isIntegrationsHistoryProps },
{
authRequired: true,
validateParams: isIntegrationsHistoryProps,
permissionsRequired: {
GET: { permissions: ['manage-outgoing-integrations', 'manage-own-outgoing-integrations'], operation: 'hasAny' },
},
},
{
async get() {
const { userId, queryParams } = this;

if (!(await hasAtLeastOnePermissionAsync(userId, ['manage-outgoing-integrations', 'manage-own-outgoing-integrations']))) {
return API.v1.unauthorized();
}

if (!queryParams.id || queryParams.id.trim() === '') {
return API.v1.failure('Invalid integration id.');
}
Expand Down Expand Up @@ -83,20 +84,22 @@ API.v1.addRoute(

API.v1.addRoute(
'integrations.list',
{ authRequired: true },
{
async get() {
if (
!(await hasAtLeastOnePermissionAsync(this.userId, [
authRequired: true,
permissionsRequired: {
GET: {
permissions: [
'manage-outgoing-integrations',
'manage-own-outgoing-integrations',
'manage-incoming-integrations',
'manage-own-incoming-integrations',
]))
) {
return API.v1.unauthorized();
}

],
operation: 'hasAny',
},
},
},
{
async get() {
const { offset, count } = await getPaginationItems(this.queryParams);
const { sort, fields: projection, query } = await this.parseJsonQuery();

Expand Down Expand Up @@ -124,20 +127,23 @@ API.v1.addRoute(

API.v1.addRoute(
'integrations.remove',
{ authRequired: true, validateParams: isIntegrationsRemoveProps },
{
async post() {
if (
!(await hasAtLeastOnePermissionAsync(this.userId, [
authRequired: true,
validateParams: isIntegrationsRemoveProps,
permissionsRequired: {
POST: {
permissions: [
'manage-outgoing-integrations',
'manage-own-outgoing-integrations',
'manage-incoming-integrations',
'manage-own-incoming-integrations',
]))
) {
return API.v1.unauthorized();
}

],
operation: 'hasAny',
},
},
},
{
async post() {
const { bodyParams } = this;

let integration: IIntegration | null = null;
Expand Down
41 changes: 19 additions & 22 deletions apps/meteor/tests/end-to-end/api/incoming-integrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ describe('[Incoming Integrations]', () => {
});

describe('[/integrations.history]', () => {
it('should return an error when trying to get history of incoming integrations', (done) => {
it('should return an error when trying to get history of incoming integrations if user does NOT have enough permissions', (done) => {
void request
.get(api('integrations.history'))
.set(credentials)
Expand All @@ -319,7 +319,7 @@ describe('[Incoming Integrations]', () => {
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
})
.end(done);
});
Expand Down Expand Up @@ -397,25 +397,22 @@ describe('[Incoming Integrations]', () => {
});
});

it('should return unauthorized error when the user does not have any integrations permissions', (done) => {
void updatePermission('manage-incoming-integrations', []).then(() => {
void updatePermission('manage-own-incoming-integrations', []).then(() => {
void updatePermission('manage-outgoing-integrations', []).then(() => {
void updatePermission('manage-outgoing-integrations', []).then(() => {
void request
.get(api('integrations.list'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
})
.end(done);
});
});
it('should return unauthorized error when the user does not have any integrations permissions', async () => {
await Promise.all([
updatePermission('manage-incoming-integrations', []),
updatePermission('manage-own-incoming-integrations', []),
updatePermission('manage-outgoing-integrations', []),
updatePermission('manage-outgoing-integrations', []),
]);
await request
.get(api('integrations.list'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
});
});
});
});

Expand Down Expand Up @@ -578,7 +575,7 @@ describe('[Incoming Integrations]', () => {
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
})
.end(done);
});
Expand All @@ -597,7 +594,7 @@ describe('[Incoming Integrations]', () => {
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
})
.end(done);
});
Expand Down
69 changes: 32 additions & 37 deletions apps/meteor/tests/end-to-end/api/outgoing-integrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,47 +289,42 @@ describe('[Outgoing Integrations]', () => {
});
});

it('should return unauthorized error when the user does not have any integrations permissions', (done) => {
void updatePermission('manage-incoming-integrations', []).then(() => {
void updatePermission('manage-own-incoming-integrations', []).then(() => {
void updatePermission('manage-outgoing-integrations', []).then(() => {
void updatePermission('manage-outgoing-integrations', []).then(() => {
void request
.get(api('integrations.list'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
})
.end(done);
});
});
it('should return unauthorized error when the user does not have any integrations permissions', async () => {
await Promise.all([
updatePermission('manage-incoming-integrations', []),
updatePermission('manage-own-incoming-integrations', []),
updatePermission('manage-outgoing-integrations', []),
updatePermission('manage-outgoing-integrations', []),
]);

await request
.get(api('integrations.list'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
});
});
});
});

describe('[/integrations.history]', () => {
it('should return an error when the user DOES NOT the necessary permission', (done) => {
void updatePermission('manage-outgoing-integrations', []).then(() => {
void updatePermission('manage-own-outgoing-integrations', []).then(() => {
void request
.get(api('integrations.history'))
.set(credentials)
.query({
id: integration._id,
})
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
})
.end(done);
it('should return an error when the user DOES NOT the necessary permission', async () => {
await updatePermission('manage-outgoing-integrations', []);
await updatePermission('manage-own-outgoing-integrations', []);
await request
.get(api('integrations.history'))
.set(credentials)
.query({
id: integration._id,
})
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
});
});
});

it('should return the history of outgoing integrations', (done) => {
Expand Down Expand Up @@ -467,7 +462,7 @@ describe('[Outgoing Integrations]', () => {
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
})
.end(done);
});
Expand All @@ -486,7 +481,7 @@ describe('[Outgoing Integrations]', () => {
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
})
.end(done);
});
Expand Down

0 comments on commit 5e8a45d

Please sign in to comment.