remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-11-14 22:28:41 +01:00
Code Issues Projects Releases Packages Wiki Activity

fix(nativeapi): check avatar permission before reading the request body

Browse source 
The gate ran inside saveFn, so a request destined for 403 still spooled up to
10MB of multipart body to temp first. Artist, playlist and radio already gate
before parsing; the shared handler now takes the gate as a parameter.
This commit is contained in:
Deluan 2026-09-07 20:59:59 -04:00
commit 114208e6d6
3 changed files with 40 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

19
server/nativeapi/image_upload.go
View file

@ -31,16 +31,16 @@ func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool {
}
func handleImageUpload(saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc {
return handleImageUploadGated(true, saveFn)
return handleImageUploadGated(checkImageUploadPermission, saveFn)
}
// handleImageUploadGated is handleImageUpload with an opt-out from the EnableArtworkUpload gate,
// so avatar uploads (gated separately by EnableUserAvatarUpload) can reuse this handler.
func handleImageUploadGated(checkArtworkFlag bool, saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc {
// handleImageUploadGated is handleImageUpload with a custom permission gate, checked before the
// request body is read, so avatar uploads (gated by EnableUserAvatarUpload) can reuse this handler.
func handleImageUploadGated(gate func(http.ResponseWriter, *http.Request) bool, saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc {
maxImageSize := artwork.MaxImageUploadSize()
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
if checkArtworkFlag && !checkImageUploadPermission(w, r) {
if !gate(w, r) {
return
}
r.Body = http.MaxBytesReader(w, r.Body, maxImageSize)
@ -103,15 +103,14 @@ func handleImageUploadGated(checkArtworkFlag bool, saveFn func(ctx context.Conte
}
func handleImageDelete(deleteFn func(ctx context.Context) error) http.HandlerFunc {
return handleImageDeleteGated(true, deleteFn)
return handleImageDeleteGated(checkImageUploadPermission, deleteFn)
}
// handleImageDeleteGated is handleImageDelete with the same EnableArtworkUpload opt-out as
// handleImageUploadGated.
func handleImageDeleteGated(checkArtworkFlag bool, deleteFn func(ctx context.Context) error) http.HandlerFunc {
// handleImageDeleteGated is handleImageDelete with the same custom gate as handleImageUploadGated.
func handleImageDeleteGated(gate func(http.ResponseWriter, *http.Request) bool, deleteFn func(ctx context.Context) error) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
if checkArtworkFlag && !checkImageUploadPermission(w, r) {
if !gate(w, r) {
return
}
if err := deleteFn(ctx); err != nil {

21
server/nativeapi/users.go
View file

@ -44,13 +44,19 @@ func canEditAvatar(ctx context.Context, targetID string) error {
return nil
}
// checkAvatarPermission gates avatar uploads by EnableUserAvatarUpload, never by EnableArtworkUpload.
func checkAvatarPermission(w http.ResponseWriter, r *http.Request) bool {
ctx := r.Context()
if err := canEditAvatar(ctx, chi.URLParamFromCtx(ctx, "id")); err != nil {
http.Error(w, "not authorized", http.StatusForbidden)
return false
}
return true
}
func (api *Router) uploadUserAvatar() http.HandlerFunc {
// false: avatars are gated by EnableUserAvatarUpload, never by EnableArtworkUpload.
return handleImageUploadGated(false, func(ctx context.Context, reader io.Reader, ext string) error {
return handleImageUploadGated(checkAvatarPermission, func(ctx context.Context, reader io.Reader, ext string) error {
userID := chi.URLParamFromCtx(ctx, "id")
if err := canEditAvatar(ctx, userID); err != nil {
return err
}
usr, err := api.ds.User(ctx).Get(userID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
@ -67,11 +73,8 @@ func (api *Router) uploadUserAvatar() http.HandlerFunc {
}
func (api *Router) deleteUserAvatar() http.HandlerFunc {
return handleImageDeleteGated(false, func(ctx context.Context) error {
return handleImageDeleteGated(checkAvatarPermission, func(ctx context.Context) error {
userID := chi.URLParamFromCtx(ctx, "id")
if err := canEditAvatar(ctx, userID); err != nil {
return err
}
usr, err := api.ds.User(ctx).Get(userID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {

19
server/nativeapi/users_test.go
View file

@ -28,6 +28,16 @@ func testRouter(ds model.DataStore) http.Handler {
return r
}
type trackingReader struct {
r io.Reader
read bool
}
func (t *trackingReader) Read(p []byte) (int, error) {
t.read = true
return t.r.Read(p)
}
var _ = Describe("User avatar routes", func() {
var router http.Handler
var ds *tests.MockDataStore
@ -89,6 +99,15 @@ var _ = Describe("User avatar routes", func() {
Expect(w.Code).To(Equal(http.StatusForbidden))
})
It("refuses a third party before reading the request body", func() {
body, ct := pngUpload()
tracked := &trackingReader{r: body}
w := httptest.NewRecorder()
router.ServeHTTP(w, newRequest("POST", "/user/u1/image", tracked, ct, otherUser))
Expect(w.Code).To(Equal(http.StatusForbidden))
Expect(tracked.read).To(BeFalse())
})
It("refuses everyone, admins included, when the flag is off", func() {
conf.Server.EnableUserAvatarUpload = false
body, ct := pngUpload()