feat: use encrypted local secrets for MCP OAuth (#27541)

## Summary

- store MCP OAuth credentials in the configured auth credential backend
- support encrypted-local OAuth storage, including legacy keyring
migration
- propagate the credential backend through MCP refresh, session, CLI,
and app-server paths

## Stack

1. #27504 — config and feature flag
2. #27535 — auth-specific secret namespaces
3. #27539 — encrypted CLI auth storage
4. this PR — encrypted MCP OAuth storage

This is a parallel review stack; the original #17931 remains unchanged.

## Tests

- `just test -p codex-rmcp-client` (the transport round-trip test passed
after building the required `codex` binary and retrying)
- `just test -p codex-mcp`
- `just test -p codex-app-server
refresh_config_uses_latest_auth_keyring_backend`
- `just test -p codex-core
refresh_mcp_servers_is_deferred_until_next_turn`
- `just test -p codex-cli mcp`
- `just fix -p codex-rmcp-client -p codex-mcp -p codex-core -p codex-cli
-p codex-app-server -p codex-protocol`
- `just bazel-lock-check`
This commit is contained in:
Celia Chen
2026-06-12 15:03:51 -07:00
committed by GitHub
Unverified
parent 4d1702586a
commit 9915d34684
27 changed files with 568 additions and 38 deletions
+1
View File
@@ -3660,6 +3660,7 @@ dependencies = [
"codex-exec-server",
"codex-keyring-store",
"codex-protocol",
"codex-secrets",
"codex-utils-cargo-bin",
"codex-utils-home-dir",
"codex-utils-pty",
+39
View File
@@ -71,6 +71,8 @@ async fn build_refresh_config(
config.mcp_oauth_credentials_store_mode,
)
.map_err(io::Error::other)?,
auth_keyring_backend_kind: serde_json::to_value(config.auth_keyring_backend_kind())
.map_err(io::Error::other)?,
})
}
@@ -104,6 +106,7 @@ mod tests {
use codex_config::ThreadConfigLoadErrorCode;
use codex_config::ThreadConfigLoader;
use codex_config::ThreadConfigSource;
use codex_config::types::AuthKeyringBackendKind;
use codex_core::config::ConfigOverrides;
use codex_core::init_state_db;
use codex_core::thread_store_from_config;
@@ -142,6 +145,38 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn refresh_config_uses_latest_auth_keyring_backend() -> anyhow::Result<()> {
let (temp_dir, thread_manager, config_manager, _loader) = refresh_test_state().await?;
std::fs::write(
temp_dir.path().join(codex_config::CONFIG_TOML_FILE),
"[features]\nsecret_auth_storage = true\n",
)?;
let mut good_thread = None;
for thread_id in thread_manager.list_thread_ids().await {
let thread = thread_manager.get_thread(thread_id).await?;
let thread_config = thread.config().await;
if thread_config.cwd.ends_with("good") {
good_thread = Some(thread);
break;
}
}
let thread = good_thread.expect("good test thread should exist");
let refresh_config = build_refresh_config(thread.as_ref(), &config_manager).await?;
let backend = serde_json::from_value::<AuthKeyringBackendKind>(
refresh_config.auth_keyring_backend_kind,
)?;
assert_eq!(
thread.config().await.auth_keyring_backend_kind(),
AuthKeyringBackendKind::Direct
);
assert_eq!(backend, AuthKeyringBackendKind::Secrets);
Ok(())
}
async fn refresh_test_state() -> anyhow::Result<(
TempDir,
Arc<ThreadManager>,
@@ -153,6 +188,10 @@ mod tests {
let bad_cwd = temp_dir.path().join("bad");
std::fs::create_dir_all(&good_cwd)?;
std::fs::create_dir_all(&bad_cwd)?;
std::fs::write(
temp_dir.path().join(codex_config::CONFIG_TOML_FILE),
"[features]\nsecret_auth_storage = false\n",
)?;
let initial_config_manager =
ConfigManager::without_managed_config_for_tests(temp_dir.path().to_path_buf());
@@ -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,
@@ -1701,6 +1701,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);
@@ -1712,6 +1713,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,
@@ -1728,6 +1730,7 @@ impl PluginRequestProcessor {
&name,
&oauth_config.url,
store_mode,
keyring_backend_kind,
oauth_config.http_headers,
oauth_config.env_http_headers,
&[],
+12 -1
View File
@@ -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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
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;
@@ -42,6 +42,7 @@ use anyhow::anyhow;
use async_channel::Sender;
use codex_config::Constrained;
use codex_config::McpServerTransportConfig;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_login::CodexAuth;
use codex_protocol::mcp::CallToolResult;
@@ -119,6 +120,7 @@ impl McpConnectionManager {
pub async fn new(
mcp_servers: &HashMap<String, EffectiveMcpServer>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
auth_entries: HashMap<String, McpAuthStatusEntry>,
approval_policy: &Constrained<AskForApproval>,
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(),
@@ -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(),
+13 -1
View File
@@ -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<String, McpAuthStatusEntry>
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<McpAuthStatus> {
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
}
+7
View File
@@ -21,6 +21,7 @@ use codex_config::Constrained;
use codex_config::McpServerConfig;
use codex_config::McpServerTransportConfig;
use codex_config::types::AppToolApproval;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_login::CodexAuth;
use codex_plugin::PluginCapabilitySummary;
@@ -114,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<u16>,
/// Optional OAuth redirect URI override for MCP login.
@@ -262,6 +265,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;
@@ -271,6 +275,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(),
@@ -330,6 +335,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;
@@ -343,6 +349,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,
+2
View File
@@ -2,6 +2,7 @@ use super::*;
use crate::McpServerRegistration;
use codex_config::Constrained;
use codex_config::types::AppToolApproval;
use codex_config::types::AuthKeyringBackendKind;
use codex_login::CodexAuth;
use codex_plugin::AppConnectorId;
use codex_plugin::PluginCapabilitySummary;
@@ -20,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,
+5
View File
@@ -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<Event>,
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<SharedAuthProvider>,
) -> Result<RmcpClient, StartupOutcomeError> {
@@ -648,6 +652,7 @@ async fn make_rmcp_client(
http_headers,
env_http_headers,
store_mode,
keyring_backend_kind,
http_client,
runtime_auth_provider,
)
+1
View File
@@ -1443,6 +1443,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
+2
View File
@@ -280,6 +280,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;
@@ -291,6 +292,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(),
@@ -152,6 +152,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,
@@ -168,6 +169,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,
&[],
@@ -202,6 +204,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
turn_context,
refresh_servers,
config.mcp_oauth_credentials_store_mode,
config.auth_keyring_backend_kind(),
elicitation_reviewer,
)
.await;
+1
View File
@@ -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(),
+35 -6
View File
@@ -299,6 +299,7 @@ impl Session {
turn_context: &TurnContext,
mcp_servers: HashMap<String, McpServerConfig>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
elicitation_reviewer: Option<ElicitationReviewerHandle>,
) {
let auth = self.services.auth_manager.auth().await;
@@ -309,8 +310,13 @@ 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 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),
@@ -332,6 +338,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(),
@@ -371,6 +378,7 @@ impl Session {
let McpServerRefreshConfig {
mcp_servers,
mcp_oauth_credentials_store_mode,
auth_keyring_backend_kind,
} = refresh_config;
let mcp_servers =
@@ -390,9 +398,23 @@ impl Session {
return;
}
};
let keyring_backend_kind =
match serde_json::from_value::<AuthKeyringBackendKind>(auth_keyring_backend_kind) {
Ok(kind) => kind,
Err(err) => {
warn!("failed to parse MCP auth keyring backend refresh config: {err}");
return;
}
};
self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer)
.await;
self.refresh_mcp_servers_inner(
turn_context,
mcp_servers,
store_mode,
keyring_backend_kind,
elicitation_reviewer,
)
.await;
}
pub(crate) async fn refresh_mcp_servers_now(
@@ -400,10 +422,17 @@ impl Session {
turn_context: &TurnContext,
mcp_servers: HashMap<String, McpServerConfig>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
elicitation_reviewer: Option<ElicitationReviewerHandle>,
) {
self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer)
.await;
self.refresh_mcp_servers_inner(
turn_context,
mcp_servers,
store_mode,
keyring_backend_kind,
elicitation_reviewer,
)
.await;
}
#[cfg(test)]
+1
View File
@@ -50,6 +50,7 @@ use codex_analytics::SubAgentThreadStartedInput;
use codex_analytics::TurnCodexErrorFact;
use codex_app_server_protocol::McpServerElicitationRequest;
use codex_app_server_protocol::McpServerElicitationRequestParams;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_exec_server::Environment;
use codex_exec_server::EnvironmentManager;
+2
View File
@@ -614,6 +614,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;
@@ -1145,6 +1146,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(),
+3
View File
@@ -7106,9 +7106,12 @@ async fn refresh_mcp_servers_is_deferred_until_next_turn() {
let mcp_oauth_credentials_store_mode =
serde_json::to_value(OAuthCredentialsStoreMode::Auto).expect("serialize store mode");
let auth_keyring_backend_kind =
serde_json::to_value(AuthKeyringBackendKind::Secrets).expect("serialize keyring backend");
let refresh_config = McpServerRefreshConfig {
mcp_servers: json!({}),
mcp_oauth_credentials_store_mode,
auth_keyring_backend_kind,
};
{
let mut guard = session.pending_mcp_server_refresh_config.lock().await;
+1
View File
@@ -181,6 +181,7 @@ pub struct McpServerRefreshConfig {
pub mcp_servers: Value,
/// OAuth credential store mode to use with this server snapshot.
pub mcp_oauth_credentials_store_mode: Value,
pub auth_keyring_backend_kind: Value,
}
#[derive(Debug, Clone, PartialEq)]
+1
View File
@@ -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 }
+5 -1
View File
@@ -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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<McpAuthStatus> {
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");
+368 -23
View File
@@ -19,7 +19,13 @@
use anyhow::Context;
use anyhow::Error;
use anyhow::Result;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_secrets::LocalSecretsNamespace;
use codex_secrets::SecretName;
use codex_secrets::SecretScope;
use codex_secrets::SecretsBackendKind;
use codex_secrets::SecretsManager;
use oauth2::AccessToken;
use oauth2::RefreshToken;
use oauth2::Scope;
@@ -51,6 +57,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 +94,19 @@ pub(crate) fn load_oauth_tokens(
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<Option<StoredOAuthTokens>> {
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 +116,10 @@ pub(crate) fn oauth_token_status(
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<StoredOAuthTokenStatus> {
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 +164,13 @@ fn refresh_expires_in_from_timestamp(tokens: &mut StoredOAuthTokens) {
}
}
fn load_oauth_tokens_from_keyring_with_fallback_to_file<K: KeyringStore>(
fn load_oauth_tokens_from_keyring_with_fallback_to_file<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
keyring_backend_kind: AuthKeyringBackendKind,
server_name: &str,
url: &str,
) -> Result<Option<StoredOAuthTokens>> {
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 +181,23 @@ fn load_oauth_tokens_from_keyring_with_fallback_to_file<K: KeyringStore>(
}
}
fn load_oauth_tokens_from_keyring<K: KeyringStore>(
fn load_oauth_tokens_from_keyring<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
keyring_backend_kind: AuthKeyringBackendKind,
server_name: &str,
url: &str,
) -> Result<Option<StoredOAuthTokens>> {
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<K: KeyringStore>(
keyring_store: &K,
server_name: &str,
url: &str,
@@ -186,26 +215,74 @@ fn load_oauth_tokens_from_keyring<K: KeyringStore>(
}
}
fn load_oauth_tokens_from_secrets_keyring<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
server_name: &str,
url: &str,
) -> Result<Option<StoredOAuthTokens>> {
let codex_home = find_codex_home()?;
let manager = SecretsManager::new_with_keyring_store_and_namespace(
codex_home.to_path_buf(),
SecretsBackendKind::Local,
Arc::new(keyring_store.clone()),
LocalSecretsNamespace::McpOAuth,
);
let secret_name = compute_secret_name(server_name, url)?;
match manager
.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 => Ok(None),
}
}
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<K: KeyringStore + Clone + 'static>(
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<K: KeyringStore>(
fn save_oauth_tokens_to_direct_keyring<K: KeyringStore>(
keyring_store: &K,
server_name: &str,
tokens: &StoredOAuthTokens,
@@ -231,12 +308,38 @@ fn save_oauth_tokens_with_keyring<K: KeyringStore>(
}
}
fn save_oauth_tokens_with_keyring_with_fallback_to_file<K: KeyringStore>(
fn save_oauth_tokens_to_secrets_keyring<K: KeyringStore + Clone + 'static>(
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 manager = SecretsManager::new_with_keyring_store_and_namespace(
codex_home.to_path_buf(),
SecretsBackendKind::Local,
Arc::new(keyring_store.clone()),
LocalSecretsNamespace::McpOAuth,
);
let secret_name = compute_secret_name(server_name, &tokens.url)?;
manager
.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:?}");
}
Ok(())
}
fn save_oauth_tokens_with_keyring_with_fallback_to_file<K: KeyringStore + Clone + 'static>(
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 +354,36 @@ pub fn delete_oauth_tokens(
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<bool> {
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<K: KeyringStore>(
fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
server_name: &str,
url: &str,
) -> Result<bool> {
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 +394,56 @@ fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore>(
Ok(keyring_removed || file_removed)
}
fn delete_oauth_tokens_from_keyring<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
keyring_backend_kind: AuthKeyringBackendKind,
server_name: &str,
url: &str,
) -> Result<bool> {
match keyring_backend_kind {
AuthKeyringBackendKind::Direct => {
delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url)
}
AuthKeyringBackendKind::Secrets => {
let direct_removed =
delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url)?;
let secrets_removed =
delete_oauth_tokens_from_secrets_keyring(keyring_store, server_name, url)?;
Ok(direct_removed || secrets_removed)
}
}
}
fn delete_oauth_tokens_from_direct_keyring<K: KeyringStore>(
keyring_store: &K,
server_name: &str,
url: &str,
) -> Result<bool> {
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<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
server_name: &str,
url: &str,
) -> Result<bool> {
let codex_home = find_codex_home()?;
let manager = SecretsManager::new_with_keyring_store_and_namespace(
codex_home.to_path_buf(),
SecretsBackendKind::Local,
Arc::new(keyring_store.clone()),
LocalSecretsNamespace::McpOAuth,
);
let secret_name = compute_secret_name(server_name, url)?;
let secrets_removed = manager
.delete(&SecretScope::Global, &secret_name)
.context("failed to delete OAuth tokens from encrypted storage")?;
Ok(secrets_removed)
}
#[derive(Clone)]
pub(crate) struct OAuthPersistor {
inner: Arc<OAuthPersistorInner>,
@@ -293,6 +454,7 @@ struct OAuthPersistorInner {
url: String,
authorization_manager: Arc<Mutex<AuthorizationManager>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
last_credentials: Mutex<Option<StoredOAuthTokens>>,
}
@@ -302,6 +464,7 @@ impl OAuthPersistor {
url: String,
authorization_manager: Arc<Mutex<AuthorizationManager>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
initial_credentials: Option<StoredOAuthTokens>,
) -> Self {
Self {
@@ -310,6 +473,7 @@ impl OAuthPersistor {
url,
authorization_manager,
store_mode,
keyring_backend_kind,
last_credentials: Mutex::new(initial_credentials),
}),
}
@@ -349,7 +513,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 +529,7 @@ impl OAuthPersistor {
&self.inner.server_name,
&self.inner.url,
self.inner.store_mode,
self.inner.keyring_backend_kind,
)
{
warn!(
@@ -561,6 +731,21 @@ fn compute_store_key(server_name: &str, server_url: &str) -> Result<String> {
Ok(format!("{server_name}|{truncated}"))
}
/// Derive a valid secret-store name from the MCP OAuth store key.
///
/// `compute_store_key` intentionally includes readable identity components and
/// a pipe separator, but `SecretName` only allows `A-Z`, `0-9`, and `_`.
/// Re-hashing keeps the secret key deterministic while satisfying that
/// restricted alphabet.
fn compute_secret_name(server_name: &str, server_url: &str) -> Result<SecretName> {
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<PathBuf> {
Ok(find_codex_home()?.join(FALLBACK_FILENAME).to_path_buf())
}
@@ -629,8 +814,10 @@ fn sha_256_prefix(value: &Value) -> Result<String> {
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 +847,10 @@ mod tests {
_dir: dir,
}
}
fn path(&self) -> &std::path::Path {
self._dir.path()
}
}
impl Drop for TempCodexHome {
@@ -680,9 +871,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 +893,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 +915,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 +935,7 @@ mod tests {
super::save_oauth_tokens_with_keyring_with_fallback_to_file(
&store,
AuthKeyringBackendKind::Direct,
&tokens.server_name,
&tokens,
)?;
@@ -759,6 +957,7 @@ mod tests {
super::save_oauth_tokens_with_keyring_with_fallback_to_file(
&store,
AuthKeyringBackendKind::Direct,
&tokens.server_name,
&tokens,
)?;
@@ -779,6 +978,149 @@ 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 manager = SecretsManager::new_with_keyring_store_and_namespace(
env.path().to_path_buf(),
SecretsBackendKind::Local,
Arc::new(store.clone()),
LocalSecretsNamespace::McpOAuth,
);
let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?;
let stored = manager
.get(&SecretScope::Global, &secret_name)?
.expect("tokens should be saved to encrypted storage");
assert_eq!(serde_json::from_str::<StoredOAuthTokens>(&stored)?, tokens);
assert_eq!(store.saved_value(&key), Some(serialized));
assert!(env.path().join("secrets").join("mcp_oauth.age").exists());
assert!(!env.path().join("secrets").join("local.age").exists());
assert!(!super::fallback_file_path()?.exists());
Ok(())
}
#[test]
fn load_oauth_tokens_with_secrets_backend_reads_encrypted_storage() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
let expected = tokens.clone();
super::save_oauth_tokens_with_keyring(
&store,
AuthKeyringBackendKind::Secrets,
&tokens.server_name,
&tokens,
)?;
let loaded = super::load_oauth_tokens_from_keyring(
&store,
AuthKeyringBackendKind::Secrets,
&tokens.server_name,
&tokens.url,
)?
.expect("tokens should load from encrypted storage");
assert_tokens_match_without_expiry(&loaded, &expected);
Ok(())
}
#[test]
fn load_oauth_tokens_with_secrets_backend_ignores_direct_entry() -> 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)?;
let loaded = super::load_oauth_tokens_from_keyring(
&store,
AuthKeyringBackendKind::Secrets,
&tokens.server_name,
&tokens.url,
)?;
assert!(loaded.is_none());
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_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 manager = SecretsManager::new_with_keyring_store_and_namespace(
env.path().to_path_buf(),
SecretsBackendKind::Local,
Arc::new(store.clone()),
LocalSecretsNamespace::McpOAuth,
);
let secret_name = super::compute_secret_name(&tokens.server_name, &tokens.url)?;
assert!(removed);
assert!(manager.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 +1134,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 +1157,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 +1179,7 @@ mod tests {
let result = super::delete_oauth_tokens_from_keyring_and_file(
&store,
OAuthCredentialsStoreMode::Auto,
AuthKeyringBackendKind::Direct,
&tokens.server_name,
&tokens.url,
);
@@ -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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
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<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
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(())
}
+9 -1
View File
@@ -13,6 +13,7 @@ use anyhow::Result;
use anyhow::anyhow;
use codex_api::SharedAuthProvider;
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 +125,7 @@ enum TransportRecipe {
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
},
@@ -371,6 +373,7 @@ impl RmcpClient {
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
) -> Result<Self> {
@@ -381,6 +384,7 @@ impl RmcpClient {
http_headers,
env_http_headers,
store_mode,
keyring_backend_kind,
http_client,
auth_provider,
};
@@ -757,6 +761,7 @@ impl RmcpClient {
http_headers,
env_http_headers,
store_mode,
keyring_backend_kind,
http_client,
auth_provider,
} => {
@@ -767,7 +772,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 +789,7 @@ impl RmcpClient {
url,
initial_tokens.clone(),
*store_mode,
*keyring_backend_kind,
default_headers.clone(),
Arc::clone(http_client),
)
@@ -1127,6 +1133,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<dyn HttpClient>,
) -> Result<(
@@ -1175,6 +1182,7 @@ async fn create_oauth_transport_and_runtime(
url.to_string(),
auth_manager,
credentials_store,
keyring_backend_kind,
Some(initial_tokens),
);
@@ -4,6 +4,7 @@ use std::time::Duration;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
use codex_config::types::AuthKeyringBackendKind;
use codex_config::types::OAuthCredentialsStoreMode;
use codex_exec_server::Environment;
use codex_rmcp_client::McpAuthStatus;
@@ -157,7 +158,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> {
token_response: WrappedOAuthTokenResponse(response),
expires_at: Some(0),
};
save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?;
save_oauth_tokens(
SERVER_NAME,
&tokens,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)?;
let status = auth_status(UNREFRESHABLE_SERVER_URL).await?;
assert_eq!(status, McpAuthStatus::NotLoggedIn);
@@ -178,7 +184,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> {
token_response: WrappedOAuthTokenResponse(response),
expires_at: Some(now.saturating_add(/*rhs*/ 60_000)),
};
save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?;
save_oauth_tokens(
SERVER_NAME,
&tokens,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)?;
let status = auth_status(UNEXPIRED_SERVER_URL).await?;
assert_eq!(status, McpAuthStatus::OAuth);
@@ -196,7 +207,12 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> {
token_response: WrappedOAuthTokenResponse(response),
expires_at: Some(0),
};
save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?;
save_oauth_tokens(
SERVER_NAME,
&tokens,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)?;
let status = auth_status(REFRESHABLE_SERVER_URL).await?;
assert_eq!(status, McpAuthStatus::OAuth);
@@ -211,6 +227,7 @@ async fn auth_status(server_url: &str) -> anyhow::Result<McpAuthStatus> {
/*http_headers*/ None,
/*env_http_headers*/ None,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)
.await
}
@@ -236,7 +253,12 @@ async fn oauth_startup_child() -> anyhow::Result<()> {
token_response: WrappedOAuthTokenResponse(response),
expires_at: Some(0),
};
save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?;
save_oauth_tokens(
SERVER_NAME,
&tokens,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
)?;
// This mirrors create_client's transport and initialization setup, except
// it omits the direct bearer token. Supplying that token would bypass the
@@ -248,6 +270,7 @@ async fn oauth_startup_child() -> anyhow::Result<()> {
/*http_headers*/ None,
/*env_http_headers*/ None,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
Environment::default_for_tests().get_http_client(),
/*auth_provider*/ None,
)
@@ -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,
)