Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions codex-rs/login/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use std::path::PathBuf;
use crate::AuthDotJson;
use crate::AuthKeyringBackendKind;
use crate::account_lease::AccountLease;
use crate::account_lease::AuthRefreshGuard;
use crate::auth::load_auth_dot_json_with_guard;
use crate::auth::save_auth_with_guard;
use crate::load_auth_dot_json;
use crate::save_auth;

Expand Down Expand Up @@ -104,17 +107,19 @@ impl AccountStore {
root_store_mode: AuthCredentialsStoreMode,
root_keyring_backend_kind: AuthKeyringBackendKind,
) -> std::io::Result<AccountProfile> {
let _root_refresh_guard = AccountLease::acquire_auth_refresh(&self.codex_home)?;
let root_auth =
load_auth_dot_json(&self.codex_home, root_store_mode, root_keyring_backend_kind)?
.ok_or_else(|| {
std::io::Error::new(std::io::ErrorKind::NotFound, "not logged in")
})?;
let root_refresh_guard = AuthRefreshGuard::acquire(&self.codex_home)?;
let root_auth = load_auth_dot_json_with_guard(
&self.codex_home,
root_store_mode,
root_keyring_backend_kind,
&root_refresh_guard,
)?
.ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "not logged in"))?;
let mut auth = root_auth.clone();
let imported_from_root_marker = is_root_account_marker(&auth);
let source_account_id = account_id_for_auth(&auth)?;
let account_home = self.account_home(&source_account_id);
let _account_refresh_guard = AccountLease::acquire_auth_refresh(&account_home)?;
let _account_refresh_guard = AuthRefreshGuard::acquire(&account_home)?;
if imported_from_root_marker {
auth = load_auth_dot_json(
&account_home,
Expand Down Expand Up @@ -209,6 +214,7 @@ impl AccountStore {
&auth,
root_store_mode,
root_keyring_backend_kind,
&root_refresh_guard,
) {
let mut rollback_errors = Vec::new();
if let Err(rollback_err) = self.save_index(&previous_index) {
Expand All @@ -219,11 +225,12 @@ impl AccountStore {
{
rollback_errors.push(format!("restore account auth: {rollback_err}"));
}
if let Err(rollback_err) = save_auth(
if let Err(rollback_err) = save_auth_with_guard(
&self.codex_home,
&root_auth,
root_store_mode,
root_keyring_backend_kind,
&root_refresh_guard,
) {
rollback_errors.push(format!("restore root auth: {rollback_err}"));
}
Expand Down Expand Up @@ -299,7 +306,7 @@ impl AccountStore {
expected_auth: &AuthDotJson,
) -> std::io::Result<bool> {
let account_home = self.account_home(account_id);
let _refresh_guard = AccountLease::acquire_auth_refresh(&account_home)?;
let _refresh_guard = AuthRefreshGuard::acquire(&account_home)?;
let current_auth = load_auth_dot_json(
&account_home,
AuthCredentialsStoreMode::File,
Expand Down Expand Up @@ -337,8 +344,8 @@ impl AccountStore {
root_keyring_backend_kind: AuthKeyringBackendKind,
) -> std::io::Result<AccountProfile> {
let account_home = self.account_home(account_id);
let _root_refresh_guard = AccountLease::acquire_auth_refresh(&self.codex_home)?;
let _account_refresh_guard = AccountLease::acquire_auth_refresh(&account_home)?;
let root_refresh_guard = AuthRefreshGuard::acquire(&self.codex_home)?;
let _account_refresh_guard = AuthRefreshGuard::acquire(&account_home)?;
let _index_guard = self.acquire_index_lock()?;
let (profile, account_home) = self
.file_account_profiles()?
Expand Down Expand Up @@ -367,6 +374,7 @@ impl AccountStore {
&auth,
root_store_mode,
root_keyring_backend_kind,
&root_refresh_guard,
)?;
Ok(profile)
}
Expand Down Expand Up @@ -641,12 +649,13 @@ fn save_root_account_marker(
auth: &AuthDotJson,
store_mode: AuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
guard: &AuthRefreshGuard,
) -> std::io::Result<()> {
let mut marker = auth.clone();
if let Some(tokens) = marker.tokens.as_mut() {
tokens.refresh_token.clear();
}
save_auth(codex_home, &marker, store_mode, keyring_backend_kind)
save_auth_with_guard(codex_home, &marker, store_mode, keyring_backend_kind, guard)
}

fn restore_file_auth(auth_home: &Path, auth: Option<&AuthDotJson>) -> std::io::Result<()> {
Expand Down
41 changes: 37 additions & 4 deletions codex-rs/login/src/account_lease.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ use std::fs::File;
use std::fs::OpenOptions;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;

pub(crate) struct AccountLease {
file: File,
}

impl AccountLease {
pub(crate) fn acquire_auth_refresh(auth_home: &Path) -> io::Result<Self> {
Self::acquire(&auth_home.join(".auth-refresh.lock"))
}

pub(crate) fn acquire(path: &Path) -> io::Result<Self> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
Expand Down Expand Up @@ -50,6 +48,41 @@ impl AccountLease {
}
}

#[derive(Clone)]
pub(crate) struct AuthRefreshGuard {
auth_home: PathBuf,
_lease: Arc<AccountLease>,
}

impl AuthRefreshGuard {
pub(crate) fn acquire(auth_home: &Path) -> io::Result<Self> {
let lease = AccountLease::acquire(&auth_home.join(".auth-refresh.lock"))?;
Ok(Self::new(auth_home, lease))
}

pub(crate) fn ensure_matches(&self, auth_home: &Path) -> io::Result<()> {
if self.auth_home == normalized(auth_home) {
Ok(())
} else {
Err(io::Error::new(
io::ErrorKind::PermissionDenied,
"auth refresh guard does not match auth home",
))
}
}

fn new(auth_home: &Path, lease: AccountLease) -> Self {
Self {
auth_home: normalized(auth_home),
_lease: Arc::new(lease),
}
}
}

fn normalized(path: &Path) -> PathBuf {
path.canonicalize().unwrap_or_else(|_| path.to_path_buf())
}

fn is_contended(err: &io::Error) -> bool {
err.kind() == io::ErrorKind::WouldBlock
|| cfg!(windows) && matches!(err.raw_os_error(), Some(32 | 33))
Expand Down
82 changes: 70 additions & 12 deletions codex-rs/login/src/auth/auth_tests.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use super::*;
use crate::auth::storage::FileAuthStorage;
use crate::auth::storage::create_auth_storage_with_store;
use crate::auth::storage::get_auth_file;
use crate::token_data::IdTokenInfo;
use anyhow::Context;
use codex_protocol::account::PlanType as AccountPlanType;
use codex_protocol::auth::AuthMode;
use codex_protocol::auth::KnownPlan as InternalKnownPlan;
use codex_protocol::auth::PlanType as InternalPlanType;
use codex_protocol::protocol::SessionSource;

use base64::Engine;
use codex_keyring_store::tests::MockKeyringStore;
use codex_protocol::config_types::ForcedLoginMethod;
use codex_protocol::config_types::ModelProviderAuthInfo;
use pretty_assertions::assert_eq;
Expand All @@ -32,7 +35,8 @@ const WORKSPACE_ID_SECOND_ALLOWED: &str = "123e4567-e89b-42d3-a456-426614174001"
const WORKSPACE_ID_DISALLOWED: &str = "123e4567-e89b-42d3-a456-426614174002";

#[tokio::test]
async fn refresh_without_id_token() {
#[serial(codex_auth_env)]
async fn refresh_without_id_token() -> anyhow::Result<()> {
let codex_home = tempdir().unwrap();
let fake_jwt = write_auth_file(
AuthFileParams {
Expand All @@ -43,24 +47,75 @@ async fn refresh_without_id_token() {
codex_home.path(),
)
.expect("failed to write auth file");

let storage = create_auth_storage(
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/oauth/token"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"access_token": "new-access-token",
"refresh_token": "new-refresh-token"
})))
.expect(1)
.mount(&server)
.await;
let _refresh_url = EnvVarGuard::set(
REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR,
&format!("{}/oauth/token", server.uri()),
);
let file_storage = FileAuthStorage::new(codex_home.path().to_path_buf());
let mut initial_auth = file_storage.load()?.expect("auth file should exist");
initial_auth
.tokens
.as_mut()
.context("auth tokens should exist")?
.account_id = Some("account-123".to_string());
file_storage.save(&initial_auth)?;
const MARKER: &str = ".codex-plus-plus-auth-file-authority";
let marker = codex_home.path().join(MARKER);
std::fs::write(&marker, "")?;
let mock_keyring = MockKeyringStore::default();
let storage = create_auth_storage_with_store(
codex_home.path().to_path_buf(),
AuthCredentialsStoreMode::File,
AuthKeyringBackendKind::default(),
AuthCredentialsStoreMode::Auto,
Arc::new(mock_keyring),
AuthKeyringBackendKind::Secrets,
);
let updated = super::persist_tokens(
&storage,
/*id_token*/ None,
Some("new-access-token".to_string()),
Some("new-refresh-token".to_string()),
)
.expect("update_tokens should succeed");
let state = ChatgptAuthState {
auth_dot_json: Arc::new(Mutex::new(Some(initial_auth))),
client: create_default_auth_client(
&refresh_token_endpoint(),
/*auth_route_config*/ None,
)?,
};
let auth = CodexAuth::Chatgpt(ChatgptAuth {
state,
storage: Arc::clone(&storage),
});
let manager =
AuthManager::from_auth_for_testing_with_home(auth, codex_home.path().to_path_buf());
assert!(matches!(
manager
.reload_if_account_id_matches(Some("account-123"), /*guard*/ None)
.await,
ReloadOutcome::ReloadedNoChange
));
let cached = manager.auth_cached().context("auth should remain cached")?;
let CodexAuth::Chatgpt(cached_chatgpt) = cached else {
anyhow::bail!("cached auth should remain ChatGPT auth");
};
assert!(Arc::ptr_eq(cached_chatgpt.storage(), &storage));

manager.refresh_token().await?;

let updated = file_storage.load()?.expect("authoritative auth file");
let tokens = updated.tokens.expect("tokens should exist");
assert_eq!(tokens.id_token.raw_jwt, fake_jwt);
assert_eq!(tokens.access_token, "new-access-token");
assert_eq!(tokens.refresh_token, "new-refresh-token");
assert!(marker.exists());
let cached = manager.auth_cached().context("auth should remain cached")?;
assert_eq!(cached.get_token_data()?, tokens);
server.verify().await;
Ok(())
}

#[test]
Expand Down Expand Up @@ -1066,6 +1121,9 @@ async fn external_bearer_only_auth_manager_disables_auto_refresh_when_interval_i

assert_eq!(first.as_deref(), Some("provider-token"));
assert_eq!(second.as_deref(), Some("provider-token"));
manager.refresh_token().await.expect("external refresh");
let refreshed = manager.auth().await.expect("refreshed auth");
assert_eq!(refreshed.api_key(), Some("next-token"));
}

#[tokio::test]
Expand Down
Loading