mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 12:27:09 +02:00
feat(jellyfin): accept avatar upload and delete on /userimage
Adds POST/DELETE /userimage (authenticated-only), enforcing the self-or-admin write rule and the EnableUserAvatarUpload flag. Also strengthens the anonymous-401 GET /userimage test to seed the private user with a real avatar file, so it fails for the right reason if the isPublicUser gate is ever removed.
This commit is contained in:
parent
d57bda78f8
commit
669d9216ec
8 changed files with 248 additions and 15 deletions
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue