From 5c1b8498ce9cdd6378940ed68878e3fa504ce646 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 3 Jan 2026 14:58:24 -0500 Subject: [PATCH] refactor: streamline users integration tests and enhance plugin user management Signed-off-by: Deluan --- plugins/host_users_test.go | 432 +++++++++++++++---------------------- plugins/manager.go | 106 ++++----- server/nativeapi/plugin.go | 88 +++++--- 3 files changed, 269 insertions(+), 357 deletions(-) diff --git a/plugins/host_users_test.go b/plugins/host_users_test.go index 87b12c9db..b46e94fdb 100644 --- a/plugins/host_users_test.go +++ b/plugins/host_users_test.go @@ -29,7 +29,7 @@ var _ = Describe("UsersService", Ordered, func() { ) BeforeEach(func() { - ctx = context.Background() + ctx = GinkgoT().Context() ds = &tests.MockDataStore{} }) @@ -144,84 +144,12 @@ var _ = Describe("UsersService", Ordered, func() { }) var _ = Describe("UsersService Integration", Ordered, func() { - var ( - manager *Manager - tmpDir string - ) + var manager *Manager BeforeAll(func() { - var err error - tmpDir, err = os.MkdirTemp("", "users-integration-test-*") - Expect(err).ToNot(HaveOccurred()) - - // Copy the test-users plugin - srcPath := filepath.Join(testdataDir, "test-users"+PackageExtension) - destPath := filepath.Join(tmpDir, "test-users"+PackageExtension) - data, err := os.ReadFile(srcPath) - Expect(err).ToNot(HaveOccurred()) - err = os.WriteFile(destPath, data, 0600) - Expect(err).ToNot(HaveOccurred()) - - // Compute SHA256 for the plugin - hash := sha256.Sum256(data) - hashHex := hex.EncodeToString(hash[:]) - - // Setup config - DeferCleanup(configtest.SetupConfig()) - conf.Server.Plugins.Enabled = true - conf.Server.Plugins.Folder = tmpDir - conf.Server.Plugins.AutoReload = false - conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") - - // Setup mock DataStore with pre-enabled plugin and users - mockPluginRepo := tests.CreateMockPluginRepo() - mockPluginRepo.Permitted = true - mockPluginRepo.SetData(model.Plugins{{ - ID: "test-users", - Path: destPath, - SHA256: hashHex, - Enabled: true, - AllUsers: true, // Allow all users - }}) - - mockUserRepo := tests.CreateMockUserRepo() - _ = mockUserRepo.Put(&model.User{ - ID: "user1", - UserName: "alice", - Name: "Alice Admin", - IsAdmin: true, - }) - _ = mockUserRepo.Put(&model.User{ - ID: "user2", - UserName: "bob", - Name: "Bob User", - IsAdmin: false, - }) - _ = mockUserRepo.Put(&model.User{ - ID: "user3", - UserName: "charlie", - Name: "Charlie User", - IsAdmin: false, - }) - - dataStore := &tests.MockDataStore{ - MockedPlugin: mockPluginRepo, - MockedUser: mockUserRepo, - } - - // Create and start manager - manager = &Manager{ - plugins: make(map[string]*plugin), - ds: dataStore, - subsonicRouter: http.NotFoundHandler(), - } - err = manager.Start(GinkgoT().Context()) - Expect(err).ToNot(HaveOccurred()) - - DeferCleanup(func() { - _ = manager.Stop() - _ = os.RemoveAll(tmpDir) - }) + var cleanup func() + manager, cleanup = setupUsersIntegrationManager(true, "") + DeferCleanup(cleanup) }) Describe("Plugin Loading", func() { @@ -236,52 +164,8 @@ var _ = Describe("UsersService Integration", Ordered, func() { }) Describe("Users Operations via Plugin", func() { - type testUsersInput struct { - Operation string `json:"operation"` - } - type user struct { - UserName string `json:"userName"` - Name string `json:"name"` - IsAdmin bool `json:"isAdmin"` - } - type testUsersOutput struct { - Users []user `json:"users,omitempty"` - Error *string `json:"error,omitempty"` - } - - callTestUsers := func(ctx context.Context, input testUsersInput) (*testUsersOutput, error) { - manager.mu.RLock() - p := manager.plugins["test-users"] - manager.mu.RUnlock() - - instance, err := p.instance(ctx) - if err != nil { - return nil, err - } - defer instance.Close(ctx) - - inputBytes, _ := json.Marshal(input) - _, outputBytes, err := instance.Call("nd_test_users", inputBytes) - if err != nil { - return nil, err - } - - var output testUsersOutput - if err := json.Unmarshal(outputBytes, &output); err != nil { - return nil, err - } - if output.Error != nil { - return nil, errors.New(*output.Error) - } - return &output, nil - } - It("should get all users when allUsers is true", func() { - ctx := GinkgoT().Context() - - output, err := callTestUsers(ctx, testUsersInput{ - Operation: "get_users", - }) + output, err := callTestUsersPlugin(GinkgoT().Context(), manager, testUsersInput{Operation: "get_users"}) Expect(err).ToNot(HaveOccurred()) Expect(output.Users).To(HaveLen(3)) @@ -294,15 +178,11 @@ var _ = Describe("UsersService Integration", Ordered, func() { }) It("should return correct user properties", func() { - ctx := GinkgoT().Context() - - output, err := callTestUsers(ctx, testUsersInput{ - Operation: "get_users", - }) + output, err := callTestUsersPlugin(GinkgoT().Context(), manager, testUsersInput{Operation: "get_users"}) Expect(err).ToNot(HaveOccurred()) // Find alice - var alice *user + var alice *testUser for i := range output.Users { if output.Users[i].UserName == "alice" { alice = &output.Users[i] @@ -317,15 +197,11 @@ var _ = Describe("UsersService Integration", Ordered, func() { }) It("should return non-admin user correctly", func() { - ctx := GinkgoT().Context() - - output, err := callTestUsers(ctx, testUsersInput{ - Operation: "get_users", - }) + output, err := callTestUsersPlugin(GinkgoT().Context(), manager, testUsersInput{Operation: "get_users"}) Expect(err).ToNot(HaveOccurred()) // Find bob - var bob *user + var bob *testUser for i := range output.Users { if output.Users[i].UserName == "bob" { bob = &output.Users[i] @@ -342,134 +218,17 @@ var _ = Describe("UsersService Integration", Ordered, func() { }) var _ = Describe("UsersService Integration with Specific Users", Ordered, func() { - var ( - manager *Manager - tmpDir string - ) + var manager *Manager BeforeAll(func() { - var err error - tmpDir, err = os.MkdirTemp("", "users-specific-test-*") - Expect(err).ToNot(HaveOccurred()) - - // Copy the test-users plugin - srcPath := filepath.Join(testdataDir, "test-users"+PackageExtension) - destPath := filepath.Join(tmpDir, "test-users"+PackageExtension) - data, err := os.ReadFile(srcPath) - Expect(err).ToNot(HaveOccurred()) - err = os.WriteFile(destPath, data, 0600) - Expect(err).ToNot(HaveOccurred()) - - // Compute SHA256 for the plugin - hash := sha256.Sum256(data) - hashHex := hex.EncodeToString(hash[:]) - - // Setup config - DeferCleanup(configtest.SetupConfig()) - conf.Server.Plugins.Enabled = true - conf.Server.Plugins.Folder = tmpDir - conf.Server.Plugins.AutoReload = false - conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") - - // Setup mock DataStore with specific allowed users (only user1 and user3) - mockPluginRepo := tests.CreateMockPluginRepo() - mockPluginRepo.Permitted = true - mockPluginRepo.SetData(model.Plugins{{ - ID: "test-users", - Path: destPath, - SHA256: hashHex, - Enabled: true, - AllUsers: false, - Users: `["user1", "user3"]`, // Only allow alice and charlie - }}) - - mockUserRepo := tests.CreateMockUserRepo() - _ = mockUserRepo.Put(&model.User{ - ID: "user1", - UserName: "alice", - Name: "Alice Admin", - IsAdmin: true, - }) - _ = mockUserRepo.Put(&model.User{ - ID: "user2", - UserName: "bob", - Name: "Bob User", - IsAdmin: false, - }) - _ = mockUserRepo.Put(&model.User{ - ID: "user3", - UserName: "charlie", - Name: "Charlie User", - IsAdmin: false, - }) - - dataStore := &tests.MockDataStore{ - MockedPlugin: mockPluginRepo, - MockedUser: mockUserRepo, - } - - // Create and start manager - manager = &Manager{ - plugins: make(map[string]*plugin), - ds: dataStore, - subsonicRouter: http.NotFoundHandler(), - } - err = manager.Start(GinkgoT().Context()) - Expect(err).ToNot(HaveOccurred()) - - DeferCleanup(func() { - _ = manager.Stop() - _ = os.RemoveAll(tmpDir) - }) + var cleanup func() + manager, cleanup = setupUsersIntegrationManager(false, `["user1", "user3"]`) + DeferCleanup(cleanup) }) Describe("Users Operations with Specific Allowed Users", func() { - type testUsersInput struct { - Operation string `json:"operation"` - } - type user struct { - UserName string `json:"userName"` - Name string `json:"name"` - IsAdmin bool `json:"isAdmin"` - } - type testUsersOutput struct { - Users []user `json:"users,omitempty"` - Error *string `json:"error,omitempty"` - } - - callTestUsers := func(ctx context.Context, input testUsersInput) (*testUsersOutput, error) { - manager.mu.RLock() - p := manager.plugins["test-users"] - manager.mu.RUnlock() - - instance, err := p.instance(ctx) - if err != nil { - return nil, err - } - defer instance.Close(ctx) - - inputBytes, _ := json.Marshal(input) - _, outputBytes, err := instance.Call("nd_test_users", inputBytes) - if err != nil { - return nil, err - } - - var output testUsersOutput - if err := json.Unmarshal(outputBytes, &output); err != nil { - return nil, err - } - if output.Error != nil { - return nil, errors.New(*output.Error) - } - return &output, nil - } - It("should only return allowed users", func() { - ctx := GinkgoT().Context() - - output, err := callTestUsers(ctx, testUsersInput{ - Operation: "get_users", - }) + output, err := callTestUsersPlugin(GinkgoT().Context(), manager, testUsersInput{Operation: "get_users"}) Expect(err).ToNot(HaveOccurred()) Expect(output.Users).To(HaveLen(2)) @@ -483,3 +242,164 @@ var _ = Describe("UsersService Integration with Specific Users", Ordered, func() }) }) }) + +// testUsersSetup contains common setup data for users integration tests +type testUsersSetup struct { + tmpDir string + destPath string + hashHex string +} + +// setupTestUsersPlugin creates a temporary directory with the test-users plugin and returns setup info +func setupTestUsersPlugin() (*testUsersSetup, error) { + tmpDir, err := os.MkdirTemp("", "users-integration-test-*") + if err != nil { + return nil, err + } + + // Copy the test-users plugin + srcPath := filepath.Join(testdataDir, "test-users"+PackageExtension) + destPath := filepath.Join(tmpDir, "test-users"+PackageExtension) + data, err := os.ReadFile(srcPath) + if err != nil { + _ = os.RemoveAll(tmpDir) + return nil, err + } + if err := os.WriteFile(destPath, data, 0600); err != nil { + _ = os.RemoveAll(tmpDir) + return nil, err + } + + // Compute SHA256 for the plugin + hash := sha256.Sum256(data) + hashHex := hex.EncodeToString(hash[:]) + + return &testUsersSetup{ + tmpDir: tmpDir, + destPath: destPath, + hashHex: hashHex, + }, nil +} + +// createTestUsers creates standard test users in the mock repo +func createTestUsers(mockUserRepo *tests.MockedUserRepo) { + _ = mockUserRepo.Put(&model.User{ + ID: "user1", + UserName: "alice", + Name: "Alice Admin", + IsAdmin: true, + }) + _ = mockUserRepo.Put(&model.User{ + ID: "user2", + UserName: "bob", + Name: "Bob User", + IsAdmin: false, + }) + _ = mockUserRepo.Put(&model.User{ + ID: "user3", + UserName: "charlie", + Name: "Charlie User", + IsAdmin: false, + }) +} + +// setupTestUsersConfig sets up common plugin configuration +func setupTestUsersConfig(tmpDir string) { + conf.Server.Plugins.Enabled = true + conf.Server.Plugins.Folder = tmpDir + conf.Server.Plugins.AutoReload = false + conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") +} + +// testUsersInput represents input for test-users plugin calls +type testUsersInput struct { + Operation string `json:"operation"` +} + +// testUser represents a user returned from test-users plugin +type testUser struct { + UserName string `json:"userName"` + Name string `json:"name"` + IsAdmin bool `json:"isAdmin"` +} + +// testUsersOutput represents output from test-users plugin +type testUsersOutput struct { + Users []testUser `json:"users,omitempty"` + Error *string `json:"error,omitempty"` +} + +// callTestUsersPlugin calls the test-users plugin with given input +func callTestUsersPlugin(ctx context.Context, manager *Manager, input testUsersInput) (*testUsersOutput, error) { + manager.mu.RLock() + p := manager.plugins["test-users"] + manager.mu.RUnlock() + + instance, err := p.instance(ctx) + if err != nil { + return nil, err + } + defer instance.Close(ctx) + + inputBytes, _ := json.Marshal(input) + _, outputBytes, err := instance.Call("nd_test_users", inputBytes) + if err != nil { + return nil, err + } + + var output testUsersOutput + if err := json.Unmarshal(outputBytes, &output); err != nil { + return nil, err + } + if output.Error != nil { + return nil, errors.New(*output.Error) + } + return &output, nil +} + +// setupUsersIntegrationManager creates a Manager for users integration tests with the given plugin settings +func setupUsersIntegrationManager(allUsers bool, allowedUsers string) (*Manager, func()) { + setup, err := setupTestUsersPlugin() + Expect(err).ToNot(HaveOccurred()) + + // Setup config + cleanupConfig := configtest.SetupConfig() + setupTestUsersConfig(setup.tmpDir) + + // Setup mock DataStore with pre-enabled plugin and users + mockPluginRepo := tests.CreateMockPluginRepo() + mockPluginRepo.Permitted = true + mockPluginRepo.SetData(model.Plugins{{ + ID: "test-users", + Path: setup.destPath, + SHA256: setup.hashHex, + Enabled: true, + AllUsers: allUsers, + Users: allowedUsers, + }}) + + mockUserRepo := tests.CreateMockUserRepo() + createTestUsers(mockUserRepo) + + dataStore := &tests.MockDataStore{ + MockedPlugin: mockPluginRepo, + MockedUser: mockUserRepo, + } + + // Create and start manager + manager := &Manager{ + plugins: make(map[string]*plugin), + ds: dataStore, + subsonicRouter: http.NotFoundHandler(), + } + err = manager.Start(GinkgoT().Context()) + Expect(err).ToNot(HaveOccurred()) + + cleanup := func() { + _ = manager.Stop() + _ = os.RemoveAll(setup.tmpDir) + cleanupConfig() + } + + return manager, cleanup +} diff --git a/plugins/manager.go b/plugins/manager.go index 1c7f1496d..0e42acaba 100644 --- a/plugins/manager.go +++ b/plugins/manager.go @@ -374,43 +374,9 @@ func (m *Manager) DisablePlugin(ctx context.Context, id string) error { // UpdatePluginConfig updates the configuration for a plugin. // If the plugin is enabled, it will be reloaded with the new config. func (m *Manager) UpdatePluginConfig(ctx context.Context, id, configJSON string) error { - if m.ds == nil { - return fmt.Errorf("datastore not configured") - } - - adminCtx := adminContext(ctx) - repo := m.ds.Plugin(adminCtx) - - plugin, err := repo.Get(id) - if err != nil { - return fmt.Errorf("getting plugin from DB: %w", err) - } - - wasEnabled := plugin.Enabled - - // Update config in DB - plugin.Config = configJSON - plugin.UpdatedAt = time.Now() - if err := repo.Put(plugin); err != nil { - return fmt.Errorf("updating plugin config in DB: %w", err) - } - - // Reload if enabled - if wasEnabled { - if err := m.unloadPlugin(id); err != nil { - log.Debug(ctx, "Plugin was not loaded", "plugin", id) - } - if err := m.loadPluginWithConfig(plugin); err != nil { - plugin.LastError = err.Error() - plugin.Enabled = false - _ = repo.Put(plugin) - return fmt.Errorf("reloading plugin with new config: %w", err) - } - } - - log.Info(ctx, "Updated plugin config", "plugin", id) - m.sendPluginRefreshEvent(ctx, id) - return nil + return m.updatePluginSettings(ctx, id, func(p *model.Plugin) { + p.Config = configJSON + }) } // UpdatePluginUsers updates the users permission settings for a plugin. @@ -418,6 +384,17 @@ func (m *Manager) UpdatePluginConfig(ctx context.Context, id, configJSON string) // If the plugin requires users permission and no users are configured (and allUsers is false), // the plugin will be automatically disabled. func (m *Manager) UpdatePluginUsers(ctx context.Context, id, usersJSON string, allUsers bool) error { + return m.updatePluginSettings(ctx, id, func(p *model.Plugin) { + p.Users = usersJSON + p.AllUsers = allUsers + }) +} + +// updatePluginSettings is a common implementation for updating plugin settings. +// The updateFn is called to apply the specific field updates to the plugin. +// If the plugin is enabled, it will be reloaded. If users permission is required +// but no longer satisfied, the plugin will be disabled. +func (m *Manager) updatePluginSettings(ctx context.Context, id string, updateFn func(*model.Plugin)) error { if m.ds == nil { return fmt.Errorf("datastore not configured") } @@ -432,26 +409,17 @@ func (m *Manager) UpdatePluginUsers(ctx context.Context, id, usersJSON string, a wasEnabled := plugin.Enabled - // Update users in DB - plugin.Users = usersJSON - plugin.AllUsers = allUsers + // Apply the specific updates + updateFn(plugin) plugin.UpdatedAt = time.Now() - // Check if plugin requires users permission and if the new settings are valid + // Check if plugin requires users permission and if it's still satisfied shouldDisable := false if wasEnabled { manifest, err := readManifest(plugin.Path) if err == nil && manifest.Permissions != nil && manifest.Permissions.Users != nil { - // Plugin requires users permission - check if it's still satisfied - if !allUsers { - if usersJSON == "" { - shouldDisable = true - } else { - var users []string - if err := json.Unmarshal([]byte(usersJSON), &users); err != nil || len(users) == 0 { - shouldDisable = true - } - } + if !hasValidUsersConfig(plugin.Users, plugin.AllUsers) { + shouldDisable = true } } } @@ -463,7 +431,7 @@ func (m *Manager) UpdatePluginUsers(ctx context.Context, id, usersJSON string, a } plugin.Enabled = false if err := repo.Put(plugin); err != nil { - return fmt.Errorf("updating plugin users in DB: %w", err) + return fmt.Errorf("updating plugin in DB: %w", err) } log.Info(ctx, "Disabled plugin due to users permission removal", "plugin", id) m.sendPluginRefreshEvent(ctx, id) @@ -471,7 +439,7 @@ func (m *Manager) UpdatePluginUsers(ctx context.Context, id, usersJSON string, a } if err := repo.Put(plugin); err != nil { - return fmt.Errorf("updating plugin users in DB: %w", err) + return fmt.Errorf("updating plugin in DB: %w", err) } // Reload if enabled @@ -483,11 +451,11 @@ func (m *Manager) UpdatePluginUsers(ctx context.Context, id, usersJSON string, a plugin.LastError = err.Error() plugin.Enabled = false _ = repo.Put(plugin) - return fmt.Errorf("reloading plugin with new users config: %w", err) + return fmt.Errorf("reloading plugin: %w", err) } } - log.Info(ctx, "Updated plugin users", "plugin", id) + log.Info(ctx, "Updated plugin settings", "plugin", id) m.sendPluginRefreshEvent(ctx, id) return nil } @@ -537,20 +505,26 @@ func (m *Manager) checkPermissionGates(p *model.Plugin) error { // Check users permission gate if manifest.Permissions != nil && manifest.Permissions.Users != nil { - if !p.AllUsers && p.Users == "" { + if !hasValidUsersConfig(p.Users, p.AllUsers) { return fmt.Errorf("users permission requires configuration: select users or enable 'all users' access") } - // Also check that Users JSON array is not empty if AllUsers is false - if !p.AllUsers { - var users []string - if err := json.Unmarshal([]byte(p.Users), &users); err != nil { - return fmt.Errorf("invalid users configuration: %w", err) - } - if len(users) == 0 { - return fmt.Errorf("users permission requires configuration: select at least one user or enable 'all users' access") - } - } } return nil } + +// hasValidUsersConfig checks if a plugin has valid users configuration. +// Returns true if allUsers is true, or if usersJSON contains at least one user. +func hasValidUsersConfig(usersJSON string, allUsers bool) bool { + if allUsers { + return true + } + if usersJSON == "" { + return false + } + var users []string + if err := json.Unmarshal([]byte(usersJSON), &users); err != nil { + return false + } + return len(users) > 0 +} diff --git a/server/nativeapi/plugin.go b/server/nativeapi/plugin.go index e1c531842..a16856e7d 100644 --- a/server/nativeapi/plugin.go +++ b/server/nativeapi/plugin.go @@ -77,48 +77,18 @@ func (api *Router) updatePlugin(w http.ResponseWriter, r *http.Request) { return } - // Handle config update first (if provided) + // Handle config update (if provided) if req.Config != nil { - // Validate JSON if not empty - if *req.Config != "" && !isValidJSON(*req.Config) { - http.Error(w, "Invalid JSON in config field", http.StatusBadRequest) - return - } - if err := api.pluginManager.UpdatePluginConfig(ctx, id, *req.Config); err != nil { - log.Error(ctx, "Error updating plugin config", "id", id, err) - http.Error(w, "Error updating plugin configuration: "+err.Error(), http.StatusInternalServerError) + if err := validateAndUpdateConfig(ctx, api.pluginManager, id, *req.Config, w); err != nil { + log.Error(ctx, "Error updating plugin config", err) return } } // Handle users permission update (if provided) if req.Users != nil || req.AllUsers != nil { - // Get current values if not provided in request - plugin, err := repo.Get(id) - if err != nil { - log.Error(ctx, "Error getting plugin for users update", "id", id, err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - return - } - - usersJSON := plugin.Users - allUsers := plugin.AllUsers - - if req.Users != nil { - // Validate users JSON if not empty - if *req.Users != "" && !isValidJSON(*req.Users) { - http.Error(w, "Invalid JSON in users field", http.StatusBadRequest) - return - } - usersJSON = *req.Users - } - if req.AllUsers != nil { - allUsers = *req.AllUsers - } - - if err := api.pluginManager.UpdatePluginUsers(ctx, id, usersJSON, allUsers); err != nil { - log.Error(ctx, "Error updating plugin users", "id", id, err) - http.Error(w, "Error updating plugin users: "+err.Error(), http.StatusInternalServerError) + if err := validateAndUpdateUsers(ctx, api.pluginManager, repo, id, req, w); err != nil { + log.Error(ctx, "Error updating plugin users", err) return } } @@ -168,3 +138,51 @@ func isValidJSON(s string) bool { var js json.RawMessage return json.Unmarshal([]byte(s), &js) == nil } + +// validateAndUpdateConfig validates the config JSON and updates the plugin. +// Returns an error if validation or update fails (error response already written). +func validateAndUpdateConfig(ctx context.Context, pm PluginManager, id, configJSON string, w http.ResponseWriter) error { + if configJSON != "" && !isValidJSON(configJSON) { + http.Error(w, "Invalid JSON in config field", http.StatusBadRequest) + return errors.New("invalid JSON") + } + if err := pm.UpdatePluginConfig(ctx, id, configJSON); err != nil { + log.Error(ctx, "Error updating plugin config", "id", id, err) + http.Error(w, "Error updating plugin configuration: "+err.Error(), http.StatusInternalServerError) + return err + } + return nil +} + +// validateAndUpdateUsers validates the users JSON and updates the plugin. +// Returns an error if validation or update fails (error response already written). +func validateAndUpdateUsers(ctx context.Context, pm PluginManager, repo model.PluginRepository, id string, req PluginUpdateRequest, w http.ResponseWriter) error { + // Get current values if not provided in request + plugin, err := repo.Get(id) + if err != nil { + log.Error(ctx, "Error getting plugin for users update", "id", id, err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return err + } + + usersJSON := plugin.Users + allUsers := plugin.AllUsers + + if req.Users != nil { + if *req.Users != "" && !isValidJSON(*req.Users) { + http.Error(w, "Invalid JSON in users field", http.StatusBadRequest) + return errors.New("invalid JSON") + } + usersJSON = *req.Users + } + if req.AllUsers != nil { + allUsers = *req.AllUsers + } + + if err := pm.UpdatePluginUsers(ctx, id, usersJSON, allUsers); err != nil { + log.Error(ctx, "Error updating plugin users", "id", id, err) + http.Error(w, "Error updating plugin users: "+err.Error(), http.StatusInternalServerError) + return err + } + return nil +}