PM-19245: Propagate error from password validation to UI (#4877)

This commit is contained in:
David Perez 2025-03-17 10:36:01 -05:00 committed by GitHub
parent 6c784d28eb
commit 7f8e848c46
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 97 additions and 75 deletions

View File

@ -99,6 +99,7 @@ import com.x8bit.bitwarden.data.auth.util.YubiKeyResult
import com.x8bit.bitwarden.data.auth.util.toSdkParams
import com.x8bit.bitwarden.data.platform.datasource.disk.ConfigDiskSource
import com.x8bit.bitwarden.data.platform.datasource.network.util.isSslHandShakeError
import com.x8bit.bitwarden.data.platform.error.MissingPropertyException
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
@ -1249,7 +1250,7 @@ class AuthRepositoryImpl(
)
override suspend fun validatePassword(password: String): ValidatePasswordResult {
val userId = activeUserId ?: return ValidatePasswordResult.Error
val userId = activeUserId ?: return ValidatePasswordResult.Error(NoActiveUserException())
return authDiskSource
.getMasterPasswordHash(userId = userId)
?.let { masterPasswordHash ->
@ -1261,13 +1262,13 @@ class AuthRepositoryImpl(
)
.fold(
onSuccess = { ValidatePasswordResult.Success(isValid = it) },
onFailure = { ValidatePasswordResult.Error },
onFailure = { ValidatePasswordResult.Error(error = it) },
)
}
?: run {
val encryptedKey = authDiskSource
.getUserKey(userId)
?: return ValidatePasswordResult.Error
?: return ValidatePasswordResult.Error(MissingPropertyException("UserKey"))
vaultSdkSource
.validatePasswordUserKey(
userId = userId,

View File

@ -15,5 +15,7 @@ sealed class ValidatePasswordResult {
/**
* There was an error determining if the validity of the password.
*/
data object Error : ValidatePasswordResult()
data class Error(
val error: Throwable,
) : ValidatePasswordResult()
}

View File

@ -0,0 +1,8 @@
package com.x8bit.bitwarden.data.platform.error
/**
* An exception indicating that a required property was missing.
*/
class MissingPropertyException(
propertyName: String,
) : IllegalStateException("Missing the required $propertyName property")

View File

@ -309,14 +309,15 @@ class ResetPasswordViewModel @Inject constructor(
private fun handleReceiveValidatePasswordResult(
action: ResetPasswordAction.Internal.ReceiveValidatePasswordResult,
) {
when (action.result) {
when (val result = action.result) {
// Display an alert if there was an error.
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = result.error,
),
)
}
@ -441,6 +442,7 @@ data class ResetPasswordState(
data class Error(
val title: Text?,
val message: Text,
val error: Throwable? = null,
) : DialogState()
/**

View File

@ -563,13 +563,14 @@ class SearchViewModel @Inject constructor(
private fun handleValidatePasswordResultReceive(
action: SearchAction.Internal.ValidatePasswordResultReceive,
) {
when (action.result) {
ValidatePasswordResult.Error -> {
when (val result = action.result) {
is ValidatePasswordResult.Error -> {
mutableStateFlow.update {
it.copy(
dialogState = SearchState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
throwable = result.error,
),
)
}

View File

@ -124,6 +124,7 @@ fun ExportVaultScreen(
BitwardenBasicDialog(
title = dialog.title?.invoke(),
message = dialog.message(),
throwable = dialog.error,
onDismissRequest = remember(viewModel) {
{ viewModel.trySendAction(ExportVaultAction.DialogDismiss) }
},

View File

@ -304,9 +304,12 @@ class ExportVaultViewModel @Inject constructor(
private fun handleReceiveValidatePasswordResult(
action: ExportVaultAction.Internal.ReceiveValidatePasswordResult,
) {
when (action.result) {
ValidatePasswordResult.Error -> {
updateStateWithError(R.string.generic_error_message.asText())
when (val result = action.result) {
is ValidatePasswordResult.Error -> {
updateStateWithError(
message = R.string.generic_error_message.asText(),
error = result.error,
)
}
is ValidatePasswordResult.Success -> {
@ -435,12 +438,13 @@ class ExportVaultViewModel @Inject constructor(
}
}
private fun updateStateWithError(message: Text) {
private fun updateStateWithError(message: Text, error: Throwable? = null) {
mutableStateFlow.update {
it.copy(
dialogState = ExportVaultState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = message,
error = error,
),
)
}
@ -475,6 +479,7 @@ data class ExportVaultState(
data class Error(
val title: Text? = null,
val message: Text,
val error: Throwable? = null,
) : DialogState()
/**

View File

@ -1839,7 +1839,7 @@ class VaultAddEditViewModel @Inject constructor(
clearDialogState()
when (action.result) {
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
showFido2ErrorDialog()
}

View File

@ -1212,10 +1212,11 @@ class VaultItemViewModel @Inject constructor(
action: VaultItemAction.Internal.ValidatePasswordReceive,
) {
when (val result = action.result) {
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
updateDialogState(
VaultItemState.DialogState.Generic(
message = R.string.generic_error_message.asText(),
error = result.error,
),
)
}

View File

@ -1306,14 +1306,13 @@ class VaultItemListingViewModel @Inject constructor(
clearDialogState()
when (val result = action.result) {
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
mutableStateFlow.update {
it.copy(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
// TODO PM-19425 update ValidatePasswordResult to propagate error.
throwable = null,
throwable = result.error,
),
)
}
@ -1326,8 +1325,6 @@ class VaultItemListingViewModel @Inject constructor(
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.invalid_master_password.asText(),
// TODO PM-19425 update ValidatePasswordResult to propagate error.
throwable = null,
),
)
}
@ -1384,7 +1381,7 @@ class VaultItemListingViewModel @Inject constructor(
clearDialogState()
when (action.result) {
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
showFido2UserVerificationErrorDialog()
}

View File

@ -419,6 +419,7 @@ private fun VaultDialogs(
is VaultState.DialogState.Error -> BitwardenBasicDialog(
title = dialogState.title(),
message = dialogState.message(),
throwable = dialogState.error,
onDismissRequest = vaultHandlers.dialogDismiss,
)

View File

@ -782,12 +782,13 @@ class VaultViewModel @Inject constructor(
action: VaultAction.Internal.ValidatePasswordResultReceive,
) {
when (val result = action.result) {
ValidatePasswordResult.Error -> {
is ValidatePasswordResult.Error -> {
mutableStateFlow.update {
it.copy(
dialog = VaultState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = result.error,
),
)
}
@ -1165,6 +1166,7 @@ data class VaultState(
data class Error(
val title: Text,
val message: Text,
val error: Throwable? = null,
) : DialogState()
}
}

View File

@ -105,6 +105,7 @@ import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.datasource.disk.model.ServerConfig
import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeConfigDiskSource
import com.x8bit.bitwarden.data.platform.datasource.network.model.ConfigResponseJson
import com.x8bit.bitwarden.data.platform.error.MissingPropertyException
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
@ -297,10 +298,16 @@ class AuthRepositoryTest {
GetTokenResponseJson.Success::toUserState,
UserStateJson::toRemovedPasswordUserStateJson,
)
mockkConstructor(NoActiveUserException::class)
mockkConstructor(
NoActiveUserException::class,
MissingPropertyException::class,
)
every {
anyConstructed<NoActiveUserException>() == any<NoActiveUserException>()
} returns true
every {
anyConstructed<MissingPropertyException>() == any<MissingPropertyException>()
} returns true
}
@AfterEach
@ -309,7 +316,10 @@ class AuthRepositoryTest {
GetTokenResponseJson.Success::toUserState,
UserStateJson::toRemovedPasswordUserStateJson,
)
unmockkStatic(NoActiveUserException::class)
unmockkStatic(
NoActiveUserException::class,
MissingPropertyException::class,
)
}
@Test
@ -5761,25 +5771,13 @@ class AuthRepositoryTest {
@Test
fun `validatePassword with no current user returns ValidatePasswordResult Error`() = runTest {
val userId = "userId"
val password = "password"
val passwordHash = "passwordHash"
fakeAuthDiskSource.userState = null
coEvery {
vaultSdkSource.validatePassword(
userId = userId,
password = password,
passwordHash = passwordHash,
)
} returns true.asSuccess()
val result = repository
.validatePassword(
password = password,
)
val result = repository.validatePassword(password = password)
assertEquals(
ValidatePasswordResult.Error,
ValidatePasswordResult.Error(error = NoActiveUserException()),
result,
)
}
@ -5800,13 +5798,10 @@ class AuthRepositoryTest {
)
} returns true.asSuccess()
val result = repository
.validatePassword(
password = password,
)
val result = repository.validatePassword(password = password)
assertEquals(
ValidatePasswordResult.Error,
ValidatePasswordResult.Error(error = MissingPropertyException("UserKey")),
result,
)
}
@ -5816,6 +5811,7 @@ class AuthRepositoryTest {
val userId = USER_ID_1
val password = "password"
val passwordHash = "passwordHash"
val error = Throwable("Fail!")
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
fakeAuthDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = passwordHash)
coEvery {
@ -5824,15 +5820,12 @@ class AuthRepositoryTest {
password = password,
passwordHash = passwordHash,
)
} returns Throwable().asFailure()
} returns error.asFailure()
val result = repository
.validatePassword(
password = password,
)
val result = repository.validatePassword(password = password)
assertEquals(
ValidatePasswordResult.Error,
ValidatePasswordResult.Error(error = error),
result,
)
}
@ -5852,10 +5845,7 @@ class AuthRepositoryTest {
)
} returns true.asSuccess()
val result = repository
.validatePassword(
password = password,
)
val result = repository.validatePassword(password = password)
assertEquals(
ValidatePasswordResult.Success(isValid = true),

View File

@ -170,12 +170,13 @@ class ResetPasswordViewModelTest : BaseViewModelTest() {
fun `SubmitClicked with error for validating current password shows error alert`() {
val currentPassword = "CurrentTest123"
val password = "Test123"
val error = Throwable("Fail!")
coEvery {
authRepository.validatePasswordAgainstPolicies(password)
} returns true
coEvery {
authRepository.validatePassword(currentPassword)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
coEvery {
authRepository.getPasswordStrength(password = any())
} returns PasswordStrengthResult.Success(passwordStrength = PasswordStrength.LEVEL_0)
@ -192,6 +193,7 @@ class ResetPasswordViewModelTest : BaseViewModelTest() {
dialogState = ResetPasswordState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = error,
),
currentPasswordInput = currentPassword,
passwordInput = password,

View File

@ -444,9 +444,10 @@ class SearchViewModelTest : BaseViewModelTest() {
setupForAutofill()
val cipherId = CIPHER_ID
val password = "password"
val error = Throwable("Fail!")
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
val viewModel = createViewModel()
assertEquals(
INITIAL_STATE_FOR_AUTOFILL,
@ -467,6 +468,7 @@ class SearchViewModelTest : BaseViewModelTest() {
dialogState = SearchState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
throwable = error,
),
),
viewModel.stateFlow.value,

View File

@ -385,11 +385,12 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
@Test
fun `ConfirmExportVaultClicked error checking password should show an error`() {
val password = "password"
val error = Throwable("Fail!")
coEvery {
authRepository.validatePassword(
password = password,
)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
val viewModel = createViewModel()
viewModel.trySendAction(ExportVaultAction.PasswordInputChanged(password))
@ -400,6 +401,7 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
dialogState = ExportVaultState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = error,
),
passwordInput = password,
),

View File

@ -3973,7 +3973,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = Throwable("Fail!"))
viewModel.trySendAction(
VaultAddEditAction.Common.MasterPasswordFido2VerificationSubmit(

View File

@ -899,11 +899,11 @@ class VaultItemViewModelTest : BaseViewModelTest() {
relatedLocations = persistentListOf(),
)
} returns loginViewState
val password = "password"
val error = Throwable("Fail!")
coEvery {
authRepo.validatePassword(password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
mutableCollectionsStateFlow.value = DataState.Loaded(emptyList())
@ -932,6 +932,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
loginState.copy(
dialog = VaultItemState.DialogState.Generic(
message = R.string.generic_error_message.asText(),
error = error,
),
),
awaitItem(),

View File

@ -817,6 +817,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
val cipherView = createMockCipherView(number = 1)
val cipherId = "mockId-1"
val password = "password"
val error = Throwable("Fail!")
mutableVaultDataStateFlow.value = DataState.Loaded(
data = VaultData(
cipherViewList = listOf(cipherView),
@ -828,7 +829,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
val viewModel = createVaultItemListingViewModel()
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
val initialState = createVaultItemListingState(
viewState = VaultItemListingState.ViewState.Content(
displayCollectionList = emptyList(),
@ -858,6 +859,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
dialogState = VaultItemListingState.DialogState.Error(
title = null,
message = R.string.generic_error_message.asText(),
throwable = error,
),
),
viewModel.stateFlow.value,
@ -4041,7 +4043,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
val password = "password"
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = Throwable("Fail!"))
viewModel.trySendAction(
VaultItemListingsAction.MasterPasswordFido2VerificationSubmit(

View File

@ -1608,9 +1608,10 @@ class VaultViewModelTest : BaseViewModelTest() {
fun `MasterPasswordRepromptSubmit for a request Error should show a generic error dialog`() =
runTest {
val password = "password"
val error = Throwable("Fail!")
coEvery {
authRepository.validatePassword(password = password)
} returns ValidatePasswordResult.Error
} returns ValidatePasswordResult.Error(error = error)
val viewModel = createViewModel()
viewModel.stateFlow.test {
@ -1635,6 +1636,7 @@ class VaultViewModelTest : BaseViewModelTest() {
dialog = VaultState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = error,
),
),
awaitItem(),
@ -1943,22 +1945,22 @@ class VaultViewModelTest : BaseViewModelTest() {
)
}
@Suppress("MaxLineLength")
@Test
fun `InternetConnectionErrorReceived should show network error if no internet connection`() = runTest {
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.Internal.InternetConnectionErrorReceived)
assertEquals(
DEFAULT_STATE.copy(
isRefreshing = false,
dialog = VaultState.DialogState.Error(
R.string.internet_connection_required_title.asText(),
R.string.internet_connection_required_message.asText(),
fun `InternetConnectionErrorReceived should show network error if no internet connection`() =
runTest {
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.Internal.InternetConnectionErrorReceived)
assertEquals(
DEFAULT_STATE.copy(
isRefreshing = false,
dialog = VaultState.DialogState.Error(
R.string.internet_connection_required_title.asText(),
R.string.internet_connection_required_message.asText(),
),
),
),
viewModel.stateFlow.value,
)
}
viewModel.stateFlow.value,
)
}
private fun createViewModel(): VaultViewModel =
VaultViewModel(