From e8c0717dc16c21174369209dffb08bd416933407 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 5 Sep 2026 16:13:01 +0200 Subject: [PATCH 01/17] fix(android): reject self-provider picker roots --- .../AndroidFileSyncRootPicker.kt | 1 + .../AndroidLocalUploadPicker.kt | 17 ++++- .../AndroidMediaStoreSyncLocalTree.kt | 1 + .../NextcloudDocumentsContract.kt | 58 +++++++++++++++ .../AndroidPickerProviderFeedbackTest.kt | 73 +++++++++++++++++++ 5 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidPickerProviderFeedbackTest.kt diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt index 81509c2f4..ec67d5ab4 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt @@ -47,6 +47,7 @@ internal class AndroidFileSyncRootPicker(private val context: Context) { } val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION val result = runCatching { + requireExternalAndroidPickerUri(uri.toString(), context.applicationContext.packageName) context.contentResolver.takePersistableUriPermission(uri, flags) FileSyncLocalRoot(uri.toString(), queryDisplayName(context.contentResolver, uri)) } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt index 9d5436bc2..2cc78ad52 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt @@ -68,6 +68,7 @@ internal class AndroidLocalUploadPicker(context: Context) { return } val result = runCatching selectionResult@{ + requireExternalAndroidPickerUri(uri.toString(), appContext.packageName) val metadata = resolver.queryUploadMetadata(uri) val mimeType = resolver.getType(uri)?.trim()?.lowercase()?.takeIf(String::isNotBlank) if (!isAcceptedUploadMimeType(mimeType, selection.acceptedMimeTypes)) { @@ -162,9 +163,13 @@ internal class AndroidLocalUploadPicker(context: Context) { } if (cancelledAfterAcquire) return@selectionResult LocalUploadSelectionResult.Cancelled LocalUploadSelectionResult.Selected(file) - }.getOrElse { + }.getOrElse { failure -> LocalUploadSelectionResult.Rejected( - "The selected file could not be opened.", + if (failure is AndroidPickerUriRejectedException) { + failure.rejection.message + } else { + "The selected file could not be opened." + }, ) } resumeLocalUploadSelectionResult( @@ -652,6 +657,14 @@ internal class AndroidLocalUploadPicker(context: Context) { "The persisted local file metadata changed.", ) } + try { + requireExternalAndroidPickerUri(source.uri.toString(), appContext.packageName) + } catch (failure: AndroidPickerUriRejectedException) { + throw AndroidLocalUploadCapabilityUnavailableException( + "The persisted local file provider is not allowed.", + failure, + ) + } requireDurableUploadCapabilityReady(source.phase) return source } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt index 4ee7ca951..042225c45 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt @@ -25,6 +25,7 @@ internal fun createAndroidFileSyncLocalTree( root = resolveMediaStoreSyncRoot(rootId, Environment.getExternalStorageDirectory()), ) } else { + requireExternalAndroidPickerUri(rootId, appContext.packageName) AndroidSafFileSyncLocalTree( resolver = appContext.contentResolver, rootId = rootId, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContract.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContract.kt index 0e75e8c86..e4136755b 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContract.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContract.kt @@ -1,5 +1,8 @@ package dev.obiente.nextcloudnative +import java.net.URI +import java.net.URISyntaxException + internal const val NEXTCLOUD_DOCUMENTS_AUTHORITY_SUFFIX = ".documents" /** Matches the manifest's `${applicationId}.documents` authority for every build variant. */ @@ -7,3 +10,58 @@ internal fun nextcloudDocumentsAuthority(applicationId: String): String { require(applicationId.isNotBlank()) { "The application ID must not be blank." } return applicationId + NEXTCLOUD_DOCUMENTS_AUTHORITY_SUFFIX } + +internal enum class AndroidPickerUriRejection(val message: String) { + OwnDocumentsProvider("Files from nati.ve cannot be selected here."), + Invalid("The selected file provider returned an invalid URI."), +} + +internal class AndroidPickerUriRejectedException( + val rejection: AndroidPickerUriRejection, +) : IllegalArgumentException(rejection.message) + +internal fun requireExternalAndroidPickerUri( + uri: String, + applicationId: String, +): Unit { + androidPickerUriRejection(uri, applicationId)?.let { rejection -> + throw AndroidPickerUriRejectedException(rejection) + } +} + +internal fun androidPickerUriRejection( + uri: String, + applicationId: String, +): AndroidPickerUriRejection? { + val parsed = try { + URI(uri) + } catch (_: URISyntaxException) { + return AndroidPickerUriRejection.Invalid + } + if (!parsed.scheme.equals("content", ignoreCase = true)) { + return AndroidPickerUriRejection.Invalid + } + val authority = parsed.authority?.takeIf(String::isNotBlank) + ?: return AndroidPickerUriRejection.Invalid + if (authority.any { character -> + character.isWhitespace() || character.isISOControl() || character in ":/\\?#" + } + ) { + return AndroidPickerUriRejection.Invalid + } + val userSeparator = authority.indexOf('@') + val providerAuthority = when { + userSeparator < 0 -> authority + userSeparator != authority.lastIndexOf('@') -> return AndroidPickerUriRejection.Invalid + userSeparator == 0 -> return AndroidPickerUriRejection.Invalid + authority.take(userSeparator).any { character -> !character.isDigit() } -> + return AndroidPickerUriRejection.Invalid + else -> authority.substring(userSeparator + 1).takeIf(String::isNotBlank) + ?: return AndroidPickerUriRejection.Invalid + } + return if (providerAuthority.equals(nextcloudDocumentsAuthority(applicationId), ignoreCase = true)) { + AndroidPickerUriRejection.OwnDocumentsProvider + } else { + null + } +} diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidPickerProviderFeedbackTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidPickerProviderFeedbackTest.kt new file mode 100644 index 000000000..a7f12d0cc --- /dev/null +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidPickerProviderFeedbackTest.kt @@ -0,0 +1,73 @@ +package dev.obiente.nextcloudnative + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class AndroidPickerProviderFeedbackTest { + private val applicationId = "dev.obiente.nextcloudnative.dev" + private val ownAuthority = nextcloudDocumentsAuthority(applicationId) + + @Test + fun `own document and tree uris are rejected before grant or capability publication`() { + val sideEffects = mutableListOf() + val ownUris = listOf( + "content://$ownAuthority/document/nc2%3Aaccount%3Aincarnation%3Afile", + "content://$ownAuthority/tree/root/document/root%2Ffolder", + "content://${ownAuthority.uppercase()}/document/file", + "content://10@$ownAuthority/tree/root", + "content://dev%2Eobiente%2Enextcloudnative%2Edev%2Edocuments/document/file", + ) + + ownUris.forEach { uri -> + val failure = assertFailsWith { + requireExternalAndroidPickerUri(uri, applicationId) + sideEffects += "take-grant" + sideEffects += "publish-capability" + } + assertEquals(AndroidPickerUriRejection.OwnDocumentsProvider, failure.rejection) + } + + assertEquals(emptyList(), sideEffects) + } + + @Test + fun `malformed picker uris fail before durable state`() { + val sideEffects = mutableListOf() + val malformedUris = listOf( + "file://$ownAuthority/document/file", + "content:///document/file", + "content://user@external.documents/document/file", + "content://10@@external.documents/document/file", + "content://external%2Fdocuments/document/file", + "content://external.documents/%broken", + ) + + malformedUris.forEach { uri -> + val failure = assertFailsWith { + requireExternalAndroidPickerUri(uri, applicationId) + sideEffects += "create-durable-state" + } + assertEquals(AndroidPickerUriRejection.Invalid, failure.rejection) + } + + assertEquals(emptyList(), sideEffects) + } + + @Test + fun `unrelated external document and tree providers remain accepted`() { + val accepted = mutableListOf() + val externalUris = listOf( + "content://com.android.providers.downloads.documents/document/42", + "content://EXTERNAL.PROVIDER/tree/root/document/root%2Ffolder", + "content://10@external_provider/tree/root", + ) + + externalUris.forEach { uri -> + requireExternalAndroidPickerUri(uri, applicationId) + accepted += uri + } + + assertEquals(externalUris, accepted) + } +} From 65b0affa3deee73bb80f029f3f637bad34310dad Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 5 Sep 2026 17:22:23 +0200 Subject: [PATCH 02/17] chore(changelog): record picker provider guard --- changes/unreleased/android-picker-provider-feedback.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changes/unreleased/android-picker-provider-feedback.md diff --git a/changes/unreleased/android-picker-provider-feedback.md b/changes/unreleased/android-picker-provider-feedback.md new file mode 100644 index 000000000..29b31c948 --- /dev/null +++ b/changes/unreleased/android-picker-provider-feedback.md @@ -0,0 +1,7 @@ +category: fix +issue: none +pull: 446 +platforms: android +user-facing: yes + +Android file and folder pickers now reject the app's own document provider so selected uploads and sync roots always come from an independent storage source. From 7bcfee53f3e22b9d841cd03c95f860f49f050460 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 5 Sep 2026 17:56:00 +0200 Subject: [PATCH 03/17] fix(android): preflight self-provider sync roots --- .../nextcloudnative/AndroidFileSyncEngine.kt | 9 +- .../AndroidFileSyncExecutionCoordination.kt | 21 +++- .../AndroidFileSyncLocalTree.kt | 2 +- .../AndroidMediaStoreSyncLocalTree.kt | 13 +- .../AndroidSafDownloadOwnershipStore.kt | 13 ++ .../AndroidSafOwnedDownloadRetirement.kt | 118 ++++++++++++++++++ .../nextcloudnative/NextcloudDocumentIds.kt | 8 +- ...oidFileSyncProviderFeedbackRecoveryTest.kt | 86 +++++++++++++ 8 files changed, 262 insertions(+), 8 deletions(-) create mode 100644 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt create mode 100644 androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt index 06cb6de16..8f5646f48 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt @@ -318,7 +318,9 @@ internal class AndroidFileSyncEngine(context: Context) { var remoteCleanupRejected = false val removed = removeConfiguredFileSyncPair( reconcileLocalDownloads = { - reconcileSafDownloadsBeforePairRemoval(appContext, pair.localRootId) + reconcileSafDownloadsBeforePairRemoval( + appContext, pair.localRootId, androidSafOwnedDownloadRecoveryPaths(pair), + ) }, cleanRemoteUploads = { val cleanupResult = cleanupJvmFileSyncOwnedUploads( @@ -410,6 +412,10 @@ internal class AndroidFileSyncEngine(context: Context) { FileSyncRejectionScope.Preflight, ) } + androidFileSyncRootRejection(initialPair.localRootId, appContext.packageName)?.let { rejection -> + return FileSyncCenterActionResult.Rejected(rejection.message, FileSyncRejectionScope.Preflight) + } + val local = createAndroidFileSyncLocalTree(appContext, initialPair.localRootId) return withAndroidMediaBackupLedger(appContext, initialPair) { mediaLedger -> val remote = androidFileSyncOwnedRemoteTree( session, userId, initialPair, webDav, @@ -432,7 +438,6 @@ internal class AndroidFileSyncEngine(context: Context) { configuration.includesSyncPath(relativePath, kind) } val remoteEntries = remote.scan(includes).map(AndroidRemoteSyncDocument::entry) - val local = createAndroidFileSyncLocalTree(appContext, initialPair.localRootId) val contentReadBudget = AndroidFileSyncContentReadBudget() val scannedLocalDocuments = local.scan(includes, remote::shouldContinueTransfer) val strengthenedLocalDocuments = strengthenAndroidFileSyncReplacementEntries( diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt index ed2e2125b..be983d357 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt @@ -160,6 +160,7 @@ internal suspend fun commitConfiguredFileSyncPairRemoval( internal suspend fun reconcileSafDownloadsBeforePairRemoval( context: Context, localRootId: String, + localRecoveryPaths: Set, ): Boolean { if (!localRootId.startsWith("content://")) return true val shouldContinue = androidFileSyncJobContinuation(currentCoroutineContext()[Job]) @@ -187,7 +188,19 @@ internal suspend fun reconcileSafDownloadsBeforePairRemoval( } if (!shouldContinue()) throw CancellationException("Pair removal was cancelled.") val reconciled = reconcileSafDownloadsBeforePairRemoval(hasPersistedGrant, hasPendingRecovery) { - createAndroidFileSyncLocalTree(context, localRootId).reconcileOwnedDownloads(shouldContinue) + if ( + androidPickerUriRejection(localRootId, context.applicationContext.packageName) == + AndroidPickerUriRejection.OwnDocumentsProvider + ) { + reconcileOwnProviderSafDownloadsBeforePairRemoval( + context = context, + localRootId = localRootId, + localRecoveryPaths = localRecoveryPaths, + shouldContinue = shouldContinue, + ) + } else { + createAndroidFileSyncLocalTree(context, localRootId).reconcileOwnedDownloads(shouldContinue) + } } if (!shouldContinue()) throw CancellationException("Pair removal was cancelled.") return reconciled @@ -245,7 +258,11 @@ internal suspend fun retireAndroidFileSyncAccountPairs(context: Context, account retiredPairs = retiredPairs, retainedPairs = retainedPairs, reconcileLocalDownloads = { pair -> - reconcileSafDownloadsBeforePairRemoval(context, pair.localRootId) + reconcileSafDownloadsBeforePairRemoval( + context, + pair.localRootId, + androidSafOwnedDownloadRecoveryPaths(pair), + ) }, cancelSchedule = { pair -> scheduler.cancel(pair.id) }, cancelNotification = { pair -> diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt index 1eda6efa1..a9a814956 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt @@ -571,7 +571,7 @@ internal class AndroidSafFileSyncLocalTree( return listedChildren.filter { it.uri in visibleUris } } - private fun downloadPublisher( + internal fun downloadPublisher( parentUri: Uri, parentPath: String, shouldContinue: () -> Boolean = { !Thread.currentThread().isInterrupted }, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt index 042225c45..76ecb46fc 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMediaStoreSyncLocalTree.kt @@ -25,7 +25,9 @@ internal fun createAndroidFileSyncLocalTree( root = resolveMediaStoreSyncRoot(rootId, Environment.getExternalStorageDirectory()), ) } else { - requireExternalAndroidPickerUri(rootId, appContext.packageName) + androidFileSyncRootRejection(rootId, appContext.packageName)?.let { rejection -> + throw AndroidPickerUriRejectedException(rejection) + } AndroidSafFileSyncLocalTree( resolver = appContext.contentResolver, rootId = rootId, @@ -34,6 +36,15 @@ internal fun createAndroidFileSyncLocalTree( } } +internal fun androidFileSyncRootRejection( + rootId: String, + applicationId: String, +): AndroidPickerUriRejection? = if (rootId.startsWith(MEDIA_STORE_SYNC_ROOT_PREFIX)) { + null +} else { + androidPickerUriRejection(rootId, applicationId) +} + internal fun createAndroidSafDownloadOwnershipStore( context: Context, treeIdentity: String, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt index 6892af3d3..9dd0c98b0 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt @@ -25,6 +25,19 @@ internal class AndroidSafDownloadOwnershipStore( ownershipFiles().isNotEmpty() } + fun hasTreeScopedPendingTransactions(): Boolean = synchronized(LOCK) { + ownershipFiles(directory, listFiles).isNotEmpty() + } + + fun pendingTransactions(): List = synchronized(LOCK) { + ownershipRows().map(StoredOwnershipRow::transaction) + } + + fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = synchronized(LOCK) { + val scope = scopeDigest(directoryIdentity) + ownershipFiles().any { file -> ownershipReference(file)?.scope == scope } + } + override fun forDirectory(directoryIdentity: String): AndroidSafDownloadOwnership { require(directoryIdentity.isNotBlank()) return ScopedOwnership(scopeDigest(directoryIdentity)) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt new file mode 100644 index 000000000..39caec710 --- /dev/null +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -0,0 +1,118 @@ +package dev.obiente.nextcloudnative + +import android.content.Context +import android.net.Uri +import android.provider.DocumentsContract +import dev.obiente.nextcloudnative.app.FileSyncPair + +internal data class AndroidSafOwnedDownloadRecoveryDirectory( + val documentId: String, + val relativePath: String, +) + +internal fun androidSafOwnedDownloadRecoveryPaths(pair: FileSyncPair): Set = + (pair.baselines.asSequence().map { baseline -> baseline.relativePath } + + pair.workItems.asSequence().map { work -> work.relativePath }) + .toSet() + +internal fun androidSafOwnedDownloadRecoveryDirectories( + rootDocumentId: String, + localRecoveryPaths: Set, + recordedDocumentIds: Set = emptySet(), +): List { + val root = NextcloudDocumentIds.parse(rootDocumentId) + return buildSet { + add("") + localRecoveryPaths.mapTo(this, NextcloudDocumentIds::parentPath) + recordedDocumentIds.forEach { documentId -> + val reference = runCatching { NextcloudDocumentIds.parse(documentId) }.getOrNull() + ?: return@forEach + if (reference.accountKey != root.accountKey) return@forEach + val parentPath = NextcloudDocumentIds.parentPath(reference.path) + val relativePath = when { + root.path.isEmpty() -> parentPath + parentPath == root.path -> "" + parentPath.startsWith(root.path + "/") -> parentPath.removePrefix(root.path + "/") + else -> return@forEach + } + add(relativePath) + } + }.map { relativePath -> + val fullPath = listOf(root.path, relativePath).filter(String::isNotBlank).joinToString("/") + AndroidSafOwnedDownloadRecoveryDirectory( + documentId = NextcloudDocumentIds.documentId(root.accountKey, fullPath), + relativePath = relativePath, + ) + } +} + +internal inline fun reconcileRecordedAndroidSafDownloadDirectories( + candidates: List, + hasPendingRecovery: () -> Boolean, + hasPendingForDirectory: (Directory) -> Boolean, + reconcileDirectory: (Directory) -> Unit, +): Boolean { + if (!hasPendingRecovery()) return true + candidates.distinct().filter(hasPendingForDirectory).forEach(reconcileDirectory) + return !hasPendingRecovery() +} + +internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( + context: Context, + localRootId: String, + localRecoveryPaths: Set, + shouldContinue: () -> Boolean, +) { + val appContext = context.applicationContext + val treeUri = Uri.parse(localRootId) + val ownership = createAndroidSafDownloadOwnershipStore(appContext, localRootId) + val indexedOwnership = ownership.indexed() + val localTree = AndroidSafFileSyncLocalTree( + resolver = appContext.contentResolver, + rootId = localRootId, + downloadOwnershipStore = ownership, + ) + val recordedDocumentIds = ownership.pendingTransactions().asSequence() + .flatMap { transaction -> + sequenceOf(transaction.stageDocumentIdentity, transaction.backupDocumentIdentity) + } + .filterNotNull() + .mapNotNull { identity -> + runCatching { + identity.takeIf { + androidPickerUriRejection(it, appContext.packageName) == + AndroidPickerUriRejection.OwnDocumentsProvider + }?.let { DocumentsContract.getDocumentId(Uri.parse(it)) } + }.getOrNull() + } + .toSet() + val candidates = androidSafOwnedDownloadRecoveryDirectories( + rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri), + localRecoveryPaths = localRecoveryPaths, + recordedDocumentIds = recordedDocumentIds, + ).map { candidate -> + candidate to DocumentsContract.buildDocumentUriUsingTree(treeUri, candidate.documentId) + } + val hasRelevantPendingRecovery = { + ownership.hasTreeScopedPendingTransactions() || candidates.any { (_, directoryUri) -> + ownership.hasPendingTransactionsForDirectory(directoryUri.toString()) + } + } + check( + reconcileRecordedAndroidSafDownloadDirectories( + candidates = candidates, + hasPendingRecovery = hasRelevantPendingRecovery, + hasPendingForDirectory = { (_, directoryUri) -> + ownership.hasPendingTransactionsForDirectory(directoryUri.toString()) + }, + reconcileDirectory = { (candidate, directoryUri) -> + localTree.downloadPublisher( + parentUri = directoryUri, + parentPath = candidate.relativePath, + shouldContinue = shouldContinue, + ownershipDirectory = indexedOwnership, + ).reconcileForSync() + }, + ), + ) { "A local download still needs safe recovery." } +} diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentIds.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentIds.kt index a2d4d5799..25acb8ff4 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentIds.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentIds.kt @@ -39,10 +39,14 @@ internal object NextcloudDocumentIds { return "$PREFIX:$accountKey:" } - fun documentId(session: NextcloudSession, path: String): String { + fun documentId(session: NextcloudSession, path: String): String = + documentId(accountKey(session), path) + + fun documentId(accountKey: String, path: String): String { + require(accountKeyPattern.matches(accountKey)) { "Invalid document account." } val normalizedPath = normalizePath(path) val encodedPath = encoder.encodeToString(normalizedPath.encodeToByteArray()) - return "$PREFIX:${accountKey(session)}:$encodedPath" + return "$PREFIX:$accountKey:$encodedPath" } fun parse(documentId: String): NextcloudDocumentReference { diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt new file mode 100644 index 000000000..62637894a --- /dev/null +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -0,0 +1,86 @@ +package dev.obiente.nextcloudnative + +import dev.obiente.nextcloudnative.app.FileSyncBaseline +import dev.obiente.nextcloudnative.app.FileSyncConfiguration +import dev.obiente.nextcloudnative.app.FileSyncPair +import dev.obiente.nextcloudnative.app.SyncEntryKind +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlinx.coroutines.runBlocking + +class AndroidFileSyncProviderFeedbackRecoveryTest { + private val applicationId = "dev.obiente.nextcloudnative.dev" + private val ownAuthority = nextcloudDocumentsAuthority(applicationId) + private val accountKey = "0123456789abcdef0123456789abcdef" + + @Test + fun `restored own provider root stops before remote preparation`() { + var remoteCalls = 0 + val rejection = androidFileSyncRootRejection( + "content://$ownAuthority/tree/${NextcloudDocumentIds.rootId(accountKey)}", + applicationId, + ) + if (rejection == null) remoteCalls += 1 + + assertEquals(AndroidPickerUriRejection.OwnDocumentsProvider, rejection) + assertEquals(0, remoteCalls) + } + + @Test + fun `legacy own provider recovery visits only recorded parent and allows removal`() = runBlocking { + val pair = FileSyncPair( + id = "pair", + accountId = accountKey, + localRootId = "content://$ownAuthority/tree/${NextcloudDocumentIds.rootId(accountKey)}", + remoteRootPath = "Mirror", + configuration = FileSyncConfiguration(deviceLabel = "Phone"), + baselines = listOf( + FileSyncBaseline("Archive/kept.txt", SyncEntryKind.File, "local", "remote"), + ), + ) + val recordedStageId = NextcloudDocumentIds.documentId( + accountKey, + "Pending/.nextcloud-native-download-123e4567-e89b-12d3-a456-426614174000", + ) + val candidates = androidSafOwnedDownloadRecoveryDirectories( + NextcloudDocumentIds.rootId(accountKey), + androidSafOwnedDownloadRecoveryPaths(pair), + setOf(recordedStageId), + ) + val pendingDocumentIds = mutableSetOf(NextcloudDocumentIds.documentId(accountKey, "Pending")) + val events = mutableListOf() + + val removed = removeConfiguredFileSyncPair( + reconcileLocalDownloads = { + reconcileRecordedAndroidSafDownloadDirectories( + candidates = candidates, + hasPendingRecovery = pendingDocumentIds::isNotEmpty, + hasPendingForDirectory = { candidate -> candidate.documentId in pendingDocumentIds }, + reconcileDirectory = { candidate -> + events += "reconcile:${candidate.relativePath}" + pendingDocumentIds -= candidate.documentId + }, + ) + }, + cleanRemoteUploads = { events += "remote-cleanup"; true }, + cleanLedger = { events += "ledger-cleanup" }, + persistRemoval = { events += "persist-removal" }, + cancelSchedule = { events += "cancel-schedule" }, + releaseLocalGrant = { events += "release-grant" }, + ) + + assertTrue(removed) + assertEquals( + listOf( + "reconcile:Pending", + "remote-cleanup", + "ledger-cleanup", + "persist-removal", + "cancel-schedule", + "release-grant", + ), + events, + ) + } +} From af7d7bc4ebe14609aaf950b9423b3f088defa285 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 01:21:18 +0200 Subject: [PATCH 04/17] fix(android): recover legacy picker roots safely --- .../nextcloudnative/AndroidAccountRemoval.kt | 4 + .../AndroidFileSyncExecutionCoordination.kt | 33 ++++++++ .../AndroidFileSyncLocalTree.kt | 2 +- .../AndroidFileSyncRootPicker.kt | 11 ++- .../AndroidSafDownloadOwnershipStore.kt | 19 ++++- .../AndroidSafDownloadPublication.kt | 3 + .../AndroidSafOwnedDownloadRetirement.kt | 52 +++++++++++-- ...oidFileSyncProviderFeedbackRecoveryTest.kt | 77 +++++++++++++++++++ .../AndroidFileSyncRootPickerTest.kt | 25 ++++++ .../AndroidSafDownloadOwnershipIndexTest.kt | 32 ++++++++ 10 files changed, 246 insertions(+), 12 deletions(-) create mode 100644 androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPickerTest.kt diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt index 6cbb56f40..a38a18e7b 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt @@ -88,6 +88,10 @@ internal suspend fun preflightAndroidAccountRemoval(context: Context, session: N internal suspend fun prepareAndroidAccountRemoval(context: Context, session: NextcloudSession) { preflightAndroidAccountRemoval(context, session) + reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRemoval( + context, + NextcloudDocumentIds.accountKey(session), + ) ANDROID_FILE_RANGE_SESSION_COORDINATOR.quiesce(NextcloudDocumentIds.accountKey(session)) } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt index be983d357..80f13ed2a 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt @@ -276,6 +276,39 @@ internal suspend fun retireAndroidFileSyncAccountPairs(context: Context, account } } +internal suspend fun reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRemoval( + context: Context, + accountId: String, +) { + AndroidFileSyncEngine.ENGINE_LOCK.withLock { + reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( + pairs = AndroidFileSyncStore(context).load().coordinator.pairs, + accountId = accountId, + reconcileLocalDownloads = { pair -> + reconcileSafDownloadsBeforePairRemoval( + context = context, + localRootId = pair.localRootId, + localRecoveryPaths = androidSafOwnedDownloadRecoveryPaths(pair), + ) + }, + ) + } +} + +internal suspend fun reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( + pairs: List, + accountId: String, + reconcileLocalDownloads: suspend (FileSyncPair) -> Boolean, +) { + require(accountId.isNotBlank()) + pairs.filter { pair -> pair.accountId == accountId }.forEach { pair -> + check(reconcileLocalDownloads(pair)) { + "A local download still needs safe recovery. Run this folder sync before removing the account." + } + currentCoroutineContext().ensureActive() + } +} + internal suspend fun retireConfiguredFileSyncAccountPairs( retiredPairs: List, retainedPairs: List, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt index a9a814956..47d7a796a 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt @@ -98,7 +98,7 @@ internal class AndroidSafFileSyncLocalTree( } } - private fun indexRecoveryLocationsIfNeeded( + internal fun indexRecoveryLocationsIfNeeded( ownershipDirectory: AndroidSafDownloadOwnershipDirectory, shouldContinue: () -> Boolean, ) = indexAndroidSafRecoveryLocationsIfNeeded(ownershipDirectory) { diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt index ec67d5ab4..7261abb1b 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPicker.kt @@ -10,6 +10,7 @@ import dev.obiente.nextcloudnative.app.FileSyncLocalRoot import kotlinx.coroutines.CancellableContinuation import kotlinx.coroutines.suspendCancellableCoroutine import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException /** * Single-flight bridge from common suspend APIs to Android's native document-tree picker. @@ -51,8 +52,7 @@ internal class AndroidFileSyncRootPicker(private val context: Context) { context.contentResolver.takePersistableUriPermission(uri, flags) FileSyncLocalRoot(uri.toString(), queryDisplayName(context.contentResolver, uri)) } - result.onSuccess(continuation::resume) - .onFailure { continuation.cancel(it) } + resumeAndroidFileSyncPickerContinuation(continuation, result) } private fun queryDisplayName(resolver: ContentResolver, treeUri: Uri): String { @@ -69,3 +69,10 @@ internal class AndroidFileSyncRootPicker(private val context: Context) { }.orEmpty().ifBlank { "Selected folder" } } } + +internal fun resumeAndroidFileSyncPickerContinuation( + continuation: CancellableContinuation, + result: Result, +) { + result.fold(continuation::resume, continuation::resumeWithException) +} diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt index 9dd0c98b0..b1ef4ba24 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt @@ -33,7 +33,7 @@ internal class AndroidSafDownloadOwnershipStore( ownershipRows().map(StoredOwnershipRow::transaction) } - fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = synchronized(LOCK) { + override fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = synchronized(LOCK) { val scope = scopeDigest(directoryIdentity) ownershipFiles().any { file -> ownershipReference(file)?.scope == scope } } @@ -56,6 +56,8 @@ internal class AndroidSafDownloadOwnershipStore( private val referencesByToken = references.associateByTo(mutableMapOf()) { reference -> reference.token } private val rowsByToken = mutableMapOf() private val observedScopesByToken = mutableMapOf>() + private val observedDirectoryIdentitiesByScope = mutableMapOf() + private val observedNamesByScope = mutableMapOf>() init { check(referencesByToken.size == references.size) { @@ -65,6 +67,19 @@ internal class AndroidSafDownloadOwnershipStore( override fun hasPendingTransactions(): Boolean = referencesByToken.isNotEmpty() + override fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = synchronized(LOCK) { + val scope = scopeDigest(directoryIdentity) + IndexedScopedOwnership(scope).transactions(observedNamesByScope[scope].orEmpty()).isNotEmpty() + } + + override fun observedPendingDirectoryIdentities(): Set = synchronized(LOCK) { + observedDirectoryIdentitiesByScope.mapNotNullTo(linkedSetOf()) { (scope, identity) -> + identity.takeIf { + IndexedScopedOwnership(scope).transactions(observedNamesByScope[scope].orEmpty()).isNotEmpty() + } + } + } + override fun forDirectory(directoryIdentity: String): AndroidSafDownloadOwnership { require(directoryIdentity.isNotBlank()) return IndexedScopedOwnership(scopeDigest(directoryIdentity)) @@ -75,6 +90,8 @@ internal class AndroidSafDownloadOwnershipStore( observedNames: Set, ) = synchronized(LOCK) { val scope = scopeDigest(directoryIdentity) + observedDirectoryIdentitiesByScope[scope] = directoryIdentity + observedNamesByScope[scope] = observedNames.toSet() observedRecoveryTokens(observedNames).forEach { token -> observedScopesByToken.getOrPut(token, ::mutableSetOf).add(scope) } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadPublication.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadPublication.kt index aee8c9eec..c16dd4424 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadPublication.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadPublication.kt @@ -28,6 +28,9 @@ internal interface AndroidSafDownloadOwnership { internal interface AndroidSafDownloadOwnershipDirectory { fun hasPendingTransactions(): Boolean + fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = + forDirectory(directoryIdentity).transactions().isNotEmpty() + fun observedPendingDirectoryIdentities(): Set = emptySet() fun forDirectory(directoryIdentity: String): AndroidSafDownloadOwnership fun observeRecoveryNames(directoryIdentity: String, observedNames: Set) = Unit } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt index 39caec710..779e16982 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -4,6 +4,7 @@ import android.content.Context import android.net.Uri import android.provider.DocumentsContract import dev.obiente.nextcloudnative.app.FileSyncPair +import kotlinx.coroutines.CancellationException internal data class AndroidSafOwnedDownloadRecoveryDirectory( val documentId: String, @@ -46,17 +47,44 @@ internal fun androidSafOwnedDownloadRecoveryDirectories( } } -internal inline fun reconcileRecordedAndroidSafDownloadDirectories( +internal fun androidSafOwnedDownloadRecoveryDirectory( + rootDocumentId: String, + directoryDocumentId: String, +): AndroidSafOwnedDownloadRecoveryDirectory? { + val root = runCatching { NextcloudDocumentIds.parse(rootDocumentId) }.getOrNull() ?: return null + val directory = runCatching { NextcloudDocumentIds.parse(directoryDocumentId) }.getOrNull() ?: return null + if (directory.accountKey != root.accountKey) return null + val relativePath = when { + root.path.isEmpty() -> directory.path + directory.path == root.path -> "" + directory.path.startsWith(root.path + "/") -> directory.path.removePrefix(root.path + "/") + else -> return null + } + return AndroidSafOwnedDownloadRecoveryDirectory(directoryDocumentId, relativePath) +} + +internal fun reconcileRecordedAndroidSafDownloadDirectories( candidates: List, hasPendingRecovery: () -> Boolean, hasPendingForDirectory: (Directory) -> Boolean, + shouldContinue: () -> Boolean = { true }, reconcileDirectory: (Directory) -> Unit, ): Boolean { if (!hasPendingRecovery()) return true - candidates.distinct().filter(hasPendingForDirectory).forEach(reconcileDirectory) + candidates.distinct().forEach { candidate -> + requireAndroidSafRetirementContinuation(shouldContinue) + if (!hasPendingForDirectory(candidate)) return@forEach + requireAndroidSafRetirementContinuation(shouldContinue) + reconcileDirectory(candidate) + } + requireAndroidSafRetirementContinuation(shouldContinue) return !hasPendingRecovery() } +internal fun requireAndroidSafRetirementContinuation(shouldContinue: () -> Boolean) { + if (!shouldContinue()) throw CancellationException("Folder sync recovery was cancelled.") +} + internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( context: Context, localRootId: String, @@ -72,6 +100,7 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( rootId = localRootId, downloadOwnershipStore = ownership, ) + localTree.indexRecoveryLocationsIfNeeded(indexedOwnership, shouldContinue) val recordedDocumentIds = ownership.pendingTransactions().asSequence() .flatMap { transaction -> sequenceOf(transaction.stageDocumentIdentity, transaction.backupDocumentIdentity) @@ -86,25 +115,32 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( }.getOrNull() } .toSet() - val candidates = androidSafOwnedDownloadRecoveryDirectories( + val recordedCandidates = androidSafOwnedDownloadRecoveryDirectories( rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri), localRecoveryPaths = localRecoveryPaths, recordedDocumentIds = recordedDocumentIds, ).map { candidate -> candidate to DocumentsContract.buildDocumentUriUsingTree(treeUri, candidate.documentId) } - val hasRelevantPendingRecovery = { - ownership.hasTreeScopedPendingTransactions() || candidates.any { (_, directoryUri) -> - ownership.hasPendingTransactionsForDirectory(directoryUri.toString()) - } + val relocatedCandidates = indexedOwnership.observedPendingDirectoryIdentities().mapNotNull { identity -> + val directoryUri = runCatching { Uri.parse(identity) }.getOrNull() ?: return@mapNotNull null + val documentId = runCatching { DocumentsContract.getDocumentId(directoryUri) }.getOrNull() + ?: return@mapNotNull null + androidSafOwnedDownloadRecoveryDirectory( + rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri), + directoryDocumentId = documentId, + )?.let { candidate -> candidate to directoryUri } } + val candidates = recordedCandidates + relocatedCandidates + val hasRelevantPendingRecovery = indexedOwnership::hasPendingTransactions check( reconcileRecordedAndroidSafDownloadDirectories( candidates = candidates, hasPendingRecovery = hasRelevantPendingRecovery, hasPendingForDirectory = { (_, directoryUri) -> - ownership.hasPendingTransactionsForDirectory(directoryUri.toString()) + indexedOwnership.hasPendingTransactionsForDirectory(directoryUri.toString()) }, + shouldContinue = shouldContinue, reconcileDirectory = { (candidate, directoryUri) -> localTree.downloadPublisher( parentUri = directoryUri, diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt index 62637894a..05af96f17 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -6,7 +6,9 @@ import dev.obiente.nextcloudnative.app.FileSyncPair import dev.obiente.nextcloudnative.app.SyncEntryKind import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertTrue +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking class AndroidFileSyncProviderFeedbackRecoveryTest { @@ -83,4 +85,79 @@ class AndroidFileSyncProviderFeedbackRecoveryTest { events, ) } + + @Test + fun `account removal recovers target downloads before credential deletion`() = runBlocking { + val retained = pair("retained", "other-account") + val first = pair("first", accountKey) + val second = pair("second", accountKey) + val events = mutableListOf() + + removeAndroidAccountCredentialData( + active = true, + prepareAccountRemoval = { + reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( + pairs = listOf(retained, first, second), + accountId = accountKey, + reconcileLocalDownloads = { pair -> + events += "recover:${pair.id}" + true + }, + ) + }, + removeQueuedUploads = { events += "remove-owned-state" }, + clearActiveAccount = { events += "delete-credential" }, + rollbackActiveRemoval = {}, + persistInactiveRemoval = {}, + rollbackInactiveRemoval = {}, + ) + + assertEquals( + listOf("recover:first", "recover:second", "delete-credential", "remove-owned-state"), + events, + ) + } + + @Test + fun `cancelled own provider retirement stops before the next directory`() { + val events = mutableListOf() + var continuationChecks = 0 + + assertFailsWith { + reconcileRecordedAndroidSafDownloadDirectories( + candidates = listOf("first", "second"), + hasPendingRecovery = { true }, + hasPendingForDirectory = { + events += "check:$it" + true + }, + shouldContinue = { + continuationChecks += 1 + continuationChecks < 3 + }, + reconcileDirectory = { events += "reconcile:$it" }, + ) + } + + assertEquals(listOf("check:first", "reconcile:first"), events) + } + + @Test + fun `relocated recovery directory keeps its exact relative path`() { + val root = NextcloudDocumentIds.documentId(accountKey, "Sync") + val relocated = NextcloudDocumentIds.documentId(accountKey, "Sync/Moved/Parent") + + assertEquals( + AndroidSafOwnedDownloadRecoveryDirectory(relocated, "Moved/Parent"), + androidSafOwnedDownloadRecoveryDirectory(root, relocated), + ) + } + + private fun pair(id: String, owner: String) = FileSyncPair( + id = id, + accountId = owner, + localRootId = "content://external/tree/$id", + remoteRootPath = id, + configuration = FileSyncConfiguration(deviceLabel = "Phone"), + ) } diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPickerTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPickerTest.kt new file mode 100644 index 000000000..ce5a946a3 --- /dev/null +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncRootPickerTest.kt @@ -0,0 +1,25 @@ +package dev.obiente.nextcloudnative + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.suspendCancellableCoroutine + +class AndroidFileSyncRootPickerTest { + @Test + fun `provider rejection remains a typed failure instead of coroutine cancellation`() { + val rejection = AndroidPickerUriRejectedException(AndroidPickerUriRejection.OwnDocumentsProvider) + + val thrown = assertFailsWith { + runBlocking { + suspendCancellableCoroutine { continuation -> + resumeAndroidFileSyncPickerContinuation(continuation, Result.failure(rejection)) + } + } + } + + assertEquals(AndroidPickerUriRejection.OwnDocumentsProvider, thrown.rejection) + assertEquals(AndroidPickerUriRejection.OwnDocumentsProvider.message, thrown.message) + } +} diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt index 21b0a5522..83a436dc3 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt @@ -206,6 +206,7 @@ class AndroidSafDownloadOwnershipIndexTest { index.observeRecoveryNames(relocatedScope, setOf(relocatedName)) assertEquals(emptyList(), index.forDirectory(originalScope).transactions()) + assertEquals(setOf(relocatedScope), index.observedPendingDirectoryIdentities()) assertEquals( listOf(transaction), index.forDirectory(relocatedScope).transactions(setOf(relocatedName)), @@ -215,6 +216,37 @@ class AndroidSafDownloadOwnershipIndexTest { } } + @Test + fun `indexed directory membership does not relist ownership rows`() { + val root = Files.createTempDirectory("saf-download-ownership-index-membership-").toFile() + try { + val pendingScope = "content://provider/tree/root/document/pending" + val store = AndroidSafDownloadOwnershipStore(root) + store.forDirectory(pendingScope).add(authenticatedRelocationTransaction()) + var listingCount = 0 + val indexed = AndroidSafDownloadOwnershipStore( + directory = root, + listFiles = { + listingCount += 1 + root.listFiles() + }, + ).indexed() + + repeat(20_000) { candidate -> + assertEquals( + candidate == 17, + indexed.hasPendingTransactionsForDirectory( + if (candidate == 17) pendingScope else "content://provider/tree/root/document/$candidate", + ), + ) + } + + assertEquals(1, listingCount) + } finally { + root.deleteRecursively() + } + } + @Test fun `token-only document in another directory cannot relocate pending ownership`() { val root = Files.createTempDirectory("saf-download-ownership-token-collision-").toFile() From 75da6a5393229df5efe7acfb0b2dfab8917d28bb Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 01:44:03 +0200 Subject: [PATCH 05/17] fix(android): prepare account removal before lease --- .../AndroidAccountCredentialController.kt | 19 ++++--- .../nextcloudnative/AndroidAccountRemoval.kt | 14 ++++- .../AndroidAccountOperationGuardTest.kt | 53 +++++++++++++++++++ 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt index 3b4b0ad16..ae64d36ff 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt @@ -158,11 +158,12 @@ internal class AndroidAccountCredentialController( val session = current.sessions[accountId] ?: return@withLock removeUnavailableAccount(accountId, current) val pendingCleanup = pendingAndroidAccountRemovalCleanup(session) - withAndroidAccountRemovalLease(NextcloudDocumentIds.accountKey(session)) { + withPreparedAndroidAccountRemovalLease( + NextcloudDocumentIds.accountKey(session), prepare = { prepareAccountRemoval(session) }, + ) { val active = current.registry.activeAccountId == accountId removeAndroidAccountCredentialData( active = active, - prepareAccountRemoval = { prepareAccountRemoval(session) }, removeQueuedUploads = { removeQueuedUploads(session) }, clearActiveAccount = { clearSession(current, pendingCleanup) }, rollbackActiveRemoval = { @@ -193,11 +194,11 @@ internal class AndroidAccountCredentialController( val unavailableSession = NextcloudSession(target.record.serverUrl, target.record.loginName, appPassword = "") val accountIdentity = NextcloudDocumentIds.accountKey(unavailableSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(unavailableSession) - withAndroidAccountRemovalLease(accountIdentity) { + withPreparedAndroidAccountRemovalLease(accountIdentity, prepare = { prepareAccountRemoval(unavailableSession) }) { removeUnavailableAndroidAccountCredentialData( accountIdentity = accountIdentity, active = target.wasActive, - prepareAccountRemoval = { prepareAccountRemoval(unavailableSession) }, + prepareAccountRemoval = {}, removeAccountOwnedWorkWithoutCredentials = { identity -> retryQueuedUploadsCleanupWithoutCredentials( pendingCleanup.accountStorageKey, @@ -267,10 +268,11 @@ internal class AndroidAccountCredentialController( } else { val accountIdentity = NextcloudDocumentIds.accountKey(session) val pendingCleanup = pendingAndroidAccountRemovalCleanup(session) - withAndroidAccountRemovalLease(accountIdentity) { + withPreparedAndroidAccountRemovalLease( + accountIdentity, prepare = { prepareAccountRemoval(session) }, + ) { removeAndroidAccountCredentialData( active = true, - prepareAccountRemoval = { prepareAccountRemoval(session) }, removeQueuedUploads = { removeQueuedUploads(session) }, clearActiveAccount = { clearSession(read.state, pendingCleanup) }, rollbackActiveRemoval = { @@ -342,9 +344,10 @@ internal class AndroidAccountCredentialController( if (activeSession != null) { val accountIdentity = NextcloudDocumentIds.accountKey(activeSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(activeSession) - withAndroidAccountRemovalLease(accountIdentity) { + withPreparedAndroidAccountRemovalLease( + accountIdentity, prepare = { prepareAccountRemoval(activeSession) }, + ) { removeRecoveredAndroidAccountCredentialData( - prepareAccountRemoval = { prepareAccountRemoval(activeSession) }, removeQueuedUploads = { removeQueuedUploads(activeSession) }, clearRecoveredAccount = { persistRecoveredInvalidStoreAfterClear(current, suspectEncrypted, pendingCleanup) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt index a38a18e7b..00d8a9aca 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt @@ -30,6 +30,16 @@ internal suspend fun withAndroidAccountRemovalLease( action = action, ) +internal suspend fun withPreparedAndroidAccountRemovalLease( + accountIdentity: String, + guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, + prepare: suspend () -> Unit, + action: suspend () -> Result, +): Result { + prepare() + return withAndroidAccountRemovalLease(accountIdentity, guard, action) +} + internal suspend fun revokeAndroidSessionAfterRemovalPreflight( preflight: suspend () -> Unit, revoke: suspend () -> Unit, @@ -67,8 +77,8 @@ internal suspend fun revokeAndroidSessionWithAccountLease( preflight: suspend () -> Unit, revoke: suspend () -> Unit, removeLocalAccount: suspend () -> Unit, -) = withAndroidAccountRemovalLease(accountIdentity, guard) { - revokeAndroidSessionAfterRemovalPreflight(preflight, revoke, removeLocalAccount) +) = withPreparedAndroidAccountRemovalLease(accountIdentity, guard, preflight) { + revokeAndroidSessionAfterRemovalPreflight({}, revoke, removeLocalAccount) } internal enum class AndroidAccountDocumentGrantScope(val pathSegment: String) { diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt index 3f2fc3801..8a4b8cc33 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt @@ -536,6 +536,59 @@ class AndroidAccountOperationGuardTest { assertEquals(replacement, current) } + @Test + fun removalPreparationCanUseTheRetainedReadLeaseBeforeRemovalBecomesExclusive() = runBlocking { + val guard = AndroidAccountOperationGuard() + val accountIdentity = "account-a" + val events = mutableListOf() + + withTimeout(1_000L) { + withPreparedAndroidAccountRemovalLease( + accountIdentity = accountIdentity, + guard = guard, + prepare = { + guard.withAccount(accountIdentity) { events += "provider-read" } + }, + ) { + events += "remove" + } + } + + assertEquals(listOf("provider-read", "remove"), events) + } + + @Test + fun accountWorkStartedAfterPreparationMakesRemovalFailClosed() = runBlocking { + val guard = AndroidAccountOperationGuard() + val accountIdentity = "account-a" + var removalEntered = false + var competingLease: AndroidAccountOperationLease? = null + + val failure = try { + assertFailsWith { + withTimeout(1_000L) { + withPreparedAndroidAccountRemovalLease( + accountIdentity = accountIdentity, + guard = guard, + prepare = { + competingLease = guard.acquireBlocking(accountIdentity) + }, + ) { + removalEntered = true + } + } + } + } finally { + competingLease?.close() + } + + assertEquals( + "Finish or discard pending document changes before removing this account.", + failure.message, + ) + assertFalse(removalEntered) + } + @Test fun directDocumentMutationLeaseRejectsReauthenticatedSessionAndReleasesTheGuard() = runBlocking { val guard = AndroidAccountOperationGuard() From b5c8ccb293b27e60859d4f8cee8f0f655b215ae4 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 02:47:03 +0200 Subject: [PATCH 06/17] fix(android): bind provider recovery to removed account --- .../nextcloudnative/AndroidAccountRemoval.kt | 1 + .../AndroidDocumentsProviderSessionBinding.kt | 271 ++++++++++++++++ .../AndroidFileSyncExecutionCoordination.kt | 5 + .../AndroidFileSyncLocalTree.kt | 64 ++-- .../AndroidSafFileSyncPublicationDirectory.kt | 46 +++ .../AndroidSafOwnedDownloadRetirement.kt | 3 + .../NextcloudDocumentsProvider.kt | 18 +- .../AndroidDocumentsProviderManifestTest.kt | 40 +++ .../NextcloudDocumentsContractTest.kt | 296 ++++++++++++++++++ 9 files changed, 702 insertions(+), 42 deletions(-) create mode 100644 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt create mode 100644 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafFileSyncPublicationDirectory.kt create mode 100644 androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderManifestTest.kt diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt index 00d8a9aca..111224dee 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt @@ -101,6 +101,7 @@ internal suspend fun prepareAndroidAccountRemoval(context: Context, session: Nex reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRemoval( context, NextcloudDocumentIds.accountKey(session), + session, ) ANDROID_FILE_RANGE_SESSION_COORDINATOR.quiesce(NextcloudDocumentIds.accountKey(session)) } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt new file mode 100644 index 000000000..bb04fc8ca --- /dev/null +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt @@ -0,0 +1,271 @@ +package dev.obiente.nextcloudnative + +import android.net.Uri +import android.os.Binder +import android.os.Process +import android.provider.DocumentsContract +import dev.obiente.nextcloudnative.app.NextcloudSession +import java.io.FileNotFoundException + +internal enum class AndroidDocumentsProviderRecoveryOperation { + QueryDocument, + QueryChildren, + OpenRead, + OpenWrite, + Create, + Rename, + Delete, + Move, +} + +internal data class AndroidDocumentsProviderResolvedSession( + val session: NextcloudSession, + val recoveryAuthorized: Boolean, +) + +private class AndroidDocumentsProviderRecoveryPermit( + val session: NextcloudSession, + val documentId: String, + val operation: AndroidDocumentsProviderRecoveryOperation, + var consumed: Boolean = false, +) + +private val ANDROID_DOCUMENTS_PROVIDER_RECOVERY_PERMITS = + ThreadLocal>() + +internal class AndroidDocumentsProviderRecoveryAccess( + private val session: NextcloudSession?, +) { + fun run( + document: Uri, + operation: AndroidDocumentsProviderRecoveryOperation, + action: (Uri) -> Result, + ): Result { + val documentId = DocumentsContract.getDocumentId(document) + val ordinaryUri = androidDocumentsProviderRecoveryUri( + documentId = documentId, + operation = operation, + buildDocumentUri = { document }, + buildChildDocumentsUri = { id -> DocumentsContract.buildChildDocumentsUriUsingTree(document, id) }, + ) + val bound = session ?: return action(ordinaryUri) + val authority = requireNotNull(document.authority) { "The recovery document authority is missing." } + val recoveryUri = androidDocumentsProviderRecoveryUri( + documentId = documentId, + operation = operation, + buildDocumentUri = { id -> DocumentsContract.buildDocumentUri(authority, id) }, + buildChildDocumentsUri = { id -> DocumentsContract.buildChildDocumentsUri(authority, id) }, + ) + return withAndroidDocumentsProviderRecoveryPermit(bound, documentId, operation) { + action(recoveryUri) + } + } + + fun normalizeResult(document: Uri, result: Uri?): Uri? = + normalizeAndroidDocumentsProviderRecoveryResult( + recoveryEnabled = session != null, + document = document, + result = result, + documentIdOf = DocumentsContract::getDocumentId, + buildTreeDocumentUri = DocumentsContract::buildDocumentUriUsingTree, + ) +} + +internal fun androidDocumentsProviderRecoveryUri( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + buildDocumentUri: (String) -> Uri, + buildChildDocumentsUri: (String) -> Uri, +): Uri = when (operation) { + AndroidDocumentsProviderRecoveryOperation.QueryChildren -> buildChildDocumentsUri(documentId) + AndroidDocumentsProviderRecoveryOperation.OpenRead, + AndroidDocumentsProviderRecoveryOperation.Rename, + AndroidDocumentsProviderRecoveryOperation.Delete, + -> buildDocumentUri(documentId) + AndroidDocumentsProviderRecoveryOperation.QueryDocument, + AndroidDocumentsProviderRecoveryOperation.OpenWrite, + AndroidDocumentsProviderRecoveryOperation.Create, + AndroidDocumentsProviderRecoveryOperation.Move, + -> error("The document operation is not permitted for recovery.") +} + +internal fun normalizeAndroidDocumentsProviderRecoveryResult( + recoveryEnabled: Boolean, + document: Uri, + result: Uri?, + documentIdOf: (Uri) -> String, + buildTreeDocumentUri: (Uri, String) -> Uri, +): Uri? = result?.let { renamed -> + if (recoveryEnabled) buildTreeDocumentUri(document, documentIdOf(renamed)) else renamed +} + +/** + * Grants one exact provider operation to synchronous self-provider recovery. The provider must stay + * in this app process because the one-shot authority intentionally cannot cross thread boundaries. + */ +internal fun withAndroidDocumentsProviderRecoveryPermit( + session: NextcloudSession, + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + action: () -> Result, +): Result { + NextcloudDocumentIds.requireForSession(documentId, session) + requireAndroidDocumentsProviderRecoveryOperation(operation) + val permit = AndroidDocumentsProviderRecoveryPermit(session, documentId, operation) + val permits = ANDROID_DOCUMENTS_PROVIDER_RECOVERY_PERMITS.get() + ?: mutableListOf().also { created -> + ANDROID_DOCUMENTS_PROVIDER_RECOVERY_PERMITS.set(created) + } + permits += permit + return try { + action() + } finally { + check(permits.remove(permit)) { "The document recovery permit was already cleared." } + if (permits.isEmpty()) ANDROID_DOCUMENTS_PROVIDER_RECOVERY_PERMITS.remove() + } +} + +internal fun resolveAndroidDocumentsProviderSession( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + allowRecoveryPermit: Boolean, + loadActiveSession: () -> NextcloudSession?, +): AndroidDocumentsProviderResolvedSession? { + val accountIdentity = runCatching { NextcloudDocumentIds.parse(documentId).accountKey }.getOrNull() + ?: return null + if (allowRecoveryPermit) { + ANDROID_DOCUMENTS_PROVIDER_RECOVERY_PERMITS.get() + .orEmpty() + .asReversed() + .firstOrNull { permit -> + !permit.consumed && permit.documentId == documentId && permit.operation == operation + } + ?.let { permit -> + permit.consumed = true + return AndroidDocumentsProviderResolvedSession(permit.session, recoveryAuthorized = true) + } + } + loadActiveSession()?.takeIf { session -> + NextcloudDocumentIds.accountKey(session) == accountIdentity + }?.let { session -> return AndroidDocumentsProviderResolvedSession(session, recoveryAuthorized = false) } + return null +} + +private fun requireAndroidDocumentsProviderRecoveryOperation( + operation: AndroidDocumentsProviderRecoveryOperation, +) { + require( + operation == AndroidDocumentsProviderRecoveryOperation.QueryChildren || + operation == AndroidDocumentsProviderRecoveryOperation.OpenRead || + operation == AndroidDocumentsProviderRecoveryOperation.Rename || + operation == AndroidDocumentsProviderRecoveryOperation.Delete, + ) { "The document operation is not permitted for recovery." } +} + +internal fun resolveAndroidDocumentsProviderSessionForCaller( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + loadActiveSession: () -> NextcloudSession?, +): AndroidDocumentsProviderResolvedSession? = resolveAndroidDocumentsProviderSession( + documentId = documentId, + operation = operation, + allowRecoveryPermit = Binder.getCallingUid() == Process.myUid(), + loadActiveSession = loadActiveSession, +) + +internal fun requireAndroidDocumentsProviderSession( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + loadActiveSession: () -> NextcloudSession?, +): AndroidDocumentsProviderResolvedSession = resolveAndroidDocumentsProviderSessionForCaller( + documentId, + operation, + loadActiveSession, +) ?: throw FileNotFoundException("This Nextcloud document is not available for the active account.") + +internal fun requireAndroidDocumentsProviderCallSession( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + loadActiveSession: () -> NextcloudSession?, +): AndroidDocumentsProviderResolvedSession = + if (AndroidExternalFileHandoffRegistry.isHandoffDocumentId(documentId)) { + val session = loadActiveSession() ?: throw FileNotFoundException("Sign in to nati.ve to browse files.") + AndroidDocumentsProviderResolvedSession(session, recoveryAuthorized = false) + } else { + requireAndroidDocumentsProviderSession(documentId, operation, loadActiveSession) + } + +internal fun withAndroidDocumentsProviderMutation( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + loadActiveSession: () -> NextcloudSession?, + action: (NextcloudSession) -> Result, +): Result { + val resolved = requireAndroidDocumentsProviderSession(documentId, operation, loadActiveSession) + return withAndroidDocumentMutation( + resolved.session, + { if (resolved.recoveryAuthorized) resolved.session else loadActiveSession() }, + action, + ) +} + +internal fun requireAndroidDocumentsProviderQuerySession( + documentId: String, + loadActiveSession: () -> NextcloudSession?, +): NextcloudSession = requireAndroidDocumentsProviderCallSession( + documentId, + AndroidDocumentsProviderRecoveryOperation.QueryDocument, + loadActiveSession, +).session + +internal fun requireAndroidDocumentsProviderChildrenSession( + documentId: String, + loadActiveSession: () -> NextcloudSession?, +): NextcloudSession = requireAndroidDocumentsProviderSession( + documentId, + AndroidDocumentsProviderRecoveryOperation.QueryChildren, + loadActiveSession, +).session + +internal fun requireAndroidDocumentsProviderOpenSession( + documentId: String, + mode: String, + loadActiveSession: () -> NextcloudSession?, +): NextcloudSession = requireAndroidDocumentsProviderCallSession( + documentId, + if (mode == "r") AndroidDocumentsProviderRecoveryOperation.OpenRead else + AndroidDocumentsProviderRecoveryOperation.OpenWrite, + loadActiveSession, +).session + +internal fun withAndroidDocumentsProviderCreate( + documentId: String, + loadActiveSession: () -> NextcloudSession?, + action: (NextcloudSession) -> Result, +): Result = withAndroidDocumentsProviderMutation( + documentId, AndroidDocumentsProviderRecoveryOperation.Create, loadActiveSession, action, +) + +internal fun withAndroidDocumentsProviderRename( + documentId: String, + loadActiveSession: () -> NextcloudSession?, + action: (NextcloudSession) -> Result, +): Result = withAndroidDocumentsProviderMutation( + documentId, AndroidDocumentsProviderRecoveryOperation.Rename, loadActiveSession, action, +) + +internal fun withAndroidDocumentsProviderDelete( + documentId: String, + loadActiveSession: () -> NextcloudSession?, + action: (NextcloudSession) -> Result, +): Result = withAndroidDocumentsProviderMutation( + documentId, AndroidDocumentsProviderRecoveryOperation.Delete, loadActiveSession, action, +) + +internal fun withAndroidDocumentsProviderMove( + documentId: String, + loadActiveSession: () -> NextcloudSession?, + action: (NextcloudSession) -> Result, +): Result = withAndroidDocumentsProviderMutation( + documentId, AndroidDocumentsProviderRecoveryOperation.Move, loadActiveSession, action, +) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt index 80f13ed2a..76eb42299 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt @@ -6,6 +6,7 @@ import android.net.Uri import dev.obiente.nextcloudnative.app.FileSyncDirection import dev.obiente.nextcloudnative.app.FileSyncOperation import dev.obiente.nextcloudnative.app.FileSyncPair +import dev.obiente.nextcloudnative.app.NextcloudSession import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job @@ -161,6 +162,7 @@ internal suspend fun reconcileSafDownloadsBeforePairRemoval( context: Context, localRootId: String, localRecoveryPaths: Set, + providerRecoverySession: NextcloudSession? = null, ): Boolean { if (!localRootId.startsWith("content://")) return true val shouldContinue = androidFileSyncJobContinuation(currentCoroutineContext()[Job]) @@ -197,6 +199,7 @@ internal suspend fun reconcileSafDownloadsBeforePairRemoval( localRootId = localRootId, localRecoveryPaths = localRecoveryPaths, shouldContinue = shouldContinue, + providerRecoverySession = providerRecoverySession, ) } else { createAndroidFileSyncLocalTree(context, localRootId).reconcileOwnedDownloads(shouldContinue) @@ -279,6 +282,7 @@ internal suspend fun retireAndroidFileSyncAccountPairs(context: Context, account internal suspend fun reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRemoval( context: Context, accountId: String, + providerRecoverySession: NextcloudSession, ) { AndroidFileSyncEngine.ENGINE_LOCK.withLock { reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( @@ -289,6 +293,7 @@ internal suspend fun reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRem context = context, localRootId = pair.localRootId, localRecoveryPaths = androidSafOwnedDownloadRecoveryPaths(pair), + providerRecoverySession = providerRecoverySession, ) }, ) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt index 47d7a796a..16e76fe43 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncLocalTree.kt @@ -4,6 +4,7 @@ import android.content.ContentResolver import android.net.Uri import android.provider.DocumentsContract import dev.obiente.nextcloudnative.app.LocalSyncEntry +import dev.obiente.nextcloudnative.app.NextcloudSession import dev.obiente.nextcloudnative.app.SyncEntryKind import dev.obiente.nextcloudnative.app.hashExactJvmFileSyncSlice import dev.obiente.nextcloudnative.app.normalizeSyncSha256 @@ -27,10 +28,12 @@ internal class AndroidSafFileSyncLocalTree( private val resolver: ContentResolver, rootId: String, private val downloadOwnershipStore: AndroidSafDownloadOwnershipStore, + private val providerRecoverySession: NextcloudSession? = null, ) : AndroidFileSyncLocalTree { private val treeUri = Uri.parse(rootId) private val rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri) private val rootUri = DocumentsContract.buildDocumentUriUsingTree(treeUri, rootDocumentId) + private val providerRecovery = AndroidDocumentsProviderRecoveryAccess(providerRecoverySession) init { require(rootId.startsWith("content://")) { "The local sync root is not a document-tree grant." } @@ -39,6 +42,9 @@ internal class AndroidSafFileSyncLocalTree( it.uri == treeUri && it.isReadPermission && it.isWritePermission }, ) { "Access to the selected local folder has expired. Select it again." } + providerRecoverySession?.let { session -> + NextcloudDocumentIds.requireForSession(rootDocumentId, session) + } } override fun scan( @@ -485,10 +491,15 @@ internal class AndroidSafFileSyncLocalTree( document: AndroidLocalSyncDocument, shouldContinue: () -> Boolean, ): String { - return requireNotNull(resolver.openInputStream(document.uri)) { - "The local replacement item could not be opened for verification." - }.use { input -> - hashAndroidSafReplacementContent(input, document.entry.size, shouldContinue) + return providerRecovery.run( + document.uri, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { recoveryUri -> + requireNotNull(resolver.openInputStream(recoveryUri)) { + "The local replacement item could not be opened for verification." + }.use { input -> + hashAndroidSafReplacementContent(input, document.entry.size, shouldContinue) + } } } @@ -594,10 +605,13 @@ internal class AndroidSafFileSyncLocalTree( ?.let { child -> androidSafReplacementContentIdentity(replacementSnapshot(child, shouldContinue)) } private fun rawChildren(parentUri: Uri, parentPath: String): List { - val parentId = DocumentsContract.getDocumentId(parentUri) - val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, parentId) - val cursor = requireNotNull(resolver.query(childrenUri, PROJECTION, null, null, null)) { - "The local file provider could not list the selected folder." + val cursor = providerRecovery.run( + parentUri, + AndroidDocumentsProviderRecoveryOperation.QueryChildren, + ) { recoveryUri -> + requireNotNull(resolver.query(recoveryUri, PROJECTION, null, null, null)) { + "The local file provider could not list the selected folder." + } } return cursor.use { buildList { @@ -636,34 +650,18 @@ internal class AndroidSafFileSyncLocalTree( private fun publicationDirectory( parentUri: Uri, parentPath: String, - ): AndroidSafPublicationDirectory = object : AndroidSafPublicationDirectory { - override fun documents(): List> = + ): AndroidSafPublicationDirectory = AndroidSafFileSyncPublicationDirectory( + resolver = resolver, + parentUri = parentUri, + documents = { rawChildren(parentUri, parentPath).map { document -> AndroidSafPublicationDocument(document.uri, document.displayName) } - - override fun createFile(displayName: String): Uri = requireNotNull( - DocumentsContract.createDocument( - resolver, - parentUri, - "application/octet-stream", - displayName, - ), - ) { "A staged local file could not be created." } - - override fun createDirectory(displayName: String): Uri = requireNotNull( - createDirectoryDocument(parentUri, displayName), - ) { "A staged local folder could not be created." } - - override fun writeFile(document: Uri, write: (OutputStream) -> Unit) { - writeDocument(document, write) - } - - override fun rename(document: Uri, displayName: String): Uri? = - DocumentsContract.renameDocument(resolver, document, displayName) - - override fun delete(document: Uri): Boolean = DocumentsContract.deleteDocument(resolver, document) - } + }, + createDirectory = { displayName -> createDirectoryDocument(parentUri, displayName) }, + writeDocument = ::writeDocument, + providerRecovery = providerRecovery, + ) private fun createDirectoryDocument(parentUri: Uri, displayName: String): Uri? = DocumentsContract.createDocument( diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafFileSyncPublicationDirectory.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafFileSyncPublicationDirectory.kt new file mode 100644 index 000000000..439cd8b2c --- /dev/null +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafFileSyncPublicationDirectory.kt @@ -0,0 +1,46 @@ +package dev.obiente.nextcloudnative + +import android.content.ContentResolver +import android.net.Uri +import android.provider.DocumentsContract +import java.io.OutputStream + +internal class AndroidSafFileSyncPublicationDirectory( + private val resolver: ContentResolver, + private val parentUri: Uri, + private val documents: () -> List>, + private val createDirectory: (String) -> Uri?, + private val writeDocument: (Uri, (OutputStream) -> Unit) -> Unit, + private val providerRecovery: AndroidDocumentsProviderRecoveryAccess, +) : AndroidSafPublicationDirectory { + override fun documents(): List> = documents.invoke() + + override fun createFile(displayName: String): Uri = requireNotNull( + DocumentsContract.createDocument(resolver, parentUri, "application/octet-stream", displayName), + ) { "A staged local file could not be created." } + + override fun createDirectory(displayName: String): Uri = requireNotNull(createDirectory.invoke(displayName)) { + "A staged local folder could not be created." + } + + override fun writeFile(document: Uri, write: (OutputStream) -> Unit) = writeDocument(document, write) + + override fun rename(document: Uri, displayName: String): Uri? = + providerRecovery.run( + document, + AndroidDocumentsProviderRecoveryOperation.Rename, + ) { recoveryUri -> + providerRecovery.normalizeResult( + document, + DocumentsContract.renameDocument(resolver, recoveryUri, displayName), + ) + } + + override fun delete(document: Uri): Boolean = + providerRecovery.run( + document, + AndroidDocumentsProviderRecoveryOperation.Delete, + ) { recoveryUri -> + DocumentsContract.deleteDocument(resolver, recoveryUri) + } +} diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt index 779e16982..a8164b4b4 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -4,6 +4,7 @@ import android.content.Context import android.net.Uri import android.provider.DocumentsContract import dev.obiente.nextcloudnative.app.FileSyncPair +import dev.obiente.nextcloudnative.app.NextcloudSession import kotlinx.coroutines.CancellationException internal data class AndroidSafOwnedDownloadRecoveryDirectory( @@ -90,6 +91,7 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( localRootId: String, localRecoveryPaths: Set, shouldContinue: () -> Boolean, + providerRecoverySession: NextcloudSession?, ) { val appContext = context.applicationContext val treeUri = Uri.parse(localRootId) @@ -99,6 +101,7 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( resolver = appContext.contentResolver, rootId = localRootId, downloadOwnershipStore = ownership, + providerRecoverySession = providerRecoverySession, ) localTree.indexRecoveryLocationsIfNeeded(indexedOwnership, shouldContinue) val recordedDocumentIds = ownership.pendingTransactions().asSequence() diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt index 7cbb908a3..84ab3c52d 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt @@ -101,7 +101,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { override fun queryDocument(documentId: String, projection: Array?): Cursor { val columns = projection?.copyOf() ?: DEFAULT_DOCUMENT_PROJECTION val cursor = MatrixCursor(columns) - val session = requireSession() + val session = requireAndroidDocumentsProviderQuerySession(documentId, services::loadSession) if (AndroidExternalFileHandoffRegistry.isHandoffDocumentId(documentId)) { val handoff = AndroidExternalFileHandoffRegistry.peek(documentId, session) ?: throw FileNotFoundException("This external file handoff has expired.") @@ -125,7 +125,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { ): Cursor { val columns = projection?.copyOf() ?: DEFAULT_DOCUMENT_PROJECTION val cursor = MatrixCursor(columns) - val session = requireSession() + val session = requireAndroidDocumentsProviderChildrenSession(parentDocumentId, services::loadSession) val parent = requireReference(parentDocumentId, session) val children = runCatching { val account = resolveAccount(session) @@ -186,7 +186,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } signal?.throwIfCanceled() - val session = requireSession() + val session = requireAndroidDocumentsProviderOpenSession(documentId, mode, services::loadSession) if (AndroidExternalFileHandoffRegistry.isHandoffDocumentId(documentId)) { if (mode != "r") throw SecurityException("External file handoffs are read-only.") return openExternalHandoffDocument(session, documentId, signal) @@ -463,7 +463,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } override fun createDocument(parentDocumentId: String, mimeType: String, displayName: String): String = - withAndroidDocumentMutation(requireSession(), services::loadSession) { session -> + withAndroidDocumentsProviderCreate(parentDocumentId, services::loadSession) { session -> val parent = requireReference(parentDocumentId, session) val account = resolveAccount(session) requireAndroidDocumentDirectory(parent) { findDocument(session, account, it, accountLeaseHeld = true) } @@ -483,13 +483,13 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } override fun renameDocument(documentId: String, displayName: String): String = - withAndroidDocumentMutation(requireSession(), services::loadSession) { session -> + withAndroidDocumentsProviderRename(documentId, services::loadSession) { session -> val reference = requireReference(documentId, session) if (reference.isRoot) throw SecurityException("The Nextcloud root cannot be renamed.") val account = resolveAccount(session) val file = findDocument(session, account, reference.path, accountLeaseHeld = true) val destination = childPath(NextcloudDocumentIds.parentPath(reference.path), requireSafeDisplayName(displayName)) - if (destination == reference.path) return@withAndroidDocumentMutation documentId + if (destination == reference.path) return@withAndroidDocumentsProviderRename documentId val etag = requireMutationEtag(file) withNoBlockingAndroidDocumentWriteback(context, session, reference.path, destination) { mutationCall { webDav.move(session, account.userId, reference.path, destination, etag) } @@ -499,7 +499,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } override fun deleteDocument(documentId: String) = - withAndroidDocumentMutation(requireSession(), services::loadSession) { session -> + withAndroidDocumentsProviderDelete(documentId, services::loadSession) { session -> val reference = requireReference(documentId, session) if (reference.isRoot) throw SecurityException("The Nextcloud root cannot be deleted.") val account = resolveAccount(session) @@ -523,7 +523,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { sourceParentDocumentId: String, targetParentDocumentId: String, ): String = - withAndroidDocumentMutation(requireSession(), services::loadSession) { session -> + withAndroidDocumentsProviderMove(sourceDocumentId, services::loadSession) { session -> val source = requireReference(sourceDocumentId, session) val sourceParent = requireReference(sourceParentDocumentId, session) val targetParent = requireReference(targetParentDocumentId, session) @@ -535,7 +535,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { requireAndroidDocumentDirectory(targetParent) { findDocument(session, account, it, accountLeaseHeld = true) } val file = findDocument(session, account, source.path, accountLeaseHeld = true) val destination = childPath(targetParent.path, file.name) - if (destination == source.path) return@withAndroidDocumentMutation sourceDocumentId + if (destination == source.path) return@withAndroidDocumentsProviderMove sourceDocumentId withNoBlockingAndroidDocumentWriteback(context, session, source.path, destination) { mutationCall { webDav.move(session, account.userId, source.path, destination, requireMutationEtag(file)) diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderManifestTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderManifestTest.kt new file mode 100644 index 000000000..1afcd8199 --- /dev/null +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderManifestTest.kt @@ -0,0 +1,40 @@ +package dev.obiente.nextcloudnative + +import java.io.File +import javax.xml.parsers.DocumentBuilderFactory +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class AndroidDocumentsProviderManifestTest { + @Test + fun `documents provider stays in the application process for thread confined recovery`() { + val manifest = parseXml(androidMainSourceDirectory().resolve("AndroidManifest.xml")) + val providers = manifest.getElementsByTagName("provider") + val documentsProvider = (0 until providers.length) + .map(providers::item) + .firstOrNull { provider -> + provider.attributes.getNamedItemNS(ANDROID_XML_NAMESPACE, "name")?.nodeValue == + ".NextcloudDocumentsProvider" + } + + assertNotNull(documentsProvider) + assertNull(documentsProvider.attributes.getNamedItemNS(ANDROID_XML_NAMESPACE, "process")) + } + + private fun androidMainSourceDirectory(): File { + val workingDirectory = File(requireNotNull(System.getProperty("user.dir"))) + return listOf(workingDirectory.resolve("src/main"), workingDirectory.resolve("androidApp/src/main")) + .firstOrNull { candidate -> candidate.resolve("AndroidManifest.xml").isFile } + ?: error("Could not locate the Android main source directory.") + } + + private fun parseXml(file: File) = DocumentBuilderFactory.newInstance().apply { + isNamespaceAware = true + setFeature("http://apache.org/xml/features/disallow-doctype-decl", true) + }.newDocumentBuilder().parse(file) + + private companion object { + const val ANDROID_XML_NAMESPACE = "http://schemas.android.com/apk/res/android" + } +} diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt index b0d6c7465..bf5c2f390 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt @@ -1,10 +1,14 @@ package dev.obiente.nextcloudnative +import dev.obiente.nextcloudnative.app.NextcloudSession import java.io.IOException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicReference import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlin.concurrent.thread import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -55,6 +59,280 @@ class NextcloudDocumentsContractTest { ) } + @Test + fun `recovery permit resolves only its inactive target operation`() { + val active = session("active", "active-secret") + val removed = session("removed", "removed-secret") + val removedDocument = NextcloudDocumentIds.documentId(removed, "Sync/report.txt") + val activeDocument = NextcloudDocumentIds.documentId(active, "Documents/current.txt") + + withAndroidDocumentsProviderRecoveryPermit( + removed, + removedDocument, + AndroidDocumentsProviderRecoveryOperation.QueryChildren, + ) { + assertEquals( + removed, + resolveSession(removedDocument, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { active } + ?.session, + ) + assertEquals( + active, + resolveSession(activeDocument, AndroidDocumentsProviderRecoveryOperation.QueryDocument) { active } + ?.session, + ) + assertEquals( + null, + resolveSession(removedDocument, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { active }, + ) + } + + assertEquals( + null, + resolveSession(removedDocument, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { active }, + ) + } + + @Test + fun `recovery permit is confined to its synchronous dispatch thread`() { + val unavailable = session("unavailable", "") + val documentId = NextcloudDocumentIds.documentId(unavailable, "Sync") + val resolved = AtomicReference() + + withAndroidDocumentsProviderRecoveryPermit( + unavailable, + documentId, + AndroidDocumentsProviderRecoveryOperation.QueryChildren, + ) { + val dispatch = thread(start = true) { + resolved.set( + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { null } + ?.session, + ) + } + dispatch.join() + assertEquals( + unavailable, + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { null } + ?.session, + ) + } + + assertEquals(null, resolved.get()) + } + + @Test + fun `external provider caller cannot consume an inactive recovery permit`() { + val active = session("active", "active-secret") + val removed = session("removed", "removed-secret") + val removedDocument = NextcloudDocumentIds.documentId(removed, "Sync") + + withAndroidDocumentsProviderRecoveryPermit( + removed, + removedDocument, + AndroidDocumentsProviderRecoveryOperation.Delete, + ) { + assertEquals( + null, + resolveSession( + removedDocument, + AndroidDocumentsProviderRecoveryOperation.Delete, + allowRecoveryPermit = false, + ) { active }, + ) + assertEquals( + removed, + resolveSession(removedDocument, AndroidDocumentsProviderRecoveryOperation.Delete) { active }?.session, + ) + } + } + + @Test + fun `paused recovery rejects unrelated mutation and writable open`() { + val active = session("active", "active-secret") + val removed = session("removed", "removed-secret") + val recoveryDocument = NextcloudDocumentIds.documentId(removed, "Sync/.nextcloud-native-stage") + val unrelatedDocument = NextcloudDocumentIds.documentId(removed, "Sync/private.txt") + val permitInstalled = CountDownLatch(1) + val finishRecovery = CountDownLatch(1) + + val recovery = thread(start = true) { + withAndroidDocumentsProviderRecoveryPermit( + removed, + recoveryDocument, + AndroidDocumentsProviderRecoveryOperation.Rename, + ) { + permitInstalled.countDown() + finishRecovery.await() + } + } + permitInstalled.await() + try { + assertEquals( + null, + resolveSession(unrelatedDocument, AndroidDocumentsProviderRecoveryOperation.Rename) { active }, + ) + assertEquals( + null, + resolveSession(recoveryDocument, AndroidDocumentsProviderRecoveryOperation.OpenWrite) { active }, + ) + assertEquals( + null, + resolveSession(recoveryDocument, AndroidDocumentsProviderRecoveryOperation.Rename) { active }?.session, + ) + } finally { + finishRecovery.countDown() + recovery.join() + } + } + + @Test + fun `read descriptor session survives its consumed permit without enabling write open`() { + val removed = session("removed", "removed-secret") + val documentId = NextcloudDocumentIds.documentId(removed, "Sync/recovery-backup") + lateinit var descriptorSession: AndroidDocumentsProviderResolvedSession + + withAndroidDocumentsProviderRecoveryPermit( + removed, + documentId, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { + descriptorSession = checkNotNull( + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.OpenRead) { null }, + ) + assertEquals( + null, + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.OpenWrite) { null }, + ) + } + + assertEquals(removed, descriptorSession.session) + assertTrue(descriptorSession.recoveryAuthorized) + assertEquals( + null, + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.OpenRead) { null }, + ) + } + + @Test + fun `recovery permit does not change external handoff session resolution`() { + val active = session("active", "active-secret") + val handoffDocumentId = "nch1:0123456789abcdef0123456789abcdef" + + assertEquals( + active, + requireAndroidDocumentsProviderCallSession( + handoffDocumentId, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { active }.session, + ) + } + + @Test + fun `recovery permit wins over a new credential incarnation of the same account`() { + val removed = session("same-account", "removed-secret") + val replacement = session("same-account", "replacement-secret") + val documentId = NextcloudDocumentIds.documentId(removed, "Sync/recovery-backup") + + withAndroidDocumentsProviderRecoveryPermit( + removed, + documentId, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { + val recovery = resolveSession( + documentId, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { replacement } + assertEquals(removed, recovery?.session) + assertTrue(recovery?.recoveryAuthorized == true) + + val ordinary = resolveSession( + documentId, + AndroidDocumentsProviderRecoveryOperation.OpenRead, + ) { replacement } + assertEquals(replacement, ordinary?.session) + assertFalse(ordinary?.recoveryAuthorized == true) + } + } + + @Test + fun `recovery uri helpers use direct provider calls and deny unsupported operations`() { + fun recoveryUri(operation: AndroidDocumentsProviderRecoveryOperation) = + androidDocumentsProviderRecoveryUri( + documentId = "document-id", + operation = operation, + buildDocumentUri = { id -> "document:$id" }, + buildChildDocumentsUri = { id -> "children:$id" }, + ) + + assertEquals("children:document-id", recoveryUri(AndroidDocumentsProviderRecoveryOperation.QueryChildren)) + assertEquals("document:document-id", recoveryUri(AndroidDocumentsProviderRecoveryOperation.OpenRead)) + assertEquals("document:document-id", recoveryUri(AndroidDocumentsProviderRecoveryOperation.Rename)) + assertEquals("document:document-id", recoveryUri(AndroidDocumentsProviderRecoveryOperation.Delete)) + listOf( + AndroidDocumentsProviderRecoveryOperation.QueryDocument, + AndroidDocumentsProviderRecoveryOperation.OpenWrite, + AndroidDocumentsProviderRecoveryOperation.Create, + AndroidDocumentsProviderRecoveryOperation.Move, + ).forEach { operation -> + assertFailsWith { recoveryUri(operation) } + } + + val session = session("removed", "removed-secret") + val documentId = NextcloudDocumentIds.documentId(session, "Sync") + assertFailsWith { + withAndroidDocumentsProviderRecoveryPermit( + session, + documentId, + AndroidDocumentsProviderRecoveryOperation.OpenWrite, + ) { + error("write recovery must remain unreachable") + } + } + } + + @Test + fun `recovery rename normalizes a changed document id back to the durable tree`() { + val normalized = normalizeAndroidDocumentsProviderRecoveryResult( + recoveryEnabled = true, + document = "tree:old-id", + result = "direct:new-id", + documentIdOf = { result -> result.substringAfter(':') }, + buildTreeDocumentUri = { _, id -> "tree:$id" }, + ) + val ordinary = normalizeAndroidDocumentsProviderRecoveryResult( + recoveryEnabled = false, + document = "tree:old-id", + result = "tree:new-id", + documentIdOf = { error("ordinary results stay unchanged") }, + buildTreeDocumentUri = { _, _ -> error("ordinary results stay unchanged") }, + ) + + assertEquals("tree:new-id", normalized) + assertEquals("tree:new-id", ordinary) + } + + @Test + fun `recovery permit is cleared when recovery fails`() { + val removed = session("removed", "removed-secret") + val documentId = NextcloudDocumentIds.documentId(removed, "Sync") + + assertFailsWith { + withAndroidDocumentsProviderRecoveryPermit( + removed, + documentId, + AndroidDocumentsProviderRecoveryOperation.QueryChildren, + ) { + throw IOException("synthetic recovery failure") + } + } + + assertEquals( + null, + resolveSession(documentId, AndroidDocumentsProviderRecoveryOperation.QueryChildren) { null }, + ) + } + @Test fun `account removal preflight runs before remote credential revocation`() = runBlocking { var revoked = false @@ -125,4 +403,22 @@ class NextcloudDocumentsContractTest { assertTrue(operation.isCancelled) assertTrue(removalCompleted.isCompleted) } + + private fun session(loginName: String, appPassword: String) = NextcloudSession( + serverUrl = "https://cloud.example.test", + loginName = loginName, + appPassword = appPassword, + ) + + private fun resolveSession( + documentId: String, + operation: AndroidDocumentsProviderRecoveryOperation, + allowRecoveryPermit: Boolean = true, + loadActiveSession: () -> NextcloudSession?, + ): AndroidDocumentsProviderResolvedSession? = resolveAndroidDocumentsProviderSession( + documentId = documentId, + operation = operation, + allowRecoveryPermit = allowRecoveryPermit, + loadActiveSession = loadActiveSession, + ) } From 32925b60f843d413ba524b0d716b63f8a7d854e1 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 04:29:25 +0200 Subject: [PATCH 07/17] refactor(android): split picker persistence helpers --- .../AndroidAccountCredentialController.kt | 3 -- .../AndroidLocalUploadPicker.kt | 18 -------- .../AndroidLocalUploadPickerPersistence.kt | 46 +++++++++++++++++++ 3 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt index ae64d36ff..2e5ff3de1 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt @@ -727,7 +727,6 @@ internal class AndroidAccountCredentialController( throw failure } } - private fun encryptState(state: AndroidAccountCredentialState): String = try { sessionCipher.encrypt(encodeAndroidAccountCredentialState(state)) } catch (failure: Exception) { @@ -737,7 +736,6 @@ internal class AndroidAccountCredentialController( ) throw failure } - private fun encryptCredentialSlot(session: NextcloudSession): String = try { sessionCipher.encrypt(encodeAndroidPersistedSession(session)) } catch (failure: Exception) { @@ -799,5 +797,4 @@ internal class AndroidAccountCredentialController( component = SupportDiagnosticComponent.Cache, failure = failure, ) - } diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt index 2cc78ad52..06d15e789 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPicker.kt @@ -761,24 +761,6 @@ internal class AndroidLocalUploadPicker(context: Context) { } } -private fun requireSafeProcessGeneration(value: String) { - require(value.length in 16..96 && value.all { it.isLetterOrDigit() || it == '-' }) { - "The picker capability process generation is invalid." - } -} - -internal fun resumeLocalUploadSelectionResult( - continuation: CancellableContinuation, - result: LocalUploadSelectionResult, - releaseSelected: (LocalUploadFile) -> Unit, -) { - continuation.resume(result) { _, undeliveredResult, _ -> - if (undeliveredResult is LocalUploadSelectionResult.Selected) { - runCatching { releaseSelected(undeliveredResult.file) } - } - } -} - private data class AndroidUploadMetadata( val displayName: String, val sizeBytes: Long?, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt new file mode 100644 index 000000000..5a3f2149f --- /dev/null +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt @@ -0,0 +1,46 @@ +package dev.obiente.nextcloudnative + +import dev.obiente.nextcloudnative.app.LocalUploadFile +import dev.obiente.nextcloudnative.app.LocalUploadSelectionResult +import kotlinx.coroutines.CancellableContinuation +import org.json.JSONObject +import kotlin.coroutines.resume + +internal fun requireSafeProcessGeneration(value: String) { + require(value.length in 16..96 && value.all { it.isLetterOrDigit() || it == '-' }) { + "The picker capability process generation is invalid." + } +} + +internal fun JSONObject.optionalStrictString(key: String): String? { + if (!has(key) || isNull(key)) return null + return requireStrictString(key) +} + +internal fun JSONObject.requireStrictString(key: String): String = get(key).let { value -> + require(value is String) { "The $key value changed type." } + value +} + +internal fun JSONObject.optionalStrictBoolean(key: String): Boolean? { + if (!has(key) || isNull(key)) return null + return get(key).let { value -> + require(value is Boolean) { "The $key value changed type." } + value + } +} + +internal fun persistedDurableUploadGrantPreExisting(payload: JSONObject): Boolean = + payload.optionalStrictBoolean("grantPreExisting") ?: false + +internal fun resumeLocalUploadSelectionResult( + continuation: CancellableContinuation, + result: LocalUploadSelectionResult, + releaseSelected: (LocalUploadFile) -> Unit, +) { + continuation.resume(result) { _, undeliveredResult, _ -> + if (undeliveredResult is LocalUploadSelectionResult.Selected) { + runCatching { releaseSelected(undeliveredResult.file) } + } + } +} From 6d34a92eb44801cda78a2051c3cb16152bf6d0cc Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 06:14:43 +0200 Subject: [PATCH 08/17] fix(android): retain unclassified download recovery --- .../AndroidSafDownloadOwnershipStore.kt | 7 +++++- ...oidFileSyncProviderFeedbackRecoveryTest.kt | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt index b1ef4ba24..d91f42d32 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt @@ -45,7 +45,12 @@ internal class AndroidSafDownloadOwnershipStore( fun indexed(): AndroidSafDownloadOwnershipDirectory = synchronized(LOCK) { val files = ownershipFiles() - IndexedOwnershipDirectory(files.mapNotNull(::ownershipReference), files.size) + val references = files.map { file -> + checkNotNull(ownershipReference(file)) { + "SAF download recovery row name is invalid." + } + } + IndexedOwnershipDirectory(references, files.size) } private inner class IndexedOwnershipDirectory( diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt index 05af96f17..b49e5e349 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -4,8 +4,10 @@ import dev.obiente.nextcloudnative.app.FileSyncBaseline import dev.obiente.nextcloudnative.app.FileSyncConfiguration import dev.obiente.nextcloudnative.app.FileSyncPair import dev.obiente.nextcloudnative.app.SyncEntryKind +import java.nio.file.Files import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertFailsWith import kotlin.test.assertTrue import kotlinx.coroutines.CancellationException @@ -118,6 +120,27 @@ class AndroidFileSyncProviderFeedbackRecoveryTest { ) } + @Test + fun `account removal blocks unclassified ownership evidence`() { + val root = Files.createTempDirectory("saf-provider-removal-invalid-row-").toFile() + try { + val invalid = root.resolve("unclassified.row").apply { writeBytes(byteArrayOf(0x01)) } + val store = AndroidSafDownloadOwnershipStore(root) + + val removalReady = reconcileSafDownloadsBeforePairRemoval( + hasPersistedGrant = true, + hasPendingRecovery = store.hasPendingTransactions(), + reconcile = { store.indexed() }, + ) + + assertFalse(removalReady) + assertTrue(invalid.isFile) + assertTrue(store.hasPendingTransactions()) + } finally { + root.deleteRecursively() + } + } + @Test fun `cancelled own provider retirement stops before the next directory`() { val events = mutableListOf() From 8a45c14821b53a64739b5589637c9eb3c7062c01 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 07:30:21 +0200 Subject: [PATCH 09/17] fix(android): revalidate account removal under lease --- .../AndroidAccountCredentialController.kt | 24 ++++------- .../nextcloudnative/AndroidAccountRemoval.kt | 42 +++++++++++++++++-- .../AndroidNextcloudServices.kt | 1 - .../AndroidAccountOperationGuardTest.kt | 41 +++++++++++++++++- 4 files changed, 85 insertions(+), 23 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt index 2e5ff3de1..1a0bf4345 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt @@ -26,13 +26,13 @@ internal class AndroidAccountCredentialController( private val clearPreviewAccount: (String) -> Unit, private val notifyDocumentRootsChanged: () -> Unit, private val resumeQueuedUploads: suspend (String) -> Unit, - private val prepareAccountRemoval: suspend (NextcloudSession) -> Unit, private val removeQueuedUploads: suspend (NextcloudSession) -> Unit, private val retryQueuedUploadsCleanup: suspend (NextcloudSession, String, String?, String?, String?) -> Unit, private val retryQueuedUploadsCleanupWithoutCredentials: suspend (String, String, String?, String?, String?) -> Unit, private val activatePersistedAccount: suspend (NextcloudSession) -> Unit, ) { private val appContext = context.applicationContext + private val accountRemovalLeases = AndroidAccountRemovalLeaseCoordinator(appContext) private val handoffCleanup = AndroidExternalFileHandoffCleanup(appContext, preferences, ::commitPreferences) private val accountRemovalCleanupJournal = AndroidAccountRemovalCleanupJournal( preferences = preferences, @@ -158,9 +158,7 @@ internal class AndroidAccountCredentialController( val session = current.sessions[accountId] ?: return@withLock removeUnavailableAccount(accountId, current) val pendingCleanup = pendingAndroidAccountRemovalCleanup(session) - withPreparedAndroidAccountRemovalLease( - NextcloudDocumentIds.accountKey(session), prepare = { prepareAccountRemoval(session) }, - ) { + accountRemovalLeases.withLease(session) { val active = current.registry.activeAccountId == accountId removeAndroidAccountCredentialData( active = active, @@ -194,7 +192,7 @@ internal class AndroidAccountCredentialController( val unavailableSession = NextcloudSession(target.record.serverUrl, target.record.loginName, appPassword = "") val accountIdentity = NextcloudDocumentIds.accountKey(unavailableSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(unavailableSession) - withPreparedAndroidAccountRemovalLease(accountIdentity, prepare = { prepareAccountRemoval(unavailableSession) }) { + accountRemovalLeases.withLease(unavailableSession) { removeUnavailableAndroidAccountCredentialData( accountIdentity = accountIdentity, active = target.wasActive, @@ -232,11 +230,9 @@ internal class AndroidAccountCredentialController( check(current.activeSession == expectedSession) { "The account changed before its remote session could be revoked." } - val accountIdentity = NextcloudDocumentIds.accountKey(expectedSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(expectedSession) - revokeAndroidSessionWithAccountLease( - accountIdentity = accountIdentity, - preflight = { prepareAccountRemoval(expectedSession) }, + accountRemovalLeases.revoke( + session = expectedSession, revoke = revokeRemoteSession, removeLocalAccount = { removeAndroidAccountCredentialData( @@ -266,11 +262,8 @@ internal class AndroidAccountCredentialController( if (session == null) { clearSession(read.state) } else { - val accountIdentity = NextcloudDocumentIds.accountKey(session) val pendingCleanup = pendingAndroidAccountRemovalCleanup(session) - withPreparedAndroidAccountRemovalLease( - accountIdentity, prepare = { prepareAccountRemoval(session) }, - ) { + accountRemovalLeases.withLease(session) { removeAndroidAccountCredentialData( active = true, removeQueuedUploads = { removeQueuedUploads(session) }, @@ -342,11 +335,8 @@ internal class AndroidAccountCredentialController( ) { val activeSession = current.activeSession if (activeSession != null) { - val accountIdentity = NextcloudDocumentIds.accountKey(activeSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(activeSession) - withPreparedAndroidAccountRemovalLease( - accountIdentity, prepare = { prepareAccountRemoval(activeSession) }, - ) { + accountRemovalLeases.withLease(activeSession) { removeRecoveredAndroidAccountCredentialData( removeQueuedUploads = { removeQueuedUploads(activeSession) }, clearRecoveredAccount = { diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt index 111224dee..d57fc9ff0 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt @@ -34,10 +34,14 @@ internal suspend fun withPreparedAndroidAccountRemovalLease( accountIdentity: String, guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, prepare: suspend () -> Unit, + revalidate: suspend () -> Unit, action: suspend () -> Result, ): Result { prepare() - return withAndroidAccountRemovalLease(accountIdentity, guard, action) + return withAndroidAccountRemovalLease(accountIdentity, guard) { + revalidate() + action() + } } internal suspend fun revokeAndroidSessionAfterRemovalPreflight( @@ -74,13 +78,45 @@ internal suspend fun revokeAndroidSessionAfterRemovalPreflight( internal suspend fun revokeAndroidSessionWithAccountLease( accountIdentity: String, guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, - preflight: suspend () -> Unit, + prepare: suspend () -> Unit, + revalidate: suspend () -> Unit, revoke: suspend () -> Unit, removeLocalAccount: suspend () -> Unit, -) = withPreparedAndroidAccountRemovalLease(accountIdentity, guard, preflight) { +) = withPreparedAndroidAccountRemovalLease(accountIdentity, guard, prepare, revalidate) { revokeAndroidSessionAfterRemovalPreflight({}, revoke, removeLocalAccount) } +internal class AndroidAccountRemovalLeaseCoordinator( + context: Context, + private val guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, +) { + private val appContext = context.applicationContext + + suspend fun withLease( + session: NextcloudSession, + action: suspend () -> Result, + ): Result = withPreparedAndroidAccountRemovalLease( + accountIdentity = NextcloudDocumentIds.accountKey(session), + guard = guard, + prepare = { prepareAndroidAccountRemoval(appContext, session) }, + revalidate = { preflightAndroidAccountRemoval(appContext, session) }, + action = action, + ) + + suspend fun revoke( + session: NextcloudSession, + revoke: suspend () -> Unit, + removeLocalAccount: suspend () -> Unit, + ) = revokeAndroidSessionWithAccountLease( + accountIdentity = NextcloudDocumentIds.accountKey(session), + guard = guard, + prepare = { prepareAndroidAccountRemoval(appContext, session) }, + revalidate = { preflightAndroidAccountRemoval(appContext, session) }, + revoke = revoke, + removeLocalAccount = removeLocalAccount, + ) +} + internal enum class AndroidAccountDocumentGrantScope(val pathSegment: String) { Document("document"), Tree("tree"), diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt index 261e9b25a..e41de96c1 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt @@ -477,7 +477,6 @@ internal class AndroidNextcloudServices( clearPreviewAccount = nativeMediaPreviewCache::clearAccount, notifyDocumentRootsChanged = ::notifyDocumentsRootsChanged, resumeQueuedUploads = durableMultipartUploads::resumeQueuedForAccount, - prepareAccountRemoval = { session -> prepareAndroidAccountRemoval(appContext, session) }, removeQueuedUploads = accountOwnedStateCleanup::remove, retryQueuedUploadsCleanup = accountOwnedStateCleanup::retry, retryQueuedUploadsCleanupWithoutCredentials = accountOwnedStateCleanup::retryWithoutCredentials, diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt index 8a4b8cc33..91ce91606 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt @@ -236,7 +236,8 @@ class AndroidAccountOperationGuardTest { revokeAndroidSessionWithAccountLease( accountIdentity = "account-a", guard = guard, - preflight = {}, + prepare = {}, + revalidate = {}, revoke = { remoteRevoked.complete(Unit) }, removeLocalAccount = { allowLocalRemoval.await() @@ -549,12 +550,13 @@ class AndroidAccountOperationGuardTest { prepare = { guard.withAccount(accountIdentity) { events += "provider-read" } }, + revalidate = { events += "revalidate" }, ) { events += "remove" } } - assertEquals(listOf("provider-read", "remove"), events) + assertEquals(listOf("provider-read", "revalidate", "remove"), events) } @Test @@ -573,6 +575,7 @@ class AndroidAccountOperationGuardTest { prepare = { competingLease = guard.acquireBlocking(accountIdentity) }, + revalidate = {}, ) { removalEntered = true } @@ -589,6 +592,40 @@ class AndroidAccountOperationGuardTest { assertFalse(removalEntered) } + @Test + fun removalStateIsRevalidatedAfterTheAccountLeaseIsAcquired() = runBlocking { + val guard = AndroidAccountOperationGuard() + val accountIdentity = "account-a" + var removalReady = true + var removalEntered = false + var revalidationHeldLease = false + + val failure = assertFailsWith { + withPreparedAndroidAccountRemovalLease( + accountIdentity = accountIdentity, + guard = guard, + prepare = { removalReady = false }, + revalidate = { + revalidationHeldLease = guard.tryWithAccount( + accountIdentity, + unavailable = { true }, + action = { false }, + ) + check(removalReady) { "Account state changed after preparation." } + }, + ) { + removalEntered = true + } + } + + assertEquals("Account state changed after preparation.", failure.message) + assertTrue(revalidationHeldLease) + assertFalse(removalEntered) + withTimeout(1_000L) { + guard.withAccount(accountIdentity) { } + } + } + @Test fun directDocumentMutationLeaseRejectsReauthenticatedSessionAndReleasesTheGuard() = runBlocking { val guard = AndroidAccountOperationGuard() From 472976ea26535716f3fd6710f8486e9b782086d1 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 08:28:56 +0200 Subject: [PATCH 10/17] fix(android): harden legacy provider retirement --- .../AndroidAccountCredentialController.kt | 2 +- .../nextcloudnative/AndroidAccountRemoval.kt | 25 +++++++++ .../AndroidSafOwnedDownloadRetirement.kt | 54 ++++++++++++++----- .../AndroidAccountOperationGuardTest.kt | 16 ++++++ ...oidFileSyncProviderFeedbackRecoveryTest.kt | 43 +++++++++++++++ 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt index 1a0bf4345..f674cd9da 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt @@ -192,7 +192,7 @@ internal class AndroidAccountCredentialController( val unavailableSession = NextcloudSession(target.record.serverUrl, target.record.loginName, appPassword = "") val accountIdentity = NextcloudDocumentIds.accountKey(unavailableSession) val pendingCleanup = pendingAndroidAccountRemovalCleanup(unavailableSession) - accountRemovalLeases.withLease(unavailableSession) { + accountRemovalLeases.withUnavailableLease(unavailableSession) { removeUnavailableAndroidAccountCredentialData( accountIdentity = accountIdentity, active = target.wasActive, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt index d57fc9ff0..0fcddfd6d 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountRemoval.kt @@ -44,6 +44,19 @@ internal suspend fun withPreparedAndroidAccountRemovalLease( } } +internal suspend fun withUnavailableAndroidAccountRemovalLease( + accountIdentity: String, + guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, + preflight: suspend () -> Unit, + action: suspend () -> Result, +): Result = withPreparedAndroidAccountRemovalLease( + accountIdentity = accountIdentity, + guard = guard, + prepare = preflight, + revalidate = preflight, + action = action, +) + internal suspend fun revokeAndroidSessionAfterRemovalPreflight( preflight: suspend () -> Unit, revoke: suspend () -> Unit, @@ -103,6 +116,18 @@ internal class AndroidAccountRemovalLeaseCoordinator( action = action, ) + // Missing credentials cannot safely repair legacy self-provider downloads before removal. + // Commit first; durable owned-state cleanup remains fail-closed and can resume after re-add. + suspend fun withUnavailableLease( + session: NextcloudSession, + action: suspend () -> Result, + ): Result = withUnavailableAndroidAccountRemovalLease( + accountIdentity = NextcloudDocumentIds.accountKey(session), + guard = guard, + preflight = { preflightAndroidAccountRemoval(appContext, session) }, + action = action, + ) + suspend fun revoke( session: NextcloudSession, revoke: suspend () -> Unit, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt index a8164b4b4..2cb5edbe8 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -82,6 +82,32 @@ internal fun reconcileRecordedAndroidSafDownloadDirectories( return !hasPendingRecovery() } +internal fun reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( + recordedCandidates: List, + discoverCandidates: () -> List, + hasPendingRecovery: () -> Boolean, + hasPendingForDirectory: (Directory) -> Boolean, + shouldContinue: () -> Boolean = { true }, + reconcileDirectory: (Directory) -> Unit, +): Boolean { + if ( + reconcileRecordedAndroidSafDownloadDirectories( + candidates = recordedCandidates, + hasPendingRecovery = hasPendingRecovery, + hasPendingForDirectory = hasPendingForDirectory, + shouldContinue = shouldContinue, + reconcileDirectory = reconcileDirectory, + ) + ) return true + return reconcileRecordedAndroidSafDownloadDirectories( + candidates = recordedCandidates + discoverCandidates(), + hasPendingRecovery = hasPendingRecovery, + hasPendingForDirectory = hasPendingForDirectory, + shouldContinue = shouldContinue, + reconcileDirectory = reconcileDirectory, + ) +} + internal fun requireAndroidSafRetirementContinuation(shouldContinue: () -> Boolean) { if (!shouldContinue()) throw CancellationException("Folder sync recovery was cancelled.") } @@ -103,7 +129,6 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( downloadOwnershipStore = ownership, providerRecoverySession = providerRecoverySession, ) - localTree.indexRecoveryLocationsIfNeeded(indexedOwnership, shouldContinue) val recordedDocumentIds = ownership.pendingTransactions().asSequence() .flatMap { transaction -> sequenceOf(transaction.stageDocumentIdentity, transaction.backupDocumentIdentity) @@ -125,20 +150,23 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( ).map { candidate -> candidate to DocumentsContract.buildDocumentUriUsingTree(treeUri, candidate.documentId) } - val relocatedCandidates = indexedOwnership.observedPendingDirectoryIdentities().mapNotNull { identity -> - val directoryUri = runCatching { Uri.parse(identity) }.getOrNull() ?: return@mapNotNull null - val documentId = runCatching { DocumentsContract.getDocumentId(directoryUri) }.getOrNull() - ?: return@mapNotNull null - androidSafOwnedDownloadRecoveryDirectory( - rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri), - directoryDocumentId = documentId, - )?.let { candidate -> candidate to directoryUri } - } - val candidates = recordedCandidates + relocatedCandidates val hasRelevantPendingRecovery = indexedOwnership::hasPendingTransactions check( - reconcileRecordedAndroidSafDownloadDirectories( - candidates = candidates, + reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( + recordedCandidates = recordedCandidates, + discoverCandidates = { + localTree.indexRecoveryLocationsIfNeeded(indexedOwnership, shouldContinue) + indexedOwnership.observedPendingDirectoryIdentities().mapNotNull { identity -> + val directoryUri = runCatching { Uri.parse(identity) }.getOrNull() + ?: return@mapNotNull null + val documentId = runCatching { DocumentsContract.getDocumentId(directoryUri) }.getOrNull() + ?: return@mapNotNull null + androidSafOwnedDownloadRecoveryDirectory( + rootDocumentId = DocumentsContract.getTreeDocumentId(treeUri), + directoryDocumentId = documentId, + )?.let { candidate -> candidate to directoryUri } + } + }, hasPendingRecovery = hasRelevantPendingRecovery, hasPendingForDirectory = { (_, directoryUri) -> indexedOwnership.hasPendingTransactionsForDirectory(directoryUri.toString()) diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt index 91ce91606..29ab409ee 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt @@ -559,6 +559,22 @@ class AndroidAccountOperationGuardTest { assertEquals(listOf("provider-read", "revalidate", "remove"), events) } + @Test + fun unavailableRemovalUsesOnlyCredentialFreePreflight() = runBlocking { + val guard = AndroidAccountOperationGuard() + val events = mutableListOf() + + withUnavailableAndroidAccountRemovalLease( + accountIdentity = "account-a", + guard = guard, + preflight = { events += "preflight" }, + ) { + events += "remove" + } + + assertEquals(listOf("preflight", "preflight", "remove"), events) + } + @Test fun accountWorkStartedAfterPreparationMakesRemovalFailClosed() = runBlocking { val guard = AndroidAccountOperationGuard() diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt index b49e5e349..0f7a6e2c8 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -165,6 +165,49 @@ class AndroidFileSyncProviderFeedbackRecoveryTest { assertEquals(listOf("check:first", "reconcile:first"), events) } + @Test + fun `recorded recovery completes before full tree discovery`() { + var pending = true + var discoveryCalls = 0 + + val reconciled = reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( + recordedCandidates = listOf("recorded"), + discoverCandidates = { + discoveryCalls += 1 + error("The unrelated tree is not inspectable") + }, + hasPendingRecovery = { pending }, + hasPendingForDirectory = { it == "recorded" && pending }, + reconcileDirectory = { pending = false }, + ) + + assertTrue(reconciled) + assertEquals(0, discoveryCalls) + } + + @Test + fun `unresolved recorded recovery discovers a relocated directory`() { + var pending = true + val events = mutableListOf() + + val reconciled = reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( + recordedCandidates = listOf("recorded"), + discoverCandidates = { + events += "discover" + listOf("relocated") + }, + hasPendingRecovery = { pending }, + hasPendingForDirectory = { it == "relocated" && pending }, + reconcileDirectory = { + events += "reconcile:$it" + pending = false + }, + ) + + assertTrue(reconciled) + assertEquals(listOf("discover", "reconcile:relocated"), events) + } + @Test fun `relocated recovery directory keeps its exact relative path`() { val root = NextcloudDocumentIds.documentId(accountKey, "Sync") From ef914563db932857f33bc89d6dfdde32eaf8a8c0 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 08:47:51 +0200 Subject: [PATCH 11/17] fix(android): recover moved provider directories --- .../AndroidSafOwnedDownloadRetirement.kt | 12 +++++++-- ...oidFileSyncProviderFeedbackRecoveryTest.kt | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt index 2cb5edbe8..734baa2cb 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -96,11 +96,19 @@ internal fun reconcileRecordedThenDiscoveredAndroidSafDownloadDirect hasPendingRecovery = hasPendingRecovery, hasPendingForDirectory = hasPendingForDirectory, shouldContinue = shouldContinue, - reconcileDirectory = reconcileDirectory, + reconcileDirectory = { candidate -> + try { + reconcileDirectory(candidate) + } catch (failure: CancellationException) { + throw failure + } catch (_: Exception) { + // A recorded document ID may be stale after the recovery directory was moved. + } + }, ) ) return true return reconcileRecordedAndroidSafDownloadDirectories( - candidates = recordedCandidates + discoverCandidates(), + candidates = discoverCandidates(), hasPendingRecovery = hasPendingRecovery, hasPendingForDirectory = hasPendingForDirectory, shouldContinue = shouldContinue, diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt index 0f7a6e2c8..c863e65d6 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -4,6 +4,7 @@ import dev.obiente.nextcloudnative.app.FileSyncBaseline import dev.obiente.nextcloudnative.app.FileSyncConfiguration import dev.obiente.nextcloudnative.app.FileSyncPair import dev.obiente.nextcloudnative.app.SyncEntryKind +import java.io.FileNotFoundException import java.nio.file.Files import kotlin.test.Test import kotlin.test.assertEquals @@ -208,6 +209,30 @@ class AndroidFileSyncProviderFeedbackRecoveryTest { assertEquals(listOf("discover", "reconcile:relocated"), events) } + @Test + fun `missing recorded directory falls through to relocated discovery`() { + var pending = true + val events = mutableListOf() + + val reconciled = reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( + recordedCandidates = listOf("stale"), + discoverCandidates = { + events += "discover" + listOf("relocated") + }, + hasPendingRecovery = { pending }, + hasPendingForDirectory = { pending }, + reconcileDirectory = { candidate -> + events += "reconcile:$candidate" + if (candidate == "stale") throw FileNotFoundException("The directory moved") + pending = false + }, + ) + + assertTrue(reconciled) + assertEquals(listOf("reconcile:stale", "discover", "reconcile:relocated"), events) + } + @Test fun `relocated recovery directory keeps its exact relative path`() { val root = NextcloudDocumentIds.documentId(accountKey, "Sync") From 8e3b47b025628b25f22e1e7d8903b132d8d8b09e Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 09:10:06 +0200 Subject: [PATCH 12/17] refactor(android): preserve sync engine boundary --- .../dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt index 8f5646f48..4b5769ec5 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt @@ -318,9 +318,7 @@ internal class AndroidFileSyncEngine(context: Context) { var remoteCleanupRejected = false val removed = removeConfiguredFileSyncPair( reconcileLocalDownloads = { - reconcileSafDownloadsBeforePairRemoval( - appContext, pair.localRootId, androidSafOwnedDownloadRecoveryPaths(pair), - ) + reconcileSafDownloadsBeforePairRemoval(appContext, pair.localRootId, androidSafOwnedDownloadRecoveryPaths(pair)) }, cleanRemoteUploads = { val cleanupResult = cleanupJvmFileSyncOwnedUploads( @@ -412,9 +410,8 @@ internal class AndroidFileSyncEngine(context: Context) { FileSyncRejectionScope.Preflight, ) } - androidFileSyncRootRejection(initialPair.localRootId, appContext.packageName)?.let { rejection -> - return FileSyncCenterActionResult.Rejected(rejection.message, FileSyncRejectionScope.Preflight) - } + val rejection = androidFileSyncRootRejection(initialPair.localRootId, appContext.packageName) + if (rejection != null) return FileSyncCenterActionResult.Rejected(rejection.message, FileSyncRejectionScope.Preflight) val local = createAndroidFileSyncLocalTree(appContext, initialPair.localRootId) return withAndroidMediaBackupLedger(appContext, initialPair) { mediaLedger -> val remote = androidFileSyncOwnedRemoteTree( From 4fb89d472b0ddbd0e702ddddeada227a2748ab51 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 09:54:05 +0200 Subject: [PATCH 13/17] fix(android): order provider recovery account fencing --- .../nextcloudnative/AndroidAccountFileRead.kt | 6 ++- .../AndroidDocumentWritebackRecovery.kt | 3 +- .../AndroidDocumentsProviderSessionBinding.kt | 30 +++++++++---- .../AndroidFileSyncExecutionCoordination.kt | 43 +++++++++++++------ .../NextcloudDocumentsProvider.kt | 11 +++-- .../AndroidFileReadCacheTest.kt | 15 +++++++ ...oidFileSyncProviderFeedbackRecoveryTest.kt | 29 +++++++++++++ .../NextcloudDocumentsContractTest.kt | 35 +++++++++++++++ 8 files changed, 144 insertions(+), 28 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt index 48efa09f2..d2dba3293 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt @@ -14,9 +14,13 @@ internal suspend fun withRetainedAndroidAccountFileRead( expectedSession: NextcloudSession, resolveSession: suspend () -> NextcloudSession?, guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, + accountLeaseHeld: Boolean = false, read: suspend () -> Result, ): Result = withContext(Dispatchers.IO) { - guard.withExactAccountSession( + if (accountLeaseHeld) { + check(resolveSession() == expectedSession) { "The account changed before the file read could finish." } + read() + } else guard.withExactAccountSession( expectedSession = expectedSession, resolveSession = resolveSession, unavailable = { error("The account changed before the file read could finish.") }, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentWritebackRecovery.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentWritebackRecovery.kt index 6795c95c9..47522dac2 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentWritebackRecovery.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentWritebackRecovery.kt @@ -59,9 +59,10 @@ internal fun acquireAndroidDocumentMutationAccountLease( internal inline fun withAndroidDocumentMutation( session: NextcloudSession, noinline loadCurrentSession: () -> NextcloudSession?, + guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, action: (NextcloudSession) -> Result, ): Result { - val lease = acquireAndroidDocumentMutationAccountLease(session, loadCurrentSession) + val lease = acquireAndroidDocumentMutationAccountLease(session, loadCurrentSession, guard) return try { action(session) } finally { diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt index bb04fc8ca..9a17395fd 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidDocumentsProviderSessionBinding.kt @@ -199,12 +199,24 @@ internal fun withAndroidDocumentsProviderMutation( documentId: String, operation: AndroidDocumentsProviderRecoveryOperation, loadActiveSession: () -> NextcloudSession?, + guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, action: (NextcloudSession) -> Result, ): Result { val resolved = requireAndroidDocumentsProviderSession(documentId, operation, loadActiveSession) + return withResolvedAndroidDocumentsProviderMutation(resolved, loadActiveSession, guard, action) +} + +internal fun withResolvedAndroidDocumentsProviderMutation( + resolved: AndroidDocumentsProviderResolvedSession, + loadActiveSession: () -> NextcloudSession?, + guard: AndroidAccountOperationGuard, + action: (NextcloudSession) -> Result, +): Result { + if (resolved.recoveryAuthorized) return action(resolved.session) return withAndroidDocumentMutation( resolved.session, - { if (resolved.recoveryAuthorized) resolved.session else loadActiveSession() }, + loadActiveSession, + guard, action, ) } @@ -221,29 +233,29 @@ internal fun requireAndroidDocumentsProviderQuerySession( internal fun requireAndroidDocumentsProviderChildrenSession( documentId: String, loadActiveSession: () -> NextcloudSession?, -): NextcloudSession = requireAndroidDocumentsProviderSession( +): AndroidDocumentsProviderResolvedSession = requireAndroidDocumentsProviderSession( documentId, AndroidDocumentsProviderRecoveryOperation.QueryChildren, loadActiveSession, -).session +) internal fun requireAndroidDocumentsProviderOpenSession( documentId: String, mode: String, loadActiveSession: () -> NextcloudSession?, -): NextcloudSession = requireAndroidDocumentsProviderCallSession( +): AndroidDocumentsProviderResolvedSession = requireAndroidDocumentsProviderCallSession( documentId, if (mode == "r") AndroidDocumentsProviderRecoveryOperation.OpenRead else AndroidDocumentsProviderRecoveryOperation.OpenWrite, loadActiveSession, -).session +) internal fun withAndroidDocumentsProviderCreate( documentId: String, loadActiveSession: () -> NextcloudSession?, action: (NextcloudSession) -> Result, ): Result = withAndroidDocumentsProviderMutation( - documentId, AndroidDocumentsProviderRecoveryOperation.Create, loadActiveSession, action, + documentId, AndroidDocumentsProviderRecoveryOperation.Create, loadActiveSession, action = action, ) internal fun withAndroidDocumentsProviderRename( @@ -251,7 +263,7 @@ internal fun withAndroidDocumentsProviderRename( loadActiveSession: () -> NextcloudSession?, action: (NextcloudSession) -> Result, ): Result = withAndroidDocumentsProviderMutation( - documentId, AndroidDocumentsProviderRecoveryOperation.Rename, loadActiveSession, action, + documentId, AndroidDocumentsProviderRecoveryOperation.Rename, loadActiveSession, action = action, ) internal fun withAndroidDocumentsProviderDelete( @@ -259,7 +271,7 @@ internal fun withAndroidDocumentsProviderDelete( loadActiveSession: () -> NextcloudSession?, action: (NextcloudSession) -> Result, ): Result = withAndroidDocumentsProviderMutation( - documentId, AndroidDocumentsProviderRecoveryOperation.Delete, loadActiveSession, action, + documentId, AndroidDocumentsProviderRecoveryOperation.Delete, loadActiveSession, action = action, ) internal fun withAndroidDocumentsProviderMove( @@ -267,5 +279,5 @@ internal fun withAndroidDocumentsProviderMove( loadActiveSession: () -> NextcloudSession?, action: (NextcloudSession) -> Result, ): Result = withAndroidDocumentsProviderMutation( - documentId, AndroidDocumentsProviderRecoveryOperation.Move, loadActiveSession, action, + documentId, AndroidDocumentsProviderRecoveryOperation.Move, loadActiveSession, action = action, ) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt index 76eb42299..a4b61d4b3 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncExecutionCoordination.kt @@ -284,22 +284,39 @@ internal suspend fun reconcileAndroidFileSyncAccountDownloadsBeforeCredentialRem accountId: String, providerRecoverySession: NextcloudSession, ) { - AndroidFileSyncEngine.ENGINE_LOCK.withLock { - reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( - pairs = AndroidFileSyncStore(context).load().coordinator.pairs, - accountId = accountId, - reconcileLocalDownloads = { pair -> - reconcileSafDownloadsBeforePairRemoval( - context = context, - localRootId = pair.localRootId, - localRecoveryPaths = androidSafOwnedDownloadRecoveryPaths(pair), - providerRecoverySession = providerRecoverySession, - ) - }, - ) + val services = AndroidNextcloudServices(context.applicationContext) + withAndroidFileSyncAccountRecoveryLease( + expectedSession = providerRecoverySession, + resolveSession = { services.loadSession(providerRecoverySession.accountId) }, + ) { + AndroidFileSyncEngine.ENGINE_LOCK.withLock { + reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( + pairs = AndroidFileSyncStore(context).load().coordinator.pairs, + accountId = accountId, + reconcileLocalDownloads = { pair -> + reconcileSafDownloadsBeforePairRemoval( + context = context, + localRootId = pair.localRootId, + localRecoveryPaths = androidSafOwnedDownloadRecoveryPaths(pair), + providerRecoverySession = providerRecoverySession, + ) + }, + ) + } } } +internal suspend fun withAndroidFileSyncAccountRecoveryLease( + expectedSession: NextcloudSession, + resolveSession: suspend () -> NextcloudSession?, + guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, + action: suspend () -> Result, +): Result = guard.withExactAccountSession( + expectedSession = expectedSession, + resolveSession = resolveSession, + unavailable = { error("The account changed before folder sync recovery could start.") }, +) { action() } + internal suspend fun reconcileConfiguredFileSyncAccountDownloadsBeforeCredentialRemoval( pairs: List, accountId: String, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt index 84ab3c52d..027d71f2f 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt @@ -125,11 +125,14 @@ class NextcloudDocumentsProvider : DocumentsProvider() { ): Cursor { val columns = projection?.copyOf() ?: DEFAULT_DOCUMENT_PROJECTION val cursor = MatrixCursor(columns) - val session = requireAndroidDocumentsProviderChildrenSession(parentDocumentId, services::loadSession) + val (session, recoveryAuthorized) = requireAndroidDocumentsProviderChildrenSession(parentDocumentId, services::loadSession) val parent = requireReference(parentDocumentId, session) val children = runCatching { val account = resolveAccount(session) - runBlocking(Dispatchers.IO) { services.listFiles(session, account.userId, parent.path) } + runBlocking(Dispatchers.IO) { + if (recoveryAuthorized) services.listFilesWhileAccountLeaseHeld(session, account.userId, parent.path) + else services.listFiles(session, account.userId, parent.path) + } }.getOrElse { failure -> val cachedChildren = offline.availableChildren(session, parent.path) if (cachedChildren.isNotEmpty() || offline.isStoredDirectory(session, parent.path)) { @@ -186,7 +189,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } signal?.throwIfCanceled() - val session = requireAndroidDocumentsProviderOpenSession(documentId, mode, services::loadSession) + val (session, recoveryAuthorized) = requireAndroidDocumentsProviderOpenSession(documentId, mode, services::loadSession) if (AndroidExternalFileHandoffRegistry.isHandoffDocumentId(documentId)) { if (mode != "r") throw SecurityException("External file handoffs are read-only.") return openExternalHandoffDocument(session, documentId, signal) @@ -200,7 +203,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } } val account = resolveAccount(session) - val file = runCatching { findDocument(session, account, reference.path) } + val file = runCatching { findDocument(session, account, reference.path, recoveryAuthorized) } .getOrElse { failure -> if (mode == "r") { virtualFiles.acquire(session, reference.path)?.let { lease -> diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileReadCacheTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileReadCacheTest.kt index 56d138995..c71a8ea6c 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileReadCacheTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileReadCacheTest.kt @@ -12,8 +12,23 @@ import kotlin.test.assertTrue import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.async import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout class AndroidFileReadCacheTest { + @Test + fun providerRecoveryUsesTheAccountLeaseHeldByItsCaller() = runBlocking { + val guard = AndroidAccountOperationGuard() + val session = NextcloudSession("https://cloud.example.test", "alice", "fixture-password") + + guard.withAccount(NextcloudDocumentIds.accountKey(session)) { + withTimeout(1_000L) { + withRetainedAndroidAccountFileRead( + session, { session }, guard, accountLeaseHeld = true, + ) {} + } + } + } + @Test fun listingMetadataSurvivesProcessRestartWithFullDavIdentity() = withCache { root, cache -> val file = file("Notes/vault.md", "\"etag-1\"").copy( diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt index c863e65d6..d59b08e5f 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncProviderFeedbackRecoveryTest.kt @@ -3,6 +3,7 @@ package dev.obiente.nextcloudnative import dev.obiente.nextcloudnative.app.FileSyncBaseline import dev.obiente.nextcloudnative.app.FileSyncConfiguration import dev.obiente.nextcloudnative.app.FileSyncPair +import dev.obiente.nextcloudnative.app.NextcloudSession import dev.obiente.nextcloudnative.app.SyncEntryKind import java.io.FileNotFoundException import java.nio.file.Files @@ -12,13 +13,41 @@ import kotlin.test.assertFalse import kotlin.test.assertFailsWith import kotlin.test.assertTrue import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.async import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withTimeout class AndroidFileSyncProviderFeedbackRecoveryTest { private val applicationId = "dev.obiente.nextcloudnative.dev" private val ownAuthority = nextcloudDocumentsAuthority(applicationId) private val accountKey = "0123456789abcdef0123456789abcdef" + @Test + fun `credential removal fences account work before waiting for the sync engine`() = runBlocking { + val session = NextcloudSession("https://cloud.example.test", "alice", "fixture-password") + val guard = AndroidAccountOperationGuard() + val engineLock = Mutex(locked = true) + val waitingForEngine = CompletableDeferred() + val recovery = async { + withAndroidFileSyncAccountRecoveryLease(session, { session }, guard) { + waitingForEngine.complete(Unit) + engineLock.withLock {} + } + } + waitingForEngine.await() + + val competingWorkEntered = guard.tryWithAccount( + NextcloudDocumentIds.accountKey(session), unavailable = { false }, action = { true }, + ) + assertFalse(competingWorkEntered) + + engineLock.unlock() + withTimeout(1_000L) { recovery.await() } + } + @Test fun `restored own provider root stops before remote preparation`() { var remoteCalls = 0 diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt index bf5c2f390..0a0304273 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsContractTest.kt @@ -333,6 +333,41 @@ class NextcloudDocumentsContractTest { ) } + @Test + fun `recovery mutation uses the account lease held by credential removal`() = runBlocking { + val removed = session("removed", "removed-secret") + val documentId = NextcloudDocumentIds.documentId(removed, "Sync/recovery-backup") + val guard = AndroidAccountOperationGuard() + var mutationEntered = false + + guard.withAccount(NextcloudDocumentIds.accountKey(removed)) { + withAndroidDocumentsProviderRecoveryPermit( + removed, + documentId, + AndroidDocumentsProviderRecoveryOperation.Rename, + ) { + val resolved = requireNotNull( + resolveAndroidDocumentsProviderSession( + documentId, + AndroidDocumentsProviderRecoveryOperation.Rename, + allowRecoveryPermit = true, + loadActiveSession = { null }, + ), + ) + withResolvedAndroidDocumentsProviderMutation( + resolved, + loadActiveSession = { null }, + guard = guard, + ) { session -> + assertEquals(removed, session) + mutationEntered = true + } + } + } + + assertTrue(mutationEntered) + } + @Test fun `account removal preflight runs before remote credential revocation`() = runBlocking { var revoked = false From 2fa9a3d6e9129004ac6e8141e189332e4c892c70 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 11:33:34 +0200 Subject: [PATCH 14/17] fix(android): scope legacy SAF retirement completion --- .../AndroidLocalUploadPickerPersistence.kt | 22 -------- .../AndroidSafDownloadOwnershipStore.kt | 21 +++++--- .../AndroidSafOwnedDownloadRetirement.kt | 38 +++++++++++++- .../AndroidSafDownloadOwnershipIndexTest.kt | 51 +++++++++++++++++++ 4 files changed, 103 insertions(+), 29 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt index 5a3f2149f..2885507f6 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidLocalUploadPickerPersistence.kt @@ -3,7 +3,6 @@ package dev.obiente.nextcloudnative import dev.obiente.nextcloudnative.app.LocalUploadFile import dev.obiente.nextcloudnative.app.LocalUploadSelectionResult import kotlinx.coroutines.CancellableContinuation -import org.json.JSONObject import kotlin.coroutines.resume internal fun requireSafeProcessGeneration(value: String) { @@ -12,27 +11,6 @@ internal fun requireSafeProcessGeneration(value: String) { } } -internal fun JSONObject.optionalStrictString(key: String): String? { - if (!has(key) || isNull(key)) return null - return requireStrictString(key) -} - -internal fun JSONObject.requireStrictString(key: String): String = get(key).let { value -> - require(value is String) { "The $key value changed type." } - value -} - -internal fun JSONObject.optionalStrictBoolean(key: String): Boolean? { - if (!has(key) || isNull(key)) return null - return get(key).let { value -> - require(value is Boolean) { "The $key value changed type." } - value - } -} - -internal fun persistedDurableUploadGrantPreExisting(payload: JSONObject): Boolean = - payload.optionalStrictBoolean("grantPreExisting") ?: false - internal fun resumeLocalUploadSelectionResult( continuation: CancellableContinuation, result: LocalUploadSelectionResult, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt index d91f42d32..d854fb9bc 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipStore.kt @@ -30,7 +30,11 @@ internal class AndroidSafDownloadOwnershipStore( } fun pendingTransactions(): List = synchronized(LOCK) { - ownershipRows().map(StoredOwnershipRow::transaction) + ownershipRows(includeAll = true).map(StoredOwnershipRow::transaction) + } + + fun legacyPendingTransactions(): List = synchronized(LOCK) { + ownershipRows(files = legacyOwnershipFiles(), includeAll = true).map(StoredOwnershipRow::transaction) } override fun hasPendingTransactionsForDirectory(directoryIdentity: String): Boolean = synchronized(LOCK) { @@ -275,9 +279,11 @@ internal class AndroidSafDownloadOwnershipStore( private fun ownershipRows( scope: String? = null, tokens: Set = emptySet(), - ): List = ownershipFiles() + files: List = ownershipFiles(), + includeAll: Boolean = false, + ): List = files .mapNotNull(::ownershipReference) - .filter { reference -> reference.scope == scope || reference.token in tokens } + .filter { reference -> includeAll || reference.scope == scope || reference.token in tokens } .map { reference -> val transaction = readRow(reference.file) check(transaction.token == reference.token) { "SAF download recovery row name is invalid." } @@ -286,11 +292,14 @@ internal class AndroidSafDownloadOwnershipStore( private fun ownershipFiles(): List = buildList { addAll(ownershipFiles(directory, listFiles)) - legacyDirectory?.takeIf { it != directory }?.let { legacy -> - addAll(ownershipFiles(legacy, legacy::listFiles)) - } + addAll(legacyOwnershipFiles()) }.distinctBy(File::getAbsolutePath) + private fun legacyOwnershipFiles(): List = legacyDirectory + ?.takeIf { it != directory } + ?.let { legacy -> ownershipFiles(legacy, legacy::listFiles) } + .orEmpty() + private fun ownershipFiles( rowDirectory: File, listing: () -> Array?, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt index 734baa2cb..54fb28242 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidSafOwnedDownloadRetirement.kt @@ -120,6 +120,24 @@ internal fun requireAndroidSafRetirementContinuation(shouldContinue: () -> Boole if (!shouldContinue()) throw CancellationException("Folder sync recovery was cancelled.") } +internal fun hasRelevantAndroidSafOwnedDownloadRecovery( + treeScopedPending: Boolean, + legacyTransactions: List, + identityBelongsToTree: (String) -> Boolean?, +): Boolean = treeScopedPending || legacyTransactions.any { transaction -> + !androidSafOwnedDownloadIsProvenUnrelatedToTree(transaction, identityBelongsToTree) +} + +internal fun androidSafOwnedDownloadIsProvenUnrelatedToTree( + transaction: AndroidSafOwnedDownloadTransaction, + identityBelongsToTree: (String) -> Boolean?, +): Boolean { + val identities = listOfNotNull(transaction.stageDocumentIdentity, transaction.backupDocumentIdentity) + if (identities.isEmpty()) return false + val memberships = identities.map { identity -> identityBelongsToTree(identity) ?: return false } + return memberships.none { it } +} + internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( context: Context, localRootId: String, @@ -158,7 +176,25 @@ internal fun reconcileOwnProviderSafDownloadsBeforePairRemoval( ).map { candidate -> candidate to DocumentsContract.buildDocumentUriUsingTree(treeUri, candidate.documentId) } - val hasRelevantPendingRecovery = indexedOwnership::hasPendingTransactions + val identityBelongsToTree: (String) -> Boolean? = identity@{ identity -> + if ( + androidPickerUriRejection(identity, appContext.packageName) != + AndroidPickerUriRejection.OwnDocumentsProvider + ) return@identity null + val documentId = runCatching { DocumentsContract.getDocumentId(Uri.parse(identity)) }.getOrNull() + ?: return@identity null + androidSafOwnedDownloadRecoveryDirectory( + DocumentsContract.getTreeDocumentId(treeUri), + documentId, + ) != null + } + val hasRelevantPendingRecovery = { + hasRelevantAndroidSafOwnedDownloadRecovery( + treeScopedPending = ownership.hasTreeScopedPendingTransactions(), + legacyTransactions = ownership.legacyPendingTransactions(), + identityBelongsToTree = identityBelongsToTree, + ) + } check( reconcileRecordedThenDiscoveredAndroidSafDownloadDirectories( recordedCandidates = recordedCandidates, diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt index 83a436dc3..5345a5a92 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidSafDownloadOwnershipIndexTest.kt @@ -66,6 +66,57 @@ class AndroidSafDownloadOwnershipIndexTest { } } + @Test + fun `selected tree ignores only legacy ownership proven to belong elsewhere`() { + val unrelated = AndroidSafOwnedDownloadTransaction( + "Elsewhere.txt", + FIRST_TOKEN, + stageDocumentIdentity = "document:other-tree", + ) + val unclassified = AndroidSafOwnedDownloadTransaction("Unknown.txt", SECOND_TOKEN) + + assertFalse( + hasRelevantAndroidSafOwnedDownloadRecovery( + treeScopedPending = false, + legacyTransactions = listOf(unrelated), + identityBelongsToTree = { false }, + ), + ) + assertTrue( + hasRelevantAndroidSafOwnedDownloadRecovery( + treeScopedPending = false, + legacyTransactions = listOf(unclassified), + identityBelongsToTree = { null }, + ), + ) + assertTrue( + hasRelevantAndroidSafOwnedDownloadRecovery( + treeScopedPending = true, + legacyTransactions = listOf(unrelated), + identityBelongsToTree = { false }, + ), + ) + } + + @Test + fun `selected tree reads legacy and tree scoped ownership separately`() { + val base = Files.createTempDirectory("saf-download-selected-tree-").toFile() + try { + val legacy = AndroidSafDownloadOwnershipStore(base) + val selected = androidSafDownloadOwnershipStoreForTree(base, "content://provider/tree/selected") + val legacyTransaction = AndroidSafOwnedDownloadTransaction("Legacy.txt", FIRST_TOKEN) + val selectedTransaction = AndroidSafOwnedDownloadTransaction("Selected.txt", SECOND_TOKEN) + legacy.forDirectory("content://provider/tree/other/document/parent").add(legacyTransaction) + selected.forDirectory("content://provider/tree/selected/document/parent").add(selectedTransaction) + + assertEquals(listOf(legacyTransaction), selected.legacyPendingTransactions()) + assertEquals(listOf(selectedTransaction, legacyTransaction), selected.pendingTransactions()) + assertTrue(selected.hasTreeScopedPendingTransactions()) + } finally { + base.deleteRecursively() + } + } + @Test fun `tree-wide recovery indexing is skipped without pending ownership`() { val root = Files.createTempDirectory("saf-download-empty-index-").toFile() From 70a4c3ecf394c6375eab2a2baeb086e5d04a6698 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sun, 6 Sep 2026 13:26:19 +0200 Subject: [PATCH 15/17] fix(android): prepare malformed reset before lease --- .../AndroidAccountCredentialController.kt | 14 ++++-- .../AndroidAccountFileListing.kt | 2 +- .../AndroidMalformedCredentialReset.kt | 11 +++- .../NextcloudDocumentsProvider.kt | 5 +- ...droidIndependentCredentialSlotResetTest.kt | 50 ++++++++++++------- 5 files changed, 53 insertions(+), 29 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt index f674cd9da..8ed939049 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountCredentialController.kt @@ -325,9 +325,17 @@ internal class AndroidAccountCredentialController( } private suspend fun clearUnregisteredIndependentCredentialSlots(suspectEncrypted: String?) = clearUnregisteredAndroidAccountCredentialSlots( - preferences, sessionCipher, accountRemovalCleanupJournal, suspectEncrypted, - prepareAccountRemoval, removeQueuedUploads, ::commitPreferences, ::recordAccountRemovalCleanupFailure, - ::clearInvalidStore) + preferences = preferences, + sessionCipher = sessionCipher, + cleanupJournal = accountRemovalCleanupJournal, + suspectEncrypted = suspectEncrypted, + prepareAccountRemoval = { session -> prepareAndroidAccountRemoval(appContext, session) }, + revalidateAccountRemoval = { session -> preflightAndroidAccountRemoval(appContext, session) }, + removeAccountOwnedState = removeQueuedUploads, + commitPreferences = ::commitPreferences, + recordCleanupFailure = ::recordAccountRemovalCleanupFailure, + clearInvalidStore = ::clearInvalidStore, + ) private suspend fun clearRecoveredInvalidStore( current: AndroidAccountCredentialState, diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileListing.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileListing.kt index 604584105..c53f205fd 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileListing.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileListing.kt @@ -22,7 +22,7 @@ internal suspend fun loadAndroidAccountFileListing( val read: suspend () -> NextcloudFileListing = { readAndroidAccountFileListing(cache, NextcloudDocumentIds.accountKey(session), path, request) } - return if (accountLeaseHeld) read() else withRetainedAndroidAccountFileRead(session, resolveSession, guard, read) + return if (accountLeaseHeld) read() else withRetainedAndroidAccountFileRead(session, resolveSession, guard, read = read) } private suspend fun readAndroidAccountFileListing( diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMalformedCredentialReset.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMalformedCredentialReset.kt index f7f44e284..1435494c1 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMalformedCredentialReset.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidMalformedCredentialReset.kt @@ -9,6 +9,7 @@ internal suspend fun clearUnregisteredAndroidAccountCredentialSlots( cleanupJournal: AndroidAccountRemovalCleanupJournal, suspectEncrypted: String?, prepareAccountRemoval: suspend (NextcloudSession) -> Unit, + revalidateAccountRemoval: suspend (NextcloudSession) -> Unit, removeAccountOwnedState: suspend (NextcloudSession) -> Unit, commitPreferences: (SharedPreferences.Editor) -> Unit, recordCleanupFailure: (Exception) -> Unit, @@ -35,6 +36,7 @@ internal suspend fun clearUnregisteredAndroidAccountCredentialSlots( ) }, prepareAccountRemoval = prepareAccountRemoval, + revalidateAccountRemoval = revalidateAccountRemoval, commitSlotRemoval = { slot, cleanup -> commitPreferences( cleanupJournal.prepareEdit(preferences.edit().remove(slot.preferenceKey), cleanup), @@ -56,6 +58,7 @@ internal suspend fun retireUnregisteredAndroidAccountCredentialSlots( retryPreexistingCleanup: suspend (AndroidIndependentCredentialSlotReset) -> Unit = {}, guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, prepareAccountRemoval: suspend (NextcloudSession) -> Unit, + revalidateAccountRemoval: suspend (NextcloudSession) -> Unit = {}, commitSlotRemoval: suspend (AndroidIndependentCredentialSlotReset, AndroidPendingAccountRemovalCleanup) -> Unit, rollbackSlotRemoval: suspend (AndroidIndependentCredentialSlotReset) -> Unit, removeAccountOwnedState: suspend (NextcloudSession) -> Unit, @@ -68,9 +71,13 @@ internal suspend fun retireUnregisteredAndroidAccountCredentialSlots( retryPreexistingCleanup(slot) } val pendingCleanup = pendingAndroidAccountRemovalCleanup(session) - withAndroidAccountRemovalLease(NextcloudDocumentIds.accountKey(session), guard) { + withPreparedAndroidAccountRemovalLease( + accountIdentity = NextcloudDocumentIds.accountKey(session), + guard = guard, + prepare = { prepareAccountRemoval(session) }, + revalidate = { revalidateAccountRemoval(session) }, + ) { removeRecoveredAndroidAccountCredentialData( - prepareAccountRemoval = { prepareAccountRemoval(session) }, removeQueuedUploads = { removeAccountOwnedState(session) }, clearRecoveredAccount = { commitSlotRemoval(slot, pendingCleanup) }, rollbackRecoveredAccount = { diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt index 027d71f2f..b4fa0b945 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt @@ -863,10 +863,7 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } private fun findDocument( - session: NextcloudSession, - account: ResolvedAccount, - path: String, - accountLeaseHeld: Boolean = false, + session: NextcloudSession, account: ResolvedAccount, path: String, accountLeaseHeld: Boolean = false, ): NextcloudFile = providerCall( message = "The requested Nextcloud document was not found.", diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidIndependentCredentialSlotResetTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidIndependentCredentialSlotResetTest.kt index 40ee694de..0bcebb695 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidIndependentCredentialSlotResetTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidIndependentCredentialSlotResetTest.kt @@ -8,6 +8,7 @@ import kotlin.test.assertFalse import kotlin.test.assertTrue import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout class AndroidIndependentCredentialSlotResetTest { @Test @@ -66,28 +67,39 @@ class AndroidIndependentCredentialSlotResetTest { val events = mutableListOf() val presentSlots = mutableSetOf(first.preferenceKey, second.preferenceKey) val tombstones = mutableSetOf() + val guard = AndroidAccountOperationGuard() - retireUnregisteredAndroidAccountCredentialSlots( - slots = listOf(first, second), - guard = AndroidAccountOperationGuard(), - prepareAccountRemoval = { session -> events += "prepare-${session.loginName}" }, - commitSlotRemoval = { slot, cleanup -> - events += "commit-${slot.session.loginName}" - presentSlots -= slot.preferenceKey - tombstones += cleanup.accountStorageKey - }, - rollbackSlotRemoval = { slot -> presentSlots += slot.preferenceKey }, - removeAccountOwnedState = { session -> - assertFalse(androidAccountCredentialSlotKey(session.accountId) in presentSlots) - assertTrue(session.accountId.storageKey in tombstones) - events += "cleanup-${session.loginName}" - }, - clearCleanup = { accountStorageKey -> tombstones -= accountStorageKey }, - recordCleanupFailure = { error("cleanup must succeed") }, - ) + withTimeout(1_000L) { + retireUnregisteredAndroidAccountCredentialSlots( + slots = listOf(first, second), + guard = guard, + prepareAccountRemoval = { session -> + guard.withAccount(NextcloudDocumentIds.accountKey(session)) { + events += "prepare-${session.loginName}" + } + }, + revalidateAccountRemoval = { session -> events += "revalidate-${session.loginName}" }, + commitSlotRemoval = { slot, cleanup -> + events += "commit-${slot.session.loginName}" + presentSlots -= slot.preferenceKey + tombstones += cleanup.accountStorageKey + }, + rollbackSlotRemoval = { slot -> presentSlots += slot.preferenceKey }, + removeAccountOwnedState = { session -> + assertFalse(androidAccountCredentialSlotKey(session.accountId) in presentSlots) + assertTrue(session.accountId.storageKey in tombstones) + events += "cleanup-${session.loginName}" + }, + clearCleanup = { accountStorageKey -> tombstones -= accountStorageKey }, + recordCleanupFailure = { error("cleanup must succeed") }, + ) + } assertEquals( - listOf("prepare-alice", "commit-alice", "cleanup-alice", "prepare-bob", "commit-bob", "cleanup-bob"), + listOf( + "prepare-alice", "revalidate-alice", "commit-alice", "cleanup-alice", + "prepare-bob", "revalidate-bob", "commit-bob", "cleanup-bob", + ), events, ) assertTrue(presentSlots.isEmpty()) From bd11db7cd7eec57c12f5d826c40c591ee510dc74 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Wed, 9 Sep 2026 08:55:48 +0200 Subject: [PATCH 16/17] fix(android): reuse recovery lease for provider range reads --- .../nextcloudnative/AndroidAccountFileRead.kt | 42 ++++++++++++++- .../AndroidNextcloudServices.kt | 49 ++++++++--------- .../NextcloudDocumentsProvider.kt | 9 ++-- .../AndroidAccountOperationGuardTest.kt | 54 +++++++++++++++++++ .../android-picker-provider-feedback.md | 2 +- 5 files changed, 124 insertions(+), 32 deletions(-) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt index d2dba3293..b29bf3644 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidAccountFileRead.kt @@ -1,6 +1,7 @@ package dev.obiente.nextcloudnative import android.util.Base64 +import dev.obiente.nextcloudnative.app.NextcloudFile import dev.obiente.nextcloudnative.app.NextcloudFileRangeSession import dev.obiente.nextcloudnative.app.NextcloudSession import java.io.FileNotFoundException @@ -139,9 +140,10 @@ internal fun openTrackedAndroidFileRangeSession( activity: AndroidFileRangeSessionActivity, guard: AndroidAccountOperationGuard = ANDROID_ACCOUNT_OPERATION_GUARD, coordinator: AndroidFileRangeSessionCoordinator = ANDROID_FILE_RANGE_SESSION_COORDINATOR, + accountLeaseHeld: Boolean = false, openSource: () -> NextcloudFileRangeSession, ): NextcloudFileRangeSession { - val lease = guard.acquireBlocking(NextcloudDocumentIds.accountKey(expectedSession)) + val lease = if (accountLeaseHeld) null else guard.acquireBlocking(NextcloudDocumentIds.accountKey(expectedSession)) return try { if (resolveSession() != expectedSession) { throw FileNotFoundException("The account changed before the file range session could start.") @@ -155,7 +157,7 @@ internal fun openTrackedAndroidFileRangeSession( activity.close() throw failure } finally { - lease.close() + lease?.close() } } @@ -163,3 +165,39 @@ internal fun androidFileRangeAuthorization(session: NextcloudSession): String = "${session.loginName}:${session.appPassword}".toByteArray(StandardCharsets.UTF_8), Base64.NO_WRAP, ) + +internal fun AndroidNextcloudServices.openDocumentProviderFileRangeSession( + session: NextcloudSession, + userId: String, + path: String, + size: Long, + expectedEtag: String, + accountLeaseHeld: Boolean, +): NextcloudFileRangeSession = if (accountLeaseHeld) { + openFileRangeSessionWhileAccountLeaseHeld(session, userId, path, size, expectedEtag) +} else { + openFileRangeSession(session, userId, path, size, expectedEtag) +} + +internal class AndroidFileRangeUnsupportedException(message: String) : Exception(message) + +internal suspend fun probeSeekableExternalHandoffGeneration( + file: NextcloudFile, + verifyEmptyGeneration: suspend () -> Unit, + openRangeSession: (size: Long, etag: String) -> NextcloudFileRangeSession, +): Boolean { + val size = file.size ?: return false + val etag = file.etag?.takeIf(String::isNotBlank) ?: return false + if (size == 0L) { + verifyEmptyGeneration() + return true + } + val rangeSession = openRangeSession(size, etag) + return try { + rangeSession.read(0L, 1).size == 1 + } catch (_: AndroidFileRangeUnsupportedException) { + false + } finally { + rangeSession.close() + } +} diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt index e41de96c1..4eda358c0 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt @@ -2366,6 +2366,27 @@ internal class AndroidNextcloudServices( path: String, size: Long, expectedEtag: String, + ): NextcloudFileRangeSession = openFileRangeSession( + session, userId, path, size, expectedEtag, accountLeaseHeld = false, + ) + + internal fun openFileRangeSessionWhileAccountLeaseHeld( + session: NextcloudSession, + userId: String, + path: String, + size: Long, + expectedEtag: String, + ): NextcloudFileRangeSession = openFileRangeSession( + session, userId, path, size, expectedEtag, accountLeaseHeld = true, + ) + + private fun openFileRangeSession( + session: NextcloudSession, + userId: String, + path: String, + size: Long, + expectedEtag: String, + accountLeaseHeld: Boolean, ): NextcloudFileRangeSession { require(size > 0L) { "The file range session size must be positive." } val safeEtag = requireSafeFileRangeEtag(expectedEtag) @@ -2373,7 +2394,10 @@ internal class AndroidNextcloudServices( val authorization = androidFileRangeAuthorization(session) val closed = AtomicBoolean(false) val activity = AndroidFileRangeSessionActivity() - return openTrackedAndroidFileRangeSession(session, { loadSession(session.accountId) }, activity) { + return openTrackedAndroidFileRangeSession( + session, { loadSession(session.accountId) }, activity, + accountLeaseHeld = accountLeaseHeld, + ) { NextcloudFileRangeSession( size = size, readBlock = { offset, length -> @@ -3908,29 +3932,6 @@ internal class AndroidNextcloudServices( } } -internal class AndroidFileRangeUnsupportedException(message: String) : Exception(message) - -internal suspend fun probeSeekableExternalHandoffGeneration( - file: NextcloudFile, - verifyEmptyGeneration: suspend () -> Unit, - openRangeSession: (size: Long, etag: String) -> NextcloudFileRangeSession, -): Boolean { - val size = file.size ?: return false - val etag = file.etag?.takeIf(String::isNotBlank) ?: return false - if (size == 0L) { - verifyEmptyGeneration() - return true - } - val rangeSession = openRangeSession(size, etag) - return try { - rangeSession.read(0L, 1).size == 1 - } catch (_: AndroidFileRangeUnsupportedException) { - false - } finally { - rangeSession.close() - } -} - private fun NextcloudFile.isNativeTiffPreviewFormat(): Boolean { if (isDirectory) return false val extension = name.substringAfterLast('.', missingDelimiterValue = "").lowercase(Locale.ROOT) diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt index b4fa0b945..8b34bc4a9 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt @@ -223,14 +223,14 @@ class NextcloudDocumentsProvider : DocumentsProvider() { } } - return openVirtualFileProxy(session, account.userId, file, signal) + return openVirtualFileProxy(session, account.userId, file, signal, recoveryAuthorized) } private fun openVirtualFileProxy( session: NextcloudSession, userId: String, file: NextcloudFile, - signal: CancellationSignal?, + signal: CancellationSignal?, accountLeaseHeld: Boolean, ): ParcelFileDescriptor { val size = file.size ?: throw FileNotFoundException( "Nextcloud did not provide a file size for seekable access.", @@ -250,12 +250,11 @@ class NextcloudDocumentsProvider : DocumentsProvider() { virtualFiles.discardHydrationStagingFile(empty) } } - val rangeSession = services.openFileRangeSession( + val rangeSession = services.openDocumentProviderFileRangeSession( session = session, userId = userId, path = file.path, - size = size, - expectedEtag = etag, + size = size, expectedEtag = etag, accountLeaseHeld = accountLeaseHeld, ) val staging = try { virtualFiles.prepareHydration(session, size) diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt index 29ab409ee..4421d5257 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidAccountOperationGuardTest.kt @@ -3,6 +3,8 @@ package dev.obiente.nextcloudnative import dev.obiente.nextcloudnative.app.NextcloudFileRangeSession import dev.obiente.nextcloudnative.app.NextcloudSession import java.io.FileNotFoundException +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -455,6 +457,58 @@ class AndroidAccountOperationGuardTest { withAndroidAccountRemovalLease(NextcloudDocumentIds.accountKey(session), guard) { } } + @Test + fun recoveryRangeSessionUsesHeldAccountLeaseAndStillValidatesSession() = runBlocking { + val guard = AndroidAccountOperationGuard() + val coordinator = AndroidFileRangeSessionCoordinator() + val session = NextcloudSession("https://cloud.example.test", "alice", "password") + var sourceOpened = false + val executor = Executors.newSingleThreadExecutor() + + try { + guard.withAccount(NextcloudDocumentIds.accountKey(session)) { + val read = executor.submit { + runBlocking { + val rangeSession = openTrackedAndroidFileRangeSession( + expectedSession = session, + resolveSession = { session }, + activity = AndroidFileRangeSessionActivity(), + guard = guard, + coordinator = coordinator, + accountLeaseHeld = true, + openSource = { + sourceOpened = true + NextcloudFileRangeSession(8L, { _, length -> ByteArray(length) }) + }, + ) + try { + rangeSession.read(0L, 1).size == 1 + } finally { + rangeSession.close() + } + } + } + assertTrue(read.get(1, TimeUnit.SECONDS)) + + assertFailsWith { + openTrackedAndroidFileRangeSession( + expectedSession = session, + resolveSession = { session.copy(appPassword = "replacement-password") }, + activity = AndroidFileRangeSessionActivity(), + guard = guard, + coordinator = coordinator, + accountLeaseHeld = true, + openSource = { error("stale recovery source must not open") }, + ) + } + } + } finally { + executor.shutdownNow() + } + + assertTrue(sourceOpened) + } + @Test fun sameAccountReauthenticationDrainsOldPasswordRangeBeforeCredentialCommit() = runBlocking { val coordinator = AndroidFileRangeSessionCoordinator() diff --git a/changes/unreleased/android-picker-provider-feedback.md b/changes/unreleased/android-picker-provider-feedback.md index 29b31c948..3205bd36f 100644 --- a/changes/unreleased/android-picker-provider-feedback.md +++ b/changes/unreleased/android-picker-provider-feedback.md @@ -4,4 +4,4 @@ pull: 446 platforms: android user-facing: yes -Android file and folder pickers now reject the app's own document provider so selected uploads and sync roots always come from an independent storage source. +Android pickers now reject the app's own document provider, ensuring uploads and sync roots use independent storage. Legacy self-provider cleanup can read uncached replacement files without blocking account removal. From 923cf9f1c757986e78759da6f5fa4ad94e68c343 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Wed, 9 Sep 2026 08:56:12 +0200 Subject: [PATCH 17/17] chore(android): tighten document provider size baseline --- tools/kotlin-file-size-baseline.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/kotlin-file-size-baseline.txt b/tools/kotlin-file-size-baseline.txt index ed786333c..598eff381 100644 --- a/tools/kotlin-file-size-baseline.txt +++ b/tools/kotlin-file-size-baseline.txt @@ -1,7 +1,7 @@ androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidFileSyncEngine.kt|851 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt|4230 androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidProjectContentClient.kt|985 -androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt|995 +androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudDocumentsProvider.kt|994 contractAcquisition/src/main/kotlin/dev/obiente/nextcloudnative/contracts/SignedAppStoreContractAcquirer.kt|1224 contractAcquisition/src/main/kotlin/dev/obiente/nextcloudnative/contracts/StaticRouteContract.kt|1883 contractAcquisition/src/test/kotlin/dev/obiente/nextcloudnative/contracts/SignedAppStoreContractAcquirerTest.kt|1798