MCP OAuth Stage 2: well-known PRM, AS metadata, JWKS endpoints#90
Merged
Conversation
Stage 2 of MCP OAuth support (issue #86). Mounts three discovery documents under /.well-known/* so MCP clients (Claude Desktop, Cursor, mcp-remote) can find the authorization server, the supported methods, and the public keys to verify tokens against. - /.well-known/oauth-protected-resource — RFC 9728 PRM document. Becomes the entry point once Stage 5's withMCPAuth issues 401 + WWW-Authenticate pointing here. - /.well-known/oauth-authorization-server — RFC 8414 AS metadata. Advertises authorize / token / register / JWKS endpoints, S256-only PKCE, code-only response type, both client secret and public-client auth methods, RS256 + EdDSA signing algs, RFC 8707 resource parameter support. - /.well-known/jwks.json — placeholder returning an empty key set until Stage 4 generates and persists the signing keypair in the harper_oauth_mcp_keys table. All three documents include Access-Control-Allow-Origin: * so browser- based MCP clients can fetch them cross-origin (Gemini's blocker on the draft of this PR — adding it before push). Two new MCP config fields: mcp.issuer (authorization-server identity) and mcp.resource (canonical resource URI for RFC 8707 audience binding). Both default to derive-from-request, but the issuer derivation trusts the Host header and so MUST be pinned in production once Stage 4 starts signing tokens with `iss` — documented in docs/configuration.md. Handlers registered via Harper's server.http(handler, { urlPath }) middleware. urlPath matching is prefix-based with segment-boundary awareness, so each handler additionally exact-checks request.pathname to reject sub-paths. Config is read at request time through a getter so options changes apply without re-registering routes (Harper's server.http has no deregistration mechanism). 552 unit tests pass locally (+25 from this PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Reviewed; no blockers found. |
|
Reviewed; no blockers found. |
This was referenced May 21, 2026
heskew
commented
May 21, 2026
Member
Author
heskew
left a comment
There was a problem hiding this comment.
🔍 Deep Review of OAuth PR #90: MCP Discovery Endpoints
I have conducted a thorough review of the changes in PR #90 (mcp/well-known-metadata), which implements Stage 2 of the MCP OAuth integration (RFC 8414, RFC 9728, and JWKS placeholders). The implementation is exceptionally clean, robust, and highly spec-compliant.
Here is a breakdown of the key strengths and design decisions in this PR:
1. ⚡ Dynamic Config & Hot-Reload Support
- Design Decision: The use of a getter function
() => OAuthResource.mcpConfigwhen registering middleware handlers is a brilliant design pattern. - Why it matters: Harper's
server.httprouter does not support endpoint deregistration. Passing a live config-resolver closure ensures that configuration updates (such as changing themcpconfiguration block or togglingenabled) take effect instantly at request time without duplicate or stale route registrations.
2. 🛡️ Defensive Prefix-Routing Guard
- Design Decision: In
makeHandler(), checkingreq.pathname !== match.exactPathbefore serving metadata is an excellent defensive measure. - Why it matters: Harper's
urlPathrouter matches prefixes at segment boundaries. Without this exact-path check, sub-paths like/.well-known/oauth-protected-resource/extrawould have erroneously matched and been served. This guard ensures they fall through correctly (e.g., to a 404).
3. 🌐 Complete CORS Support
- Design Decision: Adding
Access-Control-Allow-Origin: *andAccess-Control-Allow-Methods: GET, OPTIONSto all discovery documents. - Why it matters: This ensures browser-based MCP clients (and browser extension developer tools) can successfully fetch discovery metadata without origin restrictions. Since these documents contain entirely public metadata and do not require user credentials, a wildcard CORS origin is secure and fully appropriate here.
4. 🔒 Proactive Security Guarding (Issuer Resolution)
- Design Decision: Resolving the issuer dynamically using the request's
Hostandprotocolheaders with clear security warnings. - Why it matters: Dynamic resolution is excellent for development (e.g., local development on
localhost), but in production, trusting the client-controlledHostheader can lead to host-header spoofing. You have explicitly warned about this risk inresolveIssuerand added prominent, strong guidance indocs/configuration.mdto pinmcp.issuerin production. This is highly responsible engineering.
5. 📖 Spec-Compliant Metadata Advertising
- RFC 8414 Compliance: Correctly advertising
code_challenge_methods_supported: ["S256"]enforces secure PKCE (no insecureplainmethod allowed). - RFC 8707 Compliance: Correctly advertising
resource_parameter_supported: trueflags downstream support for resource indicators on token requests. - JWKS Placeholder: The empty JWKS placeholder
{ keys: [] }is perfectly aligned with the phased approach (to be populated in Stage 4 once signing keys are implemented).
6. 🧪 Comprehensive Unit Tests
- The 25 newly added tests in
test/lib/mcp/wellKnown.test.jsprovide robust coverage. They thoroughly test fall-throughs, path-prefix matching, localhost fallbacks, and CORS presence across all well-known handlers.
🏁 Verdict
This PR is Approved / LGTM! The code quality is top-tier, and the architecture is well-prepared for the subsequent stages (Stage 3 authorize and Stage 4 token endpoints).
🤖 Posted by Antigravity on Nathan's behalf
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
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.
Closes #92. Sub-issue of #86 (parent roll-up).
Summary
Stage 2 of MCP OAuth support. Mounts three discovery documents so MCP clients can locate the authorization server, see what it supports, and (eventually) verify the tokens it issues:
/.well-known/oauth-protected-resource(RFC 9728 PRM)/.well-known/oauth-authorization-server(RFC 8414 AS metadata)/.well-known/jwks.json(placeholder, empty key set until Stage 4)Why
Without these documents an MCP client has no way to discover the authorization server from a 401 response — the discovery contract is the entry point to the entire MCP OAuth flow. The PRM is what
withMCPAuth(Stage 5) will point at viaWWW-Authenticate: Bearer resource_metadata="...". The AS metadata is what the client reads to find/oauth/mcp/register(already shipped in #89),/oauth/mcp/authorize(Stage 3),/oauth/mcp/token(Stage 4), and/.well-known/jwks.json.Where to look
src/lib/mcp/wellKnown.ts— three builder functions + a registration helper. Note the getter pattern formcpConfig: handlers read current config at request time so live config changes apply without re-registering routes (Harper'sserver.httphas no deregistration). Each handler also exact-checksrequest.pathname— Harper'surlPathis prefix-matched, so a request to/.well-known/oauth-protected-resource/extrawould otherwise reach the handler and be served.src/index.ts— one call toregisterWellKnownHandlers(server, () => OAuthResource.mcpConfig, logger)afterupdateConfiguration(). Routes mounted once at boot.src/lib/mcp/wellKnown.ts:resolveIssuer— comment explicitly flags that the request-derived issuer trusts the Host header and MUST be pinned viamcp.issuerin production once Stage 4 signs tokens withiss. Also surfaced indocs/configuration.mdwith strong wording.code_challenge_methods_supported: ["S256"](S256 only, noplain),id_token_signing_alg_values_supported: ["RS256", "EdDSA"](both, per the Stage 4 plan),resource_parameter_supported: true(RFC 8707),token_endpoint_auth_methods_supportedincludesnonefor public clients.What was reviewed
Gemini CLI 0.42.0 reviewed the diff. Six items surfaced; two acted on before push:
Access-Control-Allow-Origin: *+Access-Control-Allow-Methods: GET, OPTIONSto all three documents. Browser-based MCP clients can now fetch cross-origin. Test coverage added.resolveIssuerand a "STRONGLY recommended in production" notice indocs/configuration.md. Stage 4 will need this pinned to prevent attacker-influencedissclaims on signed tokens.Three dismissed: localhost scheme default (Harper's
request.protocolis socket-derived and reliable in production), JWKS algorithm-advertise vs empty content mismatch (acknowledged by Gemini as acceptable; Stage 4 populates), localHarperRequestinterface redundancy (the type is narrow and accurate for this surface — unifying with the session-bearingRequestwould actively reduce clarity).Test plan
mcp-remoteor Claude Desktop hitting the discovery chain is the conformance proofOut of scope
/oauth/mcp/authorize— Stage 3/oauth/mcp/token+ JWT signing — Stage 4withMCPAuthwrapper that issues the401 + WWW-Authenticatepointing at PRM — Stage 5harper_oauth_mcp_keys🤖 Generated with Claude Code