From 4aeaccf125f99d5044e2bd3ca2ed5f32b323b040 Mon Sep 17 00:00:00 2001 From: Jason Date: Fri, 9 Oct 2026 13:39:39 +0800 Subject: [PATCH] fix(codex): unbind providers when a managed account is removed Deleting a managed ChatGPT account (or logging out of all of them) left every Codex provider bound to that account pointing at a dead local ID. #7395 made such cards recoverable by hand (switch away, pick another account); this removes the dangling bindings where they are created. - Remove/logout detach the affected Codex providers in one SQLite transaction: drop the binding, store an empty auth like a manual unbind, backfill category=official for legacy cards, keep unknown meta keys. The card then follows the native Codex login. - Only this device's account IDs are unbound. Provider rows sync across devices, so a row bound to another device's account stays bound. - The command reports "account removed but unbinding failed" separately. Any row still left dangling (failed unbinding, older versions, synced rows) keeps going through the #7395 recovery path, whose tests now create such rows directly. - Frontend invalidates the Codex provider list after remove/logout, including on partial failure. Thanks @SaladDay for the report and the first fix in #7060. Fixes #7055 Refs #6969 --- src-tauri/src/commands/auth.rs | 26 +- src-tauri/src/database/dao/providers.rs | 86 ++++++ src-tauri/src/services/provider/mod.rs | 249 ++++++++++++++++-- .../providers/forms/hooks/useManagedAuth.ts | 18 +- tests/hooks/useManagedAuth.test.tsx | 67 ++++- 5 files changed, 422 insertions(+), 24 deletions(-) diff --git a/src-tauri/src/commands/auth.rs b/src-tauri/src/commands/auth.rs index 540a990d0..77db0554e 100644 --- a/src-tauri/src/commands/auth.rs +++ b/src-tauri/src/commands/auth.rs @@ -384,7 +384,14 @@ pub(crate) async fn remove_codex_oauth_account_with_switch_lock( .codex_oauth_manager .remove_account(account_id) .await - .map_err(|error| error.to_string()) + .map_err(|error| error.to_string())?; + app_state + .db + .unbind_codex_managed_accounts(&[account_id.to_string()]) + .map_err(|error| { + format!("账号已删除，但 Codex 供应商解绑失败；请编辑供应商重新选择账号或切换供应商: {error}") + })?; + Ok(()) } #[tauri::command(rename_all = "camelCase")] @@ -451,9 +458,24 @@ pub(crate) async fn logout_codex_oauth_with_switch_lock( .proxy_service .lock_switch_for_app(AppType::Codex.as_str()) .await; + // Only this device's accounts: rows synced from another device stay bound there. + let removed: Vec = app_state + .codex_oauth_manager + .list_accounts() + .await + .into_iter() + .map(|account| account.id) + .collect(); app_state .codex_oauth_manager .clear_auth() .await - .map_err(|error| error.to_string()) + .map_err(|error| error.to_string())?; + app_state + .db + .unbind_codex_managed_accounts(&removed) + .map_err(|error| { + format!("托管账号已登出，但 Codex 供应商解绑失败；请编辑供应商重新选择账号或切换供应商: {error}") + })?; + Ok(()) } diff --git a/src-tauri/src/database/dao/providers.rs b/src-tauri/src/database/dao/providers.rs index cbfa5d067..7e04175c4 100644 --- a/src-tauri/src/database/dao/providers.rs +++ b/src-tauri/src/database/dao/providers.rs @@ -17,6 +17,92 @@ type OmoProviderRow = ( ); impl Database { + /// Detach Codex providers bound to these deleted local accounts, atomically. + /// Only this device's account IDs are passed in: provider rows sync across + /// devices, and a row bound to another device's account must stay bound. + /// Keep raw JSON so unrelated and future metadata survives the update. + pub(crate) fn unbind_codex_managed_accounts( + &self, + account_ids: &[String], + ) -> Result, AppError> { + if account_ids.is_empty() { + return Ok(Vec::new()); + } + let mut conn = lock_conn!(self.conn); + let tx = conn + .transaction() + .map_err(|e| AppError::Database(e.to_string()))?; + let rows = { + let mut stmt = tx + .prepare( + "SELECT id, meta, settings_config, category, + trim(json_extract(meta, '$.authBinding.accountId')) FROM providers + WHERE app_type = 'codex' + AND json_extract(meta, '$.authBinding.source') = 'managed_account' + AND json_extract(meta, '$.authBinding.authProvider') = 'codex_oauth' + ORDER BY id", + ) + .map_err(|e| AppError::Database(e.to_string()))?; + let rows = stmt + .query_map([], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + row.get::<_, Option>(3)?, + row.get::<_, Option>(4)?, + )) + }) + .map_err(|e| AppError::Database(e.to_string()))?; + rows.collect::, _>>() + .map_err(|e| AppError::Database(e.to_string()))? + }; + let mut affected = Vec::new(); + for (id, meta_text, settings_text, category, bound) in rows { + if !bound.is_some_and(|bound| account_ids.iter().any(|id| id.trim() == bound)) { + continue; + } + let parse_error = |error: serde_json::Error| { + AppError::Database(format!("供应商 {id} 配置无效: {error}")) + }; + let mut meta: serde_json::Value = + serde_json::from_str(&meta_text).map_err(parse_error)?; + let mut settings: serde_json::Value = + serde_json::from_str(&settings_text).map_err(parse_error)?; + let mut provider = Provider::with_id(id.clone(), String::new(), settings.clone(), None); + provider.category = category; + provider.meta = Some(serde_json::from_value(meta.clone()).map_err(parse_error)?); + if provider.category.is_none() + && crate::proxy::providers::is_codex_official_provider(&provider) + { + provider.category = Some("official".to_string()); + } + meta.as_object_mut() + .ok_or_else(|| AppError::Config(format!("供应商 {id} 的 meta 必须是对象")))? + .remove("authBinding"); + settings + .as_object_mut() + .ok_or_else(|| { + AppError::Config(format!("供应商 {id} 的 settings_config 必须是对象")) + })? + .insert("auth".to_string(), serde_json::json!({})); + tx.execute( + "UPDATE providers SET meta = ?1, settings_config = ?2, category = ?3 + WHERE app_type = 'codex' AND id = ?4", + params![ + meta.to_string(), + settings.to_string(), + provider.category, + id + ], + ) + .map_err(|e| AppError::Database(e.to_string()))?; + affected.push(id); + } + tx.commit().map_err(|e| AppError::Database(e.to_string()))?; + Ok(affected) + } + pub fn get_all_providers( &self, app_type: &str, diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index ea4726d5a..6ec4e106c 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -3247,12 +3247,10 @@ wire_api = "responses" .unwrap(); } runtime - .block_on( - crate::commands::remove_codex_oauth_account_with_switch_lock( - state, - "old-local-id", - ), - ) + .block_on(remove_codex_account_leaving_dangling_bindings( + state, + "old-local-id", + )) .unwrap(); let restarted = (mode == "direct").then(|| AppState::new(state.db.clone())); let state = restarted.as_ref().unwrap_or(state); @@ -3358,6 +3356,230 @@ wire_api = "responses" } } + /// 删号但不解绑供应商：模拟旧版删号留下的悬空行。解绑失败、别的设备同步来的行 + /// 同样会悬空，恢复路径仍要能处理。 + async fn remove_codex_account_leaving_dangling_bindings( + state: &AppState, + account_id: &str, + ) -> Result<(), String> { + let _switch_guard = state + .proxy_service + .lock_switch_for_app(AppType::Codex.as_str()) + .await; + state + .codex_oauth_manager + .remove_account(account_id) + .await + .map_err(|error| error.to_string()) + } + + fn seed_codex_recovery_accounts(state: &AppState) { + tauri::async_runtime::block_on(async { + for (id, user) in [("acct-old", "old-user"), ("acct-new", "new-user")] { + state + .codex_oauth_manager + .add_test_account_with_user_identity(id, "access", user) + .await + .expect("seed account"); + } + }); + } + + #[test] + #[serial] + fn codex_account_removal_and_logout_unbind_current_and_inactive_cards() { + for logout_all in [false, true] { + with_test_home(|state, _| { + crate::settings::reload_settings().unwrap(); + seed_codex_recovery_accounts(state); + let current = managed_codex_provider("current", "acct-old"); + let mut legacy = managed_codex_provider("legacy", "acct-old"); + legacy.category = None; + legacy.notes = Some("keep notes".into()); + legacy.settings_config["auth"] = + json!({"tokens": {"refresh_token": "stale-snapshot"}}); + legacy.settings_config["model"] = json!("keep-model"); + let other = managed_codex_provider("other", "acct-new"); + // 从另一台设备同步来的行：绑的是那台设备的本地账号，本机删号或登出都不能动它， + // 否则解绑会随云同步回去，弄坏那台设备上正常的绑定。 + let foreign = managed_codex_provider("foreign", "acct-other-device"); + let mut unrelated = managed_codex_provider("unrelated", "acct-old"); + unrelated + .meta + .as_mut() + .unwrap() + .auth_binding + .as_mut() + .unwrap() + .source = AuthBindingSource::ProviderConfig; + for provider in [¤t, &legacy, &other, &foreign, &unrelated] { + state.db.save_provider("codex", provider).unwrap(); + } + state.db.save_provider("claude", ¤t).unwrap(); + state + .db + .conn + .lock() + .unwrap() + .execute( + "UPDATE providers SET meta = json_set(meta, '$.futureField', 'keep') WHERE id = 'legacy'", + [], + ) + .unwrap(); + ProviderService::switch(state, AppType::Codex, ¤t.id).unwrap(); + if logout_all { + tauri::async_runtime::block_on( + crate::commands::logout_codex_oauth_with_switch_lock(state), + ) + .unwrap(); + } else { + tauri::async_runtime::block_on( + crate::commands::remove_codex_oauth_account_with_switch_lock( + state, "acct-old", + ), + ) + .unwrap(); + } + for id in ["current", "legacy"] { + let saved = state.db.get_provider_by_id(id, "codex").unwrap().unwrap(); + assert!(saved.meta.unwrap().auth_binding.is_none()); + assert_eq!(saved.settings_config["auth"], json!({})); + assert_eq!(saved.category.as_deref(), Some("official")); + } + let saved_legacy = state + .db + .get_provider_by_id("legacy", "codex") + .unwrap() + .unwrap(); + assert_eq!(saved_legacy.notes, legacy.notes); + assert_eq!(saved_legacy.settings_config["model"], "keep-model"); + let future: String = state + .db + .conn + .lock() + .unwrap() + .query_row( + "SELECT json_extract(meta, '$.futureField') FROM providers WHERE id = 'legacy'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(future, "keep"); + let bound_to = |id: &str, app: &str| { + state + .db + .get_provider_by_id(id, app) + .unwrap() + .unwrap() + .meta + .unwrap() + .managed_account_id_for("codex_oauth") + }; + assert_eq!(bound_to("other", "codex").is_none(), logout_all); + assert_eq!( + bound_to("foreign", "codex").as_deref(), + Some("acct-other-device") + ); + assert!(state + .db + .get_provider_by_id("unrelated", "codex") + .unwrap() + .unwrap() + .meta + .unwrap() + .auth_binding + .is_some()); + assert!(state + .db + .get_provider_by_id("current", "claude") + .unwrap() + .unwrap() + .meta + .unwrap() + .auth_binding + .is_some()); + assert_eq!( + crate::settings::get_current_provider(&AppType::Codex).as_deref(), + Some("current") + ); + assert!(!crate::codex_config::get_codex_auth_path().exists()); + assert!(!crate::codex_config::codex_managed_oauth_live_auth_marker_exists()); + ProviderService::switch(state, AppType::Codex, "legacy").unwrap(); + assert!( + !crate::codex_config::get_codex_auth_path().exists(), + "unbound legacy card must not resurrect stored credentials" + ); + assert!(state + .db + .unbind_codex_managed_accounts(&["acct-old".to_string()]) + .unwrap() + .is_empty()); + }); + } + } + + #[test] + #[serial] + fn codex_failed_batch_unbinding_rolls_back_and_leaves_a_recovery_path() { + for rebind in [false, true] { + with_test_home(|state, _| { + crate::settings::reload_settings().unwrap(); + seed_codex_recovery_accounts(state); + let current = managed_codex_provider("a-current", "acct-old"); + let second = managed_codex_provider("z-second", "acct-old"); + let target = managed_codex_provider("target", "acct-new"); + for provider in [¤t, &second, &target] { + state.db.save_provider("codex", provider).unwrap(); + } + ProviderService::switch(state, AppType::Codex, ¤t.id).unwrap(); + state + .db + .conn + .lock() + .unwrap() + .execute_batch( + "CREATE TRIGGER reject_unbind BEFORE UPDATE OF meta ON providers + WHEN OLD.id = 'z-second' AND json_extract(NEW.meta, '$.authBinding') IS NULL + BEGIN SELECT RAISE(FAIL, 'injected unbind failure'); END;", + ) + .unwrap(); + let error = tauri::async_runtime::block_on( + crate::commands::remove_codex_oauth_account_with_switch_lock(state, "acct-old"), + ) + .unwrap_err(); + assert!(error.contains("账号已删除") && error.contains("解绑失败")); + for id in [¤t.id, &second.id] { + assert_eq!( + state + .db + .get_provider_by_id(id, "codex") + .unwrap() + .unwrap() + .meta + .unwrap() + .managed_account_id_for("codex_oauth") + .as_deref(), + Some("acct-old") + ); + } + assert!( + !tauri::async_runtime::block_on(state.codex_oauth_manager.list_accounts()) + .iter() + .any(|account| account.id == "acct-old") + ); + if rebind { + let replacement = managed_codex_provider(¤t.id, "acct-new"); + ProviderService::update(state, AppType::Codex, None, replacement).unwrap(); + } else { + ProviderService::switch(state, AppType::Codex, &target.id).unwrap(); + } + let auth: Value = + read_json_file(&crate::codex_config::get_codex_auth_path()).unwrap(); + assert_eq!(auth["tokens"]["account_id"], "acct-new"); + }); + } + } + #[test] #[serial] fn deleted_codex_account_recovers_after_persisted_startup() { @@ -3391,12 +3613,9 @@ wire_api = "responses" crate::mode::controller::enter(&state, &AppType::Codex, false) .await .unwrap(); - crate::commands::remove_codex_oauth_account_with_switch_lock( - &state, - "old-local-id", - ) - .await - .unwrap(); + remove_codex_account_leaving_dangling_bindings(&state, "old-local-id") + .await + .unwrap(); state .codex_oauth_manager .add_test_account_with_workspace_and_access_token( @@ -4722,11 +4941,7 @@ wire_api = "responses" }); } runtime - .block_on( - crate::commands::remove_codex_oauth_account_with_switch_lock( - state, "old", - ), - ) + .block_on(remove_codex_account_leaving_dangling_bindings(state, "old")) .unwrap(); // A stale marker must not claim a later native login of the same user. auth["tokens"]["refresh_token"] = json!("native-rotated-token"); diff --git a/src/components/providers/forms/hooks/useManagedAuth.ts b/src/components/providers/forms/hooks/useManagedAuth.ts index ab11008b7..75384a11a 100644 --- a/src/components/providers/forms/hooks/useManagedAuth.ts +++ b/src/components/providers/forms/hooks/useManagedAuth.ts @@ -236,6 +236,13 @@ export function useManagedAuth( setError(e instanceof Error ? e.message : String(e)); await refetchStatus(); }, + onSettled: async () => { + if (authProvider === "codex_oauth") { + await queryClient.invalidateQueries({ + queryKey: ["providers", "codex"], + }); + } + }, }); const removeAccountMutation = useMutation({ @@ -253,9 +260,18 @@ export function useManagedAuth( await refetchStatus(); await queryClient.invalidateQueries({ queryKey }); }, - onError: (e) => { + onError: async (e) => { console.error("[ManagedAuth] Failed to remove account:", e); setError(e instanceof Error ? e.message : String(e)); + // Account removal can succeed before provider unbinding fails. + if (authProvider === "codex_oauth") await refetchStatus(); + }, + onSettled: async () => { + if (authProvider === "codex_oauth") { + await queryClient.invalidateQueries({ + queryKey: ["providers", "codex"], + }); + } }, }); diff --git a/tests/hooks/useManagedAuth.test.tsx b/tests/hooks/useManagedAuth.test.tsx index fd9ec5252..6a036f045 100644 --- a/tests/hooks/useManagedAuth.test.tsx +++ b/tests/hooks/useManagedAuth.test.tsx @@ -11,6 +11,7 @@ const apiMocks = vi.hoisted(() => ({ authPollForAccount: vi.fn(), authCancelLogin: vi.fn(), authRemoveAccount: vi.fn(), + authLogout: vi.fn(), })); const toastMocks = vi.hoisted(() => ({ success: vi.fn(), @@ -23,6 +24,7 @@ vi.mock("@/lib/api", () => ({ authPollForAccount: (...args: unknown[]) => apiMocks.authPollForAccount(...args), authCancelLogin: (...args: unknown[]) => apiMocks.authCancelLogin(...args), + authLogout: (...args: unknown[]) => apiMocks.authLogout(...args), authRemoveAccount: (...args: unknown[]) => apiMocks.authRemoveAccount(...args), }, @@ -41,14 +43,14 @@ vi.mock("sonner", () => ({ }, })); -function createWrapper() { - const queryClient = new QueryClient({ +function createWrapper( + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false }, }, - }); - + }), +) { return function Wrapper({ children }: { children: ReactNode }) { return ( {children} @@ -82,6 +84,7 @@ describe("useManagedAuth", () => { apiMocks.authPollForAccount.mockReset().mockResolvedValue(null); apiMocks.authCancelLogin.mockReset().mockResolvedValue(true); apiMocks.authRemoveAccount.mockReset().mockResolvedValue(undefined); + apiMocks.authLogout.mockReset().mockResolvedValue(undefined); }); it("starts reauthentication for the selected account", async () => { @@ -259,6 +262,62 @@ describe("useManagedAuth", () => { ); }); + it.each([ + ["remove", false], + ["remove", true], + ["logout", false], + ["logout", true], + ] as const)( + "refreshes providers and accounts after %s (partial failure=%s)", + async (operation, fails) => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); + queryClient.setQueryData(["providers", "codex"], { + current: "old-binding", + }); + queryClient.setQueryData(["providers", "claude"], { + current: "unrelated", + }); + const { result } = renderHook(() => useManagedAuth("codex_oauth"), { + wrapper: createWrapper(queryClient), + }); + await waitFor(() => expect(result.current.accounts).toHaveLength(1)); + apiMocks.authGetStatus.mockResolvedValue({ + provider: "codex_oauth", + authenticated: false, + accounts: [], + }); + if (fails) { + const mutation = + operation === "remove" + ? apiMocks.authRemoveAccount + : apiMocks.authLogout; + mutation.mockRejectedValue( + new Error("账号已删除，但 Codex 供应商解绑失败"), + ); + } + act(() => + operation === "remove" + ? result.current.removeAccount("acct-1") + : result.current.logout(), + ); + await waitFor(() => + expect( + queryClient.getQueryState(["providers", "codex"])?.isInvalidated, + ).toBe(true), + ); + await waitFor(() => expect(result.current.accounts).toHaveLength(0)); + expect( + queryClient.getQueryState(["providers", "claude"])?.isInvalidated, + ).toBe(false); + if (fails) expect(result.current.error).toContain("解绑失败"); + }, + ); + it("shows a success toast after removing an account", async () => { const { result } = renderHook(() => useManagedAuth("codex_oauth"), { wrapper: createWrapper(),