From 46d38afe2dfe5965624387869280360718d4f273 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 7 May 2026 12:17:14 -0400 Subject: [PATCH] feat(server/m7-user-mgmt): admin user CRUD endpoints (U2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four new admin endpoints under existing RequireAdmin middleware: - POST /api/admin/users — admin-creates-user. Body {username, password, display_name?, is_admin?}. Same username + password validation as the public /register handler. 409 on duplicate. Audits ActionCreateUserAdmin. - DELETE /api/admin/users/{id} — hard delete. Last-admin guard refuses delete when target is the only admin (409). Schema's ON DELETE CASCADE on user-FK tables handles plays/likes/ sessions cleanup. Audits ActionDeleteUser with target's username + was_admin flag. - POST /api/admin/users/{id}/reset-password — body {password}. 8-char minimum. Admin sets a new password without knowing the old one. Audits ActionPasswordResetAdmin. - PUT /api/admin/users/{id}/auto-approve — body {auto_approve: bool}. Toggles the per-user flag added in T1 (the #355 sub-feature surface). Audits ActionAutoApproveToggle. adminUserView shape extended with auto_approve_requests so the list and toggle responses carry the flag. ListUsers SQL query updated to include auto_approve_requests; generated Go updated to match. Tests cover happy paths, last-admin guard on delete, password validation, duplicate username, and the auto-approve round-trip. Co-Authored-By: Claude Sonnet 4.6 --- internal/api/admin_users.go | 284 +++++++++++++++++++++++++++++-- internal/api/admin_users_test.go | 182 ++++++++++++++++++++ internal/api/api.go | 4 + internal/db/dbq/users.sql.go | 14 +- internal/db/queries/users.sql | 2 +- 5 files changed, 463 insertions(+), 23 deletions(-) diff --git a/internal/api/admin_users.go b/internal/api/admin_users.go index b67236e9..7d5e6b64 100644 --- a/internal/api/admin_users.go +++ b/internal/api/admin_users.go @@ -2,11 +2,15 @@ package api import ( "encoding/json" + "errors" "net/http" "time" "github.com/go-chi/chi/v5" - "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgconn" + "github.com/jackc/pgerrcode" + "github.com/jackc/pgx/v5" + "golang.org/x/crypto/bcrypt" "git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/auth" @@ -14,11 +18,12 @@ import ( ) type adminUserView struct { - ID string `json:"id"` - Username string `json:"username"` - DisplayName *string `json:"display_name"` - IsAdmin bool `json:"is_admin"` - CreatedAt string `json:"created_at"` + ID string `json:"id"` + Username string `json:"username"` + DisplayName *string `json:"display_name"` + IsAdmin bool `json:"is_admin"` + AutoApproveRequests bool `json:"auto_approve_requests"` + CreatedAt string `json:"created_at"` } type listUsersResp struct { @@ -36,11 +41,12 @@ func (h *handlers) handleAdminListUsers(w http.ResponseWriter, r *http.Request) out := make([]adminUserView, 0, len(rows)) for _, row := range rows { v := adminUserView{ - ID: uuidToString(row.ID), - Username: row.Username, - DisplayName: row.DisplayName, - IsAdmin: row.IsAdmin, - CreatedAt: row.CreatedAt.Time.UTC().Format(time.RFC3339), + ID: uuidToString(row.ID), + Username: row.Username, + DisplayName: row.DisplayName, + IsAdmin: row.IsAdmin, + AutoApproveRequests: row.AutoApproveRequests, + CreatedAt: row.CreatedAt.Time.UTC().Format(time.RFC3339), } out = append(out, v) } @@ -119,10 +125,256 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request) } writeJSON(w, http.StatusOK, adminUserView{ - ID: uuidToString(updated.ID), - Username: updated.Username, - DisplayName: updated.DisplayName, - IsAdmin: updated.IsAdmin, - CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), + ID: uuidToString(updated.ID), + Username: updated.Username, + DisplayName: updated.DisplayName, + IsAdmin: updated.IsAdmin, + AutoApproveRequests: updated.AutoApproveRequests, + CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), + }) +} + +// --- Admin user create ----------------------------------------------------- + +type adminCreateUserReq struct { + Username string `json:"username"` + Password string `json:"password"` + DisplayName *string `json:"display_name"` + IsAdmin bool `json:"is_admin"` +} + +func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request) { + caller, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "auth_required", "") + return + } + var req adminCreateUserReq + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, http.StatusBadRequest, "invalid_body", "") + return + } + if !usernameRe.MatchString(req.Username) { + writeErr(w, http.StatusBadRequest, "username_invalid", "username must be 3-32 chars, alphanumeric + underscore + hyphen") + return + } + if len(req.Password) < minPasswordLength { + writeErr(w, http.StatusBadRequest, "password_too_short", "password must be at least 8 chars") + return + } + + hash, err := bcrypt.GenerateFromPassword([]byte(req.Password), bcrypt.DefaultCost) + if err != nil { + h.logger.Error("admin create user: hash failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + apiToken, err := auth.MintSessionToken() + if err != nil { + h.logger.Error("admin create user: mint token failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + q := dbq.New(h.pool) + user, err := q.CreateUserAdmin(r.Context(), dbq.CreateUserAdminParams{ + Username: req.Username, + PasswordHash: string(hash), + ApiToken: apiToken, + IsAdmin: req.IsAdmin, + DisplayName: req.DisplayName, + }) + if err != nil { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == pgerrcode.UniqueViolation { + writeErr(w, http.StatusConflict, "username_taken", "") + return + } + h.logger.Error("admin create user: insert failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + if err := audit.Write(r.Context(), h.pool, caller.ID, user.ID, audit.ActionCreateUserAdmin, + map[string]any{"is_admin": req.IsAdmin}); err != nil { + h.logger.Warn("admin create user: audit failed", "err", err) + } + + writeJSON(w, http.StatusOK, adminUserView{ + ID: uuidToString(user.ID), + Username: user.Username, + DisplayName: user.DisplayName, + IsAdmin: user.IsAdmin, + AutoApproveRequests: user.AutoApproveRequests, + CreatedAt: user.CreatedAt.Time.UTC().Format(time.RFC3339), + }) +} + +// --- Admin user delete ----------------------------------------------------- + +func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request) { + caller, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "auth_required", "") + return + } + targetID, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeErr(w, http.StatusBadRequest, "invalid_id", "") + return + } + + q := dbq.New(h.pool) + target, err := q.GetUserByID(r.Context(), targetID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, http.StatusNotFound, "not_found", "") + return + } + h.logger.Error("admin delete user: lookup failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + // Last-admin guard: refuse delete if target is the only admin. + if target.IsAdmin { + count, cerr := q.CountAdmins(r.Context()) + if cerr != nil { + h.logger.Error("admin delete user: count admins failed", "err", cerr) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + if count <= 1 { + writeErr(w, http.StatusConflict, "last_admin", + "can't delete the last admin — promote someone else first") + return + } + } + + if err := q.DeleteUser(r.Context(), targetID); err != nil { + h.logger.Error("admin delete user: delete failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionDeleteUser, + map[string]any{"username": target.Username, "was_admin": target.IsAdmin}); err != nil { + h.logger.Warn("admin delete user: audit failed", "err", err) + } + + w.WriteHeader(http.StatusNoContent) +} + +// --- Admin password reset -------------------------------------------------- + +type adminResetPasswordReq struct { + Password string `json:"password"` +} + +func (h *handlers) handleAdminResetPassword(w http.ResponseWriter, r *http.Request) { + caller, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "auth_required", "") + return + } + targetID, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeErr(w, http.StatusBadRequest, "invalid_id", "") + return + } + var req adminResetPasswordReq + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, http.StatusBadRequest, "invalid_body", "") + return + } + if len(req.Password) < minPasswordLength { + writeErr(w, http.StatusBadRequest, "password_too_short", "password must be at least 8 chars") + return + } + + q := dbq.New(h.pool) + // Verify target exists for a useful 404. + if _, err := q.GetUserByID(r.Context(), targetID); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, http.StatusNotFound, "not_found", "") + return + } + h.logger.Error("admin reset password: lookup failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + hash, err := bcrypt.GenerateFromPassword([]byte(req.Password), bcrypt.DefaultCost) + if err != nil { + h.logger.Error("admin reset password: hash failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + if err := q.ResetUserPassword(r.Context(), dbq.ResetUserPasswordParams{ + ID: targetID, + PasswordHash: string(hash), + }); err != nil { + h.logger.Error("admin reset password: update failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionPasswordResetAdmin, nil); err != nil { + h.logger.Warn("admin reset password: audit failed", "err", err) + } + + w.WriteHeader(http.StatusNoContent) +} + +// --- Admin auto-approve toggle --------------------------------------------- + +type adminAutoApproveReq struct { + AutoApprove bool `json:"auto_approve"` +} + +func (h *handlers) handleAdminAutoApproveToggle(w http.ResponseWriter, r *http.Request) { + caller, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "auth_required", "") + return + } + targetID, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeErr(w, http.StatusBadRequest, "invalid_id", "") + return + } + var req adminAutoApproveReq + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, http.StatusBadRequest, "invalid_body", "") + return + } + + q := dbq.New(h.pool) + updated, err := q.UpdateUserAutoApprove(r.Context(), dbq.UpdateUserAutoApproveParams{ + ID: targetID, + AutoApproveRequests: req.AutoApprove, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, http.StatusNotFound, "not_found", "") + return + } + h.logger.Error("admin auto-approve: update failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "") + return + } + + if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionAutoApproveToggle, + map[string]any{"auto_approve": req.AutoApprove}); err != nil { + h.logger.Warn("admin auto-approve: audit failed", "err", err) + } + + writeJSON(w, http.StatusOK, adminUserView{ + ID: uuidToString(updated.ID), + Username: updated.Username, + DisplayName: updated.DisplayName, + IsAdmin: updated.IsAdmin, + AutoApproveRequests: updated.AutoApproveRequests, + CreatedAt: updated.CreatedAt.Time.UTC().Format(time.RFC3339), }) } diff --git a/internal/api/admin_users_test.go b/internal/api/admin_users_test.go index 9cd576fa..44f35002 100644 --- a/internal/api/admin_users_test.go +++ b/internal/api/admin_users_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/go-chi/chi/v5" + "golang.org/x/crypto/bcrypt" "git.fabledsword.com/bvandeusen/minstrel/internal/auth" ) @@ -20,6 +21,11 @@ func newAdminUsersRouter(h *handlers) chi.Router { admin.Use(auth.RequireAdmin()) admin.Get("/users", h.handleAdminListUsers) admin.Put("/users/{id}/admin", h.handleUpdateUserAdmin) + // U2 additions: + admin.Post("/users", h.handleAdminCreateUser) + admin.Delete("/users/{id}", h.handleAdminDeleteUser) + admin.Post("/users/{id}/reset-password", h.handleAdminResetPassword) + admin.Put("/users/{id}/auto-approve", h.handleAdminAutoApproveToggle) }) return r } @@ -123,3 +129,179 @@ func TestAdminUsers_Demote_OtherAdmin_Allowed(t *testing.T) { t.Errorf("status = %d, want 200 (two admins exist; demoting one is fine); body=%s", rec.Code, rec.Body.String()) } } + +func TestAdminCreateUser_Happy(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "creator", "pw", true) + + body := `{"username":"newuser1","password":"abcd1234","is_admin":false}` + req := httptest.NewRequest(http.MethodPost, "/api/admin/users", bytes.NewReader([]byte(body))) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var resp adminUserView + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Username != "newuser1" || resp.IsAdmin { + t.Errorf("response unexpected: %+v", resp) + } +} + +func TestAdminCreateUser_DuplicateUsername_409(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "duper", "pw", true) + seedUser(t, pool, "existing", "pw", false) + + body := `{"username":"existing","password":"abcd1234"}` + req := httptest.NewRequest(http.MethodPost, "/api/admin/users", bytes.NewReader([]byte(body))) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusConflict { + t.Errorf("status = %d, want 409", rec.Code) + } +} + +func TestAdminDeleteUser_Happy(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "delcaller", "pw", true) + target := seedUser(t, pool, "deltarget", "pw", false) + + req := httptest.NewRequest(http.MethodDelete, "/api/admin/users/"+uuidToString(target.ID), nil) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Errorf("status = %d, want 204", rec.Code) + } + var count int + if err := pool.QueryRow(context.Background(), + "SELECT count(*) FROM users WHERE id = $1", target.ID).Scan(&count); err != nil { + t.Fatalf("verify: %v", err) + } + if count != 0 { + t.Errorf("user still exists after delete (count=%d)", count) + } +} + +func TestAdminDeleteUser_LastAdmin_Refused(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + if _, err := pool.Exec(context.Background(), "DELETE FROM users"); err != nil { + t.Fatalf("reset: %v", err) + } + soleAdmin := seedUser(t, pool, "soleadmin", "pw", true) + + req := httptest.NewRequest(http.MethodDelete, "/api/admin/users/"+uuidToString(soleAdmin.ID), nil) + req = withUser(req, soleAdmin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusConflict { + t.Errorf("status = %d, want 409 (last_admin)", rec.Code) + } +} + +func TestAdminResetPassword_Happy(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "rcaller", "pw", true) + target := seedUser(t, pool, "rtarget", "oldpw1234", false) + + body := `{"password":"newpassword1"}` + req := httptest.NewRequest(http.MethodPost, "/api/admin/users/"+uuidToString(target.ID)+"/reset-password", + bytes.NewReader([]byte(body))) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Errorf("status = %d, want 204; body=%s", rec.Code, rec.Body.String()) + } + // Old password should no longer match. + var hash string + if err := pool.QueryRow(context.Background(), + "SELECT password_hash FROM users WHERE id = $1", target.ID).Scan(&hash); err != nil { + t.Fatalf("read hash: %v", err) + } + if err := bcrypt.CompareHashAndPassword([]byte(hash), []byte("oldpw1234")); err == nil { + t.Errorf("old password still matches; reset did nothing") + } +} + +func TestAdminResetPassword_TooShort_400(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "rsadm", "pw", true) + target := seedUser(t, pool, "rstgt", "pw", false) + + body := `{"password":"abc"}` + req := httptest.NewRequest(http.MethodPost, "/api/admin/users/"+uuidToString(target.ID)+"/reset-password", + bytes.NewReader([]byte(body))) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } +} + +func TestAdminAutoApproveToggle(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + admin := seedUser(t, pool, "aacaller", "pw", true) + target := seedUser(t, pool, "aatarget", "pw", false) + + body := `{"auto_approve":true}` + req := httptest.NewRequest(http.MethodPut, "/api/admin/users/"+uuidToString(target.ID)+"/auto-approve", + bytes.NewReader([]byte(body))) + req = withUser(req, admin) + rec := httptest.NewRecorder() + newAdminUsersRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var resp adminUserView + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if !resp.AutoApproveRequests { + t.Errorf("auto_approve_requests = false on response; expected true") + } + + // Verify DB. + var aa bool + if err := pool.QueryRow(context.Background(), + "SELECT auto_approve_requests FROM users WHERE id = $1", target.ID).Scan(&aa); err != nil { + t.Fatalf("verify: %v", err) + } + if !aa { + t.Errorf("DB row not updated") + } +} diff --git a/internal/api/api.go b/internal/api/api.go index 3140266c..54629bd1 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -127,6 +127,10 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Delete("/invites/{token}", h.handleDeleteInvite) admin.Get("/users", h.handleAdminListUsers) admin.Put("/users/{id}/admin", h.handleUpdateUserAdmin) + admin.Post("/users", h.handleAdminCreateUser) + admin.Delete("/users/{id}", h.handleAdminDeleteUser) + admin.Post("/users/{id}/reset-password", h.handleAdminResetPassword) + admin.Put("/users/{id}/auto-approve", h.handleAdminAutoApproveToggle) admin.Get("/cover-sources", h.handleListCoverSources) admin.Patch("/cover-sources/{provider_id}", h.handleUpdateCoverSource) diff --git a/internal/db/dbq/users.sql.go b/internal/db/dbq/users.sql.go index 531b3d80..a221b73f 100644 --- a/internal/db/dbq/users.sql.go +++ b/internal/db/dbq/users.sql.go @@ -279,17 +279,18 @@ func (q *Queries) GetUserByUsername(ctx context.Context, username string) (User, } const listUsers = `-- name: ListUsers :many -SELECT id, username, display_name, is_admin, created_at +SELECT id, username, display_name, is_admin, auto_approve_requests, created_at FROM users ORDER BY created_at DESC ` type ListUsersRow struct { - ID pgtype.UUID - Username string - DisplayName *string - IsAdmin bool - CreatedAt pgtype.Timestamptz + ID pgtype.UUID + Username string + DisplayName *string + IsAdmin bool + AutoApproveRequests bool + CreatedAt pgtype.Timestamptz } // Admin user-management list. Sort newest-first. @@ -307,6 +308,7 @@ func (q *Queries) ListUsers(ctx context.Context) ([]ListUsersRow, error) { &i.Username, &i.DisplayName, &i.IsAdmin, + &i.AutoApproveRequests, &i.CreatedAt, ); err != nil { return nil, err diff --git a/internal/db/queries/users.sql b/internal/db/queries/users.sql index 7170cfca..55084c65 100644 --- a/internal/db/queries/users.sql +++ b/internal/db/queries/users.sql @@ -55,7 +55,7 @@ WHERE id = $1; -- name: ListUsers :many -- Admin user-management list. Sort newest-first. -SELECT id, username, display_name, is_admin, created_at +SELECT id, username, display_name, is_admin, auto_approve_requests, created_at FROM users ORDER BY created_at DESC;