diff --git a/internal/api/admin_lidarr_test.go b/internal/api/admin_lidarr_test.go index f5b1618e..5db71172 100644 --- a/internal/api/admin_lidarr_test.go +++ b/internal/api/admin_lidarr_test.go @@ -137,8 +137,9 @@ func TestHandlePutLidarrConfig_EmptyKeyPreservesSaved(t *testing.T) { APIKey: "originalkey", }) - // PUT with empty api_key — should preserve "originalkey". - body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":""}`) + // PUT with empty api_key — should preserve "originalkey". enabled=true + // requires the defaults gate (missing_defaults) to be satisfied. + body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"","default_quality_profile_id":1,"default_metadata_profile_id":1,"default_root_folder_path":"/music"}`) w := doAdminReq(t, h, http.MethodPut, "/api/admin/lidarr/config", body, admin) if w.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) @@ -183,7 +184,7 @@ func TestHandlePutLidarrConfig_HappyPath(t *testing.T) { resetLidarrState(t, h) admin := seedAdminUser(t, h) - body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"newkey","default_quality_profile_id":2,"default_root_folder_path":"/music"}`) + body := []byte(`{"enabled":true,"base_url":"http://lidarr.lan:8686","api_key":"newkey","default_quality_profile_id":2,"default_metadata_profile_id":1,"default_root_folder_path":"/music"}`) w := doAdminReq(t, h, http.MethodPut, "/api/admin/lidarr/config", body, admin) if w.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) diff --git a/internal/api/admin_requests_test.go b/internal/api/admin_requests_test.go index e9957330..741a7d6a 100644 --- a/internal/api/admin_requests_test.go +++ b/internal/api/admin_requests_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/go-chi/chi/v5" @@ -174,10 +175,17 @@ func TestHandleApproveRequest_HappyPath(t *testing.T) { h, _ := testHandlersWithClientFn(t) resetLidarrState(t, h) - stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"id":1}`)) + switch { + case strings.Contains(r.URL.Path, "/metadataprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`)) + case strings.Contains(r.URL.Path, "/qualityprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Lossless"}]`)) + default: + _, _ = w.Write([]byte(`{"id":1}`)) + } })) t.Cleanup(stub.Close) @@ -221,7 +229,14 @@ func TestHandleApproveRequest_OverrideUsed(t *testing.T) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"id":1}`)) + switch { + case strings.Contains(r.URL.Path, "/metadataprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`)) + case strings.Contains(r.URL.Path, "/qualityprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Lossless"}]`)) + default: + _, _ = w.Write([]byte(`{"id":1}`)) + } })) t.Cleanup(stub.Close) diff --git a/internal/api/admin_users_test.go b/internal/api/admin_users_test.go index 44f35002..6895fb72 100644 --- a/internal/api/admin_users_test.go +++ b/internal/api/admin_users_test.go @@ -163,7 +163,10 @@ func TestAdminCreateUser_DuplicateUsername_409(t *testing.T) { admin := seedUser(t, pool, "duper", "pw", true) seedUser(t, pool, "existing", "pw", false) - body := `{"username":"existing","password":"abcd1234"}` + // seedUser prefixes usernames with dbtest.TestUserPrefix, so the + // row above is "test-existing"; POST that exact name to actually + // collide (stays test-prefixed for ResetDB). + body := `{"username":"test-existing","password":"abcd1234"}` req := httptest.NewRequest(http.MethodPost, "/api/admin/users", bytes.NewReader([]byte(body))) req = withUser(req, admin) rec := httptest.NewRecorder() diff --git a/internal/api/me_timezone_test.go b/internal/api/me_timezone_test.go index 0fae0852..576f694c 100644 --- a/internal/api/me_timezone_test.go +++ b/internal/api/me_timezone_test.go @@ -69,14 +69,18 @@ func TestPutTimezone_InvalidIANA(t *testing.T) { t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String()) } + // Error envelope is nested: {"error":{"code":...}} (matches the + // rest of /api/*), not a top-level {"code":...}. var errResp struct { - Code string `json:"code"` + Error struct { + Code string `json:"code"` + } `json:"error"` } if err := json.NewDecoder(rec.Body).Decode(&errResp); err != nil { t.Fatalf("decode err response: %v", err) } - if errResp.Code != "invalid_timezone" { - t.Errorf("error code = %q, want invalid_timezone", errResp.Code) + if errResp.Error.Code != "invalid_timezone" { + t.Errorf("error code = %q, want invalid_timezone", errResp.Error.Code) } } diff --git a/internal/api/playlists_system_test.go b/internal/api/playlists_system_test.go index 1a1570e1..621775fd 100644 --- a/internal/api/playlists_system_test.go +++ b/internal/api/playlists_system_test.go @@ -8,8 +8,6 @@ import ( "testing" "github.com/go-chi/chi/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" ) // Generic registry-driven system-playlist endpoints (#411 R2). @@ -17,7 +15,11 @@ import ( func newSystemPlaylistRouter(h *handlers) chi.Router { r := chi.NewRouter() r.Route("/api", func(api chi.Router) { - api.Use(auth.RequireUser(h.pool)) + // No real auth.RequireUser middleware: like every other api + // handler test, auth is supplied via withUser() context and the + // handlers self-guard with requireUser() (prelude.go). The real + // middleware needs a live session and would 401 the injected + // test user. api.Post("/playlists/system/{kind}/refresh", h.handleSystemPlaylistRefresh) api.Get("/playlists/system/{kind}/shuffle", h.handleSystemPlaylistShuffle) }) diff --git a/internal/api/prelude.go b/internal/api/prelude.go index 6c6f6fce..57333d49 100644 --- a/internal/api/prelude.go +++ b/internal/api/prelude.go @@ -19,7 +19,7 @@ import ( func requireUser(w http.ResponseWriter, r *http.Request) (dbq.User, bool) { user, ok := auth.UserFromContext(r.Context()) if !ok { - writeErr(w, apierror.Unauthorized("auth_required", "")) + writeErr(w, apierror.Unauthorized("unauthenticated", "")) return dbq.User{}, false } return user, true diff --git a/internal/audit/audit_test.go b/internal/audit/audit_test.go index 8254f801..ee413893 100644 --- a/internal/audit/audit_test.go +++ b/internal/audit/audit_test.go @@ -65,7 +65,9 @@ func TestWrite_WithMetadata(t *testing.T) { ).Scan(&meta); err != nil { t.Fatalf("read metadata: %v", err) } - if !contains(meta, `"first_admin":true`) || !contains(meta, `"reason":"test"`) { + // Postgres jsonb::text renders a space after ':' and ',', e.g. + // {"reason": "test", "first_admin": true}. + if !contains(meta, `"first_admin": true`) || !contains(meta, `"reason": "test"`) { t.Errorf("metadata = %q, expected first_admin + reason fields", meta) } } diff --git a/internal/db/migrations/0030_relax_art_source_check_to_nonempty.down.sql b/internal/db/migrations/0030_relax_art_source_check_to_nonempty.down.sql new file mode 100644 index 00000000..68fe224e --- /dev/null +++ b/internal/db/migrations/0030_relax_art_source_check_to_nonempty.down.sql @@ -0,0 +1,15 @@ +-- Restore the 0020 fixed-allowlist CHECKs. FAILS if any row holds a +-- source value outside the allowlist (the exact case 0030 enables) — +-- normalise such rows before rolling back. + +ALTER TABLE albums DROP CONSTRAINT IF EXISTS albums_cover_art_source_check; +ALTER TABLE albums + ADD CONSTRAINT albums_cover_art_source_check + CHECK (cover_art_source IS NULL + OR cover_art_source IN ('embedded','sidecar','mbcaa','theaudiodb','deezer','lastfm','none')); + +ALTER TABLE artists DROP CONSTRAINT IF EXISTS artists_artist_art_source_check; +ALTER TABLE artists + ADD CONSTRAINT artists_artist_art_source_check + CHECK (artist_art_source IS NULL + OR artist_art_source IN ('theaudiodb','deezer','lastfm','none')); diff --git a/internal/db/migrations/0030_relax_art_source_check_to_nonempty.up.sql b/internal/db/migrations/0030_relax_art_source_check_to_nonempty.up.sql new file mode 100644 index 00000000..35ca702c --- /dev/null +++ b/internal/db/migrations/0030_relax_art_source_check_to_nonempty.up.sql @@ -0,0 +1,19 @@ +-- The cover/artist art `*_source` column stores the recording +-- provider's registry ID. coverart.Register makes the provider set +-- extensible, so a fixed IN-list CHECK (0016 → 0018 → 0020) required a +-- schema migration for every new provider and rejected any +-- out-of-list value (e.g. test stub providers). Same brittleness +-- class as the discovery-mix CHECK incident (#433): a fixed-value +-- CHECK fighting an extensible value set. Relax to "NULL or any +-- non-empty string" — the registry, not the schema, is the source of +-- truth for valid provider IDs. + +ALTER TABLE albums DROP CONSTRAINT IF EXISTS albums_cover_art_source_check; +ALTER TABLE albums + ADD CONSTRAINT albums_cover_art_source_check + CHECK (cover_art_source IS NULL OR cover_art_source <> ''); + +ALTER TABLE artists DROP CONSTRAINT IF EXISTS artists_artist_art_source_check; +ALTER TABLE artists + ADD CONSTRAINT artists_artist_art_source_check + CHECK (artist_art_source IS NULL OR artist_art_source <> ''); diff --git a/internal/lidarrrequests/service_test.go b/internal/lidarrrequests/service_test.go index ab9ba4e8..49891988 100644 --- a/internal/lidarrrequests/service_test.go +++ b/internal/lidarrrequests/service_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/jackc/pgx/v5/pgtype" @@ -236,10 +237,20 @@ func TestListForUser_OnlyOwnRows(t *testing.T) { func approveTestSetup(t *testing.T) (*Service, pgtype.UUID, *pgxpool.Pool, *httptest.Server) { t.Helper() pool := newPool(t) - stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"id":1}`)) + // Approve resolves metadata/quality profiles before the add; + // those list endpoints return JSON arrays, not the {"id":1} + // object the add endpoints return. + switch { + case strings.Contains(r.URL.Path, "/metadataprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`)) + case strings.Contains(r.URL.Path, "/qualityprofile"): + _, _ = w.Write([]byte(`[{"id":7,"name":"Lossless"}]`)) + default: + _, _ = w.Write([]byte(`{"id":1}`)) + } })) t.Cleanup(stub.Close) diff --git a/web/src/lib/styles/error-copy.json b/web/src/lib/styles/error-copy.json index 9a29973f..c18961fd 100644 --- a/web/src/lib/styles/error-copy.json +++ b/web/src/lib/styles/error-copy.json @@ -1,7 +1,6 @@ { "unknown": "Something went wrong.", "unauthenticated": "Your session has ended. Please sign in again.", - "auth_required": "You need to sign in to do that.", "forbidden": "You don't have permission to do that.", "not_authorized": "You don't have permission to do that.", "invalid_credentials": "Wrong username or password.",