Add password reprompt for card number and code (#3350)

This commit is contained in:
David Perez 2024-06-24 15:08:38 -05:00 committed by GitHub
parent 94b56f624f
commit 949768ac95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 467 additions and 26 deletions

View file

@ -75,13 +75,15 @@ fun VaultItemCardContent(
)
}
}
cardState.number?.let { number ->
cardState.number?.let { numberData ->
item {
Spacer(modifier = Modifier.height(8.dp))
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.number),
value = number,
value = numberData.number,
onValueChange = {},
showPassword = numberData.isVisible,
showPasswordChange = vaultCardItemTypeHandlers.onShowNumberClick,
readOnly = true,
singleLine = false,
actions = {
@ -137,13 +139,15 @@ fun VaultItemCardContent(
}
}
cardState.securityCode?.let { securityCode ->
cardState.securityCode?.let { securityCodeData ->
item {
Spacer(modifier = Modifier.height(8.dp))
BitwardenPasswordFieldWithActions(
label = stringResource(id = R.string.security_code),
value = securityCode,
value = securityCodeData.code,
onValueChange = {},
showPassword = securityCodeData.isVisible,
showPasswordChange = vaultCardItemTypeHandlers.onShowSecurityCodeClick,
readOnly = true,
singleLine = false,
actions = {

View file

@ -679,14 +679,58 @@ class VaultItemViewModel @Inject constructor(
private fun handleCardTypeActions(action: VaultItemAction.ItemType.Card) {
when (action) {
is VaultItemAction.ItemType.Card.CodeVisibilityClick -> {
handleCodeVisibilityClick(action)
}
VaultItemAction.ItemType.Card.CopyNumberClick -> handleCopyNumberClick()
VaultItemAction.ItemType.Card.CopySecurityCodeClick -> handleCopySecurityCodeClick()
is VaultItemAction.ItemType.Card.NumberVisibilityClick -> {
handleNumberVisibilityClick(action)
}
}
}
private fun handleCodeVisibilityClick(
action: VaultItemAction.ItemType.Card.CodeVisibilityClick,
) {
onCardContent { content, card ->
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.ViewCodeClick(
isVisible = action.isVisible,
),
),
)
}
return@onCardContent
}
mutableStateFlow.update { currentState ->
currentState.copy(
viewState = content.copy(
type = card.copy(
securityCode = card.securityCode?.copy(
isVisible = action.isVisible,
),
),
),
)
}
if (action.isVisible) {
organizationEventManager.trackEvent(
event = OrganizationEvent.CipherClientToggledCardCodeVisible(
cipherId = state.vaultItemId,
),
)
}
}
}
private fun handleCopyNumberClick() {
onCardContent { content, card ->
val number = requireNotNull(card.number)
val number = requireNotNull(card.number).number
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
@ -703,7 +747,7 @@ class VaultItemViewModel @Inject constructor(
private fun handleCopySecurityCodeClick() {
onCardContent { content, card ->
val securityCode = requireNotNull(card.securityCode)
val securityCode = requireNotNull(card.securityCode).code
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
@ -718,6 +762,43 @@ class VaultItemViewModel @Inject constructor(
}
}
private fun handleNumberVisibilityClick(
action: VaultItemAction.ItemType.Card.NumberVisibilityClick,
) {
onCardContent { content, card ->
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.ViewNumberClick(
isVisible = action.isVisible,
),
),
)
}
return@onCardContent
}
mutableStateFlow.update { currentState ->
currentState.copy(
viewState = content.copy(
type = card.copy(
number = card.number?.copy(
isVisible = action.isVisible,
),
),
),
)
}
if (action.isVisible) {
organizationEventManager.trackEvent(
event = OrganizationEvent.CipherClientToggledCardNumberVisible(
cipherId = state.vaultItemId,
),
)
}
}
}
//endregion Card Type Handlers
//region Internal Type Handlers
@ -1276,11 +1357,36 @@ data class VaultItemState(
*/
data class Card(
val cardholderName: String?,
val number: String?,
val number: NumberData?,
val brand: VaultCardBrand?,
val expiration: String?,
val securityCode: String?,
) : ItemType()
val securityCode: CodeData?,
) : ItemType() {
/**
* A wrapper for the number data.
*
* @property number The card number itself.
* @property isVisible Whether or not it is currently visible.
*/
@Parcelize
data class NumberData(
val number: String,
val isVisible: Boolean,
) : Parcelable
/**
* A wrapper for the code data.
*
* @property code The security code itself.
* @property isVisible Whether or not it is currently visible.
*/
@Parcelize
data class CodeData(
val code: String,
val isVisible: Boolean,
) : Parcelable
}
}
}
}
@ -1581,6 +1687,11 @@ sealed class VaultItemAction {
*/
sealed class Card : ItemType() {
/**
* The user has clicked to display the code.
*/
data class CodeVisibilityClick(val isVisible: Boolean) : Card()
/**
* The user has clicked the copy button for the number.
*/
@ -1590,6 +1701,11 @@ sealed class VaultItemAction {
* The user has clicked the copy button for the security code.
*/
data object CopySecurityCodeClick : Card()
/**
* The user has clicked to display the Number.
*/
data class NumberVisibilityClick(val isVisible: Boolean) : Card()
}
}
@ -1755,6 +1871,30 @@ sealed class PasswordRepromptAction : Parcelable {
)
}
/**
* Indicates that we should launch the [VaultItemAction.ItemType.Card.CodeVisibilityClick]
* upon password validation.
*/
@Parcelize
data class ViewCodeClick(
val isVisible: Boolean,
) : PasswordRepromptAction() {
override val vaultItemAction: VaultItemAction
get() = VaultItemAction.ItemType.Card.CodeVisibilityClick(isVisible = isVisible)
}
/**
* Indicates that we should launch the [VaultItemAction.ItemType.Card.NumberVisibilityClick]
* upon password validation.
*/
@Parcelize
data class ViewNumberClick(
val isVisible: Boolean,
) : PasswordRepromptAction() {
override val vaultItemAction: VaultItemAction
get() = VaultItemAction.ItemType.Card.NumberVisibilityClick(isVisible = isVisible)
}
/**
* Indicates that we should launch the
* [VaultItemAction.ItemType.Login.PasswordVisibilityClicked] upon password validation.

View file

@ -10,6 +10,8 @@ import com.x8bit.bitwarden.ui.vault.feature.item.VaultItemViewModel
data class VaultCardItemTypeHandlers(
val onCopyNumberClick: () -> Unit,
val onCopySecurityCodeClick: () -> Unit,
val onShowNumberClick: (Boolean) -> Unit,
val onShowSecurityCodeClick: (Boolean) -> Unit,
) {
companion object {
@ -24,6 +26,14 @@ data class VaultCardItemTypeHandlers(
onCopySecurityCodeClick = {
viewModel.trySendAction(VaultItemAction.ItemType.Card.CopySecurityCodeClick)
},
onShowNumberClick = {
viewModel.trySendAction(
VaultItemAction.ItemType.Card.NumberVisibilityClick(it),
)
},
onShowSecurityCodeClick = {
viewModel.trySendAction(VaultItemAction.ItemType.Card.CodeVisibilityClick(it))
},
)
}
}

View file

@ -115,10 +115,20 @@ fun CipherView.toViewState(
CipherType.CARD -> {
VaultItemState.ViewState.Content.ItemType.Card(
cardholderName = card?.cardholderName,
number = card?.number,
number = card?.number?.let {
VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = it,
isVisible = false,
)
},
brand = card?.cardBrand,
expiration = card?.expiration,
securityCode = card?.code,
securityCode = card?.code?.let {
VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = it,
isVisible = false,
)
},
)
}

View file

@ -1837,14 +1837,17 @@ class VaultItemScreenTest : BaseComposeTest() {
}
@Test
fun `in card state the number should be displayed according to state`() {
fun `in card state, on show number click should send NumberVisibilityClick`() {
composeTestRule.assertScrollableNodeDoesNotExist("Number")
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "number",
isVisible = false,
),
),
),
)
@ -1862,6 +1865,54 @@ class VaultItemScreenTest : BaseComposeTest() {
.assertIsDisplayed()
.performClick()
verify(exactly = 1) {
viewModel.trySendAction(
VaultItemAction.ItemType.Card.NumberVisibilityClick(isVisible = true),
)
}
}
@Test
fun `in card state the number should be displayed according to state`() {
composeTestRule.assertScrollableNodeDoesNotExist("Number")
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "number",
isVisible = false,
),
),
),
)
}
composeTestRule
.onNodeWithTextAfterScroll("Number")
.assertTextEquals("Number", "••••••")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Copy number")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Show")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "number",
isVisible = true,
),
),
),
)
}
composeTestRule
.onNodeWithText("Number")
.assertTextEquals("Number", "number")
@ -1881,7 +1932,10 @@ class VaultItemScreenTest : BaseComposeTest() {
currentState.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = number,
isVisible = false,
),
expiration = "test",
),
),
@ -1927,14 +1981,17 @@ class VaultItemScreenTest : BaseComposeTest() {
}
@Test
fun `in card state the security code should be displayed according to state`() {
fun `in card state, on show code click should send CodeVisibilityClick`() {
composeTestRule.assertScrollableNodeDoesNotExist("Security code")
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
securityCode = "123",
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "123",
isVisible = false,
),
),
),
)
@ -1953,6 +2010,55 @@ class VaultItemScreenTest : BaseComposeTest() {
.assertIsDisplayed()
.performClick()
verify(exactly = 1) {
viewModel.trySendAction(
VaultItemAction.ItemType.Card.CodeVisibilityClick(isVisible = true),
)
}
}
@Test
fun `in card state the security code should be displayed according to state`() {
composeTestRule.assertScrollableNodeDoesNotExist("Security code")
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "123",
isVisible = false,
),
),
),
)
}
composeTestRule
.onNodeWithTextAfterScroll("Security code")
.assertTextEquals("Security code", "•••")
.assertIsEnabled()
composeTestRule
.onNodeWithContentDescription("Copy security code")
.assertIsDisplayed()
composeTestRule
.onAllNodesWithContentDescription("Show")
.onLast()
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "123",
isVisible = true,
),
),
),
)
}
composeTestRule
.onNodeWithText("Security code")
.assertTextEquals("Security code", "123")
@ -1967,12 +2073,15 @@ class VaultItemScreenTest : BaseComposeTest() {
@Test
fun `in card state, on copy security code click should send CopySecurityCodeClick`() {
val number = "1234"
val code = "1234"
mutableStateFlow.update { currentState ->
currentState.copy(
viewState = EMPTY_CARD_VIEW_STATE.copy(
type = EMPTY_CARD_TYPE.copy(
securityCode = number,
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = code,
isVisible = false,
),
),
),
)
@ -2167,10 +2276,16 @@ private val DEFAULT_IDENTITY: VaultItemState.ViewState.Content.ItemType.Identity
private val DEFAULT_CARD: VaultItemState.ViewState.Content.ItemType.Card =
VaultItemState.ViewState.Content.ItemType.Card(
cardholderName = "the cardholder name",
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "the number",
isVisible = false,
),
brand = VaultCardBrand.VISA,
expiration = "the expiration",
securityCode = "the security code",
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "the security code",
isVisible = false,
),
)
private val EMPTY_COMMON: VaultItemState.ViewState.Content.Common =
@ -2212,10 +2327,16 @@ private val EMPTY_IDENTITY_TYPE: VaultItemState.ViewState.Content.ItemType.Ident
private val EMPTY_CARD_TYPE: VaultItemState.ViewState.Content.ItemType.Card =
VaultItemState.ViewState.Content.ItemType.Card(
cardholderName = "",
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "",
isVisible = false,
),
brand = VaultCardBrand.SELECT,
expiration = "",
securityCode = "",
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "",
isVisible = false,
),
)
private val EMPTY_LOGIN_VIEW_STATE: VaultItemState.ViewState.Content =

View file

@ -1925,7 +1925,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
cardState.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.CopyClick(
value = requireNotNull(DEFAULT_CARD_TYPE.number),
value = requireNotNull(DEFAULT_CARD_TYPE.number).number,
),
),
),
@ -1972,6 +1972,81 @@ class VaultItemViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `on NumberVisibilityClick should show password dialog when re-prompt is required`() =
runTest {
val cardState = DEFAULT_STATE.copy(viewState = CARD_VIEW_STATE)
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
} returns CARD_VIEW_STATE
}
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
assertEquals(cardState, viewModel.stateFlow.value)
viewModel.trySendAction(
VaultItemAction.ItemType.Card.NumberVisibilityClick(isVisible = true),
)
assertEquals(
cardState.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.ViewNumberClick(isVisible = true),
),
),
viewModel.stateFlow.value,
)
verify(exactly = 1) {
mockCipherView.toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `on NumberVisibilityClick should call trackEvent on the OrganizationEventManager and update the ViewState when re-prompt is not required`() {
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
} returns createViewState(
common = DEFAULT_COMMON.copy(requiresReprompt = false),
type = DEFAULT_CARD_TYPE,
)
}
every { clipboardManager.setText(text = "12345436") } just runs
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
viewModel.trySendAction(
VaultItemAction.ItemType.Card.NumberVisibilityClick(isVisible = true),
)
verify(exactly = 1) {
organizationEventManager.trackEvent(
event = OrganizationEvent.CipherClientToggledCardNumberVisible(
cipherId = VAULT_ITEM_ID,
),
)
mockCipherView.toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
}
}
@Test
fun `on CopySecurityCodeClick should show password dialog when re-prompt is required`() =
runTest {
@ -1994,7 +2069,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
cardState.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.CopyClick(
value = requireNotNull(DEFAULT_CARD_TYPE.securityCode),
value = requireNotNull(DEFAULT_CARD_TYPE.securityCode).code,
),
),
),
@ -2040,6 +2115,81 @@ class VaultItemViewModelTest : BaseViewModelTest() {
)
}
}
@Test
fun `on CodeVisibilityClick should show password dialog when re-prompt is required`() =
runTest {
val cardState = DEFAULT_STATE.copy(viewState = CARD_VIEW_STATE)
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
} returns CARD_VIEW_STATE
}
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
assertEquals(cardState, viewModel.stateFlow.value)
viewModel.trySendAction(
VaultItemAction.ItemType.Card.CodeVisibilityClick(isVisible = true),
)
assertEquals(
cardState.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.ViewCodeClick(isVisible = true),
),
),
viewModel.stateFlow.value,
)
verify(exactly = 1) {
mockCipherView.toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `on CodeVisibilityClick should call trackEvent on the OrganizationEventManager and update the ViewState when re-prompt is not required`() {
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
} returns createViewState(
common = DEFAULT_COMMON.copy(requiresReprompt = false),
type = DEFAULT_CARD_TYPE,
)
}
every { clipboardManager.setText(text = "987") } just runs
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
viewModel.trySendAction(
VaultItemAction.ItemType.Card.CodeVisibilityClick(isVisible = true),
)
verify(exactly = 1) {
organizationEventManager.trackEvent(
event = OrganizationEvent.CipherClientToggledCardCodeVisible(
cipherId = VAULT_ITEM_ID,
),
)
mockCipherView.toViewState(
isPremiumUser = true,
hasMasterPassword = true,
totpCodeItemData = null,
)
}
}
}
@Nested
@ -2313,10 +2463,16 @@ class VaultItemViewModelTest : BaseViewModelTest() {
private val DEFAULT_CARD_TYPE: VaultItemState.ViewState.Content.ItemType.Card =
VaultItemState.ViewState.Content.ItemType.Card(
cardholderName = "mockName",
number = VaultItemState.ViewState.Content.ItemType.Card.NumberData(
number = "12345436",
isVisible = false,
),
brand = VaultCardBrand.VISA,
expiration = "03/2027",
securityCode = "987",
securityCode = VaultItemState.ViewState.Content.ItemType.Card.CodeData(
code = "987",
isVisible = false,
),
)
private val DEFAULT_COMMON: VaultItemState.ViewState.Content.Common =