Skip to content

feat(mcp): support OAuth for remote OIDC clients - #1920

Open
mfroembgen wants to merge 3 commits into
skyhook-io:mainfrom
mfroembgen:feat/mcp-oauth
Open

mfroembgen wants to merge 3 commits into
skyhook-io:mainfrom
mfroembgen:feat/mcp-oauth

Conversation

@mfroembgen

@mfroembgen mfroembgen commented Sep 28, 2026 •

Copy link
Copy Markdown

Description

Remote MCP clients receive a 401 from an OIDC-protected Radar but cannot discover an authorization server or refresh credentials. This adds opt-in MCP OAuth through --mcp-oauth / mcp.oauth.enabled, allowing clients to use the existing browser login, approve access, and obtain their own credentials.

The implementation includes protected-resource and authorization-server discovery, dynamic public-client registration, explicit CSRF-protected consent, mandatory S256 PKCE, one-time authorization codes, short-lived access tokens, and rotating refresh tokens. Tokens are bound to the client and exact MCP endpoint; /mcp-readonly credentials cannot access /mcp or the REST API. User/group identity continues through the existing Kubernetes RBAC and impersonation paths. Browser and provider logout revoke MCP grants, including pending approvals.

OAuth state is bounded and held in memory. This initial implementation requires standalone OIDC mode and one replica; the chart rejects incompatible configurations and uses Recreate rollouts. Restarts require client re-registration and authorization. Access tokens last 10 minutes, with an absolute 24-hour authorization lifetime. Anonymous registration is rate-limited, and unapproved registrations expire after 10 minutes. Documentation covers setup, ingress discovery, protocol requirements for compatible MCP clients, and these lifecycle limits.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

How has this been tested?

  • go test ./...
  • go test -race ./internal/auth ./internal/server
  • Real MCP Go SDK client integration: discovery, registration, consent, initialize, tool listing/calls, user identity, automatic refresh, refresh replay rejection, and endpoint isolation. Covers both public MCP endpoints at the root, under a nested base path, and behind an ingress that forwards only that prefix.
  • Fake OIDC provider tests for signed login continuation and both logout paths, including revocation after the browser revocation record expires. Unit tests cover PKCE/client/redirect/resource binding, CSRF, token replay/expiry, registration limits, and escaped consent metadata.
  • helm lint deploy/helm/radar, chart render/schema checks, and make test-chart.
  • Added seven Helm unittest cases; the Helm unittest plugin was unavailable locally, so those cases await CI.
  • Independent defect/security review completed with no remaining findings.

Interoperability is verified with the official MCP Go SDK. Individual client applications and deployment against a Kubernetes cluster have not been exercised for this change.

  • Tested locally with minikube/kind
  • Tested against a remote cluster
  • Added/updated unit tests

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my code
  • I have added comments where necessary
  • My changes generate no new warnings
  • Any dependent changes have been merged

Related issues

Fixes #1434


Note

High Risk
Introduces a new OAuth authorization server, token issuance, and MCP authentication path on top of OIDC—security-critical auth surface with in-memory state and single-replica deployment assumptions.

