diff --git a/.env.example b/.env.example index 6edea7c4c..2c6c2065a 100644 --- a/.env.example +++ b/.env.example @@ -159,6 +159,21 @@ SEARXNG_INSTANCE=http://localhost:8080 # Local HTTP setups may use the callback URL inferred by the application. # GOOGLE_OAUTH_REDIRECT_URI=https://your-domain.com/api/email/oauth/google/callback +# Origin the MCP OAuth callback is sent back to, for remote (Streamable HTTP) +# MCP servers that register it dynamically. Defaults to http://localhost:$APP_PORT, +# which is right only when you reach Odysseus directly on that port. Set it for +# HTTPS, reverse-proxy, hosted, and Docker installs — inside the container the +# app always listens on 7000 and cannot see the host port map, so the default is +# wrong there whenever APP_PORT is not 7000. +# +# Not for Google MCP servers. Those use Desktop App credentials, and Google only +# accepts loopback redirect URIs for that client type, so a public origin here is +# rejected with redirect_uri_mismatch. Leave it unset for a Google-only install: +# the loopback default is what Google wants, and remote users finish through the +# paste-back page, which never has to load the redirect. +# https://developers.google.com/identity/protocols/oauth2/native-app +# OAUTH_REDIRECT_BASE_URL=https://your-domain.com + # ============================================================ # Misc # ============================================================ diff --git a/build-macos-app.sh b/build-macos-app.sh index 1208a1dce..c76075cac 100755 --- a/build-macos-app.sh +++ b/build-macos-app.sh @@ -73,6 +73,10 @@ cat > "$APP/Contents/MacOS/$APP_NAME.tmpl" <<'LAUNCHER' INSTALL_DIR="__INSTALL_DIR__" PORT="__PORT__" URL="http://127.0.0.1:${PORT}" +# uvicorn is started with --port below, but APP_PORT is what the app itself +# reads when it needs to build a URL for this instance (internal_api_base(), +# companion pairing, the MCP OAuth callback), so export it as well. +export APP_PORT="$PORT" export PATH="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:$PATH" UVICORN="$INSTALL_DIR/venv/bin/uvicorn" diff --git a/docker-compose.gpu-amd.yml b/docker-compose.gpu-amd.yml index 85d872b58..24f65e5cb 100644 --- a/docker-compose.gpu-amd.yml +++ b/docker-compose.gpu-amd.yml @@ -75,6 +75,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/docker-compose.gpu-nvidia.yml b/docker-compose.gpu-nvidia.yml index a367be001..fc66234db 100644 --- a/docker-compose.gpu-nvidia.yml +++ b/docker-compose.gpu-nvidia.yml @@ -74,6 +74,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/docker-compose.yml b/docker-compose.yml index 39774fb53..38cb654e5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -63,6 +63,11 @@ services: - GOOGLE_OAUTH_CLIENT_ID=${GOOGLE_OAUTH_CLIENT_ID:-} - GOOGLE_OAUTH_CLIENT_SECRET=${GOOGLE_OAUTH_CLIENT_SECRET:-} - GOOGLE_OAUTH_REDIRECT_URI=${GOOGLE_OAUTH_REDIRECT_URI:-} + # Externally reachable origin for MCP OAuth callbacks. The container + # always listens on 7000 and cannot see the host port map above, so + # remote MCP OAuth needs this set whenever the browser reaches + # Odysseus on anything other than http://localhost:7000. + - OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-} - TAVILY_API_KEY=${TAVILY_API_KEY:-} - SERPER_API_KEY=${SERPER_API_KEY:-} # PUID / PGID — the user/group the container drops to before diff --git a/launch-windows.ps1 b/launch-windows.ps1 index 263d95127..ab0e3542b 100644 --- a/launch-windows.ps1 +++ b/launch-windows.ps1 @@ -163,6 +163,10 @@ if (Test-Path $cudaBase) { } # 7. Start the server (use `python -m uvicorn` - bare `uvicorn` may not be on PATH) +# -Port only reaches uvicorn as a flag. Everything that builds a URL for this +# instance - internal_api_base(), companion pairing, the MCP OAuth callback - +# reads APP_PORT, so set it too or they all assume 7000. +$env:APP_PORT = $Port Write-Step ("Starting Odysseus at http://{0}:{1}" -f $BindHost, $Port) Write-Host "Press Ctrl+C to stop." Write-Host "" diff --git a/routes/mcp/mcp_routes.py b/routes/mcp/mcp_routes.py index a0ade88b6..94c83f8dd 100644 --- a/routes/mcp/mcp_routes.py +++ b/routes/mcp/mcp_routes.py @@ -475,7 +475,7 @@ def setup_mcp_routes(mcp_manager: McpManager): return RedirectResponse(auth_url) else: # Remote device — show paste-back page - return HTMLResponse(_oauth_authorize_page(auth_url, server_id, host, redirect_uri)) + return HTMLResponse(_oauth_authorize_page(auth_url, server_id, redirect_uri)) finally: db.close() @@ -612,15 +612,13 @@ def setup_mcp_routes(mcp_manager: McpManager): def _oauth_authorize_page( auth_url: str, server_id: str, - host: str, - redirect_uri: str = "http://localhost:7000/api/mcp/oauth/callback", + redirect_uri: str, ) -> str: """Page with Google sign-in link and URL paste-back form for remote access.""" - # Escape values interpolated into the page: `host` comes from the request - # Host header and `server_id` from the OAuth state — neither is trusted. + # Escape values interpolated into the page: `server_id` comes from the OAuth + # state and is not trusted. auth_url = html.escape(auth_url, quote=True) server_id = html.escape(server_id, quote=True) - host = html.escape(host, quote=True) redirect_uri = html.escape(redirect_uri, quote=True) return f""" @@ -664,7 +662,15 @@ def _oauth_authorize_page( Sign in with Google
-
+ +

Paste the URL from your browser after signing in:


diff --git a/src/mcp_oauth.py b/src/mcp_oauth.py index 27a30383e..8c69717eb 100644 --- a/src/mcp_oauth.py +++ b/src/mcp_oauth.py @@ -15,18 +15,32 @@ from urllib.parse import urlparse, parse_qs logger = logging.getLogger(__name__) + +def _resolve_redirect_base() -> str: + """Origin the browser is sent back to after authorizing. + + Falls back to the port the app binds natively (APP_PORT, read the same way + by app.py and launcher.py) rather than a fixed 7000: the macOS launcher + defaults to 7860, and a callback on the wrong port reaches nothing. The + hostname stays `localhost` rather than internal_api_base()'s 127.0.0.1 — + this URI is registered with the authorization server (via DCR, or by hand + for Google clients), so changing the host invalidates registrations that + already exist. + """ + return ( + os.environ.get("OAUTH_REDIRECT_BASE_URL") + or os.environ.get("APP_PUBLIC_URL") + or f"http://localhost:{os.environ.get('APP_PORT', '7000')}" + ).rstrip("/") + + # OAuth redirect URI registered with every authorization server via DCR. Loopback # is allowed for native/desktop clients (RFC 8252); remote users finish via the -# paste-back flow. Deployments not reachable at http://localhost:7000 (custom -# port, reverse proxy, or public domain) must set OAUTH_REDIRECT_BASE_URL (or -# APP_PUBLIC_URL) to their externally reachable origin so the redirect lands back -# on Odysseus. APP_PORT is intentionally not used: it is only the Docker host -# port-map; the app always listens on 7000 inside the container. -_REDIRECT_BASE = ( - os.environ.get("OAUTH_REDIRECT_BASE_URL") - or os.environ.get("APP_PUBLIC_URL") - or "http://localhost:7000" -).rstrip("/") +# paste-back flow. Deployments whose externally reachable origin differs from the +# port Odysseus binds — reverse proxy, public domain, or Docker, whose host port +# map is invisible inside the container — must set OAUTH_REDIRECT_BASE_URL (or +# APP_PUBLIC_URL), otherwise the redirect never lands back on Odysseus. +_REDIRECT_BASE = _resolve_redirect_base() REDIRECT_URI = f"{_REDIRECT_BASE}/api/mcp/oauth/callback" # How long the background connect waits for the user to authorize before giving up. diff --git a/start-macos.sh b/start-macos.sh index 2aa15d261..3e9048547 100755 --- a/start-macos.sh +++ b/start-macos.sh @@ -34,6 +34,10 @@ fi # values (APP_PORT / APP_BIND), then built-in defaults. PORT="${ODYSSEUS_PORT:-${APP_PORT:-7860}}" # 7860, not 7000 — macOS AirPlay Receiver holds 7000. HOST="${ODYSSEUS_HOST:-${APP_BIND:-127.0.0.1}}" # Set APP_BIND=0.0.0.0 in .env for LAN/Tailscale access. +# The port only reaches uvicorn as a flag, so export it too: everything that +# builds a URL for this instance — internal_api_base(), the companion pairing +# code, the MCP OAuth callback — reads APP_PORT and would otherwise assume 7000. +export APP_PORT="$PORT" PROBE_HOST="$HOST" if [ "$PROBE_HOST" = "0.0.0.0" ] || [ "$PROBE_HOST" = "::" ]; then PROBE_HOST="127.0.0.1" diff --git a/tests/test_mcp_oauth.py b/tests/test_mcp_oauth.py index 6fb6f43b9..6a8e21876 100644 --- a/tests/test_mcp_oauth.py +++ b/tests/test_mcp_oauth.py @@ -130,3 +130,137 @@ def test_update_recovers_from_non_dict_oauth_tokens(): srv, storage = _fake_storage('["stale", "data"]') storage._update("tokens", {"access_token": "new"}) assert json.loads(srv.oauth_tokens) == {"tokens": {"access_token": "new"}} + + +# ── Callback origin ─────────────────────────────────────────────── +# +# The redirect URI is registered with the authorization server (dynamically for +# remote MCP servers, by hand for Google ones) and the browser is sent to it +# after authorizing. It is resolved once, outside any request, so it cannot be +# derived from the request the way the email OAuth routes derive theirs — an +# operator-supplied origin is the only thing that can be right behind a proxy. +# What the default can get right is the port, which the app knows. + +_REDIRECT_ENV = ("OAUTH_REDIRECT_BASE_URL", "APP_PUBLIC_URL", "APP_PORT") + + +def _resolve_base(monkeypatch, **env): + for key in _REDIRECT_ENV: + monkeypatch.delenv(key, raising=False) + for key, value in env.items(): + monkeypatch.setenv(key, value) + return mcp_oauth._resolve_redirect_base() + + +def test_redirect_base_defaults_to_the_bound_port(monkeypatch): + # The macOS launcher serves on 7860 because AirPlay Receiver holds 7000; a + # callback pinned to 7000 lands on AirPlay instead of Odysseus. + assert _resolve_base(monkeypatch, APP_PORT="7860") == "http://localhost:7860" + + +def test_redirect_base_keeps_7000_when_app_port_is_unset(monkeypatch): + assert _resolve_base(monkeypatch) == "http://localhost:7000" + + +def test_redirect_base_prefers_the_explicit_origin(monkeypatch): + # Only an operator-supplied origin can be right behind a TLS proxy, so it + # outranks the derived default — and its trailing slash is trimmed. + resolved = _resolve_base( + monkeypatch, OAUTH_REDIRECT_BASE_URL="https://odysseus.example/", APP_PORT="7860" + ) + assert resolved == "https://odysseus.example" + + +def test_redirect_base_accepts_app_public_url_as_the_alias(monkeypatch): + assert ( + _resolve_base(monkeypatch, APP_PUBLIC_URL="https://public.example", APP_PORT="7860") + == "https://public.example" + ) + + +# ── Paste-back form origin ──────────────────────────────────────── + +def _authorize_page(): + from routes.mcp.mcp_routes import _oauth_authorize_page + + return _oauth_authorize_page( + "https://accounts.google.com/o/oauth2/v2/auth?state=srv-1", + "srv-1", + "https://odysseus.example.com/api/mcp/oauth/callback", + ) + + +def test_paste_back_form_action_is_relative(): + # Remote users finish the flow by pasting the callback URL into this form, + # so it has to post back to the origin they are on. An absolute action + # cannot: an http:// one is mixed content on an HTTPS page and gets blocked, + # and the app cannot reliably tell that it is behind TLS, because uvicorn + # only honours X-Forwarded-Proto from a peer inside --forwarded-allow-ips + # (default 127.0.0.1, which a proxy on the Docker bridge is not). A relative + # action is resolved by the browser and is right in every one of those cases. + page = _authorize_page() + assert 'action="/api/mcp/oauth/exchange/srv-1"' in page + assert 'action="http' not in page + + +# ── Docker configurability ──────────────────────────────────────── +# +# The override above is the only fix available to a Docker install: the +# container always listens on 7000 and cannot see the host port map, so the +# derived default cannot be right there. Compose has to forward the variable +# or the escape hatch does not exist. + +_COMPOSE_FILES = ( + "docker-compose.yml", + "docker-compose.gpu-nvidia.yml", + "docker-compose.gpu-amd.yml", +) + + +def _repo_root(): + from pathlib import Path + + return Path(__file__).resolve().parent.parent + + +def test_redirect_base_override_is_forwarded_into_the_container(): + import yaml + + for name in _COMPOSE_FILES: + path = _repo_root() / name + compose = yaml.safe_load(path.read_text(encoding="utf-8")) + environment = set(compose["services"]["odysseus"]["environment"]) + assert "OAUTH_REDIRECT_BASE_URL=${OAUTH_REDIRECT_BASE_URL:-}" in environment, name + + +def test_redirect_base_override_is_documented(): + import pytest + + env_example = _repo_root() / ".env.example" + if not env_example.exists(): + pytest.skip("this checkout does not include the optional .env.example file") + assert "# OAUTH_REDIRECT_BASE_URL=" in env_example.read_text(encoding="utf-8") + + +# ── Launcher port propagation ───────────────────────────────────── +# +# The derived default is only as good as APP_PORT, and every launcher hands the +# port to uvicorn as a command-line flag, which the app cannot read back. Each +# one has to put the same value in the environment or the callback falls back to +# 7000 — which is the macOS-launcher-on-7860 case this whole change is about. +# internal_api_base() and companion pairing read APP_PORT too, so they go wrong +# in the same way. + +_LAUNCHERS = ( + # file, the export, the uvicorn flag it has to agree with + ("start-macos.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'), + ("build-macos-app.sh", 'export APP_PORT="$PORT"', '--port "$PORT"'), + ("launch-windows.ps1", "$env:APP_PORT = $Port", "--port $Port"), +) + + +def test_launchers_export_the_port_they_serve_on(): + for name, export, uvicorn_flag in _LAUNCHERS: + text = (_repo_root() / name).read_text(encoding="utf-8") + assert uvicorn_flag in text, f"{name}: launcher no longer passes {uvicorn_flag}" + assert export in text, f"{name}: serves on a port the app cannot read back" diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index cf899f7c0..be8d3b8a3 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -1027,9 +1027,13 @@ def test_session_html_export_escapes_name(): def test_mcp_oauth_page_escapes_reflected_values(): src = Path(__file__).resolve().parents[1] / "routes" / "mcp" / "mcp_routes.py" text = src.read_text() - body = text.split("def _oauth_authorize_page(", 1)[1].split("return f", 1)[0] - for var in ("auth_url", "server_id", "host", "redirect_uri"): + page = text.split("def _oauth_authorize_page(", 1)[1].split("def _oauth_result_page", 1)[0] + body = page.split("return f", 1)[0] + for var in ("auth_url", "server_id", "redirect_uri"): assert f"{var} = html.escape({var}" in body, var + # The Host header is no longer reflected at all: the paste-back form posts to + # a relative action, so there is nothing to escape and nothing to smuggle. + assert "{host}" not in page def _import_mcp_routes():