aminghadersohi commented on code in PR #39604:
URL: https://github.com/apache/superset/pull/39604#discussion_r3357277884
##########
tests/unit_tests/mcp_service/test_mcp_config.py:
##########
@@ -224,3 +224,133 @@ def test_get_mcp_config_respects_app_config_override() ->
None:
custom = {"execute_sql", "health_check"}
config = get_mcp_config({"MCP_DISABLED_TOOLS": custom})
assert config["MCP_DISABLED_TOOLS"] == custom
+
+
+def test_build_composite_verifier_string_prefix():
+ """A plain-string FAB_API_KEY_PREFIXES is wrapped into a single-element
list."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ "sst_" if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_",)
+
+
+def test_build_composite_verifier_list_prefix():
+ """A list FAB_API_KEY_PREFIXES is passed through as-is."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ ["sst_", "api_"] if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_", "api_")
+
+
+def test_build_composite_verifier_invalid_prefix_falls_back_to_default():
+ """A non-iterable FAB_API_KEY_PREFIXES (e.g. None) falls back to
['sst_']."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ None if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_",)
+
+
+# -- get_mcp_api_key_enabled --
+
+
+def test_get_mcp_api_key_enabled_explicit_true():
+ """MCP_API_KEY_ENABLED=True returns True regardless of FAB setting."""
+ from superset.mcp_service.mcp_config import get_mcp_api_key_enabled
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ True if key == "MCP_API_KEY_ENABLED" else default
+ )
+
+ assert get_mcp_api_key_enabled(mock_app) is True
Review Comment:
Added in 65bfa38be6:
`test_get_mcp_api_key_enabled_fab_fallback_logs_startup_warning` calls
`get_mcp_api_key_enabled(mock_app, startup_warning=True)` with
`FAB_API_KEY_ENABLED=True` (and `MCP_API_KEY_ENABLED` unset), patches
`superset.mcp_service.mcp_config.logger`, and asserts `logger.warning` was
called once (plus the return value is `True`).
##########
tests/unit_tests/mcp_service/test_mcp_config.py:
##########
@@ -224,3 +224,133 @@ def test_get_mcp_config_respects_app_config_override() ->
None:
custom = {"execute_sql", "health_check"}
config = get_mcp_config({"MCP_DISABLED_TOOLS": custom})
assert config["MCP_DISABLED_TOOLS"] == custom
+
+
+def test_build_composite_verifier_string_prefix():
+ """A plain-string FAB_API_KEY_PREFIXES is wrapped into a single-element
list."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ "sst_" if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_",)
+
+
+def test_build_composite_verifier_list_prefix():
+ """A list FAB_API_KEY_PREFIXES is passed through as-is."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ ["sst_", "api_"] if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_", "api_")
+
+
+def test_build_composite_verifier_invalid_prefix_falls_back_to_default():
+ """A non-iterable FAB_API_KEY_PREFIXES (e.g. None) falls back to
['sst_']."""
+ from superset.mcp_service.mcp_config import _build_composite_verifier
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ None if key == "FAB_API_KEY_PREFIXES" else default
+ )
+
+ result = _build_composite_verifier(mock_app, jwt_verifier=None)
+
+ assert result._api_key_prefixes == ("sst_",)
+
+
+# -- get_mcp_api_key_enabled --
+
+
+def test_get_mcp_api_key_enabled_explicit_true():
+ """MCP_API_KEY_ENABLED=True returns True regardless of FAB setting."""
+ from superset.mcp_service.mcp_config import get_mcp_api_key_enabled
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ True if key == "MCP_API_KEY_ENABLED" else default
+ )
+
+ assert get_mcp_api_key_enabled(mock_app) is True
+
+
+def test_get_mcp_api_key_enabled_explicit_false():
+ """MCP_API_KEY_ENABLED=False returns False even when FAB setting is
True."""
+ from superset.mcp_service.mcp_config import get_mcp_api_key_enabled
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ False if key == "MCP_API_KEY_ENABLED" else True
+ )
+
+ assert get_mcp_api_key_enabled(mock_app) is False
+
+
+def test_get_mcp_api_key_enabled_falls_back_to_fab():
+ """When MCP_API_KEY_ENABLED is not set, falls back to
FAB_API_KEY_ENABLED."""
+ from superset.mcp_service.mcp_config import get_mcp_api_key_enabled
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ None
+ if key == "MCP_API_KEY_ENABLED"
+ else (True if key == "FAB_API_KEY_ENABLED" else default)
+ )
+
+ assert get_mcp_api_key_enabled(mock_app) is True
+
+
+def test_get_mcp_api_key_enabled_both_absent_returns_false():
+ """When neither setting is configured, returns False."""
+ from superset.mcp_service.mcp_config import get_mcp_api_key_enabled
+
+ mock_app = MagicMock()
+ mock_app.config.get.return_value = None
+
+ assert get_mcp_api_key_enabled(mock_app) is False
+
+
+# -- create_default_mcp_auth_factory --
+
+
+def test_create_default_mcp_auth_factory_returns_none_when_disabled():
+ """Returns None when neither MCP_AUTH_ENABLED nor API key auth is on."""
+ from superset.mcp_service.mcp_config import create_default_mcp_auth_factory
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: (
+ False
+ if key in ("MCP_AUTH_ENABLED", "MCP_API_KEY_ENABLED",
"FAB_API_KEY_ENABLED")
+ else default
+ )
+
+ result = create_default_mcp_auth_factory(mock_app)
+
+ assert result is None
+
+
+def test_create_default_mcp_auth_factory_api_key_only():
+ """Returns a CompositeTokenVerifier when only API key auth is enabled."""
+ from superset.mcp_service.composite_token_verifier import
CompositeTokenVerifier
+ from superset.mcp_service.mcp_config import create_default_mcp_auth_factory
+
+ mock_app = MagicMock()
+ mock_app.config.get.side_effect = lambda key, default=None: {
+ "MCP_AUTH_ENABLED": False,
+ "MCP_API_KEY_ENABLED": True,
+ "FAB_API_KEY_PREFIXES": ["sst_"],
+ "MCP_REQUIRED_SCOPES": [],
+ }.get(key, default)
+
+ result = create_default_mcp_auth_factory(mock_app)
+
+ assert isinstance(result, CompositeTokenVerifier)
Review Comment:
Added in 65bfa38be6: three tests covering the JWT branches of
`create_default_mcp_auth_factory` —
`test_create_default_mcp_auth_factory_jwt_with_keys` (JWT enabled with a
secret, API key off → returns the built verifier),
`test_create_default_mcp_auth_factory_jwt_enabled_without_keys_returns_none`
(JWT enabled, no keys/secret, no API-key fallback → `None`, warning logged),
and `test_create_default_mcp_auth_factory_jwt_build_failure_returns_none`
(`_build_jwt_verifier` raises `ValueError`, no API-key fallback → `None`, error
logged).
--
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]
