Fix bug with stale linked devices when changing number.

2025-12-27 06:29:54 +00:00 · 2022-09-08 09:56:41 -04:00
parent 24b7593178
commit ca0e52e141
12 changed files with 251 additions and 64 deletions
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModelTest.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModelTest.kt
@@ -34,6 +34,7 @@ import org.thoughtcrime.securesms.testing.success
 import org.thoughtcrime.securesms.testing.timeout
 import org.whispersystems.signalservice.api.account.ChangePhoneNumberRequest
 import org.whispersystems.signalservice.api.push.ServiceId
+import org.whispersystems.signalservice.internal.push.MismatchedDevices
 import org.whispersystems.signalservice.internal.push.PreKeyState
 import java.util.UUID

@@ -249,6 +250,109 @@ class ChangeNumberViewModelTest {
    assertSuccess(newPni, changeNumberRequest, setPreKeysRequest)
  }

+  @Test
+  fun testChangeNumber_givenMismatchedDevicesOnFirstCall() {
+    // GIVEN
+    val aci = Recipient.self().requireServiceId()
+    val newPni = ServiceId.from(UUID.randomUUID())
+    lateinit var changeNumberRequest: ChangePhoneNumberRequest
+    lateinit var setPreKeysRequest: PreKeyState
+
+    InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
+      Put("/v1/accounts/number") { r ->
+        changeNumberRequest = r.parsedRequestBody()
+        if (changeNumberRequest.deviceMessages.isEmpty()) {
+          MockResponse().failure(
+            409,
+            MismatchedDevices().apply {
+              missingDevices = listOf(2)
+              extraDevices = emptyList()
+            }
+          )
+        } else {
+          MockResponse().success(MockProvider.createVerifyAccountResponse(aci, newPni))
+        }
+      },
+      Get("/v2/keys/$aci/2") {
+        MockResponse().success(MockProvider.createPreKeyResponse(deviceId = 2))
+      },
+      Put("/v2/keys") { r ->
+        setPreKeysRequest = r.parsedRequestBody()
+        MockResponse().success()
+      },
+      Get("/v1/certificate/delivery") { MockResponse().success(MockProvider.senderCertificate) }
+    )
+
+    // WHEN
+    viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().resultOrThrow
+
+    // THEN
+    assertSuccess(newPni, changeNumberRequest, setPreKeysRequest)
+  }
+
+  @Test
+  fun testChangeNumber_givenRegLockAndMismatchedDevicesOnFirstTwoCalls() {
+    // GIVEN
+    val aci = Recipient.self().requireServiceId()
+    val newPni = ServiceId.from(UUID.randomUUID())
+
+    lateinit var changeNumberRequest: ChangePhoneNumberRequest
+    lateinit var setPreKeysRequest: PreKeyState
+
+    MockProvider.mockGetRegistrationLockStringFlow(kbsRepository)
+
+    InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Put("/v1/accounts/number") { r ->
+        changeNumberRequest = r.parsedRequestBody()
+        if (changeNumberRequest.registrationLock.isNullOrEmpty()) {
+          MockResponse().failure(423, MockProvider.lockedFailure)
+        } else if (changeNumberRequest.deviceMessages.isEmpty()) {
+          MockResponse().failure(
+            409,
+            MismatchedDevices().apply {
+              missingDevices = listOf(2)
+              extraDevices = emptyList()
+            }
+          )
+        } else if (changeNumberRequest.deviceMessages.size == 1) {
+          MockResponse().failure(
+            409,
+            MismatchedDevices().apply {
+              missingDevices = listOf(2, 3)
+              extraDevices = emptyList()
+            }
+          )
+        } else {
+          MockResponse().success(MockProvider.createVerifyAccountResponse(aci, newPni))
+        }
+      },
+      Get("/v2/keys/$aci/2") {
+        MockResponse().success(MockProvider.createPreKeyResponse(deviceId = 2))
+      },
+      Get("/v2/keys/$aci/3") {
+        MockResponse().success(MockProvider.createPreKeyResponse(deviceId = 3))
+      },
+      Put("/v2/keys") { r ->
+        setPreKeysRequest = r.parsedRequestBody()
+        MockResponse().success()
+      },
+      Get("/v1/certificate/delivery") { MockResponse().success(MockProvider.senderCertificate) }
+    )
+
+    // WHEN
+    viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().also { processor ->
+      processor.registrationLock() assertIs true
+      Recipient.self().requirePni() assertIsNot newPni
+      SignalStore.misc().pendingChangeNumberMetadata.assertIsNull()
+    }
+
+    viewModel.verifyCodeAndRegisterAccountWithRegistrationLock("pin").blockingGet().resultOrThrow
+
+    // THEN
+    assertSuccess(newPni, changeNumberRequest, setPreKeysRequest)
+  }
+
  private fun assertSuccess(newPni: ServiceId, changeNumberRequest: ChangePhoneNumberRequest, setPreKeysRequest: PreKeyState) {
    val pniProtocolStore = ApplicationDependencies.getProtocolStore().pni()
    val pniMetadataStore = SignalStore.account().pniPreKeys
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest_processPnpTuple.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest_processPnpTuple.kt
@@ -410,7 +410,7 @@ class RecipientDatabaseTest_processPnpTuple {
    fun process(e164: String?, pni: PNI?, aci: ACI?) {
      SignalDatabase.rawDatabase.beginTransaction()
      try {
-        generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, pniVerified = false, pnpEnabled = true).finalId
+        generatedIds += recipientDatabase.processPnpTuple(e164, pni, aci, pniVerified = false).finalId
        SignalDatabase.rawDatabase.setTransactionSuccessful()
      } finally {
        SignalDatabase.rawDatabase.endTransaction()
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/helpers/migration/MyStoryMigrationTest.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/helpers/migration/MyStoryMigrationTest.kt
@@ -109,7 +109,7 @@ class MyStoryMigrationTest {
  }

  private fun runMigration() {
-    MyStoryMigration.migrate(
+    V151_MyStoryMigration.migrate(
      InstrumentationRegistry.getInstrumentation().targetContext.applicationContext as Application,
      SignalDatabase.rawDatabase,
      0,
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/dependencies/InstrumentationApplicationDependencyProvider.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/dependencies/InstrumentationApplicationDependencyProvider.kt
@@ -77,6 +77,7 @@ class InstrumentationApplicationDependencyProvider(application: Application, def
    serviceNetworkAccessMock = mock {
      on { getConfiguration() } doReturn uncensoredConfiguration
      on { getConfiguration(any()) } doReturn uncensoredConfiguration
+      on { uncensoredConfiguration } doReturn uncensoredConfiguration
    }

    keyBackupService = mock()
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/testing/MockProvider.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/testing/MockProvider.kt
@@ -6,7 +6,14 @@ import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.stub
 import org.signal.core.util.Hex
+import org.signal.libsignal.protocol.IdentityKeyPair
+import org.signal.libsignal.protocol.ecc.Curve
+import org.signal.libsignal.protocol.state.PreKeyRecord
+import org.signal.libsignal.protocol.util.KeyHelper
+import org.signal.libsignal.protocol.util.Medium
+import org.thoughtcrime.securesms.crypto.PreKeyUtil
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies
+import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.pin.KbsRepository
 import org.thoughtcrime.securesms.pin.TokenData
 import org.thoughtcrime.securesms.test.BuildConfig
@@ -16,10 +23,14 @@ import org.whispersystems.signalservice.api.kbs.HashedPin
 import org.whispersystems.signalservice.api.kbs.MasterKey
 import org.whispersystems.signalservice.api.messages.multidevice.DeviceInfo
 import org.whispersystems.signalservice.api.push.ServiceId
+import org.whispersystems.signalservice.api.push.SignedPreKeyEntity
 import org.whispersystems.signalservice.internal.ServiceResponse
 import org.whispersystems.signalservice.internal.contacts.entities.TokenResponse
 import org.whispersystems.signalservice.internal.push.AuthCredentials
 import org.whispersystems.signalservice.internal.push.DeviceInfoList
+import org.whispersystems.signalservice.internal.push.PreKeyEntity
+import org.whispersystems.signalservice.internal.push.PreKeyResponse
+import org.whispersystems.signalservice.internal.push.PreKeyResponseItem
 import org.whispersystems.signalservice.internal.push.PushServiceSocket
 import org.whispersystems.signalservice.internal.push.SenderCertificate
 import org.whispersystems.signalservice.internal.push.VerifyAccountResponse
@@ -83,4 +94,21 @@ object MockProvider {
      on { newRegistrationSession(any(), any()) } doReturn session
    }
  }
+
+  fun createPreKeyResponse(identity: IdentityKeyPair = SignalStore.account().aciIdentityKey, deviceId: Int): PreKeyResponse {
+    val signedPreKeyRecord = PreKeyUtil.generateSignedPreKey(SecureRandom().nextInt(Medium.MAX_VALUE), identity.privateKey)
+    val oneTimePreKey = PreKeyRecord(SecureRandom().nextInt(Medium.MAX_VALUE), Curve.generateKeyPair())
+
+    val device = PreKeyResponseItem().apply {
+      this.deviceId = deviceId
+      registrationId = KeyHelper.generateRegistrationId(false)
+      signedPreKey = SignedPreKeyEntity(signedPreKeyRecord.id, signedPreKeyRecord.keyPair.publicKey, signedPreKeyRecord.signature)
+      preKey = PreKeyEntity(oneTimePreKey.id, oneTimePreKey.keyPair.publicKey)
+    }
+
+    return PreKeyResponse().apply {
+      identityKey = identity.publicKey
+      devices = listOf(device)
+    }
+  }
 }