fix(mcp): resolve session-token sub UUID to email in streamable-HTTP auth#5802
Conversation
…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>
5859e12 to
cdd5cbe
Compare
Lang-Akshay
left a comment
There was a problem hiding this comment.
Thanks for the PR @msureshkumar88
Please address following blocking Changes
| # | Area | File | Line | Blocking reason | Required change |
|---|---|---|---|---|---|
| 1 | Security / Quality | mcpgateway/transports/streamablehttp_transport.py |
2237 (also consumed at 5140) | _resolve_subject_email() uses payload.get("sub") or payload.get("email"). The repository security invariant requires canonical email-over-sub precedence when both claims exist. Selecting sub first can make cache keys, team visibility, and RBAC evaluate a different principal than the canonical email, causing identity/authz confusion. The security specialist classified this as Critical, CWE-287/CWE-863. |
Validate a non-empty string email first, fall back to sub, then UUID-resolve only the selected subject. Add deny-path coverage for conflicting email/sub claims. |
_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>
|
Thanks for the catch, @Lang-Akshay — good call flagging the precedence order. Fixed in 5688ee8: Added deny-path coverage:
Full transport suite (563 tests) still green. Ready for another look. |
|
Closes #5215
Closes #5750
Problem
Session tokens issued by
/admin/logincarrysubas theEmailUser.idUUID (mcpgateway/admin.py:4273), while legacy/API tokens carry the email. The MCP streamable-HTTP auth path treatedsubas an email unconditionally:streamablehttp_transport.py:5089—_auth_jwt()streamablehttp_transport.py:2221—_normalize_jwt_payload()(stateful-session fallback)So the
EmailUserlookup missed for every session token, with two different symptoms depending on configuration:require_user_in_dbenabled,401 {"detail": "User not found in database"}on/servers/{id}/mcp. LLM Chat could not connect to a virtual server hosted by the same gateway.subas an email — non-admin session tokens see 0 tools and failtools.executeRBAC #5750 — on the unified/mcpendpoint the miss resolved empty teams instead, giving public-only visibility (tools/listreturns 0) and a UUID-keyed RBAC check that deniedtools/call. This affected every external agent runtime using a real session token, not just LLM Chat.The failure was also sticky: the transport writes back to the auth cache (
:5153,:5164,:5206) under the same unresolved key, so it cachedCachedAuthContext(user=None)against the UUID. Subsequent requests took the cache-hit branch and returned the same 401 without re-querying.Fix
Add
_resolve_subject_email()and call it in both sites, before any cache lookup or DB query. This mirrors resolution the rest of the codebase already does —mcpgateway/auth.py:1581(get_current_user) andauth.py:553(resolve_session_teams) — reusing the existing_get_email_by_id_sync()helper. The MCP transport was the one path that never got it.Notes on the approach:
llmchat_router. The issue proposed minting a fresh loopback token inconnect(). That resolves this one call site but leaves every external MCP client on a session token broken, sollmchat_routeris unchanged — it keeps forwarding the session cookie, and the loopback call now authenticates as the real user, correctly scoped to their teams and RBAC.UUID()parse guard short-circuits legacy/API tokens before any lookup. Net effect is a reduction: session tokens currently miss the cache on every request and re-run the full fallback path.require_user_in_dbchecks still deny, rather than downgrading to anonymous. A DB failure raisesSQLAlchemyError, which the existing handler at:5344turns into 503 — deliberately unlike the fail-open revocation/user lookups at:5183/:5194, since an unresolvable subject means we don't know who the caller is./mcp,/servers/{id}/mcp,/mcp/sse,/mcp/message) share_auth_jwt, so all are fixed together.Behaviour change worth a look during review
The three deny branches read
elif settings.require_user_in_db and user_email != platform_admin_email. Previously a platform-admin session token carried a UUID that never equalledplatform_admin_email, so the admin escape hatch could not apply. It now does. That is the intended semantics of the hatch, but it is a widening on a security branch, so it has a dedicated test (test_auth_jwt_session_token_platform_admin_escape_hatch).Related, not included
mcpgateway/routers/reverse_proxy.py:450/:481use the samesub-or-emailshape for session ownership. It is not broken the same way —_validate_session_ownershipcomparessubagainstsub, so both forms match. The only divergence is a session created under one token type and accessed under the other, which yields 403 (fail-closed). Left alone here; happy to file separately if it's worth normalizing.Tests
Added to
tests/unit/mcpgateway/transports/test_streamablehttp_transport.py, covering the resolver directly, both patched call sites, and the deny paths: unknown user still 401s, legacy tokens perform no subject lookup, DB errors propagate to 503, and cache/lookup calls are keyed by email rather than UUID.Verified the behavioural tests fail against the unpatched transport and pass with the fix.