Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW] Permission to bypass message editing and removing limits #27644

Merged
merged 29 commits into from Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9c89118
draft: updating checks for block
LucianoPierdona Dec 22, 2022
78da7a5
Merge remote-tracking branch 'origin/develop' into feat/bypass-time-l…
LucianoPierdona Dec 26, 2022
cbff75b
re-add migration
LucianoPierdona Dec 26, 2022
b9392cf
added checks for bypass permissions
LucianoPierdona Dec 26, 2022
2f378dc
update permission name
LucianoPierdona Dec 27, 2022
2b84067
Update apps/meteor/app/authorization/server/functions/upsertPermissio…
LucianoPierdona Dec 27, 2022
436b852
updated delete message bypass check
LucianoPierdona Dec 27, 2022
6a2e0f1
update default roles for bypass-time-limit-edit-and-delete
LucianoPierdona Dec 27, 2022
3dc3a61
Merge branch 'develop' into feat/bypass-time-limits-permission
LucianoPierdona Dec 28, 2022
d586c4b
add migration to index
LucianoPierdona Dec 28, 2022
a0dfa4b
update check for bypass
LucianoPierdona Dec 29, 2022
4e71aaa
add tests for updateMessage and deleteMessage
LucianoPierdona Jan 3, 2023
d2319f8
Merge branch 'develop' into feat/bypass-time-limits-permission
matheusbsilva137 Jan 4, 2023
ad8e257
Merge branch 'develop' into feat/bypass-time-limits-permission
sampaiodiego Jan 10, 2023
3c9a1ac
Update apps/meteor/tests/end-to-end/api/24-methods.js
LucianoPierdona Feb 7, 2023
499ba0b
linting
LucianoPierdona Feb 7, 2023
410effa
fix tests
LucianoPierdona Feb 7, 2023
e16c95b
Merge branch 'develop' into feat/bypass-time-limits-permission
hugocostadev Feb 7, 2023
01e6e8e
update tests
LucianoPierdona Feb 7, 2023
03dd269
Merge branch 'develop' into feat/bypass-time-limits-permission
rodrigok Feb 7, 2023
50ddce4
Merge remote-tracking branch 'origin/develop' into feat/bypass-time-l…
LucianoPierdona Feb 8, 2023
14b72c7
Merge remote-tracking branch 'origin/develop' into feat/bypass-time-l…
rodrigok Feb 8, 2023
097b1de
Merge branch 'develop' into feat/bypass-time-limits-permission
kodiakhq[bot] Feb 9, 2023
9ec2111
Merge branch 'develop' into feat/bypass-time-limits-permission
kodiakhq[bot] Feb 9, 2023
04baaf6
Merge branch 'develop' into feat/bypass-time-limits-permission
kodiakhq[bot] Feb 9, 2023
d70466b
Merge remote-tracking branch 'origin/develop' into feat/bypass-time-l…
hugocostadev Feb 9, 2023
f5abd9a
renaming migration
hugocostadev Feb 9, 2023
c16eb34
Merge branch 'develop' into feat/bypass-time-limits-permission
ggazzo Feb 9, 2023
9bc72d6
Merge branch 'develop' into feat/bypass-time-limits-permission
kodiakhq[bot] Feb 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -31,11 +31,15 @@ export const canDeleteMessageAsync = async (uid: string, { u, rid, ts }: { u: IU
if (!allowed) {
return false;
}
const blockDeleteInMinutes = await getValue('Message_AllowDeleting_BlockDeleteInMinutes');
const bypassBlockTimeLimit = await hasPermissionAsync(uid, 'bypass-time-limit-edit-and-delete');

if (blockDeleteInMinutes) {
const timeElapsedForMessage = elapsedTime(ts);
return timeElapsedForMessage <= blockDeleteInMinutes;
if (!bypassBlockTimeLimit) {
const blockDeleteInMinutes = await getValue('Message_AllowDeleting_BlockDeleteInMinutes');

if (blockDeleteInMinutes) {
const timeElapsedForMessage = elapsedTime(ts);
return timeElapsedForMessage <= blockDeleteInMinutes;
}
}

const room = await Rooms.findOneById(rid, { fields: { ro: 1, unmuted: 1 } });
Expand Down
Expand Up @@ -224,6 +224,7 @@ export const upsertPermissions = async (): Promise<void> => {
{ _id: 'view-import-operations', roles: ['admin'] },
{ _id: 'clear-oembed-cache', roles: ['admin'] },
{ _id: 'videoconf-ring-users', roles: ['admin', 'owner', 'moderator', 'user'] },
{ _id: 'bypass-time-limit-edit-and-delete', roles: ['bot', 'app'] },
];

for await (const permission of permissions) {
Expand Down
4 changes: 3 additions & 1 deletion apps/meteor/app/lib/server/methods/updateMessage.js
Expand Up @@ -49,7 +49,9 @@ Meteor.methods({
}

const blockEditInMinutes = settings.get('Message_AllowEditing_BlockEditInMinutes');
if (Match.test(blockEditInMinutes, Number) && blockEditInMinutes !== 0) {
const bypassBlockTimeLimit = hasPermission(Meteor.userId(), 'bypass-time-limit-edit-and-delete');

if (!bypassBlockTimeLimit && Match.test(blockEditInMinutes, Number) && blockEditInMinutes !== 0) {
let currentTsDiff;
let msgTs;

Expand Down
8 changes: 5 additions & 3 deletions apps/meteor/app/ui-utils/client/lib/messageActionDefault.ts
Expand Up @@ -153,14 +153,16 @@ Meteor.startup(async function () {
if (isRoomFederated(room)) {
return message.u._id === Meteor.userId();
}
const hasPermission = hasAtLeastOnePermission('edit-message', message.rid);
const canEditMessage = hasAtLeastOnePermission('edit-message', message.rid);
const isEditAllowed = settings.Message_AllowEditing;
const editOwn = message.u && message.u._id === Meteor.userId();
if (!(hasPermission || (isEditAllowed && editOwn))) {
if (!(canEditMessage || (isEditAllowed && editOwn))) {
return false;
}
const blockEditInMinutes = settings.Message_AllowEditing_BlockEditInMinutes;
if (blockEditInMinutes) {
const bypassBlockTimeLimit = hasPermission('bypass-time-limit-edit-and-delete');

if (!bypassBlockTimeLimit && blockEditInMinutes) {
let msgTs;
if (message.ts != null) {
msgTs = moment(message.ts);
Expand Down
11 changes: 7 additions & 4 deletions apps/meteor/client/lib/chats/data.ts
Expand Up @@ -86,17 +86,19 @@ export const createDataAPI = ({ rid, tmid }: { rid: IRoom['_id']; tmid: IMessage
return false;
}

const hasPermission = hasAtLeastOnePermission('edit-message', message.rid);
const canEditMessage = hasAtLeastOnePermission('edit-message', message.rid);
const editAllowed = (settings.get('Message_AllowEditing') as boolean | undefined) ?? false;
const editOwn = message?.u && message.u._id === Meteor.userId();

if (!hasPermission && (!editAllowed || !editOwn)) {
if (!canEditMessage && (!editAllowed || !editOwn)) {
return false;
}

const blockEditInMinutes = settings.get('Message_AllowEditing_BlockEditInMinutes') as number | undefined;
const bypassBlockTimeLimit = hasPermission('bypass-time-limit-edit-and-delete');

const elapsedMinutes = moment().diff(message.ts, 'minutes');
if (elapsedMinutes && blockEditInMinutes && elapsedMinutes > blockEditInMinutes) {
if (!bypassBlockTimeLimit && elapsedMinutes && blockEditInMinutes && elapsedMinutes > blockEditInMinutes) {
return false;
}

Expand Down Expand Up @@ -206,8 +208,9 @@ export const createDataAPI = ({ rid, tmid }: { rid: IRoom['_id']; tmid: IMessage
}

const blockDeleteInMinutes = settings.get('Message_AllowDeleting_BlockDeleteInMinutes') as number | undefined;
const bypassBlockTimeLimit = hasPermission('bypass-time-limit-edit-and-delete');
const elapsedMinutes = moment().diff(message.ts, 'minutes');
const onTimeForDelete = !blockDeleteInMinutes || !elapsedMinutes || elapsedMinutes <= blockDeleteInMinutes;
const onTimeForDelete = bypassBlockTimeLimit || !blockDeleteInMinutes || !elapsedMinutes || elapsedMinutes <= blockDeleteInMinutes;

return deleteAllowed && onTimeForDelete;
};
Expand Down
10 changes: 6 additions & 4 deletions apps/meteor/client/methods/updateMessage.ts
Expand Up @@ -4,7 +4,7 @@ import { Tracker } from 'meteor/tracker';
import moment from 'moment';
import _ from 'underscore';

import { hasAtLeastOnePermission } from '../../app/authorization/client';
import { hasAtLeastOnePermission, hasPermission } from '../../app/authorization/client';
import { ChatMessage } from '../../app/models/client';
import { settings } from '../../app/settings/client';
import { t } from '../../app/utils/client';
Expand All @@ -23,7 +23,7 @@ Meteor.methods({
if (!originalMessage) {
return;
}
const hasPermission = hasAtLeastOnePermission('edit-message', message.rid);
const canEditMessage = hasAtLeastOnePermission('edit-message', message.rid);
const editAllowed = settings.get('Message_AllowEditing');
let editOwn = false;

Expand All @@ -42,7 +42,7 @@ Meteor.methods({
return false;
}

if (!(hasPermission || (editAllowed && editOwn))) {
if (!(canEditMessage || (editAllowed && editOwn))) {
dispatchToastMessage({
type: 'error',
message: t('error-action-not-allowed', { action: t('Message_editing') }),
Expand All @@ -51,7 +51,9 @@ Meteor.methods({
}

const blockEditInMinutes = settings.get('Message_AllowEditing_BlockEditInMinutes');
if (_.isNumber(blockEditInMinutes) && blockEditInMinutes !== 0) {
const bypassBlockTimeLimit = hasPermission('bypass-time-limit-edit-and-delete');

if (!bypassBlockTimeLimit && _.isNumber(blockEditInMinutes) && blockEditInMinutes !== 0) {
if (originalMessage.ts) {
const msgTs = moment(originalMessage.ts);
if (msgTs) {
Expand Down
Expand Up @@ -8,6 +8,7 @@ export const useMessageDeletionIsAllowed = (rid, uid) => {
const deletionIsEnabled = useSetting('Message_AllowDeleting');
const userHasPermissonToDeleteAny = usePermission('delete-message', rid);
const userHasPermissonToDeleteOwn = usePermission('delete-own-message');
const bypassBlockTimeLimit = usePermission('bypass-time-limit-edit-and-delete');
const blockDeleteInMinutes = useSetting('Message_AllowDeleting_BlockDeleteInMinutes');

const isDeletionAllowed = (() => {
Expand All @@ -24,7 +25,7 @@ export const useMessageDeletionIsAllowed = (rid, uid) => {
}

const checkTimeframe =
blockDeleteInMinutes !== 0
!bypassBlockTimeLimit && blockDeleteInMinutes !== 0
? ({ ts }) => {
if (!ts) {
return false;
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json
Expand Up @@ -295,6 +295,8 @@
"add-livechat-department-agents_description": "Permission to add omnichannel agents to departments",
"add-oauth-service": "Add OAuth Service",
"add-oauth-service_description": "Permission to add a new OAuth service",
"bypass-time-limit-edit-and-delete": "Bypass time limit",
"bypass-time-limit-edit-and-delete_description": "Permission to Bypass time limit for editing and deleting messages",
"add-team-channel": "Add Team Channel",
"add-team-channel_description": "Permission to add a channel to a team",
"add-team-member": "Add Team Member",
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/server/startup/migrations/index.ts
Expand Up @@ -43,4 +43,5 @@ import './v281';
import './v282';
import './v283';
import './v284';
import './v285';
import './xrun';
9 changes: 9 additions & 0 deletions apps/meteor/server/startup/migrations/v285.ts
@@ -0,0 +1,9 @@
import { addMigration } from '../../lib/migrations';
import { upsertPermissions } from '../../../app/authorization/server/functions/upsertPermissions';

addMigration({
version: 285,
up() {
upsertPermissions();
},
});
158 changes: 157 additions & 1 deletion apps/meteor/tests/end-to-end/api/24-methods.js
@@ -1,7 +1,7 @@
import { expect } from 'chai';

import { getCredentials, request, methodCall, api, credentials } from '../../data/api-data.js';
import { updatePermission } from '../../data/permissions.helper.js';
import { updatePermission, updateSetting } from '../../data/permissions.helper.js';

describe('Meteor.methods', function () {
this.retries(0);
Expand Down Expand Up @@ -1623,6 +1623,46 @@ describe('Meteor.methods', function () {
.end(done);
});

it('should update a message when bypass time limits permission is enabled', async () => {
await Promise.all([
updatePermission('bypass-time-limit-edit-and-delete', ['admin']),
updateSetting('Message_AllowEditing_BlockEditInMinutes', 0.01),
]);

await request
.post(methodCall('updateMessage'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'updateMessage',
params: [{ _id: messageId, rid, msg: 'https://github.com updated with bypass' }],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.a.property('success', true);
expect(res.body).to.have.a.property('message').that.is.a('string');
});

await request
.get(api(`chat.getMessage?msgId=${messageId}`))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('message').that.is.an('object');
expect(res.body.message.msg).to.equal('https://github.com updated with bypass');
});

await Promise.all([
updatePermission('bypass-time-limit-edit-and-delete', ['bot', 'app']),
updateSetting('Message_AllowEditing_BlockEditInMinutes', 0),
]);
});

it('should not parse URLs inside markdown on update', (done) => {
request
.post(methodCall('updateMessage'))
Expand Down Expand Up @@ -1667,6 +1707,122 @@ describe('Meteor.methods', function () {
});
});

describe('[@deleteMessage]', () => {
let rid = false;
let messageId;

before('create room', (done) => {
const channelName = `methods-test-channel-${Date.now()}`;
request
.post(api('groups.create'))
.set(credentials)
.send({
name: channelName,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.nested.property('group._id');
expect(res.body).to.have.nested.property('group.name', channelName);
expect(res.body).to.have.nested.property('group.t', 'p');
expect(res.body).to.have.nested.property('group.msgs', 0);
rid = res.body.group._id;
})
.end(done);
});

beforeEach('send message with URL', (done) => {
request
.post(methodCall('sendMessage'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'sendMessage',
params: [
{
_id: `${Date.now() + Math.random()}`,
rid,
msg: 'test message with https://github.com',
},
],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.a.property('success', true);
expect(res.body).to.have.a.property('message').that.is.a('string');

const data = JSON.parse(res.body.message);
expect(data).to.have.a.property('result').that.is.an('object');
expect(data.result).to.have.a.property('urls').that.is.an('array');
expect(data.result.urls[0].url).to.equal('https://github.com');
messageId = data.result._id;
})
.end(done);
});

it('should delete a message', (done) => {
request
.post(methodCall('deleteMessage'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'deleteMessage',
params: [{ _id: messageId, rid }],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.a.property('success', true);
expect(res.body).to.have.a.property('message').that.is.a('string');
const data = JSON.parse(res.body.message);
expect(data).to.have.a.property('msg', 'result');
expect(data).to.have.a.property('id', 'id');
})
.end(done);
});

it('should delete a message when bypass time limits permission is enabled', async () => {
await Promise.all([
updatePermission('bypass-time-limit-edit-and-delete', ['admin']),
updateSetting('Message_AllowEditing_BlockEditInMinutes', 0.01),
]);

await request
.post(methodCall('deleteMessage'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'deleteMessage',
params: [{ _id: messageId, rid }],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.a.property('success', true);
expect(res.body).to.have.a.property('message').that.is.a('string');
const data = JSON.parse(res.body.message);
expect(data).to.have.a.property('msg', 'result');
expect(data).to.have.a.property('id', 'id');
});

await Promise.all([
updatePermission('bypass-time-limit-edit-and-delete', ['bot', 'app']),
updateSetting('Message_AllowEditing_BlockEditInMinutes', 0),
]);
});
});

describe('[@setUserActiveStatus]', () => {
let testUser;
let testUser2;
Expand Down