Skip to content

getClaimAsBoolean() should not be falsy #10356

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
Merged
merged 1 commit into from
Oct 14, 2021
Merged

getClaimAsBoolean() should not be falsy #10356

merged 1 commit into from
Oct 14, 2021

Conversation

qavid
Copy link
Contributor

@qavid qavid commented Oct 8, 2021 •
edited
Loading

Closes gh-10148

Note that OAuth2TokenIntrospectionClaimAccessor#isActive will now throw NPE if active claim value is null.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 8, 2021
@qavid qavid marked this pull request as ready for review October 8, 2021 23:11
This was referenced Oct 9, 2021
Closed
Closed
@sjohnr sjohnr self-assigned this Oct 11, 2021
@sjohnr sjohnr added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 11, 2021
@qavid qavid force-pushed the gh-10148 branch 2 times, most recently from 7cf869f to c086bd2 Compare October 12, 2021 20:10
@qavid
Copy link
Contributor Author

qavid commented Oct 12, 2021

@sjohnr sorry for additional changes, but I have updated implementation according to this comment:

If you feel like ClaimAccessor should handle null values, let's please open another ticket with the production use cases you have where a claim value is null.

Now claimValue.getClass() throws NPE, when convertedValue is null.

@sjohnr
Copy link
Contributor

sjohnr commented Oct 13, 2021

@qavid thanks, that's totally fine. I was waiting for that PR to get reviewed first. I'll circle back to this one soon, hopefully.

sjohnr
sjohnr approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@sjohnr sjohnr left a comment

Choose a reason for hiding this comment

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

HI @qavid, just one suggestion below. Once that change is made, is this ready to merge?

@sjohnr sjohnr merged commit 64e9ac9 into spring-projects:main Oct 14, 2021
@sjohnr
Copy link
Contributor

sjohnr commented Oct 14, 2021

Thanks @qavid, this is now in main!

@sjohnr sjohnr added this to the 5.6.0-RC1 milestone Oct 14, 2021
@sjohnr sjohnr added the type: breaks-passivity A change that breaks passivity with the previous release label Oct 14, 2021
@sjohnr sjohnr mentioned this pull request Oct 14, 2021
Closed
@qavid qavid deleted the gh-10148 branch October 14, 2021 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@sjohnr sjohnr sjohnr approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@sjohnr sjohnr

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: breaks-passivity A change that breaks passivity with the previous release type: bug A general bug
Projects
None yet
Milestone
5.6.0-RC1
Development

Successfully merging this pull request may close these issues.

getClaimAsBoolean should not be falsy
3 participants
@qavid @sjohnr @spring-projects-issues