diff --git a/tests/tools/test_mcp_oauth.py b/tests/tools/test_mcp_oauth.py index b2f3f022..db0342e9 100644 --- a/tests/tools/test_mcp_oauth.py +++ b/tests/tools/test_mcp_oauth.py @@ -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" diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index fd655bf3..51e243c6 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -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, diff --git a/tools/mcp_oauth_manager.py b/tools/mcp_oauth_manager.py index 7c8a91f3..dbe2fc3e 100644 --- a/tools/mcp_oauth_manager.py +++ b/tools/mcp_oauth_manager.py @@ -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,