diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 098f389..e54be6f 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -13,7 +13,7 @@ | # | Phase | Goal | Requirements | |---|-------|------|--------------| | 1 | Foundation | Fresh `backend/` Go package boots, renders HTMX, talks to Postgres | FOUND-01..05 | -| 2 | 6/7 | In Progress| | +| 2 | Authentication | Complete (7/7) | AUTH-01..07 | | 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,7 +58,7 @@ Plans: **User-in-loop:** Approve the `users` and `sessions` table schemas (columns, indexes, deletion semantics) before sqlc generation. Approve hash algorithm choice. -**Plans:** 6/7 plans executed +**Plans:** 7/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) @@ -66,7 +66,7 @@ Plans: - [x] 02-04-PLAN.md — Signup vertical slice (form → validate → hash → InsertUser → session → cookie → redirect) - [x] 02-05-PLAN.md — Login vertical slice + in-memory rate limiter (AUTH-07) - [x] 02-06-PLAN.md — Logout + protect GET / + layout header logout button (AUTH-04, AUTH-05) -- [ ] 02-07-PLAN.md — Mount gorilla/csrf + @ui.CSRFField templ helper across every form (AUTH-06) +- [x] 02-07-PLAN.md — Mount gorilla/csrf + @ui.CSRFField templ helper across every form (AUTH-06) ### Phase 3: Tablos CRUD **Goal:** A logged-in user can list, create, view, edit, and delete their tablos end-to-end through HTMX-driven flows. diff --git a/.planning/STATE.md b/.planning/STATE.md index 8437c4f..ef94b43 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,13 +3,13 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone status: in_progress -last_updated: "2026-05-14T20:41:09.893Z" +last_updated: "2026-05-14T21:00:30.036Z" progress: total_phases: 7 - completed_phases: 1 - total_plans: 11 - completed_plans: 11 - percent: 92 + completed_phases: 2 + total_plans: 12 + completed_plans: 12 + percent: 100 --- # STATE @@ -30,7 +30,7 @@ See: `.planning/PROJECT.md` (updated 2026-05-14) | # | Phase | Status | |---|-------|--------| | 1 | Foundation | ✓ Complete | -| 2 | Authentication | ◑ In Progress (6/7 plans done) | +| 2 | Authentication | ✓ Complete (7/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 01–06 complete. Plan 07 (CSRF) next. +**Phase 2: Authentication** — All 7 plans complete (AUTH-06 closed). Phase 3: Tablos CRUD next. ## Decisions @@ -58,6 +58,8 @@ See: `.planning/PROJECT.md` (updated 2026-05-14) - **Status 401 for non-HTMX credential failures** — consistent with HTTP semantics; HTMX path returns 200+fragment - **Layout takes *auth.User explicitly** (not thread-through-ctx) — easier to test templates in isolation; no magic context values - **TestIndex_RendersHxGet rewritten to TestIndex_UnauthRedirects** — GET / is now protected; Phase 1 test was a false positive after this plan +- **csrf.PlaintextHTTPRequest(r) in dev mode** — skips TLS Referer check in plain-HTTP local dev/httptest; production unaffected +- **trustedOrigins variadic arg in auth.Mount + NewRouter** — test routers pass "localhost"; production passes none ## Performance Metrics @@ -69,6 +71,7 @@ See: `.planning/PROJECT.md` (updated 2026-05-14) | 02-authentication | 04 | ~25min | 2 | 11 | | 02-authentication | 05 | ~7min | 2 | 9 | | 02-authentication | 06 | ~12min | 1 | 9 | +| 02-authentication | 07 | ~25min | 1 | 18 | ## Notes @@ -86,6 +89,8 @@ See: `.planning/PROJECT.md` (updated 2026-05-14) - Phase 2 Plan 05 SUMMARY: `.planning/phases/02-authentication/02-05-SUMMARY.md` - Commits (02-06): b5c3fc4 (RED tests), 8b54ff4 (logout + protected routes + layout) - Phase 2 Plan 06 SUMMARY: `.planning/phases/02-authentication/02-06-SUMMARY.md` +- Commits (02-07): ae2d356 (RED tests), 389e1bc (GREEN csrf implementation) +- Phase 2 Plan 07 SUMMARY: `.planning/phases/02-authentication/02-07-SUMMARY.md` --- -*Last updated: 2026-05-14 after 02-06 execution* +*Last updated: 2026-05-14 after 02-07 execution (Phase 2 complete)* diff --git a/.planning/phases/02-authentication/02-07-SUMMARY.md b/.planning/phases/02-authentication/02-07-SUMMARY.md new file mode 100644 index 0000000..322e006 --- /dev/null +++ b/.planning/phases/02-authentication/02-07-SUMMARY.md @@ -0,0 +1,218 @@ +--- +phase: 02-authentication +plan: "07" +subsystem: auth/csrf +tags: [go, csrf, gorilla, htmx, templ, security-hardening] +requirements: [AUTH-06] +dependency_graph: + requires: [02-01, 02-03, 02-04, 02-05, 02-06] + provides: [gorilla/csrf middleware, ui.CSRFField component, env-driven CSRF key] + affects: [all state-changing POST routes, all form-rendering templates] +tech_stack: + added: [github.com/gorilla/csrf v1.7.3] + patterns: [double-submit CSRF protection, templ component composition, env-driven secrets] +key_files: + created: + - backend/internal/auth/csrf.go + - backend/internal/web/ui/csrf_field.templ + - backend/internal/web/csrf_test.go + - backend/internal/web/csrf_debug_test.go + - backend/internal/auth/csrf_test.go + modified: + - backend/internal/web/router.go + - backend/internal/web/handlers_auth.go + - backend/internal/web/handlers_auth_test.go + - backend/internal/web/handlers.go + - backend/internal/web/handlers_test.go + - backend/templates/layout.templ + - backend/templates/auth_login.templ + - backend/templates/auth_signup.templ + - backend/templates/index.templ + - backend/templates/layout_test.go + - backend/templates/auth_signup_test.go + - backend/cmd/web/main.go + - backend/go.mod + - backend/go.sum + - backend/.env.example +decisions: + - gorilla/csrf v1.7.3 in dev mode uses csrf.PlaintextHTTPRequest to skip TLS Referer check — local HTTP development + integration tests work without https + - trustedOrigins variadic arg added to NewRouter + auth.Mount — "localhost" is passed from test routers; production passes none + - csrf_debug_test.go retained as diagnostic aid (not removed after GREEN) +metrics: + duration: ~25min + completed: 2026-05-14 + tasks: 1 + files: 18 +--- + +# Phase 2 Plan 07: gorilla/csrf Integration Summary + +**One-liner:** gorilla/csrf v1.7.3 mounted on the chi stack with env-driven 32-byte CSRF key, reusable ui.CSRFField component, and all POST routes enforcing 403 on missing token. + +## What Was Built + +AUTH-06 is closed. Every state-changing POST route (POST /signup, POST /login, POST /logout) now requires a valid CSRF token. Missing or invalid tokens return 403 Forbidden from gorilla/csrf before any handler logic runs. + +### auth.Mount and auth.LoadKeyFromEnv (backend/internal/auth/csrf.go) + +`Mount(env string, key []byte, trustedOrigins ...string)` builds `csrf.Protect` with the D-14 locked options: + +- `csrf.Secure(env != "dev")` — Secure cookie flag disabled only in local dev +- `csrf.SameSite(csrf.SameSiteLaxMode)` — interim SameSite=Lax defense +- `csrf.Path("/")` — CSRF cookie scoped to entire site +- `csrf.FieldName("_csrf")` — hidden form field name +- `csrf.RequestHeader("X-CSRF-Token")` — accepted header for HTMX hx-headers future usage + +In dev mode, `csrf.PlaintextHTTPRequest(r)` is applied before the CSRF middleware to skip the TLS Referer check. This allows local plain-HTTP development and httptest integration tests to function correctly without HTTPS. + +`LoadKeyFromEnv()` reads `SESSION_SECRET` from env, hex-decodes it, and validates `len == 32`. Returns `ErrCSRFKeyInvalid` for missing, non-hex, or wrong-length input. + +### ui.CSRFField Component (backend/internal/web/ui/csrf_field.templ) + +``` +templ CSRFField(token string) { + +} +``` + +The canonical, reusable component for embedding CSRF tokens in templ forms. Future forms must use `@ui.CSRFField(token)` as the first child of every `
`. + +### Templ Form Updates + +All form-rendering templ files were updated to accept `csrfToken string` as a third argument (additive — typed `form` and `errs` arguments from Plans 04/05 were preserved): + +| File | Components Updated | +|------|--------------------| +| `backend/templates/layout.templ` | `Layout(title, *auth.User, csrfToken)` — logout form embeds `@ui.CSRFField(csrfToken)` | +| `backend/templates/auth_login.templ` | `LoginPage(form, errs, csrfToken)`, `LoginFormFragment(form, errs, csrfToken)` | +| `backend/templates/auth_signup.templ` | `SignupPage(form, errs, csrfToken)`, `SignupFormFragment(form, errs, csrfToken)` | +| `backend/templates/index.templ` | `Index(*auth.User, csrfToken)` — wraps Layout which has logout form | + +### Handler Updates + +Every GET handler that renders a form now threads `csrf.Token(r)` into the templ render call: + +- `SignupPageHandler` → `templates.SignupPage(SignupForm{}, SignupErrors{}, csrf.Token(r))` +- `LoginPageHandler` → `templates.LoginPage(LoginForm{}, LoginErrors{}, csrf.Token(r))` +- `IndexHandler` → `templates.Index(user, csrf.Token(r))` +- Error-branch renders in `SignupPostHandler` and `LoginPostHandler` also pass `csrf.Token(r)` + +All handlers use `r.PostFormValue(...)` exclusively (never `r.Body`) per Pitfall 1. + +### Router Middleware Order (D-24) + +```go +// D-24 locked order: ResolveSession BEFORE csrf.Protect (auth.Mount). +r.Use(auth.ResolveSession(deps.Store)) +// D-24: gorilla/csrf runs after ResolveSession and before all route groups (Pitfall 7). +r.Use(auth.Mount(env, csrfKey, trustedOrigins...)) +``` + +`ResolveSession` appears at line 54; `auth.Mount` at line 56 in router.go. + +### main.go Startup Guard + +```go +csrfKey, err := auth.LoadKeyFromEnv() +if err != nil { + slog.Error("invalid SESSION_SECRET", "err", err, "hint", "generate with: openssl rand -hex 32") + os.Exit(1) +} +``` + +The binary fails fast on missing or invalid `SESSION_SECRET`. `SESSION_SECRET` is documented in `.env.example` with the generation command. + +## Test Coverage + +### New Tests (backend/internal/web/csrf_test.go) + +| Test | Behavior Verified | +|------|-------------------| +| `TestCSRF_LoginMissingToken` | POST /login without _csrf → 403 | +| `TestCSRF_LoginValidToken` | GET /login → extract token → POST → 200/303 | +| `TestCSRF_SignupMissingToken` | POST /signup without _csrf → 403 | +| `TestCSRF_SignupValidToken` | GET /signup → extract token → POST → 303 | +| `TestCSRF_LogoutMissingToken` | POST /logout without _csrf → 403; session NOT deleted | +| `TestCSRF_LogoutValidToken` | GET / → extract token → POST /logout → 303; session deleted | +| `TestCSRF_HeaderFallback` | X-CSRF-Token header accepted (future HTMX hx-headers path) | +| `TestForms_ContainCSRFField` | All rendered forms contain `name="_csrf"` hidden input | +| `TestRouter_CSRFMountedAfterResolveSession` | Source scan confirms middleware order (D-24) | + +### New Tests (backend/internal/auth/csrf_test.go) + +| Test | Behavior Verified | +|------|-------------------| +| `TestLoadCSRFKey_Missing` | Unset env → error | +| `TestLoadCSRFKey_WrongLength` | 31-byte hex → error | +| `TestLoadCSRFKey_Valid` | 32-byte hex → returns []byte | +| `TestLoadCSRFKey_InvalidHex` | Non-hex string → error | + +### Prior Tests Updated (Plans 04/05/06) + +All existing signup/login/logout integration tests in `handlers_auth_test.go` were updated to use the `getCSRFToken` helper: GET the form page, extract the `_csrf` token from the rendered HTML, include the gorilla CSRF cookie, and submit `_csrf` in the POST body. Tests no longer submit bare POSTs. + +The `getCSRFToken` helper (existing in the test file) handles this pattern uniformly across all tests. + +## Middleware Chain (Final State) + +``` +RequestIDMiddleware +chimw.RealIP +SlogLoggerMiddleware +chimw.Recoverer +auth.ResolveSession(store) # D-24: reads session cookie, attaches user to ctx +auth.Mount(env, csrfKey) # D-24: gorilla/csrf after session, before routes +├── RedirectIfAuthed group +│ ├── GET /signup +│ └── GET /login +├── POST /signup +├── POST /login +├── RequireAuth group +│ ├── GET / +│ └── POST /logout +├── GET /healthz +├── GET /demo/time +└── GET /static/* +``` + +## HTMX Token Surfacing Decision + +CSRF token is embedded in every form via `@ui.CSRFField(csrfToken)` (hidden input). For future HTMX use cases that submit JSON or non-form POSTs, the `X-CSRF-Token` header is also accepted (wired via `csrf.RequestHeader("X-CSRF-Token")`). HTMX `hx-headers` configuration is left for a future polish pass — the form field is the canonical path for v1. + +## Deviations from Plan + +### Auto-adapted: gorilla/csrf dev-mode TLS skip + +The plan specified `csrf.Protect` options from D-14 but did not explicitly describe how to handle the gorilla/csrf Referer check in plain-HTTP development mode. gorilla/csrf performs a Referer header check for HTTPS requests. In dev/httptest contexts with `env="dev"`, this would cause 403s even with valid tokens. + +**Fix:** `csrf.PlaintextHTTPRequest(r)` is applied to every request before the CSRF middleware when `env == "dev"`. This is a documented gorilla/csrf API for HTTP contexts and is the correct approach per gorilla/csrf documentation. The fix is dev-env scoped and has no production impact. + +### Auto-adapted: trustedOrigins variadic arg + +The plan mentioned `trustedOrigins` as a test-only concept but did not specify how to thread it from `NewRouter` to `auth.Mount`. Added `trustedOrigins ...string` as a variadic parameter to both `auth.Mount` and `NewRouter`. Test routers pass `"localhost"` as a trusted origin. Production call in `main.go` passes no extra args. + +## Known Stubs + +None. All forms render real CSRF tokens from gorilla/csrf state. No hardcoded or placeholder token values in any template or handler. + +## Threat Surface Scan + +All threat register items for T-2-08 through T-2-08d are addressed: + +| Threat | Status | +|--------|--------| +| T-2-08: CSRF on POST routes | Mitigated — gorilla/csrf middleware enforces 403 | +| T-2-08a: Missing _csrf in future forms | Mitigated — ui.CSRFField component + acceptance grep | +| T-2-08b: CSRF key in repo/compile-time | Mitigated — LoadKeyFromEnv + fail-fast startup | +| T-2-08c: Body consumed by CSRF middleware | Mitigated — all handlers use r.PostFormValue only | +| T-2-08d: Wrong middleware order | Mitigated — D-24 locked order, source-scan test | + +## Self-Check: PASSED + +- `backend/internal/auth/csrf.go` exists: FOUND +- `backend/internal/web/ui/csrf_field.templ` exists: FOUND +- `backend/internal/web/csrf_test.go` exists: FOUND +- `backend/internal/auth/csrf_test.go` exists: FOUND +- Commit ae2d356 (RED tests): FOUND +- Commit 389e1bc (GREEN implementation): FOUND +- All 37 tests in TestCSRF|TestForms|TestRouter|TestLoadCSRFKey|TestSignup|TestLogin|TestLogout|TestProtected|TestLayout: PASS