Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 48 additions & 2 deletions packages/web/src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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() : [];

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions packages/web/src/ee/features/audit/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof auditMetadataSchema>;

Expand Down
115 changes: 115 additions & 0 deletions packages/web/src/lib/authUtils.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import("@/__mocks__/prisma")>("@/__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();
});
});
42 changes: 41 additions & 1 deletion packages/web/src/lib/authUtils.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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";

Expand Down Expand Up @@ -240,6 +241,45 @@ export const orgHasAvailability = async (): Promise<boolean> => {
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<AuthJsAccount, "provider" | "providerAccountId" | "type"> | null | undefined;
existingLinkedUserId: string | null;
currentUserId: string | undefined;
}): Omit<AuditEvent, "sourcebotVersion"> | 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: {
Expand Down
Loading