apitech
/
Rocket.Chat
mirror of https://github.com/RocketChat/Rocket.Chat
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Revert message properties validation (#16395)

Browse Source
pull/16401/head
Marcos Spessatto Defendi 5 years ago committed by GitHub
parent 9504c5b872
commit c170dde948
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 105 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      app/apps/server/bridges/messages.js
  2. 30
      app/lib/server/functions/sendMessage.js
  3. 4
      app/lib/server/methods/sendMessage.js
  4. 114
      tests/end-to-end/api/05-chat.js

2
app/apps/server/bridges/messages.js
Unescape View File

@ -15,7 +15,7 @@ export class AppMessageBridge {
const convertedMessage = this.orch.getConverters().get('messages').convertAppMessage(message);
const sentMessage = executeSendMessage(convertedMessage.u._id, convertedMessage, true);
const sentMessage = executeSendMessage(convertedMessage.u._id, convertedMessage);
return sentMessage._id;
}

30
app/lib/server/functions/sendMessage.js
Unescape View File

@ -8,7 +8,6 @@ import { Apps } from '../../../apps/server';
import { Markdown } from '../../../markdown/server';
import { isURL, isRelativeURL } from '../../../utils/lib/isURL';
import { FileUpload } from '../../../file-upload/server';
import { Users } from '../../../models/server';
/**
* IMPORTANT
@ -144,42 +143,17 @@ const validateMessage = (message) => {
}
};
const validateUserIdentity = (message, _id) => {
if (!message.alias && !message.avatar) {
return;
}
const forbiddenPropsToChangeWhenUserIsNotABot = ['avatar'];
const user = Users.findOneById(_id, { fields: { roles: 1, name: 1 } });
/**
* If the query returns no user, the message has likely
* been sent by a Livechat Visitor, so we don't need to
* validate whether the sender is a bot.
*/
if (!user) {
return;
}
const userIsNotABot = !user.roles.includes('bot');
const messageContainsAnyForbiddenProp = Object.keys(message).some((key) => forbiddenPropsToChangeWhenUserIsNotABot.includes(key));
if (userIsNotABot && (messageContainsAnyForbiddenProp || (typeof message.alias !== 'undefined' && message.alias !== user.name))) {
throw new Error('You are not authorized to change message properties');
}
};
export const sendMessage = function(user, message, room, upsert = false, trustedSender = false) {
export const sendMessage = function(user, message, room, upsert = false) {
if (!user || !message || !room._id) {
return false;
}
const { _id, username, name } = user;
if (!trustedSender) {
validateUserIdentity(message, _id);
}
validateMessage(message);
if (!message.ts) {
message.ts = new Date();
}
const { _id, username, name } = user;
message.u = {
_id,
username,

4
app/lib/server/methods/sendMessage.js
Unescape View File

@ -15,7 +15,7 @@ import { RateLimiter } from '../lib';
import { canSendMessage } from '../../../authorization/server';
import { SystemLogger } from '../../../logger/server';
export function executeSendMessage(uid, message, trustedSender = false) {
export function executeSendMessage(uid, message) {
if (message.tmid && !settings.get('Threads_enabled')) {
throw new Meteor.Error('error-not-allowed', 'not-allowed', {
method: 'sendMessage',
@ -73,7 +73,7 @@ export function executeSendMessage(uid, message, trustedSender = false) {
}
metrics.messagesSent.inc(); // TODO This line needs to be moved to it's proper place. See the comments on: https://github.com/RocketChat/Rocket.Chat/pull/5736
return sendMessage(user, message, room, false, trustedSender);
return sendMessage(user, message, room, false);
} catch (error) {
if (error === 'error-not-allowed') {
throw new Meteor.Error('error-not-allowed');

114
tests/end-to-end/api/05-chat.js
Unescape View File

@ -24,7 +24,9 @@ describe('[Chat]', function() {
.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)
@ -40,8 +42,10 @@ describe('[Chat]', function() {
.set(credentials)
.send({
channel: 'general',
alias: 'Gruggy',
text: 'Sample message',
emoji: ':smirk:',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -76,7 +80,9 @@ describe('[Chat]', function() {
.set(credentials)
.send({
channel: 'general',
alias: 'Gruggy',
text: 'Sample message',
avatar: 'http://res.guggy.com/logo_128.png',
emoji: ':smirk:',
attachments: [{
color: '#ff0000',
@ -100,7 +106,9 @@ describe('[Chat]', function() {
.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!',
@ -123,7 +131,9 @@ describe('[Chat]', function() {
.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!',
@ -146,7 +156,9 @@ describe('[Chat]', function() {
.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!',
@ -178,6 +190,7 @@ describe('[Chat]', function() {
text: 'Sample message',
emoji: ':smirk:',
avatar: 'javascript:alert("xss")',
alias: 'Gruggy',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -207,7 +220,9 @@ describe('[Chat]', function() {
.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!',
@ -239,6 +254,8 @@ describe('[Chat]', function() {
channel: 'general',
text: 'Sample message',
emoji: ':smirk:',
alias: 'Gruggy',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -263,6 +280,8 @@ describe('[Chat]', function() {
channel: 'general',
text: 'Sample message',
emoji: ':smirk:',
alias: 'Gruggy',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -286,7 +305,9 @@ describe('[Chat]', function() {
.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!',
@ -309,7 +330,9 @@ describe('[Chat]', function() {
.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!',
@ -331,8 +354,10 @@ describe('[Chat]', function() {
.set(credentials)
.send({
channel: 'general',
alias: 'Gruggy',
text: 'Sample message',
emoji: ':smirk:',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -358,6 +383,8 @@ describe('[Chat]', function() {
channel: 'general',
text: 'Sample message',
emoji: ':smirk:',
alias: 'Gruggy',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -390,7 +417,7 @@ describe('[Chat]', function() {
.end(done);
});
it('should throw an error when the user try to set alias and avatar and he is not a BOT user', (done) => {
it('should return statusCode 200 when postMessage successfully', (done) => {
request.post(api('chat.postMessage'))
.set(credentials)
.send({
@ -399,42 +426,6 @@ describe('[Chat]', function() {
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 return statusCode 200 when postMessage successfully', (done) => {
request.post(api('chat.postMessage'))
.set(credentials)
.send({
channel: 'general',
text: 'Sample message',
emoji: ':smirk:',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -497,7 +488,9 @@ describe('[Chat]', function() {
.send({
message: {
text: 'Sample message',
alias: 'Gruggy',
emoji: ':smirk:',
avatar: 'http://res.guggy.com/logo_128.png',
},
})
.expect('Content-Type', 'application/json')
@ -515,8 +508,10 @@ describe('[Chat]', function() {
.set(credentials)
.send({
channel: 'general',
alias: 'Gruggy',
text: 'Sample message',
emoji: ':smirk:',
avatar: 'http://res.guggy.com/logo_128.png',
attachments: [{
color: '#ff0000',
text: 'Yay for gruggy!',
@ -539,7 +534,9 @@ describe('[Chat]', function() {
.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!',
@ -562,7 +559,9 @@ describe('[Chat]', function() {
.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!',
@ -585,7 +584,9 @@ describe('[Chat]', function() {
.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!',
@ -611,43 +612,6 @@ describe('[Chat]', function() {
});
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',
emoji: ':smirk:',
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 throw an error when the user try to set alias and avatar and he is not a BOT user', (done) => {
request.post(api('chat.sendMessage'))
.set(credentials)
.send({
@ -695,7 +659,9 @@ describe('[Chat]', function() {
_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!',

Write Preview
Loading…
Cancel
Save