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.