diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt index 68c1d7bad..7c93ea2bb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt @@ -523,6 +523,27 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : @VisibleForTesting fun getAndPossiblyMergePnp(serviceId: ServiceId?, e164: String?, changeSelf: Boolean = false): RecipientId { require(!(serviceId == null && e164 == null)) { "Must provide an ACI or E164!" } + return getAndPossiblyMergePnp(serviceId = serviceId, pni = null, e164 = e164, pniVerified = false, changeSelf = changeSelf) + } + + /** + * Gets and merges a (serviceId, pni, e164) tuple, doing merges/updates as needed, and giving you back the final RecipientId. + * It is assumed that the tuple is verified. Do not give this method an untrusted association. + */ + fun getAndPossiblyMergePnpVerified(serviceId: ServiceId?, pni: PNI?, e164: String?): RecipientId { + if (!FeatureFlags.phoneNumberPrivacy()) { + throw AssertionError() + } + + return getAndPossiblyMergePnp(serviceId = serviceId, pni = pni, e164 = e164, pniVerified = true, changeSelf = false) + } + + private fun getAndPossiblyMergePnp(serviceId: ServiceId?, pni: PNI?, e164: String?, pniVerified: Boolean = false, changeSelf: Boolean = false): RecipientId { + require(!(serviceId == null && e164 == null)) { "Must provide an ACI or E164!" } + + if ((serviceId is PNI) && pni != null && serviceId != pni) { + throw AssertionError("Provided two non-matching PNIs! serviceId: $serviceId, pni: $pni") + } val db = writableDatabase var transactionSuccessful = false @@ -531,11 +552,13 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : db.beginTransaction() try { result = when { - serviceId is PNI -> processPnpTuple(e164, serviceId, null, pniVerified = false, changeSelf = changeSelf) - serviceId is ACI -> processPnpTuple(e164, null, serviceId, pniVerified = false, changeSelf = changeSelf) - serviceId == null -> processPnpTuple(e164, null, null, pniVerified = false, changeSelf = changeSelf) - getByPni(PNI.from(serviceId.uuid())).isPresent -> processPnpTuple(e164, PNI.from(serviceId.uuid()), null, pniVerified = false, changeSelf = changeSelf) - else -> processPnpTuple(e164, null, ACI.fromNullable(serviceId), pniVerified = false, changeSelf = changeSelf) + serviceId is ACI -> processPnpTuple(e164 = e164, pni = pni, aci = serviceId, pniVerified = pniVerified, changeSelf = changeSelf) + serviceId is PNI -> processPnpTuple(e164 = e164, pni = serviceId, aci = null, pniVerified = pniVerified, changeSelf = changeSelf) + serviceId == null -> processPnpTuple(e164 = e164, pni = pni, aci = null, pniVerified = pniVerified, changeSelf = changeSelf) + serviceId == pni -> processPnpTuple(e164 = e164, pni = pni, aci = null, pniVerified = pniVerified, changeSelf = changeSelf) + pni != null -> processPnpTuple(e164 = e164, pni = pni, aci = ACI.from(serviceId.uuid()), pniVerified = pniVerified, changeSelf = changeSelf) + getByPni(PNI.from(serviceId.uuid())).isPresent -> processPnpTuple(e164 = e164, pni = PNI.from(serviceId.uuid()), aci = null, pniVerified = pniVerified, changeSelf = changeSelf) + else -> processPnpTuple(e164 = e164, pni = pni, aci = ACI.fromNullable(serviceId), pniVerified = pniVerified, changeSelf = changeSelf) } if (result.operations.isNotEmpty()) { @@ -917,16 +940,20 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : val recipientId: RecipientId if (id < 0) { Log.w(TAG, "[applyStorageSyncContactInsert] Failed to insert. Possibly merging.") - recipientId = getAndPossiblyMerge(if (insert.address.hasValidServiceId()) insert.address.serviceId else null, insert.address.number.orElse(null)) + if (FeatureFlags.phoneNumberPrivacy()) { + recipientId = getAndPossiblyMerge(if (insert.serviceId.isValid) insert.serviceId else null, insert.number.orElse(null)) + } else { + recipientId = getAndPossiblyMergePnpVerified(if (insert.serviceId.isValid) insert.serviceId else null, insert.pni.orElse(null), insert.number.orElse(null)) + } db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId)) } else { recipientId = RecipientId.from(id) } - if (insert.identityKey.isPresent && insert.address.hasValidServiceId()) { + if (insert.identityKey.isPresent && insert.serviceId.isValid) { try { val identityKey = IdentityKey(insert.identityKey.get(), 0) - identities.updateIdentityAfterSync(insert.address.identifier, recipientId, identityKey, StorageSyncModels.remoteToLocalIdentityStatus(insert.identityState)) + identities.updateIdentityAfterSync(insert.serviceId.toString(), recipientId, identityKey, StorageSyncModels.remoteToLocalIdentityStatus(insert.identityState)) } catch (e: InvalidKeyException) { Log.w(TAG, "Failed to process identity key during insert! Skipping.", e) } @@ -954,7 +981,11 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : var recipientId = getByColumn(STORAGE_SERVICE_ID, Base64.encodeBytes(update.old.id.raw)).get() Log.w(TAG, "[applyStorageSyncContactUpdate] Found user $recipientId. Possibly merging.") - recipientId = getAndPossiblyMerge(if (update.new.address.hasValidServiceId()) update.new.address.serviceId else null, update.new.address.number.orElse(null)) + if (FeatureFlags.phoneNumberPrivacy()) { + recipientId = getAndPossiblyMergePnpVerified(if (update.new.serviceId.isValid) update.new.serviceId else null, update.new.pni.orElse(null), update.new.number.orElse(null)) + } else { + recipientId = getAndPossiblyMerge(if (update.new.serviceId.isValid) update.new.serviceId else null, update.new.number.orElse(null)) + } Log.w(TAG, "[applyStorageSyncContactUpdate] Merged into $recipientId") db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId)) @@ -970,9 +1001,9 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : try { val oldIdentityRecord = identityStore.getIdentityRecord(recipientId) - if (update.new.identityKey.isPresent && update.new.address.hasValidServiceId()) { + if (update.new.identityKey.isPresent && update.new.serviceId.isValid) { val identityKey = IdentityKey(update.new.identityKey.get(), 0) - identities.updateIdentityAfterSync(update.new.address.identifier, recipientId, identityKey, StorageSyncModels.remoteToLocalIdentityStatus(update.new.identityState)) + identities.updateIdentityAfterSync(update.new.serviceId.toString(), recipientId, identityKey, StorageSyncModels.remoteToLocalIdentityStatus(update.new.identityState)) } val newIdentityRecord = identityStore.getIdentityRecord(recipientId) @@ -3564,11 +3595,15 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : val profileName = ProfileName.fromParts(contact.givenName.orElse(null), contact.familyName.orElse(null)) val username = contact.username.orElse(null) - if (contact.address.hasValidServiceId()) { - put(SERVICE_ID, contact.address.serviceId.toString()) + if (contact.serviceId.isValid) { + put(SERVICE_ID, contact.serviceId.toString()) } - put(PHONE, contact.address.number.orElse(null)) + if (FeatureFlags.phoneNumberPrivacy()) { + put(PNI_COLUMN, contact.pni.toString()) + } + + put(PHONE, contact.number.orElse(null)) put(PROFILE_GIVEN_NAME, profileName.givenName) put(PROFILE_FAMILY_NAME, profileName.familyName) put(PROFILE_JOINED_NAME, profileName.toString()) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java index 05f6b787d..b2324578d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java @@ -391,12 +391,10 @@ public class StorageSyncJob extends BaseJob { } private static void processKnownRecords(@NonNull Context context, @NonNull StorageRecordCollection records) throws IOException { - Recipient self = freshSelf(); - new ContactRecordProcessor(context, self).process(records.contacts, StorageSyncHelper.KEY_GENERATOR); + new ContactRecordProcessor().process(records.contacts, StorageSyncHelper.KEY_GENERATOR); new GroupV1RecordProcessor(context).process(records.gv1, StorageSyncHelper.KEY_GENERATOR); new GroupV2RecordProcessor(context).process(records.gv2, StorageSyncHelper.KEY_GENERATOR); - self = freshSelf(); - new AccountRecordProcessor(context, self).process(records.account, StorageSyncHelper.KEY_GENERATOR); + new AccountRecordProcessor(context, freshSelf()).process(records.account, StorageSyncHelper.KEY_GENERATOR); if (getKnownTypes().contains(ManifestRecord.Identifier.Type.STORY_DISTRIBUTION_LIST_VALUE)) { new StoryDistributionListRecordProcessor().process(records.storyDistributionLists, StorageSyncHelper.KEY_GENERATOR); diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java index a8ae572a7..fb2356144 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/Recipient.java @@ -238,6 +238,39 @@ public class Recipient { return externalPush(serviceId, null); } + /** + * Create a recipient with a full (ACI, PNI, E164) tuple. It is assumed that the association between the PNI and serviceId is trusted. + * That means it must be from either storage service or a PNI verification message. + */ + public static @NonNull Recipient trustedPush(@NonNull ServiceId serviceId, @Nullable PNI pni, @Nullable String e164) { + if (ServiceId.UNKNOWN.equals(serviceId)) { + throw new AssertionError("Unknown serviceId!"); + } + + RecipientDatabase db = SignalDatabase.recipients(); + + RecipientId recipientId; + + if (FeatureFlags.phoneNumberPrivacy()) { + recipientId = db.getAndPossiblyMergePnpVerified(serviceId, pni, e164); + } else { + recipientId = db.getAndPossiblyMerge(serviceId, e164); + } + + Recipient resolved = resolved(recipientId); + + if (!resolved.getId().equals(recipientId)) { + Log.w(TAG, "Resolved " + recipientId + ", but got back a recipient with " + resolved.getId()); + } + + if (!resolved.isRegistered()) { + Log.w(TAG, "External push was locally marked unregistered. Marking as registered."); + db.markRegistered(recipientId, serviceId); + } + + return resolved; + } + /** * Returns a fully-populated {@link Recipient} based off of a ServiceId and phone number, creating one * in the database if necessary. We want both piece of information so we're able to associate them diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java index f4192956a..4f06de2e4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java @@ -13,6 +13,7 @@ import org.thoughtcrime.securesms.jobs.RetrieveProfileJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; +import org.thoughtcrime.securesms.util.FeatureFlags; import org.whispersystems.signalservice.api.push.ACI; import org.whispersystems.signalservice.api.push.PNI; import org.whispersystems.signalservice.api.push.ServiceId; @@ -29,23 +30,24 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor getMatching(@NonNull SignalContactRecord remote, @NonNull StorageKeyGenerator keyGenerator) { - SignalServiceAddress address = remote.getAddress(); - Optional byUuid = recipientDatabase.getByServiceId(address.getServiceId()); - Optional byE164 = address.getNumber().isPresent() ? recipientDatabase.getByE164(address.getNumber().get()) : Optional.empty(); + if (!FeatureFlags.phoneNumberPrivacy()) { + remote = remote.withoutPni(); + } - return OptionalUtil.or(byUuid, byE164) - .map(recipientDatabase::getRecordForSync) - .map(settings -> { - if (settings.getStorageId() != null) { - return StorageSyncModels.localToRemoteRecord(settings); - } else { - Log.w(TAG, "Newly discovering a registered user via storage service. Saving a storageId for them."); - recipientDatabase.updateStorageId(settings.getId(), keyGenerator.generate()); + Optional found = recipientDatabase.getByServiceId(remote.getServiceId()); - RecipientRecord updatedSettings = Objects.requireNonNull(recipientDatabase.getRecordForSync(settings.getId())); - return StorageSyncModels.localToRemoteRecord(updatedSettings); - } - }) - .map(r -> r.getContact().get()); + if (!found.isPresent() && remote.getNumber().isPresent()) { + found = recipientDatabase.getByE164(remote.getNumber().get()); + } + + if (!found.isPresent() && remote.getPni().isPresent()) { + found = recipientDatabase.getByServiceId(remote.getPni().get()); + } + + if (!found.isPresent() && remote.getPni().isPresent()) { + found = recipientDatabase.getByPni(remote.getPni().get()); + } + + return found.map(recipientDatabase::getRecordForSync) + .map(settings -> { + if (settings.getStorageId() != null) { + return StorageSyncModels.localToRemoteRecord(settings); + } else { + Log.w(TAG, "Newly discovering a registered user via storage service. Saving a storageId for them."); + recipientDatabase.updateStorageId(settings.getId(), keyGenerator.generate()); + + RecipientRecord updatedSettings = Objects.requireNonNull(recipientDatabase.getRecordForSync(settings.getId())); + return StorageSyncModels.localToRemoteRecord(updatedSettings); + } + }) + .map(r -> r.getContact().get()); } @Override @NonNull SignalContactRecord merge(@NonNull SignalContactRecord remote, @NonNull SignalContactRecord local, @NonNull StorageKeyGenerator keyGenerator) { + if (!FeatureFlags.phoneNumberPrivacy()) { + local = local.withoutPni(); + remote = remote.withoutPni(); + } + String givenName; String familyName; @@ -125,14 +147,47 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor + + @Test + fun `isInvalid, normal, false`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceId(ACI_B.toString()) + setServicePni(PNI_B.toString()) + setServiceE164(E164_B) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertFalse(result) + } + + @Test + fun `isInvalid, missing serviceId, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceE164(E164_B) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, e164 matches self, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceId(ACI_B.toString()) + setServiceE164(E164_A) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, aci matches self, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceId(ACI_A.toString()) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, pni matches self as serviceId, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceId(PNI_A.toString()) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, pni matches self as pni, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + val record = buildRecord { + setServiceId(ACI_B.toString()) + setServicePni(PNI_A.toString()) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, pniOnly pnpDisabled, true`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(false) + + val record = buildRecord { + setServiceId(PNI_B.toString()) + setServicePni(PNI_B.toString()) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertTrue(result) + } + + @Test + fun `isInvalid, pniOnly pnpEnabled, false`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(true) + + val record = buildRecord { + setServiceId(PNI_B.toString()) + setServicePni(PNI_B.toString()) + } + + // WHEN + val result = subject.isInvalid(record) + + // THEN + assertFalse(result) + } + + @Test + fun `merge, e164MatchesButPnisDont pnpEnabled, keepLocal`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(true) + + val local = buildRecord(STORAGE_ID_A) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_A) + setServicePni(PNI_A.toString()) + } + + val remote = buildRecord(STORAGE_ID_B) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_A) + setServicePni(PNI_B.toString()) + } + + // WHEN + val result = subject.merge(remote, local, TestKeyGenerator(STORAGE_ID_C)) + + // THEN + assertEquals(local.serviceId, result.serviceId) + assertEquals(local.number.get(), result.number.get()) + assertEquals(local.pni.get(), result.pni.get()) + } + + @Test + fun `merge, pnisMatchButE164sDont pnpEnabled, keepLocal`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(true) + + val local = buildRecord(STORAGE_ID_A) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_A) + setServicePni(PNI_A.toString()) + } + + val remote = buildRecord(STORAGE_ID_B) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_B) + setServicePni(PNI_A.toString()) + } + + // WHEN + val result = subject.merge(remote, local, TestKeyGenerator(STORAGE_ID_C)) + + // THEN + assertEquals(local.serviceId, result.serviceId) + assertEquals(local.number.get(), result.number.get()) + assertEquals(local.pni.get(), result.pni.get()) + } + + @Test + fun `merge, e164AndPniChange pnpEnabled, useRemote`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(true) + + val local = buildRecord(STORAGE_ID_A) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_A) + setServicePni(PNI_A.toString()) + } + + val remote = buildRecord(STORAGE_ID_B) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_B) + setServicePni(PNI_B.toString()) + } + + // WHEN + val result = subject.merge(remote, local, TestKeyGenerator(STORAGE_ID_C)) + + // THEN + assertEquals(remote.serviceId, result.serviceId) + assertEquals(remote.number.get(), result.number.get()) + assertEquals(remote.pni.get(), result.pni.get()) + } + + @Test + fun `merge, pnpDisabled, pniDropped`() { + // GIVEN + val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientDatabase) + + featureFlags.`when` { FeatureFlags.phoneNumberPrivacy() }.thenReturn(false) + + val local = buildRecord(STORAGE_ID_A) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_A) + setServicePni(PNI_A.toString()) + } + + val remote = buildRecord(STORAGE_ID_B) { + setServiceId(ACI_A.toString()) + setServiceE164(E164_B) + setServicePni(PNI_B.toString()) + } + + // WHEN + val result = subject.merge(remote, local, TestKeyGenerator(STORAGE_ID_C)) + + // THEN + assertEquals(remote.serviceId, result.serviceId) + assertEquals(remote.number.get(), result.number.get()) + assertEquals(false, result.pni.isPresent) + } + + private fun buildRecord(id: StorageId = STORAGE_ID_A, applyParams: ContactRecord.Builder.() -> ContactRecord.Builder): SignalContactRecord { + return SignalContactRecord(id, ContactRecord.getDefaultInstance().toBuilder().applyParams().build()) + } + + private class TestKeyGenerator(private val value: StorageId) : StorageKeyGenerator { + override fun generate(): ByteArray { + return value.raw + } + } + + companion object { + val STORAGE_ID_A: StorageId = StorageId.forStoryDistributionList(byteArrayOf(1, 2, 3, 4)) + val STORAGE_ID_B: StorageId = StorageId.forStoryDistributionList(byteArrayOf(5, 6, 7, 8)) + val STORAGE_ID_C: StorageId = StorageId.forStoryDistributionList(byteArrayOf(5, 6, 7, 8)) + + val ACI_A = ACI.from(UUID.fromString("3436efbe-5a76-47fa-a98a-7e72c948a82e")) + val ACI_B = ACI.from(UUID.fromString("8de7f691-0b60-4a68-9cd9-ed2f8453f9ed")) + + val PNI_A = PNI.from(UUID.fromString("154b8d92-c960-4f6c-8385-671ad2ffb999")) + val PNI_B = PNI.from(UUID.fromString("ba92b1fb-cd55-40bf-adda-c35a85375533")) + + const val E164_A = "+12221234567" + const val E164_B = "+13331234567" + + @JvmStatic + @BeforeClass + fun setUpClass() { + Log.initialize(EmptyLogger()) + } + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java b/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java index 036fb715c..a422cacb8 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java @@ -176,7 +176,8 @@ public final class StorageSyncHelperTest { String e164, String profileName) { - return new SignalContactRecord.Builder(byteArray(key), new SignalServiceAddress(aci, e164), null) + return new SignalContactRecord.Builder(byteArray(key), aci, null) + .setE164(e164) .setGivenName(profileName); } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/ServiceId.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/ServiceId.java index d26636282..9ca19500d 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/ServiceId.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/ServiceId.java @@ -78,6 +78,10 @@ public class ServiceId { return uuid.equals(UNKNOWN.uuid); } + public boolean isValid() { + return !isUnknown(); + } + public SignalProtocolAddress toProtocolAddress(int deviceId) { return new SignalProtocolAddress(uuid.toString(), deviceId); } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java index bcdf535dc..1697ff1de 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalContactRecord.java @@ -4,6 +4,7 @@ import com.google.protobuf.ByteString; import com.google.protobuf.InvalidProtocolBufferException; import org.signal.libsignal.protocol.logging.Log; +import org.whispersystems.signalservice.api.push.PNI; import org.whispersystems.signalservice.api.push.ServiceId; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.util.OptionalUtil; @@ -25,19 +26,23 @@ public final class SignalContactRecord implements SignalRecord { private final ContactRecord proto; private final boolean hasUnknownFields; - private final SignalServiceAddress address; - private final Optional givenName; - private final Optional familyName; - private final Optional profileKey; - private final Optional username; - private final Optional identityKey; + private final ServiceId serviceId; + private final Optional pni; + private final Optional e164; + private final Optional givenName; + private final Optional familyName; + private final Optional profileKey; + private final Optional username; + private final Optional identityKey; public SignalContactRecord(StorageId id, ContactRecord proto) { this.id = id; this.proto = proto; this.hasUnknownFields = ProtoUtil.hasUnknownFields(proto); - this.address = new SignalServiceAddress(ServiceId.parseOrUnknown(proto.getServiceUuid()), proto.getServiceE164()); + this.serviceId = ServiceId.parseOrUnknown(proto.getServiceId()); + this.pni = OptionalUtil.absentIfEmpty(proto.getServicePni()).map(PNI::parseOrNull); + this.e164 = OptionalUtil.absentIfEmpty(proto.getServiceE164()); this.givenName = OptionalUtil.absentIfEmpty(proto.getGivenName()); this.familyName = OptionalUtil.absentIfEmpty(proto.getFamilyName()); this.profileKey = OptionalUtil.absentIfEmpty(proto.getProfileKey()); @@ -65,12 +70,16 @@ public final class SignalContactRecord implements SignalRecord { diff.add("ID"); } - if (!Objects.equals(this.getAddress().getNumber(), that.getAddress().getNumber())) { - diff.add("E164"); + if (!Objects.equals(this.getServiceId(), that.getServiceId())) { + diff.add("ServiceId"); } - if (!Objects.equals(this.getAddress().getServiceId(), that.getAddress().getServiceId())) { - diff.add("UUID"); + if (!Objects.equals(this.getPni(), that.getPni())) { + diff.add("PNI"); + } + + if (!Objects.equals(this.getNumber(), that.getNumber())) { + diff.add("E164"); } if (!Objects.equals(this.givenName, that.givenName)) { @@ -139,8 +148,16 @@ public final class SignalContactRecord implements SignalRecord { return hasUnknownFields ? proto.toByteArray() : null; } - public SignalServiceAddress getAddress() { - return address; + public ServiceId getServiceId() { + return serviceId; + } + + public Optional getPni() { + return pni; + } + + public Optional getNumber() { + return e164; } public Optional getGivenName() { @@ -191,6 +208,13 @@ public final class SignalContactRecord implements SignalRecord { return proto.getHideStory(); } + /** + * Returns the same record, but stripped of the PNI field. Only used while PNP is in development. + */ + public SignalContactRecord withoutPni() { + return new SignalContactRecord(id, proto.toBuilder().clearServicePni().build()); + } + public ContactRecord toProto() { return proto; } @@ -213,7 +237,7 @@ public final class SignalContactRecord implements SignalRecord { private final StorageId id; private final ContactRecord.Builder builder; - public Builder(byte[] rawId, SignalServiceAddress address, byte[] serializedUnknowns) { + public Builder(byte[] rawId, ServiceId serviceId, byte[] serializedUnknowns) { this.id = StorageId.forContact(rawId); if (serializedUnknowns != null) { @@ -222,8 +246,17 @@ public final class SignalContactRecord implements SignalRecord { this.builder = ContactRecord.newBuilder(); } - builder.setServiceUuid(address.getServiceId().toString()); - builder.setServiceE164(address.getNumber().orElse("")); + builder.setServiceId(serviceId.toString()); + } + + public Builder setE164(String e164) { + builder.setServiceE164(e164 == null ? "" : e164); + return this; + } + + public Builder setPni(PNI pni) { + builder.setServicePni(pni == null ? "" : pni.toString()); + return this; } public Builder setGivenName(String givenName) { diff --git a/libsignal/service/src/main/proto/StorageService.proto b/libsignal/service/src/main/proto/StorageService.proto index fa56dd182..07fc00a4b 100644 --- a/libsignal/service/src/main/proto/StorageService.proto +++ b/libsignal/service/src/main/proto/StorageService.proto @@ -71,8 +71,9 @@ message ContactRecord { UNVERIFIED = 2; } - string serviceUuid = 1; + string serviceId = 1; string serviceE164 = 2; + string servicePni = 15; bytes profileKey = 3; bytes identityKey = 4; IdentityState identityState = 5; diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/storage/SignalContactRecordTest.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/storage/SignalContactRecordTest.java index d694bca9e..e62637793 100644 --- a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/storage/SignalContactRecordTest.java +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/storage/SignalContactRecordTest.java @@ -51,7 +51,8 @@ public class SignalContactRecordTest { String e164, String givenName) { - return new SignalContactRecord.Builder(byteArray(key), new SignalServiceAddress(serviceId, e164), null) + return new SignalContactRecord.Builder(byteArray(key), serviceId, null) + .setE164(e164) .setGivenName(givenName); } }