From 124dc2fca01b730b167719ca395093feb911b19b Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 14:02:28 +0100 Subject: [PATCH 1/3] Verify Cloudflare Access assertions at the origin Nothing yet calls this; it is the piece the admin gate needs before require_admin can refuse anyone, landed on its own so the security-critical part can be read without the wiring around it. Verifying rather than trusting the header matters twice here. Once for the reason the node code gives: an Access application deleted or misconfigured leaves the hostname open, and this is the only thing that would notice. And once for a reason particular to this backend, that every vhost proxies to the same app, so an assertion is the only thing distinguishing an administrator from any caller who found api.retina.fm. Gating a hostname cannot do that job. The audience check is the load-bearing one and the easiest to omit, because a token failing it is still perfectly signed: the team runs Access on seventeen node hostnames, and without it any engineer's node session would open the console. It was driven out by a test that genuinely failed first, as were the issuer, expiry, signature and alg=none refusals. Async on httpx rather than the node's blocking requests, since this is awaited from a dependency on the request path and a cold cache would otherwise stall the event loop for the whole 10s timeout. PyJWKSetError is caught explicitly: it descends from PyJWTError but not InvalidTokenError, so the obvious handler lets a malformed key set escape as a 500 instead of failing closed. pyjwt becomes a direct dependency. It was already imported by test_auth.py while arriving only as a transitive of fastapi-users, which pins this exact version, so the pin is forced rather than chosen. Co-Authored-By: Claude Opus 5 --- backend/core/access_identity.py | 368 +++++++++++++++++++ backend/requirements.txt | 4 + backend/tests/test_access_identity.py | 507 ++++++++++++++++++++++++++ 3 files changed, 879 insertions(+) create mode 100644 backend/core/access_identity.py create mode 100644 backend/tests/test_access_identity.py diff --git a/backend/core/access_identity.py b/backend/core/access_identity.py new file mode 100644 index 00000000..d3994ee1 --- /dev/null +++ b/backend/core/access_identity.py @@ -0,0 +1,368 @@ +"""Verifying that a request really was authenticated by Cloudflare Access. + +Cloudflare puts a signed assertion in `Cf-Access-Jwt-Assertion` on every request +it lets through. This turns that into an email address, or into nothing. + +## Why verify at all, when Access sits in front + +In normal operation nothing unauthenticated reaches the admin hostnames. This is +for the case where that stops being true. An Access application deleted, renamed +or misconfigured leaves the hostname open and the backend happily serving it, and +nothing else in the stack would notice. + +It also does work no edge check can: the same app answers on every vhost, so an +assertion is the only thing that distinguishes an administrator from any caller +who found `api.retina.fm`. + +## What is checked, and why each one matters + + signature against the team's published keys. Without it the header is a + claim anyone can type. + audience the tag of *this environment's* Access application. Without it a + token minted for any other application in the team is accepted, + and the team runs Access on seventeen node hostnames. + issuer the team domain, so a valid token from another Cloudflare team + is not enough. + expiry with a little leeway for clock drift. + +Missing any one of those turns verification into decoration. The audience is the +one most easily left out, because a token that fails it still has a perfectly +good signature. +""" + +import asyncio +import logging + +import httpx +import jwt +from jwt import PyJWKSet + +log = logging.getLogger(__name__) + +#: Cloudflare rotates signing keys, so a cached set goes stale. An unrecognised +#: key id refetches immediately regardless, so this is a ceiling on staleness +#: rather than the mechanism that handles rotation. +JWKS_TTL_SECONDS = 3600 + +#: Rejecting a freshly issued token because a clock is three seconds behind +#: would be a confusing way to fail. +CLOCK_LEEWAY_SECONDS = 30 + +#: Floor on refetching after an unrecognised key id. `kid` is read from the +#: unverified header, so anyone can name one without signing anything, and +#: without this floor each such request costs an outbound fetch to Cloudflare +#: made while holding the lock every admin request waits on. The cost is that a +#: genuine rotation is picked up up to this late, which is short beside both the +#: hour-long TTL and Cloudflare's real rotation cadence. +JWKS_REFETCH_MIN_INTERVAL_SECONDS = 60 + +#: How long a held key set keeps being used once refreshing it starts failing. +#: +#: Cloudflare Access at the edge and the endpoint publishing these keys are +#: different systems, so the outage worth designing for is the one where Access +#: admits the team normally and only the refresh fails. Refusing then would take +#: the admin console down on our own account, during someone else's partial +#: outage, using keys we are still holding and which would almost certainly +#: verify every live assertion. +#: +#: Bounded rather than indefinite: long past any plausible outage, short beside +#: Cloudflare's rotation cadence of weeks, so a genuinely retired key can never +#: be honoured for anything approaching a rotation cycle. +JWKS_GRACE_SECONDS = 6 * 3600 + +#: One refresh attempt per interval once one has failed, so a failing endpoint +#: is not retried by every request in turn while holding the lock. +JWKS_RETRY_INTERVAL_SECONDS = 30 + +#: One outage line per interval. The grace window is hours long and the request +#: rate is not ours to set, so a line each would bury what it reports in the +#: logs somebody is reading during the outage. Once a minute is ~360 lines +#: across the whole window: unmissable, and not a flood. +JWKS_WARN_INTERVAL_SECONDS = 60 + +#: Everything a fetch can fail with. httpx covers the network and the status; +#: PyJWKSetError arrives as PyJWTError, and a body that is not JSON as +#: ValueError. Named once so the fallback path and the caller agree on it. +_FETCH_ERRORS = (httpx.HTTPError, jwt.PyJWTError, ValueError, KeyError) + + +class _KeysUnavailable(Exception): + """There is no key set fit to judge a token against. + + Raised rather than returned so it cannot be confused with "this token's key + id is not among the published keys", which is a different fault, with a + different cause and a different fix. Whoever raises this has already said + why, through `_warn_outage`, so `identity` refuses without adding a line. + """ + + +def _short(value): + """Enough of an identifier to match against, not enough to fill a line.""" + value = str(value or "") + return value if len(value) <= 12 else value[:12] + "..." + + +def _kid(token): + """The key id a token claims, for the log only. + + Read without verifying anything, which is safe precisely because the caller + has already decided to refuse: this only ever describes a rejection. + """ + try: + return _short(jwt.get_unverified_header(token).get("kid")) + except Exception: + return "unreadable" + + +def _claim(token, name): + """One unverified claim, for explaining a rejection. Never decides anything.""" + try: + value = jwt.decode(token, options={"verify_signature": False}).get(name) + except Exception: + return "unreadable" + if isinstance(value, list): + return [_short(v) for v in value] + return _short(value) + + +class AccessIdentity: + """Turns a Cloudflare Access assertion into a verified email address.""" + + def __init__(self, team_domain=None, audience=None, ttl=JWKS_TTL_SECONDS, client=None): + self.team_domain = (team_domain or "").strip() + self.audience = (audience or "").strip() + self.ttl = ttl + self._client = client + self._jwks = None + self._fetched_at = 0.0 + # -inf so the first attempt is never mistaken for one inside a backoff + # window, whatever epoch the loop's monotonic clock happens to use. + self._failed_at = float("-inf") + self._last_error = None + self._warned_at = float("-inf") + # Requests are served concurrently; without this two arriving together + # could each fetch and install a key set the other had moved past. + self._lock = asyncio.Lock() + + def is_configured(self): + return bool(self.team_domain and self.audience) + + # ── signing keys ───────────────────────────────────────────── + + async def _fetch_jwks(self): + if self._client is None: + self._client = httpx.AsyncClient() + url = f"https://{self.team_domain}/cdn-cgi/access/certs" + response = await self._client.get(url, timeout=10) + response.raise_for_status() + return PyJWKSet.from_dict(response.json()) + + async def _refresh(self): + self._jwks = await self._fetch_jwks() + self._fetched_at = asyncio.get_running_loop().time() + self._last_error = None + + async def _try_refresh(self, now): + """Refresh, remembering a failure rather than raising it. + + The error is kept so the caller can re-raise it where there is nothing to + fall back on, which is what lets `identity` tell an outage apart from a + forged key id in the log. + """ + try: + await self._refresh() + return True + except _FETCH_ERRORS as exc: + self._failed_at = now + self._last_error = exc + return False + + def _warn_outage(self, now, message, *args): + """One outage line per interval, however many requests arrive. + + Shared by both outage messages rather than one throttle each: they + describe the same failing endpoint, and the transition between them + happens once, so a line arriving up to an interval late costs nothing + against a condition that is already hours old. + """ + if now - self._warned_at < JWKS_WARN_INTERVAL_SECONDS: + return + self._warned_at = now + log.warning(message, *args) + + async def _usable_keys(self, now): + """The key set to judge this request against. + + Refreshed once it has aged past the TTL. While that refresh is failing + the held set keeps being used, for a bounded grace: Access at the edge + and the endpoint publishing these keys are different systems, so + refusing here would take the console down over an outage in the other + one, using keys we are holding that almost certainly still verify every + live assertion. + """ + if self._jwks is not None and now - self._fetched_at <= self.ttl: + return self._jwks + + if now - self._failed_at >= JWKS_RETRY_INTERVAL_SECONDS and await self._try_refresh(now): + return self._jwks + + if self._jwks is None: + self._warn_outage( + now, + "Refusing every Access assertion: could not reach %s for signing keys and none are held: %s", + self.team_domain, + self._last_error, + ) + raise _KeysUnavailable + + age = now - self._fetched_at + if age > self.ttl + JWKS_GRACE_SECONDS: + self._warn_outage( + now, + "Refusing every Access assertion: %s's signing keys are %.1f hours " + "old, past the grace, and could not reach it to refresh them: %s", + self.team_domain, + age / 3600, + self._last_error, + ) + raise _KeysUnavailable + + self._warn_outage( + now, + "Verifying against signing keys fetched %.0f minutes ago: refreshing " + "them from %s is failing. This holds until they are %.0f hours old, " + "after which every assertion is refused.", + age / 60, + self.team_domain, + (self.ttl + JWKS_GRACE_SECONDS) / 3600, + ) + return self._jwks + + def _lookup(self, kid): + try: + return self._jwks[kid] + except KeyError: + return None + + async def _signing_key(self, token): + """The key this token was signed with, refetching once if it is new. + + An unrecognised key id means either a rotation we have not seen or a + forgery, and asking Cloudflare again tells them apart. Asking is floored + rather than done per request, because the key id comes from the + unverified header and the fetch happens while holding the lock every + other request waits on. + """ + kid = jwt.get_unverified_header(token).get("kid") + if not kid: + return None + + async with self._lock: + now = asyncio.get_running_loop().time() + keys = await self._usable_keys(now) + + try: + return keys[kid] + except KeyError: + pass + + # Only worth asking again if what we hold is old enough that + # Cloudflare could plausibly have published something since. Fresh + # means the answer would be the set we just read. + if now - self._fetched_at < JWKS_REFETCH_MIN_INTERVAL_SECONDS: + return None + if now - self._failed_at < JWKS_RETRY_INTERVAL_SECONDS: + return None + if not await self._try_refresh(now): + return None + return self._lookup(kid) + + # ── the answer ─────────────────────────────────────────────── + + async def identity(self, token): + """The verified email address, or None. + + None covers every way this can fail: no configuration, no token, a bad + signature, the wrong audience, the wrong team, an expired assertion, or + Cloudflare being unreachable. The caller cannot act differently on any + of them, and distinguishing them in a return value would invite somebody + to treat one as good enough. + + The *log* does distinguish them, because operationally they could not be + more different: a wrong audience is a misconfiguration nobody spots from + outside, and an unreachable Cloudflare is an outage. + + The token is never logged. It is a bearer credential for its session, and + a log quoting it hands that session to anyone who can read logs. + """ + if not token: + return None + + if not self.is_configured(): + log.warning( + "Refusing an Access assertion: CF_ACCESS_TEAM_DOMAIN or " + "CF_ACCESS_AUD is unset, so nothing can be verified against. " + "Every admin request will be refused until they are set." + ) + return None + + try: + key = await self._signing_key(token) + if key is None: + log.warning( + "Refusing an Access assertion: signed with key id %s, which is not one of %s's published keys.", + _kid(token), + self.team_domain, + ) + return None + claims = jwt.decode( + token, + key.key, + algorithms=["RS256"], + audience=self.audience, + issuer=f"https://{self.team_domain}", + leeway=CLOCK_LEEWAY_SECONDS, + options={"require": ["exp", "aud", "iss"]}, + ) + except _KeysUnavailable: + # Reported where it was found, at a rate that does not follow the + # rate requests arrive at. Adding a line here would put the flood + # back, and naming the key id would blame the caller for an outage. + return None + except jwt.InvalidAudienceError: + # What a token minted for one of the team's node applications looks + # like. The fix is configuration, not anything the caller did. + log.warning( + "Refusing an Access assertion: it names audience %r, but this " + "environment's application is %s. Either it was issued for a " + "different application, or CF_ACCESS_AUD is stale.", + _claim(token, "aud"), + _short(self.audience), + ) + return None + # PyJWTError rather than InvalidTokenError: PyJWT's error surface is not + # ours to bound, and anything it raises outside that subtree would be a + # 500 on a request that should simply be refused. + except jwt.PyJWTError as exc: + log.warning("Refusing an Access assertion: %s: %s", type(exc).__name__, exc) + return None + except (ValueError, KeyError) as exc: + log.warning("Refusing an Access assertion: malformed: %s: %s", type(exc).__name__, exc) + return None + + # Cloudflare puts the address in `email`. A token that verifies but names + # nobody is not an identity, and something truthy would let a caller + # believe it had authenticated a person. + # + # Lowercased here, at the one place an identity is produced, because the + # claim arrives as the identity provider spelled it and callers derive a + # stable id from it. ADMIN_EMAILS and get_or_create_oauth_user normalise + # the same way, so an Access identity and an OAuth one for one address + # compare equal. + email = (claims.get("email") or "").strip().lower() + if not email: + log.warning( + "Refusing an Access assertion: it verifies, but carries no email claim, so it identifies nobody." + ) + return None + return email diff --git a/backend/requirements.txt b/backend/requirements.txt index c2ad1969..1cc1c145 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -9,6 +9,10 @@ pyyaml==6.0.3 cryptography==46.0.6 boto3==1.42.89 fastapi-users[sqlalchemy]==13.0.0 +# Direct since core/access_identity.py imports it to verify Cloudflare Access +# assertions; it had arrived only as a transitive of fastapi-users, which pins +# this exact version, so it is not a free choice. [crypto] for RS256. +pyjwt[crypto]==2.8.0 aiosqlite==0.20.0 alembic==1.13.2 pyarrow==16.1.0 diff --git a/backend/tests/test_access_identity.py b/backend/tests/test_access_identity.py new file mode 100644 index 00000000..751984cb --- /dev/null +++ b/backend/tests/test_access_identity.py @@ -0,0 +1,507 @@ +"""Tests for verifying Cloudflare Access assertions. + +Uses a real RSA keypair and real signed tokens rather than mocking the +verification, because the failures that matter here are the ones where a token +verifies and should not have. A mocked verifier would pass all of them. +""" + +import json +import logging +import time + +import httpx +import jwt +import pytest +from cryptography.hazmat.primitives.asymmetric import rsa + +from core.access_identity import ( + JWKS_GRACE_SECONDS, + JWKS_REFETCH_MIN_INTERVAL_SECONDS, + JWKS_TTL_SECONDS, + AccessIdentity, +) + +TEAM = "offworldlab.cloudflareaccess.com" +ISSUER = f"https://{TEAM}" +AUD = "e5ff9de8d1ca5fbc62b38d102d92a1fc7d910d5f89ef388caf63c83e828493b3" +OTHER_AUD = "0" * 64 +EMAIL = "someone@offworldlab.com" + + +@pytest.fixture(scope="module") +def keys(): + """One keypair for the suite. Generating RSA is slow enough to matter.""" + private = rsa.generate_private_key(public_exponent=65537, key_size=2048) + return private, private.public_key() + + +def _jwk(public_key, kid): + data = json.loads(jwt.algorithms.RSAAlgorithm.to_jwk(public_key)) + data.update({"kid": kid, "alg": "RS256", "use": "sig"}) + return data + + +class FakeJWKS: + """A real httpx client over a fake transport, counting fetches. + + Real client so the production code's own request path is exercised; the + counter is what lets the caching and single-refetch behaviour be asserted. + """ + + def __init__(self, jwks, fail=False): + self.jwks = jwks + self.fail = fail + self.calls = 0 + + def client(self): + def handler(request): + self.calls += 1 + if self.fail: + raise httpx.ConnectError("unreachable") + return httpx.Response(200, json=self.jwks) + + return httpx.AsyncClient(transport=httpx.MockTransport(handler)) + + +def token(keys, *, aud=AUD, iss=ISSUER, email=EMAIL, kid="kid-1", exp_delta=300, key=None, **extra): + private, _ = keys + claims = { + "aud": aud, + "iss": iss, + "email": email, + "exp": int(time.time()) + exp_delta, + "iat": int(time.time()) - 10, + **extra, + } + return jwt.encode(claims, key or private, algorithm="RS256", headers={"kid": kid}) + + +@pytest.fixture +def verifier(keys): + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + v.fake = fake + return v + + +async def test_a_valid_assertion_yields_the_email(verifier, keys): + assert await verifier.identity(token(keys)) == EMAIL + + +async def test_the_key_set_is_cached(verifier, keys): + for _ in range(5): + await verifier.identity(token(keys)) + assert verifier.fake.calls == 1 + + +# ── the refusals that matter ───────────────────────────────────── + + +async def test_a_token_for_another_application_is_refused(verifier, keys): + """The one most easily left out, because such a token is perfectly signed. + + The team already runs Access on seventeen node hostnames, so without the + audience check anyone holding a valid session for those is admitted here. + """ + assert await verifier.identity(token(keys, aud=OTHER_AUD)) is None + + +async def test_a_token_from_another_team_is_refused(verifier, keys): + other = "https://someone-else.cloudflareaccess.com" + assert await verifier.identity(token(keys, iss=other)) is None + + +async def test_an_expired_assertion_is_refused(verifier, keys): + assert await verifier.identity(token(keys, exp_delta=-600)) is None + + +async def test_a_token_signed_by_someone_else_is_refused(verifier, keys): + """Right shape, right claims, wrong key. The signature is the whole point.""" + impostor = rsa.generate_private_key(public_exponent=65537, key_size=2048) + assert await verifier.identity(token(keys, key=impostor)) is None + + +async def test_an_unsigned_token_is_refused(verifier): + """alg=none walks past a verifier that trusts the header's own claim.""" + forged = jwt.encode( + {"aud": AUD, "iss": ISSUER, "email": EMAIL, "exp": int(time.time()) + 300}, + key="", + algorithm="none", + ) + assert await verifier.identity(forged) is None + + +async def test_the_email_is_normalised(verifier, keys): + """Cloudflare returns the claim as the identity provider supplied it, and + the id derived from it must be the same person's every time. Everything else + that handles an address here lowercases it first (ADMIN_EMAILS, + get_or_create_oauth_user), so this has to as well or the two never match.""" + assert await verifier.identity(token(keys, email=" Someone@OffworldLab.COM ")) == EMAIL + + +async def test_a_token_naming_nobody_is_refused(verifier, keys): + """Verifies, but authenticates no one. Something truthy would let a caller + believe it had identified a person.""" + assert await verifier.identity(token(keys, email="")) is None + + +async def test_a_token_with_no_audience_claim_is_refused(verifier, keys): + private, _ = keys + claims = {"iss": ISSUER, "email": EMAIL, "exp": int(time.time()) + 300} + naked = jwt.encode(claims, private, algorithm="RS256", headers={"kid": "kid-1"}) + assert await verifier.identity(naked) is None + + +async def test_rubbish_is_refused(verifier): + for value in ("", None, "not-a-token", "a.b.c"): + assert await verifier.identity(value) is None + + +# ── configuration ──────────────────────────────────────────────── + + +async def test_no_config_means_no_identity(keys): + """Fails closed. Before CF_ACCESS_AUD is set there is nothing to check + against, and admitting anyone meanwhile would be the worst default.""" + v = AccessIdentity(team_domain="", audience="") + assert v.is_configured() is False + assert await v.identity(token(keys)) is None + + +async def test_a_partial_config_is_treated_as_absent(keys): + v = AccessIdentity(team_domain=TEAM, audience="") + assert v.is_configured() is False + assert await v.identity(token(keys)) is None + + +# ── key rotation and outages ───────────────────────────────────── + + +async def test_an_unknown_key_id_triggers_one_refetch(keys): + """A key id we have not seen is either a rotation or a forgery, and one + refetch tells them apart. + + The refetch has a floor on it (see test_forged_kids_do_not_fetch_once_per_ + request), so the clock is wound back to put this rotation outside that + window. Winding it back rather than sleeping keeps the test instant, and the + floor is short next to both the hour-long TTL and Cloudflare's real rotation + cadence, so a genuine rotation is never delayed by more than the window. + """ + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "old-kid")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys, kid="old-kid")) == EMAIL + assert fake.calls == 1 + + fake.jwks = {"keys": [_jwk(public, "new-kid")]} + v._fetched_at -= JWKS_REFETCH_MIN_INTERVAL_SECONDS + 1 + assert await v.identity(token(keys, kid="new-kid")) == EMAIL + assert fake.calls == 2 + + +async def test_a_rotation_inside_the_floor_waits_for_it(keys): + """The cost of the floor, stated rather than discovered. + + A new key published seconds ago is not picked up until the window passes. + That is bounded and much shorter than the TTL that would otherwise govern. + """ + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "old-kid")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys, kid="old-kid")) == EMAIL + fake.jwks = {"keys": [_jwk(public, "new-kid")]} + assert await v.identity(token(keys, kid="new-kid")) is None + assert fake.calls == 1 + + v._fetched_at -= JWKS_REFETCH_MIN_INTERVAL_SECONDS + 1 + assert await v.identity(token(keys, kid="new-kid")) == EMAIL + + +async def test_a_kid_that_never_appears_is_refused_and_does_not_loop(keys): + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "real-kid")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys, kid="invented")) is None + assert fake.calls <= 2, "must not refetch endlessly for a forged kid" + + +async def test_one_unknown_kid_costs_one_fetch_not_two(keys): + """A cold cache fetches, misses, and must not immediately fetch the same set + again: the second attempt cannot succeed where the first just failed.""" + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "real-kid")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys, kid="invented")) is None + assert fake.calls == 1, "fetched the key set twice for one lookup" + + +async def test_forged_kids_do_not_fetch_once_per_request(keys): + """The reason there is a floor on refetching at all. + + `kid` is read from the unverified header, so anyone can name one without + signing anything. One outbound fetch per request would let an unauthenticated + caller drive a request to Cloudflare per request of their own, each holding + the lock that every admin request waits on. + """ + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "real-kid")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + for i in range(20): + assert await v.identity(token(keys, kid=f"forged-{i}")) is None + + assert fake.calls <= 2, f"{fake.calls} fetches for 20 forged assertions" + + +async def test_cloudflare_being_unreachable_refuses_rather_than_admits(keys): + """With nothing in hand there is nothing to fall back on.""" + fake = FakeJWKS({}, fail=True) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + assert await v.identity(token(keys)) is None + + +# ── the grace period ───────────────────────────────────────────── +# +# Access at the edge and the endpoint publishing these keys are different +# systems. The outage to design for is the one where Access admits the team +# normally and only the refresh fails; refusing then is an outage of our own +# making, using keys we still hold. + + +async def _aged(v, seconds): + """Wind the held set's age forward without waiting for it.""" + v._fetched_at -= seconds + + +async def test_a_stale_set_still_verifies_while_refreshing_fails(keys): + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys)) == EMAIL + fake.fail = True + await _aged(v, JWKS_TTL_SECONDS + 60) + + assert await v.identity(token(keys)) == EMAIL, "refused on keys it was holding" + + +async def test_a_failed_refresh_is_not_retried_by_every_request(keys): + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys)) == EMAIL + fake.fail = True + await _aged(v, JWKS_TTL_SECONDS + 60) + + before = fake.calls + for _ in range(20): + await v.identity(token(keys)) + assert fake.calls - before <= 1, ( + f"{fake.calls - before} attempts for 20 requests; a failing endpoint " + "must not be retried by each one in turn while holding the lock" + ) + + +async def test_past_the_grace_the_stale_set_is_refused(keys): + """The bound. Long past any plausible outage, short beside rotation.""" + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + assert await v.identity(token(keys)) == EMAIL + fake.fail = True + await _aged(v, JWKS_TTL_SECONDS + JWKS_GRACE_SECONDS + 60) + + assert await v.identity(token(keys)) is None + + +async def test_a_cold_start_outage_is_not_logged_per_request(keys, caplog): + """The other outage path, and the worse one: nothing was ever fetched, so + nothing verifies. It needs the same throttle as the grace path, which it did + not get when that one was added.""" + fake = FakeJWKS({}, fail=True) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + caplog.set_level(logging.WARNING) + for _ in range(50): + assert await v.identity(token(keys)) is None + + assert caplog.records, "went quiet entirely" + assert len(caplog.records) <= 2, f"{len(caplog.records)} warnings for 50 requests" + + +@pytest.mark.parametrize("aged_by", [0, JWKS_TTL_SECONDS + JWKS_GRACE_SECONDS + 60]) +async def test_an_outage_is_never_reported_as_an_unknown_key_id(keys, caplog, aged_by): + """Attribution, on both refusing paths. + + "not one of the published keys" names a forged or rotated key id, which is a + different fault with a different fix. Saying it during an outage sends + whoever is reading the log hunting an attacker while Cloudflare is down. + """ + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}, fail=(aged_by == 0)) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + if aged_by: + assert await v.identity(token(keys)) == EMAIL + fake.fail = True + await _aged(v, aged_by) + + caplog.set_level(logging.WARNING) + assert await v.identity(token(keys)) is None + + logged = " ".join(r.getMessage() for r in caplog.records) + assert "published keys" not in logged, f"blamed the key id for an outage: {logged}" + + +async def test_the_stale_warning_is_throttled(keys, caplog): + """Loud once, not once per request. + + The window is hours long and the request rate is not ours to set: the header + can be sent by anyone without signing anything, and nginx allows 30 a second + per address. A line each would bury the very thing it reports, during an + outage, in the logs somebody is reading to find out what broke. + """ + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + await v.identity(token(keys)) + fake.fail = True + await _aged(v, JWKS_TTL_SECONDS + 60) + + caplog.set_level(logging.WARNING) + for _ in range(50): + assert await v.identity(token(keys)) == EMAIL + + assert caplog.records, "went quiet entirely" + assert len(caplog.records) <= 2, f"{len(caplog.records)} warnings for 50 requests" + + +async def test_serving_a_stale_set_is_not_silent(keys, caplog): + """An outage being survived should still be visible, or it is discovered + only when the grace runs out and everybody is locked out at once.""" + _, public = keys + fake = FakeJWKS({"keys": [_jwk(public, "kid-1")]}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + + await v.identity(token(keys)) + fake.fail = True + await _aged(v, JWKS_TTL_SECONDS + 60) + + caplog.set_level(logging.WARNING) + assert await v.identity(token(keys)) == EMAIL + assert caplog.records, "served a stale key set without saying so" + + +async def test_a_malformed_key_set_refuses_rather_than_raising(keys): + """A key set that parses as JSON but carries no usable key. + + PyJWKSet.from_dict raises PyJWKSetError for it, which is a PyJWTError but + not an InvalidTokenError, so it is caught where the fetch happens rather + than by anything reading the token. + """ + fake = FakeJWKS({"keys": []}) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + assert await v.identity(token(keys)) is None + + +# ── clock drift ────────────────────────────────────────────────── + + +async def test_small_clock_drift_is_tolerated(verifier, keys): + assert await verifier.identity(token(keys, exp_delta=-5)) == EMAIL + + +async def test_large_drift_is_not_tolerated(verifier, keys): + assert await verifier.identity(token(keys, exp_delta=-120)) is None + + +# ── explaining refusals ────────────────────────────────────────── +# +# Every refusal returns a bare None, which is right for the caller and useless +# for whoever has to work out why the console stopped admitting anyone. These +# assert the log makes up the difference, without handing out the credential it +# is describing. + + +async def test_every_refusal_says_why(verifier, keys, caplog): + caplog.set_level(logging.WARNING) + cases = { + "wrong audience": token(keys, aud=OTHER_AUD), + "wrong team": token(keys, iss="https://someone-else.cloudflareaccess.com"), + "expired": token(keys, exp_delta=-600), + "no email": token(keys, email=""), + "unknown kid": token(keys, kid="never-published"), + } + for label, bad in cases.items(): + caplog.clear() + assert await verifier.identity(bad) is None, label + assert caplog.records, f"{label} was refused silently" + + +async def test_the_token_is_never_logged(verifier, keys, caplog): + """A refused assertion is still a live bearer credential for its session. + A log quoting it hands that session to anyone who can read logs, and these + lines exist to be read by people debugging.""" + caplog.set_level(logging.DEBUG) + for bad in ( + token(keys, aud=OTHER_AUD), + token(keys, exp_delta=-600), + token(keys, kid="never-published"), + ): + caplog.clear() + await verifier.identity(bad) + logged = " ".join(r.getMessage() for r in caplog.records) + assert bad not in logged + # Not even a substantial slice. Signatures are the long tail. + assert bad.split(".")[2][:24] not in logged + + +async def test_a_wrong_audience_names_both(verifier, keys, caplog): + """The failure a misconfiguration actually produces, and the one invisible + from outside: the hostname is up and refuses everybody.""" + caplog.set_level(logging.WARNING) + await verifier.identity(token(keys, aud=OTHER_AUD)) + logged = " ".join(r.getMessage() for r in caplog.records) + assert OTHER_AUD[:12] in logged, "does not say what the token claimed" + assert AUD[:12] in logged, "does not say what this environment expects" + + +async def test_unreachable_cloudflare_is_distinguishable_from_a_bad_token(keys, caplog): + """An outage and a forgery both return None. Confusing the two sends + somebody hunting an attacker during a network problem.""" + caplog.set_level(logging.WARNING) + fake = FakeJWKS({}, fail=True) + v = AccessIdentity(team_domain=TEAM, audience=AUD, client=fake.client()) + assert await v.identity(token(keys)) is None + logged = " ".join(r.getMessage() for r in caplog.records).lower() + assert "could not reach" in logged + + +async def test_missing_config_names_the_variables(keys, caplog): + caplog.set_level(logging.WARNING) + v = AccessIdentity(team_domain="", audience="") + assert await v.identity(token(keys)) is None + logged = " ".join(r.getMessage() for r in caplog.records) + assert "CF_ACCESS_AUD" in logged + + +async def test_no_token_at_all_is_not_logged(verifier, caplog): + """Unauthenticated requests are ordinary on the ungated vhosts. Logging each + one would bury the refusals that mean something.""" + caplog.set_level(logging.WARNING) + assert await verifier.identity(None) is None + assert await verifier.identity("") is None + assert not caplog.records + + +async def test_a_success_is_not_logged_as_a_refusal(verifier, keys, caplog): + caplog.set_level(logging.WARNING) + assert await verifier.identity(token(keys)) == EMAIL + assert not caplog.records From cf23ce2c7f9614e191c45fbb22fd106cabc3b644 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 16:11:46 +0100 Subject: [PATCH 2/3] Accept a verified Access assertion as identity The verifier landed with nothing calling it. This is the seam: get_current_user and require_admin try a verified assertion before anything else, and otherwise behave exactly as they did. A verified assertion is sufficient on its own, deliberately not also checked against AUTH_ADMIN_EMAILS. The Access policy is already the membership list, and a second list means adding a colleague takes two edits in two systems that will drift apart; the audience pin is what makes that safe, since a token minted for any of the seventeen node applications fails it. Access is tried ahead of the bypass rather than after. Where both are available the real person is the better answer: these are the endpoints that retire hardware, and "Admin (no auth)" is not an attribution. The id is derived from the email with uuid5 rather than allocated, so the same person is the same id across requests and restarts without a row having to exist and admin events stay attributable. An unconfigured verifier is never consulted, so the header is not even read where CF_ACCESS_AUD is unset. Every environment that has not been given an audience, this test suite included, behaves precisely as before. /api/auth/me stops short-circuiting on its own copy of AUTH_BYPASS and delegates instead. It was the one place that would have disagreed with the dependencies, reporting "Admin (no auth)" on the same request that require_admin attributed to a real person, and the dashboard asks it who it is talking to. auth_enabled now derives from whether the identity actually is the anonymous one, so every existing case reports what it always did. Co-Authored-By: Claude Opus 5 --- backend/core/users.py | 53 ++++++++++ backend/routes/auth.py | 10 +- backend/tests/test_access_seam.py | 155 ++++++++++++++++++++++++++++++ backend/tests/test_auth_routes.py | 21 ++++ 4 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_access_seam.py diff --git a/backend/core/users.py b/backend/core/users.py index 99ee40a1..e2759c85 100644 --- a/backend/core/users.py +++ b/backend/core/users.py @@ -21,6 +21,7 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column +from core.access_identity import AccessIdentity from core.env_parsing import parse_comma_list # ── Config ──────────────────────────────────────────────────────────────────── @@ -65,6 +66,19 @@ def _derive_auth_flags(env: Mapping[str, str]) -> tuple[bool, bool]: AUTH_ENABLED, AUTH_BYPASS = _derive_auth_flags(os.environ) +#: The header Cloudflare sets on every request its Access applications admit. +ACCESS_ASSERTION_HEADER = "Cf-Access-Jwt-Assertion" + +#: One instance, so the JWKS cache and its lock are shared across requests. +#: The audience differs per environment and comes from the compose overlays; the +#: team domain is the same everywhere and comes from the base compose file. +#: Either unset means unconfigured, and an unconfigured verifier is never +#: consulted rather than refusing assertions nobody sent. +access_identity = AccessIdentity( + team_domain=os.getenv("CF_ACCESS_TEAM_DOMAIN", ""), + audience=os.getenv("CF_ACCESS_AUD", ""), +) + _DATA_DIR = Path(__file__).resolve().parent.parent / "data" _DATA_DIR.mkdir(parents=True, exist_ok=True) # RETINA_DB_PATH exists so tests and one-off migrations can point at a scratch @@ -312,8 +326,42 @@ async def _read_user_from_request(request: Request) -> User | None: return user +def _access_user_dict(email: str) -> dict: + """A user dict for a verified Access identity, with no database row. + + Membership of the Access group is what grants the console, so anyone whose + assertion verifies for this environment's audience is an administrator; a + second list in AUTH_ADMIN_EMAILS would only be one more thing to drift. + + The id is derived from the email rather than allocated, so the same person + is the same id across requests and restarts and the destructive endpoints + stay attributable in /api/admin/events. + """ + return { + "id": str(uuid.uuid5(uuid.NAMESPACE_URL, f"mailto:{email}")), + "email": email, + "name": email.split("@")[0], + "avatar": "", + "provider": "cloudflare-access", + "role": "admin", + "is_superuser": True, + "created_at": 0, + } + + +async def _access_user_from_request(request: Request) -> dict | None: + """The verified Access identity for this request, or None.""" + if not access_identity.is_configured(): + return None + email = await access_identity.identity(request.headers.get(ACCESS_ASSERTION_HEADER)) + return _access_user_dict(email) if email else None + + async def get_current_user(request: Request) -> dict: """Return user dict or raise 401. Returns anonymous admin where AUTH_BYPASS is opted into.""" + access = await _access_user_from_request(request) + if access is not None: + return access if AUTH_BYPASS: return dict(ANONYMOUS_USER) user = await _read_user_from_request(request) @@ -324,6 +372,11 @@ async def get_current_user(request: Request) -> dict: async def require_admin(request: Request) -> dict: """Like get_current_user but also enforces superuser/admin role.""" + # Ahead of the bypass: where both are available the real person is the better + # answer, since "Admin (no auth)" is not an attribution. + access = await _access_user_from_request(request) + if access is not None: + return access if AUTH_BYPASS: return dict(ANONYMOUS_USER) user = await _read_user_from_request(request) diff --git a/backend/routes/auth.py b/backend/routes/auth.py index b69cb078..b5c7e7c7 100644 --- a/backend/routes/auth.py +++ b/backend/routes/auth.py @@ -27,7 +27,6 @@ ) from core.users import ( ANONYMOUS_USER, - AUTH_BYPASS, JWT_LIFETIME_SECONDS, JWT_SECRET, get_current_user, @@ -231,10 +230,13 @@ async def callback_github(request: Request, code: str = "", state: str = ""): @router.get("/me") async def me(request: Request): - if AUTH_BYPASS: - return {**ANONYMOUS_USER, "auth_enabled": False} user_dict = await get_current_user(request) - return {**user_dict, "auth_enabled": True} + # Delegated rather than short-circuiting on AUTH_BYPASS, so this agrees with + # what require_admin decided: a verified Access assertion outranks the + # bypass, and answering "Admin (no auth)" while the admin routes attribute a + # real person would make the console wrong about its own session. + anonymous = user_dict["id"] == ANONYMOUS_USER["id"] + return {**user_dict, "auth_enabled": not anonymous} @router.post("/logout") diff --git a/backend/tests/test_access_seam.py b/backend/tests/test_access_seam.py new file mode 100644 index 00000000..421e36f0 --- /dev/null +++ b/backend/tests/test_access_seam.py @@ -0,0 +1,155 @@ +"""The Access assertion as a third source of identity for the auth dependencies. + +The verifier itself is tested in test_access_identity.py against real signed +tokens. These cover the wiring: that a verified email becomes an admin, that an +unverified one becomes a 401, and that an unconfigured verifier changes nothing. +""" + +import asyncio +import uuid +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import HTTPException +from starlette.datastructures import State + +from core.users import get_current_user, require_admin + +EMAIL = "someone@offworldlab.com" + + +class StubVerifier: + """Stands in for AccessIdentity, recording what it was asked to verify.""" + + def __init__(self, email=None, configured=True): + self.email = email + self.configured = configured + self.seen = [] + + def is_configured(self): + return self.configured + + async def identity(self, token): + self.seen.append(token) + # Same contract as the real verifier: no token is no identity, never an + # error. A stub that answered regardless would hide a seam that admitted + # callers who sent no assertion at all. + return self.email if token else None + + +def _request(header=None): + request = MagicMock() + request.headers = {"Cf-Access-Jwt-Assertion": header} if header else {} + request.cookies = {} + request.state = State() + return request + + +# ── a verified assertion is an administrator ───────────────────── + + +def test_a_verified_assertion_makes_require_admin_an_admin(): + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + user = asyncio.run(require_admin(_request("a-token"))) + assert user["email"] == EMAIL + assert user["is_superuser"] is True + assert user["role"] == "admin" + + +def test_the_assertion_is_what_gets_verified(): + """Guards against reading the wrong header, which would silently never + authenticate anyone.""" + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + asyncio.run(require_admin(_request("the-assertion"))) + assert verifier.seen == ["the-assertion"] + + +def test_a_verified_assertion_also_satisfies_get_current_user(): + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + user = asyncio.run(get_current_user(_request("a-token"))) + assert user["email"] == EMAIL + + +def test_the_identity_carries_a_stable_id_derived_from_the_email(): + """Admin actions are attributed in /api/admin/events, so the same person + must be the same id across requests and restarts, without a database row.""" + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + first = asyncio.run(require_admin(_request("t1"))) + second = asyncio.run(require_admin(_request("t2"))) + assert first["id"] == second["id"] + assert first["id"] == str(uuid.uuid5(uuid.NAMESPACE_URL, f"mailto:{EMAIL}")) + assert first["id"] != "00000000-0000-0000-0000-000000000000" + + +def test_the_provider_says_where_the_identity_came_from(): + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + user = asyncio.run(require_admin(_request("a-token"))) + assert user["provider"] == "cloudflare-access" + + +# ── refusals ───────────────────────────────────────────────────── + + +def test_an_assertion_that_does_not_verify_is_refused(): + verifier = StubVerifier(email=None) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + with pytest.raises(HTTPException) as exc: + asyncio.run(require_admin(_request("forged"))) + assert exc.value.status_code == 401 + + +def test_the_plain_email_header_is_not_trusted(): + """Cloudflare also sets CF-Access-Authenticated-User-Email, which is an + unsigned header anyone can type. Only the signed assertion is identity.""" + verifier = StubVerifier(email=EMAIL) + request = _request() + request.headers = {"Cf-Access-Authenticated-User-Email": "attacker@example.com"} + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + with pytest.raises(HTTPException) as exc: + asyncio.run(require_admin(request)) + assert exc.value.status_code == 401 + + +def test_no_assertion_and_no_bypass_is_refused(): + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", False), patch("core.users.access_identity", verifier): + with pytest.raises(HTTPException) as exc: + asyncio.run(require_admin(_request())) + assert exc.value.status_code == 401 + + +# ── the bypass, and not consulting an unconfigured verifier ────── + + +def test_an_unconfigured_verifier_is_never_consulted(): + """Every environment without CF_ACCESS_AUD set, including the test suite. + Reading the header there would refuse tokens nobody sent and log for it.""" + verifier = StubVerifier(email=EMAIL, configured=False) + with patch("core.users.AUTH_BYPASS", True), patch("core.users.access_identity", verifier): + user = asyncio.run(require_admin(_request("a-token"))) + assert verifier.seen == [] + assert user["id"] == "00000000-0000-0000-0000-000000000000" + + +def test_the_bypass_still_works_where_it_is_opted_into(): + """A laptop has no Access assertion, and docker-compose.local.yml keeps the + flag for that reason.""" + verifier = StubVerifier(email=None, configured=False) + with patch("core.users.AUTH_BYPASS", True), patch("core.users.access_identity", verifier): + user = asyncio.run(require_admin(_request())) + assert user["role"] == "admin" + + +def test_a_verified_assertion_beats_the_bypass(): + """Where both are available the real person is the better answer: the + destructive admin endpoints are attributed, and 'Admin (no auth)' is not an + attribution.""" + verifier = StubVerifier(email=EMAIL) + with patch("core.users.AUTH_BYPASS", True), patch("core.users.access_identity", verifier): + user = asyncio.run(require_admin(_request("a-token"))) + assert user["email"] == EMAIL diff --git a/backend/tests/test_auth_routes.py b/backend/tests/test_auth_routes.py index efa89f1a..cfccf73a 100644 --- a/backend/tests/test_auth_routes.py +++ b/backend/tests/test_auth_routes.py @@ -37,6 +37,27 @@ def test_me_returns_anonymous_admin_in_test_mode(self, client): assert body["role"] == "admin" assert body["auth_enabled"] is False + def test_me_reports_the_access_identity_rather_than_the_anonymous_admin(self, client): + """The dashboard asks /me who it is talking to, so this must agree with + what require_admin would decide. Answering "Admin (no auth)" while the + admin routes are attributing a real person is the inconsistency that + makes the console show the wrong thing about its own session.""" + from unittest.mock import patch + + class Stub: + def is_configured(self): + return True + + async def identity(self, token): + return "someone@offworldlab.com" if token else None + + with patch("core.users.access_identity", Stub()): + r = client.get("/api/auth/me", headers={"Cf-Access-Jwt-Assertion": "a-token"}) + assert r.status_code == 200 + body = r.json() + assert body["email"] == "someone@offworldlab.com" + assert body["auth_enabled"] is True + def test_logout_returns_ok(self, client): r = client.post("/api/auth/logout") assert r.status_code == 200 From 840dc6a17af989e82b6f7ce6ded6f7542c82f5be Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 16:21:58 +0100 Subject: [PATCH 3/3] Configure the Access verifier per environment The seam reads two values and nothing was supplying them. This wires the team domain and the audience through compose, and teaches the parity checker that one of them is meant to differ. The team domain goes in the base file rather than being repeated per overlay: there is one Zero Trust team for the org, and a per-environment copy could drift to a different one without the audience check noticing, since that check only proves a token was minted for this application, not by whom. The audience is genuinely per environment, one tag per Access application, which is exactly the shape HOST_ADMIN already has. So it gets the same treatment: an ALLOWED_DIVERGENCE entry, because without one the second environment to be given a tag fails the parity check and no deploy passes. Confirmed load-bearing by removing it, whereupon the check fails naming the key. Only the test environment carries a tag, because only test-admin.retina.fm has an application so far. Staging and production are deliberately still unset, which leaves the verifier unconfigured there and therefore never consulted: those applications gate their hostnames the moment they exist, and staging's is probed by CI, so they wait for the smoke-test changes. The AUTH_ALLOW_ANONYMOUS_ADMIN exclusion gains a test rather than an edit. It keeps working after the flag is dropped from all three overlays, since it is what makes CI refuse a change that reintroduces the anonymous admin to one environment on its own, and it should not quietly stop covering that. Co-Authored-By: Claude Opus 5 --- backend/tests/test_env_parity_scoping.py | 22 ++++++++++++++++++++++ deploy/check-env-parity.py | 5 +++++ docker-compose.test.yml | 6 ++++++ docker-compose.yml | 7 +++++++ 4 files changed, 40 insertions(+) diff --git a/backend/tests/test_env_parity_scoping.py b/backend/tests/test_env_parity_scoping.py index 3e920cc4..733e9240 100644 --- a/backend/tests/test_env_parity_scoping.py +++ b/backend/tests/test_env_parity_scoping.py @@ -55,6 +55,28 @@ def test_edge_network_divergence_is_not_allowed_anywhere(self, parity, path, env assert not parity.allowed(path, env) +class TestCloudflareAccessEntries: + """The two halves of the Access configuration, which want opposite treatment. + + CF_ACCESS_AUD is the audience tag of one environment's Access application, so + it differs by nature exactly as HOST_ADMIN does and must be allowed to. Left + off the allowlist, every environment after the first would fail the parity + check and no deploy would pass. + + AUTH_ALLOW_ANONYMOUS_ADMIN must stay off it, and that does not stop mattering + once the flag is removed everywhere: the entry is what makes CI refuse a + change that reintroduces the anonymous admin to one environment on its own. + """ + + @pytest.mark.parametrize("env", ["test", "staging"]) + def test_the_access_audience_may_differ_per_environment(self, parity, env): + assert parity.allowed("services.server.environment.CF_ACCESS_AUD", env) + + @pytest.mark.parametrize("env", ["test", "staging"]) + def test_the_anonymous_admin_flag_may_never_differ(self, parity, env): + assert not parity.allowed("services.server.environment.AUTH_ALLOW_ANONYMOUS_ADMIN", env) + + class TestScopeValidation: def test_unknown_environment_is_rejected(self, parity): with pytest.raises(SystemExit): diff --git a/deploy/check-env-parity.py b/deploy/check-env-parity.py index 3cd6db90..87955ce0 100755 --- a/deploy/check-env-parity.py +++ b/deploy/check-env-parity.py @@ -80,6 +80,11 @@ r"^services\.server\.environment\.CORS_ORIGINS$", r"^services\.server\.environment\.CSP_CONNECT_SRC$", r"^services\.server\.environment\.HOST_[A-Z_]+$", + # The audience tag of this environment's Cloudflare Access application. One + # tag per application, so it differs per environment exactly as HOST_ADMIN + # does; an environment with none set leaves the verifier unconfigured, which + # means it is never consulted. See backend/core/access_identity.py. + r"^services\.server\.environment\.CF_ACCESS_AUD$", # Staging alone runs an E2E suite that force-retires the nodes it # registers, so staging alone confines what force can reach. Production # leaves the variable unset and so unrestricted: a decommissioned real diff --git a/docker-compose.test.yml b/docker-compose.test.yml index 6ce986a1..c08a61f2 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -65,6 +65,12 @@ services: # Anonymous admin, asked for explicitly since ClickUp 86cb49d29 took it # off RETINA_ENV. No OAuth is configured here either. - AUTH_ALLOW_ANONYMOUS_ADMIN=1 + # Audience of this environment's Access application, on test-admin.retina.fm. + # Pinning it is what stops a token minted for one of the fleet's node + # applications being accepted here; the team domain they share is in the + # base compose file. Staging and production carry their own once their + # applications exist. + - CF_ACCESS_AUD=e5ff9de8d1ca5fbc62b38d102d92a1fc7d910d5f89ef388caf63c83e828493b3 # The simulation subsystem, likewise asked for by name rather than # inherited from what this environment is called. This droplet is a # synthetic fleet and nothing else, so the ingest write path main.py gates diff --git a/docker-compose.yml b/docker-compose.yml index 31c7eec1..8895bb6e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -78,6 +78,13 @@ services: - SIM_FRAC_ANOMALOUS=0.0 - SIM_FRAC_DRONE=0.0 - SIM_FRAC_DARK=0.15 + # The Cloudflare Zero Trust team whose assertions the origin will believe. + # One team for the whole org, so it belongs here rather than in an overlay: + # a per-environment copy could drift to another team and the audience check + # alone would not catch it. The per-application audience does differ, and + # is CF_ACCESS_AUD in each overlay. Not a secret — it appears in the login + # URL of every Access redirect. + - CF_ACCESS_TEAM_DOMAIN=offworldlab.cloudflareaccess.com # bash, not sh: start.sh's supervisor uses `wait -n`, a bash builtin that # dash rejects ("Illegal option -n"), which crash-loops the server. The # script's shebang already says bash; naming it here keeps that true even if