diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemCardContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemCardContent.kt index 4dc03f794..781a548cc 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemCardContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemCardContent.kt @@ -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 = { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt index 88bdfb217..5952438f2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt @@ -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. diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/handlers/VaultCardItemTypeHandlers.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/handlers/VaultCardItemTypeHandlers.kt index cb84fb388..98af99319 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/handlers/VaultCardItemTypeHandlers.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/handlers/VaultCardItemTypeHandlers.kt @@ -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)) + }, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt index d7d750920..e40e7dc0a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt @@ -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, + ) + }, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt index 901f76f89..961836564 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt @@ -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 = "number", + 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 = number, + 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 = "the number", + 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 = "", + 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 = diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt index ab385266e..9f5ed891d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt @@ -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 { + 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 { + 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 { + 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 { + 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 = "12345436", + 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 =