refactor: streamline users integration tests and enhance plugin user management

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2026-01-03 14:58:24 -05:00
parent 08280eaea8
commit 5c1b8498ce
3 changed files with 269 additions and 357 deletions

View File

@ -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
}

View File

@ -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
}

View File

@ -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
}