From 5886055baf464410eaf01e4fbae55bec27d410c3 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 26 May 2026 15:44:28 -0700 Subject: [PATCH] changes --- codex-rs/Cargo.lock | 1 + .../request_processors/account_processor.rs | 4 +- .../src/request_processors/mcp_processor.rs | 1 + .../src/request_processors/plugins.rs | 3 + .../app-server/tests/common/auth_fixtures.rs | 4 +- codex-rs/cli/src/doctor.rs | 4 +- codex-rs/cli/src/login.rs | 20 +- codex-rs/cli/src/mcp_cmd.rs | 13 +- codex-rs/cloud-config/src/bundle_loader.rs | 4 +- codex-rs/cloud-tasks/src/util.rs | 2 +- codex-rs/codex-mcp/src/connection_manager.rs | 3 + .../codex-mcp/src/connection_manager_tests.rs | 2 + codex-rs/codex-mcp/src/mcp/auth.rs | 14 +- codex-rs/codex-mcp/src/mcp/mod.rs | 8 + codex-rs/codex-mcp/src/mcp/mod_tests.rs | 3 + codex-rs/codex-mcp/src/rmcp_client.rs | 5 + codex-rs/config/src/types.rs | 20 ++ codex-rs/core-api/src/lib.rs | 1 + codex-rs/core/src/config/mod.rs | 15 +- codex-rs/core/src/connectors.rs | 2 + codex-rs/core/src/mcp_skill_dependencies.rs | 2 + codex-rs/core/src/mcp_tool_call_tests.rs | 1 + codex-rs/core/src/session/mcp.rs | 11 +- codex-rs/core/src/session/session.rs | 2 + codex-rs/core/tests/suite/client.rs | 2 + codex-rs/exec/src/lib.rs | 2 +- codex-rs/login/src/auth/auth_tests.rs | 50 +-- codex-rs/login/src/auth/manager.rs | 54 +-- codex-rs/login/src/auth/storage.rs | 32 +- codex-rs/login/src/auth/storage_tests.rs | 20 +- codex-rs/login/src/device_code_auth.rs | 2 +- codex-rs/login/src/lib.rs | 2 +- codex-rs/login/src/server.rs | 24 +- codex-rs/login/tests/suite/auth_refresh.rs | 22 +- .../login/tests/suite/device_code_login.rs | 18 +- .../login/tests/suite/login_server_e2e.rs | 20 +- codex-rs/login/tests/suite/logout.rs | 16 +- codex-rs/models-manager/src/manager_tests.rs | 4 +- codex-rs/rmcp-client/Cargo.toml | 1 + codex-rs/rmcp-client/src/auth_status.rs | 6 +- codex-rs/rmcp-client/src/oauth.rs | 336 ++++++++++++++++-- .../rmcp-client/src/perform_oauth_login.rs | 19 +- codex-rs/rmcp-client/src/rmcp_client.rs | 11 +- .../tests/streamable_http_test_support.rs | 3 + codex-rs/tui/src/lib.rs | 2 +- 45 files changed, 591 insertions(+), 200 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2c23dbb2e9..29828ec3e5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3649,6 +3649,7 @@ dependencies = [ "codex-exec-server", "codex-keyring-store", "codex-protocol", + "codex-secrets", "codex-utils-cargo-bin", "codex-utils-home-dir", "codex-utils-pty", diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 9c78759d8d..0d7b20cc78 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -286,7 +286,7 @@ impl AccountRequestProcessor { &self.config.codex_home, ¶ms.api_key, self.config.cli_auth_credentials_store_mode, - self.config.cli_auth_keyring_backend_kind(), + self.config.auth_keyring_backend_kind(), ) { Ok(()) => { self.auth_manager.reload().await; @@ -335,7 +335,7 @@ impl AccountRequestProcessor { CLIENT_ID.to_string(), config.forced_chatgpt_workspace_id.clone(), config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) }; #[cfg(debug_assertions)] 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 facd4facc6..cdb64d5e94 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -161,6 +161,7 @@ impl McpRequestProcessor { &name, &url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), http_headers, env_http_headers, &resolved_scopes.scopes, diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 08fc99d01b..274cb127f6 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1677,6 +1677,7 @@ impl PluginRequestProcessor { ); let store_mode = config.mcp_oauth_credentials_store_mode; + let keyring_backend_kind = config.auth_keyring_backend_kind(); let callback_port = config.mcp_oauth_callback_port; let callback_url = config.mcp_oauth_callback_url.clone(); let outgoing = Arc::clone(&self.outgoing); @@ -1688,6 +1689,7 @@ impl PluginRequestProcessor { &name, &oauth_config.url, store_mode, + keyring_backend_kind, oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, @@ -1704,6 +1706,7 @@ impl PluginRequestProcessor { &name, &oauth_config.url, store_mode, + keyring_backend_kind, oauth_config.http_headers, oauth_config.env_http_headers, &[], diff --git a/codex-rs/app-server/tests/common/auth_fixtures.rs b/codex-rs/app-server/tests/common/auth_fixtures.rs index 2ba1bcec29..fd28b8ec01 100644 --- a/codex-rs/app-server/tests/common/auth_fixtures.rs +++ b/codex-rs/app-server/tests/common/auth_fixtures.rs @@ -9,7 +9,7 @@ use chrono::Utc; use codex_app_server_protocol::AuthMode; use codex_config::types::AuthCredentialsStoreMode; use codex_login::AuthDotJson; -use codex_login::CliAuthKeyringBackendKind; +use codex_login::AuthKeyringBackendKind; use codex_login::save_auth; use codex_login::token_data::TokenData; use codex_login::token_data::parse_chatgpt_jwt_claims; @@ -172,7 +172,7 @@ pub fn write_chatgpt_auth( codex_home, &auth, cli_auth_credentials_store_mode, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("write auth.json") } diff --git a/codex-rs/cli/src/doctor.rs b/codex-rs/cli/src/doctor.rs index f851179361..a78dbd1925 100644 --- a/codex-rs/cli/src/doctor.rs +++ b/codex-rs/cli/src/doctor.rs @@ -1199,7 +1199,7 @@ fn auth_check(config: &Config) -> DoctorCheck { match load_auth_dot_json( &config.codex_home, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) { Ok(Some(auth)) => { details.push(format!("stored auth mode: {}", stored_auth_mode(&auth))); @@ -2526,7 +2526,7 @@ fn provider_reachability_plan(config: &Config) -> ReachabilityPlan { let stored_auth = load_auth_dot_json( &config.codex_home, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .ok() .flatten(); diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index c341242dff..abbbee3e98 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -10,8 +10,8 @@ use codex_app_server_protocol::AuthMode; use codex_config::types::AuthCredentialsStoreMode; use codex_core::config::Config; +use codex_login::AuthKeyringBackendKind; use codex_login::CLIENT_ID; -use codex_login::CliAuthKeyringBackendKind; use codex_login::CodexAuth; use codex_login::ServerOptions; use codex_login::login_with_access_token; @@ -118,14 +118,14 @@ pub async fn login_with_chatgpt( codex_home: PathBuf, forced_chatgpt_workspace_id: Option>, cli_auth_credentials_store_mode: AuthCredentialsStoreMode, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind, + auth_keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result<()> { let opts = ServerOptions::new( codex_home, CLIENT_ID.to_string(), forced_chatgpt_workspace_id, cli_auth_credentials_store_mode, - cli_auth_keyring_backend_kind, + auth_keyring_backend_kind, ); let server = run_login_server(opts)?; @@ -150,7 +150,7 @@ pub async fn run_login_with_chatgpt(cli_config_overrides: CliConfigOverrides) -> config.codex_home.to_path_buf(), forced_chatgpt_workspace_id, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .await { @@ -182,7 +182,7 @@ pub async fn run_login_with_api_key( &config.codex_home, &api_key, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) { Ok(_) => { eprintln!("{LOGIN_SUCCESS_MESSAGE}"); @@ -213,7 +213,7 @@ pub async fn run_login_with_access_token( &access_token, config.cli_auth_credentials_store_mode, Some(&config.chatgpt_base_url), - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .await { @@ -288,7 +288,7 @@ pub async fn run_login_with_device_code( client_id.unwrap_or(CLIENT_ID.to_string()), forced_chatgpt_workspace_id, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ); if let Some(iss) = issuer_base_url { opts.issuer = iss; @@ -328,7 +328,7 @@ pub async fn run_login_with_device_code_fallback_to_browser( client_id.unwrap_or(CLIENT_ID.to_string()), forced_chatgpt_workspace_id, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ); if let Some(iss) = issuer_base_url { opts.issuer = iss; @@ -377,7 +377,7 @@ pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { &config.codex_home, config.cli_auth_credentials_store_mode, Some(&config.chatgpt_base_url), - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .await { @@ -422,7 +422,7 @@ pub async fn run_logout(cli_config_overrides: CliConfigOverrides) -> ! { match logout_with_revoke( &config.codex_home, config.cli_auth_credentials_store_mode, - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .await { diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 0103782653..1466f10394 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -211,6 +211,7 @@ async fn perform_oauth_login_retry_without_scopes( name: &str, url: &str, store_mode: codex_config::types::OAuthCredentialsStoreMode, + keyring_backend_kind: codex_config::types::AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, resolved_scopes: &ResolvedMcpOAuthScopes, @@ -223,6 +224,7 @@ async fn perform_oauth_login_retry_without_scopes( name, url, store_mode, + keyring_backend_kind, http_headers.clone(), env_http_headers.clone(), &resolved_scopes.scopes, @@ -240,6 +242,7 @@ async fn perform_oauth_login_retry_without_scopes( name, url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, &[], @@ -384,6 +387,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers, oauth_config.env_http_headers, &resolved_scopes, @@ -478,6 +482,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) &name, &url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), http_headers, env_http_headers, &resolved_scopes, @@ -514,7 +519,12 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr _ => bail!("OAuth logout is only supported for streamable_http transports."), }; - match delete_oauth_tokens(&name, &url, config.mcp_oauth_credentials_store_mode) { + match delete_oauth_tokens( + &name, + &url, + config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), + ) { Ok(true) => println!("Removed OAuth credentials for '{name}'."), Ok(false) => println!("No OAuth credentials stored for '{name}'."), Err(err) => return Err(anyhow!("failed to delete OAuth credentials: {err}")), @@ -541,6 +551,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let auth_statuses = compute_auth_statuses( effective_mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), /*auth*/ None, ) .await; diff --git a/codex-rs/cloud-config/src/bundle_loader.rs b/codex-rs/cloud-config/src/bundle_loader.rs index 2e9a1175bd..684539e4ac 100644 --- a/codex-rs/cloud-config/src/bundle_loader.rs +++ b/codex-rs/cloud-config/src/bundle_loader.rs @@ -5,8 +5,8 @@ use codex_config::CloudConfigBundleLoadError; use codex_config::CloudConfigBundleLoadErrorCode; use codex_config::CloudConfigBundleLoader; use codex_config::types::AuthCredentialsStoreMode; +use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; -use codex_login::CliAuthKeyringBackendKind; use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; @@ -63,7 +63,7 @@ pub async fn cloud_config_bundle_loader_for_storage( enable_codex_api_key_env, credentials_store_mode, Some(chatgpt_base_url.clone()), - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await; cloud_config_bundle_loader(auth_manager, chatgpt_base_url, codex_home) diff --git a/codex-rs/cloud-tasks/src/util.rs b/codex-rs/cloud-tasks/src/util.rs index 3ffde2d0dd..f683c58a4b 100644 --- a/codex-rs/cloud-tasks/src/util.rs +++ b/codex-rs/cloud-tasks/src/util.rs @@ -50,7 +50,7 @@ pub async fn load_auth_manager(chatgpt_base_url: Option) -> Option, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, auth_entries: HashMap, approval_policy: &Constrained, submit_id: String, @@ -198,6 +200,7 @@ impl McpConnectionManager { server_name.clone(), server, store_mode, + keyring_backend_kind, cancel_token.clone(), tx_event.clone(), elicitation_requests.clone(), diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index ea405ced9a..508598d343 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -20,6 +20,7 @@ use crate::tools::normalize_tools_for_model_with_prefix; use crate::tools::tool_with_model_visible_input_schema; use codex_config::Constrained; use codex_config::McpServerConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_exec_server::EnvironmentManager; use codex_protocol::ToolName; use codex_protocol::mcp::McpServerInfo; @@ -1213,6 +1214,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { let manager = McpConnectionManager::new( &mcp_servers, OAuthCredentialsStoreMode::default(), + AuthKeyringBackendKind::default(), HashMap::new(), &approval_policy, String::new(), diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 12f832f9e9..ec67f07a82 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use anyhow::Result; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_protocol::protocol::McpAuthStatus; @@ -130,6 +131,7 @@ pub fn should_retry_without_scopes(scopes: &ResolvedMcpOAuthScopes, error: &anyh pub async fn compute_auth_statuses<'a, I>( servers: I, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, auth: Option<&CodexAuth>, ) -> HashMap where @@ -152,7 +154,15 @@ where async move { let auth_status = match config.as_ref() { Some(config) => { - match compute_auth_status(&name, config, store_mode, has_runtime_auth).await { + match compute_auth_status( + &name, + config, + store_mode, + keyring_backend_kind, + has_runtime_auth, + ) + .await + { Ok(status) => status, Err(error) => { warn!( @@ -179,6 +189,7 @@ async fn compute_auth_status( server_name: &str, config: &McpServerConfig, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, has_runtime_auth: bool, ) -> Result { if !config.enabled { @@ -204,6 +215,7 @@ async fn compute_auth_status( http_headers.clone(), env_http_headers.clone(), store_mode, + keyring_backend_kind, ) .await } diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index e6b17b14c3..1a7e6d53dc 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -21,6 +21,8 @@ use codex_config::Constrained; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::types::AppToolApproval; +use codex_config::types::ApprovalsReviewer; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_plugin::PluginCapabilitySummary; @@ -113,6 +115,8 @@ pub struct McpConfig { pub codex_home: PathBuf, /// Preferred credential store for MCP OAuth tokens. pub mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode, + /// Backend used when MCP OAuth storage is configured for keyring-backed persistence. + pub auth_keyring_backend_kind: AuthKeyringBackendKind, /// Optional fixed localhost callback port for MCP OAuth login. pub mcp_oauth_callback_port: Option, /// Optional OAuth redirect URI override for MCP login. @@ -264,6 +268,7 @@ pub async fn read_mcp_resource( let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth, ) .await; @@ -273,6 +278,7 @@ pub async fn read_mcp_resource( let manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth_statuses, &config.approval_policy, String::new(), @@ -332,6 +338,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let auth_status_entries = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth, ) .await; @@ -345,6 +352,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind, auth_status_entries.clone(), &config.approval_policy, submit_id, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 767f20611f..e6665c8c89 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -1,6 +1,8 @@ use super::*; use codex_config::Constrained; use codex_config::types::AppToolApproval; +use codex_config::types::ApprovalsReviewer; +use codex_config::types::AuthKeyringBackendKind; use codex_login::CodexAuth; use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; @@ -19,6 +21,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { apps_mcp_product_sku: None, codex_home, mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(), + auth_keyring_backend_kind: AuthKeyringBackendKind::default(), mcp_oauth_callback_port: None, mcp_oauth_callback_url: None, skill_mcp_dependency_install_enabled: true, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 78078720e7..f4fc8b8072 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -45,6 +45,7 @@ use codex_async_utils::CancelErr; use codex_async_utils::OrCancelExt; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::HttpClient; use codex_exec_server::ReqwestHttpClient; @@ -141,6 +142,7 @@ impl AsyncManagedClient { server_name: String, server: EffectiveMcpServer, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, cancel_token: CancellationToken, tx_event: Sender, elicitation_requests: ElicitationRequestManager, @@ -179,6 +181,7 @@ impl AsyncManagedClient { &server_name, server.clone(), store_mode, + keyring_backend_kind, runtime_context, runtime_auth_provider, ) @@ -577,6 +580,7 @@ async fn make_rmcp_client( server_name: &str, server: EffectiveMcpServer, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, runtime_context: McpRuntimeContext, runtime_auth_provider: Option, ) -> Result { @@ -648,6 +652,7 @@ async fn make_rmcp_client( http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, runtime_auth_provider, ) diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 7b634a5188..b0b0a8b63f 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -113,6 +113,26 @@ pub enum OAuthCredentialsStoreMode { Keyring, } +/// Determine how auth credentials should use keyring-backed storage. +#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "lowercase")] +pub enum AuthKeyringBackendKind { + /// Store the serialized auth payload directly in the OS keyring. + Direct, + /// Store auth payloads in the local encrypted secrets file, with the file key in the OS keyring. + Secrets, +} + +impl Default for AuthKeyringBackendKind { + fn default() -> Self { + if cfg!(windows) { + Self::Secrets + } else { + Self::Direct + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "kebab-case")] pub enum WindowsSandboxModeToml { diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index 47452eb032..52621e366a 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -12,6 +12,7 @@ pub use codex_config::config_toml::ProjectConfig; pub use codex_config::config_toml::RealtimeAudioConfig; pub use codex_config::config_toml::RealtimeConfig; pub use codex_config::types::AuthCredentialsStoreMode; +pub use codex_config::types::AuthKeyringBackendKind; pub use codex_config::types::History; pub use codex_config::types::MemoriesConfig; pub use codex_config::types::ModelAvailabilityNuxConfig; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6f7b0582c2..63ce584abe 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -38,6 +38,7 @@ use codex_config::permissions_toml::PermissionsToml; use codex_config::sandbox_mode_requirement_for_permission_profile; use codex_config::types::ApprovalsReviewer; use codex_config::types::AuthCredentialsStoreMode; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::History; use codex_config::types::McpServerConfig; use codex_config::types::McpServerDisabledReason; @@ -70,7 +71,6 @@ use codex_features::NetworkProxyConfigToml; use codex_git_utils::resolve_root_git_project_for_trust; use codex_install_context::InstallContext; use codex_login::AuthManagerConfig; -use codex_login::CliAuthKeyringBackendKind; use codex_mcp::McpConfig; use codex_memories_read::memory_root; use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; @@ -1104,8 +1104,8 @@ impl AuthManagerConfig for Config { self.cli_auth_credentials_store_mode } - fn cli_auth_keyring_backend_kind(&self) -> CliAuthKeyringBackendKind { - Config::cli_auth_keyring_backend_kind(self) + fn auth_keyring_backend_kind(&self) -> AuthKeyringBackendKind { + Config::auth_keyring_backend_kind(self) } fn forced_chatgpt_workspace_id(&self) -> Option> { @@ -1356,13 +1356,11 @@ impl Config { workspace_roots } - pub fn cli_auth_keyring_backend_kind(&self) -> CliAuthKeyringBackendKind { - // TODO: Migrate all keyring-backed CLI auth storage to the secrets - // backend after validating this path on Windows. + pub fn auth_keyring_backend_kind(&self) -> AuthKeyringBackendKind { if self.features.enabled(Feature::SecretAuthStorage) { - CliAuthKeyringBackendKind::Secrets + AuthKeyringBackendKind::Secrets } else { - CliAuthKeyringBackendKind::Direct + AuthKeyringBackendKind::Direct } } @@ -1429,6 +1427,7 @@ impl Config { apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), codex_home: self.codex_home.to_path_buf(), mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode, + auth_keyring_backend_kind: self.auth_keyring_backend_kind(), mcp_oauth_callback_port: self.mcp_oauth_callback_port, mcp_oauth_callback_url: self.mcp_oauth_callback_url.clone(), skill_mcp_dependency_install_enabled: self diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index d8df728554..f7ffa312a0 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -279,6 +279,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( let auth_status_entries = compute_auth_statuses( mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth.as_ref(), ) .await; @@ -290,6 +291,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth_status_entries, &config.permissions.approval_policy, INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 304deb500d..b40952de0b 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -156,6 +156,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers.clone(), oauth_config.env_http_headers.clone(), &resolved_scopes.scopes, @@ -172,6 +173,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( &name, &oauth_config.url, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), oauth_config.http_headers, oauth_config.env_http_headers, &[], diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 723948b7be..b413704cd5 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1260,6 +1260,7 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: let manager = codex_mcp::McpConnectionManager::new( &HashMap::new(), turn_context.config.mcp_oauth_credentials_store_mode, + turn_context.config.auth_keyring_backend_kind(), HashMap::new(), &turn_context.approval_policy, turn_context.sub_id.clone(), diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 39333801c0..da967df900 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -303,8 +303,14 @@ impl Session { effective_mcp_servers_from_configured(mcp_servers, &mcp_config, auth.as_ref()); let host_owned_codex_apps_enabled = host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()); - let auth_statuses = - compute_auth_statuses(mcp_servers.iter(), store_mode, auth.as_ref()).await; + let keyring_backend_kind = config.auth_keyring_backend_kind(); + let auth_statuses = compute_auth_statuses( + mcp_servers.iter(), + store_mode, + keyring_backend_kind, + auth.as_ref(), + ) + .await; let mcp_runtime_context = match turn_context.environments.primary() { Some(turn_environment) => McpRuntimeContext::new( Arc::clone(&self.services.environment_manager), @@ -326,6 +332,7 @@ impl Session { let refreshed_manager = McpConnectionManager::new( &mcp_servers, store_mode, + keyring_backend_kind, auth_statuses, &turn_context.approval_policy, turn_context.sub_id.clone(), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index e42cf202e8..4c8b8ff6a7 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -611,6 +611,7 @@ impl Session { let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config_for_mcp.mcp_oauth_credentials_store_mode, + config_for_mcp.auth_keyring_backend_kind(), auth.as_ref(), ) .await; @@ -1142,6 +1143,7 @@ impl Session { let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, + config.auth_keyring_backend_kind(), auth_statuses, &session_configuration.approval_policy, INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index a522e041c2..ee7792d2a8 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -10,6 +10,7 @@ use codex_core::resolve_installation_id; use codex_core::thread_store_from_config; use codex_extension_api::empty_extension_registry; use codex_features::Feature; +use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::default_client::originator; @@ -1121,6 +1122,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { codex_home.path(), AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, + AuthKeyringBackendKind::default(), ) .await { diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 63dd23b9ab..24d780d7db 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -466,7 +466,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result if let Err(err) = enforce_login_restrictions(&AuthConfig { codex_home: config.codex_home.to_path_buf(), auth_credentials_store_mode: config.cli_auth_credentials_store_mode, - keyring_backend_kind: config.cli_auth_keyring_backend_kind(), + keyring_backend_kind: config.auth_keyring_backend_kind(), 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()), diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 6d5df6a8d3..c3dc0b7a24 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -43,7 +43,7 @@ async fn refresh_without_id_token() { let storage = create_auth_storage( codex_home.path().to_path_buf(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); let updated = super::persist_tokens( &storage, @@ -82,7 +82,7 @@ fn login_with_api_key_overwrites_existing_auth_json() { dir.path(), "sk-new", AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .expect("login_with_api_key should succeed"); @@ -115,7 +115,7 @@ async fn login_with_access_token_writes_only_token() { &agent_identity, AuthCredentialsStoreMode::File, Some(&chatgpt_base_url), - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await .expect("login_with_access_token should succeed"); @@ -221,7 +221,7 @@ async fn login_with_access_token_rejects_invalid_jwt() { "not-a-jwt", AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await .expect_err("invalid access token should fail"); @@ -252,7 +252,7 @@ async fn login_with_access_token_rejects_unsigned_jwt() { &agent_identity, AuthCredentialsStoreMode::File, Some(&chatgpt_base_url), - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await .expect_err("unsigned access token should fail"); @@ -273,7 +273,7 @@ async fn missing_auth_json_returns_none() { dir.path(), AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await .expect("call should succeed"); @@ -300,7 +300,7 @@ async fn pro_account_with_no_api_key_uses_chatgpt_auth() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .unwrap() @@ -358,7 +358,7 @@ async fn loads_api_key_from_auth_json() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .unwrap() @@ -384,14 +384,14 @@ fn logout_removes_auth_file() -> Result<(), std::io::Error> { dir.path(), &auth_dot_json, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let auth_file = get_auth_file(dir.path()); assert!(auth_file.exists()); assert!(logout( dir.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?); assert!(!auth_file.exists()); Ok(()) @@ -405,7 +405,7 @@ async fn unauthorized_recovery_reports_mode_and_step_names() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await; let managed = UnauthorizedRecovery { @@ -447,7 +447,7 @@ async fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") @@ -466,7 +466,7 @@ async fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { updated_auth_dot_json, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("updated auth should parse"); @@ -768,7 +768,7 @@ async fn build_config( AuthConfig { codex_home: codex_home.to_path_buf(), auth_credentials_store_mode: AuthCredentialsStoreMode::File, - keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + keyring_backend_kind: AuthKeyringBackendKind::Direct, forced_login_method, forced_chatgpt_workspace_id, chatgpt_base_url: None, @@ -851,7 +851,7 @@ async fn load_auth_reads_access_token_from_env() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, Some(&chatgpt_base_url), - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("env auth should load") @@ -975,7 +975,7 @@ async fn load_auth_keeps_codex_api_key_env_precedence() { /*enable_codex_api_key_env*/ true, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("env auth should load") @@ -993,7 +993,7 @@ async fn enforce_login_restrictions_logs_out_for_method_mismatch() { codex_home.path(), "sk-test", AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .expect("seed api key"); @@ -1146,14 +1146,14 @@ async fn enforce_login_restrictions_logs_out_for_agent_identity_workspace_mismat personal_access_token: None, }, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .expect("seed agent identity auth"); let config = AuthConfig { codex_home: codex_home.path().to_path_buf(), auth_credentials_store_mode: AuthCredentialsStoreMode::File, - keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + keyring_backend_kind: AuthKeyringBackendKind::Direct, forced_login_method: None, forced_chatgpt_workspace_id: Some(vec![WORKSPACE_ID_ALLOWED.to_string()]), chatgpt_base_url: Some(chatgpt_base_url), @@ -1181,7 +1181,7 @@ async fn enforce_login_restrictions_allows_api_key_if_login_method_not_set_but_f codex_home.path(), "sk-test", AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .expect("seed api key"); @@ -1407,7 +1407,7 @@ async fn plan_type_maps_known_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") @@ -1436,7 +1436,7 @@ async fn plan_type_maps_self_serve_business_usage_based_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") @@ -1468,7 +1468,7 @@ async fn plan_type_maps_enterprise_cbp_usage_based_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") @@ -1500,7 +1500,7 @@ async fn plan_type_maps_unknown_to_unknown() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") @@ -1529,7 +1529,7 @@ async fn missing_plan_type_maps_to_unknown() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await .expect("load auth") diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 90629dd194..6e792d92fc 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -32,8 +32,8 @@ pub use crate::auth::agent_identity::AgentIdentityAuth; pub use crate::auth::personal_access_token::PersonalAccessTokenAuth; pub use crate::auth::storage::AgentIdentityAuthRecord; pub use crate::auth::storage::AuthDotJson; +pub use crate::auth::storage::AuthKeyringBackendKind; use crate::auth::storage::AuthStorageBackend; -pub use crate::auth::storage::CliAuthKeyringBackendKind; use crate::auth::storage::create_auth_storage; use crate::auth::util::try_parse_error_message; use crate::default_client::build_reqwest_client; @@ -211,7 +211,7 @@ impl CodexAuth { auth_dot_json: AuthDotJson, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option<&str>, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result { let auth_mode = auth_dot_json.resolved_mode(); let client = create_client(); @@ -268,7 +268,7 @@ impl CodexAuth { codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option<&str>, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result> { load_auth( codex_home, @@ -491,7 +491,7 @@ impl CodexAuth { let storage = create_auth_storage( PathBuf::from(format!("dummy-chatgpt-auth-{dummy_auth_id}")), AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); Self::Chatgpt(ChatgptAuth { state, storage }) } @@ -565,7 +565,7 @@ async fn verified_agent_identity_record( pub fn logout( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result { let storage = create_auth_storage( codex_home.to_path_buf(), @@ -578,7 +578,7 @@ pub fn logout( pub async fn logout_with_revoke( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result { AuthManager::new( codex_home.to_path_buf(), @@ -597,7 +597,7 @@ pub fn login_with_api_key( codex_home: &Path, api_key: &str, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result<()> { let auth_dot_json = AuthDotJson { auth_mode: Some(ApiAuthMode::ApiKey), @@ -621,7 +621,7 @@ pub async fn login_with_access_token( access_token: &str, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option<&str>, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result<()> { let auth_dot_json = match classify_codex_access_token(access_token) { CodexAccessToken::PersonalAccessToken(access_token) => { @@ -677,7 +677,7 @@ pub fn login_with_chatgpt_auth_tokens( codex_home, &auth_dot_json, AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) } @@ -686,7 +686,7 @@ pub fn save_auth( codex_home: &Path, auth: &AuthDotJson, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result<()> { let storage = create_auth_storage( codex_home.to_path_buf(), @@ -704,7 +704,7 @@ pub fn save_auth( pub fn load_auth_dot_json( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result> { let storage = create_auth_storage( codex_home.to_path_buf(), @@ -718,7 +718,7 @@ pub fn load_auth_dot_json( pub struct AuthConfig { pub codex_home: PathBuf, pub auth_credentials_store_mode: AuthCredentialsStoreMode, - pub keyring_backend_kind: CliAuthKeyringBackendKind, + pub keyring_backend_kind: AuthKeyringBackendKind, pub forced_login_method: Option, pub chatgpt_base_url: Option, pub forced_chatgpt_workspace_id: Option>, @@ -821,7 +821,7 @@ fn logout_with_message( codex_home: &Path, message: String, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result<()> { // External auth tokens live in the ephemeral store, but persistent auth may still exist // from earlier logins. Clear both so a forced logout truly removes all active auth. @@ -840,19 +840,19 @@ fn logout_with_message( fn logout_all_stores( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result { if auth_credentials_store_mode == AuthCredentialsStoreMode::Ephemeral { return logout( codex_home, AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); } let removed_ephemeral = logout( codex_home, AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let removed_managed = logout( codex_home, @@ -867,7 +867,7 @@ async fn load_auth( enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option<&str>, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> std::io::Result> { // API key via env var takes precedence over any other auth method. if enable_codex_api_key_env && let Some(api_key) = read_codex_api_key_from_env() { @@ -879,7 +879,7 @@ async fn load_auth( let ephemeral_storage = create_auth_storage( codex_home.to_path_buf(), AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); if let Some(auth_dot_json) = ephemeral_storage.load()? { let auth = CodexAuth::from_auth_dot_json( @@ -1419,7 +1419,7 @@ pub struct AuthManager { auth_change_tx: watch::Sender, enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, forced_chatgpt_workspace_id: RwLock>>, chatgpt_base_url: Option, refresh_lock: Semaphore, @@ -1440,7 +1440,7 @@ pub trait AuthManagerConfig { fn cli_auth_credentials_store_mode(&self) -> AuthCredentialsStoreMode; /// Returns the backend to use when CLI auth keyring storage is selected. - fn cli_auth_keyring_backend_kind(&self) -> CliAuthKeyringBackendKind; + fn auth_keyring_backend_kind(&self) -> AuthKeyringBackendKind; /// Returns the workspace IDs that ChatGPT auth should be restricted to, if any. fn forced_chatgpt_workspace_id(&self) -> Option>; @@ -1480,7 +1480,7 @@ impl AuthManager { enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Self { let managed_auth = load_auth( &codex_home, @@ -1524,7 +1524,7 @@ impl AuthManager { auth_change_tx, enable_codex_api_key_env: false, auth_credentials_store_mode: AuthCredentialsStoreMode::File, - keyring_backend_kind: CliAuthKeyringBackendKind::default(), + keyring_backend_kind: AuthKeyringBackendKind::default(), forced_chatgpt_workspace_id: RwLock::new(None), chatgpt_base_url: None, refresh_lock: Semaphore::new(/*permits*/ 1), @@ -1545,7 +1545,7 @@ impl AuthManager { auth_change_tx, enable_codex_api_key_env: false, auth_credentials_store_mode: AuthCredentialsStoreMode::File, - keyring_backend_kind: CliAuthKeyringBackendKind::default(), + keyring_backend_kind: AuthKeyringBackendKind::default(), forced_chatgpt_workspace_id: RwLock::new(None), chatgpt_base_url: None, refresh_lock: Semaphore::new(/*permits*/ 1), @@ -1564,7 +1564,7 @@ impl AuthManager { auth_change_tx, enable_codex_api_key_env: false, auth_credentials_store_mode: AuthCredentialsStoreMode::File, - keyring_backend_kind: CliAuthKeyringBackendKind::default(), + keyring_backend_kind: AuthKeyringBackendKind::default(), forced_chatgpt_workspace_id: RwLock::new(None), chatgpt_base_url: None, refresh_lock: Semaphore::new(/*permits*/ 1), @@ -1784,7 +1784,7 @@ impl AuthManager { enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Arc { Arc::new( Self::new( @@ -1808,7 +1808,7 @@ impl AuthManager { enable_codex_api_key_env, config.cli_auth_credentials_store_mode(), Some(config.chatgpt_base_url()), - config.cli_auth_keyring_backend_kind(), + config.auth_keyring_backend_kind(), ) .await; auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id()); @@ -2073,7 +2073,7 @@ impl AuthManager { &self.codex_home, &auth_dot_json, AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .map_err(RefreshTokenError::Transient)?; self.reload().await; diff --git a/codex-rs/login/src/auth/storage.rs b/codex-rs/login/src/auth/storage.rs index edf4384843..f6cacfdc7a 100644 --- a/codex-rs/login/src/auth/storage.rs +++ b/codex-rs/login/src/auth/storage.rs @@ -23,6 +23,7 @@ use codex_agent_identity::AgentIdentityJwtClaims; use codex_agent_identity::decode_agent_identity_jwt; use codex_app_server_protocol::AuthMode; use codex_config::types::AuthCredentialsStoreMode; +pub use codex_config::types::AuthKeyringBackendKind; use codex_keyring_store::DefaultKeyringStore; use codex_keyring_store::KeyringStore; use codex_protocol::account::PlanType as AccountPlanType; @@ -106,25 +107,6 @@ pub(super) trait AuthStorageBackend: Debug + Send + Sync { fn delete(&self) -> std::io::Result; } -/// Backend used when CLI auth storage is configured for keyring-backed persistence. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum CliAuthKeyringBackendKind { - /// Store the serialized auth payload directly in the OS keyring. - Direct, - /// Store auth in the local encrypted secrets file, with the file key in the OS keyring. - Secrets, -} - -impl Default for CliAuthKeyringBackendKind { - fn default() -> Self { - if cfg!(windows) { - Self::Secrets - } else { - Self::Direct - } - } -} - #[derive(Clone, Debug)] pub(super) struct FileAuthStorage { codex_home: PathBuf, @@ -351,7 +333,7 @@ impl AutoAuthStorage { fn new( codex_home: PathBuf, keyring_store: Arc, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Self { Self { keyring_storage: create_keyring_auth_storage( @@ -438,7 +420,7 @@ impl AuthStorageBackend for EphemeralAuthStorage { pub(super) fn create_auth_storage( codex_home: PathBuf, mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Arc { let keyring_store: Arc = Arc::new(DefaultKeyringStore); create_auth_storage_with_store(codex_home, mode, keyring_store, keyring_backend_kind) @@ -448,7 +430,7 @@ fn create_auth_storage_with_store( codex_home: PathBuf, mode: AuthCredentialsStoreMode, keyring_store: Arc, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Arc { match mode { AuthCredentialsStoreMode::File => Arc::new(FileAuthStorage::new(codex_home)), @@ -467,13 +449,13 @@ fn create_auth_storage_with_store( fn create_keyring_auth_storage( codex_home: PathBuf, keyring_store: Arc, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Arc { match keyring_backend_kind { - CliAuthKeyringBackendKind::Direct => { + AuthKeyringBackendKind::Direct => { Arc::new(DirectKeyringAuthStorage::new(codex_home, keyring_store)) } - CliAuthKeyringBackendKind::Secrets => { + AuthKeyringBackendKind::Secrets => { Arc::new(SecretsKeyringAuthStorage::new(codex_home, keyring_store)) } } diff --git a/codex-rs/login/src/auth/storage_tests.rs b/codex-rs/login/src/auth/storage_tests.rs index ef22b9221a..dcb9c349a3 100644 --- a/codex-rs/login/src/auth/storage_tests.rs +++ b/codex-rs/login/src/auth/storage_tests.rs @@ -153,7 +153,7 @@ fn file_storage_delete_removes_auth_file() -> anyhow::Result<()> { let storage = create_auth_storage( dir.path().to_path_buf(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); storage.save(&auth_dot_json)?; assert!(dir.path().join("auth.json").exists()); @@ -170,7 +170,7 @@ fn ephemeral_storage_save_load_delete_is_in_memory_only() -> anyhow::Result<()> let storage = create_auth_storage( dir.path().to_path_buf(), AuthCredentialsStoreMode::Ephemeral, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); let auth_dot_json = AuthDotJson { auth_mode: Some(AuthMode::ApiKey), @@ -410,7 +410,7 @@ fn factory_uses_secrets_backend_only_when_requested() -> anyhow::Result<()> { direct_home.path().to_path_buf(), AuthCredentialsStoreMode::Keyring, Arc::new(direct_keyring.clone()), - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ); let direct_auth = auth_with_prefix("factory-direct"); direct_storage.save(&direct_auth)?; @@ -427,7 +427,7 @@ fn factory_uses_secrets_backend_only_when_requested() -> anyhow::Result<()> { secrets_home.path().to_path_buf(), AuthCredentialsStoreMode::Keyring, Arc::new(secrets_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let secrets_auth = auth_with_prefix("factory-secrets"); secrets_storage.save(&secrets_auth)?; @@ -503,7 +503,7 @@ fn auto_auth_storage_load_prefers_keyring_value() -> anyhow::Result<()> { let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let keyring_auth = auth_with_prefix("keyring"); seed_secrets_backend_with_auth(&mock_keyring, codex_home.path(), &keyring_auth)?; @@ -523,7 +523,7 @@ fn auto_auth_storage_load_uses_file_when_keyring_empty() -> anyhow::Result<()> { let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let expected = auth_with_prefix("file-only"); @@ -541,7 +541,7 @@ fn auto_auth_storage_load_falls_back_when_keyring_errors() -> anyhow::Result<()> let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let key = compute_keyring_account(codex_home.path()); @@ -564,7 +564,7 @@ fn auto_auth_storage_save_prefers_keyring() -> anyhow::Result<()> { let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let stale = auth_with_prefix("stale"); storage.file_storage.save(&stale)?; @@ -583,7 +583,7 @@ fn auto_auth_storage_save_falls_back_when_keyring_errors() -> anyhow::Result<()> let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let key = compute_keyring_account(codex_home.path()); mock_keyring.set_error(&key, KeyringError::Invalid("error".into(), "save".into())); @@ -615,7 +615,7 @@ fn auto_auth_storage_delete_removes_keyring_and_file() -> anyhow::Result<()> { let storage = AutoAuthStorage::new( codex_home.path().to_path_buf(), Arc::new(mock_keyring.clone()), - CliAuthKeyringBackendKind::Secrets, + AuthKeyringBackendKind::Secrets, ); let auth = auth_with_prefix("to-delete"); let auth_file = seed_secrets_backend_and_fallback_auth_file_for_delete( diff --git a/codex-rs/login/src/device_code_auth.rs b/codex-rs/login/src/device_code_auth.rs index 894c80fab4..6f18c5984c 100644 --- a/codex-rs/login/src/device_code_auth.rs +++ b/codex-rs/login/src/device_code_auth.rs @@ -217,7 +217,7 @@ pub async fn complete_device_code_login( tokens.access_token, tokens.refresh_token, opts.cli_auth_credentials_store_mode, - opts.cli_auth_keyring_backend_kind, + opts.auth_keyring_backend_kind, ) .await } diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index be6c427671..9cbb24598d 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -19,12 +19,12 @@ pub use server::run_login_server; pub use auth::AuthConfig; pub use auth::AuthDotJson; +pub use auth::AuthKeyringBackendKind; pub use auth::AuthManager; pub use auth::AuthManagerConfig; pub use auth::CLIENT_ID; pub use auth::CODEX_ACCESS_TOKEN_ENV_VAR; pub use auth::CODEX_API_KEY_ENV_VAR; -pub use auth::CliAuthKeyringBackendKind; pub use auth::CodexAuth; pub use auth::ExternalAuth; pub use auth::ExternalAuthChatgptMetadata; diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 5929474ab8..a14b2f54c8 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -25,7 +25,7 @@ use std::thread; use std::time::Duration; use crate::auth::AuthDotJson; -use crate::auth::CliAuthKeyringBackendKind; +use crate::auth::AuthKeyringBackendKind; use crate::auth::load_auth_dot_json; use crate::auth::revoke_auth_tokens; use crate::auth::save_auth; @@ -73,7 +73,7 @@ pub struct ServerOptions { pub forced_chatgpt_workspace_id: Option>, pub codex_streamlined_login: bool, pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, - pub cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind, + pub auth_keyring_backend_kind: AuthKeyringBackendKind, } impl ServerOptions { @@ -83,7 +83,7 @@ impl ServerOptions { client_id: String, forced_chatgpt_workspace_id: Option>, cli_auth_credentials_store_mode: AuthCredentialsStoreMode, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind, + auth_keyring_backend_kind: AuthKeyringBackendKind, ) -> Self { Self { codex_home, @@ -95,7 +95,7 @@ impl ServerOptions { forced_chatgpt_workspace_id, codex_streamlined_login: false, cli_auth_credentials_store_mode, - cli_auth_keyring_backend_kind, + auth_keyring_backend_kind, } } } @@ -366,7 +366,7 @@ async fn process_request( tokens.access_token.clone(), tokens.refresh_token.clone(), opts.cli_auth_credentials_store_mode, - opts.cli_auth_keyring_backend_kind, + opts.auth_keyring_backend_kind, ) .await { @@ -798,7 +798,7 @@ pub(crate) async fn persist_tokens_async( access_token: String, refresh_token: String, auth_credentials_store_mode: AuthCredentialsStoreMode, - keyring_backend_kind: CliAuthKeyringBackendKind, + keyring_backend_kind: AuthKeyringBackendKind, ) -> io::Result<()> { // Reuse existing synchronous logic but run it off the async runtime. let codex_home = codex_home.to_path_buf(); @@ -1201,7 +1201,7 @@ mod tests { use wiremock::matchers::path; use crate::auth::AuthDotJson; - use crate::auth::CliAuthKeyringBackendKind; + use crate::auth::AuthKeyringBackendKind; use crate::auth::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR; use crate::auth::load_auth_dot_json; use crate::auth::save_auth; @@ -1249,7 +1249,7 @@ mod tests { codex_home.path(), &chatgpt_auth("old-access", "old-refresh", "old-account"), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; persist_tokens_async( @@ -1259,14 +1259,14 @@ mod tests { "new-access".to_string(), "new-refresh".to_string(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await?; let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )? .context("auth.json should exist after login")?; assert_eq!( @@ -1315,7 +1315,7 @@ mod tests { codex_home.path(), &chatgpt_auth("old-access", "shared-refresh", "old-account"), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; persist_tokens_async( @@ -1325,7 +1325,7 @@ mod tests { "new-access".to_string(), "shared-refresh".to_string(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::Direct, + AuthKeyringBackendKind::Direct, ) .await?; diff --git a/codex-rs/login/tests/suite/auth_refresh.rs b/codex-rs/login/tests/suite/auth_refresh.rs index 33bdf646bb..ec915d6322 100644 --- a/codex-rs/login/tests/suite/auth_refresh.rs +++ b/codex-rs/login/tests/suite/auth_refresh.rs @@ -6,8 +6,8 @@ use chrono::Utc; use codex_app_server_protocol::AuthMode; use codex_config::types::AuthCredentialsStoreMode; use codex_login::AuthDotJson; +use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; -use codex_login::CliAuthKeyringBackendKind; use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_login::RefreshTokenError; use codex_login::load_auth_dot_json; @@ -292,7 +292,7 @@ async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> { ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; ctx.auth_manager @@ -361,7 +361,7 @@ async fn refresh_token_errors_on_account_mismatch() -> Result<()> { ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let err = ctx @@ -534,7 +534,7 @@ async fn auth_reloads_disk_auth_when_cached_auth_is_stale() -> Result<()> { ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let cached_auth = ctx @@ -600,7 +600,7 @@ async fn auth_reloads_disk_auth_without_calling_expired_refresh_token() -> Resul ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let cached_auth = ctx @@ -865,7 +865,7 @@ async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result< ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; ctx.auth_manager @@ -991,7 +991,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let cached_before = ctx @@ -1089,7 +1089,7 @@ async fn unauthorized_recovery_errors_on_account_mismatch() -> Result<()> { ctx.codex_home.path(), &disk_auth, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let cached_before = ctx @@ -1181,7 +1181,7 @@ impl RefreshTokenTestContext { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await; @@ -1196,7 +1196,7 @@ impl RefreshTokenTestContext { load_auth_dot_json( self.codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("load auth.json")? .context("auth.json should exist") @@ -1207,7 +1207,7 @@ impl RefreshTokenTestContext { self.codex_home.path(), auth_dot_json, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; self.auth_manager.reload().await; Ok(()) diff --git a/codex-rs/login/tests/suite/device_code_login.rs b/codex-rs/login/tests/suite/device_code_login.rs index bccc401c81..78adc84c24 100644 --- a/codex-rs/login/tests/suite/device_code_login.rs +++ b/codex-rs/login/tests/suite/device_code_login.rs @@ -4,7 +4,7 @@ use anyhow::Context; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use codex_config::types::AuthCredentialsStoreMode; -use codex_login::CliAuthKeyringBackendKind; +use codex_login::AuthKeyringBackendKind; use codex_login::ServerOptions; use codex_login::auth::load_auth_dot_json; use codex_login::run_device_code_login; @@ -111,7 +111,7 @@ fn server_opts( "client-id".to_string(), /*forced_chatgpt_workspace_id*/ None, cli_auth_credentials_store_mode, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); opts.issuer = issuer; opts.open_browser = false; @@ -152,7 +152,7 @@ async fn device_code_login_integration_succeeds() -> anyhow::Result<()> { let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("auth.json should load after login succeeds")? .context("auth.json written")?; @@ -202,7 +202,7 @@ async fn device_code_login_rejects_workspace_mismatch() -> anyhow::Result<()> { let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("auth.json should load after login fails")?; assert!( @@ -237,7 +237,7 @@ async fn device_code_login_integration_handles_usercode_http_failure() -> anyhow let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("auth.json should load after login fails")?; assert!( @@ -276,7 +276,7 @@ async fn device_code_login_integration_persists_without_api_key_on_exchange_fail "client-id".to_string(), /*forced_chatgpt_workspace_id*/ None, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); opts.issuer = issuer; opts.open_browser = false; @@ -288,7 +288,7 @@ async fn device_code_login_integration_persists_without_api_key_on_exchange_fail let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("auth.json should load after login succeeds")? .context("auth.json written")?; @@ -331,7 +331,7 @@ async fn device_code_login_integration_handles_error_payload() -> anyhow::Result "client-id".to_string(), /*forced_chatgpt_workspace_id*/ None, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); opts.issuer = issuer; opts.open_browser = false; @@ -349,7 +349,7 @@ async fn device_code_login_integration_handles_error_payload() -> anyhow::Result let auth = load_auth_dot_json( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .context("auth.json should load after login fails")?; assert!( diff --git a/codex-rs/login/tests/suite/login_server_e2e.rs b/codex-rs/login/tests/suite/login_server_e2e.rs index 0b0319002b..74a9b430f7 100644 --- a/codex-rs/login/tests/suite/login_server_e2e.rs +++ b/codex-rs/login/tests/suite/login_server_e2e.rs @@ -9,7 +9,7 @@ use std::time::Duration; use anyhow::Result; use base64::Engine; use codex_config::types::AuthCredentialsStoreMode; -use codex_login::CliAuthKeyringBackendKind; +use codex_login::AuthKeyringBackendKind; use codex_login::ServerOptions; use codex_login::run_login_server; use core_test_support::skip_if_no_network; @@ -129,7 +129,7 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> { force_state: Some(state), forced_chatgpt_workspace_id: Some(vec![chatgpt_account_id.to_string()]), codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; assert!( @@ -192,7 +192,7 @@ async fn creates_missing_codex_home_dir() -> Result<()> { force_state: Some(state), forced_chatgpt_workspace_id: None, codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; let login_port = server.actual_port; @@ -236,7 +236,7 @@ async fn login_server_includes_forced_workspaces_as_one_query_param() -> Result< WORKSPACE_ID_SECOND_ALLOWED.to_string(), ]), codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; let auth_url = Url::parse(&server.auth_url)?; @@ -275,7 +275,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> { force_state: Some(state.clone()), forced_chatgpt_workspace_id: Some(vec![WORKSPACE_ID_ALLOWED.to_string()]), codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; assert!( @@ -336,7 +336,7 @@ async fn oauth_access_denied_missing_entitlement_blocks_login_with_clear_error() force_state: Some(state.clone()), forced_chatgpt_workspace_id: None, codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; let login_port = server.actual_port; @@ -405,7 +405,7 @@ async fn oauth_access_denied_unknown_reason_uses_generic_error_page() -> Result< force_state: Some(state.clone()), forced_chatgpt_workspace_id: None, codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let server = run_login_server(opts)?; let login_port = server.actual_port; @@ -506,7 +506,7 @@ async fn falls_back_to_registered_fallback_port_when_default_port_is_in_use() -> codex_login::CLIENT_ID.to_string(), /*forced_chatgpt_workspace_id*/ None, AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ); opts.issuer = issuer; opts.open_browser = false; @@ -552,7 +552,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { force_state: Some("cancel_state".to_string()), forced_chatgpt_workspace_id: None, codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let first_server = run_login_server(first_opts)?; @@ -574,7 +574,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { force_state: Some("cancel_state_2".to_string()), forced_chatgpt_workspace_id: None, codex_streamlined_login: false, - cli_auth_keyring_backend_kind: CliAuthKeyringBackendKind::Direct, + auth_keyring_backend_kind: AuthKeyringBackendKind::Direct, }; let second_server = run_login_server(second_opts)?; diff --git a/codex-rs/login/tests/suite/logout.rs b/codex-rs/login/tests/suite/logout.rs index a1700bdfa1..7f9e048268 100644 --- a/codex-rs/login/tests/suite/logout.rs +++ b/codex-rs/login/tests/suite/logout.rs @@ -4,9 +4,9 @@ use base64::Engine; use codex_app_server_protocol::AuthMode; use codex_config::types::AuthCredentialsStoreMode; use codex_login::AuthDotJson; +use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; use codex_login::CLIENT_ID; -use codex_login::CliAuthKeyringBackendKind; use codex_login::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_login::logout_with_revoke; use codex_login::save_auth; @@ -51,13 +51,13 @@ async fn logout_with_revoke_revokes_refresh_token_then_removes_auth() -> Result< codex_home.path(), &chatgpt_auth(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let removed = logout_with_revoke( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await?; @@ -109,13 +109,13 @@ async fn logout_with_revoke_removes_auth_when_revoke_fails() -> Result<()> { codex_home.path(), &chatgpt_auth(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let removed = logout_with_revoke( codex_home.path(), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await?; @@ -150,21 +150,21 @@ async fn auth_manager_logout_with_revoke_uses_cached_auth() -> Result<()> { codex_home.path(), &chatgpt_auth_with_refresh_token(REFRESH_TOKEN), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let manager = AuthManager::new( codex_home.path().to_path_buf(), /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await; save_auth( codex_home.path(), &chatgpt_auth_with_refresh_token("newer-disk-refresh-token"), AuthCredentialsStoreMode::File, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), )?; let removed = manager.logout_with_revoke().await?; diff --git a/codex-rs/models-manager/src/manager_tests.rs b/codex-rs/models-manager/src/manager_tests.rs index 3fc25ddd6c..cd95eb59b2 100644 --- a/codex-rs/models-manager/src/manager_tests.rs +++ b/codex-rs/models-manager/src/manager_tests.rs @@ -3,8 +3,8 @@ use crate::ModelsManagerConfig; use chrono::Utc; use codex_app_server_protocol::AuthMode; use codex_login::AuthCredentialsStoreMode; +use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; -use codex_login::CliAuthKeyringBackendKind; use codex_login::CodexAuth; use codex_login::ExternalAuth; use codex_login::ExternalAuthRefreshContext; @@ -225,7 +225,7 @@ c2ln", codex_home, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - CliAuthKeyringBackendKind::default(), + AuthKeyringBackendKind::default(), ) .await .expect("auth should load") diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index 1006ad74e8..c355e0ee6b 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -21,6 +21,7 @@ codex-config = { workspace = true } codex-exec-server = { workspace = true } codex-keyring-store = { workspace = true } codex-protocol = { workspace = true } +codex-secrets = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-home-dir = { workspace = true } bytes = { workspace = true } diff --git a/codex-rs/rmcp-client/src/auth_status.rs b/codex-rs/rmcp-client/src/auth_status.rs index bef9edc6c9..cdb0a69bba 100644 --- a/codex-rs/rmcp-client/src/auth_status.rs +++ b/codex-rs/rmcp-client/src/auth_status.rs @@ -16,6 +16,7 @@ use crate::oauth::StoredOAuthTokenStatus; use crate::oauth::oauth_token_status; use crate::utils::apply_default_headers; use crate::utils::build_default_headers; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; const DISCOVERY_TIMEOUT: Duration = Duration::from_secs(5); @@ -35,6 +36,7 @@ pub async fn determine_streamable_http_auth_status( http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { if bearer_token_env_var.is_some() { return Ok(McpAuthStatus::BearerToken); @@ -45,7 +47,7 @@ pub async fn determine_streamable_http_auth_status( return Ok(McpAuthStatus::BearerToken); } - match oauth_token_status(server_name, url, store_mode)? { + match oauth_token_status(server_name, url, store_mode, keyring_backend_kind)? { StoredOAuthTokenStatus::Usable => return Ok(McpAuthStatus::OAuth), StoredOAuthTokenStatus::AuthorizationRequired => { return Ok(McpAuthStatus::NotLoggedIn); @@ -288,6 +290,7 @@ mod tests { )])), /*env_http_headers*/ None, OAuthCredentialsStoreMode::Keyring, + AuthKeyringBackendKind::default(), ) .await .expect("status should compute"); @@ -309,6 +312,7 @@ mod tests { "CODEX_RMCP_CLIENT_AUTH_STATUS_TEST_TOKEN".to_string(), )])), OAuthCredentialsStoreMode::Keyring, + AuthKeyringBackendKind::default(), ) .await .expect("status should compute"); diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 5742660277..92607a40e1 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -19,7 +19,11 @@ use anyhow::Context; use anyhow::Error; use anyhow::Result; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; +use codex_secrets::LocalSecretsBackend; +use codex_secrets::SecretName; +use codex_secrets::SecretScope; use oauth2::AccessToken; use oauth2::RefreshToken; use oauth2::Scope; @@ -51,6 +55,7 @@ use tokio::sync::Mutex; use codex_utils_home_dir::find_codex_home; const KEYRING_SERVICE: &str = "Codex MCP Credentials"; +const MCP_OAUTH_SECRET_PREFIX: &str = "MCP_OAUTH"; const REFRESH_SKEW_MILLIS: u64 = 30_000; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -87,15 +92,19 @@ pub(crate) fn load_oauth_tokens( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result> { let keyring_store = DefaultKeyringStore; match store_mode { - OAuthCredentialsStoreMode::Auto => { - load_oauth_tokens_from_keyring_with_fallback_to_file(&keyring_store, server_name, url) - } + OAuthCredentialsStoreMode::Auto => load_oauth_tokens_from_keyring_with_fallback_to_file( + &keyring_store, + keyring_backend_kind, + server_name, + url, + ), OAuthCredentialsStoreMode::File => load_oauth_tokens_from_file(server_name, url), OAuthCredentialsStoreMode::Keyring => { - load_oauth_tokens_from_keyring(&keyring_store, server_name, url) + load_oauth_tokens_from_keyring(&keyring_store, keyring_backend_kind, server_name, url) .with_context(|| "failed to read OAuth tokens from keyring".to_string()) } } @@ -105,9 +114,10 @@ pub(crate) fn oauth_token_status( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { Ok( - match load_oauth_tokens(server_name, url, store_mode)?.as_ref() { + match load_oauth_tokens(server_name, url, store_mode, keyring_backend_kind)?.as_ref() { None => StoredOAuthTokenStatus::Missing, Some(tokens) if oauth_tokens_are_usable(tokens) => StoredOAuthTokenStatus::Usable, Some(_) => StoredOAuthTokenStatus::AuthorizationRequired, @@ -152,12 +162,13 @@ fn refresh_expires_in_from_timestamp(tokens: &mut StoredOAuthTokens) { } } -fn load_oauth_tokens_from_keyring_with_fallback_to_file( +fn load_oauth_tokens_from_keyring_with_fallback_to_file( keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, server_name: &str, url: &str, ) -> Result> { - match load_oauth_tokens_from_keyring(keyring_store, server_name, url) { + match load_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url) { Ok(Some(tokens)) => Ok(Some(tokens)), Ok(None) => load_oauth_tokens_from_file(server_name, url), Err(error) => { @@ -168,7 +179,23 @@ fn load_oauth_tokens_from_keyring_with_fallback_to_file( } } -fn load_oauth_tokens_from_keyring( +fn load_oauth_tokens_from_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + url: &str, +) -> Result> { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + load_oauth_tokens_from_direct_keyring(keyring_store, server_name, url) + } + AuthKeyringBackendKind::Secrets => { + load_oauth_tokens_from_secrets_keyring(keyring_store, server_name, url) + } + } +} + +fn load_oauth_tokens_from_direct_keyring( keyring_store: &K, server_name: &str, url: &str, @@ -186,26 +213,70 @@ fn load_oauth_tokens_from_keyring( } } +fn load_oauth_tokens_from_secrets_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result> { + let codex_home = find_codex_home()?; + let backend = + LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(keyring_store.clone())); + let secret_name = compute_secret_name(server_name, url)?; + match backend + .get(&SecretScope::Global, &secret_name) + .context("failed to load MCP OAuth tokens from encrypted storage")? + { + Some(serialized) => { + let mut tokens: StoredOAuthTokens = serde_json::from_str(&serialized) + .context("failed to deserialize OAuth tokens from encrypted storage")?; + refresh_expires_in_from_timestamp(&mut tokens); + Ok(Some(tokens)) + } + None => load_oauth_tokens_from_direct_keyring(keyring_store, server_name, url), + } +} + pub fn save_oauth_tokens( server_name: &str, tokens: &StoredOAuthTokens, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result<()> { let keyring_store = DefaultKeyringStore; match store_mode { OAuthCredentialsStoreMode::Auto => save_oauth_tokens_with_keyring_with_fallback_to_file( &keyring_store, + keyring_backend_kind, server_name, tokens, ), OAuthCredentialsStoreMode::File => save_oauth_tokens_to_file(tokens), - OAuthCredentialsStoreMode::Keyring => { - save_oauth_tokens_with_keyring(&keyring_store, server_name, tokens) + OAuthCredentialsStoreMode::Keyring => save_oauth_tokens_with_keyring( + &keyring_store, + keyring_backend_kind, + server_name, + tokens, + ), + } +} + +fn save_oauth_tokens_with_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + tokens: &StoredOAuthTokens, +) -> Result<()> { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + save_oauth_tokens_to_direct_keyring(keyring_store, server_name, tokens) + } + AuthKeyringBackendKind::Secrets => { + save_oauth_tokens_to_secrets_keyring(keyring_store, server_name, tokens) } } } -fn save_oauth_tokens_with_keyring( +fn save_oauth_tokens_to_direct_keyring( keyring_store: &K, server_name: &str, tokens: &StoredOAuthTokens, @@ -231,12 +302,37 @@ fn save_oauth_tokens_with_keyring( } } -fn save_oauth_tokens_with_keyring_with_fallback_to_file( +fn save_oauth_tokens_to_secrets_keyring( keyring_store: &K, server_name: &str, tokens: &StoredOAuthTokens, ) -> Result<()> { - match save_oauth_tokens_with_keyring(keyring_store, server_name, tokens) { + let serialized = serde_json::to_string(tokens).context("failed to serialize OAuth tokens")?; + let codex_home = find_codex_home()?; + let backend = + LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(keyring_store.clone())); + let secret_name = compute_secret_name(server_name, &tokens.url)?; + backend + .set(&SecretScope::Global, &secret_name, &serialized) + .context("failed to write OAuth tokens to encrypted storage")?; + + let key = compute_store_key(server_name, &tokens.url)?; + if let Err(error) = delete_oauth_tokens_from_file(&key) { + warn!("failed to remove OAuth tokens from fallback storage: {error:?}"); + } + if let Err(error) = keyring_store.delete(KEYRING_SERVICE, &key) { + warn!("failed to remove legacy OAuth tokens from direct keyring storage: {error:?}"); + } + Ok(()) +} + +fn save_oauth_tokens_with_keyring_with_fallback_to_file( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + tokens: &StoredOAuthTokens, +) -> Result<()> { + match save_oauth_tokens_with_keyring(keyring_store, keyring_backend_kind, server_name, tokens) { Ok(()) => Ok(()), Err(error) => { let message = error.to_string(); @@ -251,28 +347,36 @@ pub fn delete_oauth_tokens( server_name: &str, url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { let keyring_store = DefaultKeyringStore; - delete_oauth_tokens_from_keyring_and_file(&keyring_store, store_mode, server_name, url) + delete_oauth_tokens_from_keyring_and_file( + &keyring_store, + store_mode, + keyring_backend_kind, + server_name, + url, + ) } -fn delete_oauth_tokens_from_keyring_and_file( +fn delete_oauth_tokens_from_keyring_and_file( keyring_store: &K, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, server_name: &str, url: &str, ) -> Result { let key = compute_store_key(server_name, url)?; - let keyring_result = keyring_store.delete(KEYRING_SERVICE, &key); + let keyring_result = + delete_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url); let keyring_removed = match keyring_result { Ok(removed) => removed, Err(error) => { - let message = error.message(); + let message = error.to_string(); warn!("failed to delete OAuth tokens from keyring: {message}"); match store_mode { OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::Keyring => { - return Err(error.into_error()) - .context("failed to delete OAuth tokens from keyring"); + return Err(error).context("failed to delete OAuth tokens from keyring"); } OAuthCredentialsStoreMode::File => false, } @@ -283,6 +387,49 @@ fn delete_oauth_tokens_from_keyring_and_file( Ok(keyring_removed || file_removed) } +fn delete_oauth_tokens_from_keyring( + keyring_store: &K, + keyring_backend_kind: AuthKeyringBackendKind, + server_name: &str, + url: &str, +) -> Result { + match keyring_backend_kind { + AuthKeyringBackendKind::Direct => { + delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url) + } + AuthKeyringBackendKind::Secrets => { + delete_oauth_tokens_from_secrets_keyring(keyring_store, server_name, url) + } + } +} + +fn delete_oauth_tokens_from_direct_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result { + let key = compute_store_key(server_name, url)?; + keyring_store + .delete(KEYRING_SERVICE, &key) + .map_err(|error| Error::new(error.into_error())) +} + +fn delete_oauth_tokens_from_secrets_keyring( + keyring_store: &K, + server_name: &str, + url: &str, +) -> Result { + let codex_home = find_codex_home()?; + let backend = + LocalSecretsBackend::new(codex_home.to_path_buf(), Arc::new(keyring_store.clone())); + let secret_name = compute_secret_name(server_name, url)?; + let secrets_removed = backend + .delete(&SecretScope::Global, &secret_name) + .context("failed to delete OAuth tokens from encrypted storage")?; + let direct_removed = delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url)?; + Ok(secrets_removed || direct_removed) +} + #[derive(Clone)] pub(crate) struct OAuthPersistor { inner: Arc, @@ -293,6 +440,7 @@ struct OAuthPersistorInner { url: String, authorization_manager: Arc>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, last_credentials: Mutex>, } @@ -302,6 +450,7 @@ impl OAuthPersistor { url: String, authorization_manager: Arc>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, initial_credentials: Option, ) -> Self { Self { @@ -310,6 +459,7 @@ impl OAuthPersistor { url, authorization_manager, store_mode, + keyring_backend_kind, last_credentials: Mutex::new(initial_credentials), }), } @@ -349,7 +499,12 @@ impl OAuthPersistor { expires_at, }; if last_credentials.as_ref() != Some(&stored) { - save_oauth_tokens(&self.inner.server_name, &stored, self.inner.store_mode)?; + save_oauth_tokens( + &self.inner.server_name, + &stored, + self.inner.store_mode, + self.inner.keyring_backend_kind, + )?; *last_credentials = Some(stored); } } @@ -360,6 +515,7 @@ impl OAuthPersistor { &self.inner.server_name, &self.inner.url, self.inner.store_mode, + self.inner.keyring_backend_kind, ) { warn!( @@ -561,6 +717,15 @@ fn compute_store_key(server_name: &str, server_url: &str) -> Result { Ok(format!("{server_name}|{truncated}")) } +fn compute_secret_name(server_name: &str, server_url: &str) -> Result { + let key = compute_store_key(server_name, server_url)?; + let mut hasher = Sha256::new(); + hasher.update(key.as_bytes()); + let digest = hasher.finalize(); + let hex = format!("{digest:X}"); + SecretName::new(&format!("{MCP_OAUTH_SECRET_PREFIX}_{}", &hex[..32])) +} + fn fallback_file_path() -> Result { Ok(find_codex_home()?.join(FALLBACK_FILENAME).to_path_buf()) } @@ -629,8 +794,10 @@ fn sha_256_prefix(value: &Value) -> Result { mod tests { use super::*; use anyhow::Result; + use codex_secrets::compute_keyring_account; use keyring::Error as KeyringError; use pretty_assertions::assert_eq; + use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; use std::sync::OnceLock; @@ -660,6 +827,10 @@ mod tests { _dir: dir, } } + + fn path(&self) -> &std::path::Path { + self._dir.path() + } } impl Drop for TempCodexHome { @@ -680,9 +851,13 @@ mod tests { let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; store.save(KEYRING_SERVICE, &key, &serialized)?; - let loaded = - super::load_oauth_tokens_from_keyring(&store, &tokens.server_name, &tokens.url)? - .expect("tokens should load from keyring"); + let loaded = super::load_oauth_tokens_from_keyring( + &store, + AuthKeyringBackendKind::Direct, + &tokens.server_name, + &tokens.url, + )? + .expect("tokens should load from keyring"); assert_tokens_match_without_expiry(&loaded, &expected); Ok(()) } @@ -698,6 +873,7 @@ mod tests { let loaded = super::load_oauth_tokens_from_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )? @@ -719,6 +895,7 @@ mod tests { let loaded = super::load_oauth_tokens_from_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )? @@ -738,6 +915,7 @@ mod tests { super::save_oauth_tokens_with_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens, )?; @@ -759,6 +937,7 @@ mod tests { super::save_oauth_tokens_with_keyring_with_fallback_to_file( &store, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens, )?; @@ -779,6 +958,114 @@ mod tests { Ok(()) } + #[test] + fn save_oauth_tokens_with_secrets_backend_writes_encrypted_storage() -> Result<()> { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + let serialized = serde_json::to_string(&tokens)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_to_file(&tokens)?; + + super::save_oauth_tokens_with_keyring_with_fallback_to_file( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + + let backend = LocalSecretsBackend::new(env.path().to_path_buf(), Arc::new(store.clone())); + let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?; + let stored = backend + .get(&SecretScope::Global, &secret_name)? + .expect("tokens should be saved to encrypted storage"); + assert_eq!(serde_json::from_str::(&stored)?, tokens); + assert!(store.saved_value(&key).is_none()); + assert!(!super::fallback_file_path()?.exists()); + Ok(()) + } + + #[test] + fn load_oauth_tokens_with_secrets_backend_reads_legacy_direct_entry() -> Result<()> { + let _env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let expected = tokens.clone(); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + let serialized = serde_json::to_string(&tokens)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + + let loaded = super::load_oauth_tokens_from_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens.url, + )? + .expect("tokens should load from legacy direct keyring storage"); + + assert_tokens_match_without_expiry(&loaded, &expected); + Ok(()) + } + + #[test] + fn save_oauth_tokens_with_secrets_backend_falls_back_to_file_when_keyring_fails() -> Result<()> + { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + store.set_error( + &compute_keyring_account(env.path()), + KeyringError::Invalid("error".into(), "save".into()), + ); + let tokens = sample_tokens(); + + super::save_oauth_tokens_with_keyring_with_fallback_to_file( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + + let saved = super::read_fallback_file()?.expect("fallback file should load"); + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + assert!(saved.contains_key(&key)); + Ok(()) + } + + #[test] + fn delete_oauth_tokens_with_secrets_backend_removes_secrets_direct_and_file() -> Result<()> { + let env = TempCodexHome::new(); + let store = MockKeyringStore::default(); + let tokens = sample_tokens(); + let serialized = serde_json::to_string(&tokens)?; + let key = super::compute_store_key(&tokens.server_name, &tokens.url)?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_with_keyring( + &store, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens, + )?; + store.save(KEYRING_SERVICE, &key, &serialized)?; + super::save_oauth_tokens_to_file(&tokens)?; + + let removed = super::delete_oauth_tokens_from_keyring_and_file( + &store, + OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Secrets, + &tokens.server_name, + &tokens.url, + )?; + + let backend = LocalSecretsBackend::new(env.path().to_path_buf(), Arc::new(store.clone())); + let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?; + assert!(removed); + assert!(backend.get(&SecretScope::Global, &secret_name)?.is_none()); + assert!(store.saved_value(&key).is_none()); + assert!(!super::fallback_file_path()?.exists()); + Ok(()) + } + #[test] fn delete_oauth_tokens_removes_all_storage() -> Result<()> { let _env = TempCodexHome::new(); @@ -792,6 +1079,7 @@ mod tests { let removed = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )?; @@ -814,6 +1102,7 @@ mod tests { let removed = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, )?; @@ -835,6 +1124,7 @@ mod tests { let result = super::delete_oauth_tokens_from_keyring_and_file( &store, OAuthCredentialsStoreMode::Auto, + AuthKeyringBackendKind::Direct, &tokens.server_name, &tokens.url, ); diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index a416b3d9b0..8c72f320bc 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -29,6 +29,7 @@ use crate::oauth::compute_expires_at_millis; use crate::save_oauth_tokens; use crate::utils::apply_default_headers; use crate::utils::build_default_headers; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; struct OauthHeaders { @@ -81,6 +82,7 @@ pub async fn perform_oauth_login( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -93,6 +95,7 @@ pub async fn perform_oauth_login( server_name, server_url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, scopes, @@ -110,6 +113,7 @@ pub async fn perform_oauth_login_silent( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -122,6 +126,7 @@ pub async fn perform_oauth_login_silent( server_name, server_url, store_mode, + keyring_backend_kind, http_headers, env_http_headers, scopes, @@ -139,6 +144,7 @@ async fn perform_oauth_login_with_browser_output( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -156,6 +162,7 @@ async fn perform_oauth_login_with_browser_output( server_name, server_url, store_mode, + keyring_backend_kind, headers, scopes, oauth_client_id, @@ -175,6 +182,7 @@ pub async fn perform_oauth_login_return_url( server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_headers: Option>, env_http_headers: Option>, scopes: &[String], @@ -192,6 +200,7 @@ pub async fn perform_oauth_login_return_url( server_name, server_url, store_mode, + keyring_backend_kind, headers, scopes, oauth_client_id, @@ -350,6 +359,7 @@ struct OauthLoginFlow { server_name: String, server_url: String, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, launch_browser: bool, timeout: Duration, } @@ -445,6 +455,7 @@ impl OauthLoginFlow { server_name: &str, server_url: &str, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, headers: OauthHeaders, scopes: &[String], oauth_client_id: Option<&str>, @@ -508,6 +519,7 @@ impl OauthLoginFlow { server_name: server_name.to_string(), server_url: server_url.to_string(), store_mode, + keyring_backend_kind, launch_browser, timeout, }) @@ -571,7 +583,12 @@ impl OauthLoginFlow { token_response: WrappedOAuthTokenResponse(credentials), expires_at, }; - save_oauth_tokens(&self.server_name, &stored, self.store_mode)?; + save_oauth_tokens( + &self.server_name, + &stored, + self.store_mode, + self.keyring_backend_kind, + )?; Ok(()) } diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index a28403e67c..7904668c5b 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -12,7 +12,9 @@ use std::time::Instant; use anyhow::Result; use anyhow::anyhow; use codex_api::SharedAuthProvider; +use codex_client::build_reqwest_client_with_custom_ca; use codex_client::maybe_build_rustls_client_config_with_custom_ca; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::McpServerEnvVar; use codex_exec_server::HttpClient; use futures::FutureExt; @@ -124,6 +126,7 @@ enum TransportRecipe { http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_client: Arc, auth_provider: Option, }, @@ -371,6 +374,7 @@ impl RmcpClient { http_headers: Option>, env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, http_client: Arc, auth_provider: Option, ) -> Result { @@ -381,6 +385,7 @@ impl RmcpClient { http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, auth_provider, }; @@ -757,6 +762,7 @@ impl RmcpClient { http_headers, env_http_headers, store_mode, + keyring_backend_kind, http_client, auth_provider, } => { @@ -767,7 +773,7 @@ impl RmcpClient { && auth_provider.is_none() && !default_headers.contains_key(AUTHORIZATION) { - match load_oauth_tokens(server_name, url, *store_mode) { + match load_oauth_tokens(server_name, url, *store_mode, *keyring_backend_kind) { Ok(tokens) => tokens, Err(err) => { warn!("failed to read tokens for server `{server_name}`: {err}"); @@ -784,6 +790,7 @@ impl RmcpClient { url, initial_tokens.clone(), *store_mode, + *keyring_backend_kind, default_headers.clone(), Arc::clone(http_client), ) @@ -1127,6 +1134,7 @@ async fn create_oauth_transport_and_runtime( url: &str, initial_tokens: StoredOAuthTokens, credentials_store: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, default_headers: HeaderMap, http_client: Arc, ) -> Result<( @@ -1175,6 +1183,7 @@ async fn create_oauth_transport_and_runtime( url.to_string(), auth_manager, credentials_store, + keyring_backend_kind, Some(initial_tokens), ); diff --git a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs index 6b5df6e297..87a52f55df 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs @@ -17,6 +17,7 @@ use std::time::Duration; use std::time::Instant; use anyhow::Context as _; +use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_exec_server::ExecServerClient; @@ -93,6 +94,7 @@ pub(crate) async fn create_client_with_http_client( /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), http_client, /*auth_provider*/ None, ) @@ -136,6 +138,7 @@ pub(crate) async fn create_remote_client( /*http_headers*/ None, /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), Arc::new(http_client), /*auth_provider*/ None, ) diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 0c759f3237..ef9855276a 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1227,7 +1227,7 @@ pub async fn run_main( if let Err(err) = enforce_login_restrictions(&AuthConfig { codex_home: config.codex_home.to_path_buf(), auth_credentials_store_mode: config.cli_auth_credentials_store_mode, - keyring_backend_kind: config.cli_auth_keyring_backend_kind(), + keyring_backend_kind: config.auth_keyring_backend_kind(), 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()),