From 3b650018e568fe9e1aa5f5663d612e9c493c0445 Mon Sep 17 00:00:00 2001
From: Kevin Aleman <kaleman960@gmail.com>
Date: Mon, 29 Apr 2024 10:25:22 -0600
Subject: [PATCH] fix: Server sending 2 notifications when `@all/@here` were
 used by a user without permissions (#32289)

---
 .changeset/flat-starfishes-crash.md           |  5 ++
 .../app/lib/server/methods/sendMessage.ts     |  3 +-
 .../hooks/BeforeSavePreventMention.ts         | 13 +---
 .../server/services/messages/service.ts       |  2 +-
 .../meteor/tests/e2e/message-mentions.spec.ts | 62 +++++++++++++++++++
 5 files changed, 72 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/flat-starfishes-crash.md

diff --git a/.changeset/flat-starfishes-crash.md b/.changeset/flat-starfishes-crash.md
new file mode 100644
index 000000000000..9c5bb2425f19
--- /dev/null
+++ b/.changeset/flat-starfishes-crash.md
@@ -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
diff --git a/apps/meteor/app/lib/server/methods/sendMessage.ts b/apps/meteor/app/lib/server/methods/sendMessage.ts
index e12ebc2d47e9..5749daa980f3 100644
--- a/apps/meteor/app/lib/server/methods/sendMessage.ts
+++ b/apps/meteor/app/lib/server/methods/sendMessage.ts
@@ -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') {
diff --git a/apps/meteor/server/services/messages/hooks/BeforeSavePreventMention.ts b/apps/meteor/server/services/messages/hooks/BeforeSavePreventMention.ts
index b87c0e613d2d..d4a278e85700 100644
--- a/apps/meteor/server/services/messages/hooks/BeforeSavePreventMention.ts
+++ b/apps/meteor/server/services/messages/hooks/BeforeSavePreventMention.ts
@@ -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,
@@ -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,
 		});
 	}
 }
diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts
index f0c30c359110..e18a9c276c50 100644
--- a/apps/meteor/server/services/messages/service.ts
+++ b/apps/meteor/server/services/messages/service.ts
@@ -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({
diff --git a/apps/meteor/tests/e2e/message-mentions.spec.ts b/apps/meteor/tests/e2e/message-mentions.spec.ts
index 01fb2dde4217..1d956dfcb735 100644
--- a/apps/meteor/tests/e2e/message-mentions.spec.ts
+++ b/apps/meteor/tests/e2e/message-mentions.spec.ts
@@ -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;