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

schema/oauth.graphql

@@ -34,6 +34,23 @@ type harper_oauth_mcp_clients @table(database: "oauth") {
 	client_secret_expires_at: Float # 0 = never expires
 }
 
+## MCP Authorization Codes (OAuth 2.1 + RFC 7636 PKCE + RFC 8707 audience)
+## Short-lived codes minted by /oauth/mcp/authorize on successful upstream auth.
+## Single-use: /oauth/mcp/token (Stage 4) reads-then-deletes. TTL via Harper.
+## Auto-expires after 5 minutes — codes are exchanged immediately after the
+## upstream redirect, so a longer window only widens the replay attack surface.
+type mcp_auth_codes @table(database: "oauth", expiration: 300) {
+	code: ID @primaryKey
+	client_id: String
+	user: String # Harper user identifier
+	resource: String # RFC 8707 audience the issued token will be bound to
+	code_challenge: String # RFC 7636 PKCE; verified at /token
+	code_challenge_method: String # Always "S256"
+	redirect_uri: String # Bound at issuance; /token must match
+	scope: String # Space-separated; may be empty
+	created_at: Float
+}
+
 ## OAuth User Session table (optional, for future use)
 ## Could store OAuth-specific user data
 # type oauth_sessions @table(database: "oauth") {

src/lib/handlers.ts

