From bbc6d0722cd7237f162fa2ca1d822e3924e0cda1 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Fri, 27 Jun 2025 10:45:00 -0400
Subject: [PATCH] feat: add library access validation to selectedMusicFolderIds

Enhanced the selectedMusicFolderIds function to validate musicFolderId parameters
against the user's accessible libraries. Invalid library IDs (those the user
doesn't have access to) are now silently filtered out, improving security by
preventing users from accessing libraries they don't have permission for.

Changes include:
- Added validation logic to check musicFolderId parameters against user's accessible libraries
- Added slices package import for efficient validation
- Enhanced function documentation to clarify validation behavior
- Added comprehensive test cases covering validation scenarios
- Maintains backward compatibility with existing behavior
---
 server/subsonic/helpers.go      | 18 +++++++++++++++---
 server/subsonic/helpers_test.go | 18 ++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go
index 0e387fafc..70a85bc57 100644
--- a/server/subsonic/helpers.go
+++ b/server/subsonic/helpers.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"mime"
 	"net/http"
+	"slices"
 	"sort"
 	"strings"
 
@@ -485,6 +486,7 @@ func getUserAccessibleLibraries(ctx context.Context) []model.Library {
 // selectedMusicFolderIds retrieves the music folder IDs from the request parameters.
 // If no IDs are provided, it returns all libraries the user has access to (based on the user found in the context).
 // If the parameter is required and not present, it returns an error.
+// Invalid library IDs (those the user doesn't have access to) are silently filtered out.
 func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) {
 	p := req.Params(r)
 	musicFolderIds, err := p.Ints("musicFolderId")
@@ -494,11 +496,21 @@ func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) {
 		return nil, err
 	}
 
+	// Get user's accessible libraries for validation
+	libraries := getUserAccessibleLibraries(r.Context())
+	accessibleLibraryIds := slice.Map(libraries, func(lib model.Library) int { return lib.ID })
+
 	if len(musicFolderIds) > 0 {
-		return musicFolderIds, nil
+		// Validate and filter out invalid library IDs
+		validIds := make([]int, 0, len(musicFolderIds))
+		for _, id := range musicFolderIds {
+			if slices.Contains(accessibleLibraryIds, id) {
+				validIds = append(validIds, id)
+			}
+		}
+		return validIds, nil
 	}
 
 	// If no musicFolderId is provided, return all libraries the user has access to.
-	libraries := getUserAccessibleLibraries(r.Context())
-	return slice.Map(libraries, func(lib model.Library) int { return lib.ID }), nil
+	return accessibleLibraryIds, nil
 }
diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go
index 9ebfd3750..2f388217a 100644
--- a/server/subsonic/helpers_test.go
+++ b/server/subsonic/helpers_test.go
@@ -203,6 +203,24 @@ var _ = Describe("helpers", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ids).To(Equal([]int{2})) // Only valid ID is returned
 			})
+
+			It("should filter out library IDs the user doesn't have access to", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=1&musicFolderId=5&musicFolderId=2&musicFolderId=99", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{1, 2})) // Only accessible libraries (1, 2) are returned, 5 and 99 are filtered out
+			})
+
+			It("should return empty slice when user has no access to any of the requested libraries", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=99&musicFolderId=100", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{})) // No accessible libraries in the request
+			})
 		})
 
 		Context("when musicFolderId parameter is not provided", func() {