mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-17 18:25:42 +02:00
[eric] oauth: build the redirect uri from the port we are actually served on, not a guessed 8324
This commit is contained in:
@@ -4,7 +4,7 @@ from backend.apps.agents.core.ws_manager import ws_manager
|
||||
from backend.apps.agents.core.models import AgentConfig, ApprovalResponse
|
||||
from backend.apps.agents.manager.session.history_compaction import estimate_post_compact_input
|
||||
from contextlib import asynccontextmanager
|
||||
from fastapi import WebSocket, WebSocketDisconnect, HTTPException
|
||||
from fastapi import WebSocket, WebSocketDisconnect, HTTPException, Request
|
||||
from fastapi.responses import JSONResponse
|
||||
import asyncio
|
||||
import json
|
||||
@@ -385,7 +385,7 @@ async def subscriptions_status():
|
||||
|
||||
|
||||
@agents.router.post("/subscriptions/connect")
|
||||
async def subscriptions_connect(body: dict):
|
||||
async def subscriptions_connect(body: dict, request: Request):
|
||||
"""Start OAuth flow for a subscription provider."""
|
||||
from backend.apps.nine_router import is_running, ensure_running, start_oauth
|
||||
provider = body.get("provider", "")
|
||||
@@ -406,7 +406,9 @@ async def subscriptions_connect(body: dict):
|
||||
pass
|
||||
|
||||
try:
|
||||
result = await start_oauth(provider)
|
||||
# The port the user's app actually reached us on beats guessing the default; only consulted
|
||||
# when OPENSWARM_PORT is unset (dev uvicorn launches), never in packaged builds.
|
||||
result = await start_oauth(provider, request.url.port)
|
||||
|
||||
if result.get("flow") == "authorization_code" and result.get("state"):
|
||||
from backend.apps.oauth_state import pending_oauth
|
||||
|
||||
@@ -7,6 +7,7 @@ Talks to the already-running 9Router over HTTP; never spawns the subprocess
|
||||
import asyncio
|
||||
import logging
|
||||
import os
|
||||
from typing import Optional
|
||||
|
||||
import httpx
|
||||
|
||||
@@ -211,7 +212,7 @@ async def p_start_codex_callback_listener(timeout: float = 300.0) -> int | None:
|
||||
return bound_port
|
||||
|
||||
|
||||
# Providers whose OAuth flow MUST run in the user's real browser via shell.openExternal, not the in-Electron window.open popup: - gemini-cli, antigravity: Google's Embedded WebView Restrictions policy uses JS-fingerprint detection that no UA spoof defeats. RFC 8252 and Google's own Desktop-app OAuth guidance both prescribe the system browser. - codex: auth.openai.com renders blank in our popup on some machines (newer embed detection + regional checks); system browser surfaces the real error. - claude: email magic-link opens in the user's default browser, which is a different cookie jar from the embedded popup, so the popup can never receive the auth. Forcing the OAuth flow into the system browser keeps everything in one cookie jar. The callback for gemini-cli/antigravity lands on /api/subscriptions/callback and runs the exchange server-side; codex uses its fixed 1455 listener; claude is special-cased in p_callback_uri_for_provider below.
|
||||
# Providers whose OAuth flow MUST run in the user's real browser via shell.openExternal, not the in-Electron window.open popup: - gemini-cli, antigravity: Google's Embedded WebView Restrictions policy uses JS-fingerprint detection that no UA spoof defeats. RFC 8252 and Google's own Desktop-app OAuth guidance both prescribe the system browser. - codex: auth.openai.com renders blank in our popup on some machines (newer embed detection + regional checks); system browser surfaces the real error. - claude: email magic-link opens in the user's default browser, which is a different cookie jar from the embedded popup, so the popup can never receive the auth. Forcing the OAuth flow into the system browser keeps everything in one cookie jar. The callback for gemini-cli/antigravity lands on /api/subscriptions/callback and runs the exchange server-side; codex uses its fixed 1455 listener; claude is special-cased in callback_uri_for_provider below.
|
||||
P_EXTERNAL_BROWSER_PROVIDERS: set[str] = {"gemini-cli", "antigravity", "codex", "claude"}
|
||||
|
||||
|
||||
@@ -219,21 +220,32 @@ def p_should_use_external_browser(provider: str) -> bool:
|
||||
return provider in P_EXTERNAL_BROWSER_PROVIDERS
|
||||
|
||||
|
||||
def p_backend_port() -> int:
|
||||
"""Best-effort lookup of the OpenSwarm backend HTTP port.
|
||||
def resolve_backend_port(observed: Optional[int] = None) -> int:
|
||||
"""The port this backend is actually reachable on, for building OAuth redirect URIs.
|
||||
|
||||
Falls back to 8324 (the default in backend/main.py) if OPENSWARM_PORT
|
||||
hasn't been set yet. backend/main.py:239 sets this env var at startup
|
||||
before any request handler runs, so `start_oauth` will always see the
|
||||
correct value.
|
||||
OPENSWARM_PORT is authoritative and Electron always passes it (main.js), so packaged builds
|
||||
take the first branch and behave exactly as before.
|
||||
|
||||
It is NOT always set in dev. main.py only exports it inside its `if __name__ == "__main__"`
|
||||
block, which never runs under `python -m uvicorn backend.main:app --port N`. The old code then
|
||||
assumed 8324 and stamped that into the redirect URI while uvicorn served a different port, so
|
||||
Google bounced the user to a dead port and Claude's callback missed the router rewrite. Codex
|
||||
kept working throughout, because OpenAI pins its own localhost:1455 listener, which is what
|
||||
made the failure look like "two providers are broken" instead of "the port is wrong".
|
||||
|
||||
`observed` is the port the caller was actually reached on (from the live request), which is
|
||||
ground truth on every launch path. Only consulted when the env var is absent.
|
||||
"""
|
||||
try:
|
||||
return int(os.environ.get("OPENSWARM_PORT", "8324"))
|
||||
except (TypeError, ValueError):
|
||||
return 8324
|
||||
raw = os.environ.get("OPENSWARM_PORT")
|
||||
if raw:
|
||||
try:
|
||||
return int(raw)
|
||||
except (TypeError, ValueError):
|
||||
pass
|
||||
return observed or 8324
|
||||
|
||||
|
||||
def p_callback_uri_for_provider(provider: str) -> str:
|
||||
def callback_uri_for_provider(provider: str, backend_port: Optional[int] = None) -> str:
|
||||
"""Return the redirect URI to pass to 9Router's authorize endpoint.
|
||||
|
||||
Most providers accept 9Router's built-in callback page at port 20128.
|
||||
@@ -252,15 +264,18 @@ def p_callback_uri_for_provider(provider: str) -> str:
|
||||
if provider == "claude":
|
||||
return f"http://localhost:{NINE_ROUTER_PORT}/callback"
|
||||
if provider in P_EXTERNAL_BROWSER_PROVIDERS:
|
||||
return f"http://localhost:{p_backend_port()}/api/subscriptions/callback"
|
||||
return f"http://localhost:{resolve_backend_port(backend_port)}/api/subscriptions/callback"
|
||||
return f"http://localhost:{NINE_ROUTER_PORT}/callback"
|
||||
|
||||
|
||||
async def start_oauth(provider: str) -> dict:
|
||||
async def start_oauth(provider: str, backend_port: Optional[int] = None) -> dict:
|
||||
"""Start OAuth flow for a provider.
|
||||
|
||||
For device_code providers (github, qwen, kiro): returns {user_code, verification_uri, device_code}
|
||||
For authorization_code providers (claude, codex, gemini-cli): returns {authUrl, codeVerifier, state}
|
||||
|
||||
`backend_port` is the port the connect request arrived on; it only matters when OPENSWARM_PORT
|
||||
is unset, which is the dev-launch case that used to send Google to a dead port.
|
||||
"""
|
||||
async with httpx.AsyncClient(timeout=15.0, headers=cli_auth_headers()) as client:
|
||||
try:
|
||||
@@ -278,7 +293,7 @@ async def start_oauth(provider: str) -> dict:
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
callback_url = p_callback_uri_for_provider(provider)
|
||||
callback_url = callback_uri_for_provider(provider, backend_port)
|
||||
if provider == "codex":
|
||||
# Codex's redirect must be an OpenAI allow-listed loopback port; bind the first free one (1455 else 1457) and use ITS redirect_uri so authorize + token exchange agree.
|
||||
bound_port = await p_start_codex_callback_listener()
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
"""The OAuth redirect URI must name the port this backend is actually reachable on.
|
||||
|
||||
Measured live 2026-07-28. Google bounced a real connect attempt to
|
||||
http://localhost:8324/api/subscriptions/callback -> ERR_CONNECTION_REFUSED, while the backend was
|
||||
serving 8326. Claude failed at the same moment for its own reason and Codex kept working, so the
|
||||
symptom presented as "two providers are broken" rather than "the port is wrong", which is the
|
||||
expensive kind of wrong.
|
||||
|
||||
Root cause: main.py exports OPENSWARM_PORT inside `if __name__ == "__main__"`. That block does not
|
||||
run under `python -m uvicorn backend.main:app --port N`, so the env var was absent and the helper
|
||||
fell back to the 8324 literal while uvicorn served something else. Packaged builds were never
|
||||
affected (electron/main.js passes OPENSWARM_PORT explicitly), which is exactly why it survived:
|
||||
it is invisible on the default port and invisible in prod.
|
||||
|
||||
The fix keeps the env var authoritative and uses the live request's port only as the fallback, so
|
||||
prod behaviour is unchanged and the dev path stops guessing.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from backend.apps.nine_router import oauth
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def no_ambient_port(monkeypatch):
|
||||
"""The suite must not inherit a real OPENSWARM_PORT from the developer's shell."""
|
||||
monkeypatch.delenv("OPENSWARM_PORT", raising=False)
|
||||
|
||||
|
||||
# --- the port helper -------------------------------------------------------------------------
|
||||
|
||||
def test_the_env_var_wins_when_set(monkeypatch):
|
||||
"""Packaged builds always set it; that path must not change."""
|
||||
monkeypatch.setenv("OPENSWARM_PORT", "8324")
|
||||
assert oauth.resolve_backend_port(observed=9999) == 8324
|
||||
|
||||
|
||||
def test_the_observed_port_is_used_when_the_env_var_is_missing():
|
||||
"""The regression: uvicorn on 8326 with no env var used to answer 8324."""
|
||||
assert oauth.resolve_backend_port(observed=8326) == 8326
|
||||
|
||||
|
||||
def test_it_still_falls_back_when_nothing_is_known():
|
||||
assert oauth.resolve_backend_port() == 8324
|
||||
|
||||
|
||||
def test_a_garbage_env_var_does_not_crash_the_connect_flow(monkeypatch):
|
||||
"""A malformed value must degrade to what we can observe, not raise mid-OAuth."""
|
||||
monkeypatch.setenv("OPENSWARM_PORT", "not-a-port")
|
||||
assert oauth.resolve_backend_port(observed=8326) == 8326
|
||||
|
||||
|
||||
def test_an_empty_env_var_is_treated_as_unset(monkeypatch):
|
||||
monkeypatch.setenv("OPENSWARM_PORT", "")
|
||||
assert oauth.resolve_backend_port(observed=8326) == 8326
|
||||
|
||||
|
||||
# --- the redirect URI itself -----------------------------------------------------------------
|
||||
|
||||
def test_google_gets_the_live_port_not_the_default():
|
||||
"""The exact failure: gemini-cli's callback runs through our own backend endpoint."""
|
||||
uri = oauth.callback_uri_for_provider("gemini-cli", 8326)
|
||||
assert uri == "http://localhost:8326/api/subscriptions/callback"
|
||||
|
||||
|
||||
def test_google_on_the_default_port_is_unchanged(monkeypatch):
|
||||
monkeypatch.setenv("OPENSWARM_PORT", "8324")
|
||||
assert oauth.callback_uri_for_provider("gemini-cli", 8326) == (
|
||||
"http://localhost:8324/api/subscriptions/callback")
|
||||
|
||||
|
||||
def test_codex_keeps_its_pinned_listener_port():
|
||||
"""OpenAI's client is bound to a fixed URI, which is why Codex kept connecting while the other
|
||||
two failed. It must never pick up the backend port."""
|
||||
uri = oauth.callback_uri_for_provider("codex", 8326)
|
||||
assert uri == "http://localhost:1455/auth/callback", (
|
||||
"OpenAI's OAuth client is registered against this exact URI; changing it breaks every "
|
||||
"ChatGPT connect")
|
||||
assert "8326" not in uri
|
||||
|
||||
|
||||
def test_claude_still_routes_through_the_router_callback():
|
||||
"""Anthropic only whitelists the router's callback; the backend port is not ours to substitute."""
|
||||
uri = oauth.callback_uri_for_provider("claude", 8326)
|
||||
assert str(oauth.NINE_ROUTER_PORT) in uri
|
||||
assert "8326" not in uri
|
||||
|
||||
|
||||
def test_no_provider_silently_hardcodes_the_default_port():
|
||||
"""Sweep every provider the flow knows about: on a non-default port, nothing may still say 8324
|
||||
unless it is deliberately router- or listener-pinned."""
|
||||
pinned = {"claude", "codex"}
|
||||
for provider in ("gemini-cli", "antigravity", "github", "qwen", "kiro"):
|
||||
if provider in pinned:
|
||||
continue
|
||||
uri = oauth.callback_uri_for_provider(provider, 8326)
|
||||
assert "8324" not in uri, f"{provider} stamped the default port into {uri}"
|
||||
Reference in New Issue
Block a user