From 51efc68b2cc0ff10e4c53a2068769d8e3e2621be Mon Sep 17 00:00:00 2001 From: Nowaker Date: Sat, 29 Aug 2026 02:18:09 -0500 Subject: [PATCH] feat(auth): make browser OAuth user-controlled The first OAuth method opens whatever browser the OS has registered as default. On a machine already signed into a different ChatGPT account that is the wrong browser, and there was no way to choose another: the URL was never printed, so it could not be opened elsewhere. Manual URL Paste was the documented escape hatch, and it rejected a pasted raw authorization code, leaving no working path at all. Add `Codex OAuth (Open URL Manually)` as a first-class method. It binds the loopback listener first, prints the authorize URL instead of opening anything, and completes through localhost exactly like the default-browser method - the two now share one session primitive rather than each owning a private listener. Manual URL Paste required a state parameter unconditionally, which is why a raw code pasted on its own was rejected. What is required is now decided by what the input is. A full callback URL, a bare query or a fragment must carry a matching non-empty state, because that input reproduces the callback's own parameters and a missing one means the paste did not come from this attempt. A raw code is accepted without state, since it carries none to compare and PKCE already binds it to the attempt that produced it. The parser reports which of those forms it saw, so the two cases cannot be mistaken for one another. Two lifecycle defects are fixed alongside, because the shared primitive is where they live: - Callback observation now starts when the session is created rather than when the host invokes its callback, so an authorization the host abandons after taking the URL still releases port 1455 on the listener's existing five-minute deadline instead of pinning it for the lifetime of the process. The observation is normalized so it cannot reject, since an unobserved rejection in the window before the callback is awaited would be an unhandled rejection. - A default browser that fails to launch previously left the user waiting on a page that never opened. `openBrowserUrl` returning false or throwing now closes the listener and reports a typed failure naming the manual-browser method. Docs, docs-parity assertions and direct regressions cover all four methods. Implementation note: OpenCode discovers this plugin's auth methods at runtime and presents their labels; numeric positions are transient indexes into that returned list, not documented or persisted method identifiers. This implementation inserts `Codex OAuth (Open URL Manually)` after the default-browser entry, moving the later entries in the runtime array. The first method also stops interpreting the undocumented `noBrowser` and `no-browser` input keys; direct callers should resolve the new named method from the returned method list instead. AI-Tool: opencode AI-Model: anthropic/claude-opus-5 AI-Platform: linux AI-Harness: Vibeterm 3e1f523-dirty --- AGENTS.md | 2 +- docs/architecture.md | 4 +- docs/development/ARCHITECTURE.md | 4 +- docs/faq.md | 2 +- docs/getting-started.md | 20 +- docs/troubleshooting.md | 12 +- index.ts | 283 ++++++++----- lib/AGENTS.md | 1 + lib/auth/loopback-flow.ts | 232 +++++++++++ lib/constants.ts | 7 +- test/doc-parity.test.ts | 52 +++ test/index.test.ts | 313 +++++++++++++-- test/loopback-flow.test.ts | 556 ++++++++++++++++++++++++++ test/oauth-server.integration.test.ts | 89 +++++ test/server.unit.test.ts | 26 ++ 15 files changed, 1433 insertions(+), 170 deletions(-) create mode 100644 lib/auth/loopback-flow.ts create mode 100644 test/loopback-flow.test.ts diff --git a/AGENTS.md b/AGENTS.md index 7fc04557..3a362ea6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ Package version: see `package.json` (`version` field). | Plugin orchestration | `index.ts` | OAuth loader, request pipeline, metrics, recovery, `ToolContext` assembly | | TUI quota status | `tui.ts`, `lib/tui-status.ts`, `lib/tui-quota-cache.ts`, `lib/codex-usage.ts` | prompt quota status, quota details, shared quota cache | | Tool registry | `lib/tools/index.ts` + `lib/tools/codex-*.ts` | 24 registered `codex-*` tools | -| OAuth flow + PKCE | `lib/auth/auth.ts`, `lib/auth/server.ts`, `lib/auth/device-code.ts`, `lib/auth/login-runner.ts` | browser/device/manual login, token refresh, workspace selection | +| OAuth flow + PKCE | `lib/auth/auth.ts`, `lib/auth/server.ts`, `lib/auth/device-code.ts`, `lib/auth/login-runner.ts`, `lib/auth/loopback-flow.ts` | browser/device/manual login, shared listener lifecycle, token refresh, workspace selection | | OAuth scopes | `lib/auth/scopes.ts` | connector scope validation and re-auth checks | | Multi-account rotation | `lib/accounts.ts`, `lib/accounts/`, `lib/rotation.ts` | `rotationStrategy` hybrid/sticky/round-robin, health scoring, cooldowns, token bucket, recovery | | Account storage | `lib/storage.ts`, `lib/storage/` | V3 facade, per-project/global paths, keychain, backup/import/export | diff --git a/docs/architecture.md b/docs/architecture.md index 567c1b68..83494a2f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -44,7 +44,7 @@ Standalone read/ops commands (no OpenCode agent loop required): `doctor`, `statu `index.ts` is the runtime entry OpenCode loads. It owns: -- OAuth login modes: browser callback, device code, and manual URL paste +- OAuth login modes: default-browser callback, open-URL-manually callback, device code, and manual URL/code paste - account manager lifecycle and local account storage (V3) - request URL/body/header transformation (native or legacy, plus responses-lite for GPT-5.6) - health-aware account selection, `rotationStrategy`, and `modelAccountPools` @@ -154,7 +154,7 @@ This guarantee is intentionally local-filesystem/same-host. A process that exits - Multi-turn continuity depends on `reasoning.encrypted_content` and the host-supplied conversation history. - Account pool limits: max **20** accounts; auth-failure cooldown **30s**; auto-removal after **3** consecutive auth failures. - Account bootstrap can hydrate from Codex CLI storage under `~/.codex` unless `CODEX_AUTH_SYNC_CODEX_CLI=0`. -- Auth methods exposed to OpenCode are the three OAuth labels only (browser, device code, manual URL). There is no registered API-key login method. +- Auth methods exposed to OpenCode are the four OAuth labels only (default browser, open URL manually, device code, manual URL/code paste). There is no registered API-key login method. - Credentials and account metadata stay local unless the user exports or migrates them. - Diagnostic commands redact sensitive account/token details by default. - The optional keychain backend must fall back without deleting JSON credentials silently. diff --git a/docs/development/ARCHITECTURE.md b/docs/development/ARCHITECTURE.md index 1247a367..deb1f5e6 100644 --- a/docs/development/ARCHITECTURE.md +++ b/docs/development/ARCHITECTURE.md @@ -43,7 +43,7 @@ OpenCode runtime | loads plugin package v index.ts - |- auth loader: browser callback, device code, manual URL paste + |- auth loader: default-browser callback, open-URL-manually callback, device code, manual URL/code paste |- account manager + V3 storage + optional keychain |- custom provider fetch pipeline |- runtime metrics, retry budgets, circuit breaker, recovery hooks @@ -95,7 +95,7 @@ tui.ts | Installer CLI | `scripts/install-oc-codex-multi-auth.js`, `scripts/install-oc-codex-multi-auth-core.js` | npm bin; config merge; cache cleanup; modern/full/legacy catalog selection; standalone doctor/status/list/limits/dashboard/health/diag/warm; TUI plugin enablement | | OpenCode plugin entry | `index.ts` | auth loader, runtime wiring, custom fetch pipeline, account manager lifecycle, `ToolContext`, OpenCode plugin export | | TUI plugin entry | `tui.ts`, `lib/tui-status.ts`, `lib/tui-quota-cache.ts`, `lib/codex-usage.ts` | prompt quota status, account-aware quota snapshots, usage refresh, details rendering | -| Auth flow | `lib/auth/auth.ts`, `lib/auth/server.ts`, `lib/auth/browser.ts`, `lib/auth/device-code.ts`, `lib/auth/login-runner.ts`, `lib/auth/scopes.ts` | PKCE OAuth, callback server, device/manual login, workspace/account selection, scope validation | +| Auth flow | `lib/auth/auth.ts`, `lib/auth/loopback-flow.ts`, `lib/auth/server.ts`, `lib/auth/browser.ts`, `lib/auth/device-code.ts`, `lib/auth/login-runner.ts`, `lib/auth/scopes.ts` | PKCE OAuth, callback server, default-browser and open-URL-manually listener flows, device code, manual URL/code paste, workspace/account selection, scope validation | | Account manager | `lib/accounts.ts`, `lib/accounts/` | account state facade, persistence, rotation, recovery, rate-limit tracking, workspace identity preservation, warm | | Storage | `lib/storage.ts`, `lib/storage/` | V3 JSON storage, atomic writes, migrations, per-project paths, backups, import/export, keychain opt-in, flagged accounts | | Request bridge | `lib/request/fetch-helpers.ts`, `lib/request/request-transformer.ts`, `lib/request/response-handler.ts`, `lib/request/retry-budget.ts`, `lib/request/rate-limit-backoff.ts`, `lib/request/helpers/` | URL/body/header shaping, Codex invariants, responses-lite, client identity, SSE conversion, retry budgets, backoff, error mapping | diff --git a/docs/faq.md b/docs/faq.md index c114da0a..deb99ef3 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -68,7 +68,7 @@ Tokens, account state, plugin config, quota cache, and logs are stored locally o ## Is there an API-key login? -No. The plugin registers three OAuth methods only (browser, device code, manual URL paste). A dummy SDK key string is used internally for the OpenAI client; ChatGPT OAuth tokens do the real auth. +No. The plugin registers four OAuth methods (default browser, open URL manually, device code, manual URL/code paste). A dummy SDK key string is used internally for the OpenAI client; ChatGPT OAuth tokens do the real auth. ## What should I do if authentication fails? diff --git a/docs/getting-started.md b/docs/getting-started.md index 36ba1f30..ba20afa7 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -95,14 +95,15 @@ opencode auth login Then choose: 1. `OpenAI` -2. One of the **three** plugin OAuth methods: - - `Codex OAuth (ChatGPT Plus/Pro)` — browser callback (default) +2. One of the **four** plugin OAuth methods: + - `Codex OAuth (ChatGPT Plus/Pro)` — opens the default browser; completes through a localhost callback + - `Codex OAuth (Open URL Manually)` - prints the authorization URL after port 1455 is listening; open it in any browser; the callback completes automatically through localhost - `Codex OAuth (Device Code)` — headless / SSH - - `Codex OAuth (Manual URL Paste)` — paste the redirect URL + - `Codex OAuth (Manual URL Paste)` - paste the full callback URL or raw authorization code; the full URL is preferred because it carries the OAuth state parameter and a supplied state mismatch is rejected before token exchange; a raw code is also accepted and PKCE-bound to the current flow There is **no** registered “Manual API Key” login path for this plugin. The provider still presents a dummy SDK key (`chatgpt-oauth`) internally; real auth is always OAuth. -The browser-based OAuth flow uses the same local callback port as Codex CLI. The authorize redirect is `http://localhost:1455/auth/callback`, while the local callback server binds `http://127.0.0.1:1455/auth/callback` and `[::1]:1455` for dual-stack localhost redirects. Authorization and token exchange go to `auth.openai.com`. +Both browser-based OAuth methods use the same local callback port as Codex CLI. The authorize redirect is `http://localhost:1455/auth/callback`, while the local callback server binds `http://127.0.0.1:1455/auth/callback` and `[::1]:1455` for dual-stack localhost redirects. Authorization and token exchange go to `auth.openai.com`. Account records persist the granted OAuth scope. The required scopes are `openid`, `profile`, `email`, and `offline_access`; an account whose recorded scope is explicitly missing one of them is marked for re-auth instead of being silently reused. An account whose scope is simply unrecorded is left enabled — absent metadata is not treated as a failed grant — and an account previously marked for re-auth is restored automatically once a complete scope is known. @@ -110,10 +111,13 @@ Account records persist the granted OAuth scope. The required scopes are `openid If you are on SSH, WSL, or another environment where the browser callback flow is inconvenient: -1. rerun `opencode auth login` -2. choose `Codex OAuth (Device Code)` -3. open the verification link, enter the one-time code, and wait for login to finish -4. if device code is unavailable on your auth server, fall back to `Codex OAuth (Manual URL Paste)` +- **If localhost port 1455 is reachable** (including via `ssh -L 1455:localhost:1455 user@remote`): + 1. rerun `opencode auth login` + 2. choose `Codex OAuth (Open URL Manually)` - it prints the URL after the listener is ready; open it in any browser; login completes automatically through localhost +- **If localhost is not reachable** (containers, restricted networks): + 1. rerun `opencode auth login` + 2. choose `Codex OAuth (Device Code)` - follow the verification link and one-time code + 3. if device code is unavailable, fall back to `Codex OAuth (Manual URL Paste)` - paste the full callback URL or raw authorization code ## Add the Plugin to OpenCode diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index dc0bd442..91a6d960 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -219,11 +219,13 @@ Failed to access Codex API **Solutions:** -1. **Manual URL paste:** +1. **Alternate login:** - Re-run `opencode auth login` - - Select **"Codex OAuth (Device Code)"** first if you are on SSH, WSL, or a headless machine - - If device code is unavailable, fall back to **"Codex OAuth (Manual URL Paste)"** - - Paste the full redirect URL after login when using the manual flow + - **If localhost port 1455 is reachable** (including via `ssh -L 1455:localhost:1455 user@remote`): + choose **`Codex OAuth (Open URL Manually)`** - it prints the URL after the listener is ready; open it in any browser; login completes automatically through localhost + - **If localhost is not reachable** (containers, restricted networks): + choose **`Codex OAuth (Device Code)`** - follow the verification link and one-time code; + if device code is unavailable, fall back to **`Codex OAuth (Manual URL Paste)`** - paste the full callback URL or raw authorization code 2. **Check port 1455 availability:** ```bash @@ -247,7 +249,7 @@ Failed to access Codex API **Solutions:** - Re-run `opencode auth login` to generate a fresh URL - Open the URL directly in browser (don't use a stale link) -- For SSH/WSL/remote, use **"Device Code"** first, then **"Manual URL Paste"** if needed +- For SSH/WSL/remote: if localhost port 1455 is reachable (including via SSH port forwarding), choose **Open URL Manually**; if localhost is not reachable, choose **Device Code**; use **Manual URL Paste** only as a last resort diff --git a/index.ts b/index.ts index 475bc755..2ad3507b 100644 --- a/index.ts +++ b/index.ts @@ -30,11 +30,16 @@ import { join } from "node:path"; import type { Plugin, PluginInput } from "@opencode-ai/plugin"; import type { Auth } from "@opencode-ai/sdk"; import { + type AuthorizationInputParseResult, createAuthorizationFlow, exchangeAuthorizationCode, parseAuthorizationInput, REDIRECT_URI, } from "./lib/auth/auth.js"; +import { + startLoopbackFlow, + type LoopbackFlowUnavailable, +} from "./lib/auth/loopback-flow.js"; import { buildDeviceCodeInstructions, completeDeviceCodeSession, @@ -53,8 +58,6 @@ import { coordinateFlaggedPersistedRefresh, coordinatePersistedRefresh, } from "./lib/storage/coordinated-refresh.js"; -import { openBrowserUrl } from "./lib/auth/browser.js"; -import { startLocalOAuthServer } from "./lib/auth/server.js"; import { promptAddAnotherAccount, promptLoginMode } from "./lib/cli.js"; import { getCodexMode, @@ -630,84 +633,116 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + const listenerUnavailableMessage = (): string => + `OAuth callback server failed to start on localhost loopback port 1455. ` + + `Retry with "${AUTH_LABELS.OAUTH_DEVICE_CODE}" or "${AUTH_LABELS.OAUTH_MANUAL}".`; + + const callbackCancelledMessage = (): string => + `OAuth callback timed out or was cancelled. ` + + `If you are on SSH, WSL, or a headless machine, retry with "${AUTH_LABELS.OAUTH_DEVICE_CODE}" or "${AUTH_LABELS.OAUTH_MANUAL}".`; + + const browserOpenFailedMessage = (): string => + `Could not launch your default browser. ` + + `Retry with "${AUTH_LABELS.OAUTH_MANUAL_BROWSER}" to print the URL and open it in the browser of your choice, ` + + `or use "${AUTH_LABELS.OAUTH_DEVICE_CODE}" or "${AUTH_LABELS.OAUTH_MANUAL}".`; + + const unavailableMessage = ( + lifecycle: LoopbackFlowUnavailable["lifecycle"], + ): string => { + switch (lifecycle) { + case "listener_unavailable": + return listenerUnavailableMessage(); + case "browser_open_failed": + return browserOpenFailedMessage(); + default: { + const unreachable: never = lifecycle; + return unreachable; + } + } + }; + + /** + * The asymmetry is deliberate: a raw code carries no state to compare + * and PKCE still binds it to this attempt's verifier, whereas + * structured input reproduces the callback's own parameters, so a + * missing or empty state there means the paste did not come from this + * attempt and must not reach the exchange. + */ + const manualInputRejection = ( + parsed: AuthorizationInputParseResult, + expectedState: string, + ): string | undefined => { + if (!parsed.code) { + return "No authorization code found. Paste the raw code, or the full callback URL (e.g., http://localhost:1455/auth/callback?code=...). If browser callback keeps failing, retry with Device Code."; + } + switch (parsed.source) { + case "raw": + return undefined; + case "url": + case "query": + case "fragment": + if (!parsed.state) { + return "That callback URL carries no OAuth state, so it cannot be matched to this login attempt. Paste the complete callback URL including its state parameter, paste the raw code on its own, or retry with Device Code."; + } + if (parsed.state !== expectedState) { + return "OAuth state mismatch. Restart login and paste the code or callback URL generated for this login attempt, or retry with Device Code."; + } + return undefined; + default: { + const unreachable: never = parsed; + return unreachable; + } + } + }; + const buildManualOAuthFlow = ( pkce: { verifier: string }, url: string, expectedState: string, replaceAll: boolean, ) => ({ - url, - method: "code" as const, - instructions: AUTH_LABELS.INSTRUCTIONS_MANUAL, - validate: (input: string): string | undefined => { - const parsed = parseAuthorizationInput(input); - if (!parsed.code) { - return "No authorization code found. Paste the full callback URL (e.g., http://localhost:1455/auth/callback?code=...). If browser callback keeps failing, retry with Device Code."; - } - if (!parsed.state) { - return parsed.source === "raw" - ? "That is a bare authorization code. This flow needs the full callback URL, including the state parameter (e.g., http://localhost:1455/auth/callback?code=...&state=...). If needed, retry with Device Code." - : "Missing OAuth state. Paste the full callback URL including both code and state parameters. If needed, retry with Device Code."; - } - if (parsed.state !== expectedState) { - return "OAuth state mismatch. Restart login and paste the callback URL generated for this login attempt, or retry with Device Code."; - } - return undefined; - }, - callback: async (input: string) => { - const parsed = parseAuthorizationInput(input); - if (!parsed.code || !parsed.state) { - return { - type: "failed" as const, - reason: "invalid_response" as const, - message: "Missing authorization code or OAuth state", - }; - } - if (parsed.state !== expectedState) { - return { - type: "failed" as const, - reason: "invalid_response" as const, - message: "OAuth state mismatch. Restart login and try again, or retry with Device Code.", - }; - } - const tokens = await exchangeAuthorizationCode( - parsed.code, - pkce.verifier, - REDIRECT_URI, - ); - if (tokens?.type === "success") { - const resolved = await resolveAndPersistAccountSelection(tokens, { - persistSelections: persistAuthenticatedSelections, - replaceAll, - }); - return resolved.primary; - } - return tokens?.type === "failed" - ? tokens - : { type: "failed" as const }; - }, - }); + url, + method: "code" as const, + instructions: AUTH_LABELS.INSTRUCTIONS_MANUAL, + validate: (input: string): string | undefined => + manualInputRejection(parseAuthorizationInput(input), expectedState), + callback: async (input: string) => { + const parsed = parseAuthorizationInput(input); + const rejection = manualInputRejection(parsed, expectedState); + if (rejection !== undefined || !parsed.code) { + return { + type: "failed" as const, + reason: "invalid_response" as const, + message: rejection ?? "Missing authorization code", + }; + } + const tokens = await exchangeAuthorizationCode( + parsed.code, + pkce.verifier, + REDIRECT_URI, + ); + if (tokens?.type === "success") { + const resolved = await resolveAndPersistAccountSelection(tokens, { + persistSelections: persistAuthenticatedSelections, + replaceAll, + }); + return resolved.primary; + } + return tokens?.type === "failed" + ? tokens + : { type: "failed" as const }; + }, + }); const runOAuthFlow = async ( forceNewLogin: boolean = false, ): Promise => { - const { pkce, state, url } = await createAuthorizationFlow({ forceNewLogin }); - logInfo(`OAuth URL: ${url}`); - - let serverInfo: Awaited> | null = null; - try { - serverInfo = await startLocalOAuthServer({ state }); - } catch (err) { - logDebug(`[${PLUGIN_NAME}] Failed to start OAuth server: ${(err as Error)?.message ?? String(err)}`); - serverInfo = null; - } - openBrowserUrl(url); - - if (!serverInfo || !serverInfo.ready) { - serverInfo?.close(); - const message = - `OAuth callback server failed to start on localhost loopback port 1455. ` + - `Retry with "${AUTH_LABELS.OAUTH_DEVICE_CODE}" or "${AUTH_LABELS.OAUTH_MANUAL}".`; + const session = await startLoopbackFlow({ + openBrowser: true, + forceNewLogin, + }); + if (session.type === "unavailable") { + const message = unavailableMessage(session.lifecycle); logWarn(`\n[${PLUGIN_NAME}] ${message}\n`); return { type: "failed" as const, @@ -715,25 +750,16 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { message, }; } - - const result = await serverInfo.waitForCode(state); - serverInfo.close(); - - if (!result) { + logInfo(`OAuth URL: ${session.url}`); + const result = await session.waitAndExchange(); + if (result.type === "cancelled") { return { type: "failed" as const, reason: "unknown" as const, - message: - `OAuth callback timed out or was cancelled. ` + - `If you are on SSH, WSL, or a headless machine, retry with "${AUTH_LABELS.OAUTH_DEVICE_CODE}" or "${AUTH_LABELS.OAUTH_MANUAL}".`, + message: callbackCancelledMessage(), }; } - - return await exchangeAuthorizationCode( - result.code, - pkce.verifier, - REDIRECT_URI, - ); + return result; }; const showToast = async ( @@ -3489,10 +3515,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { setStoragePath(authPerProjectAccounts ? process.cwd() : null); const accounts: TokenSuccessWithAccount[] = []; - const noBrowser = - inputs?.noBrowser === "true" || - inputs?.["no-browser"] === "true"; - const useManualMode = noBrowser; const explicitLoginMode = inputs?.loginMode === "fresh" || inputs?.loginMode === "add" ? inputs.loginMode @@ -4249,14 +4271,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (refreshAccountIndex !== undefined) { targetCount = 1; } - if (useManualMode) { - targetCount = 1; - } - - if (useManualMode) { - const { pkce, state, url } = await createAuthorizationFlow(); - return buildManualOAuthFlow(pkce, url, state, startFresh); - } const explicitCountProvided = typeof inputs?.accountCount === "string" && inputs.accountCount.trim().length > 0; @@ -4366,6 +4380,60 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; }, }, + { + label: AUTH_LABELS.OAUTH_MANUAL_BROWSER, + type: "oauth" as const, + authorize: async () => { + // Must happen BEFORE the callback persists, to ensure correct + // storage location: OpenCode invokes callback() separately from + // authorize(), so the path has to be pinned here. + const manualBrowserPluginConfig = loadPluginConfig(); + applyUiRuntimeFromConfig(manualBrowserPluginConfig); + const manualBrowserPerProjectAccounts = getPerProjectAccounts(manualBrowserPluginConfig); + setStoragePath(manualBrowserPerProjectAccounts ? process.cwd() : null); + + const session = await startLoopbackFlow({ openBrowser: false }); + if (session.type === "unavailable") { + const message = unavailableMessage(session.lifecycle); + logWarn(`\n[${PLUGIN_NAME}] ${message}\n`); + return { + url: "", + instructions: message, + method: "auto" as const, + callback: () => + Promise.resolve({ + type: "failed" as const, + reason: "invalid_response" as const, + message, + }), + }; + } + + return { + url: session.url, + instructions: AUTH_LABELS.INSTRUCTIONS_MANUAL_BROWSER, + method: "auto" as const, + callback: async () => { + const result = await session.waitAndExchange(); + if (result.type === "cancelled") { + return { + type: "failed" as const, + reason: "unknown" as const, + message: callbackCancelledMessage(), + }; + } + if (result.type !== "success") { + return result; + } + const resolved = await resolveAndPersistAccountSelection(result, { + persistSelections: persistAuthenticatedSelections, + replaceAll: false, + }); + return resolved.primary; + }, + }; + }, + }, { label: AUTH_LABELS.OAUTH_DEVICE_CODE, type: "oauth" as const, @@ -4405,22 +4473,23 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }, }, - { - label: AUTH_LABELS.OAUTH_MANUAL, - type: "oauth" as const, - authorize: async () => { - // Initialize storage path for manual OAuth flow - // Must happen BEFORE persistAccountPool to ensure correct storage location - const manualPluginConfig = loadPluginConfig(); + { + label: AUTH_LABELS.OAUTH_MANUAL, + type: "oauth" as const, + authorize: async () => { + // Must happen BEFORE the callback persists, to ensure correct + // storage location: OpenCode invokes callback() separately from + // authorize(), so the path has to be pinned here. + const manualPluginConfig = loadPluginConfig(); applyUiRuntimeFromConfig(manualPluginConfig); - const manualPerProjectAccounts = getPerProjectAccounts(manualPluginConfig); + const manualPerProjectAccounts = getPerProjectAccounts(manualPluginConfig); setStoragePath(manualPerProjectAccounts ? process.cwd() : null); const { pkce, state, url } = await createAuthorizationFlow(); return buildManualOAuthFlow(pkce, url, state, false); - }, - }, - ], + }, + }, + ], }, tool: createToolRegistry(ctx), }; diff --git a/lib/AGENTS.md b/lib/AGENTS.md index 3d759da2..7a2e6ed8 100644 --- a/lib/AGENTS.md +++ b/lib/AGENTS.md @@ -62,6 +62,7 @@ lib/ | OAuth scopes | `auth/scopes.ts` | connector scope checks | | Browser launch | `auth/browser.ts` | platform-specific open | | Callback server | `auth/server.ts` | HTTP on port 1455 | +| Browser/manual OAuth lifecycle | `auth/loopback-flow.ts` | listener-first shared session, opener handling, callback exchange, close-once cleanup | | URL/body transform | `request/request-transformer.ts` | model map, prompt injection, stateless compatibility | | Headers + errors | `request/fetch-helpers.ts` | Codex headers, rate limit handling, fallback, refresh | | Retry budgets | `request/retry-budget.ts` | bounded retry classes | diff --git a/lib/auth/loopback-flow.ts b/lib/auth/loopback-flow.ts new file mode 100644 index 00000000..08dce5ed --- /dev/null +++ b/lib/auth/loopback-flow.ts @@ -0,0 +1,232 @@ +/** + * Loopback OAuth session primitive. + * + * One typed session shared by the automatic-browser and manual-browser + * choices: the listener is bound BEFORE any browser is opened, so the browser + * choice cannot land on a redirect_uri nobody is listening on. Manual mode + * (`openBrowser=false`) never opens a browser. If the listener cannot bind, + * the browser is never opened and a typed `unavailable` lifecycle marker is + * returned; the caller maps that marker to user-facing guidance (label names, + * fallback instructions) because those belong at the auth-method boundary in + * index.ts, not inside this primitive. + * + * A ready session starts its callback observation IMMEDIATELY, so the + * listener's existing five-minute deadline runs from session creation rather + * than from the host's callback. An authorization the host abandons after + * taking the URL therefore still releases port 1455 instead of pinning it for + * the lifetime of the process. That observation is normalized so it can never + * reject: an unobserved rejecting promise would be an unhandled rejection in + * the window before `waitAndExchange()` awaits it. + * + * `waitAndExchange()` remains the lazy token-exchange boundary - the host + * stores the authorization and calls back separately - and awaits that shared + * outcome before exchanging the code with the flow's own PKCE verifier and + * `REDIRECT_URI`. Every terminal path (callback, timeout, external close, + * opener failure, throwing wait, throwing exchange) runs through one + * `closeOnce` gate, so `server.close()` happens exactly once. A null callback + * surfaces as a typed `cancelled` lifecycle result, and a callback arriving + * after expiry observes that already-settled cancellation. + * + * This module holds NO labels, persistence, or OpenCode method shapes. It + * does not duplicate the URL parser, PKCE generator, callback server, or + * token exchange - it composes the primitives that already own them. + */ +import type { + OAuthServerInfo, + AuthorizationFlow, + TokenResult, +} from "../types.js"; +import { + createAuthorizationFlow as defaultCreateAuthorizationFlow, + exchangeAuthorizationCode as defaultExchangeAuthorizationCode, + REDIRECT_URI, +} from "./auth.js"; +import { startLocalOAuthServer as defaultStartLocalOAuthServer } from "./server.js"; +import { openBrowserUrl as defaultOpenBrowserUrl } from "./browser.js"; + +export interface LoopbackFlowDeps { + createAuthorizationFlow: ( + opts?: { forceNewLogin?: boolean }, + ) => Promise; + startLocalOAuthServer: (opts: { state: string }) => Promise; + openBrowserUrl: (url: string) => boolean; + exchangeAuthorizationCode: ( + code: string, + verifier: string, + redirectUri?: string, + ) => Promise; +} + +export interface LoopbackFlowOptions { + /** + * `true` opens the user's default browser AFTER the listener is ready. + * `false` (manual mode) never calls `openBrowserUrl`; the caller is + * expected to hand `url` back to the user directly. + */ + openBrowser: boolean; + /** + * Forwarded verbatim to `createAuthorizationFlow`. When true, the + * authorize URL gets `prompt=login` so a cached browser session cannot + * silently reuse the previous account. + */ + forceNewLogin?: boolean; + /** + * Test seam. Individual overrides merge onto the production defaults; + * production callers pass nothing. + */ + deps?: Partial; +} + +export interface LoopbackFlowReady { + type: "ready"; + /** + * Authorize URL to hand to the user (manual mode) or that the primitive + * has already opened (automatic mode). + */ + url: string; + /** + * Idempotent listener close. Safe to call more than once; safe to call + * before `waitAndExchange()` starts, which then completes without a + * second close. + */ + close: () => void; + /** + * Awaits the callback, exchanges the code with the same PKCE verifier + * and `REDIRECT_URI` used to construct the authorize URL, and closes + * the listener exactly once (whether the callback succeeded, timed + * out, or the wait/exchange threw). A null callback surfaces as a + * typed `cancelled` lifecycle marker rather than a `TokenResult`. + */ + waitAndExchange: () => Promise; +} + +export interface LoopbackFlowUnavailable { + type: "unavailable"; + /** + * `listener_unavailable`: the callback port could not be bound, so no + * browser was opened. `browser_open_failed`: the listener was ready but + * the default browser could not be launched, so the session was closed + * rather than left waiting on a page the user never saw. + */ + lifecycle: "listener_unavailable" | "browser_open_failed"; +} + +export interface LoopbackFlowCancelled { + type: "cancelled"; + lifecycle: "callback_timeout_or_cancelled"; +} + +export type LoopbackFlowSession = LoopbackFlowReady | LoopbackFlowUnavailable; +export type LoopbackFlowWaitResult = TokenResult | LoopbackFlowCancelled; + +type CallbackObservation = + | { type: "code"; code: string } + | { type: "cancelled" } + | { type: "wait_failed"; error: unknown }; + +const DEFAULT_DEPS: LoopbackFlowDeps = { + createAuthorizationFlow: defaultCreateAuthorizationFlow, + startLocalOAuthServer: defaultStartLocalOAuthServer, + openBrowserUrl: defaultOpenBrowserUrl, + exchangeAuthorizationCode: defaultExchangeAuthorizationCode, +}; + +/** + * Start one loopback OAuth session for either the automatic or manual browser + * choice. See the module docstring for the full lifecycle contract. + */ +export async function startLoopbackFlow( + options: LoopbackFlowOptions, +): Promise { + const deps: LoopbackFlowDeps = { + ...DEFAULT_DEPS, + ...(options.deps ?? {}), + }; + + const flow = await deps.createAuthorizationFlow( + options.forceNewLogin ? { forceNewLogin: true } : undefined, + ); + const server = await deps.startLocalOAuthServer({ state: flow.state }); + + if (!server.ready) { + try { + server.close(); + } catch { + // listener never bound: close is best-effort and non-fatal + } + return { + type: "unavailable", + lifecycle: "listener_unavailable", + }; + } + + let closed = false; + const closeOnce = (): void => { + if (closed) return; + closed = true; + try { + server.close(); + } catch { + // a listener that will not close must not mask the outcome the + // caller is waiting on; the close contract is best-effort + } + }; + + const callbackOutcome: Promise = (async () => { + try { + const callback = await server.waitForCode(flow.state); + return callback + ? { type: "code", code: callback.code } + : { type: "cancelled" }; + } catch (error) { + return { type: "wait_failed", error }; + } finally { + closeOnce(); + } + })(); + + if (options.openBrowser) { + let opened: boolean; + try { + opened = deps.openBrowserUrl(flow.url); + } catch { + opened = false; + } + if (!opened) { + closeOnce(); + return { + type: "unavailable", + lifecycle: "browser_open_failed", + }; + } + } + + const waitAndExchange = async (): Promise => { + const outcome = await callbackOutcome; + if (outcome.type === "cancelled") { + return { + type: "cancelled", + lifecycle: "callback_timeout_or_cancelled", + }; + } + if (outcome.type === "wait_failed") { + throw outcome.error; + } + try { + return await deps.exchangeAuthorizationCode( + outcome.code, + flow.pkce.verifier, + REDIRECT_URI, + ); + } finally { + closeOnce(); + } + }; + + return { + type: "ready", + url: flow.url, + close: closeOnce, + waitAndExchange, + }; +} diff --git a/lib/constants.ts b/lib/constants.ts index b6e5597a..b0bc82ee 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -88,13 +88,16 @@ export const PLATFORM_OPENERS = { /** OAuth authorization labels */ export const AUTH_LABELS = { OAUTH: "Codex OAuth (ChatGPT Plus/Pro)", + OAUTH_MANUAL_BROWSER: "Codex OAuth (Open URL Manually)", OAUTH_DEVICE_CODE: "Codex OAuth (Device Code)", OAUTH_MANUAL: "Codex OAuth (Manual URL Paste)", API_KEY: "Manual API Key (Advanced)", INSTRUCTIONS: - "A browser window should open. If it doesn't, copy the URL and open it manually.", + "Opening the default browser to complete sign-in. If nothing opens, copy the URL and open it in any browser.", + INSTRUCTIONS_MANUAL_BROWSER: + "Open this URL in any browser to sign in; sign-in completes automatically through the localhost callback on port 1455.", INSTRUCTIONS_MANUAL: - "After logging in, copy the full redirect URL and paste it here.", + "After logging in, paste either the full redirect URL or the raw authorization code here.", } as const; /** Multi-account configuration */ diff --git a/test/doc-parity.test.ts b/test/doc-parity.test.ts index 5bd3700c..07783fbe 100644 --- a/test/doc-parity.test.ts +++ b/test/doc-parity.test.ts @@ -143,6 +143,10 @@ function normalizeRepoPathReference(rawValue: string): string | null { return null; } +function findMissingLabels(content: string, labels: readonly string[]): string[] { + return labels.filter((label) => !content.includes(label)); +} + describe("runtime documentation parity", () => { it("keeps the documented stateless request contract aligned with the runtime transform", async () => { const requestBody: RequestBody = { @@ -613,6 +617,54 @@ describe("runtime documentation parity", () => { expect(hits).toEqual([]); }); + it("keeps auth-method labels and count in docs aligned with AUTH_LABELS", async () => { + const { AUTH_LABELS } = await import("../lib/constants.js"); + const expectedLabels = [ + AUTH_LABELS.OAUTH, + AUTH_LABELS.OAUTH_MANUAL_BROWSER, + AUTH_LABELS.OAUTH_DEVICE_CODE, + AUTH_LABELS.OAUTH_MANUAL, + ]; + + const gettingStarted = readRepoFile("docs/getting-started.md"); + for (const label of expectedLabels) { + expect(gettingStarted, `missing label in getting-started.md: ${label}`).toContain(label); + } + expect(gettingStarted).toContain("four"); + expect(gettingStarted).not.toContain("**three**"); + + const archPublic = readRepoFile("docs/architecture.md"); + expect(archPublic, "docs/architecture.md must say four OAuth labels only").toContain( + "four OAuth labels only", + ); + expect(archPublic).not.toContain("three OAuth labels only"); + expect(archPublic).toContain("open URL manually"); + expect(archPublic).toContain("manual URL/code paste"); + + const archDev = readRepoFile("docs/development/ARCHITECTURE.md"); + expect( + archDev, + "docs/development/ARCHITECTURE.md must include open-URL-manually callback", + ).toContain("open-URL-manually callback"); + expect(archDev).toContain("manual URL/code paste"); + expect(archDev).not.toContain("browser callback, device code, manual URL paste"); + + const faq = readRepoFile("docs/faq.md"); + expect(faq, "docs/faq.md must not claim three OAuth methods").not.toContain( + "three OAuth methods", + ); + expect(faq).toContain("four OAuth methods"); + expect(faq).toContain("open URL manually"); + + // Drift probe: operate on an in-memory copy only — never modify the actual source file. + const labelToRemove = AUTH_LABELS.OAUTH_MANUAL_BROWSER; + const gettingStartedWithout = gettingStarted.replaceAll(labelToRemove, "REMOVED"); + expect(findMissingLabels(gettingStartedWithout, expectedLabels)).toEqual([labelToRemove]); + for (const label of expectedLabels.filter((l) => l !== labelToRemove)) { + expect(gettingStartedWithout).toContain(label); + } + }); + it("keeps npm scripts mentioned in current documentation aligned with package.json", () => { const packageJson = JSON.parse(readRepoFile("package.json")) as { scripts?: Record; diff --git a/test/index.test.ts b/test/index.test.ts index 0ea19dff..afeff284 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -26,15 +26,11 @@ vi.mock("@opencode-ai/plugin/tool", () => { }); vi.mock("../lib/auth/auth.js", async () => { - // parseAuthorizationInput is pure, so the manual-flow tests run the real - // classifier rather than a hand-written stand-in. A stand-in cannot be - // type-checked against the module and would silently drift from its - // `source` contract, which is what index.ts branches on. const actual = await vi.importActual( "../lib/auth/auth.js", ); - return { + ...actual, createAuthorizationFlow: vi.fn(async () => ({ pkce: { verifier: "test-verifier", challenge: "test-challenge" }, state: "test-state", @@ -47,7 +43,6 @@ vi.mock("../lib/auth/auth.js", async () => { expires: Date.now() + 3600_000, idToken: "id-token", })), - parseAuthorizationInput: vi.fn(actual.parseAuthorizationInput), decodeJWT: vi.fn((token: string) => { try { const payload = token.split(".")[1]; @@ -61,7 +56,6 @@ vi.mock("../lib/auth/auth.js", async () => { return null; } }), - REDIRECT_URI: "http://localhost:1455/auth/callback", }; }); @@ -106,7 +100,7 @@ vi.mock("../lib/refresh-queue.js", () => ({ })); vi.mock("../lib/auth/browser.js", () => ({ - openBrowserUrl: vi.fn(), + openBrowserUrl: vi.fn(() => true), })); vi.mock("../lib/auth/server.js", () => ({ @@ -744,16 +738,9 @@ describe("OpenAIOAuthPlugin", () => { expect(plugin.tool["codex-import"]).toBeDefined(); }); - it("has three auth methods", () => { - expect(plugin.auth.methods).toHaveLength(3); - expect(plugin.auth.methods[0].label).toBe("Codex OAuth (ChatGPT Plus/Pro)"); - expect(plugin.auth.methods[1].label).toBe("Codex OAuth (Device Code)"); - expect(plugin.auth.methods[2].label).toBe("Codex OAuth (Manual URL Paste)"); - }); - it("rejects manual OAuth callbacks with mismatched state", async () => { const authModule = await import("../lib/auth/auth.js"); - const manualMethod = plugin.auth.methods[2] as unknown as { + const manualMethod = plugin.auth.methods[3] as unknown as { authorize: () => Promise<{ validate: (input: string) => string | undefined; callback: (input: string) => Promise<{ type: string; reason?: string; message?: string }>; @@ -770,31 +757,6 @@ describe("OpenAIOAuthPlugin", () => { expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); }); - it("tells the user to paste the callback URL when only a bare code is supplied", async () => { - const authModule = await import("../lib/auth/auth.js"); - const manualMethod = plugin.auth.methods[2] as unknown as { - authorize: () => Promise<{ - validate: (input: string) => string | undefined; - callback: (input: string) => Promise<{ type: string; reason?: string }>; - }>; - }; - - const flow = await manualMethod.authorize(); - // A bare code carries no state, so it cannot be bound to this login - // attempt. The message has to name the missing part, not just repeat - // the generic "missing OAuth state" wording used for callback input. - const bareCode = "abc123"; - - const message = flow.validate(bareCode); - expect(message).toContain("bare authorization code"); - expect(message).toContain("state"); - expect(flow.validate("state=test-state")).toContain("No authorization code found"); - - const result = await flow.callback(bareCode); - expect(result.type).toBe("failed"); - expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); - }); - it("suggests device code when browser callback server is unavailable", async () => { const serverModule = await import("../lib/auth/server.js"); vi.mocked(serverModule.startLocalOAuthServer).mockResolvedValueOnce({ @@ -822,7 +784,7 @@ describe("OpenAIOAuthPlugin", () => { it("completes device code login and persists the account", async () => { const deviceModule = await import("../lib/auth/device-code.js"); - const deviceMethod = plugin.auth.methods[1] as unknown as { + const deviceMethod = plugin.auth.methods[2] as unknown as { authorize: () => Promise<{ instructions: string; callback: () => Promise<{ type: string }>; @@ -846,6 +808,273 @@ describe("OpenAIOAuthPlugin", () => { }); }); + describe("four-method OAuth contract", () => { + it("plugin.auth.methods has four entries in the four-method order", () => { + expect(plugin.auth.methods).toHaveLength(4); + expect(plugin.auth.methods[0].label).toBe("Codex OAuth (ChatGPT Plus/Pro)"); + expect(plugin.auth.methods[1].label).toBe("Codex OAuth (Open URL Manually)"); + expect(plugin.auth.methods[2].label).toBe("Codex OAuth (Device Code)"); + expect(plugin.auth.methods[3].label).toBe("Codex OAuth (Manual URL Paste)"); + }); + + it("manual-browser method returns a non-empty URL with method:auto and does not open the default browser", async () => { + const browserModule = await import("../lib/auth/browser.js"); + vi.mocked(browserModule.openBrowserUrl).mockClear(); + const manualBrowserMethod = plugin.auth.methods[1] as unknown as { + authorize: () => Promise<{ + url: string; + method: string; + }>; + }; + const flow = await manualBrowserMethod.authorize(); + expect(flow.url.length).toBeGreaterThan(0); + expect(flow.method).toBe("auto"); + expect(vi.mocked(browserModule.openBrowserUrl)).not.toHaveBeenCalled(); + }); + + it("manual-browser method awaits the callback server, exchanges the code, and persists a successful account", async () => { + const authModule = await import("../lib/auth/auth.js"); + const serverModule = await import("../lib/auth/server.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + vi.mocked(serverModule.startLocalOAuthServer).mockClear(); + const manualBrowserMethod = plugin.auth.methods[1] as unknown as { + authorize: () => Promise<{ + callback: () => Promise<{ type: string }>; + }>; + }; + const flow = await manualBrowserMethod.authorize(); + const result = await flow.callback(); + expect(result.type).toBe("success"); + expect(vi.mocked(serverModule.startLocalOAuthServer)).toHaveBeenCalled(); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + "auth-code", + "test-verifier", + "http://localhost:1455/auth/callback", + ); + expect(mockStorage.accounts).toHaveLength(1); + }); + + it("manual-browser method with unavailable listener returns a failed callback naming Device Code and Manual URL Paste and never opens the browser", async () => { + const serverModule = await import("../lib/auth/server.js"); + vi.mocked(serverModule.startLocalOAuthServer).mockResolvedValueOnce({ + ready: false, + close: vi.fn(), + waitForCode: vi.fn(async () => null), + port: 1455, + }); + const browserModule = await import("../lib/auth/browser.js"); + vi.mocked(browserModule.openBrowserUrl).mockClear(); + const manualBrowserMethod = plugin.auth.methods[1] as unknown as { + authorize: () => Promise<{ + url: string; + callback: () => Promise<{ type: string; message?: string }>; + }>; + }; + const flow = await manualBrowserMethod.authorize(); + expect(vi.mocked(browserModule.openBrowserUrl)).not.toHaveBeenCalled(); + const result = await flow.callback(); + expect(result.type).toBe("failed"); + expect(result.message).toContain("Device Code"); + expect(result.message).toContain("Manual URL Paste"); + }); + + it("manual paste accepts a raw authorization code and calls exchange with the flow verifier", async () => { + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const rawCode = "abc123"; + expect(flow.validate(rawCode)).toBeUndefined(); + const result = await flow.callback(rawCode); + expect(result.type).toBe("success"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + "abc123", + "test-verifier", + "http://localhost:1455/auth/callback", + ); + }); + + it("manual paste accepts a full callback URL with matching state and calls exchange", async () => { + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const validUrl = "http://localhost:1455/auth/callback?code=abc123&state=test-state"; + expect(flow.validate(validUrl)).toBeUndefined(); + const result = await flow.callback(validUrl); + expect(result.type).toBe("success"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + "abc123", + "test-verifier", + "http://localhost:1455/auth/callback", + ); + }); + + it("manual paste with mismatched supplied state fails validation and callback without calling exchange", async () => { + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string; reason?: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const mismatchedUrl = "http://localhost:1455/auth/callback?code=abc123&state=wrong-state"; + expect(flow.validate(mismatchedUrl)).toContain("state mismatch"); + const result = await flow.callback(mismatchedUrl); + expect(result.type).toBe("failed"); + expect(result.reason).toBe("invalid_response"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); + }); + + it.each([ + ["full callback URL with no state", "http://localhost:1455/auth/callback?code=abc123"], + ["full callback URL with empty state", "http://localhost:1455/auth/callback?code=abc123&state="], + ["bare query with no state", "code=abc123"], + ["bare query with empty state", "code=abc123&state="], + ["bare fragment with no state", "#code=abc123"], + ["code#state with empty state", "abc123#"], + ])( + "manual paste rejects structured input carrying no usable state (%s) before exchange", + async (_label, input) => { + // Given a manual-paste flow and structured input that omits its state + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string; reason?: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + + // When it is validated and submitted + const validation = flow.validate(input); + const result = await flow.callback(input); + + // Then both gates refuse it and no exchange is attempted + expect(validation).toBeDefined(); + expect(result.type).toBe("failed"); + expect(result.reason).toBe("invalid_response"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); + }, + ); + + it("manual paste accepts a callback URL in the provider's own parameter shape", async () => { + // Given a manual-paste flow and the URL shape the provider actually + // redirects to: an opaque dotted code, a scope parameter between code + // and state, and plus-encoded scope values + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const providerCode = "ac_ZmFrZS1hdXRoLWNvZGU.ZmFrZS1zaWduYXR1cmU"; + const providerUrl = + `http://localhost:1455/auth/callback?code=${providerCode}` + + `&scope=openid+profile+email+offline_access&state=test-state`; + + // When it is validated and submitted + const validation = flow.validate(providerUrl); + const result = await flow.callback(providerUrl); + + // Then the unrelated scope parameter does not defeat the state gate + expect(validation).toBeUndefined(); + expect(result.type).toBe("success"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + providerCode, + "test-verifier", + "http://localhost:1455/auth/callback", + ); + }); + + it("manual paste accepts that same provider code pasted on its own", async () => { + // Given a manual-paste flow and only the opaque code from that callback + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const providerCode = "ac_ZmFrZS1hdXRoLWNvZGU.ZmFrZS1zaWduYXR1cmU"; + + // When it is validated and submitted with no state alongside it + const validation = flow.validate(providerCode); + const result = await flow.callback(providerCode); + + // Then the dotted opaque code stays raw and PKCE still binds it + expect(validation).toBeUndefined(); + expect(result.type).toBe("success"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + providerCode, + "test-verifier", + "http://localhost:1455/auth/callback", + ); + }); + + it("manual paste accepts code#state whose state matches the login attempt", async () => { + // Given a manual-paste flow and code#state input for this attempt + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + + // When it is validated and submitted + const validation = flow.validate("abc123#test-state"); + const result = await flow.callback("abc123#test-state"); + + // Then it exchanges with this attempt's verifier and redirect URI + expect(validation).toBeUndefined(); + expect(result.type).toBe("success"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).toHaveBeenCalledWith( + "abc123", + "test-verifier", + "http://localhost:1455/auth/callback", + ); + }); + + it("manual paste with no code returns invalid_response without calling exchange", async () => { + const authModule = await import("../lib/auth/auth.js"); + vi.mocked(authModule.exchangeAuthorizationCode).mockClear(); + const manualMethod = plugin.auth.methods[3] as unknown as { + authorize: () => Promise<{ + validate: (input: string) => string | undefined; + callback: (input: string) => Promise<{ type: string; reason?: string }>; + }>; + }; + const flow = await manualMethod.authorize(); + const stateOnly = "state=test-state"; + expect(flow.validate(stateOnly)).toBeDefined(); + const result = await flow.callback(stateOnly); + expect(result.type).toBe("failed"); + expect(result.reason).toBe("invalid_response"); + expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); + }); + }); + describe("event handler", () => { it("handles account.select event", async () => { await plugin.event({ event: { type: "account.select", properties: { index: 0 } } }); diff --git a/test/loopback-flow.test.ts b/test/loopback-flow.test.ts new file mode 100644 index 00000000..0c203321 --- /dev/null +++ b/test/loopback-flow.test.ts @@ -0,0 +1,556 @@ +/** + * Failing-first tests for the loopback OAuth session primitive. + * + * Pins the lifecycle contract of `startLoopbackFlow`: listener readiness + * precedes any browser open, manual mode never opens, an unavailable listener + * returns a typed lifecycle marker without opening, a matching callback + * exchanges with the same verifier and REDIRECT_URI, a null callback surfaces + * as a typed cancelled result (not a TokenResult), wait/exchange exceptions + * do not leak the listener, and every ready session closes exactly once. + * + * The primitive returns lifecycle MARKERS rather than user-facing strings so + * label/policy wording stays at the auth-method boundary in index.ts. + */ +import { describe, it, expect, vi } from "vitest"; +import type { OAuthServerInfo, AuthorizationFlow, TokenResult } from "../lib/types.js"; +import { REDIRECT_URI } from "../lib/auth/auth.js"; +import { + startLoopbackFlow, + type LoopbackFlowSession, +} from "../lib/auth/loopback-flow.js"; + +interface CallLog { + events: string[]; +} + +function makeFlow(overrides: Partial = {}): AuthorizationFlow { + return { + pkce: { verifier: "verifier-xyz", challenge: "challenge-xyz" }, + state: "state-abc", + url: "https://auth.openai.com/oauth/authorize?state=state-abc", + ...overrides, + }; +} + +function makeTokenResult( + overrides: Partial> = {}, +): TokenResult { + return { + type: "success", + access: "access-token", + refresh: "refresh-token", + expires: 1, + ...overrides, + }; +} + +function makeReadyServer( + log: CallLog, + options: { + code?: string | null; + waitThrows?: Error; + } = {}, +): OAuthServerInfo & { closeMock: ReturnType; waitMock: ReturnType } { + const closeMock = vi.fn(() => { + log.events.push("close"); + }); + const waitMock = vi.fn(async () => { + log.events.push("wait"); + if (options.waitThrows) throw options.waitThrows; + if (options.code === undefined) return { code: "code-123" }; + if (options.code === null) return null; + return { code: options.code }; + }); + return { + port: 1455, + ready: true, + close: closeMock, + waitForCode: waitMock, + closeMock, + waitMock, + }; +} + +/** + * `close()` settling the pending wait with null is fidelity, not convenience: + * the real listener sets `pollAborted` on close and `waitForCode` then returns + * null on its next poll (lib/auth/server.ts). + */ +function makeDeferredServer(log: CallLog): OAuthServerInfo & { + closeMock: ReturnType; + waitMock: ReturnType; + resolveWith: (value: { code: string } | null) => void; + rejectWith: (error: Error) => void; +} { + let settle: (value: { code: string } | null) => void = () => {}; + let fail: (error: Error) => void = () => {}; + const pending = new Promise<{ code: string } | null>((resolve, reject) => { + settle = resolve; + fail = reject; + }); + const closeMock = vi.fn(() => { + log.events.push("close"); + settle(null); + }); + const waitMock = vi.fn(() => { + log.events.push("wait"); + return pending; + }); + return { + port: 1455, + ready: true, + close: closeMock, + waitForCode: waitMock, + closeMock, + waitMock, + resolveWith: settle, + rejectWith: fail, + }; +} + +function makeUnavailableServer(log: CallLog): OAuthServerInfo & { closeMock: ReturnType } { + const closeMock = vi.fn(() => { + log.events.push("close-unavailable"); + }); + return { + port: 1455, + ready: false, + close: closeMock, + waitForCode: async () => null, + closeMock, + }; +} + +function assertReady(session: LoopbackFlowSession): asserts session is Extract< + LoopbackFlowSession, + { type: "ready" } +> { + if (session.type !== "ready") { + throw new Error(`expected ready session, got ${session.type}`); + } +} + +describe("startLoopbackFlow", () => { + it("emits a ready session whose URL matches the created flow and opens the browser only after the listener is ready", async () => { + const log: CallLog = { events: [] }; + const flow = makeFlow(); + const server = makeReadyServer(log); + const openBrowserUrl = vi.fn(() => { + log.events.push("open"); + return true; + }); + + const session = await startLoopbackFlow({ + openBrowser: true, + deps: { + createAuthorizationFlow: async () => { + log.events.push("createFlow"); + return flow; + }, + startLocalOAuthServer: async ({ state }) => { + log.events.push(`startServer:${state}`); + return server; + }, + openBrowserUrl, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + expect(session.url).toBe(flow.url); + expect(log.events.slice(0, 3)).toEqual([ + "createFlow", + "startServer:state-abc", + "wait", + ]); + expect(log.events.indexOf("startServer:state-abc")).toBeLessThan( + log.events.indexOf("open"), + ); + expect(openBrowserUrl).toHaveBeenCalledWith(flow.url); + + session.close(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("never opens the browser in manual mode (openBrowser=false) even when the listener is ready", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log); + const openBrowserUrl = vi.fn(() => { + log.events.push("open"); + return true; + }); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + expect(openBrowserUrl).not.toHaveBeenCalled(); + expect(log.events).not.toContain("open"); + + session.close(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("returns a typed unavailable lifecycle marker and never opens the browser when the listener cannot bind", async () => { + const log: CallLog = { events: [] }; + const server = makeUnavailableServer(log); + const openBrowserUrl = vi.fn(() => true); + + const session = await startLoopbackFlow({ + openBrowser: true, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + expect(session.type).toBe("unavailable"); + if (session.type === "unavailable") { + expect(session.lifecycle).toBe("listener_unavailable"); + } + expect(openBrowserUrl).not.toHaveBeenCalled(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("waitAndExchange exchanges the callback code with the same PKCE verifier and REDIRECT_URI", async () => { + const log: CallLog = { events: [] }; + const flow = makeFlow({ pkce: { verifier: "verifier-match", challenge: "chal" } }); + const server = makeReadyServer(log, { code: "code-match" }); + const exchangeAuthorizationCode = vi.fn(async () => { + log.events.push("exchange"); + return makeTokenResult({ access: "a", refresh: "r" }); + }); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => flow, + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode, + }, + }); + + assertReady(session); + const result = await session.waitAndExchange(); + expect(result.type).toBe("success"); + expect(exchangeAuthorizationCode).toHaveBeenCalledWith("code-match", "verifier-match", REDIRECT_URI); + expect(server.closeMock).toHaveBeenCalledTimes(1); + expect(log.events.filter((e) => e === "close")).toHaveLength(1); + }); + + it("maps a null callback (timeout / cancel) to a typed cancelled lifecycle marker and closes exactly once", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log, { code: null }); + const exchangeAuthorizationCode = vi.fn(async () => makeTokenResult()); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode, + }, + }); + + assertReady(session); + const result = await session.waitAndExchange(); + expect(result.type).toBe("cancelled"); + if (result.type === "cancelled") { + expect(result.lifecycle).toBe("callback_timeout_or_cancelled"); + } + expect(exchangeAuthorizationCode).not.toHaveBeenCalled(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("closes exactly once when waitForCode throws", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log, { waitThrows: new Error("wait boom") }); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + await expect(session.waitAndExchange()).rejects.toThrow("wait boom"); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("closes exactly once when exchangeAuthorizationCode throws", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log, { code: "code-x" }); + const exchangeAuthorizationCode = vi.fn(async () => { + throw new Error("exchange boom"); + }); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode, + }, + }); + + assertReady(session); + await expect(session.waitAndExchange()).rejects.toThrow("exchange boom"); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("session.close() before waitAndExchange closes the ready listener exactly once", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + session.close(); + session.close(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("waitAndExchange after an external session.close() does not double-close the listener", async () => { + const log: CallLog = { events: [] }; + const server = makeReadyServer(log, { code: null }); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + session.close(); + await session.waitAndExchange(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("starts callback observation at session creation, so an abandoned authorization still closes its listener", async () => { + // Given a ready session whose host never calls waitAndExchange + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + // When the listener's own deadline expires with the session abandoned + assertReady(session); + expect(server.waitMock).toHaveBeenCalledTimes(1); + expect(server.waitMock).toHaveBeenCalledWith("state-abc"); + server.resolveWith(null); + await vi.waitFor(() => expect(server.closeMock).toHaveBeenCalled()); + + // Then the port was released without the host ever awaiting the flow + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("returns cancellation without exchanging when the host calls back after expiry", async () => { + // Given a ready session whose callback observation already expired + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + const exchangeAuthorizationCode = vi.fn(async () => makeTokenResult()); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode, + }, + }); + + assertReady(session); + server.resolveWith(null); + await vi.waitFor(() => expect(server.closeMock).toHaveBeenCalled()); + + // When the host invokes its stored callback afterwards + const result = await session.waitAndExchange(); + + // Then it observes the settled cancellation and never exchanges + expect(result.type).toBe("cancelled"); + if (result.type === "cancelled") { + expect(result.lifecycle).toBe("callback_timeout_or_cancelled"); + } + expect(exchangeAuthorizationCode).not.toHaveBeenCalled(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("stores a wait rejection without an unhandled rejection and surfaces it on the later callback", async () => { + // Given a ready session whose wait rejects before the host calls back + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason); + }; + process.on("unhandledRejection", onUnhandled); + + try { + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + server.rejectWith(new Error("wait boom")); + await vi.waitFor(() => expect(server.closeMock).toHaveBeenCalled()); + await new Promise((resolve) => setImmediate(resolve)); + + // When the host invokes its stored callback afterwards + // Then the stored error surfaces and nothing went unhandled + await expect(session.waitAndExchange()).rejects.toThrow("wait boom"); + expect(unhandled).toEqual([]); + expect(server.closeMock).toHaveBeenCalledTimes(1); + } finally { + process.off("unhandledRejection", onUnhandled); + } + }); + + it("closes and reports browser_open_failed when the opener returns false", async () => { + // Given a ready listener whose default browser will not launch + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + const openBrowserUrl = vi.fn(() => false); + + // When the automatic-browser session starts + const session = await startLoopbackFlow({ + openBrowser: true, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + // Then it fails immediately with a typed marker and releases the port + expect(session.type).toBe("unavailable"); + if (session.type === "unavailable") { + expect(session.lifecycle).toBe("browser_open_failed"); + } + expect(openBrowserUrl).toHaveBeenCalledTimes(1); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("closes and reports browser_open_failed when the opener throws", async () => { + // Given a ready listener whose opener throws + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + const openBrowserUrl = vi.fn(() => { + throw new Error("spawn boom"); + }); + + // When the automatic-browser session starts + const session = await startLoopbackFlow({ + openBrowser: true, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + // Then the throw is mapped to the same typed marker and cleanup + expect(session.type).toBe("unavailable"); + if (session.type === "unavailable") { + expect(session.lifecycle).toBe("browser_open_failed"); + } + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("lets an external close win over a callback that arrives afterwards", async () => { + // Given a ready session the caller closes while the wait is pending + const log: CallLog = { events: [] }; + const server = makeDeferredServer(log); + const exchangeAuthorizationCode = vi.fn(async () => makeTokenResult()); + + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + createAuthorizationFlow: async () => makeFlow(), + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode, + }, + }); + + assertReady(session); + session.close(); + + // When a code arrives after that close and the host calls back + server.resolveWith({ code: "late-code" }); + const result = await session.waitAndExchange(); + + // Then cancellation wins, nothing is exchanged, and close ran once + expect(result.type).toBe("cancelled"); + expect(exchangeAuthorizationCode).not.toHaveBeenCalled(); + expect(server.closeMock).toHaveBeenCalledTimes(1); + }); + + it("forwards forceNewLogin to createAuthorizationFlow", async () => { + const log: CallLog = { events: [] }; + const createAuthorizationFlow = vi.fn(async (opts?: { forceNewLogin?: boolean }) => { + log.events.push(`createFlow:${opts?.forceNewLogin ?? false}`); + return makeFlow(); + }); + const server = makeReadyServer(log); + + const session = await startLoopbackFlow({ + openBrowser: false, + forceNewLogin: true, + deps: { + createAuthorizationFlow, + startLocalOAuthServer: async () => server, + openBrowserUrl: () => true, + exchangeAuthorizationCode: async () => makeTokenResult(), + }, + }); + + assertReady(session); + expect(createAuthorizationFlow).toHaveBeenCalledWith({ forceNewLogin: true }); + session.close(); + }); +}); diff --git a/test/oauth-server.integration.test.ts b/test/oauth-server.integration.test.ts index 4ff651f7..ee065228 100644 --- a/test/oauth-server.integration.test.ts +++ b/test/oauth-server.integration.test.ts @@ -5,15 +5,22 @@ import { describe, it, expect, afterEach } from "vitest"; import http from "node:http"; import { startLocalOAuthServer } from "../lib/auth/server.js"; +import { REDIRECT_URI } from "../lib/auth/auth.js"; +import { startLoopbackFlow } from "../lib/auth/loopback-flow.js"; describe("OAuth Server Integration", () => { let serverInfo: Awaited> | null = null; + let openSession: { close: () => void } | null = null; afterEach(() => { if (serverInfo) { serverInfo.close(); serverInfo = null; } + if (openSession) { + openSession.close(); + openSession = null; + } }); it("should start server and handle valid OAuth callback", async () => { @@ -108,4 +115,86 @@ describe("OAuth Server Integration", () => { serverInfo = null; // Prevent double-close in afterEach }); + + it("captures a provider-shaped redirect on the real listener without the host awaiting first", async () => { + // Given a ready manual-browser session on the real callback listener, + // with only the network exchange stubbed + const exchange = { code: "", verifier: "", redirectUri: "" }; + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + exchangeAuthorizationCode: async (code, verifier, redirectUri) => { + exchange.code = code; + exchange.verifier = verifier; + exchange.redirectUri = redirectUri ?? ""; + return { type: "success", access: "a", refresh: "r", expires: 1 }; + }, + }, + }); + if (session.type !== "ready") { + throw new Error(`expected ready session, got ${session.type}`); + } + openSession = session; + const state = new URL(session.url).searchParams.get("state"); + expect(state).toBeTruthy(); + + // When the browser follows the provider redirect back to loopback, in the + // parameter shape the real provider sends, before the host calls back + const providerCode = "ac_ZmFrZS1hdXRoLWNvZGU.ZmFrZS1zaWduYXR1cmU"; + const response = await fetch( + `http://127.0.0.1:1455/auth/callback?code=${providerCode}&scope=openid+profile+email+offline_access&state=${state}`, + ); + + // Then the eagerly started observation already holds it, and the exchange + // stays bound to this attempt's verifier and redirect URI + expect(response.status).toBe(200); + const result = await session.waitAndExchange(); + expect(result.type).toBe("success"); + expect(exchange.code).toBe(providerCode); + expect(exchange.redirectUri).toBe(REDIRECT_URI); + expect(exchange.verifier.length).toBeGreaterThanOrEqual(43); + + openSession = null; + }); + + it("releases the real port once the redirect lands, without the host ever calling back", async () => { + // Given a ready session the host takes a URL from and then abandons + const session = await startLoopbackFlow({ + openBrowser: false, + deps: { + exchangeAuthorizationCode: async () => { + throw new Error("exchange must not run for an abandoned session"); + }, + }, + }); + if (session.type !== "ready") { + throw new Error(`expected ready session, got ${session.type}`); + } + openSession = session; + const state = new URL(session.url).searchParams.get("state"); + + // When the browser delivers the callback and waitAndExchange is never called + const response = await fetch( + `http://127.0.0.1:1455/auth/callback?code=ac_abandoned.signature&scope=openid+profile+email+offline_access&state=${state}`, + ); + expect(response.status).toBe(200); + + // Then the observation started at session creation closes the listener on + // its own, so the port is free for the next attempt + await expect + .poll( + async () => { + try { + await fetch("http://127.0.0.1:1455/auth/callback?code=x&state=y"); + return "open"; + } catch { + return "closed"; + } + }, + { timeout: 5000 }, + ) + .toBe("closed"); + + openSession = null; + }); }); diff --git a/test/server.unit.test.ts b/test/server.unit.test.ts index 9e390b42..926cdd85 100644 --- a/test/server.unit.test.ts +++ b/test/server.unit.test.ts @@ -256,6 +256,20 @@ describe('OAuth Server Unit Tests', () => { }); describe('close function', () => { + it('keeps repeated close calls non-throwing', async () => { + // Given a ready callback server + const result = await startLocalOAuthServer({ state: 'test-state' }); + + // When the host closes it repeatedly + const closeTwice = () => { + result.close(); + result.close(); + }; + + // Then both close attempts remain safe + expect(closeTwice).not.toThrow(); + }); + it('should close all bound servers when ready=true', async () => { const result = await startLocalOAuthServer({ state: 'test-state' }); result.close(); @@ -289,6 +303,18 @@ describe('OAuth Server Unit Tests', () => { }); describe('waitForCode function', () => { + it('returns cancellation after the host closes a ready server', async () => { + // Given a ready callback server that the host closes + const result = await startLocalOAuthServer({ state: 'test-state' }); + result.close(); + + // When callback observation starts after closure + const code = await result.waitForCode('test-state'); + + // Then the server reports cancellation + expect(code).toBeNull(); + }); + it('should return null immediately when ready=false', async () => { mockHttp.__setListenBehavior('fail-all');