nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-31 07:30:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(jellyfin): serve playlist covers regardless of visibility (#5813)

Browse Source 
The image endpoint only served a private playlist's cover when the request
carried a token identifying its owner or an admin. But clients fetch cover
URLs without credentials — real Jellyfin's image routes are anonymous — so
every private playlist rendered the generic placeholder in Jellyfin clients
(observed in production), while the same covers displayed fine through the
always-authenticated Subsonic/native APIs.

Drop the gate and serve playlist covers like album/artist/track artwork:
playlist ids are unguessable without credentials, so anonymous access does
not meaningfully expose private playlist contents.
This commit is contained in:
Deluan Quintão 2026-07-18 19:36:12 -04:00 committed by GitHub
parent 3158451b8d
commit 59f1b4206c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 17 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
server/jellyfin/e2e/images_test.go
View File

@ -28,9 +28,9 @@ var _ = Describe("Item images", func() {
Expect(artworkSpy.lastID).To(ContainSubstring(id))
})
It("resolves a private playlist's cover for its owner under an elevated context", func() {
// The route carries no user in ctx (public); the owner is identified by the request token,
// and resolution then runs elevated so the visibility filter doesn't eat the cover.
It("resolves a private playlist's cover under an elevated context", func() {
// The route carries no user in ctx (public); resolution runs elevated so the visibility
// filter doesn't eat the cover.
plID := createPlaylist("Private Mix", nil)
w := get("/Items/" + enc(plID) + "/Images/Primary")
Expect(w.Code).To(Equal(http.StatusOK))
@ -48,27 +48,11 @@ var _ = Describe("Item images", func() {
Expect(w.Body.String()).To(Equal("IMG"))
})
Describe("private playlist covers", func() {
It("does not resolve a private playlist's cover for an unauthenticated caller", func() {
plID := createPlaylist("Secret Mix", nil) // owned by admin, private
w := rawReq("GET", "/Items/"+enc(plID)+"/Images/Primary", "")
Expect(w.Code).To(Equal(http.StatusOK)) // placeholder, not an auth error
Expect(artworkSpy.lastID).ToNot(ContainSubstring(plID))
})
It("does not resolve a private playlist's cover for another user", func() {
plID := createPlaylist("Secret Mix", nil)
w := getAs(regularUser, "/Items/"+enc(plID)+"/Images/Primary")
Expect(w.Code).To(Equal(http.StatusOK))
Expect(artworkSpy.lastID).ToNot(ContainSubstring(plID))
})
It("resolves a public playlist's cover for anyone", func() {
plID := createPlaylist("Shared Mix", nil)
Expect(post("/Playlists/"+enc(plID), `{"IsPublic":true}`).Code).To(Equal(http.StatusNoContent))
w := rawReq("GET", "/Items/"+enc(plID)+"/Images/Primary", "")
Expect(w.Code).To(Equal(http.StatusOK))
Expect(artworkSpy.lastID).To(ContainSubstring(plID))
})
// Covers are served regardless of playlist visibility — see getItemImage for the rationale.
It("resolves a private playlist's cover for an unauthenticated caller", func() {
plID := createPlaylist("Secret Mix", nil) // owned by admin, private
w := rawReq("GET", "/Items/"+enc(plID)+"/Images/Primary", "")
Expect(w.Code).To(Equal(http.StatusOK))
Expect(artworkSpy.lastID).To(ContainSubstring(plID))
})
})

16
server/jellyfin/images.go
View File

@ -25,14 +25,13 @@ import (
)
func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
// Public endpoint (no user in ctx): library artwork isn't user-sensitive, so resolution runs
// under an elevated context to bypass the persistence visibility filter; playlist access is
// gated inside resolveArtworkID.
// Public endpoint, like real Jellyfin's image routes: clients fetch cover URLs without credentials
// and item ids are unguessable, so resolution runs elevated to bypass the visibility filter.
ctx := request.WithUser(r.Context(), model.User{IsAdmin: true})
itemId := api.resolveItemID(ctx, dto.DecodeID(chi.URLParam(r, "itemId")))
size, _ := strconv.Atoi(r.URL.Query().Get("maxwidth"))
artID := api.resolveArtworkID(ctx, r, itemId)
artID := api.resolveArtworkID(ctx, itemId)
reader, _, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false)
switch {
case errors.Is(err, context.Canceled):
@ -49,7 +48,7 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
// resolveArtworkID maps a Jellyfin item id to a Navidrome ArtworkID, probing
// album -> artist -> media file -> playlist.
func (api *Router) resolveArtworkID(ctx context.Context, r *http.Request, itemId string) string {
func (api *Router) resolveArtworkID(ctx context.Context, itemId string) string {
if al, err := api.ds.Album(ctx).Get(itemId); err == nil {
return al.CoverArtID().String()
}
@ -60,12 +59,7 @@ func (api *Router) resolveArtworkID(ctx context.Context, r *http.Request, itemId
return mf.CoverArtID().String()
}
if pl, err := api.ds.Playlist(ctx).Get(itemId); err == nil {
// Playlist covers are user-scoped: serve a private one only for a public playlist or a
// token identifying its owner/an admin, so this public route can't probe others' covers.
u, ok := api.userFromToken(r)
if pl.Public || (ok && (u.IsAdmin || pl.OwnerID == u.ID)) {
return pl.CoverArtID().String()
}
return pl.CoverArtID().String()
}
return (model.ArtworkID{}).String()
}

17
server/jellyfin/images_test.go
View File

@ -85,20 +85,7 @@ var _ = Describe("Images", func() {
Expect(w.Header().Get("Content-Type")).To(Equal("image/png"))
})
It("resolves a public playlist id to its cover artwork", func() {
ds := &tests.MockDataStore{}
ds.Playlist(context.Background()).(*tests.MockPlaylistRepo).SetData(model.Playlists{{ID: "pl1", Name: "Mix", Public: true}})
fa := &fakeArtwork{}
api := &Router{ds: ds, artwork: fa}
w, r := newImageRequest(dto.EncodeID("pl1"))
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(fa.recvId).To(ContainSubstring("pl1"))
})
It("serves the placeholder, not the cover, for a private playlist and an anonymous caller", func() {
It("resolves a playlist's cover regardless of visibility, even for an anonymous caller", func() {
ds := &tests.MockDataStore{}
ds.Playlist(context.Background()).(*tests.MockPlaylistRepo).SetData(model.Playlists{{ID: "pl1", Name: "Mix", OwnerID: "someone"}})
fa := &fakeArtwork{}
@ -108,7 +95,7 @@ var _ = Describe("Images", func() {
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(fa.recvId).ToNot(ContainSubstring("pl1"))
Expect(fa.recvId).To(ContainSubstring("pl1"))
})
// This endpoint is public (no user in the request), so artwork must be resolved under an

2
server/jellyfin/middlewares.go
View File

@ -152,7 +152,7 @@ func tokenFromRequest(r *http.Request) string {
}
// userFromToken resolves the user for the request's token; ok is false for a missing/invalid token
// or unknown subject. Used by authenticate and by public routes that optionally identify the caller.
// or unknown subject.
func (api *Router) userFromToken(r *http.Request) (model.User, bool) {
token := tokenFromRequest(r)
if token == "" {