Repository navigation
fix: return null from asInt/asLong for out-of-range numeric claims - #817
Open
abdul-khaliq-khalid wants to merge 1 commit into
Open
abdul-khaliq-khalid wants to merge 1 commit into
abdul-khaliq-khalid wants to merge 1 commit into
Conversation
asInt() and asLong() only checked isNumber() before narrowing, so a claim
such as 4294967419 (2^32 + 123) came back as 123 and satisfied
withClaim("name", 123). Use canConvertToInt()/canConvertToLong() so
out-of-range values return null, as the Claim javadoc describes.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
JsonNodeClaim.asInt()andasLong()only checkeddata.isNumber()before calling Jackson'sasInt()/asLong(), and those calls keep the low 32/64 bits of a larger number. So a claim like"org_id": 4294967419(2^32 + 123) is returned as123instead ofnull, which is what theClaimjavadoc promises when a value "can't be converted to an Integer".This matters for verification because
withClaim(String, Integer)andwithClaim(String, Long)compare againstclaim.asInt()/claim.asLong(). Today a verifier built with.withClaim("org_id", 123)accepts a token carrying4294967419, and.withClaim("org_id", 123L)accepts18446744073709551739(2^64 + 123), so the claim check passes for a value that is not the one the application asked for.The guard now uses
canConvertToInt()/canConvertToLong(), the same checkasDate()andasInstant()already use. In-range values (including in-range floating point values) behave exactly as before; out-of-range values now returnnull, so the verifier rejects them withIncorrectClaimException. No public API changes.References
Same family as the recent NumericDate range fix (GHSA-cg7w-5cc2-f2xm), applied to the integer accessors.
Testing
Added
JsonNodeClaimTest.shouldGetNullIntIfValueOutOfIntRange,JsonNodeClaimTest.shouldGetNullLongIfValueOutOfLongRange, and twoJWTVerifierTestcases that verify HS256 tokens with2^32 + 123/2^64 + 123againstwithClaim("name", 123)/withClaim("name", 123L). All four fail onmasterand pass with this change. I ran./gradlew assemble apiDiff check jacocoTestReportlocally: tests on Java 11, 17 and 21, checkstyle and apiDiff are green. The Java 8 test tasks couldn't run on my machine because there is no JDK 8 toolchain for Apple Silicon, so CI will cover that one.Checklist