diff --git a/cmd/soundtouch-service/admin_area_auth_gate_test.go b/cmd/soundtouch-service/admin_area_auth_gate_test.go new file mode 100644 index 0000000..372a303 --- /dev/null +++ b/cmd/soundtouch-service/admin_area_auth_gate_test.go @@ -0,0 +1,114 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/service/certmanager" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" + "github.com/gesellix/bose-soundtouch/pkg/service/handlers" + "github.com/gesellix/bose-soundtouch/pkg/service/setup" +) + +// TestAdminAreaAuthGate is the wiring-level regression test for #419: it +// exercises the real production router (setupRouter), not just the +// BasicAuthAdmin middleware in isolation, to pin two things at once: +// 1. /admin and /api/setup/* (and their /setup/* legacy aliases) are open +// by default and become gated once AdminAreaAuth is "enabled". +// 2. The three routes shared with soundtouch-cli/soundtouch-player +// (ca.crt, tts/speak, tts/config) stay reachable WITHOUT credentials +// regardless of the gate — the whole reason mountSetupAPI was split +// into mountSetupAPIShared/mountSetupAPIAdmin. +func TestAdminAreaAuthGate(t *testing.T) { + tempDir := t.TempDir() + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + // A real setup.Manager (with an actual CA) so /setup/ca.crt genuinely + // succeeds instead of failing on a nil dependency for an unrelated + // reason, which would make the "stays reachable" assertion meaningless. + cm := certmanager.NewCertificateManager(filepath.Join(tempDir, "certs")) + _ = cm.EnsureCA() + sm := setup.NewManager("http://localhost:8000", ds, cm) + + server := handlers.NewServer(ds, sm, "http://localhost:8000", true, false, false) + server.SetMgmtConfig("custom-admin", "custom-password") + + r := setupRouter(server, nil, nil) + ts := httptest.NewServer(r) + defer ts.Close() + + adminGatedPaths := []string{ + "/admin", + "/setup/settings", + "/api/setup/settings", + } + sharedUngatedPaths := []string{ + "/setup/ca.crt", + "/api/setup/ca.crt", + "/setup/tts/config", + "/api/setup/tts/config", + } + + t.Run("open by default (AdminAreaAuth unset)", func(t *testing.T) { + for _, path := range adminGatedPaths { + status := getStatus(t, ts.URL, path, "", "") + if status == http.StatusUnauthorized { + t.Errorf("%s: expected open access by default, got 401", path) + } + } + }) + + server.SetAdminAreaAuth("enabled") + defer server.SetAdminAreaAuth("") + + t.Run("gated paths reject without credentials once enabled", func(t *testing.T) { + for _, path := range adminGatedPaths { + status := getStatus(t, ts.URL, path, "", "") + if status != http.StatusUnauthorized { + t.Errorf("%s: expected 401 without credentials once enabled, got %d", path, status) + } + } + }) + + t.Run("gated paths accept correct credentials once enabled", func(t *testing.T) { + for _, path := range adminGatedPaths { + status := getStatus(t, ts.URL, path, "custom-admin", "custom-password") + if status == http.StatusUnauthorized { + t.Errorf("%s: expected access with correct credentials, got 401", path) + } + } + }) + + t.Run("shared cli/player routes stay reachable without credentials", func(t *testing.T) { + for _, path := range sharedUngatedPaths { + status := getStatus(t, ts.URL, path, "", "") + if status != http.StatusOK { + t.Errorf("%s: expected 200 without credentials even with the gate enabled, got %d", path, status) + } + } + }) +} + +func getStatus(t *testing.T, base, path, user, pass string) int { + t.Helper() + + req, err := http.NewRequest(http.MethodGet, base+path, nil) + if err != nil { + t.Fatalf("Failed to build request for %s: %v", path, err) + } + if user != "" || pass != "" { + req.SetBasicAuth(user, pass) + } + + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("Request to %s failed: %v", path, err) + } + defer res.Body.Close() + + return res.StatusCode +} diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index b018298..5f5e34f 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -1295,7 +1295,7 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w r.Use(server.RecordMiddleware) r.Get("/", server.HandleRoot) - r.Get("/admin", server.HandleAdmin) + r.With(server.BasicAuthAdmin()).Get("/admin", server.HandleAdmin) r.Get("/health", server.HandleHealth) r.Get("/favicon.ico", func(w http.ResponseWriter, r *http.Request) { // The favicon lives in the embedded web/img bundle, not under @@ -1582,7 +1582,23 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w // /api/setup (new canonical) from one shared registration. The Stockholm // setup-wizard static catch-all is a frontend concern and stays under /setup // only — /api/setup serves data only. - mountSetupAPI := func(r chi.Router) { + // + // Split in two: mountSetupAPIShared is reachable regardless of + // AdminAreaAuth — soundtouch-cli and the embedded player call these + // directly without Management API credentials (ca.crt for `setup + // install-ca`, tts/speak+tts/config for the Play URL / TTS integration + // surface). mountSetupAPIAdmin is everything else — genuinely admin-UI-only, + // gated by BasicAuthAdmin() once #419's admin-area toggle is enabled. + mountSetupAPIShared := func(r chi.Router) { + r.Get("/ca.crt", server.HandleGetCACert) + // TTS lives under /setup (LAN-trust, like the rest of the integration + // surface and Play URL), not /mgmt: the API key is already configured + // via /setup/settings, and -web/CLI reach this without mgmt credentials. + r.Post("/tts/speak", server.HandleTTSSpeak) + r.Get("/tts/config", server.HandleTTSConfig) + } + + mountSetupAPIAdmin := func(r chi.Router) { r.Get("/devices", server.HandleListDiscoveredDevices) r.Post("/devices", server.HandleAddManualDevice) r.Delete("/devices/{deviceId}", server.HandleRemoveDevice) @@ -1590,11 +1606,6 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w r.Get("/discovery-status", server.HandleGetDiscoveryStatus) r.Get("/settings", server.HandleGetSettings) r.Post("/settings", server.HandleUpdateSettings) - // TTS lives under /setup (LAN-trust, like the rest of the integration - // surface and Play URL), not /mgmt: the API key is already configured - // via /setup/settings, and -web/CLI reach this without mgmt credentials. - r.Post("/tts/speak", server.HandleTTSSpeak) - r.Get("/tts/config", server.HandleTTSConfig) r.Get("/info/{deviceId}", server.HandleGetDeviceInfo) r.Get("/summary/{deviceId}", server.HandleGetMigrationSummary) r.Post("/migrate/{deviceId}", server.HandleMigrateDevice) @@ -1616,7 +1627,6 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w r.Post("/test-connection/{deviceId}", server.HandleTestConnection) r.Post("/test-hosts/{deviceId}", server.HandleTestHostsRedirection) r.Post("/test-dns/{deviceId}", server.HandleTestDNSRedirection) - r.Get("/ca.crt", server.HandleGetCACert) r.Get("/logging-settings", server.HandleGetLoggingSettings) r.Post("/logging-settings", server.HandleUpdateLoggingSettings) r.Get("/version", server.HandleGetVersionInfo) @@ -1648,12 +1658,19 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w // Stockholm wizard catch-all below is frontend, not a deprecated API path. r.Group(func(r chi.Router) { r.Use(server.DeprecatedRouteMiddleware) - mountSetupAPI(r) + mountSetupAPIShared(r) + }) + r.Group(func(r chi.Router) { + r.Use(server.DeprecatedRouteMiddleware) + r.Use(server.BasicAuthAdmin()) + mountSetupAPIAdmin(r) }) // Serve Stockholm setup wizard pages for paths not matched by the // management API. The Stockholm frontend has a setup/ directory that must // be accessible at /setup/*. Frontend-only — not mirrored under /api/setup. + // Not gated by AdminAreaAuth: Stockholm is a separate, off-by-default + // (--stockholm-dir) legacy wizard, out of scope for #419. if stockholmHandler != nil { r.Get("/*", stockholmHandler.HandleStatic) r.Get("/", stockholmHandler.HandleStatic) @@ -1661,7 +1678,11 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler, w }) r.Route("/api/setup", func(r chi.Router) { - mountSetupAPI(r) + mountSetupAPIShared(r) + r.Group(func(r chi.Router) { + r.Use(server.BasicAuthAdmin()) + mountSetupAPIAdmin(r) + }) }) // Embedded web UI: control API under /api/control and the SPA under /app diff --git a/pkg/service/handlers/handlers_mgmt.go b/pkg/service/handlers/handlers_mgmt.go index 11430fe..71ecaf2 100644 --- a/pkg/service/handlers/handlers_mgmt.go +++ b/pkg/service/handlers/handlers_mgmt.go @@ -1,6 +1,7 @@ package handlers import ( + "crypto/subtle" "encoding/json" "errors" "fmt" @@ -29,6 +30,46 @@ func (s *Server) BasicAuthMgmt() func(http.Handler) http.Handler { return middleware.BasicAuth("Management API", map[string]string{username: password}) } +// BasicAuthAdmin returns a middleware gating the whole admin area (/admin, +// /setup, /api/setup — minus the small set of routes shared with +// soundtouch-cli/soundtouch-player) behind the same credentials as +// BasicAuthMgmt. Unlike BasicAuthMgmt, which captures username/password once +// at router-setup time (main.go builds the router once at startup), +// BasicAuthAdmin reads the live AdminAreaAuth mode and credentials on every +// request, so toggling the setting via the Settings UI (HandleUpdateSettings +// -> SetAdminAreaAuth) takes effect immediately — no restart. When the mode +// isn't "enabled", every request passes through unauthenticated, i.e. +// today's default behavior. See #419 and +// _/i419/design-admin-area-auth-gate.md. +func (s *Server) BasicAuthAdmin() func(http.Handler) http.Handler { + const realm = "Admin Area" + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.mu.RLock() + mode := s.adminAreaAuth + username := s.mgmtUsername + password := s.mgmtPassword + s.mu.RUnlock() + + if mode != "enabled" { + next.ServeHTTP(w, r) + return + } + + user, pass, ok := r.BasicAuth() + if !ok || user != username || subtle.ConstantTimeCompare([]byte(pass), []byte(password)) != 1 { + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Basic realm="%s"`, realm)) + w.WriteHeader(http.StatusUnauthorized) + + return + } + + next.ServeHTTP(w, r) + }) + } +} + // HandleMgmtListSpeakers returns discovered speakers for the given account. func (s *Server) HandleMgmtListSpeakers(w http.ResponseWriter, r *http.Request) { _ = chi.URLParam(r, "accountId") diff --git a/pkg/service/handlers/handlers_mgmt_test.go b/pkg/service/handlers/handlers_mgmt_test.go index 5158beb..b47b3a0 100644 --- a/pkg/service/handlers/handlers_mgmt_test.go +++ b/pkg/service/handlers/handlers_mgmt_test.go @@ -265,3 +265,113 @@ func TestBasicAuthMgmt(t *testing.T) { } }) } + +// TestBasicAuthAdmin covers the #419 admin-area gate: unlike BasicAuthMgmt +// (credentials captured once at router-setup time), BasicAuthAdmin must +// read the live AdminAreaAuth mode and credentials on every request, so a +// live toggle via the Settings UI takes effect without a restart. +func TestBasicAuthAdmin(t *testing.T) { + s := NewServer(nil, nil, "http://localhost", false, false, false) + s.SetMgmtConfig("admin", "secret123") + + handler := s.BasicAuthAdmin()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("OK")) + })) + + t.Run("Unset mode passes through unauthenticated (today's default)", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("expected status %d for unset mode, got %d", http.StatusOK, rr.Code) + } + }) + + t.Run("Disabled mode passes through unauthenticated", func(t *testing.T) { + s.SetAdminAreaAuth("disabled") + defer s.SetAdminAreaAuth("") + + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("expected status %d for disabled mode, got %d", http.StatusOK, rr.Code) + } + }) + + t.Run("Enabled mode requires valid credentials", func(t *testing.T) { + s.SetAdminAreaAuth("enabled") + defer s.SetAdminAreaAuth("") + + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + req.SetBasicAuth("admin", "secret123") + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("expected status %d with valid credentials, got %d", http.StatusOK, rr.Code) + } + }) + + t.Run("Enabled mode rejects missing credentials", func(t *testing.T) { + s.SetAdminAreaAuth("enabled") + defer s.SetAdminAreaAuth("") + + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("expected status %d with no credentials, got %d", http.StatusUnauthorized, rr.Code) + } + if rr.Header().Get("WWW-Authenticate") == "" { + t.Error("expected WWW-Authenticate header to be set") + } + }) + + t.Run("Enabled mode rejects wrong credentials", func(t *testing.T) { + s.SetAdminAreaAuth("enabled") + defer s.SetAdminAreaAuth("") + + req := httptest.NewRequest(http.MethodGet, "/admin", nil) + req.SetBasicAuth("admin", "wrongpass") + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("expected status %d with wrong credentials, got %d", http.StatusUnauthorized, rr.Code) + } + }) + + t.Run("Toggling mode live changes behavior without rebuilding the handler", func(t *testing.T) { + // The whole point of reading s.adminAreaAuth per-request rather than + // capturing it once: the same handler value must reflect a live change. + s.SetAdminAreaAuth("") + reqOpen := httptest.NewRequest(http.MethodGet, "/admin", nil) + rrOpen := httptest.NewRecorder() + handler.ServeHTTP(rrOpen, reqOpen) + + if rrOpen.Code != http.StatusOK { + t.Fatalf("expected open access before toggling, got %d", rrOpen.Code) + } + + s.SetAdminAreaAuth("enabled") + defer s.SetAdminAreaAuth("") + + reqGated := httptest.NewRequest(http.MethodGet, "/admin", nil) + rrGated := httptest.NewRecorder() + handler.ServeHTTP(rrGated, reqGated) + + if rrGated.Code != http.StatusUnauthorized { + t.Errorf("expected the SAME handler to enforce auth immediately after toggling, got %d", rrGated.Code) + } + }) +} diff --git a/pkg/service/handlers/handlers_setup.go b/pkg/service/handlers/handlers_setup.go index 9236707..81b3bb3 100644 --- a/pkg/service/handlers/handlers_setup.go +++ b/pkg/service/handlers/handlers_setup.go @@ -380,8 +380,7 @@ func (s *Server) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) { // Guard rail: refuse to enable the admin-area gate while the Management // API credentials are still the published default — that would let // anyone in with admin/change_me! anyway, just with extra friction. - if adminAreaAuth == "enabled" && - s.mgmtUsername == health.DefaultMgmtUsername && s.mgmtPassword == health.DefaultMgmtPassword { + if blocksAdminAreaAuthEnable(adminAreaAuth, s.mgmtUsername, s.mgmtPassword) { s.mu.Unlock() http.Error(w, "Cannot enable admin_area_auth while Management API credentials are still the "+ "published default (admin/change_me!). Set MGMT_USERNAME and MGMT_PASSWORD to your own "+ @@ -542,6 +541,16 @@ func NormalizeAdminAreaAuth(v string) (string, bool) { } } +// blocksAdminAreaAuthEnable reports whether enabling the admin-area gate +// must be refused because the Management API credentials are still the +// published default (admin/change_me!) — enabling it in that state would +// give a false sense of security. Extracted from HandleUpdateSettings to +// keep its cyclomatic complexity in check. +func blocksAdminAreaAuthEnable(mode, mgmtUsername, mgmtPassword string) bool { + return mode == "enabled" && + mgmtUsername == health.DefaultMgmtUsername && mgmtPassword == health.DefaultMgmtPassword +} + // normaliseTLSExtraHosts trims whitespace from each entry, drops empty // values, and deduplicates while preserving the first occurrence's // position. The settings endpoint applies this before persisting so the diff --git a/pkg/service/handlers/handlers_setup_test.go b/pkg/service/handlers/handlers_setup_test.go index 7b1d057..60d0273 100644 --- a/pkg/service/handlers/handlers_setup_test.go +++ b/pkg/service/handlers/handlers_setup_test.go @@ -277,10 +277,13 @@ func TestAdminAreaAuthInvalidValue(t *testing.T) { ts := httptest.NewServer(r) defer ts.Close() - body, _ := json.Marshal(map[string]string{ + body, err := json.Marshal(map[string]string{ "server_url": "http://127.0.0.1:8000", "admin_area_auth": "sometimes", }) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } res, err := http.Post(ts.URL+"/setup/settings", "application/json", bytes.NewBuffer(body)) if err != nil { @@ -322,10 +325,13 @@ func TestAdminAreaAuthGuardRailBlocksDefaultCreds(t *testing.T) { server.mgmtUsername = health.DefaultMgmtUsername server.mgmtPassword = health.DefaultMgmtPassword - body, _ := json.Marshal(map[string]string{ + body, err := json.Marshal(map[string]string{ "server_url": "http://127.0.0.1:8000", "admin_area_auth": "enabled", }) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } res, err := http.Post(ts.URL+"/setup/settings", "application/json", bytes.NewBuffer(body)) if err != nil { @@ -370,10 +376,13 @@ func TestAdminAreaAuthRoundTrip(t *testing.T) { server.mgmtUsername = "custom-admin" server.mgmtPassword = "custom-password" - enableBody, _ := json.Marshal(map[string]string{ + enableBody, err := json.Marshal(map[string]string{ "server_url": "http://127.0.0.1:8000", "admin_area_auth": "Enabled", // mixed case must normalise }) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } res, err := http.Post(ts.URL+"/setup/settings", "application/json", bytes.NewBuffer(enableBody)) if err != nil { @@ -411,10 +420,13 @@ func TestAdminAreaAuthRoundTrip(t *testing.T) { t.Errorf("GET /setup/settings: expected admin_area_auth \"enabled\", got %+v", got["admin_area_auth"]) } - disableBody, _ := json.Marshal(map[string]string{ + disableBody, err := json.Marshal(map[string]string{ "server_url": "http://127.0.0.1:8000", "admin_area_auth": "disabled", }) + if err != nil { + t.Fatalf("Failed to marshal request body: %v", err) + } res, err = http.Post(ts.URL+"/setup/settings", "application/json", bytes.NewBuffer(disableBody)) if err != nil {