From 12087da2be9c5a54b6a3fab849a8f05e4d2cfc3e Mon Sep 17 00:00:00 2001 From: William Grant Date: Mon, 3 Apr 2023 14:09:05 +0200 Subject: [PATCH] fix: repaired closed group disappearing messages --- ts/models/conversation.ts | 6 ++- ts/models/message.ts | 7 ++- ts/session/group/closed-group.ts | 52 +++++++++++++------ .../group/ClosedGroupMessage.ts | 19 +++---- .../ClosedGroupVisibleMessage.ts | 18 ++++--- .../visibleMessage/GroupInvitationMessage.ts | 12 ++--- .../messages/ClosedGroupChatMessage_test.ts | 20 +++++-- ts/test/session/unit/utils/Messages_test.ts | 6 ++- ts/test/test-utils/utils/message.ts | 6 ++- 9 files changed, 91 insertions(+), 55 deletions(-) diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index c5039f1dc..889297b1f 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -654,8 +654,9 @@ export class ConversationModel extends Backbone.Model { const closedGroupVisibleMessage = new ClosedGroupVisibleMessage({ chatMessage: chatMessageMediumGroup, groupId: destination, - expirationType, - expireTimer, + timestamp: sentAt, + expirationType: chatMessageParams.expirationType, + expireTimer: chatMessageParams.expireTimer, }); // we need the return await so that errors are caught in the catch {} @@ -760,6 +761,7 @@ export class ConversationModel extends Backbone.Model { const closedGroupVisibleMessage = new ClosedGroupVisibleMessage({ chatMessage: chatMessageMediumGroup, groupId: destination, + timestamp: sentAt, }); // we need the return await so that errors are caught in the catch {} await getMessageQueue().sendToGroup(closedGroupVisibleMessage); diff --git a/ts/models/message.ts b/ts/models/message.ts index 45d5b5201..b47ddcbdb 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -941,10 +941,12 @@ export class MessageModel extends Backbone.Model { ); } + const timestamp = Date.now(); // force a new timestamp to handle user fixed his clock; + const chatParams = { identifier: this.id, body, - timestamp: Date.now(), // force a new timestamp to handle user fixed his clock + timestamp, expireTimer: this.get('expireTimer'), attachments, preview: preview ? [preview] : [], @@ -978,8 +980,9 @@ export class MessageModel extends Backbone.Model { const closedGroupVisibleMessage = new ClosedGroupVisibleMessage({ identifier: this.id, - chatMessage, groupId: this.get('conversationId'), + timestamp, + chatMessage, }); return getMessageQueue().sendToGroup(closedGroupVisibleMessage); diff --git a/ts/session/group/closed-group.ts b/ts/session/group/closed-group.ts index 97818dacc..47bc4612e 100644 --- a/ts/session/group/closed-group.ts +++ b/ts/session/group/closed-group.ts @@ -29,7 +29,10 @@ import { ClosedGroupRemovedMembersMessage } from '../messages/outgoing/controlMe import { getSwarmPollingInstance } from '../apis/snode_api'; import { getNowWithNetworkOffset } from '../apis/snode_api/SNodeAPI'; import { ConversationAttributes, ConversationTypeEnum } from '../../models/conversationAttributes'; -import { DisappearingMessageConversationType } from '../../util/expiringMessages'; +import { + DisappearingMessageConversationType, + setExpirationStartTimestamp, +} from '../../util/expiringMessages'; export type GroupInfo = { id: string; @@ -87,8 +90,8 @@ export async function initiateClosedGroupUpdate( // remove from the zombies list the zombies not which are not in the group anymore zombies: convo.get('zombies')?.filter(z => members.includes(z)), activeAt: Date.now(), - expirationType: convo.get('expirationType'), // TODO Does this have a default value - expireTimer: convo.get('expireTimer'), + expirationType: convo.get('expirationType') || undefined, + expireTimer: convo.get('expireTimer') || 0, }; const diff = buildGroupDiff(convo, groupDetails); @@ -100,8 +103,6 @@ export async function initiateClosedGroupUpdate( name: groupName, members, admins: convo.get('groupAdmins'), - expirationType: convo.get('expirationType'), - expireTimer: convo.get('expireTimer'), }; if (diff.newName?.length) { @@ -171,25 +172,36 @@ export async function addUpdateMessage( groupUpdate.kicked = diff.kickedMembers; } + const expirationType = convo.get('expirationType'); + const expireTimer = convo.get('expireTimer'); + + const msgModel = { + sent_at: sentAt, + group_update: groupUpdate, + expirationType: expirationType || undefined, + expireTimer: expireTimer || 0, + expirationStartTimestamp: + expirationType === 'deleteAfterSend' + ? setExpirationStartTimestamp(expirationType, sentAt) + : undefined, + }; + if (UserUtils.isUsFromCache(sender)) { - const outgoingMessage = await convo.addSingleOutgoingMessage({ - sent_at: sentAt, - group_update: groupUpdate, - expireTimer: 0, - }); + const outgoingMessage = await convo.addSingleOutgoingMessage(msgModel); return outgoingMessage; } + const incomingMessage = await convo.addSingleIncomingMessage({ - sent_at: sentAt, - group_update: groupUpdate, - expireTimer: 0, + ...msgModel, source: sender, }); + // update the unreadCount for this convo const unreadCount = await convo.getUnreadCount(); convo.set({ unreadCount, }); + await convo.commit(); return incomingMessage; } @@ -238,6 +250,8 @@ export async function updateOrCreateClosedGroup(details: GroupInfo) { | 'left' | 'lastJoinedTimestamp' | 'zombies' + | 'expirationType' + | 'expireTimer' > = { displayNameInProfile: details.name, members: details.members, @@ -247,6 +261,8 @@ export async function updateOrCreateClosedGroup(details: GroupInfo) { active_at: details.activeAt ? details.activeAt : 0, left: details.activeAt ? false : true, lastJoinedTimestamp: details.activeAt && weWereJustAdded ? Date.now() : details.activeAt || 0, + expirationType: details.expirationType || 'off', + expireTimer: details.expireTimer || 0, }; conversation.set(updates); @@ -264,12 +280,18 @@ export async function updateOrCreateClosedGroup(details: GroupInfo) { const { expirationType, expireTimer } = details; - if (expireTimer === undefined || typeof expireTimer !== 'number') { + if ( + expirationType === undefined || + expirationType === 'off' || + expireTimer === undefined || + typeof expireTimer !== 'number' + ) { return; } await conversation.updateExpireTimer({ - // TODO clean up 2 weeks after release + // TODO clean up 2 weeks after release? + // TODO What are we cleaning? providedExpirationType: expirationType || 'deleteAfterSend', providedExpireTimer: expireTimer, providedChangeTimestamp: getNowWithNetworkOffset(), diff --git a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts index 408392cb5..3210b0afb 100644 --- a/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/group/ClosedGroupMessage.ts @@ -1,32 +1,26 @@ import { SignalService } from '../../../../../protobuf'; -import { DisappearingMessageType } from '../../../../../util/expiringMessages'; import { PubKey } from '../../../../types'; -import { ContentMessage } from '../../ContentMessage'; -import { MessageParams } from '../../Message'; +import { ExpirableMessage, ExpirableMessageParams } from '../../ExpirableMessage'; -export interface ClosedGroupMessageParams extends MessageParams { +export interface ClosedGroupMessageParams extends ExpirableMessageParams { groupId: string | PubKey; - expirationType?: DisappearingMessageType; - expireTimer?: number; } -export abstract class ClosedGroupMessage extends ContentMessage { +export abstract class ClosedGroupMessage extends ExpirableMessage { public readonly groupId: PubKey; - public readonly expirationType?: DisappearingMessageType; - public readonly expireTimer?: number; constructor(params: ClosedGroupMessageParams) { super({ timestamp: params.timestamp, identifier: params.identifier, + expirationType: params.expirationType, + expireTimer: params.expireTimer, }); this.groupId = PubKey.cast(params.groupId); if (!this.groupId || this.groupId.key.length === 0) { throw new Error('groupId must be set'); } - this.expirationType = params.expirationType; - this.expireTimer = params.expireTimer; } public static areAdminsMembers(admins: Array, members: Array) { @@ -36,11 +30,12 @@ export abstract class ClosedGroupMessage extends ContentMessage { public contentProto(): SignalService.Content { return new SignalService.Content({ dataMessage: this.dataProto(), + ...super.contentProto(), + // Closed Groups only support 'deleteAfterSend' expirationType: this.expirationType === 'deleteAfterSend' ? SignalService.Content.ExpirationType.DELETE_AFTER_SEND : undefined, - expirationTimer: this.expireTimer, }); } diff --git a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts index 61fc7a03c..d40853112 100644 --- a/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/ClosedGroupVisibleMessage.ts @@ -2,14 +2,14 @@ import { SignalService } from '../../../../protobuf'; import { PubKey } from '../../../types'; import { StringUtils } from '../../../utils'; import { VisibleMessage } from './VisibleMessage'; -import { ClosedGroupMessage } from '../controlMessage/group/ClosedGroupMessage'; -import { DisappearingMessageType } from '../../../../util/expiringMessages'; - -interface ClosedGroupVisibleMessageParams { - identifier?: string; - groupId: string | PubKey; - expirationType?: DisappearingMessageType; - expireTimer?: number; +import { + ClosedGroupMessage, + ClosedGroupMessageParams, +} from '../controlMessage/group/ClosedGroupMessage'; + +interface ClosedGroupVisibleMessageParams extends ClosedGroupMessageParams { + // TODO Do we need strings? + // groupId: string | PubKey; chatMessage: VisibleMessage; } @@ -24,7 +24,9 @@ export class ClosedGroupVisibleMessage extends ClosedGroupMessage { expirationType: params.expirationType, expireTimer: params.expireTimer, }); + this.chatMessage = params.chatMessage; + if (!params.groupId) { throw new Error('ClosedGroupVisibleMessage: groupId must be set'); } diff --git a/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts b/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts index 9f31f87e2..de47d3cbf 100644 --- a/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/GroupInvitationMessage.ts @@ -1,15 +1,9 @@ import { SignalService } from '../../../../protobuf'; -import { DisappearingMessageType } from '../../../../util/expiringMessages'; -import { MessageParams } from '../Message'; -import { VisibleMessage } from './VisibleMessage'; +import { VisibleMessage, VisibleMessageParams } from './VisibleMessage'; -interface GroupInvitationMessageParams extends MessageParams { +interface GroupInvitationMessageParams extends VisibleMessageParams { url: string; name: string; - // if disappearing messages is set for the conversation, we need to set it. - // otherwise, it will disable the expire timer on the receiving side. - expirationType?: DisappearingMessageType; - expireTimer?: number; } export class GroupInvitationMessage extends VisibleMessage { @@ -34,8 +28,8 @@ export class GroupInvitationMessage extends VisibleMessage { }); return new SignalService.DataMessage({ + ...super.dataProto(), openGroupInvitation, - expireTimer: this.expireTimer, }); } } diff --git a/ts/test/session/unit/messages/ClosedGroupChatMessage_test.ts b/ts/test/session/unit/messages/ClosedGroupChatMessage_test.ts index a244c6674..973ef1631 100644 --- a/ts/test/session/unit/messages/ClosedGroupChatMessage_test.ts +++ b/ts/test/session/unit/messages/ClosedGroupChatMessage_test.ts @@ -14,12 +14,14 @@ describe('ClosedGroupVisibleMessage', () => { groupId = TestUtils.generateFakePubKey(); }); it('can create empty message with timestamp, groupId and chatMessage', () => { + const timestamp = Date.now(); const chatMessage = new VisibleMessage({ - timestamp: Date.now(), + timestamp, body: 'body', }); const message = new ClosedGroupVisibleMessage({ groupId, + timestamp, chatMessage, }); const plainText = message.plainTextBuffer(); @@ -43,22 +45,26 @@ describe('ClosedGroupVisibleMessage', () => { }); it('correct ttl', () => { + const timestamp = Date.now(); const chatMessage = new VisibleMessage({ - timestamp: Date.now(), + timestamp, }); const message = new ClosedGroupVisibleMessage({ groupId, + timestamp, chatMessage, }); expect(message.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX); }); it('has an identifier', () => { + const timestamp = Date.now(); const chatMessage = new VisibleMessage({ - timestamp: Date.now(), + timestamp, }); const message = new ClosedGroupVisibleMessage({ groupId, + timestamp, chatMessage, }); expect(message.identifier).to.not.equal(null, 'identifier cannot be null'); @@ -66,13 +72,15 @@ describe('ClosedGroupVisibleMessage', () => { }); it('should use the identifier passed into it over the one set in chatMessage', () => { + const timestamp = Date.now(); const chatMessage = new VisibleMessage({ - timestamp: Date.now(), + timestamp, body: 'body', identifier: 'chatMessage', }); const message = new ClosedGroupVisibleMessage({ groupId, + timestamp, chatMessage, identifier: 'closedGroupMessage', }); @@ -80,13 +88,15 @@ describe('ClosedGroupVisibleMessage', () => { }); it('should use the identifier of the chatMessage if one is not specified on the closed group message', () => { + const timestamp = Date.now(); const chatMessage = new VisibleMessage({ - timestamp: Date.now(), + timestamp, body: 'body', identifier: 'chatMessage', }); const message = new ClosedGroupVisibleMessage({ groupId, + timestamp, chatMessage, }); expect(message.identifier).to.be.equal('chatMessage'); diff --git a/ts/test/session/unit/utils/Messages_test.ts b/ts/test/session/unit/utils/Messages_test.ts index 1362158a5..c508d6735 100644 --- a/ts/test/session/unit/utils/Messages_test.ts +++ b/ts/test/session/unit/utils/Messages_test.ts @@ -96,7 +96,11 @@ describe('Message Utils', () => { const device = TestUtils.generateFakePubKey(); const groupId = TestUtils.generateFakePubKey(); const chatMessage = TestUtils.generateVisibleMessage(); - const message = new ClosedGroupVisibleMessage({ chatMessage, groupId }); + const message = new ClosedGroupVisibleMessage({ + groupId, + timestamp: Date.now(), + chatMessage, + }); const rawMessage = await MessageUtils.toRawMessage(device, message); expect(rawMessage.encryption).to.equal(SignalService.Envelope.Type.CLOSED_GROUP_MESSAGE); diff --git a/ts/test/test-utils/utils/message.ts b/ts/test/test-utils/utils/message.ts index 1f511a8fc..299a34fef 100644 --- a/ts/test/test-utils/utils/message.ts +++ b/ts/test/test-utils/utils/message.ts @@ -68,10 +68,14 @@ export function generateOpenGroupV2RoomInfos(): OpenGroupRequestCommonType { return { roomId: 'main', serverUrl: 'http://open.getsession.org' }; } -export function generateClosedGroupMessage(groupId?: string): ClosedGroupVisibleMessage { +export function generateClosedGroupMessage( + groupId?: string, + timestamp?: number +): ClosedGroupVisibleMessage { return new ClosedGroupVisibleMessage({ identifier: uuid(), groupId: groupId ?? generateFakePubKey().key, + timestamp: timestamp || Date.now(), chatMessage: generateVisibleMessage(), }); }