-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
gh-16231 add JwtPrincipalConverter.java support #16311
base: main
Are you sure you want to change the base?
Conversation
@vaa25 Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
*/ | ||
public interface JwtPrincipalConverter { | ||
|
||
Object convert(Jwt jwt, String principalName); |
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.
Just connecting the two conversations here. This should return a Spring Security interface. That allows Spring to evolve around the API over time and reason about its contents. If it returns Object
, we lose that ability.
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.
This is, for example, why UserDetailsService
returns a UserDetails
and not Object
. This is similar for OpaqueTokenIntrospector
and other principal-deriving interfaces in Spring Security.
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.
For additional context, please see the code in this sample application.
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.
Authentication.principal is an Object. Any other type would restrict principal type. If Object is not allowed by SS code convention, then could it be some marker interface? That marker interface could be a superinterface for other principal interfaces like AuthenticatedPrincipal. Or maybe it could be AuthenticatedPrincipal itself. Sorry, I am not very familiar with SS architecture and not figure out what to do in this situation.
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.
No problem. What I'd recommend is that it return OAuth2AuthenticatedPrincipal
. This allows a converter to transmit any user-level authorities (which can be different from the authorities derived from the JWT) along with the custom user.
If implementing extra methods is a concern, I think we can look at adding default
implementations to some of those methods.
This doesn't restrict the type like you might think; since it is an interface, any custom principal can implement it with a few lines.
closes gh-16231