Pull request 2592: 8198-fix-real-ip-detection

Updates #8198.

Squashed commit of the following:

commit a6b5134065b191462c7eb5a15e7886c7778dd320
Merge: b7bab6d84 f2032b0f3
Author: f.setrakov <f.setrakov@adguard.com>
Date:   Thu Feb 26 18:58:11 2026 +0300

    Merge branch 'master' into 8198-fix-real-ip-detection

commit b7bab6d84a30fc1bec72ac9a3a8d09371e4048ad
Author: f.setrakov <f.setrakov@adguard.com>
Date:   Thu Feb 26 15:37:32 2026 +0300

    home: fix ip log

commit 32915fc1d2adf507b060392d8041c6b5412bea4c
Author: f.setrakov <f.setrakov@adguard.com>
Date:   Wed Feb 25 13:02:09 2026 +0300

    all: chlog, imp naming

commit 21d5ca3bf508815c9a60616c71d4a6fa56b45d52
Author: f.setrakov <f.setrakov@adguard.com>
Date:   Tue Feb 24 18:24:30 2026 +0300

    home: fix real ip in logs
This commit is contained in:
Fedor Setrakov 2026-02-27 06:49:10 +00:00
parent f2032b0f39
commit fca3eeae6d
6 changed files with 38 additions and 15 deletions

View file

@ -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
<!--
NOTE: Add new changes ABOVE THIS COMMENT.
-->

View file

@ -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,

View file

@ -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)

View file

@ -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,

View file

@ -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(

View file

@ -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,