chore: incoming group update message need their own expireTimer

we do not trust the setting from the convo anymore

ts/models/conversation.ts

@@ -136,7 +136,7 @@ type InMemoryConvoInfos = {
 const inMemoryConvoInfos: Map<string, InMemoryConvoInfos> = new Map();
 
 export class ConversationModel extends Backbone.Model<ConversationAttributes> {
-  public updateLastMessage: () => any;
+  public updateLastMessage: () => unknown; // unknown because it is a Promise that we do not wait to await
   public throttledBumpTyping: () => void;
   public throttledNotify: (message: MessageModel) => void;
   public markConversationRead: (opts: {
@@ -857,14 +857,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     // to be above the message that initiated that change, hence the subtraction.
     const timestamp = (receivedAt || Date.now()) - 1;
 
-    // NOTE if we turn off disappearing messages we want the control message to expire based on the last available setting
+    // NOTE when we turn the disappearing setting to off, we don't want it to expire with the previous expiration anymore
-    const oldExpirationMode = this.getExpirationMode();
-    const oldExpireTimer = this.getExpireTimer();
-    const oldExpirationType = DisappearingMessages.changeToDisappearingMessageType(
-      this,
-      oldExpireTimer,
-      oldExpirationMode
-    );
 
     const isV2DisappearReleased = ReleasedFeatures.isDisappearMessageV2FeatureReleasedCached();
     // when the v2 disappear is released, the changes we make are only for our outgoing messages, not shared with a contact anymore
@@ -931,8 +924,8 @@ 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
     message.set({
-      expirationType: expireTimer === 0 ? oldExpirationType : expirationType,
+      expirationType,
-      expireTimer: expireTimer === 0 ? oldExpireTimer : expireTimer,
+      expireTimer,
     });
 
     if (this.isActive()) {
@@ -2047,6 +2040,8 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     const existingLastMessageAttribute = this.get('lastMessage');
     const existingLastMessageStatus = this.get('lastMessageStatus');
 
+    // TODO when the last message get removed from a conversation, the lastUpdate is ignored and we keep the last message.
+
     if (
       lastMessageUpdate.lastMessage !== existingLastMessageAttribute ||
       lastMessageUpdate.lastMessageStatus !== existingLastMessageStatus
@@ -2384,13 +2379,15 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
   }
 
   private matchesDisappearingMode(mode: DisappearingMessageConversationModeType) {
+    const ours = this.getExpirationMode();
+    // Note: couldn't this be ours === mode with a twist maybe?
     const success =
       mode === 'deleteAfterRead'
-        ? this.getExpirationMode() === 'deleteAfterRead'
+        ? ours === 'deleteAfterRead'
         : mode === 'deleteAfterSend'
-        ? this.getExpirationMode() === 'deleteAfterSend'
+        ? ours === 'deleteAfterSend'
         : mode === 'off'
-        ? this.getExpirationMode() === 'off'
+        ? ours === 'off'
         : false;
 
     return success;
@@ -2545,8 +2542,6 @@ async function cleanUpExpireHistoryFromConvo(conversationId: string, isPrivate:
     conversationId,
     isPrivate
   );
-  console.warn('cleanUpExpirationTimerUpdateHistory', conversationId, isPrivate, updateIdsRemoved);
-
   window.inboxStore.dispatch(
     messagesDeleted(updateIdsRemoved.map(m => ({ conversationKey: conversationId, messageId: m })))
   );

ts/models/message.ts

@@ -880,12 +880,6 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
       }
 
       const timestamp = Date.now(); // force a new timestamp to handle user fixed his clock;
-      const expireTimer = conversation.getExpireTimer();
-      const expirationType = DisappearingMessages.changeToDisappearingMessageType(
-        conversation,
-        expireTimer,
-        conversation.getExpirationMode()
-      );
 
       const chatParams: VisibleMessageParams = {
         identifier: this.id,
@@ -895,8 +889,10 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
         preview: preview ? [preview] : [],
         quote,
         lokiProfile: UserUtils.getOurProfile(),
-        expirationType,
+        // Note: we should have the fields set on that object when we've added it to the DB.
-        expireTimer,
+        // We don't want to reuse the conversation setting, as it might change since this message was sent.
+        expirationType: this.getExpirationType() || null,
+        expireTimer: this.getExpireTimerSeconds(),
       };
       if (!chatParams.lokiProfile) {
         delete chatParams.lokiProfile;

ts/receiver/closedGroups.ts

@@ -24,6 +24,7 @@ import { perfEnd, perfStart } from '../session/utils/Performance';
 import { ReleasedFeatures } from '../util/releaseFeature';
 import { Storage } from '../util/storage';
 // eslint-disable-next-line import/no-unresolved, import/extensions
+import { DisappearingMessageUpdate } from '../session/disappearing_messages/types';
 import { ConfigWrapperObjectTypes } from '../webworker/workers/browser/libsession_worker_functions';
 import { getSettingsKeyFromLibsessionWrapper } from './configMessage';
 import { ECKeyPair, HexKeyPair } from './keypairs';
@@ -79,7 +80,8 @@ export async function removeAllClosedGroupEncryptionKeyPairs(groupPubKey: string
 
 export async function handleClosedGroupControlMessage(
   envelope: EnvelopePlus,
-  groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage
+  groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage,
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   const { type } = groupUpdate;
   const { Type } = SignalService.DataMessage.ClosedGroupControlMessage;
@@ -132,7 +134,7 @@ export async function handleClosedGroupControlMessage(
     type === Type.MEMBER_LEFT ||
     type === Type.ENCRYPTION_KEY_PAIR_REQUEST
   ) {
-    await performIfValid(envelope, groupUpdate);
+    await performIfValid(envelope, groupUpdate, expireUpdate);
     return;
   }
 
@@ -513,7 +515,8 @@ async function handleClosedGroupEncryptionKeyPair(
 
 async function performIfValid(
   envelope: EnvelopePlus,
-  groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage
+  groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage,
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   const { Type } = SignalService.DataMessage.ClosedGroupControlMessage;
 
@@ -577,13 +580,31 @@ async function performIfValid(
   const shouldNotApplyGroupChange = moreRecentOrNah === 'wrapper_more_recent';
 
   if (groupUpdate.type === Type.NAME_CHANGE) {
-    await handleClosedGroupNameChanged(envelope, groupUpdate, convo, shouldNotApplyGroupChange);
+    await handleClosedGroupNameChanged(
+      envelope,
+      groupUpdate,
+      convo,
+      shouldNotApplyGroupChange,
+      expireUpdate
+    );
   } else if (groupUpdate.type === Type.MEMBERS_ADDED) {
-    await handleClosedGroupMembersAdded(envelope, groupUpdate, convo, shouldNotApplyGroupChange);
+    await handleClosedGroupMembersAdded(
+      envelope,
+      groupUpdate,
+      convo,
+      shouldNotApplyGroupChange,
+      expireUpdate
+    );
   } else if (groupUpdate.type === Type.MEMBERS_REMOVED) {
-    await handleClosedGroupMembersRemoved(envelope, groupUpdate, convo, shouldNotApplyGroupChange);
+    await handleClosedGroupMembersRemoved(
+      envelope,
+      groupUpdate,
+      convo,
+      shouldNotApplyGroupChange,
+      expireUpdate
+    );
   } else if (groupUpdate.type === Type.MEMBER_LEFT) {
-    await handleClosedGroupMemberLeft(envelope, convo, shouldNotApplyGroupChange);
+    await handleClosedGroupMemberLeft(envelope, convo, shouldNotApplyGroupChange, expireUpdate);
   } else if (groupUpdate.type === Type.ENCRYPTION_KEY_PAIR_REQUEST) {
     await removeFromCache(envelope);
   }
@@ -594,7 +615,8 @@ async function handleClosedGroupNameChanged(
   envelope: EnvelopePlus,
   groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage,
   convo: ConversationModel,
-  shouldOnlyAddUpdateMessage: boolean // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  shouldOnlyAddUpdateMessage: boolean, // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   // Only add update message if we have something to show
   const newName = groupUpdate.name;
@@ -604,12 +626,13 @@ async function handleClosedGroupNameChanged(
     const groupDiff: ClosedGroup.GroupDiff = {
       newName,
     };
-    await ClosedGroup.addUpdateMessage(
+    await ClosedGroup.addUpdateMessage({
       convo,
-      groupDiff,
+      diff: groupDiff,
-      envelope.senderIdentity,
+      sender: envelope.senderIdentity,
-      toNumber(envelope.timestamp)
+      sentAt: toNumber(envelope.timestamp),
-    );
+      expireUpdate,
+    });
     if (!shouldOnlyAddUpdateMessage) {
       convo.set({ displayNameInProfile: newName });
     }
@@ -624,7 +647,8 @@ async function handleClosedGroupMembersAdded(
   envelope: EnvelopePlus,
   groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage,
   convo: ConversationModel,
-  shouldOnlyAddUpdateMessage: boolean // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  shouldOnlyAddUpdateMessage: boolean, // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   const { members: addedMembersBinary } = groupUpdate;
   const addedMembers = (addedMembersBinary || []).map(toHex);
@@ -663,12 +687,13 @@ async function handleClosedGroupMembersAdded(
   const groupDiff: ClosedGroup.GroupDiff = {
     joiningMembers: membersNotAlreadyPresent,
   };
-  await ClosedGroup.addUpdateMessage(
+  await ClosedGroup.addUpdateMessage({
     convo,
-    groupDiff,
+    diff: groupDiff,
-    envelope.senderIdentity,
+    sender: envelope.senderIdentity,
-    toNumber(envelope.timestamp)
+    sentAt: toNumber(envelope.timestamp),
-  );
+    expireUpdate,
+  });
 
   if (!shouldOnlyAddUpdateMessage) {
     convo.set({ members });
@@ -693,7 +718,8 @@ async function handleClosedGroupMembersRemoved(
   envelope: EnvelopePlus,
   groupUpdate: SignalService.DataMessage.ClosedGroupControlMessage,
   convo: ConversationModel,
-  shouldOnlyAddUpdateMessage: boolean // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  shouldOnlyAddUpdateMessage: boolean, // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   // Check that the admin wasn't removed
   const currentMembers = convo.get('members');
@@ -745,12 +771,13 @@ async function handleClosedGroupMembersRemoved(
       const groupDiff: ClosedGroup.GroupDiff = {
         kickedMembers: effectivelyRemovedMembers,
       };
-      await ClosedGroup.addUpdateMessage(
+      await ClosedGroup.addUpdateMessage({
         convo,
-        groupDiff,
+        diff: groupDiff,
-        envelope.senderIdentity,
+        sender: envelope.senderIdentity,
-        toNumber(envelope.timestamp)
+        sentAt: toNumber(envelope.timestamp),
-      );
+        expireUpdate,
+      });
       convo.updateLastMessage();
     }
 
@@ -833,7 +860,8 @@ async function handleClosedGroupLeftOurself(groupId: string, envelope: EnvelopeP
 async function handleClosedGroupMemberLeft(
   envelope: EnvelopePlus,
   convo: ConversationModel,
-  shouldOnlyAddUpdateMessage: boolean // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  shouldOnlyAddUpdateMessage: boolean, // set this to true to not apply the change to the convo itself, just add the update in the conversation
+  expireUpdate: DisappearingMessageUpdate | null
 ) {
   const sender = envelope.senderIdentity;
   const groupPublicKey = envelope.source;
@@ -869,12 +897,13 @@ async function handleClosedGroupMemberLeft(
     leavingMembers: [sender],
   };
 
-  await ClosedGroup.addUpdateMessage(
+  await ClosedGroup.addUpdateMessage({
     convo,
-    groupDiff,
+    diff: groupDiff,
-    envelope.senderIdentity,
+    sender: envelope.senderIdentity,
-    toNumber(envelope.timestamp)
+    sentAt: toNumber(envelope.timestamp),
-  );
+    expireUpdate,
+  });
   convo.updateLastMessage();
   // if a user just left and we are the admin, we remove him right away for everyone by sending a MEMBERS_REMOVED message so no need to add him as a zombie
   if (oldMembers.includes(sender)) {

ts/receiver/contentMessage.ts

@@ -23,6 +23,7 @@ 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 { ProfileManager } from '../session/profile_manager/ProfileManager';
 import { GroupUtils, UserUtils } from '../session/utils';
 import { perfEnd, perfStart } from '../session/utils/Performance';
@@ -552,7 +553,8 @@ export async function innerHandleSwarmContentMessage({
 
       await handleDataExtractionNotification(
         envelope,
-        content.dataExtractionNotification as SignalService.DataExtractionNotification
+        content.dataExtractionNotification as SignalService.DataExtractionNotification,
+        content
       );
       perfEnd(
         `handleDataExtractionNotification-${envelope.id}`,
@@ -839,7 +841,8 @@ async function handleMessageRequestResponse(
  */
 export async function handleDataExtractionNotification(
   envelope: EnvelopePlus,
-  dataNotificationMessage: SignalService.DataExtractionNotification
+  dataNotificationMessage: SignalService.DataExtractionNotification,
+  content: SignalService.Content
 ): 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;
@@ -855,18 +858,21 @@ export async function handleDataExtractionNotification(
     return;
   }
 
-  if (!type || !source) {
+  if (!type || !source || !timestamp) {
     window?.log?.info('DataNotification pre check failed');
 
     return;
   }
 
-  if (timestamp) {
   const envelopeTimestamp = toNumber(timestamp);
   const referencedAttachmentTimestamp = toNumber(referencedAttachment);
+  const expireTimer = content.expirationTimer || 0;
 
-    const expirationMode = convo.getExpirationMode();
+  const expirationMode = DisappearingMessages.changeToDisappearingConversationMode(
-    const expireTimer = convo.getExpireTimer();
+    convo,
+    DisappearingMessageMode[content.expirationType],
+    expireTimer
+  );
   let expirationType;
   let expirationStartTimestamp;
 
@@ -896,12 +902,11 @@ export async function handleDataExtractionNotification(
       source,
     },
 
-      unread: READ_MESSAGE_STATE.unread, // 1 means unread
+    unread: READ_MESSAGE_STATE.unread,
     expirationType,
     expireTimer,
     expirationStartTimestamp,
   });
 
   convo.updateLastMessage();
-  }
 }

ts/receiver/dataMessage.ts

@@ -175,7 +175,8 @@ export async function handleSwarmDataMessage({
   if (cleanDataMessage.closedGroupControlMessage) {
     await handleClosedGroupControlMessage(
       envelope,
-      cleanDataMessage.closedGroupControlMessage as SignalService.DataMessage.ClosedGroupControlMessage
+      cleanDataMessage.closedGroupControlMessage as SignalService.DataMessage.ClosedGroupControlMessage,
+      expireUpdate || null
     );
     return;
   }

ts/session/group/closed-group.ts

@@ -6,6 +6,7 @@ import { Data } from '../../data/data';
 import { ConversationModel } from '../../models/conversation';
 import { ConversationAttributes, ConversationTypeEnum } from '../../models/conversationAttributes';
 import { MessageModel } from '../../models/message';
+import { MessageAttributesOptionals } from '../../models/messageType';
 import { SignalService } from '../../protobuf';
 import {
   addKeyPairToCacheAndDBIfNeeded,
@@ -18,7 +19,7 @@ import { getConversationController } from '../conversations';
 import { generateCurve25519KeyPairWithoutPrefix } from '../crypto';
 import { encryptUsingSessionProtocol } from '../crypto/MessageEncrypter';
 import { DisappearingMessages } from '../disappearing_messages';
-import { DisappearAfterSendOnly } from '../disappearing_messages/types';
+import { DisappearAfterSendOnly, DisappearingMessageUpdate } from '../disappearing_messages/types';
 import { ClosedGroupAddedMembersMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupAddedMembersMessage';
 import { ClosedGroupEncryptionPairMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairMessage';
 import { ClosedGroupNameChangeMessage } from '../messages/outgoing/controlMessage/group/ClosedGroupNameChangeMessage';
@@ -74,9 +75,11 @@ export async function initiateClosedGroupUpdate(
     convo.getExpireTimer(),
     convo.getExpirationMode()
   );
+  const expireTimer = convo.getExpireTimer();
 
   if (expirationType === 'deleteAfterRead') {
-    throw new Error(`Groups cannot be deleteAfterRead. convo id: ${convo.id}`);
+    window.log.warn(`Groups cannot be deleteAfterRead. convo id: ${convo.id}`);
+    throw new Error(`Groups cannot be deleteAfterRead`);
   }
 
   // do not give an admins field here. We don't want to be able to update admins and
@@ -89,7 +92,7 @@ export async function initiateClosedGroupUpdate(
     zombies: convo.get('zombies')?.filter(z => members.includes(z)),
     activeAt: Date.now(),
     expirationType,
-    expireTimer: convo.getExpireTimer(),
+    expireTimer,
   };
 
   const diff = buildGroupDiff(convo, groupDetails);
@@ -103,38 +106,44 @@ export async function initiateClosedGroupUpdate(
     admins: convo.get('groupAdmins'),
   };
 
+  const sharedDetails = {
+    sender: UserUtils.getOurPubKeyStrFromCache(),
+    sentAt: Date.now(),
+
+    expireUpdate: {
+      expirationType: groupDetails.expirationType || ('unknown' as const),
+      expirationTimer: expireTimer || 0,
+      messageExpirationFromRetrieve: GetNetworkTime.getNowWithNetworkOffset() + expireTimer * 1000,
+    },
+    convo,
+  };
+
   if (diff.newName?.length) {
     const nameOnlyDiff: GroupDiff = _.pick(diff, 'newName');
 
-    const dbMessageName = await addUpdateMessage(
+    const dbMessageName = await addUpdateMessage({
-      convo,
+      diff: nameOnlyDiff,
-      nameOnlyDiff,
+      ...sharedDetails,
-      UserUtils.getOurPubKeyStrFromCache(),
+    });
-      Date.now()
-    );
     await sendNewName(convo, diff.newName, dbMessageName.id as string);
   }
 
   if (diff.joiningMembers?.length) {
     const joiningOnlyDiff: GroupDiff = _.pick(diff, 'joiningMembers');
 
-    const dbMessageAdded = await addUpdateMessage(
+    const dbMessageAdded = await addUpdateMessage({
-      convo,
+      diff: joiningOnlyDiff,
-      joiningOnlyDiff,
+      ...sharedDetails,
-      UserUtils.getOurPubKeyStrFromCache(),
+    });
-      Date.now()
-    );
     await sendAddedMembers(convo, diff.joiningMembers, dbMessageAdded.id as string, updateObj);
   }
 
   if (diff.leavingMembers?.length) {
     const leavingOnlyDiff: GroupDiff = { kickedMembers: diff.leavingMembers };
-    const dbMessageLeaving = await addUpdateMessage(
+    const dbMessageLeaving = await addUpdateMessage({
-      convo,
+      diff: leavingOnlyDiff,
-      leavingOnlyDiff,
+      ...sharedDetails,
-      UserUtils.getOurPubKeyStrFromCache(),
+    });
-      Date.now()
-    );
     const stillMembers = members;
     await sendRemovedMembers(
       convo,
@@ -146,12 +155,19 @@ export async function initiateClosedGroupUpdate(
   await convo.commit();
 }
 
-export async function addUpdateMessage(
+export async function addUpdateMessage({
-  convo: ConversationModel,
+  convo,
-  diff: GroupDiff,
+  diff,
-  sender: string,
+  sender,
-  sentAt: number
+  sentAt,
-): Promise<MessageModel> {
+  expireUpdate,
+}: {
+  convo: ConversationModel;
+  diff: GroupDiff;
+  sender: string;
+  sentAt: number;
+  expireUpdate: DisappearingMessageUpdate | null;
+}): Promise<MessageModel> {
   const groupUpdate: any = {};
 
   if (diff.newName) {
@@ -170,49 +186,38 @@ export async function addUpdateMessage(
     groupUpdate.kicked = diff.kickedMembers;
   }
 
-  const expirationMode = convo.getExpirationMode();
+  const isUs = UserUtils.isUsFromCache(sender);
-  const expireTimer = convo.getExpireTimer();
+  const msgModel: MessageAttributesOptionals = {
-  let expirationType;
+    sent_at: sentAt,
-  let expirationStartTimestamp;
+    group_update: groupUpdate,
+    source: sender,
+    conversationId: convo.id,
+    type: isUs ? 'outgoing' : 'incoming',
+  };
+
+  if (convo && expireUpdate && expireUpdate.expirationType && expireUpdate.expirationTimer > 0) {
+    const { expirationTimer, expirationType, isLegacyDataMessage } = expireUpdate;
 
-  if (convo && expirationMode && expireTimer > 0) {
+    msgModel.expirationType = expirationType === 'deleteAfterSend' ? 'deleteAfterSend' : 'unknown';
-    expirationType =
+    msgModel.expireTimer = msgModel.expirationType === 'deleteAfterSend' ? expirationTimer : 0;
-      expirationMode !== 'off'
-        ? DisappearingMessages.changeToDisappearingMessageType(convo, expireTimer, expirationMode)
-        : undefined;
 
     // NOTE Triggers disappearing for an incoming groupUpdate message
     // TODO legacy messages support will be removed in a future release
-    if (expirationMode === 'legacy' || expirationMode === 'deleteAfterSend') {
+    if (isLegacyDataMessage || expirationType === 'deleteAfterSend') {
-      expirationStartTimestamp = DisappearingMessages.setExpirationStartTimestamp(
+      msgModel.expirationStartTimestamp = DisappearingMessages.setExpirationStartTimestamp(
-        expirationMode,
+        isLegacyDataMessage ? 'legacy' : expirationType === 'unknown' ? 'off' : expirationType,
         sentAt,
         'addUpdateMessage'
       );
     }
   }
 
-  const msgModel = {
+  return isUs
-    sent_at: sentAt,
+    ? convo.addSingleOutgoingMessage(msgModel)
-    group_update: groupUpdate,
+    : convo.addSingleIncomingMessage({
-    expirationType,
-    expireTimer,
-    expirationStartTimestamp,
-  };
-
-  if (UserUtils.isUsFromCache(sender)) {
-    const outgoingMessage = await convo.addSingleOutgoingMessage(msgModel);
-    return outgoingMessage;
-  }
-
-  const incomingMessage = await convo.addSingleIncomingMessage({
         ...msgModel,
         source: sender,
       });
-
-  await convo.commit();
-
-  return incomingMessage;
 }
 
 function buildGroupDiff(convo: ConversationModel, update: GroupInfo): GroupDiff {