ch4mpy / spring-addons

Ease spring OAuth2 resource-servers configuration and testing
Apache License 2.0
545 stars 87 forks source link

Parsing of token introspection responses from Keycloak fails when using the reactive stack #226

Closed ebbe-brandstrup closed 1 month ago

ebbe-brandstrup commented 1 month ago

Hi

I am experiencing the same problem as was reported in https://github.com/ch4mpy/spring-addons/issues/153, but I am using the reactive stack (the reporter of that issue was using MVC). The fix you made for that issue back in version 7.1.13 was only done on that side of the stack.

The error I get back when the oidc starter runs its default ReactiveOpaqueTokenAuthenticationConverter bean is:

java.lang.ClassCastException: class java.time.Instant cannot be cast to class java.lang.Integer (java.time.Instant and java.lang.Integer are in module java.base of loader 'bootstrap')
    at com.c4_soft.springaddons.security.oidc.starter.reactive.resourceserver.ReactiveSpringAddonsOidcResourceServerBeans.lambda$introspectionAuthenticationConverter$13(ReactiveSpringAddonsOidcResourceServerBeans.java:333) ~[spring-addons-starter-oidc-7.8.8.jar:na]

I am using the OIDC starter (v7.8.8) in a Spring Boot 3.3.2 service. The IdP that does the token introspection is Keycloak v21.1.2.

When I look at the source code in the MVC (synchronised) package (in com.c4_soft.springaddons.security.oidc.starter.synchronised.resourceserver.SpringAddonsOidcResourceServerBeans) I think the same approach of inspecting the claims and converting as appropriate needs to be added in com.c4_soft.springaddons.security.oidc.starter.reactive.resourceserver.ReactiveSpringAddonsOidcResourceServerBeans?

And last (but not least), thank you very much for all the effort you put into this starter and into answering questions on Stack Overflow!

ch4mpy commented 1 month ago

I think the same approach of inspecting the claims and converting as appropriate needs to be added in com.c4_soft.springaddons.security.oidc.starter.reactive.resourceserver.ReactiveSpringAddonsOidcResourceServerBeans

Of course, it should. It's ridiculous it wasn't fixed at the same time as the synchronized code. Sorry about that.

Would you like to submit a PR with the fix and be listed among contributors? Or should I just push the fix and release?

P.S.

If you read what I post on StackOverflow, you might have come across my advice not to use introspection: it adds latency to each and every request to your resource server(s), will overload the authorization server if the traffic increases, and brings no benefit in terms of security.

Also, at the age of virtual threads, I use WebFlux only when forced to (when using the reactive Spring Cloud Gateway to create an OAuth2 BFF). Servlets are so much easier to debug and just as resource-efficient (if not better).

ebbe-brandstrup commented 1 month ago

I'll gladly submit a PR, yes. I'll do that later today.

My use case is very close to that Baeldung tutorial you linked. It has been a huge help in my current task (adding authentication into a Spring Cloud Gateway service that acts as an API hub for a bunch of micro services that each expose their own APIs). That gateway service will be the single entry point for SPA frontends for those APIs.

It needs to offer both the BFF approach, where the SPAs never see the tokens, as well as support SPAs that use their own public client and deal with tokens themselves. So I am using both the client and the resourceserver security filter chains in the oidc starter in my gateway.

I have /bff/api/ routes added in the matchers for the client security filter chain for SPAs that want to use the session-based approach and /api/ routes in the gateway for clients that want to manage tokens themselves (which are handled by the resourceserver security filter chain because it handles all the paths that aren't matched by the client security filter chain).

The routes for /bff/api/ all use the TokenRelay and SaveSession filters. The /api/ routes are meant to introspect the bearer tokens and route valid requests downstream (also forwarding the tokens) where the tokens then get decoded locally in a micro service, but not introspected.

I have actually come across a problem with the SaveSession filter for the BFF routes after having followed the Baeldung tutorial. Maybe I should ask in a separate issue, but let me quickly as here and just tell me if you want me to create a separate issue ;)

What I am seeing is that what gets saved as the SPRING_SECURITY_CONTEXT in the in-mem user sessions (not using Spring Sessions yet) never gets updated when token-refresh flows take place when a later request is made and the TokenRelay filter decides to use the refresh token to get a new access token. I would have expected that the SaveSession filter would replace the tokens in the user's session with the new tokens that were returned in the refresh flow. As a result, because my /me endpoint is on the gateway (not in a separate downstream service which the TokenRelay filter would ensure gets the newly minted access token) I always get back the initial expiration time of the initially generated access token, and my "keep- alive" logic in the SPA frontend can't rely on it.

ch4mpy commented 1 month ago

With your current setup, there is a useless introspection for each request to /api/**. On the gateway, I'd do a permitAll() for /api/** with any STATELESS security filter-chain. You probably have one for actuator already - with basic, oauth2ResourceServer, x509, or whatever.

The BFF pattern aims at setting an access token stored in session using the TokenRelay filter. But there is no token check to do on the gateway. If there is no Bearer token to add to a request, then the Gateway can let it pass untouched. The downstream resource servers will validate the tokens the same way whatever the request (it should see no difference between those routed through /bff/api/** and those routed through /api/**). You could even use something like that to save the maximum resources on the gateway:

@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityWebFilterChain disabledSecurity(ServerHttpSecurity http) throws Exception {
  http.securityMatcher(new PathPatternParserServerWebExchangeMatcher("/api/**"));

  http.httpBasic(basic -> basic.disable());
  http.formLogin(login -> login.disable());

  http.securityContextRepository(NoOpServerSecurityContextRepository.getInstance());
  http.csrf(csrf -> csrf.disable());

  http.authorizeExchange(exchanges -> exchanges.anyExchange().permitAll());

  return http.build();
}

There is currently a bug in the Spring Security framework: the ID tokens are not updated during the refresh token flow.

Access tokens are refreshed when accessed using the (Reactive)OAuth2AuthorizedClientManager. Maybe are you accessing them with the (Reactive)ClientRegistrationRepository? If that's the case, you should change that (use the manager instead of the repository).

The tokenRelay filter uses the manager. So if your /me endpoint was in a resource server as done in the article, you wouldn't have the problem. As a side note, I wouldn't give more responsibilities to the gateway than handling tokens and routing requests. This requires state, which doesn't scale very well. So the less responsibilities, the better.

ch4mpy commented 1 month ago

@ebbe-brandstrup I just released your fix as 7.8.9. Thank you for your contribution.