@@ -8,7 +8,8 @@ import { readFile } from 'node:fs/promises';
 import { join, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import type { RequestTarget } from 'harper';
-import type { Request, Logger, IOAuthProvider, OAuthProviderConfig } from '../types.ts';
+import type { Request, Logger, IOAuthProvider, MCPAuthorizeState, OAuthProviderConfig } from '../types.ts';
+import { handleMCPCallback } from './mcp/index.ts';
 import type { HookManager } from './hookManager.ts';
 
 /**
@@ -131,61 +132,92 @@ export async function handleCallback(
 	const error = target.get?.('error');
 	const errorDescription = target.get?.('error_description');
 
-	// Handle OAuth errors from provider
-	if (error) {
-		logger?.error?.(`OAuth error: ${error} - ${errorDescription}`);
-		const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', { error: 'oauth_failed', reason: error });
-		return {
-			status: 302,
-			headers: {
-				Location: errorUrl,
-			},
-		};
-	}
+	// Helper: build an MCP-aware error redirect to the MCP client's redirect_uri.
+	// Only used in MCP branches — the human path keeps its existing
+	// buildErrorRedirect shape (error code only, optionally with reason) to
+	// avoid changing observable behavior on the human OAuth flow.
+	const mcpErrorRedirect = (mcp: MCPAuthorizeState, errorCode: string, description: string) => {
+		const url = new URL(mcp.redirectUri);
+		url.searchParams.set('error', errorCode);
+		url.searchParams.set('error_description', description);
+		if (mcp.clientState) url.searchParams.set('state', mcp.clientState);
+		return { status: 302, headers: { Location: url.toString() } };
+	};
 
-	// Validate parameters
-	if (!code || !state) {
-		logger?.warn?.('Missing required OAuth callback parameters');
+	// Validate state presence — we use it both to route errors (via mcp
+	// payload, when present) AND to defend against CSRF.
+	if (!state) {
+		// Without state, we can't be in an MCP flow (MCP always sets state).
+		// Preserve the legacy human-OAuth error UX: if the IdP sent an error,
+		// echo it to postLoginRedirect with the original reason. Otherwise,
+		// generic invalid_request.
+		if (error) {
+			logger?.error?.(`OAuth error (no state): ${error} - ${errorDescription}`);
+			const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', {
+				error: 'oauth_failed',
+				reason: error,
+			});
+			return { status: 302, headers: { Location: errorUrl } };
+		}
+		logger?.warn?.('Missing state parameter in OAuth callback');
 		const errorUrl = buildErrorRedirect(config.postLoginRedirect || '/', { error: 'invalid_request' });
-		return {
-			status: 302,
-			headers: {
-				Location: errorUrl,
-			},
-		};
+		return { status: 302, headers: { Location: errorUrl } };
 	}
 
-	// Verify CSRF token
+	// Verify CSRF token FIRST — before checking the upstream error param.
+	// This is what lets MCP-initiated errors route back to the MCP client's
+	// redirect_uri instead of the Harper app's default path. The token is
+	// single-use; consuming it on error is fine because OAuth callbacks
+	// aren't retried with the same state.
 	const tokenData = await provider.verifyCSRFToken(state);
 	if (!tokenData) {
 		logger?.warn?.('Invalid or expired CSRF token');
-		// Redirect back to login with error parameter
+		// We can't know if this was an MCP flow (state didn't decode), so
+		// fall back to a generic redirect — same behavior as pre-fix.
 		const loginUrl = `/oauth/${providerName}/login?error=session_expired`;
-		return {
-			status: 302,
-			headers: {
-				Location: loginUrl,
-			},
-		};
+		return { status: 302, headers: { Location: loginUrl } };
 	}
 
-	// Verify state token was issued for THIS provider (prevents cross-provider attacks)
+	const mcpState = tokenData.mcp as MCPAuthorizeState | undefined;
+
+	// Verify state token was issued for THIS provider (prevents cross-provider attacks).
 	if (tokenData.providerName !== providerName) {
 		logger?.warn?.(
 			`State token provider mismatch: token issued for '${tokenData.providerName}', callback for '${providerName}'`
 		);
-		// This could be an attack - redirect back to original URL with error
-		// Do NOT redirect to login endpoint as that would restart OAuth flow
+		if (mcpState) {
+			return mcpErrorRedirect(mcpState, 'invalid_request', 'cross-provider state mismatch');
+		}
 		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
 			error: 'auth_failed',
 			reason: 'csrf',
 		});
-		return {
-			status: 302,
-			headers: {
-				Location: errorUrl,
-			},
-		};
+		return { status: 302, headers: { Location: errorUrl } };
+	}
+
+	// Now that we know the flow context, handle upstream IdP errors.
+	if (error) {
+		logger?.error?.(`OAuth error: ${error} - ${errorDescription}`);
+		if (mcpState) {
+			return mcpErrorRedirect(mcpState, error, errorDescription || error);
+		}
+		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
+			error: 'oauth_failed',
+			reason: error,
+		});
+		return { status: 302, headers: { Location: errorUrl } };
+	}
+
+	// Validate code presence — needed regardless of flow.
+	if (!code) {
+		logger?.warn?.('Missing authorization code in OAuth callback');
+		if (mcpState) {
+			return mcpErrorRedirect(mcpState, 'invalid_request', 'Missing authorization code');
+		}
+		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
+			error: 'invalid_request',
+		});
+		return { status: 302, headers: { Location: errorUrl } };
 	}
 
 	try {
@@ -215,6 +247,17 @@ export async function handleCallback(
 		// Pass providerName (registry key) not config.provider (provider type) for multi-tenant support
 		const hookData = await hookManager.callOnLogin(user, tokenResponse, request.session, request, providerName);
 
+		// MCP branch: if the CSRF state was minted by /oauth/mcp/authorize, the
+		// upstream callback's job is to mint an MCP authorization code, NOT to
+		// create a Harper session. Independent lifecycle per #86 resolved
+		// decision. The upstream IdP token never reaches the MCP client.
+		// Pass the onLogin-mapped username so the issued auth code (and the
+		// JWT exchanged for it in Stage 4) is bound to the correct identity.
+		if (mcpState) {
+			const userIdentifier = hookData?.user ?? user.username;
+			return handleMCPCallback(request, mcpState, userIdentifier, logger);
+		}
+
 		// Store in session if available
 		if (request.session) {
 			// Calculate token expiration and refresh thresholds
@@ -288,16 +331,14 @@ export async function handleCallback(
 		else if (message.includes('claim')) reason = 'user_mapping';
 		else if (message.includes('user info') || message.includes('userinfo')) reason = 'user_info';
 		else if (message.includes('hook') || message.includes('onLogin')) reason = 'login_hook';
+		if (mcpState) {
+			return mcpErrorRedirect(mcpState, 'server_error', reason);
+		}
 		const errorUrl = buildErrorRedirect(tokenData.originalUrl || config.postLoginRedirect || '/', {
 			error: 'auth_failed',
 			reason,
 		});
-		return {
-			status: 302,
-			headers: {
-				Location: errorUrl,
-			},
-		};
+		return { status: 302, headers: { Location: errorUrl } };
 	}
 }
 

src/lib/mcp/authCodeStore.ts

@@ -0,0 +1,112 @@
+/**
+ * MCP Authorization Code Store
+ *
+ * Persists short-lived authorization codes in the `mcp_auth_codes` Harper
+ * table. The table is declared with `@table(expiration: 300)`, so codes
+ * auto-expire after 5 minutes — Stage 4's /token endpoint must still
+ * delete on successful exchange (single-use), but the TTL is a safety
+ * net against codes that are never redeemed.
+ *
+ * Explicit field access on encode/decode (no `{ ...raw }`) — Harper
+ * tracked-object Proxies return empty own-keys, so spread would drop
+ * scalar fields. See CLAUDE.md "GenericTrackedObject + spread" gotcha.
+ */
+
+import type { Logger, MCPAuthCodeRecord, Table } from '../../types.ts';
+
+declare const databases: any;
+
+let authCodesTable: Table | undefined;
+
+function getAuthCodesTable(): Table {
+	if (!authCodesTable) {
+		if (!databases?.oauth?.mcp_auth_codes) {
+			throw new Error(
+				'OAuth MCP auth codes table (oauth.mcp_auth_codes) not found. ' +
+					'Please ensure the OAuth plugin is properly installed with its schema.'
+			);
+		}
+		authCodesTable = databases.oauth.mcp_auth_codes;
+	}
+	return authCodesTable as Table;
+}
+
+/**
+ * Reset the cached table reference (for testing only)
+ * @internal
+ */
+export function resetMCPAuthCodesTableCache(): void {
+	authCodesTable = undefined;
+}
+
+function encodeRecord(record: MCPAuthCodeRecord): Record<string, any> {
+	return {
+		code: record.code,
+		client_id: record.client_id,
+		user: record.user,
+		resource: record.resource,
+		code_challenge: record.code_challenge,
+		code_challenge_method: record.code_challenge_method,
+		redirect_uri: record.redirect_uri,
+		scope: record.scope,
+		created_at: record.created_at,
+	};
+}
+
+function decodeRecord(raw: Record<string, any>): MCPAuthCodeRecord {
+	return {
+		code: raw.code,
+		client_id: raw.client_id,
+		user: raw.user,
+		resource: raw.resource,
+		code_challenge: raw.code_challenge,
+		code_challenge_method: raw.code_challenge_method,
+		redirect_uri: raw.redirect_uri,
+		scope: raw.scope ?? undefined,
+		created_at: raw.created_at,
+	};
+}
+
+export class MCPAuthCodeStore {
+	private logger?: Logger;
+
+	constructor(logger?: Logger) {
+		this.logger = logger;
+	}
+
+	async set(record: MCPAuthCodeRecord): Promise<void> {
+		const table = getAuthCodesTable();
+		try {
+			await table.put(encodeRecord(record));
+			this.logger?.debug?.(`Stored MCP auth code for client ${record.client_id}`);
+		} catch (error) {
+			this.logger?.error?.('Failed to store MCP auth code:', error);
+			throw error;
+		}
+	}
+
+	async get(code: string): Promise<MCPAuthCodeRecord | null> {
+		const table = getAuthCodesTable();
+		try {
+			const raw = await table.get(code);
+			if (!raw || !raw.code) {
+				return null;
+			}
+			return decodeRecord(raw);
+		} catch (error) {
+			this.logger?.error?.('Failed to retrieve MCP auth code:', error);
+			return null;
+		}
+	}
+
+	async delete(code: string): Promise<void> {
+		const table = getAuthCodesTable();
+		try {
+			await table.delete(code);
+			this.logger?.debug?.('Deleted MCP auth code');
+		} catch (error) {
+			// Non-critical: TTL will eventually evict.
+			this.logger?.warn?.('Failed to delete MCP auth code:', error);
+		}
+	}
+}