diff --git a/server/auth.go b/server/auth.go index 5b35f72ed..86d5221ca 100644 --- a/server/auth.go +++ b/server/auth.go @@ -171,7 +171,7 @@ func validateLogin(userRepo model.UserRepository, userName, password string) (*m return u, nil } -func jwtVerifier(next http.Handler) http.Handler { +func JWTVerifier(next http.Handler) http.Handler { return jwtauth.Verify(auth.TokenAuth, tokenFromHeader, jwtauth.TokenFromCookie, jwtauth.TokenFromQuery)(next) } diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 4dfc5961c..34c887b20 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -144,11 +144,8 @@ func (n *Router) addPlaylistTrackRoute(r chi.Router) { } func (n *Router) addSongPlaylistsRoute(r chi.Router) { - r.Route("/song/{id}", func(r chi.Router) { - r.Use(server.URLParamsMiddleware) - r.Get("/playlists", func(w http.ResponseWriter, r *http.Request) { - getSongPlaylists(n.ds)(w, r) - }) + r.With(server.URLParamsMiddleware).Get("/song/{id}/playlists", func(w http.ResponseWriter, r *http.Request) { + getSongPlaylists(n.ds)(w, r) }) } diff --git a/server/nativeapi/native_api_song_test.go b/server/nativeapi/native_api_song_test.go new file mode 100644 index 000000000..0b183c1d9 --- /dev/null +++ b/server/nativeapi/native_api_song_test.go @@ -0,0 +1,464 @@ +package nativeapi + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "time" + + "github.com/deluan/rest" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Simple mock implementations for missing types +type mockShare struct { + core.Share +} + +func (m *mockShare) NewRepository(ctx context.Context) rest.Repository { + return &tests.MockShareRepo{} +} + +type mockPlaylists struct { + core.Playlists +} + +func (m *mockPlaylists) ImportFile(ctx context.Context, folder *model.Folder, filename string) (*model.Playlist, error) { + return &model.Playlist{}, nil +} + +type mockInsights struct { + metrics.Insights +} + +func (m *mockInsights) LastRun(ctx context.Context) (time.Time, bool) { + return time.Now(), true +} + +var _ = Describe("Song Endpoints", func() { + var ( + router http.Handler + ds *tests.MockDataStore + mfRepo *tests.MockMediaFileRepo + userRepo *tests.MockedUserRepo + w *httptest.ResponseRecorder + testUser model.User + testSongs model.MediaFiles + ) + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.SessionTimeout = time.Minute + + // Setup mock repositories + mfRepo = tests.CreateMockMediaFileRepo() + userRepo = tests.CreateMockUserRepo() + + ds = &tests.MockDataStore{ + MockedMediaFile: mfRepo, + MockedUser: userRepo, + MockedProperty: &tests.MockedPropertyRepo{}, + } + + // Initialize auth system + auth.Init(ds) + + // Create test user + testUser = model.User{ + ID: "user-1", + UserName: "testuser", + Name: "Test User", + IsAdmin: false, + NewPassword: "testpass", + } + err := userRepo.Put(&testUser) + Expect(err).ToNot(HaveOccurred()) + + // Create test songs + testSongs = model.MediaFiles{ + { + ID: "song-1", + Title: "Test Song 1", + Artist: "Test Artist 1", + Album: "Test Album 1", + AlbumID: "album-1", + ArtistID: "artist-1", + Duration: 180.5, + BitRate: 320, + Path: "/music/song1.mp3", + Suffix: "mp3", + Size: 5242880, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, + { + ID: "song-2", + Title: "Test Song 2", + Artist: "Test Artist 2", + Album: "Test Album 2", + AlbumID: "album-2", + ArtistID: "artist-2", + Duration: 240.0, + BitRate: 256, + Path: "/music/song2.mp3", + Suffix: "mp3", + Size: 7340032, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, + } + mfRepo.SetData(testSongs) + + // Setup router with mocked dependencies + mockShareImpl := &mockShare{} + mockPlaylistsImpl := &mockPlaylists{} + mockInsightsImpl := &mockInsights{} + + // Create the native API router and wrap it with the JWTVerifier middleware + nativeRouter := New(ds, mockShareImpl, mockPlaylistsImpl, mockInsightsImpl) + router = server.JWTVerifier(nativeRouter) + w = httptest.NewRecorder() + }) + + // Helper function to create unauthenticated request + createUnauthenticatedRequest := func(method, path string, body []byte) *http.Request { + var req *http.Request + if body != nil { + req = httptest.NewRequest(method, path, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + } else { + req = httptest.NewRequest(method, path, nil) + } + return req + } + + // Helper function to create authenticated request with JWT token + createAuthenticatedRequest := func(method, path string, body []byte) *http.Request { + req := createUnauthenticatedRequest(method, path, body) + + // Create JWT token for the test user + token, err := auth.CreateToken(&testUser) + Expect(err).ToNot(HaveOccurred()) + + // Add JWT token to Authorization header + req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+token) + + return req + } + + Describe("GET /song", func() { + Context("when user is authenticated", func() { + It("returns all songs", func() { + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response []model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + Expect(response).To(HaveLen(2)) + Expect(response[0].ID).To(Equal("song-1")) + Expect(response[0].Title).To(Equal("Test Song 1")) + Expect(response[1].ID).To(Equal("song-2")) + Expect(response[1].Title).To(Equal("Test Song 2")) + }) + + It("handles repository errors gracefully", func() { + mfRepo.SetError(true) + + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusInternalServerError)) + }) + }) + + Context("when user is not authenticated", func() { + It("returns unauthorized", func() { + req := createUnauthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + }) + + Describe("GET /song/{id}", func() { + Context("when user is authenticated", func() { + It("returns the specific song", func() { + req := createAuthenticatedRequest("GET", "/song/song-1", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + Expect(response.ID).To(Equal("song-1")) + Expect(response.Title).To(Equal("Test Song 1")) + Expect(response.Artist).To(Equal("Test Artist 1")) + }) + + It("returns 404 for non-existent song", func() { + req := createAuthenticatedRequest("GET", "/song/non-existent", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + + It("handles repository errors gracefully", func() { + mfRepo.SetError(true) + + req := createAuthenticatedRequest("GET", "/song/song-1", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusInternalServerError)) + }) + }) + + Context("when user is not authenticated", func() { + It("returns unauthorized", func() { + req := createUnauthenticatedRequest("GET", "/song/song-1", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + }) + + Describe("Song endpoints are read-only", func() { + Context("POST /song", func() { + It("should not be available (songs are not persistable)", func() { + newSong := model.MediaFile{ + Title: "New Song", + Artist: "New Artist", + Album: "New Album", + Duration: 200.0, + } + + body, _ := json.Marshal(newSong) + req := createAuthenticatedRequest("POST", "/song", body) + router.ServeHTTP(w, req) + + // Should return 405 Method Not Allowed or 404 Not Found + Expect(w.Code).To(Equal(http.StatusMethodNotAllowed)) + }) + }) + + Context("PUT /song/{id}", func() { + It("should not be available (songs are not persistable)", func() { + updatedSong := model.MediaFile{ + ID: "song-1", + Title: "Updated Song", + Artist: "Updated Artist", + Album: "Updated Album", + Duration: 250.0, + } + + body, _ := json.Marshal(updatedSong) + req := createAuthenticatedRequest("PUT", "/song/song-1", body) + router.ServeHTTP(w, req) + + // Should return 405 Method Not Allowed or 404 Not Found + Expect(w.Code).To(Equal(http.StatusMethodNotAllowed)) + }) + }) + + Context("DELETE /song/{id}", func() { + It("should not be available (songs are not persistable)", func() { + req := createAuthenticatedRequest("DELETE", "/song/song-1", nil) + router.ServeHTTP(w, req) + + // Should return 405 Method Not Allowed or 404 Not Found + Expect(w.Code).To(Equal(http.StatusMethodNotAllowed)) + }) + }) + }) + + Describe("Query parameters and filtering", func() { + Context("when using query parameters", func() { + It("handles pagination parameters", func() { + req := createAuthenticatedRequest("GET", "/song?_start=0&_end=1", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response []model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + // Should still return all songs since our mock doesn't implement pagination + // but the request should be processed successfully + Expect(len(response)).To(BeNumerically(">=", 1)) + }) + + It("handles sort parameters", func() { + req := createAuthenticatedRequest("GET", "/song?_sort=title&_order=ASC", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response []model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + Expect(response).To(HaveLen(2)) + }) + + It("handles filter parameters", func() { + // Properly encode the URL with query parameters + baseURL := "/song" + params := url.Values{} + params.Add("title", "Test Song 1") + fullURL := baseURL + "?" + params.Encode() + + req := createAuthenticatedRequest("GET", fullURL, nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response []model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + // Mock doesn't implement filtering, but request should be processed + Expect(len(response)).To(BeNumerically(">=", 1)) + }) + }) + }) + + Describe("Response headers and content type", func() { + It("sets correct content type for JSON responses", func() { + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).To(ContainSubstring("application/json")) + }) + + It("includes total count header when available", func() { + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + // The X-Total-Count header might be set by the REST framework + // We just verify the request is processed successfully + }) + }) + + Describe("Edge cases and error handling", func() { + Context("when repository is unavailable", func() { + It("handles database connection errors", func() { + mfRepo.SetError(true) + + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusInternalServerError)) + }) + }) + + Context("when no songs exist", func() { + It("returns empty array when no songs are found", func() { + mfRepo.SetData(model.MediaFiles{}) // Empty dataset + + req := createAuthenticatedRequest("GET", "/song", nil) + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + + var response []model.MediaFile + err := json.Unmarshal(w.Body.Bytes(), &response) + Expect(err).ToNot(HaveOccurred()) + + Expect(response).To(HaveLen(0)) + }) + }) + }) + + Describe("Authentication middleware integration", func() { + Context("with different user types", func() { + It("works with admin users", func() { + adminUser := model.User{ + ID: "admin-1", + UserName: "admin", + Name: "Admin User", + IsAdmin: true, + NewPassword: "adminpass", + } + err := userRepo.Put(&adminUser) + Expect(err).ToNot(HaveOccurred()) + + // Create JWT token for admin user + token, err := auth.CreateToken(&adminUser) + Expect(err).ToNot(HaveOccurred()) + + req := createUnauthenticatedRequest("GET", "/song", nil) + req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+token) + + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + It("works with regular users", func() { + regularUser := model.User{ + ID: "user-2", + UserName: "regular", + Name: "Regular User", + IsAdmin: false, + NewPassword: "userpass", + } + err := userRepo.Put(®ularUser) + Expect(err).ToNot(HaveOccurred()) + + // Create JWT token for regular user + token, err := auth.CreateToken(®ularUser) + Expect(err).ToNot(HaveOccurred()) + + req := createUnauthenticatedRequest("GET", "/song", nil) + req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+token) + + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusOK)) + }) + }) + + Context("with missing authentication context", func() { + It("rejects requests without user context", func() { + req := createUnauthenticatedRequest("GET", "/song", nil) + // No authentication header added + + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + + It("rejects requests with invalid JWT tokens", func() { + req := createUnauthenticatedRequest("GET", "/song", nil) + req.Header.Set(consts.UIAuthorizationHeader, "Bearer invalid.token.here") + + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) + }) +}) diff --git a/server/server.go b/server/server.go index 60350b6b4..49391e2b6 100644 --- a/server/server.go +++ b/server/server.go @@ -173,7 +173,7 @@ func (s *Server) initRoutes() { clientUniqueIDMiddleware, compressMiddleware(), loggerInjector, - jwtVerifier, + JWTVerifier, } // Mount the Native API /events endpoint with all default middlewares, adding the authentication middlewares diff --git a/tests/mock_data_store.go b/tests/mock_data_store.go index f380755e0..fb5bbd710 100644 --- a/tests/mock_data_store.go +++ b/tests/mock_data_store.go @@ -217,8 +217,33 @@ func (db *MockDataStore) WithTxImmediate(block func(tx model.DataStore) error, l return block(db) } -func (db *MockDataStore) Resource(context.Context, any) model.ResourceRepository { - return struct{ model.ResourceRepository }{} +func (db *MockDataStore) Resource(ctx context.Context, m any) model.ResourceRepository { + switch m.(type) { + case model.MediaFile, *model.MediaFile: + return db.MediaFile(ctx).(model.ResourceRepository) + case model.Album, *model.Album: + return db.Album(ctx).(model.ResourceRepository) + case model.Artist, *model.Artist: + return db.Artist(ctx).(model.ResourceRepository) + case model.User, *model.User: + return db.User(ctx).(model.ResourceRepository) + case model.Playlist, *model.Playlist: + return db.Playlist(ctx).(model.ResourceRepository) + case model.Radio, *model.Radio: + return db.Radio(ctx).(model.ResourceRepository) + case model.Share, *model.Share: + return db.Share(ctx).(model.ResourceRepository) + case model.Genre, *model.Genre: + return db.Genre(ctx).(model.ResourceRepository) + case model.Tag, *model.Tag: + return db.Tag(ctx).(model.ResourceRepository) + case model.Transcoding, *model.Transcoding: + return db.Transcoding(ctx).(model.ResourceRepository) + case model.Player, *model.Player: + return db.Player(ctx).(model.ResourceRepository) + default: + return struct{ model.ResourceRepository }{} + } } func (db *MockDataStore) GC(context.Context) error { diff --git a/tests/mock_mediafile_repo.go b/tests/mock_mediafile_repo.go index 85adb8a25..7bba8eda8 100644 --- a/tests/mock_mediafile_repo.go +++ b/tests/mock_mediafile_repo.go @@ -7,6 +7,7 @@ import ( "slices" "time" + "github.com/deluan/rest" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils/slice" @@ -76,9 +77,14 @@ func (m *MockMediaFileRepo) GetAll(...model.QueryOptions) (model.MediaFiles, err return nil, errors.New("error") } values := slices.Collect(maps.Values(m.Data)) - return slice.Map(values, func(p *model.MediaFile) model.MediaFile { + result := slice.Map(values, func(p *model.MediaFile) model.MediaFile { return *p - }), nil + }) + // Sort by ID to ensure deterministic ordering for tests + slices.SortFunc(result, func(a, b model.MediaFile) int { + return cmp.Compare(a.ID, b.ID) + }) + return result, nil } func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error { @@ -196,4 +202,30 @@ func (m *MockMediaFileRepo) DeleteAllMissing() (int64, error) { return count, nil } +// ResourceRepository methods +func (m *MockMediaFileRepo) Count(...rest.QueryOptions) (int64, error) { + return m.CountAll() +} + +func (m *MockMediaFileRepo) Read(id string) (interface{}, error) { + mf, err := m.Get(id) + if errors.Is(err, model.ErrNotFound) { + return nil, rest.ErrNotFound + } + return mf, err +} + +func (m *MockMediaFileRepo) ReadAll(...rest.QueryOptions) (interface{}, error) { + return m.GetAll() +} + +func (m *MockMediaFileRepo) EntityName() string { + return "mediafile" +} + +func (m *MockMediaFileRepo) NewInstance() interface{} { + return &model.MediaFile{} +} + var _ model.MediaFileRepository = (*MockMediaFileRepo)(nil) +var _ model.ResourceRepository = (*MockMediaFileRepo)(nil)