docs(02): create phase plan (7 plans, 6 waves)
7 vertical-slice plans covering AUTH-01..07: schema+sqlc skeleton → password (TDD) || session+middleware → signup → login+ratelimit → logout+protect → CSRF. All 26 CONTEXT.md decisions cited; threat model covers T-2-01..T-2-10. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
194072bd9a
commit
032e020fe3
10 changed files with 1467 additions and 32 deletions
|
|
@ -3,13 +3,13 @@ gsd_state_version: 1.0
|
|||
milestone: v1.0
|
||||
milestone_name: milestone
|
||||
status: ready_to_plan
|
||||
last_updated: "2026-05-14T18:55:27.627Z"
|
||||
last_updated: "2026-05-14T19:44:02.762Z"
|
||||
progress:
|
||||
total_phases: 7
|
||||
completed_phases: 1
|
||||
total_plans: 4
|
||||
total_plans: 11
|
||||
completed_plans: 4
|
||||
percent: 100
|
||||
percent: 36
|
||||
---
|
||||
|
||||
# STATE
|
||||
|
|
|
|||
253
.planning/phases/02-authentication/02-01-PLAN.md
Normal file
253
.planning/phases/02-authentication/02-01-PLAN.md
Normal file
|
|
@ -0,0 +1,253 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- backend/migrations/0002_auth.sql
|
||||
- backend/sqlc.yaml
|
||||
- backend/internal/db/queries/users.sql
|
||||
- backend/internal/db/queries/sessions.sql
|
||||
- backend/internal/db/sqlc/
|
||||
- backend/internal/auth/doc.go
|
||||
- backend/internal/auth/types.go
|
||||
- backend/internal/auth/testdb_test.go
|
||||
- backend/.env.example
|
||||
- backend/go.mod
|
||||
- backend/go.sum
|
||||
autonomous: true
|
||||
requirements: [AUTH-01, AUTH-02]
|
||||
tags: [go, postgres, sqlc, migration, auth-foundation]
|
||||
must_haves:
|
||||
truths:
|
||||
- "users + sessions tables exist with citext email and SHA-256 session id columns (D-01, D-04)"
|
||||
- "users table has NO deleted_at column — hard-delete only, no soft-delete reserved (D-02)"
|
||||
- "users table has NO email_verified_at column — email verification deferred (D-03)"
|
||||
- "sqlc generates Go bindings for InsertUser, GetUserByEmail, InsertSession, GetSessionWithUser, DeleteSession, DeleteSessionsByUser, ExtendSession"
|
||||
- "TEST_DATABASE_URL-driven Postgres harness creates an isolated schema, runs migrations, and tears down cleanly"
|
||||
- "internal/auth package compiles with shared types (User, Session, Params)"
|
||||
- ".env.example documents TEST_DATABASE_URL and SESSION_SECRET (D-14, D-26)"
|
||||
artifacts:
|
||||
- path: backend/migrations/0002_auth.sql
|
||||
provides: "citext extension + users + sessions tables (D-01, D-04)"
|
||||
contains: "CREATE TABLE users"
|
||||
- path: backend/internal/db/queries/users.sql
|
||||
provides: "InsertUser, GetUserByEmail named queries"
|
||||
- path: backend/internal/db/queries/sessions.sql
|
||||
provides: "InsertSession, GetSessionWithUser (with expires_at gate), DeleteSession, DeleteSessionsByUser, ExtendSession"
|
||||
- path: backend/internal/db/sqlc/users.sql.go
|
||||
provides: "Generated Go bindings for users queries"
|
||||
- path: backend/internal/db/sqlc/sessions.sql.go
|
||||
provides: "Generated Go bindings for sessions queries"
|
||||
- path: backend/internal/auth/types.go
|
||||
provides: "User and Session struct definitions consumed by later plans"
|
||||
- path: backend/internal/auth/testdb_test.go
|
||||
provides: "setupTestDB(t) helper used by every DB-touching auth test"
|
||||
key_links:
|
||||
- from: backend/sqlc.yaml
|
||||
to: backend/internal/db/queries/{users,sessions}.sql
|
||||
via: "queries directory + citext/uuid overrides"
|
||||
pattern: "db_type: \"citext\""
|
||||
- from: backend/internal/auth/testdb_test.go
|
||||
to: backend/migrations/0002_auth.sql
|
||||
via: "harness runs goose up against a per-test schema"
|
||||
pattern: "goose.Up"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Lay the Phase 2 substrate: schema migration, sqlc queries + overrides + generated bindings, the `internal/auth` package skeleton (types, doc, shared constants), and a real-Postgres test harness. No user-visible behavior yet — every later plan in this phase depends on these artifacts.
|
||||
|
||||
Purpose: avoid "scavenger hunt" downstream — Wave 2 plans implement password/session logic against contracts that already exist, not against ad-hoc interfaces invented at execution time.
|
||||
Output: applied migration, generated sqlc code in `internal/db/sqlc/`, compiling `internal/auth` package, working `TEST_DATABASE_URL` harness.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/01-foundation/01-SUMMARY.md
|
||||
@backend/sqlc.yaml
|
||||
@backend/migrations/0001_init.sql
|
||||
@backend/internal/db/pool.go
|
||||
@backend/internal/db/pool_test.go
|
||||
@backend/justfile
|
||||
|
||||
<interfaces>
|
||||
Generated code (after `sqlc generate`) will live in `backend/internal/db/sqlc/` and expose, at minimum:
|
||||
|
||||
```go
|
||||
package sqlc
|
||||
|
||||
type Queries struct { /* ... */ }
|
||||
func New(db DBTX) *Queries
|
||||
func (q *Queries) WithTx(tx pgx.Tx) *Queries
|
||||
|
||||
// users.sql
|
||||
type InsertUserParams struct { Email string; PasswordHash string }
|
||||
type User struct { ID uuid.UUID; Email string; PasswordHash string; CreatedAt time.Time; UpdatedAt time.Time }
|
||||
func (q *Queries) InsertUser(ctx context.Context, arg InsertUserParams) (User, error)
|
||||
func (q *Queries) GetUserByEmail(ctx context.Context, email string) (User, error)
|
||||
|
||||
// sessions.sql
|
||||
type InsertSessionParams struct { ID string; UserID uuid.UUID; ExpiresAt time.Time }
|
||||
func (q *Queries) InsertSession(ctx context.Context, arg InsertSessionParams) error
|
||||
type GetSessionWithUserRow struct { /* session + joined user columns */ }
|
||||
func (q *Queries) GetSessionWithUser(ctx context.Context, id string) (GetSessionWithUserRow, error)
|
||||
func (q *Queries) DeleteSession(ctx context.Context, id string) error
|
||||
func (q *Queries) DeleteSessionsByUser(ctx context.Context, userID uuid.UUID) error
|
||||
type ExtendSessionParams struct { ID string; ExpiresAt time.Time }
|
||||
func (q *Queries) ExtendSession(ctx context.Context, arg ExtendSessionParams) error
|
||||
```
|
||||
|
||||
Hand-written types this plan introduces in `internal/auth/types.go`:
|
||||
|
||||
```go
|
||||
package auth
|
||||
|
||||
type User struct { ID uuid.UUID; Email string; PasswordHash string; CreatedAt, UpdatedAt time.Time }
|
||||
type Session struct { ID string; UserID uuid.UUID; CreatedAt, ExpiresAt time.Time }
|
||||
|
||||
// SessionCookieName is the cookie key used across the package (planner picks "xtablo_session" per D-12).
|
||||
const SessionCookieName = "xtablo_session"
|
||||
// SessionTTL is the sliding 30-day window (D-09).
|
||||
const SessionTTL = 30 * 24 * time.Hour
|
||||
// SessionExtendThreshold triggers extension when remaining < 7 days (D-09).
|
||||
const SessionExtendThreshold = 7 * 24 * time.Hour
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Write migration 0002_auth.sql + Up/Down round-trip test</name>
|
||||
<read_first>
|
||||
backend/migrations/0001_init.sql
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 11)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-01, D-04)
|
||||
</read_first>
|
||||
<action>
|
||||
Create `backend/migrations/0002_auth.sql` with goose `-- +goose Up` / `-- +goose Down` annotations matching the style of `0001_init.sql`. Up: `CREATE EXTENSION IF NOT EXISTS citext;` then `CREATE EXTENSION IF NOT EXISTS pgcrypto;` then `users` table (per D-01: `id uuid PK default gen_random_uuid()`, `email citext NOT NULL UNIQUE`, `password_hash text NOT NULL`, `created_at`/`updated_at timestamptz NOT NULL default now()`) then `sessions` table (per D-04: `id text PK`, `user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE`, `created_at`/`expires_at timestamptz NOT NULL default now()/no default`) plus `CREATE INDEX sessions_user_id_idx ON sessions(user_id);` and `CREATE INDEX sessions_expires_at_idx ON sessions(expires_at);`. Down: `DROP TABLE IF EXISTS sessions; DROP TABLE IF EXISTS users;` (leave extensions). Do NOT add `deleted_at` (D-02), `email_verified_at` (D-03), or any `user_agent`/`ip_address`/`last_seen_at` columns (D-04). Then run `just migrate up` against the local compose Postgres followed by `just migrate down` then `just migrate up` again to prove the round-trip is clean.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && just migrate up && just migrate down && just migrate up && psql "$DATABASE_URL" -c "\d users" -c "\d sessions" | grep -E "(citext|uuid|sessions_user_id_idx|sessions_expires_at_idx)"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- File `backend/migrations/0002_auth.sql` exists and contains both `-- +goose Up` and `-- +goose Down` markers.
|
||||
- `grep -v '^-' backend/migrations/0002_auth.sql | grep -c "CREATE TABLE users"` == 1.
|
||||
- `grep -v '^-' backend/migrations/0002_auth.sql | grep -c "CREATE TABLE sessions"` == 1.
|
||||
- `grep -c "citext" backend/migrations/0002_auth.sql` >= 2 (extension + column type).
|
||||
- `grep -c "ON DELETE CASCADE" backend/migrations/0002_auth.sql` == 1.
|
||||
- `grep -c "sessions_user_id_idx" backend/migrations/0002_auth.sql` == 1; `grep -c "sessions_expires_at_idx" backend/migrations/0002_auth.sql` == 1.
|
||||
- `grep -c "deleted_at\|email_verified_at\|user_agent\|ip_address\|last_seen_at" backend/migrations/0002_auth.sql` == 0.
|
||||
- `just migrate up` exits 0; `psql "$DATABASE_URL" -c "SELECT 1 FROM users LIMIT 0; SELECT 1 FROM sessions LIMIT 0;"` exits 0.
|
||||
- `just migrate down` followed by `just migrate up` exits 0 (round-trip clean).
|
||||
</acceptance_criteria>
|
||||
<done>Migration applies and rolls back cleanly against the compose Postgres; tables and indexes exist as specified by D-01/D-04.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Update sqlc.yaml with citext+uuid overrides; author users.sql / sessions.sql; run sqlc generate</name>
|
||||
<read_first>
|
||||
backend/sqlc.yaml
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 10, Pitfall 3, "sqlc query file" examples)
|
||||
backend/internal/db/queries/ (currently empty)
|
||||
backend/internal/db/pool.go
|
||||
</read_first>
|
||||
<action>
|
||||
Edit `backend/sqlc.yaml`: add the `overrides:` block under `gen.go` mapping `db_type: "citext"` → `go_type: "string"` and `db_type: "uuid"` → `go_type: { import: "github.com/google/uuid", type: "UUID" }`. Per D-26 and Phase 1 sqlc-empty-queries guard pattern from `fix(01)`, ensure the empty-queries case no longer applies (these two files supply queries). Create `backend/internal/db/queries/users.sql` with two named queries: `-- name: InsertUser :one` (INSERT into users (email, password_hash) VALUES ($1, $2) RETURNING all columns) and `-- name: GetUserByEmail :one` (SELECT all columns WHERE email = $1). Create `backend/internal/db/queries/sessions.sql` with five named queries verbatim from RESEARCH "Code Examples" section: `InsertSession :exec`, `GetSessionWithUser :one` (JOIN users, WHERE s.id = $1 AND s.expires_at > now() per D-07), `DeleteSession :exec`, `DeleteSessionsByUser :exec`, `ExtendSession :exec`. Run `cd backend && sqlc generate`. Verify generated files contain `Email string` (not `pgtype.Text`) and `UserID uuid.UUID` (not `pgtype.UUID`). Add the `github.com/google/uuid` import — already in `go.mod` from Phase 1.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && sqlc generate && test -f internal/db/sqlc/users.sql.go && test -f internal/db/sqlc/sessions.sql.go && grep -q "Email string" internal/db/sqlc/users.sql.go && grep -q "uuid.UUID" internal/db/sqlc/sessions.sql.go && go build ./internal/db/...</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `backend/sqlc.yaml` contains an `overrides:` list with one entry mapping `db_type: "citext"` to `go_type: "string"` and one mapping `db_type: "uuid"` to `{ import: "github.com/google/uuid", type: "UUID" }`.
|
||||
- `backend/internal/db/queries/users.sql` exists; `grep -c "^-- name:" backend/internal/db/queries/users.sql` == 2.
|
||||
- `backend/internal/db/queries/sessions.sql` exists; `grep -c "^-- name:" backend/internal/db/queries/sessions.sql` == 5.
|
||||
- `grep -c "expires_at > now()" backend/internal/db/queries/sessions.sql` >= 1 (D-07 lazy expiry in GetSessionWithUser).
|
||||
- `cd backend && sqlc generate` exits 0.
|
||||
- Generated `internal/db/sqlc/users.sql.go` contains the literal `Email string` and `InsertUser(ctx context.Context` substrings.
|
||||
- Generated `internal/db/sqlc/sessions.sql.go` contains `uuid.UUID` and references `expires_at`.
|
||||
- `cd backend && go build ./internal/db/...` exits 0.
|
||||
</acceptance_criteria>
|
||||
<done>sqlc generates type-safe Go bindings using Go `string` for citext and `uuid.UUID` for uuid; package compiles.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Create internal/auth package skeleton + test-DB harness + env-var docs</name>
|
||||
<read_first>
|
||||
backend/internal/session/doc.go
|
||||
backend/internal/db/pool_test.go
|
||||
backend/.env.example
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Open Question 1 recommendation; Pattern 1 — TestParams; Testing Strategy)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-26)
|
||||
</read_first>
|
||||
<action>
|
||||
Create the `backend/internal/auth/` directory with:
|
||||
1. `doc.go` — package comment explaining the consolidated layout (per RESEARCH Open Question 3: password, session, ratelimit, cookie, csrf helpers all live here; the Phase 1 `internal/session/doc.go` placeholder is repurposed to a one-line forwarder or removed — planner's choice, document the choice in the doc comment).
|
||||
2. `types.go` — exported `User`, `Session` structs matching the sqlc generated row shapes (use `uuid.UUID` for IDs, `time.Time` for timestamps). Constants: `SessionCookieName = "xtablo_session"` (D-12), `SessionTTL = 30 * 24 * time.Hour` (D-09), `SessionExtendThreshold = 7 * 24 * time.Hour` (D-09). Sentinel errors: `ErrSessionNotFound`, `ErrInvalidHash`, `ErrIncompatibleVersion`. Document each constant with the D-XX decision ID that fixes its value.
|
||||
3. `testdb_test.go` — `setupTestDB(t *testing.T) (*pgxpool.Pool, func())` helper mirroring `backend/internal/db/pool_test.go`'s `os.Getenv("TEST_DATABASE_URL")` skip pattern (fall back to `DATABASE_URL` if unset is allowed). On setup: create a unique schema name (`test_<short-uuid>`), open a pool with `search_path=<schema>,public`, run goose Up against that schema using the `pressly/goose/v3` library API (import `github.com/pressly/goose/v3`), and return the pool plus a cleanup func that DROPs the schema and closes the pool. Per RESEARCH Open Question 1: this is the canonical test harness for the phase — every DB-touching auth test calls `setupTestDB(t)` and registers cleanup via `t.Cleanup`.
|
||||
4. Add `goose` library dependency: `cd backend && go get github.com/pressly/goose/v3@v3.27.1` (version already used by the just-installed CLI per the justfile).
|
||||
5. Update `backend/.env.example` to add `TEST_DATABASE_URL=postgres://xtablo:xtablo@localhost:5432/xtablo?sslmode=disable` and `SESSION_SECRET=` (with a comment `# 32 random bytes hex — generate via: openssl rand -hex 32`).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && go build ./internal/auth/... && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/auth/ -run TestSetupTestDB_Roundtrip -count=1 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Directory `backend/internal/auth/` exists with `doc.go` and `types.go`.
|
||||
- `grep -c "package auth" backend/internal/auth/doc.go` == 1.
|
||||
- `grep -E "SessionCookieName|SessionTTL|SessionExtendThreshold" backend/internal/auth/types.go | wc -l` >= 3.
|
||||
- `grep -c "ErrSessionNotFound\|ErrInvalidHash\|ErrIncompatibleVersion" backend/internal/auth/types.go` >= 3.
|
||||
- `backend/internal/auth/testdb_test.go` exists and contains `setupTestDB` and `os.Getenv("TEST_DATABASE_URL")`.
|
||||
- Include a smoke test `TestSetupTestDB_Roundtrip` in the same file that calls setupTestDB, verifies the pool pings, and asserts the `users` table is visible inside the per-test schema (`SELECT 1 FROM users LIMIT 0`).
|
||||
- `go.mod` contains `github.com/pressly/goose/v3 v3.27.1` after `go get`.
|
||||
- `cd backend && go build ./internal/auth/...` exits 0.
|
||||
- With `TEST_DATABASE_URL` set: `go test ./internal/auth/ -run TestSetupTestDB_Roundtrip -count=1` exits 0.
|
||||
- Without `TEST_DATABASE_URL` set: the same test SKIPs (does not fail).
|
||||
- `grep -c "TEST_DATABASE_URL" backend/.env.example` == 1; `grep -c "SESSION_SECRET" backend/.env.example` == 1.
|
||||
</acceptance_criteria>
|
||||
<done>internal/auth package compiles; test harness creates+drops an isolated schema and applies migrations against it; .env.example documents both new env vars.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| App ↔ Postgres | sqlc-generated queries cross this boundary; SQL injection prevented by parameterized queries |
|
||||
| Process env ↔ filesystem | `.env.example` (committed) vs `.env` (gitignored) — secrets never enter VCS |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-06 | Information disclosure | `sessions` table column design | mitigate | `id` column stores SHA-256(token) hex per D-05 — DB read leak does NOT yield live cookies. Verified by Task 2 sessions.sql schema and downstream Plan 03 session test. |
|
||||
| T-2-11 | Tampering | Migration scripts | mitigate | goose Up/Down round-trip exercised by Task 1 acceptance criteria; deterministic schema; no DDL outside migrations. |
|
||||
| T-2-12 | Information disclosure | Secrets in repo | mitigate | `.env.example` carries placeholder only; real `SESSION_SECRET` lives in `.env` (gitignored per Phase 1). Task 3 acceptance asserts the placeholder. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `just migrate up` then `just migrate down` then `just migrate up` against the local compose Postgres exits 0 for all three calls.
|
||||
- `cd backend && sqlc generate && go build ./...` exits 0.
|
||||
- `cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/auth/...` runs the roundtrip smoke test and exits 0.
|
||||
- No new column outside the D-01 / D-04 lists is introduced (grep gate in acceptance).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Schema applied; sqlc bindings compile with Go `string` for citext and `uuid.UUID` for uuid; `internal/auth` package compiles and exports the constants + types that Plans 02–07 consume.
|
||||
- Test harness is the only path DB tests in this phase use; per-test schema isolation prevents cross-test pollution.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/02-authentication/02-01-SUMMARY.md` per the GSD summary template, recording: migration filename, sqlc override decisions, package layout choice (consolidated `internal/auth` vs split — Open Question 3 outcome), goose version pinned in go.mod.
|
||||
</output>
|
||||
184
.planning/phases/02-authentication/02-02-PLAN.md
Normal file
184
.planning/phases/02-authentication/02-02-PLAN.md
Normal file
|
|
@ -0,0 +1,184 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 02
|
||||
type: tdd
|
||||
wave: 2
|
||||
depends_on: [01]
|
||||
files_modified:
|
||||
- backend/internal/auth/password.go
|
||||
- backend/internal/auth/password_test.go
|
||||
- backend/go.mod
|
||||
- backend/go.sum
|
||||
autonomous: true
|
||||
requirements: [AUTH-01]
|
||||
tags: [go, argon2id, password, crypto, tdd]
|
||||
must_haves:
|
||||
truths:
|
||||
- "auth.Hash(password, DefaultParams) returns a PHC-formatted argon2id string (D-08)"
|
||||
- "auth.Verify(phc, password) returns (true, nil) for the original password and (false, nil) for any other password — in constant time"
|
||||
- "Verify rejects malformed PHC, wrong algorithm, incompatible version with sentinel errors"
|
||||
- "auth.TestParams exists with reduced memory (8 MiB) so go test wall time stays under 5 seconds (D-26)"
|
||||
- "package init() self-tests a hash/verify round-trip and panics on regression (D-08)"
|
||||
artifacts:
|
||||
- path: backend/internal/auth/password.go
|
||||
provides: "Hash, Verify, DefaultParams (OWASP 2024), TestParams"
|
||||
contains: "argon2.IDKey"
|
||||
- path: backend/internal/auth/password_test.go
|
||||
provides: "round-trip, wrong-password, malformed-PHC, version-mismatch tests"
|
||||
key_links:
|
||||
- from: backend/internal/auth/password.go
|
||||
to: "golang.org/x/crypto/argon2"
|
||||
via: "argon2.IDKey + argon2.Version"
|
||||
pattern: "argon2\\.IDKey"
|
||||
- from: backend/internal/auth/password.go
|
||||
to: "crypto/subtle"
|
||||
via: "constant-time hash compare"
|
||||
pattern: "subtle\\.ConstantTimeCompare"
|
||||
---
|
||||
|
||||
<objective>
|
||||
TDD the argon2id password primitives: PHC-encoded Hash + constant-time Verify, with `DefaultParams` (OWASP 2024 baseline) and `TestParams` (reduced cost). Pure CPU work — zero DB, zero HTTP.
|
||||
|
||||
Purpose: every later auth task (signup, login, session creation) calls these two functions; defects here corrupt the entire phase.
|
||||
Output: `internal/auth/password.go` + tests, with `argon2id` dependency added to `go.mod`.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/02-authentication/02-01-PLAN.md
|
||||
@backend/internal/auth/types.go
|
||||
|
||||
<interfaces>
|
||||
This plan adds to package `auth`:
|
||||
|
||||
```go
|
||||
type Params struct {
|
||||
Memory uint32 // KiB
|
||||
Iterations uint32
|
||||
Parallelism uint8
|
||||
SaltLength uint32
|
||||
KeyLength uint32
|
||||
}
|
||||
|
||||
var DefaultParams = Params{Memory: 64 * 1024, Iterations: 1, Parallelism: 4, SaltLength: 16, KeyLength: 32} // OWASP 2024 / D-08
|
||||
var TestParams = Params{Memory: 8 * 1024, Iterations: 1, Parallelism: 2, SaltLength: 16, KeyLength: 32}
|
||||
|
||||
func Hash(password string, p Params) (string, error) // returns "$argon2id$v=19$m=...,t=...,p=...$<salt>$<hash>"
|
||||
func Verify(encoded, password string) (bool, error) // constant-time; (false, nil) on mismatch; sentinel error on malformed
|
||||
```
|
||||
|
||||
`ErrInvalidHash` and `ErrIncompatibleVersion` (declared in Plan 01) are returned for malformed PHC and version mismatches respectively.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 1: RED — failing tests for Hash + Verify (round-trip, wrong password, malformed PHC, version mismatch)</name>
|
||||
<read_first>
|
||||
backend/internal/auth/types.go
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 1; Pitfall 4)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-08, D-25, D-26)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- TestPassword_HashVerify: Hash(pw, TestParams) returns non-empty PHC starting with `$argon2id$v=19$m=8192,t=1,p=2$`; Verify(phc, pw) returns (true, nil).
|
||||
- TestPassword_VerifyWrong: Hash(pw, TestParams); Verify(phc, pw+"x") returns (false, nil) and DOES NOT return an error.
|
||||
- TestPassword_VerifyMalformed: Verify("not-a-phc-string", "anything") returns (false, ErrInvalidHash); Verify("$argon2id$v=19$m=8192,t=1,p=2$only-three-segments", "x") returns (false, ErrInvalidHash); Verify("$bcrypt$v=19$...", "x") returns (false, ErrInvalidHash).
|
||||
- TestPassword_VerifyVersion: a synthetic PHC string with `v=18` returns (false, ErrIncompatibleVersion).
|
||||
- TestPassword_DistinctSaltsPerCall: Hash(pw, TestParams) twice yields two different PHC strings (random salt per call).
|
||||
- TestPassword_DefaultParamsShape: DefaultParams equals OWASP 2024 baseline (`Memory == 64*1024`, `Iterations == 1`, `Parallelism == 4`, `SaltLength == 16`, `KeyLength == 32`) — guards against silent param drift.
|
||||
</behavior>
|
||||
<action>
|
||||
Create `backend/internal/auth/password_test.go` with the six tests above as table-driven where natural. Use `auth.TestParams` for all Hash calls to keep wall time low. Tests MUST fail at this step because `password.go` does not exist yet — run `go test ./internal/auth/` and confirm compile failure or test failure. Commit message: `test(02): RED — failing argon2id password tests`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && ! go test ./internal/auth/ -run TestPassword -count=1 2>&1 | grep -q "PASS:"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- File `backend/internal/auth/password_test.go` exists.
|
||||
- `grep -c "^func Test" backend/internal/auth/password_test.go` >= 6.
|
||||
- `grep -c "TestParams" backend/internal/auth/password_test.go` >= 1 (tests use reduced params).
|
||||
- `grep -c "ErrInvalidHash\|ErrIncompatibleVersion" backend/internal/auth/password_test.go` >= 2.
|
||||
- `cd backend && go test ./internal/auth/ -run TestPassword -count=1` exits NON-zero (RED state).
|
||||
- One commit on the current branch with message matching `test(02).*RED` and only `backend/internal/auth/password_test.go` in the diff.
|
||||
</acceptance_criteria>
|
||||
<done>Tests are written and failing for the right reason (missing implementation).</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 2: GREEN — implement Hash + Verify + Params + init self-test</name>
|
||||
<read_first>
|
||||
backend/internal/auth/password_test.go (created in Task 1)
|
||||
backend/internal/auth/types.go
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 1 verbatim; Pitfall 4; "Anti-Patterns")
|
||||
</read_first>
|
||||
<action>
|
||||
Add the argon2 dependency: `cd backend && go get golang.org/x/crypto@v0.51.0`. Create `backend/internal/auth/password.go` per RESEARCH Pattern 1 verbatim:
|
||||
- Imports: `crypto/rand`, `crypto/subtle`, `encoding/base64`, `errors`, `fmt`, `strings`, `golang.org/x/crypto/argon2`.
|
||||
- Types: `Params` struct with `Memory`, `Iterations`, `Parallelism`, `SaltLength`, `KeyLength` fields per the interfaces block above.
|
||||
- Vars: `DefaultParams` (OWASP 2024 — D-08) and `TestParams` (reduced).
|
||||
- `Hash(password string, p Params) (string, error)`: read salt via `rand.Read`, call `argon2.IDKey([]byte(password), salt, p.Iterations, p.Memory, p.Parallelism, p.KeyLength)`, format PHC string `$argon2id$v=%d$m=%d,t=%d,p=%d$<b64salt>$<b64hash>` using `base64.RawStdEncoding`.
|
||||
- `Verify(encoded, password string) (bool, error)`: split on `$` into 6 parts (per RESEARCH note: 6 segments because the leading `$` produces an empty first element), check `parts[1] == "argon2id"` else return `ErrInvalidHash`, parse `v=%d` and reject if `!= argon2.Version` with `ErrIncompatibleVersion`, parse params, decode salt+hash with `base64.RawStdEncoding.Strict()`, recompute via `argon2.IDKey` using `uint32(len(want))` as key length, compare with `subtle.ConstantTimeCompare(want, got) == 1`.
|
||||
- `init()` self-test (D-08): `phc, err := Hash("self-test-password-12chars", TestParams); if err != nil { panic(err) }; ok, err := Verify(phc, "self-test-password-12chars"); if err != nil || !ok { panic("argon2 self-test failed") }` — guards against param drift / build corruption.
|
||||
- Anti-pattern guard: NEVER use `bytes.Equal` for hash compare. NEVER use `sha256.New().Sum(...)` anywhere. NEVER reuse a salt across calls.
|
||||
Run tests; they must pass. Commit message: `feat(02): GREEN — argon2id Hash + Verify + self-test`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && go test ./internal/auth/ -run TestPassword -count=1 -v 2>&1 | tee /tmp/p02.log | tail -30 && grep -q "PASS" /tmp/p02.log && ! grep -q "FAIL" /tmp/p02.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- File `backend/internal/auth/password.go` exists.
|
||||
- `grep -c "func Hash" backend/internal/auth/password.go` == 1; `grep -c "func Verify" backend/internal/auth/password.go` == 1.
|
||||
- `grep -c "argon2.IDKey" backend/internal/auth/password.go` >= 2 (Hash + Verify).
|
||||
- `grep -c "subtle.ConstantTimeCompare" backend/internal/auth/password.go` >= 1.
|
||||
- `grep -c "bytes.Equal" backend/internal/auth/password.go` == 0 (anti-pattern guard).
|
||||
- `grep -c "sha256.New().Sum" backend/internal/auth/password.go` == 0 (Pitfall 6 anti-pattern guard).
|
||||
- `grep -c "func init" backend/internal/auth/password.go` == 1 (self-test).
|
||||
- `go.mod` contains `golang.org/x/crypto v0.51.0` (or higher patch in v0.51.x line).
|
||||
- `cd backend && go test ./internal/auth/ -run TestPassword -count=1` exits 0 with all 6+ tests PASS.
|
||||
- `cd backend && go test ./internal/auth/ -run TestPassword -count=1` completes in < 5 seconds wall-time (Pitfall 4 guard).
|
||||
- One commit on the branch with message matching `feat(02).*GREEN.*argon2` and diff limited to `password.go` + `go.mod` + `go.sum`.
|
||||
</acceptance_criteria>
|
||||
<done>All password tests pass; init self-test runs on every package import; reduced-cost test path keeps `go test` fast.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| In-process | Plaintext password lives in memory only between request parsing and the `Hash`/`Verify` call. Never persisted, never logged. |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-01 | Information disclosure | `password_hash` storage | mitigate | argon2id (D-08); per-password 16-byte salt; PHC string self-describes params so future cost tuning is non-breaking. Verified by TestPassword_HashVerify + TestPassword_DistinctSaltsPerCall. |
|
||||
| T-2-13 | Information disclosure | Timing side-channel on Verify | mitigate | `subtle.ConstantTimeCompare` final compare; argon2 itself is constant-time. Verified by Task 2 acceptance grep for `subtle.ConstantTimeCompare`. |
|
||||
| T-2-14 | DoS | Very long passwords | mitigate (in caller) | Length ceiling (≤128 chars per D-25) enforced by handler before reaching `Hash` — verified in Plan 04 (signup) and Plan 05 (login). This plan documents the contract via comment on `Hash`. |
|
||||
| T-2-15 | Tampering | argon2 param drift across deploys | mitigate | `init()` self-test panics on regression; `DefaultParams` shape locked by TestPassword_DefaultParamsShape. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `cd backend && go test ./internal/auth/ -run TestPassword -count=1` exits 0.
|
||||
- `cd backend && go vet ./internal/auth/...` exits 0.
|
||||
- Two commits on branch: one `test(02): RED` then one `feat(02): GREEN`.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Plaintext passwords can be hashed to PHC strings and verified back in constant time.
|
||||
- Verifier rejects malformed input, foreign algorithms, and version mismatches with the documented sentinel errors.
|
||||
- Test wall time stays under 5 seconds thanks to `TestParams` — unblocks high-frequency test runs in later plans.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/02-authentication/02-02-SUMMARY.md` recording: argon2 library version pinned, exact `DefaultParams` chosen (D-08 baseline confirmed), wall-time observed for the test suite, and any deviations from RESEARCH Pattern 1.
|
||||
</output>
|
||||
246
.planning/phases/02-authentication/02-03-PLAN.md
Normal file
246
.planning/phases/02-authentication/02-03-PLAN.md
Normal file
|
|
@ -0,0 +1,246 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 03
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on: [01]
|
||||
files_modified:
|
||||
- backend/internal/auth/session.go
|
||||
- backend/internal/auth/session_test.go
|
||||
- backend/internal/auth/cookie.go
|
||||
- backend/internal/auth/middleware.go
|
||||
- backend/internal/auth/middleware_test.go
|
||||
autonomous: true
|
||||
requirements: [AUTH-02, AUTH-03, AUTH-05]
|
||||
tags: [go, sessions, middleware, sha256, chi]
|
||||
must_haves:
|
||||
truths:
|
||||
- "auth.Store.Create issues a 32-byte crypto/rand token and stores SHA-256(token) hex in the sessions row (D-05)"
|
||||
- "auth.Store.Lookup hashes the supplied cookie, JOINs users on expires_at > now() (D-07), and returns user+session"
|
||||
- "auth.Store.Rotate deletes the old session row and creates a new one — used on every login + signup (D-10)"
|
||||
- "auth.Store.Delete hard-deletes the row (D-06)"
|
||||
- "auth.Store.MaybeExtend updates expires_at only when remaining < 7 days (D-09)"
|
||||
- "SetSessionCookie sets HttpOnly + Secure (env-gated) + SameSite=Lax + Path=/ (D-12); ClearSessionCookie uses MaxAge=-1 (D-06)"
|
||||
- "cookie value is the raw base64url token only — no extra signing layer; tampering surfaces via SHA-256 lookup miss (D-13)"
|
||||
- "ResolveSession middleware reads the cookie, looks up the session, and attaches Session+User to request context without blocking on miss"
|
||||
- "RequireAuth redirects unauth to /login (303 normally, HX-Redirect for HTMX); RedirectIfAuthed bounces authed users to / (D-23)"
|
||||
artifacts:
|
||||
- path: backend/internal/auth/session.go
|
||||
provides: "Store struct + Create/Lookup/Delete/Rotate/MaybeExtend"
|
||||
contains: "sha256.Sum256"
|
||||
- path: backend/internal/auth/cookie.go
|
||||
provides: "SetSessionCookie + ClearSessionCookie"
|
||||
- path: backend/internal/auth/middleware.go
|
||||
provides: "ResolveSession, RequireAuth, RedirectIfAuthed + context accessor Authed(ctx)"
|
||||
- path: backend/internal/auth/session_test.go
|
||||
provides: "Real-DB tests: hashed-id storage, rotate-deletes-old, lazy expiry, maybe-extend threshold"
|
||||
- path: backend/internal/auth/middleware_test.go
|
||||
provides: "ResolveSession ctx plumbing; RequireAuth 303 vs HX-Redirect; RedirectIfAuthed bounce"
|
||||
key_links:
|
||||
- from: backend/internal/auth/session.go
|
||||
to: backend/internal/db/sqlc/sessions.sql.go
|
||||
via: "Queries.InsertSession/GetSessionWithUser/DeleteSession/ExtendSession"
|
||||
pattern: "q\\.InsertSession|q\\.GetSessionWithUser"
|
||||
- from: backend/internal/auth/middleware.go
|
||||
to: "request context"
|
||||
via: "context.WithValue(sessionCtxKey, authedRequest)"
|
||||
pattern: "sessionCtxKey"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Implement the session lifecycle (token gen, SHA-256 storage, lookup, rotate, delete, maybe-extend), cookie helpers, and the three chi-compatible middlewares (ResolveSession, RequireAuth, RedirectIfAuthed). Each has a deterministic test — real Postgres for store tests, httptest for middleware tests.
|
||||
|
||||
Purpose: every authed flow in Plans 04-07 calls into this surface. Getting SHA-256 storage (Pitfall 6), rotation (Pitfall 5), and HTMX-aware redirect (Pattern 5) right here means later plans become straight-line wiring.
|
||||
Output: internal/auth/{session,cookie,middleware}.go plus tests; package still has no HTTP handlers (those land in Plans 04/05).
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/02-authentication/02-01-PLAN.md
|
||||
@backend/internal/auth/types.go
|
||||
@backend/internal/auth/testdb_test.go
|
||||
@backend/internal/db/sqlc/sessions.sql.go
|
||||
@backend/internal/web/middleware.go
|
||||
|
||||
<interfaces>
|
||||
This plan adds to package auth:
|
||||
|
||||
- session.go: type Store struct { q *sqlc.Queries; now func() time.Time }
|
||||
- NewStore(q *sqlc.Queries) *Store
|
||||
- (s *Store) Create(ctx, userID) (cookieValue string, expiresAt time.Time, err error) — D-05/D-09
|
||||
- (s *Store) Lookup(ctx, cookieValue) (*Session, *User, error)
|
||||
- (s *Store) Delete(ctx, id) error
|
||||
- (s *Store) Rotate(ctx, oldID, userID) (string, time.Time, error) — D-10
|
||||
- (s *Store) MaybeExtend(ctx, id, expiresAt) error — D-09 (extends only when remaining < SessionExtendThreshold)
|
||||
|
||||
- cookie.go:
|
||||
- SetSessionCookie(w http.ResponseWriter, value string, expiresAt time.Time, secure bool)
|
||||
- ClearSessionCookie(w http.ResponseWriter, secure bool)
|
||||
|
||||
- middleware.go:
|
||||
- ResolveSession(store *Store) func(http.Handler) http.Handler — never blocks
|
||||
- RequireAuth(next http.Handler) http.Handler — 303 or HX-Redirect to /login
|
||||
- RedirectIfAuthed(next http.Handler) http.Handler — 303 or HX-Redirect to /
|
||||
- Authed(ctx) (*Session, *User, bool) — context accessor
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Session store + cookie helpers (real-DB TDD)</name>
|
||||
<read_first>
|
||||
backend/internal/auth/types.go
|
||||
backend/internal/auth/testdb_test.go
|
||||
backend/internal/db/sqlc/sessions.sql.go
|
||||
backend/internal/db/sqlc/users.sql.go
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 2, Pattern 3, Pitfall 6, Pitfall 5)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-05, D-06, D-09, D-10, D-12)
|
||||
</read_first>
|
||||
<behavior>
|
||||
Real-DB tests (skip if TEST_DATABASE_URL unset):
|
||||
- TestSession_StoresHashedID: Create returns cookie value v; the DB row id equals hex(sha256(base64url_decode(v))) — NOT the raw token. Pitfall 6 guard.
|
||||
- TestSession_LookupRoundtrip: insert a user, Create a session, Lookup(cookieValue) returns same userID + non-zero session.ID.
|
||||
- TestSession_LookupRejectsExpired: insert a session manually with expires_at = now()-1h via raw SQL; Lookup returns ErrSessionNotFound (D-07).
|
||||
- TestSession_RotateDeletesOld: Create session A; Rotate(A.id, userID) returns a different cookie value; SELECT COUNT(*) WHERE id=A.id == 0 (Pitfall 5 guard); new row exists.
|
||||
- TestSession_DeleteRemovesRow: Create, Delete, Lookup returns ErrSessionNotFound.
|
||||
- TestSession_MaybeExtend_NoOp: row with expires_at = now()+29d (above threshold) — MaybeExtend MUST NOT change expires_at (within +/-1s).
|
||||
- TestSession_MaybeExtend_Extends: row with expires_at = now()+1d — MaybeExtend updates expires_at; assert new > old and within +/-5s of now()+30d.
|
||||
|
||||
Unit tests (no DB):
|
||||
- TestCookie_SetAttributes: SetSessionCookie(w, "value", t+30d, true) — HttpOnly=true, Secure=true, SameSite=Lax, Path=/, MaxAge ~ 30*24*3600.
|
||||
- TestCookie_ClearUsesMaxAgeMinus1: ClearSessionCookie emits Set-Cookie with Max-Age=-1 (NOT 0). Per RESEARCH Pattern 3.
|
||||
- TestCookie_SecureGatedByEnv: secure=false produces a cookie WITHOUT the Secure attribute.
|
||||
</behavior>
|
||||
<action>
|
||||
Create backend/internal/auth/session.go:
|
||||
- Store struct with q *sqlc.Queries and now func() time.Time (default time.Now).
|
||||
- NewStore(q *sqlc.Queries) *Store returns store with now: time.Now.
|
||||
- Create(ctx, userID): raw := make([]byte, 32); rand.Read(raw); cookieValue := base64.RawURLEncoding.EncodeToString(raw); sum := sha256.Sum256(raw); id := hex.EncodeToString(sum[:]); expiresAt := s.now().Add(SessionTTL); call q.InsertSession with InsertSessionParams{ID: id, UserID: userID, ExpiresAt: expiresAt}; return cookieValue, expiresAt.
|
||||
- Lookup(ctx, cookieValue): decode base64url; if err or len != 32 return ErrSessionNotFound; sum := sha256.Sum256(raw); id := hex(sum); call q.GetSessionWithUser(ctx, id); map pgx.ErrNoRows -> ErrSessionNotFound; build *Session + *User from row columns.
|
||||
- Delete(ctx, id): q.DeleteSession(ctx, id).
|
||||
- Rotate(ctx, oldID, userID): if oldID != "" then _ = q.DeleteSession(ctx, oldID) (best-effort, ignore err); return Create(ctx, userID).
|
||||
- MaybeExtend(ctx, id, expiresAt): if time.Until(expiresAt) (using s.now as the base — implement as expiresAt.Sub(s.now()) >= SessionExtendThreshold) then return nil; else q.ExtendSession with ExtendSessionParams{ID: id, ExpiresAt: s.now().Add(SessionTTL)}.
|
||||
|
||||
Create backend/internal/auth/cookie.go:
|
||||
- SetSessionCookie(w, value, expiresAt, secure): http.SetCookie with Name=SessionCookieName, Value=value, Path="/", Expires=expiresAt, MaxAge=int(time.Until(expiresAt).Seconds()), HttpOnly=true, Secure=secure, SameSite=http.SameSiteLaxMode.
|
||||
- ClearSessionCookie(w, secure): http.SetCookie with Name=SessionCookieName, Value="", Path="/", Expires=time.Unix(0,0), MaxAge=-1, HttpOnly=true, Secure=secure, SameSite=http.SameSiteLaxMode.
|
||||
|
||||
Create backend/internal/auth/session_test.go with the seven DB tests and three cookie tests above. DB tests use setupTestDB(t) from Plan 01; for raw INSERT in the expired-row + MaybeExtend setup tests, use the pool directly: pool.Exec(ctx, "INSERT INTO sessions (id, user_id, expires_at) VALUES ($1, $2, $3)", ...). Inject store.now := func() time.Time { return fixedNow } in the MaybeExtend tests to remove flakiness.
|
||||
|
||||
Anti-pattern guards: NEVER use sha256.New().Sum(raw) (Pitfall 6) — use sha256.Sum256. NEVER use MaxAge: 0 to clear a cookie. NEVER skip DeleteSession on Rotate (Pitfall 5).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/auth/ -run "TestSession|TestCookie" -count=1 -v 2>&1 | tee /tmp/p03t1.log | tail -40 && grep -q "PASS" /tmp/p03t1.log && ! grep -E "^--- FAIL" /tmp/p03t1.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Files session.go, cookie.go, session_test.go exist in backend/internal/auth/.
|
||||
- grep -c "sha256.Sum256" backend/internal/auth/session.go >= 2 (Create + Lookup).
|
||||
- grep -c "sha256.New().Sum" backend/internal/auth/session.go == 0 (Pitfall 6 guard).
|
||||
- grep -c "MaxAge: *-1\|MaxAge:-1" backend/internal/auth/cookie.go == 1.
|
||||
- grep -c "MaxAge: *0[^0-9]" backend/internal/auth/cookie.go == 0 (anti-pattern guard).
|
||||
- grep -c "DeleteSession" backend/internal/auth/session.go >= 2 (one in Delete, one in Rotate — Pitfall 5).
|
||||
- grep -c "SameSiteLaxMode" backend/internal/auth/cookie.go == 2 (set + clear).
|
||||
- cd backend && go build ./internal/auth/... exits 0.
|
||||
- With TEST_DATABASE_URL set: cd backend && go test ./internal/auth/ -run "TestSession|TestCookie" -count=1 exits 0.
|
||||
- Without TEST_DATABASE_URL set: the seven TestSession_* tests SKIP (do not fail); cookie tests still PASS.
|
||||
</acceptance_criteria>
|
||||
<done>Session lifecycle works end-to-end against a real Postgres; cookie helpers emit attributes that match D-12 exactly.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: ResolveSession + RequireAuth + RedirectIfAuthed middleware (HTMX-aware)</name>
|
||||
<read_first>
|
||||
backend/internal/auth/session.go (created in Task 1)
|
||||
backend/internal/web/middleware.go (Phase 1 RequestIDMiddleware pattern — context.WithValue + typed accessor)
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 4, Pattern 5; Pitfall 9 — 303 vs 302)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-23, D-24)
|
||||
</read_first>
|
||||
<behavior>
|
||||
Tests in backend/internal/auth/middleware_test.go (DB-touching ones use setupTestDB; pure routing ones use a fake Store):
|
||||
- TestResolveSession_NoCookie: request with no cookie -> handler runs; Authed(ctx) returns (_, _, false).
|
||||
- TestResolveSession_InvalidCookie: cookie with random garbage -> handler runs; Authed(ctx) returns false (Lookup returned ErrSessionNotFound).
|
||||
- TestResolveSession_ValidCookie: with a Create'd session cookie -> handler runs; Authed(ctx) returns (*Session, *User, true) with matching userID. Real-DB test.
|
||||
- TestRequireAuth_303WhenUnauth: no cookie, GET /protected -> 303 with Location: /login.
|
||||
- TestRequireAuth_HXRedirectWhenUnauth: header HX-Request: true -> 200 with HX-Redirect: /login header (NOT a 303). Per Pattern 5.
|
||||
- TestRequireAuth_PassesWhenAuth: with valid session in ctx -> next handler runs and returns 200.
|
||||
- TestRedirectIfAuthed_BouncesWhenAuth: valid session -> 303 with Location: / OR HX-Redirect: / when HX-Request: true.
|
||||
- TestRedirectIfAuthed_PassesWhenUnauth: no session -> next handler runs.
|
||||
</behavior>
|
||||
<action>
|
||||
Create backend/internal/auth/middleware.go:
|
||||
- Unexported context key: type sessionCtxKey struct{}; var sessionKey = sessionCtxKey{}.
|
||||
- type authed struct { Session *Session; User *User }.
|
||||
- Authed(ctx context.Context) (*Session, *User, bool): a, ok := ctx.Value(sessionKey).(*authed); if !ok return nil, nil, false; return a.Session, a.User, true.
|
||||
- ResolveSession(store *Store) func(http.Handler) http.Handler: read r.Cookie(SessionCookieName); on err or empty value -> next.ServeHTTP without ctx mutation. Else store.Lookup -> on err -> next without mutation (do NOT clear the cookie here; handler/RequireAuth decides). On success: _ = store.MaybeExtend(...) (best-effort, log err via slog.Default()); ctx2 := context.WithValue(r.Context(), sessionKey, &authed{Session: sess, User: u}); next.ServeHTTP(w, r.WithContext(ctx2)).
|
||||
- RequireAuth(next): if _, _, ok := Authed(r.Context()); !ok -> redirect(w, r, "/login"); return. Else next.ServeHTTP(w, r).
|
||||
- RedirectIfAuthed(next): if _, _, ok := Authed(r.Context()); ok -> redirect(w, r, "/"); return. Else next.ServeHTTP(w, r).
|
||||
- redirect(w, r, target): if r.Header.Get("HX-Request") == "true" then w.Header().Set("HX-Redirect", target); w.WriteHeader(http.StatusOK); return. Else http.Redirect(w, r, target, http.StatusSeeOther). 303 mandated (Pitfall 9).
|
||||
|
||||
Create backend/internal/auth/middleware_test.go with the eight tests above. For the routing-only tests, build a fake Store by injecting a Store with q nil and stub via a function-pointer field is not viable — instead, drive ResolveSession with real DB (use setupTestDB + Create a session). For RequireAuth / RedirectIfAuthed pure tests, build the ctx directly: ctx := context.WithValue(context.Background(), sessionKey, &authed{Session: &Session{ID: "x"}, User: &User{ID: uuid.New()}}). Wire via chi.NewRouter() + r.Use(...) + r.Get("/protected", ...).
|
||||
|
||||
Anti-pattern guards: do NOT use http.StatusFound (302) (Pitfall 9). Do NOT clear the cookie inside ResolveSession on lookup failure (Pattern 4 note). Do NOT make ResolveSession blocking — it always calls next.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/auth/ -run "TestResolveSession|TestRequireAuth|TestRedirectIfAuthed" -count=1 -v 2>&1 | tee /tmp/p03t2.log | tail -40 && grep -q "PASS" /tmp/p03t2.log && ! grep -E "^--- FAIL" /tmp/p03t2.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Files middleware.go and middleware_test.go exist in backend/internal/auth/.
|
||||
- grep -c "func ResolveSession" backend/internal/auth/middleware.go == 1.
|
||||
- grep -c "func RequireAuth\|func RedirectIfAuthed" backend/internal/auth/middleware.go == 2.
|
||||
- grep -c "func Authed" backend/internal/auth/middleware.go == 1.
|
||||
- grep -c "HX-Redirect" backend/internal/auth/middleware.go >= 1.
|
||||
- grep -c "http.StatusSeeOther\|StatusSeeOther" backend/internal/auth/middleware.go >= 1.
|
||||
- grep -c "http.StatusFound\|StatusFound" backend/internal/auth/middleware.go == 0 (Pitfall 9 anti-pattern guard).
|
||||
- grep -c "MaybeExtend" backend/internal/auth/middleware.go >= 1 (ResolveSession calls it on hit).
|
||||
- cd backend && go build ./internal/auth/... exits 0.
|
||||
- With TEST_DATABASE_URL set: middleware tests pass.
|
||||
- Without TEST_DATABASE_URL: DB-touching subtests SKIP; pure ctx tests PASS.
|
||||
</acceptance_criteria>
|
||||
<done>Middleware chain is ready for plug-in by Plan 04 / 05 / 06; HTMX-vs-303 redirect choice is centralized in one helper.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Browser <-> server (cookie) | Opaque token in cookie; SHA-256 of token in DB — DB read leak does not yield live cookies (D-05) |
|
||||
| Request scope | ResolveSession is the only writer of the session ctx key; downstream handlers consume via Authed(ctx) |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-04 | Spoofing | Session fixation | mitigate | Store.Rotate deletes the old session row before creating the new one (D-10). Verified by TestSession_RotateDeletesOld. |
|
||||
| T-2-05 | Information disclosure | Cookie theft via JS / non-TLS | mitigate | HttpOnly + Secure (env-gated) + SameSite=Lax (D-12) in SetSessionCookie. Verified by TestCookie_SetAttributes. |
|
||||
| T-2-06 | Information disclosure | Token leak via DB read | mitigate | DB id is hex(sha256(token)); raw token never persists. Verified by TestSession_StoresHashedID. |
|
||||
| T-2-10 | Tampering / Elevation | Broken access control | mitigate | RequireAuth gates protected routes; redirects unauth to /login. Verified by TestRequireAuth_303WhenUnauth and TestRequireAuth_PassesWhenAuth. |
|
||||
| T-2-16 | Information disclosure | Timing on session lookup | accept | Single SHA-256 + indexed PK SELECT; DB-latency dominates. No additional mitigation in v1. |
|
||||
| T-2-17 | DoS | Garbage cookies forcing DB hits | accept | One indexed SELECT per request; rate limit on /login (Plan 05) covers the attacker entrypoint that creates volume. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/auth/... exits 0 (combined with Plan 02 password tests).
|
||||
- cd backend && go vet ./internal/auth/... exits 0.
|
||||
- No use of http.StatusFound (302), no use of bytes.Equal for hash compare, no use of sha256.New().Sum, no use of MaxAge:0 — all asserted by grep gates.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- A real Postgres round-trip Create -> Lookup -> Delete works.
|
||||
- Rotate replaces the old session row (fixation defense).
|
||||
- MaybeExtend updates expires_at exactly once per ~23 days (sliding window without write storm).
|
||||
- Middleware integrates cleanly with chi r.Use and chi route groups in Plan 04+.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create .planning/phases/02-authentication/02-03-SUMMARY.md recording: chosen cookie name (SessionCookieName value), how MaybeExtend handles a clock injected for tests, observed wall time for the test suite, any deviations from RESEARCH Pattern 2/4/5.
|
||||
</output>
|
||||
277
.planning/phases/02-authentication/02-04-PLAN.md
Normal file
277
.planning/phases/02-authentication/02-04-PLAN.md
Normal file
|
|
@ -0,0 +1,277 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 04
|
||||
type: execute
|
||||
wave: 3
|
||||
depends_on: [01, 02, 03]
|
||||
files_modified:
|
||||
- backend/templates/auth_signup.templ
|
||||
- backend/templates/auth_form_errors.templ
|
||||
- backend/internal/web/handlers_auth.go
|
||||
- backend/internal/web/handlers_auth_test.go
|
||||
- backend/internal/web/router.go
|
||||
- backend/cmd/web/main.go
|
||||
autonomous: true
|
||||
requirements: [AUTH-01, AUTH-03, AUTH-05]
|
||||
tags: [go, htmx, signup, vertical-slice, templ]
|
||||
must_haves:
|
||||
truths:
|
||||
- "GET /signup renders a full page with an email + password form (D-19)"
|
||||
- "POST /signup with valid input inserts a user with argon2id-hashed password, creates a session, sets the session cookie, and redirects to / (D-11)"
|
||||
- "POST /signup with invalid input renders field-specific errors (email parse failure, password length 12-128) as an HTMX fragment when HX-Request: true, full page otherwise (D-19, D-25)"
|
||||
- "POST /signup with a duplicate email renders 'Email is already in use' (specific is OK on signup per CONTEXT.md specifics)"
|
||||
- "Authed users hitting GET /signup are redirected to / (D-23 RedirectIfAuthed)"
|
||||
- "Router wires ResolveSession into the chi middleware stack in the locked order: RequestID -> RealIP -> SlogLogger -> Recoverer -> ResolveSession (D-24)"
|
||||
artifacts:
|
||||
- path: backend/templates/auth_signup.templ
|
||||
provides: "SignupPage full templ and SignupFormFragment HTMX fragment"
|
||||
contains: "templ SignupPage"
|
||||
- path: backend/templates/auth_form_errors.templ
|
||||
provides: "Shared form-error rendering primitives used by signup + login"
|
||||
- path: backend/internal/web/handlers_auth.go
|
||||
provides: "SignupPageHandler + SignupPostHandler (auto-login + redirect)"
|
||||
- path: backend/internal/web/router.go
|
||||
provides: "Updated NewRouter signature accepting *auth.Store and env; mounts ResolveSession + RedirectIfAuthed + signup routes"
|
||||
key_links:
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/auth/session.go
|
||||
via: "auth.Store.Create on successful signup"
|
||||
pattern: "store\\.Create\\("
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/db/sqlc/users.sql.go
|
||||
via: "Queries.InsertUser with normalized email + argon2id hash"
|
||||
pattern: "InsertUser"
|
||||
- from: backend/internal/web/router.go
|
||||
to: backend/internal/auth/middleware.go
|
||||
via: "r.Use(auth.ResolveSession(store)) before route groups (D-24)"
|
||||
pattern: "ResolveSession"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Deliver the SIGNUP vertical slice end-to-end: templ form -> POST handler -> validate -> argon2 hash -> InsertUser -> Store.Create -> set cookie -> redirect to /. Wire ResolveSession + the signup routes into the chi router. A user can now create an account and is logged in afterwards.
|
||||
|
||||
Purpose: first user-visible behavior of Phase 2. After this plan ships, a real human can sign up via http://localhost:8080/signup. Login is still missing (Plan 05) and home page is still public (Plan 06), but signup -> session -> redirect works.
|
||||
Output: working signup flow with real Postgres tests.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/02-authentication/02-01-PLAN.md
|
||||
@.planning/phases/02-authentication/02-02-PLAN.md
|
||||
@.planning/phases/02-authentication/02-03-PLAN.md
|
||||
@backend/templates/layout.templ
|
||||
@backend/internal/web/ui/button.templ
|
||||
@backend/internal/web/ui/card.templ
|
||||
@backend/internal/web/router.go
|
||||
@backend/internal/web/handlers.go
|
||||
@backend/cmd/web/main.go
|
||||
|
||||
<interfaces>
|
||||
Templates (new):
|
||||
- SignupPage(formValues SignupForm, errors SignupErrors) — full page wrapped in Layout("Sign up").
|
||||
- SignupFormFragment(formValues SignupForm, errors SignupErrors) — just the form, used for HTMX swaps with `hx-target="#signup-form" hx-swap="outerHTML"`.
|
||||
|
||||
Handler types (in handlers_auth.go):
|
||||
```
|
||||
type SignupForm struct { Email, Password string }
|
||||
type SignupErrors struct { Email, Password, General string }
|
||||
```
|
||||
|
||||
Handler constructors:
|
||||
- SignupPageHandler() http.HandlerFunc — renders SignupPage with empty form.
|
||||
- SignupPostHandler(deps AuthDeps) http.HandlerFunc — validates, hashes, inserts, creates session, redirects.
|
||||
|
||||
AuthDeps (defined in handlers_auth.go):
|
||||
```
|
||||
type AuthDeps struct {
|
||||
Queries *sqlc.Queries
|
||||
Store *auth.Store
|
||||
Secure bool // env != "dev"
|
||||
}
|
||||
```
|
||||
|
||||
Router signature change:
|
||||
- NewRouter(pinger Pinger, staticDir string, deps AuthDeps) http.Handler.
|
||||
Phase 1 callers in cmd/web/main.go and existing tests must be updated to pass deps; in tests, a zero AuthDeps is acceptable because Phase 1 routes do not touch it.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Signup templates (full page + HTMX fragment) and a render smoke test</name>
|
||||
<read_first>
|
||||
backend/templates/layout.templ
|
||||
backend/templates/index.templ
|
||||
backend/internal/web/ui/button.templ
|
||||
backend/internal/web/ui/card.templ
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 9; "Templ helpers" example)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-19, D-25)
|
||||
</read_first>
|
||||
<action>
|
||||
Create backend/templates/auth_form_errors.templ exporting:
|
||||
- templ FieldError(msg string) — renders nothing if msg == "", else a small <p class="mt-1 text-sm text-red-700">{msg}</p>.
|
||||
- templ GeneralError(msg string) — renders nothing if msg == "", else a banner above the form.
|
||||
|
||||
Create backend/templates/auth_signup.templ:
|
||||
- Package templates. Import backend/internal/web/ui.
|
||||
- templ SignupPage(form SignupForm, errs SignupErrors) wrapped in @Layout("Sign up"). Use @ui.Card(nil) container. Heading "Create your account".
|
||||
- templ SignupFormFragment(form SignupForm, errs SignupErrors) — the <form id="signup-form" method="POST" action="/signup" hx-post="/signup" hx-target="#signup-form" hx-swap="outerHTML"> with two labeled inputs (email, password) and an @ui.Button submit. Email input value preserves form.Email on re-render; password is never echoed back. Each field's @FieldError is the next sibling.
|
||||
- Forms include a placeholder comment `<!-- CSRF field added in Plan 07 -->` (no actual csrf rendering yet; Plan 07 wires gorilla/csrf).
|
||||
- SignupPage delegates the inner form section to @SignupFormFragment(form, errs) so the same component is reused for HTMX swaps.
|
||||
|
||||
Define SignupForm and SignupErrors in backend/internal/web/handlers_auth.go (created in Task 2) so the templ files can import them via type aliases — OR define them in a new package backend/internal/web/forms/signup.go. Recommendation: define in templates package itself (backend/templates/auth_signup.templ can declare the types in a sibling Go file backend/templates/auth_forms.go) to avoid an import cycle between templates and internal/web. Document the choice in the SUMMARY.
|
||||
|
||||
Run `cd backend && templ generate` and `go build ./templates/...`.
|
||||
|
||||
Add a render smoke test backend/templates/auth_signup_test.go:
|
||||
- TestSignupPage_RendersForm: Render SignupPage(SignupForm{Email:"x@y.z"}, SignupErrors{}) to a bytes.Buffer; assert body contains name="email", name="password", action="/signup", hx-post="/signup", and value="x@y.z" (email roundtrips).
|
||||
- TestSignupFormFragment_RendersErrors: Render with errs.Password="Password must be 12-128 characters"; assert body contains the literal error message and does NOT contain a <html> tag (fragment, not full page).
|
||||
- TestSignupPage_DoesNotEchoPassword: Render with form.Password="hunter2hunter2"; assert body does NOT contain "hunter2".
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && templ generate && go build ./templates/... && go test ./templates/ -run TestSignup -count=1 -v 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Files auth_signup.templ, auth_form_errors.templ exist in backend/templates/.
|
||||
- SignupForm and SignupErrors types are defined exactly once (in templates package or in a forms package referenced by both templ and handler — planner's call; SUMMARY records it).
|
||||
- grep -c "templ SignupPage\|templ SignupFormFragment" backend/templates/auth_signup.templ == 2.
|
||||
- grep -c "hx-post=\"/signup\"" backend/templates/auth_signup.templ >= 1.
|
||||
- grep -c "hx-target=\"#signup-form\"" backend/templates/auth_signup.templ >= 1.
|
||||
- cd backend && templ generate exits 0; auth_signup_templ.go is produced.
|
||||
- cd backend && go test ./templates/ -run TestSignup -count=1 exits 0 with the three smoke tests PASS.
|
||||
- Password roundtrip test confirms password is NOT echoed back in any rendered HTML.
|
||||
</acceptance_criteria>
|
||||
<done>templ pages compile and render; types are agreed across templ + handler boundary; password never leaks back to the client on re-render.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: SignupPostHandler + router wiring + cmd/web main update + integration tests</name>
|
||||
<read_first>
|
||||
backend/templates/auth_signup.templ (and generated _templ.go)
|
||||
backend/internal/auth/password.go
|
||||
backend/internal/auth/session.go
|
||||
backend/internal/auth/middleware.go
|
||||
backend/internal/auth/cookie.go
|
||||
backend/internal/db/sqlc/users.sql.go
|
||||
backend/internal/web/router.go
|
||||
backend/internal/web/handlers.go
|
||||
backend/cmd/web/main.go
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 6, Pattern 9, Handler skeleton; Pitfall 1 already mitigated since CSRF is Plan 07; Pitfall 4 — argon2 cost in tests)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-11, D-19, D-20, D-23, D-24, D-25)
|
||||
backend/internal/auth/password.go (auth.TestParams — reduced-cost params for pre-seeded rows)
|
||||
</read_first>
|
||||
<behavior>
|
||||
Handler tests in backend/internal/web/handlers_auth_test.go (use setupTestDB pattern from Plan 01 — copy a thin wrapper into internal/web/testdb_test.go that delegates to the auth helper or imports it):
|
||||
- TestSignup_Success: POST /signup with email=alice@example.com password=correct-horse-12 -> 303 with Location: / (or 200 with HX-Redirect: / when HX-Request: true). Set-Cookie header present with name = SessionCookieName, HttpOnly, SameSite=Lax. SELECT FROM users WHERE email='alice@example.com' returns one row; password_hash starts with "$argon2id$". SELECT COUNT(*) FROM sessions WHERE user_id = users.id == 1.
|
||||
- TestSignup_Success_HTMX: same with HX-Request: true header -> 200 + HX-Redirect: /.
|
||||
- TestSignup_InvalidEmail: POST with email="not-an-email" -> 422 (or 200 — planner picks; document) with body containing "valid email" error string; no user row inserted.
|
||||
- TestSignup_PasswordTooShort: password length 11 -> 422 with body containing "12" (the length boundary); no user row inserted.
|
||||
- TestSignup_PasswordTooLong: password length 129 -> 422 with body referencing "128"; no user row inserted.
|
||||
- TestSignup_DuplicateEmail: pre-insert a user; POST with same email -> 200/422 with body containing "already in use" or equivalent; no second user inserted; pgx unique-violation correctly trapped via errors.As against *pgconn.PgError code "23505".
|
||||
- TestSignup_EmailNormalized: POST email=" Alice@Example.COM " -> user row stored email is the original case (citext handles case), and downstream lookup via "alice@example.com" finds it. Verify email is trimmed (no leading/trailing whitespace) in the stored value.
|
||||
- TestSignup_AlreadyAuthedBouncesHome: GET /signup with a valid session cookie set -> 303 Location: / (RedirectIfAuthed). Real-DB test.
|
||||
|
||||
**Argon2 wall-time mitigation (W4):** Production code paths in this handler call `auth.Hash` with `DefaultParams` (64 MiB, ~250ms per call). Tests that require a pre-existing user row (TestSignup_DuplicateEmail, TestSignup_AlreadyAuthedBouncesHome, and any future test pre-seeding users) MUST pre-seed via a direct sqlc `InsertUser` call using a hash precomputed once with `auth.Hash(pw, auth.TestParams)` — NOT by exercising the production handler twice. The handler's hot path is unchanged; only the test setup avoids redundant 250ms hashes. Do NOT introduce an `AuthDeps.HashParams` knob — production code stays on `DefaultParams` unconditionally. Target wall time: `go test ./internal/web -run TestSignup_` completes in ≤30s.
|
||||
</behavior>
|
||||
<action>
|
||||
Create backend/internal/web/handlers_auth.go:
|
||||
- package web. Imports: net/http, net/mail, strings, errors, backend/internal/auth, backend/internal/db/sqlc, backend/templates, github.com/jackc/pgx/v5/pgconn.
|
||||
- AuthDeps struct { Queries *sqlc.Queries; Store *auth.Store; Secure bool }.
|
||||
- SignupPageHandler() http.HandlerFunc: renders templates.SignupPage(empty form, empty errors).
|
||||
- SignupPostHandler(deps AuthDeps) http.HandlerFunc:
|
||||
1. email := strings.TrimSpace(r.PostFormValue("email")); password := r.PostFormValue("password"). Do NOT lowercase before display, but DO normalize (lowercase) before insert per RESEARCH "Email normalization".
|
||||
2. validate: if _, err := mail.ParseAddress(email); err != nil -> errs.Email = "Enter a valid email address". if len(password) < 12 -> errs.Password = "Password must be at least 12 characters". if len(password) > 128 -> errs.Password = "Password must be at most 128 characters". (D-25, V5)
|
||||
3. if any err: renderSignupError(w, r, form, errs, http.StatusUnprocessableEntity); return.
|
||||
4. normalized := strings.ToLower(email). hash, err := auth.Hash(password, auth.DefaultParams); if err != nil -> http.Error(w, "internal", 500); return.
|
||||
5. user, err := deps.Queries.InsertUser(ctx, sqlc.InsertUserParams{Email: normalized, PasswordHash: hash}). If err: check errors.As(&pgErr) && pgErr.Code == "23505" -> errs.Email = "That email is already in use." -> renderSignupError 422 (specific error OK per CONTEXT.md "Specific Ideas"). Other errors -> 500.
|
||||
6. cookieValue, expiresAt, err := deps.Store.Create(ctx, user.ID). 500 on error.
|
||||
7. auth.SetSessionCookie(w, cookieValue, expiresAt, deps.Secure) — the cookie helpers are exported by Plan 03 (SetSessionCookie / ClearSessionCookie); no rename needed in this plan.
|
||||
8. if r.Header.Get("HX-Request") == "true" -> w.Header().Set("HX-Redirect", "/"); w.WriteHeader(http.StatusOK); return. Else http.Redirect(w, r, "/", http.StatusSeeOther).
|
||||
- renderSignupError(w, r, form, errs, status int) helper: w.WriteHeader(status); if HX-Request: render SignupFormFragment, else render SignupPage.
|
||||
|
||||
Update backend/internal/web/router.go:
|
||||
- Change signature: NewRouter(pinger Pinger, staticDir string, deps AuthDeps) http.Handler.
|
||||
- Insert r.Use(auth.ResolveSession(deps.Store)) AFTER chimw.Recoverer and BEFORE any route declarations. Order is now: RequestIDMiddleware -> RealIP -> SlogLogger -> Recoverer -> ResolveSession (D-24). NOTE: csrf.Protect is added in Plan 07.
|
||||
- Mount /signup routes:
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(auth.RedirectIfAuthed)
|
||||
r.Get("/signup", SignupPageHandler())
|
||||
})
|
||||
r.Post("/signup", SignupPostHandler(deps))
|
||||
- Keep existing public routes (/, /healthz, /demo/time, /static/*) unchanged. (/ becomes protected in Plan 06.)
|
||||
|
||||
Update backend/cmd/web/main.go:
|
||||
- After db.NewPool, build deps: q := sqlc.New(pool); store := auth.NewStore(q); secure := env != "development" && env != "dev". Pass AuthDeps{Queries: q, Store: store, Secure: secure} to NewRouter.
|
||||
|
||||
Update backend/internal/web/handlers_test.go (Phase 1 tests):
|
||||
- The four existing tests pass through NewRouter with the new signature. Construct AuthDeps{} (zero value — Queries/Store nil) for Phase 1 route tests; those routes don't touch deps. If ResolveSession is unconditionally added, ensure it tolerates a nil Store on the "no cookie present" path (which it does — Lookup is only called when a cookie exists). Add a guard `if store == nil { return func(next http.Handler) http.Handler { return next } }` at the top of ResolveSession to keep the Phase 1 tests working without a real store. Document this fallback in the Plan 03 doc.go (or extend it now).
|
||||
|
||||
Write backend/internal/web/handlers_auth_test.go with the eight tests above. Reuse setupTestDB from auth package by exposing it as a small test-only helper or duplicating ~20 LOC; planner chooses (record in SUMMARY). For each test build NewRouter(stubPinger{}, "./static", AuthDeps{Queries: q, Store: store, Secure: false}) and exercise httptest.NewServer / httptest.NewRecorder.
|
||||
|
||||
Anti-pattern guards: do NOT log the email on validation errors (V7 / Pitfall: avoid email-in-logs). Do NOT echo the password back to templates. Do NOT use 302 for the success redirect (Pitfall 9). Do NOT call Hash before length validation (T-2-14 DoS guard).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && templ generate && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/web/... ./internal/auth/... ./templates/... -count=1 2>&1 | tee /tmp/p04t2.log | tail -30 && grep -q "ok " /tmp/p04t2.log && ! grep -E "^FAIL|^--- FAIL" /tmp/p04t2.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- handlers_auth.go exists; grep -c "func SignupPostHandler\|func SignupPageHandler" == 2.
|
||||
- handlers_auth.go contains the literal substring `mail.ParseAddress` and `auth.Hash` and `deps.Store.Create` and `auth.SetSessionCookie`.
|
||||
- handlers_auth.go does NOT contain `slog.*email` patterns referencing the request email (grep gate: grep -E "slog.*\"email\".*email|Info.*email" backend/internal/web/handlers_auth.go | grep -v "// " | wc -l == 0).
|
||||
- router.go: grep -c "auth.ResolveSession" == 1; grep -c "auth.RedirectIfAuthed" == 1; grep -c "/signup" >= 2 (GET and POST).
|
||||
- The middleware order in router.go (top-to-bottom r.Use calls) is exactly: RequestIDMiddleware, chimw.RealIP, SlogLoggerMiddleware, chimw.Recoverer, auth.ResolveSession. Verified by inspection (acceptance criterion documented for checker).
|
||||
- cmd/web/main.go builds AuthDeps and passes it to NewRouter; grep -c "AuthDeps{" backend/cmd/web/main.go >= 1.
|
||||
- cd backend && templ generate exits 0.
|
||||
- cd backend && go build ./... exits 0.
|
||||
- cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/web/ -run TestSignup -count=1 exits 0 with all eight TestSignup_* tests PASS.
|
||||
- cd backend && go test ./internal/web/ -run "TestHealthz|TestIndex|TestDemoTime|TestRequestID|TestSlog" -count=1 exits 0 (Phase 1 tests still pass with new NewRouter signature).
|
||||
- No Verify(false) hash compare anywhere (anti-pattern guard from Plan 02 still holds).
|
||||
</acceptance_criteria>
|
||||
<done>Signup works end-to-end against a real Postgres; an authed user bouncing off /signup is verified; Phase 1 tests still pass; cmd/web builds and runs.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Client form -> handler | POST body crosses into the trusted zone; validated by mail.ParseAddress + length check BEFORE argon2 (DoS guard) |
|
||||
| Handler -> Postgres | InsertUser uses parameterized sqlc query; unique constraint enforces email uniqueness atomically |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-01 | Information disclosure | Password storage on signup | mitigate | auth.Hash(password, DefaultParams) before InsertUser; raw password never touches DB. Verified by TestSignup_Success grepping password_hash for "$argon2id$". |
|
||||
| T-2-04 | Spoofing | Session fixation post-signup | mitigate | Store.Create issues a fresh token; user has no prior session at signup. Verified by TestSignup_Success asserting cookie present + session row exists. |
|
||||
| T-2-14 | DoS | Long-password argon2 abuse | mitigate | Length ceiling 128 enforced BEFORE auth.Hash. Verified by TestSignup_PasswordTooLong. |
|
||||
| T-2-18 | Information disclosure | Email leaked via slog | mitigate | Handler logs event="signup_failed" with no email field. Acceptance grep gate. |
|
||||
| T-2-19 | Tampering | Race on duplicate signup | mitigate | DB UNIQUE constraint + pgconn.PgError code 23505 mapped to user-facing error. Verified by TestSignup_DuplicateEmail. |
|
||||
| T-2-20 | Spoofing | XSS via reflected email | mitigate | templ auto-escapes all expressions; no templ.Raw on user input. Verified by Task 1 render smoke test (no script-injection vector). |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- All eight TestSignup_* tests pass against a real Postgres.
|
||||
- Phase 1 tests still pass with the updated NewRouter signature.
|
||||
- `just dev` boots; opening http://localhost:8080/signup renders the page (manual UAT — recorded in HUMAN-UAT.md at end of phase).
|
||||
- Middleware order in router.go matches D-24 (RequestID -> RealIP -> Slog -> Recoverer -> ResolveSession).
|
||||
- Wall-time budget: `cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/web -run "TestSignup_" -count=1` completes in ≤30s. Pre-seed test users via direct sqlc `InsertUser` using a hash computed with `auth.TestParams` to avoid redundant argon2 `DefaultParams` calls in setup.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- A new user can hit /signup, submit valid creds, and end up at / with a valid session cookie.
|
||||
- Duplicate email and validation errors render inline (HTMX) or full-page (no-JS) as appropriate.
|
||||
- No regressions in Phase 1 routes.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create .planning/phases/02-authentication/02-04-SUMMARY.md recording: where SignupForm/SignupErrors types live (templates pkg vs forms pkg), test-helper duplication strategy (shared vs duplicated setupTestDB), pgconn unique-violation code path, manual UAT screenshot/notes if captured.
|
||||
</output>
|
||||
276
.planning/phases/02-authentication/02-05-PLAN.md
Normal file
276
.planning/phases/02-authentication/02-05-PLAN.md
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 05
|
||||
type: execute
|
||||
wave: 4
|
||||
depends_on: [01, 02, 03, 04]
|
||||
files_modified:
|
||||
- backend/internal/auth/ratelimit.go
|
||||
- backend/internal/auth/ratelimit_test.go
|
||||
- backend/templates/auth_login.templ
|
||||
- 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
|
||||
autonomous: true
|
||||
requirements: [AUTH-02, AUTH-03, AUTH-05, AUTH-07]
|
||||
tags: [go, htmx, login, rate-limit, vertical-slice]
|
||||
must_haves:
|
||||
truths:
|
||||
- "GET /login renders a full templ page with email + password form (D-19)"
|
||||
- "POST /login with valid creds rotates the session, sets a fresh cookie, redirects to / (D-10, D-21)"
|
||||
- "POST /login with wrong password OR unknown email returns the SAME generic error 'Invalid email or password' (D-20)"
|
||||
- "POST /login on the 6th attempt within 60s per (lower(email)+clientIP) returns 429 with HTMX-swap error fragment (D-16, D-18, AUTH-07)"
|
||||
- "Rate limiter uses golang.org/x/time/rate with injectable now() so tests are deterministic"
|
||||
- "Rate-limiter janitor goroutine evicts entries idle > 10 min (D-16)"
|
||||
- "client IP is read from r.RemoteAddr after chimw.RealIP has already rewritten it (Phase 1 stack); IP is part of the rate-limit key only and is NOT persisted (D-17)"
|
||||
- "Authed users hitting GET /login are bounced to / (D-23 RedirectIfAuthed)"
|
||||
artifacts:
|
||||
- path: backend/internal/auth/ratelimit.go
|
||||
provides: "LimiterStore with per-key Limiter map, injectable clock, janitor goroutine"
|
||||
contains: "rate.NewLimiter"
|
||||
- path: backend/internal/auth/ratelimit_test.go
|
||||
provides: "Burst test, per-key isolation, janitor eviction — all with fake clock"
|
||||
- path: backend/templates/auth_login.templ
|
||||
provides: "LoginPage + LoginFormFragment"
|
||||
- path: backend/internal/web/handlers_auth.go
|
||||
provides: "LoginPageHandler + LoginPostHandler appended"
|
||||
key_links:
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/auth/ratelimit.go
|
||||
via: "LimiterStore.Allow(lower(email)+\":\"+ip)"
|
||||
pattern: "LimiterStore"
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/auth/password.go
|
||||
via: "auth.Verify(user.PasswordHash, supplied)"
|
||||
pattern: "auth\\.Verify"
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/auth/session.go
|
||||
via: "Store.Rotate on auth-success (D-10)"
|
||||
pattern: "Store\\.Rotate|store\\.Rotate"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Deliver the LOGIN vertical slice end-to-end: templ form -> POST handler -> rate-limit check -> user lookup -> argon2 verify -> session rotate -> cookie -> redirect. Includes the in-memory token-bucket rate limiter (5/min per email+IP, burst 5) with injectable clock and janitor.
|
||||
|
||||
Purpose: after Plan 04 a user can sign up but not return. This plan closes the loop — sign out, sign back in. Plus AUTH-07 rate-limit lands as part of the same vertical slice so the login flow ships hardened from day one.
|
||||
Output: working login flow + rate limiter, all real-DB tested.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/02-authentication/02-04-PLAN.md
|
||||
@backend/internal/auth/password.go
|
||||
@backend/internal/auth/session.go
|
||||
@backend/internal/auth/middleware.go
|
||||
@backend/internal/web/handlers_auth.go
|
||||
@backend/internal/web/router.go
|
||||
@backend/templates/auth_signup.templ
|
||||
|
||||
<interfaces>
|
||||
Rate limiter (new in this plan):
|
||||
|
||||
```
|
||||
type LimiterStore struct {
|
||||
mu sync.Mutex
|
||||
limits map[string]*entry
|
||||
rate rate.Limit
|
||||
burst int
|
||||
idleTTL time.Duration
|
||||
now func() time.Time
|
||||
}
|
||||
|
||||
type entry struct { lim *rate.Limiter; lastSeen time.Time }
|
||||
|
||||
func NewLimiterStore() *LimiterStore // rate=5/min, burst=5, idleTTL=10min, now=time.Now
|
||||
func (s *LimiterStore) Allow(key string) bool // uses AllowN(s.now(), 1) — Pattern 8
|
||||
func (s *LimiterStore) StartJanitor(interval time.Duration, stop <-chan struct{}) // goroutine
|
||||
```
|
||||
|
||||
Templates:
|
||||
- LoginPage(form LoginForm, errs LoginErrors)
|
||||
- LoginFormFragment(form LoginForm, errs LoginErrors)
|
||||
|
||||
Handlers added to handlers_auth.go:
|
||||
- LoginPageHandler() http.HandlerFunc
|
||||
- LoginPostHandler(deps AuthDeps, rl *auth.LimiterStore) http.HandlerFunc
|
||||
|
||||
AuthDeps extended with optional Now func() time.Time for tests (or pass via LimiterStore constructor — planner picks). Router accepts rl in NewRouter or constructs it internally and starts the janitor.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: LimiterStore (in-memory token bucket with injectable clock + janitor)</name>
|
||||
<read_first>
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 8; "Why AllowN(t, n) over Allow()"; Pitfall 11)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-16, D-17, D-18)
|
||||
backend/internal/auth/types.go
|
||||
</read_first>
|
||||
<behavior>
|
||||
Unit tests (no DB) in backend/internal/auth/ratelimit_test.go using an injected fakeNow:
|
||||
- TestRateLimit_BurstAllowsFiveThenDenies: with rate=5/min, burst=5, now=t0: Allow("k") returns true five consecutive times; the sixth returns false. Pattern 8.
|
||||
- TestRateLimit_RefillsAfter12s: rate.Every(12s) — at t0+12s the previously-saturated key gets one more Allow=true.
|
||||
- TestRateLimit_PerKeyIsolation: Allow("a") six times saturates a; Allow("b") at the same fakeNow still returns true (separate Limiter).
|
||||
- TestRateLimit_JanitorEvictsIdle: insert two entries at t0, advance fakeNow to t0+11min, call s.cleanupNow() (test-only export) — both entries removed; verify via s.size() or by inspecting the unexported map via a same-package test.
|
||||
- TestRateLimit_ConcurrentAllowDoesNotPanic: 100 goroutines calling Allow on overlapping keys; no data race (run with -race).
|
||||
</behavior>
|
||||
<action>
|
||||
Add dep: `cd backend && go get golang.org/x/time@v0.15.0`.
|
||||
|
||||
Create backend/internal/auth/ratelimit.go per RESEARCH Pattern 8 verbatim:
|
||||
- Imports: sync, time, golang.org/x/time/rate.
|
||||
- type entry struct { lim *rate.Limiter; lastSeen time.Time }.
|
||||
- type LimiterStore struct { mu sync.Mutex; limits map[string]*entry; rate rate.Limit; burst int; idleTTL time.Duration; now func() time.Time }.
|
||||
- NewLimiterStore() *LimiterStore: limits=make(...); rate=rate.Every(12*time.Second); burst=5; idleTTL=10*time.Minute; now=time.Now.
|
||||
- (s *LimiterStore) Allow(key string) bool: lock; e, ok := s.limits[key]; if !ok { e = &entry{lim: rate.NewLimiter(s.rate, s.burst)}; s.limits[key] = e }; t := s.now(); e.lastSeen = t; return e.lim.AllowN(t, 1).
|
||||
- (s *LimiterStore) StartJanitor(interval time.Duration, stop <-chan struct{}): go func() { tick := time.NewTicker(interval); defer tick.Stop(); for { select { case <-stop: return; case <-tick.C: s.cleanupNow() } } }().
|
||||
- (s *LimiterStore) cleanupNow(): lock; cutoff := s.now().Add(-s.idleTTL); for k, e := range s.limits { if e.lastSeen.Before(cutoff) { delete(s.limits, k) } }.
|
||||
- Test-only export (in ratelimit_test.go since same package): expose size() method or call s.cleanupNow directly.
|
||||
|
||||
Anti-pattern guards: do NOT use Allow() without args (Pitfall 8 — untestable). Do NOT share one *rate.Limiter across keys (Pitfall: that's a global limit). Do NOT omit the janitor (Pitfall 11 — memory leak).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && go test ./internal/auth/ -run TestRateLimit -count=1 -race -v 2>&1 | tee /tmp/p05t1.log | tail -25 && grep -q "PASS" /tmp/p05t1.log && ! grep -E "^--- FAIL|DATA RACE" /tmp/p05t1.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- ratelimit.go exists; grep -c "AllowN" backend/internal/auth/ratelimit.go >= 1.
|
||||
- grep -c "\\.Allow()" backend/internal/auth/ratelimit.go == 0 (anti-pattern: must use AllowN).
|
||||
- grep -c "rate.NewLimiter" backend/internal/auth/ratelimit.go >= 1.
|
||||
- grep -c "func.*StartJanitor" backend/internal/auth/ratelimit.go == 1.
|
||||
- go.mod contains `golang.org/x/time v0.15.0`.
|
||||
- All five TestRateLimit_* tests pass with -race.
|
||||
- Burst test asserts exactly 5 trues followed by a false at the same fake timestamp.
|
||||
</acceptance_criteria>
|
||||
<done>Rate limiter is deterministic, race-clean, with bounded memory via janitor.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: LoginPage + LoginPostHandler + router wiring + integration tests</name>
|
||||
<read_first>
|
||||
backend/templates/auth_signup.templ (mirror its shape)
|
||||
backend/internal/web/handlers_auth.go (existing SignupPostHandler — extend, do not duplicate the renderError pattern)
|
||||
backend/internal/auth/ratelimit.go (Task 1)
|
||||
backend/internal/auth/password.go
|
||||
backend/internal/auth/session.go
|
||||
backend/internal/auth/middleware.go
|
||||
backend/internal/web/router.go
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 6, Pattern 9, Handler skeleton; Pitfall 7, Pitfall 9)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-19, D-20, D-21, D-23)
|
||||
</read_first>
|
||||
<behavior>
|
||||
Tests in backend/internal/web/handlers_auth_test.go (extending the file from Plan 04). All use setupTestDB. Pre-seed each test with InsertUser of email=test@example.com and a known argon2 hash of password "correct-horse-12chars" using auth.TestParams (so tests stay fast).
|
||||
- TestLogin_Success: POST /login with matching creds -> 303 + Location: / (or HX-Redirect on HTMX) + Set-Cookie present. SELECT COUNT(*) FROM sessions WHERE user_id = test_user.id == 1.
|
||||
- TestLogin_Success_HTMX: HX-Request: true -> 200 + HX-Redirect: /.
|
||||
- TestLogin_WrongPassword: correct email, wrong password -> 401 (or 200; planner picks status — but the body MUST contain the EXACT string "Invalid email or password"). No Set-Cookie. No session row created.
|
||||
- TestLogin_UnknownEmail: email that does not exist -> same 401 (or 200) + EXACT same body string "Invalid email or password" (D-20 enumeration defense). Use bytes.Contains assertion.
|
||||
- TestLogin_ValidationError_BadEmail: email="not-an-email" -> 422 with field-specific "valid email" message (validation errors ARE specific per D-20).
|
||||
- TestLogin_ValidationError_ShortPassword: password length 10 -> 422 with "12" in body.
|
||||
- TestLogin_RotatesExistingSession: pre-create session A for user; POST /login with cookie A -> response Set-Cookie has a DIFFERENT value than A; SELECT COUNT(*) FROM sessions WHERE id = sha256_hex(decode(cookieA)) == 0; new row exists for user (D-10 fixation defense, Pitfall 5).
|
||||
- TestLogin_AlreadyAuthedBouncesHome: GET /login with valid session cookie -> 303 Location: / (RedirectIfAuthed).
|
||||
- TestLogin_RateLimit_6thAttemptReturns429: POST /login six times in <1s with the same email+IP and wrong password. Inject a fake clock via NewLimiterStore() + manually setting limiterStore.now to a fixed time. The 6th response is 429 (http.StatusTooManyRequests) with body containing "Too many attempts". When sent over HTMX (HX-Request: true), the response body is the LoginFormFragment with the error injected (no full page). Verified by absence of <html> tag in the HTMX-variant assertion.
|
||||
- TestLogin_RateLimit_KeyedByEmailPlusIP: Six wrong-password POSTs for emailA from IP1; the seventh attempt for emailB from IP1 is allowed (not 429). Confirms key isolation.
|
||||
- TestLogin_RateLimit_AppliesBeforeUserLookup: with rate-limited key, response is 429 even when the supplied email doesn't exist in the users table — i.e., the rate limit gates the work, not the other way around. (Optional but recommended; documented.)
|
||||
- TestLogin_DoesNotLogPassword: with -v output, assert the test logger captured no line containing the password literal. (Use a captured slog handler.)
|
||||
</behavior>
|
||||
<action>
|
||||
Create backend/templates/auth_login.templ mirroring auth_signup.templ:
|
||||
- templ LoginPage(form LoginForm, errs LoginErrors) wrapped in @Layout("Sign in").
|
||||
- templ LoginFormFragment(form LoginForm, errs LoginErrors) with id="login-form", hx-post="/login", hx-target="#login-form", hx-swap="outerHTML".
|
||||
- Reuse @FieldError and @GeneralError from auth_form_errors.templ.
|
||||
- Comment placeholder for CSRF (Plan 07).
|
||||
Define LoginForm, LoginErrors types alongside SignupForm/SignupErrors per the Plan 04 decision (same file/package).
|
||||
|
||||
Extend backend/internal/web/handlers_auth.go:
|
||||
- LoginPageHandler() http.HandlerFunc — renders templates.LoginPage(empty, empty).
|
||||
- LoginPostHandler(deps AuthDeps, rl *auth.LimiterStore) http.HandlerFunc:
|
||||
1. email := strings.TrimSpace(r.PostFormValue("email")); password := r.PostFormValue("password").
|
||||
2. Validate (specific errors): mail.ParseAddress; len(password) in [12, 128]. On err -> renderLoginError(w, r, form, errs, 422).
|
||||
3. normalized := strings.ToLower(email). ip := clientIP(r) — define clientIP(r) once: read r.RemoteAddr, split host:port via net.SplitHostPort with fallback to raw value (chimw.RealIP has already rewritten it per D-17).
|
||||
4. key := normalized + ":" + ip. if !rl.Allow(key) -> w.WriteHeader(http.StatusTooManyRequests); render LoginFormFragment / LoginPage with errs.General = "Too many attempts. Try again in a minute."; return.
|
||||
5. user, err := deps.Queries.GetUserByEmail(ctx, normalized). On any err (incl. ErrNoRows) -> renderLoginError with errs.General = "Invalid email or password" (D-20); status: 200 with the fragment (or 401 if not HTMX — planner picks; document choice). Pitfall: do NOT short-circuit Verify on user-not-found (timing leak); we accept one indexed SELECT miss as the timing channel, deemed acceptable per RESEARCH security-domain table.
|
||||
6. ok, err := auth.Verify(user.PasswordHash, password). If err != nil OR !ok -> same renderLoginError with EXACT same string "Invalid email or password".
|
||||
7. Rotate any existing session: oldID := ""; if sess, _, ok := auth.Authed(r.Context()); ok { oldID = sess.ID }. cookieValue, expiresAt, err := deps.Store.Rotate(ctx, oldID, user.ID).
|
||||
8. auth.SetSessionCookie(w, cookieValue, expiresAt, deps.Secure).
|
||||
9. Redirect: HX-Redirect: / or 303 Location: / (D-21).
|
||||
- renderLoginError analogous to renderSignupError.
|
||||
|
||||
Update backend/internal/web/router.go:
|
||||
- Extend AuthDeps OR pass rl as a separate arg to NewRouter — planner picks; recommendation: extend AuthDeps with `Limiter *auth.LimiterStore`. Update Plan 04 calls accordingly (NewRouter signature backwards-compatible if Limiter is allowed to be nil — handlers check and skip rate-limit in that case, but production main always wires it).
|
||||
- Inside the RedirectIfAuthed group (already created by Plan 04) add r.Get("/login", LoginPageHandler()).
|
||||
- Outside the group add r.Post("/login", LoginPostHandler(deps, deps.Limiter)).
|
||||
|
||||
Update backend/cmd/web/main.go:
|
||||
- rl := auth.NewLimiterStore(); stopCh := make(chan struct{}); rl.StartJanitor(time.Minute, stopCh); pass rl in AuthDeps. On shutdown (after srv.Shutdown), close(stopCh).
|
||||
|
||||
Anti-pattern guards: do NOT log the email or password on any failure path. Do NOT use http.StatusFound (302) — must be 303 (Pitfall 9). Do NOT vary the error STRING between unknown-email and wrong-password (D-20 — TestLogin_UnknownEmail and TestLogin_WrongPassword both assert the exact same body). Do NOT call Verify before the rate-limit check (DoS — argon2 work happens after rate gate per RESEARCH Handler skeleton).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && templ generate && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/web/ ./internal/auth/ ./templates/ -run "TestLogin|TestRateLimit|TestSignup|TestHealthz|TestIndex|TestDemoTime|TestRequestID|TestSlog|TestPassword|TestSession|TestCookie|TestResolveSession|TestRequireAuth|TestRedirectIfAuthed" -count=1 2>&1 | tee /tmp/p05t2.log | tail -50 && ! grep -E "^FAIL|^--- FAIL" /tmp/p05t2.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- auth_login.templ exists with LoginPage and LoginFormFragment; templ generate exits 0.
|
||||
- handlers_auth.go: grep -c "LoginPostHandler\|LoginPageHandler" >= 2.
|
||||
- handlers_auth.go contains EXACTLY ONE literal string "Invalid email or password" (single source of truth for D-20). Verified: grep -c '"Invalid email or password"' backend/internal/web/handlers_auth.go == 1.
|
||||
- handlers_auth.go: grep -c "store.Rotate\|Store.Rotate\|deps.Store.Rotate" >= 1.
|
||||
- handlers_auth.go: grep -c "auth.Verify" >= 1.
|
||||
- handlers_auth.go: grep -c "rl.Allow\|deps.Limiter.Allow\|Allow(" >= 1.
|
||||
- In handlers_auth.go the rl.Allow(...) call appears at a source-line number LESS than the deps.Queries.GetUserByEmail(...) call (rate-limit happens BEFORE user lookup). Verified by: awk '/rl\\.Allow|Limiter\\.Allow/{a=NR} /GetUserByEmail/{b=NR} END {exit !(a<b)}' backend/internal/web/handlers_auth.go.
|
||||
- router.go: GET /login is inside the RedirectIfAuthed group; POST /login is outside.
|
||||
- cmd/web/main.go: grep -c "NewLimiterStore\|StartJanitor" >= 2.
|
||||
- cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/web/ -run TestLogin -count=1 exits 0 with all 8+ TestLogin_* tests PASS.
|
||||
- TestLogin_RotatesExistingSession passes (rotation works end-to-end through the HTTP handler).
|
||||
- TestLogin_RateLimit_6thAttemptReturns429 passes with the rate limiter fed an injected clock.
|
||||
- All previous phase 2 tests still pass (regression check).
|
||||
</acceptance_criteria>
|
||||
<done>Login works, rotates sessions, returns generic errors for both unknown-email and wrong-password, and returns 429 on the 6th attempt. AUTH-02, AUTH-07 closed.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Client form -> handler | Validated server-side before any expensive crypto or DB work |
|
||||
| In-memory limiter map | Bounded by janitor; key includes client IP from chimw.RealIP-rewritten r.RemoteAddr |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-02 | Spoofing | Credential stuffing | mitigate | LimiterStore caps at 5/min per (email+IP); 6th attempt returns 429 with HTMX-aware fragment. Verified by TestLogin_RateLimit_6thAttemptReturns429. |
|
||||
| T-2-03 | Information disclosure | User enumeration via login error | mitigate | Exactly one literal "Invalid email or password" string; same status and body for unknown-email vs wrong-password. Verified by acceptance grep gate + TestLogin_WrongPassword/UnknownEmail asserting identical body. |
|
||||
| T-2-04 | Spoofing | Session fixation on login | mitigate | Store.Rotate deletes any existing session and creates a fresh one (D-10). Verified by TestLogin_RotatesExistingSession. |
|
||||
| T-2-09 | Spoofing | Brute-force password guess | mitigate | argon2id ~250ms per Verify + 5/min rate limit. Acceptance ensures Verify runs only after Allow returns true. |
|
||||
| T-2-14 | DoS | Argon2 abuse via 6+ rapid attempts | mitigate | Rate limit gates BEFORE auth.Verify (acceptance line-order grep). |
|
||||
| T-2-21 | Information disclosure | Email/password in logs | mitigate | No slog calls reference r.PostFormValue("email") or password; verified by handler-test capturing slog output and asserting absence (TestLogin_DoesNotLogPassword). |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- All Plan 05 tests pass.
|
||||
- Manual browser walkthrough: sign up new user, hit /login from another browser, log in -> redirected to /; submit 6 wrong passwords -> 6th shows 429 inline error fragment.
|
||||
- Phase 4 + Phase 1 regression tests still pass.
|
||||
- go test -race ./internal/auth/... exits 0.
|
||||
- Wall-time budget: `cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/web -run "TestLogin_" -count=1` completes in ≤30s. Per-test setup pre-seeds the user row via a direct sqlc `InsertUser` using a hash computed once with `auth.TestParams` (not `DefaultParams`); production handler still calls `auth.Verify` against the stored hash, so the one real argon2 call per Verify-success path remains. Production code is untouched — no `AuthDeps.HashParams` knob.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- AUTH-02 closed: login issues a server-managed session.
|
||||
- AUTH-07 closed: rate limit on (email+IP) demonstrated by test.
|
||||
- D-20 enforced by a single-source-of-truth grep gate on the error string.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create .planning/phases/02-authentication/02-05-SUMMARY.md recording: exact rate value (rate.Every(12s), burst=5), clientIP helper location and SplitHostPort fallback, choice of status code for invalid creds (401 vs 200 fragment), manual UAT notes.
|
||||
</output>
|
||||
192
.planning/phases/02-authentication/02-06-PLAN.md
Normal file
192
.planning/phases/02-authentication/02-06-PLAN.md
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
---
|
||||
phase: 02-authentication
|
||||
plan: 06
|
||||
type: execute
|
||||
wave: 5
|
||||
depends_on: [01, 03, 04, 05]
|
||||
files_modified:
|
||||
- backend/internal/web/handlers_auth.go
|
||||
- backend/internal/web/handlers_auth_test.go
|
||||
- backend/internal/web/router.go
|
||||
- backend/internal/web/handlers.go
|
||||
- backend/templates/layout.templ
|
||||
- backend/templates/index.templ
|
||||
autonomous: true
|
||||
requirements: [AUTH-04, AUTH-05]
|
||||
tags: [go, htmx, logout, protected-routes, layout]
|
||||
must_haves:
|
||||
truths:
|
||||
- "POST /logout (on the protected group) deletes the session row and clears the cookie (D-06, D-22)"
|
||||
- "Logout request from an unauthed user is blocked by RequireAuth (redirected to /login)"
|
||||
- "After logout, the next request has no session in ctx; the cookie is gone"
|
||||
- "GET / is now protected — unauth users get 303 to /login (303), HTMX users get HX-Redirect (D-23, AUTH-05)"
|
||||
- "The base layout header renders a Log out POST form when the request context has an authed user; nothing when unauthed"
|
||||
- "The Index page renders the authed user's email somewhere visible (smoke check that ctx user reaches the template)"
|
||||
artifacts:
|
||||
- path: backend/internal/web/handlers_auth.go
|
||||
provides: "LogoutHandler appended; renderError helpers reused"
|
||||
- path: backend/internal/web/router.go
|
||||
provides: "Protected group with RequireAuth wrapping GET / and POST /logout"
|
||||
- path: backend/templates/layout.templ
|
||||
provides: "Layout extended with optional logout button; signature accepts *auth.User"
|
||||
key_links:
|
||||
- from: backend/internal/web/handlers_auth.go
|
||||
to: backend/internal/auth/session.go
|
||||
via: "Store.Delete(sess.ID) on logout"
|
||||
pattern: "Store\\.Delete|store\\.Delete"
|
||||
- from: backend/templates/layout.templ
|
||||
to: backend/internal/auth/types.go
|
||||
via: "Layout takes *auth.User parameter"
|
||||
pattern: "auth\\.User"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Close the loop: implement POST /logout, protect GET /, and surface the logout button in the base layout. After this plan ships, the only thing left in Phase 2 is CSRF (Plan 07).
|
||||
|
||||
Purpose: AUTH-04 + AUTH-05 closure. With logout in place and `/` protected, the canonical flow signup -> redirect -> logout -> login becomes the user's daily life.
|
||||
Output: protected home, working logout, layout header that adapts to auth state.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@/Users/arthur.belleville/Documents/perso/projects/xtablo-source/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/02-authentication/02-CONTEXT.md
|
||||
@.planning/phases/02-authentication/02-RESEARCH.md
|
||||
@.planning/phases/02-authentication/02-03-PLAN.md
|
||||
@.planning/phases/02-authentication/02-04-PLAN.md
|
||||
@backend/internal/auth/session.go
|
||||
@backend/internal/auth/middleware.go
|
||||
@backend/internal/auth/cookie.go
|
||||
@backend/internal/web/router.go
|
||||
@backend/internal/web/handlers.go
|
||||
@backend/templates/layout.templ
|
||||
@backend/templates/index.templ
|
||||
|
||||
<interfaces>
|
||||
- LogoutHandler(deps AuthDeps) http.HandlerFunc — POST. Reads sess from ctx (guaranteed present because mounted under RequireAuth), Store.Delete(sess.ID), ClearSessionCookie, redirect to /login.
|
||||
- Layout signature changes:
|
||||
```
|
||||
templ Layout(title string, user *auth.User) { ... }
|
||||
```
|
||||
All call sites updated: Index, SignupPage, LoginPage. Templates referencing user inside their own bodies receive it via Layout-children pattern OR via explicit param threading (planner picks; auth pages pass nil since they're rendered with RedirectIfAuthed wrapping the GET).
|
||||
- IndexHandler(deps AuthDeps) http.HandlerFunc — renders templates.Index(user) where user is Authed(ctx).user. Existing IndexHandler() signature changes.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: LogoutHandler + protected route group + IndexHandler/Layout signature changes</name>
|
||||
<read_first>
|
||||
backend/internal/auth/session.go
|
||||
backend/internal/auth/cookie.go
|
||||
backend/internal/auth/middleware.go
|
||||
backend/internal/web/router.go
|
||||
backend/internal/web/handlers.go
|
||||
backend/templates/layout.templ
|
||||
backend/templates/index.templ
|
||||
.planning/phases/02-authentication/02-RESEARCH.md (Pattern 6 — route groups; Logout snippet; Pitfall 9 — MaxAge -1 already enforced in Plan 03)
|
||||
.planning/phases/02-authentication/02-CONTEXT.md (D-06, D-22, D-23)
|
||||
</read_first>
|
||||
<behavior>
|
||||
Tests in backend/internal/web/handlers_auth_test.go (extend Plans 04+05 test file):
|
||||
- TestLogout_Success: pre-seed user + session; POST /logout with cookie -> 303 Location: /login (or HX-Redirect when HTMX). Set-Cookie header expires the cookie (Max-Age=-1 or 0, Expires in the past). SELECT COUNT(*) FROM sessions WHERE id=hex(sha256(decode(cookie))) == 0 (D-06 hard delete).
|
||||
- TestLogout_UnauthRedirectsToLogin: POST /logout with NO cookie -> 303 Location: /login from RequireAuth, NOT a 500 (the handler never runs). Body is not the logout success path.
|
||||
- TestLogout_HXRedirect: POST /logout with HX-Request: true -> 200 + HX-Redirect: /login.
|
||||
- TestProtected_HomeUnauthRedirects: GET / with no cookie -> 303 Location: /login.
|
||||
- TestProtected_HomeUnauthHXRedirect: GET / + HX-Request: true -> 200 + HX-Redirect: /login.
|
||||
- TestProtected_HomeAuthRendersUserEmail: pre-seed user "alice@example.com" + session; GET / with cookie -> 200 with body containing "alice@example.com" (proves ctx user reached the template).
|
||||
- TestLayout_LogoutFormVisibleWhenAuthed: render Layout("X", &auth.User{Email:"a@b.c"}) to buffer; assert body contains form action="/logout" and method="POST".
|
||||
- TestLayout_LogoutFormHiddenWhenUnauthed: render Layout("X", nil); assert body does NOT contain action="/logout".
|
||||
- TestLogout_AfterLogoutSubsequentRequestUnauth: POST /logout, then immediately GET / using the SAME cookie value (simulating an attacker still holding it) — must redirect to /login (session row gone, Lookup returns ErrSessionNotFound).
|
||||
</behavior>
|
||||
<action>
|
||||
1. Extend Layout signature in backend/templates/layout.templ:
|
||||
- `templ Layout(title string, user *auth.User) { ... }` (import backend/internal/auth in the templ file).
|
||||
- Inside the header div: `if user != nil { <form method="POST" action="/logout" class="inline"><!-- CSRF in Plan 07 --><button type="submit" class="text-sm text-slate-700 hover:underline">Log out</button></form> }`.
|
||||
- Optionally render the user.Email next to it (small text). Recommendation: yes — it's the cheapest "you reached the template" smoke test.
|
||||
2. Update backend/templates/index.templ: `templ Index(user *auth.User) { @Layout("Xtablo", user) { ... } }`. Replace the existing demo block content's outer wrapper so the user.Email shows on the page (e.g., a small "Signed in as {user.Email}" line above the demo card).
|
||||
3. Update Signup and Login templates: SignupPage and LoginPage pass `nil` for user when wrapping their own @Layout("Sign up", nil) / @Layout("Sign in", nil).
|
||||
4. Update backend/internal/web/handlers.go: change IndexHandler signature to `IndexHandler() http.HandlerFunc` (no deps) but inside the handler read user from auth.Authed(r.Context()) and call templates.Index(user). It is fine to keep IndexHandler() as a no-arg constructor since auth.Authed pulls from ctx.
|
||||
5. Add LogoutHandler(deps AuthDeps) http.HandlerFunc to backend/internal/web/handlers_auth.go:
|
||||
- sess, _, ok := auth.Authed(r.Context()); if !ok { http.Redirect(w, r, "/login", http.StatusSeeOther); return } // defense in depth, though RequireAuth already gates.
|
||||
- if err := deps.Store.Delete(r.Context(), sess.ID); err != nil { slog.Error("logout delete", "err", err) /* continue and clear cookie anyway */ }.
|
||||
- auth.ClearSessionCookie(w, deps.Secure).
|
||||
- if HX-Request: w.Header().Set("HX-Redirect", "/login"); w.WriteHeader(http.StatusOK); return. Else http.Redirect(w, r, "/login", http.StatusSeeOther).
|
||||
6. Update backend/internal/web/router.go: introduce a protected group AFTER the existing public groups (and AFTER ResolveSession):
|
||||
```
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(auth.RequireAuth)
|
||||
r.Get("/", IndexHandler())
|
||||
r.Post("/logout", LogoutHandler(deps))
|
||||
})
|
||||
```
|
||||
REMOVE the old top-level r.Get("/", IndexHandler()) registration. /healthz, /demo/time, /static/* remain public.
|
||||
7. Update backend/internal/web/handlers_test.go (Phase 1 + Plan 04 tests):
|
||||
- TestIndex_RendersHxGet expected behavior changes: an UNAUTH GET / now returns 303 to /login. Update the test (or add TestIndex_AuthRendersHxGet) to seed a user + session via setupTestDB and assert the HTMX demo still renders.
|
||||
- TestRequestID_HeaderSet should use a route that's still public; switch to /healthz if it isn't already.
|
||||
- TestDemoTime_Fragment is unaffected (route still public).
|
||||
Document any test-rewrite decisions in the plan SUMMARY.
|
||||
|
||||
Anti-pattern guards:
|
||||
- Logout MUST be POST not GET (D-22). The Layout snippet uses <form method="POST">.
|
||||
- Cookie clear uses Max-Age=-1 (already enforced by ClearSessionCookie from Plan 03 — Pitfall: 0 means session cookie).
|
||||
- Do NOT short-circuit Store.Delete if ctx user is missing — RequireAuth already gates, but the handler still has the defense-in-depth redirect.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && templ generate && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/web/ ./internal/auth/ ./templates/ -count=1 2>&1 | tee /tmp/p06.log | tail -60 && ! grep -E "^FAIL|^--- FAIL" /tmp/p06.log</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- handlers_auth.go: grep -c "func LogoutHandler" == 1; grep -c "Store.Delete\|store.Delete\|deps.Store.Delete" >= 1 in the Logout block.
|
||||
- handlers_auth.go: grep -c "ClearSessionCookie" >= 1.
|
||||
- router.go: grep -c "auth.RequireAuth" >= 1; the protected group contains both `/` and `/logout`.
|
||||
- router.go: the OLD top-level `r.Get("/", IndexHandler())` is removed; grep -c 'r.Get("/", ' backend/internal/web/router.go == 0 (only the in-group registration remains).
|
||||
- layout.templ: grep -c 'method="POST"' >= 1; grep -c 'action="/logout"' >= 1; grep -c 'if user != nil' >= 1.
|
||||
- layout.templ: grep -c 'method="GET".*logout\|hx-get="/logout"' == 0 (D-22 — logout must NOT be a GET).
|
||||
- index.templ: grep -c "user.Email\|user\\.Email" >= 1.
|
||||
- cd backend && templ generate exits 0.
|
||||
- cd backend && go build ./... exits 0.
|
||||
- cd backend && TEST_DATABASE_URL=$DATABASE_URL go test ./internal/web/ -run "TestLogout|TestProtected|TestLayout" -count=1 exits 0 with all nine TestLogout_*/TestProtected_*/TestLayout_* tests PASS.
|
||||
- Existing Plan 04 + Plan 05 tests still pass (TestSignup_*, TestLogin_*).
|
||||
- TestLogout_AfterLogoutSubsequentRequestUnauth passes — proves the session is server-side invalidated, not just cookie-cleared client-side.
|
||||
</acceptance_criteria>
|
||||
<done>AUTH-04 and AUTH-05 closed. End-to-end loop works in tests: signup -> / -> logout -> /login -> login -> /.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Cookie -> ResolveSession -> ctx | Server-side authority; cookie alone is not enough — DB lookup must succeed |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-2-07 | Spoofing | Stale session after logout | mitigate | Store.Delete hard-deletes the row; even if attacker captured the cookie pre-logout, post-logout requests miss in Lookup. Verified by TestLogout_AfterLogoutSubsequentRequestUnauth. |
|
||||
| T-2-10 | Elevation of privilege | Broken access control on / | mitigate | / is now inside the RequireAuth-wrapped group; verified by TestProtected_HomeUnauthRedirects. |
|
||||
| T-2-22 | Tampering | CSRF on logout (GET-based) | mitigate | Logout is POST form (D-22). Full CSRF token check lands in Plan 07; SameSite=Lax provides interim protection against cross-origin POST. Verified by acceptance grep gate forbidding GET logout. |
|
||||
| T-2-23 | Information disclosure | User email rendered in HTML | accept | The page is gated behind auth; the user is viewing their own email. templ auto-escapes the value. No mitigation beyond auto-escape. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- All Plan 06 tests pass.
|
||||
- Manual browser walkthrough: sign up -> see /, see "Log out" button + email in header; click Log out -> bounce to /login; try direct GET / -> bounce to /login again.
|
||||
- No regressions across Plans 02-05 tests.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- AUTH-04 closed: user can log out; session is server-side invalidated.
|
||||
- AUTH-05 closed: / is protected; /login redirects authed users to /.
|
||||
- Logout button visible only when authed (verified by template smoke test).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create .planning/phases/02-authentication/02-06-SUMMARY.md recording: any Phase 1 test rewrites required by /-becomes-protected, manual UAT notes, full middleware-order snapshot from router.go (so Plan 07 has a single-source-of-truth diff base for inserting csrf.Protect).
|
||||
</output>
|
||||
|
|
@ -101,20 +101,22 @@ New / changed surfaces this plan introduces:
|
|||
- `templ CSRFField(token string) { <input type="hidden" name="_csrf" value={ token }/> }`
|
||||
- Lives alongside the existing Button/Card/Badge components established in Phase 1.
|
||||
|
||||
- Every templ form-rendering helper acquires a `csrfToken string` arg:
|
||||
- `templ LoginPage(token string, errors map[string]string)`
|
||||
- `templ LoginFormFragment(token string, errors map[string]string)`
|
||||
- `templ SignupPage(token string, errors map[string]string)`
|
||||
- `templ SignupFormFragment(token string, errors map[string]string)`
|
||||
- Every templ form-rendering helper acquires an ADDITIONAL `csrfToken string` arg appended to the existing signature established in Plans 04/05/06. The pre-existing `form` (typed SignupForm/LoginForm — for value preservation, Email roundtrip per Plan 04 D-25) and `errs` (typed SignupErrors/LoginErrors structs) arguments STAY. Final signatures:
|
||||
- `templ LoginPage(form LoginForm, errs LoginErrors, csrfToken string)`
|
||||
- `templ LoginFormFragment(form LoginForm, errs LoginErrors, csrfToken string)`
|
||||
- `templ SignupPage(form SignupForm, errs SignupErrors, csrfToken string)`
|
||||
- `templ SignupFormFragment(form SignupForm, errs SignupErrors, csrfToken string)`
|
||||
- `templ Layout(title string, user *auth.User, csrfToken string)` — the logout form needs the token even though the page itself may be the Index.
|
||||
- `templ Index(user *auth.User, csrfToken string)`
|
||||
|
||||
- Handlers thread `csrf.Token(r)` into every page/fragment render call:
|
||||
Do NOT replace the typed `form`/`errs` arguments with `map[string]string` — that would drop form-value preservation (Email roundtrip would be lost) AND swap typed Errors structs for stringly-typed maps. The change in THIS plan is purely additive (appending `csrfToken string`).
|
||||
|
||||
- Handlers thread `csrf.Token(r)` into every page/fragment render call alongside the existing form + errs values:
|
||||
- `LoginPageHandler`, `SignupPageHandler`, `IndexHandler` and the error-fragment branches of `LoginPostHandler` / `SignupPostHandler`.
|
||||
|
||||
- `cmd/web/main.go` loads `SESSION_SECRET` (hex-encoded 32 bytes) → `hex.DecodeString` → []byte; logs a fatal error if missing or len != 32. Pass the []byte and `env` string into `NewRouter`.
|
||||
|
||||
- `NewRouter` signature acquires `csrfKey []byte` and `env string` params (if not already present from earlier plans) and calls `r.Use(auth.Mount(env, csrfKey))` immediately after `r.Use(auth.ResolveSession(store, cookieName))`.
|
||||
- `NewRouter` signature acquires `csrfKey []byte` and `env string` params (if not already present from earlier plans) and calls `r.Use(auth.Mount(env, csrfKey))` immediately after `r.Use(auth.ResolveSession(store))`. Note: `ResolveSession` takes a single `*Store` argument (per Plan 03 — the cookie name comes from the `SessionCookieName` constant; do NOT call `ResolveSession(store, cookieName)`).
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
|
|
@ -148,7 +150,7 @@ New / changed surfaces this plan introduces:
|
|||
- TestCSRF_LogoutMissingToken: pre-seed session; POST /logout WITH session cookie but WITHOUT _csrf → 403. SELECT COUNT(*) FROM sessions WHERE id = $1 == 1 (NOT deleted).
|
||||
- TestCSRF_LogoutValidToken: full GET / → POST /logout with token → 303, session row deleted.
|
||||
- TestCSRF_HeaderFallback: POST /login with `X-CSRF-Token` header (no form field) → token accepted (verifies `csrf.RequestHeader("X-CSRF-Token")` wiring for future HTMX hx-headers usage).
|
||||
- TestForms_ContainCSRFField (templ smoke): render LoginPage("abc", nil), SignupPage("abc", nil), Layout("X", &auth.User{Email:"x@y"}, "abc") to a buffer and assert each contains `name="_csrf"` AND `value="abc"`. Login fragment and signup fragment also covered.
|
||||
- TestForms_ContainCSRFField (templ smoke): render LoginPage(LoginForm{}, LoginErrors{}, "abc"), SignupPage(SignupForm{}, SignupErrors{}, "abc"), Layout("X", &auth.User{Email:"x@y"}, "abc") to a buffer and assert each contains `name="_csrf"` AND `value="abc"`. Login fragment and signup fragment also covered.
|
||||
- TestRouter_CSRFMountedAfterResolveSession: a unit-level inspection — assert that NewRouter wires `auth.ResolveSession` before `auth.Mount` (D-24). Implementation: render a request that proves order, OR (simpler) read router.go source via the test using os.ReadFile and assert the substring `ResolveSession` appears before `auth.Mount` / `csrf.Protect`. Choose the source-file scan — deterministic and cheap.
|
||||
- TestMain_FailsFastOnMissingSecret: this is a build-time / startup-time assertion — added as a `TestLoadCSRFKey_*` set in a new `backend/cmd/web/main_test.go` (or `backend/internal/web/csrf_key_test.go` if main.go is too small to support a separate test target). Cases: missing env → error; len != 32 → error; valid hex 64-char → returns []byte{32}.
|
||||
</behavior>
|
||||
|
|
@ -172,15 +174,15 @@ New / changed surfaces this plan introduces:
|
|||
- Import path: align with how Button/Card/Badge are imported in existing templates.
|
||||
|
||||
5. Update `backend/templates/auth_login.templ`:
|
||||
- `LoginPage(token string, errors map[string]string)` and `LoginFormFragment(token string, errors map[string]string)`.
|
||||
- Each `<form>` has `@ui.CSRFField(token)` as the first child.
|
||||
- `LoginPage` wraps `@Layout("Sign in", nil, token)`.
|
||||
- Append `csrfToken string` as the third argument; KEEP the existing `form LoginForm` and `errs LoginErrors` arguments from Plan 05 in place. Final: `templ LoginPage(form LoginForm, errs LoginErrors, csrfToken string)` and `templ LoginFormFragment(form LoginForm, errs LoginErrors, csrfToken string)`.
|
||||
- Each `<form>` has `@ui.CSRFField(csrfToken)` as the first child.
|
||||
- `LoginPage` wraps `@Layout("Sign in", nil, csrfToken)`.
|
||||
|
||||
6. Update `backend/templates/auth_signup.templ`:
|
||||
- `SignupPage(token string, errors map[string]string)` and `SignupFormFragment(token string, errors map[string]string)`.
|
||||
- Same pattern: `@ui.CSRFField(token)` first child of every form; `@Layout("Sign up", nil, token)`.
|
||||
- Append `csrfToken string` as the third argument; KEEP the existing `form SignupForm` and `errs SignupErrors` arguments from Plan 04 in place. Final: `templ SignupPage(form SignupForm, errs SignupErrors, csrfToken string)` and `templ SignupFormFragment(form SignupForm, errs SignupErrors, csrfToken string)`.
|
||||
- Same pattern: `@ui.CSRFField(csrfToken)` first child of every form; `@Layout("Sign up", nil, csrfToken)`.
|
||||
|
||||
7. Update `backend/templates/auth_form_errors.templ` if it owns a `<form>` shell: thread `token string` through and embed `@ui.CSRFField(token)`. If it only renders error markup, no change needed.
|
||||
7. Update `backend/templates/auth_form_errors.templ` if it owns a `<form>` shell: thread `csrfToken string` through and embed `@ui.CSRFField(csrfToken)`. If it only renders error markup (FieldError/GeneralError per Plan 04), no change needed.
|
||||
|
||||
8. Update `backend/templates/index.templ`:
|
||||
- `Index(user *auth.User, csrfToken string)` → wraps `@Layout("Xtablo", user, csrfToken)`.
|
||||
|
|
@ -188,16 +190,16 @@ New / changed surfaces this plan introduces:
|
|||
9. Update `backend/internal/web/handlers.go`:
|
||||
- `IndexHandler()` reads `auth.Authed(r.Context())` for user AND `csrf.Token(r)` for the token, then `templates.Index(user, csrf.Token(r)).Render(...)`.
|
||||
|
||||
10. Update `backend/internal/web/handlers_auth.go` — every page/fragment render call now passes `csrf.Token(r)`:
|
||||
- `SignupPageHandler`: `templates.SignupPage(csrf.Token(r), nil).Render(...)`.
|
||||
- `SignupPostHandler`: on validation error / duplicate-email branch, render `SignupPage` or `SignupFormFragment` with `csrf.Token(r)`. Success branch (303 redirect) does not render templ.
|
||||
- `LoginPageHandler`: same pattern with `LoginPage`.
|
||||
- `LoginPostHandler`: same pattern with `LoginPage` / `LoginFormFragment` on error branches.
|
||||
10. Update `backend/internal/web/handlers_auth.go` — every page/fragment render call now passes `csrf.Token(r)` ALONGSIDE the existing form + errs values (do not drop typed form/errs; do not collapse to map[string]string):
|
||||
- `SignupPageHandler`: `templates.SignupPage(SignupForm{}, SignupErrors{}, csrf.Token(r)).Render(...)`.
|
||||
- `SignupPostHandler`: on validation error / duplicate-email branch, render `SignupPage(form, errs, csrf.Token(r))` or `SignupFormFragment(form, errs, csrf.Token(r))`. Success branch (303 redirect) does not render templ.
|
||||
- `LoginPageHandler`: same pattern with `LoginPage(LoginForm{}, LoginErrors{}, csrf.Token(r))`.
|
||||
- `LoginPostHandler`: same pattern with `LoginPage(form, errs, csrf.Token(r))` / `LoginFormFragment(form, errs, csrf.Token(r))` on error branches.
|
||||
- `LogoutHandler`: no templ render; unaffected except its form must now arrive with `_csrf` (handled in layout.templ).
|
||||
- **Pitfall 1:** Audit every handler in this file. They MUST read form values via `r.PostFormValue(...)` only — never `io.ReadAll(r.Body)` or `json.NewDecoder(r.Body)`. Existing Plans 04/05 already use `r.PostFormValue` per their action sections; this is a regression guard.
|
||||
|
||||
11. Update `backend/internal/web/router.go`:
|
||||
- After `r.Use(auth.ResolveSession(store, cookieName))` and BEFORE any route group: `r.Use(auth.Mount(env, csrfKey))`.
|
||||
- After `r.Use(auth.ResolveSession(deps.Store))` (single-arg form, per Plan 03 — cookie name is read from the `SessionCookieName` constant inside the package; do NOT pass a second `cookieName` argument) and BEFORE any route group: `r.Use(auth.Mount(env, csrfKey))`.
|
||||
- The protected and public route groups stay as in Plan 06; csrf.Protect runs across both.
|
||||
- Add a comment above each `r.Use` referencing D-24 locked order.
|
||||
|
||||
|
|
@ -210,7 +212,7 @@ New / changed surfaces this plan introduces:
|
|||
- Add `SESSION_SECRET=` line with a comment: `# 32 random bytes hex-encoded — generate with: openssl rand -hex 32`.
|
||||
- Add a placeholder or leave blank value; clearly NOT a real key.
|
||||
|
||||
14. Run `cd backend && templ generate && go build ./...` and fix any signature drift in call sites (Phase 1 + Plans 04..06 tests may reference old `Layout` / `Index` signatures).
|
||||
14. Run `cd backend && templ generate && go build ./...` and fix any signature drift in call sites (Phase 1 + Plans 04..06 tests may reference old `Layout` / `Index` signatures). All updates must be ADDITIVE to existing templ args — do not delete the form/errs parameters introduced by Plans 04/05.
|
||||
|
||||
Anti-pattern guards:
|
||||
- `csrf.Mount` must be AFTER `auth.ResolveSession` (D-24, Pitfall 7).
|
||||
|
|
@ -218,6 +220,8 @@ New / changed surfaces this plan introduces:
|
|||
- Handlers MUST NOT touch `r.Body` directly (Pitfall 1).
|
||||
- csrf key MUST come from env, not a compile-time constant (D-15).
|
||||
- Cookie name for csrf is gorilla's default `_gorilla_csrf` — do NOT override; D-12 only governs the session cookie name.
|
||||
- Do NOT replace the typed `SignupForm`/`SignupErrors`/`LoginForm`/`LoginErrors` arguments with `map[string]string` — Email roundtrip and typed error handling from Plans 04/05 must be preserved.
|
||||
- `ResolveSession` is single-arg (`*Store`); a two-arg form `ResolveSession(store, cookieName)` does not exist in Plan 03.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd backend && templ generate && go build ./... && TEST_DATABASE_URL="$DATABASE_URL" go test ./internal/web/ ./internal/auth/ ./templates/ ./cmd/web/ -count=1 -run "TestCSRF|TestForms_ContainCSRFField|TestRouter_CSRFMountedAfterResolveSession|TestLoadCSRFKey|TestSignup|TestLogin|TestLogout|TestProtected|TestLayout" 2>&1 | tee /tmp/p07.log | tail -80 && ! grep -E "^FAIL|^--- FAIL" /tmp/p07.log</automated>
|
||||
|
|
@ -227,7 +231,9 @@ New / changed surfaces this plan introduces:
|
|||
- `backend/internal/auth/csrf.go` exists; `grep -c "func Mount" backend/internal/auth/csrf.go == 1`; `grep -c "csrf.Protect" backend/internal/auth/csrf.go == 1`; `grep -c "csrf.Secure" backend/internal/auth/csrf.go == 1`; `grep -c "csrf.SameSiteLaxMode" backend/internal/auth/csrf.go == 1`; `grep -c "LoadKeyFromEnv" backend/internal/auth/csrf.go == 1`.
|
||||
- `backend/internal/web/ui/csrf_field.templ` exists; `grep -c "templ CSRFField" backend/internal/web/ui/csrf_field.templ == 1`; `grep -c 'name="_csrf"' backend/internal/web/ui/csrf_field.templ == 1`.
|
||||
- Every templ file with a `<form method="POST">` contains `@ui.CSRFField(`. Concretely, for each f in {layout.templ, auth_login.templ, auth_signup.templ}: `grep -v '^//' $f | awk '/<form/,/<\/form>/' | grep -c 'ui.CSRFField\|CSRFField(' >= 1`.
|
||||
- Typed form/errs args preserved in templ signatures: `grep -c "templ SignupPage(form SignupForm, errs SignupErrors, csrfToken string)" backend/templates/auth_signup.templ == 1`; same for `SignupFormFragment`, `LoginPage`, `LoginFormFragment`. No occurrence of `map[string]string` in the four form templ files: `grep -c "map\[string\]string" backend/templates/auth_signup.templ backend/templates/auth_login.templ` returns 0.
|
||||
- `backend/internal/web/router.go` has BOTH `auth.ResolveSession` and `auth.Mount` (or `csrf.Protect`) — and ResolveSession appears on an earlier line: `awk '/auth\.ResolveSession/{r=NR} /auth\.Mount|csrf\.Protect/{c=NR} END{exit !(r>0 && c>0 && r<c)}' backend/internal/web/router.go` exits 0.
|
||||
- `auth.ResolveSession` call in router.go is single-arg (no `, cookieName` second arg): `grep -E "auth\.ResolveSession\([^,)]*,\s*[a-zA-Z]" backend/internal/web/router.go | wc -l` returns 0.
|
||||
- `cmd/web/main.go` calls `auth.LoadKeyFromEnv` and `log.Fatal` (or equivalent) on error: `grep -c "LoadKeyFromEnv" backend/cmd/web/main.go == 1`; `grep -c "SESSION_SECRET" backend/cmd/web/main.go >= 1`.
|
||||
- `backend/.env.example` mentions `SESSION_SECRET`: `grep -c "SESSION_SECRET" backend/.env.example >= 1`.
|
||||
- `cd backend && templ generate` exits 0.
|
||||
|
|
@ -277,6 +283,7 @@ New / changed surfaces this plan introduces:
|
|||
- The reusable `ui.CSRFField` component is the canonical way to embed CSRF in future templ forms.
|
||||
- SESSION_SECRET is documented and loaded from env; missing key fails the binary fast at startup.
|
||||
- Middleware order (`ResolveSession` → `csrf.Protect` → route groups) matches D-24 exactly.
|
||||
- Typed `SignupForm`/`SignupErrors`/`LoginForm`/`LoginErrors` arguments from Plans 04/05 remain intact (no regression to `map[string]string`).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
|
|
|||
|
|
@ -1011,7 +1011,7 @@ func loginPostHandler(store *auth.Store, rl *auth.LimiterStore, cookieName strin
|
|||
|
||||
**Interpretation:** A7 is the only assumption with non-trivial risk. Resolution is to recommend the `TEST_DATABASE_URL` path (reuse Phase 1's compose Postgres) as the default in Testing Strategy below, with testcontainers documented as the future fresh-DB-per-test option.
|
||||
|
||||
## Open Questions
|
||||
## Open Questions (RESOLVED)
|
||||
|
||||
1. **testcontainers-go vs compose Postgres for tests — recommendation**
|
||||
- What we know: D-26 leaves it to the planner. Phase 1 already runs Postgres via `compose.yaml`. The developer uses podman. testcontainers-go has known podman friction.
|
||||
|
|
@ -1022,22 +1022,22 @@ func loginPostHandler(store *auth.Store, rl *auth.LimiterStore, cookieName strin
|
|||
- Add `TEST_DATABASE_URL` to `.env.example` (planner picks default, e.g. `postgres://xtablo:xtablo@localhost:5432/xtablo_test?sslmode=disable`).
|
||||
- Tests skip with `t.Skip` if `TEST_DATABASE_URL` is unset, mirroring the existing `internal/db/pool_test.go` pattern.
|
||||
- **Why:** Avoids the podman+testcontainers compatibility risk (A7), avoids per-test container startup latency on macOS (~5–8s), and the developer already has the compose Postgres running during `just dev`. Reserve testcontainers-go as a Phase 7+ option (CI ephemerality, fresh-DB-per-test) once value/risk is clearer.
|
||||
- **Status:** RECOMMENDED; planner may override if preferred.
|
||||
- **Status:** RESOLVED; planner may override if preferred.
|
||||
|
||||
2. **`alexedwards/argon2id` wrapper vs hand-rolled — recommendation**
|
||||
- What we know: D-08 phrasing ("hash code lives in `internal/auth/password.go` with a tiny self-test") leans hand-rolled. Wrapper saves ~80 LOC and provides identical PHC format.
|
||||
- **Recommendation:** Hand-roll (Pattern 1 verbatim). The wrapper adds a dep we don't need; the code is short and easily testable; we want the self-test as `init()`-time invariant per CONTEXT.md.
|
||||
- **Status:** RECOMMENDED hand-rolled. Planner may pick the wrapper if they prefer.
|
||||
- **Status:** RESOLVED hand-rolled. Planner may pick the wrapper if they prefer.
|
||||
|
||||
3. **`internal/auth/` package layout — recommendation**
|
||||
- What we know: D-71 (Claude's discretion) says planner picks; Phase 1 has an empty `internal/session/doc.go` placeholder.
|
||||
- **Recommendation:** Consolidate everything into `internal/auth/` (password, session, ratelimit, cookie, csrf helpers). Delete `internal/session/` (or repurpose its `doc.go` to point at `internal/auth`). Single package = single import, easier to reason about.
|
||||
- **Status:** RECOMMENDED.
|
||||
- **Status:** RESOLVED.
|
||||
|
||||
4. **Logout: protected group or lenient public?**
|
||||
- What we know: D-73 (Claude's discretion). Default: protected.
|
||||
- **Recommendation:** Protected (require auth to log out). Cleaner — unauth logouts are a no-op anyway; protected version gets free CSRF + auth context. The base-layout button only renders when authed, so the route is never reachable from an unauth page.
|
||||
- **Status:** RECOMMENDED.
|
||||
- **Status:** RESOLVED.
|
||||
|
||||
## Environment Availability
|
||||
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@
|
|||
phase: 2
|
||||
slug: authentication
|
||||
status: draft
|
||||
nyquist_compliant: false
|
||||
wave_0_complete: false
|
||||
nyquist_compliant: true
|
||||
wave_0_complete: true
|
||||
created: 2026-05-14
|
||||
---
|
||||
|
||||
|
|
@ -86,4 +86,4 @@ Filled by planner as PLAN.md tasks are emitted. Each task with `type: execute` m
|
|||
- [ ] Feedback latency < 30s
|
||||
- [ ] `nyquist_compliant: true` set in frontmatter
|
||||
|
||||
**Approval:** pending
|
||||
**Approval:** approved 2026-05-14
|
||||
|
|
|
|||
Loading…
Reference in a new issue