Ensure that backupSecretRestoreRequired is properly set.

This commit is contained in:
Greyson Parrelli
2026-08-04 15:57:33 -04:00
parent 7a10915733
commit dd76bae853
5 changed files with 47 additions and 2 deletions
@@ -578,6 +578,9 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
try {
when (val result = BackupRepository.restoreRemoteBackup()) {
is RemoteRestoreResult.Success -> {
// The restore adopted the account's existing SVRB chain, so there's nothing left for BackupMessagesJob to re-init.
SignalStore.backup.backupSecretRestoreRequired = false
send(
RemoteBackupRestoreProgress.Complete(
restoredSvrPin = SignalStore.svr.pin,
@@ -761,6 +764,13 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
SignalStore.backup.mediaRootBackupKey = MediaRootBackupKey(it.toByteArray())
}
if (accountData.reRegistration) {
// The account may already have SVRB data for this backup key. Until we've adopted that chain (either by restoring a backup, or by reading
// the remote metadata during the first BackupMessagesJob) we must not start a new chain, or the existing remote backup becomes unrestorable.
Log.i(TAG, "[applyAccountData] Re-registration. Marking that we need to restore SVRB secrets before backing up.")
SignalStore.backup.backupSecretRestoreRequired = true
}
SignalStore.account.setServicePassword(accountData.servicePassword)
SignalStore.account.setRegistered(registered = true, isAciChanged = isAciChanged)
TextSecurePreferences.setPromptedPushRegistration(context, true)
@@ -230,6 +230,34 @@ class AppRegistrationStorageControllerTest {
assertThat(SignalStore.svr.hasOptedOut()).isTrue()
}
@Test
fun `commit - re-registration - requires svrb secret restore before backing up`() = runBlocking<Unit> {
seedInProgressData(
RegistrationData(
accountData = accountData(reRegistration = true),
accountEntropyPool = aep.value
)
)
controller.commitRegistrationData()
assertThat(SignalStore.backup.backupSecretRestoreRequired).isTrue()
}
@Test
fun `commit - new account - does not require svrb secret restore`() = runBlocking<Unit> {
seedInProgressData(
RegistrationData(
accountData = accountData(reRegistration = false),
accountEntropyPool = aep.value
)
)
controller.commitRegistrationData()
assertThat(SignalStore.backup.backupSecretRestoreRequired).isFalse()
}
@Test
fun `commit - called twice - only applies account data once`() = runBlocking<Unit> {
seedInProgressData(
@@ -396,7 +424,7 @@ class AppRegistrationStorageControllerTest {
private fun readInProgressData(): RegistrationData = runBlocking { controller.readInProgressRegistrationData() }
private fun accountData(servicePassword: String = SERVICE_PASSWORD, linkedDeviceData: LinkedDeviceData? = null): AccountData {
private fun accountData(servicePassword: String = SERVICE_PASSWORD, linkedDeviceData: LinkedDeviceData? = null, reRegistration: Boolean = false): AccountData {
return AccountData(
aciIdentityKeyPair = aciIdentity.serialize().toByteString(),
pniIdentityKeyPair = pniIdentity.serialize().toByteString(),
@@ -410,7 +438,8 @@ class AppRegistrationStorageControllerTest {
pni = pni.toString(),
e164 = E164,
servicePassword = servicePassword,
linkedDeviceData = linkedDeviceData
linkedDeviceData = linkedDeviceData,
reRegistration = reRegistration
)
}
}
@@ -683,6 +683,7 @@ class RegistrationRepository(val context: Context, val networkController: Networ
this.aci = result.result.aci
this.pni = result.result.pni
this.servicePassword = keyMaterial.servicePassword
this.reRegistration = result.result.reregistration
}
storageController.commitRegistrationData()
}
@@ -45,6 +45,9 @@ message AccountData {
bool fetchesMessages = 14;
LinkedDeviceData linkedDeviceData = 15;
// Whether the service reported this registration as a re-registration of an existing account.
bool reRegistration = 16;
}
message SvrCredential {
@@ -141,6 +141,7 @@ class RegistrationEndToEndTest {
assert(committed.accountData?.pni?.isNotEmpty() == true) { "Expected committed PNI to be populated" }
assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
assert(committed.accountEntropyPool.isNotEmpty()) { "Expected committed AEP to be populated" }
assert(committed.accountData?.reRegistration == false) { "Expected a new registration to not be flagged as a re-registration" }
assert(networkController.lastCreateSessionE164 == E164) { "Expected a session for $E164 but was ${networkController.lastCreateSessionE164}" }
assert(networkController.lastRegisterAccountRequest?.e164 == E164) { "Expected registration for $E164 but was ${networkController.lastRegisterAccountRequest}" }
@@ -643,6 +644,7 @@ class RegistrationEndToEndTest {
assert(committed != null) { "Expected registration data to be committed" }
assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
assert(committed.accountData?.reRegistration == true) { "Expected the committed account data to be flagged as a re-registration" }
}
@Test