BIT-1643: Add master password reprompt to autofill selection (#854)

This commit is contained in:
Brian Yencho 2024-01-29 18:09:37 -06:00 committed by Álison Fernandes
parent 79988db49a
commit 50d963c70e
9 changed files with 439 additions and 5 deletions

View file

@ -10,6 +10,7 @@ import androidx.compose.foundation.lazy.items
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
@ -18,6 +19,7 @@ import androidx.compose.ui.unit.dp
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.ui.platform.components.BitwardenListHeaderTextWithSupportLabel
import com.x8bit.bitwarden.ui.platform.components.BitwardenListItem
import com.x8bit.bitwarden.ui.platform.components.BitwardenMasterPasswordDialog
import com.x8bit.bitwarden.ui.platform.components.BitwardenTwoButtonDialog
import com.x8bit.bitwarden.ui.platform.components.SelectionItemData
import com.x8bit.bitwarden.ui.platform.components.model.toIconResources
@ -32,6 +34,7 @@ import kotlinx.collections.immutable.toPersistentList
fun VaultItemListingContent(
state: VaultItemListingState.ViewState.Content,
vaultItemClick: (id: String) -> Unit,
masterPasswordRepromptSubmit: (id: String, password: String) -> Unit,
onOverflowItemClick: (action: ListingItemOverflowAction) -> Unit,
modifier: Modifier = Modifier,
) {
@ -71,6 +74,24 @@ fun VaultItemListingContent(
-> Unit
}
var masterPasswordRepromptCipherId by remember { mutableStateOf<String?>(null) }
if (masterPasswordRepromptCipherId != null) {
BitwardenMasterPasswordDialog(
onConfirmClick = {
val cipherId = masterPasswordRepromptCipherId
?: return@BitwardenMasterPasswordDialog
masterPasswordRepromptCipherId = null
masterPasswordRepromptSubmit(
cipherId,
it,
)
},
onDismissRequest = {
masterPasswordRepromptCipherId = null
},
)
}
LazyColumn(
modifier = modifier,
) {
@ -88,7 +109,13 @@ fun VaultItemListingContent(
startIcon = it.iconData,
label = it.title,
supportingLabel = it.subtitle,
onClick = { vaultItemClick(it.id) },
onClick = {
if (it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptCipherId = it.id
} else {
vaultItemClick(it.id)
}
},
trailingLabelIcons = it
.extraIconList
.toIconResources()

View file

@ -235,6 +235,8 @@ private fun VaultItemListingScaffold(
VaultItemListingContent(
state = state.viewState,
vaultItemClick = vaultItemListingHandlers.itemClick,
masterPasswordRepromptSubmit =
vaultItemListingHandlers.masterPasswordRepromptSubmit,
onOverflowItemClick = vaultItemListingHandlers.overflowItemClick,
modifier = modifier,
)

View file

@ -5,6 +5,7 @@ import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManager
import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData
import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage
@ -129,6 +130,10 @@ class VaultItemListingViewModel @Inject constructor(
is VaultItemListingsAction.SearchIconClick -> handleSearchIconClick()
is VaultItemListingsAction.OverflowOptionClick -> handleOverflowOptionClick(action)
is VaultItemListingsAction.ItemClick -> handleItemClick(action)
is VaultItemListingsAction.MasterPasswordRepromptSubmit -> {
handleMasterPasswordRepromptSubmit(action)
}
is VaultItemListingsAction.AddVaultItemClick -> handleAddVaultItemClick()
is VaultItemListingsAction.RefreshClick -> handleRefreshClick()
is VaultItemListingsAction.RefreshPull -> handleRefreshPull()
@ -235,6 +240,20 @@ class VaultItemListingViewModel @Inject constructor(
sendEvent(event)
}
private fun handleMasterPasswordRepromptSubmit(
action: VaultItemListingsAction.MasterPasswordRepromptSubmit,
) {
viewModelScope.launch {
val result = authRepository.validatePassword(action.password)
sendAction(
VaultItemListingsAction.Internal.ValidatePasswordResultReceive(
cipherId = action.cipherId,
result = result,
),
)
}
}
private fun handleCopyNoteClick(action: ListingItemOverflowAction.VaultAction.CopyNoteClick) {
clipboardManager.setText(action.notes)
}
@ -401,6 +420,10 @@ class VaultItemListingViewModel @Inject constructor(
is VaultItemListingsAction.Internal.GenerateTotpResultReceive -> {
handleGenerateTotpResultReceive(action)
}
is VaultItemListingsAction.Internal.ValidatePasswordResultReceive -> {
handleMasterPasswordRepromptResultReceive(action)
}
}
}
@ -503,6 +526,42 @@ class VaultItemListingViewModel @Inject constructor(
updateStateWithVaultData(vaultData, clearDialogState = false)
}
}
private fun handleMasterPasswordRepromptResultReceive(
action: VaultItemListingsAction.Internal.ValidatePasswordResultReceive,
) {
mutableStateFlow.update { it.copy(dialogState = null) }
when (val result = action.result) {
ValidatePasswordResult.Error -> {
mutableStateFlow.update {
it.copy(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
),
)
}
}
is ValidatePasswordResult.Success -> {
if (!result.isValid) {
mutableStateFlow.update {
it.copy(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.invalid_master_password.asText(),
),
)
}
return
}
// Complete the autofill selection flow
val cipherView = getCipherViewOrNull(cipherId = action.cipherId) ?: return
autofillSelectionManager.emitAutofillSelection(cipherView = cipherView)
}
}
}
//endregion VaultItemListing Handlers
private fun vaultErrorReceive(vaultData: DataState.Error<VaultData>) {
@ -779,6 +838,7 @@ data class VaultItemListingState(
val iconData: IconData,
val extraIconList: List<IconRes>,
val overflowOptions: List<ListingItemOverflowAction>,
val shouldShowMasterPasswordReprompt: Boolean,
)
/**
@ -1043,6 +1103,15 @@ 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.
*/
data class MasterPasswordRepromptSubmit(
val cipherId: String,
val password: String,
) : VaultItemListingsAction()
/**
* User has triggered a pull to refresh.
*/
@ -1089,5 +1158,13 @@ sealed class VaultItemListingsAction {
data class VaultDataReceive(
val vaultData: DataState<VaultData>,
) : Internal()
/**
* Indicates that a result for verifying the user's master password has been received.
*/
data class ValidatePasswordResultReceive(
val cipherId: String,
val result: ValidatePasswordResult,
) : Internal()
}
}

View file

@ -17,6 +17,7 @@ data class VaultItemListingHandlers(
val searchIconClick: () -> Unit,
val addVaultItemClick: () -> Unit,
val itemClick: (id: String) -> Unit,
val masterPasswordRepromptSubmit: (id: String, password: String) -> Unit,
val refreshClick: () -> Unit,
val syncClick: () -> Unit,
val lockClick: () -> Unit,
@ -48,6 +49,14 @@ data class VaultItemListingHandlers(
viewModel.trySendAction(VaultItemListingsAction.AddVaultItemClick)
},
itemClick = { viewModel.trySendAction(VaultItemListingsAction.ItemClick(it)) },
masterPasswordRepromptSubmit = { cipherId, password ->
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
),
)
},
refreshClick = { viewModel.trySendAction(VaultItemListingsAction.RefreshClick) },
syncClick = { viewModel.trySendAction(VaultItemListingsAction.SyncClick) },
lockClick = { viewModel.trySendAction(VaultItemListingsAction.LockClick) },

View file

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.vault.feature.itemlisting.util
import androidx.annotation.DrawableRes
import com.bitwarden.core.CipherRepromptType
import com.bitwarden.core.CipherType
import com.bitwarden.core.CipherView
import com.bitwarden.core.CollectionView
@ -92,6 +93,7 @@ fun List<CipherView>.toViewState(
displayItemList = toDisplayItemList(
baseIconUrl = baseIconUrl,
isIconLoadingDisabled = isIconLoadingDisabled,
isAutofill = autofillSelectionData != null,
),
)
} else {
@ -180,11 +182,13 @@ fun VaultItemListingState.ItemListingType.updateWithAdditionalDataIfNecessary(
private fun List<CipherView>.toDisplayItemList(
baseIconUrl: String,
isIconLoadingDisabled: Boolean,
isAutofill: Boolean,
): List<VaultItemListingState.DisplayItem> =
this.map {
it.toDisplayItem(
baseIconUrl = baseIconUrl,
isIconLoadingDisabled = isIconLoadingDisabled,
isAutofill = isAutofill,
)
}
@ -202,6 +206,7 @@ private fun List<SendView>.toDisplayItemList(
private fun CipherView.toDisplayItem(
baseIconUrl: String,
isIconLoadingDisabled: Boolean,
isAutofill: Boolean,
): VaultItemListingState.DisplayItem =
VaultItemListingState.DisplayItem(
id = id.orEmpty(),
@ -213,6 +218,7 @@ private fun CipherView.toDisplayItem(
),
extraIconList = toLabelIcons(),
overflowOptions = toOverflowActions(),
shouldShowMasterPasswordReprompt = isAutofill && reprompt == CipherRepromptType.PASSWORD,
)
private fun CipherView.toIconData(
@ -249,6 +255,7 @@ private fun SendView.toDisplayItem(
),
extraIconList = toLabelIcons(clock = clock),
overflowOptions = toOverflowActions(baseWebSendUrl = baseWebSendUrl),
shouldShowMasterPasswordReprompt = false,
)
@get:DrawableRes

View file

@ -17,6 +17,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performScrollToNode
import androidx.compose.ui.test.performTextInput
import androidx.core.net.toUri
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData
@ -602,13 +603,15 @@ class VaultItemListingScreenTest : BaseComposeTest() {
.assertIsDisplayed()
}
@Suppress("MaxLineLength")
@Test
fun `clicking on a display item should send ItemClick action`() {
fun `clicking on a display item when master password reprompt is not required should send ItemClick action`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1),
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = false),
),
),
)
@ -623,6 +626,117 @@ class VaultItemListingScreenTest : BaseComposeTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `clicking on a display item when master password reprompt is required should show the master password dialog`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
),
),
)
}
composeTestRule
.onNodeWithText(text = "mockTitle-1")
.assertIsDisplayed()
.performClick()
composeTestRule
.onAllNodesWithText(text = "Master password confirmation")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(
text = "This action is protected, to continue please re-enter your master " +
"password to verify your identity.",
)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(text = "Master password")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(text = "Submit")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
verify(exactly = 0) {
viewModel.trySendAction(any())
}
}
@Test
fun `clicking cancel on the master password dialog should close the dialog`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
),
),
)
}
composeTestRule
.onNodeWithText(text = "mockTitle-1")
.assertIsDisplayed()
.performClick()
composeTestRule
.onAllNodesWithText(text = "Cancel")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
composeTestRule.assertNoDialogExists()
}
@Suppress("MaxLineLength")
@Test
fun `clicking submit on the master password dialog should close the dialog and send MasterPasswordRepromptSubmit`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createDisplayItem(number = 1)
.copy(shouldShowMasterPasswordReprompt = true),
),
),
)
}
composeTestRule
.onNodeWithText(text = "mockTitle-1")
.assertIsDisplayed()
.performClick()
composeTestRule
.onAllNodesWithText(text = "Master password")
.filterToOne(hasAnyAncestor(isDialog()))
.performTextInput("password")
composeTestRule
.onAllNodesWithText(text = "Submit")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = "mockId-1",
password = "password",
),
)
}
composeTestRule.assertNoDialogExists()
}
@Test
fun `topBar title should be displayed according to state`() {
mutableStateFlow.update { DEFAULT_STATE }
@ -998,4 +1112,5 @@ private fun createDisplayItem(number: Int): VaultItemListingState.DisplayItem =
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
shouldShowMasterPasswordReprompt = false,
)

