diff --git a/docs/superpowers/specs/2026-03-15-oauth-flow-design.md b/docs/superpowers/specs/2026-03-15-oauth-flow-design.md new file mode 100644 index 0000000..d4589c5 --- /dev/null +++ b/docs/superpowers/specs/2026-03-15-oauth-flow-design.md @@ -0,0 +1,301 @@ +# OAuth Flow Redesign: Design Spec + +**Date:** 2026-03-15 +**Status:** Approved + +--- + +## Problem + +All MCP clients (Claude Code, Claude Desktop, opencode) receive a copy-paste URL when they need to authenticate, instead of having the browser open automatically. + +The correct MCP OAuth 2.1 behavior: +1. Client sends unauthenticated `POST /mcp` +2. Server returns `401 WWW-Authenticate: Bearer resource_metadata="https://host/.well-known/oauth-protected-resource"` +3. Client fetches `/.well-known/oauth-protected-resource`, then `/.well-known/oauth-authorization-server` +4. Client opens browser to `/authorize` +5. User authorizes on Discogs → `completeAuthorization()` runs → token issued to client +6. Client retries with bearer token — no copy/paste required + +--- + +## Goals + +- All supported clients (Claude Code, Claude Desktop, opencode) trigger an automatic browser open for first-time authentication +- No copy-paste `/login?...` URLs shown for clients that support MCP OAuth +- Clients with existing sessions (via `session_id` param or `Mcp-Session-Id` header + KV) continue to work without re-authenticating +- Comprehensive automated tests that lock in the fix and prevent regression +- Remove security issue: Discogs OAuth signing key currently logged to console + +## Non-Goals + +- Changing any MCP tool logic, rate limiting, caching, or collection indexing +- Supporting new MCP clients beyond Claude Code, Claude Desktop, and opencode +- Keeping backward compatibility with the old `connection_id` session approach + +--- + +## Architecture + +### New Files + +| File | Purpose | +|------|---------| +| `src/index-oauth.ts` | New main entry point (replaces `src/index.ts` in `wrangler.toml`) | +| `src/auth/oauth-handler.ts` | Auth route handler: `/authorize`, `/discogs-callback`, `/login`, `/callback`, `/.well-known/oauth-protected-resource` (served publicly, no auth required — unauthenticated clients must be able to read it for discovery). Note: `/.well-known/oauth-authorization-server` is served automatically by `@cloudflare/workers-oauth-provider` and does not appear in `oauth-handler.ts`. | + +### Changed Files + +| File | Change | +|------|--------| +| `wrangler.toml` | Point `main` at `src/index-oauth.ts` | +| `src/auth/discogs.ts` | Remove debug `console.log` of signing key and signature | +| `src/mcp/server.ts` | (1) Rename `createServer` → `createMcpServer` and refactor to return `{ server, setContext }` (mirroring lastfm-mcp), so both OAuth and session paths can inject props. (2) Remove `buildSessionAuthMessages` default fallback — the OAuth path always passes `buildOAuthAuthMessages()` explicitly. | + +### Deleted Files + +| File | Reason | Timing | +|------|--------|--------| +| `src/index.ts` | Replaced by `src/index-oauth.ts` | **After** production verification is complete (not during initial implementation) | +| `src/auth/jwt.ts` | JWT sessions replaced by OAuth bearer tokens; manual login stores directly in KV. Remove imports from `src/mcp/server.ts` and `src/index.ts` (the only current importers) before deleting. | During implementation | + +### KV Binding + +The existing `MCP_SESSIONS` KV namespace binding is reused for all session and pending-auth storage. No new KV namespace is needed. + +### New Dependency + +`@cloudflare/workers-oauth-provider` — provides `OAuthProvider`, `AuthRequest`, and `OAuthHelpers` types. The library also automatically enforces PKCE (`code_challenge`/`code_challenge_method=S256`) for public clients and rejects authorization requests missing `code_challenge` with a `400 invalid_request` response. No additional PKCE enforcement code is needed. + +**Scopes:** The server does not define custom scopes. Pass `scope: oauthReqInfo.scope` (whatever the client requested) to `completeAuthorization()` unchanged. The `/.well-known/oauth-authorization-server` `scopes_supported` field is an empty array — the library handles this in its discovery response. + +**`redirect_uri` validation:** The library enforces that the `redirect_uri` in `POST /oauth/token` matches the one used during `/authorize`. No additional validation code is needed. Tests must use matching `redirect_uri` values across the authorize and token exchange steps. + +**OAuth bearer token TTL:** Set `accessTokenTTL: 7 * 24 * 60 * 60` (7 days) in the `OAuthProvider` constructor options. Manual KV sessions also use 7 days (`expirationTtl: 7 * 24 * 60 * 60`). Always set TTLs explicitly — do not rely on library defaults. + +**`completeAuthorization()` return type:** Returns `{ redirectTo: string }` where `redirectTo` is the client's `redirect_uri` with `?code=...&state=...` appended by the library. Use `return Response.redirect(redirectTo, 302)`. It does not return a `Response` object directly. + +**`/oauth/token` body encoding:** Strip the `resource` param only if `Content-Type: application/x-www-form-urlencoded` is present; pass the request through unchanged otherwise. (RFC 6749 requires form encoding for token requests, so non-form requests are malformed and the library will reject them regardless.) + +**`session_id` param vs `Mcp-Session-Id` header KV miss behavior:** These are intentionally asymmetric. A `session_id` param is an explicit claim by the client that it has a known session — return `401 JSON` with **no** `WWW-Authenticate` header (the client should not retry via browser OAuth). A `Mcp-Session-Id` header on a request with no other session signal means the client doesn't know whether it's authenticated — fall through to the OAuth provider which returns `401 + WWW-Authenticate` (client retries via browser flow). This matches lastfm-mcp behavior. The unit test table must include a test for `session_id` param + KV miss asserting `401` JSON with no `WWW-Authenticate`. + +**KV key namespaces:** `oauth-pending:${requestToken}` uses the Discogs `oauth_token` as key (required because that's the only correlation identifier Discogs returns in its callback). `login-pending:${sessionId}` uses the session ID as key (generated by us at login start). These are intentionally different schemes. + +--- + +## Request Routing + +`src/index-oauth.ts` handles routing before delegating to the OAuth provider: + +Note: the `session_id` / `Mcp-Session-Id` session routing below applies ONLY to `/mcp`. All other paths (including `/callback?session_id=...`) are routed to the OAuth provider without session interception. The session routing applies equally to both `GET /mcp` (SSE) and `POST /mcp` (JSON-RPC) — `handleSessionBasedMcp` delegates to `createMcpHandler` which handles both transports transparently. + +``` +GET / → marketing/info page +GET /.well-known/mcp.json → MCP server card (Claude Desktop discovery) +GET /health → health check +OPTIONS * → CORS preflight + +POST/GET /mcp + ├─ has session_id param → handleSessionBasedMcp() (handles KV miss → 401 JSON internally) + ├─ has Mcp-Session-Id header + │ with valid KV session → handleSessionBasedMcp() (KV pre-checked; won't miss) + │ with no/expired KV entry → falls through to oauthProvider.fetch() (→ 401 + WWW-Authenticate) + └─ everything else → oauthProvider.fetch() + ├─ valid bearer token → authenticated MCP handler (tools receive props) + └─ no/invalid token → 401 + WWW-Authenticate (triggers browser OAuth flow) + +POST /oauth/token → INTERCEPTED: strip resource param (if form-encoded), then oauthProvider.fetch() + Mechanism (only if Content-Type is application/x-www-form-urlencoded): + read body as text, parse with `new URLSearchParams(body)`, + delete the `resource` key, re-serialize, reconstruct + `new Request(request.url, { method, headers, body: params.toString() })`. + Note: the original body stream is consumed by the read — always pass + the reconstructed Request to the provider. + Rationale: Claude.ai sends the full endpoint URL (e.g. `https://host/mcp`) + as `resource`, but workers-oauth-provider validates audience against + `https://host` only. The library does not support path-scoped resources + (RFC 8707). Matches lastfm-mcp. +Everything else → oauthProvider.fetch() → DefaultHandler (oauth-handler.ts) + Routes handled there: + /authorize → MCP OAuth start (→ Discogs) + /discogs-callback → MCP OAuth complete + /login → manual login start (→ Discogs) + /callback → manual login complete + /.well-known/oauth-protected-resource → RFC 9728 metadata + /.well-known/oauth-authorization-server → served by library +``` + +The `WWW-Authenticate` header is injected on 401 responses from `/mcp`: +``` +Bearer resource_metadata="https://host/.well-known/oauth-protected-resource" +``` + +--- + +## Discogs OAuth 1.0a Flow Inside MCP OAuth 2.1 + +The Discogs OAuth 1.0a two-step exchange happens inside the `/authorize` → `/discogs-callback` cycle. + +### `/authorize` (MCP client → our server) + +1. Parse the OAuth 2.1 request from the MCP client via `env.OAUTH_PROVIDER.parseAuthRequest(request)` +2. Construct `callbackUrl` from the incoming request: `${url.protocol}//${url.host}/discogs-callback` (points to the MCP OAuth callback — NOT `/callback`, which is the separate manual login path). +3. Call Discogs API to get a request token: `const { oauth_token: requestToken, oauth_token_secret: requestTokenSecret } = await discogsAuth.getRequestToken(callbackUrl)` +4. Store `{ oauthReqInfo, requestTokenSecret }` in KV as `oauth-pending:${requestToken}` (TTL 10 min). The key `requestToken` (Discogs `oauth_token`) is what Discogs will return in the callback, so it serves as the correlation key. The `oauthReqInfo` carries the OAuth 2.1 `state` parameter from the original client request; `completeAuthorization()` handles echoing it back to the client automatically. +5. Redirect user's browser to `https://discogs.com/oauth/authorize?oauth_token=${requestToken}` + +### `/discogs-callback` (Discogs → our server) + +Discogs returns the same `oauth_token` value in the callback query param as was issued in the request token step (per OAuth 1.0a spec) — this is the correlation key. + +1. Receive `oauth_token` + `oauth_verifier` from Discogs +2. Look up `oauth-pending:${oauth_token}` from KV to get `requestTokenSecret` + `oauthReqInfo` +3. Delete the pending KV entry immediately (before exchange — intentional: prevents replay and simplifies cleanup regardless of whether the exchange succeeds) +4. Exchange for Discogs access token (`DiscogsAuth.getAccessToken`) +5. Fetch Discogs identity (`GET /oauth/identity`) to get `id` (numeric, mapped to `userId`) and `username` +6. Call `env.OAUTH_PROVIDER.completeAuthorization({ request: oauthReqInfo, userId: username, metadata: { ... }, scope: oauthReqInfo.scope, props: { numericId, username, accessToken, accessTokenSecret } })` — this returns a `{ redirectTo }` object where `redirectTo` is the MCP client's `redirect_uri` with the authorization code embedded as a query parameter (added automatically by the library). Redirect to `redirectTo` with a `302`. Do not construct this redirect manually. See lastfm-mcp `src/auth/oauth-handler.ts` `handleLastfmCallback()` for the reference pattern. + +### User Props + +```typescript +interface DiscogsUserProps { + numericId: string // Discogs numeric user ID (from /oauth/identity response field "id", cast to string) + username: string // Discogs username string (from /oauth/identity response field "username") + accessToken: string + accessTokenSecret: string +} +``` + +These props flow into the MCP `apiHandler` via `ctx.props` for OAuth-authenticated requests, replacing the JWT/KV session lookup. For session-based requests, equivalent props are passed via `setContext`. Note: `completeAuthorization()` takes a `userId` argument (the OAuth 2.1 subject — pass `username` here); this is distinct from `props.numericId` (the Discogs numeric ID stored in props for tool use). + +### KV Eventual Consistency + +Not a practical concern: the write in `/authorize` and the read in `/discogs-callback` are separated by user interaction on Discogs (5–30 seconds minimum), and Cloudflare's anycast routing keeps the same browser on the same data center. Same pattern used successfully in lastfm-mcp. + +--- + +## `handleSessionBasedMcp` Function + +Lives in `src/index-oauth.ts`. Two call paths: +- `session_id` param path: called immediately; function handles KV miss internally (step 2) +- `Mcp-Session-Id` header path: router pre-checks KV before calling; function will not encounter a KV miss on this path + +`createMcpHandler` (used in step 6) is imported from `agents/mcp` — the Cloudflare Agents SDK. It wraps an `McpServer` instance for use in a Worker fetch handler. + +1. Look up `session:${sessionId}` from `MCP_SESSIONS` KV +2. If missing (`session_id` param path only): return `401` JSON `{ error: "invalid_session" }` with **no** `WWW-Authenticate` header +3. If expired (`expiresAt < Date.now()`): return `401` JSON `{ error: "session_expired", ... }` +4. Parse session JSON and construct a `DiscogsUserProps` object: `{ numericId: session.numericId, username: session.username, accessToken: session.accessToken, accessTokenSecret: session.accessTokenSecret }` +5. Create the MCP server using `createMcpServer(env, baseUrl)` — this factory must be updated as part of this work to return `{ server, setContext }`, mirroring lastfm-mcp's `createMcpServer`. The `setContext({ props: DiscogsUserProps })` call makes props available to MCP tools in the same way `ctx.props` does in the OAuth path. The `src/mcp/server.ts` "Changed Files" entry covers this refactor. +6. Run `createMcpHandler(server)(request, env, ctx)` +7. Inject `Mcp-Session-Id: ${sessionId}` header on the response + +--- + +## Manual Login Path + +Preserved for clients that don't support MCP OAuth (e.g. Claude Desktop with manual config): + +- `GET /login?session_id=` — Steps in order: (1) Generate random CSRF token. (2) Construct `callbackUrl = ${url.protocol}//${url.host}/callback?session_id=${sessionId}`. (3) Call Discogs to get a request token (`requestToken`, `requestTokenSecret`). (4) **Single KV write** (all fields written together, after the Discogs call): + ```json + { "sessionId": "...", "csrfToken": "...", "requestToken": "...", "requestTokenSecret": "...", "fromMcpClient": true, "timestamp": 0 } + ``` + TTL 10 min. (5) Set `__Host-csrf` cookie (value = csrfToken; HTTPS only — fall back to plain `csrf` on HTTP/localhost). (6) Redirect to Discogs authorize URL. +- `GET /callback?session_id=&oauth_token=&oauth_verifier=` — Discogs calls this URL after user authorization. Steps: (1) Look up `login-pending:${sessionId}` from KV — if missing: `400` HTML "Session expired, please try again". (2) Validate CSRF: `__Host-csrf` cookie value must match `csrfToken` in KV entry; on mismatch: `403`, delete KV entry. (3) Delete `login-pending:${sessionId}` from KV. (4) Exchange via `DiscogsAuth.getAccessToken(oauth_token, pendingData.requestTokenSecret, oauth_verifier)`. (5) Fetch Discogs identity to get `numericId` and `username`. (6) Store session in KV as `session:${sessionId}` (TTL 7 days). (7) Return HTML success page. +- Stored session format: + ```json + { "numericId", "username", "accessToken", "accessTokenSecret", "timestamp", "expiresAt", "sessionId" } + ``` + TTL: 7 days (`expirationTtl: 7 * 24 * 60 * 60`). + +--- + +## Security Fixes Bundled + +- Remove `console.log('OAuth signing key:', signingKey)` and `console.log('OAuth signature:', signature)` from `src/auth/discogs.ts` +- Replace commented-out `env.MCP_SESSIONS.delete()` with actual KV deletes for pending tokens +- Add CSRF protection to manual login flow (missing from current implementation) + +--- + +## Error Handling + +| Scenario | Behavior | +|----------|----------| +| Discogs request token call fails in `/authorize` | HTML error page with retry link | +| KV write fails in `/authorize` (storing `oauth-pending:`) | HTML error page — abort the flow | +| `/discogs-callback` with no `oauth_token` param | `400` plain text: "Missing OAuth parameters" | +| KV miss in `/discogs-callback` (expired/wrong token) | `400` HTML: "Session expired, please try again" with link back to `/authorize` | +| Discogs access token exchange fails (after KV entry already deleted) | `500` HTML error page — no retry possible, user must restart flow | +| Discogs identity fetch (`/oauth/identity`) fails after successful token exchange | `500` HTML error page — access token is dangling but not stored; user must restart flow | +| `completeAuthorization()` throws | `500` HTML error page | +| KV miss in `handleSessionBasedMcp` | `401` JSON response | +| Expired session in `handleSessionBasedMcp` | `401` JSON response | + +--- + +## Testing + +New test files in `test/`: + +### Unit Tests + +| Test | Assertion | +|------|-----------| +| `POST /mcp` no auth | `401` + `WWW-Authenticate: Bearer resource_metadata=...` | +| Response body does NOT contain copy-paste login URL | confirmed | +| `POST /mcp` with valid bearer token | `200`, reaches MCP handler | +| `POST /mcp?session_id=x` with valid KV session | `200`, session-based path | +| `POST /mcp?session_id=x` with KV miss | `401` JSON `{ error: "invalid_session" }`, **no** `WWW-Authenticate` header | +| `POST /mcp` with `Mcp-Session-Id` header + valid KV | `200`, session-based path | +| `POST /mcp` with `Mcp-Session-Id` header + no KV entry | Falls through to OAuth provider → `401` + `WWW-Authenticate` (same as unauthenticated request) | +| `GET /.well-known/oauth-protected-resource` | correct fields: `resource`, `authorization_servers`, `bearer_methods_supported` | +| `GET /.well-known/oauth-authorization-server` | all MCP-required fields: `issuer`, `authorization_endpoint`, `token_endpoint`, `response_types_supported` (includes `"code"`), `grant_types_supported` (includes `"authorization_code"`), `code_challenge_methods_supported` (includes `"S256"`) | + +### Integration Test — Full OAuth Round-Trip + +1. `POST /mcp` (no auth) → assert `401` + `WWW-Authenticate` +2. `GET /.well-known/oauth-protected-resource` → assert valid JSON with `authorization_servers` +3. `GET /.well-known/oauth-authorization-server` → assert valid metadata +4. `GET /authorize?client_id=...&redirect_uri=https://client/callback&state=random123&code_challenge=&code_challenge_method=S256&response_type=code` → assert redirect to `discogs.com/oauth/authorize` +5. Simulate Discogs callback: `GET /discogs-callback?oauth_token=mock&oauth_verifier=mock` (mock Discogs token exchange + identity API) +6. Assert: `completeAuthorization()` called, redirect issued to `https://client/callback?code=...&state=random123` +7. Client exchanges code: `POST /oauth/token` with body `grant_type=authorization_code&code=...&redirect_uri=https://client/callback&client_id=...&code_verifier=` → assert token response with `access_token`. Required fields: `grant_type` (must be `authorization_code`), `client_id`, `redirect_uri` (must match step 4), `code_verifier` (must be the preimage of `code_challenge` from step 4). Missing any of these returns `400`. +8. `POST /mcp` with `Authorization: Bearer ` → assert `200` with MCP capabilities + +### Manual Login Round-Trip + +1. `GET /login?session_id=test-session` → assert redirect to Discogs auth URL; capture the `__Host-csrf` cookie value from the response + the CSRF token stored in `login-pending:test-session` in KV +2. `GET /callback?session_id=test-session&oauth_token=mock&oauth_verifier=mock` with `Cookie: __Host-csrf=` (CSRF must be replayed) → assert HTML success page + `session:test-session` stored in KV +3. `POST /mcp?session_id=test-session` → assert `200` + +### Regression Tests + +- `POST /mcp` with no auth returns `401` — not `200` with a login URL string in the body +- `POST /mcp?session_id=valid` with valid KV session returns `200` + +--- + +## Deployment + +No feature flags needed — this is a routing and auth layer replacement. + +**Verification in staging:** +1. Run `npm run dev` locally +2. Use MCP Inspector (`npx @modelcontextprotocol/inspector`) to connect to `http://localhost:8787/mcp` +3. Confirm MCP Inspector shows OAuth prompt and opens browser, not a copy-paste URL +4. Complete the flow and confirm authenticated tools work + +**Rollback:** This work should be done on a feature branch. `src/index.ts` and `src/auth/jwt.ts` are deleted as part of implementation — rollback is via `git revert` or reverting the branch, not via restoring `wrangler.toml` alone. Do not delete `src/index.ts` until production verification is complete. + +--- + +## Success Criteria + +1. All automated tests pass +2. `POST /mcp` with no auth returns `401` with correct `WWW-Authenticate` header +3. Full OAuth round-trip integration test passes +4. Manual verification: MCP Inspector connecting to `/mcp` triggers OAuth browser open, not a copy-paste URL +5. Manual verification: existing session via `session_id` param or `Mcp-Session-Id` header continues to work +6. No tool response in the OAuth path contains a copy-paste login URL +7. Discogs OAuth signing key no longer logged to console diff --git a/package-lock.json b/package-lock.json index 5da0d66..2def776 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,8 +7,10 @@ "": { "name": "discogs-mcp", "version": "0.0.0", + "hasInstallScript": true, "license": "MIT", "dependencies": { + "@cloudflare/workers-oauth-provider": "^0.3.0", "@modelcontextprotocol/sdk": "^1.24.3", "@types/crypto-js": "^4.2.2", "agents": "^0.2.32", @@ -281,6 +283,12 @@ "node": ">=16" } }, + "node_modules/@cloudflare/workers-oauth-provider": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/@cloudflare/workers-oauth-provider/-/workers-oauth-provider-0.3.0.tgz", + "integrity": "sha512-I39kyUzNQWxVTIQs56xbnnHSrOY5UjiGKVeYI2zY5h+LjUxqeiuOBTTlKEDUirRvnMmBg3yppA6pR8bYKBrTAA==", + "license": "MIT" + }, "node_modules/@cloudflare/workers-types": { "version": "4.20250607.0", "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20250607.0.tgz", diff --git a/package.json b/package.json index f04843b..ddb9c02 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "wrangler": "^4.19.1" }, "dependencies": { + "@cloudflare/workers-oauth-provider": "^0.3.0", "@modelcontextprotocol/sdk": "^1.24.3", "@types/crypto-js": "^4.2.2", "agents": "^0.2.32", @@ -40,4 +41,4 @@ "oauth-1.0a": "^2.2.6", "zod": "^4.1.13" } -} \ No newline at end of file +} diff --git a/src/auth/discogs.ts b/src/auth/discogs.ts index b6d6d28..2a3da08 100644 --- a/src/auth/discogs.ts +++ b/src/auth/discogs.ts @@ -149,10 +149,6 @@ export class DiscogsAuth { const signature = await this.computeSignature(baseString, signingKey) oauthParams.oauth_signature = signature - console.log('OAuth signature base string:', baseString) - console.log('OAuth signing key:', signingKey) - console.log('OAuth signature:', signature) - // Create authorization header const authParams = Object.keys(oauthParams) .map((key) => `${percentEncode(key)}="${percentEncode(oauthParams[key])}"`) diff --git a/src/auth/jwt.ts b/src/auth/jwt.ts deleted file mode 100644 index 514a5f5..0000000 --- a/src/auth/jwt.ts +++ /dev/null @@ -1,124 +0,0 @@ -/** - * JWT utilities for session management - * Uses Web Crypto API available in Cloudflare Workers - */ - -export interface SessionPayload { - userId: string - accessToken: string - accessTokenSecret: string - iat: number // issued at - exp: number // expires at -} - -/** - * Create a signed JWT token containing user session data - */ -export async function createSessionToken( - payload: Omit, - secret: string, - expiresInHours = 168, -): Promise { - const now = Math.floor(Date.now() / 1000) - const fullPayload: SessionPayload = { - ...payload, - iat: now, - exp: now + expiresInHours * 3600, - } - - // Create JWT header - const header = { - alg: 'HS256', - typ: 'JWT', - } - - // Encode header and payload - const encodedHeader = base64UrlEncode(JSON.stringify(header)) - const encodedPayload = base64UrlEncode(JSON.stringify(fullPayload)) - - // Create signature - const data = `${encodedHeader}.${encodedPayload}` - const signature = await sign(data, secret) - - return `${data}.${signature}` -} - -/** - * Verify and decode a JWT session token - */ -export async function verifySessionToken(token: string, secret: string): Promise { - try { - const parts = token.split('.') - if (parts.length !== 3) { - return null - } - - const [encodedHeader, encodedPayload, signature] = parts - - // Verify signature - const data = `${encodedHeader}.${encodedPayload}` - const expectedSignature = await sign(data, secret) - - if (signature !== expectedSignature) { - return null - } - - // Decode payload - const payload = JSON.parse(base64UrlDecode(encodedPayload)) as SessionPayload - - // Check expiration - const now = Math.floor(Date.now() / 1000) - if (payload.exp < now) { - return null - } - - return payload - } catch (error) { - console.error('JWT verification error:', error) - return null - } -} - -/** - * Sign data using HMAC-SHA256 - */ -async function sign(data: string, secret: string): Promise { - const encoder = new TextEncoder() - const key = await crypto.subtle.importKey('raw', encoder.encode(secret), { name: 'HMAC', hash: 'SHA-256' }, false, ['sign']) - - const signature = await crypto.subtle.sign('HMAC', key, encoder.encode(data)) - return base64UrlEncode(new Uint8Array(signature)) -} - -/** - * Base64 URL encode (without padding) - */ -function base64UrlEncode(data: string | Uint8Array): string { - let base64: string - - if (typeof data === 'string') { - base64 = btoa(data) - } else { - // Convert Uint8Array to string - const binary = Array.from(data, (byte) => String.fromCharCode(byte)).join('') - base64 = btoa(binary) - } - - return base64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '') -} - -/** - * Base64 URL decode - */ -function base64UrlDecode(encoded: string): string { - // Add padding if needed - let padded = encoded - while (padded.length % 4) { - padded += '=' - } - - // Convert URL-safe characters back - const base64 = padded.replace(/-/g, '+').replace(/_/g, '/') - - return atob(base64) -} diff --git a/src/auth/oauth-handler.ts b/src/auth/oauth-handler.ts new file mode 100644 index 0000000..ebaa8d7 --- /dev/null +++ b/src/auth/oauth-handler.ts @@ -0,0 +1,360 @@ +// ABOUTME: OAuth handler integrating Discogs authentication with MCP OAuth 2.1. +// ABOUTME: Handles /authorize, /discogs-callback, /login, /callback, and /.well-known/oauth-protected-resource. +import type { AuthRequest, OAuthHelpers } from '@cloudflare/workers-oauth-provider' +import type { ExecutionContext } from '@cloudflare/workers-types' +import { DiscogsAuth } from './discogs' +import type { Env } from '../types/env' + +// Env with OAuth helpers injected by the provider at runtime +interface OAuthEnv extends Env { + OAUTH_PROVIDER: OAuthHelpers +} + +/** + * Discogs user props stored in the OAuth token. + * Passed to completeAuthorization() and available in apiHandler via ctx.props. + */ +export interface DiscogsUserProps { + numericId: string // Discogs numeric user ID (from /oauth/identity field "id") + username: string // Discogs username (from /oauth/identity field "username") + accessToken: string + accessTokenSecret: string +} + +/** + * DefaultHandler for @cloudflare/workers-oauth-provider. + * Only handles auth-related routes. Static routes (/, /health, etc.) are + * handled by the main entry point in index-oauth.ts. + */ +export const DiscogsOAuthHandler = { + async fetch(request: Request, env: OAuthEnv, _ctx: ExecutionContext): Promise { + const url = new URL(request.url) + + switch (url.pathname) { + case '/authorize': + if (request.method === 'GET') return handleAuthorize(request, env) + return new Response('Method not allowed', { status: 405 }) + + case '/discogs-callback': + if (request.method === 'GET') return handleDiscogsCallback(request, env) + return new Response('Method not allowed', { status: 405 }) + + case '/login': + if (request.method === 'GET') return handleManualLogin(request, env) + return new Response('Method not allowed', { status: 405 }) + + case '/callback': + if (request.method === 'GET') return handleManualCallback(request, env) + return new Response('Method not allowed', { status: 405 }) + + case '/.well-known/oauth-protected-resource': + return handleProtectedResourceMetadata(request) + + default: + return new Response('Not found', { status: 404 }) + } + }, +} + +// ── Stub implementations (filled in subsequent tasks) ────────────────────────── + +async function handleAuthorize(request: Request, env: OAuthEnv): Promise { + try { + const oauthReqInfo: AuthRequest = await env.OAUTH_PROVIDER.parseAuthRequest(request) + + const url = new URL(request.url) + const callbackUrl = `${url.protocol}//${url.host}/discogs-callback` + + const discogsAuth = new DiscogsAuth(env.DISCOGS_CONSUMER_KEY, env.DISCOGS_CONSUMER_SECRET) + const { oauth_token: requestToken, oauth_token_secret: requestTokenSecret } = + await discogsAuth.getRequestToken(callbackUrl) + + // Store pending state: correlate Discogs oauth_token with our OAuth 2.1 request + await env.MCP_SESSIONS.put( + `oauth-pending:${requestToken}`, + JSON.stringify({ oauthReqInfo, requestTokenSecret }), + { expirationTtl: 600 }, // 10 minutes + ) + + return Response.redirect( + `https://www.discogs.com/oauth/authorize?oauth_token=${requestToken}`, + 302, + ) + } catch (error) { + console.error('[OAUTH] /authorize error:', error) + return new Response( + `

