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.
This commit is contained in:
2026-09-07 23:28:08 +02:00
parent 6aaa15b130
commit 41ffc75fc5
3 changed files with 74 additions and 7 deletions
@@ -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) {
@@ -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
@@ -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()