From 41ffc75fc501a1fb196880fc91c8c1c780e405e5 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 7 Sep 2026 23:28:08 +0200 Subject: [PATCH] sync: only report the address we actually replaced Four corrections to the login-flow work earlier on this branch, all found in review. The mismatch was computed by comparing hosts exactly while reachableOrigin substitutes only across registrable domains. A server answering nc.example.com for a poll endpoint on cloud.example.com is used verbatim and correctly -- and we told the user we had replaced it because it was unreachable, blaming a setting that was right. The decision now has a name and says what it means: report a mismatch only when the address we used is not the one that was claimed. baseOf matched only the index.php spelling of the poll path, but Nextcloud drops index.php from generated routes when htaccess.IgnoreFrontController is on -- so a subdirectory install answering /nc/login/v2/poll rebuilt the root as / and lost the prefix, which is the loss that function exists to prevent. browserFailed built a fresh step and dropped the mismatch note, which is often the explanation for the failure it is replacing it with. And the note is sticky by design, but it belongs to the address that produced it: starting a new attempt now clears it, rather than carrying a claim about one server's configuration into another's. --- .../ui/accounts/AddAccountViewModel.kt | 11 +++++- .../caldav/NextcloudLoginFlow.kt | 37 ++++++++++++++++--- .../caldav/NextcloudLoginFlowTest.kt | 33 +++++++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt index ce72c26..8ad1922 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt @@ -138,6 +138,10 @@ class AddAccountViewModel @Inject constructor( // *Continue*, not start over — so a second attempt would mint a second // password on top of the first without this. discardMintedPassword() + // The mismatch is sticky on purpose, but it belongs to the address that + // produced it — carrying it into a different server's flow makes a claim + // about that server's settings which was never measured. + _state.update { it.copy(originMismatch = null) } typedInput = input val quirk = ServerQuirk.forInput(input) @@ -521,7 +525,12 @@ class AddAccountViewModel @Inject constructor( } private fun browserFailed(reason: AddAccountMessage) = _state.update { - it.copy(step = AddAccountStep.WaitingForBrowser(error = reason), openInBrowser = null) + // Keeps whatever the step was carrying. The host-mismatch note is often + // the *explanation* for the failure, so dropping it removes the warning + // exactly when it becomes worth reading. + val step = it.step as? AddAccountStep.WaitingForBrowser + ?: AddAccountStep.WaitingForBrowser() + it.copy(step = step.copy(error = reason), openInBrowser = null) } private fun updateCredentials(transform: (AddAccountStep.EnterCredentials) -> AddAccountStep.EnterCredentials) { diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt index 640fb62..f518159 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt @@ -208,7 +208,7 @@ class NextcloudLoginFlow( // app password and leaves it dangling in the user's device list. val secureServer = reachableOrigin(flow.pollEndpoint, server) PollResult.Approved( - hostMismatch = hostMismatchOf(flow.pollEndpoint, server), + hostMismatch = substitutedMismatch(flow.pollEndpoint, server, secureServer), credentials = Credentials( server = secureServer, // ⚠️ loginName is what the user typed — possibly an email, an @@ -307,17 +307,42 @@ class NextcloudLoginFlow( private companion object { const val FLOW_START_PATH = "index.php/login/v2" - /** What `start()` appends, plus the poll leg the server adds to it. */ - const val FLOW_PATH = "index.php/login/v2/poll" + /** What `start()` appends, plus the poll leg — with and without the front controller. */ + val FLOW_TAILS = listOf("index.php/login/v2/poll", "login/v2/poll") } - /** The poll endpoint with the flow's own path removed — the server's web root. */ + /** + * The poll endpoint with the flow's own path removed — the server's web root. + * + * ⚠️ Both spellings. Nextcloud drops `index.php` from generated routes when + * `htaccess.IgnoreFrontController` is on, so a subdirectory install can answer + * `/nc/login/v2/poll` — and matching only the `index.php` form would return + * "/" and lose the `/nc` prefix, which is the exact loss this function exists + * to prevent. + */ private fun baseOf(pollEndpoint: HttpUrl): String { val path = pollEndpoint.encodedPath - val tail = path.indexOf(FLOW_PATH) - return if (tail >= 0) path.take(tail).ifEmpty { "/" } else "/" + val tail = FLOW_TAILS.map { path.indexOf(it) }.firstOrNull { it >= 0 } ?: return "/" + return path.take(tail).ifEmpty { "/" } } + /** + * The mismatch to report, which is only ever one we acted on. + * + * ⚠️ [hostMismatchOf] compares hosts exactly, while [reachableOrigin] + * substitutes only when the *registrable domain* differs. So a server + * answering `nc.example.com` for a poll endpoint on `cloud.example.com` is + * used verbatim and correctly — and reporting that would tell the user we + * replaced an address we did not, blaming a setting that is right. The note + * is sticky, so it would follow them through the rest of the flow. + */ + internal fun substitutedMismatch( + expected: HttpUrl, + claimed: HttpUrl, + used: HttpUrl, + ): HostMismatch? = + hostMismatchOf(expected, claimed)?.takeIf { used.host != claimed.host } + /** A different host than the user typed — reported, not refused. */ internal fun hostMismatchOf(expected: HttpUrl, actual: HttpUrl): HostMismatch? = if (expected.host != actual.host) HostMismatch(expected.host, actual.host) else null diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt index 5ca7b2d..ccc9d8e 100644 --- a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt @@ -223,6 +223,39 @@ class NextcloudLoginFlowTest { assertThat(reachable.encodedPath).isEqualTo("/") } + @Test fun `a sibling host is used verbatim and not reported as a mismatch`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + val polled = "https://cloud.example.com/index.php/login/v2/poll".toHttpUrl() + val claimed = "https://nc.example.com/".toHttpUrl() + + // ⚠️ hostMismatchOf compares hosts exactly; reachableOrigin substitutes + // only across registrable domains. Reporting the raw comparison tells the + // user we replaced an address we in fact used, and blames a correct + // setting — stickily, for the rest of the flow. + val used = flow.reachableOrigin(polled, claimed) + assertThat(used).isEqualTo(claimed) + assertThat(flow.substitutedMismatch(polled, claimed, used)).isNull() + } + + @Test fun `a substituted host is reported`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + val polled = "https://cloud.example.com/index.php/login/v2/poll".toHttpUrl() + val claimed = "http://nextcloud:11000/".toHttpUrl() + + val used = flow.reachableOrigin(polled, claimed) + assertThat(flow.substitutedMismatch(polled, claimed, used)?.actual).isEqualTo("nextcloud") + } + + @Test fun `a subdirectory install without index_php keeps its prefix`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + // htaccess.IgnoreFrontController drops index.php from generated routes. + val expected = "https://cloud.example.com/nc/login/v2/poll".toHttpUrl() + + val reachable = flow.reachableOrigin(expected, "http://nextcloud:11000/".toHttpUrl()) + + assertThat(reachable.encodedPath).isEqualTo("/nc/") + } + @Test fun `a sibling host in the same domain is still honoured`() { val flow = NextcloudLoginFlow(OkHttpClient(), "test") val expected = "https://cloud.example.com/index.php/login/v2/poll".toHttpUrl()