Delete for Everyone: Don't allow unrestricted deletes in Note to Self

Co-authored-by: Scott Nonnenberg <scott@signal.org>
signalapp · Jan 3, 2024 · 39897c4 · 39897c4
1 parent a98c5ba
commit 39897c4
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 15 deletions.
diff --git a/ts/components/DeleteMessagesModal.tsx b/ts/components/DeleteMessagesModal.tsx
@@ -41,7 +41,7 @@ export default function DeleteMessagesModal({
       : i18n('icu:DeleteMessagesModal--deleteForMe'),
   });
 
-  if (canDeleteForEveryone || isMe) {
+  if (canDeleteForEveryone) {
     const tooManyMessages = messageCount > MAX_DELETE_FOR_EVERYONE;
     actions.push({
       'aria-disabled': tooManyMessages,

diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts
@@ -727,7 +727,7 @@ export const getPropsForMessage = (
     payment,
     canCopy: canCopy(message),
     canEditMessage: canEditMessage(message),
-    canDeleteForEveryone: canDeleteForEveryone(message),
+    canDeleteForEveryone: canDeleteForEveryone(message, conversation.isMe),
     canDownload: canDownload(message, conversationSelector),
     canReact: canReact(message, ourConversationId, conversationSelector),
     canReply: canReply(message, ourConversationId, conversationSelector),
@@ -1872,26 +1872,31 @@ export function canDeleteForEveryone(
   message: Pick<
     MessageWithUIFieldsType,
     'type' | 'deletedForEveryone' | 'sent_at' | 'sendStateByConversationId'
-  >
+  >,
+  isMe: boolean
 ): boolean {
   return (
     // Is this a message I sent?
     isOutgoing(message) &&
     // Has the message already been deleted?
     !message.deletedForEveryone &&
-    // Is it too old to delete?
-    isMoreRecentThan(message.sent_at, DAY) &&
+    // Is it too old to delete? (we relax that requirement in Note to Self)
+    (isMoreRecentThan(message.sent_at, DAY) || isMe) &&
     // Is it sent to anyone?
     someSendStatus(message.sendStateByConversationId, isSent)
   );
 }
 
 export const canDeleteMessagesForEveryone = createSelector(
-  [getMessages, (_state, messageIds: ReadonlyArray<string>) => messageIds],
-  (messagesLookup, messageIds) => {
-    return messageIds.every(messageId => {
+  [
+    getMessages,
+    (_state, options: { messageIds: ReadonlyArray<string>; isMe: boolean }) =>
+      options,
+  ],
+  (messagesLookup, options) => {
+    return options.messageIds.every(messageId => {
       const message = getOwn(messagesLookup, messageId);
-      return message != null && canDeleteForEveryone(message);
+      return message != null && canDeleteForEveryone(message, options.isMe);
     });
   }
 );

diff --git a/ts/state/smart/DeleteMessagesModal.tsx b/ts/state/smart/DeleteMessagesModal.tsx
@@ -29,7 +29,7 @@ export function SmartDeleteMessagesModal(): JSX.Element | null {
   });
 
   const canDeleteForEveryone = useSelector((state: StateType) => {
-    return canDeleteMessagesForEveryone(state, messageIds);
+    return canDeleteMessagesForEveryone(state, { messageIds, isMe });
   });
   const lastSelectedMessage = useSelector((state: StateType) => {
     return state.conversations.lastSelectedMessage;

diff --git a/ts/test-electron/state/selectors/messages_test.ts b/ts/test-electron/state/selectors/messages_test.ts
@@ -73,8 +73,9 @@ describe('state/selectors/messages', () => {
         type: 'incoming' as const,
         sent_at: Date.now() - 1000,
       };
+      const isMe = false;
 
-      assert.isFalse(canDeleteForEveryone(message));
+      assert.isFalse(canDeleteForEveryone(message, isMe));
     });
 
     it('returns false for messages that were already deleted for everyone', () => {
@@ -93,8 +94,9 @@ describe('state/selectors/messages', () => {
           },
         },
       };
+      const isMe = false;
 
-      assert.isFalse(canDeleteForEveryone(message));
+      assert.isFalse(canDeleteForEveryone(message, isMe));
     });
 
     it('returns false for messages that were are too old to delete', () => {
@@ -112,8 +114,9 @@ describe('state/selectors/messages', () => {
           },
         },
       };
+      const isMe = false;
 
-      assert.isFalse(canDeleteForEveryone(message));
+      assert.isFalse(canDeleteForEveryone(message, isMe));
     });
 
     it("returns false for messages that haven't been sent to anyone", () => {
@@ -131,8 +134,9 @@ describe('state/selectors/messages', () => {
           },
         },
       };
+      const isMe = false;
 
-      assert.isFalse(canDeleteForEveryone(message));
+      assert.isFalse(canDeleteForEveryone(message, isMe));
     });
 
     it('returns true for messages that meet all criteria for deletion', () => {
@@ -154,8 +158,9 @@ describe('state/selectors/messages', () => {
           },
         },
       };
+      const isMe = false;
 
-      assert.isTrue(canDeleteForEveryone(message));
+      assert.isTrue(canDeleteForEveryone(message, isMe));
     });
   });