fix(auth): ignore a UserInfo response without a sub (#8251)

* fix(auth): ignore a UserInfo response without a sub

fetch_userinfo() compared the UserInfo sub with the ID token's only when
it was truthy, so a response with no sub (or an empty one) was accepted
and its email, email_verified and name replaced the signed ID token's.
OIDC Core 5.3.2 requires the two subjects to match before UserInfo is
used. A missing, empty or non-string sub now raises OIDCUserInfoMismatch,
which the callback already treats as a skipped UserInfo request.

* fix(auth): log a non-string UserInfo sub apart from a missing one

The callback continues silently on a rejected UserInfo response, so the
log line is the only way to tell a provider that omits sub from one that
sends a malformed value. A non-string sub now reports its type.

* test(auth): name the UserInfo sub cases for what they cover

The case list and the callback test also cover a present but non-string
sub, so "no sub" undersold them.

backend/app/gateway/auth/oidc.py

@@ -63,7 +63,7 @@ class OIDCValidationError(OIDCError):
 
 
 class OIDCUserInfoMismatch(OIDCError):
-    """UserInfo sub does not match ID token sub."""
+    """UserInfo sub is missing or does not match ID token sub."""
 
 
 def _json_object(resp: httpx.Response, what: str) -> dict[str, Any]:
@@ -391,8 +391,10 @@ class OIDCService:
     async def fetch_userinfo(self, metadata: OIDCMetadata, access_token: str, expected_sub: str) -> dict[str, Any]:
         """Fetch userinfo from the UserInfo endpoint.
 
-        Validates that the ``sub`` claim matches ``expected_sub``
-        (from the ID token) to prevent userinfo injection.
+        The response is returned only when its ``sub`` equals ``expected_sub``
+        (from the ID token). OIDC Core 5.3.2 requires that check, and a
+        response without a ``sub`` cannot be tied to this login, so it raises
+        ``OIDCUserInfoMismatch`` too instead of overriding the ID token claims.
         """
         if not metadata.userinfo_endpoint:
             return {}
@@ -407,7 +409,12 @@ class OIDCService:
             raise OIDCError(f"UserInfo fetch failed: {exc}") from exc
         userinfo = _json_object(resp, "UserInfo response")
 
-        if userinfo.get("sub") and userinfo["sub"] != expected_sub:
+        sub = userinfo.get("sub")
+        if sub is None or sub == "":
+            raise OIDCUserInfoMismatch("UserInfo response has no sub claim")
+        if not isinstance(sub, str):
+            raise OIDCUserInfoMismatch(f"UserInfo sub claim is not a string ({type(sub).__name__})")
+        if sub != expected_sub:
             raise OIDCUserInfoMismatch("UserInfo sub does not match ID token sub")
 
         return userinfo

backend/docs/SSO.md

@@ -280,7 +280,11 @@ redirects to `/login?error=sso_failed`. A discovery document without a usable
 authorization, token or JWKS endpoint counts as failed unless the provider
 config overrides that endpoint, and it is not cached. A malformed UserInfo body
 is skipped like a failed UserInfo request: the login continues with the ID
-token claims. The cause stays in the Gateway log.
+token claims. The cause stays in the Gateway log. A UserInfo response whose
+`sub` is missing or differs from the ID token's is skipped the same way, since
+OIDC Core requires the two to match. With a provider that omits `sub` from
+UserInfo and sends the email only there, a first login gets `403` because no
+email reaches provisioning. Users already linked by subject still sign in.
 
 ## Frontend Callback Flow
 
@@ -296,7 +300,7 @@ The frontend handles the post-SSO flow at `/auth/callback`:
 - **State cookies** are HttpOnly, SameSite=Lax, Max-Age=300 seconds, and signed with the JWT secret
 - **PKCE** prevents authorization code interception attacks
 - **Nonce** prevents ID token replay attacks
-- **UserInfo sub check** ensures the UserInfo response matches the ID token subject
+- **UserInfo sub check** — UserInfo claims are used only when the response's `sub` equals the ID token subject; a response without a `sub` counts as a mismatch
 - **Reject alg=none** — ID tokens with algorithm "none" are always rejected
 - **No email auto-linking** — SSO accounts are always separate from email/password accounts. An email collision with an existing local account blocks the SSO login (409) rather than merging the two.
 - **Verified email requirement** — SSO users must have verified emails by default

backend/tests/test_oidc_auth.py

@@ -1,4 +1,5 @@
 import asyncio
+import re
 import time
 from unittest.mock import AsyncMock
 from urllib.parse import parse_qs, urlsplit
@@ -11,7 +12,7 @@ from fastapi import HTTPException
 from jwt.algorithms import RSAAlgorithm
 
 from app.gateway.auth.models import User
-from app.gateway.auth.oidc import OIDCError, OIDCIdentity, OIDCMetadata, OIDCService, OIDCValidationError
+from app.gateway.auth.oidc import OIDCError, OIDCIdentity, OIDCMetadata, OIDCService, OIDCUserInfoMismatch, OIDCValidationError
 from app.gateway.auth.user_provisioning import get_or_provision_oidc_user
 from deerflow.config.auth_config import OIDCProviderConfig
 
@@ -672,12 +673,15 @@ def _service_answering(responses: dict[str, object]) -> OIDCService:
     return service
 
 
-def _signed_provider() -> tuple[str, dict]:
-    """Return an ID token for ``_ISSUER`` and the JWKS that verifies it."""
+def _signed_provider(**claims) -> tuple[str, dict]:
+    """Return an ID token for ``_ISSUER`` and the JWKS that verifies it.
+
+    ``claims`` override the default ID token claims.
+    """
     private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
     now = int(time.time())
     id_token = jwt.encode(
-        {"iss": _ISSUER, "aud": "deer-flow", "sub": "s1", "exp": now + 300, "iat": now, "email": "user@example.com", "email_verified": True},
+        {"iss": _ISSUER, "aud": "deer-flow", "sub": "s1", "exp": now + 300, "iat": now, "email": "user@example.com", "email_verified": True, **claims},
         private_key,
         algorithm="RS256",
         headers={"kid": "k1"},
@@ -807,6 +811,105 @@ async def test_oidc_callback_falls_back_to_id_token_when_userinfo_body_is_malfor
     assert identity.email == "user@example.com"
 
 
+# ── UserInfo subject check ────────────────────────────────────────────────
+#
+# OIDC Core 5.3.2: the UserInfo ``sub`` must exactly match the ID token's
+# before any UserInfo claim is used. A response without a ``sub`` cannot be
+# tied to this login, so it is rejected like a mismatching one; otherwise its
+# email and email_verified replace the signed ID token's.
+
+# Each case carries the reason the Gateway logs, so an operator can tell an
+# absent ``sub`` from a malformed one.
+_UNUSABLE_SUB_CASES = [
+    pytest.param({}, "UserInfo response has no sub claim", id="missing"),
+    pytest.param({"sub": ""}, "UserInfo response has no sub claim", id="empty"),
+    pytest.param({"sub": None}, "UserInfo response has no sub claim", id="null"),
+    pytest.param({"sub": ["s1"]}, "UserInfo sub claim is not a string (list)", id="non-string"),
+    pytest.param({"sub": 1}, "UserInfo sub claim is not a string (int)", id="number"),
+]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    ("sub_field", "message"),
+    [
+        *_UNUSABLE_SUB_CASES,
+        pytest.param({"sub": "s2"}, "does not match", id="other-subject"),
+        pytest.param({"sub": "S1"}, "does not match", id="other-case"),
+    ],
+)
+async def test_oidc_fetch_userinfo_rejects_a_response_not_bound_to_the_id_token(sub_field, message):
+    service = _service_answering({"/userinfo": {**sub_field, "email": "other@example.com", "email_verified": True}})
+    try:
+        with pytest.raises(OIDCUserInfoMismatch, match=re.escape(message)):
+            await service.fetch_userinfo(OIDCMetadata(**_DISCOVERY), "access", expected_sub="s1")
+    finally:
+        await service.close()
+
+
+@pytest.mark.asyncio
+async def test_oidc_fetch_userinfo_returns_a_response_for_the_same_subject():
+    body = {"sub": "s1", "email": "user@example.com", "name": "User"}
+    service = _service_answering({"/userinfo": body})
+    try:
+        assert await service.fetch_userinfo(OIDCMetadata(**_DISCOVERY), "access", expected_sub="s1") == body
+    finally:
+        await service.close()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(("sub_field", "reason"), _UNUSABLE_SUB_CASES)
+async def test_oidc_callback_keeps_id_token_claims_when_userinfo_has_no_usable_sub(sub_field, reason, caplog):
+    id_token, jwks = _signed_provider(email="user@example.com", email_verified=False, name="ID Token User")
+    userinfo = {**sub_field, "email": "admin@example.com", "email_verified": True, "name": "Someone Else"}
+    service = _service_answering({"/token": {"id_token": id_token, "access_token": "access"}, "/jwks": jwks, "/userinfo": userinfo})
+    try:
+        with caplog.at_level("WARNING", logger="app.gateway.auth.oidc"):
+            identity = await service.authenticate_callback("keycloak", OIDCMetadata(**_DISCOVERY), "deer-flow", "secret", "code", "https://app.example.com/callback")
+    finally:
+        await service.close()
+
+    assert (identity.subject, identity.email, identity.email_verified, identity.name) == ("s1", "user@example.com", False, "ID Token User")
+    assert "admin@example.com" not in identity.claims.values()
+    assert reason in caplog.text
+
+
+@pytest.mark.asyncio
+async def test_oidc_callback_uses_userinfo_claims_for_the_same_subject():
+    id_token, jwks = _signed_provider(email="user@example.com", email_verified=False)
+    userinfo = {"sub": "s1", "email": "user@corp.example", "email_verified": True, "name": "UserInfo User"}
+    service = _service_answering({"/token": {"id_token": id_token, "access_token": "access"}, "/jwks": jwks, "/userinfo": userinfo})
+    try:
+        identity = await service.authenticate_callback("keycloak", OIDCMetadata(**_DISCOVERY), "deer-flow", "secret", "code", "https://app.example.com/callback")
+    finally:
+        await service.close()
+
+    assert (identity.subject, identity.email, identity.email_verified, identity.name) == ("s1", "user@corp.example", True, "UserInfo User")
+
+
+@pytest.mark.asyncio
+async def test_oidc_userinfo_without_sub_cannot_make_a_first_login_an_admin():
+    """End to end: a sub-less UserInfo email must not reach admin_emails."""
+    id_token, jwks = _signed_provider(email="user@example.com", email_verified=True)
+    service = _service_answering({"/token": {"id_token": id_token, "access_token": "access"}, "/jwks": jwks, "/userinfo": {"email": "admin@example.com", "email_verified": True}})
+    try:
+        identity = await service.authenticate_callback("keycloak", OIDCMetadata(**_DISCOVERY), "deer-flow", "secret", "code", "https://app.example.com/callback")
+    finally:
+        await service.close()
+
+    local_provider = AsyncMock()
+    local_provider.get_user_by_oauth.return_value = None
+    local_provider.get_user_by_email.return_value = None
+    await get_or_provision_oidc_user(
+        provider_id="keycloak",
+        provider_config=_provider_config(admin_emails=["admin@example.com"]),
+        identity=identity,
+        local_provider=local_provider,
+    )
+
+    local_provider.create_oauth_user.assert_awaited_once_with(email="user@example.com", oauth_provider="keycloak", oauth_id="s1", system_role="user")
+
+
 def _oidc_router_client(monkeypatch, service: OIDCService):
     from types import SimpleNamespace