Skip to content

enable oauth2 authorization for backsage-cli#223

Closed
yangcao77 wants to merge 1 commit into
redhat-ai-dev:mainfrom
yangcao77:dynamic-auth
Closed

enable oauth2 authorization for backsage-cli#223
yangcao77 wants to merge 1 commit into
redhat-ai-dev:mainfrom
yangcao77:dynamic-auth

Conversation

@yangcao77

Copy link
Copy Markdown

What does this PR do?

enable oauth2 authorization for backsage-cli

Which issue(s) does this PR fix

related to https://redhat.atlassian.net/browse/RHIDP-14122

the oauth2 authorization is a requirement for using backsage-cli

How to test changes / Special notes to the reviewer

add the changes accordingly, and run:

alias backstage-cli='NPM_CONFIG_LEGACY_PEER_DEPS=true npx -y @backstage/cli

backstage-cli auth login --backend-url <rhdh-backend-url>

should see the blow authorization page

Screenshot 2026-06-16 at 4 07 49 PM

Signed-off-by: Stephanie <yangcao@redhat.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Enable OAuth2 auth route and Backstage auth plugin for backstage-cli
⚙️ Configuration changes ✨ Enhancement 🕐 10-20 Minutes

Grey Divider

Description

• Enable Backstage auth frontend route for /oauth2/* via dynamic plugin configuration.
• Turn on experimental OIDC client metadata docs and refresh-token support.
• Allow backstage-cli auth login to complete OAuth2 authorization flow against RH Developer Hub.
Diagram

graph TD
  A["Helm values.yaml"] --> B["Dynamic plugin: auth"] --> C["Frontend route /oauth2/*"] --> D["Backstage auth UI"] --> E["OAuth2/OIDC provider"]
  A --> F["Backend auth settings"] --> D
  subgraph Legend
    direction LR
    _cfg["Config"] ~~~ _ui["UI/Route"] ~~~ _ext{{"External IdP"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Expose OAuth2 route via existing core app routing (no extra plugin)
  • ➕ Fewer moving parts and fewer dynamic plugin dependencies
  • ➕ Less risk of route/plugin version skew
  • ➖ May not be feasible if RH Developer Hub requires the plugin overlay for /oauth2/*
  • ➖ Could require deeper upstream app customization than a values-only change
2. Gate experimental auth flags behind an explicit chart value
  • ➕ Makes security/behavior changes more intentional per deployment
  • ➕ Easier to roll back or selectively enable in different environments
  • ➖ Adds extra chart surface area and documentation needs
  • ➖ Slightly more complex configuration for users

Recommendation: The values-only approach (adding the auth dynamic plugin and enabling the needed experimental OIDC/refresh-token support) is the lowest-effort way to unblock backstage-cli OAuth2 login without code changes. If these experimental flags have security or compatibility implications, consider following up by making them explicitly opt-in via dedicated chart values and documenting the expected provider requirements.

Files changed (1) +15 / -0

Other (1) +15 / -0
values.yamlEnable auth dynamic plugin route and experimental OIDC/refresh token support +15/-0

Enable auth dynamic plugin route and experimental OIDC/refresh token support

• Adds the Backstage auth dynamic plugin overlay and registers a frontend dynamic route for /oauth2/*. Enables experimental OIDC client metadata documents and refresh token support in the Backstage auth configuration.

charts/rhdh/values.yaml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Tickets: RHIDP-14122
✅ Compliance rules (platform): 4 rules

Grey Divider


Remediation recommended

1. Experimental auth enabled by default 🐞 Bug ☼ Reliability
Description
The chart default config enables auth.experimentalClientIdMetadataDocuments and
auth.experimentalRefreshToken while auth.environment is production, so all installs using
these defaults will run with experimental auth behavior. This increases compatibility/upgrade risk
for downstream deployments because the behavior is explicitly marked experimental in the values
file.
Code

charts/rhdh/values.yaml[R416-420]

+          # Enable experimental features for OIDC auth
+          experimentalClientIdMetadataDocuments:
+            enabled: true
+          experimentalRefreshToken:
+            enabled: true
Evidence
The chart’s default Backstage appConfig sets auth.environment: production and then enables both
experimental auth toggles, meaning the default rendered config will include these experimental
settings unless overridden by the deployer.

charts/rhdh/values.yaml[414-420]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`charts/rhdh/values.yaml` enables experimental auth features (`experimentalClientIdMetadataDocuments`, `experimentalRefreshToken`) by default under `auth.environment: production`. Because these are experimental toggles, enabling them unconditionally in the chart defaults can cause unexpected behavior changes or upgrade compatibility issues for downstream users who rely on the default values.

## Issue Context
This PR is intended to enable OAuth2 authorization for `@backstage/cli auth login`. If these experimental flags are required for that flow, they should be enabled in a targeted/explicit way rather than as unconditional defaults for all deployments.

## Fix Focus Areas
- charts/rhdh/values.yaml[416-420]

## Suggested fix approach
- Default both experimental flags to `false` in `values.yaml`.
- Add a dedicated chart value (or profile-specific override) to enable them only when `backstage-cli` OAuth2 login is desired.
- Document the requirement/when to enable these flags near the relevant values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@yangcao77

Copy link
Copy Markdown
Author

closing this PR. meant to merge to development branch

@yangcao77 yangcao77 closed this Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant