Merge 75416d6050 into 92a98cd558

Closes #1032

* feat(album_repository.go): add kodi-style album playcount option - #1032

Signed-off-by: Victor van der Veen <vvdveen@gmail.com>

* fix format issue and remove reference to kodi (now normalized)

Signed-off-by: Victor van der Veen <vvdveen@gmail.com>

* reduced complexity but added rounding

Signed-off-by: Victor van der Veen <vvdveen@gmail.com>

* Use constants for AlbumPlayCountMode values

---------

Signed-off-by: Victor van der Veen <vvdveen@gmail.com>
Co-authored-by: Deluan <deluan@navidrome.org>

cmd/root.go

@@ -192,6 +192,7 @@ func init() {
 	rootCmd.Flags().Bool("enabletranscodingconfig", viper.GetBool("enabletranscodingconfig"), "enables transcoding configuration in the UI")
 	rootCmd.Flags().String("transcodingcachesize", viper.GetString("transcodingcachesize"), "size of transcoding cache")
 	rootCmd.Flags().String("imagecachesize", viper.GetString("imagecachesize"), "size of image (art work) cache. set to 0 to disable cache")
+	rootCmd.Flags().String("albumplaycountmode", viper.GetString("albumplaycountmode"), "how to compute playcount for albums. absolute (default) or normalized")
 	rootCmd.Flags().Bool("autoimportplaylists", viper.GetBool("autoimportplaylists"), "enable/disable .m3u playlist auto-import`")
 
 	rootCmd.Flags().Bool("prometheus.enabled", viper.GetBool("prometheus.enabled"), "enable/disable prometheus metrics endpoint`")
@@ -214,4 +215,5 @@ func init() {
 	_ = viper.BindPFlag("enabletranscodingconfig", rootCmd.Flags().Lookup("enabletranscodingconfig"))
 	_ = viper.BindPFlag("transcodingcachesize", rootCmd.Flags().Lookup("transcodingcachesize"))
 	_ = viper.BindPFlag("imagecachesize", rootCmd.Flags().Lookup("imagecachesize"))
+	_ = viper.BindPFlag("albumplaycountmode", rootCmd.Flags().Lookup("albumplaycountmode"))
 }

conf/configuration.go

@@ -44,6 +44,7 @@ type configOptions struct {
 	EnableMediaFileCoverArt      bool
 	TranscodingCacheSize         string
 	ImageCacheSize               string
+	AlbumPlayCountMode           string
 	EnableArtworkPrecache        bool
 	AutoImportPlaylists          bool
 	PlaylistsPath                string
@@ -113,10 +114,11 @@ type scannerOptions struct {
 }
 
 type lastfmOptions struct {
-	Enabled  bool
-	ApiKey   string
-	Secret   string
-	Language string
+	Enabled        bool
+	ApiKey         string
+	Secret         string
+	Language       string
+	UseAlbumArtist bool
 }
 
 type spotifyOptions struct {
@@ -289,6 +291,7 @@ func init() {
 	viper.SetDefault("enabletranscodingconfig", false)
 	viper.SetDefault("transcodingcachesize", "100MB")
 	viper.SetDefault("imagecachesize", "100MB")
+	viper.SetDefault("albumplaycountmode", consts.AlbumPlayCountModeAbsolute)
 	viper.SetDefault("enableartworkprecache", true)
 	viper.SetDefault("autoimportplaylists", true)
 	viper.SetDefault("playlistspath", consts.DefaultPlaylistsPath)
@@ -343,6 +346,7 @@ func init() {
 	viper.SetDefault("lastfm.language", "en")
 	viper.SetDefault("lastfm.apikey", "")
 	viper.SetDefault("lastfm.secret", "")
+	viper.SetDefault("lastfm.usealbumartist", false)
 	viper.SetDefault("spotify.id", "")
 	viper.SetDefault("spotify.secret", "")
 	viper.SetDefault("listenbrainz.enabled", true)

consts/consts.go

@@ -81,6 +81,11 @@ const (
 	DefaultCacheCleanUpInterval = 10 * time.Minute
 )
 
+const (
+	AlbumPlayCountModeAbsolute   = "absolute"
+	AlbumPlayCountModeNormalized = "normalized"
+)
+
 var (
 	DefaultDownsamplingFormat = "opus"
 	DefaultTranscodings       = []map[string]interface{}{

core/agents/lastfm/agent.go

@@ -28,27 +28,29 @@ var ignoredBiographies = []string{
 }
 
 type lastfmAgent struct {
-	ds          model.DataStore
-	sessionKeys *agents.SessionKeys
-	apiKey      string
-	secret      string
-	lang        string
-	client      *client
+	ds             model.DataStore
+	sessionKeys    *agents.SessionKeys
+	apiKey         string
+	secret         string
+	lang           string
+	useAlbumArtist bool
+	client         *client
 }
 
 func lastFMConstructor(ds model.DataStore) *lastfmAgent {
 	l := &lastfmAgent{
-		ds:          ds,
-		lang:        conf.Server.LastFM.Language,
-		apiKey:      conf.Server.LastFM.ApiKey,
-		secret:      conf.Server.LastFM.Secret,
-		sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
+		ds:             ds,
+		lang:           conf.Server.LastFM.Language,
+		apiKey:         conf.Server.LastFM.ApiKey,
+		secret:         conf.Server.LastFM.Secret,
+		useAlbumArtist: conf.Server.LastFM.UseAlbumArtist,
+		sessionKeys:    &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
 	}
 	hc := &http.Client{
 		Timeout: consts.DefaultHttpClientTimeOut,
 	}
 	chc := utils.NewCachedHTTPClient(hc, consts.DefaultHttpClientTimeOut)
-	l.client = newClient(l.apiKey, l.secret, l.lang, chc)
+	l.client = newClient(l.apiKey, l.secret, l.lang, l.useAlbumArtist, chc)
 	return l
 }
 

core/agents/lastfm/agent_test.go

@@ -48,7 +48,7 @@ var _ = Describe("lastfmAgent", func() {
 		var httpClient *tests.FakeHttpClient
 		BeforeEach(func() {
 			httpClient = &tests.FakeHttpClient{}
-			client := newClient("API_KEY", "SECRET", "pt", httpClient)
+			client := newClient("API_KEY", "SECRET", "pt", false, httpClient)
 			agent = lastFMConstructor(ds)
 			agent.client = client
 		})
@@ -106,7 +106,7 @@ var _ = Describe("lastfmAgent", func() {
 		var httpClient *tests.FakeHttpClient
 		BeforeEach(func() {
 			httpClient = &tests.FakeHttpClient{}
-			client := newClient("API_KEY", "SECRET", "pt", httpClient)
+			client := newClient("API_KEY", "SECRET", "pt", false, httpClient)
 			agent = lastFMConstructor(ds)
 			agent.client = client
 		})
@@ -167,7 +167,7 @@ var _ = Describe("lastfmAgent", func() {
 		var httpClient *tests.FakeHttpClient
 		BeforeEach(func() {
 			httpClient = &tests.FakeHttpClient{}
-			client := newClient("API_KEY", "SECRET", "pt", httpClient)
+			client := newClient("API_KEY", "SECRET", "pt", false, httpClient)
 			agent = lastFMConstructor(ds)
 			agent.client = client
 		})
@@ -230,7 +230,7 @@ var _ = Describe("lastfmAgent", func() {
 		BeforeEach(func() {
 			_ = ds.UserProps(ctx).Put("user-1", sessionKeyProperty, "SK-1")
 			httpClient = &tests.FakeHttpClient{}
-			client := newClient("API_KEY", "SECRET", "en", httpClient)
+			client := newClient("API_KEY", "SECRET", "en", false, httpClient)
 			agent = lastFMConstructor(ds)
 			agent.client = client
 			track = &model.MediaFile{
@@ -265,6 +265,28 @@ var _ = Describe("lastfmAgent", func() {
 				Expect(sentParams.Get("mbid")).To(Equal(track.MbzRecordingID))
 			})
 
+			It("calls Last.fm with correct params when album artist is prioritized", func() {
+				// Set the preference to use album artist
+				agent.client.useAlbumArtist = true
+
+				httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200}
+
+				err := agent.NowPlaying(ctx, "user-1", track)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(httpClient.SavedRequest.Method).To(Equal(http.MethodPost))
+				sentParams := httpClient.SavedRequest.URL.Query()
+				Expect(sentParams.Get("method")).To(Equal("track.updateNowPlaying"))
+				Expect(sentParams.Get("sk")).To(Equal("SK-1"))
+				Expect(sentParams.Get("track")).To(Equal(track.Title))
+				Expect(sentParams.Get("album")).To(Equal(track.Album))
+				Expect(sentParams.Get("artist")).To(Equal(track.AlbumArtist))
+				Expect(sentParams.Get("albumArtist")).To(Equal(track.AlbumArtist))
+				Expect(sentParams.Get("trackNumber")).To(Equal(strconv.Itoa(track.TrackNumber)))
+				Expect(sentParams.Get("duration")).To(Equal(strconv.FormatFloat(float64(track.Duration), 'G', -1, 32)))
+				Expect(sentParams.Get("mbid")).To(Equal(track.MbzRecordingID))
+			})
+
 			It("returns ErrNotAuthorized if user is not linked", func() {
 				err := agent.NowPlaying(ctx, "user-2", track)
 				Expect(err).To(MatchError(scrobbler.ErrNotAuthorized))
@@ -293,6 +315,30 @@ var _ = Describe("lastfmAgent", func() {
 				Expect(sentParams.Get("timestamp")).To(Equal(strconv.FormatInt(ts.Unix(), 10)))
 			})
 
+			It("calls Last.fm with correct params when album artist is prioritized", func() {
+				// Set the preference to use album artist
+				agent.client.useAlbumArtist = true
+
+				ts := time.Now()
+				httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200}
+
+				err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: ts})
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(httpClient.SavedRequest.Method).To(Equal(http.MethodPost))
+				sentParams := httpClient.SavedRequest.URL.Query()
+				Expect(sentParams.Get("method")).To(Equal("track.scrobble"))
+				Expect(sentParams.Get("sk")).To(Equal("SK-1"))
+				Expect(sentParams.Get("track")).To(Equal(track.Title))
+				Expect(sentParams.Get("album")).To(Equal(track.Album))
+				Expect(sentParams.Get("artist")).To(Equal(track.AlbumArtist))
+				Expect(sentParams.Get("albumArtist")).To(Equal(track.AlbumArtist))
+				Expect(sentParams.Get("trackNumber")).To(Equal(strconv.Itoa(track.TrackNumber)))
+				Expect(sentParams.Get("duration")).To(Equal(strconv.FormatFloat(float64(track.Duration), 'G', -1, 32)))
+				Expect(sentParams.Get("mbid")).To(Equal(track.MbzRecordingID))
+				Expect(sentParams.Get("timestamp")).To(Equal(strconv.FormatInt(ts.Unix(), 10)))
+			})
+
 			It("skips songs with less than 31 seconds", func() {
 				track.Duration = 29
 				httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200}
@@ -355,7 +401,7 @@ var _ = Describe("lastfmAgent", func() {
 		var httpClient *tests.FakeHttpClient
 		BeforeEach(func() {
 			httpClient = &tests.FakeHttpClient{}
-			client := newClient("API_KEY", "SECRET", "pt", httpClient)
+			client := newClient("API_KEY", "SECRET", "pt", false, httpClient)
 			agent = lastFMConstructor(ds)
 			agent.client = client
 		})

core/agents/lastfm/auth_router.go

@@ -44,7 +44,7 @@ func NewRouter(ds model.DataStore) *Router {
 	hc := &http.Client{
 		Timeout: consts.DefaultHttpClientTimeOut,
 	}
-	r.client = newClient(r.apiKey, r.secret, "en", hc)
+	r.client = newClient(r.apiKey, r.secret, "en", false, hc)
 	return r
 }
 

core/agents/lastfm/client.go

@@ -34,15 +34,16 @@ type httpDoer interface {
 	Do(req *http.Request) (*http.Response, error)
 }
 
-func newClient(apiKey string, secret string, lang string, hc httpDoer) *client {
-	return &client{apiKey, secret, lang, hc}
+func newClient(apiKey string, secret string, lang string, useAlbumArtist bool, hc httpDoer) *client {
+	return &client{apiKey, secret, lang, useAlbumArtist, hc}
 }
 
 type client struct {
-	apiKey string
-	secret string
-	lang   string
-	hc     httpDoer
+	apiKey         string
+	secret         string
+	lang           string
+	useAlbumArtist bool
+	hc             httpDoer
 }
 
 func (c *client) albumGetInfo(ctx context.Context, name string, artist string, mbid string) (*Album, error) {
@@ -134,7 +135,7 @@ type ScrobbleInfo struct {
 func (c *client) updateNowPlaying(ctx context.Context, sessionKey string, info ScrobbleInfo) error {
 	params := url.Values{}
 	params.Add("method", "track.updateNowPlaying")
-	params.Add("artist", info.artist)
+	params.Add("artist", c.getArtistFromInfo(ctx, info))
 	params.Add("track", info.track)
 	params.Add("album", info.album)
 	params.Add("trackNumber", strconv.Itoa(info.trackNumber))
@@ -153,11 +154,24 @@ func (c *client) updateNowPlaying(ctx context.Context, sessionKey string, info S
 	return nil
 }
 
+func (c *client) getArtistFromInfo(ctx context.Context, info ScrobbleInfo) string {
+	if c.useAlbumArtist {
+		if info.albumArtist == "" || info.albumArtist == "Various Artists" {
+			log.Warn(ctx, "LastFM: albumArtist is empty or Various Artists, using artist instead", "albumArtist", info.albumArtist, "artist", info.artist)
+			return info.artist
+		}
+
+		return info.albumArtist
+	}
+
+	return info.artist
+}
+
 func (c *client) scrobble(ctx context.Context, sessionKey string, info ScrobbleInfo) error {
 	params := url.Values{}
 	params.Add("method", "track.scrobble")
 	params.Add("timestamp", strconv.FormatInt(info.timestamp.Unix(), 10))
-	params.Add("artist", info.artist)
+	params.Add("artist", c.getArtistFromInfo(ctx, info))
 	params.Add("track", info.track)
 	params.Add("album", info.album)
 	params.Add("trackNumber", strconv.Itoa(info.trackNumber))

core/agents/lastfm/client_test.go

@@ -22,7 +22,7 @@ var _ = Describe("client", func() {
 
 	BeforeEach(func() {
 		httpClient = &tests.FakeHttpClient{}
-		client = newClient("API_KEY", "SECRET", "pt", httpClient)
+		client = newClient("API_KEY", "SECRET", "pt", false, httpClient)
 	})
 
 	Describe("albumGetInfo", func() {

persistence/album_repository.go

@@ -4,11 +4,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"strings"
 
 	. "github.com/Masterminds/squirrel"
 	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/pocketbase/dbx"
@@ -172,6 +174,9 @@ func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, e
 func (r *albumRepository) toModels(dba []dbAlbum) model.Albums {
 	res := model.Albums{}
 	for i := range dba {
+		if conf.Server.AlbumPlayCountMode == consts.AlbumPlayCountModeNormalized && dba[i].Album.SongCount != 0 {
+			dba[i].Album.PlayCount = int64(math.Round(float64(dba[i].Album.PlayCount) / float64(dba[i].Album.SongCount)))
+		}
 		res = append(res, *dba[i].Album)
 	}
 	return res

persistence/album_repository_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 
 	"github.com/fatih/structs"
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -89,4 +91,60 @@ var _ = Describe("AlbumRepository", func() {
 			Expect(other.Album.Discs).To(Equal(a.Discs))
 		})
 	})
+
+	Describe("toModels", func() {
+		var repo *albumRepository
+
+		BeforeEach(func() {
+			ctx := request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid", UserName: "johndoe"})
+			repo = NewAlbumRepository(ctx, getDBXBuilder()).(*albumRepository)
+		})
+
+		It("converts dbAlbum to model.Album", func() {
+			dba := []dbAlbum{
+				{Album: &model.Album{ID: "1", Name: "name", SongCount: 2, Annotations: model.Annotations{PlayCount: 4}}},
+				{Album: &model.Album{ID: "2", Name: "name2", SongCount: 3, Annotations: model.Annotations{PlayCount: 6}}},
+			}
+			albums := repo.toModels(dba)
+			Expect(len(albums)).To(Equal(2))
+			Expect(albums[0].ID).To(Equal("1"))
+			Expect(albums[1].ID).To(Equal("2"))
+		})
+
+		DescribeTable("normalizes play count when AlbumPlayCountMode is absolute",
+			func(songCount, playCount, expected int) {
+				conf.Server.AlbumPlayCountMode = consts.AlbumPlayCountModeAbsolute
+				dba := []dbAlbum{
+					{Album: &model.Album{ID: "1", Name: "name", SongCount: songCount, Annotations: model.Annotations{PlayCount: int64(playCount)}}},
+				}
+				albums := repo.toModels(dba)
+				Expect(albums[0].PlayCount).To(Equal(int64(expected)))
+			},
+			Entry("1 song, 0 plays", 1, 0, 0),
+			Entry("1 song, 4 plays", 1, 4, 4),
+			Entry("3 songs, 6 plays", 3, 6, 6),
+			Entry("10 songs, 6 plays", 10, 6, 6),
+			Entry("70 songs, 70 plays", 70, 70, 70),
+			Entry("10 songs, 50 plays", 10, 50, 50),
+			Entry("120 songs, 121 plays", 120, 121, 121),
+		)
+
+		DescribeTable("normalizes play count when AlbumPlayCountMode is normalized",
+			func(songCount, playCount, expected int) {
+				conf.Server.AlbumPlayCountMode = consts.AlbumPlayCountModeNormalized
+				dba := []dbAlbum{
+					{Album: &model.Album{ID: "1", Name: "name", SongCount: songCount, Annotations: model.Annotations{PlayCount: int64(playCount)}}},
+				}
+				albums := repo.toModels(dba)
+				Expect(albums[0].PlayCount).To(Equal(int64(expected)))
+			},
+			Entry("1 song, 0 plays", 1, 0, 0),
+			Entry("1 song, 4 plays", 1, 4, 4),
+			Entry("3 songs, 6 plays", 3, 6, 2),
+			Entry("10 songs, 6 plays", 10, 6, 1),
+			Entry("70 songs, 70 plays", 70, 70, 1),
+			Entry("10 songs, 50 plays", 10, 50, 5),
+			Entry("120 songs, 121 plays", 120, 121, 1),
+		)
+	})
 })

server/subsonic/middlewares.go

@@ -1,7 +1,6 @@
 package subsonic
 
 import (
-	"context"
 	"crypto/md5"
 	"encoding/hex"
 	"errors"
@@ -19,6 +18,7 @@ import (
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
+	"github.com/navidrome/navidrome/server"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
 	. "github.com/navidrome/navidrome/utils/gg"
 	"github.com/navidrome/navidrome/utils/req"
@@ -44,7 +44,15 @@ func postFormToQueryParams(next http.Handler) http.Handler {
 
 func checkRequiredParameters(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		requiredParameters := []string{"u", "v", "c"}
+		var requiredParameters []string
+		var username string
+
+		if username = server.UsernameFromReverseProxyHeader(r); username != "" {
+			requiredParameters = []string{"v", "c"}
+		} else {
+			requiredParameters = []string{"u", "v", "c"}
+		}
+
 		p := req.Params(r)
 		for _, param := range requiredParameters {
 			if _, err := p.String(param); err != nil {
@@ -54,17 +62,19 @@ func checkRequiredParameters(next http.Handler) http.Handler {
 			}
 		}
 
-		username, _ := p.String("u")
+		if username == "" {
+			username, _ = p.String("u")
+		}
 		client, _ := p.String("c")
 		version, _ := p.String("v")
+
 		ctx := r.Context()
 		ctx = request.WithUsername(ctx, username)
 		ctx = request.WithClient(ctx, client)
 		ctx = request.WithVersion(ctx, version)
 		log.Debug(ctx, "API: New request "+r.URL.Path, "username", username, "client", client, "version", version)
 
-		r = r.WithContext(ctx)
-		next.ServeHTTP(w, r)
+		next.ServeHTTP(w, r.WithContext(ctx))
 	})
 }
 
@@ -72,19 +82,36 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
-			p := req.Params(r)
-			username, _ := p.String("u")
 
-			pass, _ := p.String("p")
-			token, _ := p.String("t")
-			salt, _ := p.String("s")
-			jwt, _ := p.String("jwt")
+			var usr *model.User
+			var err error
 
-			usr, err := validateUser(ctx, ds, username, pass, token, salt, jwt)
-			if errors.Is(err, model.ErrInvalidAuth) {
-				log.Warn(ctx, "API: Invalid login", "username", username, "remoteAddr", r.RemoteAddr, err)
-			} else if err != nil {
-				log.Error(ctx, "API: Error authenticating username", "username", username, "remoteAddr", r.RemoteAddr, err)
+			if username := server.UsernameFromReverseProxyHeader(r); username != "" {
+				usr, err = ds.User(ctx).FindByUsername(username)
+				if errors.Is(err, model.ErrNotFound) {
+					log.Warn(ctx, "API: Invalid login", "auth", "reverse-proxy", "username", username, "remoteAddr", r.RemoteAddr, err)
+				} else if err != nil {
+					log.Error(ctx, "API: Error authenticating username", "auth", "reverse-proxy", "username", username, "remoteAddr", r.RemoteAddr, err)
+				}
+			} else {
+				p := req.Params(r)
+				username, _ := p.String("u")
+				pass, _ := p.String("p")
+				token, _ := p.String("t")
+				salt, _ := p.String("s")
+				jwt, _ := p.String("jwt")
+
+				usr, err = ds.User(ctx).FindByUsernameWithPassword(username)
+				if errors.Is(err, model.ErrNotFound) {
+					log.Warn(ctx, "API: Invalid login", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err)
+				} else if err != nil {
+					log.Error(ctx, "API: Error authenticating username", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err)
+				}
+
+				err = validateCredentials(usr, pass, token, salt, jwt)
+				if err != nil {
+					log.Warn(ctx, "API: Invalid login", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err)
+				}
 			}
 
 			if err != nil {
@@ -100,23 +127,13 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler {
 			//	}
 			//}()
 
-			ctx = log.NewContext(r.Context(), "username", username)
 			ctx = request.WithUser(ctx, *usr)
-			r = r.WithContext(ctx)
-
-			next.ServeHTTP(w, r)
+			next.ServeHTTP(w, r.WithContext(ctx))
 		})
 	}
 }
 
-func validateUser(ctx context.Context, ds model.DataStore, username, pass, token, salt, jwt string) (*model.User, error) {
-	user, err := ds.User(ctx).FindByUsernameWithPassword(username)
-	if errors.Is(err, model.ErrNotFound) {
-		return nil, model.ErrInvalidAuth
-	}
-	if err != nil {
-		return nil, err
-	}
+func validateCredentials(user *model.User, pass, token, salt, jwt string) error {
 	valid := false
 
 	switch {
@@ -136,9 +153,9 @@ func validateUser(ctx context.Context, ds model.DataStore, username, pass, token
 	}
 
 	if !valid {
-		return nil, model.ErrInvalidAuth
+		return model.ErrInvalidAuth
 	}
-	return user, nil
+	return nil
 }
 
 func getPlayer(players core.Players) func(next http.Handler) http.Handler {
@@ -152,7 +169,7 @@ func getPlayer(players core.Players) func(next http.Handler) http.Handler {
 			userAgent := canonicalUserAgent(r)
 			player, trc, err := players.Register(ctx, playerId, client, userAgent, ip)
 			if err != nil {
-				log.Error(r.Context(), "Could not register player", "username", userName, "client", client, err)
+				log.Error(ctx, "Could not register player", "username", userName, "client", client, err)
 			} else {
 				ctx = request.WithPlayer(ctx, *player)
 				if trc != nil {

server/subsonic/middlewares_test.go

@@ -76,7 +76,7 @@ var _ = Describe("Middlewares", func() {
 	})
 
 	Describe("CheckParams", func() {
-		It("passes when all required params are available", func() {
+		It("passes when all required params are available (subsonicauth case)", func() {
 			r := newGetRequest("u=user", "v=1.15", "c=test")
 			cp := checkRequiredParameters(next)
 			cp.ServeHTTP(w, r)
@@ -91,6 +91,27 @@ var _ = Describe("Middlewares", func() {
 			Expect(next.called).To(BeTrue())
 		})
 
+		It("passes when all required params are available (reverse-proxy case)", func() {
+			conf.Server.ReverseProxyWhitelist = "127.0.0.234/32"
+			conf.Server.ReverseProxyUserHeader = "Remote-User"
+
+			r := newGetRequest("v=1.15", "c=test")
+			r.Header.Add("Remote-User", "user")
+			r = r.WithContext(request.WithReverseProxyIp(r.Context(), "127.0.0.234"))
+
+			cp := checkRequiredParameters(next)
+			cp.ServeHTTP(w, r)
+
+			username, _ := request.UsernameFrom(next.req.Context())
+			Expect(username).To(Equal("user"))
+			version, _ := request.VersionFrom(next.req.Context())
+			Expect(version).To(Equal("1.15"))
+			client, _ := request.ClientFrom(next.req.Context())
+			Expect(client).To(Equal("test"))
+
+			Expect(next.called).To(BeTrue())
+		})
+
 		It("fails when user is missing", func() {
 			r := newGetRequest("v=1.15", "c=test")
 			cp := checkRequiredParameters(next)
@@ -127,6 +148,7 @@ var _ = Describe("Middlewares", func() {
 				NewPassword: "wordpass",
 			})
 		})
+
 		It("passes authentication with correct credentials", func() {
 			r := newGetRequest("u=admin", "p=wordpass")
 			cp := authenticate(ds)(next)
@@ -226,77 +248,85 @@ var _ = Describe("Middlewares", func() {
 		})
 	})
 
-	Describe("validateUser", func() {
+	Describe("validateCredentials", func() {
+		var usr *model.User
+
 		BeforeEach(func() {
 			ur := ds.User(context.TODO())
 			_ = ur.Put(&model.User{
 				UserName:    "admin",
 				NewPassword: "wordpass",
 			})
+
+			var err error
+			usr, err = ur.FindByUsernameWithPassword("admin")
+			if err != nil {
+				panic(err)
+			}
 		})
+
 		Context("Plaintext password", func() {
 			It("authenticates with plaintext password ", func() {
-				usr, err := validateUser(context.TODO(), ds, "admin", "wordpass", "", "", "")
+				err := validateCredentials(usr, "wordpass", "", "", "")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(usr.UserName).To(Equal("admin"))
 			})
 
 			It("fails authentication with wrong password", func() {
-				_, err := validateUser(context.TODO(), ds, "admin", "INVALID", "", "", "")
+				err := validateCredentials(usr, "INVALID", "", "", "")
 				Expect(err).To(MatchError(model.ErrInvalidAuth))
 			})
 		})
 
 		Context("Encoded password", func() {
 			It("authenticates with simple encoded password ", func() {
-				usr, err := validateUser(context.TODO(), ds, "admin", "enc:776f726470617373", "", "", "")
+				err := validateCredentials(usr, "enc:776f726470617373", "", "", "")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(usr.UserName).To(Equal("admin"))
 			})
 		})
 
 		Context("Token based authentication", func() {
 			It("authenticates with token based authentication", func() {
-				usr, err := validateUser(context.TODO(), ds, "admin", "", "23b342970e25c7928831c3317edd0b67", "retnlmjetrymazgkt", "")
+				err := validateCredentials(usr, "", "23b342970e25c7928831c3317edd0b67", "retnlmjetrymazgkt", "")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(usr.UserName).To(Equal("admin"))
 			})
 
 			It("fails if salt is missing", func() {
-				_, err := validateUser(context.TODO(), ds, "admin", "", "23b342970e25c7928831c3317edd0b67", "", "")
+				err := validateCredentials(usr, "", "23b342970e25c7928831c3317edd0b67", "", "")
 				Expect(err).To(MatchError(model.ErrInvalidAuth))
 			})
 		})
 
 		Context("JWT based authentication", func() {
+			var usr *model.User
 			var validToken string
+
 			BeforeEach(func() {
 				conf.Server.SessionTimeout = time.Minute
 				auth.Init(ds)
 
-				u := &model.User{UserName: "admin"}
+				usr = &model.User{UserName: "admin"}
 				var err error
-				validToken, err = auth.CreateToken(u)
+				validToken, err = auth.CreateToken(usr)
 				if err != nil {
 					panic(err)
 				}
 			})
+
 			It("authenticates with JWT token based authentication", func() {
-				usr, err := validateUser(context.TODO(), ds, "admin", "", "", "", validToken)
+				err := validateCredentials(usr, "", "", "", validToken)
 
 				Expect(err).NotTo(HaveOccurred())
-				Expect(usr.UserName).To(Equal("admin"))
 			})
 
 			It("fails if JWT token is invalid", func() {
-				_, err := validateUser(context.TODO(), ds, "admin", "", "", "", "invalid.token")
+				err := validateCredentials(usr, "", "", "", "invalid.token")
 				Expect(err).To(MatchError(model.ErrInvalidAuth))
 			})
 
 			It("fails if JWT token sub is different than username", func() {
 				u := &model.User{UserName: "hacker"}
 				validToken, _ = auth.CreateToken(u)
-				_, err := validateUser(context.TODO(), ds, "admin", "", "", "", validToken)
+				err := validateCredentials(usr, "", "", "", validToken)
 				Expect(err).To(MatchError(model.ErrInvalidAuth))
 			})
 		})