diff options
Diffstat (limited to 'internal/admin')
| -rw-r--r-- | internal/admin/api_rooms.go | 3 | ||||
| -rw-r--r-- | internal/admin/live_owner_test.go | 227 | ||||
| -rw-r--r-- | internal/admin/owner.go | 82 | ||||
| -rw-r--r-- | internal/admin/owner_routes_test.go | 68 | ||||
| -rw-r--r-- | internal/admin/owner_test.go | 152 | ||||
| -rw-r--r-- | internal/admin/server.go | 62 | ||||
| -rw-r--r-- | internal/admin/undo.go | 4 |
7 files changed, 583 insertions, 15 deletions
diff --git a/internal/admin/api_rooms.go b/internal/admin/api_rooms.go index c93e7c6..3c96d53 100644 --- a/internal/admin/api_rooms.go +++ b/internal/admin/api_rooms.go @@ -850,7 +850,8 @@ func (s *AdminServer) handleRoomMove(w http.ResponseWriter, r *http.Request) { // 2. The file changes apply as a single journal transaction, rolling back // on any write failure. // 3. The wired runtime migration (players, Hub buckets, casino room keys, -// world room-keyed state, mob instances) runs through one owner task. +// world room-keyed state, mob instances) runs in the SAME owner task +// as preflight and the file commit (the API middleware owns dispatch). // 4. The undo record captures every before/after image and the idMap, so // undo/redo restore the whole plan, not just the renamed file. func (s *AdminServer) handleRoomRename(w http.ResponseWriter, r *http.Request) { diff --git a/internal/admin/live_owner_test.go b/internal/admin/live_owner_test.go new file mode 100644 index 0000000..7e94a1e --- /dev/null +++ b/internal/admin/live_owner_test.go @@ -0,0 +1,227 @@ +package admin + +import ( + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "thehouseoficarus/internal/casino" + "thehouseoficarus/internal/config" + "thehouseoficarus/internal/game" + gamenet "thehouseoficarus/internal/net" + "thehouseoficarus/internal/player" +) + +// Use the real authenticated HTTP router, not direct handler calls: production +// must commit files, migrate runtime state AND rebuild caches in one owner task. +func TestAdminEditorAtomicWithGameplay(t *testing.T) { + h, sess := newEditorHarness(t) + h.g.RunOnOwner(func() { + sess.Player.RoomID = 2 + h.hub.EnterRoom(sess, 2) + h.g.AccountStore.SaveCharacter(sess.Player) + h.g.Casino.JoinMachine(2, casino.MachineConfig{ID: "slots", Game: casino.GameSlots, MinBet: 1}, "alice") + }) + + // Pause after the handler has committed files but before runtime migration. + // A tick/input/observer queued here must not see the torn intermediate state. + midMutation := make(chan struct{}, 1) + releaseMigration := make(chan struct{}) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(releaseMigration) }) } + defer release() + var pause atomic.Bool + migrate := func(ids map[int]int) { + if pause.Load() { + midMutation <- struct{}{} + <-releaseMigration + } + h.g.MigrateRoomIDsOnOwner(ids) + } + s, err := NewServer(config.Default(), false, h.g.AccountStore, h.g.World, h.g.ItemStore, h.g.ObjectStore, h.g.MobStore, h.dir, + h.g.CourseStore.ReloadAll, migrate, h.g.EvictCasinoRoom, h.g.RebuildCraftIndex, filepath.Join(h.dir, "config.yaml"), h.g.RunAdminOnOwner) + if err != nil { + t.Fatal(err) + } + form := url.Values{"account": {"alice"}, "password": {"pw"}} + login := httptest.NewRequest("POST", "/login", strings.NewReader(form.Encode())) + login.Header.Set("Content-Type", "application/x-www-form-urlencoded") + loginRec := httptest.NewRecorder() + s.ServeHTTP(loginRec, login) + cookies := loginRec.Result().Cookies() + if loginRec.Code != http.StatusFound || len(cookies) == 0 { + t.Fatalf("login: %d %s", loginRec.Code, loginRec.Body.String()) + } + request := func(path, body string) error { + r := httptest.NewRequest("POST", path, strings.NewReader(body)) + r.AddCookie(cookies[0]) + rec := httptest.NewRecorder() + s.ServeHTTP(rec, r) + if rec.Code != http.StatusOK || strings.Contains(rec.Body.String(), `"error"`) { + return fmt.Errorf("%s: %d %s", path, rec.Code, rec.Body.String()) + } + return nil + } + + var observations atomic.Int64 + // Called ONLY on the owner. Both tick and session observers assert the + // disk/index/player/Hub/casino snapshot agrees, not merely that it is race-free. + check := func() { + observations.Add(1) + id := sess.Player.RoomID + path, ok := h.g.World.GetRoomPath(id) + if !ok { + t.Errorf("player in unindexed room %d", id) + return + } + if _, err := os.Stat(path); err != nil { + t.Errorf("player room %d missing on disk: %v", id, err) + } + if room, err := h.g.World.LoadRoom(id); err != nil || room.ID != id { + t.Errorf("room file/index disagrees with player %d: %v", id, err) + } + if players := h.hub.PlayersInRoom(id); len(players) != 1 || players[0] != sess { + t.Errorf("Hub disagrees with player room %d", id) + } + if machine, ok := h.g.Casino.MachineFor("alice"); !ok || machine.RoomID != id { + t.Errorf("casino disagrees with player room %d", id) + } + if saved, err := h.g.AccountStore.LoadCharacter("alice"); err != nil || saved.RoomID != id { + t.Errorf("saved character disagrees with player room %d: %v", id, err) + } + } + tickID := h.g.SubscribeTicks(1, func() bool { + h.g.RunTickSystems() + check() + return true + }) + defer h.g.Ticks.Unsubscribe(tickID) + + pause.Store(true) + requestDone := make(chan error, 1) + go func() { requestDone <- request("/api/rooms/rename", `{"id":2,"new_id":4}`) }() + select { + case <-midMutation: + case <-time.After(5 * time.Second): + t.Fatal("rename did not reach migration (nested owner dispatch?)") + } + observed := make(chan struct{}) + go func() { + h.g.RunOnOwner(func() { + h.g.HandleSession(sess, "look") + check() + close(observed) + }) + }() + select { + case <-observed: + t.Error("gameplay ran between file commit and runtime migration") + case <-time.After(20 * time.Millisecond): + } + pause.Store(false) + release() + select { + case err := <-requestDone: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("rename deadlocked") + } + <-observed + + // Repeat real rename/undo/redo while input and ticks compete for ownership. + inputDone := make(chan struct{}) + go func() { + defer close(inputDone) + for i := 0; i < 100; i++ { + h.g.HandleSessionOnOwner(sess, "look") + h.g.RunOnOwner(check) + } + }() + for i := 0; i < 5; i++ { + for _, path := range []string{"/api/undo/undo", "/api/undo/redo"} { + if err := request(path, ""); err != nil { + t.Error(err) + } + } + } + <-inputDone + h.g.RunOnOwner(check) + if observations.Load() < 100 { + t.Fatalf("too few gameplay observations: %d", observations.Load()) + } +} + +type editorHarness struct { + g *game.Game + hub *gamenet.Hub + dir string +} + +type discardConn struct{} + +func (discardConn) ReadMessage() (string, error) { return "", io.EOF } +func (discardConn) Write(p []byte) (int, error) { return len(p), nil } +func (discardConn) Close() error { return nil } +func (discardConn) SetWriteDeadline(time.Time) error { return nil } +func (discardConn) SetEcho(bool) error { return nil } + +func newEditorHarness(t *testing.T) (*editorHarness, *gamenet.Session) { + t.Helper() + dir := t.TempDir() + for _, subdir := range []string{"rooms", "players/accounts", "players/characters"} { + if err := os.MkdirAll(filepath.Join(dir, subdir), 0755); err != nil { + t.Fatal(err) + } + } + for id, contents := range map[int]string{ + 1: "id: 1\nname: Central\nexits:\n north:\n room: 2\n", + 2: "id: 2\nname: North\nexits:\n south:\n room: 1\n", + } { + if err := os.WriteFile(filepath.Join(dir, "rooms", fmt.Sprintf("%d.yaml", id)), []byte(contents), 0644); err != nil { + t.Fatal(err) + } + } + colors := &config.ColorsConfig{} + g := game.New(dir, colors, colors, config.ValidationConfig{}, 1) + hub := gamenet.NewHub() + g.SetHub(hub) + stopped := make(chan struct{}) + g.SetTransportStop(func() { close(stopped) }) + t.Cleanup(func() { + g.RequestShutdown("test complete") + select { + case <-stopped: + case <-time.After(5 * time.Second): + t.Error("game shutdown timed out") + } + }) + hash, err := player.HashPassword("pw") + if err != nil { + t.Fatal(err) + } + acc := &player.Account{Name: "alice", PasswordHash: hash, Admin: true, Characters: []string{"alice"}} + p := player.New("alice") + p.Account, p.RoomID = "alice", 1 + if err := g.AccountStore.SaveAccount(acc); err != nil { + t.Fatal(err) + } + if err := g.AccountStore.SaveCharacter(p); err != nil { + t.Fatal(err) + } + sess := &gamenet.Session{Conn: discardConn{}, Account: acc, Player: p, State: gamenet.StateGame} + hub.Add(sess) + hub.EnterRoom(sess, 1) + g.Ticks.Start(2) + return &editorHarness{g: g, hub: hub, dir: dir}, sess +} diff --git a/internal/admin/owner.go b/internal/admin/owner.go new file mode 100644 index 0000000..02288f7 --- /dev/null +++ b/internal/admin/owner.go @@ -0,0 +1,82 @@ +package admin + +import ( + "bytes" + "errors" + "io" + "net/http" +) + +const maxAdminRequestBytes = 8 << 20 + +// ownerMiddleware is the single execution boundary for ALL editor API routes, +// including reads (which must not observe partial edits) and undo/redo. Planning, +// file writes, runtime callbacks, reloads and history updates stay in one task. +// Register new API routes behind this boundary; handlers must never redispatch. +// Network I/O happens outside the task so slow browsers cannot block gameplay. +func (s *AdminServer) ownerMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxAdminRequestBytes)) + r.Body.Close() + if err != nil { + status := http.StatusBadRequest + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + status = http.StatusRequestEntityTooLarge + } + writeJSONError(w, "cannot read request body: "+err.Error(), status) + return + } + if r.Context().Err() != nil { + return + } + r = r.Clone(r.Context()) + r.Body = io.NopCloser(bytes.NewReader(body)) + response := &bufferedResponse{header: make(http.Header)} + var panicValue any + if !s.runOnOwner(func() { + // Preserve net/http's panic recovery on the HTTP goroutine; + // a handler panic must not kill the game's owner loop. + defer func() { panicValue = recover() }() + if r.Context().Err() == nil { + next.ServeHTTP(response, r) + } + }) { + writeJSONError(w, "game is shutting down", http.StatusServiceUnavailable) + return + } + if panicValue != nil { + panic(panicValue) + } + for key, values := range response.header { + w.Header()[key] = values + } + if response.status != 0 { + w.WriteHeader(response.status) + } + _, _ = w.Write(response.body.Bytes()) + }) +} + +// bufferedResponse deliberately provides no streaming/hijacking interfaces: +// editor API handlers produce finite responses, never network streams. +type bufferedResponse struct { + header http.Header + status int + body bytes.Buffer +} + +func (w *bufferedResponse) Header() http.Header { return w.header } + +func (w *bufferedResponse) WriteHeader(status int) { + if w.status == 0 { + w.status = status + } +} + +func (w *bufferedResponse) Write(p []byte) (int, error) { + if w.status == 0 { + w.WriteHeader(http.StatusOK) + } + return w.body.Write(p) +} diff --git a/internal/admin/owner_routes_test.go b/internal/admin/owner_routes_test.go new file mode 100644 index 0000000..a061b0e --- /dev/null +++ b/internal/admin/owner_routes_test.go @@ -0,0 +1,68 @@ +package admin + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "thehouseoficarus/internal/config" +) + +// The production router must not leave file-only mutations or undo/redo on +// the HTTP goroutine. A rejecting dispatcher prevents ALL API handlers from +// touching files, including future routes and read endpoints. +func TestAPIRoutesRequireOwnerAdmission(t *testing.T) { + h, _ := newEditorHarness(t) + tasks := 0 + s, err := NewServer(config.Default(), false, h.g.AccountStore, h.g.World, h.g.ItemStore, h.g.ObjectStore, h.g.MobStore, h.dir, + h.g.CourseStore.ReloadAll, h.g.MigrateRoomIDsOnOwner, h.g.EvictCasinoRoom, h.g.RebuildCraftIndex, filepath.Join(h.dir, "config.yaml"), + func(func()) bool { tasks++; return false }) + if err != nil { + t.Fatal(err) + } + cookieRec := httptest.NewRecorder() + s.setAuthCookie(cookieRec, "alice") + cookie := cookieRec.Result().Cookies()[0] + + routes := []string{ + "GET /api/map", "GET /api/dashboard", "GET /api/undo/state", + "POST /api/rooms", "PUT /api/rooms/2", "DELETE /api/rooms/2", + "POST /api/rooms/new-empty", "POST /api/rooms/link", "DELETE /api/rooms/link", + "POST /api/rooms/insert", "POST /api/rooms/remove", "POST /api/rooms/move", + "POST /api/rooms/rename", "POST /api/rooms/swap", "POST /api/rooms/bulk-edit", + "POST /api/rooms/bulk-delete", "POST /api/rooms/resolve-duplicate", + "POST /api/room-dirs/create", "POST /api/room-dirs/rename", + "POST /api/courses", "PUT /api/courses/test", "DELETE /api/courses/test", + "POST /api/triggers", "PUT /api/triggers/test", "DELETE /api/triggers/test", + "PUT /api/files?path=test.yaml", "POST /api/files?path=test.yaml", + "DELETE /api/files?path=test.yaml", "POST /api/files/rename", + "POST /api/colors", "POST /api/colors/reset", + "POST /api/undo/undo", "POST /api/undo/redo", + } + for _, entity := range []string{"objects", "items", "mobs", "drops", "hazards", "techs", "modules"} { + for _, route := range []string{"POST /api/%s", "PUT /api/%s/test", "DELETE /api/%s/test", "POST /api/%s/rename", "POST /api/%s/move", "POST /api/%s/dirs"} { + routes = append(routes, strings.ReplaceAll(route, "%s", entity)) + } + } + for _, route := range routes { + t.Run(route, func(t *testing.T) { + method, path, _ := strings.Cut(route, " ") + r := httptest.NewRequest(method, path, strings.NewReader(`{}`)) + r.AddCookie(cookie) + rec := httptest.NewRecorder() + before := tasks + s.ServeHTTP(rec, r) + if tasks != before+1 || rec.Code != http.StatusServiceUnavailable { + t.Fatalf("dispatcher calls=%d status=%d", tasks-before, rec.Code) + } + }) + } + before := tasks + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequest("POST", "/api/rooms", nil)) + if tasks != before || rec.Code != http.StatusUnauthorized { + t.Fatal("unauthenticated request reached owner") + } +} diff --git a/internal/admin/owner_test.go b/internal/admin/owner_test.go new file mode 100644 index 0000000..35538bc --- /dev/null +++ b/internal/admin/owner_test.go @@ -0,0 +1,152 @@ +package admin + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +type checkedBody struct { + t *testing.T + onOwner *bool + io.Reader +} + +func (b checkedBody) Read(p []byte) (int, error) { + if *b.onOwner { + b.t.Error("network request read on owner") + } + return b.Reader.Read(p) +} + +func (b checkedBody) Close() error { return nil } + +type checkedWriter struct { + t *testing.T + onOwner *bool + *httptest.ResponseRecorder +} + +func (w checkedWriter) Write(p []byte) (int, error) { + if *w.onOwner { + w.t.Error("network response write on owner") + } + return w.ResponseRecorder.Write(p) +} + +func (w checkedWriter) WriteHeader(status int) { + if *w.onOwner { + w.t.Error("network response headers written on owner") + } + w.ResponseRecorder.WriteHeader(status) +} + +func TestOwnerMiddlewareBuffersNetworkIO(t *testing.T) { + for _, method := range []string{"GET", "POST", "PUT", "DELETE", "PATCH"} { + t.Run(method, func(t *testing.T) { + onOwner, tasks := false, 0 + s := &AdminServer{runOnOwner: func(fn func()) bool { + tasks++ + onOwner = true + fn() + onOwner = false + return true + }} + handler := s.ownerMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !onOwner { + t.Error("handler outside owner task") + } + body, err := io.ReadAll(r.Body) + if err != nil || string(body) != "request" { + t.Errorf("body = %q, %v", body, err) + } + w.Header().Set("X-Test", "buffered") + w.WriteHeader(http.StatusCreated) + w.WriteHeader(http.StatusBadRequest) // first status wins + _, _ = w.Write([]byte("response")) + })) + r := httptest.NewRequest(method, "/api/test", nil) + r.Body = checkedBody{t, &onOwner, strings.NewReader("request")} + rec := httptest.NewRecorder() + handler.ServeHTTP(checkedWriter{t, &onOwner, rec}, r) + if tasks != 1 || rec.Code != http.StatusCreated || rec.Body.String() != "response" || rec.Header().Get("X-Test") != "buffered" { + t.Fatalf("tasks=%d response=%+v", tasks, rec) + } + }) + } +} + +type failingBody struct{} + +func (failingBody) Read([]byte) (int, error) { return 0, errors.New("broken upload") } +func (failingBody) Close() error { return nil } + +func TestOwnerMiddlewareRejectsBeforeDispatch(t *testing.T) { + for _, tc := range []struct { + name string + body io.ReadCloser + status int + }{ + {"oversize", io.NopCloser(strings.NewReader(strings.Repeat("x", maxAdminRequestBytes+1))), http.StatusRequestEntityTooLarge}, + {"read error", failingBody{}, http.StatusBadRequest}, + } { + t.Run(tc.name, func(t *testing.T) { + s := &AdminServer{runOnOwner: func(func()) bool { + t.Error("invalid request dispatched") + return false + }} + r := httptest.NewRequest("POST", "/api/test", nil) + r.Body = tc.body + rec := httptest.NewRecorder() + s.ownerMiddleware(nil).ServeHTTP(rec, r) + if rec.Code != tc.status { + t.Fatalf("status=%d, want %d", rec.Code, tc.status) + } + }) + } +} + +func TestOwnerMiddlewareShutdownAndCancellation(t *testing.T) { + t.Run("shutdown", func(t *testing.T) { + s := &AdminServer{runOnOwner: func(func()) bool { return false }} + rec := httptest.NewRecorder() + s.ownerMiddleware(nil).ServeHTTP(rec, httptest.NewRequest("POST", "/api/test", nil)) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status=%d", rec.Code) + } + }) + t.Run("cancelled while queued", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + s := &AdminServer{runOnOwner: func(fn func()) bool { + cancel() + fn() + return true + }} + s.ownerMiddleware(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Error("cancelled request executed") + })).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/api/test", nil).WithContext(ctx)) + }) +} + +func TestOwnerMiddlewarePanicReturnsToHTTPGoroutine(t *testing.T) { + ownerReturned := false + s := &AdminServer{runOnOwner: func(fn func()) bool { + fn() + ownerReturned = true + return true + }} + defer func() { + if value := recover(); value != "handler panic" || !ownerReturned { + t.Errorf("panic=%v, owner returned=%v", value, ownerReturned) + } + }() + s.ownerMiddleware(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + panic("handler panic") + })).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/api/test", nil)) + t.Fatal("panic was swallowed") +} diff --git a/internal/admin/server.go b/internal/admin/server.go index 81f0cce..3dbe8ad 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -50,6 +50,10 @@ type AdminServer struct { tmpl *template.Template cookieSecret []byte reloadCourses func() + // runOnOwner synchronously executes an API task, or returns false when + // the game is shutting down. All runtime callbacks below run INSIDE + // that task and must not dispatch to the owner again. + runOnOwner func(func()) bool // rewriteRoomIDs migrates room-ID-embedded state across all characters // (online + offline) when a room ID changes. Wired from the Game so the // admin web GUI can trigger the same migration as the swapid command @@ -69,7 +73,10 @@ type AdminServer struct { rebuildCraftIndex func() } -func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string) (*AdminServer, error) { +func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStore, w *world.World, is *item.ItemStore, os *object.ObjectStore, ms *world.MobStore, dataDir string, reloadCourses func(), rewriteRoomIDs func(map[int]int), evictRoom func(roomID int), rebuildCraftIndex func(), configPath string, runOnOwner func(func()) bool) (*AdminServer, error) { + if runOnOwner == nil { + return nil, fmt.Errorf("admin server requires an owner dispatcher") + } secret := make([]byte, 32) if _, err := rand.Read(secret); err != nil { return nil, fmt.Errorf("cookie secret: %w", err) @@ -117,6 +124,7 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor evictRoom: evictRoom, rebuildCraftIndex: rebuildCraftIndex, configPath: configPath, + runOnOwner: runOnOwner, } // Undo/redo of room-ID changes applies the (inverse) runtime migration // through the same wired callback the rename uses. @@ -206,7 +214,7 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor apiMux.HandleFunc("/api/colors", s.handleColors) apiMux.HandleFunc("/api/colors/reset", s.handleColorsReset) - mux.Handle("/api/", s.authMiddleware(apiMux)) + mux.Handle("/api/", s.authMiddleware(s.ownerMiddleware(apiMux))) mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { if !s.checkAuth(r) { @@ -264,8 +272,12 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor }) s.httpServer = &http.Server{ - Addr: fmt.Sprintf(":%d", port), - Handler: mux, + Addr: fmt.Sprintf(":%d", port), + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 60 * time.Second, + IdleTimeout: 60 * time.Second, } if useTLS { s.httpServer.TLSConfig = &tls.Config{ @@ -277,6 +289,11 @@ func NewServer(cfg *config.Config, useTLS bool, accountStore *player.AccountStor return s, nil } +// ServeHTTP exposes the same authenticated routes used by ListenAndServe. +func (s *AdminServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + s.httpServer.Handler.ServeHTTP(w, r) +} + func (s *AdminServer) ListenAndServe() error { ln, err := net.Listen("tcp", s.httpServer.Addr) if err != nil { @@ -364,24 +381,41 @@ func (s *AdminServer) handleLogin(w http.ResponseWriter, r *http.Request) { return } - // Transparent upgrade: legacy single-pass SHA-256 hashes rehash to - // Argon2id after a successful verification. + if !acc.Admin { + s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."}) + return + } + + // Expensive password hashing stays off the owner. Reload the account + // inside the task so an upgrade cannot overwrite a concurrent character + // rename/create or a password/admin change made since authentication. + var upgradedHash string if player.NeedsRehash(acc.PasswordHash) { - if hash, hashErr := player.HashPassword(password); hashErr == nil { - acc.PasswordHash = hash - if err := s.accountStore.SaveAccount(acc); err != nil { + upgradedHash, _ = player.HashPassword(password) + } + authorized := false + if !s.runOnOwner(func() { + current, err := s.accountStore.LoadAccount(account) + if err != nil || !current.Admin || current.PasswordHash != acc.PasswordHash { + return + } + authorized = true + if upgradedHash != "" { + current.PasswordHash = upgradedHash + if err := s.accountStore.SaveAccount(current); err != nil { log.Printf("admin login: password rehash save for %s: %v", account, err) } } + s.undoStack.Clear() + }) { + writeJSONError(w, "game is shutting down", http.StatusServiceUnavailable) + return } - - if !acc.Admin { - s.renderPage(w, r, "login.html", map[string]any{"Error": "Account is not authorized as admin."}) + if !authorized { + http.Error(w, "account changed; please sign in again", http.StatusUnauthorized) return } - s.setAuthCookie(w, account) - s.undoStack.Clear() http.Redirect(w, r, "/", http.StatusFound) } diff --git a/internal/admin/undo.go b/internal/admin/undo.go index 0f9744c..98d8d9a 100644 --- a/internal/admin/undo.go +++ b/internal/admin/undo.go @@ -61,6 +61,10 @@ type UndoInfo struct { RedoSize int `json:"redo_size"` } +// UndoStack protects history with a mutex, not live game state. Production +// callers must execute Push/Undo/Redo/Clear within the editor's owner task. +// Undo/Redo and the subsequent rebuildAfterUndo are one task, not separate +// dispatches; migrateRoomIDs is an owner-only callback. type UndoStack struct { mu sync.Mutex dataDir string |
