-
Notifications
You must be signed in to change notification settings - Fork 6k
Add JwtIssuerAuthenticationManagerResolver #7733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, some initial thoughts. I'm not marking it as Request Changes since we can elaborate on each
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...g/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...amework/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolver.java
Outdated
Show resolved
Hide resolved
...rk/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolverTests.java
Outdated
Show resolved
Hide resolved
a9137a3
to
3a12204
Compare
...rk/security/oauth2/server/resource/authentication/JwtAuthenticationManagerResolverTests.java
Outdated
Show resolved
Hide resolved
@dyroberts how well would this PR address some of the concerns you raised in #5385? With it, you could do either: JwtAuthenticationManagerResolver authenticationManagerResolver =
new JwtAuthenticationManagerResolver("list", "of", "issuers");
http
.oauth2ResourceServer()
.authenticationManagerResolver(authenticationManagerResolver) or Map<String, AuthenticationManager> authenticationManagers = ...;
JwtAuthenticationManagerResolver authenticationManagerResolver =
new JwtAuthenticationManagerResolver(authenticationManagers::get);
http
.oauth2ResourceServer()
.authenticationManagerResolver(authenticationManagerResolver); |
@jzheaux Looks really good! The only thing I could see as a slight improvement would be some auto-config to create a |
@dyroberts we are still looking into the idea of mapping this to config properties. While this does appear to be a common way to handle multi-tenancy in resource servers, it's not clear whether or not Resource Server Multi-Tenancy itself is so common as to want to add Spring Boot support. That said, this class leaves open this possibility. |
@gburboz @xsreality @bertramn How well would this PR address the concerns you raised in #6778 and #5351? |
Hi Josh, the PR looks good. My primary concern was to be able to add/remove tenants at runtime without needing to restart the application. The updated doc makes it clear how to do that. Thanks! |
@jzheaux will you be adding a reactive version of this class? Thanks |
@davidmelia I think it makes sense to explore that, yes - I left a bit more detail in the ticket you raised. |
Hi @jzheaux I have similar use case. Project setup Details - org.springframework.boot" version "2.3.1.RELEASE" I have gone through this support documentation but need some help to implement this. |
Thanks for reaching out, @RavindraSengar! In the future, please don't double-post as it creates noise. I've left some suggestions for you about multiple issuers over in the other ticket where you asked a similar question. |
Fixes gh-7724