[Feat]Add support for azure entra discovery endpoint#26575
[Feat]Add support for azure entra discovery endpoint#26575Sameerlite wants to merge 4 commits intolitellm_internal_stagingfrom
Conversation
Greptile SummaryThis PR adds Azure Entra (OIDC) discovery support for MCP OAuth flows in two ways: (1) a new issuer-relative candidate URL Confidence Score: 3/5The PR has a confirmed P1 security concern (permanent MCP servers become accessible to unauthenticated callers on the authorize/token endpoints) noted in the previous review cycle and still unresolved. The Azure discovery logic itself is well-structured and the new tests cover the key scenarios. However, the open P1 from the prior review round — unauthenticated access to permanent (non-session) MCP servers on the OAuth proxy endpoints — remains unaddressed, lowering confidence below 4. litellm/proxy/management_endpoints/mcp_management_endpoints.py — specifically
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Adds Azure Entra discovery support: new candidate URL for issuer-relative OIDC config, fallback _build_azure_authorization_server_metadata covering three Azure authority hosts, and handling of the non-challenge (200 OK) path via well-known discovery instead of RuntimeError. |
| litellm/proxy/management_endpoints/mcp_management_endpoints.py | Removes user_api_key_auth dependency from mcp_authorize and mcp_token routes and always passes None as the user_api_key_dict, bypassing all per-server access-control checks for permanent configured servers on these endpoints. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Adds four new mock-only tests covering: non-challenge path well-known discovery, issuer-path candidate URL resolution for Azure, Azure metadata derivation fallback, and sovereign cloud (.us/.cn) variants. All use mocks, no real network calls. |
| tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py | Removes admin_auth from three existing test invocations to match the new unauthenticated signature; adds test_mcp_authorize_and_token_routes_are_browser_callable to verify neither route carries user_api_key_auth as a dependency. |
Sequence Diagram
sequenceDiagram
participant Browser
participant LiteLLM as LiteLLM MCP Proxy
participant MCP as MCP Server
participant Azure as Azure Entra (OIDC)
Browser->>LiteLLM: GET /server/oauth/{id}/authorize
LiteLLM->>MCP: GET server_url (discovery probe)
alt Server challenges (4xx + WWW-Authenticate)
MCP-->>LiteLLM: 401 + WWW-Authenticate: resource_metadata=...
LiteLLM->>MCP: GET .well-known/oauth-protected-resource
MCP-->>LiteLLM: {authorization_servers: [azure_issuer]}
else Server does not challenge (200 OK) — NEW PATH
MCP-->>LiteLLM: 200 OK
LiteLLM->>MCP: _attempt_well_known_discovery(server_url)
MCP-->>LiteLLM: (authorization_servers, scopes)
end
LiteLLM->>Azure: _fetch_single_authorization_server_metadata(issuer)
note over LiteLLM,Azure: New candidate: {issuer}/.well-known/openid-configuration
Azure-->>LiteLLM: OIDC metadata OR 404
alt All discovery fails — NEW FALLBACK
LiteLLM->>LiteLLM: _build_azure_authorization_server_metadata(parsed_issuer)
note over LiteLLM: Constructs endpoints for known Azure authority hosts
end
LiteLLM-->>Browser: 302 redirect to Azure authorize endpoint
Browser->>Azure: Authorization code flow
Azure-->>Browser: code + state
Browser->>LiteLLM: POST /server/oauth/{id}/token (code)
LiteLLM->>Azure: Exchange code for token
Azure-->>LiteLLM: access_token
LiteLLM-->>Browser: access_token
Reviews (3): Last reviewed commit: "Fix greptile review" | Re-trigger Greptile
- google-gemini/gemini-cli#26016 docs: contribution-guide link fixes (merge-as-is) - google-gemini/gemini-cli#25974 fix: file-loaded theme lookup by internal name (merge-after-nits) - BerriAI/litellm#26575 OAuth discovery extension + route deauthing (needs-discussion) - aaif-goose/goose#8856 chore: dependabot auto-merge workflow (merge-after-nits)
| @@ -1497,12 +1499,10 @@ async def _get_cached_temporary_mcp_server_or_404( | |||
| @router.get( | |||
| "/server/oauth/{server_id}/authorize", | |||
| include_in_schema=False, | |||
There was a problem hiding this comment.
High: Authentication removed from OAuth authorize endpoint
Removing dependencies=[Depends(user_api_key_auth)] makes this endpoint callable by unauthenticated users. Combined with passing None to _get_cached_temporary_mcp_server_or_404, the authorization policy is entirely bypassed — an unauthenticated attacker can enumerate server IDs and trigger OAuth authorization flows using the proxy's stored client_id, gaining access to upstream provider OAuth flows they shouldn't reach.
If this endpoint genuinely needs to be browser-callable (OAuth redirect), it still needs some form of validation — e.g. verifying the server_id is a public/allowed server, or gating on a session token. At minimum, don't pass None for the auth dict; that disables the allowed-servers check.
| @@ -1542,12 +1542,10 @@ async def mcp_authorize( | |||
| @router.post( | |||
| "/server/oauth/{server_id}/token", | |||
| include_in_schema=False, | |||
There was a problem hiding this comment.
High: Authentication removed from OAuth token endpoint
This endpoint now accepts unauthenticated requests and exchanges authorization codes or refresh tokens against upstream OAuth providers using the MCP server's stored client_id and client_secret. An attacker who obtains a valid authorization code (e.g. via the now-unauthenticated authorize endpoint, or any other code theft vector) can exchange it for upstream access tokens through this proxy without any identity verification.
The /register endpoint on the same router still requires auth — this inconsistency suggests the removal here is unintentional or at least needs a compensating control.
| # admin-only `/server/oauth/session` setup flow and are not exposed | ||
| # to non-admins. | ||
| if not _user_has_admin_view(user_api_key_dict): | ||
| if user_api_key_dict is not None and not _user_has_admin_view( |
There was a problem hiding this comment.
High: Authorization check skipped when user_api_key_dict is None
The guard if user_api_key_dict is not None was added to support the unauthenticated callers, but it means passing None silently bypasses all access controls — the temp-cache restriction, the admin check, and the allowed-servers check. This turns server lookup into an unauthenticated operation for any caller that passes None. If these endpoints must be unauthenticated, the access policy should enforce a positive allowlist (e.g. only servers marked as public OAuth) rather than disabling the check entirely.
|
All review comments resolved — no active issues remaining. Status: 0 open Posted by Veria AI · 2026-04-27T07:58:13.988Z |
| # admin-only `/server/oauth/session` setup flow and are not exposed | ||
| # to non-admins. | ||
| if not _user_has_admin_view(user_api_key_dict): | ||
| if user_api_key_dict is not None and not _user_has_admin_view( |
There was a problem hiding this comment.
High: Authorization bypass via null user_api_key_dict
Both mcp_authorize and mcp_token now pass None for user_api_key_dict, which causes this entire authorization block to be skipped. Any caller (unauthenticated for /authorize, any authenticated user for /token) can access any MCP server by ID or name, including admin-only temporary-cached servers that would otherwise be restricted to PROXY_ADMIN users. An authenticated non-admin user can exchange authorization codes for servers outside their allowed-servers set, and unauthenticated users can initiate OAuth flows for any configured server.
If the intent is to make these endpoints browser-callable (no bearer token), you still need to enforce server-level access control — for instance by validating the session or limiting which server_id values are reachable without auth.
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes