feat: replace python-jose with PyJWT for token verification - #107
feat: replace python-jose with PyJWT for token verification#107HarryMWinters wants to merge 1 commit into
Conversation
python-jose drags in ecdsa, rsa and pyasn1, which carry open security advisories (the ecdsa Minerva timing issue will never be fixed upstream). PyJWT with the crypto extra needs only cryptography. - auth.py: select the signing key from the JWKS by the token's kid header (single-key JWKS accepted for tokens without kid); decode with jwt.decode; map every PyJWTError to HTTPException(401). - Non-JWKS keys (PEM strings, PyJWK) pass through unchanged so existing discovery stubs keep working. - Require pyjwt[crypto] >= 2.13.0 for the JWK algorithm/key-confusion fixes. - Drop python-jose and types-python-jose; add pyjwt to the mypy hook env. - Tests: JWKS fixtures plus kid selection, unknown kid, multi-key-no-kid, wrong signing key, and generator-issuer cases (coverage 91% -> 93%). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8665001162
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not isinstance(keys, dict) or "keys" not in keys: | ||
| return keys | ||
|
|
||
| jwk_set = PyJWKSet.from_dict(keys) |
There was a problem hiding this comment.
When a provider publishes a valid RSA JWK without the optional alg member and signs ID tokens with PS256, RS384, or another advertised RSA algorithm, PyJWKSet.from_dict() binds that key to RS256 by default. PyJWT then rejects every otherwise-valid token because its header algorithm does not match the key's bound algorithm, even when the algorithm appears in the discovery allow-list. Construct the selected PyJWK using the token header's allowed algorithm (or pass an unbound public key) so alg-less JWKs continue to support the provider's advertised signing algorithms.
Useful? React with 👍 / 👎.
1 participant
Why
All 13 open Dependabot security alerts on this repo were in
python-jose's transitive dependencies (cryptography,pyasn1,rsa,ecdsa). Thepython-ecdsaMinerva timing advisory (GHSA-wj6h-64fc-37mp) has no fix and upstream will not provide one, so it can only be cleared by droppingpython-jose.PyJWT with the
cryptoextra depends only oncryptography, and was already a dev dependency used by the test fixtures.What changed
fastapi_oidc/auth.pynow verifies withjwt.decode. The signing key is selected from the provider's JWKS by the token'skidheader; a single-key JWKS is still accepted for tokens without akid. EveryPyJWTErrormaps toHTTPException(401)exactly as before.PyJWK) are passed through unchanged, so existing discovery stubs and monkeypatches keep working.pyjwt[crypto] >= 2.13.0, which ships the fixes for algorithm allow-list bypass and key-family confusion when verifying with JWK keys (both relevant to this code path).python-jose[cryptography]andtypes-python-jose.ecdsa,rsaandpyasn1are gone from the lockfile.pyjwt[crypto]to the mypy pre-commit hook environment..agents, CHANGELOG) updated.Public API
get_auth()signature and the returnedauthenticate_userdependency are unchanged. Theverify_at_hashoption is gone because PyJWT never checkedat_hash; the old code disabled that check anyway.Tests
New cases: JWKS key selection by
kid, single-key JWKS withoutkid, unknownkid→ 401, multi-key JWKS withoutkid→ 401, token signed by a key not in the JWKS → 401, issuer passed as a generator. 36 tests pass; coverage 91% → 93%. Pre-commit hooks all pass.🤖 Generated with Claude Code