fix: rate limiting X-Real-IP from the real peer

features/trusted_proxy_client_ip_for_rate_limiting.feature

@@ -0,0 +1,98 @@
+Feature: The rate limiter keys on the real connection address, trusting a forwarded client IP only from a configured proxy
+
+  As the maintainer of RenewCMS
+  I want the login/password-reset rate limiter to key on the raw TCP peer address by default, and
+    to honor a proxy-supplied client IP only when that peer address is a configured trusted proxy
+  So that an untrusted client can never choose which rate-limit counter it hits by sending its own
+    X-Real-IP or X-Forwarded-For header — which is possible today, because chimiddleware.RealIP
+    rewrites r.RemoteAddr from either header unconditionally, for every request
+
+  Background:
+    Given POST /v1/auth/login and POST /v1/auth/password-reset are each wrapped, inside
+      NewAuthRouter, by a middleware that runs before RateLimiter (api/middleware/rateLimit.go),
+      which keys its counters on r.RemoteAddr
+    And this slice replaces chimiddleware.RealIP on those two route groups with
+      TrustedProxyRealIP(trustedProxies []string) func(http.Handler) http.Handler
+      (api/middleware/trustedProxyRealIP.go)
+    And TrustedProxyRealIP reads the host portion of the request's current r.RemoteAddr — the raw
+      TCP connection's peer address, as set by net/http before any middleware runs — and compares
+      it against the configured trustedProxies list
+    And when that connection address is in trustedProxies and the request carries an X-Real-IP
+      header, TrustedProxyRealIP rewrites r.RemoteAddr's host to the header's value before calling
+      the next handler, so RateLimiter downstream keys its counter on the forwarded address instead
+    And when that connection address is not in trustedProxies, r.RemoteAddr is left untouched and
+      any X-Real-IP or X-Forwarded-For header present on the request is ignored entirely — it has
+      no effect on which counter the request is recorded against
+    And TrustedProxyRealIP never reads X-Forwarded-For, from a trusted proxy or otherwise; only
+      X-Real-IP is consulted
+    And when the connection address is a trusted proxy but the request has no X-Real-IP header,
+      r.RemoteAddr is left untouched, so the request is keyed on the proxy's own address
+    And a new environment variable, TRUSTED_PROXIES, is a semicolon-separated list of IP addresses
+      (matching the CORS_ALLOWED_ORIGINS parsing convention), given a default of "" in
+      cmd/server/main.go's envDefaults (not requiredEnvVars, so existing deployments keep working
+      unchanged, always keying on the raw connection address), read by config.NewSettingsFromEnv
+      into a new config.Settings.TrustedProxies ([]string) field, documented in .env.example and
+      docs/configuration.md
+    And docs/deployment.md's nginx example sets `proxy_set_header X-Real-IP $remote_addr;` (the
+      real peer, not $proxy_add_x_forwarded_for) and its Caddy example sets the equivalent header
+      from the real peer, and both examples note that TRUSTED_PROXIES must list the proxy's own
+      address (typically 127.0.0.1 when the proxy and RenewCMS share a host) for the forwarded
+      header to be honored
+    And this is a transport-level concern with no domain or use-case involvement: neither
+      TrustedProxyRealIP nor RateLimiter imports internal/domain, internal/infrastructure/useCases,
+      persistence or mailer
+
+  Scenario: With no trusted proxies configured, a forwarded header changes nothing
+    Given TRUSTED_PROXIES is empty and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a client connecting from "203.0.113.5" sends POST /v1/auth/login with header
+      X-Real-IP: "198.51.100.1"
+    And the same client immediately sends a second POST /v1/auth/login, still with
+      X-Real-IP: "198.51.100.1"
+    Then the first response is not rejected by rate limiting and the second is 429 — the counter is
+      keyed on the connection address "203.0.113.5", not the header value
+
+  Scenario: An untrusted connection address cannot use the header to dodge its own limit
+    Given TRUSTED_PROXIES is "10.0.0.1" and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a client connecting from "203.0.113.5" (not in TRUSTED_PROXIES) sends POST /v1/auth/login
+      with header X-Real-IP: "1.1.1.1"
+    And the same client sends a second POST /v1/auth/login with a different header,
+      X-Real-IP: "2.2.2.2"
+    Then the first response is not rejected by rate limiting and the second is 429 — both requests
+      are counted against the connection address "203.0.113.5", regardless of the differing header
+      values, because "203.0.113.5" is not a trusted proxy
+
+  Scenario: A configured trusted proxy's forwarded header is honored
+    Given TRUSTED_PROXIES is "10.0.0.1" and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a request arrives with connection address "10.0.0.1" and header
+      X-Real-IP: "198.51.100.9"
+    And a second request arrives with connection address "10.0.0.1" and the same header,
+      X-Real-IP: "198.51.100.9"
+    Then the first response is not rejected by rate limiting and the second is 429 — both are
+      counted against the forwarded client "198.51.100.9", not against the proxy's own address
+
+  Scenario: Two real clients behind the same trusted proxy get independent counters
+    Given TRUSTED_PROXIES is "10.0.0.1" and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a request arrives with connection address "10.0.0.1" and header
+      X-Real-IP: "198.51.100.9", consuming that forwarded client's limit
+    And a second request arrives with connection address "10.0.0.1" and header
+      X-Real-IP: "198.51.100.10"
+    Then the second request is not rejected by rate limiting — it is tracked separately from
+      "198.51.100.9"
+
+  Scenario: A trusted proxy's own X-Forwarded-For header is never consulted
+    Given TRUSTED_PROXIES is "10.0.0.1" and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a request arrives with connection address "10.0.0.1" and header
+      X-Forwarded-For: "198.51.100.9" (no X-Real-IP header)
+    And a second such request arrives with the same connection address and the same
+      X-Forwarded-For header
+    Then the first response is not rejected by rate limiting and the second is 429 — both are
+      counted against the proxy's own connection address "10.0.0.1", because X-Forwarded-For is
+      never read
+
+  Scenario: A trusted proxy with no forwarded header falls back to its own address
+    Given TRUSTED_PROXIES is "10.0.0.1" and AUTH_RATE_LIMIT_MAX_ATTEMPTS is 1
+    When a request arrives with connection address "10.0.0.1" and no X-Real-IP header
+    And a second such request arrives, also from connection address "10.0.0.1" with no
+      X-Real-IP header
+    Then the first response is not rejected by rate limiting and the second is 429 — both are
+      counted against "10.0.0.1" itself