View file

@ -7,6 +7,7 @@ import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManagerImpl
import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData
@ -264,6 +265,141 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `MasterPasswordRepromptSubmit for a request Error should show a generic error dialog`() =
runTest {
setupMockUri()
val cipherView = createMockCipherView(number = 1)
val cipherId = "mockId-1"
val password = "password"
mutableVaultDataStateFlow.value = DataState.Loaded(
data = VaultData(
cipherViewList = listOf(cipherView),
folderViewList = emptyList(),
collectionViewList = emptyList(),
sendViewList = emptyList(),
),
)
val viewModel = createVaultItemListingViewModel()
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
val initialState = createVaultItemListingState(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createMockDisplayItemForCipher(number = 1),
),
),
)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
),
)
assertEquals(
initialState.copy(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
),
),
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with an invalid password should show an invalid password dialog`() =
runTest {
setupMockUri()
val cipherView = createMockCipherView(number = 1)
val cipherId = "mockId-1"
val password = "password"
mutableVaultDataStateFlow.value = DataState.Loaded(
data = VaultData(
cipherViewList = listOf(cipherView),
folderViewList = emptyList(),
collectionViewList = emptyList(),
sendViewList = emptyList(),
),
)
val initialState = createVaultItemListingState(
viewState = VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createMockDisplayItemForCipher(number = 1),
),
),
)
val viewModel = createVaultItemListingViewModel()
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = false)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
),
)
assertEquals(
initialState.copy(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.invalid_master_password.asText(),
),
),
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with a valid password should should post to the AutofillSelectionManager`() =
runTest {
setupMockUri()
val cipherView = createMockCipherView(number = 1)
val cipherId = "mockId-1"
val password = "password"
mutableVaultDataStateFlow.value = DataState.Loaded(
data = VaultData(
cipherViewList = listOf(cipherView),
folderViewList = emptyList(),
collectionViewList = emptyList(),
sendViewList = emptyList(),
),
)
val viewModel = createVaultItemListingViewModel()
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = true)
autofillSelectionManager.autofillSelectionFlow.test {
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordRepromptSubmit(
cipherId = cipherId,
password = password,
),
)
assertEquals(
cipherView,
awaitItem(),
)
}
}
@Test
fun `AddVaultItemClick for vault item should emit NavigateToAddVaultItem`() = runTest {
val viewModel = createVaultItemListingViewModel()

View file

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.vault.feature.itemlisting.util
import android.net.Uri
import com.bitwarden.core.CipherRepromptType
import com.bitwarden.core.CipherType
import com.bitwarden.core.CipherView
import com.bitwarden.core.SendType
@ -311,7 +312,7 @@ class VaultItemListingDataExtensionsTest {
}
@Test
fun `toViewState should transform a list of CipherViews into a ViewState`() {
fun `toViewState should transform a list of CipherViews into a ViewState when not autofill`() {
mockkStatic(CipherView::subtitle)
mockkStatic(Uri::class)
val uriMock = mockk<Uri>()
@ -324,7 +325,8 @@ class VaultItemListingDataExtensionsTest {
number = 1,
isDeleted = false,
cipherType = CipherType.LOGIN,
),
)
.copy(reprompt = CipherRepromptType.PASSWORD),
createMockCipherView(
number = 2,
isDeleted = false,
@ -378,6 +380,59 @@ class VaultItemListingDataExtensionsTest {
)
}
@Test
fun `toViewState should transform a list of CipherViews into a ViewState when autofill`() {
mockkStatic(CipherView::subtitle)
mockkStatic(Uri::class)
val uriMock = mockk<Uri>()
every { any<CipherView>().subtitle } returns null
every { Uri.parse(any()) } returns uriMock
every { uriMock.host } returns "www.mockuri.com"
val cipherViewList = listOf(
createMockCipherView(
number = 1,
isDeleted = false,
cipherType = CipherType.LOGIN,
)
.copy(reprompt = CipherRepromptType.PASSWORD),
createMockCipherView(
number = 2,
isDeleted = false,
cipherType = CipherType.CARD,
),
)
val result = cipherViewList.toViewState(
itemListingType = VaultItemListingState.ItemListingType.Vault.Login,
isIconLoadingDisabled = false,
baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl,
autofillSelectionData = AutofillSelectionData(
type = AutofillSelectionData.Type.LOGIN,
uri = null,
),
)
assertEquals(
VaultItemListingState.ViewState.Content(
displayItemList = listOf(
createMockDisplayItemForCipher(
number = 1,
cipherType = CipherType.LOGIN,
subtitle = null,
)
.copy(shouldShowMasterPasswordReprompt = true),
createMockDisplayItemForCipher(
number = 2,
cipherType = CipherType.CARD,
subtitle = null,
),
),
),
result,
)
}
@Suppress("MaxLineLength")
@Test
fun `toViewState should transform an empty list of CipherViews into a NoItems ViewState with the appropriate data`() {

View file

@ -53,6 +53,7 @@ fun createMockDisplayItemForCipher(
url = "www.mockuri$number.com",
),
),
shouldShowMasterPasswordReprompt = false,
)
}
@ -79,6 +80,7 @@ fun createMockDisplayItemForCipher(
notes = "mockNotes-$number",
),
),
shouldShowMasterPasswordReprompt = false,
)
}
@ -108,6 +110,7 @@ fun createMockDisplayItemForCipher(
securityCode = "mockCode-$number",
),
),
shouldShowMasterPasswordReprompt = false,
)
}
@ -131,6 +134,7 @@ fun createMockDisplayItemForCipher(
ListingItemOverflowAction.VaultAction.ViewClick(cipherId = "mockId-$number"),
ListingItemOverflowAction.VaultAction.EditClick(cipherId = "mockId-$number"),
),
shouldShowMasterPasswordReprompt = false,
)
}
}
@ -171,6 +175,7 @@ fun createMockDisplayItemForSend(
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
shouldShowMasterPasswordReprompt = false,
)
}
@ -201,6 +206,7 @@ fun createMockDisplayItemForSend(
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-$number"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-$number"),
),
shouldShowMasterPasswordReprompt = false,
)
}
}