PM-9684: Verify with master password on add edit view (#3586)

This commit is contained in:
Shannon Draeker 2024-07-22 15:20:00 -06:00 committed by GitHub
parent 62154f5261
commit 2475bf5a41
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 426 additions and 5 deletions

View file

@ -52,4 +52,9 @@ interface Fido2CredentialManager {
fido2CredentialRequest: Fido2CredentialRequest,
selectedCipherView: CipherView,
): Fido2RegisterCredentialResult
/**
* Whether or not the user has authentication attempts remaining.
*/
fun hasAuthenticationAttemptsRemaining(): Boolean
}

View file

@ -218,4 +218,9 @@ class Fido2CredentialManagerImpl(
e.asFailure()
}
}
override fun hasAuthenticationAttemptsRemaining(): Boolean =
authenticationAttempts < MAX_AUTHENTICATION_ATTEMPTS
}
private const val MAX_AUTHENTICATION_ATTEMPTS = 5

View file

@ -35,6 +35,7 @@ import com.x8bit.bitwarden.ui.platform.components.content.BitwardenLoadingConten
import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenBasicDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenLoadingDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenMasterPasswordDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenOverwritePasskeyConfirmationDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
@ -176,6 +177,27 @@ fun VaultAddEditScreen(
)
}
},
onSubmitMasterPasswordFido2Verification = remember(viewModel) {
{
viewModel.trySendAction(
action = VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(it),
)
}
},
onDismissFido2PasswordVerification = remember(viewModel) {
{
viewModel.trySendAction(
action = VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
)
}
},
onRetryFido2PasswordVerification = remember(viewModel) {
{
viewModel.trySendAction(
action = VaultAddEditAction.Common.RetryFido2PasswordVerificationClick,
)
}
},
)
if (pendingDeleteCipher) {
@ -311,6 +333,9 @@ private fun VaultAddEditItemDialogs(
onAutofillDismissRequest: () -> Unit,
onFido2ErrorDismiss: () -> Unit,
onConfirmOverwriteExistingPasskey: () -> Unit,
onSubmitMasterPasswordFido2Verification: (password: String) -> Unit,
onDismissFido2PasswordVerification: () -> Unit,
onRetryFido2PasswordVerification: () -> Unit,
) {
when (dialogState) {
is VaultAddEditState.DialogState.Loading -> {
@ -356,6 +381,23 @@ private fun VaultAddEditItemDialogs(
)
}
is VaultAddEditState.DialogState.Fido2MasterPasswordPrompt -> {
BitwardenMasterPasswordDialog(
onConfirmClick = { onSubmitMasterPasswordFido2Verification(it) },
onDismissRequest = onDismissFido2PasswordVerification,
)
}
is VaultAddEditState.DialogState.Fido2MasterPasswordError -> {
BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown(
title = null,
message = R.string.invalid_master_password.asText(),
),
onDismissRequest = onRetryFido2PasswordVerification,
)
}
null -> Unit
}
}

View file

