fix(auth): support JWT issuer verification + warn when unscoped - #27008
fix(auth): support JWT issuer verification + warn when unscoped#27008yuneng-berri merged 3 commits into
Conversation
[Infra] Promote Internal Staging to main
[Infra] Promote Internal Staging to main
When JWT auth is enabled but `JWT_AUDIENCE` is unset, `auth_jwt` disabled audience verification entirely. Tokens minted by any other application that shared the same IdP signing keys (Azure AD, Okta, etc.) were accepted as long as their signature checked out, even though their `aud` and `iss` claims pointed at unrelated apps. The proxy then fell into the no-team / no-user branch where access checks default-allow. This change: 1. Adds support for the `JWT_ISSUER` env var. When set, PyJWT verifies the token's `iss` claim — turning on the same defense for tokens that share an audience but come from a different IdP tenant. 2. Refactors the duplicated `jwt.decode` calls (RSA/EC/OKP path and x509 path) into a single `_build_decode_kwargs` helper that computes audience, issuer, and the corresponding `verify_*` opt-outs once per call. 3. Logs a single startup-time warning when JWT auth is enabled but neither `JWT_AUDIENCE` nor `JWT_ISSUER` is configured, so operators running the insecure default see a flag in their logs without getting spammed per-request. Default behavior (no env vars) is preserved for backward compatibility. Setting `JWT_AUDIENCE` and/or `JWT_ISSUER` opts into the verification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a real security gap in JWT auth where tokens from any app sharing the same IdP signing keys were accepted when Confidence Score: 4/5Safe to merge; no regressions or security issues introduced — only P2 style/concurrency nits. The implementation is logically correct, backward-compatible, and well-tested. Two P2 findings (non-atomic warning flag under concurrent access, and the slightly opaque No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/handle_jwt.py | Adds _build_decode_kwargs classmethod that centralises audience/issuer resolution and options building for both JWT decode paths; introduces JWT_ISSUER env-var verification and a once-per-process warning when neither scoping variable is set. Logic is correct and backward-compatible; two minor P2s (non-atomic warning flag, options or None idiom). |
| tests/test_litellm/proxy/auth/test_handle_jwt.py | Adds 6 new unit tests covering all four combinations of JWT_AUDIENCE/JWT_ISSUER env vars plus the once-per-process warning behaviour; uses a proper autouse=False fixture to reset the class-level flag between test cases. |
Reviews (1): Last reviewed commit: "fix(auth): support JWT issuer verificati..." | Re-trigger Greptile
| if ( | ||
| audience is None | ||
| and issuer is None | ||
| and not cls._unscoped_jwt_warning_emitted | ||
| ): | ||
| verbose_proxy_logger.warning( | ||
| "JWT auth is enabled but neither JWT_AUDIENCE nor JWT_ISSUER " | ||
| "is configured. Tokens minted by any application that shares " | ||
| "the same IdP signing keys will be accepted. Set JWT_AUDIENCE " | ||
| "(and ideally JWT_ISSUER) to scope this proxy." | ||
| ) | ||
| cls._unscoped_jwt_warning_emitted = True |
There was a problem hiding this comment.
Non-atomic check-then-set on
_unscoped_jwt_warning_emitted
The read of cls._unscoped_jwt_warning_emitted and the subsequent assignment are not atomic. Under asyncio or threaded concurrency two coroutines can both observe False, both emit the warning, and then both set the flag — resulting in duplicate log entries. Because this is only a warning (no correctness impact) the practical risk is very low, but a simple threading.Event or checking the flag after setting it would make the intent cleaner.
| return { | ||
| "audience": audience, | ||
| "issuer": issuer, | ||
| "options": options or None, | ||
| } |
There was a problem hiding this comment.
options or None silently conflates empty dict with None
When both JWT_AUDIENCE and JWT_ISSUER are set, options is {} (falsy), so options or None evaluates to None. PyJWT treats options=None and options={} identically (both use default option values), so the behavior is correct. However, the idiom is non-obvious: a reader might wonder whether passing options=None vs options={} has any difference. A more explicit form such as options if options else None would make the intention clearer without changing behavior.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
please fix the docs issues by filing a PR to litellm-docs @stuxf |
JWT_ISSUER was added in BerriAI/litellm#27008 to scope JWT auth to a specific issuer. The litellm repo's documentation test expects every env var read in the source to be listed in the environment-variables reference table; without this entry that test fails on the source PR.
c3f7158
into
BerriAI:litellm_internal_staging
…er-verification fix(auth): support JWT issuer verification + warn when unscoped
Summary
auth_jwtdisables audience verification entirely whenJWT_AUDIENCEis unset. Tokens minted by any other application that shares the same IdP signing keys (Azure AD, Okta, etc.) were accepted as long as the signature checked out — even when theiraudandissclaims pointed at unrelated apps. Because cross-application tokens typically lack the team/user mappings the proxy expects, the resolution falls into the "no team_object / no user_object" branch where the access checks default-allow.This change keeps the default behavior intact (so existing operators are not broken) but adds two scoping levers:
JWT_ISSUER— when set, PyJWT verifies theissclaim. Pairs withJWT_AUDIENCEto fully scope tokens to this proxy/IdP combination.JWT_AUDIENCEnorJWT_ISSUERis configured, log a single warning so operators running the insecure default see a flag in their logs.The duplicated
jwt.decodecalls (RSA/EC/OKP path and x509 path) are factored through a new_build_decode_kwargshelper that resolves audience, issuer, and the correspondingverify_*opt-outs in one place.Behavior changes
aud/issnot verified, but a warning fires once.JWT_AUDIENCEset: PyJWT verifiesaud. Tokens whose audience does not match are rejected. (This is the existing pathway, just exercised more reliably.)JWT_ISSUERset: PyJWT verifiesiss. New capability.Test plan
uv run pytest tests/test_litellm/proxy/auth/test_handle_jwt.py -q— 56 pass (50 existing + 6 new for_build_decode_kwargs)InvalidAudienceError, matching-audience token accepteduv run black --checkon touched filesType
🐛 Bug Fix
✅ Test