diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 9b5dad9..0eae419 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -197,6 +197,8 @@ dependencies { // hilt-work supplies the HiltWorkerFactory; its compiler generates the // @HiltWorker plumbing. implementation(libs.androidx.work.runtime.ktx) + // Custom Tabs: the Nextcloud login flow hands the browser an approval page. + implementation(libs.androidx.browser) implementation(libs.androidx.hilt.work) ksp(libs.androidx.hilt.compiler) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 95b6e5a..f3d4f1b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -56,6 +56,12 @@ + + + + , + ): AccountRepository.Outcome +} + +/** + * Turns a finished sign-in into an account that exists in all three places it + * has to: the Room `accounts` row, the encrypted credential, and the + * system-visible `AccountManager` entry. + * + * The order matters. The Room row comes first because its id keys the + * credential, and the system account comes last because it is the one thing a + * user can see — an entry in Settings for an account whose credential failed to + * store would be a sync that silently never works. + */ +@Singleton +class AccountRepository @Inject constructor( + private val database: TasksDatabase, + private val credentials: CredentialStore, + private val accounts: CalDavAccounts, + @IoDispatcher private val io: CoroutineDispatcher, +) : AccountCreator { + + /** What went wrong, in words a user can act on. */ + sealed interface Outcome { + data class Created(val accountId: Long) : Outcome + data object AlreadyExists : Outcome + data class CredentialFailed(val reason: String) : Outcome + } + + suspend fun all(): List = withContext(io) { database.accounts().all() } + + /** + * Creates an account and the task lists the user chose. + * + * [selected] is a subset of what discovery found; a collection the user did + * not tick is simply not created, and can be added later without touching + * anything else — `task_lists.account_id` is a nullable FK, so attaching is + * an `UPDATE`. + */ + override suspend fun create( + displayName: String, + username: String, + appPassword: String, + found: CalDavDiscovery.Outcome.Found, + selected: Set, + ): Outcome = withContext(io) { + // Both stores, not just one. There is no unique index on + // accounts.display_name and nothing prunes Room when the system account + // 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 accountId = database.runInTransaction { + val id = database.accounts().insert( + AccountEntity( + displayName = displayName, + // Persist where a 301/308 actually put us — dav4jvm#209 exists + // precisely so this is knowable, and re-following the redirect + // on every sync is what not persisting it costs. + principalUrl = (found.movedTo ?: found.principal).toString(), + // The principal's own home set. `resolve("./")` on a collection + // URL is a no-op — CalDAV hrefs already end in "/" — so the + // old version stored the first collection's own URL, and that + // collection may not even be from the account's own home set. + homeSetUrl = found.homeSets.firstOrNull()?.toString(), + username = username, + ), + ) + selected.forEach { collection -> + database.taskLists().insert( + TaskListEntity( + name = collection.displayName ?: collection.url.pathSegments + .lastOrNull { it.isNotEmpty() } + .orEmpty(), + color = collection.color ?: DEFAULT_LIST_COLOR, + accountId = id, + isReadOnly = collection.readOnly, + href = collection.url.toString(), + ), + ) + } + id + } + + if (!credentials.put(accountId, appPassword)) { + // Never leave a half-made account behind: without a credential it + // would sit in Settings failing to sync with nothing to explain it. + rollback(accountId) + return@withContext Outcome.CredentialFailed( + "the device keystore would not store the password", + ) + } + + if (!accounts.add(displayName, accountId)) { + credentials.clear(accountId) + rollback(accountId) + return@withContext Outcome.AlreadyExists + } + + Outcome.Created(accountId) + } + + /** + * Undoes a half-made account. + * + * ⚠️ The lists have to go **explicitly**. `task_lists.account_id` is + * `ON DELETE SET NULL` — deliberately, so removing a working account never + * destroys tasks — which means deleting the account row alone leaves a full + * set of orphan device-only lists behind, and every retry adds another. + */ + private fun rollback(accountId: Long) = database.runInTransaction { + database.taskLists().deleteForAccount(accountId) + database.accounts().delete(accountId) + } + + /** + * Removes an account and everything that keys off it. + * + * The lists are **not** deleted: `task_lists.account_id` is `ON DELETE SET + * 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) { + credentials.clear(accountId) + database.accounts().delete(accountId) + accounts.find(displayName)?.let { accounts.remove(it) } + } + + private companion object { + /** M3 primary-ish blue; the user recolours a list from its own screen. */ + const val DEFAULT_LIST_COLOR = 0xFF4C6FFF.toInt() + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavAccounts.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavAccounts.kt index c133b58..71039fb 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavAccounts.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavAccounts.kt @@ -74,6 +74,18 @@ class CalDavAccounts @Inject constructor( fun roomAccountId(account: Account): Long? = accountManager.getUserData(account, KEY_ROOM_ACCOUNT_ID)?.toLongOrNull() + /** + * Removes the system-visible account. + * + * `removeAccountExplicitly` works because we own the account type. The Room + * row and the credential are removed by [AccountRepository]; pruning must be + * driven by this call and by `AccountManager`'s account-removed broadcast, + * never by "absent from the visible set" — `getAccountsByType` returns + * nothing while the device is locked, and treating that as removal is how a + * restore silently deletes the user's lists. + */ + fun remove(account: Account): Boolean = accountManager.removeAccountExplicitly(account) + fun requestSync(account: Account) { ContentResolver.requestSync( account, 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 new file mode 100644 index 0000000..aa030ff --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/sync/CalDavGateway.kt @@ -0,0 +1,73 @@ +package de.jeanlucmakiola.agendula.data.sync + +import de.jeanlucmakiola.agendula.data.di.IoDispatcher +import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.CalDavHttp +import de.jeanlucmakiola.caldav.DnsJavaResolver +import de.jeanlucmakiola.caldav.NextcloudLoginFlow +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.withContext +import okhttp3.HttpUrl +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The network side of adding an account, behind one interface. + * + * It exists so the sign-in state machine can be tested. That machine decides + * which of five outcomes leads where, when a one-shot app password is spent, and + * which host a credential is scoped to — all of which are exactly the sort of + * thing that goes wrong quietly, and none of which should require a server to + * exercise. + */ +interface CalDavGateway { + + /** Discovery against [target], optionally carrying credentials. */ + suspend fun discover(target: String, credentials: Credentials? = null): CalDavDiscovery.Outcome + + /** Starts Nextcloud Login Flow v2, or returns null if this is not a Nextcloud. */ + suspend fun startLoginFlow(server: HttpUrl): NextcloudLoginFlow.Flow? + + suspend fun pollLoginFlow(flow: NextcloudLoginFlow.Flow): NextcloudLoginFlow.PollResult + + /** Credentials, and the origin whose registrable domain they are scoped to. */ + data class Credentials(val username: String, val password: String, val origin: HttpUrl) +} + +@Singleton +class OkHttpCalDavGateway @Inject constructor( + @IoDispatcher private val io: CoroutineDispatcher, +) : CalDavGateway { + + /** + * Becomes the app password's **name** in Nextcloud's Settings → Security → + * Devices & sessions. OkHttp's default would show `okhttp/4.12.0`, leaving + * the user unable to tell what to revoke — which defeats the whole point of + * using an app password. + */ + private val userAgent = "Agendula (Android)" + + override suspend fun discover( + target: String, + credentials: CalDavGateway.Credentials?, + ): CalDavDiscovery.Outcome = withContext(io) { + val client = credentials?.let { + CalDavHttp.authenticated(userAgent, it.username, it.password, it.origin) + } ?: CalDavHttp.anonymous(userAgent) + CalDavDiscovery(client, DnsJavaResolver()).discover(target) + } + + override suspend fun startLoginFlow(server: HttpUrl): NextcloudLoginFlow.Flow? = + withContext(io) { + NextcloudLoginFlow(CalDavHttp.anonymous(userAgent), userAgent) + .start(server, now()) + .getOrNull() + } + + override suspend fun pollLoginFlow(flow: NextcloudLoginFlow.Flow): NextcloudLoginFlow.PollResult = + withContext(io) { + NextcloudLoginFlow(CalDavHttp.anonymous(userAgent), userAgent).poll(flow, now()) + } + + private fun now() = System.currentTimeMillis() / 1000 +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskListDao.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskListDao.kt index f6017a6..b432a6d 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskListDao.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskListDao.kt @@ -48,4 +48,15 @@ interface TaskListDao { @Query("DELETE FROM task_lists WHERE id = :listId") fun delete(listId: Long) + + /** + * Every list belonging to [accountId]. + * + * Only for rolling back a half-created account. Removing a *working* account + * must leave its lists alone — `account_id` is `ON DELETE SET NULL` for that + * reason, and deleting an account is not an instruction to destroy the tasks + * it held. + */ + @Query("DELETE FROM task_lists WHERE account_id = :accountId") + fun deleteForAccount(accountId: Long) } 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 new file mode 100644 index 0000000..098f104 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt @@ -0,0 +1,125 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +import android.text.format.DateUtils +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.Add +import androidx.compose.material.icons.rounded.CloudSync +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.agendula.data.tasks.room.AccountEntity +import de.jeanlucmakiola.agendula.ui.common.OnResume +import de.jeanlucmakiola.floret.components.CollapsingScaffold +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.positionOf + +/** The CalDAV accounts this device syncs with. */ +@Composable +internal fun AccountsScreen( + onAddAccount: () -> Unit, + onBack: () -> Unit, + viewModel: AccountsViewModel, +) { + val accounts by viewModel.accounts.collectAsStateWithLifecycle() + var pendingRemoval by remember { mutableStateOf(null) } + + // An account can be removed from system Settings while we are away. + OnResume { viewModel.refresh() } + + CollapsingScaffold( + title = stringResource(R.string.settings_section_accounts), + onBack = onBack, + ) { + val loaded = accounts ?: return@CollapsingScaffold + + if (loaded.isEmpty()) { + Icon( + Icons.Rounded.CloudSync, + contentDescription = null, + modifier = Modifier.padding(horizontal = 16.dp), + ) + Spacer(Modifier.height(12.dp)) + Text( + stringResource(R.string.accounts_empty_title), + style = MaterialTheme.typography.titleMedium, + modifier = Modifier.padding(horizontal = 16.dp), + ) + Spacer(Modifier.height(4.dp)) + Text( + stringResource(R.string.accounts_empty_body), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp), + ) + Spacer(Modifier.height(24.dp)) + } else { + loaded.forEachIndexed { index, 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 { + account.lastSyncError != null -> + stringResource(R.string.accounts_sync_failed) + account.lastSyncAt != null -> DateUtils.getRelativeTimeSpanString( + account.lastSyncAt.toEpochMilliseconds(), + System.currentTimeMillis(), + DateUtils.MINUTE_IN_MILLIS, + ).toString() + else -> stringResource(R.string.accounts_never_synced) + }, + position = positionOf(index, loaded.size), + modifier = Modifier.padding(horizontal = 16.dp), + onClick = { pendingRemoval = account }, + ) + } + Spacer(Modifier.height(24.dp)) + } + + GroupedRow( + title = stringResource(R.string.accounts_add), + position = Position.Alone, + modifier = Modifier.padding(horizontal = 16.dp), + leading = { Icon(Icons.Rounded.Add, contentDescription = null) }, + onClick = onAddAccount, + ) + } + + // A plain confirmation, which is the one thing CLAUDE.md still allows an + // AlertDialog for. + pendingRemoval?.let { account -> + AlertDialog( + onDismissRequest = { pendingRemoval = null }, + title = { Text(stringResource(R.string.accounts_remove_confirm_title, account.displayName)) }, + text = { Text(stringResource(R.string.accounts_remove_confirm_body)) }, + confirmButton = { + TextButton(onClick = { + viewModel.remove(account) + pendingRemoval = null + }) { Text(stringResource(R.string.accounts_remove_confirm_action)) } + }, + dismissButton = { + TextButton(onClick = { pendingRemoval = null }) { + 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 new file mode 100644 index 0000000..dc38bee --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt @@ -0,0 +1,38 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +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.tasks.room.AccountEntity +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import javax.inject.Inject + +@HiltViewModel +class AccountsViewModel @Inject constructor( + private val repository: AccountRepository, +) : ViewModel() { + + private val _accounts = MutableStateFlow?>(null) + + /** `null` until the first load, so the empty state does not flash. */ + val accounts: StateFlow?> = _accounts.asStateFlow() + + init { + refresh() + } + + fun refresh() { + viewModelScope.launch { _accounts.value = repository.all() } + } + + fun remove(account: AccountEntity) { + viewModelScope.launch { + repository.remove(account.id, account.displayName) + refresh() + } + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt new file mode 100644 index 0000000..b753d5d --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountScreen.kt @@ -0,0 +1,321 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +import android.content.ActivityNotFoundException +import android.content.Intent +import androidx.browser.customtabs.CustomTabsIntent +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.CloudOff +import androidx.compose.material3.Button +import androidx.compose.material3.Checkbox +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.unit.dp +import androidx.core.net.toUri +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import androidx.hilt.navigation.compose.hiltViewModel +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.caldav.ServerQuirk +import de.jeanlucmakiola.floret.components.CollapsingScaffold +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.GroupedSurface +import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.positionOf + +/** + * Adding a CalDAV account: one flow, one back-stack entry. + * + * A stepper rather than four destinations, because the steps are not + * independently reachable — you cannot pick lists before signing in, and going + * "back" from the browser step means abandoning a server-side flow rather than + * popping a screen. + */ +@Composable +internal fun AddAccountScreen( + onDone: () -> Unit, + onBack: () -> Unit, + viewModel: AddAccountViewModel = hiltViewModel(), +) { + val state by viewModel.state.collectAsStateWithLifecycle() + val context = LocalContext.current + + LaunchedEffect(state.step) { + if (state.step !is AddAccountStep.Done) return@LaunchedEffect + onDone() + // The ViewModel is scoped to the Settings back-stack entry and survives + // this section being hidden, so a finished flow left at Done would bounce + // the next "Add account" straight back out — and would reuse this + // account's username and app password for the next one. + viewModel.onStartOver() + } + + // Custom Tabs needs three things beyond launchUrl: the entry in the + // manifest (or provider detection silently finds nothing on API 30+), a + // fallback for devices with no Custom Tabs browser at all — realistic on + // GrapheneOS, CalyxOS and plain AOSP, which is disproportionately this app's + // audience — and an explicit way back, since a dismissed tab returns nothing. + LaunchedEffect(state.openInBrowser) { + val url = state.openInBrowser ?: return@LaunchedEffect + val uri = url.toString().toUri() + try { + CustomTabsIntent.Builder().build().launchUrl(context, uri) + } catch (_: ActivityNotFoundException) { + runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, uri)) } + } + viewModel.onBrowserLaunched() + } + + CollapsingScaffold( + title = stringResource(R.string.add_account_title), + // Backing out abandons a flow that may still be polling the server every + // two seconds for the rest of its twenty-minute window. + onBack = { + viewModel.onStartOver() + onBack() + }, + ) { + val fatal = state.fatal + if (fatal != null) { + Message(icon = { Icon(Icons.Rounded.CloudOff, contentDescription = null) }, text = fatal) + Spacer(Modifier.height(16.dp)) + Button( + onClick = viewModel::onStartOver, + modifier = Modifier.padding(horizontal = 16.dp), + ) { Text(stringResource(R.string.add_account_start_over)) } + return@CollapsingScaffold + } + + when (val step = state.step) { + is AddAccountStep.EnterServer -> ServerStep(step, state.quirk, viewModel) + is AddAccountStep.Working -> WorkingStep(step) + is AddAccountStep.EnterCredentials -> CredentialsStep(step, viewModel) + is AddAccountStep.WaitingForBrowser -> BrowserStep(step, viewModel) + is AddAccountStep.ChooseLists -> ListsStep(step, viewModel) + AddAccountStep.Done -> Unit + } + } +} + +@Composable +private fun ServerStep( + step: AddAccountStep.EnterServer, + quirk: ServerQuirk?, + viewModel: AddAccountViewModel, +) { + Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + OutlinedTextField( + value = step.input, + onValueChange = viewModel::onServerInputChanged, + label = { Text(stringResource(R.string.add_account_server_label)) }, + placeholder = { Text(stringResource(R.string.add_account_server_hint)) }, + isError = step.error != null, + supportingText = step.error?.let { { Text(it) } }, + singleLine = true, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Uri, + imeAction = ImeAction.Go, + ), + modifier = Modifier.fillMaxWidth(), + ) + + // Named before the attempt, not after a 401 the user cannot act on. + quirk?.let { QuirkNote(it) } + + Button( + onClick = viewModel::onServerSubmitted, + enabled = step.input.isNotBlank(), + modifier = Modifier.fillMaxWidth(), + ) { Text(stringResource(R.string.add_account_continue)) } + } +} + +@Composable +private fun CredentialsStep(step: AddAccountStep.EnterCredentials, viewModel: AddAccountViewModel) { + Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + OutlinedTextField( + value = step.username, + onValueChange = viewModel::onUsernameChanged, + label = { Text(stringResource(R.string.add_account_username_label)) }, + singleLine = true, + modifier = Modifier.fillMaxWidth(), + ) + OutlinedTextField( + value = step.password, + onValueChange = viewModel::onPasswordChanged, + label = { Text(stringResource(R.string.add_account_password_label)) }, + visualTransformation = PasswordVisualTransformation(), + isError = step.error != null, + supportingText = { + Text(step.error ?: stringResource(R.string.add_account_password_hint)) + }, + singleLine = true, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Password, + imeAction = ImeAction.Go, + ), + modifier = Modifier.fillMaxWidth(), + ) + Button( + onClick = viewModel::onCredentialsSubmitted, + enabled = step.username.isNotBlank() && step.password.isNotEmpty(), + modifier = Modifier.fillMaxWidth(), + ) { Text(stringResource(R.string.add_account_sign_in)) } + } +} + +@Composable +private fun BrowserStep(step: AddAccountStep.WaitingForBrowser, viewModel: AddAccountViewModel) { + Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + Message( + icon = { + if (step.error == null) { + CircularProgressIndicator(Modifier.size(24.dp)) + } else { + Icon(Icons.Rounded.CloudOff, contentDescription = null) + } + }, + title = stringResource(R.string.add_account_browser_title), + text = step.error ?: stringResource(R.string.add_account_browser_body), + ) + + step.hostMismatch?.let { mismatch -> + QuirkNote( + text = stringResource( + R.string.add_account_browser_host_mismatch, + mismatch.actual, + mismatch.expected, + ), + ) + } + + TextButton( + onClick = viewModel::onBrowserCancelled, + modifier = Modifier.fillMaxWidth(), + ) { Text(stringResource(R.string.add_account_browser_use_password)) } + } +} + +@Composable +private fun WorkingStep(step: AddAccountStep.Working) { + Column( + Modifier.fillMaxWidth().padding(32.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(16.dp), + ) { + CircularProgressIndicator() + Text(step.message, style = MaterialTheme.typography.bodyLarge) + } +} + +@Composable +private fun ListsStep(step: AddAccountStep.ChooseLists, viewModel: AddAccountViewModel) { + Text( + stringResource(R.string.add_account_lists_title), + style = MaterialTheme.typography.titleMedium, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp), + ) + step.collections.forEachIndexed { index, collection -> + val checked = collection.url in step.selected + GroupedRow( + title = collection.displayName ?: collection.url.encodedPath, + summary = when { + collection.readOnly -> stringResource(R.string.add_account_lists_read_only) + collection.isShared -> stringResource(R.string.add_account_lists_shared) + else -> null + }, + position = positionOf(index, step.collections.size), + selected = checked, + modifier = Modifier.padding(horizontal = 16.dp), + trailing = { + Checkbox(checked = checked, onCheckedChange = null) + }, + onClick = { viewModel.onListToggled(collection.url) }, + ) + } + Spacer(Modifier.height(16.dp)) + Button( + onClick = viewModel::onSave, + enabled = step.selected.isNotEmpty(), + modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp), + ) { + Text( + if (step.selected.isEmpty()) { + stringResource(R.string.add_account_no_lists_selected) + } else { + stringResource(R.string.add_account_save) + }, + ) + } +} + +@Composable +private fun QuirkNote(quirk: ServerQuirk) { + val text = when (quirk) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> + "Fastmail needs an app password, and CalDAV is not available on the Basic plan." + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> + "iCloud needs an app-specific password, created at appleid.apple.com with two-factor on. " + + "Tasks stored there will not appear in Reminders." + ServerQuirk.GOOGLE_UNSUPPORTED -> + "Google Calendar does not support tasks over CalDAV." + ServerQuirk.NEXTCLOUD_BRUTE_FORCE_PROTECTED -> return + } + QuirkNote(text) +} + +@Composable +private fun QuirkNote(text: String) { + GroupedSurface( + position = Position.Alone, + color = MaterialTheme.colorScheme.tertiaryContainer, + gapBelow = false, + ) { + Text( + text, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onTertiaryContainer, + modifier = Modifier.padding(16.dp), + ) + } +} + +@Composable +private fun Message( + icon: @Composable () -> Unit, + text: String, + title: String? = null, +) { + Column( + Modifier.fillMaxWidth().padding(horizontal = 16.dp), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + icon() + title?.let { Text(it, style = MaterialTheme.typography.titleMedium) } + Text( + text, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt new file mode 100644 index 0000000..12ea333 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt @@ -0,0 +1,423 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import de.jeanlucmakiola.agendula.data.sync.AccountCreator +import de.jeanlucmakiola.agendula.data.sync.AccountRepository +import de.jeanlucmakiola.agendula.data.sync.CalDavGateway +import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.NextcloudLoginFlow +import de.jeanlucmakiola.caldav.ServerQuirk +import de.jeanlucmakiola.caldav.ServiceDiscovery +import de.jeanlucmakiola.caldav.TaskCollection +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import okhttp3.HttpUrl +import javax.inject.Inject + +/** Where the user is in adding an account. */ +sealed interface AddAccountStep { + + /** Type an address. The whole flow starts from one field. */ + data class EnterServer(val input: String = "", val error: String? = null) : AddAccountStep + + data class Working(val message: String) : AddAccountStep + + /** The server wants credentials and is not a Nextcloud we can hand to a browser. */ + data class EnterCredentials( + val username: String = "", + val password: String = "", + val error: String? = null, + ) : AddAccountStep + + /** + * The browser has the flow. We poll until the user approves, and offer an + * explicit way out — Custom Tabs return **no result** when dismissed, and + * Nextcloud's flow ends on a "you can close this window" page that never + * comes back to the app. + */ + data class WaitingForBrowser( + val hostMismatch: NextcloudLoginFlow.HostMismatch? = null, + val error: String? = null, + ) : AddAccountStep + + data class ChooseLists( + val collections: List, + val selected: Set, + ) : AddAccountStep + + data object Done : AddAccountStep +} + +data class AddAccountUiState( + val step: AddAccountStep = AddAccountStep.EnterServer(), + /** + * A warning the user should read before going further — the three providers + * whose real failure is not "wrong password", and a server that sent the + * login flow to a different host than the one typed. + */ + val quirk: ServerQuirk? = null, + /** Set when the flow cannot continue at all; the UI offers only "start over". */ + val fatal: String? = null, + /** Non-null once the browser flow has a URL to open. */ + val openInBrowser: HttpUrl? = null, +) + +@HiltViewModel +class AddAccountViewModel @Inject constructor( + private val repository: AccountCreator, + private val gateway: CalDavGateway, +) : ViewModel() { + + private val _state = MutableStateFlow(AddAccountUiState()) + val state: StateFlow = _state.asStateFlow() + + private var typedInput: String = "" + private var serverRoot: HttpUrl? = null + private var username: String = "" + private var appPassword: String = "" + private var found: CalDavDiscovery.Outcome.Found? = null + private var pollJob: Job? = null + + /** Which hosts asked for credentials, for the cross-domain diagnostic. */ + private var hostsNeedingAuth: List = emptyList() + + fun onServerInputChanged(value: String) = _state.update { + it.copy(step = AddAccountStep.EnterServer(value), quirk = ServerQuirk.forInput(value)) + } + + fun onUsernameChanged(value: String) = updateCredentials { it.copy(username = value) } + + fun onPasswordChanged(value: String) = updateCredentials { it.copy(password = value) } + + /** Step 1 → discovery, unauthenticated. */ + fun onServerSubmitted() { + val input = (_state.value.step as? AddAccountStep.EnterServer)?.input?.trim().orEmpty() + if (input.isEmpty()) return + typedInput = input + + val quirk = ServerQuirk.forInput(input) + if (quirk?.isFatal == true) { + // Google supports neither VTODO nor MKCALENDAR. Refusing with an + // explanation beats a 401 the user cannot act on. + _state.update { + it.copy( + step = AddAccountStep.EnterServer(input = input), + fatal = FATAL_GOOGLE, + quirk = quirk, + ) + } + return + } + + serverRoot = ServiceDiscovery.serverRootFor(input) + + working("Looking for a CalDAV server") + viewModelScope.launch { + when (val outcome = gateway.discover(typedInput)) { + is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome) + + is CalDavDiscovery.Outcome.NeedsAuthentication -> { + hostsNeedingAuth = outcome.hosts + offerSignIn() + } + + // No credentials were sent, so this is not a rejection — it is a + // server that answers RFC 5397 on a 200 + // instead of a 401. Sending the user back to the address field + // would make it permanently unreachable. + CalDavDiscovery.Outcome.Unauthenticated -> offerSignIn() + + is CalDavDiscovery.Outcome.NotCalDav -> backToServer(outcome.reason) + + is CalDavDiscovery.Outcome.Failed -> backToServer(outcome.reason) + } + } + } + + /** Step 2a → re-run discovery with the password the user typed. */ + fun onCredentialsSubmitted() { + val step = _state.value.step as? AddAccountStep.EnterCredentials ?: return + if (step.username.isBlank() || step.password.isEmpty()) return + username = step.username.trim() + appPassword = step.password + + working("Signing in") + viewModelScope.launch { + val credentials = serverRoot?.let { + CalDavGateway.Credentials(username, appPassword, it) + } + when (val outcome = gateway.discover(typedInput, credentials)) { + is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome) + + // A second rejection is the point at which naming the provider's + // own rule is worth more than repeating "wrong password". + is CalDavDiscovery.Outcome.NeedsAuthentication, + CalDavDiscovery.Outcome.Unauthenticated, + -> _state.update { + it.copy( + step = AddAccountStep.EnterCredentials( + username = username, + password = "", + error = quirkHint() ?: crossDomainHint() + ?: "That username or password was not accepted.", + ), + ) + } + + is CalDavDiscovery.Outcome.NotCalDav -> backToServer(outcome.reason) + is CalDavDiscovery.Outcome.Failed -> backToServer(outcome.reason) + } + } + } + + /** Step 2b → the browser flow, if this looks like a Nextcloud. */ + private suspend fun offerSignIn() { + val root = serverRoot ?: return backToServer("Could not work out the server address.") + val flow = gateway.startLoginFlow(root) + + if (flow == null) { + // Not a Nextcloud, or its login flow is unavailable. Ask for a + // username and password instead — that is the generic CalDAV path. + _state.update { + it.copy(step = AddAccountStep.EnterCredentials(error = quirkHint())) + } + return + } + + _state.update { + it.copy( + step = AddAccountStep.WaitingForBrowser(hostMismatch = flow.hostMismatch), + openInBrowser = flow.loginUrl, + ) + } + startPolling(flow) + } + + private fun startPolling(flow: NextcloudLoginFlow.Flow) { + pollJob?.cancel() + pollJob = viewModelScope.launch { + // ⚠️ Bounded here, not just by the server's answer. `poll` reports + // Expired past the flow's deadline, but an unbounded `while (true)` + // makes this loop's termination somebody else's responsibility — and + // a gateway that keeps saying "pending" would have it poll forever. + repeat(MAX_POLL_ATTEMPTS) { + delay(POLL_INTERVAL_MILLIS) + when (val result = gateway.pollLoginFlow(flow)) { + is NextcloudLoginFlow.PollResult.Approved -> { + username = result.credentials.loginName + appPassword = result.credentials.appPassword + serverRoot = result.credentials.server + working("Reading your task lists") + val outcome = gateway.discover( + // The server the credentials were *issued by*, not the + // host the user typed — the two differ in exactly the + // host-mismatch case this flow already models, and + // getting it wrong burns the one-shot app password. + target = result.credentials.server.toString(), + credentials = CalDavGateway.Credentials( + username, + appPassword, + result.credentials.server, + ), + ) + if (outcome is CalDavDiscovery.Outcome.Found) { + onDiscovered(outcome) + } else { + backToServer("Signed in, but no task lists could be read.") + } + return@launch + } + + NextcloudLoginFlow.PollResult.Pending -> Unit + + is NextcloudLoginFlow.PollResult.Expired -> { + browserFailed(result.reason) + return@launch + } + + is NextcloudLoginFlow.PollResult.Failed -> { + browserFailed(result.reason) + return@launch + } + } + } + browserFailed("The approval window closed before the server answered.") + } + } + + /** The user says they finished in the browser but nothing arrived. */ + fun onBrowserCancelled() { + pollJob?.cancel() + _state.update { + it.copy(step = AddAccountStep.EnterCredentials(error = null), openInBrowser = null) + } + } + + fun onBrowserLaunched() = _state.update { it.copy(openInBrowser = null) } + + fun onListToggled(url: HttpUrl) { + val step = _state.value.step as? AddAccountStep.ChooseLists ?: return + val selected = if (url in step.selected) step.selected - url else step.selected + url + _state.update { it.copy(step = step.copy(selected = selected)) } + } + + fun onSave() { + val step = _state.value.step as? AddAccountStep.ChooseLists ?: return + val discovered = found ?: return + val chosen = step.collections.filter { it.url in step.selected }.toSet() + + if (username.isBlank() || appPassword.isEmpty()) { + // A server that needed no credentials at all would otherwise be saved + // with an empty username and password, and fail every later sync. + _state.update { it.copy(step = AddAccountStep.EnterCredentials()) } + return + } + + working("Adding the account") + viewModelScope.launch { + val outcome = runCatching { + repository.create( + displayName = accountName(), + username = username, + appPassword = appPassword, + found = discovered, + selected = chosen, + ) + }.getOrElse { + // A constraint violation or an IO failure in DataStore must not + // take the app down and leave the spinner up forever. + AccountRepository.Outcome.CredentialFailed( + it.message ?: "the account could not be saved", + ) + } + when (outcome) { + is AccountRepository.Outcome.Created -> + _state.update { it.copy(step = AddAccountStep.Done) } + + AccountRepository.Outcome.AlreadyExists -> + fatal("That account is already set up.") + + is AccountRepository.Outcome.CredentialFailed -> fatal(outcome.reason) + } + } + } + + /** + * Full reset, including the credentials. + * + * Called on "start over", on leaving the screen, and after a successful add. + * The ViewModel is scoped to the Settings back-stack entry and the flow is an + * `AnimatedVisibility` section inside it, so it outlives the composable: + * without this, re-opening "Add account" would find `step = Done` and bounce + * straight back out, and a second account would be created with the *first* + * account's username and app password. + */ + fun onStartOver() { + pollJob?.cancel() + pollJob = null + found = null + typedInput = "" + serverRoot = null + username = "" + appPassword = "" + hostsNeedingAuth = emptyList() + _state.value = AddAccountUiState() + } + + override fun onCleared() { + pollJob?.cancel() + } + + // ------------------------------------------------------------- internals + + private fun onDiscovered(outcome: CalDavDiscovery.Outcome.Found) { + found = outcome + if (outcome.collections.isEmpty()) { + fatal("Signed in, but this account has no task lists that Agendula can use.") + return + } + _state.update { + it.copy( + step = AddAccountStep.ChooseLists( + collections = outcome.collections, + // Everything writable, pre-ticked: the common case is "all of + // them", and a read-only share is more often noise than not. + selected = outcome.collections.filterNot { c -> c.readOnly } + .map { c -> c.url } + .toSet(), + ), + openInBrowser = null, + ) + } + } + + private fun working(message: String) = + _state.update { it.copy(step = AddAccountStep.Working(message), fatal = null) } + + /** + * A dead end. The step goes back to the address underneath, so the spinner is + * actually gone rather than merely hidden behind the message — the screen + * happens to render `fatal` first, and relying on that leaves the state + * lying about what it is doing. + */ + private fun fatal(reason: String) = _state.update { + it.copy(step = AddAccountStep.EnterServer(input = typedInput), fatal = reason) + } + + private fun backToServer(reason: String) = _state.update { + it.copy(step = AddAccountStep.EnterServer(input = typedInput, error = reason)) + } + + private fun browserFailed(reason: String) = _state.update { + it.copy(step = AddAccountStep.WaitingForBrowser(error = reason), openInBrowser = null) + } + + private fun updateCredentials(transform: (AddAccountStep.EnterCredentials) -> AddAccountStep.EnterCredentials) { + val step = _state.value.step as? AddAccountStep.EnterCredentials ?: return + _state.update { it.copy(step = transform(step)) } + } + + /** The provider-specific reason a correct-looking password gets rejected. */ + private fun quirkHint(): String? = when (ServerQuirk.forInput(typedInput)) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> + "Fastmail needs an app password, not your account password — and CalDAV is not on the Basic plan." + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> + "iCloud needs an app-specific password, which you create at appleid.apple.com with two-factor on." + else -> null + } + + /** + * A home set on a different registrable domain than the account's own is + * legal (RFC 4791 §6.2.1) but unreachable for us: the credential is scoped to + * one domain. Better to name it than to leave "wrong password" standing. + */ + private fun crossDomainHint(): String? { + val root = serverRoot?.host ?: return null + val rootDomain = root.substringAfter('.', root) + val outside = hostsNeedingAuth.filterNot { it.endsWith(rootDomain) } + return outside.firstOrNull()?.let { + "This server keeps some task lists on $it, which Agendula cannot sign in to yet." + } + } + + private fun accountName(): String = + username.takeIf { it.isNotBlank() }?.let { "$it@${serverRoot?.host.orEmpty()}" } + ?: typedInput + + private companion object { + const val POLL_INTERVAL_MILLIS = 2_000L + + /** The server-side lifetime is 1200s; this covers it and then stops. */ + const val MAX_POLL_ATTEMPTS = 600 + const val FATAL_GOOGLE = + "Google Calendar does not support tasks over CalDAV — its own documentation says so — " + + "so Agendula cannot sync with it." + } +} 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 5c6da54..b05d6cc 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 @@ -49,6 +49,7 @@ import androidx.compose.material.icons.rounded.AccountTree import androidx.compose.material.icons.rounded.Circle import androidx.compose.material.icons.rounded.Flag import androidx.compose.material.icons.rounded.Percent +import androidx.compose.material.icons.rounded.CloudSync import androidx.compose.material.icons.rounded.Storage import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon @@ -95,6 +96,9 @@ import de.jeanlucmakiola.floret.identity.expandEnter import de.jeanlucmakiola.floret.reminders.ReminderOverride import de.jeanlucmakiola.floret.reminders.reminderOverrideFor import de.jeanlucmakiola.agendula.ui.common.OnResume +import de.jeanlucmakiola.agendula.ui.accounts.AccountsScreen +import de.jeanlucmakiola.agendula.ui.accounts.AccountsViewModel +import de.jeanlucmakiola.agendula.ui.accounts.AddAccountScreen import de.jeanlucmakiola.agendula.ui.common.reminderLeadTimeLabel /** The settings sub-screens reached from the hub's category rows. */ @@ -104,11 +108,17 @@ private enum class SettingsSection { Reminders, Storage, Export, + Accounts, + AddAccount, ; - /** Where back goes: Export is opened from Storage, not from the hub. */ + /** Where back goes: Export is opened from Storage, AddAccount from Accounts. */ val parent: SettingsSection? - get() = if (this == Export) Storage else null + get() = when (this) { + Export -> Storage + AddAccount -> Accounts + else -> null + } } /** @@ -132,6 +142,8 @@ fun SettingsScreen( ) { val state by viewModel.state.collectAsStateWithLifecycle() var section by rememberSaveable { mutableStateOf(null) } + // Hoisted so the add flow can refresh the list it returns to. + val accountsViewModel: AccountsViewModel = hiltViewModel() // Inside a sub-screen, system back (button or gesture) returns to the hub // rather than popping the whole Settings destination to the lists overview. @@ -166,6 +178,29 @@ fun SettingsScreen( SlideInSection(visible = section == SettingsSection.Export) { ExportScreen(onBack = { section = SettingsSection.Storage }) } + // Accounts stays composed under Add account, for the same reason Storage + // stays composed under Export: the deeper screen slides over it. + val accountsOpen = section == SettingsSection.Accounts || + section?.parent == SettingsSection.Accounts + SlideInSection(visible = accountsOpen) { + AccountsScreen( + onAddAccount = { section = SettingsSection.AddAccount }, + onBack = { section = null }, + viewModel = accountsViewModel, + ) + } + SlideInSection(visible = section == SettingsSection.AddAccount) { + AddAccountScreen( + onDone = { + // The list stays composed underneath, so nothing re-runs its + // init and OnResume never fires on a section change — without + // this the user returns to the empty state they just left. + accountsViewModel.refresh() + section = SettingsSection.Accounts + }, + onBack = { section = SettingsSection.Accounts }, + ) + } } } @@ -242,6 +277,13 @@ private fun SettingsHub( leading = { CategoryIcon(Icons.Rounded.Storage, ChipAccent.Neutral) }, onClick = { onOpenSection(SettingsSection.Storage) }, ) + GroupedRow( + title = stringResource(R.string.settings_section_accounts), + summary = stringResource(R.string.settings_accounts_subtitle), + position = Position.Middle, + leading = { CategoryIcon(Icons.Rounded.CloudSync, ChipAccent.Tertiary) }, + onClick = { onOpenSection(SettingsSection.Accounts) }, + ) LanguageRow(position = Position.Middle) ReportProblemRow(position = Position.Bottom) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index dd0c5a8..f38c2e2 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -294,4 +294,40 @@ %1$d week before %1$d weeks before + + + Accounts + Sync your tasks with a CalDAV server + No accounts yet + Add a CalDAV account and Agendula keeps your task lists in step with it — Nextcloud, Radicale, Baïkal and anything else that speaks the protocol. + Add an account + Remove account + Remove %1$s? + Its task lists stay on this device and stop syncing. Nothing is deleted from the server. + Remove + Never synced + Last sync didn\u2019t finish + + Add an account + Email address or server address + you@example.com, or https://cloud.example.com + Continue + Start over + + User name + Password + Use an app password if your server offers one — it can be revoked without changing your account password. + Sign in + + Waiting for your browser + Approve Agendula in the browser, then come back here. Your password is never sent to this app — the server issues a separate app password you can revoke at any time. + Open the browser again + Use a password instead + The server sent us to %1$s, but you typed %2$s. That usually means its overwrite.cli.url setting is wrong. + + Which lists should sync? + Read-only + Shared with you + Add account + Pick at least one list 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 new file mode 100644 index 0000000..baa3d76 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt @@ -0,0 +1,361 @@ +package de.jeanlucmakiola.agendula.ui.accounts + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.sync.AccountCreator +import de.jeanlucmakiola.agendula.data.sync.AccountRepository +import de.jeanlucmakiola.agendula.data.sync.CalDavGateway +import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.NextcloudLoginFlow +import de.jeanlucmakiola.caldav.TaskCollection +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrl +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +/** + * The sign-in state machine. + * + * Worth testing closely: it decides where five discovery outcomes lead, when a + * one-shot app password is spent, and which host a credential is scoped to — + * all of which fail quietly, and none of which needs a server to exercise. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class AddAccountViewModelTest { + + private val dispatcher = StandardTestDispatcher() + private val gateway = FakeGateway() + private val creator = FakeCreator() + + @BeforeEach fun setUp() = Dispatchers.setMain(dispatcher) + + @AfterEach fun tearDown() = Dispatchers.resetMain() + + private fun viewModel() = AddAccountViewModel(creator, gateway) + + @Nested + inner class TheAddressStep { + + @Test + fun `Google is refused before any request is made`() = runTest(dispatcher) { + val vm = viewModel() + vm.onServerInputChanged("me@gmail.com") + vm.onServerSubmitted() + advanceUntilIdle() + + // It supports neither VTODO nor MKCALENDAR — its own docs say so — so + // a 401 the user cannot act on is the wrong answer. + assertThat(vm.state.value.fatal).contains("does not support tasks") + assertThat(gateway.discoveries).isEmpty() + } + + @Test + fun `a provider quirk is named while the user is still typing`() = runTest(dispatcher) { + val vm = viewModel() + vm.onServerInputChanged("me@fastmail.com") + assertThat(vm.state.value.quirk).isNotNull() + } + + @Test + fun `an unauthenticated 200 offers sign-in rather than bouncing back`() = + runTest(dispatcher) { + // No credentials were sent, so RFC 5397's is + // not a rejection. Sending the user back to the address field + // would make such a server permanently unreachable. + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.Unauthenticated + gateway.loginFlow = null + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + assertThat(vm.state.value.step) + .isInstanceOf(AddAccountStep.EnterCredentials::class.java) + } + + @Test + fun `a server that is not CalDAV says so on the address step`() = runTest(dispatcher) { + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NotCalDav("no calendar-access") + + val vm = viewModel() + vm.onServerInputChanged("https://example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + val step = vm.state.value.step as AddAccountStep.EnterServer + assertThat(step.error).contains("calendar-access") + // The input survives, so the user can correct it rather than retype it. + assertThat(step.input).isEqualTo("https://example.com/") + } + } + + @Nested + inner class TheBrowserStep { + + @Test + fun `discovery after approval targets the server that issued the credentials`() = + runTest(dispatcher) { + // The host-mismatch case: the user typed one host, the server + // reported another. Probing the typed one with a credential scoped + // to the reported one 401s and burns the one-shot app password. + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication( + listOf("cloud.example.com"), + ) + gateway.loginFlow = flow() + gateway.pollResults += NextcloudLoginFlow.PollResult.Approved( + NextcloudLoginFlow.Credentials( + server = "https://dav.example.com/".toHttpUrl(), + loginName = "me", + appPassword = "app-pw", + ), + ) + gateway.discoveryOutcomes += found(collection("Tasks")) + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + assertThat(gateway.discoveries.last().target).isEqualTo("https://dav.example.com/") + assertThat(gateway.discoveries.last().credentials?.origin.toString()) + .isEqualTo("https://dav.example.com/") + } + + @Test + fun `the login URL is handed to the browser exactly once`() = runTest(dispatcher) { + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList()) + gateway.loginFlow = flow() + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + // runCurrent, not advanceUntilIdle: the poll loop's first act is a + // delay, so this settles discovery without consuming the flow. + runCurrent() + + assertThat(vm.state.value.openInBrowser).isNotNull() + vm.onBrowserLaunched() + assertThat(vm.state.value.openInBrowser).isNull() + vm.onStartOver() + } + + @Test + fun `polling stops on its own rather than running forever`() = runTest(dispatcher) { + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList()) + gateway.loginFlow = flow() + // The fake never approves, so only the loop's own bound ends it. + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + val step = vm.state.value.step as AddAccountStep.WaitingForBrowser + assertThat(step.error).isNotNull() + } + + @Test + fun `a server with no login flow falls back to a password`() = runTest(dispatcher) { + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList()) + gateway.loginFlow = null + + val vm = viewModel() + vm.onServerInputChanged("https://baikal.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + assertThat(vm.state.value.step) + .isInstanceOf(AddAccountStep.EnterCredentials::class.java) + } + } + + @Nested + inner class ChoosingLists { + + @Test + fun `writable lists are pre-ticked and read-only ones are not`() = runTest(dispatcher) { + gateway.discoveryOutcomes += found( + collection("Mine"), + collection("Someone else's", readOnly = true), + ) + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + val step = vm.state.value.step as AddAccountStep.ChooseLists + assertThat(step.selected).hasSize(1) + assertThat(step.selected.single().toString()).contains("Mine") + } + + @Test + fun `an account with no usable lists is a dead end worth explaining`() = + runTest(dispatcher) { + gateway.discoveryOutcomes += found() + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + assertThat(vm.state.value.fatal).contains("no task lists") + } + } + + @Nested + inner class Finishing { + + @Test + fun `starting over clears the previous account's credentials`() = runTest(dispatcher) { + // Without this, a second account is created with the first account's + // username and app password. + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList()) + gateway.loginFlow = null + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + vm.onUsernameChanged("me") + vm.onPasswordChanged("first-pw") + + vm.onStartOver() + + assertThat(vm.state.value).isEqualTo(AddAccountUiState()) + // And a save attempt now has nothing to save, rather than reusing it. + gateway.discoveryOutcomes += found(collection("Tasks")) + vm.onServerInputChanged("https://other.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + vm.onSave() + advanceUntilIdle() + assertThat(creator.created).isEmpty() + assertThat(vm.state.value.step) + .isInstanceOf(AddAccountStep.EnterCredentials::class.java) + } + + @Test + fun `a failure inside create leaves a readable message, not a spinner`() = + runTest(dispatcher) { + creator.thrown = IllegalStateException("UNIQUE constraint failed") + + val vm = signedIn() + vm.onSave() + advanceUntilIdle() + + assertThat(vm.state.value.fatal).contains("UNIQUE constraint failed") + assertThat(vm.state.value.step).isNotInstanceOf(AddAccountStep.Working::class.java) + } + + @Test + fun `a completed add reports the account it created`() = runTest(dispatcher) { + val vm = signedIn() + vm.onSave() + advanceUntilIdle() + + assertThat(creator.created).containsExactly("me@cloud.example.com") + assertThat(vm.state.value.step).isEqualTo(AddAccountStep.Done) + } + } + + /** + * A ViewModel driven the way the screen drives it, up to the list step: + * address → credentials → discovery → lists. Credentials are captured on + * submit, so going straight from the address to a `Found` leaves the + * username and password empty. + */ + private fun kotlinx.coroutines.test.TestScope.signedIn(): AddAccountViewModel { + gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList()) + gateway.loginFlow = null + + val vm = viewModel() + vm.onServerInputChanged("https://cloud.example.com/") + vm.onServerSubmitted() + advanceUntilIdle() + + gateway.discoveryOutcomes += found(collection("Tasks")) + vm.onUsernameChanged("me") + vm.onPasswordChanged("pw") + vm.onCredentialsSubmitted() + advanceUntilIdle() + return vm + } + + // ------------------------------------------------------------- fixtures + + private fun collection(name: String, readOnly: Boolean = false) = TaskCollection( + url = "https://cloud.example.com/dav/$name/".toHttpUrl(), + displayName = name, + color = null, + readOnly = readOnly, + isShared = false, + supportsSyncCollection = true, + maxResourceSize = null, + ) + + private fun found(vararg collections: TaskCollection) = CalDavDiscovery.Outcome.Found( + principal = "https://cloud.example.com/principals/me/".toHttpUrl(), + collections = collections.toList(), + homeSets = listOf("https://cloud.example.com/dav/".toHttpUrl()), + movedTo = null, + crossHostHomeSets = emptyList(), + ) + + private fun flow() = NextcloudLoginFlow.Flow( + loginUrl = "https://cloud.example.com/login/flow".toHttpUrl(), + pollEndpoint = "https://cloud.example.com/login/v2/poll".toHttpUrl(), + pollToken = "token", + deadlineEpochSeconds = Long.MAX_VALUE, + ) + + private class FakeGateway : CalDavGateway { + data class Call(val target: String, val credentials: CalDavGateway.Credentials?) + + val discoveries = mutableListOf() + val discoveryOutcomes = ArrayDeque() + val pollResults = ArrayDeque() + var loginFlow: NextcloudLoginFlow.Flow? = null + + override suspend fun discover( + target: String, + credentials: CalDavGateway.Credentials?, + ): CalDavDiscovery.Outcome { + discoveries += Call(target, credentials) + return discoveryOutcomes.removeFirstOrNull() + ?: CalDavDiscovery.Outcome.Failed("no outcome queued") + } + + override suspend fun startLoginFlow(server: HttpUrl) = loginFlow + + override suspend fun pollLoginFlow(flow: NextcloudLoginFlow.Flow) = + pollResults.removeFirstOrNull() ?: NextcloudLoginFlow.PollResult.Pending + } + + private class FakeCreator : AccountCreator { + val created = mutableListOf() + var thrown: Throwable? = null + + override suspend fun create( + displayName: String, + username: String, + appPassword: String, + found: CalDavDiscovery.Outcome.Found, + selected: Set, + ): AccountRepository.Outcome { + thrown?.let { throw it } + created += displayName + return AccountRepository.Outcome.Created(created.size.toLong()) + } + } +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt index 3cf265b..522d615 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt @@ -60,6 +60,8 @@ class CalDavDiscovery( data class Found( val principal: HttpUrl, val collections: List, + /** Every `calendar-home-set` href, in the order the principal listed them. */ + val homeSets: 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. */ @@ -244,6 +246,7 @@ class CalDavDiscovery( return Outcome.Found( principal = principal, collections = collections.values.toList(), + homeSets = homeSets.distinct(), movedTo = movedTo, crossHostHomeSets = crossHost, failedHomeSets = failures, diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavHttp.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavHttp.kt new file mode 100644 index 0000000..feb43f1 --- /dev/null +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavHttp.kt @@ -0,0 +1,89 @@ +package de.jeanlucmakiola.caldav + +import at.bitfire.dav4jvm.BasicDigestAuthHandler +import at.bitfire.dav4jvm.UrlUtils +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import java.util.concurrent.TimeUnit + +/** + * The HTTP clients the CalDAV layer talks through. + * + * ⚠️ `followRedirects(false)` is mandatory, not a preference: `DavResource` + * requires it and asserts on it. Redirects are followed by hand so a + * HTTPS→HTTP downgrade can be refused and a permanent move can be reported to + * the caller — see `dav/PROVENANCE.md` change 3. + */ +object CalDavHttp { + + /** + * One shared base client, so every derived client reuses its connection pool + * and dispatcher threads. Building a fresh `OkHttpClient` per probe gives + * each its own pool — every rung of the RFC 6764 ladder reopens TLS, and the + * abandoned clients' idle threads live until GC. + */ + private val shared: OkHttpClient by lazy { + OkHttpClient.Builder() + .followRedirects(false) + // A homelab server on the end of a slow link is normal; a hung socket + // is not. Bounded so a killed worker is the exception, not the rule. + .connectTimeout(30, TimeUnit.SECONDS) + .readTimeout(120, TimeUnit.SECONDS) + .writeTimeout(120, TimeUnit.SECONDS) + .build() + } + + /** Discovery before we have credentials, and the Nextcloud login flow. */ + fun anonymous(userAgent: String): OkHttpClient = base(userAgent).build() + + /** + * Authenticated against [origin]'s registrable domain. + * + * Uses the vendored [BasicDigestAuthHandler] rather than a hand-rolled + * interceptor, and it is worth saying why, because a preemptive-Basic + * interceptor is the obvious thing to write and this project wrote one first: + * + * - **It does Digest.** Baïkal defaults to `dav_auth_type = Digest` and OkHttp + * has no Digest support of its own (square/okhttp#205, open for years). + * Baïkal is squarely in the self-hosting audience. + * - **It already sends Basic preemptively over HTTPS**, and only over HTTPS, + * so the extra round trip on every request of a PROPFIND-heavy sync is + * avoided without a second implementation. + * - **It restricts by registrable domain**, which is what a cross-host home + * set needs: iCloud puts the principal on `caldav.icloud.com` and the home + * set on `pNN-caldav.icloud.com`, and an exact-host allowlist refuses the + * second one. + * - It caches which scheme worked, so the challenge is paid once. + */ + fun authenticated( + userAgent: String, + username: String, + password: String, + origin: HttpUrl, + ): OkHttpClient { + val handler = BasicDigestAuthHandler( + // ⚠️ The **registrable** domain, not the host. The handler compares + // its `domain` against UrlUtils.hostToDomain(request host), which + // keeps only the last two labels — so passing "cloud.example.com" + // compares it to "example.com", never matches, and the credential is + // withheld from every request. That is every self-hosted Nextcloud. + domain = UrlUtils.hostToDomain(origin.host), + username = username, + password = password, + // Never preemptively over cleartext. The handler already gates its + // own preemptive path on isHttps; stating it is cheap insurance. + insecurePreemptive = false, + ) + return base(userAgent) + .authenticator(handler) + .addNetworkInterceptor(handler) + .build() + } + + private fun base(userAgent: String) = shared.newBuilder() + .addInterceptor { chain -> + chain.proceed( + chain.request().newBuilder().header("User-Agent", userAgent).build(), + ) + } +} diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt deleted file mode 100644 index e8f0894..0000000 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptor.kt +++ /dev/null @@ -1,57 +0,0 @@ -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/ServiceDiscovery.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt index fdf7339..92a54de 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt @@ -120,6 +120,19 @@ object ServiceDiscovery { if (into.none { it.url == url }) into += Candidate(url, label) } + /** + * The server **root** for [input] — the origin, not the DAV path. + * + * Nextcloud's Login Flow v2 lives at `index.php/login/v2` off the root, so it + * needs this rather than a discovered collection URL. A typed base URL is + * taken as typed; an email address becomes `https://`. + */ + fun serverRootFor(input: String): HttpUrl? { + val trimmed = input.trim() + asBaseUrl(trimmed)?.let { return it } + return domainOf(trimmed)?.let { "https://$it".toHttpUrlOrNull() } + } + /** 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) && diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavHttpTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavHttpTest.kt new file mode 100644 index 0000000..8e4d29d --- /dev/null +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/CalDavHttpTest.kt @@ -0,0 +1,102 @@ +package de.jeanlucmakiola.caldav + +import at.bitfire.dav4jvm.BasicDigestAuthHandler +import com.google.common.truth.Truth.assertThat +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.Request +import org.junit.Test + +class CalDavHttpTest { + + private val origin = "https://cloud.example.com/remote.php/dav/".toHttpUrl() + + @Test + fun `the auth handler is scoped to the registrable domain, not the host`() { + // ⚠️ The handler compares its `domain` against + // UrlUtils.hostToDomain(request host), which keeps only the last two + // labels. Passing the full host means the comparison is + // "cloud.example.com" == "example.com" — never true — and the credential + // is withheld from every single request. That is every self-hosted + // Nextcloud, silently answering 401 forever. + val client = CalDavHttp.authenticated("Agendula", "user", "pw", origin) + val handler = client.networkInterceptors.filterIsInstance().single() + assertThat(handler.domain).isEqualTo("example.com") + } + + @Test + fun `so the credential actually reaches the host it was made for`() { + val client = CalDavHttp.authenticated("Agendula", "user", "pw", origin) + val handler = client.networkInterceptors.filterIsInstance().single() + + val authorised = handler.authenticateRequest( + Request.Builder().url("https://cloud.example.com/remote.php/dav/").build(), + null, + ) + assertThat(authorised?.header("Authorization")).isNotNull() + } + + @Test + fun `and reaches a sibling host in the same domain, which is what iCloud needs`() { + // The principal is on caldav.icloud.com and the home set on + // pNN-caldav.icloud.com. An exact-host allowlist refuses the second. + val client = CalDavHttp.authenticated( + "Agendula", + "user", + "pw", + "https://caldav.icloud.com/".toHttpUrl(), + ) + val handler = client.networkInterceptors.filterIsInstance().single() + + val authorised = handler.authenticateRequest( + Request.Builder().url("https://p42-caldav.icloud.com/1234/calendars/").build(), + null, + ) + assertThat(authorised?.header("Authorization")).isNotNull() + } + + @Test + fun `an unrelated domain gets nothing`() { + val client = CalDavHttp.authenticated("Agendula", "user", "pw", origin) + val handler = client.networkInterceptors.filterIsInstance().single() + + assertThat( + handler.authenticateRequest( + Request.Builder().url("https://evil.example.org/dav/").build(), + null, + ), + ).isNull() + } + + @Test + fun `cleartext never carries a preemptive credential`() { + val client = CalDavHttp.authenticated("Agendula", "user", "pw", origin) + val handler = client.networkInterceptors.filterIsInstance().single() + + assertThat( + handler.authenticateRequest( + Request.Builder().url("http://cloud.example.com/dav/").build(), + null, + )?.header("Authorization"), + ).isNull() + } + + @Test + fun `derived clients share one connection pool`() { + // A fresh OkHttpClient per probe gives each its own pool and dispatcher + // threads: every rung of the discovery ladder reopens TLS, and the + // abandoned clients linger until GC. + val a = CalDavHttp.anonymous("Agendula") + val b = CalDavHttp.authenticated("Agendula", "user", "pw", origin) + assertThat(a.connectionPool).isSameInstanceAs(b.connectionPool) + assertThat(a.dispatcher).isSameInstanceAs(b.dispatcher) + } + + @Test + fun `redirects are never followed automatically`() { + // DavResource requires it and asserts on it: redirects are followed by + // hand so a HTTPS-to-HTTP downgrade can be refused and a permanent move + // reported. + assertThat(CalDavHttp.anonymous("Agendula").followRedirects).isFalse() + assertThat(CalDavHttp.authenticated("Agendula", "u", "p", origin).followRedirects).isFalse() + } +} diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt deleted file mode 100644 index 1c736d5..0000000 --- a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/PreemptiveBasicInterceptorTest.kt +++ /dev/null @@ -1,102 +0,0 @@ -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/docs/SYNC-PLAN.md b/docs/SYNC-PLAN.md index f03dfe6..d7dc8da 100644 --- a/docs/SYNC-PLAN.md +++ b/docs/SYNC-PLAN.md @@ -277,6 +277,40 @@ before chunk 2 is done. ## Chunk 2d — the account-add UI +One flow, one back-stack entry, as a stepper rather than four destinations — the +steps are not independently reachable, and "back" from the browser step means +abandoning a server-side flow rather than popping a screen. It hangs off Settings +using the same sliding-section pattern Storage → Export already uses. + +⚠️ **`PreemptiveBasicInterceptor` was deleted here**, not kept. The vendored +`BasicDigestAuthHandler` already sends Basic preemptively over HTTPS, and it also +does Digest (Baïkal defaults to it and OkHttp has none), caches which scheme +worked, and restricts by *registrable domain* rather than exact host — which is +what a cross-host home set needs, since iCloud puts the principal on +`caldav.icloud.com` and the home set on `pNN-caldav.icloud.com`. Shipping two +implementations of preemptive Basic is the same defect chunk 1 removed when +`ICalendarWriter` stopped carrying its own folding. + +**A seam was added for testability, after the review found eight issues in one +untested ViewModel.** `CalDavGateway` and `AccountCreator` put the network and +the database behind interfaces, so the sign-in state machine — which decides +where five discovery outcomes lead, when a one-shot app password is spent, and +which host a credential is scoped to — is exercised without a server, a +database, a Keystore or an `AccountManager`. + +**Still open, and small:** the system entry point (Settings → Accounts → Add +account → Agendula) refuses with a message pointing at the in-app flow rather +than driving it. Wiring it means `MainActivity` handling +`SyncAuthenticator.ACTION_ADD_ACCOUNT` and answering the +`AccountAuthenticatorResponse`. + +**Done when:** the flow has had an on-device review and an explicit go-ahead — +`CLAUDE.md`'s rule, and this is the case it exists for. + +--- + +## Chunk 2d — the account-add UI (original outline) + **Goal:** the app can add a CalDAV account and list its VTODO collections. No syncing yet. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index f3c027b..0f1f39d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -32,6 +32,8 @@ turbine = "1.2.0" androidxHilt = "1.3.0" # WorkManager: sync runs here, triggered *through* the sync-adapter framework. work = "2.11.2" +# Custom Tabs, for Nextcloud Login Flow v2. +browser = "1.10.0" navigationCompose = "2.9.0" lifecycleCompose = "2.10.0" androidxTestRules = "1.7.0" @@ -134,6 +136,7 @@ androidx-hilt-navigation-compose = { group = "androidx.hilt", name = "hilt-navig androidx-hilt-work = { group = "androidx.hilt", name = "hilt-work", version.ref = "androidxHilt" } androidx-hilt-compiler = { group = "androidx.hilt", name = "hilt-compiler", version.ref = "androidxHilt" } androidx-work-runtime-ktx = { group = "androidx.work", name = "work-runtime-ktx", version.ref = "work" } +androidx-browser = { group = "androidx.browser", name = "browser", version.ref = "browser" } # Navigation-compose (the NavHost / back stack) androidx-navigation-compose = { group = "androidx.navigation", name = "navigation-compose", version.ref = "navigationCompose" }