From b4c54b7b98ffabbdaf0b6d28cfdbafd6945c124b Mon Sep 17 00:00:00 2001 From: Atte Huhtakangas Date: Tue, 20 Jan 2026 21:05:23 +0000 Subject: [PATCH 1/9] feat: add OAuth support for HTTP MCP servers - Add McpCliOAuthProvider implementing OAuthClientProvider interface - File-based token storage in ~/.mcp-cli/{tokens,clients,verifiers} - Support authorization_code and client_credentials grant types - Auto-create OAuth provider for all HTTP servers (enables server-initiated OAuth) - Handle OAuth callback with local HTTP server on configurable port - Cross-platform browser opening for authorization flow - Detect OAuth errors from UnauthorizedError and invalid_token responses This enables MCP servers like Linear that require OAuth authentication. Co-Authored-By: Claude Opus 4.5 --- src/client.ts | 116 ++++++++++- src/config.ts | 74 +++++++ src/errors.ts | 43 ++++ src/oauth.ts | 487 +++++++++++++++++++++++++++++++++++++++++++ tests/config.test.ts | 155 ++++++++++++++ tests/oauth.test.ts | 350 +++++++++++++++++++++++++++++++ 6 files changed, 1216 insertions(+), 9 deletions(-) create mode 100644 src/oauth.ts create mode 100644 tests/oauth.test.ts diff --git a/src/client.ts b/src/client.ts index 50731cf..4a02840 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2,6 +2,7 @@ * MCP Client - Connection management for MCP servers */ +import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; @@ -25,6 +26,8 @@ import { cleanupOrphanedDaemons, getDaemonConnection, } from './daemon-client.js'; +import { formatCliError, oauthFlowError } from './errors.js'; +import { McpCliOAuthProvider, waitForOAuthCallback } from './oauth.js'; import { VERSION } from './version.js'; // Re-export config utilities for convenience @@ -217,6 +220,7 @@ export async function safeClose(close: () => Promise): Promise { /** * Connect to an MCP server with retry logic * Captures stderr from stdio servers to include in error messages + * Handles OAuth flow for HTTP servers with authentication */ export async function connectToServer( serverName: string, @@ -237,9 +241,12 @@ export async function connectToServer( ); let transport: StdioClientTransport | StreamableHTTPClientTransport; + let authProvider: McpCliOAuthProvider | undefined; if (isHttpServer(config)) { - transport = createHttpTransport(config); + const result = createHttpTransport(serverName, config); + transport = result.transport; + authProvider = result.authProvider; } else { transport = createStdioTransport(config); @@ -259,13 +266,69 @@ export async function connectToServer( try { await client.connect(transport); } catch (error) { - // Enhance error with captured stderr - const stderrOutput = stderrChunks.join('').trim(); - if (stderrOutput) { - const err = error as Error; - err.message = `${err.message}\n\nServer stderr:\n${stderrOutput}`; + // Handle OAuth authorization required + if (isOAuthNeeded(error as Error) && authProvider) { + debug(`OAuth authorization required for ${serverName}`); + + // For authorization_code flow, wait for callback + const oauthConfig = (config as HttpServerConfig).oauth; + if ( + !oauthConfig?.grantType || + oauthConfig.grantType === 'authorization_code' + ) { + try { + const port = authProvider.getCallbackPort(); + console.error('Waiting for OAuth authorization...'); + + const { code } = await waitForOAuthCallback(port); + debug('Authorization code received, completing OAuth flow'); + + // Complete the OAuth flow with the authorization code + await (transport as StreamableHTTPClientTransport).finishAuth(code); + + // Create new client and transport for authenticated connection + // (original transport is in started state and can't be reused) + debug('Creating new connection with authenticated tokens'); + const newClient = new Client( + { name: 'mcp-cli', version: VERSION }, + { capabilities: {} }, + ); + const newResult = createHttpTransport(serverName, config as HttpServerConfig); + await newClient.connect(newResult.transport); + + return { + client: newClient, + close: async () => { + await newClient.close(); + }, + }; + } catch (oauthError) { + throw new Error( + formatCliError( + oauthFlowError(serverName, (oauthError as Error).message), + ), + ); + } + } else { + // client_credentials should not need interactive flow + throw new Error( + formatCliError( + oauthFlowError( + serverName, + 'client_credentials authentication failed - check clientId and clientSecret', + ), + ), + ); + } + } else { + // Enhance error with captured stderr + const stderrOutput = stderrChunks.join('').trim(); + if (stderrOutput) { + const err = error as Error; + err.message = `${err.message}\n\nServer stderr:\n${stderrOutput}`; + } + throw error; } - throw error; } // For successful connections, forward stderr to console @@ -289,17 +352,52 @@ export async function connectToServer( /** * Create HTTP transport for remote servers + * Always creates an auth provider to handle OAuth flows (server-initiated or explicit) */ function createHttpTransport( + serverName: string, config: HttpServerConfig, -): StreamableHTTPClientTransport { +): { + transport: StreamableHTTPClientTransport; + authProvider: McpCliOAuthProvider; +} { const url = new URL(config.url); - return new StreamableHTTPClientTransport(url, { + // Always create OAuth provider for HTTP servers to handle server-initiated OAuth + // If explicit oauth config exists, use it; otherwise use defaults + const oauthConfig = config.oauth || {}; + const authProvider = new McpCliOAuthProvider( + serverName, + config.url, + oauthConfig, + ); + debug( + `OAuth provider created for ${serverName} (${oauthConfig.grantType || 'authorization_code'})`, + ); + + const transport = new StreamableHTTPClientTransport(url, { + authProvider, requestInit: { headers: config.headers, }, }); + + return { transport, authProvider }; +} + +/** + * Check if an error indicates OAuth authorization is needed + */ +function isOAuthNeeded(error: Error): boolean { + // Check for UnauthorizedError from SDK + if (error instanceof UnauthorizedError) { + return true; + } + // Check for invalid_token error in message (common OAuth error response) + if (error.message.includes('invalid_token')) { + return true; + } + return false; } /** diff --git a/src/config.ts b/src/config.ts index 99a4e25..577ef5a 100644 --- a/src/config.ts +++ b/src/config.ts @@ -41,6 +41,17 @@ export interface StdioServerConfig extends BaseServerConfig { cwd?: string; } +/** + * OAuth configuration for HTTP servers + */ +export interface OAuthConfig { + grantType?: 'authorization_code' | 'client_credentials'; + clientId?: string; + clientSecret?: string; + scope?: string; + callbackPort?: number; +} + /** * HTTP server configuration (remote) */ @@ -48,6 +59,7 @@ export interface HttpServerConfig extends BaseServerConfig { url: string; headers?: Record; timeout?: number; + oauth?: OAuthConfig; } export type ServerConfig = StdioServerConfig | HttpServerConfig; @@ -494,6 +506,68 @@ export async function loadConfig( }), ); } + + // Validate OAuth config if present (only for HTTP servers) + if (hasUrl && 'oauth' in serverConfig && serverConfig.oauth) { + const oauth = serverConfig.oauth as Record; + + // Validate grantType if specified + if ( + oauth.grantType && + oauth.grantType !== 'authorization_code' && + oauth.grantType !== 'client_credentials' + ) { + throw new Error( + formatCliError({ + code: ErrorCode.CLIENT_ERROR, + type: 'CONFIG_INVALID_OAUTH', + message: `Invalid OAuth grantType for server "${serverName}"`, + details: `Got "${oauth.grantType}", expected "authorization_code" or "client_credentials"`, + suggestion: `Use "authorization_code" for interactive login or "client_credentials" for machine-to-machine auth`, + }), + ); + } + + // client_credentials requires clientId and clientSecret + if (oauth.grantType === 'client_credentials') { + if (!oauth.clientId) { + throw new Error( + formatCliError({ + code: ErrorCode.CLIENT_ERROR, + type: 'CONFIG_INVALID_OAUTH', + message: `OAuth client_credentials flow requires clientId for server "${serverName}"`, + suggestion: `Add "clientId" to the oauth config, e.g., "clientId": "\${MCP_CLIENT_ID}"`, + }), + ); + } + if (!oauth.clientSecret) { + throw new Error( + formatCliError({ + code: ErrorCode.CLIENT_ERROR, + type: 'CONFIG_INVALID_OAUTH', + message: `OAuth client_credentials flow requires clientSecret for server "${serverName}"`, + suggestion: `Add "clientSecret" to the oauth config, e.g., "clientSecret": "\${MCP_CLIENT_SECRET}"`, + }), + ); + } + } + + // Validate callbackPort if specified + if (oauth.callbackPort !== undefined) { + const port = Number(oauth.callbackPort); + if (Number.isNaN(port) || port < 1 || port > 65535) { + throw new Error( + formatCliError({ + code: ErrorCode.CLIENT_ERROR, + type: 'CONFIG_INVALID_OAUTH', + message: `Invalid callbackPort for server "${serverName}"`, + details: 'Port must be a number between 1 and 65535', + suggestion: `Use a valid port number, e.g., "callbackPort": 8095`, + }), + ); + } + } + } } // Substitute environment variables diff --git a/src/errors.ts b/src/errors.ts index 7883799..50534eb 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -384,3 +384,46 @@ export function tooManyArgumentsError( suggestion: `Run 'mcp-cli --help' for correct usage`, }; } + +// ============================================================================ +// OAuth Errors +// ============================================================================ + +export function oauthConfigError( + serverName: string, + details: string, +): CliError { + return { + code: ErrorCode.CLIENT_ERROR, + type: 'OAUTH_CONFIG_ERROR', + message: `Invalid OAuth configuration for server "${serverName}"`, + details, + suggestion: 'Check your mcp_servers.json OAuth configuration', + }; +} + +export function oauthFlowError(serverName: string, cause: string): CliError { + let suggestion = 'Try again or check your OAuth server configuration'; + + if (cause.includes('timeout')) { + suggestion = + 'Authorization timed out. Try again and complete the browser authorization within the time limit'; + } else if (cause.includes('callback')) { + suggestion = + 'Failed to receive OAuth callback. Ensure your firewall allows connections to localhost'; + } else if (cause.includes('token')) { + suggestion = + 'Token exchange failed. Check your client credentials and OAuth server configuration'; + } else if (cause.includes('refresh')) { + suggestion = + 'Token refresh failed. Your session may have expired - try re-authenticating'; + } + + return { + code: ErrorCode.AUTH_ERROR, + type: 'OAUTH_FLOW_ERROR', + message: `OAuth authentication failed for server "${serverName}"`, + details: cause, + suggestion, + }; +} diff --git a/src/oauth.ts b/src/oauth.ts new file mode 100644 index 0000000..8a2c24b --- /dev/null +++ b/src/oauth.ts @@ -0,0 +1,487 @@ +/** + * OAuth Provider Implementation for mcp-cli + * + * Implements OAuthClientProvider interface with file-based token storage + * for persistent authentication across CLI invocations. + */ + +import { exec } from 'node:child_process'; +import { + existsSync, + mkdirSync, + readFileSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { type Server, createServer } from 'node:http'; +import { homedir, platform } from 'node:os'; +import { join } from 'node:path'; +import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'; +import type { + OAuthClientInformationMixed, + OAuthClientMetadata, + OAuthTokens, +} from '@modelcontextprotocol/sdk/shared/auth.js'; +import { debug } from './config.js'; + +/** + * Get the MCP CLI storage directory + * Can be overridden via MCP_CLI_HOME env var for testing + */ +function getMcpCliDir(): string { + const baseDir = process.env.MCP_CLI_HOME || homedir(); + return join(baseDir, '.mcp-cli'); +} + +// Storage directories (lazily resolved) +function getStoragePaths() { + const mcpCliDir = getMcpCliDir(); + return { + tokens: join(mcpCliDir, 'tokens'), + clients: join(mcpCliDir, 'clients'), + verifiers: join(mcpCliDir, 'verifiers'), + }; +} + +// Default callback port for OAuth redirect +const DEFAULT_CALLBACK_PORT = 8095; + +/** + * OAuth configuration from server config + */ +export interface OAuthConfig { + grantType?: 'authorization_code' | 'client_credentials'; + clientId?: string; + clientSecret?: string; + scope?: string; + callbackPort?: number; +} + +/** + * Ensure a directory exists + */ +function ensureDir(dir: string): void { + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true, mode: 0o700 }); + } +} + +/** + * Read JSON file safely + */ +function readJsonFile(path: string): T | undefined { + try { + if (!existsSync(path)) { + return undefined; + } + const content = readFileSync(path, 'utf-8'); + return JSON.parse(content) as T; + } catch (error) { + debug(`Failed to read ${path}: ${(error as Error).message}`); + return undefined; + } +} + +/** + * Write JSON file safely + */ +function writeJsonFile(path: string, data: unknown): void { + ensureDir(join(path, '..')); + writeFileSync(path, JSON.stringify(data, null, 2), { mode: 0o600 }); +} + +/** + * Read text file safely + */ +function readTextFile(path: string): string | undefined { + try { + if (!existsSync(path)) { + return undefined; + } + return readFileSync(path, 'utf-8').trim(); + } catch (error) { + debug(`Failed to read ${path}: ${(error as Error).message}`); + return undefined; + } +} + +/** + * Write text file safely + */ +function writeTextFile(path: string, content: string): void { + ensureDir(join(path, '..')); + writeFileSync(path, content, { mode: 0o600 }); +} + +/** + * Delete file safely + */ +function deleteFile(path: string): void { + try { + if (existsSync(path)) { + unlinkSync(path); + } + } catch (error) { + debug(`Failed to delete ${path}: ${(error as Error).message}`); + } +} + +/** + * Sanitize server name for use as filename + */ +function sanitizeServerName(name: string): string { + return name.replace(/[^a-zA-Z0-9_-]/g, '_'); +} + +/** + * Get file paths for a server + */ +function getServerPaths(serverName: string): { + tokens: string; + client: string; + verifier: string; +} { + const safeName = sanitizeServerName(serverName); + const dirs = getStoragePaths(); + return { + tokens: join(dirs.tokens, `${safeName}.json`), + client: join(dirs.clients, `${safeName}.json`), + verifier: join(dirs.verifiers, `${safeName}.txt`), + }; +} + +/** + * Open URL in default browser (cross-platform) + */ +export function openBrowser(url: string): Promise { + return new Promise((resolve, reject) => { + const os = platform(); + let command: string; + + switch (os) { + case 'darwin': + command = `open "${url}"`; + break; + case 'win32': + command = `start "" "${url}"`; + break; + default: + // Linux and others + command = `xdg-open "${url}"`; + } + + debug(`Opening browser: ${command}`); + + exec(command, (error) => { + if (error) { + console.error(`Failed to open browser: ${error.message}`); + console.error(`Please manually open: ${url}`); + reject(error); + } else { + resolve(); + } + }); + }); +} + +/** + * Result from OAuth callback + */ +export interface OAuthCallbackResult { + code: string; +} + +/** + * Wait for OAuth callback on local server + * Returns the authorization code from the callback + */ +export function waitForOAuthCallback( + port: number = DEFAULT_CALLBACK_PORT, + timeoutMs = 300000, // 5 minutes default +): Promise { + return new Promise((resolve, reject) => { + let server: Server | undefined; + let timeoutId: ReturnType | undefined; + + const cleanup = () => { + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + if (server) { + server.close(); + server = undefined; + } + }; + + timeoutId = setTimeout(() => { + cleanup(); + reject( + new Error('OAuth callback timeout - no authorization code received'), + ); + }, timeoutMs); + + server = createServer((req, res) => { + // Ignore favicon requests + if (req.url === '/favicon.ico') { + res.writeHead(404); + res.end(); + return; + } + + debug(`Received OAuth callback: ${req.url}`); + + const parsedUrl = new URL(req.url || '', `http://localhost:${port}`); + const code = parsedUrl.searchParams.get('code'); + const error = parsedUrl.searchParams.get('error'); + const errorDescription = parsedUrl.searchParams.get('error_description'); + + if (code) { + debug(`Authorization code received: ${code.substring(0, 10)}...`); + + res.writeHead(200, { 'Content-Type': 'text/html' }); + res.end(` + + + Authorization Successful + +

Authorization Successful

+

You can close this window and return to the terminal.

+ + + + `); + + cleanup(); + resolve({ code }); + } else if (error) { + const message = errorDescription || error; + debug(`OAuth error: ${message}`); + + res.writeHead(400, { 'Content-Type': 'text/html' }); + res.end(` + + + Authorization Failed + +

Authorization Failed

+

Error: ${message}

