PM-13020: During totp flow master password reprompt should be honored (…

…#4136)
bitwarden · Oct 23, 2024 · c5a266d · c5a266d
1 parent fca00d3
commit c5a266d
Show file tree

Hide file tree

Showing 13 changed files with 203 additions and 6 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt
@@ -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)

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt
@@ -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.
      */

diff --git a/...main/java/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchTypeDataExtensions.kt b/...main/java/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchTypeDataExtensions.kt
@@ -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,
     )
 

diff --git a/...src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingContent.kt b/...src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingContent.kt
@@ -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)
                         }

diff --git a/...c/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/...c/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt
@@ -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.
      */

diff --git a/...a/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt b/...a/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt
@@ -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

diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt
@@ -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)

diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt
@@ -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(

diff --git a/.../java/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchTypeDataExtensionsTest.kt b/.../java/com/x8bit/bitwarden/ui/platform/feature/search/util/SearchTypeDataExtensionsTest.kt
@@ -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(