From 1b7616b4db8872017b266791b0f86415d2072f2c Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 6 Feb 2024 09:47:12 -0500 Subject: [PATCH] Fix issue with PNIs in contact sync. --- .../securesms/database/RecipientTable.kt | 2 +- .../jobs/MultiDeviceContactSyncJob.kt | 6 ++++- .../jobs/MultiDeviceContactUpdateJob.java | 14 ++++++++--- .../jobs/MultiDeviceProfileKeyUpdateJob.java | 4 +++- .../messages/multidevice/DeviceContact.java | 23 +++++++++++++++---- .../DeviceContactsInputStream.java | 6 +++-- .../DeviceContactsOutputStream.java | 8 ++++--- .../DeviceContactsInputStreamTest.java | 22 +++++++++++------- 8 files changed, 61 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index c94342e1b5..9158a5f5e8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -3458,7 +3458,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da fun getRecipientsForMultiDeviceSync(): List { val subquery = "SELECT ${ThreadTable.TABLE_NAME}.${ThreadTable.RECIPIENT_ID} FROM ${ThreadTable.TABLE_NAME}" - val selection = "$REGISTERED = ? AND $GROUP_ID IS NULL AND $ID != ? AND ($SYSTEM_CONTACT_URI NOT NULL OR $ID IN ($subquery))" + val selection = "$REGISTERED = ? AND $GROUP_ID IS NULL AND $ID != ? AND ($ACI_COLUMN NOT NULL OR $E164 NOT NULL) AND ($SYSTEM_CONTACT_URI NOT NULL OR $ID IN ($subquery))" val args = arrayOf(RegisteredState.REGISTERED.id.toString(), Recipient.self().id.serialize()) val recipients: MutableList = ArrayList() diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactSyncJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactSyncJob.kt index eae7ff23bd..59fbd5c3dd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactSyncJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactSyncJob.kt @@ -76,7 +76,11 @@ class MultiDeviceContactSyncJob(parameters: Parameters, private val attachmentPo var contact: DeviceContact? = deviceContacts.read() while (contact != null) { - val recipient = Recipient.externalPush(SignalServiceAddress(contact.address.serviceId, contact.address.number.orElse(null))) + val recipient = if (contact.aci.isPresent) { + Recipient.externalPush(SignalServiceAddress(contact.aci.get(), contact.e164.orElse(null))) + } else { + Recipient.external(context, contact.e164.get()) + } if (recipient.isSelf) { contact = deviceContacts.read() diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactUpdateJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactUpdateJob.java index 90ada93c0c..721c06a87a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactUpdateJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceContactUpdateJob.java @@ -151,12 +151,18 @@ public class MultiDeviceContactUpdateJob extends BaseJob { return; } + if (!recipient.hasE164() && !recipient.hasAci()) { + Log.w(TAG, recipientId + " has no valid identifier!"); + return; + } + Optional identityRecord = ApplicationDependencies.getProtocolStore().aci().identities().getIdentityRecord(recipient.getId()); Optional verifiedMessage = getVerifiedMessage(recipient, identityRecord); Map inboxPositions = SignalDatabase.threads().getInboxPositions(); Set archived = SignalDatabase.threads().getArchivedRecipients(); - out.write(new DeviceContact(RecipientUtil.toSignalServiceAddress(context, recipient), + out.write(new DeviceContact(recipient.getAci(), + recipient.getE164(), Optional.ofNullable(recipient.isGroup() || recipient.isSystemContact() ? recipient.getDisplayName(context) : null), getSystemAvatar(recipient.getContactUri()), Optional.of(ChatColorsMapper.getMaterialColor(recipient.getChatColors()).serialize()), @@ -222,7 +228,8 @@ public class MultiDeviceContactUpdateJob extends BaseJob { Optional expireTimer = recipient.getExpiresInSeconds() > 0 ? Optional.of(recipient.getExpiresInSeconds()) : Optional.empty(); Optional inboxPosition = Optional.ofNullable(inboxPositions.get(recipient.getId())); - out.write(new DeviceContact(RecipientUtil.toSignalServiceAddress(context, recipient), + out.write(new DeviceContact(recipient.getAci(), + recipient.getE164(), name, getSystemAvatar(recipient.getContactUri()), Optional.of(ChatColorsMapper.getMaterialColor(recipient.getChatColors()).serialize()), @@ -239,7 +246,8 @@ public class MultiDeviceContactUpdateJob extends BaseJob { byte[] profileKey = self.getProfileKey(); if (profileKey != null) { - out.write(new DeviceContact(RecipientUtil.toSignalServiceAddress(context, self), + out.write(new DeviceContact(Optional.of(SignalStore.account().getAci()), + Optional.of(SignalStore.account().getE164()), Optional.empty(), Optional.empty(), Optional.of(ChatColorsMapper.getMaterialColor(self.getChatColors()).serialize()), diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceProfileKeyUpdateJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceProfileKeyUpdateJob.java index 20fee51b76..5b3082b05a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceProfileKeyUpdateJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceProfileKeyUpdateJob.java @@ -11,6 +11,7 @@ import org.thoughtcrime.securesms.crypto.UnidentifiedAccessUtil; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.jobmanager.Job; import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint; +import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.net.NotPushRegisteredException; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientUtil; @@ -76,7 +77,8 @@ public class MultiDeviceProfileKeyUpdateJob extends BaseJob { ByteArrayOutputStream baos = new ByteArrayOutputStream(); DeviceContactsOutputStream out = new DeviceContactsOutputStream(baos); - out.write(new DeviceContact(RecipientUtil.toSignalServiceAddress(context, Recipient.self()), + out.write(new DeviceContact(Optional.ofNullable(SignalStore.account().getAci()), + Optional.ofNullable(SignalStore.account().getE164()), Optional.empty(), Optional.empty(), Optional.empty(), diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContact.java b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContact.java index b80387f470..e977f6ddff 100644 --- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContact.java +++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContact.java @@ -8,13 +8,16 @@ package org.whispersystems.signalservice.api.messages.multidevice; import org.signal.libsignal.zkgroup.profiles.ProfileKey; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentStream; +import org.whispersystems.signalservice.api.push.ServiceId; +import org.whispersystems.signalservice.api.push.ServiceId.ACI; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import java.util.Optional; public class DeviceContact { - private final SignalServiceAddress address; + private final Optional aci; + private final Optional e164; private final Optional name; private final Optional avatar; private final Optional color; @@ -25,7 +28,8 @@ public class DeviceContact { private final Optional inboxPosition; private final boolean archived; - public DeviceContact(SignalServiceAddress address, + public DeviceContact(Optional aci, + Optional e164, Optional name, Optional avatar, Optional color, @@ -36,7 +40,12 @@ public class DeviceContact { Optional inboxPosition, boolean archived) { - this.address = address; + if (aci.isEmpty() && e164.isEmpty()) { + throw new IllegalArgumentException("Must have either ACI or E164"); + } + + this.aci = aci; + this.e164 = e164; this.name = name; this.avatar = avatar; this.color = color; @@ -56,8 +65,12 @@ public class DeviceContact { return name; } - public SignalServiceAddress getAddress() { - return address; + public Optional getAci() { + return aci; + } + + public Optional getE164() { + return e164; } public Optional getColor() { diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStream.java b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStream.java index 4cec5a0099..bda562959a 100644 --- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStream.java +++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStream.java @@ -14,6 +14,7 @@ import org.signal.libsignal.zkgroup.InvalidInputException; import org.signal.libsignal.zkgroup.profiles.ProfileKey; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentStream; import org.whispersystems.signalservice.api.push.ServiceId; +import org.whispersystems.signalservice.api.push.ServiceId.ACI; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.internal.push.ContactDetails; import org.whispersystems.signalservice.internal.util.Util; @@ -45,7 +46,8 @@ public class DeviceContactsInputStream extends ChunkedInputStream { throw new IOException("Missing contact address!"); } - SignalServiceAddress address = new SignalServiceAddress(ServiceId.parseOrThrow(details.aci), details.number); + Optional aci = Optional.ofNullable(ACI.parseOrNull(details.aci)); + Optional e164 = Optional.ofNullable(details.number); Optional name = Optional.ofNullable(details.name); Optional avatar = Optional.empty(); Optional color = details.color != null ? Optional.of(details.color) : Optional.empty(); @@ -108,7 +110,7 @@ public class DeviceContactsInputStream extends ChunkedInputStream { blocked = details.blocked; archived = details.archived; - return new DeviceContact(address, name, avatar, color, verified, profileKey, blocked, expireTimer, inboxPosition, archived); + return new DeviceContact(aci, e164, name, avatar, color, verified, profileKey, blocked, expireTimer, inboxPosition, archived); } } diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsOutputStream.java b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsOutputStream.java index 3c32793338..4ffd7708ae 100644 --- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsOutputStream.java +++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsOutputStream.java @@ -38,10 +38,12 @@ public class DeviceContactsOutputStream extends ChunkedOutputStream { private void writeContactDetails(DeviceContact contact) throws IOException { ContactDetails.Builder contactDetails = new ContactDetails.Builder(); - contactDetails.aci(contact.getAddress().getServiceId().toString()); + if (contact.getAci().isPresent()) { + contactDetails.aci(contact.getAci().get().toString()); + } - if (contact.getAddress().getNumber().isPresent()) { - contactDetails.number(contact.getAddress().getNumber().get()); + if (contact.getE164().isPresent()) { + contactDetails.number(contact.getE164().get()); } if (contact.getName().isPresent()) { diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStreamTest.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStreamTest.java index b4a4a35800..66e578263f 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStreamTest.java +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/messages/multidevice/DeviceContactsInputStreamTest.java @@ -25,15 +25,18 @@ public class DeviceContactsInputStreamTest { public void read() throws IOException, InvalidInputException { ByteArrayOutputStream byteArrayOut = new ByteArrayOutputStream(); DeviceContactsOutputStream output = new DeviceContactsOutputStream(byteArrayOut); - SignalServiceAddress addressFirst = new SignalServiceAddress(ACI.from(UUID.randomUUID()), "+1404555555"); - SignalServiceAddress addressSecond = new SignalServiceAddress(ACI.from(UUID.randomUUID()), "+1444555555"); + Optional aciFirst = Optional.of(ACI.from(UUID.randomUUID())); + Optional e164First = Optional.of("+1404555555"); + Optional aciSecond = Optional.of(ACI.from(UUID.randomUUID())); + Optional e164Second = Optional.of("+1444555555"); DeviceContact first = new DeviceContact( - addressFirst, + aciFirst, + e164First, Optional.of("Teal'c"), Optional.empty(), Optional.of("ultramarine"), - Optional.of(new VerifiedMessage(addressFirst, generateIdentityKey(), VerifiedMessage.VerifiedState.DEFAULT, System.currentTimeMillis())), + Optional.of(new VerifiedMessage(new SignalServiceAddress(aciFirst.get(), e164First), generateIdentityKey(), VerifiedMessage.VerifiedState.DEFAULT, System.currentTimeMillis())), Optional.of(generateProfileKey()), false, Optional.of(0), @@ -42,11 +45,12 @@ public class DeviceContactsInputStreamTest { ); DeviceContact second = new DeviceContact( - addressSecond, + aciSecond, + e164Second, Optional.of("Bra'tac"), Optional.empty(), Optional.of("ultramarine"), - Optional.of(new VerifiedMessage(addressSecond, generateIdentityKey(), VerifiedMessage.VerifiedState.DEFAULT, System.currentTimeMillis())), + Optional.of(new VerifiedMessage(new SignalServiceAddress(aciSecond.get(), e164Second), generateIdentityKey(), VerifiedMessage.VerifiedState.DEFAULT, System.currentTimeMillis())), Optional.of(generateProfileKey()), false, Optional.of(0), @@ -65,13 +69,15 @@ public class DeviceContactsInputStreamTest { DeviceContact readFirst = input.read(); DeviceContact readSecond = input.read(); - assertEquals(first.getAddress(), readFirst.getAddress()); + assertEquals(first.getAci(), readFirst.getAci()); + assertEquals(first.getE164(), readFirst.getE164()); assertEquals(first.getName(), readFirst.getName()); assertEquals(first.getColor(), readFirst.getColor()); assertEquals(first.getVerified().get().getIdentityKey(), readFirst.getVerified().get().getIdentityKey()); assertEquals(first.isArchived(), readFirst.isArchived()); - assertEquals(second.getAddress(), readSecond.getAddress()); + assertEquals(second.getAci(), readSecond.getAci()); + assertEquals(second.getE164(), readSecond.getE164()); assertEquals(second.getName(), readSecond.getName()); assertEquals(second.getColor(), readSecond.getColor()); assertEquals(second.getVerified().get().getIdentityKey(), readSecond.getVerified().get().getIdentityKey());