+ + + `); + + cleanup(); + reject(new Error(`OAuth authorization failed: ${message}`)); + } else { + res.writeHead(400, { 'Content-Type': 'text/plain' }); + res.end('Bad request: missing authorization code'); + } + }); + + server.on('error', (error) => { + cleanup(); + reject( + new Error(`Failed to start OAuth callback server: ${error.message}`), + ); + }); + + server.listen(port, () => { + debug( + `OAuth callback server listening on http://localhost:${port}/callback`, + ); + }); + }); +} + +/** + * MCP CLI OAuth Provider + * + * Implements the OAuthClientProvider interface with file-based persistence + * for tokens, client information, and PKCE verifiers. + */ +export class McpCliOAuthProvider implements OAuthClientProvider { + private readonly serverName: string; + private readonly oauthConfig: OAuthConfig; + private readonly serverUrl: string; + private readonly paths: { tokens: string; client: string; verifier: string }; + + constructor(serverName: string, serverUrl: string, oauthConfig: OAuthConfig) { + this.serverName = serverName; + this.serverUrl = serverUrl; + this.oauthConfig = oauthConfig; + this.paths = getServerPaths(serverName); + + // Ensure storage directories exist + const dirs = getStoragePaths(); + ensureDir(dirs.tokens); + ensureDir(dirs.clients); + ensureDir(dirs.verifiers); + } + + /** + * Redirect URL for OAuth callback + * Returns undefined for client_credentials flow (non-interactive) + */ + get redirectUrl(): string | URL | undefined { + if (this.oauthConfig.grantType === 'client_credentials') { + return undefined; + } + const port = this.oauthConfig.callbackPort || DEFAULT_CALLBACK_PORT; + return `http://localhost:${port}/callback`; + } + + /** + * OAuth client metadata + */ + get clientMetadata(): OAuthClientMetadata { + const redirectUrl = this.redirectUrl; + const grantTypes = + this.oauthConfig.grantType === 'client_credentials' + ? ['client_credentials'] + : ['authorization_code', 'refresh_token']; + + // Convert URL to string if needed + const redirectUriStr = redirectUrl ? String(redirectUrl) : undefined; + + return { + client_name: `mcp-cli (${this.serverName})`, + // Zod validates these as URLs at runtime; TypeScript type expects string[] + redirect_uris: redirectUriStr ? [redirectUriStr] : [], + grant_types: grantTypes, + response_types: + this.oauthConfig.grantType === 'client_credentials' ? [] : ['code'], + token_endpoint_auth_method: this.oauthConfig.clientSecret + ? 'client_secret_post' + : 'none', + scope: this.oauthConfig.scope, + }; + } + + /** + * Load client information from config or file + */ + clientInformation(): OAuthClientInformationMixed | undefined { + // First check if client ID is configured statically + if (this.oauthConfig.clientId) { + const info: OAuthClientInformationMixed = { + client_id: this.oauthConfig.clientId, + }; + if (this.oauthConfig.clientSecret) { + info.client_secret = this.oauthConfig.clientSecret; + } + return info; + } + + // Otherwise, try to load from dynamic registration + return readJsonFile(this.paths.client); + } + + /** + * Save dynamically registered client information + */ + saveClientInformation(clientInformation: OAuthClientInformationMixed): void { + debug(`Saving client information for ${this.serverName}`); + writeJsonFile(this.paths.client, clientInformation); + } + + /** + * Load OAuth tokens + */ + tokens(): OAuthTokens | undefined { + return readJsonFile(this.paths.tokens); + } + + /** + * Save OAuth tokens + */ + saveTokens(tokens: OAuthTokens): void { + debug(`Saving tokens for ${this.serverName}`); + writeJsonFile(this.paths.tokens, tokens); + } + + /** + * Redirect to authorization URL + * Opens the URL in the default browser + */ + redirectToAuthorization(authorizationUrl: URL): void { + console.error('Opening browser for authorization...'); + console.error( + `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, + ); + openBrowser(authorizationUrl.toString()).catch(() => { + // Error already logged in openBrowser + }); + } + + /** + * Save PKCE code verifier + */ + saveCodeVerifier(codeVerifier: string): void { + debug(`Saving code verifier for ${this.serverName}`); + writeTextFile(this.paths.verifier, codeVerifier); + } + + /** + * Load PKCE code verifier + */ + codeVerifier(): string { + const verifier = readTextFile(this.paths.verifier); + if (!verifier) { + throw new Error( + 'No code verifier found - OAuth flow may have been interrupted', + ); + } + return verifier; + } + + /** + * Invalidate stored credentials + */ + invalidateCredentials(scope: 'all' | 'client' | 'tokens' | 'verifier'): void { + debug(`Invalidating credentials for ${this.serverName}: ${scope}`); + + switch (scope) { + case 'all': + deleteFile(this.paths.tokens); + deleteFile(this.paths.client); + deleteFile(this.paths.verifier); + break; + case 'client': + deleteFile(this.paths.client); + break; + case 'tokens': + deleteFile(this.paths.tokens); + break; + case 'verifier': + deleteFile(this.paths.verifier); + break; + } + } + + /** + * Prepare token request for client_credentials flow + */ + prepareTokenRequest(scope?: string): URLSearchParams | undefined { + if (this.oauthConfig.grantType !== 'client_credentials') { + return undefined; + } + + const params = new URLSearchParams({ + grant_type: 'client_credentials', + }); + + const requestScope = scope || this.oauthConfig.scope; + if (requestScope) { + params.set('scope', requestScope); + } + + return params; + } + + /** + * Get the callback port for this provider + */ + getCallbackPort(): number { + return this.oauthConfig.callbackPort || DEFAULT_CALLBACK_PORT; + } +} diff --git a/tests/config.test.ts b/tests/config.test.ts index a48602c..7931791 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -240,4 +240,159 @@ describe('config', () => { expect(isStdioServer({ url: 'https://example.com' })).toBe(false); }); }); + + describe('OAuth config validation', () => { + test('accepts valid OAuth config with authorization_code', async () => { + const configPath = join(tempDir, 'oauth_auth_code.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + grantType: 'authorization_code', + scope: 'read write', + }, + }, + }, + }) + ); + + const config = await loadConfig(configPath); + const server = config.mcpServers.test as any; + expect(server.oauth.grantType).toBe('authorization_code'); + expect(server.oauth.scope).toBe('read write'); + }); + + test('accepts valid OAuth config with client_credentials', async () => { + process.env.TEST_CLIENT_ID = 'test-id'; + process.env.TEST_CLIENT_SECRET = 'test-secret'; + + const configPath = join(tempDir, 'oauth_client_creds.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + grantType: 'client_credentials', + clientId: '${TEST_CLIENT_ID}', + clientSecret: '${TEST_CLIENT_SECRET}', + }, + }, + }, + }) + ); + + const config = await loadConfig(configPath); + const server = config.mcpServers.test as any; + expect(server.oauth.grantType).toBe('client_credentials'); + expect(server.oauth.clientId).toBe('test-id'); + expect(server.oauth.clientSecret).toBe('test-secret'); + + delete process.env.TEST_CLIENT_ID; + delete process.env.TEST_CLIENT_SECRET; + }); + + test('throws on invalid grantType', async () => { + const configPath = join(tempDir, 'oauth_bad_grant.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + grantType: 'invalid_grant', + }, + }, + }, + }) + ); + + await expect(loadConfig(configPath)).rejects.toThrow('Invalid OAuth grantType'); + }); + + test('throws on client_credentials without clientId', async () => { + const configPath = join(tempDir, 'oauth_no_client_id.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + grantType: 'client_credentials', + clientSecret: 'secret', + }, + }, + }, + }) + ); + + await expect(loadConfig(configPath)).rejects.toThrow('requires clientId'); + }); + + test('throws on client_credentials without clientSecret', async () => { + const configPath = join(tempDir, 'oauth_no_client_secret.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + grantType: 'client_credentials', + clientId: 'client-id', + }, + }, + }, + }) + ); + + await expect(loadConfig(configPath)).rejects.toThrow('requires clientSecret'); + }); + + test('throws on invalid callbackPort', async () => { + const configPath = join(tempDir, 'oauth_bad_port.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + callbackPort: 99999, + }, + }, + }, + }) + ); + + await expect(loadConfig(configPath)).rejects.toThrow('Invalid callbackPort'); + }); + + test('accepts valid callbackPort', async () => { + const configPath = join(tempDir, 'oauth_valid_port.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + url: 'https://example.com', + oauth: { + callbackPort: 9000, + }, + }, + }, + }) + ); + + const config = await loadConfig(configPath); + const server = config.mcpServers.test as any; + expect(server.oauth.callbackPort).toBe(9000); + }); + }); }); diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts new file mode 100644 index 0000000..a02b21b --- /dev/null +++ b/tests/oauth.test.ts @@ -0,0 +1,350 @@ +/** + * Unit tests for OAuth module + */ + +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { McpCliOAuthProvider, type OAuthConfig } from '../src/oauth'; + +describe('oauth', () => { + // Use a unique temp directory for each test run to avoid conflicts + let testDir: string; + const originalMcpCliHome = process.env.MCP_CLI_HOME; + + beforeEach(() => { + // Create unique test directory for each test + testDir = join(tmpdir(), `mcp-cli-oauth-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(testDir, { recursive: true }); + // Use MCP_CLI_HOME env var to override storage location + process.env.MCP_CLI_HOME = testDir; + }); + + afterEach(() => { + // Restore original MCP_CLI_HOME + if (originalMcpCliHome === undefined) { + delete process.env.MCP_CLI_HOME; + } else { + process.env.MCP_CLI_HOME = originalMcpCliHome; + } + + // Clean up test directory + try { + rmSync(testDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + }); + + describe('McpCliOAuthProvider', () => { + describe('constructor and paths', () => { + test('creates storage directories', () => { + const config: OAuthConfig = { scope: 'read' }; + new McpCliOAuthProvider('test-server', 'https://example.com', config); + + expect(existsSync(join(testDir, '.mcp-cli', 'tokens'))).toBe(true); + expect(existsSync(join(testDir, '.mcp-cli', 'clients'))).toBe(true); + expect(existsSync(join(testDir, '.mcp-cli', 'verifiers'))).toBe(true); + }); + + test('sanitizes server names for file paths', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider( + 'my/weird:server.name', + 'https://example.com', + config + ); + + // Save some data to verify path sanitization + provider.saveTokens({ + access_token: 'test', + token_type: 'Bearer', + }); + + // Check that a sanitized filename was used + const tokensDir = join(testDir, '.mcp-cli', 'tokens'); + const files = require('fs').readdirSync(tokensDir); + expect(files.length).toBe(1); + expect(files[0]).toBe('my_weird_server_name.json'); + }); + }); + + describe('redirectUrl', () => { + test('returns callback URL for authorization_code flow', () => { + const config: OAuthConfig = { grantType: 'authorization_code' }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBe('http://localhost:8095/callback'); + }); + + test('returns callback URL when grantType is not specified (default)', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBe('http://localhost:8095/callback'); + }); + + test('returns undefined for client_credentials flow', () => { + const config: OAuthConfig = { + grantType: 'client_credentials', + clientId: 'id', + clientSecret: 'secret', + }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBeUndefined(); + }); + + test('uses custom callback port', () => { + const config: OAuthConfig = { callbackPort: 9999 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBe('http://localhost:9999/callback'); + }); + }); + + describe('clientMetadata', () => { + test('returns correct metadata for authorization_code flow', () => { + const config: OAuthConfig = { scope: 'tools:read' }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const metadata = provider.clientMetadata; + expect(metadata.client_name).toBe('mcp-cli (test)'); + expect(metadata.grant_types).toContain('authorization_code'); + expect(metadata.grant_types).toContain('refresh_token'); + expect(metadata.response_types).toContain('code'); + expect(metadata.scope).toBe('tools:read'); + }); + + test('returns correct metadata for client_credentials flow', () => { + const config: OAuthConfig = { + grantType: 'client_credentials', + clientId: 'id', + clientSecret: 'secret', + }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const metadata = provider.clientMetadata; + expect(metadata.grant_types).toEqual(['client_credentials']); + expect(metadata.response_types).toEqual([]); + expect(metadata.redirect_uris).toEqual([]); + }); + + test('sets token_endpoint_auth_method based on clientSecret', () => { + const withSecret: OAuthConfig = { clientId: 'id', clientSecret: 'secret' }; + const providerWithSecret = new McpCliOAuthProvider('test', 'https://example.com', withSecret); + expect(providerWithSecret.clientMetadata.token_endpoint_auth_method).toBe('client_secret_post'); + + const withoutSecret: OAuthConfig = { clientId: 'id' }; + const providerWithoutSecret = new McpCliOAuthProvider('test', 'https://example.com', withoutSecret); + expect(providerWithoutSecret.clientMetadata.token_endpoint_auth_method).toBe('none'); + }); + }); + + describe('clientInformation', () => { + test('returns static client info from config', () => { + const config: OAuthConfig = { clientId: 'my-client', clientSecret: 'my-secret' }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const info = provider.clientInformation(); + expect(info?.client_id).toBe('my-client'); + expect(info?.client_secret).toBe('my-secret'); + }); + + test('returns undefined when no client info configured or saved', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.clientInformation()).toBeUndefined(); + }); + + test('loads saved client info from file', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + // Save client info + provider.saveClientInformation({ + client_id: 'dynamic-client', + client_secret: 'dynamic-secret', + }); + + // Should load from file + const info = provider.clientInformation(); + expect(info?.client_id).toBe('dynamic-client'); + expect(info?.client_secret).toBe('dynamic-secret'); + }); + + test('prefers static config over saved file', () => { + const config: OAuthConfig = { clientId: 'static-id' }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + // Save different client info + provider.saveClientInformation({ client_id: 'saved-id' }); + + // Should return static config + const info = provider.clientInformation(); + expect(info?.client_id).toBe('static-id'); + }); + }); + + describe('tokens', () => { + test('returns undefined when no tokens saved', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.tokens()).toBeUndefined(); + }); + + test('saves and loads tokens', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const tokens = { + access_token: 'test-access-token', + token_type: 'Bearer', + refresh_token: 'test-refresh-token', + expires_in: 3600, + }; + + provider.saveTokens(tokens); + + const loaded = provider.tokens(); + expect(loaded?.access_token).toBe('test-access-token'); + expect(loaded?.refresh_token).toBe('test-refresh-token'); + }); + + test('tokens are persisted to file', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.saveTokens({ + access_token: 'persisted-token', + token_type: 'Bearer', + }); + + // Create new provider instance + const provider2 = new McpCliOAuthProvider('test', 'https://example.com', config); + const loaded = provider2.tokens(); + expect(loaded?.access_token).toBe('persisted-token'); + }); + }); + + describe('codeVerifier', () => { + test('throws when no verifier saved', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(() => provider.codeVerifier()).toThrow('No code verifier found'); + }); + + test('saves and loads code verifier', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.saveCodeVerifier('test-verifier-123'); + expect(provider.codeVerifier()).toBe('test-verifier-123'); + }); + }); + + describe('invalidateCredentials', () => { + test('invalidates all credentials', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + // Save various credentials + provider.saveTokens({ access_token: 'token', token_type: 'Bearer' }); + provider.saveClientInformation({ client_id: 'client' }); + provider.saveCodeVerifier('verifier'); + + // Verify they exist + expect(provider.tokens()).toBeDefined(); + + // Invalidate all + provider.invalidateCredentials('all'); + + // Verify they're gone + expect(provider.tokens()).toBeUndefined(); + expect(() => provider.codeVerifier()).toThrow(); + }); + + test('invalidates only tokens', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.saveTokens({ access_token: 'token', token_type: 'Bearer' }); + provider.saveCodeVerifier('verifier'); + + provider.invalidateCredentials('tokens'); + + expect(provider.tokens()).toBeUndefined(); + expect(provider.codeVerifier()).toBe('verifier'); // Still exists + }); + + test('invalidates only verifier', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.saveTokens({ access_token: 'token', token_type: 'Bearer' }); + provider.saveCodeVerifier('verifier'); + + provider.invalidateCredentials('verifier'); + + expect(provider.tokens()).toBeDefined(); // Still exists + expect(() => provider.codeVerifier()).toThrow(); + }); + }); + + describe('prepareTokenRequest', () => { + test('returns undefined for authorization_code flow', () => { + const config: OAuthConfig = { grantType: 'authorization_code' }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.prepareTokenRequest()).toBeUndefined(); + }); + + test('returns params for client_credentials flow', () => { + const config: OAuthConfig = { + grantType: 'client_credentials', + clientId: 'id', + clientSecret: 'secret', + scope: 'read write', + }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const params = provider.prepareTokenRequest(); + expect(params?.get('grant_type')).toBe('client_credentials'); + expect(params?.get('scope')).toBe('read write'); + }); + + test('allows scope override', () => { + const config: OAuthConfig = { + grantType: 'client_credentials', + clientId: 'id', + clientSecret: 'secret', + scope: 'default-scope', + }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const params = provider.prepareTokenRequest('override-scope'); + expect(params?.get('scope')).toBe('override-scope'); + }); + }); + + describe('getCallbackPort', () => { + test('returns default port', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.getCallbackPort()).toBe(8095); + }); + + test('returns custom port', () => { + const config: OAuthConfig = { callbackPort: 12345 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.getCallbackPort()).toBe(12345); + }); + }); + }); +}); From f7cca149560549d059c889159a45fd93e00e34d8 Mon Sep 17 00:00:00 2001 From: Atte Huhtakangas Date: Tue, 20 Jan 2026 22:15:23 +0000 Subject: [PATCH 2/9] fix: OAuth race condition and concurrent auth handling - Start callback server BEFORE opening browser to fix race condition where browser redirects before server is ready - Add allowInteractiveAuth option to disable OAuth prompts when listing multiple servers (prevents multiple browsers opening) - Show helpful "requires authentication" message for unauthenticated servers when listing, with command to authenticate individually - Export AuthRequiredError and ConnectOptions from client module Co-Authored-By: Claude Opus 4.5 --- src/client.ts | 41 +++++++++-- src/commands/list.ts | 10 ++- src/oauth.ts | 163 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 188 insertions(+), 26 deletions(-) diff --git a/src/client.ts b/src/client.ts index 4a02840..bfc2cbd 100644 --- a/src/client.ts +++ b/src/client.ts @@ -27,7 +27,7 @@ import { getDaemonConnection, } from './daemon-client.js'; import { formatCliError, oauthFlowError } from './errors.js'; -import { McpCliOAuthProvider, waitForOAuthCallback } from './oauth.js'; +import { McpCliOAuthProvider } from './oauth.js'; import { VERSION } from './version.js'; // Re-export config utilities for convenience @@ -52,6 +52,27 @@ export interface McpConnection { isDaemon: boolean; } +export interface ConnectOptions { + /** + * Allow interactive OAuth flow (browser opens, user authorizes) + * When false, throws an error if authentication is required + * Default: true + */ + allowInteractiveAuth?: boolean; +} + +/** + * Error thrown when authentication is required but interactive auth is disabled + */ +export class AuthRequiredError extends Error { + constructor(serverName: string) { + super( + `Server "${serverName}" requires authentication. Run 'mcp-cli ${serverName}' to authenticate.`, + ); + this.name = 'AuthRequiredError'; + } +} + export interface ServerInfo { name: string; version?: string; @@ -225,7 +246,9 @@ export async function safeClose(close: () => Promise): Promise { export async function connectToServer( serverName: string, config: ServerConfig, + options: ConnectOptions = {}, ): Promise { + const { allowInteractiveAuth = true } = options; // Collect stderr for better error messages const stderrChunks: string[] = []; @@ -247,6 +270,9 @@ export async function connectToServer( const result = createHttpTransport(serverName, config); transport = result.transport; authProvider = result.authProvider; + + // Configure whether interactive auth is allowed + authProvider.setAllowInteractiveAuth(allowInteractiveAuth); } else { transport = createStdioTransport(config); @@ -270,6 +296,12 @@ export async function connectToServer( if (isOAuthNeeded(error as Error) && authProvider) { debug(`OAuth authorization required for ${serverName}`); + // If interactive auth was blocked by the provider, throw a clear error + if (authProvider.interactiveAuthBlocked) { + authProvider.cleanupCallbackServer(); + throw new AuthRequiredError(serverName); + } + // For authorization_code flow, wait for callback const oauthConfig = (config as HttpServerConfig).oauth; if ( @@ -277,10 +309,10 @@ export async function connectToServer( oauthConfig.grantType === 'authorization_code' ) { try { - const port = authProvider.getCallbackPort(); console.error('Waiting for OAuth authorization...'); - const { code } = await waitForOAuthCallback(port); + // Wait for the callback server that was started by redirectToAuthorization + const { code } = await authProvider.waitForCallback(); debug('Authorization code received, completing OAuth flow'); // Complete the OAuth flow with the authorization code @@ -488,6 +520,7 @@ export async function callTool( export async function getConnection( serverName: string, config: ServerConfig, + options: ConnectOptions = {}, ): Promise { // Clean up any orphaned daemons on first call await cleanupOrphanedDaemons(); @@ -535,7 +568,7 @@ export async function getConnection( // Fall back to direct connection debug(`Using direct connection for ${serverName}`); - const { client, close } = await connectToServer(serverName, config); + const { client, close } = await connectToServer(serverName, config, options); return { async listTools(): Promise { diff --git a/src/commands/list.ts b/src/commands/list.ts index 82d1014..71176aa 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -62,15 +62,19 @@ async function processWithConcurrency( /** * Fetch tools from a single server (uses daemon if enabled) + * When listing multiple servers, interactive OAuth is disabled to prevent chaos */ async function fetchServerTools( serverName: string, config: McpServersConfig, + allowInteractiveAuth = true, ): Promise { let connection: McpConnection | null = null; try { const serverConfig = getServerConfig(config, serverName); - connection = await getConnection(serverName, serverConfig); + connection = await getConnection(serverName, serverConfig, { + allowInteractiveAuth, + }); const tools = await connection.listTools(); const instructions = await connection.getInstructions(); @@ -119,9 +123,11 @@ export async function listCommand(options: ListOptions): Promise { ); // Process servers in parallel with concurrency limit + // Disable interactive OAuth when listing multiple servers to prevent chaos + const allowInteractiveAuth = serverNames.length === 1; const servers = await processWithConcurrency( serverNames, - (name) => fetchServerTools(name, config), + (name) => fetchServerTools(name, config, allowInteractiveAuth), concurrencyLimit, ); diff --git a/src/oauth.ts b/src/oauth.ts index 8a2c24b..cce1e5f 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -192,16 +192,27 @@ export interface OAuthCallbackResult { } /** - * Wait for OAuth callback on local server - * Returns the authorization code from the callback + * Callback server state for managing the OAuth callback listener */ -export function waitForOAuthCallback( - port: number = DEFAULT_CALLBACK_PORT, - timeoutMs = 300000, // 5 minutes default -): Promise { - return new Promise((resolve, reject) => { - let server: Server | undefined; +interface CallbackServerState { + server: Server; + promise: Promise; + cleanup: () => void; +} + +/** + * Start an OAuth callback server that listens for the authorization code + * Returns immediately with a promise that resolves when the callback is received + */ +function startCallbackServer( + port: number, + timeoutMs = 300000, +): Promise { + return new Promise((resolveStart, rejectStart) => { + let callbackResolve: (result: OAuthCallbackResult) => void; + let callbackReject: (error: Error) => void; let timeoutId: ReturnType | undefined; + let server: Server | undefined; const cleanup = () => { if (timeoutId) { @@ -214,9 +225,16 @@ export function waitForOAuthCallback( } }; + const callbackPromise = new Promise( + (resolve, reject) => { + callbackResolve = resolve; + callbackReject = reject; + }, + ); + timeoutId = setTimeout(() => { cleanup(); - reject( + callbackReject( new Error('OAuth callback timeout - no authorization code received'), ); }, timeoutMs); @@ -253,7 +271,7 @@ export function waitForOAuthCallback( `); cleanup(); - resolve({ code }); + callbackResolve({ code }); } else if (error) { const message = errorDescription || error; debug(`OAuth error: ${message}`); @@ -271,7 +289,7 @@ export function waitForOAuthCallback( `); cleanup(); - reject(new Error(`OAuth authorization failed: ${message}`)); + callbackReject(new Error(`OAuth authorization failed: ${message}`)); } else { res.writeHead(400, { 'Content-Type': 'text/plain' }); res.end('Bad request: missing authorization code'); @@ -280,7 +298,7 @@ export function waitForOAuthCallback( server.on('error', (error) => { cleanup(); - reject( + rejectStart( new Error(`Failed to start OAuth callback server: ${error.message}`), ); }); @@ -289,10 +307,28 @@ export function waitForOAuthCallback( debug( `OAuth callback server listening on http://localhost:${port}/callback`, ); + resolveStart({ + server: server!, + promise: callbackPromise, + cleanup, + }); }); }); } +/** + * Wait for OAuth callback on local server + * Returns the authorization code from the callback + * @deprecated Use McpCliOAuthProvider.waitForCallback() instead + */ +export async function waitForOAuthCallback( + port: number = DEFAULT_CALLBACK_PORT, + timeoutMs = 300000, +): Promise { + const state = await startCallbackServer(port, timeoutMs); + return state.promise; +} + /** * MCP CLI OAuth Provider * @@ -305,6 +341,13 @@ export class McpCliOAuthProvider implements OAuthClientProvider { private readonly serverUrl: string; private readonly paths: { tokens: string; client: string; verifier: string }; + // Callback server state - started before browser opens + private callbackServerState: CallbackServerState | null = null; + private callbackServerStarting: Promise | null = null; + + // Whether interactive auth (browser + callback) is allowed + private _allowInteractiveAuth = true; + constructor(serverName: string, serverUrl: string, oauthConfig: OAuthConfig) { this.serverName = serverName; this.serverUrl = serverUrl; @@ -399,18 +442,98 @@ export class McpCliOAuthProvider implements OAuthClientProvider { writeJsonFile(this.paths.tokens, tokens); } + /** + * Set whether interactive auth is allowed + * When disabled, redirectToAuthorization will not open browser or start server + */ + setAllowInteractiveAuth(allow: boolean): void { + this._allowInteractiveAuth = allow; + } + + /** + * Check if interactive auth was blocked + */ + get interactiveAuthBlocked(): boolean { + return !this._allowInteractiveAuth; + } + /** * Redirect to authorization URL - * Opens the URL in the default browser + * Starts the callback server FIRST, then opens the browser + * This prevents the race condition where the browser redirects before the server is ready */ redirectToAuthorization(authorizationUrl: URL): void { - console.error('Opening browser for authorization...'); - console.error( - `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, - ); - openBrowser(authorizationUrl.toString()).catch(() => { - // Error already logged in openBrowser - }); + // If interactive auth is disabled, don't start server or open browser + if (!this._allowInteractiveAuth) { + debug( + `Interactive auth disabled for ${this.serverName}, skipping browser redirect`, + ); + return; + } + + const port = this.getCallbackPort(); + + // Start the callback server BEFORE opening the browser + // This is critical to avoid race conditions + // Store the promise so waitForCallback can properly await it + this.callbackServerStarting = startCallbackServer(port) + .then((state) => { + this.callbackServerState = state; + this.callbackServerStarting = null; + debug(`Callback server ready for ${this.serverName}`); + + // Now open the browser + console.error(`\nAuthorizing ${this.serverName}...`); + console.error( + `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, + ); + openBrowser(authorizationUrl.toString()).catch(() => { + // Error already logged in openBrowser + }); + }) + .catch((error) => { + this.callbackServerStarting = null; + console.error(`Failed to start callback server: ${error.message}`); + console.error( + `Please manually visit: ${authorizationUrl.toString()}`, + ); + throw error; + }); + } + + /** + * Wait for the OAuth callback to complete + * Returns the authorization code + */ + async waitForCallback(): Promise { + // Wait for server to finish starting if in progress + if (this.callbackServerStarting) { + await this.callbackServerStarting; + } + + if (!this.callbackServerState) { + // Fallback: start server now if not already started + const port = this.getCallbackPort(); + this.callbackServerState = await startCallbackServer(port); + } + return this.callbackServerState.promise; + } + + /** + * Check if this provider has a pending callback server + */ + hasPendingCallback(): boolean { + return this.callbackServerState !== null; + } + + /** + * Clean up the callback server if running + */ + cleanupCallbackServer(): void { + if (this.callbackServerState) { + this.callbackServerState.cleanup(); + this.callbackServerState = null; + } } /** From 48737cce89f14faef14968bb6c9d1f1961af6aa0 Mon Sep 17 00:00:00 2001 From: Atte Huhtakangas Date: Wed, 21 Jan 2026 11:04:12 +0000 Subject: [PATCH 3/9] feat: add OAuth callback port fallback and pretty HTML pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add port fallback mechanism: tries 80 → 8080 → 3000 → 8095 → random - Port 80 as default with standard URL format (http://localhost/callback) - Add pretty styled HTML pages for success/error callbacks - Add callbackPorts config option for custom port fallback list - Pre-start callback server to determine actual port before auth flow - Add comprehensive tests for new port fallback features Co-Authored-By: Claude Opus 4.5 --- src/client.ts | 12 ++ src/config.ts | 2 + src/oauth.ts | 277 +++++++++++++++++++++++++++++++++++++++----- tests/oauth.test.ts | 107 +++++++++++++++++ 4 files changed, 367 insertions(+), 31 deletions(-) diff --git a/src/client.ts b/src/client.ts index bfc2cbd..dcc385d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -273,6 +273,18 @@ export async function connectToServer( // Configure whether interactive auth is allowed authProvider.setAllowInteractiveAuth(allowInteractiveAuth); + + // Pre-start callback server for authorization_code flow to determine actual port + // This ensures the redirect_uri in the authorization request uses the correct port + const oauthConfig = (config as HttpServerConfig).oauth; + if (!oauthConfig?.grantType || oauthConfig.grantType === 'authorization_code') { + try { + await authProvider.preStartCallbackServer(); + } catch (error) { + debug(`Failed to pre-start callback server: ${(error as Error).message}`); + // Continue anyway - server will start during redirectToAuthorization + } + } } else { transport = createStdioTransport(config); diff --git a/src/config.ts b/src/config.ts index 577ef5a..4dc152d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -50,6 +50,8 @@ export interface OAuthConfig { clientSecret?: string; scope?: string; callbackPort?: number; + /** Optional: explicit list of ports to try in order (overrides default fallback) */ + callbackPorts?: number[]; } /** diff --git a/src/oauth.ts b/src/oauth.ts index cce1e5f..6bdfe81 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -46,6 +46,126 @@ function getStoragePaths() { // Default callback port for OAuth redirect const DEFAULT_CALLBACK_PORT = 8095; +// Default port fallback order: 80 (standard), common alternatives, then random +const DEFAULT_PORT_FALLBACK_ORDER = [80, 8080, 3000, 8095, 0]; // 0 = random port + +/** + * Pretty HTML template for successful OAuth callback + */ +const SUCCESS_HTML = ` + + + Authorization Successful + + + +
+
+ + + +
+

