aminghadersohi commented on code in PR #45639: URL: https://github.com/apache/superset/pull/45639#discussion_r4364786139
########## docs/admin_docs/configuration/mcp-server.mdx: ########## @@ -661,6 +661,7 @@ curl -X POST http://localhost:5008/mcp \ - A dedicated guest-token verifier validates the token against the same `GUEST_TOKEN_JWT_SECRET` / `GUEST_TOKEN_JWT_ALGO` / `GUEST_TOKEN_JWT_AUDIENCE` config used by embedded dashboards, replays the embedded structural checks, and enforces revocation (global version bumps and per-dashboard `guest_token_revoked_before` cutoffs). It runs *before* the JWT verifier described above, since guest tokens are signed with a different key/algorithm and would otherwise be rejected at the transport. - A verified guest resolves to a Superset guest user as the highest-priority identity, so it's never downgraded to API-key / `MCP_DEV_USERNAME` / dev-mode resolution. Data access is scoped by the same checks (dataset allowlist, dashboard access, row-level security) that apply to embedded dashboard views. +- Guest and API-key identity resolution supports FastMCP 3 and FastMCP 4, including verifier chains wrapped in `MultiAuth`. Guest resolution revalidates the bearer and core revocation checks; API-key resolution trusts only FAB validation, not identity-routing claims supplied by an external JWT. Review Comment: Fixed in 272e4a61c091eddec8e47caa73fe820e1ffec0a2. `UPDATING.md` under `## Next` covers both migration points: custom resolvers should import `get_verifier_client_id` from `superset.mcp_service.composite_token_verifier`, and explicit API-key pass-through credentials are rejected with `PermissionError` when keys are disabled, with an empty tools/list response. The MCP_USER_RESOLVER table documents qualified client IDs and the helper as well. For this documentation-only ask, I checked four assertions against the previous head and this head: all four failed before and pass after (upgrade helper guidance, explicit rejection, empty discovery, and hook-table guidance). The documented runtime contracts are covered by `test_default_user_resolver_uses_unqualified_client_id`, `test_api_key_disabled_rejects_explicit_auth`, and `test_disabled_api_key_assertion_hides_tools`: the resolver/discovery cases fail against the pre-PR merge-base source, and pass with this PR. The two-file discovery regression also fails on the previous review head and passes after the import change. Siblings checked: - UPDATING.md:26 — fixed in 272e4a61c091eddec8e47caa73fe820e1ffec0a2 - docs/admin_docs/configuration/mcp-server.mdx:984 — fixed in 272e4a61c091eddec8e47caa73fe820e1ffec0a2 - superset/mcp_service/mcp_config.py:961 — not affected: the default resolver already uses the helper; the migration concerns custom resolvers - superset/mcp_service/auth.py:694 — not affected: guest and API-key routing already use the helper, with separate bearer validation - superset/mcp_service/auth.py:873 — not affected: the API-key caller already uses the helper and FAB validation evidence - superset/mcp_service/jwt_verifier.py:613 — not affected: creates the verifier's client ID before MultiAuth qualification; does not consume a wrapped ID - superset/mcp_service/query/schemas.py:114 — not affected: query client_id is a query correlation field, not an authentication identity - docs/admin_docs_versioned_docs/version-6.1.0/configuration/mcp-server.mdx:522 — not affected: historical release documentation, not the Next migration contract - superset/mcp_service/README.md:703 — not affected: names the resolver hook but does not instruct reading access_token.client_id - superset/mcp_service/PRODUCTION.md:89 — not affected: requires issuer-aware identity binding, not a client_id contract - superset/mcp_service/SECURITY.md:80 — not affected: issuer-binding guidance does not read client_id -- 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]
