From 101ec98f880c4bdb93fdf82170aa6e785c206919 Mon Sep 17 00:00:00 2001
From: Giordon Stark <kratsg@gmail.com>
Date: Wed, 9 Sep 2026 01:02:24 +0200
Subject: [PATCH] fix: widen verify_broker_token except clause to
 jwt.PyJWTError

RSAAlgorithm.from_jwk raises jwt.exceptions.InvalidKeyError for a
malformed or non-RSA JWKS entry (e.g. hit via _select_jwk's single-key
fallback when the JWKS happens to contain exactly one key). InvalidKeyError
is a PyJWTError but not an InvalidTokenError, so the previous
except jwt.InvalidTokenError clause let it escape verify_broker_token
uncaught, surfacing as an unhandled 500 with no audit log line instead
of the usual audited 401. A broker JWKS key-rotation glitch or
misconfiguration would silently produce unaudited 500s.

Widen the except clause to jwt.PyJWTError, the common base class for both
InvalidTokenError and InvalidKeyError, so any PyJWT-raised verification
failure is classified as a normal, audited 401.

Found and first fixed in servicex-token-service during code review of its
near-identical identity.py; ported here since krb5-token-service shares
the same pattern.

Assisted-by: Claude (Anthropic)
---
 src/krb5_token_service/identity.py |  8 +++++++-
 tests/test_identity.py             | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/krb5_token_service/identity.py b/src/krb5_token_service/identity.py
index fce86ea..bb08bce 100644
--- a/src/krb5_token_service/identity.py
+++ b/src/krb5_token_service/identity.py
@@ -156,7 +156,13 @@ async def verify_broker_token(token: str, settings: Settings) -> dict[str, Any]:
                     "require": ["exp", "iat", "sub"],
                 },
             )
-    except jwt.InvalidTokenError as exc:
+    except jwt.PyJWTError as exc:
+        # PyJWTError (not just InvalidTokenError) so that
+        # RSAAlgorithm.from_jwk's InvalidKeyError — raised for a malformed
+        # or non-RSA JWKS entry — is classified as a 401 like any other
+        # verification failure, rather than escaping uncaught as an
+        # unaudited 500. InvalidKeyError is a PyJWTError but not an
+        # InvalidTokenError.
         error = exc
     except (ValueError, KeyError) as exc:
         error = exc
diff --git a/tests/test_identity.py b/tests/test_identity.py
index f6622e7..4949b02 100644
--- a/tests/test_identity.py
+++ b/tests/test_identity.py
@@ -75,6 +75,28 @@ async def test_unknown_kid_is_401(
             await identity.verify_broker_token(token, settings)
         assert excinfo.value.status_code == 401
 
+    async def test_malformed_jwks_key_is_401(
+        self,
+        make_token: Callable[..., str],
+        settings: Settings,
+        stub_jwks_fetch: JwksFetchStub,
+    ) -> None:
+        # A JWKS entry with no n/e is what a broker key-rotation glitch (or
+        # a stray non-RSA key) can publish. RSAAlgorithm.from_jwk raises
+        # jwt.InvalidKeyError for it, which must be classified as a 401
+        # like any other verification failure, not escape as an unhandled
+        # 500. Reassign .keys rather than appending in place — jwks is
+        # session-scoped, so mutating it here would leak into other tests.
+        malformed_kid = "malformed-key"
+        stub_jwks_fetch.keys = [
+            *stub_jwks_fetch.keys,
+            {"kid": malformed_kid, "kty": "RSA", "use": "sig"},
+        ]
+        token = make_token(kid=malformed_kid)
+        with pytest.raises(HTTPException) as excinfo:
+            await identity.verify_broker_token(token, settings)
+        assert excinfo.value.status_code == 401
+
     @pytest.mark.parametrize("claim", ["exp", "iat", "sub"])
     async def test_missing_required_claim_is_401(
         self, make_token: Callable[..., str], settings: Settings, claim: str