PM-19296: Propagate login errors to the UI (#4885)

This commit is contained in:
David Perez
2025-03-18 09:05:07 -05:00
committed by GitHub
parent ef3b7730d0
commit a040a38ce8
15 changed files with 99 additions and 38 deletions

View File

@@ -585,10 +585,13 @@ class AuthRepositoryImpl(
asymmetricalKey: String,
): LoginResult {
val profile = authDiskSource.userState?.activeAccount?.profile
?: return LoginResult.Error(errorMessage = null)
?: return LoginResult.Error(errorMessage = null, error = NoActiveUserException())
val userId = profile.userId
val privateKey = authDiskSource.getPrivateKey(userId = userId)
?: return LoginResult.Error(errorMessage = null)
?: return LoginResult.Error(
errorMessage = null,
error = MissingPropertyException("Private Key"),
)
checkForVaultUnlockError(
onVaultUnlockError = { error ->
@@ -638,7 +641,7 @@ class AuthRepositoryImpl(
onFailure = { throwable ->
when {
throwable.isSslHandShakeError() -> LoginResult.CertificateError
else -> LoginResult.Error(errorMessage = null)
else -> LoginResult.Error(errorMessage = null, error = throwable)
}
},
onSuccess = { it },
@@ -687,7 +690,10 @@ class AuthRepositoryImpl(
orgIdentifier = orgIdentifier,
)
}
?: LoginResult.Error(errorMessage = null)
?: LoginResult.Error(
errorMessage = null,
error = MissingPropertyException("Identity Token Auth Model"),
)
override suspend fun login(
email: String,
@@ -707,7 +713,10 @@ class AuthRepositoryImpl(
orgIdentifier = orgIdentifier,
)
}
?: LoginResult.Error(errorMessage = null)
?: LoginResult.Error(
errorMessage = null,
error = MissingPropertyException("Identity Token Auth Model"),
)
override suspend fun login(
email: String,
@@ -1645,7 +1654,10 @@ class AuthRepositoryImpl(
LoginResult.UnofficialServerError
}
else -> LoginResult.Error(errorMessage = null)
else -> LoginResult.Error(
errorMessage = null,
error = throwable,
)
}
},
onSuccess = { loginResponse ->
@@ -1681,6 +1693,7 @@ class AuthRepositoryImpl(
is GetTokenResponseJson.Invalid.InvalidType.GenericInvalid -> {
LoginResult.Error(
errorMessage = loginResponse.errorMessage,
error = null,
)
}
}

View File

@@ -22,7 +22,10 @@ sealed class LoginResult {
/**
* There was an error logging in.
*/
data class Error(val errorMessage: String?) : LoginResult()
data class Error(
val errorMessage: String?,
val error: Throwable?,
) : LoginResult()
/**
* There was an error while logging into an unofficial Bitwarden server.

View File

@@ -8,9 +8,12 @@ import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult
* the necessary `message` if applicable.
*/
fun VaultUnlockError.toLoginErrorResult(): LoginResult.Error = when (this) {
is VaultUnlockResult.AuthenticationError -> LoginResult.Error(this.message)
is VaultUnlockResult.AuthenticationError -> {
LoginResult.Error(errorMessage = this.message, error = this.error)
}
is VaultUnlockResult.BiometricDecodingError,
is VaultUnlockResult.GenericError,
is VaultUnlockResult.InvalidStateError,
-> LoginResult.Error(errorMessage = null)
-> LoginResult.Error(errorMessage = null, error = this.error)
}

View File

@@ -161,6 +161,7 @@ class EnterpriseSignOnViewModel @Inject constructor(
showError(
message = loginResult.errorMessage?.asText()
?: R.string.login_sso_error.asText(),
error = loginResult.error,
)
}

View File

@@ -208,6 +208,7 @@ private fun LoginDialogs(
is LoginState.DialogState.Error -> BitwardenBasicDialog(
title = dialogState.title?.invoke(),
message = dialogState.message(),
throwable = dialogState.error,
onDismissRequest = onDismissRequest,
)

View File

@@ -172,6 +172,7 @@ class LoginViewModel @Inject constructor(
title = R.string.an_error_has_occurred.asText(),
message = loginResult.errorMessage?.asText()
?: R.string.generic_error_message.asText(),
error = loginResult.error,
),
)
}
@@ -326,6 +327,7 @@ data class LoginState(
data class Error(
val title: Text? = null,
val message: Text,
val error: Throwable? = null,
) : DialogState()
/**

View File

@@ -236,6 +236,7 @@ class LoginWithDeviceViewModel @Inject constructor(
.errorMessage
?.asText()
?: R.string.generic_error_message.asText(),
error = loginResult.error,
),
)
}

View File

@@ -202,6 +202,7 @@ private fun TwoFactorLoginDialogs(
?.invoke()
?: stringResource(R.string.an_error_has_occurred),
message = dialogState.message(),
throwable = dialogState.error,
onDismissRequest = onDismissRequest,
)

View File

@@ -308,6 +308,7 @@ class TwoFactorLoginViewModel @Inject constructor(
title = R.string.an_error_has_occurred.asText(),
message = loginResult.errorMessage?.asText()
?: R.string.invalid_verification_code.asText(),
error = loginResult.error,
),
)
}
@@ -658,6 +659,7 @@ data class TwoFactorLoginState(
data class Error(
val title: Text? = null,
val message: Text,
val error: Throwable? = null,
) : DialogState()
/**

View File

@@ -1365,7 +1365,10 @@ class AuthRepositoryTest {
requestPrivateKey = requestPrivateKey,
asymmetricalKey = asymmetricalKey,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(
LoginResult.Error(errorMessage = null, error = NoActiveUserException()),
result,
)
}
@Test
@@ -1377,7 +1380,10 @@ class AuthRepositoryTest {
requestPrivateKey = requestPrivateKey,
asymmetricalKey = asymmetricalKey,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(
LoginResult.Error(errorMessage = null, error = MissingPropertyException("Private Key")),
result,
)
}
@Test
@@ -1474,16 +1480,17 @@ class AuthRepositoryTest {
vaultRepository.syncIfNecessary()
settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1)
}
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
}
@Test
fun `login when pre login fails should return Error with no message`() = runTest {
val error = RuntimeException()
coEvery {
identityService.preLogin(email = EMAIL)
} returns RuntimeException().asFailure()
} returns error.asFailure()
val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify { identityService.preLogin(email = EMAIL) }
}
@@ -1492,6 +1499,7 @@ class AuthRepositoryTest {
@Test
fun `login get token fails should return Error with no message when server is an official Bitwarden server`() =
runTest {
val error = RuntimeException()
coEvery {
identityService.preLogin(email = EMAIL)
} returns PRE_LOGIN_SUCCESS.asSuccess()
@@ -1505,9 +1513,9 @@ class AuthRepositoryTest {
captchaToken = null,
uniqueAppId = UNIQUE_APP_ID,
)
} returns RuntimeException().asFailure()
} returns error.asFailure()
val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify { identityService.preLogin(email = EMAIL) }
coVerify {
@@ -1609,7 +1617,7 @@ class AuthRepositoryTest {
.asSuccess()
val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null)
assertEquals(LoginResult.Error(errorMessage = "mock_error_message"), result)
assertEquals(LoginResult.Error(errorMessage = "mock_error_message", error = null), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify { identityService.preLogin(email = EMAIL) }
coVerify {
@@ -1790,7 +1798,10 @@ class AuthRepositoryTest {
)
} returns SINGLE_USER_STATE_1
val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null)
assertEquals(LoginResult.Error(errorMessage = expectedErrorMessage), result)
assertEquals(
LoginResult.Error(errorMessage = expectedErrorMessage, error = error),
result,
)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify { identityService.preLogin(email = EMAIL) }
fakeAuthDiskSource.assertPrivateKey(
@@ -2262,7 +2273,7 @@ class AuthRepositoryTest {
captchaToken = null,
orgIdentifier = null,
)
assertEquals(LoginResult.Error(errorMessage = null), finalResult)
assertEquals(LoginResult.Error(errorMessage = null, error = error), finalResult)
assertEquals(twoFactorResponse, repository.twoFactorResponse)
fakeAuthDiskSource.assertTwoFactorToken(
email = EMAIL,
@@ -2374,11 +2385,18 @@ class AuthRepositoryTest {
captchaToken = null,
orgIdentifier = null,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(
LoginResult.Error(
errorMessage = null,
error = MissingPropertyException("Identity Token Auth Model"),
),
result,
)
}
@Test
fun `login with device get token fails should return Error with no message`() = runTest {
val error = Throwable("Fail!")
coEvery {
identityService.getToken(
email = EMAIL,
@@ -2390,7 +2408,7 @@ class AuthRepositoryTest {
captchaToken = null,
uniqueAppId = UNIQUE_APP_ID,
)
} returns Throwable("Fail").asFailure()
} returns error.asFailure()
val result = repository.login(
email = EMAIL,
requestId = DEVICE_REQUEST_ID,
@@ -2400,7 +2418,7 @@ class AuthRepositoryTest {
masterPasswordHash = PASSWORD_HASH,
captchaToken = null,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify {
identityService.getToken(
@@ -2447,7 +2465,10 @@ class AuthRepositoryTest {
masterPasswordHash = PASSWORD_HASH,
captchaToken = null,
)
assertEquals(LoginResult.Error(errorMessage = "mock_error_message"), result)
assertEquals(
LoginResult.Error(errorMessage = "mock_error_message", error = null),
result,
)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify {
identityService.getToken(
@@ -2849,6 +2870,7 @@ class AuthRepositoryTest {
@Test
fun `SSO login get token fails should return Error with no message`() = runTest {
val error = RuntimeException()
coEvery {
identityService.getToken(
email = EMAIL,
@@ -2860,7 +2882,7 @@ class AuthRepositoryTest {
captchaToken = null,
uniqueAppId = UNIQUE_APP_ID,
)
} returns RuntimeException().asFailure()
} returns error.asFailure()
val result = repository.login(
email = EMAIL,
ssoCode = SSO_CODE,
@@ -2869,7 +2891,7 @@ class AuthRepositoryTest {
captchaToken = null,
organizationIdentifier = ORGANIZATION_IDENTIFIER,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify {
identityService.getToken(
@@ -2914,7 +2936,7 @@ class AuthRepositoryTest {
captchaToken = null,
organizationIdentifier = ORGANIZATION_IDENTIFIER,
)
assertEquals(LoginResult.Error(errorMessage = "mock_error_message"), result)
assertEquals(LoginResult.Error(errorMessage = "mock_error_message", error = null), result)
assertEquals(AuthState.Unauthenticated, repository.authStateFlow.value)
coVerify {
identityService.getToken(
@@ -3059,6 +3081,7 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `SSO login get token succeeds with key connector and no master password should return failure`() =
runTest {
val error = Throwable("Fail!")
val keyConnectorUrl = "www.example.com"
val successResponse = GET_TOKEN_RESPONSE_SUCCESS.copy(
keyConnectorUrl = keyConnectorUrl,
@@ -3084,7 +3107,7 @@ class AuthRepositoryTest {
url = keyConnectorUrl,
accessToken = ACCESS_TOKEN,
)
} returns Throwable("Fail").asFailure()
} returns error.asFailure()
every {
successResponse.toUserState(
previousUserState = null,
@@ -3101,7 +3124,7 @@ class AuthRepositoryTest {
organizationIdentifier = ORGANIZATION_IDENTIFIER,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
fakeAuthDiskSource.assertPrivateKey(userId = USER_ID_1, privateKey = null)
fakeAuthDiskSource.assertUserKey(userId = USER_ID_1, userKey = null)
coVerify(exactly = 1) {
@@ -3227,6 +3250,7 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `SSO login get token succeeds with key connector, no master password, no key and no private key should return failure`() =
runTest {
val error = Throwable("Fail!")
val keyConnectorUrl = "www.example.com"
val successResponse = GET_TOKEN_RESPONSE_SUCCESS.copy(
keyConnectorUrl = keyConnectorUrl,
@@ -3259,7 +3283,7 @@ class AuthRepositoryTest {
kdfParallelism = PROFILE_1.kdfParallelism,
organizationIdentifier = ORGANIZATION_IDENTIFIER,
)
} returns Throwable("Fail").asFailure()
} returns error.asFailure()
every {
successResponse.toUserState(
previousUserState = null,
@@ -3276,7 +3300,7 @@ class AuthRepositoryTest {
organizationIdentifier = ORGANIZATION_IDENTIFIER,
)
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
fakeAuthDiskSource.assertPrivateKey(userId = USER_ID_1, privateKey = null)
fakeAuthDiskSource.assertUserKey(userId = USER_ID_1, userKey = null)
coVerify(exactly = 1) {

View File

@@ -16,7 +16,7 @@ class LoginResultExtensionsTest {
error = error,
)
.toLoginErrorResult()
assertEquals(LoginResult.Error(errorMessage), result)
assertEquals(LoginResult.Error(errorMessage = errorMessage, error = error), result)
}
@Test
@@ -24,7 +24,7 @@ class LoginResultExtensionsTest {
fun `VaultUnlockResult with null error message as default maps to LoginResult Error with null message`() {
val error = Throwable("Fail")
val result = VaultUnlockResult.AuthenticationError(error = error).toLoginErrorResult()
assertEquals(LoginResult.Error(errorMessage = null), result)
assertEquals(LoginResult.Error(errorMessage = null, error = error), result)
}
@Test
@@ -36,7 +36,7 @@ class LoginResultExtensionsTest {
val genericErrorResult = VaultUnlockResult.GenericError(error = error).toLoginErrorResult()
val biometricErrorResult =
VaultUnlockResult.BiometricDecodingError(error = error).toLoginErrorResult()
val expectedResult = LoginResult.Error(errorMessage = null)
val expectedResult = LoginResult.Error(errorMessage = null, error = error)
assertEquals(expectedResult, invalidStateResult)
assertEquals(expectedResult, genericErrorResult)

View File

@@ -323,9 +323,10 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
fun `ssoCallbackResultFlow Success with same state with login Error should show loading dialog then show an error when server is an official Bitwarden server`() =
runTest {
val orgIdentifier = "Bitwarden"
val error = Throwable("Fail!")
coEvery {
authRepository.login(any(), any(), any(), any(), any(), any())
} returns LoginResult.Error(null)
} returns LoginResult.Error(errorMessage = null, error = error)
val viewModel = createViewModel(
ssoData = DEFAULT_SSO_DATA,
@@ -366,6 +367,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.login_sso_error.asText(),
error = error,
),
orgIdentifierInput = orgIdentifier,
),

View File

@@ -247,13 +247,14 @@ class LoginViewModelTest : BaseViewModelTest() {
@Test
fun `LoginButtonClick login returns error should update errorDialogState`() = runTest {
val error = Throwable("Fail!")
coEvery {
authRepository.login(
email = EMAIL,
password = "",
captchaToken = null,
)
} returns LoginResult.Error(errorMessage = "mock_error")
} returns LoginResult.Error(errorMessage = "mock_error", error = error)
val viewModel = createViewModel()
viewModel.stateFlow.test {
assertEquals(DEFAULT_STATE, awaitItem())
@@ -271,6 +272,7 @@ class LoginViewModelTest : BaseViewModelTest() {
dialogState = LoginState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = "mock_error".asText(),
error = error,
),
),
awaitItem(),

View File

@@ -323,6 +323,7 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() {
@Test
fun `on createAuthRequestWithUpdates Success and login error should should update the state`() =
runTest {
val error = Throwable("Fail!")
coEvery {
authRepository.login(
email = EMAIL,
@@ -333,7 +334,7 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() {
masterPasswordHash = DEFAULT_LOGIN_DATA.masterPasswordHash,
captchaToken = null,
)
} returns LoginResult.Error(null)
} returns LoginResult.Error(errorMessage = null, error = error)
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.stateFlow.test {
@@ -365,6 +366,7 @@ class LoginWithDeviceViewModelTest : BaseViewModelTest() {
dialogState = LoginWithDeviceState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
error = error,
),
loginData = DEFAULT_LOGIN_DATA,
),

View File

@@ -581,6 +581,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
@Test
fun `ContinueButtonClick login returns Error should update dialogState`() = runTest {
val error = Throwable("Fail!")
coEvery {
authRepository.login(
email = DEFAULT_EMAIL_ADDRESS,
@@ -593,7 +594,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
captchaToken = null,
orgIdentifier = DEFAULT_ORG_IDENTIFIER,
)
} returns LoginResult.Error(errorMessage = null)
} returns LoginResult.Error(errorMessage = null, error = error)
val viewModel = createViewModel()
viewModel.stateFlow.test {
@@ -614,6 +615,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
dialogState = TwoFactorLoginState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.invalid_verification_code.asText(),
error = error,
),
),
awaitItem(),
@@ -640,6 +642,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
@Test
fun `ContinueButtonClick login returns Error with message should update dialogState`() =
runTest {
val error = Throwable("Fail!")
coEvery {
authRepository.login(
email = DEFAULT_EMAIL_ADDRESS,
@@ -652,7 +655,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
captchaToken = null,
orgIdentifier = DEFAULT_ORG_IDENTIFIER,
)
} returns LoginResult.Error(errorMessage = "Mock error message")
} returns LoginResult.Error(errorMessage = "Mock error message", error = error)
val viewModel = createViewModel()
viewModel.stateFlow.test {
@@ -673,6 +676,7 @@ class TwoFactorLoginViewModelTest : BaseViewModelTest() {
dialogState = TwoFactorLoginState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = "Mock error message".asText(),
error = error,
),
),
awaitItem(),