Skip to content

feat: apply defaults for customClaims #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

xmlking
Copy link
Contributor

@xmlking xmlking commented Jul 10, 2025

Before submitting this PR:

Checklist

  • No breaking changes
  • Tests pass
  • New features have new tests
  • Documentation is updated

Breaking changes

Avoid breaking changes and regressions. If you feel it is unavoidable, make it explicit in your PR comment so we can review it and see how to handle it.

Tests

  • please make sure your changes pass the current tests (Use the make test or the make watch command).
  • if you are introducing a new feature, please write as much tests as possible.

Documentation

Please make sure the documentation is updated accordingly, in particular:

@xmlking
Copy link
Contributor Author

xmlking commented Jul 10, 2025

Recreated this original PR
#616

Copy link

@Copilot 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 adds support for default values when extracting custom JWT claims, allowing missing or nil claims to fall back to user-provided defaults.

  • Extend the Config and CLI to accept a new CustomClaimsDefaults parameter.
  • Update NewCustomClaims and ExtractClaims to store and apply default values.
  • Add tests and documentation for the new defaults behavior.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
go/controller/validator_test.go Initialized CustomClaimsDefaults in the test config
go/controller/custom_claims_test.go Added test cases covering defaults for custom claims
go/controller/custom_claims.go Added defaults field and defaultOrNil logic to CustomClaims and ExtractClaims
go/controller/config.go Exposed CustomClaimsDefaults in the Config struct
go/controller/change_env.go Unmarshal CustomClaimsDefaults and pass to NewCustomClaims
go/cmd/serve.go Added --custom-claims-defaults CLI flag
go/cmd/jwt_getter.go Parsed custom-claims-defaults flag and forwarded defaults
go/cmd/config.go Loaded CustomClaimsDefaults from CLI context
docs/environment-variables.md Documented AUTH_JWT_CUSTOM_CLAIMS_DEFAULTS env var

@@ -36,14 +36,23 @@ func (ctrl *Controller) PostChangeEnv(c *gin.Context) { //nolint:funlen,cyclop
} else {
var rawClaims map[string]string
if err := json.Unmarshal([]byte(ctrl.config.CustomClaims), &rawClaims); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarhsal custom claims", "error": err.Error()})
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims", "error": err.Error()})
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

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

After writing the error response with c.JSON, the handler should return immediately to prevent further processing.

Suggested change
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims", "error": err.Error()})
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims", "error": err.Error()})
return

Copilot uses AI. Check for mistakes.

defaults = nil
} else {
if err := json.Unmarshal([]byte(ctrl.config.CustomClaimsDefaults), &defaults); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims defaults", "error": err.Error()})
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

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

After writing the error response with c.JSON for defaults unmarshal, the handler should return to avoid continuing with an invalid state.

Suggested change
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims defaults", "error": err.Error()})
c.JSON(http.StatusBadRequest, gin.H{"message": "failed to unmarshal custom claims defaults", "error": err.Error()})
return

Copilot uses AI. Check for mistakes.

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.

2 participants