Require auth managers to receive routing configuration (#34650)

## Why

Auth managers should use the application's resolved HTTP client factory instead
of silently falling back to the transport's default proxy behavior.

## What changed

- Make `AuthRouteConfig` required when constructing an `AuthManager` or
  `AuthConfig`.
- Pass each production caller's resolved routing configuration through without
  wrapping it in an optional value.
- Add a test helper that explicitly selects the transport-default proxy policy
  for callers that do not exercise custom routing.

GitOrigin-RevId: d89a3b1f8b5d4007650cdac0aae241c94d598580
This commit is contained in:
Michael Bolin
2026-07-22 01:35:06 +00:00
committed by copyberry
parent f899a79c03
commit a26bc337cf
19 changed files with 75 additions and 53 deletions
@@ -1045,7 +1045,7 @@ async fn remote_control_start_allows_missing_auth_when_enabled() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let (transport_event_tx, _transport_event_rx) =
@@ -1895,7 +1895,7 @@ async fn remote_control_waits_for_account_id_before_enrolling() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let expected_server_name = gethostname().to_string_lossy().trim().to_string();
@@ -1993,7 +1993,7 @@ async fn persisted_enable_does_not_follow_auth_to_an_account_without_a_preferenc
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let remote_control_target =
@@ -205,7 +205,7 @@ async fn list_remote_control_clients_recovers_auth_after_unauthorized() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut fresh_auth = remote_control_auth_dot_json(Some("account_id"));
@@ -303,7 +303,7 @@ async fn list_remote_control_clients_retries_unauthorized_only_once() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut fresh_auth = remote_control_auth_dot_json(Some("account_id"));
@@ -49,7 +49,7 @@ async fn auth_manager_with_replacement(
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut replacement_auth = remote_control_auth_dot_json(Some(replacement_account_id));
@@ -1087,7 +1087,7 @@ async fn remote_control_handle_discards_pairing_response_after_auth_change() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let remote_handle =
@@ -2228,7 +2228,7 @@ mod tests {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut auth_recovery = auth_manager.unauthorized_recovery();
@@ -2327,7 +2327,7 @@ mod tests {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut auth_recovery = auth_manager.unauthorized_recovery();
@@ -2455,7 +2455,7 @@ mod tests {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let mut auth_recovery = auth_manager.unauthorized_recovery();
+1 -1
View File
@@ -74,7 +74,7 @@ pub async fn cloud_config_bundle_loader_for_storage(
/*forced_chatgpt_workspace_id*/ None,
Some(chatgpt_base_url.clone()),
keyring_backend_kind,
Some(auth_route_config),
auth_route_config,
)
.await;
cloud_config_bundle_loader(
+6 -6
View File
@@ -56,7 +56,7 @@ async fn auth_manager_with_api_key() -> Arc<AuthManager> {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
)
@@ -87,7 +87,7 @@ async fn auth_manager_with_plan_and_identity(
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
)
@@ -704,7 +704,7 @@ async fn get_bundle_recovers_after_unauthorized_reload() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
);
@@ -761,7 +761,7 @@ async fn get_bundle_recovers_after_unauthorized_reload_updates_cache_identity()
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
);
@@ -826,7 +826,7 @@ async fn get_bundle_surfaces_auth_recovery_message() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
);
@@ -880,7 +880,7 @@ async fn get_bundle_refreshes_external_auth_after_unauthorized() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
);
+1 -1
View File
@@ -65,7 +65,7 @@ pub async fn load_auth_manager(
config.forced_chatgpt_workspace_id.clone(),
chatgpt_base_url.or(Some(config.chatgpt_base_url.clone())),
config.auth_keyring_backend_kind(),
Some(config.auth_route_config()),
config.auth_route_config(),
)
.await;
(Some(auth_manager), http_client_factory)
+1 -1
View File
@@ -328,7 +328,7 @@ async fn chatgpt_auth_manager(
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let auth = auth_manager.auth().await.expect("auth should load");
+2 -2
View File
@@ -1250,8 +1250,8 @@ impl AuthManagerConfig for Config {
self.chatgpt_base_url.clone()
}
fn auth_route_config(&self) -> Option<AuthRouteConfig> {
Some(Config::auth_route_config(self))
fn auth_route_config(&self) -> AuthRouteConfig {
Config::auth_route_config(self)
}
}
+1 -1
View File
@@ -491,7 +491,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
forced_login_method: config.forced_login_method,
forced_chatgpt_workspace_id: config.forced_chatgpt_workspace_id.clone(),
chatgpt_base_url: Some(config.chatgpt_base_url.clone()),
auth_route_config: Some(auth_route_config),
auth_route_config,
})
.await
{
+8 -8
View File
@@ -933,7 +933,7 @@ async fn unauthorized_recovery_reports_mode_and_step_names() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
let managed = UnauthorizedRecovery {
@@ -1123,7 +1123,7 @@ async fn external_auth_provider_can_install_headers() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
@@ -1344,7 +1344,7 @@ async fn build_config(
forced_login_method,
forced_chatgpt_workspace_id,
chatgpt_base_url: None,
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
}
}
@@ -1529,7 +1529,7 @@ async fn auth_manager_rejects_env_personal_access_token_workspace_mismatch() {
Some(vec![WORKSPACE_ID_ALLOWED.to_string()]),
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
@@ -1581,7 +1581,7 @@ async fn auth_manager_rejects_stored_personal_access_token_workspace_mismatch()
Some(vec![WORKSPACE_ID_ALLOWED.to_string()]),
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
@@ -1615,7 +1615,7 @@ async fn personal_access_token_does_not_offer_unauthorized_recovery() {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await,
);
@@ -1757,7 +1757,7 @@ async fn enforce_login_restrictions_logs_out_for_personal_access_token_workspace
forced_login_method: None,
forced_chatgpt_workspace_id: Some(vec![WORKSPACE_ID_ALLOWED.to_string()]),
chatgpt_base_url: None,
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
};
let err = super::enforce_login_restrictions(&config)
@@ -1881,7 +1881,7 @@ async fn enforce_login_restrictions_logs_out_for_agent_identity_workspace_mismat
forced_login_method: None,
forced_chatgpt_workspace_id: Some(vec![WORKSPACE_ID_ALLOWED.to_string()]),
chatgpt_base_url: Some(chatgpt_base_url),
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
};
let err = super::enforce_login_restrictions_with_agent_identity_authapi_base_url(
@@ -63,7 +63,7 @@ async fn login_with_bedrock_api_key_replaces_openai_auth() -> anyhow::Result<()>
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
@@ -113,7 +113,7 @@ async fn logout_removes_bedrock_auth() -> anyhow::Result<()> {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
@@ -138,7 +138,7 @@ async fn bedrock_only_auth_storage_creates_primary_auth() -> anyhow::Result<()>
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
crate::test_support::transport_default_auth_route_config(),
)
.await;
+21 -15
View File
@@ -58,6 +58,8 @@ use crate::token_data::parse_chatgpt_jwt_claims;
use crate::token_data::parse_jwt_expiration;
use codex_config::types::AuthCredentialsStoreMode;
use codex_http_client::HttpClient;
use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
use codex_protocol::account::PlanType as AccountPlanType;
use codex_protocol::auth::PlanType as InternalPlanType;
use codex_protocol::auth::RefreshTokenFailedError;
@@ -1045,7 +1047,7 @@ pub struct AuthConfig {
pub forced_login_method: Option<ForcedLoginMethod>,
pub chatgpt_base_url: Option<String>,
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
pub auth_route_config: Option<AuthRouteConfig>,
pub auth_route_config: AuthRouteConfig,
}
/// Enforces configured login restrictions using auth-owned HTTP settings.
@@ -1071,7 +1073,7 @@ async fn enforce_login_restrictions_with_agent_identity_authapi_base_url(
config.chatgpt_base_url.as_deref(),
config.keyring_backend_kind,
agent_identity_authapi_base_url,
config.auth_route_config.as_ref(),
Some(&config.auth_route_config),
)
.await?
else {
@@ -1778,7 +1780,7 @@ pub struct AuthManager {
agent_identity_lock: Semaphore,
agent_identity_bootstrap_cooldown: Mutex<AgentIdentityBootstrapCooldown>,
external_auth: RwLock<Option<Arc<dyn ExternalAuth>>>,
auth_route_config: Option<AuthRouteConfig>,
auth_route_config: AuthRouteConfig,
}
/// Configuration view required to construct a shared [`AuthManager`].
@@ -1804,7 +1806,7 @@ pub trait AuthManagerConfig {
fn chatgpt_base_url(&self) -> String;
/// Returns route-selection settings for auth-owned clients.
fn auth_route_config(&self) -> Option<AuthRouteConfig>;
fn auth_route_config(&self) -> AuthRouteConfig;
}
impl Debug for AuthManager {
@@ -1845,7 +1847,7 @@ impl AuthManager {
forced_chatgpt_workspace_id: Option<Vec<String>>,
chatgpt_base_url: Option<String>,
keyring_backend_kind: AuthKeyringBackendKind,
auth_route_config: Option<AuthRouteConfig>,
auth_route_config: AuthRouteConfig,
) -> Self {
let agent_identity_authapi_base_url =
agent_identity_authapi_base_url(chatgpt_base_url.as_deref()).ok();
@@ -1857,7 +1859,7 @@ impl AuthManager {
chatgpt_base_url.as_deref(),
keyring_backend_kind,
agent_identity_authapi_base_url.as_deref(),
auth_route_config.as_ref(),
Some(&auth_route_config),
)
.await
.ok()
@@ -1906,7 +1908,7 @@ impl AuthManager {
agent_identity_lock: Semaphore::new(/*permits*/ 1),
agent_identity_bootstrap_cooldown: Mutex::default(),
external_auth: RwLock::new(None),
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
})
}
@@ -1931,7 +1933,7 @@ impl AuthManager {
agent_identity_lock: Semaphore::new(/*permits*/ 1),
agent_identity_bootstrap_cooldown: Mutex::default(),
external_auth: RwLock::new(None),
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
})
}
@@ -1964,7 +1966,7 @@ impl AuthManager {
agent_identity_lock: Semaphore::new(/*permits*/ 1),
agent_identity_bootstrap_cooldown: Mutex::default(),
external_auth: RwLock::new(None),
auth_route_config: None,
auth_route_config: crate::test_support::transport_default_auth_route_config(),
})
}
@@ -1989,7 +1991,11 @@ impl AuthManager {
external_auth: RwLock::new(Some(
Arc::new(BearerTokenRefresher::new(config)) as Arc<dyn ExternalAuth>
)),
auth_route_config: None,
// External bearer auth refreshes by running the provider's command and never makes
// auth-owned HTTP requests, so this route is intentionally inert.
auth_route_config: AuthRouteConfig::from_http_client_factory(HttpClientFactory::new(
OutboundProxyPolicy::ReqwestDefault,
)),
})
}
@@ -2074,7 +2080,7 @@ impl AuthManager {
policy,
self.agent_identity_authapi_base_url.as_deref(),
forced_chatgpt_workspace_id,
self.auth_route_config.as_ref(),
Some(&self.auth_route_config),
session_source,
)
.await;
@@ -2093,7 +2099,7 @@ impl AuthManager {
policy,
self.agent_identity_authapi_base_url.as_deref(),
self.forced_chatgpt_workspace_id(),
self.auth_route_config.as_ref(),
Some(&self.auth_route_config),
session_source,
)
.await
@@ -2212,7 +2218,7 @@ impl AuthManager {
self.chatgpt_base_url.as_deref(),
self.keyring_backend_kind,
self.agent_identity_authapi_base_url.as_deref(),
self.auth_route_config.as_ref(),
Some(&self.auth_route_config),
)
.await
.ok()
@@ -2295,7 +2301,7 @@ impl AuthManager {
forced_chatgpt_workspace_id: Option<Vec<String>>,
chatgpt_base_url: Option<String>,
keyring_backend_kind: AuthKeyringBackendKind,
auth_route_config: Option<AuthRouteConfig>,
auth_route_config: AuthRouteConfig,
) -> Arc<Self> {
Arc::new(
Self::new(
@@ -2473,7 +2479,7 @@ impl AuthManager {
.auth_cached()
.and_then(|auth| auth.get_current_auth_json());
if let Err(err) =
revoke_auth_tokens(auth_dot_json.as_ref(), self.auth_route_config.as_ref()).await
revoke_auth_tokens(auth_dot_json.as_ref(), Some(&self.auth_route_config)).await
{
tracing::warn!("failed to revoke auth tokens during logout: {err}");
}
+1
View File
@@ -1,5 +1,6 @@
pub mod auth;
pub mod auth_env_telemetry;
pub mod test_support;
pub mod token_data;
mod device_code_auth;
+15
View File
@@ -0,0 +1,15 @@
//! Test-only helpers exposed for cross-crate integration tests.
//!
//! Production code should receive an [`AuthRouteConfig`](crate::AuthRouteConfig) adapted from the
//! application's resolved HTTP client factory instead of depending on this module.
use crate::AuthRouteConfig;
use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
/// Returns auth routing that preserves the transport's built-in proxy behavior.
pub fn transport_default_auth_route_config() -> AuthRouteConfig {
AuthRouteConfig::from_http_client_factory(HttpClientFactory::new(
OutboundProxyPolicy::ReqwestDefault,
))
}
+1 -1
View File
@@ -1220,7 +1220,7 @@ impl RefreshTokenTestContext {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
+1 -1
View File
@@ -207,7 +207,7 @@ async fn auth_manager_logout_with_revoke_uses_cached_auth() -> Result<()> {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
save_auth(
+2 -2
View File
@@ -412,7 +412,7 @@ mod tests {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await;
let auth = auth_manager.auth().await.expect("auth should load");
@@ -497,7 +497,7 @@ mod tests {
/*forced_chatgpt_workspace_id*/ None,
/*chatgpt_base_url*/ None,
AuthKeyringBackendKind::default(),
/*auth_route_config*/ None,
codex_login::test_support::transport_default_auth_route_config(),
)
.await,
);
+1 -1
View File
@@ -1197,7 +1197,7 @@ pub async fn run_main(
forced_login_method: config.forced_login_method,
forced_chatgpt_workspace_id: config.forced_chatgpt_workspace_id.clone(),
chatgpt_base_url: Some(config.chatgpt_base_url.clone()),
auth_route_config: Some(auth_route_config),
auth_route_config,
})
.await
{