fix(cli): harden quick start OAuth handling
This commit is contained in:
@@ -15,6 +15,7 @@ except ModuleNotFoundError: # pragma: no cover - exercised in environments with
|
|||||||
from loguru import logger
|
from loguru import logger
|
||||||
from pydantic import BaseModel
|
from pydantic import BaseModel
|
||||||
from rich.console import Console
|
from rich.console import Console
|
||||||
|
from rich.markup import escape
|
||||||
from rich.panel import Panel
|
from rich.panel import Panel
|
||||||
from rich.table import Table
|
from rich.table import Table
|
||||||
|
|
||||||
@@ -1634,30 +1635,30 @@ def _quick_start_oauth_login(config: Config, provider_name: str) -> bool:
|
|||||||
try:
|
try:
|
||||||
proxy = _quick_start_codex_proxy(config)
|
proxy = _quick_start_codex_proxy(config)
|
||||||
except ValueError as exc:
|
except ValueError as exc:
|
||||||
console.print(f"[red]{exc}[/red]")
|
console.print(f"[red]{escape(str(exc))}[/red]")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
token = None
|
token = None
|
||||||
with suppress(Exception):
|
with suppress(Exception):
|
||||||
token = get_token(proxy=proxy)
|
token = get_token(proxy=proxy)
|
||||||
if not (token and token.access):
|
if not getattr(token, "access", None):
|
||||||
console.print("[cyan]Starting interactive OAuth login...[/cyan]\n")
|
console.print("[cyan]Starting interactive OAuth login...[/cyan]\n")
|
||||||
try:
|
try:
|
||||||
token = login_oauth_interactive(
|
token = login_oauth_interactive(
|
||||||
print_fn=lambda message: console.print(message),
|
print_fn=lambda message: console.print(message, markup=False),
|
||||||
prompt_fn=lambda prompt: _get_questionary().text(prompt).ask() or "",
|
prompt_fn=lambda prompt: _get_questionary().text(prompt).ask() or "",
|
||||||
proxy=proxy,
|
proxy=proxy,
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
console.print(f"[red]OAuth login failed: {exc}[/red]")
|
console.print(f"[red]OAuth login failed: {escape(str(exc))}[/red]")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
if not (token and token.access):
|
if not getattr(token, "access", None):
|
||||||
console.print("[red]OAuth login failed[/red]")
|
console.print("[red]OAuth login failed[/red]")
|
||||||
return False
|
return False
|
||||||
|
|
||||||
account = getattr(token, "account_id", None)
|
account = getattr(token, "account_id", None)
|
||||||
suffix = f" [dim]{account}[/dim]" if account else ""
|
suffix = f" [dim]{escape(str(account))}[/dim]" if account else ""
|
||||||
console.print(f"[green]Authenticated with OpenAI Codex[/green]{suffix}")
|
console.print(f"[green]Authenticated with OpenAI Codex[/green]{suffix}")
|
||||||
return True
|
return True
|
||||||
|
|
||||||
@@ -1673,7 +1674,7 @@ def _quick_start_oauth_is_authenticated(config: Config, provider_name: str) -> b
|
|||||||
token = get_token(proxy=proxy)
|
token = get_token(proxy=proxy)
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
return bool(token and token.access)
|
return bool(getattr(token, "access", None))
|
||||||
|
|
||||||
|
|
||||||
def _quick_start_requires_base_url(provider_name: str, info: _QuickStartProviderInfo | None) -> bool:
|
def _quick_start_requires_base_url(provider_name: str, info: _QuickStartProviderInfo | None) -> bool:
|
||||||
|
|||||||
@@ -1092,6 +1092,55 @@ class TestMainMenuUpdate:
|
|||||||
assert config.providers.openai.api_key == "${UNRELATED_MISSING_KEY}"
|
assert config.providers.openai.api_key == "${UNRELATED_MISSING_KEY}"
|
||||||
assert config.providers.openai_codex.proxy == "${CODEX_PROXY}"
|
assert config.providers.openai_codex.proxy == "${CODEX_PROXY}"
|
||||||
|
|
||||||
|
def test_quick_start_openai_codex_runs_interactive_login_for_bad_cached_token(
|
||||||
|
self, monkeypatch
|
||||||
|
):
|
||||||
|
"""A malformed cached token should fall back to the interactive OAuth flow."""
|
||||||
|
import oauth_cli_kit
|
||||||
|
|
||||||
|
config = Config()
|
||||||
|
config.providers.openai_codex.proxy = "http://127.0.0.1:8080"
|
||||||
|
prompts: list[str] = []
|
||||||
|
printed: list[tuple[tuple[object, ...], dict[str, object]]] = []
|
||||||
|
|
||||||
|
class FakePrompt:
|
||||||
|
def ask(self):
|
||||||
|
return "authorization-code"
|
||||||
|
|
||||||
|
def fake_login(**kwargs):
|
||||||
|
kwargs["print_fn"]("[bold]Open the browser[/bold]")
|
||||||
|
prompts.append(kwargs["prompt_fn"]("Paste the authorization code"))
|
||||||
|
assert kwargs["proxy"] == "http://127.0.0.1:8080"
|
||||||
|
return SimpleNamespace(
|
||||||
|
access="fresh-token",
|
||||||
|
account_id="[red]account-123[/red]",
|
||||||
|
)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
oauth_cli_kit,
|
||||||
|
"get_token",
|
||||||
|
lambda **_kwargs: SimpleNamespace(account_id="missing-access"),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(oauth_cli_kit, "login_oauth_interactive", fake_login)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
onboard_wizard,
|
||||||
|
"_get_questionary",
|
||||||
|
lambda: SimpleNamespace(text=lambda *_args, **_kwargs: FakePrompt()),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
onboard_wizard.console,
|
||||||
|
"print",
|
||||||
|
lambda *args, **kwargs: printed.append((args, kwargs)),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert onboard_wizard._quick_start_oauth_login(config, "openai_codex") is True
|
||||||
|
assert prompts == ["authorization-code"]
|
||||||
|
assert any(
|
||||||
|
args == ("[bold]Open the browser[/bold]",) and kwargs == {"markup": False}
|
||||||
|
for args, kwargs in printed
|
||||||
|
)
|
||||||
|
assert any(r"\[red]account-123\[/red]" in str(args[0]) for args, _kwargs in printed)
|
||||||
|
|
||||||
def test_quick_start_codex_auth_check_ignores_unrelated_missing_env(self, monkeypatch):
|
def test_quick_start_codex_auth_check_ignores_unrelated_missing_env(self, monkeypatch):
|
||||||
"""OAuth readiness should depend only on the Codex proxy and token."""
|
"""OAuth readiness should depend only on the Codex proxy and token."""
|
||||||
import oauth_cli_kit
|
import oauth_cli_kit
|
||||||
@@ -1110,6 +1159,21 @@ class TestMainMenuUpdate:
|
|||||||
is True
|
is True
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_quick_start_codex_auth_check_rejects_malformed_token(self, monkeypatch):
|
||||||
|
"""A malformed cached token should report not-ready instead of crashing."""
|
||||||
|
import oauth_cli_kit
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
oauth_cli_kit,
|
||||||
|
"get_token",
|
||||||
|
lambda **_kwargs: SimpleNamespace(account_id="missing-access"),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert (
|
||||||
|
onboard_wizard._quick_start_oauth_is_authenticated(Config(), "openai_codex")
|
||||||
|
is False
|
||||||
|
)
|
||||||
|
|
||||||
def test_quick_start_summary_reports_missing_codex_oauth(self, monkeypatch):
|
def test_quick_start_summary_reports_missing_codex_oauth(self, monkeypatch):
|
||||||
"""The review step should distinguish OAuth from an API-key setup."""
|
"""The review step should distinguish OAuth from an API-key setup."""
|
||||||
config = Config()
|
config = Config()
|
||||||
|
|||||||
Reference in New Issue
Block a user