From fdbf92f8dcd30cfe71d5b4838b165bc912436067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Tue, 3 Nov 2026 17:46:38 +0000 Subject: [PATCH] [PM-44566] fix: Lock the Send password field when editing a Send with a password (#7559) --- .../send/addedit/AddEditSendContent.kt | 1 + .../components/AddEditSendAuthTypeChooser.kt | 83 +++++++++----- .../send/addedit/AddEditSendScreenTest.kt | 103 ++++++++++++++++++ 3 files changed, 157 insertions(+), 30 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt index 05c7705c09..846c56c43e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt @@ -177,6 +177,7 @@ fun AddEditSendContent( onPasswordCopyClick = addSendHandlers.onPasswordCopyClick, password = state.common.passwordInput, isEnabled = !policyDisablesSend && enforcedWhoCanAccess == null, + isExistingPasswordLocked = state.common.hasPassword, isSendsRestrictedByPolicy = policyDisablesSend, modifier = Modifier .testTag("SendAuthTypeChooser") diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/components/AddEditSendAuthTypeChooser.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/components/AddEditSendAuthTypeChooser.kt index a254df389c..519611b6fb 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/components/AddEditSendAuthTypeChooser.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/components/AddEditSendAuthTypeChooser.kt @@ -19,6 +19,7 @@ import com.bitwarden.ui.platform.base.util.cardStyle import com.bitwarden.ui.platform.components.button.BitwardenStandardIconButton import com.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog import com.bitwarden.ui.platform.components.dropdown.BitwardenMultiSelectButton +import com.bitwarden.ui.platform.components.field.BitwardenHiddenPasswordField import com.bitwarden.ui.platform.components.field.BitwardenPasswordField import com.bitwarden.ui.platform.components.field.BitwardenTextField import com.bitwarden.ui.platform.components.model.CardStyle @@ -31,6 +32,13 @@ import com.x8bit.bitwarden.ui.tools.feature.send.addedit.model.SendAuth import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toPersistentList +/** + * Stand-in value for a locked password field. The existing password is never readable, and + * [BitwardenHiddenPasswordField] masks any non-empty value with a fixed number of dots, so this + * only signals that a password is set. + */ +private const val LOCKED_PASSWORD_PLACEHOLDER: String = "locked" + /** * Displays UX for choosing authentication type for a send. * @@ -42,10 +50,12 @@ import kotlinx.collections.immutable.toPersistentList * (only relevant for [SendAuth.Email]). * @param onAddNewEmailClick Callback invoked when adding a new email. * @param onRemoveEmailClick Callback invoked when removing an email. - * @param password The current password value (only relevant for [SendAuth.Password]). * @param onOpenPasswordGeneratorClick Callback invoked when the Generator button is clicked * @param onPasswordCopyClick Callback invoked when the Copy button is clicked + * @param password The current password value (only relevant for [SendAuth.Password]). * @param isEnabled Whether the chooser is enabled. + * @param isExistingPasswordLocked Whether the send already has a password that cannot be + * overwritten and must be removed before a new one can be set. * @param isSendsRestrictedByPolicy if sends are restricted by a policy. * @param modifier Modifier for the composable. */ @@ -62,6 +72,7 @@ fun AddEditSendAuthTypeChooser( onPasswordCopyClick: (String) -> Unit, password: String, isEnabled: Boolean, + isExistingPasswordLocked: Boolean, isSendsRestrictedByPolicy: Boolean, modifier: Modifier = Modifier, ) { @@ -113,36 +124,48 @@ fun AddEditSendAuthTypeChooser( } is SendAuth.Password -> { - BitwardenPasswordField( - label = stringResource(id = BitwardenString.password), - value = password, - onValueChange = onPasswordChange, - cardStyle = CardStyle.Bottom, - passwordFieldTestTag = "SendPasswordEntry", - readOnly = isSendsRestrictedByPolicy, - modifier = Modifier.fillMaxWidth(), - ) { - BitwardenStandardIconButton( - vectorIconRes = BitwardenDrawable.ic_generate, - contentDescription = stringResource(id = BitwardenString.generate_password), - onClick = { - if (password.isEmpty()) { - onOpenPasswordGeneratorClick() - } else { - shouldShowDialog = true - } - }, - modifier = Modifier.testTag(tag = "RegeneratePasswordButton"), - ) - BitwardenStandardIconButton( - vectorIconRes = BitwardenDrawable.ic_copy, - contentDescription = stringResource(id = BitwardenString.copy_password), - isEnabled = password.isNotEmpty(), - onClick = { - onPasswordCopyClick(password) - }, - modifier = Modifier.testTag(tag = "CopyPasswordButton"), + if (isExistingPasswordLocked) { + BitwardenHiddenPasswordField( + label = stringResource(id = BitwardenString.password), + value = LOCKED_PASSWORD_PLACEHOLDER, + cardStyle = CardStyle.Bottom, + passwordFieldTestTag = "SendPasswordEntry", + modifier = Modifier.fillMaxWidth(), ) + } else { + BitwardenPasswordField( + label = stringResource(id = BitwardenString.password), + value = password, + onValueChange = onPasswordChange, + cardStyle = CardStyle.Bottom, + passwordFieldTestTag = "SendPasswordEntry", + readOnly = isSendsRestrictedByPolicy, + modifier = Modifier.fillMaxWidth(), + ) { + BitwardenStandardIconButton( + vectorIconRes = BitwardenDrawable.ic_generate, + contentDescription = stringResource( + id = BitwardenString.generate_password, + ), + onClick = { + if (password.isEmpty()) { + onOpenPasswordGeneratorClick() + } else { + shouldShowDialog = true + } + }, + modifier = Modifier.testTag(tag = "RegeneratePasswordButton"), + ) + BitwardenStandardIconButton( + vectorIconRes = BitwardenDrawable.ic_copy, + contentDescription = stringResource(id = BitwardenString.copy_password), + isEnabled = password.isNotEmpty(), + onClick = { + onPasswordCopyClick(password) + }, + modifier = Modifier.testTag(tag = "CopyPasswordButton"), + ) + } } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt index 83925bc16e..6daafa40a2 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt @@ -213,6 +213,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, sendAuth = SendAuth.Password, passwordInput = "somePass", ), @@ -237,6 +238,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, sendAuth = SendAuth.Password, passwordInput = "", ), @@ -259,6 +261,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, sendAuth = SendAuth.Password, ), ), @@ -282,6 +285,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, sendAuth = SendAuth.Password, passwordInput = "somePass", ), @@ -298,6 +302,97 @@ class AddEditSendScreenTest : BitwardenComposeTest() { .assertIsDisplayed() } + @Test + fun `password field should be disabled when the send already has a password`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy( + hasPassword = true, + sendAuth = SendAuth.Password, + ), + ), + ) + } + + composeTestRule + .onNodeWithTag(testTag = "SendPasswordEntry") + .performScrollTo() + .assertTextEquals("Password", "••••••••") + .assertIsNotEnabled() + .assert(!hasSetTextAction()) + } + + @Test + fun `password field should be editable when the send has no password`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, + sendAuth = SendAuth.Password, + ), + ), + ) + } + + composeTestRule + .onNodeWithTag(testTag = "SendPasswordEntry") + .performScrollTo() + .assert(hasSetTextAction()) + } + + @Test + fun `password action buttons should be hidden when the send already has a password`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy( + hasPassword = true, + sendAuth = SendAuth.Password, + ), + ), + ) + } + + composeTestRule + .onNodeWithTag(testTag = "RegeneratePasswordButton") + .assertDoesNotExist() + composeTestRule + .onNodeWithTag(testTag = "CopyPasswordButton") + .assertDoesNotExist() + composeTestRule + .onNodeWithContentDescription("Show") + .assertDoesNotExist() + } + + @Test + fun `password action buttons should be displayed when the send has no password`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, + sendAuth = SendAuth.Password, + ), + ), + ) + } + + composeTestRule + .onNodeWithTag(testTag = "RegeneratePasswordButton") + .performScrollTo() + .assertIsDisplayed() + composeTestRule + .onNodeWithTag(testTag = "CopyPasswordButton") + .performScrollTo() + .assertIsDisplayed() + composeTestRule + .onNodeWithContentDescription("Show") + .performScrollTo() + .assertIsDisplayed() + } + @Test fun `on overflow button click should display overflow menu`() { mutableStateFlow.value = DEFAULT_STATE.copy( @@ -803,6 +898,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { @Test fun `password input should change according to the state`() { val commonState = DEFAULT_COMMON_STATE.copy( + hasPassword = false, sendAuth = SendAuth.Password, passwordInput = "", ) @@ -1281,6 +1377,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "", sendAuth = SendAuth.Password, ), @@ -1550,6 +1647,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "", sendAuth = SendAuth.Password, ), @@ -1574,6 +1672,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "existing-password", sendAuth = SendAuth.Password, ), @@ -1605,6 +1704,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "existing-password", sendAuth = SendAuth.Password, ), @@ -1636,6 +1736,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "existing-password", sendAuth = SendAuth.Password, ), @@ -1667,6 +1768,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "test-password", sendAuth = SendAuth.Password, ), @@ -1691,6 +1793,7 @@ class AddEditSendScreenTest : BitwardenComposeTest() { it.copy( viewState = DEFAULT_VIEW_STATE.copy( common = DEFAULT_COMMON_STATE.copy( + hasPassword = false, passwordInput = "", sendAuth = SendAuth.Password, ),