fix(onboarding): recognize credential_pool OAuth auth for openai-codex (#797)
fix(onboarding): recognize credential_pool OAuth auth for openai-codex (#797) The onboarding readiness check in `api/onboarding.py` only looked at the legacy `providers[provider]` key in `auth.json`. Hermes runtime resolves OAuth tokens from `credential_pool[provider]` (device-code / OAuth flows), so WebUI could report "not ready" while the runtime chatted successfully. The check now covers both storage locations with a fail-closed helper. Adds three regression tests. Reported in #796, fixed by @davidsben. Co-authored-by: davidsben <davidsben@users.noreply.github.com>
This commit is contained in:
@@ -230,26 +230,39 @@ def _provider_api_key_present(
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
def _oauth_payload_has_token(payload: dict) -> bool:
|
||||||
|
"""Return True if an auth payload contains usable token material."""
|
||||||
|
if not isinstance(payload, dict):
|
||||||
|
return False
|
||||||
|
|
||||||
|
token_fields = (
|
||||||
|
payload,
|
||||||
|
payload.get("tokens") if isinstance(payload.get("tokens"), dict) else {},
|
||||||
|
)
|
||||||
|
for candidate in token_fields:
|
||||||
|
if not isinstance(candidate, dict):
|
||||||
|
continue
|
||||||
|
if any(
|
||||||
|
str(candidate.get(key) or "").strip()
|
||||||
|
for key in ("access_token", "refresh_token", "api_key")
|
||||||
|
):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
def _provider_oauth_authenticated(provider: str, hermes_home: "Path") -> bool:
|
def _provider_oauth_authenticated(provider: str, hermes_home: "Path") -> bool:
|
||||||
"""Return True if the provider has valid OAuth credentials.
|
"""Return True if the provider has valid OAuth credentials.
|
||||||
|
|
||||||
Checks via hermes_cli.auth.get_auth_status() when available, then falls
|
Reads the profile-scoped auth.json directly so onboarding respects the
|
||||||
back to reading auth.json directly for the known OAuth provider IDs
|
requested Hermes home. Known OAuth providers may store auth either in the
|
||||||
(openai-codex, copilot, copilot-acp, qwen-oauth, nous).
|
legacy providers[provider_id] singleton state or in credential_pool entries
|
||||||
|
used by current Hermes runtime auth resolution.
|
||||||
This covers users who authenticated via 'hermes auth' or 'hermes model'
|
|
||||||
but whose provider is not in _SUPPORTED_PROVIDER_SETUPS because it does
|
|
||||||
not use a plain API key.
|
|
||||||
"""
|
"""
|
||||||
provider = (provider or "").strip().lower()
|
provider = (provider or "").strip().lower()
|
||||||
if not provider:
|
if not provider:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
# Check auth.json for known OAuth provider IDs.
|
|
||||||
# hermes_home scopes the check — callers must pass the correct home directory.
|
|
||||||
# (A prior CLI fast path via hermes_cli.auth.get_auth_status() was removed
|
|
||||||
# because it ignored hermes_home and read from the real system home, breaking
|
|
||||||
# both test isolation and deployments with multiple profiles.)
|
|
||||||
_known_oauth_providers = {"openai-codex", "copilot", "copilot-acp", "qwen-oauth", "nous"}
|
_known_oauth_providers = {"openai-codex", "copilot", "copilot-acp", "qwen-oauth", "nous"}
|
||||||
if provider not in _known_oauth_providers:
|
if provider not in _known_oauth_providers:
|
||||||
return False
|
return False
|
||||||
@@ -261,20 +274,20 @@ def _provider_oauth_authenticated(provider: str, hermes_home: "Path") -> bool:
|
|||||||
if not auth_path.exists():
|
if not auth_path.exists():
|
||||||
return False
|
return False
|
||||||
store = _j.loads(auth_path.read_text(encoding="utf-8"))
|
store = _j.loads(auth_path.read_text(encoding="utf-8"))
|
||||||
|
|
||||||
providers_store = store.get("providers")
|
providers_store = store.get("providers")
|
||||||
if not isinstance(providers_store, dict):
|
if isinstance(providers_store, dict):
|
||||||
return False
|
state = providers_store.get(provider)
|
||||||
state = providers_store.get(provider)
|
if _oauth_payload_has_token(state):
|
||||||
if not isinstance(state, dict):
|
return True
|
||||||
return False
|
|
||||||
# Any non-empty token is enough to confirm the user has credentials.
|
pool_store = store.get("credential_pool")
|
||||||
# Token refresh happens at runtime inside the agent.
|
if isinstance(pool_store, dict):
|
||||||
has_token = bool(
|
entries = pool_store.get(provider)
|
||||||
str(state.get("access_token") or "").strip()
|
if isinstance(entries, list):
|
||||||
or str(state.get("api_key") or "").strip()
|
return any(_oauth_payload_has_token(entry) for entry in entries)
|
||||||
or str(state.get("refresh_token") or "").strip()
|
|
||||||
)
|
return False
|
||||||
return has_token
|
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|||||||
@@ -41,6 +41,16 @@ def make_session(created_list):
|
|||||||
return sid
|
return sid
|
||||||
|
|
||||||
|
|
||||||
|
def _make_auth_json_with_credential_pool(
|
||||||
|
provider_id: str, pool_entries: list[dict], tmp_dir: pathlib.Path
|
||||||
|
) -> pathlib.Path:
|
||||||
|
"""Write an auth.json with only credential_pool entries for provider_id."""
|
||||||
|
store = {"providers": {}, "credential_pool": {provider_id: pool_entries}}
|
||||||
|
auth_path = tmp_dir / "auth.json"
|
||||||
|
auth_path.write_text(json.dumps(store), encoding="utf-8")
|
||||||
|
return auth_path
|
||||||
|
|
||||||
|
|
||||||
# ── R1: uuid not imported in server.py (Sprint 10 split regression) ──────────
|
# ── R1: uuid not imported in server.py (Sprint 10 split regression) ──────────
|
||||||
|
|
||||||
def test_chat_start_returns_stream_id(cleanup_test_sessions):
|
def test_chat_start_returns_stream_id(cleanup_test_sessions):
|
||||||
@@ -764,3 +774,100 @@ def test_reload_recovery_persists_durable_inflight_state(cleanup_test_sessions):
|
|||||||
"messages.js must clear durable inflight snapshots when the run ends/errors/cancels"
|
"messages.js must clear durable inflight snapshots when the run ends/errors/cancels"
|
||||||
assert "const stored=loadInflightState(sid, activeStreamId);" in sessions_src, \
|
assert "const stored=loadInflightState(sid, activeStreamId);" in sessions_src, \
|
||||||
"loadSession() must hydrate in-flight state from durable browser storage on reload"
|
"loadSession() must hydrate in-flight state from durable browser storage on reload"
|
||||||
|
|
||||||
|
|
||||||
|
# ── R18: OAuth onboarding must recognize credential_pool-only auth ───────────
|
||||||
|
|
||||||
|
def test_provider_oauth_authenticated_accepts_credential_pool_entries(
|
||||||
|
cleanup_test_sessions, tmp_path
|
||||||
|
):
|
||||||
|
"""R18a: pool-only OAuth auth.json should count as authenticated.
|
||||||
|
|
||||||
|
Hermes runtime resolves Codex credentials from credential_pool; onboarding
|
||||||
|
must not insist on stale or duplicated providers[provider_id] entries.
|
||||||
|
"""
|
||||||
|
_make_auth_json_with_credential_pool(
|
||||||
|
"openai-codex",
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"id": "pool1",
|
||||||
|
"label": "device_code",
|
||||||
|
"source": "device_code",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"access_token": "***",
|
||||||
|
"refresh_token": "***",
|
||||||
|
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
tmp_path,
|
||||||
|
)
|
||||||
|
|
||||||
|
from api.onboarding import _provider_oauth_authenticated
|
||||||
|
|
||||||
|
assert _provider_oauth_authenticated("openai-codex", tmp_path) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_provider_oauth_authenticated_rejects_flag_only_credential_pool_entries(
|
||||||
|
cleanup_test_sessions, tmp_path
|
||||||
|
):
|
||||||
|
"""R18a2: metadata flags alone must not count as usable OAuth auth."""
|
||||||
|
_make_auth_json_with_credential_pool(
|
||||||
|
"openai-codex",
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"id": "pool1",
|
||||||
|
"label": "device_code",
|
||||||
|
"source": "device_code",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"has_access_token": True,
|
||||||
|
"has_refresh_token": True,
|
||||||
|
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
tmp_path,
|
||||||
|
)
|
||||||
|
|
||||||
|
from api.onboarding import _provider_oauth_authenticated
|
||||||
|
|
||||||
|
assert _provider_oauth_authenticated("openai-codex", tmp_path) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_status_from_runtime_marks_openai_codex_ready_from_credential_pool(
|
||||||
|
cleanup_test_sessions, tmp_path
|
||||||
|
):
|
||||||
|
"""R18b: provider_ready should be true when auth lives only in credential_pool."""
|
||||||
|
_make_auth_json_with_credential_pool(
|
||||||
|
"openai-codex",
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"id": "pool1",
|
||||||
|
"label": "device_code",
|
||||||
|
"source": "device_code",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"access_token": "***",
|
||||||
|
"refresh_token": "***",
|
||||||
|
"base_url": "https://chatgpt.com/backend-api/codex",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
tmp_path,
|
||||||
|
)
|
||||||
|
|
||||||
|
from api.onboarding import _status_from_runtime
|
||||||
|
import api.onboarding as _ob
|
||||||
|
|
||||||
|
orig_home = _ob._get_active_hermes_home
|
||||||
|
orig_found = _ob._HERMES_FOUND
|
||||||
|
_ob._get_active_hermes_home = lambda: tmp_path
|
||||||
|
_ob._HERMES_FOUND = True
|
||||||
|
try:
|
||||||
|
result = _status_from_runtime(
|
||||||
|
{"model": {"provider": "openai-codex", "default": "codex-mini-latest"}},
|
||||||
|
True,
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
_ob._get_active_hermes_home = orig_home
|
||||||
|
_ob._HERMES_FOUND = orig_found
|
||||||
|
|
||||||
|
assert result["provider_configured"] is True
|
||||||
|
assert result["provider_ready"] is True
|
||||||
|
assert result["setup_state"] == "ready"
|
||||||
|
|||||||
@@ -35,6 +35,20 @@ def _make_auth_json(provider_id: str, tokens: dict, tmp_dir: pathlib.Path) -> pa
|
|||||||
return auth_path
|
return auth_path
|
||||||
|
|
||||||
|
|
||||||
|
def _make_auth_json_with_credential_pool(
|
||||||
|
provider_id: str, pool_entries: list[dict], tmp_dir: pathlib.Path
|
||||||
|
) -> pathlib.Path:
|
||||||
|
"""Write an auth.json with only credential_pool entries for provider_id.
|
||||||
|
|
||||||
|
This reproduces setups where Hermes runtime resolves OAuth credentials from
|
||||||
|
credential_pool while providers[provider_id] is absent or stale.
|
||||||
|
"""
|
||||||
|
store = {"providers": {}, "credential_pool": {provider_id: pool_entries}}
|
||||||
|
auth_path = tmp_dir / "auth.json"
|
||||||
|
auth_path.write_text(json.dumps(store), encoding="utf-8")
|
||||||
|
return auth_path
|
||||||
|
|
||||||
|
|
||||||
# ── 1–3. _provider_oauth_authenticated unit tests ────────────────────────────
|
# ── 1–3. _provider_oauth_authenticated unit tests ────────────────────────────
|
||||||
|
|
||||||
class TestProviderOAuthAuthenticated:
|
class TestProviderOAuthAuthenticated:
|
||||||
@@ -62,7 +76,7 @@ class TestProviderOAuthAuthenticated:
|
|||||||
"""openai-codex with only a refresh_token -> still authenticated."""
|
"""openai-codex with only a refresh_token -> still authenticated."""
|
||||||
_make_auth_json(
|
_make_auth_json(
|
||||||
"openai-codex",
|
"openai-codex",
|
||||||
{"access_token": "", "refresh_token": "ref_only_token"},
|
{"access_token": "", "refresh_token": "***"},
|
||||||
tmp_path,
|
tmp_path,
|
||||||
)
|
)
|
||||||
assert self._call("openai-codex", tmp_path) is True
|
assert self._call("openai-codex", tmp_path) is True
|
||||||
@@ -141,7 +155,7 @@ class TestStatusFromRuntimeOAuth:
|
|||||||
"""openai-codex configured + access_token -> provider_ready True."""
|
"""openai-codex configured + access_token -> provider_ready True."""
|
||||||
_make_auth_json(
|
_make_auth_json(
|
||||||
"openai-codex",
|
"openai-codex",
|
||||||
{"access_token": "ey.test", "refresh_token": "ref"},
|
{"access_token": "***", "refresh_token": "***"},
|
||||||
tmp_path,
|
tmp_path,
|
||||||
)
|
)
|
||||||
result = self._call("openai-codex", "codex-mini-latest", tmp_path)
|
result = self._call("openai-codex", "codex-mini-latest", tmp_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user