bito-code-review[bot] commented on code in PR #37973:
URL: https://github.com/apache/superset/pull/37973#discussion_r2908685452
##########
superset/mcp_service/auth.py:
##########
@@ -107,6 +116,33 @@ def get_user_from_request() -> User:
if hasattr(g, "user") and g.user:
return g.user
+ # Try API key authentication via FAB SecurityManager
+ # Only attempt when in a request context (not for MCP internal operations
+ # like tool discovery that run with only an application context)
+ # Avoid circular import: superset/__init__.py imports create_app which
+ # depends on the MCP service module tree during app initialization.
+ from superset import is_feature_enabled
+
+ if is_feature_enabled("FAB_API_KEY_ENABLED") and has_request_context():
+ sm = current_app.appbuilder.sm
+ # _extract_api_key_from_request is FAB's internal method for reading
+ # the Bearer token from the Authorization header and matching prefixes.
+ # No public API is exposed for this; see FAB SecurityManager.
+ api_key_string = sm._extract_api_key_from_request()
+ if api_key_string is not None:
+ user = sm.validate_api_key(api_key_string)
+ if user:
+ # Reload user with all relationships eagerly loaded to avoid
+ # detached-instance errors during later permission checks.
+ user_with_rels = load_user_with_relationships(
+ username=user.username,
+ )
+ return user_with_rels or user
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>API Key Auth Fallback Risk</b></div>
<div id="fix">
The code falls back to the original user object if
load_user_with_relationships fails after API key validation, but this user may
not have relationships loaded, risking detached instance errors during
permission checks. It looks like raising an error instead ensures relationships
are always loaded as intended by the comment.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
if user:
# Reload user with all relationships eagerly loaded to avoid
# detached-instance errors during later permission checks.
user_with_rels = load_user_with_relationships(
username=user.username,
)
if user_with_rels:
return user_with_rels
else:
raise ValueError(
"Failed to reload user with relationships after API
key validation."
)
````
</div>
</details>
</div>
<small><i>Code Review Run #5e88a0</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
