From f326857cf405fb254cf6c8f38766daff074fca6e Mon Sep 17 00:00:00 2001 From: riley-oai Date: Mon, 7 Sep 2026 16:59:04 +0000 Subject: [PATCH] Restrict MCP user verification and add workspace-scoped identity (#43524) ## Why Configured MCP servers must not be able to request user verification, even when they reuse the hosted apps server name. Local verification identity selection also needs to distinguish a user's workspace memberships. ## What changed - Advertise `userVerification` only to the host-owned apps MCP server and cancel verification requests from other servers before prompting the client. - Add `CodexAuth::get_chatgpt_account_user_id()` to read the access token's opaque `chatgpt_account_user_id` only when its workspace matches the selected account. Missing or malformed claims return no identity, without falling back to another user ID or breaking ordinary authentication. ## Testing Add coverage for capability filtering, rejection of configured servers including those using the hosted apps name, and account-user identity selection with malformed or mismatched claims. Add an MCP round-trip test showing that verification waits for a client response even in full access mode, returns the supplied proof, and discards response metadata. GitOrigin-RevId: 156e83d5a0302c013b85e564b46b4bfe858cbdb0 --- codex-rs/app-server/tests/suite/v2/mod.rs | 1 + .../tests/suite/v2/user_verification_mcp.rs | 218 ++++++++++++++++++ codex-rs/codex-mcp/src/client_capabilities.rs | 23 ++ .../src/client_capabilities_tests.rs | 47 ++++ codex-rs/codex-mcp/src/connection_manager.rs | 5 + codex-rs/codex-mcp/src/elicitation_tests.rs | 49 ++++ .../src/user_verification_elicitation.rs | 13 +- .../core/tests/suite/mcp_user_verification.rs | 88 ++----- .../login/src/auth/account_user_id_tests.rs | 156 +++++++++++++ codex-rs/login/src/auth/manager.rs | 18 ++ codex-rs/login/src/token_data.rs | 37 +++ 11 files changed, 587 insertions(+), 68 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs create mode 100644 codex-rs/login/src/auth/account_user_id_tests.rs diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 281c3814f8..ee91cd9569 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -132,3 +132,4 @@ mod web_search; mod windows_sandbox_setup; mod user_verification; +mod user_verification_mcp; diff --git a/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs b/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs new file mode 100644 index 0000000000..ba5af17f29 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/user_verification_mcp.rs @@ -0,0 +1,218 @@ +//! Exercises the MCP-to-core ceremony without depending on biometric hardware. + +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Result; +use axum::Router; +use codex_config::Constrained; +use codex_core::StartThreadOptions; +use codex_core::config::Config; +use codex_extension_api::ExtensionFuture; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::McpServerContribution; +use codex_extension_api::McpServerContributionContext; +use codex_extension_api::McpServerContributor; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_protocol::approvals::ElicitationRequest; +use codex_protocol::mcp::ClientMcpExtensions; +use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::ElicitationAction; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use core_test_support::apps_test_server::apps_enabled_builder; +use core_test_support::responses::start_mock_server; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use rmcp::ServerHandler; +use rmcp::model::CallToolRequestParams; +use rmcp::model::CallToolResponse; +use rmcp::model::CallToolResult; +use rmcp::model::ContentBlock; +use rmcp::model::CustomRequest; +use rmcp::model::ServerCapabilities; +use rmcp::model::ServerInfo; +use rmcp::model::ServerRequest; +use rmcp::service::RequestContext; +use rmcp::service::RoleServer; +use rmcp::transport::StreamableHttpServerConfig; +use rmcp::transport::StreamableHttpService; +use rmcp::transport::streamable_http_server::session::local::LocalSessionManager; +use serde_json::Value; +use serde_json::json; +use tokio::net::TcpListener; +use tokio::sync::Mutex; +use tokio::time::timeout; +use tokio_util::task::AbortOnDropHandle; + +#[derive(Clone)] +struct VerificationServer { + response: Arc>>, +} + +struct HostedVerificationServer(codex_config::McpServerConfig); + +impl McpServerContributor for HostedVerificationServer { + fn id(&self) -> &'static str { + "user-verification-integration-test" + } + + fn contribute<'a>( + &'a self, + _context: McpServerContributionContext<'a, Config>, + ) -> ExtensionFuture<'a, Vec> { + Box::pin(async move { + vec![McpServerContribution::HostedApps { + config: Box::new(self.0.clone()), + }] + }) + } +} + +impl ServerHandler for VerificationServer { + fn get_info(&self) -> ServerInfo { + ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) + } + + async fn call_tool( + &self, + _request: CallToolRequestParams, + context: RequestContext, + ) -> Result { + assert_eq!( + context + .peer + .peer_info() + .and_then(|info| info.capabilities.extensions.clone()) + .and_then(|extensions| extensions.get(OPENAI_ELICITATION_EXTENSION_ID).cloned()) + .map(Value::Object), + Some(json!({"userVerification": {}})), + ); + let result = context + .peer + .send_request(ServerRequest::CustomRequest(CustomRequest::new( + "openai/elicitation/create", + Some(json!({ + "mode": "openai/userVerification", + "title": "Approve purchase", + "description": "Pay $200 to Example Store", + "challenge": "AAECA_7_", + })), + ))) + .await + .map_err(|error| { + rmcp::ErrorData::internal_error(error.to_string(), /*data*/ None) + })?; + *self.response.lock().await = Some(serde_json::to_value(result).map_err(|error| { + rmcp::ErrorData::internal_error(error.to_string(), /*data*/ None) + })?); + Ok(CallToolResult::success(vec![ContentBlock::text("verified")]).into()) + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn user_verification_mcp_round_trip_requires_proof_in_full_access() -> Result<()> { + let received = Arc::new(Mutex::new(None)); + let server = VerificationServer { + response: Arc::clone(&received), + }; + let service = StreamableHttpService::new( + move || Ok(server.clone()), + Arc::new(LocalSessionManager::default()), + StreamableHttpServerConfig::default(), + ); + let listener = TcpListener::bind("127.0.0.1:0").await?; + let base_url = format!("http://{}", listener.local_addr()?); + let router = Router::new().nest_service("/api/codex/ps/mcp", service); + let _server = AbortOnDropHandle::new(tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + })); + let model_server = start_mock_server().await; + let mut extensions = ExtensionRegistryBuilder::new(); + let plugin_service_config = serde_json::from_value(json!({ + "url": format!("{base_url}/api/codex/ps/mcp"), + }))?; + extensions.mcp_server_contributor(Arc::new(HostedVerificationServer(plugin_service_config))); + let test = apps_enabled_builder(base_url) + .with_extensions(Arc::new(extensions.build())) + .with_config(|config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); + config + .permissions + .set_permission_profile(PermissionProfile::Disabled) + .unwrap(); + }) + .build_with_auto_env(&model_server) + .await?; + // Supply the trusted host capability through the public core API. The production + // app-server only activates it for supported in-process UIs with biometric hardware. + let thread = test + .thread_manager + .start_thread(StartThreadOptions { + client_mcp_extensions: ClientMcpExtensions::new([( + OPENAI_ELICITATION_EXTENSION_ID.to_string(), + json!({"userVerification": {}}), + )]), + environments: Some(vec![test.executor_environment().selection().clone()]), + ..StartThreadOptions::new(test.config.clone()) + }) + .await? + .thread; + let caller = Arc::clone(&thread); + let call = AbortOnDropHandle::new(tokio::spawn(async move { + caller + .call_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "verify_action", + /*arguments*/ None, + /*meta*/ None, + ) + .await + })); + let EventMsg::ElicitationRequest(request) = wait_for_event(&thread, |event| { + matches!(event, EventMsg::ElicitationRequest(_)) + }) + .await + else { + unreachable!() + }; + assert_eq!(request.server_name, CODEX_APPS_MCP_SERVER_NAME); + assert_eq!( + request.request, + ElicitationRequest::UserVerification { + title: "Approve purchase".into(), + description: "Pay $200 to Example Store".into(), + challenge: "AAECA_7_".into(), + } + ); + assert!(received.lock().await.is_none()); + assert!(!call.is_finished()); + let proof = json!({"credentialId": "AQID", "signature": "BAUG"}); + thread + .submit(Op::ResolveElicitation { + server_name: request.server_name, + request_id: request.id, + decision: ElicitationAction::Accept, + content: Some(proof.clone()), + meta: Some(json!({"untrusted": "discarded"})), + }) + .await?; + assert_eq!( + timeout(Duration::from_secs(/*secs*/ 10), call).await???, + codex_protocol::mcp::CallToolResult { + content: vec![json!({"type": "text", "text": "verified"})], + structured_content: None, + is_error: Some(false), + meta: None, + }, + ); + assert_eq!( + *received.lock().await, + Some(json!({"action": "accept", "content": proof})), + ); + thread.shutdown_and_wait().await?; + test.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/codex-mcp/src/client_capabilities.rs b/codex-rs/codex-mcp/src/client_capabilities.rs index 0efc8f8185..582891dbda 100644 --- a/codex-rs/codex-mcp/src/client_capabilities.rs +++ b/codex-rs/codex-mcp/src/client_capabilities.rs @@ -8,6 +8,29 @@ use codex_protocol::mcp::OPENAI_STANDARD_FORM_INPUT_EXTENSION_ID; use serde_json::Map; use serde_json::Value; +/// Restricts verification to the host-owned plugin service, even when another server uses its name. +pub(crate) fn server_mcp_extensions( + extensions: &ClientMcpExtensions, + server_name: &str, + server: Option<&crate::catalog::ResolvedMcpServer>, +) -> ClientMcpExtensions { + let plugin_service = server.is_some_and(|server| { + server + .source() + .is_host_owned_apps(server_name, server.config()) + }); + ClientMcpExtensions::new(extensions.iter().map(|(id, settings)| { + let mut settings = settings.clone(); + if !plugin_service + && id == OPENAI_ELICITATION_EXTENSION_ID + && let Some(settings) = settings.as_object_mut() + { + settings.remove("userVerification"); + } + (id.to_string(), settings) + })) +} + /// Selects the MCP extensions Codex supports from those declared by the app-server host. /// /// App-server clients may declare unrelated extensions. Codex retains only the diff --git a/codex-rs/codex-mcp/src/client_capabilities_tests.rs b/codex-rs/codex-mcp/src/client_capabilities_tests.rs index 726f76636a..be07334992 100644 --- a/codex-rs/codex-mcp/src/client_capabilities_tests.rs +++ b/codex-rs/codex-mcp/src/client_capabilities_tests.rs @@ -61,3 +61,50 @@ fn normalizes_legacy_form_capability_into_extensions() { )])) ); } + +#[test] +fn user_verification_is_projected_only_to_the_host_owned_plugin_service() { + use crate::catalog::McpServerRegistration; + use crate::catalog::ResolvedMcpCatalog; + use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; + use crate::mcp::codex_apps_mcp_server_config; + + let config = codex_apps_mcp_server_config( + "https://example.com", + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + ); + let extensions = ClientMcpExtensions::new([( + OPENAI_ELICITATION_EXTENSION_ID.to_string(), + json!({"form": {}, "userVerification": {}}), + )]); + for (name, registration, settings) in [ + ( + CODEX_APPS_MCP_SERVER_NAME, + McpServerRegistration::from_hosted_apps( + "host", + /*contribution_order*/ 0, + config.clone(), + ), + json!({"form": {}, "userVerification": {}}), + ), + ( + CODEX_APPS_MCP_SERVER_NAME, + McpServerRegistration::from_config(CODEX_APPS_MCP_SERVER_NAME.into(), config.clone()), + json!({"form": {}}), + ), + ( + "attached", + McpServerRegistration::from_config("attached".into(), config), + json!({"form": {}}), + ), + ] { + let mut catalog = ResolvedMcpCatalog::builder(); + catalog.register(registration); + let catalog = catalog.build(); + assert_eq!( + server_mcp_extensions(&extensions, name, catalog.server(name)), + ClientMcpExtensions::new([(OPENAI_ELICITATION_EXTENSION_ID.to_string(), settings)]), + ); + } +} diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 4ac5c1e689..2f184c2323 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -288,6 +288,11 @@ impl McpConnectionSet { .filter(|(_, server)| server.enabled()) { let registration = config.mcp_server_catalog.server(&server_name); + let client_mcp_extensions = crate::client_capabilities::server_mcp_extensions( + &client_mcp_extensions, + &server_name, + registration, + ); let is_host_owned_codex_apps = registration.is_some_and(|server| { server .source() diff --git a/codex-rs/codex-mcp/src/elicitation_tests.rs b/codex-rs/codex-mcp/src/elicitation_tests.rs index a1a67d30b4..3e9f0867f4 100644 --- a/codex-rs/codex-mcp/src/elicitation_tests.rs +++ b/codex-rs/codex-mcp/src/elicitation_tests.rs @@ -587,3 +587,52 @@ async fn user_verification_drops_pending_response_when_the_request_is_cancelled( assert!(pending.await.unwrap_err().is_cancelled()); assert!(manager.router.requests.lock().unwrap().is_empty()); } + +#[tokio::test] +async fn user_verification_rejects_attached_servers_even_if_they_use_the_plugin_service_name() { + for name in ["attached", crate::CODEX_APPS_MCP_SERVER_NAME] { + let (manager, _, _) = + verification_fixture(AskForApproval::OnRequest, /*reviewer*/ None); + { + let mut authority = manager.authority.lock().unwrap(); + let config = Arc::make_mut(&mut authority.as_mut().unwrap().config); + let mut catalog = crate::catalog::ResolvedMcpCatalog::builder(); + catalog.register(crate::catalog::McpServerRegistration::from_config( + name.into(), + crate::mcp::codex_apps_mcp_server_config( + "https://example.com", + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + ), + )); + config.mcp_server_catalog = catalog.build(); + } + let (tx, events) = async_channel::bounded(1); + let sender = manager.make_sender( + name.into(), + Some(tx), + &ClientMcpExtensions::new([( + OPENAI_ELICITATION_EXTENSION_ID.to_string(), + json!({"userVerification": {}}), + )]), + ); + assert_eq!( + sender( + RequestId::Number(7), + Elicitation::UserVerification { + title: "Approve".into(), + description: String::new(), + challenge: "AQID".into() + } + ) + .await + .unwrap(), + ElicitationResponse { + action: ElicitationAction::Cancel, + content: None, + meta: None + }, + ); + assert!(events.try_recv().is_err()); + } +} diff --git a/codex-rs/codex-mcp/src/user_verification_elicitation.rs b/codex-rs/codex-mcp/src/user_verification_elicitation.rs index 29226a6f23..5b2890400c 100644 --- a/codex-rs/codex-mcp/src/user_verification_elicitation.rs +++ b/codex-rs/codex-mcp/src/user_verification_elicitation.rs @@ -13,7 +13,18 @@ pub(super) async fn route( .lock() .ok() .and_then(|authority| authority.clone()); - let Some(events) = events.filter(|_| !router.auto_deny()) else { + let plugin_service = authority.as_ref().is_some_and(|authority| { + authority + .config + .mcp_server_catalog + .server(&server_name) + .is_some_and(|server| { + server + .source() + .is_host_owned_apps(&server_name, server.config()) + }) + }); + let Some(events) = events.filter(|_| plugin_service && !router.auto_deny()) else { return Ok(ElicitationResponse { action: ElicitationAction::Cancel, content: None, diff --git a/codex-rs/core/tests/suite/mcp_user_verification.rs b/codex-rs/core/tests/suite/mcp_user_verification.rs index d53b264b77..d9eb8ca90b 100644 --- a/codex-rs/core/tests/suite/mcp_user_verification.rs +++ b/codex-rs/core/tests/suite/mcp_user_verification.rs @@ -1,19 +1,17 @@ -//! Typed verification holds the enclosing tool until the client answers the request. +//! Configured MCP servers cannot activate verification or hold a turn waiting for proof. use anyhow::Result; use codex_core::StartThreadOptions; use codex_core::TurnInputRequest; -use codex_protocol::approvals::ElicitationRequest; use codex_protocol::mcp::ClientMcpExtensions; use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID; -use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; use core_test_support::responses; use core_test_support::skip_if_no_network; use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; @@ -68,9 +66,9 @@ enum CapabilitySource { } #[test_case(CapabilitySource::HostProjection; "host_projection_does_not_advertise_verification")] -#[test_case(CapabilitySource::ExplicitSession; "explicit_session_routes_typed_verification")] +#[test_case(CapabilitySource::ExplicitSession; "explicit_session_cannot_enable_configured_server")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn mcp_user_verification_transport(source: CapabilitySource) -> Result<()> { +async fn mcp_user_verification_rejects_configured_servers(source: CapabilitySource) -> Result<()> { skip_if_no_network!(Ok(())); skip_if_wine_exec!(Ok(()), "the MCP fixture requires a host Python interpreter"); let server = responses::start_mock_server().await; @@ -84,7 +82,7 @@ async fn mcp_user_verification_transport(source: CapabilitySource) -> Result<()> Some(&declarations), /*legacy_openai_form_elicitation*/ false, ), - // Exercise the typed route when the host directly supplies this capability. + // Even an explicitly enabled session cannot grant this capability to a configured server. CapabilitySource::ExplicitSession => ClientMcpExtensions::new(declarations), }; let mut config = test.config.clone(); @@ -131,47 +129,14 @@ async fn mcp_user_verification_transport(source: CapabilitySource) -> Result<()> text_elements: Vec::new(), }])) .await?; - let mut prompted = false; - tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 10), async { - loop { - match thread.next_event().await?.msg { - EventMsg::ElicitationRequest(request) => { - assert!(matches!(source, CapabilitySource::ExplicitSession)); - assert!(!prompted, "verification must only prompt once"); - assert_eq!( - request.request, - ElicitationRequest::UserVerification { - title: "Approve".into(), - description: "".into(), - challenge: "AQID".into(), - } - ); - assert!( - follow_up.requests().is_empty(), - "the tool must wait for the response" - ); - thread - .submit(Op::ResolveElicitation { - server_name: request.server_name, - request_id: request.id, - decision: ElicitationAction::Cancel, - content: None, - meta: None, - }) - .await?; - prompted = true; - } - EventMsg::TurnComplete(_) => break, - _ => {} - } - } - anyhow::Ok(()) + wait_for_event(&thread, |event| { + assert!( + !matches!(event, EventMsg::ElicitationRequest(_)), + "configured servers must not prompt for verification" + ); + matches!(event, EventMsg::TurnComplete(_)) }) - .await??; - assert_eq!( - prompted, - matches!(source, CapabilitySource::ExplicitSession) - ); + .await; let output = follow_up .single_request() .function_call_output("verify-call"); @@ -180,26 +145,15 @@ async fn mcp_user_verification_transport(source: CapabilitySource) -> Result<()> .as_str() .expect("MCP wire response"), )?; - match source { - CapabilitySource::HostProjection => { - assert!( - echoed["extensions"][OPENAI_ELICITATION_EXTENSION_ID] - .get("userVerification") - .is_none() - ); - assert_eq!( - echoed["response"], - json!({"code": -32601, "message": "openai/elicitation/create"}) - ); - } - CapabilitySource::ExplicitSession => { - assert_eq!( - echoed["extensions"][OPENAI_ELICITATION_EXTENSION_ID], - json!({"userVerification": {}}) - ); - assert_eq!(echoed["response"], json!({"action": "cancel"})); - } - } + assert!( + echoed["extensions"][OPENAI_ELICITATION_EXTENSION_ID] + .get("userVerification") + .is_none() + ); + assert_eq!( + echoed["response"], + json!({"code": -32601, "message": "openai/elicitation/create"}) + ); thread.shutdown_and_wait().await?; test.codex.shutdown_and_wait().await?; Ok(()) diff --git a/codex-rs/login/src/auth/account_user_id_tests.rs b/codex-rs/login/src/auth/account_user_id_tests.rs new file mode 100644 index 0000000000..ac9db21352 --- /dev/null +++ b/codex-rs/login/src/auth/account_user_id_tests.rs @@ -0,0 +1,156 @@ +//! Local account-user identity selection must use the access token's selected workspace. + +use super::*; +use base64::Engine; +use pretty_assertions::assert_eq; +use serde_json::json; + +fn access_token(auth_claims: Value) -> String { + let payload = json!({"https://api.openai.com/auth": auth_claims}); + let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD + .encode(serde_json::to_vec(&payload).unwrap()); + format!("e30.{payload}.c2ln") +} + +fn managed_auth(access_token: String, selected_account: Option<&str>) -> CodexAuth { + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let CodexAuth::Chatgpt(managed) = &auth else { + panic!("expected managed ChatGPT auth"); + }; + let mut state = managed.state.auth_dot_json.lock().unwrap(); + let tokens = state.as_mut().unwrap().tokens.as_mut().unwrap(); + tokens.access_token = access_token; + tokens.account_id = selected_account.map(str::to_owned); + tokens.id_token.chatgpt_user_id = Some("different-id-token-user".to_string()); + drop(state); + auth +} + +#[test] +fn account_user_id_comes_from_the_access_token_without_synthesis() { + let auth = managed_auth( + access_token(json!({ + "chatgpt_account_user_id": "opaque-membership", + "chatgpt_account_id": "workspace-a", + "chatgpt_user_id": "chatgpt-user", + "user_id": "auth-user", + })), + Some("workspace-a"), + ); + + assert_eq!( + auth.get_chatgpt_account_user_id(), + Some("opaque-membership".to_string()) + ); +} + +#[test] +fn account_user_id_rejects_missing_malformed_or_unscoped_claims() { + for claims in [ + json!(null), + json!({"user_id": "auth-user", "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_user_id": "chatgpt-user", "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_account_user_id": null, "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_account_user_id": "", "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_account_user_id": " membership ", "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_account_user_id": 123, "chatgpt_account_id": "workspace-a"}), + json!({"chatgpt_account_user_id": "membership"}), + json!({"chatgpt_account_user_id": "membership", "chatgpt_account_id": "workspace-b"}), + json!({"chatgpt_account_user_id": "membership", "chatgpt_account_id": 123}), + ] { + let auth = managed_auth(access_token(claims.clone()), Some("workspace-a")); + assert_eq!(auth.get_chatgpt_account_user_id(), None, "{claims}"); + } + + for token in [ + "not-a-jwt".to_string(), + "e30.!.c2ln".to_string(), + format!( + "{}.extra", + access_token(json!({ + "chatgpt_account_user_id": "membership", + "chatgpt_account_id": "workspace-a", + })) + ), + ] { + let auth = managed_auth(token, Some("workspace-a")); + assert_eq!(auth.get_chatgpt_account_user_id(), None); + } +} + +#[test] +fn account_user_id_requires_a_selected_account() { + for selected_account in [None, Some(""), Some(" workspace-a ")] { + let auth = managed_auth( + access_token(json!({ + "chatgpt_account_user_id": "membership", + "chatgpt_account_id": "workspace-a", + })), + selected_account, + ); + assert_eq!(auth.get_chatgpt_account_user_id(), None); + } +} + +#[test] +fn external_account_user_id_must_match_the_selected_workspace() { + let token = access_token(json!({ + "chatgpt_account_user_id": "membership-a", + "chatgpt_account_id": "workspace-a", + })); + for (selected_account, expected) in [ + ("workspace-a", Some("membership-a".to_string())), + ("workspace-b", None), + ] { + let auth = CodexAuth::from_external_chatgpt_tokens( + &token, + selected_account, + /*chatgpt_plan_type*/ None, + ) + .unwrap(); + assert_eq!(auth.get_chatgpt_account_user_id(), expected); + } +} + +#[test] +fn malformed_account_user_id_does_not_break_ordinary_external_auth() { + let token = access_token(json!({ + "chatgpt_account_user_id": 123, + "chatgpt_account_id": "workspace-a", + "chatgpt_user_id": "chatgpt-user", + })); + let auth = CodexAuth::from_external_chatgpt_tokens( + &token, + "workspace-a", + /*chatgpt_plan_type*/ None, + ) + .unwrap(); + + assert_eq!( + ( + auth.get_chatgpt_account_user_id(), + auth.get_chatgpt_user_id(), + auth.get_account_id(), + ), + ( + None, + Some("chatgpt-user".to_string()), + Some("workspace-a".to_string()), + ) + ); +} + +#[test] +fn api_key_and_header_auth_do_not_expose_an_account_user_id() { + let mut headers = http::HeaderMap::new(); + headers.insert( + "chatgpt-account-id", + http::HeaderValue::from_static("workspace-a"), + ); + for auth in [ + CodexAuth::from_api_key("test-api-key"), + CodexAuth::Headers(AuthHeaders::new(headers)), + ] { + assert_eq!(auth.get_chatgpt_account_user_id(), None); + } +} diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 18b94ef930..de1f26ca0c 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -59,6 +59,7 @@ use crate::default_client::create_client; use crate::default_client::create_default_auth_client; use crate::outbound_proxy::AuthRouteConfig; use crate::token_data::TokenData; +use crate::token_data::parse_chatgpt_account_user_id; use crate::token_data::parse_chatgpt_jwt_claims; use crate::token_data::parse_jwt_expiration; use codex_config::ManagedAuthPolicy; @@ -631,6 +632,19 @@ impl CodexAuth { } } + /// Returns the access token's opaque account-user identity only when its workspace + /// matches the selected account. Missing claims never fall back to a user id. + /// Unlike `get_chatgpt_user_id`, this identifies one workspace membership, so keys + /// are not shared across a person's workspaces. Workload-identity exchange claims + /// describe an agent identity and cannot select a human verification credential. + /// This is local identity selection, not access-token or proof validation. + pub fn get_chatgpt_account_user_id(&self) -> Option { + let tokens = self.get_current_token_data()?; + parse_chatgpt_account_user_id(&tokens.access_token, tokens.account_id.as_deref()?) + .ok() + .flatten() + } + /// Account-facing plan classification derived from the current auth. /// Returns a high-level `AccountPlanType` (e.g., Free/Plus/Pro/Team/…) /// for UI or product decisions based on the user's subscription. @@ -3070,3 +3084,7 @@ mod tests; #[cfg(test)] #[path = "change_state_tests.rs"] mod change_state_tests; + +#[cfg(test)] +#[path = "account_user_id_tests.rs"] +mod account_user_id_tests; diff --git a/codex-rs/login/src/token_data.rs b/codex-rs/login/src/token_data.rs index 2952ecb2a7..af80bf3733 100644 --- a/codex-rs/login/src/token_data.rs +++ b/codex-rs/login/src/token_data.rs @@ -104,6 +104,18 @@ struct StandardJwtClaims { exp: Option, } +#[derive(Deserialize)] +struct AccountUserClaims { + #[serde(rename = "https://api.openai.com/auth")] + auth: Option, +} + +#[derive(Deserialize)] +struct AccountUserAuthClaims { + chatgpt_account_user_id: Option, + chatgpt_account_id: Option, +} + #[derive(Debug, Error)] pub enum IdTokenInfoError { #[error("invalid ID token format")] @@ -134,6 +146,31 @@ pub fn parse_jwt_expiration(jwt: &str) -> Result>, IdTokenI .and_then(|exp| DateTime::::from_timestamp(exp, 0))) } +// Keep membership parsing separate from ordinary auth claims: a malformed optional +// membership must disable local verification without breaking an otherwise valid login. +// Both paths share decode_jwt_payload for the JWT envelope. +pub(crate) fn parse_chatgpt_account_user_id( + jwt: &str, + account_id: &str, +) -> Result, IdTokenInfoError> { + if account_id.is_empty() || account_id.trim() != account_id { + return Ok(None); + } + if jwt.split('.').count() != 3 { + return Err(IdTokenInfoError::InvalidFormat); + } + let claims: AccountUserClaims = decode_jwt_payload(jwt)?; + let Some(auth) = claims.auth else { + return Ok(None); + }; + if auth.chatgpt_account_id.as_deref() != Some(account_id) { + return Ok(None); + } + Ok(auth + .chatgpt_account_user_id + .filter(|id| !id.is_empty() && id.trim() == id.as_str())) +} + pub fn parse_chatgpt_jwt_claims(jwt: &str) -> Result { let claims: IdClaims = decode_jwt_payload(jwt)?; let email = claims