gh-16231 add JwtPrincipalConverter.java support #16311
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.
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.
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.
For additional context, please see the code in this sample application.
There was a problem hiding this comment.
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.
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.
spring-projects-issues
added
the
status: waiting-for-triage
|
I'm going to close this as declined in favor of #6237 as I feel that will be a better driver for enabling more use cases than just this one. |
jzheaux
added
type: enhancement
closes gh-16231