fix(mcp): Figma OAuth login completes despite the omitted iss parameter
Figma's authorization-server metadata advertises authorization_response_iss_parameter_supported and its redirect omits iss, so the mcp SDK's RFC 9207 check discarded every valid code and login never finished. For that one issuer the provider fills a missing iss with the discovered issuer and warns; a mismatching iss still fails and every other server keeps the strict rule. Fixes #111135
This commit is contained in:
56
tests/tools/test_mcp_oauth_missing_iss.py
Normal file
56
tests/tools/test_mcp_oauth_missing_iss.py
Normal file
@@ -0,0 +1,56 @@
|
||||
"""Regression for #111135: Figma advertises RFC 9207 ``iss`` support and then omits ``iss`` from the
|
||||
redirect, so the SDK rejected every valid authorization code. Only that issuer is tolerated."""
|
||||
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
pytest.importorskip("mcp.shared.auth", reason="mcp 2.x SDK not installed")
|
||||
|
||||
from mcp.client.auth.oauth2 import OAuthClientProvider # noqa: E402
|
||||
from mcp.client.auth.utils import validate_authorization_response_iss # noqa: E402
|
||||
from mcp.shared.auth import AuthorizationCodeResult, OAuthMetadata # noqa: E402
|
||||
|
||||
from tools.mcp_oauth_provider import HermesProviderMixin # noqa: E402
|
||||
|
||||
|
||||
class _Provider(HermesProviderMixin, OAuthClientProvider):
|
||||
pass
|
||||
|
||||
|
||||
def _provider_for(issuer: str, *, iss_in_redirect: str | None) -> _Provider:
|
||||
provider = _Provider.__new__(_Provider)
|
||||
meta = OAuthMetadata(
|
||||
issuer=issuer,
|
||||
authorization_endpoint=f"{issuer}/oauth",
|
||||
token_endpoint=f"{issuer}/token",
|
||||
authorization_response_iss_parameter_supported=True,
|
||||
)
|
||||
|
||||
async def callback():
|
||||
return AuthorizationCodeResult(code="c0de", state="st", iss=iss_in_redirect)
|
||||
|
||||
provider.context = SimpleNamespace(oauth_metadata=meta, callback_handler=callback,
|
||||
client_info=SimpleNamespace(grant_types=["authorization_code"]))
|
||||
provider._hermes_oauth_flow = "browser"
|
||||
return provider
|
||||
|
||||
|
||||
def _redirect_passes_sdk_check(provider: _Provider) -> bool:
|
||||
provider._tolerate_missing_iss_for_known_server()
|
||||
result = asyncio.run(provider.context.callback_handler())
|
||||
try:
|
||||
validate_authorization_response_iss(result.iss, provider.context.oauth_metadata)
|
||||
except Exception:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("issuer, iss, expected", [
|
||||
("https://api.figma.com", None, True), # the advertised-but-omitted case Figma ships
|
||||
("https://api.figma.com", "https://evil.example", False), # a wrong iss is still rejected
|
||||
("https://auth.example.com", None, False), # every other server keeps the strict RFC 9207 rule
|
||||
])
|
||||
def test_missing_iss_is_tolerated_for_figma_only(issuer, iss, expected):
|
||||
assert _redirect_passes_sdk_check(_provider_for(issuer, iss_in_redirect=iss)) is expected
|
||||
@@ -15,6 +15,10 @@ if TYPE_CHECKING:
|
||||
from tools.mcp_oauth import HermesTokenStorage
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Authorization servers that advertise ``authorization_response_iss_parameter_supported`` and then
|
||||
# omit ``iss`` from the redirect (#111135). Exact issuer match, nothing else is relaxed.
|
||||
_ISS_OMITTING_ISSUERS = frozenset({"https://api.figma.com"})
|
||||
|
||||
|
||||
|
||||
class _RefreshCompletedByPeer(Exception):
|
||||
@@ -49,8 +53,29 @@ class HermesProviderMixin:
|
||||
raise OAuthNonInteractiveError(
|
||||
"MCP device authorization requires `hermes mcp login <server> --flow device`; "
|
||||
"background reconnects cannot start a device login")
|
||||
self._tolerate_missing_iss_for_known_server()
|
||||
return await super()._perform_authorization()
|
||||
|
||||
def _tolerate_missing_iss_for_known_server(self) -> None:
|
||||
"""Figma advertises ``authorization_response_iss_parameter_supported`` and then omits ``iss``
|
||||
from the redirect, so the SDK's RFC 9207 check rejects every valid code (#111135). For that
|
||||
one issuer only, fill a missing ``iss`` with the discovered issuer and warn; a present-but-
|
||||
different ``iss`` still fails the SDK check, and every other server keeps the strict rule."""
|
||||
issuer = _metadata_issuer(self.context)
|
||||
if issuer not in _ISS_OMITTING_ISSUERS:
|
||||
return
|
||||
inner = self.context.callback_handler
|
||||
|
||||
async def _fill_iss():
|
||||
result = await inner()
|
||||
if getattr(result, "iss", None) is None and getattr(result, "code", None):
|
||||
self._hermes_logger.warning(
|
||||
"MCP OAuth: %s omitted the iss parameter it advertises; accepting the redirect for that issuer only", issuer)
|
||||
result = result.model_copy(update={"iss": str(self.context.oauth_metadata.issuer)})
|
||||
return result
|
||||
|
||||
self.context.callback_handler = _fill_iss
|
||||
|
||||
def _prepare_token_request(self, request):
|
||||
"""Stamp the configured User-Agent onto a token/refresh request."""
|
||||
ua = getattr(self, "_hermes_token_user_agent", None) # tests build via __new__
|
||||
|
||||
@@ -278,6 +278,8 @@ On first connect, Hermes prints an authorize URL, opens your browser when possib
|
||||
|
||||
Refresh tokens are bound to the authorization server that granted them: Hermes records the discovered issuer alongside the cached tokens and, if a server's advertised authorization server ever changes (server migration, metadata edit, or hijack), the stored refresh token is dropped instead of being sent to the new issuer. The current access token keeps working until it expires, then a normal re-authorization runs against the new issuer.
|
||||
|
||||
The redirect back from the authorization server is checked against RFC 9207: when the server's metadata advertises `authorization_response_iss_parameter_supported`, a redirect without a matching `iss` is rejected. Figma's authorization server (`https://api.figma.com`) advertises that support and then omits `iss`; Hermes fills the missing value from the discovered issuer for that one issuer and logs a warning, so `hermes mcp login figma` completes. A present-but-different `iss` is still rejected, and no other server gets the exemption.
|
||||
|
||||
**Remote / headless hosts.** When Hermes runs on a different machine than your browser, the loopback callback can't reach your laptop. Ways to complete the flow:
|
||||
|
||||
- **Hermes Desktop (automatic):** when you run the OAuth sign-in from the Desktop app's MCP setup UI against a remote backend, Desktop hosts the callback listener on *your* machine and relays the authorization back to the gateway automatically — no tunnel, paste, or proxy needed. Requires both the Desktop app and the backend to be up to date.
|
||||
|
||||
Reference in New Issue
Block a user