features/trusted_proxy_client_ip_for_rate_limiting.qa.md

@@ -0,0 +1,70 @@
+# QA procedure: trusted_proxy_client_ip_for_rate_limiting
+
+Package: `test` (black-box), extending `test/login_and_password_reset_rate_limiting_http_test.go`'s
+setup (`mockUserRepo`, `stubHasher`, `mockMailRepo`, the full `useCases` set, `auth.NewHandler(...)`).
+
+## Test infrastructure additions needed first
+
+1. Implement `api/middleware.TrustedProxyRealIP(trustedProxies []string) func(http.Handler) http.Handler`.
+2. In tests, build a `chi.NewRouter()` mirroring `NewAuthRouter`'s `/login` and `/password-reset`
+   groups: `middleware.TrustedProxyRealIP(trustedProxies)` then
+   `middleware.NewRateLimiter(maxAttempts, window)` then the handler. `chimiddleware.RealIP` is not
+   part of this router.
+3. Simulate a request's connection address by setting `req.RemoteAddr = "<ip>:<port>"` (host:port,
+   matching real request shape) before calling the router. Simulate a proxy-forwarded header with
+   `req.Header.Set("X-Real-IP", "<ip>")` or `req.Header.Set("X-Forwarded-For", "<ip>")` as needed.
+
+## Setup shared by every case
+
+4. Call `validator.InitValidator()` once.
+5. Build `mockUserRepo`, `mockMailRepo`, `stubHasher`, and the full `useCases` set, then
+   `auth.NewHandler(...)`.
+6. Use `AUTH_RATE_LIMIT_MAX_ATTEMPTS=1` and a window covering the whole test
+   (`window=time.Hour`) throughout, so a second request from the same effective key is always the
+   one that trips 429 and no scenario needs to wait out a window.
+7. Seed `mockUserRepo` with user "alice", a correct password hash, so `POST /v1/auth/login` can
+   return 200 where a scenario needs a non-429, non-403 baseline; a wrong-password body works
+   equally well to prove "not rejected by rate limiting" since only the status code being not 429
+   matters for these cases (assert `!= 429` in addition to whatever status login returns for the
+   given credentials).
+
+## HTTP-level checks
+
+8. `trustedProxies = nil` (empty). Send `POST /v1/auth/login` with `RemoteAddr = "203.0.113.5:1"`
+   and header `X-Real-IP: 198.51.100.1`. Assert the response is not `429`. Send a second, identical
+   request (same `RemoteAddr`, same header). Assert it is `429` — proves the header had no effect;
+   both requests hit the connection-address counter.
+9. `trustedProxies = ["10.0.0.1"]`. Send `POST /v1/auth/login` with `RemoteAddr = "203.0.113.5:1"`
+   (not in the trusted list) and header `X-Real-IP: 1.1.1.1`. Assert not `429`. Send a second
+   request from the same `RemoteAddr` with header `X-Real-IP: 2.2.2.2` (a different value). Assert
+   it is `429` — proves an untrusted source can't split its own attempts across counters by varying
+   the header.
+10. `trustedProxies = ["10.0.0.1"]`. Send `POST /v1/auth/login` with `RemoteAddr = "10.0.0.1:1"` and
+    header `X-Real-IP: 198.51.100.9`. Assert not `429`. Send a second request with the same
+    `RemoteAddr` and the same header. Assert `429` — proves the trusted proxy's forwarded client is
+    honored as the counter key.
+11. `trustedProxies = ["10.0.0.1"]`. Send `POST /v1/auth/login` with `RemoteAddr = "10.0.0.1:1"` and
+    header `X-Real-IP: 198.51.100.9`, consuming that forwarded client's limit (assert not `429`).
+    Then send a second request with the same `RemoteAddr` but header `X-Real-IP: 198.51.100.10`.
+    Assert not `429` — a different forwarded client behind the same trusted proxy has its own
+    counter.
+12. `trustedProxies = ["10.0.0.1"]`. Send `POST /v1/auth/login` with `RemoteAddr = "10.0.0.1:1"` and
+    header `X-Forwarded-For: 198.51.100.9` (no `X-Real-IP`). Assert not `429`. Send a second,
+    identical request. Assert `429` — proves `X-Forwarded-For` is never read, even from a trusted
+    proxy; both requests are keyed on the proxy's own address `10.0.0.1`.
+13. `trustedProxies = ["10.0.0.1"]`. Send `POST /v1/auth/login` with `RemoteAddr = "10.0.0.1:1"` and
+    no forwarding header at all. Assert not `429`. Send a second, identical request. Assert `429` —
+    a trusted proxy with nothing to forward falls back to its own address as the key.
+
+## Manual verification (not exercised by the Go test)
+
+- Real behaviour behind the documented nginx and Caddy reverse proxies (`docs/deployment.md`) —
+  that each sets `X-Real-IP` from the actual peer address rather than a client-controlled value,
+  and that `TRUSTED_PROXIES` on the RenewCMS host is set to the proxy's real address (e.g.
+  `127.0.0.1` when co-located) — is not exercised by an in-process `httptest` router and should be
+  checked by hand against a real reverse-proxy deployment.
+- `config.NewSettingsFromEnv` parsing `TRUSTED_PROXIES` from the environment (semicolon-separated,
+  matching `CORS_ALLOWED_ORIGINS`) is a use-case-free config concern; if a pure Go test for
+  `internal/infrastructure/config` already exists for `CORS_ALLOWED_ORIGINS`
+  (`test/runtime_settings_from_env_test.go`), extend it with a parallel case rather than adding it
+  here — flagged for the coder/qa stage to place correctly.