Skip to content

Commit 1b59d3c

Browse files
fix(privacy): scrub magic link URLs (#2359)
1 parent fa84e4c commit 1b59d3c

12 files changed

Lines changed: 187 additions & 47 deletions

File tree

apps/web/public/robots.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
User-agent: *
22
Disallow: /users/sign_in
3+
Disallow: /auth/verify-magic-link

apps/web/src/app/auth/verify-magic-link/page.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,14 @@ function VerifyMagicLinkContent() {
1111

1212
useEffect(() => {
1313
const token = searchParams.get('token');
14-
const email = searchParams.get('email');
1514
const callbackUrl = searchParams.get('callbackUrl') || '/users/after-sign-in';
1615

17-
if (!token || !email) {
16+
if (!token) {
1817
window.location.href = '/users/sign_in?error=INVALID_VERIFICATION';
1918
return;
2019
}
2120

22-
// Sign in using the email provider with the token
2321
signIn('email', {
24-
email,
2522
token,
2623
callbackUrl,
2724
redirect: true,

apps/web/src/components/PostHogProvider.tsx

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { PostHogProvider as PHProvider, usePostHog } from 'posthog-js/react';
55
import { useSession } from 'next-auth/react';
66
import { Suspense, useEffect, useRef } from 'react';
77
import { usePathname, useSearchParams } from 'next/navigation';
8+
import { sanitizeAnalyticsUrl, sanitizeAnalyticsUrlValue } from '@/lib/sanitize-analytics-url';
89

910
export function PostHogProvider({ children }: { children: React.ReactNode }) {
1011
useEffect(() => {
@@ -25,6 +26,18 @@ export function PostHogProvider({ children }: { children: React.ReactNode }) {
2526
disable_web_experiments: false,
2627
capture_pageview: false, // We capture pageviews manually
2728
capture_pageleave: true, // Enable pageleave capture
29+
before_send: event => {
30+
if (!event?.properties) return event;
31+
return {
32+
...event,
33+
properties: {
34+
...event.properties,
35+
$current_url: sanitizeAnalyticsUrlValue(event.properties.$current_url),
36+
$referrer: sanitizeAnalyticsUrlValue(event.properties.$referrer),
37+
$referring_domain: sanitizeAnalyticsUrlValue(event.properties.$referring_domain),
38+
},
39+
};
40+
},
2841
loaded: function (ph) {
2942
if (!isProduction) {
3043
// Opt out of capturing in non-production environments
@@ -69,11 +82,7 @@ function PostHogPageView() {
6982

7083
useEffect(() => {
7184
if (pathname && posthog) {
72-
let url = window.origin + pathname;
73-
const search = searchParams.toString();
74-
if (search) {
75-
url += '?' + search;
76-
}
85+
const url = sanitizeAnalyticsUrl(window.origin, pathname, searchParams.toString());
7786
posthog.capture('$pageview', { $current_url: url });
7887
}
7988
}, [pathname, searchParams, posthog]);

apps/web/src/lib/auth/magic-link-tokens.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, it, expect, beforeEach } from '@jest/globals';
2-
import { createMagicLinkToken, verifyAndConsumeMagicLinkToken } from './magic-link-tokens';
2+
import {
3+
createMagicLinkToken,
4+
getMagicLinkUrl,
5+
verifyAndConsumeMagicLinkToken,
6+
} from './magic-link-tokens';
37
import { db } from '@/lib/drizzle';
48
import { sql } from 'drizzle-orm';
59

@@ -53,6 +57,18 @@ describe('Magic Link Tokens', () => {
5357
});
5458
});
5559

60+
describe('getMagicLinkUrl', () => {
61+
it('does not include email addresses in magic link URLs', async () => {
62+
const token = await createMagicLinkToken(testEmail);
63+
const url = new URL(getMagicLinkUrl(token));
64+
65+
expect(url.pathname).toBe('/auth/verify-magic-link');
66+
expect(url.searchParams.get('token')).toBe(token.plaintext_token);
67+
expect(url.searchParams.has('email')).toBe(false);
68+
expect(url.toString()).not.toContain(encodeURIComponent(testEmail));
69+
});
70+
});
71+
5672
describe('verifyAndConsumeMagicLinkToken', () => {
5773
it('should verify and consume a valid token', async () => {
5874
const created = await createMagicLinkToken(testEmail);

apps/web/src/lib/auth/magic-link-tokens.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,11 @@ export async function verifyAndConsumeMagicLinkToken(
8585
}
8686

8787
export function getMagicLinkUrl(
88-
{ plaintext_token, email }: MagicLinkTokenWithPlaintext,
88+
{ plaintext_token }: MagicLinkTokenWithPlaintext,
8989
callbackUrl?: string
9090
): string {
9191
const url = new URL(`${NEXTAUTH_URL}/auth/verify-magic-link`);
9292
url.searchParams.set('token', plaintext_token);
93-
url.searchParams.set('email', email);
9493
if (callbackUrl) {
9594
url.searchParams.set('callbackUrl', callbackUrl);
9695
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { authFailureRedirectUrl, ssoSignInRedirectUrl } from '@/lib/auth/redirect-urls';
2+
3+
describe('auth redirect URLs', () => {
4+
it('does not include email addresses on auth failure redirects', () => {
5+
const url = authFailureRedirectUrl('BLOCKED', false);
6+
7+
expect(url).toBe('/users/sign_in?error=BLOCKED');
8+
expect(new URL(url, 'https://app.kilo.ai').searchParams.has('email')).toBe(false);
9+
});
10+
11+
it('does not include email addresses on account-linking failure redirects', () => {
12+
const url = authFailureRedirectUrl('DIFFERENT-OAUTH', true);
13+
14+
expect(url).toBe('/connected-accounts?error=DIFFERENT-OAUTH');
15+
expect(new URL(url, 'https://app.kilo.ai').searchParams.has('email')).toBe(false);
16+
});
17+
18+
it('does not include email addresses on SSO enforcement redirects', () => {
19+
const url = ssoSignInRedirectUrl('example.com');
20+
21+
expect(url).toBe('/users/sign_in?domain=example.com');
22+
expect(new URL(url, 'https://app.kilo.ai').searchParams.has('email')).toBe(false);
23+
});
24+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { SSO_SIGNIN_PATH, type AuthErrorType } from '@/lib/auth/constants';
2+
3+
export function authFailureRedirectUrl(error: AuthErrorType, isAccountLinking: boolean): string {
4+
const baseUrl = isAccountLinking ? '/connected-accounts' : '/users/sign_in';
5+
return `${baseUrl}?${new URLSearchParams({ error }).toString()}`;
6+
}
7+
8+
export function ssoSignInRedirectUrl(domain: string): string {
9+
return `${SSO_SIGNIN_PATH}?${new URLSearchParams({ domain }).toString()}`;
10+
}

apps/web/src/lib/email.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ export async function sendMagicLinkEmail(
187187
templateVars: {
188188
magic_link_url: getMagicLinkUrl(magicLink, callbackUrl),
189189
email: magicLink.email,
190-
expires_in: '24 hours',
190+
expires_in: '30 minutes',
191191
expires_at: new Date(magicLink.expires_at).toISOString(),
192192
app_url: NEXTAUTH_URL,
193193
},
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { sanitizeAnalyticsUrl, sanitizeAnalyticsUrlValue } from '@/lib/sanitize-analytics-url';
2+
3+
describe('sanitizeAnalyticsUrl', () => {
4+
it('drops all query params from magic link verification URLs', () => {
5+
expect(
6+
sanitizeAnalyticsUrl(
7+
'https://app.kilo.ai',
8+
'/auth/verify-magic-link',
9+
'token=secret&email=user%40example.com&callbackUrl=%2Fprofile'
10+
)
11+
).toBe('https://app.kilo.ai/auth/verify-magic-link');
12+
});
13+
14+
it('removes sensitive query params from other URLs', () => {
15+
expect(
16+
sanitizeAnalyticsUrl(
17+
'https://app.kilo.ai',
18+
'/users/sign_in',
19+
'email=user%40example.com&state=secret&foo=bar&Token=abc'
20+
)
21+
).toBe('https://app.kilo.ai/users/sign_in?foo=bar');
22+
});
23+
24+
it('preserves non-sensitive query params', () => {
25+
expect(sanitizeAnalyticsUrl('https://app.kilo.ai', '/credits', 'tab=usage&page=2')).toBe(
26+
'https://app.kilo.ai/credits?tab=usage&page=2'
27+
);
28+
});
29+
30+
it('sanitizes full analytics URL property values', () => {
31+
expect(
32+
sanitizeAnalyticsUrlValue(
33+
'https://app.kilo.ai/users/sign_in?email=user%40example.com&state=secret&foo=bar'
34+
)
35+
).toBe('https://app.kilo.ai/users/sign_in?foo=bar');
36+
});
37+
38+
it('leaves non-URL property values unchanged', () => {
39+
expect(sanitizeAnalyticsUrlValue('not a url')).toBe('not a url');
40+
expect(sanitizeAnalyticsUrlValue(null)).toBeNull();
41+
});
42+
});
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
const SENSITIVE_QUERY_PARAMS = new Set(['callbackurl', 'code', 'email', 'state', 'token']);
2+
const SENSITIVE_PATHS = new Set(['/auth/verify-magic-link']);
3+
4+
function sanitizeUrlParts(origin: string, pathname: string, searchParams: string): string {
5+
const baseUrl = `${origin}${pathname}`;
6+
if (SENSITIVE_PATHS.has(pathname) || !searchParams) {
7+
return baseUrl;
8+
}
9+
10+
const sanitizedParams = new URLSearchParams();
11+
new URLSearchParams(searchParams).forEach((value, key) => {
12+
if (!SENSITIVE_QUERY_PARAMS.has(key.toLowerCase())) {
13+
sanitizedParams.append(key, value);
14+
}
15+
});
16+
17+
const sanitizedSearch = sanitizedParams.toString();
18+
return sanitizedSearch ? `${baseUrl}?${sanitizedSearch}` : baseUrl;
19+
}
20+
21+
export function sanitizeAnalyticsUrl(
22+
origin: string,
23+
pathname: string,
24+
searchParams: string
25+
): string {
26+
return sanitizeUrlParts(origin, pathname, searchParams);
27+
}
28+
29+
export function sanitizeAnalyticsUrlValue(value: unknown): unknown {
30+
if (typeof value !== 'string') return value;
31+
32+
if (!value.startsWith('http://') && !value.startsWith('https://') && !value.startsWith('/')) {
33+
return value;
34+
}
35+
36+
try {
37+
const baseOrigin = typeof window === 'undefined' ? 'http://localhost' : window.origin;
38+
const url = new URL(value, baseOrigin);
39+
return sanitizeUrlParts(url.origin, url.pathname, url.searchParams.toString());
40+
} catch {
41+
return value;
42+
}
43+
}

0 commit comments

Comments
 (0)