Skip to content

Conversation

@developerkunal
Copy link
Contributor

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

🔧 Changes

This PR refactors the middleware to use a pure options pattern and integrates it with the new core package.

Types and Functions Changed:

Constructor:

  • New(validateToken ValidateToken, opts ...Option)New(opts ...Option) (*JWTMiddleware, error)
  • All options now return errors for validation: type Option func(*JWTMiddleware) error

New Functions:

  • WithValidateToken(ValidateToken) Option - Required option for token validation
  • WithLogger(Logger) Option - Optional logger for debugging JWT flow
  • GetClaims[T any](context.Context) (T, error) - Generic type-safe claims retrieval
  • MustGetClaims[T any](context.Context) T - Panicking variant of GetClaims
  • HasClaims(context.Context) bool - Check if claims exist in context

Types Added:

  • Logger interface - Logging interface compatible with log/slog
  • ErrorResponse struct - Structured JSON error response
  • validatorAdapter struct - Bridge between ValidateToken and core.TokenValidator

Error Handling:

  • DefaultErrorHandler - Rewritten for RFC 6750 OAuth 2.0 Bearer Token compliance
  • Structured error responses with error/error_description/error_code fields
  • WWW-Authenticate headers for all error responses
  • Extensible for future DPoP (RFC 9449) support

Token Extractors:

  • CookieTokenExtractor(string) TokenExtractor - Now validates empty cookie names
  • ParameterTokenExtractor(string) TokenExtractor - Now validates empty parameter names
  • Fixed cookie error handling to propagate all errors (not just ErrNoCookie)

Context Key:

  • Changed from exported type ContextKey struct{} to unexported type contextKey int
  • Prevents context key collisions following Go best practices

Usage Summary:

Before:

middleware := jwtmiddleware.New(
    jwtValidator.ValidateToken,
)
claims := r.Context().Value(jwtmiddleware.ContextKey{}).(*validator.ValidatedClaims)

After:

middleware, err := jwtmiddleware.New(
    jwtmiddleware.WithValidateToken(jwtValidator.ValidateToken),
    jwtmiddleware.WithLogger(slog.Default()), // Optional
)
claims, err := jwtmiddleware.GetClaims[*validator.ValidatedClaims](r.Context())

Test Coverage:

  • Main middleware: 98.2%
  • Core: 100.0%
  • Validator: 100.0%
  • JWKS: 100.0%
  • OIDC: 100.0%
  • Total: 99.4%

📚 References

🔬 Testing

Automated Tests:

  • All existing tests updated for new API
  • Added option_test.go with comprehensive option validation tests
  • Added logger integration tests covering all CheckJWT code paths
  • Added extractor edge case tests (case-insensitive Bearer, empty names, etc.)
  • Added Test_invalidError for error wrapper methods

Manual Testing:
All examples have been updated and build successfully:

cd examples/http-example && go build        # ✅ OK
cd examples/http-jwks-example && go build   # ✅ OK
cd examples/gin-example && go build         # ✅ OK
cd examples/echo-example && go build        # ✅ OK
cd examples/iris-example && go build        # ✅ OK

Testing Steps:

  1. Run make test - All tests pass with 99.4% coverage
  2. Run make lint - Zero linting issues
  3. Build all examples - All build successfully
  4. Test with actual JWT tokens using examples

