From a7d9bd944bf7db5d379ed39a667f172ef67cfec5 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 24 Jan 2023 20:49:59 -0500 Subject: [PATCH] Insert session switchover events when appropriate. --- .../RecipientTableTest_getAndPossiblyMerge.kt | 238 ++++++++++++++++-- ...entTableTest_processPnpTupleToChangeSet.kt | 62 ++--- .../securesms/util/FeatureFlagsAccessor.java | 11 + .../securesms/database/MessageTable.java | 22 ++ .../securesms/database/MessageTypes.java | 5 + .../securesms/database/PnpOperations.kt | 7 +- .../securesms/database/RecipientTable.kt | 48 ++-- .../securesms/util/FeatureFlags.java | 2 +- app/src/main/proto/Database.proto | 6 +- app/src/main/res/values/strings.xml | 6 +- 10 files changed, 331 insertions(+), 76 deletions(-) create mode 100644 app/src/androidTest/java/org/thoughtcrime/securesms/util/FeatureFlagsAccessor.java diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt index 030fa0265..d800486eb 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_getAndPossiblyMerge.kt @@ -1,17 +1,21 @@ package org.thoughtcrime.securesms.database +import android.database.Cursor import androidx.core.content.contentValuesOf import androidx.test.ext.junit.runners.AndroidJUnit4 import org.hamcrest.MatcherAssert import org.hamcrest.Matchers import org.junit.Assert import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.signal.core.util.CursorUtil import org.signal.core.util.SqlUtil +import org.signal.core.util.requireLong +import org.signal.core.util.requireNonNullString import org.signal.core.util.select import org.signal.libsignal.protocol.IdentityKey import org.signal.libsignal.protocol.SignalProtocolAddress @@ -23,6 +27,8 @@ import org.thoughtcrime.securesms.database.model.Mention import org.thoughtcrime.securesms.database.model.MessageId import org.thoughtcrime.securesms.database.model.MessageRecord import org.thoughtcrime.securesms.database.model.ReactionRecord +import org.thoughtcrime.securesms.database.model.databaseprotos.SessionSwitchoverEvent +import org.thoughtcrime.securesms.database.model.databaseprotos.ThreadMergeEvent import org.thoughtcrime.securesms.dependencies.ApplicationDependencies import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.keyvalue.SignalStore @@ -32,6 +38,9 @@ import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.sms.IncomingEncryptedMessage import org.thoughtcrime.securesms.sms.IncomingTextMessage +import org.thoughtcrime.securesms.util.Base64 +import org.thoughtcrime.securesms.util.FeatureFlags +import org.thoughtcrime.securesms.util.FeatureFlagsAccessor import org.whispersystems.signalservice.api.push.ACI import org.whispersystems.signalservice.api.push.PNI import org.whispersystems.signalservice.api.push.ServiceId @@ -46,6 +55,7 @@ class RecipientTableTest_getAndPossiblyMerge { SignalStore.account().setE164(E164_SELF) SignalStore.account().setAci(ACI_SELF) SignalStore.account().setPni(PNI_SELF) + FeatureFlagsAccessor.forceValue(FeatureFlags.PHONE_NUMBER_PRIVACY, true) } @Test @@ -115,24 +125,40 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, null, ACI_B) } - test("e164 and pni matches, all provided, new aci") { + test("e164 and pni matches, all provided, new aci, no pni session") { given(E164_A, PNI_A, null) process(E164_A, PNI_A, ACI_A) expect(E164_A, PNI_A, ACI_A) } + test("e164 and pni matches, all provided, new aci, existing pni session") { + given(E164_A, PNI_A, null, pniSession = true) + process(E164_A, PNI_A, ACI_A) + expect(E164_A, PNI_A, ACI_A) + + expectSessionSwitchoverEvent(E164_A) + } + test("e164 and aci matches, all provided, new pni") { given(E164_A, null, ACI_A) process(E164_A, PNI_A, ACI_A) expect(E164_A, PNI_A, ACI_A) } - test("pni matches, all provided, new e164 and aci") { + test("pni matches, all provided, new e164 and aci, no pni session") { given(null, PNI_A, null) process(E164_A, PNI_A, ACI_A) expect(E164_A, PNI_A, ACI_A) } + test("pni matches, all provided, new e164 and aci, existing pni session") { + given(null, PNI_A, null, pniSession = true) + process(E164_A, PNI_A, ACI_A) + expect(E164_A, PNI_A, ACI_A) + + expectSessionSwitchoverEvent(E164_A) + } + test("pni and aci matches, all provided, new e164") { given(null, PNI_A, ACI_A) process(E164_A, PNI_A, ACI_A) @@ -172,20 +198,42 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, null) } - test("e164 and pni matches, all provided, no existing session") { + test("e164 matches, e164 and pni provided, pni changes, existing pni session") { + given(E164_A, PNI_B, null, pniSession = true) + process(E164_A, PNI_A, null) + expect(E164_A, PNI_A, null) + + expectSessionSwitchoverEvent(E164_A) + } + + test("e164 and pni matches, all provided, no pni session") { given(E164_A, PNI_A, null) process(E164_A, PNI_A, ACI_A) expect(E164_A, PNI_A, ACI_A) } - test("pni matches, all provided, no existing session") { + test("e164 and pni matches, all provided, existing pni session") { + given(E164_A, PNI_A, null, pniSession = true) + process(E164_A, PNI_A, ACI_A) + expect(E164_A, PNI_A, ACI_A) + + expectSessionSwitchoverEvent(E164_A) + } + + test("pni matches, all provided, no pni session") { given(null, PNI_A, null) process(E164_A, PNI_A, ACI_A) expect(E164_A, PNI_A, ACI_A) } - // This test, I could go either way. We decide to change the E164 on the existing row rather than create a new one. - // But it's an "unstable E164->PNI mapping" case, which we don't expect, so as long as there's a user-visible impact that should be fine. + test("pni matches, all provided, existing pni session") { + given(null, PNI_A, null, pniSession = true) + process(E164_A, PNI_A, ACI_A) + expect(E164_A, PNI_A, ACI_A) + + expectSessionSwitchoverEvent(E164_A) + } + test("pni matches, no existing pni session, changes number") { given(E164_B, PNI_A, null) process(E164_A, PNI_A, ACI_A) @@ -194,8 +242,15 @@ class RecipientTableTest_getAndPossiblyMerge { expectChangeNumberEvent() } - // This test, I could go either way. We decide to change the E164 on the existing row rather than create a new one. - // But it's an "unstable E164->PNI mapping" case, which we don't expect, so as long as there's a user-visible impact that should be fine. + test("pni matches, existing pni session, changes number") { + given(E164_B, PNI_A, null, pniSession = true) + process(E164_A, PNI_A, ACI_A) + expect(E164_A, PNI_A, ACI_A) + + expectSessionSwitchoverEvent(E164_B) + expectChangeNumberEvent() + } + test("pni and aci matches, change number") { given(E164_B, PNI_A, ACI_A) process(E164_A, PNI_A, ACI_A) @@ -220,7 +275,7 @@ class RecipientTableTest_getAndPossiblyMerge { expectChangeNumberEvent() } - test("steal, e164+pni & e164+pni, no aci provided, no sessions") { + test("steal, e164+pni & e164+pni, no aci provided, no pni session") { given(E164_A, PNI_B, null) given(E164_B, PNI_A, null) @@ -230,6 +285,18 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_B, null, null) } + test("steal, e164+pni & e164+pni, no aci provided, existing pni session") { + given(E164_A, PNI_B, null, pniSession = true) + given(E164_B, PNI_A, null) // TODO How to handle if this user had a session? They just end up losing the PNI, meaning it would become unregistered, but it could register again later with a different PNI? + + process(E164_A, PNI_A, null) + + expect(E164_A, PNI_A, null) + expect(E164_B, null, null) + + expectSessionSwitchoverEvent(E164_A) + } + test("steal, e164+pni & aci, e164 record has separate e164") { given(E164_B, PNI_A, null) given(null, null, ACI_A) @@ -252,6 +319,19 @@ class RecipientTableTest_getAndPossiblyMerge { expectChangeNumberEvent() } + test("steal, e164 & pni+e164, no aci provided") { + val id1 = given(E164_A, null, null) + val id2 = given(E164_B, PNI_A, null, pniSession = true) + + process(E164_A, PNI_A, null) + + expect(E164_A, PNI_A, null) + expect(E164_B, null, null) + + expectSessionSwitchoverEvent(id1, E164_A) + expectSessionSwitchoverEvent(id2, E164_B) + } + test("merge, e164 & pni & aci, all provided") { given(E164_A, null, null) given(null, PNI_A, null) @@ -262,6 +342,8 @@ class RecipientTableTest_getAndPossiblyMerge { expectDeleted() expectDeleted() expect(E164_A, PNI_A, ACI_A) + + expectThreadMergeEvent(E164_A) } test("merge, e164 & pni, no aci provided") { @@ -272,9 +354,11 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, null) expectDeleted() + + expectThreadMergeEvent("") } - test("merge, e164 & pni, aci provided but no aci record") { + test("merge, e164 & pni, aci provided, no pni session") { given(E164_A, null, null) given(null, PNI_A, null) @@ -282,16 +366,33 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, ACI_A) expectDeleted() + + expectThreadMergeEvent("") } - test("merge, e164 & pni+e164, no aci provided") { + test("merge, e164 & pni, aci provided, no pni session") { given(E164_A, null, null) - given(E164_B, PNI_A, null) + given(null, PNI_A, null) - process(E164_A, PNI_A, null) + process(E164_A, PNI_A, ACI_A) - expect(E164_A, PNI_A, null) - expect(E164_B, null, null) + expect(E164_A, PNI_A, ACI_A) + expectDeleted() + + expectThreadMergeEvent("") + } + + test("merge, e164 & pni, aci provided, existing pni session") { + given(E164_A, null, null) + given(null, PNI_A, null, pniSession = true) + + process(E164_A, PNI_A, ACI_A) + + expect(E164_A, PNI_A, ACI_A) + expectDeleted() + + expectThreadMergeEvent("") + expectSessionSwitchoverEvent(E164_A) } test("merge, e164+pni & pni, no aci provided") { @@ -302,6 +403,8 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, null) expectDeleted() + + expectThreadMergeEvent("") } test("merge, e164+pni & aci") { @@ -312,6 +415,8 @@ class RecipientTableTest_getAndPossiblyMerge { expectDeleted() expect(E164_A, PNI_A, ACI_A) + + expectThreadMergeEvent(E164_A) } test("merge, e164+pni & e164+pni+aci, change number") { @@ -324,6 +429,7 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, ACI_A) expectChangeNumberEvent() + expectThreadMergeEvent(E164_A) } test("merge, e164+pni & e164+aci, change number") { @@ -336,6 +442,7 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, PNI_A, ACI_A) expectChangeNumberEvent() + expectThreadMergeEvent(E164_A) } test("merge, e164 & aci") { @@ -346,6 +453,8 @@ class RecipientTableTest_getAndPossiblyMerge { expectDeleted() expect(E164_A, null, ACI_A) + + expectThreadMergeEvent(E164_A) } test("merge, e164 & e164+aci, change number") { @@ -358,6 +467,8 @@ class RecipientTableTest_getAndPossiblyMerge { expect(E164_A, null, ACI_A) expectChangeNumberEvent() + + expectThreadMergeEvent(E164_A) } test("local user, local e164 and aci provided, changeSelf=false, leave e164 alone") { @@ -539,11 +650,11 @@ class RecipientTableTest_getAndPossiblyMerge { } private fun getMention(messageId: Long): MentionModel { - SignalDatabase.rawDatabase.rawQuery("SELECT * FROM ${MentionTable.TABLE_NAME} WHERE ${MentionTable.MESSAGE_ID} = $messageId").use { cursor -> + return SignalDatabase.rawDatabase.rawQuery("SELECT * FROM ${MentionTable.TABLE_NAME} WHERE ${MentionTable.MESSAGE_ID} = $messageId").use { cursor -> cursor.moveToFirst() - return MentionModel( - recipientId = RecipientId.from(CursorUtil.requireLong(cursor, MentionTable.RECIPIENT_ID)), - threadId = CursorUtil.requireLong(cursor, MentionTable.THREAD_ID) + MentionModel( + recipientId = RecipientId.from(cursor.requireLong(MentionTable.RECIPIENT_ID)), + threadId = cursor.requireLong(MentionTable.THREAD_ID) ) } } @@ -577,6 +688,14 @@ class RecipientTableTest_getAndPossiblyMerge { if (!test.changeNumberExpected) { test.expectNoChangeNumberEvent() } + + if (!test.threadMergeExpected) { + test.expectNoThreadMergeEvent() + } + + if (!test.sessionSwitchoverExpected) { + test.expectNoSessionSwitchoverEvent() + } } catch (e: Throwable) { if (e.javaClass != exception) { val error = java.lang.AssertionError("[$name] ${e.message}") @@ -594,6 +713,8 @@ class RecipientTableTest_getAndPossiblyMerge { private lateinit var outputRecipientId: RecipientId var changeNumberExpected = false + var threadMergeExpected = false + var sessionSwitchoverExpected = false init { // Need to delete these first to prevent foreign key crash @@ -616,9 +737,8 @@ class RecipientTableTest_getAndPossiblyMerge { pni: PNI?, aci: ACI?, createThread: Boolean = true, - sms: List = emptyList(), - mms: List = emptyList() - ) { + pniSession: Boolean = false + ): RecipientId { val id = insert(e164, pni, aci) generatedIds += id if (createThread) { @@ -626,6 +746,16 @@ class RecipientTableTest_getAndPossiblyMerge { SignalDatabase.threads.getOrCreateThreadIdFor(Recipient.resolved(id)) SignalDatabase.messages.insertMessageInbox(IncomingEncryptedMessage(IncomingTextMessage(id, 1, 0, 0, 0, "", Optional.empty(), 0, false, ""), "")) } + + if (pniSession) { + if (pni == null) { + throw IllegalArgumentException("pniSession = true but pni is null!") + } + + SignalDatabase.sessions.store(pni, SignalProtocolAddress(pni.toString(), 1), SessionRecord()) + } + + return id } fun process(e164: String?, pni: PNI?, aci: ACI?, changeSelf: Boolean = false) { @@ -676,6 +806,32 @@ class RecipientTableTest_getAndPossiblyMerge { changeNumberExpected = false } + fun expectSessionSwitchoverEvent(e164: String) { + expectSessionSwitchoverEvent(outputRecipientId, e164) + } + + fun expectSessionSwitchoverEvent(recipientId: RecipientId, e164: String) { + val event: SessionSwitchoverEvent? = getLatestSessionSwitchoverEvent(recipientId) + assertNotNull(event) + assertEquals(e164, event!!.e164) + sessionSwitchoverExpected = true + } + + fun expectNoSessionSwitchoverEvent() { + assertNull(getLatestSessionSwitchoverEvent(outputRecipientId)) + } + + fun expectThreadMergeEvent(previousE164: String) { + val event: ThreadMergeEvent? = getLatestThreadMergeEvent(outputRecipientId) + assertNotNull(event) + assertEquals(previousE164, event!!.previousE164) + threadMergeExpected = true + } + + fun expectNoThreadMergeEvent() { + assertNull(getLatestThreadMergeEvent(outputRecipientId)) + } + private fun insert(e164: String?, pni: PNI?, aci: ACI?): RecipientId { val serviceIdString: String? = (aci ?: pni)?.toString() val pniString: String? = pni?.toString() @@ -746,6 +902,42 @@ class RecipientTableTest_getAndPossiblyMerge { } } + private fun getLatestThreadMergeEvent(recipientId: RecipientId): ThreadMergeEvent? { + return SignalDatabase.rawDatabase + .select(MessageTable.BODY) + .from(MessageTable.TABLE_NAME) + .where("${MessageTable.RECIPIENT_ID} = ? AND ${MessageTable.TYPE} = ?", recipientId, MessageTypes.THREAD_MERGE_TYPE) + .orderBy("${MessageTable.DATE_RECEIVED} DESC") + .limit(1) + .run() + .use { cursor: Cursor -> + if (cursor.moveToFirst()) { + val bytes = Base64.decode(cursor.requireNonNullString(MessageTable.BODY)) + ThreadMergeEvent.parseFrom(bytes) + } else { + null + } + } + } + + private fun getLatestSessionSwitchoverEvent(recipientId: RecipientId): SessionSwitchoverEvent? { + return SignalDatabase.rawDatabase + .select(MessageTable.BODY) + .from(MessageTable.TABLE_NAME) + .where("${MessageTable.RECIPIENT_ID} = ? AND ${MessageTable.TYPE} = ?", recipientId, MessageTypes.SESSION_SWITCHOVER_TYPE) + .orderBy("${MessageTable.DATE_RECEIVED} DESC") + .limit(1) + .run() + .use { cursor: Cursor -> + if (cursor.moveToFirst()) { + val bytes = Base64.decode(cursor.requireNonNullString(MessageTable.BODY)) + SessionSwitchoverEvent.parseFrom(bytes) + } else { + null + } + } + } + companion object { val ACI_A = ACI.from(UUID.fromString("aaaa0000-5a76-47fa-a98a-7e72c948a82e")) val ACI_B = ACI.from(UUID.fromString("bbbb0000-0b60-4a68-9cd9-ed2f8453f9ed")) diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_processPnpTupleToChangeSet.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_processPnpTupleToChangeSet.kt index 865f7af56..bac0d2b30 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_processPnpTupleToChangeSet.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_processPnpTupleToChangeSet.kt @@ -111,7 +111,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetPni(result.id, PNI_A), PnpOperation.SetAci(result.id, ACI_A) ) @@ -131,9 +131,9 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetPni(result.id, PNI_A), - PnpOperation.SessionSwitchoverInsert(result.id) + PnpOperation.SessionSwitchoverInsert(result.id, E164_A) ) ), result.changeSet @@ -151,7 +151,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetPni(result.id, PNI_A) ) ), @@ -170,7 +170,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetAci(result.id, ACI_A) ) ), @@ -189,9 +189,9 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetAci(result.id, ACI_A), - PnpOperation.SessionSwitchoverInsert(result.id) + PnpOperation.SessionSwitchoverInsert(result.id, E164_A) ) ), result.changeSet @@ -209,7 +209,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetPni(result.id, PNI_A) ) ), @@ -228,7 +228,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.SetAci(result.id, ACI_A) ) @@ -248,10 +248,10 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.SetAci(result.id, ACI_A), - PnpOperation.SessionSwitchoverInsert(result.id) + PnpOperation.SessionSwitchoverInsert(result.id, E164_A) ) ), result.changeSet @@ -269,7 +269,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.SetAci(result.id, ACI_A), PnpOperation.ChangeNumberInsert( @@ -277,7 +277,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { oldE164 = E164_B, newE164 = E164_A ), - PnpOperation.SessionSwitchoverInsert(result.id) + PnpOperation.SessionSwitchoverInsert(result.id, E164_A) ) ), result.changeSet @@ -295,7 +295,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), ) ), @@ -314,7 +314,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.ChangeNumberInsert( recipientId = result.id, @@ -338,7 +338,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.SetPni(result.id, PNI_A) ) @@ -358,7 +358,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.id), - operations = listOf( + operations = linkedSetOf( PnpOperation.SetE164(result.id, E164_A), PnpOperation.SetPni(result.id, PNI_A), PnpOperation.ChangeNumberInsert( @@ -387,7 +387,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.thirdId), - operations = listOf( + operations = linkedSetOf( PnpOperation.Merge( primaryId = result.firstId, secondaryId = result.secondId @@ -416,7 +416,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.Merge( primaryId = result.firstId, secondaryId = result.secondId @@ -441,7 +441,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.Merge( primaryId = result.firstId, secondaryId = result.secondId @@ -470,7 +470,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.secondId), PnpOperation.SetPni( recipientId = result.firstId, @@ -496,7 +496,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.firstId), PnpOperation.Merge( primaryId = result.firstId, @@ -522,7 +522,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.secondId), PnpOperation.SetPni(result.firstId, PNI_A) ) @@ -545,11 +545,11 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.firstId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.secondId), PnpOperation.SetPni(result.firstId, PNI_A), - PnpOperation.SessionSwitchoverInsert(result.secondId), - PnpOperation.SessionSwitchoverInsert(result.firstId) + PnpOperation.SessionSwitchoverInsert(result.secondId, E164_A), + PnpOperation.SessionSwitchoverInsert(result.firstId, E164_A) ) ), result.changeSet @@ -570,7 +570,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.secondId), - operations = listOf( + operations = linkedSetOf( PnpOperation.Merge( primaryId = result.secondId, secondaryId = result.firstId @@ -595,7 +595,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.secondId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.firstId), PnpOperation.SetPni( recipientId = result.secondId, @@ -625,7 +625,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.secondId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.firstId), PnpOperation.SetPni( recipientId = result.secondId, @@ -660,7 +660,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.secondId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemovePni(result.secondId), PnpOperation.RemoveE164(result.secondId), PnpOperation.Merge( @@ -692,7 +692,7 @@ class RecipientTableTest_processPnpTupleToChangeSet { assertEquals( PnpChangeSet( id = PnpIdResolver.PnpNoopId(result.secondId), - operations = listOf( + operations = linkedSetOf( PnpOperation.RemoveE164(result.secondId), PnpOperation.Merge( primaryId = result.secondId, diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/util/FeatureFlagsAccessor.java b/app/src/androidTest/java/org/thoughtcrime/securesms/util/FeatureFlagsAccessor.java new file mode 100644 index 000000000..103b3df41 --- /dev/null +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/util/FeatureFlagsAccessor.java @@ -0,0 +1,11 @@ +package org.thoughtcrime.securesms.util; + +/** + * A class that allows us to inject feature flags during tests. + */ +public final class FeatureFlagsAccessor { + + public static void forceValue(String key, Object value) { + FeatureFlags.FORCED_VALUES.put(FeatureFlags.PHONE_NUMBER_PRIVACY, true); + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java index b22c7cc13..4ff256c55 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.java @@ -73,6 +73,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.GroupCallUpdateD import org.thoughtcrime.securesms.database.model.databaseprotos.MessageExportState; import org.thoughtcrime.securesms.database.model.databaseprotos.ProfileChangeDetails; import org.thoughtcrime.securesms.database.model.databaseprotos.ThreadMergeEvent; +import org.thoughtcrime.securesms.database.model.databaseprotos.SessionSwitchoverEvent; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.groups.GroupMigrationMembershipChange; import org.thoughtcrime.securesms.insights.InsightsConstants; @@ -94,6 +95,7 @@ import org.thoughtcrime.securesms.sms.IncomingGroupUpdateMessage; import org.thoughtcrime.securesms.sms.IncomingTextMessage; import org.thoughtcrime.securesms.stories.Stories; import org.thoughtcrime.securesms.util.Base64; +import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.JsonUtils; import org.thoughtcrime.securesms.util.MediaUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; @@ -1125,6 +1127,26 @@ public class MessageTable extends DatabaseTable implements MessageTypes, Recipie ApplicationDependencies.getDatabaseObserver().notifyConversationListeners(threadId); } + public void insertSessionSwitchoverEvent(@NonNull RecipientId recipientId, long threadId, @NonNull SessionSwitchoverEvent event) { + if (!FeatureFlags.phoneNumberPrivacy()) { + throw new IllegalStateException("Should not occur in a non-PNP world!"); + } + + ContentValues values = new ContentValues(); + values.put(RECIPIENT_ID, recipientId.serialize()); + values.put(RECIPIENT_DEVICE_ID, 1); + values.put(DATE_RECEIVED, System.currentTimeMillis()); + values.put(DATE_SENT, System.currentTimeMillis()); + values.put(READ, 1); + values.put(TYPE, MessageTypes.SESSION_SWITCHOVER_TYPE); + values.put(THREAD_ID, threadId); + values.put(BODY, Base64.encodeBytes(event.toByteArray())); + + getWritableDatabase().insert(TABLE_NAME, null, values); + + ApplicationDependencies.getDatabaseObserver().notifyConversationListeners(threadId); + } + public void insertSmsExportMessage(@NonNull RecipientId recipientId, long threadId) { ContentValues values = new ContentValues(); values.put(RECIPIENT_ID, recipientId.serialize()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTypes.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTypes.java index 22f29e8cc..c77f76d13 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTypes.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTypes.java @@ -50,6 +50,7 @@ public interface MessageTypes { long BOOST_REQUEST_TYPE = 15; long THREAD_MERGE_TYPE = 16; long SMS_EXPORT_TYPE = 17; + long SESSION_SWITCHOVER_TYPE = 18; long BASE_INBOX_TYPE = 20; long BASE_OUTBOX_TYPE = 21; @@ -207,6 +208,10 @@ public interface MessageTypes { return (type & BASE_TYPE_MASK) == THREAD_MERGE_TYPE; } + static boolean isSessionSwitchoverType(long type) { + return (type & BASE_TYPE_MASK) == SESSION_SWITCHOVER_TYPE; + } + static boolean isSecureType(long type) { return (type & SECURE_MESSAGE_BIT) != 0; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/PnpOperations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/PnpOperations.kt index ced9718c9..dfe689c30 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/PnpOperations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/PnpOperations.kt @@ -37,7 +37,7 @@ data class PnpDataSet( * Applies the set of operations and returns the resulting dataset. * Important: This only occurs _in memory_. You must still apply the operations to disk to persist them. */ - fun perform(operations: List): PnpDataSet { + fun perform(operations: LinkedHashSet): PnpDataSet { if (operations.isEmpty()) { return this } @@ -136,7 +136,7 @@ data class PnpDataSet( */ data class PnpChangeSet( val id: PnpIdResolver, - val operations: List = emptyList(), + val operations: LinkedHashSet = linkedSetOf(), val breadCrumbs: List = emptyList() ) { // We want to exclude breadcrumbs from equality for testing purposes @@ -213,7 +213,8 @@ sealed class PnpOperation { } data class SessionSwitchoverInsert( - override val recipientId: RecipientId + override val recipientId: RecipientId, + val e164: String? ) : PnpOperation() data class ChangeNumberInsert( 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 6a432947c..4dff4e30e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -66,6 +66,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.ChatColor import org.thoughtcrime.securesms.database.model.databaseprotos.DeviceLastResetTime import org.thoughtcrime.securesms.database.model.databaseprotos.ExpiringProfileKeyCredentialColumnData import org.thoughtcrime.securesms.database.model.databaseprotos.RecipientExtras +import org.thoughtcrime.securesms.database.model.databaseprotos.SessionSwitchoverEvent import org.thoughtcrime.securesms.database.model.databaseprotos.ThreadMergeEvent import org.thoughtcrime.securesms.database.model.databaseprotos.Wallpaper import org.thoughtcrime.securesms.dependencies.ApplicationDependencies @@ -2367,7 +2368,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da affectedIds = affectedIds, oldIds = oldIds, changedNumberId = changedNumberId, - operations = changeSet.operations, + operations = changeSet.operations.toList(), breadCrumbs = changeSet.breadCrumbs ) } @@ -2436,8 +2437,14 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da merge(operation.primaryId, operation.secondaryId, inputPni) } is PnpOperation.SessionSwitchoverInsert -> { - // TODO [pnp] - Log.w(TAG, "Session switchover events aren't implemented yet!") + val threadId: Long? = threads.getThreadIdFor(operation.recipientId) + if (threadId != null) { + val event = SessionSwitchoverEvent + .newBuilder() + .setE164(operation.e164 ?: "") + .build() + SignalDatabase.messages.insertSessionSwitchoverEvent(operation.recipientId, threadId, event) + } } is PnpOperation.ChangeNumberInsert -> { if (changeSet.id is PnpIdResolver.PnpNoopId) { @@ -2544,7 +2551,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da check(fullData.commonId == null) check(listOfNotNull(fullData.byE164, fullData.byPniSid, fullData.byPniOnly, fullData.byAciSid).size >= 2) - val operations: MutableList = mutableListOf() + val operations: LinkedHashSet = linkedSetOf() operations += processPossibleE164PniSidMerge(pni, pniVerified, fullData, breadCrumbs) operations += processPossiblePniSidAciSidMerge(e164, pni, aci, fullData.perform(operations), changeSelf, breadCrumbs) @@ -2559,6 +2566,13 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da recipientId = primaryId, aci = aci ) + + if (!pniVerified && finalData.pni != null && sessions.hasAnySessionFor(finalData.pni.toString())) { + operations += PnpOperation.SessionSwitchoverInsert( + recipientId = primaryId, + e164 = finalData.e164 + ) + } } if (finalData.byE164 == null && e164 != null && (changeSelf || notSelf(e164, pni, aci))) { @@ -2599,7 +2613,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da private fun processNonMergePnpUpdate(e164: String?, pni: PNI?, aci: ACI?, pniVerified: Boolean, changeSelf: Boolean, commonId: RecipientId, breadCrumbs: MutableList): PnpChangeSet { val record: RecipientRecord = getRecord(commonId) - val operations: MutableList = mutableListOf() + val operations: LinkedHashSet = linkedSetOf() // This is a special case. The ACI passed in doesn't match the common record. We can't change ACIs, so we need to make a new record. if (aci != null && aci != record.serviceId && record.serviceId != null && !record.sidIsPni()) { @@ -2658,7 +2672,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da val newServiceId: ServiceId? = aci ?: pni ?: record.serviceId if (!pniVerified && record.serviceId != null && record.serviceId != newServiceId && sessions.hasAnySessionFor(record.serviceId.toString())) { - operations += PnpOperation.SessionSwitchoverInsert(commonId) + operations += PnpOperation.SessionSwitchoverInsert(recipientId = commonId, e164 = record.e164 ?: e164) } return PnpChangeSet( @@ -2668,15 +2682,15 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da ) } - private fun processPossibleE164PniSidMerge(pni: PNI?, pniVerified: Boolean, data: PnpDataSet, breadCrumbs: MutableList): List { + private fun processPossibleE164PniSidMerge(pni: PNI?, pniVerified: Boolean, data: PnpDataSet, breadCrumbs: MutableList): LinkedHashSet { if (pni == null || data.byE164 == null || data.byPniSid == null || data.e164Record == null || data.pniSidRecord == null || data.e164Record.id == data.pniSidRecord.id) { - return emptyList() + return linkedSetOf() } // We have found records for both the E164 and PNI, and they're different breadCrumbs += "E164PniSidMerge" - val operations: MutableList = mutableListOf() + val operations: LinkedHashSet = linkedSetOf() // The PNI record only has a single identifier. We know we must merge. if (data.pniSidRecord.sidOnly(pni)) { @@ -2704,31 +2718,35 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da ) if (!pniVerified && sessions.hasAnySessionFor(data.pniSidRecord.serviceId.toString())) { - operations += PnpOperation.SessionSwitchoverInsert(data.byPniSid) + operations += PnpOperation.SessionSwitchoverInsert(recipientId = data.byPniSid, e164 = data.pniSidRecord.e164) + + if (data.e164Record.serviceId == null || data.e164Record.sidIsPni()) { + operations += PnpOperation.SessionSwitchoverInsert(recipientId = data.byE164, e164 = data.e164Record.e164) + } } if (!pniVerified && data.e164Record.serviceId != null && data.e164Record.sidIsPni() && sessions.hasAnySessionFor(data.e164Record.serviceId.toString())) { - operations += PnpOperation.SessionSwitchoverInsert(data.byE164) + operations += PnpOperation.SessionSwitchoverInsert(recipientId = data.byE164, e164 = data.e164) } } return operations } - private fun processPossiblePniSidAciSidMerge(e164: String?, pni: PNI?, aci: ACI?, data: PnpDataSet, changeSelf: Boolean, breadCrumbs: MutableList): List { + private fun processPossiblePniSidAciSidMerge(e164: String?, pni: PNI?, aci: ACI?, data: PnpDataSet, changeSelf: Boolean, breadCrumbs: MutableList): LinkedHashSet { if (pni == null || aci == null || data.byPniSid == null || data.byAciSid == null || data.pniSidRecord == null || data.aciSidRecord == null || data.pniSidRecord.id == data.aciSidRecord.id) { - return emptyList() + return linkedSetOf() } if (!changeSelf && isSelf(e164, pni, aci)) { breadCrumbs += "ChangeSelfPreventsPniSidAciSidMerge" - return emptyList() + return linkedSetOf() } // We have found records for both the PNI and ACI, and they're different breadCrumbs += "PniSidAciSidMerge" - val operations: MutableList = mutableListOf() + val operations: LinkedHashSet = linkedSetOf() // The PNI record only has a single identifier. We know we must merge. if (data.pniSidRecord.sidOnly(pni)) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index 0f79e16fd..dba0fbf39 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -86,7 +86,7 @@ public final class FeatureFlags { private static final String USE_HARDWARE_AEC_IF_OLD = "android.calling.useHardwareAecIfOlderThanApi29"; private static final String USE_AEC3 = "android.calling.useAec3"; private static final String PAYMENTS_COUNTRY_BLOCKLIST = "android.payments.blocklist"; - private static final String PHONE_NUMBER_PRIVACY = "android.pnp"; + public static final String PHONE_NUMBER_PRIVACY = "android.pnp"; private static final String USE_FCM_FOREGROUND_SERVICE = "android.useFcmForegroundService.3"; private static final String STORIES_AUTO_DOWNLOAD_MAXIMUM = "android.stories.autoDownloadMaximum"; private static final String TELECOM_MANUFACTURER_ALLOWLIST = "android.calling.telecomAllowList"; diff --git a/app/src/main/proto/Database.proto b/app/src/main/proto/Database.proto index daf0caa0c..569c67587 100644 --- a/app/src/main/proto/Database.proto +++ b/app/src/main/proto/Database.proto @@ -265,4 +265,8 @@ message MessageExportState { message ThreadMergeEvent { string previousE164 = 1; -} \ No newline at end of file +} + +message SessionSwitchoverEvent { + string e164 = 1; +} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f8485d46b..bf85dfe82 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1364,10 +1364,12 @@ %1$s changed their phone number. Like this new feature? Help support Signal with a one-time donation. - + Your message history with %1$s and their number %2$s has been merged. - + Your message history with %1$s and another chat that belonged to them has been merged. + + %1$s belongs to %2$s You sent %s a request to activate Payments