navidrome/server/subsonic/playlists.go
Deluan Quintão 1f3034f022
fix(playlist): block track edits on synced playlists across all APIs (#5984)
* fix(playlist): block track edits on synced playlists across all APIs

A synced playlist's tracks come from its source file, so any track edit made
through the UI or an API was silently reverted on the next scan. Track mutations
funnel through two service guards, checkTracksEditable (incremental edits) and
Create (wholesale replace, used by Subsonic createPlaylist and Jellyfin's
replace path), which each duplicated the smart-playlist check. Both now consult
a shared model.Playlist.TracksEditable() predicate, so the native, Subsonic, and
Jellyfin paths are all locked: track edits return ErrNotAuthorized (403, or
Subsonic error 50) instead of being accepted and lost. Metadata-only edits
(name, comment, public, the sync flag itself) still go through checkWritable and
are unaffected. In the UI, a synced playlist's track list becomes read-only,
mirroring how smart playlists already behave.

* fix(playlist): return 409 Conflict for non-editable playlist track edits

The previous commit rejected track edits on smart and synced playlists with
ErrNotAuthorized (403). That conflates two different things: a 403 says the
caller lacks permission, but a synced or smart playlist's tracks are immutable
for everyone, including the owner and admins. It is a property of the resource,
not the caller.

Introduce ErrPlaylistNotEditable and return it from both track-edit guards. The
Native and Jellyfin APIs now map it to 409 Conflict; Subsonic maps it to error
50, the closest code it has (it has no read-only concept). The Native track
handlers previously mapped this rejection inconsistently (400 on add, 500 on
remove, 403 on reorder) through a new shared writePlaylistError helper. Genuine
authorization failures (non-owner, non-admin) still return ErrNotAuthorized.

* fix(playlist): surface synced read-only state in picker, Jellyfin, and OpenSubsonic

Follow-up to the track-edit lock: the read-only state was enforced but not
advertised consistently, so clients still offered edits that the server rejects.

- UI: the Add to Playlist picker filtered targets by isWritable only, offering
  synced playlists that then 409 on add. It now filters with canChangeTracks.
- Jellyfin: addToPlaylist/removeFromPlaylist hard-coded every error to 404, so a
  locked playlist reported "not found" instead of 409. They now return 409 for
  ErrPlaylistNotEditable while keeping the deliberate anti-probing 404 for every
  other error (a non-owner never reaches ErrPlaylistNotEditable, so 409 leaks
  nothing).
- OpenSubsonic: buildOSPlaylist marked only smart playlists readonly; owned
  synced playlists advertised readonly=false. Readonly now also covers
  !TracksEditable(), matching the existing smart-playlist treatment.

* fix(jellyfin): report CanEdit from playlist editability in permission probes

getPlaylistUsers and getPlaylistUser returned CanEdit: true unconditionally, so
Finamp (which probes this before showing edit controls) offered track editing on
synced/smart playlists whose add/remove requests now return 409. Both handlers
now fetch the playlist and set CanEdit from TracksEditable(), keeping the
deliberate non-owner looseness (CanEdit stays true for a normal playlist a
non-owner views) and mapping any lookup error to 404 like the sibling probes.

* fix(playlist): check ownership before editability when replacing tracks

Create checked TracksEditable() before ownership, so a non-owner replacing
another user's public smart/synced playlist (Jellyfin updatePlaylist with a
non-empty Ids list) received a 409 read-only conflict instead of a 403
authorization failure. The incremental guards check ownership first via
checkWritable; Create now matches that order. Subsonic is unaffected (both errors
map to code 50). Owners of their own smart/synced playlists still get the
read-only conflict.

* fix(jellyfin): return 403 for locked playlists, matching Jellyfin

Jellyfin itself refuses edits on its file-backed playlists with Forbid() (403):
PlaylistsController gates every mutation on OwnerUserId == caller or a share with
CanEdit, and playlists imported from .m3u files satisfy neither. Its CanEdit is
an ACL field, not a read-only marker, and Jellyfin core has no server-managed
playlist type at all.

Our Jellyfin routes exist to imitate that API, so ErrPlaylistNotEditable now maps
to 403 there instead of 409. The native API keeps 409 (a resource-state conflict
is the accurate REST answer where we define the contract) and Subsonic keeps
error 50, its closest code.

* chore(playlist): trim comments added by this branch

Several comments ran to three or four lines and carried rationale that belongs in
the commit history rather than the code: what Jellyfin does with its own
file-backed playlists, and restatements of the expressions directly below them.
Each block is now one or two lines covering only the non-obvious why.
2026-08-19 08:47:53 -04:00

180 lines
4.7 KiB
Go

