(webhooks) get webhook testing passing with updated credential handling

src/Connector/Base.php

@@ -3,6 +3,7 @@
 namespace NSWDPC\Messaging\Mailgun\Connector;
 
 use Mailgun\Mailgun;
+use NSWDPC\Messaging\Mailgun\Controllers\MailgunWebhook;
 use NSWDPC\Messaging\Mailgun\Services\Logger;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Config\Configurable;

src/Connector/Webhook.php

@@ -14,20 +14,23 @@
 class Webhook extends Base
 {
     /**
-     * verify signature
-     * @return bool returns true if signature is valid
+     * verify signature, which is an array of data in the main payload
+     * See https://documentation.mailgun.com/docs/mailgun/user-manual/tracking-messages/#securing-webhooks
+     * @param array $signature the signature part of the payload
      */
     public function verify_signature(array $signature): bool
     {
         if ($this->is_valid_signature($signature)) {
+            // check that the signed signature matches the signature provided
             return hash_equals($this->sign_token($signature), $signature['signature']);
         }
 
         return false;
     }
 
     /**
-     * Sign the token based on timestamp and signature in request
+     * Sign the token and timestamp from the signature data provided, with the configured signing key
+     * @param array $signature the signature part of the payload
      */
     public function sign_token(array $signature): string
     {
@@ -41,8 +44,8 @@ public function sign_token(array $signature): string
     }
 
     /**
-     * Based on Mailgun docs, determine if the signature is correct
-     * @param array $signature
+     * Based on Mailgun docs, determine if the signature is correctly formatted
+     * @param array $signature the signature part of the payload
      */
     public function is_valid_signature($signature): bool
     {

tests/WebhookTest.php

@@ -15,11 +15,11 @@
  */
 class WebhookTest extends FunctionalTest
 {
-    protected string $webhook_filter_variable = 'skjhgiehg943753-"';
+    protected string $webhook_filter_variable = 'test-filter-var-curr';
 
-    protected string $webhook_previous_filter_variable = 'snsd875bslw[';
+    protected string $webhook_previous_filter_variable = 'test-filter-var-prev';
 
-    protected string $webhook_signing_key = 'abcd1234';
+    protected string $webhook_signing_key = 'TEST_SHOULD_PASS';
 
     protected string $test_api_key = 'webhook_api_key';
 
@@ -36,15 +36,15 @@ public function setUp(): void
         Environment::setEnv('MAILGUN_WEBHOOK_DOMAIN', $this->test_api_domain);
         Environment::setEnv('MAILGUN_WEBHOOK_REGION', $this->test_api_region);
         Environment::setEnv('MAILGUN_WEBHOOK_FILTER_VARIABLE', $this->webhook_filter_variable);
-        Environment::setEnv('MAILGUN_WEBHOOK_PREVIOUS_FILTER_VARIABLE', $this->webhook_filter_variable);
+        Environment::setEnv('MAILGUN_WEBHOOK_PREVIOUS_FILTER_VARIABLE', $this->webhook_previous_filter_variable);
         Environment::setEnv('MAILGUN_WEBHOOK_SIGNING_KEY', $this->webhook_signing_key);
         Config::modify()->set(MailgunWebhook::class, 'webhooks_enabled', true);
     }
 
     protected function getTestDsn(): string
     {
-        $domain = Environment::setEnv('MAILGUN_WEBHOOK_DOMAIN');
-        $key = Environment::setEnv('MAILGUN_WEBHOOK_API_KEY');
+        $domain = Environment::getEnv('MAILGUN_WEBHOOK_DOMAIN');
+        $key = Environment::getEnv('MAILGUN_WEBHOOK_API_KEY');
         return "mailgunsync+api://{$domain}:{$key}@default";
     }
 
@@ -72,25 +72,16 @@ protected function getConnector()
         return Webhook::create($this->getTestDsn());
     }
 
-    /**
-     * Set a signing key in Configuration
-     * @param string $signing_key
-     */
-    protected function setSigningKey($signing_key)
-    {
-        Config::modify()->set(Base::class, 'webhook_signing_key', $signing_key);
-    }
-
     /**
      * Replace the signature on the request data with something to trigger success/error
-     * @param string $signing_key
-     * @param string $request_data
+     * @param string $request_data JSON encoded request data from a test payload
      * @return array
      */
-    protected function setSignatureOnRequest($signing_key, $request_data)
+    protected function setSignatureOnRequest(string $request_data)
     {
         $decoded = json_decode($request_data, true);
         $connector = $this->getConnector();
+        // sign the signature
         $signature = $connector->sign_token($decoded['signature']);
         $decoded['signature']['signature'] = $signature;
         return $decoded;
@@ -109,16 +100,16 @@ protected function setWebhookFilterVariable(array $data, $value): array
      */
     protected function sendWebhookRequest($type)
     {
-        $signing_key = "TEST_SHOULD_PASS";
-        $this->setSigningKey($signing_key);
+
+        $signingKey = Environment::getEnv('MAILGUN_WEBHOOK_SIGNING_KEY');
 
         $url = $this->getSubmissionUrl();
         $headers = [
             'Content-Type' => "application/json"
         ];
         $session = null;
-        $data = $this->setSignatureOnRequest($signing_key, $this->getWebhookRequestData($type));
-        $data = $this->setWebhookFilterVariable($data, $this->webhook_filter_variable);
+        $data = $this->setSignatureOnRequest($this->getWebhookRequestData($type));
+        $data = $this->setWebhookFilterVariable($data, Environment::getEnv('MAILGUN_WEBHOOK_FILTER_VARIABLE'));
 
         $cookies = null;
 
@@ -128,7 +119,7 @@ protected function sendWebhookRequest($type)
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Expected success response with correct signing_key failed: ' . $response->getStatusCode() . "/" . $response->getStatusDescription()
+            'Expected success response with correct signing_key ' . $signingKey . ' failed: ' . $response->getStatusCode() . "/" . $response->getStatusDescription()
         );
 
         $event = \Mailgun\Model\Event\Event::create($data['event-data']);
@@ -138,15 +129,14 @@ protected function sendWebhookRequest($type)
         $this->assertTrue($record && $record->exists(), "DB Mailgun event does not exist for event {$event->getId()}");
 
         // change the webhook config variable to the previous var
-        $data = $this->setWebhookFilterVariable($data, $this->webhook_previous_filter_variable);
+        $data = $this->setWebhookFilterVariable($data, Environment::getEnv('MAILGUN_WEBHOOK_PREVIOUS_FILTER_VARIABLE'));
         $response = $this->post($url, $data, $headers, $session, json_encode($data, JSON_UNESCAPED_SLASHES), $cookies);
         $this->assertEquals(
             200,
             $response->getStatusCode(),
-            'Expected success response with correct signing_key failed: ' . $response->getStatusCode() . "/" . $response->getStatusDescription()
+            'Expected success response with correct signing_key ' . $signingKey . ' failed: ' . $response->getStatusCode() . "/" . $response->getStatusDescription()
         );
 
-
         // change the webhook variable to something else completely
         $data = $this->setWebhookFilterVariable($data, 'not going to work');
         $response = $this->post($url, $data, $headers, $session, json_encode($data, JSON_UNESCAPED_SLASHES), $cookies);
@@ -158,12 +148,10 @@ protected function sendWebhookRequest($type)
 
         // remove webhook variable and test
         unset($data['event-data']['user-variables']['wfv']);
-        Config::modify()->set(Base::class, 'webhook_filter_variable', '');
-        Config::modify()->set(Base::class, 'webhook_previous_filter_variable', '');
-
-        // change the signing key in config, it should fail now
-        $signing_key = "YOU_SHALL_NOT_PASS";
-        $this->setSigningKey($signing_key);
+        Environment::setEnv('MAILGUN_WEBHOOK_FILTER_VARIABLE', '');
+        Environment::setEnv('MAILGUN_WEBHOOK_PREVIOUS_FILTER_VARIABLE', '');
+        // change the signing key, it should fail now as the payload signatures are signed with the 'webhook_signing_key' value
+        Environment::setEnv('MAILGUN_WEBHOOK_SIGNING_KEY', "YOU_SHALL_NOT_PASS");
         $response = $this->post($url, $data, $headers, $session, json_encode($data, JSON_UNESCAPED_SLASHES), $cookies);
         $this->assertEquals(
             406,
@@ -172,37 +160,37 @@ protected function sendWebhookRequest($type)
         );
     }
 
-    public function testWebookDelivered(): void
+    public function testWebhookDelivered(): void
     {
         $this->sendWebhookRequest("delivered");
     }
 
-    public function testWebookClick(): void
+    public function testWebhookClick(): void
     {
         $this->sendWebhookRequest("clicked");
     }
 
-    public function testWebookOpened(): void
+    public function testWebhookOpened(): void
     {
         $this->sendWebhookRequest("opened");
     }
 
-    public function testWebookFailedPermanent(): void
+    public function testWebhookFailedPermanent(): void
     {
         $this->sendWebhookRequest("failed_permanent");
     }
 
-    public function testWebookFailedTemporary(): void
+    public function testWebhookFailedTemporary(): void
     {
         $this->sendWebhookRequest("failed_temporary");
     }
 
-    public function testWebookUnsubscribed(): void
+    public function testWebhookUnsubscribed(): void
     {
         $this->sendWebhookRequest("unsubscribed");
     }
 
-    public function testWebookComplained(): void
+    public function testWebhookComplained(): void
     {
         $this->sendWebhookRequest("complained");
     }