OAuth authorization bridge for the authority role (0.6.0) - #6
Merged
Conversation
Hosted MCP clients authenticate one way only: discover an authorization server, run an authorization-code + PKCE flow in a browser, and use the access_token that comes back. The MCP authorization spec also forbids a token in the request URI, so a token query param is not a fallback for them. A server whose tokens are issued out-of-band is unreachable by those clients. The bridge is a standards-shaped envelope around the tokens a host already issues, not an identity provider. Its authorization page asks an operator to paste an access token they already hold, and the token it hands back IS that token, verified through the same token_authenticator the transport uses. Scopes, expiry, revocation and tenancy stay with the host; no new way to obtain a token exists. Deliberately absent, because none of it gates anything here: client registration returns an identifier and stores nothing (no endpoint reads a client_id); no consent step (pasting a token you hold is the grant); no refresh token (the pasted token's own expiry is the real lifetime). Deliberately NOT faked, because either would be a real vulnerability rather than a skipped ceremony: redirect_uri is matched against config.oauth_allowed_redirect_uris by exact string on both legs (an unvetted target would be an open redirect emitting authorization codes), and the PKCE code_verifier is verified constant-time against the stored S256 challenge. Authority-only and opt-in: oauth_bridge? requires the authority role AND a non-empty redirect allowlist, so the bridge cannot run without bounds on where codes may go, and a satellite (whose tokens belong to its central app) never draws it. A host that configures nothing is unchanged, including its 401. Verified end to end against a real Rails 8.1 app in an isolated child process (spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb): the whole client sequence, including that the view renders and that the issued token authenticates a subsequent MCP call. 546 specs, rubocop and brakeman clean.
Two defects found by checking the bridge against the real adopting app
rather than against assumptions about it.
1. The authority sets parent_controller = "ActionController::API" — its
transport is a JSON-only endpoint, so that is correct and must stay.
The bridge derived its controller from the same config, and
ActionController::API cannot render an HTML view, so the
authorization page would have failed on the one app that needs it.
The bridge now has its own config.oauth_parent_controller (default
ActionController::Base). Enabling it changes nothing about the
transport; a host can point it at ApplicationController for branding.
2. Building that controller unconditionally constantized its parent on
every host, which would pull view machinery into an API-only app that
never enables the bridge and break a non-Rails host outright. It is
now built only when config.oauth_bridge?, matching its routes.
Also pins coexistence, which was the question that surfaced this: the
end-to-end spec now boots a host that ALREADY serves OAuth at the
conventional top-level /oauth/{authorize,token,token/info} (the shape
Doorkeeper draws) with those routes drawn FIRST, and asserts they answer
untouched; that the bridge claims nothing at host level beyond the two
.well-known metadata documents; that every bridge endpoint stays under
the engine mount; and that the transport keeps its ActionController::API
parent while the page still renders.
552 specs across 3 orderings, rubocop and brakeman clean.
An exact-match allowlist makes a legitimate client rejected over an invisible difference (a trailing slash, a changed callback path) look identical to an attack, leaving an operator to guess the right string from an opaque 400. Log the offered value and the configured allowlist so the first failed connection names what to fix. It is the client's own public callback, never a credential. Writing the test for it surfaced that mcp_oauth_request_problem ran twice per request (once in the guard, once as the render argument) — harmless while it was pure, a double log now. Bound it to a local instead.
The bare well-known paths are ORIGIN-GLOBAL: /.well-known/oauth-authorization-server
means "the authorization server of this whole origin". Claiming it from an MCP
server bolted onto a host that already runs an unrelated OAuth provider (a REST
API's, say) asserts something false about the origin, and permanently spends a
namespace that is the pre-existing provider's to claim. Nothing consumed it, so
this was not breaking anything — but it was the wrong thing to own, and the
failure mode it invites is bad: if both ever answered, a client could bind to the
wrong authorization server silently rather than 404 loudly.
The RFCs already solve this and say so outright. RFC 8414 §3.1 inserts the
issuer's path into the well-known URL specifically so that "Using path components
enables supporting multiple issuers per host"; RFC 9728 §3.1 does the same for
protected resources. The MCP authorization spec (2025-11-25) goes further and
requires a client given a path-ful issuer to try ONLY the path-inserted forms —
there is no root fallback — so a compliant client never touches the origin root.
So the issuer is now the MCP endpoint URL itself (path-ful), and both documents
hang off it: /.well-known/oauth-{protected-resource,authorization-server}/mcp.
Issuer path == resource path, which also keeps a client deriving the same URL
whether it builds candidates from the issuer or the resource. PRM discovery does
not rely on any of this: WWW-Authenticate states the location outright and the
client fetches it directly.
A host mounted AT its origin root (a dedicated MCP domain) has no path to insert
and still gets the bare paths, which is correct there — it really is that origin's
only authorization server. Set oauth_resource_path = "/".
The end-to-end spec now asserts BOTH bare paths 404 on a host running its own
OAuth, alongside the existing coexistence checks. 557 specs across 3 orderings,
rubocop and brakeman clean.
fullstack-lint: the concern was 49% comment lines on added code. Two of them were also stale after the path-scoping change, still naming the bare well-known paths the bridge deliberately no longer serves — worse than noise, since a wrong comment outlives the reader's trust in the right ones. Most of the rest was duplication, not documentation: the endpoint list is in config/routes.rb, the README and the CHANGELOG; the path-scoping rationale now lives on Configuration#oauth_protected_resource_path, next to the code that implements it; the parent-controller reasoning lives on oauth_parent_controller. Restating all three in a 75-line header means four copies to keep true. What's kept is what a reader can't get elsewhere: that this is an envelope and not a half-built identity provider, that the stubs are deliberate, and the two invariants whose removal would be a vulnerability. 49% -> 29%, no information lost.
Two things, in the same files. ## The security review's findings The two load-bearing controls held under an adversarial review (the redirect allowlist survived eight open-redirect evasion variants; PKCE resisted downgrade and bypass on both legs), so these are the periphery around them. - Serve both metadata documents `Cache-Control: no-store`. They name the `authorization_endpoint` an operator is sent to and are built from `request.base_url`, which honours `X-Forwarded-Host` — so a shared cache holding one could hand every client an origin an attacker chose, with our own document vouching for it. Rails leaves `config.hosts` empty in production by default, so a host cannot be assumed to reject the forged header; the README now says to pin it. Asserted through real Rails, since Rails writes its own Cache-Control on commit and a fake controller could only confirm the fake. - Withhold `WWW-Authenticate` when the request origin contains a quote. It is interpolated into a quoted-string parameter, and `base_url` is caller-influenced, so a quote would close the quotes and let a caller append auth-params of their own. - Seal what an authorization code parks in the cache: key the entry by the code's SHA256 and encrypt the payload under a key derived from the code, which never lands in the store. What sits there is not the short-lived credential an authorization server would normally hold — it is the operator's pre-existing, long-lived, full-scope token, in what is documented to be the host's shared `Rails.cache`. A dump now yields ciphertext with no key, and a payload swapped in a writable cache does not decrypt. - Require a `token_authenticator` for `oauth_bridge?`. The bridge verifies the pasted token through it on both legs, so without one an operator pasted their token and got a 500. Drawing no route beats an authorization page that cannot work; the sibling introspection endpoint already fails safe this way. - Make a code single-use by the DELETE rather than the read, so of two concurrent redemptions exactly one proceeds. The race was never exploitable — both return the same token and both need the verifier — but the comment claimed an invariant the code did not keep. Also corrects the concern's "no cookie, no session" note: the GET leg does set one (`form_tag` emits an authenticity token). Nothing reads it back, so the claim it supports still holds. ## Native clients: `config.oauth_allow_native_client_redirects` An exact-match allowlist supports exactly the clients it names, which is not "any MCP client". But it cannot simply be opened up: the page is served from our OWN origin under our own certificate and asks for a live token, so an unvetted `redirect_uri` makes it a credential-phishing page we host — an attacker sends an authorize link carrying their own `code_challenge`, the operator pastes, and the code goes to the attacker, who redeems it with the verifier they chose. PKCE cannot help; they own the verifier. That attack needs the code to reach a REMOTE attacker, and that is the seam. A loopback address resolves on the operator's own machine and a private-use scheme is handed to a locally registered app, so neither can carry a code off the device — which is why RFC 8252 lets native apps skip pre-registration (§7.3 for loopback, whose ephemeral port could never be named ahead of time; §7.1 for schemes). So: loopback and private-use schemes are permitted generically by the new switch, a remote `https://` callback stays allowlist-only whatever it is set to, and every MCP client on an operator's machine connects with no config. Targets are judged on the PARSED URI, never the string — `http://127.0.0.1@evil.example/` has host `evil.example` and is remote, as is `http://127.0.0.1.evil.example/` — and a fragment, an opaque URI, or a `javascript:`/`data:`/`file:` scheme is refused. Off by default: switching it on is a decision, so it is an opt-in signal in its own right and can enable the bridge without an allowlist entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A second review (Codex) found the native-redirect rule unsound, and it was
right. `mcp_oauth_native_redirect_uri?` treated every scheme absent from a
13-entry denylist as local, so all of these were accepted:
ssh://attacker.example/cb
gopher://attacker.example/cb
ldap://attacker.example/cb
telnet://attacker.example/cb smb://… nfs://…
Every one names a REMOTE host. That falsifies the invariant the feature was
justified by and documented with — "these deliver the code to the operator's own
device" — and it is a denylist doing an allowlist's job: the only way to separate
a private-use scheme from a registered network one is to enumerate the IANA
registry, so the list is always the ones you happened to think of.
The mistake underneath was conflating two cases that are not alike. **Loopback
must be accepted unnamed because it cannot be named**: the client picks an
ephemeral port at runtime, so no allowlist could enumerate it — that is the whole
reason RFC 8252 §7.3 exists. A **private-use scheme has no such forcing reason**:
`cursor://anysphere.cursor-retrieval/oauth/callback` is a fixed string, so it can
just go in `oauth_allowed_redirect_uris` like any other entry. Accepting schemes
generically bought nothing and cost the invariant.
So the rule is now: name every target exactly, except loopback.
- `oauth_allow_native_client_redirects` -> `oauth_allow_loopback_redirects`. The
old name promised more than is safe to deliver; renaming is free while 0.6.0 is
unreleased.
- `RESERVED_REDIRECT_SCHEMES` is gone. Nothing is judged local by its absence
from a list.
- An allowlisted `cursor://` client still works, and is spec'd — the rule is
"name it", not "no desktop clients".
Rejecting the reviewer's suggested fix (require a reverse-domain scheme, per
RFC 8252 §7.1): it would reject `cursor://`, `vscode://` and `zed://`, which are
exactly the clients the feature exists for, while still trusting any scheme that
happens to contain a period.
Two more from the same review, both correct:
- The token response now carries `Cache-Control: no-store` + `Pragma: no-cache`.
RFC 6749 §5.1 makes both a MUST for any response carrying a token, and the
metadata documents already had no-store — omitting it on the response that
actually contains the token was the wrong way round.
- `POST oauth/authorize` answers 303, not Rails' default 302. That POST carried
the operator's token in its body, and only 303 unambiguously tells the browser
to fetch the callback with GET and no body (RFC 9700 §4.12).
587 examples, 0 failures across 3 random orders. The real-Rails E2E now drives
`ssh://`/`gopher://`/`ldap://` and an unnamed `cursor://` (all 400), the 303, and
the token response's headers — through the full stack, since a fake host could
only ever confirm the fake.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
Second review (Codex): 3 of 4 fixed in
|
…ound Four independent reviewers over this branch. No exploitable vulnerability: PKCE matches the RFC 7636 vector and survived 17 bypass attempts, the loopback rule survived a differential fuzz (3,586 candidates, Ruby decides / WHATWG URL adjudicates the emitted Location — 672 accepted, 0 escapes), key derivation has no length-extension path, single-use holds across MemoryStore/Redis/MemCache. What they did find was a set of claims this branch makes that the code did not keep. Each is fixed here rather than reworded, except where the code was right and the prose was wrong. ## The code was the key, and Rails logs the code The payload was sealed under `SHA256(prefix + code)`. But Rails logs an authorization code twice per flow at INFO — `Redirected to ...?code=...` (only `config.filter_redirect` touches that line) and the token endpoint's `Parameters:` (no stock `filter_parameters` entry matches `code`). So the key to the cache entry was sitting in the log: an artifact more widely read, longer retained and more replicated than the 60-second entry it protected. "A dump of that store yields ciphertext whose key never landed in it" was true and beside the point. Worse, it was two locally-defensible decisions that combine badly: I made the code a decryption key, then separately decided a logged `code` was harmless because it is single-use and already spent. Both true in isolation. Together they handed the key away. Now HMAC'd from `config.oauth_signing_secret` AND the code. The secret defaults to the Rails app's `secret_key_base` — already the "server-held, in ENV, never logged" value this wants — so a Rails host configures nothing and a non-Rails host must set one (`oauth_bridge?` refuses to run without it, rather than sealing with something weaker). ## The serializer was never pinned The comment claimed "cipher AND serializer are pinned rather than inherited", naming Rails-config-dependence as the hazard. Only `cipher:` was passed. Every default across the supported ActiveSupport range reaches `Marshal.load` — `:marshal`, and 7.1+'s `:json_allow_marshal` — so a host with cache-write access who knows one code (they run the flow with their own token) could forge a valid AEAD blob wrapping a Marshal gadget and get code execution. Narrow, but cache compromise is the exact threat the sealing exists for. Pinned to NullSerializer: the payload is already a JSON String, so JSON.parse is now the only parser that ever sees it, and the blob shrinks 140B -> 128B. ## My own quickstart would break a real deployment `cache_store` defaults to an in-process MemoryStore. The README's OAuth quickstart never set it, while the code asserted it "is documented to be the host's shared Rails.cache" — a `should` restated as an `is`. On a multi-worker Puma the two legs land on different workers, so the flow fails (N-1)/N of the time, intermittently, AFTER the operator has pasted a live token. That is exactly what `oauth_bridge?` gates on `token_authenticator` to prevent; I applied the principle to one dependency and skipped the one that fails silently. Warned once at boot, not gated: a MemoryStore is correct in a single process, and `Rails.cache` IS one in a stock development environment — gating would make the bridge undevelopable locally to prevent a production mistake. The quickstart now sets it. ## Three more - Allowlist entries are validated at assignment, like `filter_operator_overrides=` and for the same reason. An opaque URI (`com.example.app:cb` — legal syntax, and the docs invite private-use schemes into this list) cannot carry a query, so `URI#query=` raised — a 500 after the token was verified and a code cached. Now an ArgumentError at boot, naming the fix. - The bridge's four routes take `format: false`, as the metadata routes already did. Without it `/mcp/oauth/authorize.json` reached the action, found no JSON template and 500'd unauthenticated. - `code`/`state` are SET, not appended. A loopback redirect_uri is not exact-matched, so a caller can pass `?code=` themselves and get `?code=theirs&code=ours`, leaving the winner to the client's parser. The callback URL is also now built from the validated string rather than parse-and-reconstructed, which is what raised on opaque URIs. - The two browser legs are guarded by a `before_action`, not a check in each action body. `authorize` is a common method name; a gem defining one on ActionController::Base would drop the action from Rails' `action_methods`, and Rails would then serve authorize.html.erb by implicit render — skipping a body guard and showing an attacker's redirect_uri a paste page. ## Claims corrected rather than code changed "A real authorization server blocks this with a consent screen plus an authenticated session; this allowlist is what compensates" was wrong twice. The allowlist IS the redirect-URI registration a real AS does — it compensates for nothing extra. And consent would not block that attack anyway: the operator wants to connect the client and would click Allow. What the allowlist does not cover is now stated plainly: it binds which URL a code goes to, not whose session at that URL receives it, so a multi-tenant client's `state` handling (RFC 6819 §4.4.1.7) is load-bearing and outside this gem. A full authorization server has the identical exposure. `config.hosts` is now imperative, not "expected": Rails leaves it EMPTY in production, so the mitigation the comment leaned on is one Rails does not give you. ## Verification 598 examples, 0 failures across 3 random orders on all three CI rubies (3.2.5/3.3.11/3.4.9 — the floor matters again: OpenSSL::HMAC, NullSerializer). RuboCop 64/0, Brakeman 0. The unit spec's fake controller grew a real before_action chain — without one it would have kept passing while the guard was gone. Real-Rails E2E now also proves secret_key_base resolves with no config, the .json suffix 404s, and a polluted loopback callback emits exactly one code while keeping the client's own query. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fullstack-lint. CI linters were already green with their exact flags (rubocop 64/0; brakeman --force --no-progress --quiet --no-pager 0/0; rspec 598/0). This is the judgment half. The concern had drifted to 38% comments — the same file the last lint round cut to 29%, and the same cause: duplication rather than documentation. - `mcp_oauth_loopback_redirect_uri?` carried a 17-line restatement of `oauth_allow_loopback_redirects`' rationale. That argument belongs where the setting lives; here it is now a pointer plus the parsing detail, which is the only part this file alone can say. - `mcp_oauth_encryptor` had 22 comment lines over 6 of code, re-explaining why the signing secret exists — which is `Configuration#oauth_signing_secret`'s job. Kept: why HMAC, why no KDF, and why NullSerializer specifically, since that one reads like a redundant option and is the reason Marshal never sees the payload. - Dropped a comment that had already gone stale within this branch: it justified building the callback URL by string partly because "a host may legitimately allowlist an opaque URI" — untrue since the same round made that an ArgumentError at config time. Exactly the failure mode this round exists to fix, so it does not get to ship. Down to 34%, over four more methods than the file had. RSpec pass clean: no anonymous subjects, no `create(` (the suite is Rails-free), no ticket references or commented-out code in the diff. View rules N/A — the template carries no JS, and simple_form is deliberately not a dependency of a dependency-light gem. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three independent reviewers plus Codex over 1d0b248. None found a token-theft primitive; the redirect policy, PKCE, single-use and the sealing all held. One reviewer built a differential fuzzer (Ruby decides, WHATWG URL adjudicates the emitted Location): 3,586 candidates, 0 escapes. The hand-rolled callback builder and the new key derivation — the two things most likely to have broken — are the two that hold up best. Every remaining defect was one class: **validation I wrote that could be skipped, bypassed, or that closed one instance of a hole and not its twin.** Fixing the class rather than the points. ## Validation that wasn't enforced - `oauth_allowed_redirect_uris` was an `attr_reader` on a mutable array, so `<<` slipped an unchecked entry past the setter entirely. Not cosmetic: a `#frag` entry emits `…/cb#frag?code=…`, which a browser reads as HASH — the code never reaches the client's server and is readable by any script on the page. The list is now frozen. - `oauth_signing_secret=` was a bare `attr_writer`. An Integer passed `oauth_bridge?`, drew the routes, and raised TypeError out of OpenSSL::HMAC at request time — after the operator pasted a live token. That is the exact failure the allowlist setter was added to prevent, in the same commit. It now validates type and a 32-byte minimum. ## Validation that stopped short - The allowlist accepted `http://attacker.example/cb` (cleartext remote — the code on the wire) and `javascript:`/`data:`/`file:` (a browser treats them as script or local content). Both now raise. Naming bad schemes is sound HERE and nowhere else, which is worth being explicit about since this branch removed a scheme denylist for failing open: that one was request-time, where the ATTACKER picks the scheme. This list is what the HOST wrote — an attacker cannot add to it — so a denylist is a config lint, not a boundary. `LOOPBACK_HOSTS` moved to McpToolkit::Oauth so the two paths cannot drift. - `format: false` closed `/mcp/oauth/authorize.json` but `Accept: application/json` picks the format just as well and still raised MissingTemplate. Renders now pin `formats: [:html]`. I closed the vector a reviewer named and left its twin. - `filter_parameters` was a README instruction. The bridge takes a live token in a POST body and Rails logs params at INFO, so this is the gem's business: an engine initializer now adds `access_token` + `code_verifier`. The stock `:token` entry covering `access_token` is a `rails new` default the gem does not own. ## Portability, found only because someone checked the floor `status: :unprocessable_content` raises below Rack 3.1, and the gemspec pins no rack/actionpack floor. On any Rails 7.x host a mistyped paste became an unauthenticated 500 on the one page whose job is to say "that token isn't valid". Neither symbol spans the supported range (`:unprocessable_entity` is deprecated above 3.1), so it is the integer. The suite could not see this: the unit spec asserts the symbol against a fake `render` that never resolves it, and the E2E pins rails ~> 8.0. ## PKCE shape (Codex) RFC 7636 §4.1 shapes are now enforced on both the challenge and the verifier. Worth being honest about what this does NOT do: the challenge is a 43-char digest whatever the verifier was, so a client that picked a one-character verifier is indistinguishable here — it has defeated its own PKCE and we cannot see it. This is enforced because a public gem should not quietly accept what the spec forbids. The gem's own test verifiers were 35 and 36 chars and are now legal. The ordering matters and is deliberate: shape is checked BEFORE the code is consumed, so a request that could never verify doesn't cost a client its code — but a well-formed WRONG verifier still burns it, which is the only thing stopping someone with an intercepted code from retrying verifiers. Both are now spec'd separately. ## Two more comments that claimed what the code doesn't do - The NullSerializer pinning was said to stop a writable cache becoming code execution. It doesn't: `Cache::Store` marshals the entry itself, so a writable cache is an RCE problem before the encryptor is reached. The pinning is right — it is determinism across AS 6.1–8.x — and the comment now says so. - The callback builder claimed the string form avoids "invent[ing] ways for the emitted URL to differ" — but it re-encodes the query anyway, deliberately, since that is where `code` gets stripped. Only the base is preserved byte-for-byte. Also: the guard now RAISES if the parent has no filter chain rather than silently skipping — same shape as the denylist this branch removed for failing open. 607 examples, 0 failures across 3 random orders on all three CI rubies. RuboCop 65/0, Brakeman 0 (CI's exact flags). Real-Rails E2E now pins the Accept header, the 422, the frozen allowlist and the scheme lint — the unit fake can resolve none of them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verified against a real app: a stock test env's secret_key_base is ~15 bytes, under the 32-byte minimum the setter enforces. The asymmetry is intended — the minimum catches a placeholder typed into THIS setting, whereas secret_key_base is Rails' own (128 chars in a real deployment) and a weak one is an app-wide problem, not this gem's to police. But an asymmetry nobody explains reads as an oversight, and this branch has spent four rounds on comments that claimed more than the code did. Says it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Why
Hosted MCP clients (Cowork, claude.ai, Claude Desktop — one shared backend) authenticate exactly one way: discover an authorization server, run authorization-code + PKCE in a browser, use the
access_tokenthat comes back. The MCP authorization spec (revision 2025-11-25) also makes a token in the request URI aMUST NOT, so?token=…is not a fallback for them.That leaves a server whose tokens are issued out-of-band — an admin UI, a CLI, a support process — unreachable by those clients. This makes it reachable without changing how tokens are issued.
What it is
A standards-shaped envelope around tokens the host already issues. Not an identity provider. The authorization page asks an operator to paste a token they already hold, and the
access_tokenreturned is that token, verified through the sameconfig.token_authenticatorthe transport uses. Scopes, expiry, revocation and tenancy stay with the host. No new way to obtain a token exists.The whole authorization-server state is one cache entry with a 60s TTL. No tables, no migrations.
Deliberately absent (none of it gates anything here): client registration returns an identifier and stores nothing — no endpoint reads a
client_id; there is no consent step, because pasting a token you hold is the grant; no refresh token, because the pasted token's own expiry is the real lifetime.Deliberately not faked (either would be a real vulnerability, not a skipped ceremony):
redirect_uriis exact-matched againstconfig.oauth_allowed_redirect_urison both legs — the hidden form fields are tamperable, so checking only the GET would not bind the POST. Without this it is an open redirect handing out authorization codes.code_verifieris verified constant-time against the stored S256 challenge.It claims nothing origin-global
This matters for any host already running an OAuth provider (a REST API's Doorkeeper, say) on the same origin.
The flow endpoints live under the engine's mount (
<mcp>/oauth/*), so a top-level/oauth/*provider keeps every route. And the metadata is path-scoped —/.well-known/oauth-protected-resource/mcp, never the bare/.well-known/oauth-authorization-server. The bare paths are origin-global: they mean "the authorization server of this whole origin", which belongs to that pre-existing provider.This is the RFCs' own design, not a workaround. RFC 8414 §3.1: "Using path components enables supporting multiple issuers per host." And MCP 2025-11-25 requires a client given a path-ful issuer to try only the path-inserted URLs — there is no root fallback. So the issuer is the MCP endpoint URL itself.
A host mounted at its origin root (a dedicated MCP domain) sets
oauth_resource_path = "/"and gets the bare paths, which is correct there.Opt-in and authority-only
config.oauth_bridge?requires the authority role and a non-empty redirect allowlist — it cannot run without bounds on where codes may go, and a satellite (whose tokens belong to its central app) never draws it. Default off: routes undrawn, controller not even built, 401 byte-identical to 0.5.0.Config
oauth_allowed_redirect_uris[]— non-empty is what enables the bridgeoauth_resource_path"/mcp"— must match the engine's mount pointoauth_authorization_code_ttl60oauth_parent_controller"ActionController::Base"oauth_parent_controlleris separate fromparent_controllerby design: the transport is a JSON-only endpoint a host rightly points atActionController::API, which cannot render an HTML view — and the authorization page is one. Enabling the bridge changes nothing about the transport.Hosts add one line at the top level of their route set (a
/.well-known/*path can't be drawn by an engine mounted under a path):Verification
557 specs / 0 across 3 random orderings, on all three CI rubies (3.2.5, 3.3.11, 3.4.9). RuboCop 64 files / 0, Brakeman 0. No host or vendor fingerprints in the diff.
The load-bearing one is
spec/mcp_toolkit/oauth/bridge_end_to_end_spec.rb, which boots a real Rails app in an isolated child process (theengine_route_reload_specpattern) and walks the client's exact sequence — a fake controller would only confirm the fake. It asserts:WWW-Authenticate(without it a client never starts the flow at all);ActionController::APIparent;/oauth/{authorize,token,token/info}— drawn first, the order that would expose a collision — answers untouched;Known limits
app/views/mcp_toolkit/oauth/authorize.html.erb.🤖 Generated with Claude Code
Update (
8e24115): security review + native MCP clientsTwo things landed after the review above, both amending the unreleased 0.6.0 entry rather than adding a 0.6.1.
The security review's findings
An adversarial review attacked the two load-bearing controls and they held — the redirect allowlist survived eight open-redirect evasion variants (
callback/../../evil, userinfoclient.example@evil.example, encoded-slash, trailing slash, case-shiftedHTTPS://, port-suffix, array-typed params), and PKCE resistedplaindowngrade and missing/empty/wrong verifiers on both legs. XSS clean; the login-CSRF variant does not work. What follows is the periphery around them:Cache-Control: no-store. They name theauthorization_endpointan operator is sent to and are built fromrequest.base_url, which honoursX-Forwarded-Host— so a shared cache holding one could hand every client an origin an attacker chose, with our own document vouching for it. Rails leavesconfig.hostsempty in production by default, so a host cannot be assumed to reject the forged header; the README now says to pin it. Asserted through real Rails, since Rails writes its own Cache-Control on commit and a fake controller could only have confirmed the fake.WWW-Authenticateis withheld when the request origin contains a quote. It is interpolated into a quoted-string parameter andbase_urlis caller-influenced, so a quote would close the quotes and let a caller append auth-params of their own.Rails.cache. A dump now yields ciphertext with no key; a payload swapped in a writable cache does not decrypt.oauth_bridge?now requires atoken_authenticator. Without one, an operator pasted their token and got a 500. Drawing no route beats an authorization page that cannot work — the sibling introspection endpoint already fails safe this way.Native MCP clients:
config.oauth_allow_native_client_redirectsAn exact-match allowlist supports exactly the clients it names, which is not "any MCP client". But it cannot simply be opened up, and it is worth being precise about why: the page is served from the host's own origin under its own certificate and asks for a live token, so an unvetted
redirect_uridoes not merely add an open redirect — it makes the host's own domain a credential-phishing page. An attacker sends the operator an authorize link carrying the attacker'scode_challenge, the operator pastes, the code goes to the attacker, and they redeem it with the verifier they chose. PKCE cannot help; they own the verifier.That attack needs the code to reach a remote attacker, and that is the seam:
https://client.example/cb)oauth_allowed_redirect_urishttp://127.0.0.1:*,localhost,[::1])oauth_allow_native_client_redirectscursor://…,com.example.app:/cb)oauth_allow_native_client_redirectsSo every MCP client on an operator's machine connects with no configuration, while a hosted client still needs one allowlist entry. Targets are judged on the parsed URI, never the string —
http://127.0.0.1@evil.example/has hostevil.exampleand is remote, as ishttp://127.0.0.1.evil.example/— and a fragment, an opaque URI, or ajavascript:/data:/file:scheme is refused. Off by default, so it is an opt-in signal in its own right and an unconfigured host stays byte-identical to 0.5.0.Verification
MessageEncryptorwith AES-GCM,pack("m0"), and spawns a child interpreter). RuboCop 64/0, Brakeman 0.no-storeassertion./oauth/*routes untouched, bare origin-global well-known unclaimed, page renders under anActionController::APItransport, loopback accepted,127.0.0.1@evil.examplerefused.