diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt index 506ed0e..5e1c2ca 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt @@ -58,7 +58,15 @@ class NextcloudLoginFlow( data class Credentials(val server: HttpUrl, val loginName: String, val appPassword: String) sealed interface PollResult { - data class Approved(val credentials: Credentials) : PollResult + data class Approved( + val credentials: Credentials, + /** + * The server named an origin outside the one we polled, and we used + * the one we polled instead. Reported so the user can be told which + * setting is wrong, never acted on. + */ + val hostMismatch: HostMismatch? = null, + ) : PollResult /** 404: still waiting. Also what an expired or already-consumed flow returns. */ data object Pending : PollResult data class Expired(val reason: String) : PollResult @@ -166,9 +174,10 @@ class NextcloudLoginFlow( // the credentials. The 200 is returned exactly once (the server deletes // the row inside poll() before answering), so a throw here burns a live // app password and leaves it dangling in the user's device list. - val secureServer = secureOrigin(flow.pollEndpoint, server) + val secureServer = reachableOrigin(flow.pollEndpoint, server) PollResult.Approved( - Credentials( + hostMismatch = hostMismatchOf(flow.pollEndpoint, server), + credentials = Credentials( server = secureServer, // ⚠️ loginName is what the user typed — possibly an email, an // LDAP-derived value, or the right name in the wrong case. It is @@ -220,6 +229,45 @@ class NextcloudLoginFlow( actual } + /** + * The origin to actually talk to, given what the poll response claimed. + * + * ⚠️ `server` and the poll endpoint come out of *different* generators in + * Nextcloud: the poll endpoint honours `overwrite.cli.url`, while `server` + * is built from the approving request's own protocol and Host header, which + * respect `X-Forwarded-*` only once `trusted_proxies` is set. The ordinary + * docker-compose-behind-nginx install therefore answers a perfectly good + * `https://cloud.example.com` poll endpoint with + * `"server": "http://nextcloud:11000"` — a name that does not resolve on the + * phone. Following it means discovery fails, the account is never created, + * and the app password is already spent. + * + * So when the claimed origin is outside the registrable domain we just + * successfully polled, keep scheme, host and port from the poll endpoint — + * the origin empirically known to answer — and keep the claimed path. + * Coerced, never refused: by this point the credential exists and the 200 is + * spent, so throwing burns a live app password. + * + * This compares one server-emitted origin against another, never against + * what the user typed, so a correctly configured proxy — which emits the + * same origin in both — is untouched. A sibling host under the same + * registrable domain passes through, because that is a real deployment shape + * and the credential is scoped to that domain anyway. + */ + internal fun reachableOrigin(expected: HttpUrl, actual: HttpUrl): HttpUrl { + val secure = secureOrigin(expected, actual) + // The same boundary the credential is scoped by; null for an IP literal + // or a single-label host, where only the exact host will do. + val claimed = secure.topPrivateDomain() ?: secure.host + val polled = expected.topPrivateDomain() ?: expected.host + if (claimed.equals(polled, ignoreCase = true)) return secure + return expected.newBuilder() + .encodedPath(secure.encodedPath) + .query(null) + .fragment(null) + .build() + } + /** 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 fcaf333..5ce8ab4 100644 --- a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt @@ -81,6 +81,22 @@ class NextcloudLoginFlowTest { assertThat(credentials.appPassword).isEqualTo("secret-app-pw") } + @Test + fun `an unreachable claimed origin is replaced, and reported`() { + val started = startFlow() + server.enqueue( + json("""{"server":"http://nextcloud:11000/","loginName":"me","appPassword":"pw"}"""), + ) + + val result = flow.poll(started, now = 1) as NextcloudLoginFlow.PollResult.Approved + + // We talk to the host that just answered us, not the one it named. + assertThat(result.credentials.server.host).isEqualTo(server.hostName) + // And the user is told which setting is wrong, rather than being sent + // away with "no task lists" for what is a DNS failure. + assertThat(result.hostMismatch?.actual).isEqualTo("nextcloud") + } + @Test fun `429 is rate limiting, not pending`() { // "Anything that isn't 200 is pending" turns Nextcloud's brute-force @@ -187,10 +203,59 @@ class NextcloudLoginFlowTest { val expected = "https://cloud.example.com/login/v2/poll".toHttpUrl() val actual = "https://dav.example.com".toHttpUrl() - // A different host is reported by hostMismatchOf, never rewritten here. + // secureOrigin keeps its narrow job: the scheme. Which origin we then + // actually talk to is reachableOrigin's decision. assertThat(flow.secureOrigin(expected, actual)).isEqualTo(actual) } + @Test fun `an origin outside the polled domain is replaced by the one that answered`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + val expected = "https://cloud.example.com/index.php/login/v2/poll".toHttpUrl() + + // ⚠️ The ordinary reverse-proxied install: the poll endpoint comes from + // overwrite.cli.url and is right, while `server` is built from the + // approving request's own Host header and is an internal name the phone + // cannot resolve. Following it strands the user with a spent password. + val reachable = flow.reachableOrigin(expected, "http://nextcloud:11000/".toHttpUrl()) + + assertThat(reachable.host).isEqualTo("cloud.example.com") + assertThat(reachable.isHttps).isTrue() + assertThat(reachable.encodedPath).isEqualTo("/") + } + + @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() + val actual = "https://dav.example.com/".toHttpUrl() + + // A real deployment shape, and the credential is scoped to that + // registrable domain anyway. + assertThat(flow.reachableOrigin(expected, actual)).isEqualTo(actual) + } + + @Test fun `the claimed path survives the host being replaced`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + val expected = "https://cloud.example.com/index.php/login/v2/poll".toHttpUrl() + + val reachable = flow.reachableOrigin(expected, "https://internal.local/nextcloud/".toHttpUrl()) + + // A subdirectory install still lives under its subdirectory. + assertThat(reachable.host).isEqualTo("cloud.example.com") + assertThat(reachable.encodedPath).isEqualTo("/nextcloud/") + } + + @Test fun `a single-label host is compared exactly, not by a null domain`() { + val flow = NextcloudLoginFlow(OkHttpClient(), "test") + val expected = "https://localhost:8443/index.php/login/v2/poll".toHttpUrl() + + // topPrivateDomain() is null for both sides here; falling back to the + // exact host is what keeps a homelab install working. + assertThat(flow.reachableOrigin(expected, "https://localhost:8443/".toHttpUrl()).host) + .isEqualTo("localhost") + assertThat(flow.reachableOrigin(expected, "https://elsewhere/".toHttpUrl()).host) + .isEqualTo("localhost") + } + @Test fun `the login URL is still refused outright when downgraded`() { // Before approval there is nothing to lose by refusing, and the login URL // is where the *account* password gets typed.