fix: make group control message not expire

ts/interactions/conversationInteractions.ts

@@ -384,6 +384,7 @@ export async function setDisappearingMessagesByConvoId(
       providedExpireTimer: 0,
       fromSync: false,
       fromCurrentDevice: true,
+      fromConfigMessage: false,
     });
   } else {
     await conversation.updateExpireTimer({
@@ -391,6 +392,7 @@ export async function setDisappearingMessagesByConvoId(
       providedExpireTimer: seconds,
       fromSync: false,
       fromCurrentDevice: true,
+      fromConfigMessage: false,
     });
   }
 }

ts/models/conversation.ts

@@ -17,6 +17,7 @@ import {
   xor,
 } from 'lodash';
 
+import { v4 } from 'uuid';
 import { SignalService } from '../protobuf';
 import { getMessageQueue } from '../session';
 import { getConversationController } from '../session/conversations';
@@ -117,7 +118,6 @@ import {
   getSubscriberCountOutsideRedux,
 } from '../state/selectors/sogsRoomInfo'; // decide it it makes sense to move this to a redux slice?
 
-import { v4 } from 'uuid';
 import { DisappearingMessages } from '../session/disappearing_messages';
 import { DisappearingMessageConversationModeType } from '../session/disappearing_messages/types';
 import { FetchMsgExpirySwarm } from '../session/utils/job_runners/jobs/FetchMsgExpirySwarmJob';
@@ -826,7 +826,8 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     providedExpireTimer,
     providedSource,
     receivedAt, // is set if it comes from outside
-    fromSync, // if the update comes from a config or sync message
+    fromSync, // if the update comes from sync message ONLY
+    fromConfigMessage, // if the update comes from a libsession config message ONLY
     fromCurrentDevice,
     shouldCommitConvo = true,
     existingMessage,
@@ -837,10 +838,16 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     receivedAt?: number; // is set if it comes from outside
     fromSync: boolean;
     fromCurrentDevice: boolean;
+    fromConfigMessage: boolean;
     shouldCommitConvo?: boolean;
     existingMessage?: MessageModel;
   }): Promise<boolean> {
-    const isRemoteChange = Boolean((receivedAt || fromSync) && !fromCurrentDevice);
+    const isRemoteChange = Boolean(
+      (receivedAt || fromSync || fromConfigMessage) && !fromCurrentDevice
+    );
+
+    // we don't add an update message when this comes from a config message, as we already have the SyncedMessage itself with the right timestamp to display
+    const shouldAddExpireUpdateMessage = !fromConfigMessage;
 
     if (this.isPublic()) {
       throw new Error("updateExpireTimer() Disappearing messages aren't supported in communities");
@@ -887,6 +894,16 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
       });
     }
 
+    if (!shouldAddExpireUpdateMessage) {
+      await Conversation.cleanUpExpireHistoryFromConvo(this.id, this.isPrivate());
+
+      if (shouldCommitConvo) {
+        // tell the UI this conversation was updated
+        await this.commit();
+      }
+      return false;
+    }
+
     let message = existingMessage || undefined;
     const expirationType = DisappearingMessages.changeToDisappearingMessageType(
       this,
@@ -922,12 +939,13 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
         });
       }
     }
-    // force that message to expire with the old disappear setting when the setting was turned off.
-    // this is to make the update to 'off' disappear with the previous disappearing message setting
+
+    // Note: we agreed that a closed group ControlMessage message does not expire.
     message.set({
-      expirationType,
-      expireTimer,
+      expirationType: this.isClosedGroup() ? 'unknown' : expirationType,
+      expireTimer: this.isClosedGroup() ? 0 : expireTimer,
     });
+
     if (!message.get('id')) {
       message.set({ id: v4() });
     }
