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); + }); });