mirror of
https://github.com/AdguardTeam/AdGuardHome.git
synced 2026-06-28 11:51:22 +00:00
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:
parent
f2032b0f39
commit
fca3eeae6d
6 changed files with 38 additions and 15 deletions
|
|
@ -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.
|
||||
-->
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue