fix: removing files from e2ee rooms do not remove their messages #38379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open

pierre-lehnen-rc wants to merge 5 commits into develop from fix/e2ee-uploads

+78 −22

.changeset/flat-tables-applaud.md

-Original file line number
+Diff line change
@@ -0,0 +1,6 @@
+    ---
+    '@rocket.chat/core-typings': patch
+    '@rocket.chat/meteor': patch
+    ---
+    Fixes association of encrypted messages and encrypted files, so that if one of them is removed, the other gets removed as well.

apps/meteor/app/api/server/v1/rooms.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -271,7 +271,7 @@ API.v1.addRoute( @@
     			delete this.bodyParams.description;
     			await applyAirGappedRestrictionsValidation(() =>
-    				sendFileMessage(this.userId, { roomId: this.urlParams.rid, file, msgData: this.bodyParams }, { parseAttachmentsForE2EE: false }),
+    				sendFileMessage(this.userId, { roomId: this.urlParams.rid, file, msgData: this.bodyParams }),
     			);
     			await Uploads.confirmTemporaryFile(this.urlParams.fileId, this.userId);
@@ Expand Down @@

apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -166,13 +166,6 @@ export const sendFileMessage = async ( @@
     		file: Partial<IUpload>;
     		msgData?: Record<string, any>;
     	},
-    	{
-    		parseAttachmentsForE2EE,
-    	}: {
-    		parseAttachmentsForE2EE: boolean;
-    	} = {
-    		parseAttachmentsForE2EE: true,
-    	},
     ): Promise<boolean> => {
     	const user = await Users.findOneById(userId, { projection: { services: 0 } });
@@ Expand Down Expand Up / @@ -220,12 +213,10 @@ export const sendFileMessage = async ( @@
     		groupable: msgData?.groupable ?? false,
     	};
-    	if (parseAttachmentsForE2EE || msgData?.t !== 'e2e') {
-    		const { files, attachments } = await parseFileIntoMessageAttachments(file, roomId, user);
-    		data.file = files[0];
-    		data.files = files;
-    		data.attachments = attachments;
-    	}
+    	const { files, attachments } = await parseFileIntoMessageAttachments(file, roomId, user);
+    	data.file = files[0];
+    	data.files = files;
+    	data.attachments = attachments;
     	const msg = await executeSendMessage(userId, data);
@@ Expand Down @@

apps/meteor/app/lib/server/functions/cleanRoomHistory.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -48,7 +48,8 @@ export async function cleanRoomHistory({ @@
     	});
     	const targetMessageIdsForAttachmentRemoval = new Set<string>();
-    	const pruneMessageAttachment = { color: NOTIFICATION_ATTACHMENT_COLOR, text };
+    	// Since we remove every file from the messages, we don't need to specify which fileId has been removed.
+    	const pruneMessageAttachment = { type: 'removed-file', color: NOTIFICATION_ATTACHMENT_COLOR, text };
     	async function performFileAttachmentCleanupBatch() {
     		if (targetMessageIdsForAttachmentRemoval.size === 0) return;
@@ Expand Down @@

apps/meteor/app/lib/server/functions/notifications/email.js

-Original file line number
+Diff line change
@@ Expand Up @@
     		});
     	}
-    	if (message.t === 'e2e' && !message.file && !message.files?.length) {
+    	if (message.t === 'e2e') {
     		return settings.get('Email_notification_show_message') ? i18n.t('Encrypted_message_preview_unavailable', { lng }) : header;
     	}
@@ Expand Down @@

apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts

-Original file line number
+Diff line change
@@ Expand Up @@
     		msg.ignored = true;
     	}
-    	if (msg.t === 'e2e' && !msg.file) {
+    	if (msg.t === 'e2e') {
     		msg.e2e = 'pending';
     	}
@@ Expand Down @@

apps/meteor/client/lib/chats/flows/uploadFiles.ts

-Original file line number
+Diff line change
@@ Expand Up @@
     								hashes: {
     									sha256: encryptedFile.hash,
     								},
+    								fileId: _id,
     							};
     							if (/^image\/.+/.test(file.type)) {
@@ Expand Down @@

apps/meteor/client/lib/e2ee/rocketchat.e2e.room.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -9,7 +9,7 @@ import type {
  
    	EncryptedMessageContent,

    	EncryptedContent,

    } from '@rocket.chat/core-typings';

    import { isEncryptedMessageContent } from '@rocket.chat/core-typings';

    import { isEncryptedMessageContent, isFileAttachment, isRemovedFileAttachment } from '@rocket.chat/core-typings';

    import { Emitter } from '@rocket.chat/emitter';

    import type { Optional } from '@tanstack/react-query';

    import EJSON from 'ejson';

    @@ -660,6 +660,45 @@ export class E2ERoom extends Emitter {
  
    		return data;

    	}

    	async decryptMessageContent(message: IE2EEMessage): Promise<IE2EEMessage> {

    		const { attachments } = message;

    		const deletedAttachments = attachments?.filter((att) => isRemovedFileAttachment(att)) || [];

    		const deletedAllAttachment = deletedAttachments.find((att) => !att.fileId);

    		const content = await this.decrypt(message.content);

    		Object.assign(message, content);

    		// If the encrypted message had deleted files and the decrypted message has files, compare both lists to remove from the final result any file that was flagged as deleted

    		if (!deletedAttachments.length || !message.attachments?.length || !content.attachments?.length) {

    			return message;

    		}

    		message.attachments = message.attachments.map((att) => {

    			if (!isFileAttachment(att)) {

    				return att;

    			}

    			if (deletedAllAttachment) {

    				return deletedAllAttachment;

    			}

    			const fileId = att.fileId || message.file?._id;

    			if (!fileId) {

    				return att;

    			}

    			for (const removedAttachment of deletedAttachments) {

    				if (removedAttachment.fileId === fileId) {

    					return removedAttachment;

    				}

    			}

    			return att;

    		});

    		return message;

    	}

    	// Decrypt messages

    	async decryptMessage(message: IMessage | IE2EEMessage): Promise<IE2EEMessage | IMessage> {

    		if (message.t !== 'e2e' || message.e2e === 'done') {

    @@ -668,7 +707,7 @@ export class E2ERoom extends Emitter {
  
    		if (isEncryptedMessageContent(message)) {

    			return {

    				...(await this.decryptContent({ ...message })),

    				...(await this.decryptMessageContent({ ...message })),

    				e2e: 'done' as const,

    			};

    		}

apps/meteor/server/services/upload/service.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -107,7 +107,7 @@ export class UploadService extends ServiceClassInternal implements IUploadServic
  
    	private async updateMessageRemovingFiles(msg: IMessage, filesToRemove: IUpload['_id'][], user: IUser): Promise<void> {

    		const text = `_${i18n.t('File_removed')}_`;

    		const newAttachment = { color: NOTIFICATION_ATTACHMENT_COLOR, text };

    		const color = NOTIFICATION_ATTACHMENT_COLOR;

    		const newFiles = msg.files?.filter((file) => !filesToRemove.includes(file._id));

    		const newAttachments = msg.attachments?.map((attachment) => {

    @@ -120,7 +120,7 @@ export class UploadService extends ServiceClassInternal implements IUploadServic
  
    				return attachment;

    			}

    			return newAttachment;

    			return { type: 'removed-file', color, text, ...(attachment.fileId && { fileId: attachment.fileId }) };

    		});

    		const newFile = msg.file?._id && !filesToRemove.includes(msg.file._id) ? msg.file : newFiles?.[0];

packages/core-typings/src/IMessage/MessageAttachment/Files/RemovedFileAttachmentProps.ts

-Original file line number
+Diff line change
@@ -0,0 +1,11 @@
+    import type { MessageAttachmentBase } from '../MessageAttachmentBase';
+    import type { FileProp } from './FileProp';
+    export type RemovedFileAttachmentProps = MessageAttachmentBase & {
+    	type: 'removed-file';
+    	/* If fileId is missing, then every file in the message has been removed */
+    	fileId?: FileProp['_id'];
+    };
+    export const isRemovedFileAttachment = (attachment: MessageAttachmentBase): attachment is RemovedFileAttachmentProps =>
+    	'type' in attachment && attachment.type === 'removed-file';

packages/core-typings/src/IMessage/MessageAttachment/Files/index.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,4 +2,5 @@ export * from './AudioAttachmentProps'; @@
     export * from './FileAttachmentProps';
     export * from './FileProp';
     export * from './ImageAttachmentProps';
+    export * from './RemovedFileAttachmentProps';
     export * from './VideoAttachmentProps';

packages/core-typings/src/IMessage/MessageAttachment/MessageAttachment.ts

-Original file line number
+Diff line change
@@ -1,10 +1,16 @@
     import type { FileProp } from './Files';
     import type { FileAttachmentProps } from './Files/FileAttachmentProps';
+    import type { RemovedFileAttachmentProps } from './Files/RemovedFileAttachmentProps';
     import type { MessageAttachmentAction } from './MessageAttachmentAction';
     import type { MessageAttachmentDefault } from './MessageAttachmentDefault';
     import type { MessageQuoteAttachment } from './MessageQuoteAttachment';
-    export type MessageAttachment = MessageAttachmentAction | MessageAttachmentDefault | FileAttachmentProps | MessageQuoteAttachment;
+    export type MessageAttachment =
+    	| MessageAttachmentAction
+    	| MessageAttachmentDefault
+    	| FileAttachmentProps
+    	| MessageQuoteAttachment
+    	| RemovedFileAttachmentProps;
     export type FilesAndAttachments = {
     	files: FileProp[];
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: removing files from e2ee rooms do not remove their messages #38379

Diff view

Diff view

There are no files selected for viewing

Uh oh!

fix: removing files from e2ee rooms do not remove their messages #38379

Are you sure you want to change the base?

fix: removing files from e2ee rooms do not remove their messages #38379

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!