diff --git a/.changeset/chubby-tires-end.md b/.changeset/chubby-tires-end.md new file mode 100644 index 00000000000..8d9fc37bc9f --- /dev/null +++ b/.changeset/chubby-tires-end.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Refactor webhook verification to use verification from the `standardwebhooks` package, which is what our underlying provider relies on. diff --git a/packages/backend/package.json b/packages/backend/package.json index beb4bc51ddd..0f1d42b58a7 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -111,6 +111,7 @@ "@clerk/types": "workspace:^", "cookie": "1.0.2", "snakecase-keys": "8.0.1", + "standardwebhooks": "^1.0.0", "tslib": "catalog:repo" }, "devDependencies": { diff --git a/packages/backend/src/__tests__/webhooks.test.ts b/packages/backend/src/__tests__/webhooks.test.ts index 25345f2e371..7eb2ee15367 100644 --- a/packages/backend/src/__tests__/webhooks.test.ts +++ b/packages/backend/src/__tests__/webhooks.test.ts @@ -1,39 +1,45 @@ +import { Webhook } from 'standardwebhooks'; import { beforeEach, describe, expect, it } from 'vitest'; import { verifyWebhook } from '../webhooks'; describe('verifyWebhook', () => { - const mockSecret = 'test_signing_secret'; + const mockSecret = 'whsec_' + Buffer.from('test_signing_secret_32_chars_long').toString('base64'); const mockBody = JSON.stringify({ type: 'user.created', data: { id: 'user_123' } }); beforeEach(() => { process.env.CLERK_WEBHOOK_SIGNING_SECRET = mockSecret; }); + // Helper function to create a valid signature with Standard Webhooks + const createValidSignature = (id: string, timestamp: string, body: string) => { + const webhook = new Webhook(mockSecret); + // Create a signature using the Standard Webhooks library + return webhook.sign(id, new Date(parseInt(timestamp) * 1000), body); + }; + it('throws when required headers are missing', async () => { const mockRequest = new Request('https://clerk.com/webhooks', { method: 'POST', body: mockBody, headers: new Headers({ - // Missing svix-signature but with valid format for others - 'svix-id': 'msg_123', - 'svix-timestamp': '1614265330', + // Missing all required headers }), }); - await expect(verifyWebhook(mockRequest)).rejects.toThrow('Missing required Svix headers: svix-signature'); + await expect(verifyWebhook(mockRequest)).rejects.toThrow('Missing required webhook headers'); }); it('throws with all missing headers in error message', async () => { const mockRequest = new Request('https://clerk.com/webhooks', { method: 'POST', body: mockBody, - headers: new Headers({}), + headers: new Headers({ + // Missing all required headers + }), }); - await expect(verifyWebhook(mockRequest)).rejects.toThrow( - 'Missing required Svix headers: svix-id, svix-timestamp, svix-signature', - ); + await expect(verifyWebhook(mockRequest)).rejects.toThrow('svix-id, svix-timestamp, svix-signature'); }); it('throws when signing secret is missing', async () => { @@ -44,24 +50,26 @@ describe('verifyWebhook', () => { body: mockBody, headers: new Headers({ 'svix-id': 'msg_123', - 'svix-timestamp': '1614265330', + 'svix-timestamp': (Date.now() / 1000).toString(), 'svix-signature': 'v1,test_signature', }), }); - await expect(verifyWebhook(mockRequest)).rejects.toThrow( - 'Missing webhook signing secret. Set the CLERK_WEBHOOK_SIGNING_SECRET environment variable with the webhook secret from the Clerk Dashboard.', - ); + await expect(verifyWebhook(mockRequest)).rejects.toThrow('Missing webhook signing secret'); }); it('validates webhook request requirements', async () => { + const svixId = 'msg_123'; + const svixTimestamp = (Date.now() / 1000).toString(); + const validSignature = createValidSignature(svixId, svixTimestamp, mockBody); + const mockRequest = new Request('https://clerk.com/webhooks', { method: 'POST', body: mockBody, headers: new Headers({ - 'svix-id': 'msg_123', - 'svix-timestamp': '1614265330', - 'svix-signature': 'v1,test_signature', + 'svix-id': svixId, + 'svix-timestamp': svixTimestamp, + 'svix-signature': validSignature, }), }); @@ -72,4 +80,141 @@ describe('verifyWebhook', () => { expect(result).toHaveProperty('type', 'user.created'); expect(result).toHaveProperty('data.id', 'user_123'); }); + + it('should accept valid signatures', async () => { + const svixId = 'msg_123'; + const svixTimestamp = (Date.now() / 1000).toString(); + const validSignature = createValidSignature(svixId, svixTimestamp, mockBody); + + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': svixId, + 'svix-timestamp': svixTimestamp, + 'svix-signature': validSignature, + }), + }); + + // Should accept and return parsed data + const result = await verifyWebhook(mockRequest); + expect(result).toHaveProperty('type', 'user.created'); + expect(result).toHaveProperty('data.id', 'user_123'); + }); + + it('should reject invalid signatures', async () => { + const svixId = 'msg_123'; + const svixTimestamp = (Date.now() / 1000).toString(); + const invalidSignature = 'v1,invalid_signature_here'; + + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': svixId, + 'svix-timestamp': svixTimestamp, + 'svix-signature': invalidSignature, + }), + }); + + // Should reject invalid signatures + await expect(verifyWebhook(mockRequest)).rejects.toThrow('No matching signature found'); + }); + + it('should handle multiple signatures in header', async () => { + const svixId = 'msg_123'; + const svixTimestamp = (Date.now() / 1000).toString(); + const validSignature = createValidSignature(svixId, svixTimestamp, mockBody); + const invalidSignature = 'v1,invalid_signature'; + + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': svixId, + 'svix-timestamp': svixTimestamp, + 'svix-signature': `${invalidSignature} ${validSignature}`, + }), + }); + + // Should accept if any signature in the list is valid + const result = await verifyWebhook(mockRequest); + expect(result).toHaveProperty('type', 'user.created'); + expect(result).toHaveProperty('data.id', 'user_123'); + }); + + it('should handle signatures without version prefixes for backward compatibility', async () => { + const svixId = 'msg_123'; + const svixTimestamp = (Date.now() / 1000).toString(); + // Test with Standard Webhooks generated signature without custom prefix + const validSignature = createValidSignature(svixId, svixTimestamp, mockBody); + + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': svixId, + 'svix-timestamp': svixTimestamp, + 'svix-signature': validSignature, + }), + }); + + // Should accept signatures without version prefixes + const result = await verifyWebhook(mockRequest); + expect(result).toHaveProperty('type', 'user.created'); + expect(result).toHaveProperty('data.id', 'user_123'); + }); + + it('should verify against Standard Webhooks specification', async () => { + // Test with proper Clerk webhook format + const clerkPayload = '{"type":"user.created","data":{"id":"user_123","email":"test@example.com"}}'; + const msgId = 'msg_test123'; + const timestamp = (Date.now() / 1000).toString(); + + const validSignature = createValidSignature(msgId, timestamp, clerkPayload); + + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: clerkPayload, + headers: new Headers({ + 'svix-id': msgId, + 'svix-timestamp': timestamp, + 'svix-signature': validSignature, + }), + }); + + const result = await verifyWebhook(mockRequest, { signingSecret: mockSecret }); + expect(result).toHaveProperty('type', 'user.created'); + expect(result).toHaveProperty('data.id', 'user_123'); + }); + + it('should handle whitespace-only header values correctly', async () => { + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': '', // Empty - should be caught + 'svix-timestamp': ' ', // Whitespace - should be caught + 'svix-signature': 'v1,signature', + }), + }); + + // This should fail because whitespace-only headers should be treated as missing + await expect(verifyWebhook(mockRequest)).rejects.toThrow('Missing required webhook headers'); + }); + + it('should handle mixed empty and whitespace headers correctly', async () => { + const mockRequest = new Request('https://clerk.com/webhooks', { + method: 'POST', + body: mockBody, + headers: new Headers({ + 'svix-id': ' \t ', // Mixed whitespace and tabs + 'svix-timestamp': '\n', // Newline character + 'svix-signature': '', // Empty string + }), + }); + + // All should be treated as missing + await expect(verifyWebhook(mockRequest)).rejects.toThrow('svix-id, svix-timestamp, svix-signature'); + }); }); diff --git a/packages/backend/src/webhooks.ts b/packages/backend/src/webhooks.ts index 273477dae50..280bff5b918 100644 --- a/packages/backend/src/webhooks.ts +++ b/packages/backend/src/webhooks.ts @@ -1,6 +1,6 @@ import { getEnvVariable } from '@clerk/shared/getEnvVariable'; -import crypto from 'crypto'; import { errorThrower } from 'src/util/shared'; +import { Webhook } from 'standardwebhooks'; import type { WebhookEvent } from './api/resources/Webhooks'; @@ -14,16 +14,44 @@ export type VerifyWebhookOptions = { signingSecret?: string; }; +// Standard Webhooks header names +const STANDARD_WEBHOOK_ID_HEADER = 'webhook-id'; +const STANDARD_WEBHOOK_TIMESTAMP_HEADER = 'webhook-timestamp'; +const STANDARD_WEBHOOK_SIGNATURE_HEADER = 'webhook-signature'; + +// Svix header names (for mapping) const SVIX_ID_HEADER = 'svix-id'; const SVIX_TIMESTAMP_HEADER = 'svix-timestamp'; const SVIX_SIGNATURE_HEADER = 'svix-signature'; -const REQUIRED_SVIX_HEADERS = [SVIX_ID_HEADER, SVIX_TIMESTAMP_HEADER, SVIX_SIGNATURE_HEADER] as const; - export * from './api/resources/Webhooks'; /** - * Verifies the authenticity of a webhook request using Svix. Returns a promise that resolves to the verified webhook event data. + * Maps Svix headers to Standard Webhooks headers for compatibility + */ +function createStandardWebhookHeaders(request: Request): Record { + const headers: Record = {}; + + // Map Svix headers to Standard Webhooks headers + const svixId = request.headers.get(SVIX_ID_HEADER)?.trim(); + const svixTimestamp = request.headers.get(SVIX_TIMESTAMP_HEADER)?.trim(); + const svixSignature = request.headers.get(SVIX_SIGNATURE_HEADER)?.trim(); + + if (svixId) { + headers[STANDARD_WEBHOOK_ID_HEADER] = svixId; + } + if (svixTimestamp) { + headers[STANDARD_WEBHOOK_TIMESTAMP_HEADER] = svixTimestamp; + } + if (svixSignature) { + headers[STANDARD_WEBHOOK_SIGNATURE_HEADER] = svixSignature; + } + + return headers; +} + +/** + * Verifies the authenticity of a webhook request using Standard Webhooks. Returns a promise that resolves to the verified webhook event data. * * @param request - The request object. * @param options - Optional configuration object. @@ -56,9 +84,6 @@ export * from './api/resources/Webhooks'; */ export async function verifyWebhook(request: Request, options: VerifyWebhookOptions = {}): Promise { const secret = options.signingSecret ?? getEnvVariable('CLERK_WEBHOOK_SIGNING_SECRET'); - const svixId = request.headers.get(SVIX_ID_HEADER); - const svixTimestamp = request.headers.get(SVIX_TIMESTAMP_HEADER); - const svixSignature = request.headers.get(SVIX_SIGNATURE_HEADER); if (!secret) { return errorThrower.throw( @@ -66,29 +91,46 @@ export async function verifyWebhook(request: Request, options: VerifyWebhookOpti ); } - if (!svixId || !svixTimestamp || !svixSignature) { - const missingHeaders = REQUIRED_SVIX_HEADERS.filter(header => !request.headers.has(header)); - return errorThrower.throw(`Missing required Svix headers: ${missingHeaders.join(', ')}`); + // Check for required Svix headers + const webhookId = request.headers.get(SVIX_ID_HEADER)?.trim(); + const webhookTimestamp = request.headers.get(SVIX_TIMESTAMP_HEADER)?.trim(); + const webhookSignature = request.headers.get(SVIX_SIGNATURE_HEADER)?.trim(); + + if (!webhookId || !webhookTimestamp || !webhookSignature) { + const missingHeaders = []; + + if (!webhookId) { + missingHeaders.push(SVIX_ID_HEADER); + } + if (!webhookTimestamp) { + missingHeaders.push(SVIX_TIMESTAMP_HEADER); + } + if (!webhookSignature) { + missingHeaders.push(SVIX_SIGNATURE_HEADER); + } + + return errorThrower.throw(`Missing required webhook headers: ${missingHeaders.join(', ')}`); } const body = await request.text(); - const signedContent = `${svixId}.${svixTimestamp}.${body}`; + // Create Standard Webhooks compatible headers mapping + const standardHeaders = createStandardWebhookHeaders(request); - const secretBytes = Buffer.from(secret.split('_')[1], 'base64'); + // Initialize Standard Webhooks verifier + const webhook = new Webhook(secret); - const constructedSignature = crypto.createHmac('sha256', secretBytes).update(signedContent).digest('base64'); + try { + // Verify using Standard Webhooks - this provides constant-time comparison + // and proper signature format handling + const payload = webhook.verify(body, standardHeaders) as Record; - // svixSignature can be a string with one or more space separated signatures - if (svixSignature.split(' ').includes(constructedSignature)) { - return errorThrower.throw('Incoming webhook does not have a valid signature'); + return { + type: payload.type, + object: 'event', + data: payload.data, + } as WebhookEvent; + } catch (e) { + return errorThrower.throw(`Unable to verify incoming webhook: ${e instanceof Error ? e.message : 'Unknown error'}`); } - - const payload = JSON.parse(body); - - return { - type: payload.type, - object: 'event', - data: payload.data, - } as WebhookEvent; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fbddbf561ad..bfbc77d78fc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -378,6 +378,9 @@ importers: snakecase-keys: specifier: 8.0.1 version: 8.0.1 + standardwebhooks: + specifier: ^1.0.0 + version: 1.0.0 tslib: specifier: catalog:repo version: 2.8.1 @@ -4356,6 +4359,9 @@ packages: '@speed-highlight/core@1.2.7': resolution: {integrity: sha512-0dxmVj4gxg3Jg879kvFS/msl4s9F3T9UXC1InxgOf7t5NvcPD97u/WTA5vL/IxWHMn7qSxBozqrnnE2wvl1m8g==} + '@stablelib/base64@1.0.1': + resolution: {integrity: sha512-1bnPQqSxSuc3Ii6MhBysoWCg58j97aUjuCSZrGSmDxNqtytIi0k8utUenAwTZN4V5mXXYGsVUI9zeBqy+jBOSQ==} + '@statelyai/inspect@0.4.0': resolution: {integrity: sha512-VxQldRlKYcu6rzLY83RSXVwMYexkH6hNx85B89YWYyXYWtNGaWHFCwV7a/Kz8FFPeUz8EKVAnyMOg2kNpn07wQ==} peerDependencies: @@ -8103,6 +8109,9 @@ packages: fast-safe-stringify@2.1.1: resolution: {integrity: sha512-W+KJc2dmILlPplD/H4K9l9LcAHAfPtP6BY84uVLXQ6Evcz9Lcg33Y2z1IVblT6xdY54PXYVHEv+0Wpq8Io6zkA==} + fast-sha256@1.3.0: + resolution: {integrity: sha512-n11RGP/lrWEFI/bWdygLxhI+pVeo1ZYIVwvvPkW7azl/rOy+F3HYRZ2K5zeE9mmkhQppyv9sQFx0JM9UabnpPQ==} + fast-uri@2.4.0: resolution: {integrity: sha512-ypuAmmMKInk5q7XcepxlnUWDLWv4GFtaJqAzWKqn62IpQ3pejtr5dTVbt3vwqVaMKmkNR55sTT+CqUKIaT21BA==} @@ -12875,6 +12884,9 @@ packages: standard-as-callback@2.1.0: resolution: {integrity: sha512-qoRRSyROncaz1z0mvYqIE4lCd9p2R90i6GxW3uZv5ucSu8tU7B5HXUP1gG8pVZsYNVaXjk8ClXHPttLyxAL48A==} + standardwebhooks@1.0.0: + resolution: {integrity: sha512-BbHGOQK9olHPMvQNHWul6MYlrRTAOKn03rOe4A8O3CLWhNf4YHBqq2HJKKC+sfqpxiBY52pNeesD6jIiLDz8jg==} + statuses@1.5.0: resolution: {integrity: sha512-OpZ3zP+jT1PI7I8nemJX4AKmAX070ZkYPVWV/AaKTJl+tXCTGyVdC1a4SL8RUQYEwk/f34ZX8UTykN68FwrqAA==} engines: {node: '>= 0.6'} @@ -18953,6 +18965,8 @@ snapshots: '@speed-highlight/core@1.2.7': {} + '@stablelib/base64@1.0.1': {} + '@statelyai/inspect@0.4.0(ws@8.18.2)(xstate@5.19.4)': dependencies: fast-safe-stringify: 2.1.1 @@ -23796,6 +23810,8 @@ snapshots: fast-safe-stringify@2.1.1: {} + fast-sha256@1.3.0: {} + fast-uri@2.4.0: {} fast-uri@3.0.3: {} @@ -29673,6 +29689,11 @@ snapshots: standard-as-callback@2.1.0: {} + standardwebhooks@1.0.0: + dependencies: + '@stablelib/base64': 1.0.1 + fast-sha256: 1.3.0 + statuses@1.5.0: {} statuses@2.0.1: {}