From 85bc76d0a6ee83e269525ee3d8ab6757e834fca6 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 7 Aug 2025 12:54:22 -0500 Subject: [PATCH] PM-24565: Syncronize token refreshes to avoid duplicate requests (#5662) --- .../network/BitwardenServiceClientImpl.kt | 12 +- .../authenticator/RefreshAuthenticator.kt | 63 ----- .../interceptor/AuthTokenInterceptor.kt | 54 ---- .../network/interceptor/AuthTokenManager.kt | 104 ++++++++ .../network/retrofit/RetrofitsImpl.kt | 12 +- .../RefreshAuthenticatorTests.kt | 120 --------- .../interceptor/AuthTokenInterceptorTest.kt | 139 ---------- .../interceptor/AuthTokenManagerTest.kt | 248 ++++++++++++++++++ .../network/retrofit/RetrofitsTest.kt | 15 +- 9 files changed, 365 insertions(+), 402 deletions(-) delete mode 100644 network/src/main/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticator.kt delete mode 100644 network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptor.kt create mode 100644 network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenManager.kt delete mode 100644 network/src/test/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticatorTests.kt delete mode 100644 network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptorTest.kt create mode 100644 network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenManagerTest.kt diff --git a/network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt b/network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt index 90d4b3cd7b..f039e4f487 100644 --- a/network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt +++ b/network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt @@ -2,8 +2,7 @@ package com.bitwarden.network import com.bitwarden.annotation.OmitFromCoverage import com.bitwarden.core.data.serializer.ZonedDateTimeSerializer -import com.bitwarden.network.authenticator.RefreshAuthenticator -import com.bitwarden.network.interceptor.AuthTokenInterceptor +import com.bitwarden.network.interceptor.AuthTokenManager import com.bitwarden.network.interceptor.BaseUrlInterceptors import com.bitwarden.network.interceptor.HeadersInterceptor import com.bitwarden.network.model.BitwardenServiceClientConfig @@ -52,8 +51,7 @@ internal class BitwardenServiceClientImpl( private val bitwardenServiceClientConfig: BitwardenServiceClientConfig, ) : BitwardenServiceClient { - private val refreshAuthenticator: RefreshAuthenticator = RefreshAuthenticator() - private val authTokenInterceptor: AuthTokenInterceptor = AuthTokenInterceptor( + private val authTokenManager: AuthTokenManager = AuthTokenManager( clock = bitwardenServiceClientConfig.clock, authTokenProvider = bitwardenServiceClientConfig.authTokenProvider, ) @@ -75,7 +73,7 @@ internal class BitwardenServiceClientImpl( } private val retrofits: Retrofits by lazy { RetrofitsImpl( - authTokenInterceptor = authTokenInterceptor, + authTokenManager = authTokenManager, baseUrlInterceptors = BaseUrlInterceptors( baseUrlsProvider = bitwardenServiceClientConfig.baseUrlsProvider, ), @@ -84,7 +82,6 @@ internal class BitwardenServiceClientImpl( clientName = bitwardenServiceClientConfig.clientData.clientName, clientVersion = bitwardenServiceClientConfig.clientData.clientVersion, ), - refreshAuthenticator = refreshAuthenticator, logHttpBody = bitwardenServiceClientConfig.enableHttpBodyLogging, certificateProvider = bitwardenServiceClientConfig.certificateProvider, json = clientJson, @@ -206,7 +203,6 @@ internal class BitwardenServiceClientImpl( } override fun setRefreshTokenProvider(refreshTokenProvider: RefreshTokenProvider?) { - refreshAuthenticator.refreshTokenProvider = refreshTokenProvider - authTokenInterceptor.refreshTokenProvider = refreshTokenProvider + authTokenManager.refreshTokenProvider = refreshTokenProvider } } diff --git a/network/src/main/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticator.kt b/network/src/main/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticator.kt deleted file mode 100644 index b2593aa5e8..0000000000 --- a/network/src/main/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticator.kt +++ /dev/null @@ -1,63 +0,0 @@ -package com.bitwarden.network.authenticator - -import com.bitwarden.network.provider.RefreshTokenProvider -import com.bitwarden.network.util.HEADER_KEY_AUTHORIZATION -import com.bitwarden.network.util.HEADER_VALUE_BEARER_PREFIX -import com.bitwarden.network.util.parseJwtTokenDataOrNull -import okhttp3.Authenticator -import okhttp3.Request -import okhttp3.Response -import okhttp3.Route -import timber.log.Timber - -/** - * An authenticator used to refresh the access token when a 401 is returned from an API. Upon - * successfully getting a new access token, the original request is retried. - */ -internal class RefreshAuthenticator : Authenticator { - - var refreshTokenProvider: RefreshTokenProvider? = null - - override fun authenticate( - route: Route?, - response: Response, - ): Request? { - if (response.shouldSkipAuthentication()) { - // If the same request keeps failing, let's just let the 401 pass through. - return null - } - val accessToken = requireNotNull( - response - .request - .header(name = HEADER_KEY_AUTHORIZATION) - ?.substringAfter(delimiter = HEADER_VALUE_BEARER_PREFIX), - ) - return when (val userId = parseJwtTokenDataOrNull(accessToken)?.userId) { - null -> { - // We are unable to get the user ID, let's just let the 401 pass through. - null - } - - else -> { - Timber.d("Attempting to refresh token due to unauthorized") - refreshTokenProvider - ?.refreshAccessTokenSynchronously(userId = userId) - ?.fold( - onFailure = { null }, - onSuccess = { newAccessToken -> - response - .request - .newBuilder() - .header( - name = HEADER_KEY_AUTHORIZATION, - value = "$HEADER_VALUE_BEARER_PREFIX$newAccessToken", - ) - .build() - }, - ) - } - } - } - - private fun Response.shouldSkipAuthentication(): Boolean = this.priorResponse != null -} diff --git a/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptor.kt b/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptor.kt deleted file mode 100644 index 7a07a1d035..0000000000 --- a/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptor.kt +++ /dev/null @@ -1,54 +0,0 @@ -package com.bitwarden.network.interceptor - -import com.bitwarden.network.provider.RefreshTokenProvider -import com.bitwarden.network.util.HEADER_KEY_AUTHORIZATION -import com.bitwarden.network.util.HEADER_VALUE_BEARER_PREFIX -import okhttp3.Interceptor -import okhttp3.Response -import timber.log.Timber -import java.io.IOException -import java.time.Clock -import java.time.Instant -import java.time.temporal.ChronoUnit - -private const val MISSING_TOKEN_MESSAGE: String = "Auth token is missing!" -private const val MISSING_PROVIDER_MESSAGE: String = "Refresh token provider is missing!" -private const val EXPIRATION_OFFSET_MINUTES: Long = 5L - -/** - * Interceptor responsible for adding the auth token(Bearer) to API requests. - */ -internal class AuthTokenInterceptor( - private val clock: Clock, - private val authTokenProvider: AuthTokenProvider, -) : Interceptor { - var refreshTokenProvider: RefreshTokenProvider? = null - - override fun intercept(chain: Interceptor.Chain): Response { - val tokenData = authTokenProvider - .getAuthTokenDataOrNull() - ?: throw IOException(IllegalStateException(MISSING_TOKEN_MESSAGE)) - val expirationTime = Instant - .ofEpochSecond(tokenData.expiresAtSec) - .minus(EXPIRATION_OFFSET_MINUTES, ChronoUnit.MINUTES) - val token = if (clock.instant().isAfter(expirationTime)) { - Timber.d("Attempting to refresh token due to expiration") - refreshTokenProvider - ?.refreshAccessTokenSynchronously(userId = tokenData.userId) - ?.getOrElse { throw IOException(it) } - ?: throw IOException(IllegalStateException(MISSING_PROVIDER_MESSAGE)) - } else { - tokenData.accessToken - } - val request = chain - .request() - .newBuilder() - .addHeader( - name = HEADER_KEY_AUTHORIZATION, - value = "${HEADER_VALUE_BEARER_PREFIX}$token", - ) - .build() - return chain - .proceed(request) - } -} diff --git a/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenManager.kt b/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenManager.kt new file mode 100644 index 0000000000..31c337ded9 --- /dev/null +++ b/network/src/main/kotlin/com/bitwarden/network/interceptor/AuthTokenManager.kt @@ -0,0 +1,104 @@ +package com.bitwarden.network.interceptor + +import com.bitwarden.network.provider.RefreshTokenProvider +import com.bitwarden.network.util.HEADER_KEY_AUTHORIZATION +import com.bitwarden.network.util.HEADER_VALUE_BEARER_PREFIX +import com.bitwarden.network.util.parseJwtTokenDataOrNull +import okhttp3.Authenticator +import okhttp3.Interceptor +import okhttp3.Request +import okhttp3.Response +import okhttp3.Route +import timber.log.Timber +import java.io.IOException +import java.time.Clock +import java.time.Instant +import java.time.temporal.ChronoUnit + +private const val MISSING_TOKEN_MESSAGE: String = "Auth token is missing!" +private const val MISSING_PROVIDER_MESSAGE: String = "Refresh token provider is missing!" +private const val EXPIRATION_OFFSET_MINUTES: Long = 5L + +/** + * Interceptor responsible for adding the auth token(Bearer) to API requests. + */ +internal class AuthTokenManager( + private val clock: Clock, + private val authTokenProvider: AuthTokenProvider, +) : Authenticator, Interceptor { + var refreshTokenProvider: RefreshTokenProvider? = null + + override fun authenticate( + route: Route?, + response: Response, + ): Request? { + synchronized(this) { + if (response.shouldSkipAuthentication()) { + // If the same request keeps failing, let's just let the 401 pass through. + return null + } + val accessToken = requireNotNull( + response + .request + .header(name = HEADER_KEY_AUTHORIZATION) + ?.substringAfter(delimiter = HEADER_VALUE_BEARER_PREFIX), + ) + return when (val userId = parseJwtTokenDataOrNull(accessToken)?.userId) { + null -> { + // We are unable to get the user ID, let's just let the 401 pass through. + null + } + + else -> { + Timber.d("Attempting to refresh token due to unauthorized") + refreshTokenProvider + ?.refreshAccessTokenSynchronously(userId = userId) + ?.fold( + onFailure = { null }, + onSuccess = { newAccessToken -> + response + .request + .newBuilder() + .header( + name = HEADER_KEY_AUTHORIZATION, + value = "$HEADER_VALUE_BEARER_PREFIX$newAccessToken", + ) + .build() + }, + ) + } + } + } + } + + override fun intercept(chain: Interceptor.Chain): Response { + val token = synchronized(this) { + val tokenData = authTokenProvider + .getAuthTokenDataOrNull() + ?: throw IOException(IllegalStateException(MISSING_TOKEN_MESSAGE)) + val expirationTime = Instant + .ofEpochSecond(tokenData.expiresAtSec) + .minus(EXPIRATION_OFFSET_MINUTES, ChronoUnit.MINUTES) + if (clock.instant().isAfter(expirationTime)) { + Timber.d("Attempting to refresh token due to expiration") + refreshTokenProvider + ?.refreshAccessTokenSynchronously(userId = tokenData.userId) + ?.getOrElse { throw IOException(it) } + ?: throw IOException(IllegalStateException(MISSING_PROVIDER_MESSAGE)) + } else { + tokenData.accessToken + } + } + val request = chain + .request() + .newBuilder() + .addHeader( + name = HEADER_KEY_AUTHORIZATION, + value = "${HEADER_VALUE_BEARER_PREFIX}$token", + ) + .build() + return chain.proceed(request) + } + + private fun Response.shouldSkipAuthentication(): Boolean = this.priorResponse != null +} diff --git a/network/src/main/kotlin/com/bitwarden/network/retrofit/RetrofitsImpl.kt b/network/src/main/kotlin/com/bitwarden/network/retrofit/RetrofitsImpl.kt index 4d38a4fd86..8d0a6efe8e 100644 --- a/network/src/main/kotlin/com/bitwarden/network/retrofit/RetrofitsImpl.kt +++ b/network/src/main/kotlin/com/bitwarden/network/retrofit/RetrofitsImpl.kt @@ -1,8 +1,7 @@ package com.bitwarden.network.retrofit -import com.bitwarden.network.authenticator.RefreshAuthenticator import com.bitwarden.network.core.NetworkResultCallAdapterFactory -import com.bitwarden.network.interceptor.AuthTokenInterceptor +import com.bitwarden.network.interceptor.AuthTokenManager import com.bitwarden.network.interceptor.BaseUrlInterceptor import com.bitwarden.network.interceptor.BaseUrlInterceptors import com.bitwarden.network.interceptor.HeadersInterceptor @@ -27,10 +26,9 @@ import javax.net.ssl.X509TrustManager */ @Suppress("LongParameterList") internal class RetrofitsImpl( - authTokenInterceptor: AuthTokenInterceptor, + authTokenManager: AuthTokenManager, baseUrlInterceptors: BaseUrlInterceptors, headersInterceptor: HeadersInterceptor, - refreshAuthenticator: RefreshAuthenticator?, json: Json, private val certificateProvider: CertificateProvider, private val logHttpBody: Boolean = false, @@ -105,10 +103,8 @@ internal class RetrofitsImpl( private val authenticatedOkHttpClient: OkHttpClient by lazy { baseOkHttpClient .newBuilder() - .addInterceptor(authTokenInterceptor) - .also { builder -> - refreshAuthenticator?.let { builder.authenticator(it) } - } + .addInterceptor(authTokenManager) + .authenticator(authTokenManager) .build() } diff --git a/network/src/test/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticatorTests.kt b/network/src/test/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticatorTests.kt deleted file mode 100644 index 566ee4d932..0000000000 --- a/network/src/test/kotlin/com/bitwarden/network/authenticator/RefreshAuthenticatorTests.kt +++ /dev/null @@ -1,120 +0,0 @@ -package com.bitwarden.network.authenticator - -import com.bitwarden.core.data.util.asFailure -import com.bitwarden.core.data.util.asSuccess -import com.bitwarden.network.model.JwtTokenDataJson -import com.bitwarden.network.provider.RefreshTokenProvider -import com.bitwarden.network.util.parseJwtTokenDataOrNull -import io.mockk.every -import io.mockk.mockk -import io.mockk.mockkStatic -import io.mockk.unmockkStatic -import io.mockk.verify -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import org.junit.jupiter.api.AfterEach -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertNull -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test - -class RefreshAuthenticatorTests { - private lateinit var authenticator: RefreshAuthenticator - private val refreshTokenProvider: RefreshTokenProvider = mockk() - - @BeforeEach - fun setup() { - authenticator = RefreshAuthenticator() - authenticator.refreshTokenProvider = refreshTokenProvider - - mockkStatic(::parseJwtTokenDataOrNull) - } - - @AfterEach - fun tearDown() { - unmockkStatic(::parseJwtTokenDataOrNull) - } - - @Test - fun `RefreshAuthenticator returns null if API has no authorization user ID`() { - every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns null - - assertNull(authenticator.authenticate(null, RESPONSE_401)) - - verify(exactly = 0) { - refreshTokenProvider.refreshAccessTokenSynchronously(any()) - } - } - - @Test - fun `RefreshAuthenticator returns null when refresh is failure`() { - every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN - every { - refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) - } returns Throwable("Fail").asFailure() - - assertNull(authenticator.authenticate(null, RESPONSE_401)) - - verify(exactly = 1) { - refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) - } - } - - @Test - fun `RefreshAuthenticator returns null when refreshTokenProvider is null`() { - authenticator.refreshTokenProvider = null - every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN - assertNull(authenticator.authenticate(null, RESPONSE_401)) - verify(exactly = 0) { - refreshTokenProvider.refreshAccessTokenSynchronously(any()) - } - } - - @Test - fun `RefreshAuthenticator returns updated request when refresh is success`() { - val newAccessToken = "newAccessToken" - every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN - every { - refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) - } returns newAccessToken.asSuccess() - - val authenticatedRequest = authenticator.authenticate(null, RESPONSE_401) - - // The okhttp3 Request is not a data class and does not implement equals - // so we are manually checking that the correct header is added. - assertEquals( - "Bearer $newAccessToken", - authenticatedRequest!!.header("Authorization"), - ) - verify(exactly = 1) { - refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) - } - } -} - -private const val USER_ID = "2a135b23-e1fb-42c9-bec3-573857bc8181" - -private val JTW_TOKEN = JwtTokenDataJson( - userId = USER_ID, - email = "test@bitwarden.com", - isEmailVerified = true, - name = "Bitwarden Tester", - expirationAsEpochTime = 1697495714, - hasPremium = false, - authenticationMethodsReference = listOf("Application"), -) - -private const val JWT_ACCESS_TOKEN = "jwt" - -private val RESPONSE_401 = Response.Builder() - .code(401) - .request( - request = Request.Builder() - .header(name = "Authorization", value = "Bearer $JWT_ACCESS_TOKEN") - .url("https://www.bitwarden.com") - .build(), - ) - .protocol(Protocol.HTTP_2) - .message("Unauthenticated") - .build() diff --git a/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptorTest.kt b/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptorTest.kt deleted file mode 100644 index 6ac846f709..0000000000 --- a/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenInterceptorTest.kt +++ /dev/null @@ -1,139 +0,0 @@ -package com.bitwarden.network.interceptor - -import com.bitwarden.core.data.util.asFailure -import com.bitwarden.core.data.util.asSuccess -import com.bitwarden.network.model.AuthTokenData -import com.bitwarden.network.provider.RefreshTokenProvider -import io.mockk.every -import io.mockk.mockk -import junit.framework.TestCase.assertEquals -import okhttp3.Request -import org.junit.Assert.assertThrows -import org.junit.Test -import java.io.IOException -import java.time.Clock -import java.time.Instant -import java.time.ZoneOffset - -class AuthTokenInterceptorTest { - private val mockAuthTokenProvider = mockk { - every { getAuthTokenDataOrNull() } returns null - } - private val interceptor: AuthTokenInterceptor = AuthTokenInterceptor( - clock = FIXED_CLOCK, - authTokenProvider = mockAuthTokenProvider, - ) - private val request: Request = Request - .Builder() - .url("http://localhost") - .build() - - @Test - fun `intercept should add the auth token when set`() { - val authTokenData = AuthTokenData( - userId = USER_ID, - accessToken = ACCESS_TOKEN, - expiresAtSec = FIXED_CLOCK.instant().epochSecond + 3600L, - ) - every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData - - val response = interceptor.intercept( - chain = FakeInterceptorChain(request = request), - ) - assertEquals( - "Bearer $ACCESS_TOKEN", - response.request.header("Authorization"), - ) - } - - @Suppress("MaxLineLength") - @Test - fun `intercept should throw an exception when auth token is expired and refreshTokenProvider is missing`() { - val authTokenData = AuthTokenData( - userId = USER_ID, - accessToken = ACCESS_TOKEN, - expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, - ) - every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData - - val throwable = assertThrows(IOException::class.java) { - interceptor.intercept( - chain = FakeInterceptorChain(request = request), - ) - } - assertEquals( - "Refresh token provider is missing!", - throwable.cause?.message, - ) - } - - @Suppress("MaxLineLength") - @Test - fun `intercept should throw an exception when auth token is expired and refreshAccessTokenSynchronously returns an error`() { - val errorMessage = "Fail!" - interceptor.refreshTokenProvider = object : RefreshTokenProvider { - override fun refreshAccessTokenSynchronously( - userId: String, - ): Result = Throwable(errorMessage).asFailure() - } - val authTokenData = AuthTokenData( - userId = USER_ID, - accessToken = ACCESS_TOKEN, - expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, - ) - every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData - - val throwable = assertThrows(IOException::class.java) { - interceptor.intercept( - chain = FakeInterceptorChain(request = request), - ) - } - assertEquals(errorMessage, throwable.cause?.message) - } - - @Suppress("MaxLineLength") - @Test - fun `intercept should add the auth token when auth token is expired and refreshAccessTokenSynchronously returns new token`() { - val token = "token" - interceptor.refreshTokenProvider = object : RefreshTokenProvider { - override fun refreshAccessTokenSynchronously( - userId: String, - ): Result = token.asSuccess() - } - val authTokenData = AuthTokenData( - userId = USER_ID, - accessToken = ACCESS_TOKEN, - expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, - ) - every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData - - val response = interceptor.intercept( - chain = FakeInterceptorChain(request = request), - ) - assertEquals( - "Bearer $token", - response.request.header("Authorization"), - ) - } - - @Test - fun `intercept should throw an exception when an auth token data is missing`() { - val throwable = assertThrows(IOException::class.java) { - interceptor.intercept( - chain = FakeInterceptorChain(request = request), - ) - } - assertEquals( - "Auth token is missing!", - throwable.cause?.message, - ) - } -} - -private val FIXED_CLOCK: Clock = Clock.fixed( - Instant.parse("2023-10-27T12:00:00Z"), - ZoneOffset.UTC, -) - -private const val USER_ID: String = "user_id" -private const val ACCESS_TOKEN: String = "access_token" diff --git a/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenManagerTest.kt b/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenManagerTest.kt new file mode 100644 index 0000000000..2a621ac140 --- /dev/null +++ b/network/src/test/kotlin/com/bitwarden/network/interceptor/AuthTokenManagerTest.kt @@ -0,0 +1,248 @@ +package com.bitwarden.network.interceptor + +import com.bitwarden.core.data.util.asFailure +import com.bitwarden.core.data.util.asSuccess +import com.bitwarden.network.model.AuthTokenData +import com.bitwarden.network.model.JwtTokenDataJson +import com.bitwarden.network.provider.RefreshTokenProvider +import com.bitwarden.network.util.parseJwtTokenDataOrNull +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import io.mockk.verify +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.io.IOException +import java.time.Clock +import java.time.Instant +import java.time.ZoneOffset + +class AuthTokenManagerTest { + private val mockAuthTokenProvider = mockk { + every { getAuthTokenDataOrNull() } returns null + } + private val refreshTokenProvider: RefreshTokenProvider = mockk() + private val authTokenManager: AuthTokenManager = AuthTokenManager( + clock = FIXED_CLOCK, + authTokenProvider = mockAuthTokenProvider, + ) + private val request: Request = Request + .Builder() + .url("http://localhost") + .build() + + @BeforeEach + fun setup() { + authTokenManager.refreshTokenProvider = refreshTokenProvider + mockkStatic(::parseJwtTokenDataOrNull) + } + + @AfterEach + fun tearDown() { + unmockkStatic(::parseJwtTokenDataOrNull) + } + + @Nested + inner class Authenticator { + @Test + fun `returns null if API has no authorization user ID`() { + every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns null + + assertNull(authTokenManager.authenticate(null, RESPONSE_401)) + + verify(exactly = 0) { + refreshTokenProvider.refreshAccessTokenSynchronously(any()) + } + } + + @Test + fun `returns null when refresh is failure`() { + every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN + every { + refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) + } returns Throwable("Fail").asFailure() + + assertNull(authTokenManager.authenticate(null, RESPONSE_401)) + + verify(exactly = 1) { + refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) + } + } + + @Test + fun `returns null when refreshTokenProvider is null`() { + authTokenManager.refreshTokenProvider = null + every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN + assertNull(authTokenManager.authenticate(null, RESPONSE_401)) + verify(exactly = 0) { + refreshTokenProvider.refreshAccessTokenSynchronously(any()) + } + } + + @Test + fun `returns updated request when refresh is success`() { + val newAccessToken = "newAccessToken" + every { parseJwtTokenDataOrNull(JWT_ACCESS_TOKEN) } returns JTW_TOKEN + every { + refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) + } returns newAccessToken.asSuccess() + + val authenticatedRequest = authTokenManager.authenticate(null, RESPONSE_401) + + // The okhttp3 Request is not a data class and does not implement equals + // so we are manually checking that the correct header is added. + assertEquals( + "Bearer $newAccessToken", + authenticatedRequest!!.header("Authorization"), + ) + verify(exactly = 1) { + refreshTokenProvider.refreshAccessTokenSynchronously(USER_ID) + } + } + } + + @Nested + inner class Interceptor { + @Test + fun `intercept should add the auth token when set`() { + val authTokenData = AuthTokenData( + userId = USER_ID, + accessToken = ACCESS_TOKEN, + expiresAtSec = FIXED_CLOCK.instant().epochSecond + 3600L, + ) + every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData + + val response = authTokenManager.intercept( + chain = FakeInterceptorChain(request = request), + ) + assertEquals( + "Bearer $ACCESS_TOKEN", + response.request.header("Authorization"), + ) + } + + @Suppress("MaxLineLength") + @Test + fun `intercept should throw an exception when auth token is expired and refreshTokenProvider is missing`() { + val authTokenData = AuthTokenData( + userId = USER_ID, + accessToken = ACCESS_TOKEN, + expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, + ) + authTokenManager.refreshTokenProvider = null + every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData + val throwable = assertThrows(IOException::class.java) { + authTokenManager.intercept( + chain = FakeInterceptorChain(request = request), + ) + } + assertEquals( + "Refresh token provider is missing!", + throwable.cause?.message, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `intercept should throw an exception when auth token is expired and refreshAccessTokenSynchronously returns an error`() { + val errorMessage = "Fail!" + authTokenManager.refreshTokenProvider = object : RefreshTokenProvider { + override fun refreshAccessTokenSynchronously( + userId: String, + ): Result = Throwable(errorMessage).asFailure() + } + val authTokenData = AuthTokenData( + userId = USER_ID, + accessToken = ACCESS_TOKEN, + expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, + ) + every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData + + val throwable = assertThrows(IOException::class.java) { + authTokenManager.intercept( + chain = FakeInterceptorChain(request = request), + ) + } + assertEquals(errorMessage, throwable.cause?.message) + } + + @Suppress("MaxLineLength") + @Test + fun `intercept should add the auth token when auth token is expired and refreshAccessTokenSynchronously returns new token`() { + val token = "token" + authTokenManager.refreshTokenProvider = object : RefreshTokenProvider { + override fun refreshAccessTokenSynchronously( + userId: String, + ): Result = token.asSuccess() + } + val authTokenData = AuthTokenData( + userId = USER_ID, + accessToken = ACCESS_TOKEN, + expiresAtSec = FIXED_CLOCK.instant().epochSecond - 3600L, + ) + every { mockAuthTokenProvider.getAuthTokenDataOrNull() } returns authTokenData + + val response = authTokenManager.intercept( + chain = FakeInterceptorChain(request = request), + ) + assertEquals( + "Bearer $token", + response.request.header("Authorization"), + ) + } + + @Test + fun `intercept should throw an exception when an auth token data is missing`() { + val throwable = assertThrows(IOException::class.java) { + authTokenManager.intercept( + chain = FakeInterceptorChain(request = request), + ) + } + assertEquals( + "Auth token is missing!", + throwable.cause?.message, + ) + } + } +} + +private val FIXED_CLOCK: Clock = Clock.fixed( + Instant.parse("2023-10-27T12:00:00Z"), + ZoneOffset.UTC, +) + +private const val USER_ID: String = "user_id" +private const val ACCESS_TOKEN: String = "access_token" + +private val JTW_TOKEN = JwtTokenDataJson( + userId = USER_ID, + email = "test@bitwarden.com", + isEmailVerified = true, + name = "Bitwarden Tester", + expirationAsEpochTime = 1697495714, + hasPremium = false, + authenticationMethodsReference = listOf("Application"), +) + +private const val JWT_ACCESS_TOKEN = "jwt" + +private val RESPONSE_401 = Response.Builder() + .code(401) + .request( + request = Request.Builder() + .header(name = "Authorization", value = "Bearer $JWT_ACCESS_TOKEN") + .url("https://www.bitwarden.com") + .build(), + ) + .protocol(Protocol.HTTP_2) + .message("Unauthenticated") + .build() diff --git a/network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt b/network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt index 15c45841ed..ab9f28d5ef 100644 --- a/network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt +++ b/network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt @@ -1,7 +1,6 @@ package com.bitwarden.network.retrofit -import com.bitwarden.network.authenticator.RefreshAuthenticator -import com.bitwarden.network.interceptor.AuthTokenInterceptor +import com.bitwarden.network.interceptor.AuthTokenManager import com.bitwarden.network.interceptor.BaseUrlInterceptors import com.bitwarden.network.interceptor.HeadersInterceptor import com.bitwarden.network.model.NetworkResult @@ -32,7 +31,8 @@ import retrofit2.create import retrofit2.http.GET class RetrofitsTest { - private val authTokenInterceptor = mockk { + private val authTokenManager = mockk { + mockAuthenticate { isRefreshAuthenticatorCalled = true } mockIntercept { isAuthInterceptorCalled = true } } private val baseUrlInterceptors = mockk { @@ -49,9 +49,6 @@ class RetrofitsTest { private val headersInterceptors = mockk { mockIntercept { isHeadersInterceptorCalled = true } } - private val refreshAuthenticator = mockk { - mockAuthenticate { isRefreshAuthenticatorCalled = true } - } private val json = Json private val server = MockWebServer() private val certificateProvider = mockk { @@ -61,10 +58,9 @@ class RetrofitsTest { } private val retrofits = RetrofitsImpl( - authTokenInterceptor = authTokenInterceptor, + authTokenManager = authTokenManager, baseUrlInterceptors = baseUrlInterceptors, headersInterceptor = headersInterceptors, - refreshAuthenticator = refreshAuthenticator, certificateProvider = certificateProvider, json = json, ) @@ -281,10 +277,9 @@ class RetrofitsTest { anyConstructed().sslSocketFactory(any(), any()) } returns mockk(relaxed = true) val retrofits = RetrofitsImpl( - authTokenInterceptor = authTokenInterceptor, + authTokenManager = authTokenManager, baseUrlInterceptors = baseUrlInterceptors, headersInterceptor = headersInterceptors, - refreshAuthenticator = refreshAuthenticator, certificateProvider = certificateProvider, json = json, )