From d973a7ad67900750dd8d6e0128c47b1848225e20 Mon Sep 17 00:00:00 2001 From: elean-latedev Date: Tue, 4 Aug 2026 19:03:58 +0200 Subject: [PATCH] fix(mcp): stop rejecting valid API keys on non-200 upstream statuses verify_late_api_key returned `response.status_code == 200`, so every non-200 collapsed into "invalid token", and `except Exception: return False` swallowed the 5s timeout. verify_token then returned None and fastmcp emitted a 401 invalid_token whose text tells users to clear their credentials and re-register. 402 and 429 prove the key is valid: the Zernio API emits them only after authenticate() has already resolved the credential. Classify the upstream status into VALID / INVALID / UNKNOWN instead of a bool, cache positives only (sha256 keys, 10k LRU cap, 60s fresh / 1h grace on UNKNOWN), and raise AuthenticationError (HTTP 400) when a token cannot be verified rather than reporting it as invalid. An unseen token is still refused during an outage. Aug 1-3 2026: 32,190 x 402, 7,034 x 429 and 878 x 500 false rejections, plus 2,638 x 504 in a single 30-minute window. The 429s were self-inflicted: verifying on every request burned the caller's own rate-limit bucket. --- src/late/mcp/auth.py | 174 ++++++++++++++++++++++++---- tests/test_mcp_auth_verification.py | 168 +++++++++++++++++++++++++++ 2 files changed, 322 insertions(+), 20 deletions(-) create mode 100644 tests/test_mcp_auth_verification.py diff --git a/src/late/mcp/auth.py b/src/late/mcp/auth.py index 59a7f80..685abeb 100644 --- a/src/late/mcp/auth.py +++ b/src/late/mcp/auth.py @@ -1,10 +1,16 @@ """Authentication module for Zernio MCP HTTP server.""" +import hashlib +import logging import os +import time +from collections import OrderedDict +from enum import Enum from urllib.parse import urlparse import httpx from fastmcp.server.auth import AccessToken, RemoteAuthProvider, TokenVerifier +from starlette.authentication import AuthenticationError from starlette.requests import Request from late.mcp.constants import ( @@ -58,28 +64,109 @@ def is_allowed_origin(request: Request) -> bool: return any(host == s or host.endswith("." + s) for s in _allowed_origin_suffixes()) -async def verify_late_api_key(api_key: str) -> bool: +logger = logging.getLogger(__name__) + +_VERIFY_URL = "https://zernio.com/api/v1/accounts" +_VERIFY_TIMEOUT = 5.0 + +# Positive-only verification cache: sha256(token) -> monotonic timestamp of the +# last upstream confirmation. Positives only, so an attacker cannot grow it by +# spraying invalid bearers. LRU-capped anyway: the container is long-lived. +# Per process on purpose: Railway runs one uvicorn worker (Dockerfile CMD, no +# --workers), and a shared store would put a new dependency in front of auth, +# which is how you rebuild this same outage. +_VERIFIED_AT: OrderedDict[str, float] = OrderedDict() +_VERIFIED_CACHE_MAX = 10_000 +# Mirrors the 60s Redis TTL the API itself keeps on apikey:{keyHash} +# (Schedule-Posts-API libs/api-auth.ts:471), so this adds no revocation window +# the platform does not already have. +_FRESH_TTL_SECONDS = 60.0 +# Only used when upstream could not answer. Blast radius is bounded to +# nothing: the MCP stores no data and every tool call re-presents the same +# key to the same API, so a revoked key honoured here still cannot read or +# write anything. +_GRACE_TTL_SECONDS = 3600.0 + + +class Verification(Enum): + """Upstream's verdict on a bearer token. + + Three-valued on purpose: the incident this replaces collapsed every + non-200 into False, so 402, 429, 5xx and timeouts all reached the user as + "your token is invalid, clear it and re-register". """ - Verify Zernio API key by making a test request to Zernio API. - Function name kept as verify_late_api_key for backwards compatibility. + VALID = "valid" + INVALID = "invalid" + UNKNOWN = "unknown" + + +def _classify(status_code: int) -> Verification: + """Map an upstream status to a verdict about the TOKEN, not the request. + + 401/403 are the only statuses that say anything about token validity. + 402 and 429 are emitted only after the API has already resolved the key + to a principal (Schedule-Posts-API libs/api-auth.ts:829 "Valid + credential, but the billing owner is payment-suspended. 402 (not 401) so + integrators can tell 'fix your card' apart from 'bad key'", and + authenticateWithRateLimit at :665 rate-limits only after authenticate() + succeeds), so both PROVE the key is good. Billing and limits are + enforced per call by the endpoint that owns them. Everything else tells + us nothing and must not be reported as an auth failure. + """ + if status_code == 200: + return Verification.VALID + if status_code in (401, 403): + return Verification.INVALID + if status_code in (402, 429): + return Verification.VALID + return Verification.UNKNOWN - Args: - api_key: The Zernio API key to verify. - Returns: - True if API key is valid, False otherwise. +def _remember(token_key: str, now: float) -> None: + _VERIFIED_AT[token_key] = now + _VERIFIED_AT.move_to_end(token_key) + while len(_VERIFIED_AT) > _VERIFIED_CACHE_MAX: + _VERIFIED_AT.popitem(last=False) + + +async def verify_late_api_key( + api_key: str, *, client: httpx.AsyncClient | None = None +) -> Verification: + """Ask the Zernio API whether this bearer is a real credential. + + Function name kept as verify_late_api_key for backwards compatibility. + Returns a three-valued Verification, NOT a bool: never write + ``if await verify_late_api_key(...)``, every Enum member is truthy. + + client is injected by tests (httpx.MockTransport); production opens a + per-call client and must not close an injected one. """ + owns_client = client is None + client = client or httpx.AsyncClient() try: - async with httpx.AsyncClient() as client: - response = await client.get( - "https://zernio.com/api/v1/accounts", - headers={"Authorization": f"Bearer {api_key}"}, - timeout=5.0, - ) - return response.status_code == 200 - except Exception: - return False + response = await client.get( + _VERIFY_URL, + headers={"Authorization": f"Bearer {api_key}"}, + timeout=_VERIFY_TIMEOUT, + ) + except Exception as exc: + # Timeouts, DNS and TLS failures say nothing about the token. Logged + # because the previous silent `return False` is why a 26h auth + # outage ran with a green /health and no alert. + logger.warning("Zernio token verification unreachable: %s", type(exc).__name__) + return Verification.UNKNOWN + finally: + if owns_client: + await client.aclose() + + verdict = _classify(response.status_code) + if verdict is Verification.UNKNOWN: + logger.warning( + "Zernio token verification inconclusive: HTTP %s", + response.status_code, + ) + return verdict class ZernioTokenVerifier(TokenVerifier): @@ -87,14 +174,61 @@ class ZernioTokenVerifier(TokenVerifier): Accepts BOTH plain Zernio API keys and OAuth access tokens: both arrive as the same bearer string and are validated the same way — a live GET to the - Zernio API (verify_late_api_key). HTTP 200 => valid. Deliberately - format-agnostic (no JWT decode), which is why a static API key works here - just as well as an issued OAuth token. + Zernio API (verify_late_api_key). 402 and 429 also count as valid: the + Zernio API only emits them after it has already authenticated the + credential. Deliberately format-agnostic (no JWT decode), which is why a + static API key works here just as well as an issued OAuth token. """ + def __init__(self, *, client: httpx.AsyncClient | None = None) -> None: + super().__init__() + self._client = client + async def verify_token(self, token: str) -> AccessToken | None: - if not await verify_late_api_key(token): + token_key = hashlib.sha256(token.encode()).hexdigest() + now = time.monotonic() + verified_at = _VERIFIED_AT.get(token_key) + + if verified_at is not None and now - verified_at < _FRESH_TTL_SECONDS: + _VERIFIED_AT.move_to_end(token_key) + return self._grant(token) + + verdict = await verify_late_api_key(token, client=self._client) + + if verdict is Verification.VALID: + _remember(token_key, now) + return self._grant(token) + + if verdict is Verification.INVALID: + _VERIFIED_AT.pop(token_key, None) return None + + # UNKNOWN. Honour a token upstream confirmed recently; refuse to + # guess about any other one, so an outage never becomes an auth + # bypass. Raising AuthenticationError yields HTTP 400 with this + # message. Returning None instead would emit the 401 invalid_token + # whose text tells users to clear their tokens and re-register, + # which is the exact damage this fix exists to stop. Any exception + # type OTHER than AuthenticationError escapes as a 500 with the + # message swallowed. + if verified_at is not None and now - verified_at < _GRACE_TTL_SECONDS: + # Guarded because the await above can suspend for the full verify + # timeout, and a concurrent request may LRU-evict this key while + # we are parked. A bare move_to_end would raise KeyError, which is + # not AuthenticationError and so escapes as a 500. + if token_key in _VERIFIED_AT: + _VERIFIED_AT.move_to_end(token_key) + logger.warning( + "Granting %s... from cache: upstream unreachable", + token_key[:12], + ) + return self._grant(token) + raise AuthenticationError( + "Zernio API is temporarily unreachable, so this token could not " + "be verified. Your credentials are fine; retry in a moment." + ) + + def _grant(self, token: str) -> AccessToken: return AccessToken(token=token, client_id="zernio", scopes=list(OAUTH_SCOPES)) diff --git a/tests/test_mcp_auth_verification.py b/tests/test_mcp_auth_verification.py new file mode 100644 index 0000000..939037b --- /dev/null +++ b/tests/test_mcp_auth_verification.py @@ -0,0 +1,168 @@ +""" +Regression tests for the Zernio MCP auth outage: auth.py used to collapse +every non-200 upstream status into "invalid token" (``return +response.status_code == 200``) and swallow timeouts (``except Exception: +return False``). ``verify_token`` then returned ``None`` and fastmcp emitted a +401 ``invalid_token`` telling users to clear their credentials and +re-register, even though the Zernio API only emits 402/429 for a credential +it has already authenticated. + +These tests drive the real ``_classify`` / ``ZernioTokenVerifier`` against a +fake transport (``httpx.MockTransport`` -- no network, no mocking of our own +functions). +""" + +from __future__ import annotations + +import hashlib +import time + +import httpx +import pytest +from starlette.authentication import AuthenticationError + +from late.mcp import auth +from late.mcp.auth import Verification, ZernioTokenVerifier, _classify + + +@pytest.fixture(autouse=True) +def _clear_verification_cache(): + auth._VERIFIED_AT.clear() + yield + auth._VERIFIED_AT.clear() + + +@pytest.mark.parametrize( + "status_code,expected", + [ + (200, Verification.VALID), + (401, Verification.INVALID), + (403, Verification.INVALID), + (402, Verification.VALID), + (429, Verification.VALID), + (500, Verification.UNKNOWN), + (502, Verification.UNKNOWN), + (503, Verification.UNKNOWN), + (504, Verification.UNKNOWN), + (404, Verification.UNKNOWN), + ], +) +def test_classify_maps_upstream_status_to_outcome(status_code, expected): + assert _classify(status_code) is expected + + +async def test_verify_token_accepts_payment_required_key(): + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(402) + + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + verifier = ZernioTokenVerifier(client=client) + + access_token = await verifier.verify_token("some-api-key") + + assert access_token is not None + await client.aclose() + + +async def test_verify_token_raises_unavailable_for_unknown_token_when_upstream_is_down(): + def handler(_request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("boom") + + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + verifier = ZernioTokenVerifier(client=client) + + with pytest.raises(AuthenticationError): + await verifier.verify_token("never-seen-before") + + await client.aclose() + + +async def test_verify_token_honours_recently_valid_token_during_outage(): + def ok_handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(200) + + warm_client = httpx.AsyncClient(transport=httpx.MockTransport(ok_handler)) + verifier = ZernioTokenVerifier(client=warm_client) + token = "a-recently-valid-key" + warm_token = await verifier.verify_token(token) + assert warm_token is not None + await warm_client.aclose() + + # Push the cached timestamp past the fresh window (60s) but still inside + # the grace window (3600s), so the second call below actually exercises + # the UNKNOWN + grace-cache branch instead of short-circuiting on the + # fresh-cache check. + token_key = hashlib.sha256(token.encode()).hexdigest() + auth._VERIFIED_AT[token_key] = time.monotonic() - (auth._FRESH_TTL_SECONDS + 1) + + def down_handler(_request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("boom") + + down_client = httpx.AsyncClient(transport=httpx.MockTransport(down_handler)) + verifier._client = down_client + + access_token = await verifier.verify_token(token) + + assert access_token is not None + await down_client.aclose() + + +async def test_verify_token_rejects_a_genuinely_bad_key(): + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(401) + + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + verifier = ZernioTokenVerifier(client=client) + + assert await verifier.verify_token("a-revoked-key") is None + await client.aclose() + + +async def test_invalid_verdict_evicts_the_cached_entry(): + """A key that upstream later rejects must not survive on the grace path.""" + + def ok_handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(200) + + token = "a-key-that-gets-revoked" + token_key = hashlib.sha256(token.encode()).hexdigest() + + warm_client = httpx.AsyncClient(transport=httpx.MockTransport(ok_handler)) + verifier = ZernioTokenVerifier(client=warm_client) + assert await verifier.verify_token(token) is not None + await warm_client.aclose() + + # Past the fresh window so the next call really re-verifies upstream. + auth._VERIFIED_AT[token_key] = time.monotonic() - (auth._FRESH_TTL_SECONDS + 1) + + def revoked_handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(401) + + revoked_client = httpx.AsyncClient(transport=httpx.MockTransport(revoked_handler)) + verifier._client = revoked_client + assert await verifier.verify_token(token) is None + await revoked_client.aclose() + + assert token_key not in auth._VERIFIED_AT + + # With the entry evicted, an upstream outage must refuse the token rather + # than honour it from the grace cache. + def down_handler(_request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("boom") + + down_client = httpx.AsyncClient(transport=httpx.MockTransport(down_handler)) + verifier._client = down_client + with pytest.raises(AuthenticationError): + await verifier.verify_token(token) + await down_client.aclose() + + +def test_verification_cache_is_bounded(): + now = 0.0 + for i in range(auth._VERIFIED_CACHE_MAX + 50): + auth._remember(f"token-{i}", now) + + assert len(auth._VERIFIED_AT) <= auth._VERIFIED_CACHE_MAX + # Eviction is LRU, so the oldest inserts are the ones that went. + assert "token-0" not in auth._VERIFIED_AT + assert f"token-{auth._VERIFIED_CACHE_MAX + 49}" in auth._VERIFIED_AT