spring-projects / spring-projects/spring-security

Highly concurrent requests with `client_credentials` cause duplicate access token requests

Open
#15,295 6 comments 10 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: oauth2 type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

Expected Behavior
OAuth tokens could be reused. Even if multiple requests happen concurrently.

Current Behavior
If a lot of token requests happen concurrently each request retrieves its own access token.

Context
As described in https://stackoverflow.com/questions/78635726/what-is-the-expected-number-of-oauth2-client-logins-with-webflux-and-spring-secu we're issuing a lot of WebClient concurrently. In the meantime we've discoverd that the initial request has been cached. So the sample would be more correct if you'd something along

Flux.range(1, 1000)
    .flatMap { request() }

Issues like https://github.com/spring-projects/spring-security/issues/11461 show that it's a known limitation of the current implementation.

Having a look at the ClientCredentialsReactiveOAuth2AuthorizedClientProvider default implementation I drafted a version that caches the token requests using Caffeine like

class CachingClientCredentialsReactiveOAuth2AuthorizedClientProvider : ReactiveOAuth2AuthorizedClientProvider {

    private val accessTokenClient: ReactiveOAuth2AccessTokenResponseClient<OAuth2ClientCredentialsGrantRequest> =
        WebClientReactiveClientCredentialsTokenResponseClient()

    private val cache: AsyncLoadingCache<ClientRegistration, OAuth2AccessToken> = Caffeine.newBuilder()
        .expireAfter(OAuth2AccessTokenExpiry())
        .buildAsync { clientRegistration, _ -> getAccessToken(clientRegistration) }

    override fun authorize(context: OAuth2AuthorizationContext): Mono<OAuth2AuthorizedClient> {
        val clientRegistration: ClientRegistration = context.clientRegistration

        if (clientRegistration.authorizationGrantType != AuthorizationGrantType.CLIENT_CREDENTIALS) {
            return Mono.empty()
        }

        return Mono.fromFuture { cache.get(clientRegistration) }
            .map { OAuth2AuthorizedClient(clientRegistration, context.principal.name, it) }
    }

    private fun getAccessToken(clientRegistration: ClientRegistration): CompletableFuture<OAuth2AccessToken> =
        OAuth2ClientCredentialsGrantRequest(clientRegistration).toMono()
            .flatMap { accessTokenClient.getTokenResponse(it) }
            .onErrorMap(OAuth2AuthorizationException::class.java) {
                ClientAuthorizationException(it.error, clientRegistration.registrationId, it)
            }
            .map { it.accessToken }
            .toFuture()
}

class OAuth2AccessTokenExpiry : Expiry<ClientRegistration, OAuth2AccessToken> {

    override fun expireAfterCreate(
        clientRegistration: ClientRegistration,
        token: OAuth2AccessToken,
        currentTimeInNanos: Long,
    ): Long = token.cacheDurationInNanos()

    override fun expireAfterUpdate(
        clientRegistration: ClientRegistration,
        token: OAuth2AccessToken,
        currentTimeInNanos: Long,
        currentDurationInNanos: Long,
    ): Long = token.cacheDurationInNanos()

    override fun expireAfterRead(
        clientRegistration: ClientRegistration,
        token: OAuth2AccessToken,
        currentTimeInNanos: Long,
        currentDurationInNanos: Long,
    ): Long = token.cacheDurationInNanos()
}

private fun OAuth2AccessToken.cacheDurationInNanos(): Long =
    Duration.between(Instant.now(), expiresAt)
        .minus(Duration.ofMinutes(1))
        .toNanos()

The following chart shows our token requests with the default implementation and then switches to my version around 11:00:
grafik

I'm well aware that spring-security has no dependency on caffeine (and likely won't introduce one) but maybe it's possible to include such an implementation somehow? I'm open for suggestions and willing to provide a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the ClientCredentialsReactiveOAuth2AuthorizedClientProvider default implementation and compare its behavior with the limitation described in issue 11461. Reproduce many concurrent client_credentials requests, then determine an implementation that reuses the in-flight and cached access token without adding a Caffeine dependency. Done means concurrent requests no longer issue duplicate token requests while token expiry is respected.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, kotlin, spring
Domain
authentication, backend, security
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.