mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(proxy): mark session/SSO/SAML cookies Secure behind a TLS-terminating reverse proxy (#39391)
* fix(proxy): mark session/SSO/SAML cookies Secure behind a TLS-terminating reverse proxy litellm only sees a plain-HTTP hop when TLS terminates at a reverse proxy, so cookie Secure attributes previously derived from (or defaulted without regard to) the literal request scheme could be dropped in production. The token session cookie set by every login path never carried Secure/HttpOnly/ SameSite at all. Adds IPAddressUtils.is_request_https, a single trust-aware resolver used by every cookie-setting call site: PROXY_BASE_URL, then X-Forwarded-Proto only from a configured trusted proxy (general_settings.use_x_forwarded_for + mcp_trusted_proxy_ranges), then the literal scheme. An unconfigured or untrusted caller cannot spoof the header to force Secure on. Resolves LIT-6748 * fix(proxy): make the shared session-cookie helper public, type new test helpers set_session_token_cookie is imported across modules (ui_sso.py -> proxy_server.py), so the leading underscore was misleading and breached basedpyright's reportPrivateUsage budget with zero headroom. Also adds missing parameter/return type annotations to the new test helper functions per repo convention.
This commit is contained in:
parent
22cc97fe0a
commit
c16c2cd6ee
8 changed files with 610 additions and 17 deletions
|
|
@ -6,8 +6,11 @@ External callers (public IPs) only see servers with available_on_public_internet
|
|||
"""
|
||||
|
||||
import ipaddress
|
||||
import os
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Final
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from fastapi import Request
|
||||
from pydantic import TypeAdapter, ValidationError
|
||||
|
|
@ -137,7 +140,7 @@ class IPAddressUtils:
|
|||
@staticmethod
|
||||
def is_request_from_trusted_proxy(
|
||||
request: Request,
|
||||
general_settings: dict[str, Any] | None = None,
|
||||
general_settings: Mapping[str, Any] | None = None,
|
||||
) -> bool:
|
||||
"""
|
||||
Return True if X-Forwarded-* headers on this request should be trusted.
|
||||
|
|
@ -190,6 +193,36 @@ class IPAddressUtils:
|
|||
trusted_networks: Final = IPAddressUtils.parse_trusted_proxy_networks(trusted_ranges)
|
||||
return IPAddressUtils.is_trusted_proxy(direct_ip, trusted_networks)
|
||||
|
||||
@staticmethod
|
||||
def is_request_https(
|
||||
request: Request,
|
||||
general_settings: Mapping[str, Any] | None = None,
|
||||
) -> bool:
|
||||
"""
|
||||
Whether this request's PUBLIC-facing origin is HTTPS, for deciding
|
||||
whether a cookie set on the response should be marked ``Secure``.
|
||||
|
||||
litellm only sees a plain-HTTP hop whenever TLS terminates at a
|
||||
reverse proxy, so ``request.url.scheme`` alone cannot answer this in
|
||||
that deployment shape. Resolved from the first trusted signal:
|
||||
1. ``PROXY_BASE_URL`` (operator-declared public origin).
|
||||
2. ``X-Forwarded-Proto``, only when the request's direct peer is a
|
||||
configured trusted proxy -- see ``is_request_from_trusted_proxy``.
|
||||
An untrusted caller cannot spoof this header to strip Secure.
|
||||
3. The request's own literal scheme (direct TLS termination, or no
|
||||
reverse proxy in front of litellm).
|
||||
"""
|
||||
configured_base_url: Final = os.environ.get("PROXY_BASE_URL", "").strip()
|
||||
if configured_base_url:
|
||||
return urlparse(configured_base_url).scheme == "https"
|
||||
|
||||
if IPAddressUtils.is_request_from_trusted_proxy(request, general_settings=general_settings):
|
||||
forwarded_proto: Final = request.headers.get("X-Forwarded-Proto")
|
||||
if forwarded_proto:
|
||||
return forwarded_proto.split(",")[0].strip().lower() == "https"
|
||||
|
||||
return request.url.scheme == "https"
|
||||
|
||||
@staticmethod
|
||||
def extract_client_ip_from_xff_hops(
|
||||
xff_header: str,
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ from pydantic import ValidationError
|
|||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.caching.dual_cache import DualCache
|
||||
from litellm.proxy.auth.ip_address_utils import IPAddressUtils
|
||||
from litellm.proxy.management_endpoints.types import CustomOpenID, get_litellm_user_role
|
||||
from litellm.proxy.utils import get_custom_url
|
||||
|
||||
|
|
@ -131,7 +132,7 @@ class SAMLAuthHandler:
|
|||
|
||||
@staticmethod
|
||||
def _is_https(request: Request) -> bool:
|
||||
return SAMLAuthHandler._base_url(request).startswith("https")
|
||||
return IPAddressUtils.is_request_https(request)
|
||||
|
||||
@staticmethod
|
||||
def _acs_url(request: Request) -> str:
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ from litellm.proxy.auth.auth_utils import (
|
|||
has_user_setup_sso,
|
||||
)
|
||||
from litellm.proxy.auth.handle_jwt import JWTHandler
|
||||
from litellm.proxy.auth.ip_address_utils import IPAddressUtils
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.admin_ui_utils import (
|
||||
admin_ui_disabled,
|
||||
|
|
@ -1118,7 +1119,7 @@ async def google_login(
|
|||
request=request,
|
||||
)
|
||||
if sso_redirect is not None:
|
||||
_persist_return_to_cookie(sso_redirect, return_to)
|
||||
_persist_return_to_cookie(sso_redirect, return_to, request)
|
||||
return sso_redirect
|
||||
|
||||
from fastapi.responses import HTMLResponse
|
||||
|
|
@ -1138,7 +1139,7 @@ async def google_login(
|
|||
# helper the SSO branch uses, so /login can resume the connect flow instead of dead-ending at the
|
||||
# dashboard. One implementation → the two sign-in branches cannot diverge (and the login form always
|
||||
# renders, since the helper never raises on a bad return_to).
|
||||
_persist_return_to_cookie(form_response, return_to)
|
||||
_persist_return_to_cookie(form_response, return_to, request)
|
||||
return form_response
|
||||
|
||||
|
||||
|
|
@ -2741,6 +2742,7 @@ async def _sso_return_to_redirect(
|
|||
jwt_token: str,
|
||||
redis_usage_cache,
|
||||
user_api_key_cache,
|
||||
request: Request,
|
||||
) -> RedirectResponse | None:
|
||||
"""Resolve the post-SSO redirect for a ``return_to``, or None to fall through to the dashboard.
|
||||
|
||||
|
|
@ -2759,7 +2761,7 @@ async def _sso_return_to_redirect(
|
|||
|
||||
if _is_same_origin_return_path(return_to):
|
||||
redirect_response = RedirectResponse(url=return_to, status_code=303)
|
||||
redirect_response.set_cookie(key="token", value=jwt_token)
|
||||
set_session_token_cookie(redirect_response, request, jwt_token)
|
||||
redirect_response.delete_cookie("litellm_cp_return_to")
|
||||
return redirect_response
|
||||
|
||||
|
|
@ -2782,7 +2784,25 @@ async def _sso_return_to_redirect(
|
|||
return None
|
||||
|
||||
|
||||
def _persist_return_to_cookie(response: Response, return_to: str | None) -> None:
|
||||
def set_session_token_cookie(response: Response, request: Request, jwt_token: str) -> None:
|
||||
"""Set the ``token`` session cookie shared by every sign-in path.
|
||||
|
||||
Not HttpOnly: the dashboard reads this cookie via ``document.cookie`` to
|
||||
populate its own Authorization headers (see
|
||||
``ui/litellm-dashboard/src/utils/cookieUtils.ts``), so marking it
|
||||
HttpOnly would break login. Secure is still required whenever the public
|
||||
origin is HTTPS, resolved the same trust-aware way as every other
|
||||
litellm cookie."""
|
||||
response.set_cookie(
|
||||
key="token",
|
||||
value=jwt_token,
|
||||
secure=IPAddressUtils.is_request_https(request),
|
||||
httponly=False,
|
||||
samesite="lax",
|
||||
)
|
||||
|
||||
|
||||
def _persist_return_to_cookie(response: Response, return_to: str | None, request: Request) -> None:
|
||||
"""Best-effort: persist a SAFE ``return_to`` on ``response`` as the one-shot ``litellm_cp_return_to``
|
||||
cookie so ANY sign-in path — SSO / Okta / generic OR the username/password form — can resume there
|
||||
afterwards. THIS is the single source of truth, called by every sign-in branch so they cannot
|
||||
|
|
@ -2803,6 +2823,7 @@ def _persist_return_to_cookie(response: Response, return_to: str | None) -> None
|
|||
max_age=600,
|
||||
httponly=True,
|
||||
samesite="lax",
|
||||
secure=IPAddressUtils.is_request_https(request),
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -3079,8 +3100,11 @@ class SSOAuthenticationHandler:
|
|||
# incoming request is HTTP (local dev). Without
|
||||
# ``Secure`` the cookie is sent over plain HTTP,
|
||||
# letting a network observer read and replay the
|
||||
# state value and bypass this protection.
|
||||
secure_flag: Final = request is None or request.url.scheme == "https"
|
||||
# state value and bypass this protection. Trust-aware:
|
||||
# honors PROXY_BASE_URL / a trusted reverse proxy's
|
||||
# X-Forwarded-Proto instead of only the literal scheme
|
||||
# litellm sees on the wire.
|
||||
secure_flag: Final = request is None or IPAddressUtils.is_request_https(request)
|
||||
redirect_response.set_cookie(
|
||||
key="litellm_oauth_state",
|
||||
value=state_value,
|
||||
|
|
@ -3628,6 +3652,7 @@ class SSOAuthenticationHandler:
|
|||
jwt_token=jwt_token,
|
||||
redis_usage_cache=redis_usage_cache,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
request=request,
|
||||
)
|
||||
if return_to_redirect is not None:
|
||||
return return_to_redirect
|
||||
|
|
@ -3636,7 +3661,7 @@ class SSOAuthenticationHandler:
|
|||
litellm_dashboard_ui += "?login=success"
|
||||
verbose_proxy_logger.info("Redirecting to %s", litellm_dashboard_ui)
|
||||
redirect_response: Final = RedirectResponse(url=litellm_dashboard_ui, status_code=303)
|
||||
redirect_response.set_cookie(key="token", value=jwt_token)
|
||||
set_session_token_cookie(redirect_response, request, jwt_token)
|
||||
return redirect_response
|
||||
|
||||
@staticmethod
|
||||
|
|
|
|||
|
|
@ -15329,7 +15329,10 @@ async def login(request: Request):
|
|||
# authorize round-trip), mirroring the SSO callback; otherwise land on the dashboard. Gated by
|
||||
# _is_same_origin_return_path (strictly relative path) so it can never be an open redirect, and the
|
||||
# one-shot cookie is cleared after use.
|
||||
from litellm.proxy.management_endpoints.ui_sso import _sso_return_to_redirect
|
||||
from litellm.proxy.management_endpoints.ui_sso import (
|
||||
_sso_return_to_redirect,
|
||||
set_session_token_cookie,
|
||||
)
|
||||
|
||||
# Resume through the SAME resumer the SSO callback uses, rather than a second, narrower arm.
|
||||
# _persist_return_to_cookie stores both shapes it accepts (a relative same-origin path AND a
|
||||
|
|
@ -15346,6 +15349,7 @@ async def login(request: Request):
|
|||
jwt_token=jwt_token,
|
||||
redis_usage_cache=redis_usage_cache,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
request=request,
|
||||
)
|
||||
except Exception: # noqa: BLE001 # resuming must NEVER block a completed sign-in
|
||||
# The symmetric half of _persist_return_to_cookie's "never raises" contract. The resumer
|
||||
|
|
@ -15360,7 +15364,7 @@ async def login(request: Request):
|
|||
|
||||
# Create redirect response with cookie
|
||||
redirect_response: Final = RedirectResponse(url=litellm_dashboard_ui, status_code=303)
|
||||
redirect_response.set_cookie(key="token", value=jwt_token)
|
||||
set_session_token_cookie(redirect_response, request, jwt_token)
|
||||
if cp_return_to:
|
||||
redirect_response.delete_cookie(key="litellm_cp_return_to")
|
||||
return redirect_response
|
||||
|
|
@ -15370,6 +15374,7 @@ async def login(request: Request):
|
|||
async def login_v2(request: Request):
|
||||
global premium_user, general_settings, master_key
|
||||
from litellm.proxy.auth.login_utils import authenticate_user, create_ui_token_object, encode_ui_session_jwt
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
from litellm.proxy.utils import get_custom_url
|
||||
|
||||
try:
|
||||
|
|
@ -15404,7 +15409,7 @@ async def login_v2(request: Request):
|
|||
content={"redirect_url": litellm_dashboard_ui, "token": jwt_token},
|
||||
status_code=status.HTTP_200_OK,
|
||||
)
|
||||
json_response.set_cookie(key="token", value=jwt_token)
|
||||
set_session_token_cookie(json_response, request, jwt_token)
|
||||
return json_response
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception("litellm.proxy.proxy_server.login_v2(): Exception occurred - %s", e)
|
||||
|
|
@ -15504,6 +15509,8 @@ async def login_v3(request: Request):
|
|||
|
||||
@router.post("/v3/login/exchange", include_in_schema=False) # exchange single-use opaque code for JWT
|
||||
async def login_v3_exchange(request: Request):
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
try:
|
||||
if not general_settings.get("control_plane_url"):
|
||||
raise ProxyException(
|
||||
|
|
@ -15550,7 +15557,7 @@ async def login_v3_exchange(request: Request):
|
|||
},
|
||||
status_code=status.HTTP_200_OK,
|
||||
)
|
||||
json_response.set_cookie(key="token", value=cached_data["token"])
|
||||
set_session_token_cookie(json_response, request, cached_data["token"])
|
||||
return json_response
|
||||
except ProxyException:
|
||||
raise
|
||||
|
|
|
|||
|
|
@ -591,3 +591,100 @@ class TestFilterServerIdsByIpWithInfo:
|
|||
)
|
||||
assert allowed == []
|
||||
assert blocked == 2
|
||||
|
||||
|
||||
def _make_scheme_request(
|
||||
scheme: str, client_host: str = "203.0.113.5", headers: dict[str, str] | None = None
|
||||
) -> Request:
|
||||
request = MagicMock(spec=Request)
|
||||
request.client = MagicMock()
|
||||
request.client.host = client_host
|
||||
request.headers = headers or {}
|
||||
request.url = MagicMock()
|
||||
request.url.scheme = scheme
|
||||
return request
|
||||
|
||||
|
||||
class TestIsRequestHttps:
|
||||
"""Regression tests for the cookie Secure trust-boundary resolution.
|
||||
|
||||
litellm only sees a plain-HTTP hop when TLS terminates at a reverse
|
||||
proxy, so a cookie's Secure attribute must not be derived from the
|
||||
literal request scheme alone. It must also not blindly trust a
|
||||
client-spoofable X-Forwarded-Proto header with no trust boundary.
|
||||
"""
|
||||
|
||||
def test_direct_https_is_secure(self, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request("https")
|
||||
assert IPAddressUtils.is_request_https(request, general_settings={}) is True
|
||||
|
||||
def test_direct_http_is_not_secure(self, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request("http")
|
||||
assert IPAddressUtils.is_request_https(request, general_settings={}) is False
|
||||
|
||||
def test_spoofed_forwarded_proto_without_trusted_proxy_config_is_ignored(
|
||||
self, monkeypatch
|
||||
):
|
||||
# Regression: an internal HTTP hop with an attacker-supplied
|
||||
# X-Forwarded-Proto: https must NOT flip Secure on, because no
|
||||
# trust boundary (use_x_forwarded_for + mcp_trusted_proxy_ranges)
|
||||
# is configured. Blindly trusting this header is itself a
|
||||
# vulnerability.
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request(
|
||||
"http", headers={"X-Forwarded-Proto": "https"}
|
||||
)
|
||||
assert IPAddressUtils.is_request_https(request, general_settings={}) is False
|
||||
|
||||
def test_forwarded_proto_honored_only_from_trusted_proxy(self, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request(
|
||||
"http",
|
||||
client_host="10.0.0.5",
|
||||
headers={"X-Forwarded-Proto": "https"},
|
||||
)
|
||||
general_settings = {
|
||||
"use_x_forwarded_for": True,
|
||||
"mcp_trusted_proxy_ranges": ["10.0.0.0/8"],
|
||||
}
|
||||
assert IPAddressUtils.is_request_https(request, general_settings=general_settings) is True
|
||||
|
||||
def test_forwarded_proto_http_from_trusted_proxy_is_not_secure(self, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request(
|
||||
"https",
|
||||
client_host="10.0.0.5",
|
||||
headers={"X-Forwarded-Proto": "http"},
|
||||
)
|
||||
general_settings = {
|
||||
"use_x_forwarded_for": True,
|
||||
"mcp_trusted_proxy_ranges": ["10.0.0.0/8"],
|
||||
}
|
||||
assert IPAddressUtils.is_request_https(request, general_settings=general_settings) is False
|
||||
|
||||
def test_untrusted_direct_peer_falls_back_to_literal_scheme(self, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
request = _make_scheme_request(
|
||||
"http",
|
||||
client_host="203.0.113.5",
|
||||
headers={"X-Forwarded-Proto": "https"},
|
||||
)
|
||||
general_settings = {
|
||||
"use_x_forwarded_for": True,
|
||||
"mcp_trusted_proxy_ranges": ["10.0.0.0/8"],
|
||||
}
|
||||
assert IPAddressUtils.is_request_https(request, general_settings=general_settings) is False
|
||||
|
||||
def test_proxy_base_url_https_overrides_literal_http_scheme(self, monkeypatch):
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
request = _make_scheme_request("http")
|
||||
assert IPAddressUtils.is_request_https(request, general_settings={}) is True
|
||||
|
||||
def test_proxy_base_url_http_overrides_literal_https_scheme(self, monkeypatch):
|
||||
# An explicit operator-configured plain-http public origin wins over
|
||||
# the literal connection scheme, same as the https direction above.
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "http://litellm.internal")
|
||||
request = _make_scheme_request("https")
|
||||
assert IPAddressUtils.is_request_https(request, general_settings={}) is False
|
||||
|
|
|
|||
|
|
@ -642,3 +642,77 @@ async def test_read_acs_post_data_rejects_oversized_stream_without_content_lengt
|
|||
with pytest.raises(HTTPException) as exc:
|
||||
await SAMLAuthHandler.read_acs_post_data(cast(Request, request))
|
||||
assert exc.value.status_code == 413
|
||||
|
||||
|
||||
def _fake_request_with_scheme(scheme, headers=None, client_host="203.0.113.5"):
|
||||
"""A fuller fake Request than ``_fake_request``: adds ``url``, ``headers`` and
|
||||
``client``, which ``IPAddressUtils.is_request_https`` reads directly instead of
|
||||
going through ``PROXY_BASE_URL``."""
|
||||
return type(
|
||||
"Req",
|
||||
(),
|
||||
{
|
||||
"base_url": URL(f"{scheme}://proxy.example.com/"),
|
||||
"url": URL(f"{scheme}://proxy.example.com/sso/saml/login"),
|
||||
"query_params": {},
|
||||
"cookies": {},
|
||||
"headers": headers or {},
|
||||
"client": type("Client", (), {"host": client_host})(),
|
||||
},
|
||||
)()
|
||||
|
||||
|
||||
class TestSAMLAuthnCookieSecureFlag:
|
||||
"""Regression tests for the litellm_saml_authn cookie's Secure attribute.
|
||||
litellm only sees a plain-HTTP hop whenever TLS terminates at a reverse
|
||||
proxy, so Secure must not be derived from the literal request scheme alone."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_secure_over_direct_https(self, saml_env, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
cache = DualCache()
|
||||
request = _fake_request_with_scheme("https")
|
||||
redirect = await SAMLAuthHandler.build_login_redirect(request, cache)
|
||||
cookie = redirect.headers["set-cookie"]
|
||||
assert "Secure" in cookie
|
||||
assert "SameSite=none" in cookie
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_not_secure_over_direct_http(self, saml_env, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
cache = DualCache()
|
||||
request = _fake_request_with_scheme("http")
|
||||
redirect = await SAMLAuthHandler.build_login_redirect(request, cache)
|
||||
cookie = redirect.headers["set-cookie"]
|
||||
assert "Secure" not in cookie
|
||||
assert "SameSite=lax" in cookie
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_secure_behind_trusted_tls_terminating_proxy(self, saml_env, monkeypatch):
|
||||
"""THE regression: TLS terminates at a reverse proxy, litellm only sees a
|
||||
plain-HTTP hop, but the cookie must still be marked Secure when the operator
|
||||
has configured a trusted proxy reporting X-Forwarded-Proto: https."""
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"use_x_forwarded_for": True, "mcp_trusted_proxy_ranges": ["10.0.0.0/8"]},
|
||||
)
|
||||
cache = DualCache()
|
||||
request = _fake_request_with_scheme(
|
||||
"http", headers={"X-Forwarded-Proto": "https"}, client_host="10.0.0.5"
|
||||
)
|
||||
redirect = await SAMLAuthHandler.build_login_redirect(request, cache)
|
||||
cookie = redirect.headers["set-cookie"]
|
||||
assert "Secure" in cookie
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_untrusted_spoofed_forwarded_proto_is_ignored(self, saml_env, monkeypatch):
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
cache = DualCache()
|
||||
request = _fake_request_with_scheme(
|
||||
"http", headers={"X-Forwarded-Proto": "https"}, client_host="203.0.113.5"
|
||||
)
|
||||
redirect = await SAMLAuthHandler.build_login_redirect(request, cache)
|
||||
cookie = redirect.headers["set-cookie"]
|
||||
assert "Secure" not in cookie
|
||||
|
|
|
|||
|
|
@ -7604,6 +7604,112 @@ class TestPKCEStateCookieBinding:
|
|||
assert cookie_str is not None
|
||||
assert "Secure" not in cookie_str
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redirect_response_sets_secure_flag_behind_trusted_tls_terminating_proxy(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""Regression: litellm sees a plain-HTTP hop when TLS terminates at a reverse
|
||||
proxy. The Secure flag must still be set when the direct peer is a configured
|
||||
trusted proxy and it reports X-Forwarded-Proto: https -- but NOT from an
|
||||
unconfigured/untrusted caller spoofing the same header (see the sibling test
|
||||
below)."""
|
||||
from fastapi.responses import RedirectResponse
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import (
|
||||
SSOAuthenticationHandler,
|
||||
)
|
||||
|
||||
mock_redirect = RedirectResponse(
|
||||
url="http://idp.internal/authorize?state=behind-proxy-state"
|
||||
)
|
||||
mock_generic_sso = MagicMock()
|
||||
mock_generic_sso.__enter__ = MagicMock(return_value=mock_generic_sso)
|
||||
mock_generic_sso.__exit__ = MagicMock(return_value=None)
|
||||
mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect)
|
||||
|
||||
proxied_request = MagicMock(spec=Request)
|
||||
proxied_request.url.scheme = "http"
|
||||
proxied_request.headers = {"X-Forwarded-Proto": "https"}
|
||||
proxied_request.client = MagicMock()
|
||||
proxied_request.client.host = "10.0.0.5"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"use_x_forwarded_for": True, "mcp_trusted_proxy_ranges": ["10.0.0.0/8"]},
|
||||
)
|
||||
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"GENERIC_CLIENT_STATE": "behind-proxy-state",
|
||||
"GENERIC_CLIENT_USE_PKCE": "true",
|
||||
},
|
||||
):
|
||||
response = await SSOAuthenticationHandler.get_generic_sso_redirect_response(
|
||||
generic_sso=mock_generic_sso,
|
||||
state=None,
|
||||
generic_authorization_endpoint="http://idp.internal/authorize",
|
||||
request=proxied_request,
|
||||
)
|
||||
|
||||
cookie_headers = response.headers.getlist("set-cookie")
|
||||
cookie_str = next(
|
||||
(c for c in cookie_headers if "litellm_oauth_state=" in c), None
|
||||
)
|
||||
assert cookie_str is not None
|
||||
assert "Secure" in cookie_str
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redirect_response_ignores_spoofed_forwarded_proto_without_trust_config(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""The same X-Forwarded-Proto: https header must NOT flip Secure on when no
|
||||
trusted-proxy config is present -- honoring it unconditionally would let any
|
||||
client spoof the header and would not itself be the vulnerability the ticket
|
||||
warns against."""
|
||||
from fastapi.responses import RedirectResponse
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import (
|
||||
SSOAuthenticationHandler,
|
||||
)
|
||||
|
||||
mock_redirect = RedirectResponse(
|
||||
url="http://idp.internal/authorize?state=spoofed-state"
|
||||
)
|
||||
mock_generic_sso = MagicMock()
|
||||
mock_generic_sso.__enter__ = MagicMock(return_value=mock_generic_sso)
|
||||
mock_generic_sso.__exit__ = MagicMock(return_value=None)
|
||||
mock_generic_sso.get_login_redirect = AsyncMock(return_value=mock_redirect)
|
||||
|
||||
spoofed_request = MagicMock(spec=Request)
|
||||
spoofed_request.url.scheme = "http"
|
||||
spoofed_request.headers = {"X-Forwarded-Proto": "https"}
|
||||
spoofed_request.client = MagicMock()
|
||||
spoofed_request.client.host = "203.0.113.5"
|
||||
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"GENERIC_CLIENT_STATE": "spoofed-state",
|
||||
"GENERIC_CLIENT_USE_PKCE": "true",
|
||||
},
|
||||
):
|
||||
response = await SSOAuthenticationHandler.get_generic_sso_redirect_response(
|
||||
generic_sso=mock_generic_sso,
|
||||
state=None,
|
||||
generic_authorization_endpoint="http://idp.internal/authorize",
|
||||
request=spoofed_request,
|
||||
)
|
||||
|
||||
cookie_headers = response.headers.getlist("set-cookie")
|
||||
cookie_str = next(
|
||||
(c for c in cookie_headers if "litellm_oauth_state=" in c), None
|
||||
)
|
||||
assert cookie_str is not None
|
||||
assert "Secure" not in cookie_str
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pkce_callback_rejects_missing_cookie(self):
|
||||
"""When PKCE is enabled and a code_verifier is in the cache, the
|
||||
|
|
@ -8586,6 +8692,24 @@ class TestSameOriginReturnPath:
|
|||
assert _is_same_origin_return_path("") is False
|
||||
|
||||
|
||||
def _make_https_request() -> Request:
|
||||
request = MagicMock(spec=Request)
|
||||
request.url.scheme = "https"
|
||||
request.headers = {}
|
||||
request.client = MagicMock()
|
||||
request.client.host = "203.0.113.5"
|
||||
return request
|
||||
|
||||
|
||||
def _make_http_request() -> Request:
|
||||
request = MagicMock(spec=Request)
|
||||
request.url.scheme = "http"
|
||||
request.headers = {}
|
||||
request.client = MagicMock()
|
||||
request.client.host = "203.0.113.5"
|
||||
return request
|
||||
|
||||
|
||||
class TestPersistReturnToCookieSharedHelper:
|
||||
"""The single shared return_to helper used by EVERY sign-in branch (SSO / Okta / generic AND the
|
||||
username/password form). It must be best-effort and NEVER raise — a bad return_to can never block
|
||||
|
|
@ -8603,7 +8727,7 @@ class TestPersistReturnToCookieSharedHelper:
|
|||
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, "/mcp/authorize?client_id=llm_dcrc_abc")
|
||||
_persist_return_to_cookie(resp, "/mcp/authorize?client_id=llm_dcrc_abc", _make_https_request())
|
||||
assert "litellm_cp_return_to=" in self._cookie(resp)
|
||||
|
||||
def test_bad_absolute_with_control_plane_configured_does_not_raise_and_is_not_stored(self, monkeypatch):
|
||||
|
|
@ -8617,7 +8741,7 @@ class TestPersistReturnToCookieSharedHelper:
|
|||
"litellm.proxy.proxy_server.general_settings", {"control_plane_url": "https://cp.example.com"}
|
||||
)
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, "https://evil.example.com/steal") # must not raise
|
||||
_persist_return_to_cookie(resp, "https://evil.example.com/steal", _make_https_request()) # must not raise
|
||||
assert "litellm_cp_return_to=" not in self._cookie(resp)
|
||||
|
||||
def test_none_return_to_is_a_noop(self):
|
||||
|
|
@ -8626,7 +8750,7 @@ class TestPersistReturnToCookieSharedHelper:
|
|||
from litellm.proxy.management_endpoints.ui_sso import _persist_return_to_cookie
|
||||
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, None)
|
||||
_persist_return_to_cookie(resp, None, _make_https_request())
|
||||
assert "litellm_cp_return_to=" not in self._cookie(resp)
|
||||
|
||||
def test_control_plane_matching_absolute_is_stored(self, monkeypatch):
|
||||
|
|
@ -8638,5 +8762,126 @@ class TestPersistReturnToCookieSharedHelper:
|
|||
"litellm.proxy.proxy_server.general_settings", {"control_plane_url": "https://cp.example.com"}
|
||||
)
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, "https://cp.example.com/ui?page=models")
|
||||
_persist_return_to_cookie(resp, "https://cp.example.com/ui?page=models", _make_https_request())
|
||||
assert "litellm_cp_return_to=" in self._cookie(resp)
|
||||
|
||||
def test_cookie_is_secure_and_httponly_over_https(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import _persist_return_to_cookie
|
||||
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, "/mcp/authorize", _make_https_request())
|
||||
cookie = self._cookie(resp)
|
||||
assert "Secure" in cookie
|
||||
assert "HttpOnly" in cookie
|
||||
assert "SameSite=lax" in cookie
|
||||
|
||||
def test_cookie_is_not_secure_over_plain_http_direct(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import _persist_return_to_cookie
|
||||
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
resp = Response()
|
||||
_persist_return_to_cookie(resp, "/mcp/authorize", _make_http_request())
|
||||
assert "Secure" not in self._cookie(resp)
|
||||
|
||||
def test_cookie_is_secure_behind_trusted_tls_terminating_proxy(self, monkeypatch):
|
||||
"""Regression for the reported bug: TLS terminates at a reverse proxy, litellm only
|
||||
sees a plain-HTTP hop, but a trusted X-Forwarded-Proto: https must still mark the
|
||||
cookie Secure."""
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import _persist_return_to_cookie
|
||||
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"use_x_forwarded_for": True, "mcp_trusted_proxy_ranges": ["10.0.0.0/8"]},
|
||||
)
|
||||
resp = Response()
|
||||
request = _make_http_request()
|
||||
request.client.host = "10.0.0.5"
|
||||
request.headers = {"X-Forwarded-Proto": "https"}
|
||||
_persist_return_to_cookie(resp, "/mcp/authorize", request)
|
||||
assert "Secure" in self._cookie(resp)
|
||||
|
||||
|
||||
class TestSessionTokenCookie:
|
||||
"""Regression tests for the ``token`` session cookie set by every sign-in path
|
||||
(username/password login, SSO callback, the CLI /v2, /v3 login exchange helpers).
|
||||
It was previously set with no Secure/HttpOnly/SameSite attributes at all -- always
|
||||
sent over plain HTTP and readable by any script on the page. HttpOnly must stay off
|
||||
deliberately: the dashboard reads this cookie via document.cookie."""
|
||||
|
||||
@staticmethod
|
||||
def _cookie(resp) -> str:
|
||||
return resp.headers.get("set-cookie", "")
|
||||
|
||||
def test_secure_over_direct_https(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
resp = Response()
|
||||
set_session_token_cookie(resp, _make_https_request(), "jwt-token-value")
|
||||
cookie = self._cookie(resp)
|
||||
assert "token=jwt-token-value" in cookie
|
||||
assert "Secure" in cookie
|
||||
assert "SameSite=lax" in cookie
|
||||
assert "HttpOnly" not in cookie
|
||||
|
||||
def test_not_secure_over_direct_http(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
resp = Response()
|
||||
set_session_token_cookie(resp, _make_http_request(), "jwt-token-value")
|
||||
assert "Secure" not in self._cookie(resp)
|
||||
|
||||
def test_secure_behind_trusted_tls_terminating_proxy(self, monkeypatch):
|
||||
"""THE regression: TLS terminates at a reverse proxy, litellm only sees a
|
||||
plain-HTTP hop, but the session cookie must still be marked Secure when the
|
||||
operator has configured a trusted proxy that reports X-Forwarded-Proto: https."""
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"use_x_forwarded_for": True, "mcp_trusted_proxy_ranges": ["10.0.0.0/8"]},
|
||||
)
|
||||
request = _make_http_request()
|
||||
request.client.host = "10.0.0.5"
|
||||
request.headers = {"X-Forwarded-Proto": "https"}
|
||||
resp = Response()
|
||||
set_session_token_cookie(resp, request, "jwt-token-value")
|
||||
assert "Secure" in self._cookie(resp)
|
||||
|
||||
def test_untrusted_spoofed_forwarded_proto_is_ignored(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
request = _make_http_request()
|
||||
request.headers = {"X-Forwarded-Proto": "https"}
|
||||
resp = Response()
|
||||
set_session_token_cookie(resp, request, "jwt-token-value")
|
||||
assert "Secure" not in self._cookie(resp)
|
||||
|
||||
def test_proxy_base_url_https_overrides_literal_http_scheme(self, monkeypatch):
|
||||
from fastapi import Response
|
||||
|
||||
from litellm.proxy.management_endpoints.ui_sso import set_session_token_cookie
|
||||
|
||||
monkeypatch.setenv("PROXY_BASE_URL", "https://litellm.example.com")
|
||||
resp = Response()
|
||||
set_session_token_cookie(resp, _make_http_request(), "jwt-token-value")
|
||||
assert "Secure" in self._cookie(resp)
|
||||
|
|
|
|||
|
|
@ -148,6 +148,72 @@ def test_login_v2_returns_redirect_url_and_sets_cookie(monkeypatch):
|
|||
assert mock_jwt_encode.call_args.kwargs == {"algorithm": "HS256"}
|
||||
|
||||
|
||||
def _mock_login_v2_deps(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.auth.login_utils.authenticate_user",
|
||||
AsyncMock(return_value={"user_id": "test-user"}),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.auth.login_utils.create_ui_token_object",
|
||||
MagicMock(return_value={"user_id": "test-user"}),
|
||||
)
|
||||
monkeypatch.setattr("jwt.encode", MagicMock(return_value="signed-token"))
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.master_key", "test-master-key")
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", MagicMock())
|
||||
monkeypatch.setattr("litellm.proxy.utils.get_server_root_path", lambda: "")
|
||||
monkeypatch.setattr("litellm.proxy.utils.get_proxy_base_url", lambda: None)
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
|
||||
|
||||
def test_login_v2_sets_secure_cookie_over_direct_https(monkeypatch):
|
||||
"""Regression: the token cookie previously carried no Secure/HttpOnly/SameSite
|
||||
attributes at all, so it was always sent over plain HTTP."""
|
||||
_mock_login_v2_deps(monkeypatch)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
|
||||
client = TestClient(app, base_url="https://testserver")
|
||||
response = client.post("/v2/login", json={"username": "alice", "password": "secret"})
|
||||
|
||||
assert response.status_code == 200
|
||||
cookie = response.headers.get("set-cookie")
|
||||
assert "Secure" in cookie
|
||||
assert "HttpOnly" not in cookie # deliberate: the dashboard reads this cookie via JS
|
||||
assert "samesite=lax" in cookie.lower()
|
||||
|
||||
|
||||
def test_login_v2_does_not_set_secure_cookie_over_direct_http(monkeypatch):
|
||||
_mock_login_v2_deps(monkeypatch)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {})
|
||||
|
||||
client = TestClient(app, base_url="http://testserver")
|
||||
response = client.post("/v2/login", json={"username": "alice", "password": "secret"})
|
||||
|
||||
assert response.status_code == 200
|
||||
assert "Secure" not in response.headers.get("set-cookie")
|
||||
|
||||
|
||||
def test_login_v2_sets_secure_cookie_behind_trusted_tls_terminating_proxy(monkeypatch):
|
||||
"""THE regression: litellm only sees a plain-HTTP hop when TLS terminates at a
|
||||
reverse proxy, but the token cookie must still be Secure when the direct peer is
|
||||
a configured trusted proxy reporting X-Forwarded-Proto: https."""
|
||||
_mock_login_v2_deps(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{"use_x_forwarded_for": True, "mcp_trusted_proxy_ranges": ["10.0.0.0/8"]},
|
||||
)
|
||||
|
||||
client = TestClient(app, base_url="http://testserver", client=("10.0.0.5", 50000))
|
||||
response = client.post(
|
||||
"/v2/login",
|
||||
json={"username": "alice", "password": "secret"},
|
||||
headers={"X-Forwarded-Proto": "https"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert "Secure" in response.headers.get("set-cookie")
|
||||
|
||||
|
||||
def test_login_v2_returns_json_on_proxy_exception(monkeypatch):
|
||||
"""Test that /v2/login returns JSON error when ProxyException is raised"""
|
||||
from litellm.proxy._types import ProxyErrorTypes, ProxyException
|
||||
|
|
@ -356,6 +422,51 @@ def test_login_v3_exchange_happy_path(monkeypatch):
|
|||
assert exchange_response.cookies.get("token") == "signed-token"
|
||||
|
||||
|
||||
def test_login_v3_exchange_sets_secure_cookie_behind_trusted_tls_terminating_proxy(monkeypatch):
|
||||
"""Regression: /v3/login/exchange's token cookie must be Secure behind a trusted
|
||||
TLS-terminating reverse proxy even though litellm only sees a plain-HTTP hop."""
|
||||
mock_prisma_client = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.auth.login_utils.authenticate_user",
|
||||
AsyncMock(return_value={"user_id": "test-user"}),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.auth.login_utils.create_ui_token_object",
|
||||
MagicMock(return_value={"user_id": "test-user"}),
|
||||
)
|
||||
monkeypatch.setattr("jwt.encode", MagicMock(return_value="signed-token"))
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.master_key", "test-master-key")
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
{
|
||||
"control_plane_url": "https://cp.example.com",
|
||||
"use_x_forwarded_for": True,
|
||||
"mcp_trusted_proxy_ranges": ["10.0.0.0/8"],
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)
|
||||
mock_config = MagicMock()
|
||||
mock_config.worker_registry = []
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.proxy_config", mock_config)
|
||||
monkeypatch.setattr("litellm.proxy.utils.get_server_root_path", lambda: "")
|
||||
monkeypatch.setattr("litellm.proxy.utils.get_proxy_base_url", lambda: None)
|
||||
monkeypatch.delenv("PROXY_BASE_URL", raising=False)
|
||||
|
||||
client = TestClient(app, base_url="http://testserver", client=("10.0.0.5", 50000))
|
||||
|
||||
login_response = client.post("/v3/login", json={"username": "alice", "password": "secret"})
|
||||
code = login_response.json()["code"]
|
||||
|
||||
exchange_response = client.post(
|
||||
"/v3/login/exchange",
|
||||
json={"code": code},
|
||||
headers={"X-Forwarded-Proto": "https"},
|
||||
)
|
||||
assert exchange_response.status_code == 200
|
||||
assert "Secure" in exchange_response.headers.get("set-cookie")
|
||||
|
||||
|
||||
def test_login_v3_exchange_single_use(monkeypatch):
|
||||
"""Code can only be redeemed once."""
|
||||
mock_prisma_client = MagicMock()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue