Webhooks: Fixed failing delete-based events

Due to queue serialization.
Added a test to check a couple of delete events.
Added ApiTokenFactory to support.
Also made a couple of typing/doc updates while there.

Related to #4373

diff --git a/app/Activity/DispatchWebhookJob.php b/app/Activity/DispatchWebhookJob.php
index 5e6380f439d6f2eb29403e26905b9a4be9e71988..f2330c4faf967f257fd94534b98d2adf9b448439 100644 (file)
--- a/app/Activity/DispatchWebhookJob.php
+++ b/app/Activity/DispatchWebhookJob.php
@@ -24,27 +24,23 @@ class DispatchWebhookJob implements ShouldQueue
     use SerializesModels;
 
     protected Webhook $webhook;
-    protected string $event;
     protected User $initiator;
     protected int $initiatedTime;
-
-    /**
-     * @var string|Loggable
-     */
-    protected $detail;
+    protected array $webhookData;
 
     /**
      * Create a new job instance.
      *
      * @return void
      */
-    public function __construct(Webhook $webhook, string $event, $detail)
+    public function __construct(Webhook $webhook, string $event, Loggable|string $detail)
     {
         $this->webhook = $webhook;
-        $this->event = $event;
-        $this->detail = $detail;
         $this->initiator = user();
         $this->initiatedTime = time();
+
+        $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $event, $this->webhook, $detail, $this->initiator, $this->initiatedTime);
+        $this->webhookData =  $themeResponse ?? WebhookFormatter::getDefault($event, $this->webhook, $detail, $this->initiator, $this->initiatedTime)->format();
     }
 
     /**
@@ -54,15 +50,13 @@ class DispatchWebhookJob implements ShouldQueue
      */
     public function handle()
     {
-        $themeResponse = Theme::dispatch(ThemeEvents::WEBHOOK_CALL_BEFORE, $this->event, $this->webhook, $this->detail, $this->initiator, $this->initiatedTime);
-        $webhookData = $themeResponse ?? WebhookFormatter::getDefault($this->event, $this->webhook, $this->detail, $this->initiator, $this->initiatedTime)->format();
         $lastError = null;
 
         try {
             $response = Http::asJson()
                 ->withOptions(['allow_redirects' => ['strict' => true]])
                 ->timeout($this->webhook->timeout)
-                ->post($this->webhook->endpoint, $webhookData);
+                ->post($this->webhook->endpoint, $this->webhookData);
         } catch (\Exception $exception) {
             $lastError = $exception->getMessage();
             Log::error("Webhook call to endpoint {$this->webhook->endpoint} failed with error \"{$lastError}\"");

diff --git a/app/Activity/Tools/WebhookFormatter.php b/app/Activity/Tools/WebhookFormatter.php
index 4237139780a03f0912b9bb5aae3db56950134bc4..6ccb084b8de769b2856020b101beffb527199ef1 100644 (file)
--- a/app/Activity/Tools/WebhookFormatter.php
+++ b/app/Activity/Tools/WebhookFormatter.php
@@ -17,18 +17,14 @@ class WebhookFormatter
     protected string $event;
     protected User $initiator;
     protected int $initiatedTime;
-
-    /**
-     * @var string|Loggable
-     */
-    protected $detail;
+    protected string|Loggable $detail;
 
     /**
      * @var array{condition: callable(string, Model):bool, format: callable(Model):void}[]
      */
     protected $modelFormatters = [];
 
-    public function __construct(string $event, Webhook $webhook, $detail, User $initiator, int $initiatedTime)
+    public function __construct(string $event, Webhook $webhook, string|Loggable $detail, User $initiator, int $initiatedTime)
     {
         $this->webhook = $webhook;
         $this->event = $event;

diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php
index b462eaae956945c703644f7b49e7b30c8bb6e2e5..5c2d591e4083783d2ca6e75f86a0263ef38d9ec3 100644 (file)
--- a/app/Api/ApiToken.php
+++ b/app/Api/ApiToken.php
@@ -4,6 +4,7 @@ namespace BookStack\Api;
 
 use BookStack\Activity\Models\Loggable;
 use BookStack\Users\Models\User;
+use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Support\Carbon;
@@ -20,6 +21,8 @@ use Illuminate\Support\Carbon;
  */
 class ApiToken extends Model implements Loggable
 {
+    use HasFactory;
+
     protected $fillable = ['name', 'expires_at'];
     protected $casts = [
         'expires_at' => 'date:Y-m-d',

diff --git a/app/Theming/ThemeEvents.php b/app/Theming/ThemeEvents.php
index 994c3ec0dbaacfb97524aca906b890e503cc1f70..4b56b2f56c119c596e2d8684a1e2569d65428bb7 100644 (file)
--- a/app/Theming/ThemeEvents.php
+++ b/app/Theming/ThemeEvents.php
@@ -132,11 +132,12 @@ class ThemeEvents
      * If the listener returns a non-null value, that will be used as the POST data instead
      * of the system default.
      *
-     * @param string                                $event
-     * @param \BookStack\Activity\Models\Webhook            $webhook
+     * @param string                                     $event
+     * @param \BookStack\Activity\Models\Webhook         $webhook
      * @param string|\BookStack\Activity\Models\Loggable $detail
-     * @param \BookStack\Users\Models\User                  $initiator
-     * @param int                                   $initiatedTime
+     * @param \BookStack\Users\Models\User               $initiator
+     * @param int                                        $initiatedTime
+     * @returns array|null
      */
     const WEBHOOK_CALL_BEFORE = 'webhook_call_before';
 }

diff --git a/database/factories/Api/ApiTokenFactory.php b/database/factories/Api/ApiTokenFactory.php
new file mode 100644 (file)
index 0000000..adf2fff
--- /dev/null
+++ b/database/factories/Api/ApiTokenFactory.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace Database\Factories\Api;
+
+use BookStack\Api\ApiToken;
+use BookStack\Users\Models\User;
+use Illuminate\Database\Eloquent\Factories\Factory;
+use Illuminate\Support\Carbon;
+use Illuminate\Support\Str;
+
+class ApiTokenFactory extends Factory
+{
+    protected $model = ApiToken::class;
+
+    public function definition(): array
+    {
+        return [
+            'token_id' => Str::random(10),
+            'secret' => Str::random(12),
+            'name' => $this->faker->name(),
+            'expires_at' => Carbon::now()->addYear(),
+            'created_at' => Carbon::now(),
+            'updated_at' => Carbon::now(),
+            'user_id' => User::factory(),
+        ];
+    }
+}

diff --git a/tests/Actions/WebhookCallTest.php b/tests/Actions/WebhookCallTest.php
index f2def087b0c1b13b7860847f51e968e838ba1609..fc49a524eef2c40e99b5206cca6b241c50518c47 100644 (file)
--- a/tests/Actions/WebhookCallTest.php
+++ b/tests/Actions/WebhookCallTest.php
@@ -6,6 +6,8 @@ use BookStack\Activity\ActivityType;
 use BookStack\Activity\DispatchWebhookJob;
 use BookStack\Activity\Models\Webhook;
 use BookStack\Activity\Tools\ActivityLogger;
+use BookStack\Api\ApiToken;
+use BookStack\Entities\Models\PageRevision;
 use BookStack\Users\Models\User;
 use Illuminate\Http\Client\Request;
 use Illuminate\Support\Facades\Bus;
@@ -46,6 +48,24 @@ class WebhookCallTest extends TestCase
         Bus::assertNotDispatched(DispatchWebhookJob::class);
     }
 
+    public function test_webhook_runs_for_delete_actions()
+    {
+        $this->newWebhook(['active' => true, 'endpoint' => 'https://wh.example.com'], ['all']);
+        Http::fake([
+            '*' => Http::response('', 500),
+        ]);
+
+        $user = $this->users->newUser();
+        $resp = $this->asAdmin()->delete($user->getEditUrl());
+        $resp->assertRedirect('/settings/users');
+
+        /** @var ApiToken $apiToken */
+        $editor = $this->users->editor();
+        $apiToken = ApiToken::factory()->create(['user_id' => $editor]);
+        $resp = $this->delete($editor->getEditUrl('/api-tokens/' . $apiToken->id));
+        $resp->assertRedirect($editor->getEditUrl('#api_tokens'));
+    }
+
     public function test_failed_webhook_call_logs_error()
     {
         $logger = $this->withTestLogger();
@@ -120,7 +140,7 @@ class WebhookCallTest extends TestCase
         $activityLogger->add($event, $detail);
     }
 
-    protected function newWebhook(array $attrs = [], array $events = ['all']): Webhook
+    protected function newWebhook(array $attrs, array $events): Webhook
     {
         /** @var Webhook $webhook */
         $webhook = Webhook::factory()->create($attrs);