Skip to content

Commit ee2bbc4

Browse files
msukkariclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 8292882 commit ee2bbc4

4 files changed

Lines changed: 206 additions & 3 deletions

File tree

packages/web/src/auth.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import NextAuth, { DefaultSession, Session, User as AuthJsUser } from "next-auth
44
import Credentials from "next-auth/providers/credentials"
55
import EmailProvider from "next-auth/providers/nodemailer";
66
import { __unsafePrisma } from "@/prisma";
7-
import { env, getSMTPConnectionURL } from "@sourcebot/shared";
7+
import { createLogger, env, getSMTPConnectionURL } from "@sourcebot/shared";
88
import { User } from '@sourcebot/db';
99
import 'next-auth/jwt';
1010
import type { Provider } from "next-auth/providers";
@@ -15,11 +15,12 @@ import MagicLinkEmail from './emails/magicLinkEmail';
1515
import bcrypt from 'bcryptjs';
1616
import { getEEIdentityProviders } from '@/ee/features/sso/sso';
1717
import { hasEntitlement } from '@sourcebot/shared';
18-
import { onCreateUser } from '@/lib/authUtils';
18+
import { computeOAuthLinkConflictAudit, onCreateUser } from '@/lib/authUtils';
1919
import { getAuditService } from '@/ee/features/audit/factory';
2020
import { SINGLE_TENANT_ORG_ID } from './lib/constants';
2121
import { EncryptedPrismaAdapter, encryptAccountData } from '@/lib/encryptedPrismaAdapter';
2222

23+
const logger = createLogger('web-auth');
2324
const auditService = getAuditService();
2425
const eeIdentityProviders = hasEntitlement("sso") ? await getEEIdentityProviders() : [];
2526

