diff --git a/SECURITY.md b/SECURITY.md index f6a3ccc..6a1c2ab 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -17,6 +17,14 @@ impact, the things worth knowing: backend on Linux or Windows — those platforms must use `CT_LOGINTOKEN`. `ct.envs.json` stores only the _name_ of a token env var, never a value, which is why it is safe to commit. +- **The session is cached, and treated as a second secret.** Since #145, the session cookie + and CSRF token a login yields are kept per host so a one-shot CLI does not re-run the + login handshake on every command (which trips ChurchTools' login rate limit). A live + session cookie is as powerful as the token, so it is stored the same way — in the + **macOS Keychain**, under a separate `session:` entry, never in a file — and + therefore not at all on Linux/Windows, which keep handshaking per invocation. It is + bound to the host it was captured against and to a hash of the token that bought it, + never logged or printed, and removed by `ct auth logout` along with the token. - **Storing a token exposes it briefly to `ps`.** `ct auth login` shells out to `security add-generic-password -w `, so the value sits in that process's argv for the duration of the call. On a shared machine, assume another local user could observe diff --git a/docs/handbuch/field-definitions.md b/docs/handbuch/field-definitions.md index c52c6bd..f82f074 100644 --- a/docs/handbuch/field-definitions.md +++ b/docs/handbuch/field-definitions.md @@ -3,7 +3,7 @@ title: Field definitions & security levels sources: - src/commands/get.ts - src/api/ctClient.ts -sources_hash: 047a27a2bd82b120 +sources_hash: 659b65504025ebb8 reviewed: 2026-08-13 --- diff --git a/src/api/ctClient.ts b/src/api/ctClient.ts index 3b1c227..969355c 100644 --- a/src/api/ctClient.ts +++ b/src/api/ctClient.ts @@ -14,7 +14,7 @@ * here can be swapped for `openapi-fetch` while keeping this class's surface. */ import { type CtConfig } from "../config.js"; -import { fetchWithRetry } from "./http.js"; +import { fetchWithRetry, parseRetryAfterMs } from "./http.js"; import { meetsMinVersion, MIN_CT_VERSION, type CtInfo } from "./version.js"; export interface WhoAmI { @@ -66,12 +66,61 @@ export interface CtPage { const MAX_PAGES = 1000; const DEFAULT_PAGE_LIMIT = 100; +/** + * A place to keep the session (cookie + CSRF token) BETWEEN `ct` invocations, so + * a one-shot CLI does not have to re-run the login handshake — and trip + * ChurchTools' login rate limit — on every single command (#145). + * + * The client only ever asks for a session for `host` and hands back one captured + * against `host`, so the store cannot leak a session across instances (#30). The + * implementation lives in `src/auth/sessionStore.ts`; it is injected rather than + * imported so a client without one (tests, `ct auth status`' preflight) behaves + * exactly as it always has. + */ +export interface SessionCache { + load(host: string, token: string): Promise<{ cookie: string; csrfToken: string } | null>; + save(host: string, token: string, session: { cookie: string; csrfToken: string }): Promise; + drop(host: string): Promise; +} + +export interface CtClientOptions { + sessionCache?: SessionCache; +} + +/** Human-readable "wait this long" for a 429, from `Retry-After` when the server sent one. */ +function describeRetryAfter(res: Response): string { + const ms = parseRetryAfterMs(res); + if (ms === null) { + return "about a minute"; + } + const seconds = Math.ceil(ms / 1000); + return seconds >= 120 + ? `about ${Math.ceil(seconds / 60)} minutes` + : `about ${Math.max(seconds, 1)} seconds`; +} + export class CtClient { private cookie: string | null = null; private csrfToken: string | null = null; private ctVersion: string | null = null; + /** Kept so an expired session can be re-bought without the caller having to notice (#145). */ + private loginToken: string | null = null; + /** Re-entrancy guards: no self-heal while a login (or a resume probe) is already in flight. */ + private loggingIn = false; + private resuming = false; + /** + * At most ONE automatic re-login per *unrecovered* 401 — a server that always 401s must not + * become a login storm. Reset once a re-login actually produced a successful replay, so the + * budget is a guard against a hopeless loop rather than a lifetime cap: a long `apply` whose + * session dies twice (or that burned the budget on a 401 that was never about the session — + * CT answers 401 for a CSRF failure too) can still self-heal. + */ + private reauthAttempts = 0; - constructor(private readonly config: CtConfig) {} + constructor( + private readonly config: CtConfig, + private readonly options: CtClientOptions = {}, + ) {} get host(): string { return this.config.host; @@ -117,8 +166,82 @@ export class CtClient { } } + /** + * Become an authenticated client — reusing the cached session for this host + * when there is one, and only otherwise running the login handshake (#145). + * + * `fresh: true` forces the handshake (`ct auth login`, which exists precisely + * to prove the credential works, must not be answered from a cache). + */ + async authenticate(loginToken: string, opts: { fresh?: boolean } = {}): Promise { + this.loginToken = loginToken; + if (!opts.fresh) { + const resumed = await this.resumeCachedSession(loginToken); + if (resumed) { + return resumed; + } + } + return this.login(loginToken); + } + + /** + * Try the cached session: adopt the cookie + CSRF token and confirm them with + * a plain `GET /whoami`. + * + * That GET carries no `login_token`, so it is an ordinary authenticated read — + * it does NOT count against ChurchTools' *login* rate limit, which is the + * whole point of the cache. It also keeps the identity honest: `me` still + * comes from the server rather than from a stale local copy. + * + * A session the server no longer accepts (401/403) is dropped and `null` is + * returned, so the caller falls through to a real handshake. Anything else + * (network trouble, a 500) is a real failure and propagates. + */ + private async resumeCachedSession(loginToken: string): Promise { + const cache = this.options.sessionCache; + if (!cache) { + return null; + } + let cached: { cookie: string; csrfToken: string } | null = null; + try { + cached = await cache.load(this.config.host, loginToken); + } catch { + return null; // an unreadable cache is never a reason to fail a command + } + if (!cached) { + return null; + } + this.cookie = cached.cookie; + this.csrfToken = cached.csrfToken; + this.resuming = true; + try { + return await this.get("/whoami"); + } catch (err) { + if (!isSessionRejection(err)) { + throw err; + } + this.cookie = null; + this.csrfToken = null; + await cache.drop(this.config.host).catch(() => {}); + return null; + } finally { + this.resuming = false; + } + } + /** Run the login-token handshake and cache the session cookie + CSRF token. */ - async authenticate(loginToken: string): Promise { + private async login(loginToken: string): Promise { + this.loggingIn = true; + this.cookie = null; + this.csrfToken = null; + try { + return await this.performLogin(loginToken); + } finally { + this.loggingIn = false; + } + } + + private async performLogin(loginToken: string): Promise { // The token rides as a URL query param (it lands in the server's access logs). This is // unavoidable for this token class: the handshake above is documented to require the // `login_token` query param — an `Authorization` header yields a null CSRF token and breaks @@ -132,18 +255,54 @@ export class CtClient { ); this.captureCookie(res); if (!res.ok) { + if (res.status === 429) { + // Not a credential problem, and saying "Login failed" reads like one. CT + // throttles LOGINS per instance, so this fires after a burst of short `ct` + // invocations — the very thing the session cache exists to stop. + throw new CtApiError( + `ChurchTools is rate-limiting logins on ${this.config.host} — your token was not rejected. ` + + `Wait ${describeRetryAfter(res)} and try again.`, + res.status, + await safeBody(res), + ); + } throw new CtApiError(`Login failed (whoami)`, res.status, await safeBody(res)); } if (!this.cookie) { throw new CtApiError("Login succeeded but no session cookie was returned", res.status, null); } await this.refreshCsrfToken(); + // Keep the freshly bought session for the NEXT invocation. Best-effort: a store + // that refuses (no Keychain, locked Keychain) must not fail the command. + if (this.cookie && this.csrfToken) { + await this.options.sessionCache + ?.save(this.config.host, loginToken, { cookie: this.cookie, csrfToken: this.csrfToken }) + .catch(() => {}); + } // Same tolerant unwrap as request(): prefer `.data`, but fall back to the raw body if the // envelope is absent, so authenticate and request() agree on the shape. const body = (await res.json()) as { data?: WhoAmI }; return (body.data ?? body) as WhoAmI; } + /** + * Recover from a 401 on a real request: drop the cached session, log in again, + * and let the caller retry once. Returns false when re-authentication is not + * available or not appropriate, in which case the 401 surfaces as usual. + * + * A 401 means the request was rejected before it was processed, so the retry is + * safe for writes too. + */ + private async reauthenticate(): Promise { + if (this.loginToken === null || this.loggingIn || this.resuming || this.reauthAttempts >= 1) { + return false; + } + this.reauthAttempts++; + await this.options.sessionCache?.drop(this.config.host).catch(() => {}); + await this.login(this.loginToken); + return true; + } + async get(path: string): Promise { return this.request("GET", path); } @@ -322,6 +481,22 @@ export class CtClient { { isIdempotent: method === "GET" || method === "HEAD" }, ); this.captureCookie(res); + if (res.status === 401) { + // The session died (expired, or invalidated server-side — a logout elsewhere, + // a restart). Buy a new one once and replay the request, so a stale cached + // session self-heals instead of surfacing as "Not authenticated" (#145). + if (await this.reauthenticate()) { + // Drain the response we're discarding so its socket isn't left buffered. + await res.body?.cancel().catch(() => {}); + const replayed = await this.requestEnvelope(method, path, body); + // The replay went through, so the self-heal worked and the budget it spent is + // available again for a later expiry in the same run. A re-login whose replay + // 401s again throws from the line above with the budget still spent, which is + // what stops an always-401 server from becoming a login storm. + this.reauthAttempts = 0; + return replayed; + } + } if (!res.ok) { throw new CtApiError(`${method} ${path} failed`, res.status, await safeBody(res)); } @@ -421,6 +596,11 @@ export class CtClient { } } +/** True when an error says "this session is no longer good" rather than "the request was bad". */ +function isSessionRejection(err: unknown): boolean { + return err instanceof CtApiError && (err.status === 401 || err.status === 403); +} + async function safeBody(res: Response): Promise { try { return await res.json(); diff --git a/src/api/http.ts b/src/api/http.ts index 4ccf99e..fbb4888 100644 --- a/src/api/http.ts +++ b/src/api/http.ts @@ -38,27 +38,44 @@ function backoffMs(base: number, attempt: number): number { return clampDelay(exp + Math.floor(Math.random() * base)); } -function retryAfterMs(res: Response): number | null { +/** + * The wait a response's `Retry-After` header asks for, in ms — `null` when the + * header is absent or unusable, and deliberately NOT clamped: this is the + * server's actual ask. Exported so a caller that has run out of retries can + * still *tell the user* how long to wait (see the 429 login message in + * `ctClient.authenticate`), where clamping to the sleep cap would understate it. + */ +export function parseRetryAfterMs(res: Response): number | null { const header = res.headers.get("retry-after")?.trim(); if (!header) { return null; } // `Retry-After` is either a non-negative delta-seconds count or an HTTP-date. if (/^\d+$/.test(header)) { - return clampDelay(Number.parseInt(header, 10) * 1000); + return Number.parseInt(header, 10) * 1000; } // Only treat it as a date when it actually looks like one — `Date.parse` is // lax enough to accept e.g. "-5" as a year, which must not become a 0ms wait. if (/[a-zA-Z]/.test(header)) { const dateMs = Date.parse(header); if (Number.isFinite(dateMs)) { - return clampDelay(dateMs - Date.now()); + return dateMs - Date.now(); } } // Unparseable (e.g. a negative or malformed value): fall back to backoff. return null; } +/** + * The `Retry-After` wait clamped to what this client is willing to sleep for — + * an outsized (or already-elapsed) value can neither hang the CLI nor turn into + * a busy retry. + */ +function retryAfterMs(res: Response): number | null { + const ms = parseRetryAfterMs(res); + return ms === null ? null : clampDelay(ms); +} + function shouldRetryStatus(status: number, isIdempotent: boolean): boolean { if (status === 429) { return true; diff --git a/src/api/session.ts b/src/api/session.ts index b8f5e13..92000b0 100644 --- a/src/api/session.ts +++ b/src/api/session.ts @@ -4,6 +4,7 @@ */ import { CtClient, type WhoAmI } from "./ctClient.js"; import { readCredentials } from "../auth/tokenStore.js"; +import { keychainSessionCache } from "../auth/sessionStore.js"; import { normalizeHost, resolveConfig } from "../config.js"; export interface AuthedSession { @@ -45,7 +46,9 @@ export async function authedSession(): Promise { ); } - const client = new CtClient(config); + // The session cache is keyed by the SAME host the binding check above just cleared, + // so a cached session can only ever be replayed against the instance it came from (#30/#145). + const client = new CtClient(config, { sessionCache: keychainSessionCache() }); const me = await client.authenticate(token); // Hard-fail below the minimum CT version before any command reads or writes — // a stale instance half-applies (tier-0 writes succeed, hierarchy endpoints 404). diff --git a/src/auth/keychain.ts b/src/auth/keychain.ts new file mode 100644 index 0000000..7bbcb76 --- /dev/null +++ b/src/auth/keychain.ts @@ -0,0 +1,86 @@ +/** + * The low-level macOS Keychain access shared by everything this CLI persists + * locally: the login credentials ({@link ../auth/tokenStore.js}) and the cached + * session ({@link ./sessionStore.js}). + * + * Both are secrets, both live under the same `ct-cli` keychain service, and both + * need the same per-process read memoization — so the `security` plumbing lives + * here once, in a module neither of them imports the other through. + * + * There is deliberately **no file fallback**. On Linux/Windows there is no + * keychain backend, and writing a secret (a token *or* a session cookie) to a + * plaintext file is exactly what this project refuses to do; those platforms + * supply the token through `CT_HOST` / `CT_LOGINTOKEN` and simply run without a + * session cache. + * + * Note: `security ... -w ` passes the value as an argv, briefly visible + * to `ps`. Acceptable for a local developer CLI; the value never touches git. + */ +import { platform } from "node:os"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const run = promisify(execFile); + +export const KEYCHAIN_SERVICE = "ct-cli"; + +export function isMac(): boolean { + return platform() === "darwin"; +} + +/** + * Memoized keychain reads for this process, keyed by account. A single run + * resolves the host (via `resolveConfig`) AND the token (via `authedSession`) — + * each of which reaches for the stored credentials — so without this cache the + * same entry is fetched multiple times per command, spawning + * `security find-generic-password` (and prompting to unlock a locked Keychain) + * every time. A cached value of `null` = read and absent. A write through + * `keychainSet`/`keychainDelete` invalidates only the account it touched, so + * storing one entry (a session) does not force every *other* entry (the + * credential blob) to be re-read — and re-prompted for. + */ +const cachedBlobs = new Map(); + +/** Drop ALL memoized keychain reads. A blunt instrument kept for tests and for a + * fresh credential login, where every derived entry is suspect; ordinary writes + * invalidate only their own account. */ +export function resetKeychainCache(): void { + cachedBlobs.clear(); +} + +export async function keychainSet(account: string, value: string): Promise { + await run("security", ["add-generic-password", "-U", "-s", KEYCHAIN_SERVICE, "-a", account, "-w", value]); + cachedBlobs.set(account, value); // a read later in this process must see what was just written +} + +export async function keychainGet(account: string): Promise { + const cached = cachedBlobs.get(account); + if (cached !== undefined) { + return cached; + } + let value: string | null; + try { + const { stdout } = await run("security", [ + "find-generic-password", + "-s", + KEYCHAIN_SERVICE, + "-a", + account, + "-w", + ]); + value = stdout.trim() || null; + } catch { + value = null; + } + cachedBlobs.set(account, value); + return value; +} + +export async function keychainDelete(account: string): Promise { + try { + await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", account]); + } catch { + /* not present — nothing to delete */ + } + cachedBlobs.delete(account); // a later read must not return the cleared secret +} diff --git a/src/auth/sessionStore.ts b/src/auth/sessionStore.ts new file mode 100644 index 0000000..b3a9f5e --- /dev/null +++ b/src/auth/sessionStore.ts @@ -0,0 +1,177 @@ +/** + * Per-host cache of the ChurchTools **session** — the session cookie plus the + * CSRF token the login handshake yields (#145). + * + * Why this exists: `ct` is one-shot, so without it every single invocation + * re-runs `GET /api/whoami?login_token=…` + `GET /api/csrftoken`. ChurchTools + * rate-limits *logins*, so a handful of `ct get` calls in a row is enough to be + * throttled (HTTP 429) before any command reaches its actual endpoint. The login + * token is a credential; what was missing was somewhere to keep the *session* it + * buys. + * + * ## Where it is stored, and why + * + * In the **macOS Keychain**, next to the credentials it derives from — never in + * a file. A session cookie is a bearer handle on a logged-in ChurchTools + * account: for as long as it lives it is exactly as powerful as the login token, + * so it gets exactly the same protection. The Keychain is encrypted at rest and + * ACL'd by the OS; a `0600` file under `$HOME` is neither (it survives in + * backups, in `tar`red home directories, and is readable by every process + * running as the user). + * + * That choice also decides the non-macOS story: `keychain.ts` has no file + * fallback, so on Linux/Windows there is simply **no session cache**. The + * behaviour there is exactly today's — a handshake per invocation — which is the + * right trade: this project already refuses to write the login token to a + * plaintext file on those platforms, and the cookie must not be the loophole + * that does it anyway. + * + * ## Host binding (#30) + * + * The cache is keyed by host (account name `session:`) AND stores the host + * inside the blob, which is re-checked on read. A session captured against one + * instance is therefore never offered to another, matching the rule the login + * token already follows. + * + * A session is additionally bound to the *token* that bought it, by SHA-256 + * fingerprint (the hash, never the token). Log in as somebody else and the + * fingerprint no longer matches, so the previous user's session is not silently + * reused. + * + * Nothing here ever logs, prints or returns the cookie in an error: callers get + * the value or `null`. + */ +import { createHash } from "node:crypto"; +import { isMac, keychainDelete, keychainGet, keychainSet } from "./keychain.js"; +import type { SessionCache } from "../api/ctClient.js"; + +/** Keychain account prefix; the rest of the account name is the (normalized) host. */ +const SESSION_ACCOUNT_PREFIX = "session:"; + +/** + * How long a cached session may be reused before a fresh handshake is forced. + * ChurchTools does not advertise its session lifetime, so this is a deliberately + * conservative ceiling rather than a prediction — an already-expired session is + * handled by the 401 self-heal in `CtClient`, not by this number. + */ +export const SESSION_MAX_AGE_MS = 12 * 60 * 60 * 1000; + +export interface CachedSession { + /** The host this session was captured against. Re-checked on read (#30). */ + host: string; + cookie: string; + csrfToken: string; + /** Epoch ms; ages the entry out. */ + obtainedAt: number; + /** SHA-256 of the login token that bought the session — never the token itself. */ + tokenHash: string; +} + +/** A non-reversible fingerprint of the login token, safe to persist alongside the session. */ +export function tokenFingerprint(token: string): string { + return createHash("sha256").update(token).digest("hex"); +} + +function accountFor(host: string): string { + return `${SESSION_ACCOUNT_PREFIX}${host}`; +} + +/** Parse a stored blob; `null` for anything that is not a well-formed session. */ +export function parseSession(raw: string): CachedSession | null { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (typeof parsed !== "object" || parsed === null) { + return null; + } + const { host, cookie, csrfToken, obtainedAt, tokenHash } = parsed as Record; + if (typeof host !== "string" || host === "") return null; + if (typeof cookie !== "string" || cookie === "") return null; + if (typeof csrfToken !== "string") return null; + if (typeof obtainedAt !== "number" || !Number.isFinite(obtainedAt)) return null; + if (typeof tokenHash !== "string" || tokenHash === "") return null; + return { host, cookie, csrfToken, obtainedAt, tokenHash }; +} + +/** + * The cached session for `host`, or `null`. Returns `null` — rather than + * throwing — for every "cannot be reused" case: not macOS, nothing stored, + * corrupt blob, a session captured against a different host, a session bought + * with a different token, or one older than {@link SESSION_MAX_AGE_MS} + * (including a blob timestamped in the future, i.e. after a clock change). + */ +export async function readSession(host: string, token: string): Promise { + if (!isMac()) { + return null; + } + const raw = await keychainGet(accountFor(host)); + if (!raw) { + return null; + } + const session = parseSession(raw); + if (!session) { + return null; + } + // Belt and braces on top of the host-keyed account: the session must itself + // claim the host we are about to send it to (#30). + if (session.host !== host) { + return null; + } + if (session.tokenHash !== tokenFingerprint(token)) { + return null; + } + const age = Date.now() - session.obtainedAt; + if (age < 0 || age > SESSION_MAX_AGE_MS) { + return null; + } + return session; +} + +/** Persist the session for `host`. A no-op off macOS (see the module docs). */ +export async function storeSession( + host: string, + token: string, + session: { cookie: string; csrfToken: string }, +): Promise { + if (!isMac()) { + return; + } + const blob: CachedSession = { + host, + cookie: session.cookie, + csrfToken: session.csrfToken, + obtainedAt: Date.now(), + tokenHash: tokenFingerprint(token), + }; + // `keychainSet` refreshes this account's memoized read on its own, so a later read in + // this process sees the new session — without evicting the credential blob's cached + // read and re-prompting for a locked Keychain (keychain.ts). + await keychainSet(accountFor(host), JSON.stringify(blob)); +} + +/** Forget the cached session for `host` (logout, or a session the server rejected). */ +export async function clearSession(host: string): Promise { + if (!isMac()) { + return; + } + await keychainDelete(accountFor(host)); +} + +/** + * The Keychain-backed {@link SessionCache} handed to `CtClient`. + * + * `CtClient` takes the cache as a dependency rather than reaching for this + * module itself, so a client built in a test (or anywhere that must not touch + * the developer's real Keychain) simply has no cache and behaves exactly as + * before. + */ +export function keychainSessionCache(): SessionCache { + return { + load: readSession, + save: storeSession, + drop: clearSession, + }; +} diff --git a/src/auth/tokenStore.ts b/src/auth/tokenStore.ts index 2055fa7..b86516d 100644 --- a/src/auth/tokenStore.ts +++ b/src/auth/tokenStore.ts @@ -21,12 +21,19 @@ * Note: `security ... -w ` passes the value as an argv, briefly visible * to `ps`. Acceptable for a local developer CLI; the value never touches git. */ -import { platform } from "node:os"; -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; +import { clearSession } from "./sessionStore.js"; +import { + isMac, + keychainDelete, + keychainGet, + keychainSet, + resetKeychainCache, + KEYCHAIN_SERVICE, +} from "./keychain.js"; + +/** Re-exported so callers (and tests) keep one entry point for invalidating stored reads. */ +export { resetKeychainCache }; -const run = promisify(execFile); -const KEYCHAIN_SERVICE = "ct-cli"; /** The "default / last login" account — a single blob, also the single-host (pre-#22) location. */ const DEFAULT_ACCOUNT = "credentials"; /** Pre-host account name; a bare token used to live here. Cleared on logout so no secret is orphaned. */ @@ -37,10 +44,6 @@ export interface Credentials { token: string; } -function isMac(): boolean { - return platform() === "darwin"; -} - /** * Whether this platform has a credential store `ct` can write to — today, the * macOS Keychain and nothing else. Exported because callers need to know BEFORE @@ -71,57 +74,6 @@ export function parseCredentials(raw: string): Credentials | null { return { host, token }; } -async function keychainSet(account: string, value: string): Promise { - await run("security", ["add-generic-password", "-U", "-s", KEYCHAIN_SERVICE, "-a", account, "-w", value]); -} - -/** - * Memoized keychain reads for this process, keyed by account. A single run - * resolves the host (via `resolveConfig`) AND the token (via `authedSession`) — - * each of which reaches for the stored credentials — so without this cache the - * same entry is fetched multiple times per command, spawning - * `security find-generic-password` (and prompting to unlock a locked Keychain) - * every time. A cached value of `null` = read and absent. Invalidated wholesale - * on any write (`resetKeychainCache`). - */ -const cachedBlobs = new Map(); - -/** Drop the memoized keychain reads. Called after every store/clear; exported for tests. */ -export function resetKeychainCache(): void { - cachedBlobs.clear(); -} - -async function keychainGet(account: string): Promise { - const cached = cachedBlobs.get(account); - if (cached !== undefined) { - return cached; - } - let value: string | null; - try { - const { stdout } = await run("security", [ - "find-generic-password", - "-s", - KEYCHAIN_SERVICE, - "-a", - account, - "-w", - ]); - value = stdout.trim() || null; - } catch { - value = null; - } - cachedBlobs.set(account, value); - return value; -} - -async function keychainDelete(account: string): Promise { - try { - await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", account]); - } catch { - /* not present — nothing to delete */ - } -} - /** * Persist host + token in the macOS Keychain; returns a human-readable location. * Written to BOTH the per-host account (so `readCredentials(host)` finds it) and @@ -194,6 +146,10 @@ export async function readStoredHost(): Promise { * commands WITHOUT `--env` fall back to. That is reported back in * `clearedDefault` so the caller can say so instead of promising, wrongly, that * nothing else changed. + * + * Either way the host's **cached session** goes with its credentials (#145): the + * session cookie is a live authenticated handle on the instance, so leaving it + * behind would mean `ct auth logout` had not actually logged anything out. */ export interface ClearCredentialsResult { /** True when the default (unqualified) login was removed along with the host's. */ @@ -205,6 +161,7 @@ export async function clearCredentials(host?: string): Promise Promise.resolve(); @@ -47,6 +47,21 @@ describe("fetchWithRetry", () => { expect(sleep.mock.calls[0]?.[0] ?? 0).toBeGreaterThanOrEqual(500); }); + it("waits out a 429 on a write too — the server rejected it before processing", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(res(429, { "retry-after": "3" })) + .mockResolvedValueOnce(res(200)); + const sleep = vi.fn(() => Promise.resolve()); + const out = await fetchWithRetry( + "https://x/api/campuses", + { method: "POST" }, + { sleep, fetchImpl, isIdempotent: false }, + ); + expect(out.status).toBe(200); + expect(sleep).toHaveBeenCalledWith(3000); + }); + it("retries idempotent GET on 500", async () => { const fetchImpl = vi.fn().mockResolvedValueOnce(res(500)).mockResolvedValueOnce(res(200)); const out = await fetchWithRetry( @@ -88,3 +103,16 @@ describe("fetchWithRetry", () => { expect(fetchImpl).toHaveBeenCalledTimes(1); }); }); + +describe("parseRetryAfterMs", () => { + it("reports the server's full ask, unclamped, so a message can quote it", () => { + // fetchWithRetry clamps its own sleep to 60s; the *message* must not claim + // "wait about a minute" when the server asked for an hour. + expect(parseRetryAfterMs(res(429, { "retry-after": "3600" }))).toBe(3_600_000); + }); + + it("returns null when the header is missing or malformed", () => { + expect(parseRetryAfterMs(res(429))).toBeNull(); + expect(parseRetryAfterMs(res(429, { "retry-after": "-5" }))).toBeNull(); + }); +}); diff --git a/tests/session-cache.test.ts b/tests/session-cache.test.ts new file mode 100644 index 0000000..db3402f --- /dev/null +++ b/tests/session-cache.test.ts @@ -0,0 +1,291 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { CtClient, CtApiError, type SessionCache } from "../src/api/ctClient.js"; + +function jsonResponse(body: unknown, init: ResponseInit & { setCookie?: string } = {}): Response { + const headers = new Headers(init.headers); + if (init.setCookie) { + headers.append("set-cookie", init.setCookie); + } + return new Response(JSON.stringify(body), { status: init.status ?? 200, headers }); +} + +/** An in-memory stand-in for the Keychain-backed store, with the same host binding. */ +function memoryCache(): SessionCache & { entries: Map } { + const entries = new Map(); + return { + entries, + load: (host, token) => Promise.resolve(entries.get(`${host}|${token}`) ?? null), + save: (host, token, session) => { + entries.set(`${host}|${token}`, session); + return Promise.resolve(); + }, + drop: (host) => { + for (const key of [...entries.keys()]) { + if (key.startsWith(`${host}|`)) entries.delete(key); + } + return Promise.resolve(); + }, + }; +} + +const HOST = "https://mychurch.church.tools"; +const OTHER_HOST = "https://other.church.tools"; + +function urls(mock: ReturnType>): string[] { + return mock.mock.calls.map((call) => String(call[0])); +} + +beforeEach(() => { + // `fetchWithRetry` sleeps between retries (429/5xx). Run those waits instantly so + // the retry-shaped tests below don't spend seconds actually waiting. + vi.stubGlobal("setTimeout", ((fn: () => void) => { + fn(); + return 0; + }) as unknown as typeof setTimeout); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe("session cache across invocations (#145)", () => { + it("stores the session on login and reuses it in a second client — no second handshake", async () => { + const cache = memoryCache(); + const first = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "ct_session=abc; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf-1" })); + vi.stubGlobal("fetch", first); + + await new CtClient({ host: HOST }, { sessionCache: cache }).authenticate("tok"); + expect(cache.entries.get(`${HOST}|tok`)).toEqual({ cookie: "ct_session=abc", csrfToken: "csrf-1" }); + + // A brand-new process: same host, same token, cache carried over. + const second = vi.fn().mockResolvedValueOnce(jsonResponse({ data: { id: 7 } })); + vi.stubGlobal("fetch", second); + const client = new CtClient({ host: HOST }, { sessionCache: cache }); + const me = await client.authenticate("tok"); + + expect(me).toEqual({ id: 7 }); + // One request only, and it is NOT the login handshake: no login_token, no csrftoken fetch. + expect(second).toHaveBeenCalledTimes(1); + expect(urls(second)[0]).toBe(`${HOST}/api/whoami`); + expect(urls(second).join(" ")).not.toContain("login_token"); + expect(urls(second).join(" ")).not.toContain("csrftoken"); + }); + + it("resumed session carries the cached cookie and CSRF token into later requests", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "ct_session=abc", csrfToken: "csrf-1" }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } })) + .mockResolvedValueOnce(jsonResponse({ data: { id: 99 } })); + vi.stubGlobal("fetch", fetchMock); + + const client = new CtClient({ host: HOST }, { sessionCache: cache }); + await client.authenticate("tok"); + await client.request("POST", "/campuses", { name: "Mainz" }); + + const headers = new Headers(fetchMock.mock.calls[1]?.[1]?.headers); + expect(headers.get("Cookie")).toBe("ct_session=abc"); + expect(headers.get("CSRF-Token")).toBe("csrf-1"); // no extra /csrftoken round trip + }); + + it("never offers host A's session to host B (#30)", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "ct_session=abc", csrfToken: "csrf-1" }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "other=zzz; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf-other" })); + vi.stubGlobal("fetch", fetchMock); + + await new CtClient({ host: OTHER_HOST }, { sessionCache: cache }).authenticate("tok"); + + // The other host got a full handshake, and host A's cookie was never sent there. + expect(urls(fetchMock)[0]).toContain(`${OTHER_HOST}/api/whoami?login_token=`); + for (const call of fetchMock.mock.calls) { + expect(String(call[0])).toContain(OTHER_HOST); + expect(new Headers(call[1]?.headers).get("Cookie") ?? "").not.toContain("ct_session=abc"); + } + // Both sessions coexist, each under its own host. + expect(cache.entries.get(`${HOST}|tok`)?.cookie).toBe("ct_session=abc"); + expect(cache.entries.get(`${OTHER_HOST}|tok`)?.cookie).toBe("other=zzz"); + }); + + it("a rejected cached session is dropped and replaced by a real handshake", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "stale=1", csrfToken: "old" }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ message: "unauthorized" }, { status: 401 })) + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "fresh=2; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf-2" })); + vi.stubGlobal("fetch", fetchMock); + + const me = await new CtClient({ host: HOST }, { sessionCache: cache }).authenticate("tok"); + + expect(me).toEqual({ id: 7 }); + expect(urls(fetchMock)[1]).toContain("login_token=tok"); + expect(cache.entries.get(`${HOST}|tok`)).toEqual({ cookie: "fresh=2", csrfToken: "csrf-2" }); + }); + + it("401 on a real request → cache dropped, re-authenticated once, request retried", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "stale=1", csrfToken: "old" }); + const fetchMock = vi + .fn() + // resume probe succeeds (the session looks fine to /whoami) … + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } })) + // … then the real request is rejected + .mockResolvedValueOnce(jsonResponse({ message: "unauthorized" }, { status: 401 })) + // re-login + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "fresh=2; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf-2" })) + // retry of the original request + .mockResolvedValueOnce(jsonResponse({ data: [{ id: 1 }] })); + vi.stubGlobal("fetch", fetchMock); + + const client = new CtClient({ host: HOST }, { sessionCache: cache }); + await client.authenticate("tok"); + const campuses = await client.get("/campuses"); + + expect(campuses).toEqual([{ id: 1 }]); // the 401 never reaches the user + expect(urls(fetchMock)[4]).toBe(`${HOST}/api/campuses`); + expect(new Headers(fetchMock.mock.calls[4]?.[1]?.headers).get("Cookie")).toBe("fresh=2"); + expect(cache.entries.get(`${HOST}|tok`)).toEqual({ cookie: "fresh=2", csrfToken: "csrf-2" }); + }); + + it("does not turn a persistently 401-ing server into a login storm", async () => { + const cache = memoryCache(); + const fetchMock = vi.fn().mockImplementation((input) => { + const url = String(input); + if (url.includes("login_token")) { + return Promise.resolve(jsonResponse({ data: { id: 7 } }, { setCookie: "s=1; Path=/" })); + } + if (url.includes("csrftoken")) { + return Promise.resolve(jsonResponse({ data: "csrf" })); + } + return Promise.resolve(jsonResponse({ message: "unauthorized" }, { status: 401 })); + }); + vi.stubGlobal("fetch", fetchMock); + + const client = new CtClient({ host: HOST }, { sessionCache: cache }); + await client.authenticate("tok"); + await expect(client.get("/campuses")).rejects.toBeInstanceOf(CtApiError); + + const logins = urls(fetchMock).filter((u) => u.includes("login_token")); + expect(logins).toHaveLength(2); // the initial login + exactly one automatic retry + }); + + it("a recovered 401 restores the budget, so a second expiry later in the run still self-heals", async () => { + // The storm guard must not be a lifetime cap: a long-running `apply` can outlive two + // sessions, and a 401 that was never about the session at all (CT answers 401 for a CSRF + // failure too) must not silently spend the run's only self-heal. + const cache = memoryCache(); + let rejectNext = false; + const fetchMock = vi.fn().mockImplementation((input) => { + const url = String(input); + if (url.includes("login_token")) { + return Promise.resolve(jsonResponse({ data: { id: 7 } }, { setCookie: "s=1; Path=/" })); + } + if (url.includes("csrftoken")) { + return Promise.resolve(jsonResponse({ data: "csrf" })); + } + if (rejectNext) { + rejectNext = false; + return Promise.resolve(jsonResponse({ message: "unauthorized" }, { status: 401 })); + } + return Promise.resolve(jsonResponse({ data: [{ id: 1 }] })); + }); + vi.stubGlobal("fetch", fetchMock); + + const client = new CtClient({ host: HOST }, { sessionCache: cache }); + await client.authenticate("tok"); + rejectNext = true; + await expect(client.get("/campuses")).resolves.toEqual([{ id: 1 }]); + rejectNext = true; + await expect(client.get("/campuses")).resolves.toEqual([{ id: 1 }]); + + const logins = urls(fetchMock).filter((u) => u.includes("login_token")); + expect(logins).toHaveLength(3); // the initial login + one per recovered 401 + }); + + it("a client without a session cache behaves exactly as before (always handshakes)", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "s=1; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf" })); + vi.stubGlobal("fetch", fetchMock); + + await new CtClient({ host: HOST }).authenticate("tok"); + + expect(urls(fetchMock)[0]).toContain("login_token=tok"); + }); + + it("`fresh: true` ignores a cached session so `ct auth login` really verifies the token", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "ct_session=abc", csrfToken: "csrf-1" }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ data: { id: 7 } }, { setCookie: "s=2; Path=/" })) + .mockResolvedValueOnce(jsonResponse({ data: "csrf-2" })); + vi.stubGlobal("fetch", fetchMock); + + await new CtClient({ host: HOST }, { sessionCache: cache }).authenticate("tok", { fresh: true }); + + expect(urls(fetchMock)[0]).toContain("login_token=tok"); + }); + + it("a 500 during the resume probe propagates instead of silently re-logging-in", async () => { + const cache = memoryCache(); + await cache.save(HOST, "tok", { cookie: "ct_session=abc", csrfToken: "csrf-1" }); + const fetchMock = vi + .fn() + .mockResolvedValue(jsonResponse({ message: "boom" }, { status: 500 })); + vi.stubGlobal("fetch", fetchMock); + + await expect( + new CtClient({ host: HOST }, { sessionCache: cache }).authenticate("tok"), + ).rejects.toMatchObject({ name: "CtApiError", status: 500 }); + expect(urls(fetchMock).join(" ")).not.toContain("login_token"); + }); +}); + +describe("login rate limit (HTTP 429) message", () => { + it("says the login rate limit was hit — not that the credentials failed", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue(jsonResponse({ message: "Too many requests" }, { status: 429 })); + vi.stubGlobal("fetch", fetchMock); + + await expect(new CtClient({ host: HOST }).authenticate("tok")).rejects.toMatchObject({ + name: "CtApiError", + status: 429, + message: expect.stringContaining("rate-limiting logins"), + }); + }); + + it("quotes the wait from Retry-After when the server sent one", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue( + jsonResponse({ message: "Too many requests" }, { status: 429, headers: { "retry-after": "180" } }), + ); + vi.stubGlobal("fetch", fetchMock); + + await expect(new CtClient({ host: HOST }).authenticate("tok")).rejects.toThrow(/about 3 minutes/); + }); + + it("never leaks the login token into the error message", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue(jsonResponse({ message: "Too many requests" }, { status: 429 })); + vi.stubGlobal("fetch", fetchMock); + + const err = await new CtClient({ host: HOST }).authenticate("s3cret-token").catch((e: unknown) => e); + expect((err as Error).message).not.toContain("s3cret-token"); + }); +}); diff --git a/tests/sessionStore.test.ts b/tests/sessionStore.test.ts new file mode 100644 index 0000000..ed07de5 --- /dev/null +++ b/tests/sessionStore.test.ts @@ -0,0 +1,161 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// The session cache lives in the macOS Keychain (never a file — see sessionStore.ts). +// Mock the platform and the `security` spawn so we can model what is stored. +const execFileMock = vi.hoisted(() => vi.fn()); +const platformMock = vi.hoisted(() => vi.fn(() => "darwin")); +vi.mock("node:child_process", () => ({ execFile: execFileMock })); +vi.mock("node:os", () => ({ platform: platformMock })); + +import { + readSession, + storeSession, + clearSession, + tokenFingerprint, + SESSION_MAX_AGE_MS, +} from "../src/auth/sessionStore.js"; +import { clearCredentials, storeCredentials, resetKeychainCache } from "../src/auth/tokenStore.js"; + +const HOST = "https://mychurch.church.tools"; +const OTHER = "https://other.church.tools"; + +let store: Map; + +/** Model a keychain as account → stored blob; wire the `security` CLI mock to read/write it. */ +function mockKeychain(): void { + execFileMock.mockImplementation((_cmd: string, args: string[], cb: (e: unknown, v: unknown) => void) => { + const sub = args[0]; + const account = args[args.indexOf("-a") + 1]!; + if (sub === "find-generic-password") { + const val = store.get(account); + if (val === undefined) return cb(new Error("not found"), null); + return cb(null, { stdout: val, stderr: "" }); + } + if (sub === "add-generic-password") { + store.set(account, args[args.indexOf("-w") + 1]!); + return cb(null, { stdout: "", stderr: "" }); + } + if (sub === "delete-generic-password") { + store.delete(account); + return cb(null, { stdout: "", stderr: "" }); + } + return cb(new Error(`unexpected security subcommand ${sub}`), null); + }); +} + +beforeEach(() => { + execFileMock.mockReset(); + platformMock.mockReturnValue("darwin"); + store = new Map(); + mockKeychain(); + resetKeychainCache(); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("session store (#145)", () => { + it("round-trips a session for a host", async () => { + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + expect(await readSession(HOST, "tok")).toMatchObject({ + host: HOST, + cookie: "ct=abc", + csrfToken: "csrf", + }); + }); + + it("keeps the session under its own account, not the credential account", async () => { + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + expect([...store.keys()]).toEqual([`session:${HOST}`]); + expect(store.has(HOST)).toBe(false); + expect(store.has("credentials")).toBe(false); + }); + + it("stores only a hash of the login token, never the token itself", async () => { + await storeSession(HOST, "s3cret-token", { cookie: "ct=abc", csrfToken: "csrf" }); + const blob = store.get(`session:${HOST}`)!; + expect(blob).not.toContain("s3cret-token"); + expect(blob).toContain(tokenFingerprint("s3cret-token")); + }); + + it("never returns host A's session for host B (#30)", async () => { + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + expect(await readSession(OTHER, "tok")).toBeNull(); + }); + + it("refuses a blob whose stored host does not match the requested one", async () => { + // A tampered / mis-keyed entry: right account, wrong host inside. + store.set( + `session:${HOST}`, + JSON.stringify({ + host: OTHER, + cookie: "ct=abc", + csrfToken: "csrf", + obtainedAt: Date.now(), + tokenHash: tokenFingerprint("tok"), + }), + ); + expect(await readSession(HOST, "tok")).toBeNull(); + }); + + it("does not reuse a session bought with a different token", async () => { + await storeSession(HOST, "tok-a", { cookie: "ct=abc", csrfToken: "csrf" }); + expect(await readSession(HOST, "tok-b")).toBeNull(); + expect(await readSession(HOST, "tok-a")).not.toBeNull(); + }); + + it("ages a session out, and rejects one timestamped in the future", async () => { + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + vi.useFakeTimers(); + vi.setSystemTime(Date.now() + SESSION_MAX_AGE_MS + 1000); + expect(await readSession(HOST, "tok")).toBeNull(); + + vi.setSystemTime(Date.now() - 2 * SESSION_MAX_AGE_MS); // clock moved backwards + expect(await readSession(HOST, "tok")).toBeNull(); + }); + + it("treats a corrupt blob as no session rather than throwing", async () => { + store.set(`session:${HOST}`, "not json at all"); + expect(await readSession(HOST, "tok")).toBeNull(); + }); + + it("stores nothing and reads nothing on a platform without a keychain", async () => { + platformMock.mockReturnValue("linux"); + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + expect(store.size).toBe(0); // no plaintext cookie on disk anywhere + expect(await readSession(HOST, "tok")).toBeNull(); + }); + + it("clearSession removes only that host's session", async () => { + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + await storeSession(OTHER, "tok", { cookie: "ct=xyz", csrfToken: "csrf" }); + await clearSession(HOST); + expect(await readSession(HOST, "tok")).toBeNull(); + expect(await readSession(OTHER, "tok")).not.toBeNull(); + }); +}); + +describe("logout clears the cached session along with the token", () => { + it("`ct auth logout` (no --env) drops the default host's session", async () => { + await storeCredentials({ host: HOST, token: "tok" }); + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + + await clearCredentials(); + + expect(await readSession(HOST, "tok")).toBeNull(); + expect(store.has(HOST)).toBe(false); + }); + + it("`ct auth logout --env ` drops that host's session and leaves the others", async () => { + await storeCredentials({ host: HOST, token: "tok" }); + await storeCredentials({ host: OTHER, token: "tok2" }); + await storeSession(HOST, "tok", { cookie: "ct=abc", csrfToken: "csrf" }); + await storeSession(OTHER, "tok2", { cookie: "ct=xyz", csrfToken: "csrf" }); + + await clearCredentials(HOST); + + expect(await readSession(HOST, "tok")).toBeNull(); + expect(await readSession(OTHER, "tok2")).not.toBeNull(); + }); +});