From f2e2e513685a0d2cda258668ce16be74dfe2f9a1 Mon Sep 17 00:00:00 2001 From: freek Date: Mon, 8 Jul 2019 20:51:34 +0200 Subject: [PATCH] wip --- CHANGELOG.md | 5 +++++ src/Events/InvalidSignatureEvent.php | 7 +------ src/Exceptions/WebhookFailed.php | 9 ++------- .../DefaultSignatureValidator.php | 5 +++++ src/WebhookProcessor.php | 17 ++++------------ .../EverythingIsValidSignatureValidator.php | 17 ++++++++++++++++ .../NothingIsValidSignatureValidator.php | 17 ++++++++++++++++ tests/WebhookControllerTest.php | 20 +++++++++++++++++-- 8 files changed, 69 insertions(+), 28 deletions(-) create mode 100644 tests/TestClasses/EverythingIsValidSignatureValidator.php create mode 100644 tests/TestClasses/NothingIsValidSignatureValidator.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fd501bb..b157948 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `laravel-webhook-client` will be documented in this file +## 2.0.0 - 2019-07-08 + +- `DefaultSignatureValidator` is now responsible for verifying that a signature header has been set +- `InvalidSignatureEvent` now only gets the `$request` + ## 1.0.2 - 2019-07-01 - remove handle abstract method from `ProcessWebhookJob` to allow DI. diff --git a/src/Events/InvalidSignatureEvent.php b/src/Events/InvalidSignatureEvent.php index 686bfd9..ec5ce5a 100644 --- a/src/Events/InvalidSignatureEvent.php +++ b/src/Events/InvalidSignatureEvent.php @@ -9,13 +9,8 @@ class InvalidSignatureEvent /** @var \Illuminate\Http\Request */ public $request; - /** @var string|null */ - public $invalidSignature; - - public function __construct(Request $request, ?string $invalidSignature) + public function __construct(Request $request) { $this->request = $request; - - $this->invalidSignature = $invalidSignature; } } diff --git a/src/Exceptions/WebhookFailed.php b/src/Exceptions/WebhookFailed.php index b96f292..8e48ceb 100644 --- a/src/Exceptions/WebhookFailed.php +++ b/src/Exceptions/WebhookFailed.php @@ -6,14 +6,9 @@ class WebhookFailed extends Exception { - public static function missingSignature(string $headerName): WebhookFailed + public static function invalidSignature(): WebhookFailed { - return new static("The request did not contain a header named `${headerName}`."); - } - - public static function invalidSignature(string $signature, string $signatureHeaderName): WebhookFailed - { - return new static("The signature `{$signature}` found in the header named `{$signatureHeaderName}` is invalid. Make sure that the `webhook_signing_secret` config key is set to the correct value. If you are caching your config try running `php artisan cache:clear` to resolve the problem."); + return new static("The signature is invalid."); } public static function signingSecretNotSet(): WebhookFailed diff --git a/src/SignatureValidator/DefaultSignatureValidator.php b/src/SignatureValidator/DefaultSignatureValidator.php index eafe96a..73ecd3c 100644 --- a/src/SignatureValidator/DefaultSignatureValidator.php +++ b/src/SignatureValidator/DefaultSignatureValidator.php @@ -3,6 +3,7 @@ namespace Spatie\WebhookClient\SignatureValidator; use Illuminate\Http\Request; +use Spatie\WebhookClient\Events\InvalidSignatureEvent; use Spatie\WebhookClient\WebhookConfig; use Spatie\WebhookClient\Exceptions\WebhookFailed; @@ -12,6 +13,10 @@ public function isValid(Request $request, WebhookConfig $config): bool { $signature = $request->header($config->signatureHeaderName); + if (! $signature) { + return false; + } + $signingSecret = $config->signingSecret; if (empty($signingSecret)) { diff --git a/src/WebhookProcessor.php b/src/WebhookProcessor.php index 7e107da..8cfbcf6 100644 --- a/src/WebhookProcessor.php +++ b/src/WebhookProcessor.php @@ -25,7 +25,7 @@ public function __construct(Request $request, WebhookConfig $config) public function process() { - $this->guardAgainstInvalidSignature(); + $this->ensureValidSignature(); if (! $this->config->webhookProfile->shouldProcess($this->request)) { return; @@ -36,21 +36,12 @@ public function process() $this->processWebhook($webhookCall); } - protected function guardAgainstInvalidSignature() + protected function ensureValidSignature() { - $headerName = $this->config->signatureHeaderName; - - $signature = $this->request->header($headerName); - - if (! $signature) { - event(new InvalidSignatureEvent($this->request, $signature)); - throw WebhookFailed::missingSignature($headerName); - } - if (! $this->config->signatureValidator->isValid($this->request, $this->config)) { - event(new InvalidSignatureEvent($this->request, $signature)); + event(new InvalidSignatureEvent($this->request)); - throw WebhookFailed::invalidSignature($signature, $this->config->signatureHeaderName); + throw WebhookFailed::invalidSignature(); } return $this; diff --git a/tests/TestClasses/EverythingIsValidSignatureValidator.php b/tests/TestClasses/EverythingIsValidSignatureValidator.php new file mode 100644 index 0000000..67530b3 --- /dev/null +++ b/tests/TestClasses/EverythingIsValidSignatureValidator.php @@ -0,0 +1,17 @@ +withoutExceptionHandling(); - $this ->postJson('incoming-webhooks', $this->payload, $this->headers) ->assertSuccessful(); @@ -78,6 +78,22 @@ public function a_request_with_an_invalid_payload_will_not_get_processed() Event::assertDispatched(InvalidSignatureEvent::class); } + /** @test */ + public function it_can_work_with_an_alternative_signature_validator() + { + config()->set('webhook-client.configs.0.signature_validator', EverythingIsValidSignatureValidator::class); + + $this + ->postJson('incoming-webhooks', $this->payload, []) + ->assertOk(); + + config()->set('webhook-client.configs.0.signature_validator', NothingIsValidSignatureValidator::class); + + $this + ->postJson('incoming-webhooks', $this->payload, []) + ->assertStatus(500); + } + /** @test */ public function it_can_work_with_an_alternative_profile() {