BIT-1247: Restrict password visibility according to permissions (#487)

This commit is contained in:
Brian Yencho 2024-01-03 17:00:02 -06:00 committed by Álison Fernandes
parent c5989d117e
commit d9837a1895
12 changed files with 320 additions and 49 deletions

View file

@ -0,0 +1,60 @@
package com.x8bit.bitwarden.ui.platform.components
import androidx.compose.foundation.text.KeyboardOptions
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.OutlinedTextField
import androidx.compose.material3.OutlinedTextFieldDefaults
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.input.KeyboardType
import androidx.compose.ui.text.input.PasswordVisualTransformation
import androidx.compose.ui.tooling.preview.Preview
import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme
/**
* Represents a Bitwarden-styled password field that is completely hidden and non-interactable.
*
* @param label Label for the text field.
* @param value Current text on the text field.
* @param modifier Modifier for the composable.
*/
@Composable
fun BitwardenHiddenPasswordField(
label: String,
value: String,
modifier: Modifier = Modifier,
) {
OutlinedTextField(
modifier = modifier,
textStyle = MaterialTheme.typography.bodyLarge,
label = { Text(text = label) },
value = value,
onValueChange = { },
visualTransformation = PasswordVisualTransformation(),
singleLine = true,
enabled = false,
readOnly = true,
keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password),
colors = OutlinedTextFieldDefaults.colors(
disabledTextColor = MaterialTheme.colorScheme.onSurface,
disabledBorderColor = MaterialTheme.colorScheme.outline,
disabledLeadingIconColor = MaterialTheme.colorScheme.onSurfaceVariant,
disabledTrailingIconColor = MaterialTheme.colorScheme.onSurfaceVariant,
disabledLabelColor = MaterialTheme.colorScheme.onSurfaceVariant,
disabledPlaceholderColor = MaterialTheme.colorScheme.onSurfaceVariant,
disabledSupportingTextColor = MaterialTheme.colorScheme.onSurfaceVariant,
),
)
}
@Preview
@Composable
private fun BitwardenHiddenPasswordField_preview() {
BitwardenTheme {
BitwardenHiddenPasswordField(
label = "Label",
value = "Password",
)
}
}

View file

@ -16,6 +16,7 @@ import androidx.compose.ui.unit.dp
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.ui.platform.components.BitwardenFilledTonalButton
import com.x8bit.bitwarden.ui.platform.components.BitwardenFilledTonalButtonWithIcon
import com.x8bit.bitwarden.ui.platform.components.BitwardenHiddenPasswordField
import com.x8bit.bitwarden.ui.platform.components.BitwardenIconButtonWithResource
import com.x8bit.bitwarden.ui.platform.components.BitwardenListHeaderText
import com.x8bit.bitwarden.ui.platform.components.BitwardenMultiSelectButton
@ -76,26 +77,37 @@ fun LazyListScope.vaultAddEditLoginItems(
item {
Spacer(modifier = Modifier.height(8.dp))
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.password),
value = loginState.password,
onValueChange = loginItemTypeHandlers.onPasswordTextChange,
modifier = Modifier
.padding(horizontal = 16.dp),
) {
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_check_mark),
contentDescription = stringResource(id = R.string.check_password),
),
onClick = loginItemTypeHandlers.onPasswordCheckerClick,
)
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_generator),
contentDescription = stringResource(id = R.string.generate_password),
),
onClick = loginItemTypeHandlers.onOpenPasswordGeneratorClick,
if (loginState.canViewPassword) {
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.password),
value = loginState.password,
onValueChange = loginItemTypeHandlers.onPasswordTextChange,
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 16.dp),
) {
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_check_mark),
contentDescription = stringResource(id = R.string.check_password),
),
onClick = loginItemTypeHandlers.onPasswordCheckerClick,
)
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_generator),
contentDescription = stringResource(id = R.string.generate_password),
),
onClick = loginItemTypeHandlers.onOpenPasswordGeneratorClick,
)
}
} else {
BitwardenHiddenPasswordField(
label = stringResource(id = R.string.password),
value = loginState.password,
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 16.dp),
)
}
}

View file

