Skip to content

Conversation

@appleboy
Copy link
Owner

  • Clarify that standard JWT claims (sub, iss, aud, nbf, iat, jti) can now be set via PayloadFunc for RFC 7519 compliance, while only framework-controlled claims (exp, orig_iat) are restricted.
  • Update documentation to explain how to use standard claims in PayloadFunc, with examples in English and both Chinese guides.
  • Add multiple tests verifying that standard JWT claims are accepted through PayloadFunc and that framework-controlled claims cannot be overwritten.
  • Add a test demonstrating how to use the sub claim as the primary user identifier.

fix #362

- Clarify that standard JWT claims (sub, iss, aud, nbf, iat, jti) can now be set via PayloadFunc for RFC 7519 compliance, while only framework-controlled claims (exp, orig_iat) are restricted.
- Update documentation to explain how to use standard claims in PayloadFunc, with examples in English and both Chinese guides.
- Add multiple tests verifying that standard JWT claims are accepted through PayloadFunc and that framework-controlled claims cannot be overwritten.
- Add a test demonstrating how to use the sub claim as the primary user identifier.

fix #362

Signed-off-by: appleboy <[email protected]>
Copilot AI review requested due to automatic review settings December 14, 2025 04:29
- Refactor single-line assert.Equal calls into multi-line format for better readability in two test functions

Signed-off-by: appleboy <[email protected]>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enables RFC 7519 compliance by allowing standard JWT claims (sub, iss, aud, nbf, iat, jti) to be set via PayloadFunc, while keeping framework-controlled claims (exp, orig_iat) protected from user overrides.

Key Changes:

  • Modified claim filtering logic to only protect framework-controlled claims instead of all standard JWT claims
  • Added comprehensive tests verifying standard claims can be set and framework claims cannot be overwritten
  • Updated documentation in English and Chinese variants to explain standard claim usage with examples

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
auth_jwt.go Refactored frameworkClaims map to only protect exp and orig_iat, allowing users to set standard JWT claims via PayloadFunc
auth_jwt_test.go Added four comprehensive tests covering standard claim usage, framework claim protection, individual claim verification, and sub claim as user identifier
README.md Added documentation section explaining standard JWT claims with examples and note about framework-controlled claims
README.zh-CN.md Added Chinese (Simplified) documentation for standard JWT claims with translated examples
README.zh-TW.md Added Chinese (Traditional) documentation for standard JWT claims with translated examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1913 to +1916

// Verify custom claims are present
assert.Equal(t, "user123", claims["identity"])
assert.Equal(t, "admin", claims["role"])
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The test comment on line 1914 states that exp should be overwritten by the framework, but the assertion only checks that exp is not nil. To properly verify that the framework overwrites the PayloadFunc value (line 1876 sets now.Add(time.Hour * 2).Unix()), the test should assert that exp matches the framework-calculated value (now + Timeout of 1 hour), not the PayloadFunc value (now + 2 hours). Consider adding an assertion like:

expValue, ok := claims["exp"].(float64)
assert.True(t, ok)
// Verify exp is approximately now + 1 hour (Timeout), not now + 2 hours from PayloadFunc
assert.InDelta(t, time.Now().Add(time.Hour).Unix(), int64(expValue), 2.0)

Copilot uses AI. Check for mistakes.
@appleboy appleboy merged commit 54137dd into master Dec 14, 2025
7 checks passed
@appleboy appleboy deleted the payload branch December 14, 2025 04:34
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.

missing system fields?

2 participants