diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt index ff52264188..d201956ed7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt @@ -755,7 +755,8 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo val isAciChanged = SignalStore.account.aci != aci if (pni == null) { - Log.i(TAG, "[applyAccountData] No PNI in the account data. Registering an account with no phone number.") + Log.i(TAG, "[applyAccountData] No PNI in the account data. Registering an account with no phone number. Clearing any E164/PNI state from a previous registration.") + SignalStore.account.clearE164AndPni() } SignalStore.account.setAci(aci) diff --git a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt index cd7a4c096c..70ab495b3e 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt @@ -260,6 +260,42 @@ class AppRegistrationStorageControllerTest { assertThat(readInProgressData().accountDataCommitted).isTrue() } + @Test + fun `commit - numberless re-registration over existing account - clears stale pni and e164`() = runBlocking { + SignalStore.account.setAci(aci) + SignalStore.account.setPni(pni) + SignalStore.account.setE164(E164) + SignalStore.account.restoreAciIdentityKeyFromBackup(aciIdentity.publicKey.serialize(), aciIdentity.privateKey.serialize()) + SignalStore.account.restorePniIdentityKeyFromBackup(pniIdentity.publicKey.serialize(), pniIdentity.privateKey.serialize()) + SignalStore.account.pniPreKeys.isSignedPreKeyRegistered = true + SignalStore.account.pniPreKeys.activeSignedPreKeyId = 12 + SignalStore.account.setRegistered(true) + + seedInProgressData( + RegistrationData( + accountData = accountData(reRegistration = true).newBuilder() + .e164("") + .pni("") + .pniIdentityKeyPair(ByteString.EMPTY) + .pniSignedPreKey(ByteString.EMPTY) + .pniLastResortKyberPreKey(ByteString.EMPTY) + .pniRegistrationId(0) + .build(), + accountEntropyPool = aep.value + ) + ) + + controller.commitRegistrationData() + + assertThat(SignalStore.account.aci).isEqualTo(aci) + assertThat(SignalStore.account.pni).isNull() + assertThat(SignalStore.account.e164).isNull() + assertThat(SignalStore.account.hasPniIdentityKey()).isFalse() + assertThat(SignalStore.account.pniPreKeys.isSignedPreKeyRegistered).isFalse() + assertThat(SignalStore.account.aciPreKeys.isSignedPreKeyRegistered).isTrue() + assertThat(SignalStore.account.isRegistered).isTrue() + } + @Test fun `commit - pin opted out - applies svr opt out`() = runBlocking { seedInProgressData( diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index 894f34f117..dedc060688 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -1659,6 +1659,48 @@ class RegistrationEndToEndTest { assert(purchaseApi.consumedTokens == listOf(FakeOneTimePurchaseApi.PURCHASE_TOKEN)) { "Expected the purchase to be consumed once but was ${purchaseApi.consumedTokens}" } } + @Test + fun `registering without a phone number on a device already registered with one commits account data carrying no pni`() { + enableSignalLoginRegistration() + storageController.preExistingRegistrationData = preExistingRegistrationData(E164) + + var registrationComplete = false + launchRegistrationFlow(onRegistrationComplete = { registrationComplete = true }) + + startSignalLoginRegistration() + buySignalLogin() + + waitForTag(TestTags.SIGNAL_LOGIN_INFO_SCREEN) + val login = registeredSignalLogin() + + recordSignalLoginManually() + enterSignalLogin(login) + skipUsername() + + waitFor("registration to complete") { registrationComplete } + + val committed = storageController.committedData + assert(committed != null) { "Expected registration data to be committed" } + + val accountData = committed!!.accountData + assert(accountData != null) { "Expected account data to be committed" } + assert(accountData!!.aci == login.aci.toString()) { "Expected committed ACI ${login.aci} but was ${accountData.aci}" } + + // The device still holds the previous account's E164 and PNI. None of it may leak into the committed data, which is + // what the app applies to permanent storage -- a PNI here with no matching key material is unusable. + assert(accountData.e164 == null) { "Expected no committed e164 but was ${accountData.e164}" } + assert(accountData.pni == null) { "Expected no committed PNI but was ${accountData.pni}" } + assert(accountData.pniIdentityKeyPair.size == 0) { "Expected no committed PNI identity key but was ${accountData.pniIdentityKeyPair.size} bytes" } + assert(accountData.pniSignedPreKey.size == 0) { "Expected no committed PNI signed pre-key but was ${accountData.pniSignedPreKey.size} bytes" } + assert(accountData.pniLastResortKyberPreKey.size == 0) { "Expected no committed PNI last-resort kyber pre-key but was ${accountData.pniLastResortKyberPreKey.size} bytes" } + assert(accountData.pniRegistrationId == 0) { "Expected no committed PNI registration id but was ${accountData.pniRegistrationId}" } + + val request = networkController.lastRegisterAccountRequest + assert(request != null) { "Expected a registration attempt" } + assert(request!!.e164 == null) { "Expected the previous number to be left behind but was ${request.e164}" } + assert(request.pniPreKeys == null) { "An account with no phone number has no PNI, so no PNI key material should be sent" } + } + @Test fun `a purchased signal login that the password manager takes and hands back moves the user on to the username step`() { enableSignalLoginRegistration()