diff --git a/CLAUDE.md b/CLAUDE.md index 7f9a7ad..02e89c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -251,8 +251,18 @@ future metadata site can attach enrichment without reshaping the schema. (work + matched recording + series rails, each carrying its own `web_url`). Config is `metadata.{enabled,base_url}` (env `AUDIOSILO_METADATA_ENABLED` / `AUDIOSILO_METADATA_BASE_URL`; `base_url` must be an absolute http(s) URL when - enabled) - one key disables ALL outbound calls. `meta.Service` (constructed in - `api.New` only when configured; `a.meta == nil` = off) owns the compose logic + enabled) - one key disables ALL outbound calls. **Runtime toggle**: `meta.Service` + is constructed in `api.New` whenever `base_url` is valid (`MetadataConfig.ValidBaseURL`), + regardless of `enabled`, and an atomic flag (`API.metaEnabled`, seeded from + `metadata.enabled`) gates it - so an admin can flip it on/off without a restart. + The handler and the `metadata` capability both gate on `metadataOn()` + (`a.meta != nil && metaEnabled`); `a.meta == nil` (empty/invalid `base_url`) is + permanently unavailable and can't be enabled. The admin console's **Overview > + Community metadata lookup** card and `GET`/`PATCH /admin/settings` + (`handlers_settings.go`, transport-only) read/flip the flag, persisting + `metadata.enabled` to `config.yaml` via `cfg.Save()` (serialized by + `API.settingsMu`); the PATCH refuses (400) an attempt to enable when the service + is unavailable. `meta.Service` owns the compose logic (lookup -> works/{id} -> pick the recording by `recording_id`, first as fallback -> up to 3 series rails) behind a bounded in-memory TTL cache (24h positive / 1h not-found / 2min transport-error, ~2048-entry cap) so a hot path @@ -397,8 +407,9 @@ future metadata site can attach enrichment without reshaping the schema. `upload`, `transcode`, `websocket`, `api_keys`, `metadata`); flip them on as phases land. `transcode` already reflects whether ffmpeg is configured; `api_keys` is true (user-minted personal access tokens are supported); -`metadata` reflects whether the Phase 1.5 metadata lookup is configured -(`metadata.enabled && metadata.base_url != ""`). +`metadata` reflects whether the Phase 1.5 metadata lookup is live +(`metadataOn()`: a valid `metadata.base_url` AND the runtime enabled flag, which +the admin can toggle at `PATCH /admin/settings`). ## API surface @@ -408,3 +419,6 @@ and the static UI (`/`, `/connect`, `/admin`, `/web/...`). Everything else needs session bearer token; `/admin/*` additionally requires the admin role. The metadata lookup is `GET /libraries/{id}/meta?path=` (authed, scope-checked like the other `?path=` content endpoints; 404 when metadata is disabled). +Runtime-toggleable settings are `GET`/`PATCH /admin/settings` (admin only): a +feature-keyed envelope (`{"metadata":{"enabled","base_url","available"}}`) whose +`PATCH {"metadata":{"enabled":bool}}` flips the metadata lookup and persists it. diff --git a/internal/api/api.go b/internal/api/api.go index df55eed..862c759 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -7,6 +7,8 @@ import ( "context" "log/slog" "net/http" + "sync" + "sync/atomic" "time" "github.com/kodestar/audiosilo-server/internal/auth" @@ -37,10 +39,18 @@ type API struct { scanner *library.Scanner ffmpeg string // path to ffmpeg for on-the-fly transcoding; "" disables it // meta resolves book asin/isbn against the community metadata API (Phase 1.5). - // nil when metadata lookup is disabled/unconfigured; the handler and the - // `metadata` capability flag both gate on it being non-nil. - meta *meta.Service - log *slog.Logger + // It is constructed whenever metadata.base_url is a valid absolute http(s) URL + // (regardless of metadata.enabled), so the runtime admin toggle can flip the + // feature on without a restart; nil only when base_url is empty/invalid, in + // which case the feature is unavailable and cannot be enabled. metaEnabled is + // the runtime on/off flag (seeded from metadata.enabled): the handler and the + // `metadata` capability flag gate on meta != nil AND metaEnabled (metadataOn). + meta *meta.Service + metaEnabled atomic.Bool + // settingsMu serializes runtime config mutations that also persist config.yaml + // (admin settings PATCH); config fields are otherwise set once at startup. + settingsMu sync.Mutex + log *slog.Logger // baseCtx is the server lifecycle context; background work detached from a // request (e.g. backgroundScan) derives from it so it's cancelled on shutdown @@ -79,14 +89,15 @@ func New(cfg *config.Config, authSvc *auth.Service, cat *catalog.Catalog, scanne if log == nil { log = slog.Default() } - // Construct the metadata lookup service from config; nil when disabled or - // unconfigured so the feature (endpoint + capability) is off. The config is - // validated upstream (base_url is an absolute http(s) URL when enabled). + // Construct the metadata lookup service whenever base_url is a valid absolute + // http(s) URL - independent of metadata.enabled - so the admin runtime toggle + // can turn it on without a restart. nil only when base_url is empty/invalid, in + // which case the feature is permanently unavailable until the config gains one. var metaSvc *meta.Service - if cfg.Metadata.Enabled && cfg.Metadata.BaseURL != "" { + if cfg.Metadata.ValidBaseURL() { metaSvc = meta.NewService(cfg.Metadata.BaseURL, nil) } - return &API{ + a := &API{ cfg: cfg, auth: authSvc, cat: cat, @@ -103,6 +114,10 @@ func New(cfg *config.Config, authSvc *auth.Service, cat *catalog.Catalog, scanne ipLimiter: newIPRateLimiter(20, 40), // ~20 req/s, burst 40, per IP transcodeSem: make(chan struct{}, maxConcurrentTranscodes), } + // Seed the runtime flag from config; the feature is on only when a service was + // built too (metadataOn), so an enabled flag with no base_url stays off. + a.metaEnabled.Store(cfg.Metadata.Enabled) + return a } // maxConcurrentTranscodes caps simultaneous ffmpeg transcodes across all clients. @@ -190,6 +205,8 @@ func (a *API) Handler() http.Handler { // Admin. mux.Handle("GET /api/v1/admin/stats", a.requireAdmin(http.HandlerFunc(a.handleStats))) + mux.Handle("GET /api/v1/admin/settings", a.requireAdmin(http.HandlerFunc(a.handleGetSettings))) + mux.Handle("PATCH /api/v1/admin/settings", a.requireAdmin(http.HandlerFunc(a.handleUpdateSettings))) mux.Handle("GET /api/v1/admin/users", a.requireAdmin(http.HandlerFunc(a.handleListUsers))) mux.Handle("POST /api/v1/admin/users", a.requireAdmin(http.HandlerFunc(a.handleCreateUser))) mux.Handle("GET /api/v1/admin/users/{id}", a.requireAdmin(http.HandlerFunc(a.handleGetUserDetail))) diff --git a/internal/api/handlers_auth.go b/internal/api/handlers_auth.go index 9306474..73f13b6 100644 --- a/internal/api/handlers_auth.go +++ b/internal/api/handlers_auth.go @@ -35,7 +35,7 @@ func (a *API) handleServerInfo(w http.ResponseWriter, r *http.Request) { "upload": false, // Phase B "websocket": false, // Phase C "api_keys": true, // user-minted personal access tokens (POST /auth/tokens) - "metadata": a.meta != nil, // community metadata lookup (GET /libraries/{id}/meta) + "metadata": a.metadataOn(), // community metadata lookup (GET /libraries/{id}/meta); runtime-toggleable }, "auth": map[string]any{ "methods": []string{"auth_code", "password"}, diff --git a/internal/api/handlers_meta.go b/internal/api/handlers_meta.go index 272d3b4..f48069d 100644 --- a/internal/api/handlers_meta.go +++ b/internal/api/handlers_meta.go @@ -20,7 +20,7 @@ import ( // - upstream unreachable/error: 502. // - match: 200 {"matched": true, ...} (see internal/meta.Enrichment). func (a *API) handleMeta(w http.ResponseWriter, r *http.Request) { - if a.meta == nil { + if !a.metadataOn() { writeError(w, http.StatusNotFound, "metadata lookup not enabled") return } diff --git a/internal/api/handlers_settings.go b/internal/api/handlers_settings.go new file mode 100644 index 0000000..ce04ad9 --- /dev/null +++ b/internal/api/handlers_settings.go @@ -0,0 +1,72 @@ +package api + +import "net/http" + +// Runtime-toggleable server settings, surfaced in the admin console. Transport +// only: the flag lives on the API (metaEnabled, an atomic.Bool) and the durable +// value is persisted to config.yaml via config.Save(). The envelope is an object +// keyed by feature so future settings can join it without reshaping the wire +// contract. + +// metadataOn reports whether the community metadata lookup is live: the service +// was constructed (base_url is valid) AND the runtime flag is on. The /meta +// handler and the `metadata` capability both gate on this. +func (a *API) metadataOn() bool { return a.meta != nil && a.metaEnabled.Load() } + +// settingsEnvelope builds the GET/PATCH response body. `available` reports +// whether the service is constructed at all (base_url valid); when false the +// feature cannot be enabled. +func (a *API) settingsEnvelope() map[string]any { + return map[string]any{ + "metadata": map[string]any{ + "enabled": a.metaEnabled.Load(), + "base_url": a.cfg.Metadata.BaseURL, + "available": a.meta != nil, + }, + } +} + +// handleGetSettings returns the current runtime settings (admin only). +func (a *API) handleGetSettings(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, a.settingsEnvelope()) +} + +// handleUpdateSettings flips runtime settings and persists them (admin only). +// Fields are pointers so an absent field is left unchanged. Enabling the metadata +// lookup when no service is configured (empty/invalid base_url) is a 400. +func (a *API) handleUpdateSettings(w http.ResponseWriter, r *http.Request) { + var req struct { + Metadata *struct { + Enabled *bool `json:"enabled"` + } `json:"metadata"` + } + if err := decodeJSON(r, &req, 0); err != nil { + writeError(w, http.StatusBadRequest, "invalid request") + return + } + + if req.Metadata != nil && req.Metadata.Enabled != nil { + want := *req.Metadata.Enabled + if want && a.meta == nil { + writeError(w, http.StatusBadRequest, + "metadata lookup is unavailable: set metadata.base_url to an absolute http(s) URL in the server config first") + return + } + // Serialize the mutate+persist so concurrent admin PATCHes don't race on the + // config struct or the config.yaml write. + a.settingsMu.Lock() + prev := a.cfg.Metadata.Enabled + a.cfg.Metadata.Enabled = want + if err := a.cfg.Save(); err != nil { + a.cfg.Metadata.Enabled = prev // roll back the in-memory change on a failed persist + a.settingsMu.Unlock() + a.log.Error("persist settings failed", "err", err) + writeError(w, http.StatusInternalServerError, "could not save settings") + return + } + a.metaEnabled.Store(want) + a.settingsMu.Unlock() + } + + writeJSON(w, http.StatusOK, a.settingsEnvelope()) +} diff --git a/internal/api/handlers_settings_test.go b/internal/api/handlers_settings_test.go new file mode 100644 index 0000000..343d557 --- /dev/null +++ b/internal/api/handlers_settings_test.go @@ -0,0 +1,129 @@ +package api + +import ( + "context" + "net/http" + "strconv" + "strings" + "testing" + + "github.com/kodestar/audiosilo-server/internal/auth" + "github.com/kodestar/audiosilo-server/internal/config" +) + +// TestAdminSettingsMetadataToggle drives the full runtime toggle: GET reflects +// config, PATCH off flips the /server capability AND makes /meta 404, PATCH on +// restores both. It uses a mock metaserve so the meta lookup provably works +// before the flip and again after. +func TestAdminSettingsMetadataToggle(t *testing.T) { + e := newMetaEnv(t, true, 0) + libID := seedBook(t, e, "Andy Weir/The Martian", "B00FLIJJSY") + adminTok, _ := e.auth.IssueToken(context.Background(), e.adminID, auth.KindSession, "t", 0) + metaPath := "/api/v1/libraries/" + strconv.FormatInt(libID, 10) + "/meta?path=" + escape("Andy Weir/The Martian") + + // GET settings reflects the enabled, configured service. + resp, body := e.do(t, "GET", "/api/v1/admin/settings", adminTok, "") + if resp.StatusCode != http.StatusOK { + t.Fatalf("get settings = %d %s, want 200", resp.StatusCode, body) + } + for _, want := range []string{`"metadata"`, `"enabled":true`, `"available":true`, e.cfg.Metadata.BaseURL} { + if !strings.Contains(body, want) { + t.Fatalf("settings envelope missing %q: %s", want, body) + } + } + + // Before the flip: capability true and the lookup works. + if _, si := e.do(t, "GET", "/api/v1/server", "", ""); !strings.Contains(si, `"metadata":true`) { + t.Fatalf("expected capability metadata:true, got %s", si) + } + if r, b := e.do(t, "GET", metaPath, adminTok, ""); r.StatusCode != http.StatusOK || !strings.Contains(b, `"matched":true`) { + t.Fatalf("meta before flip = %d %s, want 200 matched", r.StatusCode, b) + } + + // PATCH off: response shows disabled. + resp, body = e.do(t, "PATCH", "/api/v1/admin/settings", adminTok, `{"metadata":{"enabled":false}}`) + if resp.StatusCode != http.StatusOK || !strings.Contains(body, `"enabled":false`) { + t.Fatalf("patch off = %d %s, want 200 enabled:false", resp.StatusCode, body) + } + // Capability flips false AND the lookup 404s. + if _, si := e.do(t, "GET", "/api/v1/server", "", ""); !strings.Contains(si, `"metadata":false`) { + t.Fatalf("expected capability metadata:false after off, got %s", si) + } + if r, b := e.do(t, "GET", metaPath, adminTok, ""); r.StatusCode != http.StatusNotFound { + t.Fatalf("meta while off = %d %s, want 404", r.StatusCode, b) + } + + // PATCH on: capability true and the lookup works again. + if r, b := e.do(t, "PATCH", "/api/v1/admin/settings", adminTok, `{"metadata":{"enabled":true}}`); r.StatusCode != http.StatusOK || !strings.Contains(b, `"enabled":true`) { + t.Fatalf("patch on = %d %s, want 200 enabled:true", r.StatusCode, b) + } + if _, si := e.do(t, "GET", "/api/v1/server", "", ""); !strings.Contains(si, `"metadata":true`) { + t.Fatalf("expected capability metadata:true after on, got %s", si) + } + if r, b := e.do(t, "GET", metaPath, adminTok, ""); r.StatusCode != http.StatusOK || !strings.Contains(b, `"matched":true`) { + t.Fatalf("meta after on = %d %s, want 200 matched", r.StatusCode, b) + } +} + +// TestAdminSettingsEnableWithoutBaseURL: enabling the lookup when no base_url is +// configured (the service was never constructed) is a 400, and the envelope +// reports it as unavailable. +func TestAdminSettingsEnableWithoutBaseURL(t *testing.T) { + e := newTestEnvWith(t, func(c *config.Config) { + c.Metadata.Enabled = false + c.Metadata.BaseURL = "" + }) + adminTok, _ := e.auth.IssueToken(context.Background(), e.adminID, auth.KindSession, "t", 0) + + if _, body := e.do(t, "GET", "/api/v1/admin/settings", adminTok, ""); !strings.Contains(body, `"available":false`) || !strings.Contains(body, `"enabled":false`) { + t.Fatalf("expected unavailable+disabled envelope, got %s", body) + } + resp, body := e.do(t, "PATCH", "/api/v1/admin/settings", adminTok, `{"metadata":{"enabled":true}}`) + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("enable without base_url = %d %s, want 400", resp.StatusCode, body) + } + // Still disabled + still no capability. + if _, si := e.do(t, "GET", "/api/v1/server", "", ""); !strings.Contains(si, `"metadata":false`) { + t.Fatalf("capability should stay false, got %s", si) + } +} + +// TestAdminSettingsRequiresAdmin is the required allowed+denied security pair: +// both endpoints refuse a non-admin (403) and serve an admin (200). +func TestAdminSettingsRequiresAdmin(t *testing.T) { + e := newMetaEnv(t, true, 0) + ctx := context.Background() + member, _ := e.auth.CreateUser(ctx, "member", "member-password", auth.RoleUser) + memberTok, _ := e.auth.IssueToken(ctx, member.ID, auth.KindSession, "t", 0) + adminTok, _ := e.auth.IssueToken(ctx, e.adminID, auth.KindSession, "t", 0) + + for _, tc := range []struct{ method, body string }{ + {"GET", ""}, + {"PATCH", `{"metadata":{"enabled":false}}`}, + } { + if resp, _ := e.do(t, tc.method, "/api/v1/admin/settings", memberTok, tc.body); resp.StatusCode != http.StatusForbidden { + t.Fatalf("%s settings as non-admin = %d, want 403", tc.method, resp.StatusCode) + } + if resp, b := e.do(t, tc.method, "/api/v1/admin/settings", adminTok, tc.body); resp.StatusCode != http.StatusOK { + t.Fatalf("%s settings as admin = %d %s, want 200", tc.method, resp.StatusCode, b) + } + } +} + +// TestAdminSettingsPersisted: a PATCH writes config.yaml, so re-Loading the config +// from the data dir reflects the new value (survives a restart). +func TestAdminSettingsPersisted(t *testing.T) { + e := newMetaEnv(t, true, 0) + adminTok, _ := e.auth.IssueToken(context.Background(), e.adminID, auth.KindSession, "t", 0) + + if resp, b := e.do(t, "PATCH", "/api/v1/admin/settings", adminTok, `{"metadata":{"enabled":false}}`); resp.StatusCode != http.StatusOK { + t.Fatalf("patch off = %d %s, want 200", resp.StatusCode, b) + } + loaded, _, err := config.Load(e.cfg.DataDir) + if err != nil { + t.Fatalf("reload config: %v", err) + } + if loaded.Metadata.Enabled { + t.Fatalf("metadata.enabled not persisted as false: %+v", loaded.Metadata) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index e78a468..ac97635 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -113,6 +113,21 @@ type MetadataConfig struct { BaseURL string `yaml:"base_url"` // metaserve base URL; site at / and API at /api/v1 } +// ValidBaseURL reports whether BaseURL is a usable absolute http(s) URL. The +// metadata service is constructed whenever this holds (regardless of Enabled), +// so the admin runtime toggle can flip the feature on without a restart; an +// empty or non-http(s) base_url means the feature is unavailable. +func (m MetadataConfig) ValidBaseURL() bool { + if m.BaseURL == "" { + return false + } + u, err := url.Parse(m.BaseURL) + if err != nil { + return false + } + return u.Host != "" && (u.Scheme == "http" || u.Scheme == "https") +} + // Config is the full server configuration. type Config struct { // DataDir is where the database, config and generated certs live. It is not @@ -323,11 +338,7 @@ func (c *Config) Validate() error { if c.Metadata.BaseURL == "" { return errors.New("metadata lookup requires metadata.base_url") } - u, err := url.Parse(c.Metadata.BaseURL) - if err != nil { - return fmt.Errorf("invalid metadata.base_url %q: %w", c.Metadata.BaseURL, err) - } - if u.Host == "" || (u.Scheme != "http" && u.Scheme != "https") { + if !c.Metadata.ValidBaseURL() { return fmt.Errorf("metadata.base_url must be an absolute http(s) URL, got %q", c.Metadata.BaseURL) } } diff --git a/internal/web/assets/admin.html b/internal/web/assets/admin.html index 2277e09..1037574 100644 --- a/internal/web/assets/admin.html +++ b/internal/web/assets/admin.html @@ -83,6 +83,16 @@
Looks up book details from the community metadata service for books with an ASIN or ISBN; players show the extra details section only while this is on.
+ +