mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
[codex] fix WIF optional auth routing [ci changed_files]
This commit is contained in:
@@ -176,6 +176,7 @@ impl AppsRequestProcessor {
|
||||
mcp_manager,
|
||||
plugins_manager,
|
||||
} = load_context;
|
||||
plugins_manager.set_auth_mode(auth.as_ref().map(CodexAuth::api_auth_mode));
|
||||
let AppsListParams {
|
||||
cursor,
|
||||
limit,
|
||||
@@ -194,19 +195,21 @@ impl AppsRequestProcessor {
|
||||
.plugins_for_config(&config.plugins_config_input())
|
||||
.await
|
||||
.effective_apps();
|
||||
let (mut accessible_connectors, mut all_connectors) = tokio::join!(
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(&config),
|
||||
connectors::list_cached_all_connectors(&config, auth.as_ref(), &plugin_apps)
|
||||
);
|
||||
let mut accessible_connectors =
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(&config, auth.as_ref());
|
||||
let mut all_connectors =
|
||||
connectors::list_cached_all_connectors(&config, auth.as_ref(), &plugin_apps).await;
|
||||
let cached_all_connectors = all_connectors.clone();
|
||||
|
||||
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
|
||||
let accessible_config = config.clone();
|
||||
let accessible_auth = auth.clone();
|
||||
let accessible_tx = tx.clone();
|
||||
tokio::spawn(async move {
|
||||
let result = connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
&accessible_config,
|
||||
accessible_auth.as_ref(),
|
||||
force_refetch,
|
||||
Arc::clone(&environment_manager),
|
||||
mcp_manager,
|
||||
|
||||
@@ -1730,6 +1730,7 @@ impl PluginRequestProcessor {
|
||||
),
|
||||
connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config,
|
||||
auth,
|
||||
/*force_refetch*/ true,
|
||||
Arc::clone(&environment_manager),
|
||||
self.thread_manager.mcp_manager(),
|
||||
@@ -1757,8 +1758,7 @@ impl PluginRequestProcessor {
|
||||
"failed to load accessible apps after plugin install: {err:#}"
|
||||
);
|
||||
(
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(config)
|
||||
.await
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(config, auth)
|
||||
.unwrap_or_default(),
|
||||
false,
|
||||
)
|
||||
|
||||
@@ -49,7 +49,7 @@ pub async fn list_connectors(
|
||||
}
|
||||
let (connectors_result, accessible_result) = tokio::join!(
|
||||
list_all_connectors(config, auth),
|
||||
list_accessible_connectors_from_mcp_tools(config),
|
||||
list_accessible_connectors_from_mcp_tools(config, auth),
|
||||
);
|
||||
let connectors = connectors_result?;
|
||||
let accessible = accessible_result?;
|
||||
|
||||
@@ -29,7 +29,6 @@ use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_features::Feature;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::default_client::originator;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
@@ -71,10 +70,11 @@ pub struct AccessibleConnectorsStatus {
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> anyhow::Result<Vec<AppInfo>> {
|
||||
Ok(
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
config, /*force_refetch*/ false,
|
||||
config, auth, /*force_refetch*/ false,
|
||||
)
|
||||
.await?
|
||||
.connectors,
|
||||
@@ -129,25 +129,17 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth(
|
||||
.collect())
|
||||
}
|
||||
|
||||
pub async fn list_cached_accessible_connectors_from_mcp_tools(
|
||||
pub fn list_cached_accessible_connectors_from_mcp_tools(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> Option<Vec<AppInfo>> {
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await;
|
||||
let auth = match auth_manager.auth().await {
|
||||
Ok(auth) => auth,
|
||||
Err(err) => {
|
||||
tracing::warn!(error = %err, "failed to resolve auth for cached connectors");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend))
|
||||
.apps_enabled_for_auth(auth.is_some_and(CodexAuth::uses_codex_backend))
|
||||
{
|
||||
return Some(Vec::new());
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
let cache_key = accessible_connectors_cache_key(config, auth);
|
||||
read_cached_accessible_connectors(&cache_key).map(|connectors| {
|
||||
codex_connectors::filter::filter_disallowed_connectors(
|
||||
connectors,
|
||||
@@ -175,17 +167,23 @@ pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_options(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
force_refetch: bool,
|
||||
) -> anyhow::Result<Vec<AppInfo>> {
|
||||
Ok(
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status(config, force_refetch)
|
||||
.await?
|
||||
.connectors,
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
config,
|
||||
auth,
|
||||
force_refetch,
|
||||
)
|
||||
.await?
|
||||
.connectors,
|
||||
)
|
||||
}
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
force_refetch: bool,
|
||||
) -> anyhow::Result<AccessibleConnectorsStatus> {
|
||||
// TODO: Wire callers that already own an EnvironmentManager into
|
||||
@@ -200,6 +198,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
.await?;
|
||||
list_accessible_connectors_from_mcp_tools_with_environment_manager(
|
||||
config,
|
||||
auth,
|
||||
force_refetch,
|
||||
Arc::new(environment_manager),
|
||||
)
|
||||
@@ -208,6 +207,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
force_refetch: bool,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
) -> anyhow::Result<AccessibleConnectorsStatus> {
|
||||
@@ -215,6 +215,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager(
|
||||
let mcp_manager = Arc::new(McpManager::new(plugins_manager));
|
||||
list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config,
|
||||
auth,
|
||||
force_refetch,
|
||||
environment_manager,
|
||||
mcp_manager,
|
||||
@@ -224,23 +225,21 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager(
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
force_refetch: bool,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
mcp_manager: Arc<McpManager>,
|
||||
) -> anyhow::Result<AccessibleConnectorsStatus> {
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await;
|
||||
let auth = auth_manager.auth().await?;
|
||||
if !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend))
|
||||
.apps_enabled_for_auth(auth.is_some_and(CodexAuth::uses_codex_backend))
|
||||
{
|
||||
return Ok(AccessibleConnectorsStatus {
|
||||
connectors: Vec::new(),
|
||||
codex_apps_ready: true,
|
||||
});
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
let cache_key = accessible_connectors_cache_key(config, auth);
|
||||
let mcp_config = mcp_manager.runtime_config(config).await;
|
||||
let tool_plugin_provenance = tool_plugin_provenance(&mcp_config);
|
||||
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
|
||||
@@ -256,9 +255,9 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
});
|
||||
}
|
||||
|
||||
let mut mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
|
||||
let mut mcp_servers = effective_mcp_servers(&mcp_config, auth);
|
||||
mcp_servers.retain(|name, _| name == CODEX_APPS_MCP_SERVER_NAME);
|
||||
let host_owned_codex_apps_enabled = host_owned_codex_apps_enabled(&mcp_config, auth.as_ref());
|
||||
let host_owned_codex_apps_enabled = host_owned_codex_apps_enabled(&mcp_config, auth);
|
||||
if mcp_servers.is_empty() {
|
||||
return Ok(AccessibleConnectorsStatus {
|
||||
connectors: Vec::new(),
|
||||
@@ -270,7 +269,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
mcp_servers.iter(),
|
||||
config.mcp_oauth_credentials_store_mode,
|
||||
config.auth_keyring_backend_kind(),
|
||||
auth.as_ref(),
|
||||
auth,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -292,12 +291,12 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
// one exists, but do not reintroduce the old hidden-local fallback.
|
||||
McpRuntimeContext::new(environment_manager, config.cwd.to_path_buf()),
|
||||
config.codex_home.to_path_buf(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
codex_apps_tools_cache_key(auth),
|
||||
host_owned_codex_apps_enabled,
|
||||
mcp_config.prefix_mcp_tool_names,
|
||||
mcp_config.client_elicitation_capability,
|
||||
ToolPluginProvenance::default(),
|
||||
auth.as_ref(),
|
||||
auth,
|
||||
/*elicitation_reviewer*/ None,
|
||||
)
|
||||
.await;
|
||||
@@ -458,21 +457,6 @@ async fn cached_directory_connectors_for_tool_suggest_with_auth(
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let loaded_auth;
|
||||
let auth = if let Some(auth) = auth {
|
||||
Some(auth)
|
||||
} else {
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await;
|
||||
loaded_auth = match auth_manager.auth().await {
|
||||
Ok(auth) => auth,
|
||||
Err(err) => {
|
||||
tracing::warn!(error = %err, "failed to resolve auth for connector directory cache");
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
loaded_auth.as_ref()
|
||||
};
|
||||
let Some(auth) = auth.filter(|auth| auth.uses_codex_backend()) else {
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
@@ -207,6 +207,51 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cached_accessible_connectors_require_caller_supplied_codex_auth() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("config should load");
|
||||
let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true);
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
let tools = vec![codex_app_tool(
|
||||
"calendar_list_events",
|
||||
"calendar",
|
||||
Some("Calendar"),
|
||||
&[],
|
||||
)];
|
||||
|
||||
with_accessible_connectors_cache_cleared(|| {
|
||||
refresh_accessible_connectors_cache_from_mcp_tools(&config, Some(&auth), &tools);
|
||||
|
||||
assert_eq!(
|
||||
list_cached_accessible_connectors_from_mcp_tools(&config, /*auth*/ None),
|
||||
Some(Vec::new())
|
||||
);
|
||||
assert_eq!(
|
||||
list_cached_accessible_connectors_from_mcp_tools(&config, Some(&auth)),
|
||||
Some(vec![AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Calendar".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some(connector_install_url("Calendar", "calendar")),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}])
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accessible_connectors_from_mcp_tools_preserves_description() {
|
||||
let mcp_tools = vec![ToolInfo {
|
||||
|
||||
@@ -1458,17 +1458,18 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
.into_iter()
|
||||
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
|
||||
let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
let auth = sess.services.auth_manager.auth_cached();
|
||||
let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
|
||||
turn_context.config.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
auth.as_ref(),
|
||||
) {
|
||||
Some(connectors) => Some(connectors),
|
||||
None => {
|
||||
connectors::list_accessible_connectors_from_mcp_tools(turn_context.config.as_ref())
|
||||
.await
|
||||
.ok()
|
||||
}
|
||||
None => connectors::list_accessible_connectors_from_mcp_tools(
|
||||
turn_context.config.as_ref(),
|
||||
auth.as_ref(),
|
||||
)
|
||||
.await
|
||||
.ok(),
|
||||
};
|
||||
connectors.and_then(|connectors| {
|
||||
let connector_id = tool_info.connector_id.as_deref()?;
|
||||
|
||||
@@ -602,6 +602,7 @@ impl Session {
|
||||
let auth_manager_clone = Arc::clone(&auth_manager);
|
||||
let config_for_mcp = Arc::clone(&config);
|
||||
let mcp_manager_for_mcp = Arc::clone(&mcp_manager);
|
||||
let plugins_manager_for_mcp = Arc::clone(&plugins_manager);
|
||||
let mcp_thread_init_for_startup = &mcp_thread_init;
|
||||
let auth_and_mcp_fut = async move {
|
||||
let auth = if config_for_mcp.model_provider.requires_openai_auth {
|
||||
@@ -609,6 +610,7 @@ impl Session {
|
||||
} else {
|
||||
auth_manager_clone.auth_for_optional_use().await
|
||||
};
|
||||
plugins_manager_for_mcp.set_auth_mode(auth.as_ref().map(CodexAuth::api_auth_mode));
|
||||
let mcp_config = mcp_manager_for_mcp
|
||||
.runtime_config_for_thread(&config_for_mcp, mcp_thread_init_for_startup)
|
||||
.await;
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::path::PathBuf;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use base64::Engine;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_config::types::ShellEnvironmentPolicyToml;
|
||||
use codex_protocol::permissions::ReadDenyMatcher;
|
||||
use codex_workload_identity::CredentialSourceConfig;
|
||||
@@ -121,6 +122,10 @@ async fn configured_workload_identity_authenticates_responses_turn() -> Result<(
|
||||
let config_toml = workload_identity_config_toml(&server, token_path.clone())?;
|
||||
let mut builder = test_codex_with_workload_identity(config_toml);
|
||||
let test = builder.build(&server).await?;
|
||||
assert_eq!(
|
||||
test.thread_manager.plugins_manager().auth_mode(),
|
||||
Some(AuthMode::ChatgptAuthTokens)
|
||||
);
|
||||
test.submit_turn("authenticate with workload identity")
|
||||
.await?;
|
||||
|
||||
@@ -270,6 +275,7 @@ async fn unauthenticated_provider_skips_unavailable_workload_identity() -> Resul
|
||||
config.model_provider.requires_openai_auth = false;
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
assert_eq!(test.thread_manager.plugins_manager().auth_mode(), None);
|
||||
test.submit_turn("use the unauthenticated provider").await?;
|
||||
|
||||
assert_eq!(response_mock.single_request().header("authorization"), None);
|
||||
|
||||
@@ -819,10 +819,7 @@ async fn auth_result_preserves_external_auth_resolution_errors() {
|
||||
)));
|
||||
|
||||
assert_eq!(manager.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
assert_eq!(
|
||||
manager.get_api_auth_mode(),
|
||||
Some(ApiAuthMode::ChatgptAuthTokens)
|
||||
);
|
||||
assert_eq!(manager.get_api_auth_mode(), None);
|
||||
assert!(!manager.current_auth_uses_codex_backend());
|
||||
let error = manager
|
||||
.auth_result()
|
||||
|
||||
@@ -2239,12 +2239,17 @@ impl AuthManager {
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
/// Returns the mode of auth currently available to API-facing optional features.
|
||||
///
|
||||
/// Required external ChatGPT auth is not active until it has resolved into the auth cache.
|
||||
/// Callers that need configured intent before resolution, such as request attestation, should
|
||||
/// use [`Self::auth_mode`].
|
||||
pub fn get_api_auth_mode(&self) -> Option<ApiAuthMode> {
|
||||
match self.external_auth_mode() {
|
||||
Some(AuthMode::ApiKey) => return Some(ApiAuthMode::ApiKey),
|
||||
Some(AuthMode::Chatgpt) => return Some(ApiAuthMode::ChatgptAuthTokens),
|
||||
Some(
|
||||
AuthMode::ChatgptAuthTokens
|
||||
AuthMode::Chatgpt
|
||||
| AuthMode::ChatgptAuthTokens
|
||||
| AuthMode::AgentIdentity
|
||||
| AuthMode::PersonalAccessToken
|
||||
| AuthMode::BedrockApiKey,
|
||||
|
||||
@@ -223,10 +223,12 @@ impl ModelProvider for ConfiguredModelProvider {
|
||||
}
|
||||
|
||||
fn supports_attestation(&self) -> bool {
|
||||
self.auth_manager
|
||||
.as_ref()
|
||||
.and_then(|auth_manager| auth_manager.get_api_auth_mode())
|
||||
.is_some_and(codex_models_manager::AuthMode::has_chatgpt_account)
|
||||
self.info.requires_openai_auth
|
||||
&& self
|
||||
.auth_manager
|
||||
.as_ref()
|
||||
.and_then(|auth_manager| auth_manager.auth_mode())
|
||||
.is_some_and(codex_models_manager::AuthMode::has_chatgpt_account)
|
||||
}
|
||||
|
||||
fn auth(&self) -> ModelProviderFuture<'_, codex_protocol::error::Result<Option<CodexAuth>>> {
|
||||
@@ -456,6 +458,17 @@ mod tests {
|
||||
assert!(provider.supports_attestation());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unauthenticated_provider_does_not_enable_attestation_for_external_auth() {
|
||||
let auth_manager = required_external_auth_manager_without_cached_auth();
|
||||
let provider = create_model_provider(
|
||||
provider_for("https://example.test/v1".to_string()),
|
||||
Some(auth_manager),
|
||||
);
|
||||
|
||||
assert!(!provider.supports_attestation());
|
||||
}
|
||||
|
||||
fn remote_model(slug: &str) -> ModelInfo {
|
||||
serde_json::from_value(json!({
|
||||
"slug": slug,
|
||||
|
||||
Reference in New Issue
Block a user