diff --git a/.env.example b/.env.example index 475bedf..71f6ffc 100644 --- a/.env.example +++ b/.env.example @@ -11,8 +11,9 @@ DATABASE_URL=postgresql://user:password@host.example.com:5432/postgres?sslmode=v # When PORT is set (App Platform), cookies are Secure by default; SECURE_COOKIE=0 is rejected. # Locally, set to 1 when serving over HTTPS: SECURE_COOKIE=0 -# Set to 1 only behind a trusted reverse proxy that sets X-Forwarded-For. -# TRUST_PROXY=0 +# Comma-separated CIDRs of reverse proxies allowed to set X-Forwarded-For +# (direct peer must match). Leave unset to ignore XFF and use RemoteAddr. +# TRUSTED_PROXY_CIDRS=10.0.0.0/8,192.168.0.0/16 # DigitalOcean Spaces (profile avatars). Leave unset to disable uploads. # SPACES_KEY= # SPACES_SECRET= diff --git a/cmd/server/main.go b/cmd/server/main.go index ee3a8ba..ec20f60 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "log" + "net" "net/http" "os" "os/signal" @@ -56,7 +57,7 @@ func newHandler(db *sql.DB, sessions *store.SessionStore, uploader blob.Uploader srv, err := web.New(store.NewPostgres(db), sessions.Store(), plumber.TemplateFS, plumber.StaticFS, web.Config{ AdminSetupSecret: strings.TrimSpace(os.Getenv("ADMIN_SETUP_SECRET")), SecureCookie: secureCookieFromEnv(), - TrustProxy: os.Getenv("TRUST_PROXY") == "1", + TrustedProxies: parseTrustedProxies(os.Getenv("TRUSTED_PROXY_CIDRS")), Blob: uploader, }) if err != nil { @@ -65,6 +66,22 @@ func newHandler(db *sql.DB, sessions *store.SessionStore, uploader blob.Uploader return srv.Handler() } +func parseTrustedProxies(raw string) []*net.IPNet { + var out []*net.IPNet + for _, part := range strings.Split(raw, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + _, n, err := net.ParseCIDR(part) + if err != nil { + log.Fatalf("TRUSTED_PROXY_CIDRS: bad CIDR %q: %v", part, err) + } + out = append(out, n) + } + return out +} + // secureCookieFromEnv defaults to secure when PORT is set (PaaS/production) // and refuses an explicit disable in that environment. func secureCookieFromEnv() bool { @@ -96,12 +113,19 @@ func run(httpSrv *http.Server) { case sig := <-sigCh: log.Printf("shutdown signal: %v", sig) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - if err := httpSrv.Shutdown(ctx); err != nil { + err := httpSrv.Shutdown(ctx) + cancel() + if err != nil { log.Printf("shutdown: %v", err) + _ = httpSrv.Close() } - if err := <-errCh; err != nil && !errors.Is(err, http.ErrServerClosed) { - log.Fatal(err) + select { + case err := <-errCh: + if err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Printf("server exit: %v", err) + } + case <-time.After(3 * time.Second): + log.Printf("server exit: timed out waiting for ListenAndServe") } } } diff --git a/db/queries/sessions.sql b/db/queries/sessions.sql index 81482a2..5f8e71a 100644 --- a/db/queries/sessions.sql +++ b/db/queries/sessions.sql @@ -13,6 +13,6 @@ SET data = excluded.data, expiry = excluded.expiry; DELETE FROM sessions WHERE token = $1; --- name: DeleteExpiredSessions :exec +-- name: DeleteExpiredSessions :execrows DELETE FROM sessions WHERE expiry <= now(); diff --git a/db/queries/users.sql b/db/queries/users.sql index 954fced..02e4271 100644 --- a/db/queries/users.sql +++ b/db/queries/users.sql @@ -15,7 +15,17 @@ WHERE username = $1; -- name: ListUsers :many SELECT id, username, name, role, avatar_url, state, created_at FROM users -ORDER BY created_at ASC +WHERE ( + sqlc.arg(search) = '' + OR username ILIKE '%' || sqlc.arg(search) || '%' + OR name ILIKE '%' || sqlc.arg(search) || '%' +) +AND ( + sqlc.arg(cursor_created) = '' + OR created_at < sqlc.arg(cursor_created) + OR (created_at = sqlc.arg(cursor_created) AND id < sqlc.arg(cursor_id)) +) +ORDER BY created_at DESC, id DESC LIMIT sqlc.arg(row_limit); -- name: CountAdmins :one diff --git a/db/queries/votes.sql b/db/queries/votes.sql index 7268fed..a8917ba 100644 --- a/db/queries/votes.sql +++ b/db/queries/votes.sql @@ -3,12 +3,22 @@ SELECT value FROM votes WHERE user_id = $1 AND question_id = $2; +-- name: QuestionIsVisible :one +SELECT EXISTS( + SELECT 1 FROM questions WHERE id = $1 AND hidden = 0 +)::bool; + -- name: DeleteVote :exec DELETE FROM votes WHERE user_id = $1 AND question_id = $2; --- name: UpsertVote :exec +-- name: UpsertVoteOnVisible :execrows INSERT INTO votes (user_id, question_id, value) -VALUES ($1, $2, $3) +SELECT $1, $2, $3 +FROM questions q +WHERE q.id = $2 AND q.hidden = 0 ON CONFLICT (user_id, question_id) DO UPDATE -SET value = excluded.value; +SET value = excluded.value +WHERE EXISTS ( + SELECT 1 FROM questions q2 WHERE q2.id = excluded.question_id AND q2.hidden = 0 +); diff --git a/internal/blob/spaces.go b/internal/blob/spaces.go index 1a69d36..0493d01 100644 --- a/internal/blob/spaces.go +++ b/internal/blob/spaces.go @@ -17,6 +17,7 @@ import ( type Uploader interface { Enabled() bool Upload(ctx context.Context, obj FileUpload) (publicURL string, err error) + Delete(ctx context.Context, key string) error } // FileUpload is a file body to store (e.g. an avatar). @@ -51,6 +52,8 @@ func (Disabled) Upload(context.Context, FileUpload) (string, error) { return "", fmt.Errorf("avatar uploads are not configured") } +func (Disabled) Delete(context.Context, string) error { return nil } + // FromEnv builds an Uploader from SPACES_* environment variables. func FromEnv() Uploader { return NewSpaces(SpacesConfig{ @@ -99,11 +102,45 @@ func (s *spaces) Upload(ctx context.Context, obj FileUpload) (string, error) { if _, err := s.client.PutObject(ctx, input); err != nil { return "", err } - if s.cfg.CDNBase != "" { - return s.cfg.CDNBase + "/" + key, nil + return s.publicURL(key), nil +} + +func (s *spaces) Delete(ctx context.Context, key string) error { + key = strings.TrimPrefix(key, "/") + if key == "" { + return nil + } + _, err := s.client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: aws.String(s.cfg.Bucket), + Key: aws.String(key), + }) + return err +} + +func (s *spaces) publicURL(key string) string { + if s.cfg.CDNBase != "" { + return s.cfg.CDNBase + "/" + key } - // Virtual-hosted–style Spaces URL. host := strings.TrimPrefix(s.cfg.Endpoint, "https://") host = strings.TrimPrefix(host, "http://") - return fmt.Sprintf("https://%s.%s/%s", s.cfg.Bucket, host, key), nil + return fmt.Sprintf("https://%s.%s/%s", s.cfg.Bucket, host, key) +} + +// KeyFromPublicURL extracts the object key from a Spaces/CDN URL when possible. +func KeyFromPublicURL(publicURL, cdnBase, bucket, endpoint string) string { + publicURL = strings.TrimSpace(publicURL) + if publicURL == "" { + return "" + } + cdnBase = strings.TrimRight(strings.TrimSpace(cdnBase), "/") + if cdnBase != "" && strings.HasPrefix(publicURL, cdnBase+"/") { + return strings.TrimPrefix(publicURL, cdnBase+"/") + } + host := strings.TrimPrefix(strings.TrimSpace(endpoint), "https://") + host = strings.TrimPrefix(host, "http://") + prefix := fmt.Sprintf("https://%s.%s/", bucket, host) + if strings.HasPrefix(publicURL, prefix) { + return strings.TrimPrefix(publicURL, prefix) + } + return "" } diff --git a/internal/store/memory.go b/internal/store/memory.go index 7db3e67..93e6280 100644 --- a/internal/store/memory.go +++ b/internal/store/memory.go @@ -43,7 +43,7 @@ func (m *Memory) CreateUser(_ context.Context, u *User) error { } u.Username = NormalizeUsername(u.Username) if _, ok := m.byName[u.Username]; ok { - return fmt.Errorf("username taken") + return ErrDuplicateUsername } if u.ID == "" { u.ID = uuid.NewString() @@ -94,18 +94,44 @@ func (m *Memory) UserByUsername(_ context.Context, username string) (*User, erro return &cp, nil } -func (m *Memory) ListUsers(_ context.Context) ([]User, error) { +func (m *Memory) ListUsers(_ context.Context, q ListUsersQuery) ([]User, string, string, error) { m.mu.Lock() defer m.mu.Unlock() + limit := q.Limit + if limit <= 0 { + limit = AdminUsersLimit + } + search := strings.ToLower(strings.TrimSpace(q.Search)) out := make([]User, 0, len(m.users)) for _, u := range m.users { + if search != "" && + !strings.Contains(strings.ToLower(u.Username), search) && + !strings.Contains(strings.ToLower(u.Name), search) { + continue + } + if q.CursorCreated != "" { + if u.CreatedAt > q.CursorCreated { + continue + } + if u.CreatedAt == q.CursorCreated && u.ID >= q.CursorID { + continue + } + } out = append(out, *u) } - sort.Slice(out, func(i, j int) bool { return out[i].CreatedAt < out[j].CreatedAt }) - if len(out) > AdminUsersLimit { - out = out[:AdminUsersLimit] + sort.Slice(out, func(i, j int) bool { + if out[i].CreatedAt != out[j].CreatedAt { + return out[i].CreatedAt > out[j].CreatedAt + } + return out[i].ID > out[j].ID + }) + var nextCreated, nextID string + if len(out) > limit { + last := out[limit-1] + nextCreated, nextID = last.CreatedAt, last.ID + out = out[:limit] } - return out, nil + return out, nextCreated, nextID, nil } func (m *Memory) CountAdmins(_ context.Context) (int, error) { @@ -331,16 +357,17 @@ func (m *Memory) UpsertAnswer(_ context.Context, a *Answer) error { func (m *Memory) Vote(_ context.Context, userID, questionID string, value int) error { m.mu.Lock() defer m.mu.Unlock() - if value != 1 && value != -1 { + if value != 1 && value != -1 && value != 0 { return fmt.Errorf("invalid vote") } - if _, ok := m.questions[questionID]; !ok { - return sql.ErrNoRows + q, ok := m.questions[questionID] + if !ok || q.Hidden { + return ErrHiddenOrMissing } if m.votes[questionID] == nil { m.votes[questionID] = map[string]int{} } - if cur, ok := m.votes[questionID][userID]; ok && cur == value { + if value == 0 { delete(m.votes[questionID], userID) return nil } diff --git a/internal/store/migrate.go b/internal/store/migrate.go index bec080c..49c0f61 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -3,6 +3,7 @@ package store import ( "database/sql" "fmt" + "log" ) // migrateUserProfileColumns adds avatar_url and state when missing (existing DBs). @@ -16,3 +17,71 @@ func migrateUserProfileColumns(db *sql.DB) error { } return nil } + +const migrateLockKey int64 = 0x706c756d5f6d6967 // "plum_mig" + +// applyMigrations runs versioned migrations under an advisory lock. +// Fresh databases apply schemaSQL as version 001; later versions are incremental. +func applyMigrations(db *sql.DB, schemaSQL string) error { + tx, err := db.Begin() + if err != nil { + return err + } + defer tx.Rollback() + if _, err := tx.Exec(`SELECT pg_advisory_xact_lock($1)`, migrateLockKey); err != nil { + return fmt.Errorf("migrate lock: %w", err) + } + if _, err := tx.Exec(` +CREATE TABLE IF NOT EXISTS schema_migrations ( + version TEXT PRIMARY KEY, + applied_at TIMESTAMPTZ NOT NULL DEFAULT now() +)`); err != nil { + return fmt.Errorf("schema_migrations: %w", err) + } + if err := tx.Commit(); err != nil { + return err + } + + applied, err := appliedVersions(db) + if err != nil { + return err + } + + migrations := []struct { + version string + run func(*sql.DB) error + }{ + {"001_schema", func(db *sql.DB) error { return applySchema(db, schemaSQL) }}, + {"002_user_profile_columns", migrateUserProfileColumns}, + } + for _, m := range migrations { + if applied[m.version] { + continue + } + log.Printf("migrate: applying %s", m.version) + if err := m.run(db); err != nil { + return fmt.Errorf("migrate %s: %w", m.version, err) + } + if _, err := db.Exec(`INSERT INTO schema_migrations (version) VALUES ($1)`, m.version); err != nil { + return fmt.Errorf("record %s: %w", m.version, err) + } + } + return nil +} + +func appliedVersions(db *sql.DB) (map[string]bool, error) { + rows, err := db.Query(`SELECT version FROM schema_migrations`) + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string]bool{} + for rows.Next() { + var v string + if err := rows.Scan(&v); err != nil { + return nil, err + } + out[v] = true + } + return out, rows.Err() +} diff --git a/internal/store/postgres.go b/internal/store/postgres.go index 3f1af8f..fefde24 100644 --- a/internal/store/postgres.go +++ b/internal/store/postgres.go @@ -63,13 +63,9 @@ func OpenPostgres(databaseURL, schema string) (*sql.DB, *SessionStore, error) { _ = db.Close() return nil, nil, fmt.Errorf("postgres ping: %w", err) } - if err := applySchema(db, schema); err != nil { + if err := applyMigrations(db, schema); err != nil { _ = db.Close() - return nil, nil, fmt.Errorf("apply schema: %w", err) - } - if err := migrateUserProfileColumns(db); err != nil { - _ = db.Close() - return nil, nil, fmt.Errorf("migrate profile columns: %w", err) + return nil, nil, fmt.Errorf("migrate: %w", err) } sessions := NewSessionStore(db, 5*time.Minute) return db, sessions, nil diff --git a/internal/store/postgres_store.go b/internal/store/postgres_store.go index 8dbe634..f0f92a4 100644 --- a/internal/store/postgres_store.go +++ b/internal/store/postgres_store.go @@ -71,7 +71,7 @@ func (p *Postgres) CreateUser(ctx context.Context, u *User) error { Role: string(role), CreatedAt: u.CreatedAt, }); err != nil { - return err + return mapUniqueViolation(err) } if err := tx.Commit(); err != nil { return err @@ -89,8 +89,8 @@ func (p *Postgres) UserByUsername(ctx context.Context, username string) (*User, return UserByUsername(ctx, p.db, username) } -func (p *Postgres) ListUsers(ctx context.Context) ([]User, error) { - return ListUsers(ctx, p.db) +func (p *Postgres) ListUsers(ctx context.Context, q ListUsersQuery) ([]User, string, string, error) { + return ListUsers(ctx, p.db, q) } func (p *Postgres) CountAdmins(ctx context.Context) (int, error) { diff --git a/internal/store/sessions.go b/internal/store/sessions.go index b7fb062..a5d4a38 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "log" "sync" "time" @@ -103,10 +104,21 @@ func (s *SessionStore) cleanupLoop(interval time.Duration) { defer close(s.stopped) ticker := time.NewTicker(interval) defer ticker.Stop() + var lastErrLog time.Time for { select { case <-ticker.C: - _ = s.q.DeleteExpiredSessions(context.Background()) + n, err := s.q.DeleteExpiredSessions(context.Background()) + if err != nil { + if time.Since(lastErrLog) > time.Minute { + log.Printf("session cleanup: %v", err) + lastErrLog = time.Now() + } + continue + } + if n > 0 { + log.Printf("session cleanup: deleted %d expired row(s)", n) + } case <-s.stop: return } diff --git a/internal/store/sqlc/sessions.sql.go b/internal/store/sqlc/sessions.sql.go index 9562dea..7c9f9ec 100644 --- a/internal/store/sqlc/sessions.sql.go +++ b/internal/store/sqlc/sessions.sql.go @@ -10,14 +10,17 @@ import ( "time" ) -const deleteExpiredSessions = `-- name: DeleteExpiredSessions :exec +const deleteExpiredSessions = `-- name: DeleteExpiredSessions :execrows DELETE FROM sessions WHERE expiry <= now() ` -func (q *Queries) DeleteExpiredSessions(ctx context.Context) error { - _, err := q.db.ExecContext(ctx, deleteExpiredSessions) - return err +func (q *Queries) DeleteExpiredSessions(ctx context.Context) (int64, error) { + result, err := q.db.ExecContext(ctx, deleteExpiredSessions) + if err != nil { + return 0, err + } + return result.RowsAffected() } const deleteSession = `-- name: DeleteSession :exec diff --git a/internal/store/sqlc/users.sql.go b/internal/store/sqlc/users.sql.go index 9c764be..ffa9085 100644 --- a/internal/store/sqlc/users.sql.go +++ b/internal/store/sqlc/users.sql.go @@ -129,10 +129,27 @@ func (q *Queries) GetUserRole(ctx context.Context, id string) (string, error) { const listUsers = `-- name: ListUsers :many SELECT id, username, name, role, avatar_url, state, created_at FROM users -ORDER BY created_at ASC -LIMIT $1 +WHERE ( + $1 = '' + OR username ILIKE '%' || $1 || '%' + OR name ILIKE '%' || $1 || '%' +) +AND ( + $2 = '' + OR created_at < $2 + OR (created_at = $2 AND id < $3) +) +ORDER BY created_at DESC, id DESC +LIMIT $4 ` +type ListUsersParams struct { + Search interface{} + CursorCreated interface{} + CursorID string + RowLimit int32 +} + type ListUsersRow struct { ID string Username string @@ -143,8 +160,13 @@ type ListUsersRow struct { CreatedAt string } -func (q *Queries) ListUsers(ctx context.Context, rowLimit int32) ([]ListUsersRow, error) { - rows, err := q.db.QueryContext(ctx, listUsers, rowLimit) +func (q *Queries) ListUsers(ctx context.Context, arg ListUsersParams) ([]ListUsersRow, error) { + rows, err := q.db.QueryContext(ctx, listUsers, + arg.Search, + arg.CursorCreated, + arg.CursorID, + arg.RowLimit, + ) if err != nil { return nil, err } diff --git a/internal/store/sqlc/votes.sql.go b/internal/store/sqlc/votes.sql.go index da375d5..503901d 100644 --- a/internal/store/sqlc/votes.sql.go +++ b/internal/store/sqlc/votes.sql.go @@ -42,20 +42,41 @@ func (q *Queries) GetVote(ctx context.Context, arg GetVoteParams) (int32, error) return value, err } -const upsertVote = `-- name: UpsertVote :exec -INSERT INTO votes (user_id, question_id, value) -VALUES ($1, $2, $3) -ON CONFLICT (user_id, question_id) DO UPDATE -SET value = excluded.value +const questionIsVisible = `-- name: QuestionIsVisible :one +SELECT EXISTS( + SELECT 1 FROM questions WHERE id = $1 AND hidden = 0 +)::bool ` -type UpsertVoteParams struct { +func (q *Queries) QuestionIsVisible(ctx context.Context, id string) (bool, error) { + row := q.db.QueryRowContext(ctx, questionIsVisible, id) + var column_1 bool + err := row.Scan(&column_1) + return column_1, err +} + +const upsertVoteOnVisible = `-- name: UpsertVoteOnVisible :execrows +INSERT INTO votes (user_id, question_id, value) +SELECT $1, $2, $3 +FROM questions q +WHERE q.id = $2 AND q.hidden = 0 +ON CONFLICT (user_id, question_id) DO UPDATE +SET value = excluded.value +WHERE EXISTS ( + SELECT 1 FROM questions q2 WHERE q2.id = excluded.question_id AND q2.hidden = 0 +) +` + +type UpsertVoteOnVisibleParams struct { UserID string QuestionID string Value int32 } -func (q *Queries) UpsertVote(ctx context.Context, arg UpsertVoteParams) error { - _, err := q.db.ExecContext(ctx, upsertVote, arg.UserID, arg.QuestionID, arg.Value) - return err +func (q *Queries) UpsertVoteOnVisible(ctx context.Context, arg UpsertVoteOnVisibleParams) (int64, error) { + result, err := q.db.ExecContext(ctx, upsertVoteOnVisible, arg.UserID, arg.QuestionID, arg.Value) + if err != nil { + return 0, err + } + return result.RowsAffected() } diff --git a/internal/store/store.go b/internal/store/store.go index bd481bf..2faf5be 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -6,15 +6,23 @@ import "context" const ( HuntListLimit = 100 ProfileListLimit = 50 - AdminUsersLimit = 200 + AdminUsersLimit = 50 ) +// ListUsersQuery is a paginated admin user search. +type ListUsersQuery struct { + Search string + CursorCreated string + CursorID string + Limit int +} + // Store is the application persistence API used by the web layer. type Store interface { CreateUser(ctx context.Context, u *User) error UserByID(ctx context.Context, id string) (*User, error) UserByUsername(ctx context.Context, username string) (*User, error) - ListUsers(ctx context.Context) ([]User, error) + ListUsers(ctx context.Context, q ListUsersQuery) (users []User, nextCursorCreated, nextCursorID string, err error) CountAdmins(ctx context.Context) (int, error) SetUserRole(ctx context.Context, id string, role Role) error SaveUserProfile(ctx context.Context, u *User) error @@ -29,5 +37,6 @@ type Store interface { GetAnswer(ctx context.Context, questionID string) (*Answer, error) UpsertAnswer(ctx context.Context, a *Answer) error + // Vote sets the vote to 1, -1, or 0 (clear) on a visible question. Vote(ctx context.Context, userID, questionID string, value int) error } diff --git a/internal/store/user.go b/internal/store/user.go index ed28d57..299c616 100644 --- a/internal/store/user.go +++ b/internal/store/user.go @@ -82,14 +82,14 @@ func (u *User) Create(ctx context.Context) error { if u.CreatedAt == "" { u.CreatedAt = time.Now().UTC().Format(time.RFC3339) } - return sqlc.New(u.db).CreateUser(ctx, sqlc.CreateUserParams{ + return mapUniqueViolation(sqlc.New(u.db).CreateUser(ctx, sqlc.CreateUserParams{ ID: u.ID, Username: u.Username, Name: u.Name, PasswordHash: u.PasswordHash, Role: string(u.Role), CreatedAt: u.CreatedAt, - }) + })) } // adminRoleLockKey serializes SetRole so concurrent demotions cannot bypass the @@ -171,17 +171,32 @@ func CountAdmins(ctx context.Context, db *sql.DB) (int, error) { return int(n), err } -func ListUsers(ctx context.Context, db *sql.DB) ([]User, error) { - rows, err := sqlc.New(db).ListUsers(ctx, AdminUsersLimit) +func ListUsers(ctx context.Context, db *sql.DB, q ListUsersQuery) ([]User, string, string, error) { + limit := q.Limit + if limit <= 0 { + limit = AdminUsersLimit + } + rows, err := sqlc.New(db).ListUsers(ctx, sqlc.ListUsersParams{ + Search: q.Search, + CursorCreated: q.CursorCreated, + CursorID: q.CursorID, + RowLimit: int32(limit + 1), + }) if err != nil { - return nil, err + return nil, "", "", err } out := make([]User, 0, len(rows)) for _, r := range rows { u := toUser(db, r.ID, r.Username, r.Name, r.Role, r.AvatarUrl, r.State, r.CreatedAt, "") out = append(out, *u) } - return out, nil + var nextCreated, nextID string + if len(out) > limit { + last := out[limit-1] + nextCreated, nextID = last.CreatedAt, last.ID + out = out[:limit] + } + return out, nextCreated, nextID, nil } func UserByID(ctx context.Context, db *sql.DB, id string) (*User, error) { diff --git a/internal/store/vote.go b/internal/store/vote.go index c28efad..2d24a3c 100644 --- a/internal/store/vote.go +++ b/internal/store/vote.go @@ -3,38 +3,62 @@ package store import ( "context" "database/sql" + "errors" "fmt" + "github.com/jackc/pgx/v5/pgconn" + "plumber/internal/store/sqlc" ) -// Vote toggles or sets a user's vote on a question (value must be 1 or -1). -func Vote(ctx context.Context, db *sql.DB, userID, questionID string, value int) error { - if value != 1 && value != -1 { +// ErrDuplicateUsername is returned when inserting a username that already exists. +var ErrDuplicateUsername = errors.New("username taken") + +// ErrHiddenOrMissing is returned when voting on a hidden or unknown question. +var ErrHiddenOrMissing = errors.New("question not votable") + +// SetVote sets the user's vote to value (1, -1, or 0 to clear) on a visible question. +func SetVote(ctx context.Context, db *sql.DB, userID, questionID string, value int) error { + if value != 1 && value != -1 && value != 0 { return fmt.Errorf("invalid vote") } - tx, err := db.BeginTx(ctx, nil) - if err != nil { - return err - } - defer tx.Rollback() - - q := sqlc.New(tx) - current, err := q.GetVote(ctx, sqlc.GetVoteParams{UserID: userID, QuestionID: questionID}) - if err != nil && err != sql.ErrNoRows { - return err - } - if err == nil && int(current) == value { - err = q.DeleteVote(ctx, sqlc.DeleteVoteParams{UserID: userID, QuestionID: questionID}) - } else { - err = q.UpsertVote(ctx, sqlc.UpsertVoteParams{ + q := sqlc.New(db) + if value == 0 { + visible, err := q.QuestionIsVisible(ctx, questionID) + if err != nil { + return err + } + if !visible { + return ErrHiddenOrMissing + } + return q.DeleteVote(ctx, sqlc.DeleteVoteParams{ UserID: userID, QuestionID: questionID, - Value: int32(value), }) } + n, err := q.UpsertVoteOnVisible(ctx, sqlc.UpsertVoteOnVisibleParams{ + UserID: userID, + QuestionID: questionID, + Value: int32(value), + }) if err != nil { - return err + return mapUniqueViolation(err) } - return tx.Commit() + if n == 0 { + return ErrHiddenOrMissing + } + return nil +} + +// Vote is kept as an alias for SetVote for callers that still use the old name. +func Vote(ctx context.Context, db *sql.DB, userID, questionID string, value int) error { + return SetVote(ctx, db, userID, questionID, value) +} + +func mapUniqueViolation(err error) error { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == "23505" { + return ErrDuplicateUsername + } + return err } diff --git a/internal/web/admin.go b/internal/web/admin.go index 0ee32b6..fd90212 100644 --- a/internal/web/admin.go +++ b/internal/web/admin.go @@ -3,6 +3,8 @@ package web import ( "errors" "net/http" + "net/url" + "strings" "github.com/go-chi/chi/v5" @@ -11,8 +13,11 @@ import ( type adminUsersPage struct { page - Users []store.User - Error string + Users []store.User + Error string + Search string + NextCursor string + HasMore bool } func (s *Server) requireAdmin(w http.ResponseWriter, r *http.Request) *store.User { @@ -28,14 +33,35 @@ func (s *Server) handleAdminUsers(w http.ResponseWriter, r *http.Request) { if s.requireAdmin(w, r) == nil { return } - users, err := s.store.ListUsers(r.Context()) + search := strings.TrimSpace(r.URL.Query().Get("q")) + cursorCreated := r.URL.Query().Get("cursor_created") + cursorID := r.URL.Query().Get("cursor_id") + users, nextCreated, nextID, err := s.store.ListUsers(r.Context(), store.ListUsersQuery{ + Search: search, + CursorCreated: cursorCreated, + CursorID: cursorID, + Limit: store.AdminUsersLimit, + }) if err != nil { http.Error(w, "could not load users", http.StatusInternalServerError) return } + next := "" + if nextCreated != "" { + v := url.Values{} + if search != "" { + v.Set("q", search) + } + v.Set("cursor_created", nextCreated) + v.Set("cursor_id", nextID) + next = "/admin/users?" + v.Encode() + } s.exec(w, "admin-users", adminUsersPage{ - page: s.basePage(r, "Users"), - Users: users, + page: s.basePage(r, "Users"), + Users: users, + Search: search, + NextCursor: next, + HasMore: next != "", }) } @@ -50,7 +76,7 @@ func (s *Server) handleAdminSetRole(w http.ResponseWriter, r *http.Request) { role := store.Role(r.PostFormValue("role")) err := s.store.SetUserRole(r.Context(), id, role) if errors.Is(err, store.ErrLastAdmin) { - users, listErr := s.store.ListUsers(r.Context()) + users, _, _, listErr := s.store.ListUsers(r.Context(), store.ListUsersQuery{Limit: store.AdminUsersLimit}) if listErr != nil { http.Error(w, "could not demote last admin", http.StatusBadRequest) return diff --git a/internal/web/auth.go b/internal/web/auth.go index ac56f81..ed1d2c1 100644 --- a/internal/web/auth.go +++ b/internal/web/auth.go @@ -2,6 +2,9 @@ package web import ( "crypto/subtle" + "database/sql" + "errors" + "log" "net/http" "net/url" "regexp" @@ -79,8 +82,16 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { u, err := s.store.UserByUsername(r.Context(), username) hash := loginDummyHash - if err == nil { + switch { + case err == nil: hash = []byte(u.PasswordHash) + case errors.Is(err, sql.ErrNoRows): + // unknown user — still bcrypt against dummy hash + default: + log.Printf("login lookup: %v", err) + _ = bcrypt.CompareHashAndPassword(loginDummyHash, []byte(password)) + http.Error(w, "service temporarily unavailable", http.StatusServiceUnavailable) + return } if err != nil || bcrypt.CompareHashAndPassword(hash, []byte(password)) != nil { s.loginFail.record(loginFailKey(ip, userKey)) @@ -138,7 +149,7 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) { } role := store.RoleUser if setupSecretMatches(s.cfg.AdminSetupSecret, setupSecret) { - role = store.RoleAdmin // store downgrades if an admin already exists + role = store.RoleAdmin } u := &store.User{ Username: username, @@ -146,12 +157,19 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) { Role: role, } if err := s.store.CreateUser(r.Context(), u); err != nil { - p.Error = "That username is taken." - s.exec(w, "register", p) + if errors.Is(err, store.ErrDuplicateUsername) { + p.Error = "That username is taken." + s.exec(w, "register", p) + return + } + log.Printf("register create: %v", err) + http.Error(w, "could not create account", http.StatusInternalServerError) return } if err := s.sessions.RenewToken(r.Context()); err != nil { - http.Error(w, "could not start session", http.StatusInternalServerError) + log.Printf("register session: %v", err) + s.sessions.Put(r.Context(), "flash", "Account created — please sign in.") + http.Redirect(w, r, "/login", http.StatusSeeOther) return } s.sessions.Put(r.Context(), "user_id", u.ID) diff --git a/internal/web/profile.go b/internal/web/profile.go index 01ef47b..d76a2db 100644 --- a/internal/web/profile.go +++ b/internal/web/profile.go @@ -11,7 +11,6 @@ import ( "path" "strings" - "github.com/google/uuid" "golang.org/x/image/draw" _ "golang.org/x/image/webp" @@ -63,6 +62,7 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) { } avatarURL := "" + avatarKey := "" file, hdr, err := r.FormFile("avatar") if err == nil { defer file.Close() @@ -79,9 +79,10 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) { s.renderProfile(w, r, u, "Avatar must be a JPEG, PNG, or WebP image.", state) return } - key := path.Join("avatars", u.ID, uuid.NewString()+ext) + prevURL := u.AvatarURL + avatarKey = path.Join("avatars", u.ID, "avatar"+ext) url, upErr := s.cfg.Blob.Upload(r.Context(), blob.FileUpload{ - Key: key, + Key: avatarKey, Body: bytes.NewReader(body), ContentType: contentType, Size: int64(len(body)), @@ -91,15 +92,25 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) { return } avatarURL = url + u.State = state + u.AvatarURL = avatarURL + if err := s.store.SaveUserProfile(r.Context(), u); err != nil { + _ = s.cfg.Blob.Delete(r.Context(), avatarKey) + http.Error(w, "could not save profile", http.StatusInternalServerError) + return + } + if oldKey := avatarObjectKey(prevURL, u.ID); oldKey != "" && oldKey != avatarKey { + _ = s.cfg.Blob.Delete(r.Context(), oldKey) + } + s.sessions.Put(r.Context(), "flash", "Profile saved.") + http.Redirect(w, r, "/profile", http.StatusSeeOther) + return } else if err != http.ErrMissingFile { s.renderProfile(w, r, u, "Could not read avatar file.", state) return } u.State = state - if avatarURL != "" { - u.AvatarURL = avatarURL - } if err := s.store.SaveUserProfile(r.Context(), u); err != nil { http.Error(w, "could not save profile", http.StatusInternalServerError) return @@ -108,6 +119,19 @@ func (s *Server) handleProfile(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/profile", http.StatusSeeOther) } +func avatarObjectKey(publicURL, userID string) string { + marker := "/avatars/" + userID + "/" + i := strings.Index(publicURL, marker) + if i < 0 { + return "" + } + rest := publicURL[i+1:] // avatars/... + if q := strings.IndexAny(rest, "?#"); q >= 0 { + rest = rest[:q] + } + return rest +} + // prepareAvatar reads at most maxBytes, sniffs/decodes the image, resizes to a // small avatar, and re-encodes so only bounded valid image bytes are stored. func prepareAvatar(r io.Reader, maxBytes int64) (body []byte, ext, contentType string, err error) { diff --git a/internal/web/server.go b/internal/web/server.go index 5d84339..6d484bc 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -3,11 +3,14 @@ package web import ( "context" "crypto/rand" + "database/sql" "encoding/hex" + "errors" "fmt" "html/template" "io/fs" "log" + "net" "net/http" "net/url" "strings" @@ -28,9 +31,9 @@ type Config struct { // posts the matching setup_secret. It is ignored once any admin exists. AdminSetupSecret string SecureCookie bool - // TrustProxy enables X-Forwarded-For / RealIP only behind a known proxy. - TrustProxy bool - Blob blob.Uploader + // TrustedProxies are CIDRs allowed to set X-Forwarded-For (direct peer). + TrustedProxies []*net.IPNet + Blob blob.Uploader } type Server struct { @@ -145,7 +148,7 @@ func New(st store.Store, sessionStore scs.Store, templateFS fs.FS, staticFS fs.F func (s *Server) Handler() http.Handler { r := chi.NewRouter() r.Use(middleware.RequestID) - if s.cfg.TrustProxy { + if len(s.cfg.TrustedProxies) > 0 { r.Use(middleware.RealIP) } r.Use(middleware.Logger) @@ -356,7 +359,17 @@ func (s *Server) handleQuestion(w http.ResponseWriter, r *http.Request) { } var ans *store.Answer if q.Answered { - ans, _ = s.store.GetAnswer(r.Context(), q.ID) + ans, err = s.store.GetAnswer(r.Context(), q.ID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + log.Printf("question %s marked answered but answer missing", q.ID) + http.Error(w, "answer unavailable", http.StatusInternalServerError) + return + } + log.Printf("get answer %s: %v", q.ID, err) + http.Error(w, "could not load answer", http.StatusInternalServerError) + return + } } s.exec(w, "question", questionPage{ page: s.basePage(r, q.Title), @@ -385,11 +398,17 @@ func (s *Server) handleVote(w http.ResponseWriter, r *http.Request) { value = 1 case "-1": value = -1 + case "0": + value = 0 default: http.Error(w, "invalid vote", http.StatusBadRequest) return } if err := s.store.Vote(r.Context(), u.ID, id, value); err != nil { + if errors.Is(err, store.ErrHiddenOrMissing) { + http.Error(w, "not found", http.StatusNotFound) + return + } http.Error(w, "could not vote", http.StatusInternalServerError) return } diff --git a/internal/web/server_test.go b/internal/web/server_test.go index 69fb926..34bb8a3 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -332,6 +332,8 @@ func (f *fakeBlob) Upload(_ context.Context, obj blob.FileUpload) (string, error return "https://cdn.example.com/" + obj.Key, nil } +func (f *fakeBlob) Delete(_ context.Context, _ string) error { return nil } + func TestProfilePageAndState(t *testing.T) { srv, mem := newTestServer(t, Config{}) h := srv.Handler() diff --git a/internal/web/throttle.go b/internal/web/throttle.go index 9956015..846e075 100644 --- a/internal/web/throttle.go +++ b/internal/web/throttle.go @@ -1,22 +1,33 @@ package web import ( + "container/list" + "log" "net" "net/http" "strings" "sync" + "sync/atomic" "time" ) const defaultThrottleMaxKeys = 10_000 -// throttle is a sliding-window rate limiter with expired-key eviction and a cap. +// throttle is a sliding-window rate limiter with LRU eviction at capacity. type throttle struct { mu sync.Mutex - hits map[string][]time.Time + hits map[string]*throttleEntry + lru *list.List // front = most recently used limit int window time.Duration maxKeys int + rejects atomic.Uint64 + lastLog time.Time +} + +type throttleEntry struct { + times []time.Time + el *list.Element } func newThrottle(limit int, window time.Duration, maxKeys int) *throttle { @@ -24,7 +35,8 @@ func newThrottle(limit int, window time.Duration, maxKeys int) *throttle { maxKeys = defaultThrottleMaxKeys } return &throttle{ - hits: map[string][]time.Time{}, + hits: map[string]*throttleEntry{}, + lru: list.New(), limit: limit, window: window, maxKeys: maxKeys, @@ -38,45 +50,68 @@ func (t *throttle) allow(key string) bool { t.mu.Lock() defer t.mu.Unlock() now := time.Now() - t.evictExpiredLocked(now) + cutoff := now.Add(-t.window) - 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 + ent, ok := t.hits[key] + if ok { + ent.times = pruneTimes(ent.times, cutoff) + if len(ent.times) == 0 { + t.removeLocked(key, ent) + ok = false } - return false } - if _, exists := t.hits[key]; !exists && len(t.hits) >= t.maxKeys { - t.evictExpiredLocked(now) - if len(t.hits) >= t.maxKeys { + if ok { + if len(ent.times) >= t.limit { + t.touchLocked(key, ent) return false } + ent.times = append(ent.times, now) + t.touchLocked(key, ent) + return true } - t.hits[key] = append(xs, now) + + // New key: make room via LRU if needed. + for len(t.hits) >= t.maxKeys { + oldest := t.lru.Back() + if oldest == nil { + break + } + oldKey := oldest.Value.(string) + t.removeLocked(oldKey, t.hits[oldKey]) + n := t.rejects.Add(1) + if time.Since(t.lastLog) > time.Minute { + log.Printf("throttle: LRU evicted key at capacity=%d rejects=%d", t.maxKeys, n) + t.lastLog = now + } + } + ent = &throttleEntry{times: []time.Time{now}} + ent.el = t.lru.PushFront(key) + t.hits[key] = ent return true } +func (t *throttle) touchLocked(key string, ent *throttleEntry) { + if ent.el != nil { + t.lru.MoveToFront(ent.el) + } +} + +func (t *throttle) removeLocked(key string, ent *throttleEntry) { + if ent == nil { + return + } + if ent.el != nil { + t.lru.Remove(ent.el) + } + delete(t.hits, key) +} + 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) - 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 { @@ -137,7 +172,16 @@ func (f *failureTracker) record(key string) { f.evictExpiredLocked(now) st := f.fails[key] if st.count == 0 && len(f.fails) >= f.maxKeys { - return + // Drop an arbitrary expired-or-oldest entry. + for k, v := range f.fails { + if now.Sub(v.last) > f.window/2 { + delete(f.fails, k) + break + } + } + if len(f.fails) >= f.maxKeys { + return + } } st.count++ st.last = now @@ -184,18 +228,58 @@ func progressiveDelay(failCount int) time.Duration { } func (s *Server) clientIP(r *http.Request) string { - if s.cfg.TrustProxy { - if xff := r.Header.Get("X-Forwarded-For"); xff != "" { - return strings.TrimSpace(strings.Split(xff, ",")[0]) - } - } host, _, err := net.SplitHostPort(r.RemoteAddr) if err != nil { - return r.RemoteAddr + host = r.RemoteAddr + } + peer := net.ParseIP(host) + if peer == nil || !ipInNets(peer, s.cfg.TrustedProxies) { + return host + } + xff := r.Header.Get("X-Forwarded-For") + if xff == "" { + return host + } + parts := strings.Split(xff, ",") + // Walk right-to-left; skip trusted hops; first untrusted is the client. + for i := len(parts) - 1; i >= 0; i-- { + p := net.ParseIP(strings.TrimSpace(parts[i])) + if p == nil { + continue + } + if !ipInNets(p, s.cfg.TrustedProxies) { + return p.String() + } } return host } +func ipInNets(ip net.IP, nets []*net.IPNet) bool { + for _, n := range nets { + if n.Contains(ip) { + return true + } + } + return false +} + +func parseCIDRs(raw string) []*net.IPNet { + var out []*net.IPNet + for _, part := range strings.Split(raw, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + _, n, err := net.ParseCIDR(part) + if err != nil { + log.Printf("trusted proxy CIDR ignored %q: %v", part, err) + continue + } + out = append(out, n) + } + return out +} + func authTooMany(w http.ResponseWriter) { http.Error(w, "too many attempts; try again later", http.StatusTooManyRequests) } diff --git a/internal/web/throttle_test.go b/internal/web/throttle_test.go index b234980..156c0f3 100644 --- a/internal/web/throttle_test.go +++ b/internal/web/throttle_test.go @@ -1,6 +1,7 @@ package web import ( + "net" "net/http" "net/http/httptest" "sync" @@ -33,8 +34,8 @@ func TestThrottleMaxKeys(t *testing.T) { 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.allow("three") { + t.Fatal("over maxKeys should LRU-evict and accept new key") } if th.lenKeys() != 2 { t.Fatalf("keys=%d want 2", th.lenKeys()) @@ -90,7 +91,11 @@ func TestFailureTrackerEvictsExpired(t *testing.T) { } func TestClientIPTrustProxy(t *testing.T) { - srv := &Server{cfg: Config{TrustProxy: true}} + _, proxyNet, err := net.ParseCIDR("10.0.0.0/8") + if err != nil { + t.Fatal(err) + } + srv := &Server{cfg: Config{TrustedProxies: []*net.IPNet{proxyNet}}} 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") @@ -98,9 +103,9 @@ func TestClientIPTrustProxy(t *testing.T) { 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) + req.RemoteAddr = "203.0.113.50:9" + if got := srv.clientIP(req); got != "203.0.113.50" { + t.Fatalf("untrusted peer should ignore xff, got %q", got) } } diff --git a/templates/admin_users.html b/templates/admin_users.html index d15e846..8bdc197 100644 --- a/templates/admin_users.html +++ b/templates/admin_users.html @@ -4,6 +4,11 @@
Admin