Merge pull request #1211 from Mikunj/closed-group-fixes

Closed group fixes

js/background.js

@@ -639,13 +639,15 @@
         confirm: () => {},
       };
 
-      await window.NewReceiver.onGroupReceived(ev);
-
       const convo = await ConversationController.getOrCreateAndWait(
         groupId,
         'group'
       );
 
+      const recipients = _.union(convo.get('members'), members);
+
+      await window.NewReceiver.onGroupReceived(ev);
+
       if (convo.isPublic()) {
         const API = await convo.getPublicSendData();
 
@@ -703,8 +705,6 @@
       }
       const options = {};
 
-      const recipients = _.union(convo.get('members'), members);
-
       const isMediumGroup = convo.isMediumGroup();
 
       const updateObj = {

js/models/conversations.js

@@ -1239,12 +1239,12 @@
           recipients,
         });
 
-        if (this.isPrivate()) {
-          messageWithSchema.destination = destination;
-        } else if (this.isPublic()) {
+        if (this.isPublic()) {
           // Public chats require this data to detect duplicates
           messageWithSchema.source = textsecure.storage.user.getNumber();
           messageWithSchema.sourceDevice = 1;
+        } else {
+          messageWithSchema.destination = destination;
         }
 
         const { sessionRestoration = false } = otherOptions;
@@ -1368,6 +1368,7 @@
           }
 
           if (conversationType === Message.GROUP) {
+            const members = this.get('members');
             if (this.isMediumGroup()) {
               const mediumGroupChatMessage = new libsession.Messages.Outgoing.MediumGroupChatMessage(
                 {
@@ -1375,7 +1376,6 @@
                   groupId: destination,
                 }
               );
-              const members = this.get('members');
               await Promise.all(
                 members.map(async m => {
                   const memberPubKey = new libsession.Types.PubKey(m);
@@ -1391,6 +1391,18 @@
                   groupId: destination,
                 }
               );
+
+              // Special-case the self-send case - we send only a sync message
+              if (members.length === 1) {
+                const isOurDevice = await libsession.Protocols.MultiDeviceProtocol.isOurDevice(
+                  members[0]
+                );
+                if (isOurDevice) {
+                  await message.sendSyncMessageOnly(closedGroupChatMessage);
+                  return true;
+                }
+              }
+
               await libsession
                 .getMessageQueue()
                 .sendToGroup(closedGroupChatMessage);
@@ -1815,9 +1827,12 @@
       );
       message.set({ id: messageId });
 
+      // Difference between `recipients` and `members` is that `recipients` includes the members which were removed in this update
+      const { id, name, members, avatar, recipients } = groupUpdate;
+
       if (groupUpdate.is_medium_group) {
+        const { secretKey, senderKey } = groupUpdate;
         // Constructing a "create group" message
-        const { id, name, secretKey, senderKey, members } = groupUpdate;
         const { chainKey, keyIdx } = senderKey;
 
         const createParams = {
@@ -1850,16 +1865,17 @@
       const updateParams = {
         // if we do set an identifier here, be sure to not sync the message two times in msg.handleMessageSentSuccess()
         timestamp: now,
-        groupId: this.id,
-        name: this.get('name'),
-        avatar: this.get('avatar'),
-        members: this.get('members'),
+        groupId: id,
+        name,
+        avatar,
+        members,
         admins: this.get('groupAdmins'),
       };
       const groupUpdateMessage = new libsession.Messages.Outgoing.ClosedGroupUpdateMessage(
         updateParams
       );
-      await this.sendClosedGroupMessageWithSync(groupUpdateMessage);
+
+      await this.sendClosedGroupMessageWithSync(groupUpdateMessage, recipients);
     },
 
     sendGroupInfo(recipient) {
@@ -1935,7 +1951,7 @@
       }
     },
 
-    async sendClosedGroupMessageWithSync(message) {
+    async sendClosedGroupMessageWithSync(message, recipients) {
       const {
         ClosedGroupMessage,
         ClosedGroupChatMessage,
@@ -1951,19 +1967,34 @@
         );
       }
 
-      try {
-        await libsession.getMessageQueue().sendToGroup(message);
+      const members = recipients || this.get('members');
 
-        const syncMessage = libsession.Utils.SyncMessageUtils.getSentSyncMessage(
-          {
-            destination: message.groupId,
-            message,
-          }
+      try {
+        // Exclude our device from members and send them the message
+        const ourNumber = textsecure.storage.user.getNumber();
+        const primary = await libsession.Protocols.MultiDeviceProtocol.getPrimaryDevice(
+          ourNumber
         );
+        const otherMembers = (members || []).filter(
+          member => !primary.isEqual(member)
+        );
+        const sendPromises = otherMembers.map(member => {
+          const memberPubKey = libsession.Types.PubKey.cast(member);
+          return libsession
+            .getMessageQueue()
+            .sendUsingMultiDevice(memberPubKey, message);
+        });
+        await Promise.all(sendPromises);
 
-        if (syncMessage) {
-          await libsession.getMessageQueue().sendSyncMessage(syncMessage);
-        }
+        // Send the sync message to our devices
+        const syncMessage = new libsession.Messages.Outgoing.SentSyncMessage({
+          timestamp: Date.now(),
+          identifier: message.identifier,
+          destination: message.groupId,
+          dataMessage: message.dataProto(),
+        });
+
+        await libsession.getMessageQueue().sendSyncMessage(syncMessage);
       } catch (e) {
         window.log.error(e);
       }

js/models/messages.js

@@ -1100,8 +1100,13 @@
         });
 
         // Special-case the self-send case - we send only a sync message
-        if (recipients.length === 1 && recipients[0] === this.OUR_NUMBER) {
-          return this.sendSyncMessageOnly(chatMessage);
+        if (recipients.length === 1) {
+          const isOurDevice = await libsession.Protocols.MultiDeviceProtocol.isOurDevice(
+            recipients[0]
+          );
+          if (isOurDevice) {
+            return this.sendSyncMessageOnly(chatMessage);
+          }
         }
 
         if (conversation.isPrivate()) {
@@ -1433,10 +1438,15 @@
         return;
       }
 
+      const data =
+        dataMessage instanceof libsession.Messages.Outgoing.DataMessage
+          ? dataMessage.dataProto()
+          : dataMessage;
+
       const syncMessage = new libsession.Messages.Outgoing.SentSyncMessage({
         timestamp: this.get('sent_at'),
         identifier: this.id,
-        dataMessage,
+        dataMessage: data,
         destination: this.get('destination'),
         expirationStartTimestamp: this.get('expirationStartTimestamp'),
         sent_to: this.get('sent_to'),

js/views/inbox_view.js

@@ -297,6 +297,11 @@
       const msg = conv.messageCollection.models.find(
         convMsg => convMsg.id === tmpMsg.id
       );
+
+      if (!msg) {
+        return null;
+      }
+
       return { msg };
     },
 

ts/components/session/SessionGroupSettings.tsx

@@ -222,7 +222,7 @@ export class SessionGroupSettings extends React.Component<Props, any> {
     const leaveGroupString = isPublic
       ? window.i18n('leaveOpenGroup')
       : isKickedFromGroup
-      ? window.i18n('youAreKickedFromThisGroup')
+      ? window.i18n('youGotKickedFromThisGroup')
       : window.i18n('leaveClosedGroup');
 
     const disappearingMessagesOptions = timerOptions.map(option => {

ts/receiver/dataMessage.ts

@@ -536,7 +536,7 @@ export async function handleMessageEvent(event: any): Promise<void> {
 
   let { source } = data;
 
-  const isGroupMessage = message.group;
+  const isGroupMessage = Boolean(message.group);
 
   const type = isGroupMessage
     ? ConversationType.GROUP

ts/session/messages/outgoing/content/data/group/ClosedGroupUpdateMessage.ts

@@ -1,12 +1,9 @@
-import { DataMessage } from '../DataMessage';
 import { SignalService } from '../../../../../../protobuf';
-import { StringUtils } from '../../../../../utils';
 import {
   ClosedGroupMessage,
   ClosedGroupMessageParams,
 } from './ClosedGroupMessage';
 import { AttachmentPointer } from '../ChatMessage';
-import { PubKey } from '../../../../../types';
 
 export interface ClosedGroupUpdateMessageParams
   extends ClosedGroupMessageParams {

ts/session/messages/outgoing/content/sync/SentSyncMessage.ts

@@ -2,6 +2,7 @@ import { SyncMessage } from './SyncMessage';
 import { SignalService } from '../../../../../protobuf';
 import { MessageParams } from '../../Message';
 import { PubKey } from '../../../../types';
+import { ProtobufUtils } from '../../../../utils';
 
 interface SentSyncMessageParams extends MessageParams {
   dataMessage: SignalService.IDataMessage;
@@ -21,7 +22,7 @@ export class SentSyncMessage extends SyncMessage {
   constructor(params: SentSyncMessageParams) {
     super({ timestamp: params.timestamp, identifier: params.identifier });
 
-    this.dataMessage = params.dataMessage;
+    this.dataMessage = ProtobufUtils.convertToTS(params.dataMessage);
     this.expirationStartTimestamp = params.expirationStartTimestamp;
     this.sentTo = params.sentTo;
     this.unidentifiedDeliveries = params.unidentifiedDeliveries;

ts/session/utils/Protobuf.ts

@@ -0,0 +1,37 @@
+import ByteBuffer from 'bytebuffer';
+
+/**
+ * Converts any object to a valid ts protobuf object.
+ *
+ * This is needed because there's a very jarring difference between `protobufjs` and `protobufts`.
+ * `protobufjs` returns all `bytes` as `ByteBuffer` where as `protobufts` returns all `bytes` as `Uint8Array`.
+ */
+export function convertToTS(object: any): any {
+  // No idea why js `ByteBuffer` and ts `ByteBuffer` differ ...
+  if (
+    object &&
+    object.constructor &&
+    object.constructor.name === 'ByteBuffer'
+  ) {
+    return new Uint8Array(object.toArrayBuffer());
+  } else if (
+    object instanceof ByteBuffer ||
+    object instanceof Buffer ||
+    object instanceof ArrayBuffer ||
+    object instanceof SharedArrayBuffer
+  ) {
+    const arrayBuffer = ByteBuffer.wrap(object).toArrayBuffer();
+    return new Uint8Array(arrayBuffer);
+  } else if (Array.isArray(object)) {
+    return object.map(convertToTS);
+  } else if (object && typeof object === 'object') {
+    const keys = Object.keys(object);
+    const values: { [key: string]: any } = {};
+    for (const key of keys) {
+      values[key] = convertToTS(object[key]);
+    }
+    return values;
+  }
+
+  return object;
+}

ts/session/utils/index.ts

@@ -3,6 +3,7 @@ import * as GroupUtils from './Groups';
 import * as SyncMessageUtils from './SyncMessage';
 import * as StringUtils from './String';
 import * as PromiseUtils from './Promise';
+import * as ProtobufUtils from './Protobuf';
 
 export * from './Attachments';
 export * from './TypedEmitter';
@@ -14,4 +15,5 @@ export {
   GroupUtils,
   StringUtils,
   PromiseUtils,
+  ProtobufUtils,
 };

ts/test/session/utils/Promise_test.ts

@@ -69,8 +69,8 @@ describe('Promise Utils', () => {
 
     it('will recur according to interval option', async () => {
       const expectedRecurrences = 4;
-      const timeout = 1000;
-      const interval = timeout / expectedRecurrences;
+      const timeout = 3000;
+      const interval = 50;
 
       const recurrenceSpy = sandbox.spy();
       const task = (done: any) => {