Cleanded up commit history

package.json

@@ -114,6 +114,7 @@
     "@types/convert-source-map": "^2.0.3",
     "@types/css-tree": "^3.2.0",
     "@types/eslint-config-prettier": "^6.11.3",
+    "@types/estree": "^1.0.9",
     "@types/gtag.js": "^0.0.20",
     "@types/is-url": "^1.2.32",
     "@types/jquery": "^4.0.0",

web/src/portico/integrations_dev_panel.ts

@@ -231,6 +231,7 @@ function update_url(): void {
                 params.set("topic", topic_name);
             }
         }
+        const webhook_secret = $<HTMLInputElement>("input#webhook_secret").val()!;
         const url = `${url_base}${integration_name}?${params.toString()}`;
         url_field!.value = url;
     }

web/src/settings_bots.ts

@@ -12,7 +12,8 @@ import type {Bot} from "./bot_data.ts";
 import * as bot_helper from "./bot_helper.ts";
 import {
     EMBEDDED_BOT_TYPE,
-    GENERIC_BOT_TYPE_INT,
+    GENERIC_BOT_TYPE,
+    INCOMING_WEBHOOK_BOT_TYPE,
     INCOMING_WEBHOOK_BOT_TYPE_INT,
     OUTGOING_WEBHOOK_BOT_TYPE,
     OUTGOING_WEBHOOK_BOT_TYPE_INT,
@@ -298,6 +299,20 @@ export function add_a_new_bot(): void {
                 formData.append("interface_type", interface_type);
                 break;
             }
+            case INCOMING_WEBHOOK_BOT_TYPE: {
+                const config_data: Record<string, string> = {};
+                $<HTMLInputElement>("#webhook_secret_inputbox input").each(function () {
+                    const key = $(this).attr("name")!;
+                    const value = $(this).val()?.trim();
+                    if (value) {
+                        config_data[key] = value;
+                    }
+                });
+                if (Object.keys(config_data).length > 0) {
+                    formData.append("config_data", JSON.stringify(config_data));
+                }
+                break;
+            }
             case EMBEDDED_BOT_TYPE: {
                 formData.append("service_name", service_name);
                 const config_data: Record<string, string> = {};
@@ -336,7 +351,8 @@ export function add_a_new_bot(): void {
     }
 
     function set_up_form_fields(): void {
-        $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE_INT);
+        $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change");
+        $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change");
         $("#payload_url_inputbox").hide();
         $("#create_payload_url").val("");
         $("#service_name_list").hide();
@@ -362,7 +378,18 @@ export function add_a_new_bot(): void {
 
             $("#payload_url_inputbox").hide();
             $("#create_payload_url").removeClass("required");
+
+            $("#webhook_secret_inputbox").hide();
+
             switch (bot_type) {
+                case INCOMING_WEBHOOK_BOT_TYPE: {
+                    $("#webhook_secret_inputbox").show();
+                    break;
+                }
+                case INCOMING_WEBHOOK_BOT_TYPE: {
+                    $("#webhook_secret_inputbox").show();
+                    break;
+                }
                 case OUTGOING_WEBHOOK_BOT_TYPE: {
                     $("#payload_url_inputbox").show();
                     $("#create_payload_url").addClass("required");
@@ -377,7 +404,8 @@ export function add_a_new_bot(): void {
                 }
             }
         });
-
+        $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change");
+        $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change");
         $("#select_service_name").on("change", () => {
             $("#config_inputbox").children().hide();
             const selected_bot = $<HTMLSelectOneElement>(

web/templates/settings/add_new_bot_form.hbs

@@ -53,6 +53,13 @@
                 <div><label for="create_interface_type" generated="true" class="text-error"></label></div>
             </div>
         </div>
+        <div id="webhook_secret_inputbox">
+            <div class="input-group">
+                <label for="create_webhook_secret" class="modal-field-label">{{t "Webhook secret (optional)" }}</label>
+                <input type="password" name="webhook_secret" id="create_webhook_secret" class="modal_text_input" placeholder="{{t 'ZulipSecret123' }}" value=""/>
+                <div><label for="create_webhook_secret" generated="true" class="text-error"></label></div>
+            </div>
+        </div>
         <div id="config_inputbox">
             {{#each realm_embedded_bots}}
                 {{#each (object_entries config) as |entry|}}

web/templates/settings/edit_bot_form.hbs

@@ -41,6 +41,12 @@
 
         <div id="service_data">
         </div>
+        {{#if is_incoming_webhook_bot}}
+        <div class="input-group edit_bot_webhook_secret_container">
+            <label for="edit_webhook_secret" class="modal-field-label">{{t "Webhook secret" }}</label>
+            <input type="password" name="webhook_secret" id="edit_webhook_secret" class="modal_text_input" placeholder="{{t 'Leave blank to keep current secret' }}" value=""/>
+        </div>
+        {{/if}}
         <div class="input-group edit-avatar-section">
             <label class="modal-field-label">{{t "Avatar" }}</label>
             {{!-- Shows the current avatar --}}
@@ -66,16 +72,6 @@
             <div><label for="edit_bot_avatar_file" generated="true" class="edit_bot_avatar_error text-error"></label></div>
         </div>
     </form>
-    {{#if is_incoming_webhook_bot}}
-    <div class="input-group">
-        {{> ../components/action_button
-          label=(t "Generate URL for an integration")
-          variant="subtle"
-          intent="neutral"
-          custom_classes="generate_url_for_integration"
-          }}
-    </div>
-    {{/if}}
     {{#if (and is_active is_bot_owner_current_user)}}
     <div id="zuliprc-section" class="input-group">
         <div class="zuliprc-container">

zerver/decorator.py

@@ -54,7 +54,9 @@
 from zerver.lib.utils import has_api_key_format
 from zerver.lib.webhooks.common import (
     MissingHTTPEventHeaderError,
+    WebhookSignatureConfig,
     notify_bot_owner_about_invalid_json,
+    validate_webhook_signature,
 )
 from zerver.models import UserProfile
 from zerver.models.clients import get_client
@@ -372,6 +374,7 @@ def webhook_view(
     webhook_client_name: str,
     notify_bot_owner_on_invalid_json: bool = True,
     all_event_types: Sequence[str] | None = None,
+    signature_config: WebhookSignatureConfig | None = None,
 ) -> Callable[[Callable[..., HttpResponse]], Callable[..., HttpResponse]]:
     # Unfortunately, callback protocols are insufficient for this:
     # https://mypy.readthedocs.io/en/stable/protocols.html#callback-protocols
@@ -390,6 +393,11 @@ def _wrapped_func_arguments(
                 allow_webhook_access=True,
                 client_name=full_webhook_client_name(webhook_client_name),
             )
+            validate_webhook_signature(
+                request,
+                user_profile,
+                signature_config,
+            )
 
             request_notes = RequestNotes.get_notes(request)
             request_notes.is_webhook_view = True

zerver/lib/integrations.py

@@ -12,7 +12,12 @@
 from typing_extensions import override
 
 from zerver.lib.storage import static_path
-from zerver.lib.webhooks.common import PresetUrlOption, WebhookConfigOption, WebhookUrlOption
+from zerver.lib.webhooks.common import (
+    PresetUrlOption,
+    WebhookConfigOption,
+    WebhookSignatureConfig,
+    WebhookUrlOption,
+)
 from zerver.webhooks import fixtureless_integrations
 
 """This module declares all of the (documented) integrations available
@@ -1196,6 +1201,15 @@ def is_enabled_in_catalog(self) -> bool:
     | hubot_integration_names
 )
 
+WEBHOOK_SIGNATURE_CONFIGS: dict[str, WebhookSignatureConfig] = {
+    "github": WebhookSignatureConfig(
+        integration_name="github",
+        header="X_HUB_SIGNATURE_256",
+        algorithm="sha256",
+        prefix="sha256=",
+    ),
+}
+
 # Add integrations that are not meant to have example screenshots here
 INTEGRATIONS_WITHOUT_SCREENSHOTS = (
     # Integration frameworks

zerver/lib/test_classes.py

@@ -35,6 +35,7 @@
 from django.test.testcases import SerializeMixin
 from django.urls import resolve
 from django.utils import translation
+from django.utils.encoding import force_bytes
 from django.utils.module_loading import import_string
 from django.utils.timezone import now as timezone_now
 from fakeldap import MockLDAP
@@ -54,9 +55,11 @@
 from zerver.actions.user_settings import do_change_full_name, do_change_user_setting
 from zerver.actions.users import do_change_user_role
 from zerver.decorator import do_two_factor_login
+from zerver.lib.bot_config import set_bot_config
 from zerver.lib.cache import bounce_key_prefix_for_testing
 from zerver.lib.email_notifications import MissedMessageData, handle_missedmessage_emails
 from zerver.lib.initial_password import initial_password
+from zerver.lib.integrations import WEBHOOK_SIGNATURE_CONFIGS
 from zerver.lib.mdiff import diff_strings
 from zerver.lib.message import access_message
 from zerver.lib.notification_data import UserMessageNotificationsData
@@ -92,8 +95,10 @@
 from zerver.lib.upload import upload_message_attachment_from_request
 from zerver.lib.user_groups import get_system_user_group_for_user
 from zerver.lib.webhooks.common import (
+    WEBHOOK_SECRET_TOKEN_KEY,
     call_fixture_to_headers,
     check_send_webhook_message,
+    compute_webhook_signature,
     standardize_headers,
 )
 from zerver.models import (
@@ -2547,6 +2552,8 @@ class WebhookTestCase(ZulipTestCase):
     DEFAULT_URL_TEMPLATE: str = (
         "/api/v1/external/{webhook_dir_name}?stream={stream}&api_key={api_key}"
     )
+    WEBHOOK_TEST_SECRET: str | None = None
+    VERIFY_WEBHOOK_SIGNATURES: bool = True
 
     def get_webhook_dir_name(self) -> str:
         module_parts = self.__module__.split(".")
@@ -2563,6 +2570,19 @@ def setUp(self) -> None:
         self.url_template = self.URL_TEMPLATE or self.DEFAULT_URL_TEMPLATE
         self.url = self.build_webhook_url()
 
+        if self.WEBHOOK_TEST_SECRET is not None:
+            config = WEBHOOK_SIGNATURE_CONFIGS.get(self.webhook_dir_name.lower())
+            if config is None:
+                raise AssertionError(
+                    f"WEBHOOK_TEST_SECRET was set for  '{self.webhook_dir_name}', "
+                    f"but no WebhookSignatureConfig is registered in WEBHOOK_SIGNATURE_CONFIGS."
+                )
+            set_bot_config(
+                self.test_user,
+                WEBHOOK_SECRET_TOKEN_KEY.format(integration_name=self.webhook_dir_name.lower()),
+                self.WEBHOOK_TEST_SECRET,
+            )
+
         function = import_string(
             f"zerver.webhooks.{self.webhook_dir_name}.view.api_{self.webhook_dir_name}_webhook"
         )
@@ -2653,26 +2673,42 @@ def check_webhook(
         """
         self.subscribe(self.test_user, self.channel_name)
 
+        url = getattr(self, "url", None)
+        if url is None:
+            url = self.build_webhook_url()
+
+        webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None)
+        if webhook_secret is not None:
+            set_bot_config(self.test_user, "webhook_secret", webhook_secret)
+
         payload = self.get_payload(fixture_name)
         if content_type is not None:
             extra["content_type"] = content_type
+
+        if webhook_secret is not None and config is not None:
+            header_val = compute_webhook_signature(
+                force_bytes(webhook_secret),
+                force_bytes(payload),
+                config,
+            )
+
+            django_header = "HTTP_" + config.header.upper().replace("-", "_")
+            if django_header not in extra:
+                extra[django_header] = header_val
+
         headers = call_fixture_to_headers(self.webhook_dir_name, fixture_name)
         headers = standardize_headers(headers)
         extra.update(headers)
-        try:
-            msg = self.send_webhook_payload(
-                self.test_user,
-                self.url,
-                payload,
-                **extra,
-            )
-        except EmptyResponseError:
-            if expect_noop:
-                return
-            else:
-                raise AssertionError(
-                    "No message was sent. Pass expect_noop=True if this is intentional."
-                )
+        with self.settings(VERIFY_WEBHOOK_SIGNATURES=self.VERIFY_WEBHOOK_SIGNATURES):
+            try:
+                msg = self.send_webhook_payload(self.test_user, url, payload, **extra)
+            except EmptyResponseError:
+                if expect_noop:
+                    return
+                else:
+                    raise AssertionError(
+                        "No message was sent. Pass expect_noop=True if this is intentional."
+                    )
 
         if expect_noop:
             raise Exception(
@@ -2718,6 +2754,11 @@ def send_and_test_private_message(
         Most webhooks send to streams, and you will want to look at
         check_webhook.
         """
+
+        webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None)
+        if webhook_secret is not None:
+            set_bot_config(self.test_user, "webhook_secret", webhook_secret)
+
         payload = self.get_payload(fixture_name)
         extra["content_type"] = content_type
 
@@ -2728,12 +2769,13 @@ def send_and_test_private_message(
         if sender is None:
             sender = self.test_user
 
-        msg = self.send_webhook_payload(
-            sender,
-            self.url,
-            payload,
-            **extra,
-        )
+        with self.settings(VERIFY_WEBHOOK_SIGNATURES=self.VERIFY_WEBHOOK_SIGNATURES):
+            msg = self.send_webhook_payload(
+                sender,
+                self.url,
+                payload,
+                **extra,
+            )
         self.assertEqual(msg.content, expected_message)
 
         return msg