mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(ui_sso): resolve highest privilege Entra app role, not first in claim
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.
This commit is contained in:
parent
d86336a7c6
commit
9f9f8ab164
2 changed files with 157 additions and 84 deletions
|
|
@ -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]:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue