From fcfb8ab1b027a0b26db095ac2d70fd34a73a68f5 Mon Sep 17 00:00:00 2001
From: Thijmen Wijers <thijmen.wijers@myparcel.nl>
Date: Fri, 27 Oct 2023 10:00:31 +0200
Subject: [PATCH] fix: feedback

---
 src/App/Webhook/PdkWebhookManager.php         | 26 +++---
 .../App/Webhook/PdkWebhookManagerTest.php     | 87 ++++++++++++++++++-
 2 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/src/App/Webhook/PdkWebhookManager.php b/src/App/Webhook/PdkWebhookManager.php
index 503d6f711..25d0ab85b 100644
--- a/src/App/Webhook/PdkWebhookManager.php
+++ b/src/App/Webhook/PdkWebhookManager.php
@@ -69,7 +69,14 @@ public function call($input, string $context = self::CONTEXT_WEBHOOK): Response
      */
     public function processWebhook(Request $request): void
     {
-        $logContext = ['request' => get_object_vars($request)];
+        $requiredPath = parse_url($this->webhooksRepository->getHashedUrl(), PHP_URL_PATH);
+        $logContext   = ['request' => get_object_vars($request)];
+
+        if ($request->getRequestUri() !== $requiredPath) {
+            Logger::error('Webhook received with invalid url', $logContext);
+
+            return;
+        }
 
         Logger::debug('Webhook received', $logContext);
 
@@ -117,17 +124,14 @@ protected function resolveHook(string $event): HookInterface
      */
     private function getHooks(Request $request): array
     {
-        $headers = $request->headers->all();
-        $body    = json_decode($request->getContent(), true);
+        $myParcelHeader = $request->headers->get('x-myparcel-hook');
+        $body           = json_decode($request->getContent(), true);
+        $hooks          = $body['data']['hooks'] ?? [];
 
-        if (isset($headers['x-myparcel-hook'])) {
-            $body['data']['hooks'] = array_map(function ($hook) use ($headers) {
-                $hook['event'] = $headers['x-myparcel-hook'][0];
-
-                return $hook;
-            }, $body['data']['hooks']);
-        }
+        return array_map(static function ($hook) use ($myParcelHeader) {
+            $hook['event'] = $myParcelHeader;
 
-        return $body['data']['hooks'] ?? [];
+            return $hook;
+        }, $hooks);
     }
 }
diff --git a/tests/Unit/App/Webhook/PdkWebhookManagerTest.php b/tests/Unit/App/Webhook/PdkWebhookManagerTest.php
index 8ced69fe3..9dd695850 100644
--- a/tests/Unit/App/Webhook/PdkWebhookManagerTest.php
+++ b/tests/Unit/App/Webhook/PdkWebhookManagerTest.php
@@ -21,7 +21,7 @@
 
 usesShared(new UsesMockPdkInstance(), new UsesMockEachCron());
 
-it('dispatches incoming webhook', function (string $hook) {
+it('dispatches and executes webhooks with myparcel header', function (string $hook) {
     /** @var PdkWebhooksRepositoryInterface $repository */
     $repository = Pdk::get(PdkWebhooksRepositoryInterface::class);
     /** @var PdkWebhookManagerInterface $webhookManager */
@@ -29,10 +29,85 @@
     /** @var \MyParcelNL\Pdk\Tests\Bootstrap\MockCronService $cronService */
     $cronService = Pdk::get(CronServiceInterface::class);
 
+    $repository->storeHashedUrl('https://example.com/hashed-url');
+
+    $repository->store(new WebhookSubscriptionCollection([['hook' => $hook, 'url' => $repository->getHashedUrl()]]));
+
+    $time = time();
+
+    $request = Request::create(
+        $repository->getHashedUrl(),
+        Request::METHOD_POST,
+        [],
+        [],
+        [],
+        ['HTTP_X_MYPARCEL_HOOK' => $hook],
+        json_encode([
+            'data' => [
+                'hooks' => [
+                    ['event' => $hook],
+                ],
+            ],
+        ])
+    );
+
+    $response = $webhookManager->call($request);
+
+    $scheduled = $cronService->getScheduledTasks();
+    $timestamp = $scheduled->first()['timestamp'];
+
+    expect($response->getStatusCode())
+        ->toBe(Response::HTTP_ACCEPTED)
+        ->and($request->headers->all())
+        ->toHaveKey('x-myparcel-hook')
+        ->and($response->getContent())
+        ->toBe('')
+        ->and($response->getStatusCode())
+        ->toBe(Response::HTTP_ACCEPTED)
+        ->and($scheduled->all())
+        ->toHaveLength(1)
+        // The webhook is scheduled to start immediately. Add a little window to account for the time it takes to run the test.
+        ->and($timestamp)
+        ->toBeGreaterThanOrEqual($time - 10)
+        ->and($timestamp)
+        ->toBeLessThanOrEqual($time + 10);
+
+    $cronService->executeAllTasks();
+    expect(
+        $cronService->getScheduledTasks()
+            ->all()
+    )->toBeEmpty();
+})->with(WebhookSubscription::ALL);
+
+it('dispatches and executes incoming webhook without myparcel header', function (string $hook) {
+    /** @var PdkWebhooksRepositoryInterface $repository */
+    $repository = Pdk::get(PdkWebhooksRepositoryInterface::class);
+    /** @var PdkWebhookManagerInterface $webhookManager */
+    $webhookManager = Pdk::get(PdkWebhookManagerInterface::class);
+    /** @var \MyParcelNL\Pdk\Tests\Bootstrap\MockCronService $cronService */
+    $cronService = Pdk::get(CronServiceInterface::class);
+
+    $repository->storeHashedUrl('https://example.com/hashed-url');
     $repository->store(new WebhookSubscriptionCollection([['hook' => $hook, 'url' => $repository->getHashedUrl()]]));
 
-    $time     = time();
-    $response = $webhookManager->call(new Request());
+    $time = time();
+
+    $request  = Request::create(
+        $repository->getHashedUrl(),
+        Request::METHOD_POST,
+        [],
+        [],
+        [],
+        [],
+        json_encode([
+            'data' => [
+                'hooks' => [
+                    ['event' => $hook],
+                ],
+            ],
+        ])
+    );
+    $response = $webhookManager->call($request);
 
     $scheduled = $cronService->getScheduledTasks();
 
@@ -49,4 +124,10 @@
         ->toBeGreaterThanOrEqual($time - 10)
         ->and($timestamp)
         ->toBeLessThanOrEqual($time + 10);
+
+    $cronService->executeAllTasks();
+    expect(
+        $cronService->getScheduledTasks()
+            ->all()
+    )->toBeEmpty();
 })->with(WebhookSubscription::ALL);