mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Fallback login callback port when default is busy (#19334)
## Summary - Keep the preferred ChatGPT login callback port `1455` first. - Preserve the existing `/cancel` recovery for stale Codex login servers. - Fall back to the registered localhost callback port `1457` when `1455` remains unavailable. ## Why Cursor and Codex Desktop both use the ChatGPT account login callback server. On Windows, Cursor can already be listening on `127.0.0.1:1455` / `[::1]:1455`, causing Codex Desktop sign-in to fail with: `Local callback port 1455 is already in use on this machine.` Codex already attempted to cancel a stale Codex login server on that port, but if the listener does not release the port, the old behavior was to fail. The new behavior falls back to `1457`, which matches the fixed redirect URI being registered server-side in `openai/openai#863817`. This keeps the OAuth `redirect_uri` inside Hydra's exact allow-list instead of choosing an arbitrary ephemeral port. ## Validation - `just fmt` - `cargo test -p codex-login` - `git diff --check HEAD~1..HEAD`
This commit is contained in:
@@ -50,6 +50,8 @@ use tracing::warn;
|
|||||||
|
|
||||||
const DEFAULT_ISSUER: &str = "https://auth.openai.com";
|
const DEFAULT_ISSUER: &str = "https://auth.openai.com";
|
||||||
const DEFAULT_PORT: u16 = 1455;
|
const DEFAULT_PORT: u16 = 1455;
|
||||||
|
// Keep in sync with the Codex CLI Hydra redirect URI allow-list.
|
||||||
|
const FALLBACK_PORT: u16 = 1457;
|
||||||
static LOGIN_ERROR_PAGE_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
|
static LOGIN_ERROR_PAGE_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
|
||||||
Template::parse(include_str!("assets/error.html"))
|
Template::parse(include_str!("assets/error.html"))
|
||||||
.unwrap_or_else(|err| panic!("login error page template must parse: {err}"))
|
.unwrap_or_else(|err| panic!("login error page template must parse: {err}"))
|
||||||
@@ -527,9 +529,12 @@ fn send_cancel_request(port: u16) -> io::Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn bind_server(port: u16) -> io::Result<Server> {
|
fn bind_server(port: u16) -> io::Result<Server> {
|
||||||
let bind_address = format!("127.0.0.1:{port}");
|
let preferred_bind_address = format!("127.0.0.1:{port}");
|
||||||
|
let fallback_bind_address = format!("127.0.0.1:{FALLBACK_PORT}");
|
||||||
|
let mut bind_address = preferred_bind_address.clone();
|
||||||
let mut cancel_attempted = false;
|
let mut cancel_attempted = false;
|
||||||
let mut attempts = 0;
|
let mut attempts = 0;
|
||||||
|
let mut using_fallback_port = false;
|
||||||
const MAX_ATTEMPTS: u32 = 10;
|
const MAX_ATTEMPTS: u32 = 10;
|
||||||
const RETRY_DELAY: Duration = Duration::from_millis(200);
|
const RETRY_DELAY: Duration = Duration::from_millis(200);
|
||||||
|
|
||||||
@@ -543,10 +548,10 @@ fn bind_server(port: u16) -> io::Result<Server> {
|
|||||||
.map(|io_err| io_err.kind() == io::ErrorKind::AddrInUse)
|
.map(|io_err| io_err.kind() == io::ErrorKind::AddrInUse)
|
||||||
.unwrap_or(false);
|
.unwrap_or(false);
|
||||||
|
|
||||||
// If the address is in use, there is probably another instance of the login server
|
// If the address is in use, there may be another instance of the login server
|
||||||
// running. Attempt to cancel it and retry.
|
// running. Attempt to cancel it and retry before falling back.
|
||||||
if is_addr_in_use {
|
if is_addr_in_use {
|
||||||
if !cancel_attempted {
|
if !cancel_attempted && !using_fallback_port {
|
||||||
cancel_attempted = true;
|
cancel_attempted = true;
|
||||||
if let Err(cancel_err) = send_cancel_request(port) {
|
if let Err(cancel_err) = send_cancel_request(port) {
|
||||||
eprintln!("Failed to cancel previous login server: {cancel_err}");
|
eprintln!("Failed to cancel previous login server: {cancel_err}");
|
||||||
@@ -556,6 +561,18 @@ fn bind_server(port: u16) -> io::Result<Server> {
|
|||||||
thread::sleep(RETRY_DELAY);
|
thread::sleep(RETRY_DELAY);
|
||||||
|
|
||||||
if attempts >= MAX_ATTEMPTS {
|
if attempts >= MAX_ATTEMPTS {
|
||||||
|
if port == DEFAULT_PORT && !using_fallback_port {
|
||||||
|
warn!(
|
||||||
|
%preferred_bind_address,
|
||||||
|
%fallback_bind_address,
|
||||||
|
"default login callback port is unavailable; falling back to the registered fallback port"
|
||||||
|
);
|
||||||
|
bind_address = fallback_bind_address.clone();
|
||||||
|
attempts = 0;
|
||||||
|
using_fallback_port = true;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
return Err(io::Error::new(
|
return Err(io::Error::new(
|
||||||
io::ErrorKind::AddrInUse,
|
io::ErrorKind::AddrInUse,
|
||||||
format!("Port {bind_address} is already in use"),
|
format!("Port {bind_address} is already in use"),
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
use std::io;
|
use std::io;
|
||||||
use std::net::SocketAddr;
|
use std::net::SocketAddr;
|
||||||
use std::net::TcpListener;
|
use std::net::TcpListener;
|
||||||
|
use std::sync::Arc;
|
||||||
use std::thread;
|
use std::thread;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
@@ -13,6 +14,9 @@ use codex_login::run_login_server;
|
|||||||
use core_test_support::skip_if_no_network;
|
use core_test_support::skip_if_no_network;
|
||||||
use tempfile::tempdir;
|
use tempfile::tempdir;
|
||||||
|
|
||||||
|
const DEFAULT_LOGIN_PORT: u16 = 1455;
|
||||||
|
const FALLBACK_LOGIN_PORT: u16 = 1457;
|
||||||
|
|
||||||
// See spawn.rs for details
|
// See spawn.rs for details
|
||||||
|
|
||||||
fn start_mock_issuer(chatgpt_account_id: &str) -> (SocketAddr, thread::JoinHandle<()>) {
|
fn start_mock_issuer(chatgpt_account_id: &str) -> (SocketAddr, thread::JoinHandle<()>) {
|
||||||
@@ -401,6 +405,72 @@ async fn oauth_access_denied_unknown_reason_uses_generic_error_page() -> Result<
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||||
|
async fn falls_back_to_registered_fallback_port_when_default_port_is_in_use() -> Result<()> {
|
||||||
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
|
match TcpListener::bind(("127.0.0.1", FALLBACK_LOGIN_PORT)) {
|
||||||
|
Ok(listener) => drop(listener),
|
||||||
|
Err(err) if err.kind() == io::ErrorKind::AddrInUse => {
|
||||||
|
eprintln!("Skipping test because 127.0.0.1:{FALLBACK_LOGIN_PORT} is already in use");
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
Err(err) => return Err(err.into()),
|
||||||
|
}
|
||||||
|
|
||||||
|
let default_port_listener = match TcpListener::bind(("127.0.0.1", DEFAULT_LOGIN_PORT)) {
|
||||||
|
Ok(listener) => listener,
|
||||||
|
Err(err) if err.kind() == io::ErrorKind::AddrInUse => {
|
||||||
|
eprintln!("Skipping test because 127.0.0.1:{DEFAULT_LOGIN_PORT} is already in use");
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
Err(err) => return Err(err.into()),
|
||||||
|
};
|
||||||
|
let default_port_server =
|
||||||
|
Arc::new(tiny_http::Server::from_listener(default_port_listener, None).unwrap());
|
||||||
|
let default_port_server_handle = {
|
||||||
|
let server = default_port_server.clone();
|
||||||
|
thread::spawn(move || {
|
||||||
|
while let Ok(req) = server.recv() {
|
||||||
|
let _ = req.respond(tiny_http::Response::from_string("not codex"));
|
||||||
|
}
|
||||||
|
})
|
||||||
|
};
|
||||||
|
|
||||||
|
let (issuer_addr, _issuer_handle) = start_mock_issuer("org-123");
|
||||||
|
let issuer = format!("http://{}:{}", issuer_addr.ip(), issuer_addr.port());
|
||||||
|
let tmp = tempdir()?;
|
||||||
|
|
||||||
|
let mut opts = ServerOptions::new(
|
||||||
|
tmp.path().to_path_buf(),
|
||||||
|
codex_login::CLIENT_ID.to_string(),
|
||||||
|
/*forced_chatgpt_workspace_id*/ None,
|
||||||
|
AuthCredentialsStoreMode::File,
|
||||||
|
);
|
||||||
|
opts.issuer = issuer;
|
||||||
|
opts.open_browser = false;
|
||||||
|
opts.force_state = Some("fallback_state".to_string());
|
||||||
|
|
||||||
|
let server_result = run_login_server(opts);
|
||||||
|
default_port_server.unblock();
|
||||||
|
let _ = default_port_server_handle.join();
|
||||||
|
|
||||||
|
let server = server_result?;
|
||||||
|
let actual_port = server.actual_port;
|
||||||
|
let auth_url = server.auth_url.clone();
|
||||||
|
server.cancel();
|
||||||
|
let _ = tokio::time::timeout(Duration::from_secs(2), server.block_until_done())
|
||||||
|
.await
|
||||||
|
.expect("login server should shut down after cancel");
|
||||||
|
|
||||||
|
assert_eq!(actual_port, FALLBACK_LOGIN_PORT);
|
||||||
|
assert!(auth_url.contains(&format!(
|
||||||
|
"redirect_uri=http%3A%2F%2Flocalhost%3A{FALLBACK_LOGIN_PORT}%2Fauth%2Fcallback"
|
||||||
|
)));
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||||
async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> {
|
async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|||||||
Reference in New Issue
Block a user