From b3857e4b35c5186ac1cd4fa172796c2cbbaef5e6 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 15 Jul 2026 16:56:59 -0400 Subject: [PATCH] Handle missing sessions when submitting captcha in regV5. --- .../v2/AppRegistrationNetworkController.kt | 3 + .../dependencies/DemoNetworkController.kt | 3 + .../signal/registration/NetworkController.kt | 1 + .../phonenumber/PhoneNumberEntryViewModel.kt | 10 +++ .../registration/RegistrationEndToEndTest.kt | 73 ++++++++++++++++++- .../PhoneNumberEntryViewModelTest.kt | 43 +++++++++++ 6 files changed, 132 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationNetworkController.kt b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationNetworkController.kt index af6c10c9a2..65c90166e3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationNetworkController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationNetworkController.kt @@ -212,6 +212,9 @@ class AppRegistrationNetworkController( 400 -> { RequestResult.NonSuccess(UpdateSessionError.InvalidRequest(response.body.string())) } + 404 -> { + RequestResult.NonSuccess(UpdateSessionError.SessionNotFound(response.body.string())) + } 409 -> { RequestResult.NonSuccess(UpdateSessionError.RejectedUpdate(response.body.string())) } diff --git a/demo/registration/src/main/java/org/signal/registration/sample/dependencies/DemoNetworkController.kt b/demo/registration/src/main/java/org/signal/registration/sample/dependencies/DemoNetworkController.kt index 079514c474..5feb47e4a4 100644 --- a/demo/registration/src/main/java/org/signal/registration/sample/dependencies/DemoNetworkController.kt +++ b/demo/registration/src/main/java/org/signal/registration/sample/dependencies/DemoNetworkController.kt @@ -214,6 +214,9 @@ class DemoNetworkController( 400 -> { RequestResult.NonSuccess(UpdateSessionError.InvalidRequest(response.body.string())) } + 404 -> { + RequestResult.NonSuccess(UpdateSessionError.SessionNotFound(response.body.string())) + } 409 -> { RequestResult.NonSuccess(UpdateSessionError.RejectedUpdate(response.body.string())) } diff --git a/feature/registration/src/main/java/org/signal/registration/NetworkController.kt b/feature/registration/src/main/java/org/signal/registration/NetworkController.kt index e5aac119d1..19abfb4e8e 100644 --- a/feature/registration/src/main/java/org/signal/registration/NetworkController.kt +++ b/feature/registration/src/main/java/org/signal/registration/NetworkController.kt @@ -371,6 +371,7 @@ interface NetworkController { sealed class UpdateSessionError : BadRequestError { data class RejectedUpdate(val message: String) : UpdateSessionError() + data class SessionNotFound(val message: String) : UpdateSessionError() data class InvalidRequest(val message: String) : UpdateSessionError() data class RateLimited(val retryAfter: Duration, val session: SessionMetadata) : UpdateSessionError() } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt index d51aee6bc9..3d0005c1ee 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModel.kt @@ -544,6 +544,11 @@ class PhoneNumberEntryViewModel( updateResult.result } is RequestResult.NonSuccess -> { + if (updateResult.error is NetworkController.UpdateSessionError.SessionNotFound) { + Log.w(TAG, "[SubmitPushChallengeToken] Session not found when submitting push challenge token.") + parentEventEmitter(RegistrationFlowEvent.ResetState) + return state + } Log.w(TAG, "[SubmitPushChallengeToken] Failed to submit push challenge token: ${updateResult.error}") sessionMetadata } @@ -658,6 +663,11 @@ class PhoneNumberEntryViewModel( is NetworkController.UpdateSessionError.RejectedUpdate -> { state.copy(dialogs = state.dialogs.copy(unknownError = true)) } + is NetworkController.UpdateSessionError.SessionNotFound -> { + Log.w(TAG, "[SubmitCaptcha] Session not found when submitting captcha token.") + parentEventEmitter(RegistrationFlowEvent.ResetState) + state + } is NetworkController.UpdateSessionError.RateLimited -> { state.copy(dialogs = state.dialogs.copy(rateLimitedRetryAfter = error.retryAfter)) } 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 d888ee9a9d..18b74050c2 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -8,6 +8,9 @@ package org.signal.registration import android.app.Application import android.net.Uri import android.os.Looper +import android.view.View +import android.view.ViewGroup +import android.webkit.WebView import androidx.activity.compose.LocalActivityResultRegistryOwner import androidx.activity.result.ActivityResultRegistry import androidx.activity.result.ActivityResultRegistryOwner @@ -15,6 +18,8 @@ import androidx.activity.result.contract.ActivityResultContract import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.remember +import androidx.compose.ui.InternalComposeUiApi +import androidx.compose.ui.platform.ViewRootForTest import androidx.compose.ui.semantics.SemanticsProperties import androidx.compose.ui.semantics.getOrNull import androidx.compose.ui.test.junit4.createComposeRule @@ -50,6 +55,7 @@ import org.signal.registration.NetworkController.RegistrationLockResponse import org.signal.registration.NetworkController.RestoreMasterKeyError import org.signal.registration.NetworkController.RestoreMethod import org.signal.registration.NetworkController.SvrCredentials +import org.signal.registration.NetworkController.UpdateSessionError import org.signal.registration.fakes.FakeNetworkController import org.signal.registration.fakes.FakeStorageController import org.signal.registration.fakes.SystemOutLogger @@ -68,7 +74,7 @@ import kotlin.time.Duration.Companion.days * The fakes default to a happy path. To exercise other navigation paths, override the relevant * response handler on [networkController] or state on [storageController] before driving the UI. */ -@OptIn(ExperimentalPermissionsApi::class) +@OptIn(ExperimentalPermissionsApi::class, InternalComposeUiApi::class) @RunWith(RobolectricTestRunner::class) @Config(application = Application::class) class RegistrationEndToEndTest { @@ -134,6 +140,44 @@ class RegistrationEndToEndTest { assert(storageController.restoreDecision == RestoreDecision.NEW_ACCOUNT) { "Expected NEW_ACCOUNT restore decision but was ${storageController.restoreDecision}" } } + @Test + fun `a captcha submission for a session that no longer exists resets the flow, which can then be restarted to completion`() { + // The session demands a captcha, but expires server-side before the solved captcha is submitted + networkController.onCreateSession = { + RequestResult.Success(networkController.session(allowedToRequestCode = false, requestedInformation = listOf("captcha"))) + } + networkController.onUpdateSession = { + RequestResult.NonSuccess(UpdateSessionError.SessionNotFound("no session found")) + } + + var registrationComplete = false + launchRegistrationFlow(onRegistrationComplete = { registrationComplete = true }) + + submitPhoneNumber() + solveCaptcha("captcha-token") + + // The server no longer knows the session, so the flow resets back to the beginning + waitForTag(TestTags.WELCOME_SCREEN) + assert(networkController.lastUpdateSessionRequest?.captchaToken == "captcha-token") { + "Expected the solved captcha to be submitted but was ${networkController.lastUpdateSessionRequest}" + } + assert(storageController.committedData == null) { "Expected no registration data to be committed" } + + // Starting over against a healthy server completes registration + networkController.onCreateSession = { RequestResult.Success(networkController.session()) } + networkController.onUpdateSession = { RequestResult.Success(networkController.session()) } + + submitPhoneNumber() + submitVerificationCode(VERIFICATION_CODE) + createPin(PIN) + + waitFor("registration to complete") { registrationComplete } + + val committed = storageController.committedData + assert(committed != null) { "Expected registration data to be committed" } + assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" } + } + @Test fun `a registration lock is unlocked by entering the existing pin and registration completes`() { val masterKey = MasterKey(ByteArray(32) { it.toByte() }) @@ -1094,6 +1138,33 @@ class RegistrationEndToEndTest { composeTestRule.onNodeWithTag(TestTags.REMOTE_BACKUP_RESTORE_RESTORE_BUTTON).performClick() } + /** + * From the captcha screen: simulates the user solving the captcha by driving the WebView's client with the + * `signalcaptcha://` redirect that a real solve produces. + */ + @Suppress("DEPRECATION") + private fun solveCaptcha(token: String) { + var webView: WebView? = null + waitFor("the captcha WebView") { + val composeView = (composeTestRule.onNodeWithTag(TestTags.CAPTCHA_SCREEN).fetchSemanticsNode().root as ViewRootForTest).view + webView = findWebView(composeView) + webView != null + } + webView!!.let { Shadows.shadowOf(it).webViewClient.shouldOverrideUrlLoading(it, "signalcaptcha://$token") } + } + + private fun findWebView(view: View): WebView? { + if (view is WebView) { + return view + } + if (view is ViewGroup) { + for (i in 0 until view.childCount) { + findWebView(view.getChildAt(i))?.let { return it } + } + } + return null + } + /** From the verification code screen: enters all six digits of [code], which submits automatically. */ private fun submitVerificationCode(code: String) { waitForTag(TestTags.VERIFICATION_CODE_DIGIT_0) diff --git a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt index 63c816253f..fec97b6096 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/phonenumber/PhoneNumberEntryViewModelTest.kt @@ -1121,6 +1121,33 @@ class PhoneNumberEntryViewModelTest { .isInstanceOf() } + @Test + fun `PhoneNumberSubmitted with push challenge resets state when session not found`() = runTest { + val sessionWithPushChallenge = createSessionMetadata(requestedInformation = listOf("pushChallenge")) + + coEvery { mockRepository.createSession(any()) } returns + RequestResult.Success(sessionWithPushChallenge) + coEvery { mockRepository.awaitPushChallengeToken() } returns "test-push-challenge-token" + coEvery { mockRepository.submitPushChallengeToken(any(), any()) } returns + RequestResult.NonSuccess( + NetworkController.UpdateSessionError.SessionNotFound("Session expired") + ) + + val initialState = PhoneNumberEntryState( + countryCode = "1", + nationalNumber = "5551234567" + ) + + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.PhoneNumberConfirmed, parentEventEmitter, stateEmitter) + + // Verify spinner states + assertThat(emittedStates.first().showSpinner).isTrue() + assertThat(emittedStates.last().showSpinner).isFalse() + + assertThat(emittedEvents).hasSize(1) + assertThat(emittedEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState) + } + // ==================== CaptchaCompleted Tests ==================== @Test @@ -1203,6 +1230,22 @@ class PhoneNumberEntryViewModelTest { assertThat(emittedStates.last().dialogs.unknownError).isTrue() } + @Test + fun `CaptchaCompleted handles session not found`() = runTest { + val sessionMetadata = createSessionMetadata() + val initialState = PhoneNumberEntryState(sessionMetadata = sessionMetadata) + + coEvery { mockRepository.submitCaptchaToken(any(), any()) } returns + RequestResult.NonSuccess( + NetworkController.UpdateSessionError.SessionNotFound("Session expired") + ) + + viewModel.applyEvent(initialState, PhoneNumberEntryScreenEvents.CaptchaCompleted("captcha-token"), parentEventEmitter, stateEmitter) + + assertThat(emittedEvents).hasSize(1) + assertThat(emittedEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState) + } + @Test fun `CaptchaCompleted handles network error`() = runTest { val sessionMetadata = createSessionMetadata()