From 488ec095bc1e5d3eef027a366f49453128222f7d Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:40:03 -0400 Subject: [PATCH] [PM-13073] Handle Fido2 credential errors on vault unlock screen (#4010) --- .../feature/vaultunlock/VaultUnlockScreen.kt | 20 ++++++++- .../vaultunlock/VaultUnlockViewModel.kt | 39 +++++++++++++++-- .../vaultunlock/VaultUnlockScreenTest.kt | 30 +++++++++++++ .../vaultunlock/VaultUnlockViewModelTest.kt | 43 +++++++++++++++++++ 4 files changed, 127 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt index 20fad0cf2..6fde2e627 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt @@ -38,12 +38,15 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsResult import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.inputFieldVisibilityToggleTestTag import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenInputLabel import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenInputTestTag import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenKeyboardType import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenMessage import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenTitle +import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManager import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.account.BitwardenAccountActionItem @@ -61,6 +64,7 @@ import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState import com.x8bit.bitwarden.ui.platform.components.field.BitwardenPasswordField import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.composition.LocalBiometricsManager +import com.x8bit.bitwarden.ui.platform.composition.LocalFido2CompletionManager import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme import kotlinx.collections.immutable.persistentListOf @@ -71,12 +75,13 @@ import javax.crypto.Cipher * The top level composable for the Vault Unlock screen. */ @OptIn(ExperimentalMaterial3Api::class) -@Suppress("LongMethod") +@Suppress("LongMethod", "CyclomaticComplexMethod") @Composable fun VaultUnlockScreen( viewModel: VaultUnlockViewModel = hiltViewModel(), biometricsManager: BiometricsManager = LocalBiometricsManager.current, focusManager: FocusManager = LocalFocusManager.current, + fido2CompletionManager: Fido2CompletionManager = LocalFido2CompletionManager.current, ) { val state by viewModel.stateFlow.collectAsStateWithLifecycle() val context = LocalContext.current @@ -108,6 +113,17 @@ fun VaultUnlockScreen( cipher = event.cipher, ) } + + VaultUnlockEvent.Fido2CredentialAssertionError -> { + fido2CompletionManager.completeFido2Assertion( + result = Fido2CredentialAssertionResult.Error, + ) + } + VaultUnlockEvent.Fido2GetCredentialsError -> { + fido2CompletionManager.completeFido2GetCredentialRequest( + result = Fido2GetCredentialsResult.Error, + ) + } } } @@ -121,7 +137,7 @@ fun VaultUnlockScreen( when (val dialog = state.dialog) { is VaultUnlockState.VaultUnlockDialog.Error -> BitwardenBasicDialog( visibilityState = BasicDialogState.Shown( - title = R.string.an_error_has_occurred.asText(), + title = dialog.title, message = dialog.message, ), onDismissRequest = remember(viewModel) { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt index f2e6f76e7..3f6f0bcda 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt @@ -9,6 +9,8 @@ import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.UserState 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.Fido2CredentialAssertionRequest +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsRequest import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.vault.repository.VaultRepository @@ -81,6 +83,10 @@ class VaultUnlockViewModel @Inject constructor( showBiometricInvalidatedMessage = false, vaultUnlockType = vaultUnlockType, userId = userState.activeUserId, + // TODO: [PM-13075] Handle Fido2GetCredentialsRequest special circumstance + fido2GetCredentialsRequest = null, + // TODO: [PM-13076] Handle Fido2CredentialAssertionRequest special circumstance + fido2CredentialAssertionRequest = null, ) }, ) { @@ -133,6 +139,17 @@ class VaultUnlockViewModel @Inject constructor( private fun handleDismissDialog() { mutableStateFlow.update { it.copy(dialog = null) } + when { + state.fido2GetCredentialsRequest != null -> { + sendEvent(VaultUnlockEvent.Fido2GetCredentialsError) + } + + state.fido2CredentialAssertionRequest != null -> { + sendEvent(VaultUnlockEvent.Fido2CredentialAssertionError) + } + + else -> Unit + } } private fun handleConfirmLogoutClick() { @@ -206,7 +223,8 @@ class VaultUnlockViewModel @Inject constructor( mutableStateFlow.update { it.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( - it.vaultUnlockType.emptyInputDialogMessage, + title = R.string.an_error_has_occurred.asText(), + message = it.vaultUnlockType.emptyInputDialogMessage, ), ) } @@ -269,7 +287,8 @@ class VaultUnlockViewModel @Inject constructor( mutableStateFlow.update { it.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( - if (action.isBiometricLogin) { + title = R.string.an_error_has_occurred.asText(), + message = if (action.isBiometricLogin) { R.string.generic_error_message.asText() } else { state.vaultUnlockType.unlockScreenErrorMessage @@ -285,7 +304,8 @@ class VaultUnlockViewModel @Inject constructor( mutableStateFlow.update { it.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( - R.string.generic_error_message.asText(), + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), ), ) } @@ -363,6 +383,8 @@ data class VaultUnlockState( val showBiometricInvalidatedMessage: Boolean, val vaultUnlockType: VaultUnlockType, val userId: String, + val fido2GetCredentialsRequest: Fido2GetCredentialsRequest? = null, + val fido2CredentialAssertionRequest: Fido2CredentialAssertionRequest? = null, ) : Parcelable { /** @@ -389,6 +411,7 @@ data class VaultUnlockState( */ @Parcelize data class Error( + val title: Text, val message: Text, ) : VaultUnlockDialog() @@ -415,6 +438,16 @@ sealed class VaultUnlockEvent { * Prompts the user for biometrics unlock. */ data class PromptForBiometrics(val cipher: Cipher) : BackgroundEvent, VaultUnlockEvent() + + /** + * Completes the FIDO2 get credentials request with an error response. + */ + data object Fido2GetCredentialsError : VaultUnlockEvent() + + /** + * Completes the FIDO2 credential assertion request with an error response. + */ + data object Fido2CredentialAssertionError : VaultUnlockEvent() } /** diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt index 0cb369343..ef02ea960 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt @@ -19,7 +19,10 @@ import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performTextInput import androidx.compose.ui.test.requestFocus import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2GetCredentialsResult import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow +import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManager import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager @@ -71,6 +74,10 @@ class VaultUnlockScreenTest : BaseComposeTest() { ) } just runs } + private val fido2CompletionManager: Fido2CompletionManager = mockk { + every { completeFido2Assertion(any()) } just runs + every { completeFido2GetCredentialRequest(any()) } just runs + } @Before fun setUp() { @@ -78,6 +85,7 @@ class VaultUnlockScreenTest : BaseComposeTest() { VaultUnlockScreen( viewModel = viewModel, biometricsManager = biometricsManager, + fido2CompletionManager = fido2CompletionManager, ) } } @@ -114,6 +122,28 @@ class VaultUnlockScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") + @Test + fun `on Fido2GetCredentialsError should call completeFido2GetCredentialRequest on fido2CompletionManager`() { + mutableEventFlow.tryEmit(VaultUnlockEvent.Fido2GetCredentialsError) + verify(exactly = 1) { + fido2CompletionManager.completeFido2GetCredentialRequest( + result = Fido2GetCredentialsResult.Error, + ) + } + } + + @Suppress("MaxLineLength") + @Test + fun `on Fido2AssertCredentialError should call completeFido2AssertCredential on fido2CompletionManager`() { + mutableEventFlow.tryEmit(VaultUnlockEvent.Fido2CredentialAssertionError) + verify(exactly = 1) { + fido2CompletionManager.completeFido2Assertion( + result = Fido2CredentialAssertionResult.Error, + ) + } + } + @Test fun `account icon click should show the account switcher`() { composeTestRule.assertSwitcherIsNotDisplayed( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt index a67a6dd11..33df32539 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt @@ -10,6 +10,8 @@ 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.VaultUnlockType 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.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.model.Environment @@ -382,6 +384,36 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { ) } + @Suppress("MaxLineLength") + @Test + fun `on DismissDialog should emit Fido2GetCredentialsError when state has Fido2GetCredentialsRequest`() = + runTest { + val initialState = DEFAULT_STATE.copy( + fido2GetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1), + ) + val viewModel = createViewModel(state = initialState) + viewModel.trySendAction(VaultUnlockAction.DismissDialog) + viewModel.eventFlow.test { + assertEquals(VaultUnlockEvent.Fido2GetCredentialsError, awaitItem()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `on DismissDialog should emit Fido2CredentialAssertionError when state has Fido2CredentialAssertionRequest`() = + runTest { + val initialState = DEFAULT_STATE.copy( + fido2CredentialAssertionRequest = createMockFido2CredentialAssertionRequest( + number = 1, + ), + ) + val viewModel = createViewModel(state = initialState) + viewModel.trySendAction(VaultUnlockAction.DismissDialog) + viewModel.eventFlow.test { + assertEquals(VaultUnlockEvent.Fido2CredentialAssertionError, awaitItem()) + } + } + @Test fun `on ConfirmLogoutClick should call logout on the AuthRepository`() { val viewModel = createViewModel() @@ -506,6 +538,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.validation_field_required.asText( initialState.vaultUnlockType.unlockScreenInputLabel, ), @@ -531,6 +564,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.invalid_master_password.asText(), ), ), @@ -557,6 +591,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -583,6 +618,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -665,6 +701,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.validation_field_required.asText( initialState.vaultUnlockType.unlockScreenInputLabel, ), @@ -690,6 +727,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.invalid_pin.asText(), ), ), @@ -716,6 +754,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -742,6 +781,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -840,6 +880,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -867,6 +908,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ), @@ -894,6 +936,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = VaultUnlockState.VaultUnlockDialog.Error( + R.string.an_error_has_occurred.asText(), R.string.generic_error_message.asText(), ), ),