Implement OAuth discovery through 401 challenges and streamline MCP - #1447
Conversation
The discovery documents were there and nothing ever fetched them. An MCP client learns a server supports OAuth from a 401 carrying WWW-Authenticate with the resource metadata URL; without that challenge the client has no reason to look, so the standard way of connecting — the one Claude and Cursor use — silently did not work. /mcp now answers 401 with that header when a call names a tool that needs an account and carries no credentials. Only those tools challenge: a blanket 401 would put news and weather behind an account, which is the opposite of the point. Whether a tool needs auth is derived, not declared again. HandleAuth and AccountOnly cover most of it; path-backed tools like mail_inbox authenticate inside their own handler and set neither, so the check falls through to whether the service behind the tool is Scoped — which the Spec already says once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdcPjN9ndJwMGKQSRrAGPE
Two ways to pay and two protocols to call meant every reader had to choose before they could start, and neither choice was the product. The product is: point an MCP client at /mcp. x402 comes off the surface. The landing explained a payment protocol where it should have explained how to connect; /tools offered paying in USDC as an alternative to signing in; /mcp rendered a comparison table between crypto and card. All of it is now one story — add the endpoint, sign in when asked, calls draw credits. Prices render in credits, from wallet.GetOperationCost, so the one number a caller sees is the one they are charged. The x402 implementation is untouched and still settles for anyone using it; it is no longer a thing to learn to get started. /api is a redirect to /mcp. It documented the same tools over REST with their own auth story and their own price table, which is a second door to maintain and a second decision to make. The REST paths still answer; restTools moves to api.go so the surfaces-do-not-mix test still holds with nothing rendering them. Connecting leads with the MCP authorization spec, which was already implemented and never advertised: the 401 carries WWW-Authenticate, the two discovery documents are served, and Claude Desktop and Cursor walk themselves through sign-in from there. A token at /token is the fallback for clients that can't, not the headline. Separately: /docs/about, /docs/installation and /docs/mcp printed their name twice. The markdown files open with an H1 because they are read on GitHub too, and the page shell already renders doc.Title above them. stripTitle drops the leading H1 at render, so the catalogue entry is the one title and the files stay correct on their own. The connect assertion reads connectSection directly rather than the rendered page: the tool tiles carry whatever the tools say about themselves — `pay` really does settle in USDC on somebody else's server — and a page-wide assertion would have passed here while failing in the running binary, where main.go has registered them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdcPjN9ndJwMGKQSRrAGPE
Testing with a real token found wallet_balance returning 20KB of rendered HTML — the whole wallet page — to a caller that asked for the balance. /wallet answered JSON only behind ?balance=1, and the tool declared that flag as an optional param, so an agent calling a tool named "wallet_balance" got a web page unless it guessed. The handler now honours Accept: application/json, which the tool dispatcher sets on every path-backed call, so this is fixed for anything else routed that way too. The param is gone. The rest is the sweep: the /wallet page's "Crypto (x402)" card, the pricing hero, the installation guide's "taking payments", and the config reference's framing all offered stablecoin as a way to call. They now describe the one thing a caller is charged in. The `wallet` and `pay` tool descriptions were actively wrong rather than just off-message. `wallet` claimed "this wallet pays for metered MCP tools" — it does not; calls here draw credits. `pay` said it works "on this server and any other", which invites an agent to route a local call through a payment rail. Both now say what is true: the wallet funds credits, and `pay` is for other people's servers. The USDC card stays on the signed-in /wallet page. It is a top-up rail for credits, not a second currency, and the balance is real money a user may already hold — removing the UI would strand it. It is no longer offered anywhere a person is deciding how to pay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdcPjN9ndJwMGKQSRrAGPE
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d0c603a63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // HTML returned to an agent that called a tool named wallet_balance. The | ||
| // tool dispatcher sets Accept: application/json on every path-backed call, | ||
| // so honouring Accept fixes it here and for anything else routed this way. | ||
| if r.URL.Query().Get("balance") == "1" || app.WantsJSON(r) { |
There was a problem hiding this comment.
/wallet
Because the MCP dispatcher sets Accept: application/json on every path-backed call, this condition intercepts every authenticated wallet route before the switch below. For example, wallet_transfer calls POST /wallet/transfer but receives a successful balance response without transferring anything, while wallet_topup returns the balance instead of handleTopupJSON; JSON clients calling conversion or checkout routes can be similarly diverted. Require path == "/wallet" for the Accept-based shortcut while retaining the legacy query behavior only where intended.
Useful? React with 👍 / 👎.
| // Path-backed tools authenticate inside their own HTTP handler, so | ||
| // neither flag is set on them — mail_inbox is one. Their service knows, | ||
| // though: a scoped service is closed to callers with no account, and | ||
| // that is declared once in the Spec. | ||
| return service.AccountScoped(serviceOf(tools[i].Name)) |
There was a problem hiding this comment.
Using only the service-wide Scoped flag misses authenticated operations in mixed public/private services. For example, blog_create, blog_update, and blog_delete all enforce RequireSession in service/blog/blog.go, but the blog spec is intentionally unscoped because reads are public; stream_post and the standalone save/dismiss controls have the same problem. Unauthenticated MCP calls to these tools therefore return an ordinary JSON-RPC tool error instead of the HTTP 401 challenge, so clients configured without a token never discover or start the advertised OAuth flow.
Useful? React with 👍 / 👎.
| <li><b>When payment is due,</b> the endpoint answers <code>HTTP 402</code> with a price. Your agent's | ||
| <a href="https://x402.org">x402</a> wallet pays in stablecoin (USDC) and retries — sub-second, no account, no keys.</li> | ||
| <li><b>You pay the operator</b> running this instance, directly, wallet to wallet. No middleman.</li> | ||
| <li><b>Add the endpoint</b> to your MCP client: <code>` + host + `/mcp</code></li> |
There was a problem hiding this comment.
When MU_DOMAIN is unset and a deployment accepts a client-controlled X-Forwarded-Host—as a directly exposed instance or a proxy that forwards rather than overwrites it—app.BaseURL(r) includes that value verbatim. Concatenating it into trusted HTML lets a value such as </code><script>...</script> inject executable markup into the public landing page, and the current CSP permits inline scripts; escape host as HTML before inserting it.
Useful? React with 👍 / 👎.
2 participants
No description provided.