Skip to content

Commit

Permalink
fix: Server sending 2 notifications when @all/@here were used by a …
Browse files Browse the repository at this point in the history
…user without permissions (RocketChat#32289)
  • Loading branch information
KevLehman authored Apr 29, 2024
1 parent 105a1eb commit 3b65001
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/flat-starfishes-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixed a problem in how server was processing errors that was sending 2 ephemeral error messages when @all or @here were used while they were disabled via permissions
3 changes: 2 additions & 1 deletion apps/meteor/app/lib/server/methods/sendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ export async function executeSendMessage(uid: IUser['_id'], message: AtLeast<IMe
SystemLogger.error({ msg: 'Error sending message:', err });

const errorMessage = typeof err === 'string' ? err : err.error || err.message;
const errorContext = err.details ?? {};
void api.broadcast('notify.ephemeralMessage', uid, message.rid, {
msg: i18n.t(errorMessage, { lng: user.language }),
msg: i18n.t(errorMessage, errorContext, user.language),
});

if (typeof err === 'string') {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { Authorization, MeteorError, type IApiService } from '@rocket.chat/core-services';
import { Authorization, MeteorError } from '@rocket.chat/core-services';
import { type IMessage, type IUser } from '@rocket.chat/core-typings';

import { i18n } from '../../../lib/i18n';

export class BeforeSavePreventMention {
constructor(private api?: IApiService) {}

async preventMention({
message,
user,
Expand All @@ -32,16 +30,9 @@ export class BeforeSavePreventMention {

const action = i18n.t('Notify_all_in_this_room', { lng: user.language });

// Add a notification to the chat, informing the user that this
// action is not allowed.
void this.api?.broadcast('notify.ephemeralMessage', message.u._id, message.rid, {
// TODO: i18n
msg: i18n.t('error-action-not-allowed', { action } as any, user.language),
});

// Also throw to stop propagation of 'sendMessage'.
throw new MeteorError('error-action-not-allowed', `Notify ${mention} in this room not allowed`, {
action: 'Notify_all_in_this_room',
action,
});
}
}
2 changes: 1 addition & 1 deletion apps/meteor/server/services/messages/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
private checkMAC: BeforeSaveCheckMAC;

async created() {
this.preventMention = new BeforeSavePreventMention(this.api);
this.preventMention = new BeforeSavePreventMention();
this.badWords = new BeforeSaveBadWords();
this.spotify = new BeforeSaveSpotify();
this.jumpToMessage = new BeforeSaveJumpToMessage({
Expand Down
62 changes: 62 additions & 0 deletions apps/meteor/tests/e2e/message-mentions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,68 @@ test.describe.serial('message-mentions', () => {
await expect(poHomeChannel.content.messagePopupUsers.locator('role=listitem >> text="here"')).toBeVisible();
});

test.describe('Should not allow to send @all mention if permission to do so is disabled', () => {
let targetChannel2: string;
test.beforeAll(async ({ api }) => {
expect((await api.post('/permissions.update', { permissions: [{ '_id': 'mention-all', 'roles': [] }] })).status()).toBe(200);
});

test.afterAll(async ({ api }) => {
expect((await api.post('/permissions.update', { permissions: [{ '_id': 'mention-all', 'roles': ['admin', 'owner', 'moderator', 'user'] }] })).status()).toBe(200);
await deleteChannel(api, targetChannel2);
});

test('expect to receive an error as notification when sending @all while permission is disabled', async ({ page }) => {
const adminPage = new HomeChannel(page);

await test.step('create private room', async () => {
targetChannel2 = faker.string.uuid();

await poHomeChannel.sidenav.openNewByLabel('Channel');
await poHomeChannel.sidenav.inputChannelName.type(targetChannel2);
await poHomeChannel.sidenav.btnCreate.click();

await expect(page).toHaveURL(`/group/${targetChannel2}`);
});
await test.step('receive notify message', async () => {
await adminPage.sidenav.openChat(targetChannel2);
await adminPage.content.dispatchSlashCommand('@all');
await expect(adminPage.content.lastUserMessage).toContainText('Notify all in this room is not allowed');
});
});
});

test.describe('Should not allow to send @here mention if permission to do so is disabled', () => {
let targetChannel2: string;
test.beforeAll(async ({ api }) => {
expect((await api.post('/permissions.update', { permissions: [{ '_id': 'mention-here', 'roles': [] }] })).status()).toBe(200);
});

test.afterAll(async ({ api }) => {
expect((await api.post('/permissions.update', { permissions: [{ '_id': 'mention-here', 'roles': ['admin', 'owner', 'moderator', 'user'] }] })).status()).toBe(200);
await deleteChannel(api, targetChannel2);
});

test('expect to receive an error as notification when sending here while permission is disabled', async ({ page }) => {
const adminPage = new HomeChannel(page);

await test.step('create private room', async () => {
targetChannel2 = faker.string.uuid();

await poHomeChannel.sidenav.openNewByLabel('Channel');
await poHomeChannel.sidenav.inputChannelName.type(targetChannel2);
await poHomeChannel.sidenav.btnCreate.click();

await expect(page).toHaveURL(`/group/${targetChannel2}`);
});
await test.step('receive notify message', async () => {
await adminPage.sidenav.openChat(targetChannel2);
await adminPage.content.dispatchSlashCommand('@here');
await expect(adminPage.content.lastUserMessage).toContainText('Notify all in this room is not allowed');
});
});
});

test.describe('users not in channel', () => {
let targetChannel: string;
let targetChannel2: string;
Expand Down

0 comments on commit 3b65001

Please sign in to comment.