mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Surface refresh-token rejection errors during workspace routing (#57304)
## Why When workspace discovery receives a 401 and token refresh is permanently rejected, the generic discovery error hides the guidance to sign in again. ## What changed Propagate cached permanent refresh-token failures for the current credentials through workspace routing and model-provider resolution as `CodexErr::RefreshTokenFailed`. Preserve account-change checks and only surface the cached rejection when external authentication is absent and the auth generation is unchanged. ## Testing Add an app-server regression test that simulates `refresh_token_reused` during workspace discovery and verifies that the turn fails with `Unauthorized` and tells the user to sign in again. GitOrigin-RevId: cf89c71fd6a2103e56d637838b0cb34d58b3cc32
This commit is contained in:
@@ -8,9 +8,11 @@ use codex_app_server_protocol::AccountRoutingOverride;
|
||||
use codex_backend_client::AccountEntry;
|
||||
use codex_config::ConfigLoadError;
|
||||
use codex_http_client::NetworkPolicyDenied;
|
||||
use codex_login::RefreshTokenError;
|
||||
use codex_login::WorkspaceRouting;
|
||||
use codex_login::WorkspaceRoutingRequest;
|
||||
use codex_login::WorkspaceRoutingResolver;
|
||||
use codex_login::auth::RefreshTokenFailedError;
|
||||
use codex_model_provider::ProviderAccount;
|
||||
use codex_model_provider::ProviderAccountError;
|
||||
use codex_model_provider::ProviderAccountState;
|
||||
@@ -70,6 +72,8 @@ pub(super) enum WorkspaceRoutingError {
|
||||
AccountChanged,
|
||||
#[error("workspace routing discovery unauthorized (401)")]
|
||||
DiscoveryUnauthorized,
|
||||
#[error(transparent)]
|
||||
RefreshTokenRejected(RefreshTokenFailedError),
|
||||
#[error("workspace routing discovery failed")]
|
||||
DiscoveryFailed,
|
||||
#[error("selected workspace missing from routing discovery")]
|
||||
@@ -188,6 +192,9 @@ impl WorkspaceRoutingResolver for AccountRequestProcessor {
|
||||
.map(|account| account.workspace_routing)
|
||||
.map_err(|error| match error {
|
||||
AccountReadError::InvalidAccount(error) => io::Error::other(error),
|
||||
AccountReadError::Routing(WorkspaceRoutingError::RefreshTokenRejected(
|
||||
error,
|
||||
)) => io::Error::new(io::ErrorKind::PermissionDenied, error),
|
||||
AccountReadError::Routing(error) => io::Error::new(
|
||||
if error == WorkspaceRoutingError::DiscoveryUnauthorized {
|
||||
io::ErrorKind::PermissionDenied
|
||||
@@ -398,14 +405,31 @@ impl AccountRequestProcessor {
|
||||
Ok(response) => break response,
|
||||
Err(error) => {
|
||||
let unauthorized = error.is_unauthorized();
|
||||
if unauthorized
|
||||
&& recovery.has_next()
|
||||
&& recovery.next().await.is_ok()
|
||||
{
|
||||
if unauthorized && recovery.has_next() {
|
||||
let result = recovery.next().await;
|
||||
let refreshed = *current_auth_changes.borrow();
|
||||
if refreshed.owner_generation != auth_state.owner_generation {
|
||||
return Err(WorkspaceRoutingError::AccountChanged.into());
|
||||
}
|
||||
if let Err(error) = result
|
||||
&& refreshed.generation == discovery_generation
|
||||
{
|
||||
let cached_failure = self
|
||||
.auth_manager
|
||||
.refresh_failure_for_auth(&discovery_auth);
|
||||
if let RefreshTokenError::Permanent(failed) = error
|
||||
&& !self.auth_manager.has_external_auth()
|
||||
&& cached_failure.as_ref() == Some(&failed)
|
||||
{
|
||||
return Err(
|
||||
WorkspaceRoutingError::RefreshTokenRejected(failed)
|
||||
.into(),
|
||||
);
|
||||
}
|
||||
return Err(
|
||||
WorkspaceRoutingError::DiscoveryUnauthorized.into()
|
||||
);
|
||||
}
|
||||
discovery_generation = refreshed.generation;
|
||||
discovery_auth = self
|
||||
.auth_manager
|
||||
|
||||
@@ -6,12 +6,19 @@ use app_test_support::ChatGptIdTokenClaims;
|
||||
use app_test_support::TestAppServer;
|
||||
use app_test_support::encode_id_token;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use app_test_support::write_models_cache;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
use codex_app_server_protocol::CodexErrorInfo;
|
||||
use codex_app_server_protocol::GetAccountParams;
|
||||
use codex_app_server_protocol::LoginAccountResponse;
|
||||
use codex_app_server_protocol::LogoutAccountResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -244,6 +251,76 @@ async fn stable_clients_do_not_treat_failed_workspace_discovery_as_unrestricted(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reused_refresh_token_tells_the_user_to_sign_in_again() -> Result<()> {
|
||||
let backend = MockServer::start().await;
|
||||
Mock::given(path("/backend-api/wham/config/bundle"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({})))
|
||||
.mount(&backend)
|
||||
.await;
|
||||
Mock::given(path("/backend-api/wham/accounts/check"))
|
||||
.respond_with(ResponseTemplate::new(401))
|
||||
.mount(&backend)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.respond_with(ResponseTemplate::new(401).set_body_json(json!({
|
||||
"error": {"code": "refresh_token_reused"}
|
||||
})))
|
||||
.mount(&backend)
|
||||
.await;
|
||||
let home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!(
|
||||
"chatgpt_base_url = '{0}/backend-api'\nopenai_base_url = '{0}/backend-api/codex'\ncli_auth_credentials_store = 'file'\n",
|
||||
backend.uri()
|
||||
),
|
||||
)?;
|
||||
write_models_cache(home.path()).await?;
|
||||
write_chatgpt_auth(
|
||||
home.path(),
|
||||
ChatGptAuthFixture::new("stale-token")
|
||||
.account_id("selected")
|
||||
.plan_type("pro"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
let refresh_url = format!("{}/oauth/token", backend.uri());
|
||||
let mut server = TestAppServer::builder()
|
||||
.with_codex_home(home.path())
|
||||
.with_env_overrides(&[
|
||||
("OPENAI_API_KEY", None),
|
||||
(REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR, Some(&refresh_url)),
|
||||
])
|
||||
.build_initialized_with_timeout(READ_TIMEOUT)
|
||||
.await?;
|
||||
let thread = server
|
||||
.start_thread(ThreadStartParams::default())
|
||||
.await?
|
||||
.thread;
|
||||
let completed = timeout(
|
||||
READ_TIMEOUT,
|
||||
server.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
input: vec![UserInput::Text {
|
||||
text: "hi".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(completed.turn.status, TurnStatus::Failed);
|
||||
let error = completed.turn.error.expect("failed turn error");
|
||||
assert_eq!(error.codex_error_info, Some(CodexErrorInfo::Unauthorized));
|
||||
assert!(
|
||||
error.message.contains("refresh token was already used"),
|
||||
"{error}"
|
||||
);
|
||||
assert!(error.message.contains("sign in again"), "{error}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn login_and_workspace_switch_notify_after_routing_is_ready_then_logout_clears_it()
|
||||
-> Result<()> {
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_models_manager::manager::OpenAiModelsManager;
|
||||
use codex_models_manager::manager::SharedModelsManager;
|
||||
use codex_models_manager::manager::StaticModelsManager;
|
||||
use codex_protocol::account::ProviderAccount;
|
||||
use codex_protocol::auth::RefreshTokenFailedError;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
|
||||
@@ -270,7 +271,11 @@ pub trait ModelProvider: fmt::Debug + Send + Sync {
|
||||
session: routing_context.session.clone(),
|
||||
},
|
||||
)
|
||||
.await?
|
||||
.await
|
||||
.map_err(|error| match error.downcast::<RefreshTokenFailedError>() {
|
||||
Ok(rejection) => CodexErr::RefreshTokenFailed(rejection),
|
||||
Err(error) => error.into(),
|
||||
})?
|
||||
{
|
||||
crate::workspace_routing::apply_workspace_routing(&mut provider, routing)?;
|
||||
redirect_policy = ClientRedirectPolicy::Reject;
|
||||
|
||||
Reference in New Issue
Block a user