diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index 28c1231..a862f0c 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -979,6 +979,277 @@ describe.sequential("live API — principal infrastructure failure is never down }); }); +describe.sequential("live API — session-store readiness against real PostgreSQL", () => { + /** + * Every scenario below drives a real, migrated PostgreSQL database, breaks exactly one property + * of the *session store specifically*, and asserts the public contract. The administrative + * connection each helper opens (`openAdministrativeClient`) is **test-only provisioning + * infrastructure** — it renames a relation, drops a column, or revokes a privilege in a + * disposable database, and is closed in `finally`. It is never part of the API runtime, which + * continues to own exactly one pool through `startApi`. + */ + async function withBrokenSessionStore( + label: string, + breakStore: (admin: Client) => Promise, + assertions: (context: { + readonly cookie: string; + readonly origin: string; + readonly logs: string[]; + }) => Promise, + ): Promise { + const database = await createDisposableDatabase(label); + await applyCoreMigrations(database.url); + const running = await startConfiguredApi(database.url); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + try { + const origin = apiOrigin(running); + const email = `${label}@example.test`; + const password = "correct horse battery staple"; + await signUp(origin, email, password); + const { cookiePair } = await signIn(origin, email, password); + expect(cookiePair).toBeDefined(); + + // The session resolves normally before anything is broken. + const before = await fetchPrincipal(origin, { cookie: cookiePair as string }); + expect(before.response.status).toBe(200); + expect(before.body).toMatchObject({ kind: "authenticated" }); + + const admin = await openAdministrativeClient(database.url); + try { + await breakStore(admin); + } finally { + await safeEnd(admin); + } + + await assertions({ + cookie: cookiePair as string, + logs: errorSpy.mock.calls.flat().map((value) => String(value)), + origin, + }); + } finally { + errorSpy.mockRestore(); + await running.shutdown().catch(() => undefined); + await dropDatabase(database.name); + } + } + + /** The exact public contract a broken session store must produce — never anonymous, never 403, + * and never carrying a relation name, column name, privilege detail, SQL, host, or stack. */ + async function expectSanitizedUnavailable(origin: string, cookie: string): Promise { + const response = await fetch(`${origin}/api/v1/principal`, { headers: { cookie } }); + expect(response.status).toBe(503); + expect(response.headers.get("cache-control")).toBe("no-store"); + const text = await response.clone().text(); + expect(JSON.parse(text)).toEqual({ code: "PRINCIPAL_RESOLUTION_FAILED" }); + for (const forbidden of [ + "ekklesia_core_session", + "permission denied", + "does not exist", + "relation", + "column", + "select", + "postgres", + "5432", + ]) { + expect(text.toLowerCase()).not.toContain(forbidden.toLowerCase()); + } + return response; + } + + it("returns a sanitized 503 — not anonymous — when the session table is missing, even though a generic SELECT 1 still succeeds", async () => { + await withBrokenSessionStore( + "session_missing_table", + async (admin) => { + // Prove the distinction this pass exists for: the connection is perfectly healthy for a + // trivial statement, so the previous `select 1` probe would have reported "reachable" and + // the caller would have been reported as genuinely anonymous. + const trivial = await safeQuery<{ one: number }>(admin, "SELECT 1 AS one"); + expect(trivial[0]?.one).toBe(1); + await safeQuery( + admin, + 'ALTER TABLE "ekklesia_core_session" RENAME TO "ekklesia_core_session_moved"', + ); + }, + async ({ cookie, origin }) => { + await expectSanitizedUnavailable(origin, cookie); + }, + ); + }); + + it("returns a sanitized 503 when an expected session column is missing", async () => { + await withBrokenSessionStore( + "session_missing_column", + async (admin) => { + await safeQuery(admin, 'ALTER TABLE "ekklesia_core_session" DROP COLUMN "token" CASCADE'); + }, + async ({ cookie, origin }) => { + await expectSanitizedUnavailable(origin, cookie); + }, + ); + }); + + it("returns a sanitized 503 when SELECT privilege on the session table is genuinely revoked", async () => { + // A real privilege denial needs the API to connect as a role that is *not* the table owner — + // an owner's implicit rights cannot be revoked. This provisions a dedicated restricted role + // (test-only infrastructure), grants it everything except SELECT on the session table, and + // runs the API entirely as that role. + const database = await createDisposableDatabase("session_denied"); + await applyCoreMigrations(database.url); + const restrictedRole = `ekk_restricted_${randomBytes(4).toString("hex")}`; + const restrictedPassword = randomBytes(12).toString("hex"); + + const admin = await openAdministrativeClient(database.url); + try { + await safeQuery( + admin, + `CREATE ROLE "${restrictedRole}" LOGIN PASSWORD '${restrictedPassword}'`, + ); + await safeQuery(admin, `GRANT USAGE ON SCHEMA public TO "${restrictedRole}"`); + await safeQuery( + admin, + `GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA public TO "${restrictedRole}"`, + ); + // The one thing this role may not do: read the session relation. + await safeQuery(admin, `REVOKE ALL ON "ekklesia_core_session" FROM "${restrictedRole}"`); + } finally { + await safeEnd(admin); + } + + const restrictedUrl = (() => { + const parsed = new URL(database.url); + parsed.username = restrictedRole; + parsed.password = restrictedPassword; + return parsed.toString(); + })(); + + const running = await startConfiguredApi(restrictedUrl); + try { + const origin = apiOrigin(running); + // The restricted role can still run a trivial statement and can still read non-session + // tables, so only the session store specifically is broken. + const response = await fetch(`${origin}/api/v1/principal`, { + headers: { cookie: "__Secure-better-auth.session_token=any-token-shape" }, + }); + const text = await response.clone().text(); + + expect(response.status).toBe(503); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(JSON.parse(text)).toEqual({ code: "PRINCIPAL_RESOLUTION_FAILED" }); + expect(text).not.toContain("ekklesia_core_session"); + expect(text.toLowerCase()).not.toContain("permission denied"); + expect(text).not.toContain(restrictedRole); + } finally { + await running.shutdown().catch(() => undefined); + // Drop the disposable database first (it holds the role's dependencies), then the role. + await dropDatabase(database.name).catch(() => undefined); + const cleanup = await openAdministrativeClient(baseDatabaseUrl()); + try { + await safeQuery(cleanup, `DROP ROLE IF EXISTS "${restrictedRole}"`); + } finally { + await safeEnd(cleanup); + } + } + }); + + it("keeps a genuinely unknown, expired, or revoked session anonymous while the store is healthy", async () => { + const database = await createDisposableDatabase("session_healthy"); + await applyCoreMigrations(database.url); + const running = await startConfiguredApi(database.url); + try { + const origin = apiOrigin(running); + const email = "healthy-store@example.test"; + const password = "correct horse battery staple"; + await signUp(origin, email, password); + const { cookiePair } = await signIn(origin, email, password); + expect(cookiePair).toBeDefined(); + const cookieName = (cookiePair as string).split("=")[0] as string; + + // A completely unknown token against a healthy store is genuinely anonymous. + const unknown = await fetchPrincipal(origin, { cookie: `${cookieName}=not-a-real-token` }); + expect(unknown.response.status).toBe(200); + expect(unknown.body).toEqual({ kind: "anonymous" }); + + // A revoked (signed-out) session is likewise genuinely anonymous, not a 503. + const signOut = await fetch(`${origin}/api/auth/sign-out`, { + body: "{}", + headers: { + "content-type": "application/json", + cookie: cookiePair as string, + origin: dashboardOrigin, + }, + method: "POST", + }); + expect(signOut.status).toBe(200); + const revoked = await fetchPrincipal(origin, { cookie: cookiePair as string }); + expect(revoked.response.status).toBe(200); + expect(revoked.body).toEqual({ kind: "anonymous" }); + + // An expired session (expiry forced into the past) is anonymous, not a 503. + const admin = await openAdministrativeClient(database.url); + try { + await safeQuery( + admin, + `UPDATE "ekklesia_core_session" SET expires_at = now() - interval '1 day'`, + ); + } finally { + await safeEnd(admin); + } + const secondSignIn = await signIn(origin, email, password); + const expiredCookie = secondSignIn.cookiePair as string; + const adminAgain = await openAdministrativeClient(database.url); + try { + await safeQuery( + adminAgain, + `UPDATE "ekklesia_core_session" SET expires_at = now() - interval '1 day'`, + ); + } finally { + await safeEnd(adminAgain); + } + const expired = await fetchPrincipal(origin, { cookie: expiredCookie }); + expect(expired.response.status).toBe(200); + expect(expired.body).toEqual({ kind: "anonymous" }); + } finally { + await running.shutdown().catch(() => undefined); + await dropDatabase(database.name); + } + }); + + it("performs no identity database work for an unrelated cookie, proven against a database whose session table is missing", async () => { + const database = await createDisposableDatabase("session_unrelated_cookie"); + await applyCoreMigrations(database.url); + const running = await startConfiguredApi(database.url); + try { + const origin = apiOrigin(running); + // Break the session store outright. Any request that actually touched it would now 503. + const admin = await openAdministrativeClient(database.url); + try { + await safeQuery( + admin, + 'ALTER TABLE "ekklesia_core_session" RENAME TO "ekklesia_core_session_moved"', + ); + } finally { + await safeEnd(admin); + } + + // Unrelated cookies must still resolve as anonymous with a 200: they never triggered + // session-store work in the first place. This is the original reproduction, inverted. + for (const cookie of ["x=y", "a=1; b=2; c=3", "better_auth_session_token=v"]) { + const { body, response } = await fetchPrincipal(origin, { cookie }); + expect(response.status).toBe(200); + expect(body).toEqual({ kind: "anonymous" }); + } + + // No cookie at all behaves identically. + const none = await fetchPrincipal(origin); + expect(none.response.status).toBe(200); + expect(none.body).toEqual({ kind: "anonymous" }); + } finally { + await running.shutdown().catch(() => undefined); + await dropDatabase(database.name); + } + }); +}); + describe.sequential("live API — Better Auth logging never leaks operational detail", () => { it("emits no raw exception, SQL, connection detail, origin header, or stack trace when a real identity database failure occurs", async () => { const database = await createDisposableDatabase("logging_leak"); diff --git a/core/src/identity/runtime.ts b/core/src/identity/runtime.ts index 43b64c4..5f49834 100644 --- a/core/src/identity/runtime.ts +++ b/core/src/identity/runtime.ts @@ -3,12 +3,13 @@ import { createAuthenticatedPrincipal, type EkklesiaPrincipal, } from "@ekklesia/contracts"; -import { betterAuth } from "better-auth"; +import { betterAuth, type BetterAuthOptions } from "better-auth"; import { drizzleAdapter } from "better-auth/adapters/drizzle"; +import { getCookies } from "better-auth/cookies"; import { sql } from "drizzle-orm"; import type { IdentityConfig } from "./config.js"; import type { IdentityDrizzleDatabase } from "./database.js"; -import { identityAdapterSchema } from "./schema.js"; +import { ekklesiaCoreSession, identityAdapterSchema } from "./schema.js"; export type IdentityRuntimeErrorCode = | "AUTH_REQUEST_FAILED" @@ -119,11 +120,76 @@ async function withTimeout(operation: Promise, timeoutMs: number): Promise } } -async function probeIdentityDatabase(db: IdentityDrizzleDatabase): Promise { +/** + * Whether the caller actually presented *the* configured Better Auth session cookie — not merely + * some cookie. An arbitrary anonymous cookie (`Cookie: x=y`) is not evidence that Better Auth ever + * attempted to resolve an Ekklesia session, so it must never trigger session-store work. + * + * Parsing deliberately mirrors RFC 6265's `cookie-string` grammar the same way Better Auth's own + * parser does: split on `;`, take each pair's name as the text before its first `=`, and trim + * surrounding whitespace. Matching is an exact, whole-name string comparison — never `includes`, a + * substring test, or an unanchored regular expression — so `better-auth.session_token_x`, + * `x_better-auth.session_token`, a differently-cased spelling, and a percent-encoded lookalike are + * all correctly *not* the session cookie (cookie names are case-sensitive per RFC 6265 §4.1.1, and + * cookie names are never percent-decoded). + * + * Ambiguity fails safe *towards* checking rather than away from it: a duplicated session-cookie + * name still reports `true`, because reporting `false` is what would let a genuinely broken + * session store be reported as an anonymous request. Returning `true` never grants access on its + * own — it only means the readiness check runs and Better Auth's own result still governs. + */ +export function hasConfiguredSessionCookie(headers: Headers, cookieName: string): boolean { + const cookieHeader = headers.get("cookie"); + if (cookieHeader === null) { + return false; + } + for (const pair of cookieHeader.split(";")) { + const separatorIndex = pair.indexOf("="); + const name = (separatorIndex === -1 ? pair : pair.slice(0, separatorIndex)).trim(); + if (name === cookieName) { + return true; + } + } + return false; +} + +/** + * Whether the actual Better Auth session store is usable — not merely whether PostgreSQL can + * execute a trivial statement. + * + * The previous `select 1` probe proved only that *a* connection could run *a* query, so a missing + * session relation, a drifted/absent column, or a revoked `SELECT` privilege all still looked + * "reachable", and Better Auth's ambiguous `null` was then reported as a genuinely anonymous + * request. This instead plans a real read of the exact session relation and the exact columns the + * adapter depends on, under a `where false` predicate: PostgreSQL still resolves the relation, + * every named column, and the caller's `SELECT` privilege at plan time, while guaranteeing zero + * rows are matched, read, or returned — so no session row, token, or user id can ever be + * enumerated, logged, or exposed by the check itself. + * + * Uses the injected Identity Drizzle view (the API's one shared pool); it never constructs another + * pool, client, connection string, or Drizzle instance. The bounded `withTimeout` guarantees the + * public response is never blocked by a hung query, and `clearTimeout` in its `finally` leaves no + * timer behind. + */ +async function probeSessionStore(db: IdentityDrizzleDatabase): Promise { try { - await withTimeout(db.execute(sql`select 1`), identityDatabaseProbeTimeoutMs); + await withTimeout( + db + .select({ + expiresAt: ekklesiaCoreSession.expiresAt, + id: ekklesiaCoreSession.id, + token: ekklesiaCoreSession.token, + userId: ekklesiaCoreSession.userId, + }) + .from(ekklesiaCoreSession) + .where(sql`false`), + identityDatabaseProbeTimeoutMs, + ); return true; } catch { + // Deliberately swallowed: the caught value may carry raw SQL, a relation/column name, a + // driver detail, a host/port, or a stack. The caller converts `false` into the fixed, + // sanitized `PRINCIPAL_RESOLUTION_FAILED` contract instead. return false; } } @@ -147,97 +213,109 @@ export function createIdentityRuntime( ): EkklesiaIdentityRuntime { const rateLimitOverride = options.rateLimitOverride; - const auth = constructBetterAuth(() => - betterAuth({ - advanced: { - // Better Auth's own default is environment-sensitive: `skipOriginCheck` falls back to - // `true` whenever `isTest()` is true (NODE_ENV=test, which Vitest sets, or TEST=1) unless - // `disableOriginCheck` is explicitly set — and origin skip in turn also silently skips - // CSRF validation as long as `disableCSRFCheck` is left unset (Better Auth's documented - // backward-compatibility coupling). Left implicit, this means origin/CSRF protection is - // silently *off* under the test runner and *on* in production: the exact same code path - // behaves differently in the two places it matters most to verify. Setting both flags to - // `false` explicitly removes the `isTest()` branch entirely, so the identical protection - // is enforced in every environment, including under Vitest. - disableCSRFCheck: false, - disableOriginCheck: false, - // The pinned release's own client-address resolution (`getIp`, `@better-auth/core/utils/ - // ip`) only ever reads request headers — it has no way to see the actual TCP socket peer. - // Restricting `ipAddressHeaders` to exactly one platform-owned header name, rather than - // leaving it on Better Auth's own default (`x-forwarded-for`, trusted directly with no - // proxy policy), is what stops a caller from choosing its own rate-limit bucket: the named - // header is populated only by `apps/api`'s transport boundary, after it has already - // resolved and verified the caller's address against the actual socket peer (and, in - // explicit proxy mode, a configured trusted-proxy chain) — see - // `apps/api/src/client-address.ts`. `trustedProxies` is deliberately left unset here: that - // option only inspects header *content* for proxy-shaped addresses and can never verify who - // actually sent the request, so the equivalent trust decision is made once, correctly, at - // the transport boundary instead of asked of Better Auth a second time. - ipAddress: { - ipAddressHeaders: [TRUSTED_CLIENT_ADDRESS_HEADER], - }, + // One options object feeds both `betterAuth()` and `getCookies()` below, so the session-cookie + // name this runtime detects is derived from the exact same configuration Better Auth itself + // uses. That makes drift structurally impossible rather than merely tested: there is no second + // hard-coded copy of the cookie name (or of the `__Secure-` prefix rule the configured HTTPS + // `baseURL` implies) anywhere in this repository. + const betterAuthOptions = { + advanced: { + // Better Auth's own default is environment-sensitive: `skipOriginCheck` falls back to + // `true` whenever `isTest()` is true (NODE_ENV=test, which Vitest sets, or TEST=1) unless + // `disableOriginCheck` is explicitly set — and origin skip in turn also silently skips + // CSRF validation as long as `disableCSRFCheck` is left unset (Better Auth's documented + // backward-compatibility coupling). Left implicit, this means origin/CSRF protection is + // silently *off* under the test runner and *on* in production: the exact same code path + // behaves differently in the two places it matters most to verify. Setting both flags to + // `false` explicitly removes the `isTest()` branch entirely, so the identical protection + // is enforced in every environment, including under Vitest. + disableCSRFCheck: false, + disableOriginCheck: false, + // The pinned release's own client-address resolution (`getIp`, `@better-auth/core/utils/ + // ip`) only ever reads request headers — it has no way to see the actual TCP socket peer. + // Restricting `ipAddressHeaders` to exactly one platform-owned header name, rather than + // leaving it on Better Auth's own default (`x-forwarded-for`, trusted directly with no + // proxy policy), is what stops a caller from choosing its own rate-limit bucket: the named + // header is populated only by `apps/api`'s transport boundary, after it has already + // resolved and verified the caller's address against the actual socket peer (and, in + // explicit proxy mode, a configured trusted-proxy chain) — see + // `apps/api/src/client-address.ts`. `trustedProxies` is deliberately left unset here: that + // option only inspects header *content* for proxy-shaped addresses and can never verify who + // actually sent the request, so the equivalent trust decision is made once, correctly, at + // the transport boundary instead of asked of Better Auth a second time. + ipAddress: { + ipAddressHeaders: [TRUSTED_CLIENT_ADDRESS_HEADER], }, - baseURL: options.config.baseUrl, - database: drizzleAdapter(options.db, { - provider: "pg", - schema: identityAdapterSchema, - }), - emailAndPassword: { - autoSignIn: false, - // Public registration is disabled unless explicitly enabled (`IdentityConfig. - // publicSignupEnabled`, `core/src/identity/config.ts`) — never a side effect of an absent, - // misspelled, or loosely parsed configuration value. Better Auth's own `disableSignUp` gate - // rejects every `/sign-up/email` call with a fixed `400 EMAIL_PASSWORD_SIGN_UP_DISABLED` - // before ever looking up the email, so a disabled deployment never reveals whether an - // address is already registered; sign-in and existing sessions are entirely unaffected. - disableSignUp: !options.config.publicSignupEnabled, - enabled: true, - // Pinned explicitly rather than left on Better Auth's fallback default (`|| 8` / `|| 128` - // in the pinned release): an implicit default can change silently on a future Better Auth - // upgrade, and Ekklesia's own tests assert this exact 8–128 boundary. - maxPasswordLength: 128, - minPasswordLength: 8, - }, - logger: { - disabled: false, - level: "error", - log: (level) => sanitizedBetterAuthLogger(level), - }, - // A second, independent layer alongside `logger` above: the pinned release's own transport - // dependency (`better-call`'s router) has an unconditional `console.error("# SERVER_ERROR: - // ", error)` fallback for any thrown value it does not recognize as one of Better Auth's own - // `APIError`s — this fires *before* Better Auth's `logger` option is ever consulted, so no - // logger configuration can reach it. `onAPIError.throw: true` closes that gap: Better Auth's - // own `APIError`s (wrong password, invalid origin, disabled sign-up, ...) still resolve to - // their normal HTTP response completely unaffected, but a genuinely unexpected failure (for - // example a database outage) now propagates out of `auth.handler()` instead of being caught - // and printed raw internally, so it reaches `handleAuthRequest`'s own try/catch below and - // becomes a sanitized `IdentityRuntimeError` like any other unexpected failure. - onAPIError: { - throw: true, - }, - rateLimit: { - enabled: true, - max: 100, - window: 10, - ...(rateLimitOverride - ? { - customRules: { - "/sign-in/email": { - max: rateLimitOverride.max ?? 100, - window: rateLimitOverride.window ?? 10, - }, - "/sign-up/email": { - max: rateLimitOverride.max ?? 100, - window: rateLimitOverride.window ?? 10, - }, - }, - } - : {}), - }, - secret: options.config.secret, - trustedOrigins: [...options.config.trustedOrigins], + }, + baseURL: options.config.baseUrl, + database: drizzleAdapter(options.db, { + provider: "pg", + schema: identityAdapterSchema, }), + emailAndPassword: { + autoSignIn: false, + // Public registration is disabled unless explicitly enabled (`IdentityConfig. + // publicSignupEnabled`, `core/src/identity/config.ts`) — never a side effect of an absent, + // misspelled, or loosely parsed configuration value. Better Auth's own `disableSignUp` gate + // rejects every `/sign-up/email` call with a fixed `400 EMAIL_PASSWORD_SIGN_UP_DISABLED` + // before ever looking up the email, so a disabled deployment never reveals whether an + // address is already registered; sign-in and existing sessions are entirely unaffected. + disableSignUp: !options.config.publicSignupEnabled, + enabled: true, + // Pinned explicitly rather than left on Better Auth's fallback default (`|| 8` / `|| 128` + // in the pinned release): an implicit default can change silently on a future Better Auth + // upgrade, and Ekklesia's own tests assert this exact 8–128 boundary. + maxPasswordLength: 128, + minPasswordLength: 8, + }, + logger: { + disabled: false, + level: "error", + log: (level) => sanitizedBetterAuthLogger(level), + }, + // A second, independent layer alongside `logger` above: the pinned release's own transport + // dependency (`better-call`'s router) has an unconditional `console.error("# SERVER_ERROR: + // ", error)` fallback for any thrown value it does not recognize as one of Better Auth's own + // `APIError`s — this fires *before* Better Auth's `logger` option is ever consulted, so no + // logger configuration can reach it. `onAPIError.throw: true` closes that gap: Better Auth's + // own `APIError`s (wrong password, invalid origin, disabled sign-up, ...) still resolve to + // their normal HTTP response completely unaffected, but a genuinely unexpected failure (for + // example a database outage) now propagates out of `auth.handler()` instead of being caught + // and printed raw internally, so it reaches `handleAuthRequest`'s own try/catch below and + // becomes a sanitized `IdentityRuntimeError` like any other unexpected failure. + onAPIError: { + throw: true, + }, + rateLimit: { + enabled: true, + max: 100, + window: 10, + ...(rateLimitOverride + ? { + customRules: { + "/sign-in/email": { + max: rateLimitOverride.max ?? 100, + window: rateLimitOverride.window ?? 10, + }, + "/sign-up/email": { + max: rateLimitOverride.max ?? 100, + window: rateLimitOverride.window ?? 10, + }, + }, + } + : {}), + }, + secret: options.config.secret, + trustedOrigins: [...options.config.trustedOrigins], + } satisfies BetterAuthOptions; + + const auth = constructBetterAuth(() => betterAuth(betterAuthOptions)); + // `getCookies` is Better Auth's own supported, exported helper (`better-auth/cookies`) for + // resolving the configured cookie names from an options object — not a dependency internal. It + // is given the identical object above, so the returned `sessionToken.name` is exactly the name + // the constructed runtime will read, including the `__Secure-` prefix an https `baseURL` implies. + const sessionCookieName = constructBetterAuth( + () => getCookies(betterAuthOptions).sessionToken.name, ); return Object.freeze({ @@ -263,14 +341,16 @@ export function createIdentityRuntime( return createAuthenticatedPrincipal(userId); } - // Better Auth's getSession swallows infrastructure failures internally and returns null - // for them exactly like a genuinely missing, expired, or malformed session — it never - // throws. A request with no Cookie header at all is unambiguous: Better Auth never queries - // the database for that case, so it is always genuinely anonymous. Only when a cookie was - // actually presented is the null result ambiguous enough to require disambiguation. - if (headers.has("cookie")) { - const reachable = await probeIdentityDatabase(options.db); - if (!reachable) { + // Better Auth's getSession swallows infrastructure failures internally and returns null for + // them exactly like a genuinely missing, expired, revoked, or malformed session — it never + // throws. Only a request that actually presented *the configured session cookie* is + // ambiguous enough to be worth disambiguating: with no cookie at all, or with only unrelated + // cookies, Better Auth never resolved an Ekklesia session in the first place, so `null` is + // unambiguous and no session-store work is performed. This is what keeps an arbitrary + // anonymous cookie from causing identity database work on every request. + if (hasConfiguredSessionCookie(headers, sessionCookieName)) { + const sessionStoreUsable = await probeSessionStore(options.db); + if (!sessionStoreUsable) { throw new IdentityRuntimeError("PRINCIPAL_RESOLUTION_FAILED"); } } diff --git a/core/tests/identity-runtime.test.ts b/core/tests/identity-runtime.test.ts index e464b5e..30828af 100644 --- a/core/tests/identity-runtime.test.ts +++ b/core/tests/identity-runtime.test.ts @@ -9,6 +9,7 @@ vi.mock("better-auth", () => ({ betterAuth: betterAuthMock })); vi.mock("better-auth/adapters/drizzle", () => ({ drizzleAdapter: drizzleAdapterMock })); import type { IdentityConfig } from "../src/identity/config.js"; +import type { IdentityDrizzleDatabase } from "../src/identity/database.js"; import { createIdentityRuntime, IdentityRuntimeError, @@ -26,8 +27,43 @@ function validConfig(overrides: Partial = {}): IdentityConfig { }); } -function fakeDb(overrides: Partial<{ execute: ReturnType }> = {}) { - return { __fakeDb: true, execute: vi.fn().mockResolvedValue(undefined), ...overrides } as never; +interface FakeDbOptions { + /** Makes the session-store readiness read reject, standing in for a missing relation, a dropped + * column, a denied `SELECT` privilege, an unavailable connection, or a query timeout — every one + * of which surfaces to this seam identically, as a rejected query. */ + readonly sessionProbeError?: Error; +} + +/** + * A fake Identity Drizzle view that records every query actually issued, so a test can prove the + * *absence* of identity database work — not merely that a particular helper was not called. Both + * the legacy `execute` seam and the `select(...).from(...).where(...)` chain the session-store + * readiness check uses are recorded into one ordered list. + */ +function fakeDb(options: FakeDbOptions = {}) { + const queries: string[] = []; + const execute = vi.fn(() => { + queries.push("execute"); + return Promise.resolve(undefined); + }); + // Drizzle's query builder is awaited directly, so the terminal `where(...)` returns a real + // Promise rather than a hand-rolled thenable object. + const select = vi.fn((columns: Record) => { + queries.push(`select:${Object.keys(columns).sort().join(",")}`); + const builder = { + from: vi.fn(() => builder), + where: vi.fn(() => + options.sessionProbeError + ? Promise.reject(options.sessionProbeError) + : Promise.resolve([] as unknown[]), + ), + }; + return builder; + }); + const db = { __fakeDb: true, execute, queries, select }; + // The runtime only ever uses the narrow surface faked above; the cast keeps the fake honest + // about that instead of stubbing all of `NodePgDatabase`, while `queries` stays observable. + return db as unknown as IdentityDrizzleDatabase & { readonly queries: string[] }; } describe("Core identity runtime construction boundary", () => { @@ -327,65 +363,152 @@ describe("Core identity runtime — resolvePrincipal", () => { drizzleAdapterMock.mockClear(); }); - it("maps a valid session to an authenticated principal containing only the opaque user id", async () => { + /** The configured production (https) session-cookie name, derived by Better Auth's own + * `getCookies` helper from the exact same `baseUrl` this suite's `validConfig` uses. */ + const productionSessionCookie = "__Secure-better-auth.session_token"; + + it("maps a valid session to an authenticated principal containing only the opaque user id, without any follow-up probe", async () => { const getSession = vi.fn().mockResolvedValue({ session: { id: "sess_1", token: "tok" }, user: { email: "person@example.test", id: "user_123", name: "Person" }, }); betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); + const db = fakeDb(); - const runtime = createIdentityRuntime({ config: validConfig(), db: fakeDb() }); + const runtime = createIdentityRuntime({ config: validConfig(), db }); const principal = await runtime.resolvePrincipal( - new Headers({ cookie: "better-auth.session_token=tok" }), + new Headers({ cookie: `${productionSessionCookie}=tok` }), ); expect(principal).toEqual({ kind: "authenticated", userId: "user_123" }); expect(Object.keys(principal)).toEqual(["kind", "userId"]); expect(Object.isFrozen(principal)).toBe(true); + // A resolved session needs no readiness confirmation — the store demonstrably worked. + expect(db.queries).toEqual([]); }); - it("maps a missing session with no cookie header to anonymous without probing the database", async () => { + it("performs zero identity database work for no cookie, unrelated cookies, or session-cookie lookalikes", async () => { const getSession = vi.fn().mockResolvedValue(null); - const execute = vi.fn().mockResolvedValue(undefined); betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); - const runtime = createIdentityRuntime({ config: validConfig(), db: fakeDb({ execute }) }); - const principal = await runtime.resolvePrincipal(new Headers()); + for (const cookie of [ + undefined, + "x=y", + "a=1; b=2; c=3", + // Lookalikes: a prefix, a suffix, a differently-cased spelling, a percent-encoded name, and + // the unprefixed development name (wrong for this https configuration). + `${productionSessionCookie}_extra=v`, + `extra_${productionSessionCookie}=v`, + `${productionSessionCookie.toUpperCase()}=v`, + "__Secure-better-auth%2Esession_token=v", + "better-auth.session_token=v", + ]) { + const db = fakeDb(); + const runtime = createIdentityRuntime({ config: validConfig(), db }); + const principal = await runtime.resolvePrincipal( + cookie === undefined ? new Headers() : new Headers({ cookie }), + ); - expect(principal).toEqual({ kind: "anonymous" }); - expect(execute).not.toHaveBeenCalled(); + expect(principal).toEqual({ kind: "anonymous" }); + expect(db.queries).toEqual([]); + } }); - it("maps an expired/revoked/malformed session (cookie present, null session) to anonymous when the database is reachable", async () => { + it("reads the real session relation and its expected columns only when the configured session cookie was presented", async () => { const getSession = vi.fn().mockResolvedValue(null); - const execute = vi.fn().mockResolvedValue(undefined); betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); + const db = fakeDb(); - const runtime = createIdentityRuntime({ config: validConfig(), db: fakeDb({ execute }) }); + const runtime = createIdentityRuntime({ config: validConfig(), db }); const principal = await runtime.resolvePrincipal( - new Headers({ cookie: "better-auth.session_token=stale-or-garbage" }), + new Headers({ cookie: `${productionSessionCookie}=stale-or-garbage` }), ); + // A healthy session store plus an unresolvable token is a genuinely anonymous caller. expect(principal).toEqual({ kind: "anonymous" }); - expect(execute).toHaveBeenCalledTimes(1); + // The readiness check is a real read of the session relation's expected columns — never + // `select 1`, and never a generic pool ping. + expect(db.queries).toEqual(["select:expiresAt,id,token,userId"]); }); - it("never downgrades an infrastructure failure to anonymous: throws when the database is unreachable", async () => { + it("detects the session cookie among several cookies and with irregular whitespace", async () => { const getSession = vi.fn().mockResolvedValue(null); - const execute = vi.fn().mockRejectedValue(new Error("connection terminated unexpectedly")); betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); - const runtime = createIdentityRuntime({ config: validConfig(), db: fakeDb({ execute }) }); - - let captured: unknown; - try { - await runtime.resolvePrincipal(new Headers({ cookie: "better-auth.session_token=x" })); - } catch (error) { - captured = error; + for (const cookie of [ + `a=1; ${productionSessionCookie}=tok; b=2`, + ` ${productionSessionCookie}=tok `, + `a=1;${productionSessionCookie}=tok`, + // A duplicated session cookie is ambiguous and must fail safe *towards* checking the store. + `${productionSessionCookie}=one; ${productionSessionCookie}=two`, + // A valueless session cookie still means Better Auth saw the name. + `${productionSessionCookie}`, + ]) { + const db = fakeDb(); + const runtime = createIdentityRuntime({ config: validConfig(), db }); + await runtime.resolvePrincipal(new Headers({ cookie })); + expect(db.queries).toEqual(["select:expiresAt,id,token,userId"]); } + }); - expect(captured).toBeInstanceOf(IdentityRuntimeError); - expect(captured).toMatchObject({ code: "PRINCIPAL_RESOLUTION_FAILED" }); + it("uses the unprefixed development cookie name when the configured base URL is http", async () => { + const getSession = vi.fn().mockResolvedValue(null); + betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); + const developmentConfig = validConfig({ + baseUrl: "http://localhost:3000", + trustedOrigins: Object.freeze(["http://localhost:3000"]), + }); + + const unprefixed = fakeDb(); + await createIdentityRuntime({ config: developmentConfig, db: unprefixed }).resolvePrincipal( + new Headers({ cookie: "better-auth.session_token=tok" }), + ); + expect(unprefixed.queries).toEqual(["select:expiresAt,id,token,userId"]); + + // The production `__Secure-` name is *not* what an http deployment reads. + const prefixed = fakeDb(); + await createIdentityRuntime({ config: developmentConfig, db: prefixed }).resolvePrincipal( + new Headers({ cookie: `${productionSessionCookie}=tok` }), + ); + expect(prefixed.queries).toEqual([]); + }); + + it("never downgrades a broken session store to anonymous, and leaks no database detail", async () => { + const getSession = vi.fn().mockResolvedValue(null); + betterAuthMock.mockReturnValue({ api: { getSession }, handler: vi.fn() }); + + // Every distinguishable session-store failure reaches this seam as a rejected read: a missing + // relation, a dropped column, a denied privilege, a dead connection, and a timeout alike. + const hostileFailures = [ + new Error('relation "ekklesia_core_session" does not exist'), + new Error('column "token" does not exist'), + new Error("permission denied for table ekklesia_core_session at 10.1.2.3:5432"), + new Error("connection terminated unexpectedly"), + new Error("timed out"), + ]; + for (const sessionProbeError of hostileFailures) { + const runtime = createIdentityRuntime({ + config: validConfig(), + db: fakeDb({ sessionProbeError }), + }); + + let captured: unknown; + try { + await runtime.resolvePrincipal(new Headers({ cookie: `${productionSessionCookie}=x` })); + } catch (error) { + captured = error; + } + + expect(captured).toBeInstanceOf(IdentityRuntimeError); + expect(captured).toMatchObject({ code: "PRINCIPAL_RESOLUTION_FAILED" }); + const publicText = `${String(captured)}\n${ + captured instanceof Error ? (captured.stack ?? "") : "" + }\n${JSON.stringify(captured)}`; + expect(publicText).not.toContain("ekklesia_core_session"); + expect(publicText).not.toContain("permission denied"); + expect(publicText).not.toContain("10.1.2.3"); + expect(publicText).not.toContain("does not exist"); + } }); it("treats an unexpected throw from getSession itself as a sanitized infrastructure failure", async () => { diff --git a/docs/development/api.md b/docs/development/api.md index 574ce9a..8bfbc26 100644 --- a/docs/development/api.md +++ b/docs/development/api.md @@ -87,7 +87,7 @@ Authenticated: { "kind": "authenticated", "userId": "2aKp4jPeV1QbXwyz1OH6IcAUfl2n1fc1" } ``` -Infrastructure failure (the identity database could not be reached while resolving the principal — +Infrastructure failure (the identity session store was not usable while resolving the principal — never silently reported as anonymous): ```json @@ -95,6 +95,13 @@ never silently reported as anonymous): ``` returned with HTTP `503`. +A request carrying no cookies, or only unrelated cookies, never reaches that path and never performs +identity database work at all: only a request presenting the *configured session cookie* that Better +Auth resolved to `null` is ambiguous enough to check. When it is checked, a missing session table, a +dropped column, a revoked `SELECT` privilege, an unreachable database, and a query timeout all +produce this same sanitized `503` — carrying no relation name, column name, SQL, driver message, +host, or stack. See [Identity § How a principal is resolved](identity.md#how-a-principal-is-resolved). + This is deliberately not the same thing as Better Auth's own `GET /api/auth/get-session` response, which returns Better Auth's full `{session, user}` shape (session id, expiry, IP address, user agent, email, name, image, email-verification status, and more). `/api/v1/principal` is the diff --git a/docs/development/identity.md b/docs/development/identity.md index f1ba1c5..c14f965 100644 --- a/docs/development/identity.md +++ b/docs/development/identity.md @@ -144,7 +144,8 @@ ADR 0010. `handleAuthRequest(request): Promise` (delegates directly to Better Auth's handler; its normal responses, including its own error responses, pass through unmodified) and `resolvePrincipal(headers): Promise` (never silently downgrades a database -outage to an anonymous result — see ADR 0011 for exactly how). Neither method, nor anything else +outage to an anonymous result — see ADR 0011 and [session resolution](#how-a-principal-is-resolved) +below). Neither method, nor anything else exported from `@ekklesia/core/identity`, exposes `auth.api`, Better Auth's session/user types, the Drizzle adapter, the raw database, a session token, or Better Auth configuration. `apps/api` is the only current consumer of this boundary; see [API](api.md). @@ -166,6 +167,35 @@ hung or slow query, which `connectionTimeoutMs` alone cannot do since it only bo the connection. `apps/api` sets both timeouts explicitly (see [API](api.md)); an invalid value is rejected the same sanitized way an invalid connection string already is. +## How a principal is resolved + +`resolvePrincipal(headers)` returns the anonymous principal for a missing, expired, revoked, or +malformed session, and throws a sanitized `PRINCIPAL_RESOLUTION_FAILED` for an infrastructure +failure. Telling those apart takes explicit work, because Better Auth's `getSession` swallows +internal failures and returns `null` for them exactly as it does for a genuinely absent session. + +1. **No cookies, or only unrelated cookies → no identity database work at all.** `Cookie: x=y` is + not evidence that Better Auth ever attempted to resolve an Ekklesia session, so its `null` is + unambiguous and the anonymous principal is returned immediately. An unauthenticated caller + cannot drive identity database load with a cookie header they chose themselves. +2. **The configured session cookie is present, and Better Auth returned `null` → the ambiguous + case, and only then is the session store checked.** Detection is an exact, whole-name cookie + comparison (RFC 6265 parsing; names are case-sensitive and never percent-decoded), so + `better-auth.session_token_x` and `x_better-auth.session_token` are correctly *not* the session + cookie. The name itself is derived from the same options object passed to Better Auth, via its + own exported `getCookies` helper — including the `__Secure-` prefix an `https` base URL implies + — so there is no second copy of it to drift. +3. **The check reads the real session relation**, not `select 1`: it plans a read of the exact + columns the Better Auth adapter depends on under a `where false` predicate. PostgreSQL resolves + the relation, every named column, and the caller's `SELECT` privilege while matching zero rows, + so no session row, token, or user id can be read, logged, or enumerated by the check. It runs on + the injected pool — never a second connection — under a bounded 3-second timeout. + +A missing session table, a dropped or drifted column, a revoked `SELECT` privilege, an unreachable +database, and a query timeout therefore all produce a sanitized +`503 {"code":"PRINCIPAL_RESOLUTION_FAILED"}` — never a silent anonymous result. The response body, +logs, and error carry no relation name, column name, SQL, driver message, host, or stack. + ## What is not operational yet No email verification, password recovery, invitation flow, email delivery, social or OAuth diff --git a/docs/development/testing.md b/docs/development/testing.md index 0b56345..15f18ac 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -19,8 +19,10 @@ connection and operation (`query_timeout`/`statement_timeout`) database timeouts runtime (explicit construction, exact schema mapping, only email/password and rate limiting enabled, an explicit `advanced.disableCSRFCheck`/`disableOriginCheck: false` pair, an explicit error-only sanitized logger that strips Better Auth's own message and arguments regardless of what -they contain, pinned 8–128 password-length bounds, sanitized failures, and that a database outage -during principal resolution is never silently reported as anonymous), and Core migration discovery +they contain, pinned 8–128 password-length bounds, sanitized failures, exact whole-name +session-cookie detection that never fires on a prefixed, suffixed, or differently-cased lookalike, +and that a broken session store during principal resolution is never silently reported as anonymous +while an unrelated cookie performs no identity database work at all), and Core migration discovery (deterministic ordering, the reserved `core` identity, and collision rejection) — all without a live database. `apps/api`'s own unit tests cover its pure Hono app factory (route shapes, module route mounting/enforcement/CORS/response sanitization, CORS allow/deny/preflight, deterministic 404 @@ -83,6 +85,15 @@ outage during principal resolution producing a sanitized `503` rather than a fal starting the API against an unmigrated database creating no tables at all, and a clean listener/pool shutdown. +A dedicated group within that suite proves session-store readiness against the real database rather +than a mock, since that is the only place a broken store looks exactly like an anonymous request: a +request carrying no cookies and one carrying only an unrelated cookie both resolve as anonymous +while issuing **no** identity database work at all; a request presenting the configured session +cookie against a dropped session table, a dropped column, and a revoked `SELECT` privilege each +produce a sanitized `503 PRINCIPAL_RESOLUTION_FAILED` with no relation name, column name, SQL, or +driver text in the body or the logs; and a genuinely expired or unknown session against a healthy +store still resolves as anonymous. + The same suite also proves Better Auth's own origin/CSRF protection is production-equivalent under the test runner, not just in a plain Node script: a no-cookie first authentication attempt succeeds from a trusted browser origin and is rejected (`403`, no permissive CORS header, no leaked detail)