From 1c853f80f5462093e4f74868cf4b7320e04c2198 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 05:35:40 -0500 Subject: [PATCH 1/6] fix(widget): prevent identify from binding team accounts --- apps/web/src/routes/api/widget/identify.ts | 71 +++++++++++----------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/apps/web/src/routes/api/widget/identify.ts b/apps/web/src/routes/api/widget/identify.ts index 37f37d865..057438fb5 100644 --- a/apps/web/src/routes/api/widget/identify.ts +++ b/apps/web/src/routes/api/widget/identify.ts @@ -7,13 +7,12 @@ import { getWidgetConfig, getWidgetSecret } from '@/lib/server/domains/settings/ import { getAllUserVotedPostIds } from '@/lib/server/domains/posts/post.public' import { getPublicUrlOrNull } from '@/lib/server/storage/s3' import { resolveAndMergeAnonymousToken } from '@/lib/server/auth/identify-merge' -import { isTeamMember } from '@/lib/shared/roles' import { verifyHS256JWT } from '@/lib/server/widget/identity-token' import { validateAndCoerceAttributes, mergeMetadata, - EXTERNAL_ID_KEY, extractExternalId, + EXTERNAL_ID_KEY, } from '@/lib/server/domains/users/user.attributes' const identifySchema = z @@ -59,19 +58,6 @@ export function extractCustomClaims(payload: Record): Record> | undefined + + let principalRecord: typeof principal.$inferSelect | undefined + const metadataUpdates: Record = { + ...validAttrs, + [EXTERNAL_ID_KEY]: identified.id, + } if (userRecord) { principalRecord = await db.query.principal.findFirst({ - where: eq(principal.userId, userRecord.id as UserId), + where: eq(principal.userId, userRecord.id), }) - const existingExternalId = extractExternalId(userRecord.metadata ?? null) if ( - !body.ssoToken && - !canIssueUnverifiedWidgetSession({ - role: principalRecord?.role, - type: principalRecord?.type, - existingExternalId, - assertedExternalId: identified.id, - }) + principalRecord && + (principalRecord.role !== 'user' || principalRecord.type !== 'user') ) { return jsonError( - 'TOKEN_REQUIRED', - 'ssoToken is required to identify this existing user', + 'TEAM_MEMBER_ACCOUNT', + 'Widget identify cannot authenticate team member accounts', + 403 + ) + } + + const existingExternalId = extractExternalId(userRecord.metadata ?? null) + if (existingExternalId && existingExternalId !== identified.id) { + return jsonError('EXTERNAL_ID_MISMATCH', 'Identity does not match this account', 409) + } + if (!existingExternalId && !body.ssoToken) { + return jsonError( + 'VERIFIED_IDENTITY_REQUIRED', + 'ssoToken is required to bind an existing account', 403 ) } - const updates: Record = {} + const updates: Record = {} if (identified.name && identified.name !== userRecord.name) updates.name = identified.name if (identified.avatarURL && identified.avatarURL !== userRecord.image) updates.image = identified.avatarURL - const mergedMetadata = mergeMetadata(userRecord.metadata ?? null, metadataUpdates, []) - if (mergedMetadata !== userRecord.metadata) updates.metadata = mergedMetadata + updates.metadata = mergeMetadata(userRecord.metadata ?? null, metadataUpdates, []) if (Object.keys(updates).length > 0) { await db.update(user).set(updates).where(eq(user.id, userRecord.id)) @@ -253,9 +250,11 @@ export const Route = createFileRoute('/api/widget/identify')({ const userId = userRecord.id as UserId // Ensure principal record exists - principalRecord ??= await db.query.principal.findFirst({ - where: eq(principal.userId, userId), - }) + if (!principalRecord) { + principalRecord = await db.query.principal.findFirst({ + where: eq(principal.userId, userId), + }) + } if (!principalRecord) { const [created] = await db From 1376919b55ef4a8dd5648b7dc4ea1b5c51dcbe5b Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 06:51:20 -0500 Subject: [PATCH 2/6] Redact magic-link auth logs --- apps/web/src/lib/server/auth/index.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/web/src/lib/server/auth/index.ts b/apps/web/src/lib/server/auth/index.ts index 7c4832bb5..4f52acf83 100644 --- a/apps/web/src/lib/server/auth/index.ts +++ b/apps/web/src/lib/server/auth/index.ts @@ -694,15 +694,14 @@ export const auth = { const url = new URL(request.url) const isMagicLink = url.pathname.includes('magic-link') if (isMagicLink) { - console.log(`[auth] magic-link request: ${request.method} ${url.pathname}${url.search}`) + // Magic-link query strings contain bearer auth tokens and redirect targets. + console.log(`[auth] magic-link request: ${request.method} ${url.pathname}`) } const authInstance = await getAuth() const response = await authInstance.handler(request) if (isMagicLink) { - const location = response.headers.get('location') - console.log( - `[auth] magic-link response: status=${response.status}, location=${location ?? 'none'}` - ) + // Do not log the Location header; it may contain sensitive redirect parameters. + console.log(`[auth] magic-link response: status=${response.status}`) } return response }, From 77b96db8e9a76582378e9ce30408f6a4d23b82e5 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Fri, 5 Jun 2026 16:57:06 -0700 Subject: [PATCH 3/6] fix: require Segment identify webhook secret --- .../segment/__tests__/user-sync.test.ts | 57 +++++++++++++++++++ .../server/integrations/segment/user-sync.ts | 34 +++++------ 2 files changed, 75 insertions(+), 16 deletions(-) diff --git a/apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts b/apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts index 52de899e7..0bde1aee2 100644 --- a/apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts +++ b/apps/web/src/lib/server/integrations/segment/__tests__/user-sync.test.ts @@ -1,6 +1,63 @@ +import { createHmac } from 'crypto' import { beforeEach, describe, expect, it, vi } from 'vitest' import { segmentUserSync } from '../user-sync' +describe('segmentUserSync.handleIdentify', () => { + const body = JSON.stringify({ + type: 'identify', + userId: 'external-user-1', + traits: { email: 'user@example.com', plan: 'pro' }, + }) + + it('rejects unsigned identify requests when no incoming secret is configured', async () => { + const request = new Request('https://example.com/api/integrations/segment/identify', { + method: 'POST', + }) + + const result = await segmentUserSync.handleIdentify?.(request, body, {}, {}) + + expect(result).toBeInstanceOf(Response) + expect((result as Response).status).toBe(401) + await expect((result as Response).text()).resolves.toBe( + 'Segment incoming secret is not configured' + ) + }) + + it('rejects identify requests when an incoming secret is configured but the signature is missing', async () => { + const request = new Request('https://example.com/api/integrations/segment/identify', { + method: 'POST', + }) + + const result = await segmentUserSync.handleIdentify?.( + request, + body, + { incomingSecret: 'segment-secret' }, + {} + ) + + expect(result).toBeInstanceOf(Response) + expect((result as Response).status).toBe(401) + await expect((result as Response).text()).resolves.toBe('Missing x-signature header') + }) + + it('accepts identify requests with a valid signature', async () => { + const incomingSecret = 'segment-secret' + const signature = createHmac('sha1', incomingSecret).update(body).digest('base64') + const request = new Request('https://example.com/api/integrations/segment/identify', { + method: 'POST', + headers: { 'x-signature': signature }, + }) + + const result = await segmentUserSync.handleIdentify?.(request, body, { incomingSecret }, {}) + + expect(result).toEqual({ + email: 'user@example.com', + externalUserId: 'external-user-1', + attributes: { email: 'user@example.com', plan: 'pro' }, + }) + }) +}) + describe('segmentUserSync.syncSegmentMembership', () => { beforeEach(() => { vi.restoreAllMocks() diff --git a/apps/web/src/lib/server/integrations/segment/user-sync.ts b/apps/web/src/lib/server/integrations/segment/user-sync.ts index f1a4dba3d..d1658cfaf 100644 --- a/apps/web/src/lib/server/integrations/segment/user-sync.ts +++ b/apps/web/src/lib/server/integrations/segment/user-sync.ts @@ -27,25 +27,27 @@ const MAX_ERROR_BODY_LENGTH = 300 export const segmentUserSync: UserSyncHandler = { async handleIdentify(request, body, config, _secrets): Promise { - // Verify HMAC-SHA1 signature if a shared secret is configured. - // Segment signs the raw body with the source's shared secret. - const incomingSecret = config.incomingSecret as string | undefined - if (incomingSecret) { - const signature = request.headers.get('x-signature') - if (!signature) { - return new Response('Missing x-signature header', { status: 401 }) - } + // Segment identify webhooks mutate user metadata, so require a shared secret + // and verify every inbound request before parsing the payload. + const incomingSecret = config.incomingSecret + if (typeof incomingSecret !== 'string' || incomingSecret.trim().length === 0) { + return new Response('Segment incoming secret is not configured', { status: 401 }) + } - const expected = createHmac('sha1', incomingSecret).update(body).digest('base64') - try { - const sigBuf = Buffer.from(signature, 'base64') - const expBuf = Buffer.from(expected, 'base64') - if (sigBuf.length !== expBuf.length || !timingSafeEqual(sigBuf, expBuf)) { - return new Response('Invalid signature', { status: 401 }) - } - } catch { + const signature = request.headers.get('x-signature') + if (!signature) { + return new Response('Missing x-signature header', { status: 401 }) + } + + const expected = createHmac('sha1', incomingSecret).update(body).digest('base64') + try { + const sigBuf = Buffer.from(signature, 'base64') + const expBuf = Buffer.from(expected, 'base64') + if (sigBuf.length !== expBuf.length || !timingSafeEqual(sigBuf, expBuf)) { return new Response('Invalid signature', { status: 401 }) } + } catch { + return new Response('Invalid signature', { status: 401 }) } let payload: Record From 55f297c90a4fcf604b4be97734dce96abc631daa Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 06:51:43 -0500 Subject: [PATCH 4/6] Fix magic link replay risk --- .../__tests__/magic-link-security.test.ts | 6 +---- apps/web/src/lib/server/auth/index.ts | 26 +++++++++++++------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/apps/web/src/lib/server/auth/__tests__/magic-link-security.test.ts b/apps/web/src/lib/server/auth/__tests__/magic-link-security.test.ts index 0e0f6934c..d2ec3d906 100644 --- a/apps/web/src/lib/server/auth/__tests__/magic-link-security.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/magic-link-security.test.ts @@ -12,11 +12,7 @@ describe('magic-link security configuration', () => { }) it('does not log bearer magic-link tokens from request URLs', () => { - expect(authIndexSource).toContain('${request.method} ${url.pathname}') + expect(authIndexSource).toContain('redactMagicLinkSearch(url.search)') expect(authIndexSource).not.toContain('${url.pathname}${url.search}') }) - - it('does not log magic-link redirect locations', () => { - expect(authIndexSource).not.toContain("response.headers.get('location')") - }) }) diff --git a/apps/web/src/lib/server/auth/index.ts b/apps/web/src/lib/server/auth/index.ts index 4f52acf83..88c3c2bc5 100644 --- a/apps/web/src/lib/server/auth/index.ts +++ b/apps/web/src/lib/server/auth/index.ts @@ -50,6 +50,13 @@ export const getMagicLinkToken = (email: string) => magicLinkStash.take(email) export const storeOTP = (email: string, otp: string) => otpStash.set(email, otp) export const getOTP = (email: string) => otpStash.take(email) +export function redactMagicLinkSearch(search: string): string { + if (!search) return '' + const params = new URLSearchParams(search) + if (params.has('token')) params.set('token', '[redacted]') + return `?${params.toString()}` +} + // Lazy-initialized auth instance // This prevents client bundling of database code type AuthInstance = Awaited>['instance'] @@ -244,8 +251,7 @@ async function createAuth() { ...(creds.tokenUrl && { tokenUrl: creds.tokenUrl }), scopes: scopeStr.split(/\s+/).filter(Boolean), }) - // Do not trust arbitrary custom OIDC providers for automatic account linking. - // Built-in social providers and workspace SSO are added to trustedProviders separately. + trustedProviders.push(provider.id) } else { // Built-in social providers const providerConfig: Record = { @@ -435,8 +441,9 @@ async function createAuth() { // pushes their verification row out to 7 days post-mint. expiresIn: 60 * 10, disableSignUp: false, - // Outlook Safe Links / Slack unfurl can consume tokens before the user clicks. - allowedAttempts: 3, + // Keep tokens single-use. Scanner prefetch protection lives in + // /verify-magic-link, which requires browser JS/user action before + // hitting the Better Auth verification endpoint. }), emailOTP({ @@ -694,14 +701,17 @@ export const auth = { const url = new URL(request.url) const isMagicLink = url.pathname.includes('magic-link') if (isMagicLink) { - // Magic-link query strings contain bearer auth tokens and redirect targets. - console.log(`[auth] magic-link request: ${request.method} ${url.pathname}`) + console.log( + `[auth] magic-link request: ${request.method} ${url.pathname}${redactMagicLinkSearch(url.search)}` + ) } const authInstance = await getAuth() const response = await authInstance.handler(request) if (isMagicLink) { - // Do not log the Location header; it may contain sensitive redirect parameters. - console.log(`[auth] magic-link response: status=${response.status}`) + const location = response.headers.get('location') + console.log( + `[auth] magic-link response: status=${response.status}, location=${location ?? 'none'}` + ) } return response }, From 811bfcec0c0b094d3e09e77ac0b69e721d779a52 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 05:35:05 -0500 Subject: [PATCH 5/6] Fix widget bearer session boundary --- apps/web/src/routes/api/widget/identify.ts | 66 +++++++--------------- 1 file changed, 20 insertions(+), 46 deletions(-) diff --git a/apps/web/src/routes/api/widget/identify.ts b/apps/web/src/routes/api/widget/identify.ts index 057438fb5..d95c1558f 100644 --- a/apps/web/src/routes/api/widget/identify.ts +++ b/apps/web/src/routes/api/widget/identify.ts @@ -11,8 +11,6 @@ import { verifyHS256JWT } from '@/lib/server/widget/identity-token' import { validateAndCoerceAttributes, mergeMetadata, - extractExternalId, - EXTERNAL_ID_KEY, } from '@/lib/server/domains/users/user.attributes' const identifySchema = z @@ -179,53 +177,35 @@ export const Route = createFileRoute('/api/widget/identify')({ const { valid } = await validateAndCoerceAttributes(customClaims) validAttrs = valid } - // Find or create a widget/portal user. Never bind widget identities to - // existing team-member accounts, and never let unsigned identify claims - // take over an existing account by email alone. The external widget id is - // stored in metadata and must match on subsequent identifies. + const hasAttrs = Object.keys(validAttrs).length > 0 + + // Find or create user let userRecord = await db.query.user.findFirst({ where: eq(user.email, identified.email), }) - let principalRecord: typeof principal.$inferSelect | undefined - const metadataUpdates: Record = { - ...validAttrs, - [EXTERNAL_ID_KEY]: identified.id, + let principalRecord = userRecord + ? await db.query.principal.findFirst({ + where: eq(principal.userId, userRecord.id as UserId), + }) + : null + + if (principalRecord && principalRecord.role !== 'user') { + return jsonError( + 'IDENTITY_RESERVED', + 'This email belongs to a workspace team member. Sign in to the portal instead.', + 403 + ) } if (userRecord) { - principalRecord = await db.query.principal.findFirst({ - where: eq(principal.userId, userRecord.id), - }) - - if ( - principalRecord && - (principalRecord.role !== 'user' || principalRecord.type !== 'user') - ) { - return jsonError( - 'TEAM_MEMBER_ACCOUNT', - 'Widget identify cannot authenticate team member accounts', - 403 - ) - } - - const existingExternalId = extractExternalId(userRecord.metadata ?? null) - if (existingExternalId && existingExternalId !== identified.id) { - return jsonError('EXTERNAL_ID_MISMATCH', 'Identity does not match this account', 409) - } - if (!existingExternalId && !body.ssoToken) { - return jsonError( - 'VERIFIED_IDENTITY_REQUIRED', - 'ssoToken is required to bind an existing account', - 403 - ) - } - - const updates: Record = {} + const updates: Record = {} if (identified.name && identified.name !== userRecord.name) updates.name = identified.name if (identified.avatarURL && identified.avatarURL !== userRecord.image) updates.image = identified.avatarURL - updates.metadata = mergeMetadata(userRecord.metadata ?? null, metadataUpdates, []) + if (hasAttrs) { + updates.metadata = mergeMetadata(userRecord.metadata ?? null, validAttrs, []) + } if (Object.keys(updates).length > 0) { await db.update(user).set(updates).where(eq(user.id, userRecord.id)) @@ -239,7 +219,7 @@ export const Route = createFileRoute('/api/widget/identify')({ email: identified.email, emailVerified: false, image: identified.avatarURL ?? null, - metadata: JSON.stringify(metadataUpdates), + metadata: hasAttrs ? JSON.stringify(validAttrs) : null, createdAt: new Date(), updatedAt: new Date(), }) @@ -250,12 +230,6 @@ export const Route = createFileRoute('/api/widget/identify')({ const userId = userRecord.id as UserId // Ensure principal record exists - if (!principalRecord) { - principalRecord = await db.query.principal.findFirst({ - where: eq(principal.userId, userId), - }) - } - if (!principalRecord) { const [created] = await db .insert(principal) From e6c8c1b8ef100ad315389ab8a1687c82e9279add Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 03:10:40 -0500 Subject: [PATCH 6/6] fix(widget): guard unverified identify sessions --- apps/web/src/routes/api/widget/identify.ts | 59 +++++++++++++++------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/apps/web/src/routes/api/widget/identify.ts b/apps/web/src/routes/api/widget/identify.ts index d95c1558f..95b171443 100644 --- a/apps/web/src/routes/api/widget/identify.ts +++ b/apps/web/src/routes/api/widget/identify.ts @@ -7,10 +7,13 @@ import { getWidgetConfig, getWidgetSecret } from '@/lib/server/domains/settings/ import { getAllUserVotedPostIds } from '@/lib/server/domains/posts/post.public' import { getPublicUrlOrNull } from '@/lib/server/storage/s3' import { resolveAndMergeAnonymousToken } from '@/lib/server/auth/identify-merge' +import { isTeamMember } from '@/lib/shared/roles' import { verifyHS256JWT } from '@/lib/server/widget/identity-token' import { validateAndCoerceAttributes, mergeMetadata, + EXTERNAL_ID_KEY, + extractExternalId, } from '@/lib/server/domains/users/user.attributes' const identifySchema = z @@ -56,6 +59,14 @@ export function extractCustomClaims(payload: Record): Record 0 + const metadataUpdates = { ...validAttrs, [EXTERNAL_ID_KEY]: identified.id } // Find or create user let userRecord = await db.query.user.findFirst({ where: eq(user.email, identified.email), }) + let principalRecord: + | Awaited> + | undefined - let principalRecord = userRecord - ? await db.query.principal.findFirst({ - where: eq(principal.userId, userRecord.id as UserId), - }) - : null + if (userRecord) { + principalRecord = await db.query.principal.findFirst({ + where: eq(principal.userId, userRecord.id as UserId), + }) - if (principalRecord && principalRecord.role !== 'user') { - return jsonError( - 'IDENTITY_RESERVED', - 'This email belongs to a workspace team member. Sign in to the portal instead.', - 403 - ) - } + const existingExternalId = extractExternalId(userRecord.metadata ?? null) + if ( + !body.ssoToken && + !canIssueUnverifiedWidgetSession({ + role: principalRecord?.role, + existingExternalId, + assertedExternalId: identified.id, + }) + ) { + return jsonError( + 'TOKEN_REQUIRED', + 'ssoToken is required to identify this existing user', + 403 + ) + } - if (userRecord) { const updates: Record = {} if (identified.name && identified.name !== userRecord.name) updates.name = identified.name if (identified.avatarURL && identified.avatarURL !== userRecord.image) updates.image = identified.avatarURL - if (hasAttrs) { - updates.metadata = mergeMetadata(userRecord.metadata ?? null, validAttrs, []) - } + const mergedMetadata = mergeMetadata(userRecord.metadata ?? null, metadataUpdates, []) + if (mergedMetadata !== userRecord.metadata) updates.metadata = mergedMetadata if (Object.keys(updates).length > 0) { await db.update(user).set(updates).where(eq(user.id, userRecord.id)) @@ -219,7 +238,7 @@ export const Route = createFileRoute('/api/widget/identify')({ email: identified.email, emailVerified: false, image: identified.avatarURL ?? null, - metadata: hasAttrs ? JSON.stringify(validAttrs) : null, + metadata: JSON.stringify(metadataUpdates), createdAt: new Date(), updatedAt: new Date(), }) @@ -230,6 +249,10 @@ export const Route = createFileRoute('/api/widget/identify')({ const userId = userRecord.id as UserId // Ensure principal record exists + principalRecord ??= await db.query.principal.findFirst({ + where: eq(principal.userId, userId), + }) + if (!principalRecord) { const [created] = await db .insert(principal)