bito-code-review[bot] commented on code in PR #37973:
URL: https://github.com/apache/superset/pull/37973#discussion_r2911784319
##########
tests/unit_tests/security/api_test.py:
##########
@@ -39,3 +40,21 @@ def test_csrf_not_exempt(app_context: None) -> None:
"PermissionApi",
"ViewMenuApi",
}
+
+
[email protected](
+ "app",
+ [
Review Comment:
<!-- Bito Reply -->
Yes, the suggestion is valid — adding `app` to the function signature
ensures the indirect parametrization passes the config dict to the `app`
fixture, as the parameter name must match the fixture for indirect
parametrization to work.
**tests/unit_tests/security/api_test.py**
```
@pytest.mark.parametrize(
"app",
[
{
"WTF_CSRF_ENABLED": True,
"FAB_API_KEY_ENABLED": True,
}
],
indirect=True,
)
def test_csrf_exempt_blueprints_with_api_key(app: Any, app_context: None) ->
None:
```
##########
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:
<!-- Bito Reply -->
The change improves resilience by logging a warning and falling back to the
original user object when `load_user_with_relationships` fails, instead of
raising an error. This keeps API key auth working while making lazy-load issues
diagnosable via logs.
--
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]
