From 1a902b8d4ae795ca5df2139685a3d7ed785454eb Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Thu, 20 Jan 2022 15:10:10 -0800 Subject: [PATCH] Add extra logging for change phone number Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- ts/ConversationController.ts | 50 +++++++++++++++++++------ ts/background.ts | 10 +++++ ts/models/conversations.ts | 9 +++-- ts/models/messages.ts | 1 + ts/services/storageRecordOps.ts | 1 + ts/textsecure/AccountManager.ts | 1 + ts/updateConversationsWithUuidLookup.ts | 1 + 7 files changed, 59 insertions(+), 14 deletions(-) diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index a68496f3f..4ee04a6f6 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -277,7 +277,12 @@ export class ConversationController { getOurConversationId(): string | undefined { const e164 = window.textsecure.storage.user.getNumber(); const uuid = window.textsecure.storage.user.getUuid()?.toString(); - return this.ensureContactIds({ e164, uuid, highTrust: true }); + return this.ensureContactIds({ + e164, + uuid, + highTrust: true, + reason: 'getOurConversationId', + }); } getOurConversationIdOrThrow(): string { @@ -323,11 +328,20 @@ export class ConversationController { e164, uuid, highTrust, - }: { - e164?: string | null; - uuid?: string | null; - highTrust?: boolean; - }): string | undefined { + reason, + }: + | { + e164?: string | null; + uuid?: string | null; + highTrust?: false; + reason?: void; + } + | { + e164?: string | null; + uuid?: string | null; + highTrust: true; + reason: string; + }): string | undefined { // Check for at least one parameter being provided. This is necessary // because this path can be called on startup to resolve our own ID before // our phone number or UUID are known. The existing behavior in these @@ -344,7 +358,10 @@ export class ConversationController { // 1. Handle no match at all if (!convoE164 && !convoUuid) { - log.info('ensureContactIds: Creating new contact, no matches found'); + log.info( + 'ensureContactIds: Creating new contact, no matches found', + highTrust ? reason : 'no reason' + ); const newConvo = this.getOrCreate(identifier, 'private'); if (highTrust && e164) { newConvo.updateE164(e164); @@ -373,7 +390,10 @@ export class ConversationController { // Fill in the UUID for an e164-only contact if (normalizedUuid && !convoE164.get('uuid')) { if (highTrust) { - log.info('ensureContactIds: Adding UUID to e164-only match'); + log.info( + `ensureContactIds: Adding UUID (${uuid}) to e164-only match ` + + `(${e164}), reason: ${reason}` + ); convoE164.updateUuid(normalizedUuid); updateConversation(convoE164.attributes); } @@ -387,7 +407,10 @@ export class ConversationController { const newConvo = this.getOrCreate(normalizedUuid, 'private'); if (highTrust) { - log.info('ensureContactIds: Moving e164 from old contact to new'); + log.info( + `ensureContactIds: Moving e164 (${e164}) from old contact ` + + `(${convoE164.get('uuid')}) to new (${uuid}), reason: ${reason}` + ); // Remove the e164 from the old contact... convoE164.set({ e164: undefined }); @@ -404,7 +427,10 @@ export class ConversationController { } if (!convoE164 && convoUuid) { if (e164 && highTrust) { - log.info('ensureContactIds: Adding e164 to UUID-only match'); + log.info( + `ensureContactIds: Adding e164 (${e164}) to UUID-only match ` + + `(${uuid}), reason: ${reason}` + ); convoUuid.updateE164(e164); updateConversation(convoUuid.attributes); } @@ -427,7 +453,9 @@ export class ConversationController { // Conflict: If e164 match already has a UUID, we remove its e164. if (convoE164.get('uuid') && convoE164.get('uuid') !== normalizedUuid) { log.info( - 'ensureContactIds: e164 match had different UUID than incoming pair, removing its e164.' + `ensureContactIds: e164 match (${e164}) had different ` + + `UUID(${convoE164.get('uuid')}) than incoming pair (${uuid}), ` + + `removing its e164, reason: ${reason}` ); // Remove the e164 from the old contact... diff --git a/ts/background.ts b/ts/background.ts index 617aeaded..c083dbcd5 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -2464,6 +2464,7 @@ export async function startApp(): Promise { e164: sender, uuid: senderUuid, highTrust: true, + reason: `onTyping(${typing.timestamp})`, }); // We multiplex between GV1/GV2 groups here, but we don't kick off migrations @@ -2604,6 +2605,7 @@ export async function startApp(): Promise { e164: details.number, uuid: details.uuid, highTrust: true, + reason: 'onContactReceived', }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const conversation = window.ConversationController.get(detailsId)!; @@ -2832,6 +2834,7 @@ export async function startApp(): Promise { e164: envelope.source, uuid: envelope.sourceUuid, highTrust: true, + reason: `onEnvelopeReceived(${envelope.timestamp})`, }); } } @@ -2967,6 +2970,7 @@ export async function startApp(): Promise { e164: data.source, uuid: data.sourceUuid, highTrust: true, + reason: 'onProfileKeyUpdate', }); const conversation = window.ConversationController.get(conversationId); @@ -3052,6 +3056,7 @@ export async function startApp(): Promise { uuid: destinationUuid, e164: destination, highTrust: true, + reason: `unidentifiedStatus(${timestamp})`, } ); if (!conversationId || conversationId === ourId) { @@ -3184,6 +3189,7 @@ export async function startApp(): Promise { e164: source, uuid: sourceUuid, highTrust: true, + reason: `getMessageDescriptor(${message.timestamp}): group v1`, }); const conversationId = window.ConversationController.ensureGroup(id, { @@ -3200,6 +3206,7 @@ export async function startApp(): Promise { e164: destination, uuid: destinationUuid, highTrust: true, + reason: `getMessageDescriptor(${message.timestamp}): private`, }); if (!id) { confirm(); @@ -3606,6 +3613,7 @@ export async function startApp(): Promise { e164: source, uuid: sourceUuid, highTrust: true, + reason: `onReadOrViewReceipt(${envelopeTimestamp})`, } ); log.info( @@ -3772,6 +3780,7 @@ export async function startApp(): Promise { e164, uuid, highTrust: true, + reason: 'onVerified', }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const contact = window.ConversationController.get(verifiedId)!; @@ -3802,6 +3811,7 @@ export async function startApp(): Promise { e164: source, uuid: sourceUuid, highTrust: true, + reason: `onDeliveryReceipt(${envelopeTimestamp})`, } ); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index e73f37936..149e24f91 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -1866,7 +1866,7 @@ export class ConversationModel extends window.Backbone this.set('e164', e164); if (oldValue) { - this.addChangeNumberNotification(); + this.addChangeNumberNotification(oldValue, e164); } window.Signal.Data.updateConversation(this.attributes); @@ -3229,7 +3229,10 @@ export class ConversationModel extends window.Backbone this.set('pendingUniversalTimer', undefined); } - async addChangeNumberNotification(): Promise { + async addChangeNumberNotification( + oldValue: string, + newValue: string + ): Promise { const sourceUuid = this.getCheckedUuid( 'Change number notification without uuid' ); @@ -3245,7 +3248,7 @@ export class ConversationModel extends window.Backbone log.info( `Conversation ${this.idForLogging()}: adding change number ` + - `notification for ${sourceUuid.toString()}` + `notification for ${sourceUuid.toString()} from ${oldValue} to ${newValue}` ); const convos = [ diff --git a/ts/models/messages.ts b/ts/models/messages.ts index bc8a960cd..a3a54ef59 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -2247,6 +2247,7 @@ export class MessageModel extends window.Backbone.Model { uuid: destinationUuid, e164: destination, highTrust: true, + reason: `handleDataMessage(${initialMessage.timestamp})`, }); if (!destinationConversationId) { return; diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 84a1d1d40..be640b9db 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -770,6 +770,7 @@ export async function mergeContactRecord( e164, uuid, highTrust: true, + reason: 'mergeContactRecord', }); if (!id) { diff --git a/ts/textsecure/AccountManager.ts b/ts/textsecure/AccountManager.ts index 72f2dc56d..cc1b85570 100644 --- a/ts/textsecure/AccountManager.ts +++ b/ts/textsecure/AccountManager.ts @@ -604,6 +604,7 @@ export default class AccountManager extends EventTarget { e164: number, uuid: ourUuid, highTrust: true, + reason: 'createAccount', }); if (!conversationId) { diff --git a/ts/updateConversationsWithUuidLookup.ts b/ts/updateConversationsWithUuidLookup.ts index afd52d9cf..0e8b0bc80 100644 --- a/ts/updateConversationsWithUuidLookup.ts +++ b/ts/updateConversationsWithUuidLookup.ts @@ -43,6 +43,7 @@ export async function updateConversationsWithUuidLookup({ e164, uuid: uuidFromServer, highTrust: true, + reason: 'updateConversationsWithUuidLookup', }); const maybeFinalConversation = conversationController.get(finalConversationId);