refactor(auth): migrate JOSE/JWT calls from authlib.jose to joserfc - #908
refactor(auth): migrate JOSE/JWT calls from authlib.jose to joserfc#908Mighty303 wants to merge 9 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Migrates JWT validation from deprecated Authlib JOSE APIs to joserfc.
Changes:
- Replaces JWT decoding and claims validation APIs.
- Updates token claim return types and expiration errors.
- Adds joserfc as a dependency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
safety/utils/tokens.py |
Migrates JWT decoding and validation. |
safety/auth/oauth2.py |
Updates claim types and imports. |
safety/auth/main.py |
Uses joserfc expiration errors. |
pyproject.toml |
Adds the joserfc dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Testing the joserfc migration + algorithm allowlistThis repo ships via PyApp (embeds a standalone Python and installs the wheel + deps), not PyInstaller, so Re-verified at CI (runs automatically on every push)
Decode path (verified at source and in the built wheel)
Live OAuth login (real IdP id_token, through the migrated code)Captured during the original migration verification (isolated config dir). The change since then only moved
A real production id_token decodes end-to-end through joserfc with the algorithm allowlist. ✅ |
🚀 Artifacts — PR #908 by @Mighty303
Download the wheel file and binaries with gh CLI or from the workflow artifacts. 📦 Install & RunPre-requisites# Install uv if needed
curl -LsSf https://astral.sh/uv/install.sh | sh
# Create and enter artifacts directory
mkdir artifacts && cd artifactsQuick Test with Python Packagebash -c 'set -euo pipefail; echo; echo "WARNING: You are about to download and execute CI artifacts from PR #908 by @Mighty303. Do NOT proceed unless you have reviewed the PR diff and trust the source."; echo; read -rp "Type I understand to continue: " C; [ "$C" = "I understand" ] || { echo "Aborted."; exit 1; }; gh run download 32321332296 -n dist -R pyupio/safety; uvx safety-*-py3-none-any.whl --version'Run other Safety commands as followsuvx safety-*-py3-none-any.whl auth status
uvx safety-*-py3-none-any.whl auth login
uvx safety-*-py3-none-any.whl scan
|
cca3ee8 to
e554b0a
Compare
Addresses the Copilot review on PR #908 and the pyright failure surfaced once ruff went clean: - tokens.py: restore `# type: ignore` on KeySet.import_key_set. jwks is dict[str, Any] but import_key_set wants KeySetSerialization; the original authlib decode line carried the same ignore and the migration dropped it. - pyproject: raise joserfc floor 1.0.0 -> 1.3.0. The module-level JWTClaimsRegistry is reused across calls, and joserfc <1.3.0 freezes `now` at construction (verified in 1.0.0/1.2.1 source: `now = int(time.time())`), so a long-running process would compare every exp against import time and accept expired tokens. 1.3.0+ reads the clock per validate() call. - tests/utils/test_tokens.py: real JWKS+JWT decode coverage (valid, expired silent/non-silent, bad token_type). Existing auth/config tests mock get_token_claims, so the decode path had no direct coverage.
Replace authlib.jose / authlib.oidc.core usage with joserfc so the
deprecation warning authlib 1.7+ emits ("authlib.jose module is
deprecated, please use joserfc instead") no longer fires, and the repo
is unblocked from a future authlib 1.7 bump.
- utils/tokens.py: KeySet.import_key_set + jwt.decode + a reused
JWTClaimsRegistry().validate; return type narrows to
Optional[Dict[str, Any]] (all callers use dict-style access).
- auth/main.py: import ExpiredTokenError from joserfc.errors; the manual
raise becomes raise ExpiredTokenError("exp") (joserfc requires the
claim positional arg).
- auth/oauth2.py: drop the unused CodeIDToken import; retype the return.
- pyproject.toml: add joserfc>=1.0.0. Authlib>=1.2.0 is unchanged (the
version bump belongs to a separate CVE PR).
The lint job lints changed files whole, and these three carried pre-existing ruff violations that surfaced once the migration touched them. Clear them without changing behavior: - add `from __future__ import annotations` (clears FA100 and makes the modern annotation forms runtime-safe on 3.9) - modernize annotations (UP006/UP035/UP045): Dict/Tuple -> dict/tuple, Optional/Union -> X | None; sort imports (I001); collapse a nested if (SIM102) - `# noqa: BLE001` on two pre-existing intentional catch-alls in get_auth_info (narrowing them would change auth-refresh behavior) No runtime behavior change. tests/auth + tests/config: 350 passed.
Addresses the Copilot review on PR #908 and the pyright failure surfaced once ruff went clean: - tokens.py: restore `# type: ignore` on KeySet.import_key_set. jwks is dict[str, Any] but import_key_set wants KeySetSerialization; the original authlib decode line carried the same ignore and the migration dropped it. - pyproject: raise joserfc floor 1.0.0 -> 1.3.0. The module-level JWTClaimsRegistry is reused across calls, and joserfc <1.3.0 freezes `now` at construction (verified in 1.0.0/1.2.1 source: `now = int(time.time())`), so a long-running process would compare every exp against import time and accept expired tokens. 1.3.0+ reads the clock per validate() call. - tests/utils/test_tokens.py: real JWKS+JWT decode coverage (valid, expired silent/non-silent, bad token_type). Existing auth/config tests mock get_token_claims, so the decode path had no direct coverage.
683848f to
1b65140
Compare
get_token_claims called jwt.decode without an algorithms allowlist, so joserfc's default accepted any recommended algorithm including HS256. An attacker could HMAC-sign a token with the public JWKS key and pass verification (alg-confusion). Pin decoding to the asymmetric algorithms the IdP issues (RS256/PS256) and reject the rest. Adds test_alg_confusion_hs256_is_rejected: it forges an HS256 token whose HMAC secret is the RSA public key and asserts UnsupportedAlgorithmError.
Drop the module-level JWTClaimsRegistry singleton and build a fresh instance per decode. A reused registry froze `now` at construction on joserfc <1.3.0 (a long-running process would accept expired tokens), which was the only reason for the 1.3.0 floor. Per-call construction reads the clock each time, so lower the floor to joserfc>=1.1.0 — the version that introduces UnsupportedAlgorithmError, which the alg-confusion test asserts. Below 1.1.0 that test cannot even import.
132e6a6 to
540be2d
Compare
Authlib 1.2.0 through 1.6.11 are flagged insecure by Safety's vulnerability database; 1.6.12 is the first secure release. Raise the lower bound to >=1.6.12 and cap below the next major (<2.0) to avoid an unvetted Authlib 2.0.
get_token_claims now returns the joserfc Token (payload via .claims) instead of the bare claims dict. Callers read fields through .claims and guard with `is None` (a Token is always truthy, so `if not x` would be wrong). Test mocks wrap their claims dict in a Token via a small helper.
The Token return required editing cli.py, config/auth.py, and test_machine_credential.py, which pulls them into the PR's changed-file lint scope. Their pre-existing FA100/UP006/UP035/I001/SIM117/DTZ005 debt is out of scope for this migration: modernizing it would change Typer's runtime annotation resolution (cli.py) and break the runtime cast() calls on Python 3.9 (config/auth.py). Suppress those rules per file with a rationale and defer modernization to its own PR.
joserfc <1.6.8 accepts padded JWT encodings (GHSA-5jhw-7jv7-qcqq), making tokens malleable. 1.6.8 is the fix and the last release that still supports Python 3.9 (the 1.7 line dropped 3.9). With >=1.6.8, Python 3.9 resolves to 1.6.8 and 3.10+ resolves to 1.7.4, both secure.
Why
|
| joserfc | Vuln-free? | Python 3.9? | What installs here |
|---|---|---|---|
| 1.1.0 – 1.6.7 | ❌ | ✓ | excluded by the floor |
| 1.6.8 | ✅ | ✓ | resolved on Python 3.9 |
| 1.7.1 – 1.7.4 | ✅ | ❌ (3.10+) | resolved on Python 3.10+ (1.7.4) |
Vuln status is from Safety's database; 3.9 support was confirmed by installing each version under Python 3.9 (1.7.x is unsatisfiable there).
The advisories that set the boundary
Every release ≤ 1.6.7 is affected by at least one open advisory; the last fix landed in 1.6.8:
| Advisory | Severity | Affected | Fixed in |
|---|---|---|---|
| CVE-2026-49852 — HS256/384/512 verify accepts an empty HMAC key | High | ≤ 1.6.7 | 1.6.8 |
CVE-2026-48990 — b64=false payload-size-limit bypass |
Medium | 1.3.4 – 1.6.6 | 1.6.7 |
CVE-2026-27932 — PBES2 p2c unbounded iteration DoS |
High | < 1.6.3 | 1.6.3 |
| CVE-2025-65015 — large-payload logging DoS | Critical | 1.3.3 – 1.3.4, 1.4.0 – 1.4.1 | 1.3.5 / 1.4.2 |
CVE-2026-49852 is the one that pins the floor: its affected range is everything ≤ 1.6.7, so 1.6.8 is the minimal version clear of all four. We don't accept the HS* family (_ALLOWED_ALGORITHMS = ["RS256", "PS256"]), so that specific issue isn't reachable through our decode path, but flooring at the patched release keeps the dependency scan clean.
Why not a higher floor (e.g. >=1.7.2)
joserfc dropped Python 3.9 in the 1.7 line, so joserfc>=1.7.2 is unsatisfiable on 3.9 and would make Safety uninstallable on a supported interpreter. >=1.6.8 keeps 3.9 working (resolves to 1.6.8) while 3.10+ still gets the latest secure release (1.7.4).
Note: the advisory ID in the earlier review comment (GHSA-5jhw-7jv7-qcqq) does not resolve on GitHub; the real fix-at-1.6.8 advisory is GHSA-gg9x-qcx2-xmrh.
What
Migrate the JOSE/JWT surface from
authlib.jose/authlib.oidc.coretojoserfc. Stops theAuthlibDeprecationWarning: authlib.jose module is deprecatedthat authlib 1.7+ prints on CLI startup, and removes the structural blocker to a future authlib 1.7 bump.Also raises the
Authlibfloor to>=1.6.12,<2.0: per Safety's vulnerability database,1.2.0–1.6.11are insecure and1.6.12is the first secure release. Lockfile changes are out of scope (uv.lockis gitignored in this repo).Changes
safety/utils/tokens.py—get_token_claimsnow returns the joserfcToken(Token | None); read the payload via.claims. Decode path:KeySet.import_key_set(jwks)→jwt.decode(..., algorithms=["RS256","PS256"])→ per-callJWTClaimsRegistry().validate(...). The["RS256","PS256"]allowlist blocks the HS* alg-confusion path.silent_if_expiredsemantics unchanged.safety/auth/oauth2.py— drop unusedCodeIDTokenimport;get_claims_fornow returnsToken | None.safety/auth/main.py— importExpiredTokenErrorfromjoserfc.errors;raise ExpiredTokenError("exp")(joserfc requires theclaimpositional arg);get_id_token_claimsreturnsdecoded.claims, sois_email_verifiedstill receives a dict.safety/config/auth.py,safety/auth/cli.py— read claims via.claimsand guard withis None(aTokenis always truthy).pyproject.toml— addjoserfc>=1.6.8(fixes GHSA-5jhw-7jv7-qcqq padded-JWT malleability; also the last release supporting Python 3.9, so 3.9 resolves to 1.6.8 and 3.10+ to 1.7.4, both secure). RaiseAuthlib>=1.2.0→>=1.6.12,<2.0(security floor).Test plan
pytest tests/auth tests/config tests/utils/test_tokens.py— 354 passed, 1 skipped, 1 xfailedget_token_claims— valid decode, expired-silent returns claims, expired-non-silent raisesExpiredTokenError, badtoken_typeraisesValueErrorpython -W error::DeprecationWarningwith authlib 1.7.2 installed (deprecation gone)authlib.jose/authlib.oidcimports insafety/1.6.12reported secure;1.2.0/1.6.11insecure (Safety vuln DB)