From 89caf2dcdf2bfe5ab0035db99f0aa52178f30eb8 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 20:18:22 +0200 Subject: [PATCH 01/12] fix(macos): store desktop secrets in Keychain --- .../335-macos-keychain-secret-store.md | 7 + .../nextcloudnative/app/DesktopSecretStore.kt | 219 +++++++++++++++++- .../app/DesktopSecretStoreTest.kt | 140 ++++++++++- 3 files changed, 360 insertions(+), 6 deletions(-) create mode 100644 changes/unreleased/335-macos-keychain-secret-store.md diff --git a/changes/unreleased/335-macos-keychain-secret-store.md b/changes/unreleased/335-macos-keychain-secret-store.md new file mode 100644 index 000000000..4f59caf1a --- /dev/null +++ b/changes/unreleased/335-macos-keychain-secret-store.md @@ -0,0 +1,7 @@ +category: fix +issue: 335 +pull: 430 +platforms: macos +user-facing: yes + +Store desktop login credentials and local Deck draft keys in the user's macOS Keychain instead of requiring the Linux Secret Service command. diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index cc3b7cb0e..615994ed1 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -5,6 +5,7 @@ import com.sun.jna.Native import com.sun.jna.Pointer import com.sun.jna.Structure import com.sun.jna.WString +import com.sun.jna.ptr.IntByReference import com.sun.jna.ptr.PointerByReference import com.sun.jna.win32.StdCallLibrary import com.sun.jna.win32.W32APIOptions @@ -51,20 +52,22 @@ internal class DesktopSecretStoreUnavailableException( ) : IllegalStateException(message, cause) internal enum class DesktopSecretStoreKind { + MacOsKeychain, SecretService, WindowsCredentialManager, } internal fun desktopSecretStoreKind(osName: String = System.getProperty("os.name", "")): DesktopSecretStoreKind = - if (osName.startsWith("Windows", ignoreCase = true)) { - DesktopSecretStoreKind.WindowsCredentialManager - } else { - DesktopSecretStoreKind.SecretService + when { + osName.startsWith("Windows", ignoreCase = true) -> DesktopSecretStoreKind.WindowsCredentialManager + osName.startsWith("Mac", ignoreCase = true) -> DesktopSecretStoreKind.MacOsKeychain + else -> DesktopSecretStoreKind.SecretService } internal fun defaultDesktopSecretStore( osName: String = System.getProperty("os.name", ""), ): DesktopSecretStore = when (desktopSecretStoreKind(osName)) { + DesktopSecretStoreKind.MacOsKeychain -> MacOsKeychainSecretStore() DesktopSecretStoreKind.SecretService -> SecretToolDesktopSecretStore() DesktopSecretStoreKind.WindowsCredentialManager -> WindowsCredentialManagerSecretStore() } @@ -189,6 +192,206 @@ internal class SecretToolDesktopSecretStore( } } +internal class MacOsKeychainSecretStore( + private val api: MacOsKeychainApi = MacOsKeychainApiHolder.instance, + private val releaseItem: (Pointer) -> Unit = MacOsCoreFoundationApiHolder::release, +) : DesktopSecretStore { + override fun load(reference: DesktopSecretReference): ByteArray? { + val secretLength = IntByReference() + val secretData = PointerByReference() + val item = PointerByReference() + val identity = reference.macOsIdentity() + val status = api.SecKeychainFindGenericPassword( + null, + identity.service.size, + identity.service, + identity.account.size, + identity.account, + secretLength, + secretData, + item, + ) + if (status == ERR_SEC_ITEM_NOT_FOUND) return null + checkMacOsKeychainStatus(status, "load") + val size = secretLength.value + val data = secretData.value + val itemPointer = item.value + try { + check(size in 1..MAX_SECRET_BYTES && data != null) { + "macOS Keychain returned an invalid secret size." + } + return data.getByteArray(0, size) + } finally { + if (data != null) api.SecKeychainItemFreeContent(null, data) + if (itemPointer != null) releaseItem(itemPointer) + } + } + + override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { + require(secret.isNotEmpty() && secret.size <= MAX_SECRET_BYTES) + val identity = reference.macOsIdentity() + val item = PointerByReference() + val findStatus = api.SecKeychainFindGenericPassword( + null, + identity.service.size, + identity.service, + identity.account.size, + identity.account, + null, + null, + item, + ) + when (findStatus) { + ERR_SEC_ITEM_NOT_FOUND -> add(identity, secret) + ERR_SEC_SUCCESS -> update(checkNotNull(item.value), secret) + else -> checkMacOsKeychainStatus(findStatus, "find before save") + } + } + + override fun clear(reference: DesktopSecretReference) { + val identity = reference.macOsIdentity() + val item = PointerByReference() + val status = api.SecKeychainFindGenericPassword( + null, + identity.service.size, + identity.service, + identity.account.size, + identity.account, + null, + null, + item, + ) + if (status == ERR_SEC_ITEM_NOT_FOUND) return + checkMacOsKeychainStatus(status, "find before clear") + val itemPointer = checkNotNull(item.value) { "macOS Keychain returned an empty item." } + try { + checkMacOsKeychainStatus(api.SecKeychainItemDelete(itemPointer), "clear") + } finally { + releaseItem(itemPointer) + } + } + + private fun add(identity: MacOsKeychainIdentity, secret: ByteArray) { + val status = api.SecKeychainAddGenericPassword( + null, + identity.service.size, + identity.service, + identity.account.size, + identity.account, + secret.size, + secret, + null, + ) + if (status != ERR_SEC_DUPLICATE_ITEM) { + checkMacOsKeychainStatus(status, "save") + return + } + val item = PointerByReference() + checkMacOsKeychainStatus( + api.SecKeychainFindGenericPassword( + null, + identity.service.size, + identity.service, + identity.account.size, + identity.account, + null, + null, + item, + ), + "find after concurrent save", + ) + update(checkNotNull(item.value), secret) + } + + private fun update(item: Pointer, secret: ByteArray) { + try { + checkMacOsKeychainStatus( + api.SecKeychainItemModifyAttributesAndData(item, null, secret.size, secret), + "update", + ) + } finally { + releaseItem(item) + } + } +} + +internal interface MacOsKeychainApi : com.sun.jna.Library { + fun SecKeychainFindGenericPassword( + keychainOrArray: Pointer?, + serviceNameLength: Int, + serviceName: ByteArray, + accountNameLength: Int, + accountName: ByteArray, + secretLength: IntByReference?, + secretData: PointerByReference?, + itemRef: PointerByReference, + ): Int + + fun SecKeychainAddGenericPassword( + keychain: Pointer?, + serviceNameLength: Int, + serviceName: ByteArray, + accountNameLength: Int, + accountName: ByteArray, + secretLength: Int, + secretData: ByteArray, + itemRef: PointerByReference?, + ): Int + + fun SecKeychainItemModifyAttributesAndData( + itemRef: Pointer, + attributes: Pointer?, + secretLength: Int, + secretData: ByteArray, + ): Int + + fun SecKeychainItemDelete(itemRef: Pointer): Int + + fun SecKeychainItemFreeContent(attributes: Pointer?, secretData: Pointer?): Int +} + +private data class MacOsKeychainIdentity( + val service: ByteArray, + val account: ByteArray, +) + +private fun DesktopSecretReference.macOsIdentity(): MacOsKeychainIdentity = MacOsKeychainIdentity( + service = targetName.encodeToByteArray(), + account = ( + attributes["login"] + ?: attributes["purpose"] + ?: DESKTOP_APPLICATION_ID + ).encodeToByteArray(), +) + +private fun checkMacOsKeychainStatus(status: Int, operation: String) { + if (status == ERR_SEC_SUCCESS) return + val reason = when (status) { + ERR_SEC_AUTH_FAILED -> "Keychain access was denied." + ERR_SEC_INTERACTION_NOT_ALLOWED -> "The login Keychain is locked or unavailable." + else -> "macOS Keychain failed to $operation the desktop secret (error $status)." + } + throw DesktopSecretStoreUnavailableException(reason) +} + +private object MacOsKeychainApiHolder { + val instance: MacOsKeychainApi by lazy { + Native.load(MACOS_SECURITY_FRAMEWORK, MacOsKeychainApi::class.java) + } +} + +private object MacOsCoreFoundationApiHolder { + private val api: MacOsCoreFoundationApi by lazy { + Native.load(MACOS_CORE_FOUNDATION_FRAMEWORK, MacOsCoreFoundationApi::class.java) + } + + fun release(pointer: Pointer) = api.CFRelease(pointer) +} + +private interface MacOsCoreFoundationApi : com.sun.jna.Library { + fun CFRelease(pointer: Pointer) +} + internal class WindowsCredentialManagerSecretStore( private val api: WindowsCredentialApi = WindowsCredentialApiHolder.instance, ) : DesktopSecretStore { @@ -318,6 +521,14 @@ private const val WINDOWS_CREDENTIAL_PREFIX = "Obiente/NextcloudNative" private const val CRED_TYPE_GENERIC = 1 private const val CRED_PERSIST_LOCAL_MACHINE = 2 private const val ERROR_NOT_FOUND = 1_168 +private const val ERR_SEC_SUCCESS = 0 +private const val ERR_SEC_AUTH_FAILED = -25_293 +private const val ERR_SEC_DUPLICATE_ITEM = -25_299 +private const val ERR_SEC_ITEM_NOT_FOUND = -25_300 +private const val ERR_SEC_INTERACTION_NOT_ALLOWED = -25_308 +private const val MACOS_SECURITY_FRAMEWORK = "/System/Library/Frameworks/Security.framework/Security" +private const val MACOS_CORE_FOUNDATION_FRAMEWORK = + "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation" private const val MAX_SECRET_BYTES = 2_560 private const val MISSING_SECRET_TOOL_MESSAGE = "Secure credential storage is unavailable. Install libsecret-tools on Debian or Ubuntu, or libsecret on Fedora or RHEL, then restart Nextcloud Native." diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 66a4c35bf..025fd10fe 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -1,5 +1,9 @@ package dev.obiente.nextcloudnative.app +import com.sun.jna.Memory +import com.sun.jna.Pointer +import com.sun.jna.ptr.IntByReference +import com.sun.jna.ptr.PointerByReference import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.InputStream @@ -122,13 +126,145 @@ class DesktopSecretStoreTest { } @Test - fun platformSelectionUsesWindowsCredentialManagerOnlyOnWindows() { + fun platformSelectionUsesEachNativeCredentialStore() { assertEquals( DesktopSecretStoreKind.WindowsCredentialManager, desktopSecretStoreKind("Windows 11"), ) assertEquals(DesktopSecretStoreKind.SecretService, desktopSecretStoreKind("Linux")) - assertEquals(DesktopSecretStoreKind.SecretService, desktopSecretStoreKind("Mac OS X")) + assertEquals(DesktopSecretStoreKind.MacOsKeychain, desktopSecretStoreKind("Mac OS X")) + } + + @Test + fun macOsKeychainAddsUpdatesLoadsAndClearsWithoutPuttingSecretsInIdentityFields() { + val api = FakeMacOsKeychainApi() + val releasedItems = mutableListOf() + val store = MacOsKeychainSecretStore(api, releasedItems::add) + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val first = "first-synthetic-secret".encodeToByteArray() + val second = "second-synthetic-secret".encodeToByteArray() + + assertNull(store.load(reference)) + store.save(reference, "alice", first) + assertContentEquals(first, store.load(reference)) + store.save(reference, "alice", second) + assertContentEquals(second, store.load(reference)) + store.clear(reference) + assertNull(store.load(reference)) + + assertEquals(reference.targetName, api.lastService) + assertEquals("alice", api.lastAccount) + assertFalse(api.lastService.orEmpty().contains("cloud.invalid")) + assertFalse(api.lastService.orEmpty().contains("alice")) + assertTrue(releasedItems.isNotEmpty()) + } + + @Test + fun macOsKeychainDenialIsActionableAndDoesNotExposeCredentialIdentity() { + val store = MacOsKeychainSecretStore( + api = FakeMacOsKeychainApi(findFailure = -25_293), + releaseItem = {}, + ) + val reference = desktopSessionSecretReference("https://private.invalid", "synthetic-user") + + val failure = assertFailsWith { + store.load(reference) + } + + assertTrue(failure.message.orEmpty().contains("denied")) + assertFalse(failure.message.orEmpty().contains("private.invalid")) + assertFalse(failure.message.orEmpty().contains("synthetic-user")) + } + + @Test + fun macOsKeychainConcurrentAddRaceUpdatesTheExistingItem() { + val api = FakeMacOsKeychainApi(duplicateOnFirstAdd = true) + val store = MacOsKeychainSecretStore(api, releaseItem = {}) + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "replacement-synthetic-secret".encodeToByteArray() + + store.save(reference, "alice", expected) + + assertContentEquals(expected, store.load(reference)) + } + + private class FakeMacOsKeychainApi( + private val findFailure: Int? = null, + private val duplicateOnFirstAdd: Boolean = false, + ) : MacOsKeychainApi { + private var secret: ByteArray? = null + private var addAttempted = false + private var returnedSecret: Memory? = null + private val item = Memory(1) + var lastService: String? = null + private set + var lastAccount: String? = null + private set + + override fun SecKeychainFindGenericPassword( + keychainOrArray: Pointer?, + serviceNameLength: Int, + serviceName: ByteArray, + accountNameLength: Int, + accountName: ByteArray, + secretLength: IntByReference?, + secretData: PointerByReference?, + itemRef: PointerByReference, + ): Int { + lastService = serviceName.copyOf(serviceNameLength).decodeToString() + lastAccount = accountName.copyOf(accountNameLength).decodeToString() + findFailure?.let { return it } + val stored = secret ?: return -25_300 + if (secretLength != null && secretData != null) { + returnedSecret = Memory(stored.size.toLong()).also { memory -> + memory.write(0, stored, 0, stored.size) + secretData.value = memory + } + secretLength.value = stored.size + } + itemRef.value = item + return 0 + } + + override fun SecKeychainAddGenericPassword( + keychain: Pointer?, + serviceNameLength: Int, + serviceName: ByteArray, + accountNameLength: Int, + accountName: ByteArray, + secretLength: Int, + secretData: ByteArray, + itemRef: PointerByReference?, + ): Int { + if (duplicateOnFirstAdd && !addAttempted) { + addAttempted = true + secret = "concurrent-synthetic-secret".encodeToByteArray() + return -25_299 + } + secret = secretData.copyOf(secretLength) + return 0 + } + + override fun SecKeychainItemModifyAttributesAndData( + itemRef: Pointer, + attributes: Pointer?, + secretLength: Int, + secretData: ByteArray, + ): Int { + secret = secretData.copyOf(secretLength) + return 0 + } + + override fun SecKeychainItemDelete(itemRef: Pointer): Int { + secret = null + return 0 + } + + override fun SecKeychainItemFreeContent(attributes: Pointer?, secretData: Pointer?): Int { + returnedSecret?.clear() + returnedSecret = null + return 0 + } } @Test From 74f6daa413e147b75e269dbcaf18f9643176e11e Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 20:47:13 +0200 Subject: [PATCH 02/12] fix(macos): migrate credentials and recover locked storage --- PLATFORMS.md | 4 +- README.md | 15 +-- .../335-macos-keychain-secret-store.md | 2 +- .../nextcloudnative/app/NextcloudNativeApp.kt | 44 ++----- .../app/NextcloudSessionLoading.kt | 24 ++++ .../app/NextcloudStatusMessages.kt | 57 +++++++++ .../app/NextcloudSessionLoadingTest.kt | 41 +++++++ .../nextcloudnative/app/DesktopSecretStore.kt | 87 +++++++++++-- .../app/DesktopSecretStoreTest.kt | 115 +++++++++++++++++- .../NextcloudSessionLoadingInteractionTest.kt | 25 ++++ 10 files changed, 363 insertions(+), 51 deletions(-) create mode 100644 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt create mode 100644 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt create mode 100644 ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt create mode 100644 ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt diff --git a/PLATFORMS.md b/PLATFORMS.md index ea6184a11..80df0bb67 100644 --- a/PLATFORMS.md +++ b/PLATFORMS.md @@ -4,7 +4,7 @@ This document defines the platform boundary for Nextcloud Native. It separates portable product behavior from operating-system integration so shared code does not erase native security, lifecycle, accessibility, or filesystem semantics. -**Last reviewed: 2026-08-20.** Implementation and release availability may +**Last reviewed: 2026-09-01.** Implementation and release availability may have changed. The [GitHub Releases page](https://github.com/Obiente/nc-native/releases) is the source of truth for published artifacts and their limitations. @@ -28,7 +28,7 @@ its platform acceptance criteria pass and its limitations are documented. | Android | Compose Multiplatform | Active launcher and signed alpha APK/AAB | Keystore, WorkManager, DocumentsProvider, permissions, notifications, shares, media sessions, camera backup, and calls | | Linux | Compose Desktop | Primary interactive desktop target; alpha RPM/DEB | Secret Service, desktop file integration, notifications, media keys, portals, and conventional sync roots | | Windows | Compose Desktop | Unsigned x86-64 MSI with Credential Manager, attested builds, and Cloud Files integration under prerelease qualification | Explorer validation, free trusted signing when available, notifications, media controls, and updates | -| macOS | Compose Desktop | Early DMG packaging artifact; no supported authenticated login yet | Keychain, File Provider/Finder integration, notifications, media controls, and updates | +| macOS | Compose Desktop | Early DMG packaging artifact; Keychain storage is source-tested, but authenticated use has not been live-validated or qualified | Keychain, File Provider/Finder integration, notifications, media controls, and updates | | iOS / iPadOS | Planned Compose target | No supported launcher is shipped | Keychain, File Provider, background transfer, share extension, notifications, media, and CallKit | Packaging is not feature parity. A platform becomes supported for a workflow diff --git a/README.md b/README.md index cc83889af..22044cb1e 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ boundaries. ## Implemented alpha surfaces -**Last reviewed: 2026-08-20.** Repository implementation may have changed. The +**Last reviewed: 2026-09-01.** Repository implementation may have changed. The [default branch](https://github.com/Obiente/nc-native/tree/main) is the source of truth for current code. A listed surface can still have platform, version, action, or lifecycle limitations and is not a shipped-support guarantee. @@ -167,8 +167,8 @@ action, or lifecycle limitations and is not a shipped-support guarantee. The repository already contains runnable Android and Linux desktop applications with: -- Nextcloud Login Flow v2 with Android Keystore and Linux Secret Service - credential storage; +- Nextcloud Login Flow v2 with Android Keystore, Linux Secret Service, Windows + Credential Manager, and source-tested macOS Keychain credential storage; - authenticated native Files browsing, list/grid layouts, previews, sharing foundations, text editing, and media viewing; - Photos and Memories collections, albums, tags, people, favorites, RAW/JPEG @@ -220,7 +220,7 @@ The dependency gates and data-safety criteria are in ## Platform status -**Last reviewed: 2026-08-20.** Platform availability may have changed. The +**Last reviewed: 2026-09-01.** Platform availability may have changed. The [GitHub Releases page](https://github.com/Obiente/nc-native/releases) is the source of truth for published artifacts and limitations. This table is not a stable-support guarantee. @@ -230,7 +230,7 @@ stable-support guarantee. | Android | Active application target with signed APK/AAB prereleases; hosted CI covers unit tests and packaging, while connected-device instrumentation remains separate | | Linux | Primary interactive desktop development target, distributable plus RPM/DEB prereleases | | Windows | x86-64 MSI, native Credential Manager login storage, and Cloud Files sync under active prerelease qualification | -| macOS | Early DMG packaging artifact; native Keychain login storage and supported authenticated use are not implemented yet | +| macOS | Early DMG packaging artifact; native Keychain storage is covered by deterministic source tests, but authenticated use has not been live-validated or qualified | | iOS / iPadOS | Planned platform target; no supported launcher is shipped yet | Android and desktop already share domain models, semantic components, and @@ -278,8 +278,9 @@ Android release artifacts are signed with the project's protected release key. Desktop packages are provided per successful platform build. Windows MSI packages use native Credential Manager storage, include keyless GitHub build provenance, and are currently unsigned, so SmartScreen may require choosing -`More info > Run anyway`. macOS packages still prove packaging only and do not -yet have native Keychain login integration. +`More info > Run anyway`. The source includes deterministically tested macOS +Keychain integration, but the macOS package remains a packaging artifact until +authenticated use passes a live macOS acceptance run. Read each release's known limitations before installing over an existing test build. diff --git a/changes/unreleased/335-macos-keychain-secret-store.md b/changes/unreleased/335-macos-keychain-secret-store.md index 4f59caf1a..4e51300e4 100644 --- a/changes/unreleased/335-macos-keychain-secret-store.md +++ b/changes/unreleased/335-macos-keychain-secret-store.md @@ -4,4 +4,4 @@ pull: 430 platforms: macos user-facing: yes -Store desktop login credentials and local Deck draft keys in the user's macOS Keychain instead of requiring the Linux Secret Service command. +Store desktop login credentials and local Deck draft keys in the user's macOS Keychain, migrate existing Secret Service values, and keep locked Keychain access safely retryable. diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt index 7a8b43e39..f762ea660 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt @@ -534,15 +534,21 @@ fun NextcloudNativeApp( NextcloudNativeTheme(darkTheme = darkTheme) { NextcloudAppBackground { - var session by remember { mutableStateOf(services.loadSession()) } - if (session == null) { + var sessionLoadAttempt by remember { mutableStateOf(0) } + val sessionLoad = remember(services, sessionLoadAttempt) { + loadNextcloudSessionSafely(services::loadSession) + } + var session by remember(services, sessionLoadAttempt) { + mutableStateOf((sessionLoad as? NextcloudSessionLoadState.Loaded)?.session) + } + if (sessionLoad == NextcloudSessionLoadState.SecureStorageUnavailable) { + SecureSessionStorageUnavailable(onRetry = { sessionLoadAttempt += 1 }) + } else if (session == null) { if (pendingAppUpdateReviewRequest != null) { LoggedOutAppUpdateReviewScreen( services = services, platformCapabilityRefreshRequest = platformCapabilityRefreshRequest, - onContinueToSignIn = { - handledAppUpdateReviewRequest = pendingAppUpdateReviewRequest - }, + onContinueToSignIn = { handledAppUpdateReviewRequest = pendingAppUpdateReviewRequest }, ) } else { LoginScreen( @@ -12367,34 +12373,6 @@ internal fun SectionTitle(text: String, modifier: Modifier = Modifier) { Text(text, modifier = modifier, style = MaterialTheme.typography.titleLarge, color = MaterialTheme.colorScheme.primary) } -@Composable -internal fun LoadingMessage(message: String) { - Column( - modifier = Modifier.fillMaxSize(), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - CircularProgressIndicator() - Text(message, modifier = Modifier.padding(top = NextcloudSpacing.Large)) - } -} - -@Composable -internal fun EmptyMessage(message: String) { - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(message, modifier = Modifier.padding(NextcloudSpacing.XLarge), color = MaterialTheme.colorScheme.onSurfaceVariant) - } -} - -@Composable -internal fun ErrorMessage(message: String, onRetry: (() -> Unit)? = null) { - Column(modifier = Modifier.padding(NextcloudSpacing.XLarge), verticalArrangement = Arrangement.spacedBy(12.dp)) { - Icon(NextcloudIcons.Error, contentDescription = null, tint = MaterialTheme.colorScheme.error) - Text(message, color = MaterialTheme.colorScheme.error) - onRetry?.let { retry -> OutlinedButton(onClick = retry) { Text("Try again") } } - } -} - @Composable private fun RetainedRefreshError(message: String, onRetry: () -> Unit, modifier: Modifier = Modifier) { RetainedContentNotice(message, onRetry, modifier.padding(horizontal = NextcloudSpacing.Large, vertical = NextcloudSpacing.Small)) diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt new file mode 100644 index 000000000..7cceb5703 --- /dev/null +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt @@ -0,0 +1,24 @@ +package dev.obiente.nextcloudnative.app + +import kotlinx.coroutines.CancellationException + +internal open class NextcloudSessionStorageUnavailableException( + message: String, + cause: Throwable? = null, +) : IllegalStateException(message, cause) + +internal sealed interface NextcloudSessionLoadState { + data class Loaded(val session: NextcloudSession?) : NextcloudSessionLoadState + + data object SecureStorageUnavailable : NextcloudSessionLoadState +} + +internal fun loadNextcloudSessionSafely( + loadSession: () -> NextcloudSession?, +): NextcloudSessionLoadState = try { + NextcloudSessionLoadState.Loaded(loadSession()) +} catch (failure: CancellationException) { + throw failure +} catch (_: NextcloudSessionStorageUnavailableException) { + NextcloudSessionLoadState.SecureStorageUnavailable +} diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt new file mode 100644 index 000000000..c4834e9d3 --- /dev/null +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt @@ -0,0 +1,57 @@ +package dev.obiente.nextcloudnative.app + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedButton +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import dev.obiente.nextcloudnative.app.design.NextcloudIcons +import dev.obiente.nextcloudnative.app.design.NextcloudSpacing + +@Composable +internal fun LoadingMessage(message: String) { + Column( + modifier = Modifier.fillMaxSize(), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + CircularProgressIndicator() + Text(message, modifier = Modifier.padding(top = NextcloudSpacing.Large)) + } +} + +@Composable +internal fun EmptyMessage(message: String) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(message, modifier = Modifier.padding(NextcloudSpacing.XLarge), color = MaterialTheme.colorScheme.onSurfaceVariant) + } +} + +@Composable +internal fun ErrorMessage(message: String, onRetry: (() -> Unit)? = null) { + Column(modifier = Modifier.padding(NextcloudSpacing.XLarge), verticalArrangement = Arrangement.spacedBy(12.dp)) { + Icon(NextcloudIcons.Error, contentDescription = null, tint = MaterialTheme.colorScheme.error) + Text(message, color = MaterialTheme.colorScheme.error) + onRetry?.let { retry -> OutlinedButton(onClick = retry) { Text("Try again") } } + } +} + +@Composable +internal fun SecureSessionStorageUnavailable(onRetry: () -> Unit) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + ErrorMessage( + "Secure session storage is locked or unavailable. Unlock it or allow " + + "Nextcloud Native access, then try again.", + onRetry, + ) + } +} diff --git a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt new file mode 100644 index 000000000..90ccc0cb8 --- /dev/null +++ b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt @@ -0,0 +1,41 @@ +package dev.obiente.nextcloudnative.app + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertIs +import kotlinx.coroutines.CancellationException + +class NextcloudSessionLoadingTest { + @Test + fun secureStorageFailureBecomesRetryableWithoutExposingItsMessage() { + var attempts = 0 + val expected = NextcloudSession("https://cloud.invalid", "alice", "synthetic-secret") + val load = { + attempts += 1 + if (attempts == 1) throw NextcloudSessionStorageUnavailableException("private provider failure") + expected + } + + assertEquals( + NextcloudSessionLoadState.SecureStorageUnavailable, + loadNextcloudSessionSafely(load), + ) + val recovered = assertIs(loadNextcloudSessionSafely(load)) + assertEquals(expected, recovered.session) + } + + @Test + fun cancellationRemainsControlFlow() { + assertFailsWith { + loadNextcloudSessionSafely { throw CancellationException("cancelled") } + } + } + + @Test + fun unrelatedProgrammingFailureIsNotPresentedAsUnavailableStorage() { + assertFailsWith { + loadNextcloudSessionSafely { error("synthetic invariant failure") } + } + } +} diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index 615994ed1..11378726d 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -13,6 +13,7 @@ import java.security.MessageDigest import java.util.concurrent.Executors import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException +import java.util.prefs.Preferences internal data class DesktopSecretReference( val targetName: String, @@ -49,7 +50,7 @@ internal interface DesktopSecretStore { internal class DesktopSecretStoreUnavailableException( message: String, cause: Throwable? = null, -) : IllegalStateException(message, cause) +) : NextcloudSessionStorageUnavailableException(message, cause) internal enum class DesktopSecretStoreKind { MacOsKeychain, @@ -67,11 +68,84 @@ internal fun desktopSecretStoreKind(osName: String = System.getProperty("os.name internal fun defaultDesktopSecretStore( osName: String = System.getProperty("os.name", ""), ): DesktopSecretStore = when (desktopSecretStoreKind(osName)) { - DesktopSecretStoreKind.MacOsKeychain -> MacOsKeychainSecretStore() + DesktopSecretStoreKind.MacOsKeychain -> MigratingDesktopSecretStore( + primary = MacOsKeychainSecretStore(), + legacy = SecretToolDesktopSecretStore(), + adoption = PreferencesDesktopSecretStoreAdoption(), + ) DesktopSecretStoreKind.SecretService -> SecretToolDesktopSecretStore() DesktopSecretStoreKind.WindowsCredentialManager -> WindowsCredentialManagerSecretStore() } +internal interface DesktopSecretStoreAdoption { + fun isAdopted(reference: DesktopSecretReference): Boolean + + fun markAdopted(reference: DesktopSecretReference) +} + +internal class MigratingDesktopSecretStore( + private val primary: DesktopSecretStore, + private val legacy: DesktopSecretStore, + private val adoption: DesktopSecretStoreAdoption, +) : DesktopSecretStore { + override fun load(reference: DesktopSecretReference): ByteArray? { + primary.load(reference)?.let { secret -> + adoptAndClearLegacyOnce(reference) + return secret + } + if (adoption.isAdopted(reference)) return null + val secret = legacy.load(reference) ?: return null + primary.save(reference, username = null, secret = secret) + adoptAndClearLegacyOnce(reference) + return secret + } + + override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { + primary.save(reference, username, secret) + adoptAndClearLegacyOnce(reference) + } + + override fun clear(reference: DesktopSecretReference) { + val alreadyAdopted = adoption.isAdopted(reference) + if (!alreadyAdopted) adoption.markAdopted(reference) + primary.clear(reference) + if (!alreadyAdopted) clearLegacyBestEffort(reference) + } + + private fun adoptAndClearLegacyOnce(reference: DesktopSecretReference) { + if (adoption.isAdopted(reference)) return + adoption.markAdopted(reference) + clearLegacyBestEffort(reference) + } + + private fun clearLegacyBestEffort(reference: DesktopSecretReference) { + try { + legacy.clear(reference) + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + // The durable adoption marker prevents a stale legacy value from being read again. + } + } +} + +private class PreferencesDesktopSecretStoreAdoption( + private val preferences: Preferences = Preferences.userRoot() + .node("dev/obiente/nextcloudnative/secret-store-adoption-v1"), +) : DesktopSecretStoreAdoption { + override fun isAdopted(reference: DesktopSecretReference): Boolean = + preferences.getBoolean(reference.adoptionKey(), false) + + override fun markAdopted(reference: DesktopSecretReference) { + preferences.putBoolean(reference.adoptionKey(), true) + preferences.flush() + } + + private fun DesktopSecretReference.adoptionKey(): String = MessageDigest.getInstance("SHA-256") + .digest(targetName.encodeToByteArray()) + .toHexString() +} + internal fun desktopSessionSecretReference(serverUrl: String, loginName: String): DesktopSecretReference { require(serverUrl.isNotBlank() && loginName.isNotBlank()) val identity = MessageDigest.getInstance("SHA-256") @@ -357,11 +431,10 @@ private data class MacOsKeychainIdentity( private fun DesktopSecretReference.macOsIdentity(): MacOsKeychainIdentity = MacOsKeychainIdentity( service = targetName.encodeToByteArray(), - account = ( - attributes["login"] - ?: attributes["purpose"] - ?: DESKTOP_APPLICATION_ID - ).encodeToByteArray(), + account = MessageDigest.getInstance("SHA-256") + .digest(targetName.encodeToByteArray()) + .toHexString() + .encodeToByteArray(), ) private fun checkMacOsKeychainStatus(status: Int, operation: String) { diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 025fd10fe..a2eb00107 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -153,12 +153,91 @@ class DesktopSecretStoreTest { assertNull(store.load(reference)) assertEquals(reference.targetName, api.lastService) - assertEquals("alice", api.lastAccount) + assertEquals(64, api.lastAccount?.length) + assertFalse(api.lastAccount.orEmpty().contains("alice")) assertFalse(api.lastService.orEmpty().contains("cloud.invalid")) assertFalse(api.lastService.orEmpty().contains("alice")) assertTrue(releasedItems.isNotEmpty()) } + @Test + fun existingSecretServiceSessionsAndDraftKeysMigrateBeforeKeychainAdoption() { + val sessionReference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val draftReference = desktopDeckDraftSecretReference() + val legacy = RecordingSecretStore( + mutableMapOf( + sessionReference.targetName to "session-secret".encodeToByteArray(), + draftReference.targetName to "draft-secret".encodeToByteArray(), + ), + ) + val primary = RecordingSecretStore() + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + assertContentEquals("session-secret".encodeToByteArray(), store.load(sessionReference)) + assertContentEquals("draft-secret".encodeToByteArray(), store.load(draftReference)) + + assertContentEquals("session-secret".encodeToByteArray(), primary.load(sessionReference)) + assertContentEquals("draft-secret".encodeToByteArray(), primary.load(draftReference)) + assertNull(legacy.load(sessionReference)) + assertNull(legacy.load(draftReference)) + assertTrue(adoption.isAdopted(sessionReference)) + assertTrue(adoption.isAdopted(draftReference)) + } + + @Test + fun adoptedKeychainReferenceNeverResurrectsAStaleLegacySecret() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val stale = "stale-session-secret".encodeToByteArray() + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to stale), + ignoreClear = true, + ) + val primary = RecordingSecretStore() + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + store.save(reference, "alice", "current-session-secret".encodeToByteArray()) + legacy.values[reference.targetName] = stale + store.clear(reference) + + assertNull(store.load(reference)) + assertContentEquals(stale, legacy.values.getValue(reference.targetName)) + } + + @Test + fun failedKeychainMigrationLeavesTheLegacySecretRetryable() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "legacy-session-secret".encodeToByteArray() + val legacy = RecordingSecretStore(mutableMapOf(reference.targetName to expected)) + val primary = RecordingSecretStore(failSave = true) + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + assertFailsWith { store.load(reference) } + + assertContentEquals(expected, legacy.load(reference)) + assertFalse(adoption.isAdopted(reference)) + } + + @Test + fun throwingLegacyCleanupCannotBlockAnAdoptedKeychainValue() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "keychain-session-secret".encodeToByteArray() + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "legacy-session-secret".encodeToByteArray()), + failClear = true, + ) + val primary = RecordingSecretStore(mutableMapOf(reference.targetName to expected)) + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + assertContentEquals(expected, store.load(reference)) + assertContentEquals(expected, store.load(reference)) + assertEquals(1, legacy.clearAttempts) + assertTrue(adoption.isAdopted(reference)) + } + @Test fun macOsKeychainDenialIsActionableAndDoesNotExposeCredentialIdentity() { val store = MacOsKeychainSecretStore( @@ -267,6 +346,40 @@ class DesktopSecretStoreTest { } } + private class RecordingSecretStore( + val values: MutableMap = mutableMapOf(), + private val failSave: Boolean = false, + private val ignoreClear: Boolean = false, + private val failClear: Boolean = false, + ) : DesktopSecretStore { + var clearAttempts = 0 + private set + + override fun load(reference: DesktopSecretReference): ByteArray? = + values[reference.targetName]?.copyOf() + + override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { + if (failSave) throw DesktopSecretStoreUnavailableException("Synthetic unavailable store.") + values[reference.targetName] = secret.copyOf() + } + + override fun clear(reference: DesktopSecretReference) { + clearAttempts += 1 + if (failClear) error("Synthetic legacy cleanup failure.") + if (!ignoreClear) values.remove(reference.targetName) + } + } + + private class RecordingSecretStoreAdoption : DesktopSecretStoreAdoption { + private val adopted = mutableSetOf() + + override fun isAdopted(reference: DesktopSecretReference): Boolean = reference.targetName in adopted + + override fun markAdopted(reference: DesktopSecretReference) { + adopted += reference.targetName + } + } + @Test fun sessionCredentialTargetIsStableScopedAndDoesNotExposeAccountDetails() { val first = desktopSessionSecretReference("https://cloud.invalid", "alice") diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt new file mode 100644 index 000000000..eac5850da --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt @@ -0,0 +1,25 @@ +package dev.obiente.nextcloudnative.app + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class NextcloudSessionLoadingInteractionTest { + @Test + fun unavailableSecureStorageExplainsRecoveryAndOffersRetry() { + var retries = 0 + + nativeSceneTest(390, 844, content = { + SecureSessionStorageUnavailable(onRetry = { retries += 1 }) + }) { + assertTrue( + has( + "Secure session storage is locked or unavailable. Unlock it or allow " + + "Nextcloud Native access, then try again.", + ), + ) + click("Try again") + assertEquals(1, retries) + } + } +} From 266fc9452d82dc37d6f0fa81218ff5138ba6c1bf Mon Sep 17 00:00:00 2001 From: "obiente-automations[bot]" <311907242+obiente-automations[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:51:45 +0000 Subject: [PATCH 03/12] chore(website): refresh marketing captures --- website/public/screenshots/capture-manifest.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/website/public/screenshots/capture-manifest.json b/website/public/screenshots/capture-manifest.json index c1fdcaa32..6f1e32dda 100644 --- a/website/public/screenshots/capture-manifest.json +++ b/website/public/screenshots/capture-manifest.json @@ -208,6 +208,8 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudResponseLimits.kt", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesApi.kt", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesFolderOperations.kt", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/OfficeDocumentWorkflow.kt", @@ -600,13 +602,15 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudLinkRouting.kt": "5b29a90b69bb32aba118ef6c8b3f9d6eb26c03835823119b4a0f5bb1c1f4cb17", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt": "fb424bf8979ac292ef30daba64e905e6f5123066cda793aa0244907e11b465c9", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewerActions.kt": "48aaed6948d1423113d300cc3ab86d244ab76e8225e24988e9855edf74553944", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "c7966b9c1a6fc385cbbf239cb73059411ca5dda8a01ea23d7e54f17fab464d7c", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "9a3c4910e987d8c0b916d5b3b7e66db302fa0218e19968521dbfedb8695ca762", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt": "14d43a632afa7c5bf970d90d1387285624182be00569b57c0955670de017cc6c", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotesCache.kt": "9223dd455c6a1c35a10769616fceb9dfb2d92ef4d43b0a52c2c29e40f1a196cf", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPeople.kt": "cff910ea2cc77211ef81779c49ee0c957851f2b4a3ed32b857b12ded1cee643b", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt": "34d9b43cf3bbfc8342958dc40f2df7b4573f30a2ae1bd9bcb8bb470151313a3d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt": "058a09681e46b10dd0145dac4ca16df5491ba4546ac8b319acf7c29076804b51", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudResponseLimits.kt": "fead6cbf4f723ea46f99c9155b190967205d24f198687b7bfd5a213c474374b8", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt": "70d3404aa4ef5a6617590eb1abb8db35d4959cb3323d65d4156c787a7d43acf9", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt": "72e1ef943f0cf8022c69c105fd19370f12df36b000906a8937f89f079ff7b981", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesApi.kt": "e3c685b02592ea791b0c9a478a098fc5bf428893aa4764110c793a78040f341e", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesFolderOperations.kt": "db6a04cfcd3b25b17c996cdc6dba21847d14f5970e06e6c24a3e2c85139dbe1d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/OfficeDocumentWorkflow.kt": "48057ebf53e45a042c4283aaff9a2ca5c3bb46fff3356e962540409f8c7b3b04", From 777213be6a939c9428c3ac142b675a0d74f510c5 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 21:25:35 +0200 Subject: [PATCH 04/12] fix(macos): harden secret migration recovery --- .../app/DesktopDeckCardDraftStore.kt | 15 +- .../app/DesktopNextcloudServices.kt | 18 +- .../nextcloudnative/app/DesktopSecretStore.kt | 135 +++++++++++--- .../app/DesktopSyncLifecycleRecovery.kt | 21 +++ .../app/DesktopSecretStoreTest.kt | 170 ++++++++++++++++-- .../app/DesktopSyncLifecycleRecoveryTest.kt | 28 +++ 6 files changed, 336 insertions(+), 51 deletions(-) create mode 100644 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt create mode 100644 ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt index 9732b0e3b..62e17e414 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt @@ -23,7 +23,11 @@ import org.json.JSONObject */ internal class DesktopDeckCardDraftStore( private val root: File = desktopDeckDraftDirectory(), - private val keyProvider: DesktopDeckDraftKeyProvider = PlatformDeckDraftKeyProvider(), + private val keyProvider: DesktopDeckDraftKeyProvider = PlatformDeckDraftKeyProvider( + legacySecretRequired = { + root.listFiles().orEmpty().any { file -> file.name.matches(DRAFT_FILE_PATTERN) } + }, + ), private val nowEpochMillis: () -> Long = System::currentTimeMillis, private val random: SecureRandom = SecureRandom(), ) { @@ -299,6 +303,7 @@ internal fun interface DesktopDeckDraftKeyProvider { internal class PlatformDeckDraftKeyProvider( private val secretStore: DesktopSecretStore = defaultDesktopSecretStore(), private val random: SecureRandom = SecureRandom(), + private val legacySecretRequired: () -> Boolean = { true }, ) : DesktopDeckDraftKeyProvider { @Volatile private var cached: ByteArray? = null @@ -326,7 +331,13 @@ internal class PlatformDeckDraftKeyProvider( } private fun lookup(): ByteArray? { - val encoded = secretStore.load(desktopDeckDraftSecretReference()) + val stored = try { + secretStore.load(desktopDeckDraftSecretReference()) + } catch (failure: DesktopSecretStoreUnavailableException) { + if (legacySecretRequired()) throw failure + null + } + val encoded = stored ?.let { value -> value.copyOf(minOf(value.size, MAX_ENCODED_KEY_BYTES)) } ?.decodeToString() ?.trim() diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index 84b37cd92..fb0adb024 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -1682,15 +1682,15 @@ class DesktopNextcloudServices( if (!isFileSyncPaused()) { runCatching { syncAllFileSyncPairs(DesktopFileSyncRunSource.Background) } } - val virtualFolderSession = loadSession() - runCatching { reconcileConfiguredVirtualFolders(virtualFolderSession) } - .onFailure { failure -> - publishFileSyncRunFailure( - virtualFolderSession?.let(::desktopFileCacheAccountId), - DesktopFileSyncRunSource.Background, - failure, - ) - } + reconcileDesktopBackgroundSession( + ::loadSession, ::reconcileConfiguredVirtualFolders, + ) { session, failure -> + publishFileSyncRunFailure( + session?.let(::desktopFileCacheAccountId), + DesktopFileSyncRunSource.Background, + failure, + ) + } delay(DESKTOP_FILE_SYNC_INTERVAL_MILLIS) } } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index 11378726d..8e7319cb0 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -78,9 +78,17 @@ internal fun defaultDesktopSecretStore( } internal interface DesktopSecretStoreAdoption { - fun isAdopted(reference: DesktopSecretReference): Boolean + fun state(reference: DesktopSecretReference): DesktopSecretStoreAdoptionState fun markAdopted(reference: DesktopSecretReference) + + fun markLegacyCleanupComplete(reference: DesktopSecretReference) +} + +internal enum class DesktopSecretStoreAdoptionState { + NotAdopted, + AdoptedPendingLegacyCleanup, + AdoptedAndClean, } internal class MigratingDesktopSecretStore( @@ -90,41 +98,48 @@ internal class MigratingDesktopSecretStore( ) : DesktopSecretStore { override fun load(reference: DesktopSecretReference): ByteArray? { primary.load(reference)?.let { secret -> - adoptAndClearLegacyOnce(reference) + adoptAndRetryLegacyCleanup(reference) return secret } - if (adoption.isAdopted(reference)) return null + if (adoption.state(reference) != DesktopSecretStoreAdoptionState.NotAdopted) { + retryLegacyCleanup(reference) + return null + } val secret = legacy.load(reference) ?: return null primary.save(reference, username = null, secret = secret) - adoptAndClearLegacyOnce(reference) + adoptAndRetryLegacyCleanup(reference) return secret } override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { primary.save(reference, username, secret) - adoptAndClearLegacyOnce(reference) + adoptAndRetryLegacyCleanup(reference) } override fun clear(reference: DesktopSecretReference) { - val alreadyAdopted = adoption.isAdopted(reference) - if (!alreadyAdopted) adoption.markAdopted(reference) + if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { + adoption.markAdopted(reference) + } primary.clear(reference) - if (!alreadyAdopted) clearLegacyBestEffort(reference) + retryLegacyCleanup(reference) } - private fun adoptAndClearLegacyOnce(reference: DesktopSecretReference) { - if (adoption.isAdopted(reference)) return - adoption.markAdopted(reference) - clearLegacyBestEffort(reference) + private fun adoptAndRetryLegacyCleanup(reference: DesktopSecretReference) { + if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { + adoption.markAdopted(reference) + } + retryLegacyCleanup(reference) } - private fun clearLegacyBestEffort(reference: DesktopSecretReference) { + private fun retryLegacyCleanup(reference: DesktopSecretReference) { + if (adoption.state(reference) == DesktopSecretStoreAdoptionState.AdoptedAndClean) return try { legacy.clear(reference) + adoption.markLegacyCleanupComplete(reference) } catch (failure: kotlinx.coroutines.CancellationException) { throw failure } catch (_: Exception) { - // The durable adoption marker prevents a stale legacy value from being read again. + // Adoption prevents stale reads; the pending state retries cleanup on the next operation. } } } @@ -133,17 +148,34 @@ private class PreferencesDesktopSecretStoreAdoption( private val preferences: Preferences = Preferences.userRoot() .node("dev/obiente/nextcloudnative/secret-store-adoption-v1"), ) : DesktopSecretStoreAdoption { - override fun isAdopted(reference: DesktopSecretReference): Boolean = - preferences.getBoolean(reference.adoptionKey(), false) + override fun state(reference: DesktopSecretReference): DesktopSecretStoreAdoptionState = + when (preferences.get(reference.adoptionKey(), null)) { + ADOPTED_AND_CLEAN -> DesktopSecretStoreAdoptionState.AdoptedAndClean + ADOPTED_PENDING_CLEANUP, LEGACY_ADOPTED_VALUE -> + DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup + else -> DesktopSecretStoreAdoptionState.NotAdopted + } override fun markAdopted(reference: DesktopSecretReference) { - preferences.putBoolean(reference.adoptionKey(), true) + preferences.put(reference.adoptionKey(), ADOPTED_PENDING_CLEANUP) + preferences.flush() + } + + override fun markLegacyCleanupComplete(reference: DesktopSecretReference) { + check(state(reference) != DesktopSecretStoreAdoptionState.NotAdopted) + preferences.put(reference.adoptionKey(), ADOPTED_AND_CLEAN) preferences.flush() } private fun DesktopSecretReference.adoptionKey(): String = MessageDigest.getInstance("SHA-256") .digest(targetName.encodeToByteArray()) .toHexString() + + private companion object { + const val LEGACY_ADOPTED_VALUE = "true" + const val ADOPTED_PENDING_CLEANUP = "adopted-pending-legacy-cleanup" + const val ADOPTED_AND_CLEAN = "adopted-and-clean" + } } internal fun desktopSessionSecretReference(serverUrl: String, loginName: String): DesktopSecretReference { @@ -187,7 +219,9 @@ internal class SecretToolDesktopSecretStore( override fun load(reference: DesktopSecretReference): ByteArray? { val process = runCatching { startProcess(secretToolCommand("lookup", reference)) - }.getOrElse { return null } + }.getOrElse { failure -> + throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + } val executor = Executors.newSingleThreadExecutor { runnable -> Thread(runnable, "nextcloud-native-secret-reader").apply { isDaemon = true } } @@ -203,17 +237,21 @@ internal class SecretToolDesktopSecretStore( val elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAt) val remainingMillis = (timeoutMillis - elapsedMillis).coerceAtLeast(1L) val bytes = output.get(remainingMillis, TimeUnit.MILLISECONDS) - if (process.exitValue() != 0 || bytes.isEmpty()) return null + if (process.exitValue() != 0) { + if (!hasMatchingSecret(reference)) return null + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE) + } + if (bytes.isEmpty()) return null check(bytes.size <= MAX_SECRET_BYTES) { "The desktop secret service returned an oversized value." } return bytes.trimSingleTrailingLineBreak() - } catch (_: TimeoutException) { + } catch (failure: TimeoutException) { timedOut = true runCatching { process.descendants().forEach { child -> runCatching { child.destroyForcibly() } } } process.destroyForcibly() output.cancel(true) - error("Timed out while loading a desktop secret.") + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, failure) } finally { if (!timedOut) runCatching { process.inputStream.close() } executor.shutdownNow() @@ -249,11 +287,59 @@ internal class SecretToolDesktopSecretStore( } } + private fun hasMatchingSecret(reference: DesktopSecretReference): Boolean { + val command = buildList { + add("secret-tool") + add("search") + add("--all") + add("--unlock") + reference.attributes.forEach { (key, value) -> + add(key) + add(value) + } + } + val process = runCatching { startProcess(command) }.getOrElse { failure -> + throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + } + val executor = Executors.newSingleThreadExecutor { runnable -> + Thread(runnable, "nextcloud-native-secret-search").apply { isDaemon = true } + } + val output = executor.submit { + process.inputStream.use { it.readNBytes(MAX_SECRET_SEARCH_BYTES + 1) } + } + var timedOut = false + try { + if (!process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) throw TimeoutException() + val bytes = output.get(timeoutMillis, TimeUnit.MILLISECONDS) + if (process.exitValue() != 0 || bytes.size > MAX_SECRET_SEARCH_BYTES) { + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE) + } + return bytes.isNotEmpty() + } catch (failure: TimeoutException) { + timedOut = true + process.destroyForcibly() + output.cancel(true) + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, failure) + } finally { + if (!timedOut) runCatching { process.inputStream.close() } + executor.shutdownNow() + } + } + override fun clear(reference: DesktopSecretReference) { val process = runCatching { startProcess(secretToolCommand("clear", reference)) - }.getOrElse { return } - if (!process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) process.destroyForcibly() + }.getOrElse { failure -> + throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + } + if (!process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) { + process.destroyForcibly() + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE) + } + if (process.exitValue() != 0) { + if (!hasMatchingSecret(reference)) return + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE) + } } private fun secretToolCommand(command: String, reference: DesktopSecretReference): List = buildList { @@ -603,7 +689,8 @@ private const val MACOS_SECURITY_FRAMEWORK = "/System/Library/Frameworks/Securit private const val MACOS_CORE_FOUNDATION_FRAMEWORK = "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation" private const val MAX_SECRET_BYTES = 2_560 +private const val MAX_SECRET_SEARCH_BYTES = 256 * 1024 private const val MISSING_SECRET_TOOL_MESSAGE = "Secure credential storage is unavailable. Install libsecret-tools on Debian or Ubuntu, or libsecret on Fedora or RHEL, then restart Nextcloud Native." private const val KEYRING_UNAVAILABLE_MESSAGE = - "Could not save the account securely. Make sure your desktop keyring is running and unlocked, then try again." + "Secure credential storage is unavailable. Make sure your desktop keyring is running and unlocked, then try again." diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt new file mode 100644 index 000000000..c7fda003b --- /dev/null +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt @@ -0,0 +1,21 @@ +package dev.obiente.nextcloudnative.app + +import kotlinx.coroutines.CancellationException + +internal suspend fun reconcileDesktopBackgroundSession( + loadSession: () -> NextcloudSession?, + reconcile: suspend (NextcloudSession?) -> Unit, + onFailure: (NextcloudSession?, Throwable) -> Unit = { _, _ -> }, +): Boolean { + val loaded = loadNextcloudSessionSafely(loadSession) + if (loaded == NextcloudSessionLoadState.SecureStorageUnavailable) return false + val session = (loaded as NextcloudSessionLoadState.Loaded).session + try { + reconcile(session) + } catch (failure: CancellationException) { + throw failure + } catch (failure: Throwable) { + onFailure(session, failure) + } + return true +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index a2eb00107..76cc6ea11 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -67,7 +67,7 @@ class DesktopSecretStoreTest { } @Test - fun secretLookupTimeoutIncludesReadingStandardOutput() { + fun secretLookupTimeoutIsReportedAsUnavailableSecureStorage() { val store = SecretToolDesktopSecretStore( timeoutMillis = 100, startProcess = { NeverCompletingProcess() }, @@ -79,12 +79,82 @@ class DesktopSecretStoreTest { ) val startedAt = System.nanoTime() - val failure = assertFailsWith { store.load(reference) } + val failure = assertFailsWith { store.load(reference) } - assertTrue(failure.message.orEmpty().contains("Timed out")) + assertTrue(failure.message.orEmpty().contains("running and unlocked")) assertTrue(System.nanoTime() - startedAt < 1_000_000_000L) } + @Test + fun failedSecretLookupCannotBeMistakenForConfirmedAbsenceDuringMigration() { + val reference = desktopDeckDraftSecretReference() + val legacy = SecretToolDesktopSecretStore( + startProcess = { throw java.io.IOException("synthetic missing executable") }, + ) + val primary = RecordingSecretStore() + val adoption = RecordingSecretStoreAdoption() + + assertFailsWith { + MigratingDesktopSecretStore(primary, legacy, adoption).load(reference) + } + + assertNull(primary.load(reference)) + assertEquals(DesktopSecretStoreAdoptionState.NotAdopted, adoption.state(reference)) + } + + @Test + fun rejectedSecretLookupCannotBeMistakenForConfirmedAbsenceDuringMigration() { + val reference = desktopDeckDraftSecretReference() + val legacy = SecretToolDesktopSecretStore( + startProcess = { CompletedProcess(exitCode = 1) }, + ) + + assertFailsWith { + MigratingDesktopSecretStore( + RecordingSecretStore(), + legacy, + RecordingSecretStoreAdoption(), + ).load(reference) + } + } + + @Test + fun emptyUnlockedSearchConfirmsThatNoLegacySecretExists() { + val reference = desktopDeckDraftSecretReference() + val legacy = SecretToolDesktopSecretStore( + startProcess = { command -> + CompletedProcess(exitCode = if (command[1] == "search") 0 else 1) + }, + ) + val adoption = RecordingSecretStoreAdoption() + + assertNull(MigratingDesktopSecretStore(RecordingSecretStore(), legacy, adoption).load(reference)) + + assertEquals(DesktopSecretStoreAdoptionState.NotAdopted, adoption.state(reference)) + } + + @Test + fun failedLegacyClearIsReportedUnlessSearchConfirmsTheItemIsGone() { + val reference = desktopDeckDraftSecretReference() + val stillPresent = SecretToolDesktopSecretStore( + startProcess = { command -> + if (command[1] == "search") { + CompletedProcess(0, "synthetic matching item".encodeToByteArray()) + } else { + CompletedProcess(1) + } + }, + ) + val absent = SecretToolDesktopSecretStore( + startProcess = { command -> + CompletedProcess(exitCode = if (command[1] == "search") 0 else 1) + }, + ) + + assertFailsWith { stillPresent.clear(reference) } + absent.clear(reference) + } + private class NeverCompletingProcess : Process() { private val completion = CountDownLatch(1) private val output = ByteArrayOutputStream() @@ -112,11 +182,14 @@ class DesktopSecretStoreTest { override fun isAlive(): Boolean = completion.count > 0L } - private class CompletedProcess(private val exitCode: Int) : Process() { + private class CompletedProcess( + private val exitCode: Int, + private val input: ByteArray = ByteArray(0), + ) : Process() { private val output = ByteArrayOutputStream() override fun getOutputStream(): OutputStream = output - override fun getInputStream(): InputStream = ByteArrayInputStream(ByteArray(0)) + override fun getInputStream(): InputStream = ByteArrayInputStream(input) override fun getErrorStream(): InputStream = ByteArrayInputStream(ByteArray(0)) override fun waitFor(): Int = exitCode override fun waitFor(timeout: Long, unit: TimeUnit): Boolean = true @@ -181,8 +254,8 @@ class DesktopSecretStoreTest { assertContentEquals("draft-secret".encodeToByteArray(), primary.load(draftReference)) assertNull(legacy.load(sessionReference)) assertNull(legacy.load(draftReference)) - assertTrue(adoption.isAdopted(sessionReference)) - assertTrue(adoption.isAdopted(draftReference)) + assertEquals(DesktopSecretStoreAdoptionState.AdoptedAndClean, adoption.state(sessionReference)) + assertEquals(DesktopSecretStoreAdoptionState.AdoptedAndClean, adoption.state(draftReference)) } @Test @@ -217,7 +290,7 @@ class DesktopSecretStoreTest { assertFailsWith { store.load(reference) } assertContentEquals(expected, legacy.load(reference)) - assertFalse(adoption.isAdopted(reference)) + assertEquals(DesktopSecretStoreAdoptionState.NotAdopted, adoption.state(reference)) } @Test @@ -234,8 +307,56 @@ class DesktopSecretStoreTest { assertContentEquals(expected, store.load(reference)) assertContentEquals(expected, store.load(reference)) - assertEquals(1, legacy.clearAttempts) - assertTrue(adoption.isAdopted(reference)) + assertEquals(2, legacy.clearAttempts) + assertEquals( + DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup, + adoption.state(reference), + ) + } + + @Test + fun failedLegacyCleanupRetriesWithoutReadingTheStaleValueAgain() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "keychain-session-secret".encodeToByteArray() + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "legacy-session-secret".encodeToByteArray()), + failClearAttempts = 1, + ) + val primary = RecordingSecretStore(mutableMapOf(reference.targetName to expected)) + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + assertContentEquals(expected, store.load(reference)) + assertEquals(DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup, adoption.state(reference)) + assertContentEquals(expected, store.load(reference)) + + assertEquals(2, legacy.clearAttempts) + assertEquals(DesktopSecretStoreAdoptionState.AdoptedAndClean, adoption.state(reference)) + assertNull(legacy.load(reference)) + } + + @Test + fun freshDraftKeyCanBeCreatedWhenNoDraftDependsOnAnUnavailableLegacyStore() { + val secrets = RecordingSecretStore(failLoadAttempts = 1) + val provider = PlatformDeckDraftKeyProvider( + secretStore = secrets, + legacySecretRequired = { false }, + ) + + val key = provider.encryptionKey() + + assertEquals(DesktopDeckCardDraftStore.AES_KEY_BYTES, key.size) + assertTrue(secrets.values.containsKey(desktopDeckDraftSecretReference().targetName)) + } + + @Test + fun existingDraftNeverCreatesAReplacementKeyAfterAmbiguousLegacyLookup() { + val provider = PlatformDeckDraftKeyProvider( + secretStore = RecordingSecretStore(failLoad = true), + legacySecretRequired = { true }, + ) + + assertFailsWith { provider.encryptionKey() } } @Test @@ -349,14 +470,22 @@ class DesktopSecretStoreTest { private class RecordingSecretStore( val values: MutableMap = mutableMapOf(), private val failSave: Boolean = false, + private val failLoad: Boolean = false, + private var failLoadAttempts: Int = 0, private val ignoreClear: Boolean = false, private val failClear: Boolean = false, + private var failClearAttempts: Int = 0, ) : DesktopSecretStore { var clearAttempts = 0 private set - override fun load(reference: DesktopSecretReference): ByteArray? = - values[reference.targetName]?.copyOf() + override fun load(reference: DesktopSecretReference): ByteArray? { + if (failLoad || failLoadAttempts > 0) { + if (failLoadAttempts > 0) failLoadAttempts -= 1 + throw DesktopSecretStoreUnavailableException("Synthetic unavailable store.") + } + return values[reference.targetName]?.copyOf() + } override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { if (failSave) throw DesktopSecretStoreUnavailableException("Synthetic unavailable store.") @@ -365,18 +494,27 @@ class DesktopSecretStoreTest { override fun clear(reference: DesktopSecretReference) { clearAttempts += 1 - if (failClear) error("Synthetic legacy cleanup failure.") + if (failClear || failClearAttempts > 0) { + if (failClearAttempts > 0) failClearAttempts -= 1 + error("Synthetic legacy cleanup failure.") + } if (!ignoreClear) values.remove(reference.targetName) } } private class RecordingSecretStoreAdoption : DesktopSecretStoreAdoption { - private val adopted = mutableSetOf() + private val states = mutableMapOf() - override fun isAdopted(reference: DesktopSecretReference): Boolean = reference.targetName in adopted + override fun state(reference: DesktopSecretReference): DesktopSecretStoreAdoptionState = + states[reference.targetName] ?: DesktopSecretStoreAdoptionState.NotAdopted override fun markAdopted(reference: DesktopSecretReference) { - adopted += reference.targetName + states[reference.targetName] = DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup + } + + override fun markLegacyCleanupComplete(reference: DesktopSecretReference) { + check(state(reference) != DesktopSecretStoreAdoptionState.NotAdopted) + states[reference.targetName] = DesktopSecretStoreAdoptionState.AdoptedAndClean } } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt new file mode 100644 index 000000000..ea0ab7312 --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt @@ -0,0 +1,28 @@ +package dev.obiente.nextcloudnative.app + +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DesktopSyncLifecycleRecoveryTest { + @Test + fun secureStorageFailureLeavesTheNextBackgroundReconciliationRetryable() = runBlocking { + val expected = NextcloudSession("https://cloud.invalid", "alice", "synthetic-secret") + var attempts = 0 + val reconciled = mutableListOf() + val load = { + attempts += 1 + if (attempts == 1) { + throw NextcloudSessionStorageUnavailableException("synthetic locked keychain") + } + expected + } + + assertFalse(reconcileDesktopBackgroundSession(load, reconcile = { reconciled += it })) + assertTrue(reconcileDesktopBackgroundSession(load, reconcile = { reconciled += it })) + + assertEquals(listOf(expected), reconciled) + } +} From 323f801a6430ba2c8c47d41af46936274aa3c434 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 21:46:55 +0200 Subject: [PATCH 05/12] fix(macos): keep credential recovery actionable --- .../app/DesktopNextcloudServices.kt | 4 +- .../nextcloudnative/app/DesktopSecretStore.kt | 51 ++++++++++++++----- .../app/DesktopSyncLifecycleRecovery.kt | 8 +++ .../app/DesktopSecretStoreTest.kt | 36 ++++++++++++- 4 files changed, 81 insertions(+), 18 deletions(-) diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index fb0adb024..4126a3444 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -3736,7 +3736,7 @@ class DesktopNextcloudServices( } var cleared = false try { - val accountId = loadSession()?.let(::desktopFileCacheAccountId) + val accountId = desktopStoredSessionAccountId(preferences) val syncJob = synchronized(this) { val active = backgroundFileSyncJob backgroundFileSyncJob = null @@ -3867,7 +3867,7 @@ class DesktopNextcloudServices( } finally { if (!cleared) { synchronized(fileRangeSessionLock) { sessionClearing = false } - if (loadSession() != null) startDesktopSyncLifecycle() + if (desktopStoredSessionAccountId(preferences) != null) startDesktopSyncLifecycle() } } } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index 8e7319cb0..fbded1a5e 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -98,7 +98,7 @@ internal class MigratingDesktopSecretStore( ) : DesktopSecretStore { override fun load(reference: DesktopSecretReference): ByteArray? { primary.load(reference)?.let { secret -> - adoptAndRetryLegacyCleanup(reference) + adoptAndRetryLegacyCleanupBestEffort(reference) return secret } if (adoption.state(reference) != DesktopSecretStoreAdoptionState.NotAdopted) { @@ -107,39 +107,61 @@ internal class MigratingDesktopSecretStore( } val secret = legacy.load(reference) ?: return null primary.save(reference, username = null, secret = secret) - adoptAndRetryLegacyCleanup(reference) + adoptAndRetryLegacyCleanupBestEffort(reference) return secret } override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { primary.save(reference, username, secret) - adoptAndRetryLegacyCleanup(reference) + adoptAndRetryLegacyCleanupBestEffort(reference) } override fun clear(reference: DesktopSecretReference) { - if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { - adoption.markAdopted(reference) - } + markAdoptedBestEffort(reference) primary.clear(reference) retryLegacyCleanup(reference) } - private fun adoptAndRetryLegacyCleanup(reference: DesktopSecretReference) { - if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { - adoption.markAdopted(reference) - } + private fun adoptAndRetryLegacyCleanupBestEffort(reference: DesktopSecretReference) { + markAdoptedBestEffort(reference) retryLegacyCleanup(reference) } + private fun markAdoptedBestEffort(reference: DesktopSecretReference) { + try { + if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { + adoption.markAdopted(reference) + } + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + // A valid primary secret remains usable even when migration bookkeeping is unavailable. + } + } + private fun retryLegacyCleanup(reference: DesktopSecretReference) { - if (adoption.state(reference) == DesktopSecretStoreAdoptionState.AdoptedAndClean) return + val alreadyClean = try { + adoption.state(reference) == DesktopSecretStoreAdoptionState.AdoptedAndClean + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + false + } + if (alreadyClean) return try { legacy.clear(reference) - adoption.markLegacyCleanupComplete(reference) } catch (failure: kotlinx.coroutines.CancellationException) { throw failure } catch (_: Exception) { // Adoption prevents stale reads; the pending state retries cleanup on the next operation. + return + } + try { + adoption.markLegacyCleanupComplete(reference) + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + // Legacy cleanup is already complete; only the optional durable marker is unavailable. } } } @@ -377,8 +399,9 @@ internal class MacOsKeychainSecretStore( val data = secretData.value val itemPointer = item.value try { - check(size in 1..MAX_SECRET_BYTES && data != null) { - "macOS Keychain returned an invalid secret size." + if (size !in 1..MAX_SECRET_BYTES || data == null) { + clear(reference) + return null } return data.getByteArray(0, size) } finally { diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt index c7fda003b..089d6be73 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt @@ -1,6 +1,14 @@ package dev.obiente.nextcloudnative.app import kotlinx.coroutines.CancellationException +import java.util.prefs.Preferences + +internal fun desktopStoredSessionAccountId(preferences: Preferences): String? = + preferences.get("server", null)?.let { server -> + preferences.get("login", null)?.let { login -> + desktopFileCacheAccountId(NextcloudSession(server, login, "unused")) + } + } internal suspend fun reconcileDesktopBackgroundSession( loadSession: () -> NextcloudSession?, diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 76cc6ea11..e539890cf 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -314,6 +314,19 @@ class DesktopSecretStoreTest { ) } + @Test + fun unavailableAdoptionMetadataCannotBlockAValidKeychainValue() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "keychain-session-secret".encodeToByteArray() + val store = MigratingDesktopSecretStore( + primary = RecordingSecretStore(mutableMapOf(reference.targetName to expected)), + legacy = RecordingSecretStore(), + adoption = RecordingSecretStoreAdoption(failWrites = true), + ) + + assertContentEquals(expected, store.load(reference)) + } + @Test fun failedLegacyCleanupRetriesWithoutReadingTheStaleValueAgain() { val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") @@ -376,6 +389,17 @@ class DesktopSecretStoreTest { assertFalse(failure.message.orEmpty().contains("synthetic-user")) } + @Test + fun malformedMacOsKeychainValueIsRemovedAndReturnsToSignIn() { + val api = FakeMacOsKeychainApi(initialSecret = ByteArray(2_561)) + val store = MacOsKeychainSecretStore(api, releaseItem = {}) + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + + assertNull(store.load(reference)) + assertEquals(1, api.deleteAttempts) + assertNull(store.load(reference)) + } + @Test fun macOsKeychainConcurrentAddRaceUpdatesTheExistingItem() { val api = FakeMacOsKeychainApi(duplicateOnFirstAdd = true) @@ -391,8 +415,9 @@ class DesktopSecretStoreTest { private class FakeMacOsKeychainApi( private val findFailure: Int? = null, private val duplicateOnFirstAdd: Boolean = false, + initialSecret: ByteArray? = null, ) : MacOsKeychainApi { - private var secret: ByteArray? = null + private var secret: ByteArray? = initialSecret private var addAttempted = false private var returnedSecret: Memory? = null private val item = Memory(1) @@ -400,6 +425,8 @@ class DesktopSecretStoreTest { private set var lastAccount: String? = null private set + var deleteAttempts: Int = 0 + private set override fun SecKeychainFindGenericPassword( keychainOrArray: Pointer?, @@ -456,6 +483,7 @@ class DesktopSecretStoreTest { } override fun SecKeychainItemDelete(itemRef: Pointer): Int { + deleteAttempts += 1 secret = null return 0 } @@ -502,17 +530,21 @@ class DesktopSecretStoreTest { } } - private class RecordingSecretStoreAdoption : DesktopSecretStoreAdoption { + private class RecordingSecretStoreAdoption( + private val failWrites: Boolean = false, + ) : DesktopSecretStoreAdoption { private val states = mutableMapOf() override fun state(reference: DesktopSecretReference): DesktopSecretStoreAdoptionState = states[reference.targetName] ?: DesktopSecretStoreAdoptionState.NotAdopted override fun markAdopted(reference: DesktopSecretReference) { + if (failWrites) error("Synthetic unavailable adoption metadata.") states[reference.targetName] = DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup } override fun markLegacyCleanupComplete(reference: DesktopSecretReference) { + if (failWrites) error("Synthetic unavailable adoption metadata.") check(state(reference) != DesktopSecretStoreAdoptionState.NotAdopted) states[reference.targetName] = DesktopSecretStoreAdoptionState.AdoptedAndClean } From 121797e68994fa265e41a901d1e861ca48852763 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 22:20:27 +0200 Subject: [PATCH 06/12] fix(macos): preserve legacy credential recovery --- .../nextcloudnative/app/NextcloudNativeApp.kt | 10 +++++ .../app/NextcloudSessionLoading.kt | 11 +++++ .../app/NextcloudStatusMessages.kt | 22 +++++++++ .../app/NextcloudSessionLoadingTest.kt | 12 +++++ .../app/DesktopDeckCardDraftStore.kt | 14 ++++-- .../nextcloudnative/app/DesktopSecretStore.kt | 45 +++++++++++++++---- .../app/DesktopSyncLifecycleRecovery.kt | 7 ++- .../app/DesktopDeckCardDraftStoreTest.kt | 10 +++++ .../app/DesktopSecretStoreTest.kt | 4 +- .../app/DesktopSyncLifecycleRecoveryTest.kt | 17 +++++++ .../NextcloudSessionLoadingInteractionTest.kt | 25 +++++++++++ 11 files changed, 163 insertions(+), 14 deletions(-) diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt index f762ea660..e850d0440 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt @@ -543,6 +543,16 @@ fun NextcloudNativeApp( } if (sessionLoad == NextcloudSessionLoadState.SecureStorageUnavailable) { SecureSessionStorageUnavailable(onRetry = { sessionLoadAttempt += 1 }) + } else if (sessionLoad == NextcloudSessionLoadState.LegacyMigrationUnavailable) { + LegacySessionMigrationUnavailable( + onRetry = { sessionLoadAttempt += 1 }, + onSignInAgain = { + scope.launch { + services.clearSession() + sessionLoadAttempt += 1 + } + }, + ) } else if (session == null) { if (pendingAppUpdateReviewRequest != null) { LoggedOutAppUpdateReviewScreen( diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt index 7cceb5703..189cd6118 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt @@ -7,10 +7,19 @@ internal open class NextcloudSessionStorageUnavailableException( cause: Throwable? = null, ) : IllegalStateException(message, cause) +internal class NextcloudSessionLegacyMigrationUnavailableException( + cause: Throwable, +) : NextcloudSessionStorageUnavailableException( + "The legacy secure-storage provider required for session migration is unavailable.", + cause, +) + internal sealed interface NextcloudSessionLoadState { data class Loaded(val session: NextcloudSession?) : NextcloudSessionLoadState data object SecureStorageUnavailable : NextcloudSessionLoadState + + data object LegacyMigrationUnavailable : NextcloudSessionLoadState } internal fun loadNextcloudSessionSafely( @@ -19,6 +28,8 @@ internal fun loadNextcloudSessionSafely( NextcloudSessionLoadState.Loaded(loadSession()) } catch (failure: CancellationException) { throw failure +} catch (_: NextcloudSessionLegacyMigrationUnavailableException) { + NextcloudSessionLoadState.LegacyMigrationUnavailable } catch (_: NextcloudSessionStorageUnavailableException) { NextcloudSessionLoadState.SecureStorageUnavailable } diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt index c4834e9d3..a9ea53e52 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt @@ -55,3 +55,25 @@ internal fun SecureSessionStorageUnavailable(onRetry: () -> Unit) { ) } } + +@Composable +internal fun LegacySessionMigrationUnavailable( + onRetry: () -> Unit, + onSignInAgain: () -> Unit, +) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Column( + modifier = Modifier.padding(NextcloudSpacing.XLarge), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + Icon(NextcloudIcons.Error, contentDescription = null, tint = MaterialTheme.colorScheme.error) + Text( + "The previous session needs the legacy secure-storage provider. Install the provider " + + "and try again, or discard the stored session and sign in again.", + color = MaterialTheme.colorScheme.error, + ) + OutlinedButton(onClick = onRetry) { Text("Try again") } + OutlinedButton(onClick = onSignInAgain) { Text("Sign in again") } + } + } +} diff --git a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt index 90ccc0cb8..d1f011789 100644 --- a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt +++ b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingTest.kt @@ -32,6 +32,18 @@ class NextcloudSessionLoadingTest { } } + @Test + fun missingLegacyMigrationProviderKeepsItsRecoveryCategory() { + assertEquals( + NextcloudSessionLoadState.LegacyMigrationUnavailable, + loadNextcloudSessionSafely { + throw NextcloudSessionLegacyMigrationUnavailableException( + NextcloudSessionStorageUnavailableException("private provider failure"), + ) + }, + ) + } + @Test fun unrelatedProgrammingFailureIsNotPresentedAsUnavailableStorage() { assertFailsWith { diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt index 62e17e414..57fce39d1 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt @@ -24,9 +24,7 @@ import org.json.JSONObject internal class DesktopDeckCardDraftStore( private val root: File = desktopDeckDraftDirectory(), private val keyProvider: DesktopDeckDraftKeyProvider = PlatformDeckDraftKeyProvider( - legacySecretRequired = { - root.listFiles().orEmpty().any { file -> file.name.matches(DRAFT_FILE_PATTERN) } - }, + legacySecretRequired = { desktopDeckLegacySecretRequired(root) }, ), private val nowEpochMillis: () -> Long = System::currentTimeMillis, private val random: SecureRandom = SecureRandom(), @@ -296,6 +294,16 @@ internal class DesktopDeckCardDraftStore( } } +internal fun desktopDeckLegacySecretRequired( + root: File, + listFiles: (File) -> Array? = File::listFiles, +): Boolean { + if (!root.exists()) return false + if (!root.isDirectory) return true + val entries = listFiles(root) ?: return true + return entries.any { file -> file.name.matches(DesktopDeckCardDraftStore.DRAFT_FILE_PATTERN) } +} + internal fun interface DesktopDeckDraftKeyProvider { fun encryptionKey(): ByteArray } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index fbded1a5e..bdc443013 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -49,9 +49,15 @@ internal interface DesktopSecretStore { internal class DesktopSecretStoreUnavailableException( message: String, + val reason: DesktopSecretStoreUnavailableReason = DesktopSecretStoreUnavailableReason.StorageLockedOrUnavailable, cause: Throwable? = null, ) : NextcloudSessionStorageUnavailableException(message, cause) +internal enum class DesktopSecretStoreUnavailableReason { + StorageLockedOrUnavailable, + ProviderMissing, +} + internal enum class DesktopSecretStoreKind { MacOsKeychain, SecretService, @@ -105,7 +111,14 @@ internal class MigratingDesktopSecretStore( retryLegacyCleanup(reference) return null } - val secret = legacy.load(reference) ?: return null + val secret = try { + legacy.load(reference) + } catch (failure: DesktopSecretStoreUnavailableException) { + if (failure.reason == DesktopSecretStoreUnavailableReason.ProviderMissing) { + throw NextcloudSessionLegacyMigrationUnavailableException(failure) + } + throw failure + } ?: return null primary.save(reference, username = null, secret = secret) adoptAndRetryLegacyCleanupBestEffort(reference) return secret @@ -242,7 +255,11 @@ internal class SecretToolDesktopSecretStore( val process = runCatching { startProcess(secretToolCommand("lookup", reference)) }.getOrElse { failure -> - throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException( + MISSING_SECRET_TOOL_MESSAGE, + DesktopSecretStoreUnavailableReason.ProviderMissing, + failure, + ) } val executor = Executors.newSingleThreadExecutor { runnable -> Thread(runnable, "nextcloud-native-secret-reader").apply { isDaemon = true } @@ -273,7 +290,7 @@ internal class SecretToolDesktopSecretStore( } process.destroyForcibly() output.cancel(true) - throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, cause = failure) } finally { if (!timedOut) runCatching { process.inputStream.close() } executor.shutdownNow() @@ -292,13 +309,17 @@ internal class SecretToolDesktopSecretStore( } } val process = runCatching { startProcess(command) }.getOrElse { failure -> - throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException( + MISSING_SECRET_TOOL_MESSAGE, + DesktopSecretStoreUnavailableReason.ProviderMissing, + failure, + ) } runCatching { process.outputStream.use { it.write(secret) } }.getOrElse { failure -> process.destroyForcibly() - throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, cause = failure) } if (!process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) { process.destroyForcibly() @@ -321,7 +342,11 @@ internal class SecretToolDesktopSecretStore( } } val process = runCatching { startProcess(command) }.getOrElse { failure -> - throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException( + MISSING_SECRET_TOOL_MESSAGE, + DesktopSecretStoreUnavailableReason.ProviderMissing, + failure, + ) } val executor = Executors.newSingleThreadExecutor { runnable -> Thread(runnable, "nextcloud-native-secret-search").apply { isDaemon = true } @@ -341,7 +366,7 @@ internal class SecretToolDesktopSecretStore( timedOut = true process.destroyForcibly() output.cancel(true) - throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException(KEYRING_UNAVAILABLE_MESSAGE, cause = failure) } finally { if (!timedOut) runCatching { process.inputStream.close() } executor.shutdownNow() @@ -352,7 +377,11 @@ internal class SecretToolDesktopSecretStore( val process = runCatching { startProcess(secretToolCommand("clear", reference)) }.getOrElse { failure -> - throw DesktopSecretStoreUnavailableException(MISSING_SECRET_TOOL_MESSAGE, failure) + throw DesktopSecretStoreUnavailableException( + MISSING_SECRET_TOOL_MESSAGE, + DesktopSecretStoreUnavailableReason.ProviderMissing, + failure, + ) } if (!process.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) { process.destroyForcibly() diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt index 089d6be73..0347d1028 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecovery.kt @@ -16,8 +16,11 @@ internal suspend fun reconcileDesktopBackgroundSession( onFailure: (NextcloudSession?, Throwable) -> Unit = { _, _ -> }, ): Boolean { val loaded = loadNextcloudSessionSafely(loadSession) - if (loaded == NextcloudSessionLoadState.SecureStorageUnavailable) return false - val session = (loaded as NextcloudSessionLoadState.Loaded).session + val session = when (loaded) { + is NextcloudSessionLoadState.Loaded -> loaded.session + NextcloudSessionLoadState.SecureStorageUnavailable, + NextcloudSessionLoadState.LegacyMigrationUnavailable -> return false + } try { reconcile(session) } catch (failure: CancellationException) { diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt index 083ae03de..9a0813c74 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt @@ -115,6 +115,16 @@ class DesktopDeckCardDraftStoreTest { assertTrue(file.isFile) } + @Test + fun `uninspectable draft directory conservatively requires the legacy secret`() { + val root = Files.createTempDirectory("desktop-deck-drafts-unreadable").toFile() + try { + assertTrue(desktopDeckLegacySecretRequired(root, listFiles = { null })) + } finally { + root.deleteRecursively() + } + } + @Test fun `clear removes only the requested account resource`() = withStore { root, _, store -> diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index e539890cf..30a5450f2 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -39,6 +39,7 @@ class DesktopSecretStoreTest { assertTrue(failure.message.orEmpty().contains("libsecret-tools")) assertTrue(failure.message.orEmpty().contains("libsecret")) + assertEquals(DesktopSecretStoreUnavailableReason.ProviderMissing, failure.reason) assertFalse(failure.message.orEmpty().contains("Cannot run program")) assertFalse(failure.message.orEmpty().contains("synthetic-user")) assertFalse(failure.message.orEmpty().contains("synthetic-secret")) @@ -94,10 +95,11 @@ class DesktopSecretStoreTest { val primary = RecordingSecretStore() val adoption = RecordingSecretStoreAdoption() - assertFailsWith { + val failure = assertFailsWith { MigratingDesktopSecretStore(primary, legacy, adoption).load(reference) } + assertTrue(failure.message.orEmpty().contains("legacy secure-storage provider")) assertNull(primary.load(reference)) assertEquals(DesktopSecretStoreAdoptionState.NotAdopted, adoption.state(reference)) } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt index ea0ab7312..e15548ac2 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSyncLifecycleRecoveryTest.kt @@ -25,4 +25,21 @@ class DesktopSyncLifecycleRecoveryTest { assertEquals(listOf(expected), reconciled) } + + @Test + fun missingLegacyMigrationProviderDefersBackgroundReconciliation() = runBlocking { + var reconciled = false + + assertFalse( + reconcileDesktopBackgroundSession( + loadSession = { + throw NextcloudSessionLegacyMigrationUnavailableException( + NextcloudSessionStorageUnavailableException("synthetic missing provider"), + ) + }, + reconcile = { reconciled = true }, + ), + ) + assertFalse(reconciled) + } } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt index eac5850da..53910d7f3 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt @@ -22,4 +22,29 @@ class NextcloudSessionLoadingInteractionTest { assertEquals(1, retries) } } + + + @Test + fun unavailableLegacyMigrationOffersAStoredSessionReset() { + var retries = 0 + var resets = 0 + + nativeSceneTest(390, 844, content = { + LegacySessionMigrationUnavailable( + onRetry = { retries += 1 }, + onSignInAgain = { resets += 1 }, + ) + }) { + assertTrue( + has( + "The previous session needs the legacy secure-storage provider. Install the provider " + + "and try again, or discard the stored session and sign in again.", + ), + ) + click("Try again") + click("Sign in again") + assertEquals(1, retries) + assertEquals(1, resets) + } + } } From c855635a2d95a733474e0925a0137a48dc59d4af Mon Sep 17 00:00:00 2001 From: "obiente-automations[bot]" <311907242+obiente-automations[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:25:31 +0000 Subject: [PATCH 07/12] chore(website): refresh marketing captures --- website/public/screenshots/capture-manifest.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/public/screenshots/capture-manifest.json b/website/public/screenshots/capture-manifest.json index 6f1e32dda..0ff324829 100644 --- a/website/public/screenshots/capture-manifest.json +++ b/website/public/screenshots/capture-manifest.json @@ -602,15 +602,15 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudLinkRouting.kt": "5b29a90b69bb32aba118ef6c8b3f9d6eb26c03835823119b4a0f5bb1c1f4cb17", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt": "fb424bf8979ac292ef30daba64e905e6f5123066cda793aa0244907e11b465c9", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewerActions.kt": "48aaed6948d1423113d300cc3ab86d244ab76e8225e24988e9855edf74553944", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "9a3c4910e987d8c0b916d5b3b7e66db302fa0218e19968521dbfedb8695ca762", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "9a72cfe6856221a64de9ef9d08839b1dd24cd2c99a6d02659e9bdc293b81cb9a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt": "14d43a632afa7c5bf970d90d1387285624182be00569b57c0955670de017cc6c", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotesCache.kt": "9223dd455c6a1c35a10769616fceb9dfb2d92ef4d43b0a52c2c29e40f1a196cf", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPeople.kt": "cff910ea2cc77211ef81779c49ee0c957851f2b4a3ed32b857b12ded1cee643b", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt": "34d9b43cf3bbfc8342958dc40f2df7b4573f30a2ae1bd9bcb8bb470151313a3d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt": "058a09681e46b10dd0145dac4ca16df5491ba4546ac8b319acf7c29076804b51", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudResponseLimits.kt": "fead6cbf4f723ea46f99c9155b190967205d24f198687b7bfd5a213c474374b8", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt": "70d3404aa4ef5a6617590eb1abb8db35d4959cb3323d65d4156c787a7d43acf9", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt": "72e1ef943f0cf8022c69c105fd19370f12df36b000906a8937f89f079ff7b981", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt": "0abcfe2da22b8e49f6ee292d8b340cc36dafd2cf0658cbb8ca847481a351fe19", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt": "0ba425151a5449fcdd403dc19392f1133f008974875d7b6a54e865c817123124", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesApi.kt": "e3c685b02592ea791b0c9a478a098fc5bf428893aa4764110c793a78040f341e", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesFolderOperations.kt": "db6a04cfcd3b25b17c996cdc6dba21847d14f5970e06e6c24a3e2c85139dbe1d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/OfficeDocumentWorkflow.kt": "48057ebf53e45a042c4283aaff9a2ca5c3bb46fff3356e962540409f8c7b3b04", From 16380e7ef196047bd5ea905810be87642b6c82e2 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 22:59:11 +0200 Subject: [PATCH 08/12] fix(macos): create fresh draft keys without legacy storage --- .../app/DesktopDeckCardDraftStore.kt | 3 +++ .../app/DesktopSecretStoreTest.kt | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt index 57fce39d1..9b27d119c 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt @@ -341,6 +341,9 @@ internal class PlatformDeckDraftKeyProvider( private fun lookup(): ByteArray? { val stored = try { secretStore.load(desktopDeckDraftSecretReference()) + } catch (failure: NextcloudSessionLegacyMigrationUnavailableException) { + if (legacySecretRequired()) throw failure + null } catch (failure: DesktopSecretStoreUnavailableException) { if (legacySecretRequired()) throw failure null diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 30a5450f2..82fa3db28 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -364,6 +364,27 @@ class DesktopSecretStoreTest { assertTrue(secrets.values.containsKey(desktopDeckDraftSecretReference().targetName)) } + @Test + fun freshMacOsDraftKeyCanBypassAMissingLegacyProvider() { + val primary = RecordingSecretStore() + val migrating = MigratingDesktopSecretStore( + primary = primary, + legacy = SecretToolDesktopSecretStore( + startProcess = { throw java.io.IOException("synthetic missing executable") }, + ), + adoption = RecordingSecretStoreAdoption(), + ) + val provider = PlatformDeckDraftKeyProvider( + secretStore = migrating, + legacySecretRequired = { false }, + ) + + val key = provider.encryptionKey() + + assertEquals(DesktopDeckCardDraftStore.AES_KEY_BYTES, key.size) + assertTrue(primary.values.containsKey(desktopDeckDraftSecretReference().targetName)) + } + @Test fun existingDraftNeverCreatesAReplacementKeyAfterAmbiguousLegacyLookup() { val provider = PlatformDeckDraftKeyProvider( From e246a78882cef6ec2a2b8bcd6e0f23c94f0be3d4 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Tue, 1 Sep 2026 23:36:57 +0200 Subject: [PATCH 09/12] fix(macos): recover keychain cleanup after sign-out --- .../335-macos-keychain-secret-store.md | 2 +- .../nextcloudnative/app/NextcloudNativeApp.kt | 25 ++- .../app/NextcloudStatusMessages.kt | 23 ++- .../app/DesktopNextcloudServices.kt | 5 +- .../nextcloudnative/app/DesktopSecretStore.kt | 30 +++- .../app/MacOsKeychainDeletionRecovery.kt | 142 ++++++++++++++++++ .../app/DesktopSecretStoreTest.kt | 125 ++++++++++++++- .../NextcloudSessionLoadingInteractionTest.kt | 12 +- 8 files changed, 337 insertions(+), 27 deletions(-) create mode 100644 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/MacOsKeychainDeletionRecovery.kt diff --git a/changes/unreleased/335-macos-keychain-secret-store.md b/changes/unreleased/335-macos-keychain-secret-store.md index 4e51300e4..26bff9983 100644 --- a/changes/unreleased/335-macos-keychain-secret-store.md +++ b/changes/unreleased/335-macos-keychain-secret-store.md @@ -4,4 +4,4 @@ pull: 430 platforms: macos user-facing: yes -Store desktop login credentials and local Deck draft keys in the user's macOS Keychain, migrate existing Secret Service values, and keep locked Keychain access safely retryable. +Store desktop login credentials and local Deck draft keys in the user's macOS Keychain, migrate existing Secret Service values, keep locked Keychain access safely retryable, and durably retry credential deletion after sign-out. diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt index e850d0440..4dfff0917 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt @@ -541,17 +541,28 @@ fun NextcloudNativeApp( var session by remember(services, sessionLoadAttempt) { mutableStateOf((sessionLoad as? NextcloudSessionLoadState.Loaded)?.session) } + val signInAgain = { + scope.launch { + try { + services.clearSession() + sessionLoadAttempt += 1 + } catch (failure: CancellationException) { + throw failure + } catch (_: NextcloudSessionStorageUnavailableException) { + // Keep the recoverable storage screen visible when cleanup could not be queued safely. + } + } + Unit + } if (sessionLoad == NextcloudSessionLoadState.SecureStorageUnavailable) { - SecureSessionStorageUnavailable(onRetry = { sessionLoadAttempt += 1 }) + SecureSessionStorageUnavailable( + onRetry = { sessionLoadAttempt += 1 }, + onSignInAgain = signInAgain, + ) } else if (sessionLoad == NextcloudSessionLoadState.LegacyMigrationUnavailable) { LegacySessionMigrationUnavailable( onRetry = { sessionLoadAttempt += 1 }, - onSignInAgain = { - scope.launch { - services.clearSession() - sessionLoadAttempt += 1 - } - }, + onSignInAgain = signInAgain, ) } else if (session == null) { if (pendingAppUpdateReviewRequest != null) { diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt index a9ea53e52..3c98bcecb 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt @@ -46,13 +46,24 @@ internal fun ErrorMessage(message: String, onRetry: (() -> Unit)? = null) { } @Composable -internal fun SecureSessionStorageUnavailable(onRetry: () -> Unit) { +internal fun SecureSessionStorageUnavailable( + onRetry: () -> Unit, + onSignInAgain: () -> Unit, +) { Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorMessage( - "Secure session storage is locked or unavailable. Unlock it or allow " + - "Nextcloud Native access, then try again.", - onRetry, - ) + Column( + modifier = Modifier.padding(NextcloudSpacing.XLarge), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + Icon(NextcloudIcons.Error, contentDescription = null, tint = MaterialTheme.colorScheme.error) + Text( + "Secure session storage is locked or unavailable. Unlock it or allow " + + "Nextcloud Native access, then try again, or discard the stored session and sign in again.", + color = MaterialTheme.colorScheme.error, + ) + OutlinedButton(onClick = onRetry) { Text("Try again") } + OutlinedButton(onClick = onSignInAgain) { Text("Sign in again") } + } } } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index 4126a3444..09a1344ed 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -3843,9 +3843,7 @@ class DesktopNextcloudServices( val server = preferences.get(KEY_SERVER, null) val login = preferences.get(KEY_LOGIN, null) runCatching { - if (server != null && login != null) { - secretStore.clear(desktopSessionSecretReference(server, login)) - } + if (server != null && login != null) secretStore.clear(desktopSessionSecretReference(server, login)) }.onFailure { failure -> supportDiagnostics.record( SupportDiagnosticEventDraft( @@ -3856,6 +3854,7 @@ class DesktopNextcloudServices( exception = failure.toSupportDiagnosticExceptionDraft(), ), ) + if (failure is DesktopSecretDeletionRecoveryUnavailableException) throw failure } sessionPublicationGuard.serialize { preferences.remove(KEY_SERVER) diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index bdc443013..9e415d57c 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -53,6 +53,13 @@ internal class DesktopSecretStoreUnavailableException( cause: Throwable? = null, ) : NextcloudSessionStorageUnavailableException(message, cause) +internal class DesktopSecretDeletionRecoveryUnavailableException( + cause: Throwable, +) : NextcloudSessionStorageUnavailableException( + "Secure credential cleanup could not be scheduled for retry.", + cause, +) + internal enum class DesktopSecretStoreUnavailableReason { StorageLockedOrUnavailable, ProviderMissing, @@ -406,8 +413,16 @@ internal class SecretToolDesktopSecretStore( internal class MacOsKeychainSecretStore( private val api: MacOsKeychainApi = MacOsKeychainApiHolder.instance, private val releaseItem: (Pointer) -> Unit = MacOsCoreFoundationApiHolder::release, + private val deletionRecovery: MacOsKeychainDeletionRecovery = PreferencesMacOsKeychainDeletionRecovery(), ) : DesktopSecretStore { + private val deletionCoordinator = MacOsKeychainDeletionCoordinator(deletionRecovery, ::deleteTarget) + + init { + deletionCoordinator.retryAllBestEffort() + } + override fun load(reference: DesktopSecretReference): ByteArray? { + deletionCoordinator.retry(reference.targetName) val secretLength = IntByReference() val secretData = PointerByReference() val item = PointerByReference() @@ -441,6 +456,7 @@ internal class MacOsKeychainSecretStore( override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { require(secret.isNotEmpty() && secret.size <= MAX_SECRET_BYTES) + deletionCoordinator.retry(reference.targetName) val identity = reference.macOsIdentity() val item = PointerByReference() val findStatus = api.SecKeychainFindGenericPassword( @@ -461,7 +477,11 @@ internal class MacOsKeychainSecretStore( } override fun clear(reference: DesktopSecretReference) { - val identity = reference.macOsIdentity() + deletionCoordinator.clear(reference.targetName) + } + + private fun deleteTarget(targetName: String) { + val identity = targetName.macOsIdentity() val item = PointerByReference() val status = api.SecKeychainFindGenericPassword( null, @@ -567,10 +587,12 @@ private data class MacOsKeychainIdentity( val account: ByteArray, ) -private fun DesktopSecretReference.macOsIdentity(): MacOsKeychainIdentity = MacOsKeychainIdentity( - service = targetName.encodeToByteArray(), +private fun DesktopSecretReference.macOsIdentity(): MacOsKeychainIdentity = targetName.macOsIdentity() + +private fun String.macOsIdentity(): MacOsKeychainIdentity = MacOsKeychainIdentity( + service = encodeToByteArray(), account = MessageDigest.getInstance("SHA-256") - .digest(targetName.encodeToByteArray()) + .digest(encodeToByteArray()) .toHexString() .encodeToByteArray(), ) diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/MacOsKeychainDeletionRecovery.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/MacOsKeychainDeletionRecovery.kt new file mode 100644 index 000000000..1615c34c0 --- /dev/null +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/MacOsKeychainDeletionRecovery.kt @@ -0,0 +1,142 @@ +package dev.obiente.nextcloudnative.app + +import java.security.MessageDigest +import java.util.prefs.Preferences + +internal interface MacOsKeychainDeletionRecovery { + fun pendingTargetNames(): Set + + fun markPending(targetName: String) + + fun markComplete(targetName: String) +} + +internal class MacOsKeychainDeletionCoordinator( + private val recovery: MacOsKeychainDeletionRecovery, + private val deleteTarget: (String) -> Unit, +) { + fun clear(targetName: String) { + try { + recovery.markPending(targetName) + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (failure: Exception) { + throw DesktopSecretDeletionRecoveryUnavailableException(failure) + } + deleteTarget(targetName) + markComplete(targetName) + } + + fun retryAllBestEffort() { + val targetNames = try { + recovery.pendingTargetNames() + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + return + } + targetNames.forEach { targetName -> + try { + deleteTarget(targetName) + markComplete(targetName) + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (_: Exception) { + // The durable record remains pending for the next operation or process start. + } + } + } + + fun retry(targetName: String) { + val pending = try { + targetName in recovery.pendingTargetNames() + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (failure: Exception) { + throw DesktopSecretStoreUnavailableException( + "macOS Keychain cleanup recovery is unavailable.", + cause = failure, + ) + } + if (!pending) return + deleteTarget(targetName) + markComplete(targetName) + } + + private fun markComplete(targetName: String) { + try { + recovery.markComplete(targetName) + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (failure: Exception) { + throw DesktopSecretStoreUnavailableException( + "macOS Keychain cleanup could not record completion.", + cause = failure, + ) + } + } +} + +internal class PreferencesMacOsKeychainDeletionRecovery( + private val preferences: Preferences = Preferences.userRoot() + .node("dev/obiente/nextcloudnative/macos-keychain-deletion-v1"), + private val flush: (Preferences) -> Unit = { it.flush() }, +) : MacOsKeychainDeletionRecovery { + override fun pendingTargetNames(): Set = preferences.keys() + .mapNotNullTo(linkedSetOf()) { key -> + preferences.get(key, null) + ?.takeIf { value -> value.startsWith(PENDING_PREFIX) } + ?.removePrefix(PENDING_PREFIX) + ?.takeIf(::isValidTargetName) + } + + override fun markPending(targetName: String) { + require(isValidTargetName(targetName)) + val key = targetName.recoveryKey() + val previous = preferences.get(key, null) + val existingEntries = pendingTargetNames().size + check(previous != null || existingEntries < MAX_RECOVERY_ENTRIES) { + "Too many pending macOS Keychain cleanup records." + } + preferences.put(key, PENDING_PREFIX + targetName) + try { + flush(preferences) + } catch (failure: Exception) { + if (previous == null) preferences.remove(key) else preferences.put(key, previous) + throw failure + } + } + + override fun markComplete(targetName: String) { + require(isValidTargetName(targetName)) + val key = targetName.recoveryKey() + preferences.put(key, COMPLETE_PREFIX + targetName) + try { + flush(preferences) + } catch (failure: Exception) { + // Keep this process conservative when durable completion is ambiguous. + preferences.put(key, PENDING_PREFIX + targetName) + throw failure + } + preferences.remove(key) + runCatching { flush(preferences) } + } + + private fun String.recoveryKey(): String = MessageDigest.getInstance("SHA-256") + .digest(encodeToByteArray()) + .joinToString(separator = "") { byte -> + (byte.toInt() and 0xff).toString(16).padStart(2, '0') + } + + private fun isValidTargetName(targetName: String): Boolean = + targetName.isNotBlank() && + targetName.length <= MAX_TARGET_NAME_CHARACTERS && + targetName.none(Char::isISOControl) + + private companion object { + const val PENDING_PREFIX = "pending:" + const val COMPLETE_PREFIX = "complete:" + const val MAX_TARGET_NAME_CHARACTERS = 512 + const val MAX_RECOVERY_ENTRIES = 128 + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 82fa3db28..5d45dcb80 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -11,6 +11,8 @@ import java.io.OutputStream import java.util.UUID import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit +import java.util.prefs.BackingStoreException +import java.util.prefs.Preferences import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals @@ -214,7 +216,11 @@ class DesktopSecretStoreTest { fun macOsKeychainAddsUpdatesLoadsAndClearsWithoutPuttingSecretsInIdentityFields() { val api = FakeMacOsKeychainApi() val releasedItems = mutableListOf() - val store = MacOsKeychainSecretStore(api, releasedItems::add) + val store = MacOsKeychainSecretStore( + api, + releasedItems::add, + RecordingMacOsKeychainDeletionRecovery(), + ) val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") val first = "first-synthetic-secret".encodeToByteArray() val second = "second-synthetic-secret".encodeToByteArray() @@ -400,6 +406,7 @@ class DesktopSecretStoreTest { val store = MacOsKeychainSecretStore( api = FakeMacOsKeychainApi(findFailure = -25_293), releaseItem = {}, + deletionRecovery = RecordingMacOsKeychainDeletionRecovery(), ) val reference = desktopSessionSecretReference("https://private.invalid", "synthetic-user") @@ -415,7 +422,11 @@ class DesktopSecretStoreTest { @Test fun malformedMacOsKeychainValueIsRemovedAndReturnsToSignIn() { val api = FakeMacOsKeychainApi(initialSecret = ByteArray(2_561)) - val store = MacOsKeychainSecretStore(api, releaseItem = {}) + val store = MacOsKeychainSecretStore( + api, + releaseItem = {}, + deletionRecovery = RecordingMacOsKeychainDeletionRecovery(), + ) val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") assertNull(store.load(reference)) @@ -426,7 +437,11 @@ class DesktopSecretStoreTest { @Test fun macOsKeychainConcurrentAddRaceUpdatesTheExistingItem() { val api = FakeMacOsKeychainApi(duplicateOnFirstAdd = true) - val store = MacOsKeychainSecretStore(api, releaseItem = {}) + val store = MacOsKeychainSecretStore( + api, + releaseItem = {}, + deletionRecovery = RecordingMacOsKeychainDeletionRecovery(), + ) val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") val expected = "replacement-synthetic-secret".encodeToByteArray() @@ -435,10 +450,88 @@ class DesktopSecretStoreTest { assertContentEquals(expected, store.load(reference)) } + @Test + fun failedMacOsKeychainDeletionRetriesAfterProcessRestart() { + val api = FakeMacOsKeychainApi( + initialSecret = "synthetic-session-secret".encodeToByteArray(), + deleteFailureAttempts = 1, + ) + val recovery = RecordingMacOsKeychainDeletionRecovery() + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val firstProcess = MacOsKeychainSecretStore(api, releaseItem = {}, deletionRecovery = recovery) + + assertFailsWith { firstProcess.clear(reference) } + assertEquals(setOf(reference.targetName), recovery.pendingTargetNames()) + assertEquals(1, api.deleteAttempts) + + val restarted = MacOsKeychainSecretStore(api, releaseItem = {}, deletionRecovery = recovery) + + assertNull(restarted.load(reference)) + assertEquals(emptySet(), recovery.pendingTargetNames()) + assertEquals(2, api.deleteAttempts) + } + + @Test + fun ambiguousDeletionCompletionRemainsRetryableBeforeReplacementSave() { + val api = FakeMacOsKeychainApi(initialSecret = "old-synthetic-secret".encodeToByteArray()) + val recovery = RecordingMacOsKeychainDeletionRecovery(failCompleteAttempts = 1) + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val store = MacOsKeychainSecretStore(api, releaseItem = {}, deletionRecovery = recovery) + + assertFailsWith { store.clear(reference) } + assertEquals(setOf(reference.targetName), recovery.pendingTargetNames()) + + val replacement = "replacement-synthetic-secret".encodeToByteArray() + store.save(reference, "alice", replacement) + + assertContentEquals(replacement, store.load(reference)) + assertEquals(emptySet(), recovery.pendingTargetNames()) + assertEquals(1, api.deleteAttempts) + } + + @Test + fun unavailableRecoveryJournalPreventsUntrackedKeychainDeletion() { + val api = FakeMacOsKeychainApi(initialSecret = "synthetic-session-secret".encodeToByteArray()) + val recovery = RecordingMacOsKeychainDeletionRecovery(failPending = true) + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val store = MacOsKeychainSecretStore(api, releaseItem = {}, deletionRecovery = recovery) + + assertFailsWith { store.clear(reference) } + + assertEquals(0, api.deleteAttempts) + assertContentEquals("synthetic-session-secret".encodeToByteArray(), store.load(reference)) + } + + @Test + fun ambiguousJournalCompletionStaysPendingUntilDurablyRecorded() { + val preferences = Preferences.userRoot().node( + "dev/obiente/nextcloudnative/test-keychain-deletion/${UUID.randomUUID()}", + ) + var flushAttempts = 0 + val recovery = PreferencesMacOsKeychainDeletionRecovery(preferences) { node -> + flushAttempts += 1 + if (flushAttempts == 2) throw BackingStoreException("Synthetic flush failure.") + node.flush() + } + val targetName = desktopSessionSecretReference("https://cloud.invalid", "alice").targetName + try { + recovery.markPending(targetName) + + assertFailsWith { recovery.markComplete(targetName) } + assertEquals(setOf(targetName), recovery.pendingTargetNames()) + + recovery.markComplete(targetName) + assertEquals(emptySet(), recovery.pendingTargetNames()) + } finally { + preferences.removeNode() + } + } + private class FakeMacOsKeychainApi( private val findFailure: Int? = null, private val duplicateOnFirstAdd: Boolean = false, initialSecret: ByteArray? = null, + private var deleteFailureAttempts: Int = 0, ) : MacOsKeychainApi { private var secret: ByteArray? = initialSecret private var addAttempted = false @@ -507,6 +600,10 @@ class DesktopSecretStoreTest { override fun SecKeychainItemDelete(itemRef: Pointer): Int { deleteAttempts += 1 + if (deleteFailureAttempts > 0) { + deleteFailureAttempts -= 1 + return -25_308 + } secret = null return 0 } @@ -518,6 +615,28 @@ class DesktopSecretStoreTest { } } + private class RecordingMacOsKeychainDeletionRecovery( + private val failPending: Boolean = false, + private var failCompleteAttempts: Int = 0, + ) : MacOsKeychainDeletionRecovery { + private val pending = linkedSetOf() + + override fun pendingTargetNames(): Set = pending.toSet() + + override fun markPending(targetName: String) { + if (failPending) error("Synthetic unavailable deletion recovery.") + pending += targetName + } + + override fun markComplete(targetName: String) { + if (failCompleteAttempts > 0) { + failCompleteAttempts -= 1 + error("Synthetic ambiguous deletion completion.") + } + pending -= targetName + } + } + private class RecordingSecretStore( val values: MutableMap = mutableMapOf(), private val failSave: Boolean = false, diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt index 53910d7f3..139b0f9a0 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoadingInteractionTest.kt @@ -6,20 +6,26 @@ import kotlin.test.assertTrue class NextcloudSessionLoadingInteractionTest { @Test - fun unavailableSecureStorageExplainsRecoveryAndOffersRetry() { + fun unavailableSecureStorageOffersRetryAndStoredSessionReset() { var retries = 0 + var resets = 0 nativeSceneTest(390, 844, content = { - SecureSessionStorageUnavailable(onRetry = { retries += 1 }) + SecureSessionStorageUnavailable( + onRetry = { retries += 1 }, + onSignInAgain = { resets += 1 }, + ) }) { assertTrue( has( "Secure session storage is locked or unavailable. Unlock it or allow " + - "Nextcloud Native access, then try again.", + "Nextcloud Native access, then try again, or discard the stored session and sign in again.", ), ) click("Try again") + click("Sign in again") assertEquals(1, retries) + assertEquals(1, resets) } } From 86a10050c35493ad0b65fc2da840596814ad319b Mon Sep 17 00:00:00 2001 From: "obiente-automations[bot]" <311907242+obiente-automations[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:42:30 +0000 Subject: [PATCH 10/12] chore(website): refresh marketing captures --- website/public/screenshots/capture-manifest.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/public/screenshots/capture-manifest.json b/website/public/screenshots/capture-manifest.json index 0ff324829..cf13c6660 100644 --- a/website/public/screenshots/capture-manifest.json +++ b/website/public/screenshots/capture-manifest.json @@ -602,7 +602,7 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudLinkRouting.kt": "5b29a90b69bb32aba118ef6c8b3f9d6eb26c03835823119b4a0f5bb1c1f4cb17", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt": "fb424bf8979ac292ef30daba64e905e6f5123066cda793aa0244907e11b465c9", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewerActions.kt": "48aaed6948d1423113d300cc3ab86d244ab76e8225e24988e9855edf74553944", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "9a72cfe6856221a64de9ef9d08839b1dd24cd2c99a6d02659e9bdc293b81cb9a", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "80d7e5db29193b835b94be990747195234b5dfad6116d853a627f5172aea19ae", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt": "14d43a632afa7c5bf970d90d1387285624182be00569b57c0955670de017cc6c", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotesCache.kt": "9223dd455c6a1c35a10769616fceb9dfb2d92ef4d43b0a52c2c29e40f1a196cf", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPeople.kt": "cff910ea2cc77211ef81779c49ee0c957851f2b4a3ed32b857b12ded1cee643b", @@ -610,7 +610,7 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt": "058a09681e46b10dd0145dac4ca16df5491ba4546ac8b319acf7c29076804b51", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudResponseLimits.kt": "fead6cbf4f723ea46f99c9155b190967205d24f198687b7bfd5a213c474374b8", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt": "0abcfe2da22b8e49f6ee292d8b340cc36dafd2cf0658cbb8ca847481a351fe19", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt": "0ba425151a5449fcdd403dc19392f1133f008974875d7b6a54e865c817123124", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudStatusMessages.kt": "5154dd95c432c91e306c09372ef7367f6b43be8c9d52826b36ecd8ecbfcbf3dc", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesApi.kt": "e3c685b02592ea791b0c9a478a098fc5bf428893aa4764110c793a78040f341e", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NotesFolderOperations.kt": "db6a04cfcd3b25b17c996cdc6dba21847d14f5970e06e6c24a3e2c85139dbe1d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/OfficeDocumentWorkflow.kt": "48057ebf53e45a042c4283aaff9a2ca5c3bb46fff3356e962540409f8c7b3b04", From 962e6b8d4f33d3b84c3cc86cde26a32661c3922b Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Wed, 2 Sep 2026 00:06:41 +0200 Subject: [PATCH 11/12] fix(macos): preserve draft and legacy recovery --- .../335-macos-keychain-secret-store.md | 2 +- .../app/DesktopDeckCardDraftStore.kt | 15 +++- .../app/DesktopNextcloudServices.kt | 3 +- .../nextcloudnative/app/DesktopSecretStore.kt | 50 ++++++++--- .../app/DesktopDeckCardDraftStoreTest.kt | 30 +++++++ .../app/DesktopSecretStoreTest.kt | 84 +++++++++++++++++++ 6 files changed, 166 insertions(+), 18 deletions(-) diff --git a/changes/unreleased/335-macos-keychain-secret-store.md b/changes/unreleased/335-macos-keychain-secret-store.md index 26bff9983..969ff8ef7 100644 --- a/changes/unreleased/335-macos-keychain-secret-store.md +++ b/changes/unreleased/335-macos-keychain-secret-store.md @@ -4,4 +4,4 @@ pull: 430 platforms: macos user-facing: yes -Store desktop login credentials and local Deck draft keys in the user's macOS Keychain, migrate existing Secret Service values, keep locked Keychain access safely retryable, and durably retry credential deletion after sign-out. +Store desktop login credentials and Deck draft keys in macOS Keychain, migrate Secret Service values, and keep locked access and sign-out cleanup retryable. Preserve encrypted Deck drafts instead of replacing a missing key. diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt index 9b27d119c..d39fb36e7 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStore.kt @@ -352,9 +352,18 @@ internal class PlatformDeckDraftKeyProvider( ?.let { value -> value.copyOf(minOf(value.size, MAX_ENCODED_KEY_BYTES)) } ?.decodeToString() ?.trim() - ?: return null - if (encoded.isBlank()) return null - return runCatching { Base64.getDecoder().decode(encoded) }.getOrNull() + ?: return missingKey() + if (encoded.isBlank()) return missingKey() + return runCatching { Base64.getDecoder().decode(encoded) }.getOrNull() ?: missingKey() + } + + private fun missingKey(): ByteArray? { + if (legacySecretRequired()) { + throw DesktopSecretStoreUnavailableException( + "The Deck draft encryption key is missing while encrypted drafts still exist.", + ) + } + return null } private companion object { diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index 09a1344ed..ba654ec47 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -3854,7 +3854,8 @@ class DesktopNextcloudServices( exception = failure.toSupportDiagnosticExceptionDraft(), ), ) - if (failure is DesktopSecretDeletionRecoveryUnavailableException) throw failure + if (failure is DesktopSecretDeletionRecoveryUnavailableException || + failure is DesktopSecretLegacyCleanupUnavailableException) throw failure } sessionPublicationGuard.serialize { preferences.remove(KEY_SERVER) diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index 9e415d57c..309425364 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -60,6 +60,13 @@ internal class DesktopSecretDeletionRecoveryUnavailableException( cause, ) +internal class DesktopSecretLegacyCleanupUnavailableException( + cause: Throwable, +) : NextcloudSessionStorageUnavailableException( + "The legacy secure credential could not be cleared safely.", + cause, +) + internal enum class DesktopSecretStoreUnavailableReason { StorageLockedOrUnavailable, ProviderMissing, @@ -137,29 +144,46 @@ internal class MigratingDesktopSecretStore( } override fun clear(reference: DesktopSecretReference) { - markAdoptedBestEffort(reference) - primary.clear(reference) - retryLegacyCleanup(reference) + markAdopted(reference) + val primaryFailure = try { + primary.clear(reference) + null + } catch (failure: kotlinx.coroutines.CancellationException) { + throw failure + } catch (failure: Exception) { + failure + } + retryLegacyCleanup(reference)?.let { failure -> + primaryFailure?.let(failure::addSuppressed) + throw DesktopSecretLegacyCleanupUnavailableException(failure) + } + primaryFailure?.let { throw it } } private fun adoptAndRetryLegacyCleanupBestEffort(reference: DesktopSecretReference) { - markAdoptedBestEffort(reference) - retryLegacyCleanup(reference) + val adoptionDurable = markAdopted(reference) + val legacyCleanupFailure = retryLegacyCleanup(reference) + if (!adoptionDurable && legacyCleanupFailure != null) { + throw DesktopSecretStoreUnavailableException( + "Keychain adoption and legacy credential cleanup are both unavailable.", + cause = legacyCleanupFailure, + ) + } } - private fun markAdoptedBestEffort(reference: DesktopSecretReference) { + private fun markAdopted(reference: DesktopSecretReference): Boolean = try { if (adoption.state(reference) == DesktopSecretStoreAdoptionState.NotAdopted) { adoption.markAdopted(reference) } + true } catch (failure: kotlinx.coroutines.CancellationException) { throw failure } catch (_: Exception) { - // A valid primary secret remains usable even when migration bookkeeping is unavailable. + false } - } - private fun retryLegacyCleanup(reference: DesktopSecretReference) { + private fun retryLegacyCleanup(reference: DesktopSecretReference): Exception? { val alreadyClean = try { adoption.state(reference) == DesktopSecretStoreAdoptionState.AdoptedAndClean } catch (failure: kotlinx.coroutines.CancellationException) { @@ -167,14 +191,13 @@ internal class MigratingDesktopSecretStore( } catch (_: Exception) { false } - if (alreadyClean) return + if (alreadyClean) return null try { legacy.clear(reference) } catch (failure: kotlinx.coroutines.CancellationException) { throw failure - } catch (_: Exception) { - // Adoption prevents stale reads; the pending state retries cleanup on the next operation. - return + } catch (failure: Exception) { + return failure } try { adoption.markLegacyCleanupComplete(reference) @@ -183,6 +206,7 @@ internal class MigratingDesktopSecretStore( } catch (_: Exception) { // Legacy cleanup is already complete; only the optional durable marker is unavailable. } + return null } } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt index 9a0813c74..35cfa9e19 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopDeckCardDraftStoreTest.kt @@ -115,6 +115,36 @@ class DesktopDeckCardDraftStoreTest { assertTrue(file.isFile) } + @Test + fun `missing key does not replace or delete an existing encrypted draft`() = + withStore { root, _, store -> + val session = session() + val persisted = persisted() + store.save(session, persisted) + val file = root.resolve(store.storageFileName(session, persisted.key)) + val missingSecrets = object : DesktopSecretStore { + override fun load(reference: DesktopSecretReference): ByteArray? = null + + override fun save(reference: DesktopSecretReference, username: String?, secret: ByteArray) { + error("A replacement key must not be saved.") + } + + override fun clear(reference: DesktopSecretReference) = Unit + } + val unavailable = DesktopDeckCardDraftStore( + root = root, + keyProvider = PlatformDeckDraftKeyProvider( + secretStore = missingSecrets, + legacySecretRequired = { desktopDeckLegacySecretRequired(root) }, + ), + ) + + assertFailsWith { + unavailable.load(session, persisted.key) + } + assertTrue(file.isFile) + } + @Test fun `uninspectable draft directory conservatively requires the legacy secret`() { val root = Files.createTempDirectory("desktop-deck-drafts-unreadable").toFile() diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 5d45dcb80..4747798da 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -335,6 +335,63 @@ class DesktopSecretStoreTest { assertContentEquals(expected, store.load(reference)) } + @Test + fun unavailableAdoptionAndLegacyCleanupCannotExposeAKeychainValue() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val expected = "keychain-session-secret".encodeToByteArray() + val store = MigratingDesktopSecretStore( + primary = RecordingSecretStore(mutableMapOf(reference.targetName to expected)), + legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "stale-session-secret".encodeToByteArray()), + failClear = true, + ), + adoption = RecordingSecretStoreAdoption(failWrites = true), + ) + + assertFailsWith { store.load(reference) } + } + + @Test + fun failedLegacyCleanupKeepsSignOutReferenceRetryable() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val primary = RecordingSecretStore( + mutableMapOf(reference.targetName to "keychain-session-secret".encodeToByteArray()), + ) + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "legacy-session-secret".encodeToByteArray()), + failClearAttempts = 1, + ) + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore(primary, legacy, adoption) + + assertFailsWith { store.clear(reference) } + assertNull(primary.load(reference)) + assertTrue(legacy.values.containsKey(reference.targetName)) + + store.clear(reference) + + assertNull(legacy.load(reference)) + assertEquals(2, legacy.clearAttempts) + } + + @Test + fun failedKeychainClearStillRemovesTheLegacyCredential() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val primary = RecordingSecretStore( + mutableMapOf(reference.targetName to "keychain-session-secret".encodeToByteArray()), + failClear = true, + ) + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "legacy-session-secret".encodeToByteArray()), + ) + val store = MigratingDesktopSecretStore(primary, legacy, RecordingSecretStoreAdoption()) + + assertFailsWith { store.clear(reference) } + + assertNull(legacy.load(reference)) + assertEquals(1, legacy.clearAttempts) + } + @Test fun failedLegacyCleanupRetriesWithoutReadingTheStaleValueAgain() { val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") @@ -401,6 +458,33 @@ class DesktopSecretStoreTest { assertFailsWith { provider.encryptionKey() } } + @Test + fun existingDraftNeverCreatesAReplacementForAConfirmedMissingKey() { + val secrets = RecordingSecretStore() + val provider = PlatformDeckDraftKeyProvider( + secretStore = secrets, + legacySecretRequired = { true }, + ) + + assertFailsWith { provider.encryptionKey() } + assertFalse(secrets.values.containsKey(desktopDeckDraftSecretReference().targetName)) + } + + @Test + fun existingDraftNeverCreatesAReplacementForAMalformedKey() { + val reference = desktopDeckDraftSecretReference() + val secrets = RecordingSecretStore( + mutableMapOf(reference.targetName to "not-base64".encodeToByteArray()), + ) + val provider = PlatformDeckDraftKeyProvider( + secretStore = secrets, + legacySecretRequired = { true }, + ) + + assertFailsWith { provider.encryptionKey() } + assertContentEquals("not-base64".encodeToByteArray(), secrets.values.getValue(reference.targetName)) + } + @Test fun macOsKeychainDenialIsActionableAndDoesNotExposeCredentialIdentity() { val store = MacOsKeychainSecretStore( From 5d05fde238efd5a094df7ea34587b3ad1e1cf367 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Thu, 3 Sep 2026 22:27:27 +0200 Subject: [PATCH 12/12] fix(macos): finish credential cleanup recovery --- tools/kotlin-file-size-baseline.txt | 2 +- .../nextcloudnative/app/DesktopSecretStore.kt | 6 +- .../app/DesktopSecretStoreTest.kt | 58 ++++++++++++++++++- .../app/JvmSupportIntakeTest.kt | 4 +- 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/tools/kotlin-file-size-baseline.txt b/tools/kotlin-file-size-baseline.txt index bce4b798b..8746c0f05 100644 --- a/tools/kotlin-file-size-baseline.txt +++ b/tools/kotlin-file-size-baseline.txt @@ -25,7 +25,7 @@ ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckBoardSurface. ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckRelationDialogs.kt|1234 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckScreen.kt|1937 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt|1331 -ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|12436 +ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|12435 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt|1693 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt|808 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt|1755 diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt index 309425364..90abb2b02 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStore.kt @@ -144,7 +144,7 @@ internal class MigratingDesktopSecretStore( } override fun clear(reference: DesktopSecretReference) { - markAdopted(reference) + val legacyCleanupQueued = markAdopted(reference) val primaryFailure = try { primary.clear(reference) null @@ -155,7 +155,9 @@ internal class MigratingDesktopSecretStore( } retryLegacyCleanup(reference)?.let { failure -> primaryFailure?.let(failure::addSuppressed) - throw DesktopSecretLegacyCleanupUnavailableException(failure) + if (primaryFailure != null || !legacyCleanupQueued) { + throw DesktopSecretLegacyCleanupUnavailableException(failure) + } } primaryFailure?.let { throw it } } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt index 4747798da..cc5a75f43 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopSecretStoreTest.kt @@ -352,7 +352,7 @@ class DesktopSecretStoreTest { } @Test - fun failedLegacyCleanupKeepsSignOutReferenceRetryable() { + fun queuedLegacyCleanupCannotBlockLocalSignOut() { val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") val primary = RecordingSecretStore( mutableMapOf(reference.targetName to "keychain-session-secret".encodeToByteArray()), @@ -364,14 +364,66 @@ class DesktopSecretStoreTest { val adoption = RecordingSecretStoreAdoption() val store = MigratingDesktopSecretStore(primary, legacy, adoption) - assertFailsWith { store.clear(reference) } + store.clear(reference) + assertNull(primary.load(reference)) assertTrue(legacy.values.containsKey(reference.targetName)) + assertEquals( + DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup, + adoption.state(reference), + ) - store.clear(reference) + assertNull(store.load(reference)) assertNull(legacy.load(reference)) assertEquals(2, legacy.clearAttempts) + assertEquals(DesktopSecretStoreAdoptionState.AdoptedAndClean, adoption.state(reference)) + } + + @Test + fun missingLegacyProviderCannotBlockLocalSignOut() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val primary = RecordingSecretStore( + mutableMapOf(reference.targetName to "keychain-session-secret".encodeToByteArray()), + ) + val adoption = RecordingSecretStoreAdoption() + val store = MigratingDesktopSecretStore( + primary, + SecretToolDesktopSecretStore( + startProcess = { throw java.io.IOException("synthetic missing executable") }, + ), + adoption, + ) + + store.clear(reference) + + assertNull(primary.load(reference)) + assertEquals( + DesktopSecretStoreAdoptionState.AdoptedPendingLegacyCleanup, + adoption.state(reference), + ) + } + + @Test + fun unqueuedLegacyCleanupFailureRemainsActionable() { + val reference = desktopSessionSecretReference("https://cloud.invalid", "alice") + val primary = RecordingSecretStore( + mutableMapOf(reference.targetName to "keychain-session-secret".encodeToByteArray()), + ) + val legacy = RecordingSecretStore( + mutableMapOf(reference.targetName to "legacy-session-secret".encodeToByteArray()), + failClear = true, + ) + val store = MigratingDesktopSecretStore( + primary, + legacy, + RecordingSecretStoreAdoption(failWrites = true), + ) + + assertFailsWith { store.clear(reference) } + + assertNull(primary.load(reference)) + assertTrue(legacy.values.containsKey(reference.targetName)) } @Test diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 5f934a9d6..e5d7b837c 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -2405,7 +2405,9 @@ class JvmSupportIntakeTest { val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val upload = requireNotNull( + fixture.server.takeRequest(WINDOWS_REQUEST_START_TIMEOUT_SECONDS, TimeUnit.SECONDS), + ) assertTrue(fixture.intake.cancel()) submission.join() requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS))