Skip to content

fix(jwt): OIDC discovery URLs, roles array handling, dot-notation error hints#22336

Merged
ishaan-jaff merged 8 commits intomainfrom
fix/jwt-auth-oidc-array-roles
Feb 28, 2026
Merged

fix(jwt): OIDC discovery URLs, roles array handling, dot-notation error hints#22336
ishaan-jaff merged 8 commits intomainfrom
fix/jwt-auth-oidc-array-roles

Conversation

@ishaan-jaff
Copy link
Member

Relevant issues

Fixes Azure AD JWT auth compatibility issues (Fortescue spike).

Changes

Three fixes in litellm/proxy/auth/handle_jwt.py:

1. OIDC discovery URL support

JWT_PUBLIC_KEY_URL now accepts .well-known/openid-configuration endpoints, not just direct JWKS URLs. The proxy auto-fetches the discovery doc, extracts jwks_uri, and caches the resolved URL.

# This now works (AAD v2 endpoint):
JWT_PUBLIC_KEY_URL=https://login.microsoftonline.com/<tenant>/v2.0/.well-known/openid-configuration

2. Handle roles claim as array

AAD puts team membership in "roles": ["team1"] (a list). Setting team_id_jwt_field: "roles" used to crash with unhashable type: 'list'. Now the first element is used automatically, with a debug log.

3. Better error hint for misconfigured dot-notation

When team_id_jwt_field is set to "roles.0" or "roles[0]" (neither works), the 401 now says: "Use 'roles' instead — LiteLLM automatically uses the first element when the field value is a list."

Pre-Submission checklist

  • Added tests in tests/test_litellm/proxy/auth/test_handle_jwt.py (37 tests, all pass)
  • make test-unit passes

Type

  • Bug Fix

…ror hints

Three fixes for Azure AD JWT auth:

1. OIDC discovery URL support - JWT_PUBLIC_KEY_URL can now be set to
   .well-known/openid-configuration endpoints. The proxy fetches the
   discovery doc, extracts jwks_uri, and caches it.

2. Handle roles claim as array - when team_id_jwt_field points to a list
   (e.g. AAD's "roles": ["team1"]), auto-unwrap the first element instead
   of crashing with 'unhashable type: list'.

3. Better error hint for dot-notation indexing - when team_id_jwt_field is
   set to "roles.0" or "roles[0]", the 401 error now explains to use
   "roles" instead and that LiteLLM auto-unwraps lists.
@vercel
Copy link

vercel bot commented Feb 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 28, 2026 4:32am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 28, 2026

Greptile Summary

Fixes three Azure AD JWT auth compatibility issues in litellm/proxy/auth/handle_jwt.py: (1) OIDC discovery URL support so JWT_PUBLIC_KEY_URL can point to .well-known/openid-configuration endpoints, (2) handling the roles claim as an array by auto-extracting the first element, and (3) helpful error hints when operators misconfigure team_id_jwt_field with unsupported dot/bracket notation like roles.0 or roles[0].

  • The core logic changes are well-structured, follow existing patterns (caching, error handling), and don't introduce DB queries or new Router objects in the critical path.
  • Three OIDC discovery unit tests are broken: the mock responses don't set status_code = 200, so the new HTTP status check raises before the tests exercise their intended code paths.
  • Two demo scripts (demo_jwt_fixes.py, demo_servers.py) are committed at the repo root and should be removed before merge.
  • Also includes unrelated model pricing additions for OpenRouter models in the backup JSON.

Confidence Score: 3/5

  • Core auth logic changes are sound, but test coverage has a bug that masks whether OIDC discovery actually works correctly under test.
  • The production code changes are well-targeted and follow existing patterns. However, three unit tests for the OIDC discovery feature are broken due to missing mock status_code, meaning the happy-path resolution logic is not actually tested. The demo scripts at repo root should be cleaned up before merge.
  • Pay close attention to tests/test_litellm/proxy/auth/test_handle_jwt.py — the three OIDC discovery tests need mock_response.status_code = 200 to properly exercise the code under test.

Important Files Changed

Filename Overview
litellm/proxy/auth/handle_jwt.py Three well-targeted fixes: OIDC discovery URL resolution with caching, array-value handling for team_id_jwt_field, and helpful error hints for unsupported dot/bracket notation. Logic is sound and follows existing patterns.
tests/test_litellm/proxy/auth/test_handle_jwt.py Good test coverage for all three fixes (13 new tests), but three OIDC discovery tests are missing mock_response.status_code = 200, causing them to hit the status-code check rather than exercising the intended code path.
demo_jwt_fixes.py Integration demo script added at repo root — useful for manual verification but should not be committed to the repository.
demo_servers.py Mock JWKS/OIDC server helper script added at repo root — useful for manual testing but should not be committed.
litellm/model_prices_and_context_window_backup.json Adds model pricing entries for openrouter/anthropic/claude-opus-4.6, openrouter/openrouter/auto, openrouter/openrouter/free, and openrouter/openrouter/bodybuilder. Standard data addition.
.github/demo-assets/TEST_RESULTS.md Integration test results documentation with screenshots showing all three fixes verified end-to-end.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["get_public_key(kid)"] --> B["_resolve_jwks_url(key_url)"]
    B --> C{".well-known/openid-configuration" in URL?}
    C -- No --> D["Return URL unchanged"]
    C -- Yes --> E{Cached jwks_uri?}
    E -- Yes --> F["Return cached jwks_uri"]
    E -- No --> G["Fetch OIDC discovery doc"]
    G --> H{status_code == 200?}
    H -- No --> I["Raise HTTP error"]
    H -- Yes --> J["Extract jwks_uri from response"]
    J --> K{jwks_uri present?}
    K -- No --> L["Raise missing jwks_uri error"]
    K -- Yes --> M["Cache jwks_uri & return"]

    N["get_team_id(token)"] --> O["get_nested_value(token, field)"]
    O --> P{Result is a list?}
    P -- Yes, non-empty --> Q["Use first element"]
    P -- Yes, empty --> R["Return default_value"]
    P -- No --> S["Return value as-is"]

    T["find_and_validate_specific_team_id"] --> U{No team found & team required?}
    U -- Yes --> V{field has dot + digit suffix?}
    V -- Yes --> W["Hint: use base field instead"]
    U -- Yes --> X{field has bracket notation?}
    X -- Yes --> Y["Hint: use base field instead"]
    U -- Yes --> Z["Raise with error + hint"]
Loading

Last reviewed commit: c6207c4

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

… roles, dot-notation hints)

Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ishaan-jaff
❌ cursoragent
You have signed the CLA already but the status is still pending? Let us recheck it.

…ndpoints

Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
- fix: add HTTP status code check in _resolve_jwks_url before parsing JSON
- fix: remove misleading bracket-notation hint from debug log (get_nested_value does not support it)
@ishaan-jaff
Copy link
Member Author

@greptile review again

@@ -0,0 +1,329 @@
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Demo scripts committed to repo root

demo_jwt_fixes.py and demo_servers.py appear to be one-off integration demo/test scripts. They should not be committed to the repo root — they add clutter and aren't part of the project's test suite or tooling. Consider removing these from the PR and referencing their output in the PR description or .github/demo-assets/ only.

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!

ishaan-jaff and others added 2 commits February 27, 2026 20:29
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@ishaan-jaff ishaan-jaff merged commit ee703ce into main Feb 28, 2026
27 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants