fix: map HMAC base64 decode failures to ErrInvalidTokenFormat - #889
fix: map HMAC base64 decode failures to ErrInvalidTokenFormat#889BetterAndBetterII wants to merge 2 commits into
Conversation
Corrupt token segments previously returned a raw base64.CorruptInputError, so ErrorToRFC6749Error fell through to an unrecognizable 500 instead of invalid_token. Wrap those decode errors as ErrInvalidTokenFormat. Fixes ory#727
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (261)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe HMAC validator now maps malformed Base64 in token keys and signatures to ChangesHMAC token validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Malformed Base64 HMAC token segments now return the RFC 6749 invalid_token response rather than an unrecognized server error. The scoped behavior change is covered by tests, with no remaining merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the bug, the fix, the linked issue, and the test plan. It does not reproduce the full checklist from the template, but it is sufficiently complete and directly related to the change. Full details: Linked Issues checkExplanation The PR satisfies issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 50 files. (211 skipped: 211 over the file limit.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.13.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
base64.CorruptInputErrorfor malformed token segments, soErrorToRFC6749Errorfell through to an unrecognizable 500 ("The error is unrecognizable") instead ofinvalid_token.fosite.ErrInvalidTokenFormat(same path as missing-dot tokens / malformed JWTs).Fixes #727
Test plan
go test ./token/hmac/ -count=1TestValidateRejectsInvalidBase64AsRFC6749Error($+valid token, invalid key/signature segments) assertsErrInvalidTokenFormatand RFC6749invalid_tokenSummary by CodeRabbit
Bug Fixes
Tests
invalid_tokenresponse.