mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
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>
This commit is contained in:
parent
5da590a3c5
commit
8a1c6e49fb
5 changed files with 80 additions and 8 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
4
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
4
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -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).
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue