mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Report the reason for ChatGPT token refresh requests (#53991)
## What changed Include a `refresh_reason` diagnostic in ChatGPT OAuth token refresh requests to distinguish `proactive_expiry`, `scheduled`, `unauthorized_recovery`, and `forced` refreshes. Omit the field for gateway refresh requests. ## Testing Extend refresh tests to check the reported reasons, including scheduled refreshes when access token expiry is unavailable. GitOrigin-RevId: b0294a61ffc3089a66e7b132d4f45661a9e18d38
This commit is contained in:
committed by
copyberry
parent
0b73618146
commit
f87ba04ec3
@@ -62,6 +62,7 @@ use crate::default_client::create_default_auth_client;
|
||||
use crate::oauth::ErrorBodyLimit;
|
||||
use crate::oauth::OAuthClient;
|
||||
use crate::oauth::OAuthError;
|
||||
use crate::oauth::RefreshReason;
|
||||
use crate::oauth::RefreshTokenGrant;
|
||||
use crate::oauth::TokenEncoding;
|
||||
use crate::oauth::TokenEndpoint;
|
||||
@@ -1627,6 +1628,7 @@ fn persist_tokens(
|
||||
async fn request_chatgpt_token_refresh(
|
||||
refresh_token: String,
|
||||
client: &HttpClient,
|
||||
refresh_reason: RefreshReason,
|
||||
) -> Result<RefreshResponse, RefreshTokenError> {
|
||||
let client_id = oauth_client_id();
|
||||
let endpoint = refresh_token_endpoint();
|
||||
@@ -1644,6 +1646,7 @@ async fn request_chatgpt_token_refresh(
|
||||
.refresh(RefreshTokenGrant {
|
||||
refresh_token: &refresh_token,
|
||||
resource: None,
|
||||
refresh_reason: Some(refresh_reason),
|
||||
})
|
||||
.await
|
||||
{
|
||||
@@ -2011,14 +2014,18 @@ impl UnauthorizedRecovery {
|
||||
}
|
||||
}
|
||||
UnauthorizedRecoveryStep::RefreshToken => {
|
||||
self.manager.refresh_token_from_authority().await?;
|
||||
self.manager
|
||||
.refresh_token_from_authority_with_reason(RefreshReason::UnauthorizedRecovery)
|
||||
.await?;
|
||||
self.step = UnauthorizedRecoveryStep::Done;
|
||||
return Ok(UnauthorizedRecoveryStepResult {
|
||||
auth_state_changed: Some(true),
|
||||
});
|
||||
}
|
||||
UnauthorizedRecoveryStep::ExternalRefresh => {
|
||||
self.manager.refresh_token_from_authority().await?;
|
||||
self.manager
|
||||
.refresh_token_from_authority_with_reason(RefreshReason::UnauthorizedRecovery)
|
||||
.await?;
|
||||
self.step = UnauthorizedRecoveryStep::Done;
|
||||
return Ok(UnauthorizedRecoveryStepResult {
|
||||
auth_state_changed: Some(true),
|
||||
@@ -2395,8 +2402,8 @@ impl AuthManager {
|
||||
}
|
||||
|
||||
let auth = self.auth_cached()?;
|
||||
if Self::should_refresh_proactively(&auth)
|
||||
&& let Err(err) = self.refresh_token().await
|
||||
if let Some(reason) = Self::proactive_refresh_reason(&auth)
|
||||
&& let Err(err) = self.refresh_token_with_reason(reason).await
|
||||
{
|
||||
tracing::error!("Failed to refresh token: {}", err);
|
||||
return Some(auth);
|
||||
@@ -2843,6 +2850,13 @@ impl AuthManager {
|
||||
/// we can assume that the source already refreshed it. Otherwise, ask the
|
||||
/// token authority to refresh.
|
||||
pub async fn refresh_token(&self) -> Result<(), RefreshTokenError> {
|
||||
self.refresh_token_with_reason(RefreshReason::Forced).await
|
||||
}
|
||||
|
||||
async fn refresh_token_with_reason(
|
||||
&self,
|
||||
reason: RefreshReason,
|
||||
) -> Result<(), RefreshTokenError> {
|
||||
let _refresh_guard = self.refresh_lock.acquire().await.map_err(|_| {
|
||||
RefreshTokenError::Permanent(RefreshTokenFailedError::new(
|
||||
RefreshTokenFailedReason::Other,
|
||||
@@ -2868,7 +2882,7 @@ impl AuthManager {
|
||||
tracing::info!("Skipping token refresh because auth changed after guarded reload.");
|
||||
Ok(())
|
||||
}
|
||||
ReloadOutcome::ReloadedNoChange => self.refresh_token_from_authority_impl().await,
|
||||
ReloadOutcome::ReloadedNoChange => self.refresh_token_from_authority_impl(reason).await,
|
||||
ReloadOutcome::Skipped => {
|
||||
Err(RefreshTokenError::Permanent(RefreshTokenFailedError::new(
|
||||
RefreshTokenFailedReason::Other,
|
||||
@@ -2882,16 +2896,27 @@ impl AuthManager {
|
||||
/// it and update the shared cache. If the token refresh fails, returns the
|
||||
/// error to the caller.
|
||||
pub async fn refresh_token_from_authority(&self) -> Result<(), RefreshTokenError> {
|
||||
self.refresh_token_from_authority_with_reason(RefreshReason::Forced)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn refresh_token_from_authority_with_reason(
|
||||
&self,
|
||||
reason: RefreshReason,
|
||||
) -> Result<(), RefreshTokenError> {
|
||||
let _refresh_guard = self.refresh_lock.acquire().await.map_err(|_| {
|
||||
RefreshTokenError::Permanent(RefreshTokenFailedError::new(
|
||||
RefreshTokenFailedReason::Other,
|
||||
REFRESH_TOKEN_UNKNOWN_MESSAGE.to_string(),
|
||||
))
|
||||
})?;
|
||||
self.refresh_token_from_authority_impl().await
|
||||
self.refresh_token_from_authority_impl(reason).await
|
||||
}
|
||||
|
||||
async fn refresh_token_from_authority_impl(&self) -> Result<(), RefreshTokenError> {
|
||||
async fn refresh_token_from_authority_impl(
|
||||
&self,
|
||||
reason: RefreshReason,
|
||||
) -> Result<(), RefreshTokenError> {
|
||||
tracing::info!("Refreshing token");
|
||||
|
||||
let attempted_auth = self.auth_cached();
|
||||
@@ -2913,8 +2938,12 @@ impl AuthManager {
|
||||
"Token data is not available.",
|
||||
))
|
||||
})?;
|
||||
self.refresh_and_persist_chatgpt_token(chatgpt_auth, token_data.refresh_token)
|
||||
.await
|
||||
self.refresh_and_persist_chatgpt_token(
|
||||
chatgpt_auth,
|
||||
token_data.refresh_token,
|
||||
reason,
|
||||
)
|
||||
.await
|
||||
}
|
||||
Some(
|
||||
CodexAuth::ApiKey(_)
|
||||
@@ -2998,28 +3027,24 @@ impl AuthManager {
|
||||
.is_some_and(AuthMode::uses_codex_backend)
|
||||
}
|
||||
|
||||
fn should_refresh_proactively(auth: &CodexAuth) -> bool {
|
||||
fn proactive_refresh_reason(auth: &CodexAuth) -> Option<RefreshReason> {
|
||||
let chatgpt_auth = match auth {
|
||||
CodexAuth::Chatgpt(chatgpt_auth) => chatgpt_auth,
|
||||
_ => return false,
|
||||
_ => return None,
|
||||
};
|
||||
|
||||
let auth_dot_json = match chatgpt_auth.current_auth_json() {
|
||||
Some(auth_dot_json) => auth_dot_json,
|
||||
None => return false,
|
||||
};
|
||||
let auth_dot_json = chatgpt_auth.current_auth_json()?;
|
||||
if let Some(tokens) = auth_dot_json.tokens.as_ref()
|
||||
&& let Ok(Some(expires_at)) = parse_jwt_expiration(&tokens.access_token)
|
||||
{
|
||||
return expires_at
|
||||
return (expires_at
|
||||
<= Utc::now()
|
||||
+ chrono::Duration::minutes(CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES);
|
||||
+ chrono::Duration::minutes(CHATGPT_ACCESS_TOKEN_REFRESH_WINDOW_MINUTES))
|
||||
.then_some(RefreshReason::ProactiveExpiry);
|
||||
}
|
||||
let last_refresh = match auth_dot_json.last_refresh {
|
||||
Some(last_refresh) => last_refresh,
|
||||
None => return false,
|
||||
};
|
||||
last_refresh < Utc::now() - chrono::Duration::days(TOKEN_REFRESH_INTERVAL)
|
||||
let last_refresh = auth_dot_json.last_refresh?;
|
||||
(last_refresh < Utc::now() - chrono::Duration::days(TOKEN_REFRESH_INTERVAL))
|
||||
.then_some(RefreshReason::Scheduled)
|
||||
}
|
||||
|
||||
async fn refresh_external_auth(
|
||||
@@ -3090,8 +3115,10 @@ impl AuthManager {
|
||||
&self,
|
||||
auth: &ChatgptAuth,
|
||||
refresh_token: String,
|
||||
reason: RefreshReason,
|
||||
) -> Result<(), RefreshTokenError> {
|
||||
let refresh_response = request_chatgpt_token_refresh(refresh_token, auth.client()).await?;
|
||||
let refresh_response =
|
||||
request_chatgpt_token_refresh(refresh_token, auth.client(), reason).await?;
|
||||
|
||||
persist_tokens(
|
||||
auth.storage(),
|
||||
|
||||
@@ -321,6 +321,7 @@ impl GatewayAuthManager {
|
||||
.refresh::<TokenResponse>(RefreshTokenGrant {
|
||||
refresh_token,
|
||||
resource: self.state.config.resource.as_deref(),
|
||||
refresh_reason: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -39,6 +39,27 @@ pub(crate) struct AuthorizationCodeGrant<'a> {
|
||||
pub(crate) struct RefreshTokenGrant<'a> {
|
||||
pub refresh_token: &'a str,
|
||||
pub resource: Option<&'a str>,
|
||||
pub refresh_reason: Option<RefreshReason>,
|
||||
}
|
||||
|
||||
/// Caller-reported diagnostics for AuthAPI; never evidence of token validity.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub(crate) enum RefreshReason {
|
||||
ProactiveExpiry,
|
||||
Scheduled,
|
||||
UnauthorizedRecovery,
|
||||
Forced,
|
||||
}
|
||||
|
||||
impl RefreshReason {
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::ProactiveExpiry => "proactive_expiry",
|
||||
Self::Scheduled => "scheduled",
|
||||
Self::UnauthorizedRecovery => "unauthorized_recovery",
|
||||
Self::Forced => "forced",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared OAuth protocol implementation; callers retain tokens, storage, and recovery decisions.
|
||||
@@ -85,6 +106,9 @@ impl<'a> OAuthClient<'a> {
|
||||
if let Some(resource) = grant.resource {
|
||||
parameters.push(("resource", resource));
|
||||
}
|
||||
if let Some(reason) = grant.refresh_reason {
|
||||
parameters.push(("refresh_reason", reason.as_str()));
|
||||
}
|
||||
self.exchange(¶meters, &[grant.refresh_token]).await
|
||||
}
|
||||
|
||||
|
||||
@@ -184,6 +184,7 @@ async fn rejection_preserves_status_and_omits_oversized_diagnostics() {
|
||||
.refresh::<Value>(RefreshTokenGrant {
|
||||
refresh_token,
|
||||
resource: Some("urn:gateway"),
|
||||
refresh_reason: None,
|
||||
})
|
||||
.await;
|
||||
let Err(OAuthError::Rejected(error)) = result else {
|
||||
@@ -227,6 +228,7 @@ async fn rejection_keeps_status_when_reading_the_body_fails() {
|
||||
.refresh::<Value>(RefreshTokenGrant {
|
||||
refresh_token: "refresh",
|
||||
resource: None,
|
||||
refresh_reason: None,
|
||||
})
|
||||
.await;
|
||||
let Err(OAuthError::Rejected(error)) = result else {
|
||||
@@ -287,7 +289,8 @@ async fn success_body_revocation_preserves_the_policy_error() {
|
||||
tokio::join!(
|
||||
form.refresh::<Value>(RefreshTokenGrant {
|
||||
refresh_token: "refresh",
|
||||
resource: None
|
||||
resource: None,
|
||||
refresh_reason: None,
|
||||
}),
|
||||
server
|
||||
)
|
||||
|
||||
@@ -16,6 +16,7 @@ pub(crate) use authorization::build_authorization_url;
|
||||
pub(crate) use authorization::generate_state;
|
||||
pub(crate) use client::AuthorizationCodeGrant;
|
||||
pub(crate) use client::OAuthClient;
|
||||
pub(crate) use client::RefreshReason;
|
||||
pub(crate) use client::RefreshTokenGrant;
|
||||
pub(crate) use client::TokenEncoding;
|
||||
pub(crate) use client::TokenEndpoint;
|
||||
|
||||
@@ -32,6 +32,7 @@ use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_partial_json;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
@@ -119,6 +120,7 @@ async fn refresh_token_honors_respect_system_proxy() -> Result<()> {
|
||||
"client_id": "staging-client",
|
||||
"grant_type": "refresh_token",
|
||||
"refresh_token": INITIAL_REFRESH_TOKEN,
|
||||
"refresh_reason": "forced",
|
||||
})
|
||||
);
|
||||
return Ok(());
|
||||
@@ -224,6 +226,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> {
|
||||
"client_id": "staging-client",
|
||||
"grant_type": "refresh_token",
|
||||
"refresh_token": INITIAL_REFRESH_TOKEN,
|
||||
"refresh_reason": "forced",
|
||||
})
|
||||
);
|
||||
|
||||
@@ -333,6 +336,9 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.and(body_partial_json(
|
||||
json!({"refresh_reason": "proactive_expiry"}),
|
||||
))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"access_token": "new-access-token",
|
||||
"refresh_token": "new-refresh-token"
|
||||
@@ -388,6 +394,54 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_reports_scheduled_refresh_when_access_token_expiry_is_unavailable() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.and(body_partial_json(json!({"refresh_reason": "scheduled"})))
|
||||
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(json!({
|
||||
"access_token": "new-access-token",
|
||||
"refresh_token": "new-refresh-token"
|
||||
})))
|
||||
.expect(/*r*/ 1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let ctx = RefreshTokenTestContext::new(&server).await?;
|
||||
let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN);
|
||||
ctx.write_auth(&AuthDotJson {
|
||||
auth_mode: Some(AuthMode::Chatgpt),
|
||||
openai_api_key: None,
|
||||
tokens: Some(initial_tokens.clone()),
|
||||
last_refresh: Some(Utc::now() - Duration::days(9)),
|
||||
agent_identity: None,
|
||||
personal_access_token: None,
|
||||
bedrock_api_key: None,
|
||||
bedrock_access_keys: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let refreshed = ctx
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.context("auth should refresh")?;
|
||||
assert_eq!(
|
||||
refreshed.get_token_data()?,
|
||||
TokenData {
|
||||
access_token: "new-access-token".to_string(),
|
||||
refresh_token: "new-refresh-token".to_string(),
|
||||
..initial_tokens
|
||||
}
|
||||
);
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[serial_test::serial(auth_env)]
|
||||
#[tokio::test]
|
||||
async fn auth_skips_access_token_outside_refresh_window() -> Result<()> {
|
||||
@@ -1294,6 +1348,9 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.and(body_partial_json(
|
||||
json!({"refresh_reason": "unauthorized_recovery"}),
|
||||
))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"access_token": "recovered-access-token",
|
||||
"refresh_token": "recovered-refresh-token"
|
||||
|
||||
Reference in New Issue
Block a user