You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/09 15:55:23 UTC
(superset) 02/03: fix(mcp): require an issuer-aware resolver for multi-issuer JWT configs
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix-mcp-identity-input-hardening
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 33a889d87af9c7a8cfab0c05e6cee5246a61b447
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Sun Aug 9 08:26:38 2026 -0700
fix(mcp): require an issuer-aware resolver for multi-issuer JWT configs
The default MCP user resolver maps JWT claims to Superset users by
username/email without binding the token's iss claim. When
MCP_JWT_ISSUER trusted more than one issuer, this meant distinct
issuers minting the same username/email would resolve to the same
Superset user, and the service only logged a warning about it.
Add validate_multi_issuer_user_resolver(), called both at MCP auth
provider startup and from _resolve_user_from_jwt_context, so a
multi-issuer configuration without a custom, issuer-aware
MCP_USER_RESOLVER now raises MCPAuthConfigError instead of proceeding.
This mirrors the existing fail-closed checks for MCP_JWT_AUDIENCE and
the guest-token secret. Document the requirement in SECURITY.md and
PRODUCTION.md.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
superset/mcp_service/PRODUCTION.md | 7 ++
superset/mcp_service/SECURITY.md | 7 ++
superset/mcp_service/auth.py | 27 ++---
superset/mcp_service/mcp_config.py | 37 +++++++
.../mcp_service/test_auth_user_resolution.py | 52 +++------
.../test_mcp_multi_issuer_identity_binding.py | 119 ++++++++++-----------
6 files changed, 136 insertions(+), 113 deletions(-)
diff --git a/superset/mcp_service/PRODUCTION.md b/superset/mcp_service/PRODUCTION.md
index ad402fee266..3bfffc6d9f7 100644
--- a/superset/mcp_service/PRODUCTION.md
+++ b/superset/mcp_service/PRODUCTION.md
@@ -82,6 +82,13 @@ MCP_REQUIRED_SCOPES = ["superset:read"]
MCP_DEV_USERNAME = None
```
+**Trusting Multiple Issuers**: `MCP_JWT_ISSUER` can be set to a list of
+issuers instead of a single string. Because the default user resolver maps
+token claims to Superset users by username/email without binding the
+token's `iss` claim, trusting more than one issuer requires configuring a
+custom, issuer-aware `MCP_USER_RESOLVER` (e.g. deriving a compound
+`iss`+`sub` identity). Without one, the service refuses to start.
+
**JWT Issuer Setup Examples**:
**Auth0**:
diff --git a/superset/mcp_service/SECURITY.md b/superset/mcp_service/SECURITY.md
index 26c9ed421a3..5c97eb3fbe8 100644
--- a/superset/mcp_service/SECURITY.md
+++ b/superset/mcp_service/SECURITY.md
@@ -74,6 +74,13 @@ MCP_JWT_ALGORITHM = "HS256"
MCP_JWT_SECRET = "your-shared-secret-key"
```
+**Multiple Trusted Issuers**: `MCP_JWT_ISSUER` may also be set to a list of
+issuers. The default user resolver maps token claims to Superset users by
+username/email without binding the token's `iss` claim, so trusting more
+than one issuer requires a custom, issuer-aware `MCP_USER_RESOLVER` (e.g.
+one that derives a compound `iss`+`sub` identity). The service refuses to
+start in this configuration until such a resolver is provided.
+
**JWT Token Structure**:
```json
diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py
index 92df7ce81c5..d4383ad489f 100644
--- a/superset/mcp_service/auth.py
+++ b/superset/mcp_service/auth.py
@@ -60,6 +60,7 @@ from superset.mcp_service.guest_token_verifier import GUEST_TOKEN_CLAIM
from superset.mcp_service.mcp_config import (
default_user_resolver,
get_mcp_api_key_enabled,
+ validate_multi_issuer_user_resolver,
)
from superset.mcp_service.utils.error_sanitization import (
sanitize_for_log as _sanitize_for_log,
@@ -526,6 +527,10 @@ def _resolve_user_from_jwt_context(app: Any) -> MCPUser | None: # noqa: C901
Raises:
ValueError: If JWT resolves a username that doesn't exist in the DB
(fail closed — do NOT fall through to weaker auth sources).
+ MCPAuthConfigError: If more than one JWT issuer is trusted
+ (``MCP_JWT_ISSUER`` is a list/tuple/set) and no issuer-aware
+ ``MCP_USER_RESOLVER`` is configured (fail closed — see
+ ``validate_multi_issuer_user_resolver``).
"""
try:
from fastmcp.server.dependencies import get_access_token
@@ -591,23 +596,11 @@ def _resolve_user_from_jwt_context(app: Any) -> MCPUser | None: # noqa: C901
# Single-issuer deployments (the common case) are safe — the issuer is
# already pinned by the verifier, so the username space is unambiguous and
# we keep the existing lookup key to avoid breaking them. For multi-issuer
- # configs we warn: operators should provide an issuer-aware MCP_USER_RESOLVER
- # that derives a compound (iss + sub) identity. This is the least-breaking
- # correct option (warn, don't change the key out from under existing
- # single-issuer deployments).
- configured_issuer = app.config.get("MCP_JWT_ISSUER")
- if isinstance(configured_issuer, (list, tuple, set)) and len(configured_issuer) > 1:
- if not app.config.get("MCP_USER_RESOLVER"):
- token_iss = claims.get("iss") if isinstance(claims, dict) else None
- logger.warning(
- "Multiple JWT issuers are trusted (MCP_JWT_ISSUER is a list) but "
- "the default user resolver maps token claims to Superset users by "
- "username/email without binding the issuer (iss=%s). Distinct "
- "issuers minting the same username/email will collide. Configure an "
- "issuer-aware MCP_USER_RESOLVER to derive a compound (iss+sub) "
- "identity.",
- _sanitize_for_log(token_iss),
- )
+ # configs without an issuer-aware MCP_USER_RESOLVER, fail closed rather
+ # than resolving an identity that isn't actually scoped to the trusted
+ # issuer (mirrors the startup-time config checks in
+ # create_default_mcp_auth_factory / superset.initialization).
+ validate_multi_issuer_user_resolver(app)
# Use configurable resolver or default
diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py
index 612448b98ca..1f8ff0249c9 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -510,6 +510,8 @@ def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]:
jwt_verifier: Any | None = None
if auth_enabled:
+ validate_multi_issuer_user_resolver(app)
+
jwks_uri = app.config.get("MCP_JWKS_URI")
public_key = app.config.get("MCP_JWT_PUBLIC_KEY")
secret = app.config.get("MCP_JWT_SECRET")
@@ -568,6 +570,41 @@ def _is_mcp_guest_auth_enabled(app: Flask) -> bool:
return True
+def validate_multi_issuer_user_resolver(app: Flask) -> None:
+ """Reject a multi-issuer JWT trust config that has no issuer-aware resolver.
+
+ ``default_user_resolver`` maps token claims to Superset users by
+ username/email without binding the token's ``iss`` claim. When more than
+ one issuer is trusted (``MCP_JWT_ISSUER`` configured as a list/tuple/set),
+ that lookup is not issuer-scoped: distinct issuers minting the same
+ username or email claim would resolve to the identical Superset user.
+ Single-issuer deployments are unaffected — the issuer is already pinned
+ by the verifier, so the username space is unambiguous.
+
+ Operators trusting more than one issuer must supply an issuer-aware
+ ``MCP_USER_RESOLVER`` (e.g. one that derives a compound iss+sub identity)
+ before the service will consider that configuration usable.
+ """
+ configured_issuer = app.config.get("MCP_JWT_ISSUER")
+ if (
+ isinstance(configured_issuer, (list, tuple, set))
+ and len(configured_issuer) > 1
+ and not app.config.get("MCP_USER_RESOLVER")
+ ):
+ # MCPAuthConfigError specifically: callers re-raise this type to
+ # refuse startup / fail closed rather than silently proceeding with
+ # an identity lookup that is not scoped to the trusted issuer.
+ raise MCPAuthConfigError(
+ "MCP_JWT_ISSUER trusts multiple issuers but no MCP_USER_RESOLVER "
+ "is configured. The default user resolver maps token claims to "
+ "Superset users by username/email without binding the issuer, so "
+ "distinct trusted issuers minting the same username/email would "
+ "resolve to the same Superset user. Configure an issuer-aware "
+ "MCP_USER_RESOLVER (e.g. deriving a compound iss+sub identity) "
+ "before trusting more than one issuer."
+ )
+
+
def _validate_guest_config(app: Flask) -> None:
"""Hard-fail on the default GUEST_TOKEN_JWT_SECRET; warn on an unset audience."""
if app.config.get("GUEST_TOKEN_JWT_SECRET") == CHANGE_ME_GUEST_TOKEN_JWT_SECRET:
diff --git a/tests/unit_tests/mcp_service/test_auth_user_resolution.py b/tests/unit_tests/mcp_service/test_auth_user_resolution.py
index 28bdc2ee38a..33dc1354841 100644
--- a/tests/unit_tests/mcp_service/test_auth_user_resolution.py
+++ b/tests/unit_tests/mcp_service/test_auth_user_resolution.py
@@ -608,40 +608,28 @@ def test_setup_user_context_allows_active_user(app) -> None:
# -- Multi-issuer binding guard --
-def test_multi_issuer_warns_without_custom_resolver(app, caplog) -> None:
+def test_multi_issuer_fails_closed_without_custom_resolver(app) -> None:
"""When multiple issuers are trusted and no issuer-aware resolver is set,
- a WARNING is emitted about unbound (non-issuer-scoped) user resolution."""
- import logging
+ resolution fails closed (raises) instead of returning a user via an
+ unbound (non-issuer-scoped) lookup."""
+ from superset.mcp_service.mcp_config import MCPAuthConfigError
- mock_user = _make_mock_user("alice")
token = _make_access_token(claims={"sub": "alice", "iss": "issuer-a"})
with app.app_context():
app.config["MCP_JWT_ISSUER"] = ["issuer-a", "issuer-b"]
try:
- with (
- caplog.at_level(logging.WARNING),
- patch(
- "fastmcp.server.dependencies.get_access_token", return_value=token
- ),
- patch(
- "superset.mcp_service.auth.load_user_with_relationships",
- return_value=mock_user,
- ),
+ with patch(
+ "fastmcp.server.dependencies.get_access_token", return_value=token
):
- result = _resolve_user_from_jwt_context(app)
+ with pytest.raises(MCPAuthConfigError):
+ _resolve_user_from_jwt_context(app)
finally:
app.config.pop("MCP_JWT_ISSUER", None)
- assert result is not None
- warnings = [r.message for r in caplog.records if r.levelno == logging.WARNING]
- assert any("Multiple JWT issuers are trusted" in m for m in warnings)
-
-
-def test_single_issuer_does_not_warn(app, caplog) -> None:
- """A single configured issuer is safe and emits no multi-issuer warning."""
- import logging
+def test_single_issuer_does_not_fail_closed(app) -> None:
+ """A single configured issuer is safe and resolves normally."""
mock_user = _make_mock_user("alice")
token = _make_access_token(claims={"sub": "alice", "iss": "issuer-a"})
@@ -649,7 +637,6 @@ def test_single_issuer_does_not_warn(app, caplog) -> None:
app.config["MCP_JWT_ISSUER"] = "issuer-a"
try:
with (
- caplog.at_level(logging.WARNING),
patch(
"fastmcp.server.dependencies.get_access_token", return_value=token
),
@@ -658,19 +645,16 @@ def test_single_issuer_does_not_warn(app, caplog) -> None:
return_value=mock_user,
),
):
- _resolve_user_from_jwt_context(app)
+ result = _resolve_user_from_jwt_context(app)
finally:
app.config.pop("MCP_JWT_ISSUER", None)
- warnings = [r.message for r in caplog.records if r.levelno == logging.WARNING]
- assert not any("Multiple JWT issuers are trusted" in m for m in warnings)
-
+ assert result is mock_user
-def test_multi_issuer_no_warn_with_custom_resolver(app, caplog) -> None:
- """A custom MCP_USER_RESOLVER (assumed issuer-aware) suppresses the
- multi-issuer warning."""
- import logging
+def test_multi_issuer_does_not_fail_closed_with_custom_resolver(app) -> None:
+ """A custom MCP_USER_RESOLVER (assumed issuer-aware) is exempt from the
+ multi-issuer fail-closed guard and resolves normally."""
mock_user = _make_mock_user("alice")
token = _make_access_token(claims={"sub": "alice", "iss": "issuer-a"})
@@ -679,7 +663,6 @@ def test_multi_issuer_no_warn_with_custom_resolver(app, caplog) -> None:
app.config["MCP_USER_RESOLVER"] = MagicMock(return_value="alice")
try:
with (
- caplog.at_level(logging.WARNING),
patch(
"fastmcp.server.dependencies.get_access_token", return_value=token
),
@@ -688,10 +671,9 @@ def test_multi_issuer_no_warn_with_custom_resolver(app, caplog) -> None:
return_value=mock_user,
),
):
- _resolve_user_from_jwt_context(app)
+ result = _resolve_user_from_jwt_context(app)
finally:
app.config.pop("MCP_JWT_ISSUER", None)
app.config.pop("MCP_USER_RESOLVER", None)
- warnings = [r.message for r in caplog.records if r.levelno == logging.WARNING]
- assert not any("Multiple JWT issuers are trusted" in m for m in warnings)
+ assert result is mock_user
diff --git a/tests/unit_tests/mcp_service/test_mcp_multi_issuer_identity_binding.py b/tests/unit_tests/mcp_service/test_mcp_multi_issuer_identity_binding.py
index 288b5cc470a..64007bbb1a5 100644
--- a/tests/unit_tests/mcp_service/test_mcp_multi_issuer_identity_binding.py
+++ b/tests/unit_tests/mcp_service/test_mcp_multi_issuer_identity_binding.py
@@ -16,27 +16,30 @@
# under the License.
"""Identity-binding behavior of ``_resolve_user_from_jwt_context`` when more
-than one JWT issuer is trusted (``MCP_JWT_ISSUER`` configured as a list) and
-no custom ``MCP_USER_RESOLVER`` is provided.
+than one JWT issuer is trusted (``MCP_JWT_ISSUER`` configured as a list).
The default resolver (``default_user_resolver``) derives a Superset username
from token claims (``preferred_username`` / ``username`` / ``email`` / ``sub``)
-without folding the token's ``iss`` claim into the lookup key. These tests
-pin down what that means in practice: two tokens with different ``iss``
-values but the same username claim resolve to the identical Superset user,
-with the DB lookup performed on username alone.
-
-The module emits a WARNING in this configuration (see
-``test_auth_user_resolution.test_multi_issuer_warns_without_custom_resolver``)
-but does not change the resolution behavior. These tests intentionally assert
-the current (pre-fix) behavior so that a later change binding identity to
-``iss`` (e.g. a compound iss+sub key, or failing closed when no issuer-aware
-resolver is configured) will need this test updated alongside it.
+without folding the token's ``iss`` claim into the lookup key. Without an
+issuer-aware ``MCP_USER_RESOLVER``, two tokens minted by different trusted
+issuers but sharing a username claim would otherwise resolve to the
+identical Superset user, since the DB lookup would be performed on username
+alone.
+
+To prevent that, ``_resolve_user_from_jwt_context`` fails closed (raises
+``MCPAuthConfigError``) when more than one issuer is trusted and no custom
+``MCP_USER_RESOLVER`` is configured, instead of proceeding with a lookup that
+isn't scoped to the trusted issuer. An operator-supplied, issuer-aware
+resolver (e.g. one deriving a compound iss+sub identity) is unaffected and
+continues to resolve normally.
"""
from unittest.mock import MagicMock, patch
+import pytest
+
from superset.mcp_service.auth import _resolve_user_from_jwt_context
+from superset.mcp_service.mcp_config import MCPAuthConfigError
def _make_mock_user(username: str = "alice") -> MagicMock:
@@ -57,14 +60,35 @@ def _make_access_token(claims: dict[str, str]) -> MagicMock:
return token
-def test_same_username_from_different_trusted_issuers_resolves_to_same_user(
- app,
-) -> None:
- """Two trusted issuers minting the same username claim are not
- distinguished by the default resolver: both resolve to one Superset
- user, and the DB lookup is performed by username only (no ``iss``
- component), when ``MCP_JWT_ISSUER`` is a multi-issuer list and no
- ``MCP_USER_RESOLVER`` override is configured."""
+def test_multi_issuer_without_custom_resolver_fails_closed(app) -> None:
+ """Multiple trusted issuers with no issuer-aware ``MCP_USER_RESOLVER``
+ refuses to resolve an identity rather than performing a username-only
+ lookup that isn't scoped to the trusted issuer."""
+ token = _make_access_token(
+ claims={"sub": "alice", "iss": "https://issuer-a.example.com"}
+ )
+
+ with app.app_context():
+ app.config["MCP_JWT_ISSUER"] = [
+ "https://issuer-a.example.com",
+ "https://issuer-b.example.com",
+ ]
+ app.config.pop("MCP_USER_RESOLVER", None)
+ try:
+ with patch(
+ "fastmcp.server.dependencies.get_access_token",
+ return_value=token,
+ ):
+ with pytest.raises(MCPAuthConfigError):
+ _resolve_user_from_jwt_context(app)
+ finally:
+ app.config.pop("MCP_JWT_ISSUER", None)
+
+
+def test_multi_issuer_with_custom_resolver_resolves_normally(app) -> None:
+ """An operator-supplied, issuer-aware ``MCP_USER_RESOLVER`` is unaffected
+ by the multi-issuer guard: legitimate multi-issuer deployments that
+ provide their own resolver continue to resolve users normally."""
shared_user = _make_mock_user("alice")
token_from_issuer_a = _make_access_token(
claims={"sub": "alice", "iss": "https://issuer-a.example.com"}
@@ -73,12 +97,18 @@ def test_same_username_from_different_trusted_issuers_resolves_to_same_user(
claims={"sub": "alice", "iss": "https://issuer-b.example.com"}
)
+ # A stand-in issuer-aware resolver: derives a compound iss+sub identity
+ # instead of the default username/email-only lookup.
+ def _issuer_aware_resolver(_app: object, access_token: MagicMock) -> str:
+ claims = access_token.claims
+ return f"{claims['iss']}:{claims['sub']}"
+
with app.app_context():
app.config["MCP_JWT_ISSUER"] = [
"https://issuer-a.example.com",
"https://issuer-b.example.com",
]
- app.config.pop("MCP_USER_RESOLVER", None)
+ app.config["MCP_USER_RESOLVER"] = _issuer_aware_resolver
try:
with (
patch(
@@ -105,46 +135,13 @@ def test_same_username_from_different_trusted_issuers_resolves_to_same_user(
result_b = _resolve_user_from_jwt_context(app)
finally:
app.config.pop("MCP_JWT_ISSUER", None)
+ app.config.pop("MCP_USER_RESOLVER", None)
- # Both issuers resolve to the exact same Superset identity.
assert result_a is shared_user
assert result_b is shared_user
- # The lookup key passed to the DB layer is username-only: neither call
- # includes the token's `iss` claim, confirming resolution is not
- # issuer-scoped for this (unsupported but reachable) configuration.
- mock_load_a.assert_called_once_with("alice")
- mock_load_b.assert_called_once_with("alice")
-
-
-def test_multi_issuer_without_custom_resolver_does_not_fail_closed(app) -> None:
- """The module logs a warning about the unbound multi-issuer configuration
- (see test_auth_user_resolution.py) but still returns a resolved user
- rather than refusing to authenticate the request."""
- mock_user = _make_mock_user("alice")
- token = _make_access_token(
- claims={"sub": "alice", "iss": "https://issuer-a.example.com"}
- )
-
- with app.app_context():
- app.config["MCP_JWT_ISSUER"] = [
- "https://issuer-a.example.com",
- "https://issuer-b.example.com",
- ]
- app.config.pop("MCP_USER_RESOLVER", None)
- try:
- with (
- patch(
- "fastmcp.server.dependencies.get_access_token",
- return_value=token,
- ),
- patch(
- "superset.mcp_service.auth.load_user_with_relationships",
- return_value=mock_user,
- ),
- ):
- result = _resolve_user_from_jwt_context(app)
- finally:
- app.config.pop("MCP_JWT_ISSUER", None)
-
- assert result is mock_user
+ # The custom resolver's compound iss+sub key is what reaches the DB
+ # lookup, not a bare username — confirming the guard doesn't interfere
+ # with a resolver that already binds identity to the issuer.
+ mock_load_a.assert_called_once_with("https://issuer-a.example.com:alice")
+ mock_load_b.assert_called_once_with("https://issuer-b.example.com:alice")