mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(codex-accounts): keep the old login until re-auth finishes, and relabel swapped homes (#33568)
Codex account re-auth no longer signs the account out on cancel/failure: `codex login` revokes and deletes existing auth before it starts, so re-auth now logs in inside a staging CODEX_HOME and copies auth.json in only on success. Listing accounts relabels a host account whose auth.json identity no longer matches its saved record. Fixes #10795 Fixes #22284 Supersedes #11852 Co-authored-by: Rod Boev <106971+rodboev@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,11 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { copyFileSync, mkdirSync, rmSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence'
|
||||
import {
|
||||
WINDOWS_RM_MAX_RETRIES,
|
||||
WINDOWS_RM_RETRY_DELAY_MS
|
||||
} from '../../shared/windows-transient-lock-removal'
|
||||
import type {
|
||||
CodexManagedAccount,
|
||||
CodexRateLimitAccountsState
|
||||
@@ -39,6 +46,17 @@ type CodexAccountRegistrationDependencies = {
|
||||
login: (managedHomePath: string) => Promise<void>
|
||||
}
|
||||
|
||||
const REAUTH_STAGING_DIR_NAME = '.orca-reauth-login'
|
||||
|
||||
function removeStagingHome(stagingHomePath: string): void {
|
||||
rmSync(stagingHomePath, {
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: WINDOWS_RM_MAX_RETRIES,
|
||||
retryDelay: WINDOWS_RM_RETRY_DELAY_MS
|
||||
})
|
||||
}
|
||||
|
||||
export class CodexAccountRegistration {
|
||||
constructor(private readonly dependencies: CodexAccountRegistrationDependencies) {}
|
||||
|
||||
@@ -93,7 +111,7 @@ export class CodexAccountRegistration {
|
||||
options?.activateIfSelectionWasEmpty === true && selectedAccountId === null
|
||||
|
||||
this.dependencies.configMirror.safeSyncIntoManagedHome(managedHomePath, undefined, account.id)
|
||||
await this.dependencies.login(managedHomePath)
|
||||
await this.loginThroughStagingHome(managedHomePath, account.id)
|
||||
const identity = this.dependencies.readIdentityFromHome(managedHomePath, account.id)
|
||||
if (!identity.email) {
|
||||
throw new Error('Codex login completed, but Orca could not resolve the account email.')
|
||||
@@ -134,6 +152,31 @@ export class CodexAccountRegistration {
|
||||
return this.dependencies.selection.snapshot()
|
||||
}
|
||||
|
||||
// `codex login` revokes and deletes the home's current auth first, so only a finished login may replace it (#10795).
|
||||
private async loginThroughStagingHome(managedHomePath: string, accountId: string): Promise<void> {
|
||||
const stagingHomePath = join(managedHomePath, REAUTH_STAGING_DIR_NAME)
|
||||
removeStagingHome(stagingHomePath)
|
||||
mkdirSync(stagingHomePath, { recursive: true })
|
||||
try {
|
||||
// Carry forced-workspace and credential-store settings into the login.
|
||||
copyFileSync(join(managedHomePath, 'config.toml'), join(stagingHomePath, 'config.toml'))
|
||||
} catch (error) {
|
||||
if (!isDefinitiveAbsence(error)) {
|
||||
throw error
|
||||
}
|
||||
}
|
||||
try {
|
||||
await this.dependencies.login(stagingHomePath)
|
||||
this.dependencies.managedHomes.importAuthFromHome(stagingHomePath, managedHomePath, accountId)
|
||||
} finally {
|
||||
try {
|
||||
removeStagingHome(stagingHomePath)
|
||||
} catch (error) {
|
||||
console.warn('[codex-accounts] Failed to remove reauthentication staging home:', error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private prepareManagedHomeForLogin(managedHomePath: string, accountId: string): void {
|
||||
const canonicalConfig = this.dependencies.configMirror.readForManagedHome(managedHomePath)
|
||||
this.dependencies.configMirror.assertOAuthAccountAddAllowed(canonicalConfig)
|
||||
|
||||
@@ -3,10 +3,13 @@ import type {
|
||||
CodexRateLimitAccountsState,
|
||||
CodexSystemDefaultIdentity
|
||||
} from '../../shared/managed-account-types'
|
||||
import { parseWslUncPath } from '../../shared/wsl-paths'
|
||||
import type { Store } from '../persistence'
|
||||
import type { RateLimitService } from '../rate-limits/service'
|
||||
import type { CodexRuntimeHomeService } from './runtime-home-service'
|
||||
import type { CodexConfigMirror } from './codex-config-mirror'
|
||||
import type { ResolvedCodexIdentity } from './codex-account-identity'
|
||||
import { codexAuthIdentityContradictsAccount } from './codex-auth-identity'
|
||||
import type { CodexAccountServiceLifecycle } from './codex-account-service-types'
|
||||
import { toCodexManagedAccountSummary } from './codex-account-service-types'
|
||||
import {
|
||||
@@ -27,6 +30,10 @@ type CodexAccountSelectionDependencies = {
|
||||
configMirror: CodexConfigMirror
|
||||
lifecycle: CodexAccountServiceLifecycle
|
||||
resolveSystemDefault: () => CodexSystemDefaultIdentity
|
||||
readIdentityFromHome: (
|
||||
managedHomePath: string,
|
||||
expectedAccountId: string
|
||||
) => ResolvedCodexIdentity
|
||||
removeManagedHome: (candidatePath: string, expectedAccountId: string) => void
|
||||
discardResetAttempts: (accountId: string) => Promise<void>
|
||||
}
|
||||
@@ -36,6 +43,7 @@ export class CodexAccountSelection {
|
||||
|
||||
list(): CodexRateLimitAccountsState {
|
||||
this.normalizeActiveSelection()
|
||||
this.adoptManagedHomeIdentities()
|
||||
return this.snapshot()
|
||||
}
|
||||
|
||||
@@ -144,6 +152,42 @@ export class CodexAccountSelection {
|
||||
return this.snapshot()
|
||||
}
|
||||
|
||||
// A `codex login` run inside a managed home swaps its account; label the row with what actually runs (#22284).
|
||||
private adoptManagedHomeIdentities(): void {
|
||||
const settings = this.dependencies.store.getSettings()
|
||||
let changed = false
|
||||
const accounts = settings.codexManagedAccounts.map((account) => {
|
||||
// WSL homes are read over UNC, which can block when the distro is stopped.
|
||||
if (account.managedHomeRuntime === 'wsl' || parseWslUncPath(account.managedHomePath)) {
|
||||
return account
|
||||
}
|
||||
let identity: ResolvedCodexIdentity
|
||||
try {
|
||||
identity = this.dependencies.readIdentityFromHome(account.managedHomePath, account.id)
|
||||
} catch {
|
||||
return account
|
||||
}
|
||||
if (!identity.email || !codexAuthIdentityContradictsAccount(identity, account)) {
|
||||
return account
|
||||
}
|
||||
console.warn(
|
||||
'[codex-accounts] Managed home holds a different Codex login; updating its label'
|
||||
)
|
||||
changed = true
|
||||
return {
|
||||
...account,
|
||||
email: identity.email,
|
||||
providerAccountId: identity.providerAccountId,
|
||||
workspaceLabel: identity.workspaceLabel,
|
||||
workspaceAccountId: identity.workspaceAccountId,
|
||||
updatedAt: Date.now()
|
||||
}
|
||||
})
|
||||
if (changed) {
|
||||
this.dependencies.store.updateSettings({ codexManagedAccounts: accounts })
|
||||
}
|
||||
}
|
||||
|
||||
private normalizeActiveSelection(): void {
|
||||
const settings = this.dependencies.store.getSettings()
|
||||
const selection = normalizeCodexRuntimeSelection(settings)
|
||||
|
||||
@@ -65,16 +65,20 @@ export function codexAuthCouldBelongToManagedAccount(
|
||||
account: CodexManagedAccount
|
||||
): boolean {
|
||||
const identity = readCodexAuthIdentity(runtimeAuthContents)
|
||||
return (
|
||||
!identity ||
|
||||
!identityContradictsSelection(
|
||||
{
|
||||
email: normalizeField(account.email),
|
||||
providerAccountId: normalizeField(account.providerAccountId),
|
||||
workspaceAccountId: normalizeField(account.workspaceAccountId)
|
||||
},
|
||||
identity
|
||||
)
|
||||
return !identity || !codexAuthIdentityContradictsAccount(identity, account)
|
||||
}
|
||||
|
||||
export function codexAuthIdentityContradictsAccount(
|
||||
identity: CodexAuthIdentity,
|
||||
account: CodexManagedAccount
|
||||
): boolean {
|
||||
return identityContradictsSelection(
|
||||
{
|
||||
email: normalizeField(account.email),
|
||||
providerAccountId: normalizeField(account.providerAccountId),
|
||||
workspaceAccountId: normalizeField(account.workspaceAccountId)
|
||||
},
|
||||
identity
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -234,8 +234,10 @@ describe('CodexAccountService config sync', () => {
|
||||
const spawnMock = vi.fn(
|
||||
(_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => {
|
||||
const loginHome = options.env.CODEX_HOME
|
||||
expect(loginHome).toBeTruthy()
|
||||
expect(readFileSync(join(loginHome!, '.orca-managed-home'), 'utf-8')).toBe('account-1\n')
|
||||
expect(loginHome).toMatch(/[\\/]account-1[\\/]home[\\/]\.orca-reauth-login$/)
|
||||
expect(readFileSync(join(managedHomePath, '.orca-managed-home'), 'utf-8')).toBe(
|
||||
'account-1\n'
|
||||
)
|
||||
expect(readFileSync(join(loginHome!, 'config.toml'), 'utf-8')).toBe(canonicalConfig)
|
||||
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { describe, expect, it, vi, beforeEach } from 'vitest'
|
||||
import { installFakeAppEnvironment } from '../../../config/scripts/vitest-host-ports-setup'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { PassThrough } from 'node:stream'
|
||||
import {
|
||||
createCodexAuthJson,
|
||||
createManagedHome,
|
||||
createRateLimits,
|
||||
createRuntimeHome,
|
||||
createSettings,
|
||||
createStore,
|
||||
registerCodexAccountsTestHomes,
|
||||
testState
|
||||
} from './service-test-harness'
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
app: {
|
||||
getPath: () => testState.userDataDir
|
||||
}
|
||||
}))
|
||||
beforeEach(() => installFakeAppEnvironment({ getPath: () => testState.userDataDir }))
|
||||
|
||||
vi.mock(import('node:os'), async (importOriginal) => {
|
||||
const actual = await importOriginal()
|
||||
return {
|
||||
...actual,
|
||||
homedir: () => testState.fakeHomeDir
|
||||
}
|
||||
})
|
||||
|
||||
const accountIAuth = createCodexAuthJson('i@example.com', 'provider-i', 'refresh-i')
|
||||
|
||||
function createAccountI() {
|
||||
return {
|
||||
id: 'account-i',
|
||||
email: 'i@example.com',
|
||||
managedHomePath: createManagedHome(testState.userDataDir, 'account-i', '', accountIAuth),
|
||||
providerAccountId: 'provider-i',
|
||||
workspaceLabel: null,
|
||||
workspaceAccountId: 'provider-i',
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
lastAuthenticatedAt: 1
|
||||
}
|
||||
}
|
||||
|
||||
async function createService(settings: ReturnType<typeof createSettings>) {
|
||||
const store = createStore(settings)
|
||||
const { CodexAccountService } = await import('./service')
|
||||
const service = new CodexAccountService(
|
||||
store as never,
|
||||
createRateLimits() as never,
|
||||
createRuntimeHome() as never
|
||||
)
|
||||
return { service, store }
|
||||
}
|
||||
|
||||
describe('Codex managed home identity', () => {
|
||||
registerCodexAccountsTestHomes()
|
||||
|
||||
it('keeps the existing login when a reauthentication is abandoned', async () => {
|
||||
vi.resetModules()
|
||||
// Mirrors `codex login`, which deletes the home's auth before its login server starts.
|
||||
const spawnMock = vi.fn(
|
||||
(_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => {
|
||||
rmSync(join(options.env.CODEX_HOME!, 'auth.json'), { force: true })
|
||||
const child = Object.assign(new EventEmitter(), {
|
||||
stdout: new PassThrough(),
|
||||
stderr: new PassThrough(),
|
||||
exitCode: null,
|
||||
signalCode: null,
|
||||
kill: vi.fn(() => true)
|
||||
})
|
||||
return child
|
||||
}
|
||||
)
|
||||
vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock }))
|
||||
vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' }))
|
||||
const account = createAccountI()
|
||||
const { service, store } = await createService(
|
||||
createSettings({
|
||||
codexManagedAccounts: [account],
|
||||
activeCodexManagedAccountId: 'account-i',
|
||||
activeCodexManagedAccountIdsByRuntime: { host: 'account-i', wsl: {} }
|
||||
})
|
||||
)
|
||||
|
||||
const reauth = service.reauthenticateAccount('account-i')
|
||||
await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce())
|
||||
expect(service.cancelPendingLogin()).toBe(true)
|
||||
|
||||
await expect(reauth).rejects.toThrow()
|
||||
expect(readFileSync(join(account.managedHomePath, 'auth.json'), 'utf-8')).toBe(accountIAuth)
|
||||
expect(store.getSettings().activeCodexManagedAccountId).toBe('account-i')
|
||||
})
|
||||
|
||||
it('relabels a managed account whose home now holds a different login', async () => {
|
||||
const account = createAccountI()
|
||||
writeFileSync(
|
||||
join(account.managedHomePath, 'auth.json'),
|
||||
createCodexAuthJson('c@example.com', 'provider-c', 'refresh-c'),
|
||||
'utf-8'
|
||||
)
|
||||
const { service } = await createService(createSettings({ codexManagedAccounts: [account] }))
|
||||
|
||||
const listed = service.listAccounts().accounts.find((entry) => entry.id === 'account-i')
|
||||
|
||||
expect(listed).toMatchObject({ email: 'c@example.com', providerAccountId: 'provider-c' })
|
||||
expect(listed?.updatedAt).toBeGreaterThan(account.updatedAt)
|
||||
})
|
||||
})
|
||||
@@ -514,7 +514,9 @@ describe('CodexAccountService config sync', () => {
|
||||
let clearSelectionDuringLogin = (): void => {}
|
||||
const spawnMock = vi.fn((command: string, args: string[]) => {
|
||||
expect(command).toBe('wsl.exe')
|
||||
expect(args).toEqual(buildWslCodexLoginArgs('Ubuntu', wslLinuxHomePath))
|
||||
expect(args).toEqual(
|
||||
buildWslCodexLoginArgs('Ubuntu', `${wslLinuxHomePath}/.orca-reauth-login`)
|
||||
)
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
stdout: PassThrough
|
||||
stderr: PassThrough
|
||||
@@ -525,7 +527,7 @@ describe('CodexAccountService config sync', () => {
|
||||
child.kill = vi.fn()
|
||||
clearSelectionDuringLogin()
|
||||
writeFileSync(
|
||||
join(wslManagedHomePath, 'auth.json'),
|
||||
join(wslManagedHomePath, '.orca-reauth-login', 'auth.json'),
|
||||
JSON.stringify({
|
||||
tokens: {
|
||||
id_token: `header.${Buffer.from(JSON.stringify({ email: 'new@example.com' })).toString(
|
||||
@@ -546,7 +548,12 @@ describe('CodexAccountService config sync', () => {
|
||||
vi.doMock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock }))
|
||||
vi.doMock('../../shared/wsl-paths', () => ({
|
||||
parseWslUncPath: (path: string) =>
|
||||
path === wslManagedHomePath ? { distro: 'Ubuntu', linuxPath: wslLinuxHomePath } : null
|
||||
path.startsWith(wslManagedHomePath)
|
||||
? {
|
||||
distro: 'Ubuntu',
|
||||
linuxPath: `${wslLinuxHomePath}${path.slice(wslManagedHomePath.length).replaceAll('\\', '/')}`
|
||||
}
|
||||
: null
|
||||
}))
|
||||
vi.doMock('../wsl', () => ({
|
||||
toWindowsWslPath: () => wslManagedHomePath
|
||||
@@ -654,7 +661,9 @@ describe('CodexAccountService config sync', () => {
|
||||
})
|
||||
const spawnMock = vi.fn((command: string, args: string[]) => {
|
||||
expect(command).toBe('wsl.exe')
|
||||
expect(args).toEqual(buildWslCodexLoginArgs('Ubuntu', wslLinuxHomePath))
|
||||
expect(args).toEqual(
|
||||
buildWslCodexLoginArgs('Ubuntu', `${wslLinuxHomePath}/.orca-reauth-login`)
|
||||
)
|
||||
expect(readFileSync(join(wslManagedHomePath, '.orca-managed-home'), 'utf-8')).toBe(
|
||||
'account-1\n'
|
||||
)
|
||||
@@ -667,7 +676,7 @@ describe('CodexAccountService config sync', () => {
|
||||
child.stderr = new PassThrough()
|
||||
child.kill = vi.fn()
|
||||
writeFileSync(
|
||||
join(wslManagedHomePath, 'auth.json'),
|
||||
join(wslManagedHomePath, '.orca-reauth-login', 'auth.json'),
|
||||
createCodexAuthJson('new-wsl@example.com', 'provider-wsl-1', 'refresh-token'),
|
||||
'utf-8'
|
||||
)
|
||||
@@ -682,7 +691,12 @@ describe('CodexAccountService config sync', () => {
|
||||
vi.doMock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock }))
|
||||
vi.doMock('../../shared/wsl-paths', () => ({
|
||||
parseWslUncPath: (path: string) =>
|
||||
path === wslManagedHomePath ? { distro: 'Ubuntu', linuxPath: wslLinuxHomePath } : null
|
||||
path.startsWith(wslManagedHomePath)
|
||||
? {
|
||||
distro: 'Ubuntu',
|
||||
linuxPath: `${wslLinuxHomePath}${path.slice(wslManagedHomePath.length).replaceAll('\\', '/')}`
|
||||
}
|
||||
: null
|
||||
}))
|
||||
vi.doMock('../wsl', () => ({
|
||||
toWindowsWslPath: () => wslManagedHomePath
|
||||
|
||||
@@ -127,6 +127,7 @@ export class CodexAccountService {
|
||||
configMirror: this.configMirror,
|
||||
lifecycle,
|
||||
resolveSystemDefault: () => this.resolveSystemDefaultIdentity(),
|
||||
readIdentityFromHome: (path, accountId) => this.readIdentityFromHome(path, accountId),
|
||||
removeManagedHome: (path, accountId) => this.safeRemoveManagedHome(path, accountId),
|
||||
discardResetAttempts: (accountId) => this.resetCredits.discardForRemovedAccount(accountId)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user