mirror of
https://github.com/navidrome/navidrome.git
synced 2025-12-23 15:08:04 -05:00
feat: rename "reverse proxy authentication" to "external authentication" (#4418)
* 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. * small refactor Signed-off-by: Deluan <deluan@navidrome.org> * add test Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Deluan Quintão <deluan@navidrome.org>
This commit is contained in:
@@ -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", "")
|
||||
|
||||
@@ -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))
|
||||
},
|
||||
|
||||
1
conf/testdata/cfg.ini
vendored
1
conf/testdata/cfg.ini
vendored
@@ -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
|
||||
|
||||
1
conf/testdata/cfg.json
vendored
1
conf/testdata/cfg.json
vendored
@@ -1,6 +1,7 @@
|
||||
{
|
||||
"musicFolder": "/json/music",
|
||||
"uiWelcomeMessage": "Welcome json",
|
||||
"reverseProxyUserHeader": "X-Auth-User",
|
||||
"Tags": {
|
||||
"artist": {
|
||||
"split": ";"
|
||||
|
||||
1
conf/testdata/cfg.toml
vendored
1
conf/testdata/cfg.toml
vendored
@@ -1,5 +1,6 @@
|
||||
musicFolder = "/toml/music"
|
||||
uiWelcomeMessage = "Welcome toml"
|
||||
ReverseProxyUserHeader = "X-Auth-User"
|
||||
|
||||
Tags.artist.Split = ';'
|
||||
|
||||
|
||||
1
conf/testdata/cfg.yaml
vendored
1
conf/testdata/cfg.yaml
vendored
@@ -1,5 +1,6 @@
|
||||
musicFolder: "/yaml/music"
|
||||
uiWelcomeMessage: "Welcome yaml"
|
||||
reverseProxyUserHeader: "X-Auth-User"
|
||||
Tags:
|
||||
artist:
|
||||
split: [";"]
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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]*\")[^\"]*",
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user