aminghadersohi commented on code in PR #39604:
URL: https://github.com/apache/superset/pull/39604#discussion_r3350593182
##########
superset/mcp_service/mcp_config.py:
##########
@@ -303,57 +314,142 @@
}
+def get_mcp_api_key_enabled(app: Flask, *, startup_warning: bool = False) ->
bool:
+ """Return whether API key auth is enabled for the MCP transport.
+
+ Prefers ``MCP_API_KEY_ENABLED`` when explicitly set; falls back to
+ ``FAB_API_KEY_ENABLED``. When ``startup_warning=True`` and the value
+ is inherited from ``FAB_API_KEY_ENABLED``, logs a warning so operators
+ know a FAB config change now also affects the MCP transport.
+ """
+ if (mcp_setting := app.config.get("MCP_API_KEY_ENABLED", None)) is not
None:
+ return bool(mcp_setting)
+ fab_enabled = bool(app.config.get("FAB_API_KEY_ENABLED", False))
+ if startup_warning and fab_enabled:
+ logger.warning(
+ "MCP API key auth is enabled via FAB_API_KEY_ENABLED=True. "
+ "Set MCP_API_KEY_ENABLED=True to silence this warning or "
+ "MCP_API_KEY_ENABLED=False to disable API keys on the MCP "
+ "transport without affecting the FAB REST API."
+ )
+ return fab_enabled
Review Comment:
Added in commit 8cef3fb2c2: four dedicated unit tests for
`get_mcp_api_key_enabled` in `test_mcp_config.py` covering explicit
`True`/`False` override, FAB fallback, and both-absent → `False`.
##########
superset/mcp_service/mcp_config.py:
##########
@@ -303,57 +314,142 @@
}
+def get_mcp_api_key_enabled(app: Flask, *, startup_warning: bool = False) ->
bool:
+ """Return whether API key auth is enabled for the MCP transport.
+
+ Prefers ``MCP_API_KEY_ENABLED`` when explicitly set; falls back to
+ ``FAB_API_KEY_ENABLED``. When ``startup_warning=True`` and the value
+ is inherited from ``FAB_API_KEY_ENABLED``, logs a warning so operators
+ know a FAB config change now also affects the MCP transport.
+ """
+ if (mcp_setting := app.config.get("MCP_API_KEY_ENABLED", None)) is not
None:
+ return bool(mcp_setting)
+ fab_enabled = bool(app.config.get("FAB_API_KEY_ENABLED", False))
+ if startup_warning and fab_enabled:
+ logger.warning(
+ "MCP API key auth is enabled via FAB_API_KEY_ENABLED=True. "
+ "Set MCP_API_KEY_ENABLED=True to silence this warning or "
+ "MCP_API_KEY_ENABLED=False to disable API keys on the MCP "
+ "transport without affecting the FAB REST API."
+ )
+ return fab_enabled
+
+
def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]:
- """Default MCP auth factory using app.config values."""
- if not app.config.get("MCP_AUTH_ENABLED", False):
- return None
+ """Default MCP auth factory using app.config values.
- jwks_uri = app.config.get("MCP_JWKS_URI")
- public_key = app.config.get("MCP_JWT_PUBLIC_KEY")
- secret = app.config.get("MCP_JWT_SECRET")
+ Returns an auth provider when ``MCP_AUTH_ENABLED=True`` (JWT verifier,
+ optionally wrapped with ``CompositeTokenVerifier`` for API keys) or
+ when only ``MCP_API_KEY_ENABLED=True`` (or ``FAB_API_KEY_ENABLED=True``
+ as a fallback) — API-key-only verifier that rejects all non-API-key
+ Bearer tokens at the transport.
- if not (jwks_uri or public_key or secret):
- logger.warning("MCP_AUTH_ENABLED is True but no JWT keys/secret
configured")
+ ``MCP_API_KEY_ENABLED=None`` (default) defers to ``FAB_API_KEY_ENABLED``
+ and logs a startup warning when that setting is True, so operators are
+ aware that a FAB config change now also affects the MCP transport.
+ """
+ auth_enabled = app.config.get("MCP_AUTH_ENABLED", False)
+ api_key_enabled = get_mcp_api_key_enabled(app, startup_warning=True)
+
+ if not (auth_enabled or api_key_enabled):
return None
- try:
- debug_errors = app.config.get("MCP_JWT_DEBUG_ERRORS", False)
+ jwt_verifier: Any | None = None
- common_kwargs: dict[str, Any] = {
- "issuer": app.config.get("MCP_JWT_ISSUER"),
- "audience": app.config.get("MCP_JWT_AUDIENCE"),
- "required_scopes": app.config.get("MCP_REQUIRED_SCOPES", []),
- }
+ if auth_enabled:
+ jwks_uri = app.config.get("MCP_JWKS_URI")
+ public_key = app.config.get("MCP_JWT_PUBLIC_KEY")
+ secret = app.config.get("MCP_JWT_SECRET")
- # For HS256 (symmetric), use the secret as the public_key parameter
- if app.config.get("MCP_JWT_ALGORITHM") == "HS256" and secret:
- common_kwargs["public_key"] = secret
- common_kwargs["algorithm"] = "HS256"
+ if not (jwks_uri or public_key or secret):
+ logger.warning("MCP_AUTH_ENABLED is True but no JWT keys/secret
configured")
+ if not api_key_enabled:
+ return None
else:
- # For RS256 (asymmetric), use public key or JWKS
- common_kwargs["jwks_uri"] = jwks_uri
- common_kwargs["public_key"] = public_key
- common_kwargs["algorithm"] = app.config.get("MCP_JWT_ALGORITHM",
"RS256")
-
- if debug_errors:
- # DetailedJWTVerifier: detailed server-side logging of JWT
- # validation failures. HTTP responses are always generic per
- # RFC 6750 Section 3.1.
- from superset.mcp_service.jwt_verifier import DetailedJWTVerifier
-
- auth_provider = DetailedJWTVerifier(**common_kwargs)
- else:
- # MCPJWTVerifier: minimal logging + browser-friendly error page.
- from superset.mcp_service.jwt_verifier import MCPJWTVerifier
-
- auth_provider = MCPJWTVerifier(**common_kwargs)
+ try:
+ jwt_verifier = _build_jwt_verifier(
+ app=app,
+ jwks_uri=jwks_uri,
+ public_key=public_key,
+ secret=secret,
+ )
+ except (ValueError, JoseError):
+ # 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
+
+ if api_key_enabled:
+ return _build_composite_verifier(app, jwt_verifier)
+
+ return jwt_verifier
Review Comment:
Added in commit 8cef3fb2c2: two direct unit tests for
`create_default_mcp_auth_factory` in `test_mcp_config.py`: both-disabled →
`None`, and API-key-only → `CompositeTokenVerifier`.
--
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]
