AurimasNav opened a new pull request, #43329: URL: https://github.com/apache/superset/pull/43329
### SUMMARY `_create_auth_provider` swallows any exception raised by a configured `MCP_AUTH_FACTORY` and returns `None`, and the start path treats a `None` provider as "auth disabled" — so the MCP service comes up serving unauthenticated. #41292 hardened the **default-factory** branch against exactly this by introducing `MCPAuthConfigError` and re-raising it; this PR brings the **custom-factory** branch in line. Change: the custom-factory branch now re-raises `MCPAuthConfigError` as-is (the factory's own operator-facing guidance, no secret material by contract), and wraps any other exception in `MCPAuthConfigError` so startup aborts instead of falling through. Two deliberate choices, called out for review: - **Every custom-factory failure is treated as a config error.** The default branch distinguishes config errors (raise) from generic build errors (swallow, per the `MCPAuthConfigError` docstring). That distinction is not available here: an operator-supplied factory *is* configuration, and Superset has no basis to classify any of its failures as benign. In practice a factory fails for mundane reasons — a missing environment variable (`KeyError`), a dependency moving a symbol (`ImportError`), a verifier's signature changing (`TypeError`) — none of which the operator intended as "run without auth". - **The original exception is suppressed (`from None`), not chained.** This preserves the existing "may contain secrets" concern that motivated not logging it; the wrapped message carries only the exception's type name, which is enough to locate the failure. A factory returning `None` without raising is left as-is — that is explicit operator code, unlike a crash. Fixes #43327 ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (server startup behaviour) ### TESTING INSTRUCTIONS 1. Configure an `MCP_AUTH_FACTORY` in `superset_config.py` that raises — e.g. one that reads an unset environment variable. 2. Start the MCP service (`superset mcp run`). 3. Before: the service starts and unauthenticated requests to `/mcp` succeed, with only a single `logger.error` line. After: startup aborts with `MCPAuthConfigError` naming the exception type. Unit tests added in `tests/unit_tests/mcp_service/test_mcp_server.py`, modeled on the #41292 tests: - a raising factory aborts startup, the original message is absent from the raised error, the type name is present, and the exception context is suppressed - a factory raising `MCPAuthConfigError` propagates its own message unwrapped - the happy path is unchanged ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes #43327 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API --- *Developed with AI assistance; the change and tests were reviewed and verified by a human before submission.* -- 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]
