diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 9f434819dff..4a4b8d4b5ab 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1456,7 +1456,12 @@ _OAUTH_PROVIDER_CATALOG: tuple[Dict[str, Any], ...] = ( { "id": "minimax-oauth", "name": "MiniMax (OAuth)", - "flow": "pkce", + # MiniMax's flow is structurally device-code (verification URI + + # user code, backend polls the token endpoint) with a PKCE + # extension for code-binding. The dashboard renders the same UX + # as Nous's device-code flow; the PKCE bit is a security + # extension that doesn't change the operator experience. + "flow": "device_code", "cli_command": "hermes auth add minimax-oauth", "docs_url": "https://www.minimax.io", "status_fn": None, # dispatched via auth.get_minimax_oauth_auth_status @@ -1819,7 +1824,7 @@ def _submit_anthropic_pkce(session_id: str, code_input: str) -> Dict[str, Any]: async def _start_device_code_flow(provider_id: str) -> Dict[str, Any]: - """Initiate a device-code flow (Nous or OpenAI Codex). + """Initiate a device-code flow (Nous, OpenAI Codex, or MiniMax). Calls the provider's device-auth endpoint via the existing CLI helpers, then spawns a background poller. Returns the user-facing display fields @@ -1898,6 +1903,82 @@ async def _start_device_code_flow(provider_id: str) -> Dict[str, Any]: "poll_interval": int(s.get("interval") or 5), } + if provider_id == "minimax-oauth": + # MiniMax uses a device-code-style flow (verification URI + user + # code + background poll) with a PKCE extension on top. From the + # operator's perspective it's identical to Nous's device-code + # flow; the PKCE bit (verifier + challenge from + # _minimax_pkce_pair) is a security extension that binds the + # token exchange to the original session. + from hermes_cli.auth import ( + _minimax_pkce_pair, + _minimax_request_user_code, + MINIMAX_OAUTH_CLIENT_ID, + MINIMAX_OAUTH_GLOBAL_BASE, + ) + import httpx + verifier, challenge, state = _minimax_pkce_pair() + portal_base_url = ( + os.getenv("MINIMAX_PORTAL_BASE_URL") or MINIMAX_OAUTH_GLOBAL_BASE + ).rstrip("/") + def _do_minimax_request(): + with httpx.Client( + timeout=httpx.Timeout(15.0), + headers={"Accept": "application/json"}, + follow_redirects=True, + ) as client: + return _minimax_request_user_code( + client=client, + portal_base_url=portal_base_url, + client_id=MINIMAX_OAUTH_CLIENT_ID, + code_challenge=challenge, + state=state, + ) + device_data = await asyncio.get_event_loop().run_in_executor( + None, _do_minimax_request + ) + sid, sess = _new_oauth_session("minimax-oauth", "device_code") + # The CLI flow names this `interval_ms` because MiniMax's + # `interval` field is in milliseconds (defensive default 2000ms + # in _minimax_poll_token). + interval_raw = device_data.get("interval") + sess["interval_ms"] = ( + int(interval_raw) if interval_raw is not None else None + ) + sess["user_code"] = str(device_data["user_code"]) + sess["code_verifier"] = verifier + sess["state"] = state + sess["portal_base_url"] = portal_base_url + sess["client_id"] = MINIMAX_OAUTH_CLIENT_ID + sess["region"] = "global" + # `expired_in` from MiniMax is overloaded — could be a unix-ms + # timestamp OR a seconds-from-now duration. Mirror the heuristic + # in _minimax_poll_token. Stash the raw value for the poller; + # compute a derived expires_at + UI-friendly expires_in seconds. + expired_in_raw = int(device_data["expired_in"]) + sess["expired_in_raw"] = expired_in_raw + if expired_in_raw > 1_000_000_000_000: # likely unix-ms + expires_at_ts = expired_in_raw / 1000.0 + expires_in_seconds = max(0, int(expires_at_ts - time.time())) + else: + expires_at_ts = time.time() + expired_in_raw + expires_in_seconds = expired_in_raw + sess["expires_at"] = expires_at_ts + threading.Thread( + target=_minimax_poller, + args=(sid,), + daemon=True, + name=f"oauth-poll-{sid[:6]}", + ).start() + return { + "session_id": sid, + "flow": "device_code", + "user_code": str(device_data["user_code"]), + "verification_url": str(device_data["verification_uri"]), + "expires_in": expires_in_seconds, + "poll_interval": max(2, (sess["interval_ms"] or 2000) // 1000), + } + raise HTTPException(status_code=400, detail=f"Provider {provider_id} does not support device-code flow") @@ -1959,6 +2040,86 @@ def _nous_poller(session_id: str) -> None: sess["error_message"] = str(e) +def _minimax_poller(session_id: str) -> None: + """Background poller that drives a MiniMax OAuth flow to completion. + + Mirrors `_nous_poller` but calls the MiniMax-specific token endpoint, + which uses a PKCE-style ``code_verifier`` + ``user_code`` rather than + the ``device_code`` field used by Nous. On success, builds the same + auth_state dict that ``_minimax_oauth_login`` (the CLI flow) builds + and persists via ``_minimax_save_auth_state`` — so the dashboard + path leaves the system in the same state as + ``hermes auth add minimax-oauth``. + """ + from hermes_cli.auth import ( + _minimax_poll_token, + _minimax_save_auth_state, + MINIMAX_OAUTH_GLOBAL_INFERENCE, + MINIMAX_OAUTH_SCOPE, + ) + from datetime import datetime, timezone + import httpx + with _oauth_sessions_lock: + sess = _oauth_sessions.get(session_id) + if not sess: + return + portal_base_url = sess["portal_base_url"] + client_id = sess["client_id"] + user_code = sess["user_code"] + code_verifier = sess["code_verifier"] + interval_ms = sess.get("interval_ms") + expired_in_raw = sess["expired_in_raw"] + try: + with httpx.Client( + timeout=httpx.Timeout(15.0), + headers={"Accept": "application/json"}, + follow_redirects=True, + ) as client: + token_data = _minimax_poll_token( + client=client, + portal_base_url=portal_base_url, + client_id=client_id, + user_code=user_code, + code_verifier=code_verifier, + expired_in=expired_in_raw, + interval_ms=interval_ms, + ) + # Build the auth_state dict in the same shape as the CLI flow's + # `_minimax_oauth_login` so `_minimax_save_auth_state` writes + # the canonical record. Region is fixed to "global" for the + # dashboard path; cn-region operators can still use the CLI + # flow which supports `--region cn`. + now = datetime.now(timezone.utc) + expires_in_s = int(token_data["expired_in"]) + expires_at_ts = now.timestamp() + expires_in_s + auth_state = { + "provider": "minimax-oauth", + "region": sess.get("region", "global"), + "portal_base_url": portal_base_url, + "inference_base_url": MINIMAX_OAUTH_GLOBAL_INFERENCE, + "client_id": client_id, + "scope": MINIMAX_OAUTH_SCOPE, + "token_type": token_data.get("token_type", "Bearer"), + "access_token": token_data["access_token"], + "refresh_token": token_data["refresh_token"], + "resource_url": token_data.get("resource_url"), + "obtained_at": now.isoformat(), + "expires_at": datetime.fromtimestamp( + expires_at_ts, tz=timezone.utc + ).isoformat(), + "expires_in": expires_in_s, + } + _minimax_save_auth_state(auth_state) + with _oauth_sessions_lock: + sess["status"] = "approved" + _log.info("oauth/device: minimax login completed (session=%s)", session_id) + except Exception as e: + _log.warning("minimax device-code poll failed (session=%s): %s", session_id, e) + with _oauth_sessions_lock: + sess["status"] = "error" + sess["error_message"] = str(e) + + def _codex_full_login_worker(session_id: str) -> None: """Run the complete OpenAI Codex device-code flow. @@ -2111,7 +2272,13 @@ async def start_oauth_login(provider_id: str, request: Request): detail=f"{provider_id} uses an external CLI; run `{catalog_entry['cli_command']}` manually", ) try: - if catalog_entry["flow"] == "pkce": + # The pkce branch is gated on provider_id == "anthropic" because + # `_start_anthropic_pkce()` is hardcoded to the Anthropic flow. + # Routing any other future pkce-flagged provider through it would + # silently launch the Anthropic OAuth flow (the bug fixed in this + # change for MiniMax). New PKCE providers must add their own + # start function and an explicit branch here. + if catalog_entry["flow"] == "pkce" and provider_id == "anthropic": return _start_anthropic_pkce() if catalog_entry["flow"] == "device_code": return await _start_device_code_flow(provider_id) diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py new file mode 100644 index 00000000000..6ebd0ad7235 --- /dev/null +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -0,0 +1,129 @@ +"""Regression tests for the OAuth dispatcher in hermes_cli.web_server. + +Bug history (2026-05-09): the `_OAUTH_PROVIDER_CATALOG` had two entries +flagged ``flow: "pkce"`` — anthropic and minimax-oauth — and the +dispatcher ``start_oauth_login`` hardcoded ``_start_anthropic_pkce()`` +for any pkce-flagged provider. So clicking "Login" next to MiniMax in +the dashboard's Keys tab silently launched the Anthropic/Claude OAuth +flow. + +The fix: + 1. Catalog entry for minimax-oauth changed from ``flow: "pkce"`` to + ``flow: "device_code"`` (the actual UX is verification URI + user + code + background poll, with PKCE as a security extension). + 2. New MiniMax branch added to ``_start_device_code_flow``. + 3. Dispatcher tightened: pkce branch now requires + ``provider_id == "anthropic"``, so any future PKCE provider added + without an explicit branch gets a clean ``400 Unsupported flow`` + instead of silently launching Anthropic OAuth. + +These tests pin the corrected behavior. +""" +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from hermes_cli.web_server import _SESSION_TOKEN, app + +client = TestClient(app) +HEADERS = {"X-Hermes-Session-Token": _SESSION_TOKEN} + + +def test_minimax_login_does_not_launch_anthropic_flow(): + """Click 'Login' on MiniMax → MUST NOT return claude.ai auth_url.""" + fake_user_code_resp = { + "user_code": "ABCD-1234", + "verification_uri": "https://api.minimax.io/oauth/verify", + # `expired_in` < 1e12 so the heuristic treats it as seconds. + "expired_in": 600, + "interval": 2000, + "state": "stub-state", + } + with patch( + "hermes_cli.auth._minimax_request_user_code", + return_value=fake_user_code_resp, + ), patch( + "hermes_cli.auth._minimax_pkce_pair", + return_value=("verifier-stub", "challenge-stub", "stub-state"), + ): + resp = client.post( + "/api/providers/oauth/minimax-oauth/start", + headers=HEADERS, + ) + + assert resp.status_code == 200, resp.text + body = resp.json() + + # The bug used to return Anthropic's auth_url — make sure the response + # references neither the auth_url field nor anything Claude-related. + assert "auth_url" not in body + assert "claude.ai" not in str(body).lower() + + # And the response IS the device-code shape pointing at MiniMax. + assert body["flow"] == "device_code" + assert "minimax" in body["verification_url"].lower() + assert body["user_code"] == "ABCD-1234" + assert body["expires_in"] == 600 + + +def test_anthropic_pkce_branch_still_works(): + """Sanity: the dispatcher tightening doesn't break the legitimate Anthropic PKCE path.""" + fake_anthropic_response = { + "session_id": "stub-session", + "flow": "pkce", + "auth_url": "https://claude.ai/oauth/authorize?code=true&...", + "expires_in": 600, + } + with patch( + "hermes_cli.web_server._start_anthropic_pkce", + return_value=fake_anthropic_response, + ): + resp = client.post( + "/api/providers/oauth/anthropic/start", + headers=HEADERS, + ) + + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["flow"] == "pkce" + assert "claude.ai" in body["auth_url"] + + +def test_unknown_pkce_provider_rejected_cleanly(): + """A future PKCE provider without an explicit branch must NOT silently route to Anthropic. + + Simulates a hypothetical catalog entry with ``flow: "pkce"`` and an + id other than "anthropic". The dispatcher should fall through past + the pkce branch (now gated on provider_id) and the device_code + branch, then hit "Unsupported flow" — proving the bug class is + structurally prevented. + """ + from hermes_cli import web_server as ws + + # Inject a hypothetical catalog entry that's pkce-flagged but isn't + # anthropic. This shape mirrors what would happen if a developer + # added a new provider entry without remembering to wire up its + # start function. + fake_entry = { + "id": "hypothetical-pkce-provider", + "name": "Hypothetical PKCE Provider", + "flow": "pkce", + "cli_command": "hermes auth add hypothetical-pkce-provider", + "docs_url": "https://example.com", + "status_fn": None, + } + original_catalog = ws._OAUTH_PROVIDER_CATALOG + try: + ws._OAUTH_PROVIDER_CATALOG = original_catalog + (fake_entry,) + resp = client.post( + "/api/providers/oauth/hypothetical-pkce-provider/start", + headers=HEADERS, + ) + finally: + ws._OAUTH_PROVIDER_CATALOG = original_catalog + + # Either 400 "Unsupported flow" (the explicit fall-through) or any + # 4xx — what we MUST NOT see is a 200 with claude.ai in the body. + assert resp.status_code >= 400, resp.text + assert "claude.ai" not in resp.text.lower()