openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-22 17:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(rmcp): classify unusable OAuth credentials

Browse Source
This commit is contained in:
Adam Perry @ OpenAI
2026-06-04 21:49:49 -07:00
parent b1f0f4e5e4
commit fbc112d304
3 changed files with 109 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/rmcp-client/src/auth_status.rs
+39 -2
View File
@@ -1,9 +1,12 @@
use std::collections::HashMap;
use std::time::Duration;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
use anyhow::Error;
use anyhow::Result;
use codex_protocol::protocol::McpAuthStatus;
use oauth2::TokenResponse;
use reqwest::Client;
use reqwest::StatusCode;
use reqwest::Url;
@@ -12,7 +15,8 @@ use reqwest::header::HeaderMap;
use serde::Deserialize;
use tracing::debug;
use crate::oauth::has_oauth_tokens;
use crate::oauth::StoredOAuthTokens;
use crate::oauth::load_oauth_tokens;
use crate::utils::apply_default_headers;
use crate::utils::build_default_headers;
use codex_config::types::OAuthCredentialsStoreMode;
@@ -44,7 +48,10 @@ pub async fn determine_streamable_http_auth_status(
return Ok(McpAuthStatus::BearerToken);
}
if has_oauth_tokens(server_name, url, store_mode)? {
if load_oauth_tokens(server_name, url, store_mode)?
.as_ref()
.is_some_and(stored_oauth_tokens_are_locally_usable)
{
return Ok(McpAuthStatus::OAuth);
}
@@ -60,6 +67,36 @@ pub async fn determine_streamable_http_auth_status(
}
}
fn stored_oauth_tokens_are_locally_usable(tokens: &StoredOAuthTokens) -> bool {
if tokens.client_id.trim().is_empty() {
return false;
}
let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or_else(|_| Duration::from_secs(0))
.as_millis() as u64;
let is_expired = tokens
.expires_at
.is_some_and(|expires_at| expires_at <= now_ms);
if is_expired {
return tokens
.token_response
.0
.refresh_token()
.is_some_and(|token| !token.secret().trim().is_empty());
}
!tokens
.token_response
.0
.access_token()
.secret()
.trim()
.is_empty()
}
/// Attempt to determine whether a streamable HTTP MCP server advertises OAuth login.
pub async fn supports_oauth_login(url: &str) -> Result<bool> {
Ok(discover_streamable_http_oauth(
codex-rs/rmcp-client/src/oauth.rs
-8
View File
@@ -94,14 +94,6 @@ pub(crate) fn load_oauth_tokens(
}
}
pub(crate) fn has_oauth_tokens(
server_name: &str,
url: &str,
store_mode: OAuthCredentialsStoreMode,
) -> Result<bool> {
Ok(load_oauth_tokens(server_name, url, store_mode)?.is_some())
}
fn refresh_expires_in_from_timestamp(tokens: &mut StoredOAuthTokens) {
let Some(expires_at) = tokens.expires_at else {
return;
codex-rs/rmcp-client/tests/oauth_auth_status.rs
+70 -33
View File
@@ -34,7 +34,7 @@ const CHILD_SCENARIO_ENV: &str = "MCP_TEST_AUTH_STATUS_SCENARIO";
const CHILD_SERVER_URL_ENV: &str = "MCP_TEST_AUTH_STATUS_SERVER_URL";
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn oauth_status_does_not_prove_stored_credentials_are_usable() -> anyhow::Result<()> {
async fn locally_unusable_credentials_are_not_logged_in() -> anyhow::Result<()> {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/.well-known/oauth-authorization-server/mcp"))
@@ -45,6 +45,32 @@ async fn oauth_status_does_not_prove_stored_credentials_are_usable() -> anyhow::
.expect(2)
.mount(&server)
.await;
let server_url = format!("{}/mcp", server.uri());
run_child("expired_without_refresh", &server_url).await?;
run_child("empty_fields", &server_url).await?;
server.verify().await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn refreshable_expired_credentials_report_oauth() -> anyhow::Result<()> {
run_child("expired_with_refresh", "not-a-valid-url").await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn server_rejected_credentials_still_report_oauth() -> anyhow::Result<()> {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/.well-known/oauth-authorization-server/mcp"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"authorization_endpoint": format!("{}/oauth/authorize", server.uri()),
"token_endpoint": format!("{}/oauth/token", server.uri()),
})))
.expect(1)
.mount(&server)
.await;
Mock::given(method("POST"))
.and(path("/mcp"))
.and(header(
@@ -57,7 +83,6 @@ async fn oauth_status_does_not_prove_stored_credentials_are_usable() -> anyhow::
.await;
let server_url = format!("{}/mcp", server.uri());
run_child("expired_without_refresh", &server_url).await?;
run_child("server_rejected", &server_url).await?;
server.verify().await;
@@ -65,9 +90,7 @@ async fn oauth_status_does_not_prove_stored_credentials_are_usable() -> anyhow::
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn oauth_status_only_requires_a_loadable_matching_store_entry() -> anyhow::Result<()> {
run_child("empty_fields", "not-a-valid-url").await?;
async fn unreachable_server_with_credentials_still_reports_oauth() -> anyhow::Result<()> {
let listener = TcpListener::bind("127.0.0.1:0")?;
let unreachable_url = format!("http://{}/mcp", listener.local_addr()?);
drop(listener);
@@ -122,28 +145,45 @@ async fn oauth_auth_status_child() -> anyhow::Result<()> {
return Ok(());
}
let (access_token, client_id, refresh_token, expires_at) = match scenario.as_str() {
"expired_without_refresh" => (
"expired-access-token",
"client-id",
/*refresh_token*/ None,
Some(0),
),
"server_rejected" => (
"server-rejected-access-token",
"client-id",
/*refresh_token*/ None,
/*expires_at*/ None,
),
"empty_fields" => ("", "", Some(""), /*expires_at*/ None),
"unreachable_server" => (
"access-token",
"client-id",
/*refresh_token*/ None,
/*expires_at*/ None,
),
unexpected => panic!("unexpected child scenario: {unexpected}"),
};
let (access_token, client_id, refresh_token, expires_at, expected_status) =
match scenario.as_str() {
"expired_without_refresh" => (
"expired-access-token",
"client-id",
/*refresh_token*/ None,
Some(0),
McpAuthStatus::NotLoggedIn,
),
"server_rejected" => (
"server-rejected-access-token",
"client-id",
/*refresh_token*/ None,
/*expires_at*/ None,
McpAuthStatus::OAuth,
),
"empty_fields" => (
"",
"",
Some(""),
/*expires_at*/ None,
McpAuthStatus::NotLoggedIn,
),
"unreachable_server" => (
"access-token",
"client-id",
/*refresh_token*/ None,
/*expires_at*/ None,
McpAuthStatus::OAuth,
),
"expired_with_refresh" => (
"expired-access-token",
"client-id",
Some("refresh-token"),
Some(0),
McpAuthStatus::OAuth,
),
unexpected => panic!("unexpected child scenario: {unexpected}"),
};
save_tokens(
&server_url,
access_token,
@@ -152,13 +192,10 @@ async fn oauth_auth_status_child() -> anyhow::Result<()> {
expires_at,
)?;
assert_eq!(
determine_auth_status(&server_url).await?,
McpAuthStatus::OAuth
);
assert_eq!(determine_auth_status(&server_url).await?, expected_status);
match scenario.as_str() {
"expired_without_refresh" | "server_rejected" => {
"server_rejected" => {
let client = new_client(&server_url).await?;
assert!(
initialize_client(&client).await.is_err(),
@@ -173,7 +210,7 @@ async fn oauth_auth_status_child() -> anyhow::Result<()> {
);
}
}
"empty_fields" => {}
"expired_without_refresh" | "empty_fields" | "expired_with_refresh" => {}
unexpected => panic!("unexpected child scenario: {unexpected}"),
}