Authorization Successful

+

You can close this window and return to the terminal.

+
+ + +`; + +/** + * Pretty HTML template for OAuth error + */ +function errorHtml(message: string): string { + const escapedMessage = message + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); + + return ` + + + Authorization Failed + + + +
+
+ + + +
+

Authorization Failed

+

An error occurred during authorization.

+
${escapedMessage}
+
+ +`; +} + /** * OAuth configuration from server config */ @@ -55,6 +175,8 @@ export interface OAuthConfig { clientSecret?: string; scope?: string; callbackPort?: number; + /** Optional: explicit list of ports to try in order (overrides default fallback) */ + callbackPorts?: number[]; } /** @@ -258,17 +380,7 @@ function startCallbackServer( debug(`Authorization code received: ${code.substring(0, 10)}...`); res.writeHead(200, { 'Content-Type': 'text/html' }); - res.end(` - - - Authorization Successful - -

Authorization Successful

-

You can close this window and return to the terminal.

- - - - `); + res.end(SUCCESS_HTML); cleanup(); callbackResolve({ code }); @@ -277,16 +389,7 @@ function startCallbackServer( debug(`OAuth error: ${message}`); res.writeHead(400, { 'Content-Type': 'text/html' }); - res.end(` - - - Authorization Failed - -

Authorization Failed

-

Error: ${message}

- - - `); + res.end(errorHtml(message)); cleanup(); callbackReject(new Error(`OAuth authorization failed: ${message}`)); @@ -316,6 +419,40 @@ function startCallbackServer( }); } +/** + * Start an OAuth callback server with port fallback + * Tries ports in order until one succeeds + * @returns The server state and the actual port used + */ +async function startCallbackServerWithFallback( + portsToTry: number[], + timeoutMs = 300000, +): Promise<{ state: CallbackServerState; actualPort: number }> { + const errors: string[] = []; + + for (const port of portsToTry) { + try { + debug(`Trying to start callback server on port ${port === 0 ? 'random' : port}`); + const state = await startCallbackServer(port, timeoutMs); + // Get the actual port (important when port was 0 for random) + const address = state.server.address(); + const actualPort = typeof address === 'object' && address ? address.port : port; + debug(`Callback server started on port ${actualPort}`); + return { state, actualPort }; + } catch (error) { + const message = (error as Error).message; + debug(`Port ${port} failed: ${message}`); + errors.push(`Port ${port}: ${message}`); + // Continue to next port + } + } + + // All ports failed + throw new Error( + `Failed to start OAuth callback server. Tried ports: ${portsToTry.join(', ')}.\n${errors.join('\n')}`, + ); +} + /** * Wait for OAuth callback on local server * Returns the authorization code from the callback @@ -345,6 +482,9 @@ export class McpCliOAuthProvider implements OAuthClientProvider { private callbackServerState: CallbackServerState | null = null; private callbackServerStarting: Promise | null = null; + // Actual port used by callback server (may differ from configured port due to fallback) + private actualCallbackPort: number | null = null; + // Whether interactive auth (browser + callback) is allowed private _allowInteractiveAuth = true; @@ -364,12 +504,19 @@ export class McpCliOAuthProvider implements OAuthClientProvider { /** * Redirect URL for OAuth callback * Returns undefined for client_credentials flow (non-interactive) + * Uses actualCallbackPort if pre-started, otherwise configured/default port + * Omits port from URL when using port 80 (standard HTTP port) */ get redirectUrl(): string | URL | undefined { if (this.oauthConfig.grantType === 'client_credentials') { return undefined; } - const port = this.oauthConfig.callbackPort || DEFAULT_CALLBACK_PORT; + // Use actual port if server was pre-started, otherwise fall back to configured/default + const port = this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + // Omit port from URL when using standard HTTP port 80 + if (port === 80) { + return 'http://localhost/callback'; + } return `http://localhost:${port}/callback`; } @@ -459,7 +606,7 @@ export class McpCliOAuthProvider implements OAuthClientProvider { /** * Redirect to authorization URL - * Starts the callback server FIRST, then opens the browser + * Starts the callback server FIRST (with port fallback), then opens the browser * This prevents the race condition where the browser redirects before the server is ready */ redirectToAuthorization(authorizationUrl: URL): void { @@ -471,16 +618,23 @@ export class McpCliOAuthProvider implements OAuthClientProvider { return; } - const port = this.getCallbackPort(); + const portsToTry = this.getPortsToTry(); // Start the callback server BEFORE opening the browser // This is critical to avoid race conditions // Store the promise so waitForCallback can properly await it - this.callbackServerStarting = startCallbackServer(port) - .then((state) => { + this.callbackServerStarting = startCallbackServerWithFallback(portsToTry) + .then(({ state, actualPort }) => { this.callbackServerState = state; + this.actualCallbackPort = actualPort; this.callbackServerStarting = null; - debug(`Callback server ready for ${this.serverName}`); + debug(`Callback server ready for ${this.serverName} on port ${actualPort}`); + + // Update authorization URL with actual redirect_uri + const actualRedirectUri = this.redirectUrl; + if (actualRedirectUri) { + authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); + } // Now open the browser console.error(`\nAuthorizing ${this.serverName}...`); @@ -512,9 +666,11 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } if (!this.callbackServerState) { - // Fallback: start server now if not already started - const port = this.getCallbackPort(); - this.callbackServerState = await startCallbackServer(port); + // Fallback: start server now if not already started (with port fallback) + const portsToTry = this.getPortsToTry(); + const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + this.callbackServerState = state; + this.actualCallbackPort = actualPort; } return this.callbackServerState.promise; } @@ -605,6 +761,65 @@ export class McpCliOAuthProvider implements OAuthClientProvider { * Get the callback port for this provider */ getCallbackPort(): number { - return this.oauthConfig.callbackPort || DEFAULT_CALLBACK_PORT; + return this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + } + + /** + * Get the list of ports to try for the callback server + * If a specific port is configured, it comes first + * Then falls back to the default port order + */ + getPortsToTry(): number[] { + const ports: number[] = []; + + // If explicit callbackPorts array is configured, use it + if (this.oauthConfig.callbackPorts && this.oauthConfig.callbackPorts.length > 0) { + return [...this.oauthConfig.callbackPorts]; + } + + // If a single callbackPort is configured, try it first + if (this.oauthConfig.callbackPort) { + ports.push(this.oauthConfig.callbackPort); + } + + // Add default fallback ports (excluding any already added) + for (const port of DEFAULT_PORT_FALLBACK_ORDER) { + if (!ports.includes(port)) { + ports.push(port); + } + } + + return ports; + } + + /** + * Pre-start the callback server to determine the actual port + * Call this before accessing redirectUrl to ensure correct port in authorization URL + * Returns the actual port used + */ + async preStartCallbackServer(): Promise { + // Skip for client_credentials flow + if (this.oauthConfig.grantType === 'client_credentials') { + return 0; + } + + // Already started or starting + if (this.actualCallbackPort !== null) { + return this.actualCallbackPort; + } + if (this.callbackServerStarting) { + await this.callbackServerStarting; + return this.actualCallbackPort!; + } + + const portsToTry = this.getPortsToTry(); + debug(`Pre-starting callback server for ${this.serverName}, trying ports: ${portsToTry.join(', ')}`); + + const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + this.callbackServerState = state; + this.actualCallbackPort = actualPort; + + debug(`Callback server pre-started on port ${actualPort} for ${this.serverName}`); + return actualPort; } } diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index a02b21b..a7da145 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -346,5 +346,112 @@ describe('oauth', () => { expect(provider.getCallbackPort()).toBe(12345); }); }); + + describe('getPortsToTry', () => { + test('returns default port fallback order when no config', () => { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const ports = provider.getPortsToTry(); + // Default order: 80, 8080, 3000, 8095, 0 (random) + expect(ports).toEqual([80, 8080, 3000, 8095, 0]); + }); + + test('puts configured callbackPort first in fallback order', () => { + const config: OAuthConfig = { callbackPort: 9000 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const ports = provider.getPortsToTry(); + expect(ports[0]).toBe(9000); + // Rest of default order follows (excluding duplicates) + expect(ports).toContain(80); + expect(ports).toContain(8080); + }); + + test('uses callbackPorts array when configured', () => { + const config: OAuthConfig = { callbackPorts: [3000, 3001, 3002] }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const ports = provider.getPortsToTry(); + expect(ports).toEqual([3000, 3001, 3002]); + }); + + test('callbackPorts overrides callbackPort and defaults', () => { + const config: OAuthConfig = { callbackPort: 9000, callbackPorts: [4000, 4001] }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const ports = provider.getPortsToTry(); + // callbackPorts takes precedence + expect(ports).toEqual([4000, 4001]); + }); + }); + + describe('redirectUrl with port 80', () => { + test('omits port from URL when port is 80', () => { + const config: OAuthConfig = { callbackPort: 80 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBe('http://localhost/callback'); + }); + + test('includes port in URL for non-80 ports', () => { + const config: OAuthConfig = { callbackPort: 8080 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + expect(provider.redirectUrl).toBe('http://localhost:8080/callback'); + }); + }); + + describe('preStartCallbackServer', () => { + test('returns 0 for client_credentials flow', async () => { + const config: OAuthConfig = { + grantType: 'client_credentials', + clientId: 'id', + clientSecret: 'secret', + }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + const port = await provider.preStartCallbackServer(); + expect(port).toBe(0); + }); + + test('starts server and returns actual port', async () => { + const config: OAuthConfig = { callbackPorts: [0] }; // Use random port for test + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + try { + const port = await provider.preStartCallbackServer(); + expect(port).toBeGreaterThan(0); + expect(provider.getCallbackPort()).toBe(port); + } finally { + provider.cleanupCallbackServer(); + } + }); + + test('returns same port on subsequent calls', async () => { + const config: OAuthConfig = { callbackPorts: [0] }; // Use random port for test + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + try { + const port1 = await provider.preStartCallbackServer(); + const port2 = await provider.preStartCallbackServer(); + expect(port1).toBe(port2); + } finally { + provider.cleanupCallbackServer(); + } + }); + + test('redirectUrl uses actual port after pre-start', async () => { + const config: OAuthConfig = { callbackPorts: [0] }; // Use random port + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + try { + const port = await provider.preStartCallbackServer(); + expect(provider.redirectUrl).toBe(`http://localhost:${port}/callback`); + } finally { + provider.cleanupCallbackServer(); + } + }); + }); }); }); From b5dc06be2349f17328dded09467b425be534a5fe Mon Sep 17 00:00:00 2001 From: Atte Huhtakangas Date: Tue, 27 Jan 2026 22:52:16 +0000 Subject: [PATCH 4/9] fix: detect and invalidate stale OAuth client registrations When the callback server port changes between sessions (e.g., port 3000 was used during registration but port 8080 is available now), the OAuth authorization would fail with "Invalid redirect_uri" because the server expects the originally registered redirect_uri. Changes: - clientInformation() now validates stored redirect_uris match current redirectUrl, invalidating stale registrations that would cause errors - redirectToAuthorization() reuses pre-started callback server instead of starting a new one, ensuring consistent port usage throughout the OAuth flow Co-Authored-By: Claude Opus 4.5 --- src/oauth.ts | 70 +++++++++++++++++++++++++++++++-------------- tests/oauth.test.ts | 38 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/src/oauth.ts b/src/oauth.ts index 6bdfe81..1bbe12f 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -549,6 +549,7 @@ export class McpCliOAuthProvider implements OAuthClientProvider { /** * Load client information from config or file + * Validates that stored redirect_uris match current redirectUrl */ clientInformation(): OAuthClientInformationMixed | undefined { // First check if client ID is configured statically @@ -563,7 +564,24 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } // Otherwise, try to load from dynamic registration - return readJsonFile(this.paths.client); + const stored = readJsonFile(this.paths.client); + + // Validate stored redirect_uris match current redirectUrl + // If they don't match, the server will reject with "Invalid redirect_uri" + if (stored?.redirect_uris && stored.redirect_uris.length > 0) { + const currentRedirectUrl = this.redirectUrl ? String(this.redirectUrl) : undefined; + const storedHasCurrentUrl = currentRedirectUrl && stored.redirect_uris.includes(currentRedirectUrl); + + if (!storedHasCurrentUrl) { + debug( + `Stored client redirect_uris [${stored.redirect_uris.join(', ')}] don't match current [${currentRedirectUrl}] - invalidating client registration`, + ); + this.invalidateCredentials('client'); + return undefined; + } + } + + return stored; } /** @@ -606,8 +624,8 @@ export class McpCliOAuthProvider implements OAuthClientProvider { /** * Redirect to authorization URL - * Starts the callback server FIRST (with port fallback), then opens the browser - * This prevents the race condition where the browser redirects before the server is ready + * If callback server was pre-started, reuses it; otherwise starts one with port fallback + * Opens the browser only after the server is ready */ redirectToAuthorization(authorizationUrl: URL): void { // If interactive auth is disabled, don't start server or open browser @@ -618,32 +636,40 @@ export class McpCliOAuthProvider implements OAuthClientProvider { return; } - const portsToTry = this.getPortsToTry(); + // Helper to open browser after server is ready + const openBrowserWithUrl = () => { + // Update authorization URL with actual redirect_uri (may have changed due to port fallback) + const actualRedirectUri = this.redirectUrl; + if (actualRedirectUri) { + authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); + } + + // Now open the browser + console.error(`\nAuthorizing ${this.serverName}...`); + console.error( + `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, + ); + openBrowser(authorizationUrl.toString()).catch(() => { + // Error already logged in openBrowser + }); + }; + + // If callback server was already pre-started, just open the browser + if (this.callbackServerState && this.actualCallbackPort !== null) { + debug(`Reusing pre-started callback server on port ${this.actualCallbackPort} for ${this.serverName}`); + openBrowserWithUrl(); + return; + } - // Start the callback server BEFORE opening the browser - // This is critical to avoid race conditions - // Store the promise so waitForCallback can properly await it + // Otherwise, start the callback server BEFORE opening the browser + const portsToTry = this.getPortsToTry(); this.callbackServerStarting = startCallbackServerWithFallback(portsToTry) .then(({ state, actualPort }) => { this.callbackServerState = state; this.actualCallbackPort = actualPort; this.callbackServerStarting = null; debug(`Callback server ready for ${this.serverName} on port ${actualPort}`); - - // Update authorization URL with actual redirect_uri - const actualRedirectUri = this.redirectUrl; - if (actualRedirectUri) { - authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); - } - - // Now open the browser - console.error(`\nAuthorizing ${this.serverName}...`); - console.error( - `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, - ); - openBrowser(authorizationUrl.toString()).catch(() => { - // Error already logged in openBrowser - }); + openBrowserWithUrl(); }) .catch((error) => { this.callbackServerStarting = null; diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index a7da145..ca51950 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -186,6 +186,44 @@ describe('oauth', () => { const info = provider.clientInformation(); expect(info?.client_id).toBe('static-id'); }); + + test('invalidates client when stored redirect_uris do not match current redirectUrl', () => { + const config: OAuthConfig = { callbackPort: 3000 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + // Save client info with different redirect_uri (e.g., from previous registration on port 8080) + const clientPath = join(testDir, '.mcp-cli', 'clients', 'test.json'); + const storedClient = { + client_id: 'old-client', + redirect_uris: ['http://localhost:8080/callback'], + }; + writeFileSync(clientPath, JSON.stringify(storedClient)); + + // Should return undefined because redirect_uris don't match + // (current redirectUrl is http://localhost:3000/callback but stored has 8080) + const info = provider.clientInformation(); + expect(info).toBeUndefined(); + + // Client file should be deleted + expect(existsSync(clientPath)).toBe(false); + }); + + test('returns client when stored redirect_uris match current redirectUrl', () => { + const config: OAuthConfig = { callbackPort: 3000 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + // Save client info with matching redirect_uri + const clientPath = join(testDir, '.mcp-cli', 'clients', 'test.json'); + const storedClient = { + client_id: 'matching-client', + redirect_uris: ['http://localhost:3000/callback'], + }; + writeFileSync(clientPath, JSON.stringify(storedClient)); + + // Should return the client since redirect_uris match + const info = provider.clientInformation(); + expect(info?.client_id).toBe('matching-client'); + }); }); describe('tokens', () => { From b86a3921a7714ae780e7d2c1c89eb2861292d14d Mon Sep 17 00:00:00 2001 From: Atte Huhtakangas Date: Wed, 28 Jan 2026 11:38:32 +0000 Subject: [PATCH 5/9] refactor: split oauth.ts into logical modules Split the 850-line oauth.ts into smaller, focused modules: - types.ts: Interfaces (OAuthConfig, OAuthCallbackResult) and constants - storage.ts: File storage utilities for tokens, clients, verifiers - browser.ts: Cross-platform browser opening utility - callback-server.ts: HTTP callback server with HTML templates - provider.ts: Main McpCliOAuthProvider class - index.ts: Re-exports for backwards compatibility Co-Authored-By: Claude Opus 4.5 --- src/client.ts | 2 +- src/oauth.ts | 851 ----------------------------------- src/oauth/browser.ts | 41 ++ src/oauth/callback-server.ts | 258 +++++++++++ src/oauth/index.ts | 58 +++ src/oauth/provider.ts | 411 +++++++++++++++++ src/oauth/storage.ts | 131 ++++++ src/oauth/types.ts | 40 ++ tests/oauth.test.ts | 2 +- 9 files changed, 941 insertions(+), 853 deletions(-) delete mode 100644 src/oauth.ts create mode 100644 src/oauth/browser.ts create mode 100644 src/oauth/callback-server.ts create mode 100644 src/oauth/index.ts create mode 100644 src/oauth/provider.ts create mode 100644 src/oauth/storage.ts create mode 100644 src/oauth/types.ts diff --git a/src/client.ts b/src/client.ts index dcc385d..62f4cbf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -27,7 +27,7 @@ import { getDaemonConnection, } from './daemon-client.js'; import { formatCliError, oauthFlowError } from './errors.js'; -import { McpCliOAuthProvider } from './oauth.js'; +import { McpCliOAuthProvider } from './oauth/index.js'; import { VERSION } from './version.js'; // Re-export config utilities for convenience diff --git a/src/oauth.ts b/src/oauth.ts deleted file mode 100644 index 1bbe12f..0000000 --- a/src/oauth.ts +++ /dev/null @@ -1,851 +0,0 @@ -/** - * OAuth Provider Implementation for mcp-cli - * - * Implements OAuthClientProvider interface with file-based token storage - * for persistent authentication across CLI invocations. - */ - -import { exec } from 'node:child_process'; -import { - existsSync, - mkdirSync, - readFileSync, - unlinkSync, - writeFileSync, -} from 'node:fs'; -import { type Server, createServer } from 'node:http'; -import { homedir, platform } from 'node:os'; -import { join } from 'node:path'; -import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'; -import type { - OAuthClientInformationMixed, - OAuthClientMetadata, - OAuthTokens, -} from '@modelcontextprotocol/sdk/shared/auth.js'; -import { debug } from './config.js'; - -/** - * Get the MCP CLI storage directory - * Can be overridden via MCP_CLI_HOME env var for testing - */ -function getMcpCliDir(): string { - const baseDir = process.env.MCP_CLI_HOME || homedir(); - return join(baseDir, '.mcp-cli'); -} - -// Storage directories (lazily resolved) -function getStoragePaths() { - const mcpCliDir = getMcpCliDir(); - return { - tokens: join(mcpCliDir, 'tokens'), - clients: join(mcpCliDir, 'clients'), - verifiers: join(mcpCliDir, 'verifiers'), - }; -} - -// Default callback port for OAuth redirect -const DEFAULT_CALLBACK_PORT = 8095; - -// Default port fallback order: 80 (standard), common alternatives, then random -const DEFAULT_PORT_FALLBACK_ORDER = [80, 8080, 3000, 8095, 0]; // 0 = random port - -/** - * Pretty HTML template for successful OAuth callback - */ -const SUCCESS_HTML = ` - - - Authorization Successful - - - -
-
- - - -
-

