mirror of
https://github.com/tiennm99/litellm.git
synced 2026-07-21 00:21:38 +00:00
Ishaan JaffandGitHub
7697b1c397
* fix(sso): add direct PKCE token exchange and Redis cache wiring for multi-instance SSO When PKCE is enabled, bypass fastapi-sso and perform direct token exchange so code_verifier is correctly included. Store PKCE verifiers as dict in cache for proper JSON serialization in Redis. Wire user_api_key_cache to Redis when available so PKCE verifiers are shared across ECS tasks/pods. Also adds clearer error messages when PKCE is required but not configured. * refactor(sso): extract PKCE token exchange into SSOAuthenticationHandler methods - Move import httpx/jwt to module level (top of file, not inside function) - Extract inline PKCE token exchange + userinfo logic into two static methods: _pkce_token_exchange() and _get_pkce_userinfo() - get_generic_sso_response PKCE path is now a single method call - Fix double-logging in except block for non-PKCE errors - Use %-style log formatting (no f-strings in log calls) * fix: address greptile review feedback - Fix access_token missing in PKCE path: read from combined_response directly instead of generic_sso.access_token (which is only set by verify_and_process) - Fix PKCE error hint firing when PKCE is already enabled: only show 'set GENERIC_CLIENT_USE_PKCE=true' advice when code_verifier was absent - Fix unguarded KeyError on access_token: check for error field in HTTP 200 responses before accessing token_response['access_token'] - Fix silent empty userinfo: raise ProxyException when both userinfo endpoint and id_token fallback produce no user data - Fix backward-incompatible Redis wiring: only attach Redis to user_api_key_cache when GENERIC_CLIENT_USE_PKCE=true, preserving existing in-memory behaviour * 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 * fix: address third round of greptile review feedback - 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) * fix: address fourth round of greptile review feedback - Strip OAuth token credentials from response_convertor input to prevent access_token/id_token appearing in restricted-group error messages - Reuse single httpx.AsyncClient for both token exchange and userinfo requests to avoid a second TCP/TLS handshake per SSO callback - Revert Redis wiring to user_api_key_cache: PKCE code already uses redis_usage_cache directly; wiring would route all API-key lookups through Redis unnecessarily. Add startup warning instead when PKCE+Redis mismatch. - Move _OAUTH_TOKEN_FIELDS to module level * fix remaining PKCE test assertion for dict-format verifier storage * sanitize PKCE cache log to not expose verifier content * address greptile review feedback (greploop iteration 3) * address greptile review feedback (greploop iteration 4) * address greptile review feedback (greploop iteration 5) * simplify _get_pkce_userinfo: remove shared-client complexity, use async with directly * address greptile review feedback (greploop iteration 6) * address greptile review feedback (greploop iteration 7) * address greptile review feedback (greploop iteration 8) * address greptile review feedback (greploop iteration 9) * address greptile review feedback (greploop iteration 10) * address greptile review feedback (greploop iteration 11) * address greptile review feedback (greploop iteration 12) * fix misleading comment on user_api_key_cache TTL line * address greptile review feedback (greploop iteration 13) * address greptile review feedback (greploop iteration 14) * address greptile review feedback (greploop iteration 15) * address greptile review feedback (greploop iteration 16) * address greptile review feedback (greploop iteration 17) * address greptile review feedback (greploop iteration 18) * address greptile review feedback (greploop iteration 19) * address greptile review feedback (greploop iteration 20) * address greptile review feedback (greploop iteration 21) * address greptile review feedback (greploop iteration 22) * address greptile review feedback (greploop iteration 23) * address greptile review feedback (greploop iteration 24) * address greptile review feedback (greploop iteration 25) * address greptile review feedback (greploop iteration 26) * address greptile review feedback (greploop iteration 27) * address greptile review feedback (greploop iteration 28) * address greptile review feedback (greploop iteration 29) * address greptile review feedback (greploop iteration 30) * address greptile review feedback (greploop iteration 31) * address greptile review feedback (greploop iteration 32) * address greptile review feedback (greploop iteration 33) * address greptile review feedback (greploop iteration 34) * address greptile review feedback (greploop iteration 35) * address greptile review feedback (greploop iteration 37) - read GENERIC_CLIENT_USE_PKCE env var once in prepare_token_exchange_parameters - include actual decode error in jwt.decode failure exception message - add GENERIC_CLIENT_USE_PKCE=true to no-state regression test * defer PKCE verifier deletion until after all downstream processing Move _delete_pkce_verifier to after response_convertor and process_sso_jwt_access_token complete. If JWT processing raises, the verifier stays in cache so the user can retry without restarting the full OAuth flow. * address greptile review feedback (greploop iteration 38) - fix strict-mode cache miss error message to differentiate cross-instance routing failures (Redis configured) from single-instance issues (TTL expiry, pod restart) when only in-memory cache is available - add comment above _get_pkce_userinfo call explaining that bearer credentials are always sourced from token_response in the merge step * fix null JSON response body in _pkce_token_exchange - Guard against HTTP 200 with body null: response.json() returns None for JSON null, and calling .get() on None raises AttributeError. Now raises a clean ProxyException with a clear error message. - Fix misleading userinfo warning: was always saying "empty dict" but also fires for JSON null responses; updated to say "empty or null". - Add HTTP status code assertion to cache miss test. * address greptile review feedback (greploop iteration 39) - fix credential leakage: directly assign received_response from combined_response instead of relying on nonlocal mutation; Pyright was flagging the old guard as unreachable, meaning credential stripping might not execute — now it always runs unconditionally - add test for legacy plain-string cache format backward compat branch - add test for HTTP 200 with no error field and no access_token (else branch) - add test for HTTP 200 with JSON null body (new AttributeError guard) * fix _OAUTH_TOKEN_FIELDS merge loop to preserve userinfo values on absent fields When the token endpoint omits a bearer-credential field entirely (field absent from token_response), the previous code deleted it from merged even if userinfo provided a valid value. Now: - non-null in token_response → restore authoritative token endpoint value - explicit null in token_response → remove key from merged (clean absence) - field absent from token_response → leave userinfo value unchanged * use HTTP 401 for PKCE missing config errors GENERIC_CLIENT_ID and GENERIC_TOKEN_ENDPOINT missing when PKCE is enabled are auth-flow failures, not server errors. Use 401 instead of 500 to avoid triggering false-positive server error alerts in monitoring systems. * address greptile review feedback (greploop iteration 40) - fix duplicate error logging: demote first format-error log to DEBUG so the detailed ERROR in strict-mode branch is not duplicated - add HTTP status code assertions to all PKCE ProxyException tests for better regression protection against accidental code changes * add credential absence assertions to test_pkce_token_exchange_basic_auth Verify that client_id and client_secret are NOT double-sent in the POST body when Basic Auth is used (include_client_id=False with client_secret). Catches regressions where credentials leak into both Auth header and body. * address greptile review feedback (greploop iteration 41) - add Bearer token header assertion to test_pkce_token_exchange_credentials_in_body - add cache query assertions to both non-strict mode tests to confirm the cache was accessed before the warning path triggers * address greptile review feedback (greploop iteration 42) - assert null id_token is absent from merged result in basic auth test - add test for HTTP 200 empty/null userinfo body with no id_token fallback * use caplog to verify warning logs in non-strict cache miss tests The two non-strict mode tests now use pytest's caplog fixture to assert that a warning is actually emitted, not just that the code continues without raising. This catches regressions where the warning silently disappears. * remove dead-code response=None guard in _pkce_token_exchange * clean up stale pkce verifier cache entries in non-strict mode * fix test: configure async_delete_cache as AsyncMock and assert cleanup called * add sentinel guard so pkce-no-redis warning only fires once across hot-reloads * fix misleading comments: code_verifier init and bearer-credential merge docs * add best-effort cleanup in strict-mode for corrupt/empty cache entries * add redirect_uri assertion, userinfo body in non-200 log, sentinel comment
In total litellm runs 1000+ tests
[02/20/2025] Update:
To make it easier to contribute and map what behavior is tested,
we've started mapping the litellm directory in tests/test_litellm
This folder can only run mock tests.