fix(subsonic): require admin access for Subsonic management endpoints (#5510)

* fix: require admin for radio mutations

Subsonic internet radio station mutation endpoints are admin-only in the Subsonic and OpenSubsonic specs, but the router only required an authenticated player. Add a reusable Subsonic admin middleware and apply it to create, update, and delete radio routes while leaving the list endpoint available to authenticated users. Cover the middleware and router behavior with unit and e2e tests.

* fix: streamline admin-only routes for internet radio station management

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: use admin-only middleware for starting scans

Signed-off-by: Deluan <deluan@navidrome.org>

* test: align start scan authorization coverage

StartScan authorization now lives in the shared Subsonic admin middleware instead of the handler. Remove the obsolete direct handler unit assertion so the package tests reflect the route-level guard covered by middleware and e2e tests.

* fix: require admin for getUsers

The Subsonic getUsers endpoint exposes user-list semantics and should use the same shared admin middleware as other admin-only management endpoints. Apply the route-level guard while leaving getUser unchanged, and update the multi-user e2e coverage to expect regular users to receive an authorization failure.

* test: cover admin-only Subsonic access

Add e2e coverage that admins can still call getUsers after the route-level guard and that regular authenticated users can still list internet radio stations. These cases capture the access boundaries raised during PR review.

---------

Signed-off-by: Deluan <deluan@navidrome.org>

server/e2e/subsonic_multiuser_test.go

@@ -60,15 +60,23 @@ var _ = Describe("Multi-User Isolation", Ordered, func() {
 		})
 	})
 
