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()