From 36270ec55a74160eda8f621111ce069ddfde7c42 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Tue, 16 Jul 2024 17:01:55 -0400 Subject: [PATCH] [PM-8137] Perform FIDO 2 verification on item listing when required (#3529) --- .../itemlisting/VaultItemListingViewModel.kt | 34 ++++++++-- .../VaultItemListingViewModelTest.kt | 67 +++++++++++++++++-- 2 files changed, 87 insertions(+), 14 deletions(-) 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 1fb025dcd..50450de13 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 @@ -255,12 +255,14 @@ class VaultItemListingViewModel @Inject constructor( } private fun handleUserVerificationLockOut() { + fido2CredentialManager.isUserVerified = false showFido2ErrorDialog() } private fun handleUserVerificationSuccess( action: VaultItemListingsAction.UserVerificationSuccess, ) { + fido2CredentialManager.isUserVerified = true specialCircumstanceManager .specialCircumstance ?.toFido2RequestOrNull() @@ -274,10 +276,12 @@ class VaultItemListingViewModel @Inject constructor( } private fun handleUserVerificationFail() { + fido2CredentialManager.isUserVerified = false showFido2ErrorDialog() } private fun handleUserVerificationCancelled() { + fido2CredentialManager.isUserVerified = false clearDialogState() sendEvent( VaultItemListingEvent.CompleteFido2Registration( @@ -287,6 +291,7 @@ class VaultItemListingViewModel @Inject constructor( } private fun handleUserVerificationNotSupported() { + fido2CredentialManager.isUserVerified = false showFido2ErrorDialog() } @@ -384,7 +389,7 @@ class VaultItemListingViewModel @Inject constructor( ?: run { // This scenario should not occur because `isFido2Creation` is false when // `fido2CredentialRequest` is null. We show the FIDO 2 error dialog to inform - // the user and terminate the flow. + // the user and terminate the flow just in case it does occur. showFido2ErrorDialog() return } @@ -395,13 +400,28 @@ class VaultItemListingViewModel @Inject constructor( ), ) } - val createOptions = fido2CredentialManager - .getPasskeyCreateOptionsOrNull(credentialRequest.requestJson) - ?: run { - showFido2ErrorDialog() - return - } + if (fido2CredentialManager.isUserVerified) { + // The user has performed verification implicitly so we continue FIDO 2 registration + // without checking the request's user verification settings. + registerFido2CredentialToCipher( + request = credentialRequest, + cipherView = cipherView, + ) + } else { + performUserVerificationIfRequired(credentialRequest, cipherView) + } + } + private fun performUserVerificationIfRequired( + credentialRequest: Fido2CredentialRequest, + cipherView: CipherView, + ) { + val createOptions = fido2CredentialManager + .getPasskeyCreateOptionsOrNull(credentialRequest.requestJson) + ?: run { + showFido2ErrorDialog() + return + } when (createOptions.authenticatorSelection.userVerification) { UserVerificationRequirement.DISCOURAGED -> { registerFido2CredentialToCipher( 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 c0f69b384..1fb152d24 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 @@ -141,6 +141,8 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } private val fido2CredentialManager: Fido2CredentialManager = mockk { coEvery { validateOrigin(any()) } returns Fido2ValidateOriginResult.Success + every { isUserVerified } returns false + every { isUserVerified = any() } just runs } private val organizationEventManager = mockk { @@ -486,6 +488,51 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } + @Suppress("MaxLineLength") + @Test + fun `ItemClick for vault item during FIDO 2 registration should skip user verification when user is verified`() { + setupMockUri() + val cipherView = createMockCipherView(number = 1) + val mockFido2CredentialRequest = createMockFido2CredentialRequest(number = 1) + specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( + fido2CredentialRequest = mockFido2CredentialRequest, + ) + mutableVaultDataStateFlow.value = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(cipherView), + folderViewList = emptyList(), + collectionViewList = emptyList(), + sendViewList = emptyList(), + ), + ) + every { + fido2CredentialManager.getPasskeyCreateOptionsOrNull(any()) + } returns createMockPublicKeyCredentialCreationOptions( + number = 1, + userVerificationRequirement = UserVerificationRequirement.REQUIRED, + ) + coEvery { + fido2CredentialManager.registerFido2Credential( + any(), + any(), + any(), + ) + } returns Fido2RegisterCredentialResult.Success("mockResponse") + every { fido2CredentialManager.isUserVerified } returns true + + val viewModel = createVaultItemListingViewModel() + viewModel.trySendAction(VaultItemListingsAction.ItemClick(cipherView.id.orEmpty())) + + coVerify { fido2CredentialManager.isUserVerified } + coVerify(exactly = 1) { + fido2CredentialManager.registerFido2Credential( + userId = DEFAULT_USER_STATE.activeUserId, + fido2CredentialRequest = mockFido2CredentialRequest, + selectedCipherView = cipherView, + ) + } + } + @Test fun `ItemClick for vault item should emit NavigateToVaultItem`() = runTest { val viewModel = createVaultItemListingViewModel() @@ -2051,10 +2098,11 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `UserVerificationLockout should display Fido2ErrorDialog`() { + fun `UserVerificationLockout should display Fido2ErrorDialog and set isUserVerified to false`() { val viewModel = createVaultItemListingViewModel() viewModel.trySendAction(VaultItemListingsAction.UserVerificationLockOut) + verify { fido2CredentialManager.isUserVerified = false } assertEquals( VaultItemListingState.DialogState.Fido2CreationFail( title = R.string.an_error_has_occurred.asText(), @@ -2066,11 +2114,12 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `UserVerificationCancelled should clear dialog state and emit CompleteFido2Create with cancelled result`() = + fun `UserVerificationCancelled should clear dialog state, set isUserVerified to false, and emit CompleteFido2Create with cancelled result`() = runTest { val viewModel = createVaultItemListingViewModel() viewModel.trySendAction(VaultItemListingsAction.UserVerificationCancelled) + verify { fido2CredentialManager.isUserVerified = false } assertNull(viewModel.stateFlow.value.dialogState) viewModel.eventFlow.test { assertEquals( @@ -2082,16 +2131,17 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } - @Suppress("MaxLineLength") @Test - fun `UserVerificationFail should display Fido2ErrorDialog`() { + fun `UserVerificationFail should display Fido2ErrorDialog and set isUserVerified to false`() { val viewModel = createVaultItemListingViewModel() viewModel.trySendAction(VaultItemListingsAction.UserVerificationFail) + verify { fido2CredentialManager.isUserVerified = false } assertEquals( VaultItemListingState.DialogState.Fido2CreationFail( title = R.string.an_error_has_occurred.asText(), - message = R.string.passkey_operation_failed_because_user_could_not_be_verified.asText(), + message = R.string.passkey_operation_failed_because_user_could_not_be_verified + .asText(), ), viewModel.stateFlow.value.dialogState, ) @@ -2192,7 +2242,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `UserVerificationSuccess should register FIDO 2 credential when registration result is received`() = + fun `UserVerificationSuccess should set isUserVerified to true, and register FIDO 2 credential when registration result is received`() = runTest { val mockRequest = createMockFido2CredentialRequest(number = 1) specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( @@ -2216,6 +2266,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) coVerify { + fido2CredentialManager.isUserVerified = true fido2CredentialManager.registerFido2Credential( userId = DEFAULT_ACCOUNT.userId, fido2CredentialRequest = mockRequest, @@ -2224,11 +2275,13 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } + @Suppress("MaxLineLength") @Test - fun `UserVerificationNotSupported should display Fido2ErrorDialog`() { + fun `UserVerificationNotSupported should display Fido2ErrorDialog and set isUserVerified to false`() { val viewModel = createVaultItemListingViewModel() viewModel.trySendAction(VaultItemListingsAction.UserVerificationNotSupported) + verify { fido2CredentialManager.isUserVerified = false } assertEquals( VaultItemListingState.DialogState.Fido2CreationFail( title = R.string.an_error_has_occurred.asText(),