From 2cee8943953609f080df27bc39dc26f067bb9696 Mon Sep 17 00:00:00 2001 From: Oleg Semenenko <146032743+oleg-livefront@users.noreply.github.com> Date: Wed, 24 Jan 2024 21:48:28 -0600 Subject: [PATCH] BIT-1402 adding Password checker to Add Edit Screen (#769) --- .../feature/addedit/VaultAddEditScreen.kt | 19 +++-- .../feature/addedit/VaultAddEditViewModel.kt | 78 +++++++++++++---- .../feature/addedit/VaultAddEditScreenTest.kt | 8 +- .../addedit/VaultAddEditViewModelTest.kt | 85 ++++++++++++++----- 4 files changed, 142 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt index 424216bbab..cf2ded3a04 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt @@ -19,7 +19,6 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect -import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenErrorContent @@ -82,10 +81,12 @@ fun VaultAddEditScreen( is VaultAddEditEvent.NavigateToMoveToOrganization -> { onNavigateToMoveToOrganization(event.cipherId) } + is VaultAddEditEvent.NavigateToCollections -> { // TODO implement Collections in BIT-1575 Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show() } + VaultAddEditEvent.NavigateBack -> onNavigateBack.invoke() } } @@ -227,13 +228,15 @@ private fun VaultAddEditItemDialogs( ) } - is VaultAddEditState.DialogState.Error -> BitwardenBasicDialog( - visibilityState = BasicDialogState.Shown( - title = R.string.an_error_has_occurred.asText(), - message = dialogState.message, - ), - onDismissRequest = onDismissRequest, - ) + is VaultAddEditState.DialogState.Generic -> { + BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = dialogState.title, + message = dialogState.message, + ), + onDismissRequest = onDismissRequest, + ) + } null -> Unit } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt index ab5c9f0230..2be74f4512 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt @@ -5,6 +5,8 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.bitwarden.core.CipherView import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.repository.model.DataState import com.x8bit.bitwarden.data.platform.repository.util.takeUntilLoaded @@ -56,6 +58,7 @@ private const val KEY_STATE = "state" @Suppress("TooManyFunctions", "LargeClass") class VaultAddEditViewModel @Inject constructor( savedStateHandle: SavedStateHandle, + private val authRepository: AuthRepository, private val clipboardManager: BitwardenClipboardManager, private val vaultRepository: VaultRepository, private val generatorRepository: GeneratorRepository, @@ -220,8 +223,9 @@ class VaultAddEditViewModel @Inject constructor( if (content.common.name.isBlank()) { mutableStateFlow.update { it.copy( - dialog = VaultAddEditState.DialogState.Error( - R.string.validation_field_required + dialog = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), + message = R.string.validation_field_required .asText(R.string.name.asText()), ), ) @@ -519,12 +523,15 @@ class VaultAddEditViewModel @Inject constructor( } private fun handleLoginPasswordCheckerClick() { - viewModelScope.launch { - sendEvent( - event = VaultAddEditEvent.ShowToast( - message = "Password Checker".asText(), - ), - ) + onLoginType { loginType -> + mutableStateFlow.update { + it.copy(dialog = VaultAddEditState.DialogState.Loading(R.string.loading.asText())) + } + + viewModelScope.launch { + val result = authRepository.getPasswordBreachCount(password = loginType.password) + sendAction(VaultAddEditAction.Internal.PasswordBreachReceive(result)) + } } } @@ -847,6 +854,9 @@ class VaultAddEditViewModel @Inject constructor( is VaultAddEditAction.Internal.GeneratorResultReceive -> { handleGeneratorResultReceive(action) } + + is VaultAddEditAction.Internal.PasswordBreachReceive -> + handlePasswordBreachReceive(action) } } @@ -883,7 +893,8 @@ class VaultAddEditViewModel @Inject constructor( is UpdateCipherResult.Error -> { mutableStateFlow.update { it.copy( - dialog = VaultAddEditState.DialogState.Error( + dialog = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), message = result .errorMessage ?.asText() @@ -981,8 +992,9 @@ class VaultAddEditViewModel @Inject constructor( TotpCodeResult.CodeScanningError -> { mutableStateFlow.update { it.copy( - dialog = VaultAddEditState.DialogState.Error( - R.string.authenticator_key_read_error.asText(), + dialog = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), + message = R.string.authenticator_key_read_error.asText(), ), ) } @@ -1012,6 +1024,24 @@ class VaultAddEditViewModel @Inject constructor( } } + private fun handlePasswordBreachReceive( + action: VaultAddEditAction.Internal.PasswordBreachReceive, + ) { + val message = when (val result = action.result) { + is BreachCountResult.Error -> R.string.generic_error_message.asText() + is BreachCountResult.Success -> { + if (result.breachCount > 0) { + R.string.password_exposed.asText(result.breachCount) + } else { + R.string.password_safe.asText() + } + } + } + mutableStateFlow.update { + it.copy(dialog = VaultAddEditState.DialogState.Generic(message = message)) + } + } + //endregion Internal Type Handlers //region Utility Functions @@ -1049,6 +1079,12 @@ class VaultAddEditViewModel @Inject constructor( } } + private inline fun onLoginType( + crossinline block: (VaultAddEditState.ViewState.Content.ItemType.Login) -> Unit, + ) { + onContent { (it.type as? VaultAddEditState.ViewState.Content.ItemType.Login)?.let(block) } + } + private inline fun updateLoginContent( crossinline block: (VaultAddEditState.ViewState.Content.ItemType.Login) -> VaultAddEditState.ViewState.Content.ItemType.Login, @@ -1427,17 +1463,20 @@ data class VaultAddEditState( @Parcelize sealed class DialogState : Parcelable { + /** + * Displays a generic dialog to the user. + */ + @Parcelize + data class Generic( + val title: Text? = null, + val message: Text, + ) : DialogState() + /** * Displays a loading dialog to the user. */ @Parcelize data class Loading(val label: Text) : DialogState() - - /** - * Displays an error dialog to the user. - */ - @Parcelize - data class Error(val message: Text) : DialogState() } } @@ -1884,6 +1923,11 @@ sealed class VaultAddEditAction { */ sealed class Internal : VaultAddEditAction() { + /** + * Indicates that the password breach results have been received. + */ + data class PasswordBreachReceive(val result: BreachCountResult) : Internal() + /** * Indicates that the vault totp code result has been received. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt index 0ec984fce6..50f529457c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt @@ -432,10 +432,14 @@ class VaultAddEditScreenTest : BaseComposeTest() { @Suppress("MaxLineLength") @Test fun `in ItemType_Login state clicking Password checker action should trigger PasswordCheckerClick`() { + mutableStateFlow.update { currentState -> + updateLoginType(currentState) { copy(password = "password") } + } + composeTestRule .onNodeWithTextAfterScroll(text = "Password") .onSiblings() - .onFirst() + .filterToOne(hasContentDescription("Check if password has been exposed.")) .performClick() verify { @@ -2298,7 +2302,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { common = VaultAddEditState.ViewState.Content.Common(), type = VaultAddEditState.ViewState.Content.ItemType.Login(), ), - dialog = VaultAddEditState.DialogState.Error("test".asText()), + dialog = VaultAddEditState.DialogState.Generic(message = "test".asText()), vaultAddEditType = VaultAddEditType.AddItem, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt index 9c84848ebc..0830cb7d74 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt @@ -4,11 +4,14 @@ import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test import com.bitwarden.core.CipherView import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.auth.repository.model.BreachCountResult import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.repository.model.DataState import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.data.tools.generator.repository.GeneratorRepository import com.x8bit.bitwarden.data.tools.generator.repository.util.FakeGeneratorRepository +import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult @@ -49,6 +52,8 @@ import java.util.UUID @Suppress("LargeClass") class VaultAddEditViewModelTest : BaseViewModelTest() { + private val authRepository: AuthRepository = mockk() + private val loginInitialState = createVaultAddItemState( typeContentViewState = createLoginTypeContentViewState(), ) @@ -402,7 +407,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { assertEquals( stateWithName.copy( - dialog = VaultAddEditState.DialogState.Error( + dialog = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), message = R.string.generic_error_message.asText(), ), ), @@ -449,7 +455,8 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { assertEquals( stateWithName.copy( - dialog = VaultAddEditState.DialogState.Error( + dialog = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), message = errorMessage.asText(), ), ), @@ -468,9 +475,9 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { val stateWithNoNameAndDialog = createVaultAddItemState( commonContentViewState = createCommonContentViewState(name = ""), - dialogState = VaultAddEditState.DialogState.Error( - R.string.validation_field_required - .asText(R.string.name.asText()), + dialogState = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), + message = R.string.validation_field_required.asText(R.string.name.asText()), ), ) @@ -492,9 +499,9 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { fun `HandleDialogDismiss will remove the current dialog`() = runTest { val errorState = createVaultAddItemState( vaultAddEditType = VaultAddEditType.AddItem, - dialogState = VaultAddEditState.DialogState.Error( - R.string.validation_field_required - .asText(R.string.name.asText()), + dialogState = VaultAddEditState.DialogState.Generic( + title = R.string.an_error_has_occurred.asText(), + message = R.string.validation_field_required.asText(R.string.name.asText()), ), ) @@ -687,24 +694,58 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { } @Test - fun `PasswordCheckerClick should emit ShowToast with 'Password Checker' message`() = - runTest { - val viewModel = createAddVaultItemViewModel() + fun `on CheckForBreachClick should process a password`() = runTest { + val cipherView = createMockCipherView(1) + val password = "Password" - viewModel.eventFlow.test { - viewModel - .actionChannel - .trySend(VaultAddEditAction.ItemType.LoginType.PasswordCheckerClick) + val loginState = loginInitialState.copy( + viewState = VaultAddEditState.ViewState.Content( + common = createCommonContentViewState(), + type = createLoginTypeContentViewState( + password = password, + ), + ), + ) - assertEquals( - VaultAddEditEvent.ShowToast( - "Password Checker".asText(), + val viewModel = createAddVaultItemViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = loginState, + vaultAddEditType = VaultAddEditType.AddItem, + ), + ) + + mutableVaultItemFlow.value = DataState.Loaded(data = cipherView) + + val breachCount = 5 + coEvery { + authRepository.getPasswordBreachCount(password) + } returns BreachCountResult.Success(breachCount = breachCount) + + viewModel.stateFlow.test { + assertEquals(loginState, awaitItem()) + viewModel.trySendAction(VaultAddEditAction.ItemType.LoginType.PasswordCheckerClick) + assertEquals( + loginState.copy( + dialog = VaultAddEditState.DialogState.Loading( + label = R.string.loading.asText(), ), - awaitItem(), - ) - } + ), + awaitItem(), + ) + + assertEquals( + loginState.copy( + dialog = VaultAddEditState.DialogState.Generic( + message = R.string.password_exposed.asText(breachCount), + ), + ), + awaitItem(), + ) } + coVerify(exactly = 1) { authRepository.getPasswordBreachCount(password) } + } + @Suppress("MaxLineLength") @Test fun `OpenPasswordGeneratorClick should emit NavigateToGeneratorModal with with password GeneratorMode`() = @@ -1289,6 +1330,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { vaultRepository = vaultRepository, generatorRepository = generatorRepository, resourceManager = resourceManager, + authRepository = authRepository, ) } @@ -1796,6 +1838,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { vaultRepository = vaultRepo, generatorRepository = generatorRepo, resourceManager = bitwardenResourceManager, + authRepository = authRepository, ) /**