diff --git a/persistence/user_repository.go b/persistence/user_repository.go index 15710cceb..cd8f85e3b 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -490,7 +490,7 @@ func (r *userRepository) RatingStats() ([]model.UserRatingStats, error) { sel := Select(`a.user_id`, `u.user_name`, `a.item_type`, `a.rating`, `count(*) as cnt`). From(`annotation a`). Join(`"user" u ON a.user_id = u.id`). - Where(Gt{"a.rating": 0}). + Where(And{Gt{"a.rating": 0}, Eq{"a.item_type": []interface{}{"media_file", "album"}}}). GroupBy(`a.user_id`, `u.user_name`, `a.item_type`, `a.rating`). OrderBy(`u.user_name`, `a.item_type`, `a.rating DESC`) diff --git a/server/nativeapi/rating_stats.go b/server/nativeapi/rating_stats.go index 7c92dfeb3..532eb2115 100644 --- a/server/nativeapi/rating_stats.go +++ b/server/nativeapi/rating_stats.go @@ -8,6 +8,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" ) func (api *Router) addRatingStatsRoute(r chi.Router) { @@ -17,12 +18,23 @@ func (api *Router) addRatingStatsRoute(r chi.Router) { func getRatingStats(ds model.DataStore) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + currentUser, _ := request.UserFrom(r.Context()) stats, err := ds.User(r.Context()).RatingStats() if err != nil { log.Error(r.Context(), "Error getting rating stats", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } + if !currentUser.IsAdmin { + filtered := make([]model.UserRatingStats, 0, 1) + for _, s := range stats { + if s.UserID == currentUser.ID { + filtered = append(filtered, s) + break + } + } + stats = filtered + } w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(stats); err != nil { log.Error(r.Context(), "Error encoding rating stats", err) @@ -32,6 +44,7 @@ func getRatingStats(ds model.DataStore) http.HandlerFunc { func getRatingItems(ds model.DataStore) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + currentUser, _ := request.UserFrom(r.Context()) userID := r.URL.Query().Get("userId") itemType := r.URL.Query().Get("type") ratingStr := r.URL.Query().Get("rating") @@ -42,6 +55,11 @@ func getRatingItems(ds model.DataStore) http.HandlerFunc { return } + if !currentUser.IsAdmin && currentUser.ID != userID { + http.Error(w, "forbidden", http.StatusForbidden) + return + } + items, err := ds.User(r.Context()).RatingItems(userID, itemType, rating) if err != nil { log.Error(r.Context(), "Error getting rating items", err) diff --git a/server/nativeapi/rating_stats_test.go b/server/nativeapi/rating_stats_test.go new file mode 100644 index 000000000..d24474f9c --- /dev/null +++ b/server/nativeapi/rating_stats_test.go @@ -0,0 +1,189 @@ +package nativeapi + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Rating Stats API", func() { + var ds *tests.MockDataStore + var router http.Handler + var adminUser, regularUser model.User + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + ds = &tests.MockDataStore{} + auth.Init(ds) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + router = server.JWTVerifier(nativeRouter) + + adminUser = model.User{ + ID: "admin-1", + UserName: "admin", + Name: "Admin User", + IsAdmin: true, + NewPassword: "adminpass", + } + regularUser = model.User{ + ID: "user-1", + UserName: "regular", + Name: "Regular User", + IsAdmin: false, + NewPassword: "userpass", + } + + Expect(ds.User(context.TODO()).Put(&adminUser)).To(Succeed()) + Expect(ds.User(context.TODO()).Put(®ularUser)).To(Succeed()) + + mockedUserRepo := ds.MockedUser.(*tests.MockedUserRepo) + mockedUserRepo.RatingStatsData = []model.UserRatingStats{ + { + UserID: "admin-1", + UserName: "admin", + SongStats: []model.RatingStat{ + {Rating: 5, Count: 3}, + }, + }, + { + UserID: "user-1", + UserName: "regular", + SongStats: []model.RatingStat{ + {Rating: 4, Count: 2}, + }, + }, + } + mockedUserRepo.RatingItemsData = []model.RatedItem{ + {ID: "song-1", Name: "Test Song", Artist: "Test Artist"}, + } + }) + + Describe("GET /ratingStats", func() { + Context("as admin user", func() { + It("returns all users stats", func() { + adminToken, err := auth.CreateToken(&adminUser) + Expect(err).ToNot(HaveOccurred()) + + req := createAuthenticatedRequest("GET", "/ratingStats", nil, adminToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + var stats []model.UserRatingStats + Expect(json.Unmarshal(w.Body.Bytes(), &stats)).To(Succeed()) + Expect(stats).To(HaveLen(2)) + }) + }) + + Context("as regular user", func() { + It("returns only own stats", func() { + userToken, err := auth.CreateToken(®ularUser) + Expect(err).ToNot(HaveOccurred()) + + req := createAuthenticatedRequest("GET", "/ratingStats", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + var stats []model.UserRatingStats + Expect(json.Unmarshal(w.Body.Bytes(), &stats)).To(Succeed()) + Expect(stats).To(HaveLen(1)) + Expect(stats[0].UserID).To(Equal("user-1")) + }) + }) + + Context("without authentication", func() { + It("returns 401", func() { + req := createUnauthenticatedRequest("GET", "/ratingStats", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + }) + + Describe("GET /ratingItems", func() { + Context("as admin user", func() { + It("can query any user's items", func() { + adminToken, err := auth.CreateToken(&adminUser) + Expect(err).ToNot(HaveOccurred()) + + req := createAuthenticatedRequest("GET", "/ratingItems?userId=user-1&type=song&rating=4", nil, adminToken) + req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+adminToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + }) + }) + + Context("as regular user", func() { + var userToken string + + BeforeEach(func() { + var err error + userToken, err = auth.CreateToken(®ularUser) + Expect(err).ToNot(HaveOccurred()) + }) + + It("can query own items", func() { + req := createAuthenticatedRequest("GET", "/ratingItems?userId=user-1&type=song&rating=4", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + It("returns 403 when querying another user's items", func() { + req := createAuthenticatedRequest("GET", "/ratingItems?userId=admin-1&type=song&rating=5", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusForbidden)) + }) + + It("returns 400 for invalid rating", func() { + req := createAuthenticatedRequest("GET", "/ratingItems?userId=user-1&type=song&rating=9", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("returns 400 for invalid type", func() { + req := createAuthenticatedRequest("GET", "/ratingItems?userId=user-1&type=artist&rating=3", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("returns 400 when userId is missing", func() { + req := createAuthenticatedRequest("GET", "/ratingItems?type=song&rating=3", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + }) + + Context("without authentication", func() { + It("returns 401", func() { + req := createUnauthenticatedRequest("GET", "/ratingItems?userId=user-1&type=song&rating=4", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + }) +}) diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 7c7dadbc4..5adc1625a 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -18,9 +18,11 @@ func CreateMockUserRepo() *MockedUserRepo { type MockedUserRepo struct { model.UserRepository - Error error - Data map[string]*model.User - UserLibraries map[string][]int // userID -> libraryIDs + Error error + Data map[string]*model.User + UserLibraries map[string][]int // userID -> libraryIDs + RatingStatsData []model.UserRatingStats + RatingItemsData []model.RatedItem } func (u *MockedUserRepo) CountAll(qo ...model.QueryOptions) (int64, error) { @@ -134,6 +136,20 @@ func (u *MockedUserRepo) SetUserLibraries(userID string, libraryIDs []int) error return nil } +func (u *MockedUserRepo) RatingStats() ([]model.UserRatingStats, error) { + if u.Error != nil { + return nil, u.Error + } + return u.RatingStatsData, nil +} + +func (u *MockedUserRepo) RatingItems(userID, itemType string, rating int) ([]model.RatedItem, error) { + if u.Error != nil { + return nil, u.Error + } + return u.RatingItemsData, nil +} + func (u *MockedUserRepo) Delete(id string) error { if u.Error != nil { return u.Error diff --git a/ui/src/userRatings/UserRatings.jsx b/ui/src/userRatings/UserRatings.jsx index fd366a8c9..404e38c7c 100644 --- a/ui/src/userRatings/UserRatings.jsx +++ b/ui/src/userRatings/UserRatings.jsx @@ -128,6 +128,9 @@ const RatingTable = ({ stats, label, userId, userName, type }) => { className={classes.row} style={{ cursor: count > 0 ? 'pointer' : 'default' }} onClick={() => count > 0 && history.push(linkTo)} + tabIndex={count > 0 ? 0 : undefined} + role={count > 0 ? 'button' : undefined} + onKeyDown={count > 0 ? (e) => (e.key === 'Enter' || e.key === ' ') && history.push(linkTo) : undefined} > {r}.0 {count}