Skip to content

Commit 3fe3035

Browse files
msureshkumar88Suresh Kumar Moharajan
andauthored
fix(mcp): resolve session-token sub UUID to email in streamable-HTTP auth (#5802)
* fix(mcp): resolve session-token sub UUID to email in streamable-HTTP auth Session tokens issued by /admin/login carry `sub` as the EmailUser.id UUID, while legacy/API tokens carry the email. The MCP streamable-HTTP auth path treated `sub` as an email unconditionally, so the user lookup missed for every session token. With require_user_in_db enabled this surfaced as 401 "User not found in database" on /servers/{id}/mcp — LLM Chat could not connect to a virtual server hosted by the same gateway. On the unified /mcp endpoint the same miss resolved empty teams instead, yielding public-only visibility (tools/list returns 0) and a UUID-keyed RBAC check that denied tools/call. The miss was also sticky: the transport wrote the empty result back into the auth cache under the UUID key, so subsequent requests took the cache-hit branch and returned the same 401 without re-querying. Add _resolve_subject_email() and call it in both _auth_jwt() and _normalize_jwt_payload(), before any cache lookup or DB query, so MCP keys the same identity as the REST paths already do via get_current_user() and resolve_session_teams(). Legacy/API tokens short-circuit on a UUID parse guard and take no extra database round-trip. An unresolvable subject returns the raw value so the existing require_user_in_db checks still deny the request rather than downgrading it to anonymous access. Closes #5215 Closes #5750 Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com> * fix(mcp): use canonical email-over-sub precedence in subject resolver _resolve_subject_email() picked sub before email, opposite of the repo's canonical get_user_email() precedence, letting a conflicting sub claim override the authoritative email (CWE-287/CWE-863). Prefer a non-empty string email claim, falling back to sub only when absent. Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com> --------- Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com> Co-authored-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
1 parent 328f950 commit 3fe3035

2 files changed

Lines changed: 327 additions & 3 deletions

File tree

mcpgateway/transports/streamablehttp_transport.py

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
import re
4141
from typing import Any, assert_never, AsyncGenerator, ContextManager, Dict, Iterable, List, Optional, Pattern, Tuple, Union
4242
from urllib.parse import urlsplit, urlunsplit
43-
from uuid import uuid4
43+
from uuid import UUID, uuid4
4444

4545
# Third-Party
4646
import anyio
@@ -2203,6 +2203,63 @@ async def _get_request_context_or_default() -> Tuple[str, dict[str, Any], dict[s
22032203
return s_id, request_headers_var.get(), user_context_var.get()
22042204

22052205

2206+
async def _resolve_subject_email(payload: dict[str, Any]) -> Optional[str]:
2207+
"""Resolve a JWT subject claim to the user's email address.
2208+
2209+
Session tokens (``token_use: "session"``) carry ``sub`` as the
2210+
``EmailUser.id`` UUID; legacy/API tokens carry the email directly. This
2211+
mirrors the resolution already performed by
2212+
``mcpgateway.auth.get_current_user`` and
2213+
``mcpgateway.auth.resolve_session_teams`` so that the MCP transport keys its
2214+
user lookups, auth-cache entries and RBAC context on the same identity as
2215+
the REST paths.
2216+
2217+
Per the repository's canonical email-over-sub precedence (see
2218+
``mcpgateway.auth_context.get_user_email``), a non-empty string ``email``
2219+
claim always wins over ``sub`` when both are present, so the resolved
2220+
identity can never be spoofed by a conflicting ``sub`` claim.
2221+
2222+
When the subject is a UUID that cannot be resolved (no matching user row),
2223+
the raw subject is returned unchanged so the caller's existing
2224+
``require_user_in_db`` checks reject the request rather than silently
2225+
downgrading it to anonymous access.
2226+
2227+
Args:
2228+
payload: Decoded JWT payload.
2229+
2230+
Returns:
2231+
The user's email address, or ``None`` when the token carries no subject.
2232+
2233+
Examples:
2234+
>>> import asyncio
2235+
>>> asyncio.run(_resolve_subject_email({"sub": "user@example.com"}))
2236+
'user@example.com'
2237+
>>> asyncio.run(_resolve_subject_email({"email": "fallback@example.com"}))
2238+
'fallback@example.com'
2239+
>>> asyncio.run(_resolve_subject_email({"email": "canonical@example.com", "sub": "ignored-sub"}))
2240+
'canonical@example.com'
2241+
>>> asyncio.run(_resolve_subject_email({})) is None
2242+
True
2243+
"""
2244+
email_claim = payload.get("email")
2245+
subject: Optional[str] = email_claim if isinstance(email_claim, str) and email_claim else payload.get("sub")
2246+
if not subject:
2247+
return None
2248+
2249+
try:
2250+
UUID(subject)
2251+
except (ValueError, AttributeError, TypeError):
2252+
# Already an email (legacy/API token, or a session token issued before
2253+
# the user row existed) — no database round-trip needed.
2254+
return subject
2255+
2256+
# First-Party
2257+
from mcpgateway.auth import _get_email_by_id_sync # pylint: disable=import-outside-toplevel
2258+
2259+
resolved = await asyncio.to_thread(_get_email_by_id_sync, subject)
2260+
return resolved or subject
2261+
2262+
22062263
async def _normalize_jwt_payload(payload: dict[str, Any]) -> dict[str, Any]:
22072264
"""Normalize a raw JWT payload to the canonical user context shape.
22082265
@@ -2218,7 +2275,7 @@ async def _normalize_jwt_payload(payload: dict[str, Any]) -> dict[str, Any]:
22182275
Returns:
22192276
Canonical user context dict with keys email, teams, is_admin, is_authenticated, token_use.
22202277
"""
2221-
email = payload.get("sub") or payload.get("email")
2278+
email = await _resolve_subject_email(payload)
22222279
jwt_is_admin = payload.get("is_admin", False)
22232280
if not jwt_is_admin:
22242281
user_info = payload.get("user", {})
@@ -5086,7 +5143,9 @@ async def _auth_jwt(self, *, token: str) -> bool: # noqa: PLR0911
50865143
from mcpgateway.cache.auth_cache import CachedAuthContext, get_auth_cache # pylint: disable=import-outside-toplevel
50875144

50885145
jti = user_payload.get("jti")
5089-
user_email = user_payload.get("sub") or user_payload.get("email")
5146+
# Resolve sub (a UUID for session tokens) to the user's email before any
5147+
# cache lookup or DB query, so MCP keys the same identity as the REST paths.
5148+
user_email = await _resolve_subject_email(user_payload)
50905149
nested_user = user_payload.get("user", {})
50915150
nested_is_admin = nested_user.get("is_admin", False) if isinstance(nested_user, dict) else False
50925151
is_admin = user_payload.get("is_admin", False) or nested_is_admin

tests/unit/mcpgateway/transports/test_streamablehttp_transport.py

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13277,6 +13277,271 @@ async def test_normalize_jwt_payload_session_admin_no_email_no_bypass():
1327713277
assert result["is_admin"] is True
1327813278

1327913279

13280+
# ---------------------------------------------------------------------------
13281+
# _resolve_subject_email tests (issue #5215)
13282+
#
13283+
# Session tokens carry sub = EmailUser.id (a UUID); legacy/API tokens carry the
13284+
# email. The MCP transport used to treat sub as an email unconditionally, so
13285+
# session tokens produced a 401 "User not found in database" on
13286+
# /servers/{id}/mcp and empty-team public-only visibility on /mcp.
13287+
# ---------------------------------------------------------------------------
13288+
13289+
SESSION_SUB_UUID = "387e3345-7996-4496-aa2d-09729ec8b3be"
13290+
13291+
13292+
@pytest.mark.asyncio
13293+
async def test_resolve_subject_email_uuid_sub_resolves_to_email():
13294+
"""A session token's UUID sub is resolved to the user's email via the DB."""
13295+
lookup = Mock(return_value="user@example.com")
13296+
with patch("mcpgateway.auth._get_email_by_id_sync", lookup):
13297+
result = await tr._resolve_subject_email({"sub": SESSION_SUB_UUID, "token_use": "session"})
13298+
13299+
assert result == "user@example.com"
13300+
lookup.assert_called_once_with(SESSION_SUB_UUID)
13301+
13302+
13303+
@pytest.mark.asyncio
13304+
async def test_resolve_subject_email_legacy_sub_skips_db_lookup():
13305+
"""An email sub short-circuits before any DB round-trip (hot-path guard)."""
13306+
lookup = Mock(return_value="should-not-be-used@example.com")
13307+
with patch("mcpgateway.auth._get_email_by_id_sync", lookup):
13308+
result = await tr._resolve_subject_email({"sub": "legacy@example.com", "token_use": "api"})
13309+
13310+
assert result == "legacy@example.com"
13311+
lookup.assert_not_called()
13312+
13313+
13314+
@pytest.mark.asyncio
13315+
async def test_resolve_subject_email_session_token_with_email_sub():
13316+
"""Session tokens issued without a user row carry an email sub (admin.py:4273)."""
13317+
lookup = Mock(return_value="should-not-be-used@example.com")
13318+
with patch("mcpgateway.auth._get_email_by_id_sync", lookup):
13319+
result = await tr._resolve_subject_email({"sub": "admin@example.com", "token_use": "session"})
13320+
13321+
assert result == "admin@example.com"
13322+
lookup.assert_not_called()
13323+
13324+
13325+
@pytest.mark.asyncio
13326+
async def test_resolve_subject_email_unresolvable_uuid_returns_raw_subject():
13327+
"""An unresolvable UUID keeps the raw subject so require_user_in_db still denies.
13328+
13329+
SECURITY: the request must fail closed on the caller's existing
13330+
"User not found in database" branch, never silently downgrade to anonymous.
13331+
"""
13332+
with patch("mcpgateway.auth._get_email_by_id_sync", Mock(return_value=None)):
13333+
result = await tr._resolve_subject_email({"sub": SESSION_SUB_UUID, "token_use": "session"})
13334+
13335+
assert result == SESSION_SUB_UUID
13336+
13337+
13338+
@pytest.mark.asyncio
13339+
async def test_resolve_subject_email_falls_back_to_email_claim():
13340+
"""Falls back to the 'email' claim when 'sub' is absent."""
13341+
result = await tr._resolve_subject_email({"email": "fallback@example.com"})
13342+
assert result == "fallback@example.com"
13343+
13344+
13345+
@pytest.mark.asyncio
13346+
async def test_resolve_subject_email_prefers_email_over_conflicting_sub():
13347+
"""SECURITY: a non-empty 'email' claim wins over 'sub' when both are present.
13348+
13349+
Mirrors the repository's canonical email-over-sub precedence
13350+
(mcpgateway.auth_context.get_user_email). Without this, a token with a
13351+
forged/stale 'sub' UUID could resolve to a different principal than the
13352+
canonical email, mismatching cache keys, team visibility, and RBAC
13353+
(CWE-287/CWE-863).
13354+
"""
13355+
lookup = Mock(return_value="should-not-be-used@example.com")
13356+
with patch("mcpgateway.auth._get_email_by_id_sync", lookup):
13357+
result = await tr._resolve_subject_email({"email": "canonical@example.com", "sub": SESSION_SUB_UUID, "token_use": "session"})
13358+
13359+
assert result == "canonical@example.com"
13360+
lookup.assert_not_called()
13361+
13362+
13363+
@pytest.mark.asyncio
13364+
async def test_resolve_subject_email_non_string_email_falls_back_to_sub():
13365+
"""A malformed (non-string) 'email' claim is ignored in favor of 'sub'."""
13366+
result = await tr._resolve_subject_email({"email": ["not", "a", "string"], "sub": "legacy@example.com", "token_use": "api"})
13367+
assert result == "legacy@example.com"
13368+
13369+
13370+
@pytest.mark.asyncio
13371+
async def test_resolve_subject_email_empty_email_falls_back_to_sub():
13372+
"""An empty-string 'email' claim is treated as absent and falls back to 'sub'."""
13373+
result = await tr._resolve_subject_email({"email": "", "sub": "legacy@example.com", "token_use": "api"})
13374+
assert result == "legacy@example.com"
13375+
13376+
13377+
@pytest.mark.asyncio
13378+
async def test_resolve_subject_email_no_subject_returns_none():
13379+
"""A payload with no subject yields None rather than raising."""
13380+
assert await tr._resolve_subject_email({}) is None
13381+
13382+
13383+
@pytest.mark.asyncio
13384+
async def test_resolve_subject_email_db_error_propagates():
13385+
"""A DB failure propagates so _auth_jwt's SQLAlchemyError handler returns 503.
13386+
13387+
SECURITY: identity resolution must fail closed. This deliberately differs
13388+
from the fail-open revocation/user lookups later in _auth_jwt — an
13389+
unresolvable subject means we do not know who the caller is.
13390+
"""
13391+
# Third-Party
13392+
from sqlalchemy.exc import SQLAlchemyError
13393+
13394+
with patch("mcpgateway.auth._get_email_by_id_sync", Mock(side_effect=SQLAlchemyError("db down"))):
13395+
with pytest.raises(SQLAlchemyError):
13396+
await tr._resolve_subject_email({"sub": SESSION_SUB_UUID, "token_use": "session"})
13397+
13398+
13399+
@pytest.mark.asyncio
13400+
async def test_normalize_jwt_payload_resolves_uuid_sub(monkeypatch):
13401+
"""The stateful-session fallback path resolves a UUID sub to the email too."""
13402+
monkeypatch.setattr("mcpgateway.auth._get_email_by_id_sync", lambda user_id: "dev@example.com")
13403+
13404+
raw = {"sub": SESSION_SUB_UUID, "token_use": "session"}
13405+
with patch("mcpgateway.auth.resolve_session_teams", new_callable=AsyncMock, return_value=["team-x"]):
13406+
result = await _normalize_jwt_payload_ref()(raw)
13407+
13408+
assert result["email"] == "dev@example.com"
13409+
assert result["teams"] == ["team-x"]
13410+
13411+
13412+
def _normalize_jwt_payload_ref():
13413+
"""Return the module's _normalize_jwt_payload (kept out of the test body for import hygiene).
13414+
13415+
Returns:
13416+
The _normalize_jwt_payload coroutine function under test.
13417+
"""
13418+
return tr._normalize_jwt_payload
13419+
13420+
13421+
# ---------------------------------------------------------------------------
13422+
# _auth_jwt: session-token identity resolution (issue #5215 / #5750)
13423+
# ---------------------------------------------------------------------------
13424+
13425+
13426+
def _auth_handler():
13427+
"""Build a _StreamableHttpAuthHandler over a minimal ASGI triple.
13428+
13429+
Returns:
13430+
A handler instance whose scope/receive/send are test doubles.
13431+
"""
13432+
return tr._StreamableHttpAuthHandler({"type": "http", "headers": []}, AsyncMock(), AsyncMock())
13433+
13434+
13435+
@pytest.mark.asyncio
13436+
async def test_auth_jwt_session_token_keys_cache_and_lookups_by_email(monkeypatch):
13437+
"""A session token's UUID sub must never reach the auth cache or the user lookup.
13438+
13439+
Regression guard for the sticky 401: before the fix the transport missed on a
13440+
UUID cache key, fell through to a UUID-keyed DB lookup, then wrote the empty
13441+
result back under that same UUID key.
13442+
"""
13443+
handler = _auth_handler()
13444+
13445+
monkeypatch.setattr(tr, "verify_credentials", AsyncMock(return_value={"sub": SESSION_SUB_UUID, "token_use": "session", "jti": "jti-1"}))
13446+
monkeypatch.setattr(tr._StreamableHttpAuthHandler, "_route_idp_issued_token", AsyncMock(return_value=None))
13447+
monkeypatch.setattr("mcpgateway.auth._get_email_by_id_sync", lambda user_id: "dev@example.com")
13448+
13449+
auth_cache = MagicMock()
13450+
auth_cache.get_auth_context = AsyncMock(return_value=None)
13451+
auth_cache.get_user_teams = AsyncMock(return_value=None)
13452+
auth_cache.set_auth_context = AsyncMock()
13453+
auth_cache.set_user_teams = AsyncMock()
13454+
monkeypatch.setattr("mcpgateway.cache.auth_cache.get_auth_cache", lambda: auth_cache)
13455+
monkeypatch.setattr(tr.settings, "auth_cache_enabled", True)
13456+
monkeypatch.setattr(tr.settings, "auth_cache_batch_queries", True)
13457+
13458+
monkeypatch.setattr(
13459+
"mcpgateway.auth._get_auth_context_batched_sync",
13460+
lambda email, jti: {"user": {"email": email, "is_admin": False, "is_active": True}, "team_ids": ["team-x"], "is_token_revoked": False, "personal_team_id": None},
13461+
)
13462+
monkeypatch.setattr("mcpgateway.auth.resolve_session_teams", AsyncMock(return_value=["team-x"]))
13463+
monkeypatch.setattr("mcpgateway.auth.resolve_trace_team_name", AsyncMock(return_value=None))
13464+
13465+
assert await handler._auth_jwt(token="tok") is True
13466+
13467+
# Every identity-keyed call must use the email, never the UUID.
13468+
assert auth_cache.get_auth_context.await_args.args[0] == "dev@example.com"
13469+
assert auth_cache.set_auth_context.await_args.args[0] == "dev@example.com"
13470+
assert auth_cache.set_user_teams.await_args.args[0] == "dev@example.com:True"
13471+
13472+
ctx = tr.user_context_var.get()
13473+
assert ctx["email"] == "dev@example.com"
13474+
assert ctx["teams"] == ["team-x"]
13475+
13476+
13477+
@pytest.mark.asyncio
13478+
async def test_auth_jwt_session_token_unknown_user_still_denied(monkeypatch):
13479+
"""Deny path preserved: an unresolvable UUID sub still 401s under require_user_in_db."""
13480+
handler = _auth_handler()
13481+
sent = []
13482+
monkeypatch.setattr(tr._StreamableHttpAuthHandler, "_send_error", AsyncMock(side_effect=lambda **kw: (sent.append(kw), False)[1]))
13483+
13484+
monkeypatch.setattr(tr, "verify_credentials", AsyncMock(return_value={"sub": SESSION_SUB_UUID, "token_use": "session", "jti": "jti-2"}))
13485+
monkeypatch.setattr(tr._StreamableHttpAuthHandler, "_route_idp_issued_token", AsyncMock(return_value=None))
13486+
monkeypatch.setattr("mcpgateway.auth._get_email_by_id_sync", lambda user_id: None)
13487+
monkeypatch.setattr(tr.settings, "auth_cache_enabled", False)
13488+
monkeypatch.setattr(tr.settings, "auth_cache_batch_queries", False)
13489+
monkeypatch.setattr(tr.settings, "require_user_in_db", True)
13490+
monkeypatch.setattr("mcpgateway.auth._check_token_revoked_sync", lambda jti: False)
13491+
monkeypatch.setattr("mcpgateway.auth._get_user_by_email_sync", lambda email: None)
13492+
13493+
assert await handler._auth_jwt(token="tok") is False
13494+
assert sent and sent[0]["detail"] == "User not found in database"
13495+
13496+
13497+
@pytest.mark.asyncio
13498+
async def test_auth_jwt_session_token_platform_admin_escape_hatch(monkeypatch):
13499+
"""Resolving sub lets the platform-admin escape hatch apply to session tokens.
13500+
13501+
SECURITY: this is the one place the fix widens access. Before the fix a
13502+
platform-admin session token carried a UUID that never equalled
13503+
platform_admin_email, so the hatch could not apply.
13504+
"""
13505+
handler = _auth_handler()
13506+
13507+
monkeypatch.setattr(tr, "verify_credentials", AsyncMock(return_value={"sub": SESSION_SUB_UUID, "token_use": "session", "is_admin": True, "jti": "jti-3"}))
13508+
monkeypatch.setattr(tr._StreamableHttpAuthHandler, "_route_idp_issued_token", AsyncMock(return_value=None))
13509+
monkeypatch.setattr("mcpgateway.auth._get_email_by_id_sync", lambda user_id: "admin@example.com")
13510+
monkeypatch.setattr(tr.settings, "auth_cache_enabled", False)
13511+
monkeypatch.setattr(tr.settings, "auth_cache_batch_queries", False)
13512+
monkeypatch.setattr(tr.settings, "require_user_in_db", True)
13513+
monkeypatch.setattr(tr.settings, "platform_admin_email", "admin@example.com")
13514+
monkeypatch.setattr("mcpgateway.auth._check_token_revoked_sync", lambda jti: False)
13515+
monkeypatch.setattr("mcpgateway.auth._get_user_by_email_sync", lambda email: None)
13516+
monkeypatch.setattr("mcpgateway.auth.resolve_session_teams", AsyncMock(return_value=None))
13517+
monkeypatch.setattr("mcpgateway.auth.resolve_trace_team_name", AsyncMock(return_value=None))
13518+
13519+
assert await handler._auth_jwt(token="tok") is True
13520+
assert tr.user_context_var.get()["email"] == "admin@example.com"
13521+
13522+
13523+
@pytest.mark.asyncio
13524+
async def test_auth_jwt_legacy_token_performs_no_subject_lookup(monkeypatch):
13525+
"""Legacy/API tokens must not incur an extra DB read on the auth hot path."""
13526+
handler = _auth_handler()
13527+
lookup = Mock(return_value="should-not-be-used@example.com")
13528+
13529+
monkeypatch.setattr(tr, "verify_credentials", AsyncMock(return_value={"sub": "legacy@example.com", "token_use": "api", "jti": "jti-4"}))
13530+
monkeypatch.setattr(tr._StreamableHttpAuthHandler, "_route_idp_issued_token", AsyncMock(return_value=None))
13531+
monkeypatch.setattr("mcpgateway.auth._get_email_by_id_sync", lookup)
13532+
monkeypatch.setattr(tr.settings, "auth_cache_enabled", False)
13533+
monkeypatch.setattr(tr.settings, "auth_cache_batch_queries", False)
13534+
monkeypatch.setattr(tr.settings, "require_user_in_db", False)
13535+
monkeypatch.setattr("mcpgateway.auth._check_token_revoked_sync", lambda jti: False)
13536+
monkeypatch.setattr("mcpgateway.auth._get_user_by_email_sync", lambda email: None)
13537+
monkeypatch.setattr("mcpgateway.auth.normalize_token_teams", lambda payload: [])
13538+
monkeypatch.setattr("mcpgateway.auth.resolve_trace_team_name", AsyncMock(return_value=None))
13539+
13540+
assert await handler._auth_jwt(token="tok") is True
13541+
lookup.assert_not_called()
13542+
assert tr.user_context_var.get()["email"] == "legacy@example.com"
13543+
13544+
1328013545
# ---------------------------------------------------------------------------
1328113546
# call_tool: recovered context propagation regression test
1328213547
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)