mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
fix(mcp-oauth): preserve server_url path for protected-resource validation (#16031)
Stop pre-stripping the path from the configured MCP server URL before constructing OAuthClientProvider. The MCP SDK strips the path itself via OAuthContext.get_authorization_base_url() for authorization-server discovery, but uses the full server_url through resource_url_from_server_url() + check_resource_allowed() to validate against the server's RFC 9728 Protected Resource Metadata. For servers whose PRM advertises a path-scoped resource (e.g. Notion's https://mcp.notion.com/mcp), our _parse_base_url() collapsed the URL to the origin, so check_resource_allowed() saw requested='/' vs configured='/mcp/' and refused the token. Fixes OAuth against Notion MCP (and any other path-scoped resource). Closes #16015.
This commit is contained in:
@@ -491,11 +491,36 @@ def test_configure_callback_port_uses_explicit_port():
|
||||
assert cfg["_resolved_port"] == 54321
|
||||
|
||||
|
||||
def test_parse_base_url_strips_path():
|
||||
"""_parse_base_url drops path components for OAuth discovery."""
|
||||
from tools.mcp_oauth import _parse_base_url
|
||||
def test_build_oauth_auth_preserves_server_url_path():
|
||||
"""server_url with path is forwarded to OAuthClientProvider unmodified.
|
||||
|
||||
Regression for #16015: previously ``_parse_base_url`` stripped the path,
|
||||
collapsing ``https://mcp.notion.com/mcp`` to ``https://mcp.notion.com`` and
|
||||
breaking RFC 9728 protected-resource validation against servers whose PRM
|
||||
advertises a path-scoped resource (Notion). The MCP SDK strips the path
|
||||
itself for authorization-server discovery via
|
||||
``OAuthContext.get_authorization_base_url``; Hermes must not pre-strip.
|
||||
"""
|
||||
from tools import mcp_oauth
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
class _FakeProvider:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
with patch.object(mcp_oauth, "_OAUTH_AVAILABLE", True), \
|
||||
patch.object(mcp_oauth, "OAuthClientProvider", _FakeProvider), \
|
||||
patch.object(mcp_oauth, "_is_interactive", return_value=True), \
|
||||
patch.object(mcp_oauth, "_maybe_preregister_client"), \
|
||||
patch.object(mcp_oauth, "HermesTokenStorage") as mock_storage_cls:
|
||||
mock_storage_cls.return_value = MagicMock(has_cached_tokens=lambda: True)
|
||||
build_oauth_auth(
|
||||
server_name="notion",
|
||||
server_url="https://mcp.notion.com/mcp",
|
||||
oauth_config={},
|
||||
)
|
||||
|
||||
assert captured["server_url"] == "https://mcp.notion.com/mcp"
|
||||
|
||||
assert _parse_base_url("https://example.com/mcp/v1") == "https://example.com"
|
||||
assert _parse_base_url("https://example.com") == "https://example.com"
|
||||
assert _parse_base_url("https://host.example.com:8080/api") == "https://host.example.com:8080"
|
||||
|
||||
|
||||
@@ -519,12 +519,6 @@ def _maybe_preregister_client(
|
||||
logger.debug("Pre-registered client_id=%s for '%s'", client_id, storage._server_name)
|
||||
|
||||
|
||||
def _parse_base_url(server_url: str) -> str:
|
||||
"""Strip path component from server URL, returning the base origin."""
|
||||
parsed = urlparse(server_url)
|
||||
return f"{parsed.scheme}://{parsed.netloc}"
|
||||
|
||||
|
||||
def build_oauth_auth(
|
||||
server_name: str,
|
||||
server_url: str,
|
||||
@@ -570,7 +564,7 @@ def build_oauth_auth(
|
||||
_maybe_preregister_client(storage, cfg, client_metadata)
|
||||
|
||||
return OAuthClientProvider(
|
||||
server_url=_parse_base_url(server_url),
|
||||
server_url=server_url,
|
||||
client_metadata=client_metadata,
|
||||
storage=storage,
|
||||
redirect_handler=_redirect_handler,
|
||||
|
||||
@@ -362,7 +362,6 @@ class MCPOAuthManager:
|
||||
_configure_callback_port,
|
||||
_is_interactive,
|
||||
_maybe_preregister_client,
|
||||
_parse_base_url,
|
||||
_redirect_handler,
|
||||
_wait_for_callback,
|
||||
)
|
||||
@@ -387,7 +386,7 @@ class MCPOAuthManager:
|
||||
|
||||
return _HERMES_PROVIDER_CLS(
|
||||
server_name=server_name,
|
||||
server_url=_parse_base_url(entry.server_url),
|
||||
server_url=entry.server_url,
|
||||
client_metadata=client_metadata,
|
||||
storage=storage,
|
||||
redirect_handler=_redirect_handler,
|
||||
|
||||
Reference in New Issue
Block a user