From d8ddeb68696dd7b90fe41cd3809821d2ddf7d1b0 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Thu, 14 May 2026 11:52:43 -0700 Subject: [PATCH] Support explicit MCP OAuth client IDs (#22575) ## Why Some MCP OAuth providers require a pre-registered public client ID and cannot rely on dynamic client registration. Codex already supports MCP OAuth, but it had no way to supply that client ID from config into the PKCE flow. ## What changed - add `oauth.client_id` under `[mcp_servers.]` config, including config editing and schema generation - thread the configured client ID through CLI, app-server, plugin login, and MCP skill dependency OAuth entrypoints - configure RMCP authorization with the explicit client when present, while preserving the existing dynamic-registration path when it is absent - add focused coverage for config parsing/serialization and OAuth URL generation ## Verification - `cargo test -p codex-config -p codex-rmcp-client -p codex-mcp -p codex-core-plugins` - `cargo test -p codex-core blocking_replace_mcp_servers_round_trips --lib` - `cargo test -p codex-core replace_mcp_servers_streamable_http_serializes_oauth_resource --lib` - `cargo test -p codex-core config_schema_matches_fixture --lib` ## Notes Broader local package runs still hit unrelated pre-existing stack overflows in: - `codex-app-server::in_process_start_clamps_zero_channel_capacity` - `codex-core::resume_agent_from_rollout_uses_edge_data_when_descendant_metadata_source_is_stale` --- .../src/request_processors/mcp_processor.rs | 1 + .../src/request_processors/plugins.rs | 3 + codex-rs/cli/src/mcp_cmd.rs | 6 + .../codex-mcp/src/connection_manager_tests.rs | 2 + codex-rs/codex-mcp/src/mcp/mod.rs | 1 + codex-rs/codex-mcp/src/mcp/mod_tests.rs | 2 + codex-rs/config/src/lib.rs | 1 + codex-rs/config/src/mcp_edit.rs | 9 ++ codex-rs/config/src/mcp_edit_tests.rs | 62 +++++++++ codex-rs/config/src/mcp_types.rs | 26 ++++ codex-rs/config/src/mcp_types_tests.rs | 34 +++++ codex-rs/config/src/types.rs | 1 + codex-rs/core-plugins/src/loader.rs | 22 ++- codex-rs/core-plugins/src/manager_tests.rs | 7 + codex-rs/core/config.schema.json | 19 +++ codex-rs/core/src/config/config_tests.rs | 23 ++++ codex-rs/core/src/config/edit.rs | 9 ++ codex-rs/core/src/config/edit_tests.rs | 13 ++ codex-rs/core/src/mcp_skill_dependencies.rs | 5 + codex-rs/core/tests/suite/code_mode.rs | 1 + codex-rs/core/tests/suite/hooks_mcp.rs | 1 + codex-rs/core/tests/suite/rmcp_client.rs | 1 + codex-rs/core/tests/suite/search_tool.rs | 3 + codex-rs/core/tests/suite/sqlite_state.rs | 1 + codex-rs/core/tests/suite/truncation.rs | 3 + .../rmcp-client/src/perform_oauth_login.rs | 129 +++++++++++++++++- 26 files changed, 374 insertions(+), 11 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index 812108a15c..548bb5be87 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -160,6 +160,7 @@ impl McpRequestProcessor { http_headers, env_http_headers, &resolved_scopes.scopes, + server.oauth_client_id(), server.oauth_resource.as_deref(), timeout_secs, config.mcp_oauth_callback_port, diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index ea1ff3ead4..c290d2091a 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1346,6 +1346,7 @@ impl PluginRequestProcessor { let notification_name = name.clone(); tokio::spawn(async move { + let oauth_client_id = server.oauth_client_id(); let first_attempt = perform_oauth_login_silent( &name, &oauth_config.url, @@ -1353,6 +1354,7 @@ impl PluginRequestProcessor { oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, + oauth_client_id, server.oauth_resource.as_deref(), callback_port, callback_url.as_deref(), @@ -1368,6 +1370,7 @@ impl PluginRequestProcessor { oauth_config.http_headers, oauth_config.env_http_headers, &[], + oauth_client_id, server.oauth_resource.as_deref(), callback_port, callback_url.as_deref(), diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index af75999163..5c66a27d84 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -199,6 +199,7 @@ async fn perform_oauth_login_retry_without_scopes( http_headers: Option>, env_http_headers: Option>, resolved_scopes: &ResolvedMcpOAuthScopes, + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, @@ -210,6 +211,7 @@ async fn perform_oauth_login_retry_without_scopes( http_headers.clone(), env_http_headers.clone(), &resolved_scopes.scopes, + oauth_client_id, oauth_resource, callback_port, callback_url, @@ -226,6 +228,7 @@ async fn perform_oauth_login_retry_without_scopes( http_headers, env_http_headers, &[], + oauth_client_id, oauth_resource, callback_port, callback_url, @@ -309,6 +312,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }; @@ -338,6 +342,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_config.http_headers, oauth_config.env_http_headers, &resolved_scopes, + /*oauth_client_id*/ None, /*oauth_resource*/ None, config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), @@ -431,6 +436,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) http_headers, env_http_headers, &resolved_scopes, + server.oauth_client_id(), server.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 976bd4c768..50b1bf6c12 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -893,6 +893,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }), @@ -945,6 +946,7 @@ fn mcp_init_error_display_reports_generic_errors() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }), diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index b8fb9a080c..0d24718ba9 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -446,6 +446,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), } diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 0d0afc6c48..286c191b29 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -278,6 +278,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -302,6 +303,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 371c3ffba0..3b49db638f 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -96,6 +96,7 @@ pub use mcp_types::AppToolApproval; pub use mcp_types::McpServerConfig; pub use mcp_types::McpServerDisabledReason; pub use mcp_types::McpServerEnvVar; +pub use mcp_types::McpServerOAuthConfig; pub use mcp_types::McpServerToolConfig; pub use mcp_types::McpServerTransportConfig; pub use mcp_types::RawMcpServerConfig; diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index f5881a1257..aaad3444fd 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -212,6 +212,15 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { { entry["scopes"] = array_from_strings(scopes); } + if let Some(oauth) = &config.oauth + && let Some(client_id) = &oauth.client_id + && !client_id.is_empty() + { + let mut oauth_table = TomlTable::new(); + oauth_table.set_implicit(false); + oauth_table["client_id"] = value(client_id.clone()); + entry["oauth"] = TomlItem::Table(oauth_table); + } if let Some(resource) = &config.oauth_resource && !resource.is_empty() { diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index cfcd73c3e5..0215e7b808 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::McpServerOAuthConfig; use crate::McpServerToolConfig; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -33,6 +34,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::from([ ( @@ -82,3 +84,63 @@ approval_mode = "approve" Ok(()) } + +#[tokio::test] +async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let codex_home = std::env::temp_dir().join(format!( + "codex-config-mcp-oauth-edit-test-{}-{unique_suffix}", + std::process::id() + )); + let servers = BTreeMap::from([( + "maas_outlook".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://example.com/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + experimental_environment: None, + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth: Some(McpServerOAuthConfig { + client_id: Some("eci-prd-pub-codex-123".to_string()), + }), + oauth_resource: None, + tools: HashMap::new(), + }, + )]); + + ConfigEditsBuilder::new(&codex_home) + .replace_mcp_servers(&servers) + .apply() + .await?; + + let config_path = codex_home.join(CONFIG_TOML_FILE); + let serialized = std::fs::read_to_string(&config_path)?; + assert_eq!( + serialized, + r#"[mcp_servers.maas_outlook] +url = "https://example.com/mcp" + +[mcp_servers.maas_outlook.oauth] +client_id = "eci-prd-pub-codex-123" +"# + ); + + let loaded = load_global_mcp_servers(&codex_home).await?; + assert_eq!(loaded, servers); + + std::fs::remove_dir_all(&codex_home)?; + + Ok(()) +} diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index d642d9fc57..8dfceec37e 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -114,6 +114,15 @@ impl AsRef for McpServerEnvVar { } } +/// OAuth client settings used when Codex launches an MCP OAuth flow. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct McpServerOAuthConfig { + /// Explicit OAuth client identifier to present during authorization and token exchange. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub client_id: Option, +} + #[derive(Serialize, Debug, Clone, PartialEq)] pub struct McpServerConfig { #[serde(flatten)] @@ -167,6 +176,10 @@ pub struct McpServerConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub scopes: Option>, + /// Optional OAuth client settings for MCP login. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub oauth: Option, + /// Optional OAuth resource parameter to include during MCP login (RFC 8707). #[serde(default, skip_serializing_if = "Option::is_none")] pub oauth_resource: Option, @@ -176,6 +189,14 @@ pub struct McpServerConfig { pub tools: HashMap, } +impl McpServerConfig { + pub fn oauth_client_id(&self) -> Option<&str> { + self.oauth + .as_ref() + .and_then(|oauth| oauth.client_id.as_deref()) + } +} + /// Raw MCP config shape used for deserialization and supported-field JSON /// Schema generation. /// @@ -233,6 +254,8 @@ pub struct RawMcpServerConfig { #[serde(default)] pub scopes: Option>, #[serde(default)] + pub oauth: Option, + #[serde(default)] pub oauth_resource: Option, /// Legacy display-name field accepted for backward compatibility. #[serde(default, rename = "name")] @@ -267,6 +290,7 @@ impl TryFrom for McpServerConfig { enabled_tools, disabled_tools, scopes, + oauth, oauth_resource, _name: _, tools, @@ -297,6 +321,7 @@ impl TryFrom for McpServerConfig { throw_if_set("stdio", "bearer_token", bearer_token.as_ref())?; throw_if_set("stdio", "http_headers", http_headers.as_ref())?; throw_if_set("stdio", "env_http_headers", env_http_headers.as_ref())?; + throw_if_set("stdio", "oauth", oauth.as_ref())?; throw_if_set("stdio", "oauth_resource", oauth_resource.as_ref())?; let env_vars = env_vars.unwrap_or_default(); for env_var in &env_vars { @@ -338,6 +363,7 @@ impl TryFrom for McpServerConfig { enabled_tools, disabled_tools, scopes, + oauth, oauth_resource, tools: tools.unwrap_or_default(), }) diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 50f705d0f8..f397115425 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -283,6 +283,26 @@ fn deserialize_streamable_http_server_config_with_oauth_resource() { ); } +#[test] +fn deserialize_streamable_http_server_config_with_oauth_client_id() { + let cfg: McpServerConfig = toml::from_str( + r#" + url = "https://example.com/mcp" + + [oauth] + client_id = "eci-prd-pub-codex-123" + "#, + ) + .expect("should deserialize http config with oauth client id"); + + assert_eq!( + cfg.oauth, + Some(McpServerOAuthConfig { + client_id: Some("eci-prd-pub-codex-123".to_string()), + }) + ); +} + #[test] fn deserialize_server_config_with_tool_filters() { let cfg: McpServerConfig = toml::from_str( @@ -393,6 +413,7 @@ fn deserialize_ignores_unknown_server_fields() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), } @@ -439,6 +460,19 @@ fn deserialize_rejects_headers_for_stdio() { ) .expect_err("should reject env_http_headers for stdio transport"); + let err = toml::from_str::( + r#" + command = "echo" + oauth = { client_id = "eci-prd-pub-codex-123" } + "#, + ) + .expect_err("should reject oauth for stdio transport"); + + assert!( + err.to_string().contains("oauth is not supported for stdio"), + "unexpected error: {err}" + ); + let err = toml::from_str::( r#" command = "echo" diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 74e3394f93..c7d0a5876b 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -7,6 +7,7 @@ pub use crate::mcp_types::AppToolApproval; pub use crate::mcp_types::McpServerConfig; pub use crate::mcp_types::McpServerDisabledReason; pub use crate::mcp_types::McpServerEnvVar; +pub use crate::mcp_types::McpServerOAuthConfig; pub use crate::mcp_types::McpServerToolConfig; pub use crate::mcp_types::McpServerTransportConfig; pub use crate::mcp_types::RawMcpServerConfig; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index b26f1c21b2..fc4193aa67 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1053,13 +1053,21 @@ fn normalize_plugin_mcp_server_value( } } - if let Some(JsonValue::Object(oauth)) = object.remove("oauth") - && oauth.contains_key("callbackPort") - { - warn!( - plugin = %plugin_root.display(), - "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" - ); + if let Some(JsonValue::Object(mut oauth)) = object.remove("oauth") { + if oauth.remove("callbackPort").is_some() { + warn!( + plugin = %plugin_root.display(), + "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" + ); + } + + if let Some(client_id) = oauth.remove("clientId") { + oauth.entry("client_id".to_string()).or_insert(client_id); + } + + if !oauth.is_empty() { + object.insert("oauth".to_string(), JsonValue::Object(oauth)); + } } if let Some(JsonValue::String(cwd)) = object.get("cwd") diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index e0b22c0040..449c87b186 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -22,6 +22,7 @@ use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::McpServerConfig; +use codex_config::McpServerOAuthConfig; use codex_config::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; use codex_login::CodexAuth; @@ -230,6 +231,9 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: Some(McpServerOAuthConfig { + client_id: Some("client-id".to_string()), + }), oauth_resource: None, tools: HashMap::new(), }, @@ -694,6 +698,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -805,6 +810,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -966,6 +972,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }; diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index d8c64a0d13..9fc95349d2 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1200,6 +1200,17 @@ } ] }, + "McpServerOAuthConfig": { + "additionalProperties": false, + "description": "OAuth client settings used when Codex launches an MCP OAuth flow.", + "properties": { + "client_id": { + "description": "Explicit OAuth client identifier to present during authorization and token exchange.", + "type": "string" + } + }, + "type": "object" + }, "McpServerToolConfig": { "additionalProperties": false, "description": "Per-tool approval settings for a single MCP server tool.", @@ -2124,6 +2135,14 @@ "description": "Legacy display-name field accepted for backward compatibility.", "type": "string" }, + "oauth": { + "allOf": [ + { + "$ref": "#/definitions/McpServerOAuthConfig" + } + ], + "default": null + }, "oauth_resource": { "default": null, "type": "string" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 27a67da68d..ff28e50436 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -36,6 +36,7 @@ use codex_config::types::BundledSkillsConfig; use codex_config::types::FeedbackConfigToml; use codex_config::types::HistoryPersistence; use codex_config::types::McpServerEnvVar; +use codex_config::types::McpServerOAuthConfig; use codex_config::types::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; use codex_config::types::MemoriesConfig; @@ -124,6 +125,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), } @@ -148,6 +150,7 @@ fn http_mcp(url: &str) -> McpServerConfig { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), } @@ -4428,6 +4431,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -4721,6 +4725,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -4797,6 +4802,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -4858,6 +4864,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()> enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -4909,6 +4916,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -4963,6 +4971,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5033,6 +5042,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5115,6 +5125,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5150,6 +5161,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5220,6 +5232,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5245,6 +5258,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5333,6 +5347,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5383,6 +5398,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5433,6 +5449,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { enabled_tools: Some(vec!["allowed".to_string()]), disabled_tools: Some(vec!["blocked".to_string()]), scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -5487,6 +5504,9 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh enabled_tools: None, disabled_tools: None, scopes: None, + oauth: Some(McpServerOAuthConfig { + client_id: Some("eci-prd-pub-codex-123".to_string()), + }), oauth_resource: Some("https://resource.example.com".to_string()), tools: HashMap::new(), }, @@ -5500,6 +5520,8 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; + assert!(serialized.contains("[mcp_servers.docs.oauth]")); + assert!(serialized.contains(r#"client_id = "eci-prd-pub-codex-123""#)); assert!(serialized.contains(r#"oauth_resource = "https://resource.example.com""#)); let loaded = load_global_mcp_servers(codex_home.path()).await?; @@ -5508,6 +5530,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh docs.oauth_resource.as_deref(), Some("https://resource.example.com") ); + assert_eq!(docs.oauth_client_id(), Some("eci-prd-pub-codex-123")); Ok(()) } diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 4e5b46e182..80bf4d9bf1 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -341,6 +341,15 @@ mod document_helpers { { entry["scopes"] = array_from_iter(scopes.iter().cloned()); } + if let Some(oauth) = &config.oauth + && let Some(client_id) = &oauth.client_id + && !client_id.is_empty() + { + let mut oauth_table = TomlTable::new(); + oauth_table.set_implicit(false); + oauth_table["client_id"] = value(client_id.clone()); + entry["oauth"] = TomlItem::Table(oauth_table); + } if let Some(resource) = &config.oauth_resource && !resource.is_empty() { diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index ec7342c779..751411afdf 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -1,5 +1,6 @@ use super::*; use codex_config::types::AppToolApproval; +use codex_config::types::McpServerOAuthConfig; use codex_config::types::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; use codex_config::types::SessionPickerViewMode; @@ -940,6 +941,7 @@ fn blocking_replace_mcp_servers_round_trips() { enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -969,6 +971,9 @@ fn blocking_replace_mcp_servers_round_trips() { enabled_tools: None, disabled_tools: Some(vec!["forbidden".to_string()]), scopes: None, + oauth: Some(McpServerOAuthConfig { + client_id: Some("eci-prd-pub-codex-123".to_string()), + }), oauth_resource: Some("https://resource.example.com".to_string()), tools: HashMap::new(), }, @@ -994,6 +999,9 @@ oauth_resource = \"https://resource.example.com\" [mcp_servers.http.http_headers] Z-Header = \"z\" +[mcp_servers.http.oauth] +client_id = \"eci-prd-pub-codex-123\" + [mcp_servers.stdio] command = \"cmd\" args = [\"--flag\"] @@ -1035,6 +1043,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::from([( "search".to_string(), @@ -1099,6 +1108,7 @@ foo = { command = "cmd" } enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -1153,6 +1163,7 @@ foo = { command = "cmd" } # keep me enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -1206,6 +1217,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -1260,6 +1272,7 @@ foo = { command = "cmd" } enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 44764e0064..b8552596b3 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -151,6 +151,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( server_config.scopes.clone(), oauth_config.discovered_scopes.clone(), ); + let oauth_client_id = server_config.oauth_client_id(); let first_attempt = perform_oauth_login( &name, &oauth_config.url, @@ -158,6 +159,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, + oauth_client_id, server_config.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), @@ -173,6 +175,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_config.http_headers, oauth_config.env_http_headers, &[], + oauth_client_id, server_config.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), @@ -369,6 +372,7 @@ fn mcp_dependency_to_server_config( enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }); @@ -398,6 +402,7 @@ fn mcp_dependency_to_server_config( enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }); diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 3bcb37e7b2..af63d092f8 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -244,6 +244,7 @@ async fn run_code_mode_turn_with_rmcp_config( enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/core/tests/suite/hooks_mcp.rs b/codex-rs/core/tests/suite/hooks_mcp.rs index 96ed732695..80290e31ba 100644 --- a/codex-rs/core/tests/suite/hooks_mcp.rs +++ b/codex-rs/core/tests/suite/hooks_mcp.rs @@ -193,6 +193,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 1a7062bdb1..d1973d97a9 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -332,6 +332,7 @@ fn insert_mcp_server( enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index b3cf2e3bca..bb4ca84b29 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -999,6 +999,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { enabled_tools: Some(vec!["echo".to_string(), "image".to_string()]), disabled_tools: Some(vec!["image".to_string()]), scopes: None, + oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, tools: HashMap::new(), @@ -1127,6 +1128,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result< enabled_tools: Some(vec!["echo".to_string()]), disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, tools: HashMap::new(), @@ -1272,6 +1274,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti enabled_tools: Some(vec!["echo".to_string()]), disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, tools: HashMap::new(), diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index b2a2778047..0cc7d07b16 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -386,6 +386,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 0fd072a70f..447bca6731 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -398,6 +398,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -497,6 +498,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, @@ -779,6 +781,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth: None, oauth_resource: None, tools: HashMap::new(), }, diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index f42786c652..eb6c7906a8 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -11,6 +11,9 @@ use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use reqwest::ClientBuilder; use reqwest::Url; +use rmcp::transport::AuthorizationManager; +use rmcp::transport::AuthorizationSession; +use rmcp::transport::auth::OAuthClientConfig; use rmcp::transport::auth::OAuthState; use sha2::Digest; use sha2::Sha256; @@ -81,6 +84,7 @@ pub async fn perform_oauth_login( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, @@ -92,6 +96,7 @@ pub async fn perform_oauth_login( http_headers, env_http_headers, scopes, + oauth_client_id, oauth_resource, callback_port, callback_url, @@ -108,6 +113,7 @@ pub async fn perform_oauth_login_silent( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, @@ -119,6 +125,7 @@ pub async fn perform_oauth_login_silent( http_headers, env_http_headers, scopes, + oauth_client_id, oauth_resource, callback_port, callback_url, @@ -135,6 +142,7 @@ async fn perform_oauth_login_with_browser_output( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, @@ -150,6 +158,7 @@ async fn perform_oauth_login_with_browser_output( store_mode, headers, scopes, + oauth_client_id, oauth_resource, /*launch_browser*/ true, callback_port, @@ -169,6 +178,7 @@ pub async fn perform_oauth_login_return_url( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, timeout_secs: Option, callback_port: Option, @@ -184,6 +194,7 @@ pub async fn perform_oauth_login_return_url( store_mode, headers, scopes, + oauth_client_id, oauth_resource, /*launch_browser*/ false, callback_port, @@ -436,6 +447,7 @@ impl OauthLoginFlow { store_mode: OAuthCredentialsStoreMode, headers: OauthHeaders, scopes: &[String], + oauth_client_id: Option<&str>, oauth_resource: Option<&str>, launch_browser: bool, callback_port: Option, @@ -471,11 +483,15 @@ impl OauthLoginFlow { let default_headers = build_default_headers(http_headers, env_http_headers)?; let http_client = apply_default_headers(ClientBuilder::new(), &default_headers).build()?; - let mut oauth_state = OAuthState::new(server_url, Some(http_client)).await?; let scope_refs: Vec<&str> = scopes.iter().map(String::as_str).collect(); - oauth_state - .start_authorization(&scope_refs, &redirect_uri, Some("Codex")) - .await?; + let oauth_state = start_authorization( + server_url, + http_client, + &scope_refs, + &redirect_uri, + oauth_client_id, + ) + .await?; let auth_url = append_query_param( &oauth_state.get_authorization_url().await?, "resource", @@ -585,6 +601,41 @@ impl OauthLoginFlow { } } +async fn start_authorization( + server_url: &str, + http_client: reqwest::Client, + scopes: &[&str], + redirect_uri: &str, + oauth_client_id: Option<&str>, +) -> Result { + let Some(oauth_client_id) = oauth_client_id.filter(|client_id| !client_id.trim().is_empty()) + else { + let mut oauth_state = OAuthState::new(server_url, Some(http_client)).await?; + oauth_state + .start_authorization(scopes, redirect_uri, Some("Codex")) + .await?; + return Ok(oauth_state); + }; + + let mut auth_manager = AuthorizationManager::new(server_url).await?; + auth_manager.with_client(http_client)?; + let metadata = auth_manager.discover_metadata().await?; + auth_manager.set_metadata(metadata); + auth_manager.configure_client(OAuthClientConfig { + client_id: oauth_client_id.to_string(), + client_secret: None, + scopes: scopes.iter().map(|scope| (*scope).to_string()).collect(), + redirect_uri: redirect_uri.to_string(), + })?; + let auth_url = auth_manager.get_authorization_url(scopes).await?; + + Ok(OAuthState::Session(AuthorizationSession { + auth_manager, + auth_url, + redirect_uri: redirect_uri.to_string(), + })) +} + fn append_query_param(url: &str, key: &str, value: Option<&str>) -> String { let Some(value) = value else { return url.to_string(); @@ -604,7 +655,13 @@ fn append_query_param(url: &str, key: &str, value: Option<&str>) -> String { #[cfg(test)] mod tests { + use axum::Json; + use axum::Router; + use axum::routing::get; use pretty_assertions::assert_eq; + use reqwest::Url; + use serde_json::json; + use tokio::net::TcpListener; use super::CallbackOutcome; use super::OAuthProviderError; @@ -613,6 +670,70 @@ mod tests { use super::callback_id_from_server_url; use super::callback_path_from_redirect_uri; use super::parse_oauth_callback; + use super::start_authorization; + + async fn spawn_oauth_metadata_server() -> String { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind metadata listener"); + let addr = listener.local_addr().expect("read metadata listener addr"); + let base_url = format!("http://{addr}"); + let metadata = json!({ + "authorization_endpoint": format!("{base_url}/oauth/authorize"), + "token_endpoint": format!("{base_url}/oauth/token"), + "scopes_supported": [""], + }); + let path_scoped_metadata = metadata.clone(); + let app = Router::new() + .route( + "/.well-known/oauth-authorization-server/mcp", + get(move || { + let metadata = path_scoped_metadata.clone(); + async move { Json(metadata) } + }), + ) + .route( + "/.well-known/oauth-authorization-server", + get(move || { + let metadata = metadata.clone(); + async move { Json(metadata) } + }), + ); + + tokio::spawn(async move { + axum::serve(listener, app) + .await + .expect("serve oauth metadata"); + }); + + base_url + } + + #[tokio::test] + async fn start_authorization_uses_configured_client_id() { + let base_url = spawn_oauth_metadata_server().await; + let oauth_state = start_authorization( + &format!("{base_url}/mcp"), + reqwest::Client::new(), + &[], + "http://127.0.0.1/callback", + Some("eci-prd-pub-codex-123"), + ) + .await + .expect("start oauth authorization"); + + let authorization_url = oauth_state + .get_authorization_url() + .await + .expect("read authorization url"); + let auth_url = Url::parse(&authorization_url).expect("authorization url should parse"); + let client_id = auth_url + .query_pairs() + .find(|(key, _)| key == "client_id") + .map(|(_, value)| value.into_owned()); + + assert_eq!(client_id.as_deref(), Some("eci-prd-pub-codex-123")); + } #[test] fn parse_oauth_callback_accepts_default_path() {