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.

Sign up for GitHub

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

Jump to bottom

Principal claim name in JwtAuthenticationConverter is null but documented default #10214

Closed
djechelon opened this issue Aug 19, 2021 · 1 comment
Closed

Principal claim name in JwtAuthenticationConverter is null but documented default #10214

djechelon opened this issue Aug 19, 2021 · 1 comment
Assignees
@jzheaux
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Milestone
5.6.0-M3

Comments

@djechelon
Copy link
Contributor

Describe the bug

At JwtAuthenticationConverter the principalClaimName is null, but the setter method is documented

Sets the principal claim name. Defaults to {@link JwtClaimNames#SUB}.

Expected fix
Initialize by default the principalClaimName to sub, preferrably by using constant JwtClaimNames.SUB
@djechelon djechelon added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Aug 19, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Aug 19, 2021

Hi, @djechelon, thanks for the report.

I believe this is because if the name is null it uses the JwtAuthenticationToken constructor that defaults to SUB.

That said, it would be nicer if this logic were simplified.

Can you submit a PR that sets the claim name to SUB and then also simplifies the logic to always call the constructor that takes the claim value?

@jzheaux jzheaux self-assigned this Aug 19, 2021
@jzheaux jzheaux added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Aug 19, 2021
@jzheaux jzheaux added this to the 5.6.0-M3 milestone Aug 19, 2021
@djechelon djechelon mentioned this issue Aug 20, 2021
Closed
djechelon added a commit to djechelon/spring-security that referenced this issue Aug 20, 2021
@djechelon
Fix spring-projectsgh-10214
b372c2c 
JwtAuthenticationConverter#principalClaimName is never going to be null from now on
@jzheaux jzheaux added the status: duplicate A duplicate of another issue label Aug 20, 2021
akohli96 pushed a commit to akohli96/spring-security that referenced this issue Aug 25, 2021
@djechelon
Default principalClaimName to SUB
fc6057a 
Closes spring-projectsgh-10214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jzheaux jzheaux

Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.6.0-M3
Development

No branches or pull requests
2 participants
@djechelon @jzheaux