feat: improve logging of authorization errors and allow use customization #2431
Conversation
WalkthroughAdds JWK "use" customization across tests, JWKS server, crypto marshalling, and token-decoder plumbing; propagates AllowedUse through supervisor and config/schema; masks unauthorized error details in GraphQL and websocket flows; and adds tests exercising the new use behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2431 +/- ##
===========================================
+ Coverage 36.95% 60.84% +23.88%
===========================================
Files 834 229 -605
Lines 113800 23839 -89961
Branches 4693 0 -4693
===========================================
- Hits 42052 14504 -27548
+ Misses 70150 8089 -62061
+ Partials 1598 1246 -352
🚀 New features to boost your workflow:
|
alepane21
changed the title
…r-with-valid-token
alepane21
requested review from
Noroth,
StarpTech,
devsergiy,
endigma and
jensneuse
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
router/core/websocket.go (1)
399-410: Consider adding debug logging for the original error.The error sanitization correctly prevents leaking internal error details to clients. However, unlike
handleAuthenticationFailureingraphql_prehandler.go(which logs the original error at debug level on line 1143), this path silently discards the underlying error details. Consider adding a debug log to aid troubleshooting:🔎 Suggested improvement
if h.accessController != nil { handler.request, err = h.accessController.Access(w, r) if err != nil { statusCode := http.StatusForbidden errorMessage := err if errors.Is(err, ErrUnauthorized) { statusCode = http.StatusUnauthorized errorMessage = ErrUnauthorized } + requestLogger.Debug("Failed to authenticate websocket connection from initial payload", zap.Error(err)) http.Error(handler.w, http.StatusText(statusCode), statusCode) _ = handler.writeErrorMessage(requestID, errorMessage) handler.Close(false) return } }router/pkg/authentication/jwks_token_decoder.go (2)
255-265: Consider clarifying the behavior for empty slice vs. nil.The function returns
[UseSig]whenallowedUseisnil, but returns an empty slice whenallowedUseis[]string{}. This distinction might be confusing:
nil→[UseSig](default, restricts to "sig" use)[]string{}(empty slice) →[](empty whitelist, might block all keys)[]string{""}→[USE("")](allows empty "use")While this appears intentional for the use case, consider either:
- Treating empty slice the same as nil (return default), or
- Adding a comment explaining this behavior
💡 Optional: Treat empty slice as default
func getUseWhitelist(allowedUse []string) []jwkset.USE { - if allowedUse == nil { + if len(allowedUse) == 0 { return []jwkset.USE{jwkset.UseSig} } useWhitelist := make([]jwkset.USE, len(allowedUse)) for i, u := range allowedUse { useWhitelist[i] = jwkset.USE(u) } return useWhitelist }
48-48: Add validation for AllowedUse values ingetUseWhitelist().The
AllowedUsestrings are cast directly tojwkset.USEwithout validation. While the jwkset library providesIANARegistered()for validation, the application should validate against the known USE values ("sig" and "enc" per RFC 7517) before passing them downstream. This prevents invalid values from propagating to the jwkset library.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
router-tests/authentication_test.gorouter-tests/cmd/jwks-server/main.gorouter-tests/jwks/crypto.gorouter-tests/jwks/jwks.gorouter/core/access_controller.gorouter/core/graphql_prehandler.gorouter/core/supervisor_instance.gorouter/core/websocket.gorouter/pkg/authentication/jwks_token_decoder.gorouter/pkg/config/config.gorouter/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.json
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router-tests/cmd/jwks-server/main.gorouter/pkg/config/config.schema.jsonrouter/pkg/authentication/jwks_token_decoder.gorouter/core/supervisor_instance.gorouter/pkg/config/testdata/config_full.jsonrouter-tests/jwks/jwks.gorouter/pkg/config/config.go
📚 Learning: 2025-11-19T15:13:57.821Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2273
File: router/core/graphql_handler.go:0-0
Timestamp: 2025-11-19T15:13:57.821Z
Learning: In the Cosmo router (wundergraph/cosmo), error handling follows a two-phase pattern: (1) Prehandler phase handles request parsing, validation, and setup errors using `httpGraphqlError` and `writeOperationError` (in files like graphql_prehandler.go, operation_processor.go, parse_multipart.go, batch.go); (2) Execution phase handles resolver execution errors using `WriteError` in GraphQLHandler.ServeHTTP. Because all `httpGraphqlError` instances are caught in the prehandler before ServeHTTP is invoked, any error type checks for `httpGraphqlError` in the execution-phase WriteError method are unreachable code.
Applied to files:
router/core/graphql_prehandler.go
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.jsonrouter/pkg/config/config.go
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/authentication/jwks_token_decoder.gorouter/core/supervisor_instance.gorouter/pkg/config/testdata/config_full.jsonrouter/pkg/config/config.go
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/authentication/jwks_token_decoder.gorouter/core/supervisor_instance.gorouter/pkg/config/testdata/config_full.jsonrouter/pkg/config/config.go
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: In the Cosmo router codebase, JSON schema validation prevents null values in TrafficShapingRules subgraph configurations, making nil checks unnecessary when dereferencing subgraph rule pointers in NewSubgraphTransportOptions.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
router/pkg/config/config.schema.jsonrouter/pkg/config/testdata/config_full.json
📚 Learning: 2025-09-29T10:28:07.122Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2192
File: router/pkg/config/config.go:1028-1029
Timestamp: 2025-09-29T10:28:07.122Z
Learning: The deprecation strategy for IntrospectionEnabled field in router/pkg/config/config.go is to first mark it as deprecated, then migrate all call sites to use IntrospectionConfig.Enabled, and finally remove the deprecated field. The envDefault tag should be kept during the deprecation period for backward compatibility.
Applied to files:
router/pkg/config/config.go
📚 Learning: 2025-08-28T09:59:19.999Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2167
File: router/core/supervisor_instance.go:196-204
Timestamp: 2025-08-28T09:59:19.999Z
Learning: In router/pkg/config/config.go, the BackoffJitterRetry struct uses envDefault tags to provide default values for Algorithm ("backoff_jitter") and Expression ("IsRetryableStatusCode() || IsConnectionError() || IsTimeout()"), ensuring that even when YAML configs omit these fields, valid defaults are applied automatically, preventing ProcessRetryOptions from rejecting empty values.
Applied to files:
router/pkg/config/config.go
🧬 Code graph analysis (3)
router/core/graphql_prehandler.go (2)
router/core/access_controller.go (1)
ErrUnauthorized(16-16)router/pkg/pubsub/datasource/error.go (1)
Error(3-6)
router/core/websocket.go (1)
router/core/access_controller.go (1)
ErrUnauthorized(16-16)
router-tests/jwks/jwks.go (2)
router-tests/jwks/crypto.go (1)
Crypto(17-23)router-tests/cmd/jwks-server/main.go (1)
NewServerWithCrypto(214-252)
🔇 Additional comments (9)
router-tests/jwks/crypto.go (1)
17-23: LGTM! Clean interface extension.The new
MarshalJWKWithUsemethod is properly added to the interface, and existing implementations correctly delegateMarshalJWK()to the new method with the defaultjwkset.UseSigvalue, maintaining backward compatibility.router-tests/cmd/jwks-server/main.go (1)
52-56: LGTM! Import reorganization only.No functional changes; imports were simply reordered for consistency.
router/core/access_controller.go (1)
51-53: LGTM! Good error composition pattern.Using
errors.Join(err, ErrUnauthorized)preserves the underlying authentication error for debugging and logging while ensuringerrors.Is(err, ErrUnauthorized)still returns true. This enables the sanitization logic ingraphql_prehandler.goandwebsocket.goto mask internal error details from clients.router/pkg/config/testdata/config_full.json (1)
489-542: LGTM! Test data updated to reflect new schema.The
AllowedUsefield is correctly added to each JWKS entry withnullvalues, which will exercise default behavior during testing.router/core/graphql_prehandler.go (1)
1149-1158: LGTM! Proper error sanitization for security.The implementation correctly sanitizes error details in the GraphQL response while preserving the full error for logging (line 1143), request context (line 1142), and observability spans (lines 1146-1147). This prevents leaking sensitive authentication failure details to clients.
router/pkg/config/config.go (1)
486-501: LGTM! AllowedUse field added correctly.The
AllowedUse []stringfield is properly added toJWKSConfiguration. Based on learnings, validation for JWKS configuration fields is handled at the JSON schema level rather than runtime validation in Go code.router/core/supervisor_instance.go (1)
290-309: LGTM! Configuration propagation is correct.The
AllowedUsefield is properly propagated from the JWKS configuration to theauthentication.JWKSConfig, consistent with how other JWKS settings are passed through.router-tests/authentication_test.go (1)
4019-4134: LGTM! Well-structured test for the new "use" customization feature.The test properly validates both the default behavior (rejecting tokens when JWK has empty "use" field) and the customized behavior (allowing empty "use" when explicitly configured). The helper functions follow existing patterns and make the test readable.
router-tests/jwks/jwks.go (1)
136-215: LGTM! Clean refactoring to options-based initialization.The introduction of the options pattern is idiomatic and maintains backward compatibility. Key improvements:
- Backward compatibility:
NewServerWithCryptostill works via delegation- Flexibility: Options allow customizing "use" parameter without breaking existing code
- Good defaults: Default
useisUseSig, preserving previous behavior- Validation: Checks that at least one provider is specified
The refactoring properly supports the new JWKS "use" customization feature while keeping the API clean.
…401-on-router-with-valid-token' into ale/eng-8704-jwt-authentication-401-on-router-with-valid-token
…r-with-valid-token
…r-with-valid-token
…pdate usage in NewJwksTokenDecoder
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router/pkg/authentication/jwks_token_decoder.go (1)
156-160: Add USE validation for secret-based JWKS to respectAllowedUseconfiguration.Line 159 hardcodes
jwkset.UseSigin the JWK metadata for secret-based JWKS, but the configuredAllowedUseis never validated during token verification. Unlike HTTP-based JWKS (which uses keyfunc'sUseWhitelist), secret-based JWKS storesallowedUsein thekeyFuncEntrybut the customkeyFuncWrapper(lines 194-240) never checks it against the JWK's declared USE. This means a user configuringAllowedUseto exclude "sig" will have no effect for secret-based keys.Either:
- Validate
entry.allowedUsein thekeyFuncWrappersimilarly to howentry.allowedAlgorithmsis validated (lines 212-229), or- Document that secret-based JWKS always uses "sig" and users must include "sig" in
AllowedUse.
🤖 Fix all issues with AI agents
In @router/pkg/authentication/jwks_token_decoder.go:
- Around line 186-190: The secret-based branch building keyFuncEntry is missing
the allowedAlgorithms field, causing algorithm validation to skip for those
entries; update the append call that creates keyFuncEntry (the one passing jwks,
aud: audiencesMap[key], allowedUse: c.AllowedUse) to also set allowedAlgorithms:
c.AllowedAlgorithms (or the appropriate source used by the URL-based path) so
keyFuncWrapper can correctly validate entry.allowedAlgorithms during token
verification.
- Around line 255-265: toJwksetUseType currently casts strings to jwkset.USE
without validation; change it to validate each allowedUse entry (e.g., for each
u do use := jwkset.USE(u) and call use.IANARegistered() or equivalent) and
return an error if any value is invalid so initialization fails fast; update the
signature to return ([]jwkset.USE, error) and propagate that error to the
decoder initialization call sites so the decoder fails on bad configuration
rather than during token verification.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
router/pkg/authentication/jwks_token_decoder.go
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2026-01-06T12:37:21.521Z
Learnt from: asoorm
Repo: wundergraph/cosmo PR: 2379
File: router/pkg/connectrpc/operation_registry_test.go:381-399
Timestamp: 2026-01-06T12:37:21.521Z
Learning: In Go code (Go 1.25+), prefer using sync.WaitGroup.Go(func()) to run a function in a new goroutine, letting the WaitGroup manage Add/Done automatically. Avoid manual wg.Add(1) followed by go func() { defer wg.Done(); ... }() patterns. Apply this guidance across all Go files in the wundergraph/cosmo repository where concurrency is used.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
⏰ 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). (11)
- GitHub Check: build-router
- GitHub Check: Analyze (go)
- GitHub Check: integration_test (./events)
- GitHub Check: image_scan
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
🔇 Additional comments (3)
router/pkg/authentication/jwks_token_decoder.go (3)
48-48: LGTM: AllowedUse field added to configuration.The new field follows the existing pattern and enables customization of JWK "use" values as intended.
267-284: LGTM: UseWhitelist parameter properly propagated.The signature change and implementation correctly pass the
useWhitelistto thekeyfunc.Options, enabling JWK "use" filtering at the library level.
77-77: Remove the unusedallowedUsefield fromkeyFuncEntry.The
allowedUsefield is assigned in the struct (lines 135, 189) but never accessed. The actual use case is handled at initialization time (lines 127, 182) wheretoJwksetUseType(c.AllowedUse)converts the config value and passes it directly tocreateKeyFunc. The stored field inkeyFuncEntryis dead code and should be removed.⛔ Skipped due to learnings
Learnt from: SkArchon Repo: wundergraph/cosmo PR: 2224 File: router/pkg/authentication/jwks_token_decoder.go:176-199 Timestamp: 2025-09-17T18:13:31.021Z Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.
…sTokenDecoder to use improved function
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
router/pkg/authentication/jwks_token_decoder.go (2)
164-164: Potential mismatch between hardcoded USE and AllowedUse filter.Line 164 hardcodes
USE: jwkset.UseSigwhen creating a JWK from a secret, but lines 187-190 allow users to configure customAllowedUsevalues. If a user setsAllowedUseto anything other than["sig"](e.g.,["enc"]), theUseWhitelistfilter will reject the created JWK since its metadata hasuse: "sig".This creates a configuration that can never succeed for secret-based JWKs with non-sig use values.
Consider one of these approaches:
- Use the first value from
AllowedUseto set the JWK's USE metadata (with validation that only one value is provided for secret-based configs)- Disallow
AllowedUsecustomization for secret-based JWKs and document this limitation- Remove the
UseWhitelistfiltering for secret-based paths since the USE is internally controlledAlso applies to: 187-201
282-298: Remove UseWhitelist field assignment from keyfunc.Options — field does not exist in v3.6.2.Line 291 attempts to assign
useWhitelisttokeyfuncOptions.UseWhitelist, but thekeyfunc.Optionsstruct does not have aUseWhitelistfield in v3.6.2. The JWK "use" whitelist behavior in keyfunc v3.6.2 is controlled viaJWKUseandJWKUseNoWhitelistconstants instead. Either remove theuseWhitelistparameter and assignment, or update the implementation to use the correct keyfunc API for configuring JWK "use" whitelist behavior.
🧹 Nitpick comments (1)
router/pkg/authentication/jwks_token_decoder.go (1)
77-77: Consider whether this field is needed.The
allowedUsefield is stored in eachkeyFuncEntry(lines 140, 200) but never referenced in the validation logic (lines 205-251). If it's intended for future use or debugging, consider adding a comment to clarify its purpose.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
router/pkg/authentication/jwks_token_decoder.go
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2025-09-17T18:13:31.021Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
📚 Learning: 2026-01-06T12:37:21.521Z
Learnt from: asoorm
Repo: wundergraph/cosmo PR: 2379
File: router/pkg/connectrpc/operation_registry_test.go:381-399
Timestamp: 2026-01-06T12:37:21.521Z
Learning: In Go code (Go 1.25+), prefer using sync.WaitGroup.Go(func()) to run a function in a new goroutine, letting the WaitGroup manage Add/Done automatically. Avoid manual wg.Add(1) followed by go func() { defer wg.Done(); ... }() patterns. Apply this guidance across all Go files in the wundergraph/cosmo repository where concurrency is used.
Applied to files:
router/pkg/authentication/jwks_token_decoder.go
⏰ 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). (11)
- GitHub Check: build-router
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: build_push_image (nonroot)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
router/pkg/authentication/jwks_token_decoder.go (3)
48-48: LGTM: Field addition follows project patterns.The
AllowedUsefield is appropriately typed and positioned. Consistent with project learnings, field validation is handled at the JSON schema level rather than runtime Go validation.
127-141: LGTM: Proper error handling and validation flow.The URL-based configuration path correctly:
- Validates
AllowedUsevalues viatoJwksetUseTypewith descriptive error context- Propagates the
useWhitelistto key function creation- Stores the configuration for reference
266-280: LGTM: Proper validation with backward-compatible defaults.The function correctly:
- Defaults to
UseSigwhenAllowedUseis empty, preserving backward compatibility- Validates each use value via
IANARegistered()to prevent invalid configurations- Provides clear error messages for unsupported values
…r-with-valid-token
…r-with-valid-token
alepane21
deleted the
ale/eng-8704-jwt-authentication-401-on-router-with-valid-token
branch
By default, we force to check that the jwks keys have the use param set to "sig". By standard the value is optional and it could also be left empty.
With this PR I introduce two changes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist
allowed_useparameter to JWKs configuration in router doc… cosmo-docs#223.