diff --git a/ts/jobs/conversationJobQueue.ts b/ts/jobs/conversationJobQueue.ts index 5486d73f42..184da71bcb 100644 --- a/ts/jobs/conversationJobQueue.ts +++ b/ts/jobs/conversationJobQueue.ts @@ -330,7 +330,7 @@ export class ConversationJobQueue extends JobQueue { } } } catch (error: unknown) { - const untrustedConversationIds: Array = []; + const untrustedUuids: Array = []; const processError = (toProcess: unknown) => { if (toProcess instanceof OutgoingIdentityKeyError) { @@ -339,7 +339,14 @@ export class ConversationJobQueue extends JobQueue { 'private' ); strictAssert(failedConversation, 'Conversation should be created'); - untrustedConversationIds.push(failedConversation.id); + const uuid = failedConversation.get('uuid'); + if (!uuid) { + log.error( + `failedConversation: Conversation ${failedConversation.idForLogging()} missing UUID!` + ); + return; + } + untrustedUuids.push(uuid); } else if (toProcess instanceof SendMessageChallengeError) { window.Signal.challengeHandler.register( { @@ -358,14 +365,14 @@ export class ConversationJobQueue extends JobQueue { (error.errors || []).forEach(processError); } - if (untrustedConversationIds.length) { + if (untrustedUuids.length) { log.error( - `Send failed because ${untrustedConversationIds.length} conversation(s) were untrusted. Adding to verification list.` + `Send failed because ${untrustedUuids.length} conversation(s) were untrusted. Adding to verification list.` ); window.reduxActions.conversations.conversationStoppedByMissingVerification( { conversationId: conversation.id, - untrustedConversationIds, + untrustedUuids, } ); } diff --git a/ts/jobs/helpers/getUntrustedConversationIds.ts b/ts/jobs/helpers/getUntrustedConversationIds.ts deleted file mode 100644 index 8ebb420064..0000000000 --- a/ts/jobs/helpers/getUntrustedConversationIds.ts +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2022 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import { isNotNil } from '../../util/isNotNil'; - -export function getUntrustedConversationIds( - recipients: ReadonlyArray -): Array { - return recipients - .map(recipient => { - const recipientConversation = window.ConversationController.getOrCreate( - recipient, - 'private' - ); - return recipientConversation.isUntrusted() - ? recipientConversation.id - : null; - }) - .filter(isNotNil); -} diff --git a/ts/jobs/helpers/getUntrustedConversationUuids.ts b/ts/jobs/helpers/getUntrustedConversationUuids.ts new file mode 100644 index 0000000000..48a2ad2c7f --- /dev/null +++ b/ts/jobs/helpers/getUntrustedConversationUuids.ts @@ -0,0 +1,32 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { isNotNil } from '../../util/isNotNil'; +import * as log from '../../logging/log'; + +export function getUntrustedConversationUuids( + recipients: ReadonlyArray +): Array { + return recipients + .map(recipient => { + const recipientConversation = window.ConversationController.getOrCreate( + recipient, + 'private' + ); + + if (!recipientConversation.isUntrusted()) { + return null; + } + + const uuid = recipientConversation.get('uuid'); + if (!uuid) { + log.warn( + `getUntrustedConversationUuids: Conversation ${recipientConversation.idForLogging()} had no UUID` + ); + return null; + } + + return uuid; + }) + .filter(isNotNil); +} diff --git a/ts/jobs/helpers/sendDeleteForEveryone.ts b/ts/jobs/helpers/sendDeleteForEveryone.ts index be0ea4302d..0f00f4151c 100644 --- a/ts/jobs/helpers/sendDeleteForEveryone.ts +++ b/ts/jobs/helpers/sendDeleteForEveryone.ts @@ -23,7 +23,7 @@ import type { ConversationQueueJobBundle, DeleteForEveryoneJobData, } from '../conversationJobQueue'; -import { getUntrustedConversationIds } from './getUntrustedConversationIds'; +import { getUntrustedConversationUuids } from './getUntrustedConversationUuids'; import { handleMessageSend } from '../../util/handleMessageSend'; import { isConversationAccepted } from '../../util/isConversationAccepted'; import { isConversationUnregistered } from '../../util/isConversationUnregistered'; @@ -79,14 +79,14 @@ export async function sendDeleteForEveryone( ? getRecipients(deletedForEveryoneSendStatus) : recipientsFromJob; - const untrustedConversationIds = getUntrustedConversationIds(recipients); - if (untrustedConversationIds.length) { + const untrustedUuids = getUntrustedConversationUuids(recipients); + if (untrustedUuids.length) { window.reduxActions.conversations.conversationStoppedByMissingVerification({ conversationId: conversation.id, - untrustedConversationIds, + untrustedUuids, }); throw new Error( - `Delete for everyone blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.` + `Delete for everyone blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.` ); } diff --git a/ts/jobs/helpers/sendDirectExpirationTimerUpdate.ts b/ts/jobs/helpers/sendDirectExpirationTimerUpdate.ts index efd2d5921c..c7a8724340 100644 --- a/ts/jobs/helpers/sendDirectExpirationTimerUpdate.ts +++ b/ts/jobs/helpers/sendDirectExpirationTimerUpdate.ts @@ -44,9 +44,14 @@ export async function sendDirectExpirationTimerUpdate( } if (conversation.isUntrusted()) { + const uuid = conversation + .getCheckedUuid( + 'Expiration timer send blocked: untrusted and missing uuid!' + ) + .toString(); window.reduxActions.conversations.conversationStoppedByMissingVerification({ conversationId: conversation.id, - untrustedConversationIds: [conversation.id], + untrustedUuids: [uuid], }); throw new Error( 'Expiration timer send blocked because conversation is untrusted. Failing this attempt.' diff --git a/ts/jobs/helpers/sendGroupUpdate.ts b/ts/jobs/helpers/sendGroupUpdate.ts index cbe6744b22..a11ce13374 100644 --- a/ts/jobs/helpers/sendGroupUpdate.ts +++ b/ts/jobs/helpers/sendGroupUpdate.ts @@ -19,7 +19,7 @@ import type { GroupUpdateJobData, ConversationQueueJobBundle, } from '../conversationJobQueue'; -import { getUntrustedConversationIds } from './getUntrustedConversationIds'; +import { getUntrustedConversationUuids } from './getUntrustedConversationUuids'; // Note: because we don't have a recipient map, if some sends fail, we will resend this // message to folks that got it on the first go-round. This is okay, because receivers @@ -53,14 +53,14 @@ export async function sendGroupUpdate( const { groupChangeBase64, recipients, revision } = data; - const untrustedConversationIds = getUntrustedConversationIds(recipients); - if (untrustedConversationIds.length) { + const untrustedUuids = getUntrustedConversationUuids(recipients); + if (untrustedUuids.length) { window.reduxActions.conversations.conversationStoppedByMissingVerification({ conversationId: conversation.id, - untrustedConversationIds, + untrustedUuids, }); throw new Error( - `Group update blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.` + `Group update blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.` ); } diff --git a/ts/jobs/helpers/sendNormalMessage.ts b/ts/jobs/helpers/sendNormalMessage.ts index a74990849c..ac83ae896b 100644 --- a/ts/jobs/helpers/sendNormalMessage.ts +++ b/ts/jobs/helpers/sendNormalMessage.ts @@ -108,21 +108,22 @@ export async function sendNormalMessage( const { allRecipientIdentifiers, recipientIdentifiersWithoutMe, - untrustedConversationIds, + untrustedUuids, } = getMessageRecipients({ + log, message, conversation, }); - if (untrustedConversationIds.length) { + if (untrustedUuids.length) { window.reduxActions.conversations.conversationStoppedByMissingVerification( { conversationId: conversation.id, - untrustedConversationIds, + untrustedUuids, } ); throw new Error( - `Message ${messageId} sending blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.` + `Message ${messageId} sending blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.` ); } @@ -326,19 +327,21 @@ export async function sendNormalMessage( } function getMessageRecipients({ + log, conversation, message, }: Readonly<{ + log: LoggerType; conversation: ConversationModel; message: MessageModel; }>): { allRecipientIdentifiers: Array; recipientIdentifiersWithoutMe: Array; - untrustedConversationIds: Array; + untrustedUuids: Array; } { const allRecipientIdentifiers: Array = []; const recipientIdentifiersWithoutMe: Array = []; - const untrustedConversationIds: Array = []; + const untrustedUuids: Array = []; const currentConversationRecipients = conversation.getMemberConversationIds(); @@ -365,7 +368,14 @@ function getMessageRecipients({ } if (recipient.isUntrusted()) { - untrustedConversationIds.push(recipientConversationId); + const uuid = recipient.get('uuid'); + if (!uuid) { + log.error( + `sendNormalMessage/getMessageRecipients: Untrusted conversation ${recipient.idForLogging()} missing UUID.` + ); + return; + } + untrustedUuids.push(uuid); return; } if (recipient.isUnregistered()) { @@ -387,7 +397,7 @@ function getMessageRecipients({ return { allRecipientIdentifiers, recipientIdentifiersWithoutMe, - untrustedConversationIds, + untrustedUuids, }; } diff --git a/ts/jobs/helpers/sendReaction.ts b/ts/jobs/helpers/sendReaction.ts index fea23561f1..e40aa5a945 100644 --- a/ts/jobs/helpers/sendReaction.ts +++ b/ts/jobs/helpers/sendReaction.ts @@ -34,6 +34,7 @@ import type { } from '../conversationJobQueue'; import { isConversationAccepted } from '../../util/isConversationAccepted'; import { isConversationUnregistered } from '../../util/isConversationUnregistered'; +import type { LoggerType } from '../../types/Logging'; export async function sendReaction( conversation: ConversationModel, @@ -106,18 +107,18 @@ export async function sendReaction( const { allRecipientIdentifiers, recipientIdentifiersWithoutMe, - untrustedConversationIds, - } = getRecipients(pendingReaction, conversation); + untrustedUuids, + } = getRecipients(log, pendingReaction, conversation); - if (untrustedConversationIds.length) { + if (untrustedUuids.length) { window.reduxActions.conversations.conversationStoppedByMissingVerification( { conversationId: conversation.id, - untrustedConversationIds, + untrustedUuids, } ); throw new Error( - `Reaction for message ${messageId} sending blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.` + `Reaction for message ${messageId} sending blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.` ); } @@ -382,16 +383,17 @@ const setReactions = ( }; function getRecipients( + log: LoggerType, reaction: Readonly, conversation: ConversationModel ): { allRecipientIdentifiers: Array; recipientIdentifiersWithoutMe: Array; - untrustedConversationIds: Array; + untrustedUuids: Array; } { const allRecipientIdentifiers: Array = []; const recipientIdentifiersWithoutMe: Array = []; - const untrustedConversationIds: Array = []; + const untrustedUuids: Array = []; const currentConversationRecipients = conversation.getMemberConversationIds(); @@ -412,11 +414,18 @@ function getRecipients( } if (recipient.isUntrusted()) { - untrustedConversationIds.push(recipientIdentifier); + const uuid = recipient.get('uuid'); + if (!uuid) { + log.error( + `sendReaction/getRecipients: Untrusted conversation ${recipient.idForLogging()} missing UUID.` + ); + continue; + } + untrustedUuids.push(uuid); continue; } if (recipient.isUnregistered()) { - untrustedConversationIds.push(recipientIdentifier); + untrustedUuids.push(recipientIdentifier); continue; } @@ -429,7 +438,7 @@ function getRecipients( return { allRecipientIdentifiers, recipientIdentifiersWithoutMe, - untrustedConversationIds, + untrustedUuids, }; } diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index de318dbfb9..d48d89cacf 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -56,7 +56,7 @@ import { ContactSpoofingType } from '../../util/contactSpoofing'; import { writeProfile } from '../../services/writeProfile'; import { writeUsername } from '../../services/writeUsername'; import { - getConversationIdsStoppingSend, + getConversationUuidsStoppingSend, getConversationIdsStoppedForVerification, getMe, getUsernameSaveState, @@ -280,7 +280,7 @@ type ComposerGroupCreationState = { export type ConversationVerificationData = | { type: ConversationVerificationState.PendingVerification; - conversationsNeedingVerification: ReadonlyArray; + uuidsNeedingVerification: ReadonlyArray; } | { type: ConversationVerificationState.VerificationCancelled; @@ -535,7 +535,7 @@ type ConversationStoppedByMissingVerificationActionType = { type: typeof CONVERSATION_STOPPED_BY_MISSING_VERIFICATION; payload: { conversationId: string; - untrustedConversationIds: ReadonlyArray; + untrustedUuids: ReadonlyArray; }; }; export type MessageChangedActionType = { @@ -1280,19 +1280,22 @@ function verifyConversationsStoppingSend(): ThunkAction< > { return async (dispatch, getState) => { const state = getState(); - const conversationIdsStoppingSend = getConversationIdsStoppingSend(state); + const uuidsStoppingSend = getConversationUuidsStoppingSend(state); const conversationIdsBlocked = getConversationIdsStoppedForVerification(state); log.info( `verifyConversationsStoppingSend: Starting with ${conversationIdsBlocked.length} blocked ` + - `conversations and ${conversationIdsStoppingSend.length} conversations to verify.` + `conversations and ${uuidsStoppingSend.length} conversations to verify.` ); // Mark conversations as approved/verified as appropriate const promises: Array> = []; - conversationIdsStoppingSend.forEach(async conversationId => { - const conversation = window.ConversationController.get(conversationId); + uuidsStoppingSend.forEach(async uuid => { + const conversation = window.ConversationController.get(uuid); if (!conversation) { + log.warn( + `verifyConversationsStoppingSend: Cannot verify missing converastion for uuid ${uuid}` + ); return; } @@ -1512,19 +1515,17 @@ function selectMessage( function conversationStoppedByMissingVerification(payload: { conversationId: string; - untrustedConversationIds: ReadonlyArray; + untrustedUuids: ReadonlyArray; }): ConversationStoppedByMissingVerificationActionType { // Fetching profiles to ensure that we have their latest identity key in storage const profileFetchQueue = new PQueue({ concurrency: 3, }); - payload.untrustedConversationIds.forEach(untrustedConversationId => { - const conversation = window.ConversationController.get( - untrustedConversationId - ); + payload.untrustedUuids.forEach(uuid => { + const conversation = window.ConversationController.get(uuid); if (!conversation) { log.error( - `conversationStoppedByMissingVerification: conversationId ${untrustedConversationId} not found!` + `conversationStoppedByMissingVerification: uuid ${uuid} not found!` ); return; } @@ -2440,7 +2441,7 @@ export function reducer( }; } if (action.type === CONVERSATION_STOPPED_BY_MISSING_VERIFICATION) { - const { conversationId, untrustedConversationIds } = action.payload; + const { conversationId, untrustedUuids } = action.payload; const { verificationDataByConversation } = state; const existingPendingState = getOwn( @@ -2459,16 +2460,16 @@ export function reducer( ...verificationDataByConversation, [conversationId]: { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification: untrustedConversationIds, + uuidsNeedingVerification: untrustedUuids, }, }, }; } - const conversationsNeedingVerification: ReadonlyArray = Array.from( + const uuidsNeedingVerification: ReadonlyArray = Array.from( new Set([ - ...existingPendingState.conversationsNeedingVerification, - ...untrustedConversationIds, + ...existingPendingState.uuidsNeedingVerification, + ...untrustedUuids, ]) ); @@ -2478,7 +2479,7 @@ export function reducer( ...verificationDataByConversation, [conversationId]: { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification, + uuidsNeedingVerification, }, }, }; diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index cc9f7c1786..8f59d7fbd5 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -1011,13 +1011,13 @@ export const getConversationsStoppedForVerification = createSelector( } ); -export const getConversationIdsStoppingSend = createSelector( +export const getConversationUuidsStoppingSend = createSelector( getConversationVerificationData, (pendingData): Array => { const result = new Set(); Object.values(pendingData).forEach(item => { if (item.type === ConversationVerificationState.PendingVerification) { - item.conversationsNeedingVerification.forEach(conversationId => { + item.uuidsNeedingVerification.forEach(conversationId => { result.add(conversationId); }); } @@ -1027,20 +1027,13 @@ export const getConversationIdsStoppingSend = createSelector( ); export const getConversationsStoppingSend = createSelector( - getConversationByIdSelector, - getConversationIdsStoppingSend, + getConversationSelector, + getConversationUuidsStoppingSend, ( - conversationSelector: (id: string) => undefined | ConversationType, - conversationIds: ReadonlyArray + conversationSelector: GetConversationByIdType, + uuids: ReadonlyArray ): Array => { - const conversations = conversationIds - .map(conversationId => conversationSelector(conversationId)) - .filter(isNotNil); - if (conversationIds.length !== conversations.length) { - log.warn( - `getConversationsStoppingSend: Started with ${conversationIds.length} items, ended up with ${conversations.length}.` - ); - } + const conversations = uuids.map(uuid => conversationSelector(uuid)); return sortByTitle(conversations); } ); diff --git a/ts/test-both/state/selectors/conversations_test.ts b/ts/test-both/state/selectors/conversations_test.ts index 123b827446..625afb1b06 100644 --- a/ts/test-both/state/selectors/conversations_test.ts +++ b/ts/test-both/state/selectors/conversations_test.ts @@ -27,7 +27,7 @@ import { getComposeSelectedContacts, getContactNameColorSelector, getConversationByIdSelector, - getConversationIdsStoppingSend, + getConversationUuidsStoppingSend, getConversationIdsStoppedForVerification, getConversationsByTitleSelector, getConversationSelector, @@ -311,17 +311,17 @@ describe('both/state/selectors/conversations', () => { verificationDataByConversation: { 'convo a': { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification: ['abc'], + uuidsNeedingVerification: ['abc'], }, 'convo b': { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification: ['def', 'abc'], + uuidsNeedingVerification: ['def', 'abc'], }, }, }, }; - assert.sameDeepMembers(getConversationIdsStoppingSend(state), [ + assert.sameDeepMembers(getConversationUuidsStoppingSend(state), [ 'abc', 'def', ]); @@ -354,11 +354,11 @@ describe('both/state/selectors/conversations', () => { verificationDataByConversation: { 'convo a': { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification: ['abc'], + uuidsNeedingVerification: ['abc'], }, 'convo b': { type: ConversationVerificationState.PendingVerification as const, - conversationsNeedingVerification: ['def', 'abc'], + uuidsNeedingVerification: ['def', 'abc'], }, }, }, diff --git a/ts/test-electron/state/ducks/conversations_test.ts b/ts/test-electron/state/ducks/conversations_test.ts index 54612b1234..320f943174 100644 --- a/ts/test-electron/state/ducks/conversations_test.ts +++ b/ts/test-electron/state/ducks/conversations_test.ts @@ -798,28 +798,28 @@ describe('both/state/ducks/conversations', () => { getEmptyState(), conversationStoppedByMissingVerification({ conversationId: 'convo A', - untrustedConversationIds: ['convo 1'], + untrustedUuids: ['convo 1'], }) ); const second = reducer( first, conversationStoppedByMissingVerification({ conversationId: 'convo A', - untrustedConversationIds: ['convo 2'], + untrustedUuids: ['convo 2'], }) ); const third = reducer( second, conversationStoppedByMissingVerification({ conversationId: 'convo A', - untrustedConversationIds: ['convo 1', 'convo 3'], + untrustedUuids: ['convo 1', 'convo 3'], }) ); assert.deepStrictEqual(third.verificationDataByConversation, { 'convo A': { type: ConversationVerificationState.PendingVerification, - conversationsNeedingVerification: ['convo 1', 'convo 2', 'convo 3'], + uuidsNeedingVerification: ['convo 1', 'convo 2', 'convo 3'], }, }); }); @@ -838,14 +838,14 @@ describe('both/state/ducks/conversations', () => { state, conversationStoppedByMissingVerification({ conversationId: 'convo A', - untrustedConversationIds: ['convo 1', 'convo 2'], + untrustedUuids: ['convo 1', 'convo 2'], }) ); assert.deepStrictEqual(actual.verificationDataByConversation, { 'convo A': { type: ConversationVerificationState.PendingVerification, - conversationsNeedingVerification: ['convo 1', 'convo 2'], + uuidsNeedingVerification: ['convo 1', 'convo 2'], }, }); }); @@ -877,7 +877,7 @@ describe('both/state/ducks/conversations', () => { verificationDataByConversation: { 'convo A': { type: ConversationVerificationState.PendingVerification, - conversationsNeedingVerification: ['convo 1', 'convo 2'], + uuidsNeedingVerification: ['convo 1', 'convo 2'], }, }, }; @@ -971,7 +971,7 @@ describe('both/state/ducks/conversations', () => { verificationDataByConversation: { 'convo A': { type: ConversationVerificationState.PendingVerification, - conversationsNeedingVerification: ['convo 1', 'convo 2'], + uuidsNeedingVerification: ['convo 1', 'convo 2'], }, }, };