From 6dc0dc02b8e9a34202a89d5cc1826b1ddd0d5d44 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Wed, 13 May 2026 06:34:46 +0000 Subject: [PATCH] fix(mcp): fix stale patch target in auth tests and update stale docstring - Use superset.mcp_service.auth.has_request_context as patch target in test_mcp_auth_hook_clears_stale_g_user tests; patching flask.has_request_context has no effect on the module-level import already bound in auth.py - Update test_jwt_access_token_skips_api_key_auth docstring to reference API_KEY_PASSTHROUGH_CLAIM instead of the legacy _api_key_passthrough name - Add noqa: BLE001 to broad exception catch in mcp_config.py to document that the wide catch is intentional (JWT libs raise many types, secrets guard) --- superset/mcp_service/mcp_config.py | 4 ++-- tests/unit_tests/mcp_service/test_auth_api_key.py | 2 +- tests/unit_tests/mcp_service/test_auth_user_resolution.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py index fd52d407a69..973f015ba90 100644 --- a/superset/mcp_service/mcp_config.py +++ b/superset/mcp_service/mcp_config.py @@ -326,8 +326,8 @@ def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]: public_key=public_key, secret=secret, ) - except Exception: - # Do not log the exception — it may contain secrets + except Exception: # noqa: BLE001 — JWT lib raises many types; broad catch intentional + # Do not log the exception — it may contain secrets (e.g., key material) logger.error("Failed to create MCP JWT verifier") if not api_key_enabled: return None 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 11717ae0edc..546f789d498 100644 --- a/tests/unit_tests/mcp_service/test_auth_api_key.py +++ b/tests/unit_tests/mcp_service/test_auth_api_key.py @@ -250,7 +250,7 @@ def test_relationship_reload_failure_returns_original_user( @pytest.mark.usefixtures("_enable_api_keys") def test_jwt_access_token_skips_api_key_auth(app: SupersetApp) -> None: - """When the AccessToken is a plain JWT (no ``_api_key_passthrough`` claim), + """When the AccessToken is a plain JWT (no API_KEY_PASSTHROUGH_CLAIM), API key auth is skipped — the JWT was already validated by the JWT verifier and resolved in _resolve_user_from_jwt_context.""" mock_sm = MagicMock() diff --git a/tests/unit_tests/mcp_service/test_auth_user_resolution.py b/tests/unit_tests/mcp_service/test_auth_user_resolution.py index 34669e51d1e..9142cee8253 100644 --- a/tests/unit_tests/mcp_service/test_auth_user_resolution.py +++ b/tests/unit_tests/mcp_service/test_auth_user_resolution.py @@ -285,7 +285,7 @@ def test_mcp_auth_hook_clears_stale_g_user(app) -> None: # framework's autouse app_context fixture may implicitly provide # a request context in some CI environments. with ( - patch("flask.has_request_context", return_value=False), + patch("superset.mcp_service.auth.has_request_context", return_value=False), patch( "superset.mcp_service.auth.get_user_from_request", side_effect=lambda: _assert_cleared_then_return(), @@ -324,7 +324,7 @@ def test_mcp_auth_hook_clears_stale_g_user_async(app) -> None: with app.app_context(): g.user = stale_user with ( - patch("flask.has_request_context", return_value=False), + patch("superset.mcp_service.auth.has_request_context", return_value=False), patch( "superset.mcp_service.auth.get_user_from_request", side_effect=lambda: _assert_cleared_then_return(),