@ -963,6 +963,9 @@ data class VaultAddEditState(
* @property username The username required for the login item.
* @property password The password required for the login item.
* @property uri The URI associated with the login item.
* @property totp The current TOTP (if applicable).
* @property canViewPassword Indicates whether the current user can view and copy
* passwords associated with the login item.
*/
@Parcelize
data class Login(
@ -970,6 +973,7 @@ data class VaultAddEditState(
val password: String = "",
val uri: String = "",
val totp: String? = null,
val canViewPassword: Boolean = true,
) : ItemType() {
override val displayStringResId: Int get() = ItemTypeOption.LOGIN.labelRes
}

View file

@ -23,6 +23,7 @@ fun CipherView.toViewState(): VaultAddEditState.ViewState =
password = login?.password.orEmpty(),
uri = login?.uris?.firstOrNull()?.uri.orEmpty(),
totp = login?.totp,
canViewPassword = this.viewPassword,
)
}

View file

@ -18,6 +18,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.unit.dp
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.ui.platform.components.BitwardenHiddenPasswordField
import com.x8bit.bitwarden.ui.platform.components.BitwardenIconButtonWithResource
import com.x8bit.bitwarden.ui.platform.components.BitwardenListHeaderText
import com.x8bit.bitwarden.ui.platform.components.BitwardenPasswordFieldWithActions
@ -234,34 +235,42 @@ private fun PasswordField(
onCopyPasswordClick: () -> Unit,
modifier: Modifier = Modifier,
) {
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.password),
value = passwordData.password,
showPasswordChange = { onShowPasswordClick(it) },
showPassword = passwordData.isVisible,
onValueChange = { },
readOnly = true,
singleLine = false,
actions = {
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_check_mark),
contentDescription = stringResource(
id = R.string.check_known_data_breaches_for_this_password,
if (passwordData.canViewPassword) {
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.password),
value = passwordData.password,
showPasswordChange = { onShowPasswordClick(it) },
showPassword = passwordData.isVisible,
onValueChange = { },
readOnly = true,
singleLine = false,
actions = {
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_check_mark),
contentDescription = stringResource(
id = R.string.check_known_data_breaches_for_this_password,
),
),
),
onClick = onCheckForBreachClick,
)
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_copy),
contentDescription = stringResource(id = R.string.copy_password),
),
onClick = onCopyPasswordClick,
)
},
modifier = modifier,
)
onClick = onCheckForBreachClick,
)
BitwardenIconButtonWithResource(
iconRes = IconResource(
iconPainter = painterResource(id = R.drawable.ic_copy),
contentDescription = stringResource(id = R.string.copy_password),
),
onClick = onCopyPasswordClick,
)
},
modifier = modifier,
)
} else {
BitwardenHiddenPasswordField(
label = stringResource(id = R.string.password),
value = passwordData.password,
modifier = modifier,
)
}
}
@Composable

View file

