diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt index 30d860ccd..016e6b9ff 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt @@ -147,6 +147,10 @@ interface VaultSdkSource { * * This should only be called after a successful call to [initializeCrypto] for the associated * user. + * + * Note that this function will always add a [CipherView.key] to a cipher if it is missing, + * it is important to ensure that any [CipherView] being encrypted is pushed to the cloud if + * it was previously missing a `key` to ensure synchronization and prevent data-loss. */ suspend fun encryptCipher( userId: String, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt index 0b91be027..1940cea87 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt @@ -6,6 +6,7 @@ import com.bitwarden.core.Cipher import com.bitwarden.core.CipherView import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.platform.util.asFailure +import com.x8bit.bitwarden.data.platform.util.asSuccess import com.x8bit.bitwarden.data.platform.util.flatMap import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource import com.x8bit.bitwarden.data.vault.datasource.network.model.AttachmentJsonRequest @@ -105,20 +106,24 @@ class CipherManagerImpl( cipherView: CipherView, ): DeleteCipherResult { val userId = activeUserId ?: return DeleteCipherResult.Error - return ciphersService - .softDeleteCipher(cipherId = cipherId) + return cipherView + .encryptCipherAndCheckForMigration(userId = userId, cipherId = cipherId) + .flatMap { cipher -> + ciphersService + .softDeleteCipher(cipherId = cipherId) + .flatMap { vaultSdkSource.decryptCipher(userId = userId, cipher = cipher) } + } + .flatMap { + vaultSdkSource.encryptCipher( + userId = userId, + cipherView = it.copy(deletedDate = clock.instant()), + ) + } .onSuccess { - vaultSdkSource - .encryptCipher( - userId = userId, - cipherView = cipherView.copy(deletedDate = clock.instant()), - ) - .onSuccess { cipher -> - vaultDiskSource.saveCipher( - userId = userId, - cipher = cipher.toEncryptedNetworkCipherResponse(), - ) - } + vaultDiskSource.saveCipher( + userId = userId, + cipher = it.toEncryptedNetworkCipherResponse(), + ) } .fold( onSuccess = { DeleteCipherResult.Success }, @@ -153,14 +158,13 @@ class CipherManagerImpl( attachmentId = attachmentId, ) .flatMap { - vaultSdkSource.encryptCipher( - userId = userId, - cipherView = cipherView.copy( + cipherView + .copy( attachments = cipherView.attachments?.mapNotNull { if (it.id == attachmentId) null else it }, - ), - ) + ) + .encryptCipherAndCheckForMigration(userId = userId, cipherId = cipherId) } .onSuccess { cipher -> vaultDiskSource.saveCipher( @@ -320,10 +324,10 @@ class CipherManagerImpl( fileName = fileName, key = null, ) - return vaultSdkSource - .encryptCipher( + return cipherView + .encryptCipherAndCheckForMigration( userId = userId, - cipherView = cipherView, + cipherId = requireNotNull(cipherView.id), ) .flatMap { cipher -> fileManager @@ -400,10 +404,10 @@ class CipherManagerImpl( ): Result { val userId = activeUserId ?: return IllegalStateException("No active user").asFailure() - val cipher = vaultSdkSource - .encryptCipher( + val cipher = cipherView + .encryptCipherAndCheckForMigration( userId = userId, - cipherView = cipherView, + cipherId = requireNotNull(cipherView.id), ) .fold( onSuccess = { it }, @@ -443,4 +447,36 @@ class CipherManagerImpl( .onFailure { fileManager.delete(encryptedFile) } .map { decryptedFile } } + + /** + * A helper method to check if the [CipherView] needs to be migrated when you encrypt it. + */ + private suspend fun CipherView.encryptCipherAndCheckForMigration( + userId: String, + cipherId: String, + ): Result = + if (this.key == null) { + vaultSdkSource + .encryptCipher(userId = userId, cipherView = this) + .flatMap { + ciphersService.updateCipher( + cipherId = cipherId, + body = it.toEncryptedNetworkCipher(), + ) + } + .flatMap { response -> + when (response) { + is UpdateCipherResponseJson.Invalid -> { + IllegalStateException(response.message).asFailure() + } + + is UpdateCipherResponseJson.Success -> { + vaultDiskSource.saveCipher(userId = userId, cipher = response.cipher) + response.cipher.toEncryptedSdkCipher().asSuccess() + } + } + } + } else { + vaultSdkSource.encryptCipher(userId = userId, cipherView = this) + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt index b96baa9db..3436448ad 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt @@ -34,6 +34,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.DownloadAttachmentResult import com.x8bit.bitwarden.data.vault.repository.model.RestoreCipherResult import com.x8bit.bitwarden.data.vault.repository.model.ShareCipherResult import com.x8bit.bitwarden.data.vault.repository.model.UpdateCipherResult +import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedNetworkCipher import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedNetworkCipherResponse import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkCipher import io.mockk.coEvery @@ -69,7 +70,7 @@ class CipherManagerTest { private val vaultDiskSource: VaultDiskSource = mockk() private val vaultSdkSource: VaultSdkSource = mockk() - private val cipherManager: CipherManagerImpl = CipherManagerImpl( + private val cipherManager: CipherManager = CipherManagerImpl( ciphersService = ciphersService, vaultDiskSource = vaultDiskSource, vaultSdkSource = vaultSdkSource, @@ -464,14 +465,20 @@ class CipherManagerTest { fun `softDeleteCipher with ciphersService softDeleteCipher failure should return DeleteCipherResult Error`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE + val userId = MOCK_USER_STATE.activeUserId val cipherId = "mockId-1" + val cipherView = createMockCipherView(number = 1) + val cipher = createMockSdkCipher(number = 1) + coEvery { + vaultSdkSource.encryptCipher(userId = userId, cipherView = cipherView) + } returns cipher.asSuccess() coEvery { ciphersService.softDeleteCipher(cipherId = cipherId) } returns Throwable("Fail").asFailure() val result = cipherManager.softDeleteCipher( cipherId = cipherId, - cipherView = createMockCipherView(number = 1), + cipherView = cipherView, ) assertEquals(DeleteCipherResult.Error, result) @@ -481,30 +488,31 @@ class CipherManagerTest { @Test fun `softDeleteCipher with ciphersService softDeleteCipher success should return DeleteCipherResult success`() = runTest { - mockkStatic(Cipher::toEncryptedNetworkCipherResponse) - every { - createMockSdkCipher(number = 1, clock = clock).toEncryptedNetworkCipherResponse() - } returns createMockCipher(number = 1) val fixedInstant = Instant.parse("2023-10-27T12:00:00Z") val userId = "mockId-1" val cipherId = "mockId-1" + val cipher = createMockSdkCipher(number = 1, clock = clock) + val cipherView = createMockCipherView(number = 1) + coEvery { + vaultSdkSource.encryptCipher(userId = userId, cipherView = cipherView) + } returns cipher.asSuccess() coEvery { vaultSdkSource.encryptCipher( userId = userId, - cipherView = createMockCipherView(number = 1).copy(deletedDate = fixedInstant), + cipherView = cipherView.copy(deletedDate = fixedInstant), ) - } returns createMockSdkCipher(number = 1, clock = clock).asSuccess() + } returns cipher.asSuccess() + coEvery { + vaultSdkSource.decryptCipher(userId = userId, cipher = cipher) + } returns cipherView.asSuccess() fakeAuthDiskSource.userState = MOCK_USER_STATE coEvery { ciphersService.softDeleteCipher(cipherId = cipherId) } returns Unit.asSuccess() coEvery { vaultDiskSource.saveCipher( userId = userId, - cipher = createMockCipher(number = 1), + cipher = cipher.toEncryptedNetworkCipherResponse(), ) } just runs - val cipherView = createMockCipherView(number = 1) - mockkStatic(Instant::class) - every { Instant.now() } returns fixedInstant val result = cipherManager.softDeleteCipher( cipherId = cipherId, @@ -512,8 +520,67 @@ class CipherManagerTest { ) assertEquals(DeleteCipherResult.Success, result) - unmockkStatic(Instant::class) - unmockkStatic(Cipher::toEncryptedNetworkCipherResponse) + } + + @Suppress("MaxLineLength") + @Test + fun `softDeleteCipher with cipher migration success should return DeleteCipherResult success`() = + runTest { + val fixedInstant = Instant.parse("2023-10-27T12:00:00Z") + val userId = "mockId-1" + val cipherId = "mockId-1" + val cipher = createMockSdkCipher(number = 1, clock = clock) + val cipherView = createMockCipherView(number = 1).copy(key = null) + val networkCipher = createMockCipher(number = 1).copy(key = null) + coEvery { + vaultSdkSource.encryptCipher(userId = userId, cipherView = cipherView) + } returns cipher.asSuccess() + coEvery { + ciphersService.updateCipher( + cipherId = cipherId, + body = cipher.toEncryptedNetworkCipher(), + ) + } returns UpdateCipherResponseJson.Success(networkCipher).asSuccess() + coEvery { + vaultDiskSource.saveCipher(userId = userId, cipher = networkCipher) + } just runs + coEvery { + vaultSdkSource.decryptCipher( + userId = userId, + cipher = networkCipher.toEncryptedSdkCipher(), + ) + } returns cipherView.asSuccess() + coEvery { + vaultSdkSource.encryptCipher( + userId = userId, + cipherView = cipherView.copy(deletedDate = fixedInstant), + ) + } returns cipher.asSuccess() + coEvery { + vaultSdkSource.decryptCipher(userId = userId, cipher = cipher) + } returns cipherView.asSuccess() + fakeAuthDiskSource.userState = MOCK_USER_STATE + coEvery { ciphersService.softDeleteCipher(cipherId = cipherId) } returns Unit.asSuccess() + coEvery { + vaultDiskSource.saveCipher( + userId = userId, + cipher = cipher.toEncryptedNetworkCipherResponse(), + ) + } just runs + + val result = cipherManager.softDeleteCipher( + cipherId = cipherId, + cipherView = cipherView, + ) + + assertEquals(DeleteCipherResult.Success, result) + coVerify(exactly = 1) { + ciphersService.updateCipher( + cipherId = cipherId, + body = cipher.toEncryptedNetworkCipher(), + ) + vaultDiskSource.saveCipher(userId = userId, cipher = networkCipher) + } } @Test