diff --git a/consts/consts.go b/consts/consts.go index f453ac125..e1a535c79 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -28,6 +28,7 @@ const ( UIAuthorizationHeader = "X-ND-Authorization" UIClientUniqueIDHeader = "X-ND-Client-Unique-Id" JWTSecretKey = "JWTSecret" + JWTPublicSecretKey = "JWTPublicSecret" JWTIssuer = "ND" DefaultSessionTimeout = 48 * time.Hour CookieExpiry = 365 * 24 * 3600 // One year diff --git a/core/auth/auth.go b/core/auth/auth.go index 7b3511bdf..b1e2667bd 100644 --- a/core/auth/auth.go +++ b/core/auth/auth.go @@ -19,35 +19,41 @@ import ( ) var ( - once sync.Once + once sync.Once + // TokenAuth signs UI/API session tokens. Rotated by the id migration so stale sessions die. TokenAuth *jwtauth.JWTAuth + // PublicTokenAuth signs public-link tokens (artwork, share streams), on a separate secret that survives the rotation. + PublicTokenAuth *jwtauth.JWTAuth ) -// Init creates a JWTAuth object from the secret stored in the DB. -// If the secret is not found, it will create a new one and store it in the DB. +// Init creates the JWTAuth objects from the secrets stored in the DB. +// Missing or undecryptable secrets are regenerated and stored. func Init(ds model.DataStore) { once.Do(func() { ctx := context.TODO() log.Info("Setting Session Timeout", "value", conf.Server.SessionTimeout) - secret, err := ds.Property(ctx).Get(consts.JWTSecretKey) - if err != nil || secret == "" { - log.Info(ctx, "Creating new JWT secret, used for encrypting UI sessions") - secret = createNewSecret(ctx, ds) - } else { - if secret, err = utils.Decrypt(ctx, getEncKey(), secret); err != nil { - log.Error(ctx, "Could not decrypt JWT secret, creating a new one", err) - secret = createNewSecret(ctx, ds) - } - } - - TokenAuth = jwtauth.New("HS256", []byte(secret), nil) + TokenAuth = jwtauth.New("HS256", []byte(loadOrCreateSecret(ctx, ds, consts.JWTSecretKey)), nil) + PublicTokenAuth = jwtauth.New("HS256", []byte(loadOrCreateSecret(ctx, ds, consts.JWTPublicSecretKey)), nil) }) } +func loadOrCreateSecret(ctx context.Context, ds model.DataStore, key string) string { + secret, err := ds.Property(ctx).Get(key) + if err != nil || secret == "" { + log.Info(ctx, "Creating new JWT secret", "key", key) + return createNewSecret(ctx, ds, key) + } + if secret, err = utils.Decrypt(ctx, getEncKey(), secret); err != nil { + log.Error(ctx, "Could not decrypt JWT secret, creating a new one", "key", key, err) + return createNewSecret(ctx, ds, key) + } + return secret +} + func CreatePublicToken(claims Claims) (string, error) { claims.Issuer = consts.JWTIssuer - _, token, err := TokenAuth.Encode(claims.ToMap()) + _, token, err := PublicTokenAuth.Encode(claims.ToMap()) return token, err } @@ -56,7 +62,7 @@ func CreateExpiringPublicToken(exp time.Time, claims Claims) (string, error) { if !exp.IsZero() { claims.ExpiresAt = exp } - _, token, err := TokenAuth.Encode(claims.ToMap()) + _, token, err := PublicTokenAuth.Encode(claims.ToMap()) return token, err } @@ -91,6 +97,15 @@ func Validate(tokenStr string) (Claims, error) { return ClaimsFromToken(token), nil } +// ValidatePublic verifies a public-link token against the public secret. +func ValidatePublic(tokenStr string) (Claims, error) { + token, err := jwtauth.VerifyToken(PublicTokenAuth, tokenStr) + if err != nil { + return Claims{}, err + } + return ClaimsFromToken(token), nil +} + func WithAdminUser(ctx context.Context, ds model.DataStore) context.Context { u, err := ds.User(ctx).FindFirstAdmin() if err != nil { @@ -107,14 +122,14 @@ func WithAdminUser(ctx context.Context, ds model.DataStore) context.Context { return request.WithUser(ctx, *u) } -func createNewSecret(ctx context.Context, ds model.DataStore) string { +func createNewSecret(ctx context.Context, ds model.DataStore, key string) string { secret := id.NewRandom() encSecret, err := utils.Encrypt(ctx, getEncKey(), secret) if err != nil { log.Error(ctx, "Could not encrypt JWT secret", err) return secret } - if err := ds.Property(ctx).Put(consts.JWTSecretKey, encSecret); err != nil { + if err := ds.Property(ctx).Put(key, encSecret); err != nil { log.Error(ctx, "Could not save JWT secret in DB", err) } return secret diff --git a/core/auth/auth_test.go b/core/auth/auth_test.go index 3a3585e53..e5cbb2352 100644 --- a/core/auth/auth_test.go +++ b/core/auth/auth_test.go @@ -89,6 +89,51 @@ var _ = Describe("Auth", func() { }) }) + Describe("Session/Public secret split", func() { + claims := func() map[string]any { + return map[string]any{"iss": "issuer", "exp": time.Now().Add(1 * time.Minute).Unix()} + } + + It("verifies a session token via Validate but not ValidatePublic", func() { + _, tokenStr, err := auth.TokenAuth.Encode(claims()) + Expect(err).NotTo(HaveOccurred()) + _, err = auth.Validate(tokenStr) + Expect(err).NotTo(HaveOccurred()) + _, err = auth.ValidatePublic(tokenStr) + Expect(err).To(HaveOccurred()) + }) + + It("verifies a public token via ValidatePublic but not Validate", func() { + _, tokenStr, err := auth.PublicTokenAuth.Encode(claims()) + Expect(err).NotTo(HaveOccurred()) + _, err = auth.ValidatePublic(tokenStr) + Expect(err).NotTo(HaveOccurred()) + _, err = auth.Validate(tokenStr) + Expect(err).To(HaveOccurred()) + }) + + It("decodes public tokens minted by CreatePublicToken via PublicTokenAuth", func() { + tokenStr, err := auth.CreatePublicToken(auth.Claims{ID: "art-1"}) + Expect(err).NotTo(HaveOccurred()) + claims, err := auth.ValidatePublic(tokenStr) + Expect(err).NotTo(HaveOccurred()) + Expect(claims.ID).To(Equal("art-1")) + _, err = auth.Validate(tokenStr) + Expect(err).To(HaveOccurred()) + }) + + It("decodes expiring public tokens minted by CreateExpiringPublicToken via PublicTokenAuth", func() { + exp := time.Now().Add(1 * time.Hour) + tokenStr, err := auth.CreateExpiringPublicToken(exp, auth.Claims{ID: "art-2"}) + Expect(err).NotTo(HaveOccurred()) + claims, err := auth.ValidatePublic(tokenStr) + Expect(err).NotTo(HaveOccurred()) + Expect(claims.ID).To(Equal("art-2")) + _, err = auth.Validate(tokenStr) + Expect(err).To(HaveOccurred()) + }) + }) + Describe("TouchToken", func() { It("updates the expiration time", func() { yesterday := time.Now().Add(-oneDay) diff --git a/core/publicurl/publicurl_test.go b/core/publicurl/publicurl_test.go index a195fb9cd..7e9ee8b8e 100644 --- a/core/publicurl/publicurl_test.go +++ b/core/publicurl/publicurl_test.go @@ -175,7 +175,7 @@ var _ = Describe("Public URL Utilities", func() { BeforeEach(func() { conf.Server.ShareURL = "https://share.example.com" // Initialize JWT auth for token generation - auth.TokenAuth = jwtauth.New("HS256", []byte("test secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test secret"), nil) }) It("generates a URL with the artwork token", func() { diff --git a/db/migrations/20260720015443_uniform_canonical_ids.go b/db/migrations/20260720015443_uniform_canonical_ids.go new file mode 100644 index 000000000..346f22050 --- /dev/null +++ b/db/migrations/20260720015443_uniform_canonical_ids.go @@ -0,0 +1,310 @@ +package migrations + +import ( + "context" + "crypto/md5" + "database/sql" + "encoding/hex" + "encoding/json" + "fmt" + "math/big" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/model/id" + "github.com/pressly/goose/v3" +) + +// canonicalID maps any historical Navidrome id shape to the canonical 22-char base62 encoding +// of a 128-bit value; unrecognized shapes (including empty and share ids) pass through unchanged. +func canonicalID(s string) string { + switch len(s) { + case 22: + v, ok := new(big.Int).SetString(s, 62) + if !ok || v.Sign() < 0 || v.BitLen() <= 128 { + return s + } + sum := md5.Sum([]byte(s)) + return id.Encode(sum) + case 32: + b, err := hex.DecodeString(s) + if err != nil { + return s + } + return id.Encode([16]byte(b)) + case 36: + if s[8] != '-' || s[13] != '-' || s[18] != '-' || s[23] != '-' { + return s + } + b, err := hex.DecodeString(s[:8] + s[9:13] + s[14:18] + s[19:23] + s[24:]) + if err != nil { + return s + } + return id.Encode([16]byte(b)) + } + return s +} + +func init() { + goose.AddMigrationContext(upUniformCanonicalIds, downUniformCanonicalIds) +} + +// idColumns is the exhaustive list of Navidrome-id-bearing columns; anything absent is deliberately exempt. +var idColumns = []struct{ table, col string }{ + {"media_file", "id"}, {"media_file", "pid"}, {"media_file", "artist_id"}, + {"media_file", "album_id"}, {"media_file", "album_artist_id"}, {"media_file", "folder_id"}, + {"album", "id"}, {"album", "album_artist_id"}, + {"artist", "id"}, + {"folder", "id"}, {"folder", "parent_id"}, + {"tag", "id"}, + {"library_artist", "artist_id"}, + {"user", "id"}, + {"user_props", "user_id"}, + {"playlist", "id"}, {"playlist", "owner_id"}, + {"playlist_tracks", "playlist_id"}, {"playlist_tracks", "media_file_id"}, + {"playlist_fields", "playlist_id"}, + {"annotation", "user_id"}, {"annotation", "item_id"}, + {"bookmark", "user_id"}, {"bookmark", "item_id"}, + {"player", "id"}, {"player", "user_id"}, {"player", "transcoding_id"}, + {"transcoding", "id"}, + {"radio", "id"}, + {"share", "user_id"}, + {"scrobble_buffer", "id"}, {"scrobble_buffer", "user_id"}, {"scrobble_buffer", "media_file_id"}, + {"playqueue", "id"}, {"playqueue", "user_id"}, + {"user_library", "user_id"}, + {"scrobbles", "user_id"}, {"scrobbles", "media_file_id"}, + {"media_file_artists", "media_file_id"}, {"media_file_artists", "artist_id"}, + {"album_artists", "album_id"}, {"album_artists", "artist_id"}, + {"library_tag", "tag_id"}, +} + +// embeddedIDColumns holds ids nested inside a larger value; the id-columns guard checks this +// list against the schema, so every JSON column must appear here or be exempted there. +var embeddedIDColumns = []struct { + table, col string + transform func(string) (string, bool) +}{ + {"playqueue", "items", canonicalizeIDList}, + {"share", "resource_ids", canonicalizeIDList}, + {"plugin", "users", canonicalizePluginUsers}, + {"playlist", "rules", canonicalizePlaylistRules}, +} + +func upUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { + if err := buildIDMap(ctx, tx); err != nil { + return err + } + for _, tc := range idColumns { + if err := applyIDMap(ctx, tx, tc.table, tc.col); err != nil { + return fmt.Errorf("canonicalizing %s.%s: %w", tc.table, tc.col, err) + } + } + for _, tc := range embeddedIDColumns { + if err := rewriteColumn(ctx, tx, tc.table, tc.col, tc.transform); err != nil { + return fmt.Errorf("canonicalizing %s.%s: %w", tc.table, tc.col, err) + } + } + // Legacy PID specs embed old-shaped album/track ids into composite pids; a full rescan + // rewrites every pid with the new encoding so path-based move matching stays consistent. + if strings.Contains(conf.Server.PID.Track, "legacy") || strings.Contains(conf.Server.PID.Album, "legacy") { + if err := forceFullRescan(ctx, tx); err != nil { + return err + } + } + if err := rotateSessionSecret(ctx, tx); err != nil { + return err + } + _, err := tx.ExecContext(ctx, "DROP TABLE _id_map") + return err +} + +// rotateSessionSecret renames the session JWT secret to the public key, so public-link tokens +// keep verifying while auth.Init mints a fresh session secret, killing every stale session. +func rotateSessionSecret(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, + `UPDATE property SET id = ? WHERE id = ? AND NOT EXISTS (SELECT 1 FROM property WHERE id = ?)`, + consts.JWTPublicSecretKey, consts.JWTSecretKey, consts.JWTPublicSecretKey) + return err +} + +// buildIDMap stages old->new pairs for every id that changes, indexed for the update joins. +func buildIDMap(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, + "CREATE TEMP TABLE _id_map (old_id TEXT PRIMARY KEY, new_id TEXT NOT NULL) WITHOUT ROWID") + if err != nil { + return err + } + ins, err := tx.PrepareContext(ctx, "INSERT OR IGNORE INTO _id_map (old_id, new_id) VALUES (?, ?)") + if err != nil { + return err + } + defer ins.Close() + for _, tc := range idColumns { + if err := collectColumn(ctx, tx, ins, tc.table, tc.col); err != nil { + return fmt.Errorf("collecting %s.%s: %w", tc.table, tc.col, err) + } + } + return nil +} + +func collectColumn(ctx context.Context, tx *sql.Tx, ins *sql.Stmt, table, col string) error { + rows, err := tx.QueryContext(ctx, fmt.Sprintf( + "SELECT DISTINCT %[2]s FROM %[1]s WHERE %[2]s IS NOT NULL AND %[2]s <> ''", table, col)) + if err != nil { + return err + } + defer rows.Close() + for rows.Next() { + var old string + if err := rows.Scan(&old); err != nil { + return err + } + if newID := canonicalID(old); newID != old { + if _, err := ins.ExecContext(ctx, old, newID); err != nil { + return err + } + } + } + return rows.Err() +} + +func applyIDMap(ctx context.Context, tx *sql.Tx, table, col string) error { + _, err := tx.ExecContext(ctx, fmt.Sprintf( + `UPDATE %[1]s SET %[2]s = (SELECT new_id FROM _id_map WHERE old_id = %[1]s.%[2]s) + WHERE %[2]s IN (SELECT old_id FROM _id_map)`, table, col)) + return err +} + +// canonicalizeIDList canonicalizes a comma-separated id list element-wise. +func canonicalizeIDList(s string) (string, bool) { + parts := strings.Split(s, ",") + changed := false + for i, p := range parts { + if n := canonicalID(p); n != p { + parts[i] = n + changed = true + } + } + if !changed { + return s, false + } + return strings.Join(parts, ","), true +} + +// rewriteColumn applies transform to each non-empty cell, updating only rows it changed. +func rewriteColumn(ctx context.Context, tx *sql.Tx, table, col string, transform func(string) (string, bool)) error { + rows, err := tx.QueryContext(ctx, fmt.Sprintf( + "SELECT rowid, %[2]s FROM %[1]s WHERE ifnull(%[2]s, '') <> ''", table, col)) + if err != nil { + return err + } + type change struct { + rowid int64 + val string + } + var changes []change + for rows.Next() { + var rowid int64 + var val string + if err := rows.Scan(&rowid, &val); err != nil { + _ = rows.Close() + return err + } + if newVal, ok := transform(val); ok { + changes = append(changes, change{rowid, newVal}) + } + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return err + } + _ = rows.Close() + if len(changes) == 0 { + return nil + } + upd, err := tx.PrepareContext(ctx, fmt.Sprintf("UPDATE %s SET %s = ? WHERE rowid = ?", table, col)) + if err != nil { + return err + } + defer upd.Close() + for _, c := range changes { + if _, err := upd.ExecContext(ctx, c.val, c.rowid); err != nil { + return err + } + } + return nil +} + +// canonicalizePluginUsers maps a JSON array of user ids; malformed JSON passes through untouched. +func canonicalizePluginUsers(s string) (string, bool) { + var users []string + if err := json.Unmarshal([]byte(s), &users); err != nil { + return s, false + } + changed := false + for i, u := range users { + if n := canonicalID(u); n != u { + users[i] = n + changed = true + } + } + if !changed { + return s, false + } + out, err := json.Marshal(users) + if err != nil { + return s, false + } + return string(out), true +} + +// canonicalizePlaylistRules rewrites inPlaylist/notInPlaylist ids in smart-playlist criteria; malformed JSON passes through. +func canonicalizePlaylistRules(s string) (string, bool) { + var root map[string]any + if err := json.Unmarshal([]byte(s), &root); err != nil { + return s, false + } + if !canonicalizeRulesNode(root) { + return s, false + } + out, err := json.Marshal(root) + if err != nil { + return s, false + } + return string(out), true +} + +// canonicalizeRulesNode walks the criteria tree, canonicalizing the id of any inPlaylist/notInPlaylist object. +func canonicalizeRulesNode(node any) bool { + changed := false + switch v := node.(type) { + case map[string]any: + for k, val := range v { + if lk := strings.ToLower(k); lk == "inplaylist" || lk == "notinplaylist" { + if obj, ok := val.(map[string]any); ok { + if id, ok := obj["id"].(string); ok { + if n := canonicalID(id); n != id { + obj["id"] = n + changed = true + } + } + } + } + if canonicalizeRulesNode(val) { + changed = true + } + } + case []any: + for _, item := range v { + if canonicalizeRulesNode(item) { + changed = true + } + } + } + return changed +} + +func downUniformCanonicalIds(ctx context.Context, tx *sql.Tx) error { + return nil // irreversible data migration +} diff --git a/db/migrations/id_canonical_test.go b/db/migrations/id_canonical_test.go new file mode 100644 index 000000000..0befbe469 --- /dev/null +++ b/db/migrations/id_canonical_test.go @@ -0,0 +1,47 @@ +package migrations + +import ( + "strings" + + "github.com/navidrome/navidrome/model/id" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("canonicalID", func() { + DescribeTable("transforms each historical id shape", + func(in, want string) { + Expect(canonicalID(in)).To(Equal(want)) + }, + Entry("hash-family id (fits 128 bits) is kept", "5cLJPkLA5DK2BADhoeotPk", "5cLJPkLA5DK2BADhoeotPk"), + Entry("overflowing random id is remapped via md5", "zzzzzzzzzzzzzzzzzzzzzz", "3LyqmwQBm5IRqlVjNYASwb"), + Entry("legacy 32-hex is re-encoded value-preserving", "e3b7fc2ae9447bbec37a13bf916e3cf6", "6VHl3uR4kss6sUPKA8Cwnk"), + Entry("playlist uuid is re-encoded value-preserving", "f47ac10b-58cc-4372-a567-0e02b2c3d479", "7rke2SAWaicSeSYzkhww6R"), + Entry("empty string passes through", "", ""), + Entry("share id (10 chars) passes through", "aB3xY9kQz1", "aB3xY9kQz1"), + Entry("truncated Finamp id (16 chars) passes through", "0123456789abcdef", "0123456789abcdef"), + Entry("22 chars with non-base62 char passes through", "!!!!!!!!!!!!!!!!!!!!!!", "!!!!!!!!!!!!!!!!!!!!!!"), + Entry("32 chars non-hex passes through", "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"), + Entry("36 chars without uuid dashes passes through", "000000000000000000000000000000000000", "000000000000000000000000000000000000"), + ) + + // The exemptions for participants/tags/folder_ids/similar_artists rest on this invariant. + It("is the identity on every NewHash id", func() { + for _, parts := range [][]string{ + {""}, {"a"}, {"The Beatles"}, {"genre", "electronic"}, + {"/music/Artist/Album", "1"}, {strings.Repeat("x", 500)}, + } { + h := id.NewHash(parts...) + Expect(h).To(HaveLen(22)) + Expect(canonicalID(h)).To(Equal(h), "NewHash(%v) = %q was rewritten", parts, h) + } + }) + + It("is idempotent for every shape", func() { + for _, s := range []string{"5cLJPkLA5DK2BADhoeotPk", "zzzzzzzzzzzzzzzzzzzzzz", + "e3b7fc2ae9447bbec37a13bf916e3cf6", "f47ac10b-58cc-4372-a567-0e02b2c3d479"} { + once := canonicalID(s) + Expect(canonicalID(once)).To(Equal(once)) + } + }) +}) diff --git a/db/migrations/id_columns_guard_test.go b/db/migrations/id_columns_guard_test.go new file mode 100644 index 000000000..c4cb3b241 --- /dev/null +++ b/db/migrations/id_columns_guard_test.go @@ -0,0 +1,119 @@ +package migrations + +import ( + "context" + "database/sql" + "os" + "strings" + + _ "github.com/mattn/go-sqlite3" + "github.com/pressly/goose/v3" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Version of the uniform-canonical-ids migration whose idColumns list this guard protects. +const idColumnsMigrationVersion = 20260720015443 + +var _ = Describe("idColumns inventory", func() { + It("covers every id-bearing column present at the migration version", func() { + ctx := context.Background() + db, err := sql.Open("sqlite3", "file::memory:") + Expect(err).ToNot(HaveOccurred()) + db.SetMaxOpenConns(1) // non-shared :memory: — a second conn would be an empty DB + DeferCleanup(func() { _ = db.Close() }) + _, err = db.ExecContext(ctx, "PRAGMA foreign_keys=off") + Expect(err).ToNot(HaveOccurred()) + + goose.SetBaseFS(os.DirFS(".")) + DeferCleanup(func() { goose.SetBaseFS(nil) }) + Expect(goose.SetDialect("sqlite3")).To(Succeed()) + Expect(goose.UpToContext(ctx, db, ".", idColumnsMigrationVersion)).To(Succeed()) + + covered := map[string]bool{} + for _, tc := range idColumns { + covered[tc.table+"."+tc.col] = true + } + for _, tc := range embeddedIDColumns { + covered[tc.table+"."+tc.col] = true + } + // Columns that are id-named or JSON but need no rewrite. + exempt := map[string]string{ + "share.id": "public share URLs, generated separately", + "property.id": "property key, not an entity id", + "plugin.id": "plugin name, not an entity id", + "media_file.participants": "hash-family artist ids, unchanged", + "album.participants": "hash-family artist ids, unchanged", + "media_file.tags": "hash-family tag ids, unchanged", + "album.tags": "hash-family tag ids, unchanged", + "album.folder_ids": "hash-family folder ids, unchanged", + "artist.similar_artists": "hash-family artist ids, unchanged", + "media_file.search_participants": "participant names for FTS, not ids", + "album.search_participants": "participant names for FTS, not ids", + "album.discs": "disc number -> title map", + "media_file.lyrics": "synced lyrics, no ids", + "folder.image_files": "image file names, no ids", + "plugin.manifest": "plugin-authored manifest, no Navidrome ids", + "plugin.config": "free-form plugin config, must not be rewritten", + "plugin.libraries": "integer library ids", + } + + tables, err := queryColumn(ctx, db, "SELECT name FROM sqlite_master WHERE type='table'") + Expect(err).ToNot(HaveOccurred()) + + var dangling []string + for _, table := range tables { + if strings.HasPrefix(table, "sqlite_") || table == "goose_db_version" || strings.Contains(table, "_fts") { + continue + } + rows, err := db.QueryContext(ctx, "SELECT name, type FROM pragma_table_info(?)", table) + Expect(err).ToNot(HaveOccurred()) + for rows.Next() { + var name, typ string + Expect(rows.Scan(&name, &typ)).To(Succeed()) + lname := strings.ToLower(name) + utyp := strings.ToUpper(typ) + // JSON can hide an id under any key, so every JSON column needs a verdict. + isJSON := strings.Contains(utyp, "JSON") + isIDName := lname == "id" || lname == "pid" || + strings.HasSuffix(lname, "_id") || strings.HasSuffix(lname, "_ids") + if !isJSON && !isIDName { + continue + } + if !isJSON && !strings.Contains(utyp, "TEXT") && !strings.Contains(utyp, "CHAR") { + continue // INTEGER ids (rowid PKs, library.id) are not canonical ids + } + if strings.HasPrefix(lname, "mbz_") { + continue // MusicBrainz UUIDs, not Navidrome ids + } + key := table + "." + name + if covered[key] || exempt[key] != "" { + continue + } + dangling = append(dangling, key) + } + Expect(rows.Err()).ToNot(HaveOccurred()) + Expect(rows.Close()).To(Succeed()) + } + Expect(dangling).To(BeEmpty(), + "id-bearing columns missing from idColumns; add them to the migration or exempt with a reason: %v", dangling) + }) +}) + +func queryColumn(ctx context.Context, db *sql.DB, query string) ([]string, error) { + rows, err := db.QueryContext(ctx, query) + if err != nil { + return nil, err + } + defer rows.Close() + var out []string + for rows.Next() { + var s string + if err := rows.Scan(&s); err != nil { + return nil, err + } + out = append(out, s) + } + return out, rows.Err() +} diff --git a/db/migrations/migrations_suite_test.go b/db/migrations/migrations_suite_test.go new file mode 100644 index 000000000..bb08ad8aa --- /dev/null +++ b/db/migrations/migrations_suite_test.go @@ -0,0 +1,16 @@ +package migrations + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// tests.Init is omitted: the tests package imports db, which imports this package. +func TestMigrations(t *testing.T) { + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "Migrations Suite") +} diff --git a/db/migrations/uniform_canonical_ids_test.go b/db/migrations/uniform_canonical_ids_test.go new file mode 100644 index 000000000..ced75cb7e --- /dev/null +++ b/db/migrations/uniform_canonical_ids_test.go @@ -0,0 +1,233 @@ +package migrations + +import ( + "context" + "database/sql" + "encoding/json" + + _ "github.com/mattn/go-sqlite3" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("upUniformCanonicalIds", func() { + var db *sql.DB + var tx *sql.Tx + ctx := context.Background() + + const ( + hashID = "5cLJPkLA5DK2BADhoeotPk" // canonical, kept + randOld = "zzzzzzzzzzzzzzzzzzzzzz" // overflows -> remapped + randNew = "3LyqmwQBm5IRqlVjNYASwb" + legacyOld = "e3b7fc2ae9447bbec37a13bf916e3cf6" // 32-hex -> re-encoded + legacyNew = "6VHl3uR4kss6sUPKA8Cwnk" + uuidOld = "f47ac10b-58cc-4372-a567-0e02b2c3d479" // uuid -> re-encoded + uuidNew = "7rke2SAWaicSeSYzkhww6R" + shareID = "aB3xY9kQz1" // exempt family + + sessionSecret = "encrypted-session-secret-sentinel" + ) + + BeforeEach(func() { + var err error + db, err = sql.Open("sqlite3", "file::memory:") + Expect(err).ToNot(HaveOccurred()) + db.SetMaxOpenConns(1) // non-shared :memory: — every new conn is a fresh empty DB + DeferCleanup(func() { _ = db.Close() }) + + // Minimal fixture: every table/column idColumns and the list rewrites touch. + _, err = db.Exec(` + CREATE TABLE media_file (id text, pid text, artist_id text, album_id text, album_artist_id text, folder_id text, mbz_recording_id text); + CREATE TABLE album (id text, album_artist_id text); + CREATE TABLE artist (id text); + CREATE TABLE folder (id text, parent_id text); + CREATE TABLE tag (id text); + CREATE TABLE library_artist (artist_id text); + CREATE TABLE user (id text); + CREATE TABLE user_props (user_id text); + CREATE TABLE playlist (id text, owner_id text, rules text); + CREATE TABLE playlist_tracks (playlist_id text, media_file_id text); + CREATE TABLE playlist_fields (playlist_id text); + CREATE TABLE annotation (user_id text, item_id text, item_type text); + CREATE TABLE bookmark (user_id text, item_id text); + CREATE TABLE player (id text, user_id text, transcoding_id text); + CREATE TABLE transcoding (id text); + CREATE TABLE radio (id text); + CREATE TABLE share (id text, user_id text, resource_ids text, contents text); + CREATE TABLE scrobble_buffer (id text, user_id text, media_file_id text); + CREATE TABLE playqueue (id text, user_id text, items text); + CREATE TABLE user_library (user_id text, library_id integer); + CREATE TABLE scrobbles (id integer, media_file_id text, user_id text); + CREATE TABLE media_file_artists (media_file_id text, artist_id text); + CREATE TABLE album_artists (album_id text, artist_id text); + CREATE TABLE library_tag (tag_id text, library_id integer); + CREATE TABLE plugin (id text, users text); + CREATE TABLE property (id text primary key, value text); + `) + Expect(err).ToNot(HaveOccurred()) + + seed := func(query string, args ...any) { + _, err := db.Exec(query, args...) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + } + // media_file: legacy id/pid, hash artist, legacy album, mbz uuid must stay untouched + seed(`INSERT INTO media_file VALUES (?, ?, ?, ?, '', '', ?)`, legacyOld, legacyOld, hashID, legacyOld, uuidOld) + seed(`INSERT INTO album VALUES (?, ?)`, legacyOld, hashID) + seed(`INSERT INTO user VALUES (?)`, randOld) + // uuid id, random owner, smart-playlist rules with an embedded inPlaylist id and a sibling operator + seed(`INSERT INTO playlist VALUES (?, ?, ?)`, uuidOld, randOld, `{"all":[{"inPlaylist":{"id":"`+uuidOld+`"}},{"inTheLast":{"lastPlayed":30}}]}`) + seed(`INSERT INTO annotation VALUES (?, ?, 'media_file')`, randOld, legacyOld) + seed(`INSERT INTO playqueue VALUES (?, ?, ?)`, randOld, randOld, legacyOld+","+hashID) + seed(`INSERT INTO share VALUES (?, ?, ?, 'Album Foo...')`, shareID, randOld, legacyOld+","+uuidOld) + seed(`INSERT INTO user_library VALUES (?, 1)`, randOld) + seed(`INSERT INTO scrobbles VALUES (1, ?, ?)`, legacyOld, randOld) + seed(`INSERT INTO media_file_artists VALUES (?, ?)`, legacyOld, hashID) + seed(`INSERT INTO album_artists VALUES (?, ?)`, legacyOld, hashID) + seed(`INSERT INTO library_tag VALUES (?, 1)`, hashID) + seed(`INSERT INTO plugin VALUES ('lastfm', ?)`, `["`+randOld+`","`+hashID+`"]`) + seed(`INSERT INTO plugin VALUES ('empty', '[]')`) // exempt: empty user list untouched + // malformed JSON in both a plugin list and a playlist rule: must pass through byte-for-byte + seed(`INSERT INTO plugin VALUES ('broken', 'not-json')`) + seed(`INSERT INTO playlist VALUES (?, ?, '{broken')`, hashID, hashID) + seed(`INSERT INTO property VALUES (?, ?)`, consts.JWTSecretKey, sessionSecret) + }) + + JustBeforeEach(func() { + var err error + tx, err = db.Begin() + Expect(err).ToNot(HaveOccurred()) + Expect(upUniformCanonicalIds(ctx, tx)).To(Succeed()) + Expect(tx.Commit()).To(Succeed()) + }) + + get := func(query string) string { + var s string + ExpectWithOffset(1, db.QueryRow(query).Scan(&s)).To(Succeed()) + return s + } + + It("canonicalizes every id family and keeps references consistent", func() { + Expect(get(`SELECT id FROM media_file`)).To(Equal(legacyNew)) + Expect(get(`SELECT pid FROM media_file`)).To(Equal(legacyNew)) + Expect(get(`SELECT artist_id FROM media_file`)).To(Equal(hashID)) + Expect(get(`SELECT album_id FROM media_file`)).To(Equal(legacyNew)) + Expect(get(`SELECT id FROM album`)).To(Equal(legacyNew)) + Expect(get(`SELECT id FROM user`)).To(Equal(randNew)) + Expect(get(`SELECT id FROM playlist WHERE owner_id='` + randNew + `'`)).To(Equal(uuidNew)) + Expect(get(`SELECT owner_id FROM playlist WHERE id='` + uuidNew + `'`)).To(Equal(randNew)) + Expect(get(`SELECT item_id FROM annotation`)).To(Equal(legacyNew)) + Expect(get(`SELECT user_id FROM annotation`)).To(Equal(randNew)) + }) + + It("rewrites list columns element-wise", func() { + Expect(get(`SELECT items FROM playqueue`)).To(Equal(legacyNew + "," + hashID)) + Expect(get(`SELECT resource_ids FROM share`)).To(Equal(legacyNew + "," + uuidNew)) + }) + + It("leaves exempt columns alone", func() { + Expect(get(`SELECT id FROM share`)).To(Equal(shareID)) + Expect(get(`SELECT contents FROM share`)).To(Equal("Album Foo...")) + Expect(get(`SELECT mbz_recording_id FROM media_file`)).To(Equal(uuidOld)) + }) + + It("canonicalizes junction and membership tables", func() { + Expect(get(`SELECT user_id FROM user_library`)).To(Equal(randNew)) + Expect(get(`SELECT media_file_id FROM scrobbles`)).To(Equal(legacyNew)) + Expect(get(`SELECT user_id FROM scrobbles`)).To(Equal(randNew)) + Expect(get(`SELECT media_file_id FROM media_file_artists`)).To(Equal(legacyNew)) + Expect(get(`SELECT artist_id FROM media_file_artists`)).To(Equal(hashID)) // hash-family, unchanged + Expect(get(`SELECT album_id FROM album_artists`)).To(Equal(legacyNew)) + Expect(get(`SELECT artist_id FROM album_artists`)).To(Equal(hashID)) // hash-family, unchanged + Expect(get(`SELECT tag_id FROM library_tag`)).To(Equal(hashID)) // hash-family, unchanged + }) + + It("rewrites JSON columns element-wise", func() { + Expect(get(`SELECT users FROM plugin WHERE id='lastfm'`)).To(Equal(`["` + randNew + `","` + hashID + `"]`)) + Expect(get(`SELECT id FROM plugin WHERE id='lastfm'`)).To(Equal("lastfm")) // plugin name, untouched + + var rules map[string]any + Expect(json.Unmarshal([]byte(get(`SELECT rules FROM playlist WHERE id='`+uuidNew+`'`)), &rules)).To(Succeed()) + all, ok := rules["all"].([]any) + Expect(ok).To(BeTrue()) + Expect(all).To(HaveLen(2)) + inPl := all[0].(map[string]any)["inPlaylist"].(map[string]any) + Expect(inPl["id"]).To(Equal(uuidNew)) + }) + + It("preserves sibling operators alongside a rewritten inPlaylist id", func() { + var rules map[string]any + Expect(json.Unmarshal([]byte(get(`SELECT rules FROM playlist WHERE id='`+uuidNew+`'`)), &rules)).To(Succeed()) + all := rules["all"].([]any) + var sawInPlaylist, sawInTheLast bool + for _, e := range all { + op := e.(map[string]any) + if pl, ok := op["inPlaylist"].(map[string]any); ok { + Expect(pl["id"]).To(Equal(uuidNew)) + sawInPlaylist = true + } + if last, ok := op["inTheLast"].(map[string]any); ok { + Expect(last["lastPlayed"]).To(Equal(float64(30))) + sawInTheLast = true + } + } + Expect(sawInPlaylist).To(BeTrue()) + Expect(sawInTheLast).To(BeTrue()) + }) + + It("leaves exempt JSON rows untouched", func() { + Expect(get(`SELECT users FROM plugin WHERE id='empty'`)).To(Equal("[]")) + }) + + It("passes malformed JSON columns through byte-for-byte", func() { + Expect(get(`SELECT users FROM plugin WHERE id='broken'`)).To(Equal("not-json")) + Expect(get(`SELECT rules FROM playlist WHERE id='` + hashID + `'`)).To(Equal("{broken")) + }) + + rescanCount := func() int { + var count int + ExpectWithOffset(1, db.QueryRow( + `SELECT count(*) FROM property WHERE id = ?`, consts.FullScanAfterMigrationFlagKey).Scan(&count)).To(Succeed()) + return count + } + + It("does not force a full rescan for the default PID config", func() { + Expect(rescanCount()).To(Equal(0)) + }) + + Context("with a legacy PID configuration", func() { + BeforeEach(func() { + prev := conf.Server.PID.Album + conf.Server.PID.Album = "album_legacy" + DeferCleanup(func() { conf.Server.PID.Album = prev }) + }) + + It("forces a full rescan so composite pids are rewritten", func() { + Expect(rescanCount()).To(Equal(1)) + }) + }) + + propCount := func(key string) int { + var count int + ExpectWithOffset(1, db.QueryRow(`SELECT count(*) FROM property WHERE id = ?`, key).Scan(&count)).To(Succeed()) + return count + } + + It("rotates the session secret to the public key", func() { + Expect(propCount(consts.JWTSecretKey)).To(Equal(0)) + Expect(get(`SELECT value FROM property WHERE id = '` + consts.JWTPublicSecretKey + `'`)).To(Equal(sessionSecret)) + }) + + Context("with no stored session secret", func() { + BeforeEach(func() { + _, err := db.Exec(`DELETE FROM property WHERE id = ?`, consts.JWTSecretKey) + Expect(err).ToNot(HaveOccurred()) + }) + + It("is a no-op", func() { + Expect(propCount(consts.JWTSecretKey)).To(Equal(0)) + Expect(propCount(consts.JWTPublicSecretKey)).To(Equal(0)) + }) + }) +}) diff --git a/model/id/id.go b/model/id/id.go index b54542898..2885bc6ad 100644 --- a/model/id/id.go +++ b/model/id/id.go @@ -2,20 +2,36 @@ package id import ( "crypto/md5" + "crypto/rand" "fmt" "math/big" "strings" - - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/utils/nanoid" ) func NewRandom() string { - id, err := nanoid.Generate("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz", 22) - if err != nil { - log.Error("Could not generate new ID", err) + var b [16]byte + _, _ = rand.Read(b[:]) + return Encode(b) +} + +// Encode renders a 16-byte value as the canonical 22-char zero-padded base62 id. +func Encode(b [16]byte) string { + return fmt.Sprintf("%022s", new(big.Int).SetBytes(b[:]).Text(62)) +} + +// Decode is the exact inverse of Encode. +func Decode(s string) ([]byte, error) { + if len(s) != 22 { + return nil, fmt.Errorf("invalid id length %d", len(s)) } - return id + v, ok := new(big.Int).SetString(s, 62) + if !ok || v.Sign() < 0 { + return nil, fmt.Errorf("invalid base62 id %q", s) + } + if v.BitLen() > 128 { + return nil, fmt.Errorf("id %q overflows 128 bits", s) + } + return v.FillBytes(make([]byte, 16)), nil } func NewHash(data ...string) string { @@ -24,11 +40,7 @@ func NewHash(data ...string) string { hash.Write([]byte(d)) hash.Write([]byte(string('\u200b'))) } - h := hash.Sum(nil) - bi := big.NewInt(0) - bi.SetBytes(h) - s := bi.Text(62) - return fmt.Sprintf("%022s", s) + return Encode([16]byte(hash.Sum(nil))) } func NewTagID(name, value string) string { diff --git a/model/id/id_suite_test.go b/model/id/id_suite_test.go new file mode 100644 index 000000000..e8a41f4e5 --- /dev/null +++ b/model/id/id_suite_test.go @@ -0,0 +1,17 @@ +package id_test + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestID(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "ID Suite") +} diff --git a/model/id/id_test.go b/model/id/id_test.go new file mode 100644 index 000000000..407560a75 --- /dev/null +++ b/model/id/id_test.go @@ -0,0 +1,64 @@ +package id_test + +import ( + "github.com/navidrome/navidrome/model/id" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Encode/Decode", func() { + It("encodes 16 bytes as 22-char zero-padded base62", func() { + Expect(id.Encode([16]byte{})).To(Equal("0000000000000000000000")) + allFF := [16]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff} + Expect(id.Encode(allFF)).To(Equal("7N42dgm5tFLK9N8MT7fHC7")) + }) + + It("round-trips arbitrary 16-byte values", func() { + b := [16]byte{0xe3, 0xb7, 0xfc, 0x2a, 0xe9, 0x44, 0x7b, 0xbe, + 0xc3, 0x7a, 0x13, 0xbf, 0x91, 0x6e, 0x3c, 0xf6} + s := id.Encode(b) + Expect(s).To(Equal("6VHl3uR4kss6sUPKA8Cwnk")) + Expect(id.Decode(s)).To(Equal(b[:])) + }) + + It("rejects invalid input", func() { + _, err := id.Decode("short") + Expect(err).To(HaveOccurred()) + _, err = id.Decode("!!!!!!!!!!!!!!!!!!!!!!") // 22 chars, not base62 + Expect(err).To(HaveOccurred()) + _, err = id.Decode("-000000000000000000001") // sign is not part of the alphabet + Expect(err).To(HaveOccurred()) + _, err = id.Decode("zzzzzzzzzzzzzzzzzzzzzz") // > 2^128 + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("NewRandom", func() { + It("emits 22-char canonical ids that always fit 128 bits", func() { + seen := make(map[string]struct{}) + for range 1000 { + s := id.NewRandom() + Expect(s).To(HaveLen(22)) + _, err := id.Decode(s) + Expect(err).ToNot(HaveOccurred(), "id %q must decode to 128 bits", s) + seen[s] = struct{}{} + } + Expect(seen).To(HaveLen(1000)) + }) +}) + +var _ = Describe("NewHash", func() { + It("keeps its historical output byte-for-byte (golden)", func() { + Expect(id.NewHash("test")).To(Equal("5cLJPkLA5DK2BADhoeotPk")) + Expect(id.NewHash("[unknown artist]")).To(Equal("7lsE5pS09fPS1VuFqwXbia")) + Expect(id.NewTagID("genre", "electronic")).To(Equal("7bLYq0Np81m1Wgy5N31nuG")) + }) + + It("always emits 22 decodable chars", func() { + h := id.NewHash("anything", "at", "all") + Expect(h).To(HaveLen(22)) + _, err := id.Decode(h) + Expect(err).ToNot(HaveOccurred()) + }) +}) diff --git a/model/metadata/legacy_ids.go b/model/metadata/legacy_ids.go index 18a273550..7156c4f50 100644 --- a/model/metadata/legacy_ids.go +++ b/model/metadata/legacy_ids.go @@ -9,17 +9,19 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" ) -// These are the legacy ID functions that were used in the original Navidrome ID generation. -// They are kept here for backwards compatibility with existing databases. +// These legacy ID functions hash the same inputs as the original Navidrome ID generation, +// now emitted in the canonical base62 encoding (matching what the uniform-ids migration stores). func legacyTrackID(mf model.MediaFile, prependLibId bool) string { - id := mf.Path + key := mf.Path if prependLibId && mf.LibraryID != model.DefaultLibraryID { - id = fmt.Sprintf("%d\\%s", mf.LibraryID, id) + key = fmt.Sprintf("%d\\%s", mf.LibraryID, key) } - return fmt.Sprintf("%x", md5.Sum([]byte(id))) + sum := md5.Sum([]byte(key)) + return id.Encode(sum) } func legacyAlbumID(mf model.MediaFile, md Metadata, prependLibId bool) string { @@ -33,7 +35,8 @@ func legacyAlbumID(mf model.MediaFile, md Metadata, prependLibId bool) string { if prependLibId && mf.LibraryID != model.DefaultLibraryID { albumPath = fmt.Sprintf("%d\\%s", mf.LibraryID, albumPath) } - return fmt.Sprintf("%x", md5.Sum([]byte(albumPath))) + sum := md5.Sum([]byte(albumPath)) + return id.Encode(sum) } func legacyMapAlbumArtistName(md Metadata) string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index eb66d11d1..8e38bbd42 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -218,6 +218,21 @@ var _ = Describe("getPID", func() { }) Context("legacy specs", func() { + It("emits canonical 22-char base62 ids", func() { + mf := model.MediaFile{Path: "/music/a.mp3", LibraryID: 1} + // md5("/music/a.mp3") = e3b7fc2ae9447bbec37a13bf916e3cf6 re-encoded as base62 + Expect(legacyTrackID(mf, false)).To(Equal("6VHl3uR4kss6sUPKA8Cwnk")) + }) + It("prepends the library id for a non-default library", func() { + mf := model.MediaFile{Path: "/music/a.mp3", LibraryID: 2} + // id.Encode(md5.Sum([]byte("2\\/music/a.mp3"))) + Expect(legacyTrackID(mf, true)).To(Equal("4EK5DHQBMeFuDHw6S3iooO")) + }) + It("emits a canonical album id (golden)", func() { + mf := model.MediaFile{LibraryID: 1} + // id.Encode(md5.Sum([]byte("[unknown artist]\\[unknown album]"))) + Expect(legacyAlbumID(mf, Metadata{}, false)).To(Equal("6xBmxSAUFJSQuW7UvwCq8X")) + }) Context("track_legacy", func() { When("library ID is default (1)", func() { It("should not prepend library ID even when prependLibId is true", func() { diff --git a/plugins/host_artwork_test.go b/plugins/host_artwork_test.go index ed8a0e810..218d3d892 100644 --- a/plugins/host_artwork_test.go +++ b/plugins/host_artwork_test.go @@ -225,7 +225,7 @@ func decodeArtworkURL(artworkURL string) model.ArtworkID { } // Decode JWT token - token, err := auth.TokenAuth.Decode(tokenPart) + token, err := auth.PublicTokenAuth.Decode(tokenPart) Expect(err).ToNot(HaveOccurred(), "Failed to decode JWT token") c := auth.ClaimsFromToken(token) diff --git a/server/jellyfin/README.md b/server/jellyfin/README.md index 17d2797ab..13ec79aa8 100644 --- a/server/jellyfin/README.md +++ b/server/jellyfin/README.md @@ -90,9 +90,9 @@ skipped, so it doesn't create a nameless player. Navidrome item ids are **hex-encoded at the API boundary** (`dto.EncodeID`/`DecodeID`): every id is hex-encoded on the way out and hex-decoded on the way in. This is required because some clients parse ids as radix-16 — Finamp's queue `packIds`, for instance, does `int.parse(chunk, radix:16)`, -which chokes on Navidrome's base-62 nanoids (e.g. `5QFKvMsJrd57QE2Le2dKKo`). Because a raw MD5 id -from an old migrated library is itself valid hex, correctness depends on every emit path encoding -and every receive path decoding — see `dto/ids.go`. +which chokes on Navidrome's base62 ids (e.g. `5QFKvMsJrd57QE2Le2dKKo`). Because a base62 id can +itself be valid hex, correctness depends on every emit path encoding and every receive path +decoding — see `dto/ids.go`. ## Multi-library behavior @@ -184,13 +184,13 @@ their Navidrome `ArtworkID`. Real Jellyfin item ids are GUIDs — 128-bit values, always 32 hex characters. Finamp relies on that when persisting its play queue across restarts: `packIds()` bit-packs every id into exactly 16 -bytes. Navidrome ids are longer (nanoid ids can exceed 128 bits, so they cannot be mapped into -GUIDs), which means Finamp silently stores only the first 16 characters of each id and asks for -those **truncated ids** back when restoring the queue — item lookups, then streaming, images, -favorites and playback reports for the restored tracks. +bytes. Navidrome ids are 22-character base62 strings, not 32-hex GUIDs, which means Finamp silently +stores only the first 16 characters of each id and asks for those **truncated ids** back when +restoring the queue — item lookups, then streaming, images, favorites and playback reports for the +restored tracks. This API compensates server-side (`truncated_ids.go`): a 16-character id — a length no Navidrome -id family uses — is resolved to the full id by unique-prefix lookup (an indexed range scan; +id uses — is resolved to the full id by unique-prefix lookup (an indexed range scan; ambiguity is detected and fails safe). The `/Items?ids=` batch response echoes the id **as requested**, because Finamp matches restored items back to its stored ids, and the other item endpoints accept truncated ids transparently. diff --git a/server/jellyfin/dto/ids.go b/server/jellyfin/dto/ids.go index 3490ba260..2ac7e2a79 100644 --- a/server/jellyfin/dto/ids.go +++ b/server/jellyfin/dto/ids.go @@ -3,7 +3,7 @@ package dto import "encoding/hex" // EncodeID renders a Navidrome id as lowercase hex; Jellyfin clients parse ids as radix-16 (e.g. -// Finamp's queue packing) and crash on Navidrome's base62 nanoids if emitted as-is. +// Finamp's queue packing) and crash on Navidrome's base62 ids if emitted as-is. func EncodeID(id string) string { if id == "" { return "" diff --git a/server/jellyfin/dto/ids_test.go b/server/jellyfin/dto/ids_test.go index 26a957604..d85786d99 100644 --- a/server/jellyfin/dto/ids_test.go +++ b/server/jellyfin/dto/ids_test.go @@ -6,7 +6,7 @@ import ( ) var _ = Describe("id codec", func() { - It("round-trips a base62 nanoid through Encode/Decode", func() { + It("round-trips a base62 id", func() { id := "5QFKvMsJrd57QE2Le2dKKo" Expect(DecodeID(EncodeID(id))).To(Equal(id)) }) diff --git a/server/jellyfin/truncated_ids.go b/server/jellyfin/truncated_ids.go index b58296391..bb3e4ce9b 100644 --- a/server/jellyfin/truncated_ids.go +++ b/server/jellyfin/truncated_ids.go @@ -11,8 +11,8 @@ import ( ) // truncatedIDLen is what Finamp's saved-queue persistence cuts item ids to (16 bytes, assuming -// Jellyfin GUIDs). No Navidrome id family is 16 chars (nanoid=22, legacy MD5=32, playlist -// UUID=36), so the length alone identifies a truncated id. See README. +// Jellyfin GUIDs). All Navidrome ids are 22 chars (share ids 10), so length alone flags a +// truncated id. See README. // // Handlers taking an item id resolve it via resolveItemID/resolveItemIDs; playlist-write handlers // and ParentId scoping don't (a restored queue never edits playlists or browses by container id). diff --git a/server/public/handle_images.go b/server/public/handle_images.go index 50f9238e5..633665e0f 100644 --- a/server/public/handle_images.go +++ b/server/public/handle_images.go @@ -68,7 +68,7 @@ func (pub *Router) handleImages(w http.ResponseWriter, r *http.Request) { } func decodeArtworkID(tokenString string) (model.ArtworkID, error) { - token, err := auth.TokenAuth.Decode(tokenString) + token, err := auth.PublicTokenAuth.Decode(tokenString) if err != nil { return model.ArtworkID{}, err } diff --git a/server/public/handle_images_test.go b/server/public/handle_images_test.go index 6895241f6..669f879a6 100644 --- a/server/public/handle_images_test.go +++ b/server/public/handle_images_test.go @@ -9,7 +9,7 @@ import ( var _ = Describe("decodeArtworkID", func() { BeforeEach(func() { - auth.TokenAuth = jwtauth.New("HS256", []byte("super secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("super secret"), nil) }) It("fails to decode an invalid token", func() { diff --git a/server/public/handle_shares.go b/server/public/handle_shares.go index 76f674483..13a7e4c32 100644 --- a/server/public/handle_shares.go +++ b/server/public/handle_shares.go @@ -111,8 +111,8 @@ func (pub *Router) mapShareToM3U(r *http.Request, s model.Share) *model.Share { // admin flag) and grants access to nothing beyond the share it belongs to; the // stream handler still verifies the share exists, is unexpired, and that the // track is actually a member of it. An attacker who can forge these tokens -// necessarily already holds the signing secret, which also signs real user -// sessions, so that scenario is out of scope for the share boundary specifically. +// necessarily already holds the public-link signing secret, a full-server +// compromise that is out of scope for the share boundary specifically. func encodeMediafileShare(s model.Share, id string) string { claims := auth.Claims{ ID: id, diff --git a/server/public/handle_streams.go b/server/public/handle_streams.go index 15abab693..3d624f661 100644 --- a/server/public/handle_streams.go +++ b/server/public/handle_streams.go @@ -107,7 +107,7 @@ func shareContainsTrack(share *model.Share, mediaFileID string) bool { // public-share capability, not an auth credential; see encodeMediafileShare for // why a JWT is used here. func decodeStreamInfo(tokenString string) (shareTrackInfo, error) { - c, err := auth.Validate(tokenString) + c, err := auth.ValidatePublic(tokenString) if err != nil { return shareTrackInfo{}, err } diff --git a/server/public/handle_streams_test.go b/server/public/handle_streams_test.go index 2f32ea6f2..870dfa8ef 100644 --- a/server/public/handle_streams_test.go +++ b/server/public/handle_streams_test.go @@ -29,7 +29,7 @@ func (m *mockStreamer) NewStream(_ context.Context, _ *model.MediaFile, r stream var _ = Describe("decodeStreamInfo", func() { BeforeEach(func() { - auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) }) It("decodes a valid token with all fields", func() { @@ -81,7 +81,7 @@ var _ = Describe("decodeStreamInfo", func() { var _ = Describe("encodeMediafileShare", func() { BeforeEach(func() { - auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) }) It("includes the share ID in the token", func() { @@ -113,7 +113,7 @@ var _ = Describe("handleStream", func() { var pub *Router BeforeEach(func() { - auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil) ds = &tests.MockDataStore{} shareRepo = &tests.MockShareRepo{} ds.MockedShare = shareRepo diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 3741462a2..8db624649 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -22,6 +22,7 @@ var _ = Describe("helpers", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) auth.TokenAuth = jwtauth.New("HS256", []byte("test secret"), nil) + auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test public secret"), nil) }) Describe("fakePath", func() {