Skip to content

Commit

Permalink
chore!: Improve permissions check on im endpoints (#32333)
Browse files Browse the repository at this point in the history
  • Loading branch information
@matheusbsilva137 @ggazzo
matheusbsilva137 authored and ggazzo committed May 15, 2024
1 parent f468881 commit 9c4ca83
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 86 deletions.
12 changes: 2 additions & 10 deletions apps/meteor/app/api/server/v1/im.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ API.v1.addRoute(

API.v1.addRoute(
['dm.messages.others', 'im.messages.others'],
{ authRequired: true },
{ authRequired: true, permissionsRequired: ['view-room-administration'] },
{
async get() {
if (settings.get('API_Enable_Direct_Message_History_EndPoint') !== true) {
Expand All @@ -404,10 +404,6 @@ API.v1.addRoute(
});
}

if (!(await hasPermissionAsync(this.userId, 'view-room-administration'))) {
return API.v1.unauthorized();
}

const { roomId } = this.queryParams;
if (!roomId) {
throw new Meteor.Error('error-roomid-param-not-provided', 'The parameter "roomId" is required');
Expand Down Expand Up @@ -483,13 +479,9 @@ API.v1.addRoute(

API.v1.addRoute(
['dm.list.everyone', 'im.list.everyone'],
{ authRequired: true },
{ authRequired: true, permissionsRequired: ['view-room-administration'] },
{
async get() {
if (!(await hasPermissionAsync(this.userId, 'view-room-administration'))) {
return API.v1.unauthorized();
}

const { offset, count }: { offset: number; count: number } = await getPaginationItems(this.queryParams);
const { sort, fields, query } = await this.parseJsonQuery();

Expand Down
182 changes: 106 additions & 76 deletions apps/meteor/tests/end-to-end/api/04-direct-message.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,29 +206,51 @@ describe('[Direct Messages]', function () {
.end(done);
});

it('/im.list.everyone', (done) => {
request
.get(api('im.list.everyone'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
expect(res.body).to.have.property('ims').and.to.be.an('array');
const im = res.body.ims[0];
expect(im).to.have.property('_id');
expect(im).to.have.property('t').and.to.be.eq('d');
expect(im).to.have.property('msgs').and.to.be.a('number');
expect(im).to.have.property('usernames').and.to.be.an('array');
expect(im).to.have.property('ro');
expect(im).to.have.property('sysMes');
expect(im).to.have.property('_updatedAt');
expect(im).to.have.property('ts');
expect(im).to.have.property('lastMessage');
})
.end(done);
describe('/im.list.everyone', () => {
before(async () => {
return updatePermission('view-room-administration', ['admin']);
});

after(async () => {
return updatePermission('view-room-administration', ['admin']);
});

it('should succesfully return a list of direct messages', async () => {
await request
.get(api('im.list.everyone'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
expect(res.body).to.have.property('ims').and.to.be.an('array');
const im = res.body.ims[0];
expect(im).to.have.property('_id');
expect(im).to.have.property('t').and.to.be.eq('d');
expect(im).to.have.property('msgs').and.to.be.a('number');
expect(im).to.have.property('usernames').and.to.be.an('array');
expect(im).to.have.property('ro');
expect(im).to.have.property('sysMes');
expect(im).to.have.property('_updatedAt');
expect(im).to.have.property('ts');
expect(im).to.have.property('lastMessage');
});
});

it('should fail if user does NOT have the view-room-administration permission', async () => {
await updatePermission('view-room-administration', []);
await request
.get(api('im.list.everyone'))
.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("Setting: 'Use Real Name': true", () => {
Expand Down Expand Up @@ -355,63 +377,71 @@ describe('[Direct Messages]', function () {
});

describe('/im.messages.others', () => {
it('should fail when the endpoint is disabled', (done) => {
updateSetting('API_Enable_Direct_Message_History_EndPoint', false).then(() => {
request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._id,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-endpoint-disabled');
})
.end(done);
});
it('should fail when the endpoint is disabled and the user has permissions', async () => {
await updateSetting('API_Enable_Direct_Message_History_EndPoint', false);
await request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._id,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-endpoint-disabled');
});
});
it('should fail when the endpoint is enabled but the user doesnt have permission', (done) => {
updateSetting('API_Enable_Direct_Message_History_EndPoint', true).then(() => {
updatePermission('view-room-administration', []).then(() => {
request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._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 fail when the endpoint is disabled and the user doesnt have permission', async () => {
await updateSetting('API_Enable_Direct_Message_History_EndPoint', false);
await updatePermission('view-room-administration', ['admin']);
await request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._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 succeed when the endpoint is enabled and user has permission', (done) => {
updateSetting('API_Enable_Direct_Message_History_EndPoint', true).then(() => {
updatePermission('view-room-administration', ['admin']).then(() => {
request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('messages').and.to.be.an('array');
expect(res.body).to.have.property('offset');
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
})
.end(done);
it('should fail when the endpoint is enabled but the user doesnt have permission', async () => {
await updateSetting('API_Enable_Direct_Message_History_EndPoint', true);
await updatePermission('view-room-administration', []);
await request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._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 succeed when the endpoint is enabled and user has permission', async () => {
await updateSetting('API_Enable_Direct_Message_History_EndPoint', true);
await updatePermission('view-room-administration', ['admin']);
await request
.get(api('im.messages.others'))
.set(credentials)
.query({
roomId: directMessage._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('messages').and.to.be.an('array');
expect(res.body).to.have.property('offset');
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
});
});
});
});

Expand Down

0 comments on commit 9c4ca83

Please sign in to comment.