BIT-1682: Add more master password reprompts to Item Listing screen (#938)

This commit is contained in:
Brian Yencho 2024-02-01 09:29:27 -06:00 committed by Álison Fernandes
parent 528b5605d8
commit 10cf094c3f
8 changed files with 237 additions and 36 deletions

View file

@ -36,7 +36,7 @@ fun VaultItemListingContent(
state: VaultItemListingState.ViewState.Content,
policyDisablesSend: Boolean,
vaultItemClick: (id: String) -> Unit,
masterPasswordRepromptSubmit: (id: String, password: String) -> Unit,
masterPasswordRepromptSubmit: (password: String, data: MasterPasswordRepromptData) -> Unit,
onOverflowItemClick: (action: ListingItemOverflowAction) -> Unit,
modifier: Modifier = Modifier,
) {
@ -76,20 +76,18 @@ fun VaultItemListingContent(
-> Unit
}
var masterPasswordRepromptCipherId by remember { mutableStateOf<String?>(null) }
if (masterPasswordRepromptCipherId != null) {
var masterPasswordRepromptData by remember { mutableStateOf<MasterPasswordRepromptData?>(null) }
masterPasswordRepromptData?.let { data ->
BitwardenMasterPasswordDialog(
onConfirmClick = {
val cipherId = masterPasswordRepromptCipherId
?: return@BitwardenMasterPasswordDialog
masterPasswordRepromptCipherId = null
onConfirmClick = { password ->
masterPasswordRepromptData = null
masterPasswordRepromptSubmit(
cipherId,
it,
password,
data,
)
},
onDismissRequest = {
masterPasswordRepromptCipherId = null
masterPasswordRepromptData = null
},
)
}
@ -123,8 +121,11 @@ fun VaultItemListingContent(
label = it.title,
supportingLabel = it.subtitle,
onClick = {
if (it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptCipherId = it.id
if (it.isAutofill && it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptData =
MasterPasswordRepromptData.Autofill(
cipherId = it.id,
)
} else {
vaultItemClick(it.id)
}
@ -144,6 +145,19 @@ fun VaultItemListingContent(
showConfirmationDialog = option
}
is ListingItemOverflowAction.VaultAction -> {
if (option.requiresPasswordReprompt &&
it.shouldShowMasterPasswordReprompt
) {
masterPasswordRepromptData =
MasterPasswordRepromptData.OverflowItem(
action = option,
)
} else {
onOverflowItemClick(option)
}
}
else -> onOverflowItemClick(option)
}
},

View file

@ -260,7 +260,7 @@ class VaultItemListingViewModel @Inject constructor(
val result = authRepository.validatePassword(action.password)
sendAction(
VaultItemListingsAction.Internal.ValidatePasswordResultReceive(
cipherId = action.cipherId,
masterPasswordRepromptData = action.masterPasswordRepromptData,
result = result,
),
)
@ -573,10 +573,28 @@ class VaultItemListingViewModel @Inject constructor(
}
return
}
handleMasterPasswordRepromptData(
data = action.masterPasswordRepromptData,
)
}
}
}
private fun handleMasterPasswordRepromptData(
data: MasterPasswordRepromptData,
) {
when (data) {
is MasterPasswordRepromptData.Autofill -> {
// Complete the autofill selection flow
val cipherView = getCipherViewOrNull(cipherId = action.cipherId) ?: return
val cipherView = getCipherViewOrNull(cipherId = data.cipherId) ?: return
autofillSelectionManager.emitAutofillSelection(cipherView = cipherView)
}
is MasterPasswordRepromptData.OverflowItem -> {
handleOverflowOptionClick(
VaultItemListingsAction.OverflowOptionClick(data.action),
)
}
}
}
//endregion VaultItemListing Handlers
@ -848,6 +866,9 @@ data class VaultItemListingState(
* @property subtitle subtitle of the item (nullable).
* @property iconData data for the icon to be displayed (nullable).
* @property overflowOptions list of options for the item's overflow menu.
* @property isAutofill whether or not this screen is part of an autofill flow.
* @property shouldShowMasterPasswordReprompt whether or not a master password reprompt is
* required for various secure actions.
*/
data class DisplayItem(
val id: String,
@ -856,6 +877,7 @@ data class VaultItemListingState(
val iconData: IconData,
val extraIconList: List<IconRes>,
val overflowOptions: List<ListingItemOverflowAction>,
val isAutofill: Boolean,
val shouldShowMasterPasswordReprompt: Boolean,
)
@ -1122,12 +1144,12 @@ sealed class VaultItemListingsAction {
data class ItemClick(val id: String) : VaultItemListingsAction()
/**
* A master password prompt was encountered when trying to access the cipher with the given
* [cipherId] and the given [password] was submitted.
* A master password prompt was encountered when trying to perform a senstive action described
* by the given [masterPasswordRepromptData] and the given [password] was submitted.
*/
data class MasterPasswordRepromptSubmit(
val cipherId: String,
val password: String,
val masterPasswordRepromptData: MasterPasswordRepromptData,
) : VaultItemListingsAction()
/**
@ -1181,7 +1203,7 @@ sealed class VaultItemListingsAction {
* Indicates that a result for verifying the user's master password has been received.
*/
data class ValidatePasswordResultReceive(
val cipherId: String,
val masterPasswordRepromptData: MasterPasswordRepromptData,
val result: ValidatePasswordResult,
) : Internal()
@ -1193,3 +1215,25 @@ sealed class VaultItemListingsAction {
) : Internal()
}
}
/**
* Data tracking the type of request that triggered a master password reprompt.
*/
sealed class MasterPasswordRepromptData : Parcelable {
/**
* Autofill was selected.
*/
@Parcelize
data class Autofill(
val cipherId: String,
) : MasterPasswordRepromptData()
/**
* A cipher overflow menu item action was selected.
*/
@Parcelize
data class OverflowItem(
val action: ListingItemOverflowAction.VaultAction,
) : MasterPasswordRepromptData()
}

View file

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.vault.feature.itemlisting.handlers
import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.MasterPasswordRepromptData
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingViewModel
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingsAction
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
@ -17,7 +18,7 @@ data class VaultItemListingHandlers(
val searchIconClick: () -> Unit,
val addVaultItemClick: () -> Unit,
val itemClick: (id: String) -> Unit,
val masterPasswordRepromptSubmit: (id: String, password: String) -> Unit,
val masterPasswordRepromptSubmit: (password: String, MasterPasswordRepromptData) -> Unit,
val refreshClick: () -> Unit,
val syncClick: () -> Unit,
val lockClick: () -> Unit,
@ -49,11 +50,11 @@ data class VaultItemListingHandlers(
viewModel.trySendAction(VaultItemListingsAction.AddVaultItemClick)
},
itemClick = { viewModel.trySendAction(VaultItemListingsAction.ItemClick(it)) },
masterPasswordRepromptSubmit = { cipherId, password ->
masterPasswordRepromptSubmit = { password, data ->
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
masterPasswordRepromptData = data,
),
)
},

View file

@ -218,7 +218,8 @@ private fun CipherView.toDisplayItem(
),
extraIconList = toLabelIcons(),
overflowOptions = toOverflowActions(),
shouldShowMasterPasswordReprompt = isAutofill && reprompt == CipherRepromptType.PASSWORD,
isAutofill = isAutofill,
shouldShowMasterPasswordReprompt = reprompt == CipherRepromptType.PASSWORD,
)
private fun CipherView.toIconData(
@ -255,6 +256,7 @@ private fun SendView.toDisplayItem(
),
extraIconList = toLabelIcons(clock = clock),
overflowOptions = toOverflowActions(baseWebSendUrl = baseWebSendUrl),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)

View file

@ -35,6 +35,7 @@ import com.x8bit.bitwarden.ui.platform.feature.search.model.SearchType
import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
import com.x8bit.bitwarden.ui.util.assertLockOrLogoutDialogIsDisplayed
import com.x8bit.bitwarden.ui.util.assertLogoutConfirmationDialogIsDisplayed
import com.x8bit.bitwarden.ui.util.assertMasterPasswordDialogDisplayed
import com.x8bit.bitwarden.ui.util.assertNoDialogExists
import com.x8bit.bitwarden.ui.util.assertSwitcherIsDisplayed
import com.x8bit.bitwarden.ui.util.assertSwitcherIsNotDisplayed
@ -639,7 +640,10 @@ class VaultItemListingScreenTest : BaseComposeTest() {
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = false),
.copy(
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
),
),
),
)
@ -656,13 +660,16 @@ class VaultItemListingScreenTest : BaseComposeTest() {
@Suppress("MaxLineLength")
@Test
fun `clicking on a display item when master password reprompt is required should show the master password dialog`() {
fun `clicking on a display item when master password reprompt is required for autofill should show the master password dialog`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
.copy(
isAutofill = true,
shouldShowMasterPasswordReprompt = true,
),
),
),
)
@ -709,7 +716,10 @@ class VaultItemListingScreenTest : BaseComposeTest() {
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
.copy(
isAutofill = true,
shouldShowMasterPasswordReprompt = true,
),
),
),
)
@ -735,7 +745,10 @@ class VaultItemListingScreenTest : BaseComposeTest() {
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
.copy(
isAutofill = true,
shouldShowMasterPasswordReprompt = true,
),
),
),
)
@ -757,8 +770,10 @@ class VaultItemListingScreenTest : BaseComposeTest() {
verify {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = "mockId-1",
password = "password",
masterPasswordRepromptData = MasterPasswordRepromptData.Autofill(
cipherId = "mockId-1",
),
),
)
}
@ -861,6 +876,72 @@ class VaultItemListingScreenTest : BaseComposeTest() {
}
}
@Test
fun `on cipher item overflow option click should emit the appropriate action`() {
mutableStateFlow.update {
it.copy(
itemListingType = VaultItemListingState.ItemListingType.Vault.Login,
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(createCipherDisplayItem(number = 1)),
),
)
}
composeTestRule.assertNoDialogExists()
composeTestRule
.onNodeWithContentDescription("Options")
.assertIsDisplayed()
.performClick()
composeTestRule
.onNodeWithText("Edit")
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
.performClick()
verify(exactly = 1) {
viewModel.trySendAction(
VaultItemListingsAction.OverflowOptionClick(
action = ListingItemOverflowAction.VaultAction.EditClick(
cipherId = "mockId-1",
),
),
)
}
composeTestRule.assertNoDialogExists()
}
@Suppress("MaxLineLength")
@Test
fun `on cipher item overflow option click when reprompt is required should show the master password dialog`() {
mutableStateFlow.update {
it.copy(
itemListingType = VaultItemListingState.ItemListingType.Vault.Login,
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createCipherDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
),
),
)
}
composeTestRule.assertNoDialogExists()
composeTestRule
.onNodeWithContentDescription("Options")
.assertIsDisplayed()
.performClick()
composeTestRule
.onNodeWithText("Edit")
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
.performClick()
verify(exactly = 0) { viewModel.trySendAction(any()) }
composeTestRule.assertMasterPasswordDialogDisplayed()
}
@Test
fun `send item overflow item button should update according to state`() {
mutableStateFlow.update {
@ -1166,5 +1247,20 @@ private fun createDisplayItem(number: Int): VaultItemListingState.DisplayItem =
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
private fun createCipherDisplayItem(number: Int): VaultItemListingState.DisplayItem =
VaultItemListingState.DisplayItem(
id = "mockId-$number",
title = "mockTitle-$number",
subtitle = "mockSubtitle-$number",
iconData = IconData.Local(R.drawable.ic_vault),
extraIconList = emptyList(),
overflowOptions = listOf(
ListingItemOverflowAction.VaultAction.EditClick(cipherId = "mockId-$number"),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)

View file

@ -317,8 +317,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
masterPasswordRepromptData = MasterPasswordRepromptData.Autofill(
cipherId = cipherId,
),
),
)
@ -368,8 +370,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
masterPasswordRepromptData = MasterPasswordRepromptData.Autofill(
cipherId = cipherId,
),
),
)
@ -386,7 +390,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with a valid password should post to the AutofillSelectionManager`() =
fun `MasterPasswordRepromptSubmit for a request Success with a valid password for autofill should post to the AutofillSelectionManager`() =
runTest {
setupMockUri()
val cipherView = createMockCipherView(number = 1)
@ -408,8 +412,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
autofillSelectionManager.autofillSelectionFlow.test {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
masterPasswordRepromptData = MasterPasswordRepromptData.Autofill(
cipherId = cipherId,
),
),
)
assertEquals(
@ -419,6 +425,33 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with a valid password for overflow actions should process the action`() =
runTest {
val cipherId = "cipherId-1234"
val password = "password"
val viewModel = createVaultItemListingViewModel()
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = true)
viewModel.eventFlow.test {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
password = password,
masterPasswordRepromptData = MasterPasswordRepromptData.OverflowItem(
action = ListingItemOverflowAction.VaultAction.EditClick(
cipherId = cipherId,
),
),
),
)
// An Edit action navigates to the Edit screen
assertEquals(VaultItemListingEvent.NavigateToEditCipher(cipherId), awaitItem())
}
}
@Test
fun `AddVaultItemClick for vault item should emit NavigateToAddVaultItem`() = runTest {
val viewModel = createVaultItemListingViewModel()
@ -843,7 +876,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
createVaultItemListingState(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createMockDisplayItemForCipher(number = 1),
createMockDisplayItemForCipher(number = 1).copy(isAutofill = true),
),
),
)

