Skip to content

fix(ui_sso): resolve highest privilege Entra app role, not first in claim - #36728

Merged
yucheng-berri merged 3 commits into
BerriAI:litellm_internal_stagingfrom
imranismail:fix/entra-app-roles-highest-privilege
Aug 27, 2026
Merged

yucheng-berri merged 3 commits into
BerriAI:litellm_internal_stagingfrom
imranismail:fix/entra-app-roles-highest-privilege

Conversation

@imranismail

@imranismail imranismail commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Users with multiple Entra app roles get an arbitrary one
  • Entra does not guarantee roles claim ordering
  • proxy_admin can silently lose to internal_user
  • Only Microsoft SSO is affected; generic/Okta already ranks correctly

How it solves it:

  • Reuse the existing privilege hierarchy for app roles
  • Highest privilege wins, independent of claim order
  • Extract the selection into a testable function
  • Single-role, unknown-role, and empty-claim behaviour unchanged

User Flow

Before: a platform admin who is also a member of a regular team group signs in and lands with read-only access, unable to administer the proxy

  1. The Entra admin creates two app roles on the LiteLLM enterprise application, proxy_admin and internal_user, and assigns one group to each
  2. A user is a member of both groups, the admin group and their own team's group
  3. The user opens https://litellm-domain/ui and completes the Microsoft sign-in
  4. They land on the UI with internal-user access: no Teams or Models administration, and key creation is refused
  5. Whether this happens is luck. Another user with the same two groups, or the same user in a different tenant, may land as proxy_admin instead, because the role that wins depends on the order Entra listed the roles in
  6. The admin removes the user from their team group to work around it, which also removes them from that team

After: the same user signs in and lands with the higher privilege role, keeping their team membership

  1. The Entra admin creates the same two app roles and assigns the same two groups
  2. A user is a member of both groups, the admin group and their own team's group
  3. The user opens https://litellm-domain/ui and completes the Microsoft sign-in
  4. They land on the UI as a proxy admin, with the full administration surface, and remain a member of their team
  5. The result is the same on every sign-in and for every user with that pair of groups, regardless of how Entra ordered the roles

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Before, against a live proxy on v1.95.0

This reproduces on a real tenant, not a constructed claim. The user below is a member of two Entra groups assigned to this app, one mapped to the internal_user app role and one mapped to proxy_admin_viewer. Hostname, tenant, app id and email are redacted; the role values are verbatim.

Minting a real Entra access token for the app and decoding its roles claim shows Entra listing internal_user first:

$ TOKEN=$(<mint an Entra token for the LiteLLM app>)
$ python3 -c '
import base64, json, sys
payload = sys.stdin.read().strip().split(".")[1]
payload += "=" * (-len(payload) % 4)
c = json.loads(base64.urlsafe_b64decode(payload))
print("roles claim :", json.dumps(c.get("roles")))
print("aud         :", c.get("aud"))
' <<< "$TOKEN"
roles claim : ["internal_user", "proxy_admin_viewer"]
aud         : api://<app-id>

The running proxy resolved that user to the lower privilege role, dropping proxy_admin_viewer:

$ curl -sS https://<proxy-host>/user/info -H "Authorization: Bearer $TOKEN"
user_role   : internal_user
user_email  : <redacted>
teams       : 3 team(s)

That is the bug end to end: the user holds proxy_admin_viewer in Entra, the token carries it, and the proxy stored internal_user purely because Entra happened to list it first.

After, same claim through the patched resolver

At f7d1524, the real claim above resolves the other way:

$ uv run python -c '
from litellm.proxy.management_endpoints.ui_sso import MicrosoftSSOHandler
claim = ["internal_user", "proxy_admin_viewer"]
print("claim   :", claim)
print("resolved:", MicrosoftSSOHandler.get_user_role_from_app_roles(claim).value)
'
claim   : ['internal_user', 'proxy_admin_viewer']
resolved: proxy_admin_viewer

Remaining half, the interactive sign-in

The sign-in itself needs a browser, so I cannot capture it headlessly. Steps to reproduce both halves against a local proxy, if you want the UI screenshots:

  1. Add http://localhost:4000/sso/callback to the app registration's redirect URIs
  2. Assign one user two app roles whose values are internal_user and proxy_admin_viewer, via two groups or directly
  3. Export MICROSOFT_CLIENT_ID, MICROSOFT_CLIENT_SECRET, MICROSOFT_TENANT and PROXY_BASE_URL=http://localhost:4000
  4. On d86336a (this PR's merge base), run python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --detailed_debug --reload 2>&1 | tee litellm.log
  5. Open http://localhost:4000/ui, sign in as that user, and expect to land without the admin surface. curl -sS localhost:4000/user/info -H "Authorization: Bearer $TOKEN" reports internal_user
  6. Check out f7d1524, restart the proxy, and sign in again as the same user. Expect the admin viewer surface, and /user/info reporting proxy_admin_viewer

Type

🐛 Bug Fix

Caveats (if any)

  • org_admin, team, customer are unranked by the existing hierarchy
  • Those now resolve deterministically, not by claim order
  • Worth confirming whether org_admin should be ranked

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Note

Medium Risk
Changes how Microsoft SSO persists user roles at login, which directly affects admin UI access; behavior is narrowed and well-tested but any mis-ranking of unlisted roles like org_admin could surprise tenants.

Overview
Fixes Microsoft SSO assigning the first mapped Entra app role in the token instead of the highest privilege role when a user has several app roles—Entra does not guarantee roles / app_roles claim order, so admins could land as internal_user despite also holding proxy_admin or proxy_admin_viewer.

LITELLM_USER_ROLE_HIERARCHY is introduced as a shared ordering (proxy admin → admin viewer → internal user → internal viewer) and wired into determine_role_from_groups so group-based SSO uses the same list. Microsoft callback logic now calls MicrosoftSSOHandler.get_user_role_from_app_roles, which maps all claim values via get_litellm_user_role, picks the top ranked role in that hierarchy, and falls back to a deterministic choice by role name for unranked roles (org_admin, etc.). Unmapped roles are ignored; empty or unresolvable claims still return None so existing defaults apply.

Tests in test_entraid_app_roles.py cover single roles, claim order permutations, unknown roles, and the id_token → role path (including the roles claim).

Reviewed by Cursor Bugbot for commit 2002fee. Bugbot is set up for automated code reviews on this repo. Configure here.

…laim

A user assigned more than one Entra app role — commonly by belonging to
several assigned groups — arrives at the Microsoft SSO callback with every
role in the id_token `roles` claim. LiteLLM stores a single role per user,
and get_microsoft_callback_response collapsed the list by taking the first
value that resolved to a LitellmUserRoles and breaking.

Entra does not guarantee the ordering of the `roles` claim, so which role
won was effectively arbitrary: a user in one group mapped to internal_user
and another mapped to proxy_admin_viewer could be silently demoted to
internal_user, and proxy_admin could lose to either.

The generic/Okta path already resolves this correctly via
determine_role_from_groups, which walks a documented privilege hierarchy.
Hoist that hierarchy into LITELLM_USER_ROLE_HIERARCHY and reuse it, so
app-role logins and group-mapping logins agree.

Extract the selection into MicrosoftSSOHandler.get_user_role_from_app_roles
so it is directly testable — the existing tests re-implemented the loop
inline, which is why the ordering bug was not caught.

Behaviour is unchanged for single-role claims, unrecognised values, and
empty claims. Roles the hierarchy does not rank (org_admin, team, customer)
are resolved deterministically rather than by claim order.
@CLAassistant

CLAassistant commented Aug 13, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes Microsoft SSO app-role resolution independent of Entra claim ordering by selecting the highest recognized role through the existing hierarchy.

  • Extracts app-role selection into a dedicated helper.
  • Reuses the group-role privilege hierarchy for ranked app roles.
  • Adds coverage for claim ordering, recognized and unknown roles, empty claims, and token-to-role resolution.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
litellm/proxy/management_endpoints/ui_sso.py Centralizes the role hierarchy and uses it to resolve Microsoft SSO app roles deterministically; the previously reported commentary issue is resolved.
tests/test_litellm/proxy/management_endpoints/test_entraid_app_roles.py Expands focused unit coverage for app-role extraction and deterministic privilege resolution.

Reviews (2): Last reviewed commit: "Merge branch 'BerriAI:litellm_internal_s..." | Re-trigger Greptile

Comment thread litellm/proxy/management_endpoints/ui_sso.py Outdated
@codecov

codecov Bot commented Aug 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…ions

Addresses review feedback on the app role selection helper.

Drop the explanatory comments and the Args/Returns docstring boilerplate that
restated the control flow, keeping only the part a reader cannot infer from the
code: that Entra does not guarantee claim ordering, and how unranked roles
resolve.

Type the parameter as Sequence[str] rather than list[str] and build the resolved
set as a frozenset, so the helper stops adding an LIT001 mutable-collection
annotation. Make LITELLM_USER_ROLE_HIERARCHY a tuple for the same reason.

No behaviour change: the ordering regression tests still fail against the
previous first-match-wins logic and pass here.
@codspeed

codspeed Bot commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing imranismail:fix/entra-app-roles-highest-privilege (2002fee) with litellm_internal_staging (852368d)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (0b82b08) during the generation of this report, so 852368d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@imranismail

Copy link
Copy Markdown
Contributor Author

lint is green now: the LIT001 hit was my app_roles: list[str] | None annotation, so the helper takes a Sequence[str] and builds a frozenset, and LITELLM_USER_ROLE_HIERARCHY is a tuple. scripts/type_discipline_gate.py --base d86336a passes locally. No budget ceilings moved, since the branch doesn't fix any pre-existing violations

The one remaining red check, misc / Run tests, is not from this PR. It's tests/test_litellm/interactions/test_openapi_compliance.py::TestRequestCompliance::test_turn_schema failing with KeyError: 'Turn', and it fails the same way on d86336a, this PR's merge base, with none of my changes applied:

$ git worktree add /tmp/litellm-base d86336a
$ uv run pytest "tests/test_litellm/interactions/test_openapi_compliance.py::TestRequestCompliance::test_turn_schema" -q
E       KeyError: 'Turn'
tests/test_litellm/interactions/test_openapi_compliance.py:172: KeyError
FAILED tests/test_litellm/interactions/test_openapi_compliance.py::TestRequestCompliance::test_turn_schema
1 failed in 1.64s

Nothing in this PR touches the interactions surface or the OpenAPI spec, so I've left it alone rather than papering over it here

@yucheng-berri

Copy link
Copy Markdown
Contributor

@greptileai review latest head

@yucheng-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2002fee. Configure here.

@yucheng-berri
yucheng-berri merged commit 02dcc4d into BerriAI:litellm_internal_staging Aug 27, 2026
73 checks passed
@yucheng-berri

Copy link
Copy Markdown
Contributor

Thanks for the contribution. LGTM

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