fix(mcp): remove JWT ValueError g.user fallback in auth layer (#39106)

superset/mcp_service/auth.py

@@ -487,26 +487,17 @@ def _setup_user_context() -> User | None:
                 _cleanup_session_on_error()
                 continue
             logger.error("DB connection failed on retry during user setup: %s", e)
+            _cleanup_session_on_error()
             raise
         except ValueError as e:
-            # JWT user resolution failed (e.g. SAML subject not in DB).
-            # Log a security warning but fall back to middleware-provided
-            # g.user if available. This handles cases where the JWT
-            # resolver username format doesn't match the DB username
-            # (e.g., SAML subject vs email). A separate story should
-            # investigate whether any deployments hit this path and
-            # migrate them before removing the fallback entirely.
-            if has_request_context() and hasattr(g, "user") and g.user:
-                logger.warning(
-                    "SECURITY: JWT user resolution failed (%s), falling "
-                    "back to middleware-provided g.user=%s. This fallback "
-                    "should be investigated and removed once all "
-                    "deployments use consistent username resolution.",
-                    e,
-                    g.user.username,
-                )
-                user = g.user
-                break
+            # User resolution failed — fail closed. Do not fall back to
+            # g.user from middleware, as that could allow a request to
+            # proceed as a different user in multi-tenant deployments.
+            # Clear g.user so error/audit logging doesn't attribute
+            # the denied request to the middleware-provided identity.
+            logger.error("MCP user resolution failed, denying request: %s", e)
+            if has_request_context():
+                g.pop("user", None)
             raise
 
     g.user = user

tests/unit_tests/mcp_service/test_auth_user_resolution.py

@@ -427,3 +427,30 @@ def test_default_resolver_preferred_username_takes_priority() -> None:
         }
     )
     assert default_user_resolver(None, token) == "alice"
+
+
+def test_setup_user_context_propagates_valueerror(app) -> None:
+    """ValueError from get_user_from_request propagates — no g.user fallback.
+
+    This is a security-critical test: when JWT resolution explicitly fails
+    (user not in DB), the request must be denied. The auth layer must NOT
+    silently fall back to g.user from middleware.
+
+    Uses test_request_context() so g.user is preserved (not cleared by
+    the no-request-context guard), validating that the ValueError still
+    propagates even when middleware has set g.user.
+    """
+    from superset.mcp_service.auth import _setup_user_context
+
+    fallback_user = _make_mock_user("middleware_user")
+
+    with app.test_request_context():
+        g.user = fallback_user
+        with patch(
+            "superset.mcp_service.auth.get_user_from_request",
+            side_effect=ValueError("User 'ghost' not found"),
+        ):
+            with pytest.raises(ValueError, match="User 'ghost' not found"):
+                _setup_user_context()
+            # g.user should be cleared after ValueError (no misleading audit)
+            assert not hasattr(g, "user") or g.user is None