From 07b0584a664a55e6a49f95ef028094c681b301b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jeremiah=20Men=C3=A9trey?= Date: Fri, 1 Aug 2025 16:43:35 +0200 Subject: [PATCH] Rename external auth options 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 | 25 +++++++++++++++++++++---- core/metrics/insights.go | 2 +- log/log.go | 4 ++-- server/auth.go | 18 +++++++++--------- server/auth_test.go | 10 +++++----- server/middlewares.go | 2 +- server/subsonic/middlewares.go | 2 +- server/subsonic/middlewares_test.go | 8 ++++---- 8 files changed, 44 insertions(+), 27 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index 7292c7dfe..174adce97 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -86,8 +86,7 @@ type configOptions struct { AuthRequestLimit int AuthWindowLength time.Duration PasswordEncryptionKey string - ReverseProxyUserHeader string - ReverseProxyWhitelist string + ExtAuth extAuthOptions 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("reverseproxywhitelist", "") + viper.SetDefault("extauth.userheader", "Remote-User") + viper.SetDefault("extauth.trustedsources", "") viper.SetDefault("prometheus.enabled", false) viper.SetDefault("prometheus.metricspath", consts.PrometheusDefaultPath) viper.SetDefault("prometheus.password", "") diff --git a/core/metrics/insights.go b/core/metrics/insights.go index f4f8738e7..0f2a4876e 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -207,7 +207,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 diff --git a/log/log.go b/log/log.go index 20119ab46..6082adf49 100644 --- a/log/log.go +++ b/log/log.go @@ -28,8 +28,8 @@ var redacted = &Hook{ "(Secret:\")[\\w]*", "(Spotify.*ID:\")[\\w]*", "(PasswordEncryptionKey:[\\s]*\")[^\"]*", - "(ReverseProxyUserHeader:[\\s]*\")[^\"]*", - "(ReverseProxyWhitelist:[\\s]*\")[^\"]*", + "(UserHeader:[\\s]*\")[^\"]*", + "(TrustedSources:[\\s]*\")[^\"]*", "(MetricsPath:[\\s]*\")[^\"]*", "(DevAutoCreateAdminPassword:[\\s]*\")[^\"]*", "(DevAutoLoginUsername:[\\s]*\")[^\"]*", diff --git a/server/auth.go b/server/auth.go index ed43974dd..8588549ab 100644 --- a/server/auth.go +++ b/server/auth.go @@ -193,24 +193,24 @@ func UsernameFromToken(r *http.Request) string { return token.Subject() } -func UsernameFromReverseProxyHeader(r *http.Request) string { - if conf.Server.ReverseProxyWhitelist == "" { +func UsernameFromExtAuthHeader(r *http.Request) string { + 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) { - log.Warn(r.Context(), "IP is not whitelisted for reverse proxy login", "proxy-ip", reverseProxyIp, "client-ip", r.RemoteAddr) + if !validateIPAgainstList(reverseProxyIp, conf.Server.ExtAuth.TrustedSources) { + 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 } diff --git a/server/auth_test.go b/server/auth_test.go index 06ca2ea39..633299096 100644 --- a/server/auth_test.go +++ b/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.ReverseProxyUserHeader = "Remote-User" + conf.Server.ExtAuth.TrustedSources = "192.168.0.0/16" + conf.Server.ExtAuth.UserHeader = "Remote-User" }) It("makes the first user an admin", func() { diff --git a/server/middlewares.go b/server/middlewares.go index 2afe09a5a..21f897931 100644 --- a/server/middlewares.go +++ b/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, diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index af1ba448f..d984bac42 100644 --- a/server/subsonic/middlewares.go +++ b/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 { diff --git a/server/subsonic/middlewares_test.go b/server/subsonic/middlewares_test.go index a30d5b3af..aba14a0aa 100644 --- a/server/subsonic/middlewares_test.go +++ b/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.ReverseProxyUserHeader = "Remote-User" + conf.Server.ExtAuth.TrustedSources = "127.0.0.234/32" + 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.ReverseProxyUserHeader = "Remote-User" + conf.Server.ExtAuth.TrustedSources = "192.168.1.1/24" + conf.Server.ExtAuth.UserHeader = "Remote-User" }) It("passes authentication with correct IP and header", func() {