From fca3eeae6d152d44aeb800044ccb13c7bec0ecb7 Mon Sep 17 00:00:00 2001 From: Fedor Setrakov Date: Fri, 27 Feb 2026 06:49:10 +0000 Subject: [PATCH] Pull request 2592: 8198-fix-real-ip-detection Updates #8198. Squashed commit of the following: commit a6b5134065b191462c7eb5a15e7886c7778dd320 Merge: b7bab6d84 f2032b0f3 Author: f.setrakov Date: Thu Feb 26 18:58:11 2026 +0300 Merge branch 'master' into 8198-fix-real-ip-detection commit b7bab6d84a30fc1bec72ac9a3a8d09371e4048ad Author: f.setrakov Date: Thu Feb 26 15:37:32 2026 +0300 home: fix ip log commit 32915fc1d2adf507b060392d8041c6b5412bea4c Author: f.setrakov Date: Wed Feb 25 13:02:09 2026 +0300 all: chlog, imp naming commit 21d5ca3bf508815c9a60616c71d4a6fa56b45d52 Author: f.setrakov Date: Tue Feb 24 18:24:30 2026 +0300 home: fix real ip in logs --- CHANGELOG.md | 4 +++ internal/home/auth_internal_test.go | 2 +- internal/home/authhttp.go | 36 +++++++++++++++------- internal/home/authhttp_internal_test.go | 4 +-- internal/home/home_internal_test.go | 5 +++ internal/home/profilehttp_internal_test.go | 2 +- 6 files changed, 38 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d03681260..5330a6408 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,8 +20,12 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Fixed +- Incorrect client IP logging in failed authentication attempts when using a proxy ([#8198]). + - Incorrect logger behavior in case `-v` flag is added. +[#8198]: https://github.com/AdguardTeam/AdGuardHome/issues/8198 + diff --git a/internal/home/auth_internal_test.go b/internal/home/auth_internal_test.go index 11af39e81..22dcfcf69 100644 --- a/internal/home/auth_internal_test.go +++ b/internal/home/auth_internal_test.go @@ -31,7 +31,7 @@ func TestAuth_UsersList(t *testing.T) { auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ baseLogger: testLogger, rateLimiter: emptyRateLimiter{}, - trustedProxies: nil, + trustedProxies: testTrustedProxies, dbFilename: sessionsDB, users: nil, sessionTTL: testTimeout, diff --git a/internal/home/authhttp.go b/internal/home/authhttp.go index 1f3a325ac..b9393e07d 100644 --- a/internal/home/authhttp.go +++ b/internal/home/authhttp.go @@ -108,12 +108,12 @@ func (web *webAPI) handleLogin(w http.ResponseWriter, r *http.Request) { return } - var remoteIP string + var remoteIPStr string // The real IP address of the client [realIP] cannot be used here without // taking trusted proxies into account due to security issues: // // See https://github.com/AdguardTeam/AdGuardHome/issues/2799. - if remoteIP, err = netutil.SplitHost(r.RemoteAddr); err != nil { + if remoteIPStr, err = netutil.SplitHost(r.RemoteAddr); err != nil { writeErrorWithIP( r, w, @@ -127,13 +127,13 @@ func (web *webAPI) handleLogin(w http.ResponseWriter, r *http.Request) { } if rateLimiter := web.auth.rateLimiter; rateLimiter != nil { - if left := rateLimiter.check(remoteIP); left > 0 { + if left := rateLimiter.check(remoteIPStr); left > 0 { w.Header().Set(httphdr.RetryAfter, strconv.Itoa(int(left.Seconds()))) writeErrorWithIP( r, w, http.StatusTooManyRequests, - remoteIP, + remoteIPStr, "auth: blocked for %s", left, ) @@ -147,24 +147,38 @@ func (web *webAPI) handleLogin(w http.ResponseWriter, r *http.Request) { web.logger.ErrorContext( ctx, "getting real ip", - "remote_ip", remoteIP, + "remote_ip", remoteIPStr, slogutil.KeyError, err, ) } - cookie, err := newCookie(ctx, web.auth, req, remoteIP) + remoteIP, err := netip.ParseAddr(remoteIPStr) if err != nil { - logIP := remoteIP - if web.auth.trustedProxies.Contains(ip.Unmap()) { - logIP = ip.String() - } + writeErrorWithIP( + r, + w, + http.StatusInternalServerError, + r.RemoteAddr, + "auth: parsing remote address: %s", + err, + ) + return + } + + logIP := remoteIPStr + if web.auth.trustedProxies.Contains(remoteIP.Unmap()) { + logIP = ip.String() + } + + cookie, err := newCookie(ctx, web.auth, req, remoteIPStr) + if err != nil { writeErrorWithIP(r, w, http.StatusForbidden, logIP, "%s", err) return } - web.logger.InfoContext(ctx, "successful login", "user", req.Name, "ip", ip) + web.logger.InfoContext(ctx, "successful login", "user", req.Name, "ip", logIP) http.SetCookie(w, cookie) diff --git a/internal/home/authhttp_internal_test.go b/internal/home/authhttp_internal_test.go index c65cbd5a9..4fc3ea764 100644 --- a/internal/home/authhttp_internal_test.go +++ b/internal/home/authhttp_internal_test.go @@ -467,7 +467,7 @@ func TestAuth_ServeHTTP_auth(t *testing.T) { auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ baseLogger: testLogger, rateLimiter: emptyRateLimiter{}, - trustedProxies: nil, + trustedProxies: testTrustedProxies, dbFilename: sessionsDB, users: users, sessionTTL: testTTL * time.Second, @@ -623,7 +623,7 @@ func TestAuth_ServeHTTP_logout(t *testing.T) { auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ baseLogger: testLogger, rateLimiter: emptyRateLimiter{}, - trustedProxies: nil, + trustedProxies: testTrustedProxies, dbFilename: sessionsDB, users: users, sessionTTL: testTTL * time.Second, diff --git a/internal/home/home_internal_test.go b/internal/home/home_internal_test.go index e6d3b913e..78b03154d 100644 --- a/internal/home/home_internal_test.go +++ b/internal/home/home_internal_test.go @@ -3,11 +3,13 @@ package home import ( "cmp" "net/http" + "net/netip" "testing" "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/require" ) @@ -15,6 +17,9 @@ import ( // testLogger is a common logger for tests. var testLogger = slogutil.NewDiscardLogger() +// testTrustedProxies is a common trusted proxies set for tests. +var testTrustedProxies = netutil.SliceSubnetSet([]netip.Prefix{}) + // newTestWeb is a helper that creates new webAPI and fills it's config with // given values. If conf is nil, the default configuration will be used. func newTestWeb( diff --git a/internal/home/profilehttp_internal_test.go b/internal/home/profilehttp_internal_test.go index 88c9c1384..7b82e7a31 100644 --- a/internal/home/profilehttp_internal_test.go +++ b/internal/home/profilehttp_internal_test.go @@ -62,7 +62,7 @@ func TestWeb_HandleGetProfile(t *testing.T) { auth, err := newAuth(testutil.ContextWithTimeout(t, testTimeout), &authConfig{ baseLogger: testLogger, rateLimiter: emptyRateLimiter{}, - trustedProxies: nil, + trustedProxies: testTrustedProxies, dbFilename: sessionsDB, users: nil, sessionTTL: testTTL * time.Second,