Commit graph

34507 commits

Author SHA1 Message Date
Ishaan Jaffer
1bc6e2a2a5 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
2026-03-06 13:01:47 -08:00
Ishaan Jaffer
0c647d4be9 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)
2026-03-06 12:49:12 -08:00
Ishaan Jaffer
aec865bf54 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.
2026-03-06 12:34:35 -08:00
Ishaan Jaffer
6143c42f66 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
2026-03-06 12:16:41 -08:00
Ishaan Jaffer
53297dc885 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.
2026-03-06 11:56:41 -08:00
Ishaan Jaffer
e8906aa670 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
2026-03-06 11:36:26 -08:00
Ishaan Jaffer
485da8a208 address greptile review feedback (greploop iteration 35) 2026-03-06 11:12:39 -08:00
Ishaan Jaffer
ef0b463d0f address greptile review feedback (greploop iteration 34) 2026-03-06 11:11:39 -08:00
Ishaan Jaffer
f85229a393 address greptile review feedback (greploop iteration 33) 2026-03-06 11:00:01 -08:00
Ishaan Jaffer
06f12e0ddf address greptile review feedback (greploop iteration 32) 2026-03-06 10:38:21 -08:00
Ishaan Jaffer
04d3d55287 address greptile review feedback (greploop iteration 31) 2026-03-06 10:14:56 -08:00
Ishaan Jaffer
6f7cd4baa9 address greptile review feedback (greploop iteration 30) 2026-03-06 10:02:20 -08:00
Ishaan Jaffer
3e704e72f9 address greptile review feedback (greploop iteration 29) 2026-03-06 09:50:21 -08:00
Ishaan Jaffer
e55e7546d0 address greptile review feedback (greploop iteration 28) 2026-03-06 09:29:15 -08:00
Ishaan Jaffer
7e8b9d50e8 address greptile review feedback (greploop iteration 27) 2026-03-05 20:24:17 -08:00
Ishaan Jaffer
13f8ecf27b address greptile review feedback (greploop iteration 26) 2026-03-05 20:12:13 -08:00
Ishaan Jaffer
759abb781f address greptile review feedback (greploop iteration 25) 2026-03-05 19:58:37 -08:00
Ishaan Jaffer
0fe3cec376 address greptile review feedback (greploop iteration 24) 2026-03-05 19:48:54 -08:00
Ishaan Jaffer
b7e8eb4235 address greptile review feedback (greploop iteration 23) 2026-03-05 19:23:17 -08:00
Ishaan Jaffer
e718404c22 address greptile review feedback (greploop iteration 22) 2026-03-05 19:07:12 -08:00
Ishaan Jaffer
b7df1f3d1b address greptile review feedback (greploop iteration 21) 2026-03-05 18:52:09 -08:00
Ishaan Jaffer
cd4d672f7e address greptile review feedback (greploop iteration 20) 2026-03-05 18:31:42 -08:00
Ishaan Jaffer
1252ea871a address greptile review feedback (greploop iteration 19) 2026-03-05 18:21:47 -08:00
Ishaan Jaffer
d66ed5e412 address greptile review feedback (greploop iteration 18) 2026-03-05 17:52:21 -08:00
Ishaan Jaffer
a9eb399d9e address greptile review feedback (greploop iteration 17) 2026-03-05 17:38:32 -08:00
Ishaan Jaffer
63de6d0ee0 address greptile review feedback (greploop iteration 16) 2026-03-05 17:20:40 -08:00
Ishaan Jaffer
8b491790b6 address greptile review feedback (greploop iteration 15) 2026-03-05 17:06:22 -08:00
Ishaan Jaffer
e72018db28 address greptile review feedback (greploop iteration 14) 2026-03-05 16:54:13 -08:00
Ishaan Jaffer
cbb6b6700f address greptile review feedback (greploop iteration 13) 2026-03-05 16:15:11 -08:00
Ishaan Jaffer
dfac66ed51 fix misleading comment on user_api_key_cache TTL line 2026-03-05 16:01:48 -08:00
Ishaan Jaffer
bf27a497c3 address greptile review feedback (greploop iteration 12) 2026-03-05 15:48:51 -08:00
Ishaan Jaffer
f953e171e1 address greptile review feedback (greploop iteration 11) 2026-03-05 15:29:14 -08:00
Ishaan Jaffer
1f503fdcbd address greptile review feedback (greploop iteration 10) 2026-03-05 15:15:47 -08:00
Ishaan Jaffer
de15b54c00 address greptile review feedback (greploop iteration 9) 2026-03-05 15:02:23 -08:00
Ishaan Jaffer
d8906d33f7 address greptile review feedback (greploop iteration 8) 2026-03-05 14:50:32 -08:00
Ishaan Jaffer
27f766424d address greptile review feedback (greploop iteration 7) 2026-03-05 14:39:23 -08:00
Ishaan Jaffer
6001a0b30c address greptile review feedback (greploop iteration 6) 2026-03-05 14:29:08 -08:00
Ishaan Jaffer
03a39c50d2 simplify _get_pkce_userinfo: remove shared-client complexity, use async with directly 2026-03-05 14:23:03 -08:00
Ishaan Jaffer
530627ab78 address greptile review feedback (greploop iteration 5) 2026-03-05 14:14:10 -08:00
Ishaan Jaffer
8a667f096d address greptile review feedback (greploop iteration 4) 2026-03-05 14:02:55 -08:00
Ishaan Jaffer
9161253d6a address greptile review feedback (greploop iteration 3) 2026-03-05 13:55:01 -08:00
Ishaan Jaffer
cc1f8c23f2 sanitize PKCE cache log to not expose verifier content 2026-03-05 13:45:39 -08:00
Ishaan Jaffer
c8dcf45fc9 fix remaining PKCE test assertion for dict-format verifier storage 2026-03-05 13:45:28 -08:00
Ishaan Jaffer
1a16dcf598 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
2026-03-05 12:24:44 -08:00
Ishaan Jaffer
c6f2446fa5 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)
2026-03-05 12:11:05 -08:00
Ishaan Jaffer
50631f8959 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
2026-03-05 12:04:01 -08:00
Ishaan Jaffer
c94886d2b3 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
2026-03-05 11:56:19 -08:00
Ishaan Jaffer
da82ba4dc3 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)
2026-03-05 11:37:00 -08:00
Ishaan Jaffer
08e249b6c7 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.
2026-03-05 11:31:48 -08:00
Sameer Kankute
bf9c96b912
Merge pull request #22679 from giulio-leone/fix/websearch-thinking-constraint
fix: WebSearch interception fails with thinking enabled + SpendLog dedup
2026-03-06 00:49:17 +05:30