diff --git a/.env.example b/.env.example index ba56d53..0e0a758 100644 --- a/.env.example +++ b/.env.example @@ -12,6 +12,10 @@ ANALYTICS_ENABLED=false # logged, just never recorded. Both empty disables the check. ANALYTICS_IGNORE_HEADER= ANALYTICS_IGNORE_VALUES= +# Same check on a query string param instead of a header, for plain +# download links that cannot set a header at all. Matched against +# ANALYTICS_IGNORE_VALUES above. Empty disables this check. +ANALYTICS_IGNORE_QUERY_PARAM= # 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 diff --git a/datastore/analytics.py b/datastore/analytics.py index ca122e9..33f66c0 100644 --- a/datastore/analytics.py +++ b/datastore/analytics.py @@ -16,7 +16,9 @@ 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``), or arriving from a configured ignore IP + ``ANALYTICS_IGNORE_VALUES``), the same value on a configured query + param (``ANALYTICS_IGNORE_QUERY_PARAM`` - for plain ```` links + that cannot set a header), 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. @@ -110,6 +112,7 @@ def __init__( service: str = "datastore-api", ignore_header: str = "", ignore_values: frozenset[str] = frozenset(), + ignore_query_param: str = "", ignore_ips: frozenset[str] = frozenset(), ) -> None: self.app = app @@ -122,6 +125,11 @@ def __init__( #: ``ANALYTICS_IGNORE_VALUES``; either empty disables the check. self.ignore_header = ignore_header.lower() self.ignore_values = ignore_values + #: Same check, but on a query string param instead of a header - for + #: plain `` download links, which cannot set a header at + #: all. Matched against the same ``ignore_values``. Configured via + #: ``ANALYTICS_IGNORE_QUERY_PARAM``; empty disables the check. + self.ignore_query_param = ignore_query_param #: 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). @@ -136,6 +144,12 @@ def _is_ignored(self, scope: Scope, headers: Headers) -> bool: value = headers.get(self.ignore_header) if value is not None and value.strip().lower() in self.ignore_values: return True + if self.ignore_query_param and self.ignore_values: + query: bytes = scope.get("query_string", b"") + params = parse_qs(query.decode("latin-1")) + values = params.get(self.ignore_query_param) or [] + if any(v.strip().lower() in self.ignore_values for v in values): + return True if self.ignore_ips: if self._request_ip(scope, headers) in self.ignore_ips: return True diff --git a/datastore/core/config.py b/datastore/core/config.py index ac58287..19851ff 100644 --- a/datastore/core/config.py +++ b/datastore/core/config.py @@ -79,9 +79,10 @@ class Config(BaseSettings): ANALYTICS_IGNORE_VALUES: str = Field( default="", description=( - "Comma-separated header values that skip analytics logging " - "when ANALYTICS_IGNORE_HEADER matches (e.g. `data-explorer`). " - "Matched case-insensitively. Empty disables the check." + "Comma-separated values that skip analytics logging when " + "ANALYTICS_IGNORE_HEADER or ANALYTICS_IGNORE_QUERY_PARAM " + "matches (e.g. `data-explorer`). Matched case-insensitively. " + "Empty disables both checks." ), ) @@ -92,6 +93,16 @@ def analytics_ignore_values_set(self) -> frozenset[str]: v.strip().lower() for v in self.ANALYTICS_IGNORE_VALUES.split(",") if v.strip() ) + ANALYTICS_IGNORE_QUERY_PARAM: str = Field( + default="", + description=( + "Query string param name checked the same way as " + "ANALYTICS_IGNORE_HEADER, for plain `` download links " + "that cannot set a header (e.g. `request_source`). Matched " + "against ANALYTICS_IGNORE_VALUES. Empty disables the check." + ), + ) + ANALYTICS_IGNORE_IPS: str = Field( default="", description=( diff --git a/datastore/main.py b/datastore/main.py index f89e401..9583ff0 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_query_param=config.ANALYTICS_IGNORE_QUERY_PARAM, ignore_ips=config.analytics_ignore_ips_set, ) # Added last = outermost, so 4xx/5xx envelopes carry CORS headers too. diff --git a/tests/test_analytics.py b/tests/test_analytics.py index ce1b2a1..c79c4d9 100644 --- a/tests/test_analytics.py +++ b/tests/test_analytics.py @@ -227,10 +227,12 @@ def _client_with_ignore_config( *, header: str = "", values: str = "", + query_param: str = "", ips: str = "", ) -> TestClient: monkeypatch.setenv("ANALYTICS_IGNORE_HEADER", header) monkeypatch.setenv("ANALYTICS_IGNORE_VALUES", values) + monkeypatch.setenv("ANALYTICS_IGNORE_QUERY_PARAM", query_param) monkeypatch.setenv("ANALYTICS_IGNORE_IPS", ips) get_config.cache_clear() @@ -383,6 +385,60 @@ def test_the_header_and_ip_checks_are_independent( assert recorded == [] +def test_a_dump_link_with_the_ignore_query_param_is_not_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real use case: a plain download link can't set a + header, so the same value rides in the query string instead.""" + c = _client_with_ignore_config( + monkeypatch, + fake_ckan, + cache, + query_param="request_source", + values="admin-portal", + ) + url = "https://storage.googleapis.com/bucket/dumps/x/abc.csv?Sig=abc" + + async def fake_dump(self: BigQueryBackend, resource_id: str, fmt: str) -> list[str]: + return [url] + + with c, patch.object(BigQueryBackend, "dump", fake_dump): + response = c.get( + f"{DUMP_PREFIX}/{RESOURCE}?request_source=admin-portal", + follow_redirects=False, + ) + + assert response.status_code == 302 + assert recorded == [] + + +def test_a_dump_link_without_the_ignore_query_param_is_still_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, + fake_ckan, + cache, + query_param="request_source", + values="admin-portal", + ) + url = "https://storage.googleapis.com/bucket/dumps/x/abc.csv?Sig=abc" + + async def fake_dump(self: BigQueryBackend, resource_id: str, fmt: str) -> list[str]: + return [url] + + with c, patch.object(BigQueryBackend, "dump", fake_dump): + c.get(f"{DUMP_PREFIX}/{RESOURCE}", follow_redirects=False) + + assert len(recorded) == 1 + + def test_the_ignore_check_is_disabled_when_unconfigured( client: TestClient, recorded: list[dict] ) -> None: