mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
feat: treat plugin playlists as read-only everywhere
Enforce read-only restrictions for plugin playlists (PluginID != "") consistently across backend and UI, matching the existing smart playlist behavior. The Subsonic buildOSPlaylist now uses IsReadOnly() to mark both smart and plugin playlists as readonly. Backend Update and REST adapter block name/comment changes on plugin playlists while still allowing public toggle and cover art. The UI adds isPluginPlaylist helper to canChangeTracks, disables name/comment inputs in PlaylistEdit, disables auto-import toggle in PlaylistList, and adds a server-side readonly filter to exclude both smart and plugin playlists from the "add to playlist" dialog.
This commit is contained in:
parent
0a67142f74
commit
a5fd18dc67
@ -162,6 +162,10 @@ func (s *playlists) Update(ctx context.Context, playlistID string,
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Plugin playlists allow public toggle and cover art, but block name/comment changes
|
||||
if pls.IsPluginPlaylist() && (name != nil || comment != nil) {
|
||||
return model.ErrNotAuthorized
|
||||
}
|
||||
return s.ds.WithTxImmediate(func(tx model.DataStore) error {
|
||||
repo := tx.Playlist(ctx)
|
||||
|
||||
|
||||
@ -210,6 +210,27 @@ var _ = Describe("Playlists", func() {
|
||||
err := ps.Update(ctx, "pls-smart", &newName, nil, nil, nil, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("denies name update on a plugin playlist", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
newName := "New Name"
|
||||
err := ps.Update(ctx, "pls-plugin", &newName, nil, nil, nil, nil)
|
||||
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
||||
})
|
||||
|
||||
It("denies comment update on a plugin playlist", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
newComment := "New Comment"
|
||||
err := ps.Update(ctx, "pls-plugin", nil, &newComment, nil, nil, nil)
|
||||
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
||||
})
|
||||
|
||||
It("allows public toggle on a plugin playlist", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
public := true
|
||||
err := ps.Update(ctx, "pls-plugin", nil, nil, &public, nil, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("AddTracks", func() {
|
||||
|
||||
@ -93,6 +93,10 @@ func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity
|
||||
if !usr.IsAdmin && entity.OwnerID != "" && entity.OwnerID != current.OwnerID {
|
||||
return rest.ErrPermissionDenied
|
||||
}
|
||||
// Plugin playlists allow public and ownership changes, but block name/comment
|
||||
if current.IsPluginPlaylist() && (entity.Name != current.Name || entity.Comment != current.Comment) {
|
||||
return rest.ErrPermissionDenied
|
||||
}
|
||||
// Apply ownership change (admin only)
|
||||
if entity.OwnerID != "" {
|
||||
current.OwnerID = entity.OwnerID
|
||||
|
||||
@ -102,6 +102,42 @@ var _ = Describe("REST Adapter", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("denies name change on plugin playlist", func() {
|
||||
mockPlsRepo.Data["pls-plugin"] = &model.Playlist{
|
||||
ID: "pls-plugin", Name: "Plugin PL", OwnerID: "user-1",
|
||||
PluginID: "test-plugin", PluginPlaylistID: "daily-mix",
|
||||
}
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
repo = ps.NewRepository(ctx).(rest.Persistable)
|
||||
pls := &model.Playlist{Name: "Changed Name", Comment: ""}
|
||||
err := repo.Update("pls-plugin", pls)
|
||||
Expect(err).To(Equal(rest.ErrPermissionDenied))
|
||||
})
|
||||
|
||||
It("denies comment change on plugin playlist", func() {
|
||||
mockPlsRepo.Data["pls-plugin"] = &model.Playlist{
|
||||
ID: "pls-plugin", Name: "Plugin PL", Comment: "", OwnerID: "user-1",
|
||||
PluginID: "test-plugin", PluginPlaylistID: "daily-mix",
|
||||
}
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
repo = ps.NewRepository(ctx).(rest.Persistable)
|
||||
pls := &model.Playlist{Name: "Plugin PL", Comment: "new comment"}
|
||||
err := repo.Update("pls-plugin", pls)
|
||||
Expect(err).To(Equal(rest.ErrPermissionDenied))
|
||||
})
|
||||
|
||||
It("allows public toggle on plugin playlist", func() {
|
||||
mockPlsRepo.Data["pls-plugin"] = &model.Playlist{
|
||||
ID: "pls-plugin", Name: "Plugin PL", OwnerID: "user-1",
|
||||
PluginID: "test-plugin", PluginPlaylistID: "daily-mix",
|
||||
}
|
||||
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
||||
repo = ps.NewRepository(ctx).(rest.Persistable)
|
||||
pls := &model.Playlist{Name: "Plugin PL", Public: true}
|
||||
err := repo.Update("pls-plugin", pls)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("allows admin to update any playlist", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
||||
repo = ps.NewRepository(ctx).(rest.Persistable)
|
||||
|
||||
@ -36,6 +36,10 @@ type Playlist struct {
|
||||
PluginPlaylistID string `structs:"plugin_playlist_id" json:"pluginPlaylistId,omitempty"`
|
||||
}
|
||||
|
||||
func (pls Playlist) IsReadOnly() bool {
|
||||
return pls.IsSmartPlaylist() || pls.IsPluginPlaylist()
|
||||
}
|
||||
|
||||
func (pls Playlist) IsSmartPlaylist() bool {
|
||||
return pls.Rules != nil && pls.Rules.Expression != nil
|
||||
}
|
||||
|
||||
@ -52,8 +52,9 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe
|
||||
r.ctx = ctx
|
||||
r.db = db
|
||||
r.registerModel(&model.Playlist{}, map[string]filterFunc{
|
||||
"q": playlistFilter,
|
||||
"smart": smartPlaylistFilter,
|
||||
"q": playlistFilter,
|
||||
"smart": smartPlaylistFilter,
|
||||
"readonly": readonlyPlaylistFilter,
|
||||
})
|
||||
r.setSortMappings(map[string]string{
|
||||
"owner_name": "owner_name",
|
||||
@ -75,6 +76,13 @@ func smartPlaylistFilter(string, any) Sqlizer {
|
||||
}
|
||||
}
|
||||
|
||||
func readonlyPlaylistFilter(string, any) Sqlizer {
|
||||
return And{
|
||||
smartPlaylistFilter("", nil),
|
||||
Or{Eq{"plugin_id": ""}, Eq{"plugin_id": nil}},
|
||||
}
|
||||
}
|
||||
|
||||
func (r *playlistRepository) userFilter() Sqlizer {
|
||||
user := loggedUser(r.ctx)
|
||||
if user.IsAdmin {
|
||||
|
||||
@ -165,10 +165,11 @@ func buildOSPlaylist(ctx context.Context, p model.Playlist) *responses.OpenSubso
|
||||
}
|
||||
pls := responses.OpenSubsonicPlaylist{}
|
||||
|
||||
if p.IsSmartPlaylist() {
|
||||
if p.IsReadOnly() {
|
||||
pls.Readonly = true
|
||||
|
||||
if p.EvaluatedAt != nil {
|
||||
// ValidUntil only applies to smart playlists
|
||||
if p.IsSmartPlaylist() && p.EvaluatedAt != nil {
|
||||
pls.ValidUntil = P(p.EvaluatedAt.Add(conf.Server.SmartPlaylistRefreshDelay))
|
||||
}
|
||||
} else {
|
||||
|
||||
@ -156,6 +156,41 @@ var _ = Describe("buildPlaylist", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("plugin playlist", func() {
|
||||
BeforeEach(func() {
|
||||
createdAt := time.Date(2023, 1, 15, 10, 30, 0, 0, time.UTC)
|
||||
updatedAt := time.Date(2023, 2, 20, 14, 45, 0, 0, time.UTC)
|
||||
|
||||
playlist = model.Playlist{
|
||||
ID: "pls-plugin",
|
||||
Name: "Daily Mix",
|
||||
Comment: "Generated by plugin",
|
||||
OwnerName: "admin",
|
||||
OwnerID: "1234",
|
||||
Public: false,
|
||||
SongCount: 5,
|
||||
Duration: 300,
|
||||
CreatedAt: createdAt,
|
||||
UpdatedAt: updatedAt,
|
||||
PluginID: "test-plugin",
|
||||
PluginPlaylistID: "daily-mix",
|
||||
}
|
||||
})
|
||||
|
||||
It("marks plugin playlist as readonly", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "1234", UserName: "admin"})
|
||||
result := router.buildPlaylist(ctx, playlist)
|
||||
Expect(result.Readonly).To(BeTrue())
|
||||
Expect(result.ValidUntil).To(BeNil())
|
||||
})
|
||||
|
||||
It("marks plugin playlist as readonly even for non-owner", func() {
|
||||
ctx = request.WithUser(ctx, model.User{ID: "other-user", UserName: "other"})
|
||||
result := router.buildPlaylist(ctx, playlist)
|
||||
Expect(result.Readonly).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("smart playlist", func() {
|
||||
evaluatedAt := time.Date(2023, 2, 20, 15, 45, 0, 0, time.UTC)
|
||||
validUntil := evaluatedAt.Add(5 * time.Second)
|
||||
|
||||
@ -11,5 +11,7 @@ export const isReadOnly = (ownerId) => {
|
||||
|
||||
export const isSmartPlaylist = (pls) => !!pls.rules
|
||||
|
||||
export const isPluginPlaylist = (pls) => !!pls.pluginId
|
||||
|
||||
export const canChangeTracks = (pls) =>
|
||||
isWritable(pls.ownerId) && !isSmartPlaylist(pls)
|
||||
isWritable(pls.ownerId) && !isSmartPlaylist(pls) && !isPluginPlaylist(pls)
|
||||
|
||||
@ -2,6 +2,7 @@ import {
|
||||
isWritable,
|
||||
isReadOnly,
|
||||
isSmartPlaylist,
|
||||
isPluginPlaylist,
|
||||
canChangeTracks,
|
||||
} from './playlistUtils'
|
||||
|
||||
@ -56,6 +57,18 @@ describe('playlistUtils', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('isPluginPlaylist', () => {
|
||||
it('returns true if playlist has pluginId', () => {
|
||||
const playlist = { pluginId: 'test-plugin' }
|
||||
expect(isPluginPlaylist(playlist)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false if playlist does not have pluginId', () => {
|
||||
const playlist = {}
|
||||
expect(isPluginPlaylist(playlist)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('canChangeTracks', () => {
|
||||
it('returns true if user is the owner and playlist is not smart', () => {
|
||||
localStorage.setItem('userId', 'user1')
|
||||
@ -74,5 +87,11 @@ describe('playlistUtils', () => {
|
||||
const playlist = { ownerId: 'user1', rules: [] }
|
||||
expect(canChangeTracks(playlist)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false if playlist is a plugin playlist', () => {
|
||||
localStorage.setItem('userId', 'user1')
|
||||
const playlist = { ownerId: 'user1', pluginId: 'test-plugin' }
|
||||
expect(canChangeTracks(playlist)).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -46,7 +46,7 @@ const createTestUtils = (mockDataProvider) =>
|
||||
data: mockIndexedData,
|
||||
list: {
|
||||
cachedRequests: {
|
||||
'{"pagination":{"page":1,"perPage":-1},"sort":{"field":"name","order":"ASC"},"filter":{"smart":false}}':
|
||||
'{"pagination":{"page":1,"perPage":-1},"sort":{"field":"name","order":"ASC"},"filter":{"readonly":false}}':
|
||||
{
|
||||
ids: ['sample-id1', 'sample-id2'],
|
||||
total: 2,
|
||||
|
||||
@ -264,7 +264,7 @@ export const SelectPlaylistInput = ({ onChange }) => {
|
||||
'playlist',
|
||||
{ page: 1, perPage: -1 },
|
||||
{ field: 'name', order: 'ASC' },
|
||||
{ smart: false },
|
||||
{ readonly: false },
|
||||
)
|
||||
|
||||
const options =
|
||||
|
||||
@ -53,7 +53,7 @@ const createTestComponent = (
|
||||
data: indexedData,
|
||||
list: {
|
||||
cachedRequests: {
|
||||
'{"pagination":{"page":1,"perPage":-1},"sort":{"field":"name","order":"ASC"},"filter":{"smart":false}}':
|
||||
'{"pagination":{"page":1,"perPage":-1},"sort":{"field":"name","order":"ASC"},"filter":{"readonly":false}}':
|
||||
{
|
||||
ids: Object.keys(indexedData),
|
||||
total: Object.keys(indexedData).length,
|
||||
|
||||
@ -11,7 +11,7 @@ import {
|
||||
ReferenceInput,
|
||||
SelectInput,
|
||||
} from 'react-admin'
|
||||
import { isWritable, Title } from '../common'
|
||||
import { isWritable, isPluginPlaylist, Title } from '../common'
|
||||
|
||||
const SyncFragment = ({ formData, variant, ...rest }) => {
|
||||
return (
|
||||
@ -33,12 +33,17 @@ const PlaylistEditForm = (props) => {
|
||||
const { permissions } = usePermissions()
|
||||
return (
|
||||
<SimpleForm redirect="list" variant={'outlined'} {...props}>
|
||||
<TextInput source="name" validate={required()} />
|
||||
<TextInput
|
||||
source="name"
|
||||
validate={required()}
|
||||
disabled={isPluginPlaylist(record)}
|
||||
/>
|
||||
<TextInput
|
||||
multiline
|
||||
minRows={3}
|
||||
source="comment"
|
||||
fullWidth
|
||||
disabled={isPluginPlaylist(record)}
|
||||
inputProps={{
|
||||
style: { resize: 'vertical' },
|
||||
}}
|
||||
|
||||
@ -24,6 +24,7 @@ import {
|
||||
List,
|
||||
Writable,
|
||||
isWritable,
|
||||
isPluginPlaylist,
|
||||
useSelectedFields,
|
||||
useResourceRefresh,
|
||||
} from '../common'
|
||||
@ -115,7 +116,7 @@ const ToggleAutoImport = ({ resource, source }) => {
|
||||
<Switch
|
||||
checked={record[source]}
|
||||
onClick={handleClick}
|
||||
disabled={!isWritable(record.ownerId)}
|
||||
disabled={!isWritable(record.ownerId) || isPluginPlaylist(record)}
|
||||
/>
|
||||
) : null
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user