Skip to content

Add constructor to JwtAuthenticationToken that takes a principal name #6865

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

Closed
jzheaux opened this issue May 13, 2019 · 5 comments
Closed

Add constructor to JwtAuthenticationToken that takes a principal name #6865

jzheaux opened this issue May 13, 2019 · 5 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented May 13, 2019

OAuth2IntrospectionAuthenticationToken takes a name as a constructor parameter and otherwise defaults to the token's SUB claim for the principal name.

JwtAuthenticationToken, on the other hand, provides no way to customize this.

It'd be nice to do the same for JwtAuthenticationToken:

public JwtAuthenticationToken(Jwt jwt, Collection<? extends GrantedAuthority> authorities, String name) {
    // ...

This encourages composition over inheritance while also keeping the application from having to create a custom class to override this default behavior.

@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement status: first-timers-only An issue that can only be worked on by brand new contributors labels May 13, 2019
@HaydenMeloche
Copy link
Contributor

HaydenMeloche commented May 16, 2019

I'd like to submit a PR for this issue. Taking a look at JwtAuthenticationToken it doesn't currently have a name value like OAuth2IntrospectionAuthenticationToken does. I also noticed that currently getName returns the tokens subject. Should I be creating a new name variable or setting something somewhere else?

Sorry if silly question, first time around here :)

@rhamedy
Copy link
Contributor

rhamedy commented May 17, 2019

@HaydenMeloche if I understand correctly, you will be mimicking the OAuth2IntrospectionAuthenticationToken by adding name and introducing the suggested constructor.

As far the getName question, if you look at OAuth2IntrospectionAuthenticationToken constructor the line this.name = name == null ? (String) attributes.get(SUBJECT) : name; does the trick of either setting the provided name (if not null) or defaulting to attributes.get(SUBJECT) so when the getName() returns this.name it is either of the two hence, functioning the same way as OAuth2IntrospectionAuthenticationToken.

You might be able to provide a PR for this since it is tagged with first-timers-only and no one has claimed it yet 👍

I am not part of Spring team but, I felt like I could answer your questions and I hope I did not confuse you 🙂

@jzheaux
Copy link
Contributor Author

jzheaux commented May 20, 2019

@HaydenMeloche welcome! Yes, @rhamedy's analysis is correct, and yes, it'd be great to see a PR from you.

@jzheaux jzheaux removed the status: first-timers-only An issue that can only be worked on by brand new contributors label May 20, 2019
@HaydenMeloche
Copy link
Contributor

I believe this issue can be closed now.
#6893

@jzheaux
Copy link
Contributor Author

jzheaux commented May 24, 2019

Closed via f84ab3a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants