Skip to content

fix: map HMAC base64 decode failures to ErrInvalidTokenFormat - #889

Open
BetterAndBetterII wants to merge 2 commits into
ory:masterfrom
BetterAndBetterII:fix/invalid-token-base64
Open

fix: map HMAC base64 decode failures to ErrInvalidTokenFormat#889
BetterAndBetterII wants to merge 2 commits into
ory:masterfrom
BetterAndBetterII:fix/invalid-token-base64

Conversation

@BetterAndBetterII

@BetterAndBetterII BetterAndBetterII commented Sep 2, 2026

Copy link
Copy Markdown

Summary

  • HMAC token validation returned raw base64.CorruptInputError for malformed token segments, so ErrorToRFC6749Error fell through to an unrecognizable 500 ("The error is unrecognizable") instead of invalid_token.
  • Wrap those decode failures as fosite.ErrInvalidTokenFormat (same path as missing-dot tokens / malformed JWTs).

Fixes #727

Test plan

  • go test ./token/hmac/ -count=1
  • New TestValidateRejectsInvalidBase64AsRFC6749Error ($+valid token, invalid key/signature segments) asserts ErrInvalidTokenFormat and RFC6749 invalid_token

Summary by CodeRabbit

  • Bug Fixes

    • Invalid Base64 characters in token keys or signatures now produce a standardized invalid-token error.
    • Token validation errors now include clearer diagnostic details while preserving the appropriate protocol error response.
    • Tokens containing unsupported characters in key or signature values are rejected consistently.
  • Tests

    • Added coverage confirming invalid token inputs map correctly to the standard invalid_token response.

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
@BetterAndBetterII
BetterAndBetterII requested review from a team and aeneasr as code owners September 2, 2026 12:48
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e3b1fb12-c4be-4b72-9ade-f2e9279e95ec

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5d0a5 and a05f4eb.

⛔ Files ignored due to path filters (1)
  • internal/gen/key.go is excluded by !**/gen/**
📒 Files selected for processing (261)
  • access_error.go
  • access_error_test.go
  • access_request.go
  • access_request_handler.go
  • access_request_handler_test.go
  • access_request_test.go
  • access_response.go
  • access_response_test.go
  • access_response_writer.go
  • access_response_writer_test.go
  • access_write.go
  • access_write_test.go
  • arguments.go
  • arguments_test.go
  • audience_strategy.go
  • audience_strategy_test.go
  • authorize_error.go
  • authorize_error_test.go
  • authorize_helper.go
  • authorize_helper_test.go
  • authorize_helper_whitebox_test.go
  • authorize_request.go
  • authorize_request_handler.go
  • authorize_request_handler_oidc_request_test.go
  • authorize_request_handler_test.go
  • authorize_request_test.go
  • authorize_response.go
  • authorize_response_test.go
  • authorize_response_writer.go
  • authorize_response_writer_test.go
  • authorize_validators_test.go
  • authorize_write.go
  • authorize_write_test.go
  • client.go
  • client_authentication.go
  • client_authentication_jwks_strategy.go
  • client_authentication_jwks_strategy_test.go
  • client_authentication_test.go
  • client_manager.go
  • client_test.go
  • client_with_custom_token_lifespans.go
  • client_with_custom_token_lifespans_test.go
  • compose/compose.go
  • compose/compose_oauth2.go
  • compose/compose_openid.go
  • compose/compose_par.go
  • compose/compose_pkce.go
  • compose/compose_rfc7523.go
  • compose/compose_rfc8628.go
  • compose/compose_strategy.go
  • compose/compose_userinfo_vc.go
  • config.go
  • config_default.go
  • context.go
  • device_request.go
  • device_request_handler.go
  • device_request_handler_test.go
  • device_response.go
  • device_response_test.go
  • device_response_writer.go
  • device_write.go
  • device_write_test.go
  • equalKeys_test.go
  • errors.go
  • errors_test.go
  • fosite.go
  • fosite_test.go
  • generate.go
  • go_mod_indirect_pins.go
  • handler.go
  • handler/oauth2/flow_authorize_code_auth.go
  • handler/oauth2/flow_authorize_code_auth_test.go
  • handler/oauth2/flow_authorize_code_token.go
  • handler/oauth2/flow_authorize_code_token_test.go
  • handler/oauth2/flow_authorize_implicit.go
  • handler/oauth2/flow_authorize_implicit_test.go
  • handler/oauth2/flow_client_credentials.go
  • handler/oauth2/flow_client_credentials_storage.go
  • handler/oauth2/flow_client_credentials_test.go
  • handler/oauth2/flow_refresh.go
  • handler/oauth2/flow_refresh_test.go
  • handler/oauth2/flow_resource_owner.go
  • handler/oauth2/flow_resource_owner_storage.go
  • handler/oauth2/flow_resource_owner_test.go
  • handler/oauth2/helper.go
  • handler/oauth2/helper_test.go
  • handler/oauth2/introspector.go
  • handler/oauth2/introspector_jwt.go
  • handler/oauth2/introspector_jwt_test.go
  • handler/oauth2/introspector_test.go
  • handler/oauth2/providers.go
  • handler/oauth2/revocation.go
  • handler/oauth2/revocation_storage.go
  • handler/oauth2/revocation_test.go
  • handler/oauth2/storage.go
  • handler/oauth2/strategy.go
  • handler/oauth2/strategy_hmacsha_plain.go
  • handler/oauth2/strategy_hmacsha_prefixed.go
  • handler/oauth2/strategy_hmacsha_test.go
  • handler/oauth2/strategy_jwt.go
  • handler/oauth2/strategy_jwt_session.go
  • handler/oauth2/strategy_jwt_test.go
  • handler/openid/errors.go
  • handler/openid/flow_device_auth.go
  • handler/openid/flow_device_auth_test.go
  • handler/openid/flow_device_token.go
  • handler/openid/flow_device_token_test.go
  • handler/openid/flow_explicit_auth.go
  • handler/openid/flow_explicit_auth_test.go
  • handler/openid/flow_explicit_token.go
  • handler/openid/flow_explicit_token_test.go
  • handler/openid/flow_hybrid.go
  • handler/openid/flow_hybrid_test.go
  • handler/openid/flow_implicit.go
  • handler/openid/flow_implicit_test.go
  • handler/openid/flow_refresh_token.go
  • handler/openid/flow_refresh_token_test.go
  • handler/openid/helper.go
  • handler/openid/helper_test.go
  • handler/openid/storage.go
  • handler/openid/strategy.go
  • handler/openid/strategy_jwt.go
  • handler/openid/strategy_jwt_test.go
  • handler/openid/validator.go
  • handler/openid/validator_test.go
  • handler/par/flow_pushed_authorize.go
  • handler/par/flow_pushed_authorize_test.go
  • handler/pkce/handler.go
  • handler/pkce/handler_test.go
  • handler/pkce/storage.go
  • handler/rfc7523/handler.go
  • handler/rfc7523/handler_test.go
  • handler/rfc7523/session.go
  • handler/rfc7523/storage.go
  • handler/rfc8628/auth_handler.go
  • handler/rfc8628/auth_handler_test.go
  • handler/rfc8628/storage.go
  • handler/rfc8628/strategy.go
  • handler/rfc8628/strategy_hmacsha.go
  • handler/rfc8628/strategy_hmacsha_test.go
  • handler/rfc8628/token_handler.go
  • handler/rfc8628/token_handler_test.go
  • handler/verifiable/handler.go
  • handler/verifiable/handler_test.go
  • handler/verifiable/nonce.go
  • hash.go
  • hash_bcrypt.go
  • hash_bcrypt_test.go
  • helper.go
  • helper_test.go
  • i18n/default_catalog.go
  • i18n/i18n.go
  • i18n/i18n_test.go
  • i18n_helper.go
  • i18n_helper_test.go
  • integration/authorize_code_grant_public_client_pkce_test.go
  • integration/authorize_code_grant_public_client_test.go
  • integration/authorize_code_grant_test.go
  • integration/authorize_device_grant_request_test.go
  • integration/authorize_form_post_test.go
  • integration/authorize_implicit_grant_test.go
  • integration/authorize_jwt_bearer_required_iat_test.go
  • integration/authorize_jwt_bearer_required_jti_test.go
  • integration/authorize_jwt_bearer_test.go
  • integration/authorize_response_mode_test.go
  • integration/client_credentials_grant_test.go
  • integration/clients/error.go
  • integration/clients/introspect.go
  • integration/clients/jwt_bearer.go
  • integration/helper_endpoints_test.go
  • integration/helper_setup_test.go
  • integration/introspect_jwt_bearer_token_test.go
  • integration/introspect_token_test.go
  • integration/oidc_explicit_test.go
  • integration/oidc_implicit_hybrid_public_client_pkce_test.go
  • integration/oidc_implicit_hybrid_test.go
  • integration/placeholder.go
  • integration/pushed_authorize_code_grant_test.go
  • integration/refresh_token_grant_test.go
  • integration/resource_owner_password_credentials_grant_test.go
  • integration/revoke_token_test.go
  • internal/access_request.go
  • internal/access_response.go
  • internal/access_token_storage.go
  • internal/access_token_strategy.go
  • internal/authorize_code_storage.go
  • internal/authorize_code_strategy.go
  • internal/authorize_handler.go
  • internal/authorize_request.go
  • internal/authorize_response.go
  • internal/client.go
  • internal/device_code_rate_limit_strategy.go
  • internal/hash.go
  • internal/id_token_strategy.go
  • internal/introspector.go
  • internal/oauth2_auth_jwt_storage.go
  • internal/oauth2_client_storage.go
  • internal/oauth2_explicit_storage.go
  • internal/oauth2_owner_storage.go
  • internal/oauth2_refresh_storage.go
  • internal/oauth2_revoke_storage.go
  • internal/oauth2_storage.go
  • internal/oauth2_strategy.go
  • internal/openid_id_token_storage.go
  • internal/pkce_storage_strategy.go
  • internal/pushed_authorize_handler.go
  • internal/refresh_token_strategy.go
  • internal/request.go
  • internal/revoke_handler.go
  • internal/rfc8628_code_strategy.go
  • internal/rfc8628_core_storage.go
  • internal/rw.go
  • internal/storage.go
  • internal/test_helpers.go
  • internal/token_handler.go
  • internal/transactional.go
  • introspect.go
  • introspect_test.go
  • introspection_request_handler.go
  • introspection_request_handler_test.go
  • introspection_response_writer.go
  • introspection_response_writer_test.go
  • oauth2.go
  • pushed_authorize_request_handler.go
  • pushed_authorize_request_handler_test.go
  • pushed_authorize_response.go
  • pushed_authorize_response_writer.go
  • pushed_authorize_response_writer_test.go
  • request.go
  • request_test.go
  • response_handler.go
  • revoke_handler.go
  • revoke_handler_test.go
  • scope_strategy.go
  • scope_strategy_test.go
  • session.go
  • session_test.go
  • storage.go
  • storage/memory.go
  • storage/memory_test.go
  • storage/transactional.go
  • token/hmac/bytes.go
  • token/hmac/bytes_test.go
  • token/hmac/hmacsha.go
  • token/hmac/hmacsha_test.go
  • token/jwt/claims.go
  • token/jwt/claims_id_token.go
  • token/jwt/claims_id_token_test.go
  • token/jwt/claims_jwt.go
  • token/jwt/claims_jwt_test.go
  • token/jwt/claims_test.go
  • token/jwt/header.go
  • token/jwt/header_test.go
  • token/jwt/jwt.go
  • token/jwt/jwt_test.go
  • token/jwt/map_claims.go
  • token/jwt/map_claims_test.go
  • token/jwt/token.go
  • token/jwt/token_test.go
  • token/jwt/validation_error.go
  • tools.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • token/hmac/hmacsha.go
  • token/hmac/hmacsha_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The HMAC validator now maps malformed Base64 in token keys and signatures to fosite.ErrInvalidTokenFormat. Tests verify rejection of invalid characters and RFC 6749 invalid_token conversion. Copyright headers changed from 2025 to 2026.

Changes

HMAC token validation

Layer / File(s) Summary
Invalid token format handling
token/hmac/hmacsha.go, token/hmac/hmacsha_test.go
Base64 decoding errors for token signatures and keys now return fosite.ErrInvalidTokenFormat with the original error and debug message. Tests cover $ characters, malformed Base64 input, and RFC 6749 invalid_token mapping.
Repository copyright headers
*.go, compose/*, handler/*, integration/*, internal/*, storage/*, token/*
Changed copyright header years from 2025 to 2026. No runtime behavior changed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to a05f4

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)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The HMAC validation change and related tests are in scope. However, the PR also updates copyright years across many unrelated files, which is unrelated to issue #727. Remove the unrelated copyright-year changes from this PR, or move them into a separate focused PR. Keep the HMAC validation changes and their tests here.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: mapping HMAC Base64 decoding failures to ErrInvalidTokenFormat.
Description check ✅ Passed 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 …
Linked Issues check ✅ Passed The PR satisfies issue #727 by mapping malformed Base64 token, key, and signature segments to fosite.ErrInvalidTokenFormat, enabling the RFC 6749 invalid_token response. Tests cover the reported behav…
Docstring Coverage ✅ Passed 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 5…
Full details: Description check

Explanation

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 check

Explanation

The PR satisfies issue #727 by mapping malformed Base64 token, key, and signature segments to fosite.ErrInvalidTokenFormat, enabling the RFC 6749 invalid_token response. Tests cover the reported behavior.

Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
The command is terminated due to an 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid token (base64 error) isn't mapped to an RFC6749Error error

1 participant