From ab0cfdfdc27832591b656df0283c4a437ffcddc2 Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Sun, 28 Jan 2024 08:32:33 -0600 Subject: [PATCH] Provide graceful fallbacks when there is no active user (#825) --- .../vault/repository/VaultRepositoryImpl.kt | 34 +-- .../vault/repository/VaultRepositoryTest.kt | 226 +++++++++++++++++- 2 files changed, 243 insertions(+), 17 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 b1c3ce4ed3..6baadbacf1 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 @@ -351,7 +351,9 @@ class VaultRepositoryImpl( @OptIn(ExperimentalCoroutinesApi::class) override fun getAuthCodesFlow(): StateFlow>> { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return MutableStateFlow( + DataState.Error(IllegalStateException("No active user"), null), + ) return vaultDataStateFlow .map { dataState -> dataState.map { vaultData -> @@ -392,7 +394,9 @@ class VaultRepositoryImpl( @OptIn(ExperimentalCoroutinesApi::class) override fun getAuthCodeFlow(cipherId: String): StateFlow> { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return MutableStateFlow( + DataState.Error(IllegalStateException("No active user"), null), + ) return getVaultItemStateFlow(cipherId) .flatMapLatest { cipherDataState -> val cipher = cipherDataState.data @@ -503,7 +507,7 @@ class VaultRepositoryImpl( ) override suspend fun createCipher(cipherView: CipherView): CreateCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return CreateCipherResult.Error return vaultSdkSource .encryptCipher( userId = userId, @@ -527,7 +531,7 @@ class VaultRepositoryImpl( } override suspend fun hardDeleteCipher(cipherId: String): DeleteCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return DeleteCipherResult.Error return ciphersService .hardDeleteCipher(cipherId) .onSuccess { vaultDiskSource.deleteCipher(userId, cipherId) } @@ -541,7 +545,7 @@ class VaultRepositoryImpl( cipherId: String, cipherView: CipherView, ): DeleteCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return DeleteCipherResult.Error return ciphersService .softDeleteCipher(cipherId) .fold( @@ -570,7 +574,7 @@ class VaultRepositoryImpl( attachmentId: String, cipherView: CipherView, ): DeleteAttachmentResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return DeleteAttachmentResult.Error return ciphersService .deleteCipherAttachment( cipherId = cipherId, @@ -603,7 +607,7 @@ class VaultRepositoryImpl( cipherId: String, cipherView: CipherView, ): RestoreCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return RestoreCipherResult.Error return ciphersService .restoreCipher(cipherId) .flatMap { @@ -630,7 +634,7 @@ class VaultRepositoryImpl( cipherId: String, cipherView: CipherView, ): UpdateCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return UpdateCipherResult.Error(null) return vaultSdkSource .encryptCipher( userId = userId, @@ -664,7 +668,7 @@ class VaultRepositoryImpl( cipherView: CipherView, collectionIds: List, ): ShareCipherResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return ShareCipherResult.Error(null) return vaultSdkSource .encryptCipher( userId = userId, @@ -695,7 +699,7 @@ class VaultRepositoryImpl( fileName: String, fileUri: Uri, ): CreateAttachmentResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return CreateAttachmentResult.Error val attachmentView = AttachmentView( id = null, url = null, @@ -757,7 +761,7 @@ class VaultRepositoryImpl( sendView: SendView, fileUri: Uri?, ): CreateSendResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return CreateSendResult.Error return vaultSdkSource .encryptSend( userId = userId, @@ -817,7 +821,7 @@ class VaultRepositoryImpl( sendId: String, sendView: SendView, ): UpdateSendResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return UpdateSendResult.Error(null) return vaultSdkSource .encryptSend( userId = userId, @@ -854,7 +858,7 @@ class VaultRepositoryImpl( } override suspend fun removePasswordSend(sendId: String): RemovePasswordSendResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return RemovePasswordSendResult.Error(null) return sendsService .removeSendPassword(sendId = sendId) .fold( @@ -882,7 +886,7 @@ class VaultRepositoryImpl( } override suspend fun deleteSend(sendId: String): DeleteSendResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return DeleteSendResult.Error return sendsService .deleteSend(sendId) .onSuccess { vaultDiskSource.deleteSend(userId, sendId) } @@ -896,7 +900,7 @@ class VaultRepositoryImpl( totpCode: String, time: DateTime, ): GenerateTotpResult { - val userId = requireNotNull(activeUserId) + val userId = activeUserId ?: return GenerateTotpResult.Error return vaultSdkSource.generateTotp( time = time, userId = userId, 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 fb560b53c2..3361959fd5 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 @@ -106,6 +106,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import java.net.UnknownHostException @@ -1624,6 +1625,19 @@ class VaultRepositoryTest { } } + @Test + fun `createCipher with no active user should return CreateCipherResult failure`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.createCipher(cipherView = mockk()) + + assertEquals( + CreateCipherResult.Error, + result, + ) + } + @Test fun `createCipher with encryptCipher failure should return CreateCipherResult failure`() = runTest { @@ -1701,6 +1715,22 @@ class VaultRepositoryTest { ) } + @Test + fun `updateCipher with no active user should return UpdateCipherResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.updateCipher( + cipherId = "cipherId", + cipherView = mockk(), + ) + + assertEquals( + UpdateCipherResult.Error(null), + result, + ) + } + @Test fun `updateCipher with encryptCipher failure should return UpdateCipherResult failure`() = runTest { @@ -1829,6 +1859,21 @@ class VaultRepositoryTest { assertEquals(UpdateCipherResult.Success, result) } + @Test + fun `hardDeleteCipher with no active user should return DeleteCipherResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.hardDeleteCipher( + cipherId = "cipherId", + ) + + assertEquals( + DeleteCipherResult.Error, + result, + ) + } + @Suppress("MaxLineLength") @Test fun `hardDeleteCipher with ciphersService hardDeleteCipher failure should return DeleteCipherResult Error`() = @@ -1859,6 +1904,22 @@ class VaultRepositoryTest { assertEquals(DeleteCipherResult.Success, result) } + @Test + fun `softDeleteCipher with no active user should return DeleteCipherResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.softDeleteCipher( + cipherId = "cipherId", + cipherView = mockk(), + ) + + assertEquals( + DeleteCipherResult.Error, + result, + ) + } + @Suppress("MaxLineLength") @Test fun `softDeleteCipher with ciphersService softDeleteCipher failure should return DeleteCipherResult Error`() = @@ -1919,6 +1980,23 @@ class VaultRepositoryTest { unmockkStatic(Cipher::toEncryptedNetworkCipherResponse) } + @Test + fun `deleteCipherAttachment with no active user should return DeleteAttachmentResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.deleteCipherAttachment( + cipherId = "cipherId", + attachmentId = "attachmentId", + cipherView = mockk(), + ) + + assertEquals( + DeleteAttachmentResult.Error, + result, + ) + } + @Suppress("MaxLineLength") @Test fun `deleteCipherAttachment with ciphersService deleteCipherAttachment failure should return DeleteAttachmentResult Error`() = @@ -1990,6 +2068,22 @@ class VaultRepositoryTest { unmockkStatic(Cipher::toEncryptedNetworkCipherResponse) } + @Test + fun `restoreCipher with no active user should return RestoreCipherResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.restoreCipher( + cipherId = "cipherId", + cipherView = mockk(), + ) + + assertEquals( + RestoreCipherResult.Error, + result, + ) + } + @Suppress("MaxLineLength") @Test fun `restoreCipher with ciphersService restoreCipher failure should return RestoreCipherResult Error`() = @@ -2048,6 +2142,22 @@ class VaultRepositoryTest { assertEquals(RestoreCipherResult.Success, result) } + @Test + fun `createSend with no active user should return CreateSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.createSend( + sendView = mockk(), + fileUri = mockk(), + ) + + assertEquals( + CreateSendResult.Error, + result, + ) + } + @Test fun `createSend with encryptSend failure should return CreateSendResult failure`() = runTest { @@ -2237,6 +2347,22 @@ class VaultRepositoryTest { assertEquals(CreateSendResult.Success(mockSendViewResult), result) } + @Test + fun `updateSend with no active user should return UpdateSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.updateSend( + sendId = "sendId", + sendView = mockk(), + ) + + assertEquals( + UpdateSendResult.Error(null), + result, + ) + } + @Test fun `updateSend with encryptSend failure should return UpdateSendResult failure`() = runTest { @@ -2390,6 +2516,21 @@ class VaultRepositoryTest { assertEquals(UpdateSendResult.Success(mockSendViewResult), result) } + @Test + fun `removePasswordSend with no active user should return RemovePasswordSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.removePasswordSend( + sendId = "sendId", + ) + + assertEquals( + RemovePasswordSendResult.Error(null), + result, + ) + } + @Test @Suppress("MaxLineLength") fun `removePasswordSend with sendsService removeSendPassword Error should return RemovePasswordSendResult Error`() = @@ -2448,6 +2589,21 @@ class VaultRepositoryTest { assertEquals(RemovePasswordSendResult.Success(mockSendView), result) } + @Test + fun `deleteSend with no active user should return DeleteSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.deleteSend( + sendId = "sendId", + ) + + assertEquals( + DeleteSendResult.Error, + result, + ) + } + @Test fun `deleteSend with sendsService deleteSend failure should return DeleteSendResult Error`() = runTest { @@ -2476,6 +2632,23 @@ class VaultRepositoryTest { assertEquals(DeleteSendResult.Success, result) } + @Test + fun `shareCipher with no active user should return ShareCipherResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.shareCipher( + cipherId = "cipherId", + cipherView = mockk(), + collectionIds = emptyList(), + ) + + assertEquals( + ShareCipherResult.Error(null), + result, + ) + } + @Test @Suppress("MaxLineLength") fun `shareCipher with cipherService shareCipher success should return ShareCipherResultSuccess`() = @@ -2581,6 +2754,25 @@ class VaultRepositoryTest { ) } + @Test + fun `createAttachment with no active user should return CreateAttachmentResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.createAttachment( + cipherId = "cipherId", + cipherView = mockk(), + fileSizeBytes = "mockFileSize", + fileName = "mockFileName", + fileUri = mockk(), + ) + + assertEquals( + CreateAttachmentResult.Error, + result, + ) + } + @Suppress("MaxLineLength") @Test fun `createAttachment with encryptCipher failure should return CreateAttachmentResult Error`() = @@ -2907,6 +3099,22 @@ class VaultRepositoryTest { assertEquals(CreateAttachmentResult.Success(mockCipherView), result) } + @Test + fun `generateTotp with no active user should return GenerateTotpResult Error`() = + runTest { + fakeAuthDiskSource.userState = null + + val result = vaultRepository.generateTotp( + totpCode = "totpCode", + time = DateTime.now(), + ) + + assertEquals( + GenerateTotpResult.Error, + result, + ) + } + @Test fun `generateTotp should return a success result on getting a code`() = runTest { val totpResponse = TotpResponse("Testcode", 30u) @@ -2931,7 +3139,14 @@ class VaultRepositoryTest { @Suppress("MaxLineLength") @Test - fun `getVerificationCodeFlow for a single cipher should update data state when state changes`() = + fun `getAuthCodeFlow with no active user should emit an error`() = runTest { + fakeAuthDiskSource.userState = null + assertTrue(vaultRepository.getAuthCodeFlow(cipherId = "cipherId").value is DataState.Error) + } + + @Suppress("MaxLineLength") + @Test + fun `getAuthCodeFlow for a single cipher should update data state when state changes`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val userId = "mockId-1" @@ -2989,8 +3204,15 @@ class VaultRepositoryTest { } } + @Suppress("MaxLineLength") @Test - fun `getVerificationCodesFlow should update data state when state changes`() = runTest { + fun `getAuthCodesFlow with no active user should emit an error`() = runTest { + fakeAuthDiskSource.userState = null + assertTrue(vaultRepository.getAuthCodesFlow().value is DataState.Error) + } + + @Test + fun `getAuthCodesFlow should update data state when state changes`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE val userId = "mockId-1"