From 8a1c6e49fb97d44528a451b2cf0c266468ac256d Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 21 Sep 2026 22:22:12 +0000 Subject: [PATCH] feat(auth): only allow password-login dashboard sessions to call /user/password/change Password login now stamps login_method=username_password into the UI session key metadata, and change_password rejects any caller that is not a litellm-dashboard key carrying that marker with 403 before the user row is read. SSO sessions and user-associated virtual keys can no longer use the endpoint as a current_password guessing oracle. The forced-reset session is still minted by the password login path, so it keeps access to the endpoint Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/proxy/auth/login_utils.py | 6 ++- .../password_endpoints.py | 23 ++++++++- .../proxy/auth/test_login_utils.py | 6 +-- .../test_password_endpoints.py | 49 ++++++++++++++++++- ui/litellm-dashboard/src/lib/http/schema.d.ts | 4 +- 5 files changed, 80 insertions(+), 8 deletions(-) diff --git a/litellm/proxy/auth/login_utils.py b/litellm/proxy/auth/login_utils.py index 0dddb16b531..4c2b5d3d0fe 100644 --- a/litellm/proxy/auth/login_utils.py +++ b/litellm/proxy/auth/login_utils.py @@ -58,6 +58,7 @@ if TYPE_CHECKING: BREACH_RECHECK_INTERVAL: Final = timedelta(hours=24) PASSWORD_RESET_ALLOWED_ROUTES: Final = ("/user/password/change",) +PASSWORD_SESSION_METADATA: Final = MappingProxyType({"login_method": "username_password"}) def _breach_recheck_due(last_breach_check_at: datetime | None) -> bool: @@ -431,7 +432,10 @@ async def _sign_in( user_id=user_id, team_id="litellm-dashboard", allowed_routes=list(PASSWORD_RESET_ALLOWED_ROUTES) if password_reset_required else None, - metadata={"password_reset_required": True} if password_reset_required else {}, + metadata={ + **PASSWORD_SESSION_METADATA, + **({"password_reset_required": True} if password_reset_required else {}), + }, ) else: raise ProxyException( diff --git a/litellm/proxy/management_endpoints/password_endpoints.py b/litellm/proxy/management_endpoints/password_endpoints.py index c94399ba25d..03a8b4c4010 100644 --- a/litellm/proxy/management_endpoints/password_endpoints.py +++ b/litellm/proxy/management_endpoints/password_endpoints.py @@ -11,9 +11,11 @@ signal is emitted by hand below, with field names only, never values. from typing import TYPE_CHECKING, Annotated, Final from fastapi import APIRouter, Depends, HTTPException +from pydantic import TypeAdapter from litellm._logging import verbose_proxy_logger from litellm.proxy._types import ( + UI_TEAM_ID, ChangePasswordRequest, ChangePasswordResponse, CommonProxyErrors, @@ -21,6 +23,7 @@ from litellm.proxy._types import ( LitellmTableNames, UserAPIKeyAuth, ) +from litellm.proxy.auth.login_utils import PASSWORD_SESSION_METADATA from litellm.proxy.auth.password_policy import validate_password_not_breached, validate_password_policy from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.management_helpers.audit_logs import create_object_audit_log @@ -37,6 +40,7 @@ if TYPE_CHECKING: router: Final = APIRouter() _PASSWORD_CHANGED_AUDIT_VALUES: Final = '{"fields_changed": ["password"]}' +_KEY_METADATA: Final = TypeAdapter(dict[str, object]) def _error_detail(message: str) -> HTTPExceptionErrorDetail: @@ -44,6 +48,13 @@ def _error_detail(message: str) -> HTTPExceptionErrorDetail: return detail +def _is_password_login_session(user_api_key_dict: UserAPIKeyAuth) -> bool: + if user_api_key_dict.team_id != UI_TEAM_ID: + return False + key_metadata: Final = _KEY_METADATA.validate_python(user_api_key_dict.metadata) + return all(key_metadata.get(k) == v for k, v in PASSWORD_SESSION_METADATA.items()) + + def _user_table( prisma_client: "PrismaClient | None", ) -> "TableActions[prisma_models.LiteLLM_UserTable]": @@ -63,7 +74,9 @@ async def change_password( """ Change the calling user's own password. - Requires the current password. The new password must differ from the + Only callable with the dashboard session issued by a username/password + login; SSO sessions and virtual keys are rejected with 403. Requires the + current password. The new password must differ from the current one and satisfy the configured password policy (`general_settings.password_policy_*`: minimum length, character classes, and, when enabled, breached-password screening via haveibeenpwned.com). @@ -82,6 +95,14 @@ async def change_password( detail=_error_detail(CommonProxyErrors.db_not_connected_error.value), ) + if not _is_password_login_session(user_api_key_dict): + raise HTTPException( + status_code=403, + detail=_error_detail( + "Passwords can only be changed from a dashboard session created by logging in with a password." + ), + ) + user_id: Final = user_api_key_dict.user_id if user_id is None: raise HTTPException( diff --git a/tests/test_litellm/proxy/auth/test_login_utils.py b/tests/test_litellm/proxy/auth/test_login_utils.py index ada231a3e95..3786169c320 100644 --- a/tests/test_litellm/proxy/auth/test_login_utils.py +++ b/tests/test_litellm/proxy/auth/test_login_utils.py @@ -2138,7 +2138,7 @@ class TestPasswordResetRequiredSessionMinting: result, key_kwargs = await self._login(_prisma_with_user(row)) assert key_kwargs["allowed_routes"] == ["/user/password/change"] - assert key_kwargs["metadata"] == {"password_reset_required": True} + assert key_kwargs["metadata"] == {"login_method": "username_password", "password_reset_required": True} assert result.password_reset_required is True @pytest.mark.asyncio @@ -2147,7 +2147,7 @@ class TestPasswordResetRequiredSessionMinting: result, key_kwargs = await self._login(_prisma_with_user(row)) assert key_kwargs["allowed_routes"] is None - assert not key_kwargs["metadata"] + assert key_kwargs["metadata"] == {"login_method": "username_password"} assert result.password_reset_required is False async def _login_with_screen_result(self, mock_prisma_client, breached: bool) -> tuple[LoginResult, dict, dict]: @@ -2199,7 +2199,7 @@ class TestPasswordResetRequiredSessionMinting: result, key_kwargs, _ = await self._login_with_screen_result(mock_prisma_client, breached=True) assert key_kwargs["allowed_routes"] == ["/user/password/change"] - assert key_kwargs["metadata"] == {"password_reset_required": True} + assert key_kwargs["metadata"] == {"login_method": "username_password", "password_reset_required": True} assert result.password_reset_required is True diff --git a/tests/test_litellm/proxy/management_endpoints/test_password_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_password_endpoints.py index 581a2ef8df8..c04353fec99 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_password_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_password_endpoints.py @@ -12,7 +12,8 @@ import pytest import respx from fastapi import HTTPException -from litellm.proxy._types import LitellmTableNames, ProxyErrorTypes, ProxyException, UserAPIKeyAuth +from litellm.proxy._types import UI_TEAM_ID, LitellmTableNames, ProxyErrorTypes, ProxyException, UserAPIKeyAuth +from litellm.proxy.auth.login_utils import PASSWORD_SESSION_METADATA from litellm.proxy.management_endpoints.password_endpoints import change_password from litellm.proxy.utils import hash_password, verify_password @@ -37,7 +38,15 @@ def _make_prisma(user: MagicMock | None) -> MagicMock: def _caller(user_id: str | None = "user-123") -> UserAPIKeyAuth: - return UserAPIKeyAuth(user_id=user_id) + return UserAPIKeyAuth(user_id=user_id, team_id=UI_TEAM_ID, metadata=dict(PASSWORD_SESSION_METADATA)) + + +def _sso_session_caller() -> UserAPIKeyAuth: + return UserAPIKeyAuth(user_id="user-123", team_id=UI_TEAM_ID, metadata={}) + + +def _virtual_key_caller() -> UserAPIKeyAuth: + return UserAPIKeyAuth(user_id="user-123", team_id="team-abc", metadata=dict(PASSWORD_SESSION_METADATA)) def _hibp_url_for(password: str) -> str: @@ -130,6 +139,42 @@ async def test_change_password_rejects_unchanged_password(): prisma.db.litellm_usertable.update.assert_not_called() +@pytest.mark.asyncio +@pytest.mark.parametrize( + "caller", + [ + pytest.param(_sso_session_caller(), id="sso_dashboard_session"), + pytest.param(_virtual_key_caller(), id="virtual_key_with_forged_metadata"), + ], +) +async def test_change_password_rejects_non_password_login_session(caller: UserAPIKeyAuth): + """Only the session minted by a password login may change the password, so a + stolen virtual key or an SSO session cannot use the endpoint as a + current_password guessing oracle.""" + from litellm.proxy._types import ChangePasswordRequest + + prisma = _make_prisma(_make_user_row(hash_password(CURRENT_PASSWORD))) + + with ( + patch( # test-quality-ok: change_password reads proxy_server module globals; no injection seam + "litellm.proxy.proxy_server.prisma_client", prisma + ), + patch( # test-quality-ok: change_password reads proxy_server module globals; no injection seam + "litellm.proxy.proxy_server.general_settings", _POLICY_NO_BREACH_CHECK + ), + ): + with pytest.raises(HTTPException) as exc_info: + await change_password( + data=ChangePasswordRequest(current_password=CURRENT_PASSWORD, new_password=NEW_PASSWORD), + user_api_key_dict=caller, + ) + + assert exc_info.value.status_code == 403 + assert "logging in with a password" in exc_info.value.detail["error"] + prisma.db.litellm_usertable.find_first.assert_not_called() + prisma.db.litellm_usertable.update.assert_not_called() + + @pytest.mark.asyncio async def test_change_password_rejects_session_without_user(): from litellm.proxy._types import ChangePasswordRequest diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 93753d29993..84e3270c48b 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -17386,7 +17386,9 @@ export interface paths { * Change Password * @description Change the calling user's own password. * - * Requires the current password. The new password must differ from the + * Only callable with the dashboard session issued by a username/password + * login; SSO sessions and virtual keys are rejected with 403. Requires the + * current password. The new password must differ from the * current one and satisfy the configured password policy * (`general_settings.password_policy_*`: minimum length, character classes, * and, when enabled, breached-password screening via haveibeenpwned.com).