[FIX] Discussion visibility when inside a private team. (#27601)

Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>

apps/meteor/app/authorization/server/functions/canSendMessage.ts

@@ -16,7 +16,7 @@ const subscriptionOptions = {
 async function validateRoomMessagePermissionsAsync(
 	room: IRoom | null,
 	{ uid, username, type }: { uid: IUser['_id']; username: IUser['username']; type: IUser['type'] },
-	extraData: Record<string, any>,
+	extraData?: Record<string, any>,
 ): Promise<void> {
 	if (!room) {
 		throw new Error('error-invalid-room');
@@ -48,7 +48,7 @@ async function validateRoomMessagePermissionsAsync(
 export async function canSendMessageAsync(
 	rid: IRoom['_id'],
 	{ uid, username, type }: { uid: IUser['_id']; username: IUser['username']; type: IUser['type'] },
-	extraData: Record<string, any>,
+	extraData?: Record<string, any>,
 ): Promise<IRoom> {
 	const room = await Rooms.findOneById(rid);
 	if (!room) {
@@ -62,14 +62,14 @@ export async function canSendMessageAsync(
 export function canSendMessage(
 	rid: IRoom['_id'],
 	{ uid, username, type }: { uid: IUser['_id']; username: IUser['username']; type: IUser['type'] },
-	extraData: Record<string, any>,
+	extraData?: Record<string, any>,
 ): IRoom {
 	return Promise.await(canSendMessageAsync(rid, { uid, username, type }, extraData));
 }
 export function validateRoomMessagePermissions(
 	room: IRoom,
 	{ uid, username, type }: { uid: IUser['_id']; username: IUser['username']; type: IUser['type'] },
-	extraData: Record<string, any>,
+	extraData?: Record<string, any>,
 ): void {
 	return Promise.await(validateRoomMessagePermissionsAsync(room, { uid, username, type }, extraData));
 }

apps/meteor/app/autotranslate/server/autotranslate.ts

@@ -67,7 +67,7 @@ export class TranslationProviderRegistry {
 		return TranslationProviderRegistry.enabled ? TranslationProviderRegistry.getActiveProvider()?.getSupportedLanguages(target) : undefined;
 	}
 
-	static translateMessage(message: IMessage, room: IRoom, targetLanguage: string): IMessage | undefined {
+	static translateMessage(message: IMessage, room: IRoom, targetLanguage?: string): IMessage | undefined {
 		return TranslationProviderRegistry.enabled
 			? TranslationProviderRegistry.getActiveProvider()?.translateMessage(message, room, targetLanguage)
 			: undefined;
@@ -281,7 +281,7 @@ export abstract class AutoTranslate {
 	 * @param {object} targetLanguage
 	 * @returns {object} unmodified message object.
 	 */
-	translateMessage(message: IMessage, room: IRoom, targetLanguage: string): IMessage {
+	translateMessage(message: IMessage, room: IRoom, targetLanguage?: string): IMessage {
 		let targetLanguages: string[];
 		if (targetLanguage) {
 			targetLanguages = [targetLanguage];

apps/meteor/app/discussion/server/methods/createDiscussion.js → apps/meteor/app/discussion/server/methods/createDiscussion.ts

@@ -1,7 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { Random } from 'meteor/random';
-import { Match } from 'meteor/check';
 import { TAPi18n } from 'meteor/rocketchat:tap-i18n';
+import type { IMessage, IRoom, IUser, MessageAttachmentDefault } from '@rocket.chat/core-typings';
 
 import { hasAtLeastOnePermission, canSendMessage } from '../../../authorization/server';
 import { Messages, Rooms } from '../../../models/server';
@@ -10,77 +10,93 @@ import { settings } from '../../../settings/server';
 import { callbacks } from '../../../../lib/callbacks';
 import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator';
 
-const getParentRoom = (rid) => {
+const getParentRoom = (rid: IRoom['_id']) => {
 	const room = Rooms.findOne(rid);
 	return room && (room.prid ? Rooms.findOne(room.prid, { fields: { _id: 1 } }) : room);
 };
 
-const createDiscussionMessage = (rid, user, drid, msg, message_embedded) => {
+const createDiscussionMessage = (
+	rid: IRoom['_id'],
+	user: IUser,
+	drid: IRoom['_id'],
+	msg: IMessage['msg'],
+	messageEmbedded?: MessageAttachmentDefault,
+): IMessage => {
 	const welcomeMessage = {
 		msg,
 		rid,
 		drid,
-		attachments: [message_embedded].filter((e) => e),
+		attachments: [messageEmbedded].filter((e) => e),
 	};
-	return Messages.createWithTypeRoomIdMessageAndUser('discussion-created', rid, '', user, welcomeMessage);
+	return Messages.createWithTypeRoomIdMessageAndUser('discussion-created', rid, '', user, welcomeMessage) as IMessage;
 };
 
-const mentionMessage = (rid, { _id, username, name }, message_embedded) => {
+const mentionMessage = (
+	rid: IRoom['_id'],
+	{ _id, username, name }: Pick<IUser, '_id' | 'name' | 'username'>,
+	messageEmbedded?: MessageAttachmentDefault,
+) => {
 	const welcomeMessage = {
 		rid,
 		u: { _id, username, name },
 		ts: new Date(),
 		_updatedAt: new Date(),
-		attachments: [message_embedded].filter((e) => e),
+		attachments: [messageEmbedded].filter((e) => e),
 	};
 
 	return Messages.insert(welcomeMessage);
 };
 
-const create = ({ prid, pmid, t_name, reply, users, user, encrypted }) => {
+type CreateDiscussionProperties = {
+	prid: IRoom['_id'];
+	pmid?: IMessage['_id'];
+	t_name: string;
+	reply?: string;
+	users: Array<Exclude<IUser['username'], undefined>>;
+	user: IUser;
+	encrypted?: boolean;
+};
+
+const create = ({ prid, pmid, t_name: discussionName, reply, users, user, encrypted }: CreateDiscussionProperties) => {
 	// if you set both, prid and pmid, and the rooms dont match... should throw an error)
-	let message = false;
+	let message: undefined | IMessage;
 	if (pmid) {
-		message = Messages.findOne({ _id: pmid });
+		message = Messages.findOne({ _id: pmid }) as IMessage | undefined;
 		if (!message) {
 			throw new Meteor.Error('error-invalid-message', 'Invalid message', {
 				method: 'DiscussionCreation',
 			});
 		}
 		if (prid) {
 			if (prid !== getParentRoom(message.rid)._id) {
-				throw new Meteor.Error('error-invalid-arguments', { method: 'DiscussionCreation' });
+				throw new Meteor.Error('error-invalid-arguments', 'Root message room ID does not match parent room ID ', {
+					method: 'DiscussionCreation',
+				});
 			}
 		} else {
 			prid = message.rid;
 		}
 	}
 
 	if (!prid) {
-		throw new Meteor.Error('error-invalid-arguments', { method: 'DiscussionCreation' });
+		throw new Meteor.Error('error-invalid-arguments', 'Missing parent room ID', { method: 'DiscussionCreation' });
 	}
 
-	let p_room;
+	let parentRoom;
 	try {
-		p_room = canSendMessage(prid, { uid: user._id, username: user.username, type: user.type });
+		parentRoom = canSendMessage(prid, { uid: user._id, username: user.username, type: user.type });
 	} catch (error) {
-		throw new Meteor.Error(error.message);
+		throw new Meteor.Error((error as Error).message);
 	}
 
-	if (p_room.prid) {
+	if (parentRoom.prid) {
 		throw new Meteor.Error('error-nested-discussion', 'Cannot create nested discussions', {
 			method: 'DiscussionCreation',
 		});
 	}
 
-	if (!Match.Maybe(encrypted, Boolean)) {
-		throw new Meteor.Error('error-invalid-arguments', 'Invalid encryption state', {
-			method: 'DiscussionCreation',
-		});
-	}
-
 	if (typeof encrypted !== 'boolean') {
-		encrypted = p_room.encrypted;
+		encrypted = Boolean(parentRoom.encrypted);
 	}
 
 	if (encrypted && reply) {
@@ -111,42 +127,49 @@ const create = ({ prid, pmid, t_name, reply, users, user, encrypted }) => {
 	// auto invite the replied message owner
 	const invitedUsers = message ? [message.u.username, ...users] : users;
 
-	const type = roomCoordinator.getRoomDirectives(p_room.t)?.getDiscussionType();
-	const description = p_room.encrypted ? '' : message.msg;
-	const topic = p_room.name;
+	const type = roomCoordinator.getRoomDirectives(parentRoom.t)?.getDiscussionType(parentRoom);
+	const description = parentRoom.encrypted ? '' : message?.msg;
+	const topic = parentRoom.name;
+
+	if (!type) {
+		throw new Meteor.Error('error-invalid-type', 'Cannot define discussion room type', {
+			method: 'DiscussionCreation',
+		});
+	}
 
 	const discussion = createRoom(
 		type,
 		name,
-		user.username,
-		[...new Set(invitedUsers)],
+		user.username as string,
+		[...new Set(invitedUsers)].filter(Boolean),
 		false,
 		{
-			fname: t_name,
+			fname: discussionName,
 			description, // TODO discussions remove
 			topic, // TODO discussions remove
 			prid,
 			encrypted,
 		},
 		{
 			// overrides name validation to allow anything, because discussion's name is randomly generated
-			nameValidationRegex: /.*/,
+			nameValidationRegex: '.*',
+			creator: user._id,
 		},
 	);
 
 	let discussionMsg;
-	if (pmid) {
-		if (p_room.encrypted) {
+	if (message) {
+		if (parentRoom.encrypted) {
 			message.msg = TAPi18n.__('Encrypted_message');
 		}
-		mentionMessage(discussion._id, user, attachMessage(message, p_room));
+		mentionMessage(discussion._id, user, attachMessage(message, parentRoom));
 
-		discussionMsg = createDiscussionMessage(message.rid, user, discussion._id, t_name, attachMessage(message, p_room));
+		discussionMsg = createDiscussionMessage(message.rid, user, discussion._id, discussionName, attachMessage(message, parentRoom));
 	} else {
-		discussionMsg = createDiscussionMessage(prid, user, discussion._id, t_name);
+		discussionMsg = createDiscussionMessage(prid, user, discussion._id, discussionName);
 	}
 
-	callbacks.runAsync('afterSaveMessage', discussionMsg, p_room);
+	callbacks.runAsync('afterSaveMessage', discussionMsg, parentRoom);
 
 	if (reply) {
 		sendMessage(user, { msg: reply }, discussion);
@@ -165,7 +188,7 @@ Meteor.methods({
 	 * @param {string[]} users - users to be added
 	 * @param {boolean} encrypted - if the discussion's e2e encryption should be enabled.
 	 */
-	createDiscussion({ prid, pmid, t_name, reply, users, encrypted }) {
+	createDiscussion({ prid, pmid, t_name: discussionName, reply, users, encrypted }: CreateDiscussionProperties) {
 		if (!settings.get('Discussion_enabled')) {
 			throw new Meteor.Error('error-action-not-allowed', 'You are not allowed to create a discussion', { method: 'createDiscussion' });
 		}
@@ -181,6 +204,6 @@ Meteor.methods({
 			throw new Meteor.Error('error-action-not-allowed', 'You are not allowed to create a discussion', { method: 'createDiscussion' });
 		}
 
-		return create({ uid, prid, pmid, t_name, reply, users, user: Meteor.user(), encrypted });
+		return create({ prid, pmid, t_name: discussionName, reply, users, user: Meteor.user() as IUser, encrypted });
 	},
 });

apps/meteor/definition/IRoomTypeConfig.ts

@@ -89,7 +89,7 @@ export interface IRoomTypeServerDirectives {
 	isGroupChat: (room: IRoom) => boolean;
 	canBeDeleted: (hasPermission: (permissionId: string, rid?: string) => boolean, room: IRoom) => boolean;
 	preventRenaming: () => boolean;
-	getDiscussionType: () => RoomType;
+	getDiscussionType: (room?: AtLeast<IRoom, 'teamId'>) => RoomType;
 	canAccessUploadedFile: (params: { rc_uid: string; rc_rid: string; rc_token: string }) => boolean;
 	getNotificationDetails: (
 		room: IRoom,

apps/meteor/lib/callbacks.ts

@@ -46,7 +46,7 @@ type EventLikeCallbackSignatures = {
 	'beforeReadMessages': (rid: IRoom['_id'], uid: IUser['_id']) => void;
 	'afterDeleteUser': (user: IUser) => void;
 	'afterFileUpload': (params: { user: IUser; room: IRoom; message: IMessage }) => void;
-	'afterSaveMessage': (message: IMessage, room: IRoom, uid: string) => void;
+	'afterSaveMessage': (message: IMessage, room: IRoom, uid?: string) => void;
 	'livechat.removeAgentDepartment': (params: { departmentId: ILivechatDepartmentRecord['_id']; agentsId: ILivechatAgent['_id'][] }) => void;
 	'livechat.saveAgentDepartment': (params: { departmentId: ILivechatDepartmentRecord['_id']; agentsId: ILivechatAgent['_id'][] }) => void;
 	'livechat.closeRoom': (room: IRoom) => void;

apps/meteor/server/lib/rooms/roomTypes/public.ts

@@ -1,5 +1,6 @@
 import type { AtLeast, IRoom } from '@rocket.chat/core-typings';
-import { isRoomFederated } from '@rocket.chat/core-typings';
+import { isRoomFederated, TEAM_TYPE } from '@rocket.chat/core-typings';
+import { Team } from '@rocket.chat/core-services';
 
 import { Federation } from '../../../../app/federation-v2/server/Federation';
 import { settings } from '../../../../app/settings/server';
@@ -57,7 +58,13 @@ roomCoordinator.add(PublicRoomType, {
 		return true;
 	},
 
-	getDiscussionType() {
+	getDiscussionType(room) {
+		if (room?.teamId) {
+			const team = Promise.await(Team.getOneById(room.teamId, { projection: { type: 1 } }));
+			if (team?.type === TEAM_TYPE.PRIVATE) {
+				return 'p';
+			}
+		}
 		return 'c';
 	},
 

apps/meteor/tests/end-to-end/api/09-rooms.js

@@ -957,6 +957,64 @@ describe('[Rooms]', function () {
 				})
 				.end(done);
 		});
+
+		describe('it should create a *private* discussion if the parent channel is public and inside a private team', async () => {
+			let privateTeam;
+
+			it('should create a team', (done) => {
+				request
+					.post(api('teams.create'))
+					.set(credentials)
+					.send({
+						name: `test-team-${Date.now()}`,
+						type: 1,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.have.property('team');
+						expect(res.body).to.have.nested.property('team._id');
+						privateTeam = res.body.team;
+					})
+					.end(done);
+			});
+
+			it('should add the public channel to the team', (done) => {
+				request
+					.post(api('teams.addRooms'))
+					.set(credentials)
+					.send({
+						rooms: [testChannel._id],
+						teamId: privateTeam._id,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success');
+					})
+					.end(done);
+			});
+
+			it('should create a private discussion inside the public channel', (done) => {
+				request
+					.post(api('rooms.createDiscussion'))
+					.set(credentials)
+					.send({
+						prid: testChannel._id,
+						t_name: `discussion-create-from-tests-${testChannel.name}-team`,
+					})
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.have.property('discussion').and.to.be.an('object');
+						expect(res.body.discussion).to.have.property('prid').and.to.be.equal(testChannel._id);
+						expect(res.body.discussion).to.have.property('fname').and.to.be.equal(`discussion-create-from-tests-${testChannel.name}-team`);
+						expect(res.body.discussion).to.have.property('t').and.to.be.equal('p');
+					})
+					.end(done);
+			});
+		});
 	});
 
 	describe('/rooms.getDiscussions', () => {