BIT-910 Parse server error messages on create account request (#164)

This commit is contained in:
Andrew Haisting 2023-10-26 13:29:30 -05:00 committed by Álison Fernandes
parent 249c1010f5
commit fd9ba2550f
7 changed files with 199 additions and 17 deletions

View file

@ -42,4 +42,30 @@ sealed class RegisterResponseJson {
val captchaKeys: List<String>,
)
}
/**
* Represents the json body of an invalid register request.
*
* @param message
* @param validationErrors a map where each value is a list of error messages for each key.
* The values in the array should be used for display to the user, since the keys tend to come
* back as nonsense. (eg: empty string key)
*/
@Serializable
data class Invalid(
@SerialName("message")
val message: String?,
@SerialName("validationErrors")
val validationErrors: Map<String, List<String>>?,
) : RegisterResponseJson()
/**
* A different register error with a message.
*/
@Serializable
data class Error(
@SerialName("Message")
val message: String?,
) : RegisterResponseJson()
}

View file

@ -18,16 +18,24 @@ class AccountsServiceImpl constructor(
override suspend fun preLogin(email: String): Result<PreLoginResponseJson> =
accountsApi.preLogin(PreLoginRequestJson(email = email))
// TODO add error parsing and pass along error message for validations BIT-763
@Suppress("MagicNumber")
override suspend fun register(body: RegisterRequestJson): Result<RegisterResponseJson> =
accountsApi
.register(body)
.recoverCatching { throwable ->
throwable
.toBitwardenError()
.parseErrorBodyOrNull<RegisterResponseJson.CaptchaRequired>(
code = HttpURLConnection.HTTP_BAD_REQUEST,
json = json,
) ?: throw throwable
val bitwardenError = throwable.toBitwardenError()
bitwardenError.parseErrorBodyOrNull<RegisterResponseJson.CaptchaRequired>(
code = HttpURLConnection.HTTP_BAD_REQUEST,
json = json,
) ?: bitwardenError.parseErrorBodyOrNull<RegisterResponseJson.Invalid>(
codes = listOf(
HttpURLConnection.HTTP_BAD_REQUEST,
429,
),
json = json,
) ?: bitwardenError.parseErrorBodyOrNull<RegisterResponseJson.Error>(
code = 429,
json = json,
) ?: throw throwable
}
}

View file

