From 15479338a48133d3243ed91fdd759411d5396c10 Mon Sep 17 00:00:00 2001 From: Gutts-n <57202549+Gutts-n@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:41:23 -0300 Subject: [PATCH] Add source-IP fallback to analytics ignore check --- .env.example | 5 +++ datastore/analytics.py | 33 ++++++++++++----- datastore/core/config.py | 16 +++++++++ datastore/main.py | 1 + tests/test_analytics.py | 77 +++++++++++++++++++++++++++++++++++++--- 5 files changed, 119 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 81a76b1..ba56d53 100644 --- a/.env.example +++ b/.env.example @@ -12,6 +12,11 @@ ANALYTICS_ENABLED=false # logged, just never recorded. Both empty disables the check. ANALYTICS_IGNORE_HEADER= ANALYTICS_IGNORE_VALUES= +# Fallback for callers that cannot yet set the header above: a request from +# one of these source IPs (comma-separated) skips analytics the same way. +# Weaker signal - an IP can change without this list being updated. Empty +# disables the check. +ANALYTICS_IGNORE_IPS= # Cross-origin requests: `*` allows every origin, a comma-separated list # allows only those domains (e.g. https://data.example.org,https://app.example.org), # empty disables CORS entirely. diff --git a/datastore/analytics.py b/datastore/analytics.py index d579bff..ca122e9 100644 --- a/datastore/analytics.py +++ b/datastore/analytics.py @@ -16,8 +16,10 @@ the versioned action namespace and ``/dump/*``; probes, docs and the welcome page are excluded by definition. A request carrying the configured ignore header/value (``ANALYTICS_IGNORE_HEADER`` / - ``ANALYTICS_IGNORE_VALUES``) is skipped entirely - not logged with a - distinguishing field, just never recorded. + ``ANALYTICS_IGNORE_VALUES``), or arriving from a configured ignore IP + (``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. ``authorization_dict`` Called by ``RequestContext.authorize`` with the authorized data_dict. @@ -108,6 +110,7 @@ def __init__( service: str = "datastore-api", ignore_header: str = "", ignore_values: frozenset[str] = frozenset(), + ignore_ips: frozenset[str] = frozenset(), ) -> None: self.app = app self.service = service @@ -119,12 +122,24 @@ def __init__( #: ``ANALYTICS_IGNORE_VALUES``; either empty disables the check. self.ignore_header = ignore_header.lower() self.ignore_values = ignore_values - - def _is_ignored(self, scope: Scope) -> bool: - if not self.ignore_header or not self.ignore_values: - return False - value = Headers(scope=scope).get(self.ignore_header) - return value is not None and value.strip().lower() in self.ignore_values + #: A request whose resolved ``request_ip`` is in ``ignore_ips`` skips + #: analytics the same way - a fallback for callers that cannot yet + #: set the ignore header (e.g. the DXP frontend's static egress IPs). + #: Weaker than the header check: an IP can change on redeploy or + #: scaling without anyone updating this list, so it is a secondary + #: signal, not the primary one. Configured via + #: ``ANALYTICS_IGNORE_IPS``; empty disables the check. + self.ignore_ips = ignore_ips + + def _is_ignored(self, scope: Scope, headers: Headers) -> bool: + if self.ignore_header and self.ignore_values: + value = headers.get(self.ignore_header) + if value is not None and value.strip().lower() in self.ignore_values: + return True + if self.ignore_ips: + if self._request_ip(scope, headers) in self.ignore_ips: + return True + return False async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if scope["type"] != "http" or scope["method"] not in self.METHODS: @@ -132,7 +147,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: action = action_name(scope["path"]) if action is None: return await self.app(scope, receive, send) - if self._is_ignored(scope): + if self._is_ignored(scope, Headers(scope=scope)): return await self.app(scope, receive, send) # Created here if authorize has not run yet, so both sides mutate the diff --git a/datastore/core/config.py b/datastore/core/config.py index 827edaa..ac58287 100644 --- a/datastore/core/config.py +++ b/datastore/core/config.py @@ -92,6 +92,22 @@ def analytics_ignore_values_set(self) -> frozenset[str]: v.strip().lower() for v in self.ANALYTICS_IGNORE_VALUES.split(",") if v.strip() ) + ANALYTICS_IGNORE_IPS: str = Field( + default="", + description=( + "Comma-separated source IPs that skip analytics logging - a " + "fallback for callers that cannot yet set " + "ANALYTICS_IGNORE_HEADER (e.g. a frontend's static egress IPs). " + "Weaker than the header check: an IP can change on redeploy or " + "scaling without this list being updated. Empty disables the check." + ), + ) + + @property + def analytics_ignore_ips_set(self) -> frozenset[str]: + """`ANALYTICS_IGNORE_IPS` split on commas, blanks dropped.""" + return frozenset(v.strip() for v in self.ANALYTICS_IGNORE_IPS.split(",") if v.strip()) + # CORS # Public base URL of this service. Used only to render absolute URLs in # the OpenAPI examples — live responses derive their URLs from the diff --git a/datastore/main.py b/datastore/main.py index 41449ce..f89e401 100644 --- a/datastore/main.py +++ b/datastore/main.py @@ -111,6 +111,7 @@ def create_app() -> FastAPI: service="Datastore", ignore_header=config.ANALYTICS_IGNORE_HEADER, ignore_values=config.analytics_ignore_values_set, + ignore_ips=config.analytics_ignore_ips_set, ) # Added last = outermost, so 4xx/5xx envelopes carry CORS headers too. # `CORS_ORIGINS=*` allows every origin, a comma-separated list allows diff --git a/tests/test_analytics.py b/tests/test_analytics.py index b86809d..ce1b2a1 100644 --- a/tests/test_analytics.py +++ b/tests/test_analytics.py @@ -225,11 +225,13 @@ def _client_with_ignore_config( fake_ckan: FakeCKAN, cache: InMemoryCache, *, - header: str, - values: str, + header: str = "", + values: str = "", + ips: str = "", ) -> TestClient: monkeypatch.setenv("ANALYTICS_IGNORE_HEADER", header) monkeypatch.setenv("ANALYTICS_IGNORE_VALUES", values) + monkeypatch.setenv("ANALYTICS_IGNORE_IPS", ips) get_config.cache_clear() app = create_app() @@ -315,11 +317,78 @@ def test_a_request_with_a_different_header_value_is_still_recorded( assert len(recorded) == 1 +def test_a_request_from_an_ignored_ip_is_not_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, ips="54.247.74.82,63.32.18.228" + ) + with c: + response = c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"X-Real-IP": "54.247.74.82"}, + ) + + assert response.status_code == 200 + assert recorded == [] + + +def test_a_request_from_a_different_ip_is_still_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, ips="54.247.74.82,63.32.18.228" + ) + with c: + c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"X-Real-IP": "203.0.113.7"}, + ) + + assert len(recorded) == 1 + + +def test_the_header_and_ip_checks_are_independent( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Either check alone is enough to skip - a request doesn't need to + match both.""" + c = _client_with_ignore_config( + monkeypatch, + fake_ckan, + cache, + header="Request-Source", + values="data-explorer", + ips="54.247.74.82", + ) + with c: + response = c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"X-Real-IP": "54.247.74.82"}, + ) + + assert response.status_code == 200 + assert recorded == [] + + def test_the_ignore_check_is_disabled_when_unconfigured( client: TestClient, recorded: list[dict] ) -> None: - """Default env (empty header/values, set by conftest indirectly through - Config defaults) never skips - the shared `client` fixture proves it.""" + """Default env (empty header/values/ips, set by conftest indirectly + through Config defaults) never skips - the shared `client` fixture + proves it.""" client.get( SEARCH_URL, params={"resource_id": RESOURCE},