Repository navigation
fix(mcp): prevent stale g.user from causing user impersonation across tool calls - #38747
Merged
aminghadersohi merged 9 commits intoMar 30, 2026
Conversation
Contributor
Code Review Agent Run #97f378Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
authentication
codeant-ai-for-open-source
Bot
added
the
size:L
Contributor
Sequence DiagramThis PR changes MCP authentication to resolve the user from per-request JWT context before any shared Flask state. It also clears stale g.user at the start of each tool call and fails closed when a JWT user does not exist in the database. sequenceDiagram
participant Tool as MCP Tool Call
participant Hook as Auth Hook
participant Auth as User Resolver
participant DB as Superset Database
Tool->>Hook: Invoke tool
Hook->>Hook: Clear stale g.user
Hook->>Auth: Resolve current user
alt JWT context available
Auth->>DB: Load user from JWT claims
DB-->>Auth: User found or missing
alt User missing
Auth-->>Hook: Raise authentication error
else User found
Auth-->>Hook: Return JWT user
end
else No JWT context
Auth-->>Hook: Fallback to MCP_DEV_USERNAME then g.user
end
Hook->>Tool: Execute as resolved user
Generated by CodeAnt AI |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #38747 +/- ##
==========================================
- Coverage 64.48% 64.47% -0.02%
==========================================
Files 2536 2536
Lines 130862 130895 +33
Branches 30358 30367 +9
==========================================
+ Hits 84383 84389 +6
- Misses 45016 45043 +27
Partials 1463 1463
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
Code Review Agent Run #83b5ffActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
aminghadersohi
force-pushed
the
amin/fix-mcp-stale-g-user-impersonation
branch
from
March 24, 2026 09:52
508869d to
17b0314
Compare
codeant-ai-for-open-source
Bot
added
size:XL
Contributor
Code Review Agent Run #bdda03Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
aminghadersohi
force-pushed
the
amin/fix-mcp-stale-g-user-impersonation
branch
from
March 29, 2026 12:19
4c10be6 to
a18140c
Compare
codeant-ai-for-open-source
Bot
added
size:XL
Contributor
Code Review Agent Run #2c5cc6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Contributor
Code Review Agent Run #c56f00Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
aminghadersohi
force-pushed
the
amin/fix-mcp-stale-g-user-impersonation
branch
from
March 30, 2026 14:41
3f7168c to
c8e3bae
Compare
codeant-ai-for-open-source
Bot
added
size:XL
… tool calls In non-Preset (open source) environments, g.user could persist between MCP tool calls, causing one user's actions to execute under another user's identity. This changes the auth priority to use the MCP SDK's per-request JWT context (ContextVar-based) first, falling back to MCP_DEV_USERNAME, then g.user as a last resort for middleware compat. Also clears g.user at the start of each mcp_auth_hook call and fails closed when JWT resolves a username not found in the DB.
- Fail closed when JWT present but no username extractable (don't fall through to weaker auth sources) - Add email fallback lookup when resolver returns an email address - Only clear g.user when no request context is active (preserve middleware-set identity in request-scoped contexts) - Reorder claims priority: prefer preferred_username/username/email over opaque sub (common OIDC pattern) - Use asyncio.run() instead of get_event_loop().run_until_complete() - Add tests for email fallback and request-context preservation
…avior
Replace hard-mocked return_value with side_effect callbacks that assert
g.user state before returning the mock user. This ensures:
- Stale user tests fail if g.pop("user") is removed from mcp_auth_hook
- Preserved user test fails if g.user is incorrectly cleared in request context
Addresses review feedback from codeant-ai bot.
…10 compat
The g.pop("user", None) call was inside sync_wrapper/async_wrapper which
gets copied via types.FunctionType(wrapper.__code__, merged_globals).
The merged_globals override causes Python 3.10's LOAD_GLOBAL bytecode to
not properly resolve flask.g.pop, making the stale g.user tests fail in
CI (Python 3.10) while passing locally (Python 3.12).
Moving the clearing logic into _setup_user_context() ensures it executes
as a regular function call via closure (LOAD_DEREF) rather than depending
on the merged globals dictionary.
Extract API key resolution logic from get_user_from_request() into a dedicated helper function to reduce cyclomatic complexity below the ruff C901 threshold of 10.
aminghadersohi
force-pushed
the
amin/fix-mcp-stale-g-user-impersonation
branch
from
March 30, 2026 15:03
c8e3bae to
92c6d78
Compare
The test_existing_g_user_takes_precedence test was written for the old priority order where g.user was checked first. Update it to match the new priority (JWT > API key > dev username > g.user fallback).
Codebase uses @pytest.fixture without parentheses. Local ruff adds them but CI ruff removes them. Match CI/codebase convention.
Contributor
Code Review Agent Run #d202fcActionable Suggestions - 0Additional Suggestions - 4
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…mpat CI's test framework may implicitly provide a request context even when only app.app_context() is used, causing has_request_context() to return True and skip the g.user clearing. Explicitly mock it to False to ensure consistent behavior across local and CI environments.
_setup_user_context does a local 'from flask import has_request_context' which creates a fresh binding that bypasses the module-level patch. Patch flask.has_request_context directly to ensure the mock is picked up.
Contributor
Code Review Agent Run #b76a56Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Antonio-RiveroMartnez
approved these changes
Mar 30, 2026
3 tasks done
qfcwell
pushed a commit
to qfcwell/superset
that referenced
this pull request
May 12, 2026
Open
3 tasks done
rad-pat
added a commit
to PlaidCloud/incubator-superset
that referenced
this pull request
Aug 20, 2026
Every MCP tool call failed with "No authenticated user found" on every tenant. Nothing bridged the verified JWT to a Superset user: FastMCP produced an AccessToken with claims and nothing read it, while mcp_config.default_user_resolver sat unreferenced. get_user_from_request() knew only a pre-set g.user -- populated solely by Preset's hosted WorkspaceContextMiddleware, which we do not run -- and MCP_DEV_USERNAME, which is unset in production. Same bug as apache#41610. Backport _resolve_user_from_jwt_context and the claims-aware default_user_resolver from apache#38747, absent from 6.1.0. The cherry-pick does not apply (master has since drifted to an MCPUser union via apache#39604/apache#41003), so this ports the function as introduced. JWT now takes priority over MCP_DEV_USERNAME and g.user, so a stale g.user from a previous tool call can never win, and both no-identity and unknown-user cases fail closed rather than falling through to a weaker source. The "no auth source" message is now generic, with the configuration diagnostics kept to the server log. Upstream's resolver prefers preferred_username, which matches no user row here: oauth_user_info sets the Superset username from the Keycloak display name and auth_user_oauth keys identity on email. Add plaid_user_resolver, wired via MCP_USER_RESOLVER, returning the email claim. It deliberately has no client_id fallback -- that identifies the MCP client, not a person. Routing authentication through the email lookup makes sc-24028 live: it was case-sensitive and used .first() with no ordering, so an ambiguous match resolved to an arbitrary user with that user's permissions. Compare case-insensitively and resolve deterministically via _resolve_single_user, mirroring plaid.security (active row, then exact case, then lowest id). Also make the OAuth proxy's accepted client redirect patterns configurable. They were hardcoded to loopback, and FastMCP raises when patterns are set and none match, so claude.ai web could never complete OAuth on any tenant -- the Keycloak realm never sees the client's redirect URI, only <base_url>/auth/callback. Default now covers loopback plus the Claude web callback, overridable with MCP_OAUTH_ALLOWED_CLIENT_REDIRECT_URIS. plaid/mcp_oauth.py is additionally converted to ruff's quote style; CI scopes pre-commit to the files a PR touches, so its pre-existing drift would otherwise fail this change. Verified on pw-bug-fixes from claude.ai web: OAuth completed and list_dashboards (RBAC-gated on Dashboard) returned data. sc-25291 sc-24028 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mk6GoSg5qpMXWpFBQ5vtL4
rad-pat
added a commit
to PlaidCloud/incubator-superset
that referenced
this pull request
Aug 20, 2026
Every MCP tool call failed with "No authenticated user found" on every tenant. Nothing bridged the verified JWT to a Superset user: FastMCP produced an AccessToken with claims and nothing read it, while mcp_config.default_user_resolver sat unreferenced. get_user_from_request() knew only a pre-set g.user -- populated solely by Preset's hosted WorkspaceContextMiddleware, which we do not run -- and MCP_DEV_USERNAME, which is unset in production. Same bug as apache#41610. Backport _resolve_user_from_jwt_context and the claims-aware default_user_resolver from apache#38747, absent from 6.1.0. The cherry-pick does not apply (master has since drifted to an MCPUser union via apache#39604/apache#41003), so this ports the function as introduced. JWT now takes priority over MCP_DEV_USERNAME and g.user, so a stale g.user from a previous tool call can never win, and both no-identity and unknown-user cases fail closed rather than falling through to a weaker source. The "no auth source" message is now generic, with the configuration diagnostics kept to the server log. Upstream's resolver prefers preferred_username, which matches no user row here: oauth_user_info sets the Superset username from the Keycloak display name and auth_user_oauth keys identity on email. Add plaid_user_resolver, wired via MCP_USER_RESOLVER, returning the email claim. It deliberately has no client_id fallback -- that identifies the MCP client, not a person. Routing authentication through the email lookup makes sc-24028 live: it was case-sensitive and used .first() with no ordering, so an ambiguous match resolved to an arbitrary user with that user's permissions. Compare case-insensitively and resolve deterministically via _resolve_single_user, mirroring plaid.security (active row, then exact case, then lowest id). Also make the OAuth proxy's accepted client redirect patterns configurable. They were hardcoded to loopback, and FastMCP raises when patterns are set and none match, so claude.ai web could never complete OAuth on any tenant -- the Keycloak realm never sees the client's redirect URI, only <base_url>/auth/callback. Default now covers loopback plus the Claude web callback, overridable with MCP_OAUTH_ALLOWED_CLIENT_REDIRECT_URIS. plaid/mcp_oauth.py is additionally converted to ruff's quote style; CI scopes pre-commit to the files a PR touches, so its pre-existing drift would otherwise fail this change. Verified on pw-bug-fixes from claude.ai web: OAuth completed and list_dashboards (RBAC-gated on Dashboard) returned data. sc-25291 sc-24028 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mk6GoSg5qpMXWpFBQ5vtL4
rad-pat
added a commit
to PlaidCloud/incubator-superset
that referenced
this pull request
Aug 20, 2026
Every MCP tool call failed with "No authenticated user found" on every tenant. Nothing bridged the verified JWT to a Superset user: FastMCP produced an AccessToken with claims and nothing read it, while mcp_config.default_user_resolver sat unreferenced. get_user_from_request() knew only a pre-set g.user -- populated solely by Preset's hosted WorkspaceContextMiddleware, which we do not run -- and MCP_DEV_USERNAME, which is unset in production. Same bug as apache#41610. Backport _resolve_user_from_jwt_context and the claims-aware default_user_resolver from apache#38747, absent from 6.1.0. The cherry-pick does not apply (master has since drifted to an MCPUser union via apache#39604/apache#41003), so this ports the function as introduced. JWT now takes priority over MCP_DEV_USERNAME and g.user, so a stale g.user from a previous tool call can never win, and both no-identity and unknown-user cases fail closed rather than falling through to a weaker source. The "no auth source" message is now generic, with the configuration diagnostics kept to the server log. Upstream's resolver prefers preferred_username, which matches no user row here: oauth_user_info sets the Superset username from the Keycloak display name and auth_user_oauth keys identity on email. Add plaid_user_resolver, wired via MCP_USER_RESOLVER, returning the email claim. It deliberately has no client_id fallback -- that identifies the MCP client, not a person. Routing authentication through the email lookup makes sc-24028 live: it was case-sensitive and used .first() with no ordering, so an ambiguous match resolved to an arbitrary user with that user's permissions. Compare case-insensitively and resolve deterministically via _resolve_single_user, mirroring plaid.security (active row, then exact case, then lowest id). Also make the OAuth proxy's accepted client redirect patterns configurable. They were hardcoded to loopback, and FastMCP raises when patterns are set and none match, so claude.ai web could never complete OAuth on any tenant -- the Keycloak realm never sees the client's redirect URI, only <base_url>/auth/callback. Default now covers loopback plus the Claude web callback, overridable with MCP_OAUTH_ALLOWED_CLIENT_REDIRECT_URIS. plaid/mcp_oauth.py is additionally converted to ruff's quote style; CI scopes pre-commit to the files a PR touches, so its pre-existing drift would otherwise fail this change. Verified on pw-bug-fixes from claude.ai web: OAuth completed and list_dashboards (RBAC-gated on Dashboard) returned data. sc-25291 sc-24028 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mk6GoSg5qpMXWpFBQ5vtL4
1 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
SUMMARY
In certain Superset deployments,
get_user_from_request()inmcp_service/auth.pyblindly returnsg.userif already set — without verifying it belongs to the current request. Sincemcp_auth_hookpushes Flask app context only (not request context),g.usercan persist between tool calls, leading to privilege escalation where one user's MCP actions execute under another user's identity.Root cause:
mcp_auth_hookusesnullcontext()if already in an app context, sogpersists._setup_user_context()setsg.user = user, and the next tool call finds the staleg.userand returns it immediately.Fix — changed auth priority order in
get_user_from_request():get_access_token())MCP_DEV_USERNAMEconfigg.userfallbackValueErrorAdditional hardening:
g.userat the start of eachmcp_auth_hookwrapper (belt-and-suspenders)default_user_resolver()to checkAccessToken.claimsdict first (FastMCP format) and addedpreferred_usernameOIDC claim supportSafe for all environments:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend auth logic change, no UI.
TESTING INSTRUCTIONS
pytest tests/unit_tests/mcp_service/test_auth_user_resolution.py -x -q(20 tests)pytest tests/unit_tests/mcp_service/test_auth_rbac.py -x -q(15 tests)pytest tests/unit_tests/mcp_service/ -x -q(871 tests)ADDITIONAL INFORMATION
MCP Tool Testing Results
Tested against running Superset MCP service (localhost:5008) on 2026-03-24:
health_check{}{"status":"healthy","service":"Preset MCP Service","version":"amin/ch101753/bump-fastmcp-3.1.0"}get_instance_info{}list_dashboards{"page":1,"page_size":3}list_charts{"page":1,"page_size":3}list_datasets{"page":1,"page_size":3}get_chart_info{"identifier":4}execute_sql{"database_id":2,"sql":"SELECT 1 as test_col, 'hello' as test_str"}{"test_col":1,"test_str":"hello"}in 0.13sgenerate_explore_link{"dataset_id":19,"config":{"chart_type":"xy","kind":"bar",...}}All tools executed successfully with proper auth context (Admin user).
CodeAnt-AI Description
Prevent stale MCP user data from being reused across tool calls
What Changed
g.useris cleared before tool setup in app-only flows, while request-scoped middleware identities are preservedImpact
✅ Fewer cross-user impersonation errors✅ Safer MCP tool calls✅ Clearer authentication behavior for Preset and JWT deployments💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.