From 389bac82751cb905a90584ff43c8e10e7cdae9ca Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Thu, 12 Oct 2023 17:46:11 -0300 Subject: [PATCH 1/2] test: More tests for groups kick (#30536) --- apps/meteor/tests/end-to-end/api/03-groups.js | 172 +++++++++++++++++- 1 file changed, 166 insertions(+), 6 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/03-groups.js b/apps/meteor/tests/end-to-end/api/03-groups.js index e875be80fd3b..3941df1366eb 100644 --- a/apps/meteor/tests/end-to-end/api/03-groups.js +++ b/apps/meteor/tests/end-to-end/api/03-groups.js @@ -481,21 +481,181 @@ describe('[Groups]', function () { }); describe('/groups.kick', () => { - it('should remove user from group', (done) => { - request + let testUserModerator; + let credsModerator; + let testUserOwner; + let credsOwner; + let testUserMember; + let groupTest; + + const inviteUser = async (userId) => { + await request + .post(api('groups.invite')) + .set(credsOwner) + .send({ + roomId: groupTest._id, + userId, + }) + .expect('Content-Type', 'application/json') + .expect(200); + }; + + before(async () => { + // had to do them in serie because calling them with Promise.all was failing some times + testUserModerator = await createUser(); + testUserOwner = await createUser(); + testUserMember = await createUser(); + + credsModerator = await login(testUserModerator.username, password); + credsOwner = await login(testUserOwner.username, password); + + await request + .post(api('groups.create')) + .set(credsOwner) + .send({ + name: `kick-test-group-${Date.now()}`, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('group._id'); + expect(res.body).to.have.nested.property('group.t', 'p'); + expect(res.body).to.have.nested.property('group.msgs', 0); + groupTest = res.body.group; + }); + + await inviteUser(testUserModerator._id); + + await request + .post(api('groups.addModerator')) + .set(credsOwner) + .send({ + roomId: groupTest._id, + userId: testUserModerator._id, + }) + .expect('Content-Type', 'application/json') + .expect(200); + }); + + after(async () => { + await Promise.all([ + request + .post(api('groups.delete')) + .set(credsOwner) + .send({ + roomId: groupTest._id, + }) + .expect('Content-Type', 'application/json') + .expect(200), + // updatePermission('kick-user-from-any-p-room', []), + updatePermission('remove-user', ['admin', 'owner', 'moderator']), + deleteUser(testUserModerator), + deleteUser(testUserOwner), + deleteUser(testUserMember), + ]); + }); + + it("should return an error when user is not a member of the group and doesn't have permission", async () => { + await request + .post(api('groups.kick')) + .set(credentials) + .send({ + roomId: groupTest._id, + userId: testUserMember._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-room-not-found'); + }); + }); + + it('should allow a moderator to remove user from group', async () => { + await inviteUser(testUserMember._id); + + await request + .post(api('groups.kick')) + .set(credsModerator) + .send({ + roomId: groupTest._id, + userId: testUserMember._id, + }) + .expect('Content-Type', 'application/json') + .expect(200); + }); + + it('should allow an owner to remove user from group', async () => { + await inviteUser(testUserMember._id); + + await request + .post(api('groups.kick')) + .set(credsOwner) + .send({ + roomId: groupTest._id, + userId: testUserMember._id, + }) + .expect('Content-Type', 'application/json') + .expect(200); + }); + + it.skip('should kick user from group if not a member of the room but has the required permission', async () => { + await updatePermission('kick-user-from-any-p-room', ['admin']); + await inviteUser(testUserMember._id); + + await request .post(api('groups.kick')) .set(credentials) .send({ roomId: group._id, - userId: 'rocket.cat', + userId: testUserMember._id, }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(200); + }); + + it("should return an error when the owner doesn't have the required permission", async () => { + await updatePermission('remove-user', ['admin', 'moderator']); + await inviteUser(testUserMember._id); + + await request + .post(api('groups.kick')) + .set(credsOwner) + .send({ + roomId: groupTest._id, + userId: testUserMember._id, + }) + .expect('Content-Type', 'application/json') + + .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'error-not-allowed'); + }); + }); + + it('should return an error when trying to kick the last owner from a group', async () => { + await updatePermission('kick-user-from-any-p-room', ['admin']); + + await request + .post(api('groups.kick')) + .set(credentials) + .send({ + roomId: groupTest._id, + userId: testUserOwner._id, }) - .end(done); + .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-you-are-last-owner'); + }); }); + + it('should return an error when trying to kick user that does not exist'); + it('should return an error when trying to kick user from a group that does not exist'); + it('should return an error when trying to kick user from a group that the user is not in the room'); }); describe('/groups.setDescription', () => { From 35363420f0ed20f1c3403669e1fffa0fd1730deb Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 12 Oct 2023 20:05:05 -0300 Subject: [PATCH 2/2] fix: Some HTTP requests sent by apps don't have their data parsed into JSON (#30560) --- .changeset/stale-masks-learn.md | 5 +++++ packages/server-fetch/src/parsers.ts | 30 +++++++++------------------- 2 files changed, 14 insertions(+), 21 deletions(-) create mode 100644 .changeset/stale-masks-learn.md diff --git a/.changeset/stale-masks-learn.md b/.changeset/stale-masks-learn.md new file mode 100644 index 000000000000..1523b02b0c95 --- /dev/null +++ b/.changeset/stale-masks-learn.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/server-fetch': patch +--- + +Fixed an issue where the payload of an HTTP request made by an app wouldn't be correctly encoded in some cases diff --git a/packages/server-fetch/src/parsers.ts b/packages/server-fetch/src/parsers.ts index 598ecbbd0e8e..ad0a44e96cfb 100644 --- a/packages/server-fetch/src/parsers.ts +++ b/packages/server-fetch/src/parsers.ts @@ -1,32 +1,20 @@ import type { ExtendedFetchOptions, FetchOptions, OriginalFetchOptions } from './types'; -function isPostOrPutOrDeleteWithBody(options?: ExtendedFetchOptions): boolean { - // No method === 'get' - if (!options?.method) { - return false; - } - const { method, body } = options; - const lowerMethod = method?.toLowerCase(); - return ['post', 'put', 'delete'].includes(lowerMethod) && body != null; -} - const jsonParser = (options: ExtendedFetchOptions) => { if (!options) { return {}; } - if (isPostOrPutOrDeleteWithBody(options)) { - try { - if (options && typeof options.body === 'object' && !Buffer.isBuffer(options.body)) { - options.body = JSON.stringify(options.body); - options.headers = { - 'Content-Type': 'application/json', - ...options.headers, - }; - } - } catch (e) { - // Body is not JSON, do nothing + try { + if (typeof options.body === 'object' && !Buffer.isBuffer(options.body)) { + options.body = JSON.stringify(options.body); + options.headers = { + ...options.headers, + 'Content-Type': 'application/json', // force content type to be json + }; } + } catch (e) { + // Body is not JSON, do nothing } return options as FetchOptions;