[PM-9407] Confirm overwrite existing passkey in edit mode (#3542)

2024-10-31 07:05:35 +03:00 · 2024-07-18 12:53:15 -04:00 · 2024-07-18 12:53:15 -04:00 · 1ea1e7918b
commit 1ea1e7918b
parent 815e779475
4 changed files with 263 additions and 4 deletions
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt
@ -35,6 +35,7 @@ import com.x8bit.bitwarden.ui.platform.components.content.BitwardenLoadingConten
 import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
 import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenBasicDialog
 import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenLoadingDialog
+import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenOverwritePasskeyConfirmationDialog
 import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
 import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
 import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
@ -168,6 +169,13 @@ fun VaultAddEditScreen(
        onFido2ErrorDismiss = remember(viewModel) {
            { viewModel.trySendAction(VaultAddEditAction.Common.Fido2ErrorDialogDismissed) }
        },
+        onConfirmOverwriteExistingPasskey = remember(viewModel) {
+            {
+                viewModel.trySendAction(
+                    action = VaultAddEditAction.Common.ConfirmOverwriteExistingPasskeyClick,
+                )
+            }
+        },
    )

    if (pendingDeleteCipher) {
@ -302,6 +310,7 @@ private fun VaultAddEditItemDialogs(
    onDismissRequest: () -> Unit,
    onAutofillDismissRequest: () -> Unit,
    onFido2ErrorDismiss: () -> Unit,
+    onConfirmOverwriteExistingPasskey: () -> Unit,
 ) {
    when (dialogState) {
        is VaultAddEditState.DialogState.Loading -> {
@ -340,6 +349,13 @@ private fun VaultAddEditItemDialogs(
            )
        }

+        is VaultAddEditState.DialogState.OverwritePasskeyConfirmationPrompt -> {
+            BitwardenOverwritePasskeyConfirmationDialog(
+                onConfirmClick = onConfirmOverwriteExistingPasskey,
+                onDismissRequest = onDismissRequest,
+            )
+        }
+
        null -> Unit
    }
 }
--- 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
@ -12,6 +12,7 @@ import com.x8bit.bitwarden.data.autofill.fido2.datasource.network.model.PublicKe
 import com.x8bit.bitwarden.data.autofill.fido2.manager.Fido2CredentialManager
 import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialRequest
 import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult
+import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials
 import com.x8bit.bitwarden.data.platform.manager.PolicyManager
 import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager
 import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager
@ -256,6 +257,10 @@ class VaultAddEditViewModel @Inject constructor(
                handleHiddenFieldVisibilityChange(action)
            }

+            is VaultAddEditAction.Common.ConfirmOverwriteExistingPasskeyClick -> {
+                handleConfirmOverwriteExistingPasskeyClick()
+            }
+
            VaultAddEditAction.Common.UserVerificationSuccess -> {
                handleUserVerificationSuccess()
            }
@ -367,7 +372,7 @@ class VaultAddEditViewModel @Inject constructor(
        specialCircumstanceManager.specialCircumstance
            ?.toFido2RequestOrNull()
            ?.let { request ->
-                registerFido2Credential(request, content)
+                handleFido2RequestSpecialCircumstance(request, content.toCipherView())
                return@onContent
            }

@ -394,13 +399,26 @@ class VaultAddEditViewModel @Inject constructor(
        }
    }

+    private fun handleFido2RequestSpecialCircumstance(
+        request: Fido2CredentialRequest,
+        cipherView: CipherView,
+    ) {
+        if (cipherView.isActiveWithFido2Credentials) {
+            mutableStateFlow.update {
+                it.copy(dialog = VaultAddEditState.DialogState.OverwritePasskeyConfirmationPrompt)
+            }
+        } else {
+            registerFido2Credential(request, cipherView)
+        }
+    }
+
    private fun registerFido2Credential(
        request: Fido2CredentialRequest,
-        content: VaultAddEditState.ViewState.Content,
+        cipherView: CipherView,
    ) {

        if (fido2CredentialManager.isUserVerified) {
-            registerFido2CredentialToCipher(request, content.toCipherView())
+            registerFido2CredentialToCipher(request, cipherView)
            return
        }

@ -413,7 +431,7 @@ class VaultAddEditViewModel @Inject constructor(

        when (createOptions.authenticatorSelection.userVerification) {
            UserVerificationRequirement.DISCOURAGED -> {
-                registerFido2CredentialToCipher(request, content.toCipherView())
+                registerFido2CredentialToCipher(request, cipherView)
            }

            UserVerificationRequirement.PREFERRED -> {
@ -519,6 +537,18 @@ class VaultAddEditViewModel @Inject constructor(
        }
    }

+    private fun handleConfirmOverwriteExistingPasskeyClick() {
+        specialCircumstanceManager
+            .specialCircumstance
+            ?.toFido2RequestOrNull()
+            ?.let { request ->
+                onContent { content ->
+                    registerFido2Credential(request, content.toCipherView())
+                }
+            }
+            ?: showFido2ErrorDialog()
+    }
+
    private fun handleUserVerificationLockOut() {
        fido2CredentialManager.isUserVerified = false
        showFido2ErrorDialog()
@ -2049,6 +2079,12 @@ data class VaultAddEditState(
         */
        @Parcelize
        data class Fido2Error(val message: Text) : DialogState()
+
+        /**
+         * Displays the overwrite passkey confirmation prompt to the user.
+         */
+        @Parcelize
+        data object OverwritePasskeyConfirmationPrompt : DialogState()
    }
 }

@ -2186,6 +2222,11 @@ sealed class VaultAddEditAction {
         */
        data object ConfirmDeleteClick : Common()

+        /**
+         * The user has confirmed overwriting the existing passkey.
+         */
+        data object ConfirmOverwriteExistingPasskeyClick : Common()
+
        /**
         * Represents the action when a type option is selected.
         *
--- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt
@ -3009,6 +3009,32 @@ class VaultAddEditScreenTest : BaseComposeTest() {
        verify { viewModel.trySendAction(VaultAddEditAction.Common.UserVerificationNotSupported) }
    }

+    @Suppress("MaxLineLength")
+    @Test
+    fun `OverwritePasskeyConfirmationPrompt should display based on dialog state and send ConfirmOverwriteExistingPasskeyClick on Ok click`() {
+        val stateWithDialog = DEFAULT_STATE_LOGIN
+            .copy(dialog = VaultAddEditState.DialogState.OverwritePasskeyConfirmationPrompt)
+
+        mutableStateFlow.value = stateWithDialog
+
+        composeTestRule
+            .onNodeWithText("Overwrite passkey?")
+            .assertIsDisplayed()
+            .assert(hasAnyAncestor(isDialog()))
+        composeTestRule
+            .onNodeWithText("This item already contains a passkey. Are you sure you want to overwrite the current passkey?")
+            .assertIsDisplayed()
+            .assert(hasAnyAncestor(isDialog()))
+        composeTestRule
+            .onAllNodesWithText("Ok")
+            .filterToOne(hasAnyAncestor(isDialog()))
+            .performClick()
+
+        verify {
+            viewModel.trySendAction(VaultAddEditAction.Common.ConfirmOverwriteExistingPasskeyClick)
+        }
+    }
+
    //region Helper functions

    private fun updateLoginType(
--- 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
@ -38,6 +38,7 @@ import com.x8bit.bitwarden.data.tools.generator.repository.util.FakeGeneratorRep
 import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView
+import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSdkFido2CredentialList
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
 import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
 import com.x8bit.bitwarden.data.vault.repository.model.DeleteCipherResult
@ -1393,6 +1394,181 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
            }
        }

+    @Suppress("MaxLineLength")
+    @Test
+    fun `in edit mode during FIDO 2 registration, SaveClick should display ConfirmOverwriteExistingPasskeyDialog when original cipher has a passkey`() {
+        val cipherView = createMockCipherView(
+            number = 1,
+            fido2Credentials = createMockSdkFido2CredentialList(number = 1),
+        )
+        val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
+        val stateWithName = createVaultAddItemState(
+            vaultAddEditType = vaultAddEditType,
+            commonContentViewState = createCommonContentViewState(
+                name = "mockName-1",
+                originalCipher = cipherView,
+                customFieldData = listOf(
+                    VaultAddEditState.Custom.HiddenField(
+                        itemId = "testId",
+                        name = "mockName-1",
+                        value = "mockValue-1",
+                    ),
+                ),
+                notes = "mockNotes-1",
+            ),
+        )
+        specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save(
+            fido2CredentialRequest = createMockFido2CredentialRequest(number = 1),
+        )
+        every {
+            cipherView.toViewState(
+                isClone = false,
+                isIndividualVaultDisabled = false,
+                resourceManager = resourceManager,
+                clock = fixedClock,
+            )
+        } returns stateWithName.viewState
+        mutableVaultDataFlow.value = DataState.Loaded(
+            createVaultData(cipherView = cipherView),
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithName,
+                vaultAddEditType = vaultAddEditType,
+            ),
+        )
+
+        viewModel.trySendAction(VaultAddEditAction.Common.SaveClick)
+
+        assertEquals(
+            VaultAddEditState.DialogState.OverwritePasskeyConfirmationPrompt,
+            viewModel.stateFlow.value.dialog,
+        )
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `ConfirmOverwriteExistingPasskeyClick should register credential when user is verified`() {
+        val cipherView = createMockCipherView(
+            number = 1,
+            fido2Credentials = createMockSdkFido2CredentialList(number = 1),
+        )
+        val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
+        val stateWithName = createVaultAddItemState(
+            vaultAddEditType = vaultAddEditType,
+            commonContentViewState = createCommonContentViewState(
+                name = "mockName-1",
+                originalCipher = cipherView,
+                notes = "mockNotes-1",
+            ),
+        )
+        val mockFidoRequest = createMockFido2CredentialRequest(number = 1)
+        specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save(
+            fido2CredentialRequest = mockFidoRequest,
+        )
+        coEvery {
+            fido2CredentialManager.registerFido2Credential(
+                userId = mockFidoRequest.userId,
+                fido2CredentialRequest = mockFidoRequest,
+                selectedCipherView = any(),
+            )
+        } returns Fido2RegisterCredentialResult.Success("mockResponse")
+        every { authRepository.activeUserId } returns mockFidoRequest.userId
+        every {
+            cipherView.toViewState(
+                isClone = false,
+                isIndividualVaultDisabled = false,
+                resourceManager = resourceManager,
+                clock = fixedClock,
+            )
+        } returns stateWithName.viewState
+        every { fido2CredentialManager.isUserVerified } returns true
+
+        mutableVaultDataFlow.value = DataState.Loaded(
+            createVaultData(cipherView = cipherView),
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithName,
+                vaultAddEditType = vaultAddEditType,
+            ),
+        )
+        viewModel.trySendAction(VaultAddEditAction.Common.ConfirmOverwriteExistingPasskeyClick)
+
+        coVerify {
+            fido2CredentialManager.isUserVerified
+            fido2CredentialManager.registerFido2Credential(
+                userId = mockFidoRequest.userId,
+                fido2CredentialRequest = mockFidoRequest,
+                selectedCipherView = any(),
+            )
+        }
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `ConfirmOverwriteExistingPasskeyClick should check if user verification is required`() =
+        runTest {
+            val cipherView = createMockCipherView(
+                number = 1,
+                fido2Credentials = createMockSdkFido2CredentialList(number = 1),
+            )
+            val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
+            val stateWithName = createVaultAddItemState(
+                vaultAddEditType = vaultAddEditType,
+                commonContentViewState = createCommonContentViewState(
+                    name = "mockName-1",
+                    originalCipher = cipherView,
+                    customFieldData = listOf(
+                        VaultAddEditState.Custom.HiddenField(
+                            itemId = "testId",
+                            name = "mockName-1",
+                            value = "mockValue-1",
+                        ),
+                    ),
+                    notes = "mockNotes-1",
+                ),
+            )
+            val mockFidoRequest = createMockFido2CredentialRequest(number = 1)
+            specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save(
+                fido2CredentialRequest = mockFidoRequest,
+            )
+            every {
+                cipherView.toViewState(
+                    isClone = false,
+                    isIndividualVaultDisabled = false,
+                    resourceManager = resourceManager,
+                    clock = fixedClock,
+                )
+            } returns stateWithName.viewState
+            every { fido2CredentialManager.isUserVerified } returns false
+            every {
+                fido2CredentialManager.getPasskeyCreateOptionsOrNull(any())
+            } returns createMockPublicKeyCredentialCreationOptions(
+                number = 1,
+                userVerificationRequirement = UserVerificationRequirement.REQUIRED,
+            )
+
+            mutableVaultDataFlow.value = DataState.Loaded(
+                createVaultData(cipherView = cipherView),
+            )
+
+            val viewModel = createAddVaultItemViewModel(
+                createSavedStateHandleWithState(
+                    state = stateWithName,
+                    vaultAddEditType = vaultAddEditType,
+                ),
+            )
+            viewModel.trySendAction(VaultAddEditAction.Common.ConfirmOverwriteExistingPasskeyClick)
+
+            coVerify {
+                fido2CredentialManager.isUserVerified
+                fido2CredentialManager.getPasskeyCreateOptionsOrNull(mockFidoRequest.requestJson)
+            }
+        }
+
    @Test
    fun `Saving item with an empty name field will cause a dialog to show up`() = runTest {
        mutableVaultDataFlow.value = DataState.Loaded(