mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Support confidential OAuth clients for enterprise-managed MCP auth (#56001)
## Why Enterprise-managed MCP authentication previously required public OAuth clients at both the identity provider and the MCP authorization server, preventing use of registrations that require client secrets. ## What changed - Accept separate `client_secret` and `token_endpoint_auth_method` settings for the enterprise IdP and MCP server OAuth registration. Support `client_secret_basic` and `client_secret_post`, defaulting to Basic when a secret is configured. - Validate the registered method against server metadata and use it for enterprise login and both ID-JAG exchanges. - Require a configured `oauth.authorization_server_issuer` for confidential MCP registrations and reject empty secrets or authentication methods without secrets. - Require the account OAuth client to match the configured IdP client before reusing an account grant with a provisioned secret; otherwise use dedicated enterprise login. ## Testing Extend configuration, login, exchange, and app-server lifecycle tests to cover confidential registrations, separate IdP and resource credentials, authentication method validation, and exclusion of client secrets from authorization URLs, stored login grants, and logs. GitOrigin-RevId: b3be378c2b1bb67e6a3652c648e72fd2e21e956d
This commit is contained in:
@@ -180,6 +180,7 @@ pub use marketplace_edit::remove_user_marketplace_config;
|
||||
pub use mcp_edit::load_global_mcp_servers;
|
||||
pub use mcp_ema::McpEmaAuthScope;
|
||||
pub use mcp_ema::McpEmaRegistration;
|
||||
pub use mcp_ema::McpEmaTokenEndpointAuthMethod;
|
||||
pub use mcp_ema::McpEnterpriseManagedAuthConfig;
|
||||
pub use mcp_ema::McpServerIdpOAuthConfig;
|
||||
pub use mcp_types::AppToolApproval;
|
||||
|
||||
@@ -14,19 +14,46 @@ use sha2::Digest;
|
||||
use sha2::Sha256;
|
||||
|
||||
use codex_features::Feature;
|
||||
use codex_utils_redacted_string::RedactedString;
|
||||
|
||||
use crate::ConfigLayerSource;
|
||||
use crate::ConfigLayerStack;
|
||||
use crate::McpServerConfig;
|
||||
use crate::McpServerTransportConfig;
|
||||
|
||||
/// Token endpoint authentication selected by a confidential EMA client registration.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Copy, Default, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum McpEmaTokenEndpointAuthMethod {
|
||||
#[default]
|
||||
ClientSecretBasic,
|
||||
ClientSecretPost,
|
||||
}
|
||||
|
||||
impl McpEmaTokenEndpointAuthMethod {
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::ClientSecretBasic => "client_secret_basic",
|
||||
Self::ClientSecretPost => "client_secret_post",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct McpServerIdpOAuthConfig {
|
||||
/// Issuer used for enterprise OAuth discovery and identity validation.
|
||||
pub issuer: String,
|
||||
/// Public OAuth client registered with that enterprise IdP.
|
||||
/// OAuth client registered with that enterprise IdP.
|
||||
pub client_id: String,
|
||||
/// Provisioned client secret for confidential IdP login and token exchange.
|
||||
/// Omitted for a public client.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub client_secret: Option<RedactedString>,
|
||||
/// Registered EMA client authentication method. Defaults to client_secret_basic
|
||||
/// when a secret is configured; requires a client secret.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub token_endpoint_auth_method: Option<McpEmaTokenEndpointAuthMethod>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
|
||||
@@ -44,6 +71,8 @@ pub struct McpEmaRegistration {
|
||||
server_url: String,
|
||||
resource: Option<String>,
|
||||
client_id: Option<String>,
|
||||
client_secret: Option<RedactedString>,
|
||||
token_endpoint_auth_method: Option<McpEmaTokenEndpointAuthMethod>,
|
||||
authorization_server_issuer: Option<String>,
|
||||
scopes: Vec<String>,
|
||||
}
|
||||
@@ -65,6 +94,14 @@ impl McpEmaRegistration {
|
||||
self.client_id.as_deref()
|
||||
}
|
||||
|
||||
pub fn client_secret(&self) -> Option<&str> {
|
||||
self.client_secret.as_ref().map(|secret| secret.as_str())
|
||||
}
|
||||
|
||||
pub fn token_endpoint_auth_method(&self) -> Option<McpEmaTokenEndpointAuthMethod> {
|
||||
self.token_endpoint_auth_method
|
||||
}
|
||||
|
||||
pub fn authorization_server_issuer(&self) -> Option<&str> {
|
||||
self.authorization_server_issuer.as_deref()
|
||||
}
|
||||
@@ -118,7 +155,29 @@ impl McpEnterpriseManagedAuthConfig {
|
||||
) -> io::Result<Option<Self>> {
|
||||
validate_ema_auth_sources(stack, servers)?;
|
||||
validate_xaa_opt_in_source(stack, xaa_enabled)?;
|
||||
Self::from_config_layers(stack, fallback)
|
||||
let profile = Self::from_config_layers(stack, fallback)?;
|
||||
if let Some(profile) = &profile {
|
||||
if profile
|
||||
.idp
|
||||
.client_secret
|
||||
.as_ref()
|
||||
.is_some_and(|secret| secret.trim().is_empty())
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"enterprise IdP client_secret must not be empty",
|
||||
));
|
||||
}
|
||||
if profile.idp.token_endpoint_auth_method.is_some()
|
||||
&& profile.idp.client_secret.is_none()
|
||||
{
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"enterprise IdP token_endpoint_auth_method requires client_secret",
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(profile)
|
||||
}
|
||||
|
||||
/// Selects one complete trusted registration rather than merging issuer/client pairs.
|
||||
@@ -196,6 +255,8 @@ impl McpServerConfig {
|
||||
server_url: url.clone(),
|
||||
resource: self.oauth_resource.clone(),
|
||||
client_id: oauth.client_id.clone(),
|
||||
client_secret: oauth.client_secret.clone(),
|
||||
token_endpoint_auth_method: oauth.token_endpoint_auth_method,
|
||||
authorization_server_issuer: oauth.authorization_server_issuer.clone(),
|
||||
scopes: self.scopes.clone().unwrap_or_default(),
|
||||
});
|
||||
@@ -207,8 +268,18 @@ impl McpServerConfig {
|
||||
if !self.is_local_environment() {
|
||||
return Err("ema_auth requires a host-owned MCP connection");
|
||||
}
|
||||
if self.oauth_client_secret().is_some() {
|
||||
return Err("ema_auth cannot be combined with oauth.client_secret");
|
||||
if let Some(secret) = self.oauth_client_secret() {
|
||||
if secret.trim().is_empty() {
|
||||
return Err("ema_auth oauth.client_secret must not be empty");
|
||||
}
|
||||
if self.oauth.as_ref().is_none_or(|oauth| {
|
||||
oauth
|
||||
.authorization_server_issuer
|
||||
.as_ref()
|
||||
.is_none_or(|issuer| issuer.trim().is_empty())
|
||||
}) {
|
||||
return Err("confidential ema_auth requires oauth.authorization_server_issuer");
|
||||
}
|
||||
}
|
||||
let McpServerTransportConfig::StreamableHttp {
|
||||
bearer_token_env_var,
|
||||
|
||||
@@ -57,6 +57,8 @@ fn ema_profiles_and_auth_modes_preserve_non_project_authority() {
|
||||
let expected = McpEnterpriseManagedAuthConfig {
|
||||
idp: McpServerIdpOAuthConfig {
|
||||
issuer: "https://idp.example".into(),
|
||||
client_secret: None,
|
||||
token_endpoint_auth_method: None,
|
||||
client_id: "enterprise".into(),
|
||||
},
|
||||
};
|
||||
@@ -225,6 +227,8 @@ fn ema_credential_names_bind_user_workspace_issuer_and_client() {
|
||||
let scope = McpEmaAuthScope::new(user.into(), workspace.into()).unwrap();
|
||||
let idp = McpServerIdpOAuthConfig {
|
||||
issuer: issuer.into(),
|
||||
client_secret: None,
|
||||
token_endpoint_auth_method: None,
|
||||
client_id: client.into(),
|
||||
};
|
||||
assert!(names.insert(idp.credential_name(&scope)));
|
||||
@@ -243,6 +247,8 @@ fn ema_credential_names_bind_user_workspace_issuer_and_client() {
|
||||
let scope = McpEmaAuthScope::new(identifiers[0].into(), identifiers[1].into()).unwrap();
|
||||
let name = McpServerIdpOAuthConfig {
|
||||
issuer: identifiers[2].into(),
|
||||
client_secret: None,
|
||||
token_endpoint_auth_method: None,
|
||||
client_id: identifiers[3].into(),
|
||||
}
|
||||
.credential_name(&scope);
|
||||
@@ -274,3 +280,48 @@ fn ema_rejects_alternate_credentials_and_executor_custody() {
|
||||
toml::from_str("url='https://resource.example'\nauth='ema_auth'\nhttp_headers={}").unwrap();
|
||||
assert_eq!(server.validate_ema_auth_transport(), Ok(()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn confidential_ema_registration_binds_secrets_to_trusted_issuers() {
|
||||
let (system, _, project) = local_sources("ema-confidential");
|
||||
let trusted = "[mcp_enterprise_managed_auth.idp]\nissuer='https://idp.example'\nclient_id='idp'\nclient_secret='idp-secret'\ntoken_endpoint_auth_method='client_secret_post'\n[mcp_servers.enterprise]\nurl='https://resource.example'\nauth='ema_auth'\noauth={client_id='client', client_secret='resource-secret', token_endpoint_auth_method='client_secret_basic', authorization_server_issuer='https://as.example'}";
|
||||
assert!(valid_ema_layers(vec![(system.clone(), trusted)]));
|
||||
let layers = stack(vec![
|
||||
(system.clone(), trusted),
|
||||
(
|
||||
project.clone(),
|
||||
"[mcp_enterprise_managed_auth.idp]\nclient_secret='project-secret'",
|
||||
),
|
||||
]);
|
||||
let profile =
|
||||
McpEnterpriseManagedAuthConfig::from_config_layers(&layers, /*fallback*/ None)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(profile.idp.client_secret, Some("idp-secret".into()));
|
||||
|
||||
for invalid in [
|
||||
trusted.replace("idp-secret", " "),
|
||||
trusted.replace("client_secret='idp-secret'\n", ""),
|
||||
] {
|
||||
assert!(
|
||||
McpEnterpriseManagedAuthConfig::resolve(
|
||||
&stack(vec![(system.clone(), &invalid)]),
|
||||
/*fallback*/ None,
|
||||
&Default::default(),
|
||||
/*xaa_enabled*/ false,
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
for override_config in [
|
||||
"[mcp_servers.enterprise.oauth]\nauthorization_server_issuer='https://attacker.example'",
|
||||
"[mcp_servers.enterprise.oauth]\nclient_secret='project-secret'",
|
||||
"[mcp_servers.enterprise.oauth]\ntoken_endpoint_auth_method='client_secret_post'",
|
||||
] {
|
||||
assert!(!valid_ema_layers(vec![
|
||||
(system.clone(), trusted),
|
||||
(project.clone(), override_config)
|
||||
]));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ use serde::Serialize;
|
||||
use serde::de::Error as SerdeError;
|
||||
|
||||
use crate::McpEmaRegistration;
|
||||
use crate::McpEmaTokenEndpointAuthMethod;
|
||||
use crate::RequirementSource;
|
||||
|
||||
/// Effective MCP environment id when config omits `environment_id`.
|
||||
@@ -164,6 +165,11 @@ pub struct McpServerOAuthConfig {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub client_secret: Option<RedactedString>,
|
||||
|
||||
/// Registered EMA client authentication method. Defaults to client_secret_basic
|
||||
/// when a secret is configured; requires a client secret.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub token_endpoint_auth_method: Option<McpEmaTokenEndpointAuthMethod>,
|
||||
|
||||
/// Registered callback URL associated with this OAuth client.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub callback_url: Option<String>,
|
||||
@@ -560,6 +566,20 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
|
||||
return Err("oauth.client_secret requires oauth.client_id".to_string());
|
||||
}
|
||||
}
|
||||
if let Some(oauth) = &oauth
|
||||
&& oauth.token_endpoint_auth_method.is_some()
|
||||
{
|
||||
if !matches!(auth, McpServerAuth::EmaAuth) {
|
||||
return Err(
|
||||
"oauth.token_endpoint_auth_method requires auth = \"ema_auth\"".to_string(),
|
||||
);
|
||||
}
|
||||
if oauth.client_secret.is_none() {
|
||||
return Err(
|
||||
"oauth.token_endpoint_auth_method requires oauth.client_secret".to_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
if !matches!(auth, McpServerAuth::EmaAuth)
|
||||
&& oauth
|
||||
.as_ref()
|
||||
|
||||
Reference in New Issue
Block a user