From 3db63f0598e37cb9dd0acb6330de4efbf90a9f90 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 4 Oct 2019 16:05:20 -0300 Subject: [PATCH 1/4] add delete-own-message permission --- .../server/functions/canDeleteMessage.js | 49 +++++++++++++++++++ app/authorization/server/startup.js | 1 + app/lib/server/methods/deleteMessage.js | 44 ++++++----------- app/ui-utils/client/lib/messageContext.js | 1 + app/ui/client/lib/chatMessages.js | 8 +-- app/utils/client/lib/canDeleteMessage.js | 11 +++-- 6 files changed, 75 insertions(+), 39 deletions(-) create mode 100644 app/authorization/server/functions/canDeleteMessage.js diff --git a/app/authorization/server/functions/canDeleteMessage.js b/app/authorization/server/functions/canDeleteMessage.js new file mode 100644 index 000000000000..8f966cfe6dcf --- /dev/null +++ b/app/authorization/server/functions/canDeleteMessage.js @@ -0,0 +1,49 @@ +import { hasPermissionAsync } from './hasPermission'; +import { getValue } from '../../../settings/server/raw'; + +const diff = (ts) => { + const dif = Date.now() - ts; + return Math.round((dif / 1000) / 60); +}; + +const notAllowed = () => { + throw new Error('error-action-not-allowed'); +}; + +export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { + const forceDelete = await hasPermissionAsync(uid, 'force-delete-message', rid); + + if (forceDelete) { + return true; + } + const deleteAllowed = await getValue('Message_AllowDeleting'); + + if (!deleteAllowed) { + return notAllowed(); + } + + const allowedToDeleteAny = await hasPermissionAsync(uid, 'delete-message', rid); + + const allowed = allowedToDeleteAny || (uid === u._id && await hasPermissionAsync(uid, 'delete-own-message')); + if (!allowed) { + return notAllowed(); + } + const blockDeleteInMinutes = await getValue('Message_AllowDeleting_BlockDeleteInMinutes'); + + if (!blockDeleteInMinutes) { + return true; + } + + if (ts == null) { + return notAllowed(); + } + + const currentTsDiff = diff(ts); + if (currentTsDiff > blockDeleteInMinutes) { + throw new Error('error-message-deleting-blocked', 'Message deleting is blocked', { + method: 'deleteMessage', + }); + } +}; + +export const canDeleteMessage = (uid, { u, rid, ts }) => Promise.await(canDeleteMessageAsync(uid, { u, rid, ts })); diff --git a/app/authorization/server/startup.js b/app/authorization/server/startup.js index c339b100fae9..31c3b4044d22 100644 --- a/app/authorization/server/startup.js +++ b/app/authorization/server/startup.js @@ -33,6 +33,7 @@ Meteor.startup(function() { { _id: 'delete-c', roles: ['admin', 'owner'] }, { _id: 'delete-d', roles: ['admin'] }, { _id: 'delete-message', roles: ['admin', 'owner', 'moderator'] }, + { _id: 'delete-own-message', roles: ['admin', 'user'] }, { _id: 'delete-p', roles: ['admin', 'owner'] }, { _id: 'delete-user', roles: ['admin'] }, { _id: 'edit-message', roles: ['admin', 'owner', 'moderator'] }, diff --git a/app/lib/server/methods/deleteMessage.js b/app/lib/server/methods/deleteMessage.js index 9d8878b8d2b7..f37e7b09c7e8 100644 --- a/app/lib/server/methods/deleteMessage.js +++ b/app/lib/server/methods/deleteMessage.js @@ -1,9 +1,7 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; -import moment from 'moment'; -import { hasPermission } from '../../../authorization'; -import { settings } from '../../../settings'; +import { canDeleteMessage } from '../../../authorization/server/functions/canDeleteMessage'; import { Messages } from '../../../models'; import { deleteMessage } from '../functions'; @@ -12,11 +10,15 @@ Meteor.methods({ check(message, Match.ObjectIncluding({ _id: String, })); - if (!Meteor.userId()) { + + const uid = Meteor.userId(); + + if (!uid) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'deleteMessage', }); } + const originalMessage = Messages.findOneById(message._id, { fields: { u: 1, @@ -25,38 +27,24 @@ Meteor.methods({ ts: 1, }, }); - if (originalMessage == null) { + + if (!originalMessage) { throw new Meteor.Error('error-action-not-allowed', 'Not allowed', { method: 'deleteMessage', action: 'Delete_message', }); } - const forceDelete = hasPermission(Meteor.userId(), 'force-delete-message', originalMessage.rid); - const _hasPermission = hasPermission(Meteor.userId(), 'delete-message', originalMessage.rid); - const deleteAllowed = settings.get('Message_AllowDeleting'); - const deleteOwn = originalMessage && originalMessage.u && originalMessage.u._id === Meteor.userId(); - if (!(_hasPermission || (deleteAllowed && deleteOwn)) && !forceDelete) { - throw new Meteor.Error('error-action-not-allowed', 'Not allowed', { + + try { + if (!canDeleteMessage(uid, originalMessage)) { + return; + } + return deleteMessage(originalMessage, Meteor.user()); + } catch (error) { + throw new Meteor.Error('Not allowed', { method: 'deleteMessage', action: 'Delete_message', }); } - const blockDeleteInMinutes = settings.get('Message_AllowDeleting_BlockDeleteInMinutes'); - if (blockDeleteInMinutes != null && blockDeleteInMinutes !== 0 && !forceDelete) { - if (originalMessage.ts == null) { - return; - } - const msgTs = moment(originalMessage.ts); - if (msgTs == null) { - return; - } - const currentTsDiff = moment().diff(msgTs, 'minutes'); - if (currentTsDiff > blockDeleteInMinutes) { - throw new Meteor.Error('error-message-deleting-blocked', 'Message deleting is blocked', { - method: 'deleteMessage', - }); - } - } - return deleteMessage(originalMessage, Meteor.user()); }, }); diff --git a/app/ui-utils/client/lib/messageContext.js b/app/ui-utils/client/lib/messageContext.js index 90b8cd872c9d..4786c20e76ff 100644 --- a/app/ui-utils/client/lib/messageContext.js +++ b/app/ui-utils/client/lib/messageContext.js @@ -30,6 +30,7 @@ export function messageContext({ rid } = Template.instance()) { showreply: true, showReplyButton: true, hasPermissionDeleteMessage: hasPermission('delete-message', rid), + hasPermissionDeleteOwnMessage: hasPermission('delete-own-message'), hideRoles: !settings.get('UI_DisplayRoles') || getUserPreference(uid, 'hideRoles'), UI_Use_Real_Name: settings.get('UI_Use_Real_Name'), Chatops_Username: settings.get('Chatops_Username'), diff --git a/app/ui/client/lib/chatMessages.js b/app/ui/client/lib/chatMessages.js index 81b25473f2d0..27b7b8bcb659 100644 --- a/app/ui/client/lib/chatMessages.js +++ b/app/ui/client/lib/chatMessages.js @@ -493,12 +493,8 @@ export class ChatMessages { } } - try { - await call('deleteMessage', { _id }); - } catch (error) { - console.error(error); - handleError(error); - } + + await call('deleteMessage', { _id }); } keydown(event) { diff --git a/app/utils/client/lib/canDeleteMessage.js b/app/utils/client/lib/canDeleteMessage.js index 22305afb0b52..3f35d5974529 100644 --- a/app/utils/client/lib/canDeleteMessage.js +++ b/app/utils/client/lib/canDeleteMessage.js @@ -1,13 +1,13 @@ import { Meteor } from 'meteor/meteor'; import moment from 'moment'; -import { hasAtLeastOnePermission } from '../../../authorization/client'; +import { hasPermission } from '../../../authorization/client'; import { settings } from '../../../settings/client'; export const canDeleteMessage = ({ rid, ts, uid }) => { const userId = Meteor.userId(); - const forceDelete = hasAtLeastOnePermission('force-delete-message', rid); + const forceDelete = hasPermission('force-delete-message', rid); if (forceDelete) { return true; } @@ -17,9 +17,10 @@ export const canDeleteMessage = ({ rid, ts, uid }) => { return false; } - const hasPermission = hasAtLeastOnePermission('delete-message', rid); - const deleteOwn = uid === userId; - if (!hasPermission && !deleteOwn) { + const allowed = hasPermission('delete-message', rid); + + const deleteOwn = allowed || (uid === userId && hasPermission('delete-own-message')); + if (!allowed && !deleteOwn) { return false; } From ade9e3198f6edb31eed5fc4e4d83a35e8203b4a0 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Thu, 17 Oct 2019 23:56:47 -0300 Subject: [PATCH 2/4] refactor --- .../server/functions/canDeleteMessage.js | 22 ++++++------------- app/lib/server/methods/deleteMessage.js | 14 ++---------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/app/authorization/server/functions/canDeleteMessage.js b/app/authorization/server/functions/canDeleteMessage.js index 8f966cfe6dcf..3c389fb22101 100644 --- a/app/authorization/server/functions/canDeleteMessage.js +++ b/app/authorization/server/functions/canDeleteMessage.js @@ -6,27 +6,27 @@ const diff = (ts) => { return Math.round((dif / 1000) / 60); }; -const notAllowed = () => { - throw new Error('error-action-not-allowed'); -}; - export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { const forceDelete = await hasPermissionAsync(uid, 'force-delete-message', rid); if (forceDelete) { return true; } + + if (ts == null) { + return false; + } const deleteAllowed = await getValue('Message_AllowDeleting'); if (!deleteAllowed) { - return notAllowed(); + return false; } const allowedToDeleteAny = await hasPermissionAsync(uid, 'delete-message', rid); const allowed = allowedToDeleteAny || (uid === u._id && await hasPermissionAsync(uid, 'delete-own-message')); if (!allowed) { - return notAllowed(); + return false; } const blockDeleteInMinutes = await getValue('Message_AllowDeleting_BlockDeleteInMinutes'); @@ -34,16 +34,8 @@ export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { return true; } - if (ts == null) { - return notAllowed(); - } - const currentTsDiff = diff(ts); - if (currentTsDiff > blockDeleteInMinutes) { - throw new Error('error-message-deleting-blocked', 'Message deleting is blocked', { - method: 'deleteMessage', - }); - } + return currentTsDiff <= blockDeleteInMinutes; }; export const canDeleteMessage = (uid, { u, rid, ts }) => Promise.await(canDeleteMessageAsync(uid, { u, rid, ts })); diff --git a/app/lib/server/methods/deleteMessage.js b/app/lib/server/methods/deleteMessage.js index f37e7b09c7e8..086b9caee7f9 100644 --- a/app/lib/server/methods/deleteMessage.js +++ b/app/lib/server/methods/deleteMessage.js @@ -28,23 +28,13 @@ Meteor.methods({ }, }); - if (!originalMessage) { + if (!originalMessage || !canDeleteMessage(uid, originalMessage)) { throw new Meteor.Error('error-action-not-allowed', 'Not allowed', { method: 'deleteMessage', action: 'Delete_message', }); } - try { - if (!canDeleteMessage(uid, originalMessage)) { - return; - } - return deleteMessage(originalMessage, Meteor.user()); - } catch (error) { - throw new Meteor.Error('Not allowed', { - method: 'deleteMessage', - action: 'Delete_message', - }); - } + return deleteMessage(originalMessage, Meteor.user()); }, }); From 436fad994532e663fb2741810bc9d081a2976f4a Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 28 Oct 2019 14:52:17 -0300 Subject: [PATCH 3/4] Apply suggestions from code review Co-Authored-By: Tasso Evangelista --- app/authorization/server/functions/canDeleteMessage.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/authorization/server/functions/canDeleteMessage.js b/app/authorization/server/functions/canDeleteMessage.js index 3c389fb22101..954d9879dfc2 100644 --- a/app/authorization/server/functions/canDeleteMessage.js +++ b/app/authorization/server/functions/canDeleteMessage.js @@ -1,7 +1,7 @@ import { hasPermissionAsync } from './hasPermission'; import { getValue } from '../../../settings/server/raw'; -const diff = (ts) => { +const elapsedTime = (ts) => { const dif = Date.now() - ts; return Math.round((dif / 1000) / 60); }; @@ -13,7 +13,7 @@ export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { return true; } - if (ts == null) { + if (!ts) { return false; } const deleteAllowed = await getValue('Message_AllowDeleting'); @@ -35,7 +35,8 @@ export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { } const currentTsDiff = diff(ts); - return currentTsDiff <= blockDeleteInMinutes; + const timeElapsedForMessage = elapsedTime(ts); + return timeElapsedForMessage <= blockDeleteInMinutes; }; export const canDeleteMessage = (uid, { u, rid, ts }) => Promise.await(canDeleteMessageAsync(uid, { u, rid, ts })); From 3ea17366cd6f460ddf0a1678b35825719639ad43 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 28 Oct 2019 15:03:54 -0300 Subject: [PATCH 4/4] Update canDeleteMessage.js --- app/authorization/server/functions/canDeleteMessage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/authorization/server/functions/canDeleteMessage.js b/app/authorization/server/functions/canDeleteMessage.js index 954d9879dfc2..aeb1e06ef126 100644 --- a/app/authorization/server/functions/canDeleteMessage.js +++ b/app/authorization/server/functions/canDeleteMessage.js @@ -34,7 +34,6 @@ export const canDeleteMessageAsync = async (uid, { u, rid, ts }) => { return true; } - const currentTsDiff = diff(ts); const timeElapsedForMessage = elapsedTime(ts); return timeElapsedForMessage <= blockDeleteInMinutes; };