refactor(jellyfin): share resolveLibraryScope between items and artists

This commit is contained in:
Deluan 2026-07-05 16:16:26 -04:00
commit 30d3efebb9
4 changed files with 43 additions and 23 deletions

View file

@ -2,10 +2,8 @@ package jellyfin
import (
"net/http"
"strconv"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/utils/req"
)
@ -18,14 +16,7 @@ func (api *Router) getArtists(w http.ResponseWriter, r *http.Request) {
opts := model.QueryOptions{Offset: p.IntOr("StartIndex", 0), Max: p.IntOr("Limit", 0)}
applySort(&opts, "MusicArtist", p.StringOr("SortBy", ""), p.StringOr("SortOrder", ""))
scopeIDs := accessibleLibraryIDs(ctx)
if parentId := p.StringOr("ParentId", ""); parentId != "" {
if libID, err := strconv.Atoi(parentId); err == nil {
if u, _ := request.UserFrom(ctx); u.HasLibraryAccess(libID) {
scopeIDs = []int{libID}
}
}
}
scopeIDs, _ := resolveLibraryScope(ctx, p.StringOr("ParentId", ""))
res, err := api.listArtists(ctx, opts, scopeIDs, p.StringOr("SearchTerm", ""), false)
if err != nil {

View file

@ -21,6 +21,11 @@ var _ = Describe("Browsing", func() {
return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: libs})
}
// admin has no explicit Libraries; access is granted via the IsAdmin bypass, not membership.
ctxAdmin := func() context.Context {
return request.WithUser(context.Background(), model.User{ID: "admin", IsAdmin: true, Libraries: nil})
}
BeforeEach(func() {
ds = &tests.MockDataStore{}
api = &Router{ds: ds}
@ -116,6 +121,23 @@ var _ = Describe("Browsing", func() {
Expect(artistRepo.Options.Offset).To(Equal(5))
Expect(artistRepo.Options.Max).To(Equal(10))
})
It("does not restrict results for an admin user", func() {
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Artists", nil).WithContext(ctxAdmin())
api.getArtists(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
// accessibleLibraryIDs is empty for an admin (Libraries is nil), so
// ApplyArtistLibraryFilter([]) is a no-op: no library_id restriction is added.
if artistRepo.Options.Filters == nil {
return
}
sql, _, err := artistRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).NotTo(ContainSubstring("library_artist.library_id"))
})
})
Describe("getGenres", func() {

View file

@ -3,7 +3,6 @@ package jellyfin
import (
"context"
"net/http"
"strconv"
"strings"
"github.com/Masterminds/squirrel"
@ -42,19 +41,10 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryRe
opts := model.QueryOptions{Offset: p.IntOr("StartIndex", 0), Max: p.IntOr("Limit", 0)}
applySort(&opts, itemType, p.StringOr("SortBy", ""), p.StringOr("SortOrder", ""))
// ParentId is ambiguous: it can be a library id (browsing a UserView) or an entity id (an
// artist for MusicAlbum queries, an album for Audio queries). It's only treated as a library
// when the user actually has access to it; otherwise it falls through as an entity id, which
// safely matches nothing rather than leaking another library's content.
scopeIDs := accessibleLibraryIDs(ctx)
scopeIDs, isLibraryParent := resolveLibraryScope(ctx, parentId)
entityParent := parentId
if parentId != "" {
if libID, err := strconv.Atoi(parentId); err == nil {
if u, _ := request.UserFrom(ctx); u.HasLibraryAccess(libID) {
scopeIDs = []int{libID}
entityParent = ""
}
}
if isLibraryParent {
entityParent = ""
}
switch itemType {

View file

@ -2,6 +2,7 @@ package jellyfin
import (
"context"
"strconv"
"github.com/navidrome/navidrome/model/request"
)
@ -13,3 +14,19 @@ func accessibleLibraryIDs(ctx context.Context) []int {
u, _ := request.UserFrom(ctx)
return u.Libraries.IDs()
}
// resolveLibraryScope handles ParentId's ambiguity: it can be a library id (browsing a
// UserView) or an entity id (an artist for MusicAlbum queries, an album for Audio queries).
// It's only treated as a library when the user actually has access to it; otherwise
// isLibraryParent is false and callers should fall through to treating parentId as an entity
// id, which safely matches nothing rather than leaking another library's content.
func resolveLibraryScope(ctx context.Context, parentId string) (scopeIDs []int, isLibraryParent bool) {
if parentId != "" {
if id, err := strconv.Atoi(parentId); err == nil {
if u, _ := request.UserFrom(ctx); u.HasLibraryAccess(id) {
return []int{id}, true
}
}
}
return accessibleLibraryIDs(ctx), false
}