diff --git a/CHANGELOG.md b/CHANGELOG.md index 57e0f65..e1660b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version stored earlier, or imported from another instance, keeps its case, since its client uses it (audit 2026-10-12, H4). +- **The last global administrator stays.** Deleting or demoting a global + administrator counted the others first and then wrote, so two + administrators deleting or demoting each other at the same moment could + both succeed and leave nobody able to open Server. The count is now taken + in the same transaction as the write; first-user setup is one statement + (audit 2026-10-12, M7). +- **A user is saved whole or not at all.** The users form wrote the name, + the password, the role and the assignments one after another; a failure + part way left a user renamed with the old role, or a domain administrator + with nothing assigned. Saving and creating a user are now one transaction + each, the e-mail included (audit 2026-10-12, M9). + ### Added - **The delivery log measures its own guesses** (migration `0007`). A table diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index 599140e..167f165 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -663,10 +663,10 @@ has already run is never edited — the next file is added; the published installation; look). Tests: `Ops`/`ops` refused; the map written lower- case. Then verify Postfix's compare on the stand once (`MAIL FROM` as the other case) so the doc's claim is seen, not assumed. — **Opus** -- [ ] **Last global in one transaction**: `DeleteUser`/`SetUserRole` take a +- [x] **Last global in one transaction**: `DeleteUser`/`SetUserRole` take a `tx`, count and act inside it; `CreateGlobalUser` likewise. A concurrency test with two goroutines. — **Opus** -- [ ] **`submitUserUpdate` in one transaction** (store method that takes the +- [x] **`submitUserUpdate` in one transaction** (store method that takes the whole change). The users pages get direct handler tests while they are rewired - a row of review-2026-08-followups, placed here 2026-10-14. — **Opus** - [ ] **Import validates like the form**: route `DMARCRua`, IPs, limit mode, diff --git a/docs/roadmap.md b/docs/roadmap.md index f104946..c6901ce 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 15/62 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, seven stages | in progress (stage 0 shipped `2.4.2`; next stages 1+2 → `2.5.0`) | 17/62 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and diff --git a/internal/store/users.go b/internal/store/users.go index bc5b0a4..94d0b7d 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -76,23 +76,22 @@ func (s *Store) UserExists() (bool, error) { return n > 0, nil } -// CreateGlobalUser inserts the first global user during setup. +// CreateGlobalUser inserts the first global user during setup. The check that +// no user exists yet and the insert are one statement, so two setup requests +// at once cannot both create one (audit 2026-10-12, M7). func (s *Store) CreateGlobalUser(username, passwordHash string) error { - exists, err := s.UserExists() - if err != nil { - return err - } - if exists { - return fmt.Errorf("create global user: users already exist") - } now := time.Now().UTC().Format(time.RFC3339) - _, err = s.db.Exec( - "INSERT INTO users (username, password_hash, role, created_at) VALUES (?, ?, ?, ?)", + res, err := s.db.Exec( + `INSERT INTO users (username, password_hash, role, created_at) + SELECT ?, ?, ?, ? WHERE NOT EXISTS (SELECT 1 FROM users)`, username, passwordHash, RoleGlobal, now, ) if err != nil { return fmt.Errorf("create global user: %w", err) } + if n, _ := res.RowsAffected(); n == 0 { + return fmt.Errorf("create global user: users already exist") + } return nil } @@ -220,17 +219,37 @@ func (s *Store) CountGlobalUsers() (int, error) { return n, nil } -// CreateUser inserts a panel user. A domain user is created with its reach in -// the same call and must be given one (ErrEmptyReach); for a global user the -// reach is ignored. +// CreateUser inserts a panel user without an account e-mail; see AddUser. func (s *Store) CreateUser(username, passwordHash string, role Role, reach Reach) (int64, error) { - if role == RoleDomain && reach.Empty() { + return s.AddUser(UserChange{Username: username, PasswordHash: passwordHash, Role: role, Reach: reach}) +} + +// UserChange is everything the users form sets on a user, written at once by +// AddUser and SaveUser. ID is ignored by AddUser. +type UserChange struct { + ID int64 + Username, PasswordHash, Email string + Role Role + Reach Reach // a domain user's; ignored for a global one +} + +// AddUser inserts a panel user with its e-mail and, for a domain user, its +// reach, in one transaction: a domain user must be given one (ErrEmptyReach), +// and a failure part way leaves no user rather than one without it (audit +// 2026-10-12, M9). +func (s *Store) AddUser(c UserChange) (int64, error) { + if c.Role == RoleDomain && c.Reach.Empty() { return 0, ErrEmptyReach } + tx, err := s.db.Begin() + if err != nil { + return 0, fmt.Errorf("create user: %w", err) + } + defer tx.Rollback() now := time.Now().UTC().Format(time.RFC3339) - res, err := s.db.Exec( - "INSERT INTO users (username, password_hash, role, created_at) VALUES (?, ?, ?, ?)", - username, passwordHash, role, now, + res, err := tx.Exec( + "INSERT INTO users (username, password_hash, role, email, created_at) VALUES (?, ?, ?, ?, ?)", + c.Username, c.PasswordHash, c.Role, c.Email, now, ) if err != nil { if isUniqueViolation(err) { @@ -242,14 +261,79 @@ func (s *Store) CreateUser(username, passwordHash string, role Role, reach Reach if err != nil { return 0, fmt.Errorf("create user id: %w", err) } - if role == RoleDomain { - if err := s.setUserReach(id, reach); err != nil { + if c.Role == RoleDomain { + if err := writeUserReach(tx, id, c.Reach); err != nil { return 0, err } } + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("create user: %w", err) + } return id, nil } +// SaveUser writes what the users form changed - username, password hash, +// e-mail, role and reach - in one transaction (audit 2026-10-12, M9: five +// writes in a row left, on a failure part way, a user renamed with the old +// role, or a domain user with nothing assigned). A domain user must keep a +// reach (ErrEmptyReach); a user made global loses theirs, so a later demotion +// does not bring an old assignment back. The last global administrator cannot +// be demoted (ErrLastGlobal): the count is taken inside the transaction, +// after the write, so two administrators demoting each other at once cannot +// both succeed (M7). +func (s *Store) SaveUser(c UserChange) error { + if c.Role == RoleDomain && c.Reach.Empty() { + return ErrEmptyReach + } + tx, err := s.db.Begin() + if err != nil { + return fmt.Errorf("save user: %w", err) + } + defer tx.Rollback() + // A write first: it takes the write lock, so what is counted below is + // what is committed. + res, err := tx.Exec( + "UPDATE users SET username = ?, password_hash = ?, email = ?, role = ? WHERE id = ?", + c.Username, c.PasswordHash, c.Email, c.Role, c.ID, + ) + if err != nil { + if isUniqueViolation(err) { + return ErrUserExists + } + return fmt.Errorf("save user: %w", err) + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrUserNotFound + } + if err := requireAGlobal(tx); err != nil { + return err + } + reach := c.Reach + if c.Role == RoleGlobal { + reach = Reach{} + } + if err := writeUserReach(tx, c.ID, reach); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("save user: %w", err) + } + return nil +} + +// requireAGlobal is ErrLastGlobal when the transaction has left no global +// user. +func requireAGlobal(tx *sql.Tx) error { + var n int + if err := tx.QueryRow("SELECT COUNT(*) FROM users WHERE role = ?", RoleGlobal).Scan(&n); err != nil { + return fmt.Errorf("count global users: %w", err) + } + if n == 0 { + return ErrLastGlobal + } + return nil +} + // UpdateUser replaces a user's username, password hash and account e-mail. func (s *Store) UpdateUser(id int64, username, passwordHash, email string) error { res, err := s.db.Exec( @@ -272,22 +356,6 @@ func (s *Store) UpdateUser(id int64, username, passwordHash, email string) error return nil } -// SetUserRole updates a user's role. -func (s *Store) SetUserRole(userID int64, role Role) error { - res, err := s.db.Exec("UPDATE users SET role = ? WHERE id = ?", role, userID) - if err != nil { - return fmt.Errorf("set user role: %w", err) - } - n, err := res.RowsAffected() - if err != nil { - return fmt.Errorf("set user role: %w", err) - } - if n == 0 { - return ErrUserNotFound - } - return nil -} - // ClearUserReach removes everything a user is assigned — both lists and both // All flags. A user promoted to the global role keeps none of it, so a later // demotion does not quietly bring an old assignment back. @@ -311,32 +379,28 @@ func (s *Store) SetUserReach(userID int64, reach Reach) error { return s.setUserReach(userID, reach) } -// DeleteUser removes a panel user. ErrLastGlobal when deleting the only global user. +// DeleteUser removes a panel user. ErrLastGlobal when deleting the only global +// user: the delete and the count are one transaction, so two administrators +// deleting each other at once cannot both succeed (audit 2026-10-12, M7). func (s *Store) DeleteUser(id int64) error { - u, err := s.GetUser(id) + tx, err := s.db.Begin() if err != nil { + return fmt.Errorf("delete user: %w", err) + } + defer tx.Rollback() + res, err := tx.Exec("DELETE FROM users WHERE id = ?", id) + if err != nil { + return fmt.Errorf("delete user: %w", err) + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrUserNotFound + } + if err := requireAGlobal(tx); err != nil { return err } - if u.Role == RoleGlobal { - n, err := s.CountGlobalUsers() - if err != nil { - return err - } - if n <= 1 { - return ErrLastGlobal - } - } - res, err := s.db.Exec("DELETE FROM users WHERE id = ?", id) - if err != nil { + if err := tx.Commit(); err != nil { return fmt.Errorf("delete user: %w", err) } - n, err := res.RowsAffected() - if err != nil { - return fmt.Errorf("delete user: %w", err) - } - if n == 0 { - return ErrUserNotFound - } return nil } @@ -416,15 +480,23 @@ func (s *Store) listUserDomainNames(userID int64) ([]string, error) { } // setUserReach writes the flags and both assignment tables in one transaction. -// With an All flag set the rows of that direction are dropped rather than -// stored: the flag already covers them, and stale rows would come back to life -// the day the flag is cleared. func (s *Store) setUserReach(userID int64, reach Reach) error { tx, err := s.db.Begin() if err != nil { return fmt.Errorf("set user reach begin: %w", err) } defer tx.Rollback() + if err := writeUserReach(tx, userID, reach); err != nil { + return err + } + return tx.Commit() +} + +// writeUserReach writes a user's reach inside tx. With an All flag set the +// rows of that direction are dropped rather than stored: the flag already +// covers them, and stale rows would come back to life the day the flag is +// cleared. +func writeUserReach(tx *sql.Tx, userID int64, reach Reach) error { if _, err := tx.Exec("UPDATE users SET all_domains = ?, all_inbound_domains = ? WHERE id = ?", reach.AllDomains, reach.AllInbound, userID); err != nil { return fmt.Errorf("set user reach flags: %w", err) @@ -449,6 +521,5 @@ func (s *Store) setUserReach(userID int64, reach Reach) error { } } } - return tx.Commit() - + return nil } diff --git a/internal/store/users_tx_test.go b/internal/store/users_tx_test.go new file mode 100644 index 0000000..d4ae7d6 --- /dev/null +++ b/internal/store/users_tx_test.go @@ -0,0 +1,151 @@ +package store + +import ( + "errors" + "sync" + "testing" +) + +// Two global administrators who demote, or delete, each other at the same +// moment: one of them must stay global. The count is taken inside the +// transaction that writes, so the second sees the first's change (audit +// 2026-10-12, M7). +func TestTheLastGlobalSurvivesTwoAtOnce(t *testing.T) { + for _, how := range []string{"demote", "delete"} { + t.Run(how, func(t *testing.T) { + st := openTestStore(t) + d := addTestDomain(t, st, "example.com") + var ids []int64 + for _, name := range []string{"ann", "bob"} { + id, err := st.CreateUser(name, "hash", RoleGlobal, Reach{}) + if err != nil { + t.Fatal(err) + } + ids = append(ids, id) + } + var wg sync.WaitGroup + errs := make([]error, len(ids)) + for i, id := range ids { + wg.Add(1) + go func(i int, id int64) { + defer wg.Done() + if how == "delete" { + errs[i] = st.DeleteUser(id) + return + } + errs[i] = st.SaveUser(UserChange{ID: id, Username: []string{"ann", "bob"}[i], PasswordHash: "hash", + Role: RoleDomain, Reach: Reach{DomainIDs: []int64{d.ID}}}) + }(i, id) + } + wg.Wait() + refused := 0 + for _, err := range errs { + switch { + case errors.Is(err, ErrLastGlobal): + refused++ + case err != nil: + t.Fatalf("unexpected error: %v", err) + } + } + n, err := st.CountGlobalUsers() + if err != nil { + t.Fatal(err) + } + if refused != 1 || n != 1 { + t.Fatalf("%d refused, %d global users left; want 1 and 1", refused, n) + } + }) + } +} + +// SaveUser writes all of a change or none of it. +func TestSaveUserIsOneChange(t *testing.T) { + st := openTestStore(t) + d := addTestDomain(t, st, "example.com") + if _, err := st.CreateUser("root", "hash", RoleGlobal, Reach{}); err != nil { + t.Fatal(err) + } + id, err := st.CreateUser("shop", "hash", RoleDomain, Reach{DomainIDs: []int64{d.ID}}) + if err != nil { + t.Fatal(err) + } + if _, err := st.CreateUser("taken", "hash", RoleGlobal, Reach{}); err != nil { + t.Fatal(err) + } + + // A taken name: nothing else of the change is written either. + err = st.SaveUser(UserChange{ID: id, Username: "taken", PasswordHash: "new", Email: "x@example.com", Role: RoleGlobal}) + if !errors.Is(err, ErrUserExists) { + t.Fatalf("SaveUser with a taken name = %v", err) + } + // A reach naming a domain that does not exist fails at the last write. + err = st.SaveUser(UserChange{ID: id, Username: "renamed", PasswordHash: "new", Role: RoleDomain, Reach: Reach{DomainIDs: []int64{9999}}}) + if err == nil { + t.Fatal("SaveUser with a reach to no domain succeeded") + } + if err := st.SaveUser(UserChange{ID: id, Username: "renamed", PasswordHash: "new", Role: RoleDomain}); !errors.Is(err, ErrEmptyReach) { + t.Fatalf("SaveUser of a domain user with nothing assigned = %v", err) + } + u, err := st.GetUser(id) + if err != nil { + t.Fatal(err) + } + if u.Username != "shop" || u.PasswordHash != "hash" || u.Email != "" || u.Role != RoleDomain || len(u.DomainIDs) != 1 { + t.Fatalf("a refused change left %+v", u) + } + + // Made global, the user loses the reach. + if err := st.SaveUser(UserChange{ID: id, Username: "shop", PasswordHash: "hash", Email: "s@example.com", Role: RoleGlobal, Reach: Reach{DomainIDs: []int64{d.ID}}}); err != nil { + t.Fatal(err) + } + u, _ = st.GetUser(id) + if u.Role != RoleGlobal || u.Email != "s@example.com" || !u.Reach().Empty() { + t.Fatalf("promoted: %+v", u) + } +} + +// AddUser writes the user, the e-mail and the reach at once; a reach that +// cannot be written leaves no user behind. +func TestAddUserIsOneChange(t *testing.T) { + st := openTestStore(t) + d := addTestDomain(t, st, "example.com") + id, err := st.AddUser(UserChange{Username: "shop", PasswordHash: "hash", Email: "s@example.com", Role: RoleDomain, Reach: Reach{DomainIDs: []int64{d.ID}}}) + if err != nil { + t.Fatal(err) + } + u, _ := st.GetUser(id) + if u.Email != "s@example.com" || len(u.DomainIDs) != 1 { + t.Fatalf("created %+v", u) + } + if _, err := st.AddUser(UserChange{Username: "ghost", PasswordHash: "hash", Role: RoleDomain, Reach: Reach{DomainIDs: []int64{9999}}}); err == nil { + t.Fatal("AddUser with a reach to no domain succeeded") + } + if _, err := st.GetUserByUsername("ghost"); !errors.Is(err, ErrUserNotFound) { + t.Fatalf("the failed create left a user: %v", err) + } +} + +// Setup creates the first user once, however many requests race for it. +func TestCreateGlobalUserOnlyOnce(t *testing.T) { + st := openTestStore(t) + var wg sync.WaitGroup + errs := make([]error, 4) + for i := range errs { + wg.Add(1) + go func(i int) { + defer wg.Done() + errs[i] = st.CreateGlobalUser([]string{"a", "b", "c", "d"}[i], "hash") + }(i) + } + wg.Wait() + ok := 0 + for _, err := range errs { + if err == nil { + ok++ + } + } + n, _ := st.CountGlobalUsers() + if ok != 1 || n != 1 { + t.Fatalf("%d setups succeeded, %d global users; want 1 and 1", ok, n) + } +} diff --git a/internal/web/handlers/handlers_users.go b/internal/web/handlers/handlers_users.go index f00d62e..7de913d 100644 --- a/internal/web/handlers/handlers_users.go +++ b/internal/web/handlers/handlers_users.go @@ -210,8 +210,9 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) { h.renderUserForm(w, r, http.StatusInternalServerError, 0, userFormView{FormErr: "Internal error. Please try again."}) return } - id, err := h.store.CreateUser(username, string(hash), role, reach) - if err != nil { + // The user, the e-mail and the reach in one transaction: a failure leaves + // no user rather than half of one (audit 2026-10-12, M9). + if _, err := h.store.AddUser(store.UserChange{Username: username, PasswordHash: string(hash), Email: email, Role: role, Reach: reach}); err != nil { if errors.Is(err, store.ErrUserExists) { h.renderUserForm(w, r, http.StatusConflict, 0, userFormView{FormErr: "That username is already in use.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) return @@ -220,15 +221,6 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) { h.renderUserForm(w, r, http.StatusInternalServerError, 0, userFormView{FormErr: "Could not create user. Please check the logs."}) return } - // No domain follows a user who has just been created, so there is nothing - // to resync when their address is first stored. - if email != "" { - if err := h.store.UpdateUser(id, username, string(hash), email); err != nil { - logf("panel: create user %d: store e-mail: %v", id, err) - h.renderUserForm(w, r, http.StatusInternalServerError, id, userFormView{FormErr: "The user was created, but the e-mail could not be saved. Set it here.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) - return - } - } http.Redirect(w, r, "/server/users?done=created", http.StatusSeeOther) } @@ -274,14 +266,19 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st return } + lastGlobal := func() { + msg := "Cannot demote the last global administrator." + if u.ID == p.ID { + msg = "You cannot demote yourself without another global administrator." + } + h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: msg, FormUsername: username, FormEmail: email, FormRole: string(u.Role), Reach: reach}) + } + // Asked here to answer before a password is hashed; SaveUser decides, + // inside its transaction. if u.Role == store.RoleGlobal && role == store.RoleDomain { n, err := h.store.CountGlobalUsers() if err != nil || n <= 1 { - msg := "Cannot demote the last global administrator." - if u.ID == p.ID { - msg = "You cannot demote yourself without another global administrator." - } - h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: msg, FormUsername: username, FormEmail: email, FormRole: string(u.Role), Reach: reach}) + lastGlobal() return } } @@ -301,11 +298,17 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st hash = string(newHash) } - if err := h.store.UpdateUser(u.ID, username, hash, email); err != nil { - if errors.Is(err, store.ErrUserExists) { - h.renderUserForm(w, r, http.StatusConflict, u.ID, userFormView{FormErr: "That username is already in use.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) - return - } + // Name, password, e-mail, role and reach in one transaction (audit + // 2026-10-12, M9). Sessions end only once it has committed. + err := h.store.SaveUser(store.UserChange{ID: u.ID, Username: username, PasswordHash: hash, Email: email, Role: role, Reach: reach}) + switch { + case errors.Is(err, store.ErrUserExists): + h.renderUserForm(w, r, http.StatusConflict, u.ID, userFormView{FormErr: "That username is already in use.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) + return + case errors.Is(err, store.ErrLastGlobal): + lastGlobal() + return + case err != nil: logf("panel: update user: %v", err) h.renderUserForm(w, r, http.StatusInternalServerError, u.ID, userFormView{FormErr: "Could not save user. Please check the logs.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) return @@ -313,28 +316,6 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st if password != "" { h.endSessionsAfterPasswordSet(r, p, u) } - - if role != u.Role { - if err := h.store.SetUserRole(u.ID, role); err != nil { - logf("panel: set user role: %v", err) - h.renderUserForm(w, r, http.StatusInternalServerError, u.ID, userFormView{FormErr: "Could not update role.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) - return - } - if role == store.RoleGlobal { - if err := h.store.ClearUserReach(u.ID); err != nil { - logf("panel: clear user reach: %v", err) - } - } - } - - if role == store.RoleDomain { - if err := h.store.SetUserReach(u.ID, reach); err != nil { - logf("panel: set user reach: %v", err) - h.renderUserForm(w, r, http.StatusInternalServerError, u.ID, userFormView{FormErr: "Could not save domain assignments.", FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) - return - } - } - http.Redirect(w, r, "/server/users?done=updated", http.StatusSeeOther) } diff --git a/internal/web/handlers/handlers_users_test.go b/internal/web/handlers/handlers_users_test.go new file mode 100644 index 0000000..dc5f652 --- /dev/null +++ b/internal/web/handlers/handlers_users_test.go @@ -0,0 +1,99 @@ +package handlers + +import ( + "net/http" + "net/url" + "strings" + "testing" + + "github.com/mixeme/selfpost/internal/store" +) + +// The users form's writes, driven through the handlers (audit 2026-10-12, M7 +// and M9: the save is one store call now, and these pin what it does). + +// A save that changes name, e-mail and role lands whole; promoted to global +// the user keeps no assignment. +func TestUserFormSavesTheWholeChange(t *testing.T) { + h, st := inboundHandlers(t) + out1, _ := st.AddDomain("out.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}}) + rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, + url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"global"}}) + if rec.Code != http.StatusSeeOther { + t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String()) + } + after, err := st.GetUser(u.ID) + if err != nil { + t.Fatal(err) + } + if after.Username != "shop2" || after.Email != "shop@example.com" || after.Role != store.RoleGlobal || !after.Reach().Empty() { + t.Fatalf("after the save: %+v", after) + } + + // And back to a domain user, with an assignment. + rec = send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, + url.Values{"username": {"shop2"}, "email": {"shop@example.com"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) + if rec.Code != http.StatusSeeOther { + t.Fatalf("POST = %d:\n%s", rec.Code, rec.Body.String()) + } + after, _ = st.GetUser(u.ID) + if after.Role != store.RoleDomain || len(after.DomainIDs) != 1 || after.DomainIDs[0] != out1.ID { + t.Fatalf("demoted: %+v", after) + } +} + +// The last global administrator is not demoted, and a refused save changes +// nothing - not the name typed in the same form either. +func TestUserFormKeepsTheLastGlobal(t *testing.T) { + h, st := inboundHandlers(t) + out1, _ := st.AddDomain("out.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(root.ID), map[string]string{"uid": idStr(root.ID)}, + url.Values{"username": {"renamed"}, "role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "You cannot demote yourself without another global administrator.") { + t.Fatalf("demoting the last global = %d:\n%s", rec.Code, rec.Body.String()) + } + after, _ := st.GetUser(root.ID) + if after.Username != "root" || after.Role != store.RoleGlobal { + t.Fatalf("the refused save changed the user: %+v", after) + } +} + +// A domain user must keep something assigned; the refusal changes nothing. +func TestUserFormRefusesADomainUserWithNothingAssigned(t *testing.T) { + h, st := inboundHandlers(t) + out1, _ := st.AddDomain("out.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + u := userWith(t, st, "shop", store.RoleDomain, store.Reach{DomainIDs: []int64{out1.ID}}) + rec := send(h.HandleUserEdit, &root, "POST", "/server/users/"+idStr(u.ID), map[string]string{"uid": idStr(u.ID)}, + url.Values{"username": {"renamed"}, "role": {"domain"}}) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "Assign at least one outbound or inbound domain") { + t.Fatalf("nothing assigned = %d:\n%s", rec.Code, rec.Body.String()) + } + after, _ := st.GetUser(u.ID) + if after.Username != "shop" || len(after.DomainIDs) != 1 { + t.Fatalf("the refused save changed the user: %+v", after) + } +} + +// A user is created with the e-mail and the assignment in the one save. +func TestUserFormCreatesWithEmailAndReach(t *testing.T) { + h, st := inboundHandlers(t) + out1, _ := st.AddDomain("out.example.com", "mail") + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + rec := send(h.HandleUserNew, &root, "POST", "/server/users/new", nil, + url.Values{"username": {"fresh"}, "password": {"a-long-enough-password"}, "email": {"fresh@example.com"}, + "role": {"domain"}, "domain_ids": {idStr(out1.ID)}}) + if rec.Code != http.StatusSeeOther { + t.Fatalf("create = %d:\n%s", rec.Code, rec.Body.String()) + } + u, err := st.GetUserByUsername("fresh") + if err != nil { + t.Fatal(err) + } + if u.Email != "fresh@example.com" || u.Role != store.RoleDomain || len(u.DomainIDs) != 1 { + t.Fatalf("created %+v", u) + } +}