From dafa1312770f501c8cad9594ba505aba6c556ac6 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen Date: Tue, 5 Nov 2024 15:18:28 -0500 Subject: [PATCH] Add interceptor to convert network response JSON keys to camel case Adds an OkHttp interceptor, `ResponseJsonKeyNameInterceptor`, which converts all JSON object keys in network responses from pascal case to camel case. This addresses inconsistencies in key casing from the server. Includes unit tests for the interceptor to ensure it handles various scenarios, such as different casing styles, empty or null responses, invalid JSON, and non-JSON content types. --- .../network/model/GetTokenResponseJson.kt | 64 +++------ .../KeyConnectorUserDecryptionOptionsJson.kt | 2 +- .../model/MasterPasswordPolicyOptionsJson.kt | 14 +- .../network/model/RefreshTokenResponseJson.kt | 8 +- .../network/model/RegisterResponseJson.kt | 14 +- .../SendVerificationEmailResponseJson.kt | 12 +- .../TrustedDeviceUserDecryptionOptionsJson.kt | 10 +- .../model/UserDecryptionOptionsJson.kt | 6 +- .../auth/repository/AuthRepositoryImpl.kt | 4 +- .../network/di/PlatformNetworkModule.kt | 9 ++ .../ResponseJsonKeyTransformerInterceptor.kt | 37 +++++ .../network/retrofit/RetrofitsImpl.kt | 3 + .../data/platform/util/JsonExtensions.kt | 52 +++++++ .../datasource/disk/AuthDiskSourceTest.kt | 18 +-- .../network/service/IdentityServiceTest.kt | 128 +++++++----------- .../auth/repository/AuthRepositoryTest.kt | 3 - .../interceptor/AuthTokenInterceptorTest.kt | 2 +- .../ResponseJsonKeyNameInterceptorTest.kt | 99 ++++++++++++++ .../network/retrofit/RetrofitsTest.kt | 23 ++-- .../data/platform/util/JsonExtensionsTest.kt | 30 ++++ .../feature/rootnav/RootNavViewModelTest.kt | 2 +- docs/ARCHITECTURE.md | 4 + 22 files changed, 357 insertions(+), 187 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyTransformerInterceptor.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyNameInterceptorTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt index ee25582af..50467d6b9 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/GetTokenResponseJson.kt @@ -32,52 +32,52 @@ sealed class GetTokenResponseJson { */ @Serializable data class Success( - @SerialName("access_token") + @SerialName("accessToken") val accessToken: String, - @SerialName("refresh_token") + @SerialName("refreshToken") val refreshToken: String, - @SerialName("token_type") + @SerialName("tokenType") val tokenType: String, - @SerialName("expires_in") + @SerialName("expiresIn") val expiresInSeconds: Int, - @SerialName("Key") + @SerialName("key") val key: String?, - @SerialName("PrivateKey") + @SerialName("privateKey") val privateKey: String?, - @SerialName("Kdf") + @SerialName("kdf") val kdfType: KdfTypeJson, - @SerialName("KdfIterations") + @SerialName("kdfIterations") val kdfIterations: Int?, - @SerialName("KdfMemory") + @SerialName("kdfMemory") val kdfMemory: Int?, - @SerialName("KdfParallelism") + @SerialName("kdfParallelism") val kdfParallelism: Int?, - @SerialName("ForcePasswordReset") + @SerialName("forcePasswordReset") val shouldForcePasswordReset: Boolean, - @SerialName("ResetMasterPassword") + @SerialName("resetMasterPassword") val shouldResetMasterPassword: Boolean, - @SerialName("TwoFactorToken") + @SerialName("twoFactorToken") val twoFactorToken: String?, - @SerialName("MasterPasswordPolicy") + @SerialName("masterPasswordPolicy") val masterPasswordPolicyOptions: MasterPasswordPolicyOptionsJson?, - @SerialName("UserDecryptionOptions") + @SerialName("userDecryptionOptions") val userDecryptionOptions: UserDecryptionOptionsJson?, - @SerialName("KeyConnectorUrl") + @SerialName("keyConnectorUrl") val keyConnectorUrl: String?, ) : GetTokenResponseJson() @@ -86,7 +86,7 @@ sealed class GetTokenResponseJson { */ @Serializable data class CaptchaRequired( - @SerialName("HCaptcha_SiteKey") + @SerialName("hCaptchaSiteKey") val captchaKey: String, ) : GetTokenResponseJson() @@ -95,35 +95,15 @@ sealed class GetTokenResponseJson { */ @Serializable data class Invalid( - @SerialName("ErrorModel") - val errorModel: ErrorModel?, @SerialName("errorModel") - val legacyErrorModel: LegacyErrorModel?, + val errorModel: ErrorModel?, ) : GetTokenResponseJson() { - /** - * The error message returned from the server, or null. - */ - val errorMessage: String? - get() = errorModel?.errorMessage ?: legacyErrorModel?.errorMessage - /** * The error body of an invalid request containing a message. */ @Serializable data class ErrorModel( - @SerialName("Message") - val errorMessage: String, - ) - - /** - * The legacy error body of an invalid request containing a message. - * - * This model is used to support older versions of the error response model that used - * lower-case keys. - */ - @Serializable - data class LegacyErrorModel( @SerialName("message") val errorMessage: String, ) @@ -145,16 +125,16 @@ sealed class GetTokenResponseJson { */ @Serializable data class TwoFactorRequired( - @SerialName("TwoFactorProviders2") + @SerialName("twoFactorProviders2") val authMethodsData: Map, - @SerialName("TwoFactorProviders") + @SerialName("twoFactorProviders") val twoFactorProviders: List?, - @SerialName("CaptchaBypassToken") + @SerialName("captchaBypassToken") val captchaToken: String?, - @SerialName("SsoEmail2faSessionToken") + @SerialName("ssoEmail2faSessionToken") val ssoToken: String?, ) : GetTokenResponseJson() } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt index 221be523e..abcd8d657 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt @@ -10,6 +10,6 @@ import kotlinx.serialization.Serializable */ @Serializable data class KeyConnectorUserDecryptionOptionsJson( - @SerialName("KeyConnectorUrl") + @SerialName("keyConnectorUrl") val keyConnectorUrl: String, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/MasterPasswordPolicyOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/MasterPasswordPolicyOptionsJson.kt index f8c9fdba5..b2d8cadc2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/MasterPasswordPolicyOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/MasterPasswordPolicyOptionsJson.kt @@ -16,24 +16,24 @@ import kotlinx.serialization.Serializable */ @Serializable data class MasterPasswordPolicyOptionsJson( - @SerialName("MinComplexity") + @SerialName("minComplexity") val minimumComplexity: Int?, - @SerialName("MinLength") + @SerialName("minLength") val minimumLength: Int?, - @SerialName("RequireUpper") + @SerialName("requireUpper") val shouldRequireUppercase: Boolean?, - @SerialName("RequireLower") + @SerialName("requireLower") val shouldRequireLowercase: Boolean?, - @SerialName("RequireNumbers") + @SerialName("requireNumbers") val shouldRequireNumbers: Boolean?, - @SerialName("RequireSpecial") + @SerialName("requireSpecial") val shouldRequireSpecialCharacters: Boolean?, - @SerialName("EnforceOnLogin") + @SerialName("enforceOnLogin") val shouldEnforceOnLogin: Boolean?, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RefreshTokenResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RefreshTokenResponseJson.kt index df96edb20..3d9cada7e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RefreshTokenResponseJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RefreshTokenResponseJson.kt @@ -13,15 +13,15 @@ import kotlinx.serialization.Serializable */ @Serializable data class RefreshTokenResponseJson( - @SerialName("access_token") + @SerialName("accessToken") val accessToken: String, - @SerialName("expires_in") + @SerialName("expiresIn") val expiresIn: Int, - @SerialName("refresh_token") + @SerialName("refreshToken") val refreshToken: String, - @SerialName("token_type") + @SerialName("tokenType") val tokenType: String, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RegisterResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RegisterResponseJson.kt index bff331194..22067f7dd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RegisterResponseJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/RegisterResponseJson.kt @@ -38,7 +38,7 @@ sealed class RegisterResponseJson { */ @Serializable data class ValidationErrors( - @SerialName("HCaptcha_SiteKey") + @SerialName("hCaptchaSiteKey") val captchaKeys: List, ) } @@ -53,17 +53,9 @@ sealed class RegisterResponseJson { @Serializable data class Invalid( @SerialName("message") - private val invalidMessage: String? = null, - - @SerialName("Message") - private val errorMessage: String? = null, + val invalidMessage: String? = null, @SerialName("validationErrors") val validationErrors: Map>?, - ) : RegisterResponseJson() { - /** - * A generic error message. - */ - val message: String? get() = invalidMessage ?: errorMessage - } + ) : RegisterResponseJson() } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/SendVerificationEmailResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/SendVerificationEmailResponseJson.kt index 5d5650993..69ae6c06e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/SendVerificationEmailResponseJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/SendVerificationEmailResponseJson.kt @@ -29,17 +29,9 @@ sealed class SendVerificationEmailResponseJson { @Serializable data class Invalid( @SerialName("message") - private val invalidMessage: String? = null, - - @SerialName("Message") - private val errorMessage: String? = null, + val invalidMessage: String? = null, @SerialName("validationErrors") val validationErrors: Map>?, - ) : SendVerificationEmailResponseJson() { - /** - * A generic error message. - */ - val message: String? get() = invalidMessage ?: errorMessage - } + ) : SendVerificationEmailResponseJson() } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt index 42872d9c7..af0810d28 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt @@ -15,18 +15,18 @@ import kotlinx.serialization.Serializable */ @Serializable data class TrustedDeviceUserDecryptionOptionsJson( - @SerialName("EncryptedPrivateKey") + @SerialName("encryptedPrivateKey") val encryptedPrivateKey: String?, - @SerialName("EncryptedUserKey") + @SerialName("encryptedUserKey") val encryptedUserKey: String?, - @SerialName("HasAdminApproval") + @SerialName("hasAdminApproval") val hasAdminApproval: Boolean, - @SerialName("HasLoginApprovingDevice") + @SerialName("hasLoginApprovingDevice") val hasLoginApprovingDevice: Boolean, - @SerialName("HasManageResetPasswordPermission") + @SerialName("hasManageResetPasswordPermission") val hasManageResetPasswordPermission: Boolean, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt index d4dcb29e7..d302f2f0e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt @@ -14,12 +14,12 @@ import kotlinx.serialization.Serializable */ @Serializable data class UserDecryptionOptionsJson( - @SerialName("HasMasterPassword") + @SerialName("hasMasterPassword") val hasMasterPassword: Boolean, - @SerialName("TrustedDeviceOption") + @SerialName("trustedDeviceOption") val trustedDeviceUserDecryptionOptions: TrustedDeviceUserDecryptionOptionsJson?, - @SerialName("KeyConnectorOption") + @SerialName("keyConnectorOption") val keyConnectorUserDecryptionOptions: KeyConnectorUserDecryptionOptionsJson?, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 19776db93..e7d02ede7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -869,7 +869,7 @@ class AuthRepositoryImpl( ?.values ?.firstOrNull() ?.firstOrNull() - ?: it.message, + ?: it.invalidMessage, ) } } @@ -1524,7 +1524,7 @@ class AuthRepositoryImpl( ) is GetTokenResponseJson.Invalid -> LoginResult.Error( - errorMessage = loginResponse.errorMessage, + errorMessage = loginResponse.errorModel?.errorMessage, ) } }, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/di/PlatformNetworkModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/di/PlatformNetworkModule.kt index ff4761773..929140ace 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/di/PlatformNetworkModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/di/PlatformNetworkModule.kt @@ -5,6 +5,7 @@ import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.Refres import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthTokenInterceptor import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.HeadersInterceptor +import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.ResponseJsonKeyTransformerInterceptor import com.x8bit.bitwarden.data.platform.datasource.network.retrofit.Retrofits import com.x8bit.bitwarden.data.platform.datasource.network.retrofit.RetrofitsImpl import com.x8bit.bitwarden.data.platform.datasource.network.serializer.ZonedDateTimeSerializer @@ -66,6 +67,12 @@ object PlatformNetworkModule { @Singleton fun providesHeadersInterceptor(): HeadersInterceptor = HeadersInterceptor() + @Provides + @Singleton + fun providesResponseJsonKeyTransformerInterceptor( + json: Json, + ): ResponseJsonKeyTransformerInterceptor = ResponseJsonKeyTransformerInterceptor(json) + @Provides @Singleton fun providesRefreshAuthenticator(): RefreshAuthenticator = RefreshAuthenticator() @@ -76,6 +83,7 @@ object PlatformNetworkModule { authTokenInterceptor: AuthTokenInterceptor, baseUrlInterceptors: BaseUrlInterceptors, headersInterceptor: HeadersInterceptor, + responseJsonKeyTransformerInterceptor: ResponseJsonKeyTransformerInterceptor, refreshAuthenticator: RefreshAuthenticator, json: Json, ): Retrofits = @@ -84,6 +92,7 @@ object PlatformNetworkModule { baseUrlInterceptors = baseUrlInterceptors, headersInterceptor = headersInterceptor, refreshAuthenticator = refreshAuthenticator, + responseJsonKeyTransformerInterceptor = responseJsonKeyTransformerInterceptor, json = json, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyTransformerInterceptor.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyTransformerInterceptor.kt new file mode 100644 index 000000000..fe1cdd529 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyTransformerInterceptor.kt @@ -0,0 +1,37 @@ +package com.x8bit.bitwarden.data.platform.datasource.network.interceptor + +import com.x8bit.bitwarden.data.platform.util.parseToJsonElementOrNull +import com.x8bit.bitwarden.data.platform.util.transformKeysToCamelCase +import kotlinx.serialization.json.Json +import okhttp3.Interceptor +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody + +/** + * An OkHttp interceptor that transforms the JSON response body by converting all JSON object keys + * to camel case. + * + * This interceptor is useful for handling inconsistencies in JSON responses where key casing might + * vary. + */ +class ResponseJsonKeyTransformerInterceptor(private val json: Json) : Interceptor { + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val response = chain.proceed(request) + + val responseBody = response.body + ?: return response + + val transformedJsonResponseBody = json + .parseToJsonElementOrNull(responseBody.string()) + ?.transformKeysToCamelCase() + ?.toString() + ?.toResponseBody(response.body?.contentType()) + ?: return response + + return response.newBuilder() + .body(transformedJsonResponseBody) + .build() + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt index e681f484c..62b23c4bd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthToke import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptor import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.HeadersInterceptor +import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.ResponseJsonKeyTransformerInterceptor import com.x8bit.bitwarden.data.platform.datasource.network.util.HEADER_KEY_AUTHORIZATION import kotlinx.serialization.json.Json import okhttp3.MediaType.Companion.toMediaType @@ -23,6 +24,7 @@ class RetrofitsImpl( baseUrlInterceptors: BaseUrlInterceptors, headersInterceptor: HeadersInterceptor, refreshAuthenticator: RefreshAuthenticator, + responseJsonKeyTransformerInterceptor: ResponseJsonKeyTransformerInterceptor, json: Json, ) : Retrofits { //region Authenticated Retrofits @@ -86,6 +88,7 @@ class RetrofitsImpl( private val baseOkHttpClient: OkHttpClient = OkHttpClient.Builder() .addInterceptor(headersInterceptor) + .addInterceptor(responseJsonKeyTransformerInterceptor) .build() private val authenticatedOkHttpClient: OkHttpClient by lazy { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/JsonExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/JsonExtensions.kt index b481db793..a6c80ce8c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/JsonExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/JsonExtensions.kt @@ -2,6 +2,13 @@ package com.x8bit.bitwarden.data.platform.util import kotlinx.serialization.SerializationException import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlin.collections.component1 +import kotlin.collections.component2 +import kotlin.collections.map /** * Attempts to decode the given JSON [string] into the given type [T]. If there is an error in @@ -17,3 +24,48 @@ inline fun Json.decodeFromStringOrNull( } catch (e: IllegalArgumentException) { null } + +/** + * Safely parses a JSON string to a JsonElement, returning null if the parsing fails. + */ +fun Json.parseToJsonElementOrNull(json: String): JsonElement? = try { + parseToJsonElement(json) +} catch (_: SerializationException) { + null +} + +/** + * Recursively transforms keys of a JsonElement to camel case. + * + * This function handles both JsonObject and JsonArray, converting keys within JsonObjects to camel + * case. + * + * @return A new JsonElement with transformed keys. + */ +fun JsonElement.transformKeysToCamelCase(): JsonElement = + when (this) { + is JsonObject -> buildJsonObject { + this@transformKeysToCamelCase.entries + .forEach { (key: String, value: JsonElement) -> + val transformedKey = if (key.contains("-") || key.contains("_")) { + key + .lowercase() + .split("_", "-") + .mapIndexed { index, originalKey -> + if (index > 0) { + originalKey.replaceFirstChar { it.uppercase() } + } else { + originalKey.lowercase() + } + } + .joinToString("") + } else { + key.replaceFirstChar { it.lowercase() } + } + this@buildJsonObject.put(transformedKey, value.transformKeysToCamelCase()) + } + } + + is JsonArray -> JsonArray(this.map { it.transformKeysToCamelCase() }) + else -> this + } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index f27a32320..d2ac0e35d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -1223,16 +1223,16 @@ private const val USER_STATE_JSON = """ "kdfMemory": 16, "kdfParallelism": 4, "accountDecryptionOptions": { - "HasMasterPassword": true, - "TrustedDeviceOption": { - "EncryptedPrivateKey": "encryptedPrivateKey", - "EncryptedUserKey": "encryptedUserKey", - "HasAdminApproval": true, - "HasLoginApprovingDevice": true, - "HasManageResetPasswordPermission": true + "hasMasterPassword": true, + "trustedDeviceOption": { + "encryptedPrivateKey": "encryptedPrivateKey", + "encryptedUserKey": "encryptedUserKey", + "hasAdminApproval": true, + "hasLoginApprovingDevice": true, + "hasManageResetPasswordPermission": true }, - "KeyConnectorOption": { - "KeyConnectorUrl": "keyConnectorUrl" + "keyConnectorOption": { + "keyConnectorUrl": "keyConnectorUrl" } } }, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt index 90d64d205..c5bd7d0b2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/network/service/IdentityServiceTest.kt @@ -167,7 +167,7 @@ class IdentityServiceTest : BaseServiceTest() { val result = identityService.register(registerRequestBody) assertEquals( RegisterResponseJson.Invalid( - errorMessage = "Slow down! Too many requests. Try again soon.", + invalidMessage = "Slow down! Too many requests. Try again soon.", validationErrors = null, ), result.getOrThrow(), @@ -179,7 +179,7 @@ class IdentityServiceTest : BaseServiceTest() { val json = """ { "validationErrors": { - "HCaptcha_SiteKey": [ + "hCaptchaSiteKey": [ "mock_token" ] } @@ -273,22 +273,6 @@ class IdentityServiceTest : BaseServiceTest() { assertEquals(INVALID_LOGIN.asSuccess(), result) } - @Test - fun `getToken when response is a 400 with a legacy error body should return Invalid`() = - runTest { - server.enqueue(MockResponse().setResponseCode(400).setBody(LEGACY_INVALID_LOGIN_JSON)) - val result = identityService.getToken( - email = EMAIL, - authModel = IdentityTokenAuthModel.MasterPassword( - username = EMAIL, - password = PASSWORD_HASH, - ), - captchaToken = null, - uniqueAppId = UNIQUE_APP_ID, - ) - assertEquals(LEGACY_INVALID_LOGIN.asSuccess(), result) - } - @Test fun `prevalidateSso when response is success should return PrevalidateSsoResponseJson`() = runTest { @@ -358,7 +342,7 @@ class IdentityServiceTest : BaseServiceTest() { val result = identityService.registerFinish(registerFinishRequestBody) assertEquals( RegisterResponseJson.Invalid( - errorMessage = "Slow down! Too many requests. Try again soon.", + invalidMessage = "Slow down! Too many requests. Try again soon.", validationErrors = null, ), result.getOrThrow(), @@ -503,10 +487,10 @@ private val PREVALIDATE_SSO_BODY = PrevalidateSsoResponseJson( private const val REFRESH_TOKEN_JSON = """ { - "access_token": "accessToken", - "expires_in": 3600, - "refresh_token": "refreshToken", - "token_type": "Bearer" + "accessToken": "accessToken", + "expiresIn": 3600, + "refreshToken": "refreshToken", + "tokenType": "Bearer" } """ @@ -519,23 +503,23 @@ private val REFRESH_TOKEN_BODY = RefreshTokenResponseJson( private const val CAPTCHA_BODY_JSON = """ { - "HCaptcha_SiteKey": "123" + "hCaptchaSiteKey": "123" } """ private val CAPTCHA_BODY = GetTokenResponseJson.CaptchaRequired("123") private const val TWO_FACTOR_BODY_JSON = """ { - "TwoFactorProviders2": {"1": {"Email": "ex***@email.com"}, "0": {"Email": null}}, - "SsoEmail2faSessionToken": "exampleToken", - "CaptchaBypassToken": "BWCaptchaBypass_ABCXYZ", - "TwoFactorProviders": ["1", "3", "0"] + "twoFactorProviders2": {"1": {"email": "ex***@email.com"}, "0": {"email": null}}, + "ssoEmail2faSessionToken": "exampleToken", + "captchaBypassToken": "BWCaptchaBypass_ABCXYZ", + "twoFactorProviders": ["1", "3", "0"] } """ private val TWO_FACTOR_BODY = GetTokenResponseJson.TwoFactorRequired( authMethodsData = mapOf( - TwoFactorAuthMethod.EMAIL to JsonObject(mapOf("Email" to JsonPrimitive("ex***@email.com"))), - TwoFactorAuthMethod.AUTHENTICATOR_APP to JsonObject(mapOf("Email" to JsonNull)), + TwoFactorAuthMethod.EMAIL to JsonObject(mapOf("email" to JsonPrimitive("ex***@email.com"))), + TwoFactorAuthMethod.AUTHENTICATOR_APP to JsonObject(mapOf("email" to JsonNull)), ), ssoToken = "exampleToken", captchaToken = "BWCaptchaBypass_ABCXYZ", @@ -544,41 +528,41 @@ private val TWO_FACTOR_BODY = GetTokenResponseJson.TwoFactorRequired( private const val LOGIN_SUCCESS_JSON = """ { - "access_token": "accessToken", - "expires_in": 3600, - "token_type": "Bearer", - "refresh_token": "refreshToken", - "PrivateKey": "privateKey", - "Key": "key", - "MasterPasswordPolicy": { - "MinComplexity": 10, - "MinLength": 100, - "RequireUpper": true, - "RequireLower": true, - "RequireNumbers": true, - "RequireSpecial": true, - "EnforceOnLogin": true + "accessToken": "accessToken", + "expiresIn": 3600, + "tokenType": "Bearer", + "refreshToken": "refreshToken", + "privateKey": "privateKey", + "key": "key", + "masterPasswordPolicy": { + "minComplexity": 10, + "minLength": 100, + "requireUpper": true, + "requireLower": true, + "requireNumbers": true, + "requireSpecial": true, + "enforceOnLogin": true }, - "ForcePasswordReset": true, - "ResetMasterPassword": true, - "Kdf": 1, - "KdfIterations": 600000, - "KdfMemory": 16, - "KdfParallelism": 4, - "UserDecryptionOptions": { - "HasMasterPassword": true, - "TrustedDeviceOption": { - "EncryptedPrivateKey": "encryptedPrivateKey", - "EncryptedUserKey": "encryptedUserKey", - "HasAdminApproval": true, - "HasLoginApprovingDevice": true, - "HasManageResetPasswordPermission": true + "forcePasswordReset": true, + "resetMasterPassword": true, + "kdf": 1, + "kdfIterations": 600000, + "kdfMemory": 16, + "kdfParallelism": 4, + "userDecryptionOptions": { + "hasMasterPassword": true, + "trustedDeviceOption": { + "encryptedPrivateKey": "encryptedPrivateKey", + "encryptedUserKey": "encryptedUserKey", + "hasAdminApproval": true, + "hasLoginApprovingDevice": true, + "hasManageResetPasswordPermission": true }, - "KeyConnectorOption": { - "KeyConnectorUrl": "keyConnectorUrl" + "keyConnectorOption": { + "keyConnectorUrl": "keyConnectorUrl" } }, - "KeyConnectorUrl": "keyConnectorUrl" + "keyConnectorUrl": "keyConnectorUrl" } """ @@ -622,25 +606,17 @@ private val LOGIN_SUCCESS = GetTokenResponseJson.Success( ) private const val INVALID_LOGIN_JSON = """ -{ - "ErrorModel": { - "Message": "123" - } -} -""" - -private const val LEGACY_INVALID_LOGIN_JSON = """ { "errorModel": { - "message": "Legacy-123" + "message": "123" } } """ private const val TOO_MANY_REQUEST_ERROR_JSON = """ { - "Object": "error", - "Message": "Slow down! Too many requests. Try again soon." + "object": "error", + "message": "Slow down! Too many requests. Try again soon." } """ @@ -669,14 +645,6 @@ private val INVALID_LOGIN = GetTokenResponseJson.Invalid( errorModel = GetTokenResponseJson.Invalid.ErrorModel( errorMessage = "123", ), - legacyErrorModel = null, -) - -private val LEGACY_INVALID_LOGIN = GetTokenResponseJson.Invalid( - errorModel = null, - legacyErrorModel = GetTokenResponseJson.Invalid.LegacyErrorModel( - errorMessage = "Legacy-123", - ), ) private val SEND_VERIFICATION_EMAIL_REQUEST = SendVerificationEmailRequestJson( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 2f22d25be..37cd2979c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -1527,7 +1527,6 @@ class AuthRepositoryTest { errorModel = GetTokenResponseJson.Invalid.ErrorModel( errorMessage = "mock_error_message", ), - legacyErrorModel = null, ) .asSuccess() @@ -2320,7 +2319,6 @@ class AuthRepositoryTest { errorModel = GetTokenResponseJson.Invalid.ErrorModel( errorMessage = "mock_error_message", ), - legacyErrorModel = null, ) .asSuccess() @@ -2789,7 +2787,6 @@ class AuthRepositoryTest { errorModel = GetTokenResponseJson.Invalid.ErrorModel( errorMessage = "mock_error_message", ), - legacyErrorModel = null, ) .asSuccess() diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/AuthTokenInterceptorTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/AuthTokenInterceptorTest.kt index a5420b980..2238e3944 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/AuthTokenInterceptorTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/AuthTokenInterceptorTest.kt @@ -50,7 +50,7 @@ class AuthTokenInterceptorTest { } private const val USER_ID: String = "user_id" -private const val ACCESS_TOKEN: String = "access_token" +private const val ACCESS_TOKEN: String = "accessToken" private val USER_STATE: UserStateJson = UserStateJson( activeUserId = USER_ID, accounts = mapOf(USER_ID to mockk()), diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyNameInterceptorTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyNameInterceptorTest.kt new file mode 100644 index 000000000..8172508be --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyNameInterceptorTest.kt @@ -0,0 +1,99 @@ +package com.x8bit.bitwarden.data.platform.datasource.network.interceptor + +import com.x8bit.bitwarden.data.platform.datasource.network.di.PlatformNetworkModule +import com.x8bit.bitwarden.data.platform.util.parseToJsonElementOrNull +import com.x8bit.bitwarden.data.util.assertJsonEquals +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import kotlinx.serialization.json.Json +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class ResponseJsonKeyNameInterceptorTest { + + private val interceptor = ResponseJsonKeyTransformerInterceptor( + json = PlatformNetworkModule.providesJson(), + ) + private val request = Request.Builder() + .url("http://localhost") + .build() + + @AfterEach + fun tearDown() { + unmockkStatic(Json::parseToJsonElementOrNull) + } + + @Test + fun `intercept should return original response when response body is null`() { + val originalResponse = Response.Builder() + .request(request) + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .build() + val response = interceptor.intercept( + chain = FakeInterceptorChain( + request = request, + responseProvider = { originalResponse }, + ), + ) + assertEquals( + originalResponse, + response, + ) + } + + @Test + fun `intercept should return original response when parseToJsonElementOrNull is null`() { + mockkStatic(Json::parseToJsonElementOrNull) + every { Json.parseToJsonElementOrNull(any()) } returns null + + val originalResponse = Response.Builder() + .request(request) + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .body("".toResponseBody()) + .build() + val response = interceptor.intercept( + chain = FakeInterceptorChain( + request = request, + responseProvider = { originalResponse }, + ), + ) + assertEquals( + originalResponse, + response, + ) + } + + @Test + fun `intercept should return transformed response`() { + val response = interceptor.intercept( + chain = FakeInterceptorChain( + request = request, + responseProvider = { + Response.Builder() + .request(it) + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .body( + """[{"PascalArray":[{"PascalCase":0}]}]""".toResponseBody(), + ) + .build() + }, + ), + ) + assertJsonEquals( + """[{"pascalArray":[{"pascalCase":0}]}]""", + response.body!!.string(), + ) + } +} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsTest.kt index 99e02a9ba..bf8598d19 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsTest.kt @@ -4,6 +4,7 @@ import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.Refres import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthTokenInterceptor import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.BaseUrlInterceptors import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.HeadersInterceptor +import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.ResponseJsonKeyTransformerInterceptor import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -39,11 +40,15 @@ class RetrofitsTest { } } private val headersInterceptors = mockk { - mockIntercept { isheadersInterceptorCalled = true } + mockIntercept { isHeadersInterceptorCalled = true } } private val refreshAuthenticator = mockk { mockAuthenticate { isRefreshAuthenticatorCalled = true } } + private val responseJsonKeyTransformerInterceptor = + mockk { + mockIntercept { isResponseJsonKeyTransformerInterceptorCalled = true } + } private val json = Json private val server = MockWebServer() @@ -52,15 +57,17 @@ class RetrofitsTest { baseUrlInterceptors = baseUrlInterceptors, headersInterceptor = headersInterceptors, refreshAuthenticator = refreshAuthenticator, + responseJsonKeyTransformerInterceptor = responseJsonKeyTransformerInterceptor, json = json, ) private var isAuthInterceptorCalled = false private var isApiInterceptorCalled = false - private var isheadersInterceptorCalled = false + private var isHeadersInterceptorCalled = false private var isIdentityInterceptorCalled = false private var isEventsInterceptorCalled = false private var isRefreshAuthenticatorCalled = false + private var isResponseJsonKeyTransformerInterceptorCalled = false @Before fun setUp() { @@ -158,7 +165,7 @@ class RetrofitsTest { assertTrue(isAuthInterceptorCalled) assertTrue(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertFalse(isIdentityInterceptorCalled) assertFalse(isEventsInterceptorCalled) } @@ -176,7 +183,7 @@ class RetrofitsTest { assertTrue(isAuthInterceptorCalled) assertFalse(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertFalse(isIdentityInterceptorCalled) assertTrue(isEventsInterceptorCalled) } @@ -194,7 +201,7 @@ class RetrofitsTest { assertFalse(isAuthInterceptorCalled) assertTrue(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertFalse(isIdentityInterceptorCalled) assertFalse(isEventsInterceptorCalled) } @@ -212,7 +219,7 @@ class RetrofitsTest { assertFalse(isAuthInterceptorCalled) assertFalse(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertTrue(isIdentityInterceptorCalled) assertFalse(isEventsInterceptorCalled) } @@ -231,7 +238,7 @@ class RetrofitsTest { assertTrue(isAuthInterceptorCalled) assertFalse(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertFalse(isIdentityInterceptorCalled) assertFalse(isEventsInterceptorCalled) } @@ -250,7 +257,7 @@ class RetrofitsTest { assertFalse(isAuthInterceptorCalled) assertFalse(isApiInterceptorCalled) - assertTrue(isheadersInterceptorCalled) + assertTrue(isHeadersInterceptorCalled) assertFalse(isIdentityInterceptorCalled) assertFalse(isEventsInterceptorCalled) } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/util/JsonExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/util/JsonExtensionsTest.kt index d8cca88c5..cf7eda814 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/util/JsonExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/util/JsonExtensionsTest.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.platform.util +import com.x8bit.bitwarden.data.util.assertJsonEquals import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json @@ -47,6 +48,35 @@ class JsonExtensionsTest { ), ) } + + @Test + fun `transformPascalKeysToCamelCase should transform keys with '-' or '_' to camelCase`() { + val jsonData = json.parseToJsonElement("""[{"kebab-array":[{"snake_case":0}]}]""") + assertJsonEquals( + """[{"kebabArray":[{"snakeCase":0}]}]""", + jsonData.transformKeysToCamelCase().toString(), + ) + } + + @Suppress("MaxLineLength") + @Test + fun `transformKeysToCamelCase should return transformed response when root object is JSONArray`() { + val jsonData = Json.parseToJsonElement("""[{"PascalArray":[{"PascalCase":0}]}]""") + assertJsonEquals( + """[{"pascalArray":[{"pascalCase":0}]}]""", + jsonData.transformKeysToCamelCase().toString(), + ) + } + + @Test + fun `parseToJsonElementOrNull should return null when json is empty string`() { + assertNull(json.parseToJsonElementOrNull("")) + } + + @Test + fun `parseToJsonElementOrNull should return null when json is invalid`() { + assertNull(json.parseToJsonElementOrNull("{OK}")) + } } @Serializable diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt index fb53b747e..03e676d8b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt @@ -1344,4 +1344,4 @@ private val FIXED_CLOCK: Clock = Clock.fixed( ZoneOffset.UTC, ) -private const val ACCESS_TOKEN: String = "access_token" +private const val ACCESS_TOKEN: String = "accessToken" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 22321beb2..f5b731fee 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -60,6 +60,10 @@ In some cases a source of data may be continuously observed and in these cases a Nearly all classes in the data layer consist of interfaces representing exposed behavior and a corresponding `...Impl` class implementing that interface (ex: [AuthDiskSource](../app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSource.kt) / [AuthDiskSourceImpl](../app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceImpl.kt)). All `...Impl` classes are intended to be manually constructed while their associated interfaces are provided for dependency injection via a [Hilt Module](https://dagger.dev/hilt/modules.html) (ex: [PlatformNetworkModule](../app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/di/PlatformNetworkModule.kt)). This prevents the `...Impl` classes from being injected by accident and allows the interfaces to be easily mocked/faked in tests. +### Note on Bitwarden server communication + +All network responses containing JSON are transformed such that their keys conform to the `camelCase` naming convention. This is done to address inconsistencies in key casing from the server. Transformation is handled by [ResponseJsonKeyNameInterceptor](../app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/interceptor/ResponseJsonKeyTransformerInterceptor.kt). + ## UI Layer The UI layer adheres to the concept of [unidirectional data flow](https://developer.android.com/develop/ui/compose/architecture#udf) and makes use of the MVVM design pattern. Both concepts are in line what Google currently recommends as the best approach for building the UI-layer of a modern Android application and this allows us to make use of all the available tooling Google provides as part of the [Jetpack suite of libraries](https://developer.android.com/jetpack). The MVVM implementation is built around the Android [ViewModel](https://developer.android.com/topic/libraries/architecture/viewmodel) class and the UI itself is constructed using the [Jetpack Compose](https://developer.android.com/develop/ui/compose), a declarative UI framework specifically built around the unidirectional data flow approach.