mirror of
https://github.com/openai/codex.git
synced 2026-04-24 22:54:54 +00:00
built from #14256. PR description from @etraut-openai: This PR addresses a hole in [PR 11802](https://github.com/openai/codex/pull/11802). The previous PR assumed that app server clients would respond to token refresh failures by presenting the user with an error ("you must log in again") and then not making further attempts to call network endpoints using the expired token. While they do present the user with this error, they don't prevent further attempts to call network endpoints and can repeatedly call `getAuthStatus(refreshToken=true)` resulting in many failed calls to the token refresh endpoint. There are three solutions I considered here: 1. Change the getAuthStatus app server call to return a null auth if the caller specified "refreshToken" on input and the refresh attempt fails. This will cause clients to immediately log out the user and return them to the log in screen. This is a really bad user experience. It's also a breaking change in the app server contract that could break third-party clients. 2. Augment the getAuthStatus app server call to return an additional field that indicates the state of "token could not be refreshed". This is a non-breaking change to the app server API, but it requires non-trivial changes for all clients to properly handle this new field properly. 3. Change the getAuthStatus implementation to handle the case where a token refresh fails by marking the AuthManager's in-memory access and refresh tokens as "poisoned" so it they are no longer used. This is the simplest fix that requires no client changes. I chose option 3. Here's Codex's explanation of this change: When an app-server client asks `getAuthStatus(refreshToken=true)`, we may try to refresh a stale ChatGPT access token. If that refresh fails permanently (for example `refresh_token_reused`, expired, or revoked), the old behavior was bad in two ways: 1. We kept the in-memory auth snapshot alive as if it were still usable. 2. Later auth checks could retry refresh again and again, creating a storm of doomed `/oauth/token` requests and repeatedly surfacing the same failure. This is especially painful for app-server clients because they poll auth status and can keep driving the refresh path without any real chance of recovery. This change makes permanent refresh failures terminal for the current managed auth snapshot without changing the app-server API contract. What changed: - `AuthManager` now poisons the current managed auth snapshot in memory after a permanent refresh failure, keyed to the unchanged `AuthDotJson`. - Once poisoned, later refresh attempts for that same snapshot fail fast locally without calling the auth service again. - The poison is cleared automatically when auth materially changes, such as a new login, logout, or reload of different auth state from storage. - `getAuthStatus(includeToken=true)` now omits `authToken` after a permanent refresh failure instead of handing out the stale cached bearer token. This keeps the current auth method visible to clients, avoids forcing an immediate logout flow, and stops repeated refresh attempts for credentials that cannot recover. --------- Co-authored-by: Eric Traut <etraut@openai.com>
527 lines
16 KiB
Rust
527 lines
16 KiB
Rust
use anyhow::Result;
|
|
use app_test_support::ChatGptAuthFixture;
|
|
use app_test_support::McpProcess;
|
|
use app_test_support::to_response;
|
|
use app_test_support::write_chatgpt_auth;
|
|
use chrono::Duration;
|
|
use chrono::Utc;
|
|
use codex_app_server_protocol::AuthMode;
|
|
use codex_app_server_protocol::GetAuthStatusParams;
|
|
use codex_app_server_protocol::GetAuthStatusResponse;
|
|
use codex_app_server_protocol::JSONRPCError;
|
|
use codex_app_server_protocol::JSONRPCResponse;
|
|
use codex_app_server_protocol::LoginAccountResponse;
|
|
use codex_app_server_protocol::RequestId;
|
|
use codex_core::auth::AuthCredentialsStoreMode;
|
|
use codex_core::auth::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
|
use pretty_assertions::assert_eq;
|
|
use std::path::Path;
|
|
use tempfile::TempDir;
|
|
use tokio::time::timeout;
|
|
use wiremock::Mock;
|
|
use wiremock::MockServer;
|
|
use wiremock::ResponseTemplate;
|
|
use wiremock::matchers::method;
|
|
use wiremock::matchers::path;
|
|
|
|
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
|
|
|
fn create_config_toml_custom_provider(
|
|
codex_home: &Path,
|
|
requires_openai_auth: bool,
|
|
) -> std::io::Result<()> {
|
|
let config_toml = codex_home.join("config.toml");
|
|
let requires_line = if requires_openai_auth {
|
|
"requires_openai_auth = true\n"
|
|
} else {
|
|
""
|
|
};
|
|
let contents = format!(
|
|
r#"
|
|
model = "mock-model"
|
|
approval_policy = "never"
|
|
sandbox_mode = "danger-full-access"
|
|
|
|
model_provider = "mock_provider"
|
|
|
|
[features]
|
|
shell_snapshot = false
|
|
|
|
[model_providers.mock_provider]
|
|
name = "Mock provider for test"
|
|
base_url = "http://127.0.0.1:0/v1"
|
|
wire_api = "responses"
|
|
request_max_retries = 0
|
|
stream_max_retries = 0
|
|
{requires_line}
|
|
"#
|
|
);
|
|
std::fs::write(config_toml, contents)
|
|
}
|
|
|
|
fn create_config_toml(codex_home: &Path) -> std::io::Result<()> {
|
|
let config_toml = codex_home.join("config.toml");
|
|
std::fs::write(
|
|
config_toml,
|
|
r#"
|
|
model = "mock-model"
|
|
approval_policy = "never"
|
|
sandbox_mode = "danger-full-access"
|
|
|
|
[features]
|
|
shell_snapshot = false
|
|
"#,
|
|
)
|
|
}
|
|
|
|
fn create_config_toml_forced_login(codex_home: &Path, forced_method: &str) -> std::io::Result<()> {
|
|
let config_toml = codex_home.join("config.toml");
|
|
let contents = format!(
|
|
r#"
|
|
model = "mock-model"
|
|
approval_policy = "never"
|
|
sandbox_mode = "danger-full-access"
|
|
forced_login_method = "{forced_method}"
|
|
|
|
[features]
|
|
shell_snapshot = false
|
|
"#
|
|
);
|
|
std::fs::write(config_toml, contents)
|
|
}
|
|
|
|
async fn login_with_api_key_via_request(mcp: &mut McpProcess, api_key: &str) -> Result<()> {
|
|
let request_id = mcp.send_login_account_api_key_request(api_key).await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let response: LoginAccountResponse = to_response(resp)?;
|
|
assert_eq!(response, LoginAccountResponse::ApiKey {});
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_no_auth() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
|
|
let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(false),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(status.auth_method, None, "expected no auth method");
|
|
assert_eq!(status.auth_token, None, "expected no token");
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_with_api_key() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
login_with_api_key_via_request(&mut mcp, "sk-test-key").await?;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(false),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(status.auth_method, Some(AuthMode::ApiKey));
|
|
assert_eq!(status.auth_token, Some("sk-test-key".to_string()));
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_with_api_key_when_auth_not_required() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml_custom_provider(codex_home.path(), false)?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
login_with_api_key_via_request(&mut mcp, "sk-test-key").await?;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(false),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(status.auth_method, None, "expected no auth method");
|
|
assert_eq!(status.auth_token, None, "expected no token");
|
|
assert_eq!(
|
|
status.requires_openai_auth,
|
|
Some(false),
|
|
"requires_openai_auth should be false",
|
|
);
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_with_api_key_no_include_token() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
login_with_api_key_via_request(&mut mcp, "sk-test-key").await?;
|
|
|
|
// Build params via struct so None field is omitted in wire JSON.
|
|
let params = GetAuthStatusParams {
|
|
include_token: None,
|
|
refresh_token: Some(false),
|
|
};
|
|
let request_id = mcp.send_get_auth_status_request(params).await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(status.auth_method, Some(AuthMode::ApiKey));
|
|
assert!(status.auth_token.is_none(), "token must be omitted");
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_with_api_key_refresh_requested() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
login_with_api_key_via_request(&mut mcp, "sk-test-key").await?;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(true),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(
|
|
status,
|
|
GetAuthStatusResponse {
|
|
auth_method: Some(AuthMode::ApiKey),
|
|
auth_token: Some("sk-test-key".to_string()),
|
|
requires_openai_auth: Some(true),
|
|
}
|
|
);
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_omits_token_after_permanent_refresh_failure() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
write_chatgpt_auth(
|
|
codex_home.path(),
|
|
ChatGptAuthFixture::new("stale-access-token")
|
|
.refresh_token("stale-refresh-token")
|
|
.account_id("acct_123")
|
|
.email("user@example.com")
|
|
.plan_type("pro"),
|
|
AuthCredentialsStoreMode::File,
|
|
)?;
|
|
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("POST"))
|
|
.and(path("/oauth/token"))
|
|
.respond_with(ResponseTemplate::new(401).set_body_json(serde_json::json!({
|
|
"error": {
|
|
"code": "refresh_token_reused"
|
|
}
|
|
})))
|
|
.expect(1)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let refresh_url = format!("{}/oauth/token", server.uri());
|
|
let mut mcp = McpProcess::new_with_env(
|
|
codex_home.path(),
|
|
&[
|
|
("OPENAI_API_KEY", None),
|
|
(
|
|
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
|
|
Some(refresh_url.as_str()),
|
|
),
|
|
],
|
|
)
|
|
.await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(true),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(
|
|
status,
|
|
GetAuthStatusResponse {
|
|
auth_method: Some(AuthMode::Chatgpt),
|
|
auth_token: None,
|
|
requires_openai_auth: Some(true),
|
|
}
|
|
);
|
|
|
|
let second_request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(true),
|
|
})
|
|
.await?;
|
|
|
|
let second_resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(second_request_id)),
|
|
)
|
|
.await??;
|
|
let second_status: GetAuthStatusResponse = to_response(second_resp)?;
|
|
assert_eq!(second_status, status);
|
|
|
|
server.verify().await;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_omits_token_after_proactive_refresh_failure() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
write_chatgpt_auth(
|
|
codex_home.path(),
|
|
ChatGptAuthFixture::new("stale-access-token")
|
|
.refresh_token("stale-refresh-token")
|
|
.account_id("acct_123")
|
|
.email("user@example.com")
|
|
.plan_type("pro")
|
|
.last_refresh(Some(Utc::now() - Duration::days(9))),
|
|
AuthCredentialsStoreMode::File,
|
|
)?;
|
|
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("POST"))
|
|
.and(path("/oauth/token"))
|
|
.respond_with(ResponseTemplate::new(401).set_body_json(serde_json::json!({
|
|
"error": {
|
|
"code": "refresh_token_reused"
|
|
}
|
|
})))
|
|
.expect(2)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let refresh_url = format!("{}/oauth/token", server.uri());
|
|
let mut mcp = McpProcess::new_with_env(
|
|
codex_home.path(),
|
|
&[
|
|
("OPENAI_API_KEY", None),
|
|
(
|
|
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
|
|
Some(refresh_url.as_str()),
|
|
),
|
|
],
|
|
)
|
|
.await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
let request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(false),
|
|
})
|
|
.await?;
|
|
|
|
let resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let status: GetAuthStatusResponse = to_response(resp)?;
|
|
assert_eq!(
|
|
status,
|
|
GetAuthStatusResponse {
|
|
auth_method: Some(AuthMode::Chatgpt),
|
|
auth_token: None,
|
|
requires_openai_auth: Some(true),
|
|
}
|
|
);
|
|
|
|
server.verify().await;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn get_auth_status_returns_token_after_proactive_refresh_recovery() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml(codex_home.path())?;
|
|
write_chatgpt_auth(
|
|
codex_home.path(),
|
|
ChatGptAuthFixture::new("stale-access-token")
|
|
.refresh_token("stale-refresh-token")
|
|
.account_id("acct_123")
|
|
.email("user@example.com")
|
|
.plan_type("pro")
|
|
.last_refresh(Some(Utc::now() - Duration::days(9))),
|
|
AuthCredentialsStoreMode::File,
|
|
)?;
|
|
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("POST"))
|
|
.and(path("/oauth/token"))
|
|
.respond_with(ResponseTemplate::new(401).set_body_json(serde_json::json!({
|
|
"error": {
|
|
"code": "refresh_token_reused"
|
|
}
|
|
})))
|
|
.expect(2)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let refresh_url = format!("{}/oauth/token", server.uri());
|
|
let mut mcp = McpProcess::new_with_env(
|
|
codex_home.path(),
|
|
&[
|
|
("OPENAI_API_KEY", None),
|
|
(
|
|
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
|
|
Some(refresh_url.as_str()),
|
|
),
|
|
],
|
|
)
|
|
.await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
let failed_request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(true),
|
|
})
|
|
.await?;
|
|
|
|
let failed_resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(failed_request_id)),
|
|
)
|
|
.await??;
|
|
let failed_status: GetAuthStatusResponse = to_response(failed_resp)?;
|
|
assert_eq!(
|
|
failed_status,
|
|
GetAuthStatusResponse {
|
|
auth_method: Some(AuthMode::Chatgpt),
|
|
auth_token: None,
|
|
requires_openai_auth: Some(true),
|
|
}
|
|
);
|
|
|
|
write_chatgpt_auth(
|
|
codex_home.path(),
|
|
ChatGptAuthFixture::new("recovered-access-token")
|
|
.refresh_token("recovered-refresh-token")
|
|
.account_id("acct_123")
|
|
.email("user@example.com")
|
|
.plan_type("pro")
|
|
.last_refresh(Some(Utc::now())),
|
|
AuthCredentialsStoreMode::File,
|
|
)?;
|
|
|
|
let recovered_request_id = mcp
|
|
.send_get_auth_status_request(GetAuthStatusParams {
|
|
include_token: Some(true),
|
|
refresh_token: Some(false),
|
|
})
|
|
.await?;
|
|
|
|
let recovered_resp: JSONRPCResponse = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(recovered_request_id)),
|
|
)
|
|
.await??;
|
|
let recovered_status: GetAuthStatusResponse = to_response(recovered_resp)?;
|
|
assert_eq!(
|
|
recovered_status,
|
|
GetAuthStatusResponse {
|
|
auth_method: Some(AuthMode::Chatgpt),
|
|
auth_token: Some("recovered-access-token".to_string()),
|
|
requires_openai_auth: Some(true),
|
|
}
|
|
);
|
|
|
|
server.verify().await;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn login_api_key_rejected_when_forced_chatgpt() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
create_config_toml_forced_login(codex_home.path(), "chatgpt")?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
|
|
let request_id = mcp
|
|
.send_login_account_api_key_request("sk-test-key")
|
|
.await?;
|
|
|
|
let err: JSONRPCError = timeout(
|
|
DEFAULT_READ_TIMEOUT,
|
|
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
|
|
assert_eq!(
|
|
err.error.message,
|
|
"API key login is disabled. Use ChatGPT login instead."
|
|
);
|
|
Ok(())
|
|
}
|