From c09fe554bc0c7788ced0fdc17a3ab54ab3b39f35 Mon Sep 17 00:00:00 2001 From: Shannon Draeker <125921730+shannon-livefront@users.noreply.github.com> Date: Thu, 25 Jul 2024 10:59:20 -0600 Subject: [PATCH] PM-9681: Setup Bitwarden PIN (#3626) --- .../accountsecurity/AccountSecurityScreen.kt | 5 +- .../accountsecurity/PinInputDialog.kt | 15 +-- .../itemlisting/VaultItemListingScreen.kt | 44 ++++++- .../itemlisting/VaultItemListingViewModel.kt | 99 +++++++++++++-- .../itemlisting/VaultItemListingScreenTest.kt | 74 +++++++++++ .../VaultItemListingViewModelTest.kt | 116 +++++++++++++++++- 6 files changed, 334 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt index d3ac2eabb1..d92255419d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt @@ -417,7 +417,7 @@ private fun UnlockWithPinRow( ) { var shouldShowPinInputDialog by rememberSaveable { mutableStateOf(false) } var shouldShowPinConfirmationDialog by rememberSaveable { mutableStateOf(false) } - var pin by rememberSaveable { mutableStateOf("") } + var pin by remember { mutableStateOf("") } BitwardenWideSwitch( label = stringResource(id = R.string.unlock_with_pin), isChecked = isUnlockWithPinEnabled, @@ -439,8 +439,6 @@ private fun UnlockWithPinRow( when { shouldShowPinInputDialog -> { PinInputDialog( - pin = pin, - onPinChange = { pin = it }, onCancelClick = { shouldShowPinInputDialog = false onUnlockWithPinToggleAction( @@ -449,6 +447,7 @@ private fun UnlockWithPinRow( pin = "" }, onSubmitClick = { + pin = it if (pin.isNotEmpty()) { shouldShowPinInputDialog = false if (isUnlockWithPasswordEnabled) { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/PinInputDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/PinInputDialog.kt index 5f4fa23302..0e927efa76 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/PinInputDialog.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/PinInputDialog.kt @@ -14,6 +14,10 @@ import androidx.compose.foundation.verticalScroll import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier @@ -35,8 +39,6 @@ import com.x8bit.bitwarden.ui.platform.components.util.maxDialogHeight /** * A dialog for setting a user's PIN. * - * @param pin The current value of the PIN. - * @param onPinChange A callback for internal changes to the PIN. * @param onCancelClick A callback for when the "Cancel" button is clicked. * @param onSubmitClick A callback for when the "Submit" button is clicked. * @param onDismissRequest A callback for when the dialog is requesting to be dismissed. @@ -45,12 +47,11 @@ import com.x8bit.bitwarden.ui.platform.components.util.maxDialogHeight @Suppress("LongMethod") @Composable fun PinInputDialog( - pin: String, - onPinChange: (String) -> Unit, onCancelClick: () -> Unit, - onSubmitClick: () -> Unit, + onSubmitClick: (String) -> Unit, onDismissRequest: () -> Unit, ) { + var pin by remember { mutableStateOf("") } Dialog( onDismissRequest = onDismissRequest, ) { @@ -107,7 +108,7 @@ fun PinInputDialog( BitwardenTextField( label = stringResource(id = R.string.pin), value = pin, - onValueChange = onPinChange, + onValueChange = { pin = it }, keyboardType = KeyboardType.Number, modifier = Modifier .testTag("AlertInputField") @@ -135,7 +136,7 @@ fun PinInputDialog( BitwardenFilledButton( label = stringResource(id = R.string.submit), - onClick = onSubmitClick, + onClick = { onSubmitClick(pin) }, modifier = Modifier.testTag("AcceptAlertButton"), ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt index 8959f2250a..35c7a71ffd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt @@ -51,6 +51,7 @@ import com.x8bit.bitwarden.ui.platform.composition.LocalBiometricsManager import com.x8bit.bitwarden.ui.platform.composition.LocalFido2CompletionManager import com.x8bit.bitwarden.ui.platform.composition.LocalIntentManager import com.x8bit.bitwarden.ui.platform.feature.search.model.SearchType +import com.x8bit.bitwarden.ui.platform.feature.settings.accountsecurity.PinInputDialog import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager import com.x8bit.bitwarden.ui.vault.feature.itemlisting.handlers.VaultItemListingHandlers @@ -222,6 +223,23 @@ fun VaultItemListingScreen( ) } }, + onSubmitPinSetUpFido2Verification = remember(viewModel) { + { pin, cipherId -> + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpSubmit( + pin = pin, + selectedCipherId = cipherId, + ), + ) + } + }, + onRetryPinSetUpFido2Verification = remember(viewModel) { + { + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpRetryClick(it), + ) + } + }, onDismissFido2Verification = remember(viewModel) { { viewModel.trySendAction( @@ -248,9 +266,11 @@ private fun VaultItemListingDialogs( onDismissFido2ErrorDialog: () -> Unit, onConfirmOverwriteExistingPasskey: (cipherViewId: String) -> Unit, onSubmitMasterPasswordFido2Verification: (password: String, cipherId: String) -> Unit, - onRetryFido2PasswordVerification: (cipherViewId: String) -> Unit, + onRetryFido2PasswordVerification: (cipherId: String) -> Unit, onSubmitPinFido2Verification: (pin: String, cipherId: String) -> Unit, onRetryFido2PinVerification: (cipherViewId: String) -> Unit, + onSubmitPinSetUpFido2Verification: (pin: String, cipherId: String) -> Unit, + onRetryPinSetUpFido2Verification: (cipherId: String) -> Unit, onDismissFido2Verification: () -> Unit, ) { when (dialogState) { @@ -329,6 +349,28 @@ private fun VaultItemListingDialogs( ) } + is VaultItemListingState.DialogState.Fido2PinSetUpPrompt -> { + PinInputDialog( + onCancelClick = onDismissFido2Verification, + onSubmitClick = { pin -> + onSubmitPinSetUpFido2Verification(pin, dialogState.selectedCipherId) + }, + onDismissRequest = onDismissFido2Verification, + ) + } + + is VaultItemListingState.DialogState.Fido2PinSetUpError -> { + BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = dialogState.title, + message = dialogState.message, + ), + onDismissRequest = { + onRetryPinSetUpFido2Verification(dialogState.selectedCipherId) + }, + ) + } + null -> Unit } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt index 412e00e8a6..9154ad1ebc 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt @@ -176,6 +176,7 @@ class VaultItemListingViewModel @Inject constructor( .launchIn(viewModelScope) } + @Suppress("LongMethod") override fun handleAction(action: VaultItemListingsAction) { when (action) { is VaultItemListingsAction.LockAccountClick -> handleLockAccountClick(action) @@ -202,6 +203,12 @@ class VaultItemListingViewModel @Inject constructor( handleRetryFido2PinVerificationClick(action) } + is VaultItemListingsAction.PinFido2SetUpSubmit -> handlePinFido2SetUpSubmit(action) + + is VaultItemListingsAction.PinFido2SetUpRetryClick -> { + handlePinFido2SetUpRetryClick(action) + } + VaultItemListingsAction.DismissFido2VerificationDialogClick -> { handleDismissFido2VerificationDialogClick() } @@ -370,7 +377,13 @@ class VaultItemListingViewModel @Inject constructor( } } else { // Prompt the user to set up a PIN for their account. - // TODO: https://bitwarden.atlassian.net/browse/PM-9681 + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Fido2PinSetUpPrompt( + selectedCipherId = selectedCipherId, + ), + ) + } } } @@ -426,6 +439,44 @@ class VaultItemListingViewModel @Inject constructor( } } + private fun handlePinFido2SetUpSubmit(action: VaultItemListingsAction.PinFido2SetUpSubmit) { + if (action.pin.isBlank()) { + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Fido2PinSetUpError( + title = null, + message = R.string.validation_field_required.asText(R.string.pin.asText()), + selectedCipherId = action.selectedCipherId, + ), + ) + } + return + } + + // There's no need to ask the user whether or not they want to use their master password + // on login, and shouldRequireMasterPasswordOnRestart is hardcoded to false, because the + // user can only reach this part of the flow if they have no master password. + settingsRepository.storeUnlockPin( + pin = action.pin, + shouldRequireMasterPasswordOnRestart = false, + ) + + // After storing the PIN, the user can proceed with their original FIDO 2 request. + handleValidAuthentication(selectedCipherId = action.selectedCipherId) + } + + private fun handlePinFido2SetUpRetryClick( + action: VaultItemListingsAction.PinFido2SetUpRetryClick, + ) { + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Fido2PinSetUpPrompt( + selectedCipherId = action.selectedCipherId, + ), + ) + } + } + private fun handleDismissFido2VerificationDialogClick() { showFido2ErrorDialog() } @@ -1496,6 +1547,25 @@ data class VaultItemListingState( val message: Text, val selectedCipherId: String, ) : DialogState() + + /** + * Represents a dialog to prompt the user to set up a PIN for the FIDO 2 user + * verification flow. + */ + @Parcelize + data class Fido2PinSetUpPrompt( + val selectedCipherId: String, + ) : DialogState() + + /** + * Represents a dialog to alert the user that the PIN is a required field. + */ + @Parcelize + data class Fido2PinSetUpError( + val title: Text?, + val message: Text, + val selectedCipherId: String, + ) : DialogState() } /** @@ -1882,11 +1952,6 @@ sealed class VaultItemListingsAction { val selectedCipherId: String, ) : VaultItemListingsAction() - /** - * Click to dismiss the FIDO 2 password or PIN verification dialog. - */ - data object DismissFido2VerificationDialogClick : VaultItemListingsAction() - /** * Click to retry the FIDO 2 password verification. */ @@ -1895,7 +1960,7 @@ sealed class VaultItemListingsAction { ) : VaultItemListingsAction() /** - * Click to submit the PIN for FIDO 2 verification. + * Click to submit the PIN for FIDO 2 user verification. */ data class PinFido2VerificationSubmit( val pin: String, @@ -1909,6 +1974,26 @@ sealed class VaultItemListingsAction { val selectedCipherId: String, ) : VaultItemListingsAction() + /** + * Click to submit to set up a PIN for the FIDO 2 user verification flow. + */ + data class PinFido2SetUpSubmit( + val pin: String, + val selectedCipherId: String, + ) : VaultItemListingsAction() + + /** + * Click to retry setting up a PIN for the FIDO 2 user verification flow. + */ + data class PinFido2SetUpRetryClick( + val selectedCipherId: String, + ) : VaultItemListingsAction() + + /** + * Click to dismiss the FIDO 2 password or PIN verification dialog. + */ + data object DismissFido2VerificationDialogClick : VaultItemListingsAction() + /** * Click the refresh button. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt index 8774ff0154..ed3f5ccff5 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt @@ -1672,6 +1672,80 @@ class VaultItemListingScreenTest : BaseComposeTest() { } } + @Test + fun `fido2 pin set up dialog should display and function according to state`() { + val selectedCipherId = "selectedCipherId" + val dialogMessage = "Enter your PIN code." + composeTestRule.onNode(isDialog()).assertDoesNotExist() + composeTestRule.onNodeWithText(dialogMessage).assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Fido2PinSetUpPrompt( + selectedCipherId = selectedCipherId, + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "Cancel") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + verify { + viewModel.trySendAction( + VaultItemListingsAction.DismissFido2VerificationDialogClick, + ) + } + + composeTestRule + .onAllNodesWithText(text = "PIN") + .filterToOne(hasAnyAncestor(isDialog())) + .performTextInput("PIN") + composeTestRule + .onAllNodesWithText(text = "Submit") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify { + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpSubmit( + pin = "PIN", + selectedCipherId = selectedCipherId, + ), + ) + } + } + + @Test + fun `fido2 pin set up error dialog should display and function according to state`() { + val selectedCipherId = "selectedCipherId" + val dialogMessage = "The PIN field is required." + composeTestRule.onNode(isDialog()).assertDoesNotExist() + composeTestRule.onNodeWithText(dialogMessage).assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Fido2PinSetUpError( + title = null, + message = dialogMessage.asText(), + selectedCipherId = selectedCipherId, + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + verify { + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpRetryClick( + selectedCipherId = selectedCipherId, + ), + ) + } + } + @Test fun `CompleteFido2Registration event should call Fido2CompletionManager with result`() { val result = Fido2RegisterCredentialResult.Success("mockResponse") diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt index 5d1ec72b82..95af83542b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt @@ -9,8 +9,8 @@ import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult import com.x8bit.bitwarden.data.auth.repository.model.UserState -import com.x8bit.bitwarden.data.auth.repository.model.ValidatePinResult 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 @@ -2435,6 +2435,41 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + @Suppress("MaxLineLength") + @Test + fun `UserVerificationNotSupported should display Fido2PinSetUpPrompt when user has no password or pin`() { + val viewModel = createVaultItemListingViewModel() + val selectedCipherId = "selectedCipherId" + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + accounts = listOf( + DEFAULT_ACCOUNT.copy( + vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, + trustedDevice = UserState.TrustedDevice( + isDeviceTrusted = true, + hasMasterPassword = false, + hasAdminApproval = true, + hasLoginApprovingDevice = true, + hasResetPasswordPermission = true, + ), + ), + ), + ) + + viewModel.trySendAction( + VaultItemListingsAction.UserVerificationNotSupported( + selectedCipherId = selectedCipherId, + ), + ) + + verify { fido2CredentialManager.isUserVerified = false } + assertEquals( + VaultItemListingState.DialogState.Fido2PinSetUpPrompt( + selectedCipherId = selectedCipherId, + ), + viewModel.stateFlow.value.dialogState, + ) + } + @Suppress("MaxLineLength") @Test fun `MasterPasswordFido2VerificationSubmit should display Fido2ErrorDialog when password verification fails`() { @@ -2776,6 +2811,85 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + @Test + fun `PinFido2SetUpSubmit should display Fido2PinSetUpError for empty PIN`() { + val viewModel = createVaultItemListingViewModel() + val pin = "" + val selectedCipherId = "selectedCipherId" + + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpSubmit( + pin = pin, + selectedCipherId = selectedCipherId, + ), + ) + + assertEquals( + VaultItemListingState.DialogState.Fido2PinSetUpError( + title = null, + message = R.string.validation_field_required.asText(R.string.pin.asText()), + selectedCipherId = selectedCipherId, + ), + viewModel.stateFlow.value.dialogState, + ) + } + + @Test + fun `PinFido2SetUpSubmit should save PIN and register credential for non-empty PIN`() { + setupMockUri() + val viewModel = createVaultItemListingViewModel() + val cipherView = createMockCipherView(number = 1) + val pin = "PIN" + val selectedCipherId = "selectedCipherId" + mutableVaultDataStateFlow.value = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(cipherView), + collectionViewList = emptyList(), + folderViewList = emptyList(), + sendViewList = emptyList(), + ), + ) + every { + settingsRepository.storeUnlockPin( + pin = pin, + shouldRequireMasterPasswordOnRestart = false, + ) + } just runs + + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpSubmit( + pin = pin, + selectedCipherId = selectedCipherId, + ), + ) + + verify(exactly = 1) { + settingsRepository.storeUnlockPin( + pin = pin, + shouldRequireMasterPasswordOnRestart = false, + ) + } + } + + @Test + fun `PinFido2SetUpRetryClick should display Fido2PinSetUpPrompt`() { + val viewModel = createVaultItemListingViewModel() + val selectedCipherId = "selectedCipherId" + + viewModel.trySendAction( + VaultItemListingsAction.PinFido2SetUpRetryClick( + selectedCipherId = selectedCipherId, + ), + ) + + assertEquals( + VaultItemListingState.DialogState.Fido2PinSetUpPrompt( + selectedCipherId = selectedCipherId, + ), + viewModel.stateFlow.value.dialogState, + ) + } + @Test fun `DismissFido2VerificationDialogClick should display Fido2ErrorDialog`() { val viewModel = createVaultItemListingViewModel()