From 84f2ce777a18d7a10ed3ecb2bc3b07d37787f550 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 3 Oct 2022 11:46:25 +1100 Subject: [PATCH 1/2] fix: include profile in message request response --- protos/SignalService.proto | 4 +- ts/models/conversation.ts | 2 +- ts/receiver/contentMessage.ts | 14 ++++++ .../controlMessage/MessageRequestResponse.ts | 50 ++++++++++++++++++- .../outgoing/visibleMessage/VisibleMessage.ts | 9 +++- 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/protos/SignalService.proto b/protos/SignalService.proto index 7527f6b54..d94cbdfc5 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -37,7 +37,9 @@ message Unsend { message MessageRequestResponse { // @required - required bool isApproved = 1; + required bool isApproved = 1; + optional bytes profileKey = 2; + optional DataMessage.LokiProfile profile = 3; } message Content { diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index fd2fe7f21..96eddfe49 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -908,7 +908,7 @@ export class ConversationModel extends Backbone.Model { const messageRequestResponseParams: MessageRequestResponseParams = { timestamp, - // lokiProfile: UserUtils.getOurProfile(), // we can't curently include our profile in that response + lokiProfile: UserUtils.getOurProfile(), }; const messageRequestResponse = new MessageRequestResponse(messageRequestResponseParams); diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index 110a807c4..a61bc8eaa 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -27,6 +27,7 @@ import { } from '../interactions/conversations/unsendingInteractions'; import { ConversationTypeEnum } from '../models/conversationAttributes'; import { findCachedBlindedMatchOrLookupOnAllServers } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; +import { appendFetchAvatarAndProfileJob } from './userProfileImageUpdates'; export async function handleSwarmContentMessage(envelope: EnvelopePlus, messageHash: string) { try { @@ -604,6 +605,11 @@ async function handleMessageRequestResponse( messageRequestResponse: SignalService.MessageRequestResponse ) { const { isApproved } = messageRequestResponse; + if (!isApproved) { + window?.log?.error('handleMessageRequestResponse: isApproved is false -- dropping message.'); + await removeFromCache(envelope); + return; + } if (!messageRequestResponse) { window?.log?.error('handleMessageRequestResponse: Invalid parameters -- dropping message.'); await removeFromCache(envelope); @@ -674,6 +680,14 @@ async function handleMessageRequestResponse( } } + if (messageRequestResponse.profile && !isEmpty(messageRequestResponse.profile)) { + void appendFetchAvatarAndProfileJob( + conversationToApprove, + messageRequestResponse.profile, + messageRequestResponse.profileKey + ); + } + if (!conversationToApprove || conversationToApprove.didApproveMe() === isApproved) { if (conversationToApprove) { await conversationToApprove.commit(); diff --git a/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts b/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts index 95bde6a94..60d6e13a5 100644 --- a/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts +++ b/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts @@ -1,18 +1,47 @@ +import ByteBuffer from 'bytebuffer'; +import { isEmpty } from 'lodash'; import { SignalService } from '../../../../protobuf'; +import { LokiProfile } from '../../../../types/Message'; import { ContentMessage } from '../ContentMessage'; import { MessageParams } from '../Message'; // tslint:disable-next-line: no-empty-interface -export interface MessageRequestResponseParams extends MessageParams {} +export interface MessageRequestResponseParams extends MessageParams { + lokiProfile?: LokiProfile; +} export class MessageRequestResponse extends ContentMessage { // we actually send a response only if it is an accept // private readonly isApproved: boolean; + private readonly profileKey?: Uint8Array; + private readonly displayName?: string; + private readonly avatarPointer?: string; constructor(params: MessageRequestResponseParams) { super({ timestamp: params.timestamp, } as MessageRequestResponseParams); + + if (params.lokiProfile && params.lokiProfile.profileKey) { + if ( + params.lokiProfile.profileKey instanceof Uint8Array || + (params.lokiProfile.profileKey as any) instanceof ByteBuffer + ) { + this.profileKey = new Uint8Array(params.lokiProfile.profileKey); + } else { + this.profileKey = new Uint8Array( + ByteBuffer.wrap(params.lokiProfile.profileKey).toArrayBuffer() + ); + } + } + + this.displayName = params.lokiProfile && params.lokiProfile.displayName; + + // no need to iclude the avatarPointer if there is no profileKey associated with it. + this.avatarPointer = + params.lokiProfile?.avatarPointer && !isEmpty(this.profileKey) + ? params.lokiProfile.avatarPointer + : undefined; } public contentProto(): SignalService.Content { @@ -22,8 +51,27 @@ export class MessageRequestResponse extends ContentMessage { } public messageRequestResponseProto(): SignalService.MessageRequestResponse { + let profileKey: Uint8Array | undefined; + let profile: SignalService.DataMessage.LokiProfile | undefined; + if (this.avatarPointer || this.displayName) { + profile = new SignalService.DataMessage.LokiProfile(); + + if (this.avatarPointer) { + profile.profilePicture = this.avatarPointer; + } + + if (this.displayName) { + profile.displayName = this.displayName; + } + } + + if (this.profileKey && this.profileKey.length) { + profileKey = this.profileKey; + } return new SignalService.MessageRequestResponse({ isApproved: true, + profileKey, + profile, }); } } diff --git a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts index c5fe43d0c..166e6804b 100644 --- a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts @@ -1,4 +1,5 @@ import ByteBuffer from 'bytebuffer'; +import { isEmpty } from 'lodash'; import { DataMessage } from '..'; import { SignalService } from '../../../../protobuf'; import { LokiProfile } from '../../../../types/Message'; @@ -108,7 +109,12 @@ export class VisibleMessage extends DataMessage { } this.displayName = params.lokiProfile && params.lokiProfile.displayName; - this.avatarPointer = params.lokiProfile && params.lokiProfile.avatarPointer; + // no need to iclude the avatarPointer if there is no profileKey associated with it. + this.avatarPointer = + params.lokiProfile?.avatarPointer && !isEmpty(this.profileKey) + ? params.lokiProfile.avatarPointer + : undefined; + this.preview = params.preview; this.reaction = params.reaction; this.syncTarget = params.syncTarget; @@ -149,6 +155,7 @@ export class VisibleMessage extends DataMessage { } dataMessage.profile = profile; } + if (this.profileKey && this.profileKey.length) { dataMessage.profileKey = this.profileKey; } From 4ed837e57eb0a60bd25200200058d85693b6ccc7 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 3 Oct 2022 15:19:46 +1100 Subject: [PATCH 2/2] fix: add test for MessageRequestResponse outgoing message --- .../controlMessage/MessageRequestResponse.ts | 50 +----- .../outgoing/visibleMessage/VisibleMessage.ts | 82 +++++---- .../messages/MessageRequestResponse_test.ts | 160 ++++++++++++++++++ 3 files changed, 218 insertions(+), 74 deletions(-) create mode 100644 ts/test/session/unit/messages/MessageRequestResponse_test.ts diff --git a/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts b/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts index 60d6e13a5..dcedf5269 100644 --- a/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts +++ b/ts/session/messages/outgoing/controlMessage/MessageRequestResponse.ts @@ -1,9 +1,8 @@ -import ByteBuffer from 'bytebuffer'; -import { isEmpty } from 'lodash'; import { SignalService } from '../../../../protobuf'; import { LokiProfile } from '../../../../types/Message'; import { ContentMessage } from '../ContentMessage'; import { MessageParams } from '../Message'; +import { buildProfileForOutgoingMessage } from '../visibleMessage/VisibleMessage'; // tslint:disable-next-line: no-empty-interface export interface MessageRequestResponseParams extends MessageParams { @@ -14,34 +13,16 @@ export class MessageRequestResponse extends ContentMessage { // we actually send a response only if it is an accept // private readonly isApproved: boolean; private readonly profileKey?: Uint8Array; - private readonly displayName?: string; - private readonly avatarPointer?: string; + private readonly profile?: SignalService.DataMessage.ILokiProfile; constructor(params: MessageRequestResponseParams) { super({ timestamp: params.timestamp, } as MessageRequestResponseParams); - if (params.lokiProfile && params.lokiProfile.profileKey) { - if ( - params.lokiProfile.profileKey instanceof Uint8Array || - (params.lokiProfile.profileKey as any) instanceof ByteBuffer - ) { - this.profileKey = new Uint8Array(params.lokiProfile.profileKey); - } else { - this.profileKey = new Uint8Array( - ByteBuffer.wrap(params.lokiProfile.profileKey).toArrayBuffer() - ); - } - } - - this.displayName = params.lokiProfile && params.lokiProfile.displayName; - - // no need to iclude the avatarPointer if there is no profileKey associated with it. - this.avatarPointer = - params.lokiProfile?.avatarPointer && !isEmpty(this.profileKey) - ? params.lokiProfile.avatarPointer - : undefined; + const profile = buildProfileForOutgoingMessage(params); + this.profile = profile.lokiProfile; + this.profileKey = profile.profileKey; } public contentProto(): SignalService.Content { @@ -51,27 +32,10 @@ export class MessageRequestResponse extends ContentMessage { } public messageRequestResponseProto(): SignalService.MessageRequestResponse { - let profileKey: Uint8Array | undefined; - let profile: SignalService.DataMessage.LokiProfile | undefined; - if (this.avatarPointer || this.displayName) { - profile = new SignalService.DataMessage.LokiProfile(); - - if (this.avatarPointer) { - profile.profilePicture = this.avatarPointer; - } - - if (this.displayName) { - profile.displayName = this.displayName; - } - } - - if (this.profileKey && this.profileKey.length) { - profileKey = this.profileKey; - } return new SignalService.MessageRequestResponse({ isApproved: true, - profileKey, - profile, + profileKey: this.profileKey?.length ? this.profileKey : undefined, + profile: this.profile, }); } } diff --git a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts index 166e6804b..43e718dc5 100644 --- a/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts +++ b/ts/session/messages/outgoing/visibleMessage/VisibleMessage.ts @@ -81,8 +81,7 @@ export class VisibleMessage extends DataMessage { private readonly body?: string; private readonly quote?: Quote; private readonly profileKey?: Uint8Array; - private readonly displayName?: string; - private readonly avatarPointer?: string; + private readonly profile?: SignalService.DataMessage.ILokiProfile; private readonly preview?: Array; /// In the case of a sync message, the public key of the person the message was targeted at. @@ -95,25 +94,11 @@ export class VisibleMessage extends DataMessage { this.body = params.body; this.quote = params.quote; this.expireTimer = params.expireTimer; - if (params.lokiProfile && params.lokiProfile.profileKey) { - if ( - params.lokiProfile.profileKey instanceof Uint8Array || - (params.lokiProfile.profileKey as any) instanceof ByteBuffer - ) { - this.profileKey = new Uint8Array(params.lokiProfile.profileKey); - } else { - this.profileKey = new Uint8Array( - ByteBuffer.wrap(params.lokiProfile.profileKey).toArrayBuffer() - ); - } - } - this.displayName = params.lokiProfile && params.lokiProfile.displayName; - // no need to iclude the avatarPointer if there is no profileKey associated with it. - this.avatarPointer = - params.lokiProfile?.avatarPointer && !isEmpty(this.profileKey) - ? params.lokiProfile.avatarPointer - : undefined; + const profile = buildProfileForOutgoingMessage(params); + + this.profile = profile.lokiProfile; + this.profileKey = profile.profileKey; this.preview = params.preview; this.reaction = params.reaction; @@ -143,17 +128,8 @@ export class VisibleMessage extends DataMessage { dataMessage.syncTarget = this.syncTarget; } - if (this.avatarPointer || this.displayName) { - const profile = new SignalService.DataMessage.LokiProfile(); - - if (this.avatarPointer) { - profile.profilePicture = this.avatarPointer; - } - - if (this.displayName) { - profile.displayName = this.displayName; - } - dataMessage.profile = profile; + if (this.profile) { + dataMessage.profile = this.profile; } if (this.profileKey && this.profileKey.length) { @@ -208,3 +184,47 @@ export class VisibleMessage extends DataMessage { return this.identifier === comparator.identifier && this.timestamp === comparator.timestamp; } } + +export function buildProfileForOutgoingMessage(params: { lokiProfile?: LokiProfile }) { + let profileKey: Uint8Array | undefined; + if (params.lokiProfile && params.lokiProfile.profileKey) { + if ( + params.lokiProfile.profileKey instanceof Uint8Array || + (params.lokiProfile.profileKey as any) instanceof ByteBuffer + ) { + profileKey = new Uint8Array(params.lokiProfile.profileKey); + } else { + profileKey = new Uint8Array(ByteBuffer.wrap(params.lokiProfile.profileKey).toArrayBuffer()); + } + } + + const displayName = params.lokiProfile?.displayName; + + // no need to iclude the avatarPointer if there is no profileKey associated with it. + const avatarPointer = + params.lokiProfile?.avatarPointer && + !isEmpty(profileKey) && + params.lokiProfile.avatarPointer && + !isEmpty(params.lokiProfile.avatarPointer) + ? params.lokiProfile.avatarPointer + : undefined; + + let lokiProfile: SignalService.DataMessage.ILokiProfile | undefined; + if (avatarPointer || displayName) { + lokiProfile = new SignalService.DataMessage.LokiProfile(); + + // we always need a profileKey tom decode an avatar pointer + if (avatarPointer && avatarPointer.length && profileKey) { + lokiProfile.profilePicture = avatarPointer; + } + + if (displayName) { + lokiProfile.displayName = displayName; + } + } + + return { + lokiProfile, + profileKey: lokiProfile?.profilePicture ? profileKey : undefined, + }; +} diff --git a/ts/test/session/unit/messages/MessageRequestResponse_test.ts b/ts/test/session/unit/messages/MessageRequestResponse_test.ts new file mode 100644 index 000000000..85da9a965 --- /dev/null +++ b/ts/test/session/unit/messages/MessageRequestResponse_test.ts @@ -0,0 +1,160 @@ +import { expect } from 'chai'; +import { v4 } from 'uuid'; + +import { SignalService } from '../../../../protobuf'; +import { Constants } from '../../../../session'; +import { MessageRequestResponse } from '../../../../session/messages/outgoing/controlMessage/MessageRequestResponse'; +// tslint:disable: no-unused-expression + +// tslint:disable-next-line: max-func-body-length +describe('MessageRequestResponse', () => { + let message: MessageRequestResponse | undefined; + it('correct ttl', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + }); + + expect(message.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX); + }); + + it('has an identifier', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + }); + + expect(message.identifier).to.not.equal(null, 'identifier cannot be null'); + expect(message.identifier).to.not.equal(undefined, 'identifier cannot be undefined'); + }); + + it('has an identifier matching if given', () => { + const identifier = v4(); + message = new MessageRequestResponse({ + timestamp: Date.now(), + identifier, + }); + + expect(message.identifier).to.not.equal(identifier, 'identifier should match'); + }); + + it('isApproved is always true', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + expect(decoded.messageRequestResponse) + .to.have.property('isApproved') + .to.be.eq(true, 'isApproved is true'); + }); + + it('can create response without lokiProfile', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + expect(decoded.messageRequestResponse) + .to.have.property('profile') + .to.be.eq(null, 'no profile field if no profile given'); + }); + + it('can create response with display name only', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + lokiProfile: { displayName: 'Jane', profileKey: null }, + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.deep.eq('Jane'); + expect(decoded.messageRequestResponse?.profile?.profilePicture).to.be.empty; + expect(decoded.messageRequestResponse?.profileKey).to.be.empty; + }); + + it('empty profileKey does not get included', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + lokiProfile: { displayName: 'Jane', profileKey: new Uint8Array(0) }, + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.eq('Jane'); + + expect(decoded.messageRequestResponse?.profile?.profilePicture).to.be.empty; + expect(decoded.messageRequestResponse?.profileKey).to.be.empty; + }); + + it('can create response with display name and profileKey and profileImage', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + lokiProfile: { + displayName: 'Jane', + profileKey: new Uint8Array([1, 2, 3, 4, 5, 6]), + avatarPointer: 'https://somevalidurl.com', + }, + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.deep.eq('Jane'); + + expect(decoded.messageRequestResponse?.profileKey).to.be.not.empty; + + if (!decoded.messageRequestResponse?.profileKey?.buffer) { + throw new Error('decoded.messageRequestResponse?.profileKey?.buffer should be set'); + } + expect(decoded.messageRequestResponse?.profile?.profilePicture).to.be.eq( + 'https://somevalidurl.com' + ); + // don't ask me why deep.eq ([1,2,3, ...]) gives nothing interesting but a 8192 buffer not matching + expect(decoded.messageRequestResponse?.profileKey.length).to.be.eq(6); + expect(decoded.messageRequestResponse?.profileKey[0]).to.be.eq(1); + expect(decoded.messageRequestResponse?.profileKey[1]).to.be.eq(2); + expect(decoded.messageRequestResponse?.profileKey[2]).to.be.eq(3); + expect(decoded.messageRequestResponse?.profileKey[3]).to.be.eq(4); + expect(decoded.messageRequestResponse?.profileKey[4]).to.be.eq(5); + expect(decoded.messageRequestResponse?.profileKey[5]).to.be.eq(6); + }); + + it('profileKey not included if profileUrl not set', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + lokiProfile: { displayName: 'Jane', profileKey: new Uint8Array([1, 2, 3, 4, 5, 6]) }, + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.deep.eq('Jane'); + + if (!decoded.messageRequestResponse?.profileKey?.buffer) { + throw new Error('decoded.messageRequestResponse?.profileKey?.buffer should be set'); + } + + expect(decoded.messageRequestResponse?.profile?.profilePicture).to.be.empty; + expect(decoded.messageRequestResponse?.profileKey).to.be.empty; + }); + + it('url not included if profileKey not set', () => { + message = new MessageRequestResponse({ + timestamp: Date.now(), + lokiProfile: { + displayName: 'Jane', + profileKey: null, + avatarPointer: 'https://somevalidurl.com', + }, + }); + const plainText = message.plainTextBuffer(); + const decoded = SignalService.Content.decode(plainText); + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.deep.eq('Jane'); + + if (!decoded.messageRequestResponse?.profileKey?.buffer) { + throw new Error('decoded.messageRequestResponse?.profileKey?.buffer should be set'); + } + + expect(decoded.messageRequestResponse?.profile?.displayName).to.be.eq('Jane'); + expect(decoded.messageRequestResponse?.profile?.profilePicture).to.be.empty; + expect(decoded.messageRequestResponse?.profileKey).to.be.empty; + }); +});