-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add configurable JWT signature algorithm support #36
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
base: main
Are you sure you want to change the base?
Conversation
Add support for JWT_SIGNATURE_ALGORITHM environment variable to allow operators to configure the JWT signature algorithm for token validation. Fixes: LFXV2-914 Changes: - Add parseSignatureAlgorithm() function to validate and map algorithm strings - Extend JWTAuthConfig struct with SignatureAlgorithm field - Update NewJWTAuth() to use configured algorithm with logging for non-defaults - Support 9 algorithms: PS256/384/512, RS256/384/512, ES256/384/512 - Add comprehensive test coverage (18 test cases) for algorithm validation - Update Helm chart (v0.5.3 -> v0.5.4) with jwtSignatureAlgorithm configuration - Add JWT_SIGNATURE_ALGORITHM to deployment template - Update documentation (CLAUDE.md, .env.example) Default behavior: - PS256 remains the default algorithm when env var is not set - Backward compatible with existing deployments - Case-sensitive algorithm names (uppercase required) - Fails fast at startup for invalid algorithm configuration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Jordan Evans <[email protected]>
WalkthroughAdds configurable JWT signature algorithm support: a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this 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 configurable JWT signature algorithm support to the project service, allowing operators to specify which JWT signature algorithm to use for token validation via the JWT_SIGNATURE_ALGORITHM environment variable. The implementation maintains backward compatibility by defaulting to PS256 when the variable is not set.
Key changes:
- Adds
parseSignatureAlgorithm()function with validation for 9 supported algorithms (PS256/384/512, RS256/384/512, ES256/384/512) - Extends JWT authentication configuration and initialization to use the selected algorithm
- Includes comprehensive test coverage with 18 test cases covering valid algorithms, edge cases, and error conditions
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/infrastructure/auth/jwt.go |
Core implementation: adds parseSignatureAlgorithm() function, extends JWTAuthConfig struct with SignatureAlgorithm field, updates NewJWTAuth() to parse and use configured algorithm |
internal/infrastructure/auth/jwt_test.go |
Test coverage: updates constant test to use renamed defaultSignatureAlgorithm, adds 4 test cases to TestJWTAuth_ConfigurationHandling, adds new TestParseSignatureAlgorithm with 18 test cases |
cmd/project-api/main.go |
Integration: adds JWT_SIGNATURE_ALGORITHM environment variable to JWTAuthConfig initialization |
charts/lfx-v2-project-service/values.yaml |
Helm configuration: adds jwtSignatureAlgorithm with default value "PS256" and documentation of supported algorithms |
charts/lfx-v2-project-service/templates/deployment.yaml |
Deployment template: passes jwtSignatureAlgorithm value as JWT_SIGNATURE_ALGORITHM environment variable |
charts/lfx-v2-project-service/Chart.yaml |
Version bump: increments chart version from 0.5.3 to 0.5.4 |
CLAUDE.md |
Documentation: adds JWT_SIGNATURE_ALGORITHM to environment variables table with PS256 default |
.env.example |
Example configuration: adds JWT_SIGNATURE_ALGORITHM with list of supported algorithms |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/infrastructure/auth/jwt.go (1)
99-111: LGTM! Fail-fast validation implemented correctly.The upfront parsing and validation of the signature algorithm ensures the service fails fast at startup if misconfigured, meeting the PR objectives. The logging appropriately notifies operators when a non-default algorithm is selected.
Optional: Consider a constant for the default algorithm string.
The hardcoded
"PS256"string at line 107 could be extracted to a constant likedefaultSignatureAlgorithmString = "PS256"for consistency, though the current implementation is clear and maintainable.const ( defaultSignatureAlgorithmString = "PS256" defaultSignatureAlgorithm = validator.PS256 // ... ) // Later in NewJWTAuth: if config.SignatureAlgorithm != "" && config.SignatureAlgorithm != defaultSignatureAlgorithmString { // ... }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.env.example(1 hunks)CLAUDE.md(1 hunks)charts/lfx-v2-project-service/Chart.yaml(1 hunks)charts/lfx-v2-project-service/templates/deployment.yaml(1 hunks)charts/lfx-v2-project-service/values.yaml(1 hunks)cmd/project-api/main.go(1 hunks)internal/infrastructure/auth/jwt.go(4 hunks)internal/infrastructure/auth/jwt_test.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CLAUDE.md
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain CLAUDE.md for AI assistant instructions and technical details
Files:
CLAUDE.md
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure code formatting and linting pass (make fmt, make lint, make check)
Files:
internal/infrastructure/auth/jwt.gocmd/project-api/main.gointernal/infrastructure/auth/jwt_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Place unit tests alongside implementation with the same filename and *_test.go suffix
Each production function must have exactly one corresponding TestFunction test (table with multiple cases allowed)
Use table-driven tests for coverage (subtests over a single TestFunction)
Mock all external dependencies (e.g., repositories, message builders) in unit tests
Focus unit tests on exported package functions
Files:
internal/infrastructure/auth/jwt_test.go
🧬 Code graph analysis (1)
internal/infrastructure/auth/jwt_test.go (1)
internal/infrastructure/auth/jwt.go (1)
JWTAuthConfig(56-65)
🪛 GitHub Actions: Project API Build
cmd/project-api/main.go
[error] 84-84: ./main.go:84:50: too many arguments in call to generator.Generate
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
- GitHub Check: MegaLinter
🔇 Additional comments (13)
CLAUDE.md (1)
219-219: LGTM! Environment variable documented correctly.The addition of
JWT_SIGNATURE_ALGORITHMto the environment variables table is clear and consistent with the other entries.charts/lfx-v2-project-service/Chart.yaml (1)
8-8: LGTM! Appropriate version bump.The patch version increment from 0.5.3 to 0.5.4 correctly reflects the addition of new configuration options.
.env.example (1)
22-24: LGTM! Clear documentation of supported algorithms.The comment comprehensively lists all supported JWT signature algorithms, making it easy for operators to configure the appropriate value.
charts/lfx-v2-project-service/templates/deployment.yaml (1)
40-41: LGTM! Environment variable properly configured.The JWT_SIGNATURE_ALGORITHM environment variable is correctly wired to the Helm values and follows the established pattern for other environment variables.
charts/lfx-v2-project-service/values.yaml (1)
50-53: LGTM! Comprehensive configuration documentation.The
jwtSignatureAlgorithmconfiguration is well-documented, including the list of supported algorithms and the important note about case sensitivity.internal/infrastructure/auth/jwt_test.go (3)
219-219: LGTM! Constant reference updated correctly.The test correctly references the renamed constant
defaultSignatureAlgorithm.
310-342: LGTM! Comprehensive algorithm validation test coverage.The new test cases effectively validate:
- Multiple algorithm families (ES256, RS256)
- Invalid algorithm rejection
- Case sensitivity enforcement (uppercase required)
This ensures the configuration handling properly rejects misconfigured algorithms at startup as intended by the fail-fast design.
362-412: Excellent! Thorough test coverage for algorithm parsing.The
TestParseSignatureAlgorithmfunction provides comprehensive coverage:
- All 9 supported algorithms across PS/RS/ES families
- Default behavior for empty string
- Case sensitivity validation
- Rejection of unsupported algorithms (HMAC family, unknown values)
This follows the coding guideline of having one test function per production function with table-driven test cases.
internal/infrastructure/auth/jwt.go (4)
21-26: LGTM! Constants properly renamed and documented.The rename from
signatureAlgorithmtodefaultSignatureAlgorithmbetter conveys its purpose, and the constants are well-documented.
28-53: LGTM! Well-implemented algorithm parser.The
parseSignatureAlgorithmfunction is well-designed:
- Sensible default (PS256) for empty input
- Map-based lookup is maintainable and efficient
- Comprehensive error message listing all supported algorithms
- Case-sensitive validation enforced as documented
63-64: LGTM! Configuration field properly added.The
SignatureAlgorithmfield addition toJWTAuthConfigis correctly placed and documented.
141-141: LGTM! Correctly uses the parsed algorithm.The JWT validator now uses the dynamically parsed algorithm instead of the hardcoded constant, enabling the configurable behavior.
cmd/project-api/main.go (1)
54-54: LGTM! Configuration field properly populated.The
SignatureAlgorithmfield is correctly populated from theJWT_SIGNATURE_ALGORITHMenvironment variable, following the same pattern as other configuration fields (JWKSURL,Audience,MockLocalPrincipal).Note on pipeline failure: The reported pipeline error at line 84 (
too many arguments in call to generator.Generate) does not appear to be related to the changes in this file. Line 84 contains<-done, which is a channel receive operation, and there are nogenerator.Generatecalls anywhere in the codebase. This appears to be a stale error or from a different context.
Signed-off-by: Jordan Evans <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Makefile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: MegaLinter
- GitHub Check: Build and Test
Add support for JWT_SIGNATURE_ALGORITHM environment variable to allow operators to configure the JWT signature algorithm for token validation.
Fixes: LFXV2-914
Changes:
Default behavior:
🤖 Generated with Claude Code