Improve reaction targeting

ts/messageModifiers/Reactions.ts

@@ -14,7 +14,13 @@ import { ReactionSource } from '../reactions/ReactionSource';
 import { DataReader, DataWriter } from '../sql/Client';
 import * as Errors from '../types/errors';
 import * as log from '../logging/log';
-import { getAuthor, isIncoming, isOutgoing } from '../messages/helpers';
+import {
+  getAuthor,
+  isIncoming,
+  isIncomingStory,
+  isOutgoing,
+  isOutgoingStory,
+} from '../messages/helpers';
 import { getMessageSentTimestampSet } from '../util/getMessageSentTimestampSet';
 import { isDirectConversation, isMe } from '../util/whatTypeOfConversation';
 import {
@@ -67,6 +73,7 @@ function remove(reaction: ReactionAttributesType): void {
 export function findReactionsForMessage(
   message: ReadonlyMessageAttributesType
 ): Array<ReactionAttributesType> {
+  const ourAci = window.textsecure.storage.user.getCheckedAci();
   const matchingReactions = Array.from(reactionCache.values()).filter(
     reaction => {
       return isMessageAMatchForReaction({
@@ -74,6 +81,7 @@ export function findReactionsForMessage(
         targetTimestamp: reaction.targetTimestamp,
         targetAuthorAci: reaction.targetAuthorAci,
         reactionSenderConversationId: reaction.fromId,
+        ourAci,
       });
     }
   );
@@ -94,6 +102,7 @@ async function findMessageForReaction({
   logId: string;
 }): Promise<MessageAttributesType | undefined> {
   const messages = await DataReader.getMessagesBySentAt(targetTimestamp);
+  const ourAci = window.textsecure.storage.user.getCheckedAci();
 
   const matchingMessages = messages.filter(message =>
     isMessageAMatchForReaction({
@@ -101,6 +110,7 @@ async function findMessageForReaction({
       targetTimestamp,
       targetAuthorAci,
       reactionSenderConversationId,
+      ourAci,
     })
   );
 
@@ -119,16 +129,18 @@ async function findMessageForReaction({
   return matchingMessages[0];
 }
 
-function isMessageAMatchForReaction({
+export function isMessageAMatchForReaction({
   message,
   targetTimestamp,
   targetAuthorAci,
   reactionSenderConversationId,
+  ourAci,
 }: {
   message: ReadonlyMessageAttributesType;
   targetTimestamp: number;
   targetAuthorAci: string;
   reactionSenderConversationId: string;
+  ourAci: AciString;
 }): boolean {
   if (!getMessageSentTimestampSet(message).has(targetTimestamp)) {
     return false;
@@ -158,7 +170,7 @@ function isMessageAMatchForReaction({
     return true;
   }
 
-  if (message.type === 'outgoing') {
+  if (isOutgoing(message) || isOutgoingStory(message, ourAci)) {
     const sendStateByConversationId = getPropForTimestamp({
       log,
       message,
@@ -172,13 +184,20 @@ function isMessageAMatchForReaction({
       return false;
     }
 
+    if (isStory(message)) {
+      return (
+        isSent(sendState.status) && Boolean(sendState.isAllowedToReplyToStory)
+      );
+    }
+
     return isSent(sendState.status);
   }
 
-  if (message.type === 'incoming') {
+  if (isIncoming(message) || isIncomingStory(message, ourAci)) {
     const messageConversation = window.ConversationController.get(
       message.conversationId
     );
+
     if (!messageConversation) {
       return false;
     }
@@ -190,7 +209,8 @@ function isMessageAMatchForReaction({
     );
   }
 
-  return true;
+  // Only incoming, outgoing, and story messages can be reacted to
+  return false;
 }
 
 export async function onReaction(

ts/messages/helpers.ts

@@ -11,7 +11,7 @@ import type {
   QuotedAttachmentType,
   QuotedMessageType,
 } from '../model-types.d';
-import type { ServiceIdString } from '../types/ServiceId';
+import type { AciString, ServiceIdString } from '../types/ServiceId';
 import { PaymentEventKind } from '../types/Payment';
 import type { AnyPaymentEvent } from '../types/Payment';
 import type { LocalizerType } from '../types/Util';
@@ -35,6 +35,27 @@ export function isStory(
   return message.type === 'story';
 }
 
+function isFromUs(
+  message: Pick<ReadonlyMessageAttributesType, 'sourceServiceId'>,
+  ourAci: AciString
+) {
+  return message.sourceServiceId === ourAci;
+}
+
+export function isOutgoingStory(
+  message: Pick<ReadonlyMessageAttributesType, 'type' | 'sourceServiceId'>,
+  ourAci: AciString
+): boolean {
+  return isStory(message) && isFromUs(message, ourAci);
+}
+
+export function isIncomingStory(
+  message: Pick<ReadonlyMessageAttributesType, 'type' | 'sourceServiceId'>,
+  ourAci: AciString
+): boolean {
+  return isStory(message) && !isFromUs(message, ourAci);
+}
+
 export type MessageAttributesWithPaymentEvent = ReadonlyMessageAttributesType &
   ReadonlyDeep<{
     payment: AnyPaymentEvent;

ts/models/conversations.ts

@@ -3348,16 +3348,14 @@ export class ConversationModel extends window.Backbone
       return;
     }
 
-    const lastMessage = this.get('timestamp') || Date.now();
+    const timestamp = Date.now();
-
     log.info(
       'adding verified change advisory for',
       this.idForLogging(),
       verifiedChangeId,
-      lastMessage
+      timestamp
     );
 
-    const timestamp = Date.now();
     const message = new MessageModel({
       ...generateMessageId(incrementMessageCounter()),
       conversationId: this.id,
@@ -3365,7 +3363,7 @@ export class ConversationModel extends window.Backbone
       readStatus: ReadStatus.Read,
       received_at_ms: timestamp,
       seenStatus: options.local ? SeenStatus.Seen : SeenStatus.Unseen,
-      sent_at: lastMessage,
+      sent_at: timestamp,
       timestamp,
       type: 'verified-change',
       verified,

ts/test-electron/util/reactions_test.ts

@@ -0,0 +1,521 @@
+// Copyright 2025 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { v4 as generateGuid } from 'uuid';
+import { DataWriter } from '../../sql/Client';
+import { generateAci, generatePni } from '../../types/ServiceId';
+import { isMessageAMatchForReaction } from '../../messageModifiers/Reactions';
+import { generateMessageId } from '../../util/generateMessageId';
+import { incrementMessageCounter } from '../../util/incrementMessageCounter';
+import type { ConversationModel } from '../../models/conversations';
+import type { MessageAttributesType } from '../../model-types';
+import { SendStatus } from '../../messages/MessageSendState';
+
+describe('isMessageAMatchForReaction', () => {
+  let contactA: ConversationModel;
+  let contactB: ConversationModel;
+  let contactC: ConversationModel;
+  let ourConversation: ConversationModel;
+  const OUR_ACI = generateAci();
+  const OUR_PNI = generatePni();
+  beforeEach(async () => {
+    await DataWriter.removeAll();
+    await window.textsecure.storage.user.setCredentials({
+      number: '+15550000000',
+      aci: OUR_ACI,
+      pni: OUR_PNI,
+      deviceId: 2,
+      deviceName: 'my device',
+      password: 'password',
+    });
+
+    window.ConversationController.reset();
+    await window.ConversationController.load();
+
+    contactA = window.ConversationController.getOrCreate(
+      generateAci(),
+      'private'
+    );
+    contactB = window.ConversationController.getOrCreate(
+      generateAci(),
+      'private'
+    );
+    contactC = window.ConversationController.getOrCreate(
+      generateAci(),
+      'private'
+    );
+
+    ourConversation = window.ConversationController.getOrCreate(
+      OUR_ACI,
+      'private'
+    );
+  });
+
+  describe('incoming 1:1 message', () => {
+    let message: MessageAttributesType;
+    beforeEach(() => {
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'incoming',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: contactA.id,
+        sourceServiceId: contactA.attributes.serviceId,
+        source: contactA.id,
+      };
+    });
+
+    it('matches on our reaction', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: ourConversation.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not match if target author differs', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactB.getCheckedAci(''),
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not match if reaction sender is not in the conversation', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+
+    it('does not match if timestamp differs', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 124,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does match if timestamp matches edit history', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message: {
+            ...message,
+            editHistory: [
+              {
+                timestamp: 124,
+                received_at: 124,
+              },
+            ],
+          },
+          targetTimestamp: 124,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it("matches on sender's own reaction", async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not match if reaction comes from a different sender', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('outgoing 1:1 message', () => {
+    let message: MessageAttributesType;
+    beforeEach(() => {
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'outgoing',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: contactA.id,
+        sourceServiceId: ourConversation.attributes.serviceId,
+        source: ourConversation.id,
+        sendStateByConversationId: {
+          [contactA.id]: {
+            status: SendStatus.Sent,
+          },
+        },
+      };
+    });
+    it("matches on recipient's reaction", async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('matches on our own reaction', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: ourConversation.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not match if reaction comes from a different sender', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not match if message not fully sent', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message: {
+            ...message,
+            sendStateByConversationId: {
+              [contactA.id]: {
+                status: SendStatus.Pending,
+              },
+            },
+          },
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('incoming group message', () => {
+    let message: MessageAttributesType;
+    let group: ConversationModel;
+    beforeEach(() => {
+      group = window.ConversationController.getOrCreate(
+        generateGuid(),
+        'group',
+        {
+          groupVersion: 2,
+          membersV2: [
+            {
+              aci: contactA.getCheckedAci(''),
+              joinedAtVersion: 2,
+              role: 1,
+            },
+            {
+              aci: contactB.getCheckedAci(''),
+              joinedAtVersion: 2,
+              role: 1,
+            },
+            {
+              aci: OUR_ACI,
+              joinedAtVersion: 2,
+              role: 1,
+            },
+          ],
+        }
+      );
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'incoming',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: group.id,
+        sourceServiceId: contactA.attributes.serviceId,
+        source: contactA.id,
+      };
+    });
+
+    it("matches on another recipient's reaction", async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not matches if sender is not in group', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactC.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('outgoing 1:1 story', () => {
+    let message: MessageAttributesType;
+    beforeEach(() => {
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'story',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: contactA.id,
+        sourceServiceId: ourConversation.attributes.serviceId,
+        source: ourConversation.id,
+        sendStateByConversationId: {
+          [contactA.id]: {
+            status: SendStatus.Sent,
+            isAllowedToReplyToStory: true,
+          },
+          [contactB.id]: {
+            status: SendStatus.Sent,
+            isAllowedToReplyToStory: false,
+          },
+        },
+      };
+    });
+    it('allows reactions from those allowed to react', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not allow reactions from those disallowed from reacting', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not allow reactions from non-recipients', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactC.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('incoming 1:1 story', () => {
+    let message: MessageAttributesType;
+    beforeEach(() => {
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'story',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: contactA.id,
+        sourceServiceId: contactA.attributes.serviceId,
+        source: contactA.id,
+      };
+    });
+    it('allows reactions from self', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: ourConversation.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not allow reactions from others', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('outgoing group story', () => {
+    let message: MessageAttributesType;
+    beforeEach(() => {
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'story',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: contactA.id,
+        sourceServiceId: ourConversation.attributes.serviceId,
+        source: ourConversation.id,
+        sendStateByConversationId: {
+          [contactA.id]: {
+            status: SendStatus.Sent,
+            isAllowedToReplyToStory: true,
+          },
+          [contactB.id]: {
+            status: SendStatus.Sent,
+            isAllowedToReplyToStory: false,
+          },
+        },
+      };
+    });
+    it('allows reactions from those allowed to react', async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not allow reactions from those disallowed from reacting', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not allow reactions from non-recipients', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: OUR_ACI,
+          reactionSenderConversationId: contactC.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('incoming group story message', () => {
+    let message: MessageAttributesType;
+    let group: ConversationModel;
+    beforeEach(() => {
+      group = window.ConversationController.getOrCreate(
+        generateGuid(),
+        'group',
+        {
+          groupVersion: 2,
+          membersV2: [
+            {
+              aci: contactA.getCheckedAci(''),
+              joinedAtVersion: 2,
+              role: 1,
+            },
+            {
+              aci: contactB.getCheckedAci(''),
+              joinedAtVersion: 2,
+              role: 1,
+            },
+            {
+              aci: OUR_ACI,
+              joinedAtVersion: 2,
+              role: 1,
+            },
+          ],
+        }
+      );
+      message = {
+        ...generateMessageId(incrementMessageCounter()),
+        type: 'story',
+        timestamp: 123,
+        sent_at: 123,
+        conversationId: group.id,
+        sourceServiceId: contactA.attributes.serviceId,
+        source: contactA.id,
+      };
+    });
+
+    it("matches on another recipient's reaction", async () => {
+      assert.isTrue(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactB.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+    it('does not matches if sender is not in group', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message,
+          targetTimestamp: 123,
+          targetAuthorAci: contactA.getCheckedAci(''),
+          reactionSenderConversationId: contactC.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+  describe('other message types', () => {
+    it('does not match on other message types', async () => {
+      assert.isFalse(
+        isMessageAMatchForReaction({
+          message: {
+            ...generateMessageId(incrementMessageCounter()),
+            type: 'verified-change',
+            timestamp: 123,
+            sent_at: 123,
+            conversationId: contactA.id,
+            sourceServiceId: ourConversation.attributes.serviceId,
+            source: ourConversation.id,
+          },
+          targetTimestamp: 123,
+          targetAuthorAci: ourConversation.getCheckedAci(''),
+          reactionSenderConversationId: contactA.id,
+          ourAci: OUR_ACI,
+        })
+      );
+    });
+  });
+});