diff --git a/ts/background.ts b/ts/background.ts index 36195b3029..5ebf8dc811 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -146,7 +146,6 @@ import { showToast } from './util/showToast'; import { startInteractionMode } from './windows/startInteractionMode'; import type { MainWindowStatsType } from './windows/context'; import { deliveryReceiptsJobQueue } from './jobs/deliveryReceiptsJobQueue'; -import { updateOurUsernameAndPni } from './util/updateOurUsernameAndPni'; import { ReactionSource } from './reactions/ReactionSource'; import { singleProtoJobQueue } from './jobs/singleProtoJobQueue'; import { getInitialState } from './state/getInitialState'; @@ -2262,18 +2261,15 @@ export async function startApp(): Promise { try { // Note: we always have to register our capabilities all at once, so we do this // after connect on every startup - await Promise.all([ - server.registerCapabilities({ - announcementGroup: true, - giftBadges: true, - 'gv2-3': true, - senderKey: true, - changeNumber: true, - stories: true, - pni: isPnpEnabled(), - }), - updateOurUsernameAndPni(), - ]); + await server.registerCapabilities({ + announcementGroup: true, + giftBadges: true, + 'gv2-3': true, + senderKey: true, + changeNumber: true, + stories: true, + pni: isPnpEnabled(), + }); } catch (error) { log.error( 'Error: Unable to register our capabilities.', @@ -3532,10 +3528,7 @@ export async function startApp(): Promise { log.info('onFetchLatestSync: fetching latest local profile'); const ourUuid = window.textsecure.storage.user.getUuid()?.toString(); const ourE164 = window.textsecure.storage.user.getNumber(); - await Promise.all([ - getProfile(ourUuid, ourE164), - updateOurUsernameAndPni(), - ]); + await getProfile(ourUuid, ourE164); break; } case FETCH_LATEST_ENUM.STORAGE_MANIFEST: diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 12c0ce9e5d..ae568f1f7b 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -97,6 +97,7 @@ import { getProfileName, getTitle, getTitleNoDefault, + canHaveUsername, } from '../util/getTitle'; import { markConversationRead } from '../util/markConversationRead'; import { handleMessageSend } from '../util/handleMessageSend'; @@ -349,6 +350,10 @@ export class ConversationModel extends window.Backbone this.on('newmessage', this.onNewMessage); this.on('change:profileKey', this.onChangeProfileKey); + this.on( + 'change:name change:profileName change:profileFamilyName change:e164', + () => this.maybeClearUsername() + ); const sealedSender = this.get('sealedSender'); if (sealedSender === undefined) { @@ -1826,7 +1831,9 @@ export class ConversationModel extends window.Backbone // We had previously stored `null` instead of `undefined` in some cases. We should // be able to remove this `dropNull` once usernames have gone to production. - username: dropNull(this.get('username')), + username: canHaveUsername(this.attributes, ourConversationId) + ? dropNull(this.get('username')) + : undefined, about: this.getAboutText(), aboutText: this.get('about'), @@ -4218,6 +4225,50 @@ export class ConversationModel extends window.Backbone ); } + async maybeClearUsername(): Promise { + const ourConversationId = + window.ConversationController.getOurConversationId(); + + // Clear username once we have other information about the contact + if ( + canHaveUsername(this.attributes, ourConversationId) || + !this.get('username') + ) { + return; + } + + log.info(`maybeClearUsername(${this.idForLogging()}): clearing username`); + + this.unset('username'); + window.Signal.Data.updateConversation(this.attributes); + this.captureChange('clearUsername'); + } + + async updateUsername( + username: string | undefined, + { shouldSave = true }: { shouldSave?: boolean } = {} + ): Promise { + const ourConversationId = + window.ConversationController.getOurConversationId(); + + if (!canHaveUsername(this.attributes, ourConversationId)) { + return; + } + + if (this.get('username') === username) { + return; + } + + log.info(`updateUsername(${this.idForLogging()}): updating username`); + + this.set('username', username); + this.captureChange('updateUsername'); + + if (shouldSave) { + await window.Signal.Data.updateConversation(this.attributes); + } + } + async updateLastMessage(): Promise { if (!this.id) { return; diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index ff9297ea3b..afbd74c9bb 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -33,6 +33,7 @@ import { getSafeLongFromTimestamp, getTimestampFromLong, } from '../util/timestampLongUtils'; +import { canHaveUsername } from '../util/getTitle'; import { get as getUniversalExpireTimer, set as setUniversalExpireTimer, @@ -156,6 +157,11 @@ export async function toContactRecord( if (e164) { contactRecord.serviceE164 = e164; } + const username = conversation.get('username'); + const ourID = window.ConversationController.getOurConversationId(); + if (username && canHaveUsername(conversation.attributes, ourID)) { + contactRecord.username = username; + } const pni = conversation.get('pni'); if (pni && RemoteConfig.isEnabled('desktop.pnp')) { contactRecord.pni = pni; @@ -978,6 +984,10 @@ export async function mergeContactRecord( }; } + await conversation.updateUsername(dropNull(contactRecord.username), { + shouldSave: false, + }); + let needsProfileFetch = false; if (contactRecord.profileKey && contactRecord.profileKey.length > 0) { needsProfileFetch = await conversation.setProfileKey( diff --git a/ts/services/username.ts b/ts/services/username.ts index f9dbc1da7e..5b7537e1f8 100644 --- a/ts/services/username.ts +++ b/ts/services/username.ts @@ -2,8 +2,6 @@ // SPDX-License-Identifier: AGPL-3.0-only import { singleProtoJobQueue } from '../jobs/singleProtoJobQueue'; -import dataInterface from '../sql/Client'; -import { updateOurUsernameAndPni } from '../util/updateOurUsernameAndPni'; import { sleep } from '../util/sleep'; import type { UsernameReservationType } from '../types/Username'; import { ReserveUsernameError } from '../types/Username'; @@ -54,7 +52,6 @@ export async function reserveUsername( const { nickname, previousUsername, abortSignal } = options; const me = window.ConversationController.getOurConversationOrThrow(); - await updateOurUsernameAndPni(); if (me.get('username') !== previousUsername) { throw new Error('reserveUsername: Username has changed on another device'); @@ -96,8 +93,7 @@ async function updateUsernameAndSyncProfile( const me = window.ConversationController.getOurConversationOrThrow(); // Update backbone, update DB, then tell linked devices about profile update - me.set({ username }); - dataInterface.updateConversation(me.attributes); + await me.updateUsername(username); try { await singleProtoJobQueue.add( @@ -123,7 +119,6 @@ export async function confirmUsername( const { previousUsername, username, reservationToken } = reservation; const me = window.ConversationController.getOurConversationOrThrow(); - await updateOurUsernameAndPni(); if (me.get('username') !== previousUsername) { throw new Error('Username has changed on another device'); @@ -161,7 +156,6 @@ export async function deleteUsername( } const me = window.ConversationController.getOurConversationOrThrow(); - await updateOurUsernameAndPni(); if (me.get('username') !== previousUsername) { throw new Error('Username has changed on another device'); diff --git a/ts/test-mock/pnp/username_test.ts b/ts/test-mock/pnp/username_test.ts new file mode 100644 index 0000000000..415e3cc691 --- /dev/null +++ b/ts/test-mock/pnp/username_test.ts @@ -0,0 +1,137 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import { Proto, StorageState } from '@signalapp/mock-server'; +import type { PrimaryDevice } from '@signalapp/mock-server'; +import createDebug from 'debug'; + +import * as durations from '../../util/durations'; +import { uuidToBytes } from '../../util/uuidToBytes'; +import { MY_STORY_ID } from '../../types/Stories'; +import { Bootstrap } from '../bootstrap'; +import type { App } from '../bootstrap'; + +export const debug = createDebug('mock:test:username'); + +const IdentifierType = Proto.ManifestRecord.Identifier.Type; + +const USERNAME = 'signalapp.55'; + +describe('pnp/username', function needsName() { + this.timeout(durations.MINUTE); + + let bootstrap: Bootstrap; + let app: App; + let usernameContact: PrimaryDevice; + + beforeEach(async () => { + bootstrap = new Bootstrap({ contactCount: 0 }); + await bootstrap.init(); + + const { server, phone } = bootstrap; + + usernameContact = await server.createPrimaryDevice({ + profileName: 'ACI Contact', + }); + + let state = StorageState.getEmpty(); + + state = state.updateAccount({ + profileKey: phone.profileKey.serialize(), + e164: phone.device.number, + }); + + state = state.addContact(usernameContact, { + username: USERNAME, + serviceE164: undefined, + }); + + // Put contact into left pane + state = state.pin(usernameContact); + + // Add my story + state = state.addRecord({ + type: IdentifierType.STORY_DISTRIBUTION_LIST, + record: { + storyDistributionList: { + allowsReplies: true, + identifier: uuidToBytes(MY_STORY_ID), + isBlockList: true, + name: MY_STORY_ID, + recipientUuids: [], + }, + }, + }); + + await phone.setStorageState(state); + + app = await bootstrap.link(); + }); + + afterEach(async function after() { + if (this.currentTest?.state !== 'passed') { + await bootstrap.saveLogs(app); + } + + await app.close(); + await bootstrap.teardown(); + }); + + it('drops username when contact name becomes known', async () => { + const { phone } = bootstrap; + + const window = await app.getWindow(); + const leftPane = window.locator('.left-pane-wrapper'); + + debug('find username in the left pane'); + await leftPane + .locator( + `[data-testid="${usernameContact.device.uuid}"] >> "@${USERNAME}"` + ) + .waitFor(); + + debug('adding profile key for username contact'); + let state: StorageState = await phone.expectStorageState( + 'consistency check' + ); + state = state.updateContact(usernameContact, { + profileKey: usernameContact.profileKey.serialize(), + }); + await phone.setStorageState(state); + await phone.sendFetchStorage({ + timestamp: bootstrap.getTimestamp(), + }); + + debug('find profile name in the left pane'); + await leftPane + .locator( + `[data-testid="${usernameContact.device.uuid}"] >> ` + + `"${usernameContact.profileName}"` + ) + .waitFor(); + + debug('verify that storage service state is updated'); + { + const newState = await phone.waitForStorageState({ + after: state, + }); + + const { added, removed } = newState.diff(state); + assert.strictEqual(added.length, 1, 'only one record must be added'); + assert.strictEqual(removed.length, 1, 'only one record must be removed'); + + assert.strictEqual( + added[0].contact?.serviceUuid, + usernameContact.device.uuid + ); + assert.strictEqual(added[0].contact?.username, ''); + + assert.strictEqual( + removed[0].contact?.serviceUuid, + usernameContact.device.uuid + ); + assert.strictEqual(removed[0].contact?.username, USERNAME); + } + }); +}); diff --git a/ts/util/getTitle.ts b/ts/util/getTitle.ts index 5d7190bea3..7e2af8e78d 100644 --- a/ts/util/getTitle.ts +++ b/ts/util/getTitle.ts @@ -36,7 +36,7 @@ export function getTitleNoDefault( return ( (isShort ? attributes.systemGivenName : undefined) || - attributes.name || + getSystemName(attributes) || (isShort ? attributes.profileName : undefined) || getProfileName(attributes) || getNumber(attributes) || @@ -44,6 +44,30 @@ export function getTitleNoDefault( ); } +// Note that the used attributes field should match the ones we listen for +// change on in ConversationModel (see `ConversationModel#maybeClearUsername`) +export function canHaveUsername( + attributes: Pick< + ConversationAttributesType, + 'id' | 'type' | 'name' | 'profileName' | 'profileFamilyName' | 'e164' + >, + ourConversationId: string | undefined +): boolean { + if (!isDirectConversation(attributes)) { + return false; + } + + if (ourConversationId === attributes.id) { + return true; + } + + return ( + !getSystemName(attributes) && + !getProfileName(attributes) && + !getNumber(attributes) + ); +} + export function getProfileName( attributes: Pick< ConversationAttributesType, @@ -57,6 +81,22 @@ export function getProfileName( return undefined; } +export function getSystemName( + attributes: Pick< + ConversationAttributesType, + 'systemGivenName' | 'systemFamilyName' | 'type' + > +): string | undefined { + if (isDirectConversation(attributes)) { + return combineNames( + attributes.systemGivenName, + attributes.systemFamilyName + ); + } + + return undefined; +} + export function getNumber( attributes: Pick ): string { diff --git a/ts/util/lookupConversationWithoutUuid.ts b/ts/util/lookupConversationWithoutUuid.ts index 719287ae96..76c89a2221 100644 --- a/ts/util/lookupConversationWithoutUuid.ts +++ b/ts/util/lookupConversationWithoutUuid.ts @@ -97,7 +97,7 @@ export async function lookupConversationWithoutUuid( conversationId = convo.id; - convo.set({ username: foundUsername.username }); + await convo.updateUsername(foundUsername.username); } } diff --git a/ts/util/updateOurUsernameAndPni.ts b/ts/util/updateOurUsernameAndPni.ts deleted file mode 100644 index 3675053bc0..0000000000 --- a/ts/util/updateOurUsernameAndPni.ts +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2021 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import { strictAssert } from './assert'; -import { dropNull } from './dropNull'; - -export async function updateOurUsernameAndPni(): Promise { - const { server } = window.textsecure; - - strictAssert( - server, - 'updateOurUsernameAndPni: window.textsecure.server not available' - ); - - const me = window.ConversationController.getOurConversationOrThrow(); - const { username } = await server.whoami(); - - me.set({ username: dropNull(username) }); - window.Signal.Data.updateConversation(me.attributes); - - const manager = window.getAccountManager(); - strictAssert( - manager, - 'updateOurUsernameAndPni: AccountManager not available' - ); -}