@ -8,6 +8,8 @@ import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialRequest
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult
@ -216,6 +218,7 @@ class VaultAddEditViewModel @Inject constructor(
//region Common Handlers
@Suppress("LongMethod")
private fun handleCommonActions(action: VaultAddEditAction.Common) {
when (action) {
is VaultAddEditAction.Common.CustomFieldValueChange -> {
@ -284,6 +287,18 @@ class VaultAddEditViewModel @Inject constructor(
VaultAddEditAction.Common.UserVerificationNotSupported -> {
handleUserVerificationNotSupported()
}
is VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit -> {
handleMasterPasswordFido2VerificationSubmit(action)
}
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick -> {
handleDismissFido2PasswordVerificationDialogClick()
}
VaultAddEditAction.Common.RetryFido2PasswordVerificationClick -> {
handleRetryFido2PasswordVerificationClick()
}
}
}
@ -556,6 +571,10 @@ class VaultAddEditViewModel @Inject constructor(
private fun handleUserVerificationSuccess() {
fido2CredentialManager.isUserVerified = true
getRequestAndRegisterCredential()
}
private fun getRequestAndRegisterCredential() =
specialCircumstanceManager
.specialCircumstance
?.toFido2RequestOrNull()
@ -568,7 +587,6 @@ class VaultAddEditViewModel @Inject constructor(
}
}
?: showFido2ErrorDialog()
}
private fun handleUserVerificationFail() {
fido2CredentialManager.isUserVerified = false
@ -597,9 +615,51 @@ class VaultAddEditViewModel @Inject constructor(
private fun handleUserVerificationNotSupported() {
fido2CredentialManager.isUserVerified = false
val activeAccount = authRepository
.userStateFlow
.value
?.activeAccount
?: run {
showFido2ErrorDialog()
return
}
if (activeAccount.vaultUnlockType == VaultUnlockType.PIN) {
// TODO: https://bitwarden.atlassian.net/browse/PM-9682
} else if (activeAccount.hasMasterPassword) {
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordPrompt)
}
} else {
// Prompt the user to set up a PIN for their account.
// TODO: https://bitwarden.atlassian.net/browse/PM-9681
}
}
private fun handleMasterPasswordFido2VerificationSubmit(
action: VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit,
) {
viewModelScope.launch {
val result = authRepository.validatePassword(action.password)
sendAction(
VaultAddEditAction.Internal.ValidateFido2PasswordResultReceive(
result = result,
),
)
}
}
private fun handleDismissFido2PasswordVerificationDialogClick() {
showFido2ErrorDialog()
}
private fun handleRetryFido2PasswordVerificationClick() {
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordPrompt)
}
}
private fun handleAddNewCustomFieldClick(
action: VaultAddEditAction.Common.AddNewCustomFieldClick,
) {
@ -1263,6 +1323,10 @@ class VaultAddEditViewModel @Inject constructor(
is VaultAddEditAction.Internal.Fido2RegisterCredentialResultReceive -> {
handleFido2RegisterCredentialResultReceive(action)
}
is VaultAddEditAction.Internal.ValidateFido2PasswordResultReceive -> {
handleValidateFido2PasswordResultReceive(action)
}
}
}
@ -1508,6 +1572,39 @@ class VaultAddEditViewModel @Inject constructor(
sendEvent(VaultAddEditEvent.CompleteFido2Registration(action.result))
}
private fun handleValidateFido2PasswordResultReceive(
action: VaultAddEditAction.Internal.ValidateFido2PasswordResultReceive,
) {
mutableStateFlow.update { it.copy(dialog = null) }
when (action.result) {
ValidatePasswordResult.Error -> {
showFido2ErrorDialog()
}
is ValidatePasswordResult.Success -> {
if (!action.result.isValid) {
fido2CredentialManager.authenticationAttempts += 1
if (fido2CredentialManager.hasAuthenticationAttemptsRemaining()) {
mutableStateFlow.update {
it.copy(
dialog = VaultAddEditState.DialogState.Fido2MasterPasswordError,
)
}
} else {
showFido2ErrorDialog()
}
return
}
fido2CredentialManager.isUserVerified = true
fido2CredentialManager.authenticationAttempts = 0
getRequestAndRegisterCredential()
}
}
}
//endregion Internal Type Handlers
//region Utility Functions
@ -2085,6 +2182,20 @@ data class VaultAddEditState(
*/
@Parcelize
data object OverwritePasskeyConfirmationPrompt : DialogState()
/**
* Displays a dialog to prompt the user for their master password as part of the FIDO 2
* user verification flow.
*/
@Parcelize
data object Fido2MasterPasswordPrompt : DialogState()
/**
* Displays a dialog to alert the user that their password for the FIDO 2 user
* verification flow was incorrect and to retry.
*/
@Parcelize
data object Fido2MasterPasswordError : DialogState()
}
}
@ -2351,6 +2462,23 @@ sealed class VaultAddEditAction {
* User verification cannot be performed with device biometrics or credentials.
*/
data object UserVerificationNotSupported : Common()
/**
* The user has clicked to submit the master password for FIDO 2 verification.
*/
data class MasterPasswordFido2VerificationSubmit(
val password: String,
) : Common()
/**
* The user has clicked to dismiss the FIDO 2 password verification dialog.
*/
data object DismissFido2PasswordVerificationDialogClick : Common()
/**
* The user has clicked to retry the FIDO 2 password verification.
*/
data object RetryFido2PasswordVerificationClick : Common()
}
/**
@ -2695,5 +2823,13 @@ sealed class VaultAddEditAction {
data class Fido2RegisterCredentialResultReceive(
val result: Fido2RegisterCredentialResult,
) : Internal()
/**
* Indicates that a result for verifying the user's master password as part of the FIDO 2
* user verification flow has been received.
*/
data class ValidateFido2PasswordResultReceive(
val result: ValidatePasswordResult,
) : Internal()
}
}

