mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-05 02:41:56 +00:00
fix: store hashed password on /user/new when password field is provided
Two bugs existed with password handling on /user/new: 1. NewUserRequest did not declare a `password` field, so any password supplied by the caller was silently discarded by Pydantic before the endpoint handler even ran. 2. generate_key_helper_fn does not accept a `password` parameter and the user_data dict it builds does not include it, so the password would never reach the database even if it were deserialized. Fix: add `password: Optional[str] = None` to NewUserRequest, then in new_user() pop the (already hashed) password from data_json before forwarding to generate_key_helper_fn, and persist it via a targeted litellm_usertable.update() call after the user row is created. The /user/update path already had _hash_password_in_dict() and is unaffected by this change. Fixes https://github.com/BerriAI/litellm/issues/25328 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
62757ff48f
commit
e63a7246ed
3 changed files with 111 additions and 0 deletions
|
|
@ -1425,6 +1425,7 @@ class NewUserRequest(GenerateRequestBase):
|
|||
send_invite_email: Optional[bool] = None
|
||||
sso_user_id: Optional[str] = None
|
||||
organizations: Optional[List[str]] = None
|
||||
password: Optional[str] = None
|
||||
|
||||
|
||||
class NewUserResponse(GenerateKeyResponse):
|
||||
|
|
|
|||
|
|
@ -455,6 +455,10 @@ async def new_user(
|
|||
data_json = data.json() # type: ignore
|
||||
data_json = _update_internal_new_user_params(data_json, data)
|
||||
_hash_password_in_dict(data_json)
|
||||
# Pop password before passing to generate_key_helper_fn — the function
|
||||
# does not accept a password parameter and user_data does not include it.
|
||||
# We store it via a targeted update after the user row is created.
|
||||
hashed_password = data_json.pop("password", None)
|
||||
teams = data.teams
|
||||
if teams is None:
|
||||
teams = check_if_default_team_set()
|
||||
|
|
@ -463,6 +467,15 @@ async def new_user(
|
|||
)
|
||||
|
||||
response = await generate_key_helper_fn(request_type="user", **data_json)
|
||||
|
||||
# Store the hashed password on the newly created user row.
|
||||
if hashed_password is not None:
|
||||
created_user_id = response.get("user_id")
|
||||
if created_user_id is not None:
|
||||
await prisma_client.db.litellm_usertable.update(
|
||||
where={"user_id": created_user_id},
|
||||
data={"password": hashed_password},
|
||||
)
|
||||
# Admin UI Logic
|
||||
# Add User to Team and Organization
|
||||
# if team_id passed add this user to the team
|
||||
|
|
|
|||
|
|
@ -2516,3 +2516,100 @@ class TestGetUserIdFromRequestValidation:
|
|||
request = self._make_request(f"user_id={exact_id}")
|
||||
result = get_user_id_from_request(request)
|
||||
assert result == exact_id
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_new_user_password_is_hashed_and_stored(mocker):
|
||||
"""
|
||||
/user/new with a password field must:
|
||||
1. Hash the password (not store plaintext).
|
||||
2. Actually persist the hashed password on the user row.
|
||||
|
||||
Regression test for https://github.com/BerriAI/litellm/issues/25328
|
||||
"""
|
||||
from litellm.proxy._types import NewUserRequest, UserAPIKeyAuth
|
||||
from litellm.proxy.management_endpoints.internal_user_endpoints import new_user
|
||||
from litellm.proxy.utils import verify_password
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
|
||||
async def mock_count(*args, **kwargs):
|
||||
return 5
|
||||
|
||||
mock_prisma_client.db.litellm_usertable.count = mock_count
|
||||
|
||||
async def mock_no_duplicate(*args, **kwargs):
|
||||
return None
|
||||
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.internal_user_endpoints._check_duplicate_user_email",
|
||||
mock_no_duplicate,
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.internal_user_endpoints._check_duplicate_user_id",
|
||||
mock_no_duplicate,
|
||||
)
|
||||
|
||||
mock_license_check = mocker.MagicMock()
|
||||
mock_license_check.is_over_limit.return_value = False
|
||||
|
||||
created_user_id = "test-user-password-123"
|
||||
|
||||
mock_generate_key = mocker.AsyncMock(
|
||||
return_value={
|
||||
"user_id": created_user_id,
|
||||
"token": "sk-test-token",
|
||||
"expires": None,
|
||||
"max_budget": None,
|
||||
}
|
||||
)
|
||||
|
||||
# Capture the data passed to litellm_usertable.update
|
||||
update_calls: list = []
|
||||
|
||||
async def mock_update(where, data):
|
||||
update_calls.append({"where": where, "data": data})
|
||||
|
||||
mock_prisma_client.db.litellm_usertable.update = mock_update
|
||||
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.internal_user_endpoints.UserManagementEventHooks.async_user_created_hook",
|
||||
mocker.AsyncMock(),
|
||||
)
|
||||
|
||||
mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)
|
||||
mocker.patch("litellm.proxy.proxy_server._license_check", mock_license_check)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.internal_user_endpoints.generate_key_helper_fn",
|
||||
mock_generate_key,
|
||||
)
|
||||
|
||||
plaintext_password = "super_secret_pass"
|
||||
user_request = NewUserRequest(
|
||||
user_id=created_user_id,
|
||||
user_email="newuser@example.com",
|
||||
password=plaintext_password,
|
||||
)
|
||||
mock_user_key = UserAPIKeyAuth(user_id="admin", user_role="proxy_admin")
|
||||
|
||||
await new_user(data=user_request, user_api_key_dict=mock_user_key)
|
||||
|
||||
# Password must NOT be passed to generate_key_helper_fn
|
||||
call_kwargs = mock_generate_key.call_args.kwargs
|
||||
assert "password" not in call_kwargs, (
|
||||
"password should be popped before calling generate_key_helper_fn"
|
||||
)
|
||||
|
||||
# Password must be stored via a separate update call
|
||||
assert len(update_calls) == 1, (
|
||||
f"Expected exactly 1 usertable.update call for password, got {len(update_calls)}"
|
||||
)
|
||||
stored_password = update_calls[0]["data"]["password"]
|
||||
|
||||
# Must NOT be stored as plaintext
|
||||
assert stored_password != plaintext_password, "Password must not be stored as plaintext"
|
||||
|
||||
# Must be verifiable
|
||||
assert verify_password(plaintext_password, stored_password), (
|
||||
"Stored password hash must verify against the original plaintext"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue