mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-07-21 13:05:00 +01:00
Fix lifecycle of regV5 scratch storage.
This commit is contained in:
+2
-10
@@ -67,7 +67,6 @@ import org.whispersystems.signalservice.api.link.TransferArchiveResponse
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
import java.time.LocalDateTime
|
||||
import kotlin.time.Duration.Companion.minutes
|
||||
|
||||
/**
|
||||
* Implementation of [StorageController] that bridges to the app's existing storage infrastructure.
|
||||
@@ -77,7 +76,6 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
|
||||
companion object {
|
||||
private val TAG = Log.tag(AppRegistrationStorageController::class)
|
||||
private const val TEMP_PROTO_FILENAME = "registration-in-progress.proto"
|
||||
private val TEMP_PROTO_TIMEOUT = 15.minutes
|
||||
private val MODERN_BACKUP_PATTERN = Regex("^signal-backup-(\\d{4})-(\\d{2})-(\\d{2})-(\\d{2})-(\\d{2})-(\\d{2})$")
|
||||
private val LEGACY_BACKUP_PATTERN = Regex("^signal-(\\d{4})-(\\d{2})-(\\d{2})-(\\d{2})-(\\d{2})-(\\d{2})\\.backup$")
|
||||
}
|
||||
@@ -155,13 +153,6 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
|
||||
override suspend fun readInProgressRegistrationData(): RegistrationData = withContext(Dispatchers.IO) {
|
||||
val file = File(context.cacheDir, TEMP_PROTO_FILENAME)
|
||||
if (file.exists()) {
|
||||
val age = System.currentTimeMillis() - file.lastModified()
|
||||
if (age > TEMP_PROTO_TIMEOUT.inWholeMilliseconds) {
|
||||
Log.w(TAG, "In-progress registration data is stale (${age}ms old), discarding.")
|
||||
file.delete()
|
||||
return@withContext RegistrationData()
|
||||
}
|
||||
|
||||
try {
|
||||
RegistrationData.ADAPTER.decode(file.readBytes())
|
||||
} catch (e: Exception) {
|
||||
@@ -537,7 +528,8 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
|
||||
}
|
||||
|
||||
private suspend fun writeRegistrationData(data: RegistrationData) = withContext(Dispatchers.IO) {
|
||||
val stamped = data.newBuilder().lastUpdatedMillis(System.currentTimeMillis()).build()
|
||||
val file = File(context.cacheDir, TEMP_PROTO_FILENAME)
|
||||
file.writeBytes(RegistrationData.ADAPTER.encode(data))
|
||||
file.writeBytes(RegistrationData.ADAPTER.encode(stamped))
|
||||
}
|
||||
}
|
||||
|
||||
+2
-1
@@ -440,7 +440,8 @@ class DemoStorageController(private val context: Context) : StorageController {
|
||||
}
|
||||
|
||||
private suspend fun writeRegistrationData(data: RegistrationData) = withContext(Dispatchers.IO) {
|
||||
val stamped = data.newBuilder().lastUpdatedMillis(System.currentTimeMillis()).build()
|
||||
val file = File(context.filesDir, TEMP_PROTO_FILENAME)
|
||||
file.writeBytes(RegistrationData.ADAPTER.encode(data))
|
||||
file.writeBytes(RegistrationData.ADAPTER.encode(stamped))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -798,6 +798,22 @@ class RegistrationRepository(val context: Context, val networkController: Networ
|
||||
storageController.clearLocalDataAndRestart()
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes all in-progress registration data. Called once registration is fully complete, so the scratch data is
|
||||
* never reused by a later flow.
|
||||
*/
|
||||
suspend fun clearInProgressRegistrationData() = withContext(Dispatchers.IO) {
|
||||
storageController.clearAllData()
|
||||
}
|
||||
|
||||
/**
|
||||
* The time the in-progress registration data was last written, as epoch milliseconds, or null if nothing has been
|
||||
* written yet. Read from the in-progress data's `lastUpdatedMillis`, which is stamped on every write.
|
||||
*/
|
||||
suspend fun getInProgressRegistrationDataLastUpdated(): Long? = withContext(Dispatchers.IO) {
|
||||
storageController.readInProgressRegistrationData().lastUpdatedMillis.takeIf { it > 0 }
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears any persisted flow state JSON from the in-progress registration data.
|
||||
*/
|
||||
|
||||
@@ -181,7 +181,7 @@ class RegistrationViewModel(private val repository: RegistrationRepository, save
|
||||
is RegistrationFlowEvent.ResetState -> repository.clearFlowState()
|
||||
is RegistrationFlowEvent.NavigateToScreen -> {
|
||||
if (event.route is RegistrationRoute.FullyComplete) {
|
||||
repository.clearFlowState()
|
||||
repository.clearInProgressRegistrationData()
|
||||
} else {
|
||||
repository.saveFlowState(_state.value)
|
||||
}
|
||||
@@ -195,7 +195,7 @@ class RegistrationViewModel(private val repository: RegistrationRepository, save
|
||||
is RegistrationFlowEvent.RestoreMethodTokenReceived,
|
||||
is RegistrationFlowEvent.UserSuppliedAepSubmitted,
|
||||
is RegistrationFlowEvent.UserSuppliedAepVerified -> repository.saveFlowState(_state.value)
|
||||
is RegistrationFlowEvent.RegistrationComplete -> repository.clearFlowState()
|
||||
is RegistrationFlowEvent.RegistrationComplete -> repository.clearInProgressRegistrationData()
|
||||
|
||||
// No need to persist anything new, fields accounted for in proto already
|
||||
is RegistrationFlowEvent.Registered,
|
||||
|
||||
+6
@@ -50,6 +50,8 @@ import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.text.input.KeyboardType
|
||||
import androidx.compose.ui.text.style.TextAlign
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.compose.LifecycleEventEffect
|
||||
import kotlinx.coroutines.delay
|
||||
import org.signal.core.ui.compose.AllDevicePreviews
|
||||
import org.signal.core.ui.compose.Previews
|
||||
@@ -146,6 +148,10 @@ fun VerificationCodeScreen(
|
||||
focusRequesters[0].requestFocus()
|
||||
}
|
||||
|
||||
LifecycleEventEffect(Lifecycle.Event.ON_RESUME) {
|
||||
onEvent(VerificationCodeScreenEvents.Foregrounded)
|
||||
}
|
||||
|
||||
Scaffold(
|
||||
snackbarHost = { SnackbarHost(snackbarHostState) },
|
||||
modifier = modifier
|
||||
|
||||
+6
@@ -35,4 +35,10 @@ sealed class VerificationCodeScreenEvents {
|
||||
* Event to update countdown timers. Should be triggered periodically (e.g., every second).
|
||||
*/
|
||||
data object CountdownTick : VerificationCodeScreenEvents()
|
||||
|
||||
/**
|
||||
* The screen returned to the foreground. Used to check whether the in-progress registration data (and thus the
|
||||
* verification session) has grown too stale to keep waiting on, in which case we restart the flow.
|
||||
*/
|
||||
data object Foregrounded : VerificationCodeScreenEvents()
|
||||
}
|
||||
|
||||
+24
@@ -41,6 +41,7 @@ import org.signal.registration.screens.util.navigateTo
|
||||
import org.signal.registration.screens.verificationcode.VerificationCodeState.OneTimeEvent
|
||||
import kotlin.time.Duration
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
import kotlin.time.Duration.Companion.minutes
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
|
||||
class VerificationCodeViewModel(
|
||||
@@ -54,6 +55,12 @@ class VerificationCodeViewModel(
|
||||
companion object {
|
||||
private val TAG = Log.tag(VerificationCodeViewModel::class)
|
||||
|
||||
/**
|
||||
* How old the in-progress registration data can be before we assume the verification session has expired and
|
||||
* restart the flow. Checked whenever the screen is foregrounded.
|
||||
*/
|
||||
private val IN_PROGRESS_DATA_TIMEOUT = 15.minutes
|
||||
|
||||
/**
|
||||
* Cold [Flow] of verification codes automatically retrieved from incoming SMS messages via the Play Services SMS
|
||||
* retriever. Registers a [BroadcastReceiver] for [SmsRetriever.SMS_RETRIEVED_ACTION] while collected, and
|
||||
@@ -123,10 +130,27 @@ class VerificationCodeViewModel(
|
||||
is VerificationCodeScreenEvents.HavingTrouble -> throw NotImplementedError("having trouble flow") // TODO [registration] - Having trouble flow
|
||||
is VerificationCodeScreenEvents.ConsumeInnerOneTimeEvent -> state.copy(oneTimeEvent = null)
|
||||
is VerificationCodeScreenEvents.CountdownTick -> applyCountdownTick(state)
|
||||
is VerificationCodeScreenEvents.Foregrounded -> applyForegrounded(state)
|
||||
}
|
||||
stateEmitter(result)
|
||||
}
|
||||
|
||||
/**
|
||||
* If the in-progress registration data has grown older than [IN_PROGRESS_DATA_TIMEOUT], the verification session has
|
||||
* likely expired server-side. Rather than let the user enter a code only to fail, restart the flow from the beginning.
|
||||
*/
|
||||
private suspend fun applyForegrounded(state: VerificationCodeState): VerificationCodeState {
|
||||
val lastUpdated = repository.getInProgressRegistrationDataLastUpdated() ?: return state
|
||||
val age = (clock() - lastUpdated).milliseconds
|
||||
|
||||
if (age >= IN_PROGRESS_DATA_TIMEOUT) {
|
||||
Log.w(TAG, "[Foregrounded] In-progress registration data is stale (${age.inWholeMilliseconds}ms old). Restarting the flow.")
|
||||
parentEventEmitter(RegistrationFlowEvent.ResetState)
|
||||
}
|
||||
|
||||
return state
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
fun applyParentState(state: VerificationCodeState, parentState: RegistrationFlowState): VerificationCodeState {
|
||||
if (parentState.sessionMetadata == null || parentState.sessionE164 == null) {
|
||||
|
||||
@@ -39,7 +39,11 @@ message RegistrationData {
|
||||
bool fetchesMessages = 23;
|
||||
|
||||
string flowStateJson = 24;
|
||||
// Next: 25
|
||||
|
||||
// Epoch milliseconds of the last write to this data. Used to detect data that has grown too stale to trust (e.g. an
|
||||
// expired verification session). Stamped on every write.
|
||||
int64 lastUpdatedMillis = 25;
|
||||
// Next: 26
|
||||
}
|
||||
|
||||
message SvrCredential {
|
||||
|
||||
+4
-4
@@ -201,7 +201,7 @@ class RegistrationViewModelTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `onEvent NavigateToScreen FullyComplete clears flow state`() = runTest(testDispatcher) {
|
||||
fun `onEvent NavigateToScreen FullyComplete clears in-progress registration data`() = runTest(testDispatcher) {
|
||||
coEvery { mockRepository.restoreFlowState() } returns null
|
||||
coEvery { mockRepository.getPreExistingRegistrationData() } returns null
|
||||
|
||||
@@ -211,7 +211,7 @@ class RegistrationViewModelTest {
|
||||
viewModel.onEvent(RegistrationFlowEvent.NavigateToScreen(RegistrationRoute.FullyComplete))
|
||||
advanceUntilIdle()
|
||||
|
||||
coVerify { mockRepository.clearFlowState() }
|
||||
coVerify { mockRepository.clearInProgressRegistrationData() }
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -257,7 +257,7 @@ class RegistrationViewModelTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `onEvent RegistrationComplete commits final data and clears flow state`() = runTest(testDispatcher) {
|
||||
fun `onEvent RegistrationComplete commits final data and clears in-progress registration data`() = runTest(testDispatcher) {
|
||||
coEvery { mockRepository.restoreFlowState() } returns null
|
||||
coEvery { mockRepository.getPreExistingRegistrationData() } returns null
|
||||
|
||||
@@ -268,7 +268,7 @@ class RegistrationViewModelTest {
|
||||
advanceUntilIdle()
|
||||
|
||||
coVerify { mockRepository.commitFinalRegistrationData() }
|
||||
coVerify { mockRepository.clearFlowState() }
|
||||
coVerify { mockRepository.clearInProgressRegistrationData() }
|
||||
}
|
||||
|
||||
// ==================== applyEvent Tests (Navigation & State Reducers) ====================
|
||||
|
||||
+35
@@ -37,6 +37,7 @@ import org.signal.registration.RegistrationFlowEvent
|
||||
import org.signal.registration.RegistrationFlowState
|
||||
import org.signal.registration.RegistrationRepository
|
||||
import org.signal.registration.RegistrationRoute
|
||||
import kotlin.time.Duration.Companion.minutes
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
@@ -869,6 +870,40 @@ class VerificationCodeViewModelTest {
|
||||
assertThat(emittedStates.last().oneTimeEvent).isEqualTo(VerificationCodeState.OneTimeEvent.UnableToSendSms)
|
||||
}
|
||||
|
||||
// ==================== applyEvent: Foregrounded Tests ====================
|
||||
|
||||
@Test
|
||||
fun `Foregrounded emits ResetState when in-progress data is older than the timeout`() = runTest {
|
||||
val now = 100.minutes.inWholeMilliseconds
|
||||
coEvery { mockRepository.getInProgressRegistrationDataLastUpdated() } returns now - 16.minutes.inWholeMilliseconds
|
||||
|
||||
val vm = VerificationCodeViewModel(mockRepository, parentState, parentEventEmitter, clock = { now })
|
||||
vm.applyEvent(VerificationCodeState(), VerificationCodeScreenEvents.Foregrounded, stateEmitter)
|
||||
|
||||
assertThat(emittedEvents).hasSize(1)
|
||||
assertThat(emittedEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Foregrounded does not emit ResetState when in-progress data is within the timeout`() = runTest {
|
||||
val now = 100.minutes.inWholeMilliseconds
|
||||
coEvery { mockRepository.getInProgressRegistrationDataLastUpdated() } returns now - 14.minutes.inWholeMilliseconds
|
||||
|
||||
val vm = VerificationCodeViewModel(mockRepository, parentState, parentEventEmitter, clock = { now })
|
||||
vm.applyEvent(VerificationCodeState(), VerificationCodeScreenEvents.Foregrounded, stateEmitter)
|
||||
|
||||
assertThat(emittedEvents).hasSize(0)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Foregrounded does not emit ResetState when there is no in-progress data`() = runTest {
|
||||
coEvery { mockRepository.getInProgressRegistrationDataLastUpdated() } returns null
|
||||
|
||||
viewModel.applyEvent(VerificationCodeState(), VerificationCodeScreenEvents.Foregrounded, stateEmitter)
|
||||
|
||||
assertThat(emittedEvents).hasSize(0)
|
||||
}
|
||||
|
||||
// ==================== Helper Functions ====================
|
||||
|
||||
private fun createSessionMetadata(
|
||||
|
||||
Reference in New Issue
Block a user