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

Preserve safe enterprise MCP authentication error details (#56153)

Browse Source 
## Why

Enterprise MCP token exchange and sign-in failures can collapse into generic errors, hiding useful diagnostics such as an OAuth `invalid_scope` rejection or a cancelled sign-in.

## What changed

Introduce `EmaErrorDetail` to carry static operation messages and sanitized SDK diagnostics through token-source errors and enterprise login completion. Exchange failures can report the stage, HTTP status, and allowlisted OAuth error code, such as `IdentityProvider: HTTP 400: invalid_scope`, without retaining arbitrary provider text or raw error chains.

Preserve specific discovery, credential storage, cancellation, and account/configuration mismatch errors instead of replacing them with generic failure messages.

## Testing

Extend tests to cover `invalid_scope` diagnostics, specific sign-in completion errors, and exclusion of credentials from diagnostics even when an outer error context contains sensitive text.

GitOrigin-RevId: e9280a5db3565ea91da0810bab528da5ee088d64
This commit is contained in:
jif
2026-10-22 11:51:23 +00:00
committed by copyberry
parent 61f34c35af
commit 67fb9da4b6
14 changed files with 313 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/account_processor/enterprise_login.rs
+23 -12
View File
@@ -3,9 +3,11 @@
//! The credential store separately fences logins invalidated by another process.
use super::*;
use anyhow::Context;
use codex_config::McpEmaRegistration;
use codex_config::types::AuthKeyringBackendKind;
use codex_mcp::ema_auth_scope;
use codex_rmcp_client::EmaErrorDetail;
use codex_rmcp_client::EnterpriseOAuthLoginRequest;
use codex_rmcp_client::perform_enterprise_oauth_login_return_url;
use futures::FutureExt;
@@ -21,12 +23,12 @@ pub(crate) struct EnterpriseLoginTarget {
pub(crate) enum EnterpriseLoginCompletion {
Pending,
Succeeded,
Failed,
Failed(EmaErrorDetail),
}
impl EnterpriseLoginCompletion {
pub(crate) fn is_complete(self) -> bool {
matches!(self, Self::Succeeded | Self::Failed)
matches!(self, Self::Succeeded | Self::Failed(_))
}
}
@@ -227,13 +229,19 @@ impl EnterpriseLoginState {
_ = setup_cancel.cancelled() => Err(internal_error("enterprise sign-in was cancelled")),
_ = cancel.cancelled() => Err(internal_error("enterprise sign-in was cancelled")),
result = perform_enterprise_oauth_login_return_url(request) =>
result.map_err(|_| internal_error("failed to start enterprise sign-in")),
result.map_err(|error| internal_error(
EmaErrorDetail::from_error(
&error.context(EmaErrorDetail::new("failed to start enterprise sign-in")),
).to_string(),
)),
};
let handle = match handle {
Ok(handle) => handle,
Err(error) => {
self.clear(&login_id).await;
completed.send_replace(EnterpriseLoginCompletion::Failed);
completed.send_replace(EnterpriseLoginCompletion::Failed(EmaErrorDetail::new(
"failed to start enterprise sign-in",
)));
return Err(error);
}
};
@@ -244,9 +252,12 @@ impl EnterpriseLoginState {
tokio::spawn(async move {
let result = tokio::select! {
biased;
_ = cancel.cancelled() => Err(anyhow::anyhow!("Enterprise sign-in was cancelled.")),
_ = cancel.cancelled() =>
Err(EmaErrorDetail::new("Enterprise sign-in was cancelled.").into()),
result = async {
let credentials = handle.wait().await?;
let credentials = handle.wait().await.context(EmaErrorDetail::new(
"Enterprise sign-in authorization failed.",
))?;
callback_closed.clone().await;
let mut active = credentials.commit_if(|| async {
lifecycle.auth_manager.reload().await;
@@ -273,7 +284,9 @@ impl EnterpriseLoginState {
// the grant, and let new threads pin only the committed credential.
lifecycle.config_manager.revoke_mcp_ema();
Some(active)
}).await?;
}).await.context(EmaErrorDetail::new(
"Enterprise sign-in credential commit failed.",
))?;
// A successful commit is no longer cancelable. Clear it while
// still holding the commit gate.
*active = None;
@@ -286,11 +299,9 @@ impl EnterpriseLoginState {
if result.is_err() {
lifecycle.clear(&completion_id).await;
}
let success = result.is_ok();
completed.send_replace(if success {
EnterpriseLoginCompletion::Succeeded
} else {
EnterpriseLoginCompletion::Failed
completed.send_replace(match result {
Ok(()) => EnterpriseLoginCompletion::Succeeded,
Err(error) => EnterpriseLoginCompletion::Failed(EmaErrorDetail::from_error(&error)),
});
});
Ok(EnterpriseLogin {
codex-rs/app-server/src/request_processors/mcp_processor.rs
+7 -1
View File
@@ -253,7 +253,13 @@ impl McpRequestProcessor {
login.login_id,
Box::pin(async move {
match completion.wait_for(|state| state.is_complete()).await {
Ok(result) if *result == EnterpriseLoginCompletion::Succeeded => Ok(()),
Ok(result) => match *result {
EnterpriseLoginCompletion::Succeeded => Ok(()),
EnterpriseLoginCompletion::Failed(detail) => Err(detail.into()),
EnterpriseLoginCompletion::Pending => {
Err(anyhow::anyhow!("Enterprise sign-in failed."))
}
},
_ => Err(anyhow::anyhow!("Enterprise sign-in failed.")),
}
}),
codex-rs/app-server/tests/suite/v2/account_enterprise_tests.rs
+12 -4
View File
@@ -530,7 +530,7 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
enterprise_callback(&replacement, &issuer, callback_port)?;
assert_eq!(
enterprise_completion(&mut client).await?,
json!({"name":"enterprise","threadId":thread_id,"loginId":first["loginId"],"success":false,"error":"Enterprise sign-in failed."})
json!({"name":"enterprise","threadId":thread_id,"loginId":first["loginId"],"success":false,"error":"Enterprise sign-in was cancelled."})
);
assert_eq!(
enterprise_rpc(
@@ -543,7 +543,7 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
);
assert_eq!(
enterprise_completion(&mut client).await?,
json!({"name":"enterprise","threadId":thread_id,"loginId":replacement["loginId"],"success":false,"error":"Enterprise sign-in failed."})
json!({"name":"enterprise","threadId":thread_id,"loginId":replacement["loginId"],"success":false,"error":"Enterprise sign-in was cancelled."})
);
let idp = McpServerIdpOAuthConfig {
issuer: issuer.clone(),
@@ -606,7 +606,7 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
assert_eq!(response.status(), http::StatusCode::OK);
assert_eq!(
enterprise_completion(&mut client).await?,
json!({"name":"enterprise","threadId":thread_id,"loginId":previous["loginId"],"success":false,"error":"Enterprise sign-in failed."})
json!({"name":"enterprise","threadId":thread_id,"loginId":previous["loginId"],"success":false,"error":"enterprise sign-in no longer matches the active account or configuration"})
);
let retry = enterprise_rpc(
@@ -967,6 +967,7 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
Some(codex_app_server_protocol::McpServerStartupFailureReason::ReauthenticationRequired);
for (error, reason) in [
(Some("invalid_client"), None),
(Some("invalid_scope"), None),
(Some("insufficient_user_authentication"), reauthentication),
(Some("invalid_grant"), reauthentication),
(None, reauthentication),
@@ -1002,6 +1003,13 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
})
.await??;
assert_eq!(failure.failure_reason, reason, "{error:?}: {failure:?}");
if error == Some("invalid_scope") {
assert!(
failure.error.as_deref().is_some_and(
|error| error.contains("IdentityProvider: HTTP 400: invalid_scope")
)
);
}
if reason.is_some() {
let status = enterprise_rpc(
&client,
@@ -1403,7 +1411,7 @@ oauth = {{ client_id = "mcp-client", client_secret = "resource-secret", token_en
.await??;
assert_eq!(
login_completion.context("enterprise login completion")?,
json!({"name":"enterprise","threadId":recovered["thread"]["id"],"loginId":stalled["loginId"],"success":false,"error":"Enterprise sign-in failed."})
json!({"name":"enterprise","threadId":recovered["thread"]["id"],"loginId":stalled["loginId"],"success":false,"error":"Enterprise sign-in was cancelled."})
);
assert_eq!(
resource_exchanges.load(std::sync::atomic::Ordering::SeqCst),
codex-rs/codex-mcp/src/ema_auth.rs
+15 -3
View File
@@ -21,6 +21,7 @@ use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_rmcp_client::EmaAuthFailure;
use codex_rmcp_client::EmaAuthenticatedHttpClient;
use codex_rmcp_client::EmaErrorDetail;
use codex_rmcp_client::EmaRequestAuthority;
use codex_rmcp_client::McpOAuthRefreshMode;
use codex_rmcp_client::McpProtocolMode;
@@ -124,7 +125,14 @@ impl EmaAuth {
pub(crate) async fn check_account(&self) -> Result<()> {
self.authority.check()?;
if !self.manager.account_login_is_current(&self.account).await? {
if !self
.manager
.account_login_is_current(&self.account)
.await
.context(EmaErrorDetail::new(
"enterprise MCP account credential validation failed",
))?
{
self.authority.revoke();
bail!(codex_rmcp_client::EmaTokenSourceError::new(
codex_rmcp_client::EmaCredentialKind::Account,
@@ -212,12 +220,16 @@ pub(crate) async fn stored_ema_credentials(
OAuthCredentialsStoreMode::Keyring,
keyring_backend,
)
.map_err(|_| anyhow!("failed to read enterprise credentials"))
.map_err(|_| anyhow!(EmaErrorDetail::new("failed to read enterprise credentials")))
})
})
})
.await
.map_err(|_| anyhow!("enterprise credential read task failed"))?
.map_err(|_| {
anyhow!(EmaErrorDetail::new(
"enterprise credential read task failed"
))
})?
}
// Discovery and both credential exchanges must check the owner at each dispatch,
codex-rs/codex-mcp/src/ema_token_source.rs
+48 -15
View File
@@ -16,6 +16,7 @@ use codex_rmcp_client::EmaAuthTokenExchangeRequest;
use codex_rmcp_client::EmaCredentialKind;
use codex_rmcp_client::EmaCredentialLease;
use codex_rmcp_client::EmaCredentialSnapshot;
use codex_rmcp_client::EmaErrorDetail;
use codex_rmcp_client::EmaIdpIdentityRequest;
use codex_rmcp_client::EmaInvalidGrantSource;
use codex_rmcp_client::EmaTokenSource;
@@ -61,7 +62,11 @@ impl EnterpriseTokenSource {
http_client: Arc<dyn HttpClient>,
) -> Result<Self> {
auth.check_account().await?;
let account = auth.manager.lease_account_oauth_grant().await?;
let account = auth
.manager
.lease_account_oauth_grant()
.await
.map_err(|_| EmaErrorDetail::new("enterprise MCP account credential read failed"))?;
let client_id = account
.as_ref()
.filter(|lease| {
@@ -112,7 +117,12 @@ impl EnterpriseTokenSource {
&& error
.downcast_ref::<EmaTokenSourceError>()
.is_some_and(EmaTokenSourceError::is_authentication_required)
&& let Some(lease) = self.auth.manager.lease_account_oauth_grant().await?
&& let Some(lease) = self
.auth
.manager
.lease_account_oauth_grant()
.await
.map_err(|_| EmaErrorDetail::new("enterprise MCP account credential read failed"))?
{
let rejected = self
.auth
@@ -143,7 +153,8 @@ impl EnterpriseTokenSource {
.auth
.manager
.lease_account_oauth_grant()
.await?
.await
.map_err(|_| EmaErrorDetail::new("enterprise MCP account credential read failed"))?
.ok_or(EmaAuthFailure::ReauthenticationRequired)?;
if !account_grant_is_usable(lease.tokens(), self.registration.idp())
|| lease
@@ -194,7 +205,10 @@ impl EnterpriseTokenSource {
.auth
.manager
.lease_account_oauth_grant()
.await?
.await
.map_err(|_| {
EmaErrorDetail::new("enterprise MCP account credential read failed")
})?
.ok_or(EmaAuthFailure::ReauthenticationRequired)?;
if AccountOAuthGrantRevision::from(lease.tokens())
!= AccountOAuthGrantRevision::from(tokens)
@@ -254,14 +268,22 @@ impl EnterpriseTokenSource {
.await?;
match receipt {
ExchangeReceipt::Account(grant) => {
if grant.current_tokens().await?.as_ref().is_none_or(|tokens| {
!account_grant_is_usable(tokens, idp)
|| self
.auth
.config
.ema_state
.requires_sign_in(&self.registration, tokens)
}) {
if grant
.current_tokens()
.await
.map_err(|_| {
EmaErrorDetail::new("enterprise MCP account credential validation failed")
})?
.as_ref()
.is_none_or(|tokens| {
!account_grant_is_usable(tokens, idp)
|| self
.auth
.config
.ema_state
.requires_sign_in(&self.registration, tokens)
})
{
// Discard this bearer. The bounded retry below reacquires the grant
// only if its credential changed under the same admitted owner.
bail!(EmaAuthFailure::ReauthenticationRequired);
@@ -305,8 +327,16 @@ impl EnterpriseTokenSource {
bail!(EmaTokenSourceError::unexpected(EmaCredentialKind::Account));
}
if idp_rejected(&error)
&& let Some(lease) =
self.auth.manager.lease_account_oauth_grant().await?
&& let Some(lease) = self
.auth
.manager
.lease_account_oauth_grant()
.await
.map_err(|_| {
EmaErrorDetail::new(
"enterprise MCP account credential read failed",
)
})?
{
if AccountOAuthGrantRevision::from(lease.tokens())
!= AccountOAuthGrantRevision::from(grant.tokens())
@@ -374,7 +404,10 @@ impl EmaTokenSource for EnterpriseTokenSource {
None if codex_rmcp_client::is_authentication_required_error(&error) => {
EmaTokenSourceError::new(kind, EmaAuthFailure::ReauthenticationRequired)
}
None => EmaTokenSourceError::unexpected(kind),
None => EmaTokenSourceError::Unexpected {
credential: kind,
detail: EmaErrorDetail::from_error(&error),
},
}
.into()
}
codex-rs/rmcp-client/src/ema_auth.rs
+7 -3
View File
@@ -16,6 +16,7 @@ use serde_json::Value;
use url::Url;
use crate::EmaCredentialLease;
use crate::EmaErrorDetail;
use crate::ema_auth_policy::advertised_capability;
use crate::ema_auth_policy::resolve_ema_client_auth;
use crate::ema_auth_policy::validate_ema_auth_resource;
@@ -95,7 +96,10 @@ pub async fn exchange_ema_auth_token<L: Send, R: Send>(
request.mcp_client_secret,
request.token_endpoint_auth_method,
)
.await?;
.await
.context(EmaErrorDetail::new(
"enterprise MCP authorization server discovery failed",
))?;
let issuer = metadata
.issuer
.as_deref()
@@ -179,9 +183,9 @@ async fn discover_ema_authorization_metadata(
continue;
};
if metadata.issuer.as_deref() != Some(issuer.as_str()) {
bail!(
bail!(EmaErrorDetail::new(
"enterprise authorization metadata issuer does not match its advertised issuer"
);
));
}
validate_ema_oauth_endpoint(
&metadata.token_endpoint,
codex-rs/rmcp-client/src/ema_auth_policy.rs
+81 -4
View File
@@ -5,6 +5,8 @@ use anyhow::Result;
use anyhow::anyhow;
use anyhow::bail;
use codex_config::McpEmaTokenEndpointAuthMethod;
use rmcp::transport::auth::enterprise::EmaError;
use rmcp::transport::auth::enterprise::EmaExchangeStage;
use serde_json::Value;
use url::Host;
use url::Url;
@@ -35,8 +37,74 @@ pub enum EmaCredentialKind {
Keyring,
}
/// Carries only a credential kind and an already sanitized auth failure to transport.
/// Unexpected errors deliberately retain neither provider text nor their source chain.
/// A diagnostic containing only static local text or sanitized SDK exchange fields.
/// Never retains an arbitrary error's display text, provider response, or source chain.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct EmaErrorDetail {
message: &'static str,
stage: Option<EmaExchangeStage>,
status: Option<u16>,
}
impl EmaErrorDetail {
pub fn new(message: &'static str) -> Self {
Self {
message,
stage: None,
status: None,
}
}
pub fn from_error(error: &anyhow::Error) -> Self {
// SDK diagnostics are already sanitized, including allowlisted OAuth codes.
// Prefer them to an outer operation label when both are available.
if let Some(error) = error.downcast_ref::<EmaError>() {
let (stage, status, message) = match error {
EmaError::InvalidRequest(message) => (None, None, *message),
EmaError::InvalidResponse { stage, message } => (Some(*stage), None, *message),
EmaError::RequestFailed(stage) => (Some(*stage), None, "request failed"),
EmaError::InvalidGrant(stage) => (Some(*stage), None, "invalid_grant"),
EmaError::InsufficientUserAuthentication(stage) => {
(Some(*stage), None, "insufficient_user_authentication")
}
EmaError::OAuthRejected {
stage,
status,
code,
} => (Some(*stage), Some(*status), *code),
_ => (None, None, "enterprise MCP token exchange failed"),
};
return Self {
message,
stage,
status,
};
}
error
.chain()
.filter_map(|cause| cause.downcast_ref::<Self>())
.last()
.copied()
.or_else(|| error.downcast_ref::<Self>().copied())
.unwrap_or_else(|| Self::new("enterprise MCP token exchange failed"))
}
}
impl std::fmt::Display for EmaErrorDetail {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(stage) = self.stage {
write!(f, "{stage:?}: ")?;
}
if let Some(status) = self.status {
write!(f, "HTTP {status}: ")?;
}
f.write_str(self.message)
}
}
impl std::error::Error for EmaErrorDetail {}
/// Carries a credential kind and sanitized failure to transport, without raw error chains.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum EmaTokenSourceError {
AuthFailure {
@@ -45,6 +113,7 @@ pub enum EmaTokenSourceError {
},
Unexpected {
credential: EmaCredentialKind,
detail: EmaErrorDetail,
},
}
@@ -57,12 +126,17 @@ impl EmaTokenSourceError {
}
pub fn unexpected(credential: EmaCredentialKind) -> Self {
Self::Unexpected { credential }
Self::Unexpected {
credential,
detail: EmaErrorDetail::new("enterprise MCP token exchange failed"),
}
}
fn credential(self) -> EmaCredentialKind {
match self {
Self::AuthFailure { credential, .. } | Self::Unexpected { credential } => credential,
Self::AuthFailure { credential, .. } | Self::Unexpected { credential, .. } => {
credential
}
}
}
@@ -107,6 +181,9 @@ impl EmaTokenSourceError {
impl std::fmt::Display for EmaTokenSourceError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Self::Unexpected { detail, .. } = self {
return std::fmt::Display::fmt(detail, f);
}
let message = match (self.credential(), self.failure()) {
(
_,
codex-rs/rmcp-client/src/ema_auth_tests.rs
+9 -9
View File
@@ -233,15 +233,15 @@ async fn modern_challenge_confines_metadata_redirects() -> Result<()> {
.with_priority(1)
.mount(&authorization)
.await;
let error = discover_ema_authorization_metadata(
EmaDiscoveryRequest::from(&exchange),
/*client_secret*/ None,
/*token_endpoint_auth_method*/ None,
)
.await
.err()
.context("cross-origin redirect must fail")?;
assert!(error.to_string().contains("cross-origin redirect"));
let error = exchange_ema_auth_token(exchange, std::convert::identity)
.await
.err()
.context("cross-origin redirect must fail")?;
assert!(format!("{error:#}").contains("cross-origin redirect"));
assert_eq!(
EmaErrorDetail::from_error(&error).to_string(),
"enterprise MCP authorization server discovery failed"
);
assert!(
attacker
.received_requests()
codex-rs/rmcp-client/src/ema_exchange_tests.rs
+22
View File
@@ -19,6 +19,9 @@ use wiremock::matchers::method;
use wiremock::matchers::path;
use super::*;
use crate::EmaCredentialKind;
use crate::EmaErrorDetail;
use crate::EmaTokenSourceError;
use crate::ema_auth_policy::validate_ema_oauth_endpoint;
use crate::ema_claims::ID_JAG_TOKEN_TYPE;
use crate::ema_claims::validate_oidc_identity_assertion;
@@ -387,6 +390,7 @@ async fn provider_errors_preserve_credential_policy_without_reflecting_credentia
"insufficient_user_authentication",
"unauthorized_client",
"invalid_client",
"invalid_scope",
"malformed",
] {
let server = MockServer::builder().start().await;
@@ -436,6 +440,24 @@ async fn provider_errors_preserve_credential_policy_without_reflecting_credentia
!format!("{error:#?}").contains(SENTINEL),
"provider reflected a credential"
);
// Even an unsafe outer context must not reach the transport diagnostic.
let detail = EmaErrorDetail::from_error(&error.context(SENTINEL));
let transport_error = EmaTokenSourceError::Unexpected {
credential: EmaCredentialKind::Keyring,
detail,
};
assert!(!format!("{transport_error:#?} {transport_error}").contains(SENTINEL));
if code == "invalid_scope" {
let stage = if endpoint == "/idp/token" {
"IdentityProvider"
} else {
"ResourceAuthorizationServer"
};
assert_eq!(
transport_error.to_string(),
format!("{stage}: HTTP 400: invalid_scope")
);
}
assert_eq!(
server.received_requests().await.expect("requests").len(),
if endpoint == "/as/token" { 2 } else { 1 },
codex-rs/rmcp-client/src/ema_identity.rs
+24 -7
View File
@@ -18,6 +18,7 @@ use rmcp::transport::auth::OAuthHttpClient;
use tracing::instrument::WithSubscriber;
use crate::EmaCredentialLease;
use crate::EmaErrorDetail;
use crate::ema_auth_policy::advertised_capability;
use crate::ema_auth_policy::ema_reauthentication_required;
use crate::ema_auth_policy::resolve_ema_client_auth;
@@ -96,7 +97,10 @@ pub async fn resolve_ema_account_idp_identity<L: Send>(
http_client,
redirect_mode,
)
.await?;
.await
.context(EmaErrorDetail::new(
"enterprise IdP discovery or validation failed",
))?;
Ok(EmaIdpIdentity {
token_endpoint,
refresh_token,
@@ -146,13 +150,19 @@ pub(crate) async fn resolve_ema_idp_authorization_manager(
let resolution = manager
.resolve_metadata()
.await
.context("failed to discover enterprise IdP authorization metadata")?;
.context(EmaErrorDetail::new(
"failed to discover enterprise IdP authorization metadata",
))?;
if !resolution.source.is_discovered() {
bail!("enterprise IdP must publish authorization metadata");
bail!(EmaErrorDetail::new(
"enterprise IdP must publish authorization metadata"
));
}
let metadata = resolution.metadata;
if metadata.issuer.as_deref() != Some(issuer) {
bail!("enterprise IdP authorization metadata issuer does not match configuration");
bail!(EmaErrorDetail::new(
"enterprise IdP authorization metadata issuer does not match configuration"
));
}
validate_ema_oauth_endpoint(&metadata.token_endpoint, "enterprise IdP token endpoint")?;
@@ -196,7 +206,10 @@ async fn resolve_ema_idp_identity_in<K: KeyringStore + Clone + 'static>(
request.http_client,
request.redirect_mode,
)
.await?;
.await
.context(EmaErrorDetail::new(
"enterprise IdP discovery or validation failed",
))?;
let credentials = request.credentials;
let credential_lock = RefreshCredentialLock::acquire_for_server(
&credentials.credentials.credentials().server_name,
@@ -204,7 +217,7 @@ async fn resolve_ema_idp_identity_in<K: KeyringStore + Clone + 'static>(
)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("failed to lock enterprise credentials"))?;
.map_err(|_| anyhow!(EmaErrorDetail::new("failed to lock enterprise credentials")))?;
let keyring_store = keyring_store.clone();
// The worker retains the short read lock if its caller stops waiting for the reread.
tokio::task::spawn_blocking(move || {
@@ -230,7 +243,11 @@ async fn resolve_ema_idp_identity_in<K: KeyringStore + Clone + 'static>(
})
})
.await
.map_err(|_| anyhow!("enterprise IdP credential reread task failed"))?
.map_err(|_| {
anyhow!(EmaErrorDetail::new(
"enterprise IdP credential reread task failed"
))
})?
}
async fn resolve_ema_idp_token_endpoint(
codex-rs/rmcp-client/src/enterprise_oauth_login.rs
+26 -9
View File
@@ -25,6 +25,7 @@ use tracing::instrument::WithSubscriber;
use url::Host;
use url::Url;
use crate::EmaErrorDetail;
use crate::StoredOAuthTokens;
use crate::ema_auth_policy::validate_ema_oauth_endpoint;
use crate::ema_claims::validate_oidc_identity_assertion;
@@ -138,7 +139,7 @@ impl EnterpriseOAuthLoginHandle {
.complete(/*emit_browser_url*/ false)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("enterprise IdP authorization failed"))?;
.map_err(|_| anyhow!(EmaErrorDetail::new("enterprise IdP authorization failed")))?;
validate_enterprise_credentials(&stored)?;
Ok(EnterpriseOAuthCredentials {
stored,
@@ -171,15 +172,20 @@ impl EnterpriseOAuthCredentials {
self.keyring_backend,
)
.await?;
let generation = guard
.generation_file
.current()
.map_err(|_| anyhow!("failed to read enterprise login generation"))?;
let generation = guard.generation_file.current().map_err(|_| {
anyhow!(EmaErrorDetail::new(
"failed to read enterprise login generation"
))
})?;
if generation.as_ref() != Some(&self.generation) {
bail!("enterprise sign-in was invalidated by logout");
bail!(EmaErrorDetail::new(
"enterprise sign-in was invalidated by logout"
));
}
let Some(authority) = is_current().await else {
bail!("enterprise sign-in no longer matches the active account or configuration");
bail!(EmaErrorDetail::new(
"enterprise sign-in no longer matches the active account or configuration"
));
};
tracing::subscriber::with_default(tracing::subscriber::NoSubscriber::default(), || {
save_oauth_tokens_with_lock_held(
@@ -190,7 +196,11 @@ impl EnterpriseOAuthCredentials {
self.keyring_backend,
)
})
.map_err(|_| anyhow!("failed to store enterprise credentials"))?;
.map_err(|_| {
anyhow!(EmaErrorDetail::new(
"failed to store enterprise credentials"
))
})?;
Ok(authority)
}
}
@@ -241,7 +251,14 @@ pub async fn perform_enterprise_oauth_login_return_url(
)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("failed to start enterprise IdP authorization"))?;
.map_err(|error| {
if error.downcast_ref::<EmaErrorDetail>().is_some() {
return EmaErrorDetail::from_error(&error);
}
EmaErrorDetail::from_error(&error.context(EmaErrorDetail::new(
"failed to start enterprise IdP authorization",
)))
})?;
// Keep the current attempt valid until replacement setup succeeds. A logout
// or another completed setup during this work must still invalidate this start.
let generation = async {
codex-rs/rmcp-client/src/enterprise_oauth_login_tests.rs
+23
View File
@@ -152,6 +152,22 @@ async fn enterprise_callback_errors_and_sdk_logs_exclude_credentials() -> Result
{
return Ok(());
}
let server = MockServer::start().await;
let issuer = format!("{}/idp", server.uri());
metadata(&server, &format!("https://{SECRET}.example/idp")).await;
let error = login(
&issuer, /*callback_url*/ None, /*auth_method*/ None,
)
.await
.err()
.expect("reject mismatched discovery issuer");
assert_eq!(
error.downcast_ref::<EmaErrorDetail>(),
Some(&EmaErrorDetail::new(
"failed to discover enterprise IdP authorization metadata"
))
);
assert!(!format!("{error} {error:?} {error:#}").contains(SECRET));
for (callback_error, token_success) in [(true, false), (false, false), (false, true)] {
let server = MockServer::start().await;
let issuer = format!("{}/idp", server.uri());
@@ -520,6 +536,13 @@ async fn enterprise_public_api_storage_and_privacy() -> Result<()> {
delete_enterprise_oauth_tokens(CREDENTIAL_NAME, &issuer, AuthKeyringBackendKind::Direct)
.await
.unwrap_err();
let save_error = save_error.context(EmaErrorDetail::new(
"enterprise sign-in credential commit failed",
));
assert_eq!(
EmaErrorDetail::from_error(&save_error).to_string(),
"failed to store enterprise credentials"
);
for error in [save_error, delete_error] {
assert!(!format!("{error} {error:?} {error:#}").contains(SECRET));
}
codex-rs/rmcp-client/src/lib.rs
+1
View File
@@ -51,6 +51,7 @@ pub use ema_auth::EmaAuthTokenExchangeRequest;
pub use ema_auth::exchange_ema_auth_token;
pub use ema_auth_policy::EmaAuthFailure;
pub use ema_auth_policy::EmaCredentialKind;
pub use ema_auth_policy::EmaErrorDetail;
pub use ema_auth_policy::EmaInvalidGrantSource;
pub use ema_auth_policy::EmaTokenSourceError;
pub use ema_auth_policy::validate_ema_auth_resource;
codex-rs/rmcp-client/src/oauth/ema_identity.rs
+15 -6
View File
@@ -11,6 +11,7 @@ use codex_keyring_store::KeyringStore;
use tracing::instrument::WithSubscriber;
use crate::ema_auth_policy::EmaAuthFailure;
use crate::ema_auth_policy::EmaErrorDetail;
use crate::ema_auth_policy::ema_reauthentication_required;
use crate::ema_claims::OidcClaims;
use crate::ema_claims::oidc_identity;
@@ -41,7 +42,7 @@ pub struct EmaCredentialLease {
fn sanitize_ema_credential_error(error: anyhow::Error, message: &'static str) -> anyhow::Error {
match error.downcast::<EmaAuthFailure>() {
Ok(failure) => anyhow::Error::new(failure).context(message),
Err(_) => anyhow!(message),
Err(_) => EmaErrorDetail::new(message).into(),
}
}
@@ -91,7 +92,7 @@ impl StoredOAuthCredentialSnapshot {
)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("failed to lock enterprise credentials"))?;
.map_err(|_| anyhow!(EmaErrorDetail::new("failed to lock enterprise credentials")))?;
let keyring_store = keyring_store.clone();
tokio::task::spawn_blocking(move || {
tracing::subscriber::with_default(tracing::subscriber::NoSubscriber::default(), || {
@@ -113,7 +114,11 @@ impl StoredOAuthCredentialSnapshot {
})
})
.await
.map_err(|_| anyhow!("enterprise credential pinning task failed"))?
.map_err(|_| {
anyhow!(EmaErrorDetail::new(
"enterprise credential pinning task failed"
))
})?
.map_err(|error: anyhow::Error| {
sanitize_ema_credential_error(error, "failed to pin enterprise credentials")
})
@@ -129,7 +134,7 @@ impl EmaCredentialSnapshot {
)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("failed to lock enterprise credentials"))?;
.map_err(|_| anyhow!(EmaErrorDetail::new("failed to lock enterprise credentials")))?;
let pinned = self.clone();
tokio::task::spawn_blocking(move || {
tracing::subscriber::with_default(tracing::subscriber::NoSubscriber::default(), || {
@@ -151,7 +156,11 @@ impl EmaCredentialSnapshot {
})
})
.await
.map_err(|_| anyhow!("enterprise credential validation task failed"))?
.map_err(|_| {
anyhow!(EmaErrorDetail::new(
"enterprise credential validation task failed"
))
})?
.map_err(|error: anyhow::Error| {
sanitize_ema_credential_error(error, "failed to validate enterprise credentials")
})
@@ -177,7 +186,7 @@ impl EmaCredentialSnapshot {
)
.with_subscriber(tracing::subscriber::NoSubscriber::default())
.await
.map_err(|_| anyhow!("failed to lock enterprise credentials"))?;
.map_err(|_| anyhow!(EmaErrorDetail::new("failed to lock enterprise credentials")))?;
if !is_current.await {
return Ok(());
}