fix(oauth): disambiguate OAuth connections on username to prevent cross-IdP overwrites (#5803)

Integrated into release/v3.8.43
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-01 21:55:30 -03:00
committed by GitHub
parent e7ae29d607
commit b55a330dc5
3 changed files with 182 additions and 6 deletions

View File

@@ -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))

View File

@@ -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.

View File

@@ -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);
});