mirror of
https://github.com/navidrome/navidrome.git
synced 2025-08-13 14:01:14 +03:00
* fix: enable library access for headless processes Fixed multi-library filtering to allow headless processes (shares, external providers) to access data by skipping library restrictions when no user context is present. Previously, the library filtering system returned empty results (WHERE 1=0) for processes without user authentication, breaking functionality like public shares and external service integrations. Key changes: - Modified applyLibraryFilter methods to skip filtering when user.ID == invalidUserId - Refactored tag repository to use helper method for library filtering logic - Fixed SQL aggregation bug in tag statistics calculation across multiple libraries - Added comprehensive test coverage for headless process scenarios - Updated genre repository to support proper column mappings for aggregated data This preserves the secure "safe by default" approach for authenticated users while restoring backward compatibility for background processes that need unrestricted data access. Signed-off-by: Deluan <deluan@navidrome.org> * fix: resolve SQL ambiguity errors in share queries Fixed SQL ambiguity errors that were breaking share links after the Multi-library PR. The Multi-library changes introduced JOINs between album and library tables, both of which have 'id' columns, causing 'ambiguous column name: id' errors when unqualified column references were used in WHERE clauses. Changes made: - Updated core/share.go to use 'album.id' instead of 'id' in contentsLabelFromAlbums - Updated persistence/share_repository.go to use 'album.id' in album share loading - Updated persistence/sql_participations.go to use 'artist.id' for consistency - Added regression tests to prevent future SQL ambiguity issues This resolves HTTP 500 errors that users experienced when accessing existing share URLs after the Multi-library feature was merged. Signed-off-by: Deluan <deluan@navidrome.org> * fix: improve headless library access handling Added proper user context validation and reordered joins in applyLibraryFilterToArtistQuery to ensure library filtering works correctly for both authenticated and headless operations. The user_library join is now only applied when a valid user context exists, while the library_artist join is always applied to maintain proper data relationships. (+1 squashed commit) Squashed commits: [a28c6965b] fix: remove headless library access guard Removed the invalidUserId guard condition in applyLibraryFilterToArtistQuery that was preventing proper library filtering for headless operations. This fix ensures that library filtering joins are always applied consistently, allowing headless library access to work correctly with the library_artist junction table filtering. The previous guard was skipping all library filtering when no user context was present, which could cause issues with headless operations that still need to respect library boundaries through the library_artist relationship. * fix: simplify genre selection query in genre repository Signed-off-by: Deluan <deluan@navidrome.org> * fix: enhance tag library filtering tests for headless access Signed-off-by: Deluan <deluan@navidrome.org> * test: add comprehensive test coverage for headless library access Added extensive test coverage for headless library access improvements including: - Added 17 new tests across 4 test files covering headless access scenarios - artist_repository_test.go: Added headless process tests for GetAll, Count, Get operations and explicit library_id filtering functionality - genre_repository_test.go: Added library filtering tests for headless processes including GetAll, Count, ReadAll, and Read operations - sql_base_repository_test.go: Added applyLibraryFilter method tests covering admin users, regular users, and headless processes with/without custom table names - share_repository_test.go: Added headless access tests and SQL ambiguity verification for the album.id vs id fix in loadMedia function - Cleaned up test setup by replacing log.NewContext usage with GinkgoT().Context() and removing unnecessary configtest.SetupConfig() calls for better test isolation These tests ensure that headless processes (background operations without user context) can access all libraries while respecting explicit filters, and verify that the SQL ambiguity fixes work correctly without breaking existing functionality. * revert: remove user context handling from scrobble buffer getParticipants Reverts commit 5b8ef74f05109ecf30ddfc936361b84314522869. The artist repository no longer requires user context for proper library filtering, so the workaround of temporarily injecting user context into the scrobbleBufferRepository.Next method is no longer needed. This simplifies the code and removes the dependency on fetching user information during background scrobbling operations. * fix: improve library access filtering for artists Enhanced artist repository filtering to properly handle library access restrictions and prevent artists with no accessible content from appearing in results. Backend changes: - Modified roleFilter to use direct JSON_EXTRACT instead of EXISTS subquery for better performance - Enhanced applyLibraryFilterToArtistQuery to filter out artists with empty stats (no content) - Changed from LEFT JOIN to INNER JOIN with library_artist table for stricter filtering - Added condition to exclude artists where library_artist.stats = '{}' (empty content) Frontend changes: - Added null-checking in getCounter function to prevent TypeError when accessing undefined records - Improved optional chaining for safer property access in role-based statistics display These changes ensure that users only see artists that have actual accessible content in their permitted libraries, fixing issues where artists appeared in the list despite having no albums or songs available to the user. * fix: update library access logic for non-admin users and enhance test coverage Signed-off-by: Deluan <deluan@navidrome.org> * fix: refine library artist query and implement cleanup for empty entries Signed-off-by: Deluan <deluan@navidrome.org> * refactor: consolidate artist repository tests to eliminate duplication Significantly refactored artist_repository_test.go to reduce code duplication and improve maintainability by ~27% (930 to 680 lines). Key improvements include: - Added test helper functions createTestArtistWithMBID() and createUserWithLibraries() to eliminate repetitive test data creation - Consolidated duplicate MBID search tests using DescribeTable for parameterized testing - Removed entire 'Permission-Based Behavior Comparison' section (~150 lines) that duplicated functionality already covered in other test contexts - Reorganized search tests into cohesive 'MBID and Text Search' section with proper setup/teardown and shared test infrastructure - Streamlined missing artist tests and moved them to dedicated section - Maintained 100% test coverage while eliminating redundant test patterns All tests continue to pass with identical functionality and coverage. --------- Signed-off-by: Deluan <deluan@navidrome.org>
285 lines
8.7 KiB
Go
285 lines
8.7 KiB
Go
package persistence
|
|
|
|
import (
|
|
"context"
|
|
|
|
"github.com/Masterminds/squirrel"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/model/request"
|
|
"github.com/navidrome/navidrome/utils/hasher"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("sqlRepository", func() {
|
|
var r sqlRepository
|
|
BeforeEach(func() {
|
|
r.ctx = request.WithUser(context.Background(), model.User{ID: "user-id"})
|
|
r.tableName = "table"
|
|
})
|
|
|
|
Describe("applyOptions", func() {
|
|
var sq squirrel.SelectBuilder
|
|
BeforeEach(func() {
|
|
sq = squirrel.Select("*").From("test")
|
|
r.sortMappings = map[string]string{
|
|
"name": "title",
|
|
}
|
|
})
|
|
It("does not add any clauses when options is empty", func() {
|
|
sq = r.applyOptions(sq, model.QueryOptions{})
|
|
sql, _, _ := sq.ToSql()
|
|
Expect(sql).To(Equal("SELECT * FROM test"))
|
|
})
|
|
It("adds all option clauses", func() {
|
|
sq = r.applyOptions(sq, model.QueryOptions{
|
|
Sort: "name",
|
|
Order: "desc",
|
|
Max: 1,
|
|
Offset: 2,
|
|
})
|
|
sql, _, _ := sq.ToSql()
|
|
Expect(sql).To(Equal("SELECT * FROM test ORDER BY title desc LIMIT 1 OFFSET 2"))
|
|
})
|
|
})
|
|
|
|
Describe("toSQL", func() {
|
|
It("returns error for invalid SQL", func() {
|
|
sq := squirrel.Select("*").From("test").Where(1)
|
|
_, _, err := r.toSQL(sq)
|
|
Expect(err).To(HaveOccurred())
|
|
})
|
|
|
|
It("returns the same query when there are no placeholders", func() {
|
|
sq := squirrel.Select("*").From("test")
|
|
query, params, err := r.toSQL(sq)
|
|
Expect(err).NotTo(HaveOccurred())
|
|
Expect(query).To(Equal("SELECT * FROM test"))
|
|
Expect(params).To(BeEmpty())
|
|
})
|
|
|
|
It("replaces one placeholder correctly", func() {
|
|
sq := squirrel.Select("*").From("test").Where(squirrel.Eq{"id": 1})
|
|
query, params, err := r.toSQL(sq)
|
|
Expect(err).NotTo(HaveOccurred())
|
|
Expect(query).To(Equal("SELECT * FROM test WHERE id = {:p0}"))
|
|
Expect(params).To(HaveKeyWithValue("p0", 1))
|
|
})
|
|
|
|
It("replaces multiple placeholders correctly", func() {
|
|
sq := squirrel.Select("*").From("test").Where(squirrel.Eq{"id": 1, "name": "test"})
|
|
query, params, err := r.toSQL(sq)
|
|
Expect(err).NotTo(HaveOccurred())
|
|
Expect(query).To(Equal("SELECT * FROM test WHERE id = {:p0} AND name = {:p1}"))
|
|
Expect(params).To(HaveKeyWithValue("p0", 1))
|
|
Expect(params).To(HaveKeyWithValue("p1", "test"))
|
|
})
|
|
})
|
|
|
|
Describe("sanitizeSort", func() {
|
|
BeforeEach(func() {
|
|
r.registerModel(&struct {
|
|
Field string `structs:"field"`
|
|
}{}, nil)
|
|
r.sortMappings = map[string]string{
|
|
"sort1": "mappedSort1",
|
|
}
|
|
})
|
|
|
|
When("sanitizing sort", func() {
|
|
It("returns empty if the sort key is not found in the model nor in the mappings", func() {
|
|
sort, _ := r.sanitizeSort("unknown", "")
|
|
Expect(sort).To(BeEmpty())
|
|
})
|
|
|
|
It("returns the mapped value when sort key exists", func() {
|
|
sort, _ := r.sanitizeSort("sort1", "")
|
|
Expect(sort).To(Equal("mappedSort1"))
|
|
})
|
|
|
|
It("is case insensitive", func() {
|
|
sort, _ := r.sanitizeSort("Sort1", "")
|
|
Expect(sort).To(Equal("mappedSort1"))
|
|
})
|
|
|
|
It("returns the field if it is a valid field", func() {
|
|
sort, _ := r.sanitizeSort("field", "")
|
|
Expect(sort).To(Equal("field"))
|
|
})
|
|
|
|
It("is case insensitive for fields", func() {
|
|
sort, _ := r.sanitizeSort("FIELD", "")
|
|
Expect(sort).To(Equal("field"))
|
|
})
|
|
})
|
|
When("sanitizing order", func() {
|
|
It("returns 'asc' if order is empty", func() {
|
|
_, order := r.sanitizeSort("", "")
|
|
Expect(order).To(Equal(""))
|
|
})
|
|
|
|
It("returns 'asc' if order is 'asc'", func() {
|
|
_, order := r.sanitizeSort("", "ASC")
|
|
Expect(order).To(Equal("asc"))
|
|
})
|
|
|
|
It("returns 'desc' if order is 'desc'", func() {
|
|
_, order := r.sanitizeSort("", "desc")
|
|
Expect(order).To(Equal("desc"))
|
|
})
|
|
|
|
It("returns 'asc' if order is unknown", func() {
|
|
_, order := r.sanitizeSort("", "something")
|
|
Expect(order).To(Equal("asc"))
|
|
})
|
|
})
|
|
})
|
|
|
|
Describe("buildSortOrder", func() {
|
|
BeforeEach(func() {
|
|
r.sortMappings = map[string]string{}
|
|
})
|
|
|
|
Context("single field", func() {
|
|
It("sorts by specified field", func() {
|
|
sql := r.buildSortOrder("name", "desc")
|
|
Expect(sql).To(Equal("name desc"))
|
|
})
|
|
It("defaults to 'asc'", func() {
|
|
sql := r.buildSortOrder("name", "")
|
|
Expect(sql).To(Equal("name asc"))
|
|
})
|
|
It("inverts pre-defined order", func() {
|
|
sql := r.buildSortOrder("name desc", "desc")
|
|
Expect(sql).To(Equal("name asc"))
|
|
})
|
|
It("forces snake case for field names", func() {
|
|
sql := r.buildSortOrder("AlbumArtist", "asc")
|
|
Expect(sql).To(Equal("album_artist asc"))
|
|
})
|
|
})
|
|
Context("multiple fields", func() {
|
|
It("handles multiple fields", func() {
|
|
sql := r.buildSortOrder("name desc,age asc, status desc ", "asc")
|
|
Expect(sql).To(Equal("name desc, age asc, status desc"))
|
|
})
|
|
It("inverts multiple fields", func() {
|
|
sql := r.buildSortOrder("name desc, age, status asc", "desc")
|
|
Expect(sql).To(Equal("name asc, age desc, status desc"))
|
|
})
|
|
It("handles spaces in mapped field", func() {
|
|
r.sortMappings = map[string]string{
|
|
"has_lyrics": "(lyrics != '[]'), updated_at",
|
|
}
|
|
sql := r.buildSortOrder("has_lyrics", "desc")
|
|
Expect(sql).To(Equal("(lyrics != '[]') desc, updated_at desc"))
|
|
})
|
|
|
|
})
|
|
Context("function fields", func() {
|
|
It("handles functions with multiple params", func() {
|
|
sql := r.buildSortOrder("substr(id, 7)", "asc")
|
|
Expect(sql).To(Equal("substr(id, 7) asc"))
|
|
})
|
|
It("handles functions with multiple params mixed with multiple fields", func() {
|
|
sql := r.buildSortOrder("name desc, substr(id, 7), status asc", "desc")
|
|
Expect(sql).To(Equal("name asc, substr(id, 7) desc, status desc"))
|
|
})
|
|
It("handles nested functions", func() {
|
|
sql := r.buildSortOrder("name desc, coalesce(nullif(release_date, ''), nullif(original_date, '')), status asc", "desc")
|
|
Expect(sql).To(Equal("name asc, coalesce(nullif(release_date, ''), nullif(original_date, '')) desc, status desc"))
|
|
})
|
|
})
|
|
})
|
|
|
|
Describe("resetSeededRandom", func() {
|
|
var id string
|
|
BeforeEach(func() {
|
|
id = r.seedKey()
|
|
hasher.SetSeed(id, "")
|
|
})
|
|
It("does not reset seed if sort is not random", func() {
|
|
var options []model.QueryOptions
|
|
r.resetSeededRandom(options)
|
|
Expect(hasher.CurrentSeed(id)).To(BeEmpty())
|
|
})
|
|
It("resets seed if sort is random", func() {
|
|
options := []model.QueryOptions{{Sort: "random"}}
|
|
r.resetSeededRandom(options)
|
|
Expect(hasher.CurrentSeed(id)).NotTo(BeEmpty())
|
|
})
|
|
It("resets seed if sort is random and seed is provided", func() {
|
|
options := []model.QueryOptions{{Sort: "random", Seed: "seed"}}
|
|
r.resetSeededRandom(options)
|
|
Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
|
|
})
|
|
It("keeps seed when paginating", func() {
|
|
options := []model.QueryOptions{{Sort: "random", Seed: "seed", Offset: 0}}
|
|
r.resetSeededRandom(options)
|
|
Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
|
|
|
|
options = []model.QueryOptions{{Sort: "random", Offset: 1}}
|
|
r.resetSeededRandom(options)
|
|
Expect(hasher.CurrentSeed(id)).To(Equal("seed"))
|
|
})
|
|
})
|
|
|
|
Describe("applyLibraryFilter", func() {
|
|
var sq squirrel.SelectBuilder
|
|
|
|
BeforeEach(func() {
|
|
sq = squirrel.Select("*").From("test_table")
|
|
})
|
|
|
|
Context("Admin User", func() {
|
|
BeforeEach(func() {
|
|
r.ctx = request.WithUser(context.Background(), model.User{ID: "admin", IsAdmin: true})
|
|
})
|
|
|
|
It("should not apply library filter for admin users", func() {
|
|
result := r.applyLibraryFilter(sq)
|
|
sql, _, _ := result.ToSql()
|
|
Expect(sql).To(Equal("SELECT * FROM test_table"))
|
|
})
|
|
})
|
|
|
|
Context("Regular User", func() {
|
|
BeforeEach(func() {
|
|
r.ctx = request.WithUser(context.Background(), model.User{ID: "user123", IsAdmin: false})
|
|
})
|
|
|
|
It("should apply library filter for regular users", func() {
|
|
result := r.applyLibraryFilter(sq)
|
|
sql, args, _ := result.ToSql()
|
|
Expect(sql).To(ContainSubstring("IN (SELECT ul.library_id FROM user_library ul WHERE ul.user_id = ?)"))
|
|
Expect(args).To(ContainElement("user123"))
|
|
})
|
|
|
|
It("should use custom table name when provided", func() {
|
|
result := r.applyLibraryFilter(sq, "custom_table")
|
|
sql, args, _ := result.ToSql()
|
|
Expect(sql).To(ContainSubstring("custom_table.library_id IN"))
|
|
Expect(args).To(ContainElement("user123"))
|
|
})
|
|
})
|
|
|
|
Context("Headless Process (No User Context)", func() {
|
|
BeforeEach(func() {
|
|
r.ctx = context.Background() // No user context
|
|
})
|
|
|
|
It("should not apply library filter for headless processes", func() {
|
|
result := r.applyLibraryFilter(sq)
|
|
sql, _, _ := result.ToSql()
|
|
Expect(sql).To(Equal("SELECT * FROM test_table"))
|
|
})
|
|
|
|
It("should not apply library filter even with custom table name", func() {
|
|
result := r.applyLibraryFilter(sq, "custom_table")
|
|
sql, _, _ := result.ToSql()
|
|
Expect(sql).To(Equal("SELECT * FROM test_table"))
|
|
})
|
|
})
|
|
})
|
|
})
|