From 71d867cbdb93f12d4db7d39d10fac09e95b47fba Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:33:05 +0200 Subject: [PATCH] =?UTF-8?q?test(b3-1):=20auth=20characterization=20tests?= =?UTF-8?q?=20=E2=80=94=20freeze=20the=20slice=20before=20B3-2=20(#1449)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(b3-0): record PR #1448 = d383d8c7 in the evidence block Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * docs(b3-1): inventory of what the 85 auth tests already pin, per route and property Route x property table in the B3-1 evidence block: the existing test for each row, or GAP and the characterization row that fills it (next commit). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * test(b3-1): auth characterization — fill the inventory gaps against today's behaviour 12 tests / 44 table rows over the mounted auth router, no mocks: read faults via ALTER TABLE RENAME, write faults via RAISE(FAIL) triggers. Three rows pin defects as-is with `// known:` and ledger entries OC-0376 (register 500 after the account commit), OC-0377 (verify-totp maps a DB error to 401), OC-0378 (challenge consumed before the session insert); fixed in B3-9, not here. Mutation spot-check: 401->500 in totpChallengeSecret, 500->401 in loginAuthenticate, 503->401 in AuthMiddleware each turned the rows that name them RED. The nine watched ledger-count claims move 56 open / 375 -> 59 / 378. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * docs(b3-1): coverage before/after, row count and pre-squash SHAs in the evidence block auth_handler.go 78.0% -> 90.2%, totp_handler.go 78.8% -> 91.1% (statements). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * fix(b3-1): Codex P2s — guaranteed-invalid TOTP code, B3-9 scope, hp-0 breakdown - wrongTOTPCode derives a code outside the three accepted steps instead of assuming "000000" is invalid (1-in-333k flake in the per-user cap row) - OC-0376/0377/0378 added to B3-9 in the step table and its section - hp-0 open-record breakdown recomputed for the 59-open ledger Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * docs(b3-1): Codex round 2 — OC-0378 fix must keep the challenge claim atomic; coverage command syntax - OC-0378 suggestedFix: Consume first, restore/re-issue on session failure; issuing before Consume lets two concurrent verifies both create sessions - evidence block: go test -coverprofile=cover.out ./api/ (flag needs a file) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu * fix(b3-1): Codex round 3 — exclude the +2 TOTP step, sequence auth B3-9 after B3-2, keep the replay claim in OC-0378's remedy - wrongTOTPCode excludes {-1,0,+1,+2}: the verifier samples the clock after the helper, so a step boundary in between shifts its window to {0,+1,+2} - B3-9 "Parallel with": OC-0345/0346 any; OC-0323 with B3-8; OC-0376..0378 after B3-2 (matches the safe-parallelism rule and the section text) - OC-0378 suggestedFix: a restored challenge must carry the accepted verification or roll back the MarkUsed claim, or the retry is a replay Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Rg9QQWVN3E5UUgBD2dydtu --------- Co-authored-by: Claude Fable 5 --- .superpowers/findings-ledger.json | 53 +- Server/api/auth_characterization_test.go | 991 ++++++++++++++++++ docs/plans/README.md | 2 +- docs/plans/b0-baseline-2026-08-25.md | 2 +- ...rver-architecture-guardrails-2026-08-29.md | 135 ++- docs/plans/hp-0-scorecard-2026-08-25.md | 20 +- .../repo-health-issue-register-2026-08-23.md | 4 +- 7 files changed, 1176 insertions(+), 31 deletions(-) create mode 100644 Server/api/auth_characterization_test.go diff --git a/.superpowers/findings-ledger.json b/.superpowers/findings-ledger.json index f828ee6c..76ae2c53 100644 --- a/.superpowers/findings-ledger.json +++ b/.superpowers/findings-ledger.json @@ -1,5 +1,5 @@ { - "nextId": 376, + "nextId": 379, "findings": [ { "id": "OC-0001", @@ -8484,6 +8484,57 @@ "suggestedFix": "In the loop at Client/src/pages/MainPage.ts:808-829, leave the signature accumulation alone but skip the local user for relabeling: hoist `const selfId = getCurrentUserId();` and wrap both setLabel blocks in `if (uid !== selfId) { ... }` — VideoModeController is the sole writer of the self tiles' \"(You)\" / \"Your Screen\" labels.", "confidence": "high", "finder": "opus" + }, + { + "id": "OC-0376", + "title": "Register commits the account and burns the invite, then answers 500 when the session insert fails", + "file": "Server/api/auth_handler.go", + "line": 201, + "severity": "low", + "why": "handleRegister hashes the password first so a hashing failure cannot burn an invite (the comment at line 153 states that intent), and CreateUserWithInvite consumes the invite and creates the user in one transaction. The session insert that follows is outside that transaction: when CreateSession fails the handler returns 500 \"failed to create session\", but the user row and the invite use are already committed. The caller sees a failed registration; retrying gets 400 \"invalid invite or credentials\" (username taken, invite exhausted) while a login with the same password succeeds. Same shape on the verify-totp path (OC-0378).", + "repro": "cd Server && go test -count=1 -run 'TestAuthCharacterization_RegisterPolicyAndFailurePaths/session_insert_fails' ./api/ — the row installs a BEFORE INSERT ON sessions trigger, registers with a valid invite, and pins today's outcome: 500, user row present, invite use_count 1.", + "evidence": "Server/api/auth_handler.go:166 uid, err := database.CreateUserWithInvite(...) // commits user + invite use\nServer/api/auth_handler.go:201 if _, err := database.CreateSession(...); err != nil { // 500 after the commit\nServer/api/auth_handler.go:153 // Hash password before consuming the invite so that a hashing failure\n // does not burn a valid invite code.", + "suggestedFix": "Either answer 201 without a token when the session insert fails after the account commit (the account exists; the client logs in), or move the session insert into the CreateUserWithInvite transaction so registration is atomic. Belongs to the AuthService in B3-2/B3-9, not to the handler.", + "status": "open", + "found": "2026-08-29", + "hunt": "b3-1-auth-characterization-2026-08-29", + "lens": "characterization", + "confidence": "high", + "finder": "claude" + }, + { + "id": "OC-0377", + "title": "verify-totp maps a database error while loading the challenged user to 401, indistinguishable from an expired challenge", + "file": "Server/api/totp_handler.go", + "line": 141, + "severity": "low", + "why": "totpChallengeSecret folds `err != nil` from GetUserByID into the same 401 \"invalid or expired two-factor challenge\" that a missing user or missing secret gets. A transient database fault during the second factor therefore reads as a bad challenge: the client drops the partial token and asks the user to log in again, and the attempt has already been recorded against the per-user totp_fail cap by the limiter.Allow call above it. Every sibling path in the slice maps a non-sentinel database error to 5xx (login 500, AuthMiddleware 503) precisely so an outage is not mistaken for a credential failure.", + "repro": "cd Server && go test -count=1 -run 'TestAuthCharacterization_VerifyTOTPFailurePaths/user_lookup_fails' ./api/ — the row renames the users table after the challenge is issued and pins today's 401.", + "evidence": "Server/api/totp_handler.go:140 user, err := database.GetUserByID(r.Context(), challengeUserID)\nServer/api/totp_handler.go:141 if err != nil || user == nil || user.TOTPSecret == nil {\nServer/api/totp_handler.go:142 writeJSON(w, http.StatusUnauthorized, errorResponse{ ... \"invalid or expired two-factor challenge\" })\nServer/api/auth_handler.go:488-497 the login sibling: a non-nil error is a genuine DB failure -> 500 \"login temporarily unavailable\"", + "suggestedFix": "Split the condition: `err != nil` -> 500 INTERNAL_ERROR (\"two-factor verification temporarily unavailable\") without RegisterFailure and with the limiter reservation undone or not made; keep 401 for `user == nil || user.TOTPSecret == nil`. Fix in B3-9 after B3-2 lands, and flip the characterization row with it.", + "status": "open", + "found": "2026-08-29", + "hunt": "b3-1-auth-characterization-2026-08-29", + "lens": "characterization", + "confidence": "high", + "finder": "claude" + }, + { + "id": "OC-0378", + "title": "verify-totp consumes the partial challenge before the session insert, so a store failure forces the user back to the password step", + "file": "Server/api/totp_handler.go", + "line": 107, + "severity": "low", + "why": "handleVerifyTOTP calls partialStore.Consume before issueSession. When CreateSession fails the handler answers 500 \"failed to create session\", but the challenge is already gone (and the code is marked used by VerifyTOTPCodeOnce), so the only way forward is a fresh POST /login with the password. A verified second factor is discarded because of a persistence hiccup that has nothing to do with the credential. Same shape as OC-0376 on the register path.", + "repro": "cd Server && go test -count=1 -run 'TestAuthCharacterization_VerifyTOTPFailurePaths/session_insert_fails' ./api/ — the row installs a BEFORE INSERT ON sessions trigger, verifies a valid code, pins the 500, drops the trigger and pins that the same partial token is now refused with 401.", + "evidence": "Server/api/totp_handler.go:107 if _, ok := partialStore.Consume(partialToken); !ok { // challenge gone here\nServer/api/totp_handler.go:115 token, err := issueSession(r.Context(), database, user.ID, challenge.Device, challenge.IP) // fails after it", + "suggestedFix": "Keep the claim atomic and first: Consume the challenge before issuing the session (as today), then on CreateSession failure re-issue or restore the challenge for the same user/device/IP so the verified second factor is not discarded. The restore must also keep the accepted verification usable: VerifyTOTPCodeOnce has already recorded (user, code) in UsedTOTPCodeStore for 90 s, so an immediate retry with the authenticator's still-current code would be refused as a replay - either carry the verified state on the restored challenge (retry issues the session without a new code) or roll back that MarkUsed claim together with the challenge. Do NOT issue the session before Consume: two concurrent requests holding the same partial token can pass Lookup with different valid codes from the +/-1 step window (the used-code store keys on (user, code), not the token), both would create sessions, and the losing Consume would leave an unreturned bearer session in the database; if the order must change, the loser has to revoke the session it created. Belongs to the AuthService in B3-2/B3-9.", + "status": "open", + "found": "2026-08-29", + "hunt": "b3-1-auth-characterization-2026-08-29", + "lens": "characterization", + "confidence": "high", + "finder": "claude" } ] } diff --git a/Server/api/auth_characterization_test.go b/Server/api/auth_characterization_test.go new file mode 100644 index 00000000..ae15ca72 --- /dev/null +++ b/Server/api/auth_characterization_test.go @@ -0,0 +1,991 @@ +package api_test + +// B3-1 characterization tests for the auth slice (docs/plans/ +// b3-server-architecture-guardrails-2026-08-29.md §B3-1). Every row pins +// TODAY's behaviour of auth_handler.go and totp_handler.go so that B3-2 can +// move the orchestration into service.AuthService without changing it. A row +// that reveals a defect is pinned as-is with a `// known:` comment and a +// ledger entry — it is not fixed here. +// +// Only the gaps in auth_handler_test.go, totp_handler_test.go and +// auth_handler_delete_broadcast_test.go are filled; the inventory of what +// those files already pin is the table in the plan's B3-1 evidence block. +// +// Fault injection uses the database itself, so no handler is mocked: +// - a SELECT fault is `ALTER TABLE x RENAME TO x_gone` (the sqlc query then +// fails with "no such table", a wrapped error that is not a sentinel); +// - a write fault is a BEFORE INSERT/UPDATE/DELETE trigger that RAISE(FAIL)s +// (the pattern TestConfirmTOTP_RevokeFailureSurfacesWarning introduced). + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "maps" + "net/http" + "net/http/httptest" + "regexp" + "slices" + "strings" + "testing" + "time" + + "github.com/J3vb/OwnCord/Server/auth" + "github.com/J3vb/OwnCord/Server/db" + "golang.org/x/crypto/bcrypt" +) + +// ─── Harness ───────────────────────────────────────────────────────────────── + +// errBody is the errorResponse shape every refusal in the slice uses. +type errBody struct { + Error string `json:"error"` + Message string `json:"message"` +} + +// send issues one request against the mounted auth router. token, ip and ua +// are optional; the body is JSON-encoded unless it is already a []byte. +func send(t *testing.T, router http.Handler, method, path, token, ip, ua string, body any) *httptest.ResponseRecorder { + t.Helper() + var raw []byte + switch b := body.(type) { + case nil: + case []byte: + raw = b + default: + var err error + if raw, err = json.Marshal(b); err != nil { + t.Fatalf("marshal body: %v", err) + } + } + req := httptest.NewRequest(method, path, bytes.NewReader(raw)) + req.Header.Set("Content-Type", "application/json") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + if ua != "" { + req.Header.Set("User-Agent", ua) + } + if ip == "" { + ip = "127.0.0.1" + } + req.RemoteAddr = ip + ":9999" + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + return rr +} + +func decodeErr(t *testing.T, rr *httptest.ResponseRecorder) errBody { + t.Helper() + var e errBody + if err := json.Unmarshal(rr.Body.Bytes(), &e); err != nil { + t.Fatalf("decode error body %q: %v", rr.Body.String(), err) + } + return e +} + +// wantErr asserts status plus the exact error code and message. +func wantErr(t *testing.T, rr *httptest.ResponseRecorder, status int, code, message string) { + t.Helper() + if rr.Code != status { + t.Fatalf("status = %d, want %d; body = %s", rr.Code, status, rr.Body.String()) + } + e := decodeErr(t, rr) + if e.Error != code || e.Message != message { + t.Fatalf("body = {%q, %q}, want {%q, %q}", e.Error, e.Message, code, message) + } +} + +// hideTable makes every query against table fail with a non-sentinel error. +func hideTable(t *testing.T, database *db.DB, table string) { + t.Helper() + if _, err := database.ExecContext(context.Background(), "ALTER TABLE "+table+" RENAME TO "+table+"_gone"); err != nil { + t.Fatalf("hide %s: %v", table, err) + } +} + +// failWrite makes every `op` against table fail. op is INSERT, DELETE or +// "UPDATE OF ". +func failWrite(t *testing.T, database *db.DB, op, table string) { + t.Helper() + name := "fault_" + regexp.MustCompile(`[^a-z]+`).ReplaceAllString(strings.ToLower(op+"_"+table), "_") + if _, err := database.ExecContext(context.Background(), + "CREATE TRIGGER "+name+" BEFORE "+op+" ON "+table+" BEGIN SELECT RAISE(FAIL, 'injected fault'); END"); err != nil { + t.Fatalf("install %s: %v", name, err) + } +} + +// seedUser creates a user with the given password and returns its ID. +func seedUser(t *testing.T, database *db.DB, username, password string, roleID int) int64 { + t.Helper() + hash, err := auth.HashPassword(password) + if err != nil { + t.Fatalf("HashPassword: %v", err) + } + uid, err := database.CreateUser(context.Background(), username, hash, roleID) + if err != nil { + t.Fatalf("CreateUser(%s): %v", username, err) + } + return uid +} + +// seedSession creates a login session for uid and returns the bearer token. +func seedSession(t *testing.T, database *db.DB, uid int64) string { + t.Helper() + token, err := auth.GenerateToken() + if err != nil { + t.Fatalf("GenerateToken: %v", err) + } + if _, err := database.CreateSession(context.Background(), uid, auth.HashToken(token), "test", "127.0.0.1"); err != nil { + t.Fatalf("CreateSession: %v", err) + } + return token +} + +func setSetting(t *testing.T, database *db.DB, key, value string) { + t.Helper() + if _, err := database.ExecContext(context.Background(), + `INSERT OR REPLACE INTO settings (key, value) VALUES (?, ?)`, key, value); err != nil { + t.Fatalf("set %s: %v", key, err) + } +} + +func seedInvite(t *testing.T, database *db.DB) string { + t.Helper() + ownerID := seedUser(t, database, "inviteowner", "ownerPass1", 1) + code, err := database.CreateInvite(context.Background(), ownerID, 1, nil) + if err != nil { + t.Fatalf("CreateInvite: %v", err) + } + return code +} + +func inviteUseCount(t *testing.T, database *db.DB, code string) int { + t.Helper() + var n int + if err := database.QueryRowContext(context.Background(), `SELECT use_count FROM invites WHERE code = ?`, code).Scan(&n); err != nil { + t.Fatalf("invite use_count: %v", err) + } + return n +} + +func userByName(t *testing.T, database *db.DB, username string) *db.User { + t.Helper() + u, err := database.GetUserByUsername(context.Background(), username) + if err != nil { + t.Fatalf("GetUserByUsername(%s): %v", username, err) + } + return u +} + +// loginPartial logs in a TOTP-enrolled user and returns the partial token. +func loginPartial(t *testing.T, router http.Handler, username, password, ip, ua string) string { + t.Helper() + rr := send(t, router, http.MethodPost, "/api/v1/auth/login", "", ip, ua, map[string]string{"username": username, "password": password}) + if rr.Code != http.StatusOK { + t.Fatalf("login status = %d, want 200; body = %s", rr.Code, rr.Body.String()) + } + var resp map[string]any + _ = json.Unmarshal(rr.Body.Bytes(), &resp) + pt, _ := resp["partial_token"].(string) + if pt == "" || resp["requires_2fa"] != true { + t.Fatalf("expected a 2FA challenge, got %s", rr.Body.String()) + } + return pt +} + +// enrolTOTP stores secret for uid the way the confirm handler does +// (encrypted with the router's key) and returns the plaintext secret. +func enrolTOTP(t *testing.T, database *db.DB, uid int64) string { + t.Helper() + secret, err := auth.GenerateTOTPSecret() + if err != nil { + t.Fatalf("GenerateTOTPSecret: %v", err) + } + enc, err := auth.EncryptTOTPSecret(testTOTPKey, secret) + if err != nil { + t.Fatalf("EncryptTOTPSecret: %v", err) + } + if err := database.UpdateUserTOTPSecret(context.Background(), uid, &enc); err != nil { + t.Fatalf("UpdateUserTOTPSecret: %v", err) + } + return secret +} + +func totpCode(t *testing.T, secret string) string { + t.Helper() + code, err := auth.GenerateTOTPCode(secret, time.Now().UTC()) + if err != nil { + t.Fatalf("GenerateTOTPCode: %v", err) + } + return code +} + +// wrongTOTPCode returns a six-digit code the verifier rejects for secret: +// it differs from the codes of the previous, current and next 30-second +// steps, which are the three VerifyTOTPCode accepts, and from the step after +// that — the verifier samples the clock later than this helper, so a request +// that crosses a step boundary in between is checked against {0,+1,+2}. A +// constant such as "000000" collides with one of them once in ~333k runs. +func wrongTOTPCode(t *testing.T, secret string) string { + t.Helper() + now := time.Now().UTC() + valid := map[string]bool{} + for _, d := range []time.Duration{-30 * time.Second, 0, 30 * time.Second, 60 * time.Second} { + code, err := auth.GenerateTOTPCode(secret, now.Add(d)) + if err != nil { + t.Fatalf("GenerateTOTPCode: %v", err) + } + valid[code] = true + } + for i := range 5 { + if code := fmt.Sprintf("%06d", i); !valid[code] { + return code + } + } + t.Fatal("unreachable: five candidates, at most four valid codes") + return "" +} + +// ─── Enumeration defence ───────────────────────────────────────────────────── + +// Every credential rejection on /login is the same 401 — status, body, +// Content-Type and rate-limiter side effects — whether the account does not +// exist, exists, is banned, or is spelled in another case. +func TestAuthCharacterization_LoginRejectionsAreIndistinguishable(t *testing.T) { + type outcome struct { + status int + body string + ctype string + windows int + locks int + } + attempt := func(seed func(*db.DB), username string) outcome { + database := newAuthTestDB(t) + seed(database) + limiter := auth.NewRateLimiter() + router := buildAuthRouter(database, limiter) + rr := send(t, router, http.MethodPost, "/api/v1/auth/login", "", "", "", map[string]string{"username": username, "password": "wrongPass1"}) + w, l := limiter.Len() + return outcome{rr.Code, rr.Body.String(), rr.Header().Get("Content-Type"), w, l} + } + none := func(*db.DB) {} + known := func(d *db.DB) { seedUser(t, d, "target", "correctPass1", 4) } + banned := func(d *db.DB) { + uid := seedUser(t, d, "target", "correctPass1", 4) + if err := d.BanUser(context.Background(), uid, "characterization", nil); err != nil { + t.Fatalf("BanUser: %v", err) + } + } + rows := []struct { + name string + seed func(*db.DB) + username string + }{ + {"unknown user", none, "target"}, + {"wrong password", known, "target"}, + {"banned user, wrong password", banned, "target"}, + {"case variant, wrong password", known, "TARGET"}, + } + ref := attempt(rows[0].seed, rows[0].username) + if ref.status != http.StatusUnauthorized { + t.Fatalf("reference status = %d, want 401; body = %s", ref.status, ref.body) + } + var e errBody + if err := json.Unmarshal([]byte(ref.body), &e); err != nil || e.Error != "UNAUTHORIZED" || e.Message != "invalid credentials" { + t.Fatalf("reference body = %s, want UNAUTHORIZED/invalid credentials", ref.body) + } + // One route window ("login:"+ip) plus the two failure windows + // ("login_fail:"+ip, "login_user_fail:"+name); no lockout yet. + if ref.windows != 3 || ref.locks != 0 { + t.Fatalf("reference limiter state = %d windows / %d lockouts, want 3 / 0", ref.windows, ref.locks) + } + for _, row := range rows[1:] { + if got := attempt(row.seed, row.username); got != ref { + t.Errorf("%s: %+v, want byte-identical to unknown user %+v", row.name, got, ref) + } + } +} + +// The timing class of an unknown-user rejection matches a wrong-password +// rejection: auth.CheckPassword runs a dummy bcrypt compare when there is no +// hash. The api suite hashes at bcrypt.MinCost, where the compare is too +// cheap to measure, so this row raises the cost for its own hashes only. +func TestAuthCharacterization_UnknownUserTakesAsLongAsWrongPassword(t *testing.T) { + const cost = 10 // ~50-100 ms per compare: well above scheduler noise + auth.SetCostForTesting(cost) + t.Cleanup(func() { auth.SetCostForTesting(bcrypt.MinCost) }) + + database := newAuthTestDB(t) + seedUser(t, database, "timed", "correctPass1", 4) + + median := func(username string) time.Duration { + samples := make([]time.Duration, 0, 3) + for range 3 { + // A fresh limiter per sample keeps every attempt inside the + // per-IP route limit and the failure windows. + router := buildAuthRouter(database, auth.NewRateLimiter()) + start := time.Now() + rr := send(t, router, http.MethodPost, "/api/v1/auth/login", "", "", "", map[string]string{"username": username, "password": "wrongPass1"}) + samples = append(samples, time.Since(start)) + if rr.Code != http.StatusUnauthorized { + t.Fatalf("%s: status = %d, want 401", username, rr.Code) + } + } + slices.Sort(samples) + return samples[1] + } + // Warm the dummy hash (sync.Once) so its one-off generation is not timed. + auth.CheckPassword("", "warm") + + wrong := median("timed") + unknown := median("nobody") + if unknown < wrong/2 { + t.Fatalf("unknown-user rejection took %v, wrong-password %v: the dummy bcrypt compare is not running on the unknown path", unknown, wrong) + } +} + +// Registration refusals that depend on state an attacker wants to probe — +// a taken username versus an unusable invite — share one 400 body. +func TestAuthCharacterization_RegisterRejectionsAreIndistinguishable(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "taken", "somePass1", 4) + code := seedInvite(t, database) + past := time.Now().Add(-time.Hour) + expired, err := database.CreateInvite(context.Background(), 1, 0, &past) + if err != nil { + t.Fatalf("CreateInvite(expired): %v", err) + } + + // One IP per attempt: the register route allows 3/min per IP. + register := func(username, invite, ip string) *httptest.ResponseRecorder { + return send(t, router, http.MethodPost, "/api/v1/auth/register", "", ip, "", + map[string]string{"username": username, "password": "securePass1", "invite_code": invite}) + } + ref := register("taken", code, "203.0.113.1") + wantErr(t, ref, http.StatusBadRequest, "INVALID_CREDENTIALS", "invalid invite or credentials") + for name, rr := range map[string]*httptest.ResponseRecorder{ + "unknown invite": register("fresh", "no-such-invite", "203.0.113.2"), + "expired invite": register("fresh", expired, "203.0.113.3"), + } { + if rr.Code != ref.Code || rr.Body.String() != ref.Body.String() { + t.Errorf("%s: %d %s, want byte-identical to taken username %d %s", name, rr.Code, rr.Body.String(), ref.Code, ref.Body.String()) + } + } + if n := inviteUseCount(t, database, code); n != 0 { + t.Errorf("invite use_count after rejected registrations = %d, want 0", n) + } +} + +// ─── Sentinel and failure-path mapping: /register ──────────────────────────── + +func TestAuthCharacterization_RegisterPolicyAndFailurePaths(t *testing.T) { + body := func(invite string) map[string]string { + return map[string]string{"username": "fresh", "password": "securePass1", "invite_code": invite} + } + t.Run("settings unreadable -> 500, no policy leak", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + hideTable(t, database, "settings") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load registration policy") + }) + t.Run("registration_open unparsable -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + setSetting(t, database, "registration_open", "maybe") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load registration policy") + }) + t.Run("require_2fa unparsable -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + setSetting(t, database, "require_2fa", "yes") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load registration policy") + }) + t.Run("settings rows absent -> defaults (open, no 2FA) -> 201", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + if _, err := database.ExecContext(context.Background(), `DELETE FROM settings`); err != nil { + t.Fatalf("clear settings: %v", err) + } + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + if rr.Code != http.StatusCreated { + t.Fatalf("status = %d, want 201; body = %s", rr.Code, rr.Body.String()) + } + }) + t.Run("require_2fa true -> 403 before any credential is read", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + setSetting(t, database, "require_2fa", "true") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusForbidden, "FORBIDDEN", "registration is unavailable while two-factor authentication is required") + }) + t.Run("registration closed -> 403 before any credential is read", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + setSetting(t, database, "registration_open", "0") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusForbidden, "FORBIDDEN", "registration is currently closed") + }) + t.Run("user insert fails -> 500, invite not consumed", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + failWrite(t, database, "INSERT", "users") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "registration failed — please try again") + if n := inviteUseCount(t, database, code); n != 0 { + t.Errorf("invite use_count = %d, want 0 (transaction rolled back)", n) + } + if userByName(t, database, "fresh") != nil { + t.Error("user row exists after a failed insert") + } + }) + t.Run("session insert fails after the user is committed -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + failWrite(t, database, "INSERT", "sessions") + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") + // known: the account and the invite use are already committed when the + // session insert fails, so the caller sees a 500 for a registration + // that succeeded — a retry gets "invalid invite or credentials" while + // a login with the same password works (ledger OC-0376). + if userByName(t, database, "fresh") == nil { + t.Error("user row missing: this row pins the partial-success behaviour, which has changed") + } + if n := inviteUseCount(t, database, code); n != 1 { + t.Errorf("invite use_count = %d, want 1 (pinned partial success)", n) + } + }) +} + +// ─── Sentinel and failure-path mapping: /login ─────────────────────────────── + +func TestAuthCharacterization_LoginFailurePaths(t *testing.T) { + login := func(t *testing.T, router http.Handler, password string) *httptest.ResponseRecorder { + t.Helper() + return send(t, router, http.MethodPost, "/api/v1/auth/login", "", "", "", map[string]string{"username": "target", "password": password}) + } + t.Run("user lookup fails -> 500, no attempt recorded", func(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildAuthRouter(database, limiter) + seedUser(t, database, "target", "correctPass1", 4) + hideTable(t, database, "users") + rr := login(t, router, "correctPass1") + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "login temporarily unavailable") + // Only the route's own per-IP window; the failure counters were not + // touched, so an outage cannot lock anyone out. + if w, l := limiter.Len(); w != 1 || l != 0 { + t.Errorf("limiter state = %d windows / %d lockouts, want 1 / 0", w, l) + } + }) + t.Run("policy unreadable after a correct password -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "target", "correctPass1", 4) + hideTable(t, database, "settings") + rr := login(t, router, "correctPass1") + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load authentication policy") + }) + t.Run("policy unreadable with a wrong password -> 401 (credential guard runs first)", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "target", "correctPass1", 4) + hideTable(t, database, "settings") + rr := login(t, router, "wrongPass1") + wantErr(t, rr, http.StatusUnauthorized, "UNAUTHORIZED", "invalid credentials") + }) + t.Run("require_2fa unparsable -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "target", "correctPass1", 4) + setSetting(t, database, "require_2fa", "yes") + rr := login(t, router, "correctPass1") + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load authentication policy") + }) + t.Run("require_2fa parses case-insensitively with whitespace", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "target", "correctPass1", 4) + setSetting(t, database, "require_2fa", " TRUE ") + rr := login(t, router, "correctPass1") + wantErr(t, rr, http.StatusForbidden, "FORBIDDEN", "two-factor authentication must be enabled on this account before login") + }) + t.Run("require_2fa true and enrolled -> challenge, not 403", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "target", "correctPass1", 4) + enrolTOTP(t, database, uid) + setSetting(t, database, "require_2fa", "true") + loginPartial(t, router, "target", "correctPass1", "", "") + }) + t.Run("session insert fails -> 500", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + seedUser(t, database, "target", "correctPass1", 4) + failWrite(t, database, "INSERT", "sessions") + rr := login(t, router, "correctPass1") + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") + }) +} + +// ─── Session issue ─────────────────────────────────────────────────────────── + +func TestAuthCharacterization_SessionShape(t *testing.T) { + hexToken := regexp.MustCompile(`^[0-9a-f]{64}$`) + userKeys := []string{"about", "created_at", "custom_status", "display_name", "id", "role_id", "status", "totp_enabled", "username"} + checkSuccess := func(t *testing.T, rr *httptest.ResponseRecorder, wantStatus int) string { + t.Helper() + if rr.Code != wantStatus { + t.Fatalf("status = %d, want %d; body = %s", rr.Code, wantStatus, rr.Body.String()) + } + var resp map[string]any + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + keys := slices.Sorted(maps.Keys(resp)) + if want := []string{"requires_2fa", "token", "user"}; !slices.Equal(keys, want) { + t.Errorf("response keys = %v, want %v", keys, want) + } + token, _ := resp["token"].(string) + if !hexToken.MatchString(token) { + t.Errorf("token = %q, want 64 lowercase hex chars", token) + } + user, _ := resp["user"].(map[string]any) + got := slices.Sorted(maps.Keys(user)) + if !slices.Equal(got, userKeys) { + t.Errorf("user keys = %v, want %v (avatar omitted when empty; nullable fields present as null)", got, userKeys) + } + for _, k := range []string{"display_name", "about", "custom_status"} { + if user[k] != nil { + t.Errorf("user.%s = %v, want null when unset", k, user[k]) + } + } + return token + } + longUA := strings.Repeat("u", 600) + + t.Run("login issues one bearer session bound to device and IP", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "shape", "correctPass1", 4) + rr := send(t, router, http.MethodPost, "/api/v1/auth/login", "", "203.0.113.5", longUA, map[string]string{"username": "shape", "password": "correctPass1"}) + token := checkSuccess(t, rr, http.StatusOK) + var n int + if err := database.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM sessions WHERE user_id = ?`, uid).Scan(&n); err != nil || n != 1 { + t.Fatalf("session rows = %d (%v), want exactly one", n, err) + } + s, err := database.GetSessionByTokenHash(context.Background(), auth.HashToken(token)) + if err != nil || s == nil { + t.Fatalf("session lookup by the issued token: %v", err) + } + if s.UserID != uid || s.IP != "203.0.113.5" || s.Device != longUA[:512] { + t.Errorf("session = {user %d, ip %q, device len %d}, want {%d, 203.0.113.5, 512}", s.UserID, s.IP, len(s.Device), uid) + } + if me := send(t, router, http.MethodGet, "/api/v1/auth/me", token, "", "", nil); me.Code != http.StatusOK { + t.Errorf("/me with the issued token = %d, want 200", me.Code) + } + }) + t.Run("register issues the same shape with 201", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + code := seedInvite(t, database) + rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "203.0.113.6", longUA, + map[string]string{"username": "shape", "password": "securePass1", "invite_code": code}) + token := checkSuccess(t, rr, http.StatusCreated) + s, err := database.GetSessionByTokenHash(context.Background(), auth.HashToken(token)) + if err != nil || s == nil { + t.Fatalf("session lookup: %v", err) + } + if s.IP != "203.0.113.6" || s.Device != longUA[:512] { + t.Errorf("session = {ip %q, device len %d}, want {203.0.113.6, 512}", s.IP, len(s.Device)) + } + }) + t.Run("verify-totp binds the session to the login request, not the verify request", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "shape", "correctPass1", 4) + secret := enrolTOTP(t, database, uid) + pt := loginPartial(t, router, "shape", "correctPass1", "198.51.100.7", "phone-ua") + rr := send(t, router, http.MethodPost, "/api/v1/auth/verify-totp", pt, "203.0.113.9", "laptop-ua", map[string]string{"code": totpCode(t, secret)}) + token := checkSuccess(t, rr, http.StatusOK) + s, err := database.GetSessionByTokenHash(context.Background(), auth.HashToken(token)) + if err != nil || s == nil { + t.Fatalf("session lookup: %v", err) + } + if s.IP != "198.51.100.7" || s.Device != "phone-ua" { + t.Errorf("session = {ip %q, device %q}, want the challenge's {198.51.100.7, phone-ua}", s.IP, s.Device) + } + }) +} + +// ─── Logout ────────────────────────────────────────────────────────────────── + +func TestAuthCharacterization_Logout(t *testing.T) { + custom := "in a meeting" + setup := func(t *testing.T) (*db.DB, http.Handler, int64, string) { + t.Helper() + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "out", "correctPass1", 4) + if err := database.UpdateUserCustomStatus(context.Background(), uid, &custom); err != nil { + t.Fatalf("UpdateUserCustomStatus: %v", err) + } + return database, router, uid, seedSession(t, database, uid) + } + t.Run("clears the custom status", func(t *testing.T) { + database, router, uid, token := setup(t) + rr := send(t, router, http.MethodPost, "/api/v1/auth/logout", token, "", "", nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("status = %d, want 204; body = %s", rr.Code, rr.Body.String()) + } + u, _ := database.GetUserByID(context.Background(), uid) + if u.CustomStatus != nil { + t.Errorf("custom_status after logout = %q, want cleared", *u.CustomStatus) + } + }) + t.Run("session delete fails -> 500, session survives", func(t *testing.T) { + database, router, _, token := setup(t) + failWrite(t, database, "DELETE", "sessions") + rr := send(t, router, http.MethodPost, "/api/v1/auth/logout", token, "", "", nil) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to logout") + if s, _ := database.GetSessionByTokenHash(context.Background(), auth.HashToken(token)); s == nil { + t.Error("session gone although the delete was refused") + } + }) + t.Run("custom-status clear fails -> still 204, session revoked", func(t *testing.T) { + database, router, uid, token := setup(t) + failWrite(t, database, "UPDATE OF custom_status", "users") + rr := send(t, router, http.MethodPost, "/api/v1/auth/logout", token, "", "", nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("status = %d, want 204 (status clear is best-effort); body = %s", rr.Code, rr.Body.String()) + } + if s, _ := database.GetSessionByTokenHash(context.Background(), auth.HashToken(token)); s != nil { + t.Error("session survived logout") + } + u, _ := database.GetUserByID(context.Background(), uid) + if u.CustomStatus == nil || *u.CustomStatus != custom { + t.Errorf("custom_status = %v, want unchanged %q (the clear was refused)", u.CustomStatus, custom) + } + }) +} + +// ─── Authenticated routes when the token cannot be resolved ────────────────── + +// A database fault while resolving the bearer token is 503, never 401 — a +// 401 would make the client discard a valid session (AuthMiddleware's +// documented reason). Pinned per auth route so the slice cannot change it. +func TestAuthCharacterization_TokenResolutionFaultIs503(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "res", "correctPass1", 4) + token := seedSession(t, database, uid) + hideTable(t, database, "users") + for name, req := range map[string]struct { + method, path string + body any + }{ + "GET /me": {http.MethodGet, "/api/v1/auth/me", nil}, + "POST /logout": {http.MethodPost, "/api/v1/auth/logout", nil}, + "DELETE /account": {http.MethodDelete, "/api/v1/auth/account", map[string]string{"password": "correctPass1"}}, + "POST /totp/enable": {http.MethodPost, "/api/v1/users/me/totp/enable", map[string]string{"password": "correctPass1"}}, + "POST /totp/confirm": {http.MethodPost, "/api/v1/users/me/totp/confirm", map[string]string{"password": "correctPass1", "code": "000000"}}, + "DELETE /users/me/totp": {http.MethodDelete, "/api/v1/users/me/totp", map[string]string{"password": "correctPass1"}}, + } { + rr := send(t, router, req.method, req.path, token, "", "", req.body) + if rr.Code != http.StatusServiceUnavailable { + t.Errorf("%s: status = %d, want 503; body = %s", name, rr.Code, rr.Body.String()) + continue + } + if e := decodeErr(t, rr); e.Error != "SERVICE_UNAVAILABLE" || e.Message != "authentication service temporarily unavailable" { + t.Errorf("%s: body = %+v", name, e) + } + } +} + +// ─── DELETE /account ───────────────────────────────────────────────────────── + +func TestAuthCharacterization_DeleteAccountFailurePaths(t *testing.T) { + t.Run("purge fails -> 500, account and session intact", func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "gone", "correctPass1", 4) + token := seedSession(t, database, uid) + failWrite(t, database, "DELETE", "sessions") + rr := send(t, router, http.MethodDelete, "/api/v1/auth/account", token, "", "", map[string]string{"password": "correctPass1"}) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to delete account") + u, _ := database.GetUserByID(context.Background(), uid) + if u == nil || u.Banned || u.Username != "gone" { + t.Errorf("user after failed delete = %+v, want untouched", u) + } + if me := send(t, router, http.MethodGet, "/api/v1/auth/me", token, "", "", nil); me.Code != http.StatusOK { + t.Errorf("/me after failed delete = %d, want 200 (session must survive the rolled-back transaction)", me.Code) + } + }) + t.Run("malformed body -> 400 before the password is checked", func(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildAuthRouter(database, limiter) + uid := seedUser(t, database, "gone", "correctPass1", 4) + token := seedSession(t, database, uid) + rr := send(t, router, http.MethodDelete, "/api/v1/auth/account", token, "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "malformed request body") + // Route window only — a malformed body is not a failed attempt. + if w, l := limiter.Len(); w != 1 || l != 0 { + t.Errorf("limiter state = %d windows / %d lockouts, want 1 / 0", w, l) + } + }) + t.Run("wrong password -> 400 incorrect password, counted", func(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildAuthRouter(database, limiter) + uid := seedUser(t, database, "gone", "correctPass1", 4) + token := seedSession(t, database, uid) + rr := send(t, router, http.MethodDelete, "/api/v1/auth/account", token, "", "", map[string]string{"password": "wrongPass1"}) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "incorrect password") + if w, _ := limiter.Len(); w != 2 { + t.Errorf("limiter windows = %d, want 2 (route + delete_fail)", w) + } + }) +} + +// ─── POST /verify-totp ─────────────────────────────────────────────────────── + +func TestAuthCharacterization_VerifyTOTPFailurePaths(t *testing.T) { + setup := func(t *testing.T) (*db.DB, http.Handler, int64, string) { + t.Helper() + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "two", "correctPass1", 4) + secret := enrolTOTP(t, database, uid) + return database, router, uid, secret + } + verify := func(t *testing.T, router http.Handler, pt, code, ip string) *httptest.ResponseRecorder { + t.Helper() + return send(t, router, http.MethodPost, "/api/v1/auth/verify-totp", pt, ip, "", map[string]string{"code": code}) + } + const challengeGone = "invalid or expired two-factor challenge" + + t.Run("user lookup fails -> 401", func(t *testing.T) { + database, router, _, secret := setup(t) + pt := loginPartial(t, router, "two", "correctPass1", "", "") + hideTable(t, database, "users") + // known: totpChallengeSecret folds a database error into the same 401 + // an expired challenge gets, so a DB fault during the second factor + // reads as a bad code and burns the caller's attempt budget (ledger + // OC-0377). The plan's rule is 5xx for a non-sentinel error; pinned + // as-is for B3-2, fixed in B3-9. + wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone) + }) + t.Run("secret removed after the challenge was issued -> 401", func(t *testing.T) { + database, router, uid, secret := setup(t) + pt := loginPartial(t, router, "two", "correctPass1", "", "") + if err := database.UpdateUserTOTPSecret(context.Background(), uid, nil); err != nil { + t.Fatalf("clear secret: %v", err) + } + wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone) + }) + t.Run("secret encrypted under another key -> 500", func(t *testing.T) { + database, router, uid, secret := setup(t) + pt := loginPartial(t, router, "two", "correctPass1", "", "") + otherKey := bytes.Repeat([]byte{7}, 32) + enc, err := auth.EncryptTOTPSecret(otherKey, secret) + if err != nil { + t.Fatalf("EncryptTOTPSecret: %v", err) + } + if err := database.UpdateUserTOTPSecret(context.Background(), uid, &enc); err != nil { + t.Fatalf("store foreign secret: %v", err) + } + wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to verify two-factor code") + }) + t.Run("session insert fails -> 500 and the challenge is consumed", func(t *testing.T) { + database, router, _, secret := setup(t) + pt := loginPartial(t, router, "two", "correctPass1", "", "") + failWrite(t, database, "INSERT", "sessions") + wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") + // known: the partial token was consumed before the session insert, so + // the user must repeat the password step after a transient store + // failure (ledger OC-0378). Pinned as-is; the code is also marked used. + if _, err := database.ExecContext(context.Background(), `DROP TRIGGER fault_insert_sessions`); err != nil { + t.Fatalf("drop trigger: %v", err) + } + wantErr(t, verify(t, router, pt, totpCode(t, secret), "203.0.113.2"), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone) + }) + t.Run("per-user failure cap spans challenges -> 429 on the 11th attempt", func(t *testing.T) { + _, router, _, secret := setup(t) + attempt := 0 + for range 2 { + pt := loginPartial(t, router, "two", "correctPass1", "", "") + // partialAuthMaxFailures (5) consumes the challenge; the per-user + // totp_fail counter (10) keeps counting across challenges. + for range 5 { + attempt++ + rr := verify(t, router, pt, wrongTOTPCode(t, secret), fmt.Sprintf("203.0.113.%d", attempt)) + wantErr(t, rr, http.StatusUnauthorized, "UNAUTHORIZED", "invalid two-factor code") + } + } + pt := loginPartial(t, router, "two", "correctPass1", "", "") + rr := verify(t, router, pt, totpCode(t, secret), "203.0.113.99") + wantErr(t, rr, http.StatusTooManyRequests, "RATE_LIMITED", "too many failed attempts, try again later") + }) +} + +// ─── TOTP management: /enable, /confirm, DELETE /users/me/totp ─────────────── + +func TestAuthCharacterization_TOTPManagementFailurePaths(t *testing.T) { + setup := func(t *testing.T) (*db.DB, http.Handler, int64, string) { + t.Helper() + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + uid := seedUser(t, database, "mgmt", "correctPass1", 4) + return database, router, uid, seedSession(t, database, uid) + } + pw := map[string]string{"password": "correctPass1"} + + t.Run("enable while enrolled -> 409", func(t *testing.T) { + database, router, uid, token := setup(t) + enrolTOTP(t, database, uid) + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, "", "", pw) + wantErr(t, rr, http.StatusConflict, "TOTP_ALREADY_ENABLED", "disable 2FA before re-enabling") + }) + t.Run("enable with malformed body -> 400", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "malformed request body") + }) + t.Run("enable with empty password -> 400 password is required", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, "", "", map[string]string{}) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "password is required") + }) + t.Run("password-confirmation lockout is shared by enable, confirm and disable", func(t *testing.T) { + _, router, _, token := setup(t) + // pwConfirmFailureThreshold (3) failures are recorded; the 4th trips + // the lockout while still answering 400. Distinct IPs keep the shared + // "totp:" route window (5/min per IP) out of the picture. + for i := range 4 { + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, fmt.Sprintf("203.0.113.%d", i+1), "", map[string]string{"password": "wrongPass1"}) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "password confirmation failed") + } + const locked = "too many failed attempts, try again later" + wantErr(t, send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, "203.0.113.11", "", pw), http.StatusTooManyRequests, "RATE_LIMITED", locked) + wantErr(t, send(t, router, http.MethodPost, "/api/v1/users/me/totp/confirm", token, "203.0.113.12", "", map[string]string{"password": "correctPass1", "code": "000000"}), http.StatusTooManyRequests, "RATE_LIMITED", locked) + wantErr(t, send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "203.0.113.13", "", pw), http.StatusTooManyRequests, "RATE_LIMITED", locked) + }) + t.Run("confirm: secret persist fails -> 500, nothing enrolled", func(t *testing.T) { + database, router, uid, token := setup(t) + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/enable", token, "", "", pw) + if rr.Code != http.StatusOK { + t.Fatalf("enable: %d %s", rr.Code, rr.Body.String()) + } + var enable map[string]any + _ = json.Unmarshal(rr.Body.Bytes(), &enable) + secret := extractSecretFromURI(t, enable["qr_uri"].(string)) + failWrite(t, database, "UPDATE OF totp_secret", "users") + rr = send(t, router, http.MethodPost, "/api/v1/users/me/totp/confirm", token, "", "", map[string]string{"password": "correctPass1", "code": totpCode(t, secret)}) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to enable two-factor authentication") + if u, _ := database.GetUserByID(context.Background(), uid); u.TOTPSecret != nil { + t.Error("secret persisted although the update was refused") + } + }) + t.Run("confirm with malformed body -> 400", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodPost, "/api/v1/users/me/totp/confirm", token, "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "malformed request body") + }) + t.Run("disable: policy unreadable -> 500, still enrolled", func(t *testing.T) { + database, router, uid, token := setup(t) + enrolTOTP(t, database, uid) + hideTable(t, database, "settings") + rr := send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "", "", pw) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to load authentication policy") + if u, _ := database.GetUserByID(context.Background(), uid); u.TOTPSecret == nil { + t.Error("secret cleared although the policy read failed") + } + }) + t.Run("disable: secret clear fails -> 500, still enrolled", func(t *testing.T) { + database, router, uid, token := setup(t) + enrolTOTP(t, database, uid) + failWrite(t, database, "UPDATE OF totp_secret", "users") + rr := send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "", "", pw) + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to disable two-factor authentication") + if u, _ := database.GetUserByID(context.Background(), uid); u.TOTPSecret == nil { + t.Error("secret cleared although the update was refused") + } + }) + t.Run("disable when not enrolled -> 204 (idempotent)", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "", "", pw) + if rr.Code != http.StatusNoContent { + t.Fatalf("status = %d, want 204; body = %s", rr.Code, rr.Body.String()) + } + }) + t.Run("disable with an empty body -> 400 password is required", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "", "", nil) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "password is required") + }) + t.Run("disable with a malformed body -> 400", func(t *testing.T) { + _, router, _, token := setup(t) + rr := send(t, router, http.MethodDelete, "/api/v1/users/me/totp", token, "", "", []byte(`{not json`)) + wantErr(t, rr, http.StatusBadRequest, "INVALID_INPUT", "malformed request body") + }) +} + +// ─── Route-level rate limits ───────────────────────────────────────────────── + +// The per-IP RateLimitMiddleware on each auth route: N requests per minute +// pass, the N+1th is 429 with Retry-After, regardless of the credentials. +func TestAuthCharacterization_RouteRateLimits(t *testing.T) { + const slowDown = "too many requests, please slow down" + rows := []struct { + name string + limit int + setup func(t *testing.T, database *db.DB) (token string) + method string + path string + body any + want int // status of the requests inside the limit + }{ + {"login 5/min", 5, func(*testing.T, *db.DB) string { return "" }, + http.MethodPost, "/api/v1/auth/login", map[string]string{"username": "nobody", "password": "wrongPass1"}, http.StatusUnauthorized}, + {"verify-totp 10/min", 10, func(*testing.T, *db.DB) string { return "bogus" }, + http.MethodPost, "/api/v1/auth/verify-totp", map[string]string{"code": "000000"}, http.StatusUnauthorized}, + {"delete account 5/min", 5, func(t *testing.T, d *db.DB) string { return seedSession(t, d, seedUser(t, d, "rl", "correctPass1", 4)) }, + http.MethodDelete, "/api/v1/auth/account", map[string]string{}, http.StatusBadRequest}, + {"totp management 5/min shared across the three routes", 5, func(t *testing.T, d *db.DB) string { return seedSession(t, d, seedUser(t, d, "rl", "correctPass1", 4)) }, + http.MethodPost, "/api/v1/users/me/totp/enable", []byte(`{not json`), http.StatusBadRequest}, + } + for _, row := range rows { + t.Run(row.name, func(t *testing.T) { + database := newAuthTestDB(t) + router := buildAuthRouter(database, auth.NewRateLimiter()) + token := row.setup(t, database) + for i := range row.limit { + if rr := send(t, router, row.method, row.path, token, "", "", row.body); rr.Code != row.want { + t.Fatalf("request %d: status = %d, want %d; body = %s", i+1, rr.Code, row.want, rr.Body.String()) + } + } + path := row.path + if strings.Contains(path, "/totp/enable") { + path = "/api/v1/users/me/totp/confirm" // the sibling shares the "totp:" window + } + rr := send(t, router, row.method, path, token, "", "", row.body) + wantErr(t, rr, http.StatusTooManyRequests, "RATE_LIMITED", slowDown) + if ra := rr.Header().Get("Retry-After"); ra != "60" { + t.Errorf("Retry-After = %q, want 60", ra) + } + }) + } +} diff --git a/docs/plans/README.md b/docs/plans/README.md index 6050bc2d..70bdf896 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -66,7 +66,7 @@ Planning documents are not trackers. Do not read a defect count out of one. | Phase order and gates | [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) | | Current measured baseline | [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) | -Ledger at 2026-08-29: **315 fixed / 56 open / 3 declined / 1 duplicate = 375**. +Ledger at 2026-08-29: **315 fixed / 59 open / 3 declined / 1 duplicate = 378**. All 38 open records still resolved to a live `file:line` at `5cc0888964e26276d1aca145e83270a2c1b9febd` when that sweep was run — it was a manual pass, not something a command reproduces. What the tooling does check: diff --git a/docs/plans/b0-baseline-2026-08-25.md b/docs/plans/b0-baseline-2026-08-25.md index b349809a..6f9cac4b 100644 --- a/docs/plans/b0-baseline-2026-08-25.md +++ b/docs/plans/b0-baseline-2026-08-25.md @@ -216,7 +216,7 @@ authority over the leftovers listed below. B1 is unblocked. which also refutes its own header note: repository-settings writes were **not** blocked from the agent sandbox. - Step 8: individual adjudication of the 38 open `OC-*` records. The count was - verified as **315 fixed / 56 open / 3 declined / 1 duplicate = 375**, matching + verified as **315 fixed / 59 open / 3 declined / 1 duplicate = 378**, matching the register, and a staleness pass confirmed **all 38 still resolve to a live `file:line`** at this commit — none is superseded by later work, so all 38 are genuinely open (11 medium, 27 low, all from hunt `general-2026-08-22-b`). diff --git a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md index 900bb171..d57a5a4b 100644 --- a/docs/plans/b3-server-architecture-guardrails-2026-08-29.md +++ b/docs/plans/b3-server-architecture-guardrails-2026-08-29.md @@ -5,7 +5,8 @@ ([hp-2-scorecard-2026-08-29.md](hp-2-scorecard-2026-08-29.md)) — claims verified at `bf7b886d` **Status:** in progress — plan merged 2026-08-29 (PR #1447 = `ad4defc2`); -B3-0 in review 2026-08-29 (evidence in its section; closes entry-gate item 3). +B3-0 merged 2026-08-29 (PR #1448 = `d383d8c7`; closes entry-gate item 3); +B3-1 in progress 2026-08-29. Update this line, not only the step table, when a step lands. Primary inputs: @@ -30,19 +31,19 @@ surface to it. ## Steps at a glance -| Step | What | Size | Parallel with | -| -------- | ----------------------------------------------------------------------------------------------------- | -------- | ---------------------- | -| **B3-0** | Boundary inventory: every upper-layer `db` import with a disposition; hub lifecycle; before-graph | 1–2 days | B3-6, B3-7 | -| **B3-1** | Auth characterization tests — enumeration, sentinels, sessions, TOTP, rate limits, failure paths | 1 day | B3-6, B3-7 | -| **B3-2** | The auth vertical slice (S-10): route → `service.AuthService` → `db`, behaviour-neutral | 2–3 days | B3-6, B3-7 | -| **HP-3** | First vertical-slice review — scorecard | — | — | -| **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract | 1–2 days | B3-4 | -| **B3-4** | Hub constructor options (S-11): required collaborators validated at construction | 1 day | after B3-3 | -| **B3-5** | `ws` in-package split (S-08): responsibilities into named files, pure moves + adjacent rewrites | 2–3 days | after B3-3/B3-4 | -| **B3-6** | Guardrails: coverage floor (S-06), hub simulation + fault transport + fuzz seeds, benchmarks, rules | 3–4 days | B3-0..B3-2 | -| **B3-7** | Alpha-shaped test dataset: seed profile + anonymised `v1.2.0-alpha.4` snapshot | 1–2 days | B3-0..B3-2 | -| **B3-8** | Remaining domain families behind services (S-09), one PR each; S-03/S-04 fold into the channel family | spread | after HP-3, per-family | -| **B3-9** | The B3-tagged findings: OC-0323, OC-0345, OC-0346 (test-first, `bughunt-fix` shape) | 1 day | any | +| Step | What | Size | Parallel with | +| -------- | ---------------------------------------------------------------------------------------------------------------------------------- | -------- | ---------------------------------------------------------------- | +| **B3-0** | Boundary inventory: every upper-layer `db` import with a disposition; hub lifecycle; before-graph — **DONE 2026-08-29 (PR #1448)** | 1–2 days | B3-6, B3-7 | +| **B3-1** | Auth characterization tests — enumeration, sentinels, sessions, TOTP, rate limits, failure paths | 1 day | B3-6, B3-7 | +| **B3-2** | The auth vertical slice (S-10): route → `service.AuthService` → `db`, behaviour-neutral | 2–3 days | B3-6, B3-7 | +| **HP-3** | First vertical-slice review — scorecard | — | — | +| **B3-3** | Lifecycle extraction: `main.go` → `internal/app/` with one composite close contract | 1–2 days | B3-4 | +| **B3-4** | Hub constructor options (S-11): required collaborators validated at construction | 1 day | after B3-3 | +| **B3-5** | `ws` in-package split (S-08): responsibilities into named files, pure moves + adjacent rewrites | 2–3 days | after B3-3/B3-4 | +| **B3-6** | Guardrails: coverage floor (S-06), hub simulation + fault transport + fuzz seeds, benchmarks, rules | 3–4 days | B3-0..B3-2 | +| **B3-7** | Alpha-shaped test dataset: seed profile + anonymised `v1.2.0-alpha.4` snapshot | 1–2 days | B3-0..B3-2 | +| **B3-8** | Remaining domain families behind services (S-09), one PR each; S-03/S-04 fold into the channel family | spread | after HP-3, per-family | +| **B3-9** | The B3-tagged findings: OC-0323, OC-0345, OC-0346 + B3-1's OC-0376, OC-0377, OC-0378 (test-first, `bughunt-fix` shape) | 1 day | OC-0345/0346: any; OC-0323: with B3-8; OC-0376..0378: after B3-2 | Order: B3-0 → B3-1 → B3-2 → **HP-3** → B3-3 → B3-4 → B3-5 → B3-8. B3-6, B3-7 and B3-9 run beside the slice (roadmap "Safe parallelism": guardrail tooling @@ -138,7 +139,8 @@ red on a synthetic violation; the graph table exists for the three auth files. One PR. **Evidence, 2026-08-29** — branch `feat/b3-0-boundary-inventory` from `dev` -`ad4defc2`; PR to `dev` recorded below. Closes entry-gate item 3. +`ad4defc2`; PR #1448 to `dev`, squash-merged 2026-08-29 as `d383d8c7`. +Closes entry-gate item 3. - **Inventory:** `docs/architecture/server-boundaries.md`, linked from `docs/architecture/server.md` §D2 and `docs/architecture/README.md`. The @@ -230,6 +232,98 @@ diff is reviewable against a frozen behaviour set. Exit: the characterization file is green on HEAD; its row count and the two files' coverage are in the evidence block. One PR. +**Evidence, 2026-08-29** — branch `feat/b3-1-auth-characterization` from `dev` +`d383d8c7`; PR to `dev` recorded below. + +- **Inventory.** The three existing files hold **85** tests (`auth_handler_test.go` + 61, `totp_handler_test.go` 22, `auth_handler_delete_broadcast_test.go` 2 — + the plan's "82" was a miscount). Route × property, with the test that pins + it today or `GAP` and the `TestAuthCharacterization_*` row that fills it. + Two of the plan's words do not exist in the code and are read as their + nearest state: there is no "disabled" account (banned is the analogue — a + banned user with the right password is 403 by design, `TestLogin_BannedUser`, + and with the wrong password is the generic 401), and there is no + `db.ErrDisabled` (the sentinels the slice sees are `ErrNotFound`, the UNIQUE + violation, `ErrLastAdmin`, and `(nil, nil)` for a missing user). + + | Route | Property | Pinned today by | GAP → filled by | + | ----------------------------- | ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | + | `POST /register` | enumeration: taken username vs unknown/expired invite share one body | `TestRegister_ErrorNeverRevealUsername` (substring only) | GAP → `RegisterRejectionsAreIndistinguishable` (byte-identical 400 body, invite `use_count` stays 0) | + | `POST /register` | sentinels: `ErrNotFound` (invite) → 400, UNIQUE → 400, other → 500 | `TestRegister_InvalidInvite`, `_InviteUsedUp`, `_ExpiredInvite`, `_DuplicateUsername_DoesNotConsumeInvite` (status only) | GAP (other → 500, invite not consumed) → `RegisterPolicyAndFailurePaths/user insert fails` | + | `POST /register` | policy: `registration_open`, `require_2fa`, unparsable value, missing rows → defaults | `TestRegister_RegistrationClosed` (403) | GAP → `RegisterPolicyAndFailurePaths/{settings unreadable, registration_open unparsable, require_2fa unparsable, settings rows absent, require_2fa true, registration closed}` | + | `POST /register` | session issue: token shape, `user` shape, session row device/IP | `TestRegister_Success` (token + user present), `TestRegister_UsesTrustedForwardedIP` (session IP) | GAP (exact key set, 64-hex token, device truncated at 512) → `SessionShape/register` | + | `POST /register` | failure path: session insert fails after the account commit | — | GAP → `RegisterPolicyAndFailurePaths/session insert fails` — `// known:` OC-0376 (500, user committed, invite burned) | + | `POST /register` | rate limit: 3/min per IP | `TestRegister_RateLimit` | — | + | `POST /register` | input: oversized username before sanitizing, HTML escaping, weak password, missing fields | `TestRegister_OversizedUsernameRejectedBeforeSanitizing`, `_UsernameNotHTMLEscaped`, `_WeakPassword`, `_MissingFields` | — | + | `POST /login` | enumeration: unknown vs wrong password vs banned+wrong vs case variant → identical 401 | `TestLogin_UnknownUser`, `_WrongPassword` (status), `_GenericErrorOnBadCredentials` (substring) | GAP → `LoginRejectionsAreIndistinguishable` (status, body, Content-Type and limiter state all equal) | + | `POST /login` | timing class: the dummy bcrypt compare on the unknown-user path | `auth/password_test.go` `TestCheckPassword_EmptyHashTimingResistance` asserts the return value only; the api suite hashes at `bcrypt.MinCost`, where the compare is unmeasurable | GAP → `UnknownUserTakesAsLongAsWrongPassword` (cost 10 inside the row, median of 3, unknown ≥ ½ wrong) | + | `POST /login` | sentinels: `(nil, nil)` → 401; wrapped DB error → 500 with no attempt recorded | — | GAP → `LoginFailurePaths/user lookup fails` (500 `login temporarily unavailable`, limiter holds only the route window) | + | `POST /login` | policy: `require_2fa` unparsable → 500; case-insensitive parse; enrolled + required → challenge | `TestLogin_Require2FASettingRejectsUsersWithoutEnrollment` (403) | GAP → `LoginFailurePaths/{policy unreadable after a correct password, policy unreadable with a wrong password, require_2fa unparsable, require_2fa parses case-insensitively, require_2fa true and enrolled}` | + | `POST /login` | session issue: one row, device = UA truncated to 512, IP, token 64 hex, exact `user` key set | `TestLogin_Success` (token present) | GAP → `SessionShape/login`, `LoginFailurePaths/session insert fails` (500) | + | `POST /login` | banned with the right password → 403 | `TestLogin_BannedUser` | — | + | `POST /login` | rate limits: per-username lockout (cross-IP, case-folded, concurrent burst, reset on success) | `TestLogin_UsernameLockoutAcrossDifferentIPs`, `_UsernameLockoutBlocksCorrectPasswordFromFreshIP`, `_UsernameLockoutIgnoresCasing`, `_ConcurrentBurstCannotExceedUsernameBudget`, `_NineFailuresThenCorrectPasswordSucceeds`, `_SuccessResetsUsernameFailureCounter` | — | + | `POST /login` | rate limits: route 5/min per IP | — (`TestLogin_LockoutUsesTrustedForwardedIP` only proves a second client is not locked) | GAP → `RouteRateLimits/login` (6th request 429 `Retry-After: 60`). The deeper `login_lock:`+ip lockout (10 failures) sits behind this 5/min window inside one minute and is not reachable over HTTP in a test; the limiter itself is covered in `auth/` | + | `POST /login` | rate limits: persisted lockout | `auth/ratelimit_persist_test.go`, `db/lockout_queries_test.go` (`NewPersistentRateLimiter`; the api suite mounts the in-memory `NewRateLimiter`) | — | + | `POST /login` | input: password not trimmed, username trimmed, oversized username before keys, missing fields | `TestLogin_PasswordWith*Space*`, `_UsernameIsStillTrimmed`, `_OversizedUsernameRejectedBeforeRateLimiterKey`, `_MissingFields` | — | + | `POST /logout` | revoke: session deleted server-side, second logout 401 | `TestLogout_Success`, `_SessionGoneAfterLogout`, `_NoAuth`, `_InvalidToken` | — | + | `POST /logout` | side effect: custom status cleared; clear failure is best-effort | — | GAP → `Logout/{clears the custom status, custom-status clear fails}` | + | `POST /logout` | failure path: session delete fails → 500, session survives | — | GAP → `Logout/session delete fails` | + | `GET /me` | shape and auth | `TestMe_Success`, `_ReturnsCorrectUserFields`, `_NoAuth`, `_InvalidToken` | — | + | every authenticated route | failure path: token resolution hits a DB error → 503, never 401 | `coverage_push_test.go` (middleware, one route outside the slice) | GAP → `TokenResolutionFaultIs503` (`/me`, `/logout`, `/account`, `/totp/enable`, `/totp/confirm`, `DELETE /users/me/totp`) | + | `DELETE /account` | sentinels: `ErrLastAdmin` → 403; other → 500 with the transaction rolled back | `TestDeleteAccount_LastAdmin`, `_Success` (anonymised, banned, session gone), `_BroadcastsMemberBan`, `_NoBroadcasterOmitted` | GAP (other → 500) → `DeleteAccountFailurePaths/purge fails` (user intact, session still answers `/me`) | + | `DELETE /account` | password confirmation, lockout after 3 failures, malformed body not counted | `TestDeleteAccount_MissingPassword`, `_WrongPassword`, `_LockoutAfterRepeatedFailures`, `_NoAuth` | GAP (exact bodies, limiter accounting) → `DeleteAccountFailurePaths/{malformed body, wrong password}` | + | `DELETE /account` | rate limit: route 5/min per IP | — | GAP → `RouteRateLimits/delete account` | + | `POST /verify-totp` | partial-login store: challenge shape, consumed after 5 failures, replay, banned mid-window | `TestLogin_RequiresTOTPChallenge`, `TestVerifyTotp_Success`, `_ConsumesChallengeAfterRepeatedFailures`, `TestVerifyTOTP_{InvalidCode, MissingToken, InvalidPartialToken, MalformedBody, ReplayProtection, BannedAfterPasswordStep}`, `TestLogin_UsernameLockoutBlocksTOTPChallenge` | — | + | `POST /verify-totp` | sentinels: secret removed → 401; undecryptable secret → 500 | — | GAP → `VerifyTOTPFailurePaths/{secret removed, secret encrypted under another key}` | + | `POST /verify-totp` | failure path: user lookup DB error | — | GAP → `VerifyTOTPFailurePaths/user lookup fails` — `// known:` OC-0377 (401 today, 5xx by the rule) | + | `POST /verify-totp` | failure path: session insert fails | — | GAP → `VerifyTOTPFailurePaths/session insert fails` — `// known:` OC-0378 (500, challenge already consumed) | + | `POST /verify-totp` | session issue: bound to the login request's device/IP, not the verify request's | — | GAP → `SessionShape/verify-totp` | + | `POST /verify-totp` | rate limits: per-user `totp_fail` cap (10, unscaled) across challenges; route 10/min per IP | — | GAP → `VerifyTOTPFailurePaths/per-user failure cap spans challenges`, `RouteRateLimits/verify-totp` | + | `POST /users/me/totp/enable` | enrol: pending secret not persisted, password confirmation, unauthenticated | `TestEnableConfirmDisableTotp`, `TestTOTPManagement_RequiresPasswordConfirmation`, `TestEnableTOTP_{Success, WrongPassword, Unauthenticated}` | GAP → `TOTPManagementFailurePaths/{enable while enrolled → 409, enable with malformed body, enable with empty password}` | + | `POST /users/me/totp/confirm` | verify + persist, revoke other sessions, API-token principal, revoke failure → 200 + warning | `TestConfirmTOTP_{Success, InvalidCode, InvalidCode_Handler, NoPendingSecret, NoPendingEnrollment, MissingPassword, WrongPassword, WrongPassword_Handler, NoAuth, APITokenPrincipal_RevokesAllSessions, RevokeFailureSurfacesWarning}` | GAP (persist fails → 500) → `TOTPManagementFailurePaths/{confirm: secret persist fails, confirm with malformed body}` | + | `DELETE /users/me/totp` | disable, `require_2fa` blocks, API-token principal, revoke failure → 200 + warning | `TestDisableTOTP_{Success, WrongPassword, WrongPassword_Handler, Require2FABlocksDisable, BlockedByServerPolicy, NoAuth, Unauthenticated, APITokenPrincipal_RevokesAllSessions, RevokeFailureSurfacesWarning}` | GAP → `TOTPManagementFailurePaths/{disable: policy unreadable → 500, disable: secret clear fails → 500, disable when not enrolled → 204, disable with an empty body → 400, disable with a malformed body → 400}` | + | the three TOTP routes | rate limits: shared `pw_confirm` lockout (3 failures); shared `totp:` route window 5/min | — | GAP → `TOTPManagementFailurePaths/password-confirmation lockout is shared`, `RouteRateLimits/totp management` | + +- Not injectable without a mock seam, left unpinned on purpose: `GetUserByID` + failing _after_ the register commit (`handleRegister`, "registration + succeeded but user fetch failed"), `auth.GenerateToken`/`HashPassword` + failing (crypto/rand), and the partial-token TTL expiry (no clock on the + store). B3-2's service interface is where a seam for these would go; they are + not behaviour the slice moves. +- **Characterization file:** `Server/api/auth_characterization_test.go` — + **12 tests, 44 table rows** (56 `PASS` lines under `-v`), green on HEAD and + under `-race`. Fault injection is the database itself (`ALTER TABLE x RENAME +TO x_gone` for read faults, `RAISE(FAIL)` triggers for write faults), so no + handler is mocked. Three `// known:` rows → ledger **OC-0376**, **OC-0377**, + **OC-0378** (all `low`, open, B3-9). Because a pinning test is green by + construction, three mutations stood in for RED: `401→500` in + `totpChallengeSecret` (2 rows RED), `500→401` in `loginAuthenticate` (1 row + RED), `503→401` in `AuthMiddleware` (1 test RED); tree restored. +- **Coverage** (`go test -coverprofile=cover.out ./api/` from `Server/`, statements, filtered to the two + files — B3-2 must not drop it): + + | File | Before (`d383d8c7`) | After B3-1 | + | --------------------- | ------------------- | ------------------- | + | `api/auth_handler.go` | 198/254 = **78.0%** | 229/254 = **90.2%** | + | `api/totp_handler.go` | 141/179 = **78.8%** | 163/179 = **91.1%** | + | `api` package | 83.5% | 85.8% | + + Functions still short of 100%: `handleRegister` 80.6% (the post-commit + `GetUserByID` failure and `GenerateToken` failure, not injectable), + `registerReadRequest`/`loginReadRequest` (invalid-username and + password-strength branches are `auth/` tests), `handleMe` 66.7% (the + no-principal branch is unreachable behind `AuthMiddleware`), `issueSession` + 83.3% (`GenerateToken` failure). + +- **Pre-squash SHAs:** `659c8cbd` (B3-0 SHA recorded), `0905a942` (inventory + table), `b7317d03` (characterization file + ledger + claim updates), plus the + coverage commit that carries this bullet. +- Gates before every commit: `check:docs`, `check:hygiene`; from `Server/` the + four build-tag variants, `go vet`, `go test -race ./...`, `go test -tags +deadlock ./ws/`, `golangci-lint run` (first run tripped `prealloc` and + `modernize` on the timing row — fixed), `sqlc generate` and `genprotocol` + drift checks. + ## B3-2 — The auth vertical slice (S-10) Layout-refactor Phase 2 items 2–6; the HP-3 subject. One PR, commits in the @@ -505,8 +599,15 @@ direct database use above the domain layer is justified or removed". Roadmap rule 2. `bughunt-fix` on OC-0345 (owner middleware role re-read → reuse the authenticated context; unavailable stays 503, not 403) and OC-0346 (recovery middleware ordered after tracing so the panic log carries the -trace id). OC-0323 rides B3-8's message/read-state family. Any of the three -that cannot land in B3 is re-tagged in HP-3's scorecard with the reason. +trace id). OC-0323 rides B3-8's message/read-state family. B3-1 added three +more, all in the auth slice and all pinned as-is by its characterization +rows: OC-0376 (register commits the account and burns the invite, then 500s +when the session insert fails), OC-0377 (verify-totp maps a database error to +401), OC-0378 (verify-totp consumes the challenge before the session insert). +They are fixed here, after B3-2 has moved the orchestration into +`service.AuthService` — each fix flips its `// known:` row in the same commit. +Any of the six that cannot land in B3 is re-tagged in HP-3's scorecard with +the reason. ## Exit gate diff --git a/docs/plans/hp-0-scorecard-2026-08-25.md b/docs/plans/hp-0-scorecard-2026-08-25.md index f3b495c8..f5c1ed57 100644 --- a/docs/plans/hp-0-scorecard-2026-08-25.md +++ b/docs/plans/hp-0-scorecard-2026-08-25.md @@ -37,7 +37,7 @@ baseline is **truthful, reproducible, and sufficient to begin B1**. | Docker build + boot smoke | unavailable | pass | **pass**, 50.1 MB, boots `:8443` | `ENV-02` closed | | Largest lazy chunk | — | budget in B7 | 1,998.25 kB min / 1,344.96 kB gzip | measured | | Generated/doc drift | refresh in B0 | 0 | **0** — `sqlc-verify`, `protocol-verify` green | CI | -| Ledger path resolution | — | 0 dead | **0 dead paths / 375 records** | re-verified 2026-08-29 | +| Ledger path resolution | — | 0 dead | **0 dead paths / 378 records** | re-verified 2026-08-29 | | Desktop/browser/device matrix | incomplete | 100% by B10 | **incomplete** | B6–B8 | | 250/100/25 capacity profile | unproven | met by B6 | **unproven** | `S-14`, B6 | | Upgrade/rollback/restore | unproven | green by B6 | **unproven** | B6 | @@ -68,22 +68,24 @@ Open ledger, re-verified 2026-08-29: | Status | Count | | --------- | ------- | | fixed | 315 | -| open | **56** | +| open | **59** | | declined | 3 | | duplicate | 1 | -| **total** | **375** | +| **total** | **378** | -Of the 56 open records: +Of the 59 open records: -- **1 high, 12 medium, 43 low. Zero critical.** The high is `OC-0350`, an +- **1 high, 12 medium, 46 low. Zero critical.** The high is `OC-0350`, an admin-panel login defect raised by the 2026-08-29 hunt and not yet phased. -- Three hunts: 29 from `general-2026-08-22-b`, 26 from `general-2026-08-29`, - 1 from `b2-1-fixture-capture-2026-08-28`. -- **55 of the 56 resolve to a live `file:line`** — 0 dead paths across all 375 +- Four hunts: 29 from `general-2026-08-22-b`, 26 from `general-2026-08-29`, + 3 from `b3-1-auth-characterization-2026-08-29` (defects the auth + characterization rows pin as-is, assigned to B3-9), 1 from + `b2-1-fixture-capture-2026-08-28`. +- **58 of the 59 resolve to a live `file:line`** — 0 dead paths across all 378 records, re-checked 2026-08-29. `OC-0323` is the exception: its line drifted past end of file when B2 work shortened `Server/service/channel.go`, so it needs re-pointing before it is fixed. -- 33 sit under `Client/`, 23 under `Server/`. +- 33 sit under `Client/`, 26 under `Server/`. - **None of the 2026-08-22 records is assigned to B1**; their register phases span B2–B10. The 2026-08-29 records are not yet phased in the register. diff --git a/docs/plans/repo-health-issue-register-2026-08-23.md b/docs/plans/repo-health-issue-register-2026-08-23.md index 94d16bd8..20d60fd4 100644 --- a/docs/plans/repo-health-issue-register-2026-08-23.md +++ b/docs/plans/repo-health-issue-register-2026-08-23.md @@ -77,10 +77,10 @@ together as if each row were a unique defect: | Status | Count | | --------- | ------: | | Fixed | 315 | -| Open | 56 | +| Open | 59 | | Declined | 3 | | Duplicate | 1 | -| **Total** | **375** | +| **Total** | **378** | The rows below cover `OC-0311`–`OC-0348` from the 2026-08-22 hunt, nine of which have since been fixed; `OC-0349`–`OC-0375` are recorded in the ledger and