[PM-10428] Default UserVerificationRequirement to PREFERRED (#3659)

bitwarden · Jul 31, 2024 · 1f8d50e · 1f8d50e
1 parent 260b3bf
commit 1f8d50e
Show file tree

Hide file tree

Showing 8 changed files with 5 additions and 128 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/PasskeyAssertionOptions.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/PasskeyAssertionOptions.kt
@@ -16,5 +16,5 @@ data class PasskeyAssertionOptions(
     @SerialName("rpId")
     val relyingPartyId: String?,
     @SerialName("userVerification")
-    val userVerification: UserVerificationRequirement?,
+    val userVerification: UserVerificationRequirement = UserVerificationRequirement.PREFERRED,
 )
diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/PasskeyAttestationOptions.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/PasskeyAttestationOptions.kt
@@ -32,7 +32,7 @@ data class PasskeyAttestationOptions(
         @SerialName("residentKey")
         val residentKeyRequirement: ResidentKeyRequirement? = null,
         @SerialName("userVerification")
-        val userVerification: UserVerificationRequirement? = null,
+        val userVerification: UserVerificationRequirement = UserVerificationRequirement.PREFERRED,
     ) {
         /**
          * Enum class representing the types of attachments associated with selection criteria.

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt
@@ -468,12 +468,6 @@ class VaultAddEditViewModel @Inject constructor(
             UserVerificationRequirement.REQUIRED -> {
                 sendEvent(VaultAddEditEvent.Fido2UserVerification(isRequired = true))
             }
-
-            null -> {
-                // Per WebAuthn spec members should be ignored when invalid. Since the request
-                // violates spec we display an error and terminate the operation.
-                showFido2ErrorDialog()
-            }
         }
     }
 

diff --git a/...c/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/...c/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt
@@ -657,12 +657,6 @@ class VaultItemListingViewModel @Inject constructor(
                     ),
                 )
             }
-
-            null -> {
-                // Per WebAuthn spec, members should be ignored when invalid. Since the request
-                // violates spec, we display an error and terminate the operation.
-                showFido2ErrorDialog()
-            }
         }
     }
 
@@ -1450,10 +1444,6 @@ class VaultItemListingViewModel @Inject constructor(
             UserVerificationRequirement.REQUIRED -> {
                 sendUserVerificationEvent(isRequired = true, selectedCipher = selectedCipher)
             }
-
-            null -> {
-                showFido2ErrorDialog()
-            }
         }
     }
 

diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt
@@ -1050,51 +1050,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
             }
         }
 
-    @Suppress("MaxLineLength")
-    @Test
-    fun `in add mode during fido2, SaveClick should show Fido2ErrorDialog when user is not verified and registration user verification option is null`() =
-        runTest {
-            val fido2CredentialRequest = createMockFido2CredentialRequest(number = 1)
-            val stateWithName = createVaultAddItemState(
-                vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN),
-                commonContentViewState = createCommonContentViewState(
-                    name = "mockName-1",
-                ),
-            )
-                .copy(shouldExitOnSave = true)
-            specialCircumstanceManager.specialCircumstance =
-                SpecialCircumstance.Fido2Save(
-                    fido2CredentialRequest = fido2CredentialRequest,
-                )
-            every {
-                fido2CredentialManager.getPasskeyAttestationOptionsOrNull(
-                    requestJson = fido2CredentialRequest.requestJson,
-                )
-            } returns createMockPasskeyAttestationOptions(
-                number = 1,
-                userVerificationRequirement = null,
-            )
-            mutableVaultDataFlow.value = DataState.Loaded(
-                createVaultData(),
-            )
-            val viewModel = createAddVaultItemViewModel(
-                createSavedStateHandleWithState(
-                    state = stateWithName,
-                    vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN),
-                ),
-            )
-
-            viewModel.trySendAction(VaultAddEditAction.Common.SaveClick)
-
-            assertEquals(
-                VaultAddEditState.DialogState.Fido2Error(
-                    message = R.string.passkey_operation_failed_because_user_could_not_be_verified
-                        .asText(),
-                ),
-                viewModel.stateFlow.value.dialog,
-            )
-        }
-
     @Test
     fun `in add mode, createCipherInOrganization success should ShowToast and NavigateBack`() =
         runTest {

diff --git a/...a/com/x8bit/bitwarden/ui/vault/feature/addedit/util/PasskeyAssertionOptionsTestHelpers.kt b/...a/com/x8bit/bitwarden/ui/vault/feature/addedit/util/PasskeyAssertionOptionsTestHelpers.kt
@@ -10,7 +10,7 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement
  */
 fun createMockPasskeyAssertionOptions(
     number: Int,
-    userVerificationRequirement: UserVerificationRequirement? =
+    userVerificationRequirement: UserVerificationRequirement =
         UserVerificationRequirement.PREFERRED,
 ) = PasskeyAssertionOptions(
     challenge = "mockChallenge-$number",

diff --git a/...com/x8bit/bitwarden/ui/vault/feature/addedit/util/PasskeyAttestationOptionsTestHelpers.kt b/...com/x8bit/bitwarden/ui/vault/feature/addedit/util/PasskeyAttestationOptionsTestHelpers.kt
@@ -11,7 +11,8 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement
 @Suppress("MaxLineLength")
 fun createMockPasskeyAttestationOptions(
     number: Int,
-    userVerificationRequirement: UserVerificationRequirement? = null,
+    userVerificationRequirement: UserVerificationRequirement =
+        UserVerificationRequirement.PREFERRED,
 ) = PasskeyAttestationOptions(
     authenticatorSelection = PasskeyAttestationOptions
         .AuthenticatorSelectionCriteria(userVerification = userVerificationRequirement),

diff --git a/...st/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/...st/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt
@@ -2455,69 +2455,6 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
             }
         }
 
-    @Suppress("MaxLineLength")
-    @Test
-    fun `Fido2AssertionRequest should show error dialog when user is not verified and verification is null`() =
-        runTest {
-            val mockAssertionRequest = createMockFido2CredentialAssertionRequest(number = 1)
-                .copy(cipherId = "mockId-1")
-            val mockFido2CredentialList = createMockSdkFido2CredentialList(number = 1)
-            val mockCipherView = createMockCipherView(
-                number = 1,
-                fido2Credentials = mockFido2CredentialList,
-            )
-            specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Assertion(
-                mockAssertionRequest,
-            )
-            every { authRepository.activeUserId } returns "activeUserId"
-            every {
-                fido2CredentialManager.getPasskeyAssertionOptionsOrNull(
-                    mockAssertionRequest.requestJson,
-                )
-            } returns createMockPasskeyAssertionOptions(
-                number = 1,
-                userVerificationRequirement = null,
-            )
-            every {
-                vaultRepository
-                    .ciphersStateFlow
-                    .value
-                    .data
-            } returns listOf(
-                createMockCipherView(
-                    number = 1,
-                    fido2Credentials = mockFido2CredentialList,
-                ),
-            )
-            coEvery {
-                fido2CredentialManager.authenticateFido2Credential(
-                    userId = "activeUserId",
-                    request = mockAssertionRequest,
-                    selectedCipherView = mockCipherView,
-                )
-            } returns Fido2CredentialAssertionResult.Success(responseJson = "responseJson")
-
-            val viewModel = createVaultItemListingViewModel()
-            viewModel.eventFlow.test {
-                assertEquals(
-                    VaultItemListingState.DialogState.Fido2OperationFail(
-                        title = R.string.an_error_has_occurred.asText(),
-                        message = R.string.passkey_operation_failed_because_user_could_not_be_verified
-                            .asText(),
-                    ),
-                    viewModel.stateFlow.value.dialogState,
-                )
-                verify { fido2CredentialManager.isUserVerified }
-                coVerify(exactly = 0) {
-                    fido2CredentialManager.authenticateFido2Credential(
-                        userId = "activeUserId",
-                        request = mockAssertionRequest,
-                        selectedCipherView = mockCipherView,
-                    )
-                }
-            }
-        }
-
     @Suppress("MaxLineLength")
     @Test
     fun `Fido2AssertionRequest should show error dialog when assertion options are null`() =