From 9f9f8ab164a715b87285aa73dd959a5a9d012b37 Mon Sep 17 00:00:00 2001 From: Imran Ismail Date: Thu, 13 Aug 2026 15:59:14 +1200 Subject: [PATCH] fix(ui_sso): resolve highest privilege Entra app role, not first in claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- litellm/proxy/management_endpoints/ui_sso.py | 80 +++++++-- .../test_entraid_app_roles.py | 161 +++++++++++------- 2 files changed, 157 insertions(+), 84 deletions(-) diff --git a/litellm/proxy/management_endpoints/ui_sso.py b/litellm/proxy/management_endpoints/ui_sso.py index a2c50590dd5..75ce0862e9a 100644 --- a/litellm/proxy/management_endpoints/ui_sso.py +++ b/litellm/proxy/management_endpoints/ui_sso.py @@ -806,6 +806,17 @@ def normalize_email(email: str | None) -> str | None: return email.lower() if isinstance(email, str) else email +# Role hierarchy (highest to lowest privilege). Shared by every code path that has +# to collapse several candidate roles down to the single role LiteLLM stores on a +# user, so group-based and app-role-based logins resolve the same way. +LITELLM_USER_ROLE_HIERARCHY: Final = [ + LitellmUserRoles.PROXY_ADMIN, + LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, + LitellmUserRoles.INTERNAL_USER, + LitellmUserRoles.INTERNAL_USER_VIEW_ONLY, +] + + def determine_role_from_groups( user_groups: list[str], role_mappings: "RoleMappings", @@ -830,19 +841,11 @@ def determine_role_from_groups( # No role mappings configured, return default_role return role_mappings.default_role - # Role hierarchy (highest to lowest) - role_hierarchy: Final = [ - LitellmUserRoles.PROXY_ADMIN, - LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, - LitellmUserRoles.INTERNAL_USER, - LitellmUserRoles.INTERNAL_USER_VIEW_ONLY, - ] - # Convert user_groups to a set for efficient lookup user_groups_set: Final = set(user_groups) if isinstance(user_groups, list) else set() # Find the highest privilege role the user belongs to - for role in role_hierarchy: + for role in LITELLM_USER_ROLE_HIERARCHY: if role in role_mappings.roles: role_groups = role_mappings.roles[role] if isinstance(role_groups, list) and user_groups_set.intersection(set(role_groups)): @@ -4151,15 +4154,7 @@ class MicrosoftSSOHandler: verbose_proxy_logger.debug("Extracted app roles from id_token: %s", app_roles) # Combine groups and app roles - user_role: LitellmUserRoles | None = None - if app_roles: - # Check if any app role is a valid LitellmUserRoles - for role_str in app_roles: - role = get_litellm_user_role(role_str) - if role is not None: - user_role = role - verbose_proxy_logger.debug("Found valid LitellmUserRoles '%s' in app_roles", role.value) - break + user_role: Final = MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) verbose_proxy_logger.debug("Combined team_ids (groups + app roles): %s", user_team_ids) @@ -4197,6 +4192,55 @@ class MicrosoftSSOHandler: verbose_proxy_logger.debug("Microsoft SSO OpenID Response: %s", openid_response) return openid_response + @staticmethod + def get_user_role_from_app_roles( + app_roles: list[str] | None, + ) -> LitellmUserRoles | None: + """ + Pick the LiteLLM role for a user from the Entra app roles on their id_token. + + A user assigned several app roles (directly, or by being a member of more + than one assigned group) arrives here with every role in the `roles` claim, + in an order Entra does not guarantee. LiteLLM stores a single role per user, + so these have to be collapsed to one: resolve each claim value against + LitellmUserRoles and keep the highest privilege match, so the result does + not depend on claim ordering. Unrecognised values are ignored. + + Args: + app_roles: App role values from the id_token's `roles`/`app_roles` claim + + Returns: + The highest privilege role present, or None if the claim held no + recognised role (callers then fall back to the user's stored role or + the configured default). + """ + if not app_roles: + return None + + resolved: Final = { + role for role in (get_litellm_user_role(role_str) for role_str in app_roles) if role is not None + } + if not resolved: + verbose_proxy_logger.debug("No valid LitellmUserRoles found in app_roles: %s", app_roles) + return None + + for role in LITELLM_USER_ROLE_HIERARCHY: + if role in resolved: + verbose_proxy_logger.debug( + "Selected highest privilege LitellmUserRoles '%s' from app_roles: %s", role.value, app_roles + ) + return role + + # Every resolved role is a valid LitellmUserRoles that the hierarchy above + # does not rank (org_admin, team, customer). There is no privilege ordering + # defined for those, so pick deterministically instead of depending on the + # order Entra happened to emit. + fallback: Final = min(resolved, key=lambda role: role.value) + verbose_proxy_logger.debug( + "app_roles %s resolved to non-hierarchy role(s); using '%s'", app_roles, fallback.value + ) + return fallback + @staticmethod def get_app_roles_from_id_token(id_token: str | None) -> list[str]: """ diff --git a/tests/test_litellm/proxy/management_endpoints/test_entraid_app_roles.py b/tests/test_litellm/proxy/management_endpoints/test_entraid_app_roles.py index 2ce36b73de0..0c3fe175b48 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_entraid_app_roles.py +++ b/tests/test_litellm/proxy/management_endpoints/test_entraid_app_roles.py @@ -1,91 +1,120 @@ import jwt +import pytest -from litellm.proxy.management_endpoints.ui_sso import MicrosoftSSOHandler -from litellm.proxy.management_endpoints.types import get_litellm_user_role from litellm.proxy._types import LitellmUserRoles +from litellm.proxy.management_endpoints.ui_sso import MicrosoftSSOHandler + + +def _id_token(**claims) -> str: + """Build a signed id_token carrying the given claims.""" + payload = { + "sub": "user123", + "email": "user@company.com", + "aud": "litellm-app", + "iss": "https://login.microsoftonline.com/tenant-id/v2.0", + "exp": 9999999999, + **claims, + } + return jwt.encode(payload, "secret", algorithm="HS256") def test_extracts_proxy_admin_role_from_jwt(): """Ensure supported app roles like 'proxy_admin' are extracted from the id_token.""" - payload = { - "sub": "user123", - "email": "admin@company.com", - "app_roles": ["proxy_admin"], - "aud": "litellm-app", - "iss": "https://login.microsoftonline.com/tenant-id/v2.0", - "exp": 9999999999, - } + token = _id_token(app_roles=["proxy_admin"]) - token = jwt.encode(payload, "secret", algorithm="HS256") roles = MicrosoftSSOHandler.get_app_roles_from_id_token(token) assert roles == ["proxy_admin"] -def test_maps_internal_user_role(): - """Ensure internal_user role is correctly mapped to LitellmUserRoles.""" - payload = { - "sub": "user456", - "email": "user@company.com", - "app_roles": ["internal_user"], - "aud": "litellm-app", - "iss": "https://login.microsoftonline.com/tenant-id/v2.0", - "exp": 9999999999, - } +def test_extracts_app_roles_from_roles_claim(): + """Entra emits app role values in the `roles` claim; both spellings are read.""" + token = _id_token(roles=["internal_user"]) - token = jwt.encode(payload, "secret", algorithm="HS256") roles = MicrosoftSSOHandler.get_app_roles_from_id_token(token) - # Map to LitellmUserRoles - chosen = None - for r in roles: - mapped = get_litellm_user_role(r) - if mapped is not None: - chosen = mapped - break - - assert chosen == LitellmUserRoles.INTERNAL_USER + assert roles == ["internal_user"] -def test_maps_proxy_admin_viewer_role(): - """Ensure proxy_admin_viewer role is correctly mapped.""" - payload = { - "sub": "user789", - "email": "viewer@company.com", - "app_roles": ["proxy_admin_viewer"], - "aud": "litellm-app", - "iss": "https://login.microsoftonline.com/tenant-id/v2.0", - "exp": 9999999999, - } - - token = jwt.encode(payload, "secret", algorithm="HS256") - roles = MicrosoftSSOHandler.get_app_roles_from_id_token(token) - - chosen = None - for r in roles: - mapped = get_litellm_user_role(r) - if mapped is not None: - chosen = mapped - break - - assert chosen == LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY +@pytest.mark.parametrize( + "app_roles, expected", + [ + (["proxy_admin"], LitellmUserRoles.PROXY_ADMIN), + (["proxy_admin_viewer"], LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY), + (["internal_user"], LitellmUserRoles.INTERNAL_USER), + (["internal_user_viewer"], LitellmUserRoles.INTERNAL_USER_VIEW_ONLY), + # Case-insensitive, matching get_litellm_user_role. + (["PROXY_ADMIN_VIEWER"], LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY), + # Roles outside the privilege hierarchy still resolve. + (["org_admin"], LitellmUserRoles.ORG_ADMIN), + ], +) +def test_maps_single_app_role(app_roles, expected): + """A lone app role maps to its LitellmUserRoles equivalent.""" + assert MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) == expected -def test_defaults_to_internal_user_viewer_when_no_role(): - """Ensure default role is internal_user_viewer when no app role is present.""" - payload = { - "sub": "user_no_role", - "email": "noRole@company.com", - "aud": "litellm-app", - "iss": "https://login.microsoftonline.com/tenant-id/v2.0", - "exp": 9999999999, - } +@pytest.mark.parametrize( + "app_roles", + [ + ["internal_user", "proxy_admin_viewer"], + ["proxy_admin_viewer", "internal_user"], + ], +) +def test_highest_privilege_role_wins_regardless_of_claim_order(app_roles): + """ + A user in one group mapped to `internal_user` and another mapped to + `proxy_admin_viewer` gets the higher privilege role either way. + + Entra does not guarantee the ordering of the `roles` claim, so the resolved + role must not depend on it. + """ + assert MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) == (LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY) + + +@pytest.mark.parametrize( + "app_roles", + [ + ["internal_user", "proxy_admin_viewer", "proxy_admin"], + ["proxy_admin", "proxy_admin_viewer", "internal_user"], + ["proxy_admin_viewer", "internal_user", "proxy_admin"], + ], +) +def test_proxy_admin_beats_every_other_role(app_roles): + """proxy_admin outranks every other role in the hierarchy, in any claim order.""" + assert MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) == LitellmUserRoles.PROXY_ADMIN + + +def test_unrecognised_app_roles_are_ignored(): + """App roles that are not LitellmUserRoles values do not shadow ones that are.""" + app_roles = ["Some.Custom.Role", "msiam_access", "internal_user"] + + assert MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) == LitellmUserRoles.INTERNAL_USER + + +@pytest.mark.parametrize("app_roles", [None, [], ["msiam_access"], ["User"]]) +def test_returns_none_when_no_role_resolves(app_roles): + """ + Returning None lets the caller keep the user's stored role or apply + default_internal_user_params, rather than forcing a role. + """ + assert MicrosoftSSOHandler.get_user_role_from_app_roles(app_roles) is None + + +def test_no_role_claim_yields_no_app_roles(): + """An id_token with no role claim produces no app roles, and so no role.""" + token = _id_token() - token = jwt.encode(payload, "secret", algorithm="HS256") roles = MicrosoftSSOHandler.get_app_roles_from_id_token(token) assert roles == [] + assert MicrosoftSSOHandler.get_user_role_from_app_roles(roles) is None - # Default role would be internal_user_viewer - default_role = LitellmUserRoles.INTERNAL_USER_VIEW_ONLY - assert default_role.value == "internal_user_viewer" + +def test_end_to_end_from_id_token_to_role(): + """The id_token -> role path resolves the highest privilege role.""" + token = _id_token(roles=["internal_user", "proxy_admin_viewer"]) + + roles = MicrosoftSSOHandler.get_app_roles_from_id_token(token) + + assert MicrosoftSSOHandler.get_user_role_from_app_roles(roles) == LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY