diff --git a/ADAPTER_ARCHITECTURE.md b/ADAPTER_ARCHITECTURE.md index c84f06dee..cb892ca2b 100644 --- a/ADAPTER_ARCHITECTURE.md +++ b/ADAPTER_ARCHITECTURE.md @@ -294,9 +294,18 @@ retrying a connection, and completing sign-in. - Cancellation releases challenge bookkeeping and rejects late results before they can persist credentials. The existing authenticated-session transition still owns the final credential commit. -- Only a classified DNS failure before an HTTP exchange may retry - automatically. An ambiguous response after the one-time exchange requires a - new sign-in attempt. +- Only a classified failure while the connection is being established, before + an HTTP exchange starts, may retry automatically: unresolved DNS, a refused + or timed-out connection, an unreachable network, a TLS timeout, or a socket + failure during DNS lookup or connect. Certificate, TLS handshake, protocol, + and unclassified failures end the attempt. +- A failure after the one-time exchange starts, or an invalid one-time approval + response, is ambiguous. It is never retried and requires a new sign-in + attempt. +- Each poll uses its own HTTP client with OkHttp connection-failure retry + disabled and a connection pool that keeps no idle connection, so a poll never + reuses a connection from an earlier poll or attempt. Cancelling the attempt + cancels the poll's calls, including one that has not been sent yet. - Retry copy describes the last observed failure. It cannot report the current health of DNS, the server, or a VPN. diff --git a/androidApp/src/androidTest/kotlin/dev/obiente/nextcloudnative/AndroidLoginPollTransportInstrumentedTest.kt b/androidApp/src/androidTest/kotlin/dev/obiente/nextcloudnative/AndroidLoginPollTransportInstrumentedTest.kt new file mode 100644 index 000000000..c22890db5 --- /dev/null +++ b/androidApp/src/androidTest/kotlin/dev/obiente/nextcloudnative/AndroidLoginPollTransportInstrumentedTest.kt @@ -0,0 +1,210 @@ +package dev.obiente.nextcloudnative + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import dev.obiente.nextcloudnative.app.JvmNetworkFailureDiagnostic +import dev.obiente.nextcloudnative.app.JvmNetworkRequestAttempt +import dev.obiente.nextcloudnative.app.LoginChallenge +import dev.obiente.nextcloudnative.app.LoginPollHttpResponse +import dev.obiente.nextcloudnative.app.LoginPollResult +import dev.obiente.nextcloudnative.app.executeLoginPollHttp +import dev.obiente.nextcloudnative.app.toJvmNetworkFailureDiagnostic +import dev.obiente.nextcloudnative.app.trackJvmNetworkFailures +import dev.obiente.nextcloudnative.app.withLoginPollHttpClient +import java.io.IOException +import java.net.ServerSocket +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import mockwebserver3.Dispatcher +import mockwebserver3.MockResponse +import mockwebserver3.MockWebServer +import mockwebserver3.RecordedRequest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.tls.HandshakeCertificates +import okhttp3.tls.HeldCertificate +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Runs Login Flow v2 poll isolation and cancellation on the Android runtime and its TLS stack. + * + * All servers are local synthetic fixtures; no request leaves the device. + */ +@RunWith(AndroidJUnit4::class) +class AndroidLoginPollTransportInstrumentedTest { + private val certificate = HeldCertificate.Builder() + .commonName("localhost") + .addSubjectAlternativeName("localhost") + .build() + + @Test + fun everyPollUsesANewTlsConnectionUntilApproval() = runBlocking { + val connections = CopyOnWriteArrayList() + withServer( + dispatcher = { request, server -> + connections += request.connectionIndex + if (connections.size < 3) pending() else server.approved() + }, + ) { server, base -> + val challenge = server.challenge() + val results = List(3) { + withLoginPollHttpClient(base) { client -> poll(client, challenge) } + } + + assertEquals(LoginPollResult.Pending, results[0]) + assertEquals(LoginPollResult.Pending, results[1]) + assertTrue(results[2] is LoginPollResult.Approved) + assertEquals(3, connections.distinct().size) + } + } + + @Test + fun cancellingTheAttemptCancelsAPollHeldByTheServer() = runBlocking { + val arrived = CountDownLatch(1) + val release = CountDownLatch(1) + withServer( + dispatcher = { _, _ -> + arrived.countDown() + release.await(30, TimeUnit.SECONDS) + pending() + }, + onClose = release::countDown, + ) { server, base -> + val attempt = AtomicReference(null) + var completed = false + val job = launch(Dispatchers.IO) { + withLoginPollHttpClient(base) { client -> + poll(client, server.challenge(), onAttempt = attempt::set) + } + completed = true + } + assertTrue(arrived.await(10, TimeUnit.SECONDS)) + + job.cancel() + assertTrue(requireNotNull(attempt.get()).cancelled) + withTimeout(5_000) { job.join() } + + assertTrue(job.isCancelled) + assertFalse(completed) + } + } + + @Test + fun refusedConnectionBeforeTheExchangeIsRetryable() = runBlocking { + val closedPort = ServerSocket(0).use { it.localPort } + val pollEndpoint = "https://localhost:$closedPort/login/v2/poll" + val challenge = LoginChallenge( + enteredServerUrl = "https://localhost:$closedPort", + pollEndpoint = pollEndpoint, + pollFallbackEndpoint = null, + token = "synthetic-one-time-token", + loginUrl = "https://localhost:$closedPort/login/v2/flow/synthetic", + ) + + val result = withLoginPollHttpClient(trustingClient()) { client -> poll(client, challenge) } + + assertTrue(result is LoginPollResult.RetryablePreExchangeFailure) + assertEquals("NETWORK_CONNECT_FAILED", (result as LoginPollResult.RetryablePreExchangeFailure).code) + } + + private suspend fun withServer( + dispatcher: (RecordedRequest, MockWebServer) -> MockResponse, + onClose: () -> Unit = {}, + test: suspend (MockWebServer, OkHttpClient) -> Unit, + ) { + MockWebServer().use { server -> + server.useHttps( + HandshakeCertificates.Builder().heldCertificate(certificate).build().sslSocketFactory(), + ) + server.dispatcher = object : Dispatcher() { + override fun dispatch(request: RecordedRequest): MockResponse = dispatcher(request, server) + } + server.start() + try { + test(server, trustingClient()) + } finally { + onClose() + } + } + } + + private fun trustingClient(): OkHttpClient { + val trusted = HandshakeCertificates.Builder() + .addTrustedCertificate(certificate.certificate) + .build() + return OkHttpClient.Builder() + .sslSocketFactory(trusted.sslSocketFactory(), trusted.trustManager) + .trackJvmNetworkFailures() + .build() + } + + private suspend fun poll( + client: OkHttpClient, + challenge: LoginChallenge, + onAttempt: (JvmNetworkRequestAttempt) -> Unit = {}, + ): LoginPollResult { + var networkFailure: JvmNetworkFailureDiagnostic? = null + return executeLoginPollHttp( + challenge = challenge, + fallbackAlreadySelected = false, + poll = { endpoint -> + networkFailure = null + val attempt = JvmNetworkRequestAttempt().also(onAttempt) + val request = Request.Builder() + .url(endpoint) + .post("token=synthetic-one-time-token".toRequestBody(FORM)) + .tag(JvmNetworkRequestAttempt::class.java, attempt) + .build() + try { + client.newCall(request).execute().use { response -> + LoginPollHttpResponse(response.code, response.body.string()) + } + } catch (failure: IOException) { + networkFailure = failure.toJvmNetworkFailureDiagnostic( + attempt = attempt, + readOnlyRequest = false, + replayableRequest = false, + ) + throw failure + } + }, + networkFailure = { networkFailure }, + ).interpretation.result + } + + private fun MockWebServer.challenge(): LoginChallenge { + val origin = url("/").toString().trimEnd('/') + return LoginChallenge( + enteredServerUrl = origin, + pollEndpoint = "$origin/login/v2/poll", + pollFallbackEndpoint = null, + token = "synthetic-one-time-token", + loginUrl = "$origin/login/v2/flow/synthetic", + ) + } + + private fun pending() = MockResponse.Builder().code(404).body("[]").build() + + private fun MockWebServer.approved() = MockResponse.Builder() + .code(200) + .body( + """{"server":"${url("/").toString().trimEnd('/')}","loginName":"person",""" + + """"appPassword":"synthetic-app-password"}""", + ) + .build() + + private companion object { + val FORM = "application/x-www-form-urlencoded".toMediaType() + } +} diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt index 9ad3dae42..9a1c60e3f 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt @@ -31,6 +31,7 @@ import dev.obiente.nextcloudnative.app.LoginTransportSecurity import dev.obiente.nextcloudnative.app.LOGIN_FLOW_RESPONSE_MAX_BYTES import dev.obiente.nextcloudnative.app.LoginPollHttpResponse import dev.obiente.nextcloudnative.app.executeLoginPollHttp +import dev.obiente.nextcloudnative.app.withLoginPollHttpClient import dev.obiente.nextcloudnative.app.interpretLoginChallengeHttpResponse import dev.obiente.nextcloudnative.app.loginPollEndpointFallbackDiagnostic import dev.obiente.nextcloudnative.app.normalizeServerUrl @@ -347,7 +348,6 @@ internal class AndroidNextcloudServices( .useAndroidNextcloudCertificateTrust(appContext) .trackJvmNetworkFailures() .build() - private val loginPollHttpClient = httpClient.newBuilder().retryOnConnectionFailure(false).build() private val loginPollFallbackTokens = ConcurrentHashMap.newKeySet() private val loginPollPendingTokens = ConcurrentHashMap.newKeySet() private val noRedirectHttpClient = httpClient.newBuilder() @@ -1202,7 +1202,7 @@ internal class AndroidNextcloudServices( override suspend fun pollLogin(challenge: LoginChallenge): LoginPollResult = withContext(Dispatchers.IO) { val formBody = "token=" + URLEncoder.encode(challenge.token, StandardCharsets.UTF_8.name()) var networkFailure: JvmNetworkFailureDiagnostic? = null - val execution = executeLoginPollHttp( + val execution = withLoginPollHttpClient(httpClient) { loginPollHttpClient -> executeLoginPollHttp( challenge = challenge, fallbackAlreadySelected = challenge.token in loginPollFallbackTokens, poll = { endpoint -> @@ -1219,7 +1219,7 @@ internal class AndroidNextcloudServices( ).let { LoginPollHttpResponse(it.status, it.text) } }, networkFailure = { networkFailure }, - ) + ) } execution.selectedFallbackReason?.let { reason -> loginPollFallbackTokens += challenge.token runCatching { @@ -3730,9 +3730,6 @@ internal class AndroidNextcloudServices( LoginTransportSecurity.Tls } - private val LoginTransportSecurity.diagnosticValue: String - get() = if (this == LoginTransportSecurity.PlainHttp) "plaintext" else "tls" - private fun JSONArray.toAppEntries(): List = buildList { for (index in 0 until length()) { val item = optJSONObject(index) ?: continue diff --git a/changes/unreleased/488-login-poll-fresh-connections.md b/changes/unreleased/488-login-poll-fresh-connections.md new file mode 100644 index 000000000..c43416d6a --- /dev/null +++ b/changes/unreleased/488-login-poll-fresh-connections.md @@ -0,0 +1,7 @@ +category: fix +issue: 488 +pull: none +platforms: android, desktop +user-facing: yes + +Keep browser sign-in working after returning to the app during approval: each check uses a new connection, and connection failures before a request is sent are retried. Cancelling sign-in stops a check in flight. diff --git a/docs/login-diagnostics.md b/docs/login-diagnostics.md index ca98132fd..c3fad7475 100644 --- a/docs/login-diagnostics.md +++ b/docs/login-diagnostics.md @@ -1,6 +1,6 @@ # Sign-in recovery and login diagnostics -**Last reviewed: 2026-09-28.** This describes repository implementation, which may +**Last reviewed: 2026-10-08.** This describes repository implementation, which may have changed. Check the [current source](https://github.com/obiente/native/blob/main/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt) and the release notes for the installed build before assuming a released version includes these controls. @@ -13,8 +13,9 @@ setup. While the app waits for browser approval or retries the connection, **Cancel sign-in** stops the attempt and lets you edit the server address -again. On Android, the system Back action does the same. Cancelling does not -revoke credentials the server has already issued. +again. On Android, the system Back action does the same. Cancelling also stops +an approval check that is still waiting for the server. It does not revoke +credentials the server has already issued. The form scrolls when a short window, landscape layout or larger text makes it taller than the screen. In landscape it keeps a bounded width. If the server @@ -24,20 +25,32 @@ the URL parser's error text. ## Connection problems A name-resolution message means a DNS lookup failed before any request reached -the server. It does not prove the server is down, or that the failure is still -happening. Check the address and the device's network or VPN connection. +the server. A "could not be reached" message means the app could not open a +connection to the server, so no sign-in request was sent. Neither proves the +server is down, or that the failure is still happening. Check the address and +the device's network or VPN connection. -The app retries this kind of failure after a bounded delay. When the server +The app retries both kinds of failure after a bounded delay. When the server answers again, the browser-approval status returns. You can also cancel, change the address or connection, and start a fresh attempt. +Each approval check opens a new connection and closes it after the response. +Leaving the app for the browser can suspend it or end its network connections. +Because no connection is kept between checks, returning to the app does not +reuse one that ended while the app was in the background. A new sign-in attempt +also never shares a connection with a check from an earlier attempt. + ## Time limit and uncertain results Each attempt has a five-minute deadline, including network waits. If the result became uncertain after the one-time approval exchange, start a new sign-in. Do not try to replay that exchange. The app does not retry this -case automatically. +case automatically. The result is uncertain when the connection failed after +the app had started sending a request, or when the server reported an approval +but its one-time response was malformed, incomplete, or contained invalid +account values. A connection failure before the request was sent is retried +instead. ## Save a login report @@ -85,8 +98,16 @@ Account publication failures retain the existing credential rollback policy. Synthetic regression tests in `LoginPollingTest` and `LoginAttemptStateTest` exercise retry phases, timeout, cancellation, browser-handoff failure, and late -completion after a replacement attempt. They do not establish connectivity to -any particular server or device network. +completion after a replacement attempt. `JvmLoginPollConnectionTest` runs +approval checks against a local mock server while simulating connections that +end during a suspension. `JvmLoginPollIsolationTest` holds a check open on an +HTTP/2 mock server to cover cancellation and a replacement check. These tests +run on the desktop JVM. `AndroidLoginPollTransportInstrumentedTest` repeats the +new-connection, cancellation and refused-connection checks against a local TLS +mock server on the Android runtime. It runs only through the instrumented +emulator workflow in [CONTRIBUTING.md](../CONTRIBUTING.md), not in pull request +CI. None of these tests establish connectivity to any particular server or +device network, and they do not reproduce Android process freezing. ## Evidence and release status diff --git a/tools/kotlin-file-size-baseline.txt b/tools/kotlin-file-size-baseline.txt index a875c8a79..7f421c62c 100644 --- a/tools/kotlin-file-size-baseline.txt +++ b/tools/kotlin-file-size-baseline.txt @@ -1,4 +1,4 @@ -androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt|4132 +androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt|4129 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidProjectContentClient.kt|985 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt|900 contractAcquisition/src/main/kotlin/dev/obiente/nextcloudnative/contracts/SignedAppStoreContractAcquirer.kt|1212 @@ -48,7 +48,7 @@ ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopAppUpdates.kt|9 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileReadCache.kt|1536 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncEngine.kt|884 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncRemoteTree.kt|857 -ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6169 +ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6168 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopVirtualRangeCache.kt|2759 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/LinuxVirtualFileSystem.kt|1697 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt|1069 diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt index 987436aeb..6c14d3d7b 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt @@ -19,6 +19,10 @@ internal enum class LoginAttemptPhase(val message: String, val buttonLabel: Stri "The server name could not be resolved. Check your network or VPN. Retrying...", "Retrying connection...", ), + ReconnectingToServer( + "The server could not be reached. Check your network or VPN. Retrying...", + "Retrying connection...", + ), Completing("Finishing sign-in...", "Finishing sign-in..."), } diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt index 7fb0cf0a5..b79f71988 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt @@ -4,6 +4,7 @@ import kotlin.math.min internal const val LOGIN_POLL_PENDING_DELAY_MILLIS = 2_000L internal const val LOGIN_POLL_MAX_RETRY_DELAY_MILLIS = 15_000L +private const val LOGIN_POLL_DNS_UNRESOLVED_CODE = "NETWORK_DNS_UNRESOLVED" internal suspend fun pollLoginUntilApproved( poll: suspend () -> LoginPollResult, @@ -23,7 +24,13 @@ internal suspend fun pollLoginUntilApproved( is LoginPollResult.RetryablePreExchangeFailure -> { transientFailures += 1 val delayMillis = loginPollRetryDelayMillis(transientFailures) - onStatus(LoginAttemptPhase.WaitingForNetwork) + onStatus( + if (result.code == LOGIN_POLL_DNS_UNRESOLVED_CODE) { + LoginAttemptPhase.WaitingForNetwork + } else { + LoginAttemptPhase.ReconnectingToServer + }, + ) waitBeforeNextPoll(delayMillis, true) } is LoginPollResult.FatalFailure -> error(result.message) diff --git a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/LoginPollingTest.kt b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/LoginPollingTest.kt index 355b483e5..9d8696652 100644 --- a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/LoginPollingTest.kt +++ b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/LoginPollingTest.kt @@ -51,6 +51,33 @@ class LoginPollingTest { assertEquals(LoginAttemptPhase.AwaitingApproval, statuses.last()) } + @Test + fun connectionFailuresBeforeTheExchangeRetryWithoutClaimingDnsFailure() = runBlocking { + val session = NextcloudSession("https://cloud.example.com", "person", "secret") + val results = ArrayDeque().apply { + add(LoginPollResult.Pending) + add(LoginPollResult.RetryablePreExchangeFailure("NETWORK_CONNECT_FAILED")) + add(LoginPollResult.Approved(session)) + } + val waits = mutableListOf>() + val statuses = mutableListOf() + + val approved = pollLoginUntilApproved( + poll = { results.removeFirst() }, + waitBeforeNextPoll = { delay, awaitNetwork -> waits += delay to awaitNetwork }, + hasTimedOut = { false }, + onStatus = statuses::add, + ) + + assertEquals(session, approved) + assertEquals(listOf(2_000L to false, 2_000L to true), waits) + assertEquals( + listOf(LoginAttemptPhase.AwaitingApproval, LoginAttemptPhase.ReconnectingToServer), + statuses, + ) + assertTrue("resolved" !in LoginAttemptPhase.ReconnectingToServer.message) + } + @Test fun anAmbiguousPostExchangeFailureIsNeverRetried() = runBlocking { var polls = 0 diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index 50184f8d4..ffab0d414 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -708,7 +708,6 @@ class DesktopNextcloudServices( environment = desktopSupportDiagnosticsEnvironment(), client = httpClient.newBuilder().retryOnConnectionFailure(false).build(), ) - private val loginPollHttpClient = httpClient.newBuilder().retryOnConnectionFailure(false).build() private val loginPollFallbackTokens = ConcurrentHashMap.newKeySet() private val loginPollPendingTokens = ConcurrentHashMap.newKeySet() private val fileMutationHttpExecutor = DesktopHttpMutationExecutor(httpClient) @@ -4004,7 +4003,7 @@ class DesktopNextcloudServices( override suspend fun pollLogin(challenge: LoginChallenge): LoginPollResult = withContext(Dispatchers.IO) { var networkFailure: JvmNetworkFailureDiagnostic? = null - val execution = executeLoginPollHttp( + val execution = withLoginPollHttpClient(httpClient) { loginPollHttpClient -> executeLoginPollHttp( challenge = challenge, fallbackAlreadySelected = challenge.token in loginPollFallbackTokens, poll = { endpoint -> @@ -4021,7 +4020,7 @@ class DesktopNextcloudServices( ).let { LoginPollHttpResponse(it.status, it.text) } }, networkFailure = { networkFailure }, - ) + ) } execution.selectedFallbackReason?.let { reason -> loginPollFallbackTokens += challenge.token runCatching { diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransportTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransportTest.kt index f942950d8..f4d4a8e10 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransportTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransportTest.kt @@ -2,10 +2,14 @@ package dev.obiente.nextcloudnative.app import java.io.IOException import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.async +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.job import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertFalse import kotlin.test.assertIs import kotlin.test.assertNull @@ -211,6 +215,35 @@ class JvmLoginFlowTransportTest { } } + @Test + fun `compatibility probe that cannot connect leaves the pending advertised route active`() = runBlocking { + var diagnostic: JvmNetworkFailureDiagnostic? = null + val execution = executeLoginPollHttp( + challenge = challenge( + pollEndpoint = "https://cloud.example.test/login/v2/poll", + fallbackEndpoint = "https://cloud.example.test/index.php/login/v2/poll", + ), + fallbackAlreadySelected = false, + poll = { endpoint -> + diagnostic = null + if ("/index.php/" in endpoint) { + diagnostic = dnsFailure().copy( + code = "NETWORK_CONNECT_FAILED", + phase = JvmNetworkFailurePhase.Connect, + ) + throw IOException("synthetic connect failure") + } + LoginPollHttpResponse(status = 404, body = "[]") + }, + networkFailure = { diagnostic }, + ) + + assertEquals(LoginPollResult.Pending, execution.interpretation.result) + assertNull(execution.selectedFallbackReason) + assertEquals(false, execution.usedFallback) + assertEquals(false, execution.responseUsedFallback) + } + @Test fun `failure after compatibility exchange is ambiguous`() = runBlocking { var diagnostic: JvmNetworkFailureDiagnostic? = null @@ -278,6 +311,32 @@ class JvmLoginFlowTransportTest { Unit } + @Test + fun `a call failing because its poll was cancelled is not classified as a network result`() = runBlocking { + var classified = false + val execution = async { + executeLoginPollHttp( + challenge = challenge( + pollEndpoint = "https://cloud.example.test/login/v2/poll", + fallbackEndpoint = "https://cloud.example.test/index.php/login/v2/poll", + ), + fallbackAlreadySelected = false, + poll = { + // OkHttp reports a call cancelled with its coroutine as an I/O failure. + currentCoroutineContext().job.cancel() + throw IOException("Canceled") + }, + networkFailure = { + classified = true + null + }, + ) + } + + assertFailsWith { execution.await() } + assertFalse(classified) + } + private fun challenge(pollEndpoint: String, fallbackEndpoint: String?) = LoginChallenge( enteredServerUrl = "https://cloud.example.test/nextcloud", pollEndpoint = pollEndpoint, diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollConnectionTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollConnectionTest.kt new file mode 100644 index 000000000..7388bbd6f --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollConnectionTest.kt @@ -0,0 +1,232 @@ +package dev.obiente.nextcloudnative.app + +import java.io.FilterInputStream +import java.io.FilterOutputStream +import java.io.IOException +import java.io.InputStream +import java.io.OutputStream +import java.net.InetAddress +import java.net.Socket +import java.net.SocketException +import java.net.SocketTimeoutException +import java.util.concurrent.CopyOnWriteArrayList +import javax.net.SocketFactory +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertNotSame +import kotlin.test.assertTrue +import mockwebserver3.MockResponse +import mockwebserver3.MockWebServer +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody + +/** + * Reproduces a connection that dies while the app is suspended between Login Flow v2 polls. + * + * A destroyed or silently dropped socket still passes OkHttp's pooled-connection health check, + * so the next request is written before the failure surfaces. That failure is correctly + * ambiguous, so the poll client must never reuse a connection across the wait between polls. + */ +class JvmLoginPollConnectionTest { + @Test + fun `a poll written to a reused dead connection stays ambiguous`() = runBlocking { + MockWebServer().use { server -> + server.start() + server.enqueue(challengeResponse()) + server.enqueue(pendingResponse()) + server.enqueue(server.approvedResponse()) + val sockets = SuspendableSocketFactory() + val base = baseClient(sockets) + val sharedPoolClient = base.newBuilder().retryOnConnectionFailure(false).build() + val challenge = challenge(server.url("/login/v2/poll").toString()) + + beginLogin(base, server.url("/index.php/login/v2").toString()) + assertEquals(LoginPollResult.Pending, poll(sharedPoolClient, challenge)) + assertEquals(1, sockets.created.size) + sockets.suspendAll() + + val failure = assertIs( + poll(sharedPoolClient, challenge), + ) + assertEquals("NETWORK_SOCKET_FAILED", failure.code) + assertTrue(failure.message.contains("could not be confirmed safely")) + } + } + + @Test + fun `login polls survive connections that died while the app was suspended`() = runBlocking { + MockWebServer().use { server -> + server.start() + server.enqueue(challengeResponse()) + server.enqueue(pendingResponse()) + server.enqueue(pendingResponse()) + server.enqueue(server.approvedResponse()) + val sockets = SuspendableSocketFactory() + val base = baseClient(sockets) + val challenge = challenge(server.url("/login/v2/poll").toString()) + suspend fun pollOnce() = withLoginPollHttpClient(base) { client -> poll(client, challenge) } + + beginLogin(base, server.url("/index.php/login/v2").toString()) + sockets.suspendAll() + assertEquals(LoginPollResult.Pending, pollOnce()) + sockets.suspendAll() + assertEquals(LoginPollResult.Pending, pollOnce()) + sockets.suspendAll() + val approved = assertIs(pollOnce()) + + assertEquals("person", approved.session.loginName) + assertEquals(4, sockets.created.size) + assertEquals(4, server.requestCount) + } + } + + @Test + fun `login poll client never replays a request and keeps no idle connection`() { + val base = OkHttpClient.Builder().trackJvmNetworkFailures().build() + val pollClient = base.newLoginPollHttpClient() + + assertFalse(pollClient.retryOnConnectionFailure) + assertNotSame(base.connectionPool, pollClient.connectionPool) + assertNotSame(pollClient.connectionPool, base.newLoginPollHttpClient().connectionPool) + assertEquals(0, pollClient.connectionPool.idleConnectionCount()) + assertEquals(base.eventListenerFactory, pollClient.eventListenerFactory) + assertEquals(base.socketFactory, pollClient.socketFactory) + } + + private fun baseClient(sockets: SuspendableSocketFactory): OkHttpClient = OkHttpClient.Builder() + .socketFactory(sockets) + // Racing IPv4 and IPv6 loopback addresses would create a losing socket per connection. + .fastFallback(false) + .trackJvmNetworkFailures() + .build() + + private fun beginLogin(client: OkHttpClient, url: String) { + val request = Request.Builder().url(url).post(ByteArray(0).toRequestBody(null)).build() + client.newCall(request).execute().use { response -> + assertEquals(200, response.code) + response.body.string() + } + } + + private suspend fun poll(client: OkHttpClient, challenge: LoginChallenge): LoginPollResult { + var networkFailure: JvmNetworkFailureDiagnostic? = null + return executeLoginPollHttp( + challenge = challenge, + fallbackAlreadySelected = false, + poll = { endpoint -> + networkFailure = null + val attempt = JvmNetworkRequestAttempt() + val request = Request.Builder() + .url(endpoint) + .post("token=synthetic-one-time-token".toRequestBody(FORM)) + .tag(JvmNetworkRequestAttempt::class.java, attempt) + .build() + try { + client.newCall(request).execute().use { response -> + LoginPollHttpResponse(response.code, response.body.string()) + } + } catch (failure: IOException) { + networkFailure = failure.toJvmNetworkFailureDiagnostic( + attempt = attempt, + readOnlyRequest = false, + replayableRequest = false, + ) + throw failure + } + }, + networkFailure = { networkFailure }, + ).interpretation.result + } + + private fun challenge(pollEndpoint: String) = LoginChallenge( + enteredServerUrl = pollEndpoint.substringBefore("/login/v2/poll"), + pollEndpoint = pollEndpoint, + pollFallbackEndpoint = null, + token = "synthetic-one-time-token", + loginUrl = pollEndpoint.replace("/poll", "/flow/synthetic"), + transportSecurity = LoginTransportSecurity.PlainHttp, + ) + + private fun challengeResponse() = MockResponse.Builder().code(200).body("{}").build() + + private fun pendingResponse() = MockResponse.Builder().code(404).body("[]").build() + + private fun MockWebServer.approvedResponse() = MockResponse.Builder() + .code(200) + .body( + """{"server":"${url("/").toString().trimEnd('/')}","loginName":"person",""" + + """"appPassword":"synthetic-app-password"}""", + ) + .build() + + private companion object { + val FORM = "application/x-www-form-urlencoded".toMediaType() + } +} + +/** + * Creates sockets that can be marked dead without the client observing a close, like a socket + * destroyed while its process was frozen or a connection whose network path was dropped. + */ +private class SuspendableSocketFactory : SocketFactory() { + val created = CopyOnWriteArrayList() + + fun suspendAll() = created.forEach(SuspendableSocket::markDead) + + override fun createSocket(): Socket = SuspendableSocket().also(created::add) + + override fun createSocket(host: String, port: Int): Socket = unsupported() + + override fun createSocket(host: String, port: Int, localHost: InetAddress, localPort: Int): Socket = + unsupported() + + override fun createSocket(host: InetAddress, port: Int): Socket = unsupported() + + override fun createSocket(address: InetAddress, port: Int, localAddress: InetAddress, localPort: Int): Socket = + unsupported() + + private fun unsupported(): Nothing = throw UnsupportedOperationException("OkHttp connects unconnected sockets") +} + +private class SuspendableSocket : Socket() { + @Volatile + private var dead = false + + fun markDead() { + dead = true + } + + override fun getInputStream(): InputStream = object : FilterInputStream(super.getInputStream()) { + override fun read(): Int { + if (dead) throw SocketTimeoutException("no data arrived") + return super.read() + } + + override fun read(buffer: ByteArray, offset: Int, length: Int): Int { + if (dead) throw SocketTimeoutException("no data arrived") + return super.read(buffer, offset, length) + } + } + + override fun getOutputStream(): OutputStream = object : FilterOutputStream(super.getOutputStream()) { + override fun write(value: Int) { + if (dead) throw SocketException("Software caused connection abort") + out.write(value) + } + + override fun write(buffer: ByteArray, offset: Int, length: Int) { + if (dead) throw SocketException("Software caused connection abort") + out.write(buffer, offset, length) + } + + override fun flush() { + if (dead) throw SocketException("Software caused connection abort") + out.flush() + } + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollIsolationTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollIsolationTest.kt new file mode 100644 index 000000000..9068c3a7e --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmLoginPollIsolationTest.kt @@ -0,0 +1,240 @@ +package dev.obiente.nextcloudnative.app + +import java.io.IOException +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.job +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertNotEquals +import kotlin.test.assertTrue +import mockwebserver3.MockResponse +import mockwebserver3.MockWebServer +import mockwebserver3.RecordedRequest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody + +/** + * Covers a poll that is still in flight when its sign-in attempt is cancelled or replaced. + * + * The server holds the first poll open, so the second poll starts while the first one's HTTP/2 + * connection is still active and therefore not subject to idle-connection eviction. + */ +class JvmLoginPollIsolationTest { + @Test + fun `a shared poll client multiplexes a replacement poll onto the active connection`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val shared = base.newLoginPollHttpClient() + val first = async(Dispatchers.IO) { poll(shared, server.challenge()) } + assertTrue(gate.firstArrived.await(10, TimeUnit.SECONDS)) + + val second = withContext(Dispatchers.IO) { poll(shared, server.challenge()) } + gate.release.countDown() + + assertEquals(LoginPollResult.Pending, second) + assertEquals(LoginPollResult.Pending, first.await()) + assertEquals(1, gate.connectionIndexes.distinct().size) + } + } + + @Test + fun `a replacement poll never shares the connection of a poll still in flight`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val first = async(Dispatchers.IO) { + withLoginPollHttpClient(base) { client -> poll(client, server.challenge()) } + } + assertTrue(gate.firstArrived.await(10, TimeUnit.SECONDS)) + + val second = withContext(Dispatchers.IO) { + withLoginPollHttpClient(base) { client -> poll(client, server.challenge()) } + } + gate.release.countDown() + + assertEquals(LoginPollResult.Pending, second) + assertEquals(LoginPollResult.Pending, first.await()) + assertEquals(2, gate.connectionIndexes.size) + assertNotEquals(gate.connectionIndexes[0], gate.connectionIndexes[1]) + } + } + + @Test + fun `cancelling the poll coroutine cancels its blocked call`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val attempt = AtomicReference(null) + var completed = false + val job = launch(Dispatchers.IO) { + withLoginPollHttpClient(base) { client -> + poll(client, server.challenge(), onAttempt = attempt::set) + } + completed = true + } + assertTrue(gate.firstArrived.await(10, TimeUnit.SECONDS)) + + job.cancel() + // The cancelling thread cancels the call itself, without waiting for a dispatcher thread. + assertTrue(requireNotNull(attempt.get()).cancelled) + // The server holds the response far longer than this, so only a cancelled call ends early. + withTimeout(5_000) { job.join() } + + assertTrue(job.isCancelled) + assertFalse(completed) + assertTrue(requireNotNull(attempt.get()).cancelled) + } + } + + @Test + fun `a call that starts after cancellation is never sent`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val calls = LoginPollCallCancellation() + val client = base.newBuilder().addInterceptor(calls).build() + calls.cancel() + + val attempt = JvmNetworkRequestAttempt() + assertFailsWith { + client.newCall(pollRequest(server.challenge().pollEndpoint, attempt)).execute() + } + + assertTrue(attempt.cancelled) + assertFalse(attempt.exchangeStarted) + assertTrue(gate.connectionIndexes.isEmpty()) + assertEquals(0, server.requestCount) + } + } + + @Test + fun `a call registered before cancellation is cancelled while blocked`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val calls = LoginPollCallCancellation() + val client = base.newBuilder().addInterceptor(calls).build() + val attempt = JvmNetworkRequestAttempt() + val blocked = async(Dispatchers.IO) { + runCatching { + client.newCall(pollRequest(server.challenge().pollEndpoint, attempt)).execute().close() + } + } + assertTrue(gate.firstArrived.await(10, TimeUnit.SECONDS)) + + calls.cancel() + val outcome = withTimeout(5_000) { blocked.await() } + + assertIs(outcome.exceptionOrNull()) + assertTrue(attempt.cancelled) + } + } + + @Test + fun `cancellation just before a poll is sent stops it in either order`() = runBlocking { + withHeldFirstPoll { server, gate, base -> + val attempt = AtomicReference(null) + var completed = false + val job = launch(Dispatchers.IO) { + val attemptJob = coroutineContext.job + withLoginPollHttpClient(base) { client -> + // The cancellation sweep races the call's registration from here on. + attemptJob.cancel() + poll(client, server.challenge(), onAttempt = attempt::set) + } + completed = true + } + + // The server would hold a sent poll far longer than this. + withTimeout(5_000) { job.join() } + + assertTrue(job.isCancelled) + assertFalse(completed) + val sent = gate.connectionIndexes.isNotEmpty() + assertTrue(!sent || requireNotNull(attempt.get()).cancelled) + } + } + + private fun pollRequest(endpoint: String, attempt: JvmNetworkRequestAttempt): Request = Request.Builder() + .url(endpoint) + .post("token=synthetic-one-time-token".toRequestBody(FORM)) + .tag(JvmNetworkRequestAttempt::class.java, attempt) + .build() + + private suspend fun withHeldFirstPoll( + test: suspend (MockWebServer, HeldFirstPoll, OkHttpClient) -> Unit, + ) { + val gate = HeldFirstPoll() + MockWebServer().use { server -> + server.protocols = listOf(Protocol.H2_PRIOR_KNOWLEDGE) + server.dispatcher = gate + server.start() + val base = OkHttpClient.Builder() + .protocols(listOf(Protocol.H2_PRIOR_KNOWLEDGE)) + .trackJvmNetworkFailures() + .build() + try { + test(server, gate, base) + } finally { + gate.release.countDown() + } + } + } + + private suspend fun poll( + client: OkHttpClient, + challenge: LoginChallenge, + onAttempt: (JvmNetworkRequestAttempt) -> Unit = {}, + ): LoginPollResult = executeLoginPollHttp( + challenge = challenge, + fallbackAlreadySelected = false, + poll = { endpoint -> + val attempt = JvmNetworkRequestAttempt().also(onAttempt) + client.newCall(pollRequest(endpoint, attempt)).execute().use { response -> + LoginPollHttpResponse(response.code, response.body.string()) + } + }, + networkFailure = { null }, + ).interpretation.result + + private fun MockWebServer.challenge(): LoginChallenge { + val pollEndpoint = url("/login/v2/poll").toString() + return LoginChallenge( + enteredServerUrl = pollEndpoint.substringBefore("/login/v2/poll"), + pollEndpoint = pollEndpoint, + pollFallbackEndpoint = null, + token = "synthetic-one-time-token", + loginUrl = pollEndpoint.replace("/poll", "/flow/synthetic"), + transportSecurity = LoginTransportSecurity.PlainHttp, + ) + } + + private companion object { + val FORM = "application/x-www-form-urlencoded".toMediaType() + } +} + +/** Answers every poll as pending, but holds the first one until the test releases it. */ +private class HeldFirstPoll : mockwebserver3.Dispatcher() { + val firstArrived = CountDownLatch(1) + val release = CountDownLatch(1) + val connectionIndexes = CopyOnWriteArrayList() + + override fun dispatch(request: RecordedRequest): MockResponse { + val isFirst = synchronized(connectionIndexes) { + connectionIndexes += request.connectionIndex + connectionIndexes.size == 1 + } + if (isFirst) { + firstArrived.countDown() + release.await(30, TimeUnit.SECONDS) + } + return MockResponse.Builder().code(404).body("[]").build() + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/LoginEndpointSecurityTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/LoginEndpointSecurityTest.kt index fcc5ffd64..2d1dfd8d6 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/LoginEndpointSecurityTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/LoginEndpointSecurityTest.kt @@ -137,7 +137,7 @@ class LoginEndpointSecurityTest { } @Test - fun onlyPreExchangeDnsFailureIsSafeToRetry() { + fun onlyTransientConnectionSetupFailuresAreSafeToRetry() { val dns = JvmNetworkFailureDiagnostic( code = "NETWORK_DNS_UNRESOLVED", phase = JvmNetworkFailurePhase.Dns, @@ -146,10 +146,40 @@ class LoginEndpointSecurityTest { exchangeStarted = false, protocol = null, ) - val afterExchange = dns.copy(exchangeStarted = true) - - assertIs(classifyLoginPollNetworkFailure(dns)) - assertIs(classifyLoginPollNetworkFailure(afterExchange)) + val connectionSetupFailures = listOf( + dns, + dns.copy(code = "NETWORK_CONNECT_FAILED", phase = JvmNetworkFailurePhase.Connect), + dns.copy(code = "NETWORK_CONNECT_TIMEOUT", phase = JvmNetworkFailurePhase.Connect), + dns.copy(code = "NETWORK_UNREACHABLE", phase = JvmNetworkFailurePhase.Connect), + dns.copy(code = "NETWORK_TLS_TIMEOUT", phase = JvmNetworkFailurePhase.Tls), + dns.copy(code = "NETWORK_SOCKET_FAILED", phase = JvmNetworkFailurePhase.Connect), + dns.copy(code = "NETWORK_CONNECTION_RESET", phase = JvmNetworkFailurePhase.Connect), + ) + connectionSetupFailures.forEach { failure -> + val result = assertIs( + classifyLoginPollNetworkFailure(failure), + ) + assertEquals(failure.code, result.code) + // A request that may have reached the server is never retried, whatever its cause. + val afterExchange = failure.copy( + exchangeStarted = true, + phase = JvmNetworkFailurePhase.ResponseHeaders, + ) + val ambiguous = assertIs( + classifyLoginPollNetworkFailure(afterExchange), + ) + assertEquals(failure.code, ambiguous.code) + } + listOf( + dns.copy(code = "NETWORK_CERTIFICATE_REJECTED", phase = JvmNetworkFailurePhase.Tls), + dns.copy(code = "NETWORK_TLS_HANDSHAKE", phase = JvmNetworkFailurePhase.Tls), + dns.copy(code = "NETWORK_PROTOCOL_FAILED", phase = JvmNetworkFailurePhase.Connect), + dns.copy(code = "NETWORK_UNKNOWN_FAILED", phase = JvmNetworkFailurePhase.Unknown), + dns.copy(code = "NETWORK_SOCKET_FAILED", phase = JvmNetworkFailurePhase.Tls), + ).forEach { failure -> + assertIs(classifyLoginPollNetworkFailure(failure)) + } + assertIs(classifyLoginPollNetworkFailure(null)) assertEquals( false, loginResultOriginMatchesEntered("https://cloud.example.com", "https://other.example.com"), diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransport.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransport.kt index a216d8ddc..87f556b36 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransport.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmLoginFlowTransport.kt @@ -1,6 +1,20 @@ package dev.obiente.nextcloudnative.app +import java.io.IOException +import java.util.concurrent.TimeUnit import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.suspendCancellableCoroutine +import okhttp3.Call +import okhttp3.ConnectionPool +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Response data class LoginPollHttpResponse( val status: Int, @@ -21,6 +35,93 @@ data class LoginPollHttpExecution( val selectedFallbackReason: LoginPollFallbackReason? = null, ) +/** + * Derives the client used for one-time Login Flow v2 polls from the platform's base client. + * + * The HTTP stack must never replay a poll after its request may have reached the server, so + * connection-failure recovery stays disabled. Without that recovery, a pooled connection that + * died while the app was suspended or in the background, or that a proxy closed after its + * keep-alive window, fails only after the request has started and is indistinguishable from a + * lost one-time approval response. Each poll therefore opens a new connection and closes it + * after the response, so no connection outlives the wait between polls. Failures while that + * connection is being established remain provably pre-exchange. + * + * The pool belongs to this client alone. Another client's in-flight poll, such as one from a + * cancelled or replaced sign-in attempt, can therefore never share its HTTP/2 connection. + */ +fun OkHttpClient.newLoginPollHttpClient(): OkHttpClient = loginPollHttpClientBuilder().build() + +private fun OkHttpClient.loginPollHttpClientBuilder(): OkHttpClient.Builder = newBuilder() + .retryOnConnectionFailure(false) + .connectionPool(ConnectionPool(0, 1, TimeUnit.SECONDS)) + +/** + * Runs one poll with a new [newLoginPollHttpClient] derived from [baseClient]. + * + * OkHttp's blocking calls do not observe coroutine cancellation. Every call made through the + * client is therefore registered with a [LoginPollCallCancellation] that the calling coroutine's + * cancellation triggers, so a cancelled or replaced sign-in attempt cannot send a poll or leave + * one running against the server. + * + * The calls are cancelled synchronously by the thread that cancels the coroutine. Cancellation + * never waits for a dispatcher thread, so it still works while blocking work saturates + * [kotlinx.coroutines.Dispatchers.IO]. + */ +suspend fun withLoginPollHttpClient( + baseClient: OkHttpClient, + block: suspend (OkHttpClient) -> T, +): T = coroutineScope { + val calls = LoginPollCallCancellation() + val client = baseClient.loginPollHttpClientBuilder() + .apply { interceptors().add(0, calls) } + .build() + val cancellation = launch(Dispatchers.Unconfined, start = CoroutineStart.UNDISPATCHED) { + suspendCancellableCoroutine { continuation -> + continuation.invokeOnCancellation { calls.cancel() } + } + } + try { + block(client) + } finally { + cancellation.cancel() + } +} + +/** + * Cancels every call of one poll, including calls that start after cancellation was requested. + * + * As the first application interceptor it sees each concrete [Call] before any network work. + * Registration and cancellation share one lock: a call registered before [cancel] is cancelled by + * it, and a call that arrives afterwards is cancelled before its request can be sent. Calls stay + * registered for the poll's lifetime so cancellation also stops a response body still being read. + */ +internal class LoginPollCallCancellation : Interceptor { + private val lock = Any() + private var cancelled = false + private val calls = mutableListOf() + + fun cancel() { + val registered = synchronized(lock) { + cancelled = true + calls.toList() + } + registered.forEach(Call::cancel) + } + + override fun intercept(chain: Interceptor.Chain): Response { + val call = chain.call() + val alreadyCancelled = synchronized(lock) { + if (!cancelled) calls += call + cancelled + } + if (alreadyCancelled) { + call.cancel() + throw IOException("Canceled") + } + return chain.proceed(chain.request()) + } +} + suspend fun executeLoginPollHttp( challenge: LoginChallenge, fallbackAlreadySelected: Boolean, @@ -43,6 +144,8 @@ suspend fun executeLoginPollHttp( poll(endpoint) } catch (failure: Throwable) { if (failure is CancellationException) throw failure + // A call cancelled with its coroutine fails as an I/O error; it is not a network outcome. + currentCoroutineContext().ensureActive() throw LoginPollRequestFailure(classifyLoginPollNetworkFailure(networkFailure()), failure) } diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/LoginPollNetworkFailure.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/LoginPollNetworkFailure.kt index df10b70ff..5758c9a81 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/LoginPollNetworkFailure.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/LoginPollNetworkFailure.kt @@ -1,9 +1,17 @@ package dev.obiente.nextcloudnative.app +/** + * Classifies a failed poll request by whether the one-time exchange could have reached the server. + * + * Only failures that occurred while the connection was being established, before OkHttp started + * writing the request, are retried. Certificate, TLS handshake, protocol, and unknown failures + * remain fatal because waiting does not resolve them, and any failure after the exchange started + * stays ambiguous because the server may already have issued its one-time response. + */ fun classifyLoginPollNetworkFailure( diagnostic: JvmNetworkFailureDiagnostic?, ): LoginPollResult = when { - diagnostic?.code == "NETWORK_DNS_UNRESOLVED" && !diagnostic.exchangeStarted -> + diagnostic != null && diagnostic.isTransientBeforeExchange() -> LoginPollResult.RetryablePreExchangeFailure(diagnostic.code) diagnostic?.exchangeStarted == true -> LoginPollResult.AmbiguousAfterExchangeFailure( message = "The login response could not be confirmed safely after contacting the server. " + @@ -21,3 +29,19 @@ fun ambiguousLoginPollResponse(message: String): LoginPollResult = message = "$message Restart sign-in so the app does not reuse a one-time approval response.", code = "LOGIN_POLL_RESPONSE_INVALID", ) + +private fun JvmNetworkFailureDiagnostic.isTransientBeforeExchange(): Boolean = + !exchangeStarted && ( + code in TRANSIENT_CONNECTION_SETUP_CODES || + (code in TRANSIENT_SOCKET_CODES && phase in CONNECTION_SETUP_PHASES) + ) + +private val TRANSIENT_CONNECTION_SETUP_CODES = setOf( + "NETWORK_DNS_UNRESOLVED", + "NETWORK_CONNECT_FAILED", + "NETWORK_CONNECT_TIMEOUT", + "NETWORK_UNREACHABLE", + "NETWORK_TLS_TIMEOUT", +) +private val TRANSIENT_SOCKET_CODES = setOf("NETWORK_SOCKET_FAILED", "NETWORK_CONNECTION_RESET") +private val CONNECTION_SETUP_PHASES = setOf(JvmNetworkFailurePhase.Dns, JvmNetworkFailurePhase.Connect) diff --git a/website/public/screenshots/capture-manifest.json b/website/public/screenshots/capture-manifest.json index 5976f9d17..1b4488b67 100644 --- a/website/public/screenshots/capture-manifest.json +++ b/website/public/screenshots/capture-manifest.json @@ -678,10 +678,10 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LivePhotoDetection.kt": "d4afebb50e544a7e1ff8de73be5a48e60b657f85f127172f4b6d59213f0100f5", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LivePhotos.kt": "380a1f135dd50b1746967a6805b69dd4e48ab77dc864aeef394440c021c223d7", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LocalFileUpload.kt": "71b363286921b21fb43bae04832ddb15627f12fbcc62c58262ce474481833658", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt": "5f8088ae7861aceccdadbdb0700483bf7d13710ff49ccb297d029b36ce107f45", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginAttemptState.kt": "1ba0d22d310b92f8b1b106851437f547f3f86a1a5530d60c2d8ae6186bbe61b4", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginDiagnostics.kt": "4eac0172e469d673d594c9fd83a85bf275e7c945ec1bd85cf8ea18679f9444bc", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginDiagnosticsView.kt": "42031d385d274da6a96871977511683d1842d8e92823f91de3957c3511dcf750", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt": "8e63644cc3a6eba20356477ed19f5b4277001182e31c7a977344d7e34ed8a274", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginPolling.kt": "49cf95635e24f3660ecc8a98dbaf83dc4855fa6140efd2e2b9d24cbb5115308a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/LoginScreen.kt": "3200fefe482fbc765e5e0cd4f22f837379faba038cb0c110ee1675a3f003230a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/MarkdownEditing.kt": "74ed912eef556a53b7eb0008946aa5b4425cb69456bea75368d309a39052a331", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/MediaBackupLedger.kt": "dd48146115c292d81cb58bba56c138960499b4b5702bfc7b0401234bfdde3d30",