133 lines
8.3 KiB
Markdown
133 lines
8.3 KiB
Markdown
# 0012: Production-equivalent identity origin/CSRF protection under test
|
|
|
|
- Status: Accepted
|
|
- Date: 2026-07-21
|
|
- Amends: [0011](0011-identity-http-transport-and-public-enrollment.md)
|
|
|
|
## Context
|
|
|
|
ADR 0011 stated that Better Auth's own server-side origin/CSRF rejection was real, verified
|
|
production behavior, but could not be reproduced reliably under Vitest, and left the discrepancy
|
|
undiagnosed rather than commit a flaky or misleading automated test. A follow-up corrective slice
|
|
(`phase-4/identity-http-hardening`) revisited that gap, alongside four related hardening items
|
|
found during the same review: Better Auth's default logger could emit raw exceptions and
|
|
user-controlled input; the identity database pool had no bound on a hung query; `apps/api`'s
|
|
shutdown had a narrow idempotency gap under concurrent callers; and the email/password length
|
|
boundary was left on an implicit Better Auth default rather than an explicit one. This ADR covers
|
|
only the origin/CSRF root cause and fix, since it is the one item that changes a previously
|
|
documented architectural guarantee; the other four are implementation hardening, documented in
|
|
[Identity](../../development/identity.md) and [API](../../development/api.md) rather than a new
|
|
ADR.
|
|
|
|
## Decision
|
|
|
|
### Root cause: Better Auth silently disables origin/CSRF checking under a test runner by default
|
|
|
|
Reading the pinned `[email protected]` release's own `create-context.mjs` directly:
|
|
|
|
```js
|
|
skipOriginCheck: options.advanced?.disableOriginCheck !== void 0
|
|
? options.advanced.disableOriginCheck
|
|
: isTest() ? true : false,
|
|
```
|
|
|
|
`isTest()` (from `@better-auth/core/env`) is `nodeENV === "test" || toBoolean(env.TEST)`. Vitest
|
|
sets `NODE_ENV=test` by default. So whenever `advanced.disableOriginCheck` is left unset — exactly
|
|
Ekklesia's configuration before this ADR — Better Auth silently sets `skipOriginCheck: true` under
|
|
Vitest and `false` in production: the identical code path behaves differently in the one place most
|
|
important to verify it and the one place it actually matters. Better Auth's own
|
|
`shouldSkipCSRFForBackwardCompat` then also skips CSRF validation whenever `skipOriginCheck` is
|
|
`true` and `advanced.disableCSRFCheck` is left unset, coupling the two. This is Better Auth's own
|
|
documented default, read from `isTest()` at Better Auth construction time — not a Vitest bug, a
|
|
race condition, or anything specific to this repository's test harness. It explains ADR 0011's
|
|
mystery well enough to act on: whatever precisely made that ADR's own `NODE_ENV`-forcing attempt
|
|
unreliable (`isTest()` also honors a separate `TEST` environment variable, which a `NODE_ENV`
|
|
override alone would not clear, and which Vitest or its surrounding tooling may set independently),
|
|
an explicit `advanced.disableOriginCheck`/`disableCSRFCheck` value removes the `isTest()` branch
|
|
entirely and cannot be affected by any test-runner environment variable.
|
|
|
|
### Fix: pin both flags explicitly
|
|
|
|
`core/src/identity/runtime.ts` now passes:
|
|
|
|
```ts
|
|
advanced: {
|
|
disableCSRFCheck: false,
|
|
disableOriginCheck: false,
|
|
},
|
|
```
|
|
|
|
This removes the `isTest()` branch entirely — the flags are always explicit, so identical
|
|
protection is enforced in every environment, including under Vitest. No other origin/CSRF-related
|
|
option changes: `trustedOrigins` is still Phase 4A's single `IdentityConfig.trustedOrigins` list,
|
|
cookies are still Better Auth's host-only defaults, and CORS is still the exact-match, no-wildcard
|
|
policy ADR 0011 already established.
|
|
|
|
### Automated coverage replaces the manual-only caveat
|
|
|
|
`apps/api/tests/api-http.integration.ts` now asserts, against the real compiled server and a real
|
|
PostgreSQL-backed session, all four scenarios ADR 0011 could previously only claim from manual
|
|
verification:
|
|
|
|
- A no-cookie first authentication attempt from a trusted browser origin succeeds.
|
|
- A no-cookie first authentication attempt from an untrusted browser origin is rejected (`403`),
|
|
with no `Access-Control-Allow-Origin` header and no leaked internal detail in the body.
|
|
- A cookie-bearing state-changing request (sign-out) from an untrusted browser origin is rejected
|
|
the same way, and — critically — the presented session remains valid afterward, proving the
|
|
rejected request never reached sign-out logic.
|
|
- A denied origin never receives a permissive CORS header, in either of the above cases.
|
|
|
|
The manual-reproduction paragraphs in ADR 0011 are left intact as the historical record of what was
|
|
verified and not yet understood at the time; a short correction note there points here.
|
|
|
|
### A narrower consequence: Fetch-API clients now need a trusted origin on sign-in/sign-up even without a cookie
|
|
|
|
Enabling the checks surfaced a real, previously-untested distinction. Better Auth's first-login
|
|
CSRF protection (`formCsrfMiddleware`, applied only to `/sign-in/email` and `/sign-up/email`) does
|
|
not key off cookies alone: it also treats the presence of any `Sec-Fetch-*` header as evidence the
|
|
caller is a Fetch-API client (browser or Node's own `fetch()`, which sends `Sec-Fetch-Mode: cors`
|
|
automatically) and requires a trusted `Origin`/`Referer` in that case regardless of cookie
|
|
presence. A previously-committed test claiming a *server-to-server* sign-up with no `Origin` header
|
|
succeeds was actually exercising Node's `fetch()`, and once the check was genuinely enabled it
|
|
correctly failed. The test now uses a raw `node:http` request (which sends no `Sec-Fetch-*`
|
|
headers, matching curl and most backend HTTP client libraries) to prove the real claim: a *non-Fetch*
|
|
server-to-server caller needs no `Origin` header; a Fetch-API caller (browser or Node's `fetch()`)
|
|
does, on these two specific routes, cookie or not. See [API](../../development/api.md#origin-cors-and-cookies).
|
|
|
|
### A related discovery: Better Auth's own transport dependency logs raw errors outside any logger option
|
|
|
|
Verifying the logging hardening item from the same corrective slice against a real induced identity
|
|
database failure surfaced a second, independent gap: `better-call` (the low-level HTTP dispatch
|
|
library Better Auth is built on) has an unconditional `console.error("# SERVER_ERROR: ", error)`
|
|
fallback for any error it does not recognize as a Better Auth `APIError`, which fires before Better
|
|
Auth's own `logger` option is ever consulted. `core/src/identity/runtime.ts` now also sets
|
|
`onAPIError: { throw: true }`, which lets Better Auth's own deliberate `APIError`s (wrong password,
|
|
invalid origin, ...) resolve normally while routing genuinely unexpected failures out to Core's
|
|
already-sanitized `handleAuthRequest` boundary instead. This is a logging fix, not an origin/CSRF
|
|
one; the full account is in [API](../../development/api.md#logging).
|
|
|
|
## Alternatives
|
|
|
|
- Setting `NODE_ENV=production` inside the test process (as ADR 0011's own investigation already
|
|
tried) was not a viable fix even before the root cause was known: it is fragile (couples
|
|
unrelated behavior to a global environment variable inside the test runner) and, as this ADR's
|
|
investigation showed, does not actually address the mechanism — Better Auth reads the same
|
|
`isTest()` signal regardless, and pinning `NODE_ENV` inside the same process it already runs in
|
|
is not a meaningfully different configuration from leaving `advanced.disableOriginCheck` unset.
|
|
- Leaving the gap as a documented manual-only guarantee (ADR 0011's original position) was rejected
|
|
once the root cause was known and fixable: an explicit, narrow configuration change closes it
|
|
completely, so there is no remaining reason to accept unverified production behavior.
|
|
- Disabling Better Auth's built-in origin/CSRF checks and reimplementing origin validation directly
|
|
in `apps/api`'s CORS layer was rejected: it would duplicate a check Better Auth already performs
|
|
correctly once configured explicitly, and would weaken the cookie-bearing-request guarantee CORS
|
|
alone cannot provide (CORS is a browser-enforced, response-header mechanism; it does not stop a
|
|
request from being processed server-side).
|
|
|
|
## Consequences
|
|
|
|
Better Auth's origin/CSRF protection is now genuinely production-equivalent under the automated
|
|
test suite, closing the gap ADR 0011 left open. Future changes to authentication endpoints can rely
|
|
on `pnpm test:migrations:integration` to actually exercise this guarantee rather than requiring a
|
|
separate manual verification pass. No route, response shape, cookie policy, or CORS behavior
|
|
changed for any caller already sending a trusted `Origin` header.
|