openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-11 21:10:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Protect MCP OAuth persistence from stale credential writes (#60849)

Browse Source 
## Why

A pending token refresh must not overwrite a newer login or recreate credentials after logout. Legacy file entries also need to retain their stored identity across refreshes.

## What changed

- Lock and revalidate stored credentials before persisting changed authorization-manager credentials, requiring authorization again if storage has changed.
- Update file-backed refreshes only when the matched entry still contains the expected credentials, preserving its key, server name, and ownership. Compare only fields the file format can persist.
- Remove matching legacy file entries when saving login credentials or deleting credentials, while preserving unrelated entries and profile-scoped enterprise credentials' separate local entries.

## Testing

Add regression coverage for newer logins and logout during pending persistence, repeated refreshes of legacy and escaped-name entries, and cleanup that preserves unrelated credentials. Update guard-retention and telemetry assertions for the validation path.

GitOrigin-RevId: 3d7c603950afcff72f0bcd943edae5ecfbcb5bd2
This commit is contained in:
Matthew Zeng
2026-11-11 19:18:48 +00:00
committed by copyberry
parent b117b2d6fe
commit 392ced7d3a
8 changed files with 394 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/rmcp-client/src/oauth.rs
+198 -49
View File
@@ -74,6 +74,7 @@ use self::store_lock::OAuthStoreLockFailure;
use codex_config::McpServerOAuthConfig;
use codex_keyring_store::DefaultKeyringStore;
use codex_keyring_store::KeyringStore;
use rmcp::transport::auth::AuthError;
use rmcp::transport::auth::AuthorizationManager;
use tokio::sync::Mutex;
@@ -343,6 +344,21 @@ fn normalized_oauth_credentials(tokens: Option<&StoredOAuthTokens>) -> Option<St
})
}
fn normalized_file_oauth_credentials(
tokens: Option<&StoredOAuthTokens>,
) -> Option<StoredOAuthTokens> {
normalized_oauth_credentials(tokens).map(|mut tokens| {
// Compare the representation File can persist, not vendor-only response fields.
let response = &mut tokens.token_response.0;
response.set_extra_fields(VendorExtraTokenFields::default());
response.set_token_type(BasicTokenType::Bearer);
if response.scopes().is_some_and(Vec::is_empty) {
response.set_scopes(None);
}
tokens
})
}
fn oauth_tokens_are_usable(tokens: &StoredOAuthTokens) -> bool {
if tokens.client_id.trim().is_empty() {
return false;
@@ -588,7 +604,7 @@ fn save_oauth_tokens_with_keyring_and_cleanup_file<K: KeyringStore + Clone + 'st
save_oauth_tokens_with_keyring(keyring_store, keyring_backend_kind, server_name, tokens)?;
let key = compute_store_key(server_name, &tokens.url)?;
let mut observation = telemetry::policy(store_mode, keyring_backend_kind, Operation::Cleanup);
let result = delete_oauth_tokens_from_file(&key);
let result = delete_oauth_tokens_from_file(server_name, &tokens.url, &key);
observation.record_delete_attempt(Store::File, &result);
if result.is_err() {
warn!("failed to remove OAuth tokens from fallback storage");
@@ -688,7 +704,7 @@ fn delete_oauth_tokens_from_keyring_and_file<K: KeyringStore + Clone + 'static>(
}
};
let file_removed = delete_oauth_tokens_from_file(&key)?;
let file_removed = delete_oauth_tokens_from_file(server_name, url, &key)?;
Ok(keyring_removed || file_removed)
});
observation.record_delete_attempt(Store::Multiple, &result);
@@ -760,6 +776,16 @@ struct OAuthPersistorInner {
last_credentials: Mutex<Option<StoredOAuthTokens>>,
}
impl OAuthPersistorInner {
fn refreshed_server_name(&self, previous: Option<&StoredOAuthTokens>) -> String {
// File refreshes update the matched row in place, including its original server name.
match (self.credential_store.backend, previous) {
(resolved_store::Backend::File, Some(previous)) => previous.server_name.clone(),
_ => self.server_name.clone(),
}
}
}
impl OAuthPersistor {
pub(crate) fn new(
server_name: String,
@@ -792,11 +818,77 @@ impl OAuthPersistor {
reason = "AuthorizationManager async access must be serialized through its mutex"
)]
pub(crate) async fn persist_if_needed(&self) -> Result<()> {
let (client_id, maybe_credentials) = {
let manager = self.inner.authorization_manager.clone();
let guard = manager.lock().await;
guard.get_credentials().await
}?;
{
let manager = self.inner.authorization_manager.lock().await;
if !self.credentials_changed(&manager).await? {
return Ok(());
}
}
let credential_guard = self.acquire_persistence_guard().await?;
let manager = self.inner.authorization_manager.lock().await;
self.persist_with_manager(&manager, &credential_guard).await
}
pub(super) async fn acquire_persistence_guard(
&self,
) -> std::result::Result<RefreshCredentialLock, AuthError> {
let guard =
RefreshCredentialLock::acquire_for_server(&self.inner.server_name, &self.inner.url)
.await
.map_err(|_| {
AuthError::CredentialStoreError(
"failed to lock MCP OAuth credentials".to_string(),
)
})?;
let inner = Arc::clone(&self.inner);
let current = tokio::task::spawn_blocking(move || {
inner
.credential_store
.load(&DefaultKeyringStore, &inner.server_name, &inner.url)
})
.await
.map_err(|_| {
AuthError::CredentialStoreError(
"MCP OAuth credential validation task failed".to_string(),
)
})?
.map_err(|_| {
AuthError::CredentialStoreError("failed to validate MCP OAuth credentials".to_string())
})?;
let expected = self.stored_credentials().await;
let expected = match self.inner.credential_store.backend {
resolved_store::Backend::File => normalized_file_oauth_credentials(expected.as_ref()),
resolved_store::Backend::Keyring(_) => expected,
};
if normalized_oauth_credentials(current.as_ref()) != expected {
warn!(
server_name = %self.inner.server_name,
"stored MCP OAuth credentials changed; authorization must be rebuilt"
);
return Err(AuthError::AuthorizationRequired);
}
Ok(guard)
}
async fn credentials_changed(&self, manager: &AuthorizationManager) -> Result<bool> {
let (client_id, maybe_credentials) = manager.get_credentials().await?;
let last_credentials = self.inner.last_credentials.lock().await;
Ok(match maybe_credentials {
Some(credentials) => last_credentials.as_ref().is_none_or(|previous| {
previous.client_id != client_id
|| previous.token_response != WrappedOAuthTokenResponse(credentials)
}),
None => last_credentials.is_some(),
})
}
pub(super) async fn persist_with_manager(
&self,
manager: &AuthorizationManager,
_credential_guard: &RefreshCredentialLock,
) -> Result<()> {
let (client_id, maybe_credentials) = manager.get_credentials().await?;
match maybe_credentials {
Some(credentials) => {
@@ -814,7 +906,7 @@ impl OAuthPersistor {
compute_expires_at_millis(&credentials)
};
let stored = StoredOAuthTokens {
server_name: self.inner.server_name.clone(),
server_name: self.inner.refreshed_server_name(last_credentials.as_ref()),
url: self.inner.url.clone(),
issuer: last_credentials
.as_ref()
@@ -827,6 +919,7 @@ impl OAuthPersistor {
self.inner.credential_store.save_with_refresh_telemetry(
&DefaultKeyringStore,
&self.inner.server_name,
last_credentials.as_ref(),
&stored,
)?;
*last_credentials = Some(stored);
@@ -878,6 +971,16 @@ struct FallbackTokenEntry {
executor_owned: bool,
}
impl FallbackTokenEntry {
fn matches_legacy_key(&self, server_name: &str, url: &str) -> bool {
!server_name.starts_with("executor:")
&& !server_name.starts_with("local:")
&& !self.executor_owned
&& self.server_name == server_name
&& self.server_url == url
}
}
fn load_oauth_tokens_from_file(server_name: &str, url: &str) -> Result<Option<StoredOAuthTokens>> {
let _store_lock = OAuthStoreLock::acquire_for_read(OAuthStore::File)?;
load_oauth_tokens_from_file_with_lock_held(server_name, url)
@@ -897,20 +1000,14 @@ fn load_oauth_tokens_from_file_with_lock_held(
let local_server_name = server_name.strip_prefix("local:").unwrap_or(server_name);
for (stored_key, entry) in &store {
let matches_credential = if server_name.starts_with("executor:") {
stored_key == &key
&& entry.executor_owned
&& entry.server_name == server_name
&& entry.server_url == url
} else if entry.executor_owned {
false
let matches_credential = if stored_key != &key {
entry.matches_legacy_key(server_name, url)
} else if server_name.starts_with("executor:") {
entry.executor_owned && entry.server_name == server_name && entry.server_url == url
} else {
entry.server_url == url
// Escaped names may also match another server's stored, escaped name.
// Only accept a legacy unescaped entry under this identity's own key.
&& (!server_name.starts_with("local:") || stored_key == &key)
&& (entry.server_name == local_server_name
|| (stored_key == &key && entry.server_name == server_name))
!entry.executor_owned
&& entry.server_url == url
&& (entry.server_name == local_server_name || entry.server_name == server_name)
};
if !matches_credential {
continue;
@@ -963,34 +1060,36 @@ fn save_oauth_tokens_to_file_with_lock_held(tokens: &StoredOAuthTokens) -> Resul
anyhow::bail!("executor OAuth credential key conflicts with a host-owned credential");
}
let token_response = &tokens.token_response.0;
let expires_at = tokens
.expires_at
.or_else(|| compute_expires_at_millis(token_response));
let refresh_token = token_response
.refresh_token()
.map(|token| token.secret().to_string());
let scopes = token_response
.scopes()
.map(|s| s.iter().map(|s| s.to_string()).collect())
.unwrap_or_default();
let entry = FallbackTokenEntry {
server_name: tokens.server_name.clone(),
server_url: tokens.url.clone(),
issuer: tokens.issuer.clone(),
client_id: tokens.client_id.clone(),
access_token: token_response.access_token().secret().to_string(),
expires_at,
refresh_token,
scopes,
executor_owned,
};
store.insert(key, entry);
remove_file_credential(&mut store, &tokens.server_name, &tokens.url, &key);
store.insert(key, FallbackTokenEntry::from(tokens));
write_fallback_file(&store)
}
fn delete_oauth_tokens_from_file(key: &str) -> Result<bool> {
impl From<&StoredOAuthTokens> for FallbackTokenEntry {
fn from(tokens: &StoredOAuthTokens) -> Self {
let token_response = &tokens.token_response.0;
Self {
server_name: tokens.server_name.clone(),
server_url: tokens.url.clone(),
issuer: tokens.issuer.clone(),
client_id: tokens.client_id.clone(),
access_token: token_response.access_token().secret().to_string(),
expires_at: tokens
.expires_at
.or_else(|| compute_expires_at_millis(token_response)),
refresh_token: token_response
.refresh_token()
.map(|token| token.secret().to_string()),
scopes: token_response
.scopes()
.map(|scopes| scopes.iter().map(|scope| scope.to_string()).collect())
.unwrap_or_default(),
executor_owned: tokens.server_name.starts_with("executor:"),
}
}
}
fn delete_oauth_tokens_from_file(server_name: &str, url: &str, key: &str) -> Result<bool> {
let _store_lock = OAuthStoreLock::acquire_for_write(OAuthStore::File)?;
let mut store = match read_fallback_file_unlocked()? {
Some(store) => store,
@@ -1004,8 +1103,7 @@ fn delete_oauth_tokens_from_file(key: &str) -> Result<bool> {
anyhow::bail!("executor OAuth credential key conflicts with a host-owned credential");
}
let removed = store.remove(key).is_some();
let removed = remove_file_credential(&mut store, server_name, url, key);
if removed {
write_fallback_file(&store)?;
}
@@ -1013,6 +1111,21 @@ fn delete_oauth_tokens_from_file(key: &str) -> Result<bool> {
Ok(removed)
}
fn remove_file_credential(
store: &mut FallbackFile,
server_name: &str,
url: &str,
key: &str,
) -> bool {
let original_len = store.len();
// Enterprise keys are profile-scoped. A legacy ordinary row may share the name and URL.
let enterprise_scoped = server_name.starts_with("ema-idp:");
store.retain(|stored_key, entry| {
stored_key != key && (enterprise_scoped || !entry.matches_legacy_key(server_name, url))
});
store.len() != original_len
}
pub(crate) fn compute_expires_at_millis(response: &OAuthTokenResponse) -> Option<u64> {
let expires_in = response.expires_in()?;
let now = SystemTime::now()
@@ -1638,6 +1751,18 @@ mod tests {
let key = super::compute_store_key(&tokens.server_name, &tokens.url)?;
store.save(KEYRING_SERVICE, &key, &serialized)?;
super::save_oauth_tokens_to_file(&tokens)?;
let mut entries = read_fallback_file_unlocked()?.expect("stored credentials");
entries.insert(
"older-build-key".to_string(),
FallbackTokenEntry::from(&tokens),
);
let other = StoredOAuthTokens {
server_name: "other".into(),
..tokens.clone()
};
let other_key = compute_store_key(&other.server_name, &other.url)?;
entries.insert(other_key.clone(), FallbackTokenEntry::from(&other));
write_fallback_file(&entries)?;
let removed = super::delete_oauth_tokens_from_keyring_and_file(
&store,
@@ -1648,7 +1773,31 @@ mod tests {
)?;
assert!(removed);
assert!(!store.contains(&key));
assert!(!super::fallback_file_path()?.exists());
let remaining = read_fallback_file_unlocked()?.expect("other credential");
assert_eq!(remaining.into_keys().collect::<Vec<_>>(), vec![other_key]);
Ok(())
}
#[test]
fn enterprise_file_cleanup_preserves_legacy_local_credentials() -> Result<()> {
let _env = TempCodexHome::new();
let mut tokens = sample_tokens();
tokens.server_name = "ema-idp:demo".to_string();
let key = compute_store_key(&tokens.server_name, &tokens.url)?;
let local_key = compute_store_key("local:ema-idp:demo", &tokens.url)?;
let entry = FallbackTokenEntry::from(&tokens);
write_fallback_file(&FallbackFile::from([
(key.clone(), entry.clone()),
(local_key.clone(), entry),
]))?;
assert!(delete_oauth_tokens_from_file(
&tokens.server_name,
&tokens.url,
&key
)?);
let remaining = read_fallback_file_unlocked()?.expect("legacy local credential");
assert_eq!(remaining.into_keys().collect::<Vec<_>>(), vec![local_key]);
Ok(())
}
codex-rs/rmcp-client/src/oauth/credential_store.rs
+2
View File
@@ -253,11 +253,13 @@ impl<K: KeyringStore + Clone + 'static> CredentialStore for OAuthCredentialStore
.upgrade()
.context("OAuth credential mutation requires an active refresh guard")
.map_err(credential_store_error)?;
let expected = self.inner.last_credentials.lock().await.clone();
let tokens = tokio::task::spawn_blocking(move || -> Result<StoredOAuthTokens> {
let _guard = guard;
inner.store.save_with_refresh_telemetry(
&inner.keyring,
&inner.server_name,
expected.as_ref(),
&tokens,
)?;
Ok(tokens)
codex-rs/rmcp-client/src/oauth/refresh_transaction.rs
+2 -1
View File
@@ -249,6 +249,7 @@ impl OAuthPersistor {
let result = self.inner.credential_store.save_with_refresh_telemetry(
keyring_store,
&self.inner.server_name,
Some(&latest),
&refreshed,
);
if let Err(error) = result {
@@ -390,7 +391,7 @@ fn refreshed_tokens(
token_response.set_scopes(previous.token_response.0.scopes().cloned());
}
StoredOAuthTokens {
server_name: inner.server_name.clone(),
server_name: inner.refreshed_server_name(Some(previous)),
url: inner.url.clone(),
issuer: previous.issuer.clone(),
client_id: previous.client_id.clone(),
codex-rs/rmcp-client/src/oauth/resolved_store.rs
+42 -3
View File
@@ -10,7 +10,9 @@ use codex_keyring_store::KeyringStore;
use codex_otel::auth_storage::AuthStorageOriginator;
use codex_otel::auth_storage::Operation;
use codex_otel::auth_storage::Store;
use rmcp::transport::auth::AuthError;
use super::FallbackTokenEntry;
use super::OAuthKeyringLoadError;
use super::OAuthStore;
use super::OAuthStoreLock;
@@ -24,6 +26,7 @@ use super::load_oauth_tokens_from_file;
use super::load_oauth_tokens_from_file_with_lock_held;
use super::load_oauth_tokens_from_keyring;
use super::load_oauth_tokens_from_secrets_keyring_with_lock_held;
use super::normalized_file_oauth_credentials;
use super::normalized_oauth_credentials;
use super::read_fallback_file_unlocked;
use super::save_oauth_tokens_to_direct_keyring;
@@ -180,15 +183,51 @@ impl ResolvedOAuthCredentialStore {
result
}
/// Records the refresh-persistence milestone in addition to the underlying pinned save.
/// Persists a refresh to the pinned authority and records the refresh-persistence milestone.
///
/// File replaces the matched row only if it still holds the expected credentials. Login
/// writes continue to use the ordinary save path.
pub(crate) fn save_with_refresh_telemetry<K: KeyringStore + Clone + 'static>(
self,
keyring_store: &K,
server_name: &str,
expected: Option<&StoredOAuthTokens>,
tokens: &StoredOAuthTokens,
) -> Result<()> {
let mut observation = telemetry::resolved(self, Operation::RefreshPersist);
let result = self.save(keyring_store, server_name, tokens);
let result = match self.backend {
Backend::File => {
let mut save = telemetry::resolved(self, Operation::Save);
let result = (|| {
let _store_lock = OAuthStoreLock::acquire_for_write(OAuthStore::File)?;
let (key, current) =
load_oauth_tokens_from_file_with_lock_held(server_name, &tokens.url)?
.ok_or(AuthError::AuthorizationRequired)?;
if normalized_oauth_credentials(Some(&current))
!= normalized_file_oauth_credentials(expected)
{
return Err(AuthError::AuthorizationRequired.into());
}
let mut store =
read_fallback_file_unlocked()?.context("OAuth file disappeared")?;
// Keep the same matched row and stored identity, including legacy keys.
let entry = store
.get_mut(&key)
.context("OAuth file entry disappeared")?;
*entry = FallbackTokenEntry {
server_name: current.server_name,
server_url: current.url,
executor_owned: entry.executor_owned,
..FallbackTokenEntry::from(tokens)
};
write_fallback_file(&store)
})();
save.record_save_attempt(Store::File, &result);
result
}
Backend::Keyring(_) => self.save(keyring_store, server_name, tokens),
};
observation.record_save_attempt(telemetry::store(self), &result);
if matches!(self.backend, Backend::Keyring(_))
&& let Err(error) = &result
@@ -307,7 +346,7 @@ impl ResolvedOAuthCredentialStore {
let mut observation = telemetry::resolved(self, Operation::Delete);
let result = match self.backend {
Backend::File => compute_store_key(server_name, url)
.and_then(|key| delete_oauth_tokens_from_file(&key)),
.and_then(|key| delete_oauth_tokens_from_file(server_name, url, &key)),
Backend::Keyring(AuthKeyringBackendKind::Direct) => {
delete_oauth_tokens_from_direct_keyring(keyring_store, server_name, url)
}
codex-rs/rmcp-client/src/oauth/tests/credential_store_tests.rs
+3 -3
View File
@@ -59,8 +59,8 @@ async fn mutations_require_and_retain_the_transaction_guard() -> Result<()> {
load_oauth_tokens_from_file(&initial.server_name, &initial.url)?.is_none(),
!clear,
);
// Guard acquisition now rereads storage. Restore the deleted credential first,
// then delete it again under the guard so the queued save must recreate it.
// Guard acquisition requires a durable credential. Delete it under the guard in the
// save case: a queued save must retain the guard without recreating the row.
save_oauth_tokens_to_file(&initial)?;
let guard = CredentialStore::acquire_refresh_guard(&store)
.await?
@@ -87,7 +87,7 @@ async fn mutations_require_and_retain_the_transaction_guard() -> Result<()> {
drop(aggregate_lock);
let _guard = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), contender).await??;
let durable = load_oauth_tokens_from_file(&initial.server_name, &initial.url)?;
assert_eq!(durable.is_none(), clear);
assert!(durable.is_none());
}
Ok(())
}
codex-rs/rmcp-client/src/oauth/tests/persistor_tests.rs
+140
View File
@@ -44,6 +44,7 @@ use super::assert_tokens_match_without_expiry;
use super::sample_tokens;
use crate::http_client_adapter::StreamableHttpClientAdapter;
use crate::http_client_adapter::StreamableHttpRedirectMode;
use crate::oauth::FallbackTokenEntry;
use crate::oauth::OAuthCredentialStore;
use crate::oauth::OAuthPersistor;
use crate::oauth::OAuthRuntime;
@@ -55,10 +56,12 @@ use crate::oauth::compute_store_key;
use crate::oauth::delete_oauth_tokens;
use crate::oauth::load_oauth_tokens_from_file;
use crate::oauth::normalized_oauth_credentials;
use crate::oauth::read_fallback_file_unlocked;
use crate::oauth::refresh_lock::RefreshCredentialLock;
use crate::oauth::save_oauth_tokens;
use crate::oauth::save_oauth_tokens_to_file;
use crate::oauth::stored_oauth_credentials;
use crate::oauth::write_fallback_file;
use crate::oauth_http_client::OAuthHttpClientAdapter;
use crate::oauth_http_client::PROACTIVE_REFRESH_TIMEOUT;
use crate::startup_error::is_authentication_required_error;
@@ -1035,3 +1038,140 @@ fn expired_tokens(url: &str) -> StoredOAuthTokens {
.set_expires_in(Some(&Duration::ZERO));
tokens
}
#[expect(
clippy::await_holding_invalid_type,
reason = "exercise the existing manager-locked refresh before the persistence hook"
)]
#[tokio::test(flavor = "current_thread")]
async fn legacy_persistence_preserves_new_login_and_logout() -> Result<()> {
for logout in [false, true] {
let (_env, server, initial) = test_context().await?;
write_fallback_file(&[("!older-build".into(), FallbackTokenEntry::from(&initial))].into())?;
let persistor = persistor_for(&initial).await?;
// Simulate credentials changed by the existing AuthClient before its persistence hook.
let refresh = Mock::given(method("POST")).and(path("/oauth/token"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"access_token": "refreshed", "refresh_token": "rotated", "token_type": "Bearer", "expires_in": 3600
}))).expect(1).mount_as_scoped(&server).await;
persistor
.inner
.authorization_manager
.lock()
.await
.refresh_token()
.await?;
drop(refresh);
let replacement = if logout {
None
} else {
let mut next = initial.clone();
next.client_id = "new-login".to_string();
Some(next)
};
match &replacement {
Some(next) => {
save_oauth_tokens(
&next.server_name,
next,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::Direct,
)
.await?
}
None => {
delete_oauth_tokens(
&initial.server_name,
&initial.url,
OAuthCredentialsStoreMode::File,
AuthKeyringBackendKind::Direct,
)
.await?;
}
}
assert!(matches!(
persistor
.persist_if_needed()
.await
.unwrap_err()
.downcast_ref::<AuthError>(),
Some(AuthError::AuthorizationRequired)
));
assert_eq!(
load_oauth_tokens_from_file(&initial.server_name, &initial.url)?,
replacement
);
}
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn legacy_file_refreshes_preserve_the_matched_credential() -> Result<()> {
assert_legacy_file_refreshes("server", "server").await?;
assert_legacy_file_refreshes("local:ema-idp:demo", "ema-idp:demo").await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "exercise the existing manager-locked refresh before the persistence hook"
)]
async fn assert_legacy_file_refreshes(requested_name: &str, stored_name: &str) -> Result<()> {
let (_env, server, mut initial) = test_context().await?;
initial.server_name = stored_name.to_string();
let legacy_key = if requested_name == stored_name {
"!legacy-oauth-key".to_string()
} else {
// An escaped local name can read its old unescaped row only under its own key.
compute_store_key(requested_name, &initial.url)?
};
write_fallback_file(&[(legacy_key.clone(), FallbackTokenEntry::from(&initial))].into())?;
let persistor = OAuthPersistor::new(
requested_name.to_string(),
initial.url.clone(),
Arc::new(TokioMutex::new(authorization_manager_for(&initial).await?)),
ResolvedOAuthCredentialStore::file(),
Some(initial.clone()),
/*oauth_config*/ None,
);
for generation in 1..=2 {
let access = format!("access-{generation}");
let refresh = format!("refresh-{generation}");
let mock = Mock::given(method("POST"))
.and(path("/oauth/token"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"access_token": access,
"refresh_token": refresh,
"token_type": "Bearer",
"expires_in": 3600,
"id_token": "vendor-id-token",
})))
.expect(1)
.mount_as_scoped(&server)
.await;
if generation == 1 {
persistor.refresh_if_needed().await?;
} else {
persistor
.inner
.authorization_manager
.lock()
.await
.refresh_token()
.await?;
persistor.persist_if_needed().await?;
}
let entries = read_fallback_file_unlocked()?.expect("File credentials");
assert_eq!(entries.len(), 1);
let saved = entries.get(&legacy_key).expect("original File entry");
assert_eq!(saved.server_name, stored_name);
assert_eq!(saved.access_token, access);
assert_eq!(saved.refresh_token.as_ref(), Some(&refresh));
let cached = persistor
.stored_credentials()
.await
.expect("cached credentials");
assert_eq!(cached.server_name, stored_name);
drop(mock);
}
Ok(())
}
codex-rs/rmcp-client/src/oauth/tests/store_lock_tests.rs
+3 -1
View File
@@ -584,11 +584,13 @@ fn file_store_load_and_delete_observe_aggregate_lock() -> Result<()> {
assert_tokens_match_without_expiry(&loaded, &tokens);
let key = crate::oauth::compute_store_key(&tokens.server_name, &tokens.url)?;
let server_name = tokens.server_name.clone();
let url = tokens.url.clone();
let removed = complete_after_store_lock_contention(
env.path(),
OAuthStore::File,
|| Ok(()),
move || crate::oauth::delete_oauth_tokens_from_file(&key),
move || crate::oauth::delete_oauth_tokens_from_file(&server_name, &url, &key),
)?;
assert!(removed);
assert!(load_oauth_tokens_from_file(&tokens.server_name, &tokens.url)?.is_none());
codex-rs/rmcp-client/src/oauth/tests/telemetry_tests.rs
+4 -4
View File
@@ -420,10 +420,10 @@ async fn legacy_refresh_persistence_records_each_write_once() -> Result<()> {
};
assert_eq!(result.is_err(), fail_save);
let mut expected = Vec::new();
for operation in if proactive {
vec!["load", "save", "refresh_persist"]
for operation in if !proactive && fail_save {
vec!["load"]
} else {
vec!["save", "refresh_persist"]
vec!["load", "save", "refresh_persist"]
} {
expected.push(expected_metric(
/*count*/ 1,
@@ -435,7 +435,7 @@ async fn legacy_refresh_persistence_records_each_write_once() -> Result<()> {
("secure_outcome", "not_attempted"),
(
"outcome",
if fail_save && operation != "load" {
if fail_save && (!proactive || operation != "load") {
"error"
} else {
"success"