From df12ba6e9841de4f652a2c8ebc8750fd61e95ca3 Mon Sep 17 00:00:00 2001 From: Samyak Piya <76403666+samyakpiya@users.noreply.github.com> Date: Sat, 28 Dec 2024 05:47:14 -0500 Subject: [PATCH] Webhook Secret Field Implementation and Security Enhancements (#9187) (#9219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #9187 This pull request introduces a new feature and several enhancements for managing webhook security by adding a secret field and enabling HMAC signature-based authentication. Below is a detailed breakdown of the changes made: ## Frontend Updates ### Secret Field on Webhook Edit Page - Added a new **Secret** section on the webhook edit page. - Includes a text input field for entering a webhook secret. - Added a descriptive note explaining the purpose of the secret for webhook authentication. ### State Management and Persistence - Integrated the secret field into the Webhook type definition and state management. - Connected the secret field UI to the data layer, ensuring seamless persistence of the secret field. ### Validation Improvement - Trims leading and trailing whitespace from webhook secret inputs to avoid potential validation issues. ## Backend Updates ### Database and Entity Changes - Introduced a nullable `secret` field to the `WebhookWorkspaceEntity` for securely storing webhook signing secrets. - Field uses a standard field ID: `20202020-97ce-410f-bff9-e9ccb038fb67`. ### Signature Generation - Implemented HMAC-SHA256 signature generation for webhook payloads when a secret is present: - Signatures are added as a custom `X-Twenty-Webhook-Signature` header. - Secret is excluded from the payload to maintain security. ### Enhanced Security Measures - Added additional headers for enhanced security: - **Timestamp Header**: Prevents replay attacks. - **Nonce Header**: Mitigates duplicate requests. - Updated the OpenAPI specification to include documentation on these security-related headers and signature verification. ## Documentation Updates - Updated OpenAPI documentation for webhook endpoints: - Described security-related headers (signature, timestamp, nonce). - Included detailed instructions for verifying HMAC signatures to assist consumers. ## Testing and Demonstration - [Loom Video Link](https://www.loom.com/share/bd827e4d045f46d99f3c8186e5e5676a?sid=a5e61904-0536-4e82-8055-3d05e4598393): Demonstrating the functionality of the secret field and webhook security features. - [Script Example Link](https://runkit.com/samyakpiya/676af044040c0400086d400a): A script showing how consumers can verify webhook authenticity using the HMAC signature. - [Testing Site Instance](https://webhook.site/#!/view/3472468b-ebcd-4b7f-a083-c4ba20825bb4/6885fdce-8843-4d3f-8fe0-1d8abdd53f68/1): Contains the logged requests sent during testing and is available for review. ## Steps for Review 1. Verify the secret field functionality on the webhook edit page, including state persistence and UI updates. 2. Review the security enhancements, including header additions and HMAC signature generation. 3. Validate OpenAPI documentation changes for completeness and clarity. --------- Co-authored-by: Félix Malfait --- .../developers/types/webhook/Webhook.ts | 2 +- .../SettingsDevelopersWebhookDetail.tsx | 22 +++++++++- .../SettingsDevelopersWebhooksNew.tsx | 2 - .../open-api/utils/computeWebhooks.utils.ts | 42 +++++++++++++++++++ .../constants/standard-field-ids.ts | 1 + .../webhook/jobs/call-webhook-jobs.job.ts | 10 ++++- .../modules/webhook/jobs/call-webhook.job.ts | 36 +++++++++++++++- .../webhook.workspace-entity.ts | 10 +++++ .../remove-secret-from-webhook-record.ts | 10 +++++ 9 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 packages/twenty-server/src/utils/remove-secret-from-webhook-record.ts diff --git a/packages/twenty-front/src/modules/settings/developers/types/webhook/Webhook.ts b/packages/twenty-front/src/modules/settings/developers/types/webhook/Webhook.ts index 6e202c9a790..e837e941736 100644 --- a/packages/twenty-front/src/modules/settings/developers/types/webhook/Webhook.ts +++ b/packages/twenty-front/src/modules/settings/developers/types/webhook/Webhook.ts @@ -2,7 +2,7 @@ export type Webhook = { id: string; targetUrl: string; description?: string; - operation: string; operations: string[]; + secret?: string; __typename: 'Webhook'; }; diff --git a/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx b/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx index 86a9bd0eedf..f9dde7fc24a 100644 --- a/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx +++ b/packages/twenty-front/src/pages/settings/developers/webhooks/components/SettingsDevelopersWebhookDetail.tsx @@ -1,3 +1,4 @@ +import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; import styled from '@emotion/styled'; import { useMemo, useState } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; @@ -14,7 +15,6 @@ import { Section, useIcons, } from 'twenty-ui'; -import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; import { AnalyticsActivityGraph } from '@/analytics/components/AnalyticsActivityGraph'; import { AnalyticsGraphEffect } from '@/analytics/components/AnalyticsGraphEffect'; @@ -74,6 +74,7 @@ export const SettingsDevelopersWebhooksDetail = () => { const [operations, setOperations] = useState([ WEBHOOK_EMPTY_OPERATION, ]); + const [secret, setSecret] = useState(''); const [isDirty, setIsDirty] = useState(false); const { getIcon } = useIcons(); @@ -97,6 +98,7 @@ export const SettingsDevelopersWebhooksDetail = () => { : []; setOperations(addEmptyOperationIfNecessary(baseOperations)); + setSecret(data?.secret ?? ''); setIsDirty(false); }, }); @@ -153,9 +155,9 @@ export const SettingsDevelopersWebhooksDetail = () => { await updateOneRecord({ idToUpdate: webhookId, updateOneRecordInput: { - operation: cleanedOperations?.[0], operations: cleanedOperations, description: description, + secret: secret, }, }); navigate(developerPath); @@ -291,6 +293,22 @@ export const SettingsDevelopersWebhooksDetail = () => { ))} +
+ + { + setSecret(secret.trim()); + setIsDirty(true); + }} + fullWidth + /> +
{isAnalyticsEnabled && isAnalyticsV2Enabled && ( { const navigate = useNavigate(); const [formValues, setFormValues] = useState<{ targetUrl: string; - operation: string; operations: string[]; }>({ targetUrl: '', - operation: '*.*', operations: ['*.*'], }); const [isTargetUrlValid, setIsTargetUrlValid] = useState(true); diff --git a/packages/twenty-server/src/engine/core-modules/open-api/utils/computeWebhooks.utils.ts b/packages/twenty-server/src/engine/core-modules/open-api/utils/computeWebhooks.utils.ts index 8f7dd097750..d48736a8c16 100644 --- a/packages/twenty-server/src/engine/core-modules/open-api/utils/computeWebhooks.utils.ts +++ b/packages/twenty-server/src/engine/core-modules/open-api/utils/computeWebhooks.utils.ts @@ -19,6 +19,48 @@ export const computeWebhooks = ( post: { tags: [item.nameSingular], security: [], + parameters: [ + { + in: 'header', + name: 'X-Twenty-Webhook-Signature', + schema: { + type: 'string', + }, + description: + 'HMAC SHA256 signature of the request payload using the webhook secret. To compute the signature:\n' + + '1. Concatenate `X-Twenty-Webhook-Timestamp`, a colon (:), and the JSON string of the request payload.\n' + + '2. Compute the HMAC SHA256 hash using the shared secret as the key.\n' + + '3. Send the resulting hex digest as this header value.\n' + + 'Example (Node.js):\n```javascript\n' + + 'const crypto = require("crypto");\n' + + 'const timestamp = "1735066639761";\n' + + 'const payload = JSON.stringify({...});\n' + + 'const secret = "your-secret";\n' + + 'const stringToSign = `${timestamp}:${JSON.stringify(payload)}`;\n' + + 'const signature = crypto.createHmac("sha256", secret)\n .update(stringToSign)\n .digest("hex");\n```', + required: false, + }, + { + in: 'header', + name: 'X-Twenty-Webhook-Timestamp', + schema: { + type: 'string', + }, + description: + 'Unix timestamp of when the webhook was sent. This timestamp is included in the HMAC signature generation to prevent replay attacks.', + required: false, + }, + { + in: 'header', + name: 'X-Twenty-Webhook-Nonce', + schema: { + type: 'string', + }, + description: + 'Unique identifier for this webhook request to prevent replay attacks. Consumers should ensure this nonce is not reused.', + required: false, + }, + ], requestBody: { content: { 'application/json': { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts index 462c3748ddb..48a5f85bf92 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts @@ -439,6 +439,7 @@ export const WEBHOOK_STANDARD_FIELD_IDS = { operation: '20202020-15b7-458e-bf30-74770a54410c', operations: '20202020-15b7-458e-bf30-74770a54411c', description: '20202020-15b7-458e-bf30-74770a54410d', + secret: '20202020-97ce-410f-bff9-e9ccb038fb67', }; export const WORKFLOW_EVENT_LISTENER_STANDARD_FIELD_IDS = { diff --git a/packages/twenty-server/src/modules/webhook/jobs/call-webhook-jobs.job.ts b/packages/twenty-server/src/modules/webhook/jobs/call-webhook-jobs.job.ts index a20855be530..20c7c78802d 100644 --- a/packages/twenty-server/src/modules/webhook/jobs/call-webhook-jobs.job.ts +++ b/packages/twenty-server/src/modules/webhook/jobs/call-webhook-jobs.job.ts @@ -15,6 +15,7 @@ import { CallWebhookJob, CallWebhookJobData, } from 'src/modules/webhook/jobs/call-webhook.job'; +import { removeSecretFromWebhookRecord } from 'src/utils/remove-secret-from-webhook-record'; @Processor(MessageQueue.webhookQueue) export class CallWebhookJobsJob { @@ -62,15 +63,22 @@ export class CallWebhookJobsJob { const record = eventData.properties.after || eventData.properties.before; const updatedFields = eventData.properties.updatedFields; + const isWebhookEvent = nameSingular === 'webhook'; + const sanitizedRecord = removeSecretFromWebhookRecord( + record, + isWebhookEvent, + ); + webhooks.forEach((webhook) => { const webhookData = { targetUrl: webhook.targetUrl, + secret: webhook.secret, eventName, objectMetadata, workspaceId, webhookId: webhook.id, eventDate: new Date(), - record, + record: sanitizedRecord, ...(updatedFields && { updatedFields }), }; diff --git a/packages/twenty-server/src/modules/webhook/jobs/call-webhook.job.ts b/packages/twenty-server/src/modules/webhook/jobs/call-webhook.job.ts index 31b72988d52..74ce5f42377 100644 --- a/packages/twenty-server/src/modules/webhook/jobs/call-webhook.job.ts +++ b/packages/twenty-server/src/modules/webhook/jobs/call-webhook.job.ts @@ -1,6 +1,8 @@ import { HttpService } from '@nestjs/axios'; import { Logger } from '@nestjs/common'; +import crypto from 'crypto'; + import { AnalyticsService } from 'src/engine/core-modules/analytics/analytics.service'; import { Process } from 'src/engine/core-modules/message-queue/decorators/process.decorator'; import { Processor } from 'src/engine/core-modules/message-queue/decorators/processor.decorator'; @@ -15,6 +17,7 @@ export type CallWebhookJobData = { eventDate: Date; record: any; updatedFields?: string[]; + secret?: string; }; @Processor(MessageQueue.webhookQueue) @@ -25,6 +28,17 @@ export class CallWebhookJob { private readonly analyticsService: AnalyticsService, ) {} + private generateSignature( + payload: CallWebhookJobData, + secret: string, + timestamp: string, + ): string { + return crypto + .createHmac('sha256', secret) + .update(`${timestamp}:${JSON.stringify(payload)}`) + .digest('hex'); + } + @Process(CallWebhookJob.name) async handle(data: CallWebhookJobData): Promise { const commonPayload = { @@ -34,10 +48,30 @@ export class CallWebhookJob { }; try { + const headers: Record = { + 'Content-Type': 'application/json', + }; + + const { secret, ...payloadWithoutSecret } = data; + + if (secret) { + headers['X-Twenty-Webhook-Timestamp'] = Date.now().toString(); + headers['X-Twenty-Webhook-Signature'] = this.generateSignature( + payloadWithoutSecret, + secret, + headers['X-Twenty-Webhook-Timestamp'], + ); + headers['X-Twenty-Webhook-Nonce'] = crypto + .randomBytes(16) + .toString('hex'); + } + const response = await this.httpService.axiosRef.post( data.targetUrl, - data, + payloadWithoutSecret, + { headers }, ); + const success = response.status >= 200 && response.status < 300; const eventInput = { action: 'webhook.response', diff --git a/packages/twenty-server/src/modules/webhook/standard-objects/webhook.workspace-entity.ts b/packages/twenty-server/src/modules/webhook/standard-objects/webhook.workspace-entity.ts index 6c003cb15fc..ee55e07da21 100644 --- a/packages/twenty-server/src/modules/webhook/standard-objects/webhook.workspace-entity.ts +++ b/packages/twenty-server/src/modules/webhook/standard-objects/webhook.workspace-entity.ts @@ -60,4 +60,14 @@ export class WebhookWorkspaceEntity extends BaseWorkspaceEntity { }) @WorkspaceIsNullable() description: string; + + @WorkspaceField({ + standardId: WEBHOOK_STANDARD_FIELD_IDS.secret, + type: FieldMetadataType.TEXT, + label: 'Secret', + description: + 'Optional secret used to compute the HMAC signature for webhook payloads. This secret is shared between Twenty and the webhook consumer to authenticate webhook requests.', + icon: 'IconLock', + }) + secret: string; } diff --git a/packages/twenty-server/src/utils/remove-secret-from-webhook-record.ts b/packages/twenty-server/src/utils/remove-secret-from-webhook-record.ts new file mode 100644 index 00000000000..3d1fbe0996b --- /dev/null +++ b/packages/twenty-server/src/utils/remove-secret-from-webhook-record.ts @@ -0,0 +1,10 @@ +export const removeSecretFromWebhookRecord = ( + record: Record | undefined, + isWebhookEvent: boolean, +): Record | undefined => { + if (!isWebhookEvent || !record) return record; + + const { secret: _secret, ...sanitizedRecord } = record; + + return sanitizedRecord; +};