AppAuth-Android icon indicating copy to clipboard operation
AppAuth-Android copied to clipboard

Reconsider asynchronous process of refreshing the access token

Open dIeGoLi opened this issue 5 years ago • 5 comments

I try to integrate oidc auth in an existing android project. Network calls are generally made using AsyncTask. Following synchronous flow (executed in the single background thread for async tasks).

  1. Create http client and request
  2. Set http headers
  3. execute request, parse response etc.

The access token should be a part of the header. The access token has to be refreshed potentially. Thats why this library offers authState.performActionWithFreshTokens(AuthorizationService, Callback). I see how this can be a good choice. But as well it is quite hard to integrate in an existing large codebase because you have to restructure synchronous flows which already are on a background thread. This approach would force me to change a lot. I think the library should provider better ways to retrieve a fresh access token.

I tried to create synchronous flow using the guarded block pattern. But this fails because the waiting thread (single thread of AsyncTask thread pool) which has to wait does not allow to perform the token refresh because internally the AuthorizationService uses a TokenRequestTask which is an AsyncTask executed on the default async task executor as well (which is deprecated).

        new TokenRequestTask(
                request,
                clientAuthentication,
                mClientConfiguration.getConnectionBuilder(),
                SystemClock.INSTANCE,
                callback)
                .execute();

From Async Task:

Note: this function schedules the task on a queue for a single background thread or pool of threads depending on the platform version ....

    public final AsyncTask<Params, Progress, Result> execute(Params... params) {
        return executeOnExecutor(sDefaultExecutor, params);
    }

There exists a new alternative public final AsyncTask<Params, Progress, Result> executeOnExecutor(Executor exec, Params... params)

I see three possible solutions.

  1. AuthenticationService should use an exclusive Executor and use executeOnExecutor
  2. The executor could be a public property or fallback to default if not set.
  3. A new method in AuthState similar to:
public String getAccessToken(refreshIfNecessary: true) { 
   do basically the same but without using async task in AuthorizationService
}

Do you agree with me? Or do I maybe not see an obvious alternative? As well my android programming got a little rusty :) I would be as well willing to contribute, in case you think it is a good idea.

Thanks for your time!

dIeGoLi avatar Oct 28 '20 16:10 dIeGoLi

Unfortunately i see that issue has been discussed already here #123 and here #338 But it seems because this project lacks an active mantainer #444 we can not expect quick improvements. I hope a new maintainer can be found or maybe a commercial product evolves.

dIeGoLi avatar Oct 28 '20 18:10 dIeGoLi

I would personally rather get rid of AsyncTasks and use Kotlin coroutines. The token refresh calls should also live outside of the AuthorizationService so calles can wire up an Authenticator in retrofit that doesn't leak the custom tabs stuff.

raderto avatar Jan 13 '21 17:01 raderto

Hi all, I hit the same problem and came up with this OkHttpClient interceptor:

class AppAuthTokenRefreshInterceptor(context: Context) : Interceptor {

    private val mStateManager = AuthStateManager.getInstance(context)
    private val mConfiguration = Configuration.getInstance(context)
    private val mAuthService = AuthorizationService(
            context,
            AppAuthConfiguration.Builder()
                    .setConnectionBuilder(mConfiguration.connectionBuilder)
                    .build()
    )

    override fun intercept(chain: Interceptor.Chain): Response {
        val requestBuilder = chain.request().newBuilder()

        /*
         * we can runBlocking because this will run in a
         * background thread
         */
        runBlocking {

            suspendCoroutine<Unit> { continuation ->

                mStateManager.current.performActionWithFreshTokens(
                        mAuthService
                ) { accessToken, idToken, ex ->

                    requestBuilder.addHeader("Authorization", "Bearer $accessToken")
                    continuation.resume(Unit)

                }

            }

        }

        return chain.proceed(requestBuilder.build())
    }
}

I tested this solution with AppAuth sample application and retrofit, it seems to work as expected.

AlbyST avatar May 07 '21 08:05 AlbyST

Hi all, I hit the same problem and came up with this OkHttpClient interceptor:

class AppAuthTokenRefreshInterceptor(context: Context) : Interceptor {

    private val mStateManager = AuthStateManager.getInstance(context)
    private val mConfiguration = Configuration.getInstance(context)
    private val mAuthService = AuthorizationService(
            context,
            AppAuthConfiguration.Builder()
                    .setConnectionBuilder(mConfiguration.connectionBuilder)
                    .build()
    )

    override fun intercept(chain: Interceptor.Chain): Response {
        val requestBuilder = chain.request().newBuilder()

        /*
         * we can runBlocking because this will run in a
         * background thread
         */
        runBlocking {

            suspendCoroutine<Unit> { continuation ->

                mStateManager.current.performActionWithFreshTokens(
                        mAuthService
                ) { accessToken, idToken, ex ->

                    requestBuilder.addHeader("Authorization", "Bearer $accessToken")
                    continuation.resume(Unit)

                }

            }

        }

        return chain.proceed(requestBuilder.build())
    }
}

I tested this solution with AppAuth sample application and retrofit, it seems to work as expected.

Magic!

agustinsivoplas avatar Oct 10 '22 22:10 agustinsivoplas

Hi all, I hit the same problem and came up with this OkHttpClient interceptor:

class AppAuthTokenRefreshInterceptor(context: Context) : Interceptor {

    private val mStateManager = AuthStateManager.getInstance(context)
    private val mConfiguration = Configuration.getInstance(context)
    private val mAuthService = AuthorizationService(
            context,
            AppAuthConfiguration.Builder()
                    .setConnectionBuilder(mConfiguration.connectionBuilder)
                    .build()
    )

    override fun intercept(chain: Interceptor.Chain): Response {
        val requestBuilder = chain.request().newBuilder()

        /*
         * we can runBlocking because this will run in a
         * background thread
         */
        runBlocking {

            suspendCoroutine<Unit> { continuation ->

                mStateManager.current.performActionWithFreshTokens(
                        mAuthService
                ) { accessToken, idToken, ex ->

                    requestBuilder.addHeader("Authorization", "Bearer $accessToken")
                    continuation.resume(Unit)

                }

            }

        }

        return chain.proceed(requestBuilder.build())
    }
}

I tested this solution with AppAuth sample application and retrofit, it seems to work as expected.

Could you please add the imports to this? Thanks!

mobilekosmos avatar Mar 09 '23 04:03 mobilekosmos