PM-13020: During totp flow master password reprompt should be honored (#4136)

This commit is contained in:
David Perez 2024-10-23 07:51:28 -05:00 committed by GitHub
parent fca00d38f5
commit c5a266dfc0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 203 additions and 6 deletions

View file

@ -116,7 +116,9 @@ fun SearchContent(
supportingLabelTestTag = it.subtitleTestTag,
optionsTestTag = it.overflowTestTag,
onClick = {
if (it.autofillSelectionOptions.isNotEmpty()) {
if (it.isTotp && it.shouldDisplayMasterPasswordReprompt) {
masterPasswordRepromptData = MasterPasswordRepromptData.Totp(it.id)
} else if (it.autofillSelectionOptions.isNotEmpty()) {
autofillSelectionOptionsItem = it
} else {
searchHandlers.onItemClick(it.id)

View file

@ -570,6 +570,10 @@ class SearchViewModel @Inject constructor(
),
)
}
is MasterPasswordRepromptData.Totp -> {
trySendAction(SearchAction.ItemClick(itemId = data.cipherId))
}
}
}
@ -680,6 +684,7 @@ class SearchViewModel @Inject constructor(
baseIconUrl = state.baseIconUrl,
isIconLoadingDisabled = state.isIconLoadingDisabled,
isAutofill = state.isAutofill,
isTotp = state.isTotp,
isPremiumUser = state.isPremium,
)
}
@ -826,6 +831,7 @@ data class SearchState(
val overflowOptions: List<ListingItemOverflowAction>,
val overflowTestTag: String?,
val autofillSelectionOptions: List<AutofillSelectionOption>,
val isTotp: Boolean,
val shouldDisplayMasterPasswordReprompt: Boolean,
) : Parcelable
}
@ -1171,6 +1177,14 @@ sealed class MasterPasswordRepromptData : Parcelable {
val cipherId: String,
) : MasterPasswordRepromptData()
/**
* Autofill was selected.
*/
@Parcelize
data class Totp(
val cipherId: String,
) : MasterPasswordRepromptData()
/**
* A cipher overflow menu item action was selected.
*/

View file

