mirror of
https://github.com/bitwarden/android.git
synced 2024-11-27 12:00:19 +03:00
BIT-1165: Log out the current user after maximum invalid unlock attempts (#642)
This commit is contained in:
parent
74fac97257
commit
61e914f8ac
4 changed files with 294 additions and 28 deletions
|
@ -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(
|
||||
|
|
|
@ -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()
|
||||
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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<UserState?>(DEFAULT_USER_STATE)
|
||||
private val environmentRepository = FakeEnvironmentRepository()
|
||||
private val authRepository = mockk<AuthRepository>() {
|
||||
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<VaultUnlockResult>()
|
||||
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<VaultUnlockResult>()
|
||||
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),
|
||||
)
|
||||
|
|
Loading…
Reference in a new issue