diff --git a/CHANGELOG.md b/CHANGELOG.md index 5789f2bcc..2b5dc846b 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) +- [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 diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 3c89d1dcc..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; @@ -239,6 +239,38 @@ const nextAuthResult = NextAuth({ } }, callbacks: { + async signIn({ account }) { + const matchingProvider = account + ? getProviders().find((p) => getEffectiveProviderId(p.provider) === account.provider) + : undefined; + + // 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 isAccountLinkingAttempt = matchingProvider?.purpose === 'account_linking'; + const session = await auth(); + if (isAccountLinkingAttempt && session === null) { + return false; + } + + return true; + }, // 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 @@ -367,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; }