PM-9682: Verify with PIN on add edit view (#3610)

This commit is contained in:
Shannon Draeker 2024-07-24 09:40:25 -06:00 committed by GitHub
parent b44a320dc8
commit b48837e13c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 388 additions and 54 deletions

View file

@ -30,7 +30,7 @@ import com.x8bit.bitwarden.ui.platform.components.field.BitwardenPasswordField
@OptIn(ExperimentalComposeUiApi::class)
@Composable
fun BitwardenPinDialog(
onConfirmClick: (masterPassword: String) -> Unit,
onConfirmClick: (pin: String) -> Unit,
onDismissRequest: () -> Unit,
) {
var pin by remember { mutableStateOf("") }

View file

@ -37,6 +37,7 @@ 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.BitwardenPinDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
@ -184,13 +185,6 @@ fun VaultAddEditScreen(
)
}
},
onDismissFido2PasswordVerification = remember(viewModel) {
{
viewModel.trySendAction(
action = VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
)
}
},
onRetryFido2PasswordVerification = remember(viewModel) {
{
viewModel.trySendAction(
@ -198,6 +192,27 @@ fun VaultAddEditScreen(
)
}
},
onSubmitPinFido2Verification = remember(viewModel) {
{
viewModel.trySendAction(
VaultAddEditAction.Common.PinFido2VerificationSubmit(it),
)
}
},
onRetryFido2PinVerification = remember(viewModel) {
{
viewModel.trySendAction(
VaultAddEditAction.Common.RetryFido2PinVerificationClick,
)
}
},
onDismissFido2Verification = remember(viewModel) {
{
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
}
},
)
if (pendingDeleteCipher) {
@ -326,6 +341,7 @@ fun VaultAddEditScreen(
}
}
@Suppress("LongMethod")
@Composable
private fun VaultAddEditItemDialogs(
dialogState: VaultAddEditState.DialogState?,
@ -334,8 +350,10 @@ private fun VaultAddEditItemDialogs(
onFido2ErrorDismiss: () -> Unit,
onConfirmOverwriteExistingPasskey: () -> Unit,
onSubmitMasterPasswordFido2Verification: (password: String) -> Unit,
onDismissFido2PasswordVerification: () -> Unit,
onRetryFido2PasswordVerification: () -> Unit,
onSubmitPinFido2Verification: (pin: String) -> Unit,
onRetryFido2PinVerification: () -> Unit,
onDismissFido2Verification: () -> Unit,
) {
when (dialogState) {
is VaultAddEditState.DialogState.Loading -> {
@ -383,8 +401,8 @@ private fun VaultAddEditItemDialogs(
is VaultAddEditState.DialogState.Fido2MasterPasswordPrompt -> {
BitwardenMasterPasswordDialog(
onConfirmClick = { onSubmitMasterPasswordFido2Verification(it) },
onDismissRequest = onDismissFido2PasswordVerification,
onConfirmClick = onSubmitMasterPasswordFido2Verification,
onDismissRequest = onDismissFido2Verification,
)
}
@ -398,6 +416,23 @@ private fun VaultAddEditItemDialogs(
)
}
is VaultAddEditState.DialogState.Fido2PinPrompt -> {
BitwardenPinDialog(
onConfirmClick = onSubmitPinFido2Verification,
onDismissRequest = onDismissFido2Verification,
)
}
is VaultAddEditState.DialogState.Fido2PinError -> {
BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown(
title = null,
message = R.string.invalid_pin.asText(),
),
onDismissRequest = onRetryFido2PinVerification,
)
}
null -> Unit
}
}

View file

@ -9,6 +9,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.UserState
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult
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
@ -292,13 +293,21 @@ class VaultAddEditViewModel @Inject constructor(
handleMasterPasswordFido2VerificationSubmit(action)
}
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick -> {
handleDismissFido2PasswordVerificationDialogClick()
}
VaultAddEditAction.Common.RetryFido2PasswordVerificationClick -> {
handleRetryFido2PasswordVerificationClick()
}
is VaultAddEditAction.Common.PinFido2VerificationSubmit -> {
handlePinFido2VerificationSubmit(action)
}
VaultAddEditAction.Common.RetryFido2PinVerificationClick -> {
handleRetryFido2PinVerificationClick()
}
VaultAddEditAction.Common.DismissFido2VerificationDialogClick -> {
handleDismissFido2VerificationDialogClick()
}
}
}
@ -626,7 +635,9 @@ class VaultAddEditViewModel @Inject constructor(
}
if (activeAccount.vaultUnlockType == VaultUnlockType.PIN) {
// TODO: https://bitwarden.atlassian.net/browse/PM-9682
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2PinPrompt)
}
} else if (activeAccount.hasMasterPassword) {
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordPrompt)
@ -650,16 +661,35 @@ class VaultAddEditViewModel @Inject constructor(
}
}
private fun handleDismissFido2PasswordVerificationDialogClick() {
showFido2ErrorDialog()
}
private fun handleRetryFido2PasswordVerificationClick() {
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2MasterPasswordPrompt)
}
}
private fun handlePinFido2VerificationSubmit(
action: VaultAddEditAction.Common.PinFido2VerificationSubmit,
) {
viewModelScope.launch {
val result = authRepository.validatePin(action.pin)
sendAction(
VaultAddEditAction.Internal.ValidateFido2PinResultReceive(
result = result,
),
)
}
}
private fun handleRetryFido2PinVerificationClick() {
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2PinPrompt)
}
}
private fun handleDismissFido2VerificationDialogClick() {
showFido2ErrorDialog()
}
private fun handleAddNewCustomFieldClick(
action: VaultAddEditAction.Common.AddNewCustomFieldClick,
) {
@ -1327,6 +1357,10 @@ class VaultAddEditViewModel @Inject constructor(
is VaultAddEditAction.Internal.ValidateFido2PasswordResultReceive -> {
handleValidateFido2PasswordResultReceive(action)
}
is VaultAddEditAction.Internal.ValidateFido2PinResultReceive -> {
handleValidateFido2PinResultReceive(action)
}
}
}
@ -1575,7 +1609,7 @@ class VaultAddEditViewModel @Inject constructor(
private fun handleValidateFido2PasswordResultReceive(
action: VaultAddEditAction.Internal.ValidateFido2PasswordResultReceive,
) {
mutableStateFlow.update { it.copy(dialog = null) }
clearDialogState()
when (action.result) {
ValidatePasswordResult.Error -> {
@ -1583,28 +1617,57 @@ class VaultAddEditViewModel @Inject constructor(
}
is ValidatePasswordResult.Success -> {
if (!action.result.isValid) {
fido2CredentialManager.authenticationAttempts += 1
if (fido2CredentialManager.hasAuthenticationAttemptsRemaining()) {
mutableStateFlow.update {
it.copy(
dialog = VaultAddEditState.DialogState.Fido2MasterPasswordError,
)
}
} else {
showFido2ErrorDialog()
}
return
if (action.result.isValid) {
handleValidAuthentication()
} else {
handleInvalidAuthentication(
errorDialogState = VaultAddEditState.DialogState.Fido2MasterPasswordError,
)
}
fido2CredentialManager.isUserVerified = true
fido2CredentialManager.authenticationAttempts = 0
getRequestAndRegisterCredential()
}
}
}
private fun handleValidateFido2PinResultReceive(
action: VaultAddEditAction.Internal.ValidateFido2PinResultReceive,
) {
clearDialogState()
when (action.result) {
ValidatePinResult.Error -> {
showFido2ErrorDialog()
}
is ValidatePinResult.Success -> {
if (action.result.isValid) {
handleValidAuthentication()
} else {
handleInvalidAuthentication(
errorDialogState = VaultAddEditState.DialogState.Fido2PinError,
)
}
}
}
}
private fun handleInvalidAuthentication(errorDialogState: VaultAddEditState.DialogState) {
fido2CredentialManager.authenticationAttempts += 1
if (fido2CredentialManager.hasAuthenticationAttemptsRemaining()) {
mutableStateFlow.update {
it.copy(dialog = errorDialogState)
}
} else {
showFido2ErrorDialog()
}
}
private fun handleValidAuthentication() {
fido2CredentialManager.isUserVerified = true
fido2CredentialManager.authenticationAttempts = 0
getRequestAndRegisterCredential()
}
//endregion Internal Type Handlers
//region Utility Functions
@ -2196,6 +2259,20 @@ data class VaultAddEditState(
*/
@Parcelize
data object Fido2MasterPasswordError : DialogState()
/**
* Displays a dialog to prompt the user for their PIN as part of the FIDO 2
* user verification flow.
*/
@Parcelize
data object Fido2PinPrompt : DialogState()
/**
* Displays a dialog to alert the user that their PIN for the FIDO 2 user
* verification flow was incorrect and to retry.
*/
@Parcelize
data object Fido2PinError : DialogState()
}
}
@ -2287,7 +2364,6 @@ sealed class VaultAddEditEvent {
* Each subclass of this sealed class denotes a distinct action that can be taken.
*/
sealed class VaultAddEditAction {
/**
* Represents actions common across all item types.
*/
@ -2464,21 +2540,33 @@ sealed class VaultAddEditAction {
data object UserVerificationNotSupported : Common()
/**
* The user has clicked to submit the master password for FIDO 2 verification.
* The user has clicked to submit their 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.
* The user has clicked to retry their FIDO 2 password verification.
*/
data object RetryFido2PasswordVerificationClick : Common()
/**
* The user has clicked to submit their PIN for FIDO 2 verification.
*/
data class PinFido2VerificationSubmit(
val pin: String,
) : Common()
/**
* The user has clicked to retry their FIDO 2 PIN verification.
*/
data object RetryFido2PinVerificationClick : Common()
/**
* The user has clicked to dismiss the FIDO 2 password or PIN verification dialog.
*/
data object DismissFido2VerificationDialogClick : Common()
}
/**
@ -2825,11 +2913,19 @@ sealed class VaultAddEditAction {
) : Internal()
/**
* Indicates that a result for verifying the user's master password as part of the FIDO 2
* Indicates that the 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()
/**
* Indicates that the result for verifying the user's PIN as part of the FIDO 2
* user verification flow has been received.
*/
data class ValidateFido2PinResultReceive(
val result: ValidatePinResult,
) : Internal()
}
}

View file

@ -240,7 +240,7 @@ class VaultAddEditScreenTest : BaseComposeTest() {
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
}
@ -250,7 +250,7 @@ class VaultAddEditScreenTest : BaseComposeTest() {
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
}
@ -298,6 +298,85 @@ class VaultAddEditScreenTest : BaseComposeTest() {
}
}
@Test
fun `fido2 pin prompt dialog should display based on state`() {
val dialogTitle = "Verify PIN"
composeTestRule.onNode(isDialog()).assertDoesNotExist()
composeTestRule.onNodeWithText(dialogTitle).assertDoesNotExist()
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2PinPrompt)
}
composeTestRule
.onNodeWithText(dialogTitle)
.assertIsDisplayed()
.assert(hasAnyAncestor(isDialog()))
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
}
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
}
composeTestRule
.onAllNodesWithText(text = "PIN")
.filterToOne(hasAnyAncestor(isDialog()))
.performTextInput("PIN")
composeTestRule
.onAllNodesWithText(text = "Submit")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.PinFido2VerificationSubmit(
pin = "PIN",
),
)
}
}
@Test
fun `fido2 pin error dialog should display based on state`() {
val dialogMessage = "Invalid PIN. Try again."
composeTestRule.onNode(isDialog()).assertDoesNotExist()
composeTestRule.onNodeWithText(dialogMessage).assertDoesNotExist()
mutableStateFlow.update {
it.copy(dialog = VaultAddEditState.DialogState.Fido2PinError)
}
composeTestRule
.onNodeWithText(dialogMessage)
.assertIsDisplayed()
.assert(hasAnyAncestor(isDialog()))
composeTestRule
.onAllNodesWithText(text = "Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.RetryFido2PinVerificationClick,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `clicking dismiss dialog on Fido2Error dialog should send Fido2ErrorDialogDismissed action`() {

View file

@ -15,6 +15,7 @@ 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.ValidatePinResult
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
@ -3184,6 +3185,25 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
)
}
@Suppress("MaxLineLength")
@Test
fun `UserVerificationNotSupported should display Fido2PinPrompt when user has pin`() {
val userState = createUserState()
mutableUserStateFlow.value = userState.copy(
accounts = listOf(
userState.accounts.first().copy(
vaultUnlockType = VaultUnlockType.PIN,
),
),
)
viewModel.trySendAction(VaultAddEditAction.Common.UserVerificationNotSupported)
verify { fido2CredentialManager.isUserVerified = false }
assertEquals(
VaultAddEditState.DialogState.Fido2PinPrompt,
viewModel.stateFlow.value.dialog,
)
}
@Suppress("MaxLineLength")
@Test
fun `UserVerificationNotSupported should display Fido2MasterPasswordPrompt when user has password but no pin`() {
@ -3291,9 +3311,27 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
@Test
fun `DismissFido2PasswordVerificationDialogClick should display Fido2ErrorDialog`() {
fun `RetryFido2PasswordVerificationClick should display Fido2MasterPasswordPrompt`() {
viewModel.trySendAction(VaultAddEditAction.Common.RetryFido2PasswordVerificationClick)
assertEquals(
VaultAddEditState.DialogState.Fido2MasterPasswordPrompt,
viewModel.stateFlow.value.dialog,
)
}
@Suppress("MaxLineLength")
@Test
fun `PinFido2VerificationSubmit should display Fido2Error when Pin verification fails`() {
val pin = "PIN"
coEvery {
authRepository.validatePin(pin = pin)
} returns ValidatePinResult.Error
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2PasswordVerificationDialogClick,
VaultAddEditAction.Common.PinFido2VerificationSubmit(
pin = pin,
),
)
assertEquals(
@ -3303,14 +3341,100 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
),
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePin(pin = pin)
}
}
@Suppress("MaxLineLength")
@Test
fun `PinFido2VerificationSubmit should display Fido2PinError when user has retries remaining`() {
val pin = "PIN"
coEvery {
authRepository.validatePin(pin = pin)
} returns ValidatePinResult.Success(isValid = false)
viewModel.trySendAction(
VaultAddEditAction.Common.PinFido2VerificationSubmit(
pin = pin,
),
)
assertEquals(
VaultAddEditState.DialogState.Fido2PinError,
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePin(pin = pin)
}
}
@Suppress("MaxLineLength")
@Test
fun `PinFido2VerificationSubmit should display Fido2Error when user has no retries remaining`() {
val pin = "PIN"
every { fido2CredentialManager.hasAuthenticationAttemptsRemaining() } returns false
coEvery {
authRepository.validatePin(pin = pin)
} returns ValidatePinResult.Success(isValid = false)
viewModel.trySendAction(
VaultAddEditAction.Common.PinFido2VerificationSubmit(
pin = pin,
),
)
assertEquals(
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified
.asText(),
),
viewModel.stateFlow.value.dialog,
)
coVerify {
authRepository.validatePin(pin = pin)
}
}
@Suppress("MaxLineLength")
@Test
fun `PinFido2VerificationSubmit should register credential when pin authenticated successfully`() {
val pin = "PIN"
coEvery {
authRepository.validatePin(pin = pin)
} returns ValidatePinResult.Success(isValid = true)
viewModel.trySendAction(
VaultAddEditAction.Common.PinFido2VerificationSubmit(
pin = pin,
),
)
coVerify {
authRepository.validatePin(pin = pin)
}
}
@Test
fun `RetryFido2PasswordVerificationClick should display Fido2MasterPasswordPrompt`() {
viewModel.trySendAction(VaultAddEditAction.Common.RetryFido2PasswordVerificationClick)
fun `RetryFido2PinVerificationClick should display Fido2PinPrompt`() {
viewModel.trySendAction(VaultAddEditAction.Common.RetryFido2PinVerificationClick)
assertEquals(
VaultAddEditState.DialogState.Fido2MasterPasswordPrompt,
VaultAddEditState.DialogState.Fido2PinPrompt,
viewModel.stateFlow.value.dialog,
)
}
@Test
fun `DismissFido2VerificationDialogClick should display Fido2ErrorDialog`() {
viewModel.trySendAction(
VaultAddEditAction.Common.DismissFido2VerificationDialogClick,
)
assertEquals(
VaultAddEditState.DialogState.Fido2Error(
message = R.string.passkey_operation_failed_because_user_could_not_be_verified
.asText(),
),
viewModel.stateFlow.value.dialog,
)
}