From e4167a02a3d26d54193f0dbd34a95201b132bbb6 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Tue, 7 Jul 2026 16:34:31 +0000 Subject: [PATCH] Handle rejected MCP OAuth refresh tokens --- .../src/oauth/refresh_transaction.rs | 19 ++++++++++++ .../src/oauth/tests/persistor_tests.rs | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/codex-rs/rmcp-client/src/oauth/refresh_transaction.rs b/codex-rs/rmcp-client/src/oauth/refresh_transaction.rs index cc3a9ce361..1ab4c14b80 100644 --- a/codex-rs/rmcp-client/src/oauth/refresh_transaction.rs +++ b/codex-rs/rmcp-client/src/oauth/refresh_transaction.rs @@ -178,6 +178,25 @@ impl OAuthPersistor { debug!("received refreshed MCP OAuth credentials from the provider"); refreshed_tokens(token_response, &latest, &self.inner) } + Ok(Err(error @ AuthError::TokenRefreshFailed(_))) => { + // RMCP 1.8 collapses definitive OAuth rejection (for example, + // `invalid_grant`) and transient token-endpoint failures into this string + // variant. Match RMCP's own request path for now so rejected refresh tokens + // prompt reauthorization instead of surfacing as generic MCP startup failures. + // This can also prompt reauthorization after a transient failure. + // TODO: Once modelcontextprotocol/rust-sdk#963 is available in the RMCP version + // used by Codex, map only its typed refresh-token rejection error here. + warn!( + error = %error, + "MCP OAuth refresh failed; reauthorization required by RMCP compatibility policy" + ); + return Err(AuthError::AuthorizationRequired).with_context(|| { + format!( + "failed to refresh OAuth tokens for server {}: {error}", + self.inner.server_name + ) + }); + } Ok(Err(error)) => { warn!( error = %error, diff --git a/codex-rs/rmcp-client/src/oauth/tests/persistor_tests.rs b/codex-rs/rmcp-client/src/oauth/tests/persistor_tests.rs index ba1e788913..f32c2b691c 100644 --- a/codex-rs/rmcp-client/src/oauth/tests/persistor_tests.rs +++ b/codex-rs/rmcp-client/src/oauth/tests/persistor_tests.rs @@ -39,6 +39,7 @@ use crate::oauth::compute_store_key; use crate::oauth::load_oauth_tokens_from_file; use crate::oauth::refresh_lock::RefreshCredentialLock; use crate::oauth::save_oauth_tokens_to_file; +use crate::startup_error::is_authentication_required_error; const REFRESH_LOCK_CONTENTION_EVENT_TARGET: &str = "codex_rmcp_client::oauth::refresh_lock::contention"; @@ -201,6 +202,35 @@ async fn missing_authoritative_credentials_require_reauthorization() -> Result<( Ok(()) } +#[tokio::test(flavor = "current_thread")] +async fn rejected_refresh_token_requires_reauthorization() -> Result<()> { + let (_env, server, initial) = test_context().await?; + Mock::given(method("POST")) + .and(path("/oauth/token")) + .and(body_string_contains("grant_type=refresh_token")) + .and(body_string_contains("refresh_token=refresh-token")) + .respond_with(ResponseTemplate::new(400).set_body_json(serde_json::json!({ + "error": "invalid_grant", + "error_description": "refresh token expired or revoked", + }))) + .expect(1) + .mount(&server) + .await; + save_oauth_tokens_to_file(&initial)?; + let persistor = persistor_for(&initial).await?; + + let error = persistor + .refresh_if_needed() + .await + .expect_err("a provider-rejected refresh token should require reauthorization"); + assert!(is_authentication_required_error(&error)); + let stored = load_oauth_tokens_from_file(&initial.server_name, &initial.url)? + .expect("rejected refresh must preserve the durable credentials"); + assert_tokens_match_without_expiry(&stored, &initial); + server.verify().await; + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn caller_cancellation_does_not_cancel_refresh_persistence() -> Result<()> { let (_env, server, initial) = test_context().await?;