From b71355f46bf3651bc7bacd57f0871d7614712549 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Tue, 23 Jun 2020 15:59:37 -0300 Subject: [PATCH 1/2] Fix deprecate check permission on integrations --- app/api/server/v1/chat.js | 2 +- app/integrations/server/api/api.js | 2 +- app/integrations/server/lib/triggerHandler.js | 2 +- .../server/functions/processWebhookMessage.js | 19 +++++++++++-------- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/api/server/v1/chat.js b/app/api/server/v1/chat.js index 815a85f1757ad..6c77dd068317e 100644 --- a/app/api/server/v1/chat.js +++ b/app/api/server/v1/chat.js @@ -128,7 +128,7 @@ API.v1.addRoute('chat.pinMessage', { authRequired: true }, { API.v1.addRoute('chat.postMessage', { authRequired: true }, { post() { - const messageReturn = processWebhookMessage(this.bodyParams, this.user, undefined, true)[0]; + const messageReturn = processWebhookMessage(this.bodyParams, this.user)[0]; if (!messageReturn) { return API.v1.failure('unknown-error'); diff --git a/app/integrations/server/api/api.js b/app/integrations/server/api/api.js index aa55b3bd1fad5..63d32c9be3386 100644 --- a/app/integrations/server/api/api.js +++ b/app/integrations/server/api/api.js @@ -247,7 +247,7 @@ function executeIntegrationRest() { this.bodyParams.bot = { i: this.integration._id }; try { - const message = processWebhookMessage(this.bodyParams, this.user, defaultValues); + const message = processWebhookMessage(this.bodyParams, this.user, defaultValues, this.integration); if (_.isEmpty(message)) { return API.v1.failure('unknown-error'); } diff --git a/app/integrations/server/lib/triggerHandler.js b/app/integrations/server/lib/triggerHandler.js index eca745531fa48..7ce09dc03f884 100644 --- a/app/integrations/server/lib/triggerHandler.js +++ b/app/integrations/server/lib/triggerHandler.js @@ -205,7 +205,7 @@ integrations.triggerHandler = new class RocketChatIntegrationHandler { message.channel = `#${ tmpRoom._id }`; } - message = processWebhookMessage(message, user, defaultValues); + message = processWebhookMessage(message, user, defaultValues, trigger); return message; } diff --git a/app/lib/server/functions/processWebhookMessage.js b/app/lib/server/functions/processWebhookMessage.js index 3fcfa34b8665d..32f4e0b044373 100644 --- a/app/lib/server/functions/processWebhookMessage.js +++ b/app/lib/server/functions/processWebhookMessage.js @@ -5,10 +5,9 @@ import s from 'underscore.string'; import { getRoomByNameOrIdWithOptionToJoin } from './getRoomByNameOrIdWithOptionToJoin'; import { sendMessage } from './sendMessage'; import { validateRoomMessagePermissions } from '../../../authorization/server/functions/canSendMessage'; -import { Subscriptions } from '../../../models'; import { getDirectMessageByIdWithOptionToJoin, getDirectMessageByNameOrIdWithOptionToJoin } from './getDirectMessageByNameOrIdWithOptionToJoin'; -export const processWebhookMessage = function(messageObj, user, defaultValues = { channel: '', alias: '', avatar: '', emoji: '' }, mustBeJoined = false) { +export const processWebhookMessage = function(messageObj, user, defaultValues = { channel: '', alias: '', avatar: '', emoji: '' }, integration = null) { const sentData = []; const channels = [].concat(messageObj.channel || messageObj.roomId || defaultValues.channel); @@ -44,11 +43,6 @@ export const processWebhookMessage = function(messageObj, user, defaultValues = throw new Meteor.Error('invalid-channel'); } - if (mustBeJoined && !Subscriptions.findOneByRoomIdAndUserId(room._id, user._id, { fields: { _id: 1 } })) { - // throw new Meteor.Error('invalid-room', 'Invalid room provided to send a message to, must be joined.'); - throw new Meteor.Error('invalid-channel'); // Throwing the generic one so people can't "brute force" find rooms - } - if (messageObj.attachments && !_.isArray(messageObj.attachments)) { console.log('Attachments should be Array, ignoring value'.red, messageObj.attachments); messageObj.attachments = undefined; @@ -84,7 +78,16 @@ export const processWebhookMessage = function(messageObj, user, defaultValues = } } - validateRoomMessagePermissions(room, { uid: user._id, ...user }); + try { + validateRoomMessagePermissions(room, { uid: user._id, ...user }); + } catch (error) { + if (!integration) { + throw error; + } + console.warn(`Warning: The integration "${ integration.name }" failed to send a message to "${ [].concat(integration.channel).join(',') }" because user "${ integration.username }" doesn't have permission or is not part of the channel.`); + console.warn('This behavior is deprecated and starting from version v4.0.0 the following error will be thrown and the message will not be sent.'); + console.error(error); + } const messageReturn = sendMessage(user, message, room); sentData.push({ channel, message: messageReturn }); From 7d72720fef96a206215b5abcea1284f17367a725 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Tue, 23 Jun 2020 16:44:55 -0300 Subject: [PATCH 2/2] Show warning once per hour --- .../server/functions/processWebhookMessage.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/app/lib/server/functions/processWebhookMessage.js b/app/lib/server/functions/processWebhookMessage.js index 32f4e0b044373..63eeeb16252bc 100644 --- a/app/lib/server/functions/processWebhookMessage.js +++ b/app/lib/server/functions/processWebhookMessage.js @@ -1,12 +1,20 @@ import { Meteor } from 'meteor/meteor'; import _ from 'underscore'; import s from 'underscore.string'; +import mem from 'mem'; import { getRoomByNameOrIdWithOptionToJoin } from './getRoomByNameOrIdWithOptionToJoin'; import { sendMessage } from './sendMessage'; import { validateRoomMessagePermissions } from '../../../authorization/server/functions/canSendMessage'; import { getDirectMessageByIdWithOptionToJoin, getDirectMessageByNameOrIdWithOptionToJoin } from './getDirectMessageByNameOrIdWithOptionToJoin'; +// show deprecation warning only once per hour for each integration +const showDeprecation = mem(({ integration, channels, username }, error) => { + console.warn(`Warning: The integration "${ integration }" failed to send a message to "${ [].concat(channels).join(',') }" because user "${ username }" doesn't have permission or is not a member of the channel.`); + console.warn('This behavior is deprecated and starting from version v4.0.0 the following error will be thrown and the message will not be sent.'); + console.error(error); +}, { maxAge: 360000, cacheKey: (integration) => JSON.stringify(integration) }); + export const processWebhookMessage = function(messageObj, user, defaultValues = { channel: '', alias: '', avatar: '', emoji: '' }, integration = null) { const sentData = []; const channels = [].concat(messageObj.channel || messageObj.roomId || defaultValues.channel); @@ -43,7 +51,7 @@ export const processWebhookMessage = function(messageObj, user, defaultValues = throw new Meteor.Error('invalid-channel'); } - if (messageObj.attachments && !_.isArray(messageObj.attachments)) { + if (messageObj.attachments && !Array.isArray(messageObj.attachments)) { console.log('Attachments should be Array, ignoring value'.red, messageObj.attachments); messageObj.attachments = undefined; } @@ -84,9 +92,11 @@ export const processWebhookMessage = function(messageObj, user, defaultValues = if (!integration) { throw error; } - console.warn(`Warning: The integration "${ integration.name }" failed to send a message to "${ [].concat(integration.channel).join(',') }" because user "${ integration.username }" doesn't have permission or is not part of the channel.`); - console.warn('This behavior is deprecated and starting from version v4.0.0 the following error will be thrown and the message will not be sent.'); - console.error(error); + showDeprecation({ + integration: integration.name, + channels: integration.channel, + username: integration.username, + }, error); } const messageReturn = sendMessage(user, message, room);