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

This commit is contained in:
Scott Nonnenberg 2024-01-03 08:46:39 -08:00 committed by GitHub
parent 677ab64335
commit 0cc6228ede
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 25 additions and 15 deletions

View file

@ -41,7 +41,7 @@ export default function DeleteMessagesModal({
: i18n('icu:DeleteMessagesModal--deleteForMe'), : i18n('icu:DeleteMessagesModal--deleteForMe'),
}); });
if (canDeleteForEveryone || isMe) { if (canDeleteForEveryone) {
const tooManyMessages = messageCount > MAX_DELETE_FOR_EVERYONE; const tooManyMessages = messageCount > MAX_DELETE_FOR_EVERYONE;
actions.push({ actions.push({
'aria-disabled': tooManyMessages, 'aria-disabled': tooManyMessages,

View file

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

View file

@ -29,7 +29,7 @@ export function SmartDeleteMessagesModal(): JSX.Element | null {
}); });
const canDeleteForEveryone = useSelector((state: StateType) => { const canDeleteForEveryone = useSelector((state: StateType) => {
return canDeleteMessagesForEveryone(state, messageIds); return canDeleteMessagesForEveryone(state, { messageIds, isMe });
}); });
const lastSelectedMessage = useSelector((state: StateType) => { const lastSelectedMessage = useSelector((state: StateType) => {
return state.conversations.lastSelectedMessage; return state.conversations.lastSelectedMessage;

View file

@ -73,8 +73,9 @@ describe('state/selectors/messages', () => {
type: 'incoming' as const, type: 'incoming' as const,
sent_at: Date.now() - 1000, 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', () => { 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', () => { 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", () => { 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', () => { 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));
}); });
}); });