Conversation
📝 WalkthroughWalkthroughOAuth2 authentication configuration updated across the system: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 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 |
|
This work compliments work I am doing to add an example of using a user defined token to connect to the GH mcp server via the gateway. I am using claude as the client to prove it out |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/keycloak/realm-import.yaml (1)
641-647:⚠️ Potential issue | 🟡 MinorMove
offline_accesstodefaultOptionalClientScopes.Including
offline_accessindefaultDefaultClientScopesgrants long-lived offline refresh tokens to all clients by default, which violates principle of least privilege. Offline tokens persist across user logout and server restarts, creating a security surface area. Keycloak best practices and OpenID Connect spec recommendoffline_accessremain optional, requiring explicit client request. Clients needing offline tokens (e.g., Claude Code) can explicitly request the scope instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/keycloak/realm-import.yaml` around lines 641 - 647, The realm config currently lists "offline_access" inside defaultDefaultClientScopes which grants offline tokens to all clients; remove "offline_access" from the defaultDefaultClientScopes array and add it to the defaultOptionalClientScopes array so offline tokens are only granted when a client explicitly requests the offline_access scope; update the arrays referenced by the keys defaultDefaultClientScopes and defaultOptionalClientScopes accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/auth.mk`:
- Line 23: The docs still list OAUTH_SCOPES_SUPPORTED without the offline_access
scope while the build variable
OAUTH_SCOPES_SUPPORTED="basic,groups,roles,profile,offline_access" was added;
update the authentication guide text that enumerates OAuth scopes to include
offline_access so users copy-pasting the provided kubectl set env command get a
working CLI OAuth flow, by editing the guide's section that lists available
scopes (update the scope string to include offline_access wherever
OAUTH_SCOPES_SUPPORTED is documented).
---
Outside diff comments:
In `@config/keycloak/realm-import.yaml`:
- Around line 641-647: The realm config currently lists "offline_access" inside
defaultDefaultClientScopes which grants offline tokens to all clients; remove
"offline_access" from the defaultDefaultClientScopes array and add it to the
defaultOptionalClientScopes array so offline tokens are only granted when a
client explicitly requests the offline_access scope; update the arrays
referenced by the keys defaultDefaultClientScopes and
defaultOptionalClientScopes accordingly.
🪄 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: ae3b75d9-124b-49d3-bcb3-665d587eaf6e
📒 Files selected for processing (3)
build/auth.mkcmd/mcp-broker-router/main.goconfig/keycloak/realm-import.yaml
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guides/authentication.md (1)
203-209:⚠️ Potential issue | 🟡 MinorExample response out of sync with new scope.
The env var now advertises
offline_access, but the examplescopes_supportedarray below still lists only four scopes. Update the sample output so readers aren't surprised when they hit the endpoint.Proposed fix
# "scopes_supported": [ # "basic", # "groups", # "roles", -# "profile" +# "profile", +# "offline_access" # ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/authentication.md` around lines 203 - 209, Update the example JSON response in the documentation so the "scopes_supported" array matches the current advertised scopes (include "offline_access" in addition to "basic", "groups", "roles", "profile"); locate the sample block that defines the "scopes_supported" array in authentication.md and add "offline_access" to that array so the example output matches the environment variable behavior.
🧹 Nitpick comments (1)
docs/guides/external-mcp-server.md (1)
316-334: Clarify thatx-github-patis forwarded in cleartext.Worth a one-line callout near the client config: the PAT travels as a plain request header to the gateway, so HTTPS between client and gateway is strongly recommended (the sample URL here is
http://mcp.127-0-0-1.sslip.io:8001/mcp, which is fine for local but shouldn't be extrapolated to prod). Also consider mentioning that the PAT will appear in any access/debug logs that capture headers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/external-mcp-server.md` around lines 316 - 334, The docs currently show the x-github-pat header in the .claude.json/mcp-gateway example without warning that it is transmitted in cleartext over HTTP; add a one-line callout immediately after the client config (referencing the example .claude.json and the mcp-gateway/url) stating that the PAT is sent as a plaintext request header to the gateway, recommend using HTTPS in production, and warn that the PAT may appear in access/debug logs that capture headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/samples/remote-github/authpolicy.yaml`:
- Around line 17-19: The issuerUrl value under the 'keycloak' -> 'jwt' entry in
the sample authpolicy.yaml currently uses https while the authentication guide
uses http, causing scheme mismatch and JWT validation failures; update the
sample's issuerUrl to match the guide's scheme (or vice versa) so both use the
same scheme, e.g., change the issuerUrl value for the 'issuerUrl' key to use
http consistently across the sample and the authentication guide (ensure the
'keycloak' jwt.issuerUrl string is identical in both places).
In `@docs/guides/external-mcp-server.md`:
- Around line 22-36: The Local Environment section uses repo-specific make
targets (make local-env-setup, make auth-example-setup) and repo-only scripts;
replace that block with generic, standalone instructions that describe deploying
Keycloak, Kuadrant, and an external MCP server via applying manifests (e.g.,
"kubectl apply -f <manifest-url>") or Helm charts, and remove direct references
to the Makefile targets; move the kubectl delete authpolicy commands into a
short conditional callout titled "If you ran example scripts" so they are
optional (retain the exact resource names mcp-auth-policy and mcps-auth-policy
and the kubectl delete pattern in that callout) and ensure the language follows
the guideline to avoid repo-internal references.
- Around line 189-191: Tidy the two separate blockquote notes into a single
consolidated blockquote with a standardized label (e.g., "> **Note:**" or
GitHub-style "> [!NOTE]"), remove the blank line between them to resolve MD028,
and correct the wording "self signed" to "self-signed"; ensure the resulting
blockquote contains both sentences (the browser/self-signed-certificate note and
the NODE_TLS_REJECT_UNAUTHORIZED command for claude sessions) exactly as content
so nothing is lost.
- Line 245: The issuerUrl scheme is inconsistent: update the issuerUrl value in
external-mcp-server.md to use the same scheme as authentication.md and
OAUTH_AUTHORIZATION_SERVERS (e.g., change
https://keycloak.127-0-0-1.sslip.io:8002/realms/mcp to
http://keycloak.127-0-0-1.sslip.io:8002/realms/mcp) and then search for all
occurrences of the issuerUrl key and the OAUTH_AUTHORIZATION_SERVERS entry to
ensure every guide/config uses the identical exact URL string (same scheme,
host, port, and path) so JWT issuer validation will match.
---
Outside diff comments:
In `@docs/guides/authentication.md`:
- Around line 203-209: Update the example JSON response in the documentation so
the "scopes_supported" array matches the current advertised scopes (include
"offline_access" in addition to "basic", "groups", "roles", "profile"); locate
the sample block that defines the "scopes_supported" array in authentication.md
and add "offline_access" to that array so the example output matches the
environment variable behavior.
---
Nitpick comments:
In `@docs/guides/external-mcp-server.md`:
- Around line 316-334: The docs currently show the x-github-pat header in the
.claude.json/mcp-gateway example without warning that it is transmitted in
cleartext over HTTP; add a one-line callout immediately after the client config
(referencing the example .claude.json and the mcp-gateway/url) stating that the
PAT is sent as a plaintext request header to the gateway, recommend using HTTPS
in production, and warn that the PAT may appear in access/debug logs that
capture headers.
🪄 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: ace45d99-2a86-4f71-921d-25d5f0795f2d
📒 Files selected for processing (4)
config/keycloak/realm-import.yamlconfig/samples/remote-github/authpolicy.yamldocs/guides/authentication.mddocs/guides/external-mcp-server.md
✅ Files skipped from review due to trivial changes (1)
- config/keycloak/realm-import.yaml
| 'keycloak': | ||
| jwt: | ||
| issuerUrl: https://keycloak.127-0-0-1.sslip.io:8002/realms/mcp |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'keycloak\.127-0-0-1\.sslip\.io:8002' -C1Repository: Kuadrant/mcp-gateway
Length of output: 6926
🏁 Script executed:
cat -n config/samples/remote-github/authpolicy.yaml | head -25Repository: Kuadrant/mcp-gateway
Length of output: 907
🏁 Script executed:
tail -c 20 config/samples/remote-github/authpolicy.yaml | od -cRepository: Kuadrant/mcp-gateway
Length of output: 106
🏁 Script executed:
tail -c 1 config/samples/remote-github/authpolicy.yaml | wc -lRepository: Kuadrant/mcp-gateway
Length of output: 65
🏁 Script executed:
sed -n '160,165p' docs/guides/authentication.mdRepository: Kuadrant/mcp-gateway
Length of output: 233
Fix scheme inconsistency in issuerUrl across guides and samples.
The sample uses https://keycloak.127-0-0-1.sslip.io:8002/realms/mcp (line 19), but docs/guides/authentication.md (line 162) uses http://. Users mixing configurations from different documentation sources will encounter JWT validation failures. Standardize on one scheme across all guides and samples.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/samples/remote-github/authpolicy.yaml` around lines 17 - 19, The
issuerUrl value under the 'keycloak' -> 'jwt' entry in the sample
authpolicy.yaml currently uses https while the authentication guide uses http,
causing scheme mismatch and JWT validation failures; update the sample's
issuerUrl to match the guide's scheme (or vice versa) so both use the same
scheme, e.g., change the issuerUrl value for the 'issuerUrl' key to use http
consistently across the sample and the authentication guide (ensure the
'keycloak' jwt.issuerUrl string is identical in both places).
| ## Local Environment | ||
|
|
||
| The fastest way to set up the GitHub MCP server is using the provided script: | ||
| If you have the repository checked out, you can set up a complete local environment with Keycloak, Kuadrant, and an example external MCP server: | ||
|
|
||
| ```bash | ||
| # Set your GitHub PAT | ||
| export GITHUB_PAT="ghp_YOUR_GITHUB_TOKEN_HERE" | ||
|
|
||
| # Run the setup script | ||
| ./config/samples/remote-github/create_resources.sh | ||
| make local-env-setup # Kind cluster with gateway | ||
| make auth-example-setup # Only needed for Auth step. Sets up Keycloak + Kuadrant + AuthPolicy prerequisites | ||
| ``` | ||
|
|
||
| The script will: | ||
| - Validate your GITHUB_PAT environment variable and token format | ||
| - Create ServiceEntry, DestinationRule, HTTPRoute, Secret, and MCPServerRegistration | ||
| - Apply the AuthPolicy for OAuth + API key handling | ||
| `auth-example-setup` creates its own AuthPolicies (`mcp-auth-policy` and `mcps-auth-policy` in `gateway-system`). If you ran that command, remove the existing AuthPolicies before following this guide: | ||
|
|
||
| All the sample YAML files are available in `config/samples/remote-github/` for reference or customization. For a detailed explanation of each component, continue reading the manual setup steps below. | ||
| ```bash | ||
| kubectl delete authpolicy mcp-auth-policy -n gateway-system --ignore-not-found | ||
| kubectl delete authpolicy mcps-auth-policy -n gateway-system --ignore-not-found | ||
| ``` |
There was a problem hiding this comment.
Repo-internal make targets don't belong in user guides.
This "Local Environment" block relies on make local-env-setup / make auth-example-setup, which only work if the reader has the repo checked out. Per the guides guideline, docs should be framed as standalone guidance for any MCP Gateway install, not tied to repo-specific setup (Kind, Makefile targets, clone-dependent scripts).
Either drop this section or rewrite it with generic install steps (Keycloak/Kuadrant manifests + kubectl apply) and move the "delete existing AuthPolicies" note to a conditional callout for users who ran the example scripts.
As per coding guidelines: "Do not include repo-internal references such as files in config/ or Makefile targets in user-facing guides; instead provide inline YAML or kubectl commands".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/external-mcp-server.md` around lines 22 - 36, The Local
Environment section uses repo-specific make targets (make local-env-setup, make
auth-example-setup) and repo-only scripts; replace that block with generic,
standalone instructions that describe deploying Keycloak, Kuadrant, and an
external MCP server via applying manifests (e.g., "kubectl apply -f
<manifest-url>") or Helm charts, and remove direct references to the Makefile
targets; move the kubectl delete authpolicy commands into a short conditional
callout titled "If you ran example scripts" so they are optional (retain the
exact resource names mcp-auth-policy and mcps-auth-policy and the kubectl delete
pattern in that callout) and ensure the language follows the guideline to avoid
repo-internal references.
There was a problem hiding this comment.
+1 on not including any local dev stuff in this guide.
We could still mention if you just want to try out something locally, point to the repo and mention the targets, but let's not actually instruct people to do that.
A guide is meant to be more generally applicable to any environment (maybe you were thinking of a tutorial that's hand holding start to finish).
So, no need to go overboard. We shouldn't give instructions on how to install keycloak, kuadrant and an external MCP server. They can be pre-reqs, or link to relevant install docs.
There was a problem hiding this comment.
@david-martin — agreed, that's a cleaner approach. The fix would be to drop the "Local Environment" section as instructions and replace it with a short informational note, something like:
> [!NOTE]
> For local testing, the repository includes `make local-env-setup` and `make auth-example-setup` targets. See the repo README for details. If you used those targets, delete their pre-created AuthPolicies before following this guide:
> ```bash
> kubectl delete authpolicy mcp-auth-policy -n gateway-system --ignore-not-found
> kubectl delete authpolicy mcps-auth-policy -n gateway-system --ignore-not-found
> ```And the Part 2 additional prerequisites can stay as links to existing docs (e.g. authentication.md), which is already the case in the current file. That keeps the guide standalone while still giving local-dev users a pointer without hand-holding them through it.
✏️ Learnings added
Learnt from: david-martin
Repo: Kuadrant/mcp-gateway PR: 782
File: docs/guides/external-mcp-server.md:22-36
Timestamp: 2026-04-21T08:49:21.868Z
Learning: In Kuadrant/mcp-gateway docs/guides/**/*.md, a brief repo pointer (e.g. a single NOTE callout mentioning make targets) is acceptable for local-testing context, but the section must not be framed as step-by-step instructions. The guide body should remain standalone (prerequisites or links to install docs), not repo-clone-dependent.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: internal/controller/CLAUDE.md:0-0
Timestamp: 2026-03-25T10:39:49.745Z
Learning: For external MCP servers, create appropriate Istio ServiceEntry and HTTPRoute resources and refer to docs/guides/external-mcp-server.md for detailed instructions
Learnt from: david-martin
Repo: Kuadrant/mcp-gateway PR: 635
File: docs/guides/scaling.md:19-22
Timestamp: 2026-03-05T15:18:11.869Z
Learning: In docs/guides/ and their subguides (e.g., docs/guides/scaling.md), ensure the content is framed as standalone guidance applicable to any MCP Gateway installation and not tied to repo-specific setup steps (such as clone-dependent scripts or make targets like make configure-redis). Review for references to repository-local commands or setup steps, and remove or generalize them to be applicable broadly. If including examples, use generic installation instructions rather than repo-specific commands.
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Do not assume a specific cluster setup (Kind, minikube, etc.) in guides; only assume the MCP Gateway is installed
Learnt from: jasonmadigan
Repo: Kuadrant/mcp-gateway PR: 738
File: docs/guides/tool-revocation.md:49-55
Timestamp: 2026-04-09T09:58:21.018Z
Learning: In Kuadrant/mcp-gateway documentation guide markdown files under docs/guides/**/*.md, treat the example gateway defaults as intentional and do not flag them as “overly specific” or “not configurable.” Specifically, when used in guide examples, allow the identifier name `mcp-gateway`, namespace `gateway-system`, and port `8001` without suggesting they be replaced with user-provided variables.
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: internal/controller/CLAUDE.md:0-0
Timestamp: 2026-03-25T10:39:49.745Z
Learning: Applies to internal/controller/**/*MCPServerRegistration*.yaml : For internal services in MCPServerRegistration, use hostname pattern *.mcp.local to match wildcard Gateway listeners
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Do not include repo-internal references such as files in `config/` or Makefile targets in user-facing guides; instead provide inline YAML or `kubectl` commands
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: When adding a new guide to `docs/guides/`, add it to both `docs/guides/README.md` in this repo and the MCP Gateway nav section in the docs.kuadrant.io `mkdocs.yml`
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Keep guides concise by explaining what the user needs to do rather than the full internal design
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Include a Prerequisites section listing what must already be in place before starting a guide
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : User-facing documentation for operators and developers should be placed in `docs/guides/`
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Use `kubectl set env` for deployment configuration in guides as a consistent pattern
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Applies to docs/guides/**/*.md : Use numbered steps (Step 1, Step 2, etc.) in guides, with each step having a clear action and verification command
Learnt from: CR
Repo: Kuadrant/mcp-gateway PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-16T10:56:05.106Z
Learning: Follow the Diataxis framework for writing guides, focusing on how-to guides that are goal-oriented and assume the reader has a working system
Learnt from: jasonmadigan
Repo: Kuadrant/mcp-gateway PR: 683
File: docs/guides/vault-token-exchange.md:160-161
Timestamp: 2026-03-26T12:07:22.059Z
Learning: In this repo’s GitHub-flavored docs (all `docs/**/*.md`), GitHub admonition blocks may appear in either of these forms: `> [!NOTE]` or `> **Note:**`. Prefer the GitHub-style `> [!NOTE]` syntax. Do not flag `> [!NOTE]` as incorrect, since the repo’s mkdocs setup/plugin converts GitHub admonitions to the proper mkdocs format for the docs site.
| authentication: | ||
| 'keycloak': | ||
| jwt: | ||
| issuerUrl: https://keycloak.127-0-0-1.sslip.io:8002/realms/mcp |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check authentication.md around line 162
fd -t f authentication.md | head -5Repository: Kuadrant/mcp-gateway
Length of output: 93
🏁 Script executed:
# Search for the OAUTH_AUTHORIZATION_SERVERS reference
rg -n "OAUTH_AUTHORIZATION_SERVERS" --max-count 5Repository: Kuadrant/mcp-gateway
Length of output: 1161
🏁 Script executed:
# Also check external-mcp-server.md at line 245 and its context
fd -t f external-mcp-server.md | head -5Repository: Kuadrant/mcp-gateway
Length of output: 98
🏁 Script executed:
# Get content around line 162 in authentication.md
sed -n '155,170p' docs/guides/authentication.mdRepository: Kuadrant/mcp-gateway
Length of output: 558
🏁 Script executed:
# Get content around line 245 in external-mcp-server.md
sed -n '240,250p' docs/guides/external-mcp-server.mdRepository: Kuadrant/mcp-gateway
Length of output: 458
🏁 Script executed:
# Get broader context of authentication.md to find all issuerUrl references
rg -n "issuerUrl" docs/guides/authentication.md -A 2 -B 2Repository: Kuadrant/mcp-gateway
Length of output: 233
🏁 Script executed:
# Get broader context of external-mcp-server.md to find all issuerUrl references
rg -n "issuerUrl" docs/guides/external-mcp-server.md -A 2 -B 2Repository: Kuadrant/mcp-gateway
Length of output: 426
issuerUrl scheme inconsistent across guides.
docs/guides/external-mcp-server.md (line 245) uses https:// while docs/guides/authentication.md (line 162) and OAUTH_AUTHORIZATION_SERVERS (line 90) both use http://. JWT issuer URLs must match exactly for token validation; users following these guides in sequence will encounter validation failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/external-mcp-server.md` at line 245, The issuerUrl scheme is
inconsistent: update the issuerUrl value in external-mcp-server.md to use the
same scheme as authentication.md and OAUTH_AUTHORIZATION_SERVERS (e.g., change
https://keycloak.127-0-0-1.sslip.io:8002/realms/mcp to
http://keycloak.127-0-0-1.sslip.io:8002/realms/mcp) and then search for all
occurrences of the issuerUrl key and the OAUTH_AUTHORIZATION_SERVERS entry to
ensure every guide/config uses the identical exact URL string (same scheme,
host, port, and path) so JWT issuer validation will match.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/guides/external-mcp-server.md (2)
244-244:⚠️ Potential issue | 🟠 MajorissuerUrl scheme still inconsistent.
The past review comment flagged that this guide uses
https://keycloak...whileauthentication.mdandOAUTH_AUTHORIZATION_SERVERSusehttp://. JWT issuer URLs must match exactly for token validation to succeed.Verify the correct scheme and ensure all references use the same URL (scheme, host, port, path) across all guides and configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/external-mcp-server.md` at line 244, The issuerUrl value shown uses https but other places (authentication.md and the OAUTH_AUTHORIZATION_SERVERS entries) use http, which must match exactly for JWT validation; update the issuerUrl entry (the issuerUrl setting in this guide) to use the same scheme, host, port and path as the canonical OAUTH_AUTHORIZATION_SERVERS/authentication.md configuration, or vice versa—verify the authoritative environment and make all references (issuerUrl in this guide, the OAUTH_AUTHORIZATION_SERVERS constant, and entries in authentication.md) use that identical URL string (scheme://host:port/path).
22-36:⚠️ Potential issue | 🟠 MajorRepo-internal make targets still present.
This section still references
make local-env-setupandmake auth-example-setup, which only work if the user has cloned the repository. The past review comment on this exact block requested removal or replacement with generic standalone instructions.Either remove this section entirely or replace it with generic kubectl/Helm commands that work for any MCP Gateway installation. As per coding guidelines: "Do not include repo-internal references such as files in
config/or Makefile targets in user-facing guides; instead provide inline YAML orkubectlcommands".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/external-mcp-server.md` around lines 22 - 36, Remove the repo-internal make target references (`make local-env-setup`, `make auth-example-setup`) and the note about deleting `mcp-auth-policy`/`mcps-auth-policy`; instead provide generic, standalone instructions that work for any MCP Gateway installation (e.g., use explicit kubectl/helm commands or inline YAML manifests to install Keycloak/Kuadrant and create the example external MCP server and AuthPolicy resources). Update the "Local Environment" section to replace the `make` lines with those generic kubectl/helm/inlined YAML steps and ensure any reference to `mcp-auth-policy` and `mcps-auth-policy` is either removed or shown as optional resources with full kubectl delete/create commands so the guide contains no repo-specific Makefile targets or config references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/guides/external-mcp-server.md`:
- Line 244: The issuerUrl value shown uses https but other places
(authentication.md and the OAUTH_AUTHORIZATION_SERVERS entries) use http, which
must match exactly for JWT validation; update the issuerUrl entry (the issuerUrl
setting in this guide) to use the same scheme, host, port and path as the
canonical OAUTH_AUTHORIZATION_SERVERS/authentication.md configuration, or vice
versa—verify the authoritative environment and make all references (issuerUrl in
this guide, the OAUTH_AUTHORIZATION_SERVERS constant, and entries in
authentication.md) use that identical URL string (scheme://host:port/path).
- Around line 22-36: Remove the repo-internal make target references (`make
local-env-setup`, `make auth-example-setup`) and the note about deleting
`mcp-auth-policy`/`mcps-auth-policy`; instead provide generic, standalone
instructions that work for any MCP Gateway installation (e.g., use explicit
kubectl/helm commands or inline YAML manifests to install Keycloak/Kuadrant and
create the example external MCP server and AuthPolicy resources). Update the
"Local Environment" section to replace the `make` lines with those generic
kubectl/helm/inlined YAML steps and ensure any reference to `mcp-auth-policy`
and `mcps-auth-policy` is either removed or shown as optional resources with
full kubectl delete/create commands so the guide contains no repo-specific
Makefile targets or config references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05ef6043-4557-46f6-8879-6fd0e0d0d3f5
📒 Files selected for processing (1)
docs/guides/external-mcp-server.md
Signed-off-by: craig <cbrookes@redhat.com>
Signed-off-by: craig <cbrookes@redhat.com>
…ider such as GH Signed-off-by: craig <cbrookes@redhat.com>
Signed-off-by: craig <cbrookes@redhat.com>
5e05d7d to
9d31195
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/external-mcp-server.md`:
- Around line 186-205: The Step 7 "Connect an MCP Client" flow contradicts the
stated Part 1 scope because it instructs readers to authenticate with Keycloak
before AuthPolicy/OIDC is configured in Part 2; fix by either moving the entire
"Connect an MCP Client" section to the end of Part 2 or split it into two
subsections: (A) a Part 1 verification that shows how to add the mcpServers
entry (the "mcp-gateway" http url) for discovery only and confirms tools can be
listed without authentication, and (B) a Part 2 addition that adds the
Keycloak/OIDC authentication steps and the authenticated client flow (including
the Keycloak prompt and credentials), ensuring the docs around "Part 1: Basic
setup" and "Part 2: User requests" are consistent.
- Around line 346-359: Add a "Next Steps" section after the existing "Cleanup"
section that lists and links to the related guides (authentication.md,
register-mcp-servers.md, tool-revocation.md) and any other relevant docs; update
the docs/guides content near the "Cleanup" header (look for the Cleanup block
and kubectl delete commands) to append a short paragraph with bullet links to
those guides and a one-line suggestion of what the reader should do next (e.g.,
review authentication, register additional MCP servers, or revoke tool access).
🪄 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: dd6d5d80-2f3a-4361-89c5-ddd604a1c7a4
📒 Files selected for processing (5)
build/auth.mkconfig/keycloak/realm-import.yamlconfig/samples/remote-github/authpolicy.yamldocs/guides/authentication.mddocs/guides/external-mcp-server.md
✅ Files skipped from review due to trivial changes (1)
- docs/guides/authentication.md
🚧 Files skipped from review as they are similar to previous changes (2)
- build/auth.mk
- config/keycloak/realm-import.yaml
| ### Step 7: Connect an MCP Client | ||
|
|
||
|
|
||
| > **Note:** You may need to open Keycloak in your browser and accept the self-signed certificate if doing this locally. | ||
| > If you are using a Claude session, you may need to start it with `NODE_TLS_REJECT_UNAUTHORIZED=0 claude` if doing this locally. | ||
|
|
||
| Configure your MCP client to connect to the gateway. For Claude Code, add to `.claude.json`: | ||
|
|
||
| ```json | ||
| { | ||
| "mcpServers": { | ||
| "mcp-gateway": { | ||
| "type": "http", | ||
| "url": "http://mcp.127-0-0-1.sslip.io:8001/mcp" | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| After connecting and authenticating with Keycloak (credentials: `mcp`/`mcp`), you should see the GitHub tools listed. However, calling any tool will fail with an authentication error because no user credential is being sent to the upstream GitHub server. Part 2 addresses this. |
There was a problem hiding this comment.
Step 7 flow contradicts Part 1 scope.
Part 1 is described as "Basic setup — connect to the external server so the broker can discover and list its tools" and explicitly says "User requests are handled separately — see Part 2". But Step 7 tells the reader to authenticate with Keycloak, while no AuthPolicy/OIDC is configured until Part 2 (Step 8). Readers following Part 1 alone will either see unauthenticated tool discovery (no Keycloak prompt) or be confused when Keycloak never appears.
Either move the "Connect an MCP Client" step to the end of Part 2, or split it: a Part 1 verification that lists tools without auth, and a Part 2 step that adds the client config and describes the Keycloak flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/external-mcp-server.md` around lines 186 - 205, The Step 7
"Connect an MCP Client" flow contradicts the stated Part 1 scope because it
instructs readers to authenticate with Keycloak before AuthPolicy/OIDC is
configured in Part 2; fix by either moving the entire "Connect an MCP Client"
section to the end of Part 2 or split it into two subsections: (A) a Part 1
verification that shows how to add the mcpServers entry (the "mcp-gateway" http
url) for discovery only and confirms tools can be listed without authentication,
and (B) a Part 2 addition that adds the Keycloak/OIDC authentication steps and
the authenticated client flow (including the Keycloak prompt and credentials),
ensuring the docs around "Part 1: Basic setup" and "Part 2: User requests" are
consistent.
| ## Cleanup | ||
|
|
||
| ```bash | ||
| # Part 1 resources | ||
| kubectl delete mcpserverregistration github -n mcp-test | ||
| kubectl delete httproute github-mcp-external -n mcp-test | ||
| kubectl delete serviceentry github-mcp-external -n mcp-test | ||
| kubectl delete destinationrule github-mcp-external -n mcp-test | ||
| kubectl delete secret github-token -n mcp-test | ||
| kubectl delete authpolicy mcps-auth-policy -n mcp-test --ignore-not-found | ||
|
|
||
| # Part 2 resources (if applied) | ||
| kubectl delete authpolicy mcp-auth-policy -n gateway-system --ignore-not-found | ||
| kubectl delete authpolicy github-token-replacement-policy -n mcp-test --ignore-not-found | ||
| ``` |
There was a problem hiding this comment.
Missing Next Steps section.
The guide ends at Cleanup with no pointers to related guides (e.g., authentication.md, register-mcp-servers.md, tool-revocation.md).
As per coding guidelines: "Include a Next Steps section linking to related guides at the end of each guide".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/external-mcp-server.md` around lines 346 - 359, Add a "Next
Steps" section after the existing "Cleanup" section that lists and links to the
related guides (authentication.md, register-mcp-servers.md, tool-revocation.md)
and any other relevant docs; update the docs/guides content near the "Cleanup"
header (look for the Cleanup block and kubectl delete commands) to append a
short paragraph with bullet links to those guides and a one-line suggestion of
what the reader should do next (e.g., review authentication, register additional
MCP servers, or revoke tool access).
david-martin
left a comment
There was a problem hiding this comment.
Checking, is the doc meant to be a how-to guide or a tutorial? (https://diataxis.fr/)
We can make things a lot easier on ourselves if we stick to one or the other.
To date we haven't really had any tutorials, but this doc seems to bounce between the 2.
|
I guess it is a tutorial I have not seen that distinction before |
|
It's used in https://docs.kuadrant.io, with 4 top level docs types aligning to that framework (Concepts, Reference, Tutorials & Guides) All tutorials start at 0 and 'handhold' at every step so everyone gets to the exact same point. They're not intended to solve production use cases. Moreso for learning. (like a lesson) There's a lot in this doc, enough that it feels like theres content for both (e.g. a guide for how connect your MCP gateway to an external MCP server with auth (using the github MCP as an inline example), and a tutorial that's github MCP server specific where everyone that follows has that example working at the end). |
|
Sure lets go with a follow on issue describing what we want from each. Should be simple to change at that point |
@guicassolato would appreciate a quick look
This allows CLI based tools like claude code to use the example OAuth guide
Summary by CodeRabbit
Release Notes
New Features
offline_accessOAuth scope support for enhanced authentication flexibility.x-github-patheader for secure credential handling.Documentation