mirror of
https://github.com/navidrome/navidrome.git
synced 2026-05-03 06:51:16 +00:00
feat(plugins): require users permission for PlaylistProvider and validate owner
PlaylistProvider capability now requires 'users' permission in the manifest (matching existing Scrobbler behavior) and validates that the resolved owner user ID is in the plugin's allowed users list before creating playlists.
This commit is contained in:
parent
657119fcb3
commit
9053a4ffe9
@ -65,6 +65,13 @@ func ValidateWithCapabilities(m *Manifest, capabilities []Capability) error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// PlaylistProvider capability requires users permission
|
||||||
|
if hasCapability(capabilities, CapabilityPlaylistProvider) {
|
||||||
|
if m.Permissions == nil || m.Permissions.Users == nil {
|
||||||
|
return fmt.Errorf("playlist provider capability requires 'users' permission to be declared in manifest")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Scheduler permission requires SchedulerCallback capability
|
// Scheduler permission requires SchedulerCallback capability
|
||||||
if m.Permissions != nil && m.Permissions.Scheduler != nil {
|
if m.Permissions != nil && m.Permissions.Scheduler != nil {
|
||||||
if !hasCapability(capabilities, CapabilityScheduler) {
|
if !hasCapability(capabilities, CapabilityScheduler) {
|
||||||
|
|||||||
@ -463,5 +463,48 @@ var _ = Describe("Manifest", func() {
|
|||||||
err := ValidateWithCapabilities(m, []Capability{})
|
err := ValidateWithCapabilities(m, []Capability{})
|
||||||
Expect(err).ToNot(HaveOccurred())
|
Expect(err).ToNot(HaveOccurred())
|
||||||
})
|
})
|
||||||
|
|
||||||
|
It("validates playlist provider capability with users permission", func() {
|
||||||
|
m := &Manifest{
|
||||||
|
Name: "Test",
|
||||||
|
Author: "Author",
|
||||||
|
Version: "1.0.0",
|
||||||
|
Permissions: &Permissions{
|
||||||
|
Users: &UsersPermission{},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
err := ValidateWithCapabilities(m, []Capability{CapabilityPlaylistProvider})
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
})
|
||||||
|
|
||||||
|
It("returns error when playlist provider capability without users permission", func() {
|
||||||
|
m := &Manifest{
|
||||||
|
Name: "Test",
|
||||||
|
Author: "Author",
|
||||||
|
Version: "1.0.0",
|
||||||
|
}
|
||||||
|
|
||||||
|
err := ValidateWithCapabilities(m, []Capability{CapabilityPlaylistProvider})
|
||||||
|
Expect(err).To(HaveOccurred())
|
||||||
|
Expect(err.Error()).To(ContainSubstring("playlist provider"))
|
||||||
|
Expect(err.Error()).To(ContainSubstring("users"))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("returns error when playlist provider has permissions but no users", func() {
|
||||||
|
m := &Manifest{
|
||||||
|
Name: "Test",
|
||||||
|
Author: "Author",
|
||||||
|
Version: "1.0.0",
|
||||||
|
Permissions: &Permissions{
|
||||||
|
Http: &HTTPPermission{},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
err := ValidateWithCapabilities(m, []Capability{CapabilityPlaylistProvider})
|
||||||
|
Expect(err).To(HaveOccurred())
|
||||||
|
Expect(err.Error()).To(ContainSubstring("playlist provider"))
|
||||||
|
Expect(err.Error()).To(ContainSubstring("users"))
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -3,6 +3,7 @@ package plugins
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"slices"
|
||||||
"strings"
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
@ -133,6 +134,14 @@ func (o *playlistSyncer) discoverAndSync() {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
ownerID := user.ID
|
ownerID := user.ID
|
||||||
|
|
||||||
|
// Validate that the plugin is permitted to create playlists for this user
|
||||||
|
if !o.plugin.allUsers && !slices.Contains(o.plugin.allowedUserIDs, ownerID) {
|
||||||
|
log.Error(ctx, "Plugin not permitted to create playlists for user", "plugin", o.pluginName,
|
||||||
|
"playlistID", info.ID, "username", info.OwnerUsername)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
dbID := id.NewHash(o.pluginName, info.ID, ownerID)
|
dbID := id.NewHash(o.pluginName, info.ID, ownerID)
|
||||||
o.syncPlaylist(info, dbID, ownerID)
|
o.syncPlaylist(info, dbID, ownerID)
|
||||||
}
|
}
|
||||||
|
|||||||
@ -173,6 +173,42 @@ var _ = Describe("PlaylistProvider", Ordered, func() {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
Describe("user permission validation", func() {
|
||||||
|
It("skips playlists for unauthorized users when AllUsers is false", func() {
|
||||||
|
// Create manager with restricted users — only "other-user" is allowed,
|
||||||
|
// but the plugin returns playlists for "admin" which resolves to "user-1"
|
||||||
|
restrictedManager, _ := createTestManagerWithPluginOverrides(nil,
|
||||||
|
map[string]pluginOverride{
|
||||||
|
"test-playlist-provider": {AllUsers: false, Users: `["other-user"]`},
|
||||||
|
},
|
||||||
|
"test-playlist-provider"+PackageExtension,
|
||||||
|
)
|
||||||
|
|
||||||
|
// No playlists should be created because "user-1" is not in allowed users
|
||||||
|
restrictedPlsRepo := restrictedManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo)
|
||||||
|
Consistently(func() int {
|
||||||
|
return restrictedPlsRepo.Len()
|
||||||
|
}, "500ms").Should(Equal(0))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("creates playlists for authorized users when AllUsers is false", func() {
|
||||||
|
// Create manager with restricted users — "user-1" is allowed,
|
||||||
|
// and the plugin returns playlists for "admin" which resolves to "user-1"
|
||||||
|
allowedManager, _ := createTestManagerWithPluginOverrides(nil,
|
||||||
|
map[string]pluginOverride{
|
||||||
|
"test-playlist-provider": {AllUsers: false, Users: `["user-1"]`},
|
||||||
|
},
|
||||||
|
"test-playlist-provider"+PackageExtension,
|
||||||
|
)
|
||||||
|
|
||||||
|
// Playlists should be created because "user-1" is in allowed users
|
||||||
|
allowedPlsRepo := allowedManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo)
|
||||||
|
Eventually(func() int {
|
||||||
|
return allowedPlsRepo.Len()
|
||||||
|
}).Should(BeNumerically(">=", 2))
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
Describe("stop", func() {
|
Describe("stop", func() {
|
||||||
It("stops the syncer when the manager stops", func() {
|
It("stops the syncer when the manager stops", func() {
|
||||||
stopManager, _ := createTestManagerWithPlugins(nil,
|
stopManager, _ := createTestManagerWithPlugins(nil,
|
||||||
|
|||||||
@ -82,10 +82,26 @@ func createTestManagerWithPlugins(pluginConfig map[string]map[string]string, plu
|
|||||||
return createTestManagerWithPluginsAndMetrics(pluginConfig, noopMetricsRecorder{}, plugins...)
|
return createTestManagerWithPluginsAndMetrics(pluginConfig, noopMetricsRecorder{}, plugins...)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// pluginOverride allows tests to override model.Plugin fields for specific plugins.
|
||||||
|
type pluginOverride struct {
|
||||||
|
AllUsers bool
|
||||||
|
Users string // JSON array of user IDs, e.g. `["user-1"]`
|
||||||
|
}
|
||||||
|
|
||||||
|
// createTestManagerWithPluginOverrides creates a new plugin Manager with the given plugin config,
|
||||||
|
// per-plugin overrides, and specified plugins.
|
||||||
|
func createTestManagerWithPluginOverrides(pluginConfig map[string]map[string]string, overrides map[string]pluginOverride, plugins ...string) (*Manager, string) {
|
||||||
|
return createTestManagerFull(pluginConfig, overrides, noopMetricsRecorder{}, plugins...)
|
||||||
|
}
|
||||||
|
|
||||||
// createTestManagerWithPluginsAndMetrics creates a new plugin Manager with the given plugin config,
|
// createTestManagerWithPluginsAndMetrics creates a new plugin Manager with the given plugin config,
|
||||||
// metrics recorder, and specified plugins. It creates a temp directory, copies the specified plugins, and starts the manager.
|
// metrics recorder, and specified plugins. It creates a temp directory, copies the specified plugins, and starts the manager.
|
||||||
// Returns the manager and temp directory path.
|
// Returns the manager and temp directory path.
|
||||||
func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]string, metrics PluginMetricsRecorder, plugins ...string) (*Manager, string) {
|
func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]string, metrics PluginMetricsRecorder, plugins ...string) (*Manager, string) {
|
||||||
|
return createTestManagerFull(pluginConfig, nil, metrics, plugins...)
|
||||||
|
}
|
||||||
|
|
||||||
|
func createTestManagerFull(pluginConfig map[string]map[string]string, overrides map[string]pluginOverride, metrics PluginMetricsRecorder, plugins ...string) (*Manager, string) {
|
||||||
// Create temp directory
|
// Create temp directory
|
||||||
tmpDir, err := os.MkdirTemp("", "plugins-test-*")
|
tmpDir, err := os.MkdirTemp("", "plugins-test-*")
|
||||||
Expect(err).ToNot(HaveOccurred())
|
Expect(err).ToNot(HaveOccurred())
|
||||||
@ -114,14 +130,21 @@ func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]s
|
|||||||
configJSON = string(configBytes)
|
configJSON = string(configBytes)
|
||||||
}
|
}
|
||||||
|
|
||||||
enabledPlugins = append(enabledPlugins, model.Plugin{
|
p := model.Plugin{
|
||||||
ID: pluginName,
|
ID: pluginName,
|
||||||
Path: destPath,
|
Path: destPath,
|
||||||
SHA256: hashHex,
|
SHA256: hashHex,
|
||||||
Enabled: true,
|
Enabled: true,
|
||||||
Config: configJSON,
|
Config: configJSON,
|
||||||
AllUsers: true, // Allow all users by default in tests
|
AllUsers: true, // Allow all users by default in tests
|
||||||
})
|
}
|
||||||
|
if overrides != nil {
|
||||||
|
if o, ok := overrides[pluginName]; ok {
|
||||||
|
p.AllUsers = o.AllUsers
|
||||||
|
p.Users = o.Users
|
||||||
|
}
|
||||||
|
}
|
||||||
|
enabledPlugins = append(enabledPlugins, p)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Setup config
|
// Setup config
|
||||||
|
|||||||
@ -2,5 +2,10 @@
|
|||||||
"name": "Test Playlist Provider",
|
"name": "Test Playlist Provider",
|
||||||
"author": "Navidrome Test",
|
"author": "Navidrome Test",
|
||||||
"version": "1.0.0",
|
"version": "1.0.0",
|
||||||
"description": "A test playlist provider plugin for integration testing"
|
"description": "A test playlist provider plugin for integration testing",
|
||||||
|
"permissions": {
|
||||||
|
"users": {
|
||||||
|
"reason": "Required for playlist ownership"
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user