From 816d110acb400c0e60112c399078178b531cc097 Mon Sep 17 00:00:00 2001 From: Marcos Spessatto Defendi Date: Fri, 20 Apr 2018 22:57:13 -0300 Subject: [PATCH] [BREAK] Validate incoming message schema (#9922) * Added validation in chat.postMessage endpoint * fix required parameters and add more tests cases. * Added the same validation to chat.sendMessage method too. * changed validation level, set in sendMessage method * Improvements in the sendMessage function, removed underscore uses, and try catch unnecessary * Fix test in chat.sendMessage endpoint, and fix error in sendMessage function * fix review --- packages/rocketchat-api/server/v1/chat.js | 1 + .../server/functions/sendMessage.js | 61 ++- .../server/methods/sendMessage.js | 6 +- tests/end-to-end/api/05-chat.js | 406 +++++++++++++----- .../api/07-incoming-integrations.js | 1 + 5 files changed, 351 insertions(+), 124 deletions(-) diff --git a/packages/rocketchat-api/server/v1/chat.js b/packages/rocketchat-api/server/v1/chat.js index b656a520b54e..3bfaacea2907 100644 --- a/packages/rocketchat-api/server/v1/chat.js +++ b/packages/rocketchat-api/server/v1/chat.js @@ -1,4 +1,5 @@ /* global processWebhookMessage */ + RocketChat.API.v1.addRoute('chat.delete', { authRequired: true }, { post() { check(this.bodyParams, Match.ObjectIncluding({ diff --git a/packages/rocketchat-lib/server/functions/sendMessage.js b/packages/rocketchat-lib/server/functions/sendMessage.js index 3c44943d0961..c4b15ef2f46d 100644 --- a/packages/rocketchat-lib/server/functions/sendMessage.js +++ b/packages/rocketchat-lib/server/functions/sendMessage.js @@ -1,10 +1,66 @@ -import _ from 'underscore'; +const validateAttachmentsFields = attachmentFields => { + check(attachmentFields, Match.ObjectIncluding({ + short: Match.Maybe(Boolean), + title: String, + value: String + })); +}; + +const validateAttachment = attachment => { + check(attachment, Match.ObjectIncluding({ + color: Match.Maybe(String), + text: Match.Maybe(String), + ts: Match.Maybe(String), + thumb_url: Match.Maybe(String), + message_link: Match.Maybe(String), + collapsed: Match.Maybe(Boolean), + author_name: Match.Maybe(String), + author_link: Match.Maybe(String), + author_icon: Match.Maybe(String), + title: Match.Maybe(String), + title_link: Match.Maybe(String), + title_link_download: Match.Maybe(Boolean), + image_url: Match.Maybe(String), + audio_url: Match.Maybe(String), + video_url: Match.Maybe(String) + })); + if (attachment.fields.length) { + attachment.fields.map(validateAttachmentsFields); + } +}; + +const validateBodyAttachments = attachments => attachments.map(validateAttachment); RocketChat.sendMessage = function(user, message, room, upsert = false) { if (!user || !message || !room._id) { return false; } + check(message, Match.ObjectIncluding({ + _id: Match.Maybe(String), + msg: Match.Maybe(String), + text: Match.Maybe(String), + alias: Match.Maybe(String), + emoji: Match.Maybe(String), + avatar: Match.Maybe(String), + attachments: Match.Maybe(Array) + })); + + if (Array.isArray(message.attachments) && message.attachments.length) { + validateBodyAttachments(message.attachments); + } + + if (!message.ts) { + message.ts = new Date(); + } + const { _id, username, name } = user; + message.u = { + _id, + username, + name + }; + message.rid = room._id; + if (!Match.test(message.msg, String)) { message.msg = ''; } @@ -13,9 +69,6 @@ RocketChat.sendMessage = function(user, message, room, upsert = false) { message.ts = new Date(); } - message.rid = room._id; - message.u = _.pick(user, ['_id', 'username', 'name']); - if (!room.usernames || room.usernames.length === 0) { const updated_room = RocketChat.models.Rooms.findOneById(room._id); if (updated_room) { diff --git a/packages/rocketchat-lib/server/methods/sendMessage.js b/packages/rocketchat-lib/server/methods/sendMessage.js index 40b7e55f2192..ff2b1e5a1c4d 100644 --- a/packages/rocketchat-lib/server/methods/sendMessage.js +++ b/packages/rocketchat-lib/server/methods/sendMessage.js @@ -10,6 +10,10 @@ Meteor.methods({ }); } + if (!message.rid) { + throw new Error('The \'rid\' property on the message object is missing.'); + } + if (message.ts) { const tsDiff = Math.abs(moment(message.ts).diff()); if (tsDiff > 60000) { @@ -54,7 +58,7 @@ Meteor.methods({ return false; } - if ((room.muted||[]).includes(user.username)) { + if ((room.muted || []).includes(user.username)) { RocketChat.Notifications.notifyUser(Meteor.userId(), 'message', { _id: Random.id(), rid: room._id, diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index 0b3dd98f0212..fa4f56e48b02 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -20,76 +20,110 @@ describe('[Chat]', function() { before(done => getCredentials(done)); - it('/chat.postMessage', (done) => { - request.post(api('chat.postMessage')) - .set(credentials) - .send({ - channel: 'general', - text: 'Sample message', - alias: 'Gruggy', - emoji: ':smirk:', - avatar: 'http://res.guggy.com/logo_128.png', - attachments: [{ - color: '#ff0000', - text: 'Yay for gruggy!', - ts: '2016-12-09T16:53:06.761Z', - thumb_url: 'http://res.guggy.com/logo_128.png', - message_link: 'https://google.com', - collapsed: false, - author_name: 'Bradley Hilton', - author_link: 'https://rocket.chat/', - author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', - title: 'Attachment Example', - title_link: 'https://youtube.com', - title_link_download: 'https://rocket.chat/download', - image_url: 'http://res.guggy.com/logo_128.png', - audio_url: 'http://www.w3schools.com/tags/horse.mp3', - video_url: 'http://www.w3schools.com/tags/movie.mp4', - fields: [{ - short: true, - title: 'Test', - value: 'Testing out something or other' - }, { - short: true, - title: 'Another Test', - value: '[Link](https://google.com/) something and this and that.' + describe('/chat.postMessage', () => { + + it('should throw an error when at least one of required parameters(channel, roomId) is not sent', (done) => { + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png' + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', '[invalid-channel]'); + }) + .end(done); + }); + + it('should throw an error when it has some properties with the wrong type(attachments.title_link_download, attachments.fields, message_link)', (done) => { + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 12, + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: 'https://youtube.com', + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: '' }] - }] - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.nested.property('message.msg', 'Sample message'); - message._id = res.body.message._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('error'); + }) + .end(done); + }); - it('/chat.getMessage', (done) => { - request.get(api('chat.getMessage')) - .set(credentials) - .query({ - msgId: message._id - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.nested.property('message._id', message._id); - }) - .end(done); - }); + it('should throw an error when the properties (attachments.fields.title, attachments.fields.value) are with the wrong type', (done) => { + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [{ + short: true, + title: 12, + value: false + }] + }] + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done); + }); - it('/chat.sendMessage', (done) => { - message._id = `id-${ Date.now() }`; - request.post(api('chat.sendMessage')) - .set(credentials) - .send({ - message: { - _id: message._id, - rid: 'GENERAL', - msg: 'Sample message', + it('should return statusCode 200 when postMessage successfully', (done) => { + request.post(api('chat.postMessage')) + .set(credentials) + .send({ + channel: 'general', + text: 'Sample message', alias: 'Gruggy', emoji: ':smirk:', avatar: 'http://res.guggy.com/logo_128.png', @@ -105,7 +139,7 @@ describe('[Chat]', function() { author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', title: 'Attachment Example', title_link: 'https://youtube.com', - title_link_download: 'https://rocket.chat/download', + title_link_download: true, image_url: 'http://res.guggy.com/logo_128.png', audio_url: 'http://www.w3schools.com/tags/horse.mp3', video_url: 'http://www.w3schools.com/tags/movie.mp4', @@ -119,66 +153,199 @@ describe('[Chat]', function() { value: '[Link](https://google.com/) something and this and that.' }] }] - } - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.nested.property('message.msg', 'Sample message'); - }) - .end(done); + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('message.msg', 'Sample message'); + message._id = res.body.message._id; + }) + .end(done); + }); }); - it('/chat.getMessage', (done) => { - request.get(api('chat.getMessage')) - .set(credentials) - .query({ - msgId: message._id - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.nested.property('message._id', message._id); - }) - .end(done); + describe('/chat.getMessage', () => { + it('should retrieve the message successfully', (done) => { + request.get(api('chat.getMessage')) + .set(credentials) + .query({ + msgId: message._id + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('message._id', message._id); + }) + .end(done); + }); }); - it('/chat.update', (done) => { - request.post(api('chat.update')) - .set(credentials) - .send({ - roomId: 'GENERAL', - msgId: message._id, - text: 'This message was edited via API' - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.nested.property('message.msg', 'This message was edited via API'); - }) - .end(done); + describe('/chat.sendMessage', () => { + + it('should throw an error when the required param \'rid\' is not sent', (done) => { + request.post(api('chat.sendMessage')) + .set(credentials) + .send({ + message: { + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png' + } + }) + .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 \'rid\' property on the message object is missing.'); + }) + .end(done); + }); + + it('should throw an error when it has some properties with the wrong type(attachments.title_link_download, attachments.fields, message_link)', (done) => { + request.post(api('chat.sendMessage')) + .set(credentials) + .send({ + message: { + channel: 'general', + text: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 12, + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: 'https://youtube.com', + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: '' + }] + } + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error'); + }) + .end(done); + }); + + it('should send a message successfully', (done) => { + message._id = `id-${ Date.now() }`; + request.post(api('chat.sendMessage')) + .set(credentials) + .send({ + message: { + _id: message._id, + rid: 'GENERAL', + msg: 'Sample message', + alias: 'Gruggy', + emoji: ':smirk:', + avatar: 'http://res.guggy.com/logo_128.png', + attachments: [{ + color: '#ff0000', + text: 'Yay for gruggy!', + ts: '2016-12-09T16:53:06.761Z', + thumb_url: 'http://res.guggy.com/logo_128.png', + message_link: 'https://google.com', + collapsed: false, + author_name: 'Bradley Hilton', + author_link: 'https://rocket.chat/', + author_icon: 'https://avatars.githubusercontent.com/u/850391?v=3', + title: 'Attachment Example', + title_link: 'https://youtube.com', + title_link_download: true, + image_url: 'http://res.guggy.com/logo_128.png', + audio_url: 'http://www.w3schools.com/tags/horse.mp3', + video_url: 'http://www.w3schools.com/tags/movie.mp4', + fields: [{ + short: true, + title: 'Test', + value: 'Testing out something or other' + }, { + short: true, + title: 'Another Test', + value: '[Link](https://google.com/) something and this and that.' + }] + }] + } + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('message.msg', 'Sample message'); + }) + .end(done); + }); }); - it('/chat.search', (done) => { - request.get(api('chat.search')) - .set(credentials) - .query({ - roomId: 'GENERAL', - searchText: 'This message was edited via API' - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('messages'); - }) - .end(done); + describe('/chat.update', () => { + it('should update a message successfully', (done) => { + request.post(api('chat.update')) + .set(credentials) + .send({ + roomId: 'GENERAL', + msgId: message._id, + text: 'This message was edited via API' + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('message.msg', 'This message was edited via API'); + }) + .end(done); + }); + }); + + describe('/chat.search', () => { + it('should return a list of messages when execute successfully', (done) => { + request.get(api('chat.search')) + .set(credentials) + .query({ + roomId: 'GENERAL', + searchText: 'This message was edited via API' + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('messages'); + }) + .end(done); + }); }); describe('/chat.react', () => { + it('should react a message successfully', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: 'smile', + messageId: message._id + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }) + .end(done); + }); + it('should return statusCode: 200 when the emoji is valid', (done) => { request.post(api('chat.react')) .set(credentials) @@ -291,4 +458,5 @@ describe('[Chat]', function() { }); }); }); + }); diff --git a/tests/end-to-end/api/07-incoming-integrations.js b/tests/end-to-end/api/07-incoming-integrations.js index f7906504ce06..9f882989dff7 100644 --- a/tests/end-to-end/api/07-incoming-integrations.js +++ b/tests/end-to-end/api/07-incoming-integrations.js @@ -16,6 +16,7 @@ describe('Incoming Integrations', function() { type: 'webhook-incoming', name: 'Incoming test', enabled: true, + alias: 'test', username: 'rocket.cat', scriptEnabled: false, channel: '#general'