From f40ca97393795341df4a9dbeb87aaa3ebf14f9c8 Mon Sep 17 00:00:00 2001 From: Jonathon Leight Date: Mon, 22 Jun 2026 10:54:07 -0400 Subject: [PATCH] Username change functionality --- internal/auth/account.go | 36 ++++ internal/auth/templates/account.html | 37 +++- internal/auth/web.go | 1 + internal/core/admin.go | 27 +++ .../core/templates/admin_user_history.html | 43 +++++ internal/core/templates/admin_users.html | 1 + internal/core/web.go | 1 + internal/store/command_log.go | 15 +- internal/store/console_sessions.go | 4 +- .../migrations/0020_username_changes.sql | 39 ++++ internal/store/username.go | 177 ++++++++++++++++++ internal/store/username_test.go | 112 +++++++++++ internal/store/users.go | 13 +- internal/web/ratelimit.go | 4 + 14 files changed, 490 insertions(+), 20 deletions(-) create mode 100644 internal/core/templates/admin_user_history.html create mode 100644 internal/store/migrations/0020_username_changes.sql create mode 100644 internal/store/username.go create mode 100644 internal/store/username_test.go diff --git a/internal/auth/account.go b/internal/auth/account.go index b0e34bd..622e2d3 100644 --- a/internal/auth/account.go +++ b/internal/auth/account.go @@ -2,10 +2,12 @@ package auth import ( "encoding/hex" + "errors" "net/http" "strconv" "time" + "github.com/jleight/meshtender/internal/store" "github.com/jleight/meshtender/internal/web" ) @@ -51,11 +53,19 @@ func (s *Handlers) pageAccount(w http.ResponseWriter, r *http.Request) { } views = append(views, passkeyView{ID: c.ID, ShortID: short, Name: c.Name, Added: c.CreatedAt}) } + // When set, the user changed their username recently and must wait until + // this time before changing it again. + nextRename, err := s.Store.NextRenameAllowed(r.Context(), uid) + if err != nil { + http.Error(w, "could not load account", http.StatusInternalServerError) + return + } s.Render(w, r, "account.html", map[string]any{ "User": u, "DisplayName": u.DisplayName, // nil when unset "HasPassword": u.PasswordHash != nil, "Passkeys": views, + "NextRename": nextRename, // nil when a rename is allowed now "Error": r.URL.Query().Get("error"), "OK": r.URL.Query().Get("ok"), "PKMsg": r.URL.Query().Get("pk"), @@ -63,6 +73,32 @@ func (s *Handlers) pageAccount(w http.ResponseWriter, r *http.Request) { }) } +// handleChangeUsername renames the current user, enforcing validation, the +// per-user rename interval, and the release cooldown on names others hold. +func (s *Handlers) handleChangeUsername(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + uid := s.Auth.CurrentUserID(ctx) + newName := NormalizeUsername(r.FormValue("username")) + if !ValidUsername(newName) { + accountRedirect(w, r, "error", "Choose a username 3–32 characters long, using only letters, digits, and _ . -") + return + } + meta := store.UsernameChangeContext{ChangedBy: uid, IP: web.ClientIP(r), UserAgent: r.UserAgent()} + err := s.Store.SetUsername(ctx, uid, newName, meta, true) + switch { + case errors.Is(err, store.ErrDuplicate), errors.Is(err, store.ErrUsernameReserved): + // Collapse "taken" and "reserved" into one message so we don't reveal + // that a name was previously in use by someone else. + accountRedirect(w, r, "error", "That username isn't available. Please choose another.") + case errors.Is(err, store.ErrRenameTooSoon): + accountRedirect(w, r, "error", "You can only change your username once every 30 days.") + case err != nil: + accountRedirect(w, r, "error", "Could not change your username.") + default: + accountRedirect(w, r, "ok", "Username changed to @"+newName+".") + } +} + // handleUpdateProfile saves the user's display name. func (s *Handlers) handleUpdateProfile(w http.ResponseWriter, r *http.Request) { uid := s.Auth.CurrentUserID(r.Context()) diff --git a/internal/auth/templates/account.html b/internal/auth/templates/account.html index cbbf7ff..66f520c 100644 --- a/internal/auth/templates/account.html +++ b/internal/auth/templates/account.html @@ -12,25 +12,44 @@ {{if .OK}}
{{.OK}}
{{end}}
-

Profile

+

Username

-

Your username can't be changed. Your display name is shown to people you share with.

-
-
+

Your username is your unique handle (3–32 chars: letters, digits, _ . -). You can change it once every 30 days. When you do, your old username is reserved for 90 days before anyone else can take it.

+ +
- +
+ @ + +
+
+ +
+
+ {{if .NextRename}} + You changed your username recently. You can change it again on {{.NextRename.Format "Jan 2, 2006"}}. + {{end}} + +
+
+ +
+

Display name

+
+

Your display name is what other people see — on shares and in organizations. It can be anything, and it isn't tied to sign-in.

+
+
-
-
- +
+ +
-
diff --git a/internal/auth/web.go b/internal/auth/web.go index ee25a4e..6399646 100644 --- a/internal/auth/web.go +++ b/internal/auth/web.go @@ -50,6 +50,7 @@ func (s *Handlers) Routes() chi.Router { r.Group(func(r chi.Router) { r.Use(s.Auth.RequireSSO) r.Get("/account", s.pageAccount) + r.Post("/account/username", s.handleChangeUsername) r.Post("/account/profile", s.handleUpdateProfile) r.Post("/account/password", s.handleChangePassword) r.Post("/account/passkeys/rename", s.handleRenamePasskey) diff --git a/internal/core/admin.go b/internal/core/admin.go index 0a7a928..b35de8d 100644 --- a/internal/core/admin.go +++ b/internal/core/admin.go @@ -155,3 +155,30 @@ func (s *Handlers) handleSetUserCaps(w http.ResponseWriter, r *http.Request) { } http.Redirect(w, r, "/admin/users", http.StatusSeeOther) } + +// usernameHistoryLimit bounds the rename history shown on the admin page. +const usernameHistoryLimit = 100 + +// pageUserHistory shows a user's username-change history. Admin-only (mounted +// under the manage-users capability): old usernames are never exposed elsewhere. +func (s *Handlers) pageUserHistory(w http.ResponseWriter, r *http.Request) { + id, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64) + if err != nil { + http.NotFound(w, r) + return + } + u, err := s.Store.GetUserByID(r.Context(), id) + if err != nil { + http.NotFound(w, r) + return + } + changes, err := s.Store.ListUsernameChanges(r.Context(), id, usernameHistoryLimit) + if err != nil { + http.Error(w, "could not load history", http.StatusInternalServerError) + return + } + s.Render(w, r, "admin_user_history.html", map[string]any{ + "Account": u, + "Changes": changes, + }) +} diff --git a/internal/core/templates/admin_user_history.html b/internal/core/templates/admin_user_history.html new file mode 100644 index 0000000..ceeee27 --- /dev/null +++ b/internal/core/templates/admin_user_history.html @@ -0,0 +1,43 @@ +{{define "title"}}Username history · MeshTender{{end}} +{{define "header"}} +
+
+
Administration
+

Username history

+
+
+{{end}} +{{define "content"}} +
+
+

+ Rename history for {{.Account.Name}} @{{.Account.Username}} + (user #{{.Account.ID}}). This record is visible to user managers only — old usernames are never shown + on public or member-facing pages. +

+ + {{if .Changes}} +
+ + + + + + {{range .Changes}} + + + + + + + {{end}} + +
WhenChangeByIP
{{.ChangedAt.Format "Jan 2, 2006 15:04"}}@{{.OldUsername}} → @{{.NewUsername}}{{if .BySelf}}self{{else if .ByActor}}admin (@{{.ByActor}}){{else}}admin{{end}}{{if .IP}}{{.IP}}{{else}}—{{end}}
+
+ {{else}} +

This user has never changed their username.

+ {{end}} +
+
+{{template "icon-arrow-left" "me-1"}}Users +{{end}} diff --git a/internal/core/templates/admin_users.html b/internal/core/templates/admin_users.html index 0e9f35d..3c3de3d 100644 --- a/internal/core/templates/admin_users.html +++ b/internal/core/templates/admin_users.html @@ -29,6 +29,7 @@ manage catalog + History {{end}} diff --git a/internal/core/web.go b/internal/core/web.go index 6f64cac..b79b3f9 100644 --- a/internal/core/web.go +++ b/internal/core/web.go @@ -165,6 +165,7 @@ func (s *Handlers) appRouter() chi.Router { r.With(s.requireCap(capCatalog)).Get("/catalog", s.pageCatalog) r.With(s.requireCap(capCatalog)).Post("/catalog/{id}", s.handleUpdateCommand) r.With(s.requireCap(capUsers)).Get("/users", s.pageUsers) + r.With(s.requireCap(capUsers)).Get("/users/{id}/history", s.pageUserHistory) r.With(s.requireCap(capUsers)).Post("/users/{id}", s.handleSetUserCaps) }) }) diff --git a/internal/store/command_log.go b/internal/store/command_log.go index 49d5320..9a69b35 100644 --- a/internal/store/command_log.go +++ b/internal/store/command_log.go @@ -14,7 +14,7 @@ type CommandLogEntry struct { ID int64 RepeaterID int64 UserID *int64 - SenderName string // username (or display name); "(deleted)" if user gone + SenderName string // username snapshotted at send time; "(deleted)" if unknown CommandText string SentAt time.Time AckReceived bool @@ -26,8 +26,8 @@ type CommandLogEntry struct { func (s *Store) LogCommand(ctx context.Context, repeaterID, userID, sessionID, commandID int64, text string) (int64, error) { var id int64 err := s.pool.QueryRow(ctx, ` - INSERT INTO command_log (repeater_id, user_id, session_id, command_id, command_text) - VALUES ($1, $2, $3, $4, $5) RETURNING id`, + INSERT INTO command_log (repeater_id, user_id, session_id, command_id, command_text, sender_username) + VALUES ($1, $2, $3, $4, $5, (SELECT username FROM users WHERE id = $2)) RETURNING id`, repeaterID, userID, sessionID, nullID(commandID), text).Scan(&id) if err != nil { return 0, fmt.Errorf("log command: %w", err) @@ -95,9 +95,8 @@ func (s *Store) ListCommandLogSessionsPage(ctx context.Context, repeaterID int64 // behavior of the previous query). headers, err := s.pool.Query(ctx, ` SELECT cs.id, cs.started_at, cs.ended_at, - COALESCE(NULLIF(su.display_name, ''), su.username, '(deleted)') + COALESCE(cs.sender_username, '(deleted)') FROM console_sessions cs - LEFT JOIN users su ON su.id = cs.user_id WHERE cs.repeater_id = $1 AND ($2 OR (cs.started_at, cs.id) < ($3, $4)) AND EXISTS (SELECT 1 FROM command_log l WHERE l.session_id = cs.id) @@ -172,11 +171,10 @@ type OwnerCommandLogEntry struct { func (s *Store) ListRecentCommandsForOwner(ctx context.Context, ownerID int64, limit int) ([]OwnerCommandLogEntry, error) { rows, err := s.pool.Query(ctx, ` SELECT l.repeater_id, r.public_id, r.name, - COALESCE(NULLIF(u.display_name, ''), u.username, '(deleted)'), + COALESCE(l.sender_username, '(deleted)'), l.command_text, l.sent_at, l.ack_received, l.response_text FROM command_log l JOIN repeaters r ON r.id = l.repeater_id - LEFT JOIN users u ON u.id = l.user_id WHERE r.owner_id = $1 ORDER BY l.sent_at DESC LIMIT $2`, ownerID, limit) @@ -195,10 +193,9 @@ func (s *Store) ListRecentCommandsForOwner(ctx context.Context, ownerID int64, l func (s *Store) ListCommandLog(ctx context.Context, repeaterID int64, limit int) ([]*CommandLogEntry, error) { rows, err := s.pool.Query(ctx, ` SELECT l.id, l.repeater_id, l.user_id, - COALESCE(NULLIF(u.display_name, ''), u.username, '(deleted)'), + COALESCE(l.sender_username, '(deleted)'), l.command_text, l.sent_at, l.ack_received, l.response_text FROM command_log l - LEFT JOIN users u ON u.id = l.user_id WHERE l.repeater_id = $1 ORDER BY l.sent_at DESC LIMIT $2`, repeaterID, limit) diff --git a/internal/store/console_sessions.go b/internal/store/console_sessions.go index ab87e6e..cccf705 100644 --- a/internal/store/console_sessions.go +++ b/internal/store/console_sessions.go @@ -8,8 +8,10 @@ import ( // StartConsoleSession records the start of a console session and returns its id. func (s *Store) StartConsoleSession(ctx context.Context, repeaterID, userID int64) (int64, error) { var id int64 + // Snapshot the username so the log stays a point-in-time record after renames. err := s.pool.QueryRow(ctx, - `INSERT INTO console_sessions (repeater_id, user_id) VALUES ($1, $2) RETURNING id`, + `INSERT INTO console_sessions (repeater_id, user_id, sender_username) + VALUES ($1, $2, (SELECT username FROM users WHERE id = $2)) RETURNING id`, repeaterID, userID).Scan(&id) if err != nil { return 0, fmt.Errorf("start console session: %w", err) diff --git a/internal/store/migrations/0020_username_changes.sql b/internal/store/migrations/0020_username_changes.sql new file mode 100644 index 0000000..c3857ac --- /dev/null +++ b/internal/store/migrations/0020_username_changes.sql @@ -0,0 +1,39 @@ +-- +goose Up +-- Usernames become user-changeable. The stable identity is still users.id; the +-- username is a renamable, unique, human-checkable handle. This migration adds +-- the machinery to keep that safe: +-- +-- 1. username_changes is an admin/security-only audit trail. It maps a +-- historical handle back to a user (forensics after a rename), backs the +-- release cooldown (a freed name is reserved for a window), and records who +-- made each change. Old usernames live here and are never shown publicly. +-- 2. sender_username on command_log / console_sessions snapshots the actor's +-- username at write time, so the command log is an immutable, point-in-time +-- record. It no longer depends on the live (renamable, and free-form, +-- spoofable) display name and it survives account deletion. +CREATE TABLE username_changes ( + id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + user_id BIGINT REFERENCES users(id) ON DELETE SET NULL, + old_username TEXT NOT NULL, + new_username TEXT NOT NULL, + changed_at TIMESTAMPTZ NOT NULL DEFAULT now(), + changed_by BIGINT REFERENCES users(id) ON DELETE SET NULL, + ip TEXT, + user_agent TEXT +); +-- Forensics: a single user's rename timeline. +CREATE INDEX username_changes_user_id_idx ON username_changes(user_id, changed_at DESC); +-- Cooldown lookup: was this name released recently, and by whom? +CREATE INDEX username_changes_old_username_idx ON username_changes(lower(old_username), changed_at DESC); + +ALTER TABLE command_log ADD COLUMN sender_username TEXT; +ALTER TABLE console_sessions ADD COLUMN sender_username TEXT; +-- Backfill existing rows from the current username (the best point-in-time +-- value available retroactively). +UPDATE command_log l SET sender_username = u.username FROM users u WHERE u.id = l.user_id; +UPDATE console_sessions cs SET sender_username = u.username FROM users u WHERE u.id = cs.user_id; + +-- +goose Down +ALTER TABLE console_sessions DROP COLUMN sender_username; +ALTER TABLE command_log DROP COLUMN sender_username; +DROP TABLE username_changes; diff --git a/internal/store/username.go b/internal/store/username.go new file mode 100644 index 0000000..69d01e9 --- /dev/null +++ b/internal/store/username.go @@ -0,0 +1,177 @@ +package store + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/jackc/pgx/v5" +) + +// Usernames are user-changeable, but with two guards that make a freed handle +// safe to leave behind: +// +// - UsernameReleaseCooldown: once a username is given up, nobody else may +// claim it for this window — only its previous owner can take it back. This +// blunts the impersonation/squatting attack where a renamed-away handle is +// grabbed to inherit its history. +// - UsernameRenameInterval: a user may only rename themselves this often. +// Admin-initiated changes bypass it. +const ( + UsernameReleaseCooldown = 90 * 24 * time.Hour + UsernameRenameInterval = 30 * 24 * time.Hour +) + +// ErrUsernameReserved is returned when a username was recently released by +// someone else and is still within its cooldown. It is deliberately distinct +// from ErrDuplicate internally, but callers should surface both as a generic +// "unavailable" so they don't reveal that a name was previously in use. +var ErrUsernameReserved = errors.New("store: username reserved") + +// ErrRenameTooSoon is returned when a self-service rename is attempted before +// the per-user rename interval has elapsed. +var ErrRenameTooSoon = errors.New("store: rename too soon") + +// UsernameChangeContext is the audit metadata recorded alongside a rename. +type UsernameChangeContext struct { + ChangedBy int64 // the acting user: the account itself, or an admin + IP string // client IP, "" if unknown + UserAgent string // client UA, "" if unknown +} + +// nameReservedByOther reports whether candidate was released within the cooldown +// by some user other than exceptUserID (pass 0 to match every prior owner, e.g. +// at signup where there is no incumbent). The previous owner is always allowed +// to reclaim their own freed name. q is the pool or an open transaction. +func nameReservedByOther(ctx context.Context, q rowQuerier, candidate string, exceptUserID int64) (bool, error) { + cutoff := time.Now().Add(-UsernameReleaseCooldown) + var reserved bool + err := q.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM username_changes + WHERE lower(old_username) = $1 + AND changed_at > $2 + AND user_id IS DISTINCT FROM $3 + )`, candidate, cutoff, exceptUserID).Scan(&reserved) + if err != nil { + return false, fmt.Errorf("check reserved: %w", err) + } + return reserved, nil +} + +// SetUsername renames userID to newUsername, recording the change in the audit +// trail. All checks and the update run in one transaction; the UNIQUE +// constraint is the final race guard. +// +// enforceInterval gates the per-user rename rate limit: pass true for +// self-service changes, false for admin-initiated ones (which still respect +// uniqueness and the release cooldown on names others hold). Returns nil with no +// change when newUsername already matches the current one. +func (s *Store) SetUsername(ctx context.Context, userID int64, newUsername string, meta UsernameChangeContext, enforceInterval bool) error { + return s.inTx(ctx, func(tx pgx.Tx) error { + var old string + if err := tx.QueryRow(ctx, `SELECT username FROM users WHERE id = $1 FOR UPDATE`, userID).Scan(&old); err != nil { + return notFoundOr(err, "get user") + } + if old == newUsername { + return nil // no-op + } + + if enforceInterval { + var last *time.Time + if err := tx.QueryRow(ctx, + `SELECT max(changed_at) FROM username_changes WHERE user_id = $1 AND changed_by = $1`, + userID).Scan(&last); err != nil { + return fmt.Errorf("rate check: %w", err) + } + if last != nil && last.After(time.Now().Add(-UsernameRenameInterval)) { + return ErrRenameTooSoon + } + } + + reserved, err := nameReservedByOther(ctx, tx, newUsername, userID) + if err != nil { + return err + } + if reserved { + return ErrUsernameReserved + } + + if _, err := tx.Exec(ctx, `UPDATE users SET username = $1 WHERE id = $2`, newUsername, userID); err != nil { + if isUniqueViolation(err) { + return ErrDuplicate + } + return fmt.Errorf("update username: %w", err) + } + + if _, err := tx.Exec(ctx, ` + INSERT INTO username_changes (user_id, old_username, new_username, changed_by, ip, user_agent) + VALUES ($1, $2, $3, $4, $5, $6)`, + userID, old, newUsername, meta.ChangedBy, nullStr(meta.IP), nullStr(meta.UserAgent)); err != nil { + return fmt.Errorf("record username change: %w", err) + } + return nil + }) +} + +// NextRenameAllowed returns the earliest time userID may change their own +// username again, or nil if they may change it now (no prior self-service change +// within the interval). +func (s *Store) NextRenameAllowed(ctx context.Context, userID int64) (*time.Time, error) { + var last *time.Time + if err := s.pool.QueryRow(ctx, + `SELECT max(changed_at) FROM username_changes WHERE user_id = $1 AND changed_by = $1`, + userID).Scan(&last); err != nil { + return nil, fmt.Errorf("next rename allowed: %w", err) + } + if last == nil { + return nil, nil + } + next := last.Add(UsernameRenameInterval) + if next.After(time.Now()) { + return &next, nil + } + return nil, nil +} + +// UsernameChange is one row of a user's rename history, for the admin view. +type UsernameChange struct { + OldUsername string + NewUsername string + ChangedAt time.Time + BySelf bool // changed_by == user_id + ByActor *string // acting username when admin-initiated and still known + IP *string + UserAgent *string +} + +// ListUsernameChanges returns a user's rename history, newest first. Admin-only: +// old usernames are never exposed on public or member-facing surfaces. +func (s *Store) ListUsernameChanges(ctx context.Context, userID int64, limit int) ([]UsernameChange, error) { + rows, err := s.pool.Query(ctx, ` + SELECT c.old_username, c.new_username, c.changed_at, + (c.changed_by IS NOT DISTINCT FROM c.user_id) AS by_self, + a.username, c.ip, c.user_agent + FROM username_changes c + LEFT JOIN users a ON a.id = c.changed_by + WHERE c.user_id = $1 + ORDER BY c.changed_at DESC, c.id DESC + LIMIT $2`, userID, limit) + if err != nil { + return nil, fmt.Errorf("list username changes: %w", err) + } + return collectRows(rows, func(r pgx.Row) (UsernameChange, error) { + var c UsernameChange + err := r.Scan(&c.OldUsername, &c.NewUsername, &c.ChangedAt, &c.BySelf, &c.ByActor, &c.IP, &c.UserAgent) + return c, err + }) +} + +// nullStr returns nil for an empty string so it stores as SQL NULL. +func nullStr(s string) *string { + if s == "" { + return nil + } + return &s +} diff --git a/internal/store/username_test.go b/internal/store/username_test.go new file mode 100644 index 0000000..793bfe7 --- /dev/null +++ b/internal/store/username_test.go @@ -0,0 +1,112 @@ +package store + +import ( + "errors" + "testing" +) + +// selfChange is the metadata for a user renaming themselves. +func selfChange(uid int64) UsernameChangeContext { + return UsernameChangeContext{ChangedBy: uid, IP: "203.0.113.7", UserAgent: "test-agent"} +} + +func TestSetUsername(t *testing.T) { + st, ctx := orgTestStore(t) + + mk := func(name string) int64 { + u, err := st.CreateUser(ctx, name, "") + if err != nil { + t.Fatalf("create %s: %v", name, err) + } + return u.ID + } + + t.Run("rename records audit and updates user", func(t *testing.T) { + uid := mk("alice") + if err := st.SetUsername(ctx, uid, "alice2", selfChange(uid), true); err != nil { + t.Fatalf("rename: %v", err) + } + u, err := st.GetUserByID(ctx, uid) + if err != nil || u.Username != "alice2" { + t.Fatalf("got %q (%v), want alice2", u.Username, err) + } + hist, err := st.ListUsernameChanges(ctx, uid, 10) + if err != nil { + t.Fatalf("history: %v", err) + } + if len(hist) != 1 { + t.Fatalf("got %d history rows, want 1", len(hist)) + } + h := hist[0] + if h.OldUsername != "alice" || h.NewUsername != "alice2" || !h.BySelf { + t.Fatalf("bad history row: %+v", h) + } + if h.IP == nil || *h.IP != "203.0.113.7" { + t.Fatalf("ip not recorded: %+v", h.IP) + } + }) + + t.Run("no-op rename writes no history", func(t *testing.T) { + uid := mk("samename") + if err := st.SetUsername(ctx, uid, "samename", selfChange(uid), true); err != nil { + t.Fatalf("no-op: %v", err) + } + hist, _ := st.ListUsernameChanges(ctx, uid, 10) + if len(hist) != 0 { + t.Fatalf("no-op wrote %d history rows", len(hist)) + } + }) + + t.Run("duplicate of active user is rejected", func(t *testing.T) { + a := mk("dupa") + mk("dupb") + if err := st.SetUsername(ctx, a, "dupb", selfChange(a), true); !errors.Is(err, ErrDuplicate) { + t.Fatalf("got %v, want ErrDuplicate", err) + } + }) + + t.Run("rate limit blocks a second self-service rename", func(t *testing.T) { + uid := mk("rl1") + if err := st.SetUsername(ctx, uid, "rl2", selfChange(uid), true); err != nil { + t.Fatalf("first rename: %v", err) + } + if err := st.SetUsername(ctx, uid, "rl3", selfChange(uid), true); !errors.Is(err, ErrRenameTooSoon) { + t.Fatalf("got %v, want ErrRenameTooSoon", err) + } + // An admin-initiated change (enforceInterval=false) bypasses the limit. + admin := mk("rladmin") + if err := st.SetUsername(ctx, uid, "rl3", UsernameChangeContext{ChangedBy: admin}, false); err != nil { + t.Fatalf("admin bypass: %v", err) + } + // NextRenameAllowed reflects the user's own recent change. + next, err := st.NextRenameAllowed(ctx, uid) + if err != nil { + t.Fatalf("next: %v", err) + } + if next == nil { + t.Fatalf("expected a cooldown deadline, got nil") + } + }) + + t.Run("released name is reserved from others but reclaimable by owner", func(t *testing.T) { + owner := mk("shared") + other := mk("other") + // Owner releases "shared". + if err := st.SetUsername(ctx, owner, "shared-new", selfChange(owner), true); err != nil { + t.Fatalf("release: %v", err) + } + // Someone else can't take it during the cooldown. + if err := st.SetUsername(ctx, other, "shared", selfChange(other), true); !errors.Is(err, ErrUsernameReserved) { + t.Fatalf("got %v, want ErrUsernameReserved", err) + } + // Signup can't grab it either. + if _, err := st.CreateUser(ctx, "shared", ""); !errors.Is(err, ErrUsernameReserved) { + t.Fatalf("signup got %v, want ErrUsernameReserved", err) + } + // The original owner may reclaim it (rate limit aside, tested via the + // admin path to isolate the cooldown rule). + if err := st.SetUsername(ctx, owner, "shared", UsernameChangeContext{ChangedBy: owner}, false); err != nil { + t.Fatalf("owner reclaim: %v", err) + } + }) +} diff --git a/internal/store/users.go b/internal/store/users.go index 00603b7..f2e7550 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -49,12 +49,23 @@ func scanUser(row pgx.Row) (*User, error) { } // CreateUser inserts a new user and returns it. displayName may be empty (then -// stored as NULL). Returns ErrDuplicate if the username is already taken. +// stored as NULL). Returns ErrDuplicate if the username is already taken, or +// ErrUsernameReserved if it was recently released by someone else and is still +// within its cooldown. func (s *Store) CreateUser(ctx context.Context, username, displayName string) (*User, error) { var dn *string if displayName != "" { dn = &displayName } + // A brand-new account has no incumbent identity, so any prior owner's recent + // release reserves the name (exceptUserID 0 matches no one). + reserved, err := nameReservedByOther(ctx, s.pool, username, 0) + if err != nil { + return nil, err + } + if reserved { + return nil, ErrUsernameReserved + } u, err := scanUser(s.pool.QueryRow(ctx, `INSERT INTO users (username, display_name) VALUES ($1, $2) RETURNING `+userCols, username, dn)) diff --git a/internal/web/ratelimit.go b/internal/web/ratelimit.go index 7f4a843..9362df3 100644 --- a/internal/web/ratelimit.go +++ b/internal/web/ratelimit.go @@ -94,3 +94,7 @@ func clientIP(r *http.Request) string { } return r.RemoteAddr } + +// ClientIP exposes clientIP for handlers that record the caller's address +// (e.g. the username-change audit trail). +func ClientIP(r *http.Request) string { return clientIP(r) }