From 6fff4d6df11ebca7f62da617466795c5f4499fb6 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:11:37 -0300 Subject: [PATCH] fix(auth): dedup Codex OAuth import by workspace AND user id (#6301) (#6385) --- CHANGELOG.md | 2 + src/lib/oauth/utils/codexAuthImport.ts | 71 ++++++++- ...odex-auth-import-userid-dedup-6301.test.ts | 150 ++++++++++++++++++ 3 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 tests/unit/codex-auth-import-userid-dedup-6301.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ee3d282aeb..33a8a33ffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### 🐛 Bug Fixes +- **fix(auth):** importing a **distinct** Codex/ChatGPT OAuth `auth.json` is no longer falsely rejected as "already exists" when it belongs to a different user in the same workspace ([#6301](https://github.com/diegosouzapw/OmniRoute/issues/6301)). `findExistingCodexConnection` (in `src/lib/oauth/utils/codexAuthImport.ts`) deduped **only** on `providerSpecificData.workspaceId === accountId`, where `accountId` is the shared `chatgpt_account_id`/`tokens.account_id` — so two members of the same ChatGPT Team collapsed onto a single connection (409 `duplicate_account`). The id_token's `https://api.openai.com/auth` claim carries a per-user `chatgpt_user_id` alongside the workspace id (the device-flow path already persisted it as `chatgptUserId`, but the import path did not). Now `parseAndValidateCodexAuth` extracts `userId` (`chatgpt_user_id` → `user_id` → JWT `sub`) into `ParsedCodexAuth`, the create/update paths persist `chatgptUserId` in `providerSpecificData` (mirroring `codex.ts`), and dedup keys on `workspaceId` **AND** `chatgptUserId` — with a backward-compat fallback to legacy accountId-only matching when no stored connection for that workspace records a `chatgptUserId`, so genuinely-same accounts still dedup. Regression guard: `tests/unit/codex-auth-import-userid-dedup-6301.test.ts` (4). (thanks @anungma) + - **fix(providers):** importing models for the **venice-web** provider no longer fails with a red "Provider venice-web does not support models listing" ([#6269](https://github.com/diegosouzapw/OmniRoute/issues/6269)). `venice-web` is a web-cookie provider with an executor but no upstream `/v1/models` endpoint and no registry `models`, so the models route fell through to the tail `400`. Mirroring the `jules`/`linkup-search`/`ollama-search` fix (#5569), it now ships a static local catalog entry in `src/lib/providers/staticModels.ts` — seeding the current Venice lineup (`venice-uncensored`, `llama-3.3-70b`, `qwen3-235b`, `qwen3-4b`, `deepseek-r1-671b`; Venice rotates its catalog, see docs.venice.ai/models/overview) — so the route returns `200` with `source:"local_catalog"`, `intentional:true`. Regression guard: `tests/unit/static-models-venice-web-6269.test.ts`. (thanks @chirag127) - **fix(api):** the specialty model catalogs (`/v1/embeddings`, `/v1/images`, `/v1/music`, `/v1/videos` model lists) are now derived from the **unified catalog filtered by a predicate** (`getSpecialtyModelsResponse`) instead of ad-hoc per-route logic, so they consistently respect active-credential visibility and stay in sync with the main catalog. Regression guard: `tests/unit/specialty-model-catalog-routes.test.ts`. (thanks @makcimbx) diff --git a/src/lib/oauth/utils/codexAuthImport.ts b/src/lib/oauth/utils/codexAuthImport.ts index 500421c1ef..22ff670750 100644 --- a/src/lib/oauth/utils/codexAuthImport.ts +++ b/src/lib/oauth/utils/codexAuthImport.ts @@ -65,6 +65,28 @@ function extractCodexAccountId( ); } +// Two DISTINCT users can share the SAME workspace/account id (e.g. two members of the +// same ChatGPT Team). The account id alone is NOT a unique connection key. The id_token +// auth claim carries the per-user `chatgpt_user_id`; use it (falling back to `user_id`, +// then the JWT `sub`) so imports can be deduped by workspace AND user. See #6301. +function extractCodexUserId(idToken: string): string | null { + const payload = decodeJwtPayload(idToken); + if (!payload) return null; + const authInfo = toRecord(payload["https://api.openai.com/auth"]); + return ( + toNonEmptyString(authInfo.chatgpt_user_id) || + toNonEmptyString(authInfo.user_id) || + toNonEmptyString(payload.sub) || + null + ); +} + +// On overwrite, keep the incoming per-user id but fall back to the one already stored +// on the existing connection (legacy imports carried none) rather than dropping it (#6301). +function mergeCodexUserId(incomingUserId: string | null, existing: JsonRecord): string | null { + return incomingUserId ?? toNonEmptyString(toRecord(existing.providerSpecificData).chatgptUserId); +} + // ──── Public types ──────────────────────────────────────────────────────────── export interface ParsedCodexAuth { @@ -72,6 +94,9 @@ export interface ParsedCodexAuth { accessToken: string; refreshToken: string; accountId: string; + // Per-user identity within the workspace (chatgpt_user_id / user_id / JWT sub). + // Distinct users can share the same accountId, so this disambiguates them (#6301). + userId: string | null; email: string | null; expiresAt: string | null; } @@ -142,6 +167,7 @@ export function parseAndValidateCodexAuth(raw: unknown): ParsedCodexAuth { accessToken, refreshToken, accountId, + userId: extractCodexUserId(idToken), email: extractJwtEmail(idToken), expiresAt: extractExpiresAt(accessToken, idToken), }; @@ -153,7 +179,7 @@ export async function createConnectionFromAuthFile( parsed: ParsedCodexAuth, options: CreateConnectionOptions ): Promise<{ connection: JsonRecord; created: boolean }> { - const existing = await findExistingCodexConnection(parsed.accountId); + const existing = await findExistingCodexConnection(parsed.accountId, parsed.userId); if (existing) { if (!options.overwriteExisting) { @@ -180,6 +206,7 @@ export async function createConnectionFromAuthFile( providerSpecificData: { ...toRecord(existing.providerSpecificData), workspaceId: parsed.accountId, + chatgptUserId: mergeCodexUserId(parsed.userId, existing), importedAt: new Date().toISOString(), }, }); @@ -202,6 +229,7 @@ export async function createConnectionFromAuthFile( testStatus: "active", providerSpecificData: { workspaceId: parsed.accountId, + chatgptUserId: parsed.userId, importedAt: new Date().toISOString(), }, }); @@ -228,12 +256,39 @@ export async function createConnectionFromAuthFile( return { connection, created: true }; } -async function findExistingCodexConnection(accountId: string): Promise { - const connections = await getProviderConnections({ provider: "codex" }); - return ( - (connections.find((c) => { - const psd = toRecord((c as JsonRecord).providerSpecificData); - return toNonEmptyString(psd.workspaceId) === accountId; - }) as JsonRecord | undefined) ?? null +// Dedup key is the workspace/account id AND the per-user id. Two distinct users in the +// same workspace share an accountId but have different userIds, so they must NOT collide +// (#6301). Backward-compat: connections imported before the chatgptUserId field existed +// carry no stored userId — when NONE of the workspace matches has a stored userId we fall +// back to the legacy accountId-only match so genuinely-same accounts still dedup. +// From the connections already matched on workspace/account id, pick the one that +// belongs to the incoming user. A different user in the same workspace is NOT a +// duplicate — but only refuse to dedup when some stored connection actually records a +// (different) userId; if none do, they are legacy records and we dedup with the first. +function pickCodexConnectionForUser( + workspaceMatches: JsonRecord[], + userId: string +): JsonRecord | null { + const exact = workspaceMatches.find( + (c) => toNonEmptyString(toRecord(c.providerSpecificData).chatgptUserId) === userId ); + if (exact) return exact; + const anyHasStoredUserId = workspaceMatches.some( + (c) => toNonEmptyString(toRecord(c.providerSpecificData).chatgptUserId) !== null + ); + return anyHasStoredUserId ? null : workspaceMatches[0]; +} + +async function findExistingCodexConnection( + accountId: string, + userId: string | null +): Promise { + const connections = await getProviderConnections({ provider: "codex" }); + const workspaceMatches = (connections as JsonRecord[]).filter( + (c) => toNonEmptyString(toRecord(c.providerSpecificData).workspaceId) === accountId + ); + if (workspaceMatches.length === 0) return null; + // No incoming userId → legacy accountId-only dedup with the first workspace match. + if (!userId) return workspaceMatches[0]; + return pickCodexConnectionForUser(workspaceMatches, userId); } diff --git a/tests/unit/codex-auth-import-userid-dedup-6301.test.ts b/tests/unit/codex-auth-import-userid-dedup-6301.test.ts new file mode 100644 index 0000000000..39406d90d8 --- /dev/null +++ b/tests/unit/codex-auth-import-userid-dedup-6301.test.ts @@ -0,0 +1,150 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// #6301: importing a DISTINCT Codex/ChatGPT OAuth auth.json is falsely detected as +// "already exists" when it shares the same account/workspace id but has a different +// user identity. Dedup must key on workspace AND chatgpt_user_id. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-codex-userid-dedup-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const { parseAndValidateCodexAuth, createConnectionFromAuthFile } = await import( + "../../src/lib/oauth/utils/codexAuthImport.ts" +); + +type JsonRecord = Record; + +function buildJwt(payload: JsonRecord): string { + const header = Buffer.from(JSON.stringify({ alg: "RS256", typ: "JWT" })).toString("base64url"); + const body = Buffer.from(JSON.stringify(payload)).toString("base64url"); + return `${header}.${body}.fake-signature`; +} + +// Build a Codex CLI auth.json sharing accountId but with a caller-chosen chatgpt_user_id. +function buildAuthFile(accountId: string, userId: string, email: string): JsonRecord { + const idToken = buildJwt({ + email, + exp: 9999999999, + "https://api.openai.com/auth": { + chatgpt_account_id: accountId, + chatgpt_user_id: userId, + }, + }); + return { + auth_mode: "chatgpt", + OPENAI_API_KEY: null, + tokens: { + id_token: idToken, + access_token: `at-${userId}`, + refresh_token: `rt-${userId}`, + // Intentionally omit account_id so it is derived from the JWT claim (shared). + }, + last_refresh: new Date().toISOString(), + }; +} + +async function resetStorage() { + core.resetDbInstance(); + for (let attempt = 0; attempt < 10; attempt++) { + try { + if (fs.existsSync(TEST_DATA_DIR)) { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } + break; + } catch (error) { + const code = (error as { code?: string } | null)?.code; + if ((code === "EBUSY" || code === "EPERM") && attempt < 9) { + await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1))); + } else { + throw error; + } + } + } + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("parseAndValidateCodexAuth extracts userId from chatgpt_user_id claim", () => { + const parsed = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-alice", "alice@example.com") + ); + assert.equal(parsed.accountId, "acct-shared"); + assert.equal(parsed.userId, "user-alice"); +}); + +test("#6301: same workspace, DIFFERENT user → both imports create a new connection", async () => { + const alice = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-alice", "alice@example.com") + ); + const bob = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-bob", "bob@example.com") + ); + + // Sanity: same account id, distinct user id. + assert.equal(alice.accountId, bob.accountId); + assert.notEqual(alice.userId, bob.userId); + + const first = await createConnectionFromAuthFile(alice, {}); + assert.equal(first.created, true); + + // The bug: this used to throw 409 duplicate_account. It must now create a new one. + const second = await createConnectionFromAuthFile(bob, {}); + assert.equal(second.created, true); + assert.notEqual((second.connection as JsonRecord).id, (first.connection as JsonRecord).id); +}); + +test("same workspace AND same user → still deduped (update, not create)", async () => { + const alice1 = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-alice", "alice@example.com") + ); + const first = await createConnectionFromAuthFile(alice1, {}); + assert.equal(first.created, true); + + // Re-import the same identity with overwrite → dedup to the existing connection. + const alice2 = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-alice", "alice@example.com") + ); + const second = await createConnectionFromAuthFile(alice2, { overwriteExisting: true }); + assert.equal(second.created, false); + assert.equal((second.connection as JsonRecord).id, (first.connection as JsonRecord).id); +}); + +test("backward-compat: legacy connection without stored userId still dedups by accountId", async () => { + const providersDb = await import("../../src/lib/db/providers.ts"); + + // Simulate a connection imported before the chatgptUserId field existed. + const legacy = await providersDb.createProviderConnection({ + provider: "codex", + authType: "oauth", + name: "Legacy Codex", + accessToken: "at-legacy", + refreshToken: "rt-legacy", + idToken: "id-legacy", + isActive: true, + testStatus: "active", + providerSpecificData: { + workspaceId: "acct-shared", + importedAt: new Date().toISOString(), + // no chatgptUserId + }, + }); + + const incoming = parseAndValidateCodexAuth( + buildAuthFile("acct-shared", "user-alice", "alice@example.com") + ); + const result = await createConnectionFromAuthFile(incoming, { overwriteExisting: true }); + assert.equal(result.created, false); + assert.equal((result.connection as JsonRecord).id, legacy.id); +});