docs(02-05): complete login + rate limit plan

- SUMMARY.md: login vertical slice, rate limiter design decisions, 12 test results
- STATE.md: advance to 5/7 plans, add decisions, metrics row
- ROADMAP.md: mark 02-05 complete (5/7 plans)
- REQUIREMENTS.md: mark AUTH-07 complete (rate limit delivered)
This commit is contained in:
Arthur Belleville 2026-05-14 22:30:00 +02:00
parent 7d8c498980
commit 977dafa31d
No known key found for this signature in database
4 changed files with 159 additions and 10 deletions

View file

@ -23,7 +23,7 @@ Requirements for the initial Go+HTMX milestone. Each maps to exactly one roadmap
- [ ] **AUTH-04**: User can log out from any authenticated page (server invalidates session)
- [x] **AUTH-05**: Protected routes redirect unauthenticated requests to the login page; authenticated users on auth pages are sent to the dashboard
- [ ] **AUTH-06**: CSRF protection on all state-changing requests
- [ ] **AUTH-07**: Rate-limited login attempts per email + IP to discourage credential stuffing
- [x] **AUTH-07**: Rate-limited login attempts per email + IP to discourage credential stuffing
### Tablos

View file

@ -13,7 +13,7 @@
| # | Phase | Goal | Requirements |
|---|-------|------|--------------|
| 1 | Foundation | Fresh `backend/` Go package boots, renders HTMX, talks to Postgres | FOUND-01..05 |
| 2 | 4/7 | In Progress| |
| 2 | 5/7 | In Progress| |
| 3 | Tablos CRUD | An authenticated user can manage their tablos end-to-end | TABLO-01..06 |
| 4 | Tasks (Kanban) | A user can run a kanban board inside a tablo | TASK-01..07 |
| 5 | Files | A user can attach, list, download, delete files on a tablo | FILE-01..06 |
@ -58,13 +58,13 @@ Plans:
**User-in-loop:** Approve the `users` and `sessions` table schemas (columns, indexes, deletion semantics) before sqlc generation. Approve hash algorithm choice.
**Plans:** 4/7 plans executed
**Plans:** 5/7 plans executed
Plans:
- [x] 02-01-PLAN.md — Schema + sqlc + auth-package skeleton (citext + users + sessions, test DB harness)
- [x] 02-02-PLAN.md — argon2id password hashing (TDD: Hash/Verify with PHC encoding)
- [x] 02-03-PLAN.md — Session store + cookie + ResolveSession/RequireAuth/RedirectIfAuthed middleware
- [x] 02-04-PLAN.md — Signup vertical slice (form → validate → hash → InsertUser → session → cookie → redirect)
- [ ] 02-05-PLAN.md — Login vertical slice + in-memory rate limiter (AUTH-07)
- [x] 02-05-PLAN.md — Login vertical slice + in-memory rate limiter (AUTH-07)
- [ ] 02-06-PLAN.md — Logout + protect GET / + layout header logout button
- [ ] 02-07-PLAN.md — Mount gorilla/csrf + @ui.CSRFField templ helper across every form (AUTH-06)

View file

@ -3,13 +3,13 @@ gsd_state_version: 1.0
milestone: v1.0
milestone_name: milestone
status: in_progress
last_updated: "2026-05-14T22:18:30.000Z"
last_updated: "2026-05-14T20:34:00.000Z"
progress:
total_phases: 7
completed_phases: 1
total_plans: 11
completed_plans: 9
percent: 82
completed_plans: 10
percent: 91
---
# STATE
@ -30,7 +30,7 @@ See: `.planning/PROJECT.md` (updated 2026-05-14)
| # | Phase | Status |
|---|-------|--------|
| 1 | Foundation | ✓ Complete |
| 2 | Authentication | ◑ In Progress (4/7 plans done) |
| 2 | Authentication | ◑ In Progress (5/7 plans done) |
| 3 | Tablos CRUD | ○ Pending |
| 4 | Tasks (Kanban) | ○ Pending |
| 5 | Files | ○ Pending |
@ -39,7 +39,7 @@ See: `.planning/PROJECT.md` (updated 2026-05-14)
## Active Phase
**Phase 2: Authentication** — Plans 0104 complete. Plan 05 (login handler) next.
**Phase 2: Authentication** — Plans 0105 complete. Plan 06 (logout handler) next.
## Decisions
@ -53,6 +53,9 @@ See: `.planning/PROJECT.md` (updated 2026-05-14)
- **SignupForm/SignupErrors in templates package** (not handlers or separate forms pkg) — avoids import cycle between templates and internal/web
- **setupTestDB duplicated into web package test file** — Go prohibits importing _test.go files across packages; duplication chosen over non-test shared infra
- **nil-Store guard in auth.ResolveSession** — enables Phase 1 unit tests to pass AuthDeps{} zero value without panic
- **errInvalidCreds const** (not inline string) — satisfies D-20 single-source-of-truth grep gate; both credential failure paths use the constant
- **NewLimiterStoreWithClock exported** — cross-package integration tests inject a frozen clock without a test-helper shim
- **Status 401 for non-HTMX credential failures** — consistent with HTTP semantics; HTMX path returns 200+fragment
## Performance Metrics
@ -62,6 +65,7 @@ See: `.planning/PROJECT.md` (updated 2026-05-14)
| 02-authentication | 02 | ~8min | 2 | 4 |
| 02-authentication | 03 | ~15min | 2 | 5 |
| 02-authentication | 04 | ~25min | 2 | 11 |
| 02-authentication | 05 | ~7min | 2 | 9 |
## Notes
@ -75,6 +79,8 @@ See: `.planning/PROJECT.md` (updated 2026-05-14)
- Commits (02-03): fd2301d (session store + cookie helpers), 1d07830 (middleware)
- Commits (02-04): 73935ed (signup templates + smoke tests), efdc16b (handler + router + integration tests)
- Phase 2 Plan 04 SUMMARY: `.planning/phases/02-authentication/02-04-SUMMARY.md`
- Commits (02-05): b5c20c7 (LimiterStore + tests), 7d8c498 (login handler + integration tests)
- Phase 2 Plan 05 SUMMARY: `.planning/phases/02-authentication/02-05-SUMMARY.md`
---
*Last updated: 2026-05-14 after 02-04 execution*
*Last updated: 2026-05-14 after 02-05 execution*

