From b041b5e97c0de1ef82e2b3bbe35cfcbb261a1d91 Mon Sep 17 00:00:00 2001 From: BigBodyCobain Date: Fri, 22 May 2026 10:44:50 -0600 Subject: [PATCH] Fix #298: move Sentinel credentials from browser storage to backend .env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by @tg12. Pre-fix, the Settings panel stored real third-party Copernicus CDSE client_id + client_secret in browser localStorage / sessionStorage via the privacy storage helper, and the proxy routes required those values to come back in every tile/token request body. Any same-origin script (XSS, malicious browser extension, dev-tools HAR export) had read access to the credentials. This change moves them server-side, behind the same .env-backed admin flow every other third-party API key (OpenSky, AIS Stream, Finnhub, Shodan, …) already uses. Backend ------- backend/services/api_settings.py * Added SENTINEL_CLIENT_ID and SENTINEL_CLIENT_SECRET entries to API_REGISTRY. The existing GET/PUT /api/settings/api-keys flow (already require_local_operator-gated, .env-backed) now manages them — no new route surface. backend/routers/tools.py * /api/sentinel/token and /api/sentinel/tile resolve credentials via a new _resolve_sentinel_credentials() helper: body fields win for back-compat with any legacy callers, otherwise the helper reads SENTINEL_CLIENT_ID / SENTINEL_CLIENT_SECRET from os.environ. * When neither source has a value, the route returns 400 with a friendly pointer ("Set SENTINEL_CLIENT_ID and SENTINEL_CLIENT_SECRET in the API Keys panel") instead of the curt "required" message. The user's standing rule against hostile errors applies. * Function bodies only — decorator lines untouched, so this PR does not conflict with #303 (which adds Depends(require_local_operator) to the same routes). Frontend -------- frontend/src/lib/sentinelHub.ts — rewritten * Removed: getSentinelCredentials / setSentinelCredentials / clearSentinelCredentials / getSentinelCredentialStorageMode. These were the browser-storage read/write helpers; their existence was the bug. * Added: checkBackendSentinelStatus(), refreshSentinelStatus(), getCachedSentinelStatus(), and a kept-for-back-compat hasSentinelCredentials() shim. Status is sourced from /api/settings/api-keys (the same endpoint the API Keys panel already uses), so we don't add a new route just for this read. * Added: migrateLegacySentinelBrowserKeys() — one-shot, idempotent helper that clears sb_sentinel_client_id / _secret / _instance_id from BOTH localStorage and sessionStorage. We deliberately do NOT auto-POST those legacy browser values to the backend; doing so would silently migrate a secret across a trust boundary without operator consent. Operators re-enter once in the API Keys panel and the legacy keys get wiped here. * fetchSentinelTile and getSentinelToken no longer send client_id / client_secret in the request body. The backend uses .env. frontend/src/components/SettingsPanel.tsx * Dropped sb_sentinel_client_id / _secret / _instance_id from PRIVACY_SENSITIVE_BROWSER_KEYS — they're no longer written. * SentinelTab rewritten: removed the inline Client ID / Client Secret inputs + Save / Clear / Test buttons. Replaced with a status panel that calls checkBackendSentinelStatus() on mount, a one-click "Open API Keys Panel" button, and a migration banner that appears only when migrateLegacySentinelBrowserKeys() actually cleared something. * Setup guide STEP 3 now points to the API Keys panel instead of the local form. frontend/src/app/page.tsx * Added a one-time useEffect that fires checkBackendSentinelStatus() on mount so the cached value (which the synchronous hasSentinelCredentials() shim reads) is populated before MaplibreViewer's tile-URL memo runs. Tests ----- backend/tests/test_sentinel_credentials_server_side.py (new) * API_REGISTRY surface — sentinel_client_id / sentinel_client_secret are registered with the right env_keys, ALLOWED_ENV_KEYS lets /api/settings/api-keys PUT them. * Resolution order — body wins, env is fallback, neither → 400 with the friendly pointer message, and NO upstream HTTP call when neither source has credentials (asserted via MagicMock(side_effect=AssertionError)). * /api/sentinel/tile same shape. frontend/src/__tests__/utils/sentinelHub.test.ts (new) * migrateLegacySentinelBrowserKeys clears localStorage AND sessionStorage, reports what it cleared, idempotent. * fetchSentinelTile + getSentinelToken POST WITHOUT client_id / client_secret in the body (plants leaked credentials in browser storage first to prove they are NOT picked up). * checkBackendSentinelStatus parses /api/settings/api-keys correctly: true only when both keys is_set, false on partial config or network errors. All 7 backend tests + 8 frontend tests pass locally. The test_no_new_duplicate_routes guard and the api-settings test suite still pass. Credit: @tg12 for the audit report. --- backend/routers/tools.py | 65 +++- backend/services/api_settings.py | 25 ++ .../test_sentinel_credentials_server_side.py | 277 ++++++++++++++++++ .../src/__tests__/utils/sentinelHub.test.ts | 169 +++++++++++ frontend/src/app/page.tsx | 10 + frontend/src/components/SettingsPanel.tsx | 249 +++++++--------- frontend/src/lib/sentinelHub.ts | 187 +++++++----- 7 files changed, 767 insertions(+), 215 deletions(-) create mode 100644 backend/tests/test_sentinel_credentials_server_side.py create mode 100644 frontend/src/__tests__/utils/sentinelHub.test.ts diff --git a/backend/routers/tools.py b/backend/routers/tools.py index 0faa392..90d71c9 100644 --- a/backend/routers/tools.py +++ b/backend/routers/tools.py @@ -97,18 +97,57 @@ def api_sentinel2_search( return search_sentinel2_scene(lat, lng) +# Issue #298 (tg12): Sentinel credentials moved server-side +# --------------------------------------------------------------------------- +# Previously the frontend kept Copernicus CDSE client_id + client_secret in +# browser localStorage / sessionStorage and forwarded them on every tile +# request through this proxy. That exposed real third-party credentials to +# any same-origin script (XSS, malicious browser extension, dev-tools HAR +# export). +# +# Resolution order (first match wins): +# 1. Request body — kept for back-compat. A small number of legacy +# operator setups may still post credentials; we don't break them. +# 2. Backend .env — SENTINEL_CLIENT_ID / SENTINEL_CLIENT_SECRET, managed +# through the existing /api/settings/api-keys flow (admin-gated). +# +# The frontend in ``sentinelHub.ts`` no longer reads browser storage and no +# longer forwards credentials — every dashboard request now lands in (2). +# --------------------------------------------------------------------------- +def _resolve_sentinel_credentials(body_id: str, body_secret: str) -> tuple[str, str]: + """Return (client_id, client_secret) using body values when present, + otherwise falling back to backend .env. Empty strings if neither is set.""" + import os as _os + cid = (body_id or "").strip() or (_os.environ.get("SENTINEL_CLIENT_ID", "") or "").strip() + csec = (body_secret or "").strip() or (_os.environ.get("SENTINEL_CLIENT_SECRET", "") or "").strip() + return cid, csec + + @router.post("/api/sentinel/token") @limiter.limit("60/minute") async def api_sentinel_token(request: Request): - """Proxy Copernicus CDSE OAuth2 token request (avoids browser CORS block).""" + """Proxy Copernicus CDSE OAuth2 token request (avoids browser CORS block). + + Credentials are resolved by ``_resolve_sentinel_credentials`` — body + fields are honored for back-compat, otherwise the backend .env values + populated through ``/api/settings/api-keys`` are used. + """ import requests as req body = await request.body() from urllib.parse import parse_qs params = parse_qs(body.decode("utf-8")) - client_id = params.get("client_id", [""])[0] - client_secret = params.get("client_secret", [""])[0] + body_id = params.get("client_id", [""])[0] + body_secret = params.get("client_secret", [""])[0] + client_id, client_secret = _resolve_sentinel_credentials(body_id, body_secret) if not client_id or not client_secret: - raise HTTPException(400, "client_id and client_secret required") + # Friendly, non-hostile error — points the operator at the place + # they configure other API keys instead of just saying "required". + raise HTTPException( + 400, + "Sentinel client_id/client_secret are not configured. " + "Set SENTINEL_CLIENT_ID and SENTINEL_CLIENT_SECRET in the " + "API Keys panel (Settings → API Keys) or your backend .env.", + ) token_url = "https://identity.dataspace.copernicus.eu/auth/realms/CDSE/protocol/openid-connect/token" try: resp = await asyncio.to_thread(req.post, token_url, @@ -163,8 +202,11 @@ async def api_sentinel_tile(request: Request): except Exception: return JSONResponse(status_code=422, content={"ok": False, "detail": "invalid JSON body"}) - client_id = body.get("client_id", "") - client_secret = body.get("client_secret", "") + # Issue #298: same resolution order as /api/sentinel/token — body + # values for back-compat, otherwise backend .env. + body_id = body.get("client_id", "") + body_secret = body.get("client_secret", "") + client_id, client_secret = _resolve_sentinel_credentials(body_id, body_secret) preset = body.get("preset", "TRUE-COLOR") date_str = body.get("date", "") z = body.get("z", 0) @@ -172,7 +214,16 @@ async def api_sentinel_tile(request: Request): y = body.get("y", 0) if not client_id or not client_secret or not date_str: - raise HTTPException(400, "client_id, client_secret, and date required") + # Distinguish "no creds" from "no date" so the operator knows + # what to fix. Same friendly pointer as the /token route. + if not client_id or not client_secret: + raise HTTPException( + 400, + "Sentinel client_id/client_secret are not configured. " + "Set SENTINEL_CLIENT_ID and SENTINEL_CLIENT_SECRET in the " + "API Keys panel (Settings → API Keys) or your backend .env.", + ) + raise HTTPException(400, "date required") now = _time.time() credential_fp = _credential_fingerprint(client_id, client_secret) diff --git a/backend/services/api_settings.py b/backend/services/api_settings.py index 530f95b..8e5265b 100644 --- a/backend/services/api_settings.py +++ b/backend/services/api_settings.py @@ -150,6 +150,31 @@ API_REGISTRY = [ "url": "https://finnhub.io/register", "required": False, }, + # Issue #298 (tg12): Sentinel Hub / Copernicus Data Space Ecosystem + # credentials were previously held in browser localStorage / sessionStorage + # by the Settings panel. Moved server-side to the same .env-backed + # store every other third-party API key lives in. The Sentinel proxy + # routes (POST /api/sentinel/token, /tile) now fall back to these + # env values when the request body omits credentials — see + # backend/routers/tools.py for the resolution order. + { + "id": "sentinel_client_id", + "env_key": "SENTINEL_CLIENT_ID", + "name": "Sentinel Hub / Copernicus — Client ID", + "description": "OAuth2 client ID for Copernicus Data Space Ecosystem (CDSE). Required for the Sentinel-2 imagery overlay and the right-click Sentinel-2 Intel Card. Sign in at dataspace.copernicus.eu and create OAuth credentials.", + "category": "Imagery", + "url": "https://dataspace.copernicus.eu/", + "required": False, + }, + { + "id": "sentinel_client_secret", + "env_key": "SENTINEL_CLIENT_SECRET", + "name": "Sentinel Hub / Copernicus — Client Secret", + "description": "OAuth2 client secret paired with the Client ID above. Used by the backend to mint short-lived access tokens against the CDSE identity provider. Stored in the backend .env; never sent to the browser.", + "category": "Imagery", + "url": "https://dataspace.copernicus.eu/", + "required": False, + }, ] ALLOWED_ENV_KEYS = { diff --git a/backend/tests/test_sentinel_credentials_server_side.py b/backend/tests/test_sentinel_credentials_server_side.py new file mode 100644 index 0000000..f7629c1 --- /dev/null +++ b/backend/tests/test_sentinel_credentials_server_side.py @@ -0,0 +1,277 @@ +"""Issue #298 (tg12): Sentinel credentials must live server-side. + +Before the fix, ``frontend/src/components/SettingsPanel.tsx`` stored +``client_id`` and ``client_secret`` in ``localStorage`` / +``sessionStorage`` via the privacy storage helper, and the proxy routes +in ``backend/routers/tools.py`` REQUIRED those values to come in the +request body. Any same-origin script (XSS, malicious extension, +dev-tools HAR export) had read access to real third-party Sentinel +credentials. + +After the fix: + + * ``SENTINEL_CLIENT_ID`` and ``SENTINEL_CLIENT_SECRET`` are entries + in the ``api_settings.API_REGISTRY`` and are persisted via the + existing ``/api/settings/api-keys`` flow (admin-gated, .env-backed, + never returned to the browser). + * The proxy routes prefer request-body values for back-compat but + fall back to ``os.environ.get("SENTINEL_CLIENT_ID")`` / + ``os.environ.get("SENTINEL_CLIENT_SECRET")`` when the body omits + them. The dashboard's ``sentinelHub.ts`` no longer sends credentials + in the body — every request now hits the env path. + * When neither source has a value, the route returns a 400 with a + pointer to the API Keys panel rather than a curt "client_id and + client_secret required" message. + +These tests cover the resolution order and the registry surface. +""" + +from __future__ import annotations + +from unittest.mock import patch, MagicMock + +import pytest + + +# --------------------------------------------------------------------------- +# Helper: import the routes module fresh per test so monkey-patched +# environment variables are picked up by the route's os.environ.get call. +# (The lookup is per-request, not at import time, so this isn't strictly +# required — but it makes the test layout obvious.) +# --------------------------------------------------------------------------- + + +@pytest.fixture +def loopback_client(): + """ASGI client with peer IP 127.0.0.1 so the Sentinel routes' (post-#303) + ``require_local_operator`` gate passes. + + Built without a context manager so the privacy-core lifespan check + doesn't run in the test env. + """ + import asyncio + from httpx import ASGITransport, AsyncClient + from main import app + + class _Loop: + def __init__(self): + self._loop = asyncio.new_event_loop() + self._transport = ASGITransport(app=app, client=("127.0.0.1", 12345)) + self._base = "http://127.0.0.1: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 put(self, url, **kw): return self._do("PUT", url, **kw) + + def close(self): self._loop.close() + + c = _Loop() + yield c + c.close() + + +# --------------------------------------------------------------------------- +# API_REGISTRY surface +# --------------------------------------------------------------------------- + + +class TestApiRegistry: + def test_sentinel_keys_registered(self): + """Both Sentinel keys must be entries in API_REGISTRY so the + existing /api/settings/api-keys PUT flow can write them to .env.""" + from services.api_settings import API_REGISTRY, ALLOWED_ENV_KEYS + + ids = {row["id"] for row in API_REGISTRY} + assert "sentinel_client_id" in ids + assert "sentinel_client_secret" in ids + + # Critical: ALLOWED_ENV_KEYS is the gate on which .env keys the + # API can mutate. If we forgot to add the env_key field on the + # registry rows, callers couldn't actually save the values. + assert "SENTINEL_CLIENT_ID" in ALLOWED_ENV_KEYS + assert "SENTINEL_CLIENT_SECRET" in ALLOWED_ENV_KEYS + + def test_api_keys_put_accepts_sentinel_keys(self, loopback_client, monkeypatch, tmp_path): + """End-to-end: PUT /api/settings/api-keys with SENTINEL_CLIENT_ID + + SENTINEL_CLIENT_SECRET must persist to .env.""" + import services.api_settings as api_settings + + # Redirect both .env paths to tmp so the test doesn't mutate + # the developer's real backend .env. + tmp_env = tmp_path / ".env" + monkeypatch.setattr(api_settings, "ENV_PATH", tmp_env) + monkeypatch.setattr(api_settings, "OPERATOR_KEYS_ENV_PATH", tmp_path / "operator_api_keys.env") + + r = loopback_client.put( + "/api/settings/api-keys", + json={ + "SENTINEL_CLIENT_ID": "test-sentinel-id", + "SENTINEL_CLIENT_SECRET": "test-sentinel-secret", + }, + ) + assert r.status_code == 200, f"PUT failed: {r.text}" + body = r.json() + assert body.get("ok") is True + + # File on disk should now carry both keys. + parsed = api_settings._parse_env_file(tmp_env) + assert parsed.get("SENTINEL_CLIENT_ID") == "test-sentinel-id" + assert parsed.get("SENTINEL_CLIENT_SECRET") == "test-sentinel-secret" + + +# --------------------------------------------------------------------------- +# Credential resolution — body wins, env is fallback, neither is 400 +# --------------------------------------------------------------------------- + + +class TestSentinelTokenCredResolution: + def test_env_fallback_when_body_empty(self, loopback_client, monkeypatch): + """No body credentials → backend reads .env values.""" + monkeypatch.setenv("SENTINEL_CLIENT_ID", "env-id") + monkeypatch.setenv("SENTINEL_CLIENT_SECRET", "env-secret") + + # Mock the upstream Copernicus call so we don't hit the network. + # Capture what was sent so we can prove env values were used. + captured: dict = {} + fake_resp = MagicMock() + fake_resp.status_code = 200 + fake_resp.content = b'{"access_token": "stub", "expires_in": 300}' + + def fake_post(url, *args, **kwargs): + captured["url"] = url + captured["data"] = kwargs.get("data", {}) + return fake_resp + + with patch("requests.post", side_effect=fake_post): + r = loopback_client.post( + "/api/sentinel/token", + data={}, # ← deliberately empty body + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + assert r.status_code == 200 + # The forwarded creds must come from env, not from a stale cache + # or fallback string. + assert captured.get("data", {}).get("client_id") == "env-id" + assert captured.get("data", {}).get("client_secret") == "env-secret" + + def test_body_credentials_win_over_env(self, loopback_client, monkeypatch): + """Body values (back-compat path) must win when both sources + are present. This preserves the pre-#298 behavior for any + legacy callers that still post credentials.""" + monkeypatch.setenv("SENTINEL_CLIENT_ID", "env-id") + monkeypatch.setenv("SENTINEL_CLIENT_SECRET", "env-secret") + + captured: dict = {} + fake_resp = MagicMock() + fake_resp.status_code = 200 + fake_resp.content = b'{"access_token": "stub"}' + + def fake_post(url, *args, **kwargs): + captured["data"] = kwargs.get("data", {}) + return fake_resp + + with patch("requests.post", side_effect=fake_post): + r = loopback_client.post( + "/api/sentinel/token", + data={"client_id": "body-id", "client_secret": "body-secret"}, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + assert r.status_code == 200 + assert captured["data"]["client_id"] == "body-id" + assert captured["data"]["client_secret"] == "body-secret" + + def test_400_when_neither_source_has_credentials(self, loopback_client, monkeypatch): + """If body is empty AND env is empty, return 400 with a + friendly pointer to the API Keys panel — not a curt + "required" message and not a 500.""" + monkeypatch.delenv("SENTINEL_CLIENT_ID", raising=False) + monkeypatch.delenv("SENTINEL_CLIENT_SECRET", raising=False) + + # If the route ever calls requests.post here, the gate is broken + # — empty creds should never produce an outbound HTTP call. + fake = MagicMock(side_effect=AssertionError( + "requests.post should not be called when no credentials are configured" + )) + with patch("requests.post", fake): + r = loopback_client.post( + "/api/sentinel/token", + data={}, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + assert r.status_code == 400 + detail = r.json().get("detail", "") + # The pointer to the API Keys panel is what makes this non-hostile. + assert "API Keys panel" in detail or "SENTINEL_CLIENT_ID" in detail + assert fake.call_count == 0 + + +class TestSentinelTileCredResolution: + def test_env_fallback_when_body_omits_credentials(self, loopback_client, monkeypatch): + """Tile route: no body credentials → uses env values.""" + monkeypatch.setenv("SENTINEL_CLIENT_ID", "env-id") + monkeypatch.setenv("SENTINEL_CLIENT_SECRET", "env-secret") + + token_resp = MagicMock() + token_resp.status_code = 200 + token_resp.json = MagicMock(return_value={"access_token": "stub", "expires_in": 300}) + + process_resp = MagicMock() + process_resp.status_code = 200 + process_resp.content = b"" + process_resp.headers = {"content-type": "image/png"} + + captured: list = [] + + def fake_post(url, *args, **kwargs): + captured.append({"url": url, "data": kwargs.get("data"), "json": kwargs.get("json")}) + if "openid-connect/token" in url: + return token_resp + return process_resp + + with patch("requests.post", side_effect=fake_post): + r = loopback_client.post( + "/api/sentinel/tile", + json={ + # Note: no client_id / client_secret in body + "preset": "TRUE-COLOR", + "date": "2026-01-01", + "z": 6, "x": 30, "y": 20, + }, + ) + + assert r.status_code == 200 + # First call was the token mint; verify it used env creds. + token_call = next(c for c in captured if "openid-connect/token" in c["url"]) + assert token_call["data"]["client_id"] == "env-id" + assert token_call["data"]["client_secret"] == "env-secret" + + def test_400_when_neither_source_has_credentials(self, loopback_client, monkeypatch): + monkeypatch.delenv("SENTINEL_CLIENT_ID", raising=False) + monkeypatch.delenv("SENTINEL_CLIENT_SECRET", raising=False) + + fake = MagicMock(side_effect=AssertionError( + "requests.post should not be called when no credentials are configured" + )) + with patch("requests.post", fake): + r = loopback_client.post( + "/api/sentinel/tile", + json={ + "preset": "TRUE-COLOR", + "date": "2026-01-01", + "z": 6, "x": 30, "y": 20, + }, + ) + + assert r.status_code == 400 + detail = r.json().get("detail", "") + assert "API Keys panel" in detail or "SENTINEL_CLIENT_ID" in detail + assert fake.call_count == 0 diff --git a/frontend/src/__tests__/utils/sentinelHub.test.ts b/frontend/src/__tests__/utils/sentinelHub.test.ts new file mode 100644 index 0000000..b34745d --- /dev/null +++ b/frontend/src/__tests__/utils/sentinelHub.test.ts @@ -0,0 +1,169 @@ +/** + * Issue #298 (tg12): Sentinel credentials must no longer live in browser + * storage, and the proxy calls must not forward them in request bodies. + * These tests pin both invariants on ``lib/sentinelHub``: + * + * 1. ``migrateLegacySentinelBrowserKeys()`` clears the legacy keys + * idempotently and reports what it cleared. + * 2. ``fetchSentinelTile()`` and ``getSentinelToken()`` POST WITHOUT + * ``client_id`` or ``client_secret`` in the body — the backend + * resolves credentials from its ``.env``. A future refactor that + * accidentally re-introduces browser-storage reads (e.g. by + * restoring ``getSentinelCredentials()`` and forwarding it) gets a + * loud test failure here rather than a silent privacy regression. + * 3. ``checkBackendSentinelStatus()`` queries ``/api/settings/api-keys`` + * and returns true only when both Sentinel keys report ``is_set``. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + migrateLegacySentinelBrowserKeys, + fetchSentinelTile, + getSentinelToken, + checkBackendSentinelStatus, + refreshSentinelStatus, +} from '@/lib/sentinelHub'; + +const originalFetch = globalThis.fetch; + +describe('lib/sentinelHub — issue #298 server-side credentials', () => { + beforeEach(() => { + window.localStorage.clear(); + window.sessionStorage.clear(); + refreshSentinelStatus(); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + window.localStorage.clear(); + window.sessionStorage.clear(); + refreshSentinelStatus(); + }); + + describe('migrateLegacySentinelBrowserKeys', () => { + it('clears legacy localStorage keys and reports what it cleared', () => { + window.localStorage.setItem('sb_sentinel_client_id', 'sh-leaked-id'); + window.localStorage.setItem('sb_sentinel_client_secret', 'leaked-secret'); + window.localStorage.setItem('sb_sentinel_instance_id', 'leaked-instance'); + + const result = migrateLegacySentinelBrowserKeys(); + + expect(window.localStorage.getItem('sb_sentinel_client_id')).toBeNull(); + expect(window.localStorage.getItem('sb_sentinel_client_secret')).toBeNull(); + expect(window.localStorage.getItem('sb_sentinel_instance_id')).toBeNull(); + expect(result.cleared.sort()).toEqual([ + 'sb_sentinel_client_id', + 'sb_sentinel_client_secret', + 'sb_sentinel_instance_id', + ].sort()); + }); + + it('clears sessionStorage too (privacy-strict mode used to put them there)', () => { + window.sessionStorage.setItem('sb_sentinel_client_id', 'sh-session-id'); + window.sessionStorage.setItem('sb_sentinel_client_secret', 'session-secret'); + + const result = migrateLegacySentinelBrowserKeys(); + + expect(window.sessionStorage.getItem('sb_sentinel_client_id')).toBeNull(); + expect(window.sessionStorage.getItem('sb_sentinel_client_secret')).toBeNull(); + expect(result.cleared).toContain('sb_sentinel_client_id'); + expect(result.cleared).toContain('sb_sentinel_client_secret'); + }); + + it('is idempotent — calling it on a clean store reports nothing cleared', () => { + const result = migrateLegacySentinelBrowserKeys(); + expect(result.cleared).toEqual([]); + }); + }); + + describe('proxy requests no longer forward credentials', () => { + it('fetchSentinelTile POSTs without client_id/client_secret in the body', async () => { + // Plant credentials in browser storage to prove they would NOT be + // picked up even if present. Pre-#298, this would have been read + // from localStorage and posted in the body. + window.localStorage.setItem('sb_sentinel_client_id', 'sh-leaked-id'); + window.localStorage.setItem('sb_sentinel_client_secret', 'leaked-secret'); + + const fetchMock = vi.fn(async () => new Response(new ArrayBuffer(0), { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + + await fetchSentinelTile(6, 30, 20, 'TRUE-COLOR', '2026-01-01'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [, init] = fetchMock.mock.calls[0] as [unknown, RequestInit]; + const body = JSON.parse(String(init.body)); + expect(body).not.toHaveProperty('client_id'); + expect(body).not.toHaveProperty('client_secret'); + // Sanity: the legitimate fields are still there. + expect(body).toMatchObject({ preset: 'TRUE-COLOR', date: '2026-01-01', z: 6, x: 30, y: 20 }); + }); + + it('getSentinelToken POSTs with an empty form body (backend uses env)', async () => { + window.localStorage.setItem('sb_sentinel_client_id', 'sh-leaked-id'); + window.localStorage.setItem('sb_sentinel_client_secret', 'leaked-secret'); + + const fetchMock = vi.fn(async () => + new Response(JSON.stringify({ access_token: 'stub', expires_in: 300 }), { status: 200 }), + ); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + + const token = await getSentinelToken(); + + expect(token).toBe('stub'); + expect(fetchMock).toHaveBeenCalledTimes(1); + const [, init] = fetchMock.mock.calls[0] as [unknown, RequestInit]; + const body = String(init.body); + // Body is a URLSearchParams stringification. We assert that the + // leaked credential never appears in it. + expect(body).not.toContain('sh-leaked-id'); + expect(body).not.toContain('leaked-secret'); + }); + }); + + describe('checkBackendSentinelStatus', () => { + it('returns true when both Sentinel keys report is_set on /api/settings/api-keys', async () => { + const fetchMock = vi.fn(async (input: unknown) => { + const url = String(input); + if (url.endsWith('/api/settings/api-keys')) { + return new Response( + JSON.stringify([ + { id: 'sentinel_client_id', env_key: 'SENTINEL_CLIENT_ID', is_set: true }, + { id: 'sentinel_client_secret', env_key: 'SENTINEL_CLIENT_SECRET', is_set: true }, + { id: 'opensky_client_id', env_key: 'OPENSKY_CLIENT_ID', is_set: false }, + ]), + { status: 200 }, + ); + } + return new Response('not found', { status: 404 }); + }); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + + const configured = await checkBackendSentinelStatus(); + expect(configured).toBe(true); + }); + + it('returns false when only one of the two keys is set', async () => { + const fetchMock = vi.fn(async () => + new Response( + JSON.stringify([ + { id: 'sentinel_client_id', env_key: 'SENTINEL_CLIENT_ID', is_set: true }, + { id: 'sentinel_client_secret', env_key: 'SENTINEL_CLIENT_SECRET', is_set: false }, + ]), + { status: 200 }, + ), + ); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + + const configured = await checkBackendSentinelStatus(); + expect(configured).toBe(false); + }); + + it('fails safely (false) when the backend errors', async () => { + const fetchMock = vi.fn(async () => { throw new Error('network down'); }); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + + const configured = await checkBackendSentinelStatus(); + expect(configured).toBe(false); + }); + }); +}); diff --git a/frontend/src/app/page.tsx b/frontend/src/app/page.tsx index b8e388c..26a3ea7 100644 --- a/frontend/src/app/page.tsx +++ b/frontend/src/app/page.tsx @@ -50,6 +50,7 @@ import { hasSentinelInfoBeenSeen, markSentinelInfoSeen, hasSentinelCredentials, + checkBackendSentinelStatus, } from '@/lib/sentinelHub'; import { useTranslation } from '@/i18n'; import { LocateBar } from './LocateBar'; @@ -107,6 +108,15 @@ export default function Dashboard() { useEffect(() => { localStorage.setItem('sb_ticker_open', tickerOpen.toString()); }, [tickerOpen]); + + // Issue #298: kick the one-time backend Sentinel-status check on mount. + // This populates the cached value that ``hasSentinelCredentials()`` reads + // synchronously elsewhere (MaplibreViewer's tile-URL memo, the + // Sentinel-info modal flow). Fire-and-forget — the cache stays false + // until resolved so the UI fails safely. + useEffect(() => { + void checkBackendSentinelStatus(); + }, []); const [settingsOpen, setSettingsOpen] = useState(false); const [legendOpen, setLegendOpen] = useState(false); const [shortcutsOpen, setShortcutsOpen] = useState(false); diff --git a/frontend/src/components/SettingsPanel.tsx b/frontend/src/components/SettingsPanel.tsx index 11316a1..4daa324 100644 --- a/frontend/src/components/SettingsPanel.tsx +++ b/frontend/src/components/SettingsPanel.tsx @@ -74,17 +74,18 @@ import { Trash2, RotateCcw, Satellite, - Eye, - EyeOff, Copy, Check, Radar, } from 'lucide-react'; import { - clearSentinelCredentials, - getSentinelCredentialStorageMode, - getSentinelCredentials, - setSentinelCredentials, + // Issue #298: Sentinel credentials now live server-side. The legacy + // browser-storage helpers (getSentinelCredentials / setSentinelCredentials + // / clearSentinelCredentials / getSentinelCredentialStorageMode) have + // been removed from sentinelHub.ts. We use the new status check + the + // one-time migration helper instead. + checkBackendSentinelStatus, + migrateLegacySentinelBrowserKeys, } from '@/lib/sentinelHub'; import { getPrivacyProfilePreference, @@ -143,10 +144,14 @@ const WEIGHT_COLORS: Record = { const SETTINGS_FOCUS_KEY = 'sb_settings_focus'; const WORMHOLE_RETURN_KEY = 'sb_wormhole_return_target'; const WORMHOLE_READY_EVENT = 'sb:wormhole-ready'; +// Issue #298 (tg12): Sentinel credentials moved from browser storage to +// the backend ``.env`` (managed through the API Keys panel). The legacy +// keys (``sb_sentinel_client_id`` / ``sb_sentinel_client_secret`` / +// ``sb_sentinel_instance_id``) are no longer treated as sensitive +// browser state because they are no longer written. ``SentinelTab`` +// runs ``migrateLegacySentinelBrowserKeys()`` once on mount to clear +// any leftover values from pre-#298 installs. const PRIVACY_SENSITIVE_BROWSER_KEYS = [ - 'sb_sentinel_client_id', - 'sb_sentinel_client_secret', - 'sb_sentinel_instance_id', 'sb_infonet_head', 'sb_infonet_head_history', 'sb_infonet_peers', @@ -2615,7 +2620,9 @@ const SettingsPanel = React.memo(function SettingsPanel({ )} {/* ==================== SENTINEL HUB TAB ==================== */} - {activeTab === 'sentinel' && } + {activeTab === 'sentinel' && ( + setActiveTab('api-keys')} /> + )} {activeTab === 'sar' && } @@ -2625,63 +2632,58 @@ const SettingsPanel = React.memo(function SettingsPanel({ }); // ─── Sentinel Hub Settings Tab ───────────────────────────────────────────── -function SentinelTab() { - const [clientId, setClientId] = useState(() => getSentinelCredentials().clientId); - const [clientSecret, setClientSecret] = useState(() => getSentinelCredentials().clientSecret); - const [testing, setTesting] = useState(false); - const [status, setStatus] = useState<{ ok: boolean; msg: string } | null>(null); - const [dirty, setDirty] = useState(false); - const [showSecret, setShowSecret] = useState(false); - const storageMode = getSentinelCredentialStorageMode(); +// Issue #298 (tg12): Sentinel credentials now live in the backend ``.env`` +// and are managed through the existing API Keys panel — same flow as every +// other third-party API key (OpenSky, AIS Stream, Finnhub, …). This tab no +// longer collects credentials. It does three things: +// 1. Runs migrateLegacySentinelBrowserKeys() once to wipe pre-#298 +// values out of localStorage / sessionStorage. +// 2. Shows the operator whether the backend has the credentials. +// 3. Offers a one-click jump to the API Keys panel where they enter them. +function SentinelTab({ onGoToApiKeys }: { onGoToApiKeys: () => void }) { + const [backendConfigured, setBackendConfigured] = useState(null); + const [migrationResult, setMigrationResult] = useState<{ cleared: string[] } | null>(null); + const [refreshing, setRefreshing] = useState(false); - const save = () => { - setSentinelCredentials(clientId.trim(), clientSecret.trim()); - setDirty(false); - setStatus({ - ok: true, - msg: `Credentials saved to browser ${storageMode === 'session' ? 'session' : 'local'} storage.`, - }); - }; + useEffect(() => { + // One-time legacy browser-key wipe. Idempotent — does nothing on a + // fresh install. We do NOT silently POST any browser-stored values + // to the backend; operators who relied on them re-enter once in the + // API Keys panel. Doing the wipe regardless ensures pre-#298 secrets + // don't linger in localStorage indefinitely. + setMigrationResult(migrateLegacySentinelBrowserKeys()); - const testConnection = async () => { - setTesting(true); - setStatus(null); + // Check whether the backend has SENTINEL_CLIENT_ID/SECRET set. + void checkBackendSentinelStatus().then(setBackendConfigured); + }, []); + + const refresh = async () => { + setRefreshing(true); try { - const resp = await fetch(`${API_BASE}/api/sentinel/token`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams({ - client_id: clientId.trim(), - client_secret: clientSecret.trim(), - }), - }); - if (resp.ok) { - setStatus({ ok: true, msg: 'Connected — token acquired successfully.' }); - } else { - const text = await resp.text().catch(() => ''); - setStatus({ ok: false, msg: `Auth failed (${resp.status}): ${text.slice(0, 120)}` }); - } - } catch (err) { - const msg = - typeof err === 'object' && err !== null && 'message' in err - ? String((err as { message?: string }).message) - : 'unknown'; - setStatus({ ok: false, msg: `Network error: ${msg}` }); + // refreshSentinelStatus() invalidates the module-level cache so the + // next check actually hits the backend instead of returning the + // memoized value. Lazy-imported so SSR/tests don't choke. + const { refreshSentinelStatus } = await import('@/lib/sentinelHub'); + refreshSentinelStatus(); + const ok = await checkBackendSentinelStatus(); + setBackendConfigured(ok); } finally { - setTesting(false); + setRefreshing(false); } }; - const clear = () => { - clearSentinelCredentials(); - setClientId(''); - setClientSecret(''); - setDirty(false); - setStatus({ ok: true, msg: 'Credentials cleared.' }); - }; - - const inputCls = - 'w-full bg-[var(--bg-primary)]/60 border border-[var(--border-primary)] px-3 py-2 text-[11px] font-mono text-[var(--text-secondary)] outline-none focus:border-purple-500 placeholder:text-[var(--text-muted)]/50 transition-colors'; + const statusColor = + backendConfigured === null + ? 'text-[var(--text-muted)]' + : backendConfigured + ? 'text-green-400' + : 'text-yellow-400'; + const statusLabel = + backendConfigured === null + ? 'CHECKING…' + : backendConfigured + ? 'CONFIGURED ON BACKEND' + : 'NOT CONFIGURED'; return (
@@ -2733,106 +2735,73 @@ function SentinelTab() {

STEP 3:{' '} - Paste both values in the fields below, hit{' '} - SAVE, then{' '} - TEST CONNECTION to verify. - That's it! + Paste both values into the API Keys panel + under SENTINEL_CLIENT_ID and{' '} + SENTINEL_CLIENT_SECRET, then hit Save. + The backend uses them to mint short-lived tokens — your browser never sees + the secret again.

- {/* Credential Inputs */} -
-
- - { - setClientId(e.target.value); - setDirty(true); - }} - placeholder="sh-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" - spellCheck={false} - autoComplete="off" - className={inputCls} - /> + {/* Backend status */} +
+
+ + BACKEND STATUS + + + {statusLabel} +
-
- - { - setClientSecret(e.target.value); - setDirty(true); - }} - placeholder="Paste client secret here..." - spellCheck={false} - autoComplete="new-password" - className={inputCls} - /> +

+ {backendConfigured === false + ? 'Sentinel credentials are not yet set in the backend .env. Open the API Keys panel to enter them — the tile overlay and Sentinel-2 Intel Card will work as soon as both fields are saved.' + : backendConfigured === true + ? 'Sentinel credentials are configured on the backend. The dashboard fetches tokens automatically; your browser does not handle the secret.' + : 'Checking backend configuration…'} +

+
+
- {/* Status */} - {status && ( -
- {status.msg} + {/* Migration notice (only if we actually cleared anything) */} + {migrationResult && migrationResult.cleared.length > 0 && ( +
+

LEGACY BROWSER CREDENTIALS CLEARED

+

+ Found and removed pre-#298 Sentinel credentials from browser storage + ({migrationResult.cleared.join(', ')}). Re-enter them in the API Keys panel + above; they'll be stored server-side from now on and never sent back to + the browser. +

)} - {/* Actions */} + {/* Footer + Usage Meter */}
-
- - - -
- {/* Usage Meter */} -

- Credentials stay in browser-only storage and never touch ShadowBroker servers. - {storageMode === 'session' - ? ' Current privacy mode keeps them in session storage only.' - : ' Current privacy mode keeps them in local storage for persistence.'} + Credentials are stored in the backend .env{' '} + and never sent to the browser. The tile proxy mints short-lived OAuth tokens + on demand using those values.

diff --git a/frontend/src/lib/sentinelHub.ts b/frontend/src/lib/sentinelHub.ts index c82b81d..f74fd02 100644 --- a/frontend/src/lib/sentinelHub.ts +++ b/frontend/src/lib/sentinelHub.ts @@ -1,77 +1,137 @@ /** - * Sentinel Hub (Copernicus CDSE) — client-side token management & Process API tile fetcher. + * Sentinel Hub (Copernicus CDSE) — client-side token + Process API tile fetcher. * - * Credentials are stored in browser-controlled storage only. In privacy/session - * mode they stay session-scoped; otherwise they persist in local storage. Token - * exchange is proxied through the ShadowBroker backend (/api/sentinel/token) to - * avoid CORS blocks from the Copernicus identity provider. Credentials are - * forwarded, never stored server-side. + * Issue #298 (tg12): Credentials are now stored server-side in the backend + * ``.env`` (managed through the existing ``/api/settings/api-keys`` flow, + * same as every other third-party API key). The browser no longer holds + * ``client_id`` / ``client_secret`` in localStorage or sessionStorage and + * no longer forwards them in proxy requests. * - * Uses the Process API with inline evalscripts — no Instance ID / Configuration needed. + * Old browser-storage keys (``sb_sentinel_client_id`` / ``sb_sentinel_client_secret`` + * / ``sb_sentinel_instance_id``) are migrated out by ``SettingsPanel`` on + * first mount after the upgrade — see ``migrateLegacySentinelBrowserKeys()`` + * exported below. */ import { API_BASE } from '@/lib/api'; -import { - getSensitiveBrowserItem, - getSensitiveBrowserStorageMode, - removeSensitiveBrowserItem, - setSensitiveBrowserItem, -} from '@/lib/privacyBrowserStorage'; -// Token exchange proxied through our backend (Copernicus blocks browser CORS) +// Token exchange proxied through our backend (Copernicus blocks browser CORS). const TOKEN_PROXY_URL = `${API_BASE}/api/sentinel/token`; -// browser-storage keys -const LS_CLIENT_ID = 'sb_sentinel_client_id'; -const LS_CLIENT_SECRET = 'sb_sentinel_client_secret'; - // In-memory token cache (never persisted) let cachedToken: string | null = null; let tokenExpiry = 0; // Dedup: only one in-flight token request at a time let _tokenPromise: Promise | null = null; -// ─── Credential helpers ──────────────────────────────────────────────────── +// In-memory cache of "does the backend have Sentinel credentials configured?" +// so the rest of the UI can short-circuit tile load attempts without a server +// round-trip per tile. Refreshed by callers via `refreshSentinelStatus()`. +let _backendCredentialsConfigured: boolean | null = null; +let _backendStatusPromise: Promise | null = null; -export function getSentinelCredentials(): { - clientId: string; - clientSecret: string; -} { - if (typeof window === 'undefined') return { clientId: '', clientSecret: '' }; - return { - clientId: getSensitiveBrowserItem(LS_CLIENT_ID) || '', - clientSecret: getSensitiveBrowserItem(LS_CLIENT_SECRET) || '', - }; +// ─── Credential status (server-side) ─────────────────────────────────────── + +/** + * Ask the backend whether Sentinel credentials are configured in ``.env``. + * Caches the result in memory; call ``refreshSentinelStatus()`` after the + * operator saves new API keys in the settings panel. + * + * Returns ``false`` on network errors so the UI fails safely (no broken + * tile requests). Never returns the secret itself — that stays server-side. + */ +export async function checkBackendSentinelStatus(): Promise { + if (_backendCredentialsConfigured !== null) return _backendCredentialsConfigured; + if (_backendStatusPromise) return _backendStatusPromise; + + _backendStatusPromise = (async () => { + try { + const resp = await fetch(`${API_BASE}/api/settings/api-keys`, { + headers: { Accept: 'application/json' }, + }); + if (!resp.ok) return false; + const list = await resp.json(); + // /api/settings/api-keys returns an array of { id, env_key, is_set, ... } + const ids = new Set(['sentinel_client_id', 'sentinel_client_secret']); + const configured = Array.isArray(list) + && list.filter((row: { id?: string; is_set?: boolean }) => + row && row.id && ids.has(row.id) && row.is_set === true, + ).length === 2; + _backendCredentialsConfigured = configured; + return configured; + } catch { + _backendCredentialsConfigured = false; + return false; + } finally { + _backendStatusPromise = null; + } + })(); + + return _backendStatusPromise; } -export function setSentinelCredentials(clientId: string, clientSecret: string): void { - setSensitiveBrowserItem(LS_CLIENT_ID, clientId); - setSensitiveBrowserItem(LS_CLIENT_SECRET, clientSecret); - // Invalidate cached token when credentials change +/** Invalidate the cached status — call this after the API Keys panel saves. */ +export function refreshSentinelStatus(): void { + _backendCredentialsConfigured = null; + // Drop any cached token too — credentials may have changed. cachedToken = null; tokenExpiry = 0; } -export function clearSentinelCredentials(): void { - removeSensitiveBrowserItem(LS_CLIENT_ID); - removeSensitiveBrowserItem(LS_CLIENT_SECRET); - // Also remove legacy instance ID if present - removeSensitiveBrowserItem('sb_sentinel_instance_id'); - if (typeof window !== 'undefined') { - localStorage.removeItem('sb_sentinel_instance_id'); - sessionStorage.removeItem('sb_sentinel_instance_id'); - } - cachedToken = null; - tokenExpiry = 0; -} - -export function getSentinelCredentialStorageMode(): 'local' | 'session' { - return getSensitiveBrowserStorageMode(); +/** + * Synchronous getter — returns the last known status without a network call. + * Returns ``null`` until ``checkBackendSentinelStatus()`` has run at least once. + */ +export function getCachedSentinelStatus(): boolean | null { + return _backendCredentialsConfigured; } +/** + * Back-compat shim. Pre-#298 callers asked ``hasSentinelCredentials()`` to + * decide whether to render the Sentinel layer / open the API key prompt. + * The credential now lives server-side, so this is just the cached + * server-status check. Returns ``false`` until the first + * ``checkBackendSentinelStatus()`` resolves (callers should kick that off + * once at app startup — see ``page.tsx`` mount effect). + */ export function hasSentinelCredentials(): boolean { - const { clientId, clientSecret } = getSentinelCredentials(); - return Boolean(clientId && clientSecret); + return _backendCredentialsConfigured === true; +} + +/** + * One-time migration helper: clear the legacy browser-storage keys that + * pre-#298 versions used to persist Sentinel credentials. Idempotent and + * safe to call on every page load — does nothing if no keys are present. + * + * Called by ``SettingsPanel`` on mount. We do NOT auto-POST the legacy + * browser values to the backend, because doing so would silently migrate + * a secret across a trust boundary without operator consent. Operators + * who relied on browser-stored credentials will re-enter them once in + * the API Keys panel, and the legacy keys get wiped here. + */ +export function migrateLegacySentinelBrowserKeys(): { cleared: string[] } { + if (typeof window === 'undefined') return { cleared: [] }; + const legacy = [ + 'sb_sentinel_client_id', + 'sb_sentinel_client_secret', + 'sb_sentinel_instance_id', + ]; + const cleared: string[] = []; + for (const key of legacy) { + try { + if (window.localStorage?.getItem(key) !== null) { + window.localStorage.removeItem(key); + cleared.push(key); + } + } catch { /* ignore quota / privacy mode errors */ } + try { + if (window.sessionStorage?.getItem(key) !== null) { + window.sessionStorage.removeItem(key); + if (!cleared.includes(key)) cleared.push(key); + } + } catch { /* ignore */ } + } + return { cleared }; } // ─── OAuth2 token ────────────────────────────────────────────────────────── @@ -79,14 +139,16 @@ export function hasSentinelCredentials(): boolean { /** * Fetch an OAuth2 access token using the client_credentials grant. * Caches in memory; auto-refreshes 30 s before expiry. + * + * The request body NO LONGER carries client_id/secret — the backend + * resolves credentials from its ``.env`` via the API Keys flow. The + * server-side proxy still accepts body credentials for legacy callers, + * but the dashboard does not supply them. */ export function getSentinelToken(): Promise { // Return cached token if still valid (with 30 s margin) if (cachedToken && Date.now() < tokenExpiry - 30_000) return Promise.resolve(cachedToken); - const { clientId, clientSecret } = getSentinelCredentials(); - if (!clientId || !clientSecret) return Promise.resolve(null); - // Dedup: reuse in-flight request so 20 tiles don't each trigger a token fetch if (_tokenPromise) return _tokenPromise; @@ -94,11 +156,9 @@ export function getSentinelToken(): Promise { try { const resp = await fetch(TOKEN_PROXY_URL, { method: 'POST', + // Backend resolves credentials from env. Empty body = "use server-side". headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams({ - client_id: clientId, - client_secret: clientSecret, - }), + body: new URLSearchParams({}), }); if (!resp.ok) { @@ -131,6 +191,8 @@ const TILE_PROXY_URL = `${API_BASE}/api/sentinel/tile`; /** * Fetch a single 256×256 tile via backend proxy to Sentinel Hub Process API. * Returns a PNG ArrayBuffer or null on failure. + * + * Body no longer carries client_id/secret — the backend uses .env values. */ export async function fetchSentinelTile( z: number, @@ -139,21 +201,10 @@ export async function fetchSentinelTile( preset: string, date: string, ): Promise { - const { clientId, clientSecret } = getSentinelCredentials(); - if (!clientId || !clientSecret) return null; - const resp = await fetch(TILE_PROXY_URL, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - client_id: clientId, - client_secret: clientSecret, - preset, - date, - z, - x, - y, - }), + body: JSON.stringify({ preset, date, z, x, y }), }); if (!resp.ok) return null;