diff --git a/orchestrator/app/routers/mcp.py b/orchestrator/app/routers/mcp.py index bb28cd57..18a61e7b 100644 --- a/orchestrator/app/routers/mcp.py +++ b/orchestrator/app/routers/mcp.py @@ -35,6 +35,7 @@ from ..services.channels.registry import decrypt_credentials, encrypt_credentials from ..services.marketplace_federation import install_guard, mcp_install_prompt from ..services.mcp.client import connect_mcp +from ..services.mcp.oauth_redirect import build_mcp_oauth_callback_url from ..users import current_active_user logger = logging.getLogger(__name__) @@ -1168,11 +1169,7 @@ async def reconnect_mcp_config( if not server_url: raise HTTPException(status_code=400, detail="No server URL resolved for reconnect") - redirect_uri = ( - f"{(get_settings().public_base_url.rstrip('/'))}/api/mcp/oauth/callback" - if get_settings().public_base_url - else f"{request.url.scheme}://{request.url.netloc}/api/mcp/oauth/callback" - ) + redirect_uri = build_mcp_oauth_callback_url(request, get_settings()) try: result = await start_oauth_flow( diff --git a/orchestrator/app/routers/mcp_oauth.py b/orchestrator/app/routers/mcp_oauth.py index ce992f8b..a11cdb74 100644 --- a/orchestrator/app/routers/mcp_oauth.py +++ b/orchestrator/app/routers/mcp_oauth.py @@ -33,6 +33,7 @@ resolve_catalog_server, start_oauth_flow, ) +from ..services.mcp.oauth_redirect import build_mcp_oauth_callback_url from ..users import current_active_user logger = logging.getLogger(__name__) @@ -229,11 +230,7 @@ async def oauth_status( def _callback_url(request: Request) -> str: """Build the absolute URL of this router's /callback endpoint.""" - settings = get_settings() - base = ( - getattr(settings, "public_base_url", "") or f"{request.url.scheme}://{request.url.netloc}" - ) - return f"{base.rstrip('/')}/api/mcp/oauth/callback" + return build_mcp_oauth_callback_url(request, get_settings()) def _callback_html( diff --git a/orchestrator/app/services/mcp/oauth_redirect.py b/orchestrator/app/services/mcp/oauth_redirect.py new file mode 100644 index 00000000..f6792cd6 --- /dev/null +++ b/orchestrator/app/services/mcp/oauth_redirect.py @@ -0,0 +1,17 @@ +"""Helpers for MCP OAuth redirect URLs.""" + +from __future__ import annotations + +from typing import Any + +from fastapi import Request + + +def build_mcp_oauth_callback_url(request: Request, settings: Any) -> str: + """Build the callback URL providers should redirect back to.""" + request_origin = f"{request.url.scheme}://{request.url.netloc}" + if getattr(settings, "is_desktop_mode", False): + base = request_origin + else: + base = getattr(settings, "public_base_url", "") or request_origin + return f"{base.rstrip('/')}/api/mcp/oauth/callback" diff --git a/orchestrator/tests/services/mcp/test_routers_smoke.py b/orchestrator/tests/services/mcp/test_routers_smoke.py index 15851259..734f3e68 100644 --- a/orchestrator/tests/services/mcp/test_routers_smoke.py +++ b/orchestrator/tests/services/mcp/test_routers_smoke.py @@ -6,6 +6,9 @@ from __future__ import annotations +from types import SimpleNamespace +from uuid import uuid4 + import pytest from pydantic import ValidationError @@ -117,6 +120,67 @@ def test_start_oauth_request_allows_platform_app_with_slug(): assert body.marketplace_agent_slug == "mcp-github-oauth" +def test_mcp_oauth_callback_uses_loopback_origin_in_desktop(monkeypatch): + """Desktop MCP OAuth redirects must target the sidecar callback server.""" + from app.routers import mcp_oauth + + request = SimpleNamespace(url=SimpleNamespace(scheme="http", netloc="127.0.0.1:42424")) + settings = SimpleNamespace(public_base_url="https://app.tesslate.com", is_desktop_mode=True) + monkeypatch.setattr(mcp_oauth, "get_settings", lambda: settings) + + assert mcp_oauth._callback_url(request) == "http://127.0.0.1:42424/api/mcp/oauth/callback" + + +def test_mcp_oauth_callback_prefers_public_base_url_outside_desktop(monkeypatch): + """Hosted MCP OAuth redirects still use the configured public callback.""" + from app.routers import mcp_oauth + + request = SimpleNamespace(url=SimpleNamespace(scheme="http", netloc="127.0.0.1:42424")) + settings = SimpleNamespace(public_base_url="https://app.tesslate.com/", is_desktop_mode=False) + monkeypatch.setattr(mcp_oauth, "get_settings", lambda: settings) + + assert mcp_oauth._callback_url(request) == "https://app.tesslate.com/api/mcp/oauth/callback" + + +async def test_mcp_reconnect_uses_loopback_origin_in_desktop(monkeypatch): + """Reconnect should use the same desktop-safe callback as initial OAuth.""" + from app.routers import mcp + + config_id = uuid4() + user_id = uuid4() + captured: dict[str, str] = {} + config = SimpleNamespace( + id=config_id, + marketplace_agent_id=None, + oauth_connection=SimpleNamespace( + server_url="https://example.com/mcp", + registration_method="dcr", + ), + scope_level="user", + team_id=None, + project_id=None, + ) + request = SimpleNamespace(url=SimpleNamespace(scheme="http", netloc="127.0.0.1:42424")) + user = SimpleNamespace(id=user_id, default_team_id=None) + settings = SimpleNamespace(public_base_url="https://app.tesslate.com", is_desktop_mode=True) + + async def fake_get_owned_config(*args, **kwargs): + return config + + async def fake_start_oauth_flow(**kwargs): + captured["redirect_uri"] = kwargs["redirect_uri"] + return SimpleNamespace(authorize_url="https://provider.example/authorize", flow_id="flow-1") + + monkeypatch.setattr(mcp, "get_settings", lambda: settings) + monkeypatch.setattr(mcp, "_get_owned_config", fake_get_owned_config) + monkeypatch.setattr("app.services.mcp.oauth_flow.start_oauth_flow", fake_start_oauth_flow) + + result = await mcp.reconnect_mcp_config(config_id, request, user=user, db=SimpleNamespace()) + + assert result.flow_id == "flow-1" + assert captured["redirect_uri"] == "http://127.0.0.1:42424/api/mcp/oauth/callback" + + def test_assignment_ownership_uses_or_filter(): """Agent assignment endpoints must use OR-based ownership filter.