diff --git a/litellm/proxy/auth/ip_address_utils.py b/litellm/proxy/auth/ip_address_utils.py index 558ea54495f..c2614b85016 100644 --- a/litellm/proxy/auth/ip_address_utils.py +++ b/litellm/proxy/auth/ip_address_utils.py @@ -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, diff --git a/litellm/proxy/management_endpoints/sso/saml_sso.py b/litellm/proxy/management_endpoints/sso/saml_sso.py index 3e67b211f62..466b100ea1f 100644 --- a/litellm/proxy/management_endpoints/sso/saml_sso.py +++ b/litellm/proxy/management_endpoints/sso/saml_sso.py @@ -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: diff --git a/litellm/proxy/management_endpoints/ui_sso.py b/litellm/proxy/management_endpoints/ui_sso.py index 6b98d9f9a26..1feefa5725d 100644 --- a/litellm/proxy/management_endpoints/ui_sso.py +++ b/litellm/proxy/management_endpoints/ui_sso.py @@ -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 diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 1e0792cb8a5..27132c90e05 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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 diff --git a/tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py b/tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py index e0585ab04f1..4c219760762 100644 --- a/tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py +++ b/tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py @@ -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 diff --git a/tests/test_litellm/proxy/management_endpoints/test_saml_sso.py b/tests/test_litellm/proxy/management_endpoints/test_saml_sso.py index 57decc7d458..36d6414ba9f 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_saml_sso.py +++ b/tests/test_litellm/proxy/management_endpoints/test_saml_sso.py @@ -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 diff --git a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py index e648bd09734..dd8c752a868 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py +++ b/tests/test_litellm/proxy/management_endpoints/test_ui_sso.py @@ -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) diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 5f692f8f109..4ed6a468371 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -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()