From ced356882c8be9b8590a3bc9563d2f0d95896a28 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Wed, 29 Jul 2026 10:06:18 -0400 Subject: [PATCH] net: strip the ephemeral source port from the forwarded client IP IIS ARR sets X-Forwarded-For to clientip:port, and the port changes every connection. Left in, the audit log showed IP:PORT, the dashboard IP fallback never matched a stored (portless) DashboardDefault.ipaddress, and login rate limiting keyed per-connection instead of per-host. Add an IPv6-safe clientip.client_ip / strip_port helper and use it in the audit log, the dashboard resolver, and the login rate-limit key. --- shopdb/core/api/auth.py | 9 +++--- shopdb/core/api/dashboarddefaults.py | 9 +++--- shopdb/core/models/auditlog.py | 7 ++--- shopdb/utils/clientip.py | 39 ++++++++++++++++++++++++ tests/test_core/test_clientip.py | 45 ++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 shopdb/utils/clientip.py create mode 100644 tests/test_core/test_clientip.py diff --git a/shopdb/core/api/auth.py b/shopdb/core/api/auth.py index 977e26e..b785166 100644 --- a/shopdb/core/api/auth.py +++ b/shopdb/core/api/auth.py @@ -26,11 +26,10 @@ LOCKOUT_MINUTES = 15 def _login_ip(): - """Caller IP for rate limiting, honoring the first X-Forwarded-For hop.""" - forwarded = request.headers.get('X-Forwarded-For') - if forwarded: - return forwarded.split(',')[0].strip() - return request.remote_addr or 'unknown' + """Caller IP for rate limiting, port stripped so the key is per-host, not + per-connection (ARR forwards clientip:port with an ephemeral port).""" + from shopdb.utils.clientip import client_ip + return client_ip(request) or 'unknown' def _login_ratelimited(): diff --git a/shopdb/core/api/dashboarddefaults.py b/shopdb/core/api/dashboarddefaults.py index 7e52fea..740f185 100644 --- a/shopdb/core/api/dashboarddefaults.py +++ b/shopdb/core/api/dashboarddefaults.py @@ -17,11 +17,10 @@ dashboarddefaults_bp = Blueprint('dashboarddefaults', __name__) def _request_ip(): - """Caller IP, honoring a single proxy hop via X-Forwarded-For.""" - forwarded = request.headers.get('X-Forwarded-For') - if forwarded: - return forwarded.split(',')[0].strip() - return request.remote_addr + """Caller IP for the IP fallback, port stripped so it matches a stored + (portless) DashboardDefault.ipaddress. ARR forwards clientip:port.""" + from shopdb.utils.clientip import client_ip + return client_ip(request) def _serialize(default): diff --git a/shopdb/core/models/auditlog.py b/shopdb/core/models/auditlog.py index 7e7896c..b397c85 100644 --- a/shopdb/core/models/auditlog.py +++ b/shopdb/core/models/auditlog.py @@ -117,10 +117,9 @@ class AuditLog(db.Model): useragent = None if request_obj: - # Handle proxy forwarding - ipaddress = request_obj.headers.get('X-Forwarded-For', request_obj.remote_addr) - if ipaddress and ',' in ipaddress: - ipaddress = ipaddress.split(',')[0].strip() + # First-hop client IP, port stripped (ARR forwards clientip:port). + from shopdb.utils.clientip import client_ip + ipaddress = client_ip(request_obj) useragent = request_obj.headers.get('User-Agent', '')[:255] entry = cls( diff --git a/shopdb/utils/clientip.py b/shopdb/utils/clientip.py new file mode 100644 index 0000000..4b22e82 --- /dev/null +++ b/shopdb/utils/clientip.py @@ -0,0 +1,39 @@ +"""Client IP extraction behind a reverse proxy. + +Prod runs behind IIS ARR, which sets X-Forwarded-For to "clientip:port" (and +appends a hop per proxy). The trailing source port is ephemeral - it changes +every connection - so it must be stripped before the value is stored (audit), +matched (dashboard IP fallback), or used as a rate-limit key (login). Left in, +it makes every request look like a different client. + +Port stripping is IPv6-aware: a bracketed [2001:db8::1]:443 loses its port; a +bare 2001:db8::1 (many colons, no brackets) is returned untouched. +""" + + +def strip_port(ipaddress): + """Return ipaddress without a trailing :port. IPv6-safe. None/empty-safe.""" + if not ipaddress: + return ipaddress + ipaddress = ipaddress.strip() + if ipaddress.startswith('['): + # [IPv6] or [IPv6]:port -> the address inside the brackets + return ipaddress[1:].split(']', 1)[0] + if ipaddress.count(':') == 1: + # exactly one colon = IPv4:port (a bare IPv6 has several colons) + return ipaddress.split(':', 1)[0] + return ipaddress + + +def client_ip(request_obj): + """First-hop client IP behind one proxy, port stripped. + + Honors the first X-Forwarded-For hop, else remote_addr. Returns None only + when neither is present (callers that need a non-null key coerce it). + """ + forwarded = request_obj.headers.get('X-Forwarded-For') + if forwarded: + raw = forwarded.split(',')[0].strip() + else: + raw = request_obj.remote_addr + return strip_port(raw) diff --git a/tests/test_core/test_clientip.py b/tests/test_core/test_clientip.py new file mode 100644 index 0000000..b3bd53c --- /dev/null +++ b/tests/test_core/test_clientip.py @@ -0,0 +1,45 @@ +"""strip_port / client_ip normalization (ARR forwards clientip:port).""" + +from shopdb.utils.clientip import strip_port, client_ip + + +def test_strip_ipv4_port(): + assert strip_port('10.134.48.20:52344') == '10.134.48.20' + + +def test_ipv4_without_port_untouched(): + assert strip_port('10.134.48.20') == '10.134.48.20' + + +def test_bare_ipv6_untouched(): + assert strip_port('2001:db8::1') == '2001:db8::1' + + +def test_bracketed_ipv6_with_port(): + assert strip_port('[2001:db8::1]:443') == '2001:db8::1' + + +def test_bracketed_ipv6_without_port(): + assert strip_port('[2001:db8::1]') == '2001:db8::1' + + +def test_none_and_empty_safe(): + assert strip_port(None) is None + assert strip_port('') == '' + + +class _FakeRequest: + def __init__(self, headers=None, remote_addr=None): + self.headers = headers or {} + self.remote_addr = remote_addr + + +def test_client_ip_prefers_forwarded_first_hop_stripped(): + request_obj = _FakeRequest(headers={'X-Forwarded-For': '10.0.0.5:51000, 10.0.0.1'}, + remote_addr='127.0.0.1') + assert client_ip(request_obj) == '10.0.0.5' + + +def test_client_ip_falls_back_to_remote_addr_stripped(): + request_obj = _FakeRequest(headers={}, remote_addr='192.168.1.9:60000') + assert client_ip(request_obj) == '192.168.1.9'