improve the token url elicitatoin demo by enabling authpolicies to be…#1064
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 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)
📝 WalkthroughWalkthroughThis PR parameterizes the URL-elicitation demo: adds an OIDC callback AuthPolicy with OPA redirect/token-exchange logic, adds a template renderer script, replaces hardcoded issuer/domain values with template variables in existing AuthPolicies, updates mcpserverregistration to use templated token URL, and makes demo.sh render and apply the generated manifests. ChangesOIDC Demo Parameterization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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
added
review-effort/medium
![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 `@demos/url-elicitation-github/authpolicy-callback.yaml`:
- Around line 37-43: encoded_callback (computed in the rego snippet) is used as
the redirect_uri in the auth request (auth_url) but the token-exchange uses the
template variable {{ REDIRECT_URI_ENCODED }}, which can diverge and cause
redirect_uri mismatch; ensure both encodings are identical by replacing the
template var usage with the same rego-computed encoded_callback (or derive
REDIRECT_URI_ENCODED from the same base + "/auth/callback" encoding logic), so
the token exchange and auth request use the exact same encoded redirect URI
(symbols to locate: encoded_callback, auth_url, redirect_uri, {{
REDIRECT_URI_ENCODED }}, base).
- Around line 89-96: The jwt Set-Cookie in the unauthorized response includes
the Secure attribute which prevents browsers from storing the cookie when the
gateway uses http (default EXTERNAL_URL); change the headers.set-cookie
expression in the unauthorized block so the cookie omits Secure for non-HTTPS
origins (make Secure conditional based on EXTERNAL_URL/EXTERNAL_HOST or an
EXTERNAL_SCHEME env var) or remove Secure during local/dev, and apply the same
change to the target cookie in authpolicy-tokens.yaml (the target cookie at the
referenced line 28) so both cookies are only marked Secure when the gateway uses
https.
- Around line 46-67: When authenticated and has_target are true but target_safe
is false, location is left undefined; add a fallback rule to set location to a
safe path (e.g., concat("", [base, request.path]) or base + request.path) for
the case authenticated and has_target and not target_safe so the redirect always
has a Location header. Update the policy where location is defined (rules
referencing authenticated, has_target, target_safe, cookies.target, base,
auth_url) to include this fallback branch that uses request.path instead of the
unsafe cookies.target.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c00ff7f-8465-4662-a14a-06e4bd990813
📒 Files selected for processing (6)
demos/url-elicitation-github/authpolicy-callback.yamldemos/url-elicitation-github/authpolicy-gateway.yamldemos/url-elicitation-github/authpolicy-tokens.yamldemos/url-elicitation-github/callback-authpolicy.yamldemos/url-elicitation-github/demo.shdemos/url-elicitation-github/mcpserverregistration.yaml
💤 Files with no reviewable changes (1)
- demos/url-elicitation-github/callback-authpolicy.yaml
… generated Signed-off-by: craig <cbrookes@redhat.com>
Signed-off-by: craig <cbrookes@redhat.com>
Signed-off-by: craig <cbrookes@redhat.com>
Signed-off-by: craig <cbrookes@redhat.com>
maleck13
force-pushed
the
oidc-policy-generate
branch
from
0ab459e to
7010887
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
demos/url-elicitation-github/authpolicy-callback.yaml (1)
37-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRedirect URI encodings still diverge.
encoded_callback(line 38) only escapes:,/,=, while the token exchange uses{{ REDIRECT_URI_ENCODED }}(line 85) fromjq@uri``. If the regoauth_urlpath is taken, OIDC will reject the token exchange on a `redirect_uri` mismatch unless the two encodings are byte-identical.Also applies to line 85.
🤖 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 `@demos/url-elicitation-github/authpolicy-callback.yaml` around lines 37 - 43, The redirect URI encoding in the Rego snippet is inconsistent: encoded_callback (constructed via chained replace calls) will differ from the REDIRECT_URI_ENCODED produced by jq `@uri`, causing token exchange failures; update the Rego logic so both flows use the same encoded value — either accept REDIRECT_URI_ENCODED into the policy and use it for auth_url, or replace the chained replace logic in encoded_callback with a proper percent-encoding routine that matches jq `@uri` (RFC3986-style) so auth_url and the token-exchange REDIRECT_URI_ENCODED are byte-identical (modify identifiers encoded_callback and auth_url accordingly, or reference REDIRECT_URI_ENCODED in the Rego template).
🤖 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 `@demos/url-elicitation-github/authpolicy-callback.yaml`:
- Line 89: The YAML value for the url key (the line containing url: {{
ISSUER_URL }}/protocol/openid-connect/token) is being parsed as a flow mapping
because the scalar starts with '{'; fix it by quoting the templated URL value
(e.g., wrap the entire {{ ISSUER_URL }}/protocol/openid-connect/token in single
or double quotes) so YAML parsers and sed substitutions handle it correctly;
update the url entry where it appears in authpolicy-callback.yaml accordingly.
In `@demos/url-elicitation-github/authpolicy-tokens.yaml`:
- Line 16: The YAML emitter is treating the unquoted template as a flow mapping;
update the issuerUrl field to be a quoted scalar so the template renders and
lints correctly — change the issuerUrl entry that currently reads issuerUrl: {{
ISSUER_URL }} to use quotes around the templated value (issuerUrl: "{{
ISSUER_URL }}"), referencing the issuerUrl key and ISSUER_URL variable in the
fix.
In `@demos/url-elicitation-github/mcpserverregistration.yaml`:
- Line 16: The YAML line url: {{ EXTERNAL_URL }}/tokens is invalid as written;
wrap the templated URL in quotes (e.g., set url: "{{ EXTERNAL_URL }}/tokens") so
the file parses as valid YAML, and also ensure the MCPServerRegistration
manifest includes a credentialRef field referencing a Kubernetes Secret (the
credentialRef key in the resource) per the coding guideline.
In `@scripts/generate-oidc-authpolicies.sh`:
- Line 39: EXTERNAL_HOST assignment retains the port when the URL contains a
path because the sed substitutions strip the port only if it comes at the end;
change the sed sequence so the path is removed before stripping the port. Update
the EXTERNAL_HOST line (reference EXTERNAL_HOST and EXTERNAL_URL) to first
remove the scheme, then remove any path segment, then remove a trailing :port,
so domain={{ EXTERNAL_HOST }} receives a bare host without port or path.
---
Duplicate comments:
In `@demos/url-elicitation-github/authpolicy-callback.yaml`:
- Around line 37-43: The redirect URI encoding in the Rego snippet is
inconsistent: encoded_callback (constructed via chained replace calls) will
differ from the REDIRECT_URI_ENCODED produced by jq `@uri`, causing token exchange
failures; update the Rego logic so both flows use the same encoded value —
either accept REDIRECT_URI_ENCODED into the policy and use it for auth_url, or
replace the chained replace logic in encoded_callback with a proper
percent-encoding routine that matches jq `@uri` (RFC3986-style) so auth_url and
the token-exchange REDIRECT_URI_ENCODED are byte-identical (modify identifiers
encoded_callback and auth_url accordingly, or reference REDIRECT_URI_ENCODED in
the Rego template).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e093b63f-ccae-4c84-8f50-ec382d00cb32
📒 Files selected for processing (7)
demos/url-elicitation-github/authpolicy-callback.yamldemos/url-elicitation-github/authpolicy-gateway.yamldemos/url-elicitation-github/authpolicy-tokens.yamldemos/url-elicitation-github/callback-authpolicy.yamldemos/url-elicitation-github/demo.shdemos/url-elicitation-github/mcpserverregistration.yamlscripts/generate-oidc-authpolicies.sh
💤 Files with no reviewable changes (1)
- demos/url-elicitation-github/callback-authpolicy.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- demos/url-elicitation-github/authpolicy-gateway.yaml
- demos/url-elicitation-github/demo.sh
Signed-off-by: craig <cbrookes@redhat.com>
Member
|
👀 |
david-martin
approved these changes
Jun 26, 2026
david-martin
left a comment
david-martin
left a comment
Member
There was a problem hiding this comment.
verified the script changes locally.
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.
This PR updates the demo users can do to use token url elicitation. It allows the host to be specified and generates the resources with that value.
Summary by CodeRabbit
Configuration & Improvements
Behavior & Security
Chores