-	Describe("getUsers for regular user", func() {
-		It("returns only the requesting user's info", func() {
-			resp := doReqWithUser(regularUser, "getUsers")
+	Describe("getUsers authorization", func() {
+		It("succeeds for admin user", func() {
+			resp := doReqWithUser(adminUser, "getUsers")
 
 			Expect(resp.Status).To(Equal(responses.StatusOK))
 			Expect(resp.Users).ToNot(BeNil())
 			Expect(resp.Users.User).To(HaveLen(1))
-			Expect(resp.Users.User[0].Username).To(Equal("regular"))
-			Expect(resp.Users.User[0].AdminRole).To(BeFalse())
+			Expect(resp.Users.User[0].Username).To(Equal(adminUser.UserName))
+			Expect(resp.Users.User[0].AdminRole).To(BeTrue())
+		})
+
+		It("fails for regular user because getUsers is admin-only", func() {
+			resp := doReqWithUser(regularUser, "getUsers")
+
+			Expect(resp.Status).To(Equal(responses.StatusFailed))
+			Expect(resp.Error).ToNot(BeNil())
+			Expect(resp.Error.Code).To(Equal(responses.ErrorAuthorizationFail))
 		})
 	})
 })

server/e2e/subsonic_radio_test.go

@@ -46,6 +46,30 @@ var _ = Describe("Internet Radio Endpoints", Ordered, func() {
 		Expect(radioID).ToNot(BeEmpty())
 	})
 
+	It("getInternetRadioStations remains available to regular users", func() {
+		resp := doReqWithUser(regularUser, "getInternetRadioStations")
+
+		Expect(resp.Status).To(Equal(responses.StatusOK))
+		Expect(resp.InternetRadioStations).ToNot(BeNil())
+		Expect(resp.InternetRadioStations.Radios).To(HaveLen(1))
+		Expect(resp.InternetRadioStations.Radios[0].Name).To(Equal("Test Radio"))
+	})
+
+	It("createInternetRadioStation requires admin user", func() {
+		resp := doReqWithUser(regularUser, "createInternetRadioStation",
+			"streamUrl", "https://stream.example.com/hacked",
+			"name", "Hacked Radio",
+		)
+
+		Expect(resp.Status).To(Equal(responses.StatusFailed))
+		Expect(resp.Error).ToNot(BeNil())
+		Expect(resp.Error.Code).To(Equal(responses.ErrorAuthorizationFail))
+
+		resp = doReq("getInternetRadioStations")
+		Expect(resp.InternetRadioStations.Radios).To(HaveLen(1))
+		Expect(resp.InternetRadioStations.Radios[0].Name).To(Equal("Test Radio"))
+	})
+
 	It("updateInternetRadioStation modifies the station", func() {
 		resp := doReq("updateInternetRadioStation",
 			"id", radioID,
@@ -64,6 +88,35 @@ var _ = Describe("Internet Radio Endpoints", Ordered, func() {
 		Expect(resp.InternetRadioStations.Radios[0].HomepageUrl).To(Equal("https://updated.example.com"))
 	})
 
+	It("updateInternetRadioStation requires admin user", func() {
+		resp := doReqWithUser(regularUser, "updateInternetRadioStation",
+			"id", radioID,
+			"streamUrl", "https://stream.example.com/hacked",
+			"name", "Hacked Radio",
+		)
+
+		Expect(resp.Status).To(Equal(responses.StatusFailed))
+		Expect(resp.Error).ToNot(BeNil())
+		Expect(resp.Error.Code).To(Equal(responses.ErrorAuthorizationFail))
+
+		resp = doReq("getInternetRadioStations")
+		Expect(resp.InternetRadioStations.Radios).To(HaveLen(1))
+		Expect(resp.InternetRadioStations.Radios[0].Name).To(Equal("Updated Radio"))
+		Expect(resp.InternetRadioStations.Radios[0].StreamUrl).To(Equal("https://stream.example.com/radio-v2"))
+	})
+
+	It("deleteInternetRadioStation requires admin user", func() {
+		resp := doReqWithUser(regularUser, "deleteInternetRadioStation", "id", radioID)
+
+		Expect(resp.Status).To(Equal(responses.StatusFailed))
+		Expect(resp.Error).ToNot(BeNil())
+		Expect(resp.Error.Code).To(Equal(responses.ErrorAuthorizationFail))
+
+		resp = doReq("getInternetRadioStations")
+		Expect(resp.InternetRadioStations.Radios).To(HaveLen(1))
+		Expect(resp.InternetRadioStations.Radios[0].ID).To(Equal(radioID))
+	})
+
 	It("deleteInternetRadioStation removes it", func() {
 		resp := doReq("deleteInternetRadioStation", "id", radioID)
 

server/subsonic/api.go

@@ -171,12 +171,12 @@ func (api *Router) routes() http.Handler {
 		r.Group(func(r chi.Router) {
 			r.Use(getPlayer(api.players))
 			h(r, "getUser", api.GetUser)
-			h(r, "getUsers", api.GetUsers)
+			h(r.With(adminOnly), "getUsers", api.GetUsers)
 		})
 		r.Group(func(r chi.Router) {
 			r.Use(getPlayer(api.players))
 			h(r, "getScanStatus", api.GetScanStatus)
-			h(r, "startScan", api.StartScan)
+			h(r.With(adminOnly), "startScan", api.StartScan)
 		})
 		r.Group(func(r chi.Router) {
 			r.Use(getPlayer(api.players))
@@ -195,10 +195,13 @@ func (api *Router) routes() http.Handler {
 		})
 		r.Group(func(r chi.Router) {
 			r.Use(getPlayer(api.players))
-			h(r, "createInternetRadioStation", api.CreateInternetRadio)
-			h(r, "deleteInternetRadioStation", api.DeleteInternetRadio)
 			h(r, "getInternetRadioStations", api.GetInternetRadios)
-			h(r, "updateInternetRadioStation", api.UpdateInternetRadio)
+			r.Group(func(r chi.Router) {
+				r.Use(adminOnly)
+				h(r, "createInternetRadioStation", api.CreateInternetRadio)
+				h(r, "deleteInternetRadioStation", api.DeleteInternetRadio)
+				h(r, "updateInternetRadioStation", api.UpdateInternetRadio)
+			})
 		})
 		if conf.Server.EnableSharing {
 			r.Group(func(r chi.Router) {

server/subsonic/library_scanning.go

@@ -40,10 +40,6 @@ func (api *Router) StartScan(r *http.Request) (*responses.Subsonic, error) {
 		return nil, newError(responses.ErrorGeneric, "Internal error")
 	}
 
-	if !loggedUser.IsAdmin {
-		return nil, newError(responses.ErrorAuthorizationFail)
-	}
-
 	p := req.Params(r)
 	fullScan := p.BoolOr("fullScan", false)
 

server/subsonic/library_scanning_test.go

@@ -23,29 +23,6 @@ var _ = Describe("LibraryScanning", func() {
 	})
 
 	Describe("StartScan", func() {
-		It("requires admin authentication", func() {
-			// Create non-admin user
-			ctx := request.WithUser(context.Background(), model.User{
-				ID:      "user-id",
-				IsAdmin: false,
-			})
-
-			// Create request
-			r := httptest.NewRequest("GET", "/rest/startScan", nil)
-			r = r.WithContext(ctx)
-
-			// Call endpoint
-			response, err := api.StartScan(r)
-
-			// Should return authorization error
-			Expect(err).To(HaveOccurred())
-			Expect(response).To(BeNil())
-			var subErr subError
-			ok := errors.As(err, &subErr)
-			Expect(ok).To(BeTrue())
-			Expect(subErr.code).To(Equal(responses.ErrorAuthorizationFail))
-		})
-
 		It("triggers a full scan with no parameters", func() {
 			// Create admin user
 			ctx := request.WithUser(context.Background(), model.User{

server/subsonic/middlewares.go

@@ -155,6 +155,23 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler {
 	}
 }
 
+func adminOnly(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		loggedUser, ok := request.UserFrom(r.Context())
+		if !ok {
+			sendError(w, r, newError(responses.ErrorGeneric, "Internal error"))
+			return
+		}
+
+		if !loggedUser.IsAdmin {
+			sendError(w, r, newError(responses.ErrorAuthorizationFail))
+			return
+		}
+
+		next.ServeHTTP(w, r)
+	})
+}
+
 func validateCredentials(user *model.User, pass, token, salt, jwt string) error {
 	valid := false
 

server/subsonic/middlewares_test.go

@@ -308,6 +308,36 @@ var _ = Describe("Middlewares", func() {
 		})
 	})
 
+	Describe("AdminOnly", func() {
+		It("passes admin users", func() {
+			r := newGetRequest()
+			r = r.WithContext(request.WithUser(r.Context(), model.User{ID: "admin-id", IsAdmin: true}))
+
+			adminOnly(next).ServeHTTP(w, r)
+
+			Expect(next.called).To(BeTrue())
+		})
+
+		It("rejects non-admin users", func() {
+			r := newGetRequest()
+			r = r.WithContext(request.WithUser(r.Context(), model.User{ID: "user-id", IsAdmin: false}))
+
+			adminOnly(next).ServeHTTP(w, r)
+
+			Expect(w.Body.String()).To(ContainSubstring(`code="50"`))
+			Expect(next.called).To(BeFalse())
+		})
+
+		It("returns an internal error when user is missing from context", func() {
+			r := newGetRequest()
+
+			adminOnly(next).ServeHTTP(w, r)
+
+			Expect(w.Body.String()).To(ContainSubstring(`code="0"`))
+			Expect(next.called).To(BeFalse())
+		})
+	})
+
 	Describe("GetPlayer", func() {
 		var mockedPlayers *mockPlayers
 		var r *http.Request