fix(subsonic): getUser answers an admin's lookup with the requested user (#6292)

An admin asking for another username got their own name and roles back.
The handler now looks the user up by exact username and answers with
that user's payload; an unknown username is ErrDataNotFound. A
non-admin asking about someone else is still ErrNotAuthorized.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/subsonic/subsonic.go

@@ -13,6 +13,7 @@ import (
 	"github.com/jackc/pgx/v5/pgxpool"
 
 	"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/playevents"
 )
 
@@ -26,6 +27,7 @@ import (
 func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, cfg Config, events *playevents.Writer, hops func() int) {
 	b := &browseHandlers{pool: pool}
 	m := newMediaHandlers(pool, events, logger)
+	u := &userHandlers{lookup: dbq.New(pool).GetUserByUsername}
 	r.Route("/rest", func(sub chi.Router) {
 		sub.Use(Middleware(pool, cfg, auth.NewLoginGuard(), hops))
 		register(sub, "/ping", handlePing)
@@ -46,7 +48,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, cfg Config, ev
 		register(sub, "/unstar", m.handleUnstar)
 		register(sub, "/getStarred", m.handleGetStarred)
 		register(sub, "/getStarred2", m.handleGetStarred2)
-		register(sub, "/getUser", handleGetUser)
+		register(sub, "/getUser", u.getUser)
 
 		// Subsonic clients expect every /rest/* miss to come back as a
 		// Subsonic-shaped error envelope, not chi's plain-text 404. Without

internal/subsonic/user.go

@@ -1,9 +1,13 @@
 package subsonic
 
 import (
+	"context"
 	"encoding/xml"
+	"errors"
 	"net/http"
 
+	"github.com/jackc/pgx/v5"
+
 	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
 )
 
@@ -38,26 +42,41 @@ type User struct {
 	VideoConversionRole bool     `json:"videoConversionRole" xml:"videoConversionRole,attr"`
 }
 
-// handleGetUser returns the authenticated user when no username is provided
-// or when the requested username matches; admins may look up any user. Per
-// spec a non-admin asking about another user gets ErrNotAuthorized.
-func handleGetUser(w http.ResponseWriter, r *http.Request) {
+// userHandlers serves getUser. lookup finds a user by exact username; it is
+// a field so tests can answer without a database.
+type userHandlers struct {
+	lookup func(ctx context.Context, username string) (dbq.User, error)
+}
+
+// getUser returns the authenticated user when no username is provided or
+// when the requested username matches; admins may look up any user, and an
+// unknown username is ErrDataNotFound. Per spec a non-admin asking about
+// another user gets ErrNotAuthorized.
+func (h *userHandlers) getUser(w http.ResponseWriter, r *http.Request) {
 	caller, ok := UserFromContext(r.Context())
 	if !ok {
 		WriteFail(w, r, ErrNotAuthorized, "Not authenticated")
 		return
 	}
 	requested := r.URL.Query().Get("username")
-	if requested != "" && requested != caller.Username && !caller.IsAdmin {
+	if requested == "" || requested == caller.Username {
+		Write(w, r, UserResponse{Envelope: NewEnvelope("ok"), User: userPayload(caller)})
+		return
+	}
+	if !caller.IsAdmin {
 		WriteFail(w, r, ErrNotAuthorized, "Not authorized to view other users")
 		return
 	}
-	// No per-user lookup: an admin asking for another username is answered
-	// with the caller's own roles.
-	Write(w, r, UserResponse{
-		Envelope: NewEnvelope("ok"),
-		User:     userPayload(caller),
-	})
+	target, err := h.lookup(r.Context(), requested)
+	if errors.Is(err, pgx.ErrNoRows) {
+		WriteFail(w, r, ErrDataNotFound, "User not found")
+		return
+	}
+	if err != nil {
+		WriteFail(w, r, ErrGeneric, "Failed to look up user")
+		return
+	}
+	Write(w, r, UserResponse{Envelope: NewEnvelope("ok"), User: userPayload(target)})
 }
 
 func userPayload(u dbq.User) User {

internal/subsonic/user_test.go

@@ -8,6 +8,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/jackc/pgx/v5"
+
 	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
 )
 
@@ -51,6 +53,41 @@ func TestHandleGetUser_NonAdminCannotLookupOthers(t *testing.T) {
 	}
 }
 
+func TestHandleGetUser_AdminLooksUpAnotherUser(t *testing.T) {
+	admin := dbq.User{Username: "admin", IsAdmin: true}
+	bob := dbq.User{Username: "bob", IsAdmin: false}
+	_, body := callGetUser(t, admin, "bob", bob)
+	if body.Resp.Status != "ok" {
+		t.Fatalf("status = %q, want ok (err=%+v)", body.Resp.Status, body.Resp.Error)
+	}
+	if body.Resp.User.Username != "bob" {
+		t.Errorf("username = %q, want bob: the answer must describe the user asked about, not the caller", body.Resp.User.Username)
+	}
+	if body.Resp.User.AdminRole || body.Resp.User.SettingsRole {
+		t.Errorf("bob is not an admin, but the answer carries admin/settings roles %v/%v",
+			body.Resp.User.AdminRole, body.Resp.User.SettingsRole)
+	}
+}
+
+func TestHandleGetUser_AdminLooksUpUnknownUser(t *testing.T) {
+	admin := dbq.User{Username: "admin", IsAdmin: true}
+	_, body := callGetUser(t, admin, "nobody")
+	if body.Resp.Status != "failed" || body.Resp.Error == nil ||
+		body.Resp.Error.Code != ErrDataNotFound {
+		t.Errorf("expected ErrDataNotFound, got status=%q err=%+v",
+			body.Resp.Status, body.Resp.Error)
+	}
+}
+
+func TestHandleGetUser_OwnUsernameNeedsNoLookup(t *testing.T) {
+	// No known users: a lookup would fail, so success proves none was made.
+	user := dbq.User{Username: "alice", IsAdmin: false}
+	_, body := callGetUser(t, user, "alice")
+	if body.Resp.Status != "ok" || body.Resp.User.Username != "alice" {
+		t.Errorf("status=%q username=%q, want ok/alice", body.Resp.Status, body.Resp.User.Username)
+	}
+}
+
 type getUserPayload struct {
 	Resp struct {
 		Status string `json:"status"`
@@ -59,8 +96,18 @@ type getUserPayload struct {
 	} `json:"subsonic-response"`
 }
 
-func callGetUser(t *testing.T, user dbq.User, requestedUsername string) (*httptest.ResponseRecorder, getUserPayload) {
+// callGetUser runs getUser as caller, with the users in known as the only
+// accounts a lookup can find.
+func callGetUser(t *testing.T, user dbq.User, requestedUsername string, known ...dbq.User) (*httptest.ResponseRecorder, getUserPayload) {
 	t.Helper()
+	h := &userHandlers{lookup: func(_ context.Context, username string) (dbq.User, error) {
+		for _, u := range known {
+			if u.Username == username {
+				return u, nil
+			}
+		}
+		return dbq.User{}, pgx.ErrNoRows
+	}}
 	url := "/rest/getUser.view?f=json"
 	if requestedUsername != "" {
 		url += "&username=" + requestedUsername
@@ -68,7 +115,7 @@ func callGetUser(t *testing.T, user dbq.User, requestedUsername string) (*httpte
 	req := httptest.NewRequest("GET", url, nil)
 	req = req.WithContext(context.WithValue(req.Context(), userCtxKey, user))
 	w := httptest.NewRecorder()
-	handleGetUser(w, req)
+	h.getUser(w, req)
 
 	var body getUserPayload
 	if err := json.NewDecoder(strings.NewReader(w.Body.String())).Decode(&body); err != nil {