@@ -950,7 +968,9 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
       );
 
       if (!message.getExpirationStartTimestamp()) {
-        const canBeDeleteAfterSend = this.isMe() || this.isGroup();
+        // Note: we agreed that a closed group ControlMessage message does not expire.
+
+        const canBeDeleteAfterSend = this.isMe() || !(this.isGroup() && message.isControlMessage());
         if (
           (canBeDeleteAfterSend && expirationMode === 'legacy') ||
           expirationMode === 'deleteAfterSend'
@@ -1009,13 +1029,13 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     }
     if (this.isClosedGroup()) {
       if (this.isAdmin(UserUtils.getOurPubKeyStrFromCache())) {
+        // NOTE: we agreed that outgoing ExpirationTimerUpdate **for groups** are not expiring,
+        // but they still need the content to be right(as this is what we use for the change itself)
+
         const expireUpdateForGroup = {
           ...expireUpdate,
           groupId: this.get('id'),
         };
-        // NOTE: we agreed that outgoing ExpirationTimerUpdate **for groups** are not expiring.
-        expireUpdate.expirationType = 'unknown';
-        expireUpdate.expireTimer = 0;
 
         const expirationTimerMessage = new ExpirationTimerUpdateMessage(expireUpdateForGroup);
 

ts/models/message.ts

@@ -198,6 +198,15 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
     return Boolean(flags & expirationTimerFlag) && !isEmpty(this.getExpirationTimerUpdate());
   }
 
+  public isControlMessage() {
+    return (
+      this.isExpirationTimerUpdate() ||
+      this.isDataExtractionNotification() ||
+      this.isMessageRequestResponse ||
+      this.isGroupUpdate()
+    );
+  }
+
   public isIncoming() {
     return this.get('type') === 'incoming';
   }
@@ -1232,6 +1241,10 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
     throttledAllMessagesDispatch();
   }
 
+  private isGroupUpdate() {
+    return !isEmpty(this.get('group_update'));
+  }
+
   /**
    * Before, group_update attributes could be just the string 'You' and not an array.
    * Using this method to get the group update makes sure than the joined, kicked, or left are always an array of string, or undefined

ts/receiver/closedGroups.ts

@@ -325,6 +325,7 @@ export async function handleNewClosedGroup(
         receivedAt: GetNetworkTime.getNowWithNetworkOffset(),
         fromSync: false,
         fromCurrentDevice: false,
+        fromConfigMessage: false,
       });
 
       await removeFromCache(envelope);

ts/receiver/configMessage.ts

@@ -248,6 +248,7 @@ async function handleUserProfileUpdate(result: IncomingConfResult): Promise<Inco
         fromSync: true,
         shouldCommitConvo: false,
         fromCurrentDevice: false,
+        fromConfigMessage: true,
       });
       changes = success;
     }
@@ -394,6 +395,7 @@ async function handleContactsUpdate(result: IncomingConfResult): Promise<Incomin
           fromSync: true,
           fromCurrentDevice: false,
           shouldCommitConvo: false,
+          fromConfigMessage: true,
         });
         changes = changes || success;
       }
@@ -627,6 +629,7 @@ async function handleLegacyGroupUpdate(latestEnvelopeTimestamp: number) {
         fromSync: true,
         shouldCommitConvo: false,
         fromCurrentDevice: false,
+        fromConfigMessage: true,
       });
       changes = success;
     }

ts/receiver/contentMessage.ts

@@ -13,17 +13,13 @@ import {
   deleteMessagesFromSwarmAndCompletelyLocally,
   deleteMessagesFromSwarmAndMarkAsDeletedLocally,
 } from '../interactions/conversations/unsendingInteractions';
-import {
-  CONVERSATION_PRIORITIES,
-  ConversationTypeEnum,
-  READ_MESSAGE_STATE,
-} from '../models/conversationAttributes';
+import { CONVERSATION_PRIORITIES, ConversationTypeEnum } from '../models/conversationAttributes';
 import { findCachedBlindedMatchOrLookupOnAllServers } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys';
 import { getConversationController } from '../session/conversations';
 import { concatUInt8Array, getSodiumRenderer } from '../session/crypto';
 import { removeMessagePadding } from '../session/crypto/BufferPadding';
 import { DisappearingMessages } from '../session/disappearing_messages';
-import { DisappearingMessageMode } from '../session/disappearing_messages/types';
+import { DisappearingMessageUpdate } from '../session/disappearing_messages/types';
 import { ProfileManager } from '../session/profile_manager/ProfileManager';
 import { GroupUtils, UserUtils } from '../session/utils';
 import { perfEnd, perfStart } from '../session/utils/Performance';
@@ -482,17 +478,17 @@ export async function innerHandleSwarmContentMessage({
       );
     }
 
+    const expireUpdate = await DisappearingMessages.checkForExpireUpdateInContentMessage(
+      content,
+      conversationModelForUIUpdate,
+      messageExpirationFromRetrieve
+    );
     if (content.dataMessage) {
       // because typescript is funky with incoming protobufs
       if (isEmpty(content.dataMessage.profileKey)) {
         content.dataMessage.profileKey = null;
       }
 
-      const expireUpdate = await DisappearingMessages.checkForExpireUpdateInContentMessage(
-        content,
-        conversationModelForUIUpdate,
-        messageExpirationFromRetrieve
-      );
       // TODO legacy messages support will be removed in a future release
       if (expireUpdate?.isDisappearingMessagesV2Released) {
         await DisappearingMessages.checkHasOutdatedDisappearingMessageClient(
@@ -554,7 +550,7 @@ export async function innerHandleSwarmContentMessage({
       await handleDataExtractionNotification(
         envelope,
         content.dataExtractionNotification as SignalService.DataExtractionNotification,
-        content
+        expireUpdate || null
       );
       perfEnd(
         `handleDataExtractionNotification-${envelope.id}`,
@@ -835,14 +831,14 @@ async function handleMessageRequestResponse(
 }
 
 /**
- * A DataExtractionNotification message can only come from a 1 o 1 conversation.
+ * A DataExtractionNotification message can only come from a 1o1 conversation.
  *
- * We drop them if the convo is not a 1 o 1 conversation.
+ * We drop them if the convo is not a 1o1 conversation.
  */
 export async function handleDataExtractionNotification(
   envelope: EnvelopePlus,
   dataNotificationMessage: SignalService.DataExtractionNotification,
-  content: SignalService.Content
+  expireUpdate: DisappearingMessageUpdate | null
 ): Promise<void> {
   // we currently don't care about the timestamp included in the field itself, just the timestamp of the envelope
   const { type, timestamp: referencedAttachment } = dataNotificationMessage;
@@ -852,9 +848,8 @@ export async function handleDataExtractionNotification(
 
   const convo = getConversationController().get(source);
   if (!convo || !convo.isPrivate()) {
-    window?.log?.info(
-      'Got DataNotification for unknown or non private convo or read receipt not enabled'
-    );
+    window?.log?.info('Got DataNotification for unknown or non-private convo');
+
     return;
   }
 
@@ -866,34 +861,8 @@ export async function handleDataExtractionNotification(
 
   const envelopeTimestamp = toNumber(timestamp);
   const referencedAttachmentTimestamp = toNumber(referencedAttachment);
-  const expireTimer = content.expirationTimer || 0;
-
-  const expirationMode = DisappearingMessages.changeToDisappearingConversationMode(
-    convo,
-    DisappearingMessageMode[content.expirationType],
-    expireTimer
-  );
-  let expirationType;
-  let expirationStartTimestamp;
-
-  if (convo && expirationMode && expireTimer > 0) {
-    expirationType =
-      expirationMode !== 'off'
-        ? DisappearingMessages.changeToDisappearingMessageType(convo, expireTimer, expirationMode)
-        : undefined;
-
-    // NOTE Triggers disappearing for an incoming DataExtractionNotification message
-    // TODO legacy messages support will be removed in a future release
-    if (expirationMode === 'legacy' || expirationMode === 'deleteAfterSend') {
-      expirationStartTimestamp = DisappearingMessages.setExpirationStartTimestamp(
-        expirationMode,
-        undefined,
-        'handleDataExtractionNotification'
-      );
-    }
-  }
 
-  await convo.addSingleIncomingMessage({
+  let created = await convo.addSingleIncomingMessage({
     source,
     sent_at: envelopeTimestamp,
     dataExtractionNotification: {
@@ -901,12 +870,13 @@ export async function handleDataExtractionNotification(
       referencedAttachmentTimestamp, // currently unused
       source,
     },
-
-    unread: READ_MESSAGE_STATE.unread,
-    expirationType,
-    expireTimer,
-    expirationStartTimestamp,
   });
-
+  created = DisappearingMessages.getMessageReadyToDisappear(
+    convo,
+    created,
+    0,
+    expireUpdate || undefined
+  );
+  await created.commit();
   convo.updateLastMessage();
 }

ts/receiver/queuedJob.ts

@@ -434,6 +434,7 @@ export async function handleMessageJob(
         existingMessage: messageModel,
         shouldCommitConvo: false,
         fromCurrentDevice: false,
+        fromConfigMessage: false,
         // NOTE we don't commit yet because we want to get the message id, see below
       });
     } else {

ts/session/disappearing_messages/index.ts

@@ -399,11 +399,15 @@ function checkForExpiringOutgoingMessage(message: MessageModel, location?: strin
   const expireTimer = message.getExpireTimerSeconds();
   const expirationType = message.getExpirationType();
 
+  const isGroupConvo = !!convo?.isClosedGroup();
+  const isControlMessage = message.isControlMessage();
+
   if (
     convo &&
     expirationType &&
     expireTimer > 0 &&
-    Boolean(message.getExpirationStartTimestamp()) === false
+    !message.getExpirationStartTimestamp() &&
+    !(isGroupConvo && isControlMessage)
   ) {
     const expirationMode = changeToDisappearingConversationMode(convo, expirationType, expireTimer);
 
@@ -444,6 +448,24 @@ function getMessageReadyToDisappear(
     messageExpirationFromRetrieve,
   } = expireUpdate;
 
+  // This message is an ExpirationTimerUpdate
+  if (messageFlags === SignalService.DataMessage.Flags.EXPIRATION_TIMER_UPDATE) {
+    const expirationTimerUpdate = {
+      expirationType,
+      expireTimer,
+      source: messageModel.get('source'),
+    };
+
+    messageModel.set({
+      expirationTimerUpdate,
+    });
+  }
+
+  // Note: We agreed that a control message for legacy groups does not expire
+  if (conversationModel.isClosedGroup() && messageModel.isControlMessage()) {
+    return messageModel;
+  }
+
   /**
    * This is quite tricky, but when we receive a message from the network, it might be a disappearing after read one, which was already read by another device.
    * If that's the case, we need to not only mark the message as read, but also mark it as read at the right time.
@@ -496,19 +518,6 @@ function getMessageReadyToDisappear(
     });
   }
 
-  // This message is an ExpirationTimerUpdate
-  if (messageFlags === SignalService.DataMessage.Flags.EXPIRATION_TIMER_UPDATE) {
-    const expirationTimerUpdate = {
-      expirationType,
-      expireTimer,
-      source: messageModel.get('source'),
-    };
-
-    messageModel.set({
-      expirationTimerUpdate,
-    });
-  }
-
   return messageModel;
 }
 

ts/session/group/closed-group.ts

@@ -109,12 +109,8 @@ export async function initiateClosedGroupUpdate(
   const sharedDetails = {
     sender: UserUtils.getOurPubKeyStrFromCache(),
     sentAt: Date.now(),
-
-    expireUpdate: {
-      expirationType: groupDetails.expirationType || ('unknown' as const),
-      expirationTimer: expireTimer || 0,
-      messageExpirationFromRetrieve: GetNetworkTime.getNowWithNetworkOffset() + expireTimer * 1000,
-    },
+    // Note: we agreed that legacy group control messages do not expire
+    expireUpdate: null,
     convo,
   };
 

ts/test/session/unit/disappearing_messages/DisappearingMessage_test.ts

@@ -561,6 +561,7 @@ describe('DisappearingMessage', () => {
           shouldCommitConvo: false,
           existingMessage: undefined,
           fromCurrentDevice: false,
+          fromConfigMessage: false,
         });
         await expect(promise).is.rejectedWith(
           "updateExpireTimer() Disappearing messages aren't supported in communities"
@@ -590,6 +591,7 @@ describe('DisappearingMessage', () => {
           shouldCommitConvo: false,
           existingMessage: undefined,
           fromCurrentDevice: false,
+          fromConfigMessage: false,
         });
         expect(updateSuccess, 'should be true').to.be.true;
       });
@@ -613,6 +615,7 @@ describe('DisappearingMessage', () => {
           shouldCommitConvo: false,
           existingMessage: undefined,
           fromCurrentDevice: false,
+          fromConfigMessage: false,
         });
         expect(updateSuccess, 'should be true').to.be.true;
         expect(