Fix auth throttle DoS and serialize admin bootstrap.

Evict/cap limiter keys, replace hard username lockouts with IP+user progressive delays cleared on success, and create bootstrap admins under the same advisory/mutex lock as role changes.

internal/store/memory.go

@@ -54,7 +54,17 @@ func (m *Memory) CreateUser(_ context.Context, u *User) error {
 	if u.CreatedAt == "" {
 		u.CreatedAt = time.Now().UTC().Format(time.RFC3339)
 	}
+	role := u.Role
+	if role == RoleAdmin {
+		for _, existing := range m.users {
+			if existing.Role == RoleAdmin {
+				role = RoleUser
+				break
+			}
+		}
+	}
 	cp := *u
+	cp.Role = role
 	cp.db = nil
 	m.users[cp.ID] = &cp
 	m.byName[cp.Username] = cp.ID

internal/store/memory_test.go

@@ -9,14 +9,20 @@ import (
 func TestMemoryConcurrentLastAdminDemotion(t *testing.T) {
 	m := NewMemory()
 	ctx := context.Background()
-	a := &User{Username: "admin_a", PasswordHash: "x", Role: RoleAdmin}
-	b := &User{Username: "admin_b", PasswordHash: "x", Role: RoleAdmin}
+	a := &User{Username: "admin_a", PasswordHash: "x", Role: RoleUser}
+	b := &User{Username: "admin_b", PasswordHash: "x", Role: RoleUser}
 	if err := m.CreateUser(ctx, a); err != nil {
 		t.Fatal(err)
 	}
 	if err := m.CreateUser(ctx, b); err != nil {
 		t.Fatal(err)
 	}
+	if err := m.SetUserRole(ctx, a.ID, RoleAdmin); err != nil {
+		t.Fatal(err)
+	}
+	if err := m.SetUserRole(ctx, b.ID, RoleAdmin); err != nil {
+		t.Fatal(err)
+	}
 
 	var wg sync.WaitGroup
 	errs := make(chan error, 2)
@@ -54,3 +60,42 @@ func TestMemoryConcurrentLastAdminDemotion(t *testing.T) {
 		t.Fatalf("admins remaining = %d, want 1", n)
 	}
 }
+
+func TestMemoryConcurrentBootstrapAdmin(t *testing.T) {
+	m := NewMemory()
+	ctx := context.Background()
+	a := &User{Username: "boot_a", PasswordHash: "x", Role: RoleAdmin}
+	b := &User{Username: "boot_b", PasswordHash: "x", Role: RoleAdmin}
+
+	var wg sync.WaitGroup
+	errs := make(chan error, 2)
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		errs <- m.CreateUser(ctx, a)
+	}()
+	go func() {
+		defer wg.Done()
+		errs <- m.CreateUser(ctx, b)
+	}()
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	n, err := m.CountAdmins(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n != 1 {
+		t.Fatalf("bootstrap race left %d admins, want 1", n)
+	}
+	if a.Role == RoleAdmin && b.Role == RoleAdmin {
+		t.Fatal("both users kept RoleAdmin")
+	}
+	if a.Role != RoleAdmin && b.Role != RoleAdmin {
+		t.Fatal("neither user is admin")
+	}
+}

internal/store/postgres_store.go

@@ -3,6 +3,12 @@ package store
 import (
 	"context"
 	"database/sql"
+	"fmt"
+	"time"
+
+	"github.com/google/uuid"
+
+	"plumber/internal/store/sqlc"
 )
 
 // Postgres implements Store against a sqlc-backed database.
@@ -16,10 +22,65 @@ func NewPostgres(db *sql.DB) *Postgres {
 }
 
 func (p *Postgres) CreateUser(ctx context.Context, u *User) error {
+	if u == nil {
+		return fmt.Errorf("user: nil")
+	}
+	if u.Role != RoleUser && u.Role != RoleAdmin {
+		return fmt.Errorf("invalid role")
+	}
+	u.Username = NormalizeUsername(u.Username)
+	if u.ID == "" {
+		u.ID = uuid.NewString()
+	}
+	if u.Name == "" {
+		u.Name = u.Username
+	}
+	if u.CreatedAt == "" {
+		u.CreatedAt = time.Now().UTC().Format(time.RFC3339)
+	}
+
+	if u.Role != RoleAdmin {
 		u.db = p.db
 		return u.Create(ctx)
 	}
 
+	// Bootstrap admin: serialize count+insert so two setup-secret registers
+	// cannot both observe zero admins.
+	tx, err := p.db.BeginTx(ctx, nil)
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+	if _, err := tx.ExecContext(ctx, `SELECT pg_advisory_xact_lock($1)`, adminRoleLockKey); err != nil {
+		return err
+	}
+	q := sqlc.New(tx)
+	n, err := q.CountAdmins(ctx, string(RoleAdmin))
+	if err != nil {
+		return err
+	}
+	role := RoleAdmin
+	if n > 0 {
+		role = RoleUser
+	}
+	if err := q.CreateUser(ctx, sqlc.CreateUserParams{
+		ID:           u.ID,
+		Username:     u.Username,
+		Name:         u.Name,
+		PasswordHash: u.PasswordHash,
+		Role:         string(role),
+		CreatedAt:    u.CreatedAt,
+	}); err != nil {
+		return err
+	}
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+	u.Role = role
+	u.db = p.db
+	return nil
+}
+
 func (p *Postgres) UserByID(ctx context.Context, id string) (*User, error) {
 	return UserByID(ctx, p.db, id)
 }

internal/web/auth.go

@@ -71,7 +71,9 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 	username := strings.TrimSpace(r.PostFormValue("username"))
 	password := r.PostFormValue("password")
 	next := safeNext(r.PostFormValue("next"))
-	if !s.allowLoginAttempt(w, r, store.NormalizeUsername(username)) {
+	userKey := store.NormalizeUsername(username)
+	ip := s.clientIP(r)
+	if !s.allowLoginAttempt(w, r, userKey) {
 		return
 	}
 
@@ -81,6 +83,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 		hash = []byte(u.PasswordHash)
 	}
 	if err != nil || bcrypt.CompareHashAndPassword(hash, []byte(password)) != nil {
+		s.loginFail.record(loginFailKey(ip, userKey))
 		w.WriteHeader(http.StatusUnauthorized)
 		s.exec(w, "login", authPage{
 			page:     s.basePage(r, "Sign in"),
@@ -90,6 +93,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 		})
 		return
 	}
+	s.loginFail.clear(loginFailKey(ip, userKey))
 	if err := s.sessions.RenewToken(r.Context()); err != nil {
 		http.Error(w, "could not start session", http.StatusInternalServerError)
 		return
@@ -133,8 +137,8 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	role := store.RoleUser
-	if s.consumeAdminSetup(r, setupSecret) {
-		role = store.RoleAdmin
+	if setupSecretMatches(s.cfg.AdminSetupSecret, setupSecret) {
+		role = store.RoleAdmin // store downgrades if an admin already exists
 	}
 	u := &store.User{
 		Username:     username,
@@ -154,21 +158,11 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
-// consumeAdminSetup grants first-admin when a strong one-time setup secret matches
-// and no admin exists yet. Username alone is never enough.
-func (s *Server) consumeAdminSetup(r *http.Request, provided string) bool {
-	want := s.cfg.AdminSetupSecret
+func setupSecretMatches(want, provided string) bool {
 	if want == "" || provided == "" {
 		return false
 	}
-	if subtle.ConstantTimeCompare([]byte(provided), []byte(want)) != 1 {
-		return false
-	}
-	n, err := s.store.CountAdmins(r.Context())
-	if err != nil || n > 0 {
-		return false
-	}
-	return true
+	return subtle.ConstantTimeCompare([]byte(provided), []byte(want)) == 1
 }
 
 func (s *Server) handleAuthPrompt(w http.ResponseWriter, r *http.Request) {

internal/web/server.go

@@ -40,8 +40,8 @@ type Server struct {
 	cfg        Config
 	static     http.Handler
 	loginIP    *throttle
-	loginUser  *throttle
 	registerIP *throttle
+	loginFail  *failureTracker
 }
 
 type page struct {
@@ -136,9 +136,9 @@ func New(st store.Store, sessionStore scs.Store, templateFS fs.FS, staticFS fs.F
 		tmpl:       tmpl,
 		cfg:        cfg,
 		static:     http.StripPrefix("/static/", http.FileServer(http.FS(sub))),
-		loginIP:    newThrottle(20, 15*time.Minute),
-		loginUser:  newThrottle(10, 15*time.Minute),
-		registerIP: newThrottle(10, 15*time.Minute),
+		loginIP:    newThrottle(20, 15*time.Minute, defaultThrottleMaxKeys),
+		registerIP: newThrottle(10, 15*time.Minute, defaultThrottleMaxKeys),
+		loginFail:  newFailureTracker(15*time.Minute, defaultThrottleMaxKeys),
 	}, nil
 }
 

internal/web/throttle.go

@@ -8,19 +8,26 @@ import (
 	"time"
 )
 
-// throttle is a simple sliding-window rate limiter for auth endpoints.
+const defaultThrottleMaxKeys = 10_000
+
+// throttle is a sliding-window rate limiter with expired-key eviction and a cap.
 type throttle struct {
 	mu      sync.Mutex
 	hits    map[string][]time.Time
 	limit   int
 	window  time.Duration
+	maxKeys int
 }
 
-func newThrottle(limit int, window time.Duration) *throttle {
+func newThrottle(limit int, window time.Duration, maxKeys int) *throttle {
+	if maxKeys <= 0 {
+		maxKeys = defaultThrottleMaxKeys
+	}
 	return &throttle{
 		hits:    map[string][]time.Time{},
 		limit:   limit,
 		window:  window,
+		maxKeys: maxKeys,
 	}
 }
 
@@ -31,8 +38,46 @@ func (t *throttle) allow(key string) bool {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	now := time.Now()
+	t.evictExpiredLocked(now)
+
+	xs := pruneTimes(t.hits[key], now.Add(-t.window))
+	if len(xs) >= t.limit {
+		if len(xs) == 0 {
+			delete(t.hits, key)
+		} else {
+			t.hits[key] = xs
+		}
+		return false
+	}
+	if _, exists := t.hits[key]; !exists && len(t.hits) >= t.maxKeys {
+		t.evictExpiredLocked(now)
+		if len(t.hits) >= t.maxKeys {
+			return false
+		}
+	}
+	t.hits[key] = append(xs, now)
+	return true
+}
+
+func (t *throttle) lenKeys() int {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	return len(t.hits)
+}
+
+func (t *throttle) evictExpiredLocked(now time.Time) {
 	cutoff := now.Add(-t.window)
-	xs := t.hits[key]
+	for k, xs := range t.hits {
+		xs = pruneTimes(xs, cutoff)
+		if len(xs) == 0 {
+			delete(t.hits, k)
+		} else {
+			t.hits[k] = xs
+		}
+	}
+}
+
+func pruneTimes(xs []time.Time, cutoff time.Time) []time.Time {
 	n := 0
 	for _, ts := range xs {
 		if ts.After(cutoff) {
@@ -40,13 +85,102 @@ func (t *throttle) allow(key string) bool {
 			n++
 		}
 	}
-	xs = xs[:n]
-	if len(xs) >= t.limit {
-		t.hits[key] = xs
-		return false
+	return xs[:n]
+}
+
+// failureTracker records auth failures for progressive delay (not a hard lockout).
+type failureTracker struct {
+	mu      sync.Mutex
+	fails   map[string]failState
+	window  time.Duration
+	maxKeys int
+}
+
+type failState struct {
+	count int
+	last  time.Time
+}
+
+func newFailureTracker(window time.Duration, maxKeys int) *failureTracker {
+	if maxKeys <= 0 {
+		maxKeys = defaultThrottleMaxKeys
+	}
+	return &failureTracker{
+		fails:   map[string]failState{},
+		window:  window,
+		maxKeys: maxKeys,
+	}
+}
+
+func (f *failureTracker) delay(key string) time.Duration {
+	if f == nil || key == "" {
+		return 0
+	}
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	now := time.Now()
+	f.evictExpiredLocked(now)
+	st, ok := f.fails[key]
+	if !ok {
+		return 0
+	}
+	return progressiveDelay(st.count)
+}
+
+func (f *failureTracker) record(key string) {
+	if f == nil || key == "" {
+		return
+	}
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	now := time.Now()
+	f.evictExpiredLocked(now)
+	st := f.fails[key]
+	if st.count == 0 && len(f.fails) >= f.maxKeys {
+		return
+	}
+	st.count++
+	st.last = now
+	f.fails[key] = st
+}
+
+func (f *failureTracker) clear(key string) {
+	if f == nil || key == "" {
+		return
+	}
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	delete(f.fails, key)
+}
+
+func (f *failureTracker) lenKeys() int {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return len(f.fails)
+}
+
+func (f *failureTracker) evictExpiredLocked(now time.Time) {
+	cutoff := now.Add(-f.window)
+	for k, st := range f.fails {
+		if st.last.Before(cutoff) {
+			delete(f.fails, k)
+		}
+	}
+}
+
+func progressiveDelay(failCount int) time.Duration {
+	switch {
+	case failCount <= 1:
+		return 0
+	case failCount == 2:
+		return 200 * time.Millisecond
+	case failCount == 3:
+		return 500 * time.Millisecond
+	case failCount == 4:
+		return time.Second
+	default:
+		return 2 * time.Second
 	}
-	t.hits[key] = append(xs, now)
-	return true
 }
 
 func (s *Server) clientIP(r *http.Request) string {
@@ -66,11 +200,19 @@ func authTooMany(w http.ResponseWriter) {
 	http.Error(w, "too many attempts; try again later", http.StatusTooManyRequests)
 }
 
+func loginFailKey(ip, usernameKey string) string {
+	return ip + "\x00" + usernameKey
+}
+
 func (s *Server) allowLoginAttempt(w http.ResponseWriter, r *http.Request, usernameKey string) bool {
-	if !s.loginIP.allow(s.clientIP(r)) || !s.loginUser.allow(usernameKey) {
+	ip := s.clientIP(r)
+	if !s.loginIP.allow(ip) {
 		authTooMany(w)
 		return false
 	}
+	if d := s.loginFail.delay(loginFailKey(ip, usernameKey)); d > 0 {
+		time.Sleep(d)
+	}
 	return true
 }
 

internal/web/throttle_test.go

@@ -0,0 +1,122 @@
+package web
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+	"time"
+)
+
+func TestThrottleWindowAndEviction(t *testing.T) {
+	th := newThrottle(2, 50*time.Millisecond, 100)
+	if !th.allow("a") || !th.allow("a") {
+		t.Fatal("first two should pass")
+	}
+	if th.allow("a") {
+		t.Fatal("third within window should fail")
+	}
+	time.Sleep(60 * time.Millisecond)
+	if !th.allow("a") {
+		t.Fatal("after window should pass")
+	}
+	// Expired empty keys should be removed on next allow of another key path.
+	time.Sleep(60 * time.Millisecond)
+	_ = th.allow("b")
+	if th.lenKeys() > 2 {
+		t.Fatalf("expected eviction of stale keys, got %d", th.lenKeys())
+	}
+}
+
+func TestThrottleMaxKeys(t *testing.T) {
+	th := newThrottle(5, time.Minute, 2)
+	if !th.allow("one") || !th.allow("two") {
+		t.Fatal("first keys should pass")
+	}
+	if th.allow("three") {
+		t.Fatal("over maxKeys should reject new key")
+	}
+	if th.lenKeys() != 2 {
+		t.Fatalf("keys=%d want 2", th.lenKeys())
+	}
+}
+
+func TestThrottleConcurrent(t *testing.T) {
+	th := newThrottle(50, time.Minute, 1000)
+	var wg sync.WaitGroup
+	var okCount int
+	var mu sync.Mutex
+	for i := 0; i < 100; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			if th.allow("same") {
+				mu.Lock()
+				okCount++
+				mu.Unlock()
+			}
+		}()
+	}
+	wg.Wait()
+	if okCount != 50 {
+		t.Fatalf("ok=%d want 50", okCount)
+	}
+}
+
+func TestFailureTrackerProgressiveAndClear(t *testing.T) {
+	f := newFailureTracker(time.Minute, 100)
+	if d := f.delay("k"); d != 0 {
+		t.Fatalf("fresh delay=%v", d)
+	}
+	f.record("k")
+	f.record("k")
+	if d := f.delay("k"); d != 200*time.Millisecond {
+		t.Fatalf("delay after 2 fails=%v", d)
+	}
+	f.clear("k")
+	if d := f.delay("k"); d != 0 {
+		t.Fatalf("after clear delay=%v", d)
+	}
+}
+
+func TestFailureTrackerEvictsExpired(t *testing.T) {
+	f := newFailureTracker(30*time.Millisecond, 100)
+	f.record("old")
+	time.Sleep(40 * time.Millisecond)
+	_ = f.delay("other") // triggers eviction
+	if f.lenKeys() != 0 {
+		t.Fatalf("expired key remained, keys=%d", f.lenKeys())
+	}
+}
+
+func TestClientIPTrustProxy(t *testing.T) {
+	srv := &Server{cfg: Config{TrustProxy: true}}
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	req.RemoteAddr = "10.0.0.1:1234"
+	req.Header.Set("X-Forwarded-For", "203.0.113.9, 10.0.0.1")
+	if got := srv.clientIP(req); got != "203.0.113.9" {
+		t.Fatalf("trusted xff got %q", got)
+	}
+
+	srv.cfg.TrustProxy = false
+	if got := srv.clientIP(req); got != "10.0.0.1" {
+		t.Fatalf("untrusted should use RemoteAddr host, got %q", got)
+	}
+}
+
+func TestNoGlobalUsernameHardLockout(t *testing.T) {
+	// Victim IP should still be allowed after another IP burns attempts for the same username.
+	srv := &Server{
+		loginIP:   newThrottle(20, time.Minute, 100),
+		loginFail: newFailureTracker(time.Minute, 100),
+	}
+	for i := 0; i < 20; i++ {
+		srv.loginFail.record(loginFailKey("1.1.1.1", "alice"))
+	}
+	victim := httptest.NewRequest(http.MethodPost, "/login", nil)
+	victim.RemoteAddr = "2.2.2.2:9"
+	w := httptest.NewRecorder()
+	if !srv.allowLoginAttempt(w, victim, "alice") {
+		t.Fatal("victim IP must not be hard-locked by username-only attempts")
+	}
+}