address greptile review feedback (greploop iteration 5)

This commit is contained in:
Ishaan Jaffer 2026-03-05 14:14:10 -08:00
parent 8a667f096d
commit 530627ab78
2 changed files with 18 additions and 8 deletions

View file

@ -2529,8 +2529,8 @@ class SSOAuthenticationHandler:
await redis_usage_cache.async_delete_cache(key=cache_key)
else:
await user_api_key_cache.async_delete_cache(key=cache_key)
elif os.getenv("GENERIC_CLIENT_USE_PKCE", "false").lower() == "true":
# PKCE is enabled but verifier is missing — likely a cross-instance cache miss.
else:
# PKCE is enabled (already checked above) but verifier is missing — likely a cross-instance cache miss.
verbose_proxy_logger.error(
"PKCE is enabled but no code_verifier found in cache for state '%s'. "
"This usually means the authorization and callback were handled by different "
@ -2644,7 +2644,12 @@ class SSOAuthenticationHandler:
json_err,
response.text[:500],
)
raise
raise ProxyException(
message=f"Token endpoint returned invalid JSON: {json_err}",
type=ProxyErrorTypes.auth_error,
param="token_exchange",
code=status.HTTP_401_UNAUTHORIZED,
)
# Some providers return HTTP 200 with an error body (e.g. expired code, replay attack).
if "access_token" not in token_response:
@ -2725,7 +2730,12 @@ class SSOAuthenticationHandler:
)
finally:
if _own_client:
await _client.aclose()
try:
await _client.aclose()
except Exception as close_err:
verbose_proxy_logger.debug(
"Error closing httpx client in _get_pkce_userinfo: %s", close_err
)
except Exception as e:
verbose_proxy_logger.warning(
"Userinfo endpoint error: %s, falling back to id_token", e

View file

@ -4529,7 +4529,7 @@ async def test_pkce_token_exchange_basic_auth():
assert isinstance(kwargs["auth"], httpx.BasicAuth)
return mock_response
with patch("httpx.AsyncClient") as mock_client_cls:
with patch("litellm.proxy.management_endpoints.ui_sso.httpx.AsyncClient") as mock_client_cls:
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)
@ -4574,7 +4574,7 @@ async def test_pkce_token_exchange_credentials_in_body():
mock.json.return_value = token_resp
return mock
with patch("httpx.AsyncClient") as mock_client_cls:
with patch("litellm.proxy.management_endpoints.ui_sso.httpx.AsyncClient") as mock_client_cls:
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)
@ -4608,7 +4608,7 @@ async def test_pkce_token_exchange_http200_with_error_body():
error_body = {"error": "invalid_grant", "error_description": "Code already used"}
with patch("httpx.AsyncClient") as mock_client_cls:
with patch("litellm.proxy.management_endpoints.ui_sso.httpx.AsyncClient") as mock_client_cls:
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)
@ -4647,7 +4647,7 @@ async def test_pkce_userinfo_falls_back_to_id_token():
).rstrip(b"=").decode()
fake_id_token = f"eyJhbGciOiJSUzI1NiJ9.{encoded_payload}.fakesig"
with patch("httpx.AsyncClient") as mock_client_cls:
with patch("litellm.proxy.management_endpoints.ui_sso.httpx.AsyncClient") as mock_client_cls:
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=False)