PM-18496: Propagate prevalidateSso API error message (#4759)

This commit is contained in:
David Perez
2025-02-21 08:34:18 -06:00
committed by GitHub
parent 4943df24b3
commit 80bd1bfde2
11 changed files with 182 additions and 122 deletions

View File

@@ -53,7 +53,7 @@ interface UnauthenticatedIdentityApi {
@GET("/sso/prevalidate")
suspend fun prevalidateSso(
@Query("domainHint") organizationIdentifier: String,
): NetworkResult<PrevalidateSsoResponseJson>
): NetworkResult<PrevalidateSsoResponseJson.Success>
/**
* This call needs to be synchronous so we need it to return a [Call] directly. The identity

View File

@@ -7,6 +7,20 @@ import kotlinx.serialization.Serializable
* Response body from the SSO prevalidate request.
*/
@Serializable
data class PrevalidateSsoResponseJson(
@SerialName("token") val token: String?,
)
sealed class PrevalidateSsoResponseJson {
/**
* Models json body of a successful response.
*/
@Serializable
data class Success(
@SerialName("token") val token: String?,
) : PrevalidateSsoResponseJson()
/**
* Models json body of an error response.
*/
@Serializable
data class Error(
@SerialName("message") val message: String?,
) : PrevalidateSsoResponseJson()
}

View File

@@ -85,18 +85,23 @@ class IdentityServiceImpl(
.toResult()
.recoverCatching { throwable ->
val bitwardenError = throwable.toBitwardenError()
bitwardenError.parseErrorBodyOrNull<GetTokenResponseJson.CaptchaRequired>(
code = 400,
json = json,
) ?: bitwardenError.parseErrorBodyOrNull<GetTokenResponseJson.TwoFactorRequired>(
code = 400,
json = json,
) ?: bitwardenError.parseErrorBodyOrNull<GetTokenResponseJson.Invalid>(
code = 400,
json = json,
) ?: throw throwable
bitwardenError
.parseErrorBodyOrNull<GetTokenResponseJson.CaptchaRequired>(
code = 400,
json = json,
)
?: bitwardenError.parseErrorBodyOrNull<GetTokenResponseJson.TwoFactorRequired>(
code = 400,
json = json,
)
?: bitwardenError.parseErrorBodyOrNull<GetTokenResponseJson.Invalid>(
code = 400,
json = json,
)
?: throw throwable
}
@Suppress("MagicNumber")
override suspend fun prevalidateSso(
organizationIdentifier: String,
): Result<PrevalidateSsoResponseJson> = unauthenticatedIdentityApi
@@ -104,6 +109,15 @@ class IdentityServiceImpl(
organizationIdentifier = organizationIdentifier,
)
.toResult()
.recoverCatching { throwable ->
val bitwardenError = throwable.toBitwardenError()
bitwardenError
.parseErrorBodyOrNull<PrevalidateSsoResponseJson.Error>(
code = 400,
json = json,
)
?: throw throwable
}
override fun refreshTokenSynchronously(
refreshToken: String,
@@ -133,6 +147,7 @@ class IdentityServiceImpl(
?: throw throwable
}
@Suppress("MagicNumber")
override suspend fun sendVerificationEmail(
body: SendVerificationEmailRequestJson,
): Result<SendVerificationEmailResponseJson> {
@@ -151,6 +166,7 @@ class IdentityServiceImpl(
}
}
@Suppress("MagicNumber")
override suspend fun verifyEmailRegistrationToken(
body: VerifyEmailTokenRequestJson,
): Result<VerifyEmailTokenResponseJson> = unauthenticatedIdentityApi

View File

@@ -17,6 +17,7 @@ import com.x8bit.bitwarden.data.auth.datasource.network.model.DeviceDataModel
import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel
import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.PrevalidateSsoResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.RefreshTokenResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.RegisterFinishRequestJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.RegisterRequestJson
@@ -1199,13 +1200,21 @@ class AuthRepositoryImpl(
)
.fold(
onSuccess = {
if (it.token.isNullOrBlank()) {
PrevalidateSsoResult.Failure
} else {
PrevalidateSsoResult.Success(it.token)
when (it) {
is PrevalidateSsoResponseJson.Error -> {
PrevalidateSsoResult.Failure(message = it.message)
}
is PrevalidateSsoResponseJson.Success -> {
if (it.token.isNullOrBlank()) {
PrevalidateSsoResult.Failure()
} else {
PrevalidateSsoResult.Success(token = it.token)
}
}
}
},
onFailure = { PrevalidateSsoResult.Failure },
onFailure = { PrevalidateSsoResult.Failure() },
)
override fun setSsoCallbackResult(result: SsoCallbackResult) {

View File

@@ -14,5 +14,7 @@ sealed class PrevalidateSsoResult {
/**
* There was an error in prevalidation.
*/
data object Failure : PrevalidateSsoResult()
data class Failure(
val message: String? = null,
) : PrevalidateSsoResult()
}

View File

@@ -76,26 +76,12 @@ fun EnterpriseSignOnScreen(
}
}
when (val dialog = state.dialogState) {
is EnterpriseSignOnState.DialogState.Error -> {
BitwardenBasicDialog(
title = dialog
.title
?.invoke()
?: stringResource(id = R.string.an_error_has_occurred),
message = dialog.message(),
onDismissRequest = remember(viewModel) {
{ viewModel.trySendAction(EnterpriseSignOnAction.DialogDismiss) }
},
)
}
is EnterpriseSignOnState.DialogState.Loading -> {
BitwardenLoadingDialog(text = dialog.message())
}
null -> Unit
}
EnterpriseSignOnDialogs(
dialogState = state.dialogState,
onDismissRequest = remember(viewModel) {
{ viewModel.trySendAction(EnterpriseSignOnAction.DialogDismiss) }
},
)
val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState())
BitwardenScaffold(
@@ -174,3 +160,25 @@ private fun EnterpriseSignOnScreenContent(
Spacer(modifier = Modifier.navigationBarsPadding())
}
}
@Composable
private fun EnterpriseSignOnDialogs(
dialogState: EnterpriseSignOnState.DialogState?,
onDismissRequest: () -> Unit,
) {
when (dialogState) {
is EnterpriseSignOnState.DialogState.Error -> {
BitwardenBasicDialog(
title = dialogState.title(),
message = dialogState.message(),
onDismissRequest = onDismissRequest,
)
}
is EnterpriseSignOnState.DialogState.Loading -> {
BitwardenLoadingDialog(text = dialogState.message())
}
null -> Unit
}
}

View File

@@ -108,8 +108,8 @@ class EnterpriseSignOnViewModel @Inject constructor(
handleOnGenerateUriForSsoResult(action)
}
EnterpriseSignOnAction.Internal.OnSsoPrevalidationFailure -> {
handleOnSsoPrevalidationFailure()
is EnterpriseSignOnAction.Internal.OnSsoPrevalidationFailure -> {
handleOnSsoPrevalidationFailure(action)
}
is EnterpriseSignOnAction.Internal.OnOrganizationDomainSsoDetailsReceive -> {
@@ -146,7 +146,6 @@ class EnterpriseSignOnViewModel @Inject constructor(
prevalidateSso()
}
@Suppress("MaxLineLength", "LongMethod")
private fun handleOnLoginResult(action: EnterpriseSignOnAction.Internal.OnLoginResult) {
when (val loginResult = action.loginResult) {
is LoginResult.CaptchaRequired -> {
@@ -159,27 +158,19 @@ class EnterpriseSignOnViewModel @Inject constructor(
}
is LoginResult.Error -> {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
message = loginResult
.errorMessage
?.asText()
?: R.string.login_sso_error.asText(),
),
)
}
showError(
message = loginResult.errorMessage?.asText()
?: R.string.login_sso_error.asText(),
)
}
is LoginResult.UnofficialServerError -> {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
message = R.string.this_is_not_a_recognized_bitwarden_server_you_may_need_to_check_with_your_provider_or_update_your_server
.asText(),
),
)
}
@Suppress("MaxLineLength")
showError(
message = R.string
.this_is_not_a_recognized_bitwarden_server_you_may_need_to_check_with_your_provider_or_update_your_server
.asText(),
)
}
is LoginResult.Success -> {
@@ -198,25 +189,14 @@ class EnterpriseSignOnViewModel @Inject constructor(
}
LoginResult.CertificateError -> {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.we_couldnt_verify_the_servers_certificate.asText(),
),
)
}
showError(message = R.string.we_couldnt_verify_the_servers_certificate.asText())
}
is LoginResult.NewDeviceVerification -> {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
message = loginResult.errorMessage?.asText()
?: R.string.login_sso_error.asText(),
),
)
}
showError(
message = loginResult.errorMessage?.asText()
?: R.string.login_sso_error.asText(),
)
}
}
}
@@ -228,8 +208,10 @@ class EnterpriseSignOnViewModel @Inject constructor(
sendEvent(EnterpriseSignOnEvent.NavigateToSsoLogin(action.uri))
}
private fun handleOnSsoPrevalidationFailure() {
showDefaultError()
private fun handleOnSsoPrevalidationFailure(
action: EnterpriseSignOnAction.Internal.OnSsoPrevalidationFailure,
) {
showError(message = action.message?.asText() ?: R.string.login_sso_error.asText())
}
private fun handleOnOrganizationDomainSsoDetailsFailure() {
@@ -320,9 +302,10 @@ class EnterpriseSignOnViewModel @Inject constructor(
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.organization_sso_identifier_required.asText(),
),
orgIdentifierInput = authRepository.rememberedOrgIdentifier ?: "",
orgIdentifierInput = authRepository.rememberedOrgIdentifier.orEmpty(),
)
}
return
@@ -387,15 +370,11 @@ class EnterpriseSignOnViewModel @Inject constructor(
val organizationIdentifier = state.orgIdentifierInput
if (organizationIdentifier.isBlank()) {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
message = R.string.validation_field_required.asText(
R.string.org_identifier.asText(),
),
),
)
}
showError(
message = R.string.validation_field_required.asText(
R.string.org_identifier.asText(),
),
)
return
}
@@ -404,7 +383,11 @@ class EnterpriseSignOnViewModel @Inject constructor(
viewModelScope.launch {
when (val prevalidateSso = authRepository.prevalidateSso(organizationIdentifier)) {
is PrevalidateSsoResult.Failure -> {
sendAction(EnterpriseSignOnAction.Internal.OnSsoPrevalidationFailure)
sendAction(
action = EnterpriseSignOnAction.Internal.OnSsoPrevalidationFailure(
message = prevalidateSso.message,
),
)
}
is PrevalidateSsoResult.Success -> {
@@ -423,7 +406,7 @@ class EnterpriseSignOnViewModel @Inject constructor(
when (ssoCallbackResult) {
is SsoCallbackResult.MissingCode -> {
showDefaultError()
showError()
}
is SsoCallbackResult.Success -> {
@@ -442,7 +425,7 @@ class EnterpriseSignOnViewModel @Inject constructor(
sendAction(EnterpriseSignOnAction.Internal.OnLoginResult(result))
}
} else {
showDefaultError()
showError()
}
}
}
@@ -504,11 +487,15 @@ class EnterpriseSignOnViewModel @Inject constructor(
sendAction(EnterpriseSignOnAction.Internal.OnGenerateUriForSsoResult(Uri.parse(uri)))
}
private fun showDefaultError() {
private fun showError(
title: Text = R.string.an_error_has_occurred.asText(),
message: Text = R.string.login_sso_error.asText(),
) {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
message = R.string.login_sso_error.asText(),
title = title,
message = message,
),
)
}
@@ -544,7 +531,7 @@ data class EnterpriseSignOnState(
*/
@Parcelize
data class Error(
val title: Text? = null,
val title: Text,
val message: Text,
) : DialogState()
@@ -639,7 +626,9 @@ sealed class EnterpriseSignOnAction {
/**
* SSO prevalidation failed.
*/
data object OnSsoPrevalidationFailure : Internal()
data class OnSsoPrevalidationFailure(
val message: String?,
) : Internal()
/**
* A result was received when requesting an [OrganizationDomainSsoDetailsResult].

View File

@@ -290,22 +290,26 @@ class IdentityServiceTest : BaseServiceTest() {
assertEquals(LEGACY_INVALID_LOGIN.asSuccess(), result)
}
@Suppress("MaxLineLength")
@Test
fun `prevalidateSso when response is success should return PrevalidateSsoResponseJson`() =
fun `prevalidateSso when response is success should return PrevalidateSsoResponseJson Success`() =
runTest {
val organizationId = "organizationId"
server.enqueue(MockResponse().setResponseCode(200).setBody(PREVALIDATE_SSO_JSON))
server.enqueue(
MockResponse().setResponseCode(200).setBody(PREVALIDATE_SSO_SUCCESS_JSON),
)
val result = identityService.prevalidateSso(organizationId)
assertEquals(PREVALIDATE_SSO_BODY.asSuccess(), result)
assertEquals(PREVALIDATE_SSO_SUCCESS_BODY.asSuccess(), result)
}
@Suppress("MaxLineLength")
@Test
fun `prevalidateSso when response is an error should return an error`() =
fun `prevalidateSso when response is an error should return PrevalidateSsoResponseJson error`() =
runTest {
val organizationId = "organizationId"
server.enqueue(MockResponse().setResponseCode(400))
server.enqueue(MockResponse().setResponseCode(400).setBody(PREVALIDATE_SSO_ERROR_JSON))
val result = identityService.prevalidateSso(organizationId)
assertTrue(result.isFailure)
assertEquals(PREVALIDATE_SSO_ERROR_BODY.asSuccess(), result)
}
@Suppress("MaxLineLength")
@@ -496,16 +500,26 @@ class IdentityServiceTest : BaseServiceTest() {
}
}
private const val PREVALIDATE_SSO_JSON = """
private const val PREVALIDATE_SSO_SUCCESS_JSON = """
{
"token": "2ff00750-e2d6-47a6-ae54-67b981e78030"
}
"""
private val PREVALIDATE_SSO_BODY = PrevalidateSsoResponseJson(
private const val PREVALIDATE_SSO_ERROR_JSON = """
{
"message": "Organization not found from identifier."
}
"""
private val PREVALIDATE_SSO_SUCCESS_BODY = PrevalidateSsoResponseJson.Success(
token = "2ff00750-e2d6-47a6-ae54-67b981e78030",
)
private val PREVALIDATE_SSO_ERROR_BODY = PrevalidateSsoResponseJson.Error(
message = "Organization not found from identifier.",
)
private const val REFRESH_TOKEN_JSON = """
{
"access_token": "accessToken",

View File

@@ -5356,14 +5356,24 @@ class AuthRepositoryTest {
}
@Test
fun `prevalidateSso Failure should return Failure `() = runTest {
fun `prevalidateSso Failure should return Failure`() = runTest {
val organizationId = "organizationid"
val throwable = Throwable()
coEvery {
identityService.prevalidateSso(organizationId)
} returns throwable.asFailure()
val result = repository.prevalidateSso(organizationId)
assertEquals(PrevalidateSsoResult.Failure, result)
assertEquals(PrevalidateSsoResult.Failure(), result)
}
@Test
fun `prevalidateSso Failure response should return Failure with message`() = runTest {
val organizationId = "organizationid"
coEvery {
identityService.prevalidateSso(organizationId)
} returns PrevalidateSsoResponseJson.Error(message = "Fail").asSuccess()
val result = repository.prevalidateSso(organizationId)
assertEquals(PrevalidateSsoResult.Failure(message = "Fail"), result)
}
@Test
@@ -5371,9 +5381,9 @@ class AuthRepositoryTest {
val organizationId = "organizationid"
coEvery {
identityService.prevalidateSso(organizationId)
} returns PrevalidateSsoResponseJson(token = "").asSuccess()
} returns PrevalidateSsoResponseJson.Success(token = "").asSuccess()
val result = repository.prevalidateSso(organizationId)
assertEquals(PrevalidateSsoResult.Failure, result)
assertEquals(PrevalidateSsoResult.Failure(), result)
}
@Test
@@ -5381,7 +5391,7 @@ class AuthRepositoryTest {
val organizationId = "organizationid"
coEvery {
identityService.prevalidateSso(organizationId)
} returns PrevalidateSsoResponseJson(token = "token").asSuccess()
} returns PrevalidateSsoResponseJson.Success(token = "token").asSuccess()
val result = repository.prevalidateSso(organizationId)
assertEquals(PrevalidateSsoResult.Success(token = "token"), result)
}
@@ -7047,16 +7057,6 @@ class AuthRepositoryTest {
),
)
private val SINGLE_USER_STATE_1_NEW_ACCOUNT = UserStateJson(
activeUserId = USER_ID_1,
accounts = mapOf(
USER_ID_1 to ACCOUNT_1.copy(
profile = ACCOUNT_1.profile.copy(
creationDate = ZonedDateTime.parse("2024-09-14T01:00:00.00Z"),
),
),
),
)
private val SINGLE_USER_STATE_2 = UserStateJson(
activeUserId = USER_ID_2,
accounts = mapOf(

View File

@@ -143,6 +143,7 @@ class EnterpriseSignOnScreenTest : BaseComposeTest() {
mutableStateFlow.update {
it.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = "Error dialog title".asText(),
message = "Error dialog message".asText(),
),
)
@@ -151,7 +152,7 @@ class EnterpriseSignOnScreenTest : BaseComposeTest() {
composeTestRule.onNode(isDialog()).assertIsDisplayed()
composeTestRule
.onNodeWithText("An error has occurred.")
.onNodeWithText("Error dialog title")
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
@@ -188,6 +189,7 @@ class EnterpriseSignOnScreenTest : BaseComposeTest() {
mutableStateFlow.update {
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = "title".asText(),
message = "message".asText(),
),
)

View File

@@ -117,7 +117,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
coEvery {
authRepository.prevalidateSso(organizationId)
} returns PrevalidateSsoResult.Failure
} returns PrevalidateSsoResult.Failure()
val viewModel = createViewModel(state)
viewModel.stateFlow.test {
@@ -136,6 +136,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
state.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.login_sso_error.asText(),
),
),
@@ -198,6 +199,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.validation_field_required.asText(
R.string.org_identifier.asText(),
),
@@ -288,6 +290,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.login_sso_error.asText(),
),
),
@@ -305,6 +308,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.login_sso_error.asText(),
),
),
@@ -358,6 +362,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.login_sso_error.asText(),
),
orgIdentifierInput = orgIdentifier,
@@ -424,6 +429,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = "new device verification required".asText(),
),
orgIdentifierInput = orgIdentifier,
@@ -847,6 +853,7 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
dialogState = EnterpriseSignOnState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.organization_sso_identifier_required.asText(),
),
orgIdentifierInput = "Bitwarden",
@@ -1040,6 +1047,5 @@ class EnterpriseSignOnViewModelTest : BaseViewModelTest() {
codeVerifier = "def",
)
private const val DEFAULT_EMAIL = "test@gmail.com"
private const val DEFAULT_ORG_ID = "orgId"
}
}