From 469060f574a9f0dfb889a1222c9bdd1afb2ed619 Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 20:58:37 -0700 Subject: [PATCH 1/7] fix(web): reject OAuth account-linking when no signed-in session If a user clicks "Connect Bitbucket" and their session-token cookie is missing or expired by the time the BB redirect arrives at our callback, @auth/core silently falls through to createUser and mints a new orphan User row from the OAuth profile. The orphan has no email, no UserToOrg, and the user's session cookie gets rebound to it, leaving them on a "request access" page. Add a signIn callback that calls auth() and refuses the request when the provider's purpose is account_linking and no session is present. SSO providers and credentials login are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + packages/web/src/auth.ts | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5789f2bcc..f68c384ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [EE] Fixed issue where repo permissions could go stale when an upstream endpoint returned HTTP 410 Gone (e.g. Bitbucket Cloud's CHANGE-2770). [#1216](https://github.com/sourcebot-dev/sourcebot/pull/1216) - [EE] Fixed Bitbucket Cloud account-driven permission sync after Atlassian's CHANGE-2770 removed `GET /2.0/user/permissions/repositories`. [#1217](https://github.com/sourcebot-dev/sourcebot/pull/1217) - Fixed issue where session invalidation (signout, user deletion, removal from org) was not reflected by `/api/auth/session`. [#1219](https://github.com/sourcebot-dev/sourcebot/pull/1219) +- Fixed issue where an OAuth account-linking attempt without a valid signed-in session would silently create an orphan User row instead of rejecting the request. ## [4.17.2] - 2026-05-16 ### Added diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 3c89d1dcc..4b8612611 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -239,6 +239,44 @@ const nextAuthResult = NextAuth({ } }, callbacks: { + // Refuse OAuth signin for providers configured purely for account + // linking when no authenticated user is present on the request. + // + // Background: @auth/core's handleLoginOrRegister (callback/handle-login.js) + // reads the session token from the request and, if it can't decode it + // (e.g., the session cookie expired browser-side while the user was + // mid-OAuth-dance, or it never made it across the cross-site redirect), + // falls through to `createUser({ ...profile })` — silently spawning a + // new orphan User row from the OAuth profile. That's correct behavior + // for `purpose: "sso"` providers (an unauthenticated user logging in + // via SSO should become a new Sourcebot user). It's wrong for + // `purpose: "account_linking"` providers: by definition, those should + // only ever attach an upstream identity to an *existing* signed-in + // user, never mint a new Sourcebot user. + // + // Returning `false` here short-circuits the callback action with an + // `AccessDenied` before handleLoginOrRegister can run, redirecting + // the user to the error page instead of leaving them stranded as a + // new orphan identity with no UserToOrg row. + async signIn({ account }) { + if (!account || (account.type !== 'oauth' && account.type !== 'oidc')) { + return true; + } + + const matchingProvider = getProviders().find((p) => { + const providerId = typeof p.provider === 'function' + ? p.provider().id + : p.provider.id; + return providerId === account.provider; + }); + + if (matchingProvider?.purpose !== 'account_linking') { + return true; + } + + const session = await auth(); + return session !== null; + }, // Restrict post-auth redirects (sign-in / sign-out, `callbackUrl`, // `redirectTo`) to the same origin as the application. This mirrors // Auth.js's documented default; we set it explicitly so the protection From 5afc28912ccd467f23d669413060eb643d2b055e Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 20:59:53 -0700 Subject: [PATCH 2/7] chore: mark CHANGELOG entry [EE] and link to #1221 The fix is gated behind the sso entitlement (no OAuth identity providers are loaded in OSS deployments), so the [EE] prefix is appropriate per CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f68c384ea..2b5dc846b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [EE] Fixed issue where repo permissions could go stale when an upstream endpoint returned HTTP 410 Gone (e.g. Bitbucket Cloud's CHANGE-2770). [#1216](https://github.com/sourcebot-dev/sourcebot/pull/1216) - [EE] Fixed Bitbucket Cloud account-driven permission sync after Atlassian's CHANGE-2770 removed `GET /2.0/user/permissions/repositories`. [#1217](https://github.com/sourcebot-dev/sourcebot/pull/1217) - Fixed issue where session invalidation (signout, user deletion, removal from org) was not reflected by `/api/auth/session`. [#1219](https://github.com/sourcebot-dev/sourcebot/pull/1219) -- Fixed issue where an OAuth account-linking attempt without a valid signed-in session would silently create an orphan User row instead of rejecting the request. +- [EE] Fixed issue where an OAuth account-linking attempt without a valid signed-in session would silently create an orphan User row instead of rejecting the request. [#1221](https://github.com/sourcebot-dev/sourcebot/pull/1221) ## [4.17.2] - 2026-05-16 ### Added From 37833e35ccd0f85f8f5bc64ee47f5e0747d3f0df Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 21:02:58 -0700 Subject: [PATCH 3/7] chore: tighten wording in signIn callback comment Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/web/src/auth.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 4b8612611..8e5a7e180 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -244,9 +244,9 @@ const nextAuthResult = NextAuth({ // // Background: @auth/core's handleLoginOrRegister (callback/handle-login.js) // reads the session token from the request and, if it can't decode it - // (e.g., the session cookie expired browser-side while the user was - // mid-OAuth-dance, or it never made it across the cross-site redirect), - // falls through to `createUser({ ...profile })` — silently spawning a + // (e.g., the session cookie expired browser-side mid auth flow, or it + // never made it across the cross-site redirect), + // falls through to `createUser({ ...profile })`, silently spawning a // new orphan User row from the OAuth profile. That's correct behavior // for `purpose: "sso"` providers (an unauthenticated user logging in // via SSO should become a new Sourcebot user). It's wrong for From 6bc883f25a8e51c6c2af2d969d57a9ab3c4ce423 Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 21:12:18 -0700 Subject: [PATCH 4/7] clean up --- packages/web/src/auth.ts | 66 +++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 8e5a7e180..aa1993d55 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -239,43 +239,45 @@ const nextAuthResult = NextAuth({ } }, callbacks: { - // Refuse OAuth signin for providers configured purely for account - // linking when no authenticated user is present on the request. - // - // Background: @auth/core's handleLoginOrRegister (callback/handle-login.js) - // reads the session token from the request and, if it can't decode it - // (e.g., the session cookie expired browser-side mid auth flow, or it - // never made it across the cross-site redirect), - // falls through to `createUser({ ...profile })`, silently spawning a - // new orphan User row from the OAuth profile. That's correct behavior - // for `purpose: "sso"` providers (an unauthenticated user logging in - // via SSO should become a new Sourcebot user). It's wrong for - // `purpose: "account_linking"` providers: by definition, those should - // only ever attach an upstream identity to an *existing* signed-in - // user, never mint a new Sourcebot user. - // - // Returning `false` here short-circuits the callback action with an - // `AccessDenied` before handleLoginOrRegister can run, redirecting - // the user to the error page instead of leaving them stranded as a - // new orphan identity with no UserToOrg row. async signIn({ account }) { - if (!account || (account.type !== 'oauth' && account.type !== 'oidc')) { - return true; - } - - const matchingProvider = getProviders().find((p) => { + const matchingProvider = account + ? getProviders().find((p) => { const providerId = typeof p.provider === 'function' - ? p.provider().id - : p.provider.id; + ? p.provider().id + : p.provider.id; return providerId === account.provider; - }); - - if (matchingProvider?.purpose !== 'account_linking') { - return true; + }) + : undefined; + + const isAccountLinkingAttempt = + (account?.type === 'oauth' || account?.type === 'oidc') && + matchingProvider?.purpose === 'account_linking'; + + // Refuse OAuth signin for providers configured purely for account + // linking when no authenticated user is present on the request. + // + // Background: @auth/core's handleLoginOrRegister (callback/handle-login.js) + // reads the session token from the request and, if it can't decode it + // (e.g., the session cookie expired browser-side mid auth flow, or it + // never made it across the cross-site redirect), + // falls through to `createUser({ ...profile })`, silently spawning a + // new orphan User row from the OAuth profile. That's correct behavior + // for `purpose: "sso"` providers (an unauthenticated user logging in + // via SSO should become a new Sourcebot user). It's wrong for + // `purpose: "account_linking"` providers: by definition, those should + // only ever attach an upstream identity to an *existing* signed-in + // user, never mint a new Sourcebot user. + // + // Returning `false` here short-circuits the callback action with an + // `AccessDenied` before handleLoginOrRegister can run, redirecting + // the user to the error page instead of leaving them stranded as a + // new orphan identity with no UserToOrg row. + const session = await auth(); + if (isAccountLinkingAttempt && session === null) { + return false; } - const session = await auth(); - return session !== null; + return true; }, // Restrict post-auth redirects (sign-in / sign-out, `callbackUrl`, // `redirectTo`) to the same origin as the application. This mirrors From 18c3dabbd7fbaa23fe7d4ca4a2f4f422e6bc494a Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 21:13:02 -0700 Subject: [PATCH 5/7] formatting --- packages/web/src/auth.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index aa1993d55..616093ac8 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -241,18 +241,15 @@ const nextAuthResult = NextAuth({ callbacks: { async signIn({ account }) { const matchingProvider = account - ? getProviders().find((p) => { - const providerId = typeof p.provider === 'function' - ? p.provider().id - : p.provider.id; - return providerId === account.provider; - }) - : undefined; - - const isAccountLinkingAttempt = - (account?.type === 'oauth' || account?.type === 'oidc') && - matchingProvider?.purpose === 'account_linking'; - + ? getProviders().find((p) => { + const providerId = typeof p.provider === 'function' + ? p.provider().id + : p.provider.id; + return providerId === account.provider; + }) + : undefined; + + // Refuse OAuth signin for providers configured purely for account // linking when no authenticated user is present on the request. // @@ -272,6 +269,7 @@ const nextAuthResult = NextAuth({ // `AccessDenied` before handleLoginOrRegister can run, redirecting // the user to the error page instead of leaving them stranded as a // new orphan identity with no UserToOrg row. + const isAccountLinkingAttempt = matchingProvider?.purpose === 'account_linking'; const session = await auth(); if (isAccountLinkingAttempt && session === null) { return false; From 85af3fe0c21173137c69fb49d932294d7e8f3ecf Mon Sep 17 00:00:00 2001 From: msukkari Date: Fri, 22 May 2026 13:32:30 -0700 Subject: [PATCH 6/7] chore: remove signIn diagnostic logging --- packages/web/src/auth.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 616093ac8..3918a7310 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -242,14 +242,25 @@ const nextAuthResult = NextAuth({ async signIn({ account }) { const matchingProvider = account ? getProviders().find((p) => { - const providerId = typeof p.provider === 'function' - ? p.provider().id - : p.provider.id; + // NextAuth/Auth.js provider factories (e.g. Bitbucket, + // GitHub, GitLab) hardcode a default `id` at the top of + // the returned object and nest the caller's options + // (including any `id` override) under `.options`. At + // runtime the framework merges options over the + // top-level defaults, so the effective provider id can + // live under either field depending on whether the + // caller passed an override. Read `.options.id` first + // and fall back to the top-level `id`. + const config = ( + typeof p.provider === 'function' + ? (p.provider as unknown as () => unknown)() + : p.provider + ) as { id?: string; options?: { id?: string } }; + const providerId = config.options?.id ?? config.id; return providerId === account.provider; }) : undefined; - // Refuse OAuth signin for providers configured purely for account // linking when no authenticated user is present on the request. // @@ -271,6 +282,7 @@ const nextAuthResult = NextAuth({ // new orphan identity with no UserToOrg row. const isAccountLinkingAttempt = matchingProvider?.purpose === 'account_linking'; const session = await auth(); + if (isAccountLinkingAttempt && session === null) { return false; } From b73281f485bc7579944de27025223fbfdbb8cfda Mon Sep 17 00:00:00 2001 From: msukkari Date: Fri, 22 May 2026 13:51:32 -0700 Subject: [PATCH 7/7] refactor(web): extract getEffectiveProviderId helper Shared between signIn callback and getIssuerUrlForAccount. The latter previously read only the top-level provider id, which silently returned undefined for factory-based providers with an `id:` override (Bitbucket Cloud, GitHub, GitLab, Google, Okta, Keycloak, Entra, Authentik), meaning the stored issuerUrl was never resolved correctly for those accounts. --- packages/web/src/auth.ts | 51 +++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 3918a7310..56510b160 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -129,7 +129,7 @@ export const getProviders = () => { } else { if (!user.hashedPassword) { return null; - } + } if (!bcrypt.compareSync(password, user.hashedPassword)) { return null; @@ -241,24 +241,7 @@ const nextAuthResult = NextAuth({ callbacks: { async signIn({ account }) { const matchingProvider = account - ? getProviders().find((p) => { - // NextAuth/Auth.js provider factories (e.g. Bitbucket, - // GitHub, GitLab) hardcode a default `id` at the top of - // the returned object and nest the caller's options - // (including any `id` override) under `.options`. At - // runtime the framework merges options over the - // top-level defaults, so the effective provider id can - // live under either field depending on whether the - // caller passed an override. Read `.options.id` first - // and fall back to the top-level `id`. - const config = ( - typeof p.provider === 'function' - ? (p.provider as unknown as () => unknown)() - : p.provider - ) as { id?: string; options?: { id?: string } }; - const providerId = config.options?.id ?? config.id; - return providerId === account.provider; - }) + ? getProviders().find((p) => getEffectiveProviderId(p.provider) === account.provider) : undefined; // Refuse OAuth signin for providers configured purely for account @@ -282,7 +265,6 @@ const nextAuthResult = NextAuth({ // new orphan identity with no UserToOrg row. const isAccountLinkingAttempt = matchingProvider?.purpose === 'account_linking'; const session = await auth(); - if (isAccountLinkingAttempt && session === null) { return false; } @@ -417,18 +399,29 @@ export const auth = cache(async (): Promise => { return nextAuthResult.auth(); }); +// NextAuth/Auth.js provider factories (e.g. Bitbucket, GitHub, GitLab) hardcode +// a default `id` at the top of the returned object and nest the caller's +// options (including any `id` override) under `.options`. At runtime the +// framework merges options over the top-level defaults, so the effective +// provider id can live under either field depending on whether the caller +// passed an override. Read `.options.id` first and fall back to the top-level +// `id`. The function form of `Provider` is part of the NextAuth type union but +// unused in this codebase; we handle it for type completeness. +const getEffectiveProviderId = (provider: Provider): string | undefined => { + const config = ( + typeof provider === 'function' + ? (provider as unknown as () => unknown)() + : provider + ) as { id?: string; options?: { id?: string } }; + return config.options?.id ?? config.id; +} + /** * Returns the issuer URL for a given auth.js account */ const getIssuerUrlForAccount = async (account: { provider: string; }) => { - const providers = getProviders(); - const matchingProvider = providers.find((provider) => { - if (typeof provider.provider === "function") { - const providerInfo = provider.provider(); - return providerInfo.id === account.provider; - } else { - return provider.provider.id === account.provider; - } - }); + const matchingProvider = getProviders().find( + (p) => getEffectiveProviderId(p.provider) === account.provider + ); return matchingProvider?.issuerUrl; }