From a8af4ff6553292a85b47f77f64323317d3ccc661 Mon Sep 17 00:00:00 2001 From: Gutts-n <57202549+Gutts-n@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:46:54 -0300 Subject: [PATCH] Add source-IP fallback to analytics ignore check --- ckanext/analytics/event.py | 58 +++++++++++++++++++-------- ckanext/analytics/tests/test_event.py | 31 +++++++++++++- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/ckanext/analytics/event.py b/ckanext/analytics/event.py index e7ac953..0361879 100644 --- a/ckanext/analytics/event.py +++ b/ckanext/analytics/event.py @@ -11,8 +11,10 @@ latter fires only on success, so every 403, 409 and 500 would be missing. A request carrying the configured ignore header/value (``CKANEXT_ANALYTICS_IGNORE_HEADER`` -/ ``CKANEXT_ANALYTICS_IGNORE_VALUES``) is skipped entirely - not logged with a -distinguishing field, just never recorded. +/ ``CKANEXT_ANALYTICS_IGNORE_VALUES``), or arriving from a configured ignore IP +(``CKANEXT_ANALYTICS_IGNORE_IPS``, a weaker fallback for callers that cannot yet +set the header), is skipped entirely - not logged with a distinguishing field, +just never recorded. ``Attribution`` holds the rules, ``RequestEvent`` turns a request into a dict, and ``record_request`` is the listener CKAN calls. Flask only, no CKAN import, so the @@ -52,6 +54,17 @@ if v.strip() ) +#: A request from one of these source IPs skips analytics the same way - +#: a fallback for callers that cannot yet set the header above (e.g. a +#: frontend's static egress IPs). Weaker than the header check: an IP can +#: change on redeploy or scaling without anyone updating this list. Empty +#: disables the check. +IGNORE_IPS = frozenset( + v.strip() + for v in os.environ.get("CKANEXT_ANALYTICS_IGNORE_IPS", "").split(",") + if v.strip() +) + class Attribution: """Which entity an action refers to, and in which parameter. @@ -172,13 +185,32 @@ def from_request(cls, request: Any, response: Any) -> RequestEvent | None: return None return cls(request, response) + @classmethod + def is_ignored(cls, request: Any) -> bool: + """Whether the ignore header/value or the ignore IP say to skip this + request. Either check alone is enough.""" + if IGNORE_HEADER and IGNORE_VALUES: + value = request.headers.get(IGNORE_HEADER) + if value is not None and value.strip().lower() in IGNORE_VALUES: + return True + if IGNORE_IPS: + if cls._request_ip(request) in IGNORE_IPS: + return True + return False + @staticmethod - def is_ignored(request: Any) -> bool: - """Whether ``IGNORE_HEADER`` / ``IGNORE_VALUES`` say to skip this request.""" - if not IGNORE_HEADER or not IGNORE_VALUES: - return False - value = request.headers.get(IGNORE_HEADER) - return value is not None and value.strip().lower() in IGNORE_VALUES + def _request_ip(request: Any) -> str | None: + """Same resolution rules as the ``request_ip`` property, but callable + before an instance exists (``is_ignored`` runs before construction).""" + real_ip = request.headers.get(RequestEvent.REAL_IP_HEADER) + if real_ip: + return real_ip.strip() or None + + forwarded = request.headers.get(RequestEvent.FORWARDED_FOR_HEADER) + if forwarded: + return forwarded.rsplit(",", 1)[-1].strip() or None + + return request.remote_addr @staticmethod def is_download_endpoint(endpoint: str | None) -> bool: @@ -261,15 +293,7 @@ def request_ip(self) -> str | None: ``X-Forwarded-For`` and be believed. Fine for analytics, not for access control. """ - real_ip = self.request.headers.get(self.REAL_IP_HEADER) - if real_ip: - return real_ip.strip() or None - - forwarded = self.request.headers.get(self.FORWARDED_FOR_HEADER) - if forwarded: - return forwarded.rsplit(",", 1)[-1].strip() or None - - return self.request.remote_addr + return self._request_ip(self.request) def params(self) -> dict[str, Any]: """Entity references the caller sent, wherever they put them. diff --git a/ckanext/analytics/tests/test_event.py b/ckanext/analytics/tests/test_event.py index dec0fb7..5e0da6d 100644 --- a/ckanext/analytics/tests/test_event.py +++ b/ckanext/analytics/tests/test_event.py @@ -251,8 +251,37 @@ def test_a_request_with_a_different_header_value_is_still_recorded(client, recor assert len(recorded) == 1 +def test_a_request_from_an_ignored_ip_is_not_recorded(client, recorded, monkeypatch): + monkeypatch.setattr(event, "IGNORE_IPS", frozenset({"54.247.74.82", "63.32.18.228"})) + + client.get("/api/3/action/package_show", headers={"X-Real-IP": "54.247.74.82"}) + + assert recorded == [] + + +def test_a_request_from_a_different_ip_is_still_recorded(client, recorded, monkeypatch): + monkeypatch.setattr(event, "IGNORE_IPS", frozenset({"54.247.74.82", "63.32.18.228"})) + + client.get("/api/3/action/package_show", headers={"X-Real-IP": "203.0.113.7"}) + + assert len(recorded) == 1 + + +def test_the_header_and_ip_checks_are_independent(client, recorded, monkeypatch): + """Either check alone is enough to skip - a request doesn't need to + match both.""" + monkeypatch.setattr(event, "IGNORE_HEADER", "Request-Source") + monkeypatch.setattr(event, "IGNORE_VALUES", frozenset({"data-explorer"})) + monkeypatch.setattr(event, "IGNORE_IPS", frozenset({"54.247.74.82"})) + + client.get("/api/3/action/package_show", headers={"X-Real-IP": "54.247.74.82"}) + + assert recorded == [] + + def test_the_ignore_check_is_disabled_when_unconfigured(client, recorded): - """Default (module-level IGNORE_HEADER/IGNORE_VALUES empty) never skips.""" + """Default (module-level IGNORE_HEADER/IGNORE_VALUES/IGNORE_IPS empty) + never skips.""" client.get("/api/3/action/package_show", headers={"Request-Source": "data-explorer"}) assert len(recorded) == 1