mirror of
https://github.com/bitwarden/android.git
synced 2024-10-31 07:05:35 +03:00
[PM-13315] Prevent account switching during FIDO 2 unlock (#4054)
This commit is contained in:
parent
56ad1ef05b
commit
f73ce842fc
4 changed files with 208 additions and 5 deletions
|
@ -12,6 +12,8 @@ import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
|
|||
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionRequest
|
||||
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsRequest
|
||||
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance
|
||||
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
|
||||
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
|
||||
import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult
|
||||
|
@ -42,12 +44,13 @@ private const val KEY_STATE = "state"
|
|||
/**
|
||||
* Manages application state for the initial vault unlock screen.
|
||||
*/
|
||||
@Suppress("TooManyFunctions")
|
||||
@Suppress("TooManyFunctions", "LongParameterList")
|
||||
@HiltViewModel
|
||||
class VaultUnlockViewModel @Inject constructor(
|
||||
private val authRepository: AuthRepository,
|
||||
private val vaultRepo: VaultRepository,
|
||||
private val biometricsEncryptionManager: BiometricsEncryptionManager,
|
||||
private val specialCircumstanceManager: SpecialCircumstanceManager,
|
||||
private val fido2CredentialManager: Fido2CredentialManager,
|
||||
environmentRepo: EnvironmentRepository,
|
||||
savedStateHandle: SavedStateHandle,
|
||||
|
@ -68,6 +71,11 @@ class VaultUnlockViewModel @Inject constructor(
|
|||
// There is no valid way to unlock this app.
|
||||
authRepository.logout()
|
||||
}
|
||||
val specialCircumstance = specialCircumstanceManager.specialCircumstance
|
||||
val showAccountMenu =
|
||||
VaultUnlockArgs(savedStateHandle).unlockType == UnlockType.STANDARD &&
|
||||
(specialCircumstance !is SpecialCircumstance.Fido2GetCredentials &&
|
||||
specialCircumstance !is SpecialCircumstance.Fido2Assertion)
|
||||
VaultUnlockState(
|
||||
accountSummaries = accountSummaries,
|
||||
avatarColorString = activeAccountSummary.avatarColorHex,
|
||||
|
@ -79,7 +87,7 @@ class VaultUnlockViewModel @Inject constructor(
|
|||
input = "",
|
||||
isBiometricEnabled = activeAccount.isBiometricsEnabled,
|
||||
isBiometricsValid = isBiometricsValid,
|
||||
showAccountMenu = VaultUnlockArgs(savedStateHandle).unlockType == UnlockType.STANDARD,
|
||||
showAccountMenu = showAccountMenu,
|
||||
showBiometricInvalidatedMessage = false,
|
||||
vaultUnlockType = vaultUnlockType,
|
||||
userId = userState.activeUserId,
|
||||
|
@ -328,6 +336,16 @@ class VaultUnlockViewModel @Inject constructor(
|
|||
// out.
|
||||
val userState = action.userState ?: return
|
||||
|
||||
// If the Vault is being unlocked for a FIDO 2 request, make sure we're unlocking the
|
||||
// correct Vault
|
||||
state.fido2RequestUserId
|
||||
?.let { fido2RequestUserId ->
|
||||
// If the current Vault is not the selected Vault, switch accounts.
|
||||
if (userState.activeUserId != fido2RequestUserId) {
|
||||
authRepository.switchAccount(fido2RequestUserId)
|
||||
return
|
||||
}
|
||||
}
|
||||
// If the Vault is already unlocked, do nothing.
|
||||
if (userState.activeAccount.isVaultUnlocked) return
|
||||
// If the user state has changed to add a new account, do nothing.
|
||||
|
@ -402,6 +420,20 @@ data class VaultUnlockState(
|
|||
*/
|
||||
val showKeyboard: Boolean get() = !showBiometricLogin && !hideInput
|
||||
|
||||
/**
|
||||
* Indicates if the vault is being unlocked as a result of receiving a FIDO 2 request.
|
||||
*/
|
||||
val isUnlockingForFido2Request: Boolean
|
||||
get() = fido2GetCredentialsRequest != null ||
|
||||
fido2CredentialAssertionRequest != null
|
||||
|
||||
/**
|
||||
* Returns the user ID present in the current FIDO 2 request, or null when no FIDO 2 request is
|
||||
* present.
|
||||
*/
|
||||
val fido2RequestUserId: String?
|
||||
get() = fido2GetCredentialsRequest?.userId ?: fido2CredentialAssertionRequest?.userId
|
||||
|
||||
/**
|
||||
* Represents the various dialogs the vault unlock screen can display.
|
||||
*/
|
||||
|
|
|
@ -2,9 +2,12 @@ package com.x8bit.bitwarden.data.autofill.fido2.model
|
|||
|
||||
import android.content.pm.SigningInfo
|
||||
|
||||
fun createMockFido2CredentialAssertionRequest(number: Int = 1): Fido2CredentialAssertionRequest =
|
||||
fun createMockFido2CredentialAssertionRequest(
|
||||
number: Int = 1,
|
||||
userId: String = "mockUserId-$number",
|
||||
): Fido2CredentialAssertionRequest =
|
||||
Fido2CredentialAssertionRequest(
|
||||
userId = "mockUserId-$number",
|
||||
userId = userId,
|
||||
cipherId = "mockCipherId-$number",
|
||||
credentialId = "mockCredentialId-$number",
|
||||
requestJson = "mockRequestJson-$number",
|
||||
|
|
|
@ -5,12 +5,13 @@ import android.os.Bundle
|
|||
|
||||
fun createMockFido2GetCredentialsRequest(
|
||||
number: Int,
|
||||
userId: String = "mockUserId-$number",
|
||||
signingInfo: SigningInfo = SigningInfo(),
|
||||
origin: String? = null,
|
||||
): Fido2GetCredentialsRequest = Fido2GetCredentialsRequest(
|
||||
candidateQueryData = Bundle(),
|
||||
id = "mockId-$number",
|
||||
userId = "mockUserId-$number",
|
||||
userId = userId,
|
||||
requestJson = "requestJson-$number",
|
||||
clientDataHash = null,
|
||||
packageName = "mockPackageName-$number",
|
||||
|
|
|
@ -14,6 +14,8 @@ import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
|
|||
import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CredentialAssertionRequest
|
||||
import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2GetCredentialsRequest
|
||||
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance
|
||||
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
|
||||
|
@ -38,6 +40,7 @@ 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.Assertions.assertFalse
|
||||
import org.junit.jupiter.api.Test
|
||||
import javax.crypto.Cipher
|
||||
|
||||
|
@ -77,6 +80,9 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
|
|||
every { isUserVerified } returns true
|
||||
every { isUserVerified = any() } just runs
|
||||
}
|
||||
private val specialCircumstanceManager: SpecialCircumstanceManager = mockk {
|
||||
every { specialCircumstance } returns null
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `on init with biometrics enabled and valid should emit PromptForBiometrics`() = runTest {
|
||||
|
@ -176,6 +182,36 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
|
|||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `showAccountMenu should be true when unlockType is not STANDARD`() {
|
||||
val viewModel = createViewModel(unlockType = UnlockType.TDE)
|
||||
assertFalse(viewModel.stateFlow.value.showAccountMenu)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `showAccountMenu should be false when unlocking for FIDO 2 credential discovery`() {
|
||||
every {
|
||||
specialCircumstanceManager.specialCircumstance
|
||||
} returns SpecialCircumstance.Fido2GetCredentials(
|
||||
createMockFido2GetCredentialsRequest(number = 1),
|
||||
)
|
||||
val viewModel = createViewModel()
|
||||
|
||||
assertFalse(viewModel.stateFlow.value.showAccountMenu)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `showAccountMenu should be false when unlocking for FIDO 2 credential authentication`() {
|
||||
every {
|
||||
specialCircumstanceManager.specialCircumstance
|
||||
} returns SpecialCircumstance.Fido2Assertion(
|
||||
createMockFido2CredentialAssertionRequest(number = 1),
|
||||
)
|
||||
val viewModel = createViewModel()
|
||||
|
||||
assertFalse(viewModel.stateFlow.value.showAccountMenu)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `UserState updates with a null value should do nothing`() {
|
||||
val viewModel = createViewModel()
|
||||
|
@ -318,6 +354,136 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
|
|||
)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `UserState updates with a FIDO2 GetCredentialsRequest should switch accounts when the requested user is not the active user`() {
|
||||
val mockFido2GetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1)
|
||||
val initialState = DEFAULT_STATE.copy(
|
||||
fido2GetCredentialsRequest = mockFido2GetCredentialsRequest,
|
||||
accountSummaries = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false)
|
||||
.toAccountSummary(isActive = true),
|
||||
),
|
||||
)
|
||||
|
||||
val viewModel = createViewModel(state = initialState)
|
||||
|
||||
assertEquals(
|
||||
initialState,
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
|
||||
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
|
||||
accounts = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false),
|
||||
),
|
||||
)
|
||||
|
||||
verify {
|
||||
authRepository.switchAccount(mockFido2GetCredentialsRequest.userId)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `UserState updates with a FIDO2 GetCredentialsRequest should not switch accounts when the requested user is the active user`() {
|
||||
val mockFido2GetCredentialsRequest = createMockFido2GetCredentialsRequest(
|
||||
number = 1,
|
||||
userId = DEFAULT_USER_STATE.activeUserId,
|
||||
)
|
||||
val initialState = DEFAULT_STATE.copy(
|
||||
fido2GetCredentialsRequest = mockFido2GetCredentialsRequest,
|
||||
accountSummaries = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false)
|
||||
.toAccountSummary(isActive = true),
|
||||
),
|
||||
userId = mockFido2GetCredentialsRequest.userId,
|
||||
)
|
||||
|
||||
val viewModel = createViewModel(state = initialState)
|
||||
|
||||
assertEquals(
|
||||
initialState,
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
|
||||
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
|
||||
accounts = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false),
|
||||
),
|
||||
)
|
||||
|
||||
verify(exactly = 0) {
|
||||
authRepository.switchAccount(any())
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `UserState updates with a FIDO2 CredentialAssertionRequest should switch accounts when the requested user is not the active user`() {
|
||||
val mockFido2CredentialAssertionRequest =
|
||||
createMockFido2CredentialAssertionRequest(number = 1)
|
||||
val initialState = DEFAULT_STATE.copy(
|
||||
fido2CredentialAssertionRequest = mockFido2CredentialAssertionRequest,
|
||||
accountSummaries = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false)
|
||||
.toAccountSummary(isActive = true),
|
||||
),
|
||||
)
|
||||
|
||||
val viewModel = createViewModel(state = initialState)
|
||||
|
||||
assertEquals(
|
||||
initialState,
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
|
||||
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
|
||||
accounts = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false),
|
||||
),
|
||||
)
|
||||
|
||||
verify {
|
||||
authRepository.switchAccount(mockFido2CredentialAssertionRequest.userId)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `UserState updates with a FIDO2 CredentialAssertionRequest should not switch accounts when the requested user is the active user`() {
|
||||
val mockFido2CredentialAssertionRequest =
|
||||
createMockFido2CredentialAssertionRequest(
|
||||
number = 1,
|
||||
userId = DEFAULT_USER_STATE.activeUserId,
|
||||
)
|
||||
val initialState = DEFAULT_STATE.copy(
|
||||
fido2CredentialAssertionRequest = mockFido2CredentialAssertionRequest,
|
||||
accountSummaries = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false)
|
||||
.toAccountSummary(isActive = true),
|
||||
),
|
||||
userId = mockFido2CredentialAssertionRequest.userId,
|
||||
)
|
||||
|
||||
val viewModel = createViewModel(state = initialState)
|
||||
|
||||
assertEquals(
|
||||
initialState,
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
|
||||
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
|
||||
accounts = listOf(
|
||||
DEFAULT_ACCOUNT.copy(isVaultUnlocked = false),
|
||||
),
|
||||
)
|
||||
|
||||
verify(exactly = 0) {
|
||||
authRepository.switchAccount(any())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `on BiometricsUnlockClick should emit PromptForBiometrics when cipher is non-null`() =
|
||||
runTest {
|
||||
|
@ -1078,6 +1244,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
|
|||
environmentRepo = environmentRepo,
|
||||
biometricsEncryptionManager = biometricsEncryptionManager,
|
||||
fido2CredentialManager = fido2CredentialManager,
|
||||
specialCircumstanceManager = specialCircumstanceManager,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue