Skip to content

Commit d739b58

Browse files
brendan-kellamgithub-actions[bot]
brendan-kellam
and
github-actions[bot]
authored
feat(web): associate Sentry errors with users (#1452)
* feat(web): associate Sentry errors with users * refactor(web): set Sentry user in withAuth path instead of auth() Move setSentryUser out of the memoized auth() resolver and into getAuthContext so it captures the resolved principal for all auth sources (session, OAuth Bearer, API key) rather than clearing the identity on API-key / Bearer requests where auth() returns null. Co-authored-by: Brendan Kellam <10233483+brendan-kellam@users.noreply.github.com> * nit * fix(web): associate Sentry errors with resolved users --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Brendan Kellam <10233483+brendan-kellam@users.noreply.github.com>
1 parent 9371f1d commit d739b58

8 files changed

Lines changed: 208 additions & 4 deletions

File tree

‎packages/web/src/app/layout.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { PlanProvider } from "@/features/entitlements/planProvider";
2121
import { getEntitlements } from "@/lib/entitlements";
2222
import { IdentityProvidersProvider } from "@/features/auth/identityProvidersProvider";
2323
import { getIdentityProviderMetadata } from "@/lib/identityProviders";
24+
import { SentryUserProvider } from "./sentryUserProvider";
2425

2526
export const metadata: Metadata = {
2627
metadataBase: env.AUTH_URL ? new URL(env.AUTH_URL) : undefined,
@@ -74,6 +75,9 @@ export default async function RootLayout({
7475
<body>
7576
<Toaster />
7677
<SessionProvider>
78+
<SentryUserProvider
79+
isPiiEnabled={env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true'}
80+
/>
7781
<PlanProvider entitlements={entitlements}>
7882
<IdentityProvidersProvider providers={identityProviders}>
7983
<PostHogProvider
@@ -106,4 +110,4 @@ export default async function RootLayout({
106110
</body>
107111
</html>
108112
);
109-
}
113+
}

‎packages/web/src/app/sentryUserProvider.test.tsx‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { cleanup, render } from '@testing-library/react';
2+
import { afterEach, describe, expect, test, vi } from 'vitest';
3+
4+
const mocks = vi.hoisted(() => ({
5+
setSentryUser: vi.fn(),
6+
useSession: vi.fn(),
7+
}));
8+
9+
vi.mock('@/lib/sentryUser', () => ({
10+
setSentryUser: mocks.setSentryUser,
11+
}));
12+
13+
vi.mock('next-auth/react', () => ({
14+
useSession: mocks.useSession,
15+
}));
16+
17+
const { SentryUserProvider } = await import('./sentryUserProvider');
18+
19+
afterEach(() => {
20+
cleanup();
21+
vi.clearAllMocks();
22+
});
23+
24+
describe('SentryUserProvider', () => {
25+
test('waits for the session before changing the Sentry user', () => {
26+
mocks.useSession.mockReturnValue({ data: undefined, status: 'loading' });
27+
28+
render(<SentryUserProvider isPiiEnabled={false} />);
29+
30+
expect(mocks.setSentryUser).not.toHaveBeenCalled();
31+
});
32+
33+
test('sets and clears the Sentry user as authentication changes', () => {
34+
const user = {
35+
id: 'user-1',
36+
email: 'user@example.com',
37+
name: 'Example User',
38+
};
39+
mocks.useSession.mockReturnValue({
40+
data: { user },
41+
status: 'authenticated',
42+
});
43+
44+
const { rerender } = render(<SentryUserProvider isPiiEnabled={true} />);
45+
expect(mocks.setSentryUser).toHaveBeenLastCalledWith(user, true);
46+
47+
mocks.useSession.mockReturnValue({ data: null, status: 'unauthenticated' });
48+
rerender(<SentryUserProvider isPiiEnabled={true} />);
49+
50+
expect(mocks.setSentryUser).toHaveBeenLastCalledWith(null, true);
51+
});
52+
});

‎packages/web/src/app/sentryUserProvider.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use client';
2+
3+
import { setSentryUser } from '@/lib/sentryUser';
4+
import { useSession } from 'next-auth/react';
5+
import { useEffect } from 'react';
6+
7+
interface SentryUserProviderProps {
8+
isPiiEnabled: boolean;
9+
}
10+
11+
export function SentryUserProvider({ isPiiEnabled }: SentryUserProviderProps) {
12+
const { data: session, status } = useSession();
13+
14+
useEffect(() => {
15+
if (status === 'loading') {
16+
return;
17+
}
18+
19+
setSentryUser(session?.user ?? null, isPiiEnabled);
20+
}, [isPiiEnabled, session, status]);
21+
22+
return null;
23+
}

‎packages/web/src/auth.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { getAnonymousId } from '@/lib/anonymousId';
2222
import { captureEvent } from '@/lib/posthog';
2323
import { isEmailCodeLoginEnabled, isCredentialsLoginEnabled } from '@sourcebot/shared'
2424
import { onCreateUser } from './features/membership/onCreateUser';
25+
import { setSentryUser } from './lib/sentryUser';
2526

2627
export const runtime = 'nodejs';
2728

@@ -452,7 +453,12 @@ export const { handlers, signIn, signOut } = nextAuthResult;
452453
* without re-running the upstream resolver.
453454
*/
454455
export const auth = cache(async (): Promise<Session | null> => {
455-
return nextAuthResult.auth();
456+
const session = await nextAuthResult.auth();
457+
setSentryUser(
458+
session?.user ?? null,
459+
env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true',
460+
);
461+
return session;
456462
});
457463

458464
/**

‎packages/web/src/lib/sentryUser.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { beforeEach, describe, expect, test, vi } from 'vitest';
2+
3+
const setUser = vi.fn();
4+
5+
vi.mock('@sentry/nextjs', () => ({
6+
setUser,
7+
}));
8+
9+
const { setSentryUser } = await import('./sentryUser');
10+
11+
describe('setSentryUser', () => {
12+
beforeEach(() => {
13+
setUser.mockClear();
14+
});
15+
16+
test('associates errors with the user id without PII by default', () => {
17+
setSentryUser({
18+
id: 'user-1',
19+
email: 'user@example.com',
20+
name: 'Example User',
21+
}, false);
22+
23+
expect(setUser).toHaveBeenCalledWith({ id: 'user-1' });
24+
});
25+
26+
test('includes user details when PII collection is enabled', () => {
27+
setSentryUser({
28+
id: 'user-1',
29+
email: 'user@example.com',
30+
name: 'Example User',
31+
}, true);
32+
33+
expect(setUser).toHaveBeenCalledWith({
34+
id: 'user-1',
35+
email: 'user@example.com',
36+
username: 'Example User',
37+
});
38+
});
39+
40+
test('clears the user for unauthenticated requests', () => {
41+
setSentryUser(null, true);
42+
43+
expect(setUser).toHaveBeenCalledWith(null);
44+
});
45+
});

‎packages/web/src/lib/sentryUser.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as Sentry from "@sentry/nextjs";
2+
3+
type SentryUser = {
4+
id: string;
5+
email?: string | null;
6+
name?: string | null;
7+
};
8+
9+
/**
10+
* Associates subsequent Sentry events with the current user, including their
11+
* email and name only when PII collection is enabled. If Sentry is not
12+
* configured, this is effectively a no-op and does not send any data.
13+
*/
14+
export function setSentryUser(user: SentryUser | null, isPiiEnabled: boolean) {
15+
if (!user) {
16+
Sentry.setUser(null);
17+
return;
18+
}
19+
20+
Sentry.setUser({
21+
id: user.id,
22+
...(isPiiEnabled ? {
23+
email: user.email ?? undefined,
24+
username: user.name ?? undefined,
25+
} : {}),
26+
});
27+
}

‎packages/web/src/middleware/withAuth.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const mocks = vi.hoisted(() => {
2121
isAnonymousAccessAvailable: vi.fn(() => false),
2222
syncWithLighthouse: vi.fn(async (_orgId: number) => undefined),
2323
getSeatCap: vi.fn(() => undefined as number | undefined),
24+
setSentryUser: vi.fn(),
2425
env: {} as Record<string, string>,
2526
}
2627
});
@@ -29,6 +30,10 @@ vi.mock('../auth', () => ({
2930
auth: mocks.auth,
3031
}));
3132

33+
vi.mock('@/lib/sentryUser', () => ({
34+
setSentryUser: mocks.setSentryUser,
35+
}));
36+
3237
vi.mock('next/headers', () => ({
3338
headers: mocks.headers,
3439
}));
@@ -390,6 +395,39 @@ describe('getAuthenticatedUser', () => {
390395
});
391396

392397
describe('getAuthContext', () => {
398+
test('sets the Sentry user for direct callers', async () => {
399+
const userId = 'test-user-id';
400+
const user = {
401+
...MOCK_USER_WITH_ACCOUNTS,
402+
id: userId,
403+
};
404+
prisma.user.findUnique.mockResolvedValue(user);
405+
prisma.org.findUnique.mockResolvedValue(MOCK_ORG);
406+
prisma.userToOrg.findUnique.mockResolvedValue({
407+
joinedAt: new Date(),
408+
userId,
409+
orgId: MOCK_ORG.id,
410+
suspendedAt: null,
411+
scimExternalId: null,
412+
lastActiveAt: new Date(),
413+
role: OrgRole.MEMBER,
414+
});
415+
mocks.env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED = 'true';
416+
setMockSession(createMockSession({ user: { id: userId } }));
417+
418+
await getAuthContext();
419+
420+
expect(mocks.setSentryUser).toHaveBeenCalledWith(user, true);
421+
});
422+
423+
test('clears the Sentry user for unauthenticated direct callers', async () => {
424+
prisma.org.findUnique.mockResolvedValue(MOCK_ORG);
425+
426+
await getAuthContext();
427+
428+
expect(mocks.setSentryUser).toHaveBeenCalledWith(null, false);
429+
});
430+
393431
test('should return a auth context object if a valid session is present and the user is a member of the organization', async () => {
394432
const userId = 'test-user-id';
395433
prisma.user.findUnique.mockResolvedValue({
@@ -734,6 +772,10 @@ describe('getAuthContext', () => {
734772
org: MOCK_ORG,
735773
prisma: undefined,
736774
});
775+
expect(mocks.setSentryUser).toHaveBeenCalledWith(
776+
expect.objectContaining({ id: userId }),
777+
false,
778+
);
737779
});
738780

739781
describe('DISABLE_API_KEY_USAGE_FOR_NON_OWNER_USERS', () => {

‎packages/web/src/middleware/withAuth.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { activatePendingMembership } from "@/features/membership/membership.serv
1313
import { hasRequiredOAuthScopes, parseOAuthScopeString } from "@/ee/features/oauth/utils";
1414
import { DPOP_AUTH_SCHEME, DPOP_PROOF_HEADER, verifyDpopProof } from "@/ee/features/oauth/dpop";
1515
import { getCurrentRequest } from "@/lib/requestContext";
16+
import { setSentryUser } from "@/lib/sentryUser";
1617

1718
const LAST_ACTIVE_AT_THRESHOLD_MS = 5 * 60 * 1000;
1819

@@ -70,6 +71,12 @@ export const withOptionalAuth = async <T>(fn: (params: OptionalAuthContext) => P
7071

7172
export const getAuthContext = async (options: AuthOptions = {}): Promise<OptionalAuthContext | ServiceError> => {
7273
const authResult = await getAuthenticatedUser();
74+
const user = authResult?.user;
75+
76+
setSentryUser(
77+
user ?? null,
78+
env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true',
79+
);
7380

7481
const org = await __unsafePrisma.org.findUnique({
7582
where: {
@@ -81,8 +88,6 @@ export const getAuthContext = async (options: AuthOptions = {}): Promise<Optiona
8188
return notFound("Organization not found");
8289
}
8390

84-
const user = authResult?.user;
85-
8691
const membership = user ? await __unsafePrisma.userToOrg.findUnique({
8792
where: {
8893
orgId_userId: {

0 commit comments

Comments
 (0)