From ee2bbc40b2c3c41ead75e2d58d0f3bac1368ce78 Mon Sep 17 00:00:00 2001 From: msukkari Date: Fri, 22 May 2026 12:35:21 -0700 Subject: [PATCH 1/2] feat(web): audit OAuth account-link conflicts When NextAuth would throw OAuthAccountNotLinked because a signed-in user attempts to add an OAuth identity that is already linked to a different Sourcebot user, emit an audit row with action `account.link_failed_already_linked` so the rejection is recoverable from the audit log instead of only the `[auth][error]` container logs. Fixes SOU-1184 Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/web/src/auth.ts | 50 ++++++++- packages/web/src/ee/features/audit/types.ts | 2 + packages/web/src/lib/authUtils.test.ts | 115 ++++++++++++++++++++ packages/web/src/lib/authUtils.ts | 42 ++++++- 4 files changed, 206 insertions(+), 3 deletions(-) create mode 100644 packages/web/src/lib/authUtils.test.ts diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 3c89d1dcc..bb1bafea0 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -4,7 +4,7 @@ import NextAuth, { DefaultSession, Session, User as AuthJsUser } from "next-auth import Credentials from "next-auth/providers/credentials" import EmailProvider from "next-auth/providers/nodemailer"; import { __unsafePrisma } from "@/prisma"; -import { env, getSMTPConnectionURL } from "@sourcebot/shared"; +import { createLogger, env, getSMTPConnectionURL } from "@sourcebot/shared"; import { User } from '@sourcebot/db'; import 'next-auth/jwt'; import type { Provider } from "next-auth/providers"; @@ -15,11 +15,12 @@ import MagicLinkEmail from './emails/magicLinkEmail'; import bcrypt from 'bcryptjs'; import { getEEIdentityProviders } from '@/ee/features/sso/sso'; import { hasEntitlement } from '@sourcebot/shared'; -import { onCreateUser } from '@/lib/authUtils'; +import { computeOAuthLinkConflictAudit, onCreateUser } from '@/lib/authUtils'; import { getAuditService } from '@/ee/features/audit/factory'; import { SINGLE_TENANT_ORG_ID } from './lib/constants'; import { EncryptedPrismaAdapter, encryptAccountData } from '@/lib/encryptedPrismaAdapter'; +const logger = createLogger('web-auth'); const auditService = getAuditService(); const eeIdentityProviders = hasEntitlement("sso") ? await getEEIdentityProviders() : []; @@ -239,6 +240,51 @@ const nextAuthResult = NextAuth({ } }, callbacks: { + // Detect OAuth account-link conflicts before @auth/core throws + // OAuthAccountNotLinked. When a currently signed-in user attempts to + // sign in via an OAuth provider whose (provider, providerAccountId) is + // already linked to a different Sourcebot user, emit an audit row so + // identity-hijack attempts are recoverable from the audit log instead + // of only the `[auth][error]` container logs. + // + // We return true unconditionally so @auth/core still raises + // OAuthAccountNotLinked downstream, preserving the existing login-page + // error UX. + async signIn({ account }) { + try { + if ( + account?.provider && + account.providerAccountId && + (account.type === 'oauth' || account.type === 'oidc') + ) { + const [existing, currentSession] = await Promise.all([ + __unsafePrisma.account.findUnique({ + where: { + provider_providerAccountId: { + provider: account.provider, + providerAccountId: account.providerAccountId, + }, + }, + select: { userId: true }, + }), + nextAuthResult.auth(), + ]); + + const auditEvent = computeOAuthLinkConflictAudit({ + account, + existingLinkedUserId: existing?.userId ?? null, + currentUserId: currentSession?.user?.id, + }); + + if (auditEvent) { + await auditService.createAudit(auditEvent); + } + } + } catch (error) { + logger.error(`Failed to audit OAuth account-link conflict: ${error}`); + } + 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 diff --git a/packages/web/src/ee/features/audit/types.ts b/packages/web/src/ee/features/audit/types.ts index e79b6957f..3c6ea4b20 100644 --- a/packages/web/src/ee/features/audit/types.ts +++ b/packages/web/src/ee/features/audit/types.ts @@ -18,6 +18,8 @@ export const auditMetadataSchema = z.object({ api_key: z.string().optional(), emails: z.string().optional(), // comma separated list of emails source: z.string().optional(), // request source (e.g., 'mcp') from X-Sourcebot-Client-Source header + provider: z.string().optional(), // OAuth provider id (e.g., 'github') + providerAccountId: z.string().optional(), // upstream identity id within the provider }) export type AuditMetadata = z.infer; diff --git a/packages/web/src/lib/authUtils.test.ts b/packages/web/src/lib/authUtils.test.ts new file mode 100644 index 000000000..fd6303767 --- /dev/null +++ b/packages/web/src/lib/authUtils.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, test, vi } from "vitest"; + +vi.mock("server-only", () => ({ default: vi.fn() })); +vi.mock("@/prisma", async () => { + const actual = await vi.importActual("@/__mocks__/prisma"); + return { ...actual }; +}); + +import { computeOAuthLinkConflictAudit } from "./authUtils"; +import { SINGLE_TENANT_ORG_ID } from "@/lib/constants"; + +describe("computeOAuthLinkConflictAudit", () => { + const oauthAccount = { + provider: "github", + providerAccountId: "upstream-123", + type: "oauth" as const, + }; + + test("returns audit event when upstream identity is linked to a different user than the one currently signed in", () => { + const result = computeOAuthLinkConflictAudit({ + account: oauthAccount, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }); + + expect(result).toEqual({ + action: "account.link_failed_already_linked", + actor: { id: "user-attacker", type: "user" }, + target: { id: "user-victim", type: "user" }, + orgId: SINGLE_TENANT_ORG_ID, + metadata: { + provider: "github", + providerAccountId: "upstream-123", + }, + }); + }); + + test("returns audit event for OIDC providers too", () => { + const result = computeOAuthLinkConflictAudit({ + account: { ...oauthAccount, type: "oidc" }, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }); + + expect(result).not.toBeNull(); + expect(result?.action).toBe("account.link_failed_already_linked"); + }); + + test("returns null when the upstream identity is linked to the same user that is signed in (re-authentication)", () => { + const result = computeOAuthLinkConflictAudit({ + account: oauthAccount, + existingLinkedUserId: "user-same", + currentUserId: "user-same", + }); + + expect(result).toBeNull(); + }); + + test("returns null when there is no signed-in user (fresh OAuth login that NextAuth will resolve to the existing user)", () => { + const result = computeOAuthLinkConflictAudit({ + account: oauthAccount, + existingLinkedUserId: "user-victim", + currentUserId: undefined, + }); + + expect(result).toBeNull(); + }); + + test("returns null when the upstream identity is not yet linked to any user", () => { + const result = computeOAuthLinkConflictAudit({ + account: oauthAccount, + existingLinkedUserId: null, + currentUserId: "user-attacker", + }); + + expect(result).toBeNull(); + }); + + test("returns null for non-OAuth providers (credentials, email, webauthn)", () => { + for (const type of ["credentials", "email", "webauthn"] as const) { + const result = computeOAuthLinkConflictAudit({ + account: { ...oauthAccount, type }, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }); + expect(result, `type=${type}`).toBeNull(); + } + }); + + test("returns null when account is missing required fields", () => { + expect( + computeOAuthLinkConflictAudit({ + account: null, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }) + ).toBeNull(); + + expect( + computeOAuthLinkConflictAudit({ + account: { provider: "", providerAccountId: "id", type: "oauth" }, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }) + ).toBeNull(); + + expect( + computeOAuthLinkConflictAudit({ + account: { provider: "github", providerAccountId: "", type: "oauth" }, + existingLinkedUserId: "user-victim", + currentUserId: "user-attacker", + }) + ).toBeNull(); + }); +}); diff --git a/packages/web/src/lib/authUtils.ts b/packages/web/src/lib/authUtils.ts index d3c0c9341..b744704fa 100644 --- a/packages/web/src/lib/authUtils.ts +++ b/packages/web/src/lib/authUtils.ts @@ -1,4 +1,4 @@ -import type { User as AuthJsUser } from "next-auth"; +import type { Account as AuthJsAccount, User as AuthJsUser } from "next-auth"; import { __unsafePrisma } from "@/prisma"; import { OrgRole } from "@sourcebot/db"; import { SINGLE_TENANT_ORG_ID, SOURCEBOT_GUEST_USER_EMAIL, SOURCEBOT_GUEST_USER_ID, SOURCEBOT_SUPPORT_EMAIL } from "@/lib/constants"; @@ -7,6 +7,7 @@ import { isServiceError } from "@/lib/utils"; import { orgNotFound, ServiceError, userNotFound } from "@/lib/serviceError"; import { createLogger } from "@sourcebot/shared"; import { getAuditService } from "@/ee/features/audit/factory"; +import type { AuditEvent } from "@/ee/features/audit/types"; import { StatusCodes } from "http-status-codes"; import { ErrorCode } from "./errorCodes"; @@ -240,6 +241,45 @@ export const orgHasAvailability = async (): Promise => { return true; } +/** + * Decides whether an OAuth account-link attempt should be audited as a + * conflict (the upstream identity is already linked to a different Sourcebot + * user). Pure for testability — the caller is responsible for fetching the + * existing Account row and the current session. + */ +export const computeOAuthLinkConflictAudit = ({ + account, + existingLinkedUserId, + currentUserId, +}: { + account: Pick | null | undefined; + existingLinkedUserId: string | null; + currentUserId: string | undefined; +}): Omit | null => { + if (!account?.provider || !account.providerAccountId) { + return null; + } + if (account.type !== "oauth" && account.type !== "oidc") { + return null; + } + if (!existingLinkedUserId || !currentUserId) { + return null; + } + if (existingLinkedUserId === currentUserId) { + return null; + } + return { + action: "account.link_failed_already_linked", + actor: { id: currentUserId, type: "user" }, + target: { id: existingLinkedUserId, type: "user" }, + orgId: SINGLE_TENANT_ORG_ID, + metadata: { + provider: account.provider, + providerAccountId: account.providerAccountId, + }, + }; +}; + export const addUserToOrganization = async (userId: string, orgId: number): Promise<{ success: boolean } | ServiceError> => { const user = await __unsafePrisma.user.findUnique({ where: { From b44830e3e3d16438c35fbe63d647b6836cd5b68e Mon Sep 17 00:00:00 2001 From: msukkari Date: Fri, 22 May 2026 12:36:00 -0700 Subject: [PATCH 2/2] docs: add CHANGELOG entry for #1223 Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5789f2bcc..b59f8eb9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- [EE] Added `account.link_failed_already_linked` audit action emitted when an OAuth account-link attempt is rejected because the upstream identity is already linked to a different Sourcebot user. [#1223](https://github.com/sourcebot-dev/sourcebot/pull/1223) + ### Fixed - Fixed issue where repo permissions could go stale when authentication or token refresh related errors occured. [#1215](https://github.com/sourcebot-dev/sourcebot/pull/1215) - [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)