From 4128186f342104a6e68db0cd24717b1be6536e36 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 04:33:01 +0000 Subject: [PATCH] test(plugin-auth): prove ADR-0069 D4 revokes live sessions; record sys_session as the session of record (#4785) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maintainer's ruling on #4785: the session of record is ALWAYS `sys_session` (the database); the kernel `cache` service serves auth as the rate-limit counter store only. Dual-write (`session.storeSessionInDatabase: true`) is rejected. No runtime change — this records the decision and proves the behaviour that depends on it. `session-of-record.test.ts` is the deliverable the conflict was hiding behind: ADR-0069 D4 declares three session controls (idle timeout, absolute lifetime, concurrent cap) that all revoke by writing the `sys_session` row, and nothing asserted that write actually ends a LIVE session. Every test here therefore asserts de-authentication of a real session cookie through the real better-auth pipeline, not that a column was stamped — a stamped row nobody reads is exactly the failure mode #4785 describes. Verified by mutation: neutering either enforcement path, or stamping `revoked_at` without expiring `expires_at`, turns the corresponding tests red. Two facts found while writing it, both pinned: - `AuthManager` never plumbs `storeSessionInDatabase`, so the rejected dual-write shape is unreachable through configuration; - the default composition (OIDC provider on) makes better-auth REFUSE to boot with a `secondaryStorage` rather than degrade quietly — so the standard `serve` cannot silently reach the broken architecture. ADR-0069: D2's "shared store" scoped to rate-limit counters, with a cache-backed session store named as a NEW decision requiring its own revocation-consistency requirements; D4 records `sys_session` as a precondition rather than a deployment preference; status lines made factual. `content/docs/kernel/contracts/cache-service.mdx` no longer lists session storage among the cache's uses, and says what `cacheSecondaryStorage()` costs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t --- .../session-of-record-is-sys-session.md | 53 ++ .../docs/kernel/contracts/cache-service.mdx | 2 +- ...069-enterprise-authentication-hardening.md | 41 +- .../plugin-auth/src/auth-plugin.test.ts | 9 + .../plugin-auth/src/session-of-record.test.ts | 499 ++++++++++++++++++ 5 files changed, 600 insertions(+), 4 deletions(-) create mode 100644 .changeset/session-of-record-is-sys-session.md create mode 100644 packages/plugins/plugin-auth/src/session-of-record.test.ts diff --git a/.changeset/session-of-record-is-sys-session.md b/.changeset/session-of-record-is-sys-session.md new file mode 100644 index 0000000000..8c1c1fbc3e --- /dev/null +++ b/.changeset/session-of-record-is-sys-session.md @@ -0,0 +1,53 @@ +--- +"@objectstack/plugin-auth": patch +--- + +docs(plugin-auth): the session of record is always `sys_session` — cache backs rate-limit counters only (#4785) + +Settles an architectural question that had been answered two different ways by +the code and the docs. **Nothing about the runtime changes**: this records the +decision, proves the behaviour that depends on it, and corrects the docs that +described the road not taken. + +**The decision.** ObjectStack's session of record is always the `sys_session` +table. The kernel `cache` service serves authentication as the ADR-0069 D2 +rate-limit counter store and nothing else. It is never bound as better-auth's +`secondaryStorage`, because that option is not a counter store — handing +better-auth one also relocates sessions into it (`createSession` skips the +`sys_session` row; `findSession` answers from the cached snapshot without +reading the database). ADR-0069 D4's three session controls — idle timeout, +absolute lifetime, concurrent-session cap — all revoke by writing that row, so a +cache-backed session store would silently disable every one of them. Dual-writing +(`session.storeSessionInDatabase: true`) was considered and rejected as the worst +of the options: the row exists, so the controls *appear* to work, while the read +path still answers from the cache. + +**Why this needed settling rather than just fixing.** The conflict had never +fired — the cache lookup that would have wired `secondaryStorage` ran before the +cache service registered, so the binding never took in a standard composition. +The declaration and the runtime disagreed for a month and no test could tell, +because no test asserted that a D4 control ends a *live session*; they asserted +at most that a row got stamped. A stamped row nobody reads is exactly the failure +mode in question. + +**What is new.** `session-of-record.test.ts` drives the real better-auth pipeline +end to end and proves each of the three D4 controls actually de-authenticates a +live session cookie — not that a column was written. It also pins the +counter-factual: with a `secondaryStorage` bound, `sys_session` stays empty and +the idle timeout never fires. Two facts that make the guarantee hold for real +deployments are pinned with it — `AuthManager` does not plumb +`storeSessionInDatabase`, so the rejected dual-write shape is unreachable through +configuration; and the default composition (OIDC provider on) makes better-auth +*refuse to boot* with a `secondaryStorage` rather than degrade quietly. + +**For hosts.** `cacheSecondaryStorage()` remains exported for anyone who wants +better-auth's cached session store deliberately. It now says plainly what it +costs: opting in disables the ADR-0069 D4 session controls, and a revoked session +stays usable until its cached copy expires. Moving sessions into the cache +platform-wide would be a new decision requiring its own revocation-consistency +requirements, not a configuration change. + +ADR-0069's D2 "shared store" is scoped to rate-limit counters, D4 records +`sys_session` as a precondition rather than a deployment preference, and the +`ICacheService` contract page no longer lists session storage among the cache's +uses. diff --git a/content/docs/kernel/contracts/cache-service.mdx b/content/docs/kernel/contracts/cache-service.mdx index 9ae03f3964..0dace2dd34 100644 --- a/content/docs/kernel/contracts/cache-service.mdx +++ b/content/docs/kernel/contracts/cache-service.mdx @@ -5,7 +5,7 @@ description: Reference for the Cache Service contract — key-value caching with # ICacheService Contract -The Cache Service provides a **key-value caching** layer used throughout ObjectStack for metadata caching, query result caching, session storage, and rate limiting. Implementations can range from in-memory Maps to Redis clusters. +The Cache Service provides a **key-value caching** layer used throughout ObjectStack for metadata caching, query result caching, and rate limiting. Implementations can range from in-memory Maps to Redis clusters. It does **not** store sessions: the session of record is always the `sys_session` table, because ADR-0069 D4's session controls (idle timeout, absolute lifetime, concurrent-session cap) revoke a session by writing that row. A host may opt in explicitly with `cacheSecondaryStorage()` from `@objectstack/plugin-auth`, but should know what it buys: better-auth then answers session lookups from the cached snapshot without reading the database, so **opting in disables all three D4 session controls** — a revoked session stays usable until its cached copy expires. **Source:** `packages/spec/src/contracts/cache-service.ts` diff --git a/docs/adr/0069-enterprise-authentication-hardening.md b/docs/adr/0069-enterprise-authentication-hardening.md index dfd426cca5..e99d1ee699 100644 --- a/docs/adr/0069-enterprise-authentication-hardening.md +++ b/docs/adr/0069-enterprise-authentication-hardening.md @@ -1,6 +1,6 @@ # ADR-0069: Enterprise authentication hardening — password policy, enforced MFA, SSO, session controls, network gating, and anti-brute-force, all enforcement-wired -**Status**: Accepted — **P1 + P2 implemented** (2026-07-04); P3 partially landed (see Addendum). Original proposal 2026-06-24. +**Status**: Accepted — **P1 + P2 implemented** (2026-07-04); P3 partially landed (see Addendum). Original proposal 2026-06-24. Session-of-record question settled 2026-08-04 (#4785): sessions live in `sys_session`; the kernel `cache` service backs D2's rate-limit counters only. **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (**the governing constraint** — a security property that isn't enforced at runtime is forbidden; no toggle may be a "false surface"), [ADR-0007](./0007-settings-manifest-and-kv-store.md) (settings manifest + cascade KV store), [ADR-0057](./0057-erp-authorization-core-business-units-and-scope-depth.md) (`sys_role` is platform-native, decoupled from better-auth; org scoping), [ADR-0066](./0066-unified-authorization-model.md) (capability/assignment split), [ADR-0068](./0068-unified-user-context-and-built-in-identity-roles.md) (`current_user` contract, built-in roles) **Consumers**: `@objectstack/plugin-auth` (better-auth wiring, `bindAuthSettings`/`applyConfigPatch`, auth route middleware), `@objectstack/service-settings` (`auth.manifest.ts`), `@objectstack/platform-objects` (identity objects `sys_user`/`sys_session`/`sys_account`), `@objectstack/rest` (auth request middleware seam), `../objectui` (settings UI rendering) @@ -58,7 +58,21 @@ Legend: **[native]** = a better-auth config/plugin does the enforcing; **[custom |---|---|---|---| | `lockout_threshold` (failed attempts, 0 = off) | 0 | `before`/`after` hook on `/sign-in/email` | **[custom]+[field]** `sys_user.failed_login_count`, `sys_user.locked_until`; increment on failure, set `locked_until = now + lockout_duration` past threshold, **reject even on correct password** while locked, reset on success | | `lockout_duration_minutes` | 15 | as above | **[custom]** | -| `rate_limit_window_seconds` / `rate_limit_max` (per IP, auth endpoints) | 60 / 10 | better-auth core `rateLimit` | **[native]** enable + tune better-auth `rateLimit` (stricter `customRules` for `/sign-in/*`, `/sign-up/*`, `/reset-password`); use a **shared store** (not in-memory) for multi-node | +| `rate_limit_window_seconds` / `rate_limit_max` (per IP, auth endpoints) | 60 / 10 | better-auth core `rateLimit` | **[native]** enable + tune better-auth `rateLimit` (stricter `customRules` for `/sign-in/*`, `/sign-up/*`, `/reset-password`); use a **shared store** (not in-memory) for multi-node — see the scoping note below | + +> **What "shared store" means here, exactly: rate-limit counters and nothing else (#4785).** +> The store is wired as better-auth's `rateLimit.customStorage`, fed by the kernel `cache` +> service (`createLazyCacheRateLimitStorage`), and it holds **counters only**. It is +> deliberately **not** better-auth's `secondaryStorage`, because that option is not a +> counter store — handing better-auth one also relocates the **session of record** into it +> (`createSession` skips the `sys_session` row, `findSession` answers from the cached +> snapshot without reading the database), which silently disables every D4 control below. +> **The session of record is always `sys_session`, the database.** Moving it into the cache +> would be a NEW decision, not an implementation detail of this row: it would have to +> supersede D4's revocation mechanism and state its own revocation-consistency +> requirements — how a revocation invalidates the cached snapshot on every node, and what +> happens when that invalidation fails — in the same ADR that proposes it. Absent such a +> decision, a cache-backed session store is out of scope for D2. > Distinction that matters: better-auth `rateLimit` throttles **requests per IP/path** (native); **account lockout** (per-identity, survives IP rotation) is **custom** and needs the two `sys_user` fields above + an admin "unlock" action. @@ -82,6 +96,27 @@ Legend: **[native]** = a better-auth config/plugin does the enforcing; **[custom | "Sign out all other sessions" | — | — | **[native]** already wired (`/revoke-other-sessions`, action on `sys_session`) — keep | | `session_expiry_days` / `session_refresh_days` | 7 / 1 | `session.expiresIn`/`updateAge` | **[native]** existing — keep | +> **The session of record is `sys_session` — the database (#4785).** All three controls +> above revoke the same way: stamp the row (`expires_at` into the past, plus +> `revoked_at`/`revoke_reason` for the audit trail). That mechanism is only sound while +> better-auth reads sessions from the database, so this is a **precondition of D4, not a +> deployment preference**: the kernel `cache` service serves auth as the D2 rate-limit +> counter store only, and is never bound as `secondaryStorage`. `expires_at` is the field +> that actually revokes — better-auth's session read checks it and nothing else, so +> `revoked_at`/`revoke_reason` are diagnostics, and a revocation that stamped only those +> would be inert. +> +> Two of these three controls are enforced in `customSession`, which runs **after** the +> session for the current request has already been validated, so idle-timeout and +> absolute-max take effect on the **next** request (the detecting request still succeeds). +> The concurrent cap runs in the sign-in after-hook and is effective immediately. +> +> `packages/plugins/plugin-auth/src/session-of-record.test.ts` proves each control really +> ends a live session end-to-end through the real better-auth pipeline, and pins the +> counter-factual — that a cache-backed session store makes them inert. The absence of +> exactly that test is why the conflict in #4785 stayed invisible from 2026-07-04 until it +> was found by reading the code. + ### D5 — Network gating / IP allowlist (P2) | Setting | Default | Seam | Mechanism | @@ -139,7 +174,7 @@ Each row in D1-D6 names exactly one of these seams. No setting is introduced wit | Phase | Status | Notes | |---|---|---| | **P1** (D1/D2/D3 + D7 fields) | ✅ **implemented** | Password complexity/history/expiry (`assertPasswordComplexity`/`assertPasswordNotReused`/`stampPasswordChangedAt`), HIBP (`haveIBeenPwned` plugin), account lockout (`assertAccountNotLocked`/`recordSignInOutcome` + `unlock_user` action), enforced MFA + grace (`computeAuthGate` → `MFA_REQUIRED`, per-org `require_mfa`), rate-limit tuning (`customRules`). All settings in `auth.manifest.ts`, bound via `bindAuthSettings`. Login-audit fields `last_login_at`/`last_login_ip` stamped on sign-in (`stampLastLogin`). | -| **P2** (D4/D5) | 🟡 **mostly implemented** | Session idle/absolute/concurrent (`enforceSessionControls`/`enforceConcurrentCap`), the **global** IP allow-list (`isClientIpAllowed`, `auth.allowed_ip_ranges`), and the **shared multi-node rate-limit counters** (better-auth `rateLimit.customStorage` fed by the kernel cache service through `createLazyCacheRateLimitStorage`; shared iff the cache is — Redis adapter in a cluster) are landed. **Correction (#4772):** this row previously claimed a shared **session** store via `secondaryStorage` as landed. It was not: the binding was taken in `AuthPlugin.init()`, which runs *before* `CacheServicePlugin` registers `cache`, so it never fired in the standard composition — and the counters it was supposed to share never reached the cache either. The counters now ride `rateLimit.customStorage`, resolved at counting time. The **session** half is deliberately NOT auto-wired: better-auth answers `findSession` from a `secondaryStorage` snapshot without reading the database, while D4 above revokes by writing the `sys_session` row, so a cache-backed session store silently disables idle-timeout / absolute-max / concurrent-cap enforcement. `cacheSecondaryStorage` remains exported for a host that opts into that trade knowingly. **Remaining:** per-org `sys_organization.allowed_ip_ranges` (+ optional `sys_user.allowed_ip_ranges` override) — tracked in #2571; the session-store question — tracked in #4785. | +| **P2** (D4/D5) | 🟡 **mostly implemented** | Session idle/absolute/concurrent (`enforceSessionControls`/`enforceConcurrentCap`), the **global** IP allow-list (`isClientIpAllowed`, `auth.allowed_ip_ranges`), and the **shared multi-node rate-limit counters** (better-auth `rateLimit.customStorage` fed by the kernel cache service through `createLazyCacheRateLimitStorage`; shared iff the cache is — Redis adapter in a cluster) are landed. **Correction (#4772):** this row previously claimed a shared **session** store via `secondaryStorage` as landed. It was not: the binding was taken in `AuthPlugin.init()`, which runs *before* `CacheServicePlugin` registers `cache`, so it never fired in the standard composition — and the counters it was supposed to share never reached the cache either. The counters now ride `rateLimit.customStorage`, resolved at counting time. The **session** half is NOT wired, by decision: better-auth answers `findSession` from a `secondaryStorage` snapshot without reading the database, while D4 above revokes by writing the `sys_session` row, so a cache-backed session store silently disables idle-timeout / absolute-max / concurrent-cap enforcement. **Settled 2026-08-04 (#4785): the session of record is always `sys_session`; cache serves auth as the rate-limit counter store only.** `cacheSecondaryStorage` remains exported for a host that opts into that trade knowingly and accepts that it disables the D4 controls — the default composition cannot reach it silently, because the OIDC provider plugin is on by default and better-auth refuses `secondaryStorage` without `session.storeSessionInDatabase`, which `AuthManager` does not plumb. Dual-writing (`storeSessionInDatabase: true`) was considered and **rejected**: the row exists so the controls appear to work, while the read path still answers from the cache. All three controls are proven end-to-end in `session-of-record.test.ts`. **Remaining:** per-org `sys_organization.allowed_ip_ranges` (+ optional `sys_user.allowed_ip_ranges` override) — tracked in #2571. | | **P2/P3** (D6) | 🟡 partial | Generic OIDC RP wired (`genericOAuth`/`sso`); admin OIDC **trust-list settings UI** still env/`sys_sso_provider`-only. | | **P3** (SAML, broader social) | 🟡 partial | `@better-auth/sso` present (SAML now better-auth-native — see Addendum); broader settings-driven social providers pending. | diff --git a/packages/plugins/plugin-auth/src/auth-plugin.test.ts b/packages/plugins/plugin-auth/src/auth-plugin.test.ts index 895c73ccfb..87a16cfb57 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.test.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.test.ts @@ -1315,6 +1315,15 @@ describe('AuthPlugin', () => { // better-auth answers `findSession` from a secondaryStorage snapshot // without reading the database, so a cache-backed session store would // silently disable them. The cache reaches the COUNTERS only. + // + // #4785 settled this as a DECISION, not a workaround: the session of + // record is always `sys_session`. This assertion is the wiring half of + // that decision; `session-of-record.test.ts` is the other half — it + // drives the real better-auth pipeline to prove each of the three D4 + // controls actually ends a live session, and pins the counter-factual + // (with a `secondaryStorage` bound, `sys_session` stays EMPTY and the + // idle timeout never fires). Read the two together before changing + // either: this line alone says what we do, not why it matters. const cache = makeCache(); const { manager } = await bootWith(async () => cache); expect((manager as any).config.secondaryStorage).toBeUndefined(); diff --git a/packages/plugins/plugin-auth/src/session-of-record.test.ts b/packages/plugins/plugin-auth/src/session-of-record.test.ts new file mode 100644 index 0000000000..be8f357b6f --- /dev/null +++ b/packages/plugins/plugin-auth/src/session-of-record.test.ts @@ -0,0 +1,499 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4785 — "where is the session of record?", settled: it is ALWAYS `sys_session` +// (the database). The kernel `cache` service serves auth as the rate-limit +// counter store and nothing else. +// +// These are the tests whose ABSENCE let the conflict stay invisible. ADR-0069 D4 +// declares three session controls — idle timeout, absolute lifetime, concurrent +// cap — and all three revoke the same way: by writing the `sys_session` row +// (`expires_at` into the past + `revoked_at`/`revoke_reason`). Nothing proved +// that write actually stopped a live session, so when better-auth's +// `secondaryStorage` was (nearly) wired to the cache, no test could tell that +// D4 had silently stopped working. +// +// So every test here asserts the END of the chain, not the middle: a request +// that carries a real session cookie stops being authenticated. Asserting only +// that the row was stamped would re-create the exact blind spot — a stamped row +// nobody reads is precisely the failure mode #4785 describes. +// +// Real better-auth pipeline throughout (the #3585 EdDSA tests set this +// precedent: patch the real thing, never stub our own code). Requests go in as +// `Request` objects through `AuthManager.handleRequest`; the session cookie is +// the one better-auth minted; the revocation path is the one `customSession` +// and the sign-in after-hook really run. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; +import { AuthManager } from './auth-manager'; +import { cacheSecondaryStorage } from './secondary-storage'; + +/** + * In-memory IDataEngine, same shape as the #3585 harness. + * + * Two deliberate fidelity choices, because this file's whole value is that the + * fake cannot be more forgiving than the real engine: + * + * - **`fields` really projects.** `enforceSessionControls` asks for + * `['id','created_at','last_activity_at','revoked_at']` and `enforceConcurrentCap` + * for `['id','created_at','expires_at','revoked_at']`; the ObjectQL adapter + * forwards better-auth's `select` as `fields` too. A fake that returned whole + * rows would let a control read a column it never requested and still pass. + * - **`delete` is pinned to ObjectQL's own dispatch predicate** + * ({@link assertEngineDeleteDispatch}), unlike the rest of this fake. This one + * fires for real here: expiring a session in place makes better-auth's + * `getSession` clean the row up via `deleteSession(token)`, so the delete path + * is genuinely exercised by every revocation test below. + */ +const createMemoryEngine = () => { + const tables = new Map(); + const rows = (name: string) => { + if (!tables.has(name)) tables.set(name, []); + return tables.get(name)!; + }; + const eq = (a: any, b: any) => + a instanceof Date || b instanceof Date + ? new Date(a as any).getTime() === new Date(b as any).getTime() + : a === b; + const matches = (row: any, where: Record = {}) => + Object.entries(where).every(([k, v]) => { + const actual = row[k]; + if (v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date)) { + if ('$ne' in v) return !eq(actual, v.$ne); + if ('$in' in v) return (v.$in as any[]).some((x) => eq(actual, x)); + if ('$gt' in v) return actual > v.$gt; + if ('$gte' in v) return actual >= v.$gte; + if ('$lt' in v) return actual < v.$lt; + if ('$lte' in v) return actual <= v.$lte; + if ('$regex' in v) return new RegExp(String(v.$regex)).test(String(actual ?? '')); + } + return eq(actual, v); + }); + /** `fields` projection — `id` always survives, as it does in ObjectQL. */ + const project = (row: any, fields?: string[]) => { + if (!Array.isArray(fields) || fields.length === 0) return { ...row }; + const out: any = {}; + for (const f of ['id', ...fields]) if (f in row) out[f] = row[f]; + return out; + }; + let seq = 0; + return { + tables, + async insert(name: string, data: any) { + const row = { id: data.id ?? `row_${++seq}`, ...data }; + rows(name).push(row); + return { ...row }; + }, + async findOne(name: string, q: any = {}) { + const row = rows(name).find((r) => matches(r, q.where)); + return row ? project(row, q.fields) : null; + }, + async find(name: string, q: any = {}) { + let out = rows(name).filter((r) => matches(r, q.where)); + const order = q.orderBy?.[0]; + if (order) { + out = [...out].sort( + (a, b) => (a[order.field] > b[order.field] ? 1 : -1) * (order.order === 'desc' ? -1 : 1), + ); + } + if (q.offset) out = out.slice(q.offset); + if (q.limit) out = out.slice(0, q.limit); + return out.map((r) => project(r, q.fields)); + }, + async count(name: string, q: any = {}) { + return rows(name).filter((r) => matches(r, q.where)).length; + }, + async update(name: string, patch: any) { + const row = rows(name).find((r) => r.id === patch.id); + if (!row) return null; + Object.assign(row, patch); + return { ...row }; + }, + async delete(name: string, q: any = {}) { + assertEngineDeleteDispatch(q); + const table = rows(name); + const keep = table.filter((r) => !matches(r, q.where)); + tables.set(name, keep); + return table.length - keep.length; + }, + }; +}; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-4785'; + +const makeManager = (engine: any, config: Record = {}) => + new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + ...config, + } as any); + +const signUp = (manager: AuthManager, email: string) => + manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/sign-up/email', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, password: PASSWORD, name: 'Session Of Record' }), + }), + ); + +const signIn = (manager: AuthManager, email: string) => + manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/sign-in/email', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email, password: PASSWORD }), + }), + ); + +const getSession = (manager: AuthManager, cookie: string) => + manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/get-session', { headers: { cookie } }), + ); + +const cookieFrom = (response: Response): string => + (response.headers.getSetCookie?.() ?? [response.headers.get('set-cookie') ?? '']) + .map((c) => c.split(';')[0]) + .filter(Boolean) + .join('; '); + +/** + * Is this cookie still authenticated? + * + * better-auth answers `/get-session` with HTTP 200 and a JSON `null` body when + * the session is gone — NOT a 401 — so a status-only assertion would pass + * against a fully revoked session. Read the body. + */ +const isAuthenticated = async (manager: AuthManager, cookie: string): Promise => { + const res = await getSession(manager, cookie); + if (res.status !== 200) return false; + const body = await res.json().catch(() => null); + return Boolean((body as any)?.user?.id); +}; + +/** The single `sys_session` row for a cookie's session, straight from the table. */ +const sessionRows = (engine: any) => (engine.tables.get('sys_session') ?? []) as any[]; + +/** Backdate columns on a stored session row, simulating the passage of time. */ +const ageSession = (engine: any, id: string, patch: Record) => { + const row = sessionRows(engine).find((r) => r.id === id); + if (!row) throw new Error(`no sys_session row ${id}`); + Object.assign(row, patch); +}; + +const MINUTE = 60_000; +const HOUR = 60 * MINUTE; + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#4785 — the session of record is `sys_session` (the database)', () => { + it('a sign-up writes a real sys_session row, and the cookie authenticates against it', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + + const signed = await signUp(manager, 'record@example.com'); + expect(signed.status).toBe(200); + const cookie = cookieFrom(signed); + + // The premise every D4 control rests on: the row EXISTS in the database. + expect(sessionRows(engine)).toHaveLength(1); + expect(await isAuthenticated(manager, cookie)).toBe(true); + }); + + it('deleting the row de-authenticates the cookie — the database is what is read', async () => { + // The converse of the test above, and the one that actually proves "of + // record": if better-auth were answering from anywhere else, dropping the + // row would leave the cookie working. + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const cookie = cookieFrom(await signUp(manager, 'authority@example.com')); + expect(await isAuthenticated(manager, cookie)).toBe(true); + + engine.tables.set('sys_session', []); + + expect(await isAuthenticated(manager, cookie)).toBe(false); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('ADR-0069 D4 — idle timeout really revokes a live session', () => { + it('an idle session stops being authenticated, and says why in the row', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { sessionIdleTimeoutMinutes: 30 }); + + const cookie = cookieFrom(await signUp(manager, 'idle@example.com')); + const id = sessionRows(engine)[0]!.id; + expect(await isAuthenticated(manager, cookie)).toBe(true); + + // 90 minutes of inactivity against a 30-minute timeout. `created_at` stays + // recent so ONLY the idle rule can fire — this test must not pass for the + // absolute-lifetime reason. + ageSession(engine, id, { last_activity_at: new Date(Date.now() - 90 * MINUTE) }); + + // `enforceSessionControls` runs inside `customSession`, i.e. AFTER + // better-auth has already validated this request's session. So the request + // that DETECTS the timeout is still authenticated, and the next one is not. + // That one-request lag is the documented design, and pinning it here is + // what stops someone "fixing" the lag by moving the check somewhere the + // revocation write no longer happens. + expect(await isAuthenticated(manager, cookie)).toBe(true); + + const row = sessionRows(engine).find((r) => r.id === id)!; + expect(row.revoke_reason).toBe('idle_timeout'); + expect(row.revoked_at).toBeInstanceOf(Date); + expect(new Date(row.expires_at).getTime()).toBeLessThan(Date.now()); + + // The assertion that matters: the cookie is dead. + expect(await isAuthenticated(manager, cookie)).toBe(false); + }); + + it('an ACTIVE session is not revoked, and its last_activity_at is touched', async () => { + // The other half of the control: a timeout that revokes everything would + // pass every assertion above and be useless. + const engine = createMemoryEngine(); + const manager = makeManager(engine, { sessionIdleTimeoutMinutes: 30 }); + + const cookie = cookieFrom(await signUp(manager, 'active@example.com')); + const id = sessionRows(engine)[0]!.id; + + // 5 minutes idle against a 30-minute timeout — inside the window. Backdated + // past the 60s touch throttle so the touch is observable. + ageSession(engine, id, { last_activity_at: new Date(Date.now() - 5 * MINUTE) }); + + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(await isAuthenticated(manager, cookie)).toBe(true); + + const row = sessionRows(engine).find((r) => r.id === id)!; + expect(row.revoked_at).toBeUndefined(); + expect(row.revoke_reason).toBeUndefined(); + expect(Date.now() - new Date(row.last_activity_at).getTime()).toBeLessThan(MINUTE); + }); + + it('is off when the setting is 0 — an ancient session stays authenticated', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { sessionIdleTimeoutMinutes: 0 }); + + const cookie = cookieFrom(await signUp(manager, 'off@example.com')); + ageSession(engine, sessionRows(engine)[0]!.id, { + last_activity_at: new Date(Date.now() - 400 * HOUR), + }); + + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(await isAuthenticated(manager, cookie)).toBe(true); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('ADR-0069 D4 — absolute lifetime really revokes a live session', () => { + it('a session past its absolute cap stops being authenticated however active it is', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { sessionAbsoluteMaxHours: 8 }); + + const cookie = cookieFrom(await signUp(manager, 'absolute@example.com')); + const id = sessionRows(engine)[0]!.id; + expect(await isAuthenticated(manager, cookie)).toBe(true); + + // Created 12h ago (cap 8h) but active RIGHT NOW. This is the case + // better-auth's own `updateAge` can never catch — a session that keeps + // sliding its window forever — which is why D4 calls the absolute cap + // custom. Idle is off, so only the absolute rule can fire. + ageSession(engine, id, { + created_at: new Date(Date.now() - 12 * HOUR), + last_activity_at: new Date(), + }); + + expect(await isAuthenticated(manager, cookie)).toBe(true); // detecting request + + const row = sessionRows(engine).find((r) => r.id === id)!; + expect(row.revoke_reason).toBe('absolute_max'); + expect(new Date(row.expires_at).getTime()).toBeLessThan(Date.now()); + + expect(await isAuthenticated(manager, cookie)).toBe(false); + }); + + it('a session inside its absolute cap survives', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { sessionAbsoluteMaxHours: 8 }); + + const cookie = cookieFrom(await signUp(manager, 'within@example.com')); + ageSession(engine, sessionRows(engine)[0]!.id, { + created_at: new Date(Date.now() - 2 * HOUR), + last_activity_at: new Date(), + }); + + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(sessionRows(engine)[0]!.revoke_reason).toBeUndefined(); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('ADR-0069 D4 — concurrent-session cap really revokes the oldest session', () => { + it('a third sign-in under cap=2 kills the oldest cookie and spares the newer two', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { maxConcurrentSessions: 2 }); + + // Three sign-ins for ONE user. `enforceConcurrentCap` runs from the + // `/sign-in/email` after-hook, so sign-up alone never triggers it. + const first = cookieFrom(await signUp(manager, 'cap@example.com')); + const firstId = sessionRows(engine)[0]!.id; + // The cap sorts by `created_at` desc. Rows minted inside the same + // millisecond would sort unstably and make this test flaky for a reason + // that has nothing to do with the control, so each session is given a + // distinct, explicitly ordered birth time. + ageSession(engine, firstId, { created_at: new Date(Date.now() - 3 * HOUR) }); + + const second = cookieFrom(await signIn(manager, 'cap@example.com')); + const secondId = sessionRows(engine).find((r) => r.id !== firstId)!.id; + ageSession(engine, secondId, { created_at: new Date(Date.now() - 2 * HOUR) }); + + // Both live so far — cap is 2. + expect(await isAuthenticated(manager, first)).toBe(true); + expect(await isAuthenticated(manager, second)).toBe(true); + + const third = cookieFrom(await signIn(manager, 'cap@example.com')); + + const firstRow = sessionRows(engine).find((r) => r.id === firstId)!; + expect(firstRow.revoke_reason).toBe('concurrent_cap'); + expect(new Date(firstRow.expires_at).getTime()).toBeLessThan(Date.now()); + + // Evict-oldest, not reject-newest: the oldest cookie is dead immediately + // (no detecting-request lag — the revocation landed before this read), and + // the two newest still work. + expect(await isAuthenticated(manager, first)).toBe(false); + expect(await isAuthenticated(manager, second)).toBe(true); + expect(await isAuthenticated(manager, third)).toBe(true); + }); + + it('is off when the setting is 0 — a third session survives', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { maxConcurrentSessions: 0 }); + + const first = cookieFrom(await signUp(manager, 'nocap@example.com')); + await signIn(manager, 'nocap@example.com'); + await signIn(manager, 'nocap@example.com'); + + expect(await isAuthenticated(manager, first)).toBe(true); + expect(sessionRows(engine).some((r) => r.revoke_reason)).toBe(false); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +describe('#4785 — why cache is NOT the session store (the rejected architecture)', () => { + // The counter-factual, run against the REAL adapter rather than argued in a + // comment. `auth-plugin.test.ts` pins that the kernel cache is never bound as + // `secondaryStorage`; this pins WHAT THAT PIN IS PROTECTING, so the cost of + // undoing it is visible in a failing test rather than in a security incident + // six months later. + const makeCache = () => { + const store = new Map(); + return { + store, + get: async (k: string) => (store.has(k) ? store.get(k) : undefined), + set: async (k: string, v: unknown) => void store.set(k, v), + delete: async (k: string) => store.delete(k), + has: async (k: string) => store.has(k), + clear: async () => store.clear(), + stats: async () => ({ hits: 0, misses: 0, keyCount: store.size }), + }; + }; + + it('opting into cacheSecondaryStorage empties sys_session and makes idle timeout inert', async () => { + const engine = createMemoryEngine(); + const cache = makeCache(); + const manager = makeManager(engine, { + sessionIdleTimeoutMinutes: 30, + secondaryStorage: cacheSecondaryStorage(cache as any), + // The OIDC provider is on by default and refuses this configuration + // outright — see the boot-refusal test below. Off here so the session + // relocation itself is what's under test. + plugins: { oidcProvider: false }, + }); + + const cookie = cookieFrom(await signUp(manager, 'cached@example.com')); + expect(await isAuthenticated(manager, cookie)).toBe(true); + + // 1. No row at all: `createSession` skips the insert when a + // `secondaryStorage` is present and `storeSessionInDatabase` is not set. + // The session lives in the cache instead. + expect(sessionRows(engine)).toHaveLength(0); + expect(cache.store.size).toBeGreaterThan(0); + + // 2. Therefore `enforceSessionControls` has nothing to find, nothing to + // stamp, and its `findOne` miss is indistinguishable from a healthy + // session — it returns early. The declared idle timeout is inert, and + // the cookie outlives it: the session survives for its full TTL. + // + // Ageing happens in the cache, which IS the store of record under this + // configuration — exactly where it would age in production. + for (const [k, v] of cache.store) { + if (typeof v !== 'string' || !v.includes('"session"')) continue; + const snap = JSON.parse(v); + snap.session.updatedAt = new Date(Date.now() - 90 * MINUTE).toISOString(); + cache.store.set(k, JSON.stringify(snap)); + } + + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(await isAuthenticated(manager, cookie)).toBe(true); + expect(sessionRows(engine)).toHaveLength(0); + }); + + it('option C (dual-write) is not reachable through config — asking for it changes nothing', async () => { + // The maintainer rejected `session.storeSessionInDatabase: true` as the + // WORST option: the row exists, so `enforceSessionControls` finds it and + // stamps it and every database-side assertion looks healthy — while + // `findSession` still answers from the cache snapshot, so revocation + // appears to work and does not. + // + // `AuthManager` never plumbs the flag: it builds better-auth's `session` + // block from `AUTH_SESSION_CONFIG` plus `expiresIn`/`updateAge` only, so a + // host that passes it is silently building the cache-only shape instead. + // Pinned behaviourally rather than by reading private options — this is the + // property that matters, and it holds no matter how the block is assembled. + const engine = createMemoryEngine(); + const cache = makeCache(); + const manager = makeManager(engine, { + sessionIdleTimeoutMinutes: 30, + secondaryStorage: cacheSecondaryStorage(cache as any), + plugins: { oidcProvider: false }, + session: { storeSessionInDatabase: true }, + }); + + const cookie = cookieFrom(await signUp(manager, 'dualwrite@example.com')); + expect(await isAuthenticated(manager, cookie)).toBe(true); + + // Requested dual-write; got cache-only. No row was ever written, so there + // is nothing for a D4 control to stamp. + expect(sessionRows(engine)).toHaveLength(0); + }); + + it('the DEFAULT composition refuses to boot with a secondaryStorage rather than degrading quietly', async () => { + // The safety net under all of the above, and the reason this hole could + // never have opened silently in a standard `serve`: the OIDC provider + // plugin is enabled by default, and better-auth 1.7 hard-refuses + // `secondaryStorage` without `storeSessionInDatabase`. Since AuthManager + // does not plumb that flag (test above), the default composition cannot + // reach the cache-backed session store at all — it throws at boot. + // + // Loud absence beats a quiet downgrade (AGENTS.md "Absence must be loud"), + // and this is what makes ADR-0069 D4's guarantee hold for real deployments + // rather than by convention. + const engine = createMemoryEngine(); + const cache = makeCache(); + const manager = makeManager(engine, { + sessionIdleTimeoutMinutes: 30, + secondaryStorage: cacheSecondaryStorage(cache as any), + }); + + await expect(signUp(manager, 'default@example.com')).rejects.toThrow( + /storeSessionInDatabase/, + ); + }); +});