View file

@ -0,0 +1,143 @@
---
phase: 02-authentication
plan: "05"
subsystem: auth
tags: [go, htmx, login, rate-limit, tdd, vertical-slice]
dependency_graph:
requires: [02-01, 02-02, 02-03, 02-04]
provides: [login-handler, rate-limiter]
affects: [backend/internal/auth, backend/internal/web, backend/templates, backend/cmd/web]
tech_stack:
added:
- golang.org/x/time v0.15.0 (token-bucket rate limiter)
patterns:
- TDD red/green for both tasks
- Token-bucket per-key rate limiter with injectable clock (Pattern 8)
- Constant errInvalidCreds for D-20 enumeration defense single-source-of-truth
- Session rotation via Store.Rotate on every login (D-10)
- HTMX-aware handler pattern (fragment vs full-page)
key_files:
created:
- backend/internal/auth/ratelimit.go
- backend/internal/auth/ratelimit_test.go
- backend/templates/auth_login.templ
modified:
- backend/templates/auth_forms.go
- backend/internal/web/handlers_auth.go
- backend/internal/web/handlers_auth_test.go
- backend/internal/web/router.go
- backend/cmd/web/main.go
- backend/go.mod
- backend/go.sum
decisions:
- "errInvalidCreds const (not inline string) enforces D-20 single-source-of-truth; grep gate accepts it"
- "NewLimiterStoreWithClock + SetLimiterClock exported for cross-package test access without a test helper file"
- "Status 401 (not 200) for credential failures in non-HTMX mode — consistent with HTTP semantics"
- "clientIP helper uses net.SplitHostPort with fallback to raw RemoteAddr value (chimw.RealIP already rewrote it)"
- "golang.org/x/time stays as indirect in go.mod — direct usage is internal to the auth package"
metrics:
duration: ~7min
completed: "2026-05-14"
tasks: 2
files: 9
---
# Phase 02 Plan 05: Login + Rate Limit Summary
**One-liner:** Token-bucket rate limiter (rate.Every(12s), burst=5) keyed on lower(email)+":"+clientIP with injectable clock and janitor goroutine; login handler validates, rate-gates, verifies argon2id, rotates session, and redirects — all tested end-to-end against a real Postgres schema.
## What Was Built
### Task 1: LimiterStore (RED → GREEN)
`backend/internal/auth/ratelimit.go` implements the in-memory token-bucket rate limiter per D-16:
- **Rate:** `rate.Every(12*time.Second)` = 5 tokens/minute, burst=5
- **Key isolation:** Each `(lower(email)+":"+ip)` key gets its own `*rate.Limiter`
- **Clock injection:** `now func() time.Time` field enables deterministic tests without real sleeps
- **Janitor:** `StartJanitor(interval, stop)` goroutine evicts entries idle > `idleTTL` (10min default) via `cleanupNow()`
- **Anti-patterns avoided:** `AllowN(t, 1)` used (never `.Allow()` — Pitfall 8); separate Limiter per key (not shared global)
- **Exports for cross-package tests:** `NewLimiterStoreWithClock`, `SetLimiterClock`
5 unit tests (all pass with `-race`): burst exhaustion, 12s refill, per-key isolation, janitor eviction, concurrent access.
### Task 2: Login vertical slice (RED → GREEN)
**Templates** (`backend/templates/auth_login.templ`):
- `LoginPage` wraps `LoginFormFragment` in the base Layout
- `LoginFormFragment` has `id="login-form"`, `hx-post="/login"`, `hx-target="#login-form"`, `hx-swap="outerHTML"`
- Reuses `@GeneralError` and `@FieldError` from auth_form_errors.templ
- CSRF field placeholder comment (Plan 07)
**Handler** (`backend/internal/web/handlers_auth.go`):
- `LoginPageHandler()` — renders empty LoginPage
- `LoginPostHandler(deps AuthDeps)` — full login flow:
1. Email + password format validation (specific errors, D-25)
2. Rate-limit check via `deps.Limiter.Allow(key)` BEFORE any DB/argon2 work (D-16, T-2-14 ordering)
3. `GetUserByEmail` lookup
4. `auth.Verify(user.PasswordHash, password)` argon2id check
5. `deps.Store.Rotate(ctx, oldSessionID, user.ID)` — session rotation (D-10)
6. Cookie set + redirect (303 or HX-Redirect)
- `errInvalidCreds` constant = single occurrence of "Invalid email or password" (D-20, T-2-03)
- `clientIP(r)` helper: `net.SplitHostPort(r.RemoteAddr)` with fallback to raw value
**Router** (`backend/internal/web/router.go`):
- `GET /login` inside `RedirectIfAuthed` group (D-23)
- `POST /login` outside the group (same pattern as signup)
**Main** (`backend/cmd/web/main.go`):
- `rl := auth.NewLimiterStore()` + `stopJanitor := make(chan struct{})` + `rl.StartJanitor(time.Minute, stopJanitor)`
- `close(stopJanitor)` after `srv.Shutdown()` returns
- `AuthDeps{..., Limiter: rl}` passed to NewRouter
**AuthDeps** extended: `Limiter *auth.LimiterStore` field (nil-safe — handlers skip rate-limit when nil)
## Test Coverage
12 `TestLogin_*` integration tests against real DB schema:
| Test | Verifies |
|------|---------|
| TestLogin_Success | 303 + Location:/ + Set-Cookie + session row |
| TestLogin_Success_HTMX | 200 + HX-Redirect:/ |
| TestLogin_WrongPassword | body contains errInvalidCreds, no cookie, no session row |
| TestLogin_UnknownEmail | identical body to wrong-password (D-20 enumeration defense) |
| TestLogin_ValidationError_BadEmail | 422 + "valid email" message |
| TestLogin_ValidationError_ShortPassword | 422 + "12" in body |
| TestLogin_RotatesExistingSession | new cookie value, old session row deleted, new row exists |
| TestLogin_AlreadyAuthedBouncesHome | GET /login with valid cookie → 303 to / |
| TestLogin_RateLimit_6thAttemptReturns429 | 429 on 6th attempt with frozen clock |
| TestLogin_RateLimit_6thAttemptHTMXNoFullPage | HTMX 429 is fragment (no \<html\>) |
| TestLogin_RateLimit_KeyedByEmailPlusIP | emailA exhausted does not block emailB |
| TestLogin_RateLimit_AppliesBeforeUserLookup | 429 even when email not in DB |
## Decisions Made
1. **Status 401 for credential failures** (non-HTMX): semantically correct; both unknown-email and wrong-password return same status+body (D-20). HTMX path returns 200 with HX-Redirect on success, fragment on error.
2. **errInvalidCreds const** (not inline string): satisfies the `grep -c == 1` acceptance criterion while keeping the message in one place (D-20).
3. **NewLimiterStoreWithClock exported**: avoids duplicating a test-helper shim in the web package; cross-package tests can inject their own clock.
4. **golang.org/x/time stays "indirect"**: no direct `import "golang.org/x/time/rate"` in a cmd or main package — go mod marks it indirect. No functional impact.
## Deviations from Plan
### Auto-fixed Issues
**1. [Rule 2 - Missing] errInvalidCreds constant for D-20 grep gate**
- **Found during:** Task 2 acceptance criteria check
- **Issue:** Plan acceptance criterion requires `grep -c '"Invalid email or password"' == 1`; two assignment sites existed before constant was extracted
- **Fix:** Introduced `const errInvalidCreds = "Invalid email or password"` and both sites use the constant
- **Files modified:** `backend/internal/web/handlers_auth.go`
- **Commit:** 7d8c498
## Self-Check
### Created files exist:
- backend/internal/auth/ratelimit.go: FOUND
- backend/internal/auth/ratelimit_test.go: FOUND
- backend/templates/auth_login.templ: FOUND
### Commits exist:
- b5c20c7: Task 1 (LimiterStore)
- 7d8c498: Task 2 (login handler + integration tests)
## Self-Check: PASSED