From 31f79fd8e28e6918a3d1dc1f1a2f054c5a8f99ca Mon Sep 17 00:00:00 2001 From: BigBodyCobain Date: Fri, 22 May 2026 00:46:25 -0600 Subject: [PATCH 1/2] Fix #287: proxy-aware rate-limit key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by @tg12 in the external security/correctness audit. Before this change, backend/limiter.py was: from slowapi.util import get_remote_address limiter = Limiter(key_func=get_remote_address) get_remote_address only ever returns request.client.host — it does not look at X-Forwarded-For. Behind the bundled Next.js proxy (or any other reverse proxy), every connected operator's client.host is the frontend container's bridge IP, so @limiter.limit("120/minute") collapses into one shared bucket for everybody on the same backend. One heavy tab can starve every other operator on that node. This change swaps in shadowbroker_rate_limit_key, which: * Reads X-Forwarded-For ONLY when the immediate peer matches the SAME hostname-bound allowlist we use for Docker-bridge local-operator trust (auth._resolve_trusted_bridge_ips — fix #250). Default is `frontend,shadowbroker-frontend`, override via SHADOWBROKER_TRUSTED_FRONTEND_HOSTS. * Picks the FIRST entry in the XFF chain — that's the operator end, not the proxy end. * Falls back to request.client.host for any peer not on the allowlist. Direct hits, unrelated containers, and unknown hosts are bucketed exactly like before. * Falls back to request.client.host when the resolver itself raises (e.g. DNS down). XFF is never accepted on a peer we can't confirm is the trusted frontend — there is no way to spoof another operator's bucket from outside. No new env vars. No new operator config. Single-operator nodes are unaffected — same behaviour as before. The 120/minute and 60/minute windows on the existing endpoints are unchanged; only the KEY they bucket on changes. Tests cover: * Direct loopback → keys on peer (regression check vs. get_remote_address default). * Untrusted peer sending XFF → XFF ignored, keys on peer. * Trusted frontend peer with XFF → keys on first XFF entry. * First XFF entry picked from a multi-hop chain. * Trusted peer without XFF → falls back to peer IP. * Empty/whitespace XFF entries skipped. * Header lookup is case-insensitive. * Two operators behind same proxy → different keys (the whole point of the fix). * Spoof attempt from internet-facing untrusted IP can't steal the victim's bucket. * Resolver raising is treated as untrusted (fail-closed). * No-client request shape doesn't raise. --- backend/limiter.py | 106 ++++++++++- backend/tests/test_rate_limit_proxy_aware.py | 186 +++++++++++++++++++ 2 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_rate_limit_proxy_aware.py diff --git a/backend/limiter.py b/backend/limiter.py index 38404a8..a35e4c5 100644 --- a/backend/limiter.py +++ b/backend/limiter.py @@ -1,4 +1,108 @@ +"""Rate-limit key function for slowapi. + +Issue #287 (tg12): the previous implementation used +``slowapi.util.get_remote_address`` which only ever returns +``request.client.host``. Behind the bundled Next.js proxy (or any other +reverse proxy), every connected operator's ``client.host`` is the +frontend container's bridge IP. ``@limiter.limit("120/minute")`` then +collapses into one shared bucket for everybody on the same backend — +one heavy tab can starve every other operator on the node. + +This module replaces that key function with one that: + + * Reads ``X-Forwarded-For`` ONLY when the immediate peer is a trusted + frontend container (same allowlist used by the Docker bridge + local-operator trust path — see ``backend/auth.py`` ``#250``). + * Picks the FIRST entry in the XFF chain. That's the client end of + the proxy chain, which is the operator we want to bucket on. + * Falls back to ``request.client.host`` for any peer that isn't on + the trusted-frontend allowlist. Direct hits, unrelated containers, + and unknown hosts are bucketed exactly like before — there is no + way for an untrusted caller to spoof XFF and steal another + operator's rate-limit bucket. + +Single-operator nodes are unaffected: the frontend resolves to one IP, +that IP is on the trust list, the XFF header is read, and you get one +bucket per operator (i.e. you). +""" + +from __future__ import annotations + +from typing import Any + from slowapi import Limiter from slowapi.util import get_remote_address -limiter = Limiter(key_func=get_remote_address) + +def _client_host(request: Any) -> str: + """Return the immediate peer's IP, normalised to a lowercase string.""" + client = getattr(request, "client", None) + if client is None: + return "" + host = getattr(client, "host", "") or "" + return host.lower() + + +def _first_forwarded_for(value: str) -> str: + """Return the first non-empty entry from an ``X-Forwarded-For`` header. + + RFC 7239 / de-facto XFF format is ``client, proxy1, proxy2, …``. The + client end is what we want to bucket on. Empty parts (which appear + in some malformed headers) are skipped so we don't end up keying on + an empty string. + """ + for raw in value.split(","): + candidate = raw.strip() + if candidate: + return candidate.lower() + return "" + + +def _is_trusted_frontend_peer(host: str) -> bool: + """True iff ``host`` is one of the resolved trusted-frontend IPs. + + Imported lazily so this module stays usable in unit tests that + don't want to pull the whole auth module into scope. + """ + if not host: + return False + try: + from auth import _resolve_trusted_bridge_ips + except Exception: # pragma: no cover - defensive + return False + try: + trusted_ips = _resolve_trusted_bridge_ips() + except Exception: # pragma: no cover - defensive + return False + return host in trusted_ips + + +def shadowbroker_rate_limit_key(request: Any) -> str: + """slowapi key_func that is proxy-aware on trusted frontend peers only. + + Behaviour matrix: + + * Direct loopback / unknown peer → ``request.client.host`` + (identical to slowapi's default ``get_remote_address``). + * Peer is a trusted frontend container AND ``X-Forwarded-For`` is + present → first XFF entry (the actual operator). + * Peer is a trusted frontend container but no XFF → fall back to + ``request.client.host`` (the bridge IP). One shared bucket for + everyone in that case, same as before — but you only get there + if the trusted frontend forgot to forward XFF, which it won't. + """ + peer = _client_host(request) + if _is_trusted_frontend_peer(peer): + headers = getattr(request, "headers", None) + if headers is not None: + xff = headers.get("x-forwarded-for") or headers.get("X-Forwarded-For") + if xff: + first = _first_forwarded_for(xff) + if first: + return first + # Untrusted peer (or trusted peer without XFF): match the original + # get_remote_address behaviour byte-for-byte. + return get_remote_address(request) + + +limiter = Limiter(key_func=shadowbroker_rate_limit_key) diff --git a/backend/tests/test_rate_limit_proxy_aware.py b/backend/tests/test_rate_limit_proxy_aware.py new file mode 100644 index 0000000..a603316 --- /dev/null +++ b/backend/tests/test_rate_limit_proxy_aware.py @@ -0,0 +1,186 @@ +"""Tests for issue #287: proxy-aware slowapi key function. + +Contract: + * Untrusted peer → key is the peer IP (matches old get_remote_address). + * Trusted frontend peer with X-Forwarded-For → key is first XFF entry. + * Trusted frontend peer without X-Forwarded-For → key is the peer IP + (fail-soft: no behaviour change vs. before #287). + * XFF from an untrusted peer is IGNORED — there must be no way to + spoof another operator's bucket by sending XFF directly. + * The first XFF entry is used (not the last — that's the trusted + proxy talking to the backend, not the actual operator). +""" + +import pytest + + +class _FakeClient: + def __init__(self, host: str): + self.host = host + + +class _FakeRequest: + """Minimal slowapi-compatible request shim — has ``client`` and + ``headers`` attributes, which is all the key_func touches.""" + + def __init__(self, client_host: str, headers: dict | None = None): + self.client = _FakeClient(client_host) if client_host is not None else None + self.headers = dict(headers or {}) + # slowapi's get_remote_address also tries request.client; we + # exercise both branches via the same shim. + + +# ───────────────────────── untrusted peers ────────────────────────────── + + +class TestUntrustedPeer: + def test_direct_loopback_uses_client_host(self, monkeypatch): + """Direct hit from 127.0.0.1 — no XFF — keys on the peer IP.""" + from limiter import shadowbroker_rate_limit_key + # Make sure the trusted-frontend cache resolves to nothing relevant. + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset()) + req = _FakeRequest("127.0.0.1") + assert shadowbroker_rate_limit_key(req) == "127.0.0.1" + + def test_xff_from_untrusted_peer_is_ignored(self, monkeypatch): + """A random caller sending X-Forwarded-For must NOT steal another + operator's bucket. The XFF is dropped on the floor.""" + from limiter import shadowbroker_rate_limit_key + # Trusted set deliberately does NOT include 1.2.3.4. + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest("1.2.3.4", {"X-Forwarded-For": "9.9.9.9"}) + # Falls back to the peer IP, not 9.9.9.9. + assert shadowbroker_rate_limit_key(req) == "1.2.3.4" + + def test_unknown_host_with_xff_uses_peer_host(self, monkeypatch): + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset()) + req = _FakeRequest("10.0.0.5", {"X-Forwarded-For": "1.1.1.1"}) + assert shadowbroker_rate_limit_key(req) == "10.0.0.5" + + +# ───────────────────────── trusted frontend peers ─────────────────────── + + +class TestTrustedFrontendPeer: + def test_trusted_peer_with_xff_uses_first_xff_entry(self, monkeypatch): + """When the immediate peer is the trusted frontend container and + XFF carries the operator's chain, we key on the operator.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest("172.20.0.5", {"X-Forwarded-For": "203.0.113.7"}) + assert shadowbroker_rate_limit_key(req) == "203.0.113.7" + + def test_first_xff_entry_picked_in_chain(self, monkeypatch): + """`client, proxy1, proxy2` → we pick the client, not the proxies. + Picking the last entry would mean every operator behind the same + upstream gets bucketed together, which is the bug we're fixing.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest( + "172.20.0.5", + {"X-Forwarded-For": "203.0.113.7, 198.51.100.1, 10.0.0.1"}, + ) + assert shadowbroker_rate_limit_key(req) == "203.0.113.7" + + def test_trusted_peer_without_xff_falls_back_to_peer(self, monkeypatch): + """If the trusted frontend forgot to forward XFF (legacy clients, + broken deploys), don't crash — bucket on the bridge IP exactly + like the pre-#287 behaviour.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest("172.20.0.5", headers={}) + assert shadowbroker_rate_limit_key(req) == "172.20.0.5" + + def test_trusted_peer_with_empty_xff_falls_back(self, monkeypatch): + """``X-Forwarded-For: , ,`` → no usable entries → falls back.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest("172.20.0.5", {"X-Forwarded-For": " , , "}) + assert shadowbroker_rate_limit_key(req) == "172.20.0.5" + + def test_xff_header_case_insensitive(self, monkeypatch): + """HTTP header names are case-insensitive — slowapi normalises + but our shim doesn't, so we explicitly check both forms.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + req = _FakeRequest("172.20.0.5", {"x-forwarded-for": "203.0.113.7"}) + assert shadowbroker_rate_limit_key(req) == "203.0.113.7" + + +# ───────────────────────── isolation guarantees ───────────────────────── + + +class TestIsolation: + def test_two_operators_behind_same_proxy_get_different_keys(self, monkeypatch): + """The whole reason this fix exists — two operators behind the + SAME proxy must end up in DIFFERENT buckets.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + op_a = _FakeRequest("172.20.0.5", {"X-Forwarded-For": "10.1.1.1"}) + op_b = _FakeRequest("172.20.0.5", {"X-Forwarded-For": "10.1.1.2"}) + key_a = shadowbroker_rate_limit_key(op_a) + key_b = shadowbroker_rate_limit_key(op_b) + assert key_a != key_b + assert key_a == "10.1.1.1" + assert key_b == "10.1.1.2" + + def test_no_xff_spoof_from_outside(self, monkeypatch): + """If we ever expose the backend port directly to the internet, + an attacker MUST NOT be able to steal another operator's bucket + by sending their own XFF header.""" + from limiter import shadowbroker_rate_limit_key + # Trusted set is the frontend container IP; the attacker is on a + # different (untrusted) IP and tries to spoof a victim's IP. + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset({"172.20.0.5"})) + attacker = _FakeRequest("203.0.113.66", {"X-Forwarded-For": "10.1.1.1"}) + victim_via_proxy = _FakeRequest("172.20.0.5", {"X-Forwarded-For": "10.1.1.1"}) + assert shadowbroker_rate_limit_key(attacker) == "203.0.113.66" + assert shadowbroker_rate_limit_key(victim_via_proxy) == "10.1.1.1" + # The attacker burning their own bucket doesn't touch the victim's. + assert shadowbroker_rate_limit_key(attacker) != shadowbroker_rate_limit_key( + victim_via_proxy + ) + + def test_limiter_object_uses_proxy_aware_key(self): + """Smoke check that the module-level Limiter exports the new key + function rather than slowapi's default.""" + from limiter import limiter, shadowbroker_rate_limit_key + # slowapi stores it as ._key_func; we don't want to depend on + # that internal name, so just check the function is reachable. + assert callable(shadowbroker_rate_limit_key) + assert limiter is not None + + +# ───────────────────────── defensive corners ──────────────────────────── + + +class TestDefensive: + def test_no_client_object(self, monkeypatch): + """Some upstream middleware paths (websocket, ASGI lifespan) + produce requests with no ``client`` attribute — must not raise.""" + from limiter import shadowbroker_rate_limit_key + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", lambda: frozenset()) + + class _NoClient: + def __init__(self): + self.client = None + self.headers = {} + + # slowapi's get_remote_address returns "127.0.0.1" as a default + # in this case, so we just ensure no exception escapes. + result = shadowbroker_rate_limit_key(_NoClient()) + assert isinstance(result, str) + + def test_resolver_raises_is_treated_as_untrusted(self, monkeypatch): + """If DNS blows up inside the trusted-bridge resolver, we MUST + fall back to peer IP — never accept XFF blindly.""" + from limiter import shadowbroker_rate_limit_key + + def _explode(): + raise RuntimeError("DNS down") + + monkeypatch.setattr("auth._resolve_trusted_bridge_ips", _explode) + req = _FakeRequest("172.20.0.5", {"X-Forwarded-For": "9.9.9.9"}) + # XFF must be ignored when we can't confirm peer is trusted. + assert shadowbroker_rate_limit_key(req) == "172.20.0.5" From c54ea7fd9f6e27e586fe82b767da33bf29f65427 Mon Sep 17 00:00:00 2001 From: BigBodyCobain Date: Fri, 22 May 2026 09:58:25 -0600 Subject: [PATCH 2/2] Fix #299/#300/#301: gate Sentinel proxy routes with require_local_operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by @tg12 in three audit issues opened the same day: #299 — POST /api/sentinel/token is an unauthenticated Copernicus OAuth relay for caller-supplied client_id/secret. #300 — POST /api/sentinel/tile is an unauthenticated quota/bandwidth relay for Sentinel Hub Process API tile fetches. #301 — GET /api/sentinel2/search is an unauthenticated Planetary Computer STAC + Esri imagery search relay. All three lived in backend/routers/tools.py decorated only with @limiter.limit(...) — no Depends(require_local_operator). That made the backend a free anonymous relay for any caller's Sentinel / Planetary Computer queries, in the same shape we already closed for #240/#241 (oracle resolve) and #211/#213/#214 (thermal verify, OpenMHZ calls + audio relay). Fix: add dependencies=[Depends(require_local_operator)] to each route. Loopback / Docker-bridge / admin-key callers (the operator dashboard) are unaffected — they still resolve through the same allowlist used by every other operator-only helper in this file. Anonymous remote callers now receive 403 BEFORE any outbound HTTP call to Copernicus or Planetary Computer happens. Tests ----- test_sentinel_routes_auth_gate.py — 8 new tests: * anonymous-remote → 403 on all three routes * NO upstream HTTP call when the gate fires (asserted via MagicMock(side_effect=AssertionError) on requests.post and services.sentinel_search.search_sentinel2_scene). This is the property that makes the gate real — without it, a 403 returned after the upstream call still burns quota. * 127.0.0.1 loopback caller reaches the handler (no false-positive where the gate accidentally blocks the local operator too). * Uses raw ASGITransport(client=(peer_ip, ...)) rather than FastAPI's TestClient because TestClient reports client.host as "testclient" which is not on the loopback allowlist. test_control_surface_auth.py — extended the existing parameterised regression with the three new routes. That regression is the global "no remote control surface ships without auth" guard for the whole codebase; adding these to it means a future refactor that drops the dependency from any of them will fail CI alongside the existing ~30 gated routes. The egress-on-403 property and the parameterised regression together give two independent proofs that the gate fires before the upstream network call, even if FastAPI's internal dependant tree shape changes across versions (an earlier draft of this PR included a static walker of the route table; it was removed because behavioural evidence is strictly stronger and version-independent). --- backend/routers/tools.py | 29 ++- backend/tests/test_control_surface_auth.py | 28 +++ .../tests/test_sentinel_routes_auth_gate.py | 231 ++++++++++++++++++ 3 files changed, 285 insertions(+), 3 deletions(-) create mode 100644 backend/tests/test_sentinel_routes_auth_gate.py diff --git a/backend/routers/tools.py b/backend/routers/tools.py index 0faa392..40bbba3 100644 --- a/backend/routers/tools.py +++ b/backend/routers/tools.py @@ -85,7 +85,30 @@ async def api_geocode_reverse( return await asyncio.to_thread(reverse_geocode, lat, lng, local_only) -@router.get("/api/sentinel2/search") +# ── Sentinel proxy routes (Issue #299/#300/#301, reported by tg12) ────────── +# These three endpoints relay external Sentinel / Planetary Computer +# requests through the backend to avoid browser CORS blocks. They are +# operator-only helpers — they MUST NOT be callable by anonymous remote +# users, because: +# +# * /api/sentinel/token — caller supplies their own Sentinel client_id + +# client_secret. Without operator gating, the backend becomes a free +# anonymous OAuth-mint relay for any Copernicus account. +# * /api/sentinel/tile — same shape as the token route but for tile +# imagery. Without gating, the backend acts as an anonymous quota and +# bandwidth relay for Sentinel Hub Process API calls. +# * /api/sentinel2/search — hits the Planetary Computer STAC search API +# and falls back to Esri imagery. No caller credentials are involved, +# but the route is still an anonymous external-search relay. We gate +# it the same way for consistency with the rest of the operator-only +# helper surface. +# +# Gating is via require_local_operator (loopback / bridge / admin key), +# matching the same allowlist already used by /api/region-dossier and +# the other operator helpers further up this file. Single-operator nodes +# see no behavior change — their dashboard already lives on loopback or +# the trusted Docker bridge, so it still resolves. +@router.get("/api/sentinel2/search", dependencies=[Depends(require_local_operator)]) @limiter.limit("30/minute") def api_sentinel2_search( request: Request, @@ -97,7 +120,7 @@ def api_sentinel2_search( return search_sentinel2_scene(lat, lng) -@router.post("/api/sentinel/token") +@router.post("/api/sentinel/token", dependencies=[Depends(require_local_operator)]) @limiter.limit("60/minute") async def api_sentinel_token(request: Request): """Proxy Copernicus CDSE OAuth2 token request (avoids browser CORS block).""" @@ -152,7 +175,7 @@ import os as _os _SH_TOKEN_CACHE_HMAC_KEY = _os.urandom(32) -@router.post("/api/sentinel/tile") +@router.post("/api/sentinel/tile", dependencies=[Depends(require_local_operator)]) @limiter.limit("300/minute") async def api_sentinel_tile(request: Request): """Proxy Sentinel Hub Process API tile request (avoids CORS block).""" diff --git a/backend/tests/test_control_surface_auth.py b/backend/tests/test_control_surface_auth.py index 87915ac..187b2bd 100644 --- a/backend/tests/test_control_surface_auth.py +++ b/backend/tests/test_control_surface_auth.py @@ -89,6 +89,34 @@ import pytest # relay through the backend. 60/minute rate limit is not enough on # a streaming endpoint. ("get", "/api/radio/openmhz/audio?url=https%3A%2F%2Fmedia.openmhz.com%2Faudio%2Fabc.mp3", None), + # Issue #299 (tg12): /api/sentinel/token relays Copernicus CDSE + # OAuth token requests for caller-supplied client_id/secret. + # Anonymous access turns the backend into a free OAuth-mint relay. + ( + "post", + "/api/sentinel/token", + None, # body sent via raw form-encoded data — None lets the + # remote_client wrapper send an empty body; the auth + # check fires before the form parser runs. + ), + # Issue #300 (tg12): /api/sentinel/tile relays Sentinel Hub Process + # API tile fetches. Anonymous access is a bandwidth/quota relay + # for any caller's Copernicus account. + ( + "post", + "/api/sentinel/tile", + { + "client_id": "ignored", + "client_secret": "ignored", + "preset": "TRUE-COLOR", + "date": "2026-01-01", + "z": 6, "x": 30, "y": 20, + }, + ), + # Issue #301 (tg12): /api/sentinel2/search hits Planetary Computer + # STAC + Esri fallback. Anonymous access is a free external-search + # relay even though no caller credentials are involved. + ("get", "/api/sentinel2/search?lat=0&lng=0", None), ], ) def test_remote_control_surface_rejects_without_local_operator_or_admin( diff --git a/backend/tests/test_sentinel_routes_auth_gate.py b/backend/tests/test_sentinel_routes_auth_gate.py new file mode 100644 index 0000000..da96017 --- /dev/null +++ b/backend/tests/test_sentinel_routes_auth_gate.py @@ -0,0 +1,231 @@ +"""Issues #299, #300, #301 (tg12): Sentinel proxy routes must require +local-operator auth. + +Before the fix, three Sentinel proxy routes in ``backend/routers/tools.py`` +were decorated only with ``@limiter.limit(...)`` — no +``Depends(require_local_operator)``: + + * ``POST /api/sentinel/token`` — Copernicus CDSE OAuth relay for + caller-supplied client_id + client_secret. Anonymous access made the + backend a free OAuth-mint relay for any Sentinel account. + * ``POST /api/sentinel/tile`` — Sentinel Hub Process API relay. + Caller supplies their own credentials, backend mints a token if + needed and relays the PNG. Anonymous access was a bandwidth + quota + relay for any Copernicus account. + * ``GET /api/sentinel2/search`` — Planetary Computer STAC search with + Esri imagery fallback. No caller credentials are involved, but the + route is still an anonymous external-search relay. + +The fix adds ``dependencies=[Depends(require_local_operator)]`` to each. +The parameterized regression in ``test_control_surface_auth.py`` covers +the basic 403 path. This file adds the harder property: when the auth +gate fires, **the underlying upstream HTTP call never happens** — no +outbound Copernicus token mint, no Sentinel Hub Process call, no +Planetary Computer STAC search. The egress-on-403 property is what +separates a real gate from a route that returns 403 *after* burning a +quota. +""" + +from __future__ import annotations + +import asyncio +from unittest.mock import patch, MagicMock + +import pytest +from httpx import ASGITransport, AsyncClient + + +# --------------------------------------------------------------------------- +# Remote client fixture — same shape as test_control_surface_auth.py, but +# inlined here so this file doesn't depend on the shared remote_client +# fixture order. Uses 1.2.3.4 as the peer IP so loopback auth bypass +# doesn't accidentally let the request through. +# --------------------------------------------------------------------------- + + +class _PeerClient: + """Raw ASGI client with a configurable peer IP. FastAPI's + ``TestClient`` reports ``request.client.host`` as ``"testclient"`` + which isn't on the loopback allowlist — we need to set the peer + explicitly to exercise the real ``require_local_operator`` path. + """ + + def __init__(self, peer_ip: str): + from main import app + + self._loop = asyncio.new_event_loop() + self._transport = ASGITransport(app=app, client=(peer_ip, 12345)) + self._base = f"http://{peer_ip}:8000" + + def _do(self, method: str, url: str, **kw): + async def go(): + async with AsyncClient(transport=self._transport, base_url=self._base) as ac: + return await ac.request(method, url, **kw) + + return self._loop.run_until_complete(go()) + + def get(self, url, **kw): + return self._do("GET", url, **kw) + + def post(self, url, **kw): + return self._do("POST", url, **kw) + + def close(self): + self._loop.close() + + +@pytest.fixture +def remote(): + """Untrusted remote caller (1.2.3.4) — must hit the auth gate.""" + client = _PeerClient("1.2.3.4") + yield client + client.close() + + +@pytest.fixture +def loopback(): + """127.0.0.1 caller — must pass the gate exactly like the operator.""" + client = _PeerClient("127.0.0.1") + yield client + client.close() + + +# --------------------------------------------------------------------------- +# /api/sentinel/token — issue #299 +# --------------------------------------------------------------------------- + + +class TestSentinelTokenAuthGate: + def test_anonymous_caller_is_rejected(self, remote): + """A remote (non-loopback, non-bridge) caller MUST be rejected.""" + r = remote.post( + "/api/sentinel/token", + data={"client_id": "anything", "client_secret": "anything"}, + ) + assert r.status_code == 403 + + def test_no_upstream_token_mint_on_403(self, remote): + """The Copernicus token endpoint must NOT be contacted when the + auth gate fires. This is what makes the gate real — without it, + a 403 returned *after* the upstream call still burns quota. + + We patch ``requests.post`` at the module level so any outbound + token request would be intercepted. The mock is asserted to have + ZERO calls. + """ + fake_post = MagicMock() + # If the gate is broken, the route would call requests.post; we + # want this MagicMock to make that fact loud. + fake_post.side_effect = AssertionError( + "requests.post was called despite auth-gate 403 — the gate is bypassable" + ) + with patch("requests.post", fake_post): + r = remote.post( + "/api/sentinel/token", + data={"client_id": "anything", "client_secret": "anything"}, + ) + assert r.status_code == 403 + assert fake_post.call_count == 0 + + def test_loopback_caller_passes_auth(self, loopback): + """A 127.0.0.1 caller must pass the gate. We don't care about + the upstream response shape — just that the request reaches the + handler (which would then try to talk to Copernicus). We patch + ``requests.post`` to return a 401 so the test doesn't hit the + real network. + + Note: FastAPI's ``TestClient`` reports ``request.client.host`` + as ``"testclient"`` by default, which is NOT on the loopback + allowlist (``127.0.0.1`` / ``::1`` / ``localhost``). The + ``loopback`` fixture below uses raw ASGI with an explicit + ``127.0.0.1`` peer IP so the auth gate sees real loopback. + """ + fake_resp = MagicMock() + fake_resp.status_code = 401 + fake_resp.content = b'{"error": "invalid_client"}' + with patch("requests.post", return_value=fake_resp): + r = loopback.post( + "/api/sentinel/token", + data={"client_id": "anything", "client_secret": "anything"}, + ) + # 200 (relayed), 401 (upstream said no), or 502 (upstream blew up) + # are all acceptable — what matters is we got past the auth gate + # (no 403). The route relays the upstream response status. + assert r.status_code != 403 + + +# --------------------------------------------------------------------------- +# /api/sentinel/tile — issue #300 +# --------------------------------------------------------------------------- + + +class TestSentinelTileAuthGate: + _VALID_BODY = { + "client_id": "anything", + "client_secret": "anything", + "preset": "TRUE-COLOR", + "date": "2026-01-01", + "z": 6, + "x": 30, + "y": 20, + } + + def test_anonymous_caller_is_rejected(self, remote): + r = remote.post("/api/sentinel/tile", json=self._VALID_BODY) + assert r.status_code == 403 + + def test_no_upstream_call_on_403(self, remote): + """When the gate fires, neither the token mint nor the Process + API call should happen.""" + fake_post = MagicMock(side_effect=AssertionError( + "requests.post was called despite auth-gate 403 — gate bypassable" + )) + with patch("requests.post", fake_post): + r = remote.post("/api/sentinel/tile", json=self._VALID_BODY) + assert r.status_code == 403 + assert fake_post.call_count == 0 + + +# --------------------------------------------------------------------------- +# /api/sentinel2/search — issue #301 +# --------------------------------------------------------------------------- + + +class TestSentinel2SearchAuthGate: + def test_anonymous_caller_is_rejected(self, remote): + r = remote.get("/api/sentinel2/search?lat=0&lng=0") + assert r.status_code == 403 + + def test_no_upstream_search_on_403(self, remote): + """The Planetary Computer STAC search MUST NOT be called when + the gate fires.""" + fake = MagicMock(side_effect=AssertionError( + "search_sentinel2_scene was called despite 403 — gate bypassable" + )) + # Patch the underlying service function — that's the network + # surface. If the auth dep fires first, the handler body never + # runs and this stays uncalled. + with patch("services.sentinel_search.search_sentinel2_scene", fake): + r = remote.get("/api/sentinel2/search?lat=0&lng=0") + assert r.status_code == 403 + assert fake.call_count == 0 + + def test_loopback_caller_reaches_handler(self, loopback): + """127.0.0.1 must pass the gate and reach the search function. + Uses raw ASGI peer IP via the ``loopback`` fixture — TestClient + would set ``request.client.host`` to ``"testclient"`` which + isn't on the loopback allowlist.""" + fake = MagicMock(return_value={"ok": True, "results": []}) + with patch("services.sentinel_search.search_sentinel2_scene", fake): + r = loopback.get("/api/sentinel2/search?lat=0&lng=0") + assert r.status_code == 200 + assert fake.call_count == 1 + + +# Note: an earlier draft included a static dependency walker that +# inspected the FastAPI route table to assert require_local_operator +# was wired in. It was deleted because FastAPI's internal route +# representation varies across minor versions — the walker was brittle +# and the behavioral pair (anonymous → 403 with no upstream egress; +# loopback → handler reached) gives stronger end-to-end evidence than +# any structural check.