@ -586,13 +586,18 @@ data class VaultItemState(
) : ItemType() {
/**
* A wrapper for the password data, this includes the [password] itself
* and whether it should be visible.
* A wrapper for the password data.
*
* @property password The password itself.
* @property isVisible Whether or not it is currently visible.
* @property canViewPassword Indicates whether the current user can view and
* copy passwords associated with the login item.
*/
@Parcelize
data class PasswordData(
val password: String,
val isVisible: Boolean,
val canViewPassword: Boolean,
) : Parcelable
/**

View file

@ -46,6 +46,7 @@ fun CipherView.toViewState(
VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = it,
isVisible = false,
canViewPassword = viewPassword,
)
},
uris = loginValues.uris.orEmpty().map { it.toUriData() },

View file

@ -3,7 +3,9 @@ package com.x8bit.bitwarden.ui.vault.feature.addedit
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.platform.ClipboardManager
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsEnabled
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.assertIsOff
import androidx.compose.ui.test.assertIsOn
import androidx.compose.ui.test.assertTextContains
@ -253,6 +255,85 @@ class VaultAddEditScreenTest : BaseComposeTest() {
.assertIsDisplayed()
}
@Test
fun `in ItemType_Login state the password should change according to state`() {
composeTestRule
.onNodeWithTextAfterScroll("Password")
.assertTextEquals("Password", "")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Check if password has been exposed.")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Generate password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Show")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = VaultAddEditState.ViewState.Content(
common = VaultAddEditState.ViewState.Content.Common(),
type = VaultAddEditState.ViewState.Content.ItemType.Login(
password = "p@ssw0rd",
),
),
)
}
composeTestRule
.onNodeWithText("Password")
.assertTextEquals("Password", "••••••••")
composeTestRule
.onNodeWithContentDescription("Check if password has been exposed.")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Generate password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Show")
.assertIsDisplayed()
// Click on the visibility icon to show the password
composeTestRule
.onNodeWithContentDescription("Show")
.performClick()
composeTestRule
.onNodeWithText("Password")
.assertTextEquals("Password", "p@ssw0rd")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Hide")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = VaultAddEditState.ViewState.Content(
common = VaultAddEditState.ViewState.Content.Common(),
type = VaultAddEditState.ViewState.Content.ItemType.Login(
password = "p@ssw0rd",
canViewPassword = false,
),
),
)
}
composeTestRule
.onNodeWithText("Password")
.assertTextEquals("Password", "••••••••")
.assertIsNotEnabled()
composeTestRule
.onNodeWithContentDescription("Check if password has been exposed.")
.assertDoesNotExist()
composeTestRule
.onNodeWithContentDescription("Generate password")
.assertDoesNotExist()
composeTestRule
.onNodeWithContentDescription("Hide")
.assertDoesNotExist()
}
@Test
fun `in ItemType_Login state changing Username text field should trigger UsernameTextChange`() {
composeTestRule

View file

@ -154,6 +154,7 @@ class CipherViewExtensionsTest {
password = "password",
uri = "www.example.com",
totp = "otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example",
canViewPassword = false,
),
),
result,

View file

@ -3,7 +3,10 @@ package com.x8bit.bitwarden.ui.vault.feature.item
import androidx.compose.ui.platform.ClipboardManager
import androidx.compose.ui.test.assert
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsEnabled
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.assertTextContains
import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.filterToOne
import androidx.compose.ui.test.hasAnyAncestor
import androidx.compose.ui.test.hasContentDescription
@ -465,6 +468,95 @@ class VaultItemScreenTest : BaseComposeTest() {
}
}
@Test
fun `in login state the password should change according to state`() {
composeTestRule.assertScrollableNodeDoesNotExist("Password")
mutableStateFlow.update {
it.copy(
viewState = EMPTY_LOGIN_VIEW_STATE.copy(
type = EMPTY_LOGIN_TYPE.copy(
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "p@ssw0rd",
isVisible = false,
canViewPassword = true,
),
),
),
)
}
composeTestRule
.onNodeWithTextAfterScroll("Password")
.assertTextEquals("Password", "••••••••")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Check known data breaches for this password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Copy password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Show")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = EMPTY_LOGIN_VIEW_STATE.copy(
type = EMPTY_LOGIN_TYPE.copy(
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "p@ssw0rd",
isVisible = true,
canViewPassword = true,
),
),
),
)
}
composeTestRule
.onNodeWithText("Password")
.assertTextEquals("Password", "p@ssw0rd")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Check known data breaches for this password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Copy password")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Hide")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = EMPTY_LOGIN_VIEW_STATE.copy(
type = EMPTY_LOGIN_TYPE.copy(
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "p@ssw0rd",
isVisible = true,
canViewPassword = false,
),
),
),
)
}
composeTestRule
.onNodeWithText("Password")
.assertTextEquals("Password", "••••••••")
.assertIsNotEnabled()
composeTestRule
.onNodeWithContentDescription("Check known data breaches for this password")
.assertDoesNotExist()
composeTestRule
.onNodeWithContentDescription("Copy password")
.assertDoesNotExist()
composeTestRule
.onNodeWithContentDescription("Hide")
.assertDoesNotExist()
}
@Test
fun `in login state, linked custom fields should be displayed according to state`() {
val linkedFieldUserName =
@ -550,6 +642,7 @@ class VaultItemScreenTest : BaseComposeTest() {
val passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "12345",
isVisible = true,
canViewPassword = true,
)
mutableStateFlow.update { currentState ->
currentState.copy(
@ -592,6 +685,7 @@ class VaultItemScreenTest : BaseComposeTest() {
val passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "12345",
isVisible = true,
canViewPassword = true,
)
mutableStateFlow.update { currentState ->
currentState.copy(
@ -1092,6 +1186,7 @@ private val DEFAULT_LOGIN: VaultItemState.ViewState.Content.ItemType.Login =
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "the password",
isVisible = false,
canViewPassword = true,
),
uris = listOf(
VaultItemState.ViewState.Content.ItemType.Login.UriData(

View file

@ -670,6 +670,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = DEFAULT_LOGIN_PASSWORD,
isVisible = false,
canViewPassword = true,
),
uris = listOf(
VaultItemState.ViewState.Content.ItemType.Login.UriData(

View file

@ -181,6 +181,7 @@ fun createLoginContent(isEmpty: Boolean): VaultItemState.ViewState.Content.ItemT
passwordData = VaultItemState.ViewState.Content.ItemType.Login.PasswordData(
password = "password",
isVisible = false,
canViewPassword = false,
)
.takeUnless { isEmpty },
uris = if (isEmpty) {