From d93624c5dd93bdbb947d645187c1a5b0fed34c1b Mon Sep 17 00:00:00 2001 From: Julian Raufelder Date: Thu, 3 Feb 2022 22:37:03 +0100 Subject: [PATCH] Fix race condition when AuthActivity finished and resumes BrowseFiles CloudList gets refreshed and token is updated, if the token update is faster, resuming overwrites the delegates in DispatchingCloudContentRepository again due to authentication fail because the old folder is used in the onFolderReloadContent because folder in BrowseFilesActivity isn't yet updated. --- .../data/cloud/crypto/CryptoCloud.java | 2 +- .../domain/usecases/vault/ReloadVault.java | 24 --------- .../presenter/BrowseFilesPresenter.kt | 49 ++++++++++++++----- 3 files changed, 39 insertions(+), 36 deletions(-) delete mode 100644 domain/src/main/java/org/cryptomator/domain/usecases/vault/ReloadVault.java diff --git a/data/src/main/java/org/cryptomator/data/cloud/crypto/CryptoCloud.java b/data/src/main/java/org/cryptomator/data/cloud/crypto/CryptoCloud.java index 7dc116cd..eeadb135 100644 --- a/data/src/main/java/org/cryptomator/data/cloud/crypto/CryptoCloud.java +++ b/data/src/main/java/org/cryptomator/data/cloud/crypto/CryptoCloud.java @@ -8,7 +8,7 @@ public class CryptoCloud implements Cloud { private final Vault vault; - public CryptoCloud(Vault vault) { + CryptoCloud(Vault vault) { this.vault = vault; } diff --git a/domain/src/main/java/org/cryptomator/domain/usecases/vault/ReloadVault.java b/domain/src/main/java/org/cryptomator/domain/usecases/vault/ReloadVault.java deleted file mode 100644 index 55196ada..00000000 --- a/domain/src/main/java/org/cryptomator/domain/usecases/vault/ReloadVault.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.cryptomator.domain.usecases.vault; - -import org.cryptomator.domain.Vault; -import org.cryptomator.domain.exception.BackendException; -import org.cryptomator.domain.repository.VaultRepository; -import org.cryptomator.generator.Parameter; -import org.cryptomator.generator.UseCase; - -@UseCase -class ReloadVault { - - private final VaultRepository vaultRepository; - private final Vault vault; - - public ReloadVault(VaultRepository vaultRepository, @Parameter Vault vault) { - this.vaultRepository = vaultRepository; - this.vault = vault; - } - - public Vault execute() throws BackendException { - return vaultRepository.load(vault.getId()); - } - -} diff --git a/presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt b/presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt index 79082205..fffe2809 100644 --- a/presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt +++ b/presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt @@ -6,8 +6,8 @@ import android.content.Intent import android.net.Uri import android.provider.DocumentsContract import android.widget.Toast -import org.cryptomator.data.cloud.crypto.CryptoCloud import org.cryptomator.data.cloud.crypto.CryptoFolder +import org.cryptomator.domain.Cloud import org.cryptomator.domain.CloudFile import org.cryptomator.domain.CloudFolder import org.cryptomator.domain.CloudNode @@ -23,6 +23,7 @@ import org.cryptomator.domain.usecases.CloudFolderRecursiveListing import org.cryptomator.domain.usecases.CloudNodeRecursiveListing import org.cryptomator.domain.usecases.CopyDataUseCase import org.cryptomator.domain.usecases.DownloadFile +import org.cryptomator.domain.usecases.GetDecryptedCloudForVaultUseCase import org.cryptomator.domain.usecases.ResultRenamed import org.cryptomator.domain.usecases.cloud.CreateFolderUseCase import org.cryptomator.domain.usecases.cloud.DeleteNodesUseCase @@ -110,6 +111,7 @@ class BrowseFilesPresenter @Inject constructor( // private val moveFilesUseCase: MoveFilesUseCase, // private val moveFoldersUseCase: MoveFoldersUseCase, // private val getCloudListRecursiveUseCase: GetCloudListRecursiveUseCase, // + private val getDecryptedCloudForVaultUseCase: GetDecryptedCloudForVaultUseCase, // private val contentResolverUtil: ContentResolverUtil, // private val addExistingVaultWorkflow: AddExistingVaultWorkflow, // private val createNewVaultWorkflow: CreateNewVaultWorkflow, // @@ -132,6 +134,8 @@ class BrowseFilesPresenter @Inject constructor( // private lateinit var existingFilesForUpload: MutableMap private lateinit var downloadFiles: MutableList + private var resumedAfterAuthentication = false + @InjectIntent lateinit var intent: BrowseFilesIntent @@ -206,6 +210,7 @@ class BrowseFilesPresenter @Inject constructor( // view?.showLoading(false) when { authenticationExceptionHandler.handleAuthenticationException(this@BrowseFilesPresenter, e, ActivityResultCallbacks.getCloudListAfterAuthentication(cloudFolderModel)) -> { + resumedAfterAuthentication = true return } e is EmptyDirFileException -> { @@ -225,24 +230,42 @@ class BrowseFilesPresenter @Inject constructor( // }) } - @Callback + @Callback(dispatchResultOkOnly = false) fun getCloudListAfterAuthentication(result: ActivityResult, cloudFolderModel: CloudFolderModel) { - val cloudModel = result.getSingleResult(CloudModel::class.java) - val cloudNode = cloudFolderModel.toCloudNode() - - val updatedCloud = if (cloudNode is CryptoFolder) { - CryptoCloud(Vault.aCopyOf(cloudFolderModel.vault()?.toVault()).withCloud(cloudModel.toCloud()).build()) + resumedAfterAuthentication = false + if(result.isResultOk) { + val cloudModel = result.getSingleResult(CloudModel::class.java) // FIXME update other vaults using this cloud as well + val cloudNode = cloudFolderModel.toCloudNode() + if (cloudNode is CryptoFolder) { + updatedDecryptedCloudFor(Vault.aCopyOf(cloudFolderModel.vault()!!.toVault()).withCloud(cloudModel.toCloud()).build(), cloudFolderModel) + } else { + updatePlaintextCloud(cloudFolderModel, cloudModel) + } } else { - cloudModel.toCloud() + Timber.tag("BrowseFilesPresenter").e("Authentication failed") } + } - cloudNode.withCloud(updatedCloud)?.let { + private fun updatePlaintextCloud(cloudFolderModel: CloudFolderModel, updatedCloud: CloudModel) { + cloudFolderModel.toCloudNode().withCloud(updatedCloud.toCloud())?.let { val folder = cloudFolderModelMapper.toModel(it) view?.updateActiveFolderDueToAuthenticationProblem(folder) getCloudList(folder) } ?: throw FatalBackendException("cloudFolderModel with updated Cloud shouldn't be null") } + private fun updatedDecryptedCloudFor(vault: Vault, cloudFolderModel: CloudFolderModel) { + getDecryptedCloudForVaultUseCase // + .withVault(vault) // + .run(object : DefaultResultHandler() { + override fun onSuccess(cloud: Cloud) { + val folder = cloudFolderModelMapper.toModel(cloudFolderModel.toCloudNode().withCloud(cloud)!!) + view?.updateActiveFolderDueToAuthenticationProblem(folder) + getCloudList(folder) + } + }) + } + fun onCreateFolderPressed(cloudFolder: CloudFolderModel, folderName: String?) { createFolderUseCase // .withParent(cloudFolder.toCloudNode()) // @@ -1128,7 +1151,9 @@ class BrowseFilesPresenter @Inject constructor( // } fun onFolderReloadContent(folder: CloudFolderModel) { - getCloudList(folder) + if(!resumedAfterAuthentication) { + getCloudList(folder) + } } fun onExportFolderClicked(cloudFolder: CloudFolderModel, exportTriggeredByUser: ExportOperation) { @@ -1262,6 +1287,7 @@ class BrowseFilesPresenter @Inject constructor( // companion object { const val OPEN_FILE_FINISHED = 12 + val EXPORT_AFTER_APP_CHOOSER: ExportOperation = object : ExportOperation { override fun export(presenter: BrowseFilesPresenter, downloadFiles: List) { presenter.copyFile(downloadFiles) @@ -1285,7 +1311,8 @@ class BrowseFilesPresenter @Inject constructor( // renameFolderUseCase, // copyDataUseCase, // moveFilesUseCase, // - moveFoldersUseCase + moveFoldersUseCase, // + getDecryptedCloudForVaultUseCase ) this.authenticationExceptionHandler = authenticationExceptionHandler }