From c512a49fa5ba97b16cb041dffd67def799a20e81 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Wed, 10 Jun 2026 18:59:41 -0700 Subject: [PATCH] test(auth_v2): pin the hardened auth behaviors from the security fixes Cover the security fixes landed in 71a189b, ca896ac, 6f3fc5e and 4503499: - HTTP basic now verifies the password via an injected BasicAuthVerifier: correct creds 200, wrong password / unknown user / no verifier wired all 401 (fail closed), and the password is never carried on the credential; plus a unit test that hash_basic_password is salted and InMemoryBasicAuthStore verifies it - mTLS only trusts the forwarded subject-DN header from a peer inside the trusted-proxy CIDRs; a forged header from an untrusted peer is ignored - SCIM discovery endpoints (ServiceProviderConfig, ResourceTypes, Schemas) are public, Users/Groups stay guarded, DELETE on a missing resource is a SCIM 404 Error, and PATCH honors nested dotted paths while rejecting filter paths 400 - SAML ACS sets a Secure session cookie, binds the redirect target server-side so a client-supplied form RelayState is never trusted (falls back to the default path), and the session store enforces TTL expiry and size eviction Mutation-checked: removing the basic-auth password check or the mTLS trusted-peer gate fails these. --- .../proxy/auth_v2/test_authenticators.py | 103 +++++++++++++++--- tests/test_litellm/proxy/auth_v2/test_saml.py | 49 ++++++++- tests/test_litellm/proxy/auth_v2/test_scim.py | 83 ++++++++++++++ 3 files changed, 218 insertions(+), 17 deletions(-) diff --git a/tests/test_litellm/proxy/auth_v2/test_authenticators.py b/tests/test_litellm/proxy/auth_v2/test_authenticators.py index 78a788430e6..30a18d46192 100644 --- a/tests/test_litellm/proxy/auth_v2/test_authenticators.py +++ b/tests/test_litellm/proxy/auth_v2/test_authenticators.py @@ -9,11 +9,13 @@ import pytest from litellm.proxy.auth_v2.authenticators import ( ApiKeyAuthenticator, HttpAuthenticator, + InMemoryBasicAuthStore, JwtVerifier, MutualTlsAuthenticator, OAuth2Authenticator, OidcAuthenticator, build_authenticators, + hash_basic_password, ) from litellm.proxy.auth_v2.config import ( ApiKeySchemeConfig, @@ -22,6 +24,7 @@ from litellm.proxy.auth_v2.config import ( MutualTlsConfig, OAuth2IntrospectionConfig, OidcProviderConfig, + TrustedProxyConfig, ) from litellm.proxy.auth_v2.errors import AuthError from litellm.proxy.auth_v2.models import AuthMethod, SecuritySchemeType @@ -121,12 +124,16 @@ async def test_api_key_authenticator_returns_none_when_absent(): # --------------------------------------------------------------------------- # -def _http_auth(public_key: Any, *, basic: HttpBasicConfig = None) -> HttpAuthenticator: +def _http_auth( + public_key: Any, *, basic: HttpBasicConfig = None, basic_verifier=None +) -> HttpAuthenticator: verifier = JwtVerifier( OidcProviderConfig(issuer=TEST_ISSUER, audience=[TEST_AUDIENCE]), jwks_client=FakeJwksClient(public_key), ) - return HttpAuthenticator(basic or HttpBasicConfig(), [verifier]) + return HttpAuthenticator( + basic or HttpBasicConfig(), [verifier], basic_verifier=basic_verifier + ) async def test_http_bearer_valid_token_resolves_credential(rsa_keypair, token_factory): @@ -167,15 +174,59 @@ async def test_http_basic_disabled_ignores_basic_scheme(rsa_keypair): assert await auth.authenticate(request) is None -async def test_http_basic_enabled_decodes_username(rsa_keypair): +def _basic_store() -> InMemoryBasicAuthStore: + return InMemoryBasicAuthStore({"alice": hash_basic_password("supersecret")}) + + +async def test_http_basic_verifies_correct_credentials(rsa_keypair): _, public_key = rsa_keypair - auth = _http_auth(public_key, basic=HttpBasicConfig(enabled=True)) + auth = _http_auth( + public_key, basic=HttpBasicConfig(enabled=True), basic_verifier=_basic_store() + ) creds = base64.b64encode(b"alice:supersecret").decode() request = make_request(headers={"authorization": f"Basic {creds}"}) credential = await auth.authenticate(request) assert credential is not None assert credential.method == AuthMethod.HTTP_BASIC assert credential.subject == "alice" + # the password must never be carried on the credential (leak regression) + assert "_basic_password" not in credential.claims + assert "supersecret" not in str(credential.claims) + + +async def test_http_basic_wrong_password_rejected(rsa_keypair): + _, public_key = rsa_keypair + auth = _http_auth( + public_key, basic=HttpBasicConfig(enabled=True), basic_verifier=_basic_store() + ) + creds = base64.b64encode(b"alice:WRONG").decode() + request = make_request(headers={"authorization": f"Basic {creds}"}) + with pytest.raises(AuthError) as exc: + await auth.authenticate(request) + assert exc.value.status_code == 401 + + +async def test_http_basic_unknown_user_rejected(rsa_keypair): + _, public_key = rsa_keypair + auth = _http_auth( + public_key, basic=HttpBasicConfig(enabled=True), basic_verifier=_basic_store() + ) + creds = base64.b64encode(b"mallory:supersecret").decode() + request = make_request(headers={"authorization": f"Basic {creds}"}) + with pytest.raises(AuthError) as exc: + await auth.authenticate(request) + assert exc.value.status_code == 401 + + +async def test_http_basic_without_verifier_fails_closed(rsa_keypair): + # basic enabled but no verifier wired -> must never accept (fail closed) + _, public_key = rsa_keypair + auth = _http_auth(public_key, basic=HttpBasicConfig(enabled=True)) + creds = base64.b64encode(b"alice:supersecret").decode() + request = make_request(headers={"authorization": f"Basic {creds}"}) + with pytest.raises(AuthError) as exc: + await auth.authenticate(request) + assert exc.value.status_code == 401 async def test_http_basic_malformed_payload_raises(rsa_keypair): @@ -195,6 +246,19 @@ def test_http_challenge_advertises_basic_only_when_enabled(rsa_keypair): assert "Bearer" in enabled.challenge() +def test_hash_basic_password_is_salted_and_verifiable(): + # the stored hash is never the plaintext, and re-hashing yields a fresh salt + first = hash_basic_password("supersecret") + second = hash_basic_password("supersecret") + assert "supersecret" not in first + assert first != second # random salt per call + + store = InMemoryBasicAuthStore({"alice": first}) + assert store.verify("alice", "supersecret") + assert not store.verify("alice", "supersecre") + assert not store.verify("unknown", "supersecret") + + # --------------------------------------------------------------------------- # # OAuth2Authenticator (at+jwt enforcement + opaque token path) # --------------------------------------------------------------------------- # @@ -345,10 +409,16 @@ async def test_oidc_unknown_issuer_raises(rsa_keypair, token_factory): # --------------------------------------------------------------------------- # -async def test_mtls_reads_forwarded_subject_header(): - auth = MutualTlsAuthenticator( - MutualTlsConfig(enabled=True, forwarded_subject_header="x-client-dn") - ) +# make_request's default peer is 203.0.113.7; trust that /24 for the proxy path +_TRUSTED_NET = TrustedProxyConfig(trusted_proxy_cidrs=["203.0.113.0/24"]) + + +def _mtls(config: MutualTlsConfig, network: TrustedProxyConfig = None): + return MutualTlsAuthenticator(config, network or _TRUSTED_NET) + + +async def test_mtls_reads_forwarded_subject_header_from_trusted_peer(): + auth = _mtls(MutualTlsConfig(enabled=True, forwarded_subject_header="x-client-dn")) request = make_request(headers={"x-client-dn": "CN=svc-a,O=Co,C=US"}) credential = await auth.authenticate(request) assert credential is not None @@ -357,15 +427,22 @@ async def test_mtls_reads_forwarded_subject_header(): assert credential.client_certificate.subject_dn == "CN=svc-a,O=Co,C=US" -async def test_mtls_forwarded_header_absent_returns_none(): - auth = MutualTlsAuthenticator( - MutualTlsConfig(enabled=True, forwarded_subject_header="x-client-dn") +async def test_mtls_forwarded_header_from_untrusted_peer_is_ignored(): + # spoofing guard: a client that is not a trusted proxy cannot forge the DN header + auth = _mtls(MutualTlsConfig(enabled=True, forwarded_subject_header="x-client-dn")) + request = make_request( + headers={"x-client-dn": "CN=attacker"}, client=("8.8.8.8", 4444) ) + assert await auth.authenticate(request) is None + + +async def test_mtls_forwarded_header_absent_returns_none(): + auth = _mtls(MutualTlsConfig(enabled=True, forwarded_subject_header="x-client-dn")) assert await auth.authenticate(make_request()) is None async def test_mtls_reads_asgi_tls_extension(): - auth = MutualTlsAuthenticator(MutualTlsConfig(enabled=True)) + auth = _mtls(MutualTlsConfig(enabled=True)) request = make_request( scope_extra={"extensions": {"tls": {"client_cert_name": "CN=from-asgi"}}} ) @@ -375,7 +452,7 @@ async def test_mtls_reads_asgi_tls_extension(): async def test_mtls_no_cert_returns_none(): - auth = MutualTlsAuthenticator(MutualTlsConfig(enabled=True)) + auth = _mtls(MutualTlsConfig(enabled=True)) assert await auth.authenticate(make_request()) is None diff --git a/tests/test_litellm/proxy/auth_v2/test_saml.py b/tests/test_litellm/proxy/auth_v2/test_saml.py index 83a13c72654..e3c5f655fe0 100644 --- a/tests/test_litellm/proxy/auth_v2/test_saml.py +++ b/tests/test_litellm/proxy/auth_v2/test_saml.py @@ -150,6 +150,9 @@ def saml_env(tmp_path: Path) -> SamlEnv: sp_key_file=sp_key, sp_cert_file=sp_cert, xmlsec_binary=xmlsec1, + # this harness mints IdP-initiated (unsolicited) responses; pin the config + # explicitly so the suite is independent of the allow_unsolicited default + allow_unsolicited=True, ) return SamlEnv(config=config, idp=idp) @@ -299,7 +302,10 @@ def test_acs_rejects_garbage_response(saml_env): assert store._users == {} -def test_acs_redirects_to_safe_relay_state(saml_env): +def test_acs_ignores_untrusted_form_relay_state(saml_env): + # the redirect target is bound server-side to the originating AuthnRequest, so a + # client-supplied form RelayState on an (unsolicited) response is NOT trusted and + # the ACS falls back to default_redirect_path app, _ = _build_app(saml_env) client = TestClient(app) acs = client.post( @@ -308,10 +314,10 @@ def test_acs_redirects_to_safe_relay_state(saml_env): follow_redirects=False, ) assert acs.status_code == 303 - assert acs.headers["location"] == "/dashboard" + assert acs.headers["location"] == "/" -def test_acs_rejects_open_redirect_relay_state(saml_env): +def test_acs_never_redirects_to_attacker_relay_state(saml_env): app, _ = _build_app(saml_env) client = TestClient(app) acs = client.post( @@ -323,7 +329,7 @@ def test_acs_rejects_open_redirect_relay_state(saml_env): follow_redirects=False, ) assert acs.status_code == 303 - # unsafe RelayState falls back to default_redirect_path, never the attacker URL + assert "evil.example.com" not in acs.headers["location"] assert acs.headers["location"] == "/" @@ -415,3 +421,38 @@ def test_metadata_source_classifies_input(metadata, expected_key): from litellm.proxy.auth_v2.saml import _metadata_source assert expected_key in _metadata_source(metadata) + + +def test_acs_session_cookie_is_secure(saml_env): + app, _ = _build_app(saml_env) + client = TestClient(app) + acs = client.post( + "/auth/saml/acs", + data={"SAMLResponse": saml_env.mint_response()}, + follow_redirects=False, + ) + assert "saml_session" in acs.cookies + assert "secure" in acs.headers["set-cookie"].lower() + + +# --------------------------------------------------------------------------- # +# SamlSessionStore TTL + size eviction (no xmlsec1 needed) +# --------------------------------------------------------------------------- # + + +def test_session_store_expires_entries(): + from litellm.proxy.auth_v2.saml import SamlSessionStore + + store = SamlSessionStore(ttl_seconds=0) + session_id = store.create_session({"name_id": "alice@example.com"}) + # ttl of 0 means the entry is already past its expiry on the next read + assert store.get(session_id) is None + + +def test_session_store_evicts_when_over_capacity(): + from litellm.proxy.auth_v2.saml import SamlSessionStore + + store = SamlSessionStore(max_size=3) + ids = [store.create_session({"name_id": f"user-{i}"}) for i in range(5)] + live = [sid for sid in ids if store.get(sid) is not None] + assert len(live) <= 3 diff --git a/tests/test_litellm/proxy/auth_v2/test_scim.py b/tests/test_litellm/proxy/auth_v2/test_scim.py index 06edb468b24..e05e9086115 100644 --- a/tests/test_litellm/proxy/auth_v2/test_scim.py +++ b/tests/test_litellm/proxy/auth_v2/test_scim.py @@ -189,3 +189,86 @@ def test_scim_requires_scim_write_scope(): response = underscoped.get("/scim/v2/Users") assert response.status_code == 403 assert "insufficient_scope" in response.headers.get("WWW-Authenticate", "") + + +# --------------------------------------------------------------------------- # +# Discovery endpoints are public (RFC 7644); Users/Groups stay guarded +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "path", ["/ServiceProviderConfig", "/ResourceTypes", "/Schemas"] +) +def test_discovery_endpoints_are_public(path): + # no credential at all -> still 200 (provisioning clients negotiate before auth) + unauth = TestClient(_app()) + assert unauth.get(f"/scim/v2{path}").status_code == 200 + + +def test_users_endpoint_is_not_public(): + unauth = TestClient(_app()) + assert unauth.get("/scim/v2/Users").status_code == 401 + + +# --------------------------------------------------------------------------- # +# DELETE on a missing resource returns a SCIM 404 Error, not 204 +# --------------------------------------------------------------------------- # + + +def test_delete_missing_user_returns_scim_404(client): + response = client.delete("/scim/v2/Users/no-such-user") + assert response.status_code == 404 + body = response.json() + assert body["schemas"] == [ERROR_SCHEMA] + assert body["status"] == "404" + + +def test_delete_missing_group_returns_scim_404(client): + response = client.delete("/scim/v2/Groups/no-such-group") + assert response.status_code == 404 + body = response.json() + assert body["schemas"] == [ERROR_SCHEMA] + assert body["status"] == "404" + + +def test_second_delete_of_group_returns_404(client): + group_id = client.post( + "/scim/v2/Groups", + json={"schemas": [GROUP_SCHEMA], "displayName": "Temp"}, + ).json()["id"] + assert client.delete(f"/scim/v2/Groups/{group_id}").status_code == 204 + assert client.delete(f"/scim/v2/Groups/{group_id}").status_code == 404 + + +# --------------------------------------------------------------------------- # +# PATCH supports nested dotted paths; filter paths are rejected +# --------------------------------------------------------------------------- # + + +def test_patch_nested_path_sets_subattribute(client): + user_id = _create_user(client).json()["id"] + response = client.patch( + f"/scim/v2/Users/{user_id}", + json={ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [{"op": "replace", "path": "name.givenName", "value": "Ada"}], + }, + ) + assert response.status_code == 200 + assert response.json()["name"]["givenName"] == "Ada" + assert client.get(f"/scim/v2/Users/{user_id}").json()["name"]["givenName"] == "Ada" + + +def test_patch_filter_path_is_rejected(client): + user_id = _create_user(client).json()["id"] + response = client.patch( + f"/scim/v2/Users/{user_id}", + json={ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [ + {"op": "replace", "path": 'emails[type eq "work"].value', "value": "x"} + ], + }, + ) + assert response.status_code == 400 + assert response.json()["schemas"] == [ERROR_SCHEMA]