diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/MainActivity.kt b/app/src/main/java/de/jeanlucmakiola/agendula/MainActivity.kt index 99ff240..c8c044e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/MainActivity.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/MainActivity.kt @@ -79,7 +79,7 @@ class MainActivity : ComponentActivity() { lifecycleScope.launch { runCatching { accounts.rescheduleAll() - accounts.all().forEach { syncTrigger.enqueue(it.displayName) } + accounts.syncable().forEach { syncTrigger.enqueue(it.displayName) } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt index d1f44cb..b00b0fc 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt @@ -8,8 +8,11 @@ import de.jeanlucmakiola.caldav.CalDavDiscovery import de.jeanlucmakiola.caldav.TaskCollection import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import javax.inject.Inject import javax.inject.Singleton +import kotlin.time.Duration.Companion.seconds /** * The one thing the sign-in flow needs from [AccountRepository]. @@ -45,6 +48,8 @@ class AccountRepository @Inject constructor( private val accounts: CalDavAccounts, private val syncTrigger: SyncTrigger, private val cadence: SyncCadenceStore, + private val accountState: AccountStateStore, + private val gateway: CalDavGateway, @IoDispatcher private val io: CoroutineDispatcher, ) : AccountCreator { @@ -77,8 +82,20 @@ class AccountRepository @Inject constructor( // disappears, so "removed from system Settings, re-added here" would // otherwise leave a second Room row and a duplicate of every list. val existsInSystem = accounts.find(displayName) != null - val existsInRoom = database.accounts().all().any { it.displayName == displayName } - if (existsInSystem || existsInRoom) return@withContext Outcome.AlreadyExists + val existing = database.accounts().all().firstOrNull { it.displayName == displayName } + + // ⚠️ Re-authentication, not a duplicate. An account stopped by a 401 has + // no other way back: `create` is the only path that writes a credential, + // and refusing it here left the user with "that account is already set + // up" and no option but to remove the account — discarding the choice to + // keep its lists attached. Narrow on purpose: a *healthy* account of the + // same name is still a duplicate, so this can never silently overwrite a + // working credential. + if (existing != null && accountState.needsSignIn(existing.id)) { + return@withContext reauthenticate(existing.id, displayName, appPassword) + } + + if (existsInSystem || existing != null) return@withContext Outcome.AlreadyExists val accountId = database.runInTransaction { val id = database.accounts().insert( @@ -136,6 +153,27 @@ class AccountRepository @Inject constructor( Outcome.Created(accountId) } + /** + * Replaces the credential of an account the server had stopped accepting. + * + * The lists, their sync tokens and every task stay exactly as they are — + * the password was the only thing that went stale. + */ + private suspend fun reauthenticate( + accountId: Long, + displayName: String, + appPassword: String, + ): Outcome { + if (!credentials.put(accountId, appPassword)) { + return Outcome.CredentialFailed("the device keystore would not store the password") + } + accountState.setNeedsSignIn(accountId, false) + database.accounts().recordSync(accountId, at = null, error = null) + syncTrigger.schedule(displayName) + syncTrigger.enqueue(displayName) + return Outcome.Created(accountId) + } + /** * Undoes a half-made account. * @@ -158,7 +196,28 @@ class AccountRepository @Inject constructor( * restore onto a device that never ran the account-add flow. */ suspend fun rescheduleAll() = withContext(io) { - database.accounts().all().forEach { syncTrigger.schedule(it.displayName) } + val stopped = accountState.needingSignIn() + database.accounts().all().forEach { account -> + // ⚠️ A stopped account must not come back on the timer. `KEEP` only + // keeps work that is unfinished, and CANCELLED counts as finished — + // so rescheduling would re-enqueue the very request a 401 removed, + // and the next app open would put a dead app password back on a + // four-hour loop against a server that throttles by IP. + // + // This is also where the cancellation happens at all: the engine + // cannot cancel from inside the worker it is running in. + if (account.id in stopped) { + syncTrigger.cancel(account.displayName) + } else { + syncTrigger.schedule(account.displayName) + } + } + } + + /** The accounts a caller may sync right now — stopped ones excluded. */ + suspend fun syncable(): List = withContext(io) { + val stopped = accountState.needingSignIn() + database.accounts().all().filterNot { it.id in stopped } } /** @@ -168,21 +227,60 @@ class AccountRepository @Inject constructor( * NULL`, so they become device-only lists. Removing an account is not an * instruction to destroy the tasks it held. */ - suspend fun remove(accountId: Long, displayName: String) = withContext(io) { - syncTrigger.cancel(displayName) - // The lists survive as device-only lists, so their cursors must not: a - // re-added account would otherwise inherit a "reconciled recently" that - // was true of a different account's data. - cadence.forget( - database.taskLists().syncedForAccount(accountId).map { it.id }.toSet(), - ) - credentials.clear(accountId) - database.accounts().delete(accountId) - accounts.find(displayName)?.let { accounts.remove(it) } + suspend fun remove(accountId: Long, displayName: String, deleteLocalData: Boolean = false) = + withContext(io) { + syncTrigger.cancel(displayName) + revokeAppPassword(accountId) + // The lists survive as device-only lists, so their cursors must not: + // a re-added account would otherwise inherit a "reconciled recently" + // that was true of a different account's data. + cadence.forget( + database.taskLists().syncedForAccount(accountId).map { it.id }.toSet(), + ) + // Play's Account Deletion policy does not apply to us — there is no + // Agendula account to delete — but "I want it gone from this device + // too" is a reasonable thing to want, and it is the only way to get + // the tasks off the device without also uninstalling. + if (deleteLocalData) database.taskLists().deleteForAccount(accountId) + + accountState.setNeedsSignIn(accountId, false) + credentials.clear(accountId) + database.accounts().delete(accountId) + accounts.find(displayName)?.let { accounts.remove(it) } + } + + /** + * Best effort, and before the credential is cleared — it is the credential. + * + * A failure here is never allowed to stop the removal: the user asked for the + * account to go, and a server that is unreachable, or was never a Nextcloud, + * is not a reason to keep it. + */ + private suspend fun revokeAppPassword(accountId: Long) { + val account = database.accounts().account(accountId) ?: return + val username = account.username ?: return + val origin = account.principalUrl?.toHttpUrlOrNull() ?: return + val password = (credentials.get(accountId) as? CredentialStore.Secret.Present)?.value + ?: return + runCatching { + // ⚠️ Bounded well below the shared client's 30 s connect / 120 s read. + // The user pressed "remove"; an unreachable server must not leave the + // account sitting on screen for half a minute with nothing happening. + // Losing the revocation is the lesser failure, and it is the one the + // user can still fix by hand on the server. + withTimeoutOrNull(REVOCATION_TIMEOUT) { + gateway.revokeAppPassword( + CalDavGateway.Credentials(username, password, origin), + ) + } + } } private companion object { /** M3 primary-ish blue; the user recolours a list from its own screen. */ const val DEFAULT_LIST_COLOR = 0xFF4C6FFF.toInt() + + /** Long enough for a reachable server, short enough not to feel stuck. */ + val REVOCATION_TIMEOUT = 5.seconds } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountStateStore.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountStateStore.kt new file mode 100644 index 0000000..f946ffb --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/AccountStateStore.kt @@ -0,0 +1,47 @@ +package de.jeanlucmakiola.agendula.data.sync + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringSetPreferencesKey +import de.jeanlucmakiola.agendula.data.di.SyncStateDataStore +import kotlinx.coroutines.flow.first +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Which accounts the server has stopped accepting. + * + * Separate from `accounts.last_sync_error` because the two mean different + * things to the user and to the engine: an error is "this did not work, we will + * try again", while this is "**we have stopped trying** and only you can change + * that". Conflating them is how a client ends up retrying a revoked app password + * on a timer. + * + * Lives with the other per-device sync state, and is therefore excluded from + * backup — see [SyncStateDataStore]. That is also correct on its own terms: the + * credential does not survive a restore either, so a restored "needs sign-in" is + * at best redundant and at worst stale. + */ +@Singleton +class AccountStateStore @Inject constructor( + @SyncStateDataStore private val dataStore: DataStore, +) { + + suspend fun needingSignIn(): Set = + dataStore.data.first()[KEY].orEmpty().mapNotNull { it.toLongOrNull() }.toSet() + + suspend fun needsSignIn(accountId: Long): Boolean = accountId in needingSignIn() + + suspend fun setNeedsSignIn(accountId: Long, needed: Boolean) { + dataStore.edit { prefs -> + val current = prefs[KEY].orEmpty().toMutableSet() + if (needed) current += accountId.toString() else current -= accountId.toString() + prefs[KEY] = current + } + } + + private companion object { + val KEY = stringSetPreferencesKey("accounts_needing_sign_in") + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavGateway.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavGateway.kt index aa030ff..eccb87c 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavGateway.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavGateway.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.agendula.data.sync import de.jeanlucmakiola.agendula.data.di.IoDispatcher +import de.jeanlucmakiola.caldav.AppPassword import de.jeanlucmakiola.caldav.CalDavDiscovery import de.jeanlucmakiola.caldav.CalDavHttp import de.jeanlucmakiola.caldav.DnsJavaResolver @@ -30,6 +31,14 @@ interface CalDavGateway { suspend fun pollLoginFlow(flow: NextcloudLoginFlow.Flow): NextcloudLoginFlow.PollResult + /** + * Hands an app password back to the server, best effort. + * + * Without it, uninstalling never revokes anything — the credential we minted + * outlives the app in the user's device list. + */ + suspend fun revokeAppPassword(credentials: Credentials): Boolean + /** Credentials, and the origin whose registrable domain they are scoped to. */ data class Credentials(val username: String, val password: String, val origin: HttpUrl) } @@ -69,5 +78,14 @@ class OkHttpCalDavGateway @Inject constructor( NextcloudLoginFlow(CalDavHttp.anonymous(userAgent), userAgent).poll(flow, now()) } + override suspend fun revokeAppPassword( + credentials: CalDavGateway.Credentials, + ): Boolean = withContext(io) { + val client = CalDavHttp.authenticated( + userAgent, credentials.username, credentials.password, credentials.origin, + ) + AppPassword.revoke(client, credentials.origin) + } + private fun now() = System.currentTimeMillis() / 1000 } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt index 03b7793..d61742e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt @@ -5,6 +5,7 @@ import de.jeanlucmakiola.agendula.data.tasks.ical.ResourceValidator import de.jeanlucmakiola.agendula.data.tasks.ical.VTodoMapper import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity +import at.bitfire.dav4jvm.exception.UnauthorizedException import de.jeanlucmakiola.caldav.ChangeSet import de.jeanlucmakiola.caldav.DeleteOutcome import de.jeanlucmakiola.caldav.PutOutcome @@ -109,7 +110,10 @@ class CollectionSyncer( fun execute(): SyncReport { val state = remote.state().getOrElse { - return report.copy(failure = "collection unavailable: $it") + return report.copy( + failure = "collection unavailable: $it", + authFailure = it is UnauthorizedException, + ) } writable = !state.collection.readOnly shared = state.collection.isShared @@ -162,7 +166,10 @@ class CollectionSyncer( */ private fun runFull() { val refs = remote.list().getOrElse { - report = report.copy(failure = "listing failed: $it") + report = report.copy( + failure = "listing failed: $it", + authFailure = it is UnauthorizedException, + ) return } // ⚠️ Only strong tags are carried forward. A weak one cannot serve @@ -216,7 +223,12 @@ class CollectionSyncer( applyRemovals(page.removed) downloadChanged(page.changed) - // ⚠️ After the bodies, never before. + // ⚠️ After the bodies, and only if they actually landed. A + // network drop mid-multiget would otherwise commit a cursor past + // changes that were never downloaded — a permanent hole in the + // collection, invisible until the next full reconciliation a day + // later. `runFull` has always had this guard; this path did not. + if (report.failure != null) return false store.setSyncToken(list.id, page.token) val next = page.token @@ -545,6 +557,12 @@ class CollectionSyncer( return } fetched.resources.forEach { apply(it, index) } + // ⚠️ Listed by the query, no body from the multiget. It is in the + // listing, so the sweep leaves it alone, and it never reaches + // `apply`, so nothing counts it — without this it is re-requested + // on every sync for ever, which is the loop quarantine exists to + // break. + fetched.missing.forEach { fail(it.toString(), "listed but not returned") } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncEngine.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncEngine.kt index 3cc8ff9..5998cd6 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncEngine.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncEngine.kt @@ -28,6 +28,7 @@ class SyncEngine @Inject constructor( private val credentials: CredentialStore, private val quarantine: QuarantineStore, private val cadence: SyncCadenceStore, + private val accountState: AccountStateStore, @IoDispatcher private val io: CoroutineDispatcher, ) { @@ -45,6 +46,15 @@ class SyncEngine @Inject constructor( val account = database.accounts().all().firstOrNull { it.displayName == accountName } ?: return@withContext Result.Misconfigured("no such account: $accountName") + // ⚠️ Before anything reaches the network. A periodic request that outlives + // the stop — or a manual trigger on a stopped account — must not spend a + // request on a credential we already know the server rejects: Nextcloud + // throttles then 429s per source IP, and that lands on the user's other + // clients rather than on us. + if (accountState.needsSignIn(account.id)) { + return@withContext Result.NeedsSignIn("waiting for you to sign in again") + } + val username = account.username ?: return@withContext fatal(account.id, Result.Misconfigured("account has no username")) val origin = account.principalUrl?.toHttpUrlOrNull() @@ -55,14 +65,30 @@ class SyncEngine @Inject constructor( val password = when (val secret = credentials.get(account.id)) { is CredentialStore.Secret.Present -> secret.value - CredentialStore.Secret.Absent -> - return@withContext fatal(account.id, Result.NeedsSignIn("no stored password")) - is CredentialStore.Secret.Unrecoverable -> - return@withContext fatal(account.id, Result.NeedsSignIn(secret.reason)) + CredentialStore.Secret.Absent -> { + stopForSignIn(account.id, "no stored password") + return@withContext Result.NeedsSignIn("no stored password") + } + is CredentialStore.Secret.Unrecoverable -> { + stopForSignIn(account.id, secret.reason) + return@withContext Result.NeedsSignIn(secret.reason) + } } val client = CalDavHttp.authenticated(USER_AGENT, username, password, origin) val reports = syncCollections(account) { url -> CalendarCollection(client, url) } + + if (reports.any { it.authFailure }) { + // ⚠️ Stop the account rather than let the schedule keep trying. + // Nextcloud throttles and then 429s **per source IP**, so a timer on a + // dead app password degrades every other Nextcloud client on the + // user's network — and there is nothing here to retry: the fix is a + // sign-in only the user can perform. + stopForSignIn(account.id, "the server rejected the credentials") + return@withContext Result.NeedsSignIn("the server rejected the credentials") + } + + accountState.setNeedsSignIn(account.id, false) Result.Synced(reports) } @@ -74,6 +100,24 @@ class SyncEngine @Inject constructor( * can no longer be decrypted — the silent failure the account layer exists to * avoid. */ + /** + * Marks an account as stopped until the user signs in again. + * + * ⚠️ It does **not** cancel the work, even though stopping the timer is the + * whole point — because this runs *inside* `SyncWorker`, and one of the two + * unique names it would cancel is the WorkSpec currently executing us. + * WorkManager would interrupt the coroutine, so `Result.NeedsSignIn` would + * never be returned and the adapter would see CANCELLED rather than FAILED. + * + * The flag does the work instead: [sync] refuses before touching the network, + * so a firing that survives costs nothing, and [AccountRepository.rescheduleAll] + * cancels the schedule from outside any worker. + */ + private suspend fun stopForSignIn(accountId: Long, reason: String) { + accountState.setNeedsSignIn(accountId, true) + database.accounts().recordSync(accountId, at = null, error = reason) + } + private fun fatal(accountId: Long, result: Result): Result { val reason = when (result) { is Result.NeedsSignIn -> result.reason diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncReport.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncReport.kt index 17d9bb5..9932135 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncReport.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/SyncReport.kt @@ -45,6 +45,17 @@ data class SyncReport( * error in the UI. */ val incrementalNote: String? = null, + /** + * The server refused our credentials. + * + * ⚠️ Escalates to the whole account and stops it, unlike every other failure + * here. Nextcloud's brute-force protection throttles and then **429s per + * source IP**, so a client that keeps retrying a dead app password on a timer + * takes the user's *other* Nextcloud clients down with it, on that network, + * and looks from the outside like we broke their server. There is nothing to + * retry anyway: only the user can fix it. + */ + val authFailure: Boolean = false, /** Set when the collection failed as a whole. The account keeps going. */ val failure: String? = null, ) { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidator.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidator.kt index 457f504..5220723 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidator.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidator.kt @@ -57,7 +57,16 @@ object ResourceValidator { val defined = calendar.components("VTIMEZONE") .mapNotNull { it.property("TZID")?.value } .toSet() - val undefined = todos.flatMap(::tzidsIn).filterNot { it in defined }.distinct() + val undefined = todos.flatMap(::tzidsIn) + // ⚠️ The solidus form is exempt, and the rule above says so: §3.2.19 + // requires a local VTIMEZONE only for a TZID *without* a leading + // solidus, because the prefix marks a globally defined identifier. + // Thunderbird writes `/mozilla.org/20050126_1/Europe/Berlin` on every + // zoned task, `java.time` cannot resolve it, and rejecting it would + // quarantine every Lightning-authored task permanently. + .filterNot { it.startsWith('/') } + .filterNot { it in defined } + .distinct() if (undefined.isNotEmpty()) { return Rejection("references the unknown time zone ${undefined.first()}") } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt index 6d22260..10c4e25 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt @@ -72,6 +72,13 @@ object VTodoMapper { private val SUPPRESSED_BY_RESIDUE = setOf( "DTSTART", "DUE", "COMPLETED", "RECURRENCE-ID", "CREATED", "LAST-MODIFIED", "STATUS", "RELATED-TO", + // ⚠️ The scalars belong here too, and their absence was invisible: the + // round-trip corpus filters SEQUENCE out of comparison entirely, so a + // task read from `SEQUENCE:x` was re-emitted carrying *both* `SEQUENCE:0` + // and `SEQUENCE:x` with nothing to catch it. `write` always authors + // SEQUENCE, so the duplicate is unconditional — and under + // `Prefer: handling=strict` sabre will not quietly repair it. + "SEQUENCE", "PRIORITY", "PERCENT-COMPLETE", "CLASS", ) /** What a VTODO yields. Row identity ([TaskEntity.id], `listId`) is the caller's. */ @@ -331,6 +338,14 @@ object VTodoMapper { // no parent" — dropping the link there would destroy a relationship // over a row we simply have not fetched. "RELATED-TO" -> parentUid != null && property.value.trim() != parentUid + // Each of these reaches the residue only when it could not be parsed, + // which left the column at its fallback. A column that has since moved + // off that fallback is the user's edit, and it wins. + "PRIORITY" -> entity.priority != PRIORITY_NONE + "PERCENT-COMPLETE" -> entity.percentComplete != null + "CLASS" -> entity.classification != null + // Not SEQUENCE: it is the organiser's counter and never ours to bump, + // so the column stays at its fallback and the residue always wins. else -> false } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt index 42b5762..5d74654 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt @@ -2,13 +2,19 @@ package de.jeanlucmakiola.agendula.ui.accounts import android.text.format.DateUtils import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.selection.toggleable import androidx.compose.material.icons.Icons import androidx.compose.material.icons.rounded.Add import androidx.compose.material.icons.rounded.CloudSync +import androidx.compose.material.icons.rounded.Login import androidx.compose.material.icons.rounded.Sync import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Checkbox import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme @@ -20,7 +26,9 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier +import androidx.compose.ui.Alignment import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.agendula.R @@ -40,6 +48,7 @@ internal fun AccountsScreen( ) { val accounts by viewModel.accounts.collectAsStateWithLifecycle() var pendingRemoval by remember { mutableStateOf(null) } + var deleteLocalData by remember { mutableStateOf(false) } // An account can be removed from system Settings while we are away. OnResume { viewModel.refresh() } @@ -71,13 +80,17 @@ internal fun AccountsScreen( ) Spacer(Modifier.height(24.dp)) } else { - loaded.forEachIndexed { index, account -> + loaded.forEachIndexed { index, row -> + val account = row.account GroupedRow( title = account.displayName, // Never the raw values: lastSyncError is an exception string // and lastSyncAt renders as an ISO-8601 UTC instant, and both // bypass strings.xml entirely. summary = when { + // Distinct from a failed sync on purpose: this one has + // stopped retrying, and only the user can restart it. + row.needsSignIn -> stringResource(R.string.accounts_needs_sign_in) account.lastSyncError != null -> stringResource(R.string.accounts_sync_failed) account.lastSyncAt != null -> DateUtils.getRelativeTimeSpanString( @@ -90,11 +103,20 @@ internal fun AccountsScreen( position = positionOf(index, loaded.size), modifier = Modifier.padding(horizontal = 16.dp), trailing = { - IconButton(onClick = { viewModel.syncNow(account) }) { - Icon( - Icons.Rounded.Sync, - contentDescription = stringResource(R.string.accounts_sync_now), - ) + if (row.needsSignIn) { + IconButton(onClick = onAddAccount) { + Icon( + Icons.Rounded.Login, + contentDescription = stringResource(R.string.accounts_sign_in_again), + ) + } + } else { + IconButton(onClick = { viewModel.syncNow(account) }) { + Icon( + Icons.Rounded.Sync, + contentDescription = stringResource(R.string.accounts_sync_now), + ) + } } }, onClick = { pendingRemoval = account }, @@ -113,20 +135,43 @@ internal fun AccountsScreen( } // A plain confirmation, which is the one thing CLAUDE.md still allows an - // AlertDialog for. + // AlertDialog for. The checkbox modifies the confirmation rather than turning + // it into a chooser — a full-screen picker for "are you sure" would be worse, + // and a radio list is banned outright. pendingRemoval?.let { account -> AlertDialog( - onDismissRequest = { pendingRemoval = null }, + onDismissRequest = { pendingRemoval = null; deleteLocalData = false }, title = { Text(stringResource(R.string.accounts_remove_confirm_title, account.displayName)) }, - text = { Text(stringResource(R.string.accounts_remove_confirm_body)) }, + text = { + Column { + Text(stringResource(R.string.accounts_remove_confirm_body)) + Spacer(Modifier.height(16.dp)) + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.toggleable( + value = deleteLocalData, + role = Role.Checkbox, + onValueChange = { deleteLocalData = it }, + ), + ) { + Checkbox(checked = deleteLocalData, onCheckedChange = null) + Spacer(Modifier.width(12.dp)) + Text( + stringResource(R.string.accounts_remove_delete_local), + style = MaterialTheme.typography.bodyMedium, + ) + } + } + }, confirmButton = { TextButton(onClick = { - viewModel.remove(account) + viewModel.remove(account, deleteLocalData) pendingRemoval = null + deleteLocalData = false }) { Text(stringResource(R.string.accounts_remove_confirm_action)) } }, dismissButton = { - TextButton(onClick = { pendingRemoval = null }) { + TextButton(onClick = { pendingRemoval = null; deleteLocalData = false }) { Text(stringResource(android.R.string.cancel)) } }, diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt index df1b260..df2290f 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.agendula.data.sync.AccountRepository +import de.jeanlucmakiola.agendula.data.sync.AccountStateStore import de.jeanlucmakiola.agendula.data.sync.SyncTrigger import de.jeanlucmakiola.agendula.data.tasks.room.AccountEntity import kotlinx.coroutines.flow.MutableStateFlow @@ -16,19 +17,26 @@ import javax.inject.Inject class AccountsViewModel @Inject constructor( private val repository: AccountRepository, private val syncTrigger: SyncTrigger, + private val accountState: AccountStateStore, ) : ViewModel() { - private val _accounts = MutableStateFlow?>(null) + /** An account row, plus the one thing the row cannot read from the entity. */ + data class AccountRow(val account: AccountEntity, val needsSignIn: Boolean) + + private val _accounts = MutableStateFlow?>(null) /** `null` until the first load, so the empty state does not flash. */ - val accounts: StateFlow?> = _accounts.asStateFlow() + val accounts: StateFlow?> = _accounts.asStateFlow() init { refresh() } fun refresh() { - viewModelScope.launch { _accounts.value = repository.all() } + viewModelScope.launch { + val stopped = accountState.needingSignIn() + _accounts.value = repository.all().map { AccountRow(it, it.id in stopped) } + } } /** @@ -45,9 +53,9 @@ class AccountsViewModel @Inject constructor( syncTrigger.enqueue(account.displayName, expedited = true) } - fun remove(account: AccountEntity) { + fun remove(account: AccountEntity, deleteLocalData: Boolean) { viewModelScope.launch { - repository.remove(account.id, account.displayName) + repository.remove(account.id, account.displayName, deleteLocalData) refresh() } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/LicencesScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/LicencesScreen.kt new file mode 100644 index 0000000..5a3792b --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/LicencesScreen.kt @@ -0,0 +1,61 @@ +package de.jeanlucmakiola.agendula.ui.licences + +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalUriHandler +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.floret.components.CollapsingScaffold +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.positionOf + +/** + * Third-party attribution. + * + * Reachable from Settings rather than buried, because MPL-2.0 §3.2(a) is about + * what the *recipient of the binary* is told — a notice only a developer reading + * the repository would find does not discharge it. Each row opens the project's + * source, which is the specific thing §3.2(a) requires us to point at. + */ +@Composable +internal fun LicencesScreen(onBack: () -> Unit) { + val uriHandler = LocalUriHandler.current + + CollapsingScaffold( + title = stringResource(R.string.licences_title), + onBack = onBack, + ) { + Text( + stringResource(R.string.licences_intro), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp), + ) + Spacer(Modifier.height(16.dp)) + + OpenSourceLicences.ALL.forEachIndexed { index, attribution -> + GroupedRow( + title = attribution.name, + summary = "${attribution.copyright} · ${attribution.licence.spdxId}", + position = positionOf(index, OpenSourceLicences.ALL.size), + modifier = Modifier.padding(horizontal = 16.dp), + onClick = { uriHandler.openUri(attribution.sourceUrl) }, + ) + } + + Spacer(Modifier.height(24.dp)) + Text( + stringResource(R.string.licences_footer), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp), + ) + Spacer(Modifier.height(24.dp)) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/OpenSourceLicences.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/OpenSourceLicences.kt new file mode 100644 index 0000000..3db0cd1 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/licences/OpenSourceLicences.kt @@ -0,0 +1,107 @@ +package de.jeanlucmakiola.agendula.ui.licences + +/** + * The third-party code Agendula ships, and what each licence obliges us to say. + * + * ⚠️ This screen is a **licence obligation, not an about-page nicety**. Until it + * exists the app is in plain violation: + * + * - **MPL-2.0 §3.2(a)** requires that recipients of the *executable* be told how + * to obtain the source of the covered files, and **§3.4** that the file headers + * be retained. That is dav4jvm, which we vendor — vendoring makes the + * obligation ours rather than a transitive dependency's. + * - **BSD-3-Clause** requires the copyright notice and disclaimer to be + * reproduced "in the documentation and/or other materials provided with the + * distribution", which for an app is exactly this. + * - **Apache-2.0 §4(d)** propagates any `NOTICE` file, and §4(a) requires the + * licence to travel with the work. + * + * Hand-maintained on purpose. Generators read POM metadata, and POM metadata is + * routinely wrong — the licence name is often not an SPDX id and the licence URL + * frequently 404s, which produces confidently empty output. A short honest list + * beats a long generated one that omits the entry that mattered. + * + * **When adding a dependency, add it here.** There is no automated check that + * can tell you that you forgot. + */ +data class Attribution( + val name: String, + val copyright: String, + val licence: Licence, + /** Where the source can actually be obtained — MPL-2.0 §3.2(a)'s requirement. */ + val sourceUrl: String, +) + +enum class Licence(val spdxId: String, val url: String) { + APACHE_2("Apache-2.0", "https://www.apache.org/licenses/LICENSE-2.0"), + BSD_3("BSD-3-Clause", "https://opensource.org/license/bsd-3-clause"), + MPL_2("MPL-2.0", "https://mozilla.org/MPL/2.0/"), + MIT("MIT", "https://opensource.org/license/mit"), +} + +object OpenSourceLicences { + + /** + * ⚠️ dav4jvm is **vendored**, not depended on — see `dav/PROVENANCE.md`. That + * makes MPL-2.0 §3.2(a) directly ours: we distribute modified covered files + * inside our binary, so we must say where their source is. The upstream URL + * satisfies it only together with `PROVENANCE.md`, which lists every change + * we made; both are named below. + */ + val ALL: List = listOf( + Attribution( + name = "dav4jvm (vendored, modified — see dav/PROVENANCE.md)", + copyright = "© bitfire web engineering (Ricki Hirner, Bernhard Stockmann)", + licence = Licence.MPL_2, + sourceUrl = "https://github.com/bitfireAT/dav4jvm", + ), + Attribution( + name = "dnsjava", + copyright = "© Brian Wellington and the dnsjava contributors", + licence = Licence.BSD_3, + sourceUrl = "https://github.com/dnsjava/dnsjava", + ), + Attribution( + name = "OkHttp", + copyright = "© Square, Inc.", + licence = Licence.APACHE_2, + sourceUrl = "https://github.com/square/okhttp", + ), + Attribution( + name = "lib-recur", + copyright = "© dmfs GmbH", + licence = Licence.APACHE_2, + sourceUrl = "https://github.com/dmfs/lib-recur", + ), + Attribution( + name = "Kotlin, kotlinx.coroutines, kotlinx.datetime, kotlinx.serialization", + copyright = "© JetBrains s.r.o. and Kotlin Programming Language contributors", + licence = Licence.APACHE_2, + sourceUrl = "https://github.com/JetBrains/kotlin", + ), + Attribution( + name = "AndroidX, Jetpack Compose, Room, WorkManager, DataStore, Glance", + copyright = "© The Android Open Source Project", + licence = Licence.APACHE_2, + sourceUrl = "https://cs.android.com/androidx/platform/frameworks/support", + ), + Attribution( + name = "Dagger and Hilt", + copyright = "© Google LLC and The Dagger Authors", + licence = Licence.APACHE_2, + sourceUrl = "https://github.com/google/dagger", + ), + Attribution( + name = "Material Design icons", + copyright = "© Google LLC", + licence = Licence.APACHE_2, + sourceUrl = "https://github.com/google/material-design-icons", + ), + Attribution( + name = "floret-kit", + copyright = "© Jean-Luc Makiola", + licence = Licence.MIT, + sourceUrl = "https://codeberg.org/jlmakiola/floret-kit", + ), + ) +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt index b05d6cc..7c6f61a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt @@ -40,6 +40,7 @@ import androidx.compose.material.icons.filled.ExpandLess import androidx.compose.material.icons.filled.ExpandMore import androidx.compose.material.icons.filled.Favorite import androidx.compose.material.icons.filled.Gavel +import androidx.compose.material.icons.filled.PrivacyTip import androidx.compose.material.icons.filled.Language import androidx.compose.material.icons.filled.Notifications import androidx.compose.material.icons.filled.Palette @@ -68,6 +69,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.res.colorResource import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource @@ -78,6 +80,7 @@ import androidx.core.net.toUri import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.agendula.ui.licences.LicencesScreen import de.jeanlucmakiola.agendula.data.prefs.ThemeMode import de.jeanlucmakiola.agendula.domain.TaskFormField import de.jeanlucmakiola.agendula.ui.export.ExportScreen @@ -110,6 +113,7 @@ private enum class SettingsSection { Export, Accounts, AddAccount, + Licences, ; /** Where back goes: Export is opened from Storage, AddAccount from Accounts. */ @@ -189,6 +193,9 @@ fun SettingsScreen( viewModel = accountsViewModel, ) } + SlideInSection(visible = section == SettingsSection.Licences) { + LicencesScreen(onBack = { section = null }) + } SlideInSection(visible = section == SettingsSection.AddAccount) { AddAccountScreen( onDone = { @@ -285,12 +292,41 @@ private fun SettingsHub( onClick = { onOpenSection(SettingsSection.Accounts) }, ) LanguageRow(position = Position.Middle) + GroupedRow( + title = stringResource(R.string.settings_licences), + summary = stringResource(R.string.settings_licences_subtitle), + position = Position.Middle, + leading = { CategoryIcon(Icons.Default.Gavel, ChipAccent.Neutral) }, + onClick = { onOpenSection(SettingsSection.Licences) }, + ) + PrivacyPolicyRow(position = Position.Middle) ReportProblemRow(position = Position.Bottom) AppVersionText() } } +/** + * ⚠️ Linked **in the app**, not only in the Play Console. + * + * Play requires both, and the in-app link is the half that is routinely missed. + * Agendula's data-safety declaration is *Collected, not Shared*, encrypted in + * transit: Play defines collection as transmitting off-device irrespective of + * who receives it, so "not collected" is not defensible for a client that PUTs + * the user's tasks to their own server. + */ +@Composable +private fun PrivacyPolicyRow(position: Position) { + val uriHandler = LocalUriHandler.current + val privacyUrl = stringResource(R.string.about_privacy_url) + GroupedRow( + title = stringResource(R.string.settings_privacy), + position = position, + leading = { CategoryIcon(Icons.Default.PrivacyTip, ChipAccent.Neutral) }, + onClick = { uriHandler.openUri(privacyUrl) }, + ) +} + /** * The app-language row. Deliberately not floret-kit's `LanguagePickerRow`: the * picker it opens carries a "Help translate" header, and inviting contributions diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 33bb2df..7a4bd50 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -211,6 +211,12 @@ Source License Support development + Open source licenses + The projects Agendula is built on + Open source licenses + Agendula includes the work below. Tap any entry to get its source code. + Some of this code is included in modified form. Where that\u2019s the case, every change is listed in the source repository linked above. + Privacy policy Version %1$s Agendula app icon App language @@ -223,6 +229,7 @@ Crash report https://codeberg.org/jlmakiola/agendula/issues/new https://codeberg.org/jlmakiola/agendula/src/branch/main/LICENSE + https://codeberg.org/jlmakiola/agendula/src/branch/main/docs/PRIVACY.md https://ko-fi.com/jeanlucmakiola https://weblate.dev.jeanlucmakiola.de/engage/agendula/ Theme @@ -308,6 +315,9 @@ Never synced Last sync didn\u2019t finish Sync now + Sign in again to keep syncing + Sign in again + Also delete this account\u2019s tasks from this device Syncing Syncing tasks diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/IncrementalSyncTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/IncrementalSyncTest.kt index 3a249af..9cb39ee 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/IncrementalSyncTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/IncrementalSyncTest.kt @@ -235,6 +235,27 @@ class IncrementalSyncTest { assertThat(store.rows.single().title).isEqualTo("Server's canonical form") } + @Test fun `a failed download never advances the cursor`() { + remote.changePages += page(changed = listOf("one.ics"), token = "urn:x:2") + remote.fetchFailure = "connection reset" + + sync() + + // ⚠️ A cursor committed past bodies that never landed is a permanent hole + // in the collection, invisible until the next full reconciliation. + assertThat(store.tokenWrites).doesNotContain("urn:x:2") + } + + @Test fun `a listed resource the server will not return is counted`() { + remote.changePages += page(changed = listOf("ghost.ics"), token = "urn:x:2") + + val report = sync() + + // In the listing, so the sweep leaves it; never applied, so nothing else + // would ever count it. + assertThat(report.quarantined.single().reason).contains("listed but not returned") + } + // ------------------------------------------------------------- cadence @Test fun `a due full reconciliation ignores the token entirely`() { diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt index b5becf4..1b4478b 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt @@ -92,6 +92,7 @@ class FakeRemote(override val url: HttpUrl = "http://server/dav/tasks/".toHttpUr val changePages = ArrayDeque() var stateFailure: String? = null var listFailure: String? = null + var fetchFailure: String? = null /** Returns non-null to pre-empt the default behaviour for that href. */ var onPut: ((HttpUrl) -> PutOutcome?)? = null @@ -137,6 +138,7 @@ class FakeRemote(override val url: HttpUrl = "http://server/dav/tasks/".toHttpUr override fun fetch(hrefs: List): Result { log += "FETCH ${hrefs.joinToString(",") { it.pathSegments.last() }}" + fetchFailure?.let { return Result.failure(IllegalStateException(it)) } val found = hrefs.mapNotNull { href -> resources[href.toString()]?.let { RemoteResource(href, it.eTag, it.body) } } diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/CalendarResourceTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/CalendarResourceTest.kt index 28daab8..2db7b61 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/CalendarResourceTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/CalendarResourceTest.kt @@ -49,6 +49,29 @@ class CalendarResourceTest { assertThat(CalendarResource.parse("BEGIN:VCARD\r\nEND:VCARD\r\n")).isEmpty() } + @Test fun `an unparseable scalar is never emitted twice`() { + // ⚠️ Every one of these reaches the residue verbatim because it could not + // be parsed, and `write` authors its column form unconditionally. Without + // suppression the resource carries both — and `Prefer: handling=strict` + // means sabre will not quietly repair the duplicate. + val source = javaClass.classLoader!! + .getResourceAsStream("vtodo/malformed-scalars.ics")!! + .readBytes().decodeToString() + val vtodo = CalendarResource.todosIn(CalendarResource.parse(source)).single() + + val written = CalendarResource.serialize( + listOf(VTodoMapper.write(VTodoMapper.read(vtodo).entity)), + ) + + listOf("SEQUENCE", "PRIORITY", "PERCENT-COMPLETE", "CLASS", "STATUS").forEach { name -> + assertThat(Regex("(?m)^$name[;:]").findAll(written).count()) + .isEqualTo(1) + } + // The residue's copy is the one that survives, verbatim. + assertThat(written).contains("SEQUENCE:x") + assertThat(written).contains("PRIORITY:11") + } + private fun todo(vararg params: ICalParam) = ICalComponent( name = "VTODO", properties = listOf( diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidatorTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidatorTest.kt index 3fd6c21..8db277c 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidatorTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/ResourceValidatorTest.kt @@ -177,6 +177,17 @@ class ResourceValidatorTest { """))).contains("unknown time zone") } + @Test fun `a solidus-prefixed TZID needs no local definition`() { + // Thunderbird writes this on every zoned task. §3.2.19 exempts the + // solidus form, and rejecting it would quarantine those tasks forever. + assertThat(reject(vcalendar(""" + BEGIN:VTODO + UID:a + DTSTART;TZID=/mozilla.org/20050126_1/Europe/Berlin:20260301T090000 + END:VTODO + """))).isNull() + } + @Test fun `a TZID with its definition present is fine`() { assertThat(reject(vcalendar(""" BEGIN:VTIMEZONE diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt index baa3d76..1bc8a66 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt @@ -340,6 +340,11 @@ class AddAccountViewModelTest { override suspend fun pollLoginFlow(flow: NextcloudLoginFlow.Flow) = pollResults.removeFirstOrNull() ?: NextcloudLoginFlow.PollResult.Pending + + /** Revocation is best effort and the flow never depends on it. */ + override suspend fun revokeAppPassword( + credentials: CalDavGateway.Credentials, + ): Boolean = true } private class FakeCreator : AccountCreator { diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/AppPassword.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/AppPassword.kt new file mode 100644 index 0000000..4d305e1 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/AppPassword.kt @@ -0,0 +1,82 @@ +package de.jeanlucmakiola.caldav + +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import java.io.IOException + +/** + * Gives an app password back when the account is removed. + * + * ⚠️ Without this, **uninstalling never revokes access**. Nextcloud's Login Flow + * v2 mints a device-specific password that survives the app entirely: it stays + * listed under Settings → Security → Devices & sessions until the user notices + * and deletes it by hand, on an entry named after an app that is no longer + * installed. Minting a credential and then abandoning it is not an acceptable + * end state for a client that asked for one. + * + * Best-effort by design. The account is being removed either way, and a server + * that is unreachable, or was never a Nextcloud, must not block that. + */ +object AppPassword { + + /** Nextcloud's OCS endpoint for "delete the password I authenticated with". */ + private const val PATH = "ocs/v2.php/core/apppassword" + + /** + * Derives the OCS root from a Nextcloud principal URL. + * + * ⚠️ The principal URL is **not** the server root, and appending to it is the + * bug this function exists to prevent: a principal is + * `…/remote.php/dav/principals/users/alice/`, so + * `principal + "ocs/v2.php/…"` produces a path that 404s on every server, + * every time, silently — the revocation reads as "attempted" and does + * nothing at all. + * + * Nextcloud mounts WebDAV under `remote.php`, so everything before that + * segment is the server root, and that is true of a subpath install + * (`https://host/nextcloud/`) as much as of a root one. Falling back to the + * origin is right for a server that does not use `remote.php` — it is not a + * Nextcloud, so the endpoint does not exist there under any path. + */ + fun ocsRootFor(principal: HttpUrl): HttpUrl { + val mount = principal.pathSegments.indexOfFirst { it.equals("remote.php", ignoreCase = true) } + // No `remote.php` means this is not a Nextcloud layout, and guessing a + // prefix from an arbitrary DAV path would aim the DELETE somewhere + // unrelated. The origin is the only defensible answer, and on a server + // without the endpoint it simply 404s. + val prefix = if (mount < 0) emptyList() else principal.pathSegments.take(mount) + return principal.newBuilder() + .encodedPath("/") + .apply { + prefix.filter { it.isNotEmpty() }.forEach { addPathSegment(it) } + // A trailing empty segment keeps this a directory URL, so + // appending the OCS path cannot fuse onto the last segment. + if (prefix.any { it.isNotEmpty() }) addPathSegment("") + } + .build() + } + + /** + * @return true when the server confirmed the revocation. False means the + * credential may still exist server-side — the caller carries on regardless. + */ + fun revoke(httpClient: OkHttpClient, principal: HttpUrl): Boolean = try { + val url = ocsRootFor(principal).newBuilder().addPathSegments(PATH).build() + httpClient.newCall( + Request.Builder() + .url(url) + .delete() + // ⚠️ Not optional. Without this header Nextcloud answers the OCS + // API with a 401 and a CSRF complaint rather than doing the work, + // which reads exactly like a wrong password. + .header("OCS-APIRequest", "true") + .header("Accept", "application/json") + .build(), + ).execute().use { it.isSuccessful } + } catch (_: IOException) { + false + } catch (_: IllegalArgumentException) { + false + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/AppPasswordTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/AppPasswordTest.kt new file mode 100644 index 0000000..bd49093 --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/AppPasswordTest.kt @@ -0,0 +1,62 @@ +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 AppPasswordTest { + + private val server = MockWebServer() + private val httpClient = OkHttpClient() + + @Before fun start() = server.start() + + @After fun stop() = server.shutdown() + + @Test fun `the OCS root is derived from a principal, not appended to it`() { + // ⚠️ Appending to the principal produces a path that 404s on every + // server, silently — the revocation looks attempted and does nothing. + val principal = "https://cloud.example.com/remote.php/dav/principals/users/alice/" + assertThat(AppPassword.ocsRootFor(principal.toHttpUrl()).toString()) + .isEqualTo("https://cloud.example.com/") + } + + @Test fun `a subpath install keeps its subpath`() { + val principal = "https://host/nextcloud/remote.php/dav/principals/users/alice/" + assertThat(AppPassword.ocsRootFor(principal.toHttpUrl()).toString()) + .isEqualTo("https://host/nextcloud/") + } + + @Test fun `a principal with no remote_php falls back to the origin`() { + val principal = "https://baikal.example.com/dav.php/principals/alice/" + assertThat(AppPassword.ocsRootFor(principal.toHttpUrl()).toString()) + .isEqualTo("https://baikal.example.com/") + } + + @Test fun `the revocation is a DELETE with the OCS header`() { + server.enqueue(MockResponse().setResponseCode(200)) + + val revoked = AppPassword.revoke( + httpClient, + server.url("/remote.php/dav/principals/users/alice/"), + ) + + assertThat(revoked).isTrue() + val request = server.takeRequest() + assertThat(request.method).isEqualTo("DELETE") + assertThat(request.path).isEqualTo("/ocs/v2.php/core/apppassword") + // ⚠️ Without this header Nextcloud answers with a CSRF complaint that + // reads exactly like a wrong password. + assertThat(request.getHeader("OCS-APIRequest")).isEqualTo("true") + } + + @Test fun `an unreachable server is reported, not thrown`() { + server.enqueue(MockResponse().setResponseCode(404)) + assertThat(AppPassword.revoke(httpClient, server.url("/remote.php/dav/"))).isFalse() + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerMatrixTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerMatrixTest.kt new file mode 100644 index 0000000..d45edff --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerMatrixTest.kt @@ -0,0 +1,180 @@ +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.Ignore +import org.junit.Test + +/** + * The per-server trap matrix from `docs/SYNC.md` § *Server reality*. + * + * Everything reproducible from the protocol alone runs here against + * MockWebServer. The handful that genuinely need a live server are `@Ignore`d + * with the reason spelled out — they are named so they can be turned on by hand, + * not deleted and rediscovered. + */ +class ServerMatrixTest { + + private val server = MockWebServer() + private val httpClient = OkHttpClient.Builder().followRedirects(false).build() + private lateinit var collection: CalendarCollection + + @Before fun start() { + server.start() + collection = CalendarCollection(httpClient, server.url("/dav/tasks/")) + } + + @After fun stop() = server.shutdown() + + // ------------------------------------------------------------- Nextcloud + + @Test fun `Nextcloud per-calendar UID uniqueness is a rejection, not a retry`() { + // 409 no-uid-conflict: another resource in this collection already has + // that UID. Retrying the same body reproduces it exactly. + server.enqueue( + MockResponse().setResponseCode(409) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody( + "" + + "", + ), + ) + + val outcome = collection.create("one.ics", "x") + + assertThat(outcome).isInstanceOf(PutOutcome.Rejected::class.java) + assertThat((outcome as PutOutcome.Rejected).code).isEqualTo(409) + } + + @Test fun `Nextcloud's trashbin makes a reused href answer 403`() { + // The trashbin renames a deleted resource to `-deleted.ics`, so + // delete, recreate and delete again at the same href returns 403. Task + // apps hit this constantly because they reuse hrefs. + server.enqueue(MockResponse().setResponseCode(403)) + + val outcome = collection.delete(server.url("/dav/tasks/one.ics"), "e") + + // Not "already gone" and not a conflict — a refusal that must be + // quarantined rather than retried forever. + assertThat(outcome).isInstanceOf(DeleteOutcome.Rejected::class.java) + assertThat((outcome as DeleteOutcome.Rejected).code).isEqualTo(403) + } + + @Test fun `a shared Nextcloud calendar is recognised as shared`() { + // ⚠️ The whole mutilation guard hangs off this bit. `CalendarObject::get()` + // serves a whitelist-reduced body from a share while leaving the ETag + // untouched, so an engine that thinks the collection is not shared will + // write that reduced body back over the owner's task. + server.enqueue( + MockResponse().setResponseCode(207) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody( + """ + + + /dav/tasks/ + + + + + HTTP/1.1 200 OK + + + """.trimIndent(), + ), + ) + + assertThat(collection.state().getOrThrow().collection.isShared).isTrue() + } + + // ------------------------------------------------------------------ SOGo + + @Test fun `SOGo's second-granularity token is carried verbatim`() { + // SOGo's tokens are second-granularity integers rather than URIs. Nothing + // may parse, normalise or compare them as anything but opaque text. + server.enqueue( + MockResponse().setResponseCode(207) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody( + "1730000000", + ), + ) + + val page = collection.changes("1729999999") as ChangeSet.Page + + assertThat(page.token).isEqualTo("1730000000") + assertThat(server.takeRequest().body.readUtf8()).contains("1729999999") + } + + @Test fun `SOGo's main calendar survives classification`() { + // ⚠️ SOGo reports collection + calendar + schedule-outbox together for + // every non-Apple client. The obvious "exclude schedule-outbox" rule + // drops the user's only calendar. + server.enqueue( + MockResponse().setResponseCode(207) + .setHeader("Content-Type", "application/xml; charset=utf-8") + .setBody( + """ + + + /dav/tasks/ + + + + + HTTP/1.1 200 OK + + + """.trimIndent(), + ), + ) + + assertThat(collection.state().isSuccess).isTrue() + } + + // -------------------------------------------------------------- Radicale + + @Test fun `Radicale advertising sync-collection without meaning it degrades`() { + // Radicale advertised the report for years without implementing it, so a + // 501 must fall back rather than fail the collection. + server.enqueue(MockResponse().setResponseCode(501)) + assertThat(collection.changes("t")).isEqualTo(ChangeSet.Unsupported) + } + + // ---------------------------------------------------- needs a real server + + @Ignore( + "Needs a live Baikal on dav_auth_type = Digest. OkHttp has no Digest of " + + "its own (square/okhttp#205), so this exercises the vendored " + + "BasicDigestAuthHandler against a real challenge/nonce cycle, which " + + "MockWebServer cannot reproduce faithfully.", + ) + @Test fun `Baikal on Digest authenticates`() = Unit + + @Ignore( + "Needs a live Nextcloud with a calendar shared read-write from another " + + "account, holding a CLASS:CONFIDENTIAL task. Verifies that we never " + + "write back the whitelist-reduced body CalendarObject::get() serves " + + "— the single most destructive bug available here, and one no mock " + + "can prove absent because the reduction happens server-side.", + ) + @Test fun `a confidential task on a shared Nextcloud calendar survives a round trip`() = Unit + + @Ignore( + "Needs a live SOGo. Its ETag is a row-version counter and the body is " + + "regenerated per principal, so the same ETag can accompany different " + + "bytes. Proving we do not silently keep a stale body needs the real " + + "server's regeneration behaviour.", + ) + @Test fun `an ETag-unchanged body change on SOGo is detected`() = Unit + + @Ignore( + "Needs a live Nextcloud. MKCALENDAR is rate-limited to 10 per hour, " + + "which is the behaviour under test and cannot be mocked usefully.", + ) + @Test fun `Nextcloud rate-limits collection creation`() = Unit +} diff --git a/docs/PRIVACY.md b/docs/PRIVACY.md new file mode 100644 index 0000000..0086fc2 --- /dev/null +++ b/docs/PRIVACY.md @@ -0,0 +1,76 @@ +# Agendula — privacy policy + +_Last updated: 2026-09-07._ + +Agendula is a task app for Android, published by Jean-Luc Makiola. This policy +describes what happens to your data. It is short because very little happens to +it. + +## The short version + +Agendula has no servers. There is no Agendula account, no analytics, no +advertising, no tracking and no third-party SDK that reports anything anywhere. +Your tasks live on your device, and — only if you set that up yourself — on a +CalDAV server **you** choose. + +## What is stored on your device + +- Your task lists, tasks, reminders and app settings. +- If you add a CalDAV account: the server address, your username, and your + password or app password. The password is encrypted with a key held in the + Android Keystore, which cannot be exported from the device. + +## What leaves your device + +**Only if you add a CalDAV account**, and only to the server you entered: + +- Your tasks in those lists, as iCalendar data, and the credentials needed to + authenticate. +- Requests are made over HTTPS. Cleartext HTTP is refused unless you explicitly + opt in for a specific account. + +Nothing is sent anywhere else. In particular, nothing is sent to the developer. + +Under Google Play's Data Safety definitions this counts as **collected** — Play +defines collection as transmitting data off the device, regardless of who +receives it — and **not shared**, because the only recipient is the server you +nominated. Data is encrypted in transit. + +Your CalDAV provider has its own privacy policy, and your data on their server is +governed by it. Agendula has no relationship with them. + +## Crash reports + +If the app crashes, it can show you the report and ask whether to send it. It is +never sent without you choosing to send it, and you can read the whole report +first. + +## Backups + +If Android Auto Backup is enabled on your device, your tasks and settings may be +backed up to your own Google account. Two things are deliberately excluded: your +stored CalDAV password, and Agendula's per-device sync bookkeeping. + +## Deleting your data + +- **Remove a CalDAV account** from Settings → Accounts. This deletes the stored + credential and, where the server supports it, revokes the app password. Task + lists become device-only lists rather than being destroyed. +- **Remove an account and delete its local data** removes the lists and tasks as + well. +- **Uninstalling the app** removes everything Agendula stored on the device. + +Deleting data from your CalDAV server is done on that server. + +## Children + +Agendula is not directed at children and collects nothing about anyone. + +## Changes + +Material changes will be noted here with a new date at the top. The history of +this file is public in the repository. + +## Contact + +mail@jeanlucmakiola.de diff --git a/docs/SYNC-PLAN.md b/docs/SYNC-PLAN.md index 984c56f..433a363 100644 --- a/docs/SYNC-PLAN.md +++ b/docs/SYNC-PLAN.md @@ -756,6 +756,125 @@ so the follow-up branch is a file move and not a rediscovery. **Done when:** the matrix is green where a server exists to run it against, the attribution screen ships, and a `releaseTest` build syncs on a real device. +### What shipped, and what is still owed + +**Shipped in the chunk-5 commit** — everything verifiable without a device or a +live server: + +- **Attribution.** `Settings → Open source licenses`, hand-maintained. Not a + generated list: POM metadata routinely names a non-SPDX licence and points at a + URL that 404s, so generators produce confidently empty output. dav4jvm is + vendored, which makes MPL-2.0 §3.2(a) *ours* rather than a dependency's — the + row names `dav/PROVENANCE.md` alongside upstream, because §3.2(a) is about + where a recipient of the binary can obtain the source of what we modified. +- **Revocation, both directions.** A 401 escalates from the collection to the + account, marks it, **cancels its periodic work**, and stops. Cancelling is the + part that matters: Nextcloud throttles and then 429s per source IP, so a timer + on a dead app password degrades every other Nextcloud client on that network. + On removal, `DELETE /ocs/v2.php/core/apppassword` with `OCS-APIRequest: true`, + best effort — without it, uninstalling never revokes anything and the password + outlives the app in the user's device list. +- **Play compliance.** A privacy policy in `docs/PRIVACY.md`, linked **in the + app** (the half that is usually missed) and declaring *Collected, not Shared* — + "not collected" is indefensible when Play defines collection as transmission + off-device regardless of recipient. A "delete this account's tasks from this + device too" option on removal. `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` was + already absent and stays absent. +- **The server matrix**, as far as a protocol mock can carry it: Nextcloud's + `no-uid-conflict`, its trashbin 403 on a reused href, and share detection; + SOGo's second-granularity opaque tokens and its triple resource type; Radicale + advertising `sync-collection` without implementing it. + +**Still owed, and blocked on things a loop cannot do:** + +1. **The live matrix.** Four tests are `@Ignore`d with their reasons written out + rather than deleted: Baïkal on Digest, a `CLASS:CONFIDENTIAL` task on a shared + Nextcloud calendar, an ETag-unchanged body change on SOGo, and Nextcloud's + MKCALENDAR rate limit. Each names why a mock cannot stand in for it. +2. **`releaseTest` on device**, and the R8 outages `proguard-rules.pro` already + documents. +3. **The restore path**, which needs a restore onto a fresh device — the one + verification `SYNC.md` calls for explicitly and the one that cannot be faked. +4. **cert4android**, which replaces the current blanket user-CA trust and is a + UI-bearing dependency. +5. **Collection creation feature detection** (OPTIONS → MKCALENDAR / extended + MKCOL) and disabling the "new task list" affordance where neither exists. + Deferred because the app has no CalDAV-side list-creation UI yet, so there is + nothing to disable — it becomes real the moment that UI does. + +### Found by the review, and worth naming + +**⚠️ Revocation was aimed at the principal URL and revoked nothing.** The OCS +endpoint sits at the *server root*; the principal is +`…/remote.php/dav/principals/users/alice/`, so appending to it produced a path +that 404s on every server, every time — and the failure was swallowed as +best-effort. The whole feature was a no-op. The root is now derived by cutting at +`remote.php`, which is right for a subpath install too, with the origin as the +fallback. + +**⚠️ Opening the app resurrected accounts a 401 had just stopped.** +`ExistingPeriodicWorkPolicy.KEEP` only keeps work that is *unfinished*, and +CANCELLED counts as finished — so `rescheduleAll()` re-enqueued the very request +the stop had removed, and put a dead app password back on a four-hour loop +against a server that throttles by source IP. Exactly what the stop exists to +prevent. + +**⚠️ The cursor advanced past bodies that were never applied.** A network drop +mid-multiget set `report.failure` and returned, and the incremental path stored +the new token anyway — a permanent hole in the collection, invisible until the +next full reconciliation a day later. `runFull` had this guard from the start; +the incremental path did not. + +**Stopping an account cancelled the worker doing the stopping.** `syncTrigger.cancel` +kills both unique names, one of which is the WorkSpec currently executing — +WorkManager interrupts the coroutine, so `NeedsSignIn` was never returned and the +adapter saw CANCELLED rather than FAILED. The flag now does the work: `sync()` +refuses before touching the network, and the schedule is cancelled from outside +any worker. + +**"Sign in again" was a dead end.** `create` is the only path that writes a +credential and it rejected the duplicate name, so a user with a revoked app +password could only remove the account — discarding the choice to keep its lists. +It now re-authenticates in place, but *only* for an account already marked as +needing sign-in, so a healthy credential can never be silently overwritten. + +**Four scalars were emitted twice.** `SEQUENCE`, `PRIORITY`, `PERCENT-COMPLETE` +and `CLASS` reach the residue verbatim when unparseable, and `write` authors them +from their columns unconditionally — so a task read from `SEQUENCE:x` round-tripped +carrying both `SEQUENCE:0` and `SEQUENCE:x`. Invisible to the corpus, which +filters `SEQUENCE` out of comparison entirely. Now suppressed, with a test that +counts occurrences rather than comparing canonically. + +**A solidus-prefixed `TZID` was rejected as undefined** — the rule's own comment +says §3.2.19 exempts it. Thunderbird writes +`/mozilla.org/20050126_1/Europe/Berlin` on every zoned task, so the validator +would have quarantined every Lightning-authored task permanently. + +**Two smaller ones:** a resource the query lists but the multiget will not return +was re-requested for ever with nothing counting it, and account removal blocked +on a 30-second connect timeout before anything visible happened. + +## What moves to floret-kit + +Recorded here so the follow-up branch is a file move rather than a rediscovery. +The rule that made it a move rather than a rewrite held: **no task-domain type +and no Android UI type crossed into the DAV or CalDAV layers.** + +| Moves | Why it generalises | +|---|---| +| `:dav` (vendored dav4jvm) → `core-dav` | Not app-specific in any way; Calendula needs the identical tree | +| `:caldav` — discovery, `CalendarCollection`, `sync-collection`, quirks | Pure protocol, plain JVM, no Android types | +| `NextcloudLoginFlow` | Nothing about it is task-shaped | +| `CredentialStore`, `CalDavAccounts`, `SyncAuthenticator` | Keystore + `AccountManager` plumbing, identical for any CalDAV client | +| `LicencesScreen` / `Attribution` | Every app in the family owes the same notices | + +| Stays app-local | Why | +|---|---| +| `VTodoMapper`, `CalendarResource`, `ResourceValidator` | VTODO ↔ *our* Room schema; the residue contract is ours | +| `CollectionSyncer`, `SyncReport`, conflict policy and its UI | Encodes decision 2, which is a product decision | +| `SyncEngine`, `SyncWorker`, `SyncTrigger` | Bound to our entities and our scheduling choices | +| Storage modes, External mode | Agendula-specific by construction | + --- ## Definition of done, every chunk