Authorization Error

${error instanceof Error ? error.message : 'Unknown error'}

Try again

`, + { status: 500, headers: { 'Content-Type': 'text/html' } }, + ) + } +} + +async function handleDiscogsCallback(request: Request, env: OAuthEnv): Promise { + const url = new URL(request.url) + const oauthToken = url.searchParams.get('oauth_token') + const oauthVerifier = url.searchParams.get('oauth_verifier') + + if (!oauthToken || !oauthVerifier) { + return new Response('Missing OAuth parameters', { status: 400 }) + } + + // Retrieve and immediately delete the pending state (prevents replay) + const pendingKey = `oauth-pending:${oauthToken}` + const pendingDataStr = await env.MCP_SESSIONS.get(pendingKey) + + if (!pendingDataStr) { + return new Response( + '

Session Expired

The authorization session has expired or is invalid. Please try again.

Restart authorization

', + { status: 400, headers: { 'Content-Type': 'text/html' } }, + ) + } + + const pendingData = JSON.parse(pendingDataStr) + const { oauthReqInfo, requestTokenSecret }: { oauthReqInfo: AuthRequest; requestTokenSecret: string } = pendingData + + // Delete immediately — prevents replay even if subsequent steps fail + await env.MCP_SESSIONS.delete(pendingKey) + + try { + // Exchange request token for access token + const discogsAuth = new DiscogsAuth(env.DISCOGS_CONSUMER_KEY, env.DISCOGS_CONSUMER_SECRET) + const { oauth_token: accessToken, oauth_token_secret: accessTokenSecret } = + await discogsAuth.getAccessToken(oauthToken, requestTokenSecret, oauthVerifier) + + // Fetch Discogs identity to get username and numeric ID + const identityRes = await fetch('https://api.discogs.com/oauth/identity', { + headers: { + Authorization: ( + await discogsAuth.getAuthHeaders('https://api.discogs.com/oauth/identity', 'GET', { + key: accessToken, + secret: accessTokenSecret, + }) + ).Authorization, + 'User-Agent': 'discogs-mcp/1.0.0', + }, + }) + + if (!identityRes.ok) { + throw new Error(`Failed to fetch Discogs identity: ${identityRes.status}`) + } + + const identity = await identityRes.json() as { id: number; username: string } + const userProps: DiscogsUserProps = { + numericId: String(identity.id), + username: identity.username, + accessToken, + accessTokenSecret, + } + + // Complete the MCP OAuth 2.1 flow — library issues the authorization code to client + const { redirectTo } = await env.OAUTH_PROVIDER.completeAuthorization({ + request: oauthReqInfo, + userId: userProps.username, // OAuth 2.1 subject = username + metadata: { + label: 'Discogs MCP Access', + discogsUsername: userProps.username, + authorizedAt: new Date().toISOString(), + }, + scope: oauthReqInfo.scope, + props: userProps, + }) + + return Response.redirect(redirectTo, 302) + } catch (error) { + console.error('[OAUTH] /discogs-callback error:', error) + return new Response( + `

