sync: hand back an app password the flow is not going to use
Nextcloud returns a minted password exactly once. Every path that left the browser flow without saving an account dropped it: discovery failing after approval, an account whose lists we cannot use, start over, the back arrow, switching to a typed password, or leaving Settings altogether. It stays valid on the server for ever, and every attempt is named "Agendula (Android)" -- so a user retrying against a misconfigured server ends up with six identical entries and no way to tell which one their working account uses. They prune nothing, or prune the wrong one. One owned field and one sink rather than a revoke per path: ten paths today, and the eleventh would be forgotten. Ownership passes to the account on Created and is released nowhere else without revoking. The sink runs on the application scope, not viewModelScope -- androidx closes that before onCleared, so a launch there never runs its body. Revocation goes through a new revokeAt, which takes the OCS root directly. ocsRootFor is principal-shaped and falls back to the bare origin, so sending a login flow's server base through it would collapse a subpath install's /nextcloud/ to / and DELETE a path that 404s -- the silent no-op that function exists to prevent. Also stops reporting a rejected credential as an empty account: a 401 after approval is either a home set outside the domain the credential is scoped to, which retrying only mints another password for, or the server having a moment. The registrable-domain check that decides this is now shared with crossDomainHint, which had been computing "com" for example.com and so never firing.
This commit is contained in:
@@ -81,8 +81,24 @@ object AppPassword {
|
||||
httpClient: OkHttpClient,
|
||||
principal: HttpUrl,
|
||||
timeout: Duration = REVOCATION_TIMEOUT,
|
||||
): Boolean = revokeAt(httpClient, ocsRootFor(principal), timeout)
|
||||
|
||||
/**
|
||||
* The same revocation, for a caller that already holds the server root.
|
||||
*
|
||||
* ⚠️ Do not route such a caller through [revoke]. [ocsRootFor] is
|
||||
* *principal*-shaped: it looks for `remote.php` and falls back to the bare
|
||||
* origin when there is none. A login flow hands back a server base, so a
|
||||
* subpath install's `https://host/nextcloud/` would collapse to
|
||||
* `https://host/` and the DELETE would 404 on every one of them — attempted,
|
||||
* and doing nothing, which is the failure [ocsRootFor] exists to prevent.
|
||||
*/
|
||||
fun revokeAt(
|
||||
httpClient: OkHttpClient,
|
||||
ocsRoot: HttpUrl,
|
||||
timeout: Duration = REVOCATION_TIMEOUT,
|
||||
): Boolean = try {
|
||||
val url = ocsRootFor(principal).newBuilder().addPathSegments(PATH).build()
|
||||
val url = ocsRoot.newBuilder().addPathSegments(PATH).build()
|
||||
httpClient.newBuilder()
|
||||
// Shares the pool and dispatcher, so this costs nothing.
|
||||
.callTimeout(timeout.toJavaDuration())
|
||||
|
||||
@@ -42,6 +42,20 @@ class AppPasswordTest {
|
||||
.isEqualTo("https://baikal.example.com/")
|
||||
}
|
||||
|
||||
@Test fun `a server root is used verbatim, not treated as a principal`() {
|
||||
server.enqueue(MockResponse().setResponseCode(200))
|
||||
|
||||
val revoked = AppPassword.revokeAt(httpClient, server.url("/nextcloud/"))
|
||||
|
||||
// ⚠️ ocsRootFor looks for remote.php and falls back to the bare origin.
|
||||
// A login flow hands back a server base, so routing one through revoke()
|
||||
// would collapse /nextcloud/ to / and DELETE a path that 404s on every
|
||||
// subpath install — attempted, and doing nothing.
|
||||
assertThat(revoked).isTrue()
|
||||
assertThat(server.takeRequest().path)
|
||||
.isEqualTo("/nextcloud/ocs/v2.php/core/apppassword")
|
||||
}
|
||||
|
||||
@Test fun `a server that never answers does not hold the removal`() {
|
||||
// Accepts the connection and says nothing — the off-VPN homelab shape.
|
||||
server.enqueue(MockResponse().setSocketPolicy(SocketPolicy.NO_RESPONSE))
|
||||
|
||||
Reference in New Issue
Block a user