fix: address second round of greptile review feedback

- Fix PKCE error hint: check env var directly (not code_verifier presence) to
  distinguish 'PKCE not configured' from 'PKCE enabled but cache miss'
- Fix misleading Redis TTL comment in proxy_server.py
This commit is contained in:
Ishaan Jaffer 2026-03-05 12:04:01 -08:00
parent c94886d2b3
commit 50631f8959
2 changed files with 8 additions and 5 deletions

View file

@ -830,10 +830,13 @@ async def get_generic_sso_response(
except Exception as e:
error_message = str(e)
# Only surface "enable PKCE" advice when PKCE was NOT already in use.
# If code_verifier is set, the token exchange itself failed — that's a
# provider-side error, not a configuration problem.
if code_verifier is None and (
# Surface a helpful PKCE misconfiguration hint only when:
# 1. The error mentions PKCE/code verifier, AND
# 2. PKCE is not currently configured (GENERIC_CLIENT_USE_PKCE != true)
# If PKCE IS configured but code_verifier was absent (cross-instance cache miss),
# the real fix is shared Redis/sticky sessions — not enabling PKCE (it's already on).
pkce_configured = os.getenv("GENERIC_CLIENT_USE_PKCE", "false").lower() == "true"
if not pkce_configured and (
"PKCE" in error_message or "code verifier" in error_message.lower()
):
is_okta = (

View file

@ -3030,7 +3030,7 @@ class ProxyConfig:
if user_api_key_cache_ttl is not None:
user_api_key_cache.update_cache_ttl(
default_in_memory_ttl=float(user_api_key_cache_ttl),
default_redis_ttl=None, # will be set below if Redis is available
default_redis_ttl=None, # PKCE verifiers set explicit TTL on each store; Redis TTL not configured here
)
### CONFIGURE USER API KEY CACHE TO USE REDIS FOR PKCE (if enabled) ###