Authorization Successful

-

You can close this window and return to the terminal.

-
- - -`; - -/** - * Pretty HTML template for OAuth error - */ -function errorHtml(message: string): string { - const escapedMessage = message - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"'); - - return ` - - - Authorization Failed - - - -
-
- - - -
-

Authorization Failed

-

An error occurred during authorization.

-
${escapedMessage}
-
- -`; -} - -/** - * OAuth configuration from server config - */ -export interface OAuthConfig { - grantType?: 'authorization_code' | 'client_credentials'; - clientId?: string; - clientSecret?: string; - scope?: string; - callbackPort?: number; - /** Optional: explicit list of ports to try in order (overrides default fallback) */ - callbackPorts?: number[]; -} - -/** - * Ensure a directory exists - */ -function ensureDir(dir: string): void { - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true, mode: 0o700 }); - } -} - -/** - * Read JSON file safely - */ -function readJsonFile(path: string): T | undefined { - try { - if (!existsSync(path)) { - return undefined; - } - const content = readFileSync(path, 'utf-8'); - return JSON.parse(content) as T; - } catch (error) { - debug(`Failed to read ${path}: ${(error as Error).message}`); - return undefined; - } -} - -/** - * Write JSON file safely - */ -function writeJsonFile(path: string, data: unknown): void { - ensureDir(join(path, '..')); - writeFileSync(path, JSON.stringify(data, null, 2), { mode: 0o600 }); -} - -/** - * Read text file safely - */ -function readTextFile(path: string): string | undefined { - try { - if (!existsSync(path)) { - return undefined; - } - return readFileSync(path, 'utf-8').trim(); - } catch (error) { - debug(`Failed to read ${path}: ${(error as Error).message}`); - return undefined; - } -} - -/** - * Write text file safely - */ -function writeTextFile(path: string, content: string): void { - ensureDir(join(path, '..')); - writeFileSync(path, content, { mode: 0o600 }); -} - -/** - * Delete file safely - */ -function deleteFile(path: string): void { - try { - if (existsSync(path)) { - unlinkSync(path); - } - } catch (error) { - debug(`Failed to delete ${path}: ${(error as Error).message}`); - } -} - -/** - * Sanitize server name for use as filename - */ -function sanitizeServerName(name: string): string { - return name.replace(/[^a-zA-Z0-9_-]/g, '_'); -} - -/** - * Get file paths for a server - */ -function getServerPaths(serverName: string): { - tokens: string; - client: string; - verifier: string; -} { - const safeName = sanitizeServerName(serverName); - const dirs = getStoragePaths(); - return { - tokens: join(dirs.tokens, `${safeName}.json`), - client: join(dirs.clients, `${safeName}.json`), - verifier: join(dirs.verifiers, `${safeName}.txt`), - }; -} - -/** - * Open URL in default browser (cross-platform) - */ -export function openBrowser(url: string): Promise { - return new Promise((resolve, reject) => { - const os = platform(); - let command: string; - - switch (os) { - case 'darwin': - command = `open "${url}"`; - break; - case 'win32': - command = `start "" "${url}"`; - break; - default: - // Linux and others - command = `xdg-open "${url}"`; - } - - debug(`Opening browser: ${command}`); - - exec(command, (error) => { - if (error) { - console.error(`Failed to open browser: ${error.message}`); - console.error(`Please manually open: ${url}`); - reject(error); - } else { - resolve(); - } - }); - }); -} - -/** - * Result from OAuth callback - */ -export interface OAuthCallbackResult { - code: string; -} - -/** - * Callback server state for managing the OAuth callback listener - */ -interface CallbackServerState { - server: Server; - promise: Promise; - cleanup: () => void; -} - -/** - * Start an OAuth callback server that listens for the authorization code - * Returns immediately with a promise that resolves when the callback is received - */ -function startCallbackServer( - port: number, - timeoutMs = 300000, -): Promise { - return new Promise((resolveStart, rejectStart) => { - let callbackResolve: (result: OAuthCallbackResult) => void; - let callbackReject: (error: Error) => void; - let timeoutId: ReturnType | undefined; - let server: Server | undefined; - - const cleanup = () => { - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - if (server) { - server.close(); - server = undefined; - } - }; - - const callbackPromise = new Promise( - (resolve, reject) => { - callbackResolve = resolve; - callbackReject = reject; - }, - ); - - timeoutId = setTimeout(() => { - cleanup(); - callbackReject( - new Error('OAuth callback timeout - no authorization code received'), - ); - }, timeoutMs); - - server = createServer((req, res) => { - // Ignore favicon requests - if (req.url === '/favicon.ico') { - res.writeHead(404); - res.end(); - return; - } - - debug(`Received OAuth callback: ${req.url}`); - - const parsedUrl = new URL(req.url || '', `http://localhost:${port}`); - const code = parsedUrl.searchParams.get('code'); - const error = parsedUrl.searchParams.get('error'); - const errorDescription = parsedUrl.searchParams.get('error_description'); - - if (code) { - debug(`Authorization code received: ${code.substring(0, 10)}...`); - - res.writeHead(200, { 'Content-Type': 'text/html' }); - res.end(SUCCESS_HTML); - - cleanup(); - callbackResolve({ code }); - } else if (error) { - const message = errorDescription || error; - debug(`OAuth error: ${message}`); - - res.writeHead(400, { 'Content-Type': 'text/html' }); - res.end(errorHtml(message)); - - cleanup(); - callbackReject(new Error(`OAuth authorization failed: ${message}`)); - } else { - res.writeHead(400, { 'Content-Type': 'text/plain' }); - res.end('Bad request: missing authorization code'); - } - }); - - server.on('error', (error) => { - cleanup(); - rejectStart( - new Error(`Failed to start OAuth callback server: ${error.message}`), - ); - }); - - server.listen(port, () => { - debug( - `OAuth callback server listening on http://localhost:${port}/callback`, - ); - resolveStart({ - server: server!, - promise: callbackPromise, - cleanup, - }); - }); - }); -} - -/** - * Start an OAuth callback server with port fallback - * Tries ports in order until one succeeds - * @returns The server state and the actual port used - */ -async function startCallbackServerWithFallback( - portsToTry: number[], - timeoutMs = 300000, -): Promise<{ state: CallbackServerState; actualPort: number }> { - const errors: string[] = []; - - for (const port of portsToTry) { - try { - debug(`Trying to start callback server on port ${port === 0 ? 'random' : port}`); - const state = await startCallbackServer(port, timeoutMs); - // Get the actual port (important when port was 0 for random) - const address = state.server.address(); - const actualPort = typeof address === 'object' && address ? address.port : port; - debug(`Callback server started on port ${actualPort}`); - return { state, actualPort }; - } catch (error) { - const message = (error as Error).message; - debug(`Port ${port} failed: ${message}`); - errors.push(`Port ${port}: ${message}`); - // Continue to next port - } - } - - // All ports failed - throw new Error( - `Failed to start OAuth callback server. Tried ports: ${portsToTry.join(', ')}.\n${errors.join('\n')}`, - ); -} - -/** - * Wait for OAuth callback on local server - * Returns the authorization code from the callback - * @deprecated Use McpCliOAuthProvider.waitForCallback() instead - */ -export async function waitForOAuthCallback( - port: number = DEFAULT_CALLBACK_PORT, - timeoutMs = 300000, -): Promise { - const state = await startCallbackServer(port, timeoutMs); - return state.promise; -} - -/** - * MCP CLI OAuth Provider - * - * Implements the OAuthClientProvider interface with file-based persistence - * for tokens, client information, and PKCE verifiers. - */ -export class McpCliOAuthProvider implements OAuthClientProvider { - private readonly serverName: string; - private readonly oauthConfig: OAuthConfig; - private readonly serverUrl: string; - private readonly paths: { tokens: string; client: string; verifier: string }; - - // Callback server state - started before browser opens - private callbackServerState: CallbackServerState | null = null; - private callbackServerStarting: Promise | null = null; - - // Actual port used by callback server (may differ from configured port due to fallback) - private actualCallbackPort: number | null = null; - - // Whether interactive auth (browser + callback) is allowed - private _allowInteractiveAuth = true; - - constructor(serverName: string, serverUrl: string, oauthConfig: OAuthConfig) { - this.serverName = serverName; - this.serverUrl = serverUrl; - this.oauthConfig = oauthConfig; - this.paths = getServerPaths(serverName); - - // Ensure storage directories exist - const dirs = getStoragePaths(); - ensureDir(dirs.tokens); - ensureDir(dirs.clients); - ensureDir(dirs.verifiers); - } - - /** - * Redirect URL for OAuth callback - * Returns undefined for client_credentials flow (non-interactive) - * Uses actualCallbackPort if pre-started, otherwise configured/default port - * Omits port from URL when using port 80 (standard HTTP port) - */ - get redirectUrl(): string | URL | undefined { - if (this.oauthConfig.grantType === 'client_credentials') { - return undefined; - } - // Use actual port if server was pre-started, otherwise fall back to configured/default - const port = this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; - // Omit port from URL when using standard HTTP port 80 - if (port === 80) { - return 'http://localhost/callback'; - } - return `http://localhost:${port}/callback`; - } - - /** - * OAuth client metadata - */ - get clientMetadata(): OAuthClientMetadata { - const redirectUrl = this.redirectUrl; - const grantTypes = - this.oauthConfig.grantType === 'client_credentials' - ? ['client_credentials'] - : ['authorization_code', 'refresh_token']; - - // Convert URL to string if needed - const redirectUriStr = redirectUrl ? String(redirectUrl) : undefined; - - return { - client_name: `mcp-cli (${this.serverName})`, - // Zod validates these as URLs at runtime; TypeScript type expects string[] - redirect_uris: redirectUriStr ? [redirectUriStr] : [], - grant_types: grantTypes, - response_types: - this.oauthConfig.grantType === 'client_credentials' ? [] : ['code'], - token_endpoint_auth_method: this.oauthConfig.clientSecret - ? 'client_secret_post' - : 'none', - scope: this.oauthConfig.scope, - }; - } - - /** - * Load client information from config or file - * Validates that stored redirect_uris match current redirectUrl - */ - clientInformation(): OAuthClientInformationMixed | undefined { - // First check if client ID is configured statically - if (this.oauthConfig.clientId) { - const info: OAuthClientInformationMixed = { - client_id: this.oauthConfig.clientId, - }; - if (this.oauthConfig.clientSecret) { - info.client_secret = this.oauthConfig.clientSecret; - } - return info; - } - - // Otherwise, try to load from dynamic registration - const stored = readJsonFile(this.paths.client); - - // Validate stored redirect_uris match current redirectUrl - // If they don't match, the server will reject with "Invalid redirect_uri" - if (stored?.redirect_uris && stored.redirect_uris.length > 0) { - const currentRedirectUrl = this.redirectUrl ? String(this.redirectUrl) : undefined; - const storedHasCurrentUrl = currentRedirectUrl && stored.redirect_uris.includes(currentRedirectUrl); - - if (!storedHasCurrentUrl) { - debug( - `Stored client redirect_uris [${stored.redirect_uris.join(', ')}] don't match current [${currentRedirectUrl}] - invalidating client registration`, - ); - this.invalidateCredentials('client'); - return undefined; - } - } - - return stored; - } - - /** - * Save dynamically registered client information - */ - saveClientInformation(clientInformation: OAuthClientInformationMixed): void { - debug(`Saving client information for ${this.serverName}`); - writeJsonFile(this.paths.client, clientInformation); - } - - /** - * Load OAuth tokens - */ - tokens(): OAuthTokens | undefined { - return readJsonFile(this.paths.tokens); - } - - /** - * Save OAuth tokens - */ - saveTokens(tokens: OAuthTokens): void { - debug(`Saving tokens for ${this.serverName}`); - writeJsonFile(this.paths.tokens, tokens); - } - - /** - * Set whether interactive auth is allowed - * When disabled, redirectToAuthorization will not open browser or start server - */ - setAllowInteractiveAuth(allow: boolean): void { - this._allowInteractiveAuth = allow; - } - - /** - * Check if interactive auth was blocked - */ - get interactiveAuthBlocked(): boolean { - return !this._allowInteractiveAuth; - } - - /** - * Redirect to authorization URL - * If callback server was pre-started, reuses it; otherwise starts one with port fallback - * Opens the browser only after the server is ready - */ - redirectToAuthorization(authorizationUrl: URL): void { - // If interactive auth is disabled, don't start server or open browser - if (!this._allowInteractiveAuth) { - debug( - `Interactive auth disabled for ${this.serverName}, skipping browser redirect`, - ); - return; - } - - // Helper to open browser after server is ready - const openBrowserWithUrl = () => { - // Update authorization URL with actual redirect_uri (may have changed due to port fallback) - const actualRedirectUri = this.redirectUrl; - if (actualRedirectUri) { - authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); - } - - // Now open the browser - console.error(`\nAuthorizing ${this.serverName}...`); - console.error( - `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, - ); - openBrowser(authorizationUrl.toString()).catch(() => { - // Error already logged in openBrowser - }); - }; - - // If callback server was already pre-started, just open the browser - if (this.callbackServerState && this.actualCallbackPort !== null) { - debug(`Reusing pre-started callback server on port ${this.actualCallbackPort} for ${this.serverName}`); - openBrowserWithUrl(); - return; - } - - // Otherwise, start the callback server BEFORE opening the browser - const portsToTry = this.getPortsToTry(); - this.callbackServerStarting = startCallbackServerWithFallback(portsToTry) - .then(({ state, actualPort }) => { - this.callbackServerState = state; - this.actualCallbackPort = actualPort; - this.callbackServerStarting = null; - debug(`Callback server ready for ${this.serverName} on port ${actualPort}`); - openBrowserWithUrl(); - }) - .catch((error) => { - this.callbackServerStarting = null; - console.error(`Failed to start callback server: ${error.message}`); - console.error( - `Please manually visit: ${authorizationUrl.toString()}`, - ); - throw error; - }); - } - - /** - * Wait for the OAuth callback to complete - * Returns the authorization code - */ - async waitForCallback(): Promise { - // Wait for server to finish starting if in progress - if (this.callbackServerStarting) { - await this.callbackServerStarting; - } - - if (!this.callbackServerState) { - // Fallback: start server now if not already started (with port fallback) - const portsToTry = this.getPortsToTry(); - const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); - this.callbackServerState = state; - this.actualCallbackPort = actualPort; - } - return this.callbackServerState.promise; - } - - /** - * Check if this provider has a pending callback server - */ - hasPendingCallback(): boolean { - return this.callbackServerState !== null; - } - - /** - * Clean up the callback server if running - */ - cleanupCallbackServer(): void { - if (this.callbackServerState) { - this.callbackServerState.cleanup(); - this.callbackServerState = null; - } - } - - /** - * Save PKCE code verifier - */ - saveCodeVerifier(codeVerifier: string): void { - debug(`Saving code verifier for ${this.serverName}`); - writeTextFile(this.paths.verifier, codeVerifier); - } - - /** - * Load PKCE code verifier - */ - codeVerifier(): string { - const verifier = readTextFile(this.paths.verifier); - if (!verifier) { - throw new Error( - 'No code verifier found - OAuth flow may have been interrupted', - ); - } - return verifier; - } - - /** - * Invalidate stored credentials - */ - invalidateCredentials(scope: 'all' | 'client' | 'tokens' | 'verifier'): void { - debug(`Invalidating credentials for ${this.serverName}: ${scope}`); - - switch (scope) { - case 'all': - deleteFile(this.paths.tokens); - deleteFile(this.paths.client); - deleteFile(this.paths.verifier); - break; - case 'client': - deleteFile(this.paths.client); - break; - case 'tokens': - deleteFile(this.paths.tokens); - break; - case 'verifier': - deleteFile(this.paths.verifier); - break; - } - } - - /** - * Prepare token request for client_credentials flow - */ - prepareTokenRequest(scope?: string): URLSearchParams | undefined { - if (this.oauthConfig.grantType !== 'client_credentials') { - return undefined; - } - - const params = new URLSearchParams({ - grant_type: 'client_credentials', - }); - - const requestScope = scope || this.oauthConfig.scope; - if (requestScope) { - params.set('scope', requestScope); - } - - return params; - } - - /** - * Get the callback port for this provider - */ - getCallbackPort(): number { - return this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; - } - - /** - * Get the list of ports to try for the callback server - * If a specific port is configured, it comes first - * Then falls back to the default port order - */ - getPortsToTry(): number[] { - const ports: number[] = []; - - // If explicit callbackPorts array is configured, use it - if (this.oauthConfig.callbackPorts && this.oauthConfig.callbackPorts.length > 0) { - return [...this.oauthConfig.callbackPorts]; - } - - // If a single callbackPort is configured, try it first - if (this.oauthConfig.callbackPort) { - ports.push(this.oauthConfig.callbackPort); - } - - // Add default fallback ports (excluding any already added) - for (const port of DEFAULT_PORT_FALLBACK_ORDER) { - if (!ports.includes(port)) { - ports.push(port); - } - } - - return ports; - } - - /** - * Pre-start the callback server to determine the actual port - * Call this before accessing redirectUrl to ensure correct port in authorization URL - * Returns the actual port used - */ - async preStartCallbackServer(): Promise { - // Skip for client_credentials flow - if (this.oauthConfig.grantType === 'client_credentials') { - return 0; - } - - // Already started or starting - if (this.actualCallbackPort !== null) { - return this.actualCallbackPort; - } - if (this.callbackServerStarting) { - await this.callbackServerStarting; - return this.actualCallbackPort!; - } - - const portsToTry = this.getPortsToTry(); - debug(`Pre-starting callback server for ${this.serverName}, trying ports: ${portsToTry.join(', ')}`); - - const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); - this.callbackServerState = state; - this.actualCallbackPort = actualPort; - - debug(`Callback server pre-started on port ${actualPort} for ${this.serverName}`); - return actualPort; - } -} diff --git a/src/oauth/browser.ts b/src/oauth/browser.ts new file mode 100644 index 0000000..ea46dff --- /dev/null +++ b/src/oauth/browser.ts @@ -0,0 +1,41 @@ +/** + * Browser utilities for OAuth + */ + +import { exec } from 'node:child_process'; +import { platform } from 'node:os'; +import { debug } from '../config.js'; + +/** + * Open URL in default browser (cross-platform) + */ +export function openBrowser(url: string): Promise { + return new Promise((resolve, reject) => { + const os = platform(); + let command: string; + + switch (os) { + case 'darwin': + command = `open "${url}"`; + break; + case 'win32': + command = `start "" "${url}"`; + break; + default: + // Linux and others + command = `xdg-open "${url}"`; + } + + debug(`Opening browser: ${command}`); + + exec(command, (error) => { + if (error) { + console.error(`Failed to open browser: ${error.message}`); + console.error(`Please manually open: ${url}`); + reject(error); + } else { + resolve(); + } + }); + }); +} diff --git a/src/oauth/callback-server.ts b/src/oauth/callback-server.ts new file mode 100644 index 0000000..830a974 --- /dev/null +++ b/src/oauth/callback-server.ts @@ -0,0 +1,258 @@ +/** + * OAuth callback server + * + * HTTP server that listens for OAuth authorization callbacks + * and extracts the authorization code. + */ + +import { createServer } from 'node:http'; +import { debug } from '../config.js'; +import type { CallbackServerState, OAuthCallbackResult } from './types.js'; + +/** + * Pretty HTML template for successful OAuth callback + */ +const SUCCESS_HTML = ` + + + Authorization Successful + + + +
+
+ + + +
+

