mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Honor configured proxy routes for auth refreshes (#34655)
## Why ChatGPT token refresh requests need to follow the same configured routing policy as other authentication traffic, including environments that use the system proxy. ## What changed - Require an `AuthRouteConfig` throughout login, logout, token refresh, personal access token, and agent identity flows. - Build auth HTTP clients directly from that configuration instead of falling back to a default HTTP client when routing configuration is absent. ## Testing - Add an integration test that refreshes a token through a cached system-proxy route and verifies the proxy receives the expected request. GitOrigin-RevId: 7d54ab3219939a49921c51faf08cd4c2eabca51c
This commit is contained in:
1 parent
1823c13771
commit
d4fcb2873b
24 files changed
+281
-127
No files matched your search
@@ -4,6 +4,9 @@ use base64::Engine;
|
||||
use chrono::Duration;
|
||||
use chrono::Utc;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_http_client::cache_system_proxy_route_for_test;
|
||||
use codex_login::AuthDotJson;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::AuthManager;
|
||||
@@ -21,7 +24,10 @@ use pretty_assertions::assert_eq;
|
||||
use serde::Serialize;
|
||||
use serde_json::json;
|
||||
use std::ffi::OsString;
|
||||
use std::net::TcpListener;
|
||||
use std::process::Command;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration as StdDuration;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
@@ -31,6 +37,147 @@ use wiremock::matchers::path;
|
||||
|
||||
const INITIAL_ACCESS_TOKEN: &str = "initial-access-token";
|
||||
const INITIAL_REFRESH_TOKEN: &str = "initial-refresh-token";
|
||||
const SYSTEM_PROXY_TEST_ENDPOINT: &str = "http://auth-proxy.invalid/oauth/token";
|
||||
const SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_AUTH_SYSTEM_PROXY_TEST_SUBPROCESS";
|
||||
const SYSTEM_PROXY_TEST_PROXY_URL_ENV_VAR: &str = "CODEX_AUTH_SYSTEM_PROXY_TEST_PROXY_URL";
|
||||
const SYSTEM_PROXY_TEST_NAME: &str =
|
||||
"suite::auth_refresh::refresh_token_honors_respect_system_proxy";
|
||||
const PROXY_ENV_KEYS: [&str; 8] = [
|
||||
"HTTP_PROXY",
|
||||
"http_proxy",
|
||||
"HTTPS_PROXY",
|
||||
"https_proxy",
|
||||
"ALL_PROXY",
|
||||
"all_proxy",
|
||||
"NO_PROXY",
|
||||
"no_proxy",
|
||||
];
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_honors_respect_system_proxy() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if std::env::var_os(SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR).is_none() {
|
||||
let response_body =
|
||||
r#"{"access_token":"new-access-token","refresh_token":"new-refresh-token"}"#;
|
||||
let listener = TcpListener::bind(("127.0.0.1", 0))?;
|
||||
let proxy_address = listener.local_addr()?;
|
||||
let proxy = tiny_http::Server::from_listener(listener, None)
|
||||
.map_err(|error| anyhow::anyhow!("failed to start auth proxy: {error}"))?;
|
||||
let proxy_thread = std::thread::spawn(move || {
|
||||
let mut request = proxy
|
||||
.recv_timeout(StdDuration::from_secs(30))
|
||||
.expect("proxy should receive an auth refresh request")
|
||||
.expect("proxy should receive a request before the timeout");
|
||||
let request_line = format!("{} {} HTTP/1.1", request.method(), request.url());
|
||||
let mut request_body = String::new();
|
||||
request
|
||||
.as_reader()
|
||||
.read_to_string(&mut request_body)
|
||||
.expect("proxy should read request body");
|
||||
let content_type = tiny_http::Header::from_bytes(
|
||||
b"Content-Type".as_slice(),
|
||||
b"application/json".as_slice(),
|
||||
)
|
||||
.expect("content type header should be valid");
|
||||
request
|
||||
.respond(tiny_http::Response::from_string(response_body).with_header(content_type))
|
||||
.expect("proxy should write response");
|
||||
(request_line, request_body)
|
||||
});
|
||||
|
||||
let proxy_url = format!("http://{proxy_address}");
|
||||
let mut command = Command::new(std::env::current_exe()?);
|
||||
command.arg("--exact").arg(SYSTEM_PROXY_TEST_NAME);
|
||||
for key in PROXY_ENV_KEYS {
|
||||
command.env_remove(key);
|
||||
}
|
||||
command
|
||||
.env(SYSTEM_PROXY_TEST_SUBPROCESS_ENV_VAR, "1")
|
||||
.env(SYSTEM_PROXY_TEST_PROXY_URL_ENV_VAR, proxy_url)
|
||||
.env(CLIENT_ID_OVERRIDE_ENV_VAR, "staging-client")
|
||||
.env_remove(REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR);
|
||||
|
||||
let output = command.output()?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"subprocess test `{SYSTEM_PROXY_TEST_NAME}` failed\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
let (proxy_request_line, proxy_request_body) = proxy_thread
|
||||
.join()
|
||||
.expect("proxy thread should finish after the child test");
|
||||
assert_eq!(
|
||||
proxy_request_line,
|
||||
"POST http://auth-proxy.invalid/oauth/token HTTP/1.1"
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::from_str::<serde_json::Value>(&proxy_request_body)?,
|
||||
json!({
|
||||
"client_id": "staging-client",
|
||||
"grant_type": "refresh_token",
|
||||
"refresh_token": INITIAL_REFRESH_TOKEN,
|
||||
})
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
let proxy_url = std::env::var(SYSTEM_PROXY_TEST_PROXY_URL_ENV_VAR)
|
||||
.context("proxy URL should be set in the auth refresh test subprocess")?;
|
||||
cache_system_proxy_route_for_test(SYSTEM_PROXY_TEST_ENDPOINT, proxy_url);
|
||||
let _endpoint_guard = EnvGuard::set(
|
||||
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
|
||||
SYSTEM_PROXY_TEST_ENDPOINT.to_string(),
|
||||
);
|
||||
let auth_manager = AuthManager::shared(
|
||||
codex_home.path().to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
/*forced_chatgpt_workspace_id*/ None,
|
||||
/*chatgpt_base_url*/ None,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/
|
||||
codex_login::AuthRouteConfig::from_http_client_factory(HttpClientFactory::new(
|
||||
OutboundProxyPolicy::RespectSystemProxy,
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN);
|
||||
let initial_auth = AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(Utc::now() - Duration::days(1)),
|
||||
agent_identity: None,
|
||||
personal_access_token: None,
|
||||
bedrock_api_key: None,
|
||||
};
|
||||
save_auth(
|
||||
codex_home.path(),
|
||||
&initial_auth,
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
)?;
|
||||
auth_manager.reload().await;
|
||||
|
||||
auth_manager
|
||||
.refresh_token_from_authority()
|
||||
.await
|
||||
.context("refresh should succeed through the configured proxy")?;
|
||||
|
||||
let refreshed_auth = auth_manager.auth().await.context("auth should be cached")?;
|
||||
let expected_tokens = TokenData {
|
||||
access_token: "new-access-token".to_string(),
|
||||
refresh_token: "new-refresh-token".to_string(),
|
||||
..initial_tokens
|
||||
};
|
||||
assert_eq!(refreshed_auth.get_token_data()?, expected_tokens);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
|
||||
@@ -112,7 +112,7 @@ fn server_opts(
|
||||
/*forced_chatgpt_workspace_id*/ None,
|
||||
cli_auth_credentials_store_mode,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
codex_login::test_support::transport_default_auth_route_config(),
|
||||
);
|
||||
opts.issuer = issuer;
|
||||
opts.open_browser = false;
|
||||
@@ -278,7 +278,7 @@ async fn device_code_login_integration_persists_without_api_key_on_exchange_fail
|
||||
/*forced_chatgpt_workspace_id*/ None,
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
codex_login::test_support::transport_default_auth_route_config(),
|
||||
);
|
||||
opts.issuer = issuer;
|
||||
opts.open_browser = false;
|
||||
@@ -334,7 +334,7 @@ async fn device_code_login_integration_handles_error_payload() -> anyhow::Result
|
||||
/*forced_chatgpt_workspace_id*/ None,
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
codex_login::test_support::transport_default_auth_route_config(),
|
||||
);
|
||||
opts.issuer = issuer;
|
||||
opts.open_browser = false;
|
||||
|
||||
@@ -125,7 +125,7 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> {
|
||||
let opts = ServerOptions {
|
||||
codex_home: server_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -190,7 +190,7 @@ async fn hosted_login_redirects_to_configured_open_app_url() -> Result<()> {
|
||||
let server = run_login_server(ServerOptions {
|
||||
codex_home: tmp.path().to_path_buf(),
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -243,7 +243,7 @@ async fn creates_missing_codex_home_dir() -> Result<()> {
|
||||
let opts = ServerOptions {
|
||||
codex_home: server_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -286,7 +286,7 @@ async fn login_server_includes_forced_workspaces_as_one_query_param() -> Result<
|
||||
let opts = ServerOptions {
|
||||
codex_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -330,7 +330,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> {
|
||||
let opts = ServerOptions {
|
||||
codex_home: codex_home.clone(),
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -393,7 +393,7 @@ async fn oauth_access_denied_missing_entitlement_blocks_login_with_clear_error()
|
||||
let opts = ServerOptions {
|
||||
codex_home: codex_home.clone(),
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -464,7 +464,7 @@ async fn oauth_access_denied_unknown_reason_uses_generic_error_page() -> Result<
|
||||
let opts = ServerOptions {
|
||||
codex_home: codex_home.clone(),
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
@@ -575,7 +575,7 @@ async fn falls_back_to_registered_fallback_port_when_default_port_is_in_use() ->
|
||||
/*forced_chatgpt_workspace_id*/ None,
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
codex_login::test_support::transport_default_auth_route_config(),
|
||||
);
|
||||
opts.issuer = issuer;
|
||||
opts.open_browser = false;
|
||||
@@ -614,7 +614,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> {
|
||||
let first_opts = ServerOptions {
|
||||
codex_home: first_codex_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer: issuer.clone(),
|
||||
port: 0,
|
||||
@@ -638,7 +638,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> {
|
||||
let second_opts = ServerOptions {
|
||||
codex_home: second_codex_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
auth_route_config: None,
|
||||
auth_route_config: codex_login::test_support::transport_default_auth_route_config(),
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: login_port,
|
||||
|
||||
@@ -61,7 +61,7 @@ async fn logout_with_revoke_revokes_refresh_token_then_removes_auth() -> Result<
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
&codex_login::test_support::transport_default_auth_route_config(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -120,7 +120,7 @@ async fn logout_with_revoke_uses_stored_auth_when_access_token_env_is_set() -> R
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
&codex_login::test_support::transport_default_auth_route_config(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -163,7 +163,7 @@ async fn logout_with_revoke_removes_auth_when_revoke_fails() -> Result<()> {
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
AuthKeyringBackendKind::default(),
|
||||
/*auth_route_config*/ None,
|
||||
&codex_login::test_support::transport_default_auth_route_config(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
Reference in new issue
Block a user