You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/05/14 23:37:54 UTC
(superset) 02/18: fix(mcp): wire composite verifier and add ApiKey permission sync
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch work-pr-39604
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 11e44ac5bf7057cfd3e8d6a8240f0e7f01bbe66f
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Thu Apr 23 19:52:17 2026 -0400
fix(mcp): wire composite verifier and add ApiKey permission sync
Wire CompositeTokenVerifier into create_default_mcp_auth_factory,
add _api_key_passthrough detection in _resolve_user_from_jwt_context,
create ApiKey permissions in create_custom_permissions, and update
test_auth_api_key with pass-through and non-matching prefix tests.
---
superset/mcp_service/auth.py | 8 +++
superset/mcp_service/mcp_config.py | 15 +++++
superset/security/manager.py | 9 +++
tests/unit_tests/mcp_service/test_auth_api_key.py | 82 ++++++++++++++++++-----
4 files changed, 98 insertions(+), 16 deletions(-)
diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py
index 583b721b9ea..c0c7c925121 100644
--- a/superset/mcp_service/auth.py
+++ b/superset/mcp_service/auth.py
@@ -218,6 +218,14 @@ def _resolve_user_from_jwt_context(app: Any) -> User | None:
if access_token is None:
return None
+ # API key pass-through: CompositeTokenVerifier accepted this token
+ # at the transport layer but defers actual validation to
+ # _resolve_user_from_api_key() (priority 2 in get_user_from_request).
+ claims = getattr(access_token, "claims", None)
+ if isinstance(claims, dict) and claims.get("_api_key_passthrough"):
+ logger.debug("API key pass-through token detected, deferring to API key auth")
+ return None
+
# Use configurable resolver or default
from superset.mcp_service.mcp_config import default_user_resolver
diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py
index 65fdeba095d..847c46cd603 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -335,6 +335,21 @@ def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]:
auth_provider = JWTVerifier(**common_kwargs)
+ # Wrap with CompositeTokenVerifier when API key auth is enabled
+ # so that API key tokens (e.g. sst_...) pass through the transport
+ # layer instead of being rejected by the JWT verifier.
+ if app.config.get("FAB_API_KEY_ENABLED", False):
+ from superset.mcp_service.composite_token_verifier import (
+ CompositeTokenVerifier,
+ )
+
+ api_key_prefixes = app.config.get("FAB_API_KEY_PREFIXES", ["sst_"])
+ auth_provider = CompositeTokenVerifier(
+ jwt_verifier=auth_provider,
+ api_key_prefixes=api_key_prefixes,
+ )
+ logger.info("API key auth enabled for MCP (prefixes: %s)", api_key_prefixes)
+
return auth_provider
except Exception:
# Do not log the exception — it may contain the HS256 secret
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 9a5055d43fe..44c23f86597 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -1422,6 +1422,15 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self.add_permission_view_menu("can_tag", "Chart")
self.add_permission_view_menu("can_tag", "Dashboard")
+ # API Key permissions (FAB's ApiKeyApi blueprint).
+ # Superset uses AppBuilder(update_perms=False) so FAB skips
+ # permission creation during blueprint registration. Create them
+ # explicitly here so that ``superset init`` picks them up and
+ # sync_role_definitions assigns them to the Admin role.
+ if current_app.config.get("FAB_API_KEY_ENABLED", False):
+ for perm in ("can_list", "can_create", "can_get", "can_delete"):
+ self.add_permission_view_menu(perm, "ApiKey")
+
def create_missing_perms(self) -> None:
"""
Creates missing FAB permissions for datasources, schemas and metrics.
diff --git a/tests/unit_tests/mcp_service/test_auth_api_key.py b/tests/unit_tests/mcp_service/test_auth_api_key.py
index 6a0bcab6719..7fa9b293481 100644
--- a/tests/unit_tests/mcp_service/test_auth_api_key.py
+++ b/tests/unit_tests/mcp_service/test_auth_api_key.py
@@ -22,7 +22,10 @@ from unittest.mock import MagicMock, patch
import pytest
from flask import g
-from superset.mcp_service.auth import get_user_from_request
+from superset.mcp_service.auth import (
+ _resolve_user_from_jwt_context,
+ get_user_from_request,
+)
@pytest.fixture
@@ -222,25 +225,72 @@ def test_relationship_reload_failure_returns_original_user(app, mock_user) -> No
assert result is mock_user
+# -- Bearer token present but not matching API key prefix --
+
+
+@pytest.mark.usefixtures("_enable_api_keys")
+def test_non_matching_bearer_token_skips_api_key_auth(app) -> None:
+ """When a Bearer token is present but does not match FAB_API_KEY_PREFIXES
+ (e.g., a JWT token), extract_api_key_from_request returns None and API key
+ auth is skipped, falling through to the next auth method."""
+ mock_sm = MagicMock()
+ mock_sm.extract_api_key_from_request.return_value = None
+
+ with app.test_request_context(
+ headers={"Authorization": "Bearer eyJhbGciOiJIUzI1NiJ9.not-an-api-key"}
+ ):
+ g.user = None
+ app.appbuilder = MagicMock()
+ app.appbuilder.sm = mock_sm
+
+ with pytest.raises(ValueError, match="No authenticated user found"):
+ get_user_from_request()
+
+ # extract was called but returned None, so validate should NOT be called
+ mock_sm.extract_api_key_from_request.assert_called_once()
+ mock_sm.validate_api_key.assert_not_called()
+
+
+# -- API key pass-through from CompositeTokenVerifier --
+
+
+def test_jwt_context_with_api_key_passthrough_returns_none(app) -> None:
+ """When CompositeTokenVerifier passes through an API key token,
+ _resolve_user_from_jwt_context should detect the _api_key_passthrough
+ claim and return None so get_user_from_request falls through to
+ _resolve_user_from_api_key."""
+ mock_access_token = MagicMock()
+ mock_access_token.claims = {"_api_key_passthrough": True}
+
+ with patch(
+ "fastmcp.server.dependencies.get_access_token",
+ return_value=mock_access_token,
+ ):
+ result = _resolve_user_from_jwt_context(app)
+
+ assert result is None
+
+
# -- SecurityManager method name regression test --
-def test_security_manager_has_expected_api_key_methods() -> None:
+def test_security_manager_has_expected_api_key_methods(app) -> None:
"""Regression test: verify the SecurityManager method names referenced in
auth._resolve_user_from_api_key() actually exist on the FAB SecurityManager
class. This catches future renames before they silently break API key auth
- at runtime (SC-99414: _extract_api_key_from_request vs
+ at runtime (see PR #39437: _extract_api_key_from_request vs
extract_api_key_from_request)."""
- from superset import security_manager
-
- sm = security_manager
- assert hasattr(sm, "extract_api_key_from_request"), (
- "FAB SecurityManager is missing 'extract_api_key_from_request'. "
- "auth._resolve_user_from_api_key() references this method by name — "
- "update auth.py if the FAB API changed."
- )
- assert hasattr(sm, "validate_api_key"), (
- "FAB SecurityManager is missing 'validate_api_key'. "
- "auth._resolve_user_from_api_key() references this method by name — "
- "update auth.py if the FAB API changed."
- )
+ with app.app_context():
+ from superset import security_manager
+
+ sm = security_manager
+ assert hasattr(sm, "extract_api_key_from_request"), (
+ "FAB SecurityManager is missing 'extract_api_key_from_request'. "
+ "auth._resolve_user_from_api_key() references this method by name — "
+ "update auth.py if the FAB API changed."
+ )
+ assert hasattr(sm, "validate_api_key"), (
+ "FAB SecurityManager is missing 'validate_api_key'. "
+ "auth._resolve_user_from_api_key() references this method by name — "
+ "update auth.py if the FAB API changed."
+ )