Authorization Successful

+

You can close this window and return to the terminal.

+
+ + +`; + +/** + * Pretty HTML template for OAuth error + */ +function errorHtml(message: string): string { + const escapedMessage = message + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); + + return ` + + + Authorization Failed + + + +
+
+ + + +
+

Authorization Failed

+

An error occurred during authorization.

+
${escapedMessage}
+
+ +`; +} + +/** + * Start an OAuth callback server that listens for the authorization code + * Returns immediately with a promise that resolves when the callback is received + */ +export function startCallbackServer( + port: number, + timeoutMs = 300000, +): Promise { + return new Promise((resolveStart, rejectStart) => { + let callbackResolve: (result: OAuthCallbackResult) => void; + let callbackReject: (error: Error) => void; + let timeoutId: ReturnType | undefined; + let server: ReturnType | undefined; + + const cleanup = () => { + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + if (server) { + server.close(); + server = undefined; + } + }; + + const callbackPromise = new Promise( + (resolve, reject) => { + callbackResolve = resolve; + callbackReject = reject; + }, + ); + + timeoutId = setTimeout(() => { + cleanup(); + callbackReject( + new Error('OAuth callback timeout - no authorization code received'), + ); + }, timeoutMs); + + server = createServer((req, res) => { + // Ignore favicon requests + if (req.url === '/favicon.ico') { + res.writeHead(404); + res.end(); + return; + } + + debug(`Received OAuth callback: ${req.url}`); + + const parsedUrl = new URL(req.url || '', `http://localhost:${port}`); + const code = parsedUrl.searchParams.get('code'); + const error = parsedUrl.searchParams.get('error'); + const errorDescription = parsedUrl.searchParams.get('error_description'); + + if (code) { + debug(`Authorization code received: ${code.substring(0, 10)}...`); + + res.writeHead(200, { 'Content-Type': 'text/html' }); + res.end(SUCCESS_HTML); + + cleanup(); + callbackResolve({ code }); + } else if (error) { + const message = errorDescription || error; + debug(`OAuth error: ${message}`); + + res.writeHead(400, { 'Content-Type': 'text/html' }); + res.end(errorHtml(message)); + + cleanup(); + callbackReject(new Error(`OAuth authorization failed: ${message}`)); + } else { + res.writeHead(400, { 'Content-Type': 'text/plain' }); + res.end('Bad request: missing authorization code'); + } + }); + + server.on('error', (error) => { + cleanup(); + rejectStart( + new Error(`Failed to start OAuth callback server: ${error.message}`), + ); + }); + + server.listen(port, () => { + debug( + `OAuth callback server listening on http://localhost:${port}/callback`, + ); + resolveStart({ + server: server!, + promise: callbackPromise, + cleanup, + }); + }); + }); +} + +/** + * Start an OAuth callback server with port fallback + * Tries ports in order until one succeeds + * @returns The server state and the actual port used + */ +export async function startCallbackServerWithFallback( + portsToTry: number[], + timeoutMs = 300000, +): Promise<{ state: CallbackServerState; actualPort: number }> { + const errors: string[] = []; + + for (const port of portsToTry) { + try { + debug(`Trying to start callback server on port ${port === 0 ? 'random' : port}`); + const state = await startCallbackServer(port, timeoutMs); + // Get the actual port (important when port was 0 for random) + const address = state.server.address(); + const actualPort = typeof address === 'object' && address ? address.port : port; + debug(`Callback server started on port ${actualPort}`); + return { state, actualPort }; + } catch (error) { + const message = (error as Error).message; + debug(`Port ${port} failed: ${message}`); + errors.push(`Port ${port}: ${message}`); + // Continue to next port + } + } + + // All ports failed + throw new Error( + `Failed to start OAuth callback server. Tried ports: ${portsToTry.join(', ')}.\n${errors.join('\n')}`, + ); +} diff --git a/src/oauth/index.ts b/src/oauth/index.ts new file mode 100644 index 0000000..91a21b3 --- /dev/null +++ b/src/oauth/index.ts @@ -0,0 +1,58 @@ +/** + * OAuth module for mcp-cli + * + * Provides OAuth authentication support for HTTP MCP servers, + * including authorization code flow with PKCE and client credentials flow. + */ + +// Types +export { + type CallbackServerState, + DEFAULT_CALLBACK_PORT, + DEFAULT_PORT_FALLBACK_ORDER, + type OAuthCallbackResult, + type OAuthConfig, +} from './types.js'; + +// Storage utilities +export { + deleteFile, + ensureDir, + getMcpCliDir, + getServerPaths, + getStoragePaths, + readJsonFile, + readTextFile, + sanitizeServerName, + writeJsonFile, + writeTextFile, +} from './storage.js'; + +// Browser utilities +export { openBrowser } from './browser.js'; + +// Callback server +export { + startCallbackServer, + startCallbackServerWithFallback, +} from './callback-server.js'; + +// Main provider +export { McpCliOAuthProvider } from './provider.js'; + +// Legacy export for backwards compatibility +import { startCallbackServer } from './callback-server.js'; +import { DEFAULT_CALLBACK_PORT, type OAuthCallbackResult } from './types.js'; + +/** + * Wait for OAuth callback on local server + * Returns the authorization code from the callback + * @deprecated Use McpCliOAuthProvider.waitForCallback() instead + */ +export async function waitForOAuthCallback( + port: number = DEFAULT_CALLBACK_PORT, + timeoutMs = 300000, +): Promise { + const state = await startCallbackServer(port, timeoutMs); + return state.promise; +} diff --git a/src/oauth/provider.ts b/src/oauth/provider.ts new file mode 100644 index 0000000..69e66c8 --- /dev/null +++ b/src/oauth/provider.ts @@ -0,0 +1,411 @@ +/** + * MCP CLI OAuth Provider + * + * Implements the OAuthClientProvider interface with file-based persistence + * for tokens, client information, and PKCE verifiers. + */ + +import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'; +import type { + OAuthClientInformationMixed, + OAuthClientMetadata, + OAuthTokens, +} from '@modelcontextprotocol/sdk/shared/auth.js'; +import { debug } from '../config.js'; +import { openBrowser } from './browser.js'; +import { startCallbackServerWithFallback } from './callback-server.js'; +import { + deleteFile, + ensureDir, + getServerPaths, + getStoragePaths, + readJsonFile, + readTextFile, + writeJsonFile, + writeTextFile, +} from './storage.js'; +import { + type CallbackServerState, + DEFAULT_CALLBACK_PORT, + DEFAULT_PORT_FALLBACK_ORDER, + type OAuthCallbackResult, + type OAuthConfig, +} from './types.js'; + +export class McpCliOAuthProvider implements OAuthClientProvider { + private readonly serverName: string; + private readonly oauthConfig: OAuthConfig; + private readonly serverUrl: string; + private readonly paths: { tokens: string; client: string; verifier: string }; + + // Callback server state - started before browser opens + private callbackServerState: CallbackServerState | null = null; + private callbackServerStarting: Promise | null = null; + + // Actual port used by callback server (may differ from configured port due to fallback) + private actualCallbackPort: number | null = null; + + // Whether interactive auth (browser + callback) is allowed + private _allowInteractiveAuth = true; + + constructor(serverName: string, serverUrl: string, oauthConfig: OAuthConfig) { + this.serverName = serverName; + this.serverUrl = serverUrl; + this.oauthConfig = oauthConfig; + this.paths = getServerPaths(serverName); + + // Ensure storage directories exist + const dirs = getStoragePaths(); + ensureDir(dirs.tokens); + ensureDir(dirs.clients); + ensureDir(dirs.verifiers); + } + + /** + * Redirect URL for OAuth callback + * Returns undefined for client_credentials flow (non-interactive) + * Uses actualCallbackPort if pre-started, otherwise configured/default port + * Omits port from URL when using port 80 (standard HTTP port) + */ + get redirectUrl(): string | URL | undefined { + if (this.oauthConfig.grantType === 'client_credentials') { + return undefined; + } + // Use actual port if server was pre-started, otherwise fall back to configured/default + const port = this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + // Omit port from URL when using standard HTTP port 80 + if (port === 80) { + return 'http://localhost/callback'; + } + return `http://localhost:${port}/callback`; + } + + /** + * OAuth client metadata + */ + get clientMetadata(): OAuthClientMetadata { + const redirectUrl = this.redirectUrl; + const grantTypes = + this.oauthConfig.grantType === 'client_credentials' + ? ['client_credentials'] + : ['authorization_code', 'refresh_token']; + + // Convert URL to string if needed + const redirectUriStr = redirectUrl ? String(redirectUrl) : undefined; + + return { + client_name: `mcp-cli (${this.serverName})`, + // Zod validates these as URLs at runtime; TypeScript type expects string[] + redirect_uris: redirectUriStr ? [redirectUriStr] : [], + grant_types: grantTypes, + response_types: + this.oauthConfig.grantType === 'client_credentials' ? [] : ['code'], + token_endpoint_auth_method: this.oauthConfig.clientSecret + ? 'client_secret_post' + : 'none', + scope: this.oauthConfig.scope, + }; + } + + /** + * Load client information from config or file + * Validates that stored redirect_uris match current redirectUrl + */ + clientInformation(): OAuthClientInformationMixed | undefined { + // First check if client ID is configured statically + if (this.oauthConfig.clientId) { + const info: OAuthClientInformationMixed = { + client_id: this.oauthConfig.clientId, + }; + if (this.oauthConfig.clientSecret) { + info.client_secret = this.oauthConfig.clientSecret; + } + return info; + } + + // Otherwise, try to load from dynamic registration + const stored = readJsonFile(this.paths.client); + + // Validate stored redirect_uris match current redirectUrl + // If they don't match, the server will reject with "Invalid redirect_uri" + if (stored?.redirect_uris && stored.redirect_uris.length > 0) { + const currentRedirectUrl = this.redirectUrl ? String(this.redirectUrl) : undefined; + const storedHasCurrentUrl = currentRedirectUrl && stored.redirect_uris.includes(currentRedirectUrl); + + if (!storedHasCurrentUrl) { + debug( + `Stored client redirect_uris [${stored.redirect_uris.join(', ')}] don't match current [${currentRedirectUrl}] - invalidating client registration`, + ); + this.invalidateCredentials('client'); + return undefined; + } + } + + return stored; + } + + /** + * Save dynamically registered client information + */ + saveClientInformation(clientInformation: OAuthClientInformationMixed): void { + debug(`Saving client information for ${this.serverName}`); + writeJsonFile(this.paths.client, clientInformation); + } + + /** + * Load OAuth tokens + */ + tokens(): OAuthTokens | undefined { + return readJsonFile(this.paths.tokens); + } + + /** + * Save OAuth tokens + */ + saveTokens(tokens: OAuthTokens): void { + debug(`Saving tokens for ${this.serverName}`); + writeJsonFile(this.paths.tokens, tokens); + } + + /** + * Set whether interactive auth is allowed + * When disabled, redirectToAuthorization will not open browser or start server + */ + setAllowInteractiveAuth(allow: boolean): void { + this._allowInteractiveAuth = allow; + } + + /** + * Check if interactive auth was blocked + */ + get interactiveAuthBlocked(): boolean { + return !this._allowInteractiveAuth; + } + + /** + * Redirect to authorization URL + * If callback server was pre-started, reuses it; otherwise starts one with port fallback + * Opens the browser only after the server is ready + */ + redirectToAuthorization(authorizationUrl: URL): void { + // If interactive auth is disabled, don't start server or open browser + if (!this._allowInteractiveAuth) { + debug( + `Interactive auth disabled for ${this.serverName}, skipping browser redirect`, + ); + return; + } + + // Helper to open browser after server is ready + const openBrowserWithUrl = () => { + // Update authorization URL with actual redirect_uri (may have changed due to port fallback) + const actualRedirectUri = this.redirectUrl; + if (actualRedirectUri) { + authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); + } + + // Now open the browser + console.error(`\nAuthorizing ${this.serverName}...`); + console.error( + `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, + ); + openBrowser(authorizationUrl.toString()).catch(() => { + // Error already logged in openBrowser + }); + }; + + // If callback server was already pre-started, just open the browser + if (this.callbackServerState && this.actualCallbackPort !== null) { + debug(`Reusing pre-started callback server on port ${this.actualCallbackPort} for ${this.serverName}`); + openBrowserWithUrl(); + return; + } + + // Otherwise, start the callback server BEFORE opening the browser + const portsToTry = this.getPortsToTry(); + this.callbackServerStarting = startCallbackServerWithFallback(portsToTry) + .then(({ state, actualPort }) => { + this.callbackServerState = state; + this.actualCallbackPort = actualPort; + this.callbackServerStarting = null; + debug(`Callback server ready for ${this.serverName} on port ${actualPort}`); + openBrowserWithUrl(); + }) + .catch((error) => { + this.callbackServerStarting = null; + console.error(`Failed to start callback server: ${error.message}`); + console.error( + `Please manually visit: ${authorizationUrl.toString()}`, + ); + throw error; + }); + } + + /** + * Wait for the OAuth callback to complete + * Returns the authorization code + */ + async waitForCallback(): Promise { + // Wait for server to finish starting if in progress + if (this.callbackServerStarting) { + await this.callbackServerStarting; + } + + if (!this.callbackServerState) { + // Fallback: start server now if not already started (with port fallback) + const portsToTry = this.getPortsToTry(); + const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + this.callbackServerState = state; + this.actualCallbackPort = actualPort; + } + return this.callbackServerState.promise; + } + + /** + * Check if this provider has a pending callback server + */ + hasPendingCallback(): boolean { + return this.callbackServerState !== null; + } + + /** + * Clean up the callback server if running + */ + cleanupCallbackServer(): void { + if (this.callbackServerState) { + this.callbackServerState.cleanup(); + this.callbackServerState = null; + } + } + + /** + * Save PKCE code verifier + */ + saveCodeVerifier(codeVerifier: string): void { + debug(`Saving code verifier for ${this.serverName}`); + writeTextFile(this.paths.verifier, codeVerifier); + } + + /** + * Load PKCE code verifier + */ + codeVerifier(): string { + const verifier = readTextFile(this.paths.verifier); + if (!verifier) { + throw new Error( + 'No code verifier found - OAuth flow may have been interrupted', + ); + } + return verifier; + } + + /** + * Invalidate stored credentials + */ + invalidateCredentials(scope: 'all' | 'client' | 'tokens' | 'verifier'): void { + debug(`Invalidating credentials for ${this.serverName}: ${scope}`); + + switch (scope) { + case 'all': + deleteFile(this.paths.tokens); + deleteFile(this.paths.client); + deleteFile(this.paths.verifier); + break; + case 'client': + deleteFile(this.paths.client); + break; + case 'tokens': + deleteFile(this.paths.tokens); + break; + case 'verifier': + deleteFile(this.paths.verifier); + break; + } + } + + /** + * Prepare token request for client_credentials flow + */ + prepareTokenRequest(scope?: string): URLSearchParams | undefined { + if (this.oauthConfig.grantType !== 'client_credentials') { + return undefined; + } + + const params = new URLSearchParams({ + grant_type: 'client_credentials', + }); + + const requestScope = scope || this.oauthConfig.scope; + if (requestScope) { + params.set('scope', requestScope); + } + + return params; + } + + /** + * Get the callback port for this provider + */ + getCallbackPort(): number { + return this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + } + + /** + * Get the list of ports to try for the callback server + * If a specific port is configured, it comes first + * Then falls back to the default port order + */ + getPortsToTry(): number[] { + const ports: number[] = []; + + // If explicit callbackPorts array is configured, use it + if (this.oauthConfig.callbackPorts && this.oauthConfig.callbackPorts.length > 0) { + return [...this.oauthConfig.callbackPorts]; + } + + // If a single callbackPort is configured, try it first + if (this.oauthConfig.callbackPort) { + ports.push(this.oauthConfig.callbackPort); + } + + // Add default fallback ports (excluding any already added) + for (const port of DEFAULT_PORT_FALLBACK_ORDER) { + if (!ports.includes(port)) { + ports.push(port); + } + } + + return ports; + } + + /** + * Pre-start the callback server to determine the actual port + * Call this before accessing redirectUrl to ensure correct port in authorization URL + * Returns the actual port used + */ + async preStartCallbackServer(): Promise { + // Skip for client_credentials flow + if (this.oauthConfig.grantType === 'client_credentials') { + return 0; + } + + // Already started or starting + if (this.actualCallbackPort !== null) { + return this.actualCallbackPort; + } + if (this.callbackServerStarting) { + await this.callbackServerStarting; + return this.actualCallbackPort!; + } + + const portsToTry = this.getPortsToTry(); + debug(`Pre-starting callback server for ${this.serverName}, trying ports: ${portsToTry.join(', ')}`); + + const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + this.callbackServerState = state; + this.actualCallbackPort = actualPort; + + debug(`Callback server pre-started on port ${actualPort} for ${this.serverName}`); + return actualPort; + } +} diff --git a/src/oauth/storage.ts b/src/oauth/storage.ts new file mode 100644 index 0000000..3c496b0 --- /dev/null +++ b/src/oauth/storage.ts @@ -0,0 +1,131 @@ +/** + * OAuth file storage utilities + * + * Handles persistent storage of OAuth tokens, client information, + * and PKCE verifiers with secure file permissions. + */ + +import { + existsSync, + mkdirSync, + readFileSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { debug } from '../config.js'; + +/** + * Get the MCP CLI storage directory + * Can be overridden via MCP_CLI_HOME env var for testing + */ +export function getMcpCliDir(): string { + const baseDir = process.env.MCP_CLI_HOME || homedir(); + return join(baseDir, '.mcp-cli'); +} + +/** + * Get storage directory paths + */ +export function getStoragePaths() { + const mcpCliDir = getMcpCliDir(); + return { + tokens: join(mcpCliDir, 'tokens'), + clients: join(mcpCliDir, 'clients'), + verifiers: join(mcpCliDir, 'verifiers'), + }; +} + +/** + * Ensure a directory exists with secure permissions + */ +export function ensureDir(dir: string): void { + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true, mode: 0o700 }); + } +} + +/** + * Read JSON file safely + */ +export function readJsonFile(path: string): T | undefined { + try { + if (!existsSync(path)) { + return undefined; + } + const content = readFileSync(path, 'utf-8'); + return JSON.parse(content) as T; + } catch (error) { + debug(`Failed to read ${path}: ${(error as Error).message}`); + return undefined; + } +} + +/** + * Write JSON file safely with secure permissions + */ +export function writeJsonFile(path: string, data: unknown): void { + ensureDir(join(path, '..')); + writeFileSync(path, JSON.stringify(data, null, 2), { mode: 0o600 }); +} + +/** + * Read text file safely + */ +export function readTextFile(path: string): string | undefined { + try { + if (!existsSync(path)) { + return undefined; + } + return readFileSync(path, 'utf-8').trim(); + } catch (error) { + debug(`Failed to read ${path}: ${(error as Error).message}`); + return undefined; + } +} + +/** + * Write text file safely with secure permissions + */ +export function writeTextFile(path: string, content: string): void { + ensureDir(join(path, '..')); + writeFileSync(path, content, { mode: 0o600 }); +} + +/** + * Delete file safely + */ +export function deleteFile(path: string): void { + try { + if (existsSync(path)) { + unlinkSync(path); + } + } catch (error) { + debug(`Failed to delete ${path}: ${(error as Error).message}`); + } +} + +/** + * Sanitize server name for use as filename + */ +export function sanitizeServerName(name: string): string { + return name.replace(/[^a-zA-Z0-9_-]/g, '_'); +} + +/** + * Get file paths for a server's OAuth data + */ +export function getServerPaths(serverName: string): { + tokens: string; + client: string; + verifier: string; +} { + const safeName = sanitizeServerName(serverName); + const dirs = getStoragePaths(); + return { + tokens: join(dirs.tokens, `${safeName}.json`), + client: join(dirs.clients, `${safeName}.json`), + verifier: join(dirs.verifiers, `${safeName}.txt`), + }; +} diff --git a/src/oauth/types.ts b/src/oauth/types.ts new file mode 100644 index 0000000..251b04f --- /dev/null +++ b/src/oauth/types.ts @@ -0,0 +1,40 @@ +/** + * OAuth type definitions + */ + +import type { Server } from 'node:http'; + +/** + * OAuth configuration from server config + */ +export interface OAuthConfig { + grantType?: 'authorization_code' | 'client_credentials'; + clientId?: string; + clientSecret?: string; + scope?: string; + callbackPort?: number; + /** Optional: explicit list of ports to try in order (overrides default fallback) */ + callbackPorts?: number[]; +} + +/** + * Result from OAuth callback + */ +export interface OAuthCallbackResult { + code: string; +} + +/** + * Callback server state for managing the OAuth callback listener + */ +export interface CallbackServerState { + server: Server; + promise: Promise; + cleanup: () => void; +} + +// Default callback port for OAuth redirect +export const DEFAULT_CALLBACK_PORT = 8095; + +// Default port fallback order: 80 (standard), common alternatives, then random +export const DEFAULT_PORT_FALLBACK_ORDER = [80, 8080, 3000, 8095, 0]; // 0 = random port diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index ca51950..0ca0c5e 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -6,7 +6,7 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { McpCliOAuthProvider, type OAuthConfig } from '../src/oauth'; +import { McpCliOAuthProvider, type OAuthConfig } from '../src/oauth/index'; describe('oauth', () => { // Use a unique temp directory for each test run to avoid conflicts From cbfaf3790f4d0390f62414c4c79203859777dff9 Mon Sep 17 00:00:00 2001 From: philschmid Date: Thu, 5 Feb 2026 17:19:09 +0000 Subject: [PATCH 6/9] feat(oauth): non-blocking auth flow for AI agents CLI NEVER opens browser - always returns auth URL for AI agents. Key changes: - Removed allowInteractiveAuth option entirely (CLI is for AI agents) - redirectToAuthorization() now captures auth URL, never opens browser - AuthRequiredError includes authorization URL for immediate action - Callback server runs in background (5 min timeout) - CLI returns immediately - List command shows working servers + auth URLs for servers needing login - Random port by default to avoid conflicts with multiple OAuth servers - Added comprehensive OAuth configuration docs to README This builds on the previous OAuth commits in this branch. --- CHANGELOG.md | 11 +++ README.md | 136 ++++++++++++++++++++++++++++++++++ src/client.ts | 122 ++++++++++--------------------- src/commands/list.ts | 23 ++++-- src/oauth/callback-server.ts | 20 +++-- src/oauth/provider.ts | 137 ++++++++++++++++------------------- src/oauth/types.ts | 4 +- tests/oauth.test.ts | 12 ++- 8 files changed, 285 insertions(+), 180 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c56163..b04f885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [Unreleased] + +### Added + +- **Non-Blocking OAuth Flow for AI Agents** + - `AuthRequiredError` now includes authorization URL for immediate action + - Callback server runs in background (5 min timeout) - CLI returns immediately + - `mcp-cli` (list all) shows working servers + auth URLs for servers needing login + - Random port by default to avoid conflicts with multiple OAuth servers + - Updated README with sequence diagram and AI agent guidance + ## [0.3.0] - 2026-01-22 ### Added diff --git a/README.md b/README.md index 4d1c6a3..624852b 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,141 @@ Restrict which tools are available from a server using `allowedTools` and `disab "disabledTools": ["delete_file"] ``` +### OAuth Authentication + +For HTTP MCP servers that require OAuth (like Notion, GitHub, etc.), the CLI handles authentication automatically: + +```mermaid +sequenceDiagram + participant AI as AI Agent + participant CLI as mcp-cli + participant Server as Callback Server + participant User as User + AI->>CLI: mcp-cli + CLI->>CLI: Detect some servers need auth + CLI->>Server: Spawn background callback server + CLI-->>AI: List working servers + auth URL for others + AI->>User: "Please authenticate via this link" + User->>Server: Completes OAuth in browser + Server->>Server: Saves tokens to ~/.mcp-cli/tokens/ + User->>AI: "Done" + AI->>CLI: mcp-cli (retry) + CLI-->>AI: All servers now listed +``` + +**Configuration:** + +Most OAuth-enabled servers work with just a URL - the CLI handles dynamic client registration and PKCE automatically: + +```json +{ + "mcpServers": { + "notion": { + "url": "https://mcp.notion.com/mcp" + } + } +} +``` + +**OAuth Configuration Options:** + +For servers requiring custom OAuth settings, use the `oauth` object: + +```json +{ + "mcpServers": { + "my-server": { + "url": "https://api.example.com/mcp", + "oauth": { + "callbackPort": 8095, + "clientId": "your-client-id", + "clientSecret": "your-client-secret", + "scope": "read write", + "grantType": "authorization_code" + } + } + } +} +``` + +| Option | Description | Default | +|--------|-------------|---------| +| `callbackPort` | Port for OAuth callback server | Random | +| `clientId` | Pre-registered OAuth client ID | (dynamic registration) | +| `clientSecret` | OAuth client secret (for confidential clients) | (none) | +| `scope` | OAuth scopes to request | (none) | +| `grantType` | `authorization_code` or `client_credentials` | `authorization_code` | + +**Examples by scenario:** + +1. **Public server with dynamic registration** (most common): +```json +{ + "notion": { "url": "https://mcp.notion.com/mcp" } +} +``` + +2. **Server with pre-registered public client:** +```json +{ + "github": { + "url": "https://mcp.github.com/mcp", + "oauth": { "clientId": "abc123" } + } +} +``` + +3. **Confidential client (client_credentials):** +```json +{ + "internal-api": { + "url": "https://api.internal.com/mcp", + "oauth": { + "clientId": "service-account", + "clientSecret": "secret-key", + "grantType": "client_credentials" + } + } +} +``` + +**Token Storage:** + +Tokens are persisted in `~/.mcp-cli/` with secure file permissions (0600): + +``` +~/.mcp-cli/ +├── tokens/server-name.json # Access/refresh tokens +├── clients/server-name.json # Dynamic client registration +└── verifiers/server-name.txt # PKCE verifiers (temporary) +``` + +**Clearing cached tokens:** + +```bash +rm ~/.mcp-cli/tokens/notion.json # Re-authenticate on next call +rm -rf ~/.mcp-cli/ # Clear all OAuth data +``` + +**For AI Agents (non-interactive mode):** + +When listing multiple servers, working servers show their tools while auth-required servers display an actionable auth URL: + +``` +deepwiki + • read_wiki_structure + • read_wiki_contents + • ask_question + +notion + • +``` + +The callback server stays running in the background. After the user authenticates and confirms, retry the command to see all servers. + ### Config Resolution The CLI searches for configuration in this order: @@ -360,6 +495,7 @@ The CLI searches for configuration in this order: | `MCP_STRICT_ENV` | Error on missing `${VAR}` in config | `true` | | `MCP_NO_DAEMON` | Disable connection caching (force fresh connections) | `false` | | `MCP_DAEMON_TIMEOUT` | Idle timeout for cached connections (seconds) | `60` | +| `MCP_CLI_HOME` | Override OAuth storage directory (default: `$HOME`) | (none) | ## Using with AI Agents diff --git a/src/client.ts b/src/client.ts index 62f4cbf..1499466 100644 --- a/src/client.ts +++ b/src/client.ts @@ -52,24 +52,27 @@ export interface McpConnection { isDaemon: boolean; } -export interface ConnectOptions { - /** - * Allow interactive OAuth flow (browser opens, user authorizes) - * When false, throws an error if authentication is required - * Default: true - */ - allowInteractiveAuth?: boolean; -} +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export type ConnectOptions = Record; /** * Error thrown when authentication is required but interactive auth is disabled */ export class AuthRequiredError extends Error { - constructor(serverName: string) { - super( - `Server "${serverName}" requires authentication. Run 'mcp-cli ${serverName}' to authenticate.`, - ); + public readonly serverName: string; + public readonly authUrl: string | undefined; + + constructor(serverName: string, authUrl?: string) { + const message = authUrl + ? `[AUTH REQUIRED] ${serverName} + Authenticate at: ${authUrl} + Callback server running in background (5 min timeout). + After authenticating, confirm "done" and retry this command.` + : `Server "${serverName}" requires authentication. Run 'mcp-cli info ${serverName}' to start authentication.`; + super(message); this.name = 'AuthRequiredError'; + this.serverName = serverName; + this.authUrl = authUrl; } } @@ -119,6 +122,11 @@ function getRetryConfig(): RetryConfig { * Uses error codes when available, falls back to message matching */ export function isTransientError(error: Error): boolean { + // AuthRequiredError should never be retried - it indicates callback server is running + if (error.name === 'AuthRequiredError') { + return false; + } + // Check error code first (more reliable than message matching) const nodeError = error as NodeJS.ErrnoException; if (nodeError.code) { @@ -248,7 +256,6 @@ export async function connectToServer( config: ServerConfig, options: ConnectOptions = {}, ): Promise { - const { allowInteractiveAuth = true } = options; // Collect stderr for better error messages const stderrChunks: string[] = []; @@ -271,17 +278,19 @@ export async function connectToServer( transport = result.transport; authProvider = result.authProvider; - // Configure whether interactive auth is allowed - authProvider.setAllowInteractiveAuth(allowInteractiveAuth); - // Pre-start callback server for authorization_code flow to determine actual port // This ensures the redirect_uri in the authorization request uses the correct port const oauthConfig = (config as HttpServerConfig).oauth; - if (!oauthConfig?.grantType || oauthConfig.grantType === 'authorization_code') { + if ( + !oauthConfig?.grantType || + oauthConfig.grantType === 'authorization_code' + ) { try { await authProvider.preStartCallbackServer(); } catch (error) { - debug(`Failed to pre-start callback server: ${(error as Error).message}`); + debug( + `Failed to pre-start callback server: ${(error as Error).message}`, + ); // Continue anyway - server will start during redirectToAuthorization } } @@ -304,75 +313,24 @@ export async function connectToServer( try { await client.connect(transport); } catch (error) { - // Handle OAuth authorization required if (isOAuthNeeded(error as Error) && authProvider) { debug(`OAuth authorization required for ${serverName}`); - // If interactive auth was blocked by the provider, throw a clear error - if (authProvider.interactiveAuthBlocked) { - authProvider.cleanupCallbackServer(); - throw new AuthRequiredError(serverName); - } + // Always throw AuthRequiredError with auth URL - CLI is for AI agents + // Callback server continues running in background for 5 min + throw new AuthRequiredError( + serverName, + authProvider.capturedAuthUrl ?? undefined, + ); + } - // For authorization_code flow, wait for callback - const oauthConfig = (config as HttpServerConfig).oauth; - if ( - !oauthConfig?.grantType || - oauthConfig.grantType === 'authorization_code' - ) { - try { - console.error('Waiting for OAuth authorization...'); - - // Wait for the callback server that was started by redirectToAuthorization - const { code } = await authProvider.waitForCallback(); - debug('Authorization code received, completing OAuth flow'); - - // Complete the OAuth flow with the authorization code - await (transport as StreamableHTTPClientTransport).finishAuth(code); - - // Create new client and transport for authenticated connection - // (original transport is in started state and can't be reused) - debug('Creating new connection with authenticated tokens'); - const newClient = new Client( - { name: 'mcp-cli', version: VERSION }, - { capabilities: {} }, - ); - const newResult = createHttpTransport(serverName, config as HttpServerConfig); - await newClient.connect(newResult.transport); - - return { - client: newClient, - close: async () => { - await newClient.close(); - }, - }; - } catch (oauthError) { - throw new Error( - formatCliError( - oauthFlowError(serverName, (oauthError as Error).message), - ), - ); - } - } else { - // client_credentials should not need interactive flow - throw new Error( - formatCliError( - oauthFlowError( - serverName, - 'client_credentials authentication failed - check clientId and clientSecret', - ), - ), - ); - } - } else { - // Enhance error with captured stderr - const stderrOutput = stderrChunks.join('').trim(); - if (stderrOutput) { - const err = error as Error; - err.message = `${err.message}\n\nServer stderr:\n${stderrOutput}`; - } - throw error; + // Enhance error with captured stderr + const stderrOutput = stderrChunks.join('').trim(); + if (stderrOutput) { + const err = error as Error; + err.message = `${err.message}\n\nServer stderr:\n${stderrOutput}`; } + throw error; } // For successful connections, forward stderr to console diff --git a/src/commands/list.ts b/src/commands/list.ts index 71176aa..fd549d0 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -3,6 +3,7 @@ */ import { + AuthRequiredError, type McpConnection, type ToolInfo, debug, @@ -62,25 +63,33 @@ async function processWithConcurrency( /** * Fetch tools from a single server (uses daemon if enabled) - * When listing multiple servers, interactive OAuth is disabled to prevent chaos + * Never opens browser - CLI is used by AI agents, returns auth URL instead */ async function fetchServerTools( serverName: string, config: McpServersConfig, - allowInteractiveAuth = true, ): Promise { let connection: McpConnection | null = null; try { const serverConfig = getServerConfig(config, serverName); - connection = await getConnection(serverName, serverConfig, { - allowInteractiveAuth, - }); + connection = await getConnection(serverName, serverConfig); const tools = await connection.listTools(); const instructions = await connection.getInstructions(); debug(`${serverName}: loaded ${tools.length} tools`); return { name: serverName, tools, instructions }; } catch (error) { + // AuthRequiredError is caught and returned as a response, not retried + // This ensures the callback server stays running and the auth URL is shown + if (error instanceof AuthRequiredError) { + debug(`${serverName}: auth required - ${error.message}`); + return { + name: serverName, + tools: [], + error: error.message, + }; + } + const errorMsg = (error as Error).message; debug(`${serverName}: connection failed - ${errorMsg}`); return { @@ -123,11 +132,9 @@ export async function listCommand(options: ListOptions): Promise { ); // Process servers in parallel with concurrency limit - // Disable interactive OAuth when listing multiple servers to prevent chaos - const allowInteractiveAuth = serverNames.length === 1; const servers = await processWithConcurrency( serverNames, - (name) => fetchServerTools(name, config, allowInteractiveAuth), + (name) => fetchServerTools(name, config), concurrencyLimit, ); diff --git a/src/oauth/callback-server.ts b/src/oauth/callback-server.ts index 830a974..41136bb 100644 --- a/src/oauth/callback-server.ts +++ b/src/oauth/callback-server.ts @@ -214,11 +214,14 @@ export function startCallbackServer( debug( `OAuth callback server listening on http://localhost:${port}/callback`, ); - resolveStart({ - server: server!, - promise: callbackPromise, - cleanup, - }); + const httpServer = server; + if (httpServer) { + resolveStart({ + server: httpServer, + promise: callbackPromise, + cleanup, + }); + } }); }); } @@ -236,11 +239,14 @@ export async function startCallbackServerWithFallback( for (const port of portsToTry) { try { - debug(`Trying to start callback server on port ${port === 0 ? 'random' : port}`); + debug( + `Trying to start callback server on port ${port === 0 ? 'random' : port}`, + ); const state = await startCallbackServer(port, timeoutMs); // Get the actual port (important when port was 0 for random) const address = state.server.address(); - const actualPort = typeof address === 'object' && address ? address.port : port; + const actualPort = + typeof address === 'object' && address ? address.port : port; debug(`Callback server started on port ${actualPort}`); return { state, actualPort }; } catch (error) { diff --git a/src/oauth/provider.ts b/src/oauth/provider.ts index 69e66c8..68f995e 100644 --- a/src/oauth/provider.ts +++ b/src/oauth/provider.ts @@ -45,8 +45,8 @@ export class McpCliOAuthProvider implements OAuthClientProvider { // Actual port used by callback server (may differ from configured port due to fallback) private actualCallbackPort: number | null = null; - // Whether interactive auth (browser + callback) is allowed - private _allowInteractiveAuth = true; + // Captured authorization URL for non-interactive mode + private _capturedAuthUrl: string | null = null; constructor(serverName: string, serverUrl: string, oauthConfig: OAuthConfig) { this.serverName = serverName; @@ -72,7 +72,10 @@ export class McpCliOAuthProvider implements OAuthClientProvider { return undefined; } // Use actual port if server was pre-started, otherwise fall back to configured/default - const port = this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + const port = + this.actualCallbackPort ?? + this.oauthConfig.callbackPort ?? + DEFAULT_CALLBACK_PORT; // Omit port from URL when using standard HTTP port 80 if (port === 80) { return 'http://localhost/callback'; @@ -124,13 +127,18 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } // Otherwise, try to load from dynamic registration - const stored = readJsonFile(this.paths.client); + const stored = readJsonFile< + OAuthClientInformationMixed & { redirect_uris?: string[] } + >(this.paths.client); // Validate stored redirect_uris match current redirectUrl // If they don't match, the server will reject with "Invalid redirect_uri" if (stored?.redirect_uris && stored.redirect_uris.length > 0) { - const currentRedirectUrl = this.redirectUrl ? String(this.redirectUrl) : undefined; - const storedHasCurrentUrl = currentRedirectUrl && stored.redirect_uris.includes(currentRedirectUrl); + const currentRedirectUrl = this.redirectUrl + ? String(this.redirectUrl) + : undefined; + const storedHasCurrentUrl = + currentRedirectUrl && stored.redirect_uris.includes(currentRedirectUrl); if (!storedHasCurrentUrl) { debug( @@ -168,77 +176,35 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } /** - * Set whether interactive auth is allowed - * When disabled, redirectToAuthorization will not open browser or start server + * Get the captured authorization URL (for non-interactive mode) */ - setAllowInteractiveAuth(allow: boolean): void { - this._allowInteractiveAuth = allow; - } - - /** - * Check if interactive auth was blocked - */ - get interactiveAuthBlocked(): boolean { - return !this._allowInteractiveAuth; + get capturedAuthUrl(): string | null { + return this._capturedAuthUrl; } /** * Redirect to authorization URL - * If callback server was pre-started, reuses it; otherwise starts one with port fallback - * Opens the browser only after the server is ready + * Captures auth URL for AI agents - never opens browser + * Background callback server is already pre-started */ redirectToAuthorization(authorizationUrl: URL): void { - // If interactive auth is disabled, don't start server or open browser - if (!this._allowInteractiveAuth) { - debug( - `Interactive auth disabled for ${this.serverName}, skipping browser redirect`, + debug(`Capturing auth URL for ${this.serverName} (non-interactive mode)`); + + // Update auth URL with actual redirect_uri (from pre-started server) + const actualRedirectUri = this.redirectUrl; + if (actualRedirectUri) { + authorizationUrl.searchParams.set( + 'redirect_uri', + String(actualRedirectUri), ); - return; } - // Helper to open browser after server is ready - const openBrowserWithUrl = () => { - // Update authorization URL with actual redirect_uri (may have changed due to port fallback) - const actualRedirectUri = this.redirectUrl; - if (actualRedirectUri) { - authorizationUrl.searchParams.set('redirect_uri', String(actualRedirectUri)); - } + // Capture the auth URL for inclusion in error message + this._capturedAuthUrl = authorizationUrl.toString(); + debug(`Captured auth URL: ${this._capturedAuthUrl}`); - // Now open the browser - console.error(`\nAuthorizing ${this.serverName}...`); - console.error( - `If the browser doesn't open, visit: ${authorizationUrl.toString()}`, - ); - openBrowser(authorizationUrl.toString()).catch(() => { - // Error already logged in openBrowser - }); - }; - - // If callback server was already pre-started, just open the browser - if (this.callbackServerState && this.actualCallbackPort !== null) { - debug(`Reusing pre-started callback server on port ${this.actualCallbackPort} for ${this.serverName}`); - openBrowserWithUrl(); - return; - } - - // Otherwise, start the callback server BEFORE opening the browser - const portsToTry = this.getPortsToTry(); - this.callbackServerStarting = startCallbackServerWithFallback(portsToTry) - .then(({ state, actualPort }) => { - this.callbackServerState = state; - this.actualCallbackPort = actualPort; - this.callbackServerStarting = null; - debug(`Callback server ready for ${this.serverName} on port ${actualPort}`); - openBrowserWithUrl(); - }) - .catch((error) => { - this.callbackServerStarting = null; - console.error(`Failed to start callback server: ${error.message}`); - console.error( - `Please manually visit: ${authorizationUrl.toString()}`, - ); - throw error; - }); + // Background server is already pre-started (from preStartCallbackServer) + // It will continue running for the timeout period (default 5 min) } /** @@ -254,7 +220,8 @@ export class McpCliOAuthProvider implements OAuthClientProvider { if (!this.callbackServerState) { // Fallback: start server now if not already started (with port fallback) const portsToTry = this.getPortsToTry(); - const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + const { state, actualPort } = + await startCallbackServerWithFallback(portsToTry); this.callbackServerState = state; this.actualCallbackPort = actualPort; } @@ -269,7 +236,14 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } /** - * Clean up the callback server if running + * Clean up the callback server if running. + * + * Production code does NOT call this - the server self-cleans on: + * - Success: when authorization code is received + * - Error: when OAuth error is returned + * - Timeout: after 5 minute timeout expires + * + * This method exists for tests that start servers without completing OAuth. */ cleanupCallbackServer(): void { if (this.callbackServerState) { @@ -347,7 +321,11 @@ export class McpCliOAuthProvider implements OAuthClientProvider { * Get the callback port for this provider */ getCallbackPort(): number { - return this.actualCallbackPort ?? this.oauthConfig.callbackPort ?? DEFAULT_CALLBACK_PORT; + return ( + this.actualCallbackPort ?? + this.oauthConfig.callbackPort ?? + DEFAULT_CALLBACK_PORT + ); } /** @@ -359,7 +337,10 @@ export class McpCliOAuthProvider implements OAuthClientProvider { const ports: number[] = []; // If explicit callbackPorts array is configured, use it - if (this.oauthConfig.callbackPorts && this.oauthConfig.callbackPorts.length > 0) { + if ( + this.oauthConfig.callbackPorts && + this.oauthConfig.callbackPorts.length > 0 + ) { return [...this.oauthConfig.callbackPorts]; } @@ -395,17 +376,25 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } if (this.callbackServerStarting) { await this.callbackServerStarting; - return this.actualCallbackPort!; + // After awaiting, actualCallbackPort is guaranteed to be set + if (this.actualCallbackPort !== null) { + return this.actualCallbackPort; + } } const portsToTry = this.getPortsToTry(); - debug(`Pre-starting callback server for ${this.serverName}, trying ports: ${portsToTry.join(', ')}`); + debug( + `Pre-starting callback server for ${this.serverName}, trying ports: ${portsToTry.join(', ')}`, + ); - const { state, actualPort } = await startCallbackServerWithFallback(portsToTry); + const { state, actualPort } = + await startCallbackServerWithFallback(portsToTry); this.callbackServerState = state; this.actualCallbackPort = actualPort; - debug(`Callback server pre-started on port ${actualPort} for ${this.serverName}`); + debug( + `Callback server pre-started on port ${actualPort} for ${this.serverName}`, + ); return actualPort; } } diff --git a/src/oauth/types.ts b/src/oauth/types.ts index 251b04f..ad38a40 100644 --- a/src/oauth/types.ts +++ b/src/oauth/types.ts @@ -36,5 +36,5 @@ export interface CallbackServerState { // Default callback port for OAuth redirect export const DEFAULT_CALLBACK_PORT = 8095; -// Default port fallback order: 80 (standard), common alternatives, then random -export const DEFAULT_PORT_FALLBACK_ORDER = [80, 8080, 3000, 8095, 0]; // 0 = random port +// Default port: random (0) - OS assigns unique port for each server, avoiding conflicts +export const DEFAULT_PORT_FALLBACK_ORDER = [0]; diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index 0ca0c5e..413c303 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -391,19 +391,17 @@ describe('oauth', () => { const provider = new McpCliOAuthProvider('test', 'https://example.com', config); const ports = provider.getPortsToTry(); - // Default order: 80, 8080, 3000, 8095, 0 (random) - expect(ports).toEqual([80, 8080, 3000, 8095, 0]); + // Default: always use random port (0) - OS assigns unique port to avoid conflicts + expect(ports).toEqual([0]); }); - test('puts configured callbackPort first in fallback order', () => { + test('puts configured callbackPort first with random fallback', () => { const config: OAuthConfig = { callbackPort: 9000 }; const provider = new McpCliOAuthProvider('test', 'https://example.com', config); const ports = provider.getPortsToTry(); - expect(ports[0]).toBe(9000); - // Rest of default order follows (excluding duplicates) - expect(ports).toContain(80); - expect(ports).toContain(8080); + // Configured port first, then random (0) as fallback + expect(ports).toEqual([9000, 0]); }); test('uses callbackPorts array when configured', () => { From 91d499ef8976d7296238db19f2d8558e8bc95d3c Mon Sep 17 00:00:00 2001 From: Namig Gadirov Date: Tue, 14 Jul 2026 22:23:50 +0400 Subject: [PATCH 7/9] fix(oauth): keep process alive to complete auth, wire up token exchange Two bugs prevented the non-blocking OAuth flow from ever completing: 1. Command handlers (info, call) and the top-level main() always called process.exit() right after reporting AuthRequiredError. This killed the whole process - including the in-process HTTP callback server - before the user had any chance to open the auth URL, let alone finish the redirect. Result: "connection refused" on the callback URL. 2. Nothing ever consumed the authorization code once the callback server captured it. McpCliOAuthProvider.waitForCallback() existed but was dead code - no caller awaited it or exchanged the code for tokens, so even a successful redirect would never persist a token. Fix: - client.ts now kicks off a fire-and-forget chain after throwing AuthRequiredError: waitForCallback() then SDK auth() with the received authorizationCode, then tokens saved via the provider. - New oauth/pending.ts tracks these in-flight completions so command handlers and main() can check hasPendingOAuth() before force-exiting, letting the event loop stay alive (kept open by the callback server's listening socket) until the flow finishes or the 5 minute timeout elapses. - info.ts / call.ts / index.ts updated to skip process.exit() when an OAuth completion is still pending, setting process.exitCode instead so the eventual natural exit still reports the right status. Manually verified end-to-end against mcp.atlassian.com: auth URL, browser login, callback received, code exchanged, token saved, mcp-cli info atlassian (31 tools) and a real tool call both succeed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/client.ts | 34 +++++++++++++++++++++++++++++++++- src/commands/call.ts | 9 +++++++++ src/commands/info.ts | 9 +++++++++ src/index.ts | 12 ++++++++++++ src/oauth/pending.ts | 26 ++++++++++++++++++++++++++ 5 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 src/oauth/pending.ts diff --git a/src/client.ts b/src/client.ts index 1499466..96ed6fa 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2,7 +2,10 @@ * MCP Client - Connection management for MCP servers */ -import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'; +import { + UnauthorizedError, + auth, +} from '@modelcontextprotocol/sdk/client/auth.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; @@ -28,6 +31,7 @@ import { } from './daemon-client.js'; import { formatCliError, oauthFlowError } from './errors.js'; import { McpCliOAuthProvider } from './oauth/index.js'; +import { registerPendingOAuth } from './oauth/pending.js'; import { VERSION } from './version.js'; // Re-export config utilities for convenience @@ -316,6 +320,34 @@ export async function connectToServer( if (isOAuthNeeded(error as Error) && authProvider) { debug(`OAuth authorization required for ${serverName}`); + // Fire-and-forget: finish the OAuth flow once the user completes + // authentication in the browser. This is what actually consumes the + // callback server's captured code and exchanges it for tokens - + // without it, the callback server would receive the redirect but + // nothing would ever save the resulting tokens. + // Registered so command handlers know to keep the process alive + // (the open callback server socket does this naturally) instead of + // force-exiting right after this error is reported. + const serverUrl = (config as HttpServerConfig).url; + const completion = authProvider + .waitForCallback() + .then(async (result) => { + debug( + `Authorization code received for ${serverName}, exchanging for tokens`, + ); + await auth(authProvider as McpCliOAuthProvider, { + serverUrl, + authorizationCode: result.code, + }); + debug(`OAuth flow completed for ${serverName} - tokens saved`); + }) + .catch((completionError) => { + debug( + `OAuth flow failed for ${serverName}: ${(completionError as Error).message}`, + ); + }); + registerPendingOAuth(completion); + // Always throw AuthRequiredError with auth URL - CLI is for AI agents // Callback server continues running in background for 5 min throw new AuthRequiredError( diff --git a/src/commands/call.ts b/src/commands/call.ts index 9edcaad..29a7ddb 100644 --- a/src/commands/call.ts +++ b/src/commands/call.ts @@ -29,6 +29,7 @@ import { toolExecutionError, toolNotFoundError, } from '../errors.js'; +import { hasPendingOAuth } from '../oauth/pending.js'; import { formatJson } from '../output.js'; export interface CallOptions { @@ -155,6 +156,14 @@ export async function callCommand(options: CallOptions): Promise { serverConnectionError(serverName, (error as Error).message), ), ); + if (hasPendingOAuth()) { + // Don't force-exit - a background callback server is still waiting + // for the OAuth redirect. Letting the event loop run naturally (kept + // alive by the server's open socket) allows the token exchange to + // complete once the user finishes authenticating. + process.exitCode = ErrorCode.NETWORK_ERROR; + return; + } process.exit(ErrorCode.NETWORK_ERROR); } diff --git a/src/commands/info.ts b/src/commands/info.ts index e88bea6..5d3318b 100644 --- a/src/commands/info.ts +++ b/src/commands/info.ts @@ -15,6 +15,7 @@ import { serverConnectionError, toolNotFoundError, } from '../errors.js'; +import { hasPendingOAuth } from '../oauth/pending.js'; import { formatServerDetails, formatToolSchema } from '../output.js'; export interface InfoOptions { @@ -67,6 +68,14 @@ export async function infoCommand(options: InfoOptions): Promise { serverConnectionError(serverName, (error as Error).message), ), ); + if (hasPendingOAuth()) { + // Don't force-exit - a background callback server is still waiting + // for the OAuth redirect. Letting the event loop run naturally (kept + // alive by the server's open socket) allows the token exchange to + // complete once the user finishes authenticating. + process.exitCode = ErrorCode.NETWORK_ERROR; + return; + } process.exit(ErrorCode.NETWORK_ERROR); } diff --git a/src/index.ts b/src/index.ts index 088deed..be7c3ab 100755 --- a/src/index.ts +++ b/src/index.ts @@ -32,6 +32,7 @@ import { unknownOptionError, unknownSubcommandError, } from './errors.js'; +import { hasPendingOAuth } from './oauth/pending.js'; import { VERSION } from './version.js'; interface ParsedArgs { @@ -464,11 +465,22 @@ process.on('SIGTERM', () => { // Run main() .then(() => { + if (hasPendingOAuth()) { + // A background OAuth callback server is still waiting for the user to + // finish authenticating in the browser. Don't force-exit - let the + // event loop stay alive (kept open by the server's listening socket) + // until the flow completes or its 5 minute timeout elapses. + return; + } // Use setImmediate to let stdout flush before exiting setImmediate(() => process.exit(0)); }) .catch((error) => { // Error message already formatted by command handlers console.error(error.message); + if (hasPendingOAuth()) { + process.exitCode = ErrorCode.CLIENT_ERROR; + return; + } setImmediate(() => process.exit(ErrorCode.CLIENT_ERROR)); }); diff --git a/src/oauth/pending.ts b/src/oauth/pending.ts new file mode 100644 index 0000000..96e13e3 --- /dev/null +++ b/src/oauth/pending.ts @@ -0,0 +1,26 @@ +/** + * Tracks in-flight OAuth completions. + * + * When a server requires authorization, the CLI returns an auth URL to the + * caller immediately (so AI agents aren't blocked) while a background + * callback server keeps listening for the redirect. That listener - and the + * token exchange that follows once the code arrives - runs as a dangling + * promise in the same process. If the CLI force-exits right after printing + * the auth URL, that promise (and the callback server's open socket) is + * killed before the user can even open the link. + * + * Command handlers must check `hasPendingOAuth()` before calling + * `process.exit()` so the event loop is allowed to stay alive - kept open by + * the callback server's listening socket - until the flow completes or the + * 5 minute timeout elapses. + */ +const pending = new Set>(); + +export function registerPendingOAuth(promise: Promise): void { + pending.add(promise); + promise.finally(() => pending.delete(promise)); +} + +export function hasPendingOAuth(): boolean { + return pending.size > 0; +} From b5225681e3aba220768a44c648fa0521916ec21e Mon Sep 17 00:00:00 2001 From: Namig Gadirov Date: Tue, 14 Jul 2026 22:51:48 +0400 Subject: [PATCH 8/9] feat: auto-open OAuth authorization URL in default browser Previously the auth provider only captured the authorization URL for AI agents to relay to the user, requiring a manual copy/paste into the browser. This now automatically launches the URL in the system default browser (open/xdg-open/start) as soon as it's ready, while still capturing/printing it for agents and as a fallback if the browser can't be launched. - Add oauth.autoOpenBrowser config option (default: true) to opt out for headless/CI environments - Wire config through config.ts and oauth/types.ts - Update AuthRequiredError message to reflect auto-open behavior - Add tests covering auto-open default, opt-out, and URL capture - Update README and CHANGELOG Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 ++++ README.md | 18 ++++++++++++++- src/client.ts | 3 ++- src/config.ts | 21 +++++++++++++++++ src/oauth/provider.ts | 14 ++++++++++- src/oauth/types.ts | 8 +++++++ tests/oauth.test.ts | 54 ++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 118 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b04f885..6a59fe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ### Added +- **Automatic Browser Launch for OAuth** + - The authorization URL is now opened automatically in the system's default browser (`open`/`xdg-open`/`start`) as soon as it's ready + - New `oauth.autoOpenBrowser` config option (default `true`) to disable this for headless/CI environments + - URL is still captured/printed as before, so AI agents can surface it if the browser can't be opened automatically - **Non-Blocking OAuth Flow for AI Agents** - `AuthRequiredError` now includes authorization URL for immediate action - Callback server runs in background (5 min timeout) - CLI returns immediately diff --git a/README.md b/README.md index 624852b..9d1054e 100644 --- a/README.md +++ b/README.md @@ -350,8 +350,8 @@ sequenceDiagram AI->>CLI: mcp-cli CLI->>CLI: Detect some servers need auth CLI->>Server: Spawn background callback server + CLI->>User: Auto-opens auth URL in default browser CLI-->>AI: List working servers + auth URL for others - AI->>User: "Please authenticate via this link" User->>Server: Completes OAuth in browser Server->>Server: Saves tokens to ~/.mcp-cli/tokens/ User->>AI: "Done" @@ -359,6 +359,11 @@ sequenceDiagram CLI-->>AI: All servers now listed ``` +The authorization URL is automatically opened in the system's default +browser (`open` on macOS, `xdg-open` on Linux, `start` on Windows) as soon +as it's ready, so you usually don't need to copy/paste anything - just +complete the login/consent screen that pops up. + **Configuration:** Most OAuth-enabled servers work with just a URL - the CLI handles dynamic client registration and PKCE automatically: @@ -401,6 +406,7 @@ For servers requiring custom OAuth settings, use the `oauth` object: | `clientSecret` | OAuth client secret (for confidential clients) | (none) | | `scope` | OAuth scopes to request | (none) | | `grantType` | `authorization_code` or `client_credentials` | `authorization_code` | +| `autoOpenBrowser` | Automatically open the auth URL in the default browser | `true` | **Examples by scenario:** @@ -435,6 +441,16 @@ For servers requiring custom OAuth settings, use the `oauth` object: } ``` +4. **Headless/CI environment (disable auto-open browser):** +```json +{ + "notion": { + "url": "https://mcp.notion.com/mcp", + "oauth": { "autoOpenBrowser": false } + } +} +``` + **Token Storage:** Tokens are persisted in `~/.mcp-cli/` with secure file permissions (0600): diff --git a/src/client.ts b/src/client.ts index 96ed6fa..0bc9eda 100644 --- a/src/client.ts +++ b/src/client.ts @@ -69,7 +69,8 @@ export class AuthRequiredError extends Error { constructor(serverName: string, authUrl?: string) { const message = authUrl ? `[AUTH REQUIRED] ${serverName} - Authenticate at: ${authUrl} + Opening browser to authenticate: ${authUrl} + (If it doesn't open automatically, copy/paste the URL above.) Callback server running in background (5 min timeout). After authenticating, confirm "done" and retry this command.` : `Server "${serverName}" requires authentication. Run 'mcp-cli info ${serverName}' to start authentication.`; diff --git a/src/config.ts b/src/config.ts index 4dc152d..ebb752f 100644 --- a/src/config.ts +++ b/src/config.ts @@ -52,6 +52,11 @@ export interface OAuthConfig { callbackPort?: number; /** Optional: explicit list of ports to try in order (overrides default fallback) */ callbackPorts?: number[]; + /** + * Whether to automatically open the authorization URL in the system's + * default browser once it is ready. Defaults to `true`. + */ + autoOpenBrowser?: boolean; } /** @@ -569,6 +574,22 @@ export async function loadConfig( ); } } + + // Validate autoOpenBrowser if specified + if ( + oauth.autoOpenBrowser !== undefined && + typeof oauth.autoOpenBrowser !== 'boolean' + ) { + throw new Error( + formatCliError({ + code: ErrorCode.CLIENT_ERROR, + type: 'CONFIG_INVALID_OAUTH', + message: `Invalid autoOpenBrowser for server "${serverName}"`, + details: 'autoOpenBrowser must be a boolean', + suggestion: `Use "autoOpenBrowser": false to disable automatic browser launch`, + }), + ); + } } } diff --git a/src/oauth/provider.ts b/src/oauth/provider.ts index 68f995e..a44fc19 100644 --- a/src/oauth/provider.ts +++ b/src/oauth/provider.ts @@ -184,7 +184,9 @@ export class McpCliOAuthProvider implements OAuthClientProvider { /** * Redirect to authorization URL - * Captures auth URL for AI agents - never opens browser + * Captures auth URL for AI agents and automatically opens it in the + * system's default browser so a human can complete the login without + * having to copy/paste the link manually. * Background callback server is already pre-started */ redirectToAuthorization(authorizationUrl: URL): void { @@ -205,6 +207,16 @@ export class McpCliOAuthProvider implements OAuthClientProvider { // Background server is already pre-started (from preStartCallbackServer) // It will continue running for the timeout period (default 5 min) + + // Auto-launch the default browser unless explicitly disabled. + // Fire-and-forget: openBrowser() already logs/handles failures + // internally (falling back to printing the URL), so we never block + // or throw here - the captured URL remains available regardless. + if (this.oauthConfig.autoOpenBrowser !== false) { + openBrowser(this._capturedAuthUrl).catch(() => { + // Failure already logged by openBrowser; nothing else to do. + }); + } } /** diff --git a/src/oauth/types.ts b/src/oauth/types.ts index ad38a40..49953a1 100644 --- a/src/oauth/types.ts +++ b/src/oauth/types.ts @@ -15,6 +15,14 @@ export interface OAuthConfig { callbackPort?: number; /** Optional: explicit list of ports to try in order (overrides default fallback) */ callbackPorts?: number[]; + /** + * Whether to automatically open the authorization URL in the system's + * default browser once it is ready. Defaults to `true`. Set to `false` + * to keep the previous non-interactive behavior (URL is only captured/ + * printed, never opened automatically) - useful for headless/CI/agent + * environments without a usable display. + */ + autoOpenBrowser?: boolean; } /** diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index 413c303..3db5d53 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -2,7 +2,7 @@ * Unit tests for OAuth module */ -import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { describe, test, expect, beforeEach, afterEach, spyOn } from 'bun:test'; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -438,6 +438,58 @@ describe('oauth', () => { }); }); + describe('redirectToAuthorization', () => { + test('captures the authorization URL', () => { + const config: OAuthConfig = { callbackPort: 8123, autoOpenBrowser: false }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.redirectToAuthorization( + new URL('https://example.com/authorize?client_id=abc'), + ); + + expect(provider.capturedAuthUrl).not.toBeNull(); + expect(provider.capturedAuthUrl).toContain('https://example.com/authorize'); + expect(provider.capturedAuthUrl).toContain('redirect_uri='); + }); + + test('opens the browser automatically by default', async () => { + const openBrowserModule = await import('../src/oauth/browser'); + const openBrowserSpy = spyOn(openBrowserModule, 'openBrowser').mockResolvedValue( + undefined, + ); + + const config: OAuthConfig = { callbackPort: 8124 }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.redirectToAuthorization( + new URL('https://example.com/authorize?client_id=abc'), + ); + + expect(openBrowserSpy).toHaveBeenCalledTimes(1); + expect(openBrowserSpy).toHaveBeenCalledWith(provider.capturedAuthUrl); + + openBrowserSpy.mockRestore(); + }); + + test('does not open the browser when autoOpenBrowser is false', async () => { + const openBrowserModule = await import('../src/oauth/browser'); + const openBrowserSpy = spyOn(openBrowserModule, 'openBrowser').mockResolvedValue( + undefined, + ); + + const config: OAuthConfig = { callbackPort: 8125, autoOpenBrowser: false }; + const provider = new McpCliOAuthProvider('test', 'https://example.com', config); + + provider.redirectToAuthorization( + new URL('https://example.com/authorize?client_id=abc'), + ); + + expect(openBrowserSpy).not.toHaveBeenCalled(); + + openBrowserSpy.mockRestore(); + }); + }); + describe('preStartCallbackServer', () => { test('returns 0 for client_credentials flow', async () => { const config: OAuthConfig = { From 6a0a0eee9bb12fbe6a0f02bce25701c3c297b842 Mon Sep 17 00:00:00 2001 From: Namig Gadirov Date: Wed, 15 Jul 2026 08:25:58 +0400 Subject: [PATCH 9/9] feat(oauth): store tokens in macOS Keychain instead of plaintext files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OAuth access/refresh tokens are the most sensitive artifacts mcp-cli persists. Previously they were written to plaintext JSON files under ~/.mcp-cli/tokens/ (0600 permissions), readable by anything running as the same OS user — a real risk given how many npx/bun packages get executed in typical dev workflows. On macOS, tokens are now stored in the user's login Keychain via the `security` CLI, giving OS-level encryption at rest and Keychain ACLs. Non-macOS platforms fall back to the existing file-based storage unchanged. - Add src/oauth/keychain.ts: thin wrapper around `security` find/add/delete-generic-password, gated by isKeychainSupported() (darwin-only; disableable via MCP_CLI_DISABLE_KEYCHAIN=1 for tests) - provider.ts: tokens()/saveTokens() prefer Keychain, with automatic one-time migration of legacy plaintext token files into the Keychain; invalidateCredentials() also purges the Keychain entry - Add tests/keychain.test.ts with fully mocked `security` calls (no real Keychain access, safe on any platform/CI) - Fix tests/oauth.test.ts to set MCP_CLI_DISABLE_KEYCHAIN=1 in beforeEach/afterEach — these tests were previously writing real secrets into the developer's login Keychain with no cleanup Verified: typecheck, lint, full test suite (248/248), build, and a live end-to-end run against the Atlassian MCP server confirming automatic migration of an existing plaintext token into the Keychain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/oauth/keychain.ts | 111 ++++++++++++++++++++++++++++ src/oauth/provider.ts | 54 +++++++++++++- tests/keychain.test.ts | 162 +++++++++++++++++++++++++++++++++++++++++ tests/oauth.test.ts | 52 +++++++++++++ 4 files changed, 377 insertions(+), 2 deletions(-) create mode 100644 src/oauth/keychain.ts create mode 100644 tests/keychain.test.ts diff --git a/src/oauth/keychain.ts b/src/oauth/keychain.ts new file mode 100644 index 0000000..20a67aa --- /dev/null +++ b/src/oauth/keychain.ts @@ -0,0 +1,111 @@ +/** + * macOS Keychain-backed secret storage. + * + * OAuth access/refresh tokens are the most sensitive artifacts mcp-cli + * persists. On macOS we store them in the user's login Keychain (via the + * `security` CLI) instead of a plaintext JSON file, so they benefit from + * OS-level encryption at rest and Keychain ACLs. On non-macOS platforms + * (or if the `security` binary is unavailable) callers should fall back to + * the existing file-based storage in storage.ts. + */ + +import { execFileSync } from 'node:child_process'; +import { debug } from '../config.js'; + +const SERVICE = 'mcp-cli-oauth-tokens'; + +/** Keychain "account" field — one entry per MCP server. */ +function accountFor(serverName: string): string { + return serverName; +} + +/** + * Whether this platform can use macOS Keychain storage. + * Checked lazily (not cached) so tests can stub `process.platform`. + * + * Set MCP_CLI_DISABLE_KEYCHAIN=1 to force file-based storage even on macOS — + * used by the test suite so it never touches the developer's real login + * Keychain (mirrors how MCP_CLI_HOME isolates file storage in tests). + */ +export function isKeychainSupported(): boolean { + if (process.env.MCP_CLI_DISABLE_KEYCHAIN === '1') return false; + return process.platform === 'darwin'; +} + +/** + * Read a secret from the macOS Keychain. Returns undefined if not + * supported, not found, or the `security` call fails for any reason. + */ +export function keychainRead(serverName: string): string | undefined { + if (!isKeychainSupported()) return undefined; + try { + const out = execFileSync( + 'security', + [ + 'find-generic-password', + '-a', + accountFor(serverName), + '-s', + SERVICE, + '-w', + ], + { stdio: ['ignore', 'pipe', 'ignore'] }, + ); + const value = out.toString('utf-8').trim(); + return value.length > 0 ? value : undefined; + } catch (error) { + debug( + `Keychain read miss for "${serverName}": ${(error as Error).message}`, + ); + return undefined; + } +} + +/** + * Write (or overwrite) a secret in the macOS Keychain. + * Returns true on success, false if unsupported or the write failed — + * callers should fall back to file storage in that case. + */ +export function keychainWrite(serverName: string, secret: string): boolean { + if (!isKeychainSupported()) return false; + try { + execFileSync( + 'security', + [ + 'add-generic-password', + '-a', + accountFor(serverName), + '-s', + SERVICE, + '-w', + secret, + '-U', // update in place if an entry already exists + ], + { stdio: ['ignore', 'ignore', 'ignore'] }, + ); + return true; + } catch (error) { + debug( + `Keychain write failed for "${serverName}": ${(error as Error).message}`, + ); + return false; + } +} + +/** + * Delete a secret from the macOS Keychain. No-op if unsupported or missing. + */ +export function keychainDelete(serverName: string): void { + if (!isKeychainSupported()) return; + try { + execFileSync( + 'security', + ['delete-generic-password', '-a', accountFor(serverName), '-s', SERVICE], + { stdio: ['ignore', 'ignore', 'ignore'] }, + ); + } catch (error) { + debug( + `Keychain delete miss for "${serverName}": ${(error as Error).message}`, + ); + } +} diff --git a/src/oauth/provider.ts b/src/oauth/provider.ts index a44fc19..d3f525a 100644 --- a/src/oauth/provider.ts +++ b/src/oauth/provider.ts @@ -14,6 +14,12 @@ import type { import { debug } from '../config.js'; import { openBrowser } from './browser.js'; import { startCallbackServerWithFallback } from './callback-server.js'; +import { + isKeychainSupported, + keychainDelete, + keychainRead, + keychainWrite, +} from './keychain.js'; import { deleteFile, ensureDir, @@ -161,17 +167,59 @@ export class McpCliOAuthProvider implements OAuthClientProvider { } /** - * Load OAuth tokens + * Load OAuth tokens. + * + * On macOS, tokens live in the login Keychain (encrypted at rest, OS-gated + * access) rather than a plaintext file. If a legacy plaintext token file + * exists from a previous version, it is transparently migrated into the + * Keychain and then removed from disk. */ tokens(): OAuthTokens | undefined { + if (isKeychainSupported()) { + const raw = keychainRead(this.serverName); + if (raw) { + try { + return JSON.parse(raw) as OAuthTokens; + } catch (error) { + debug( + `Failed to parse Keychain tokens for ${this.serverName}: ${(error as Error).message}`, + ); + } + } + + // Migrate any legacy plaintext token file into the Keychain. + const legacy = readJsonFile(this.paths.tokens); + if (legacy) { + debug( + `Migrating plaintext tokens for ${this.serverName} into macOS Keychain`, + ); + if (keychainWrite(this.serverName, JSON.stringify(legacy))) { + deleteFile(this.paths.tokens); + } + return legacy; + } + return undefined; + } + return readJsonFile(this.paths.tokens); } /** - * Save OAuth tokens + * Save OAuth tokens. + * + * Prefers the macOS Keychain; falls back to the plaintext token file if + * Keychain storage is unavailable or the `security` call fails. */ saveTokens(tokens: OAuthTokens): void { debug(`Saving tokens for ${this.serverName}`); + if ( + isKeychainSupported() && + keychainWrite(this.serverName, JSON.stringify(tokens)) + ) { + // Ensure no stale plaintext copy is left behind on disk. + deleteFile(this.paths.tokens); + return; + } writeJsonFile(this.paths.tokens, tokens); } @@ -294,6 +342,7 @@ export class McpCliOAuthProvider implements OAuthClientProvider { switch (scope) { case 'all': deleteFile(this.paths.tokens); + keychainDelete(this.serverName); deleteFile(this.paths.client); deleteFile(this.paths.verifier); break; @@ -302,6 +351,7 @@ export class McpCliOAuthProvider implements OAuthClientProvider { break; case 'tokens': deleteFile(this.paths.tokens); + keychainDelete(this.serverName); break; case 'verifier': deleteFile(this.paths.verifier); diff --git a/tests/keychain.test.ts b/tests/keychain.test.ts new file mode 100644 index 0000000..25ebda8 --- /dev/null +++ b/tests/keychain.test.ts @@ -0,0 +1,162 @@ +/** + * Unit tests for macOS Keychain-backed token storage. + * + * All `security` CLI calls are mocked via spyOn — these tests never touch + * the developer's real login Keychain, and run the same on any platform/CI. + */ + +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'; +import * as childProcess from 'node:child_process'; +import { + isKeychainSupported, + keychainDelete, + keychainRead, + keychainWrite, +} from '../src/oauth/keychain'; + +describe('keychain', () => { + const originalPlatform = process.platform; + + function setPlatform(platform: NodeJS.Platform) { + Object.defineProperty(process, 'platform', { value: platform }); + } + + afterEach(() => { + setPlatform(originalPlatform); + delete process.env.MCP_CLI_DISABLE_KEYCHAIN; + }); + + describe('isKeychainSupported', () => { + test('returns true on darwin', () => { + setPlatform('darwin'); + expect(isKeychainSupported()).toBe(true); + }); + + test('returns false on non-darwin platforms', () => { + setPlatform('linux'); + expect(isKeychainSupported()).toBe(false); + }); + + test('returns false when MCP_CLI_DISABLE_KEYCHAIN=1, even on darwin', () => { + setPlatform('darwin'); + process.env.MCP_CLI_DISABLE_KEYCHAIN = '1'; + expect(isKeychainSupported()).toBe(false); + }); + }); + + describe('with mocked `security` CLI (darwin)', () => { + beforeEach(() => { + setPlatform('darwin'); + }); + + test('keychainRead returns the stored secret', () => { + const spy = spyOn(childProcess, 'execFileSync').mockReturnValue( + Buffer.from('super-secret-token\n'), + ); + + const result = keychainRead('my-server'); + + expect(result).toBe('super-secret-token'); + expect(spy).toHaveBeenCalledWith( + 'security', + ['find-generic-password', '-a', 'my-server', '-s', 'mcp-cli-oauth-tokens', '-w'], + expect.anything(), + ); + spy.mockRestore(); + }); + + test('keychainRead returns undefined when item not found', () => { + const spy = spyOn(childProcess, 'execFileSync').mockImplementation(() => { + throw new Error('SecKeychainSearchCopyNext: The specified item could not be found'); + }); + + expect(keychainRead('missing-server')).toBeUndefined(); + spy.mockRestore(); + }); + + test('keychainWrite invokes add-generic-password with -U and returns true', () => { + const spy = spyOn(childProcess, 'execFileSync').mockReturnValue(Buffer.from('')); + + const result = keychainWrite('my-server', '{"access_token":"abc"}'); + + expect(result).toBe(true); + expect(spy).toHaveBeenCalledWith( + 'security', + [ + 'add-generic-password', + '-a', + 'my-server', + '-s', + 'mcp-cli-oauth-tokens', + '-w', + '{"access_token":"abc"}', + '-U', + ], + expect.anything(), + ); + spy.mockRestore(); + }); + + test('keychainWrite returns false when `security` call fails', () => { + const spy = spyOn(childProcess, 'execFileSync').mockImplementation(() => { + throw new Error('security: command failed'); + }); + + expect(keychainWrite('my-server', 'secret')).toBe(false); + spy.mockRestore(); + }); + + test('keychainDelete invokes delete-generic-password', () => { + const spy = spyOn(childProcess, 'execFileSync').mockReturnValue(Buffer.from('')); + + keychainDelete('my-server'); + + expect(spy).toHaveBeenCalledWith( + 'security', + ['delete-generic-password', '-a', 'my-server', '-s', 'mcp-cli-oauth-tokens'], + expect.anything(), + ); + spy.mockRestore(); + }); + + test('keychainDelete is a no-op (does not throw) when item is missing', () => { + const spy = spyOn(childProcess, 'execFileSync').mockImplementation(() => { + throw new Error('item not found'); + }); + + expect(() => keychainDelete('missing-server')).not.toThrow(); + spy.mockRestore(); + }); + }); + + describe('on non-darwin platforms', () => { + beforeEach(() => { + setPlatform('linux'); + }); + + test('keychainRead is a no-op returning undefined without calling `security`', () => { + const spy = spyOn(childProcess, 'execFileSync'); + + expect(keychainRead('my-server')).toBeUndefined(); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + test('keychainWrite is a no-op returning false without calling `security`', () => { + const spy = spyOn(childProcess, 'execFileSync'); + + expect(keychainWrite('my-server', 'secret')).toBe(false); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + test('keychainDelete is a no-op without calling `security`', () => { + const spy = spyOn(childProcess, 'execFileSync'); + + keychainDelete('my-server'); + + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + }); +}); diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index 3db5d53..a9bf329 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -12,6 +12,7 @@ describe('oauth', () => { // Use a unique temp directory for each test run to avoid conflicts let testDir: string; const originalMcpCliHome = process.env.MCP_CLI_HOME; + const originalDisableKeychain = process.env.MCP_CLI_DISABLE_KEYCHAIN; beforeEach(() => { // Create unique test directory for each test @@ -19,6 +20,9 @@ describe('oauth', () => { mkdirSync(testDir, { recursive: true }); // Use MCP_CLI_HOME env var to override storage location process.env.MCP_CLI_HOME = testDir; + // Force file-based token storage so these tests never touch the real + // macOS Keychain (Keychain-specific behavior is covered in keychain.test.ts) + process.env.MCP_CLI_DISABLE_KEYCHAIN = '1'; }); afterEach(() => { @@ -29,6 +33,13 @@ describe('oauth', () => { process.env.MCP_CLI_HOME = originalMcpCliHome; } + // Restore original MCP_CLI_DISABLE_KEYCHAIN + if (originalDisableKeychain === undefined) { + delete process.env.MCP_CLI_DISABLE_KEYCHAIN; + } else { + process.env.MCP_CLI_DISABLE_KEYCHAIN = originalDisableKeychain; + } + // Clean up test directory try { rmSync(testDir, { recursive: true, force: true }); @@ -266,6 +277,47 @@ describe('oauth', () => { const loaded = provider2.tokens(); expect(loaded?.access_token).toBe('persisted-token'); }); + + test('prefers Keychain storage on macOS and migrates legacy plaintext files', async () => { + const originalDisable = process.env.MCP_CLI_DISABLE_KEYCHAIN; + delete process.env.MCP_CLI_DISABLE_KEYCHAIN; + + const store = new Map(); + const keychain = await import('../src/oauth/keychain'); + const readSpy = spyOn(keychain, 'keychainRead').mockImplementation((server: string) => + store.get(server), + ); + const writeSpy = spyOn(keychain, 'keychainWrite').mockImplementation( + (server: string, secret: string) => { + store.set(server, secret); + return true; + }, + ); + const supportedSpy = spyOn(keychain, 'isKeychainSupported').mockReturnValue(true); + + try { + const config: OAuthConfig = {}; + const provider = new McpCliOAuthProvider('kc-test', 'https://example.com', config); + + provider.saveTokens({ access_token: 'kc-token', token_type: 'Bearer' }); + + // Token should have gone to the (mocked) Keychain, not a plaintext file. + expect(store.get('kc-test')).toContain('kc-token'); + expect(existsSync(provider['paths'].tokens)).toBe(false); + + const loaded = provider.tokens(); + expect(loaded?.access_token).toBe('kc-token'); + } finally { + readSpy.mockRestore(); + writeSpy.mockRestore(); + supportedSpy.mockRestore(); + if (originalDisable === undefined) { + delete process.env.MCP_CLI_DISABLE_KEYCHAIN; + } else { + process.env.MCP_CLI_DISABLE_KEYCHAIN = originalDisable; + } + } + }); }); describe('codeVerifier', () => {