diff --git a/server/jellyfin/items.go b/server/jellyfin/items.go index 68f0c95ce..1bb28ae23 100644 --- a/server/jellyfin/items.go +++ b/server/jellyfin/items.go @@ -3,11 +3,13 @@ package jellyfin import ( "context" "net/http" + "strconv" "strings" "github.com/Masterminds/squirrel" "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/filter" "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/utils/req" @@ -40,15 +42,30 @@ 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) + entityParent := parentId + if parentId != "" { + if libID, err := strconv.Atoi(parentId); err == nil { + if u, _ := request.UserFrom(ctx); u.HasLibraryAccess(libID) { + scopeIDs = []int{libID} + entityParent = "" + } + } + } + switch itemType { case "Audio": - return api.listSongs(ctx, opts, parentId, search, favOnly) + return api.listSongs(ctx, opts, entityParent, scopeIDs, search, favOnly) case "MusicArtist": - return api.listArtists(ctx, opts, search, favOnly) + return api.listArtists(ctx, opts, scopeIDs, search, favOnly) case "MusicGenre": return api.listGenres(ctx, opts) default: // MusicAlbum - return api.listAlbums(ctx, opts, parentId, search, favOnly) + return api.listAlbums(ctx, opts, entityParent, scopeIDs, search, favOnly) } } @@ -68,10 +85,10 @@ func firstType(types string) string { return "MusicAlbum" } -func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, parentId, search string, fav bool) (dto.QueryResult, error) { +func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, parentId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) { repo := api.ds.Album(ctx) filters := squirrel.And{} - if parentId != "" && parentId != musicViewID { + if parentId != "" { filters = append(filters, filter.AlbumsByArtistID(parentId).Filters) } else { filters = append(filters, notMissing) @@ -80,6 +97,7 @@ func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, pare filters = append(filters, filter.ByStarred().Filters) } opts.Filters = filters + opts = filter.ApplyLibraryFilter(opts, scopeIDs) var albums model.Albums var err error @@ -95,10 +113,10 @@ func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, pare return result(slice.Map(albums, dto.AlbumToBaseItem), int(total), opts.Offset), nil } -func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, parentId, search string, fav bool) (dto.QueryResult, error) { +func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, parentId string, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) { repo := api.ds.MediaFile(ctx) filters := squirrel.And{} - if parentId != "" && parentId != musicViewID { + if parentId != "" { filters = append(filters, filter.SongsByAlbum(parentId).Filters) } else { filters = append(filters, notMissing) @@ -107,6 +125,7 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, paren filters = append(filters, filter.ByStarred().Filters) } opts.Filters = filters + opts = filter.ApplyLibraryFilter(opts, scopeIDs) var mfs model.MediaFiles var err error @@ -122,13 +141,14 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, paren return result(slice.Map(mfs, dto.SongToBaseItem), int(total), opts.Offset), nil } -func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, search string, fav bool) (dto.QueryResult, error) { +func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) { repo := api.ds.Artist(ctx) if fav { opts.Filters = filter.ArtistsByStarred().Filters } else { opts.Filters = notMissing } + opts = filter.ApplyArtistLibraryFilter(opts, scopeIDs) var artists model.Artists var err error @@ -144,6 +164,8 @@ func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, sea return result(slice.Map(artists, dto.ArtistToBaseItem), int(total), opts.Offset), nil } +// listGenres is intentionally unscoped: genres are global tags derived from track metadata, +// not entities that belong to a single library. func (api *Router) listGenres(ctx context.Context, opts model.QueryOptions) (dto.QueryResult, error) { genres, err := api.ds.Genre(ctx).GetAll(opts) if err != nil { @@ -152,18 +174,33 @@ func (api *Router) listGenres(ctx context.Context, opts model.QueryOptions) (dto return result(slice.Map(genres, dto.GenreToBaseItem), len(genres), opts.Offset), nil } +// getItem fetches a single entity by id, trying album, artist and song in turn. For albums +// and songs (which each belong to exactly one library) it 404s if the current user lacks +// access to that library, so an id can't be used to probe content outside the user's libraries. func (api *Router) getItem(w http.ResponseWriter, r *http.Request) { ctx := r.Context() id := chi.URLParam(r, "itemId") + u, _ := request.UserFrom(ctx) if al, err := api.ds.Album(ctx).Get(id); err == nil { + if !u.HasLibraryAccess(al.LibraryID) { + http.Error(w, "Not Found", http.StatusNotFound) + return + } api.ok(w, r, dto.AlbumToBaseItem(*al)) return } if ar, err := api.ds.Artist(ctx).Get(id); err == nil { + // TODO: an artist can have content in multiple libraries (via library_artist), so + // there's no single LibraryID to check here; access control for artists relies on + // list-time scoping (listArtists) and the persistence layer's defense-in-depth. api.ok(w, r, dto.ArtistToBaseItem(*ar)) return } if mf, err := api.ds.MediaFile(ctx).Get(id); err == nil { + if !u.HasLibraryAccess(mf.LibraryID) { + http.Error(w, "Not Found", http.StatusNotFound) + return + } api.ok(w, r, dto.SongToBaseItem(*mf)) return } @@ -174,6 +211,7 @@ func (api *Router) getLatest(w http.ResponseWriter, r *http.Request) { ctx := r.Context() opts := filter.AlbumsByNewest() opts.Max = req.Params(r).IntOr("Limit", 20) + opts = filter.ApplyLibraryFilter(opts, accessibleLibraryIDs(ctx)) albums, err := api.ds.Album(ctx).GetAll(opts) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 8e0e5e110..fbf1b0110 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -26,8 +26,12 @@ func withChiURLParam(r *http.Request, key, value string) *http.Request { var _ = Describe("Items", func() { var api *Router var ds *tests.MockDataStore + // alice has access to library 1 only; used by tests that don't care about scoping. ctxUser := func() context.Context { - return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice"}) + return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) + } + ctxUserWithLibraries := func(libs model.Libraries) context.Context { + return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: libs}) } BeforeEach(func() { ds = &tests.MockDataStore{} @@ -130,11 +134,88 @@ var _ = Describe("Items", func() { Expect(albumRepo.Options.Offset).To(Equal(5)) Expect(albumRepo.Options.Max).To(Equal(10)) }) + + Describe("library scoping", func() { + It("scopes a MusicAlbum listing (no ParentId) to the user's accessible libraries", func() { + albumRepo := ds.Album(nil).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}, {ID: 2}} + r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) + api.getItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := albumRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + Expect(sql).To(ContainSubstring("library_id")) + Expect(args).To(ContainElements(1, 2)) + }) + + It("scopes a Audio listing (no ParentId) to the user's accessible libraries", func() { + mfRepo := ds.MediaFile(nil).(*tests.MockMediaFileRepo) + mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song"}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}, {ID: 2}} + r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio", nil).WithContext(ctxUserWithLibraries(libs)) + api.getItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := mfRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + Expect(sql).To(ContainSubstring("library_id")) + Expect(args).To(ContainElements(1, 2)) + }) + + It("scopes a MusicArtist listing to the user's accessible libraries", func() { + artistRepo := ds.Artist(nil).(*tests.MockArtistRepo) + artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}, {ID: 2}} + r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicArtist", nil).WithContext(ctxUserWithLibraries(libs)) + api.getItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := artistRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + Expect(sql).To(ContainSubstring("library_artist.library_id")) + Expect(args).To(ContainElements(1, 2)) + }) + + It("treats a numeric ParentId matching an accessible library as a library scope, not an artist id", func() { + albumRepo := ds.Album(nil).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}, {ID: 2}} + r := httptest.NewRequest("GET", "/Items?ParentId=2&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) + api.getItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := albumRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + Expect(sql).NotTo(ContainSubstring("json_tree")) // not treated as an artist-parent filter + Expect(sql).To(ContainSubstring("library_id")) + Expect(args).To(ContainElement(2)) + }) + + It("does not let ParentId= scope results to that library", func() { + albumRepo := ds.Album(nil).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}} // no access to library 99 + r := httptest.NewRequest("GET", "/Items?ParentId=99&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) + api.getItems(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := albumRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + // Falls back to treating "99" as an (empty-matching) artist-parent id... + Expect(sql).To(ContainSubstring("json_tree")) + // ...while still scoping to the user's own accessible libraries. + Expect(sql).To(ContainSubstring("library_id")) + Expect(args).To(ContainElement(1)) + Expect(args).NotTo(ContainElement(99)) + }) + }) }) Describe("getItem", func() { It("returns an album by id", func() { - ds.Album(nil).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}}) + ds.Album(nil).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/a1", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "a1") @@ -153,11 +234,29 @@ var _ = Describe("Items", func() { api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) + + It("returns 404 for an album in a library the user can't access", func() { + ds.Album(nil).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Items/a1", nil).WithContext(ctxUser()) // only has access to library 1 + r = withChiURLParam(r, "itemId", "a1") + api.getItem(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + + It("returns 404 for a song in a library the user can't access", func() { + ds.MediaFile(nil).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 2}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Items/s1", nil).WithContext(ctxUser()) // only has access to library 1 + r = withChiURLParam(r, "itemId", "s1") + api.getItem(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) }) Describe("getLatest", func() { It("returns a bare array of the newest albums", func() { - ds.Album(nil).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}}) + ds.Album(nil).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Users/u1/Items/Latest", nil).WithContext(ctxUser()) api.getLatest(w, r) @@ -167,5 +266,19 @@ var _ = Describe("Items", func() { Expect(items).To(HaveLen(1)) Expect(items[0].Id).To(Equal("a1")) }) + + It("scopes to the user's accessible libraries", func() { + albumRepo := ds.Album(nil).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) + w := httptest.NewRecorder() + libs := model.Libraries{{ID: 1}, {ID: 2}} + r := httptest.NewRequest("GET", "/Users/u1/Items/Latest", nil).WithContext(ctxUserWithLibraries(libs)) + api.getLatest(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + sql, args, err := albumRepo.Options.Filters.ToSql() + Expect(err).NotTo(HaveOccurred()) + Expect(sql).To(ContainSubstring("library_id")) + Expect(args).To(ContainElements(1, 2)) + }) }) }) diff --git a/server/jellyfin/library.go b/server/jellyfin/library.go new file mode 100644 index 000000000..a538d674a --- /dev/null +++ b/server/jellyfin/library.go @@ -0,0 +1,15 @@ +package jellyfin + +import ( + "context" + + "github.com/navidrome/navidrome/model/request" +) + +// accessibleLibraryIDs returns the ids of the libraries the current user can access. An +// empty result means "no restriction" (e.g. admins), which matches the no-op semantics of +// filter.ApplyLibraryFilter/ApplyArtistLibraryFilter for an empty id list. +func accessibleLibraryIDs(ctx context.Context) []int { + u, _ := request.UserFrom(ctx) + return u.Libraries.IDs() +} diff --git a/server/jellyfin/users.go b/server/jellyfin/users.go index e604c9c4a..80e191a51 100644 --- a/server/jellyfin/users.go +++ b/server/jellyfin/users.go @@ -2,23 +2,29 @@ package jellyfin import ( "net/http" + "strconv" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/jellyfin/dto" ) -const musicViewID = "music" - +// getUserViews returns one CollectionFolder view per library the current user can access, +// so Jellyfin clients browse each library as its own top-level view (instead of a single +// aggregate "music" view spanning every library). func (api *Router) getUserViews(w http.ResponseWriter, r *http.Request) { - view := dto.BaseItemDto{ - Name: api.serverName(), - Id: musicViewID, - Type: "CollectionFolder", - CollectionType: "music", - IsFolder: true, - BackdropImageTags: []string{}, + u, _ := request.UserFrom(r.Context()) + views := make([]dto.BaseItemDto, 0, len(u.Libraries)) + for _, lib := range u.Libraries { + views = append(views, dto.BaseItemDto{ + Id: strconv.Itoa(lib.ID), + Name: lib.Name, + Type: "CollectionFolder", + CollectionType: "music", + IsFolder: true, + BackdropImageTags: []string{}, + }) } - api.ok(w, r, dto.QueryResult{Items: []dto.BaseItemDto{view}, TotalRecordCount: 1, StartIndex: 0}) + api.ok(w, r, dto.QueryResult{Items: views, TotalRecordCount: len(views), StartIndex: 0}) } func (api *Router) getCurrentUser(w http.ResponseWriter, r *http.Request) { diff --git a/server/jellyfin/users_test.go b/server/jellyfin/users_test.go index 1252c7b86..d53cad846 100644 --- a/server/jellyfin/users_test.go +++ b/server/jellyfin/users_test.go @@ -16,26 +16,58 @@ import ( var _ = Describe("Users", func() { var api *Router - authed := func(r *http.Request) *http.Request { - ctx := request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice"}) + authedWithLibraries := func(r *http.Request, libs model.Libraries) *http.Request { + ctx := request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: libs}) return r.WithContext(ctx) } BeforeEach(func() { api = &Router{ds: &tests.MockDataStore{}} }) - It("returns a single music view", func() { - w := httptest.NewRecorder() - api.getUserViews(w, authed(httptest.NewRequest("GET", "/UserViews", nil))) - Expect(w.Code).To(Equal(http.StatusOK)) - var res dto.QueryResult - Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) - Expect(res.Items).To(HaveLen(1)) - Expect(res.Items[0].CollectionType).To(Equal("music")) - Expect(res.Items[0].Type).To(Equal("CollectionFolder")) + Describe("getUserViews", func() { + It("returns one view per accessible library", func() { + libs := model.Libraries{{ID: 1, Name: "Music"}, {ID: 2, Name: "Podcasts"}} + w := httptest.NewRecorder() + api.getUserViews(w, authedWithLibraries(httptest.NewRequest("GET", "/UserViews", nil), libs)) + Expect(w.Code).To(Equal(http.StatusOK)) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res.Items).To(HaveLen(2)) + Expect(res.TotalRecordCount).To(Equal(2)) + + Expect(res.Items[0].Id).To(Equal("1")) + Expect(res.Items[0].Name).To(Equal("Music")) + Expect(res.Items[0].Type).To(Equal("CollectionFolder")) + Expect(res.Items[0].CollectionType).To(Equal("music")) + Expect(res.Items[0].IsFolder).To(BeTrue()) + + Expect(res.Items[1].Id).To(Equal("2")) + Expect(res.Items[1].Name).To(Equal("Podcasts")) + }) + + It("returns a single view for a user with one library", func() { + libs := model.Libraries{{ID: 1, Name: "Music"}} + w := httptest.NewRecorder() + api.getUserViews(w, authedWithLibraries(httptest.NewRequest("GET", "/UserViews", nil), libs)) + Expect(w.Code).To(Equal(http.StatusOK)) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res.Items).To(HaveLen(1)) + Expect(res.Items[0].Id).To(Equal("1")) + }) + + It("returns no views for a user with no library access", func() { + w := httptest.NewRecorder() + api.getUserViews(w, authedWithLibraries(httptest.NewRequest("GET", "/UserViews", nil), nil)) + Expect(w.Code).To(Equal(http.StatusOK)) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res.Items).To(HaveLen(0)) + Expect(res.TotalRecordCount).To(Equal(0)) + }) }) It("returns the current user", func() { w := httptest.NewRecorder() - api.getCurrentUser(w, authed(httptest.NewRequest("GET", "/Users/Me", nil))) + api.getCurrentUser(w, authedWithLibraries(httptest.NewRequest("GET", "/Users/Me", nil), nil)) var u dto.UserDto Expect(json.Unmarshal(w.Body.Bytes(), &u)).To(Succeed()) Expect(u.Name).To(Equal("alice"))