Switch to using token authentication #66
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omertuc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe authentication configuration in both Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
lightspeed-stack.template.yaml (1)
21-23: Hard-coding the JWK URL hurts flexibility & non-prod deployments
Consider following the template.yaml approach and let callers inject${SSO_BASE_URL}(or similar) instead of baking the Red Hat production URL into the image-level config.- url: https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/certs + # Keep prod as the default but allow override via env / Helm / Kustomize + url: ${SSO_BASE_URL:-https://sso.redhat.com}/auth/realms/redhat-external/protocol/openid-connect/certsThis preserves current behaviour while allowing staging / offline environments to substitute their own IdP.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lightspeed-stack(1 hunks)lightspeed-stack.template.yaml(1 hunks)template.yaml(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). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
🔇 Additional comments (2)
lightspeed-stack (1)
1-1: Submodule commit validated; confirm CI orchestration & integration coverage✅ Verified that commit
e534d930b58fadd8d9d89ebc0f6c070cc665ef82exists on https://github.com/lightspeed-core/lightspeed-stack.gitNext steps—please ensure:
- Every pipeline stage that builds or tests this repo invokes
git submodule update --init --recursive
so the new SHA is checked out.- Any breaking changes from lightspeed-stack PR #243 (JWK auth) are exercised by integration tests in this repo.
lightspeed-stack.template.yaml (1)
24-26: Verify claim names match Red Hat SSO tokens
You’ve overridden the defaults (sub→user_id,preferred_username→username) in lightspeed-stack.template.yaml (lines 24–26). Unless your SSO realm emits those exact claims, every request will be unauthenticated.Run this check with a real RH SSO access token:
TOKEN=<your_RHSso_access_token> payload=$(echo "$TOKEN" | cut -d'.' -f2 | base64 -d 2>/dev/null) echo "$payload" | jq '{sub, preferred_username, user_id, username}'• If
user_id/usernamearen’t present, remove the override or use the actual claim names.
• Confirm before merging to prevent auth failures.
| module: jwk-token | ||
| jwk_config: | ||
| url: ${SSO_BASE_URL}/protocol/openid-connect/certs | ||
| jwt_configuration: | ||
| user_id_claim: user_id | ||
| username_claim: username |
There was a problem hiding this comment.
Missing SSO_BASE_URL parameter breaks oc process / helm template
${SSO_BASE_URL} is referenced here but no matching entry exists under the parameters: list (lines 2-66). Processing this template will fail with “unresolved parameter”.
Add the parameter with a sensible default:
- name: SSO_BASE_URL
+ description: "Base URL of the SSO realm that issues JWTs"
+ name: SSO_BASE_URL
+ value: "https://sso.redhat.com/auth/realms/redhat-external"Place it near the other service-level parameters for consistency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In template.yaml around lines 119 to 124, the parameter SSO_BASE_URL is used but
not defined in the parameters section (lines 2-66), causing template processing
errors. Add a new parameter entry for SSO_BASE_URL with a sensible default value
near the other service-level parameters to ensure it is resolved correctly
during oc process or helm template commands.
|
Draft until quay.io comes back up and lightspeed-stack pushes a dev image ( |
Update lightspeed-stack to the latest version which includes this PR: lightspeed-core/lightspeed-stack#243 Modify our configuration to use the new jwk-token authentication module, with the JWK URL pointing to the Red Hat SSO server and using the user ID / username fields that can be found in a typical JWT issued by Red Hat SSO.
| module: "noop-with-token" | ||
| module: jwk-token | ||
| jwk_config: | ||
| url: ${SSO_BASE_URL}/protocol/openid-connect/certs |
There was a problem hiding this comment.
It's fine, this is how Assisted does it as well
|
/lgtm |
2 participants
Update lightspeed-stack to the latest version which includes this PR:
lightspeed-core/lightspeed-stack#243
Modify our configuration to use the new jwk-token authentication module, with the JWK URL pointing to the Red Hat SSO server and using the user ID / username fields that can be found in a typical JWT issued by Red Hat SSO.
Summary by CodeRabbit
New Features
Chores