From d77045b49d6c838bb2fd41b095a7c9890f1d94b3 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 6 Mar 2024 13:09:00 -0600 Subject: [PATCH] onSentMessage: Create destination conversation before further processing Co-authored-by: Scott Nonnenberg Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- ts/background.ts | 25 ++++++- ts/services/storage.ts | 37 ++++------ ts/services/storageRecordOps.ts | 38 ++++++++-- ts/test-mock/messaging/sendSync_test.ts | 94 +++++++++++++++++++++++++ ts/util/privacy.ts | 19 +++++ 5 files changed, 184 insertions(+), 29 deletions(-) create mode 100644 ts/test-mock/messaging/sendSync_test.ts diff --git a/ts/background.ts b/ts/background.ts index 04cfd22304..c16b907797 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -138,7 +138,11 @@ import { import { themeChanged } from './shims/themeChanged'; import { createIPCEvents } from './util/createIPCEvents'; import type { ServiceIdString } from './types/ServiceId'; -import { ServiceIdKind, isServiceIdString } from './types/ServiceId'; +import { + ServiceIdKind, + isPniString, + isServiceIdString, +} from './types/ServiceId'; import { isAciString } from './util/isAciString'; import { normalizeAci } from './util/normalizeAci'; import * as log from './logging/log'; @@ -2727,6 +2731,25 @@ export async function startApp(): Promise { const sourceServiceId = window.textsecure.storage.user.getAci(); strictAssert(source && sourceServiceId, 'Missing user number and uuid'); + // Make sure destination conversation is created before we hit getMessageDescriptor + if (data.destinationServiceId !== sourceServiceId) { + const { mergePromises } = + window.ConversationController.maybeMergeContacts({ + e164: data.destination, + aci: isAciString(data.destinationServiceId) + ? data.destinationServiceId + : undefined, + pni: isPniString(data.destinationServiceId) + ? data.destinationServiceId + : undefined, + reason: `onSentMessage(${data.timestamp})`, + }); + + if (mergePromises.length > 0) { + await Promise.all(mergePromises); + } + } + const messageDescriptor = getMessageDescriptor({ ...data, }); diff --git a/ts/services/storage.ts b/ts/services/storage.ts index debf4e4321..dbd440351b 100644 --- a/ts/services/storage.ts +++ b/ts/services/storage.ts @@ -67,6 +67,7 @@ import type { import { MY_STORY_ID } from '../types/Stories'; import { isNotNil } from '../util/isNotNil'; import { isSignalConversation } from '../util/isSignalConversation'; +import { redactExtendedStorageID, redactStorageID } from '../util/privacy'; type IManifestRecordIdentifier = Proto.ManifestRecord.IIdentifier; @@ -104,22 +105,6 @@ const conflictBackOff = new BackOff([ 30 * durations.SECOND, ]); -function redactStorageID( - storageID: string, - version?: number, - conversation?: ConversationModel -): string { - const convoId = conversation ? ` ${conversation?.idForLogging()}` : ''; - return `${version ?? '?'}:${storageID.substring(0, 3)}${convoId}`; -} - -function redactExtendedStorageID({ - storageID, - storageVersion, -}: ExtendedStorageID): string { - return redactStorageID(storageID, storageVersion); -} - function encryptRecord( storageID: string | undefined, storageRecord: Proto.IStorageRecord @@ -942,6 +927,10 @@ async function mergeRecord( itemToMerge: MergeableItemType ): Promise { const { itemType, storageID, storageRecord } = itemToMerge; + const redactedStorageID = redactExtendedStorageID({ + storageID, + storageVersion, + }); const ITEM_TYPE = Proto.ManifestRecord.Identifier.Type; @@ -953,7 +942,10 @@ async function mergeRecord( try { if (itemType === ITEM_TYPE.UNKNOWN) { - log.warn('storageService.mergeRecord: Unknown item type', storageID); + log.warn( + 'storageService.mergeRecord: Unknown item type', + redactedStorageID + ); } else if (itemType === ITEM_TYPE.CONTACT && storageRecord.contact) { mergeResult = await mergeContactRecord( storageID, @@ -999,10 +991,7 @@ async function mergeRecord( } else { isUnsupported = true; log.warn( - `storageService.merge(${redactStorageID( - storageID, - storageVersion - )}): unknown item type=${itemType}` + `storageService.merge(${redactedStorageID}): unknown item type=${itemType}` ); } @@ -1430,9 +1419,13 @@ async function fetchRemoteRecords( const remoteRecord = remoteOnlyRecords.get(base64ItemID); if (!remoteRecord) { + const redactedStorageID = redactExtendedStorageID({ + storageID: base64ItemID, + storageVersion, + }); throw new Error( "Got a remote record that wasn't requested with " + - `storageID: ${base64ItemID}` + `storageID: ${redactedStorageID}` ); } diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 4f2fbe1da1..1f4625ae2b 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -64,6 +64,7 @@ import { MY_STORY_ID, StorySendMode } from '../types/Stories'; import { findAndDeleteOnboardingStoryIfExists } from '../util/findAndDeleteOnboardingStoryIfExists'; import { downloadOnboardingStory } from '../util/downloadOnboardingStory'; import { drop } from '../util/drop'; +import { redactExtendedStorageID } from '../util/privacy'; const MY_STORY_BYTES = uuidToBytes(MY_STORY_ID); @@ -694,8 +695,12 @@ export async function mergeGroupV1Record( storageVersion: number, groupV1Record: Proto.IGroupV1Record ): Promise { + const redactedStorageID = redactExtendedStorageID({ + storageID, + storageVersion, + }); if (!groupV1Record.id) { - throw new Error(`No ID for ${storageID}`); + throw new Error(`No ID for ${redactedStorageID}`); } const groupId = Bytes.toBinary(groupV1Record.id); @@ -861,8 +866,12 @@ export async function mergeGroupV2Record( storageVersion: number, groupV2Record: Proto.IGroupV2Record ): Promise { + const redactedStorageID = redactExtendedStorageID({ + storageID, + storageVersion, + }); if (!groupV2Record.masterKey) { - throw new Error(`No master key for ${storageID}`); + throw new Error(`No master key for ${redactedStorageID}`); } const masterKeyBuffer = groupV2Record.masterKey; @@ -1013,9 +1022,16 @@ export async function mergeContactRecord( // We're going to ignore this; it's likely a PNI-only contact we've already merged if (conversation.getServiceId() !== serviceId) { + const previousStorageID = conversation.get('storageID'); + const redactedpreviousStorageID = previousStorageID + ? redactExtendedStorageID({ + storageID: previousStorageID, + storageVersion: conversation.get('storageVersion'), + }) + : ''; log.warn( `mergeContactRecord: ${conversation.idForLogging()} ` + - `with storageId ${conversation.get('storageID')} ` + + `with storageId ${redactedpreviousStorageID} ` + `had serviceId that didn't match provided serviceId ${serviceId}` ); return { @@ -1548,8 +1564,14 @@ export async function mergeStoryDistributionListRecord( storageVersion: number, storyDistributionListRecord: Proto.IStoryDistributionListRecord ): Promise { + const redactedStorageID = redactExtendedStorageID({ + storageID, + storageVersion, + }); if (!storyDistributionListRecord.identifier) { - throw new Error(`No storyDistributionList identifier for ${storageID}`); + throw new Error( + `No storyDistributionList identifier for ${redactedStorageID}` + ); } const details: Array = []; @@ -1683,8 +1705,12 @@ export async function mergeStickerPackRecord( storageVersion: number, stickerPackRecord: Proto.IStickerPackRecord ): Promise { + const redactedStorageID = redactExtendedStorageID({ + storageID, + storageVersion, + }); if (!stickerPackRecord.packId || Bytes.isEmpty(stickerPackRecord.packId)) { - throw new Error(`No stickerPackRecord identifier for ${storageID}`); + throw new Error(`No stickerPackRecord identifier for ${redactedStorageID}`); } const details: Array = []; @@ -1714,7 +1740,7 @@ export async function mergeStickerPackRecord( !stickerPackRecord.packKey || Bytes.isEmpty(stickerPackRecord.packKey) ) { - throw new Error(`No stickerPackRecord key for ${storageID}`); + throw new Error(`No stickerPackRecord key for ${redactedStorageID}`); } stickerPack = { diff --git a/ts/test-mock/messaging/sendSync_test.ts b/ts/test-mock/messaging/sendSync_test.ts new file mode 100644 index 0000000000..059d21901b --- /dev/null +++ b/ts/test-mock/messaging/sendSync_test.ts @@ -0,0 +1,94 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import createDebug from 'debug'; +import Long from 'long'; + +import type { App } from '../playwright'; +import * as durations from '../../util/durations'; +import { Bootstrap } from '../bootstrap'; + +export const debug = createDebug('mock:test:sendSync'); + +describe('sendSync', function (this: Mocha.Suite) { + this.timeout(durations.MINUTE); + + let bootstrap: Bootstrap; + let app: App; + + beforeEach(async () => { + bootstrap = new Bootstrap(); + await bootstrap.init(); + app = await bootstrap.link(); + }); + + afterEach(async function (this: Mocha.Context) { + if (!bootstrap) { + return; + } + + await bootstrap.maybeSaveLogs(this.currentTest, app); + await app.close(); + await bootstrap.teardown(); + }); + + it('creates conversation for sendSync to PNI', async () => { + const { desktop, phone, server } = bootstrap; + + debug('Creating stranger'); + const STRANGER_NAME = 'Mysterious Stranger'; + const stranger = await server.createPrimaryDevice({ + profileName: STRANGER_NAME, + }); + + const timestamp = Date.now(); + const messageText = 'hey there, just reaching out'; + const destinationServiceId = stranger.device.pni; + const destination = stranger.device.number; + const originalDataMessage = { + body: messageText, + timestamp: Long.fromNumber(timestamp), + }; + const content = { + syncMessage: { + sent: { + destinationServiceId, + destination, + timestamp: Long.fromNumber(timestamp), + message: originalDataMessage, + }, + }, + }; + const sendOptions = { + timestamp, + }; + await phone.sendRaw(desktop, content, sendOptions); + + const page = await app.getWindow(); + const leftPane = page.locator('#LeftPane'); + + debug('checking left pane for conversation'); + const strangerName = await leftPane + .locator( + '.module-conversation-list__item--contact-or-conversation .module-contact-name' + ) + .first() + .innerText(); + + assert.equal( + strangerName.slice(-4), + destination?.slice(-4), + 'no profile, just phone number' + ); + + debug('opening conversation'); + await leftPane + .locator('.module-conversation-list__item--contact-or-conversation') + .first() + .click(); + + debug('checking for latest message'); + await page.locator(`.module-message__text >> "${messageText}"`).waitFor(); + }); +}); diff --git a/ts/util/privacy.ts b/ts/util/privacy.ts index a94d4a1b54..efe910a62b 100644 --- a/ts/util/privacy.ts +++ b/ts/util/privacy.ts @@ -8,6 +8,9 @@ import path from 'path'; import { compose } from 'lodash/fp'; import { escapeRegExp, isString, isRegExp } from 'lodash'; +import type { ExtendedStorageID } from '../types/StorageService.d'; +import type { ConversationModel } from '../models/conversations'; + export const APP_ROOT_PATH = path.join(__dirname, '..', '..'); const PHONE_NUMBER_PATTERN = /\+\d{7,12}(\d{3})/g; @@ -23,6 +26,22 @@ const REDACTION_PLACEHOLDER = '[REDACTED]'; export type RedactFunction = (value: string) => string; +export function redactStorageID( + storageID: string, + version?: number, + conversation?: ConversationModel +): string { + const convoId = conversation ? ` ${conversation?.idForLogging()}` : ''; + return `${version ?? '?'}:${storageID.substring(0, 3)}${convoId}`; +} + +export function redactExtendedStorageID({ + storageID, + storageVersion, +}: ExtendedStorageID): string { + return redactStorageID(storageID, storageVersion); +} + export const _redactPath = (filePath: string): RedactFunction => { if (!isString(filePath)) { throw new TypeError("'filePath' must be a string");