The async/sync delete_response_api_handler always passed json=data into
httpx.delete, where data is {} from the transformer. httpx serializes that
to a 2-byte body. The Azure Responses DELETE endpoint now rejects any
request body with code: unexpected_body, breaking
test_basic_openai_responses_delete_endpoint on the llm_responses_api_testing
job. Build the kwargs dict and only set json= when data is truthy.
Add unit tests that patch httpx.delete and assert json/data are not in the
captured kwargs for the Azure DELETE path (sync and async).
Two Greptile review findings addressed:
1. (P1, security) The ``litellm_oauth_state`` cookie is the sole
guard against Login-CSRF in the PKCE flow but was set without the
``Secure`` attribute, so a network observer on plain HTTP could
read and replay it — bypassing the protection this PR adds.
Thread the originating ``Request`` down through
``get_sso_login_redirect`` and ``get_generic_sso_redirect_response``
and set ``Secure`` based on ``request.url.scheme == "https"``.
When no request is supplied (programmatic callers / tests) default
to ``Secure=True`` — production-safe. Local HTTP dev still works
because the request scheme is observed at runtime.
2. (P2) The cookie was set unconditionally, but the callback only
validates it inside the PKCE branch. Two concurrent SSO sessions
(one PKCE, one plain) could overwrite each other's state cookie
and produce spurious 400s for the plain-flow user.
Move the ``set_cookie`` call inside the existing
``if code_verifier and "state" in redirect_params`` block so the
cookie is only written when PKCE is active and the validation
will actually fire.
Tests cover both paths: PKCE-on (cookie set with Secure default),
PKCE-off (cookie not set), and HTTP dev request (Secure dropped so
the browser will actually attach the cookie on the callback hop).
``variant`` is user-controlled (passed through from
``litellm.video_content(variant=...)``) and was interpolated raw into
the URL query string. A value like ``thumbnail&extra=1`` would inject
additional query parameters into the upstream request — the same
class of issue this PR's path-segment encoding addresses. Wrap the
value in ``quote(value, safe="")`` so ``&`` / ``=`` / ``#`` cannot
terminate the ``variant`` value or open a new parameter.
Adds a regression test asserting that a malicious ``thumbnail&extra=1``
ends up percent-encoded in the URL, and that the legitimate
``thumbnail`` value still round-trips cleanly.
The Generic SSO PKCE flow used the URL ``state`` parameter as the
cache key for the PKCE ``code_verifier`` without binding the state
to the caller's browser. An attacker who pre-minted a state and
cached a verifier under it could hand the resulting login link to a
victim; the victim's auth code would then be exchanged with the
attacker's verifier on the callback, producing an access token
under the attacker's control (Login CSRF / token theft).
The non-PKCE branch is unaffected because it delegates to
fastapi-sso's ``verify_and_process``, which performs its own
session-cookie check. The PKCE branch bypasses that helper, which
is exactly the gap this commit closes.
Two-part fix in ``ui_sso.py``:
- ``get_generic_sso_redirect_response`` now sets a
``litellm_oauth_state`` cookie (HttpOnly, SameSite=Lax, 10-min TTL)
carrying the state value used in the redirect URL. The cookie is
set on the redirect response just like the existing
``litellm_cp_return_to`` cookie a few lines earlier in the file.
- ``get_generic_sso_response`` validates ``request.cookies.get(
"litellm_oauth_state")`` against ``request.query_params.get(
"state")`` via ``secrets.compare_digest`` before invoking the
PKCE token exchange. Mismatch (or either being missing) raises a
``ProxyException`` with HTTP 400.
The pre-existing TODO above the redirect logic ("state should be a
random string and added to the user session with cookie") is now
addressed and removed.
Tests cover the redirect-side cookie set, the missing-cookie reject
shape, the URL/cookie-mismatch reject shape, and the matching-cookie
happy path.
