When logging in using ChatGPT, make sure to overwrite API key (#3611)

When logging in using ChatGPT using the `codex login` command, a
successful login should write a new `auth.json` file with the ChatGPT
token information. The old code attempted to retain the API key and
merge the token information into the existing `auth.json` file. With the
new simplified login mechanism, `auth.json` should have auth information
for only ChatGPT or API Key, not both.

The `codex login --api-key <key>` code path was already doing the right
thing here, but the `codex login` command was incorrect. This PR fixes
the problem and adds test cases for both commands.

codex-rs/core/src/auth.rs

@@ -408,6 +408,32 @@ mod tests {
         assert_eq!(auth_dot_json, same_auth_dot_json);
     }
 
+    #[test]
+    fn login_with_api_key_overwrites_existing_auth_json() {
+        let dir = tempdir().unwrap();
+        let auth_path = dir.path().join("auth.json");
+        let stale_auth = json!({
+            "OPENAI_API_KEY": "sk-old",
+            "tokens": {
+                "id_token": "stale.header.payload",
+                "access_token": "stale-access",
+                "refresh_token": "stale-refresh",
+                "account_id": "stale-acc"
+            }
+        });
+        std::fs::write(
+            &auth_path,
+            serde_json::to_string_pretty(&stale_auth).unwrap(),
+        )
+        .unwrap();
+
+        super::login_with_api_key(dir.path(), "sk-new").expect("login_with_api_key should succeed");
+
+        let auth = super::try_read_auth_json(&auth_path).expect("auth.json should parse");
+        assert_eq!(auth.openai_api_key.as_deref(), Some("sk-new"));
+        assert!(auth.tokens.is_none(), "tokens should be cleared");
+    }
+
     #[tokio::test]
     async fn pro_account_with_no_api_key_uses_chatgpt_auth() {
         let codex_home = tempdir().unwrap();

codex-rs/login/src/server.rs

@@ -238,8 +238,8 @@ async fn process_request(
                         &opts.codex_home,
                         api_key.clone(),
                         tokens.id_token.clone(),
-                        Some(tokens.access_token.clone()),
-                        Some(tokens.refresh_token.clone()),
+                        tokens.access_token.clone(),
+                        tokens.refresh_token.clone(),
                     )
                     .await
                     {
@@ -446,8 +446,8 @@ async fn persist_tokens_async(
     codex_home: &Path,
     api_key: Option<String>,
     id_token: String,
-    access_token: Option<String>,
-    refresh_token: Option<String>,
+    access_token: String,
+    refresh_token: String,
 ) -> io::Result<()> {
     // Reuse existing synchronous logic but run it off the async runtime.
     let codex_home = codex_home.to_path_buf();
@@ -459,43 +459,29 @@ async fn persist_tokens_async(
             std::fs::create_dir_all(parent).map_err(io::Error::other)?;
         }
 
-        let mut auth = read_or_default(&auth_file);
-        if let Some(key) = api_key {
-            auth.openai_api_key = Some(key);
-        }
-        let tokens = auth.tokens.get_or_insert_with(TokenData::default);
-        tokens.id_token = parse_id_token(&id_token).map_err(io::Error::other)?;
-        // Persist chatgpt_account_id if present in claims
+        let mut tokens = TokenData {
+            id_token: parse_id_token(&id_token).map_err(io::Error::other)?,
+            access_token,
+            refresh_token,
+            account_id: None,
+        };
         if let Some(acc) = jwt_auth_claims(&id_token)
             .get("chatgpt_account_id")
             .and_then(|v| v.as_str())
         {
             tokens.account_id = Some(acc.to_string());
         }
-        if let Some(at) = access_token {
-            tokens.access_token = at;
-        }
-        if let Some(rt) = refresh_token {
-            tokens.refresh_token = rt;
-        }
-        auth.last_refresh = Some(Utc::now());
+        let auth = AuthDotJson {
+            openai_api_key: api_key,
+            tokens: Some(tokens),
+            last_refresh: Some(Utc::now()),
+        };
         codex_core::auth::write_auth_json(&auth_file, &auth)
     })
     .await
     .map_err(|e| io::Error::other(format!("persist task failed: {e}")))?
 }
 
-fn read_or_default(path: &Path) -> AuthDotJson {
-    match codex_core::auth::try_read_auth_json(path) {
-        Ok(auth) => auth,
-        Err(_) => AuthDotJson {
-            openai_api_key: None,
-            tokens: None,
-            last_refresh: None,
-        },
-    }
-}
-
 fn compose_success_url(port: u16, issuer: &str, id_token: &str, access_token: &str) -> String {
     let token_claims = jwt_auth_claims(id_token);
     let access_claims = jwt_auth_claims(access_token);

codex-rs/login/tests/suite/login_server_e2e.rs

@@ -90,6 +90,22 @@ async fn end_to_end_login_flow_persists_auth_json() {
     let tmp = tempdir().unwrap();
     let codex_home = tmp.path().to_path_buf();
 
+    // Seed auth.json with stale API key + tokens that should be overwritten.
+    let stale_auth = serde_json::json!({
+        "OPENAI_API_KEY": "sk-stale",
+        "tokens": {
+            "id_token": "stale.header.payload",
+            "access_token": "stale-access",
+            "refresh_token": "stale-refresh",
+            "account_id": "stale-acc"
+        }
+    });
+    std::fs::write(
+        codex_home.join("auth.json"),
+        serde_json::to_string_pretty(&stale_auth).unwrap(),
+    )
+    .unwrap();
+
     let state = "test_state_123".to_string();
 
     // Run server in background
@@ -122,10 +138,10 @@ async fn end_to_end_login_flow_persists_auth_json() {
     let auth_path = codex_home.join("auth.json");
     let data = std::fs::read_to_string(&auth_path).unwrap();
     let json: serde_json::Value = serde_json::from_str(&data).unwrap();
-    assert!(
-        !json["OPENAI_API_KEY"].is_null(),
-        "OPENAI_API_KEY should be set"
-    );
+    // The following assert is here because of the old oauth flow that exchanges tokens for an
+    // API key. See obtain_api_key in server.rs for details. Once we remove this old mechanism
+    // from the code, this test should be updated to expect that the API key is no longer present.
+    assert_eq!(json["OPENAI_API_KEY"], "access-123");
     assert_eq!(json["tokens"]["access_token"], "access-123");
     assert_eq!(json["tokens"]["refresh_token"], "refresh-123");
     assert_eq!(json["tokens"]["account_id"], "acc-123");