@ -148,6 +148,7 @@ fun List<CipherView>.toViewState(
hasMasterPassword: Boolean,
isIconLoadingDisabled: Boolean,
isAutofill: Boolean,
isTotp: Boolean,
isPremiumUser: Boolean,
): SearchState.ViewState =
when {
@ -159,6 +160,7 @@ fun List<CipherView>.toViewState(
hasMasterPassword = hasMasterPassword,
isIconLoadingDisabled = isIconLoadingDisabled,
isAutofill = isAutofill,
isTotp = isTotp,
isPremiumUser = isPremiumUser,
)
.sortAlphabetically(),
@ -172,11 +174,13 @@ fun List<CipherView>.toViewState(
}
}
@Suppress("LongParameterList")
private fun List<CipherView>.toDisplayItemList(
baseIconUrl: String,
hasMasterPassword: Boolean,
isIconLoadingDisabled: Boolean,
isAutofill: Boolean,
isTotp: Boolean,
isPremiumUser: Boolean,
): List<SearchState.DisplayItem> =
this.map {
@ -185,15 +189,18 @@ private fun List<CipherView>.toDisplayItemList(
hasMasterPassword = hasMasterPassword,
isIconLoadingDisabled = isIconLoadingDisabled,
isAutofill = isAutofill,
isTotp = isTotp,
isPremiumUser = isPremiumUser,
)
}
@Suppress("LongParameterList")
private fun CipherView.toDisplayItem(
baseIconUrl: String,
hasMasterPassword: Boolean,
isIconLoadingDisabled: Boolean,
isAutofill: Boolean,
isTotp: Boolean,
isPremiumUser: Boolean,
): SearchState.DisplayItem =
SearchState.DisplayItem(
@ -221,6 +228,7 @@ private fun CipherView.toDisplayItem(
.filter {
this.login != null || (it != AutofillSelectionOption.AUTOFILL_AND_SAVE)
},
isTotp = isTotp,
shouldDisplayMasterPasswordReprompt = reprompt == CipherRepromptType.PASSWORD,
)
@ -360,6 +368,7 @@ private fun SendView.toDisplayItem(
overflowTestTag = "SendOptionsButton",
totpCode = null,
autofillSelectionOptions = emptyList(),
isTotp = false,
shouldDisplayMasterPasswordReprompt = false,
)

View file

@ -215,11 +215,14 @@ fun VaultItemListingContent(
supportingLabelTestTag = it.subtitleTestTag,
optionsTestTag = it.optionsTestTag,
onClick = {
if (it.isAutofill && it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptData =
MasterPasswordRepromptData.Autofill(
cipherId = it.id,
)
if (it.isTotp && it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptData = MasterPasswordRepromptData.Totp(
cipherId = it.id,
)
} else if (it.isAutofill && it.shouldShowMasterPasswordReprompt) {
masterPasswordRepromptData = MasterPasswordRepromptData.Autofill(
cipherId = it.id,
)
} else {
vaultItemClick(it.id)
}

View file

@ -1153,6 +1153,10 @@ class VaultItemListingViewModel @Inject constructor(
VaultItemListingsAction.OverflowOptionClick(data.action),
)
}
is MasterPasswordRepromptData.Totp -> {
sendEvent(VaultItemListingEvent.NavigateToEditCipher(data.cipherId))
}
}
}
@ -1972,6 +1976,7 @@ data class VaultItemListingState(
val optionsTestTag: String,
val isAutofill: Boolean,
val isFido2Creation: Boolean,
val isTotp: Boolean,
val shouldShowMasterPasswordReprompt: Boolean,
)
@ -2551,6 +2556,14 @@ sealed class MasterPasswordRepromptData : Parcelable {
val cipherId: String,
) : MasterPasswordRepromptData()
/**
* Totp was selected.
*/
@Parcelize
data class Totp(
val cipherId: String,
) : MasterPasswordRepromptData()
/**
* A cipher overflow menu item action was selected.
*/

View file

@ -137,6 +137,7 @@ fun VaultData.toViewState(
isFido2Creation = fido2CreationData != null,
fido2CredentialAutofillViews = fido2CredentialAutofillViews,
isPremiumUser = isPremiumUser,
isTotp = totpData != null,
),
displayFolderList = folderList.map { folderView ->
VaultItemListingState.FolderDisplayItem(
@ -282,6 +283,7 @@ private fun List<CipherView>.toDisplayItemList(
isFido2Creation: Boolean,
fido2CredentialAutofillViews: List<Fido2CredentialAutofillView>?,
isPremiumUser: Boolean,
isTotp: Boolean,
): List<VaultItemListingState.DisplayItem> =
this.map {
it.toDisplayItem(
@ -295,6 +297,7 @@ private fun List<CipherView>.toDisplayItemList(
fido2CredentialAutofillView.cipherId == it.id
},
isPremiumUser = isPremiumUser,
isTotp = isTotp,
)
}
@ -318,6 +321,7 @@ private fun CipherView.toDisplayItem(
isFido2Creation: Boolean,
fido2CredentialAutofillView: Fido2CredentialAutofillView?,
isPremiumUser: Boolean,
isTotp: Boolean,
): VaultItemListingState.DisplayItem =
VaultItemListingState.DisplayItem(
id = id.orEmpty(),
@ -345,6 +349,7 @@ private fun CipherView.toDisplayItem(
optionsTestTag = "CipherOptionsButton",
isAutofill = isAutofill,
isFido2Creation = isFido2Creation,
isTotp = isTotp,
shouldShowMasterPasswordReprompt = (reprompt == CipherRepromptType.PASSWORD) &&
hasMasterPassword,
)
@ -416,6 +421,7 @@ private fun SendView.toDisplayItem(
isAutofill = false,
shouldShowMasterPasswordReprompt = false,
isFido2Creation = false,
isTotp = false,
)
@get:DrawableRes

View file

@ -345,6 +345,26 @@ class SearchScreenTest : BaseComposeTest() {
composeTestRule.assertNoDialogExists()
}
@Test
fun `clicking on totp when reprompt is required should show master password dialog`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = SearchState.ViewState.Content(
displayItems = listOf(
createMockDisplayItemForCipher(number = 1, isTotp = true).copy(
shouldDisplayMasterPasswordReprompt = true,
),
),
),
totpData = mockk(),
)
composeTestRule
.onNodeWithText(text = "mockName-1")
.assertIsDisplayed()
.performClick()
composeTestRule.assertMasterPasswordDialogDisplayed()
}
@Test
fun `clicking cancel on the master password dialog should close the dialog`() {
mutableStateFlow.value = createStateForAutofill(isRepromptRequired = true)

View file

@ -592,6 +592,32 @@ class SearchViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with a valid password for totp should emit NavigateToEditCipher`() =
runTest {
setupMockUri()
val cipherId = CIPHER_ID
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Success(isValid = true)
val viewModel = createViewModel(initialState = DEFAULT_STATE.copy(totpData = mockk()))
viewModel.eventFlow.test {
viewModel.trySendAction(
SearchAction.MasterPasswordRepromptSubmit(
password = password,
masterPasswordRepromptData = MasterPasswordRepromptData.Totp(
cipherId = cipherId,
),
),
)
assertEquals(SearchEvent.NavigateToEditCipher(cipherId), awaitItem())
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit for a request Success with a valid password for an overflow action should perform the action`() =
@ -990,6 +1016,7 @@ class SearchViewModelTest : BaseViewModelTest() {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
} returns expectedViewState
val dataState = DataState.Loaded(
@ -1092,6 +1119,7 @@ class SearchViewModelTest : BaseViewModelTest() {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
} returns expectedViewState
mutableVaultDataStateFlow.tryEmit(
@ -1204,6 +1232,7 @@ class SearchViewModelTest : BaseViewModelTest() {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
} returns expectedViewState
val dataState = DataState.Error(
@ -1319,6 +1348,7 @@ class SearchViewModelTest : BaseViewModelTest() {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
} returns expectedViewState
val dataState = DataState.NoNetwork(
@ -1494,6 +1524,7 @@ class SearchViewModelTest : BaseViewModelTest() {
isAutofill = true,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
} returns expectedViewState
val dataState = DataState.Loaded(

View file

@ -299,6 +299,7 @@ class SearchTypeDataExtensionsTest {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = true,
)
assertEquals(SearchState.ViewState.Empty(message = null), result)
@ -324,6 +325,7 @@ class SearchTypeDataExtensionsTest {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
assertEquals(
@ -364,6 +366,7 @@ class SearchTypeDataExtensionsTest {
isAutofill = true,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = false,
)
assertEquals(
@ -414,6 +417,7 @@ class SearchTypeDataExtensionsTest {
isAutofill = false,
hasMasterPassword = true,
isPremiumUser = true,
isTotp = true,
)
assertEquals(

View file

@ -15,6 +15,7 @@ import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflo
fun createMockDisplayItemForCipher(
number: Int,
cipherType: CipherType = CipherType.LOGIN,
isTotp: Boolean = false,
): SearchState.DisplayItem =
when (cipherType) {
CipherType.LOGIN -> {
@ -65,6 +66,7 @@ fun createMockDisplayItemForCipher(
totpCode = "mockTotp-$number",
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = isTotp,
)
}
@ -102,6 +104,7 @@ fun createMockDisplayItemForCipher(
totpCode = null,
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = false,
)
}
@ -145,6 +148,7 @@ fun createMockDisplayItemForCipher(
totpCode = null,
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = false,
)
}
@ -179,6 +183,7 @@ fun createMockDisplayItemForCipher(
totpCode = null,
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = false,
)
}
}
@ -227,6 +232,7 @@ fun createMockDisplayItemForSend(
totpCode = null,
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = false,
)
}
@ -265,6 +271,7 @@ fun createMockDisplayItemForSend(
totpCode = null,
autofillSelectionOptions = emptyList(),
shouldDisplayMasterPasswordReprompt = false,
isTotp = true,
)
}
}

View file

@ -1052,6 +1052,58 @@ class VaultItemListingScreenTest : BaseComposeTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `clicking on a display item when master password reprompt is required for totp flow should show the master password dialog`() {
mutableStateFlow.update {
it.copy(
viewState = VaultItemListingState.ViewState.Content(
displayCollectionList = emptyList(),
displayItemList = listOf(
createDisplayItem(number = 1).copy(
isTotp = true,
shouldShowMasterPasswordReprompt = true,
),
),
displayFolderList = emptyList(),
),
)
}
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 {
@ -2146,6 +2198,7 @@ private fun createDisplayItem(number: Int): VaultItemListingState.DisplayItem =
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = null,
isTotp = false,
)
private fun createCipherDisplayItem(number: Int): VaultItemListingState.DisplayItem =
@ -2170,4 +2223,5 @@ private fun createCipherDisplayItem(number: Int): VaultItemListingState.DisplayI
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = null,
isTotp = true,
)

View file

@ -917,6 +917,31 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `MasterPasswordRepromptSubmit with a valid password for totp flow should emit NavigateToEditCipher`() =
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.Totp(
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()
@ -1467,6 +1492,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
createMockDisplayItemForCipher(
number = 1,
secondSubtitleTestTag = "PasskeySite",
isTotp = true,
),
),
displayFolderList = emptyList(),

View file

@ -12,12 +12,14 @@ import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflo
/**
* Create a mock [VaultItemListingState.DisplayItem] with a given [number].
*/
@Suppress("LongParameterList")
fun createMockDisplayItemForCipher(
number: Int,
cipherType: CipherType = CipherType.LOGIN,
subtitle: String? = "mockUsername-$number",
secondSubtitleTestTag: String? = null,
requiresPasswordReprompt: Boolean = true,
isTotp: Boolean = false,
): VaultItemListingState.DisplayItem =
when (cipherType) {
CipherType.LOGIN -> {
@ -71,6 +73,7 @@ fun createMockDisplayItemForCipher(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = "LoginCipherIcon",
isTotp = isTotp,
)
}
@ -111,6 +114,7 @@ fun createMockDisplayItemForCipher(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = "SecureNoteCipherIcon",
isTotp = false,
)
}
@ -157,6 +161,7 @@ fun createMockDisplayItemForCipher(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = "CardCipherIcon",
isTotp = false,
)
}
@ -194,6 +199,7 @@ fun createMockDisplayItemForCipher(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = "IdentityCipherIcon",
isTotp = false,
)
}
}
@ -245,6 +251,7 @@ fun createMockDisplayItemForSend(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = null,
isTotp = false,
)
}
@ -286,6 +293,7 @@ fun createMockDisplayItemForSend(
isFido2Creation = false,
shouldShowMasterPasswordReprompt = false,
iconTestTag = null,
isTotp = false,
)
}
}