This is an automated email from the ASF dual-hosted git repository. aminghadersohi pushed a commit to branch revert-40309-research-mcp-hello-page in repository https://gitbox.apache.org/repos/asf/superset.git
commit cf13cf5a3a348070f98de572948de69ea87d6fb4 Author: Amin Ghadersohi <[email protected]> AuthorDate: Wed May 27 09:57:57 2026 -0400 Revert "feat(mcp): return browser-friendly hello page for GET /mcp from brows…" This reverts commit 09a94fa26b4c0aa1bc97629031de3cd9077caf23. --- MANIFEST.in | 1 - superset/mcp_service/hello.html | 115 ------------------- superset/mcp_service/jwt_verifier.py | 78 ++----------- superset/mcp_service/mcp_config.py | 6 +- tests/unit_tests/mcp_service/test_jwt_verifier.py | 6 +- .../mcp_service/test_jwt_verifier_browser_hello.py | 126 --------------------- 6 files changed, 17 insertions(+), 315 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 970a1581b12..4d7a98b2a7a 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -21,7 +21,6 @@ include README.md include superset-frontend/package.json recursive-include superset/examples * recursive-include superset/migrations * -recursive-include superset/mcp_service *.html recursive-include superset/templates * recursive-include superset/translations * recursive-include superset/static * diff --git a/superset/mcp_service/hello.html b/superset/mcp_service/hello.html deleted file mode 100644 index 6bf5d953c74..00000000000 --- a/superset/mcp_service/hello.html +++ /dev/null @@ -1,115 +0,0 @@ -<!-- -Licensed to the Apache Software Foundation (ASF) under one -or more contributor license agreements. See the NOTICE file -distributed with this work for additional information -regarding copyright ownership. The ASF licenses this file -to you under the Apache License, Version 2.0 (the -"License"); you may not use this file except in compliance -with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, -software distributed under the License is distributed on an -"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -KIND, either express or implied. See the License for the -specific language governing permissions and limitations -under the License. ---> -<!DOCTYPE html> -<html lang="en"> -<head> - <meta charset="UTF-8"> - <meta name="viewport" content="width=device-width, initial-scale=1.0"> - <title>Superset MCP Server</title> - <style> - *, *::before, *::after { box-sizing: border-box; } - body { - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; - background: #f5f5f5; - color: #1a1a1a; - margin: 0; - padding: 40px 16px; - line-height: 1.6; - } - .card { - max-width: 640px; - margin: 0 auto; - background: #ffffff; - border-radius: 8px; - box-shadow: 0 1px 4px rgba(0,0,0,.12); - padding: 40px 40px 32px; - } - h1 { font-size: 1.4rem; margin: 0 0 8px; } - .badge { - display: inline-block; - background: #e8f4fd; - color: #0070c0; - font-size: .75rem; - font-weight: 600; - padding: 2px 8px; - border-radius: 4px; - margin-bottom: 20px; - letter-spacing: .04em; - text-transform: uppercase; - } - p { margin: 0 0 20px; color: #444; } - h2 { font-size: 1rem; margin: 24px 0 8px; color: #1a1a1a; } - pre { - background: #f0f0f0; - border-radius: 6px; - padding: 16px; - font-size: .85rem; - overflow-x: auto; - margin: 0 0 24px; - } - code { - font-family: "SFMono-Regular", Consolas, "Liberation Mono", Menlo, monospace; - } - .note { - font-size: .85rem; - color: #666; - border-left: 3px solid #ddd; - padding-left: 12px; - margin-top: 24px; - } - </style> -</head> -<body> - <div class="card"> - <div class="badge">MCP API Endpoint</div> - <h1>Superset MCP Server</h1> - <p> - This is the <strong>Model Context Protocol (MCP)</strong> endpoint for - Apache Superset. It is an API designed for AI coding assistants — - not a web page to browse directly. - </p> - <h2>How to connect</h2> - <p>Add the following to your MCP client configuration, - replacing the URL and API key with your actual values:</p> - <pre><code>{ - "mcpServers": { - "superset": { - "url": "<this-url>", - "transport": "streamable-http", - "headers": { - "Authorization": "Bearer <your-api-key>" - } - } - } -}</code></pre> - <h2>Supported clients</h2> - <p>This endpoint works with any MCP-compatible client, including:</p> - <ul style="color:#444;margin:0 0 20px;padding-left:20px;"> - <li>Claude Desktop</li> - <li>Claude Code (CLI)</li> - <li>Cursor</li> - <li>Any client that supports the <code>streamable-http</code> transport</li> - </ul> - <div class="note"> - Replace <code><this-url></code> with the full URL of this page and - <code><your-api-key></code> with a valid Superset API key or JWT token. - </div> - </div> -</body> -</html> diff --git a/superset/mcp_service/jwt_verifier.py b/superset/mcp_service/jwt_verifier.py index cd69fea08b6..abd8019d9e4 100644 --- a/superset/mcp_service/jwt_verifier.py +++ b/superset/mcp_service/jwt_verifier.py @@ -29,7 +29,6 @@ import base64 import logging import time from contextvars import ContextVar -from importlib.resources import files as _resource_files from typing import Any, cast from authlib.jose.errors import ( @@ -46,7 +45,7 @@ from starlette.authentication import AuthenticationError from starlette.middleware import Middleware from starlette.middleware.authentication import AuthenticationMiddleware from starlette.requests import HTTPConnection -from starlette.responses import HTMLResponse, JSONResponse, Response +from starlette.responses import JSONResponse from superset.utils import json @@ -62,54 +61,21 @@ _jwt_failure_reason: ContextVar[str | None] = ContextVar( "_jwt_failure_reason", default=None ) -_MCP_BROWSER_HELLO_HTML: str | None = None +def _json_auth_error_handler( + conn: HTTPConnection, exc: AuthenticationError +) -> JSONResponse: + """JSON 401 error handler for authentication failures. -def _get_browser_hello_html() -> str: - global _MCP_BROWSER_HELLO_HTML - if _MCP_BROWSER_HELLO_HTML is None: - _MCP_BROWSER_HELLO_HTML = ( - _resource_files("superset.mcp_service") - .joinpath("hello.html") - .read_text(encoding="utf-8") - ) - return _MCP_BROWSER_HELLO_HTML - - -def _prefers_browser_html(conn: HTTPConnection) -> bool: - """Return True when the request looks like a browser navigation. - - Checks both the HTTP method (GET/HEAD only) and the Accept header - (text/html present, application/json and text/event-stream absent). - Case-insensitive to handle unusual but valid header values. - """ - if conn.scope.get("method") not in ("GET", "HEAD"): - return False - accept = conn.headers.get("accept", "").lower() - return ( - "text/html" in accept - and "application/json" not in accept - and "text/event-stream" not in accept - ) - - -def _auth_error_handler(conn: HTTPConnection, exc: AuthenticationError) -> Response: - """Auth error handler for unauthenticated MCP requests. - - Returns a friendly HTML page for browser navigation requests so users - who open the MCP URL in a browser see setup instructions instead of a - raw JSON 401. - - For all other clients (API, SSE, non-GET methods) returns a standard - JSON 401 per RFC 6750 Section 3.1. + Per RFC 6750 Section 3.1, error responses MUST NOT leak server + configuration or token claim values. Only generic error codes are + returned to clients. Detailed failure reasons are logged server-side + only for debugging. References: - RFC 6750 Section 3.1: https://datatracker.ietf.org/doc/html/rfc6750#section-3.1 - CVE-2022-29266, CVE-2019-7644: verbose JWT errors led to exploits """ - if _prefers_browser_html(conn): - return HTMLResponse(status_code=200, content=_get_browser_hello_html()) - # Log detailed reason server-side only logger.warning("JWT authentication failed: %s", exc) @@ -125,28 +91,6 @@ def _auth_error_handler(conn: HTTPConnection, exc: AuthenticationError) -> Respo ) -class MCPJWTVerifier(JWTVerifier): - """JWTVerifier with Superset MCP auth error handling. - - Provides browser-friendly HTML responses for unauthenticated browser - navigation requests (GET/HEAD with Accept: text/html), while maintaining - RFC 6750-compliant JSON 401 responses for API and SSE clients. - - Use this as the base for all Superset JWT verifiers so the browser hello - page is active regardless of which verifier variant is configured. - """ - - def get_middleware(self) -> list[Any]: - return [ - Middleware( - AuthenticationMiddleware, - backend=BearerAuthBackend(self), - on_error=_auth_error_handler, - ), - Middleware(AuthContextMiddleware), - ] - - class DetailedBearerAuthBackend(BearerAuthBackend): """ Bearer auth backend that raises AuthenticationError with specific @@ -180,7 +124,7 @@ class DetailedBearerAuthBackend(BearerAuthBackend): return None -class DetailedJWTVerifier(MCPJWTVerifier): +class DetailedJWTVerifier(JWTVerifier): """ JWT verifier with tiered server-side logging for each validation step. @@ -356,7 +300,7 @@ class DetailedJWTVerifier(MCPJWTVerifier): Middleware( AuthenticationMiddleware, backend=DetailedBearerAuthBackend(self), - on_error=_auth_error_handler, + on_error=_json_auth_error_handler, ), Middleware(AuthContextMiddleware), ] diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py index 997755f1b1c..d12b44bbc87 100644 --- a/superset/mcp_service/mcp_config.py +++ b/superset/mcp_service/mcp_config.py @@ -343,10 +343,10 @@ def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]: auth_provider = DetailedJWTVerifier(**common_kwargs) else: - # MCPJWTVerifier: minimal logging + browser-friendly error page. - from superset.mcp_service.jwt_verifier import MCPJWTVerifier + # Default JWTVerifier: minimal logging, generic error responses. + from fastmcp.server.auth.providers.jwt import JWTVerifier - auth_provider = MCPJWTVerifier(**common_kwargs) + auth_provider = JWTVerifier(**common_kwargs) return auth_provider except Exception: diff --git a/tests/unit_tests/mcp_service/test_jwt_verifier.py b/tests/unit_tests/mcp_service/test_jwt_verifier.py index 0eb934696bc..b6ba58aa7f2 100644 --- a/tests/unit_tests/mcp_service/test_jwt_verifier.py +++ b/tests/unit_tests/mcp_service/test_jwt_verifier.py @@ -26,7 +26,7 @@ import pytest from authlib.jose.errors import BadSignatureError, DecodeError, ExpiredTokenError from superset.mcp_service.jwt_verifier import ( - _auth_error_handler, + _json_auth_error_handler, _jwt_failure_reason, DetailedBearerAuthBackend, DetailedJWTVerifier, @@ -400,7 +400,7 @@ def test_get_middleware_returns_custom_components(hs256_verifier): == "DetailedBearerAuthBackend" ) # on_error should be the RFC 6750-compliant generic handler - assert auth_middleware.kwargs["on_error"] is _auth_error_handler + assert auth_middleware.kwargs["on_error"] is _json_auth_error_handler class _FakeHeaders(dict[str, str]): @@ -496,7 +496,7 @@ def test_error_handler_never_leaks_jwt_details(): for reason in sensitive_reasons: exc = AuthenticationError(reason) - response = _auth_error_handler(mock_conn, exc) + response = _json_auth_error_handler(mock_conn, exc) assert response.status_code == 401 diff --git a/tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py b/tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py deleted file mode 100644 index baece40b1be..00000000000 --- a/tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py +++ /dev/null @@ -1,126 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""Tests for browser-friendly hello page in _auth_error_handler.""" - -from unittest.mock import MagicMock - -from starlette.authentication import AuthenticationError -from starlette.responses import HTMLResponse, JSONResponse - -from superset.mcp_service.jwt_verifier import _auth_error_handler - - -def _make_conn(accept: str, method: str = "GET") -> MagicMock: - conn = MagicMock() - conn.headers = {"accept": accept} if accept else {} - conn.scope = {"method": method} - return conn - - -def test_browser_accept_returns_html_200() -> None: - conn = _make_conn("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, HTMLResponse) - assert response.status_code == 200 - assert b"MCP" in response.body - assert b"mcpServers" in response.body - - -def test_browser_accept_html_only_returns_200() -> None: - conn = _make_conn("text/html") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, HTMLResponse) - assert response.status_code == 200 - - -def test_json_accept_returns_401() -> None: - conn = _make_conn("application/json") - exc = AuthenticationError("bad token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, JSONResponse) - assert response.status_code == 401 - - -def test_event_stream_accept_returns_401() -> None: - conn = _make_conn("text/event-stream") - exc = AuthenticationError("bad token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, JSONResponse) - assert response.status_code == 401 - - -def test_no_accept_header_returns_401() -> None: - conn = MagicMock() - conn.headers = {} - conn.scope = {"method": "GET"} - exc = AuthenticationError("bad token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, JSONResponse) - assert response.status_code == 401 - - -def test_json_401_body_is_rfc6750_compliant() -> None: - conn = _make_conn("application/json") - exc = AuthenticationError("expired") - response = _auth_error_handler(conn, exc) - assert response.status_code == 401 - assert response.headers.get("www-authenticate") == 'Bearer error="invalid_token"' - - -def test_html_accepted_alongside_other_types_but_not_json() -> None: - conn = _make_conn("text/html, */*") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, HTMLResponse) - assert response.status_code == 200 - - -def test_accept_both_html_and_json_returns_401() -> None: - # When a client lists both, treat it as an API client (application/json wins) - conn = _make_conn("text/html, application/json") - exc = AuthenticationError("bad token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, JSONResponse) - assert response.status_code == 401 - - -def test_post_with_html_accept_returns_401() -> None: - # Non-GET/HEAD methods always get JSON 401, even with text/html Accept - conn = _make_conn("text/html", method="POST") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, JSONResponse) - assert response.status_code == 401 - - -def test_head_with_html_accept_returns_html_200() -> None: - conn = _make_conn("text/html", method="HEAD") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, HTMLResponse) - assert response.status_code == 200 - - -def test_accept_header_case_insensitive() -> None: - conn = _make_conn("TEXT/HTML") - exc = AuthenticationError("no token") - response = _auth_error_handler(conn, exc) - assert isinstance(response, HTMLResponse) - assert response.status_code == 200
