diff --git a/README.md b/README.md index f8eee3f..040acad 100644 --- a/README.md +++ b/README.md @@ -12,20 +12,20 @@ npm install @doist/cli-core ## What's in it -| Module | Key exports | Purpose | -| -------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `auth` (subpath) | `attachLoginCommand`, `attachLogoutCommand`, `attachStatusCommand`, `attachTokenViewCommand`, `runOAuthFlow`, `createPkceProvider`, `createSecureStore`, `createKeyringTokenStore`, `migrateLegacyAuth`, `persistBundle`, PKCE helpers, `AuthProvider` / `TokenStore` / `TokenBundle` / `RefreshInput` / `AccountRef` / `SecureStore` / `UserRecordStore` types, `AttachLogoutRevokeContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), a thin cross-platform OS-keyring wrapper (`createSecureStore`), and a multi-account keyring-backed `TokenStore` (`createKeyringTokenStore`) that stores secrets in the OS credential manager and degrades to plaintext in the consumer's config when the keyring is unavailable (WSL/headless Linux/containers). The store contract supports an optional `setBundle(account, bundle)` write method (required on `KeyringTokenStore`) so consumers that need refresh-token persistence can opt in via `TokenBundle`; `active()` stays narrow (access token + account only) so callers that don't need refresh state don't pay extra keyring IPC. `AuthProvider` and `TokenStore` remain the escape hatches for DCR or fully bespoke backends. `logout` / `status` / `token` always attach `--user ` and thread the parsed ref to `store.active(ref)` (and `store.clear(ref)` on `logout`). `commander` (when using the attachers), `open` (browser launch), and `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`) are optional peer/optional deps. | -| `commands` (subpath) | `registerChangelogCommand`, `registerUpdateCommand` (+ semver helpers) | Commander wiring for cli-core's standard commands (e.g. ` changelog`, ` update`, ` update switch`). **Requires** `commander` as an optional peer-dep. | -| `config` | `getConfigPath`, `readConfig`, `readConfigStrict`, `writeConfig`, `updateConfig`, `CoreConfig`, `UpdateChannel` | Read / write a per-CLI JSON config file with typed error codes; `CoreConfig` is the shape of fields cli-core itself owns (extend it for per-CLI fields). | -| `empty` | `printEmpty` | Print an empty-state message gated on `--json` / `--ndjson` so machine consumers never see human strings on stdout. | -| `errors` | `CliError` | Typed CLI error class with `code` and exit-code mapping. | -| `global-args` | `parseGlobalArgs`, `stripUserFlag`, `createGlobalArgsStore`, `createAccessibleGate`, `createSpinnerGate`, `getProgressJsonlPath`, `isProgressJsonlEnabled` | Parse well-known global flags (`--json`, `--ndjson`, `--quiet`, `--verbose`, `--accessible`, `--no-spinner`, `--progress-jsonl`, `--user `) and derive predicates from them. `stripUserFlag` removes `--user` tokens from argv so the cleaned array can be forwarded to Commander when the flag has no root-program attachment. | -| `json` | `formatJson`, `formatNdjson` | Stable JSON / newline-delimited JSON formatting for stdout. | -| `markdown` (subpath) | `preloadMarkdown`, `renderMarkdown`, `TerminalRendererOptions` | Lazy-init terminal markdown renderer. **Requires** `marked` and `marked-terminal-renderer` as peer-deps — install only if your CLI uses this subpath. | -| `options` | `ViewOptions` | Type contract for `{ json?, ndjson? }` per-command options that machine-output gates derive from. | -| `spinner` | `createSpinner` | Loading spinner factory wrapping `yocto-spinner` with disable gates. | -| `terminal` | `isCI`, `isStderrTTY`, `isStdinTTY`, `isStdoutTTY` | TTY / CI detection helpers. | -| `testing` (subpath) | `describeEmptyMachineOutput` | Vitest helpers reusable by consuming CLIs (e.g. parametrised empty-state suite covering `--json` / `--ndjson` / human modes). | +| Module | Key exports | Purpose | +| -------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `auth` (subpath) | `attachLoginCommand`, `attachLogoutCommand`, `attachStatusCommand`, `attachTokenViewCommand`, `runOAuthFlow`, `refreshAccessToken`, `createPkceProvider`, `createSecureStore`, `createKeyringTokenStore`, `migrateLegacyAuth`, `persistBundle`, `bundleFromExchange`, PKCE helpers, `AuthProvider` / `TokenStore` / `TokenBundle` / `ActiveBundleSnapshot` / `RefreshInput` / `AccountRef` / `SecureStore` / `UserRecordStore` types, `AttachLogoutRevokeContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), a thin cross-platform OS-keyring wrapper (`createSecureStore`), and a multi-account keyring-backed `TokenStore` (`createKeyringTokenStore`) that stores secrets in the OS credential manager and degrades to plaintext in the consumer's config when the keyring is unavailable (WSL/headless Linux/containers). The store contract supports an optional `setBundle(account, bundle)` write method (required on `KeyringTokenStore`) so consumers that need refresh-token persistence can opt in via `TokenBundle`; `active()` stays narrow (access token + account only) so callers that don't need refresh state don't pay extra keyring IPC. `AuthProvider` and `TokenStore` remain the escape hatches for DCR or fully bespoke backends. `logout` / `status` / `token` always attach `--user ` and thread the parsed ref to `store.active(ref)` (and `store.clear(ref)` on `logout`). `commander` (when using the attachers), `open` (browser launch), `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`), and `oauth4webapi` (when a consumer opts into silent refresh) are optional peer/optional deps. | +| `commands` (subpath) | `registerChangelogCommand`, `registerUpdateCommand` (+ semver helpers) | Commander wiring for cli-core's standard commands (e.g. ` changelog`, ` update`, ` update switch`). **Requires** `commander` as an optional peer-dep. | +| `config` | `getConfigPath`, `readConfig`, `readConfigStrict`, `writeConfig`, `updateConfig`, `CoreConfig`, `UpdateChannel` | Read / write a per-CLI JSON config file with typed error codes; `CoreConfig` is the shape of fields cli-core itself owns (extend it for per-CLI fields). | +| `empty` | `printEmpty` | Print an empty-state message gated on `--json` / `--ndjson` so machine consumers never see human strings on stdout. | +| `errors` | `CliError` | Typed CLI error class with `code` and exit-code mapping. | +| `global-args` | `parseGlobalArgs`, `stripUserFlag`, `createGlobalArgsStore`, `createAccessibleGate`, `createSpinnerGate`, `getProgressJsonlPath`, `isProgressJsonlEnabled` | Parse well-known global flags (`--json`, `--ndjson`, `--quiet`, `--verbose`, `--accessible`, `--no-spinner`, `--progress-jsonl`, `--user `) and derive predicates from them. `stripUserFlag` removes `--user` tokens from argv so the cleaned array can be forwarded to Commander when the flag has no root-program attachment. | +| `json` | `formatJson`, `formatNdjson` | Stable JSON / newline-delimited JSON formatting for stdout. | +| `markdown` (subpath) | `preloadMarkdown`, `renderMarkdown`, `TerminalRendererOptions` | Lazy-init terminal markdown renderer. **Requires** `marked` and `marked-terminal-renderer` as peer-deps — install only if your CLI uses this subpath. | +| `options` | `ViewOptions` | Type contract for `{ json?, ndjson? }` per-command options that machine-output gates derive from. | +| `spinner` | `createSpinner` | Loading spinner factory wrapping `yocto-spinner` with disable gates. | +| `terminal` | `isCI`, `isStderrTTY`, `isStdinTTY`, `isStdoutTTY` | TTY / CI detection helpers. | +| `testing` (subpath) | `describeEmptyMachineOutput` | Vitest helpers reusable by consuming CLIs (e.g. parametrised empty-state suite covering `--json` / `--ndjson` / human modes). | ## Usage @@ -169,6 +169,8 @@ attachLoginCommand(auth, { `attachLoginCommand` returns the new `Command` so you can chain `.description(...)` / `.option(...)` / `.addHelpText(...)`. Any consumer-attached options land in the `flags` object passed to `resolveScopes`, `onSuccess`, and the provider hooks. +The `authorizeUrl` / `tokenUrl` / `clientId` resolvers may return `string` **or** `Promise` — so a consumer can resolve the base URL or client id asynchronously (reading config, prompting the user) without abandoning `createPkceProvider`. An injected `fetchImpl` is used for the token exchange **and** the refresh grant (threaded into `oauth4webapi` via its `customFetch`), so a custom transport — proxy dispatcher, decompression — applies on every OAuth call rather than being bypassed by the library's global `fetch`. + #### Sibling attachers (`logout` / `status` / `token`) The same registrar shape covers the other three auth subcommands. Each returns the new `Command` for chaining and shares the same `TokenStore` instance. @@ -215,7 +217,7 @@ attachTokenViewCommand(auth, { `attachLogoutCommand` snapshots `store.active(ref)` when either `--user ` is supplied or one of the consumer hooks (`revokeToken` / `onCleared`) needs the prior account, calls `store.clear(ref)`, awaits `revokeToken({ token, account, ref, view, flags })` for best-effort server-side revocation, emits `✓ Logged out` (human) or `{ "ok": true }` (`--json`, silent under `--ndjson`), and finally fires `onCleared({ account, ref, view, flags })`. `ref` is the parsed `--user` argument (or `undefined`) so consumers can distinguish "nothing was stored" (`account: null`, `ref: undefined`) from "cleared an unreadable record by ref" (`account: null`, `ref: "me"`). `revokeToken` failures are always swallowed; the pre-flight snapshot's error contract is covered in the `--user ` section below. The exported `AttachLogoutRevokeContext` is the ctx type for typing standalone revoke implementations. -`attachStatusCommand` reads `store.active()`, optionally runs `fetchLive` (consumer translates 401 → `CliError('NO_TOKEN', …)`), then dispatches to `renderJson` (`--json` / `--ndjson` via `formatJson` / `formatNdjson`, defaults to the account itself, **only invoked in machine-output mode**) or `renderText` (human mode, string or array of lines). When the store is empty it throws `CliError('NOT_AUTHENTICATED', 'Not signed in.')` unless `onNotAuthenticated` is supplied. +`attachStatusCommand` reads the active credential — preferring `store.activeBundle` when `fetchLive` is supplied, so the access token and the full bundle come from a single keyring read — optionally runs `fetchLive` (consumer translates 401 → `CliError('NO_TOKEN', …)`), then dispatches to `renderJson` (`--json` / `--ndjson` via `formatJson` / `formatNdjson`, defaults to the account itself, **only invoked in machine-output mode**) or `renderText` (human mode, string or array of lines). When the store is empty it throws `CliError('NOT_AUTHENTICATED', 'Not signed in.')` unless `onNotAuthenticated` is supplied. `fetchLive` receives `{ account, token, bundle?, view, flags }` — `bundle` carries the refresh-side metadata (expiry, refresh token) when the store implements `activeBundle`, so a consumer can render expiry without a second read. Both attachers strip the standard `--json` / `--ndjson` / `--user` registrar flags from the parsed options and pass the remainder to their callbacks as `flags` — same escape hatch `attachLoginCommand` uses, so a consumer can chain e.g. `.option('--full')` and read it in `revokeToken` / `onCleared` / `renderText` / `fetchLive` / `renderJson` / `onNotAuthenticated`. @@ -292,9 +294,23 @@ For multi-account storage (OS keychain, per-user config slots, …), implement t Stores that target servers issuing refresh tokens may implement the optional `setBundle(account, bundle, options?)` method. `TokenBundle` carries `{ accessToken, refreshToken?, accessTokenExpiresAt?, refreshTokenExpiresAt? }`. Stores that omit `setBundle` continue to work — cli-core helpers (`persistBundle`) fall back to `set(account, bundle.accessToken)` and silently drop refresh state. `KeyringTokenStore` implements `setBundle` as a required override and routes the refresh token to a sibling keyring slot. -`active()` still returns the narrow `{ token, account }` snapshot — refresh-state material is stored but not surfaced on the hot read path so commands that only need the access token don't pay an extra keyring IPC. A bundle-aware read path lands alongside the silent-refresh helper in a follow-up PR. +`active()` stays narrow — `{ token, account }` only — so commands that only need the access token don't pay extra keyring IPC. The companion read method `activeBundle?(ref)` returns the full bundle (`{ account, bundle }`) and is the read path the silent-refresh helper requires. Optional on the contract; `KeyringTokenStore` implements it as a required override and parallel-reads both keyring slots, honouring the `hasRefreshToken: false` record gate to skip the refresh-slot IPC on access-only records. + +The `persistBundle({ store, account, bundle, promoteDefault? })` helper is the recommended write path for bundle-capable consumers — it prefers `setBundle` when available and falls back to `set` otherwise (the `set` fallback can't honour `promoteDefault`, so multi-account stores wanting silent-refresh-safe selection must implement `setBundle`). `runOAuthFlow` persists the full bundle through `persistBundle` with `promoteDefault: true`. + +##### Silent refresh (`refreshAccessToken`) + +`refreshAccessToken({ store, provider, lockPath, skewMs?, force?, ref?, handshake? })` rotates the access token using the stored refresh token. Use **proactively** before each authenticated call (skew defaults to 60s) or **reactively** with `force: true` after a 401. Persists the rotated bundle via `persistBundle` _without_ `promoteDefault` so a background rotation can't re-pin account selection. `handshake` (default `{}`) is forwarded to `provider.refreshToken` for resolvers that need runtime context (e.g. a `--env`-derived base URL). + +`lockPath` is caller-provided (cli-core doesn't interpret `~` or know where your config lives) — `O_EXCL` on that path serialises concurrent CLI invocations so only one POSTs and the rest re-read the rotated bundle. Recommended path: `${getConfigPath(serviceName)}.refresh.lock`. + +Error contract: + +- `AUTH_REFRESH_EXPIRED` — server rejected the refresh token (`invalid_grant`, including 400 and 401 since some reverse proxies remap). Caller should prompt re-login. +- `AUTH_REFRESH_TRANSIENT` — 5xx, network, non-JSON body, lock timeout. Caller may retry. +- `AUTH_REFRESH_UNAVAILABLE` — refresh isn't possible in the current setup: no refresh token stored, the store doesn't implement **both** `activeBundle` and `setBundle` (a full bundle must be readable and persistable), the credential was removed mid-refresh, the provider doesn't implement `refreshToken`, or the optional `oauth4webapi` peer dep isn't installed. -The `persistBundle({ store, account, bundle, promoteDefault? })` helper is the recommended write path for bundle-capable consumers — it prefers `setBundle` when available and falls back to `set` otherwise (the `set` fallback can't honour `promoteDefault`, so multi-account stores wanting silent-refresh-safe selection must implement `setBundle`). cli-core's own `runOAuthFlow` still persists via `store.set()` today; it switches to `persistBundle` when the refresh helper lands. +The PKCE provider (`createPkceProvider`) implements `refreshToken` via the [`oauth4webapi`](https://github.com/panva/oauth4webapi) library, declared as an **optional peer dependency** — only CLIs that opt into refresh need to install it (`npm install oauth4webapi`). Providers built directly against the `AuthProvider` interface (e.g. DCR, device code) implement the `refreshToken?` hook themselves; the storage and helper contract is identical. #### Keyring primitive (`createSecureStore`) diff --git a/package-lock.json b/package-lock.json index 2a36f8a..d619bf8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,6 +22,7 @@ "lefthook": "2.1.6", "marked": "18.0.3", "marked-terminal-renderer": "2.2.0", + "oauth4webapi": "3.8.6", "open": "11.0.0", "oxfmt": "0.49.0", "oxlint": "1.64.0", @@ -39,6 +40,7 @@ "commander": ">=14", "marked": ">=18", "marked-terminal-renderer": ">=2", + "oauth4webapi": ">=3", "open": ">=10", "vitest": ">=4.1" }, @@ -52,6 +54,9 @@ "marked-terminal-renderer": { "optional": true }, + "oauth4webapi": { + "optional": true + }, "open": { "optional": true }, @@ -7919,6 +7924,16 @@ "node": ">=18" } }, + "node_modules/oauth4webapi": { + "version": "3.8.6", + "resolved": "https://registry.npmjs.org/oauth4webapi/-/oauth4webapi-3.8.6.tgz", + "integrity": "sha512-iwemM91xz8nryHti2yTmg5fhyEMVOkOXwHNqbvcATjyajb5oQxCQzrNOA6uElRHuMhQQTKUyFKV9y/CNyg25BQ==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", diff --git a/package.json b/package.json index 6a9c428..6aaec21 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "lefthook": "2.1.6", "marked": "18.0.3", "marked-terminal-renderer": "2.2.0", + "oauth4webapi": "3.8.6", "open": "11.0.0", "oxfmt": "0.49.0", "oxlint": "1.64.0", @@ -90,6 +91,7 @@ "commander": ">=14", "marked": ">=18", "marked-terminal-renderer": ">=2", + "oauth4webapi": ">=3", "open": ">=10", "vitest": ">=4.1" }, @@ -103,6 +105,9 @@ "marked-terminal-renderer": { "optional": true }, + "oauth4webapi": { + "optional": true + }, "open": { "optional": true }, diff --git a/src/auth/errors.ts b/src/auth/errors.ts index bf77311..439e987 100644 --- a/src/auth/errors.ts +++ b/src/auth/errors.ts @@ -10,6 +10,9 @@ export type AuthErrorCode = | 'AUTH_TOKEN_EXCHANGE_FAILED' | 'AUTH_STORE_WRITE_FAILED' | 'AUTH_STORE_READ_FAILED' + | 'AUTH_REFRESH_EXPIRED' + | 'AUTH_REFRESH_TRANSIENT' + | 'AUTH_REFRESH_UNAVAILABLE' | 'NOT_AUTHENTICATED' | 'TOKEN_FROM_ENV' | 'NO_ACCOUNT_SELECTED' diff --git a/src/auth/flow.test.ts b/src/auth/flow.test.ts index a510304..1d37bf5 100644 --- a/src/auth/flow.test.ts +++ b/src/auth/flow.test.ts @@ -22,7 +22,7 @@ import { execFile } from 'node:child_process' import { readFileSync } from 'node:fs' import openBrowserModule from 'open' import { type RunOAuthFlowOptions, runOAuthFlow } from './flow.js' -import type { AuthProvider, TokenStore } from './types.js' +import type { AuthProvider, TokenBundle, TokenStore } from './types.js' type Account = { id: string; label?: string; email: string } @@ -360,6 +360,87 @@ describe('runOAuthFlow', () => { expect(result.token).toBe('tok-1') }) + it('persists the full bundle via setBundle when the store implements it (promoteDefault: true)', async () => { + const setBundle = vi.fn( + async ( + _account: Account, + _bundle: TokenBundle, + _options?: { promoteDefault?: boolean }, + ) => undefined, + ) + const set = vi.fn(async () => undefined) + const store: TokenStore = { + async active() { + return null + }, + set, + setBundle, + async clear() {}, + async list() { + return [] + }, + async setDefault() {}, + } + const { provider, getRedirect } = instrument({ + exchangeCode: async () => ({ + accessToken: 'tok-1', + refreshToken: 'r-1', + expiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_800_000_000_000, + }), + }) + + await runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: driveCallback(getRedirect), + onAuthorizeUrl: () => undefined, + }), + ) + + expect(set).not.toHaveBeenCalled() + expect(setBundle).toHaveBeenCalledTimes(1) + const [, persistedBundle, opts] = setBundle.mock.calls[0] + expect(persistedBundle).toEqual({ + accessToken: 'tok-1', + refreshToken: 'r-1', + accessTokenExpiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_800_000_000_000, + }) + expect(opts).toEqual({ promoteDefault: true }) + }) + + it('falls back to set(accessToken) when the store does not implement setBundle (refresh state dropped)', async () => { + const set = vi.fn(async () => undefined) + const store: TokenStore = { + async active() { + return null + }, + set, + async clear() {}, + async list() { + return [] + }, + async setDefault() {}, + } + const { provider, getRedirect } = instrument({ + exchangeCode: async () => ({ accessToken: 'tok-1', refreshToken: 'r-1' }), + }) + + const result = await runOAuthFlow( + flowOptions({ + provider, + store, + openBrowser: driveCallback(getRedirect), + onAuthorizeUrl: () => undefined, + }), + ) + + expect(result.token).toBe('tok-1') + expect(set).toHaveBeenCalledWith(expect.objectContaining({ id: '1' }), 'tok-1') + }) + it('wraps non-CliError store.set failures in AUTH_STORE_WRITE_FAILED', async () => { const { provider, getRedirect } = instrument() const store: TokenStore = { diff --git a/src/auth/flow.ts b/src/auth/flow.ts index d95611b..7a09237 100644 --- a/src/auth/flow.ts +++ b/src/auth/flow.ts @@ -4,6 +4,7 @@ import { type IncomingMessage, type Server, type ServerResponse, createServer } import { promisify } from 'node:util' import { CliError, getErrorMessage } from '../errors.js' import { isStdoutTTY } from '../terminal.js' +import { bundleFromExchange, persistBundle } from './persist.js' import { generateState } from './pkce.js' import type { AuthAccount, AuthProvider, TokenStore } from './types.js' @@ -187,15 +188,12 @@ export async function runOAuthFlow( })) checkAborted() - try { - await options.store.set(account, exchange.accessToken) - } catch (error) { - if (error instanceof CliError) throw error - throw new CliError( - 'AUTH_STORE_WRITE_FAILED', - `Failed to persist token: ${getErrorMessage(error)}`, - ) - } + await persistBundle({ + store: options.store, + account, + bundle: bundleFromExchange(exchange), + promoteDefault: true, + }) return { token: exchange.accessToken, account } } finally { diff --git a/src/auth/index.ts b/src/auth/index.ts index e0595f6..4616185 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -20,12 +20,15 @@ export { generateVerifier, } from './pkce.js' export type { GenerateVerifierOptions } from './pkce.js' -export { persistBundle } from './persist.js' +export { bundleFromExchange, persistBundle } from './persist.js' export type { PersistBundleOptions } from './persist.js' export { createPkceProvider } from './providers/pkce.js' export type { PkceLazyString, PkceProviderOptions } from './providers/pkce.js' +export { refreshAccessToken } from './refresh.js' +export type { RefreshAccessTokenOptions, RefreshAccessTokenResult } from './refresh.js' export type { AccountRef, + ActiveBundleSnapshot, AuthAccount, AuthorizeInput, AuthorizeResult, diff --git a/src/auth/keyring/internal.test.ts b/src/auth/keyring/internal.test.ts new file mode 100644 index 0000000..4afbf52 --- /dev/null +++ b/src/auth/keyring/internal.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it, vi } from 'vitest' + +import type { AuthAccount } from '../types.js' +import { readRefreshTokenForRecord } from './internal.js' +import type { SecureStore } from './secure-store.js' +import type { UserRecord } from './types.js' + +type Account = AuthAccount & { id: string } +const account: Account = { id: '42' } + +function fakeStore(impl: Partial): SecureStore { + return { + async getSecret() { + return null + }, + async setSecret() {}, + async deleteSecret() { + return false + }, + ...impl, + } +} + +describe('readRefreshTokenForRecord', () => { + it('short-circuits to not-present when hasRefreshToken is false', async () => { + const getSpy = vi.fn(async () => 'should-not-be-read') + const store = fakeStore({ getSecret: getSpy }) + const record: UserRecord = { account, hasRefreshToken: false } + + const outcome = await readRefreshTokenForRecord(record, store) + expect(outcome).toEqual({ ok: false, reason: 'not-present' }) + expect(getSpy).not.toHaveBeenCalled() + }) + + it('returns fallbackRefreshToken in preference to a (possibly stale) keyring slot', async () => { + const getSpy = vi.fn(async () => 'stale') + const store = fakeStore({ getSecret: getSpy }) + const record: UserRecord = { + account, + hasRefreshToken: true, + fallbackRefreshToken: 'plaintext_fallback', + } + + const outcome = await readRefreshTokenForRecord(record, store) + expect(outcome).toEqual({ ok: true, token: 'plaintext_fallback' }) + expect(getSpy).not.toHaveBeenCalled() + }) +}) diff --git a/src/auth/keyring/internal.ts b/src/auth/keyring/internal.ts index 8145f75..a9511fc 100644 --- a/src/auth/keyring/internal.ts +++ b/src/auth/keyring/internal.ts @@ -13,6 +13,18 @@ export type ReadAccessTokenOutcome = | { ok: true; token: string } | { ok: false; reason: 'slot-empty' | 'slot-unavailable' | 'slot-error'; detail: string } +/** + * Outcome of resolving the refresh token for a record. Mirrors + * `ReadAccessTokenOutcome`, plus an extra `not-present` variant for records + * the store knows carry no refresh state (`hasRefreshToken: false`) — the + * gate lets `activeBundle` skip the slot IPC entirely on access-only + * records. + */ +export type ReadRefreshTokenOutcome = + | { ok: true; token: string } + | { ok: false; reason: 'not-present' } + | { ok: false; reason: 'slot-empty' | 'slot-unavailable' | 'slot-error'; detail: string } + /** * `fallbackToken` first (so an offline-keyring write is preferred over a * stale slot), then the keyring slot. Single-source for "is this record @@ -25,16 +37,22 @@ export async function readAccessTokenForRecord( ): Promise { const fallback = record.fallbackToken?.trim() if (fallback) return { ok: true, token: fallback } + return readSecretSlot(secureStore, 'keyring slot returned no credential') +} +/** + * Read + trim a keyring slot, normalising the empty / unavailable / error + * cases. The per-record concerns (fallback field, the refresh `not-present` + * gate, the empty-slot detail string) stay in the callers. + */ +async function readSecretSlot( + store: SecureStore, + emptyDetail: string, +): Promise { try { - const raw = await secureStore.getSecret() - const trimmed = raw?.trim() + const trimmed = (await store.getSecret())?.trim() if (trimmed) return { ok: true, token: trimmed } - return { - ok: false, - reason: 'slot-empty', - detail: 'keyring slot returned no credential', - } + return { ok: false, reason: 'slot-empty', detail: emptyDetail } } catch (error) { if (error instanceof SecureStoreUnavailableError) { return { ok: false, reason: 'slot-unavailable', detail: error.message } @@ -42,3 +60,21 @@ export async function readAccessTokenForRecord( return { ok: false, reason: 'slot-error', detail: getErrorMessage(error) } } } + +/** + * Refresh-side analogue of `readAccessTokenForRecord`. Honours the + * `hasRefreshToken: false` gate — a record that knows it has no refresh + * material short-circuits to `not-present` without touching the keyring. + * Legacy records (`hasRefreshToken === undefined`) probe the slot once. + */ +export async function readRefreshTokenForRecord( + record: UserRecord, + refreshStore: SecureStore, +): Promise { + if (record.hasRefreshToken === false) return { ok: false, reason: 'not-present' } + + const fallback = record.fallbackRefreshToken?.trim() + if (fallback) return { ok: true, token: fallback } + + return readSecretSlot(refreshStore, 'keyring refresh slot returned no credential') +} diff --git a/src/auth/keyring/token-store.test.ts b/src/auth/keyring/token-store.test.ts index 6bdb556..4e04975 100644 --- a/src/auth/keyring/token-store.test.ts +++ b/src/auth/keyring/token-store.test.ts @@ -506,6 +506,84 @@ describe('createKeyringTokenStore', () => { expect(state.records.get('42')?.hasRefreshToken).toBe(false) }) + it('activeBundle round-trips the persisted bundle (account + access + refresh + expiry)', async () => { + const { km, store } = mapFixture() + await store.setBundle(account, bundle, { promoteDefault: true }) + + const snapshot = await store.activeBundle() + expect(snapshot).toEqual({ + account, + bundle: { + accessToken: 'tok_a', + refreshToken: 'tok_r', + accessTokenExpiresAt: 1_700_000_000_000, + }, + }) + // Sanity: both slots read, refresh slot was actually populated. + expect(km.slots.get(refreshAccountSlot('user-42'))?.secret).toBe('tok_r') + }) + + it('activeBundle skips the refresh-slot IPC when hasRefreshToken is false', async () => { + const { km, store } = mapFixture({ + '42': { account, hasRefreshToken: false }, + }) + km.slots.set('user-42', { secret: 'tok_a' }) + + const snapshot = await store.activeBundle() + expect(snapshot?.bundle).toEqual({ accessToken: 'tok_a' }) + // Refresh slot must not have been read (no IPC). + expect(km.getCalls.get(refreshAccountSlot('user-42'))).toBeUndefined() + }) + + it('activeBundle returns a refresh-less bundle when the refresh slot is unavailable', async () => { + // Legacy record (no hasRefreshToken gate) where the refresh slot + // is offline (SecureStoreUnavailableError). The bundle returns + // without refreshToken; the silent-refresh helper translates + // that to AUTH_REFRESH_UNAVAILABLE. + const km = buildKeyringMap() + mockedCreateSecureStore.mockImplementation(km.create) + const harness = buildUserRecords() + harness.state.records.set('42', { account }) + km.slots.set('user-42', { secret: 'tok_a' }) + km.slots.set(refreshAccountSlot('user-42'), { + secret: null, + getErr: new SecureStoreUnavailableError('locked'), + }) + + const store = createKeyringTokenStore({ + serviceName: SERVICE, + userRecords: harness.store, + recordsLocation: LOCATION, + }) + + const snapshot = await store.activeBundle() + expect(snapshot?.bundle).toEqual({ accessToken: 'tok_a' }) + }) + + it('activeBundle rejects AUTH_STORE_READ_FAILED on a non-keyring refresh-slot error', async () => { + // A generic (non-SecureStoreUnavailable) refresh-slot read error is + // a real fault, not a degrade-to-access-only case — surface it. + const km = buildKeyringMap() + mockedCreateSecureStore.mockImplementation(km.create) + const harness = buildUserRecords() + harness.state.records.set('42', { account }) + km.slots.set('user-42', { secret: 'tok_a' }) + km.slots.set(refreshAccountSlot('user-42'), { + secret: null, + getErr: new Error('disk fried'), + }) + + const store = createKeyringTokenStore({ + serviceName: SERVICE, + userRecords: harness.store, + recordsLocation: LOCATION, + }) + + await expect(store.activeBundle()).rejects.toMatchObject({ + code: 'AUTH_STORE_READ_FAILED', + }) + }) + it('clear() wipes both keyring slots', async () => { const { km, store, state } = mapFixture({ '42': { account, hasRefreshToken: true }, diff --git a/src/auth/keyring/token-store.ts b/src/auth/keyring/token-store.ts index 8fb61bd..a8cc5c4 100644 --- a/src/auth/keyring/token-store.ts +++ b/src/auth/keyring/token-store.ts @@ -1,7 +1,17 @@ import { CliError } from '../../errors.js' -import type { AccountRef, AuthAccount, TokenBundle, TokenStore } from '../types.js' +import type { + AccountRef, + ActiveBundleSnapshot, + AuthAccount, + TokenBundle, + TokenStore, +} from '../types.js' import { accountNotFoundError } from '../user-flag.js' -import { readAccessTokenForRecord } from './internal.js' +import { + type ReadAccessTokenOutcome, + readAccessTokenForRecord, + readRefreshTokenForRecord, +} from './internal.js' import { writeBundleWithKeyringFallback, writeRecordWithKeyringFallback } from './record-write.js' import { createSecureStore, @@ -47,6 +57,12 @@ export type KeyringTokenStore = TokenStore + /** + * Override `activeBundle` as required (not optional) — the keyring store + * always knows how to read refresh state. Lets cli-core helpers + * (`refreshAccessToken`) call it without a non-null assertion. + */ + activeBundle(ref?: AccountRef): Promise | null> /** Storage result from the most recent `set()` / `setBundle()` call, or `undefined` before any (and reset to `undefined` when the most recent write threw). */ getLastStorageResult(): TokenStorageResult | undefined /** Storage result from the most recent `clear()` call, or `undefined` before any (and reset to `undefined` when the most recent `clear()` threw or was a no-op). */ @@ -58,6 +74,16 @@ const DEFAULT_MATCH_ACCOUNT = ( ref: AccountRef, ): boolean => account.id === ref || account.label === ref +function accessReadError(outcome: Extract): CliError { + const message = + outcome.reason === 'slot-empty' + ? `${SECURE_STORE_DESCRIPTION} returned no credential for the stored account; the keyring entry may have been removed externally.` + : outcome.reason === 'slot-unavailable' + ? `${SECURE_STORE_DESCRIPTION} unavailable; could not read stored token (${outcome.detail})` + : `Access-slot read failed (${outcome.detail})` + return new CliError('AUTH_STORE_READ_FAILED', message) +} + /** * Multi-account `TokenStore` that keeps secrets in the OS credential manager * and per-user metadata in the consumer's `UserRecordStore`. Falls back to a @@ -206,33 +232,70 @@ export function createKeyringTokenStore( } } + /** + * Resolve the target record for `ref` (or the implicit default). Shared by + * `active` / `activeBundle`. The ref-only path skips `getDefaultId()` — + * `resolveTarget` never consults it when `ref` is supplied, so the extra + * read would be pure latency on every authenticated command. + */ + async function resolveRecord( + ref: AccountRef | undefined, + ): Promise | null> { + const snapshot: Snapshot = + ref === undefined + ? await readFullSnapshot() + : { records: await userRecords.list(), defaultId: null } + return resolveTarget(snapshot, ref) + } + return { async active(ref) { - // Ref-only path skips `getDefaultId()` — `resolveTarget` never - // touches it when `ref` is supplied, so the extra read would be - // pure latency on every authenticated command. - const snapshot: Snapshot = - ref === undefined - ? await readFullSnapshot() - : { records: await userRecords.list(), defaultId: null } - const record = resolveTarget(snapshot, ref) + const record = await resolveRecord(ref) if (!record) return null - // Reads the access slot only. Refresh-state material lives in - // the keyring and on the record, but `active()` stays cheap and - // returns the pre-PR1 snapshot shape — a future bundle-aware - // read path lights up the refresh slot only when callers - // actually need it (silent refresh). + // Reads the access slot only. Refresh-state material lives in the + // keyring + on the record, but `active()` stays cheap and returns + // the narrow snapshot shape — `activeBundle` lights up the refresh + // slot only when a caller actually needs it (silent refresh). const outcome = await readAccessTokenForRecord(record, secureStoreFor(record.account)) if (outcome.ok) return { token: outcome.token, account: record.account } - // Map structured outcomes to the typed error contract. - const message = - outcome.reason === 'slot-empty' - ? `${SECURE_STORE_DESCRIPTION} returned no credential for the stored account; the keyring entry may have been removed externally.` - : outcome.reason === 'slot-unavailable' - ? `${SECURE_STORE_DESCRIPTION} unavailable; could not read stored token (${outcome.detail})` - : `Access-slot read failed (${outcome.detail})` - throw new CliError('AUTH_STORE_READ_FAILED', message) + throw accessReadError(outcome) + }, + + async activeBundle(ref) { + const record = await resolveRecord(ref) + if (!record) return null + + // Read both slots in parallel. The refresh side honours the + // `hasRefreshToken: false` gate inside `readRefreshTokenForRecord`, + // so an access-only record short-circuits without a refresh-slot + // IPC. Access-slot failures map to the same typed error as + // `active()`; a refresh-slot failure degrades silently — the + // bundle returns without `refreshToken` and the silent-refresh + // helper translates that to `AUTH_REFRESH_UNAVAILABLE`. + const [accessOutcome, refreshOutcome] = await Promise.all([ + readAccessTokenForRecord(record, secureStoreFor(record.account)), + readRefreshTokenForRecord(record, refreshSecureStoreFor(record.account)), + ]) + if (!accessOutcome.ok) throw accessReadError(accessOutcome) + if (refreshOutcome.ok === false && refreshOutcome.reason === 'slot-error') { + throw new CliError( + 'AUTH_STORE_READ_FAILED', + `Refresh-slot read failed (${refreshOutcome.detail})`, + ) + } + + const bundle: TokenBundle = { + accessToken: accessOutcome.token, + ...(refreshOutcome.ok ? { refreshToken: refreshOutcome.token } : {}), + ...(record.accessTokenExpiresAt !== undefined + ? { accessTokenExpiresAt: record.accessTokenExpiresAt } + : {}), + ...(record.refreshTokenExpiresAt !== undefined + ? { refreshTokenExpiresAt: record.refreshTokenExpiresAt } + : {}), + } + return { account: record.account, bundle } }, async set(account, token) { diff --git a/src/auth/persist.ts b/src/auth/persist.ts index df5f389..9a3e08f 100644 --- a/src/auth/persist.ts +++ b/src/auth/persist.ts @@ -1,5 +1,5 @@ import { CliError, getErrorMessage } from '../errors.js' -import type { AuthAccount, TokenBundle, TokenStore } from './types.js' +import type { AuthAccount, ExchangeResult, TokenBundle, TokenStore } from './types.js' export type PersistBundleOptions = { store: TokenStore @@ -22,6 +22,30 @@ export type PersistBundleOptions = { * silent-refresh-safe selection (no re-pinning on background rotation) * MUST implement `setBundle`. */ +/** + * Build a `TokenBundle` from an `ExchangeResult`. `prev` carries the previous + * refresh token + its expiry forward when the server didn't reissue one (most + * don't on a refresh-grant). When the server DID return a new refresh token, + * its expiry is whatever the server gave (or unknown) — never the old token's, + * which would attach stale expiry metadata to a different credential. + */ +export function bundleFromExchange( + exchange: ExchangeResult, + prev?: TokenBundle, +): TokenBundle { + const rotatedRefresh = exchange.refreshToken !== undefined + const refreshToken = exchange.refreshToken ?? prev?.refreshToken + const refreshTokenExpiresAt = rotatedRefresh + ? exchange.refreshTokenExpiresAt + : (exchange.refreshTokenExpiresAt ?? prev?.refreshTokenExpiresAt) + return { + accessToken: exchange.accessToken, + ...(refreshToken !== undefined ? { refreshToken } : {}), + ...(exchange.expiresAt !== undefined ? { accessTokenExpiresAt: exchange.expiresAt } : {}), + ...(refreshTokenExpiresAt !== undefined ? { refreshTokenExpiresAt } : {}), + } +} + export async function persistBundle( options: PersistBundleOptions, ): Promise { diff --git a/src/auth/providers/pkce.test.ts b/src/auth/providers/pkce.test.ts index 6bfebbf..f81148a 100644 --- a/src/auth/providers/pkce.test.ts +++ b/src/auth/providers/pkce.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { createPkceProvider } from './pkce.js' @@ -62,6 +62,26 @@ describe('createPkceProvider', () => { expect(url.searchParams.get('scope')).toBe('data:read_write,data:delete') }) + it('supports async resolvers (consumer resolves base URL / client id asynchronously)', async () => { + const provider = createPkceProvider({ + authorizeUrl: async ({ handshake }) => `${handshake.baseUrl as string}/oauth/authorize`, + tokenUrl: 'unused', + clientId: async () => 'async-client', + validate, + }) + const result = await provider.authorize({ + redirectUri: 'http://localhost/callback', + state: 's', + scopes: [], + readOnly: false, + flags: {}, + handshake: { baseUrl: 'https://async.example' }, + }) + const url = new URL(result.authorizeUrl) + expect(url.origin).toBe('https://async.example') + expect(url.searchParams.get('client_id')).toBe('async-client') + }) + it('exchangeCode POSTs without client_secret and surfaces token endpoint failures as AUTH_TOKEN_EXCHANGE_FAILED', async () => { const ok = createPkceProvider({ authorizeUrl: 'unused', @@ -145,3 +165,177 @@ describe('createPkceProvider', () => { ).rejects.toMatchObject({ code: 'AUTH_TOKEN_EXCHANGE_FAILED' }) }) }) + +describe('createPkceProvider.refreshToken', () => { + function refreshProvider() { + return createPkceProvider({ + authorizeUrl: 'https://example.com/oauth/authorize', + tokenUrl: 'https://example.com/oauth/token', + clientId: 'client-xyz', + validate, + }) + } + + function stubFetch(impl: typeof fetch): void { + vi.spyOn(globalThis, 'fetch').mockImplementation(impl) + } + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('POSTs the refresh grant and returns the rotated bundle', async () => { + stubFetch( + (async () => + new Response( + JSON.stringify({ + access_token: 'tok-new', + refresh_token: 'r-new', + expires_in: 3600, + token_type: 'bearer', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + )) as typeof fetch, + ) + + const result = await refreshProvider().refreshToken!({ + refreshToken: 'r-old', + handshake: {}, + }) + + expect(result).toMatchObject({ accessToken: 'tok-new', refreshToken: 'r-new' }) + expect(result.expiresAt).toBeGreaterThan(Date.now()) + }) + + it('routes the refresh grant through an injected fetchImpl, never the global fetch', async () => { + // Custom-transport consumers (proxy dispatcher) inject `fetchImpl`; + // it must reach oauth4webapi's customFetch. Global fetch is rigged to + // throw so any leak to it fails loudly. + let capturedUrl: string | undefined + const fetchImpl = vi.fn(async (input: RequestInfo | URL) => { + capturedUrl = String(input) + return new Response(JSON.stringify({ access_token: 'tok-new', token_type: 'bearer' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + }) as unknown as typeof fetch + const globalSpy = vi.spyOn(globalThis, 'fetch').mockImplementation((() => { + throw new Error('global fetch must not be used when fetchImpl is injected') + }) as typeof fetch) + + const provider = createPkceProvider({ + authorizeUrl: 'https://example.com/oauth/authorize', + tokenUrl: 'https://example.com/oauth/token', + clientId: 'client-xyz', + validate, + fetchImpl, + }) + + const result = await provider.refreshToken!({ refreshToken: 'r-old', handshake: {} }) + + expect(result.accessToken).toBe('tok-new') + expect(capturedUrl).toBe('https://example.com/oauth/token') + expect(globalSpy).not.toHaveBeenCalled() + }) + + it('resolves async tokenUrl / clientId from the handshake on the refresh path', async () => { + let capturedUrl: string | undefined + let capturedClientId: string | undefined + const fetchImpl = vi.fn(async (input: RequestInfo | URL, init: RequestInit = {}) => { + capturedUrl = String(input) + capturedClientId = + new URLSearchParams(init.body as string).get('client_id') ?? undefined + return new Response(JSON.stringify({ access_token: 'tok-new', token_type: 'bearer' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + }) as unknown as typeof fetch + + const provider = createPkceProvider({ + authorizeUrl: 'unused', + tokenUrl: async ({ handshake }) => `${handshake.baseUrl as string}/oauth/token`, + clientId: async () => 'async-client', + validate, + fetchImpl, + }) + + const result = await provider.refreshToken!({ + refreshToken: 'r-old', + handshake: { baseUrl: 'https://wiki.example.com' }, + }) + + expect(result.accessToken).toBe('tok-new') + expect(capturedUrl).toBe('https://wiki.example.com/oauth/token') + expect(capturedClientId).toBe('async-client') + }) + + it('maps invalid_grant to AUTH_REFRESH_EXPIRED (any HTTP status — proxies remap 400/401)', async () => { + stubFetch( + (async () => + new Response(JSON.stringify({ error: 'invalid_grant' }), { + status: 401, + headers: { 'Content-Type': 'application/json' }, + })) as typeof fetch, + ) + + await expect( + refreshProvider().refreshToken!({ refreshToken: 'r-old', handshake: {} }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_EXPIRED' }) + }) + + it('maps everything else (network, 5xx, non-JSON, other OAuth errors) to AUTH_REFRESH_TRANSIENT', async () => { + stubFetch((async () => { + throw new Error('connection reset') + }) as typeof fetch) + + await expect( + refreshProvider().refreshToken!({ refreshToken: 'r-old', handshake: {} }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_TRANSIENT' }) + }) + + it('surfaces the server OAuth error code + description (not oauth4webapi’s generic message)', async () => { + stubFetch( + (async () => + new Response( + JSON.stringify({ + error: 'invalid_request', + error_description: 'Missing client_secret for confidential client', + }), + { status: 400, headers: { 'Content-Type': 'application/json' } }, + )) as typeof fetch, + ) + + await expect( + refreshProvider().refreshToken!({ refreshToken: 'r-old', handshake: {} }), + ).rejects.toMatchObject({ + code: 'AUTH_REFRESH_TRANSIENT', + message: + 'Refresh request failed: invalid_request (Missing client_secret for confidential client)', + }) + }) + + it('maps a missing oauth4webapi peer dep to AUTH_REFRESH_UNAVAILABLE', async () => { + // The optional peer dep isn't installed → the lazy import fails. Force + // that by mocking the module to throw, then re-importing the provider + // so its lazy `import('oauth4webapi')` resolves to the throwing mock. + vi.resetModules() + vi.doMock('oauth4webapi', () => { + throw new Error("Cannot find package 'oauth4webapi'") + }) + try { + const { createPkceProvider: freshCreate } = await import('./pkce.js') + const provider = freshCreate({ + authorizeUrl: 'https://example.com/oauth/authorize', + tokenUrl: 'https://example.com/oauth/token', + clientId: 'client-xyz', + validate, + }) + await expect( + provider.refreshToken!({ refreshToken: 'r-old', handshake: {} }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_UNAVAILABLE' }) + } finally { + vi.doUnmock('oauth4webapi') + vi.resetModules() + } + }) +}) diff --git a/src/auth/providers/pkce.ts b/src/auth/providers/pkce.ts index 2e641a7..9c52f85 100644 --- a/src/auth/providers/pkce.ts +++ b/src/auth/providers/pkce.ts @@ -1,3 +1,4 @@ +import type { AuthorizationServer, Client, TokenEndpointRequestOptions } from 'oauth4webapi' import { CliError, getErrorMessage } from '../../errors.js' import { deriveChallenge, generateVerifier } from '../pkce.js' import type { @@ -7,9 +8,19 @@ import type { AuthorizeResult, ExchangeInput, ExchangeResult, + RefreshInput, ValidateInput, } from '../types.js' +// Upper bound on the refresh-token POST. Kept under the refresh helper's +// stale-lock threshold so a timed-out grant releases the lock before another +// invocation would consider it abandoned. +const REFRESH_TIMEOUT_MS = 10_000 + +function expiresAtFromExpiresIn(expiresIn: number | undefined): number | undefined { + return typeof expiresIn === 'number' ? Date.now() + expiresIn * 1000 : undefined +} + /** * Lazy resolver: a literal string, or a function that builds one from the * current PKCE handshake (so callers can derive the URL or client_id from @@ -17,7 +28,10 @@ import type { */ export type PkceLazyString = | string - | ((ctx: { handshake: Record; flags: Record }) => string) + | ((ctx: { + handshake: Record + flags: Record + }) => string | Promise) export type PkceProviderOptions = { /** OAuth 2.0 authorize endpoint. Function form supports per-flow base URLs (Outline self-hosted). */ @@ -63,8 +77,10 @@ export function createPkceProvider( length: options.verifierLength, }) const challenge = deriveChallenge(verifier) - const clientId = resolve(options.clientId, input.handshake, input.flags) - const authorizeUrl = resolve(options.authorizeUrl, input.handshake, input.flags) + const [clientId, authorizeUrl] = await Promise.all([ + resolve(options.clientId, input.handshake, input.flags), + resolve(options.authorizeUrl, input.handshake, input.flags), + ]) const url = new URL(authorizeUrl) url.searchParams.set('response_type', 'code') @@ -96,7 +112,7 @@ export function createPkceProvider( // before calling exchange, so a `tokenUrl: ({ flags }) => ...` // resolver sees the same flags it saw during authorize. const flags = (input.handshake.flags as Record | undefined) ?? {} - const tokenUrl = resolve(options.tokenUrl, input.handshake, flags) + const tokenUrl = await resolve(options.tokenUrl, input.handshake, flags) const body = new URLSearchParams({ grant_type: 'authorization_code', @@ -152,25 +168,123 @@ export function createPkceProvider( return { accessToken: payload.access_token, refreshToken: payload.refresh_token, - expiresAt: - typeof payload.expires_in === 'number' - ? Date.now() + payload.expires_in * 1000 - : undefined, + expiresAt: expiresAtFromExpiresIn(payload.expires_in), } }, validateToken: options.validate, + + async refreshToken(input: RefreshInput): Promise> { + const oauth = await loadOauth4webapi() + // Mirror `exchangeCode`: a resolver that reads `flags` sees the + // same view during silent refresh as it did at authorize time. + const flags = (input.handshake.flags as Record | undefined) ?? {} + const [tokenUrl, clientId] = await Promise.all([ + resolve(options.tokenUrl, input.handshake, flags), + resolve(options.clientId, input.handshake, flags), + ]) + const as: AuthorizationServer = { issuer: tokenUrl, token_endpoint: tokenUrl } + const client: Client = { client_id: clientId, token_endpoint_auth_method: 'none' } + // Bound the network call so a hung token endpoint can't block the + // CLI indefinitely (and, for refresh consumers, can't hold the + // refresh lock forever). Route through the consumer's injected + // fetch when present, so a custom transport (proxy dispatcher, + // decompression) applies to the refresh grant too — oauth4webapi + // otherwise captures the global `fetch`. + const requestOptions: TokenEndpointRequestOptions = { + signal: AbortSignal.timeout(REFRESH_TIMEOUT_MS), + ...(options.fetchImpl ? { [oauth.customFetch]: options.fetchImpl } : {}), + } + try { + const response = await oauth.refreshTokenGrantRequest( + as, + client, + oauth.None(), + input.refreshToken, + requestOptions, + ) + const result = await oauth.processRefreshTokenResponse(as, client, response) + return { + accessToken: result.access_token, + refreshToken: result.refresh_token, + expiresAt: expiresAtFromExpiresIn(result.expires_in), + } + } catch (error) { + // A ResponseBodyError carries the server's OAuth error JSON. + // `invalid_grant` (any status — some proxies remap 400 → 401) + // means the refresh token itself was rejected; re-login is the + // only recovery. Every other code is transient from cli-core's + // POV — but surface the actual `error`/`error_description` so a + // misconfigured server (e.g. `invalid_request: Missing + // client_secret`) is diagnosable rather than hidden behind + // oauth4webapi's generic "server responded with an error". + if (error instanceof oauth.ResponseBodyError) { + const detail = error.error_description + ? `${error.error} (${error.error_description})` + : error.error + if (error.error === 'invalid_grant') { + throw new CliError( + 'AUTH_REFRESH_EXPIRED', + `Refresh token rejected: ${detail}`, + { + hints: ['Re-run the login command to reauthorize.'], + }, + ) + } + throw new CliError( + 'AUTH_REFRESH_TRANSIENT', + `Refresh request failed: ${detail}`, + { + hints: ['Try again.'], + }, + ) + } + // Network failure, non-JSON body, WWWAuthenticateChallengeError, … + throw new CliError( + 'AUTH_REFRESH_TRANSIENT', + `Refresh request failed: ${getErrorMessage(error)}`, + { hints: ['Try again.'] }, + ) + } + }, } } -function resolve( +async function resolve( resolver: PkceLazyString, handshake: Record, flags: Record, -): string { +): Promise { return typeof resolver === 'function' ? resolver({ handshake, flags }) : resolver } +// Optional peer dep — only refresh consumers install it. The dynamic import +// (and a missing-peer failure) is memoised so it isn't repeated on every +// refresh, which sits on the authenticated-call path. +let oauthModulePromise: Promise | undefined + +async function loadOauth4webapi(): Promise { + oauthModulePromise ??= import('oauth4webapi') + try { + return await oauthModulePromise + } catch (error) { + const code = (error as NodeJS.ErrnoException | undefined)?.code + if (code === 'ERR_MODULE_NOT_FOUND' || code === 'MODULE_NOT_FOUND') { + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + 'oauth4webapi is required for refresh-token support.', + { hints: ['Run `npm install oauth4webapi` in your CLI.'] }, + ) + } + // Installed but failed to initialise — surface the real cause rather + // than a misleading "install it" hint. + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + `Failed to load oauth4webapi: ${getErrorMessage(error)}`, + ) + } +} + async function safeReadText(response: Response): Promise { try { const text = (await response.text()).trim() diff --git a/src/auth/refresh.test.ts b/src/auth/refresh.test.ts new file mode 100644 index 0000000..32b3aea --- /dev/null +++ b/src/auth/refresh.test.ts @@ -0,0 +1,354 @@ +import { mkdtemp, rm, stat, utimes, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { setTimeout as sleep } from 'node:timers/promises' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { CliError } from '../errors.js' +import { refreshAccessToken } from './refresh.js' +import type { + ActiveBundleSnapshot, + AuthProvider, + ExchangeResult, + TokenBundle, + TokenStore, +} from './types.js' + +type Account = { id: string; label?: string; email: string } + +const account: Account = { id: '42', email: 'a@b' } + +function bundle(overrides: Partial = {}): TokenBundle { + return { + accessToken: 'tok_a', + refreshToken: 'r_a', + accessTokenExpiresAt: Date.now() + 10_000, + ...overrides, + } +} + +type StoreState = { + snapshot: ActiveBundleSnapshot | null + activeBundleSpy: ReturnType + setBundleSpy: ReturnType + setBundleCalls: { account: Account; bundle: TokenBundle; options?: unknown }[] +} + +function fakeStore( + initial: ActiveBundleSnapshot | null, + overrides: Partial> = {}, +): { store: TokenStore; state: StoreState } { + const setBundleCalls: StoreState['setBundleCalls'] = [] + const state: StoreState = { + snapshot: initial, + activeBundleSpy: vi.fn(), + setBundleSpy: vi.fn(), + setBundleCalls, + } + state.activeBundleSpy.mockImplementation(async () => state.snapshot) + state.setBundleSpy.mockImplementation( + async (acc: Account, b: TokenBundle, options?: unknown) => { + setBundleCalls.push({ account: acc, bundle: b, options }) + state.snapshot = { account: acc, bundle: b } + }, + ) + const store: TokenStore = { + async active() { + return state.snapshot + ? { token: state.snapshot.bundle.accessToken, account: state.snapshot.account } + : null + }, + activeBundle: state.activeBundleSpy as unknown as TokenStore['activeBundle'], + async set() {}, + setBundle: state.setBundleSpy as unknown as TokenStore['setBundle'], + async clear() {}, + async list() { + return [] + }, + async setDefault() {}, + ...overrides, + } + return { store, state } +} + +function fakeProvider( + refreshImpl?: (input: { refreshToken: string }) => Promise>, +): { provider: AuthProvider; refreshSpy: ReturnType } { + const refreshSpy = vi.fn( + refreshImpl ?? + (async () => ({ + accessToken: 'tok_new', + refreshToken: 'r_new', + expiresAt: Date.now() + 60_000, + })), + ) + const provider: AuthProvider = { + async authorize() { + return { authorizeUrl: '', handshake: {} } + }, + async exchangeCode() { + return { accessToken: '' } + }, + async validateToken() { + return account + }, + refreshToken: refreshSpy as unknown as AuthProvider['refreshToken'], + } + return { provider, refreshSpy } +} + +describe('refreshAccessToken', () => { + let lockDir: string + let lockPath: string + + beforeEach(async () => { + lockDir = await mkdtemp(join(tmpdir(), 'cli-core-refresh-')) + lockPath = join(lockDir, 'refresh.lock') + }) + + afterEach(async () => { + await rm(lockDir, { recursive: true, force: true }) + }) + + it('rotates the bundle when access expiry is inside the skew window', async () => { + const { store, state } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + }) + const { provider, refreshSpy } = fakeProvider() + + const result = await refreshAccessToken({ + store, + provider, + skewMs: 5_000, + lockPath, + }) + + expect(result.rotated).toBe(true) + expect(result.bundle.accessToken).toBe('tok_new') + expect(refreshSpy).toHaveBeenCalledWith({ refreshToken: 'r_a', handshake: {} }) + // promoteDefault omitted (not just `false`): a silent rotation must + // not re-pin selection, and the helper distinguishes "absent" from + // "explicit opt-out" via arg count. + expect(state.setBundleCalls).toHaveLength(1) + expect(state.setBundleCalls[0].options).toBeUndefined() + }) + + it.each([ + [ + 'expiry is outside the skew window', + () => + fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 60_000 }), + }).store, + ], + [ + 'no expiry is tracked (consumer reactive-refreshes on 401)', + () => + fakeStore({ account, bundle: { accessToken: 'tok_a', refreshToken: 'r_a' } }).store, + ], + ])('returns rotated:false without POSTing when %s', async (_label, makeStore) => { + const store = makeStore() + const { provider, refreshSpy } = fakeProvider() + + const result = await refreshAccessToken({ store, provider, skewMs: 5_000, lockPath }) + + expect(result.rotated).toBe(false) + expect(refreshSpy).not.toHaveBeenCalled() + }) + + it('force:true rotates regardless of expiry', async () => { + const { store } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 60_000 }), + }) + const { provider, refreshSpy } = fakeProvider() + + const result = await refreshAccessToken({ + store, + provider, + force: true, + lockPath, + }) + + expect(result.rotated).toBe(true) + expect(refreshSpy).toHaveBeenCalledTimes(1) + }) + + it.each([ + [ + 'the bundle has no refresh token', + () => ({ + store: fakeStore({ + account, + bundle: { accessToken: 'tok_a', accessTokenExpiresAt: Date.now() + 1_000 }, + }).store, + provider: fakeProvider().provider, + }), + ], + [ + 'no credential is stored', + () => ({ store: fakeStore(null).store, provider: fakeProvider().provider }), + ], + [ + 'the provider does not implement refreshToken', + () => { + const { refreshToken: _drop, ...provider } = fakeProvider().provider + void _drop + return { store: fakeStore({ account, bundle: bundle() }).store, provider } + }, + ], + [ + 'the store does not implement activeBundle', + () => ({ + store: fakeStore({ account, bundle: bundle() }, { activeBundle: undefined }).store, + provider: fakeProvider().provider, + }), + ], + [ + 'the store does not implement setBundle', + () => ({ + store: fakeStore({ account, bundle: bundle() }, { setBundle: undefined }).store, + provider: fakeProvider().provider, + }), + ], + ])('throws AUTH_REFRESH_UNAVAILABLE when %s', async (_label, setup) => { + const { store, provider } = setup() + + await expect( + refreshAccessToken({ store, provider, force: true, lockPath }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_UNAVAILABLE' }) + }) + + it('lock contention: returns the rotated bundle when the holder rotated during the wait', async () => { + // Pre-create the lock file to simulate a holder. + await writeFile(lockPath, '', { flag: 'wx' }) + + const initial: ActiveBundleSnapshot = { + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + } + const { store, state } = fakeStore(initial) + const { provider, refreshSpy } = fakeProvider() + + // Simulate the holder rotating then releasing the lock, awaited + // alongside the call so no background task outlives the test. + const holder = (async () => { + await sleep(120) + state.snapshot = { + account, + bundle: { accessToken: 'tok_held', refreshToken: 'r_held' }, + } + await rm(lockPath, { force: true }) + })() + + const [result] = await Promise.all([ + refreshAccessToken({ store, provider, skewMs: 5_000, lockPath }), + holder, + ]) + + expect(result.rotated).toBe(true) + expect(result.bundle.accessToken).toBe('tok_held') + // Waiter must not have POSTed — the holder already did. + expect(refreshSpy).not.toHaveBeenCalled() + }) + + it('lock contention: throws AUTH_REFRESH_TRANSIENT when the holder times out without rotating', async () => { + await writeFile(lockPath, '', { flag: 'wx' }) + + const { store } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + }) + const { provider, refreshSpy } = fakeProvider() + + await expect( + refreshAccessToken({ store, provider, skewMs: 5_000, lockPath }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_TRANSIENT' }) + expect(refreshSpy).not.toHaveBeenCalled() + }, 5_000) + + it('carries the previous refresh token forward when the server omits it from the response', async () => { + const { store, state } = fakeStore({ + account, + bundle: bundle({ + refreshToken: 'r_existing', + accessTokenExpiresAt: Date.now() + 1_000, + refreshTokenExpiresAt: 9_999_999_999_999, + }), + }) + const { provider } = fakeProvider(async () => ({ + accessToken: 'tok_new', + expiresAt: Date.now() + 60_000, + })) + + const result = await refreshAccessToken({ + store, + provider, + force: true, + lockPath, + }) + + expect(result.bundle.refreshToken).toBe('r_existing') + expect(result.bundle.refreshTokenExpiresAt).toBe(9_999_999_999_999) + expect(state.setBundleCalls[0].bundle.refreshToken).toBe('r_existing') + }) + + it('releases the lock after a failed refresh so a retry can proceed', async () => { + const { store } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + }) + const { provider } = fakeProvider(async () => { + throw new CliError('AUTH_REFRESH_EXPIRED', 'rejected by server') + }) + + await expect( + refreshAccessToken({ store, provider, skewMs: 5_000, lockPath }), + ).rejects.toMatchObject({ code: 'AUTH_REFRESH_EXPIRED' }) + + // The `finally` released the O_EXCL lock — no orphan left to block retries. + await expect(stat(lockPath)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('steals a stale lock left by a crashed holder and proceeds', async () => { + // A lock from a holder that never released, backdated past the stale + // threshold — the next refresh should steal it rather than time out. + await writeFile(lockPath, 'dead-holder-token', { flag: 'wx' }) + const past = new Date(Date.now() - 60_000) + await utimes(lockPath, past, past) + + const { store } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + }) + const { provider, refreshSpy } = fakeProvider() + + const result = await refreshAccessToken({ store, provider, skewMs: 5_000, lockPath }) + + expect(result.rotated).toBe(true) + expect(refreshSpy).toHaveBeenCalledTimes(1) + }) + + it('forwards a non-empty handshake to provider.refreshToken', async () => { + const { store } = fakeStore({ + account, + bundle: bundle({ accessTokenExpiresAt: Date.now() + 1_000 }), + }) + const { provider, refreshSpy } = fakeProvider() + + await refreshAccessToken({ + store, + provider, + force: true, + lockPath, + handshake: { baseUrl: 'https://wiki.example.com' }, + }) + + expect(refreshSpy).toHaveBeenCalledWith({ + refreshToken: 'r_a', + handshake: { baseUrl: 'https://wiki.example.com' }, + }) + }) +}) diff --git a/src/auth/refresh.ts b/src/auth/refresh.ts new file mode 100644 index 0000000..ed1c02c --- /dev/null +++ b/src/auth/refresh.ts @@ -0,0 +1,250 @@ +import { randomUUID } from 'node:crypto' +import { open, readFile, stat, unlink } from 'node:fs/promises' +import { setTimeout as sleep } from 'node:timers/promises' +import { CliError, getErrorMessage } from '../errors.js' +import { bundleFromExchange, persistBundle } from './persist.js' +import type { AccountRef, AuthAccount, AuthProvider, TokenBundle, TokenStore } from './types.js' + +export type RefreshAccessTokenOptions = { + store: TokenStore + provider: AuthProvider + ref?: AccountRef + /** + * Refresh proactively when the access token's expiry is within this many + * ms of now. Default 60_000 (60s). Ignored when `force: true`. + */ + skewMs?: number + /** + * Reactive path: caller hit a 401 and wants a rotation regardless of + * expiry. Skips the skew check; still honours all the "unavailable" + * gates (no refresh token, no provider hook, no `activeBundle`/`setBundle`). + */ + force?: boolean + /** + * Path to the O_EXCL concurrency lock file. Required — cli-core does not + * interpret `~` or know where the consumer's config lives. Recommended: + * `${getConfigPath(serviceName)}.refresh.lock`. + */ + lockPath: string + /** + * Forwarded to `provider.refreshToken` as its `handshake`, so consumers + * can pass runtime context the provider's resolvers need (e.g. a + * `--env`-derived base URL / client id). Defaults to `{}`. + */ + handshake?: Record +} + +export type RefreshAccessTokenResult = { + rotated: boolean + bundle: TokenBundle + account: TAccount +} + +const DEFAULT_SKEW_MS = 60_000 +const LOCK_WAIT_TIMEOUT_MS = 2_000 +const LOCK_POLL_INTERVAL_MS = 50 +// A lock older than this was almost certainly left by a crashed holder — the +// refresh POST is bounded by a provider-side timeout well under this — so it's +// safe to steal rather than block every future refresh forever. +const LOCK_STALE_MS = 15_000 + +/** + * Rotate the access token using the stored refresh token. Proactive when + * `accessTokenExpiresAt` is within `skewMs` of now; reactive when `force: + * true`. Uses an `O_EXCL` file lock at `lockPath` so concurrent CLI + * invocations don't issue parallel refresh-token grants — one POSTs, the + * others re-read the rotated bundle from the store. + * + * Throws `AUTH_REFRESH_UNAVAILABLE` when refresh isn't possible in the + * current setup: store doesn't implement `activeBundle` + `setBundle`, + * provider doesn't implement `refreshToken`, no credential, or no refresh + * token. Server-side rejections surface as `AUTH_REFRESH_EXPIRED` (re-login + * required) or `AUTH_REFRESH_TRANSIENT` (retryable). + */ +export async function refreshAccessToken( + options: RefreshAccessTokenOptions, +): Promise> { + const { store, provider, ref, force, lockPath } = options + const skewMs = options.skewMs ?? DEFAULT_SKEW_MS + const handshake = options.handshake ?? {} + + // Refresh must both read the full bundle and persist the rotated one. A + // store missing either capability can't participate — fail loudly rather + // than silently dropping the rotated refresh token via a `set()` fallback. + if (!store.activeBundle || !store.setBundle) { + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + 'TokenStore must implement activeBundle + setBundle for refresh.', + { hints: ['Re-run the login command to reauthorize.'] }, + ) + } + if (!provider.refreshToken) { + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + 'Auth provider does not implement refreshToken.', + ) + } + const activeBundle = store.activeBundle.bind(store) + const refreshGrant = provider.refreshToken.bind(provider) + + const snapshot = await activeBundle(ref) + if (!snapshot) { + throw new CliError('AUTH_REFRESH_UNAVAILABLE', 'No stored credential to refresh.', { + hints: ['Re-run the login command to reauthorize.'], + }) + } + + if (!force && !needsRefresh(snapshot.bundle, skewMs)) { + return { rotated: false, bundle: snapshot.bundle, account: snapshot.account } + } + + if (!snapshot.bundle.refreshToken) { + throw new CliError('AUTH_REFRESH_UNAVAILABLE', 'Stored credential has no refresh token.', { + hints: ['Re-run the login command to reauthorize.'], + }) + } + + const lockToken = await acquireLock(lockPath) + if (!lockToken) { + // Holder didn't release in time. It may have rotated then crashed + // before unlinking — re-read once and adopt the rotated bundle if so. + const fresh = await activeBundle(ref) + if (fresh && hasRotated(snapshot.bundle, fresh.bundle)) { + return { rotated: true, bundle: fresh.bundle, account: fresh.account } + } + throw new CliError( + 'AUTH_REFRESH_TRANSIENT', + 'Timed out waiting for a concurrent refresh to complete.', + { hints: ['Try again.'] }, + ) + } + + try { + // Re-read under the lock (covers a clean acquire too): a concurrent + // holder may have rotated between our snapshot read and acquiring the + // lock, so adopt their bundle rather than POST a now-stale refresh + // token. A throw here releases the lock via `finally`. + const current = await activeBundle(ref) + if (!current) { + // A concurrent `logout`/`clear` removed the credential after our + // first read — do not POST + persist it back into existence. + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + 'Credential was removed during refresh.', + { + hints: ['Re-run the login command to reauthorize.'], + }, + ) + } + if (hasRotated(snapshot.bundle, current.bundle)) { + return { rotated: true, bundle: current.bundle, account: current.account } + } + const refreshToken = current.bundle.refreshToken + if (!refreshToken) { + throw new CliError( + 'AUTH_REFRESH_UNAVAILABLE', + 'Stored credential has no refresh token.', + { hints: ['Re-run the login command to reauthorize.'] }, + ) + } + const exchange = await refreshGrant({ refreshToken, handshake }) + const account = exchange.account ?? current.account + const bundle = bundleFromExchange(exchange, current.bundle) + await persistBundle({ store, account, bundle }) + return { rotated: true, bundle, account } + } finally { + await releaseLock(lockPath, lockToken) + } +} + +function needsRefresh(bundle: TokenBundle, skewMs: number): boolean { + // No expiry tracked → can't proactively refresh; defer to reactive 401. + if (bundle.accessTokenExpiresAt === undefined) return false + return bundle.accessTokenExpiresAt - Date.now() < skewMs +} + +function hasRotated(before: TokenBundle, after: TokenBundle): boolean { + return ( + after.accessToken !== before.accessToken || + after.accessTokenExpiresAt !== before.accessTokenExpiresAt || + after.refreshToken !== before.refreshToken || + after.refreshTokenExpiresAt !== before.refreshTokenExpiresAt + ) +} + +/** + * Acquire the lock, returning a unique ownership token (or `null` on + * contention timeout). The token is written into the lock file so + * `releaseLock` only unlinks a lock it still owns — a holder whose stale + * lock was stolen mid-flight won't delete the new holder's lock. + */ +async function acquireLock(lockPath: string): Promise { + const token = randomUUID() + if (await tryAcquire(lockPath, token)) return token + const deadline = Date.now() + LOCK_WAIT_TIMEOUT_MS + while (Date.now() < deadline) { + await sleep(LOCK_POLL_INTERVAL_MS) + if (await tryAcquire(lockPath, token)) return token + } + return null +} + +// Write the lock with O_EXCL. On EEXIST, steal it if it's stale (older than +// LOCK_STALE_MS — assumes the provider bounds its HTTP, which the built-in +// PKCE provider does); otherwise report contention. +async function tryAcquire(lockPath: string, token: string): Promise { + if (await tryWriteLock(lockPath, token)) return true + if (await lockIsStale(lockPath)) { + await forceUnlink(lockPath) + return tryWriteLock(lockPath, token) + } + return false +} + +async function lockIsStale(lockPath: string): Promise { + try { + const { mtimeMs } = await stat(lockPath) + return Date.now() - mtimeMs > LOCK_STALE_MS + } catch { + return false + } +} + +async function tryWriteLock(lockPath: string, token: string): Promise { + let handle + try { + handle = await open(lockPath, 'wx') + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + if (code === 'EEXIST') return false + throw new CliError( + 'AUTH_REFRESH_TRANSIENT', + `Failed to acquire refresh lock: ${getErrorMessage(error)}`, + { hints: ['Try again.'] }, + ) + } + try { + await handle.writeFile(token) + } finally { + await handle.close() + } + return true +} + +async function releaseLock(lockPath: string, token: string): Promise { + try { + const owner = (await readFile(lockPath, 'utf8')).trim() + if (owner === token) await unlink(lockPath) + } catch { + // best-effort: a missing/unreadable lock (manual cleanup, crash, a + // steal by another holder) must not surface as a refresh failure. + } +} + +async function forceUnlink(lockPath: string): Promise { + try { + await unlink(lockPath) + } catch { + // already gone + } +} diff --git a/src/auth/status.ts b/src/auth/status.ts index c27ff09..a9d37eb 100644 --- a/src/auth/status.ts +++ b/src/auth/status.ts @@ -2,8 +2,58 @@ import type { Command } from 'commander' import { CliError } from '../errors.js' import { formatJson, formatNdjson } from '../json.js' import type { ViewOptions } from '../options.js' -import type { AuthAccount, TokenStore } from './types.js' -import { attachUserFlag, extractUserRef, requireSnapshotForRef } from './user-flag.js' +import type { AccountRef, AuthAccount, TokenBundle, TokenStore } from './types.js' +import { + accountNotFoundError, + attachUserFlag, + extractUserRef, + requireSnapshotForRef, +} from './user-flag.js' + +type StatusSnapshot = { + token: string + account: TAccount + bundle?: TokenBundle +} + +/** + * Resolve the auth snapshot for `status`. When the consumer supplies + * `fetchLive` (and the store implements `activeBundle`), read the bundle once + * and derive both the access token and the bundle from it — avoiding a second + * keyring round-trip on the hot path. Otherwise fall back to the narrow + * `active()` read. A bundle-read fault (e.g. a refresh-slot error) also falls + * back to `active()` so it can't break status, which is the user's first port + * of call when something is wrong. An explicit-ref miss throws + * `ACCOUNT_NOT_FOUND`, matching `requireSnapshotForRef`. + */ +async function resolveStatusSnapshot( + store: TokenStore, + ref: AccountRef | undefined, + wantsBundle: boolean, +): Promise | null> { + if (wantsBundle && store.activeBundle) { + try { + const snap = await store.activeBundle(ref) + if (snap) { + return { + token: snap.bundle.accessToken, + account: snap.account, + bundle: snap.bundle, + } + } + if (ref !== undefined) throw accountNotFoundError(ref) + return null + } catch (error) { + // Only a typed read failure falls through to the access-only + // `active()` read (e.g. a refresh-slot fault that `active()` + // doesn't hit). Anything else — `ACCOUNT_NOT_FOUND`, or an + // unexpected store bug — propagates rather than being masked. + if (!(error instanceof CliError) || error.code !== 'AUTH_STORE_READ_FAILED') throw error + } + } + const snapshot = await requireSnapshotForRef(store, ref) + return snapshot ? { token: snapshot.token, account: snapshot.account } : null +} export type AttachStatusContext = { account: TAccount @@ -25,6 +75,12 @@ export type AttachStatusCommandOptions flags: Record @@ -52,9 +108,11 @@ export type AttachStatusCommandOptions( parent: Command, @@ -72,7 +130,7 @@ export function attachStatusCommand( ndjson: Boolean(ndjson), } const ref = extractUserRef(cmd) - const snapshot = await requireSnapshotForRef(options.store, ref) + const snapshot = await resolveStatusSnapshot(options.store, ref, Boolean(options.fetchLive)) if (!snapshot) { if (options.onNotAuthenticated) { await options.onNotAuthenticated({ view, flags }) @@ -84,6 +142,7 @@ export function attachStatusCommand( ? await options.fetchLive({ account: snapshot.account, token: snapshot.token, + ...(snapshot.bundle ? { bundle: snapshot.bundle } : {}), view, flags, }) diff --git a/src/auth/types.ts b/src/auth/types.ts index 934d9a9..7379be9 100644 --- a/src/auth/types.ts +++ b/src/auth/types.ts @@ -90,6 +90,12 @@ export type TokenBundle = { refreshTokenExpiresAt?: number } +/** Read-side snapshot returned by `activeBundle`. Mirrors `setBundle`'s write side. */ +export type ActiveBundleSnapshot = { + account: TAccount + bundle: TokenBundle +} + /** Opaque account selector. Stores own the matching rule (id, email, label, …). */ export type AccountRef = string @@ -123,6 +129,18 @@ export type TokenStore = { bundle: TokenBundle, options?: { promoteDefault?: boolean }, ): Promise + /** + * Full-bundle read for refresh-capable consumers. Returns the matching + * account + bundle, or `null` on miss. Optional on the contract — the + * silent-refresh helper throws `AUTH_REFRESH_UNAVAILABLE` when a custom + * store doesn't implement it. `KeyringTokenStore` overrides this as + * required so cli-core helpers can call it without a non-null assertion. + * + * Stores MAY throw `CliError('AUTH_STORE_READ_FAILED', …)` on the same + * conditions as `active()` (e.g. keyring offline while a matching + * record exists). + */ + activeBundle?(ref?: AccountRef): Promise | null> /** Remove a stored credential. No-op when `ref` doesn't match. */ clear(ref?: AccountRef): Promise /** Every stored account with a default marker. */