diff --git a/js/models/conversations.js b/js/models/conversations.js index 5b0592222..ec29bd28f 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -417,13 +417,13 @@ libsession .getMessageQueue() .sendUsingMultiDevice(device, typingMessage) - .ignore(); + .catch(log.error); } else { // the recipients on the case of a group are found by the messageQueue using message.groupId libsession .getMessageQueue() .sendToGroup(typingMessage) - .ignore(); + .catch(log.error); } }, @@ -1925,7 +1925,7 @@ libsession .getMessageQueue() .sendToGroup(groupUpdateMessage) - .ignore(); + .catch(log.error); }, sendGroupInfo(recipient) { @@ -1949,7 +1949,7 @@ libsession .getMessageQueue() .send(recipientPubKey, groupUpdateMessage) - .ignore(); + .catch(log.error); } }, diff --git a/test/models/conversations_test.js b/test/models/conversations_test.js index 97d0ed1bd..b4aadef27 100644 --- a/test/models/conversations_test.js +++ b/test/models/conversations_test.js @@ -96,7 +96,7 @@ describe('Conversation', () => { it('adds conversation to message collection upon leaving group', async () => { const convo = new Whisper.ConversationCollection().add({ type: 'group', - id: 'a random string', + id: '052d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab', }); await convo.leaveGroup(); assert.notEqual(convo.messageCollection.length, 0); diff --git a/ts/session/messages/outgoing/content/TypingMessage.ts b/ts/session/messages/outgoing/content/TypingMessage.ts index 371d0739f..3c1f247bd 100644 --- a/ts/session/messages/outgoing/content/TypingMessage.ts +++ b/ts/session/messages/outgoing/content/TypingMessage.ts @@ -3,23 +3,26 @@ import { SignalService } from '../../../../protobuf'; import { TextEncoder } from 'util'; import { MessageParams } from '../Message'; import { StringUtils } from '../../../utils'; +import { PubKey } from '../../../types'; interface TypingMessageParams extends MessageParams { isTyping: boolean; typingTimestamp?: number; - groupId?: string; + groupId?: string | PubKey; } export class TypingMessage extends ContentMessage { - private readonly isTyping: boolean; - private readonly typingTimestamp?: number; - private readonly groupId?: string; + public readonly isTyping: boolean; + public readonly typingTimestamp?: number; + public readonly groupId?: PubKey; constructor(params: TypingMessageParams) { super({ timestamp: params.timestamp, identifier: params.identifier }); this.isTyping = params.isTyping; this.typingTimestamp = params.typingTimestamp; - this.groupId = params.groupId; + + const { groupId } = params; + this.groupId = groupId ? PubKey.cast(groupId) : undefined; } public ttl(): number { @@ -41,7 +44,7 @@ export class TypingMessage extends ContentMessage { const typingMessage = new SignalService.TypingMessage(); if (this.groupId) { typingMessage.groupId = new Uint8Array( - StringUtils.encode(this.groupId, 'utf8') + StringUtils.encode(this.groupId.key, 'utf8') ); } typingMessage.action = action; diff --git a/ts/session/messages/outgoing/content/data/ExpirationTimerUpdateMessage.ts b/ts/session/messages/outgoing/content/data/ExpirationTimerUpdateMessage.ts index 7b2044487..6329ec32b 100644 --- a/ts/session/messages/outgoing/content/data/ExpirationTimerUpdateMessage.ts +++ b/ts/session/messages/outgoing/content/data/ExpirationTimerUpdateMessage.ts @@ -3,23 +3,26 @@ import { SignalService } from '../../../../../protobuf'; import { MessageParams } from '../../Message'; import { StringUtils } from '../../../../utils'; import { DataMessage } from './DataMessage'; +import { PubKey } from '../../../../types'; interface ExpirationTimerUpdateMessageParams extends MessageParams { - groupId?: string; + groupId?: string | PubKey; expireTimer: number | null; profileKey?: Uint8Array; } export class ExpirationTimerUpdateMessage extends DataMessage { - private readonly groupId?: string; - private readonly expireTimer: number | null; - private readonly profileKey?: Uint8Array; + public readonly groupId?: PubKey; + public readonly expireTimer: number | null; + public readonly profileKey?: Uint8Array; constructor(params: ExpirationTimerUpdateMessageParams) { super({ timestamp: params.timestamp, identifier: params.identifier }); - this.groupId = params.groupId; this.expireTimer = params.expireTimer; this.profileKey = params.profileKey; + + const { groupId } = params; + this.groupId = groupId ? PubKey.cast(groupId) : undefined; } public ttl(): number { @@ -32,7 +35,7 @@ export class ExpirationTimerUpdateMessage extends DataMessage { const groupMessage = new SignalService.GroupContext(); if (this.groupId) { groupMessage.id = new Uint8Array( - StringUtils.encode(this.groupId, 'utf8') + StringUtils.encode(this.groupId.key, 'utf8') ); groupMessage.type = SignalService.GroupContext.Type.DELIVER; } diff --git a/ts/session/messages/outgoing/content/data/group/ClosedGroupMessage.ts b/ts/session/messages/outgoing/content/data/group/ClosedGroupMessage.ts index 5fe9b561c..91bb9a874 100644 --- a/ts/session/messages/outgoing/content/data/group/ClosedGroupMessage.ts +++ b/ts/session/messages/outgoing/content/data/group/ClosedGroupMessage.ts @@ -16,8 +16,7 @@ export abstract class ClosedGroupMessage extends DataMessage { timestamp: params.timestamp, identifier: params.identifier, }); - const { groupId } = params; - this.groupId = typeof groupId === 'string' ? new PubKey(groupId) : groupId; + this.groupId = PubKey.cast(params.groupId); } public ttl(): number { diff --git a/ts/session/messages/outgoing/content/data/index.ts b/ts/session/messages/outgoing/content/data/index.ts index 76abca7ee..a8022724b 100644 --- a/ts/session/messages/outgoing/content/data/index.ts +++ b/ts/session/messages/outgoing/content/data/index.ts @@ -3,3 +3,4 @@ export * from './DeviceUnlinkMessage'; export * from './GroupInvitationMessage'; export * from './ChatMessage'; export * from './group'; +export * from './ExpirationTimerUpdateMessage'; diff --git a/ts/session/protocols/MultiDeviceProtocol.ts b/ts/session/protocols/MultiDeviceProtocol.ts index 45c5b7b08..140e394b8 100644 --- a/ts/session/protocols/MultiDeviceProtocol.ts +++ b/ts/session/protocols/MultiDeviceProtocol.ts @@ -140,7 +140,7 @@ export class MultiDeviceProtocol { public static async getPairingAuthorisations( device: PubKey | string ): Promise> { - const pubKey = typeof device === 'string' ? new PubKey(device) : device; + const pubKey = PubKey.cast(device); await this.fetchPairingAuthorisationsIfNeeded(pubKey); return getPairingAuthorisationsFor(pubKey.key); @@ -153,7 +153,7 @@ export class MultiDeviceProtocol { public static async removePairingAuthorisations( device: PubKey | string ): Promise { - const pubKey = typeof device === 'string' ? new PubKey(device) : device; + const pubKey = PubKey.cast(device); return removePairingAuthorisationsFor(pubKey.key); } @@ -166,7 +166,7 @@ export class MultiDeviceProtocol { public static async getAllDevices( user: PubKey | string ): Promise> { - const pubKey = typeof user === 'string' ? new PubKey(user) : user; + const pubKey = PubKey.cast(user); const authorisations = await this.getPairingAuthorisations(pubKey); if (authorisations.length === 0) { return [pubKey]; @@ -190,7 +190,7 @@ export class MultiDeviceProtocol { public static async getPrimaryDevice( user: PubKey | string ): Promise { - const pubKey = typeof user === 'string' ? new PubKey(user) : user; + const pubKey = PubKey.cast(user); const authorisations = await this.getPairingAuthorisations(pubKey); if (authorisations.length === 0) { return pubKey; @@ -237,7 +237,7 @@ export class MultiDeviceProtocol { * @param device The device to check. */ public static async isOurDevice(device: PubKey | string): Promise { - const pubKey = typeof device === 'string' ? new PubKey(device) : device; + const pubKey = PubKey.cast(device); try { const ourDevices = await this.getOurDevices(); diff --git a/ts/session/sending/MessageQueue.ts b/ts/session/sending/MessageQueue.ts index 60e812104..b675b857a 100644 --- a/ts/session/sending/MessageQueue.ts +++ b/ts/session/sending/MessageQueue.ts @@ -6,9 +6,11 @@ import { import { ClosedGroupMessage, ContentMessage, + ExpirationTimerUpdateMessage, OpenGroupMessage, SessionRequestMessage, SyncMessage, + TypingMessage, } from '../messages/outgoing'; import { PendingMessageCache } from './PendingMessageCache'; import { @@ -33,13 +35,16 @@ export class MessageQueue implements MessageQueueInterface { void this.processAllPending(); } - public async sendUsingMultiDevice(user: PubKey, message: ContentMessage) { + public async sendUsingMultiDevice( + user: PubKey, + message: ContentMessage + ): Promise { const userDevices = await MultiDeviceProtocol.getAllDevices(user.key); await this.sendMessageToDevices(userDevices, message); } - public async send(device: PubKey, message: ContentMessage) { + public async send(device: PubKey, message: ContentMessage): Promise { await this.sendMessageToDevices([device], message); } @@ -75,26 +80,8 @@ export class MessageQueue implements MessageQueueInterface { } public async sendToGroup( - message: OpenGroupMessage | ClosedGroupMessage - ): Promise { - // Closed groups - if (message instanceof ClosedGroupMessage) { - // Get devices in closed group - const recipients = await GroupUtils.getGroupMembers(message.groupId); - if (recipients.length === 0) { - return false; - } - - // Send to all devices of members - await Promise.all( - recipients.map(async recipient => - this.sendUsingMultiDevice(recipient, message) - ) - ); - - return true; - } - + message: OpenGroupMessage | ContentMessage + ): Promise { // Open groups if (message instanceof OpenGroupMessage) { // No queue needed for Open Groups; send directly @@ -108,20 +95,42 @@ export class MessageQueue implements MessageQueueInterface { } else { this.events.emit('fail', message, error); } - - return result; } catch (e) { console.warn( `Failed to send message to open group: ${message.group.server}`, e ); this.events.emit('fail', message, error); - - return false; } + + return; + } + + let groupId: PubKey | undefined; + if (message instanceof ClosedGroupMessage) { + groupId = message.groupId; + } else if (message instanceof TypingMessage) { + groupId = message.groupId; + } else if (message instanceof ExpirationTimerUpdateMessage) { + groupId = message.groupId; } - return false; + if (!groupId) { + throw new Error('Invalid group message passed in sendToGroup.'); + } + + // Get devices in group + const recipients = await GroupUtils.getGroupMembers(groupId); + if (recipients.length === 0) { + return; + } + + // Send to all devices of members + await Promise.all( + recipients.map(async recipient => + this.sendUsingMultiDevice(recipient, message) + ) + ); } public async sendSyncMessage(message: SyncMessage | undefined): Promise { diff --git a/ts/session/sending/MessageQueueInterface.ts b/ts/session/sending/MessageQueueInterface.ts index 5bed428ca..2cd58354e 100644 --- a/ts/session/sending/MessageQueueInterface.ts +++ b/ts/session/sending/MessageQueueInterface.ts @@ -17,8 +17,8 @@ export interface MessageQueueInterfaceEvents { export interface MessageQueueInterface { events: TypedEventEmitter; - sendUsingMultiDevice(user: PubKey, message: ContentMessage): void; - send(device: PubKey, message: ContentMessage): void; - sendToGroup(message: GroupMessageType): void; + sendUsingMultiDevice(user: PubKey, message: ContentMessage): Promise; + send(device: PubKey, message: ContentMessage): Promise; + sendToGroup(message: GroupMessageType): Promise; sendSyncMessage(message: SyncMessage | undefined): Promise; } diff --git a/ts/session/types/PubKey.ts b/ts/session/types/PubKey.ts index a6c0a2077..0d83ec958 100644 --- a/ts/session/types/PubKey.ts +++ b/ts/session/types/PubKey.ts @@ -5,11 +5,35 @@ export class PubKey { ); public readonly key: string; + /** + * A PubKey object. + * If `pubKeyString` is not valid then this will throw an `Error`. + * + * @param pubkeyString The public key string. + */ constructor(pubkeyString: string) { - PubKey.validate(pubkeyString); + if (!PubKey.validate(pubkeyString)) { + throw new Error(`Invalid pubkey string passed: ${pubkeyString}`); + } this.key = pubkeyString; } + /** + * Cast a `value` to a `PubKey`. + * If `value` is not valid then this will throw. + * + * @param value The value to cast. + */ + public static cast(value: string | PubKey): PubKey { + return typeof value === 'string' ? new PubKey(value) : value; + } + + /** + * Try convert `pubKeyString` to `PubKey`. + * + * @param pubkeyString The public key string. + * @returns `PubKey` if valid otherwise returns `undefined`. + */ public static from(pubkeyString: string): PubKey | undefined { // Returns a new instance if the pubkey is valid if (PubKey.validate(pubkeyString)) { diff --git a/ts/test/session/messages/ClosedGroupChatMessage_test.ts b/ts/test/session/messages/ClosedGroupChatMessage_test.ts index 4215824d0..b7b93115e 100644 --- a/ts/test/session/messages/ClosedGroupChatMessage_test.ts +++ b/ts/test/session/messages/ClosedGroupChatMessage_test.ts @@ -6,22 +6,32 @@ import { } from '../../../session/messages/outgoing'; import { SignalService } from '../../../protobuf'; import { TextEncoder } from 'util'; +import { TestUtils } from '../../test-utils'; +import { StringUtils } from '../../../session/utils'; +import { PubKey } from '../../../session/types'; describe('ClosedGroupChatMessage', () => { + let groupId: PubKey; + beforeEach(() => { + groupId = TestUtils.generateFakePubKey(); + }); it('can create empty message with timestamp, groupId and chatMessage', () => { const chatMessage = new ChatMessage({ timestamp: Date.now(), body: 'body', }); const message = new ClosedGroupChatMessage({ - groupId: '12', + groupId, chatMessage, }); const plainText = message.plainTextBuffer(); const decoded = SignalService.Content.decode(plainText); expect(decoded.dataMessage) .to.have.property('group') - .to.have.deep.property('id', new TextEncoder().encode('12')); + .to.have.deep.property( + 'id', + new Uint8Array(StringUtils.encode(groupId.key, 'utf8')) + ); expect(decoded.dataMessage) .to.have.property('group') .to.have.deep.property('type', SignalService.GroupContext.Type.DELIVER); @@ -39,7 +49,7 @@ describe('ClosedGroupChatMessage', () => { timestamp: Date.now(), }); const message = new ClosedGroupChatMessage({ - groupId: '12', + groupId, chatMessage, }); expect(message.ttl()).to.equal(24 * 60 * 60 * 1000); @@ -50,7 +60,7 @@ describe('ClosedGroupChatMessage', () => { timestamp: Date.now(), }); const message = new ClosedGroupChatMessage({ - groupId: '12', + groupId, chatMessage, }); expect(message.identifier).to.not.equal(null, 'identifier cannot be null'); @@ -67,7 +77,7 @@ describe('ClosedGroupChatMessage', () => { identifier: 'chatMessage', }); const message = new ClosedGroupChatMessage({ - groupId: '12', + groupId, chatMessage, identifier: 'closedGroupMessage', }); @@ -81,7 +91,7 @@ describe('ClosedGroupChatMessage', () => { identifier: 'chatMessage', }); const message = new ClosedGroupChatMessage({ - groupId: '12', + groupId, chatMessage, }); expect(message.identifier).to.be.equal('chatMessage'); diff --git a/ts/test/session/messages/TypingMessage_test.ts b/ts/test/session/messages/TypingMessage_test.ts index 8753f1408..ad14f4961 100644 --- a/ts/test/session/messages/TypingMessage_test.ts +++ b/ts/test/session/messages/TypingMessage_test.ts @@ -5,6 +5,8 @@ import { SignalService } from '../../../protobuf'; import { TextEncoder } from 'util'; import Long from 'long'; import { toNumber } from 'lodash'; +import { StringUtils } from '../../../session/utils'; +import { TestUtils } from '../../test-utils'; describe('TypingMessage', () => { it('has Action.STARTED if isTyping = true', () => { @@ -60,7 +62,7 @@ describe('TypingMessage', () => { }); it('has groupId set if a value given', () => { - const groupId = '6666666666'; + const groupId = TestUtils.generateFakePubKey(); const message = new TypingMessage({ timestamp: Date.now(), isTyping: true, @@ -68,7 +70,9 @@ describe('TypingMessage', () => { }); const plainText = message.plainTextBuffer(); const decoded = SignalService.Content.decode(plainText); - const manuallyEncodedGroupId = new TextEncoder().encode(groupId); + const manuallyEncodedGroupId = new Uint8Array( + StringUtils.encode(groupId.key, 'utf8') + ); expect(decoded.typingMessage?.groupId).to.be.deep.equal( manuallyEncodedGroupId diff --git a/ts/test/session/protocols/SessionProtocol_test.ts b/ts/test/session/protocols/SessionProtocol_test.ts index 712088aa2..5f87a99b9 100644 --- a/ts/test/session/protocols/SessionProtocol_test.ts +++ b/ts/test/session/protocols/SessionProtocol_test.ts @@ -12,7 +12,7 @@ import { PubKey } from '../../../session/types'; describe('SessionProtocol', () => { const sandbox = sinon.createSandbox(); const ourNumber = 'ourNumber'; - const pubkey = new PubKey('deviceid'); + const pubkey = TestUtils.generateFakePubKey(); let getItemById: sinon.SinonStub; let send: sinon.SinonStub; @@ -88,14 +88,14 @@ describe('SessionProtocol', () => { it('protocol: sendSessionRequest should add the deviceID to the sentMap', async () => { expect(SessionProtocol.getSentSessionsTimestamp()) - .to.have.property('deviceid') + .to.have.property(pubkey.key) .to.be.approximately(Date.now(), 100); }); it('protocol: sendSessionRequest should not have pendingSend set after', async () => { expect( SessionProtocol.getPendingSendSessionTimestamp() - ).to.not.have.property('deviceid'); + ).to.not.have.property(pubkey.key); }); }); @@ -107,34 +107,32 @@ describe('SessionProtocol', () => { it('protocol: onSessionEstablished should remove the device in sentTimestamps', async () => { expect(SessionProtocol.getSentSessionsTimestamp()).to.have.property( - 'deviceid' + pubkey.key ); await SessionProtocol.onSessionEstablished(pubkey); expect(SessionProtocol.getSentSessionsTimestamp()).to.not.have.property( - 'deviceid' + pubkey.key ); }); it('protocol: onSessionEstablished should remove the device in sentTimestamps and ONLY that one', async () => { // add a second item to the map - await SessionProtocol.sendSessionRequest( - resetMessage, - new PubKey('deviceid2') - ); + const anotherPubKey = TestUtils.generateFakePubKey(); + await SessionProtocol.sendSessionRequest(resetMessage, anotherPubKey); expect(SessionProtocol.getSentSessionsTimestamp()).to.have.property( - 'deviceid' + pubkey.key ); expect(SessionProtocol.getSentSessionsTimestamp()).to.have.property( - 'deviceid2' + anotherPubKey.key ); await SessionProtocol.onSessionEstablished(pubkey); expect(SessionProtocol.getSentSessionsTimestamp()).to.not.have.property( - 'deviceid' + pubkey.key ); expect(SessionProtocol.getSentSessionsTimestamp()).to.have.property( - 'deviceid2' + anotherPubKey.key ); }); }); @@ -144,7 +142,7 @@ describe('SessionProtocol', () => { const hasSent = await SessionProtocol.hasSentSessionRequest(pubkey); expect(hasSent).to.be.equal( false, - 'hasSent should be false for `deviceid`' + `hasSent should be false for ${pubkey.key}` ); }); @@ -154,7 +152,7 @@ describe('SessionProtocol', () => { const hasSent = await SessionProtocol.hasSentSessionRequest(pubkey); expect(hasSent).to.be.equal( true, - 'hasSent should be true for `deviceid`' + `hasSent should be true for ${pubkey.key}` ); }); @@ -174,7 +172,7 @@ describe('SessionProtocol', () => { const hasSent = await SessionProtocol.hasSentSessionRequest(pubkey); expect(hasSent).to.be.equal( true, - 'hasSent should be true for `deviceid`' + `hasSent should be true for ${pubkey.key}` ); }); @@ -189,7 +187,7 @@ describe('SessionProtocol', () => { const hasSent = await SessionProtocol.hasSentSessionRequest(pubkey); expect(hasSent).to.be.equal( true, - 'hasSent should be true for `deviceid`' + `hasSent should be true for ${pubkey.key}` ); send.resetHistory(); @@ -207,7 +205,7 @@ describe('SessionProtocol', () => { // trigger the requestProcessed and check the map is updated await SessionProtocol.onSessionRequestProcessed(pubkey); expect(SessionProtocol.getProcessedSessionsTimestamp()) - .to.have.property('deviceid') + .to.have.property(pubkey.key) .to.be.approximately(Date.now(), 5); }); @@ -217,14 +215,15 @@ describe('SessionProtocol', () => { await SessionProtocol.onSessionRequestProcessed(pubkey); expect(SessionProtocol.getProcessedSessionsTimestamp()) - .to.have.property('deviceid') + .to.have.property(pubkey.key) .to.be.approximately(Date.now(), 5); await TestUtils.timeout(5); - const oldTimestamp = SessionProtocol.getProcessedSessionsTimestamp() - .deviceid; + const oldTimestamp = SessionProtocol.getProcessedSessionsTimestamp()[ + pubkey.key + ]; await SessionProtocol.onSessionRequestProcessed(pubkey); expect(SessionProtocol.getProcessedSessionsTimestamp()) - .to.have.property('deviceid') + .to.have.property(pubkey.key) .to.be.approximately(Date.now(), 5) .to.not.be.equal(oldTimestamp); }); diff --git a/ts/test/session/sending/MessageQueue_test.ts b/ts/test/session/sending/MessageQueue_test.ts index 7d3bf5fc3..3f1a4cb72 100644 --- a/ts/test/session/sending/MessageQueue_test.ts +++ b/ts/test/session/sending/MessageQueue_test.ts @@ -312,6 +312,13 @@ describe('MessageQueue', () => { }); describe('sendToGroup', () => { + it('should throw an error if invalid non-group message was passed', async () => { + const chatMessage = TestUtils.generateChatMessage(); + await expect( + messageQueueStub.sendToGroup(chatMessage) + ).to.be.rejectedWith('Invalid group message passed in sendToGroup.'); + }); + describe('closed groups', async () => { it('can send to closed group', async () => { const members = TestUtils.generateFakePubKeys(4).map( @@ -324,8 +331,7 @@ describe('MessageQueue', () => { .resolves(); const message = TestUtils.generateClosedGroupMessage(); - const success = await messageQueueStub.sendToGroup(message); - expect(success).to.equal(true, 'sending to group failed'); + await messageQueueStub.sendToGroup(message); expect(sendUsingMultiDeviceStub.callCount).to.equal(members.length); const arg = sendUsingMultiDeviceStub.getCall(0).args; @@ -342,12 +348,7 @@ describe('MessageQueue', () => { .resolves(); const message = TestUtils.generateClosedGroupMessage(); - const response = await messageQueueStub.sendToGroup(message); - - expect(response).to.equal( - false, - 'sendToGroup sent a message to an empty group' - ); + await messageQueueStub.sendToGroup(message); expect(sendUsingMultiDeviceStub.callCount).to.equal(0); }); }); @@ -365,9 +366,8 @@ describe('MessageQueue', () => { it('can send to open group', async () => { const message = TestUtils.generateOpenGroupMessage(); - const success = await messageQueueStub.sendToGroup(message); + await messageQueueStub.sendToGroup(message); expect(sendToOpenGroupStub.callCount).to.equal(1); - expect(success).to.equal(true, 'Sending to open group failed'); }); it('should emit a success event when send was successful', async () => {