mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
codex: restore fixed create-api-key callback port
Use the Hydra-registered localhost:5000 redirect URI for project API key creation, but fail fast on port conflicts instead of sending /cancel to a potentially unrelated local service. Validation: - cargo test -p codex-login - just fmt - just fix -p codex-login - just argument-comment-lint Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -9,6 +9,7 @@ use serde::Deserialize;
|
||||
use url::Url;
|
||||
|
||||
use crate::oauth_callback_server::AuthorizationCodeServer;
|
||||
use crate::oauth_callback_server::PortConflictStrategy;
|
||||
use crate::oauth_callback_server::start_authorization_code_server;
|
||||
use crate::pkce::PkceCodes;
|
||||
|
||||
@@ -16,7 +17,9 @@ const AUTH_ISSUER: &str = "https://auth.openai.com";
|
||||
const PLATFORM_HYDRA_CLIENT_ID: &str = "app_2SKx67EdpoN0G6j64rFvigXD";
|
||||
const PLATFORM_AUDIENCE: &str = "https://api.openai.com/v1";
|
||||
const API_BASE: &str = "https://api.openai.com";
|
||||
const CALLBACK_PORT: u16 = 0;
|
||||
// This client is registered with Hydra for http://localhost:5000/auth/callback,
|
||||
// so the browser redirect must stay on port 5000.
|
||||
const CALLBACK_PORT: u16 = 5000;
|
||||
const CALLBACK_PATH: &str = "/auth/callback";
|
||||
const SCOPE: &str = "openid email profile offline_access";
|
||||
const APP: &str = "api";
|
||||
@@ -104,6 +107,7 @@ pub fn start_create_api_key() -> Result<PendingCreateApiKey, CreateApiKeyError>
|
||||
let client = build_http_client()?;
|
||||
let callback_server = start_authorization_code_server(
|
||||
options.callback_port,
|
||||
PortConflictStrategy::Fail,
|
||||
CALLBACK_PATH,
|
||||
/*force_state*/ None,
|
||||
|redirect_uri, pkce, state| {
|
||||
|
||||
@@ -24,6 +24,15 @@ use tiny_http::StatusCode;
|
||||
use crate::pkce::PkceCodes;
|
||||
use crate::pkce::generate_pkce;
|
||||
|
||||
/// Strategy for handling a callback port that is already in use.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum PortConflictStrategy {
|
||||
/// Attempt to cancel a previous callback server on the same port and retry.
|
||||
CancelPrevious,
|
||||
/// Return an error immediately without sending any request to the occupied port.
|
||||
Fail,
|
||||
}
|
||||
|
||||
/// Handle used to signal the callback server loop to exit.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct ShutdownHandle {
|
||||
@@ -84,6 +93,7 @@ impl AuthorizationCodeServer {
|
||||
|
||||
pub(crate) fn start_authorization_code_server<F>(
|
||||
port: u16,
|
||||
port_conflict_strategy: PortConflictStrategy,
|
||||
callback_path: &str,
|
||||
force_state: Option<String>,
|
||||
auth_url_builder: F,
|
||||
@@ -95,7 +105,7 @@ where
|
||||
let state = force_state.unwrap_or_else(generate_state);
|
||||
let callback_path = callback_path.to_string();
|
||||
|
||||
let (server, actual_port, rx) = bind_server_with_request_channel(port)?;
|
||||
let (server, actual_port, rx) = bind_server_with_request_channel(port, port_conflict_strategy)?;
|
||||
let redirect_uri = format!("http://localhost:{actual_port}{callback_path}");
|
||||
let auth_url = match auth_url_builder(&redirect_uri, &pkce, &state) {
|
||||
Ok(auth_url) => auth_url,
|
||||
@@ -139,8 +149,9 @@ pub(crate) enum HandledRequest<T> {
|
||||
|
||||
pub(crate) fn bind_server_with_request_channel(
|
||||
port: u16,
|
||||
port_conflict_strategy: PortConflictStrategy,
|
||||
) -> io::Result<(Arc<Server>, u16, tokio::sync::mpsc::Receiver<Request>)> {
|
||||
let server = bind_server(port)?;
|
||||
let server = bind_server(port, port_conflict_strategy)?;
|
||||
let actual_port = match server.server_addr().to_ip() {
|
||||
Some(addr) => addr.port(),
|
||||
None => {
|
||||
@@ -311,7 +322,7 @@ fn send_cancel_request(port: u16) -> io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn bind_server(port: u16) -> io::Result<Server> {
|
||||
fn bind_server(port: u16, port_conflict_strategy: PortConflictStrategy) -> io::Result<Server> {
|
||||
let bind_address = format!("127.0.0.1:{port}");
|
||||
let mut cancel_attempted = false;
|
||||
let mut attempts = 0;
|
||||
@@ -331,6 +342,13 @@ fn bind_server(port: u16) -> io::Result<Server> {
|
||||
// If the address is in use, there is probably another instance of the callback
|
||||
// server running. Attempt to cancel it and retry.
|
||||
if is_addr_in_use {
|
||||
if port_conflict_strategy == PortConflictStrategy::Fail {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::AddrInUse,
|
||||
format!("Port {bind_address} is already in use"),
|
||||
));
|
||||
}
|
||||
|
||||
if !cancel_attempted {
|
||||
cancel_attempted = true;
|
||||
if let Err(cancel_err) = send_cancel_request(port) {
|
||||
@@ -454,6 +472,20 @@ fn authorization_code_error_message(error_code: &str, error_description: Option<
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn bind_server_fails_without_canceling_when_port_conflict_strategy_is_fail() {
|
||||
let listener =
|
||||
std::net::TcpListener::bind("127.0.0.1:0").expect("bind ephemeral test listener");
|
||||
let port = listener.local_addr().expect("read local addr").port();
|
||||
|
||||
let error = match bind_server(port, PortConflictStrategy::Fail) {
|
||||
Ok(_) => panic!("expected occupied port to fail immediately"),
|
||||
Err(error) => error,
|
||||
};
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::AddrInUse);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn process_authorization_code_request_keeps_server_running_on_state_mismatch() {
|
||||
let response = process_authorization_code_request(
|
||||
|
||||
@@ -36,6 +36,7 @@ use tracing::info;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::oauth_callback_server::HandledRequest;
|
||||
use crate::oauth_callback_server::PortConflictStrategy;
|
||||
use crate::oauth_callback_server::ShutdownHandle;
|
||||
use crate::oauth_callback_server::bind_server_with_request_channel;
|
||||
use crate::oauth_callback_server::generate_state;
|
||||
@@ -110,7 +111,8 @@ pub fn run_login_server(opts: ServerOptions) -> io::Result<LoginServer> {
|
||||
let pkce = generate_pkce();
|
||||
let state = opts.force_state.clone().unwrap_or_else(generate_state);
|
||||
|
||||
let (server, actual_port, rx) = bind_server_with_request_channel(opts.port)?;
|
||||
let (server, actual_port, rx) =
|
||||
bind_server_with_request_channel(opts.port, PortConflictStrategy::CancelPrevious)?;
|
||||
let redirect_uri = format!("http://localhost:{actual_port}/auth/callback");
|
||||
let auth_url = build_authorize_url(
|
||||
&opts.issuer,
|
||||
|
||||
Reference in New Issue
Block a user