From f17d23bf0bb3a04bc2dcab502c625f59e6035e62 Mon Sep 17 00:00:00 2001 From: Innokentiy Solntsev Date: Wed, 22 Jul 2026 22:37:54 +0200 Subject: [PATCH] fix(oauth): honor connectionId on token refresh so email-less providers don't duplicate (#8062) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit persistOAuthConnection gated its whole dedup step behind if(tokenData.email). The matcher (findExistingOAuthConnectionMatch) already matches by explicit connectionId first, but it was never reached when the payload had no top-level email. GitHub Copilot's device-code flow keeps identity under providerSpecificData.githubEmail, so tokenData.email is undefined — a refresh (which passes the existing connectionId) skipped the match and fell through to createProviderConnection, producing a duplicate connection. - Widen the gate to if(connectionId || tokenData.email) so an explicit connectionId is honored regardless of email. - Guard the matcher's email branch with if(!tokenData.email) return false, so a widened gate can't false-match an email-less connection via safeEqual(undefined, undefined). Fixes #8059. --- .../8059-oauth-refresh-connection-dedup.md | 1 + src/lib/oauth/connectionPersistence.ts | 11 ++- ...auth-refresh-connection-dedup-8059.test.ts | 90 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/8059-oauth-refresh-connection-dedup.md create mode 100644 tests/unit/oauth-refresh-connection-dedup-8059.test.ts diff --git a/changelog.d/fixes/8059-oauth-refresh-connection-dedup.md b/changelog.d/fixes/8059-oauth-refresh-connection-dedup.md new file mode 100644 index 0000000000..29e077184e --- /dev/null +++ b/changelog.d/fixes/8059-oauth-refresh-connection-dedup.md @@ -0,0 +1 @@ +- **fix(oauth):** Refreshing the token on an email-less OAuth connection (e.g. GitHub Copilot, whose identity lives under `providerSpecificData`) no longer creates a duplicate connection. `persistOAuthConnection` gated its whole dedup behind `if (tokenData.email)`, so the explicit `connectionId` passed on refresh was discarded and a new connection was created every time; the connectionId is now honored regardless of email, and email dedup only applies when a non-empty email is present. ([#8059](https://github.com/diegosouzapw/OmniRoute/issues/8059)) diff --git a/src/lib/oauth/connectionPersistence.ts b/src/lib/oauth/connectionPersistence.ts index da64863787..4b4ad753fc 100644 --- a/src/lib/oauth/connectionPersistence.ts +++ b/src/lib/oauth/connectionPersistence.ts @@ -68,6 +68,10 @@ export function findExistingOAuthConnectionMatch( ): Record | undefined { return existing.find((c) => { if (c.id && safeEqual(connectionId, c.id)) return true; + // Email dedup only when the payload actually carries an email. Without this + // guard `safeEqual(undefined, undefined)` is true, so an email-less payload + // would false-match the first email-less connection of the provider. + if (!tokenData.email) return false; if (!safeEqual(c.email, tokenData.email) || c.authType !== "oauth") return false; if (provider === "codex") { return isSameCodexAccount(c.providerSpecificData, tokenData.providerSpecificData); @@ -130,7 +134,12 @@ export async function persistOAuthConnection( : null; let connection: any; - if (tokenData.email) { + // A connectionId is an explicit "update THIS connection" signal (token refresh + // / re-auth of a known connection); honor it even when the payload has no + // top-level email. Some providers (e.g. GitHub Copilot) keep identity under + // providerSpecificData, so gating dedup on tokenData.email alone created a + // duplicate connection on every refresh (#8059). + if (connectionId || tokenData.email) { const existing = await getProviderConnections({ provider }); const match = findExistingOAuthConnectionMatch(existing, provider, tokenData, connectionId); const matchId = typeof match?.id === "string" ? match.id : null; diff --git a/tests/unit/oauth-refresh-connection-dedup-8059.test.ts b/tests/unit/oauth-refresh-connection-dedup-8059.test.ts new file mode 100644 index 0000000000..3094985f31 --- /dev/null +++ b/tests/unit/oauth-refresh-connection-dedup-8059.test.ts @@ -0,0 +1,90 @@ +/** + * #8059 — token refresh must not duplicate an email-less OAuth connection. + * + * persistOAuthConnection gated its whole dedup step behind `if (tokenData.email)`. + * GitHub Copilot's device-code flow keeps identity under + * providerSpecificData.githubEmail, so the top-level `tokenData.email` is + * undefined — a refresh (which passes the existing connectionId) skipped the + * match entirely and fell through to createProviderConnection, producing a + * duplicate. The fix honors an explicit connectionId regardless of email, and + * only applies email dedup when a non-empty email is present. + */ +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-oauth-refresh-dedup-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "oauth-refresh-dedup-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const { persistOAuthConnection, findExistingOAuthConnectionMatch } = await import( + "../../src/lib/oauth/connectionPersistence.ts" +); + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(resetStorage); +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +async function oauthConns(provider: string) { + const all = await providersDb.getProviderConnections({}); + return all.filter((c: any) => c.provider === provider && c.authType === "oauth"); +} + +// GitHub Copilot: no top-level email, identity under providerSpecificData. +const copilotToken = (refresh: string) => ({ + refreshToken: refresh, + providerSpecificData: { + copilotToken: `cop_${refresh}`, + githubLogin: "octocat", + githubName: "The Octocat", + githubEmail: "octocat@github.com", + }, +}); + +test("token refresh with matching connectionId updates the connection, no duplicate (#8059)", async () => { + const created = await persistOAuthConnection("github", copilotToken("r1")); + assert.equal((await oauthConns("github")).length, 1, "initial connect creates one connection"); + + // Refresh: same connection, new token, still no top-level email. + const refreshed = await persistOAuthConnection("github", copilotToken("r2"), created.id); + + const conns = await oauthConns("github"); + assert.equal(conns.length, 1, "refresh must not create a duplicate"); + assert.equal(refreshed.id, created.id, "refresh updates the existing connection"); + assert.equal(conns[0].refreshToken, "r2", "the token was actually updated"); +}); + +test("findExistingOAuthConnectionMatch: connectionId wins even without an email", () => { + const existing = [ + { id: "conn-1", provider: "github", authType: "oauth", email: null }, + { id: "conn-2", provider: "github", authType: "oauth", email: null }, + ]; + const match = findExistingOAuthConnectionMatch(existing, "github", copilotToken("x"), "conn-2"); + assert.equal(match?.id, "conn-2"); +}); + +test("findExistingOAuthConnectionMatch: an email-less payload does not false-match an email-less connection", () => { + const existing = [{ id: "conn-1", provider: "github", authType: "oauth", email: null }]; + // No connectionId, no top-level email -> must not match (would clobber a + // different account whose email is likewise absent). + const match = findExistingOAuthConnectionMatch(existing, "github", copilotToken("x")); + assert.equal(match, undefined); +}); + +test("findExistingOAuthConnectionMatch: email dedup still works when an email is present", () => { + const existing = [{ id: "conn-1", provider: "claude", authType: "oauth", email: "a@b.com" }]; + const match = findExistingOAuthConnectionMatch(existing, "claude", { email: "a@b.com" }); + assert.equal(match?.id, "conn-1"); +});