diff --git a/CHANGELOG.md b/CHANGELOG.md index acbf704144..bcfe877de4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,8 @@ - **fix(github):** drop a trailing assistant prefill before dispatching to GitHub Copilot chat to avoid 400 errors. (thanks @baslr) +- **fix(oauth):** prevent cross-IdP account overwrites by disambiguating OAuth connections on `username` when present, not email alone. (thanks @KunN-21) + - **providers (Kiro — Claude Sonnet 5):** the Kiro provider's model catalog was missing `claude-sonnet-5`, so the model could not be selected or routed even on accounts that already had access to it ("claude-sonnet-5 is not supported"). Added the model to the Kiro registry (`open-sse/config/providers/registry/kiro/index.ts`) as a 1M-context / 128K-output Claude model, mirroring the existing Claude entries; the registry `models[]` feeds both the model selector and the live CodeWhisperer `ListAvailableModels` fallback, so the model is now selectable and routable. Regression guard: `tests/unit/kiro-claude-sonnet-5-2267.test.ts`. (thanks [@openbioinfo](https://github.com/openbioinfo)) - **settings (model aliases — self-heal after restart):** the Settings → Routing page showed "No exact-match aliases configured" after a server restart even though the aliases were persisted in the DB. Aliases are held in a module-local `_customAliases` map in `modelDeprecation.ts` that the boot path hydrates, but Next.js compiles the app-route module graph separately from the startup graph (the same webpack chunk-splitting class as #5312), so the `GET /api/settings/model-aliases` handler read a different, un-hydrated copy. The handler now self-heals: when its in-memory alias map is empty it reads `settings.modelAliases` from the DB (via the existing `getSettings()` db module — no raw SQL in the route) and repopulates the map, so the UI reflects the persisted aliases on the first GET after a restart. Follow-up: the root cause is now also fixed — the `_customAliases` store in `modelDeprecation.ts` is backed by `globalThis` (key `__omniroute_customAliases__`), so the startup and app-route module graphs share **one** store and the route reads the boot-hydrated aliases directly (the DB self-heal remains as a harmless fallback), mirroring the same `globalThis` singleton pattern already applied to `thinkingBudget.ts`/`backgroundTaskDetector.ts` (#5312). Regression guards: `tests/unit/model-aliases-settings-route-selfheal.test.ts` + `tests/unit/model-aliases-globalthis-5777.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2)) diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index 029e78d333..9323dbc10e 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -170,13 +170,31 @@ export async function createProviderConnection(data: JsonRecord) { // For Codex with workspaceId, don't fall back to email-only check // This allows creating new connections for different workspaces } else { - // For other providers (or Codex without workspaceId), use email check + // For other providers (or Codex without workspaceId), match on email — + // disambiguated by providerSpecificData.username when present on both + // sides. Two different IdPs can share the same email address (e.g. a + // Google account and a HuggingFace account); matching on email alone + // would silently overwrite the other account's connection on the + // second login. Only fall back to the bare email-only match when + // neither side carries a username (legacy rows created before this + // disambiguation existed). + const incomingUsername = toStringOrNull(providerSpecificData.username); + const emailMatches = db + .prepare( + "SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'oauth' AND email = ?" + ) + .all(data.provider, data.email) as JsonRecord[]; existing = - (db - .prepare( - "SELECT * FROM provider_connections WHERE provider = ? AND auth_type = 'oauth' AND email = ?" - ) - .get(data.provider, data.email) as JsonRecord | undefined) || null; + emailMatches.find((row) => { + const existingUsername = toStringOrNull( + parseProviderSpecificData(row.provider_specific_data)?.username + ); + if (incomingUsername && existingUsername) { + return incomingUsername === existingUsername; + } + if (incomingUsername || existingUsername) return false; + return true; + }) || null; } } else if (data.authType === "apikey") { // Name-based upsert (existing behavior): same provider + same name → update. diff --git a/tests/unit/db-providers-cross-idp-dedup-2244.test.ts b/tests/unit/db-providers-cross-idp-dedup-2244.test.ts new file mode 100644 index 0000000000..fd74044cc4 --- /dev/null +++ b/tests/unit/db-providers-cross-idp-dedup-2244.test.ts @@ -0,0 +1,156 @@ +// Cross-IdP OAuth account dedup — createProviderConnection matched OAuth +// connections by email only, so two different IdPs that happen to share an +// email address (e.g. a Google account and a HuggingFace account) would +// silently overwrite each other on the second login. Disambiguate on +// providerSpecificData.username when BOTH the incoming and an existing +// connection carry one; fall back to the legacy email-only match when +// neither side has a username (backward compat for rows created before +// this fix). This test fails before the fix (case b overwrites instead of +// inserting a new row). +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"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cross-idp-dedup-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); + +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: unknown) { + 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("#2244 cross-IdP dedup: same email + same username updates the existing connection", async () => { + const first = await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "shared@example.com", + providerSpecificData: { username: "alice-google" }, + isActive: true, + }); + const second = await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "shared@example.com", + providerSpecificData: { username: "alice-google" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "glm" }); + assert.equal(conns.length, 1, "same email + same username must dedupe to a single connection"); + assert.equal((first as { id: string }).id, (second as { id: string }).id); +}); + +test("#2244 cross-IdP dedup: same email + DIFFERENT username creates a separate connection", async () => { + await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "shared@example.com", + providerSpecificData: { username: "alice-google" }, + isActive: true, + }); + await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "shared@example.com", + providerSpecificData: { username: "alice-huggingface" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "glm" }); + assert.equal( + conns.length, + 2, + "two different IdP identities sharing an email must NOT be collapsed into one connection" + ); + const usernames = conns + .map((c) => (c as { providerSpecificData?: { username?: string } }).providerSpecificData?.username) + .sort(); + assert.deepEqual(usernames, ["alice-google", "alice-huggingface"]); +}); + +test("#2244 cross-IdP dedup: legacy rows without username still dedupe against incoming without username", async () => { + const first = await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "legacy@example.com", + providerSpecificData: {}, + isActive: true, + }); + const second = await providersDb.createProviderConnection({ + provider: "glm", + authType: "oauth", + email: "legacy@example.com", + providerSpecificData: {}, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "glm" }); + assert.equal( + conns.length, + 1, + "legacy email-only rows without a username must keep deduping on email alone" + ); + assert.equal((first as { id: string }).id, (second as { id: string }).id); +}); + +test("#2244 cross-IdP dedup: Codex workspaceId matching path is unaffected", async () => { + const first = await providersDb.createProviderConnection({ + provider: "codex", + authType: "oauth", + email: "team@example.com", + providerSpecificData: { workspaceId: "ws-1", username: "team-user" }, + isActive: true, + }); + const second = await providersDb.createProviderConnection({ + provider: "codex", + authType: "oauth", + email: "team@example.com", + providerSpecificData: { workspaceId: "ws-1", username: "team-user-renamed" }, + isActive: true, + }); + const third = await providersDb.createProviderConnection({ + provider: "codex", + authType: "oauth", + email: "team@example.com", + providerSpecificData: { workspaceId: "ws-2", username: "team-user" }, + isActive: true, + }); + + const conns = await providersDb.getProviderConnections({ provider: "codex" }); + assert.equal( + conns.length, + 2, + "Codex must keep matching on workspaceId + email regardless of username, and a different workspace must stay a separate connection" + ); + assert.equal((first as { id: string }).id, (second as { id: string }).id); + assert.notEqual((first as { id: string }).id, (third as { id: string }).id); +});