mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
use scopes_supported for OAuth when present on MCP servers (#14419)
Fixes [#8889](https://github.com/openai/codex/issues/8889). ## Summary - Discover and use advertised MCP OAuth `scopes_supported` when no explicit or configured scopes are present. - Apply the same scope precedence across `mcp add`, `mcp login`, skill dependency auto-login, and app-server MCP OAuth login. - Keep discovered scopes ephemeral and non-persistent. - Retry once without scopes for CLI and skill auto-login flows if the OAuth provider rejects discovered scopes. ## Motivation Some MCP servers advertise the scopes they expect clients to request during OAuth, but Codex was ignoring that metadata and typically starting OAuth with no scopes unless the user manually passed `--scopes` or configured `server.scopes`. That made compliant MCP servers harder to use out of the box and is the behavior described in [#8889](https://github.com/openai/codex/issues/8889). This change also brings our behavior in line with the MCP authorization spec's scope selection guidance: https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#scope-selection-strategy ## Behavior Scope selection now follows this order everywhere: 1. Explicit request scopes / CLI `--scopes` 2. Configured `server.scopes` 3. Discovered `scopes_supported` 4. Legacy empty-scope behavior Compatibility notes: - Existing working setups keep the same behavior because explicit and configured scopes still win. - Discovered scopes are never written back into config or token storage. - If discovery is missing, malformed, or empty, behavior falls back to the previous empty-scope path. - App-server login gets the same precedence rules, but does not add a transparent retry path in this change. ## Implementation - Extend streamable HTTP OAuth discovery to parse and normalize `scopes_supported`. - Add a shared MCP scope resolver in `core` so all login entrypoints use the same precedence rules. - Preserve provider callback errors from the OAuth flow so CLI/skill flows can safely distinguish provider rejections from other failures. - Reuse discovered scopes from the existing OAuth support check where possible instead of persisting new config.
This commit is contained in:
@@ -3,8 +3,9 @@ use std::collections::HashMap;
|
||||
use anyhow::Result;
|
||||
use codex_protocol::protocol::McpAuthStatus;
|
||||
use codex_rmcp_client::OAuthCredentialsStoreMode;
|
||||
use codex_rmcp_client::OAuthProviderError;
|
||||
use codex_rmcp_client::determine_streamable_http_auth_status;
|
||||
use codex_rmcp_client::supports_oauth_login;
|
||||
use codex_rmcp_client::discover_streamable_http_oauth;
|
||||
use futures::future::join_all;
|
||||
use tracing::warn;
|
||||
|
||||
@@ -16,6 +17,7 @@ pub struct McpOAuthLoginConfig {
|
||||
pub url: String,
|
||||
pub http_headers: Option<HashMap<String, String>>,
|
||||
pub env_http_headers: Option<HashMap<String, String>>,
|
||||
pub discovered_scopes: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -25,6 +27,20 @@ pub enum McpOAuthLoginSupport {
|
||||
Unknown(anyhow::Error),
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum McpOAuthScopesSource {
|
||||
Explicit,
|
||||
Configured,
|
||||
Discovered,
|
||||
Empty,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ResolvedMcpOAuthScopes {
|
||||
pub scopes: Vec<String>,
|
||||
pub source: McpOAuthScopesSource,
|
||||
}
|
||||
|
||||
pub async fn oauth_login_support(transport: &McpServerTransportConfig) -> McpOAuthLoginSupport {
|
||||
let McpServerTransportConfig::StreamableHttp {
|
||||
url,
|
||||
@@ -40,17 +56,67 @@ pub async fn oauth_login_support(transport: &McpServerTransportConfig) -> McpOAu
|
||||
return McpOAuthLoginSupport::Unsupported;
|
||||
}
|
||||
|
||||
match supports_oauth_login(url).await {
|
||||
Ok(true) => McpOAuthLoginSupport::Supported(McpOAuthLoginConfig {
|
||||
match discover_streamable_http_oauth(url, http_headers.clone(), env_http_headers.clone()).await
|
||||
{
|
||||
Ok(Some(discovery)) => McpOAuthLoginSupport::Supported(McpOAuthLoginConfig {
|
||||
url: url.clone(),
|
||||
http_headers: http_headers.clone(),
|
||||
env_http_headers: env_http_headers.clone(),
|
||||
discovered_scopes: discovery.scopes_supported,
|
||||
}),
|
||||
Ok(false) => McpOAuthLoginSupport::Unsupported,
|
||||
Ok(None) => McpOAuthLoginSupport::Unsupported,
|
||||
Err(err) => McpOAuthLoginSupport::Unknown(err),
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn discover_supported_scopes(
|
||||
transport: &McpServerTransportConfig,
|
||||
) -> Option<Vec<String>> {
|
||||
match oauth_login_support(transport).await {
|
||||
McpOAuthLoginSupport::Supported(config) => config.discovered_scopes,
|
||||
McpOAuthLoginSupport::Unsupported | McpOAuthLoginSupport::Unknown(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn resolve_oauth_scopes(
|
||||
explicit_scopes: Option<Vec<String>>,
|
||||
configured_scopes: Option<Vec<String>>,
|
||||
discovered_scopes: Option<Vec<String>>,
|
||||
) -> ResolvedMcpOAuthScopes {
|
||||
if let Some(scopes) = explicit_scopes {
|
||||
return ResolvedMcpOAuthScopes {
|
||||
scopes,
|
||||
source: McpOAuthScopesSource::Explicit,
|
||||
};
|
||||
}
|
||||
|
||||
if let Some(scopes) = configured_scopes {
|
||||
return ResolvedMcpOAuthScopes {
|
||||
scopes,
|
||||
source: McpOAuthScopesSource::Configured,
|
||||
};
|
||||
}
|
||||
|
||||
if let Some(scopes) = discovered_scopes
|
||||
&& !scopes.is_empty()
|
||||
{
|
||||
return ResolvedMcpOAuthScopes {
|
||||
scopes,
|
||||
source: McpOAuthScopesSource::Discovered,
|
||||
};
|
||||
}
|
||||
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: Vec::new(),
|
||||
source: McpOAuthScopesSource::Empty,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn should_retry_without_scopes(scopes: &ResolvedMcpOAuthScopes, error: &anyhow::Error) -> bool {
|
||||
scopes.source == McpOAuthScopesSource::Discovered
|
||||
&& error.downcast_ref::<OAuthProviderError>().is_some()
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct McpAuthStatusEntry {
|
||||
pub config: McpServerConfig,
|
||||
@@ -111,3 +177,112 @@ async fn compute_auth_status(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use anyhow::anyhow;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::McpOAuthScopesSource;
|
||||
use super::OAuthProviderError;
|
||||
use super::ResolvedMcpOAuthScopes;
|
||||
use super::resolve_oauth_scopes;
|
||||
use super::should_retry_without_scopes;
|
||||
|
||||
#[test]
|
||||
fn resolve_oauth_scopes_prefers_explicit() {
|
||||
let resolved = resolve_oauth_scopes(
|
||||
Some(vec!["explicit".to_string()]),
|
||||
Some(vec!["configured".to_string()]),
|
||||
Some(vec!["discovered".to_string()]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: vec!["explicit".to_string()],
|
||||
source: McpOAuthScopesSource::Explicit,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_oauth_scopes_prefers_configured_over_discovered() {
|
||||
let resolved = resolve_oauth_scopes(
|
||||
None,
|
||||
Some(vec!["configured".to_string()]),
|
||||
Some(vec!["discovered".to_string()]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: vec!["configured".to_string()],
|
||||
source: McpOAuthScopesSource::Configured,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_oauth_scopes_uses_discovered_when_needed() {
|
||||
let resolved = resolve_oauth_scopes(None, None, Some(vec!["discovered".to_string()]));
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: vec!["discovered".to_string()],
|
||||
source: McpOAuthScopesSource::Discovered,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_oauth_scopes_preserves_explicitly_empty_configured_scopes() {
|
||||
let resolved = resolve_oauth_scopes(None, Some(Vec::new()), Some(vec!["ignored".into()]));
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: Vec::new(),
|
||||
source: McpOAuthScopesSource::Configured,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_oauth_scopes_falls_back_to_empty() {
|
||||
let resolved = resolve_oauth_scopes(None, None, None);
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
ResolvedMcpOAuthScopes {
|
||||
scopes: Vec::new(),
|
||||
source: McpOAuthScopesSource::Empty,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn should_retry_without_scopes_only_for_discovered_provider_errors() {
|
||||
let discovered = ResolvedMcpOAuthScopes {
|
||||
scopes: vec!["scope".to_string()],
|
||||
source: McpOAuthScopesSource::Discovered,
|
||||
};
|
||||
let provider_error = anyhow!(OAuthProviderError::new(
|
||||
Some("invalid_scope".to_string()),
|
||||
Some("scope rejected".to_string()),
|
||||
));
|
||||
|
||||
assert!(should_retry_without_scopes(&discovered, &provider_error));
|
||||
|
||||
let configured = ResolvedMcpOAuthScopes {
|
||||
scopes: vec!["scope".to_string()],
|
||||
source: McpOAuthScopesSource::Configured,
|
||||
};
|
||||
assert!(!should_retry_without_scopes(&configured, &provider_error));
|
||||
assert!(!should_retry_without_scopes(
|
||||
&discovered,
|
||||
&anyhow!("timed out waiting for OAuth callback"),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,8 @@ use tracing::warn;
|
||||
|
||||
use super::auth::McpOAuthLoginSupport;
|
||||
use super::auth::oauth_login_support;
|
||||
use super::auth::resolve_oauth_scopes;
|
||||
use super::auth::should_retry_without_scopes;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::config::Config;
|
||||
@@ -236,20 +238,52 @@ pub(crate) async fn maybe_install_mcp_dependencies(
|
||||
)
|
||||
.await;
|
||||
|
||||
if let Err(err) = perform_oauth_login(
|
||||
let resolved_scopes = resolve_oauth_scopes(
|
||||
None,
|
||||
server_config.scopes.clone(),
|
||||
oauth_config.discovered_scopes.clone(),
|
||||
);
|
||||
let first_attempt = perform_oauth_login(
|
||||
&name,
|
||||
&oauth_config.url,
|
||||
config.mcp_oauth_credentials_store_mode,
|
||||
oauth_config.http_headers,
|
||||
oauth_config.env_http_headers,
|
||||
&[],
|
||||
oauth_config.http_headers.clone(),
|
||||
oauth_config.env_http_headers.clone(),
|
||||
&resolved_scopes.scopes,
|
||||
server_config.oauth_resource.as_deref(),
|
||||
config.mcp_oauth_callback_port,
|
||||
config.mcp_oauth_callback_url.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
warn!("failed to login to MCP dependency {name}: {err}");
|
||||
.await;
|
||||
|
||||
if let Err(err) = first_attempt {
|
||||
if should_retry_without_scopes(&resolved_scopes, &err) {
|
||||
sess.notify_background_event(
|
||||
turn_context,
|
||||
format!(
|
||||
"Retrying MCP {name} authentication without scopes after provider rejection."
|
||||
),
|
||||
)
|
||||
.await;
|
||||
|
||||
if let Err(err) = perform_oauth_login(
|
||||
&name,
|
||||
&oauth_config.url,
|
||||
config.mcp_oauth_credentials_store_mode,
|
||||
oauth_config.http_headers,
|
||||
oauth_config.env_http_headers,
|
||||
&[],
|
||||
server_config.oauth_resource.as_deref(),
|
||||
config.mcp_oauth_callback_port,
|
||||
config.mcp_oauth_callback_url.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
warn!("failed to login to MCP dependency {name}: {err}");
|
||||
}
|
||||
} else {
|
||||
warn!("failed to login to MCP dependency {name}: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user