From da42423fe2e41fdfbee200956e5fe4dc8d0abc6a Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 4 Sep 2026 17:36:42 +0200 Subject: [PATCH] sync(chunk 2b): CalDAV discovery and auth protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New :caldav module — MIT, plain JVM, api-depends on :dav. Separate from the vendored MPL tree so the licences stay unmixed, and so "no Android types" is a compile-time guarantee rather than a discipline. Chunk 2 split again: the Android account layer (Keystore, AccountManager, Custom Tabs, account-add UI) is 2c, with different verification and an on-device review. - ServiceDiscovery: the RFC 6764 ladder. SRV priority/weight, TXT path=, non-443 ports, "." targets, well-known then root. - CollectionClassifier: the two filters that are inversions of the obvious rule. An absent or empty supported-calendar-component-set means "supports everything", and classification is a positive test for CALDAV:calendar on an unordered set — excluding schedule-outbox would drop SOGo's main calendar. - CalDavDiscovery: OPTIONS gate, principal, every home set, Depth-1 by name. A failing home set does not fail the account, and every home set failing is reported as an error rather than as an account with no lists. - NextcloudLoginFlow: POST not GET, a User-Agent the user can recognise when revoking, 404-means-pending only, both URLs origin-checked, host mismatch carried rather than refused (reverse proxies are ordinary). - PreemptiveBasicInterceptor, ServerQuirks. dnsjava 3.6.3 (BSD-3) added: Android's DnsResolver is callback-only and cannot do the TXT path lookup, and JNDI's DNS provider does not exist on Android. Behind an interface, so every trap is tested with a fake and no network. :dav gains change 6 — is parsed rather than inferred from a null href, which also fires on a merely non-conformant empty element. 52 tests here, 78 in :dav. SYNC.md's live-probed trap table is executable now. --- .forgejo/workflows/ci.yaml | 2 +- caldav/build.gradle.kts | 36 ++ .../jeanlucmakiola/caldav/CalDavDiscovery.kt | 260 +++++++++++++ .../caldav/CollectionClassifier.kt | 107 ++++++ .../jeanlucmakiola/caldav/DnsJavaResolver.kt | 34 ++ .../caldav/NextcloudLoginFlow.kt | 213 +++++++++++ .../caldav/PreemptiveBasicInterceptor.kt | 57 +++ .../de/jeanlucmakiola/caldav/ServerQuirks.kt | 60 +++ .../jeanlucmakiola/caldav/ServiceDiscovery.kt | 139 +++++++ .../caldav/CalDavDiscoveryTest.kt | 359 ++++++++++++++++++ .../caldav/NextcloudLoginFlowTest.kt | 169 +++++++++ .../caldav/PreemptiveBasicInterceptorTest.kt | 102 +++++ .../jeanlucmakiola/caldav/ServerQuirksTest.kt | 36 ++ .../caldav/ServiceDiscoveryTest.kt | 128 +++++++ dav/PROVENANCE.md | 14 + .../dav4jvm/property/CurrentUserPrincipal.kt | 32 +- .../at/bitfire/dav4jvm/LocalChangesTest.kt | 46 +++ docs/SYNC-PLAN.md | 39 +- gradle/libs.versions.toml | 10 + settings.gradle.kts | 1 + 20 files changed, 1833 insertions(+), 11 deletions(-) create mode 100644 caldav/build.gradle.kts create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CollectionClassifier.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/DnsJavaResolver.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt create mode 100644 caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt create mode 100644 caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavDiscoveryTest.kt create mode 100644 caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt create mode 100644 caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt create mode 100644 caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt create mode 100644 caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServiceDiscoveryTest.kt diff --git a/.forgejo/workflows/ci.yaml b/.forgejo/workflows/ci.yaml index a337e70..c3add42 100644 --- a/.forgejo/workflows/ci.yaml +++ b/.forgejo/workflows/ci.yaml @@ -146,7 +146,7 @@ jobs: # the whole safety argument in dav/PROVENANCE.md. - name: Unit tests if: steps.scope.outputs.code == 'true' - run: ./gradlew testDebugUnitTest :dav:test + run: ./gradlew testDebugUnitTest :dav:test :caldav:test - name: Assemble debug APK if: steps.scope.outputs.code == 'true' diff --git a/caldav/build.gradle.kts b/caldav/build.gradle.kts new file mode 100644 index 0000000..e159e77 --- /dev/null +++ b/caldav/build.gradle.kts @@ -0,0 +1,36 @@ +plugins { + // No version — AGP already puts the Kotlin plugin on the build classpath. + id("org.jetbrains.kotlin.jvm") +} + +// Our CalDAV protocol layer: discovery, auth, Nextcloud Login Flow v2. MIT, and +// deliberately a separate module from the MPL-2.0 vendored `:dav`. +// +// A plain JVM module, like `:dav`, and for the same two reasons: it enforces "no +// Android types" at compile time rather than by discipline, and it lets the whole +// protocol layer be tested against MockWebServer on the JVM. Anything that needs +// the platform — Keystore, AccountManager, Custom Tabs — belongs in `:app`. +java { + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 +} + +kotlin { + compilerOptions { + jvmTarget = org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_17 + } +} + +dependencies { + api(project(":dav")) + implementation(libs.dnsjava) + // Runtime API only (Json.parseToJsonElement) — no @Serializable, so the + // serialization compiler plugin is not needed here. + implementation(libs.kotlinx.serialization.json) + + compileOnly(libs.xpp3) + testImplementation(libs.xpp3) + testImplementation(libs.junit4) + testImplementation(libs.okhttp.mockwebserver) + testImplementation(libs.truth) +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt new file mode 100644 index 0000000..3cf265b --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt @@ -0,0 +1,260 @@ +package de.jeanlucmakiola.caldav + +import at.bitfire.dav4jvm.DavResource +import at.bitfire.dav4jvm.Response +import at.bitfire.dav4jvm.property.CalendarColor +import at.bitfire.dav4jvm.property.CalendarHomeSet +import at.bitfire.dav4jvm.property.CurrentUserPrincipal +import at.bitfire.dav4jvm.property.CurrentUserPrivilegeSet +import at.bitfire.dav4jvm.property.DisplayName +import at.bitfire.dav4jvm.property.MaxICalendarSize +import at.bitfire.dav4jvm.property.ResourceType +import at.bitfire.dav4jvm.property.SupportedCalendarComponentSet +import at.bitfire.dav4jvm.property.SupportedReportSet +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import okhttp3.OkHttpClient + +/** + * RFC 6764 discovery: from what the user typed to the list of task collections. + * + * The happy path is five requests. Everything else here is a rule that exists + * because a real server broke the obvious implementation — see + * `docs/SYNC.md` § *Generic CalDAV discovery*, whose live-probed trap table this + * encodes. + */ +class CalDavDiscovery( + private val httpClient: OkHttpClient, + private val dns: DnsResolver = DnsResolver.None, + /** + * Whether a typed `http://` base URL may be probed. + * + * Off by default. Credentials are never sent over cleartext — the + * interceptor withholds them — so an http:// server would otherwise answer + * 401 forever and the user would be told their password was wrong when the + * real problem is the scheme. `docs/SYNC.md` requires any escape hatch to be + * a narrow, warned, per-account opt-in; this is that switch. + */ + private val allowCleartext: Boolean = false, +) { + + /** Properties requested **by name**, because allprop legitimately omits them. */ + private val collectionProperties = arrayOf( + ResourceType.NAME, + DisplayName.NAME, + CalendarColor.NAME, + SupportedCalendarComponentSet.NAME, + SupportedReportSet.NAME, + CurrentUserPrivilegeSet.NAME, + MaxICalendarSize.NAME, + ) + + /** A home set that could not be listed. One failing must not hide the others. */ + data class HomeSetFailure( + val url: HttpUrl, + val reason: String, + val needsAuthentication: Boolean, + ) + + sealed interface Outcome { + data class Found( + val principal: HttpUrl, + val collections: List, + /** Set when a 301/308 moved us; the caller must persist it. */ + val movedTo: HttpUrl?, + /** Home sets on a different host than the principal. Normative, but worth surfacing. */ + val crossHostHomeSets: List, + /** Home sets that could not be read. The rest of the result is still good. */ + val failedHomeSets: List = emptyList(), + ) : Outcome + + /** + * The server wants credentials. Not a failure — authenticate and retry. + * + * [hosts] names *which* hosts asked, which the caller needs in order to + * widen the credential allowlist. A cross-host home set (iCloud puts the + * principal on `caldav.icloud.com` and the home set on + * `pNN-caldav.icloud.com`) is otherwise unreachable: the interceptor + * withholds the credential from the second host, and without naming it + * here the caller can never learn what to allow. + */ + data class NeedsAuthentication(val hosts: List) : Outcome + + /** A 200 whose body says the credentials were not accepted (RFC 5397 §3). */ + data object Unauthenticated : Outcome + + data class NotCalDav(val reason: String) : Outcome + + data class Failed(val reason: String) : Outcome + } + + /** + * Walks every candidate for [input] and returns the first real result. + * + * A 401 stops the walk immediately: it means we found a DAV server and simply + * have no credentials for it, and continuing down the ladder would replace a + * precise "sign in" with a vague "nothing found". + */ + fun discover(input: String): Outcome { + ServiceDiscovery.asBaseUrl(input)?.let { typed -> + if (!typed.isHttps && !allowCleartext) { + return Outcome.Failed( + "\"$input\" is an unencrypted http:// address. Credentials are never sent " + + "over cleartext, so this can only ever answer \"not authorised\".", + ) + } + } + + val candidates = ServiceDiscovery.candidatesFor(input, dns) + if (candidates.isEmpty()) return Outcome.Failed("could not read \"$input\" as an address or URL") + + var lastFailure: Outcome = Outcome.Failed("no candidate answered") + for (candidate in candidates) { + when (val outcome = probe(candidate.url)) { + is Outcome.Found, is Outcome.NeedsAuthentication, Outcome.Unauthenticated -> return outcome + else -> lastFailure = outcome + } + } + return lastFailure + } + + internal fun probe(base: HttpUrl): Outcome { + val resource = DavResource(httpClient, base) + + // The OPTIONS gate. `DAV: calendar-access` is what distinguishes a CalDAV + // server from any other WebDAV host, and it is what keeps Google's SRV + // record — which points at something that answers 405 to PROPFIND — from + // looking like a discovery that merely found no calendars. + var davCapabilities: Set = emptySet() + runCatching { resource.options { capabilities, _ -> davCapabilities = capabilities } } + + var principalHref: String? = null + var unauthenticated = false + val propfindResult = runCatching { + resource.propfind(0, CurrentUserPrincipal.NAME) { response, _ -> + response[CurrentUserPrincipal::class.java]?.let { + principalHref = it.href + unauthenticated = unauthenticated || it.unauthenticated + } + } + } + + propfindResult.exceptionOrNull()?.let { error -> + // 401 is not a failure — iCloud and Zoho answer it from + // /.well-known/caldav, which *is* the DAV root, and it is RFC-legal. + if (isUnauthorized(error)) return Outcome.NeedsAuthentication(listOf(base.host)) + return Outcome.Failed(error.message ?: error.toString()) + } + + // ⚠️ RFC 5397 §3: a 200 carrying means the + // credentials were rejected. Without this check a failed login looks like + // a successful discovery that found nothing — the shape of bug report + // nobody can act on. The element is parsed explicitly (dav/PROVENANCE.md + // change 6) rather than inferred from a null href, which would also fire + // for a merely non-conformant empty element. + if (unauthenticated) return Outcome.Unauthenticated + + val href = principalHref + ?: return if (davCapabilities.contains("calendar-access")) { + Outcome.Failed("server advertises calendar-access but returned no principal") + } else { + Outcome.NotCalDav("no DAV:current-user-principal, and no calendar-access in OPTIONS") + } + + if (davCapabilities.isNotEmpty() && !davCapabilities.contains("calendar-access")) { + return Outcome.NotCalDav("OPTIONS advertises ${davCapabilities.joinToString()} but not calendar-access") + } + + val principal = resource.location.resolve(href) + ?: return Outcome.Failed("principal href \"$href\" is not a usable URL") + + return fromPrincipal(principal, movedTo = resource.permanentLocation) + } + + internal fun fromPrincipal(principal: HttpUrl, movedTo: HttpUrl? = null): Outcome { + val homeSets = mutableListOf() + val principalResource = DavResource(httpClient, principal) + val homeSetResult = runCatching { + principalResource.propfind(0, CalendarHomeSet.NAME) { response, _ -> + // ⚠️ Iterate ALL hrefs. Multiple home sets are normative (RFC 4791 + // §6.2.1's own example) and iCloud depends on it: the principal is + // on caldav.icloud.com and the home set on pNN-caldav.icloud.com. + response[CalendarHomeSet::class.java]?.hrefs?.forEach { raw -> + principal.resolve(raw)?.let(homeSets::add) + } + } + } + homeSetResult.exceptionOrNull()?.let { error -> + if (isUnauthorized(error)) return Outcome.NeedsAuthentication(listOf(principal.host)) + return Outcome.Failed(error.message ?: error.toString()) + } + + if (homeSets.isEmpty()) return Outcome.Failed("principal has no calendar-home-set") + + // Cross-host is legal and required, but never over plain HTTP: the + // credentials follow the home set, and a downgrade would send them in the + // clear. The caller surfaces the host change to the user. + val crossHost = homeSets.filter { it.host != principal.host } + val insecure = homeSets.filter { principal.isHttps && !it.isHttps } + if (insecure.isNotEmpty()) { + return Outcome.Failed("calendar-home-set downgrades to HTTP: ${insecure.first()}") + } + + val collections = linkedMapOf() + val failures = mutableListOf() + var anySucceeded = false + + for (homeSet in homeSets.distinct()) { + val result = runCatching { + DavResource(httpClient, homeSet).propfind(1, *collectionProperties) { response, relation -> + if (relation == Response.HrefRelation.SELF) return@propfind + CollectionClassifier.classify(response)?.let { collections[it.url] = it } + } + } + result.fold( + // A failed home set must not fail the account — the same rule the + // engine runs on. One broken share must not hide every other list, + // and a 401 from a cross-host home set must not tell the user to + // sign in again with credentials that just worked. + onSuccess = { anySucceeded = true }, + onFailure = { + failures += HomeSetFailure( + url = homeSet, + reason = it.message ?: it.toString(), + needsAuthentication = isUnauthorized(it), + ) + }, + ) + } + + // Every home set failing is a server problem, not a discovery that found + // no lists. Reporting it as success gives the user "connected, no task + // lists" for what is actually an error. + if (!anySucceeded) { + val needAuth = failures.filter { it.needsAuthentication } + return if (needAuth.isNotEmpty() && needAuth.size == failures.size) { + Outcome.NeedsAuthentication(needAuth.map { it.url.host }.distinct()) + } else { + Outcome.Failed( + failures.firstOrNull()?.reason ?: "no calendar-home-set could be listed", + ) + } + } + + return Outcome.Found( + principal = principal, + collections = collections.values.toList(), + movedTo = movedTo, + crossHostHomeSets = crossHost, + failedHomeSets = failures, + ) + } + + private fun isUnauthorized(error: Throwable): Boolean = + error is at.bitfire.dav4jvm.exception.UnauthorizedException + + companion object { + /** `https://host/path` → the URL, or null. Convenience for callers. */ + fun url(value: String): HttpUrl? = value.toHttpUrlOrNull() + } +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CollectionClassifier.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CollectionClassifier.kt new file mode 100644 index 0000000..10e3705 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CollectionClassifier.kt @@ -0,0 +1,107 @@ +package de.jeanlucmakiola.caldav + +import at.bitfire.dav4jvm.Property +import at.bitfire.dav4jvm.Response +import at.bitfire.dav4jvm.property.CurrentUserPrivilegeSet +import at.bitfire.dav4jvm.property.ResourceType +import at.bitfire.dav4jvm.property.SupportedCalendarComponentSet +import at.bitfire.dav4jvm.property.SupportedReportSet +import okhttp3.HttpUrl + +/** A collection that survived classification, and what we know about it. */ +data class TaskCollection( + val url: HttpUrl, + val displayName: String?, + /** Packed ARGB, the same form `task_lists.color` stores. */ + val color: Int?, + val readOnly: Boolean, + /** + * A share rather than the user's own collection. + * + * Worth carrying: Nextcloud **rewrites task bodies on GET from a shared + * calendar** — stripping `VALARM`, and reducing `CLASS:CONFIDENTIAL` to a + * VEVENT-shaped whitelist — while leaving the ETag untouched. Re-PUTting what + * we downloaded destroys the owner's task, so the engine needs to know. + */ + val isShared: Boolean, + val supportsSyncCollection: Boolean, + val maxResourceSize: Long?, +) + +/** + * Decides which collections in a Depth-1 listing can hold tasks. + * + * Both of the rules here are **inversions of the obvious one**, and + * `docs/SYNC.md` calls them the important part of discovery. Getting either + * backwards produces a client that silently finds nothing on a large fraction of + * real servers, with no error to diagnose. + */ +object CollectionClassifier { + + /** `CS:shared`, which Nextcloud adds alongside `caldav:calendar` on a share. */ + private val SHARED = Property.Name("http://calendarserver.org/ns/", "shared") + + fun classify(response: Response): TaskCollection? { + val resourceType = response[ResourceType::class.java] ?: return null + + // ⚠️ Positive test on an *unordered set*, never exclusion and never by + // position. Excluding `schedule-outbox` — the obvious rule — drops SOGo's + // main personal calendar, which reports collection + calendar + + // schedule-outbox simultaneously for every non-Apple client, i.e. for us. + // The positive rule also keeps shared calendars (they add CS:shared) and + // still drops Nextcloud's nc:deleted-calendar, which deliberately strips + // caldav:calendar. + if (ResourceType.CALENDAR !in resourceType.types) return null + + if (!supportsTasks(response)) return null + + val privileges = response[CurrentUserPrivilegeSet::class.java] + // Absent means writable. RFC 3744 §3.7 lets a server withhold the + // property, and DAVx5 defaults to writable for the same reason: assuming + // read-only hides collections the user can perfectly well write to. + // A 403 on write is handled where it happens. + val readOnly = privileges != null && + !privileges.mayWriteContent && !privileges.mayBind + + val reports = response[SupportedReportSet::class.java] + + return TaskCollection( + url = response.href, + displayName = response[at.bitfire.dav4jvm.property.DisplayName::class.java]?.displayName, + // CalendarColor.color is a packed ARGB Int. Calling toString() on it + // yields "-65536" for red — an unparseable decimal, not a colour. + color = response[at.bitfire.dav4jvm.property.CalendarColor::class.java]?.color, + readOnly = readOnly, + isShared = SHARED in resourceType.types, + // A hint, not a contract — Radicale advertised this for years without + // implementing it, so the engine must still degrade gracefully. + supportsSyncCollection = reports?.reports?.contains(SupportedReportSet.SYNC_COLLECTION) == true, + maxResourceSize = response[at.bitfire.dav4jvm.property.MaxICalendarSize::class.java]?.maxSize, + ) + } + + /** + * ⚠️ **An absent `supported-calendar-component-set` means "supports + * everything", not "supports nothing".** + * + * RFC 4791 §5.2.3 says the property SHOULD NOT be returned from an allprop + * request, so it is legitimately missing unless asked for **by name** — and + * even then plenty of servers omit it. Keeping only collections whose set + * *includes* VTODO therefore drops every server that does not advertise it. + * + * The grammar is `(comp+)`; an empty element is non-conformant, and + * dav4jvm's parser starts all-`false`, so an empty one arrives + * indistinguishable from "no VTODO". Treat that as all too — the cost of + * being wrong is an empty collection list, and the cost of the other error is + * a task list the user cannot reach. + */ + internal fun supportsTasks(response: Response): Boolean { + val components = response[SupportedCalendarComponentSet::class.java] ?: return true + val advertisesNothing = !components.supportsEvents && + !components.supportsTasks && + !components.supportsJournal + if (advertisesNothing) return true + return components.supportsTasks + } + +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/DnsJavaResolver.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/DnsJavaResolver.kt new file mode 100644 index 0000000..3824dd1 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/DnsJavaResolver.kt @@ -0,0 +1,34 @@ +package de.jeanlucmakiola.caldav + +import org.xbill.DNS.Lookup +import org.xbill.DNS.SRVRecord +import org.xbill.DNS.TXTRecord +import org.xbill.DNS.Type + +/** + * SRV/TXT lookups over dnsjava. + * + * Android exposes no usable SRV API — `DnsResolver` arrived in API 29 but is + * callback-only and does not help with the `TXT path=` half — and JNDI's DNS + * provider does not exist on Android at all. dnsjava is what DAVx5 uses. + * + * A lookup that fails returns an empty list rather than throwing: DNS being + * unavailable means "no SRV record", which is an ordinary and common answer, and + * the well-known ladder still has rungs left. + */ +class DnsJavaResolver : DnsResolver { + + override fun srv(name: String): List = runCatching { + Lookup(name, Type.SRV).run() + .orEmpty() + .filterIsInstance() + .map { SrvRecord(it.priority, it.weight, it.port, it.target.toString(true)) } + }.getOrDefault(emptyList()) + + override fun txt(name: String): List = runCatching { + Lookup(name, Type.TXT).run() + .orEmpty() + .filterIsInstance() + .flatMap { it.strings } + }.getOrDefault(emptyList()) +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt new file mode 100644 index 0000000..1ed8fb7 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlow.kt @@ -0,0 +1,213 @@ +package de.jeanlucmakiola.caldav + +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import okhttp3.FormBody +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import okhttp3.OkHttpClient +import okhttp3.Request + +/** + * Nextcloud Login Flow v2. + * + * The user approves the app in a browser and the server mints a dedicated **app + * password**, so the account password never touches this device and the grant can + * be revoked from the server's own settings. It is not deprecated, there is no + * v3, and OAuth2 is a worse fit. + * + * Every rule below is a correction from `docs/SYNC.md`'s audit of the first + * draft, and each one turns a diagnosable error back into a diagnosable error. + */ +class NextcloudLoginFlow( + private val httpClient: OkHttpClient, + /** + * ⚠️ Becomes the **app password's name** in Settings → Security → Devices & + * sessions. With OkHttp's default the user sees `okhttp/4.12.0` and cannot + * tell what to revoke — which defeats the entire point of the flow. + */ + private val userAgent: String, +) { + + private val json = Json { ignoreUnknownKeys = true } + + /** The 20-minute server-side lifetime (`lifetime = 1200` in `LoginFlowV2Mapper.php`). */ + val flowLifetimeSeconds = 1200L + + /** + * A started flow. Persist this **before** launching the browser: the flow + * outlives our process, and Custom Tabs return no result when dismissed. + */ + data class Flow( + val loginUrl: HttpUrl, + val pollEndpoint: HttpUrl, + val pollToken: String, + val deadlineEpochSeconds: Long, + /** + * Set when the server sent us to a different host than the user typed. + * **Carried, not thrown**: it is generated from `overwrite.cli.url` / + * `overwriteprotocol` / `trusted_proxies` and is legitimate behind a + * reverse proxy, which describes a large share of self-hosted installs. + * Refusing outright would make Login Flow v2 unusable for them. The UI + * confirms it with the user, quoting the cause. + */ + val hostMismatch: HostMismatch? = null, + ) + + data class Credentials(val server: HttpUrl, val loginName: String, val appPassword: String) + + sealed interface PollResult { + data class Approved(val credentials: Credentials) : PollResult + /** 404: still waiting. Also what an expired or already-consumed flow returns. */ + data object Pending : PollResult + data class Expired(val reason: String) : PollResult + data class Failed(val reason: String) : PollResult + } + + /** + * @param server the base URL the user typed + * @param now epoch seconds, for the locally tracked deadline + */ + fun start(server: HttpUrl, now: Long): Result = runCatching { + val request = Request.Builder() + .url(server.newBuilder().addPathSegments("index.php/login/v2").build()) + .header("User-Agent", userAgent) + // OCS-APIRequest is *not* needed: v2 is a Frontpage route, not OCS. + .post(FormBody.Builder().build()) + .build() + + httpClient.newCall(request).execute().use { response -> + if (!response.isSuccessful) error("login flow init failed: HTTP ${response.code}") + val body = response.body?.string().orEmpty() + val root = json.parseToJsonElement(body).jsonObject + val poll = root["poll"]?.jsonObject ?: error("no poll object in login flow response") + + val endpointRaw = poll["endpoint"]?.jsonPrimitive?.content + ?: error("no poll endpoint") + val endpoint = endpointRaw.toHttpUrlOrNull() ?: error("poll endpoint is not a URL") + requireSecureOrigin(server, endpoint) + + val loginUrl = root["login"]?.jsonPrimitive?.content?.toHttpUrlOrNull() + ?: error("no login URL") + // ⚠️ The login URL is where the user types their **account** password, + // in a browser. Validating only the poll endpoint leaves the more + // dangerous of the two unchecked: a misconfigured or hostile server — + // or a MITM on a typed http:// base — could point the browser at any + // origin and harvest it. + requireSecureOrigin(server, loginUrl) + + Flow( + loginUrl = loginUrl, + pollEndpoint = endpoint, + pollToken = poll["token"]?.jsonPrimitive?.content ?: error("no poll token"), + deadlineEpochSeconds = now + flowLifetimeSeconds, + hostMismatch = hostMismatchOf(server, endpoint) ?: hostMismatchOf(server, loginUrl), + ) + } + } + + /** + * One poll. The 200 is returned **exactly once** — the server deletes the row + * inside `poll()` before returning — so the caller must persist the result + * immediately. + */ + fun poll(flow: Flow, now: Long): PollResult { + if (now > flow.deadlineEpochSeconds) { + // 404 is also what an expired flow returns, so the deadline is tracked + // locally or "expired" is indistinguishable from "still waiting". + return PollResult.Expired("the 20-minute approval window has passed") + } + + val request = Request.Builder() + .url(flow.pollEndpoint) + .header("User-Agent", userAgent) + // ⚠️ POST, form-encoded. A GET gets 405. + .post(FormBody.Builder().add("token", flow.pollToken).build()) + .build() + + return runCatching { + httpClient.newCall(request).execute().use { response -> + when { + // ⚠️ 404 means pending, and *only* 404 does. "Anything that + // isn't 200 is pending" swallows 429 (brute-force protection), + // 503 (maintenance), a Cloudflare challenge page and every + // DNS/TLS failure — turning a diagnosable error into a + // twenty-minute spinner. + response.code == 404 -> PollResult.Pending + + response.code == 200 -> { + val contentType = response.header("Content-Type").orEmpty() + // A Cloudflare challenge is a 200 carrying HTML. + if (!contentType.contains("application/json", ignoreCase = true)) { + PollResult.Failed("server answered 200 with $contentType, not JSON") + } else { + parseCredentials(flow, response.body?.string().orEmpty()) + } + } + + response.code == 429 -> + PollResult.Failed("the server is rate-limiting this address (429)") + + response.code == 503 -> + PollResult.Failed("the server is in maintenance mode (503)") + + else -> PollResult.Failed("unexpected HTTP ${response.code}") + } + } + }.getOrElse { PollResult.Failed(it.message ?: it.toString()) } + } + + private fun parseCredentials(flow: Flow, body: String): PollResult = runCatching { + val root = json.parseToJsonElement(body).jsonObject + val server = root["server"]?.jsonPrimitive?.content?.toHttpUrlOrNull() + ?: error("no server URL in poll response") + // Scheme only. A host mismatch here must never discard the credentials: + // the 200 is returned exactly once — the server deletes the row inside + // poll() before returning — so throwing would burn the app password and + // force the user through the whole flow again. + requireSecureOrigin(flow.pollEndpoint, server) + PollResult.Approved( + Credentials( + server = server, + // ⚠️ loginName is what the user typed — possibly an email, an + // LDAP-derived value, or the right name in the wrong case. It is + // the Basic auth username and nothing else. Interpolating it into + // `remote.php/dav/calendars//` is the classic "logged + // in but no calendars" bug; the principal comes from discovery. + loginName = root["loginName"]?.jsonPrimitive?.content ?: error("no loginName"), + appPassword = root["appPassword"]?.jsonPrimitive?.content ?: error("no appPassword"), + ), + ) + }.getOrElse { PollResult.Failed(it.message ?: it.toString()) } + + /** + * The endpoint is generated from `overwrite.cli.url` / `overwriteprotocol` / + * `trusted_proxies`, which are misconfigured on a large fraction of + * self-hosted installs — so it is validated rather than trusted verbatim. + * + * A downgrade to `http` is refused outright: the poll token is exchanged for a + * long-lived app password, which makes it a credential-grade secret. + */ + /** + * A downgrade to `http` is **fatal**. The poll token is exchanged for a + * long-lived app password and the login URL takes the account password, so + * both are credential-grade. + */ + internal fun requireSecureOrigin(expected: HttpUrl, actual: HttpUrl) { + if (expected.isHttps && !actual.isHttps) { + error("the server returned an http:// URL (${actual.host}) for an https:// server") + } + } + + /** 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 + + /** The server pointed the flow at a different host than the user typed. */ + data class HostMismatch(val expected: String, val actual: String) { + val message: String + get() = "the server sent us to \"$actual\" but you typed \"$expected\" — " + + "its overwrite.cli.url is probably wrong" + } +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt new file mode 100644 index 0000000..e8f0894 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt @@ -0,0 +1,57 @@ +package de.jeanlucmakiola.caldav + +import okhttp3.Credentials +import okhttp3.HttpUrl +import okhttp3.Interceptor +import okhttp3.Response + +/** + * Sends Basic credentials up front, rather than waiting to be challenged. + * + * OkHttp's `Authenticator` is **reactive only**: it fires after a 401, which + * costs an extra round trip on every request of a PROPFIND-heavy sync — and it + * never fires at all on servers that answer 403 or 404 without a challenge. + * + * Two guards, and neither is optional. The credential goes out **only over + * HTTPS**, and **only to the account's own origin** — a home set may legally live + * on another host, and OkHttp deliberately strips `Authorization` across a + * redirect, so re-attaching it is something we must do knowingly, per host, after + * validating the target. Never blanket. + */ +class PreemptiveBasicInterceptor( + private val username: String, + private val password: String, + /** Hosts this credential may be sent to. */ + private val allowedHosts: Set, +) : Interceptor { + + constructor(username: String, password: String, origin: HttpUrl) : + this(username, password, setOf(origin.host)) + + /** + * Lowercased once. OkHttp already lower-cases and punycodes `url.host`, so an + * IDN written in Unicode here would never match — callers pass the host from + * an [HttpUrl], which is already in that form. + */ + private val hosts = allowedHosts.map { it.lowercase() }.toSet() + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val url = request.url + + val allowed = url.isHttps && url.host in hosts + if (!allowed || request.header("Authorization") != null) { + return chain.proceed(request) + } + + return chain.proceed( + request.newBuilder() + // ⚠️ UTF-8, not OkHttp's ISO-8859-1 default. A password with ä, ö + // or ß is otherwise sent as different bytes than the server + // expects — coming back as a 401 the user reads as "wrong + // password". The vendored BasicDigestAuthHandler does the same. + .header("Authorization", Credentials.basic(username, password, Charsets.UTF_8)) + .build(), + ) + } +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt new file mode 100644 index 0000000..a78d7c5 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt @@ -0,0 +1,60 @@ +package de.jeanlucmakiola.caldav + +import okhttp3.HttpUrl + +/** + * What we know about a server before talking to it. + * + * This exists for one reason: *"wrong password" that is actually "you used your + * account password"* is the single most common support ticket any CalDAV client + * inherits. Detecting it at account-add time by domain turns a dead end into one + * sentence of instruction. + */ +enum class ServerQuirk(val domains: Set) { + + /** Fastmail: needs an app password, and CalDAV is not on the Basic plan. */ + FASTMAIL_APP_PASSWORD(setOf("fastmail.com", "fastmail.fm", "messagingengine.com")), + + /** iCloud: app-specific password, and 2FA must be on to mint one. */ + ICLOUD_APP_SPECIFIC_PASSWORD(setOf("icloud.com", "me.com", "mac.com")), + + /** + * Google: OAuth2-only, and it supports neither VTODO nor MKCALENDAR — its own + * documentation says *"Doesn't support VTODO or VJOURNAL data"*. `SYNC.md` + * drops it as a target, so this is a refusal with an explanation rather than + * a 401 the user cannot act on. + */ + GOOGLE_UNSUPPORTED(setOf("gmail.com", "googlemail.com", "google.com")), + + /** + * Nextcloud's brute-force protection throttles then 429s **per source IP**, so + * a retry loop on a dead app password takes down the user's other Nextcloud + * clients on that network. Not domain-detectable; set when a server identifies + * itself. Kept here so the engine has one place to ask. + */ + NEXTCLOUD_BRUTE_FORCE_PROTECTED(emptySet()), + ; + + companion object { + + /** The quirk implied by an email address or a URL host, if any. */ + fun forInput(input: String): ServerQuirk? { + val host = ServiceDiscovery.asBaseUrl(input)?.host + ?: ServiceDiscovery.domainOf(input) + ?: return null + return forHost(host) + } + + fun forHost(host: String): ServerQuirk? { + val lower = host.lowercase().trimEnd('.') + return entries.firstOrNull { quirk -> + quirk.domains.any { lower == it || lower.endsWith(".$it") } + } + } + + fun forUrl(url: HttpUrl): ServerQuirk? = forHost(url.host) + } + + /** True when discovery should not even be attempted. */ + val isFatal: Boolean get() = this == GOOGLE_UNSUPPORTED +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt new file mode 100644 index 0000000..fdf7339 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt @@ -0,0 +1,139 @@ +package de.jeanlucmakiola.caldav + +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull + +/** One `_caldavs._tcp` SRV record. */ +data class SrvRecord( + val priority: Int, + val weight: Int, + val port: Int, + val target: String, +) + +/** + * DNS lookups for RFC 6764. Behind an interface so the pipeline is testable + * without a network, and so the resolver can be swapped per platform. + */ +interface DnsResolver { + fun srv(name: String): List + fun txt(name: String): List + + /** For the base-URL path, where DNS is never consulted. */ + object None : DnsResolver { + override fun srv(name: String) = emptyList() + override fun txt(name: String) = emptyList() + } +} + +/** + * Turns what the user typed into the ordered list of URLs worth probing. + * + * This is RFC 6764 §6 (`SRV` + `TXT`) followed by the well-known ladder, and it + * is where `docs/SYNC.md`'s live-probed trap table lives. Every rule below cost + * somebody a support ticket: + * + * - **`TXT path=` is not optional.** Posteo publishes `path=/`, GMX and Web.de + * `path=/begenda/dav/users/`. Skipping the TXT lookup lands both on the wrong + * path and discovery finds nothing. + * - **The SRV port is not 443.** Posteo is SRV-only on **8443**, and its + * `/.well-known/caldav` 404s. Hardcoding the port fails it outright. + * - **A `.` target means "explicitly unavailable"** (RFC 2782), not "no record". + * `_caldav._tcp.fastmail.com` and `runbox.com` both answer `0 0 0 .`. + * - **`/` is the last rung**, after the TXT path and `/.well-known/caldav`. The + * draft omitted it. + * - **Priority and weight are honoured**, lowest priority first. + */ +object ServiceDiscovery { + + private const val SRV_SERVICE = "_caldavs._tcp" + private const val WELL_KNOWN = "/.well-known/caldav" + + /** A URL to probe, and where it came from — the "why" a failure report needs. */ + data class Candidate(val url: HttpUrl, val origin: String) + + /** + * Hosts whose SRV record leads somewhere that is not a DAV server. + * + * Google publishes a valid `_caldavs._tcp` record pointing at + * `calendar.google.com`, which answers **405** to PROPFIND. A strict RFC 6764 + * client follows it into a dead end for every `@gmail.com` address. Google is + * out of scope anyway — it supports neither VTODO nor MKCALENDAR — so this is + * caught at the front rather than surfaced as a baffling 405. + */ + private val SRV_DEAD_ENDS = setOf("calendar.google.com") + + /** + * @param input what the user typed: an email address, a `mailto:` URI, or a + * base URL + */ + fun candidatesFor(input: String, dns: DnsResolver = DnsResolver.None): List { + val trimmed = input.trim() + if (trimmed.isEmpty()) return emptyList() + + // A typed base URL is used as typed. PROPFIND on it can return principal, + // home-set and collection in one response, so DNS is never consulted. + asBaseUrl(trimmed)?.let { return listOf(Candidate(it, "base URL as typed")) } + + val domain = domainOf(trimmed) ?: return emptyList() + val candidates = mutableListOf() + + val srv = dns.srv("$SRV_SERVICE.$domain") + .filterNot { it.target == "." || it.target.isEmpty() || it.port == 0 } + // DNS names are case-insensitive and dnsjava returns the wire case, so + // "Calendar.Google.com." would otherwise walk straight past the guard. + .filterNot { it.target.trimEnd('.').lowercase() in SRV_DEAD_ENDS } + .sortedWith(compareBy({ it.priority }, { -it.weight })) + + // The TXT path applies to the SRV target, and to the bare domain when + // there is no SRV record — that is the GMX/Web.de shape. + val txtPath = dns.txt("$SRV_SERVICE.$domain") + .firstNotNullOfOrNull { record -> + record.split(' ', ';') + .firstOrNull { it.startsWith("path=", ignoreCase = true) } + ?.substringAfter('=') + ?.takeIf { it.isNotEmpty() } + } + + val origins = srv.map { record -> + val host = record.target.trimEnd('.').lowercase() + // Port 443 is the default and stays implicit; anything else is + // explicit, which is the whole point of Posteo's 8443. + val port = if (record.port == 443) "" else ":${record.port}" + "https://$host$port" to "SRV $host$port" + }.ifEmpty { + listOf("https://$domain" to "domain as typed") + } + + for ((origin, label) in origins) { + if (txtPath != null) { + add(candidates, origin, txtPath, "$label + TXT path=$txtPath") + } + add(candidates, origin, WELL_KNOWN, "$label + .well-known") + add(candidates, origin, "/", "$label + root") + } + return candidates + } + + private fun add(into: MutableList, origin: String, path: String, label: String) { + val url = (origin.trimEnd('/') + "/" + path.trimStart('/')).toHttpUrlOrNull() ?: return + if (into.none { it.url == url }) into += Candidate(url, label) + } + + /** The input as a base URL, or null if it is an address rather than a URL. */ + internal fun asBaseUrl(input: String): HttpUrl? { + if (!input.startsWith("http://", ignoreCase = true) && + !input.startsWith("https://", ignoreCase = true) + ) { + return null + } + return input.toHttpUrlOrNull() + } + + /** The domain of an email address, a `mailto:` URI, or a bare domain. */ + internal fun domainOf(input: String): String? { + val withoutScheme = input.removePrefix("mailto:").removePrefix("MAILTO:") + val domain = withoutScheme.substringAfterLast('@').trim().trimEnd('.') + return domain.takeIf { it.isNotEmpty() && it.contains('.') && !it.contains('/') } + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavDiscoveryTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavDiscoveryTest.kt new file mode 100644 index 0000000..e727509 --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavDiscoveryTest.kt @@ -0,0 +1,359 @@ +package de.jeanlucmakiola.caldav + +import com.google.common.truth.Truth.assertThat +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test + +class CalDavDiscoveryTest { + + private val httpClient = OkHttpClient.Builder().followRedirects(false).build() + private val server = MockWebServer() + private lateinit var discovery: CalDavDiscovery + + @Before fun start() { + server.start() + discovery = CalDavDiscovery(httpClient) + } + + @After fun stop() = server.shutdown() + + private fun options(dav: String = "1, 2, 3, calendar-access") = + MockResponse().setResponseCode(200).setHeader("DAV", dav) + + private fun multistatus(body: String) = MockResponse() + .setResponseCode(207) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody(body) + + private fun principalResponse(href: String?) = multistatus( + """ + + + /dav/ + + ${href?.let { "$it" } ?: ""} + HTTP/1.1 200 OK + + + """.trimIndent(), + ) + + private fun homeSetResponse(vararg hrefs: String) = multistatus( + """ + + + /principals/me/ + + ${hrefs.joinToString("\n") { "$it" }} + HTTP/1.1 200 OK + + + """.trimIndent(), + ) + + /** One `` for a collection in a Depth-1 listing. */ + private fun collection( + href: String, + resourceTypes: String, + componentSet: String? = null, + displayName: String = "Tasks", + extraProps: String = "", + ) = """ + + $href + + $resourceTypes + $displayName + ${componentSet ?: ""} + $extraProps + HTTP/1.1 200 OK + + """ + + private fun listing(vararg responses: String) = multistatus( + """ + + /dav/calendars/me/ + + HTTP/1.1 200 OK + ${responses.joinToString("\n")} + + """.trimIndent(), + ) + + private fun discoverCollections(listingBody: MockResponse): List { + server.enqueue(homeSetResponse("/dav/calendars/me/")) + server.enqueue(listingBody) + val outcome = discovery.fromPrincipal(server.url("/principals/me/")) + assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.Found::class.java) + return (outcome as CalDavDiscovery.Outcome.Found).collections + } + + // ------------------------------------------------------------ principal + + @Test + fun `a 200 carrying unauthenticated is a failed login, not an empty result`() { + // RFC 5397 section 3. Without this check a rejected credential looks like + // a successful discovery that happened to find nothing — which is the + // shape of bug report nobody can act on. + server.enqueue(options()) + server.enqueue(principalResponse(null)) + + assertThat(discovery.probe(server.url("/dav/"))) + .isEqualTo(CalDavDiscovery.Outcome.Unauthenticated) + } + + @Test + fun `a 401 means sign in, not failure`() { + // iCloud and Zoho answer 401 from /.well-known/caldav — the endpoint *is* + // the DAV root and wants auth. RFC-legal, and must not end the walk. + server.enqueue(options()) + server.enqueue(MockResponse().setResponseCode(401)) + + assertThat(discovery.probe(server.url("/dav/"))) + .isInstanceOf(CalDavDiscovery.Outcome.NeedsAuthentication::class.java) + } + + @Test + fun `a WebDAV server without calendar-access is not a CalDAV server`() { + server.enqueue(options(dav = "1, 2, 3")) + server.enqueue(principalResponse("/principals/me/")) + + val outcome = discovery.probe(server.url("/dav/")) + assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.NotCalDav::class.java) + } + + // ------------------------------------------------------------ home sets + + @Test + fun `every calendar-home-set href is followed, not just the first`() { + // Multiple home sets are normative (RFC 4791 section 6.2.1's own example) + // and iCloud depends on it. + server.enqueue(homeSetResponse("/dav/one/", "/dav/two/")) + server.enqueue(listing(collection("/dav/one/tasks/", ""))) + server.enqueue(listing(collection("/dav/two/more/", ""))) + + val outcome = discovery.fromPrincipal(server.url("/principals/me/")) + val found = outcome as CalDavDiscovery.Outcome.Found + assertThat(found.collections.map { it.url.encodedPath }) + .containsExactly("/dav/one/tasks/", "/dav/two/more/") + } + + @Test + fun `a 401 on one home set does not invalidate the whole account`() { + // The iCloud shape: principal on one host, home set on another. The + // interceptor withholds the credential from the second host by design, so + // a 401 there must not tell the user to sign in again with credentials + // that just worked. + server.enqueue(homeSetResponse("/dav/one/", "/dav/two/")) + server.enqueue(listing(collection("/dav/one/tasks/", ""))) + server.enqueue(MockResponse().setResponseCode(401)) + + val found = discovery.fromPrincipal(server.url("/principals/me/")) as CalDavDiscovery.Outcome.Found + assertThat(found.collections.map { it.url.encodedPath }).containsExactly("/dav/one/tasks/") + assertThat(found.failedHomeSets).hasSize(1) + assertThat(found.failedHomeSets.single().needsAuthentication).isTrue() + } + + @Test + fun `every home set failing is a server error, not an empty account`() { + // Returning Found with no collections says "connected, no task lists" for + // what is actually a 500 — a bug report nobody can act on. + server.enqueue(homeSetResponse("/dav/one/")) + server.enqueue(MockResponse().setResponseCode(500)) + + assertThat(discovery.fromPrincipal(server.url("/principals/me/"))) + .isInstanceOf(CalDavDiscovery.Outcome.Failed::class.java) + } + + @Test + fun `when every home set wants credentials, it names the hosts to allow`() { + server.enqueue(homeSetResponse("/dav/one/")) + server.enqueue(MockResponse().setResponseCode(401)) + + val outcome = discovery.fromPrincipal(server.url("/principals/me/")) + assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.NeedsAuthentication::class.java) + // Without the host, the caller can never widen the credential allowlist — + // the cross-host home set stays permanently unreachable. + assertThat((outcome as CalDavDiscovery.Outcome.NeedsAuthentication).hosts) + .containsExactly(server.hostName) + } + + @Test + fun `a typed http URL is refused with the scheme as the reason`() { + // Credentials are never sent over cleartext, so this could only ever + // answer "not authorised" — which the user reads as a wrong password. + val outcome = discovery.discover("http://cloud.example.com/dav/") + assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.Failed::class.java) + assertThat((outcome as CalDavDiscovery.Outcome.Failed).reason).contains("http://") + } + + @Test + fun `a colour comes back as packed ARGB, not as a decimal string`() { + val collections = discoverCollections( + listing( + collection( + "/dav/calendars/me/tasks/", + "", + extraProps = """#FF0000FF""", + ), + ), + ) + assertThat(collections.single().color).isEqualTo(0xFFFF0000.toInt()) + } + + @Test + fun `a share is flagged, because Nextcloud rewrites bodies fetched from one`() { + val collections = discoverCollections( + listing( + collection("/dav/calendars/me/shared/", ""), + ), + ) + assertThat(collections.single().isShared).isTrue() + } + + @Test + fun `a principal with no home set is a failure worth naming`() { + server.enqueue(homeSetResponse()) + val outcome = discovery.fromPrincipal(server.url("/principals/me/")) + assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.Failed::class.java) + } + + // --------------------------------------------------- the two filters + + @Test + fun `an absent supported-calendar-component-set means supports everything`() { + // RFC 4791 section 5.2.3 says it SHOULD NOT come back from allprop, so it + // is legitimately missing. Requiring VTODO to be listed silently drops + // every server that does not advertise it. + val collections = discoverCollections( + listing(collection("/dav/calendars/me/tasks/", "")), + ) + assertThat(collections.map { it.url.encodedPath }).containsExactly("/dav/calendars/me/tasks/") + } + + @Test + fun `an empty component set is treated as everything, not as nothing`() { + // The grammar is (comp+) so this is non-conformant, and dav4jvm's parser + // starts all-false — arriving indistinguishable from "no VTODO". + val collections = discoverCollections( + listing( + collection( + "/dav/calendars/me/tasks/", + "", + componentSet = "", + ), + ), + ) + assertThat(collections).hasSize(1) + } + + @Test + fun `a VEVENT-only collection is excluded`() { + val collections = discoverCollections( + listing( + collection( + "/dav/calendars/me/events/", + "", + componentSet = """""", + ), + ), + ) + assertThat(collections).isEmpty() + } + + @Test + fun `SOGo's personal calendar survives, because the test is positive not exclusionary`() { + // SOGo reports collection + calendar + schedule-outbox simultaneously for + // every non-Apple client, i.e. for us. Excluding schedule-outbox — the + // obvious rule — would drop the user's main calendar. + val collections = discoverCollections( + listing( + collection( + "/SOGo/dav/me/Calendar/personal/", + "", + ), + ), + ) + assertThat(collections).hasSize(1) + } + + @Test + fun `a shared calendar is kept`() { + val collections = discoverCollections( + listing( + collection("/dav/calendars/me/shared/", ""), + ), + ) + assertThat(collections).hasSize(1) + } + + @Test + fun `Nextcloud's trashed calendar is dropped, because it strips caldav-calendar`() { + val collections = discoverCollections( + listing( + collection("/dav/calendars/me/deleted/", ""), + ), + ) + assertThat(collections).isEmpty() + } + + @Test + fun `inboxes, outboxes and notification collections are not task lists`() { + val collections = discoverCollections( + listing( + collection("/dav/calendars/me/inbox/", ""), + collection("/dav/calendars/me/outbox/", ""), + collection("/dav/calendars/me/notifications/", ""), + collection("/dav/calendars/me/tasks/", ""), + ), + ) + assertThat(collections.map { it.url.encodedPath }).containsExactly("/dav/calendars/me/tasks/") + } + + // ------------------------------------------------------- privileges + + @Test + fun `an absent privilege set means writable`() { + // RFC 3744 section 3.7 lets a server withhold it, and assuming read-only + // hides collections the user can perfectly well write to. + val collections = discoverCollections( + listing(collection("/dav/calendars/me/tasks/", "")), + ) + assertThat(collections.single().readOnly).isFalse() + } + + @Test + fun `a read-only share is reported as read-only`() { + val collections = discoverCollections( + listing( + collection( + "/dav/calendars/me/shared/", + "", + extraProps = "", + ), + ), + ) + assertThat(collections.single().readOnly).isTrue() + } + + @Test + fun `sync-collection support is reported when advertised`() { + val collections = discoverCollections( + listing( + collection( + "/dav/calendars/me/tasks/", + "", + extraProps = "", + ), + ), + ) + assertThat(collections.single().supportsSyncCollection).isTrue() + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt new file mode 100644 index 0000000..1059e7e --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/NextcloudLoginFlowTest.kt @@ -0,0 +1,169 @@ +package de.jeanlucmakiola.caldav + +import com.google.common.truth.Truth.assertThat +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test + +class NextcloudLoginFlowTest { + + private val server = MockWebServer() + private lateinit var flow: NextcloudLoginFlow + + @Before fun start() { + server.start() + flow = NextcloudLoginFlow(OkHttpClient(), userAgent = "Agendula/1.0 (Pixel 8)") + } + + @After fun stop() = server.shutdown() + + private fun json(body: String) = MockResponse() + .setResponseCode(200) + .setHeader("Content-Type", "application/json; charset=utf-8") + .setBody(body) + + private fun startFlow(): NextcloudLoginFlow.Flow { + server.enqueue( + json( + """ + {"poll":{"token":"tok-123","endpoint":"${server.url("/index.php/login/v2/poll")}"}, + "login":"${server.url("/index.php/login/v2/flow/abc")}"} + """.trimIndent(), + ), + ) + return flow.start(server.url("/"), now = 0).getOrThrow() + } + + @Test + fun `init posts, and carries a User-Agent the user can recognise`() { + startFlow() + val request = server.takeRequest() + assertThat(request.method).isEqualTo("POST") + // The User-Agent becomes the app password's *name* in Settings → Security + // → Devices & sessions. With OkHttp's default the user sees + // "okhttp/4.12.0" and cannot tell what to revoke. + assertThat(request.getHeader("User-Agent")).isEqualTo("Agendula/1.0 (Pixel 8)") + } + + @Test + fun `polling is a POST - a GET gets 405`() { + val started = startFlow() + server.enqueue(MockResponse().setResponseCode(404)) + flow.poll(started, now = 1) + + server.takeRequest() // the init request + val poll = server.takeRequest() + assertThat(poll.method).isEqualTo("POST") + assertThat(poll.body.readUtf8()).contains("token=tok-123") + } + + @Test + fun `404 means pending`() { + val started = startFlow() + server.enqueue(MockResponse().setResponseCode(404)) + assertThat(flow.poll(started, now = 1)).isEqualTo(NextcloudLoginFlow.PollResult.Pending) + } + + @Test + fun `approval yields the app password, and loginName is kept as a username only`() { + val started = startFlow() + server.enqueue( + json("""{"server":"${server.url("/")}","loginName":"Me@Example.COM","appPassword":"secret-app-pw"}"""), + ) + val result = flow.poll(started, now = 1) + assertThat(result).isInstanceOf(NextcloudLoginFlow.PollResult.Approved::class.java) + val credentials = (result as NextcloudLoginFlow.PollResult.Approved).credentials + assertThat(credentials.loginName).isEqualTo("Me@Example.COM") + assertThat(credentials.appPassword).isEqualTo("secret-app-pw") + } + + @Test + fun `429 is rate limiting, not pending`() { + // "Anything that isn't 200 is pending" turns Nextcloud's brute-force + // protection into a twenty-minute spinner. + val started = startFlow() + server.enqueue(MockResponse().setResponseCode(429)) + assertThat(flow.poll(started, now = 1)) + .isInstanceOf(NextcloudLoginFlow.PollResult.Failed::class.java) + } + + @Test + fun `503 is maintenance, not pending`() { + val started = startFlow() + server.enqueue(MockResponse().setResponseCode(503)) + assertThat(flow.poll(started, now = 1)) + .isInstanceOf(NextcloudLoginFlow.PollResult.Failed::class.java) + } + + @Test + fun `a 200 that is not JSON is a captive portal, not an approval`() { + // A Cloudflare challenge is a 200 carrying HTML. + val started = startFlow() + server.enqueue( + MockResponse().setResponseCode(200) + .setHeader("Content-Type", "text/html") + .setBody("Checking your browser…"), + ) + assertThat(flow.poll(started, now = 1)) + .isInstanceOf(NextcloudLoginFlow.PollResult.Failed::class.java) + } + + @Test + fun `the deadline is tracked locally, because 404 also means expired`() { + val started = startFlow() + assertThat(started.deadlineEpochSeconds).isEqualTo(1200L) + // No response enqueued: an expired flow must not even reach the network. + assertThat(flow.poll(started, now = 1201)) + .isInstanceOf(NextcloudLoginFlow.PollResult.Expired::class.java) + } + + @Test + fun `a URL that downgrades to http is refused`() { + // The poll token is exchanged for a long-lived app password, and the login + // URL takes the account password — both are credential-grade. + val https = "https://cloud.example.com/".toHttpUrl() + val http = "http://cloud.example.com/poll".toHttpUrl() + val failure = runCatching { flow.requireSecureOrigin(https, http) }.exceptionOrNull() + assertThat(failure).isNotNull() + assertThat(failure!!.message).contains("http://") + } + + @Test + fun `the browser login URL gets the same scheme check as the poll endpoint`() { + // It is where the user types their *account* password, so leaving it + // unchecked is the more dangerous of the two omissions. `start` applies + // requireSecureOrigin to both; this asserts the rule itself, because + // reaching the branch over the wire would need a TLS MockWebServer and a + // test that merely fails to connect would pass for the wrong reason. + val typed = "https://cloud.example.com/".toHttpUrl() + val harvester = "http://evil.example.com/harvest".toHttpUrl() + val failure = runCatching { flow.requireSecureOrigin(typed, harvester) }.exceptionOrNull() + assertThat(failure).isNotNull() + assertThat(failure!!.message).contains("evil.example.com") + } + + @Test + fun `a host change is carried, not refused - reverse proxies are legitimate`() { + // overwrite.cli.url pointing somewhere other than what the user typed is + // ordinary on self-hosted installs. Throwing would make the flow unusable + // for them; the UI confirms it instead. + server.enqueue( + json( + """ + {"poll":{"token":"t","endpoint":"https://cloud.example.com/poll"}, + "login":"https://cloud.example.com/flow"} + """.trimIndent(), + ), + ) + val started = flow.start(server.url("/"), now = 0) + assertThat(started.isSuccess).isTrue() + val mismatch = started.getOrThrow().hostMismatch + assertThat(mismatch).isNotNull() + assertThat(mismatch!!.actual).isEqualTo("cloud.example.com") + assertThat(mismatch.message).contains("overwrite.cli.url") + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt new file mode 100644 index 0000000..1c736d5 --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt @@ -0,0 +1,102 @@ +package de.jeanlucmakiola.caldav + +import com.google.common.truth.Truth.assertThat +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Before +import org.junit.Test + +class PreemptiveBasicInterceptorTest { + + private val server = MockWebServer() + + @Before fun start() = server.start() + @After fun stop() = server.shutdown() + + private fun clientFor(allowedHosts: Set) = OkHttpClient.Builder() + .addInterceptor(PreemptiveBasicInterceptor("user", "pw", allowedHosts)) + .build() + + private fun authHeaderOf(client: OkHttpClient): String? { + server.enqueue(MockResponse().setResponseCode(200)) + client.newCall(Request.Builder().url(server.url("/dav/")).build()).execute().close() + return server.takeRequest().getHeader("Authorization") + } + + @Test + fun `plain HTTP never carries the credential, whatever the host list says`() { + // MockWebServer is HTTP, so this also documents why the happy path below + // is tested through the interceptor directly rather than over the wire. + assertThat(authHeaderOf(clientFor(setOf(server.hostName)))).isNull() + } + + @Test + fun `a host outside the account's origin gets nothing`() { + assertThat(authHeaderOf(clientFor(setOf("someone-else.example.com")))).isNull() + } + + @Test + fun `the credential is attached up front on an allowed HTTPS host`() { + // OkHttp's Authenticator is reactive-only: it costs an extra round trip on + // every request of a PROPFIND-heavy sync, and never fires at all against a + // server that answers 403 or 404 without a challenge. + val interceptor = PreemptiveBasicInterceptor("user", "pw", setOf("cloud.example.com")) + val request = Request.Builder().url("https://cloud.example.com/dav/").build() + val chain = FakeChain(request) + + interceptor.intercept(chain) + + assertThat(chain.proceeded!!.header("Authorization")).isEqualTo("Basic dXNlcjpwdw==") + } + + @Test + fun `an existing Authorization header is never overwritten`() { + val interceptor = PreemptiveBasicInterceptor("user", "pw", setOf("cloud.example.com")) + val request = Request.Builder() + .url("https://cloud.example.com/dav/") + .header("Authorization", "Digest something") + .build() + val chain = FakeChain(request) + + interceptor.intercept(chain) + + assertThat(chain.proceeded!!.header("Authorization")).isEqualTo("Digest something") + } + + @Test + fun `a cross-host home set does not silently receive the credential`() { + // OkHttp strips Authorization across hosts on purpose. Re-attaching it is + // something we do knowingly, per validated host — never blanket. + val interceptor = PreemptiveBasicInterceptor("user", "pw", setOf("caldav.icloud.com")) + val chain = FakeChain(Request.Builder().url("https://p42-caldav.icloud.com/dav/").build()) + + interceptor.intercept(chain) + + assertThat(chain.proceeded!!.header("Authorization")).isNull() + } + + private class FakeChain(private val request: Request) : okhttp3.Interceptor.Chain { + var proceeded: Request? = null + override fun request() = request + override fun proceed(request: Request): okhttp3.Response { + proceeded = request + return okhttp3.Response.Builder() + .request(request) + .protocol(okhttp3.Protocol.HTTP_1_1) + .code(200) + .message("OK") + .build() + } + override fun connection() = null + override fun call() = throw UnsupportedOperationException() + override fun connectTimeoutMillis() = 0 + override fun withConnectTimeout(timeout: Int, unit: java.util.concurrent.TimeUnit) = this + override fun readTimeoutMillis() = 0 + override fun withReadTimeout(timeout: Int, unit: java.util.concurrent.TimeUnit) = this + override fun writeTimeoutMillis() = 0 + override fun withWriteTimeout(timeout: Int, unit: java.util.concurrent.TimeUnit) = this + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt new file mode 100644 index 0000000..283544c --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt @@ -0,0 +1,36 @@ +package de.jeanlucmakiola.caldav + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class ServerQuirksTest { + + @Test + fun `the three providers whose real error is not wrong password`() { + assertThat(ServerQuirk.forInput("me@fastmail.com")) + .isEqualTo(ServerQuirk.FASTMAIL_APP_PASSWORD) + assertThat(ServerQuirk.forInput("me@icloud.com")) + .isEqualTo(ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD) + assertThat(ServerQuirk.forInput("me@gmail.com")) + .isEqualTo(ServerQuirk.GOOGLE_UNSUPPORTED) + } + + @Test + fun `Google is fatal - it supports neither VTODO nor MKCALENDAR`() { + assertThat(ServerQuirk.GOOGLE_UNSUPPORTED.isFatal).isTrue() + assertThat(ServerQuirk.FASTMAIL_APP_PASSWORD.isFatal).isFalse() + } + + @Test + fun `subdomains count, and a lookalike domain does not`() { + assertThat(ServerQuirk.forHost("mail.icloud.com")).isEqualTo(ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD) + assertThat(ServerQuirk.forHost("noticloud.com")).isNull() + assertThat(ServerQuirk.forHost("icloud.com.example.org")).isNull() + } + + @Test + fun `a self-hosted server has no quirk`() { + assertThat(ServerQuirk.forInput("https://cloud.example.de/remote.php/dav/")).isNull() + assertThat(ServerQuirk.forInput("me@example.de")).isNull() + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServiceDiscoveryTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServiceDiscoveryTest.kt new file mode 100644 index 0000000..67b807a --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServiceDiscoveryTest.kt @@ -0,0 +1,128 @@ +package de.jeanlucmakiola.caldav + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +/** + * `docs/SYNC.md`'s discovery trap table, made executable. Every case here was + * live-probed against a real provider; each one breaks the obvious + * implementation. + */ +class ServiceDiscoveryTest { + + private class FakeDns( + val srv: Map> = emptyMap(), + val txt: Map> = emptyMap(), + ) : DnsResolver { + override fun srv(name: String) = srv[name].orEmpty() + override fun txt(name: String) = txt[name].orEmpty() + } + + private fun urls(input: String, dns: DnsResolver = DnsResolver.None) = + ServiceDiscovery.candidatesFor(input, dns).map { it.url.toString() } + + @Test + fun `a typed base URL is used as typed and never triggers DNS`() { + val dns = FakeDns(srv = mapOf("_caldavs._tcp.example.com" to listOf(SrvRecord(0, 0, 8443, "dav.example.com")))) + assertThat(urls("https://cloud.example.com/remote.php/dav/", dns)) + .containsExactly("https://cloud.example.com/remote.php/dav/") + } + + @Test + fun `the well-known ladder ends at the root, not at well-known`() { + // The draft stopped at /.well-known/caldav; a server that 404s there and + // serves DAV from / would be undiscoverable. + assertThat(urls("me@example.com")).containsExactly( + "https://example.com/.well-known/caldav", + "https://example.com/", + ).inOrder() + } + + @Test + fun `Posteo is SRV-only on port 8443, and its well-known 404s`() { + val dns = FakeDns( + srv = mapOf("_caldavs._tcp.posteo.de" to listOf(SrvRecord(0, 0, 8443, "posteo.de."))), + txt = mapOf("_caldavs._tcp.posteo.de" to listOf("path=/")), + ) + // Hardcoding 443 fails Posteo outright. + assertThat(urls("me@posteo.de", dns)).containsExactly( + "https://posteo.de:8443/", + "https://posteo.de:8443/.well-known/caldav", + ).inOrder() + } + + @Test + fun `GMX publishes a TXT path that discovery cannot skip`() { + val dns = FakeDns( + txt = mapOf("_caldavs._tcp.gmx.net" to listOf("path=/begenda/dav/users/")), + ) + // Skip the TXT lookup and GMX lands on the wrong path, finding nothing. + assertThat(urls("me@gmx.net", dns)).contains("https://gmx.net/begenda/dav/users/") + assertThat(urls("me@gmx.net", dns).first()).isEqualTo("https://gmx.net/begenda/dav/users/") + } + + @Test + fun `a null SRV target means explicitly unavailable, not no record`() { + // RFC 2782. _caldav._tcp.fastmail.com and runbox.com both answer `0 0 0 .` + val dns = FakeDns(srv = mapOf("_caldavs._tcp.runbox.com" to listOf(SrvRecord(0, 0, 0, ".")))) + assertThat(urls("me@runbox.com", dns)).containsExactly( + "https://runbox.com/.well-known/caldav", + "https://runbox.com/", + ).inOrder() + } + + @Test + fun `Google's SRV record points at something that is not a DAV server`() { + // calendar.google.com answers 405 to PROPFIND. A strict RFC 6764 client + // follows it into a dead end for every @gmail.com address. + val dns = FakeDns( + srv = mapOf("_caldavs._tcp.gmail.com" to listOf(SrvRecord(5, 0, 443, "calendar.google.com."))), + ) + assertThat(urls("me@gmail.com", dns)).doesNotContain("https://calendar.google.com/") + } + + @Test + fun `SRV priority wins, then weight`() { + val dns = FakeDns( + srv = mapOf( + "_caldavs._tcp.example.com" to listOf( + SrvRecord(priority = 20, weight = 0, port = 443, target = "backup.example.com"), + SrvRecord(priority = 10, weight = 5, port = 443, target = "low.example.com"), + SrvRecord(priority = 10, weight = 90, port = 443, target = "main.example.com"), + ), + ), + ) + assertThat(urls("me@example.com", dns).first()).startsWith("https://main.example.com/") + assertThat(urls("me@example.com", dns)).containsAtLeast( + "https://main.example.com/.well-known/caldav", + "https://low.example.com/.well-known/caldav", + "https://backup.example.com/.well-known/caldav", + ).inOrder() + } + + @Test + fun `port 443 stays implicit so URLs compare equal`() { + val dns = FakeDns(srv = mapOf("_caldavs._tcp.example.com" to listOf(SrvRecord(0, 0, 443, "dav.example.com")))) + assertThat(urls("me@example.com", dns).first()).isEqualTo("https://dav.example.com/.well-known/caldav") + } + + @Test + fun `mailto and a bare domain both resolve`() { + assertThat(ServiceDiscovery.domainOf("mailto:me@example.com")).isEqualTo("example.com") + assertThat(ServiceDiscovery.domainOf("me@example.com")).isEqualTo("example.com") + assertThat(ServiceDiscovery.domainOf("example.com")).isEqualTo("example.com") + } + + @Test + fun `something that is neither an address nor a URL yields nothing`() { + assertThat(ServiceDiscovery.candidatesFor("hello")).isEmpty() + assertThat(ServiceDiscovery.candidatesFor("")).isEmpty() + } + + @Test + fun `every candidate carries where it came from`() { + val dns = FakeDns(txt = mapOf("_caldavs._tcp.gmx.net" to listOf("path=/begenda/dav/users/"))) + assertThat(ServiceDiscovery.candidatesFor("me@gmx.net", dns).map { it.origin }) + .contains("domain as typed + TXT path=/begenda/dav/users/") + } +} diff --git a/dav/PROVENANCE.md b/dav/PROVENANCE.md index 8e3f514..dc0dd17 100644 --- a/dav/PROVENANCE.md +++ b/dav/PROVENANCE.md @@ -107,6 +107,20 @@ through its `Calendar`, and any caller could `setTimeZone` on it and undo change for everyone else. It is now private and built per call. Nothing else in the tree referenced it. +### 6. `` is parsed instead of inferred + +`CurrentUserPrincipal.Factory` read only the `` child, so an +unauthenticated body (RFC 5397 §3 — a **200** whose content means the request was +not authenticated) arrived as "property present, href null" — identical to a +conformant-but-empty element, and to a server that omits the property entirely. +A caller inferring rejection from the null href therefore also fires on merely +non-conformant servers, and on a request that carried no credential at all. + +The factory now makes one pass over both children (`XmlUtils.processTag` consumes +to the end tag and so cannot be called twice) and reports `unauthenticated` +explicitly. Without it, a rejected credential is indistinguishable from a +successful discovery that found nothing. + --- ## Build integration diff --git a/dav/src/main/kotlin/at/bitfire/dav4jvm/property/CurrentUserPrincipal.kt b/dav/src/main/kotlin/at/bitfire/dav4jvm/property/CurrentUserPrincipal.kt index 460831b..0ee5fab 100644 --- a/dav/src/main/kotlin/at/bitfire/dav4jvm/property/CurrentUserPrincipal.kt +++ b/dav/src/main/kotlin/at/bitfire/dav4jvm/property/CurrentUserPrincipal.kt @@ -10,17 +10,29 @@ import at.bitfire.dav4jvm.DavResource import at.bitfire.dav4jvm.Property import at.bitfire.dav4jvm.PropertyFactory import at.bitfire.dav4jvm.XmlUtils +import at.bitfire.dav4jvm.XmlUtils.propertyName import org.xmlpull.v1.XmlPullParser // see RFC 5397: WebDAV Current Principal Extension data class CurrentUserPrincipal( - val href: String? + val href: String?, + /** + * RFC 5397 §3: the server answered 200 but the request was **not** + * authenticated. Upstream parsed only the `` child, so this arrived + * indistinguishable from a conformant-but-empty element and from a server + * that simply omits the property — and a rejected credential then looks + * like a successful discovery that found nothing. Local addition; see + * dav/PROVENANCE.md change 6. + */ + val unauthenticated: Boolean = false ): Property { companion object { @JvmField val NAME = Property.Name(XmlUtils.NS_WEBDAV, "current-user-principal") + + val UNAUTHENTICATED = Property.Name(XmlUtils.NS_WEBDAV, "unauthenticated") } @@ -30,11 +42,23 @@ data class CurrentUserPrincipal( override fun create(parser: XmlPullParser): CurrentUserPrincipal { // + // One pass over both children: XmlUtils.processTag consumes to the end + // tag, so it cannot be called twice on the same element. var href: String? = null - XmlUtils.processTag(parser, DavResource.HREF) { - href = XmlUtils.readText(parser) + var unauthenticated = false + + val depth = parser.depth + var eventType = parser.eventType + while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && + parser.depth == depth)) { + if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1) + when (parser.propertyName()) { + DavResource.HREF -> href = XmlUtils.readText(parser) + UNAUTHENTICATED -> unauthenticated = true + } + eventType = parser.next() } - return CurrentUserPrincipal(href) + return CurrentUserPrincipal(href, unauthenticated) } } diff --git a/dav/src/test/kotlin/at/bitfire/dav4jvm/LocalChangesTest.kt b/dav/src/test/kotlin/at/bitfire/dav4jvm/LocalChangesTest.kt index 35992ed..0907b2d 100644 --- a/dav/src/test/kotlin/at/bitfire/dav4jvm/LocalChangesTest.kt +++ b/dav/src/test/kotlin/at/bitfire/dav4jvm/LocalChangesTest.kt @@ -6,11 +6,13 @@ package at.bitfire.dav4jvm +import at.bitfire.dav4jvm.property.CurrentUserPrincipal import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before @@ -121,6 +123,50 @@ class LocalChangesTest { assertEquals(mockServer.url("/other/"), resource.location) } + // -- change 6: is parsed, not inferred --------------- + + private fun principalBody(inner: String) = MockResponse() + .setResponseCode(207) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody( + """ + /dav/ + $inner + HTTP/1.1 200 OK + + """.trimIndent(), + ) + + private fun principalOf(inner: String): CurrentUserPrincipal? { + mockServer.enqueue(principalBody(inner)) + var found: CurrentUserPrincipal? = null + DavResource(httpClient, mockServer.url("/dav/")) + .propfind(0, CurrentUserPrincipal.NAME) { response, _ -> + found = response[CurrentUserPrincipal::class.java] + } + return found + } + + @Test + fun unauthenticatedIsDistinguishableFromAMerelyEmptyElement() { + // RFC 5397 §3: a 200 whose body says the request was not authenticated. + // Upstream parsed only , so this arrived identical to an empty + // element and to a server that omits the property — and a rejected + // credential then looks like a discovery that simply found nothing. + assertTrue(principalOf("")!!.unauthenticated) + + val empty = principalOf("") + assertNull(empty!!.href) + assertFalse(empty.unauthenticated) + } + + @Test + fun anOrdinaryPrincipalHrefStillParses() { + val principal = principalOf("/principals/me/") + assertEquals("/principals/me/", principal!!.href) + assertFalse(principal.unauthenticated) + } + // -- change 1: commons-lang3 removed from date parsing -------------------- /** diff --git a/docs/SYNC-PLAN.md b/docs/SYNC-PLAN.md index 725ffe6..84d86a6 100644 --- a/docs/SYNC-PLAN.md +++ b/docs/SYNC-PLAN.md @@ -57,7 +57,8 @@ Nextcloud/Radicale/Baïkal in chunk 5. |---|---|---|---| | 1 | **Mapper** — VTODO ↔ Room | nothing | Unknown properties survive a read-modify-write cycle | | 2a | **Vendored DAV foundation** | nothing | Vendor a tree we can actually maintain, and know which of its defects are real | -| 2b | **Auth, discovery, account** | 2a | Never send a credential into an unvalidated redirect chain | +| 2b | **Discovery and auth protocol** | 2a | The two collection filters are inversions of the obvious rule | +| 2c | **The Android account layer** | 2b | Never send a credential into an unvalidated redirect chain | | 3 | **Engine core** — read, write, conflicts | 1, 2 | A failed resource must not fail the collection | | 4 | **Incremental sync + scheduling** | 3 | Never persist a sync token before the bodies it covers are applied | | 5 | **Hardening, compliance, real servers** | 1–4 | Ship no licence violation, and no retry loop on a dead app password | @@ -200,7 +201,32 @@ runtime dependency are gone, and `:app` links against the module. --- -## Chunk 2b — auth, discovery, account +## Chunk 2b — the discovery and auth protocol + +⚠️ **Split again while building it.** The seam is the platform: everything here +is protocol, testable on a JVM against MockWebServer, and none of it touches +Android. Keystore, `AccountManager`, Custom Tabs and the account-add UI are a +different kind of work with a different kind of verification — including the +on-device review — so they are chunk 2c. + +Lands in a new `:caldav` module: MIT, plain JVM, `api`-depending on `:dav`. A +separate module from the vendored MPL tree keeps the licences unmixed and makes +"no Android types" a compile-time guarantee rather than a discipline. + +**One dependency added:** `dnsjava` 3.6.3 (BSD-3) for the SRV/TXT half of +RFC 6764. Android's own `DnsResolver` is callback-only and cannot do the +`TXT path=` lookup, and JNDI's DNS provider does not exist on Android at all. +It is what DAVx5 uses. Behind a `DnsResolver` interface so the pipeline is +testable without a network. + +**Done when:** the trap table is executable — Posteo's SRV-only 8443, GMX's +`TXT path=`, null SRV targets, Google's dead-end record, SOGo's triple +resourcetype, Nextcloud's trashed calendar, the absent/empty component set, and +`` as a 200 that means failure. + +--- + +## Chunk 2c — the Android account layer **Goal:** the app can add a CalDAV account and list its VTODO collections. No syncing yet. @@ -466,10 +492,11 @@ attribution screen ships, and a `releaseTest` build syncs on a real device. ## Definition of done, every chunk 1. It builds: `./gradlew :app:assembleDebug`. -2. Unit tests pass: `./gradlew :app:testDebugUnitTest :dav:test`. ⚠️ Name - `:dav:test` explicitly — `testDebugUnitTest` is an Android-variant task and - does not exist on a plain JVM module, so the vendored suite would otherwise be - compiled by nobody and run by nobody. +2. Unit tests pass: + `./gradlew :app:testDebugUnitTest :dav:test :caldav:test`. ⚠️ Name the plain + JVM modules explicitly — `testDebugUnitTest` is an Android-variant task and + does not exist on them, so their suites would otherwise be compiled by nobody + and run by nobody. Add each new module here **and** to CI. 3. Lint passes: `./gradlew :app:lintDebug`. Added after chunk 1 shipped a literal byte-order mark that builds and tests both accepted and CI's lint step would have rejected. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 291f87e..76da49e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -43,6 +43,13 @@ okhttp = "4.12.0" # testImplementation and never reaches the APK. xpp3 = "1.1.6" +# --- :caldav (our CalDAV protocol layer) ------------------------------------- +# SRV/TXT lookups for RFC 6764 discovery. Android's resolver exposes no SRV API +# below API 29's DnsResolver (which is callback-only and cannot do TXT paths), +# and JNDI's DNS provider does not exist on Android at all. dnsjava is what DAVx5 +# uses. BSD-3 — it goes on the attribution screen with the rest. +dnsjava = "3.6.3" + # --- :provider (vendored dmfs task provider) --------------------------------- # Versions the upstream 1.4.2 source was written against. These are its runtime # dependencies, not ours — nothing above the data layer touches them, and they @@ -140,6 +147,9 @@ okhttp = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okhtt okhttp-mockwebserver = { group = "com.squareup.okhttp3", name = "mockwebserver", version.ref = "okhttp" } xpp3 = { group = "org.ogce", name = "xpp3", version.ref = "xpp3" } +# :caldav — RFC 6764 service discovery +dnsjava = { group = "dnsjava", name = "dnsjava", version.ref = "dnsjava" } + # :provider — vendored dmfs task provider (see provider/PROVENANCE.md) dmfs-jems = { group = "org.dmfs", name = "jems", version.ref = "dmfsJems" } dmfs-jems-testing = { group = "org.dmfs", name = "jems-testing", version.ref = "dmfsJems" } diff --git a/settings.gradle.kts b/settings.gradle.kts index c9c782f..ae7dfb8 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -27,4 +27,5 @@ dependencyResolutionManagement { rootProject.name = "Agendula" include(":app") include(":dav") +include(":caldav") includeBuild("floret-kit")