From 5f54944366a30577e0157fb603af104d47c46a98 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 3 Jan 2026 18:16:16 -0500 Subject: [PATCH] feat: implement user permissions for SubsonicAPI and scrobbler plugins Signed-off-by: Deluan --- plugins/README.md | 26 ++- plugins/examples/nowplaying-py/manifest.json | 6 +- plugins/examples/webhook-rs/manifest.json | 3 + plugins/host_subsonicapi.go | 82 ++++---- plugins/host_subsonicapi_test.go | 111 ++++++----- plugins/manager.go | 15 +- plugins/manager_loader.go | 25 ++- plugins/manager_plugin.go | 16 +- plugins/manifest-schema.json | 14 +- plugins/manifest.go | 43 +++++ plugins/manifest_gen.go | 26 +-- plugins/manifest_test.go | 176 ++++++++++++++++++ plugins/package.go | 13 +- plugins/plugins_suite_test.go | 11 +- plugins/scrobbler_adapter.go | 29 ++- plugins/scrobbler_adapter_test.go | 35 ++++ plugins/testdata/test-scrobbler/manifest.json | 7 +- .../test-subsonicapi-plugin/manifest.json | 6 +- 18 files changed, 458 insertions(+), 186 deletions(-) diff --git a/plugins/README.md b/plugins/README.md index 1c03fa238..d1d33867b 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -220,6 +220,20 @@ Integrates with external scrobbling services. Export one or more of these functi | `nd_scrobbler_now_playing` | See below | (none) | Send now playing | | `nd_scrobbler_scrobble` | See below | (none) | Submit a scrobble | +> **Important:** Scrobbler plugins require the `users` permission in their manifest. Scrobble events are only sent for users assigned to the plugin through Navidrome's configuration. The `nd_scrobbler_is_authorized` function is called after the server-side user check passes. + +**Manifest permission:** + +```json +{ + "permissions": { + "users": { + "reason": "Receive scrobble events for users assigned to this plugin" + } + } +} +``` + **NowPlaying/Scrobble Input:** ```json @@ -647,16 +661,16 @@ Call Navidrome's Subsonic API internally (no network round-trip). { "permissions": { "subsonicapi": { - "reason": "Access library data", - "allowedUsernames": ["user1", "user2"], - "allowAdmins": false + "reason": "Access library data" + }, + "users": { + "reason": "Access user information for SubsonicAPI authorization" } } } ``` -- `allowedUsernames` – Restrict which users the plugin can act as (empty = any user) -- `allowAdmins` – Whether plugin can call API as admin users (default: false) +> **Important:** The `subsonicapi` permission requires the `users` permission. User access is controlled through the plugin's database configuration, not the manifest. Configure which users can use the plugin through the Navidrome UI or API. **Host function:** @@ -1039,7 +1053,7 @@ Plugins run in a secure WebAssembly sandbox provided by [Extism](https://extism. 3. **No Network Listeners** – Plugins cannot bind ports 4. **Config Isolation** – Plugins only receive their own config section 5. **Memory Limits** – Controlled by the WebAssembly runtime -6. **SubsonicAPI Restrictions** – Configurable user/admin access controls +6. **User-Scoped Authorization** – Plugins with `subsonicapi` or `scrobbler` capabilities can only access/receive events for users assigned to them through Navidrome's configuration. The `users` permission is required for these features. 7. **Users Permission** – Plugins requesting user access must be explicitly configured with allowed users; sensitive data (passwords, emails) is never exposed diff --git a/plugins/examples/nowplaying-py/manifest.json b/plugins/examples/nowplaying-py/manifest.json index c819ec8ce..6284c4617 100644 --- a/plugins/examples/nowplaying-py/manifest.json +++ b/plugins/examples/nowplaying-py/manifest.json @@ -9,8 +9,10 @@ "reason": "Schedule periodic checks for now playing status" }, "subsonicapi": { - "reason": "Query the getNowPlaying API endpoint", - "allowAdmins": true + "reason": "Query the getNowPlaying API endpoint" + }, + "users": { + "reason": "Access user information for SubsonicAPI authorization" } } } diff --git a/plugins/examples/webhook-rs/manifest.json b/plugins/examples/webhook-rs/manifest.json index 22ebe22de..97686596c 100644 --- a/plugins/examples/webhook-rs/manifest.json +++ b/plugins/examples/webhook-rs/manifest.json @@ -8,6 +8,9 @@ "http": { "reason": "To send webhook notifications to configured URLs", "allowedHosts": ["*"] + }, + "users": { + "reason": "Receive scrobble events for users assigned to this plugin" } } } diff --git a/plugins/host_subsonicapi.go b/plugins/host_subsonicapi.go index 11ada60d6..b7a98fcea 100644 --- a/plugins/host_subsonicapi.go +++ b/plugins/host_subsonicapi.go @@ -8,7 +8,6 @@ import ( "net/http/httptest" "net/url" "path" - "strings" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -27,19 +26,27 @@ const subsonicAPIVersion = "1.16.1" // URL Format: Only the path and query parameters are used - host/protocol are ignored. // Automatic Parameters: The service adds 'c' (client), 'v' (version), 'f' (format). type subsonicAPIServiceImpl struct { - pluginID string - router SubsonicRouter - ds model.DataStore - permissions *subsonicAPIPermissions + pluginID string + router SubsonicRouter + ds model.DataStore + allowedUserIDs []string // User IDs this plugin can access (from DB configuration) + allUsers bool // If true, plugin can access all users + userIDMap map[string]struct{} } // newSubsonicAPIService creates a new SubsonicAPIService for a plugin. -func newSubsonicAPIService(pluginID string, router SubsonicRouter, ds model.DataStore, permissions *SubsonicAPIPermission) host.SubsonicAPIService { +func newSubsonicAPIService(pluginID string, router SubsonicRouter, ds model.DataStore, allowedUserIDs []string, allUsers bool) host.SubsonicAPIService { + userIDMap := make(map[string]struct{}) + for _, id := range allowedUserIDs { + userIDMap[id] = struct{}{} + } return &subsonicAPIServiceImpl{ - pluginID: pluginID, - router: router, - ds: ds, - permissions: parseSubsonicAPIPermissions(permissions), + pluginID: pluginID, + router: router, + ds: ds, + allowedUserIDs: allowedUserIDs, + allUsers: allUsers, + userIDMap: userIDMap, } } @@ -107,46 +114,29 @@ func (s *subsonicAPIServiceImpl) Call(ctx context.Context, uri string) (string, } func (s *subsonicAPIServiceImpl) checkPermissions(ctx context.Context, username string) error { - if s.permissions == nil { + // If allUsers is true, allow any user + if s.allUsers { return nil } - if len(s.permissions.AllowedUsernames) > 0 { - if _, ok := s.permissions.usernameMap[strings.ToLower(username)]; !ok { - return fmt.Errorf("username %s is not allowed", username) - } + + // Must have at least one allowed user ID configured + if len(s.allowedUserIDs) == 0 { + return fmt.Errorf("no users configured for plugin %s", s.pluginID) } - if !s.permissions.AllowAdmins { - usr, err := s.ds.User(ctx).FindByUsername(username) - if err != nil { - if errors.Is(err, model.ErrNotFound) { - return fmt.Errorf("username %s not found", username) - } - return err - } - if usr.IsAdmin { - return fmt.Errorf("calling SubsonicAPI as admin user is not allowed") + + // Look up the user by username to get their ID + usr, err := s.ds.User(ctx).FindByUsername(username) + if err != nil { + if errors.Is(err, model.ErrNotFound) { + return fmt.Errorf("username %s not found", username) } + return err } + + // Check if the user's ID is in the allowed list + if _, ok := s.userIDMap[usr.ID]; !ok { + return fmt.Errorf("user %s is not authorized for this plugin", username) + } + return nil } - -type subsonicAPIPermissions struct { - AllowedUsernames []string - AllowAdmins bool - usernameMap map[string]struct{} -} - -func parseSubsonicAPIPermissions(data *SubsonicAPIPermission) *subsonicAPIPermissions { - if data == nil { - return &subsonicAPIPermissions{} - } - perms := &subsonicAPIPermissions{ - AllowedUsernames: data.AllowedUsernames, - AllowAdmins: data.AllowAdmins, - usernameMap: make(map[string]struct{}), - } - for _, u := range data.AllowedUsernames { - perms.usernameMap[strings.ToLower(u)] = struct{}{} - } - return perms -} diff --git a/plugins/host_subsonicapi_test.go b/plugins/host_subsonicapi_test.go index 1843c4e49..257332105 100644 --- a/plugins/host_subsonicapi_test.go +++ b/plugins/host_subsonicapi_test.go @@ -82,10 +82,11 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() { mockPluginRepo := dataStore.Plugin(GinkgoT().Context()).(*tests.MockPluginRepo) mockPluginRepo.Permitted = true enabledPlugin := model.Plugin{ - ID: "test-subsonicapi-plugin", - Path: pluginPath, - SHA256: hashHex, - Enabled: true, + ID: "test-subsonicapi-plugin", + Path: pluginPath, + SHA256: hashHex, + Enabled: true, + AllUsers: true, // Allow all users for test plugin } mockPluginRepo.SetData(model.Plugins{enabledPlugin}) @@ -208,24 +209,20 @@ var _ = Describe("SubsonicAPIService", func() { }) Describe("Permission Enforcement", func() { - Context("with AllowedUsernames restriction", func() { + Context("with specific user IDs allowed", func() { It("blocks users not in the allowed list", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowedUsernames: []string{"alloweduser"}, - AllowAdmins: true, - }) + // allowedUserIDs contains "user2", but testuser is "user1" + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{"user2"}, false) ctx := GinkgoT().Context() _, err := service.Call(ctx, "/ping?u=testuser") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("not allowed")) + Expect(err.Error()).To(ContainSubstring("not authorized")) }) It("allows users in the allowed list", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowedUsernames: []string{"alloweduser"}, - AllowAdmins: true, - }) + // allowedUserIDs contains "user2" which is "alloweduser" + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{"user2"}, false) ctx := GinkgoT().Context() response, err := service.Call(ctx, "/ping?u=alloweduser") @@ -233,48 +230,19 @@ var _ = Describe("SubsonicAPIService", func() { Expect(response).To(ContainSubstring("ok")) }) - It("is case-insensitive for usernames", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowedUsernames: []string{"AllowedUser"}, - AllowAdmins: true, - }) - - ctx := GinkgoT().Context() - response, err := service.Call(ctx, "/ping?u=alloweduser") - Expect(err).ToNot(HaveOccurred()) - Expect(response).To(ContainSubstring("ok")) - }) - }) - - Context("with AllowAdmins=false", func() { - It("blocks admin users", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowAdmins: false, - }) + It("blocks admin users when not in allowed list", func() { + // allowedUserIDs only contains "user1" (testuser), not "admin1" + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{"user1"}, false) ctx := GinkgoT().Context() _, err := service.Call(ctx, "/ping?u=adminuser") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("admin user is not allowed")) + Expect(err.Error()).To(ContainSubstring("not authorized")) }) - It("allows non-admin users", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowAdmins: false, - }) - - ctx := GinkgoT().Context() - response, err := service.Call(ctx, "/ping?u=testuser") - Expect(err).ToNot(HaveOccurred()) - Expect(response).To(ContainSubstring("ok")) - }) - }) - - Context("with AllowAdmins=true", func() { - It("allows admin users", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, &SubsonicAPIPermission{ - AllowAdmins: true, - }) + It("allows admin users when in allowed list", func() { + // allowedUserIDs contains "admin1" + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{"admin1"}, false) ctx := GinkgoT().Context() response, err := service.Call(ctx, "/ping?u=adminuser") @@ -283,21 +251,50 @@ var _ = Describe("SubsonicAPIService", func() { }) }) - Context("with no permissions set (nil)", func() { - It("allows all users", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, nil) + Context("with allUsers=true", func() { + It("allows all users regardless of allowed list", func() { + service := newSubsonicAPIService("test-plugin", router, dataStore, nil, true) ctx := GinkgoT().Context() response, err := service.Call(ctx, "/ping?u=testuser") Expect(err).ToNot(HaveOccurred()) Expect(response).To(ContainSubstring("ok")) }) + + It("allows admin users when allUsers is true", func() { + service := newSubsonicAPIService("test-plugin", router, dataStore, nil, true) + + ctx := GinkgoT().Context() + response, err := service.Call(ctx, "/ping?u=adminuser") + Expect(err).ToNot(HaveOccurred()) + Expect(response).To(ContainSubstring("ok")) + }) + }) + + Context("with no users configured", func() { + It("returns error when no users are configured", func() { + service := newSubsonicAPIService("test-plugin", router, dataStore, nil, false) + + ctx := GinkgoT().Context() + _, err := service.Call(ctx, "/ping?u=testuser") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no users configured")) + }) + + It("returns error for empty user list", func() { + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{}, false) + + ctx := GinkgoT().Context() + _, err := service.Call(ctx, "/ping?u=testuser") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no users configured")) + }) }) }) Describe("URL Handling", func() { It("returns error for missing username parameter", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, nil) + service := newSubsonicAPIService("test-plugin", router, dataStore, nil, true) ctx := GinkgoT().Context() _, err := service.Call(ctx, "/ping") @@ -306,7 +303,7 @@ var _ = Describe("SubsonicAPIService", func() { }) It("returns error for invalid URL", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, nil) + service := newSubsonicAPIService("test-plugin", router, dataStore, nil, true) ctx := GinkgoT().Context() _, err := service.Call(ctx, "://invalid") @@ -315,7 +312,7 @@ var _ = Describe("SubsonicAPIService", func() { }) It("extracts endpoint from path correctly", func() { - service := newSubsonicAPIService("test-plugin", router, dataStore, nil) + service := newSubsonicAPIService("test-plugin", router, dataStore, []string{"user1"}, false) ctx := GinkgoT().Context() _, err := service.Call(ctx, "/rest/ping.view?u=testuser") @@ -328,7 +325,7 @@ var _ = Describe("SubsonicAPIService", func() { Describe("Router Availability", func() { It("returns error when router is nil", func() { - service := newSubsonicAPIService("test-plugin", nil, dataStore, nil) + service := newSubsonicAPIService("test-plugin", nil, dataStore, nil, true) ctx := GinkgoT().Context() _, err := service.Call(ctx, "/ping?u=testuser") diff --git a/plugins/manager.go b/plugins/manager.go index 0e42acaba..779cf1485 100644 --- a/plugins/manager.go +++ b/plugins/manager.go @@ -259,10 +259,19 @@ func (m *Manager) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) { return nil, false } - // Create a new scrobbler adapter for this plugin + // Build user ID map for fast lookups + userIDMap := make(map[string]struct{}) + for _, id := range plugin.allowedUserIDs { + userIDMap[id] = struct{}{} + } + + // Create a new scrobbler adapter for this plugin with user authorization config return &ScrobblerPlugin{ - name: plugin.name, - plugin: plugin, + name: plugin.name, + plugin: plugin, + allowedUserIDs: plugin.allowedUserIDs, + allUsers: plugin.allUsers, + userIDMap: userIDMap, }, true } diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index 8a0e48fee..285d03bd4 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -50,8 +50,7 @@ var hostServices = []hostServiceEntry{ name: "SubsonicAPI", hasPermission: func(p *Permissions) bool { return p != nil && p.Subsonicapi != nil }, create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) { - perm := ctx.permissions.Subsonicapi - service := newSubsonicAPIService(ctx.pluginName, ctx.manager.subsonicRouter, ctx.manager.ds, perm) + service := newSubsonicAPIService(ctx.pluginName, ctx.manager.subsonicRouter, ctx.manager.ds, ctx.allowedUsers, ctx.allUsers) return host.RegisterSubsonicAPIHostFunctions(service), nil }, }, @@ -330,15 +329,23 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error { capabilities := detectCapabilities(instance) instance.Close(m.ctx) + // Validate manifest against detected capabilities + if err := ValidateWithCapabilities(pkg.Manifest, capabilities); err != nil { + compiled.Close(m.ctx) + return fmt.Errorf("manifest validation: %w", err) + } + m.mu.Lock() m.plugins[p.ID] = &plugin{ - name: p.ID, - path: p.Path, - manifest: pkg.Manifest, - compiled: compiled, - capabilities: capabilities, - closers: closers, - metrics: m.metrics, + name: p.ID, + path: p.Path, + manifest: pkg.Manifest, + compiled: compiled, + capabilities: capabilities, + closers: closers, + metrics: m.metrics, + allowedUserIDs: allowedUsers, + allUsers: p.AllUsers, } m.mu.Unlock() diff --git a/plugins/manager_plugin.go b/plugins/manager_plugin.go index ec801f345..08c0073b6 100644 --- a/plugins/manager_plugin.go +++ b/plugins/manager_plugin.go @@ -12,13 +12,15 @@ import ( // plugin represents a loaded plugin type plugin struct { - name string // Plugin name (from filename) - path string // Path to the wasm file - manifest *Manifest - compiled *extism.CompiledPlugin - capabilities []Capability // Auto-detected capabilities based on exported functions - closers []io.Closer // Cleanup functions to call on unload - metrics PluginMetricsRecorder + name string // Plugin name (from filename) + path string // Path to the wasm file + manifest *Manifest + compiled *extism.CompiledPlugin + capabilities []Capability // Auto-detected capabilities based on exported functions + closers []io.Closer // Cleanup functions to call on unload + metrics PluginMetricsRecorder + allowedUserIDs []string // User IDs this plugin can access (from DB configuration) + allUsers bool // If true, plugin can access all users } // instance creates a new plugin instance for the given context. diff --git a/plugins/manifest-schema.json b/plugins/manifest-schema.json index 7c7a5acb3..2eac3120f 100644 --- a/plugins/manifest-schema.json +++ b/plugins/manifest-schema.json @@ -147,24 +147,12 @@ }, "SubsonicAPIPermission": { "type": "object", - "description": "SubsonicAPI service permissions", + "description": "SubsonicAPI service permissions. Requires 'users' permission to be declared.", "additionalProperties": false, "properties": { "reason": { "type": "string", "description": "Explanation for why SubsonicAPI access is needed" - }, - "allowedUsernames": { - "type": "array", - "description": "List of usernames the plugin can pass as u. Any user if empty", - "items": { - "type": "string" - } - }, - "allowAdmins": { - "type": "boolean", - "description": "If false, reject calls where the u is an admin", - "default": false } } }, diff --git a/plugins/manifest.go b/plugins/manifest.go index 388cf9cfa..9b6a1ad97 100644 --- a/plugins/manifest.go +++ b/plugins/manifest.go @@ -1,7 +1,50 @@ package plugins +import ( + "encoding/json" + "fmt" +) + //go:generate go tool go-jsonschema -p plugins --struct-name-from-title -o manifest_gen.go manifest-schema.json +// ParseManifest unmarshals manifest JSON and performs cross-field validation. +// This is the single entry point for manifest parsing after reading from a file. +func ParseManifest(data []byte) (*Manifest, error) { + var m Manifest + if err := json.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("parsing manifest JSON: %w", err) + } + if err := m.Validate(); err != nil { + return nil, fmt.Errorf("validating manifest: %w", err) + } + return &m, nil +} + +// Validate performs cross-field validation that cannot be expressed in JSON Schema. +// This validates rules like "SubsonicAPI permission requires users permission". +func (m *Manifest) Validate() error { + // SubsonicAPI permission requires users permission + if m.Permissions != nil && m.Permissions.Subsonicapi != nil { + if m.Permissions.Users == nil { + return fmt.Errorf("'subsonicapi' permission requires 'users' permission to be declared") + } + } + return nil +} + +// ValidateWithCapabilities validates the manifest against detected capabilities. +// This must be called after WASM capability detection since Scrobbler capability +// is detected from exported functions, not manifest declarations. +func ValidateWithCapabilities(m *Manifest, capabilities []Capability) error { + // Scrobbler capability requires users permission + if hasCapability(capabilities, CapabilityScrobbler) { + if m.Permissions == nil || m.Permissions.Users == nil { + return fmt.Errorf("scrobbler capability requires 'users' permission to be declared in manifest") + } + } + return nil +} + // AllowedHosts returns a list of allowed hosts for HTTP requests. // Returns the hosts directly from the manifest's permissions. func (m *Manifest) AllowedHosts() []string { diff --git a/plugins/manifest_gen.go b/plugins/manifest_gen.go index 742ea9218..971e2ffc1 100644 --- a/plugins/manifest_gen.go +++ b/plugins/manifest_gen.go @@ -169,36 +169,12 @@ type SchedulerPermission struct { Reason *string `json:"reason,omitempty" yaml:"reason,omitempty" mapstructure:"reason,omitempty"` } -// SubsonicAPI service permissions +// SubsonicAPI service permissions. Requires 'users' permission to be declared. type SubsonicAPIPermission struct { - // If false, reject calls where the u is an admin - AllowAdmins bool `json:"allowAdmins,omitempty" yaml:"allowAdmins,omitempty" mapstructure:"allowAdmins,omitempty"` - - // List of usernames the plugin can pass as u. Any user if empty - AllowedUsernames []string `json:"allowedUsernames,omitempty" yaml:"allowedUsernames,omitempty" mapstructure:"allowedUsernames,omitempty"` - // Explanation for why SubsonicAPI access is needed Reason *string `json:"reason,omitempty" yaml:"reason,omitempty" mapstructure:"reason,omitempty"` } -// UnmarshalJSON implements json.Unmarshaler. -func (j *SubsonicAPIPermission) UnmarshalJSON(value []byte) error { - var raw map[string]interface{} - if err := json.Unmarshal(value, &raw); err != nil { - return err - } - type Plain SubsonicAPIPermission - var plain Plain - if err := json.Unmarshal(value, &plain); err != nil { - return err - } - if v, ok := raw["allowAdmins"]; !ok || v == nil { - plain.AllowAdmins = false - } - *j = SubsonicAPIPermission(plain) - return nil -} - // Enable experimental WebAssembly threads support type ThreadsFeature struct { // Explanation for why threads support is needed diff --git a/plugins/manifest_test.go b/plugins/manifest_test.go index 7941c5951..636bfe92e 100644 --- a/plugins/manifest_test.go +++ b/plugins/manifest_test.go @@ -216,4 +216,180 @@ var _ = Describe("Manifest", func() { Expect(m.HasExperimentalThreads()).To(BeTrue()) }) }) + + Describe("ParseManifest", func() { + It("parses a valid manifest with users permission", func() { + data := []byte(`{ + "name": "Test Plugin", + "author": "Test Author", + "version": "1.0.0", + "permissions": { + "subsonicapi": {}, + "users": {} + } + }`) + + m, err := ParseManifest(data) + Expect(err).ToNot(HaveOccurred()) + Expect(m.Name).To(Equal("Test Plugin")) + Expect(m.Permissions.Subsonicapi).ToNot(BeNil()) + Expect(m.Permissions.Users).ToNot(BeNil()) + }) + + It("returns error for invalid JSON", func() { + data := []byte(`{invalid}`) + + _, err := ParseManifest(data) + Expect(err).To(HaveOccurred()) + }) + + It("returns error when subsonicapi is requested without users permission", func() { + data := []byte(`{ + "name": "Test Plugin", + "author": "Test Author", + "version": "1.0.0", + "permissions": { + "subsonicapi": {} + } + }`) + + _, err := ParseManifest(data) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("subsonicapi")) + Expect(err.Error()).To(ContainSubstring("users")) + }) + }) + + Describe("Validate", func() { + It("validates manifest with subsonicapi and users permissions", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + Permissions: &Permissions{ + Subsonicapi: &SubsonicAPIPermission{}, + Users: &UsersPermission{}, + }, + } + + err := m.Validate() + Expect(err).ToNot(HaveOccurred()) + }) + + It("returns error when subsonicapi without users permission", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + Permissions: &Permissions{ + Subsonicapi: &SubsonicAPIPermission{}, + }, + } + + err := m.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("subsonicapi")) + }) + + It("validates manifest without subsonicapi", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + Permissions: &Permissions{ + Http: &HTTPPermission{}, + }, + } + + err := m.Validate() + Expect(err).ToNot(HaveOccurred()) + }) + + It("validates manifest without any permissions", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + } + + err := m.Validate() + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Describe("ValidateWithCapabilities", func() { + It("validates scrobbler capability with users permission", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + Permissions: &Permissions{ + Users: &UsersPermission{}, + }, + } + + err := ValidateWithCapabilities(m, []Capability{CapabilityScrobbler}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("returns error when scrobbler capability without users permission", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + } + + err := ValidateWithCapabilities(m, []Capability{CapabilityScrobbler}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("scrobbler")) + Expect(err.Error()).To(ContainSubstring("users")) + }) + + It("validates non-scrobbler capability without users permission", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + } + + err := ValidateWithCapabilities(m, []Capability{CapabilityMetadataAgent}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("validates multiple capabilities including scrobbler", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + Permissions: &Permissions{ + Users: &UsersPermission{}, + }, + } + + err := ValidateWithCapabilities(m, []Capability{CapabilityMetadataAgent, CapabilityScrobbler}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("validates with nil capabilities", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + } + + err := ValidateWithCapabilities(m, nil) + Expect(err).ToNot(HaveOccurred()) + }) + + It("validates with empty capabilities", func() { + m := &Manifest{ + Name: "Test", + Author: "Author", + Version: "1.0.0", + } + + err := ValidateWithCapabilities(m, []Capability{}) + Expect(err).ToNot(HaveOccurred()) + }) + }) }) diff --git a/plugins/package.go b/plugins/package.go index e3e0bf31f..475761231 100644 --- a/plugins/package.go +++ b/plugins/package.go @@ -2,7 +2,6 @@ package plugins import ( "archive/zip" - "encoding/json" "errors" "fmt" "io" @@ -62,13 +61,13 @@ func openPackage(ndpPath string) (*ndpPackage, error) { } // Parse and validate manifest - var manifest Manifest - if err := json.Unmarshal(manifestBytes, &manifest); err != nil { + manifest, err := ParseManifest(manifestBytes) + if err != nil { return nil, fmt.Errorf("parsing manifest: %w", err) } return &ndpPackage{ - Manifest: &manifest, + Manifest: manifest, WasmBytes: wasmBytes, }, nil } @@ -90,12 +89,12 @@ func readManifest(ndpPath string) (*Manifest, error) { return nil, fmt.Errorf("reading manifest: %w", err) } - var manifest Manifest - if err := json.Unmarshal(manifestBytes, &manifest); err != nil { + manifest, err := ParseManifest(manifestBytes) + if err != nil { return nil, fmt.Errorf("parsing manifest: %w", err) } - return &manifest, nil + return manifest, nil } } diff --git a/plugins/plugins_suite_test.go b/plugins/plugins_suite_test.go index 013e80b6b..459a7fac7 100644 --- a/plugins/plugins_suite_test.go +++ b/plugins/plugins_suite_test.go @@ -97,11 +97,12 @@ func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]s } enabledPlugins = append(enabledPlugins, model.Plugin{ - ID: pluginName, - Path: destPath, - SHA256: hashHex, - Enabled: true, - Config: configJSON, + ID: pluginName, + Path: destPath, + SHA256: hashHex, + Enabled: true, + Config: configJSON, + AllUsers: true, // Allow all users by default in tests }) } diff --git a/plugins/scrobbler_adapter.go b/plugins/scrobbler_adapter.go index c5cdc3454..4ade70d72 100644 --- a/plugins/scrobbler_adapter.go +++ b/plugins/scrobbler_adapter.go @@ -33,12 +33,23 @@ func init() { // ScrobblerPlugin is an adapter that wraps an Extism plugin and implements // the scrobbler.Scrobbler interface for scrobbling to external services. type ScrobblerPlugin struct { - name string - plugin *plugin + name string + plugin *plugin + allowedUserIDs []string // User IDs this plugin can access (from DB configuration) + allUsers bool // If true, plugin can access all users + userIDMap map[string]struct{} // Cached map for fast lookups } -// IsAuthorized checks if the user is authorized with this scrobbler +// IsAuthorized checks if the user is authorized with this scrobbler. +// First checks if the user is allowed to use this plugin (server-side), +// then delegates to the plugin for service-specific authorization. func (s *ScrobblerPlugin) IsAuthorized(ctx context.Context, userId string) bool { + // First check server-side authorization based on plugin configuration + if !s.isUserAllowed(userId) { + return false + } + + // Then delegate to the plugin for service-specific authorization username := getUsernameFromContext(ctx) input := capabilities.IsAuthorizedRequest{ Username: username, @@ -52,6 +63,18 @@ func (s *ScrobblerPlugin) IsAuthorized(ctx context.Context, userId string) bool return result } +// isUserAllowed checks if the given user ID is allowed to use this plugin. +func (s *ScrobblerPlugin) isUserAllowed(userId string) bool { + if s.allUsers { + return true + } + if len(s.allowedUserIDs) == 0 { + return false + } + _, ok := s.userIDMap[userId] + return ok +} + // NowPlaying sends a now playing notification to the scrobbler func (s *ScrobblerPlugin) NowPlaying(ctx context.Context, userId string, track *model.MediaFile, position int) error { username := getUsernameFromContext(ctx) diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go index 625e15036..9421837b1 100644 --- a/plugins/scrobbler_adapter_test.go +++ b/plugins/scrobbler_adapter_test.go @@ -71,6 +71,41 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { }) }) + Describe("isUserAllowed", func() { + It("returns true when allUsers is true", func() { + sp := &ScrobblerPlugin{allUsers: true} + Expect(sp.isUserAllowed("any-user")).To(BeTrue()) + }) + + It("returns false when allowedUserIDs is empty and allUsers is false", func() { + sp := &ScrobblerPlugin{allUsers: false, allowedUserIDs: []string{}} + Expect(sp.isUserAllowed("user-1")).To(BeFalse()) + }) + + It("returns false when allowedUserIDs is nil and allUsers is false", func() { + sp := &ScrobblerPlugin{allUsers: false} + Expect(sp.isUserAllowed("user-1")).To(BeFalse()) + }) + + It("returns true when user is in allowedUserIDs", func() { + sp := &ScrobblerPlugin{ + allUsers: false, + allowedUserIDs: []string{"user-1", "user-2"}, + userIDMap: map[string]struct{}{"user-1": {}, "user-2": {}}, + } + Expect(sp.isUserAllowed("user-1")).To(BeTrue()) + }) + + It("returns false when user is not in allowedUserIDs", func() { + sp := &ScrobblerPlugin{ + allUsers: false, + allowedUserIDs: []string{"user-1", "user-2"}, + userIDMap: map[string]struct{}{"user-1": {}, "user-2": {}}, + } + Expect(sp.isUserAllowed("user-3")).To(BeFalse()) + }) + }) + Describe("NowPlaying", func() { It("successfully calls the plugin", func() { track := &model.MediaFile{ diff --git a/plugins/testdata/test-scrobbler/manifest.json b/plugins/testdata/test-scrobbler/manifest.json index febb10bd0..6a6ec48ed 100644 --- a/plugins/testdata/test-scrobbler/manifest.json +++ b/plugins/testdata/test-scrobbler/manifest.json @@ -2,5 +2,10 @@ "name": "Test Scrobbler", "author": "Navidrome Test", "version": "1.0.0", - "description": "A test scrobbler plugin for integration testing" + "description": "A test scrobbler plugin for integration testing", + "permissions": { + "users": { + "reason": "Receive scrobble events for users assigned to this plugin" + } + } } diff --git a/plugins/testdata/test-subsonicapi-plugin/manifest.json b/plugins/testdata/test-subsonicapi-plugin/manifest.json index e4168608d..027e17761 100644 --- a/plugins/testdata/test-subsonicapi-plugin/manifest.json +++ b/plugins/testdata/test-subsonicapi-plugin/manifest.json @@ -5,8 +5,10 @@ "description": "Test plugin for SubsonicAPI host function", "permissions": { "subsonicapi": { - "reason": "Testing SubsonicAPI access", - "allowAdmins": true + "reason": "Testing SubsonicAPI access" + }, + "users": { + "reason": "Access user information for SubsonicAPI authorization" } } }