diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index de4c55a1e..e47b18cf1 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -135,7 +135,7 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router { provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker) sonicSonic := sonic.New(dataStore, manager, matcherMatcher) lyricsLyrics := lyrics.NewLyrics(dataStore, manager) - router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker) + router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker, uploader) return router } diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 11f884ba4..b290dde7d 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -40,6 +40,7 @@ type Router struct { sonic sonic.Engine lyrics lyrics.Lyrics broker events.Broker + imgUpload artwork.Uploader lyricsCache cache.SimpleCache[string, model.LyricList] similarFlight singleflight.Group serverIDMu sync.Mutex @@ -49,11 +50,11 @@ type Router struct { func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStreamer, transcodeDecider stream.TranscodeDecider, players core.Players, scrobbler scrobbler.PlayTracker, playlists playlists.Playlists, provider external.Provider, - sonicSvc sonic.Engine, lyricsSvc lyrics.Lyrics, broker events.Broker) *Router { + sonicSvc sonic.Engine, lyricsSvc lyrics.Lyrics, broker events.Broker, imgUpload artwork.Uploader) *Router { r := &Router{ ds: ds, artwork: artwork, streamer: streamer, transcodeDecider: transcodeDecider, players: players, scrobbler: scrobbler, playlists: playlists, provider: provider, - sonic: sonicSvc, lyrics: lyricsSvc, broker: broker, + sonic: sonicSvc, lyrics: lyricsSvc, broker: broker, imgUpload: imgUpload, lyricsCache: cache.NewSimpleCache[string, model.LyricList](cache.Options{ SizeLimit: 1000, DefaultTTL: 5 * time.Minute, @@ -111,6 +112,8 @@ func (api *Router) routes() http.Handler { r.Get("/users/{userId}/views", api.getUserViews) r.Get("/users/me", api.getCurrentUser) r.Get("/users/{userId}", api.getCurrentUser) + r.Post("/userimage", api.postUserImage) + r.Delete("/userimage", api.deleteUserImage) // Cursor-backed collections: each streams straight from the DB, holding a connection for the // whole client-paced response, so enough slow clients would take the entire pool and stall the diff --git a/server/jellyfin/api_test.go b/server/jellyfin/api_test.go index fe69c8321..3d9c04126 100644 --- a/server/jellyfin/api_test.go +++ b/server/jellyfin/api_test.go @@ -18,7 +18,7 @@ import ( var _ = Describe("Router", func() { It("serves the public handshake through the mounted handler", func() { ds := &tests.MockDataStore{} - api := New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/System/Info/Public", nil) api.ServeHTTP(w, r) @@ -26,7 +26,7 @@ var _ = Describe("Router", func() { }) It("returns 404 JSON for unknown routes", func() { - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Nonexistent/Route", nil) api.ServeHTTP(w, r) @@ -36,7 +36,7 @@ var _ = Describe("Router", func() { }) It("returns 404 JSON for a known path with an unsupported method", func() { - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("PATCH", "/System/Info/Public", nil) api.ServeHTTP(w, r) @@ -53,7 +53,7 @@ var _ = Describe("Router", func() { Expect(err).ToNot(HaveOccurred()) fp := &fakePlayers{} - api := New(ds, nil, nil, nil, fp, nil, nil, nil, nil, nil, nil) + api := New(ds, nil, nil, nil, fp, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Users/Me", nil) @@ -70,7 +70,7 @@ var _ = Describe("Router", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.AuthRequestLimit = 2 conf.Server.AuthWindowLength = time.Minute - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) login := func() int { w := httptest.NewRecorder() diff --git a/server/jellyfin/e2e/e2e_suite_test.go b/server/jellyfin/e2e/e2e_suite_test.go index 1ee19ea01..b46390125 100644 --- a/server/jellyfin/e2e/e2e_suite_test.go +++ b/server/jellyfin/e2e/e2e_suite_test.go @@ -326,6 +326,7 @@ func setupTestDB() { sonicProviderFake = &fakeSonicProvider{} sonicSvc := sonic.New(ds, &fakeSonicLoader{provider: sonicProviderFake}, matcher.New(ds)) decider := stream.NewTranscodeDecider(ds, harness.NoopFFmpeg{}) + imgUpload := artwork.NewUploader(ds) router = jellyfin.New( ds, artworkSpy, @@ -333,11 +334,12 @@ func setupTestDB() { decider, core.NewPlayers(ds), scrobbler.NewPlayTracker(ds, events.NoopBroker(), nil), - playlists.NewPlaylists(ds, artwork.NewUploader(ds)), + playlists.NewPlaylists(ds, imgUpload), providerFake, sonicSvc, lyrics.NewLyrics(ds, nil), events.NoopBroker(), + imgUpload, ) } diff --git a/server/jellyfin/routing_test.go b/server/jellyfin/routing_test.go index 62b52cfff..70da5b1eb 100644 --- a/server/jellyfin/routing_test.go +++ b/server/jellyfin/routing_test.go @@ -19,7 +19,7 @@ var _ = Describe("Case-insensitive routing", func() { var api *Router BeforeEach(func() { - api = New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api = New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) }) It("serves a fully lowercase path directly", func() { diff --git a/server/jellyfin/socket_test.go b/server/jellyfin/socket_test.go index 401c791fd..b4be0e244 100644 --- a/server/jellyfin/socket_test.go +++ b/server/jellyfin/socket_test.go @@ -94,7 +94,7 @@ var _ = Describe("handleSocket", func() { Expect(err).ToNot(HaveOccurred()) token = t - api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) }) It("upgrades when authenticated via the api_key query parameter", func() { diff --git a/server/jellyfin/user_image.go b/server/jellyfin/user_image.go index b21c2c04c..e58b8e4c3 100644 --- a/server/jellyfin/user_image.go +++ b/server/jellyfin/user_image.go @@ -1,10 +1,16 @@ package jellyfin import ( + "bytes" + "io" "net/http" "strings" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/imghttp" "github.com/navidrome/navidrome/server/jellyfin/dto" ) @@ -65,3 +71,93 @@ func isPublicUser(username string) bool { } return false } + +var avatarExtByContentType = map[string]string{ + "image/png": ".png", + "image/jpeg": ".jpg", + "image/jpg": ".jpg", + "image/gif": ".gif", + "image/webp": ".webp", +} + +// targetUser resolves the userid param, defaulting to the caller, and applies the +// self-or-admin write rule plus the feature flag (which refuses everyone, admins included). +func (api *Router) targetUser(w http.ResponseWriter, r *http.Request) (*model.User, bool) { + ctx := r.Context() + caller, _ := request.UserFrom(ctx) + id := caller.ID + if raw := r.URL.Query().Get("userid"); raw != "" { + decoded, ok := dto.DecodeID(raw) + if !ok { + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, false + } + id = decoded + } + if !conf.Server.EnableUserAvatarUpload || (!caller.IsAdmin && caller.ID != id) { + http.Error(w, "Forbidden", http.StatusForbidden) + return nil, false + } + usr, err := api.ds.User(ctx).Get(id) + if err != nil { + http.Error(w, "Not Found", http.StatusNotFound) + return nil, false + } + return usr, true +} + +func (api *Router) postUserImage(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + usr, ok := api.targetUser(w, r) + if !ok { + return + } + mimeType := strings.TrimSpace(strings.SplitN(r.Header.Get("Content-Type"), ";", 2)[0]) + ext, known := avatarExtByContentType[strings.ToLower(mimeType)] + if !known { + http.Error(w, "Incorrect ContentType.", http.StatusBadRequest) + return + } + + // Jellyfin clients base64-encode the wire body (4/3 bigger), so the read cap allows for inflation. + limit := artwork.MaxImageUploadSize() + body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, limit*4/3+4)) + if err != nil { + http.Error(w, "file too large", http.StatusBadRequest) + return + } + imgBytes, err := decodeImageBody(body) + if err != nil { + http.Error(w, "Bad Request", http.StatusBadRequest) + return + } + + filename, err := api.imgUpload.SetAvatar(ctx, usr.ID, usr.UserName, usr.UploadedImagePath(), bytes.NewReader(imgBytes), ext) + if err != nil { + log.Error(ctx, "Jellyfin API: could not save avatar", "user", usr.UserName, err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + if err := api.ds.User(ctx).UpdateImage(usr.ID, filename); err != nil { + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusNoContent) +} + +func (api *Router) deleteUserImage(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + usr, ok := api.targetUser(w, r) + if !ok { + return + } + if err := api.imgUpload.RemoveImage(ctx, usr.UploadedImagePath()); err != nil { + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + if err := api.ds.User(ctx).UpdateImage(usr.ID, ""); err != nil { + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/server/jellyfin/user_image_test.go b/server/jellyfin/user_image_test.go index da78816b6..d746f9476 100644 --- a/server/jellyfin/user_image_test.go +++ b/server/jellyfin/user_image_test.go @@ -1,16 +1,23 @@ package jellyfin import ( + "bytes" "context" + "encoding/base64" "encoding/json" + "image" + "image/png" + "io" "net/http" "net/http/httptest" "os" "path/filepath" + "strings" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/jellyfin/dto" @@ -33,7 +40,7 @@ var _ = Describe("GET /userimage", func() { var api *Router var ds *tests.MockDataStore var ur *tests.MockedUserRepo - var pub, priv *model.User + var pub, priv, noAvatar *model.User BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) @@ -48,10 +55,16 @@ var _ = Describe("GET /userimage", func() { pub.UploadedImage = writeUserAvatar(pub) Expect(ur.Put(pub)).To(Succeed()) + // Has a real avatar so the anonymous-401 test below fails on a served image (200), not a + // 404, if the isPublicUser gate is ever removed. priv = &model.User{ID: testID("u1"), UserName: "alice"} + priv.UploadedImage = writeUserAvatar(priv) Expect(ur.Put(priv)).To(Succeed()) - api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + noAvatar = &model.User{ID: testID("u4"), UserName: "carol"} + Expect(ur.Put(noAvatar)).To(Succeed()) + + api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) }) get := func(query string) *httptest.ResponseRecorder { @@ -84,10 +97,10 @@ var _ = Describe("GET /userimage", func() { }) It("returns 404 for an authenticated user with no avatar", func() { - tok, err := auth.CreateToken(priv) + tok, err := auth.CreateToken(noAvatar) Expect(err).ToNot(HaveOccurred()) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/userimage?userId="+dto.EncodeID(priv.ID), nil) + r := httptest.NewRequest("GET", "/userimage?userId="+dto.EncodeID(noAvatar.ID), nil) r.Header.Set("X-Emby-Token", tok) api.ServeHTTP(w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) @@ -110,6 +123,125 @@ var _ = Describe("GET /userimage", func() { }) }) +var _ = Describe("POST /userimage and DELETE /userimage", func() { + var router *Router + var ds *tests.MockDataStore + var ur *tests.MockedUserRepo + var caller *model.User + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DataFolder = conf.NewDir(GinkgoT().TempDir()) + conf.Server.EnableUserAvatarUpload = true + + ds = &tests.MockDataStore{} + auth.Init(ds) + ur = ds.User(context.Background()).(*tests.MockedUserRepo) + + caller = &model.User{ID: "u1", UserName: "regular"} + Expect(ur.Put(caller)).To(Succeed()) + // A real canonical id, unlike caller's literal "u1", so it round-trips through dto.EncodeID. + Expect(ur.Put(&model.User{ID: testID("u2"), UserName: "other"})).To(Succeed()) + + router = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, artwork.NewUploader(ds)) + }) + + tokenFor := func(u *model.User) string { + tok, err := auth.CreateToken(u) + Expect(err).ToNot(HaveOccurred()) + return tok + } + authenticatedRequestWithBody := func(method, target string, body io.Reader) *http.Request { + r := httptest.NewRequest(method, target, body) + r.Header.Set("X-Emby-Token", tokenFor(caller)) + return r + } + authenticatedRequest := func(method, target string) *http.Request { + return authenticatedRequestWithBody(method, target, nil) + } + pngBytes := func() []byte { + var buf bytes.Buffer + Expect(png.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 8, 8)))).To(Succeed()) + return buf.Bytes() + } + + Describe("POST /userimage", func() { + It("accepts a base64 body with a charset suffix", func() { + body := base64.StdEncoding.EncodeToString(pngBytes()) + r := authenticatedRequestWithBody("POST", "/userimage", strings.NewReader(body)) + r.Header.Set("Content-Type", "image/png; charset=utf-8") + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusNoContent)) + + usr, _ := ds.User(context.Background()).Get("u1") + Expect(usr.UploadedImage).To(Equal("u1_regular.png")) + }) + + It("accepts raw image bytes too", func() { + r := authenticatedRequestWithBody("POST", "/userimage", bytes.NewReader(pngBytes())) + r.Header.Set("Content-Type", "image/png") + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusNoContent)) + }) + + It("rejects an unknown content type", func() { + r := authenticatedRequestWithBody("POST", "/userimage", strings.NewReader("x")) + r.Header.Set("Content-Type", "text/plain") + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("refuses a third party", func() { + body := base64.StdEncoding.EncodeToString(pngBytes()) + r := authenticatedRequestWithBody("POST", "/userimage?userId="+dto.EncodeID(testID("u2")), strings.NewReader(body)) + r.Header.Set("Content-Type", "image/png") + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusForbidden)) + }) + + It("refuses even an admin when the flag is off", func() { + conf.Server.EnableUserAvatarUpload = false + admin := &model.User{ID: "admin1", UserName: "boss", IsAdmin: true} + Expect(ur.Put(admin)).To(Succeed()) + + body := base64.StdEncoding.EncodeToString(pngBytes()) + r := httptest.NewRequest("POST", "/userimage", strings.NewReader(body)) + r.Header.Set("X-Emby-Token", tokenFor(admin)) + r.Header.Set("Content-Type", "image/png") + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + Expect(w.Code).To(Equal(http.StatusForbidden)) + }) + + It("is not reachable anonymously", func() { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest("POST", "/userimage", strings.NewReader("x"))) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + + Describe("DELETE /userimage", func() { + It("clears the avatar and removes the file from disk", func() { + name := writeUserAvatar(caller) + Expect(ur.UpdateImage(caller.ID, name)).To(Succeed()) + path := caller.UploadedImagePath() + + w := httptest.NewRecorder() + router.ServeHTTP(w, authenticatedRequest("DELETE", "/userimage")) + Expect(w.Code).To(Equal(http.StatusNoContent)) + + usr, _ := ds.User(context.Background()).Get("u1") + Expect(usr.UploadedImage).To(BeEmpty()) + _, err := os.Stat(path) + Expect(os.IsNotExist(err)).To(BeTrue()) + }) + }) +}) + var _ = Describe("isPublicUser", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig())