[WIP] Add support for Backstage/RHDH MCP DCR #219
Open
maysunfaisal wants to merge 2 commits into
Open
Conversation
maysunfaisal
requested review from
JslYoon,
gabemontero,
johnmcollier,
michael-valdron and
thepetk
as code owners
June 10, 2026 21:31
Code Review by Qodo
1.
|
PR Summary by QodoEnable DCR auth for Lightspeed MCP servers (RHDH/Backstage Helm) WalkthroughsDescription• Switch Lightspeed frontend/backend to custom OCI builds with DCR support • Enable Backstage experimental Dynamic Client Registration and OAuth2 consent route • Turn on RBAC permissions and mount policy CSV via ConfigMap Diagramgraph TD
H["Helm values.yaml"] --> B["Backstage (RHDH) pod"] --> A["Auth plugin (/oauth2)"] --> I{{"OIDC provider"}}
B --> L["Custom Lightspeed plugins"] --> M{{"MCP server"}}
C[("ConfigMap: rbac-policy.csv")] --> B
subgraph Legend
direction LR
_cfg["Config"] ~~~ _svc["Service/Pod"] ~~~ _cm[("ConfigMap")] ~~~ _ext{{"External"}}
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Wait for upstream Lightspeed DCR images/overlays
2. Constrain DCR redirect URI patterns (instead of '*')
3. Externalize RBAC policy management (OPA/GitOps-managed policies)
Recommendation: The PR’s approach is appropriate for enabling DCR quickly (custom plugin builds + Helm wiring), but plan to switch back to upstream plugin overlays once the referenced upstream work lands. Also strongly consider tightening File ChangesOther (3)
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+424
to
+427
| experimentalDynamicClientRegistration: | ||
| enabled: true | ||
| allowedRedirectUriPatterns: | ||
| - '*' |
There was a problem hiding this comment.
3. Wildcard redirect uris 🐞 Bug ⛨ Security
values.yaml enables experimentalDynamicClientRegistration while allowing allowedRedirectUriPatterns: ['*'], which permits redirect URIs to match anything. This undermines OAuth redirect URI validation and can allow authorization code/token exfiltration to attacker-controlled redirect endpoints.
Agent Prompt
## Issue description
`auth.experimentalDynamicClientRegistration.allowedRedirectUriPatterns` is set to `'*'`, effectively allowing any redirect URI for dynamically registered clients.
## Issue Context
OAuth redirect URI validation is a key security control; wildcard patterns make it possible for a malicious client registration to supply a redirect URI controlled by an attacker.
## Fix Focus Areas
- charts/rhdh/values.yaml[422-427]
## Suggested fix
Replace the wildcard with a narrowly-scoped allowlist (e.g., specific origins/paths you control such as `${RHDH_BASE_URL}/oauth2/*` or exact callback URLs). If multiple environments are needed, make this configurable via chart values with safe defaults (no wildcard).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+575
to
+581
| permission: | ||
| enabled: true | ||
| rbac: | ||
| policies-csv-file: /opt/app-root/src/rbac-policy.csv | ||
| admin: | ||
| users: | ||
| - name: user:default/mfaisal |
There was a problem hiding this comment.
4. Hardcoded rbac admin user 🐞 Bug ⛨ Security
The chart hardcodes user:default/mfaisal as a permission RBAC admin and also binds that same user to role:default/mcp-admin in the shipped rbac-policy.csv. Any environment where that identity exists will grant elevated permissions unintentionally via default chart install.
Agent Prompt
## Issue description
A specific human identity (`user:default/mfaisal`) is embedded in the chart defaults as an RBAC admin and is also granted the `mcp-admin` role in the policy CSV ConfigMap.
## Issue Context
Charts are often reused across environments; hardcoding a privileged user can unintentionally grant admin access in clusters where that identity exists.
## Fix Focus Areas
- charts/rhdh/values.yaml[575-581]
- charts/rhdh/templates/rbac-policy-configmap.yaml[1-23]
## Suggested fix
- Remove the hardcoded user from defaults.
- Parameterize admin users and role bindings via values (e.g., `.Values.backstage.upstream.backstage.appConfig.permission.rbac.admin.users` and/or a chart value like `.Values.permission.rbacPolicyCsv`), with an empty default.
- If a demo user is needed, gate it behind an explicit `demo: true`/`unsafeDefaults: true` value so production installs don’t inherit it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
gabemontero
reviewed
Jun 11, 2026
| data: | ||
| rbac-policy.csv: | | ||
| p, role:default/mcp-admin, catalog.entity.read, read, allow | ||
| p, role:default/mcp-admin, catalog.entity.create, create, allow |
Contributor
There was a problem hiding this comment.
it is going to be very cool to get this level of authorization granularity once the DCR stuff lands @maysunfaisal @johnmcollier :-)
maysunfaisal
force-pushed
the
feat/dcr-mcp-auth-v2
branch
3 times, most recently
from
June 16, 2026 23:13
4b5a589 to
90948b7
Compare
- Add custom Lightspeed OCI images with DCR support (dcr-0.4.0) - Configure MCP server with auth: dcr in values.yaml - Enable experimentalDynamicClientRegistration - Add @backstage/plugin-auth for OAuth2 consent page Co-authored-by: Cursor <cursoragent@cursor.com>
maysunfaisal
force-pushed
the
feat/dcr-mcp-auth-v2
branch
from
June 25, 2026 21:28
90948b7 to
5e75e62
Compare
- Replace custom DCR OCI builds with official bs_1.49.4__2.8.6 images - Remove disabled upstream lightspeed entries (no longer needed) - Rename app-config key from lightspeed to intelligent-assistant - Revert setup-secrets.sh to development (remove OPENAI env vars) Co-authored-by: Cursor <cursoragent@cursor.com>
maysunfaisal
force-pushed
the
feat/dcr-mcp-auth-v2
branch
from
July 13, 2026 20:17
ee444df to
28dc6c3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
I'll clean up this PR/branch as things progress on redhat-developer/rhdh-plugins#3347
Which issue(s) does this PR fix
https://redhat.atlassian.net/browse/RHIDP-11657
How to test changes / Special notes to the reviewer