[PM-13073] Handle Fido2 credential errors on vault unlock screen (#4010)

This commit is contained in:
Patrick Honkonen 2024-10-03 15:40:03 -04:00 committed by GitHub
parent 32f2bfb29f
commit 488ec095bc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 127 additions and 5 deletions

View file

@ -38,12 +38,15 @@ import androidx.compose.ui.unit.dp
import androidx.hilt.navigation.compose.hiltViewModel import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.x8bit.bitwarden.R 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.inputFieldVisibilityToggleTestTag
import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenInputLabel 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.unlockScreenInputTestTag
import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenKeyboardType 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.unlockScreenMessage
import com.x8bit.bitwarden.ui.auth.feature.vaultunlock.util.unlockScreenTitle 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.EventsEffect
import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.components.account.BitwardenAccountActionItem 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.field.BitwardenPasswordField
import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
import com.x8bit.bitwarden.ui.platform.composition.LocalBiometricsManager 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.manager.biometrics.BiometricsManager
import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme
import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.persistentListOf
@ -71,12 +75,13 @@ import javax.crypto.Cipher
* The top level composable for the Vault Unlock screen. * The top level composable for the Vault Unlock screen.
*/ */
@OptIn(ExperimentalMaterial3Api::class) @OptIn(ExperimentalMaterial3Api::class)
@Suppress("LongMethod") @Suppress("LongMethod", "CyclomaticComplexMethod")
@Composable @Composable
fun VaultUnlockScreen( fun VaultUnlockScreen(
viewModel: VaultUnlockViewModel = hiltViewModel(), viewModel: VaultUnlockViewModel = hiltViewModel(),
biometricsManager: BiometricsManager = LocalBiometricsManager.current, biometricsManager: BiometricsManager = LocalBiometricsManager.current,
focusManager: FocusManager = LocalFocusManager.current, focusManager: FocusManager = LocalFocusManager.current,
fido2CompletionManager: Fido2CompletionManager = LocalFido2CompletionManager.current,
) { ) {
val state by viewModel.stateFlow.collectAsStateWithLifecycle() val state by viewModel.stateFlow.collectAsStateWithLifecycle()
val context = LocalContext.current val context = LocalContext.current
@ -108,6 +113,17 @@ fun VaultUnlockScreen(
cipher = event.cipher, 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) { when (val dialog = state.dialog) {
is VaultUnlockState.VaultUnlockDialog.Error -> BitwardenBasicDialog( is VaultUnlockState.VaultUnlockDialog.Error -> BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown( visibilityState = BasicDialogState.Shown(
title = R.string.an_error_has_occurred.asText(), title = dialog.title,
message = dialog.message, message = dialog.message,
), ),
onDismissRequest = remember(viewModel) { onDismissRequest = remember(viewModel) {

View file

@ -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.UserState
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType 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.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.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.VaultRepository
@ -81,6 +83,10 @@ class VaultUnlockViewModel @Inject constructor(
showBiometricInvalidatedMessage = false, showBiometricInvalidatedMessage = false,
vaultUnlockType = vaultUnlockType, vaultUnlockType = vaultUnlockType,
userId = userState.activeUserId, 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() { private fun handleDismissDialog() {
mutableStateFlow.update { it.copy(dialog = null) } mutableStateFlow.update { it.copy(dialog = null) }
when {
state.fido2GetCredentialsRequest != null -> {
sendEvent(VaultUnlockEvent.Fido2GetCredentialsError)
}
state.fido2CredentialAssertionRequest != null -> {
sendEvent(VaultUnlockEvent.Fido2CredentialAssertionError)
}
else -> Unit
}
} }
private fun handleConfirmLogoutClick() { private fun handleConfirmLogoutClick() {
@ -206,7 +223,8 @@ class VaultUnlockViewModel @Inject constructor(
mutableStateFlow.update { mutableStateFlow.update {
it.copy( it.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( 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 { mutableStateFlow.update {
it.copy( it.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( 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() R.string.generic_error_message.asText()
} else { } else {
state.vaultUnlockType.unlockScreenErrorMessage state.vaultUnlockType.unlockScreenErrorMessage
@ -285,7 +304,8 @@ class VaultUnlockViewModel @Inject constructor(
mutableStateFlow.update { mutableStateFlow.update {
it.copy( it.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( 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 showBiometricInvalidatedMessage: Boolean,
val vaultUnlockType: VaultUnlockType, val vaultUnlockType: VaultUnlockType,
val userId: String, val userId: String,
val fido2GetCredentialsRequest: Fido2GetCredentialsRequest? = null,
val fido2CredentialAssertionRequest: Fido2CredentialAssertionRequest? = null,
) : Parcelable { ) : Parcelable {
/** /**
@ -389,6 +411,7 @@ data class VaultUnlockState(
*/ */
@Parcelize @Parcelize
data class Error( data class Error(
val title: Text,
val message: Text, val message: Text,
) : VaultUnlockDialog() ) : VaultUnlockDialog()
@ -415,6 +438,16 @@ sealed class VaultUnlockEvent {
* Prompts the user for biometrics unlock. * Prompts the user for biometrics unlock.
*/ */
data class PromptForBiometrics(val cipher: Cipher) : BackgroundEvent, VaultUnlockEvent() 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()
} }
/** /**

View file

@ -19,7 +19,10 @@ import androidx.compose.ui.test.performScrollTo
import androidx.compose.ui.test.performTextInput import androidx.compose.ui.test.performTextInput
import androidx.compose.ui.test.requestFocus import androidx.compose.ui.test.requestFocus
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType 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.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.base.BaseComposeTest
import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager
@ -71,6 +74,10 @@ class VaultUnlockScreenTest : BaseComposeTest() {
) )
} just runs } just runs
} }
private val fido2CompletionManager: Fido2CompletionManager = mockk {
every { completeFido2Assertion(any()) } just runs
every { completeFido2GetCredentialRequest(any()) } just runs
}
@Before @Before
fun setUp() { fun setUp() {
@ -78,6 +85,7 @@ class VaultUnlockScreenTest : BaseComposeTest() {
VaultUnlockScreen( VaultUnlockScreen(
viewModel = viewModel, viewModel = viewModel,
biometricsManager = biometricsManager, 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 @Test
fun `account icon click should show the account switcher`() { fun `account icon click should show the account switcher`() {
composeTestRule.assertSwitcherIsNotDisplayed( composeTestRule.assertSwitcherIsNotDisplayed(

View file

@ -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.UserState
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType 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.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.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.platform.repository.model.Environment 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 @Test
fun `on ConfirmLogoutClick should call logout on the AuthRepository`() { fun `on ConfirmLogoutClick should call logout on the AuthRepository`() {
val viewModel = createViewModel() val viewModel = createViewModel()
@ -506,6 +538,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.validation_field_required.asText( R.string.validation_field_required.asText(
initialState.vaultUnlockType.unlockScreenInputLabel, initialState.vaultUnlockType.unlockScreenInputLabel,
), ),
@ -531,6 +564,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.invalid_master_password.asText(), R.string.invalid_master_password.asText(),
), ),
), ),
@ -557,6 +591,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -583,6 +618,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -665,6 +701,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.validation_field_required.asText( R.string.validation_field_required.asText(
initialState.vaultUnlockType.unlockScreenInputLabel, initialState.vaultUnlockType.unlockScreenInputLabel,
), ),
@ -690,6 +727,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.invalid_pin.asText(), R.string.invalid_pin.asText(),
), ),
), ),
@ -716,6 +754,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -742,6 +781,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -840,6 +880,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -867,6 +908,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),
@ -894,6 +936,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = VaultUnlockState.VaultUnlockDialog.Error( dialog = VaultUnlockState.VaultUnlockDialog.Error(
R.string.an_error_has_occurred.asText(),
R.string.generic_error_message.asText(), R.string.generic_error_message.asText(),
), ),
), ),