fix: Edit Discussion name validation allow special characters (Rock…

…etChat#31983) Co-authored-by: dougfabris <[email protected]> Co-authored-by: Diego Sampaio <[email protected]>
abhipatel0211 · Mar 15, 2024 · f438a14 · f438a14
1 parent 0f98995
commit f438a14
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 35 deletions.
diff --git a/.changeset/mean-rabbits-hope.md b/.changeset/mean-rabbits-hope.md
@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fix error on changing a discussion name
diff --git a/apps/meteor/app/channel-settings/server/functions/saveRoomName.ts b/apps/meteor/app/channel-settings/server/functions/saveRoomName.ts
@@ -55,9 +55,10 @@ export async function saveRoomName(
 		return;
 	}
 
-	const slugifiedRoomName = await getValidRoomName(displayName, rid);
 	const isDiscussion = Boolean(room?.prid);
 
+	const slugifiedRoomName = isDiscussion ? displayName : await getValidRoomName(displayName, rid);
+
 	let update;
 
 	if (isDiscussion || isRoomFederated(room)) {
@@ -70,7 +71,9 @@ export async function saveRoomName(
 		return;
 	}
 
-	room.name && (await Integrations.updateRoomName(room.name, slugifiedRoomName));
+	if (room.name && !isDiscussion) {
+		await Integrations.updateRoomName(room.name, slugifiedRoomName);
+	}
 	if (sendMessage) {
 		await Message.saveSystemMessage('r', rid, displayName, user);
 	}

diff --git a/apps/meteor/app/discussion/server/methods/createDiscussion.ts b/apps/meteor/app/discussion/server/methods/createDiscussion.ts
@@ -168,8 +168,6 @@ const create = async ({
 			encrypted,
 		},
 		{
-			// overrides name validation to allow anything, because discussion's name is randomly generated
-			nameValidationRegex: '.*',
 			creator: user._id,
 		},
 	);

diff --git a/apps/meteor/app/lib/server/functions/createDirectRoom.ts b/apps/meteor/app/lib/server/functions/createDirectRoom.ts
@@ -43,7 +43,6 @@ export async function createDirectRoom(
 	members: IUser[] | string[],
 	roomExtraData = {},
 	options: {
-		nameValidationRegex?: string;
 		creator?: string;
 		subscriptionExtra?: ISubscriptionExtraData;
 	},

diff --git a/apps/meteor/app/lib/server/functions/createRoom.ts b/apps/meteor/app/lib/server/functions/createRoom.ts
@@ -164,15 +164,16 @@ export const createRoom = async <T extends RoomType>(
 		delete extraData.reactWhenReadOnly;
 	}
 
+	// this might not be the best way to check if the room is a discussion, we may need a specific field for that
+	const isDiscussion = 'prid' in extraData && extraData.prid !== '';
+
 	const now = new Date();
 
 	const roomProps: Omit<IRoom, '_id' | '_updatedAt'> = {
 		fname: name,
 		_updatedAt: now,
 		...extraData,
-		name: await getValidRoomName(name.trim(), undefined, {
-			...(options?.nameValidationRegex && { nameValidationRegex: options.nameValidationRegex }),
-		}),
+		name: isDiscussion ? name : await getValidRoomName(name.trim(), undefined),
 		t: type,
 		msgs: 0,
 		usersCount: 0,

diff --git a/apps/meteor/app/utils/server/lib/getValidRoomName.ts b/apps/meteor/app/utils/server/lib/getValidRoomName.ts
@@ -6,11 +6,7 @@ import { Meteor } from 'meteor/meteor';
 import { validateName } from '../../../lib/server/functions/validateName';
 import { settings } from '../../../settings/server';
 
-export const getValidRoomName = async (
-	displayName: string,
-	rid = '',
-	options: { allowDuplicates?: boolean; nameValidationRegex?: string } = {},
-) => {
+export const getValidRoomName = async (displayName: string, rid = '', options: { allowDuplicates?: boolean } = {}) => {
 	let slugifiedName = displayName;
 
 	if (settings.get('UI_Allow_room_names_with_special_chars')) {
@@ -36,14 +32,10 @@ export const getValidRoomName = async (
 
 	let nameValidation;
 
-	if (options.nameValidationRegex) {
-		nameValidation = new RegExp(options.nameValidationRegex);
-	} else {
-		try {
-			nameValidation = new RegExp(`^${settings.get('UTF8_Channel_Names_Validation')}$`);
-		} catch (error) {
-			nameValidation = new RegExp('^[0-9a-zA-Z-_.]+$');
-		}
+	try {
+		nameValidation = new RegExp(`^${settings.get('UTF8_Channel_Names_Validation')}$`);
+	} catch (error) {
+		nameValidation = new RegExp('^[0-9a-zA-Z-_.]+$');
 	}
 
 	if (!nameValidation.test(slugifiedName) || !validateName(slugifiedName)) {

diff --git a/apps/meteor/server/services/room/service.ts b/apps/meteor/server/services/room/service.ts
@@ -70,11 +70,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService {
 		return removeUserFromRoom(roomId, user, options);
 	}
 
-	async getValidRoomName(
-		displayName: string,
-		roomId = '',
-		options: { allowDuplicates?: boolean; nameValidationRegex?: string } = {},
-	): Promise<string> {
+	async getValidRoomName(displayName: string, roomId = '', options: { allowDuplicates?: boolean } = {}): Promise<string> {
 		return getValidRoomName(displayName, roomId, options);
 	}
 

diff --git a/apps/meteor/tests/end-to-end/api/09-rooms.js b/apps/meteor/tests/end-to-end/api/09-rooms.js
@@ -9,7 +9,7 @@ import { getCredentials, api, request, credentials } from '../../data/api-data.j
 import { sendSimpleMessage, deleteMessage } from '../../data/chat.helper';
 import { imgURL, svgLogoFileName, svgLogoURL } from '../../data/interactions';
 import { getSettingValueById, updateEEPermission, updatePermission, updateSetting } from '../../data/permissions.helper';
-import { closeRoom, createRoom } from '../../data/rooms.helper';
+import { closeRoom, createRoom, deleteRoom } from '../../data/rooms.helper';
 import { password } from '../../data/user';
 import { createUser, deleteUser, login } from '../../data/users.helper';
 import { IS_EE } from '../../e2e/config/constants';
@@ -1705,12 +1705,25 @@ describe('[Rooms]', function () {
 	describe('/rooms.saveRoomSettings', () => {
 		let testChannel;
 		const randomString = `randomString${Date.now()}`;
+		let discussion;
 
-		before('create a channel', async () => {
+		before(async () => {
 			const result = await createRoom({ type: 'c', name: `channel.test.${Date.now()}-${Math.random()}` });
 			testChannel = result.body.channel;
+
+			const resDiscussion = await request
+				.post(api('rooms.createDiscussion'))
+				.set(credentials)
+				.send({
+					prid: testChannel._id,
+					t_name: `discussion-create-from-tests-${testChannel.name}`,
+				});
+
+			discussion = resDiscussion.body.discussion;
 		});
 
+		after(() => Promise.all([deleteRoom({ type: 'p', roomId: discussion._id }), deleteRoom({ type: 'p', roomId: testChannel._id })]));
+
 		it('should update the room settings', (done) => {
 			const imageDataUri = `data:image/png;base64,${fs.readFileSync(path.join(process.cwd(), imgURL)).toString('base64')}`;
 
@@ -1765,5 +1778,34 @@ describe('[Rooms]', function () {
 				})
 				.end(done);
 		});
+
+		it('should be able to update the discussion name with spaces', async () => {
+			const newDiscussionName = `${randomString} with spaces`;
+
+			await request
+				.post(api('rooms.saveRoomSettings'))
+				.set(credentials)
+				.send({
+					rid: discussion._id,
+					roomName: newDiscussionName,
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200);
+
+			await request
+				.get(api('rooms.info'))
+				.set(credentials)
+				.query({
+					roomId: discussion._id,
+				})
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('room').and.to.be.an('object');
+
+					expect(res.body.room).to.have.property('_id', discussion._id);
+					expect(res.body.room).to.have.property('fname', newDiscussionName);
+				});
+		});
 	});
 });
diff --git a/packages/core-services/src/types/IRoomService.ts b/packages/core-services/src/types/IRoomService.ts
@@ -8,7 +8,6 @@ export interface ISubscriptionExtraData {
 }
 
 interface ICreateRoomOptions extends Partial<Record<string, string | ISubscriptionExtraData>> {
-	nameValidationRegex?: string;
 	creator: string;
 	subscriptionExtra?: ISubscriptionExtraData;
 }
@@ -38,11 +37,7 @@ export interface IRoomService {
 		silenced?: boolean,
 	): Promise<boolean | undefined>;
 	removeUserFromRoom(roomId: string, user: IUser, options?: { byUser: Pick<IUser, '_id' | 'username'> }): Promise<void>;
-	getValidRoomName(
-		displayName: string,
-		roomId?: string,
-		options?: { allowDuplicates?: boolean; nameValidationRegex?: string },
-	): Promise<string>;
+	getValidRoomName(displayName: string, roomId?: string, options?: { allowDuplicates?: boolean }): Promise<string>;
 	saveRoomTopic(
 		roomId: string,
 		roomTopic: string | undefined,

diff --git a/packages/rest-typings/src/v1/teams/index.ts b/packages/rest-typings/src/v1/teams/index.ts
@@ -72,7 +72,6 @@ export type TeamsEndpoints = {
 					teamMain?: boolean;
 				} & { [key: string]: string | boolean };
 				options?: {
-					nameValidationRegex?: string;
 					creator: string;
 					subscriptionExtra?: {
 						open: boolean;