diff --git a/server/subsonic/users.go b/server/subsonic/users.go index 0499b5ee0..39214eee2 100644 --- a/server/subsonic/users.go +++ b/server/subsonic/users.go @@ -4,26 +4,40 @@ import ( "net/http" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" ) +// buildUserResponse creates a User response object from a User model +func buildUserResponse(user model.User) responses.User { + userResponse := responses.User{ + Username: user.UserName, + AdminRole: user.IsAdmin, + Email: user.Email, + StreamRole: true, + ScrobblingEnabled: true, + DownloadRole: conf.Server.EnableDownloads, + ShareRole: conf.Server.EnableSharing, + } + + if conf.Server.Jukebox.Enabled { + userResponse.JukeboxRole = !conf.Server.Jukebox.AdminOnly || user.IsAdmin + } + + return userResponse +} + // TODO This is a placeholder. The real one has to read this info from a config file or the database func (api *Router) GetUser(r *http.Request) (*responses.Subsonic, error) { loggedUser, ok := request.UserFrom(r.Context()) if !ok { return nil, newError(responses.ErrorGeneric, "Internal error") } + response := newResponse() - response.User = &responses.User{} - response.User.Username = loggedUser.UserName - response.User.AdminRole = loggedUser.IsAdmin - response.User.Email = loggedUser.Email - response.User.StreamRole = true - response.User.ScrobblingEnabled = true - response.User.DownloadRole = conf.Server.EnableDownloads - response.User.ShareRole = conf.Server.EnableSharing - response.User.JukeboxRole = conf.Server.Jukebox.Enabled + user := buildUserResponse(loggedUser) + response.User = &user return response, nil } @@ -32,17 +46,8 @@ func (api *Router) GetUsers(r *http.Request) (*responses.Subsonic, error) { if !ok { return nil, newError(responses.ErrorGeneric, "Internal error") } - user := responses.User{} - user.Username = loggedUser.Name - user.AdminRole = loggedUser.IsAdmin - user.Email = loggedUser.Email - user.StreamRole = true - user.ScrobblingEnabled = true - user.DownloadRole = conf.Server.EnableDownloads - user.ShareRole = conf.Server.EnableSharing - if conf.Server.Jukebox.Enabled { - user.JukeboxRole = !conf.Server.Jukebox.AdminOnly || loggedUser.IsAdmin - } + + user := buildUserResponse(loggedUser) response := newResponse() response.Users = &responses.Users{User: []responses.User{user}} return response, nil diff --git a/server/subsonic/users_test.go b/server/subsonic/users_test.go new file mode 100644 index 000000000..d08462290 --- /dev/null +++ b/server/subsonic/users_test.go @@ -0,0 +1,96 @@ +package subsonic + +import ( + "context" + "net/http/httptest" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/server/subsonic/responses" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Users", func() { + var router *Router + var testUser model.User + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + router = &Router{} + + testUser = model.User{ + ID: "user123", + UserName: "testuser", + Name: "Test User", + Email: "test@example.com", + IsAdmin: false, + } + }) + + Describe("Happy path", func() { + It("should return consistent user data in both GetUser and GetUsers", func() { + conf.Server.EnableDownloads = true + conf.Server.EnableSharing = true + conf.Server.Jukebox.Enabled = false + + // Create request with user in context + req := httptest.NewRequest("GET", "/rest/getUser", nil) + ctx := request.WithUser(context.Background(), testUser) + req = req.WithContext(ctx) + + userResponse, err1 := router.GetUser(req) + usersResponse, err2 := router.GetUsers(req) + + Expect(err1).ToNot(HaveOccurred()) + Expect(err2).ToNot(HaveOccurred()) + + // Verify GetUser response structure + Expect(userResponse.Status).To(Equal(responses.StatusOK)) + Expect(userResponse.User).ToNot(BeNil()) + Expect(userResponse.User.Username).To(Equal("testuser")) + Expect(userResponse.User.Email).To(Equal("test@example.com")) + Expect(userResponse.User.AdminRole).To(BeFalse()) + Expect(userResponse.User.StreamRole).To(BeTrue()) + Expect(userResponse.User.ScrobblingEnabled).To(BeTrue()) + Expect(userResponse.User.DownloadRole).To(BeTrue()) + Expect(userResponse.User.ShareRole).To(BeTrue()) + + // Verify GetUsers response structure + Expect(usersResponse.Status).To(Equal(responses.StatusOK)) + Expect(usersResponse.Users).ToNot(BeNil()) + Expect(usersResponse.Users.User).To(HaveLen(1)) + + // Verify both methods return identical user data + singleUser := userResponse.User + userFromList := &usersResponse.Users.User[0] + + Expect(singleUser.Username).To(Equal(userFromList.Username)) + Expect(singleUser.Email).To(Equal(userFromList.Email)) + Expect(singleUser.AdminRole).To(Equal(userFromList.AdminRole)) + Expect(singleUser.StreamRole).To(Equal(userFromList.StreamRole)) + Expect(singleUser.ScrobblingEnabled).To(Equal(userFromList.ScrobblingEnabled)) + Expect(singleUser.DownloadRole).To(Equal(userFromList.DownloadRole)) + Expect(singleUser.ShareRole).To(Equal(userFromList.ShareRole)) + Expect(singleUser.JukeboxRole).To(Equal(userFromList.JukeboxRole)) + }) + }) + + DescribeTable("Jukebox role permissions", + func(jukeboxEnabled, adminOnly, isAdmin, expectedJukeboxRole bool) { + conf.Server.Jukebox.Enabled = jukeboxEnabled + conf.Server.Jukebox.AdminOnly = adminOnly + testUser.IsAdmin = isAdmin + + response := buildUserResponse(testUser) + Expect(response.JukeboxRole).To(Equal(expectedJukeboxRole)) + }, + Entry("jukebox disabled", false, false, false, false), + Entry("jukebox enabled, not admin-only, regular user", true, false, false, true), + Entry("jukebox enabled, not admin-only, admin user", true, false, true, true), + Entry("jukebox enabled, admin-only, regular user", true, true, false, false), + Entry("jukebox enabled, admin-only, admin user", true, true, true, true), + ) +})