From 33c845636c3903ff277d5eccdef7a34e24d448a3 Mon Sep 17 00:00:00 2001 From: yusufipk Date: Sat, 1 Aug 2026 20:29:28 +0300 Subject: [PATCH] fix(analytics): sign the acquisition cookies and bound what they can write Both cookies were read straight into database columns after nothing more than a format check. httpOnly keeps JavaScript out of them and does nothing about curl, so the anonymous id was a string the caller picked: enough to write a first-touch row for a visitor who never existed, to file it under a channel of their choosing, and to claim that id's events at signup, since the backfill matches on the id alone. They are now signed with an HMAC over AUTH_SECRET, through Web Crypto rather than node:crypto because the proxy runs on the edge and the pages that read the cookies back run in Node. The first-touch body moved to base64url on the way: cookie values are percent-encoded and decoded by several layers that do not agree on how many times, and a payload carrying its own percent escapes comes back subtly different and takes the signature with it. Signing stops a caller choosing an id, not collecting one, since dropping the cookie and asking for the landing page again mints another. So the bot and prefetch filters moved to where the rows are written rather than only where the cookies are issued, which also fixes a returning visitor's prefetch of /register recording a signup start, and a per-client hourly ceiling now sits in front of the write. The ceiling is skipped when TRUSTED_PROXY_MODE is unset, where every caller resolves to 127.0.0.1 and the bucket would empty on real traffic long before it emptied on a flood. Four smaller things around it: - /api/events checked the flag and the origin after paying for a rate-limit write, so a host who never turned analytics on was still writing a row per anonymous POST. Both checks are free and now come first, and the limiter answers 204 rather than 429: a beacon has nobody to tell, and a flooder should not be handed the reset time. - /api/onboarding/source was keyed by IP on an authenticated route. Without TRUSTED_PROXY_MODE that is five answers an hour for the whole deployment, and with it a shared office address locks out everyone after one colleague answered. Keyed by account, like /api/onboarding/complete beside it. - The cookies took their Secure flag from request.nextUrl.protocol, which behind a TLS-terminating reverse proxy is the container-internal http address. It comes off the configured public origin now. - sanitizeLandingPath took anything that started with a slash, including from the cookie, so a hand-written one could put newlines and markup into a column an admin table may render one day. Also: the paid-account query had no LIMIT and returned every active account's name and email, the growth route answered 403 where it meant 401, and the schema claimed no free text is stored when self_reported_note holds 200 characters of it. --- app/(auth)/register/page.tsx | 10 +- app/(marketing)/[slug]/page.tsx | 5 +- app/admin/growth/page.tsx | 7 ++ app/api/admin/growth/route.ts | 5 +- app/api/auth/register/route.ts | 4 +- app/api/events/route.ts | 18 ++- app/api/onboarding/source/route.ts | 20 +++- app/page.tsx | 5 +- lib/analytics/channel.ts | 19 +++- lib/analytics/cookies.ts | 83 ++++++++++++-- lib/analytics/scoreboard.ts | 20 +++- lib/analytics/signing.ts | 115 +++++++++++++++++++ lib/analytics/signup.ts | 8 +- lib/analytics/visitor.ts | 99 +++++++++++++++- lib/rate-limit.ts | 26 ++++- prisma/schema.prisma | 7 +- proxy.ts | 39 ++++--- tests/api/analytics-events.test.ts | 96 ++++++++++++---- tests/unit/lib/analytics-channel.test.ts | 14 +++ tests/unit/lib/analytics-cookies.test.ts | 102 +++++++++++++++-- tests/unit/proxy.test.ts | 138 +++++++++++++++++------ 21 files changed, 711 insertions(+), 129 deletions(-) create mode 100644 lib/analytics/signing.ts diff --git a/app/(auth)/register/page.tsx b/app/(auth)/register/page.tsx index 2610263..84930c1 100644 --- a/app/(auth)/register/page.tsx +++ b/app/(auth)/register/page.tsx @@ -1,6 +1,5 @@ -import { cookies } from 'next/headers'; import { after } from 'next/server'; -import { readVisitorContext, recordVisitorEvent } from '@/lib/analytics/visitor'; +import { readPageVisitor, recordVisitorEvent } from '@/lib/analytics/visitor'; import { isInviteCodeRequired } from '@/lib/feature-flags'; import { getInvitationPreviewByToken } from '@/lib/invitations'; import { isInvitationPreviewAllowed } from '@/lib/invitation-preview-limit'; @@ -12,9 +11,10 @@ interface RegisterPageProps { export default async function RegisterPage({ searchParams }: RegisterPageProps) { // Reaching this page is the funnel step. Recording it here rather than from the - // browser also keeps it honest: a prefetch of this route is filtered in the - // proxy, so signup starts can never outnumber the landing views above them. - const visitor = readVisitorContext(await cookies()); + // browser also keeps it honest: a prefetch of this route is filtered out by + // readPageVisitor, so signup starts can never outnumber the landing views + // above them. + const visitor = await readPageVisitor(); after(() => recordVisitorEvent('SIGNUP_STARTED', visitor)); const googleEnabled = Boolean(process.env.GOOGLE_CLIENT_ID && process.env.GOOGLE_CLIENT_SECRET); diff --git a/app/(marketing)/[slug]/page.tsx b/app/(marketing)/[slug]/page.tsx index 6a5b0b5..27691af 100644 --- a/app/(marketing)/[slug]/page.tsx +++ b/app/(marketing)/[slug]/page.tsx @@ -1,9 +1,8 @@ import type { Metadata } from 'next'; -import { cookies } from 'next/headers'; import { notFound } from 'next/navigation'; import { after } from 'next/server'; import { ComparisonPage } from '@/components/marketing/comparison-page'; -import { readVisitorContext, recordVisitorEvent } from '@/lib/analytics/visitor'; +import { readPageVisitor, recordVisitorEvent } from '@/lib/analytics/visitor'; import { auth } from '@/lib/auth'; import { comparisonPages, getComparisonPage } from '@/lib/marketing/comparison-pages'; import { buildComparisonJsonLd, buildComparisonMetadata } from '@/lib/marketing/metadata'; @@ -46,7 +45,7 @@ export default async function MarketingSlugPage({ params }: MarketingSlugPagePro // A comparison page is a landing page: for most of these visitors it is the // first thing they see, so it belongs in the same visitor count as `/`. if (!isLoggedIn) { - const visitor = readVisitorContext(await cookies()); + const visitor = await readPageVisitor(); after(() => recordVisitorEvent('LANDING_VIEW', visitor)); } diff --git a/app/admin/growth/page.tsx b/app/admin/growth/page.tsx index 58d34ab..003cb1e 100644 --- a/app/admin/growth/page.tsx +++ b/app/admin/growth/page.tsx @@ -295,6 +295,13 @@ export default async function AdminGrowthPage() {

Value events are videos, share links, outside feedback, approvals and projects. Rows marked at risk have produced none for {AT_RISK_SILENT_DAYS} days. + {scoreboard.paidAccountsTruncated && ( + <> + {' '} + Quietest {scoreboard.paidAccountLimit} only; there are more paid accounts than this + table shows. + + )}