Overview
Adds opt-in MCP OAuth (--mcp-oauth, Helm mcp.oauth.enabled) so remote MCP clients on standalone OIDC deployments can authenticate without copying browser session cookies. Radar acts as the MCP authorization server: discovery (/.well-known/*), dynamic public-client registration, browser consent after existing OIDC login, authorization code + S256 PKCE, and short-lived access tokens with rotating refresh tokens bound to /mcp or /mcp-readonly only (not the REST API).

Wiring and constraints: startup and the chart fail fast unless MCP is on, auth.mode=oidc, a single replica, and not Cloud/tunnel/catalog modes. OAuth state is in-memory, so the chart uses Recreate rollouts and documents re-register/re-authorize after restarts. Browser logout, OIDC backchannel logout, and a signed return_to continuation after IdP login tie MCP grants to browser sessions.

Docs, values/schema, and Helm unittest cases cover setup, ingress paths, token lifetimes, and limits.

Reviewed by Cursor Bugbot for commit f9bf36c. Bugbot is set up for automated code reviews on this repo. Configure here.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add OAuth authorization for remote MCP clients in OIDC deployments

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Enable remote MCP clients to sign in through Radar’s existing OIDC browser flow.
• Issue consent-based, endpoint-bound credentials with PKCE, refresh rotation, and logout
 revocation.
• Restrict deployment to single-replica standalone OIDC and document client setup and token
 lifetimes.
Diagram

sequenceDiagram
    actor Client as Remote Client
    participant Routes as Radar Routes
    participant OAuth as MCP OAuth
    actor Browser
    participant OIDC as OIDC Login
    participant IdP as OIDC Provider
    participant MCP as MCP Handler
    participant K8s as Kubernetes RBAC
    Client->>Routes: Request MCP endpoint
    Routes->>OAuth: Check credentials
    OAuth-->>Client: Discovery challenge
    Client->>OAuth: Discover and register
    Client->>Browser: Open authorization
    Browser->>OAuth: Request access
    OAuth->>OIDC: Require login
    OIDC->>IdP: Authenticate user
    IdP-->>OIDC: Return identity
    OIDC-->>Browser: Resume consent
    Browser->>OAuth: Approve with CSRF
    OAuth-->>Client: Return authorization code
    Client->>OAuth: Exchange code with PKCE
    OAuth-->>Client: Issue endpoint-bound tokens
    Client->>Routes: Request with bearer token
    Routes->>OAuth: Validate token and audience
    OAuth->>MCP: Pass user identity
    MCP->>K8s: Enforce user permissions
    Client->>OAuth: Rotate refresh token
    OIDC->>OAuth: Revoke grants on logout
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Persist OAuth state in a shared store
  • ➕ Supports multiple replicas and preserves registrations and grants across restarts.
  • ➖ Adds storage, atomic rotation, cleanup, and operational complexity to the initial feature.
2. Use identity-provider tokens directly
  • ➕ Avoids operating a separate token issuer in Radar.
  • ➖ Depends on provider support for MCP client registration and consent; does not inherently provide Radar’s exact-endpoint token boundary.

Recommendation: The opt-in, process-local authorization server is a reasonable first release because it keeps MCP credentials separate from provider tokens and preserves Radar’s existing RBAC path. Retain the explicit single-replica restriction; consider shared state only if high availability or restart continuity becomes a requirement.

Files changed (17) +1734 / -14

Enhancement (7) +765 / -9
main.goAdd an opt-in MCP OAuth CLI flag +5/-0

Add an opt-in MCP OAuth CLI flag

• Adds '--mcp-oauth', rejects incompatible authentication and deployment modes, and passes the setting into application configuration.

cmd/explorer/main.go

bootstrap.goPropagate MCP OAuth into server creation +3/-1

Propagate MCP OAuth into server creation

• Adds the application configuration field and forwards it to the server.

internal/app/bootstrap.go

mcp_oauth.goImplement the MCP OAuth authorization server +601/-0

Implement the MCP OAuth authorization server

• Adds discovery, public-client registration, browser consent, PKCE code exchange, endpoint-bound access tokens, rotating refresh tokens, and revocation. Bounds and prunes process-local state while retaining browser-cookie authentication for MCP.

internal/auth/mcp_oauth.go

oidc.goConnect OIDC login and logout to MCP grants +17/-4

Connect OIDC login and logout to MCP grants

• Resumes pending MCP authorization after OIDC login and revokes associated MCP grants on browser or backchannel logout.

internal/auth/oidc.go

oidc_return.goSecure the post-login consent continuation +71/-0

Secure the post-login consent continuation

• Adds a signed, expiring, OIDC-state-bound cookie restricted to the MCP authorization path, with an app-root fallback.

internal/auth/oidc_return.go

mcp_oauth.goRoute MCP authentication and discovery +34/-0

Route MCP authentication and discovery

• Selects OAuth-aware authentication only for enabled MCP endpoints and mounts authorization-server and protected-resource metadata.

internal/server/mcp_oauth.go

server.goInitialize and mount opt-in MCP OAuth +34/-4

Initialize and mount opt-in MCP OAuth

• Validates server prerequisites, creates the OAuth server, connects OIDC logout, and registers OAuth routes. Retains the existing unauthenticated-discovery behavior when OAuth is disabled.

internal/server/server.go

Tests (4) +860 / -0
deployment_mcp_oauth_test.yamlTest Helm deployment constraints +72/-0

Test Helm deployment constraints

• Adds chart cases for default behavior, enabled arguments and rollout strategy, and unsupported configurations.

deploy/helm/radar/tests/deployment_mcp_oauth_test.yaml

mcp_oauth_test.goTest OAuth protocol and security boundaries +478/-0

Test OAuth protocol and security boundaries

• Covers discovery, consent and CSRF, PKCE, audience isolation, replay and expiry, registration limits, escaping, and logout revocation.

internal/auth/mcp_oauth_test.go

oidc_return_test.goTest safe OIDC login continuation +75/-0

Test safe OIDC login continuation

• Checks continuation at root and nested base paths and rejects open redirects, cross-flow reuse, and tampering.

internal/auth/oidc_return_test.go

mcp_oauth_test.goExercise a real MCP SDK OAuth client +235/-0

Exercise a real MCP SDK OAuth client

• Tests discovery through tool calls and automatic refresh against both MCP endpoints, including nested and prefix-only routing. Checks user identity and token isolation from other endpoints and REST.

internal/server/mcp_oauth_test.go

Documentation (3) +75 / -1
authentication.mdExplain MCP OAuth within OIDC authentication +8/-0

Explain MCP OAuth within OIDC authentication

• Describes browser login, separate MCP credentials, deployment prerequisites, token lifetimes, and in-memory restart behavior.

docs/authentication.md

in-cluster.mdAdd MCP OAuth to in-cluster setup guidance +4/-1

Add MCP OAuth to in-cluster setup guidance

• Lists the Helm setting and explains its OIDC, HTTPS callback, single-replica, and rollout requirements.

docs/in-cluster.md

mcp.mdDocument remote MCP sign-in and client setup +63/-0

Document remote MCP sign-in and client setup

• Adds deployment and ingress discovery guidance, endpoint isolation and token lifecycle limits, and a Kiro OAuth example.

docs/mcp.md

Other (3) +34 / -4
deployment.yamlEnforce supported MCP OAuth deployments +18/-1

Enforce supported MCP OAuth deployments

• Rejects disabled MCP, non-OIDC, Cloud, and multi-replica configurations. Passes '--mcp-oauth' to Radar and selects Recreate rollouts.

deploy/helm/radar/templates/deployment.yaml

values.schema.jsonDeclare the MCP OAuth chart setting +9/-2

Declare the MCP OAuth chart setting

• Adds schema validation for 'mcp.oauth.enabled' and clarifies the single-replica requirement.

deploy/helm/radar/values.schema.json

values.yamlDocument the disabled-by-default Helm value +7/-1

Document the disabled-by-default Helm value

• Introduces 'mcp.oauth.enabled: false' with comments covering OIDC, replica, rollout, and restart requirements.

deploy/helm/radar/values.yaml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Active clients lose credential refresh ✓ Resolved 🐞 Bug ☼ Reliability
Description
HandleToken retains every used refresh token until its grant expires, then rejects all exchanges
when the shared refresh store reaches 4096 entries. At roughly 29 clients refreshing every 10
minutes, the store fills before their 24-hour grants expire, blocking both refreshes and new code
exchanges.
Code

internal/auth/mcp_oauth.go[R369-371]

+	if len(s.access) >= mcpStoreLimit || len(s.refresh) >= mcpStoreLimit {
+		mcpError(w, 503, "temporarily_unavailable")
+		return
Evidence
Each exchange inserts a refresh entry; rotation marks the prior entry used without removing it.
Pruning retains used entries until the grant's 24-hour expiry, while the process-wide capacity check
rejects every token request once 4096 refresh entries remain.

internal/auth/mcp_oauth.go[21-26]
internal/auth/mcp_oauth.go[369-371]
internal/auth/mcp_oauth.go[389-417]
internal/auth/mcp_oauth.go[537-543]

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

## Issue description
Used refresh tokens accumulate for the full 24-hour grant lifetime, exhausting the shared 4096-entry store under ordinary refresh traffic.
## Fix Focus Areas
- internal/auth/mcp_oauth.go[369-371]
- internal/auth/mcp_oauth.go[389-417]
- internal/auth/mcp_oauth.go[537-543]
## Recommended Fix
Bound replay-detection history independently of active refresh tokens while preserving family revocation for replays within the chosen detection window. Ensure used entries cannot prevent valid clients from refreshing throughout their grants, and test sustained rotation across multiple grants.

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


2. Subpath clients cannot discover OAuth 🐞 Bug ≡ Correctness
Description
MCPOAuthServer.Authenticate constructs the resource_metadata challenge URL by appending
/.well-known/oauth-protected-resource after s.issuer, even though s.issuer already includes
the configured base path. For a deployment under /radar, clients that follow the advertised
https://host/radar/.well-known/oauth-protected-resource/mcp URL receive no protected-resource
metadata because the server mounts it at
https://host/.well-known/oauth-protected-resource/radar/mcp, preventing the OAuth flow from
starting.
Code

internal/auth/mcp_oauth.go[474]

+		challenge := `Bearer resource_metadata="` + s.issuer + "/.well-known/oauth-protected-resource" + r.URL.Path + `", scope="mcp"`
Evidence
The issuer is derived as origin + basePath, so inserting the well-known segment after s.issuer
produces a base-path-prefixed discovery location. The router instead registers protected-resource
metadata with the base path after the origin-level well-known prefix, as required by the advertised
route construction.

internal/auth/mcp_oauth.go[90-99]
internal/auth/mcp_oauth.go[465-480]
internal/server/server.go[397-402]
internal/server/mcp_oauth.go[25-33]

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

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
`Authenticate` advertises protected-resource metadata below the issuer path, while the router correctly mounts RFC 9728 metadata below the origin with the base path appended after `/.well-known/oauth-protected-resource`. This breaks the `WWW-Authenticate` discovery URL for every non-root `basePath` deployment.
Fix Focus Areas
- internal/auth/mcp_oauth.go[474-474]
- internal/server/mcp_oauth.go[25-33]
Recommended Fix
Construct the `resource_metadata` challenge URL from `s.origin`, followed by `/.well-known/oauth-protected-resource`, `s.basePath`, and the requested MCP path. Keep it identical to the route registered by `mountMCPOAuthMetadata`; for example, a `/radar/mcp` resource must advertise `https://host/.well-known/oauth-protected-resource/radar/mcp`.

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



Remediation recommended

3. A metadata comment repeats the method ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
HandleMetadata is documented only as serving OAuth authorization-server metadata, which its name
and response fields already convey. A reader looking for the endpoint’s discovery contract gets no
additional context from the comment.
Code

internal/auth/mcp_oauth.go[119]

+// HandleMetadata serves OAuth authorization server metadata.
Evidence
The added comment says only that the method serves authorization-server metadata; the method name
and immediately following response fields make that behavior apparent.

Rule 3036542: Avoid explanatory comments that restate obvious code behavior
internal/auth/mcp_oauth.go[119-128]

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

## Issue description
The `HandleMetadata` comment only restates the method's behavior.
## Fix Focus Areas
- internal/auth/mcp_oauth.go[119-128]
## Recommended Fix
Replace the comment with useful discovery-contract context, or remove it if no additional context is needed.

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


4. Parallel approvals block each other ✓ Resolved 🐞 Bug ≡ Correctness
Description
HandleAuthorize writes every pending approval's distinct CSRF value to the same
radar_mcp_consent browser cookie. Opening a second approval page replaces the first page's cookie,
so submitting the first page fails with 403; completing either approval also clears the cookie
needed by the other.
Code

internal/auth/mcp_oauth.go[R275-277]

+	csrf := mcpRandom()
+	pending.CSRF = mcpHash(csrf)
+	pending.SID = session.SID
Evidence
Every consent GET generates a different pending CSRF value but sets the same cookie name. The POST
compares that request's stored hash with the browser's current cookie, and a successful POST deletes
the shared cookie.

internal/auth/mcp_oauth.go[275-285]
internal/auth/mcp_oauth.go[295-308]
internal/auth/mcp_oauth.go[323-324]

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

## Issue description
Concurrent consent pages share one CSRF cookie, causing an earlier page to fail after another page opens or completes.
## Fix Focus Areas
- internal/auth/mcp_oauth.go[275-285]
- internal/auth/mcp_oauth.go[295-306]
- internal/auth/mcp_oauth.go[323-324]
## Recommended Fix
Bind the browser CSRF cookie to its pending approval, using a distinct cookie name or equivalent per-request mechanism. Validate and clear only that approval's cookie, and add a test that completes two approval pages opened in parallel.

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


5. Anonymous traffic blocks new clients ✓ Resolved 🐞 Bug ☼ Reliability
Description
HandleRegister increments one process-wide registration counter before parsing the request body,
regardless of whether registration succeeds. Any unauthenticated caller can send 21 POST requests
within a minute, including malformed ones, and receive 429 alongside every legitimate new client
until the window resets.
Code

internal/auth/mcp_oauth.go[R150-156]

+	s.mu.Lock()
+	if s.now().Sub(s.registrationWindow) >= time.Minute {
+		s.registrationWindow = s.now()
+		s.registrations = 0
+	}
+	s.registrations++
+	limited := s.registrations > 20
Evidence
The public registration handler counts each POST before reading or validating its body. Its counter
is stored once on the OAuth server, and requests exceeding 20 in the current minute receive 429,
irrespective of their source.

internal/auth/mcp_oauth.go[67-79]
internal/auth/mcp_oauth.go[144-169]
internal/server/server.go[493-500]

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

## Issue description
The single global registration counter lets an unauthenticated caller exhaust the entire deployment's registration allowance with malformed requests.
## Fix Focus Areas
- internal/auth/mcp_oauth.go[144-163]
- internal/auth/mcp_oauth.go[164-169]
## Recommended Fix
Apply a bounded per-source registration limit, with an appropriate separate global safety cap, so one source cannot consume all legitimate clients' allowance. Test that malformed requests from one source do not block registration from another.

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


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread internal/auth/mcp_oauth.go Outdated
Comment thread internal/auth/mcp_oauth.go Outdated
Comment thread internal/auth/mcp_oauth.go
Comment thread internal/auth/mcp_oauth.go Outdated
mcpError(w, 401, "invalid_token")
return
}
challenge := `Bearer resource_metadata="` + s.issuer + "/.well-known/oauth-protected-resource" + r.URL.Path + `", scope="mcp"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Subpath clients cannot discover oauth 🐞 Bug ≡ Correctness

MCPOAuthServer.Authenticate constructs the resource_metadata challenge URL by appending
/.well-known/oauth-protected-resource after s.issuer, even though s.issuer already includes
the configured base path. For a deployment under /radar, clients that follow the advertised
https://host/radar/.well-known/oauth-protected-resource/mcp URL receive no protected-resource
metadata because the server mounts it at
https://host/.well-known/oauth-protected-resource/radar/mcp, preventing the OAuth flow from
starting.
Agent Prompt
Issue description
`Authenticate` advertises protected-resource metadata below the issuer path, while the router correctly mounts RFC 9728 metadata below the origin with the base path appended after `/.well-known/oauth-protected-resource`. This breaks the `WWW-Authenticate` discovery URL for every non-root `basePath` deployment.

Fix Focus Areas
- internal/auth/mcp_oauth.go[474-474]
- internal/server/mcp_oauth.go[25-33]

Recommended Fix
Construct the `resource_metadata` challenge URL from `s.origin`, followed by `/.well-known/oauth-protected-resource`, `s.basePath`, and the requested MCP path. Keep it identical to the route registered by `mountMCPOAuthMetadata`; for example, a `/radar/mcp` resource must advertise `https://host/.well-known/oauth-protected-resource/radar/mcp`.

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2325093. Configure here.

Comment thread internal/auth/mcp_oauth.go
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.

Support OAuth 2.1 authentication for remote MCP clients

1 participant