NO-ISSUE: Add script to enable localhost Keycloak redirect URIs for local dev - #88
NO-ISSUE: Add script to enable localhost Keycloak redirect URIs for local dev#88batzionb wants to merge 1 commit into
Conversation
…r local dev When running the UI locally with pnpm dev, OIDC login fails unless the Keycloak osac-ui client is configured to accept localhost redirect URIs. This adds a helper script and documents the dev mode setup in the README. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: batzionb <brotman@redhat.com>
|
@batzionb: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: batzionb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe change adds a helper script and README instructions for enabling local ChangesLocal Keycloak redirect configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant RedirectScript
participant KubectlOrOc
participant Keycloak
Developer->>RedirectScript: Run with local development options
RedirectScript->>KubectlOrOc: Discover OSAC and Keycloak routes
KubectlOrOc-->>RedirectScript: Return route and namespace data
RedirectScript->>Keycloak: Authenticate and find osac-ui client
Keycloak-->>RedirectScript: Return client configuration
RedirectScript->>Keycloak: Create or update redirectUris
Keycloak-->>RedirectScript: Return operation status
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/enable-local-ui-redirect-uri.sh`:
- Around line 109-122: Update discover_osac_namespace so automatic discovery
does not select an arbitrary osac-ui route when multiple namespaces match.
Require an unambiguous result, or fail with an actionable error instructing the
operator to provide --namespace, while preserving the explicit OSAC_NAMESPACE
path and preventing any IdP client modification when discovery is ambiguous.
- Around line 139-146: Remove the default TLS verification bypass from
try_keycloak_token and the other four curl call sites in the script, replacing
curl -sk with certificate-validating requests. Support custom cluster
certificates through an explicit --cacert bundle where needed; if insecure mode
must remain available, require an explicit opt-in with a prominent warning
rather than enabling it by default.
- Around line 95-107: Remove the LOCAL_REDIRECT_WILDCARD and
ALT_REDIRECT_WILDCARD values and stop including wildcard redirect URIs in both
the client creation and merge paths. Retain the exact LOCAL_CALLBACK and
ALT_CALLBACK entries for deployed, localhost, and 127.0.0.1 flows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: feaac82a-3fd5-4e0a-86f9-94b39e286fbb
📒 Files selected for processing (2)
README.mdscripts/enable-local-ui-redirect-uri.sh
| LOCAL_ORIGIN="${LOCAL_ORIGIN%/}" | ||
| LOCAL_CALLBACK="${LOCAL_ORIGIN}/callback" | ||
| LOCAL_REDIRECT_WILDCARD="${LOCAL_ORIGIN}/*" | ||
|
|
||
| # Also allow 127.0.0.1 when origin uses localhost (common alternate). | ||
| ALT_ORIGIN="" | ||
| ALT_CALLBACK="" | ||
| ALT_REDIRECT_WILDCARD="" | ||
| if [[ "${LOCAL_ORIGIN}" == http://localhost:* ]]; then | ||
| ALT_ORIGIN="http://127.0.0.1:${LOCAL_ORIGIN##http://localhost:}" | ||
| ALT_CALLBACK="${ALT_ORIGIN}/callback" | ||
| ALT_REDIRECT_WILDCARD="${ALT_ORIGIN}/*" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant sections.
git ls-files scripts/enable-local-ui-redirect-uri.sh
wc -l scripts/enable-local-ui-redirect-uri.sh
sed -n '1,320p' scripts/enable-local-ui-redirect-uri.sh
# Find every use of the wildcard redirect variables and the redirectUris construction.
rg -n "LOCAL_REDIRECT_WILDCARD|ALT_REDIRECT_WILDCARD|deployedWildcard|redirectUris|merge_redirect_uris|callback" scripts/enable-local-ui-redirect-uri.sh
# Look for any docs/comments about Keycloak redirect URI behavior in the repo.
rg -n "redirect URI|redirectUris|Keycloak|wildcard.*redirect|CVE-2023-6927" .Repository: osac-project/osac-ui
Length of output: 21655
🌐 Web query:
Keycloak CVE-2023-6927 redirect URI wildcard asterisk exact redirect_uri callback documentation
💡 Result:
CVE-2023-6927 was a security vulnerability in Keycloak involving an open redirect via the "form_post.jwt" JARM response mode [1][2]. The flaw allowed an attacker to potentially steal authorization codes or tokens by exploiting clients configured with a wildcard in their redirect URIs, effectively bypassing a previous security patch [2][3]. This vulnerability was fixed in Keycloak version 23.0.4 [3]. Regarding Keycloak's handling of redirect URI wildcards: 1. Usage and Constraints: Keycloak supports using a single asterisk () as a wildcard at the end of a valid redirect URI (e.g., http://host.com/path/) [4][5]. The use of full wildcards (*) for all URIs is not recommended for production environments [5]. 2. Security Best Practices: OAuth 2.0 specifications generally recommend using exact string matching for redirect URIs to enhance security [6]. Keycloak documentation explicitly warns that using wildcards can introduce security risks and advises users to make registered redirect URIs as specific as possible [5][7]. 3. Interaction with Other Components: Wildcard expansion is not supported for query parameters [6]. Furthermore, to mitigate security risks, Keycloak performs standard, exact string matching instead of wildcard comparison if the redirect URI contains a userinfo component or a path that manages access to parent directories (e.g., /../) [5]. 4. Evolution of Validation: Keycloak has consistently tightened its redirect URI validation logic over time to address various vulnerabilities (such as CVE-2023-6927 and the more recent CVE-2026-7504, which involved URL validation bypasses) [3][8][9]. Developers are encouraged to avoid wildcard patterns in production whenever feasible [5][6].
Citations:
- 1: http://osv.dev/vulnerability/CVE-2023-6927
- 2: GHSA-9vm7-v8wj-3fqw
- 3: https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-6927
- 4: https://securityblog.omegapoint.se/en/writeup-keycloak-cve-2023-6927/
- 5: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/clients/oidc/con-basic-settings.adoc
- 6: Redirect_uri doesn't accept parameters keycloak/keycloak#33104
- 7: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/threat/redirect.adoc
- 8: [CVE-2026-7504] Security Vulnerability Report: Redirect URI Validation Bypass in Keycloak keycloak/keycloak#49109
- 9: https://nvd.nist.gov/vuln/detail/cve-2026-7504
Drop the /* redirect URIs
Keycloak recommends exact redirect URI matches; .../callback is enough for both the deployed client and local localhost/127.0.0.1 flows. Keeping /* on the client widens the auth-code/token-theft surface. Remove the wildcard entries from both the create and merge paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/enable-local-ui-redirect-uri.sh` around lines 95 - 107, Remove the
LOCAL_REDIRECT_WILDCARD and ALT_REDIRECT_WILDCARD values and stop including
wildcard redirect URIs in both the client creation and merge paths. Retain the
exact LOCAL_CALLBACK and ALT_CALLBACK entries for deployed, localhost, and
127.0.0.1 flows.
| discover_osac_namespace() { | ||
| if [[ -n "${OSAC_NAMESPACE}" ]]; then | ||
| echo "${OSAC_NAMESPACE}" | ||
| return | ||
| fi | ||
|
|
||
| local ns | ||
| ns="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)" | ||
| if [[ -z "${ns}" ]]; then | ||
| echo "ERROR: Could not find osac-ui route. Pass --namespace." >&2 | ||
| exit 1 | ||
| fi | ||
| echo "${ns}" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Ambiguous namespace auto-discovery could target the wrong (e.g. production) Keycloak client.
get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' blindly picks the first match if multiple osac-ui routes exist across namespaces (staging/prod/multiple dev envs on the same cluster). Since this script modifies an IdP client's redirect URIs, silently discovering the wrong namespace risks adding localhost redirect URIs to a client the operator didn't intend to touch.
🛡️ Proposed fix
local ns
- ns="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)"
+ local count
+ count="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[*].metadata.namespace}' 2>/dev/null | wc -w || true)"
+ if [[ "${count}" -gt 1 ]]; then
+ echo "ERROR: Found osac-ui route in multiple namespaces. Pass --namespace explicitly." >&2
+ exit 1
+ fi
+ ns="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)"📝 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.
| discover_osac_namespace() { | |
| if [[ -n "${OSAC_NAMESPACE}" ]]; then | |
| echo "${OSAC_NAMESPACE}" | |
| return | |
| fi | |
| local ns | |
| ns="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)" | |
| if [[ -z "${ns}" ]]; then | |
| echo "ERROR: Could not find osac-ui route. Pass --namespace." >&2 | |
| exit 1 | |
| fi | |
| echo "${ns}" | |
| } | |
| discover_osac_namespace() { | |
| if [[ -n "${OSAC_NAMESPACE}" ]]; then | |
| echo "${OSAC_NAMESPACE}" | |
| return | |
| fi | |
| local ns | |
| local count | |
| count="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[*].metadata.namespace}' 2>/dev/null | wc -w || true)" | |
| if [[ "${count}" -gt 1 ]]; then | |
| echo "ERROR: Found osac-ui route in multiple namespaces. Pass --namespace explicitly." >&2 | |
| exit 1 | |
| fi | |
| ns="$("${KUBE_CMD[@]}" get route osac-ui -A -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)" | |
| if [[ -z "${ns}" ]]; then | |
| echo "ERROR: Could not find osac-ui route. Pass --namespace." >&2 | |
| exit 1 | |
| fi | |
| echo "${ns}" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/enable-local-ui-redirect-uri.sh` around lines 109 - 122, Update
discover_osac_namespace so automatic discovery does not select an arbitrary
osac-ui route when multiple namespaces match. Require an unambiguous result, or
fail with an actionable error instructing the operator to provide --namespace,
while preserving the explicit OSAC_NAMESPACE path and preventing any IdP client
modification when discovery is ambiguous.
| try_keycloak_token() { | ||
| local kc_url="$1" username="$2" password="$3" | ||
| curl -sk "${kc_url}/realms/master/protocol/openid-connect/token" \ | ||
| -d "grant_type=password" \ | ||
| -d "client_id=admin-cli" \ | ||
| -d "username=${username}" \ | ||
| -d "password=${password}" | jq -r '.access_token // empty' | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
TLS certificate verification disabled (-k) on all Keycloak admin API calls, including credential exchange.
curl -sk disables certificate validation. This call sends the admin username/password in the request body — combined with disabled cert verification, this is vulnerable to MITM credential theft. Same -k pattern repeats at lines 186-187, 271-275, 280-281, and 316-320 (flagged by static analysis, CWE-295). Prefer letting curl validate against the system/cluster CA (--cacert <bundle> if the route uses a custom CA), or make insecure mode an explicit opt-in flag with a loud warning rather than the default.
🔐 Proposed fix (sketch)
+CURL_TLS_OPTS=()
+if [[ "${INSECURE_TLS:-false}" == true ]]; then
+ CURL_TLS_OPTS=(-k)
+ echo "WARNING: TLS certificate verification disabled" >&2
+fi
try_keycloak_token() {
local kc_url="$1" username="$2" password="$3"
- curl -sk "${kc_url}/realms/master/protocol/openid-connect/token" \
+ curl -s "${CURL_TLS_OPTS[@]}" "${kc_url}/realms/master/protocol/openid-connect/token" \Apply the same pattern to the other four curl -sk call sites.
📝 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.
| try_keycloak_token() { | |
| local kc_url="$1" username="$2" password="$3" | |
| curl -sk "${kc_url}/realms/master/protocol/openid-connect/token" \ | |
| -d "grant_type=password" \ | |
| -d "client_id=admin-cli" \ | |
| -d "username=${username}" \ | |
| -d "password=${password}" | jq -r '.access_token // empty' | |
| } | |
| CURL_TLS_OPTS=() | |
| if [[ "${INSECURE_TLS:-false}" == true ]]; then | |
| CURL_TLS_OPTS=(-k) | |
| echo "WARNING: TLS certificate verification disabled" >&2 | |
| fi | |
| try_keycloak_token() { | |
| local kc_url="$1" username="$2" password="$3" | |
| curl -s "${CURL_TLS_OPTS[@]}" "${kc_url}/realms/master/protocol/openid-connect/token" \ | |
| -d "grant_type=password" \ | |
| -d "client_id=admin-cli" \ | |
| -d "username=${username}" \ | |
| -d "password=${password}" | jq -r '.access_token // empty' | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 140-144: curl is invoked with -k/--insecure, which disables TLS certificate verification and exposes the connection to man-in-the-middle attacks. Remove the insecure flag and let curl validate the server certificate; if you need to trust a private CA, pin it with --cacert instead.
Context: curl -sk "${kc_url}/realms/master/protocol/openid-connect/token"
-d "grant_type=password"
-d "client_id=admin-cli"
-d "username=${username}"
-d "password=${password}"
Note: [CWE-295] Improper Certificate Validation.
(curl-insecure-tls-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/enable-local-ui-redirect-uri.sh` around lines 139 - 146, Remove the
default TLS verification bypass from try_keycloak_token and the other four curl
call sites in the script, replacing curl -sk with certificate-validating
requests. Support custom cluster certificates through an explicit --cacert
bundle where needed; if insecure mode must remain available, require an explicit
opt-in with a prominent warning rather than enabling it by default.
Source: Linters/SAST tools
2 participants
Summary
scripts/enable-local-ui-redirect-uri.sh— patches the Keycloakosac-uiclient to acceptlocalhost:5173and127.0.0.1:5173redirect URIs sopnpm devcan complete OIDC login--keycloak-username/--keycloak-passwordarguments,--dry-run,--verify-only, auto-detection of OSAC namespace and Keycloak routeTest plan
./scripts/enable-local-ui-redirect-uri.sh --helpand verify usage output--dry-runagainst a live cluster and verify planned changes are correct--dry-runand confirmpnpm devOIDC login succeeds onlocalhost:5173--verify-onlyafter patching and confirm exit 0Assisted-by: Claude Code noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation