fix(auth): screen bulk-update passwords concurrently before any db write

/user/bulk_update awaited a separate HIBP lookup for each user in the
batch, so a degraded-slow HIBP (5s timeout per lookup) could stretch a
500-user batch to ~2500s and time out the request after some updates
had already persisted.

validate_passwords_bulk dedupes the batch's passwords, strength-checks
first, then fires every needed HIBP lookup concurrently, bounding the
worst case at one 5s timeout window. bulk_update_processed_users now
screens the whole batch before the serial update loop, so a rejected
password fails only its own entry and validation failures precede any
persistence.
This commit is contained in:
Oliver Jensen 2026-09-08 16:27:06 +02:00 committed by GitHub
parent e08f697d95
commit 332826e4cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 390 additions and 384 deletions

View file

@ -2019,4 +2019,4 @@ BATCH_ENQUEUED_TOKEN_LIMIT_METADATA_KEY: Final = "batch_enqueued_token_limit"
EMPTY_MAPPING: Final = MappingProxyType({})
# API endpoint for breached password k-anonymity search
HIBP_RANGE_API_BASE: Final = "https://api.pwnedpasswords.com/range"
HIBP_RANGE_API_BASE: Final = "https://api.pwnedpasswords.com/range"

View file

@ -11,9 +11,11 @@ of the password's SHA-1 hash ever leave the proxy, and the check fails open
(allows the password) when HIBP is unreachable.
"""
import asyncio
import hashlib
from collections.abc import Mapping
from collections.abc import Mapping, Sequence
from dataclasses import dataclass
from types import MappingProxyType
from typing import Final
from litellm._logging import verbose_proxy_logger
@ -136,6 +138,33 @@ async def _is_password_breached(password: str, client: AsyncHTTPHandler) -> bool
return breached
def is_breach_check_enabled(general_settings: Mapping[str, object]) -> bool:
return general_settings.get("password_policy_check_breached_passwords", True) is not False
async def is_password_breached(
password: str,
general_settings: Mapping[str, object],
client: AsyncHTTPHandler | None = None,
) -> bool:
"""False when the check is disabled, the password is absent from the HIBP
corpus, or HIBP is unreachable (fail open)."""
if not is_breach_check_enabled(general_settings):
return False
return await _is_password_breached(password, client if client is not None else _hibp_client())
def breached_password_error() -> ProxyException:
return ProxyException(
message=(
"This password appears in known data breaches and cannot be used. Please choose a different password."
),
type=ProxyErrorTypes.validation_error,
param="password",
code=400,
)
async def validate_password_not_breached(
password: str,
general_settings: Mapping[str, object],
@ -144,16 +173,42 @@ async def validate_password_not_breached(
"""Raise ``ProxyException`` (400) if ``password`` appears in a known data breach.
Fails open: an unreachable or misbehaving HIBP allows the password."""
check_enabled: Final = general_settings.get("password_policy_check_breached_passwords", True) is not False
if not check_enabled:
if not await is_password_breached(password, general_settings, client):
return
if not await _is_password_breached(password, client if client is not None else _hibp_client()):
return
raise ProxyException(
message=(
"This password appears in known data breaches and cannot be used. Please choose a different password."
),
type=ProxyErrorTypes.validation_error,
param="password",
code=400,
raise breached_password_error()
def _strength_verdict(password: str, general_settings: Mapping[str, object]) -> ProxyException | None:
try:
validate_password_policy(password, general_settings)
except ProxyException as e:
return e
return None
async def validate_passwords_bulk(
passwords: Sequence[str],
general_settings: Mapping[str, object],
client: AsyncHTTPHandler | None = None,
) -> Mapping[str, ProxyException | None]:
"""Per-unique-password policy verdicts for a batch: the ProxyException to
surface, or None when the password is acceptable.
Deduplicates first, then issues every needed HIBP lookup concurrently, so a
batch caller pays one HIBP timeout window in the worst case instead of one
per password (each lookup still fails open independently)."""
unique_passwords: Final = tuple(dict.fromkeys(passwords))
strength_verdicts: Final[Mapping[str, ProxyException | None]] = MappingProxyType(
{password: _strength_verdict(password, general_settings) for password in unique_passwords}
)
to_screen: Final = tuple(password for password in unique_passwords if strength_verdicts[password] is None)
breached_flags: Final = await asyncio.gather(
*(is_password_breached(password, general_settings, client) for password in to_screen)
)
breached_passwords: Final = frozenset(password for password, breached in zip(to_screen, breached_flags) if breached)
return MappingProxyType(
{
password: breached_password_error() if password in breached_passwords else strength_verdicts[password]
for password in unique_passwords
}
)

View file

@ -26,9 +26,14 @@ from fastapi import APIRouter, Depends, Header, HTTPException, Request, status
import litellm
from litellm._logging import verbose_proxy_logger
from litellm._uuid import uuid
from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler
from litellm.proxy._types import *
from litellm.proxy.auth.auth_checks import get_team_object, get_user_object
from litellm.proxy.auth.password_policy import validate_password_not_breached, validate_password_policy
from litellm.proxy.auth.password_policy import (
validate_password_not_breached,
validate_password_policy,
validate_passwords_bulk,
)
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.common_utils.user_api_key_cache import (
object_permission_cache_key,
@ -157,11 +162,17 @@ def _team_membership_table(
return team_membership_table
async def _hash_password_in_dict(data: dict, general_settings: Mapping[str, object]) -> None:
"""Validate and hash password field in-place if present."""
async def _hash_password_in_dict(
data: dict, general_settings: Mapping[str, object], password_prevalidated: bool = False
) -> None:
"""Validate and hash password field in-place if present.
``password_prevalidated`` skips the policy checks for callers that already
validated the password (the bulk path screens its whole batch upfront)."""
if "password" in data and data["password"] is not None:
validate_password_policy(data["password"], general_settings)
await validate_password_not_breached(data["password"], general_settings)
if not password_prevalidated:
validate_password_policy(data["password"], general_settings)
await validate_password_not_breached(data["password"], general_settings)
data["password"] = hash_password(data["password"])
@ -1416,6 +1427,7 @@ async def _update_single_user_helper(
user_request: UpdateUserRequest,
user_api_key_dict: UserAPIKeyAuth,
litellm_changed_by: str | None = None,
password_prevalidated: bool = False,
) -> dict[str, Any]:
"""
Helper function to update a single user.
@ -1438,7 +1450,7 @@ async def _update_single_user_helper(
data_json: Final[dict] = user_request.model_dump(exclude_unset=True)
non_default_values = _update_internal_user_params(data_json=data_json, data=user_request)
await _hash_password_in_dict(non_default_values, general_settings)
await _hash_password_in_dict(non_default_values, general_settings, password_prevalidated=password_prevalidated)
existing_user_row: BaseModel | None = None
if user_request.user_id:
@ -1682,19 +1694,38 @@ async def bulk_update_processed_users(
users_to_update: list[UpdateUserRequest],
user_api_key_dict: UserAPIKeyAuth,
litellm_changed_by: str | None = None,
hibp_client: AsyncHTTPHandler | None = None,
) -> BulkUpdateUserResponse:
from litellm.proxy.proxy_server import general_settings
results: Final[list[UserUpdateResult]] = []
successful_updates = 0
failed_updates = 0
# Screen the batch's passwords upfront and concurrently: done per-user
# inside the loop below, each HIBP lookup would be awaited serially and a
# degraded-slow HIBP could stretch a full batch to minutes, timing out the
# request after some updates already persisted.
password_verdicts: Final = await validate_passwords_bulk(
tuple(u.password for u in users_to_update if u.password is not None),
general_settings,
client=hibp_client,
)
# Process each user update independently
try:
for user_request in users_to_update:
try:
if (
user_request.password is not None
and (password_error := password_verdicts.get(user_request.password)) is not None
):
raise password_error
response = await _update_single_user_helper(
user_request=user_request,
user_api_key_dict=user_api_key_dict,
litellm_changed_by=litellm_changed_by,
password_prevalidated=True,
)
# Record success
results.append(

View file

@ -7,6 +7,7 @@ The breach-check (HIBP) tests inject a real AsyncHTTPHandler wrapping an
httpx.MockTransport, so no network is touched and nothing is monkeypatched.
"""
import asyncio
import hashlib
import httpx
@ -21,6 +22,7 @@ from litellm.proxy.auth.password_policy import (
get_password_policy,
validate_password_not_breached,
validate_password_policy,
validate_passwords_bulk,
)
STRONG_PASSWORD = "Str0ng!Passw0rd"
@ -268,3 +270,76 @@ async def test_breach_check_fails_open_on_malformed_response_body():
client=_client_returning(f"{_sha1_upper('password12345')[5:]}:not-a-number"),
)
assert result is None
@pytest.mark.asyncio
async def test_validate_passwords_bulk_screens_concurrently():
"""All HIBP lookups for a batch must be in flight at once: each handler
call stalls until every expected request has arrived, and a handler that
gives up waiting reports the password as breached. Serial awaiting (the
old per-user behavior) leaves each earlier request waiting forever for the
later ones, so every verdict comes back as a breach and the test fails."""
passwords = ("Uniqu3!Passw0rd-a", "Uniqu3!Passw0rd-b", "Uniqu3!Passw0rd-c")
suffix_by_prefix = {_sha1_upper(p)[:5]: _sha1_upper(p)[5:] for p in passwords}
all_arrived = asyncio.Event()
arrivals: list[str] = []
async def handler(request: httpx.Request) -> httpx.Response:
arrivals.append(request.url.path)
if len(arrivals) == len(passwords):
all_arrived.set()
try:
await asyncio.wait_for(all_arrived.wait(), timeout=5)
except TimeoutError:
return httpx.Response(200, text=f"{suffix_by_prefix[request.url.path.rsplit('/', 1)[-1]]}:1")
return httpx.Response(200, text="0000000000000000000000000000000000A:1")
verdicts = await validate_passwords_bulk(passwords, {}, client=_client_with_transport(handler))
assert set(arrivals) == {f"/range/{prefix}" for prefix in suffix_by_prefix}
assert all(verdicts[p] is None for p in passwords)
@pytest.mark.asyncio
async def test_validate_passwords_bulk_deduplicates_lookups():
"""500 users sharing one password must cost exactly one HIBP lookup."""
password = "Sh@red-Passw0rd!"
request_count = 0
def handler(request: httpx.Request) -> httpx.Response:
nonlocal request_count
request_count += 1
return httpx.Response(200, text="0000000000000000000000000000000000A:1")
verdicts = await validate_passwords_bulk((password,) * 500, {}, client=_client_with_transport(handler))
assert request_count == 1
assert verdicts == {password: None}
@pytest.mark.asyncio
async def test_validate_passwords_bulk_mixed_verdicts():
"""Weak passwords are rejected without an HIBP lookup; breached ones get
the breach error; acceptable ones map to None."""
breached = "Br3ached!Passw0rd"
clean = "Cl3an!!Passw0rd42"
weak = "short1!"
breached_sha1 = _sha1_upper(breached)
looked_up_prefixes: list[str] = []
def handler(request: httpx.Request) -> httpx.Response:
looked_up_prefixes.append(request.url.path.rsplit("/", 1)[-1])
if request.url.path == f"/range/{breached_sha1[:5]}":
return httpx.Response(200, text=f"{breached_sha1[5:]}:99")
return httpx.Response(200, text="0000000000000000000000000000000000A:1")
verdicts = await validate_passwords_bulk((breached, clean, weak), {}, client=_client_with_transport(handler))
assert _sha1_upper(weak)[:5] not in looked_up_prefixes
assert verdicts[clean] is None
assert "data breaches" in verdicts[breached].message
assert verdicts[breached].code == "400"
assert "12 characters" in verdicts[weak].message
@pytest.mark.asyncio
async def test_validate_passwords_bulk_empty_batch_makes_no_lookups():
verdicts = await validate_passwords_bulk((), {}, client=_client_never_called())
assert verdicts == {}