From 21fed995c0362148da4615abcf81854ff9b2b1a8 Mon Sep 17 00:00:00 2001 From: David Perez Date: Mon, 6 May 2024 08:34:53 -0500 Subject: [PATCH] Clear vault data in memory when the vault is locked (#1339) --- .../vault/repository/VaultRepositoryImpl.kt | 15 ++- .../vault/repository/VaultRepositoryTest.kt | 116 ++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 772844ac6..dd191b327 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -107,6 +107,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.flatMapLatest @@ -114,6 +115,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn @@ -225,8 +227,17 @@ class VaultRepositoryImpl( get() = mutableSendDataStateFlow.asStateFlow() init { - authDiskSource - .userSwitchingChangesFlow + // Cancel any ongoing sync request and clear the vault data in memory every time + // the user switches or the vault is locked for the active user. + merge( + authDiskSource.userSwitchingChangesFlow, + vaultLockManager + .vaultUnlockDataStateFlow + .filter { vaultUnlockDataList -> + // Clear if the active user is not currently unlocking or unlocked + vaultUnlockDataList.none { it.userId == activeUserId } + }, + ) .onEach { syncJob.cancel() clearUnlockedData() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 3dcdaf2dd..543086869 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -375,6 +375,122 @@ class VaultRepositoryTest { } } + @Suppress("MaxLineLength") + @Test + fun `mutableVaultStateFlow should clear unlocked data when it does not contain the active user`() = + runTest { + fakeAuthDiskSource.userState = UserStateJson( + activeUserId = "mockId-1", + accounts = mapOf( + "mockId-1" to MOCK_ACCOUNT, + "mockId-2" to mockk(), + ), + ) + mutableVaultStateFlow.value = listOf( + VaultUnlockData(userId = "mockId-1", status = VaultUnlockData.Status.UNLOCKING), + VaultUnlockData(userId = "mockId-2", status = VaultUnlockData.Status.UNLOCKED), + ) + val userId = "mockId-1" + coEvery { + vaultSdkSource.decryptCipherList( + userId = userId, + cipherList = listOf(createMockSdkCipher(1, clock)), + ) + } returns listOf(createMockCipherView(number = 1)).asSuccess() + coEvery { + vaultSdkSource.decryptFolderList( + userId = userId, + folderList = listOf(createMockSdkFolder(1)), + ) + } returns listOf(createMockFolderView(number = 1)).asSuccess() + coEvery { + vaultSdkSource.decryptCollectionList( + userId = userId, + collectionList = listOf(createMockSdkCollection(1)), + ) + } returns listOf(createMockCollectionView(number = 1)).asSuccess() + coEvery { + vaultSdkSource.decryptSendList( + userId = userId, + sendList = listOf(createMockSdkSend(number = 1)), + ) + } returns listOf(createMockSendView(number = 1)).asSuccess() + val ciphersFlow = bufferedMutableSharedFlow>() + val collectionsFlow = bufferedMutableSharedFlow>() + val foldersFlow = bufferedMutableSharedFlow>() + val sendsFlow = bufferedMutableSharedFlow>() + val domainsFlow = bufferedMutableSharedFlow() + setupVaultDiskSourceFlows( + ciphersFlow = ciphersFlow, + collectionsFlow = collectionsFlow, + foldersFlow = foldersFlow, + sendsFlow = sendsFlow, + domainsFlow = domainsFlow, + ) + + turbineScope { + val ciphersStateFlow = vaultRepository.ciphersStateFlow.testIn(backgroundScope) + val collectionsStateFlow = + vaultRepository.collectionsStateFlow.testIn(backgroundScope) + val foldersStateFlow = vaultRepository.foldersStateFlow.testIn(backgroundScope) + val sendsStateFlow = vaultRepository.sendDataStateFlow.testIn(backgroundScope) + val domainsStateFlow = vaultRepository.domainsStateFlow.testIn(backgroundScope) + + assertEquals(DataState.Loading, ciphersStateFlow.awaitItem()) + assertEquals(DataState.Loading, collectionsStateFlow.awaitItem()) + assertEquals(DataState.Loading, foldersStateFlow.awaitItem()) + assertEquals(DataState.Loading, sendsStateFlow.awaitItem()) + assertEquals(DataState.Loading, domainsStateFlow.awaitItem()) + + ciphersFlow.tryEmit(listOf(createMockCipher(number = 1))) + collectionsFlow.tryEmit(listOf(createMockCollection(number = 1))) + foldersFlow.tryEmit(listOf(createMockFolder(number = 1))) + sendsFlow.tryEmit(listOf(createMockSend(number = 1))) + domainsFlow.tryEmit(createMockDomains(number = 1)) + + // No events received until unlocked + ciphersStateFlow.expectNoEvents() + collectionsStateFlow.expectNoEvents() + foldersStateFlow.expectNoEvents() + sendsStateFlow.expectNoEvents() + // Domains does not care about being unlocked + assertEquals( + DataState.Loaded(createMockDomainsData(number = 1)), + domainsStateFlow.awaitItem(), + ) + setVaultToUnlocked(userId = userId) + + assertEquals( + DataState.Loaded(listOf(createMockCipherView(number = 1))), + ciphersStateFlow.awaitItem(), + ) + assertEquals( + DataState.Loaded(listOf(createMockCollectionView(number = 1))), + collectionsStateFlow.awaitItem(), + ) + assertEquals( + DataState.Loaded(listOf(createMockFolderView(number = 1))), + foldersStateFlow.awaitItem(), + ) + assertEquals( + DataState.Loaded(SendData(listOf(createMockSendView(number = 1)))), + sendsStateFlow.awaitItem(), + ) + // Domain data has not changed + domainsStateFlow.expectNoEvents() + + mutableVaultStateFlow.value = listOf( + VaultUnlockData(userId = "mockId-2", status = VaultUnlockData.Status.UNLOCKED), + ) + + assertEquals(DataState.Loading, ciphersStateFlow.awaitItem()) + assertEquals(DataState.Loading, collectionsStateFlow.awaitItem()) + assertEquals(DataState.Loading, foldersStateFlow.awaitItem()) + assertEquals(DataState.Loading, sendsStateFlow.awaitItem()) + assertEquals(DataState.Loading, domainsStateFlow.awaitItem()) + } + } + @Test fun `ciphersStateFlow should emit decrypted list of ciphers when decryptCipherList succeeds`() = runTest {