spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.79k stars 5.9k forks source link

Spring Cloud Gateway - TokenRelayGatewayFilter - Fetching of access token by refresh token run into stale token error response #10082

Closed viZu closed 3 years ago

viZu commented 3 years ago

Describe the bug Hi, We are using Spring Cloud Gateway as BFF for a frontend application which is written in next.js and communicates to downsream services via ajax calls mainly. In this setup Spring Cloud Gateway acts as session termination component to pass the access token of a user further to the downstream services, which then check if the current user is allowed to access a resource. We recently ran into an issue where the TokenRelayGatewayFilter is not able to retrieve an access token for the current session to pass it further. I tried to debug the root cause of this and stumbled accross the WebClientReactiveRefreshTokenTokenResponseClient, which sends the users refresh token to our identity provider (Keycloak in that case) to fetch a new access token.

For some requests we receive a stale token response, which then results into returning a redirect response via ajax, which we cannot really handle, since redirecting for an ajax call does not make always sense (like for POST requests).

I further debugged the issue and saw that the TokenRelayGatewayFilter is using the same refresh token several times consecutively until Keycloak responds with that stale token error response. Which is because you can reuse a refresh token only once with our current setup. One way to fix this, is to extend this to a higher number, but since this is a company wide setting. Also if this results in any security issues, which I haven't thought about yet.

Therefore I would prefer that the request which fetches the access token by refresh token should store the new refresh token, which is returned within the response. Which it looks like already happens, but not often enough at it seems. Here is a sequence of used (shortened) refresh tokens which were send by TokenRelayGatewayFilter

eyJhbGci.....be4HhgDAZIQ8
eyJhbGci.....be4HhgDAZIQ8
eyJhbGci.....gCpTcen_i3u0
eyJhbGci.....gCpTcen_i3u0
eyJhbGci.....b-LK6LUegE-4
eyJhbGci.....b-LK6LUegE-4
eyJhbGci.....b-LK6LUegE-4  <--- Here I get the stale token response

Usage ot the TokenRelayGatewayFilter is nothing special:

    @Bean
    fun r(builder: RouteLocatorBuilder) = builder.routes {
        route(id = "http-bin") {
            path("/api/http/**")
            filters {
                rewritePath("/api/http", "")
                filter(tokenRelayGatewayFilterFactory.apply())
            }
            uri(downStreamUris.httpbin)
        }
        ...
    }

Further information about our setup:

We use ReactiveSessionRepository to wrap the JDCB based session repository, since we do not have any database in use which is supported by spring reactive session.

Here is the code for that wrapper:

class JdbcBackedWebSessionRepository<S>(
    private var jdbcSessionRepository: FindByIndexNameSessionRepository<S>
) : ReactiveSessionRepository<S> where S : Session {

    override fun createSession(): Mono<S> {
        return Mono.just(jdbcSessionRepository.createSession())
    }

    override fun save(session: S?): Mono<Void> {
        return Mono.fromRunnable {
            jdbcSessionRepository.save(session)
        }
    }

    override fun findById(id: String?): Mono<S> {
        return Mono.justOrEmpty(jdbcSessionRepository.findById(id))
    }

    override fun deleteById(id: String?): Mono<Void> {
        return Mono.fromRunnable {
            jdbcSessionRepository.deleteById(id)
        }
    }
}

To Reproduce Steps to reproduce the behavior.

Expected behavior As mentioned above: The WebClientReactiveRefreshTokenTokenResponseClient should always use the most recent refresh token. Also I am not sure, why TokenRelayGatewayFilter always triggers a fetch for the access token via refresh token, even if the current access token seems to be fine (like it is not expired).

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not. At times, we may require a sample, so it is good to try and include a sample up front.

sjohnr commented 3 years ago

@viZu, thanks for the report. From what you described, this seems like a concurrency issue, as it is not always happening consistently, but does happen frequently, is that correct? Since this seems to be triggered by Spring Cloud Gateway, have you opened an issue in that project to investigate the cause? If you are sure this is a Spring Security issue, can you provide a minimal, reproducible sample without SCG that reproduces the issue?

fabb commented 3 years ago

Also I am not sure, why TokenRelayGatewayFilter always triggers a fetch for the access token via refresh token, even if the current access token seems to be fine (like it is not expired).

That‘s weird. @sjohnr could that be related to a misconfiguration?

fabb commented 3 years ago

For some requests we receive a stale token response, which then results into returning a redirect response via ajax, which we cannot really handle, since redirecting for an ajax call does not make always sense (like for POST requests).

Can this behavior of spring security somehow be turned off? A redirect does not make too much sense for api requests, a 401 (in combination with a WWW-Authenticate header) would make more sense which then can be handled properly.

sjohnr commented 3 years ago

For some requests we receive a stale token response, which then results into returning a redirect response via ajax, which we cannot really handle, since redirecting for an ajax call does not make always sense (like for POST requests).

Can this behavior of spring security somehow be turned off? A redirect does not make too much sense for api requests, a 401 (in combination with a WWW-Authenticate header) would make more sense which then can be handled properly.

@fabb, thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. We prefer to use GitHub issues only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it).

In the mean time, I'd like to focus on determining how to reproduce this issue from the OP, and whether it is a spring-security or spring-cloud-gateway issue.

spring-projects-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

viZu commented 3 years ago

I now found a solution for using refresh tokens several times. There was a bug in my code which wraps the JDBC session handling, using Mono.defer fixed that bug. Here is the updated code fragment for anyone who is interested:

class JdbcBackedWebSessionRepository<S>(
    private var jdbcSessionRepository: FindByIndexNameSessionRepository<S>
) : ReactiveSessionRepository<S> where S : Session {

    override fun createSession(): Mono<S> {
        return Mono.defer {
            Mono.just(jdbcSessionRepository.createSession())
        }
    }

    override fun save(session: S?): Mono<Void> {
        return Mono.defer {
            Mono.fromRunnable<Void> {
                jdbcSessionRepository.save(session)
            }
        }
    }

    override fun findById(id: String?): Mono<S> {
        return Mono.defer {
            Mono.justOrEmpty(jdbcSessionRepository.findById(id))
        }
    }

    override fun deleteById(id: String?): Mono<Void> {
        return Mono.defer {
            Mono.fromRunnable<Void> {
                jdbcSessionRepository.deleteById(id)
            }
        }
    }
}

However there is still an issue with not reusing valid access tokens in TokenRelayGatewayFilter but i will open a new issue in spring cloud gateway for that