diff --git a/backend/internal/auth/audit.go b/backend/internal/auth/audit.go index 277a78ad..22777117 100644 --- a/backend/internal/auth/audit.go +++ b/backend/internal/auth/audit.go @@ -92,10 +92,10 @@ func (s *AuditService) LogEvent(ctx context.Context, userID *uuid.UUID, tenantID // userID/tenantID are passed explicitly because most auth events (login, refresh, // OAuth/SAML callbacks) fire BEFORE any auth middleware populates the context. func (s *AuditService) LogFiber(c *fiber.Ctx, userID, tenantID *uuid.UUID, action AuditAction, success bool, failureReason *string) error { + // c.IP() only, never the raw X-Forwarded-For (#877). Fiber resolves the + // forwarded address only when the peer is a trusted proxy (F-04); reading the + // header directly let any client write the IP of its choice into the trail. ip := c.IP() - if xff := c.Get("X-Forwarded-For"); xff != "" { - ip = xff - } userAgent := c.Get("User-Agent") var deviceFP *string diff --git a/backend/internal/auth/audit_ip_test.go b/backend/internal/auth/audit_ip_test.go new file mode 100644 index 00000000..307cb1ec --- /dev/null +++ b/backend/internal/auth/audit_ip_test.go @@ -0,0 +1,74 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package auth + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + + "github.com/opendefender/openrisk/internal/domain" +) + +type recordingAuditRepo struct{ logs []*domain.AuthAuditLog } + +func (r *recordingAuditRepo) Create(_ context.Context, l *domain.AuthAuditLog) error { + r.logs = append(r.logs, l) + return nil +} +func (r *recordingAuditRepo) GetByUser(context.Context, uuid.UUID, int, int) ([]*domain.AuthAuditLog, error) { + return nil, nil +} +func (r *recordingAuditRepo) GetByTenant(context.Context, uuid.UUID, int, int) ([]*domain.AuthAuditLog, error) { + return nil, nil +} + +// auditedIP runs one request through a Fiber app configured as main.go does it +// (F-04) and returns the IP the audit row recorded. +func auditedIP(t *testing.T, trusted []string, forwarded string) string { + t.Helper() + repo := &recordingAuditRepo{} + svc := NewAuditService(repo) + app := fiber.New(fiber.Config{ + EnableTrustedProxyCheck: true, + TrustedProxies: trusted, + ProxyHeader: fiber.HeaderXForwardedFor, + }) + app.Post("/login", func(c *fiber.Ctx) error { + return svc.LogFiber(c, nil, nil, AuditActionLogin, false, nil) + }) + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.Header.Set(fiber.HeaderXForwardedFor, forwarded) + if _, err := app.Test(req); err != nil { + t.Fatal(err) + } + if len(repo.logs) != 1 { + t.Fatalf("want one audit row, got %d", len(repo.logs)) + } + return repo.logs[0].IP +} + +// #877: an untrusted client used to write any address it liked into the audit +// trail through X-Forwarded-For. +func TestLogFiber_UntrustedPeerCannotChooseTheAuditedIP(t *testing.T) { + got := auditedIP(t, []string{"10.9.9.9"}, "203.0.113.9") + if got == "203.0.113.9" { + t.Fatal("the audit row recorded the client-supplied X-Forwarded-For") + } + if got != "0.0.0.0" { // app.Test's peer address + t.Fatalf("want the peer address, got %q", got) + } +} + +func TestLogFiber_TrustedProxyForwardedAddressIsKept(t *testing.T) { + if got := auditedIP(t, []string{"0.0.0.0"}, "203.0.113.9"); got != "203.0.113.9" { + t.Fatalf("behind a trusted proxy, want the forwarded client, got %q", got) + } +} diff --git a/backend/internal/middleware/auth.go b/backend/internal/middleware/auth.go index 47140270..f0e640f9 100644 --- a/backend/internal/middleware/auth.go +++ b/backend/internal/middleware/auth.go @@ -390,10 +390,8 @@ func MFARateLimit(store *RateLimitStore) fiber.Handler { var key string if !ok || userID == uuid.Nil { // Fallback to IP if no user ID (for MFA challenge endpoint) + // c.IP() only: it honours TrustedProxies (F-04, #877). key = c.IP() - if forwarded := c.Get("X-Forwarded-For"); forwarded != "" { - key = forwarded - } } else { key = fmt.Sprintf("user:%s", userID.String()) } @@ -419,10 +417,8 @@ func OAuthRateLimit(store *RateLimitStore) fiber.Handler { } // Use IP address for rate limiting + // c.IP() only: it honours TrustedProxies (F-04, #877). key := c.IP() - if forwarded := c.Get("X-Forwarded-For"); forwarded != "" { - key = forwarded - } // Check rate limit: 10 requests per minute per IP if !store.IsAllowed(key, 10, 1*time.Minute) {