From 980aaee0d90d7290e164775059d169cea6dc4fbe Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Wed, 29 Jul 2026 11:08:18 -0400 Subject: [PATCH] refactor(setup): derive setup-complete status from admin user count Setup completion was previously tracked by a manually-flipped setup_complete row in system_settings, written via a JWT-protected PUT /api/setup/complete endpoint. This meant any admin user created outside the setup wizard (future CLI, seed scripts, direct DB inserts) would not flip the switch, leaving the app stuck redirecting to /setup. The trigger is now derived from real data: setup is complete iff at least one admin user exists. This is self-correcting regardless of how users are created, and re-engages setup automatically if all admins are ever removed. Changes: - Add internal/setupstatus package with IsSetupComplete() (queries CountAdmins, 10s in-memory cache, fails open on DB error) and Invalidate() to clear the cache. Uses an AdminCounter interface to avoid importing the database package. - Add CountAdmins sqlc query (SELECT COUNT(*) FROM users WHERE role = 'admin') and regenerate. - Rewire router/setup.go isSetupComplete() to delegate to setupstatus; drop the old setup_complete setting read, cache vars, and the PUT /api/setup/complete route. - Call setupstatus.Invalidate() in the auth handler after CreateUser, UpdateUserRole, and DeleteUser so the cache reflects admin-count changes immediately. - Align first-user promotion in Register to key off !adminExists instead of len(users) == 0, so the two checks cannot diverge. - Remove the now-dead SetSetupComplete/GetSetupStatus handlers. - Drop the setup_complete seed row from schema.sql. - Remove the apiPut('/setup/complete') call from the setup wizard finishSetup(); the admin account created in submitAdmin already marks setup complete server-side. --- database/schema/schema.sql | 3 +- internal/database/querier.go | 1 + internal/database/queries.sql.go | 11 +++++ internal/database/queries/queries.sql | 3 ++ internal/handlers/auth.go | 45 +++++++++++-------- internal/handlers/system_settings.go | 26 ----------- internal/router/setup.go | 62 +-------------------------- internal/setupstatus/status.go | 62 +++++++++++++++++++++++++++ web/src/setup.ts | 8 ++-- 9 files changed, 111 insertions(+), 110 deletions(-) create mode 100644 internal/setupstatus/status.go diff --git a/database/schema/schema.sql b/database/schema/schema.sql index 25eee9f..5ecf45d 100644 --- a/database/schema/schema.sql +++ b/database/schema/schema.sql @@ -49,8 +49,7 @@ CREATE TABLE IF NOT EXISTS system_settings ( INSERT INTO system_settings (setting_key, setting_value, description) VALUES ('scan_poll_interval_seconds', '60', 'How often to scan all libraries in minutes'), ('auto_scan_enabled', 'true', 'Whether auto-scanning is enabled system-wide'), -('default_timezone', 'UTC', 'System default timezone'), -('setup_complete', 'false', 'Whether the initial setup wizard has been completed') +('default_timezone', 'UTC', 'System default timezone') ON CONFLICT (setting_key) DO NOTHING; -- Create refresh_tokens table diff --git a/internal/database/querier.go b/internal/database/querier.go index 1d7b71d..1412be4 100644 --- a/internal/database/querier.go +++ b/internal/database/querier.go @@ -30,6 +30,7 @@ type Querier interface { ClearDeviceSyncQueue(ctx context.Context, deviceID pgtype.UUID) error ClearKoboShelf(ctx context.Context, deviceID pgtype.UUID) error ClearKoboShelfByName(ctx context.Context, arg ClearKoboShelfByNameParams) error + CountAdmins(ctx context.Context) (int64, error) // Count unlinked books for a device CountUnlinkedBooks(ctx context.Context, deviceID pgtype.UUID) (int64, error) CountUserDevices(ctx context.Context, userID pgtype.UUID) (int64, error) diff --git a/internal/database/queries.sql.go b/internal/database/queries.sql.go index 03b44ea..d65967d 100644 --- a/internal/database/queries.sql.go +++ b/internal/database/queries.sql.go @@ -226,6 +226,17 @@ func (q *Queries) ClearKoboShelfByName(ctx context.Context, arg ClearKoboShelfBy return err } +const CountAdmins = `-- name: CountAdmins :one +SELECT COUNT(*) FROM users WHERE role = 'admin' +` + +func (q *Queries) CountAdmins(ctx context.Context) (int64, error) { + row := q.db.QueryRow(ctx, CountAdmins) + var count int64 + err := row.Scan(&count) + return count, err +} + const CountUnlinkedBooks = `-- name: CountUnlinkedBooks :one SELECT COUNT(*) as count FROM unlinked_books diff --git a/internal/database/queries/queries.sql b/internal/database/queries/queries.sql index a9fd9ea..55823de 100644 --- a/internal/database/queries/queries.sql +++ b/internal/database/queries/queries.sql @@ -346,6 +346,9 @@ WHERE role = 'admin' ORDER BY created_at ASC LIMIT 1; +-- name: CountAdmins :one +SELECT COUNT(*) FROM users WHERE role = 'admin'; + -- name: ReassignLibraries :exec UPDATE libraries SET created_by_admin_id = $2, updated_at = NOW() WHERE created_by_admin_id = $1; diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index e847377..8710290 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -6,6 +6,7 @@ package handlers import ( "bookhoard/internal/database" "bookhoard/internal/middleware" + "bookhoard/internal/setupstatus" "context" "errors" "fmt" @@ -82,22 +83,22 @@ type UserProfile struct { } type UpdateProfileRequest struct { - Username string `json:"username,omitempty" validate:"omitempty,min=3,max=50"` - Email string `json:"email,omitempty" validate:"omitempty,email"` - FirstName string `json:"first_name,omitempty" validate:"omitempty,max=100"` - LastName string `json:"last_name,omitempty" validate:"omitempty,max=100"` - Theme string `json:"theme,omitempty" validate:"omitempty"` - Timezone string `json:"timezone,omitempty" validate:"omitempty"` + Username string `json:"username,omitempty" form:"username" validate:"omitempty,min=3,max=50"` + Email string `json:"email,omitempty" form:"email" validate:"omitempty,email"` + FirstName string `json:"first_name,omitempty" form:"first_name" validate:"omitempty,max=100"` + LastName string `json:"last_name,omitempty" form:"last_name" validate:"omitempty,max=100"` + Theme string `json:"theme,omitempty" form:"theme" validate:"omitempty"` + Timezone string `json:"timezone,omitempty" form:"timezone" validate:"omitempty"` } type AdminUpdateUserRequest struct { - Username string `json:"username,omitempty" validate:"omitempty,min=3,max=50"` - Email string `json:"email,omitempty" validate:"omitempty,email"` - FirstName string `json:"first_name,omitempty" validate:"omitempty,max=100"` - LastName string `json:"last_name,omitempty" validate:"omitempty,max=100"` - Theme string `json:"theme,omitempty" validate:"omitempty"` - Timezone string `json:"timezone,omitempty" validate:"omitempty"` - Role string `json:"role,omitempty" validate:"omitempty,oneof=user admin"` + Username string `json:"username,omitempty" form:"username" validate:"omitempty,min=3,max=50"` + Email string `json:"email,omitempty" form:"email" validate:"omitempty,email"` + FirstName string `json:"first_name,omitempty" form:"first_name" validate:"omitempty,max=100"` + LastName string `json:"last_name,omitempty" form:"last_name" validate:"omitempty,max=100"` + Theme string `json:"theme,omitempty" form:"theme" validate:"omitempty"` + Timezone string `json:"timezone,omitempty" form:"timezone" validate:"omitempty"` + Role string `json:"role,omitempty" form:"role" validate:"omitempty,oneof=user admin"` } // Register handles POST /api/auth/register @@ -190,7 +191,7 @@ func (h *AuthHandler) Register(c *echo.Context) error { } var userRole string - if len(users) == 0 { + if !adminExists { userRole = "admin" } else { userRole = req.Role @@ -232,6 +233,10 @@ func (h *AuthHandler) Register(c *echo.Context) error { return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) } + // A new user may have changed the admin count (e.g. first user becomes + // admin), so refresh the setup-status cache. + setupstatus.Invalidate() + if err := h.CreateDefaultCollectionsForUser(c.Request().Context(), user.ID); err != nil { if c.Request().Header.Get("HX-Request") == "true" { return c.HTML(http.StatusInternalServerError, `
Failed to create default collections
`) @@ -548,6 +553,9 @@ func (h *AuthHandler) UpdateProfile(c *echo.Context) error { if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) } + + // Role changes can affect the admin count, so refresh the setup-status cache. + setupstatus.Invalidate() } // Update username (if provided) @@ -785,9 +793,9 @@ func (h *AuthHandler) UpdatePassword(c *echo.Context) error { } type PasswordRequest struct { - CurrentPassword string `json:"current_password,omitempty"` - NewPassword string `json:"new_password" validate:"required,passwordcomplex"` - ConfirmPassword string `json:"confirm_password" validate:"required"` + CurrentPassword string `json:"current_password,omitempty" form:"current_password"` + NewPassword string `json:"new_password" form:"new_password" validate:"required,passwordcomplex"` + ConfirmPassword string `json:"confirm_password" form:"confirm_password" validate:"required"` } var req PasswordRequest @@ -934,6 +942,9 @@ func (h *AuthHandler) DeleteUser(c *echo.Context) error { return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) } + // Deletion may have changed the admin count, so refresh the setup-status cache. + setupstatus.Invalidate() + // Create success message based on context var message string if targetUserID != "" && targetUserUUID.Bytes != currentUser.ID.Bytes { diff --git a/internal/handlers/system_settings.go b/internal/handlers/system_settings.go index fd3209f..7e3c1eb 100644 --- a/internal/handlers/system_settings.go +++ b/internal/handlers/system_settings.go @@ -95,32 +95,6 @@ func (h *SystemSettingsHandler) UpdateScanSettings(c *echo.Context) error { }) } -func (h *SystemSettingsHandler) SetSetupComplete(c *echo.Context) error { - err := h.db.UpdateSystemSetting(c.Request().Context(), database.UpdateSystemSettingParams{ - SettingKey: "setup_complete", - SettingValue: "true", - }) - if err != nil { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) - } - return c.JSON(http.StatusOK, map[string]string{"message": "setup complete"}) -} - -func (h *SystemSettingsHandler) GetSetupStatus(c *echo.Context) error { - val, err := h.db.GetSystemSetting(c.Request().Context(), "setup_complete") - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return c.JSON(http.StatusOK, map[string]bool{"setup_complete": false}) - } - return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) - } - complete, err := strconv.ParseBool(val) - if err != nil { - complete = false - } - return c.JSON(http.StatusOK, map[string]bool{"setup_complete": complete}) -} - func (h *SystemSettingsHandler) GetScanSettings(c *echo.Context) error { scanFrequencySetting, err := h.db.GetSystemSetting(c.Request().Context(), "scan_poll_interval_seconds") if err != nil { diff --git a/internal/router/setup.go b/internal/router/setup.go index d8e69a2..d951635 100644 --- a/internal/router/setup.go +++ b/internal/router/setup.go @@ -3,65 +3,18 @@ package router import ( "bytes" "context" - "errors" "log" "net/http" - "strconv" "strings" - "sync" - "time" + "bookhoard/internal/setupstatus" "bookhoard/templates" - "github.com/jackc/pgx/v5" "github.com/labstack/echo/v5" ) -var ( - setupCacheMu sync.RWMutex - setupCacheComplete bool = true - setupCacheExpiry time.Time - setupCacheTTL = 10 * time.Second -) - func isSetupComplete(cfg *Config) bool { - setupCacheMu.RLock() - if time.Now().Before(setupCacheExpiry) { - complete := setupCacheComplete - setupCacheMu.RUnlock() - return complete - } - setupCacheMu.RUnlock() - - val, err := cfg.Queries.GetSystemSetting(context.Background(), "setup_complete") - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - setupCacheMu.Lock() - setupCacheComplete = false - setupCacheExpiry = time.Now().Add(setupCacheTTL) - setupCacheMu.Unlock() - return false - } - return true - } - - complete, err := strconv.ParseBool(val) - if err != nil { - complete = false - } - - setupCacheMu.Lock() - setupCacheComplete = complete - setupCacheExpiry = time.Now().Add(setupCacheTTL) - setupCacheMu.Unlock() - return complete -} - -func invalidateSetupCache() { - setupCacheMu.Lock() - setupCacheComplete = true - setupCacheExpiry = time.Time{} - setupCacheMu.Unlock() + return setupstatus.IsSetupComplete(context.Background(), cfg.Queries) } func setupRedirectMiddleware(cfg *Config) echo.MiddlewareFunc { @@ -104,15 +57,4 @@ func registerSetupRoutes(cfg *Config) { } return c.HTML(http.StatusOK, buf.String()) }) - - jwtMiddleware := createJWTMiddleware(cfg) - protected := e.Group("/api/setup", jwtMiddleware) - protected.PUT("/complete", func(c *echo.Context) error { - err := cfg.SystemSettingsHandler.SetSetupComplete(c) - if err != nil { - return err - } - invalidateSetupCache() - return nil - }) } diff --git a/internal/setupstatus/status.go b/internal/setupstatus/status.go new file mode 100644 index 0000000..d95fc19 --- /dev/null +++ b/internal/setupstatus/status.go @@ -0,0 +1,62 @@ +// Package setupstatus reports whether the application's initial setup has been +// completed. Setup is considered complete as soon as at least one admin user +// exists, regardless of how that user was created (setup wizard, API, or a +// future CLI). This keeps the setup gate a derived property of real data +// rather than a manually-flipped flag that can drift out of sync. +package setupstatus + +import ( + "context" + "sync" + "time" +) + +// AdminCounter is satisfied by *database.Queries. It is defined as an interface +// here so this package does not import the database package, keeping the +// dependency graph flat and avoiding import cycles. +type AdminCounter interface { + CountAdmins(ctx context.Context) (int64, error) +} + +var ( + cacheMu sync.RWMutex + cacheComplete bool = true + cacheExpiry time.Time + cacheTTL = 10 * time.Second +) + +// IsSetupComplete reports whether setup is complete. Setup is complete when at +// least one admin user exists. A short in-memory cache avoids hammering the +// database on every request. On a database error the function fails open +// (returns true) so a transient outage does not lock users out of the app. +func IsSetupComplete(ctx context.Context, q AdminCounter) bool { + cacheMu.RLock() + if time.Now().Before(cacheExpiry) { + complete := cacheComplete + cacheMu.RUnlock() + return complete + } + cacheMu.RUnlock() + + count, err := q.CountAdmins(ctx) + complete := true + if err == nil { + complete = count > 0 + } + + cacheMu.Lock() + cacheComplete = complete + cacheExpiry = time.Now().Add(cacheTTL) + cacheMu.Unlock() + return complete +} + +// Invalidate clears the cached setup status so the next call to IsSetupComplete +// re-reads from the database. Call this after any write that could change the +// admin user count (user creation, role promotion/demotion, user deletion). +func Invalidate() { + cacheMu.Lock() + cacheComplete = true + cacheExpiry = time.Time{} + cacheMu.Unlock() +} diff --git a/web/src/setup.ts b/web/src/setup.ts index 94209e0..2031434 100644 --- a/web/src/setup.ts +++ b/web/src/setup.ts @@ -460,15 +460,13 @@ function initSetup(): void { async finishSetup() { this.loading = true; try { - const response = await apiPut("/setup/complete"); - await handleVoidResponse(response); + // Setup completion is derived from the existence of an admin user, so + // there is no separate "complete" endpoint to call. The admin account + // created in submitAdmin already marks setup as done server-side. if (this.libraries.length > 0) { setSelectedLibrary(this.libraries[0].id); } window.location.href = "/dashboard"; - } catch (err) { - handleError(err, "Failed to complete setup"); - window.location.href = "/dashboard"; } finally { this.loading = false; }