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

MappedJwtClaimSetConverter#withDefaults doesn't remove claims from JWT as documented #10142

Closed
wants to merge 2 commits into from

Conversation

fguenci
Copy link

@fguenci fguenci commented Jul 27, 2021

Previously, ClaimTypeConverter doesn't overwrite claim's value converted to null.

This commit allows ClaimTypeConverter to overwrite and return claim with
value converted to null.

Closes #10135

to null.

This commit allows ClaimTypeConverter to overwrite and return claim with
value converted to null.

Closes spring-projects#10135
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 27, 2021
@pivotal-cla
Copy link

@fguenci Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@marcusdacoregio marcusdacoregio added 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 labels Jul 27, 2021
@marcusdacoregio marcusdacoregio assigned jzheaux and unassigned sjohnr Jul 27, 2021
@marcusdacoregio marcusdacoregio added the type: bug A general bug label Jul 27, 2021
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @fguenci, this looks great! I've left some feedback inline to help with overall consistency with the rest of the codebase.

Also, in preparation for merging, will you please squash commits and format the final commit like so:

Title ~50 Characters or Less

Any additional description with a  width of ~70 characters

Closes gh-10135

maybe something like this:

Preserve Null Claim Values

Closes gh-10135

If you like, you can find more details in the contributing guidelines.

@pivotal-cla
Copy link

@fguenci Thank you for signing the Contributor License Agreement!

Prior to this commit ClaimTypeConverter returned the claims with the
original value for all the claims with a null converted value.
The changes allows ClaimTypeConverter to overwrite and return claims
with converted value of null.

Closes spring-projectsgh-10135
@fguenci fguenci requested a review from jzheaux July 29, 2021 16:26
@jzheaux
Copy link
Contributor

jzheaux commented Aug 12, 2021

Thanks, @fguenci! I squashed your commits and merged them in 30a1c1a. I also added a polish commit of b83a4c2 to preserve backward compatibility in ClaimTypeConverter.

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: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MappedJwtClaimSetConverter#withDefaults doesn't remove claims from JWT as documented
6 participants