diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt index c984b17d1..0011c2123 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt @@ -42,6 +42,11 @@ import kotlinx.coroutines.launch private const val SECONDS_PER_MINUTE = 60 private const val MILLISECONDS_PER_SECOND = 1000 +/** + * The number of times a user may fail to unlock before they are automatically logged out. + */ +private const val MAXIMUM_INVALID_UNLOCK_ATTEMPTS = 5 + /** * Primary implementation [VaultLockManager]. */ @@ -130,13 +135,19 @@ class VaultLockManagerImpl( } } .fold( - onFailure = { VaultUnlockResult.GenericError }, + onFailure = { + incrementInvalidUnlockCount(userId = userId) + VaultUnlockResult.GenericError + }, onSuccess = { initializeCryptoResult -> initializeCryptoResult .toVaultUnlockResult() .also { if (it is VaultUnlockResult.Success) { + clearInvalidUnlockCount(userId = userId) setVaultToUnlocked(userId = userId) + } else { + incrementInvalidUnlockCount(userId = userId) } } }, @@ -146,6 +157,31 @@ class VaultLockManagerImpl( .onCompletion { setVaultToNotUnlocking(userId = userId) } .first() + /** + * Increments the stored invalid unlock count for the given [userId] and automatically logs out + * if this new value is greater than [MAXIMUM_INVALID_UNLOCK_ATTEMPTS]. + */ + private fun incrementInvalidUnlockCount(userId: String) { + val previousInvalidUnlockAttempts = + authDiskSource.getInvalidUnlockAttempts(userId = userId) ?: 0 + val invalidUnlockAttempts = previousInvalidUnlockAttempts + 1 + authDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = invalidUnlockAttempts, + ) + + if (invalidUnlockAttempts >= MAXIMUM_INVALID_UNLOCK_ATTEMPTS) { + userLogoutManager.logout(userId) + } + } + + private fun clearInvalidUnlockCount(userId: String) { + authDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = null, + ) + } + private fun setVaultToUnlocked(userId: String) { mutableVaultStateStateFlow.update { it.copy( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt index 8def8d00d..cf7cd7ba4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt @@ -128,6 +128,7 @@ class VaultUnlockViewModel @Inject constructor( } private fun handleUnlockClick() { + val activeUserId = authRepository.activeUserId ?: return mutableStateFlow.update { it.copy(dialog = VaultUnlockState.VaultUnlockDialog.Loading) } viewModelScope.launch { val vaultUnlockResult = when (state.vaultUnlockType) { @@ -143,13 +144,25 @@ class VaultUnlockViewModel @Inject constructor( ) } } - sendAction(VaultUnlockAction.Internal.ReceiveVaultUnlockResult(vaultUnlockResult)) + sendAction( + VaultUnlockAction.Internal.ReceiveVaultUnlockResult( + userId = activeUserId, + vaultUnlockResult = vaultUnlockResult, + ), + ) } } private fun handleReceiveVaultUnlockResult( action: VaultUnlockAction.Internal.ReceiveVaultUnlockResult, ) { + if (action.userId != authRepository.activeUserId) { + // The active user has automatically switched before receiving the event. Ignore any + // results and just clear any loading dialog. + mutableStateFlow.update { it.copy(dialog = null) } + return + } + when (action.vaultUnlockResult) { VaultUnlockResult.AuthenticationError -> { mutableStateFlow.update { @@ -318,6 +331,7 @@ sealed class VaultUnlockAction { * Indicates a vault unlock result has been received. */ data class ReceiveVaultUnlockResult( + val userId: String, val vaultUnlockResult: VaultUnlockResult, ) : Internal() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt index 8d2f3fefe..40a805484 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt @@ -772,7 +772,7 @@ class VaultLockManagerTest { @Suppress("MaxLineLength") @Test - fun `unlockVault with initializeCrypto success for a Never VaultTimeout should return Success and save the auto-unlock key`() = + fun `unlockVault with initializeCrypto success for a Never VaultTimeout should return Success, save the auto-unlock key, and clear invalid unlock attempts`() = runTest { val userId = "userId" val kdf = MOCK_PROFILE.toSdkParams() @@ -813,10 +813,16 @@ class VaultLockManagerTest { vaultLockManager.vaultStateFlow.value, ) mutableVaultTimeoutStateFlow.value = VaultTimeout.Never - fakeAuthDiskSource.storeUserAutoUnlockKey( - userId = userId, - userAutoUnlockKey = null, - ) + fakeAuthDiskSource.apply { + storeUserAutoUnlockKey( + userId = userId, + userAutoUnlockKey = null, + ) + storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 4, + ) + } val result = vaultLockManager.unlockVault( userId = userId, @@ -839,10 +845,16 @@ class VaultLockManagerTest { vaultLockManager.vaultStateFlow.value, ) - fakeAuthDiskSource.assertUserAutoUnlockKey( - userId = userId, - userAutoUnlockKey = userAutoUnlockKey, - ) + fakeAuthDiskSource.apply { + assertUserAutoUnlockKey( + userId = userId, + userAutoUnlockKey = userAutoUnlockKey, + ) + assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = null, + ) + } coVerify(exactly = 1) { vaultSdkSource.initializeCrypto( userId = userId, @@ -870,7 +882,7 @@ class VaultLockManagerTest { @Suppress("MaxLineLength") @Test - fun `unlockVault with initializeCrypto authentication failure for users should return AuthenticationError`() = + fun `unlockVault with initializeCrypto authentication failure for users should return AuthenticationError and increment invalid unlock attempts`() = runTest { val userId = "userId" val kdf = MOCK_PROFILE.toSdkParams() @@ -901,6 +913,11 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 1, + ) + val result = vaultLockManager.unlockVault( userId = userId, kdf = kdf, @@ -921,6 +938,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 2, + ) coVerify(exactly = 1) { vaultSdkSource.initializeCrypto( userId = userId, @@ -939,7 +960,7 @@ class VaultLockManagerTest { @Suppress("MaxLineLength") @Test - fun `unlockVault with initializeCrypto authentication failure for orgs should return AuthenticationError`() = + fun `unlockVault with initializeCrypto authentication failure for orgs should return AuthenticationError and increment invalid unlock attempts`() = runTest { val userId = "userId" val kdf = MOCK_PROFILE.toSdkParams() @@ -976,6 +997,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 1, + ) val result = vaultLockManager.unlockVault( userId = userId, @@ -997,6 +1022,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 2, + ) coVerify(exactly = 1) { vaultSdkSource.initializeCrypto( userId = userId, @@ -1019,8 +1048,9 @@ class VaultLockManagerTest { } } + @Suppress("MaxLineLength") @Test - fun `unlockVault with initializeCrypto failure for users should return GenericError`() = + fun `unlockVault with initializeCrypto failure for users should return GenericError and increment invalid unlock attempts`() = runTest { val userId = "userId" val kdf = MOCK_PROFILE.toSdkParams() @@ -1050,6 +1080,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 1, + ) val result = vaultLockManager.unlockVault( userId = userId, @@ -1071,6 +1105,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 2, + ) coVerify(exactly = 1) { vaultSdkSource.initializeCrypto( userId = userId, @@ -1087,8 +1125,9 @@ class VaultLockManagerTest { } } + @Suppress("MaxLineLength") @Test - fun `unlockVault with initializeCrypto failure for orgs should return GenericError`() = + fun `unlockVault with initializeCrypto failure for orgs should return GenericError and increment invalid unlock attempts`() = runTest { val userId = "userId" val kdf = MOCK_PROFILE.toSdkParams() @@ -1124,6 +1163,10 @@ class VaultLockManagerTest { ), vaultLockManager.vaultStateFlow.value, ) + fakeAuthDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 1, + ) val result = vaultLockManager.unlockVault( userId = userId, @@ -1137,6 +1180,101 @@ class VaultLockManagerTest { organizationKeys = organizationKeys, ) + assertEquals(VaultUnlockResult.GenericError, result) + assertEquals( + VaultState( + unlockedVaultUserIds = emptySet(), + unlockingVaultUserIds = emptySet(), + ), + vaultLockManager.vaultStateFlow.value, + ) + fakeAuthDiskSource.assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 2, + ) + coVerify(exactly = 1) { + vaultSdkSource.initializeCrypto( + userId = userId, + request = InitUserCryptoRequest( + kdfParams = kdf, + email = email, + privateKey = privateKey, + method = InitUserCryptoMethod.Password( + password = masterPassword, + userKey = userKey, + ), + ), + ) + } + coVerify(exactly = 1) { + vaultSdkSource.initializeOrganizationCrypto( + userId = userId, + request = InitOrgCryptoRequest(organizationKeys = organizationKeys), + ) + } + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVault error when reaching the maximum number of invalid unlock attempts should log out the user`() = + runTest { + val userId = "userId" + val kdf = MOCK_PROFILE.toSdkParams() + val email = MOCK_PROFILE.email + val masterPassword = "drowssap" + val userKey = "12345" + val privateKey = "54321" + val organizationKeys = mapOf("orgId1" to "orgKey1") + coEvery { + vaultSdkSource.initializeCrypto( + userId = userId, + request = InitUserCryptoRequest( + kdfParams = kdf, + email = email, + privateKey = privateKey, + method = InitUserCryptoMethod.Password( + password = masterPassword, + userKey = userKey, + ), + ), + ) + } returns InitializeCryptoResult.Success.asSuccess() + coEvery { + vaultSdkSource.initializeOrganizationCrypto( + userId = userId, + request = InitOrgCryptoRequest(organizationKeys = organizationKeys), + ) + } returns Throwable("Fail").asFailure() + assertEquals( + VaultState( + unlockedVaultUserIds = emptySet(), + unlockingVaultUserIds = emptySet(), + ), + vaultLockManager.vaultStateFlow.value, + ) + fakeAuthDiskSource.storeInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 4, + ) + + val result = vaultLockManager.unlockVault( + userId = userId, + kdf = kdf, + email = email, + initUserCryptoMethod = InitUserCryptoMethod.Password( + password = masterPassword, + userKey = userKey, + ), + privateKey = privateKey, + organizationKeys = organizationKeys, + ) + + fakeAuthDiskSource.assertInvalidUnlockAttempts( + userId = userId, + invalidUnlockAttempts = 5, + ) + verify { userLogoutManager.logout(userId = userId) } + assertEquals(VaultUnlockResult.GenericError, result) assertEquals( VaultState( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt index caa47a210..f9e73b060 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt @@ -11,6 +11,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository +import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest @@ -24,6 +25,8 @@ import io.mockk.mockk import io.mockk.runs import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.update import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test @@ -33,6 +36,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(DEFAULT_USER_STATE) private val environmentRepository = FakeEnvironmentRepository() private val authRepository = mockk() { + every { activeUserId } answers { mutableUserStateFlow.value?.activeUserId } every { userStateFlow } returns mutableUserStateFlow every { specialCircumstance } returns null every { specialCircumstance = any() } just runs @@ -347,6 +351,43 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { } } + @Test + fun `on UnlockClick for password unlock should clear dialog when user has changed`() { + val password = "abcd1234" + val initialState = DEFAULT_STATE.copy( + input = password, + vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, + ) + val resultFlow = bufferedMutableSharedFlow() + val viewModel = createViewModel(state = initialState) + coEvery { + vaultRepository.unlockVaultWithMasterPasswordAndSync(password) + } coAnswers { resultFlow.first() } + + viewModel.trySendAction(VaultUnlockAction.UnlockClick) + assertEquals( + initialState.copy(dialog = VaultUnlockState.VaultUnlockDialog.Loading), + viewModel.stateFlow.value, + ) + + val updatedUserId = "updatedUserId" + mutableUserStateFlow.update { + it?.copy( + activeUserId = updatedUserId, + accounts = listOf(DEFAULT_ACCOUNT.copy(userId = updatedUserId)), + ) + } + resultFlow.tryEmit(VaultUnlockResult.GenericError) + + assertEquals( + initialState.copy(dialog = null), + viewModel.stateFlow.value, + ) + coVerify { + vaultRepository.unlockVaultWithMasterPasswordAndSync(password) + } + } + @Test fun `on UnlockClick for PIN unlock should display error dialog on AuthenticationError`() { val pin = "1234" @@ -447,6 +488,43 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { } } + @Test + fun `on UnlockClick for PIN unlock should clear dialog when user has changed`() { + val pin = "1234" + val initialState = DEFAULT_STATE.copy( + input = pin, + vaultUnlockType = VaultUnlockType.PIN, + ) + val resultFlow = bufferedMutableSharedFlow() + val viewModel = createViewModel(state = initialState) + coEvery { + vaultRepository.unlockVaultWithPinAndSync(pin) + } coAnswers { resultFlow.first() } + + viewModel.trySendAction(VaultUnlockAction.UnlockClick) + assertEquals( + initialState.copy(dialog = VaultUnlockState.VaultUnlockDialog.Loading), + viewModel.stateFlow.value, + ) + + val updatedUserId = "updatedUserId" + mutableUserStateFlow.update { + it?.copy( + activeUserId = updatedUserId, + accounts = listOf(DEFAULT_ACCOUNT.copy(userId = updatedUserId)), + ) + } + resultFlow.tryEmit(VaultUnlockResult.GenericError) + + assertEquals( + initialState.copy(dialog = null), + viewModel.stateFlow.value, + ) + coVerify { + vaultRepository.unlockVaultWithPinAndSync(pin) + } + } + private fun createViewModel( state: VaultUnlockState? = DEFAULT_STATE, environmentRepo: EnvironmentRepository = environmentRepository, @@ -481,19 +559,19 @@ private val DEFAULT_STATE: VaultUnlockState = VaultUnlockState( vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, ) +private val DEFAULT_ACCOUNT = UserState.Account( + userId = "activeUserId", + name = "Active User", + email = "active@bitwarden.com", + environment = Environment.Us, + avatarColorHex = "#aa00aa", + isPremium = true, + isLoggedIn = true, + isVaultUnlocked = true, + organizations = emptyList(), +) + private val DEFAULT_USER_STATE = UserState( activeUserId = "activeUserId", - accounts = listOf( - UserState.Account( - userId = "activeUserId", - name = "Active User", - email = "active@bitwarden.com", - environment = Environment.Us, - avatarColorHex = "#aa00aa", - isPremium = true, - isLoggedIn = true, - isVaultUnlocked = true, - organizations = emptyList(), - ), - ), + accounts = listOf(DEFAULT_ACCOUNT), )