What is the best practice to refresh OAuth access ...

# squarelibraries

[Lukasz Kalnik]

What is the best practice to refresh OAuth access tokens using a refresh token in OkHttp?
I have the following setup, but it's failing in case of parallel calls. The synchronized(lock) doesn't seem to prevent the second call to try to refresh the token as well.

val okHttpClient = OkHttpClient.Builder()
    .addInterceptor(AccessTokenInterceptor(oAuthRepository))
    .authenticator(RefreshTokenAuthenticator(oAuthRepository))
    .build()

val lock = Any()

private class AccessTokenInterceptor(
    private val oAuthRepository: OAuthRepository
) : Interceptor {

    override fun intercept(chain: Chain): Response {
        synchronized(lock) {
            val token = oAuthRepository.accessToken

            val request = chain.request().newBuilder().apply {
                if (token != null) header("Authorization", "Bearer $token")
            }.build()
            return chain.proceed(request)
        }
    }
}

private class RefreshTokenAuthenticator(
    private val oAuthRepository: OAuthRepository
) : Authenticator {

    override fun authenticate(route: Route?, response: Response): Request? {
        if (response.responseCount >= 2) return null

        synchronized(lock) {
            return runBlocking {
                oAuthRepository.refreshTokens().map {
                    oAuthRepository.accessToken
                }
            }.fold(
                ifLeft = { null },
                ifRight = {
                    it?.let { token ->
                        response.request.newBuilder()
                            .header("Authorization", "Bearer $token")
                            .build()
                    }
                }
            )
        }
    }
}

[yschimke]

Just a wild guess, but are the two refreshes happening for the same call? Might be the same thread and reentrant?

[Lukasz Kalnik]

I'm quite sure they happen for two subsequent calls.
But now, after reading a few examples on the web, I think I know what's missing.
The synchronized block for the second (and subsequent) calls is not skipped - it's just delayed. That means every next call tries (unnecessarily) to refresh the token (which has just been refreshed). So after entering the synchronized block I should check if the token has changed, and in case it's the new token, skip the refresh and just use it to authenticate.
This means I have to add:

override fun authenticate(route: Route?, response: Response): Request? {
    val oldToken = oAuthRepository.accessToken

    synchronized(lock) {
        val newToken = oAuthRepository.accessToken
        // if the newToken is different, it means that it has been refreshed in the mean time by another call
        // so we can just use it to authenticate and skip the refresh
        if (newToken != oldToken) return response.request.newBuilder()
            .header("Authorization", "Bearer $newToken")
            .build()
        // Otherwise this is the first call trying to refresh a token - so just refresh it
        // Previous refresh flow
     }
}

[Lukasz Kalnik]

See example here

[jessewilson]

do not hold a lock while calling return chain.proceed(request) unless you want to avoid all paralellism

[Lukasz Kalnik]

Thank you, I was also thinking the lock in the AccessTokenInterceptor was actually unnecessary/harmful.

[Colton Idle]

This is what I use and it seems to work without issues. I've stress tested it with charles proxy and send a bunch of 401s and stuff and it seems to work perfectly. Maybe not the best code... but it works.

https://gist.github.com/ColtonIdle/3da8930a486ae5d88ae014a93539a0ac

[Lukasz Kalnik]

Thanks. I now check if the access token is still the same after entering the synchronized block. If it changed, that means it was already refreshed in another thread and I just skip the refresh and use the new token.

[Lukasz Kalnik]

Like here:
https://gist.github.com/naturalwarren/bc7f64f003a0e6034c6e74a2aa8917f6