diff --git a/conf/configuration.go b/conf/configuration.go index f8e4a8084..77e9c94a5 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -87,8 +87,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"` @@ -230,6 +229,11 @@ type pluginsOptions struct { CacheSize string } +type extAuthOptions struct { + TrustedSources string + UserHeader string +} + var ( Server = &configOptions{} hooks []func() @@ -248,6 +252,10 @@ func LoadFromFile(confFile string) { func Load(noConfigDump bool) { parseIniFileConfiguration() + // Map deprecated options to their new names for backwards compatibility + mapDeprecatedOption("ReverseProxyWhitelist", "ExtAuth.TrustedSources") + mapDeprecatedOption("ReverseProxyUserHeader", "ExtAuth.UserHeader") + err := viper.Unmarshal(&Server) if err != nil { _, _ = fmt.Fprintln(os.Stderr, "FATAL: Error parsing config:", err) @@ -351,6 +359,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 { @@ -370,6 +379,14 @@ func logDeprecatedOptions(options ...string) { } } +// mapDeprecatedOption 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 mapDeprecatedOption(legacyName, newName string) { + if viper.IsSet(legacyName) { + viper.Set(newName, viper.Get(legacyName)) + } +} + // 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. @@ -538,8 +555,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/conf/configuration_test.go b/conf/configuration_test.go index 15d12795e..06973456f 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -41,6 +41,9 @@ var _ = Describe("Configuration", func() { Expect(conf.Server.Tags["custom"].Aliases).To(Equal([]string{format, "test"})) Expect(conf.Server.Tags["artist"].Split).To(Equal([]string{";"})) + // Check deprecated option mapping + Expect(conf.Server.ExtAuth.UserHeader).To(Equal("X-Auth-User")) + // The config file used should be the one we created Expect(conf.Server.ConfigFile).To(Equal(filename)) }, diff --git a/conf/testdata/cfg.ini b/conf/testdata/cfg.ini index e0062ff0e..cc8b2a4a5 100644 --- a/conf/testdata/cfg.ini +++ b/conf/testdata/cfg.ini @@ -1,6 +1,7 @@ [default] MusicFolder = /ini/music UIWelcomeMessage = 'Welcome ini' ; Just a comment to test the LoadOptions +ReverseProxyUserHeader = 'X-Auth-User' [Tags] Custom.Aliases = ini,test diff --git a/conf/testdata/cfg.json b/conf/testdata/cfg.json index 127103a53..28fb039d2 100644 --- a/conf/testdata/cfg.json +++ b/conf/testdata/cfg.json @@ -1,6 +1,7 @@ { "musicFolder": "/json/music", "uiWelcomeMessage": "Welcome json", + "reverseProxyUserHeader": "X-Auth-User", "Tags": { "artist": { "split": ";" diff --git a/conf/testdata/cfg.toml b/conf/testdata/cfg.toml index d94d786e2..589e2a100 100644 --- a/conf/testdata/cfg.toml +++ b/conf/testdata/cfg.toml @@ -1,5 +1,6 @@ musicFolder = "/toml/music" uiWelcomeMessage = "Welcome toml" +ReverseProxyUserHeader = "X-Auth-User" Tags.artist.Split = ';' diff --git a/conf/testdata/cfg.yaml b/conf/testdata/cfg.yaml index 66e12c4eb..e44d2ebbb 100644 --- a/conf/testdata/cfg.yaml +++ b/conf/testdata/cfg.yaml @@ -1,5 +1,6 @@ musicFolder: "/yaml/music" uiWelcomeMessage: "Welcome yaml" +reverseProxyUserHeader: "X-Auth-User" Tags: artist: split: [";"] diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 820e6d7b6..411bc9ac1 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -223,7 +223,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 801fd7214..24f3dff6e 100644 --- a/log/log.go +++ b/log/log.go @@ -29,8 +29,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() {