package subsonic
import (
"context"
"errors"
"fmt"
"net/http"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/subsonic/responses"
"github.com/navidrome/navidrome/utils/req"
"github.com/navidrome/navidrome/utils/slice"
)
func (api *Router) GetPlaylists(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
allPls, err := api.playlists.GetAll(ctx, model.QueryOptions{Sort: "name"})
if err != nil {
log.Error(r, err)
return nil, err
}
response := newResponse()
response.Playlists = &responses.Playlists{
Playlist: slice.MapWithArg(allPls, ctx, api.buildPlaylist),
}
return response, nil
}
func (api *Router) GetPlaylist(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
p := req.Params(r)
id, err := p.String("id")
if err != nil {
return nil, err
}
return api.getPlaylist(ctx, id)
}
func (api *Router) getPlaylist(ctx context.Context, id string) (*responses.Subsonic, error) {
pls, err := api.playlists.GetWithTracks(ctx, id)
if errors.Is(err, model.ErrNotFound) {
log.Error(ctx, err.Error(), "id", id)
return nil, newError(responses.ErrorDataNotFound, "playlist not found")
}
if err != nil {
log.Error(ctx, err)
return nil, err
}
response := newResponse()
response.Playlist = &responses.PlaylistWithSongs{
Playlist: api.buildPlaylist(ctx, *pls),
}
response.Playlist.Entry = slice.MapWithArg(pls.MediaFiles(), ctx, childFromMediaFile)
return response, nil
}
func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
p := req.Params(r)
songIds := p.Strings("songId")
playlistId, _ := p.String("playlistId")
name, _ := p.String("name")
if playlistId == "" && name == "" {
return nil, errors.New("required parameter name is missing")
}
id, err := api.playlists.Create(ctx, playlistId, name, songIds)
if err != nil {
log.Error(r, err)
return nil, err
}
return api.getPlaylist(ctx, id)
}
func (api *Router) DeletePlaylist(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
id, err := p.String("id")
if err != nil {
return nil, err
}
err = api.playlists.Delete(r.Context(), id)
if errors.Is(err, model.ErrNotAuthorized) {
return nil, newError(responses.ErrorAuthorizationFail)
}
if err != nil {
log.Error(r, err)
return nil, err
}
return newResponse(), nil
}
func (api *Router) UpdatePlaylist(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
playlistId, err := p.String("playlistId")
if err != nil {
return nil, err
}
songsToAdd := p.Strings("songIdToAdd")
songIndexesToRemove, _ := p.Ints("songIndexToRemove")
var plsName *string
if s, err := p.String("name"); err == nil {
plsName = &s
}
comment := p.StringPtr("comment")
public := p.BoolPtr("public")
log.Debug(r, "Updating playlist", "id", playlistId)
if plsName != nil {
log.Trace(r, fmt.Sprintf("-- New Name: '%s'", *plsName))
}
log.Trace(r, fmt.Sprintf("-- Adding: '%v'", songsToAdd))
log.Trace(r, fmt.Sprintf("-- Removing: '%v'", songIndexesToRemove))
err = api.playlists.Update(r.Context(), playlistId, plsName, comment, public, songsToAdd, songIndexesToRemove)
if errors.Is(err, model.ErrNotAuthorized) {
return nil, newError(responses.ErrorAuthorizationFail)
}
if err != nil {
log.Error(r, "Error updating playlist", "id", playlistId, err)
return nil, err
}
return newResponse(), nil
}
func (api *Router) buildPlaylist(ctx context.Context, p model.Playlist) responses.Playlist {
pls := responses.Playlist{}
pls.Id = p.ID
pls.Name = p.Name
pls.SongCount = int32(p.SongCount)
pls.Duration = int32(p.Duration)
pls.Created = p.CreatedAt
if p.IsSmartPlaylist() {
if p.EvaluatedAt != nil {
pls.Changed = *p.EvaluatedAt
} else {
pls.Changed = time.Now()
}
} else {
pls.Changed = p.UpdatedAt
}
player, ok := request.PlayerFrom(ctx)
if ok && isClientInList(conf.Server.Subsonic.MinimalClients, player.Client) {
return pls
}
pls.Comment = p.Comment
pls.Owner = p.OwnerName
pls.Public = p.Public
pls.CoverArt = coverArtOrEmpty(p.CoverArtID(), p.ImageAbsent)
pls.OpenSubsonicPlaylist = buildOSPlaylist(ctx, p)
return pls
}
func buildOSPlaylist(ctx context.Context, p model.Playlist) *responses.OpenSubsonicPlaylist {
player, ok := request.PlayerFrom(ctx)
if ok && isClientInList(conf.Server.Subsonic.LegacyClients, player.Client) {
return nil
}
pls := responses.OpenSubsonicPlaylist{}
if p.IsSmartPlaylist() {
pls.Readonly = true
if p.EvaluatedAt != nil {
pls.ValidUntil = new(p.EvaluatedAt.Add(p.RefreshDelay()))
}
} else {
user, ok := request.UserFrom(ctx)
pls.Readonly = !ok || p.OwnerID != user.ID || !p.TracksEditable()
}
return &pls
}