Require webhook events

docs/development/API/admin_api.md

@@ -1877,7 +1877,7 @@ Note that this differs from the Mastodon API variant: Mastodon API only returns
 - Method: `POST`
 - Params:
   - `url`: **string** Webhook URL
-  - *optional* `events`: **[string]** Types of events to trigger on (`account.created`, `report.created`)
+  - `events`: **[string]** Types of events to trigger on (`account.created`, `report.created`)
   - *optional* `enabled`: **boolean** Whether webhook is enabled
 - Response: A webhook
 

lib/pleroma/web/api_spec/operations/admin/webhook_operation.ex

@@ -55,9 +55,10 @@ defmodule Pleroma.Web.ApiSpec.Admin.WebhookOperation do
           %Schema{
             description: "POST body for creating a webhook",
             type: :object,
+            required: [:url, :events],
             properties: %{
-              url: %Schema{type: :string, format: :uri, required: true},
-              events: event_type(true),
+              url: %Schema{type: :string, format: :uri},
+              events: event_type(),
               enabled: %Schema{type: :boolean}
             }
           }
@@ -179,15 +180,14 @@ defmodule Pleroma.Web.ApiSpec.Admin.WebhookOperation do
     }
   end
 
-  defp event_type(required \\ nil) do
+  defp event_type do
     %Schema{
       type: :array,
       items: %Schema{
         title: "Event",
         description: "Event type",
         type: :string,
-        enum: ["account.created", "report.created"],
-        required: required
+        enum: ["account.created", "report.created"]
       }
     }
   end

lib/pleroma/webhook.ex

@@ -36,6 +36,7 @@ defmodule Pleroma.Webhook do
     webhook
     |> cast(params, [:url, :events, :enabled, :internal])
     |> validate_required([:url, :events])
+    |> validate_events_present()
     |> unique_constraint(:url)
     |> put_secret()
   end
@@ -43,6 +44,7 @@ defmodule Pleroma.Webhook do
   def update_changeset(%__MODULE__{} = webhook, params \\ %{}) do
     webhook
     |> cast(params, [:url, :events, :enabled, :internal])
+    |> validate_events_present()
     |> unique_constraint(:url)
   end
 
@@ -73,6 +75,15 @@ defmodule Pleroma.Webhook do
     |> Repo.update()
   end
 
+  defp validate_events_present(changeset) do
+    cond do
+      Keyword.has_key?(changeset.errors, :events) -> changeset
+      get_field(changeset, :internal) -> changeset
+      match?([_ | _], get_field(changeset, :events)) -> changeset
+      true -> add_error(changeset, :events, "can't be blank")
+    end
+  end
+
   defp put_secret(changeset) do
     changeset
     |> put_change(:secret, generate_secret())

test/pleroma/webhook_test.exs

@@ -33,6 +33,24 @@ defmodule Pleroma.WebhookTest do
     assert {:error, _changeset} = Webhook.update(webhook, %{events: [:"status.created"]})
   end
 
+  test "rejects empty events for non-internal webhooks" do
+    assert {:error, changeset} = Webhook.create(%{url: "https://example.com/webhook", events: []})
+    assert %{events: ["can't be blank"]} = errors_on(changeset)
+
+    {:ok, webhook} =
+      Webhook.create(%{url: "https://example.com/valid-webhook", events: [:"report.created"]})
+
+    assert {:error, changeset} = Webhook.update(webhook, %{events: []})
+    assert %{events: ["can't be blank"]} = errors_on(changeset)
+
+    assert {:ok, _webhook} =
+             Webhook.create(%{
+               url: "https://example.com/internal-webhook",
+               events: [],
+               internal: true
+             })
+  end
+
   test "filter webhooks by type" do
     {:ok, %{id: id1}} =
       Webhook.create(%{url: "https://example.com/webhook1", events: [:"report.created"]})