mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 20:37:12 +02:00
055fbde3cf
The RealIP middleware rewrote RemoteAddr from the True-Client-IP, X-Real-IP and X-Forwarded-For headers on every request, including when no trusted reverse proxy was configured. The login rate limiters on /auth/login and the Jellyfin /Users/AuthenticateByName derived their bucket from that value, so an unauthenticated client could rotate a forwarding header and get a fresh bucket for every password attempt, defeating the brute-force protection. Resolve the client IP with chi's ClientIPFrom* middlewares instead. The forwarding headers are only honoured when ExtAuth.TrustedSources is set and the connecting peer is in that list, reusing the trust check that external authentication already applies; otherwise the peer address is used. The X-Forwarded-For chain is now walked against the trusted CIDRs rather than taking its leftmost entry, so a spoofed value prepended by the client is skipped. Both limiters now key on the resolved address. The resolved address is still mirrored into RemoteAddr, so request logging, player registration and the Jellyfin local-network check keep reporting the client rather than the proxy. Reported by gehan-psbc.
110 lines
4.2 KiB
Go
110 lines
4.2 KiB
Go
package jellyfin
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"time"
|
|
|
|
"github.com/go-chi/chi/v5/middleware"
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
"github.com/navidrome/navidrome/core/auth"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/tests"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("Router", func() {
|
|
It("serves the public handshake through the mounted handler", func() {
|
|
ds := &tests.MockDataStore{}
|
|
api := New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("GET", "/System/Info/Public", nil)
|
|
api.ServeHTTP(w, r)
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
})
|
|
|
|
It("returns 404 JSON for unknown routes", func() {
|
|
api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("GET", "/Nonexistent/Route", nil)
|
|
api.ServeHTTP(w, r)
|
|
Expect(w.Code).To(Equal(http.StatusNotFound))
|
|
Expect(w.Header().Get("Content-Type")).To(ContainSubstring("application/json"))
|
|
Expect(w.Body.String()).To(Equal("{}"))
|
|
})
|
|
|
|
It("returns 404 JSON for a known path with an unsupported method", func() {
|
|
api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("PATCH", "/System/Info/Public", nil)
|
|
api.ServeHTTP(w, r)
|
|
Expect(w.Code).To(Equal(http.StatusNotFound))
|
|
Expect(w.Body.String()).To(Equal("{}"))
|
|
})
|
|
|
|
It("registers a player on a general authenticated request, not just playback reports", func() {
|
|
ds := &tests.MockDataStore{}
|
|
auth.Init(ds)
|
|
ur := ds.User(GinkgoT().Context()).(*tests.MockedUserRepo)
|
|
Expect(ur.Put(&model.User{ID: testID("u1"), UserName: "alice", NewPassword: "secret"})).To(Succeed())
|
|
token, err := auth.CreateToken(&model.User{ID: testID("u1"), UserName: "alice"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
fp := &fakePlayers{}
|
|
api := New(ds, nil, nil, nil, fp, nil, nil, nil, nil, nil, nil)
|
|
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("GET", "/Users/Me", nil)
|
|
r.Header.Set("X-Emby-Authorization", `MediaBrowser Client="Jellify", Device="Phone", DeviceId="dev-1", Version="1.0"`)
|
|
r.Header.Set("X-Emby-Token", token)
|
|
api.ServeHTTP(w, r)
|
|
|
|
Expect(w.Code).To(Equal(http.StatusOK))
|
|
Expect(fp.registerCalls).To(Equal(1))
|
|
Expect(fp.lastClient).To(Equal("Jellify"))
|
|
})
|
|
|
|
It("rate-limits AuthenticateByName by IP when a login limit is configured", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.AuthRequestLimit = 2
|
|
conf.Server.AuthWindowLength = time.Minute
|
|
api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
|
|
|
|
login := func() int {
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Users/AuthenticateByName", strings.NewReader(`{"Username":"x","Pw":"y"}`))
|
|
r.RemoteAddr = "10.0.0.1:1234"
|
|
api.ServeHTTP(w, r)
|
|
return w.Code
|
|
}
|
|
// The bad credentials would be 401; the limiter cuts in on the 3rd attempt with 429.
|
|
Expect(login()).To(Equal(http.StatusUnauthorized))
|
|
Expect(login()).To(Equal(http.StatusUnauthorized))
|
|
Expect(login()).To(Equal(http.StatusTooManyRequests))
|
|
})
|
|
|
|
It("rate-limits AuthenticateByName by resolved client IP, not by the proxy connection", func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
conf.Server.AuthRequestLimit = 1
|
|
conf.Server.AuthWindowLength = time.Minute
|
|
api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
|
|
// Every request arrives on the same proxy connection, so only the resolved client IP
|
|
// can separate the buckets.
|
|
handler := middleware.ClientIPFromHeader("X-Real-IP")(api)
|
|
|
|
login := func(clientIP string) int {
|
|
w := httptest.NewRecorder()
|
|
r := httptest.NewRequest("POST", "/Users/AuthenticateByName", strings.NewReader(`{"Username":"x","Pw":"y"}`))
|
|
r.RemoteAddr = "10.0.0.1:1234"
|
|
r.Header.Set("X-Real-IP", clientIP)
|
|
handler.ServeHTTP(w, r)
|
|
return w.Code
|
|
}
|
|
Expect(login("203.0.113.1")).To(Equal(http.StatusUnauthorized))
|
|
Expect(login("203.0.113.1")).To(Equal(http.StatusTooManyRequests))
|
|
Expect(login("203.0.113.2")).To(Equal(http.StatusUnauthorized))
|
|
})
|
|
})
|