From a280fd2c345d449d44c9ee59e550a8ae590d743b Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 23:23:55 +0000 Subject: [PATCH] test: cover resolved client IP in request and auth logs Assert the ClientIP middleware stores the resolved address in the request context for trusted and untrusted peers, that the request log records the forwarded client rather than the proxy peer, and that a failed-login record carries the resolved address. Model: opus-4-8 --- .../handlers/auth_clientip_internal_test.go | 41 ++++++ .../middleware/client_ip_internal_test.go | 118 ++++++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 internal/handlers/auth_clientip_internal_test.go create mode 100644 internal/middleware/client_ip_internal_test.go diff --git a/internal/handlers/auth_clientip_internal_test.go b/internal/handlers/auth_clientip_internal_test.go new file mode 100644 index 0000000..91d3fd2 --- /dev/null +++ b/internal/handlers/auth_clientip_internal_test.go @@ -0,0 +1,41 @@ +package handlers + +import ( + "bytes" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/clientip" + "sneak.berlin/go/pixa/internal/config" +) + +// TestFailedLoginLogsResolvedClientIP verifies the failed-login record +// carries the resolved client IP from the request context, not the raw +// proxy peer address. +func TestFailedLoginLogsResolvedClientIP(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + + h := &Handlers{ + log: slog.New(slog.NewJSONHandler(&buf, nil)), + config: &config.Config{SigningKey: testSigningKey}, + } + + form := url.Values{loginKeyField: {"wrong-key"}} + req := httptest.NewRequestWithContext( + t.Context(), http.MethodPost, "/", + strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req = req.WithContext(clientip.WithClientIP(req.Context(), "203.0.113.7")) + + h.handleLoginPost(httptest.NewRecorder(), req) + + if !strings.Contains(buf.String(), `"remote_addr":"203.0.113.7"`) { + t.Errorf("failed-login log missing resolved client IP; got %q", buf.String()) + } +} diff --git a/internal/middleware/client_ip_internal_test.go b/internal/middleware/client_ip_internal_test.go new file mode 100644 index 0000000..7200f46 --- /dev/null +++ b/internal/middleware/client_ip_internal_test.go @@ -0,0 +1,118 @@ +package middleware + +import ( + "bytes" + "log/slog" + "net/http" + "net/http/httptest" + "net/netip" + "strings" + "testing" + + "sneak.berlin/go/pixa/internal/clientip" + "sneak.berlin/go/pixa/internal/config" +) + +// testForwardedClient is the client address the proxy forwards. +const testForwardedClient = "203.0.113.7" + +// newTestMiddleware builds a Middleware whose resolver trusts the given +// CIDRs and whose logger writes JSON to buf. +func newTestMiddleware(t *testing.T, buf *bytes.Buffer, trusted ...string) *Middleware { + t.Helper() + + prefixes := make([]netip.Prefix, 0, len(trusted)) + + for _, c := range trusted { + p, err := netip.ParsePrefix(c) + if err != nil { + t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err) + } + + prefixes = append(prefixes, p) + } + + return &Middleware{ + log: slog.New(slog.NewJSONHandler(buf, nil)), + config: &config.Config{TrustedProxies: prefixes}, + clientIP: clientip.NewResolver(prefixes), + } +} + +// TestClientIPMiddlewareStoresResolvedIP verifies the ClientIP middleware +// puts the resolved address into the request context for a trusted and an +// untrusted peer. +func TestClientIPMiddlewareStoresResolvedIP(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + remoteAddr string + forwarded string + want string + }{ + { + name: "trusted peer honors forwarded client", + remoteAddr: "10.0.0.1:5000", + forwarded: testForwardedClient, + want: testForwardedClient, + }, + { + name: "untrusted peer ignores forwarded header", + remoteAddr: "198.51.100.9:5000", + forwarded: testForwardedClient, + want: "198.51.100.9", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mw := newTestMiddleware(t, &bytes.Buffer{}, "10.0.0.0/8") + + var got string + + handler := mw.ClientIP()(http.HandlerFunc( + func(_ http.ResponseWriter, r *http.Request) { + got = clientip.FromContext(r.Context()) + })) + + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/", nil) + req.RemoteAddr = tt.remoteAddr + req.Header.Set("X-Forwarded-For", tt.forwarded) + + handler.ServeHTTP(httptest.NewRecorder(), req) + + if got != tt.want { + t.Errorf("client IP in context = %q, want %q", got, tt.want) + } + }) + } +} + +// TestLoggingUsesResolvedClientIP verifies the logging middleware records +// the resolved forwarded client IP rather than the proxy peer address. +func TestLoggingUsesResolvedClientIP(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + + mw := newTestMiddleware(t, &buf, "10.0.0.0/8") + + handler := mw.ClientIP()(mw.Logging()(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }))) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:5000" + req.Header.Set("X-Forwarded-For", testForwardedClient) + + handler.ServeHTTP(httptest.NewRecorder(), req) + + if !strings.Contains(buf.String(), `"remoteIP":"`+testForwardedClient+`"`) { + t.Errorf("log output missing resolved client IP; got %q", buf.String()) + } +}