scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-08 03:08:45 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 115

fix: address third round of greptile review feedback

Browse source 
- Fix CRITICAL log firing on every non-PKCE callback: only log when PKCE is enabled
- Remove unused pkce_env_value intermediate variable
- Prefer reusing redis_usage_cache over creating separate RedisCache instance
  (avoids losing advanced connection options like SSL, timeouts, db)
This commit is contained in:
Ishaan Jaffer 2026-03-05 12:11:05 -08:00
parent 50631f8959
commit c6f2446fa5
2 changed files with 23 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
litellm/proxy/management_endpoints/ui_sso.py
View file

@ -1937,8 +1937,7 @@ class SSOAuthenticationHandler:
# Handle PKCE (Proof Key for Code Exchange) if enabled
# Set GENERIC_CLIENT_USE_PKCE=true to enable PKCE for enhanced OAuth security
pkce_env_value = os.getenv("GENERIC_CLIENT_USE_PKCE", "false")
use_pkce = pkce_env_value.lower() == "true"
use_pkce = os.getenv("GENERIC_CLIENT_USE_PKCE", "false").lower() == "true"
if use_pkce:
(
@ -2517,14 +2516,17 @@ class SSOAuthenticationHandler:
await redis_usage_cache.async_delete_cache(key=cache_key)
else:
await user_api_key_cache.async_delete_cache(key=cache_key)
else:
elif os.getenv("GENERIC_CLIENT_USE_PKCE", "false").lower() == "true":
# PKCE is enabled but verifier is missing — likely a cross-instance cache miss.
verbose_proxy_logger.error(
f"✙ CRITICAL: No PKCE code_verifier found in cache for state '{state}'. "
f"This indicates: (1) authorization request and callback handled by different instances without shared cache, "
f"(2) cache entry expired (TTL: 600s), or (3) Redis serialization error. "
f"SOLUTION: Ensure Redis is configured correctly. "
f"Cache type: {type(user_api_key_cache).__name__}. "
f"Cached data: {cached_data}"
"PKCE is enabled but no code_verifier found in cache for state '%s'. "
"This usually means the authorization and callback were handled by different "
"instances without a shared cache. "
"Ensure Redis is configured and GENERIC_CLIENT_USE_PKCE=true. "
"Cache type: %s. Cached data: %s",
state,
type(user_api_key_cache).__name__,
cached_data,
)
return token_params

41
litellm/proxy/proxy_server.py
View file

@ -3039,38 +3039,21 @@ class ProxyConfig:
# that use Redis only for LLM response caching and not for session state.
use_pkce = os.getenv("GENERIC_CLIENT_USE_PKCE", "false").lower() == "true"
if use_pkce and user_api_key_cache.redis_cache is None:
redis_host = get_secret("REDIS_HOST", None)
redis_port = get_secret("REDIS_PORT", None)
redis_password = get_secret("REDIS_PASSWORD", None)
if redis_host is not None:
try:
from litellm.caching.caching import RedisCache
user_redis_cache = RedisCache(
host=redis_host,
port=redis_port,
password=redis_password,
)
user_api_key_cache.redis_cache = user_redis_cache
verbose_proxy_logger.info(
"Configured user_api_key_cache to use Redis at %s:%s "
"(PKCE enabled — verifiers shared across instances).",
redis_host,
redis_port,
)
except Exception as e:
verbose_proxy_logger.warning(
"Failed to configure Redis for user_api_key_cache: %s. "
"Falling back to in-memory cache. Multi-instance PKCE will not work.",
e,
)
if redis_usage_cache is not None:
# Reuse the existing Redis connection so all advanced connection
# options (SSL, db, timeouts) are inherited rather than re-created
# from a subset of env vars.
user_api_key_cache.redis_cache = redis_usage_cache
verbose_proxy_logger.info(
"Configured user_api_key_cache to use Redis "
"(PKCE enabled — verifiers shared across instances)."
)
else:
verbose_proxy_logger.warning(
"GENERIC_CLIENT_USE_PKCE=true but REDIS_HOST is not set. "
"GENERIC_CLIENT_USE_PKCE=true but Redis is not configured for LiteLLM caching. "
"PKCE verifiers will not be shared across instances. "
"Configure Redis or enable sticky sessions for multi-instance deployments."
"Configure Redis via the 'cache' section in your proxy config, "
"or enable sticky sessions for multi-instance deployments."
)
### STORE MODEL IN DB ### feature flag for `/model/new`
store_model_in_db = general_settings.get("store_model_in_db", False)