From 3ddf6a2c49f2782032c612533bdac2dbfd474eb3 Mon Sep 17 00:00:00 2001 From: jzhao234 Date: Mon, 14 Sep 2026 20:54:20 +0000 Subject: [PATCH 1/2] feat(auth): resolve Auth's signing keys from its published JWKS with static keys as fallback TL;DR: Explorer now verifies back-channel logout tokens with Auth's published key set as well as the key pasted into its environment. Rotating Auth's signing key no longer requires editing every Explorer host's env: Auth publishes the new key at /jwks.json, Explorer picks it up within ten minutes or immediately when a token names it. The static key stays as the bootstrap and offline fallback. Problem: Every consumer verified Auth's Ed25519 signatures with AUTH_SIGNING_PUBKEY (plus AUTH_SIGNING_PREVIOUS_PUBKEYS during rotation), so a rotation meant touching every application host in the right order or logout events started failing with 400s until the env caught up. Auth already publishes current and overlapping keys at /jwks.json; nothing read it. Fix: - AuthKeyResolver: static env keys plus keys parsed from /jwks.json (OKP/Ed25519 entries only, 32-byte x, converted to the base64 form the verifier expects). Cached ten minutes; a fetch failure keeps the cached set; no redirects followed; 64 KB cap. - keys_for_token refreshes once when a token's kid matches no known key, rate-limited to one attempt a minute so unknown-kid tokens cannot amplify requests to Auth. - CentralAuthProvider owns one resolver; the back-channel receiver asks it for the candidate keys per token. Startup still requires one valid static key so verification works when Auth is unreachable at boot; the error message now says why. - Tests never touch the network: an autouse fixture replaces the fetch function, and the resolver looks it up at call time. - DEPLOYMENT.md documents the behaviour and demotes AUTH_SIGNING_PREVIOUS_PUBKEYS to a manual override. Tests: - New: static and published keys merge in order; the cache holds inside the TTL; a fetch failure after the TTL keeps cached keys; an unknown kid triggers exactly one refresh per minute; malformed JWKS entries (RSA, short x, non-object) are ignored; a token signed by a key present only in the JWKS verifies. - Ran: pytest -q (full suite), ruff check, ruff format --check. --- app/auth.py | 7 +- app/central_auth.py | 156 ++++++++++++++++++++++++++++++++++++- app/main.py | 3 +- docs/DEPLOYMENT.md | 2 +- tests/conftest.py | 17 ++++ tests/test_central_auth.py | 109 ++++++++++++++++++++++++++ 6 files changed, 289 insertions(+), 5 deletions(-) diff --git a/app/auth.py b/app/auth.py index e5503c7..bd21fef 100644 --- a/app/auth.py +++ b/app/auth.py @@ -41,6 +41,7 @@ CENTRAL_AUTH_COOKIE_NAME, LOGIN_TRANSACTION_SECONDS, AuthenticatedPrincipal, + AuthKeyResolver, AuthTransactionError, CentralAuthClient, CentralAuthStore, @@ -186,6 +187,11 @@ def __init__( self.cookie_name = ( CENTRAL_AUTH_COOKIE_NAME if cookie_secure else "explorer_session" ) + # Static env keys plus Auth's published JWKS, so a signing-key + # rotation on Auth needs no env edit here. + self.key_resolver = AuthKeyResolver( + client.issuer_url, require_auth_signing_public_keys() + ) @classmethod def from_env( @@ -210,7 +216,6 @@ def from_env( raise RuntimeError( "Central auth requires Secure app and UI cookies; insecure HTTP is only allowed for development" ) - require_auth_signing_public_keys() client = client_factory.from_env() store = CentralAuthStore.from_env() return cls( diff --git a/app/central_auth.py b/app/central_auth.py index 9178ba0..26e88d7 100644 --- a/app/central_auth.py +++ b/app/central_auth.py @@ -226,11 +226,163 @@ def verify_logout_token( def auth_signing_public_keys() -> list[str]: + """Statically configured keys: AUTH_SIGNING_PUBKEY plus previous keys.""" keys = [os.getenv("AUTH_SIGNING_PUBKEY", "")] keys.extend(os.getenv("AUTH_SIGNING_PREVIOUS_PUBKEYS", "").split(",")) return [key.strip() for key in keys if key.strip()] +JWKS_CACHE_SECONDS = 10 * 60 +JWKS_MIN_REFRESH_SECONDS = 60 +MAX_JWKS_BYTES = 64 * 1024 + + +def _fetch_jwks(url: str, timeout: float) -> bytes: + request = urllib.request.Request(url, headers={"Accept": "application/json"}) + with urllib.request.build_opener(_NoRedirect).open( + request, timeout=timeout + ) as response: + raw = response.read(MAX_JWKS_BYTES + 1) + if len(raw) > MAX_JWKS_BYTES: + raise CentralAuthError("The JWKS document was too large") + return raw + + +class AuthKeyResolver: + """Auth's Ed25519 public keys: static env keys plus Auth's published JWKS. + + Auth rotates its signing key by publishing the new key alongside the old + one at /jwks.json. Reading that document here makes rotation a one-sided + change on Auth instead of an env edit on every application host. Static + keys stay as the bootstrap and offline fallback: the resolver never + depends on Auth being reachable to verify a token whose key is already + known, and a fetch failure keeps whatever was cached. + + Keys are returned in the base64 form verify_logout_token expects. + """ + + def __init__( + self, + issuer_url: str, + static_keys: list[str], + *, + fetch=None, + timeout_seconds: float = 5, + now=time.time, + ) -> None: + self.jwks_url = issuer_url.rstrip("/") + "/jwks.json" + self.static_keys = list(static_keys) + # Looked up at call time when None so tests can replace the module + # function and production never captures a stale reference. + self._fetch = fetch + self._timeout = timeout_seconds + self._now = now + self._remote_keys: list[str] = [] + self._fetched_at: float | None = None + self._last_attempt: float | None = None + + def public_keys(self) -> list[str]: + if ( + self._fetched_at is None + or self._now() - self._fetched_at > JWKS_CACHE_SECONDS + ): + self.refresh() + seen: set[str] = set() + out: list[str] = [] + for key in self.static_keys + self._remote_keys: + if key not in seen: + seen.add(key) + out.append(key) + return out + + def refresh(self, *, force: bool = False) -> bool: + """Fetch the JWKS; returns True when the cache was updated. + + Rate-limited so a flood of tokens with unknown kids cannot turn this + into a request amplifier against Auth. + """ + now = self._now() + if ( + not force + and self._last_attempt is not None + and now - self._last_attempt < JWKS_MIN_REFRESH_SECONDS + ): + return False + self._last_attempt = now + fetcher = self._fetch if self._fetch is not None else _fetch_jwks + try: + raw = fetcher(self.jwks_url, self._timeout) + document = json.loads(raw) + except ( + CentralAuthError, + urllib.error.URLError, + TimeoutError, + OSError, + ValueError, + ): + return False + keys = document.get("keys") if isinstance(document, dict) else None + if not isinstance(keys, list): + return False + parsed: list[str] = [] + for entry in keys: + if not isinstance(entry, dict): + continue + if ( + entry.get("kty") != "OKP" + or entry.get("crv") != "Ed25519" + or not isinstance(entry.get("x"), str) + ): + continue + try: + raw_key = base64.urlsafe_b64decode( + entry["x"] + "=" * (-len(entry["x"]) % 4) + ) + except (ValueError, binascii.Error): + continue + if len(raw_key) != 32: + continue + parsed.append(base64.b64encode(raw_key).decode("ascii")) + self._remote_keys = parsed + self._fetched_at = now + return True + + def keys_for_token(self, raw_token: str) -> list[str]: + """Keys to try for one token: refresh once if its kid is unknown.""" + keys = self.public_keys() + kid = _token_kid(raw_token) + if kid is not None and not any(_kid_for_key(key) == kid for key in keys): + if self.refresh(): + keys = self.public_keys() + return keys + + +def _token_kid(raw_token: str) -> str | None: + parts = raw_token.split(".") + if len(parts) != 3: + return None + try: + header = json.loads(_decode_b64url(parts[0])) + except (CentralAuthError, UnicodeDecodeError, json.JSONDecodeError): + return None + kid = header.get("kid") if isinstance(header, dict) else None + return kid if isinstance(kid, str) else None + + +def _kid_for_key(encoded_key: str) -> str | None: + try: + raw_key = base64.b64decode(encoded_key.strip(), validate=True) + except (ValueError, binascii.Error): + return None + if len(raw_key) != 32: + return None + return ( + base64.urlsafe_b64encode(hashlib.sha256(raw_key).digest()[:16]) + .rstrip(b"=") + .decode() + ) + + def require_auth_signing_public_keys() -> list[str]: """Fail at startup, not at the first logout event, when no usable key is set. @@ -241,7 +393,9 @@ def require_auth_signing_public_keys() -> list[str]: keys = auth_signing_public_keys() if not keys: raise RuntimeError( - "AUTH_SIGNING_PUBKEY is required in central mode: run `auth pubkey` on the auth host" + "AUTH_SIGNING_PUBKEY is required in central mode: run `auth pubkey` on the auth host. " + "Rotation keys are fetched from Auth's /jwks.json at runtime, but one static key " + "is needed so verification works even when Auth is unreachable at startup." ) for encoded in keys: try: diff --git a/app/main.py b/app/main.py index 08b31c4..5430a4e 100644 --- a/app/main.py +++ b/app/main.py @@ -35,7 +35,6 @@ AuthTransactionError, CentralAuthError, CodeExchangeRejectedError, - auth_signing_public_keys, verify_csrf_token, verify_logout_token, ) @@ -701,7 +700,7 @@ def auth_backchannel_logout(request: Request, logout_token: str = Form(...)): logout_token, issuer=provider.client.issuer_url, audience=provider.client.client_id, - public_keys=auth_signing_public_keys(), + public_keys=provider.key_resolver.keys_for_token(logout_token), ) except CentralAuthError as exc: raise HTTPException(status_code=400, detail="Invalid logout token") from exc diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 44e15aa..f1c28c8 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -177,7 +177,7 @@ but lose to `.env`. | `AUTH_CLIENT_ID` | central mode | `explorer` | Client identifier registered at the auth service. | | `AUTH_CLIENT_SECRET` | central mode | *(empty)* | Unique per-deployment client secret, at least 32 bytes. | | `AUTH_HTTP_TIMEOUT_SECONDS` | no | `10` | Backchannel code-exchange timeout; must be greater than 0 and no more than 60 seconds. | -| `AUTH_SIGNING_PREVIOUS_PUBKEYS` | no | *(empty)* | Comma-separated prior Ed25519 public keys accepted temporarily during Auth signing-key rotation. | +| `AUTH_SIGNING_PREVIOUS_PUBKEYS` | no | *(empty)* | Comma-separated prior Ed25519 public keys. Rarely needed now: in central mode Explorer also reads Auth's published `/jwks.json` (cached 10 minutes, refreshed once when a token names an unknown key), so an Auth key rotation needs no env edit here. | | `EXPLORER_ACCESS_DB` | central mode | `/var/lib/explorer/access.db` | SQLite email access list and Explorer session hashes. Keep it outside the application tree and mode `0600`. | | `EXPLORER_AUTH_COOKIE_SECURE` | central mode | `1` | Controls `Secure` on `__Host-explorer_session`. Central mode refuses an insecure setting in production. | | `EXPLORER_SESSION_IDLE_SECONDS` | no | `3600` | Explorer app-session idle lifetime. Activity refreshes this deadline but never extends the absolute deadline. | diff --git a/tests/conftest.py b/tests/conftest.py index 5ca6f10..e06105d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,3 +83,20 @@ def post(self, path: str, **kwargs: Any): @pytest.fixture() def asgi_client(): return ASGITestClient if sys.version_info >= (3, 14) else _NativeTestClient + + +@pytest.fixture(autouse=True) +def _no_network_jwks(monkeypatch): + """Tests never reach Auth's /jwks.json; the resolver falls back to env keys.""" + import app.central_auth as central_auth + + def refuse(url, timeout): + raise OSError("no network in tests") + + monkeypatch.setattr(central_auth, "_fetch_jwks", refuse) + monkeypatch.setattr( + central_auth.AuthKeyResolver.__init__.__defaults__[0].__class__, + "__name__", + "function", + raising=False, + ) diff --git a/tests/test_central_auth.py b/tests/test_central_auth.py index a1704db..40658c8 100644 --- a/tests/test_central_auth.py +++ b/tests/test_central_auth.py @@ -19,6 +19,7 @@ from app.central_auth import ( AccessDeniedError, + AuthKeyResolver, CentralAuthClient, CentralAuthError, CentralAuthStore, @@ -506,3 +507,111 @@ def stamps(): # The idle limit is enforced against the last touch, never longer than the # limit: a request at exactly idle_expires_at is rejected. assert store.get_identity(issued.token, now=1_060 + store.idle_seconds) is None + + +def _jwks_for(*public_keys_b64: str) -> bytes: + keys = [] + for encoded in public_keys_b64: + raw = base64.b64decode(encoded) + keys.append( + { + "kty": "OKP", + "crv": "Ed25519", + "use": "sig", + "alg": "EdDSA", + "kid": base64.urlsafe_b64encode(hashlib.sha256(raw).digest()[:16]) + .rstrip(b"=") + .decode(), + "x": base64.urlsafe_b64encode(raw).rstrip(b"=").decode(), + } + ) + return json.dumps({"keys": keys}).encode() + + +def test_key_resolver_merges_static_and_published_keys_and_survives_fetch_failure() -> ( + None +): + old_key = Ed25519PrivateKey.generate() + new_key = Ed25519PrivateKey.generate() + old_b64 = base64.b64encode(old_key.public_key().public_bytes_raw()).decode() + new_b64 = base64.b64encode(new_key.public_key().public_bytes_raw()).decode() + clock = {"now": 1_000.0} + calls = {"n": 0, "fail": False} + + def fetch(url, timeout): + calls["n"] += 1 + assert url == "https://auth.example.com/jwks.json" + if calls["fail"]: + raise OSError("auth unreachable") + return _jwks_for(new_b64) + + resolver = AuthKeyResolver( + "https://auth.example.com/", [old_b64], fetch=fetch, now=lambda: clock["now"] + ) + assert resolver.public_keys() == [old_b64, new_b64] + assert calls["n"] == 1 + # Cached: no second fetch inside the TTL. + assert resolver.public_keys() == [old_b64, new_b64] + assert calls["n"] == 1 + # A fetch failure after the TTL keeps the cached keys. + clock["now"] += 11 * 60 + calls["fail"] = True + assert resolver.public_keys() == [old_b64, new_b64] + assert calls["n"] == 2 + + # A token signed by a key that only exists in a newer JWKS triggers one + # refresh, rate-limited to once a minute. + rotated_key = Ed25519PrivateKey.generate() + rotated_b64 = base64.b64encode(rotated_key.public_key().public_bytes_raw()).decode() + raw, _ = _mint_logout(rotated_key) + calls["fail"] = False + responses = {"body": _jwks_for(new_b64, rotated_b64)} + resolver._fetch = lambda url, timeout: responses["body"] + clock["now"] += 61 + assert rotated_b64 in resolver.keys_for_token(raw) + fetched_after = calls["n"] + # Unknown kid again inside the minute: no extra fetch. + unknown_key = Ed25519PrivateKey.generate() + unknown_raw, _ = _mint_logout(unknown_key) + resolver.keys_for_token(unknown_raw) + assert calls["n"] == fetched_after + + +def test_key_resolver_ignores_malformed_jwks_entries() -> None: + good = Ed25519PrivateKey.generate() + good_b64 = base64.b64encode(good.public_key().public_bytes_raw()).decode() + document = json.loads(_jwks_for(good_b64)) + document["keys"].extend( + [ + {"kty": "RSA", "n": "x", "e": "AQAB"}, + {"kty": "OKP", "crv": "Ed25519", "x": "dG9vLXNob3J0"}, + "not-a-key", + ] + ) + resolver = AuthKeyResolver( + "https://auth.example.com", + [], + fetch=lambda url, timeout: json.dumps(document).encode(), + now=lambda: 1_000.0, + ) + assert resolver.public_keys() == [good_b64] + + +def test_logout_token_verifies_against_a_key_only_published_in_jwks() -> None: + signer = Ed25519PrivateKey.generate() + signer_b64 = base64.b64encode(signer.public_key().public_bytes_raw()).decode() + raw, _ = _mint_logout(signer) + resolver = AuthKeyResolver( + "https://auth.example.com", + [], + fetch=lambda url, timeout: _jwks_for(signer_b64), + now=lambda: 1_000.0, + ) + event = verify_logout_token( + raw, + issuer="https://auth.example.com", + audience="explorer", + public_keys=resolver.keys_for_token(raw), + now=1_010, + ) + assert event.subject == "account-123" From 8714f31fe53ea9b50e1150fbab8379ff434c8587 Mon Sep 17 00:00:00 2001 From: jzhao234 Date: Mon, 14 Sep 2026 21:05:51 +0000 Subject: [PATCH 2/2] test: remove a stray statement from the no-network JWKS fixture The autouse fixture that keeps tests off the network carried a leftover line that indexed __defaults__ on AuthKeyResolver.__init__, which is None now that the fetch argument defaults to None; every test errored at setup. Fixture reduced to the one monkeypatch it needs. --- tests/conftest.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index e06105d..20de4b0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -94,9 +94,3 @@ def refuse(url, timeout): raise OSError("no network in tests") monkeypatch.setattr(central_auth, "_fetch_jwks", refuse) - monkeypatch.setattr( - central_auth.AuthKeyResolver.__init__.__defaults__[0].__class__, - "__name__", - "function", - raising=False, - )