From c714962b0e2f83175379e24abe83d01a01e071bd Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 22 Sep 2023 14:43:45 -0300 Subject: [PATCH 1/4] fix: Message disappears from room after deletion even if "Show Deleted Status" is enabled (#30452) --- .changeset/kind-books-love.md | 5 +++++ apps/meteor/client/lib/chats/data.ts | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 .changeset/kind-books-love.md diff --git a/.changeset/kind-books-love.md b/.changeset/kind-books-love.md new file mode 100644 index 000000000000..40ce15453ff4 --- /dev/null +++ b/.changeset/kind-books-love.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixed message disappearing from room after erased even if "Show Deleted Status" is enabled diff --git a/apps/meteor/client/lib/chats/data.ts b/apps/meteor/client/lib/chats/data.ts index bd6e01458863..f2c049ad04b1 100644 --- a/apps/meteor/client/lib/chats/data.ts +++ b/apps/meteor/client/lib/chats/data.ts @@ -217,7 +217,6 @@ export const createDataAPI = ({ rid, tmid }: { rid: IRoom['_id']; tmid: IMessage const deleteMessage = async (mid: IMessage['_id']): Promise => { await sdk.call('deleteMessage', { _id: mid }); - Messages.remove({ _id: mid }); }; const drafts = new Map(); From 3f78ae475cf7b767ab9bcfb01117e4af84c57e05 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Mon, 25 Sep 2023 10:30:38 -0300 Subject: [PATCH 2/4] chore: Move bad words filter callback to service (#30241) --- apps/meteor/app/api/server/v1/channels.ts | 5 +- .../server/hooks/joinDiscussionOnMessage.ts | 29 ------- .../server/functions/notifications/index.ts | 5 -- .../app/lib/server/functions/sendMessage.js | 3 + .../app/lib/server/functions/updateMessage.ts | 15 ++-- apps/meteor/app/lib/server/index.ts | 1 - .../server/lib/sendNotificationsOnMessage.js | 10 +-- .../app/lib/server/methods/filterBadWords.ts | 65 ---------------- .../meteor/app/lib/server/methods/joinRoom.ts | 52 +++---------- .../app/message-pin/server/pinMessage.ts | 23 +++--- .../app/slashcommands-join/server/server.ts | 11 ++- .../server/lib/rooms/roomTypes/direct.ts | 4 +- .../services/messages/hooks/badwords.ts | 26 +++++++ .../server/services/messages/service.ts | 78 +++++++++++++++++++ apps/meteor/server/services/room/service.ts | 30 ++++++- .../src/types/IMessageService.ts | 1 + .../core-services/src/types/IRoomService.ts | 1 + 17 files changed, 180 insertions(+), 179 deletions(-) delete mode 100644 apps/meteor/app/discussion/server/hooks/joinDiscussionOnMessage.ts delete mode 100644 apps/meteor/app/lib/server/methods/filterBadWords.ts create mode 100644 apps/meteor/server/services/messages/hooks/badwords.ts diff --git a/apps/meteor/app/api/server/v1/channels.ts b/apps/meteor/app/api/server/v1/channels.ts index 70b7fc875082..4a7aec073442 100644 --- a/apps/meteor/app/api/server/v1/channels.ts +++ b/apps/meteor/app/api/server/v1/channels.ts @@ -1,4 +1,4 @@ -import { Team } from '@rocket.chat/core-services'; +import { Team, Room } from '@rocket.chat/core-services'; import type { IRoom, ISubscription, IUser, RoomType } from '@rocket.chat/core-typings'; import { Integrations, Messages, Rooms, Subscriptions, Uploads, Users } from '@rocket.chat/models'; import { @@ -31,7 +31,6 @@ import { saveRoomSettings } from '../../../channel-settings/server/methods/saveR import { mountIntegrationQueryBasedOnPermissions } from '../../../integrations/server/lib/mountQueriesBasedOnPermission'; import { addUsersToRoomMethod } from '../../../lib/server/methods/addUsersToRoom'; import { createChannelMethod } from '../../../lib/server/methods/createChannel'; -import { joinRoomMethod } from '../../../lib/server/methods/joinRoom'; import { leaveRoomMethod } from '../../../lib/server/methods/leaveRoom'; import { settings } from '../../../settings/server'; import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser'; @@ -209,7 +208,7 @@ API.v1.addRoute( const { joinCode, ...params } = this.bodyParams; const findResult = await findChannelByIdOrName({ params }); - await joinRoomMethod(this.userId, findResult._id, joinCode); + await Room.join({ room: findResult, user: this.user, joinCode }); return API.v1.success({ channel: await findChannelByIdOrName({ params, userId: this.userId }), diff --git a/apps/meteor/app/discussion/server/hooks/joinDiscussionOnMessage.ts b/apps/meteor/app/discussion/server/hooks/joinDiscussionOnMessage.ts deleted file mode 100644 index b953d4658c85..000000000000 --- a/apps/meteor/app/discussion/server/hooks/joinDiscussionOnMessage.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Subscriptions } from '@rocket.chat/models'; - -import { callbacks } from '../../../../lib/callbacks'; -import { joinRoomMethod } from '../../../lib/server/methods/joinRoom'; - -callbacks.add( - 'beforeSaveMessage', - async (message, room) => { - // abort if room is not a discussion - if (!room?.prid) { - return message; - } - - // check if user already joined the discussion - const sub = await Subscriptions.findOneByRoomIdAndUserId(room._id, message.u._id, { - projection: { _id: 1 }, - }); - - if (sub) { - return message; - } - - await joinRoomMethod(message.u._id, room._id); - - return message; - }, - callbacks.priority.MEDIUM, - 'joinDiscussionOnMessage', -); diff --git a/apps/meteor/app/lib/server/functions/notifications/index.ts b/apps/meteor/app/lib/server/functions/notifications/index.ts index 934014b794a1..11e4418c4510 100644 --- a/apps/meteor/app/lib/server/functions/notifications/index.ts +++ b/apps/meteor/app/lib/server/functions/notifications/index.ts @@ -5,7 +5,6 @@ import { escapeRegExp } from '@rocket.chat/string-helpers'; import { callbacks } from '../../../../../lib/callbacks'; import { i18n } from '../../../../../server/lib/i18n'; import { settings } from '../../../../settings/server'; -import { joinRoomMethod } from '../../methods/joinRoom'; /** * This function returns a string ready to be shown in the notification @@ -66,7 +65,3 @@ export function messageContainsHighlight(message: IMessage, highlights: string[] return regexp.test(message.msg); }); } - -export async function callJoinRoom(userId: string, rid: string): Promise { - await joinRoomMethod(userId, rid); -} diff --git a/apps/meteor/app/lib/server/functions/sendMessage.js b/apps/meteor/app/lib/server/functions/sendMessage.js index a1399b5b19e9..72247d4b1870 100644 --- a/apps/meteor/app/lib/server/functions/sendMessage.js +++ b/apps/meteor/app/lib/server/functions/sendMessage.js @@ -1,3 +1,4 @@ +import { Message } from '@rocket.chat/core-services'; import { Messages } from '@rocket.chat/models'; import { Match, check } from 'meteor/check'; @@ -247,6 +248,8 @@ export const sendMessage = async function (user, message, room, upsert = false, parseUrlsInMessage(message, previewUrls); + message = await Message.beforeSave({ message, room, user }); + message = await callbacks.run('beforeSaveMessage', message, room); if (message) { if (message.t === 'otr') { diff --git a/apps/meteor/app/lib/server/functions/updateMessage.ts b/apps/meteor/app/lib/server/functions/updateMessage.ts index 05c17906374e..9c544bd9a333 100644 --- a/apps/meteor/app/lib/server/functions/updateMessage.ts +++ b/apps/meteor/app/lib/server/functions/updateMessage.ts @@ -1,3 +1,4 @@ +import { Message } from '@rocket.chat/core-services'; import type { IEditedMessage, IMessage, IUser, AtLeast } from '@rocket.chat/core-typings'; import { Messages, Rooms } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; @@ -48,6 +49,14 @@ export const updateMessage = async function ( parseUrlsInMessage(message, previewUrls); + const room = await Rooms.findOneById(message.rid); + if (!room) { + return; + } + + // TODO remove type cast + message = await Message.beforeSave({ message: message as IMessage, room, user }); + message = await callbacks.run('beforeSaveMessage', message); const { _id, ...editedMessage } = message; @@ -67,12 +76,6 @@ export const updateMessage = async function ( }, ); - const room = await Rooms.findOneById(message.rid); - - if (!room) { - return; - } - if (Apps?.isLoaded()) { // This returns a promise, but it won't mutate anything about the message // so, we don't really care if it is successful or fails diff --git a/apps/meteor/app/lib/server/index.ts b/apps/meteor/app/lib/server/index.ts index 597d03752dcc..8fa779ec9644 100644 --- a/apps/meteor/app/lib/server/index.ts +++ b/apps/meteor/app/lib/server/index.ts @@ -23,7 +23,6 @@ import './methods/deleteUserOwnAccount'; import './methods/executeSlashCommandPreview'; import './startup/filterATAllTag'; import './startup/filterATHereTag'; -import './methods/filterBadWords'; import './methods/getChannelHistory'; import './methods/getRoomJoinCode'; import './methods/getRoomRoles'; diff --git a/apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.js b/apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.js index 17296d8f374a..ce262e4e6756 100644 --- a/apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.js +++ b/apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.js @@ -1,3 +1,4 @@ +import { Room } from '@rocket.chat/core-services'; import { Subscriptions, Users } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; import moment from 'moment'; @@ -7,12 +8,7 @@ import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator'; import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { Notification } from '../../../notification-queue/server/NotificationQueue'; import { settings } from '../../../settings/server'; -import { - callJoinRoom, - messageContainsHighlight, - parseMessageTextPerUser, - replaceMentionedUsernamesWithFullNames, -} from '../functions/notifications'; +import { messageContainsHighlight, parseMessageTextPerUser, replaceMentionedUsernamesWithFullNames } from '../functions/notifications'; import { notifyDesktopUser, shouldNotifyDesktop } from '../functions/notifications/desktop'; import { getEmailData, shouldNotifyEmail } from '../functions/notifications/email'; import { getPushData, shouldNotifyMobile } from '../functions/notifications/mobile'; @@ -365,7 +361,7 @@ export async function sendAllNotifications(message, room) { const users = await Promise.all( mentions.map(async (userId) => { - await callJoinRoom(userId, room._id); + await Room.join({ room, user: { _id: userId } }); return userId; }), diff --git a/apps/meteor/app/lib/server/methods/filterBadWords.ts b/apps/meteor/app/lib/server/methods/filterBadWords.ts deleted file mode 100644 index 3db0ee76e997..000000000000 --- a/apps/meteor/app/lib/server/methods/filterBadWords.ts +++ /dev/null @@ -1,65 +0,0 @@ -import type { IMessage } from '@rocket.chat/core-typings'; -import Filter from 'bad-words'; -import { Meteor } from 'meteor/meteor'; -import { Tracker } from 'meteor/tracker'; - -import { callbacks } from '../../../../lib/callbacks'; -import { settings } from '../../../settings/server'; - -const Dep = new Tracker.Dependency(); -Meteor.startup(() => { - settings.watchMultiple(['Message_AllowBadWordsFilter', 'Message_BadWordsFilterList', 'Message_BadWordsWhitelist'], () => { - Dep.changed(); - }); - Tracker.autorun(() => { - Dep.depend(); - const allowBadWordsFilter = settings.get('Message_AllowBadWordsFilter'); - - callbacks.remove('beforeSaveMessage', 'filterBadWords'); - - if (!allowBadWordsFilter) { - return; - } - - const badWordsList = settings.get('Message_BadWordsFilterList') as string | undefined; - const whiteList = settings.get('Message_BadWordsWhitelist') as string | undefined; - - const options = { - list: - badWordsList - ?.split(',') - .map((word) => word.trim()) - .filter(Boolean) || [], - // library definition does not allow optional definition - exclude: undefined, - splitRegex: undefined, - placeHolder: undefined, - regex: undefined, - replaceRegex: undefined, - emptyList: undefined, - }; - - const filter = new Filter(options); - - if (whiteList?.length) { - filter.removeWords(...whiteList.split(',').map((word) => word.trim())); - } - - callbacks.add( - 'beforeSaveMessage', - (message: IMessage) => { - if (!message.msg) { - return message; - } - try { - message.msg = filter.clean(message.msg); - } finally { - // eslint-disable-next-line no-unsafe-finally - return message; - } - }, - callbacks.priority.HIGH, - 'filterBadWords', - ); - }); -}); diff --git a/apps/meteor/app/lib/server/methods/joinRoom.ts b/apps/meteor/app/lib/server/methods/joinRoom.ts index 355fd49916ae..0fa3ac0b3c3b 100644 --- a/apps/meteor/app/lib/server/methods/joinRoom.ts +++ b/apps/meteor/app/lib/server/methods/joinRoom.ts @@ -1,63 +1,31 @@ -import type { IRoom, IRoomWithJoinCode, IUser } from '@rocket.chat/core-typings'; -import { Rooms, Users } from '@rocket.chat/models'; +import { Room } from '@rocket.chat/core-services'; +import type { IRoom } from '@rocket.chat/core-typings'; +import { Rooms } from '@rocket.chat/models'; import type { ServerMethods } from '@rocket.chat/ui-contexts'; import { check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; -import { RoomMemberActions } from '../../../../definition/IRoomTypeConfig'; -import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator'; -import { canAccessRoomAsync } from '../../../authorization/server'; -import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; -import { addUserToRoom } from '../functions/addUserToRoom'; - declare module '@rocket.chat/ui-contexts' { // eslint-disable-next-line @typescript-eslint/naming-convention interface ServerMethods { - joinRoom(rid: IRoom['_id'], code?: unknown): boolean | undefined; + joinRoom(rid: IRoom['_id'], code?: string): boolean | undefined; } } -export const joinRoomMethod = async (userId: IUser['_id'], rid: IRoom['_id'], code?: unknown): Promise => { - check(rid, String); - - const user = await Users.findOneById(userId); - - if (!user) { - throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'joinRoom' }); - } - - const room = await Rooms.findOneById(rid); - - if (!room) { - throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'joinRoom' }); - } - - if (!(await roomCoordinator.getRoomDirectives(room.t)?.allowMemberAction(room, RoomMemberActions.JOIN, user._id))) { - throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'joinRoom' }); - } - - if (!(await canAccessRoomAsync(room, user))) { - throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'joinRoom' }); - } - if (room.joinCodeRequired === true && code !== room.joinCode && !(await hasPermissionAsync(user._id, 'join-without-join-code'))) { - throw new Meteor.Error('error-code-invalid', 'Invalid Room Password', { - method: 'joinRoom', - }); - } - - return addUserToRoom(rid, user); -}; - Meteor.methods({ async joinRoom(rid, code) { check(rid, String); const userId = await Meteor.userId(); - if (!userId) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'joinRoom' }); } - return joinRoomMethod(userId, rid, code); + const room = await Rooms.findOneById(rid); + if (!room) { + throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'joinRoom' }); + } + + return Room.join({ room, user: { _id: userId }, ...(code ? { joinCode: code } : {}) }); }, }); diff --git a/apps/meteor/app/message-pin/server/pinMessage.ts b/apps/meteor/app/message-pin/server/pinMessage.ts index a36883abb0a5..906f0c98c181 100644 --- a/apps/meteor/app/message-pin/server/pinMessage.ts +++ b/apps/meteor/app/message-pin/server/pinMessage.ts @@ -1,6 +1,6 @@ import { Message } from '@rocket.chat/core-services'; -import { isQuoteAttachment } from '@rocket.chat/core-typings'; -import type { IMessage, IUser, MessageAttachment, MessageQuoteAttachment } from '@rocket.chat/core-typings'; +import { isQuoteAttachment, isRegisterUser } from '@rocket.chat/core-typings'; +import type { IMessage, MessageAttachment, MessageQuoteAttachment } from '@rocket.chat/core-typings'; import { Messages, Rooms, Subscriptions, Users, ReadReceipts } from '@rocket.chat/models'; import type { ServerMethods } from '@rocket.chat/ui-contexts'; import { check } from 'meteor/check'; @@ -82,15 +82,13 @@ Meteor.methods({ throw new Meteor.Error('not-authorized', 'Not Authorized', { method: 'pinMessage' }); } - const me = await Users.findOneById>>(userId, { - projection: { username: 1, name: 1 }, - }); + const me = await Users.findOneById(userId); if (!me) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'pinMessage' }); } // If we keep history of edits, insert a new message to store history information - if (settings.get('Message_KeepHistory') && me.username) { + if (settings.get('Message_KeepHistory') && isRegisterUser(me)) { await Messages.cloneAndSaveAsHistoryById(message._id, me); } @@ -110,6 +108,8 @@ Meteor.methods({ username: me.username, }; + originalMessage = await Message.beforeSave({ message: originalMessage, room, user: me }); + originalMessage = await callbacks.run('beforeSaveMessage', originalMessage); await Messages.setPinnedByIdAndUserId(originalMessage._id, originalMessage.pinnedBy, originalMessage.pinned); @@ -186,15 +186,13 @@ Meteor.methods({ throw new Meteor.Error('not-authorized', 'Not Authorized', { method: 'unpinMessage' }); } - const me = await Users.findOneById>>(userId, { - projection: { username: 1, name: 1 }, - }); + const me = await Users.findOneById(userId); if (!me) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'unpinMessage' }); } // If we keep history of edits, insert a new message to store history information - if (settings.get('Message_KeepHistory') && me.username) { + if (settings.get('Message_KeepHistory') && isRegisterUser(me)) { await Messages.cloneAndSaveAsHistoryById(originalMessage._id, me); } @@ -203,7 +201,6 @@ Meteor.methods({ _id: userId, username: me.username, }; - originalMessage = await callbacks.run('beforeSaveMessage', originalMessage); const room = await Rooms.findOneById(originalMessage.rid, { projection: { ...roomAccessAttributes, lastMessage: 1 } }); if (!room) { @@ -214,6 +211,10 @@ Meteor.methods({ throw new Meteor.Error('not-authorized', 'Not Authorized', { method: 'unpinMessage' }); } + originalMessage = await Message.beforeSave({ message: originalMessage, room, user: me }); + + originalMessage = await callbacks.run('beforeSaveMessage', originalMessage); + if (isTheLastMessage(room, message)) { await Rooms.setLastMessagePinned(room._id, originalMessage.pinnedBy, originalMessage.pinned); } diff --git a/apps/meteor/app/slashcommands-join/server/server.ts b/apps/meteor/app/slashcommands-join/server/server.ts index dfe27d8d5dc4..33d0278f81a3 100644 --- a/apps/meteor/app/slashcommands-join/server/server.ts +++ b/apps/meteor/app/slashcommands-join/server/server.ts @@ -1,10 +1,9 @@ -import { api } from '@rocket.chat/core-services'; +import { api, Room } from '@rocket.chat/core-services'; import type { SlashCommandCallbackParams } from '@rocket.chat/core-typings'; import { Rooms, Subscriptions } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; import { i18n } from '../../../server/lib/i18n'; -import { joinRoomMethod } from '../../lib/server/methods/joinRoom'; import { settings } from '../../settings/server'; import { slashCommands } from '../../utils/lib/slashCommand'; @@ -16,13 +15,13 @@ slashCommands.add({ return; } - channel = channel.replace('#', ''); - const room = await Rooms.findOneByNameAndType(channel, 'c'); - if (!userId) { return; } + channel = channel.replace('#', ''); + + const room = await Rooms.findOneByNameAndType(channel, 'c'); if (!room) { void api.broadcast('notify.ephemeralMessage', userId, message.rid, { msg: i18n.t('Channel_doesnt_exist', { @@ -44,7 +43,7 @@ slashCommands.add({ }); } - await joinRoomMethod(userId, room._id); + await Room.join({ room, user: { _id: userId } }); }, options: { description: 'Join_the_given_channel', diff --git a/apps/meteor/server/lib/rooms/roomTypes/direct.ts b/apps/meteor/server/lib/rooms/roomTypes/direct.ts index b18418258cb3..ad1913345b85 100644 --- a/apps/meteor/server/lib/rooms/roomTypes/direct.ts +++ b/apps/meteor/server/lib/rooms/roomTypes/direct.ts @@ -1,4 +1,4 @@ -import type { IRoom, AtLeast } from '@rocket.chat/core-typings'; +import type { AtLeast } from '@rocket.chat/core-typings'; import { isRoomFederated } from '@rocket.chat/core-typings'; import { Subscriptions } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; @@ -42,7 +42,7 @@ roomCoordinator.add(DirectMessageRoomType, { } }, - async allowMemberAction(room: IRoom, action, userId) { + async allowMemberAction(room, action, userId) { if (isRoomFederated(room)) { return Federation.actionAllowed(room, action, userId); } diff --git a/apps/meteor/server/services/messages/hooks/badwords.ts b/apps/meteor/server/services/messages/hooks/badwords.ts new file mode 100644 index 000000000000..17641d8f9c7f --- /dev/null +++ b/apps/meteor/server/services/messages/hooks/badwords.ts @@ -0,0 +1,26 @@ +export async function configureBadWords(badWordsList?: string, goodWordsList?: string) { + const { default: Filter } = await import('bad-words'); + + const options = { + list: + badWordsList + ?.split(',') + .map((word) => word.trim()) + .filter(Boolean) || [], + // library definition does not allow optional definition + exclude: undefined, + splitRegex: undefined, + placeHolder: undefined, + regex: undefined, + replaceRegex: undefined, + emptyList: undefined, + }; + + const badWords = new Filter(options); + + if (goodWordsList?.length) { + badWords.removeWords(...goodWordsList.split(',').map((word) => word.trim())); + } + + return badWords; +} diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts index 48f7ad42276c..f981422727f7 100644 --- a/apps/meteor/server/services/messages/service.ts +++ b/apps/meteor/server/services/messages/service.ts @@ -2,6 +2,7 @@ import type { IMessageService } from '@rocket.chat/core-services'; import { ServiceClassInternal } from '@rocket.chat/core-services'; import type { IMessage, MessageTypesValues, IUser, IRoom } from '@rocket.chat/core-typings'; import { Messages } from '@rocket.chat/models'; +import type BadWordsFilter from 'bad-words'; import { deleteMessage } from '../../../app/lib/server/functions/deleteMessage'; import { sendMessage } from '../../../app/lib/server/functions/sendMessage'; @@ -9,10 +10,30 @@ import { updateMessage } from '../../../app/lib/server/functions/updateMessage'; import { executeSendMessage } from '../../../app/lib/server/methods/sendMessage'; import { executeSetReaction } from '../../../app/reactions/server/setReaction'; import { settings } from '../../../app/settings/server'; +import { configureBadWords } from './hooks/badwords'; export class MessageService extends ServiceClassInternal implements IMessageService { protected name = 'message'; + private badWordsFilter?: BadWordsFilter; + + async created() { + await this.configureBadWords(); + } + + private async configureBadWords() { + settings.watchMultiple( + ['Message_AllowBadWordsFilter', 'Message_BadWordsFilterList', 'Message_BadWordsWhitelist'], + async ([enabled, badWordsList, whiteList]) => { + if (!enabled) { + this.badWordsFilter = undefined; + return; + } + this.badWordsFilter = await configureBadWords(badWordsList as string, whiteList as string); + }, + ); + } + async sendMessage({ fromId, rid, msg }: { fromId: string; rid: string; msg: string }): Promise { return executeSendMessage(fromId, { rid, msg }); } @@ -55,4 +76,61 @@ export class MessageService extends ServiceClassInternal implements IMessageServ return result.insertedId; } + + async beforeSave({ + message, + room: _room, + user: _user, + }: { + message: IMessage; + room: IRoom; + user: Pick; + }): Promise { + // TODO looks like this one was not being used (so I'll left it commented) + // await this.joinDiscussionOnMessage({ message, room, user }); + + // conditionals here should be fast, so they won't add up for each message + if (this.isBadWordsFilterEnabled()) { + message = await this.filterBadWords(message); + } + + return message; + } + + private isBadWordsFilterEnabled() { + return !!settings.get('Message_AllowBadWordsFilter'); + } + + private async filterBadWords(message: IMessage): Promise { + if (!message.msg || !this.badWordsFilter) { + return message; + } + + try { + message.msg = this.badWordsFilter.clean(message.msg); + } catch (error) { + // ignore + } + + return message; + } + + // joinDiscussionOnMessage + // private async joinDiscussionOnMessage({ message, room, user }: { message: IMessage; room: IRoom; user: IUser }) { + // // abort if room is not a discussion + // if (!room.prid) { + // return; + // } + + // // check if user already joined the discussion + // const sub = await Subscriptions.findOneByRoomIdAndUserId(room._id, message.u._id, { + // projection: { _id: 1 }, + // }); + + // if (sub) { + // return; + // } + + // await Room.join({ room, user }); + // } } diff --git a/apps/meteor/server/services/room/service.ts b/apps/meteor/server/services/room/service.ts index ac978da88c77..61b5bfeee504 100644 --- a/apps/meteor/server/services/room/service.ts +++ b/apps/meteor/server/services/room/service.ts @@ -1,6 +1,6 @@ -import { ServiceClassInternal, Authorization } from '@rocket.chat/core-services'; +import { ServiceClassInternal, Authorization, MeteorError } from '@rocket.chat/core-services'; import type { ICreateRoomParams, IRoomService } from '@rocket.chat/core-services'; -import type { AtLeast, IRoom, IUser } from '@rocket.chat/core-typings'; +import { type AtLeast, type IRoom, type IUser, isRoomWithJoinCode } from '@rocket.chat/core-typings'; import { Users } from '@rocket.chat/models'; import { saveRoomTopic } from '../../../app/channel-settings/server/functions/saveRoomTopic'; @@ -8,6 +8,7 @@ import { addUserToRoom } from '../../../app/lib/server/functions/addUserToRoom'; import { createRoom } from '../../../app/lib/server/functions/createRoom'; // TODO remove this import import { removeUserFromRoom } from '../../../app/lib/server/functions/removeUserFromRoom'; import { getValidRoomName } from '../../../app/utils/server/lib/getValidRoomName'; +import { RoomMemberActions } from '../../../definition/IRoomTypeConfig'; import { roomCoordinator } from '../../lib/rooms/roomCoordinator'; import { createDirectMessage } from '../../methods/createDirectMessage'; @@ -94,4 +95,29 @@ export class RoomService extends ServiceClassInternal implements IRoomService { async getRouteLink(room: AtLeast): Promise { return roomCoordinator.getRouteLink(room.t as string, { rid: room._id, name: room.name }); } + + /** + * Method called by users to join a room. + */ + async join({ room, user, joinCode }: { room: IRoom; user: Pick; joinCode?: string }) { + if (!(await roomCoordinator.getRoomDirectives(room.t)?.allowMemberAction(room, RoomMemberActions.JOIN, user._id))) { + throw new MeteorError('error-not-allowed', 'Not allowed', { method: 'joinRoom' }); + } + + if (!(await Authorization.canAccessRoom(room, user))) { + throw new MeteorError('error-not-allowed', 'Not allowed', { method: 'joinRoom' }); + } + + if ( + isRoomWithJoinCode(room) && + (!joinCode || joinCode !== room.joinCode) && + !(await Authorization.hasPermission(user._id, 'join-without-join-code')) + ) { + throw new MeteorError('error-code-invalid', 'Invalid Room Password', { + method: 'joinRoom', + }); + } + + return addUserToRoom(room._id, user); + } } diff --git a/packages/core-services/src/types/IMessageService.ts b/packages/core-services/src/types/IMessageService.ts index ea8f207df67d..b38d6a9559d6 100644 --- a/packages/core-services/src/types/IMessageService.ts +++ b/packages/core-services/src/types/IMessageService.ts @@ -9,6 +9,7 @@ export interface IMessageService { user: Pick, extraData?: Partial, ): Promise; + beforeSave(param: { message: IMessage; room: IRoom; user: IUser }): Promise; sendMessageWithValidation(user: IUser, message: Partial, room: Partial, upsert?: boolean): Promise; deleteMessage(user: IUser, message: IMessage): Promise; updateMessage(message: IMessage, user: IUser, originalMsg?: IMessage): Promise; diff --git a/packages/core-services/src/types/IRoomService.ts b/packages/core-services/src/types/IRoomService.ts index e69707e18a36..d9eee82029af 100644 --- a/packages/core-services/src/types/IRoomService.ts +++ b/packages/core-services/src/types/IRoomService.ts @@ -52,4 +52,5 @@ export interface IRoomService { sendMessage?: boolean, ): Promise; getRouteLink(room: AtLeast): Promise; + join(param: { room: IRoom; user: Pick; joinCode?: string }): Promise; } From 5e37d1d5a6eb9b4d56b3492f53e09c865b8f2612 Mon Sep 17 00:00:00 2001 From: Douglas Fabris Date: Mon, 25 Sep 2023 11:33:26 -0300 Subject: [PATCH 3/4] chore: `ResetPasswordForm` a11y improvements (#30476) --- .../web-ui-registration/src/RegisterForm.tsx | 4 +- .../src/ResetPasswordForm.tsx | 56 ++++++++++++------- .../src/hooks/useSendForgotPassword.ts | 7 ++- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index eb0aa7229f6d..4400ddca5b02 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -218,7 +218,9 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo {requiresPasswordConfirmation && ( - {t('registration.component.form.confirmPassword')} + + {t('registration.component.form.confirmPassword')} + { const { t } = useTranslation(); + const emailId = useUniqueId(); + const formLabelId = useUniqueId(); + const forgotPasswordFormRef = useRef(null); - const [sent, setSent] = useState(false); const { register, handleSubmit, formState: { errors, isSubmitting }, } = useForm<{ email: string; - }>(); + }>({ mode: 'onBlur' }); - const resetPassword = useSendForgotPassword(); + useEffect(() => { + if (forgotPasswordFormRef.current) { + forgotPasswordFormRef.current.focus(); + } + }, []); + + const { mutateAsync, isSuccess } = useSendForgotPassword(); return (
{ - resetPassword({ email: data.email }); - setSent(true); + mutateAsync({ email: data.email }); })} > - {t('registration.component.resetPassword')} + {t('registration.component.resetPassword')} - {t('registration.component.form.email')} - + + {t('registration.component.form.email')} + + - - {errors.email && {errors.email.message || t('registration.component.form.requiredField')}} + + {errors.email && ( + + {errors.email.message} + + )} - {sent && ( + {isSuccess && ( - + {t('registration.page.resetPassword.sent')} @@ -69,7 +88,6 @@ export const ResetPasswordForm = ({ setLoginRoute }: { setLoginRoute: DispatchLo {t('registration.page.resetPassword.sendInstructions')} - { setLoginRoute('login'); diff --git a/packages/web-ui-registration/src/hooks/useSendForgotPassword.ts b/packages/web-ui-registration/src/hooks/useSendForgotPassword.ts index 5e14c164b6cf..0dcef2d04bf4 100644 --- a/packages/web-ui-registration/src/hooks/useSendForgotPassword.ts +++ b/packages/web-ui-registration/src/hooks/useSendForgotPassword.ts @@ -1,7 +1,10 @@ import { useEndpoint } from '@rocket.chat/ui-contexts'; +import { useMutation } from '@tanstack/react-query'; export const useSendForgotPassword = () => { - const forgot = useEndpoint('POST', '/v1/users.forgotPassword'); + const sendForgotPassword = useEndpoint('POST', '/v1/users.forgotPassword'); - return forgot; + return useMutation({ + mutationFn: sendForgotPassword, + }); }; From 12e66c01a41714f6999d63b809bd9f585b140711 Mon Sep 17 00:00:00 2001 From: Douglas Fabris Date: Mon, 25 Sep 2023 12:28:50 -0300 Subject: [PATCH 4/4] chore: `ResetPasswordPage` a11y improvements (#30479) --- apps/meteor/tests/e2e/forgot-password.spec.ts | 11 +- apps/meteor/tests/e2e/page-objects/auth.ts | 5 + apps/meteor/tests/e2e/reset-password.spec.ts | 35 +++++ .../web-ui-registration/src/RegisterForm.tsx | 16 ++- .../src/ResetPassword/ResetPasswordPage.tsx | 129 +++++++++++------- 5 files changed, 140 insertions(+), 56 deletions(-) create mode 100644 apps/meteor/tests/e2e/reset-password.spec.ts diff --git a/apps/meteor/tests/e2e/forgot-password.spec.ts b/apps/meteor/tests/e2e/forgot-password.spec.ts index ee531d59cdfa..441944f5b227 100644 --- a/apps/meteor/tests/e2e/forgot-password.spec.ts +++ b/apps/meteor/tests/e2e/forgot-password.spec.ts @@ -2,7 +2,7 @@ import { Registration } from './page-objects'; import { test, expect } from './utils/test'; test.describe.parallel('Forgot Password', () => { - let poRegistration: Registration; + let poRegistration: Registration; test.beforeEach(async ({ page }) => { poRegistration = new Registration(page); @@ -11,7 +11,7 @@ test.describe.parallel('Forgot Password', () => { await poRegistration.btnForgotPassword.click(); }); - test('Email validation', async () => { + test('Send email to recover account', async () => { await test.step('expect trigger a validation error if no email is provided', async () => { await poRegistration.btnSendInstructions.click(); await expect(poRegistration.inputEmail).toBeInvalid(); @@ -31,11 +31,16 @@ test.describe.parallel('Forgot Password', () => { await expect(poRegistration.inputEmail).toBeInvalid(); }); - await test.step('expect to show a success toast if a valid email is provided', async () => { + await test.step('expect to show a success callout if a valid email is provided', async () => { await poRegistration.inputEmail.fill('mail@mail.com'); await poRegistration.btnSendInstructions.click(); await expect(poRegistration.forgotPasswordEmailCallout).toBeVisible(); }); }); + + test('should not have any accessibility violations', async ({ makeAxeBuilder }) => { + const results = await makeAxeBuilder().analyze(); + expect(results.violations).toEqual([]); + }) }); diff --git a/apps/meteor/tests/e2e/page-objects/auth.ts b/apps/meteor/tests/e2e/page-objects/auth.ts index b03e45ee22b7..9b47d2e44adc 100644 --- a/apps/meteor/tests/e2e/page-objects/auth.ts +++ b/apps/meteor/tests/e2e/page-objects/auth.ts @@ -11,6 +11,11 @@ export class Registration { return this.page.locator('role=button[name="Send instructions"]'); } + get btnReset(): Locator { + return this.page.locator('role=button[name="Reset"]'); + } + + get btnLogin(): Locator { return this.page.locator('role=button[name="Login"]'); } diff --git a/apps/meteor/tests/e2e/reset-password.spec.ts b/apps/meteor/tests/e2e/reset-password.spec.ts new file mode 100644 index 000000000000..fc5e0b703784 --- /dev/null +++ b/apps/meteor/tests/e2e/reset-password.spec.ts @@ -0,0 +1,35 @@ +import { Registration } from './page-objects'; +import { setSettingValueById } from './utils/setSettingValueById'; +import { test, expect } from './utils/test'; + +test.describe.parallel('Reset Password', () => { + let poRegistration: Registration; + + test.beforeEach(async ({ api, page }) => { + poRegistration = new Registration(page); + await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', true); + + await page.goto('/reset-password/someToken'); + }); + + test.afterAll(async ({ api }) => { + await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', true); + }) + + test('should confirm password be invalid', async () => { + await poRegistration.inputPassword.fill('123456'); + await poRegistration.inputPasswordConfirm.fill('123455'); + await poRegistration.btnReset.click(); + await expect(poRegistration.inputPasswordConfirm).toBeInvalid(); + }); + + test('should confirm password not be visible', async ({ api }) => { + await setSettingValueById(api, 'Accounts_RequirePasswordConfirmation', false); + await expect(poRegistration.inputPasswordConfirm).not.toBeVisible(); + }) + + test('should not have any accessibility violations', async ({ makeAxeBuilder }) => { + const results = await makeAxeBuilder().analyze(); + expect(results.violations).toEqual([]); + }) +}); diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index 4400ddca5b02..6202916c87f2 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -129,9 +129,10 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo {errors.name && ( - {t('registration.component.form.requiredField')} + {errors.name.message} )} @@ -159,6 +160,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo })} placeholder={usernameOrEmailPlaceholder || t('registration.component.form.emailPlaceholder')} error={errors?.email?.message} + aria-required='true' aria-invalid={errors.email ? 'true' : 'false'} aria-describedby={`${emailId}-error`} id={emailId} @@ -180,6 +182,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo required: t('registration.component.form.requiredField'), })} error={errors?.username?.message} + aria-required='true' aria-invalid={errors.username ? 'true' : 'false'} aria-describedby={`${usernameId}-error`} id={usernameId} @@ -202,7 +205,8 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo required: t('registration.component.form.requiredField'), validate: () => (!passwordIsValid ? t('Password_must_meet_the_complexity_requirements') : true), })} - error={errors.password && (errors.password?.message || t('registration.component.form.requiredField'))} + error={errors.password?.message} + aria-required='true' aria-invalid={errors.password ? 'true' : undefined} id={passwordId} placeholder={passwordPlaceholder || t('Create_a_password')} @@ -229,6 +233,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo validate: (val: string) => (watch('password') === val ? true : t('registration.component.form.invalidConfirmPass')), })} error={errors.passwordConfirmation?.message} + aria-required='true' aria-invalid={errors.passwordConfirmation ? 'true' : 'false'} id={passwordConfirmationId} aria-describedby={`${passwordConfirmationId}-error`} @@ -254,6 +259,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo required: t('registration.component.form.requiredField'), })} error={errors?.reason?.message} + aria-required='true' aria-invalid={errors.reason ? 'true' : 'false'} aria-describedby={`${reasonId}-error`} id={reasonId} @@ -261,7 +267,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo {errors.reason && ( - {t('registration.component.form.requiredField')} + {errors.reason.message} )} diff --git a/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx b/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx index d3a3e6fa7413..19b0a13983bb 100644 --- a/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx +++ b/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx @@ -1,10 +1,11 @@ -import { Button, Field, Modal, PasswordInput } from '@rocket.chat/fuselage'; +import { Button, FieldGroup, Field, FieldLabel, ButtonGroup, PasswordInput, FieldRow, FieldError } from '@rocket.chat/fuselage'; import { useUniqueId } from '@rocket.chat/fuselage-hooks'; import { Form } from '@rocket.chat/layout'; import { PasswordVerifier, useValidatePassword } from '@rocket.chat/ui-client'; import type { TranslationKey } from '@rocket.chat/ui-contexts'; import { useSetting, useRouter, useRouteParameter, useUser, useMethod, useTranslation, useLoginWithToken } from '@rocket.chat/ui-contexts'; import type { ReactElement } from 'react'; +import { useEffect, useRef } from 'react'; import { useForm } from 'react-hook-form'; import HorizontalTemplate from '../template/HorizontalTemplate'; @@ -21,10 +22,15 @@ const ResetPasswordPage = (): ReactElement => { const resetPassword = useMethod('resetPassword'); const token = useRouteParameter('token'); + const resetPasswordFormRef = useRef(null); const passwordId = useUniqueId(); + const passwordConfirmationId = useUniqueId(); const passwordVerifierId = useUniqueId(); + const formLabelId = useUniqueId(); const requiresPasswordConfirmation = useSetting('Accounts_RequirePasswordConfirmation'); + const passwordPlaceholder = String(useSetting('Accounts_PasswordPlaceholder')); + const passwordConfirmationPlaceholder = String(useSetting('Accounts_ConfirmPasswordPlaceholder')); const router = useRouter(); @@ -36,7 +42,7 @@ const ResetPasswordPage = (): ReactElement => { register, handleSubmit, setError, - formState: { errors, isValid }, + formState: { errors, isSubmitting }, watch, } = useForm<{ password: string; @@ -48,76 +54,103 @@ const ResetPasswordPage = (): ReactElement => { const password = watch('password'); const passwordIsValid = useValidatePassword(password); - const submit = handleSubmit(async (data) => { + useEffect(() => { + if (resetPasswordFormRef.current) { + resetPasswordFormRef.current.focus(); + } + }, []); + + const handleResetPassword = async ({ password }: { password: string }) => { try { if (token) { - const result = await resetPassword(token, data.password); + const result = await resetPassword(token, password); await loginWithToken(result.token); router.navigate('/home'); } else { - await setUserPassword(data.password); + await setUserPassword(password); } } catch ({ error, reason }: any) { const _error = reason ?? error; setError('password', { message: String(_error) }); } - }); + }; return ( - + - {t('Reset_password')} + {t('Reset_password')} + {t(changePasswordReason)} - - {t(changePasswordReason)} - - (!passwordIsValid ? t('Password_must_meet_the_complexity_requirements') : true), - })} - error={errors.password?.message} - aria-invalid={errors.password ? 'true' : 'false'} - id={passwordId} - placeholder={t('Create_a_password')} - name='password' - autoComplete='off' - aria-describedby={passwordVerifierId} - /> - - {errors?.password && ( - - {errors.password.message} - - )} - - {requiresPasswordConfirmation && ( - + + + + {t('registration.component.form.password')} + + password === val, + {...register('password', { + required: t('registration.component.form.requiredField'), + validate: () => (!passwordIsValid ? t('Password_must_meet_the_complexity_requirements') : true), })} - error={errors.passwordConfirmation?.type === 'validate' ? t('registration.component.form.invalidConfirmPass') : undefined} - aria-invalid={errors.passwordConfirmation ? 'true' : false} - id='passwordConfirmation' - placeholder={t('Confirm_password')} - disabled={!passwordIsValid} + error={errors?.password?.message} + aria-invalid={errors.password ? 'true' : 'false'} + aria-required='true' + id={passwordId} + placeholder={passwordPlaceholder || t('Create_a_password')} + aria-describedby={`${passwordVerifierId} ${passwordId}-error`} /> - + + {errors?.password && ( + + {errors.password.message} + + )} + + + {requiresPasswordConfirmation && ( + + + {t('registration.component.form.confirmPassword')} + + + (password === val ? true : t('registration.component.form.invalidConfirmPass')), + })} + error={errors?.passwordConfirmation?.message} + aria-required='true' + aria-invalid={errors.passwordConfirmation ? 'true' : 'false'} + aria-describedby={`${passwordConfirmationId}-error`} + id={passwordConfirmationId} + placeholder={passwordConfirmationPlaceholder || t('Confirm_password')} + disabled={!passwordIsValid} + /> + + {errors.passwordConfirmation && ( + + {errors.passwordConfirmation?.message} + + )} + )} - {errors && {errors.password?.message}} - + - - - +