Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vaa25
Copy link

@vaa25 vaa25 commented Dec 19, 2024

closes gh-16231

@pivotal-cla
Copy link

@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);
Copy link
Contributor

@jzheaux jzheaux Dec 19, 2024

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Contributor

@jzheaux jzheaux Dec 19, 2024

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jwt Principal customization
5 participants