From c7a169e4c0096153c5720feac4c1a8a73db0aef3 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Tue, 27 Feb 2018 15:12:50 -0300 Subject: [PATCH 1/7] Added validation in chat.postMessage endpoint --- packages/rocketchat-api/server/v1/chat.js | 63 ++++++++++++++++++++++- tests/end-to-end/api/05-chat.js | 2 +- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/rocketchat-api/server/v1/chat.js b/packages/rocketchat-api/server/v1/chat.js index 6a866923121c4..67097415935fe 100644 --- a/packages/rocketchat-api/server/v1/chat.js +++ b/packages/rocketchat-api/server/v1/chat.js @@ -7,7 +7,7 @@ RocketChat.API.v1.addRoute('chat.delete', { authRequired: true }, { asUser: Match.Maybe(Boolean) })); - const msg = RocketChat.models.Messages.findOneById(this.bodyParams.msgId, { fields: { u: 1, rid: 1 }}); + const msg = RocketChat.models.Messages.findOneById(this.bodyParams.msgId, { fields: { u: 1, rid: 1 } }); if (!msg) { return RocketChat.API.v1.failure(`No message found with the id of "${ this.bodyParams.msgId }".`); @@ -106,6 +106,67 @@ RocketChat.API.v1.addRoute('chat.pinMessage', { authRequired: true }, { RocketChat.API.v1.addRoute('chat.postMessage', { authRequired: true }, { post() { + const validateBodyAttachments = (attachments) => { + + 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); + } + }; + + attachments.map(validateAttachment); + }; + + const validateBodyParams = (bodyParams) => { + check(bodyParams, Match.ObjectIncluding({ + roomId: Match.Maybe(String), + channel: 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(bodyParams.attachments) && bodyParams.attachments.length) { + validateBodyAttachments(bodyParams.attachments); + } + }; + + try { + validateBodyParams(this.bodyParams); + } catch (error) { + return RocketChat.API.v1.failure({ + error: error.message + }); + } + const messageReturn = processWebhookMessage(this.bodyParams, this.user, undefined, true)[0]; if (!messageReturn) { diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index c6dca60d2c282..a93da9aac929e 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -32,7 +32,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', From b42d2215c978b36e299789bd9c04643a7f81ff4a Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Tue, 27 Feb 2018 17:24:18 -0300 Subject: [PATCH 2/7] fix required parameters and add more tests cases. --- packages/rocketchat-api/server/v1/chat.js | 6 + tests/end-to-end/api/05-chat.js | 359 ++++++++++++++-------- 2 files changed, 233 insertions(+), 132 deletions(-) diff --git a/packages/rocketchat-api/server/v1/chat.js b/packages/rocketchat-api/server/v1/chat.js index 67097415935fe..1067f981df559 100644 --- a/packages/rocketchat-api/server/v1/chat.js +++ b/packages/rocketchat-api/server/v1/chat.js @@ -144,6 +144,12 @@ RocketChat.API.v1.addRoute('chat.postMessage', { authRequired: true }, { }; const validateBodyParams = (bodyParams) => { + const hasAtLeastOneOfRequiredParams = Boolean(bodyParams.roomId) || Boolean(bodyParams.channel); + + if (!hasAtLeastOneOfRequiredParams) { + throw new Error('At least one, \'roomId\' or \'channel\' should be provided'); + } + check(bodyParams, Match.ObjectIncluding({ roomId: Match.Maybe(String), channel: Match.Maybe(String), diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index a93da9aac929e..fe28a284e2a4d 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -11,76 +11,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: 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.' + 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', 'At least one, \'roomId\' or \'channel\' should be provided'); + }) + .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', @@ -96,7 +130,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', @@ -110,77 +144,138 @@ 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); + }); + }); + + 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.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.sendMessage', () => { + 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: '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.' + }] + }] + } + }) + .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.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.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); + }); }); - 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.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); + }); }); - it('/chat.react', (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); + 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); + }); }); + }); From c8d047ac63b4225b6d5f9f32a8c247c093c4d8a2 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Wed, 28 Feb 2018 18:45:45 -0300 Subject: [PATCH 3/7] Added the same validation to chat.sendMessage method too. --- packages/rocketchat-api/package.js | 3 +- .../server/v1/chat/chat.helper.js | 68 +++++++++++++++++ .../server/v1/{ => chat}/chat.js | 71 +++--------------- tests/end-to-end/api/05-chat.js | 75 ++++++++++++++++++- 4 files changed, 153 insertions(+), 64 deletions(-) create mode 100644 packages/rocketchat-api/server/v1/chat/chat.helper.js rename packages/rocketchat-api/server/v1/{ => chat}/chat.js (82%) diff --git a/packages/rocketchat-api/package.js b/packages/rocketchat-api/package.js index df6d1b9b3ab4a..8a59eaac5b8c5 100644 --- a/packages/rocketchat-api/package.js +++ b/packages/rocketchat-api/package.js @@ -31,7 +31,8 @@ Package.onUse(function(api) { api.addFiles('server/v1/channels.js', 'server'); api.addFiles('server/v1/rooms.js', 'server'); api.addFiles('server/v1/subscriptions.js', 'server'); - api.addFiles('server/v1/chat.js', 'server'); + api.addFiles('server/v1/chat/chat.js', 'server'); + api.addFiles('server/v1/chat/chat.helper.js', 'server'); api.addFiles('server/v1/commands.js', 'server'); api.addFiles('server/v1/groups.js', 'server'); api.addFiles('server/v1/im.js', 'server'); diff --git a/packages/rocketchat-api/server/v1/chat/chat.helper.js b/packages/rocketchat-api/server/v1/chat/chat.helper.js new file mode 100644 index 0000000000000..22fba7adcf57e --- /dev/null +++ b/packages/rocketchat-api/server/v1/chat/chat.helper.js @@ -0,0 +1,68 @@ +const validateBodyAttachments = (attachments) => { + + 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); + } + }; + + attachments.map(validateAttachment); +}; + +export const validateMessageObject = ({ message, method }) => { + const hasAtLeastOneOfRequiredParamsForPostMessage = Boolean(message.roomId) || Boolean(message.channel); + const hasAtLeastOneOfRequiredParamsForSendMessage = Boolean(message.rid); + + if (method === 'postMessage' && !hasAtLeastOneOfRequiredParamsForPostMessage) { + throw new Error('At least one, \'roomId\' or \'channel\' should be provided'); + } + + if (method === 'sendMessage' && !hasAtLeastOneOfRequiredParamsForSendMessage) { + throw new Error('The \'rid\' param, must be provided'); + } + + try { + check(message, Match.ObjectIncluding({ + roomId: Match.Maybe(String), + channel: 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); + } + } catch (error) { + throw error; + } + +}; diff --git a/packages/rocketchat-api/server/v1/chat.js b/packages/rocketchat-api/server/v1/chat/chat.js similarity index 82% rename from packages/rocketchat-api/server/v1/chat.js rename to packages/rocketchat-api/server/v1/chat/chat.js index 1067f981df559..af0754200d670 100644 --- a/packages/rocketchat-api/server/v1/chat.js +++ b/packages/rocketchat-api/server/v1/chat/chat.js @@ -1,4 +1,6 @@ /* global processWebhookMessage */ +import { validateMessageObject } from './chat.helper'; + RocketChat.API.v1.addRoute('chat.delete', { authRequired: true }, { post() { check(this.bodyParams, Match.ObjectIncluding({ @@ -106,67 +108,8 @@ RocketChat.API.v1.addRoute('chat.pinMessage', { authRequired: true }, { RocketChat.API.v1.addRoute('chat.postMessage', { authRequired: true }, { post() { - const validateBodyAttachments = (attachments) => { - - 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); - } - }; - - attachments.map(validateAttachment); - }; - - const validateBodyParams = (bodyParams) => { - const hasAtLeastOneOfRequiredParams = Boolean(bodyParams.roomId) || Boolean(bodyParams.channel); - - if (!hasAtLeastOneOfRequiredParams) { - throw new Error('At least one, \'roomId\' or \'channel\' should be provided'); - } - - check(bodyParams, Match.ObjectIncluding({ - roomId: Match.Maybe(String), - channel: 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(bodyParams.attachments) && bodyParams.attachments.length) { - validateBodyAttachments(bodyParams.attachments); - } - }; - try { - validateBodyParams(this.bodyParams); + validateMessageObject({ message: this.bodyParams, method: 'postMessage' }); } catch (error) { return RocketChat.API.v1.failure({ error: error.message @@ -221,6 +164,14 @@ RocketChat.API.v1.addRoute('chat.sendMessage', { authRequired: true }, { throw new Meteor.Error('error-invalid-params', 'The "message" parameter must be provided.'); } + try { + validateMessageObject({ message: this.bodyParams.message, method: 'sendMessage' }); + } catch (error) { + return RocketChat.API.v1.failure({ + error: error.message + }); + } + let message; Meteor.runAsUser(this.userId, () => message = Meteor.call('sendMessage', this.bodyParams.message)); diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index fe28a284e2a4d..3cd405ade7268 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -2,8 +2,17 @@ /* globals expect */ /* eslint no-unused-vars: 0 */ -import {getCredentials, api, login, request, credentials, message, log, apiPrivateChannelName } from '../../data/api-data.js'; -import {adminEmail, password} from '../../data/user.js'; +import { + getCredentials, + api, + login, + request, + credentials, + message, + log, + apiPrivateChannelName +} from '../../data/api-data.js'; +import { adminEmail, password } from '../../data/user.js'; import supertest from 'supertest'; describe('[Chat]', function() { @@ -174,6 +183,66 @@ describe('[Chat]', function() { }); 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\' param, must be provided'); + }) + .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')) @@ -198,7 +267,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', From ad512fb5e868ea3b5a17f3ff702b25928c8ef328 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Fri, 2 Mar 2018 11:48:49 -0300 Subject: [PATCH 4/7] changed validation level, set in sendMessage method --- packages/rocketchat-api/package.js | 3 +- .../server/v1/{chat => }/chat.js | 17 ----- .../server/v1/chat/chat.helper.js | 68 ------------------- .../server/functions/sendMessage.js | 55 +++++++++++++++ .../server/methods/sendMessage.js | 6 +- tests/end-to-end/api/05-chat.js | 2 +- .../api/07-incoming-integrations.js | 1 + 7 files changed, 63 insertions(+), 89 deletions(-) rename packages/rocketchat-api/server/v1/{chat => }/chat.js (95%) delete mode 100644 packages/rocketchat-api/server/v1/chat/chat.helper.js diff --git a/packages/rocketchat-api/package.js b/packages/rocketchat-api/package.js index 8a59eaac5b8c5..df6d1b9b3ab4a 100644 --- a/packages/rocketchat-api/package.js +++ b/packages/rocketchat-api/package.js @@ -31,8 +31,7 @@ Package.onUse(function(api) { api.addFiles('server/v1/channels.js', 'server'); api.addFiles('server/v1/rooms.js', 'server'); api.addFiles('server/v1/subscriptions.js', 'server'); - api.addFiles('server/v1/chat/chat.js', 'server'); - api.addFiles('server/v1/chat/chat.helper.js', 'server'); + api.addFiles('server/v1/chat.js', 'server'); api.addFiles('server/v1/commands.js', 'server'); api.addFiles('server/v1/groups.js', 'server'); api.addFiles('server/v1/im.js', 'server'); diff --git a/packages/rocketchat-api/server/v1/chat/chat.js b/packages/rocketchat-api/server/v1/chat.js similarity index 95% rename from packages/rocketchat-api/server/v1/chat/chat.js rename to packages/rocketchat-api/server/v1/chat.js index af0754200d670..27fcb47052e87 100644 --- a/packages/rocketchat-api/server/v1/chat/chat.js +++ b/packages/rocketchat-api/server/v1/chat.js @@ -1,5 +1,4 @@ /* global processWebhookMessage */ -import { validateMessageObject } from './chat.helper'; RocketChat.API.v1.addRoute('chat.delete', { authRequired: true }, { post() { @@ -108,14 +107,6 @@ RocketChat.API.v1.addRoute('chat.pinMessage', { authRequired: true }, { RocketChat.API.v1.addRoute('chat.postMessage', { authRequired: true }, { post() { - try { - validateMessageObject({ message: this.bodyParams, method: 'postMessage' }); - } catch (error) { - return RocketChat.API.v1.failure({ - error: error.message - }); - } - const messageReturn = processWebhookMessage(this.bodyParams, this.user, undefined, true)[0]; if (!messageReturn) { @@ -164,14 +155,6 @@ RocketChat.API.v1.addRoute('chat.sendMessage', { authRequired: true }, { throw new Meteor.Error('error-invalid-params', 'The "message" parameter must be provided.'); } - try { - validateMessageObject({ message: this.bodyParams.message, method: 'sendMessage' }); - } catch (error) { - return RocketChat.API.v1.failure({ - error: error.message - }); - } - let message; Meteor.runAsUser(this.userId, () => message = Meteor.call('sendMessage', this.bodyParams.message)); diff --git a/packages/rocketchat-api/server/v1/chat/chat.helper.js b/packages/rocketchat-api/server/v1/chat/chat.helper.js deleted file mode 100644 index 22fba7adcf57e..0000000000000 --- a/packages/rocketchat-api/server/v1/chat/chat.helper.js +++ /dev/null @@ -1,68 +0,0 @@ -const validateBodyAttachments = (attachments) => { - - 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); - } - }; - - attachments.map(validateAttachment); -}; - -export const validateMessageObject = ({ message, method }) => { - const hasAtLeastOneOfRequiredParamsForPostMessage = Boolean(message.roomId) || Boolean(message.channel); - const hasAtLeastOneOfRequiredParamsForSendMessage = Boolean(message.rid); - - if (method === 'postMessage' && !hasAtLeastOneOfRequiredParamsForPostMessage) { - throw new Error('At least one, \'roomId\' or \'channel\' should be provided'); - } - - if (method === 'sendMessage' && !hasAtLeastOneOfRequiredParamsForSendMessage) { - throw new Error('The \'rid\' param, must be provided'); - } - - try { - check(message, Match.ObjectIncluding({ - roomId: Match.Maybe(String), - channel: 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); - } - } catch (error) { - throw error; - } - -}; diff --git a/packages/rocketchat-lib/server/functions/sendMessage.js b/packages/rocketchat-lib/server/functions/sendMessage.js index 197ca28729fd1..3b0588ce25c29 100644 --- a/packages/rocketchat-lib/server/functions/sendMessage.js +++ b/packages/rocketchat-lib/server/functions/sendMessage.js @@ -1,9 +1,64 @@ import _ from 'underscore'; +const validateBodyAttachments = (attachments) => { + + 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); + } + }; + + attachments.map(validateAttachment); +}; + RocketChat.sendMessage = function(user, message, room, upsert = false) { if (!user || !message || !room._id) { return false; } + + try { + check(message, Match.ObjectIncluding({ + 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); + } + } catch (error) { + throw error; + } + if (message.ts == null) { message.ts = new Date(); } diff --git a/packages/rocketchat-lib/server/methods/sendMessage.js b/packages/rocketchat-lib/server/methods/sendMessage.js index 03e4272efaf93..0ecf21a299bf3 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\' param, must be provided'); + } + 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 3cd405ade7268..d74e3c332b64a 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -35,7 +35,7 @@ describe('[Chat]', function() { .expect(400) .expect((res) => { expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'At least one, \'roomId\' or \'channel\' should be provided'); + expect(res.body).to.have.property('error', '[invalid-channel]'); }) .end(done); }); diff --git a/tests/end-to-end/api/07-incoming-integrations.js b/tests/end-to-end/api/07-incoming-integrations.js index f7906504ce06b..9f882989dff74 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' From be4a6efb92758d910c2e0cc951a02bc416c74316 Mon Sep 17 00:00:00 2001 From: Marcos Vinicius Spessatto Defendi Date: Thu, 19 Apr 2018 16:51:46 -0300 Subject: [PATCH 5/7] Improvements in the sendMessage function, removed underscore uses, and try catch unnecessary --- .../server/functions/sendMessage.js | 42 +++++++++---------- .../server/methods/sendMessage.js | 2 +- tests/end-to-end/api/05-chat.js | 28 +++++++------ 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/rocketchat-lib/server/functions/sendMessage.js b/packages/rocketchat-lib/server/functions/sendMessage.js index ba25ae4b650e3..27f0854b221e9 100644 --- a/packages/rocketchat-lib/server/functions/sendMessage.js +++ b/packages/rocketchat-lib/server/functions/sendMessage.js @@ -1,5 +1,3 @@ -import _ from 'underscore'; - const validateBodyAttachments = (attachments) => { const validateAttachmentsFields = (attachmentFields) => { @@ -42,27 +40,30 @@ RocketChat.sendMessage = function(user, message, room, upsert = false) { return false; } - try { - check(message, Match.ObjectIncluding({ - 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); - } - } catch (error) { - throw error; + 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 == null) { + if (!message.ts) { message.ts = new Date(); } - message.u = _.pick(user, ['_id', 'username', 'name']); + const { _id, username, name } = message; + message.u = { + _id, + username, + name + }; + message.rid = room._id; if (!Match.test(message.msg, String)) { message.msg = ''; @@ -72,9 +73,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 02afd3fc6bd49..ff2b1e5a1c4dd 100644 --- a/packages/rocketchat-lib/server/methods/sendMessage.js +++ b/packages/rocketchat-lib/server/methods/sendMessage.js @@ -11,7 +11,7 @@ Meteor.methods({ } if (!message.rid) { - throw new Error('The \'rid\' param, must be provided'); + throw new Error('The \'rid\' property on the message object is missing.'); } if (message.ts) { diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index 0398dd4e7feb7..eb8616beb6d52 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -312,7 +312,6 @@ describe('[Chat]', function() { }); }); -<<<<<<< HEAD describe('/chat.search', () => { it('should return a list of messages when execute successfully', (done) => { request.get(api('chat.search')) @@ -320,24 +319,12 @@ describe('[Chat]', function() { .query({ roomId: 'GENERAL', searchText: 'This message was edited via API' -======= - describe('/chat.react', () => { - it('should return statusCode: 200 when the emoji is valid', (done) => { - request.post(api('chat.react')) - .set(credentials) - .send({ - emoji: ':squid:', - messageId: message._id ->>>>>>> develop }) .expect('Content-Type', 'application/json') .expect(200) .expect((res) => { expect(res.body).to.have.property('success', true); -<<<<<<< HEAD expect(res.body).to.have.property('messages'); -======= ->>>>>>> develop }) .end(done); }); @@ -359,6 +346,21 @@ describe('[Chat]', function() { .end(done); }); + it('should return statusCode: 200 when the emoji is valid', (done) => { + request.post(api('chat.react')) + .set(credentials) + .send({ + emoji: ':squid:', + 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 and has no colons', (done) => { request.post(api('chat.react')) .set(credentials) From c770bf49c93610adde78830902e2e72c8ef802c7 Mon Sep 17 00:00:00 2001 From: Marcos Defendi Date: Fri, 20 Apr 2018 12:05:41 -0300 Subject: [PATCH 6/7] Fix test in chat.sendMessage endpoint, and fix error in sendMessage function --- packages/rocketchat-lib/server/functions/sendMessage.js | 2 +- tests/end-to-end/api/05-chat.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rocketchat-lib/server/functions/sendMessage.js b/packages/rocketchat-lib/server/functions/sendMessage.js index 27f0854b221e9..79927a6f33f96 100644 --- a/packages/rocketchat-lib/server/functions/sendMessage.js +++ b/packages/rocketchat-lib/server/functions/sendMessage.js @@ -57,7 +57,7 @@ RocketChat.sendMessage = function(user, message, room, upsert = false) { if (!message.ts) { message.ts = new Date(); } - const { _id, username, name } = message; + const { _id, username, name } = user; message.u = { _id, username, diff --git a/tests/end-to-end/api/05-chat.js b/tests/end-to-end/api/05-chat.js index eb8616beb6d52..fa4f56e48b02b 100644 --- a/tests/end-to-end/api/05-chat.js +++ b/tests/end-to-end/api/05-chat.js @@ -199,7 +199,7 @@ describe('[Chat]', function() { .expect(400) .expect((res) => { expect(res.body).to.have.property('success', false); - expect(res.body).to.have.property('error', 'The \'rid\' param, must be provided'); + expect(res.body).to.have.property('error', 'The \'rid\' property on the message object is missing.'); }) .end(done); }); From d948c979b185321a0f3a50a937f031c3e05f3468 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 20 Apr 2018 19:41:20 -0300 Subject: [PATCH 7/7] fix review --- .../server/functions/sendMessage.js | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/packages/rocketchat-lib/server/functions/sendMessage.js b/packages/rocketchat-lib/server/functions/sendMessage.js index 79927a6f33f96..c4b15ef2f46d4 100644 --- a/packages/rocketchat-lib/server/functions/sendMessage.js +++ b/packages/rocketchat-lib/server/functions/sendMessage.js @@ -1,40 +1,36 @@ -const validateBodyAttachments = (attachments) => { - - 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 validateAttachmentsFields = attachmentFields => { + check(attachmentFields, Match.ObjectIncluding({ + short: Match.Maybe(Boolean), + title: String, + value: String + })); +}; - attachments.map(validateAttachment); +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;