View file

@ -972,7 +972,7 @@ class VaultItemListingViewModel @Inject constructor(
is ValidatePasswordResult.Success -> {
if (!action.result.isValid) {
fido2CredentialManager.authenticationAttempts += 1
if (fido2CredentialManager.authenticationAttempts < 5) {
if (fido2CredentialManager.hasAuthenticationAttemptsRemaining()) {
mutableStateFlow.update {
it.copy(
dialogState = VaultItemListingState

View file

@ -36,6 +36,7 @@ import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertNotEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Assertions.assertTrue
@ -561,6 +562,19 @@ class Fido2CredentialManagerTest {
result is Fido2RegisterCredentialResult.Error,
)
}
@Suppress("MaxLineLength")
@Test
fun `hasAuthenticationAttemptsRemaining returns true when authenticationAttempts is less than 5`() {
assertTrue(fido2CredentialManager.hasAuthenticationAttemptsRemaining())
}
@Suppress("MaxLineLength")
@Test
fun `hasAuthenticationAttemptsRemaining returns false when authenticationAttempts is greater than 5`() {
fido2CredentialManager.authenticationAttempts = 6
assertFalse(fido2CredentialManager.hasAuthenticationAttemptsRemaining())
}
}
private const val DEFAULT_APP_SIGNATURE = "0987654321ABCDEF"

View file

@ -219,6 +219,85 @@ class VaultAddEditScreenTest : BaseComposeTest() {
.assertIsDisplayed()
}
@Test
fun `fido2 master password prompt dialog should display based on state`() {
val dialogTitle = "Master password confirmation"
composeTestRule.onNode(isDialog()).assertDoesNotExist()
composeTestRule.onNodeWithText(dialogTitle).assertDoesNotExist()
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordPrompt)
}
composeTestRule
.onNodeWithText(dialogTitle)
.assertIsDisplayed()
.assert(hasAnyAncestor(isDialog()))
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
)
}
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
)
}
composeTestRule
.onAllNodesWithText(text = "Master password")
.filterToOne(hasAnyAncestor(isDialog()))
.performTextInput("password")
composeTestRule
.onAllNodesWithText(text = "Submit")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(
password = "password",
),
)
}
}
@Test
fun `fido2 master password error dialog should display based on state`() {
val dialogMessage = "Invalid master password. Try again."
composeTestRule.onNode(isDialog()).assertDoesNotExist()
composeTestRule.onNodeWithText(dialogMessage).assertDoesNotExist()
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordError)
}
composeTestRule
.onNodeWithText(dialogMessage)
.assertIsDisplayed()
.assert(hasAnyAncestor(isDialog()))
composeTestRule
.onAllNodesWithText(text = "Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.RetryFido2PasswordVerificationClick,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `clicking dismiss dialog on Fido2Error dialog should send Fido2ErrorDialogDismissed action`() {

View file

@ -14,6 +14,7 @@ import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult
import com.x8bit.bitwarden.data.auth.repository.model.Organization
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialRequest
@ -126,6 +127,9 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
private val fido2CredentialManager = mockk<Fido2CredentialManager> {
every { isUserVerified } returns false
every { isUserVerified = any() } just runs
every { authenticationAttempts } returns 0
every { authenticationAttempts = any() } just runs
every { hasAuthenticationAttemptsRemaining() } returns true
}
private val vaultRepository: VaultRepository = mockk {
every { vaultDataStateFlow } returns mutableVaultDataFlow
@ -3168,11 +3172,145 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `UserVerificationNotSupported should set isUserVerified to false and show Fido2ErrorDialog`() {
fun `UserVerificationNotSupported should display Fido2ErrorDialog when active account not found`() {
mutableUserStateFlow.value = null
viewModel.trySendAction(VaultAddEditAction.Common.UserVerificationNotSupported)
verify { fido2CredentialManager.isUserVerified = false }
assertEquals(
VaultAddEditState.DialogState.Fido2Error(R.string.passkey_operation_failed_because_user_could_not_be_verified.asText()),
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified.asText(),
),
viewModel.stateFlow.value.dialog,
)
}
@Suppress("MaxLineLength")
@Test
fun `UserVerificationNotSupported should display Fido2MasterPasswordPrompt when user has password but no pin`() {
viewModel.trySendAction(VaultAddEditAction.Common.UserVerificationNotSupported)
verify { fido2CredentialManager.isUserVerified = false }
assertEquals(
VaultAddEditState.DialogState.Fido2MasterPasswordPrompt,
viewModel.stateFlow.value.dialog,
)
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordFido2VerificationSubmit should display Fido2Error when password verification fails`() {
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(
password = password,
),
)
assertEquals(
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified
.asText(),
),
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePassword(password = password)
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordFido2VerificationSubmit should display Fido2MasterPasswordError when user has retries remaining`() {
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = false)
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(
password = password,
),
)
assertEquals(
VaultAddEditState.DialogState.Fido2MasterPasswordError,
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePassword(password = password)
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordFido2VerificationSubmit should display Fido2Error when user has no retries remaining`() {
val password = "password"
every { fido2CredentialManager.hasAuthenticationAttemptsRemaining() } returns false
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = false)
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(
password = password,
),
)
assertEquals(
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified
.asText(),
),
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePassword(password = password)
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordFido2VerificationSubmit should register credential when password authenticated successfully`() =
runTest {
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = true)
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(
password = password,
),
)
coVerify {
authRepository.validatePassword(password = password)
}
}
@Test
fun `DismissFido2PasswordVerificationDialogClick should display Fido2ErrorDialog`() {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
)
assertEquals(
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified
.asText(),
),
viewModel.stateFlow.value.dialog,
)
}
@Test
fun `RetryFido2PasswordVerificationClick should display Fido2MasterPasswordPrompt`() {
viewModel.trySendAction(VaultAddEditAction.Common.RetryFido2PasswordVerificationClick)
assertEquals(
VaultAddEditState.DialogState.Fido2MasterPasswordPrompt,
viewModel.stateFlow.value.dialog,
)
}
@ -3404,6 +3542,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
}
//region Helper functions
@Suppress("MaxLineLength")

View file

@ -146,6 +146,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
every { isUserVerified = any() } just runs
every { authenticationAttempts } returns 0
every { authenticationAttempts = any() } just runs
every { hasAuthenticationAttemptsRemaining() } returns true
}
private val organizationEventManager = mockk<OrganizationEventManager> {
@ -2471,7 +2472,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
val viewModel = createVaultItemListingViewModel()
val selectedCipherId = "selectedCipherId"
val password = "password"
every { fido2CredentialManager.authenticationAttempts } returns 5
every { fido2CredentialManager.hasAuthenticationAttemptsRemaining() } returns false
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = false)