ci: trigger CI for fix

fix(tests): update test_jwt_verifier to import _auth_error_handler
The PR renamed _json_auth_error_handler to _auth_error_handler in jwt_verifier.py (to reflect that it now returns HTML for browsers rather than always JSON), but test_jwt_verifier.py still imported the old name, causing a collection-time ImportError that failed all unit tests.
2026-06-11 10:39:15 +00:00 · 2026-05-21 02:12:20 +00:00 · 2026-05-21 01:15:53 +00:00 · 2026-05-20 23:40:00 +00:00 · 2026-05-20 23:12:20 +00:00
4 changed files with 283 additions and 17 deletions
--- a/superset/mcp_service/jwt_verifier.py
+++ b/superset/mcp_service/jwt_verifier.py
@@ -45,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 JSONResponse
+from starlette.responses import HTMLResponse, JSONResponse, Response

 from superset.utils import json

@@ -61,21 +61,139 @@ _jwt_failure_reason: ContextVar[str | None] = ContextVar(
    "_jwt_failure_reason", default=None
 )

+_MCP_BROWSER_HELLO_HTML = """<!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": "&lt;this-url&gt;",
+      "transport": "streamable-http",
+      "headers": {
+        "Authorization": "Bearer &lt;your-api-key&gt;"
+      }
+    }
+  }
+}</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>&lt;this-url&gt;</code> with the full URL of this page and
+      <code>&lt;your-api-key&gt;</code> with a valid Superset API key or JWT token.
+    </div>
+  </div>
+</body>
+</html>"""

-def _json_auth_error_handler(
-    conn: HTTPConnection, exc: AuthenticationError
-) -> JSONResponse:
-    """JSON 401 error handler for authentication failures.

-    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.
+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.

    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=_MCP_BROWSER_HELLO_HTML)
+
    # Log detailed reason server-side only
    logger.warning("JWT authentication failed: %s", exc)

@@ -91,6 +209,28 @@ def _json_auth_error_handler(
    )


+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
@@ -124,7 +264,7 @@ class DetailedBearerAuthBackend(BearerAuthBackend):
        return None


-class DetailedJWTVerifier(JWTVerifier):
+class DetailedJWTVerifier(MCPJWTVerifier):
    """
    JWT verifier with tiered server-side logging for each validation step.

@@ -300,7 +440,7 @@ class DetailedJWTVerifier(JWTVerifier):
            Middleware(
                AuthenticationMiddleware,
                backend=DetailedBearerAuthBackend(self),
-                on_error=_json_auth_error_handler,
+                on_error=_auth_error_handler,
            ),
            Middleware(AuthContextMiddleware),
        ]
--- 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:
-            # Default JWTVerifier: minimal logging, generic error responses.
-            from fastmcp.server.auth.providers.jwt import JWTVerifier
+            # MCPJWTVerifier: minimal logging + browser-friendly error page.
+            from superset.mcp_service.jwt_verifier import MCPJWTVerifier

-            auth_provider = JWTVerifier(**common_kwargs)
+            auth_provider = MCPJWTVerifier(**common_kwargs)

        return auth_provider
    except Exception:
--- 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 (
-    _json_auth_error_handler,
+    _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 _json_auth_error_handler
+    assert auth_middleware.kwargs["on_error"] is _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 = _json_auth_error_handler(mock_conn, exc)
+        response = _auth_error_handler(mock_conn, exc)

        assert response.status_code == 401

--- a/tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py
+++ b/tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py
@@ -0,0 +1,126 @@
+# 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
Author	SHA1	Message	Date
Amin Ghadersohi	5c393dbf6f	ci: trigger CI for fix	2026-05-21 02:12:20 +00:00
Amin Ghadersohi	cb212db2bc	fix(tests): update test_jwt_verifier to import _auth_error_handler The PR renamed _json_auth_error_handler to _auth_error_handler in jwt_verifier.py (to reflect that it now returns HTML for browsers rather than always JSON), but test_jwt_verifier.py still imported the old name, causing a collection-time ImportError that failed all unit tests.	2026-05-21 01:15:53 +00:00
Amin Ghadersohi	a072261aa4	fix(mcp): wire browser hello page to all auth paths, restrict to GET/HEAD - Introduce MCPJWTVerifier(JWTVerifier) base class that registers _auth_error_handler as the Starlette on_error callback; previously the callback was only wired inside DetailedJWTVerifier (MCP_JWT_DEBUG_ERRORS=True), so the HTML page was never shown in the default configuration - mcp_config.py non-debug path now uses MCPJWTVerifier instead of bare JWTVerifier; DetailedJWTVerifier inherits MCPJWTVerifier - Add _prefers_browser_html() helper: checks method (GET/HEAD only) and Accept header (case-insensitive); prevents POST/OPTIONS with text/html from incorrectly receiving a 200 HTML response - Rename _json_auth_error_handler -> _auth_error_handler, return type narrowed to Response (Starlette base class, matching on_error signature) - Add tests: POST+text/html -> 401, HEAD+text/html -> 200, uppercase Accept Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-20 23:40:00 +00:00
Amin Ghadersohi	e9c25ff64e	feat(mcp): return browser-friendly hello page for GET /mcp from browsers When a browser opens the MCP endpoint (Accept: text/html without application/json or text/event-stream), return a 200 HTML page explaining what the endpoint is and how to configure it in Claude Desktop, Claude Code, or Cursor. API and SSE clients continue to receive the existing JSON 401 response unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-20 23:12:20 +00:00