fix(auth): ExtAuth logout redirect on unauthenticated loads, and warning spam from untrusted sources (#6176)

* fix(ui): only redirect to ExtAuth logout URL for proxy-authenticated sessions

react-admin calls authProvider.logout() when the boot-time checkAuth fails
and after a 401, not only when the user clicks Logout. With
ExtAuth.LogoutURL set, every unauthenticated page load (e.g. direct LAN
access that bypasses the auth proxy) was sent to the IdP sign-out page and
the login form was never shown.

Redirect only when the page was authenticated by the reverse proxy
(config.auth is present). Other sessions fall back to the login form.

Fixes #6175

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(server): only warn about untrusted ExtAuth sources when the header is sent

UsernameFromExtAuthHeader checked the source IP before looking for the user
header, so every request from an IP outside ExtAuth.TrustedSources logged a
warning, even when it carried no header at all. With direct LAN access
alongside a forward-auth proxy, a single polling client produced a constant
stream of warnings (twice per Subsonic request, since the middleware chain
resolves the username in both checkRequiredParameters and authenticate).

Look for the header first and warn only when an untrusted source actually
sends it, which is the case worth seeing: a misconfigured proxy or a spoof
attempt.

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

server/auth.go

@@ -218,14 +218,14 @@ func UsernameFromExtAuthHeader(r *http.Request) string {
 		log.Error("ExtAuth enabled but no proxy IP found in request context. Please report this error.")
 		return ""
 	}
-	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.ExtAuth.UserHeader)
 	if username == "" {
 		return ""
 	}
+	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 ""
+	}
 	log.Trace(r, "Found username in ExtAuth.UserHeader", "username", username)
 	return username
 }

server/auth_test.go

@@ -16,12 +16,15 @@ import (
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/core/auth"
+	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/id"
 	"github.com/navidrome/navidrome/model/request"
 	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 )
 
 var _ = Describe("Auth", func() {
@@ -234,6 +237,56 @@ var _ = Describe("Auth", func() {
 		})
 	})
 
+	Describe("UsernameFromExtAuthHeader", func() {
+		var hook *test.Hook
+		var r *http.Request
+
+		BeforeEach(func() {
+			conf.Server.ExtAuth.TrustedSources = "192.168.0.0/16"
+			prevLevel := log.CurrentLevel()
+			l, h := test.NewNullLogger()
+			hook = h
+			prevLogger := log.SetDefaultLogger(l)
+			log.SetLevel(log.LevelWarn)
+			DeferCleanup(func() {
+				log.SetDefaultLogger(prevLogger)
+				log.SetLevel(prevLevel)
+			})
+			r = httptest.NewRequest("GET", "/", nil)
+		})
+
+		warnings := func() []*logrus.Entry {
+			var ws []*logrus.Entry
+			for _, e := range hook.AllEntries() {
+				if e.Level == logrus.WarnLevel {
+					ws = append(ws, e)
+				}
+			}
+			return ws
+		}
+
+		It("returns the username from a trusted source", func() {
+			r.Header.Set("Remote-User", "janedoe")
+			r = r.WithContext(request.WithReverseProxyIp(r.Context(), "192.168.0.42"))
+			Expect(UsernameFromExtAuthHeader(r)).To(Equal("janedoe"))
+			Expect(warnings()).To(BeEmpty())
+		})
+
+		It("does not warn when an untrusted source sends no user header", func() {
+			r = r.WithContext(request.WithReverseProxyIp(r.Context(), "8.8.8.8"))
+			Expect(UsernameFromExtAuthHeader(r)).To(BeEmpty())
+			Expect(warnings()).To(BeEmpty())
+		})
+
+		It("warns when an untrusted source sends the user header", func() {
+			r.Header.Set("Remote-User", "janedoe")
+			r = r.WithContext(request.WithReverseProxyIp(r.Context(), "8.8.8.8"))
+			Expect(UsernameFromExtAuthHeader(r)).To(BeEmpty())
+			Expect(warnings()).To(HaveLen(1))
+			Expect(warnings()[0].Message).To(Equal("IP is not whitelisted for external authentication"))
+		})
+	})
+
 	Describe("tokenFromHeader", func() {
 		It("returns the token when the Authorization header is set correctly", func() {
 			req := httptest.NewRequest("GET", "/", nil)

ui/src/authProvider.js

@@ -64,7 +64,8 @@ const authProvider = {
 
   logout: () => {
     removeItems()
-    if (config.extAuthLogoutURL) {
+    // Only proxy-authenticated sessions go to the IdP; others (e.g. direct LAN access) get the login form
+    if (config.extAuthLogoutURL && config.auth) {
       window.location.href = config.extAuthLogoutURL
       return Promise.resolve(false)
     }

ui/src/authProvider.test.js

@@ -0,0 +1,47 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
+import config from './config'
+import authProvider from './authProvider'
+
+vi.mock('./config', () => ({ default: {} }))
+
+describe('authProvider.logout', () => {
+  const logoutURL = 'https://auth.example.com/signout'
+
+  beforeEach(() => {
+    vi.stubGlobal('location', { href: '' })
+    localStorage.setItem('is-authenticated', 'true')
+    localStorage.setItem('token', 'abc')
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+    localStorage.clear()
+    delete config.extAuthLogoutURL
+    delete config.auth
+  })
+
+  it('clears the stored session', async () => {
+    await authProvider.logout()
+    expect(localStorage.getItem('is-authenticated')).toBeNull()
+    expect(localStorage.getItem('token')).toBeNull()
+  })
+
+  it('does not redirect when no logout URL is configured', async () => {
+    config.auth = { id: '1' }
+    await expect(authProvider.logout()).resolves.toBeUndefined()
+    expect(window.location.href).toBe('')
+  })
+
+  it('redirects to the logout URL when the page was authenticated by the proxy', async () => {
+    config.extAuthLogoutURL = logoutURL
+    config.auth = { id: '1' }
+    await expect(authProvider.logout()).resolves.toBe(false)
+    expect(window.location.href).toBe(logoutURL)
+  })
+
+  it('does not redirect when the page was not authenticated by the proxy', async () => {
+    config.extAuthLogoutURL = logoutURL
+    await expect(authProvider.logout()).resolves.toBeUndefined()
+    expect(window.location.href).toBe('')
+  })
+})

ui/src/setupTests.js

@@ -14,6 +14,9 @@ const localStorageMock = (function () {
     setItem: function (key, value) {
       store[key] = value.toString()
     },
+    removeItem: function (key) {
+      delete store[key]
+    },
     clear: function () {
       store = {}
     },