diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberEnterCodeFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberEnterCodeFragment.kt index 8e2b13e583..8fa1506731 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberEnterCodeFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberEnterCodeFragment.kt @@ -78,15 +78,6 @@ class ChangeNumberEnterCodeFragment : LoggingFragment(R.layout.fragment_change_n phoneStateListener = SignalStrengthPhoneStateListener(this, PhoneStateCallback()) - requireActivity().onBackPressedDispatcher.addCallback( - viewLifecycleOwner, - object : OnBackPressedCallback(true) { - override fun handleOnBackPressed() { - navigateUp() - } - } - ) - binding.codeEntryLayout.wrongNumber.setOnClickListener { navigateUp() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberLockActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberLockActivity.kt index 0cf329fa03..232e967ac9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberLockActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberLockActivity.kt @@ -15,6 +15,9 @@ import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.MainActivity import org.thoughtcrime.securesms.PassphraseRequiredActivity import org.thoughtcrime.securesms.R +import org.thoughtcrime.securesms.dependencies.AppDependencies +import org.thoughtcrime.securesms.jobs.AccountConsistencyWorkerJob +import org.thoughtcrime.securesms.jobs.PreKeysSyncJob import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.logsubmit.SubmitDebugLogActivity import org.thoughtcrime.securesms.util.DynamicNoActionBarTheme @@ -55,7 +58,16 @@ class ChangeNumberLockActivity : PassphraseRequiredActivity() { setContentView(R.layout.activity_change_number_lock) - checkWhoAmI() + reattemptChange() + } + + private fun reattemptChange() { + val metadata = SignalStore.misc.pendingChangeNumberMetadata + if (metadata != null && metadata.newE164 != "") { + viewModel.reattemptChangeLocalNumber(::onChangeStatusConfirmed, ::onFailedToGetChangeNumberStatus) + } else { + onMissingChangeNumberMetadata() + } } override fun onResume() { @@ -63,10 +75,6 @@ class ChangeNumberLockActivity : PassphraseRequiredActivity() { dynamicTheme.onResume(this) } - private fun checkWhoAmI() { - viewModel.checkWhoAmI(::onChangeStatusConfirmed, ::onFailedToGetChangeNumberStatus) - } - private fun onChangeStatusConfirmed() { SignalStore.misc.clearPendingChangeNumberMetadata() @@ -87,7 +95,7 @@ class ChangeNumberLockActivity : PassphraseRequiredActivity() { MaterialAlertDialogBuilder(this) .setTitle(R.string.ChangeNumberLockActivity__change_status_unconfirmed) .setMessage(getString(R.string.ChangeNumberLockActivity__we_could_not_determine_the_status_of_your_change_number_request, error.javaClass.simpleName)) - .setPositiveButton(R.string.ChangeNumberLockActivity__retry) { _, _ -> checkWhoAmI() } + .setPositiveButton(R.string.ChangeNumberLockActivity__retry) { _, _ -> reattemptChange() } .setNegativeButton(R.string.ChangeNumberLockActivity__leave) { _, _ -> finish() } .setNeutralButton(R.string.ChangeNumberLockActivity__submit_debug_log) { _, _ -> startActivity(Intent(this, SubmitDebugLogActivity::class.java)) @@ -96,4 +104,30 @@ class ChangeNumberLockActivity : PassphraseRequiredActivity() { .setCancelable(false) .show() } + + private fun onMissingChangeNumberMetadata() { + Log.w(TAG, "Change number metadata is missing, gonna let it ride but this shouldn't happen") + + MaterialAlertDialogBuilder(this) + .setTitle(R.string.ChangeNumberLockActivity__change_status_unconfirmed) + .setMessage(getString(R.string.ChangeNumberLockActivity__we_could_not_determine_the_status_of_your_change_number_request, "MissingMetadata")) + .setPositiveButton(android.R.string.ok) { _, _ -> + SignalStore.misc.unlockChangeNumber() + + AppDependencies + .jobManager + .startChain(PreKeysSyncJob.create()) + .then(AccountConsistencyWorkerJob()) + .enqueue() + + startActivity(MainActivity.clearTop(this)) + finish() + } + .setNeutralButton(R.string.ChangeNumberLockActivity__submit_debug_log) { _, _ -> + startActivity(Intent(this, SubmitDebugLogActivity::class.java)) + finish() + } + .setCancelable(false) + .show() + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt index 92772f25a8..3d4e74cb88 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberRepository.kt @@ -24,6 +24,7 @@ import org.thoughtcrime.securesms.database.IdentityTable import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.database.model.databaseprotos.PendingChangeNumberMetadata import org.thoughtcrime.securesms.dependencies.AppDependencies +import org.thoughtcrime.securesms.jobmanager.impl.BackoffUtil import org.thoughtcrime.securesms.jobs.RefreshAttributesJob import org.thoughtcrime.securesms.keyvalue.CertificateType import org.thoughtcrime.securesms.keyvalue.SignalStore @@ -51,7 +52,6 @@ import org.whispersystems.signalservice.internal.push.SyncMessage import org.whispersystems.signalservice.internal.push.VerifyAccountResponse import org.whispersystems.signalservice.internal.push.WhoAmIResponse import java.io.IOException -import java.security.MessageDigest import java.security.SecureRandom import kotlin.coroutines.resume import kotlin.time.Duration @@ -104,22 +104,14 @@ class ChangeNumberRepository( @WorkerThread fun changeLocalNumber(e164: String, pni: ServiceId.PNI) { - val oldStorageId: ByteArray? = Recipient.self().storageId SignalDatabase.recipients.updateSelfE164(e164, pni) - val newStorageId: ByteArray? = Recipient.self().storageId - - if (e164 != SignalStore.account.requireE164() && MessageDigest.isEqual(oldStorageId, newStorageId)) { - Log.w(TAG, "Self storage id was not rotated, attempting to rotate again") - SignalDatabase.recipients.rotateStorageId(Recipient.self().id) - StorageSyncHelper.scheduleSyncForDataChange() - val secondAttemptStorageId: ByteArray? = Recipient.self().storageId - if (MessageDigest.isEqual(oldStorageId, secondAttemptStorageId)) { - Log.w(TAG, "Second attempt also failed to rotate storage id") - } - } - AppDependencies.recipientCache.clear() + if (e164 != SignalStore.account.requireE164()) { + SignalDatabase.recipients.rotateStorageId(Recipient.self().fresh().id) + StorageSyncHelper.scheduleSyncForDataChange() + } + SignalStore.account.setE164(e164) SignalStore.account.setPni(pni) AppDependencies.resetProtocolStores() @@ -132,34 +124,30 @@ class ChangeNumberRepository( throw AssertionError("No change number metadata") } - val originalPni = ServiceId.PNI.parseOrThrow(metadata.previousPni) + val pniIdentityKeyPair = IdentityKeyPair(metadata.pniIdentityKeyPair.toByteArray()) + val pniRegistrationId = metadata.pniRegistrationId + val pniSignedPreyKeyId = metadata.pniSignedPreKeyId + val pniLastResortKyberPreKeyId = metadata.pniLastResortKyberPreKeyId - if (originalPni == pni) { - Log.i(TAG, "No change has occurred, PNI is unchanged: $pni") - } else { - val pniIdentityKeyPair = IdentityKeyPair(metadata.pniIdentityKeyPair.toByteArray()) - val pniRegistrationId = metadata.pniRegistrationId - val pniSignedPreyKeyId = metadata.pniSignedPreKeyId - val pniLastResortKyberPreKeyId = metadata.pniLastResortKyberPreKeyId + val pniProtocolStore = AppDependencies.protocolStore.pni() + val pniMetadataStore = SignalStore.account.pniPreKeys - val pniProtocolStore = AppDependencies.protocolStore.pni() - val pniMetadataStore = SignalStore.account.pniPreKeys + SignalStore.account.pniRegistrationId = pniRegistrationId + SignalStore.account.setPniIdentityKeyAfterChangeNumber(pniIdentityKeyPair) - SignalStore.account.pniRegistrationId = pniRegistrationId - SignalStore.account.setPniIdentityKeyAfterChangeNumber(pniIdentityKeyPair) + val signedPreKey = pniProtocolStore.loadSignedPreKey(pniSignedPreyKeyId) + val oneTimeEcPreKeys = PreKeyUtil.generateAndStoreOneTimeEcPreKeys(pniProtocolStore, pniMetadataStore) + val lastResortKyberPreKey = pniProtocolStore.loadLastResortKyberPreKeys().firstOrNull { it.id == pniLastResortKyberPreKeyId } + val oneTimeKyberPreKeys = PreKeyUtil.generateAndStoreOneTimeKyberPreKeys(pniProtocolStore, pniMetadataStore) - val signedPreKey = pniProtocolStore.loadSignedPreKey(pniSignedPreyKeyId) - val oneTimeEcPreKeys = PreKeyUtil.generateAndStoreOneTimeEcPreKeys(pniProtocolStore, pniMetadataStore) - val lastResortKyberPreKey = pniProtocolStore.loadLastResortKyberPreKeys().firstOrNull { it.id == pniLastResortKyberPreKeyId } - val oneTimeKyberPreKeys = PreKeyUtil.generateAndStoreOneTimeKyberPreKeys(pniProtocolStore, pniMetadataStore) + if (lastResortKyberPreKey == null) { + Log.w(TAG, "Last-resort kyber prekey is missing!") + } - if (lastResortKyberPreKey == null) { - Log.w(TAG, "Last-resort kyber prekey is missing!") - } - - pniMetadataStore.activeSignedPreKeyId = signedPreKey.id - Log.i(TAG, "Submitting prekeys with PNI identity key: ${pniIdentityKeyPair.publicKey.fingerprint}") + pniMetadataStore.activeSignedPreKeyId = signedPreKey.id + Log.i(TAG, "Submitting prekeys with PNI identity key: ${pniIdentityKeyPair.publicKey.fingerprint}") + retryChangeLocalNumberNetworkOperation { SignalNetwork.keys.setPreKeys( PreKeyUpload( serviceIdType = ServiceIdType.PNI, @@ -168,25 +156,26 @@ class ChangeNumberRepository( lastResortKyberPreKey = lastResortKyberPreKey, oneTimeKyberPreKeys = oneTimeKyberPreKeys ) - ).successOrThrow() - pniMetadataStore.isSignedPreKeyRegistered = true - pniMetadataStore.lastResortKyberPreKeyId = pniLastResortKyberPreKeyId - - pniProtocolStore.identities().saveIdentityWithoutSideEffects( - Recipient.self().id, - pni, - pniProtocolStore.identityKeyPair.publicKey, - IdentityTable.VerifiedStatus.VERIFIED, - true, - System.currentTimeMillis(), - true ) + }.successOrThrow() - SignalStore.misc.hasPniInitializedDevices = true - AppDependencies.groupsV2Authorization.clear() - } + pniMetadataStore.isSignedPreKeyRegistered = true + pniMetadataStore.lastResortKyberPreKeyId = pniLastResortKyberPreKeyId - Recipient.self().live().refresh() + pniProtocolStore.identities().saveIdentityWithoutSideEffects( + Recipient.self().id, + pni, + pniProtocolStore.identityKeyPair.publicKey, + IdentityTable.VerifiedStatus.VERIFIED, + true, + System.currentTimeMillis(), + true + ) + + SignalStore.misc.hasPniInitializedDevices = true + AppDependencies.groupsV2Authorization.clear() + + Recipient.self().fresh() StorageSyncHelper.scheduleSyncForDataChange() AppDependencies.resetNetwork() @@ -195,6 +184,8 @@ class ChangeNumberRepository( AppDependencies.jobManager.add(RefreshAttributesJob()) rotateCertificates() + + SignalStore.misc.unlockChangeNumber() } @WorkerThread @@ -205,8 +196,8 @@ class ChangeNumberRepository( for (certificateType in certificateTypes) { val certificate: ByteArray? = when (certificateType) { - CertificateType.ACI_AND_E164 -> SignalNetwork.certificate.getSenderCertificate().successOrThrow() - CertificateType.ACI_ONLY -> SignalNetwork.certificate.getSenderCertificateForPhoneNumberPrivacy().successOrThrow() + CertificateType.ACI_AND_E164 -> retryChangeLocalNumberNetworkOperation { SignalNetwork.certificate.getSenderCertificate() }.successOrThrow() + CertificateType.ACI_ONLY -> retryChangeLocalNumberNetworkOperation { SignalNetwork.certificate.getSenderCertificateForPhoneNumberPrivacy() }.successOrThrow() else -> throw AssertionError() } @@ -216,6 +207,25 @@ class ChangeNumberRepository( } } + private fun retryChangeLocalNumberNetworkOperation(operation: () -> NetworkResult): NetworkResult { + var tries = 0 + var result = operation() + while (tries < 5) { + when (result) { + is NetworkResult.Success, + is NetworkResult.ApplicationError -> return result + is NetworkResult.StatusCodeError, + is NetworkResult.NetworkError -> Log.w(TAG, "Network related error attempting change number operation, try: $tries", result.getCause()) + } + + tries++ + BackoffUtil.exponentialBackoff(tries, 10.seconds.inWholeMilliseconds) + result = operation() + } + + return result + } + suspend fun changeNumberWithRecoveryPassword(recoveryPassword: String, newE164: String): ChangeNumberResult { return changeNumberInternal(recoveryPassword = recoveryPassword, newE164 = newE164) } @@ -265,6 +275,7 @@ class ChangeNumberRepository( ) SignalStore.misc.setPendingChangeNumberMetadata(metadata) + SignalStore.misc.lockChangeNumber() withContext(Dispatchers.IO) { result = SignalNetwork.account.changeNumber(request) } @@ -279,6 +290,11 @@ class ChangeNumberRepository( completed = true } } + + if (result is NetworkResult.StatusCodeError) { + SignalStore.misc.unlockChangeNumber() + } + Log.i(TAG, "Returning change number network result.") return ChangeNumberResult.from( result.map { accountRegistrationResponse: VerifyAccountResponse -> @@ -369,7 +385,9 @@ class ChangeNumberRepository( pniIdentityKeyPair = pniIdentity.serialize().toByteString(), pniRegistrationId = pniRegistrationIds[primaryDeviceId]!!, pniSignedPreKeyId = devicePniSignedPreKeys[primaryDeviceId]!!.keyId, - pniLastResortKyberPreKeyId = devicePniLastResortKyberPreKeys[primaryDeviceId]!!.keyId + pniLastResortKyberPreKeyId = devicePniLastResortKyberPreKeys[primaryDeviceId]!!.keyId, + previousE164 = SignalStore.account.requireE164(), + newE164 = newE164 ) return ChangeNumberRequestData(request, metadata) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberState.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberState.kt index 4e19fa8276..3c68ad863c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberState.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberState.kt @@ -47,3 +47,9 @@ sealed interface ChangeNumberOutcome { data object VerificationCodeWorked : ChangeNumberOutcome class ChangeNumberRequestOutcome(val result: VerificationCodeRequestResult) : ChangeNumberOutcome } + +sealed interface ChangeLocalNumberOutcome { + data object NotPerformed : ChangeLocalNumberOutcome + data object Success : ChangeLocalNumberOutcome + data object Failure : ChangeLocalNumberOutcome +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberVerifyFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberVerifyFragment.kt index b5ebf97c0f..caa485f43f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberVerifyFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberVerifyFragment.kt @@ -18,6 +18,7 @@ import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.LoggingFragment import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.components.settings.app.changenumber.ChangeNumberUtil.changeNumberSuccess +import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.registration.data.RegistrationRepository import org.thoughtcrime.securesms.registration.data.network.Challenge import org.thoughtcrime.securesms.registration.data.network.VerificationCodeRequestResult @@ -39,7 +40,7 @@ class ChangeNumberVerifyFragment : LoggingFragment(R.layout.fragment_change_phon val toolbar: Toolbar = view.findViewById(R.id.toolbar) toolbar.setTitle(R.string.ChangeNumberVerifyFragment__change_number) toolbar.setNavigationOnClickListener { - findNavController().navigateUp() + navigateUp() viewModel.resetLocalSessionState() } @@ -51,6 +52,16 @@ class ChangeNumberVerifyFragment : LoggingFragment(R.layout.fragment_change_phon requestCode() } + private fun navigateUp() { + if (SignalStore.misc.isChangeNumberLocked) { + Log.d(TAG, "Change number locked, navigateUp") + startActivity(ChangeNumberLockActivity.createIntent(requireContext())) + } else { + Log.d(TAG, "navigateUp") + findNavController().navigateUp() + } + } + private fun onStateUpdate(state: ChangeNumberState) { if (state.challengesRequested.contains(Challenge.CAPTCHA) && state.captchaToken.isNotNullOrBlank()) { viewModel.submitCaptchaToken(requireContext()) @@ -140,7 +151,7 @@ class ChangeNumberVerifyFragment : LoggingFragment(R.layout.fragment_change_phon MaterialAlertDialogBuilder(requireContext()).apply { setMessage(message) setPositiveButton(android.R.string.ok) { _, _ -> - findNavController().navigateUp() + navigateUp() viewModel.resetLocalSessionState() } show() diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModel.kt index f8e9d412dd..f219ca3bec 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModel.kt @@ -36,8 +36,6 @@ import org.thoughtcrime.securesms.registration.viewmodel.SvrAuthCredentialSet import org.thoughtcrime.securesms.util.dualsim.MccMncProducer import org.whispersystems.signalservice.api.push.ServiceId import java.io.IOException -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock /** * [ViewModel] for the change number flow. @@ -48,8 +46,6 @@ class ChangeNumberViewModel : ViewModel() { companion object { private val TAG = Log.tag(ChangeNumberViewModel::class.java) - - val CHANGE_NUMBER_LOCK = ReentrantLock() } private val repository = ChangeNumberRepository() @@ -165,7 +161,7 @@ class ChangeNumberViewModel : ViewModel() { fun resetLocalSessionState() { Log.v(TAG, "resetLocalSessionState()") store.update { - it.copy(inProgress = false, changeNumberOutcome = null, captchaToken = null, challengesRequested = emptyList(), allowedToRequestCode = false) + it.copy(inProgress = false, sessionId = null, changeNumberOutcome = null, captchaToken = null, challengesRequested = emptyList(), allowedToRequestCode = false) } } @@ -185,27 +181,47 @@ class ChangeNumberViewModel : ViewModel() { // region Public actions - fun checkWhoAmI(onSuccess: () -> Unit, onError: (Throwable) -> Unit) { - Log.v(TAG, "checkWhoAmI()") + fun reattemptChangeLocalNumber(onSuccess: () -> Unit, onError: (Throwable) -> Unit) { + require(SignalStore.misc.pendingChangeNumberMetadata != null) + val metadata = SignalStore.misc.pendingChangeNumberMetadata!! + + Log.v(TAG, "reattemptChangeLocalNumber()") viewModelScope.launch(Dispatchers.IO) { try { val whoAmI = repository.whoAmI() + val remoteE164 = whoAmI.number + val remotePni = ServiceId.PNI.parseOrThrow(whoAmI.pni) - if (whoAmI.number == SignalStore.account.e164) { - return@launch bail { Log.i(TAG, "Local and remote numbers match, nothing needs to be done.") } + val remoteMatchesPrevious = remoteE164 == metadata.previousE164 + val remoteMatchesNew = remoteE164 == metadata.newE164 + val remoteMatchesLocal = remoteE164 == SignalStore.account.requireE164() + + val reattempt = if (remoteMatchesLocal && remoteMatchesPrevious) { + Log.i(TAG, "Remote matches local and previous, no state has changed on server or local, can skip reattempt") + false + } else if (remoteMatchesNew) { + Log.i(TAG, "Remote matches new, should be safe to reattempt set local number") + true + } else { + Log.w(TAG, "Unexpected e164 state, remoteMatchesPrevious:$remoteMatchesPrevious remoteMatchesNew:$remoteMatchesNew remoteMatchesLocal:$remoteMatchesLocal") + withContext(Dispatchers.Main) { + onError(IllegalStateException("Unexpected e164 state")) + } + return@launch } - Log.i(TAG, "Local (${SignalStore.account.e164}) and remote (${whoAmI.number}) numbers do not match, updating local.") - - withLockOnSerialExecutor { - repository.changeLocalNumber(whoAmI.number, ServiceId.PNI.parseOrThrow(whoAmI.pni)) + if (reattempt) { + Log.i(TAG, "Reattempting local change.\n Local state (e164=${SignalStore.account.e164}, pni=${SignalStore.account.pni})\n Remote state (e164=${whoAmI.number}, pni=$remotePni)") + repository.changeLocalNumber(whoAmI.number, remotePni) } + SignalStore.misc.unlockChangeNumber() + withContext(Dispatchers.Main) { onSuccess() } } catch (ioException: IOException) { - Log.w(TAG, "Encountered an exception when requesting whoAmI()", ioException) + Log.w(TAG, "Encountered an exception when reattempting change local number", ioException) withContext(Dispatchers.Main) { onError(ioException) } @@ -230,7 +246,7 @@ class ChangeNumberViewModel : ViewModel() { ) } - viewModelScope.launch { + viewModelScope.launch(Dispatchers.IO) { verifyCodeInternal(context = context, pin = null, verificationErrorHandler = verificationErrorHandler, numberChangeErrorHandler = numberChangeErrorHandler) } } @@ -239,7 +255,7 @@ class ChangeNumberViewModel : ViewModel() { Log.v(TAG, "verifyCodeAndRegisterAccountWithRegistrationLock()") store.update { it.copy(inProgress = true) } - viewModelScope.launch { + viewModelScope.launch(Dispatchers.IO) { verifyCodeInternal(context = context, pin = pin, verificationErrorHandler = verificationErrorHandler, numberChangeErrorHandler = numberChangeErrorHandler) } } @@ -344,21 +360,18 @@ class ChangeNumberViewModel : ViewModel() { fun initiateChangeNumberSession(context: Context, mode: RegistrationRepository.E164VerificationMode) { Log.v(TAG, "changeNumber()") store.update { it.copy(inProgress = true) } - viewModelScope.launch { + viewModelScope.launch(Dispatchers.IO) { val encryptionDrained = repository.ensureDecryptionsDrained() ?: false if (!encryptionDrained) { return@launch bail { Log.i(TAG, "Failed to drain encryption.") } } - val changed = changeNumberWithRecoveryPassword() - - if (changed) { - Log.d(TAG, "Successfully changed number using recovery password, which cleaned up after itself.") - return@launch + when (changeNumberWithRecoveryPassword()) { + ChangeLocalNumberOutcome.NotPerformed -> requestVerificationCode(context, mode) + ChangeLocalNumberOutcome.Success -> Log.d(TAG, "Successfully changed number using recovery password") + ChangeLocalNumberOutcome.Failure -> Log.w(TAG, "Change number failed, bailing") } - - requestVerificationCode(context, mode) } } @@ -398,39 +411,47 @@ class ChangeNumberViewModel : ViewModel() { }) } - private suspend fun changeNumberWithRecoveryPassword(): Boolean { + private suspend fun changeNumberWithRecoveryPassword(): ChangeLocalNumberOutcome { Log.v(TAG, "changeNumberWithRecoveryPassword()") SignalStore.svr.recoveryPassword?.let { recoveryPassword -> val result = repository.changeNumberWithRecoveryPassword(recoveryPassword = recoveryPassword, newE164 = number.e164Number) if (result is ChangeNumberResult.Success) { - handleSuccessfulChangedRemoteNumber(e164 = result.numberChangeResult.number, pni = ServiceId.PNI.parseOrThrow(result.numberChangeResult.pni), changeNumberOutcome = ChangeNumberOutcome.RecoveryPasswordWorked) - return true + return handleSuccessfulChangedRemoteNumber(e164 = result.numberChangeResult.number, pni = ServiceId.PNI.parseOrThrow(result.numberChangeResult.pni), changeNumberOutcome = ChangeNumberOutcome.RecoveryPasswordWorked) + } else if (result is ChangeNumberResult.UnknownError) { + store.update { + it.copy(inProgress = false, changeNumberOutcome = ChangeNumberOutcome.ChangeNumberRequestOutcome(VerificationCodeRequestResult.UnknownError(result.getCause()))) + } + return ChangeLocalNumberOutcome.Failure } Log.d(TAG, "Encountered error while trying to change number with recovery password.", result.getCause()) } - return false + return ChangeLocalNumberOutcome.NotPerformed } - private suspend fun handleSuccessfulChangedRemoteNumber(e164: String, pni: ServiceId.PNI, changeNumberOutcome: ChangeNumberOutcome) { + private fun handleSuccessfulChangedRemoteNumber(e164: String, pni: ServiceId.PNI, changeNumberOutcome: ChangeNumberOutcome): ChangeLocalNumberOutcome { var result = changeNumberOutcome - Log.v(TAG, "handleSuccessfulChangedRemoteNumber(${result.javaClass.simpleName}") - try { - withLockOnSerialExecutor { - repository.changeLocalNumber(e164, pni) - } + + Log.v(TAG, "handleSuccessfulChangedRemoteNumber(${result.javaClass.simpleName})") + val changeLocalNumberResult: ChangeLocalNumberOutcome = try { + repository.changeLocalNumber(e164, pni) + SignalStore.misc.clearPendingChangeNumberMetadata() + ChangeLocalNumberOutcome.Success } catch (ioException: IOException) { Log.w(TAG, "Failed to change local number!", ioException) result = ChangeNumberOutcome.ChangeNumberRequestOutcome(VerificationCodeRequestResult.UnknownError(ioException)) + ChangeLocalNumberOutcome.Failure } store.update { it.copy(inProgress = false, changeNumberOutcome = result) } + + return changeLocalNumberResult } - private fun handleVerificationError(result: VerificationCodeRequestResult, verificationErrorHandler: (VerificationCodeRequestResult) -> Unit) { + private suspend fun handleVerificationError(result: VerificationCodeRequestResult, verificationErrorHandler: (VerificationCodeRequestResult) -> Unit) { Log.v(TAG, "handleVerificationError(${result.javaClass.simpleName}") when (result) { is VerificationCodeRequestResult.Success -> Unit @@ -446,11 +467,13 @@ class ChangeNumberViewModel : ViewModel() { else -> Log.i(TAG, "Received exception during verification.", result.getCause()) } - verificationErrorHandler(result) + withContext(Dispatchers.Main) { + verificationErrorHandler(result) + } } - private fun handleChangeNumberError(result: ChangeNumberResult, numberChangeErrorHandler: (ChangeNumberResult) -> Unit) { - Log.v(TAG, "handleChangeNumberError(${result.javaClass.simpleName}") + private suspend fun handleChangeNumberError(result: ChangeNumberResult, numberChangeErrorHandler: (ChangeNumberResult) -> Unit) { + Log.v(TAG, "handleChangeNumberError(${result.javaClass.simpleName})") when (result) { is ChangeNumberResult.Success -> Unit is ChangeNumberResult.RegistrationLocked -> @@ -472,7 +495,9 @@ class ChangeNumberViewModel : ViewModel() { else -> Log.i(TAG, "Received exception during change number.", result.getCause()) } - numberChangeErrorHandler(result) + withContext(Dispatchers.Main) { + numberChangeErrorHandler(result) + } } private suspend fun requestVerificationCode(context: Context, mode: RegistrationRepository.E164VerificationMode) { @@ -506,6 +531,11 @@ class ChangeNumberViewModel : ViewModel() { emptyList() } + if (result is VerificationCodeRequestResult.AlreadyVerified) { + Log.w(TAG, "Already verified, not handled properly in change number flow, attempt to recover") + resetLocalSessionState() + } + Log.d(TAG, "Received result: ${result.javaClass.canonicalName}\nwith challenges: ${challengesRequested.joinToString { it.key }}") store.update { @@ -538,24 +568,6 @@ class ChangeNumberViewModel : ViewModel() { } } - /** - * Anything that runs through this will be run serially, with locks. - */ - private suspend fun withLockOnSerialExecutor(action: () -> T): T = withContext(serialContext) { - Log.v(TAG, "withLock()") - val result = CHANGE_NUMBER_LOCK.withLock { - SignalStore.misc.lockChangeNumber() - Log.v(TAG, "Change number lock acquired.") - try { - action() - } finally { - SignalStore.misc.unlockChangeNumber() - } - } - Log.v(TAG, "Change number lock released.") - return@withContext result - } - // endregion enum class ContinueStatus { diff --git a/app/src/main/protowire/Database.proto b/app/src/main/protowire/Database.proto index 5504bd3295..e974d4c3d0 100644 --- a/app/src/main/protowire/Database.proto +++ b/app/src/main/protowire/Database.proto @@ -259,11 +259,13 @@ message SignalStoreList { } message PendingChangeNumberMetadata { - bytes previousPni = 1; - bytes pniIdentityKeyPair = 2; - int32 pniRegistrationId = 3; - int32 pniSignedPreKeyId = 4; - int32 pniLastResortKyberPreKeyId = 5; + bytes previousPni = 1; + bytes pniIdentityKeyPair = 2; + int32 pniRegistrationId = 3; + int32 pniSignedPreKeyId = 4; + int32 pniLastResortKyberPreKeyId = 5; + string previousE164 = 6; + string newE164 = 7; } message MessageExportState {