@@ -239,6 +240,51 @@ const nextAuthResult = NextAuth({
239240
}
240241
},
241242
callbacks: {
243+
// Detect OAuth account-link conflicts before @auth/core throws
244+
// OAuthAccountNotLinked. When a currently signed-in user attempts to
245+
// sign in via an OAuth provider whose (provider, providerAccountId) is
246+
// already linked to a different Sourcebot user, emit an audit row so
247+
// identity-hijack attempts are recoverable from the audit log instead
248+
// of only the `[auth][error]` container logs.
249+
//
250+
// We return true unconditionally so @auth/core still raises
251+
// OAuthAccountNotLinked downstream, preserving the existing login-page
252+
// error UX.
253+
async signIn({ account }) {
254+
try {
255+
if (
256+
account?.provider &&
257+
account.providerAccountId &&
258+
(account.type === 'oauth' || account.type === 'oidc')
259+
) {
260+
const [existing, currentSession] = await Promise.all([
261+
__unsafePrisma.account.findUnique({
262+
where: {
263+
provider_providerAccountId: {
264+
provider: account.provider,
265+
providerAccountId: account.providerAccountId,
266+
},
267+
},
268+
select: { userId: true },
269+
}),
270+
nextAuthResult.auth(),
271+
]);
272+
273+
const auditEvent = computeOAuthLinkConflictAudit({
274+
account,
275+
existingLinkedUserId: existing?.userId ?? null,
276+
currentUserId: currentSession?.user?.id,
277+
});
278+
279+
if (auditEvent) {
280+
await auditService.createAudit(auditEvent);
281+
}
282+
}
283+
} catch (error) {
284+
logger.error(`Failed to audit OAuth account-link conflict: ${error}`);
285+
}
286+
return true;
287+
},
242288
// Restrict post-auth redirects (sign-in / sign-out, `callbackUrl`,
243289
// `redirectTo`) to the same origin as the application. This mirrors
244290
// Auth.js's documented default; we set it explicitly so the protection

packages/web/src/ee/features/audit/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export const auditMetadataSchema = z.object({
1818
api_key: z.string().optional(),
1919
emails: z.string().optional(), // comma separated list of emails
2020
source: z.string().optional(), // request source (e.g., 'mcp') from X-Sourcebot-Client-Source header
21+
provider: z.string().optional(), // OAuth provider id (e.g., 'github')
22+
providerAccountId: z.string().optional(), // upstream identity id within the provider
2123
})
2224
export type AuditMetadata = z.infer<typeof auditMetadataSchema>;
2325

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { describe, expect, test, vi } from "vitest";
2+
3+
vi.mock("server-only", () => ({ default: vi.fn() }));
4+
vi.mock("@/prisma", async () => {
5+
const actual = await vi.importActual<typeof import("@/__mocks__/prisma")>("@/__mocks__/prisma");
6+
return { ...actual };
7+
});
8+
9+
import { computeOAuthLinkConflictAudit } from "./authUtils";
10+
import { SINGLE_TENANT_ORG_ID } from "@/lib/constants";
11+
12+
describe("computeOAuthLinkConflictAudit", () => {
13+
const oauthAccount = {
14+
provider: "github",
15+
providerAccountId: "upstream-123",
16+
type: "oauth" as const,
17+
};
18+
19+
test("returns audit event when upstream identity is linked to a different user than the one currently signed in", () => {
20+
const result = computeOAuthLinkConflictAudit({
21+
account: oauthAccount,
22+
existingLinkedUserId: "user-victim",
23+
currentUserId: "user-attacker",
24+
});
25+
26+
expect(result).toEqual({
27+
action: "account.link_failed_already_linked",
28+
actor: { id: "user-attacker", type: "user" },
29+
target: { id: "user-victim", type: "user" },
30+
orgId: SINGLE_TENANT_ORG_ID,
31+
metadata: {
32+
provider: "github",
33+
providerAccountId: "upstream-123",
34+
},
35+
});
36+
});
37+
38+
test("returns audit event for OIDC providers too", () => {
39+
const result = computeOAuthLinkConflictAudit({
40+
account: { ...oauthAccount, type: "oidc" },
41+
existingLinkedUserId: "user-victim",
42+
currentUserId: "user-attacker",
43+
});
44+
45+
expect(result).not.toBeNull();
46+
expect(result?.action).toBe("account.link_failed_already_linked");
47+
});
48+
49+
test("returns null when the upstream identity is linked to the same user that is signed in (re-authentication)", () => {
50+
const result = computeOAuthLinkConflictAudit({
51+
account: oauthAccount,
52+
existingLinkedUserId: "user-same",
53+
currentUserId: "user-same",
54+
});
55+
56+
expect(result).toBeNull();
57+
});
58+
59+
test("returns null when there is no signed-in user (fresh OAuth login that NextAuth will resolve to the existing user)", () => {
60+
const result = computeOAuthLinkConflictAudit({
61+
account: oauthAccount,
62+
existingLinkedUserId: "user-victim",
63+
currentUserId: undefined,
64+
});
65+
66+
expect(result).toBeNull();
67+
});
68+
69+
test("returns null when the upstream identity is not yet linked to any user", () => {
70+
const result = computeOAuthLinkConflictAudit({
71+
account: oauthAccount,
72+
existingLinkedUserId: null,
73+
currentUserId: "user-attacker",
74+
});
75+
76+
expect(result).toBeNull();
77+
});
78+
79+
test("returns null for non-OAuth providers (credentials, email, webauthn)", () => {
80+
for (const type of ["credentials", "email", "webauthn"] as const) {
81+
const result = computeOAuthLinkConflictAudit({
82+
account: { ...oauthAccount, type },
83+
existingLinkedUserId: "user-victim",
84+
currentUserId: "user-attacker",
85+
});
86+
expect(result, `type=${type}`).toBeNull();
87+
}
88+
});
89+
90+
test("returns null when account is missing required fields", () => {
91+
expect(
92+
computeOAuthLinkConflictAudit({
93+
account: null,
94+
existingLinkedUserId: "user-victim",
95+
currentUserId: "user-attacker",
96+
})
97+
).toBeNull();
98+
99+
expect(
100+
computeOAuthLinkConflictAudit({
101+
account: { provider: "", providerAccountId: "id", type: "oauth" },
102+
existingLinkedUserId: "user-victim",
103+
currentUserId: "user-attacker",
104+
})
105+
).toBeNull();
106+
107+
expect(
108+
computeOAuthLinkConflictAudit({
109+
account: { provider: "github", providerAccountId: "", type: "oauth" },
110+
existingLinkedUserId: "user-victim",
111+
currentUserId: "user-attacker",
112+
})
113+
).toBeNull();
114+
});
115+
});

packages/web/src/lib/authUtils.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { User as AuthJsUser } from "next-auth";
1+
import type { Account as AuthJsAccount, User as AuthJsUser } from "next-auth";
22
import { __unsafePrisma } from "@/prisma";
33
import { OrgRole } from "@sourcebot/db";
44
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";
77
import { orgNotFound, ServiceError, userNotFound } from "@/lib/serviceError";
88
import { createLogger } from "@sourcebot/shared";
99
import { getAuditService } from "@/ee/features/audit/factory";
10+
import type { AuditEvent } from "@/ee/features/audit/types";
1011
import { StatusCodes } from "http-status-codes";
1112
import { ErrorCode } from "./errorCodes";
1213

@@ -240,6 +241,45 @@ export const orgHasAvailability = async (): Promise<boolean> => {
240241
return true;
241242
}
242243

244+
/**
245+
* Decides whether an OAuth account-link attempt should be audited as a
246+
* conflict (the upstream identity is already linked to a different Sourcebot
247+
* user). Pure for testability — the caller is responsible for fetching the
248+
* existing Account row and the current session.
249+
*/
250+
export const computeOAuthLinkConflictAudit = ({
251+
account,
252+
existingLinkedUserId,
253+
currentUserId,
254+
}: {
255+
account: Pick<AuthJsAccount, "provider" | "providerAccountId" | "type"> | null | undefined;
256+
existingLinkedUserId: string | null;
257+
currentUserId: string | undefined;
258+
}): Omit<AuditEvent, "sourcebotVersion"> | null => {
259+
if (!account?.provider || !account.providerAccountId) {
260+
return null;
261+
}
262+
if (account.type !== "oauth" && account.type !== "oidc") {
263+
return null;
264+
}
265+
if (!existingLinkedUserId || !currentUserId) {
266+
return null;
267+
}
268+
if (existingLinkedUserId === currentUserId) {
269+
return null;
270+
}
271+
return {
272+
action: "account.link_failed_already_linked",
273+
actor: { id: currentUserId, type: "user" },
274+
target: { id: existingLinkedUserId, type: "user" },
275+
orgId: SINGLE_TENANT_ORG_ID,
276+
metadata: {
277+
provider: account.provider,
278+
providerAccountId: account.providerAccountId,
279+
},
280+
};
281+
};
282+
243283
export const addUserToOrganization = async (userId: string, orgId: number): Promise<{ success: boolean } | ServiceError> => {
244284
const user = await __unsafePrisma.user.findUnique({
245285
where: {

0 commit comments

Comments
 (0)