Skip to content

feat: auth — register, login, refresh rotation, logout, demo mode - #2

Merged
kilianmc merged 1 commit into
devfrom
feat/auth
Aug 13, 2026
Merged

kilianmc merged 1 commit into
devfrom
feat/auth

Conversation

@kilianmc

Copy link
Copy Markdown
Owner

Deny-by-default authentication for the whole API, a read-only demo mode, refresh-token families with reuse detection, and a Postgres-backed rate limiter. Version → 1.2.0.

What landed

Area
server/auth/ passwords (argon2id), tokens (HS256), refresh (families + rotation), cookies, ratelimit, deps (the gate), routes
Schema 0002_auth — app_user, auth_session, rate_limit
Endpoints register, login, refresh, logout, demo, me
CI .github/workflows/migrate.yml — manual, approval-gated migration runner

Three decisions worth reviewing

Deny-by-default is a single global dependency, not per-router opt-in. Opt-in fails open: the failure mode of forgetting it is an unprotected endpoint that passes every test anyone thought to write. A route is public only by appearing in PUBLIC_ROUTES (eight entries), and two tests walk the live route table — every unlisted route must 401 anonymously, and every mutating route must 403 for a demo token. A stale entry in the list is also caught, since a typo there leaves the real route protected while appearing to open it.

Access tokens live 3 hours, deliberately. Refresh rotation is a database write, so a 15-minute token would wake Neon every 15 minutes for an entire training session and become the single largest consumer of the free tier's compute allowance. Verification is stateless and never queries; revocation lives in the refresh family instead, and the client refreshes lazily on a 401 rather than on a timer.

POST /api/auth/demo issues zero SQL. The handler takes no parameters at all — no session is reachable from it, so zero-DB holds by construction and reintroducing a query means adding a dependency back on that line, in a visible diff. The test asserts it returns a valid token with DATABASE_URL unset entirely.

Two corrections made during review

A lost-update race in refresh.rotate() silently bypassed reuse detection. The row was read without a lock, so two concurrent presentations of the same token both saw rotated_at IS NULL, both passed the reuse check, and both minted a successor — exactly the attacker-replays-while-the-victim-refreshes case the family mechanism exists to catch. Fixed with .with_for_update(). Verified non-vacuous: removing the lock makes the new test fail with two live tokens in one family.

The demo rate limit was self-defeating and has been removed. Enforcing it was a Postgres write, so a rejected request restarted Neon's 5-minute autosuspend window exactly as an accepted one did — the control cost the resource it protected. One request per minute keeps the database awake 100% of the time (~182 CU-hr/month against a 100 CU-hr allowance) while fitting inside any limit Hobby permits. Replaced by a Vercel WAF rule on /api/auth/* (20 req / 10 min / IP). ⚠️ Deleting that WAF rule silently removes the only rate limit on demo-token minting — recorded in CLAUDE.md, since nothing in the codebase can hint at it.

Login limits were left at 10/15 min on purpose: a login is one write, so the Neon cost is the same 5-minute wake at 3 attempts or 30, and lowering it only punishes someone mistyping a password. A second bucket keyed on the attempted email was added instead, which is what survives an attacker rotating IPs. Both counters go through one INSERT.

Testing

71 tests. Per the testing policy, auth is on the write-tests list, as are the route-enumeration invariants. Notable: the rotation-race test uses two real sessions on separate connections; the demo test runs with no database configured; the single-statement rate-limit test inspects the emitted SQL. Each new test was confirmed to fail without its change.

Follow-ups, not in this PR

  • Security response headers (HSTS, CSP, Referrer-Policy) — CSP needs care against Vite's output and Module Federation, and a real deploy to verify.
  • Auth UI is PR chore: upgrade FastAPI/Starlette, add security response headers #6. Note the contract: a client in demo mode must drop its demo token before calling login or register, or those 403 (the demo write-ban covers all mutating routes).
  • Neon dev already has 0002 applied and is seeded; main deliberately still has no schema.

Verification

npm run check → 9/9 green locally. With DATABASE_URL unset: 48 passed, 23 skipped.

Deny-by-default authentication for the whole API, plus a read-only demo
mode, refresh-token families with reuse detection, and a Postgres-backed
rate limiter.

Enforcement is a single global dependency rather than per-router opt-in,
because forgetting an opt-in fails open — an unprotected endpoint that
behaves perfectly in every test anyone thought to write. A route is public
only by appearing in PUBLIC_ROUTES, and two tests walk the route table to
prove it: one asserts every unlisted route 401s anonymously, the other
asserts every mutating route 403s for a demo token.

Access tokens are HS256 JWTs verified without touching the database, and
live 3 hours rather than 15 minutes: refresh rotation is a write, so a
short token would wake Neon every 15 minutes for a whole training session
and become the largest consumer of the free tier's compute allowance.
Revocation therefore lives in the refresh family, not the access token.

/api/auth/demo issues zero SQL — the handler takes no parameters at all,
so zero-DB holds by construction rather than by convention. Its rate limit
moved to a Vercel WAF rule on /api/auth/*, because enforcing a limit in
Postgres is itself a write: a rejected request restarted Neon's 5-minute
autosuspend window exactly as an accepted one did, so the control cost the
resource it existed to protect.

Also adds a manual workflow_dispatch migration runner. Migrations must
never be automatic on push — a schema change must not race a deploy — and
production runs now require an approval instead of someone's laptop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
climb-trainer Ready Ready Preview Aug 13, 2026 5:55pm

@kilianmc
kilianmc merged commit 7bdfd1a into dev Aug 13, 2026
5 checks passed
@kilianmc
kilianmc deleted the feat/auth branch August 13, 2026 18:01
kilianmc added a commit that referenced this pull request Aug 13, 2026
`workflow_dispatch` workflows only register if the file exists on the
repository's DEFAULT branch, which here is `main`. `migrate.yml` landed on
`dev` in PR #2, so it was never registered: `gh workflow run migrate.yml`
returned `HTTP 404: workflow migrate.yml not found on the default branch`
and it did not appear in the Actions UI at all. The two GitHub environments
(`dev`, `production`) and their connection secrets were correct but
unreachable.

This is a deliberate exception to "main receives only dev->main promotion
PRs": the file is CI-only, has no runtime effect, and cannot do its job —
keeping schema changes off a laptop — while unregistered.

The content is byte-identical to dev's copy on purpose. The branches' common
ancestor does not contain this file, so any difference would surface as an
add/add conflict at the first promotion; identical content merges silently.

Note for whoever dispatches it: choose the ref deliberately. GitHub takes the
job definition AND the checkout from the selected ref, and `main` has no
`migrations/` directory or alembic dependency until the first promotion, so
until then it must be run with `--ref dev`.

No version bump: this is not a release, and the root package.json stays the
sole source of truth for the app's version.

Co-authored-by: Kilian Mateo <13885240+kilianmc@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
Preview — b8c547be Deployed Aug 13, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant