MCP OAuth Stage 3: /oauth/mcp/authorize endpoint (PKCE-S256)

[heskew]

Drafted by Claude (Opus 4.7, 1M context) on Nathan's behalf, following the harper-engineering-guidelines development lifecycle. Code, tests, the pre-push Gemini cross-review, and this description are all agent-authored; pushed under Nathan's account because the agent runs locally on his machine. Human review expected before merge.

Closes #93. Sub-issue of #86.

Summary

Stage 3 of MCP OAuth. Adds the user-facing authorize endpoint plus the surrounding plumbing — auth-code table with native TTL, bridge through the existing upstream IdP flow, callback branch that mints the code and redirects to the MCP client. No JWTs yet (that's Stage 4 / #94).

Where to look

• src/lib/mcp/authorize.ts — handleAuthorize with two-phase validation per OAuth 2.1 §3.1.2.5: client_id + redirect_uri must validate before any redirect; everything else routes errors to the verified redirect_uri. The interesting helper is redirectUriMatches — implements RFC 8252 §7.3 loopback port flexibility for 127.0.0.1 / [::1] / localhost. Native MCP clients bind dynamic ports at runtime and can't pre-register them.
• src/lib/handlers.ts — handleCallback restructured. CSRF state is now verified BEFORE the upstream error param is processed, so MCP-initiated errors (user denied at GitHub, etc.) route back to the MCP client's redirect_uri with OAuth-spec error codes instead of bouncing to the Harper app's default path. The legacy "no state, has error" UX is preserved for human-OAuth flows. On the success path, the onLogin-mapped username (hookData?.user ?? user.username) flows into the auth-code record, so Stage 4's JWT inherits the mapped identity.
• src/lib/mcp/callback.ts — the MCP branch from handleCallback. Mints the code with crypto.randomBytes(32).toString('base64url'), persists the binding fields (client_id, user, resource, PKCE challenge, redirect_uri, scope), redirects to the client URI. No Harper session is created on this branch (independent MCP/human lifecycle per Add MCP OAuth flow support #86 resolved decision). Asserts no upstream IdP token leaks into the redirect URL.
• src/lib/mcp/authCodeStore.ts — CRUD against the new mcp_auth_codes table. Explicit field access in encode/decode (CLAUDE.md tracked-object spread gotcha).
• schema/oauth.graphql — new mcp_auth_codes table with @table(database: "oauth", expiration: 300) for native TTL.
• src/types.ts — MCPAuthorizeState (carried through CSRF metadata) and MCPAuthCodeRecord. New mcp.providers config field.

Design decisions worth flagging

• Single-provider constraint for v1. mcp.providers (or, when unset, the full registry) must resolve to exactly one effective provider; 0 or >1 returns server_error. Multi-provider chooser UI is v1.1. Documented in the selectMCPProvider helper.
• resource param exact match. Validated against resolveResource(request, mcpConfig) from Stage 2. The plugin now requires mcp.issuer at startup when mcp.enabled (see Rebase + hardening below), so the issuer and audience can't be derived from the client-controlled Host header.
• Auth code storage. Single Harper table with expiration: 300 — 5-minute TTL is a safety net; Stage 4's /token will read-then-delete on exchange for one-time use.

What was reviewed

Gemini CLI 0.42.0 cross-reviewed the pre-push diff. Four real findings, all addressed before push:

1. RFC 8252 §7.3 loopback port flexibility — original code did exact-string match on redirect_uri, breaking native clients. Now uses port-flexible matching on loopback hosts. Tests added covering all loopback variants.
2. onLogin username mapping lost in MCP branch — auth code was binding the raw OAuth username, ignoring the mapped identity from the hook. Now passes hookData?.user ?? user.username through.
3. handleCallback error ordering — upstream IdP error was handled before CSRF verification, so MCP errors bounced to the Harper default path instead of the MCP client. CSRF verify is now first; legacy no-state UX preserved.
4. MCP-unaware error redirects — the cross-provider mismatch and catch blocks ignored tokenData.mcp and used originalUrl || postLoginRedirect (undefined for MCP flows). Both branches now route to the MCP client redirect_uri when mcpState is present.

One Gemini point (resource URI stability via Host header) was informational — already covered by Stage 2's documentation.

Rebase + hardening (2026-06-09)

Rebased onto current main (was ~10 commits behind — the alpha.2 release + dynamic-provider-cache work); clean, zero conflicts. Then ran a full cross-model review (Codex + Harper-domain pass; the Gemini leg was unavailable this run). No blockers. It surfaced one significant concern and one suggestion, both now fixed in this PR (commit 609b130):

• Require mcp.issuer when mcp.enabled (fail fast at startup). With neither mcp.issuer nor mcp.resource pinned, both were derived from the client-controlled Host header — letting a client influence the advertised iss and the aud bound into the auth code (audience confusion once Stage 4 signs tokens). A pinned issuer anchors both (resource defaults to <issuer>/mcp); a pinned resource alone is not sufficient — it leaves iss Host-derived. Codex caught that subtlety on the first cut.
• Reject a provider keyed mcp. The /oauth/mcp/* dispatcher reserves that path segment, so a provider named mcp would silently 404 at request time. Now rejected loudly at config load.

Deferred to Stage 4: client-supplied scope must be intersected against permitted scopes before it lands in a JWT (it's only stored, not consumed, in this PR).

Test plan

• 608 unit tests pass locally (npm test)
• npm run lint clean
• npm run format:check clean
• No regression in the human-OAuth callback path — existing handleCallback tests cover the legacy behavior including the "no state, has error" preservation
• End-to-end verification deferred to Stage 7 (MCP OAuth Stage 7: end-to-end integration fixture #97) — mcp-remote driving the full discovery → DCR → authorize → token → bearer call is the conformance proof

Out of scope

• /oauth/mcp/token exchange (Stage 4, MCP OAuth Stage 4: /oauth/mcp/token endpoint + JWT issuer #94) — codes from this PR are unused until that lands
• JWT signing keys + JWKS population (Stage 4)
• withMCPAuth wrapper that consumes Stage 4 tokens (Stage 5, MCP OAuth Stage 5: withMCPAuth wrapper #95)
• Audit hooks (Stage 6, MCP OAuth Stage 6: audit logging + onMCPTokenIssued hook #96)
• Multi-provider chooser UI (v1.1)

🤖 Generated with Claude Code

[github-actions]

Reviewed; no blockers found. Prior finding 1 resolved by address review commit eb707ba; prior finding 2 resolved (file no longer in diff).

The PR correctly implements the MCP /oauth/mcp/authorize endpoint, adhering to OAuth 2.1 recommendations by enforcing PKCE S256 and requiring a pinned mcp.issuer to prevent Host-header-driven identity spoofing. The single-provider requirement for MCP v1 is properly enforced in selectMCPProvider, and the short-lived authorization codes are securely persisted in a Harper table with a 5-minute TTL. Security invariants (CSRF single-use, provider-of-record enforcement, and path-length validation) are all maintained in the new endpoints. Comprehensive unit tests cover the new logic, including loopback port flexibility per RFC 8252.

[claude]

Reviewed; no blockers found.

[heskew]

No blockers.

---

🤖 Posted by Antigravity on Nathan's behalf

[heskew]

Pushed a follow-up commit (609b130): rebased onto current main (was ~10 commits behind) and applied two fail-fast guards surfaced by a cross-model review (Codex + Harper-domain pass):

• Require mcp.issuer when mcp.enabled — otherwise iss/aud derive from the client-controlled Host header. A pinned resource alone isn't enough (leaves iss Host-derived).
• Reject a provider keyed mcp (reserved by the /oauth/mcp/* dispatcher).

No blockers. Full write-up in the description under Rebase + hardening. 608 unit tests pass.

---

🤖 Posted by Claude on Nathan's behalf

[heskew]

Codex deep-review pass: I found one small Stage 3 contract gap around PKCE validation. Everything else I traced in the authorize/callback bridge looked consistent with the PR scope.

[heskew]

Codex deep-review finding: could we validate the PKCE challenge syntax here, not just presence? Issue #93 calls for a well-formed code_challenge, and RFC 7636 defines it as 43-128 unreserved characters. I confirmed code_challenge=x currently reaches the upstream IdP, which can burn a full login before Stage 4 rejects/miscompares the unusable code. A small regex/length check here plus too-short/too-long/invalid-character tests should close it cleanly.