Clarify OAuth store policy and pinning names

This commit is contained in:
Steven Lee
2026-07-08 16:48:57 +00:00
parent d493019b6d
commit 0a49007296
4 changed files with 19 additions and 18 deletions
+6 -6
View File
@@ -69,7 +69,7 @@ use codex_utils_home_dir::find_codex_home;
pub(crate) use self::resolved_store::ResolvedOAuthCredentialStore;
pub(crate) use self::resolved_store::ResolvedOAuthTokens;
pub(crate) use self::resolved_store::resolve_oauth_tokens;
pub(crate) use self::resolved_store::resolve_oauth_tokens_from_store_policy;
const KEYRING_SERVICE: &str = "Codex MCP Credentials";
const MCP_OAUTH_SECRET_PREFIX: &str = "MCP_OAUTH";
@@ -111,7 +111,7 @@ pub(crate) fn oauth_token_status(
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
) -> Result<StoredOAuthTokenStatus> {
let resolved = resolve_oauth_tokens(
let resolved = resolve_oauth_tokens_from_store_policy(
&DefaultKeyringStore,
server_name,
url,
@@ -862,7 +862,7 @@ mod tests {
use super::test_support::TempCodexHome;
#[test]
fn resolve_oauth_tokens_uses_keyring_when_available() -> Result<()> {
fn resolve_oauth_tokens_from_store_policy_uses_keyring_when_available() -> Result<()> {
let _env = TempCodexHome::new();
let store = MockKeyringStore::default();
let tokens = sample_tokens();
@@ -871,7 +871,7 @@ mod tests {
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
let resolved = super::resolve_oauth_tokens(
let resolved = super::resolve_oauth_tokens_from_store_policy(
&store,
&tokens.server_name,
&tokens.url,
@@ -896,7 +896,7 @@ mod tests {
super::save_oauth_tokens_to_file(&tokens)?;
let resolved = super::resolve_oauth_tokens(
let resolved = super::resolve_oauth_tokens_from_store_policy(
&store,
&tokens.server_name,
&tokens.url,
@@ -920,7 +920,7 @@ mod tests {
super::save_oauth_tokens_to_file(&tokens)?;
let resolved = super::resolve_oauth_tokens(
let resolved = super::resolve_oauth_tokens_from_store_policy(
&store,
&tokens.server_name,
&tokens.url,
@@ -33,7 +33,8 @@ pub(crate) enum ResolvedOAuthCredentialStore {
impl ResolvedOAuthCredentialStore {
/// Loads credentials only from this already-resolved authority.
///
/// Unlike `resolve_oauth_tokens`, this never evaluates configured `Auto` fallback policy.
/// Unlike `resolve_oauth_tokens_from_store_policy`, this never evaluates configured
/// `Auto` fallback policy.
pub(crate) fn load<K: KeyringStore + Clone + 'static>(
self,
keyring_store: &K,
@@ -102,7 +103,7 @@ pub(crate) struct ResolvedOAuthTokens {
pub(crate) store: ResolvedOAuthCredentialStore,
}
pub(crate) fn resolve_oauth_tokens<K: KeyringStore + Clone + 'static>(
pub(crate) fn resolve_oauth_tokens_from_store_policy<K: KeyringStore + Clone + 'static>(
keyring_store: &K,
server_name: &str,
url: &str,
@@ -31,7 +31,7 @@ use crate::oauth::WrappedOAuthTokenResponse;
use crate::oauth::fallback_file_path;
use crate::oauth::load_oauth_tokens_from_file;
use crate::oauth::load_oauth_tokens_from_keyring;
use crate::oauth::resolve_oauth_tokens;
use crate::oauth::resolve_oauth_tokens_from_store_policy;
use crate::oauth::save_oauth_tokens_to_file;
use crate::oauth::save_oauth_tokens_to_file_with_lock_held;
use crate::oauth::save_oauth_tokens_to_secrets_keyring_with_lock_held;
@@ -214,7 +214,7 @@ fn auto_load_secrets_lock_failure_does_not_fall_back_to_file() -> Result<()> {
let lock_dir = env.path().join("mcp-oauth-locks");
std::fs::create_dir(lock_dir.join("secrets-store.lock"))?;
let error = resolve_oauth_tokens(
let error = resolve_oauth_tokens_from_store_policy(
&keyring_store,
&tokens.server_name,
&tokens.url,
+8 -8
View File
@@ -70,7 +70,7 @@ use crate::oauth::OAuthPersistor;
use crate::oauth::ResolvedOAuthCredentialStore;
use crate::oauth::ResolvedOAuthTokens;
use crate::oauth::StoredOAuthTokens;
use crate::oauth::resolve_oauth_tokens;
use crate::oauth::resolve_oauth_tokens_from_store_policy;
use crate::oauth_http_client::OAuthHttpClientAdapter;
use crate::stdio_server_launcher::StdioServerCommand;
use crate::stdio_server_launcher::StdioServerLauncher;
@@ -130,7 +130,7 @@ enum TransportRecipe {
env_http_headers: Option<HashMap<String, String>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
resolved_store: Arc<OnceLock<ResolvedOAuthCredentialStore>>,
pinned_credential_store: Arc<OnceLock<ResolvedOAuthCredentialStore>>,
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
},
@@ -407,7 +407,7 @@ impl RmcpClient {
env_http_headers,
store_mode,
keyring_backend_kind,
resolved_store: Arc::new(OnceLock::new()),
pinned_credential_store: Arc::new(OnceLock::new()),
http_client,
auth_provider,
};
@@ -787,7 +787,7 @@ impl RmcpClient {
env_http_headers,
store_mode,
keyring_backend_kind,
resolved_store,
pinned_credential_store,
http_client,
auth_provider,
} => {
@@ -804,14 +804,14 @@ impl RmcpClient {
&& auth_provider.is_none()
&& !default_headers.contains_key(AUTHORIZATION)
{
if let Some(store) = resolved_store.get().copied() {
if let Some(store) = pinned_credential_store.get().copied() {
// Rebuilds reread the source selected during first construction. Only the
// initial construction below evaluates configured store policy.
store
.load(&DefaultKeyringStore, server_name, url)?
.map(|tokens| ResolvedOAuthTokens { tokens, store })
} else {
match resolve_oauth_tokens(
match resolve_oauth_tokens_from_store_policy(
&DefaultKeyringStore,
server_name,
url,
@@ -822,9 +822,9 @@ impl RmcpClient {
if let Some(resolved) = tokens.as_ref() {
// Retries and session recovery rebuild this transport. Pin the
// first concrete source so Auto is not reevaluated mid-client.
resolved_store.set(resolved.store).map_err(|_| {
pinned_credential_store.set(resolved.store).map_err(|_| {
anyhow!(
"OAuth credential store resolved concurrently for MCP server `{server_name}`"
"OAuth credential store pinned concurrently for MCP server `{server_name}`"
)
})?;
}