mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Support staging OAuth client ID overrides (#28257)
## Summary - allow app-server ChatGPT login to use a configured OAuth client ID - reuse the same client ID for refresh and revoke requests - cover staging login, refresh, and revoke request payloads ## Tests - `just test -p codex-login` - `just test -p codex-app-server login_account_chatgpt_uses_debug_oauth_overrides` - `just test -p codex-login logout_with_revoke_revokes_refresh_token_then_removes_auth` - `just fix -p codex-login` - `just fix -p codex-app-server` - `just fmt`
This commit is contained in:
committed by
GitHub
Unverified
parent
aae4c0b02b
commit
336f907ec1
@@ -353,13 +353,13 @@ use codex_feedback::FeedbackUploadOptions;
|
||||
use codex_git_utils::git_diff_to_remote;
|
||||
use codex_git_utils::resolve_root_git_project_for_trust;
|
||||
use codex_login::AuthManager;
|
||||
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;
|
||||
use codex_login::complete_device_code_login;
|
||||
use codex_login::login_with_api_key;
|
||||
use codex_login::oauth_client_id;
|
||||
use codex_login::request_device_code;
|
||||
use codex_login::run_login_server;
|
||||
use codex_mcp::McpRuntimeContext;
|
||||
|
||||
@@ -339,7 +339,7 @@ impl AccountRequestProcessor {
|
||||
codex_streamlined_login,
|
||||
..LoginServerOptions::new(
|
||||
config.codex_home.to_path_buf(),
|
||||
CLIENT_ID.to_string(),
|
||||
oauth_client_id(),
|
||||
config.forced_chatgpt_workspace_id.clone(),
|
||||
config.cli_auth_credentials_store_mode,
|
||||
config.auth_keyring_backend_kind(),
|
||||
|
||||
@@ -34,6 +34,7 @@ use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::CLIENT_ID_OVERRIDE_ENV_VAR;
|
||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_login::login_with_api_key;
|
||||
use codex_protocol::account::PlanType as AccountPlanType;
|
||||
@@ -1381,6 +1382,58 @@ async fn login_account_chatgpt_start_can_be_cancelled() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
// Serialize tests that launch the login server since it binds to a fixed port.
|
||||
#[serial(login_port)]
|
||||
async fn login_account_chatgpt_uses_debug_oauth_overrides() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), CreateConfigTomlParams::default())?;
|
||||
|
||||
let mut mcp = TestAppServer::new_with_env(
|
||||
codex_home.path(),
|
||||
&[
|
||||
(CLIENT_ID_OVERRIDE_ENV_VAR, Some("staging-client")),
|
||||
(LOGIN_ISSUER_ENV_VAR, Some("https://auth.example.com")),
|
||||
],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp.send_login_account_chatgpt_request().await?;
|
||||
let resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let login: LoginAccountResponse = to_response(resp)?;
|
||||
let LoginAccountResponse::Chatgpt { login_id, auth_url } = login else {
|
||||
bail!("unexpected login response: {login:?}");
|
||||
};
|
||||
let auth_url = Url::parse(&auth_url)?;
|
||||
assert_eq!(
|
||||
auth_url.origin().ascii_serialization(),
|
||||
"https://auth.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
auth_url
|
||||
.query_pairs()
|
||||
.find_map(|(key, value)| (key == "client_id").then_some(value.into_owned())),
|
||||
Some("staging-client".to_string())
|
||||
);
|
||||
|
||||
let cancel_id = mcp
|
||||
.send_cancel_login_account_request(CancelLoginAccountParams { login_id })
|
||||
.await?;
|
||||
let cancel_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(cancel_id)),
|
||||
)
|
||||
.await??;
|
||||
let _: CancelLoginAccountResponse = to_response(cancel_resp)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
// Serialize tests that launch the login server since it binds to a fixed port.
|
||||
#[serial(login_port)]
|
||||
|
||||
@@ -109,6 +109,7 @@ const REFRESH_TOKEN_URL: &str = "https://auth.openai.com/oauth/token";
|
||||
pub(super) const REVOKE_TOKEN_URL: &str = "https://auth.openai.com/oauth/revoke";
|
||||
pub const REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR: &str = "CODEX_REFRESH_TOKEN_URL_OVERRIDE";
|
||||
pub const REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR: &str = "CODEX_REVOKE_TOKEN_URL_OVERRIDE";
|
||||
pub const CLIENT_ID_OVERRIDE_ENV_VAR: &str = "CODEX_APP_SERVER_LOGIN_CLIENT_ID";
|
||||
static NEXT_DUMMY_AUTH_ID: AtomicU64 = AtomicU64::new(1);
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
@@ -1021,7 +1022,7 @@ async fn request_chatgpt_token_refresh(
|
||||
client: &CodexHttpClient,
|
||||
) -> Result<RefreshResponse, RefreshTokenError> {
|
||||
let refresh_request = RefreshRequest {
|
||||
client_id: CLIENT_ID,
|
||||
client_id: oauth_client_id(),
|
||||
grant_type: "refresh_token",
|
||||
refresh_token,
|
||||
};
|
||||
@@ -1116,7 +1117,7 @@ fn extract_refresh_token_error_code(body: &str) -> Option<String> {
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct RefreshRequest {
|
||||
client_id: &'static str,
|
||||
client_id: String,
|
||||
grant_type: &'static str,
|
||||
refresh_token: String,
|
||||
}
|
||||
@@ -1131,6 +1132,13 @@ struct RefreshResponse {
|
||||
// Shared constant for token refresh (client id used for oauth token refresh flow)
|
||||
pub const CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann";
|
||||
|
||||
pub fn oauth_client_id() -> String {
|
||||
std::env::var(CLIENT_ID_OVERRIDE_ENV_VAR)
|
||||
.ok()
|
||||
.filter(|client_id| !client_id.trim().is_empty())
|
||||
.unwrap_or_else(|| CLIENT_ID.to_string())
|
||||
}
|
||||
|
||||
fn refresh_token_endpoint() -> String {
|
||||
std::env::var(REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR)
|
||||
.unwrap_or_else(|_| REFRESH_TOKEN_URL.to_string())
|
||||
|
||||
@@ -10,10 +10,10 @@ use std::time::Duration;
|
||||
use codex_app_server_protocol::AuthMode as ApiAuthMode;
|
||||
use codex_client::CodexHttpClient;
|
||||
|
||||
use super::manager::CLIENT_ID;
|
||||
use super::manager::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use super::manager::REVOKE_TOKEN_URL;
|
||||
use super::manager::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use super::manager::oauth_client_id;
|
||||
use super::storage::AuthDotJson;
|
||||
use super::util::try_parse_error_message;
|
||||
use crate::default_client::create_client;
|
||||
@@ -35,10 +35,10 @@ impl RevokeTokenKind {
|
||||
}
|
||||
}
|
||||
|
||||
fn client_id(self) -> Option<&'static str> {
|
||||
fn client_id(self) -> Option<String> {
|
||||
match self {
|
||||
Self::Access => None,
|
||||
Self::Refresh => Some(CLIENT_ID),
|
||||
Self::Refresh => Some(oauth_client_id()),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -48,7 +48,7 @@ struct RevokeTokenRequest<'a> {
|
||||
token: &'a str,
|
||||
token_type_hint: &'static str,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
client_id: Option<&'static str>,
|
||||
client_id: Option<String>,
|
||||
}
|
||||
|
||||
pub(super) async fn revoke_auth_tokens(
|
||||
|
||||
@@ -23,6 +23,7 @@ pub use auth::AuthKeyringBackendKind;
|
||||
pub use auth::AuthManager;
|
||||
pub use auth::AuthManagerConfig;
|
||||
pub use auth::CLIENT_ID;
|
||||
pub use auth::CLIENT_ID_OVERRIDE_ENV_VAR;
|
||||
pub use auth::CODEX_ACCESS_TOKEN_ENV_VAR;
|
||||
pub use auth::CODEX_API_KEY_ENV_VAR;
|
||||
pub use auth::CodexAuth;
|
||||
@@ -45,6 +46,7 @@ pub use auth::login_with_api_key;
|
||||
pub use auth::login_with_bedrock_api_key;
|
||||
pub use auth::logout;
|
||||
pub use auth::logout_with_revoke;
|
||||
pub use auth::oauth_client_id;
|
||||
pub use auth::read_codex_access_token_from_env;
|
||||
pub use auth::read_openai_api_key_from_env;
|
||||
pub use auth::save_auth;
|
||||
|
||||
@@ -8,6 +8,7 @@ use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::AuthDotJson;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CLIENT_ID_OVERRIDE_ENV_VAR;
|
||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_login::RefreshTokenError;
|
||||
use codex_login::load_auth_dot_json;
|
||||
@@ -31,11 +32,12 @@ use wiremock::matchers::path;
|
||||
const INITIAL_ACCESS_TOKEN: &str = "initial-access-token";
|
||||
const INITIAL_REFRESH_TOKEN: &str = "initial-refresh-token";
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_succeeds_updates_storage() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let _client_id_guard = EnvGuard::set(CLIENT_ID_OVERRIDE_ENV_VAR, "staging-client".to_string());
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
@@ -66,6 +68,16 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> {
|
||||
.await
|
||||
.context("refresh should succeed")?;
|
||||
|
||||
let requests = server.received_requests().await.unwrap_or_default();
|
||||
assert_eq!(
|
||||
serde_json::from_slice::<serde_json::Value>(&requests[0].body)?,
|
||||
json!({
|
||||
"client_id": "staging-client",
|
||||
"grant_type": "refresh_token",
|
||||
"refresh_token": INITIAL_REFRESH_TOKEN,
|
||||
})
|
||||
);
|
||||
|
||||
let refreshed_tokens = TokenData {
|
||||
access_token: "new-access-token".to_string(),
|
||||
refresh_token: "new-refresh-token".to_string(),
|
||||
@@ -97,7 +109,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -163,7 +175,7 @@ async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -225,7 +237,7 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_skips_access_token_outside_refresh_window() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -263,7 +275,7 @@ async fn auth_skips_access_token_outside_refresh_window() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -324,7 +336,7 @@ async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_errors_on_account_mismatch() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -399,7 +411,7 @@ async fn refresh_token_errors_on_account_mismatch() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn returns_fresh_tokens_as_is() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -448,7 +460,7 @@ async fn returns_fresh_tokens_as_is() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refreshes_token_when_access_token_is_expired() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -510,7 +522,7 @@ async fn refreshes_token_when_access_token_is_expired() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_reloads_disk_auth_when_cached_auth_is_stale() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -568,7 +580,7 @@ async fn auth_reloads_disk_auth_when_cached_auth_is_stale() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_reloads_disk_auth_without_calling_expired_refresh_token() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -634,7 +646,7 @@ async fn auth_reloads_disk_auth_without_calling_expired_refresh_token() -> Resul
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_returns_permanent_error_for_expired_refresh_token() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -689,7 +701,7 @@ async fn refresh_token_returns_permanent_error_for_expired_refresh_token() -> Re
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_does_not_retry_after_permanent_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -758,7 +770,7 @@ async fn refresh_token_does_not_retry_after_permanent_failure() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_does_not_retry_after_bad_request_reused_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -827,7 +839,7 @@ async fn refresh_token_does_not_retry_after_bad_request_reused_failure() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -915,7 +927,7 @@ async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result<
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn refresh_token_returns_transient_error_on_server_failure() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -969,7 +981,7 @@ async fn refresh_token_returns_transient_error_on_server_failure() -> Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -1068,7 +1080,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn unauthorized_recovery_errors_on_account_mismatch() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -1154,7 +1166,7 @@ async fn unauthorized_recovery_errors_on_account_mismatch() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_refresh)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn unauthorized_recovery_requires_chatgpt_auth() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -7,6 +7,7 @@ use codex_login::AuthDotJson;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CLIENT_ID;
|
||||
use codex_login::CLIENT_ID_OVERRIDE_ENV_VAR;
|
||||
use codex_login::CODEX_ACCESS_TOKEN_ENV_VAR;
|
||||
use codex_login::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_login::logout_with_revoke;
|
||||
@@ -28,11 +29,12 @@ use wiremock::matchers::path;
|
||||
const ACCESS_TOKEN: &str = "access-token";
|
||||
const REFRESH_TOKEN: &str = "refresh-token";
|
||||
|
||||
#[serial_test::serial(logout_revoke)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn logout_with_revoke_revokes_refresh_token_then_removes_auth() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let _client_id_guard = EnvGuard::set(CLIENT_ID_OVERRIDE_ENV_VAR, "staging-client".to_string());
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/revoke"))
|
||||
@@ -77,14 +79,14 @@ async fn logout_with_revoke_revokes_refresh_token_then_removes_auth() -> Result<
|
||||
json!({
|
||||
"token": REFRESH_TOKEN,
|
||||
"token_type_hint": "refresh_token",
|
||||
"client_id": CLIENT_ID,
|
||||
"client_id": "staging-client",
|
||||
})
|
||||
);
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(logout_revoke)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn logout_with_revoke_uses_stored_auth_when_access_token_env_is_set() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -126,7 +128,7 @@ async fn logout_with_revoke_uses_stored_auth_when_access_token_env_is_set() -> R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(logout_revoke)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn logout_with_revoke_removes_auth_when_revoke_fails() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -169,7 +171,7 @@ async fn logout_with_revoke_removes_auth_when_revoke_fails() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(logout_revoke)]
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_manager_logout_with_revoke_uses_cached_auth() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user