View file

@ -358,7 +358,8 @@ class VaultItemListingDataExtensionsTest {
number = 1,
cipherType = CipherType.LOGIN,
subtitle = null,
),
)
.copy(shouldShowMasterPasswordReprompt = true),
createMockDisplayItemForCipher(
number = 2,
cipherType = CipherType.CARD,
@ -421,12 +422,16 @@ class VaultItemListingDataExtensionsTest {
cipherType = CipherType.LOGIN,
subtitle = null,
)
.copy(shouldShowMasterPasswordReprompt = true),
.copy(
isAutofill = true,
shouldShowMasterPasswordReprompt = true,
),
createMockDisplayItemForCipher(
number = 2,
cipherType = CipherType.CARD,
subtitle = null,
),
)
.copy(isAutofill = true),
),
),
result,

View file

@ -53,6 +53,7 @@ fun createMockDisplayItemForCipher(
url = "www.mockuri$number.com",
),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}
@ -80,6 +81,7 @@ fun createMockDisplayItemForCipher(
notes = "mockNotes-$number",
),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}
@ -110,6 +112,7 @@ fun createMockDisplayItemForCipher(
securityCode = "mockCode-$number",
),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}
@ -134,6 +137,7 @@ fun createMockDisplayItemForCipher(
ListingItemOverflowAction.VaultAction.ViewClick(cipherId = "mockId-$number"),
ListingItemOverflowAction.VaultAction.EditClick(cipherId = "mockId-$number"),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}
@ -175,6 +179,7 @@ fun createMockDisplayItemForSend(
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}
@ -206,6 +211,7 @@ fun createMockDisplayItemForSend(
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
)
}