diff --git a/app/api/admin/growth/route.ts b/app/api/admin/growth/route.ts index 7a710dd..2e75810 100644 --- a/app/api/admin/growth/route.ts +++ b/app/api/admin/growth/route.ts @@ -10,7 +10,10 @@ import { logError } from '@/lib/logger'; export async function GET(request: NextRequest) { try { const session = await auth(); - if (!session?.user?.isAdmin) { + if (!session?.user?.id) { + return apiErrors.unauthorized(); + } + if (!session.user.isAdmin) { return apiErrors.forbidden('Admin access required'); } diff --git a/app/api/auth/register/route.ts b/app/api/auth/register/route.ts index e7c671b..801a8e3 100644 --- a/app/api/auth/register/route.ts +++ b/app/api/auth/register/route.ts @@ -18,7 +18,7 @@ import { } from '@/lib/email-verification'; import { isValidEmailAddress, normalizeEmail } from '@/lib/email-validation'; import { recordSignupCompleted } from '@/lib/analytics/signup'; -import { readVisitorContext } from '@/lib/analytics/visitor'; +import { readRequestVisitor } from '@/lib/analytics/visitor'; export async function POST(request: NextRequest) { try { @@ -139,7 +139,7 @@ export async function POST(request: NextRequest) { // been accepted, so an account that gets rolled back never leaves a signup. await recordSignupCompleted({ userId: user.id, - visitor: readVisitorContext(request.cookies), + visitor: await readRequestVisitor(request), }); // Send verification email if SMTP is configured diff --git a/app/api/events/route.ts b/app/api/events/route.ts index 417680e..262686d 100644 --- a/app/api/events/route.ts +++ b/app/api/events/route.ts @@ -1,7 +1,8 @@ import { NextRequest } from 'next/server'; import { rateLimit } from '@/lib/rate-limit'; import { isTrustedSameOriginRequest } from '@/lib/request-origin'; -import { readVisitorContext, recordVisitorEvent } from '@/lib/analytics/visitor'; +import { readRequestVisitor, recordVisitorEvent } from '@/lib/analytics/visitor'; +import { isProductAnalyticsEnabled } from '@/lib/feature-flags'; // The one funnel event that cannot be observed from the server: a click on a // call to action, which never reaches us as a request of its own. @@ -21,16 +22,23 @@ export async function POST(request: NextRequest) { headers: { 'Cache-Control': 'private, no-store' }, }); - const limited = await rateLimit(request, 'analytics-beacon'); - if (limited) return limited; - + // Both cheap and both free of side effects, so they come before the limiter. + // Checking the flag here rather than only inside the recorder keeps a host who + // never turned analytics on from paying a rate-limit write for every anonymous + // POST to an endpoint they are not using. + if (!isProductAnalyticsEnabled()) return noContent; if (!isTrustedSameOriginRequest(request)) return noContent; + // 204 rather than the limiter's 429: a beacon has nobody to tell, and a + // flooder should not be handed a signal for when the window resets. + const limited = await rateLimit(request, 'analytics-beacon'); + if (limited) return noContent; + const body = await request.json().catch(() => null); const name = typeof body?.name === 'string' ? body.name : ''; if (!ALLOWED_EVENTS.has(name)) return noContent; - await recordVisitorEvent('CTA_CLICKED', readVisitorContext(request.cookies)); + await recordVisitorEvent('CTA_CLICKED', await readRequestVisitor(request)); return noContent; } diff --git a/app/api/onboarding/source/route.ts b/app/api/onboarding/source/route.ts index c4cab9f..fd1c811 100644 --- a/app/api/onboarding/source/route.ts +++ b/app/api/onboarding/source/route.ts @@ -1,7 +1,7 @@ import { NextRequest } from 'next/server'; import { auth } from '@/lib/auth'; import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response'; -import { rateLimit } from '@/lib/rate-limit'; +import { RATE_LIMIT_CONFIGS, checkRateLimit, rateLimitHeaders } from '@/lib/rate-limit'; import { setSelfReportedSource } from '@/lib/analytics/record'; import { isAcquisitionChannel } from '@/lib/analytics/cookies'; import { isProductAnalyticsEnabled } from '@/lib/feature-flags'; @@ -18,8 +18,22 @@ export async function POST(request: NextRequest) { return apiErrors.unauthorized(); } - const limited = await rateLimit(request, 'onboarding-complete'); - if (limited) return limited; + // Keyed by account, like /api/onboarding/complete beside it. An IP key would + // be the wrong bucket twice over: without TRUSTED_PROXY_MODE every caller + // resolves to 127.0.0.1, so five answers an hour would be five for the whole + // deployment, and with it a shared office address would lock out everyone + // after one colleague answered. + const config = RATE_LIMIT_CONFIGS['onboarding-source']; + const limit = await checkRateLimit(session.user.id, 'onboarding-source', config); + if (!limit.allowed) { + return new Response(JSON.stringify({ error: 'Too many requests. Please try again later.' }), { + status: 429, + headers: { + 'Content-Type': 'application/json', + ...rateLimitHeaders(limit, config.maxRequests), + }, + }); + } if (!isProductAnalyticsEnabled()) { return apiErrors.badRequest('Analytics are disabled by this host'); diff --git a/app/page.tsx b/app/page.tsx index f7a2285..7ddc5f0 100644 --- a/app/page.tsx +++ b/app/page.tsx @@ -1,8 +1,7 @@ -import { cookies } from 'next/headers'; import { after } from 'next/server'; import { LandingPage } from '@/components/LandingPage'; import { auth } from '@/lib/auth'; -import { readVisitorContext, recordVisitorEvent } from '@/lib/analytics/visitor'; +import { readPageVisitor, recordVisitorEvent } from '@/lib/analytics/visitor'; export default async function HomePage() { const session = await auth(); @@ -11,7 +10,7 @@ export default async function HomePage() { // Signed-in users land here too, and counting them would put existing // customers at the top of the acquisition funnel. if (!isLoggedIn) { - const visitor = readVisitorContext(await cookies()); + const visitor = await readPageVisitor(); after(() => recordVisitorEvent('LANDING_VIEW', visitor)); } diff --git a/lib/analytics/channel.ts b/lib/analytics/channel.ts index 342e4b1..0062865 100644 --- a/lib/analytics/channel.ts +++ b/lib/analytics/channel.ts @@ -75,11 +75,24 @@ export function extractReferrerHost( return host; } -/** Path only, no query string and no fragment, capped. */ +// What a URL path is allowed to be made of, per RFC 3986: unreserved characters, +// percent escapes, sub-delims and the separators. Everything a real route can +// carry, and nothing that survives being pasted into a page or a log line. +const LANDING_PATH_PATTERN = /^\/[A-Za-z0-9\-._~%!$&'()*+,;=:@/]*$/; + +/** + * Path only, no query string and no fragment, capped and character-checked. + * + * The proxy feeds this `request.nextUrl.pathname`, which is already a path. The + * cookie reader feeds it whatever the cookie said, which is why the allowlist is + * here rather than left to the caller: an unchecked value would put newlines and + * markup into a column that some later admin table renders. + */ export function sanitizeLandingPath(pathname: string | null | undefined): string { if (typeof pathname !== 'string' || !pathname.startsWith('/')) return '/'; - const path = pathname.split('?')[0]?.split('#')[0] ?? '/'; - return path.slice(0, MAX_PATH_LENGTH) || '/'; + const path = (pathname.split('?')[0]?.split('#')[0] ?? '/').slice(0, MAX_PATH_LENGTH); + if (!path || !LANDING_PATH_PATTERN.test(path)) return '/'; + return path; } function suffixMatch(host: string, domain: string): boolean { diff --git a/lib/analytics/cookies.ts b/lib/analytics/cookies.ts index ee16352..999035f 100644 --- a/lib/analytics/cookies.ts +++ b/lib/analytics/cookies.ts @@ -5,11 +5,16 @@ // YouTube link on Tuesday and signs up on Friday is still counted against // YouTube; there is no cross-site identifier and nothing is sent anywhere. // +// Both are also signed. Nothing here trusts a cookie it did not issue: read +// through `readAnonymousIdCookie` and `readFirstTouchCookie`, never through the +// `decode` helpers, which are the unsigned inner layer. +// // Imported by the proxy, so this file must stay free of Prisma and of anything // else that cannot run on the edge. import type { AcquisitionChannel } from '@prisma/client'; import { sanitizeLandingPath, sanitizeTag, normalizeHost } from '@/lib/analytics/channel'; +import { signCookieValue, unsignCookieValue } from '@/lib/analytics/signing'; export const ANONYMOUS_ID_COOKIE = 'of_aid'; export const FIRST_TOUCH_COOKIE = 'of_ft'; @@ -58,7 +63,7 @@ export function isValidAnonymousId(value: string | null | undefined): value is s return typeof value === 'string' && ANONYMOUS_ID_PATTERN.test(value); } -/** 26 lowercase base36 characters from the Web Crypto API, which the edge has. */ +/** 128 bits from the Web Crypto API, which the edge has, as 32 base36 characters. */ export function generateAnonymousId(): string { const bytes = new Uint8Array(16); crypto.getRandomValues(bytes); @@ -69,29 +74,58 @@ export function generateAnonymousId(): string { return id; } +// base64url rather than encodeURIComponent, and not for compactness. Cookie +// values are percent-encoded on the way out and decoded on the way back, by +// several layers that do not all agree on how many times; a payload that already +// contains percent escapes comes back subtly different and takes the signature +// down with it. base64url has nothing either layer wants to touch. +function toBase64Url(text: string): string { + let binary = ''; + for (const byte of new TextEncoder().encode(text)) { + binary += String.fromCharCode(byte); + } + return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + +function fromBase64Url(value: string): string | null { + try { + const padded = value.replace(/-/g, '+').replace(/_/g, '/'); + const binary = atob(padded.padEnd(Math.ceil(padded.length / 4) * 4, '=')); + const bytes = Uint8Array.from(binary, (character) => character.charCodeAt(0)); + return new TextDecoder().decode(bytes); + } catch { + return null; + } +} + export function encodeFirstTouch(touch: FirstTouch): string { const payload: EncodedFirstTouch = { c: touch.channel, p: touch.landingPath }; if (touch.utmSource) payload.s = touch.utmSource; if (touch.utmMedium) payload.m = touch.utmMedium; if (touch.utmCampaign) payload.k = touch.utmCampaign; if (touch.referrerHost) payload.r = touch.referrerHost; - return encodeURIComponent(JSON.stringify(payload)); + return toBase64Url(JSON.stringify(payload)); } /** - * Parses the cookie back, re-sanitizing every field. + * Parses the cookie body back, re-sanitizing every field. * - * The cookie is httpOnly but it still came from the client, so a hand-edited one - * must not be able to put arbitrary text into a database column. Anything that - * fails validation makes the whole value null: a half-trusted first touch is - * worse than none. + * The second line of defence, not the first: callers go through + * `readFirstTouchCookie`, which checks the signature before this ever runs. The + * re-sanitizing stays because a value that survives both checks can still be one + * this deployment signed a year ago, under an older set of rules. Anything that + * fails validation makes the whole value null, since a half-trusted first touch + * is worse than none. */ export function decodeFirstTouch(raw: string | null | undefined): FirstTouch | null { if (!raw) return null; + const json = fromBase64Url(raw); + if (!json) return null; + let parsed: unknown; try { - parsed = JSON.parse(decodeURIComponent(raw)); + parsed = JSON.parse(json); } catch { return null; } @@ -111,3 +145,36 @@ export function decodeFirstTouch(raw: string | null | undefined): FirstTouch | n landingPath: sanitizeLandingPath(value.p), }; } + +// --------------------------------------------------------------------------- +// The signed forms, which are the only ones anything outside this file uses. +// --------------------------------------------------------------------------- + +/** The cookie value to set, or null when there is no secret to sign it with. */ +export function signAnonymousId(anonymousId: string): Promise { + return signCookieValue(anonymousId); +} + +export function signFirstTouch(touch: FirstTouch): Promise { + return signCookieValue(encodeFirstTouch(touch)); +} + +/** + * The anonymous id this deployment issued, or null. + * + * Null covers every failure the same way: no cookie, a cookie signed with + * another key, one edited by hand, one whose id no longer matches the shape we + * mint. A visitor we cannot vouch for is not counted rather than counted wrong. + */ +export async function readAnonymousIdCookie( + raw: string | null | undefined +): Promise { + const anonymousId = await unsignCookieValue(raw); + return isValidAnonymousId(anonymousId) ? anonymousId : null; +} + +export async function readFirstTouchCookie( + raw: string | null | undefined +): Promise { + return decodeFirstTouch(await unsignCookieValue(raw)); +} diff --git a/lib/analytics/scoreboard.ts b/lib/analytics/scoreboard.ts index 3ec846a..de29580 100644 --- a/lib/analytics/scoreboard.ts +++ b/lib/analytics/scoreboard.ts @@ -29,6 +29,16 @@ export const AT_RISK_SILENT_DAYS = 14; const DEFAULT_WEEKS = 12; const CHANNEL_WINDOW_DAYS = 28; +/** + * How many paid accounts the per-account table carries. + * + * The list is ordered quietest first, so the cap drops the accounts that are + * using the product most, which are the ones nobody needs to read a row about. + * It is reported rather than applied silently: a truncated table that looks + * complete is worse than a smaller one that says so. + */ +const PAID_ACCOUNT_LIMIT = 500; + export interface WeeklyRow { weekStart: Date; visitors: number; @@ -72,6 +82,9 @@ export interface Scoreboard { channels: ChannelRow[]; channelWindowDays: number; paidAccounts: PaidAccountRow[]; + /** True when there are more paid accounts than the table shows. */ + paidAccountsTruncated: boolean; + paidAccountLimit: number; atRisk: PaidAccountRow[]; currentActivePaid: number | null; currentMrrCents: number | null; @@ -240,6 +253,7 @@ export async function getScoreboard(options?: { weeks?: number }): Promise ({ + // One row over the limit was fetched purely to tell "exactly full" from "cut off". + const paidAccountsTruncated = paidAccounts.length > PAID_ACCOUNT_LIMIT; + const accounts: PaidAccountRow[] = paidAccounts.slice(0, PAID_ACCOUNT_LIMIT).map((row) => ({ userId: row.user_id, name: row.name, email: row.email, @@ -313,6 +329,8 @@ export async function getScoreboard(options?: { weeks?: number }): Promise b.visitors - a.visitors), channelWindowDays: CHANNEL_WINDOW_DAYS, paidAccounts: accounts, + paidAccountsTruncated, + paidAccountLimit: PAID_ACCOUNT_LIMIT, atRisk: accounts.filter( (account) => !account.lastValueEventAt || account.lastValueEventAt < silentBefore ), diff --git a/lib/analytics/signing.ts b/lib/analytics/signing.ts new file mode 100644 index 0000000..02dcb8b --- /dev/null +++ b/lib/analytics/signing.ts @@ -0,0 +1,115 @@ +// Signing for the two acquisition cookies. +// +// httpOnly keeps JavaScript out of these cookies. It does nothing about curl, +// and both cookies are read straight into database columns, so without a +// signature the anonymous id is simply a string the caller picked. Picking one +// is enough to write a first-touch row for a visitor who never existed, or to +// claim another visitor's events at signup, since the backfill matches on the +// id alone. +// +// Web Crypto rather than node:crypto: this is imported by the proxy, which runs +// on the edge, and by the pages that read the cookies back, which run in Node. +// Both have crypto.subtle; only Node has createHmac. + +import { logWarn } from '@/lib/logger'; + +const SEPARATOR = '.'; + +/** + * 132 bits of an HMAC-SHA256, base64url. Truncating a MAC is standard practice + * and keeps a cookie that rides on every request small. + */ +const SIGNATURE_LENGTH = 22; + +let cachedSecret: string | null = null; +let cachedKey: Promise | null = null; +let warnedAboutMissingSecret = false; + +function readSecret(): string | null { + const secret = process.env.AUTH_SECRET?.trim() || process.env.NEXTAUTH_SECRET?.trim(); + if (secret) return secret; + + // Not thrown. The proxy runs on every request and the pages render for every + // visitor; failing those to protect a funnel chart would be the wrong trade. + // Analytics simply records nothing, which is visible on /admin/growth the same + // day, and it is announced once per process rather than per request. + if (!warnedAboutMissingSecret) { + warnedAboutMissingSecret = true; + logWarn( + 'AUTH_SECRET (or NEXTAUTH_SECRET) is not set, so acquisition cookies cannot be ' + + 'signed. Nothing will be recorded while it is missing.' + ); + } + return null; +} + +function getKey(secret: string): Promise { + if (!cachedKey || cachedSecret !== secret) { + cachedSecret = secret; + cachedKey = crypto.subtle.importKey( + 'raw', + new TextEncoder().encode(secret), + { name: 'HMAC', hash: 'SHA-256' }, + false, + ['sign'] + ); + } + return cachedKey; +} + +function toBase64Url(buffer: ArrayBuffer): string { + let binary = ''; + for (const byte of new Uint8Array(buffer)) { + binary += String.fromCharCode(byte); + } + return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + +async function macOf(value: string, secret: string): Promise { + const signature = await crypto.subtle.sign( + 'HMAC', + await getKey(secret), + new TextEncoder().encode(value) + ); + return toBase64Url(signature).slice(0, SIGNATURE_LENGTH); +} + +/** Constant time, so a forged cookie learns nothing from how long it took to reject. */ +function equals(a: string, b: string): boolean { + if (a.length !== b.length) return false; + let difference = 0; + for (let index = 0; index < a.length; index += 1) { + difference |= a.charCodeAt(index) ^ b.charCodeAt(index); + } + return difference === 0; +} + +/** + * `.`. + * + * The MAC goes first and is fixed-length, so the split is a slice at a known + * offset rather than a search for a separator that a future payload might + * happen to contain. + * + * Returns null when there is no secret to sign with, which the callers treat as + * "set no cookie". + */ +export async function signCookieValue(value: string): Promise { + const secret = readSecret(); + if (!secret) return null; + return `${await macOf(value, secret)}${SEPARATOR}${value}`; +} + +/** The signed value back, or null if it was absent, truncated, or edited. */ +export async function unsignCookieValue(signed: string | null | undefined): Promise { + if (typeof signed !== 'string' || signed.length <= SIGNATURE_LENGTH + 1) return null; + + const secret = readSecret(); + if (!secret) return null; + + if (signed[SIGNATURE_LENGTH] !== SEPARATOR) return null; + const mac = signed.slice(0, SIGNATURE_LENGTH); + const value = signed.slice(SIGNATURE_LENGTH + 1); + + return equals(mac, await macOf(value, secret)) ? value : null; +} diff --git a/lib/analytics/signup.ts b/lib/analytics/signup.ts index 142fd77..cbcc96c 100644 --- a/lib/analytics/signup.ts +++ b/lib/analytics/signup.ts @@ -4,11 +4,9 @@ // a channel that only shows up for one of them is worse than no channel at all. import { eventKey, recordEvent, attachAcquisitionToUser } from '@/lib/analytics/record'; -import { readVisitorContext, type VisitorContext } from '@/lib/analytics/visitor'; +import { NO_VISITOR, readVisitorContext, type VisitorContext } from '@/lib/analytics/visitor'; import { isProductAnalyticsEnabled } from '@/lib/feature-flags'; -const NO_VISITOR: VisitorContext = { anonymousId: null, firstTouch: null }; - /** * The visitor context of the request being handled. * @@ -20,8 +18,8 @@ const NO_VISITOR: VisitorContext = { anonymousId: null, firstTouch: null }; export async function readVisitorContextFromHeaders(): Promise { if (!isProductAnalyticsEnabled()) return NO_VISITOR; try { - const { cookies } = await import('next/headers'); - return readVisitorContext(await cookies()); + const { cookies, headers } = await import('next/headers'); + return await readVisitorContext(await cookies(), await headers()); } catch { return NO_VISITOR; } diff --git a/lib/analytics/visitor.ts b/lib/analytics/visitor.ts index 734ec77..74f2fc2 100644 --- a/lib/analytics/visitor.ts +++ b/lib/analytics/visitor.ts @@ -8,17 +8,30 @@ // a purity argument. Blocking rates differ by channel, and an undercounted // denominator would make GitHub and Hacker News traffic look like it converts // better than it does. +// +// Recording server-side also means an anonymous request can write rows, so the +// three filters that decide whether a request counts live here rather than only +// in the proxy: the signature on the cookie, the bot and prefetch checks, and a +// per-client ceiling. In the proxy they only govern which cookies get issued, +// which is not the same thing as which rows get written. import type { AnalyticsEventName } from '@prisma/client'; import { ANONYMOUS_ID_COOKIE, FIRST_TOUCH_COOKIE, - decodeFirstTouch, - isValidAnonymousId, + readAnonymousIdCookie, + readFirstTouchCookie, type FirstTouch, } from '@/lib/analytics/cookies'; +import { isCountableDocumentRequest, isLikelyBot } from '@/lib/analytics/bots'; import { dailyEventKey, recordEvent, recordFirstTouch } from '@/lib/analytics/record'; import { isProductAnalyticsEnabled } from '@/lib/feature-flags'; +import { + RATE_LIMIT_CONFIGS, + checkRateLimit, + getClientIpFromHeaders, + isClientIpTrustworthy, +} from '@/lib/rate-limit'; /** Both `cookies()` from next/headers and `request.cookies` satisfy this. */ export interface AnalyticsCookieReader { @@ -28,16 +41,71 @@ export interface AnalyticsCookieReader { export interface VisitorContext { anonymousId: string | null; firstTouch: FirstTouch | null; + /** Carried so the ceiling below can be applied after the response, not during it. */ + clientIp: string | null; } -export function readVisitorContext(store: AnalyticsCookieReader): VisitorContext { - const rawId = store.get(ANONYMOUS_ID_COOKIE)?.value; +export const NO_VISITOR: VisitorContext = { anonymousId: null, firstTouch: null, clientIp: null }; + +/** + * The visitor behind a set of cookies, or an empty context. + * + * Both cookies are verified, so an id that reaches a database column is one this + * deployment issued. A forged one is not repaired or partially trusted, it is + * simply not a visitor. + */ +export async function readVisitorContext( + store: AnalyticsCookieReader, + headers?: Headers +): Promise { + if (!isProductAnalyticsEnabled()) return NO_VISITOR; + + const anonymousId = await readAnonymousIdCookie(store.get(ANONYMOUS_ID_COOKIE)?.value); + if (!anonymousId) return NO_VISITOR; + return { - anonymousId: isValidAnonymousId(rawId) ? rawId : null, - firstTouch: decodeFirstTouch(store.get(FIRST_TOUCH_COOKIE)?.value), + anonymousId, + firstTouch: await readFirstTouchCookie(store.get(FIRST_TOUCH_COOKIE)?.value), + clientIp: headers ? getClientIpFromHeaders(headers) : null, }; } +/** + * The visitor behind a page render. + * + * Applies the same two header checks the proxy does, because they mean different + * things in the two places. In the proxy they decide who gets a cookie; here they + * decide what counts. A returning visitor already holds a cookie, so without this + * Next prefetching /register as a CTA scrolls into view would record a signup + * start for a page nobody opened. + */ +export async function readPageVisitor(): Promise { + if (!isProductAnalyticsEnabled()) return NO_VISITOR; + + const { cookies, headers } = await import('next/headers'); + const requestHeaders = await headers(); + if (!isCountableDocumentRequest(requestHeaders)) return NO_VISITOR; + if (isLikelyBot(requestHeaders.get('user-agent'))) return NO_VISITOR; + + return readVisitorContext(await cookies(), requestHeaders); +} + +/** + * The visitor behind an API request. + * + * No document check: a beacon is `sec-fetch-dest: empty` by definition, and the + * routes that call this are reached by a form submission rather than by a + * navigation. + */ +export async function readRequestVisitor(request: { + cookies: AnalyticsCookieReader; + headers: Headers; +}): Promise { + if (!isProductAnalyticsEnabled()) return NO_VISITOR; + if (isLikelyBot(request.headers.get('user-agent'))) return NO_VISITOR; + return readVisitorContext(request.cookies, request.headers); +} + const DIRECT_TOUCH: FirstTouch = { channel: 'DIRECT', utmSource: null, @@ -47,6 +115,24 @@ const DIRECT_TOUCH: FirstTouch = { landingPath: '/', }; +/** + * A ceiling on how many visitors one client can invent per hour. + * + * A fresh signed cookie is one request away: drop the cookie, ask for the + * landing page again, and the proxy mints another id. The signature stops a + * caller from choosing an id, and this stops them from collecting an unbounded + * number of real ones. Skipped when the client IP is not real, where the bucket + * would be shared by everybody and would throttle the site rather than the + * flood. + */ +async function withinVisitorCeiling(clientIp: string | null): Promise { + if (!clientIp || !isClientIpTrustworthy()) return true; + + const config = RATE_LIMIT_CONFIGS['analytics-visitor']; + const result = await checkRateLimit(clientIp, 'analytics-visitor', config); + return result.allowed; +} + /** * Records an event for a visitor with no account, once per visitor per UTC day. * @@ -59,6 +145,7 @@ export async function recordVisitorEvent( ): Promise { if (!isProductAnalyticsEnabled()) return; if (!visitor.anonymousId) return; + if (!(await withinVisitorCeiling(visitor.clientIp))) return; const touch = visitor.firstTouch ?? DIRECT_TOUCH; diff --git a/lib/rate-limit.ts b/lib/rate-limit.ts index 011b767..b8e7bda 100644 --- a/lib/rate-limit.ts +++ b/lib/rate-limit.ts @@ -90,8 +90,11 @@ export const RATE_LIMIT_CONFIGS: Record = { 'verify-email': { windowMs: 15 * 60 * 1000, maxRequests: 20 }, // 20 per 15 min (clicked link) 'resend-verification': { windowMs: 60 * 60 * 1000, maxRequests: 5 }, // 5 per hour - // Onboarding — one-time action, very strict + // Onboarding — one-time action, very strict. Both are keyed by user id, not IP: + // an office behind one address must not be able to lock its colleagues out of + // finishing onboarding. 'onboarding-complete': { windowMs: 60 * 60 * 1000, maxRequests: 5 }, // 5 per hour + 'onboarding-source': { windowMs: 60 * 60 * 1000, maxRequests: 5 }, // 5 per hour // Member management 'invite-member': { windowMs: 60 * 60 * 1000, maxRequests: 30 }, // 30 per hour @@ -108,6 +111,13 @@ export const RATE_LIMIT_CONFIGS: Record = { // Analytics beacon — anonymous and public, so bound it per IP 'analytics-beacon': { windowMs: 60 * 60 * 1000, maxRequests: 30 }, // 30 per hour + // Anonymous visitor events recorded server-side from the landing pages. Bounds + // a flood that would otherwise write two rows per request forever, and is + // deliberately generous: these are the denominator of every rate on the + // scoreboard, so a limit that bites real traffic costs more than the flood it + // stops. Only applied when the client IP is real — see isClientIpTrustworthy. + 'analytics-visitor': { windowMs: 60 * 60 * 1000, maxRequests: 240 }, // 240 per hour + // General reads — generous api: { windowMs: 60 * 1000, maxRequests: 100 }, // 100 per minute }; @@ -264,6 +274,20 @@ export function getClientIpFromHeaders(headers: Headers): string { return '127.0.0.1'; } +/** + * Whether {@link getClientIp} resolves to the caller rather than to 127.0.0.1. + * + * Without TRUSTED_PROXY_MODE every request shares one bucket. That is a usable + * global brake on an endpoint nobody hits in a loop, and useless on a landing + * page: the bucket would empty on real traffic long before it emptied on an + * attacker, and the counting this whole subsystem exists for would stop. Callers + * that only make sense per-client check this first. + */ +export function isClientIpTrustworthy(): boolean { + const mode = process.env.TRUSTED_PROXY_MODE?.trim().toLowerCase(); + return mode === 'cloudflare' || mode === 'nginx'; +} + /** * Create rate limit headers for response */ diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 0f6f42b..b1ccefb 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -750,9 +750,10 @@ model VideoUploadSession { // OPENFRAME_ENABLE_ANALYTICS is set, so a self-hosted instance carries the // tables empty and pays nothing for them. // -// No free text from user content is stored. Referrers are reduced to a host and -// landing pages to a path, both without query strings, so a shared link with a -// name or token in it cannot leak in here. +// Referrers are reduced to a host and landing pages to a path, both without +// query strings, so a shared link with a name or token in it cannot leak in +// here. The one free-text column is user_acquisitions.self_reported_note, which +// holds up to 200 characters the account typed into the onboarding question. enum AcquisitionChannel { DIRECT diff --git a/proxy.ts b/proxy.ts index be1a726..a99d5a0 100644 --- a/proxy.ts +++ b/proxy.ts @@ -10,17 +10,22 @@ import { ANONYMOUS_ID_COOKIE, ANONYMOUS_ID_MAX_AGE_SECONDS, FIRST_TOUCH_COOKIE, - encodeFirstTouch, generateAnonymousId, - isValidAnonymousId, + readAnonymousIdCookie, + signAnonymousId, + signFirstTouch, } from '@/lib/analytics/cookies'; import { isCountableDocumentRequest, isLikelyBot } from '@/lib/analytics/bots'; import { isProductAnalyticsEnabled } from '@/lib/feature-flags'; +import { getPublicOrigin } from '@/lib/request-origin'; // Runs on the edge, so nothing here touches the database. It only decides who a -// visitor is and what brought them, then hands both downstream as cookies. The -// rows are written by the pages, which run in Node. -function applyAcquisitionCookies(request: NextRequest, response: NextResponse): void { +// visitor is and what brought them, then hands both downstream as signed +// cookies. The rows are written by the pages, which run in Node. +async function applyAcquisitionCookies( + request: NextRequest, + response: NextResponse +): Promise { if (!isProductAnalyticsEnabled()) return; if (!isCountableDocumentRequest(request.headers)) return; if (isLikelyBot(request.headers.get('user-agent'))) return; @@ -28,19 +33,24 @@ function applyAcquisitionCookies(request: NextRequest, response: NextResponse): const cookieOptions = { httpOnly: true, sameSite: 'lax' as const, - secure: request.nextUrl.protocol === 'https:', + // Not `request.nextUrl.protocol`. Behind a TLS-terminating reverse proxy, + // which is the deployment shape the README documents, that is the + // container-internal `http://localhost:3000` and the flag would silently + // come off in exactly the setup that needs it. + secure: getPublicOrigin(request).startsWith('https:'), path: '/', maxAge: ANONYMOUS_ID_MAX_AGE_SECONDS, }; - const existingId = request.cookies.get(ANONYMOUS_ID_COOKIE)?.value; - if (!isValidAnonymousId(existingId)) { - const anonymousId = generateAnonymousId(); + const existingId = await readAnonymousIdCookie(request.cookies.get(ANONYMOUS_ID_COOKIE)?.value); + if (!existingId) { + const signedId = await signAnonymousId(generateAnonymousId()); + if (!signedId) return; // Set on the request as well as the response: without this the page rendering // *this* request cannot see the id, and the first landing view of every new // visitor, the one carrying the campaign tags, goes unrecorded. - request.cookies.set(ANONYMOUS_ID_COOKIE, anonymousId); - response.cookies.set(ANONYMOUS_ID_COOKIE, anonymousId, cookieOptions); + request.cookies.set(ANONYMOUS_ID_COOKIE, signedId); + response.cookies.set(ANONYMOUS_ID_COOKIE, signedId, cookieOptions); } if (request.cookies.get(FIRST_TOUCH_COOKIE)) return; @@ -53,7 +63,7 @@ function applyAcquisitionCookies(request: NextRequest, response: NextResponse): const utmSource = sanitizeTag(params.get('utm_source')); const utmMedium = sanitizeTag(params.get('utm_medium')); - const firstTouch = encodeFirstTouch({ + const firstTouch = await signFirstTouch({ channel: classifyChannel({ utmSource, utmMedium, referrerHost }), utmSource, utmMedium, @@ -61,15 +71,16 @@ function applyAcquisitionCookies(request: NextRequest, response: NextResponse): referrerHost, landingPath: sanitizeLandingPath(request.nextUrl.pathname), }); + if (!firstTouch) return; request.cookies.set(FIRST_TOUCH_COOKIE, firstTouch); response.cookies.set(FIRST_TOUCH_COOKIE, firstTouch, cookieOptions); } -export function proxy(request: NextRequest) { +export async function proxy(request: NextRequest) { const response = NextResponse.next({ request }); response.headers.set('Content-Security-Policy', buildContentSecurityPolicy()); - applyAcquisitionCookies(request, response); + await applyAcquisitionCookies(request, response); return response; } diff --git a/tests/api/analytics-events.test.ts b/tests/api/analytics-events.test.ts index 5cfced9..c14f10c 100644 --- a/tests/api/analytics-events.test.ts +++ b/tests/api/analytics-events.test.ts @@ -13,7 +13,7 @@ import { POST as beacon } from '@/app/api/events/route'; import { POST as register } from '@/app/api/auth/register/route'; import { recordSubscriptionTransition } from '@/lib/analytics/billing-events'; import { recordSignupCompleted } from '@/lib/analytics/signup'; -import { encodeFirstTouch, type FirstTouch } from '@/lib/analytics/cookies'; +import { signAnonymousId, signFirstTouch, type FirstTouch } from '@/lib/analytics/cookies'; import { apiRequest, callRoute } from '../helpers/request'; import { signedOut } from '../helpers/session'; import { createUser } from '../factories'; @@ -21,6 +21,9 @@ import { createUser } from '../factories'; const ANON_ID = 'a1b2c3d4e5f60718293a4b5c6d7e8f90'; const INVITE_CODE = 'test-invite'; const ORIGIN = 'http://localhost:3000'; +const SECRET = 'analytics-test-secret'; +const BROWSER_UA = + 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126 Safari/537.36'; const GITHUB_TOUCH: FirstTouch = { channel: 'GITHUB', @@ -31,23 +34,30 @@ const GITHUB_TOUCH: FirstTouch = { landingPath: '/', }; -function visitorCookies(anonymousId = ANON_ID, touch: FirstTouch = GITHUB_TOUCH) { - return { of_aid: anonymousId, of_ft: encodeFirstTouch(touch) }; +/** What the proxy would have set. Signed, because nothing downstream trusts anything else. */ +async function visitorCookies(anonymousId = ANON_ID, touch: FirstTouch = GITHUB_TOUCH) { + return { + of_aid: (await signAnonymousId(anonymousId)) ?? '', + of_ft: (await signFirstTouch(touch)) ?? '', + }; } -function beaconRequest(options?: { +async function beaconRequest(options?: { name?: string; origin?: string | null; + userAgent?: string | null; cookies?: Record; }) { const headers: Record = {}; const origin = options?.origin === undefined ? ORIGIN : options.origin; if (origin) headers.origin = origin; + const userAgent = options?.userAgent === undefined ? BROWSER_UA : options.userAgent; + if (userAgent) headers['user-agent'] = userAgent; return apiRequest('/api/events', { body: { name: options?.name ?? 'cta_clicked' }, headers, - cookies: options?.cookies ?? visitorCookies(), + cookies: options?.cookies ?? (await visitorCookies()), }); } @@ -59,6 +69,7 @@ async function eventNames(): Promise { beforeEach(() => { signedOut(); vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); + vi.stubEnv('NEXTAUTH_SECRET', SECRET); }); afterEach(() => { @@ -67,7 +78,7 @@ afterEach(() => { describe('POST /api/events', () => { it('records a CTA click and the first touch behind it', async () => { - const response = await callRoute(beacon, beaconRequest()); + const response = await callRoute(beacon, await beaconRequest()); expect(response.status).toBe(204); @@ -91,29 +102,33 @@ describe('POST /api/events', () => { }); it('records one event however many times the same visitor clicks', async () => { - await callRoute(beacon, beaconRequest()); - await callRoute(beacon, beaconRequest()); - await callRoute(beacon, beaconRequest()); + await callRoute(beacon, await beaconRequest()); + await callRoute(beacon, await beaconRequest()); + await callRoute(beacon, await beaconRequest()); expect(await db.analyticsEvent.count()).toBe(1); }); it('counts two different visitors separately', async () => { - await callRoute(beacon, beaconRequest()); + await callRoute(beacon, await beaconRequest()); await callRoute( beacon, - beaconRequest({ cookies: visitorCookies('f0e1d2c3b4a596877869504132231415') }) + await beaconRequest({ cookies: await visitorCookies('f0e1d2c3b4a596877869504132231415') }) ); expect(await db.analyticsEvent.count()).toBe(2); }); it('never keeps the first touch of a visitor who came back through another link', async () => { - await callRoute(beacon, beaconRequest()); + await callRoute(beacon, await beaconRequest()); await callRoute( beacon, - beaconRequest({ - cookies: visitorCookies(ANON_ID, { ...GITHUB_TOUCH, channel: 'GOOGLE', utmSource: null }), + await beaconRequest({ + cookies: await visitorCookies(ANON_ID, { + ...GITHUB_TOUCH, + channel: 'GOOGLE', + utmSource: null, + }), }) ); @@ -126,7 +141,7 @@ describe('POST /api/events', () => { // Without this the endpoint would let any anonymous caller write a payment // into the funnel. for (const name of ['subscription_started', 'signup_completed', 'SUBSCRIPTION_STARTED', '']) { - const response = await callRoute(beacon, beaconRequest({ name })); + const response = await callRoute(beacon, await beaconRequest({ name })); expect(response.status, name).toBe(204); } @@ -134,23 +149,59 @@ describe('POST /api/events', () => { }); it('ignores a cross-origin caller', async () => { - const response = await callRoute(beacon, beaconRequest({ origin: 'https://evil.example' })); + const response = await callRoute( + beacon, + await beaconRequest({ origin: 'https://evil.example' }) + ); expect(response.status).toBe(204); expect(await db.analyticsEvent.count()).toBe(0); }); it('ignores a caller with no anonymous id cookie', async () => { - await callRoute(beacon, beaconRequest({ cookies: {} })); + await callRoute(beacon, await beaconRequest({ cookies: {} })); expect(await db.analyticsEvent.count()).toBe(0); expect(await db.acquisitionTouch.count()).toBe(0); }); + it('ignores a hand-written cookie, whatever channel it claims', async () => { + // httpOnly stops JavaScript, not curl. Without the signature this is a + // visitor of the caller's choosing, filed under a channel of their choosing, + // and every row on the scoreboard is theirs to write. + await callRoute( + beacon, + await beaconRequest({ + cookies: { + of_aid: 'deadbeefdeadbeefdeadbeefdeadbeef', + of_ft: encodeURIComponent(JSON.stringify({ c: 'GITHUB', p: '/' })), + }, + }) + ); + + expect(await db.analyticsEvent.count()).toBe(0); + expect(await db.acquisitionTouch.count()).toBe(0); + }); + + it('ignores a cookie signed by another deployment', async () => { + const cookies = await visitorCookies(); + vi.stubEnv('NEXTAUTH_SECRET', 'some-other-secret'); + + await callRoute(beacon, await beaconRequest({ cookies })); + + expect(await db.analyticsEvent.count()).toBe(0); + }); + + it('ignores a script that sends no user agent', async () => { + await callRoute(beacon, await beaconRequest({ userAgent: null })); + + expect(await db.analyticsEvent.count()).toBe(0); + }); + it('writes nothing at all when the flag is off', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'false'); - const response = await callRoute(beacon, beaconRequest()); + const response = await callRoute(beacon, await beaconRequest()); expect(response.status).toBe(204); expect(await db.analyticsEvent.count()).toBe(0); @@ -169,7 +220,8 @@ describe('signup attribution', () => { password: 'correct horse battery', inviteCode: INVITE_CODE, }, - cookies: visitorCookies(), + headers: { 'user-agent': BROWSER_UA }, + cookies: await visitorCookies(), }) ); } @@ -197,7 +249,7 @@ describe('signup attribution', () => { }); it('claims the events the visitor produced before they had an account', async () => { - await callRoute(beacon, beaconRequest()); + await callRoute(beacon, await beaconRequest()); await registerWithCookies('backfilled@example.com'); const user = await db.user.findUniqueOrThrow({ where: { email: 'backfilled@example.com' } }); @@ -213,11 +265,11 @@ describe('signup attribution', () => { await recordSignupCompleted({ userId: user.id, - visitor: { anonymousId: ANON_ID, firstTouch: GITHUB_TOUCH }, + visitor: { anonymousId: ANON_ID, firstTouch: GITHUB_TOUCH, clientIp: null }, }); await recordSignupCompleted({ userId: user.id, - visitor: { anonymousId: ANON_ID, firstTouch: GITHUB_TOUCH }, + visitor: { anonymousId: ANON_ID, firstTouch: GITHUB_TOUCH, clientIp: null }, }); expect(await db.analyticsEvent.count({ where: { name: 'SIGNUP_COMPLETED' } })).toBe(1); diff --git a/tests/unit/lib/analytics-channel.test.ts b/tests/unit/lib/analytics-channel.test.ts index 3a197f3..c44a175 100644 --- a/tests/unit/lib/analytics-channel.test.ts +++ b/tests/unit/lib/analytics-channel.test.ts @@ -76,6 +76,20 @@ describe('sanitizeLandingPath', () => { expect(sanitizeLandingPath('https://open-frame.net/x')).toBe('/'); expect(sanitizeLandingPath(null)).toBe('/'); }); + + it('keeps what a real route can carry', () => { + expect(sanitizeLandingPath('/vs/frame.io')).toBe('/vs/frame.io'); + expect(sanitizeLandingPath('/watch/cm4x-01_a')).toBe('/watch/cm4x-01_a'); + expect(sanitizeLandingPath('/blog/%C3%BCr%C3%BCn')).toBe('/blog/%C3%BCr%C3%BCn'); + }); + + it('drops a path that could only have come from a hand-written cookie', () => { + // The proxy feeds this a real pathname. The cookie reader feeds it whatever + // the cookie said, and that value ends up in a database column. + expect(sanitizeLandingPath('/')).toBe('/'); + expect(sanitizeLandingPath('/ok\nX-Injected: 1')).toBe('/'); + expect(sanitizeLandingPath('/a b')).toBe('/'); + }); }); describe('classifyChannel', () => { diff --git a/tests/unit/lib/analytics-cookies.test.ts b/tests/unit/lib/analytics-cookies.test.ts index 82adb41..e2e73f0 100644 --- a/tests/unit/lib/analytics-cookies.test.ts +++ b/tests/unit/lib/analytics-cookies.test.ts @@ -1,14 +1,31 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { decodeFirstTouch, encodeFirstTouch, generateAnonymousId, isAcquisitionChannel, isValidAnonymousId, + readAnonymousIdCookie, + readFirstTouchCookie, + signAnonymousId, + signFirstTouch, type FirstTouch, } from '@/lib/analytics/cookies'; import { isCountableDocumentRequest, isLikelyBot } from '@/lib/analytics/bots'; +beforeEach(() => { + vi.stubEnv('NEXTAUTH_SECRET', 'cookie-test-secret'); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +/** The cookie body a forged value would have to carry, in the wire form the reader expects. */ +function body(payload: unknown): string { + return btoa(JSON.stringify(payload)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + const TOUCH: FirstTouch = { channel: 'GITHUB', utmSource: 'github', @@ -36,23 +53,27 @@ describe('first touch cookie', () => { }); it('rejects a hand-edited cookie carrying an unknown channel', () => { - const forged = encodeURIComponent(JSON.stringify({ c: 'INVESTOR_DEMO', p: '/' })); - expect(decodeFirstTouch(forged)).toBeNull(); + expect(decodeFirstTouch(body({ c: 'INVESTOR_DEMO', p: '/' }))).toBeNull(); }); it('re-sanitizes fields rather than trusting the cookie', () => { - const forged = encodeURIComponent( - JSON.stringify({ c: 'DIRECT', p: '/x', s: '', r: 'not a host' }) + const decoded = decodeFirstTouch( + body({ c: 'DIRECT', p: '/x', s: '', r: 'not a host' }) ); - const decoded = decodeFirstTouch(forged); expect(decoded?.utmSource).toBeNull(); expect(decoded?.referrerHost).toBeNull(); }); + it('re-sanitizes a landing path that could only have been hand-written', () => { + expect(decodeFirstTouch(body({ c: 'DIRECT', p: '/' }))?.landingPath).toBe( + '/' + ); + }); + it('returns null for garbage and for an absent cookie', () => { - expect(decodeFirstTouch('%%%not-json%%%')).toBeNull(); + expect(decodeFirstTouch('%%%not-base64%%%')).toBeNull(); expect(decodeFirstTouch(null)).toBeNull(); - expect(decodeFirstTouch(encodeURIComponent(JSON.stringify(['DIRECT'])))).toBeNull(); + expect(decodeFirstTouch(body(['DIRECT']))).toBeNull(); }); }); @@ -73,6 +94,71 @@ describe('anonymous id', () => { }); }); +// Everything above tests the unsigned inner layer. Nothing outside the module +// uses it: a cookie is only a visitor once the signature says this deployment +// issued it, which is what stops a caller from inventing one with curl. +describe('signed cookies', () => { + const TOUCH_TO_SIGN: FirstTouch = { + channel: 'YOUTUBE', + utmSource: 'yt', + utmMedium: null, + utmCampaign: null, + referrerHost: 'youtube.com', + landingPath: '/', + }; + + it('round-trips an id and a first touch', async () => { + const id = generateAnonymousId(); + expect(await readAnonymousIdCookie(await signAnonymousId(id))).toBe(id); + expect(await readFirstTouchCookie(await signFirstTouch(TOUCH_TO_SIGN))).toEqual(TOUCH_TO_SIGN); + }); + + it('rejects a well-formed id that carries no signature', async () => { + expect(await readAnonymousIdCookie('a1b2c3d4e5f60718293a4b5c6d7e8f90')).toBeNull(); + }); + + it('rejects a value whose body was edited under a valid signature', async () => { + const signed = (await signAnonymousId(generateAnonymousId())) ?? ''; + const [mac] = signed.split('.'); + + expect(await readAnonymousIdCookie(`${mac}.a1b2c3d4e5f60718293a4b5c6d7e8f90`)).toBeNull(); + }); + + it('rejects a first touch re-signed to name another channel', async () => { + const forgedBody = encodeFirstTouch({ ...TOUCH_TO_SIGN, channel: 'GITHUB' }); + const signed = (await signFirstTouch(TOUCH_TO_SIGN)) ?? ''; + const [mac] = signed.split('.'); + + expect(await readFirstTouchCookie(`${mac}.${forgedBody}`)).toBeNull(); + }); + + it('rejects a cookie signed with another deployment key', async () => { + const signed = await signAnonymousId(generateAnonymousId()); + + vi.stubEnv('NEXTAUTH_SECRET', 'someone-elses-secret'); + + expect(await readAnonymousIdCookie(signed)).toBeNull(); + }); + + it('signs nothing and accepts nothing when there is no secret', async () => { + const signed = await signAnonymousId(generateAnonymousId()); + + vi.stubEnv('NEXTAUTH_SECRET', undefined); + vi.stubEnv('AUTH_SECRET', undefined); + + expect(await signAnonymousId(generateAnonymousId())).toBeNull(); + expect(await readAnonymousIdCookie(signed)).toBeNull(); + }); + + it('rejects the empty, the truncated and the separator-less', async () => { + expect(await readAnonymousIdCookie('')).toBeNull(); + expect(await readAnonymousIdCookie(undefined)).toBeNull(); + expect(await readAnonymousIdCookie('.')).toBeNull(); + expect(await readAnonymousIdCookie('a'.repeat(22))).toBeNull(); + expect(await readFirstTouchCookie('not-signed-at-all')).toBeNull(); + }); +}); + describe('isAcquisitionChannel', () => { it('accepts the nine buckets and nothing else', () => { expect(isAcquisitionChannel('REVIEW_LINK')).toBe(true); diff --git a/tests/unit/proxy.test.ts b/tests/unit/proxy.test.ts index b0320ef..a1860dc 100644 --- a/tests/unit/proxy.test.ts +++ b/tests/unit/proxy.test.ts @@ -1,15 +1,22 @@ // The proxy is where a visitor gets an identity, and it is the only place that // can: it runs on the edge, before the page, on every document request. // -// The load-bearing detail below is that the id is written to the *request* as -// well as the response. A cookie set only on the response is invisible to the +// Two load-bearing details below. The id is written to the *request* as well as +// the response, because a cookie set only on the response is invisible to the // page rendering that same request, so the very first landing view, the one -// carrying the campaign tags that brought the visitor, would go unrecorded. +// carrying the campaign tags that brought the visitor, would go unrecorded. And +// both cookies are signed, because they are read straight into database columns +// and httpOnly stops JavaScript, not curl. -import { describe, it, expect, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { NextRequest } from 'next/server'; import { proxy } from '@/proxy'; -import { ANONYMOUS_ID_COOKIE, FIRST_TOUCH_COOKIE, decodeFirstTouch } from '@/lib/analytics/cookies'; +import { + ANONYMOUS_ID_COOKIE, + FIRST_TOUCH_COOKIE, + readAnonymousIdCookie, + readFirstTouchCookie, +} from '@/lib/analytics/cookies'; const BROWSER_UA = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126 Safari/537.36'; @@ -30,43 +37,51 @@ function documentRequest( return new NextRequest(new URL(url), { headers }); } +beforeEach(() => { + vi.stubEnv('NEXTAUTH_SECRET', 'proxy-test-secret'); + // getPublicOrigin prefers a configured origin over the request URL, so the + // tests that care about the request URL have to start from neither being set. + vi.stubEnv('NEXTAUTH_URL', undefined); + vi.stubEnv('NEXT_PUBLIC_APP_URL', undefined); +}); + afterEach(() => { vi.unstubAllEnvs(); }); describe('proxy', () => { - it('always sets the content security policy', () => { + it('always sets the content security policy', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'false'); - const response = proxy(documentRequest('https://open-frame.net/')); + const response = await proxy(documentRequest('https://open-frame.net/')); expect(response.headers.get('Content-Security-Policy')).toContain("default-src 'self'"); }); - it('sets no acquisition cookie at all when the flag is off', () => { + it('sets no acquisition cookie at all when the flag is off', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'false'); - const response = proxy(documentRequest('https://open-frame.net/?utm_source=github')); + const response = await proxy(documentRequest('https://open-frame.net/?utm_source=github')); expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined(); expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined(); }); - it('gives a new visitor an id and stores what brought them', () => { + it('gives a new visitor an id and stores what brought them', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); const request = documentRequest( 'https://open-frame.net/?utm_source=youtube&utm_medium=video&utm_campaign=launch' ); - const response = proxy(request); + const response = await proxy(request); - const id = response.cookies.get(ANONYMOUS_ID_COOKIE); - expect(id?.value).toMatch(/^[a-z0-9]{32}$/); - expect(id?.httpOnly).toBe(true); - expect(id?.sameSite).toBe('lax'); - expect(id?.secure).toBe(true); + const cookie = response.cookies.get(ANONYMOUS_ID_COOKIE); + expect(await readAnonymousIdCookie(cookie?.value)).toMatch(/^[a-z0-9]{32}$/); + expect(cookie?.httpOnly).toBe(true); + expect(cookie?.sameSite).toBe('lax'); + expect(cookie?.secure).toBe(true); - const touch = decodeFirstTouch(response.cookies.get(FIRST_TOUCH_COOKIE)?.value); + const touch = await readFirstTouchCookie(response.cookies.get(FIRST_TOUCH_COOKIE)?.value); expect(touch).toEqual({ channel: 'YOUTUBE', utmSource: 'youtube', @@ -77,33 +92,38 @@ describe('proxy', () => { }); // The page rendering this same request has to be able to read both. - expect(request.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBe(id?.value); + expect(request.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBe(cookie?.value); expect(request.cookies.get(FIRST_TOUCH_COOKIE)?.value).toBeDefined(); }); - it('classifies a visit that only carries a referrer', () => { + it('classifies a visit that only carries a referrer', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const response = proxy( + const response = await proxy( documentRequest('https://open-frame.net/vs/frameio', { headers: { referer: 'https://github.com/yusufipk/OpenFrame' }, }) ); - expect(decodeFirstTouch(response.cookies.get(FIRST_TOUCH_COOKIE)?.value)).toMatchObject({ + expect( + await readFirstTouchCookie(response.cookies.get(FIRST_TOUCH_COOKIE)?.value) + ).toMatchObject({ channel: 'GITHUB', referrerHost: 'github.com', landingPath: '/vs/frameio', }); }); - it('does not overwrite the first touch of a returning visitor', () => { + it('does not overwrite the first touch of a returning visitor', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const existingId = 'a1b2c3d4e5f60718293a4b5c6d7e8f90'; + const issued = await proxy(documentRequest('https://open-frame.net/?utm_source=github')); - const response = proxy( + const response = await proxy( documentRequest('https://open-frame.net/?utm_source=google', { - cookies: { [ANONYMOUS_ID_COOKIE]: existingId, [FIRST_TOUCH_COOKIE]: 'anything' }, + cookies: { + [ANONYMOUS_ID_COOKIE]: issued.cookies.get(ANONYMOUS_ID_COOKIE)?.value ?? '', + [FIRST_TOUCH_COOKIE]: issued.cookies.get(FIRST_TOUCH_COOKIE)?.value ?? '', + }, }) ); @@ -111,22 +131,56 @@ describe('proxy', () => { expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined(); }); - it('replaces an id that does not look like one we issued', () => { + it('replaces an id it did not sign, however well formed it looks', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const response = proxy( + // The shape a real id has, chosen by the caller rather than issued here. + // Accepting it would let anyone mint visitors, and claim the events of one + // whose id they guessed. + const forged = 'a1b2c3d4e5f60718293a4b5c6d7e8f90'; + + const response = await proxy( documentRequest('https://open-frame.net/', { - cookies: { [ANONYMOUS_ID_COOKIE]: 'nope' }, + cookies: { [ANONYMOUS_ID_COOKIE]: forged }, }) ); - expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toMatch(/^[a-z0-9]{32}$/); + const issued = await readAnonymousIdCookie(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value); + expect(issued).toMatch(/^[a-z0-9]{32}$/); + expect(issued).not.toBe(forged); }); - it('ignores crawlers, so they never enter the visitor count', () => { + it('replaces an id signed with another deployment key', async () => { + vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); + const issued = await proxy(documentRequest('https://open-frame.net/')); + const stolen = issued.cookies.get(ANONYMOUS_ID_COOKIE)?.value ?? ''; + + vi.stubEnv('NEXTAUTH_SECRET', 'a-different-secret'); + const response = await proxy( + documentRequest('https://open-frame.net/', { + cookies: { [ANONYMOUS_ID_COOKIE]: stolen }, + }) + ); + + expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBeDefined(); + expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).not.toBe(stolen); + }); + + it('sets nothing when there is no secret to sign with', async () => { + vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); + vi.stubEnv('NEXTAUTH_SECRET', undefined); + vi.stubEnv('AUTH_SECRET', undefined); + + const response = await proxy(documentRequest('https://open-frame.net/')); + + expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined(); + expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined(); + }); + + it('ignores crawlers, so they never enter the visitor count', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const response = proxy( + const response = await proxy( documentRequest('https://open-frame.net/', { headers: { 'user-agent': 'Googlebot/2.1 (+http://www.google.com/bot.html)' }, }) @@ -135,15 +189,15 @@ describe('proxy', () => { expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined(); }); - it('ignores a prefetch and an API call', () => { + it('ignores a prefetch and an API call', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const prefetch = proxy( + const prefetch = await proxy( documentRequest('https://open-frame.net/register', { headers: { 'next-router-prefetch': '1' }, }) ); - const api = proxy( + const api = await proxy( documentRequest('https://open-frame.net/api/projects', { headers: { 'sec-fetch-dest': 'empty' }, }) @@ -153,11 +207,23 @@ describe('proxy', () => { expect(api.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined(); }); - it('leaves the cookie insecure on plain http, so local development works', () => { + it('leaves the cookie insecure on plain http, so local development works', async () => { vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); - const response = proxy(documentRequest('http://localhost:3000/')); + const response = await proxy(documentRequest('http://localhost:3000/')); expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.secure).toBe(false); }); + + it('keeps the cookie secure behind a TLS-terminating proxy', async () => { + vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true'); + // What a Docker deployment looks like from inside the container: the request + // arrived over http on an internal address, and only the configured origin + // knows the site is served over TLS. + vi.stubEnv('NEXTAUTH_URL', 'https://open-frame.net'); + + const response = await proxy(documentRequest('http://localhost:3000/')); + + expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.secure).toBe(true); + }); });