…egration

  Changes:
  - Refactor middleware constructor from New(validateToken, opts...) to New(opts...)
  - Add WithValidateToken() as required option with fail-fast validation
  - Integrate middleware with core package using validatorAdapter bridge
  - Implement unexported contextKey int pattern for collision-free context storage
  - Add type-safe generic claims access: GetClaims[T](), MustGetClaims[T](), HasClaims()

  Logging:
  - Add WithLogger() option for comprehensive JWT validation logging
  - Implement debug, warn, and error logging throughout CheckJWT flow
  - Propagate logger from middleware through core to validator
  - Log token extraction, validation, errors, and exclusion handling

  Error Handling:
  - Implement RFC 6750 OAuth 2.0 Bearer Token error responses
  - Add structured ErrorResponse with error/error_description/error_code fields
  - Generate WWW-Authenticate headers for all error responses
  - Design extensible architecture for future DPoP (RFC 9449) support
  - Add comprehensive error handler tests (13 scenarios)

  Token Extractors:
  - Add input validation to CookieTokenExtractor and ParameterTokenExtractor
  - Fix cookie error handling to propagate non-ErrNoCookie errors
  - Add tests for case-insensitive Bearer scheme and edge cases
  - Validate empty parameter/cookie names at construction time

  Tests:
  - Add option_test.go with comprehensive coverage of all options
  - Add logger integration tests covering all CheckJWT paths
  - Add invalidError tests for Error(), Is(), and Unwrap() methods
  - Add extractor edge case tests (uppercase, mixed case, multiple spaces)
  - Achieve 99.4% total coverage (main: 98.2%, core: 100%, validator: 100%)

  Examples:
  - Update all examples (http, jwks, gin, echo, iris) to use new API
  - Replace old constructor calls with pure options pattern
  - Update claims access to use generic GetClaims[T]() API
  - Add commented logger examples in http-example

  Breaking Changes:
  - Constructor signature: New(opts...) instead of New(validateToken, opts...)
  - Claims access: GetClaims[T](ctx) instead of ctx.Value(ContextKey{})
  - Context key changed to unexported type for collision prevention

  Test Coverage:
  - Main middleware: 98.2%
  - Core: 100.0%
  - Validator: 100.0%
  - JWKS: 100.0%
  - OIDC: 100.0%
  - Total: 99.4%
@developerkunal developerkunal requested a review from a team as a code owner November 21, 2025 16:52
@developerkunal developerkunal changed the title refactor: implement pure options pattern for middleware with core integration refactor: PR 1.5 implement pure options pattern for middleware with core integration Nov 21, 2025
@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 95.69892% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.88%. Comparing base (1da96e0) to head (54615e2).

Files with missing lines Patch % Lines
middleware.go 91.66% 3 Missing and 3 partials ⚠️
extractor.go 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           v3-phase1-pr4-jwx-migration     #360      +/-   ##
===============================================================
- Coverage                        99.46%   98.88%   -0.59%     
===============================================================
  Files                               13       13              
  Lines                              562      718     +156     
===============================================================
+ Hits                               559      710     +151     
- Misses                               3        4       +1     
- Partials                             0        4       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@developerkunal developerkunal changed the base branch from master to v3-phase1-pr4-jwx-migration November 21, 2025 16:53
  Remove duplicate context key management from HTTP middleware and use
  core's SetClaims/GetClaims/HasClaims functions consistently. This
  establishes the standard pattern for all adapters.

  Changes:
  - Remove contextKey and claimsContextKey from middleware.go
  - Update CheckJWT to use core.SetClaims() for storing claims
  - Update GetClaims/MustGetClaims/HasClaims to delegate to core
  - Update test assertion to match core's error message

  Benefits:
  - Single source of truth for context key management in core
  - All adapters (HTTP, gRPC, Gin, Echo) will use same context key
  - Claims stored by any adapter can be retrieved by any other adapter
  - Zero collision risk with unexported contextKey type in core
  - Maintains clean API - HTTP users don't need to import core

  This ensures cross-adapter compatibility while keeping the HTTP
  middleware API user-friendly with convenience wrappers.
- Change WithValidateToken to WithValidator to accept *validator.Validator
- Update ErrValidateTokenNil to ErrValidatorNil
- Refactor validatorAdapter to use TokenValidator interface
- Update all examples (http, http-jwks, gin, echo, iris) to use WithValidator
- Add setupRouter/setupApp functions to all examples for testability
- Create comprehensive integration tests for all examples
- Update test fixtures to use non-expiring test token (expires 2099)
- Add testify dependency to example projects for testing
- Fix iris example to use iris native httptest package

This change enables future extensibility for methods like ValidateDPoP
by allowing explicit passing of the validator instance.
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.

3 participants