From 435fb0b0768b5059992605bb0c0eb2c4812cdbda Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 2 Mar 2026 16:59:05 -0500 Subject: [PATCH] feat(server): add EnableCoverArtUpload config option Allow administrators to disable playlist cover art upload/removal for non-admin users via the new EnableCoverArtUpload config option (default: true). - Guard uploadPlaylistImage and deletePlaylistImage endpoints (403 for non-admin when disabled) - Set CoverArtRole in Subsonic GetUser/GetUsers responses based on config and admin status - Pass config to frontend and conditionally hide upload/remove UI controls - Admins always retain upload capability regardless of setting --- conf/configuration.go | 2 + server/nativeapi/playlists.go | 12 +++ server/nativeapi/playlists_test.go | 126 +++++++++++++++++++++------- server/serve_index.go | 1 + server/subsonic/users.go | 1 + server/subsonic/users_test.go | 16 ++++ ui/src/config.js | 1 + ui/src/playlist/PlaylistDetails.jsx | 5 +- 8 files changed, 131 insertions(+), 33 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index 0d32815e..b6f65183 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -76,6 +76,7 @@ type configOptions struct { EnableFavourites bool EnableStarRating bool EnableUserEditing bool + EnableCoverArtUpload bool EnableSharing bool ShareURL string DefaultShareExpiration time.Duration @@ -668,6 +669,7 @@ func setViperDefaults() { viper.SetDefault("enablereplaygain", true) viper.SetDefault("enablecoveranimation", true) viper.SetDefault("enablenowplaying", true) + viper.SetDefault("enablecoverartupload", true) viper.SetDefault("enablesharing", false) viper.SetDefault("shareurl", "") viper.SetDefault("defaultshareexpiration", 8760*time.Hour) diff --git a/server/nativeapi/playlists.go b/server/nativeapi/playlists.go index c7230a20..797654a3 100644 --- a/server/nativeapi/playlists.go +++ b/server/nativeapi/playlists.go @@ -17,9 +17,11 @@ import ( "github.com/deluan/rest" "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/playlists" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/utils/req" _ "golang.org/x/image/webp" ) @@ -237,6 +239,11 @@ const maxImageSize = 10 << 20 // 10MB func uploadPlaylistImage(pls playlists.Playlists) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + user, _ := request.UserFrom(ctx) + if !conf.Server.EnableCoverArtUpload && !user.IsAdmin { + http.Error(w, "cover art upload is disabled", http.StatusForbidden) + return + } p := req.Params(r) playlistId, _ := p.String(":id") @@ -306,6 +313,11 @@ func uploadPlaylistImage(pls playlists.Playlists) http.HandlerFunc { func deletePlaylistImage(pls playlists.Playlists) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + user, _ := request.UserFrom(ctx) + if !conf.Server.EnableCoverArtUpload && !user.IsAdmin { + http.Error(w, "cover art upload is disabled", http.StatusForbidden) + return + } p := req.Params(r) playlistId, _ := p.String(":id") diff --git a/server/nativeapi/playlists_test.go b/server/nativeapi/playlists_test.go index 961d10b6..7f0cd7de 100644 --- a/server/nativeapi/playlists_test.go +++ b/server/nativeapi/playlists_test.go @@ -3,6 +3,7 @@ package nativeapi import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "time" @@ -14,50 +15,56 @@ import ( "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/core/playlists" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -type mockPlaylistTrackRepo struct { - model.PlaylistTrackRepository - tracks model.PlaylistTracks -} +var _ = Describe("Playlist Image Endpoints", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) -func (m *mockPlaylistTrackRepo) Count(...rest.QueryOptions) (int64, error) { - return int64(len(m.tracks)), nil -} + DescribeTable("uploadPlaylistImage guard", + func(enableCoverArtUpload, isAdmin bool, expectedStatus int) { + conf.Server.EnableCoverArtUpload = enableCoverArtUpload + handler := uploadPlaylistImage(&mockPlaylistsService{}) -func (m *mockPlaylistTrackRepo) ReadAll(...rest.QueryOptions) (any, error) { - return m.tracks, nil -} + req := httptest.NewRequest("POST", "/playlist/pls-1/image", nil) + ctx := request.WithUser(GinkgoT().Context(), model.User{ID: "user-1", IsAdmin: isAdmin}) + req = req.WithContext(ctx) -func (m *mockPlaylistTrackRepo) EntityName() string { - return "playlist_track" -} + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + Expect(w.Code).To(Equal(expectedStatus)) + }, + Entry("enabled, regular user passes guard", true, false, http.StatusBadRequest), + Entry("enabled, admin passes guard", true, true, http.StatusBadRequest), + Entry("disabled, admin passes guard", false, true, http.StatusBadRequest), + Entry("disabled, regular user is forbidden", false, false, http.StatusForbidden), + ) -func (m *mockPlaylistTrackRepo) NewInstance() any { - return &model.PlaylistTrack{} -} + DescribeTable("deletePlaylistImage guard", + func(enableCoverArtUpload, isAdmin bool, expectedStatus int) { + conf.Server.EnableCoverArtUpload = enableCoverArtUpload + handler := deletePlaylistImage(&mockPlaylistsService{}) -func (m *mockPlaylistTrackRepo) Read(id string) (any, error) { - for _, t := range m.tracks { - if t.ID == id { - return &t, nil - } - } - return nil, rest.ErrNotFound -} + req := httptest.NewRequest("DELETE", "/playlist/pls-1/image", nil) + ctx := request.WithUser(GinkgoT().Context(), model.User{ID: "user-1", IsAdmin: isAdmin}) + req = req.WithContext(ctx) -type mockPlaylistsService struct { - playlists.Playlists - tracksRepo rest.Repository -} - -func (m *mockPlaylistsService) TracksRepository(_ context.Context, _ string, _ bool) rest.Repository { - return m.tracksRepo -} + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + Expect(w.Code).To(Equal(expectedStatus)) + }, + Entry("enabled, regular user passes guard", true, false, http.StatusNotFound), + Entry("enabled, admin passes guard", true, true, http.StatusNotFound), + Entry("disabled, admin passes guard", false, true, http.StatusNotFound), + Entry("disabled, regular user is forbidden", false, false, http.StatusForbidden), + ) +}) var _ = Describe("Playlist Tracks Endpoint", func() { var ( @@ -174,3 +181,58 @@ var _ = Describe("Playlist Tracks Endpoint", func() { }) }) }) + +type mockPlaylistTrackRepo struct { + model.PlaylistTrackRepository + tracks model.PlaylistTracks +} + +func (m *mockPlaylistTrackRepo) Count(...rest.QueryOptions) (int64, error) { + return int64(len(m.tracks)), nil +} + +func (m *mockPlaylistTrackRepo) ReadAll(...rest.QueryOptions) (any, error) { + return m.tracks, nil +} + +func (m *mockPlaylistTrackRepo) EntityName() string { + return "playlist_track" +} + +func (m *mockPlaylistTrackRepo) NewInstance() any { + return &model.PlaylistTrack{} +} + +func (m *mockPlaylistTrackRepo) Read(id string) (any, error) { + for _, t := range m.tracks { + if t.ID == id { + return &t, nil + } + } + return nil, rest.ErrNotFound +} + +type mockPlaylistsService struct { + playlists.Playlists + tracksRepo rest.Repository + removeImageFn func(ctx context.Context, id string) error + setImageFn func(ctx context.Context, id string, reader io.Reader, ext string) error +} + +func (m *mockPlaylistsService) RemoveImage(ctx context.Context, id string) error { + if m.removeImageFn != nil { + return m.removeImageFn(ctx, id) + } + return model.ErrNotFound +} + +func (m *mockPlaylistsService) SetImage(ctx context.Context, id string, reader io.Reader, ext string) error { + if m.setImageFn != nil { + return m.setImageFn(ctx, id, reader, ext) + } + return model.ErrNotFound +} + +func (m *mockPlaylistsService) TracksRepository(_ context.Context, _ string, _ bool) rest.Repository { + return m.tracksRepo +} diff --git a/server/serve_index.go b/server/serve_index.go index 92ef47e2..6b0c890a 100644 --- a/server/serve_index.go +++ b/server/serve_index.go @@ -61,6 +61,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl "losslessFormats": strings.ToUpper(strings.Join(mime.LosslessFormats, ",")), "devActivityPanel": conf.Server.DevActivityPanel, "enableUserEditing": conf.Server.EnableUserEditing, + "enableCoverArtUpload": conf.Server.EnableCoverArtUpload, "enableSharing": conf.Server.EnableSharing, "shareURL": conf.Server.ShareURL, "defaultDownloadableShare": conf.Server.DefaultDownloadableShare, diff --git a/server/subsonic/users.go b/server/subsonic/users.go index aeac6992..4f6dccaa 100644 --- a/server/subsonic/users.go +++ b/server/subsonic/users.go @@ -22,6 +22,7 @@ func buildUserResponse(user model.User) responses.User { ScrobblingEnabled: true, DownloadRole: conf.Server.EnableDownloads, ShareRole: conf.Server.EnableSharing, + CoverArtRole: conf.Server.EnableCoverArtUpload || user.IsAdmin, Folder: slice.Map(user.Libraries, func(lib model.Library) int32 { return int32(lib.ID) }), } diff --git a/server/subsonic/users_test.go b/server/subsonic/users_test.go index 95e16590..1fd5dce7 100644 --- a/server/subsonic/users_test.go +++ b/server/subsonic/users_test.go @@ -63,6 +63,7 @@ var _ = Describe("Users", func() { Expect(userResponse.User.ScrobblingEnabled).To(BeTrue()) Expect(userResponse.User.DownloadRole).To(BeTrue()) Expect(userResponse.User.ShareRole).To(BeTrue()) + Expect(userResponse.User.CoverArtRole).To(BeTrue()) Expect(userResponse.User.Folder).To(ContainElements(int32(10), int32(20))) // Verify GetUsers response structure @@ -81,6 +82,7 @@ var _ = Describe("Users", func() { Expect(singleUser.ScrobblingEnabled).To(Equal(userFromList.ScrobblingEnabled)) Expect(singleUser.DownloadRole).To(Equal(userFromList.DownloadRole)) Expect(singleUser.ShareRole).To(Equal(userFromList.ShareRole)) + Expect(singleUser.CoverArtRole).To(Equal(userFromList.CoverArtRole)) Expect(singleUser.JukeboxRole).To(Equal(userFromList.JukeboxRole)) Expect(singleUser.Folder).To(Equal(userFromList.Folder)) }) @@ -102,6 +104,20 @@ var _ = Describe("Users", func() { Entry("jukebox enabled, admin-only, admin user", true, true, true, true), ) + DescribeTable("CoverArt role permissions", + func(enableCoverArtUpload, isAdmin, expectedCoverArtRole bool) { + conf.Server.EnableCoverArtUpload = enableCoverArtUpload + testUser.IsAdmin = isAdmin + + response := buildUserResponse(testUser) + Expect(response.CoverArtRole).To(Equal(expectedCoverArtRole)) + }, + Entry("enabled, regular user", true, false, true), + Entry("enabled, admin user", true, true, true), + Entry("disabled, regular user", false, false, false), + Entry("disabled, admin user", false, true, true), + ) + Describe("Folder list population", func() { It("should populate Folder field with user's accessible library IDs", func() { testUser.Libraries = model.Libraries{ diff --git a/ui/src/config.js b/ui/src/config.js index 5acf10b6..0672a58f 100644 --- a/ui/src/config.js +++ b/ui/src/config.js @@ -22,6 +22,7 @@ const defaultConfig = { defaultUIVolume: 100, uiSearchDebounceMs: 200, enableUserEditing: true, + enableCoverArtUpload: true, enableSharing: true, shareURL: '', defaultDownloadableShare: true, diff --git a/ui/src/playlist/PlaylistDetails.jsx b/ui/src/playlist/PlaylistDetails.jsx index b24446cb..eefed83b 100644 --- a/ui/src/playlist/PlaylistDetails.jsx +++ b/ui/src/playlist/PlaylistDetails.jsx @@ -20,6 +20,7 @@ import { SizeField, isWritable, } from '../common' +import config from '../config' import subsonic from '../subsonic' import { REST_URL } from '../consts' import { httpClient } from '../dataProvider' @@ -134,7 +135,9 @@ const PlaylistDetails = (props) => { const imageUrl = subsonic.getCoverArtUrl(record, 300, true) const fullImageUrl = subsonic.getCoverArtUrl(record) - const canEdit = isWritable(record.ownerId) + const canEdit = + isWritable(record.ownerId) && + (config.enableCoverArtUpload || localStorage.getItem('role') === 'admin') // Reset image state when playlist changes useEffect(() => {