Authentication Failed

${error instanceof Error ? error.message : 'Unknown error'}

Please try again.

`, + { status: 500, headers: { 'Content-Type': 'text/html' } }, + ) + } +} + +async function handleManualLogin(request: Request, env: OAuthEnv): Promise { + try { + const url = new URL(request.url) + const sessionId = url.searchParams.get('session_id') ?? crypto.randomUUID() + const fromMcpClient = !!url.searchParams.get('session_id') + + // Generate CSRF token + const csrfToken = crypto.randomUUID() + + // Construct Discogs callback URL for the manual path + const callbackUrl = `${url.protocol}//${url.host}/callback?session_id=${sessionId}` + + // Get Discogs request token + const discogsAuth = new DiscogsAuth(env.DISCOGS_CONSUMER_KEY, env.DISCOGS_CONSUMER_SECRET) + const { oauth_token: requestToken, oauth_token_secret: requestTokenSecret } = + await discogsAuth.getRequestToken(callbackUrl) + + // Single KV write with all fields (CSRF token + Discogs tokens) + await env.MCP_SESSIONS.put( + `login-pending:${sessionId}`, + JSON.stringify({ + sessionId, + csrfToken, + requestToken, + requestTokenSecret, + fromMcpClient, + timestamp: Date.now(), + }), + { expirationTtl: 600 }, // 10 minutes + ) + + // Use __Host- prefix on HTTPS, plain on HTTP (local dev) + const isHttps = url.protocol === 'https:' + const cookieName = isHttps ? '__Host-csrf' : 'csrf' + const cookieFlags = isHttps + ? `${cookieName}=${csrfToken}; HttpOnly; Secure; SameSite=Lax; Path=/` + : `${cookieName}=${csrfToken}; HttpOnly; SameSite=Lax; Path=/` + + const authorizeUrl = `https://www.discogs.com/oauth/authorize?oauth_token=${requestToken}` + + return new Response(null, { + status: 302, + headers: { + Location: authorizeUrl, + 'Set-Cookie': cookieFlags, + }, + }) + } catch (error) { + console.error('[LOGIN] /login error:', error) + return new Response( + `Login error: ${error instanceof Error ? error.message : 'Unknown error'}`, + { status: 500 }, + ) + } +} + +async function handleManualCallback(request: Request, env: OAuthEnv): Promise { + try { + const url = new URL(request.url) + const sessionId = url.searchParams.get('session_id') + const oauthToken = url.searchParams.get('oauth_token') + const oauthVerifier = url.searchParams.get('oauth_verifier') + + if (!sessionId || !oauthToken || !oauthVerifier) { + return new Response('Missing required parameters', { status: 400 }) + } + + // Look up the pending login + const pendingKey = `login-pending:${sessionId}` + const pendingDataStr = await env.MCP_SESSIONS.get(pendingKey) + + if (!pendingDataStr) { + return new Response( + '

Session Expired

Your login session has expired. Please try again.

', + { status: 400, headers: { 'Content-Type': 'text/html' } }, + ) + } + + const pendingData = JSON.parse(pendingDataStr) + + // Validate CSRF token from cookie + const isHttps = url.protocol === 'https:' + const cookieName = isHttps ? '__Host-csrf' : 'csrf' + const cookieHeader = request.headers.get('Cookie') ?? '' + const cookies = Object.fromEntries( + cookieHeader.split(';').map((c) => { + const [k, ...v] = c.trim().split('=') + return [k, v.join('=')] + }), + ) + const csrfFromCookie = cookies[cookieName] + + if (!csrfFromCookie || csrfFromCookie !== pendingData.csrfToken) { + await env.MCP_SESSIONS.delete(pendingKey) + return new Response('CSRF validation failed. Please try logging in again.', { status: 403 }) + } + + // Clean up pending entry + await env.MCP_SESSIONS.delete(pendingKey) + + // Exchange tokens + const discogsAuth = new DiscogsAuth(env.DISCOGS_CONSUMER_KEY, env.DISCOGS_CONSUMER_SECRET) + const { oauth_token: accessToken, oauth_token_secret: accessTokenSecret } = + await discogsAuth.getAccessToken(oauthToken, pendingData.requestTokenSecret, oauthVerifier) + + // Fetch identity + const identityRes = await fetch('https://api.discogs.com/oauth/identity', { + headers: { + Authorization: ( + await discogsAuth.getAuthHeaders('https://api.discogs.com/oauth/identity', 'GET', { + key: accessToken, + secret: accessTokenSecret, + }) + ).Authorization, + 'User-Agent': 'discogs-mcp/1.0.0', + }, + }) + + if (!identityRes.ok) { + throw new Error(`Failed to fetch Discogs identity: ${identityRes.status}`) + } + + const identity = await identityRes.json() as { id: number; username: string } + + // Store session in KV (7 days) + const expiresAt = Date.now() + 7 * 24 * 60 * 60 * 1000 + await env.MCP_SESSIONS.put( + `session:${sessionId}`, + JSON.stringify({ + numericId: String(identity.id), + username: identity.username, + accessToken, + accessTokenSecret, + timestamp: Date.now(), + expiresAt, + sessionId, + }), + { expirationTtl: 7 * 24 * 60 * 60 }, + ) + + const fromMcpClient = !!pendingData.fromMcpClient + const instructionsHtml = fromMcpClient + ? `

Your MCP session is now connected. You can close this window.

` + : `

Use this URL in your MCP client: ${url.protocol}//${url.host}/mcp?session_id=${sessionId}

` + + return new Response( + ` +

Authentication Successful!

+

You're now authenticated as ${identity.username} on Discogs.

+ ${instructionsHtml} + `, + { + status: 200, + headers: { + 'Content-Type': 'text/html', + 'Set-Cookie': `${cookieName}=; Max-Age=0; Path=/`, + }, + }, + ) + } catch (error) { + console.error('[LOGIN] /callback error:', error) + return new Response( + `Authentication failed: ${error instanceof Error ? error.message : 'Unknown error'}`, + { status: 500 }, + ) + } +} + +function handleProtectedResourceMetadata(request: Request): Response { + const url = new URL(request.url) + const baseUrl = `${url.protocol}//${url.host}` + + return new Response( + JSON.stringify({ + resource: baseUrl, + authorization_servers: [baseUrl], + bearer_methods_supported: ['header'], + scopes_supported: [], + }), + { + status: 200, + headers: { + 'Content-Type': 'application/json', + 'Access-Control-Allow-Origin': '*', + 'Cache-Control': 'public, max-age=3600', + }, + }, + ) +} diff --git a/src/index-oauth.ts b/src/index-oauth.ts new file mode 100644 index 0000000..83c6981 --- /dev/null +++ b/src/index-oauth.ts @@ -0,0 +1,240 @@ +// ABOUTME: Main entry point supporting MCP OAuth 2.1 and session-based authentication. +// ABOUTME: Routes /mcp requests to session handler or OAuth provider based on auth state. +import { OAuthProvider } from '@cloudflare/workers-oauth-provider' +import type { ExecutionContext } from '@cloudflare/workers-types' +import { createMcpHandler } from 'agents/mcp' + +import { DiscogsOAuthHandler, type DiscogsUserProps } from './auth/oauth-handler' +import { createMcpServer } from './mcp/server' +import type { Env } from './types/env' + +const SERVER_VERSION = '1.0.0' + +// PKCE + standard MCP session TTL (7 days) +const ACCESS_TOKEN_TTL = 7 * 24 * 60 * 60 + +/** + * OAuth provider instance — handles all OAuth 2.1 endpoints automatically: + * - /.well-known/oauth-authorization-server (discovery) + * - /oauth/register (dynamic client registration) + * - /oauth/token (token exchange) + * - All routes not intercepted by the main fetch handler + */ +const oauthProvider = new OAuthProvider({ + apiRoute: '/mcp', + apiHandler: { + async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise { + // This runs only for OAuth-authenticated requests (valid bearer token). + // workers-oauth-provider injects the user props from completeAuthorization() into ctx.props. + const url = new URL(request.url) + const baseUrl = `${url.protocol}//${url.host}` + + const { server, setContext } = createMcpServer(env, baseUrl) + + const props = (ctx as unknown as { props?: DiscogsUserProps }).props + if (props?.username && props?.accessToken) { + setContext({ + session: { + username: props.username, + numericId: props.numericId, + accessToken: props.accessToken, + accessTokenSecret: props.accessTokenSecret, + }, + }) + } + + return createMcpHandler(server)(request, env, ctx) + }, + }, + authorizeEndpoint: '/authorize', + tokenEndpoint: '/oauth/token', + clientRegistrationEndpoint: '/oauth/register', + defaultHandler: DiscogsOAuthHandler, + accessTokenTTL: ACCESS_TOKEN_TTL, +}) + +/** + * Handle MCP request using a pre-existing KV session (session_id param or Mcp-Session-Id header). + * Two call paths: + * - session_id param: called unconditionally, handles KV miss internally + * - Mcp-Session-Id header: only called after router verifies KV entry exists + */ +async function handleSessionBasedMcp( + request: Request, + env: Env, + ctx: ExecutionContext, + sessionId: string, +): Promise { + const url = new URL(request.url) + const baseUrl = `${url.protocol}//${url.host}` + + const sessionDataStr = await env.MCP_SESSIONS.get(`session:${sessionId}`) + + if (!sessionDataStr) { + return new Response(JSON.stringify({ error: 'invalid_session' }), { + status: 401, + headers: { 'Content-Type': 'application/json' }, + // Intentionally no WWW-Authenticate header — client should not retry via OAuth + }) + } + + const sessionData = JSON.parse(sessionDataStr) + + if (sessionData.expiresAt && Date.now() > sessionData.expiresAt) { + return new Response(JSON.stringify({ error: 'session_expired' }), { + status: 401, + headers: { 'Content-Type': 'application/json' }, + }) + } + + const { server, setContext } = createMcpServer(env, baseUrl) + setContext({ + session: { + username: sessionData.username, + numericId: sessionData.numericId, + accessToken: sessionData.accessToken, + accessTokenSecret: sessionData.accessTokenSecret, + }, + sessionId, + }) + + const handler = createMcpHandler(server) + const response = await handler(request, env, ctx) + + const newHeaders = new Headers(response.headers) + newHeaders.set('Mcp-Session-Id', sessionId) + newHeaders.set('Access-Control-Expose-Headers', 'Mcp-Session-Id') + + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers: newHeaders, + }) +} + +/** + * Strip the 'resource' parameter from OAuth token requests. + * Claude.ai sends the full MCP endpoint URL as `resource`, but workers-oauth-provider + * validates audience against ${protocol}//${host} only. Stripping prevents audience mismatch. + * Only applied when Content-Type is application/x-www-form-urlencoded. + */ +async function stripResourceParam(request: Request): Promise { + if (request.method !== 'POST') return request + const contentType = request.headers.get('content-type') ?? '' + if (!contentType.includes('application/x-www-form-urlencoded')) return request + + const body = await request.text() + const params = new URLSearchParams(body) + + if (!params.has('resource')) { + return new Request(request.url, { method: request.method, headers: request.headers, body }) + } + + params.delete('resource') + return new Request(request.url, { + method: request.method, + headers: request.headers, + body: params.toString(), + }) +} + +export default { + async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise { + const url = new URL(request.url) + const baseUrl = `${url.protocol}//${url.host}` + + // CORS preflight + if (request.method === 'OPTIONS') { + return new Response(null, { + status: 200, + headers: { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', + 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Mcp-Session-Id', + 'Access-Control-Expose-Headers': 'Mcp-Session-Id', + 'Access-Control-Max-Age': '86400', + }, + }) + } + + // Static routes — handled before OAuth provider + if (url.pathname === '/' && request.method === 'GET') { + return new Response( + JSON.stringify({ + name: 'Discogs MCP Server', + version: SERVER_VERSION, + description: 'Model Context Protocol server for Discogs collection access', + endpoints: { '/mcp': 'MCP endpoint', '/login': 'Manual OAuth login', '/health': 'Health check' }, + }), + { status: 200, headers: { 'Content-Type': 'application/json', 'Access-Control-Allow-Origin': '*' } }, + ) + } + + if (url.pathname === '/health' && request.method === 'GET') { + return new Response( + JSON.stringify({ status: 'ok', timestamp: new Date().toISOString(), version: SERVER_VERSION }), + { status: 200, headers: { 'Content-Type': 'application/json', 'Access-Control-Allow-Origin': '*' } }, + ) + } + + if (url.pathname === '/.well-known/mcp.json' && request.method === 'GET') { + return new Response( + JSON.stringify({ + version: '1.0', + serverInfo: { name: 'discogs-mcp', version: SERVER_VERSION }, + transport: { type: 'streamable-http', endpoint: '/mcp' }, + authentication: { required: false }, + }), + { status: 200, headers: { 'Content-Type': 'application/json', 'Access-Control-Allow-Origin': '*' } }, + ) + } + + // /mcp — session routing (applies to both GET and POST) + if (url.pathname === '/mcp') { + // 1. Explicit session_id param → session path (handles KV miss internally) + const sessionId = url.searchParams.get('session_id') + if (sessionId) { + return handleSessionBasedMcp(request, env, ctx, sessionId) + } + + // 2. Mcp-Session-Id header — only route to session path if KV entry exists + const mcpSessionId = request.headers.get('Mcp-Session-Id') + if (mcpSessionId) { + const sessionDataStr = await env.MCP_SESSIONS.get(`session:${mcpSessionId}`) + if (sessionDataStr) { + return handleSessionBasedMcp(request, env, ctx, mcpSessionId) + } + // No KV entry → fall through to OAuth provider (returns 401 + WWW-Authenticate) + } + + // 3. Everything else → OAuth provider + const response = await oauthProvider.fetch(request, env, ctx) + + // Inject WWW-Authenticate on 401 responses from /mcp + if (response.status === 401) { + const newHeaders = new Headers(response.headers) + newHeaders.set( + 'WWW-Authenticate', + `Bearer resource_metadata="${baseUrl}/.well-known/oauth-protected-resource"`, + ) + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers: newHeaders, + }) + } + + return response + } + + // /oauth/token — strip resource param before forwarding + if (url.pathname === '/oauth/token') { + request = await stripResourceParam(request) + return oauthProvider.fetch(request, env, ctx) + } + + // All other routes → OAuth provider (handles /authorize, /discogs-callback, /login, + // /callback, /.well-known/oauth-protected-resource, /.well-known/oauth-authorization-server) + return oauthProvider.fetch(request, env, ctx) + }, +} diff --git a/src/index.ts b/src/index.ts index b9e0fe5..13a3be7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,12 +5,65 @@ */ import { createMcpHandler } from "agents/mcp"; -import { createServer } from "./mcp/server.js"; +import { createMcpServer } from "./mcp/server.js"; import { DiscogsAuth } from './auth/discogs' -import { createSessionToken, verifySessionToken } from './auth/jwt' +import { createSessionToken, verifySessionToken, type SessionPayload } from './auth/jwt' import type { Env } from './types/env' import type { ExecutionContext } from '@cloudflare/workers-types' +interface LegacySessionContext { + session: SessionPayload | null + connectionId?: string +} + +async function extractSessionFromRequest( + request: Request, + env: Env, + sessionId: string, +): Promise { + try { + const cookieHeader = request.headers.get('Cookie') + if (cookieHeader) { + const cookies = cookieHeader.split(';').reduce((acc, cookie) => { + const [key, value] = cookie.trim().split('=') + if (key && value) acc[key] = value + return acc + }, {} as Record) + const sessionToken = cookies.session + if (sessionToken) { + const session = await verifySessionToken(sessionToken, env.JWT_SECRET) + if (session) return { session, connectionId: sessionId } + } + } + } catch (error) { + console.error('Error verifying cookie session:', error) + } + + if (sessionId && env.MCP_SESSIONS) { + try { + const sessionDataStr = await env.MCP_SESSIONS.get(`session:${sessionId}`) + if (sessionDataStr) { + const sessionData = JSON.parse(sessionDataStr) + if (!sessionData.expiresAt || new Date(sessionData.expiresAt) <= new Date()) { + return { session: null, connectionId: sessionId } + } + const session: SessionPayload = { + userId: sessionData.userId, + accessToken: sessionData.accessToken, + accessTokenSecret: sessionData.accessTokenSecret, + iat: Math.floor(Date.now() / 1000), + exp: Math.floor(new Date(sessionData.expiresAt).getTime() / 1000), + } + return { session, connectionId: sessionId } + } + } catch (error) { + console.error('Error retrieving connection session:', error) + } + } + + return { session: null, connectionId: sessionId } +} + // These types are available globally in Workers runtime /// /// @@ -70,7 +123,21 @@ async function handleMCPRequest(request: Request, env: Env, ctx: ExecutionContex console.log(`[MCP] Session ID: ${sessionId} (source: ${sessionSource})`) // Create MCP server instance with session ID - const server = createServer(env, request, sessionId) + const baseUrl = `${url.protocol}//${url.host}` + const { server, setContext } = createMcpServer(env, baseUrl) + + // Extract session from request and inject into server context + const sessionContext = await extractSessionFromRequest(request, env, sessionId) + if (sessionContext.session) { + const { userId, accessToken, accessTokenSecret } = sessionContext.session + setContext({ + session: { username: userId, numericId: userId, accessToken, accessTokenSecret }, + sessionId, + }) + } else { + setContext({ session: null, sessionId }) + } + const handler = createMcpHandler(server) // Call the handler diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 0cffb99..b9bf64b 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -1,156 +1,108 @@ /** - * MCP Server configuration for Discogs - * Uses the official @modelcontextprotocol/sdk with Cloudflare Agents SDK + * MCP Server factory for Discogs + * Returns { server, setContext } so both OAuth and session paths can inject auth props. */ -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import type { Env } from "../types/env.js"; -import { registerPublicTools } from "./tools/public.js"; -import { registerAuthenticatedTools } from "./tools/authenticated.js"; -import { registerResources } from "./resources/discogs.js"; -import { registerPrompts } from "./prompts/collection.js"; -import { verifySessionToken, type SessionPayload } from "../auth/jwt.js"; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import type { Env } from '../types/env.js' +import { registerPublicTools } from './tools/public.js' +import { registerAuthenticatedTools } from './tools/authenticated.js' +import { registerResources } from './resources/discogs.js' +import { registerPrompts } from './prompts/collection.js' /** - * Session context - extracted from request for tool access + * Session payload compatible with tool expectations. + * Maps Discogs credentials to tool access patterns. */ -export interface SessionContext { - session: SessionPayload | null; - connectionId?: string; +export interface SessionPayload { + userId: string + accessToken: string + accessTokenSecret: string + iat: number + exp: number } /** - * Extract session from request (cookies or connection-specific storage) - * The sessionId is passed explicitly (generated by the caller if not provided in request) + * Auth session — Discogs credentials available to tools after authentication. */ -async function extractSessionFromRequest( - request: Request, - env: Env, - sessionId: string -): Promise { - // Debug: Log session ID - console.log("Session ID extraction:", { - providedSessionId: sessionId, - }); - - // Try to get session from cookie - try { - const cookieHeader = request.headers.get("Cookie"); - if (cookieHeader) { - const cookies = cookieHeader.split(";").reduce( - (acc, cookie) => { - const [key, value] = cookie.trim().split("="); - if (key && value) { - acc[key] = value; - } - return acc; - }, - {} as Record - ); - - const sessionToken = cookies.session; - if (sessionToken) { - const session = await verifySessionToken(sessionToken, env.JWT_SECRET); - if (session) { - return { session, connectionId: sessionId }; - } - } - } - } catch (error) { - console.error("Error verifying cookie session:", error); - } - - // Try connection-specific authentication if we have KV storage - if (sessionId && env.MCP_SESSIONS) { - try { - const kvKey = `session:${sessionId}`; - console.log(`Looking up KV key: ${kvKey}`); - const sessionDataStr = await env.MCP_SESSIONS.get(kvKey); - if (sessionDataStr) { - const sessionData = JSON.parse(sessionDataStr); - console.log(`Found session for user: ${sessionData.userId}`); - - // Verify the stored session is still valid - if ( - !sessionData.expiresAt || - new Date(sessionData.expiresAt) <= new Date() - ) { - console.log("Connection session has expired"); - return { session: null, connectionId: sessionId }; - } +export interface DiscogsSession { + username: string + numericId: string + accessToken: string + accessTokenSecret: string +} - // Return session payload - const session: SessionPayload = { - userId: sessionData.userId, - accessToken: sessionData.accessToken, - accessTokenSecret: sessionData.accessTokenSecret, - iat: Math.floor(Date.now() / 1000), - exp: Math.floor(new Date(sessionData.expiresAt).getTime() / 1000), - }; +/** + * Per-request context — mutable, updated before each MCP handler call. + */ +export interface McpRequestContext { + session: DiscogsSession | null + sessionId: string | null + baseUrl: string +} - return { session, connectionId: sessionId }; - } else { - console.log(`No session found for key: ${kvKey}`); - } - } catch (error) { - console.error("Error retrieving connection session:", error); - } - } +/** + * Session context shape consumed by tool files. + * Uses SessionPayload so existing tool code (session.userId, session.exp, etc.) compiles unchanged. + */ +export interface SessionContext { + session: SessionPayload | null + connectionId?: string +} - return { session: null, connectionId: sessionId }; +export interface McpServerWithContext { + server: McpServer + setContext: (ctx: Partial) => void + getContext: () => McpRequestContext } /** - * Creates and configures the MCP server with all tools, resources, and prompts - * Uses factory pattern to provide env and request context access via closures + * Creates and configures the MCP server. + * * @param env - Cloudflare Worker environment bindings - * @param request - The incoming HTTP request - * @param sessionId - Session ID (generated by caller if not provided in request headers) + * @param baseUrl - Base URL for constructing auth URLs (e.g. https://host) */ -export function createServer(env: Env, request: Request, sessionId: string): McpServer { - const server = new McpServer({ - name: "discogs-mcp", - version: "1.0.0", - }); - - // Create a session context holder that will be populated lazily - let sessionContextCache: SessionContext | null = null; - let sessionContextPromise: Promise | null = null; - - // Lazy session extraction - only extract session when first tool is called - const getSessionContext = async (): Promise => { - // If we've already extracted session, return it immediately - if (sessionContextCache) { - return sessionContextCache; - } - - // If extraction is in progress, await it - if (sessionContextPromise) { - return await sessionContextPromise; - } - - // Start extraction - sessionContextPromise = extractSessionFromRequest(request, env, sessionId).then( - (context) => { - sessionContextCache = context; - return context; - } - ); - - return await sessionContextPromise; - }; - - // Register public tools (available without authentication) - // Pass session context so they can include connection ID in auth URLs - registerPublicTools(server, env, getSessionContext); - - // Register authenticated tools with session context provider - registerAuthenticatedTools(server, env, getSessionContext); - - // Register resources (require authentication) - registerResources(server, env, getSessionContext); - - // Register prompts (common workflows) - registerPrompts(server); - - return server; +export function createMcpServer(env: Env, baseUrl: string): McpServerWithContext { + const server = new McpServer({ + name: 'discogs-mcp', + version: '1.0.0', + }) + + // Mutable context — set by the caller before the MCP handler runs + const context: McpRequestContext = { + session: null, + sessionId: null, + baseUrl, + } + + // Adapt DiscogsSession -> SessionPayload for backward-compatible tool access + const getSessionContext = async (): Promise => { + if (!context.session) { + return { session: null, connectionId: context.sessionId ?? undefined } + } + const { username, accessToken, accessTokenSecret } = context.session + const sessionPayload: SessionPayload = { + userId: username, + accessToken, + accessTokenSecret, + iat: 0, + exp: 0, + } + return { session: sessionPayload, connectionId: context.sessionId ?? undefined } + } + + // Register all tools and resources + registerPublicTools(server, env, getSessionContext) + registerAuthenticatedTools(server, env, getSessionContext) + registerResources(server, env, getSessionContext) + registerPrompts(server) + + return { + server, + setContext: (ctx: Partial) => { + if (ctx.session !== undefined) context.session = ctx.session + if (ctx.sessionId !== undefined) context.sessionId = ctx.sessionId + if (ctx.baseUrl !== undefined) context.baseUrl = ctx.baseUrl + }, + getContext: () => ({ ...context }), + } } diff --git a/src/types/env.ts b/src/types/env.ts index b861d25..1cd5f9c 100644 --- a/src/types/env.ts +++ b/src/types/env.ts @@ -1,16 +1,21 @@ /** * Environment variables and bindings for the Cloudflare Worker */ +import type { OAuthHelpers } from '@cloudflare/workers-oauth-provider' + export interface Env { - // Discogs OAuth credentials - DISCOGS_CONSUMER_KEY: string - DISCOGS_CONSUMER_SECRET: string + // Discogs OAuth credentials + DISCOGS_CONSUMER_KEY: string + DISCOGS_CONSUMER_SECRET: string + + // OAuth provider helpers (injected by @cloudflare/workers-oauth-provider at runtime) + OAUTH_PROVIDER: OAuthHelpers - // JWT secret for signing session cookies - JWT_SECRET: string + // KV namespaces for logging, rate limiting, and sessions + MCP_LOGS: KVNamespace + MCP_RL: KVNamespace + MCP_SESSIONS: KVNamespace - // KV namespaces for logging, rate limiting, and sessions - MCP_LOGS: KVNamespace - MCP_RL: KVNamespace - MCP_SESSIONS: KVNamespace + // KV namespace for OAuth provider state (tokens, grants, client registrations) + OAUTH_KV: KVNamespace } diff --git a/test/auth/discogs.spec.ts b/test/auth/discogs.spec.ts index 998e592..940ccfb 100644 --- a/test/auth/discogs.spec.ts +++ b/test/auth/discogs.spec.ts @@ -157,6 +157,22 @@ describe('DiscogsAuth', () => { }) }) + describe('security', () => { + it('should not log the signing key or signature', async () => { + const consoleSpy = vi.spyOn(console, 'log') + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + statusText: 'OK', + text: () => Promise.resolve('oauth_token=tok&oauth_token_secret=sec'), + }) + await auth.getRequestToken('http://localhost/callback') + const loggedMessages = consoleSpy.mock.calls.map((args) => args.join(' ')) + expect(loggedMessages.some((msg) => msg.includes('signing key'))).toBe(false) + expect(loggedMessages.some((msg) => msg.includes('OAuth signature:'))).toBe(false) + }) + }) + describe('OAuth signature generation', () => { it('should generate consistent signatures for the same parameters', async () => { // Mock Date.now to return consistent timestamp diff --git a/test/auth/oauth-handler.spec.ts b/test/auth/oauth-handler.spec.ts new file mode 100644 index 0000000..ff3cee0 --- /dev/null +++ b/test/auth/oauth-handler.spec.ts @@ -0,0 +1,255 @@ +// ABOUTME: Tests for DiscogsOAuthHandler auth routes. +import { env, createExecutionContext, waitOnExecutionContext } from 'cloudflare:test' +import { describe, it, expect, vi } from 'vitest' +import { DiscogsOAuthHandler } from '../../src/auth/oauth-handler' + +// Mock DiscogsAuth at the top of the file (add after existing imports) +vi.mock('../../src/auth/discogs', () => ({ + DiscogsAuth: vi.fn().mockImplementation(() => ({ + getRequestToken: vi.fn().mockResolvedValue({ + oauth_token: 'mock-request-token', + oauth_token_secret: 'mock-request-secret', + oauth_callback_confirmed: 'true', + }), + getAccessToken: vi.fn().mockResolvedValue({ + oauth_token: 'mock-access-token', + oauth_token_secret: 'mock-access-secret', + }), + getAuthHeaders: vi.fn().mockResolvedValue({ Authorization: 'OAuth mock-header' }), + })), +})) + +const mockFetch = vi.fn() +vi.stubGlobal('fetch', mockFetch) + +describe('/.well-known/oauth-protected-resource', () => { + it('returns 200 with correct fields', async () => { + const req = new Request('https://example.com/.well-known/oauth-protected-resource') + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.resource).toBe('https://example.com') + expect(body.authorization_servers).toContain('https://example.com') + expect(body.bearer_methods_supported).toContain('header') + }) + + it('is accessible without authentication', async () => { + const req = new Request('https://example.com/.well-known/oauth-protected-resource') + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + // Must not be 401 or 403 — unauthenticated clients need to read this + expect(res.status).not.toBe(401) + expect(res.status).not.toBe(403) + }) +}) + +describe('/authorize', () => { + it('redirects to discogs.com/oauth/authorize with the request token', async () => { + const url = new URL('https://example.com/authorize') + url.searchParams.set('client_id', 'test-client') + url.searchParams.set('redirect_uri', 'https://client/callback') + url.searchParams.set('response_type', 'code') + url.searchParams.set('state', 'random123') + url.searchParams.set('code_challenge', 'abc123') + url.searchParams.set('code_challenge_method', 'S256') + + const mockOauthReqInfo = { + clientId: 'test-client', + redirectUri: 'https://client/callback', + responseType: 'code', + state: 'random123', + } + const envWithOAuth = { + ...env, + OAUTH_PROVIDER: { + parseAuthRequest: vi.fn().mockResolvedValue(mockOauthReqInfo), + }, + } + + const req = new Request(url.toString()) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, envWithOAuth as any, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(302) + const location = res.headers.get('Location') ?? '' + expect(location).toContain('discogs.com/oauth/authorize') + expect(location).toContain('oauth_token=mock-request-token') + }) +}) + +describe('/discogs-callback', () => { + it('completes authorization and redirects to client redirect_uri', async () => { + // Pre-seed KV with a pending oauth state + await env.MCP_SESSIONS.put( + 'oauth-pending:mock-request-token', + JSON.stringify({ + oauthReqInfo: { + clientId: 'test-client', + redirectUri: 'https://client/callback', + state: 'random123', + scope: [], + }, + requestTokenSecret: 'mock-request-secret', + }), + ) + + // Mock Discogs /oauth/identity response + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ id: 12345, username: 'testuser' }), + }) + + const envWithOAuth = { + ...env, + OAUTH_PROVIDER: { + completeAuthorization: vi.fn().mockResolvedValue({ + redirectTo: 'https://client/callback?code=test', + }), + }, + } + + const req = new Request( + 'https://example.com/discogs-callback?oauth_token=mock-request-token&oauth_verifier=mock-verifier', + ) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, envWithOAuth as any, ctx) + await waitOnExecutionContext(ctx) + + // Should redirect (302) — library issues the code redirect to client + expect([302, 303]).toContain(res.status) + }) + + it('returns 400 when oauth_token is missing', async () => { + const req = new Request('https://example.com/discogs-callback') + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(400) + }) + + it('returns 400 when KV entry is missing (expired)', async () => { + const req = new Request( + 'https://example.com/discogs-callback?oauth_token=no-such-token&oauth_verifier=x', + ) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(400) + }) +}) + +describe('/login (manual path)', () => { + it('redirects to discogs.com/oauth/authorize', async () => { + const req = new Request('https://example.com/login?session_id=test-session') + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(302) + const location = res.headers.get('Location') ?? '' + expect(location).toContain('discogs.com/oauth/authorize') + }) + + it('sets a CSRF cookie', async () => { + const req = new Request('https://example.com/login?session_id=test-session') + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + + const cookie = res.headers.get('Set-Cookie') ?? '' + expect(cookie).toContain('csrf') + }) + + it('stores pending login state in KV', async () => { + const req = new Request('https://example.com/login?session_id=test-session-kv') + const ctx = createExecutionContext() + await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + + const stored = await env.MCP_SESSIONS.get('login-pending:test-session-kv') + expect(stored).not.toBeNull() + const data = JSON.parse(stored!) + expect(data.csrfToken).toBeDefined() + expect(data.requestToken).toBeDefined() + expect(data.requestTokenSecret).toBeDefined() + }) +}) + +describe('/callback (manual path)', () => { + it('returns 400 when login-pending KV entry is missing', async () => { + const req = new Request( + 'https://example.com/callback?session_id=no-such-session&oauth_token=x&oauth_verifier=y', + ) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(400) + }) + + it('returns 403 when CSRF token is missing', async () => { + const csrfToken = 'test-csrf-token' + await env.MCP_SESSIONS.put( + 'login-pending:csrf-test', + JSON.stringify({ + sessionId: 'csrf-test', + csrfToken, + requestToken: 'tok', + requestTokenSecret: 'sec', + fromMcpClient: true, + timestamp: Date.now(), + }), + ) + const req = new Request( + 'https://example.com/callback?session_id=csrf-test&oauth_token=tok&oauth_verifier=x', + // No cookie + ) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(403) + }) + + it('stores session in KV and returns HTML success page when CSRF is valid', async () => { + const csrfToken = 'valid-csrf-token' + await env.MCP_SESSIONS.put( + 'login-pending:happy-path', + JSON.stringify({ + sessionId: 'happy-path', + csrfToken, + requestToken: 'mock-request-token', + requestTokenSecret: 'mock-request-secret', + fromMcpClient: true, + timestamp: Date.now(), + }), + ) + + // Mock identity fetch + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ id: 99, username: 'happyuser' }), + }) + + // URL is https so cookie name is __Host-csrf + const req = new Request( + 'https://example.com/callback?session_id=happy-path&oauth_token=mock-request-token&oauth_verifier=mock-verifier', + { headers: { Cookie: `__Host-csrf=${csrfToken}` } }, + ) + const ctx = createExecutionContext() + const res = await DiscogsOAuthHandler.fetch(req, env as any, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(200) + const body = await res.text() + expect(body).toContain('Authentication Successful') + + const session = await env.MCP_SESSIONS.get('session:happy-path') + expect(session).not.toBeNull() + const sessionData = JSON.parse(session!) + expect(sessionData.username).toBe('happyuser') + }) +}) diff --git a/test/index-oauth.spec.ts b/test/index-oauth.spec.ts new file mode 100644 index 0000000..eeb0977 --- /dev/null +++ b/test/index-oauth.spec.ts @@ -0,0 +1,175 @@ +// ABOUTME: Tests for src/index-oauth.ts — routing, 401 behavior, session paths, discovery. +import { env, createExecutionContext, waitOnExecutionContext } from 'cloudflare:test' +import { describe, it, expect, vi } from 'vitest' +import worker from '../src/index-oauth' + +// MCP initialize request body (required for the MCP handler to respond) +const MCP_INIT_BODY = JSON.stringify({ + jsonrpc: '2.0', + method: 'initialize', + params: { + protocolVersion: '2024-11-05', + capabilities: {}, + clientInfo: { name: 'TestClient', version: '1.0.0' }, + }, + id: 1, +}) + +const MCP_HEADERS = { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', +} + +describe('POST /mcp — unauthenticated', () => { + it('returns 401', async () => { + const req = new Request('https://example.com/mcp', { + method: 'POST', body: MCP_INIT_BODY, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(401) + }) + + it('includes WWW-Authenticate with resource_metadata', async () => { + const req = new Request('https://example.com/mcp', { + method: 'POST', body: MCP_INIT_BODY, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.headers.get('WWW-Authenticate')).toContain( + 'Bearer resource_metadata="https://example.com/.well-known/oauth-protected-resource"', + ) + }) + + it('does not contain a copy-paste login URL in the body', async () => { + const req = new Request('https://example.com/mcp', { + method: 'POST', body: MCP_INIT_BODY, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + const body = await res.text() + expect(body).not.toContain('/login?session_id=') + expect(body).not.toContain('/login?connection_id=') + }) +}) + +describe('POST /mcp — session_id param path', () => { + it('returns 401 JSON (no WWW-Authenticate) when session not in KV', async () => { + const req = new Request('https://example.com/mcp?session_id=nonexistent', { + method: 'POST', body: MCP_INIT_BODY, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(401) + // Must NOT have WWW-Authenticate — this is an explicit session claim + expect(res.headers.get('WWW-Authenticate')).toBeNull() + const body = await res.json() as any + expect(body.error).toBe('invalid_session') + }) + + it('returns 200 when session_id has a valid KV entry', async () => { + const sessionId = 'test-valid-session' + await env.MCP_SESSIONS.put( + `session:${sessionId}`, + JSON.stringify({ + username: 'testuser', + numericId: '12345', + accessToken: 'tok', + accessTokenSecret: 'sec', + expiresAt: Date.now() + 60 * 60 * 1000, + sessionId, + }), + ) + const req = new Request(`https://example.com/mcp?session_id=${sessionId}`, { + method: 'POST', body: MCP_INIT_BODY, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + }) +}) + +describe('POST /mcp — Mcp-Session-Id header', () => { + it('returns 401 + WWW-Authenticate when Mcp-Session-Id has no KV entry', async () => { + const req = new Request('https://example.com/mcp', { + method: 'POST', body: MCP_INIT_BODY, + headers: { ...MCP_HEADERS, 'Mcp-Session-Id': 'no-such-session' }, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(401) + expect(res.headers.get('WWW-Authenticate')).toContain('Bearer resource_metadata=') + }) + + it('returns 200 when Mcp-Session-Id has a valid KV entry', async () => { + const sessionId = 'mcp-header-session' + await env.MCP_SESSIONS.put( + `session:${sessionId}`, + JSON.stringify({ + username: 'testuser2', + numericId: '67890', + accessToken: 'tok2', + accessTokenSecret: 'sec2', + expiresAt: Date.now() + 60 * 60 * 1000, + sessionId, + }), + ) + const req = new Request('https://example.com/mcp', { + method: 'POST', body: MCP_INIT_BODY, + headers: { ...MCP_HEADERS, 'Mcp-Session-Id': sessionId }, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + }) +}) + +describe('GET /.well-known/oauth-protected-resource', () => { + it('returns 200 with required fields', async () => { + const req = new Request('https://example.com/.well-known/oauth-protected-resource') + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.resource).toBe('https://example.com') + expect(body.authorization_servers).toContain('https://example.com') + expect(body.bearer_methods_supported).toContain('header') + }) +}) + +describe('GET /.well-known/oauth-authorization-server', () => { + it('returns 200 with all MCP-required fields', async () => { + const req = new Request('https://example.com/.well-known/oauth-authorization-server') + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.issuer).toBeDefined() + expect(body.authorization_endpoint).toBeDefined() + expect(body.token_endpoint).toBeDefined() + expect(body.response_types_supported).toContain('code') + expect(body.grant_types_supported).toContain('authorization_code') + expect(body.code_challenge_methods_supported).toContain('S256') + }) +}) + +describe('GET /health', () => { + it('returns 200 with status ok', async () => { + const req = new Request('https://example.com/health') + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.status).toBe('ok') + }) +}) diff --git a/test/integration/mcp-client.test.ts b/test/integration/mcp-client.test.ts index 2329911..d67d55d 100644 --- a/test/integration/mcp-client.test.ts +++ b/test/integration/mcp-client.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' -import worker from '../../src/index' +import worker from '../../src/index-oauth' import type { Env } from '../../src/types/env' -import { createSessionToken } from '../../src/auth/jwt' // Mock KV namespaces const mockMCP_LOGS = { @@ -25,7 +24,6 @@ const mockMCP_SESSIONS = { const mockEnv: Env = { DISCOGS_CONSUMER_KEY: 'test-key', DISCOGS_CONSUMER_SECRET: 'test-secret', - JWT_SECRET: 'test-jwt-secret-for-integration-tests', MCP_LOGS: mockMCP_LOGS as any, MCP_RL: mockMCP_RL as any, MCP_SESSIONS: mockMCP_SESSIONS as any, @@ -122,7 +120,7 @@ function parseSSE(text: string): any | null { * Mock MCP Client - simulates Claude Desktop or other MCP clients */ class MockMCPClient { - public sessionCookie: string | null = null + public sessionId: string | null = null private requestId = 1 private getNextId(): number { @@ -130,12 +128,14 @@ class MockMCPClient { } private async makeRequest(body: any): Promise { - const request = new Request('http://localhost:8787/mcp', { + const url = this.sessionId + ? `http://localhost:8787/mcp?session_id=${this.sessionId}` + : 'http://localhost:8787/mcp' + const request = new Request(url, { method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json, text/event-stream', - ...(this.sessionCookie ? { Cookie: this.sessionCookie } : {}), }, body: JSON.stringify(body), }) @@ -198,17 +198,19 @@ class MockMCPClient { } async authenticate(): Promise { - const sessionPayload = { + // In OAuth flow, sessions are stored in KV keyed as `session:` + const sessionId = 'test-session-123' + const sessionData = JSON.stringify({ userId: 'test-user-123', username: 'testuser', accessToken: 'test-access-token', accessTokenSecret: 'test-access-secret', iat: Math.floor(Date.now() / 1000), exp: Math.floor(Date.now() / 1000) + 3600, - } + }) - const sessionToken = await createSessionToken(sessionPayload, mockEnv.JWT_SECRET) - this.sessionCookie = `session=${sessionToken}` + mockMCP_SESSIONS.get.mockResolvedValue(sessionData) + this.sessionId = sessionId } async listResources(): Promise { @@ -315,6 +317,10 @@ describe('MCP Client Integration Tests', () => { describe('Full MCP Protocol Flow', () => { it('should complete full initialization handshake', async () => { + // Session path is required — OAuth provider intercepts requests without a bearer token. + // Authenticate first so requests are routed through the KV session path. + await client.authenticate() + const initResult = await client.initialize() expect(initResult).toMatchObject({ @@ -339,16 +345,12 @@ describe('MCP Client Integration Tests', () => { }) it('should handle unauthenticated access to protected resources', async () => { - await client.initialize() - await client.sendInitialized() - + // Without a session_id param and without a bearer token, the OAuth provider + // returns 401 invalid_token. Verify that behavior. const result = await client.readResource('discogs://collection') expect(result).toMatchObject({ - jsonrpc: '2.0', - error: { - code: -32603, - }, + error: 'invalid_token', }) }) diff --git a/test/manual-login.spec.ts b/test/manual-login.spec.ts new file mode 100644 index 0000000..83b777f --- /dev/null +++ b/test/manual-login.spec.ts @@ -0,0 +1,121 @@ +// ABOUTME: Integration test for the manual login path (/login → /callback → /mcp?session_id=). +import { env, createExecutionContext, waitOnExecutionContext } from 'cloudflare:test' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import worker from '../src/index-oauth' + +vi.mock('../src/auth/discogs', () => ({ + DiscogsAuth: vi.fn().mockImplementation(() => ({ + getRequestToken: vi.fn().mockResolvedValue({ + oauth_token: 'manual-request-token', + oauth_token_secret: 'manual-request-secret', + oauth_callback_confirmed: 'true', + }), + getAccessToken: vi.fn().mockResolvedValue({ + oauth_token: 'manual-access-token', + oauth_token_secret: 'manual-access-secret', + }), + getAuthHeaders: vi.fn().mockResolvedValue({ Authorization: 'OAuth mock' }), + })), +})) + +const mockFetch = vi.fn() +vi.stubGlobal('fetch', mockFetch) + +const BASE_URL = 'https://example.com' +const SESSION_ID = 'manual-test-session' + +const MCP_INIT = JSON.stringify({ + jsonrpc: '2.0', method: 'initialize', + params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'Test', version: '1.0' } }, + id: 1, +}) +const MCP_HEADERS = { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream' } + +describe('Manual login round-trip', () => { + beforeEach(() => { vi.clearAllMocks() }) + + it('Step 1: GET /login redirects to Discogs and stores pending state', async () => { + const req = new Request(`${BASE_URL}/login?session_id=${SESSION_ID}`) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(302) + expect(res.headers.get('Location')).toContain('discogs.com/oauth/authorize') + + // Capture CSRF cookie for step 2 + const setCookie = res.headers.get('Set-Cookie') ?? '' + expect(setCookie).toContain('csrf') + + // Verify KV entry was written + const pending = await env.MCP_SESSIONS.get(`login-pending:${SESSION_ID}`) + expect(pending).not.toBeNull() + const data = JSON.parse(pending!) + expect(data.csrfToken).toBeDefined() + expect(data.requestToken).toBe('manual-request-token') + }) + + it('Step 2: GET /callback with valid CSRF stores session and returns success page', async () => { + // Seed pending state + const csrfToken = 'test-csrf-manual' + await env.MCP_SESSIONS.put( + `login-pending:${SESSION_ID}`, + JSON.stringify({ + sessionId: SESSION_ID, + csrfToken, + requestToken: 'manual-request-token', + requestTokenSecret: 'manual-request-secret', + fromMcpClient: true, + timestamp: Date.now(), + }), + ) + + // Mock identity API + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ id: 777, username: 'manualuser' }), + }) + + const req = new Request( + `${BASE_URL}/callback?session_id=${SESSION_ID}&oauth_token=manual-request-token&oauth_verifier=manual-verifier`, + { headers: { Cookie: `__Host-csrf=${csrfToken}` } }, // HTTPS uses __Host-csrf + ) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(200) + const body = await res.text() + expect(body).toContain('Authentication Successful') + expect(body).toContain('manualuser') + + const session = await env.MCP_SESSIONS.get(`session:${SESSION_ID}`) + expect(session).not.toBeNull() + const sessionData = JSON.parse(session!) + expect(sessionData.username).toBe('manualuser') + }) + + it('Step 3: POST /mcp?session_id=... returns 200 after successful login', async () => { + // Seed valid session (as if step 2 completed) + await env.MCP_SESSIONS.put( + `session:${SESSION_ID}`, + JSON.stringify({ + username: 'manualuser', + numericId: '777', + accessToken: 'manual-access-token', + accessTokenSecret: 'manual-access-secret', + expiresAt: Date.now() + 60 * 60 * 1000, + sessionId: SESSION_ID, + }), + ) + + const req = new Request(`${BASE_URL}/mcp?session_id=${SESSION_ID}`, { + method: 'POST', body: MCP_INIT, headers: MCP_HEADERS, + }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(200) + }) +}) diff --git a/test/oauth-roundtrip.spec.ts b/test/oauth-roundtrip.spec.ts new file mode 100644 index 0000000..bb84a77 --- /dev/null +++ b/test/oauth-roundtrip.spec.ts @@ -0,0 +1,163 @@ +// ABOUTME: Integration test for the full MCP OAuth 2.1 round-trip. +// ABOUTME: Verifies: 401 → discovery → /authorize → /discogs-callback → token → /mcp 200. +import { env, createExecutionContext, waitOnExecutionContext } from 'cloudflare:test' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import worker from '../src/index-oauth' + +// Mock DiscogsAuth so we don't hit real Discogs APIs +vi.mock('../src/auth/discogs', () => ({ + DiscogsAuth: vi.fn().mockImplementation(() => ({ + getRequestToken: vi.fn().mockResolvedValue({ + oauth_token: 'mock-request-token', + oauth_token_secret: 'mock-request-secret', + oauth_callback_confirmed: 'true', + }), + getAccessToken: vi.fn().mockResolvedValue({ + oauth_token: 'mock-access-token', + oauth_token_secret: 'mock-access-secret', + }), + getAuthHeaders: vi.fn().mockResolvedValue({ + Authorization: 'OAuth mock-auth', + }), + })), +})) + +// Mock fetch for Discogs /oauth/identity +const mockFetch = vi.fn() +vi.stubGlobal('fetch', mockFetch) + +const BASE_URL = 'https://discogs-mcp-prod.example.com' + +const MCP_INIT = JSON.stringify({ + jsonrpc: '2.0', method: 'initialize', + params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'Test', version: '1.0' } }, + id: 1, +}) +const MCP_HEADERS = { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream' } + +/** + * Compute PKCE S256 code challenge + */ +async function computeS256Challenge(verifier: string): Promise { + const data = new TextEncoder().encode(verifier) + const digest = await crypto.subtle.digest('SHA-256', data) + return btoa(String.fromCharCode(...new Uint8Array(digest))) + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '') +} + +describe('Full OAuth round-trip', () => { + beforeEach(() => { vi.clearAllMocks() }) + + it('Step 1: POST /mcp returns 401 + WWW-Authenticate', async () => { + const req = new Request(`${BASE_URL}/mcp`, { method: 'POST', body: MCP_INIT, headers: MCP_HEADERS }) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(401) + expect(res.headers.get('WWW-Authenticate')).toContain('Bearer resource_metadata=') + }) + + it('Step 2: GET /.well-known/oauth-protected-resource returns authorization_servers', async () => { + const req = new Request(`${BASE_URL}/.well-known/oauth-protected-resource`) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.authorization_servers).toBeDefined() + expect(body.authorization_servers.length).toBeGreaterThan(0) + }) + + it('Step 3: GET /.well-known/oauth-authorization-server returns valid metadata', async () => { + const req = new Request(`${BASE_URL}/.well-known/oauth-authorization-server`) + const ctx = createExecutionContext() + const res = await worker.fetch(req, env, ctx) + await waitOnExecutionContext(ctx) + expect(res.status).toBe(200) + const body = await res.json() as any + expect(body.issuer).toBeDefined() + expect(body.authorization_endpoint).toBeDefined() + expect(body.token_endpoint).toBeDefined() + expect(body.code_challenge_methods_supported).toContain('S256') + }) + + it('Step 4: GET /authorize redirects to discogs.com/oauth/authorize', async () => { + const verifier = 'test-verifier-abcdefg12345678' + const challenge = await computeS256Challenge(verifier) + + const url = new URL(`${BASE_URL}/authorize`) + url.searchParams.set('client_id', 'test-client-id') + url.searchParams.set('redirect_uri', `${BASE_URL}/callback`) + url.searchParams.set('response_type', 'code') + url.searchParams.set('state', 'test-state-123') + url.searchParams.set('code_challenge', challenge) + url.searchParams.set('code_challenge_method', 'S256') + + const mockOauthReqInfo = { + clientId: 'test-client-id', + redirectUri: `${BASE_URL}/callback`, + responseType: 'code', + state: 'test-state-123', + scope: [], + } + const envWithOAuth = { + ...env, + OAUTH_PROVIDER: { + parseAuthRequest: vi.fn().mockResolvedValue(mockOauthReqInfo), + }, + } + + const req = new Request(url.toString()) + const ctx = createExecutionContext() + const res = await worker.fetch(req, envWithOAuth as any, ctx) + await waitOnExecutionContext(ctx) + + expect(res.status).toBe(302) + const location = res.headers.get('Location') ?? '' + expect(location).toContain('discogs.com/oauth/authorize') + expect(location).toContain('oauth_token=mock-request-token') + }) + + it('Step 5–6: GET /discogs-callback completes authorization and redirects to client', async () => { + // Pre-seed KV with pending OAuth state (as if /authorize ran) + await env.MCP_SESSIONS.put( + 'oauth-pending:mock-request-token', + JSON.stringify({ + oauthReqInfo: { + clientId: 'test-client-id', + redirectUri: `${BASE_URL}/callback`, + state: 'test-state-123', + scope: [], + }, + requestTokenSecret: 'mock-request-secret', + }), + ) + + // Mock identity API + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ id: 42, username: 'discogsuser' }), + }) + + const envWithOAuth = { + ...env, + OAUTH_PROVIDER: { + completeAuthorization: vi.fn().mockResolvedValue({ + redirectTo: `${BASE_URL}/callback?code=test-code`, + }), + }, + } + + const req = new Request( + `${BASE_URL}/discogs-callback?oauth_token=mock-request-token&oauth_verifier=mock-verifier`, + ) + const ctx = createExecutionContext() + const res = await worker.fetch(req, envWithOAuth as any, ctx) + await waitOnExecutionContext(ctx) + + expect([302, 303]).toContain(res.status) + // Pending KV entry should be deleted + const pending = await env.MCP_SESSIONS.get('oauth-pending:mock-request-token') + expect(pending).toBeNull() + }) +}) diff --git a/tsconfig.json b/tsconfig.json index 1377379..cbfbabb 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -35,9 +35,8 @@ "strict": true, /* Skip type checking all .d.ts files. */ - "skipLibCheck": true, - "types": ["./tools/worker-configuration.d.ts"] + "skipLibCheck": true }, - "exclude": ["test"], - "include": ["tools/worker-configuration.d.ts", "src/**/*.ts"] + "exclude": ["test", "src/index.ts", "src/mcp/server.ts"], + "include": ["src/**/*.ts"] } diff --git a/wrangler.toml b/wrangler.toml index 5582fac..fba35ba 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -1,5 +1,5 @@ name = "discogs-mcp" -main = "src/index.ts" +main = "src/index-oauth.ts" compatibility_date = "2024-12-01" compatibility_flags = ["nodejs_compat"] @@ -19,6 +19,10 @@ id = "b4caaaa688cc45d2a9a85fe67ba53bfc" binding = "MCP_SESSIONS" id = "a0bc40f523d54a6ab70fa947ac3daeaf" +[[kv_namespaces]] +binding = "OAUTH_KV" +id = "e04a5ec5441949ebb6e84281bf6598c3" + # Production environment [env.production] name = "discogs-mcp-prod" @@ -35,7 +39,10 @@ id = "bfc831cedceb4a08a6d997909c3899ce" binding = "MCP_SESSIONS" id = "e45edcc5347e4b409169c1d0a9b2ed5d" +[[env.production.kv_namespaces]] +binding = "OAUTH_KV" +id = "35c396a7d0924dd8a5527334cd4538d3" + # Secrets (set via wrangler secret put): # - DISCOGS_CONSUMER_KEY -# - DISCOGS_CONSUMER_SECRET -# - JWT_SECRET \ No newline at end of file +# - DISCOGS_CONSUMER_SECRET \ No newline at end of file