From a1a0b3de8ab7f8309b70d2d5263b71f66df6b0e9 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Sat, 17 Aug 2024 00:20:40 -0300 Subject: [PATCH] chore!: Remove `meteor/check` from `chat` endpoints (#32532) Co-authored-by: Guilherme Gazzo --- .changeset/tiny-rice-train.md | 6 + apps/meteor/app/api/server/v1/chat.ts | 38 +--- apps/meteor/client/lib/lists/ThreadsList.ts | 2 +- .../room/contextualBar/Threads/ThreadList.tsx | 1 - apps/meteor/tests/end-to-end/api/chat.ts | 189 ++++++++++++++++++ packages/rest-typings/src/v1/chat.ts | 16 +- 6 files changed, 221 insertions(+), 31 deletions(-) create mode 100644 .changeset/tiny-rice-train.md diff --git a/.changeset/tiny-rice-train.md b/.changeset/tiny-rice-train.md new file mode 100644 index 000000000000..a523c3e698c0 --- /dev/null +++ b/.changeset/tiny-rice-train.md @@ -0,0 +1,6 @@ +--- +"@rocket.chat/meteor": major +"@rocket.chat/rest-typings": major +--- + +Removed `meteor/check` from `chat` endpoints diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index 3ccc9caeafa0..1299a09d8d2e 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -1,9 +1,14 @@ import { Message } from '@rocket.chat/core-services'; import type { IMessage } from '@rocket.chat/core-typings'; import { Messages, Users, Rooms, Subscriptions } from '@rocket.chat/models'; -import { isChatReportMessageProps, isChatGetURLPreviewProps } from '@rocket.chat/rest-typings'; +import { + isChatReportMessageProps, + isChatGetURLPreviewProps, + isChatUpdateProps, + isChatGetThreadsListProps, + isChatDeleteProps, +} from '@rocket.chat/rest-typings'; import { escapeRegExp } from '@rocket.chat/string-helpers'; -import { Match, check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; import { reportMessage } from '../../../../server/lib/moderation/reportMessage'; @@ -26,18 +31,9 @@ import { findDiscussionsFromRoom, findMentionedMessages, findStarredMessages } f API.v1.addRoute( 'chat.delete', - { authRequired: true }, + { authRequired: true, validateParams: isChatDeleteProps }, { async post() { - check( - this.bodyParams, - Match.ObjectIncluding({ - msgId: String, - roomId: String, - asUser: Match.Maybe(Boolean), - }), - ); - const msg = await Messages.findOneById(this.bodyParams.msgId, { projection: { u: 1, rid: 1 } }); if (!msg) { @@ -308,20 +304,9 @@ API.v1.addRoute( API.v1.addRoute( 'chat.update', - { authRequired: true }, + { authRequired: true, validateParams: isChatUpdateProps }, { async post() { - check( - this.bodyParams, - Match.ObjectIncluding({ - roomId: String, - msgId: String, - text: String, // Using text to be consistant with chat.postMessage - customFields: Match.Maybe(Object), - previewUrls: Match.Maybe([String]), - }), - ); - const msg = await Messages.findOneById(this.bodyParams.msgId); // Ensure the message exists @@ -504,13 +489,10 @@ API.v1.addRoute( API.v1.addRoute( 'chat.getThreadsList', - { authRequired: true }, + { authRequired: true, validateParams: isChatGetThreadsListProps }, { async get() { const { rid, type, text } = this.queryParams; - check(rid, String); - check(type, Match.Maybe(String)); - check(text, Match.Maybe(String)); const { offset, count } = await getPaginationItems(this.queryParams); const { sort, fields, query } = await this.parseJsonQuery(); diff --git a/apps/meteor/client/lib/lists/ThreadsList.ts b/apps/meteor/client/lib/lists/ThreadsList.ts index ac592a366865..5e77f0254a08 100644 --- a/apps/meteor/client/lib/lists/ThreadsList.ts +++ b/apps/meteor/client/lib/lists/ThreadsList.ts @@ -18,7 +18,7 @@ export type ThreadsListOptions = { uid: IUser['_id']; } | { - type: 'all'; + type: undefined; } ); diff --git a/apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx b/apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx index cb5c5875f00c..e9ec3d61d1e7 100644 --- a/apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx +++ b/apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx @@ -83,7 +83,6 @@ const ThreadList = () => { return { rid, text, - type: 'all', }; } switch (type) { diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index 7c10eeba45bc..9e55fb168e4c 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -1425,6 +1425,87 @@ describe('[Chat]', () => { await updateSetting('Message_CustomFields_Enabled', false); await updateSetting('Message_CustomFields', ''); }); + it('should fail updating a message if no room id is provided', () => { + return request + .post(api('chat.update')) + .set(credentials) + .send({ + msgId: message._id, + text: 'This message was edited via API', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + + it('should fail updating a message if no message id is provided', () => { + return request + .post(api('chat.update')) + .set(credentials) + .send({ + roomId: testChannel._id, + text: 'This message was edited via API', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + + it('should fail updating a message if no text is provided', () => { + return request + .post(api('chat.update')) + .set(credentials) + .send({ + roomId: testChannel._id, + msgId: message._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', 'invalid-params'); + }); + }); + + it('should fail updating a message if an invalid message id is provided', () => { + return request + .post(api('chat.update')) + .set(credentials) + .send({ + roomId: testChannel._id, + msgId: 'invalid-id', + text: 'This message was edited via API', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'No message found with the id of "invalid-id".'); + }); + }); + + it('should fail updating a message if it is not in the provided room', () => { + return request + .post(api('chat.update')) + .set(credentials) + .send({ + roomId: 'invalid-room', + msgId: message._id, + text: 'This message was edited via API', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'The room id provided does not match where the message is from.'); + }); + }); it('should update a message successfully', (done) => { void request @@ -1643,6 +1724,64 @@ describe('[Chat]', () => { }) .end(done); }); + it('should fail deleting a message if no message id is provided', async () => { + return request + .post(api('chat.delete')) + .set(credentials) + .send({ + roomId: testChannel._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', 'invalid-params'); + }); + }); + it('should fail deleting a message if no room id is provided', async () => { + return request + .post(api('chat.delete')) + .set(credentials) + .send({ + msgId, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + it('should fail deleting a message if it is not in the provided room', async () => { + return request + .post(api('chat.delete')) + .set(credentials) + .send({ + roomId: 'invalid-room', + msgId, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'The room id provided does not match where the message is from.'); + }); + }); + it('should fail deleting a message if an invalid id is provided', async () => { + return request + .post(api('chat.delete')) + .set(credentials) + .send({ + roomId: testChannel._id, + msgId: 'invalid-id', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', `No message found with the id of "invalid-id".`); + }); + }); it('should delete a message successfully', (done) => { void request .post(api('chat.delete')) @@ -2950,6 +3089,56 @@ describe('Threads', () => { }); }); + it("should fail returning a room's thread list if no roomId is provided", async () => { + await updatePermission('view-c-room', ['admin', 'user']); + return request + .get(api('chat.getThreadsList')) + .set(credentials) + .query({}) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + + it("should fail returning a room's thread list if an invalid type is provided", async () => { + return request + .get(api('chat.getThreadsList')) + .set(credentials) + .query({ + rid: testChannel._id, + type: 'invalid-type', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + + it("should return the room's thread list", async () => { + return request + .get(api('chat.getThreadsList')) + .set(credentials) + .query({ + rid: testChannel._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('threads').and.to.be.an('array'); + expect(res.body).to.have.property('total'); + expect(res.body).to.have.property('offset'); + expect(res.body).to.have.property('count'); + expect(res.body.threads).to.have.lengthOf(1); + expect(res.body.threads[0]._id).to.be.equal(threadMessage.tmid); + }); + }); + it("should return the room's thread list even requested with count and offset params", (done) => { void updatePermission('view-c-room', ['admin', 'user']).then(() => { void request diff --git a/packages/rest-typings/src/v1/chat.ts b/packages/rest-typings/src/v1/chat.ts index ce486da5534c..84e51b3c379c 100644 --- a/packages/rest-typings/src/v1/chat.ts +++ b/packages/rest-typings/src/v1/chat.ts @@ -255,8 +255,9 @@ export const isChatReportMessageProps = ajv.compile(ChatRepor type ChatGetThreadsList = PaginatedRequest<{ rid: IRoom['_id']; - type: 'unread' | 'following' | 'all'; + type?: 'unread' | 'following'; text?: string; + fields?: string; }>; const ChatGetThreadsListSchema = { @@ -267,6 +268,7 @@ const ChatGetThreadsListSchema = { }, type: { type: 'string', + enum: ['following', 'unread'], nullable: true, }, text: { @@ -281,6 +283,18 @@ const ChatGetThreadsListSchema = { type: 'number', nullable: true, }, + sort: { + type: 'string', + nullable: true, + }, + query: { + type: 'string', + nullable: true, + }, + fields: { + type: 'string', + nullable: true, + }, }, required: ['rid'], additionalProperties: false,