sync: talk to the host that answered the poll, not the one it named
The poll response's `server` was taken verbatim apart from its scheme. That field and the poll endpoint come out of different generators in Nextcloud: the 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. So the ordinary docker-compose-behind-nginx install answers a perfectly good https://cloud.example.com poll endpoint with "server": "http://nextcloud:11000" -- a name the phone cannot resolve. Discovery then fails, the account is never created, the user is told the account has no task lists, and the app password is already spent and never revoked. Retrying does the same thing again and leaves another dangling entry in their device list. reachableOrigin keeps scheme, host and port from the endpoint we just polled successfully whenever the claimed origin sits outside its registrable domain, and keeps the claimed path so subdirectory installs still work. It compares one server-emitted origin against another, never against what the user typed, so a correctly configured proxy is untouched and a sibling host in the same domain still passes. Coerced, never refused, per the rule the file already states: the credential exists by now and the 200 is spent. The mismatch rides out on Approved so the user can be told which setting is wrong.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user