Merge 07b0584a664a55e6a49f95ef028094c681b301b9 into 2385c8a548f6d71e8b1acba503ae0161a9ddcc1e

ReverseProxyWhitelist was regularly confusing users that enabled it for
non-authenticating reverse proxy setups.

The new option name makes it clear that it's related to authentication, not
just reverse proxies.

conf/configuration.go

@@ -86,8 +86,7 @@ type configOptions struct {
 	AuthRequestLimit                int
 	AuthWindowLength                time.Duration
 	PasswordEncryptionKey           string
-	ReverseProxyUserHeader          string
+	ExtAuth                         extAuthOptions
-	ReverseProxyWhitelist           string
 	Plugins                         pluginsOptions
 	PluginConfig                    map[string]map[string]string
 	HTTPSecurityHeaders             secureOptions       `json:",omitzero"`
@@ -226,6 +225,11 @@ type pluginsOptions struct {
 	CacheSize string
 }
 
+type extAuthOptions struct {
+	TrustedSources string
+	UserHeader     string
+}
+
 var (
 	Server = &configOptions{}
 	hooks  []func()
@@ -243,6 +247,7 @@ func LoadFromFile(confFile string) {
 
 func Load(noConfigDump bool) {
 	parseIniFileConfiguration()
+	mapDeprecatedOptions()
 
 	err := viper.Unmarshal(&Server)
 	if err != nil {
@@ -347,6 +352,7 @@ func Load(noConfigDump bool) {
 	logDeprecatedOptions("Scanner.GenreSeparators")
 	logDeprecatedOptions("Scanner.GroupAlbumReleases")
 	logDeprecatedOptions("DevEnableBufferedScrobble") // Deprecated: Buffered scrobbling is now always enabled and this option is ignored
+	logDeprecatedOptions("ReverseProxyWhitelist", "ReverseProxyUserHeader")
 
 	// Call init hooks
 	for _, hook := range hooks {
@@ -366,6 +372,17 @@ func logDeprecatedOptions(options ...string) {
 	}
 }
 
+// mapDeprecatedOptions is used to provide backwards compatibility for deprecated options. It should be called after
+// the config has been read by viper, but before unmarshalling it into the Config struct.
+func mapDeprecatedOptions() {
+	if viper.IsSet("ReverseProxyWhitelist") {
+		viper.Set("ExtAuth.TrustedSources", viper.Get("ReverseProxyWhitelist"))
+	}
+	if viper.IsSet("ReverseProxyUserHeader") {
+		viper.Set("ExtAuth.UserHeader", viper.Get("ReverseProxyUserHeader"))
+	}
+}
+
 // parseIniFileConfiguration is used to parse the config file when it is in INI format. For INI files, it
 // would require a nested structure, so instead we unmarshal it to a map and then merge the nested [default]
 // section into the root level.
@@ -533,8 +550,8 @@ func setViperDefaults() {
 	viper.SetDefault("authrequestlimit", 5)
 	viper.SetDefault("authwindowlength", 20*time.Second)
 	viper.SetDefault("passwordencryptionkey", "")
-	viper.SetDefault("reverseproxyuserheader", "Remote-User")
+	viper.SetDefault("extauth.userheader", "Remote-User")
-	viper.SetDefault("reverseproxywhitelist", "")
+	viper.SetDefault("extauth.trustedsources", "")
 	viper.SetDefault("prometheus.enabled", false)
 	viper.SetDefault("prometheus.metricspath", consts.PrometheusDefaultPath)
 	viper.SetDefault("prometheus.password", "")

core/metrics/insights.go

@@ -215,7 +215,7 @@ var staticData = sync.OnceValue(func() insights.Data {
 	data.Config.ScanSchedule = conf.Server.Scanner.Schedule
 	data.Config.ScanWatcherWait = uint64(math.Trunc(conf.Server.Scanner.WatcherWait.Seconds()))
 	data.Config.ScanOnStartup = conf.Server.Scanner.ScanOnStartup
-	data.Config.ReverseProxyConfigured = conf.Server.ReverseProxyWhitelist != ""
+	data.Config.ReverseProxyConfigured = conf.Server.ExtAuth.TrustedSources != ""
 	data.Config.HasCustomPID = conf.Server.PID.Track != "" || conf.Server.PID.Album != ""
 	data.Config.HasCustomTags = len(conf.Server.Tags) > 0
 

log/log.go

@@ -29,8 +29,8 @@ var redacted = &Hook{
 		"(Secret:\")[\\w]*",
 		"(Spotify.*ID:\")[\\w]*",
 		"(PasswordEncryptionKey:[\\s]*\")[^\"]*",
-		"(ReverseProxyUserHeader:[\\s]*\")[^\"]*",
+		"(UserHeader:[\\s]*\")[^\"]*",
-		"(ReverseProxyWhitelist:[\\s]*\")[^\"]*",
+		"(TrustedSources:[\\s]*\")[^\"]*",
 		"(MetricsPath:[\\s]*\")[^\"]*",
 		"(DevAutoCreateAdminPassword:[\\s]*\")[^\"]*",
 		"(DevAutoLoginUsername:[\\s]*\")[^\"]*",

server/auth.go

@@ -193,24 +193,24 @@ func UsernameFromToken(r *http.Request) string {
 	return token.Subject()
 }
 
-func UsernameFromReverseProxyHeader(r *http.Request) string {
+func UsernameFromExtAuthHeader(r *http.Request) string {
-	if conf.Server.ReverseProxyWhitelist == "" {
+	if conf.Server.ExtAuth.TrustedSources == "" {
 		return ""
 	}
 	reverseProxyIp, ok := request.ReverseProxyIpFrom(r.Context())
 	if !ok {
-		log.Error("ReverseProxyWhitelist enabled but no proxy IP found in request context. Please report this error.")
+		log.Error("ExtAuth enabled but no proxy IP found in request context. Please report this error.")
 		return ""
 	}
-	if !validateIPAgainstList(reverseProxyIp, conf.Server.ReverseProxyWhitelist) {
+	if !validateIPAgainstList(reverseProxyIp, conf.Server.ExtAuth.TrustedSources) {
-		log.Warn(r.Context(), "IP is not whitelisted for reverse proxy login", "proxy-ip", reverseProxyIp, "client-ip", r.RemoteAddr)
+		log.Warn(r.Context(), "IP is not whitelisted for external authentication", "proxy-ip", reverseProxyIp, "client-ip", r.RemoteAddr)
 		return ""
 	}
-	username := r.Header.Get(conf.Server.ReverseProxyUserHeader)
+	username := r.Header.Get(conf.Server.ExtAuth.UserHeader)
 	if username == "" {
 		return ""
 	}
-	log.Trace(r, "Found username in ReverseProxyUserHeader", "username", username)
+	log.Trace(r, "Found username in ExtAuth.UserHeader", "username", username)
 	return username
 }
 
@@ -256,7 +256,7 @@ func authenticateRequest(ds model.DataStore, r *http.Request, findUsernameFns ..
 func Authenticator(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, err := authenticateRequest(ds, r, UsernameFromConfig, UsernameFromToken, UsernameFromReverseProxyHeader)
+			ctx, err := authenticateRequest(ds, r, UsernameFromConfig, UsernameFromToken, UsernameFromExtAuthHeader)
 			if err != nil {
 				_ = rest.RespondWithError(w, http.StatusUnauthorized, "Not authenticated")
 				return
@@ -291,7 +291,7 @@ func JWTRefresher(next http.Handler) http.Handler {
 func handleLoginFromHeaders(ds model.DataStore, r *http.Request) map[string]interface{} {
 	username := UsernameFromConfig(r)
 	if username == "" {
-		username = UsernameFromReverseProxyHeader(r)
+		username = UsernameFromExtAuthHeader(r)
 		if username == "" {
 			return nil
 		}

server/auth_test.go

@@ -80,7 +80,7 @@ var _ = Describe("Auth", func() {
 				req.Header.Add("Remote-User", "janedoe")
 				resp = httptest.NewRecorder()
 				conf.Server.UILoginBackgroundURL = ""
-				conf.Server.ReverseProxyWhitelist = "192.168.0.0/16,2001:4860:4860::/48"
+				conf.Server.ExtAuth.TrustedSources = "192.168.0.0/16,2001:4860:4860::/48"
 			})
 
 			It("sets auth data if IPv4 matches whitelist", func() {
@@ -155,7 +155,7 @@ var _ = Describe("Auth", func() {
 
 			It("does not set auth data when listening on unix socket without whitelist", func() {
 				conf.Server.Address = "unix:/tmp/navidrome-test"
-				conf.Server.ReverseProxyWhitelist = ""
+				conf.Server.ExtAuth.TrustedSources = ""
 
 				// No ReverseProxyIp in request context
 				serveIndex(ds, fs, nil)(resp, req)
@@ -176,7 +176,7 @@ var _ = Describe("Auth", func() {
 
 			It("sets auth data when listening on unix socket with correct whitelist", func() {
 				conf.Server.Address = "unix:/tmp/navidrome-test"
-				conf.Server.ReverseProxyWhitelist = conf.Server.ReverseProxyWhitelist + ",@"
+				conf.Server.ExtAuth.TrustedSources = conf.Server.ExtAuth.TrustedSources + ",@"
 
 				req = req.WithContext(request.WithReverseProxyIp(req.Context(), "@"))
 				serveIndex(ds, fs, nil)(resp, req)
@@ -302,8 +302,8 @@ var _ = Describe("Auth", func() {
 			ds = &tests.MockDataStore{}
 			req = httptest.NewRequest("GET", "/", nil)
 			req = req.WithContext(request.WithReverseProxyIp(req.Context(), trustedIP))
-			conf.Server.ReverseProxyWhitelist = "192.168.0.0/16"
+			conf.Server.ExtAuth.TrustedSources = "192.168.0.0/16"
-			conf.Server.ReverseProxyUserHeader = "Remote-User"
+			conf.Server.ExtAuth.UserHeader = "Remote-User"
 		})
 
 		It("makes the first user an admin", func() {

server/middlewares.go

@@ -168,7 +168,7 @@ func clientUniqueIDMiddleware(next http.Handler) http.Handler {
 // realIPMiddleware applies middleware.RealIP, and additionally saves the request's original RemoteAddr to the request's
 // context if navidrome is behind a trusted reverse proxy.
 func realIPMiddleware(next http.Handler) http.Handler {
-	if conf.Server.ReverseProxyWhitelist != "" {
+	if conf.Server.ExtAuth.TrustedSources != "" {
 		return chi.Chain(
 			reqToCtx(request.ReverseProxyIp, func(r *http.Request) any { return r.RemoteAddr }),
 			middleware.RealIP,

server/subsonic/middlewares.go

@@ -56,7 +56,7 @@ func fromInternalOrProxyAuth(r *http.Request) (string, bool) {
 		return username, true
 	}
 
-	return server.UsernameFromReverseProxyHeader(r), false
+	return server.UsernameFromExtAuthHeader(r), false
 }
 
 func checkRequiredParameters(next http.Handler) http.Handler {

server/subsonic/middlewares_test.go

@@ -95,8 +95,8 @@ var _ = Describe("Middlewares", func() {
 		})
 
 		It("passes when all required params are available (reverse-proxy case)", func() {
-			conf.Server.ReverseProxyWhitelist = "127.0.0.234/32"
+			conf.Server.ExtAuth.TrustedSources = "127.0.0.234/32"
-			conf.Server.ReverseProxyUserHeader = "Remote-User"
+			conf.Server.ExtAuth.UserHeader = "Remote-User"
 
 			r := newGetRequest("v=1.15", "c=test")
 			r.Header.Add("Remote-User", "user")
@@ -254,8 +254,8 @@ var _ = Describe("Middlewares", func() {
 		When("using reverse proxy authentication", func() {
 			BeforeEach(func() {
 				DeferCleanup(configtest.SetupConfig())
-				conf.Server.ReverseProxyWhitelist = "192.168.1.1/24"
+				conf.Server.ExtAuth.TrustedSources = "192.168.1.1/24"
-				conf.Server.ReverseProxyUserHeader = "Remote-User"
+				conf.Server.ExtAuth.UserHeader = "Remote-User"
 			})
 
 			It("passes authentication with correct IP and header", func() {