Fix validation logic for ID token and update Key Manager configuration#413
Fix validation logic for ID token and update Key Manager configuration#413
Conversation
📝 WalkthroughWalkthroughTwo configuration updates affecting ID token validation logic and OAuth2/OpenID endpoint configuration. The ID token validation flag logic is inverted to enable only when explicitly set to 'true', and keyManager OAuth2 endpoints are updated to point to Thunder platform with new issuer, audience, and JWKS URL values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployments/helm-charts/wso2-agent-manager/values.yaml`:
- Around line 112-114: The jwksUrl is using a localhost-based hostname which
won’t resolve in-cluster; update the jwksUrl value to use the cluster DNS name
consistent with values-cp.yaml/values-op.yaml and the oidc.tokenUrl pattern in
this file (so it points at the Thunder service FQDN reachable from pods), e.g.,
replace "http://thunder.amp.localhost:8080/oauth2/jwks" with the in-cluster URL
following the same host/port/path convention used by oidc.tokenUrl; ensure the
key name jwksUrl remains unchanged.
| issuer: "thunder" | ||
| audience: "amp-console-client" | ||
| jwksUrl: "http://thunder.amp.localhost:8080/oauth2/jwks" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Thunder-related service definitions in YAML files
rg -n "thunder" --type yaml -i -A 5 -B 2Repository: wso2/agent-manager
Length of output: 50374
🏁 Script executed:
# Check if there's CoreDNS or DNS-related configuration
rg -n "dns|coredns|localhost" --type yaml | head -50Repository: wso2/agent-manager
Length of output: 3940
🏁 Script executed:
# Look for any documentation or comments about local development setup
rg -n "localhost|local.*dev|development" --type yaml -A 2 -B 2 | grep -i "thunder\|jwks\|issuer" -A 5 -B 5Repository: wso2/agent-manager
Length of output: 8064
🏁 Script executed:
# Check the full context of the values.yaml file around the changed lines
wc -l deployments/helm-charts/wso2-agent-manager/values.yamlRepository: wso2/agent-manager
Length of output: 120
jwksUrl must use in-cluster Kubernetes DNS, not a localhost-based URL
The agent-manager pod resolves DNS through CoreDNS, where thunder.amp.localhost will not resolve to the Thunder service. This will cause JWKS fetch failures and break JWT validation.
Correct this to match the pattern already used in values-cp.yaml and values-op.yaml, and align with oidc.tokenUrl in the same file (line 122):
- jwksUrl: "http://thunder.amp.localhost:8080/oauth2/jwks"
+ jwksUrl: "http://amp-thunder-extension-service.amp-thunder.svc.cluster.local:8090/oauth2/jwks"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| issuer: "thunder" | |
| audience: "amp-console-client" | |
| jwksUrl: "http://thunder.amp.localhost:8080/oauth2/jwks" | |
| issuer: "thunder" | |
| audience: "amp-console-client" | |
| jwksUrl: "http://amp-thunder-extension-service.amp-thunder.svc.cluster.local:8090/oauth2/jwks" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deployments/helm-charts/wso2-agent-manager/values.yaml` around lines 112 -
114, The jwksUrl is using a localhost-based hostname which won’t resolve
in-cluster; update the jwksUrl value to use the cluster DNS name consistent with
values-cp.yaml/values-op.yaml and the oidc.tokenUrl pattern in this file (so it
points at the Thunder service FQDN reachable from pods), e.g., replace
"http://thunder.amp.localhost:8080/oauth2/jwks" with the in-cluster URL
following the same host/port/path convention used by oidc.tokenUrl; ensure the
key name jwksUrl remains unchanged.
2 participants
Purpose
This pull request updates the authentication and token validation configuration to ensure stricter compliance with OIDC standards and to align with the actual deployment environment. The most important changes are grouped below:
Authentication and Token Validation:
config.template.jsby ensuringvalidateIDTokenis set totrueonly when theVALIDATE_ID_TOKENenvironment variable is'true', making the validation behavior correct and predictable.Agent Manager Key Manager Configuration:
keyManagerconfiguration invalues.yamlto use the correctissuer("thunder"),audience("amp-console-client"), and provided the appropriatejwksUrlfor JWT validation, aligning the Helm chart configuration with the actual authentication provider setup.Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores