mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(xai-oauth): show "not received" page when loopback callback has no code
When xAI's auth backend fails to redirect (e.g. the German "We couldn't reach your app" fallback shown in #27385), users sometimes navigate manually to the bare loopback callback URL — `http://127.0.0.1:<port>/callback` with no query string. The handler used to return 200 "xAI authorization received" for any GET that hit the expected path, because `parse_qs("")` yields no `code` and no `error`, leaving `result` untouched while the success page was still served. The CLI's wait loop, of course, still saw no code and timed out with `AuthError: xAI authorization timed out waiting for the local callback.` The user is left looking at a browser tab that claims success and a terminal that says failure — exactly the contradiction in #27385. This change makes the empty-callback case return 400 with an explicit "not received" page and a hint to retry `hermes auth add xai-oauth`. The wait-loop semantics are unchanged: `result["code"]` and `result["error"]` both stay None, so the CLI still raises a real timeout rather than treating the bare hit as a successful callback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1fabd6e100
commit
bf6eeb3f93
2 changed files with 104 additions and 4 deletions
|
|
@ -2405,15 +2405,37 @@ def _make_xai_callback_handler(expected_path: str) -> tuple[type[BaseHTTPRequest
|
|||
"error": params.get("error", [None])[0],
|
||||
"error_description": params.get("error_description", [None])[0],
|
||||
}
|
||||
|
||||
# Treat a hit on the callback path with neither `code` nor `error`
|
||||
# as a missing OAuth callback (e.g. xAI's auth backend failed to
|
||||
# redirect and the user navigated to the bare loopback URL by hand).
|
||||
# Show an explicit "not received" page rather than the success page —
|
||||
# otherwise the browser claims authorization succeeded while the CLI
|
||||
# is still waiting for a real callback and eventually times out.
|
||||
if incoming["code"] is None and incoming["error"] is None:
|
||||
self.send_response(400)
|
||||
self._maybe_write_cors_headers()
|
||||
self.send_header("Content-Type", "text/html; charset=utf-8")
|
||||
self.end_headers()
|
||||
body = (
|
||||
"<html><body>"
|
||||
"<h1>xAI authorization not received.</h1>"
|
||||
"<p>No authorization code was present in this callback URL. "
|
||||
"Return to the terminal and re-run "
|
||||
"<code>hermes auth add xai-oauth</code> to retry.</p>"
|
||||
"</body></html>"
|
||||
)
|
||||
self.wfile.write(body.encode("utf-8"))
|
||||
return
|
||||
|
||||
# ThreadingHTTPServer allows a fallback/manual callback to complete
|
||||
# while a browser connection is stuck. Once we have a terminal
|
||||
# OAuth result (code or error), keep the first one so a later
|
||||
# concurrent/invalid callback cannot overwrite state before
|
||||
# validation in _xai_oauth_loopback_login().
|
||||
if incoming["code"] or incoming["error"]:
|
||||
with result_lock:
|
||||
if not (result["code"] or result["error"]):
|
||||
result.update(incoming)
|
||||
with result_lock:
|
||||
if not (result["code"] or result["error"]):
|
||||
result.update(incoming)
|
||||
|
||||
self.send_response(200)
|
||||
self._maybe_write_cors_headers()
|
||||
|
|
|
|||
|
|
@ -326,6 +326,84 @@ def test_xai_callback_server_latches_first_terminal_callback_result():
|
|||
thread.join(timeout=1.0)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Loopback callback handler GET responses
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _get_callback(redirect_uri: str, query: str = "") -> tuple[int, str]:
|
||||
"""GET the loopback callback URL with an optional query string."""
|
||||
from urllib.request import Request, urlopen
|
||||
from urllib.error import HTTPError
|
||||
|
||||
target = redirect_uri + (("?" + query) if query else "")
|
||||
req = Request(target, method="GET")
|
||||
try:
|
||||
with urlopen(req, timeout=5.0) as resp:
|
||||
return resp.getcode(), resp.read().decode("utf-8", "replace")
|
||||
except HTTPError as exc:
|
||||
return exc.code, exc.read().decode("utf-8", "replace")
|
||||
|
||||
|
||||
def test_xai_callback_handler_returns_400_when_callback_url_lacks_code_and_error():
|
||||
"""Bare loopback URL (no code, no error) must not claim authorization received.
|
||||
|
||||
Regression for #27385: when xAI's auth backend fails to redirect and the user
|
||||
manually navigates to http://127.0.0.1:<port>/callback, the handler used to
|
||||
return 200 "xAI authorization received" while the CLI's wait loop still timed
|
||||
out — leaving the user with a contradictory success page and a CLI error.
|
||||
"""
|
||||
server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0)
|
||||
try:
|
||||
status, body = _get_callback(redirect_uri)
|
||||
assert status == 400
|
||||
assert "not received" in body.lower()
|
||||
assert "hermes auth add xai-oauth" in body
|
||||
# Wait loop must still see no code/error so it raises a real timeout,
|
||||
# rather than treating this empty hit as a successful callback.
|
||||
assert result["code"] is None
|
||||
assert result["error"] is None
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
thread.join(timeout=1.0)
|
||||
|
||||
|
||||
def test_xai_callback_handler_accepts_callback_with_code():
|
||||
"""A real OAuth redirect (code + state) still records both and shows success."""
|
||||
server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0)
|
||||
try:
|
||||
status, body = _get_callback(redirect_uri, query="code=abc&state=xyz")
|
||||
assert status == 200
|
||||
assert "xAI authorization received" in body
|
||||
assert result["code"] == "abc"
|
||||
assert result["state"] == "xyz"
|
||||
assert result["error"] is None
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
thread.join(timeout=1.0)
|
||||
|
||||
|
||||
def test_xai_callback_handler_records_error_callback():
|
||||
"""A redirect carrying an `error` param must surface the failure page and capture detail."""
|
||||
server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0)
|
||||
try:
|
||||
status, body = _get_callback(
|
||||
redirect_uri,
|
||||
query="error=access_denied&error_description=user%20cancelled",
|
||||
)
|
||||
assert status == 200
|
||||
assert "xAI authorization failed" in body
|
||||
assert result["error"] == "access_denied"
|
||||
assert result["error_description"] == "user cancelled"
|
||||
assert result["code"] is None
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
thread.join(timeout=1.0)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Token roundtrip + reads
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue