From d57bda78f8f95b5494c6bb1d188f33a492721dd7 Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 7 Sep 2026 19:52:35 -0400 Subject: [PATCH] feat(jellyfin): serve user avatars from GET /userimage Registered unauthenticated (matching real Jellyfin's login-picker use case), but narrowed to conf.Server.Jellyfin.ExposedPublicUsers for anonymous callers. Also fills PrimaryImageTag on userToDto and getPublicUsers. --- server/jellyfin/api.go | 2 + server/jellyfin/auth.go | 1 + server/jellyfin/user_image.go | 67 ++++++++++ server/jellyfin/user_image_test.go | 188 +++++++++++++++++++++++++++++ server/jellyfin/users.go | 16 +-- 5 files changed, 264 insertions(+), 10 deletions(-) create mode 100644 server/jellyfin/user_image.go create mode 100644 server/jellyfin/user_image_test.go diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 6484a3bb4..11f884ba4 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -86,6 +86,8 @@ func (api *Router) routes() http.Handler { inner.Post("/users/authenticatebyname", api.authenticateByName) } inner.Get("/users/public", api.getPublicUsers) + // Unauthenticated on purpose, matching Jellyfin, but narrowed to ExposedPublicUsers. + inner.Get("/userimage", api.getUserImage) // Images are intentionally public: artwork isn't sensitive, matching Jellyfin's image handling. // Bound concurrency like Subsonic's getCoverArt: image decode/resize is CPU- and memory-heavy, diff --git a/server/jellyfin/auth.go b/server/jellyfin/auth.go index e7070d341..c1f69d1ef 100644 --- a/server/jellyfin/auth.go +++ b/server/jellyfin/auth.go @@ -61,6 +61,7 @@ func userToDto(u *model.User, serverName, serverID string) *dto.UserDto { ServerName: serverName, HasPassword: true, HasConfiguredPassword: true, + PrimaryImageTag: u.AvatarTag(), Policy: userPolicy(u), Configuration: userConfiguration(), } diff --git a/server/jellyfin/user_image.go b/server/jellyfin/user_image.go new file mode 100644 index 000000000..b21c2c04c --- /dev/null +++ b/server/jellyfin/user_image.go @@ -0,0 +1,67 @@ +package jellyfin + +import ( + "net/http" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/server/imghttp" + "github.com/navidrome/navidrome/server/jellyfin/dto" +) + +// getUserImage is registered unauthenticated, like Jellyfin's own GET /UserImage, so a login picker +// can show avatars; anonymous callers are limited to the ExposedPublicUsers allowlist. +func (api *Router) getUserImage(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + // This route skips the authenticate middleware, so a caller's token (if any) is resolved directly. + caller, authenticated := api.userFromToken(r) + + rawID := r.URL.Query().Get("userid") + if rawID == "" { + if !authenticated { + http.Error(w, "UserId is required if unauthenticated", http.StatusBadRequest) + return + } + rawID = dto.EncodeID(caller.ID) + } + id, ok := dto.DecodeID(rawID) + if !ok { + http.Error(w, "Bad Request", http.StatusBadRequest) + return + } + + usr, err := api.ds.User(ctx).Get(id) + if err != nil { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + if !authenticated && !isPublicUser(usr.UserName) { + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + if !imghttp.ServeUserAvatar(w, r, usr) { + http.Error(w, "Not Found", http.StatusNotFound) + } +} + +// publicUsernames splits and normalizes the raw ExposedPublicUsers config: comma-separated, trimmed, +// skipping empty entries. Shared with getPublicUsers so the two allowlist checks can't drift apart. +func publicUsernames() []string { + var names []string + for name := range strings.SplitSeq(conf.Server.Jellyfin.ExposedPublicUsers, ",") { + name = strings.TrimSpace(name) + if name != "" { + names = append(names, name) + } + } + return names +} + +func isPublicUser(username string) bool { + for _, name := range publicUsernames() { + if strings.EqualFold(name, username) { + return true + } + } + return false +} diff --git a/server/jellyfin/user_image_test.go b/server/jellyfin/user_image_test.go new file mode 100644 index 000000000..da78816b6 --- /dev/null +++ b/server/jellyfin/user_image_test.go @@ -0,0 +1,188 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// writeUserAvatar seeds a fake avatar file on disk and returns the UploadedImage filename to store +// on the user record. +func writeUserAvatar(u *model.User) string { + name := u.ID + "_" + u.UserName + ".png" + path := filepath.Join(conf.Server.DataFolder.String(), consts.AvatarFolder, name) + Expect(os.MkdirAll(filepath.Dir(path), 0755)).To(Succeed()) + Expect(os.WriteFile(path, []byte{0x89, 'P', 'N', 'G'}, 0600)).To(Succeed()) + return name +} + +var _ = Describe("GET /userimage", func() { + var api *Router + var ds *tests.MockDataStore + var ur *tests.MockedUserRepo + var pub, priv *model.User + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DataFolder = conf.NewDir(GinkgoT().TempDir()) + conf.Server.Jellyfin.ExposedPublicUsers = "publicuser" + + ds = &tests.MockDataStore{} + auth.Init(ds) + ur = ds.User(context.Background()).(*tests.MockedUserRepo) + + pub = &model.User{ID: testID("pub1"), UserName: "publicuser"} + pub.UploadedImage = writeUserAvatar(pub) + Expect(ur.Put(pub)).To(Succeed()) + + priv = &model.User{ID: testID("u1"), UserName: "alice"} + Expect(ur.Put(priv)).To(Succeed()) + + api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + }) + + get := func(query string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + api.ServeHTTP(w, httptest.NewRequest("GET", "/userimage"+query, nil)) + return w + } + + It("serves an allowlisted user's avatar without authentication", func() { + w := get("?userId=" + dto.EncodeID(pub.ID)) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + // This is the leak this whole design exists to prevent: anonymous access must be denied for + // anyone not on the allowlist, not just fall through to a 404. + It("refuses an anonymous request for a user not on the allowlist with 401", func() { + w := get("?userId=" + dto.EncodeID(priv.ID)) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + + It("refuses every anonymous request when the allowlist is empty", func() { + conf.Server.Jellyfin.ExposedPublicUsers = "" + w := get("?userId=" + dto.EncodeID(pub.ID)) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + + It("rejects an anonymous request with no userId with 400", func() { + w := get("") + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("returns 404 for an authenticated user with no avatar", func() { + tok, err := auth.CreateToken(priv) + Expect(err).ToNot(HaveOccurred()) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/userimage?userId="+dto.EncodeID(priv.ID), nil) + r.Header.Set("X-Emby-Token", tok) + api.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + + It("matches an allowlist entry regardless of surrounding whitespace and casing", func() { + conf.Server.Jellyfin.ExposedPublicUsers = " PublicUser " + w := get("?userId=" + dto.EncodeID(pub.ID)) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + It("404s an authenticated request for an id that decodes but does not exist", func() { + tok, err := auth.CreateToken(priv) + Expect(err).ToNot(HaveOccurred()) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/userimage?userId="+dto.EncodeID(testID("ghost")), nil) + r.Header.Set("X-Emby-Token", tok) + api.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) +}) + +var _ = Describe("isPublicUser", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + It("matches a configured name case-insensitively and trims whitespace", func() { + conf.Server.Jellyfin.ExposedPublicUsers = " Alice ,bob" + Expect(isPublicUser("alice")).To(BeTrue()) + Expect(isPublicUser("ALICE")).To(BeTrue()) + Expect(isPublicUser("bob")).To(BeTrue()) + }) + + It("rejects a user not on the allowlist", func() { + conf.Server.Jellyfin.ExposedPublicUsers = "alice" + Expect(isPublicUser("eve")).To(BeFalse()) + }) + + It("rejects everyone when the allowlist is empty", func() { + conf.Server.Jellyfin.ExposedPublicUsers = "" + Expect(isPublicUser("alice")).To(BeFalse()) + }) +}) + +var _ = Describe("PrimaryImageTag", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + It("is present on userToDto when the user has an avatar", func() { + u := model.User{ID: "u1", UserName: "deluan", UploadedImage: "u1_deluan.png"} + got := userToDto(&u, "srv", "sid") + Expect(got.PrimaryImageTag).To(Equal(u.AvatarTag())) + Expect(got.PrimaryImageTag).ToNot(BeEmpty()) + }) + + It("is empty on userToDto when the user has no avatar", func() { + u := model.User{ID: "u1", UserName: "deluan"} + Expect(userToDto(&u, "srv", "sid").PrimaryImageTag).To(BeEmpty()) + }) + + It("is present on getPublicUsers when the listed user has an avatar", func() { + ds := &tests.MockDataStore{} + ur := ds.User(context.Background()).(*tests.MockedUserRepo) + u := &model.User{ID: testID("pub1"), UserName: "publicuser", UploadedImage: "x.png"} + Expect(ur.Put(u)).To(Succeed()) + conf.Server.Jellyfin.ExposedPublicUsers = "publicuser" + + api := &Router{ds: ds} + w := httptest.NewRecorder() + api.getPublicUsers(w, httptest.NewRequest("GET", "/users/public", nil)) + Expect(w.Code).To(Equal(http.StatusOK)) + + var users []dto.UserDto + Expect(json.Unmarshal(w.Body.Bytes(), &users)).To(Succeed()) + Expect(users).To(HaveLen(1)) + Expect(users[0].PrimaryImageTag).To(Equal(u.AvatarTag())) + Expect(users[0].PrimaryImageTag).ToNot(BeEmpty()) + }) + + It("is empty on getPublicUsers when the listed user has no avatar", func() { + ds := &tests.MockDataStore{} + ur := ds.User(context.Background()).(*tests.MockedUserRepo) + Expect(ur.Put(&model.User{ID: testID("pub1"), UserName: "publicuser"})).To(Succeed()) + conf.Server.Jellyfin.ExposedPublicUsers = "publicuser" + + api := &Router{ds: ds} + w := httptest.NewRecorder() + api.getPublicUsers(w, httptest.NewRequest("GET", "/users/public", nil)) + Expect(w.Code).To(Equal(http.StatusOK)) + + var users []dto.UserDto + Expect(json.Unmarshal(w.Body.Bytes(), &users)).To(Succeed()) + Expect(users).To(HaveLen(1)) + Expect(users[0].PrimaryImageTag).To(BeEmpty()) + }) +}) diff --git a/server/jellyfin/users.go b/server/jellyfin/users.go index bbc60c892..622715fbf 100644 --- a/server/jellyfin/users.go +++ b/server/jellyfin/users.go @@ -4,7 +4,6 @@ import ( "net/http" "strings" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/jellyfin/dto" @@ -35,11 +34,7 @@ func (api *Router) getPublicUsers(w http.ResponseWriter, r *http.Request) { serverID := api.serverID(ctx) seen := make(map[string]bool) users := []dto.UserDto{} - for name := range strings.SplitSeq(conf.Server.Jellyfin.ExposedPublicUsers, ",") { - name = strings.TrimSpace(name) - if name == "" { - continue - } + for _, name := range publicUsernames() { key := strings.ToLower(name) if seen[key] { continue @@ -51,10 +46,11 @@ func (api *Router) getPublicUsers(w http.ResponseWriter, r *http.Request) { continue } users = append(users, dto.UserDto{ - Name: usr.UserName, - Id: dto.EncodeID(usr.ID), - ServerId: serverID, - HasPassword: true, + Name: usr.UserName, + Id: dto.EncodeID(usr.ID), + ServerId: serverID, + HasPassword: true, + PrimaryImageTag: usr.AvatarTag(), }) } api.ok(w, r, users)