diff --git a/db/settings.go b/db/settings.go index a394c9b..f8fafcc 100644 --- a/db/settings.go +++ b/db/settings.go @@ -1,6 +1,9 @@ package db -import "database/sql" +import ( + "context" + "database/sql" +) // Settings is a tiny key-value store for global app config the UI toggles at // runtime (e.g. traffic_capture). Missing keys fall back to caller defaults. @@ -19,7 +22,12 @@ func (d *DB) GetSetting(key string) (value string, ok bool, err error) { // SetSetting upserts a setting value. func (d *DB) SetSetting(key, value string) error { - _, err := d.Exec(` + return d.SetSettingContext(context.Background(), key, value) +} + +// SetSettingContext upserts a setting and respects cancellation or a deadline. +func (d *DB) SetSettingContext(ctx context.Context, key, value string) error { + _, err := d.ExecContext(ctx, ` INSERT INTO settings(key, value) VALUES ($1, $2) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`, key, value) return err diff --git a/server/basic_auth.go b/server/basic_auth.go index 685f72d..e20be2b 100644 --- a/server/basic_auth.go +++ b/server/basic_auth.go @@ -1,6 +1,7 @@ package server import ( + "context" "crypto/hmac" "crypto/sha256" "encoding/json" @@ -19,6 +20,7 @@ import ( const basicAuthKey = "auth.http_basic" const basicAuthCookie = "artex_basic_auth" const basicAuthTTL = 12 * time.Hour +const basicAuthSaveTimeout = 10 * time.Second type basicAuthConfig struct { Enabled bool `json:"enabled"` @@ -29,18 +31,21 @@ type basicAuthConfig struct { type basicAuthSettingsStore interface { GetSetting(string) (string, bool, error) - SetSetting(string, string) error + SetSettingContext(context.Context, string, string) error } // Keep one atomic configuration in the DB and publish it only after a successful // save. Requests use the in-memory snapshot, avoiding a DB read for every asset. type basicAuthGate struct { - mu sync.RWMutex - cfg basicAuthConfig - store basicAuthSettingsStore + saveMu sync.Mutex // serializes persistence without blocking request snapshots + mu sync.RWMutex + cfg basicAuthConfig + store basicAuthSettingsStore } func (g *basicAuthGate) load(store basicAuthSettingsStore) error { + g.saveMu.Lock() + defer g.saveMu.Unlock() raw, ok, err := store.GetSetting(basicAuthKey) if err != nil { return err @@ -169,13 +174,15 @@ func (s *Server) saveBasicAuthSettings(w http.ResponseWriter, r *http.Request) { return } g := &s.basicAuth - g.mu.Lock() - defer g.mu.Unlock() - if g.store == nil { + g.saveMu.Lock() + defer g.saveMu.Unlock() + g.mu.RLock() + cfg, store := g.cfg, g.store + g.mu.RUnlock() + if store == nil { writeErr(w, 503, errDataSourceUnavailable) return } - cfg := g.cfg if req.Enabled != nil { cfg.Enabled = *req.Enabled } @@ -205,11 +212,18 @@ func (s *Server) saveBasicAuthSettings(w http.ResponseWriter, r *http.Request) { // Every save revokes previous gate cookies, including disable/re-enable. cfg.Revision = uuid.NewString() data, err := json.Marshal(cfg) - if err != nil || g.store.SetSetting(basicAuthKey, string(data)) != nil { + ctx, cancel := context.WithTimeout(r.Context(), basicAuthSaveTimeout) + defer cancel() + if err != nil || store.SetSettingContext(ctx, basicAuthKey, string(data)) != nil { writeErr(w, 503, "Basic Auth 配置保存失败，请重试") return } + // Hashing, persistence and response output never hold the request mutex. + // Publish only committed settings; requests keep using the previous snapshot + // until this short critical section completes. + g.mu.Lock() g.cfg = cfg + g.mu.Unlock() w.Header().Set("Cache-Control", "no-store") // Keep the administrator who just saved the settings in their session. if err := s.setBasicAuthCookie(w, r, cfg); err != nil { diff --git a/server/basic_auth_test.go b/server/basic_auth_test.go index 5f1443d..82cbd0c 100644 --- a/server/basic_auth_test.go +++ b/server/basic_auth_test.go @@ -1,11 +1,13 @@ package server import ( + "context" "encoding/json" "errors" "net/http" "net/http/httptest" "strings" + "sync" "testing" "time" @@ -21,7 +23,10 @@ type basicAuthMemoryStore struct { func (m *basicAuthMemoryStore) GetSetting(string) (string, bool, error) { return m.raw, m.raw != "", m.err } -func (m *basicAuthMemoryStore) SetSetting(_, value string) error { +func (m *basicAuthMemoryStore) SetSettingContext(ctx context.Context, _ string, value string) error { + if err := ctx.Err(); err != nil { + return err + } if m.err != nil { return m.err } @@ -45,6 +50,173 @@ func saveBasicAuthForTest(s *Server, body string) *httptest.ResponseRecorder { return w } +type blockedBasicAuthStore struct { + basicAuthSettingsStore + started chan basicAuthConfig + release chan struct{} + contexts chan context.Context +} + +func (b *blockedBasicAuthStore) SetSettingContext(ctx context.Context, key, value string) error { + var cfg basicAuthConfig + if err := json.Unmarshal([]byte(value), &cfg); err != nil { + return err + } + b.started <- cfg + if b.contexts != nil { + b.contexts <- ctx + } + select { + case <-b.release: + case <-ctx.Done(): + return ctx.Err() + } + return b.basicAuthSettingsStore.SetSettingContext(ctx, key, value) +} + +func TestBasicAuthBlockedSaveDoesNotBlockRequests(t *testing.T) { + for _, enabled := range []bool{false, true} { + name := "disabled" + if enabled { + name = "enabled" + } + t.Run(name, func(t *testing.T) { + s, store := basicAuthTestServer(t) + body := `{"enabled":false,"username":"entry","password":"original-pass"}` + if enabled { + body = `{"enabled":true,"username":"entry","password":"original-pass"}` + } + initial := saveBasicAuthForTest(s, body) + if initial.Code != http.StatusOK { + t.Fatal(initial.Body.String()) + } + before := s.basicAuth.snapshot() + blocked := &blockedBasicAuthStore{basicAuthSettingsStore: store, started: make(chan basicAuthConfig, 2), release: make(chan struct{})} + s.basicAuth.mu.Lock() + s.basicAuth.store = blocked + s.basicAuth.mu.Unlock() + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(blocked.release) }) } + t.Cleanup(release) + first := make(chan *httptest.ResponseRecorder, 1) + go func() { first <- saveBasicAuthForTest(s, `{"username":"new-entry"}`) }() + select { + case <-blocked.started: + case <-time.After(time.Second): + t.Fatal("save did not reach database") + } + // Exercise every kind of request sharing the gate while persistence waits. + reads := make(chan error, 1) + go func() { + if s.basicAuth.snapshot() != before { + reads <- errors.New("uncommitted settings became visible") + return + } + h := s.requireBasicAuth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) })) + for _, path := range []string{"/", "/_next/static/app.js", "/api/tasks", "/api/health"} { + r := httptest.NewRequest(http.MethodGet, path, nil) + if enabled && path != "/api/health" { + r.AddCookie(initial.Result().Cookies()[0]) + } + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusNoContent { + reads <- errors.New("request rejected while old configuration is still active") + return + } + } + w := httptest.NewRecorder() + s.Handler().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/health", nil)) + if w.Code != http.StatusOK { + reads <- errors.New("health handler unavailable during save") + return + } + reads <- nil + }() + select { + case err := <-reads: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("database save blocked configuration reads and HTTP requests") + } + // A concurrent partial update must wait and preserve the first update. + second := make(chan *httptest.ResponseRecorder, 1) + go func() { second <- saveBasicAuthForTest(s, `{"enabled":true}`) }() + select { + case <-blocked.started: + t.Fatal("concurrent saves reached persistence together") + case <-time.After(50 * time.Millisecond): + } + release() + for _, done := range []chan *httptest.ResponseRecorder{first, second} { + select { + case response := <-done: + if response.Code != http.StatusOK { + t.Fatal(response.Body.String()) + } + case <-time.After(time.Second): + t.Fatal("save did not finish after database release") + } + } + cfg := s.basicAuth.snapshot() + if cfg.Username != "new-entry" || !cfg.Enabled || cfg.PasswordHash != before.PasswordHash { + t.Fatal("concurrent partial save lost a committed update") + } + }) + } +} + +func TestBasicAuthBlockedSaveRespectsRequestCancellation(t *testing.T) { + s, store := basicAuthTestServer(t) + before := s.basicAuth.snapshot() + blocked := &blockedBasicAuthStore{basicAuthSettingsStore: store, started: make(chan basicAuthConfig, 1), release: make(chan struct{}), contexts: make(chan context.Context, 1)} + s.basicAuth.mu.Lock() + s.basicAuth.store = blocked + s.basicAuth.mu.Unlock() + t.Cleanup(func() { close(blocked.release) }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + r := httptest.NewRequest(http.MethodPut, "/api/settings/basic-auth", strings.NewReader(`{"username":"new-entry"}`)).WithContext(ctx) + done := make(chan *httptest.ResponseRecorder, 1) + go func() { + w := httptest.NewRecorder() + s.saveBasicAuthSettings(w, r) + done <- w + }() + select { + case <-blocked.started: + case <-time.After(time.Second): + t.Fatal("save did not reach database") + } + writeContext := <-blocked.contexts + deadline, bounded := writeContext.Deadline() + if !bounded || time.Until(deadline) > 10*time.Second { + t.Fatal("database write has no bounded deadline") + } + cancel() + select { + case w := <-done: + if w.Code != http.StatusServiceUnavailable || len(w.Result().Cookies()) != 0 { + t.Fatalf("canceled save must fail without issuing a cookie: %d %s", w.Code, w.Body.String()) + } + case <-time.After(time.Second): + t.Fatal("database save ignored request cancellation") + } + if s.basicAuth.snapshot() != before || store.raw != "" { + t.Fatal("canceled save changed the committed settings") + } + // A canceled save must also release the save mutex for a later update. + s.basicAuth.mu.Lock() + s.basicAuth.store = store + s.basicAuth.mu.Unlock() + w := saveBasicAuthForTest(s, `{"username":"later-entry"}`) + if w.Code != http.StatusOK { + t.Fatalf("later save failed: %s", w.Body.String()) + } +} + func TestBasicAuthGate(t *testing.T) { s, _ := basicAuthTestServer(t) h := s.requireBasicAuth(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(204) }))