From cf81d6f8ff853336fa842b6a970eb78c9b790f71 Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Wed, 3 Jun 2026 04:41:25 +0000 Subject: [PATCH] [codex-login] clear all native HTTP state on auth changes [ci changed_files] --- codex-rs/app-server/src/message_processor.rs | 2 +- codex-rs/app-server/src/request_processors.rs | 4 +- .../request_processors/account_processor.rs | 58 +++--------- codex-rs/app-server/tests/suite/v2/account.rs | 4 + codex-rs/http-state/src/lib.rs | 10 +++ codex-rs/login/src/auth/auth_tests.rs | 12 +++ codex-rs/login/src/auth/manager.rs | 90 ++----------------- codex-rs/login/src/http_state.rs | 27 ++++-- codex-rs/login/src/lib.rs | 1 - 9 files changed, 67 insertions(+), 141 deletions(-) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index e8e71ae16c..7d8ce2f85e 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1290,7 +1290,7 @@ impl MessageProcessor { } ClientRequest::LogoutAccount { .. } => { self.account_processor - .logout_account(request_id.clone(), app_server_client_name.as_deref()) + .logout_account(request_id.clone()) .await } ClientRequest::CancelLoginAccount { params, .. } => { diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index d5339985da..0c2d3beb83 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -341,9 +341,9 @@ use codex_login::CLIENT_ID; use codex_login::CodexAuth; use codex_login::ServerOptions as LoginServerOptions; use codex_login::ShutdownHandle; -use codex_login::auth::login_with_chatgpt_auth_tokens_for_surface; +use codex_login::auth::login_with_chatgpt_auth_tokens; use codex_login::complete_device_code_login; -use codex_login::login_with_api_key_for_surface; +use codex_login::login_with_api_key; use codex_login::request_device_code; use codex_login::run_login_server; use codex_mcp::McpRuntimeContext; diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index a0848df126..74722428f2 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -100,11 +100,8 @@ impl AccountRequestProcessor { pub(crate) async fn logout_account( &self, request_id: ConnectionRequestId, - app_server_client_name: Option<&str>, ) -> Result, JSONRPCErrorError> { - self.logout_v2(request_id, Self::http_state_surface(app_server_client_name)) - .await - .map(|()| None) + self.logout_v2(request_id).await.map(|()| None) } pub(crate) async fn cancel_login_account( @@ -229,12 +226,8 @@ impl AccountRequestProcessor { ) -> Result<(), JSONRPCErrorError> { match params { LoginAccountParams::ApiKey { api_key } => { - self.login_api_key_v2( - request_id, - LoginApiKeyParams { api_key }, - http_state_surface, - ) - .await; + self.login_api_key_v2(request_id, LoginApiKeyParams { api_key }) + .await; } LoginAccountParams::Chatgpt { codex_streamlined_login, @@ -256,7 +249,6 @@ impl AccountRequestProcessor { access_token, chatgpt_account_id, chatgpt_plan_type, - http_state_surface, ) .await; } @@ -273,7 +265,6 @@ impl AccountRequestProcessor { async fn login_api_key_common( &self, params: &LoginApiKeyParams, - http_state_surface: HttpStateSurface, ) -> std::result::Result<(), JSONRPCErrorError> { if self.auth_manager.is_external_chatgpt_auth_active() { return Err(self.external_auth_active_error()); @@ -296,11 +287,10 @@ impl AccountRequestProcessor { } } - match login_with_api_key_for_surface( + match login_with_api_key( &self.config.codex_home, ¶ms.api_key, self.config.cli_auth_credentials_store_mode, - http_state_surface, ) { Ok(()) => { self.auth_manager.reload().await; @@ -310,14 +300,9 @@ impl AccountRequestProcessor { } } - async fn login_api_key_v2( - &self, - request_id: ConnectionRequestId, - params: LoginApiKeyParams, - http_state_surface: HttpStateSurface, - ) { + async fn login_api_key_v2(&self, request_id: ConnectionRequestId, params: LoginApiKeyParams) { let result = self - .login_api_key_common(¶ms, http_state_surface) + .login_api_key_common(¶ms) .await .map(|()| LoginAccountResponse::ApiKey {}); let logged_in = result.is_ok(); @@ -578,15 +563,9 @@ impl AccountRequestProcessor { access_token: String, chatgpt_account_id: String, chatgpt_plan_type: Option, - http_state_surface: HttpStateSurface, ) { let result = self - .login_chatgpt_auth_tokens_response( - access_token, - chatgpt_account_id, - chatgpt_plan_type, - http_state_surface, - ) + .login_chatgpt_auth_tokens_response(access_token, chatgpt_account_id, chatgpt_plan_type) .await; let logged_in = result.is_ok(); self.outgoing.send_result(request_id, result).await; @@ -602,7 +581,6 @@ impl AccountRequestProcessor { access_token: String, chatgpt_account_id: String, chatgpt_plan_type: Option, - http_state_surface: HttpStateSurface, ) -> Result { if matches!( self.config.forced_login_method, @@ -629,12 +607,11 @@ impl AccountRequestProcessor { ))); } - login_with_chatgpt_auth_tokens_for_surface( + login_with_chatgpt_auth_tokens( &self.config.codex_home, &access_token, &chatgpt_account_id, chatgpt_plan_type.as_deref(), - http_state_surface, ) .map_err(|err| internal_error(format!("failed to set external auth: {err}")))?; self.auth_manager.reload().await; @@ -719,10 +696,7 @@ impl AccountRequestProcessor { } } - async fn logout_common( - &self, - http_state_surface: HttpStateSurface, - ) -> std::result::Result, JSONRPCErrorError> { + async fn logout_common(&self) -> std::result::Result, JSONRPCErrorError> { // Cancel any active login attempt. { let mut guard = self.active_login.lock().await; @@ -731,11 +705,7 @@ impl AccountRequestProcessor { } } - match self - .auth_manager - .logout_with_revoke_for_surface(http_state_surface) - .await - { + match self.auth_manager.logout_with_revoke().await { Ok(_) => {} Err(err) => { return Err(internal_error(format!("logout failed: {err}"))); @@ -757,12 +727,8 @@ impl AccountRequestProcessor { .map(CodexAuth::api_auth_mode)) } - async fn logout_v2( - &self, - request_id: ConnectionRequestId, - http_state_surface: HttpStateSurface, - ) -> Result<(), JSONRPCErrorError> { - let result = self.logout_common(http_state_surface).await; + async fn logout_v2(&self, request_id: ConnectionRequestId) -> Result<(), JSONRPCErrorError> { + let result = self.logout_common().await; let account_updated = result .as_ref() diff --git a/codex-rs/app-server/tests/suite/v2/account.rs b/codex-rs/app-server/tests/suite/v2/account.rs index 3fee010440..4c61447ac0 100644 --- a/codex-rs/app-server/tests/suite/v2/account.rs +++ b/codex-rs/app-server/tests/suite/v2/account.rs @@ -217,6 +217,7 @@ async fn logout_account_removes_auth_and_notifies() -> Result<()> { HttpStateSurface::CodexDesktopSsh, INTEGRITY_STATE.to_string(), )?; + http_state.set(HttpStateSurface::CodexCli, INTEGRITY_STATE.to_string())?; assert!(codex_home.path().join("auth.json").exists()); let mut mcp = @@ -256,6 +257,7 @@ async fn logout_account_removes_auth_and_notifies() -> Result<()> { "auth.json should be deleted" ); assert_eq!(http_state.get(HttpStateSurface::CodexDesktopSsh)?, None); + assert_eq!(http_state.get(HttpStateSurface::CodexCli)?, None); let get_id = mcp .send_get_account_request(GetAccountParams { @@ -942,6 +944,7 @@ async fn login_account_api_key_succeeds_and_notifies() -> Result<()> { HttpStateSurface::CodexDesktopSsh, INTEGRITY_STATE.to_string(), )?; + http_state.set(HttpStateSurface::CodexCli, INTEGRITY_STATE.to_string())?; let mut mcp = TestAppServer::new(codex_home.path()).await?; let initialized = timeout( DEFAULT_READ_TIMEOUT, @@ -988,6 +991,7 @@ async fn login_account_api_key_succeeds_and_notifies() -> Result<()> { assert!(codex_home.path().join("auth.json").exists()); assert_eq!(http_state.get(HttpStateSurface::CodexDesktopSsh)?, None); + assert_eq!(http_state.get(HttpStateSurface::CodexCli)?, None); Ok(()) } diff --git a/codex-rs/http-state/src/lib.rs b/codex-rs/http-state/src/lib.rs index 96f3ea398c..c53e43f824 100644 --- a/codex-rs/http-state/src/lib.rs +++ b/codex-rs/http-state/src/lib.rs @@ -29,6 +29,16 @@ pub enum HttpStateSurface { } impl HttpStateSurface { + pub const ALL: [Self; 7] = [ + Self::CodexCli, + Self::CodexTui, + Self::CodexExec, + Self::CodexVscode, + Self::CodexDesktop, + Self::CodexDesktopSsh, + Self::CodexRemoteControl, + ]; + pub fn try_from_app_server_client_name(client_name: &str) -> Option { match client_name { "codex_cli" => Some(Self::CodexCli), diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 63ab3e0c42..955e493a57 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -3,6 +3,8 @@ use crate::auth::storage::FileAuthStorage; use crate::auth::storage::get_auth_file; use crate::token_data::IdTokenInfo; use codex_app_server_protocol::AuthMode; +use codex_http_state::HttpStateStore; +use codex_http_state::HttpStateSurface; use codex_protocol::account::PlanType as AccountPlanType; use codex_protocol::auth::KnownPlan as InternalKnownPlan; use codex_protocol::auth::PlanType as InternalPlanType; @@ -791,6 +793,10 @@ async fn enforce_login_restrictions_logs_out_for_method_mismatch() { let _access_token_guard = remove_access_token_env_var(); login_with_api_key(codex_home.path(), "sk-test", AuthCredentialsStoreMode::File) .expect("seed api key"); + let http_state = HttpStateStore::new(codex_home.path().to_path_buf()); + http_state + .set(HttpStateSurface::CodexTui, "stale-state".to_string()) + .expect("seed TUI state"); let config = build_config( codex_home.path(), @@ -807,6 +813,12 @@ async fn enforce_login_restrictions_logs_out_for_method_mismatch() { !codex_home.path().join("auth.json").exists(), "auth.json should be removed on mismatch" ); + assert_eq!( + http_state + .get(HttpStateSurface::CodexTui) + .expect("TUI state should load"), + None, + ); } #[tokio::test] diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 3ecbf78553..0cbb68bdbd 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -21,7 +21,6 @@ use codex_agent_identity::decode_agent_identity_jwt; use codex_agent_identity::fetch_agent_identity_jwks; use codex_app_server_protocol::AuthMode; use codex_app_server_protocol::AuthMode as ApiAuthMode; -use codex_http_state::HttpStateSurface; use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::ModelProviderAuthInfo; @@ -516,19 +515,6 @@ pub fn logout( pub async fn logout_with_revoke( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, -) -> std::io::Result { - logout_with_revoke_for_surface( - codex_home, - auth_credentials_store_mode, - HttpStateSurface::CodexCli, - ) - .await -} - -pub async fn logout_with_revoke_for_surface( - codex_home: &Path, - auth_credentials_store_mode: AuthCredentialsStoreMode, - http_state_surface: HttpStateSurface, ) -> std::io::Result { AuthManager::new( codex_home.to_path_buf(), @@ -537,7 +523,7 @@ pub async fn logout_with_revoke_for_surface( /*chatgpt_base_url*/ None, ) .await - .logout_with_revoke_for_surface(http_state_surface) + .logout_with_revoke() .await } @@ -546,20 +532,6 @@ pub fn login_with_api_key( codex_home: &Path, api_key: &str, auth_credentials_store_mode: AuthCredentialsStoreMode, -) -> std::io::Result<()> { - login_with_api_key_for_surface( - codex_home, - api_key, - auth_credentials_store_mode, - HttpStateSurface::CodexCli, - ) -} - -pub fn login_with_api_key_for_surface( - codex_home: &Path, - api_key: &str, - auth_credentials_store_mode: AuthCredentialsStoreMode, - http_state_surface: HttpStateSurface, ) -> std::io::Result<()> { let auth_dot_json = AuthDotJson { auth_mode: Some(ApiAuthMode::ApiKey), @@ -569,7 +541,7 @@ pub fn login_with_api_key_for_surface( agent_identity: None, }; save_auth(codex_home, &auth_dot_json, auth_credentials_store_mode)?; - crate::http_state::clear(codex_home, http_state_surface); + crate::http_state::clear_all(codex_home); Ok(()) } @@ -579,23 +551,6 @@ pub async fn login_with_access_token( access_token: &str, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option<&str>, -) -> std::io::Result<()> { - login_with_access_token_for_surface( - codex_home, - access_token, - auth_credentials_store_mode, - chatgpt_base_url, - HttpStateSurface::CodexCli, - ) - .await -} - -pub async fn login_with_access_token_for_surface( - codex_home: &Path, - access_token: &str, - auth_credentials_store_mode: AuthCredentialsStoreMode, - chatgpt_base_url: Option<&str>, - http_state_surface: HttpStateSurface, ) -> std::io::Result<()> { let base_url = chatgpt_base_url .unwrap_or(DEFAULT_CHATGPT_BACKEND_BASE_URL) @@ -610,7 +565,7 @@ pub async fn login_with_access_token_for_surface( agent_identity: Some(access_token.to_string()), }; save_auth(codex_home, &auth_dot_json, auth_credentials_store_mode)?; - crate::http_state::clear(codex_home, http_state_surface); + crate::http_state::clear_all(codex_home); Ok(()) } @@ -620,22 +575,6 @@ pub fn login_with_chatgpt_auth_tokens( access_token: &str, chatgpt_account_id: &str, chatgpt_plan_type: Option<&str>, -) -> std::io::Result<()> { - login_with_chatgpt_auth_tokens_for_surface( - codex_home, - access_token, - chatgpt_account_id, - chatgpt_plan_type, - HttpStateSurface::CodexCli, - ) -} - -pub fn login_with_chatgpt_auth_tokens_for_surface( - codex_home: &Path, - access_token: &str, - chatgpt_account_id: &str, - chatgpt_plan_type: Option<&str>, - http_state_surface: HttpStateSurface, ) -> std::io::Result<()> { let auth_dot_json = AuthDotJson::from_external_access_token( access_token, @@ -647,7 +586,7 @@ pub fn login_with_chatgpt_auth_tokens_for_surface( &auth_dot_json, AuthCredentialsStoreMode::Ephemeral, )?; - crate::http_state::clear(codex_home, http_state_surface); + crate::http_state::clear_all(codex_home); Ok(()) } @@ -778,7 +717,7 @@ fn logout_with_message( // External auth tokens live in the ephemeral store, but persistent auth may still exist // from earlier logins. Clear both so a forced logout truly removes all active auth. let removal_result = logout_all_stores(codex_home, auth_credentials_store_mode); - crate::http_state::clear(codex_home, HttpStateSurface::CodexCli); + crate::http_state::clear_all(codex_home); let error_message = match removal_result { Ok(_) => message, Err(err) => format!("{message}. Failed to remove auth.json: {err}"), @@ -1841,29 +1780,14 @@ impl AuthManager { /// reloads the in‑memory auth cache so callers immediately observe the /// unauthenticated state. pub async fn logout(&self) -> std::io::Result { - self.logout_for_surface(HttpStateSurface::CodexCli).await - } - - pub async fn logout_for_surface( - &self, - http_state_surface: HttpStateSurface, - ) -> std::io::Result { let result = logout_all_stores(&self.codex_home, self.auth_credentials_store_mode); - crate::http_state::clear(&self.codex_home, http_state_surface); + crate::http_state::clear_all(&self.codex_home); // Always reload to clear any cached auth (even if file absent). self.reload().await; result } pub async fn logout_with_revoke(&self) -> std::io::Result { - self.logout_with_revoke_for_surface(HttpStateSurface::CodexCli) - .await - } - - pub async fn logout_with_revoke_for_surface( - &self, - http_state_surface: HttpStateSurface, - ) -> std::io::Result { let auth_dot_json = self .auth_cached() .and_then(|auth| auth.get_current_auth_json()); @@ -1871,7 +1795,7 @@ impl AuthManager { tracing::warn!("failed to revoke auth tokens during logout: {err}"); } let result = logout_all_stores(&self.codex_home, self.auth_credentials_store_mode); - crate::http_state::clear(&self.codex_home, http_state_surface); + crate::http_state::clear_all(&self.codex_home); // Always reload to clear any cached auth (even if file absent). self.reload().await; result diff --git a/codex-rs/login/src/http_state.rs b/codex-rs/login/src/http_state.rs index 95d3e75af3..afc5656840 100644 --- a/codex-rs/login/src/http_state.rs +++ b/codex-rs/login/src/http_state.rs @@ -10,18 +10,20 @@ pub(crate) fn replace_after_login( state: Option, ) { let store = HttpStateStore::new(codex_home.to_path_buf()); - let result = match state { - Some(state) => store.set(surface, state), - None => store.clear(surface), - }; - if let Err(err) = result { + clear_all(codex_home); + if let Some(state) = state + && let Err(err) = store.set(surface, state) + { warn!(%surface, "failed to reset HTTP state after login: {err}"); } } -pub(crate) fn clear(codex_home: &Path, surface: HttpStateSurface) { - if let Err(err) = HttpStateStore::new(codex_home.to_path_buf()).clear(surface) { - warn!(%surface, "failed to clear HTTP state: {err}"); +pub(crate) fn clear_all(codex_home: &Path) { + let store = HttpStateStore::new(codex_home.to_path_buf()); + for surface in HttpStateSurface::ALL { + if let Err(err) = store.clear(surface) { + warn!(%surface, "failed to clear HTTP state: {err}"); + } } } @@ -34,12 +36,21 @@ mod tests { fn replace_after_login_sets_and_clears_surface_state() { let codex_home = TempDir::new().expect("tempdir"); let store = HttpStateStore::new(codex_home.path().to_path_buf()); + store + .set(HttpStateSurface::CodexCli, "stale-cli-state".to_string()) + .expect("CLI state should store"); replace_after_login( codex_home.path(), HttpStateSurface::CodexDesktop, Some("minted-state".to_string()), ); + assert_eq!( + store + .get(HttpStateSurface::CodexCli) + .expect("CLI state should load"), + None, + ); assert_eq!( store .get(HttpStateSurface::CodexDesktop) diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index 4524e01700..a147ff8a84 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -41,7 +41,6 @@ pub use auth::enforce_login_restrictions; pub use auth::load_auth_dot_json; pub use auth::login_with_access_token; pub use auth::login_with_api_key; -pub use auth::login_with_api_key_for_surface; pub use auth::logout; pub use auth::logout_with_revoke; pub use auth::read_codex_access_token_from_env;