@ -146,7 +146,7 @@ class AuthRepositoryImpl @Inject constructor(
}
}
@Suppress("ReturnCount")
@Suppress("ReturnCount", "LongMethod")
override suspend fun register(
email: String,
masterPassword: String,
@ -199,6 +199,21 @@ class AuthRepositoryImpl @Inject constructor(
is RegisterResponseJson.Success -> {
RegisterResult.Success(captchaToken = it.captchaBypassToken)
}
is RegisterResponseJson.Invalid -> {
RegisterResult.Error(
errorMessage = it
.validationErrors
?.values
?.firstOrNull()
?.firstOrNull()
?: it.message,
)
}
is RegisterResponseJson.Error -> {
RegisterResult.Error(it.message)
}
}
},
onFailure = {

View file

@ -12,13 +12,13 @@ import retrofit2.HttpException
* If the receiver is not an [HttpException] or the error body cannot be parsed, null will be
* returned.
*
* @param code HTTP code associated with the error. Only responses with this code will be attempted
* to be parsed.
* @param codes a list of HTTP codes associated with the error. Only responses with a matching code
* will be attempted to be parsed.
* @param json [Json] serializer to use.
*/
inline fun <reified T> BitwardenError.parseErrorBodyOrNull(code: Int, json: Json): T? =
inline fun <reified T> BitwardenError.parseErrorBodyOrNull(codes: List<Int>, json: Json): T? =
(this as? BitwardenError.Http)
?.takeIf { it.code == code }
?.takeIf { codes.any { it == this.code } }
?.responseBodyString
?.let { responseBody ->
try {
@ -27,3 +27,9 @@ inline fun <reified T> BitwardenError.parseErrorBodyOrNull(code: Int, json: Json
null
}
}
/**
* Helper for calling [parseErrorBodyOrNull] with a single code.
*/
inline fun <reified T> BitwardenError.parseErrorBodyOrNull(code: Int, json: Json): T? =
parseErrorBodyOrNull(listOf(code), json)

View file

@ -137,7 +137,6 @@ class CreateAccountViewModel @Inject constructor(
}
is RegisterResult.Error -> {
// TODO parse and display server errors BIT-910
mutableStateFlow.update {
it.copy(
dialog = CreateAccountDialog.Error(

View file

@ -10,7 +10,6 @@ import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import okhttp3.mockwebserver.MockResponse
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import retrofit2.create
@ -19,7 +18,9 @@ class AccountsServiceTest : BaseServiceTest() {
private val accountsApi: AccountsApi = retrofit.create()
private val service = AccountsServiceImpl(
accountsApi = accountsApi,
json = Json,
json = Json {
ignoreUnknownKeys = true
},
)
@Test
@ -117,7 +118,7 @@ class AccountsServiceTest : BaseServiceTest() {
}
@Test
fun `register failure json should be failure`() = runTest {
fun `register failure with Invalid json should be Invalid`() = runTest {
val json = """
{
"message": "The model state is invalid.",
@ -134,7 +135,32 @@ class AccountsServiceTest : BaseServiceTest() {
"""
val response = MockResponse().setResponseCode(400).setBody(json)
server.enqueue(response)
assertTrue(service.register(registerRequestBody).isFailure)
val result = service.register(registerRequestBody)
assertEquals(
RegisterResponseJson.Invalid(
message = "The model state is invalid.",
validationErrors = mapOf("" to listOf("Email '' is already taken.")),
),
result.getOrThrow(),
)
}
@Test
fun `register failure with Error json should return Error`() = runTest {
val json = """
{
"Object": "error",
"Message": "Slow down! Too many requests. Try again soon."
}
""".trimIndent()
val response = MockResponse().setResponseCode(429).setBody(json)
server.enqueue(response)
val result = service.register(registerRequestBody)
assertEquals(
RegisterResponseJson.Error(
message = "Slow down! Too many requests. Try again soon.",
),
result.getOrThrow(),
)
}
@Test

View file

@ -459,6 +459,108 @@ class AuthRepositoryTest {
assertEquals(RegisterResult.Error(errorMessage = null), result)
}
@Test
fun `register returns Invalid should return Error with invalid message`() = runTest {
coEvery { accountsService.preLogin(EMAIL) } returns Result.success(PRE_LOGIN_SUCCESS)
coEvery {
accountsService.register(
body = RegisterRequestJson(
email = EMAIL,
masterPasswordHash = PASSWORD_HASH,
masterPasswordHint = null,
captchaResponse = null,
key = ENCRYPTED_USER_KEY,
keys = RegisterRequestJson.Keys(
publicKey = PUBLIC_KEY,
encryptedPrivateKey = PRIVATE_KEY,
),
kdfType = PBKDF2_SHA256,
kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(),
),
)
} returns Result.success(RegisterResponseJson.Invalid("message", mapOf()))
val result = repository.register(
email = EMAIL,
masterPassword = PASSWORD,
masterPasswordHint = null,
captchaToken = null,
shouldCheckDataBreaches = false,
)
assertEquals(RegisterResult.Error(errorMessage = "message"), result)
}
@Test
fun `register returns Invalid should return Error with first message in map`() = runTest {
coEvery { accountsService.preLogin(EMAIL) } returns Result.success(PRE_LOGIN_SUCCESS)
coEvery {
accountsService.register(
body = RegisterRequestJson(
email = EMAIL,
masterPasswordHash = PASSWORD_HASH,
masterPasswordHint = null,
captchaResponse = null,
key = ENCRYPTED_USER_KEY,
keys = RegisterRequestJson.Keys(
publicKey = PUBLIC_KEY,
encryptedPrivateKey = PRIVATE_KEY,
),
kdfType = PBKDF2_SHA256,
kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(),
),
)
} returns Result.success(
RegisterResponseJson.Invalid(
message = "message",
validationErrors = mapOf("" to listOf("expected")),
),
)
val result = repository.register(
email = EMAIL,
masterPassword = PASSWORD,
masterPasswordHint = null,
captchaToken = null,
shouldCheckDataBreaches = false,
)
assertEquals(RegisterResult.Error(errorMessage = "expected"), result)
}
@Test
fun `register returns Error body should return Error with message`() = runTest {
coEvery { accountsService.preLogin(EMAIL) } returns Result.success(PRE_LOGIN_SUCCESS)
coEvery {
accountsService.register(
body = RegisterRequestJson(
email = EMAIL,
masterPasswordHash = PASSWORD_HASH,
masterPasswordHint = null,
captchaResponse = null,
key = ENCRYPTED_USER_KEY,
keys = RegisterRequestJson.Keys(
publicKey = PUBLIC_KEY,
encryptedPrivateKey = PRIVATE_KEY,
),
kdfType = PBKDF2_SHA256,
kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(),
),
)
} returns Result.success(
RegisterResponseJson.Error(
message = "message",
),
)
val result = repository.register(
email = EMAIL,
masterPassword = PASSWORD,
masterPasswordHint = null,
captchaToken = null,
shouldCheckDataBreaches = false,
)
assertEquals(RegisterResult.Error(errorMessage = "message"), result)
}
@Test
fun `setCaptchaCallbackToken should change the value of captchaTokenFlow`() = runTest {
repository.captchaTokenResultFlow.test {