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 7c4832bb5..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,7 +701,9 @@ 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}`) + console.log( + `[auth] magic-link request: ${request.method} ${url.pathname}${redactMagicLinkSearch(url.search)}` + ) } const authInstance = await getAuth() const response = await authInstance.handler(request) 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 diff --git a/apps/web/src/routes/api/widget/identify.ts b/apps/web/src/routes/api/widget/identify.ts index 37f37d865..95b171443 100644 --- a/apps/web/src/routes/api/widget/identify.ts +++ b/apps/web/src/routes/api/widget/identify.ts @@ -61,15 +61,10 @@ export function extractCustomClaims(payload: Record): Record> | undefined + let principalRecord: + | Awaited> + | undefined if (userRecord) { principalRecord = await db.query.principal.findFirst({ @@ -211,7 +208,6 @@ export const Route = createFileRoute('/api/widget/identify')({ !body.ssoToken && !canIssueUnverifiedWidgetSession({ role: principalRecord?.role, - type: principalRecord?.type, existingExternalId, assertedExternalId: identified.id, })