fix(auth): new users inherit the system-wide default timezone
default_timezone existed as a system setting (admin-editable, exposed via SettingsRegistry.DefaultTimezone) but nothing read it at user creation — the CreateUser query didn't even carry the column, so every new user got the DB default UTC regardless. Both creation paths (public register + admin create) now seed the user's timezone from the setting, validated with time.LoadLocation and falling back to UTC when unwired/empty/invalid. Test-infra fixes surfaced by the run: TestUpdateProfileSelfEdit was self-editing the SHARED admin (renaming it past the harness cleanup's preserve-list, leaking a second admin row that broke the last-admin tests) — it now edits a dedicated user with a unique name; the last-admin tests get an ensureSingleAdmin precondition helper; the websocket collection name is run-unique (non-test names persist in the shared DB and UNIQUE(user_id,name) 409s on rerun).
This commit is contained in:
1 parent
bc6ce41762
commit
740213f0a4
9 files changed
+108
-14
No files matched your search
@@ -524,6 +524,7 @@ func TestRegenerateDeviceToken_Forbidden(t *testing.T) {
|
|||||||
FirstName: pgtype.Text{String: "Different", Valid: true},
|
FirstName: pgtype.Text{String: "Different", Valid: true},
|
||||||
LastName: pgtype.Text{String: "User", Valid: true},
|
LastName: pgtype.Text{String: "User", Valid: true},
|
||||||
Role: "user",
|
Role: "user",
|
||||||
|
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||||
})
|
})
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
|||||||
@@ -190,6 +190,21 @@ func (s *OwnershipAndAdminTestSuite) TestAdminCreateUser() {
|
|||||||
assert.Equal(s.T(), unique, created["username"])
|
assert.Equal(s.T(), unique, created["username"])
|
||||||
assert.Equal(s.T(), "user", created["role"])
|
assert.Equal(s.T(), "user", created["role"])
|
||||||
|
|
||||||
|
// New users inherit the system-wide default timezone (this was the
|
||||||
|
// point of the setting; before 2026-10-13 every new user got UTC
|
||||||
|
// regardless of it).
|
||||||
|
expectedTZ := "UTC"
|
||||||
|
var settingTZ string
|
||||||
|
err = s.setup.DBPool.QueryRow(s.setup.QueueCtx,
|
||||||
|
"SELECT setting_value FROM system_settings WHERE setting_key = 'default_timezone'").Scan(&settingTZ)
|
||||||
|
if err == nil {
|
||||||
|
if _, locErr := time.LoadLocation(settingTZ); locErr == nil && settingTZ != "" {
|
||||||
|
expectedTZ = settingTZ
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert.Equal(s.T(), expectedTZ, created["timezone"],
|
||||||
|
"created user must carry the system default timezone")
|
||||||
|
|
||||||
// Duplicate username → 409.
|
// Duplicate username → 409.
|
||||||
req, _ = http.NewRequest("POST", s.setup.Server.URL+"/api/auth/users", bytes.NewBuffer(payload))
|
req, _ = http.NewRequest("POST", s.setup.Server.URL+"/api/auth/users", bytes.NewBuffer(payload))
|
||||||
req.Header.Set("Content-Type", "application/json")
|
req.Header.Set("Content-Type", "application/json")
|
||||||
|
|||||||
@@ -55,6 +55,7 @@ func createSyncTestUser(t *testing.T, db *database.Queries) pgtype.UUID {
|
|||||||
FirstName: pgtype.Text{String: "Test", Valid: true},
|
FirstName: pgtype.Text{String: "Test", Valid: true},
|
||||||
LastName: pgtype.Text{String: "Sync", Valid: true},
|
LastName: pgtype.Text{String: "Sync", Valid: true},
|
||||||
Role: "user",
|
Role: "user",
|
||||||
|
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||||
})
|
})
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
|||||||
@@ -246,6 +246,7 @@ func createRegularUserOnce(t *testing.T, db *database.Queries) UserTestData {
|
|||||||
FirstName: pgtype.Text{String: "Regular", Valid: true},
|
FirstName: pgtype.Text{String: "Regular", Valid: true},
|
||||||
LastName: pgtype.Text{String: "User", Valid: true},
|
LastName: pgtype.Text{String: "User", Valid: true},
|
||||||
Role: "user",
|
Role: "user",
|
||||||
|
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||||
})
|
})
|
||||||
require.NoError(t, err, "Should create regular test user")
|
require.NoError(t, err, "Should create regular test user")
|
||||||
|
|
||||||
@@ -696,6 +697,7 @@ func setupTestServer(t *testing.T) *TestServerSetup {
|
|||||||
FirstName: pgtype.Text{String: "Regular", Valid: true},
|
FirstName: pgtype.Text{String: "Regular", Valid: true},
|
||||||
LastName: pgtype.Text{String: "User", Valid: true},
|
LastName: pgtype.Text{String: "User", Valid: true},
|
||||||
Role: "user",
|
Role: "user",
|
||||||
|
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||||
})
|
})
|
||||||
require.NoError(t, err, "Failed to create regular test user")
|
require.NoError(t, err, "Failed to create regular test user")
|
||||||
|
|
||||||
|
|||||||
@@ -8,22 +8,65 @@ import (
|
|||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"bookhoard/internal/database"
|
||||||
|
|
||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
|
"github.com/jackc/pgx/v5/pgtype"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"golang.org/x/crypto/bcrypt"
|
"golang.org/x/crypto/bcrypt"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ensureSingleAdmin makes the "last admin" precondition deterministic in
|
||||||
|
// the shared test DB: only the preserved testuser admin remains. Other
|
||||||
|
// suites may have promoted or renamed users; the last-admin semantics
|
||||||
|
// under test require a true singleton.
|
||||||
|
func ensureSingleAdmin(t *testing.T, setup *TestServerSetup) {
|
||||||
|
ctx := context.Background()
|
||||||
|
_, err := setup.DBPool.Exec(ctx,
|
||||||
|
"DELETE FROM users WHERE role = 'admin' AND email != 'testuser@tests.bookhoard.internal'")
|
||||||
|
require.NoError(t, err, "Failed to prune extra admins")
|
||||||
|
|
||||||
|
if _, err := setup.DB.GetUserByEmail(ctx, "testuser@tests.bookhoard.internal"); err != nil {
|
||||||
|
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
|
||||||
|
_, cerr := setup.DB.CreateUser(ctx, database.CreateUserParams{
|
||||||
|
Email: "testuser@tests.bookhoard.internal",
|
||||||
|
Username: "testuser",
|
||||||
|
PasswordHash: passwordHash,
|
||||||
|
Role: "admin",
|
||||||
|
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||||
|
})
|
||||||
|
require.NoError(t, cerr, "Failed to restore the shared admin")
|
||||||
|
}
|
||||||
|
|
||||||
|
users, _ := setup.DB.ListUsers(ctx)
|
||||||
|
adminCount := 0
|
||||||
|
for _, u := range users {
|
||||||
|
if u.Role == "admin" {
|
||||||
|
adminCount++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
require.Equal(t, 1, adminCount, "precondition: exactly one admin")
|
||||||
|
}
|
||||||
|
|
||||||
func TestUpdateProfileSelfEdit(t *testing.T) {
|
func TestUpdateProfileSelfEdit(t *testing.T) {
|
||||||
t.Run("PUT /api/auth/profile - Update all fields successfully", func(t *testing.T) {
|
t.Run("PUT /api/auth/profile - Update all fields successfully", func(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
_ = createTestUserOnce(t, setup.DB)
|
// Self-edit a DEDICATED user, never the shared admin: renaming
|
||||||
token := setup.Token
|
// testuser here leaked a second admin row into the shared DB (the
|
||||||
|
// renamed email escapes the harness's preserve-list cleanup) and
|
||||||
|
// broke the last-admin tests downstream.
|
||||||
|
selfUser := createRegularUserOnce(t, setup.DB)
|
||||||
|
token := loginUserWithCredentials(t, setup.Server, selfUser.Email, selfUser.Password)
|
||||||
|
|
||||||
|
// Unique rename target: a fixed name collides with stale rows from
|
||||||
|
// earlier runs of this very test (409 username conflict).
|
||||||
|
suffix := uuid.New().String()[:8]
|
||||||
|
newUsername := "updateduser-" + suffix
|
||||||
updateData := map[string]interface{}{
|
updateData := map[string]interface{}{
|
||||||
"username": "updateduser",
|
"username": newUsername,
|
||||||
"email": "updated@tests.bookhoard.internal",
|
"email": "updated-" + suffix + "@tests.bookhoard.internal",
|
||||||
"first_name": "Updated",
|
"first_name": "Updated",
|
||||||
"last_name": "Name",
|
"last_name": "Name",
|
||||||
"theme": "dracula",
|
"theme": "dracula",
|
||||||
@@ -39,9 +82,9 @@ func TestUpdateProfileSelfEdit(t *testing.T) {
|
|||||||
|
|
||||||
assert.Equal(t, http.StatusOK, rec.Code)
|
assert.Equal(t, http.StatusOK, rec.Code)
|
||||||
|
|
||||||
user, err := setup.DB.GetUserByUsername(context.Background(), "updateduser")
|
user, err := setup.DB.GetUserByUsername(context.Background(), newUsername)
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Equal(t, "updated@tests.bookhoard.internal", user.Email)
|
assert.Equal(t, "updated-"+suffix+"@tests.bookhoard.internal", user.Email)
|
||||||
assert.Equal(t, "Updated", user.FirstName.String)
|
assert.Equal(t, "Updated", user.FirstName.String)
|
||||||
assert.Equal(t, "Name", user.LastName.String)
|
assert.Equal(t, "Name", user.LastName.String)
|
||||||
assert.Equal(t, "dracula", user.Theme.String)
|
assert.Equal(t, "dracula", user.Theme.String)
|
||||||
@@ -215,6 +258,10 @@ func TestUpdateProfileAdminMode(t *testing.T) {
|
|||||||
t.Run("PUT /api/auth/profile/:id - Try to demote last admin", func(t *testing.T) {
|
t.Run("PUT /api/auth/profile/:id - Try to demote last admin", func(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
|
// Deterministic singleton-admin precondition (shared DB may carry
|
||||||
|
// promoted/renamed admin rows from other suites).
|
||||||
|
ensureSingleAdmin(t, setup)
|
||||||
|
|
||||||
// Delete the regular user so we have only 1 admin (testuser)
|
// Delete the regular user so we have only 1 admin (testuser)
|
||||||
regularUser := createRegularUserOnce(t, setup.DB)
|
regularUser := createRegularUserOnce(t, setup.DB)
|
||||||
err := setup.DB.DeleteUser(context.Background(), uuidToPGType(regularUser.ID))
|
err := setup.DB.DeleteUser(context.Background(), uuidToPGType(regularUser.ID))
|
||||||
@@ -435,6 +482,7 @@ func TestDeleteUserConsolidated(t *testing.T) {
|
|||||||
t.Run("DELETE /api/auth/profile - Last admin tries self-delete", func(t *testing.T) {
|
t.Run("DELETE /api/auth/profile - Last admin tries self-delete", func(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
|
ensureSingleAdmin(t, setup)
|
||||||
_ = createTestUserOnce(t, setup.DB)
|
_ = createTestUserOnce(t, setup.DB)
|
||||||
token := setup.Token
|
token := setup.Token
|
||||||
|
|
||||||
@@ -471,6 +519,7 @@ func TestDeleteUserConsolidated(t *testing.T) {
|
|||||||
t.Run("DELETE /api/auth/profile/:id - Admin tries delete last admin", func(t *testing.T) {
|
t.Run("DELETE /api/auth/profile/:id - Admin tries delete last admin", func(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
|
|
||||||
|
ensureSingleAdmin(t, setup)
|
||||||
_ = createTestUserOnce(t, setup.DB)
|
_ = createTestUserOnce(t, setup.DB)
|
||||||
token := setup.Token
|
token := setup.Token
|
||||||
|
|
||||||
|
|||||||
@@ -240,9 +240,11 @@ func TestWebSocketUserScopedBroadcast(t *testing.T) {
|
|||||||
// Use pre-created admin user (setup.Token) and regular user (setup.RegularToken)
|
// Use pre-created admin user (setup.Token) and regular user (setup.RegularToken)
|
||||||
// Both users already created by setupTestServer()
|
// Both users already created by setupTestServer()
|
||||||
|
|
||||||
// Create a collection for admin user
|
// Create a collection for admin user. Unique name: the shared DB keeps
|
||||||
|
// non-"test"-named collections across runs, and UNIQUE(user_id, name)
|
||||||
|
// turns a fixed name into a 409 on the second run.
|
||||||
collectionReq := map[string]interface{}{
|
collectionReq := map[string]interface{}{
|
||||||
"name": "Admin Collection",
|
"name": "Admin Collection " + uuid.New().String()[:8],
|
||||||
"description": "Test collection for WebSocket test",
|
"description": "Test collection for WebSocket test",
|
||||||
}
|
}
|
||||||
collectionBody, _ := json.Marshal(collectionReq)
|
collectionBody, _ := json.Marshal(collectionReq)
|
||||||
|
|||||||
@@ -2003,9 +2003,9 @@ func (q *Queries) CreateUnlinkedBook(ctx context.Context, arg CreateUnlinkedBook
|
|||||||
}
|
}
|
||||||
|
|
||||||
const CreateUser = `-- name: CreateUser :one
|
const CreateUser = `-- name: CreateUser :one
|
||||||
INSERT INTO users (email, username, password_hash, first_name, last_name, theme, role)
|
INSERT INTO users (email, username, password_hash, first_name, last_name, theme, role, timezone)
|
||||||
VALUES ($1, $2, $3, $4, $5, $6, $7)
|
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
|
||||||
RETURNING id, email, username, theme, first_name, last_name, role, created_at, updated_at
|
RETURNING id, email, username, theme, first_name, last_name, role, timezone, created_at, updated_at
|
||||||
`
|
`
|
||||||
|
|
||||||
type CreateUserParams struct {
|
type CreateUserParams struct {
|
||||||
@@ -2016,6 +2016,7 @@ type CreateUserParams struct {
|
|||||||
LastName pgtype.Text `db:"last_name" json:"last_name"`
|
LastName pgtype.Text `db:"last_name" json:"last_name"`
|
||||||
Theme pgtype.Text `db:"theme" json:"theme"`
|
Theme pgtype.Text `db:"theme" json:"theme"`
|
||||||
Role string `db:"role" json:"role"`
|
Role string `db:"role" json:"role"`
|
||||||
|
Timezone pgtype.Text `db:"timezone" json:"timezone"`
|
||||||
}
|
}
|
||||||
|
|
||||||
type CreateUserRow struct {
|
type CreateUserRow struct {
|
||||||
@@ -2026,6 +2027,7 @@ type CreateUserRow struct {
|
|||||||
FirstName pgtype.Text `db:"first_name" json:"first_name"`
|
FirstName pgtype.Text `db:"first_name" json:"first_name"`
|
||||||
LastName pgtype.Text `db:"last_name" json:"last_name"`
|
LastName pgtype.Text `db:"last_name" json:"last_name"`
|
||||||
Role string `db:"role" json:"role"`
|
Role string `db:"role" json:"role"`
|
||||||
|
Timezone pgtype.Text `db:"timezone" json:"timezone"`
|
||||||
CreatedAt pgtype.Timestamptz `db:"created_at" json:"created_at"`
|
CreatedAt pgtype.Timestamptz `db:"created_at" json:"created_at"`
|
||||||
UpdatedAt pgtype.Timestamptz `db:"updated_at" json:"updated_at"`
|
UpdatedAt pgtype.Timestamptz `db:"updated_at" json:"updated_at"`
|
||||||
}
|
}
|
||||||
@@ -2039,6 +2041,7 @@ func (q *Queries) CreateUser(ctx context.Context, arg CreateUserParams) (CreateU
|
|||||||
arg.LastName,
|
arg.LastName,
|
||||||
arg.Theme,
|
arg.Theme,
|
||||||
arg.Role,
|
arg.Role,
|
||||||
|
arg.Timezone,
|
||||||
)
|
)
|
||||||
var i CreateUserRow
|
var i CreateUserRow
|
||||||
err := row.Scan(
|
err := row.Scan(
|
||||||
@@ -2049,6 +2052,7 @@ func (q *Queries) CreateUser(ctx context.Context, arg CreateUserParams) (CreateU
|
|||||||
&i.FirstName,
|
&i.FirstName,
|
||||||
&i.LastName,
|
&i.LastName,
|
||||||
&i.Role,
|
&i.Role,
|
||||||
|
&i.Timezone,
|
||||||
&i.CreatedAt,
|
&i.CreatedAt,
|
||||||
&i.UpdatedAt,
|
&i.UpdatedAt,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
-- name: CreateUser :one
|
-- name: CreateUser :one
|
||||||
INSERT INTO users (email, username, password_hash, first_name, last_name, theme, role)
|
INSERT INTO users (email, username, password_hash, first_name, last_name, theme, role, timezone)
|
||||||
VALUES ($1, $2, $3, $4, $5, $6, $7)
|
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
|
||||||
RETURNING id, email, username, theme, first_name, last_name, role, created_at, updated_at;
|
RETURNING id, email, username, theme, first_name, last_name, role, timezone, created_at, updated_at;
|
||||||
|
|
||||||
-- name: GetUserByEmail :one
|
-- name: GetUserByEmail :one
|
||||||
SELECT id, email, username, theme, first_name, last_name, role, created_at, updated_at FROM users WHERE email = $1;
|
SELECT id, email, username, theme, first_name, last_name, role, created_at, updated_at FROM users WHERE email = $1;
|
||||||
|
|||||||
@@ -39,6 +39,23 @@ var SessionDurationSec = int(DefaultSessionDuration.Seconds())
|
|||||||
// SetSettings wires the tunable settings registry (optional).
|
// SetSettings wires the tunable settings registry (optional).
|
||||||
func (h *AuthHandler) SetSettings(s *database.SettingsRegistry) { h.settings = s }
|
func (h *AuthHandler) SetSettings(s *database.SettingsRegistry) { h.settings = s }
|
||||||
|
|
||||||
|
// defaultTimezone returns the system-wide default timezone for new users.
|
||||||
|
// Falls back to UTC when no registry is wired, the setting is empty, or
|
||||||
|
// the value is not a valid IANA location.
|
||||||
|
func (h *AuthHandler) defaultTimezone() string {
|
||||||
|
if h.settings == nil {
|
||||||
|
return "UTC"
|
||||||
|
}
|
||||||
|
tz := h.settings.DefaultTimezone()
|
||||||
|
if tz == "" {
|
||||||
|
return "UTC"
|
||||||
|
}
|
||||||
|
if _, err := time.LoadLocation(tz); err != nil {
|
||||||
|
return "UTC"
|
||||||
|
}
|
||||||
|
return tz
|
||||||
|
}
|
||||||
|
|
||||||
// sessionDuration returns the active session duration from the registry.
|
// sessionDuration returns the active session duration from the registry.
|
||||||
func (h *AuthHandler) sessionDuration() time.Duration {
|
func (h *AuthHandler) sessionDuration() time.Duration {
|
||||||
if h.settings != nil {
|
if h.settings != nil {
|
||||||
@@ -263,6 +280,7 @@ func (h *AuthHandler) Register(c *echo.Context) error {
|
|||||||
LastName: pgtype.Text{String: req.LastName, Valid: req.LastName != ""},
|
LastName: pgtype.Text{String: req.LastName, Valid: req.LastName != ""},
|
||||||
Theme: pgtype.Text{String: "tokyo-night", Valid: true},
|
Theme: pgtype.Text{String: "tokyo-night", Valid: true},
|
||||||
Role: userRole,
|
Role: userRole,
|
||||||
|
Timezone: pgtype.Text{String: h.defaultTimezone(), Valid: true},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if c.Request().Header.Get("HX-Request") == "true" {
|
if c.Request().Header.Get("HX-Request") == "true" {
|
||||||
@@ -735,6 +753,7 @@ func (h *AuthHandler) AdminCreateUser(c *echo.Context) error {
|
|||||||
LastName: pgtype.Text{String: req.LastName, Valid: req.LastName != ""},
|
LastName: pgtype.Text{String: req.LastName, Valid: req.LastName != ""},
|
||||||
Theme: pgtype.Text{String: "tokyo-night", Valid: true},
|
Theme: pgtype.Text{String: "tokyo-night", Valid: true},
|
||||||
Role: role,
|
Role: role,
|
||||||
|
Timezone: pgtype.Text{String: h.defaultTimezone(), Valid: true},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||||
@@ -756,6 +775,7 @@ func (h *AuthHandler) AdminCreateUser(c *echo.Context) error {
|
|||||||
"email": user.Email,
|
"email": user.Email,
|
||||||
"username": user.Username,
|
"username": user.Username,
|
||||||
"role": user.Role,
|
"role": user.Role,
|
||||||
|
"timezone": user.Timezone,
|
||||||
"created_at": user.CreatedAt,
|
"created_at": user.CreatedAt,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user