From 4b98d4f968f354a8e651ea90233d9223170033ca Mon Sep 17 00:00:00 2001 From: Jordan Partridge Date: Sun, 14 Dec 2025 22:01:49 -0700 Subject: [PATCH] Refactor webhook handling to use Spatie webhook client Replace custom WebhookController with Spatie's laravel-webhook-client package for better webhook handling: - Add spatie/laravel-webhook-client package - Create GateSignatureValidator for HMAC signature verification - Create GateWebhookProfile for payload validation - Create ProcessGateWebhookJob to fire CertificationCompleted events - Configure webhook-client.php for gate endpoint - Update routes to use Spatie's webhooks() macro - Update tests to verify new webhook flow - Add unit tests for GateSignatureValidator - Maintain 100% test coverage --- app/Http/Controllers/WebhookController.php | 27 ------ app/Http/Requests/GateWebhookRequest.php | 32 ------- app/Webhooks/GateSignatureValidator.php | 30 +++++++ app/Webhooks/GateWebhookProfile.php | 30 +++++++ app/Webhooks/ProcessGateWebhookJob.php | 26 ++++++ composer.json | 3 +- composer.lock | 80 ++++++++++++++++- config/webhook-client.php | 77 ++++++++++++++++ ...2_15_045715_create_webhook_calls_table.php | 23 +++++ routes/api.php | 4 +- tests/Feature/WebhookGateTest.php | 83 +++++++++++++++-- tests/Unit/GateSignatureValidatorTest.php | 90 +++++++++++++++++++ 12 files changed, 434 insertions(+), 71 deletions(-) delete mode 100644 app/Http/Controllers/WebhookController.php delete mode 100644 app/Http/Requests/GateWebhookRequest.php create mode 100644 app/Webhooks/GateSignatureValidator.php create mode 100644 app/Webhooks/GateWebhookProfile.php create mode 100644 app/Webhooks/ProcessGateWebhookJob.php create mode 100644 config/webhook-client.php create mode 100644 database/migrations/2025_12_15_045715_create_webhook_calls_table.php create mode 100644 tests/Unit/GateSignatureValidatorTest.php diff --git a/app/Http/Controllers/WebhookController.php b/app/Http/Controllers/WebhookController.php deleted file mode 100644 index 6c01bc4..0000000 --- a/app/Http/Controllers/WebhookController.php +++ /dev/null @@ -1,27 +0,0 @@ -validated('repository'), - sha: $request->validated('sha'), - verdict: $request->validated('verdict'), - reason: $request->validated('reason'), - checks: $request->validated('checks'), - triggeredBy: $request->validated('triggered_by'), - prNumber: $request->validated('pr_number'), - ); - - return response()->json(['status' => 'accepted']); - } -} diff --git a/app/Http/Requests/GateWebhookRequest.php b/app/Http/Requests/GateWebhookRequest.php deleted file mode 100644 index b3e7115..0000000 --- a/app/Http/Requests/GateWebhookRequest.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ - public function rules(): array - { - return [ - 'repository' => ['required', 'string'], - 'sha' => ['required', 'string'], - 'verdict' => ['required', Rule::in(['approved', 'rejected', 'escalate'])], - 'reason' => ['nullable', 'string'], - 'checks' => ['nullable', 'array'], - 'triggered_by' => ['nullable', 'string'], - 'pr_number' => ['nullable', 'integer'], - ]; - } -} diff --git a/app/Webhooks/GateSignatureValidator.php b/app/Webhooks/GateSignatureValidator.php new file mode 100644 index 0000000..d202338 --- /dev/null +++ b/app/Webhooks/GateSignatureValidator.php @@ -0,0 +1,30 @@ +header($config->signatureHeaderName); + $secret = $config->signingSecret; + + if (empty($secret)) { + return true; + } + + if (empty($signature)) { + return false; + } + + $computedSignature = hash_hmac('sha256', $request->getContent(), $secret); + + return hash_equals($computedSignature, $signature); + } +} diff --git a/app/Webhooks/GateWebhookProfile.php b/app/Webhooks/GateWebhookProfile.php new file mode 100644 index 0000000..a7272eb --- /dev/null +++ b/app/Webhooks/GateWebhookProfile.php @@ -0,0 +1,30 @@ +all(); + + if (empty($payload['repository']) || ! is_string($payload['repository'])) { + return false; + } + + if (empty($payload['sha']) || ! is_string($payload['sha'])) { + return false; + } + + if (empty($payload['verdict']) || ! in_array($payload['verdict'], ['approved', 'rejected', 'escalate'], true)) { + return false; + } + + return true; + } +} diff --git a/app/Webhooks/ProcessGateWebhookJob.php b/app/Webhooks/ProcessGateWebhookJob.php new file mode 100644 index 0000000..8b22f78 --- /dev/null +++ b/app/Webhooks/ProcessGateWebhookJob.php @@ -0,0 +1,26 @@ +webhookCall->payload; + + CertificationCompleted::fire( + repository: $payload['repository'], + sha: $payload['sha'], + verdict: $payload['verdict'], + reason: $payload['reason'] ?? null, + checks: $payload['checks'] ?? null, + triggeredBy: $payload['triggered_by'] ?? null, + prNumber: isset($payload['pr_number']) ? (int) $payload['pr_number'] : null, + ); + } +} diff --git a/composer.json b/composer.json index 7e64dfa..1070f0e 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "php": "^8.2", "hirethunk/verbs": "^0.7.0", "laravel/framework": "^12.0", - "laravel/tinker": "^2.10.1" + "laravel/tinker": "^2.10.1", + "spatie/laravel-webhook-client": "^3.4" }, "require-dev": { "fakerphp/faker": "^1.23", diff --git a/composer.lock b/composer.lock index fe0deb9..5552f14 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d3a3c7a1006b0a5a6723e1df90518800", + "content-hash": "c62ecb987d6083fa4564f7184751df1e", "packages": [ { "name": "brick/math", @@ -4578,6 +4578,84 @@ ], "time": "2025-07-17T15:46:43+00:00" }, + { + "name": "spatie/laravel-webhook-client", + "version": "3.4.5", + "source": { + "type": "git", + "url": "https://github.com/spatie/laravel-webhook-client.git", + "reference": "83e50b9797ad9fbbf290d79cf71ee3e5e6a076f3" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/spatie/laravel-webhook-client/zipball/83e50b9797ad9fbbf290d79cf71ee3e5e6a076f3", + "reference": "83e50b9797ad9fbbf290d79cf71ee3e5e6a076f3", + "shasum": "" + }, + "require": { + "illuminate/bus": "^9.0 || ^10.0 || ^11.0 || ^12.0", + "illuminate/database": "^9.0 || ^10.0 || ^11.0 || ^12.0", + "illuminate/http": "^9.0 || ^10.0 || ^11.0 || ^12.0", + "illuminate/support": "^9.0 || ^10.0 || ^11.0 || ^12.0", + "php": "^8.1 || ^8.2", + "spatie/laravel-package-tools": "^1.11" + }, + "require-dev": { + "orchestra/testbench": "^7.0 || ^8.0 || ^9.0 || ^10.0", + "pestphp/pest": "^3.8 || ^4.0", + "pestphp/pest-plugin-laravel": "^3.2 || ^4.0", + "phpstan/extension-installer": "^1.4.3", + "phpstan/phpstan-deprecation-rules": "^1.2.1", + "phpstan/phpstan-phpunit": "^1.4.2", + "phpunit/phpunit": "^11.5.3 || ^12.0", + "spatie/laravel-ray": "^1.43.1" + }, + "type": "library", + "extra": { + "laravel": { + "providers": [ + "Spatie\\WebhookClient\\WebhookClientServiceProvider" + ] + }, + "branch-alias": { + "dev-master": "11.x-dev" + } + }, + "autoload": { + "psr-4": { + "Spatie\\WebhookClient\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Freek Van der Herten", + "email": "freek@spatie.be", + "homepage": "https://spatie.be", + "role": "Developer" + } + ], + "description": "Receive webhooks in Laravel apps", + "homepage": "https://github.com/spatie/laravel-webhook-client", + "keywords": [ + "laravel-webhook-client", + "spatie" + ], + "support": { + "issues": "https://github.com/spatie/laravel-webhook-client/issues", + "source": "https://github.com/spatie/laravel-webhook-client/tree/3.4.5" + }, + "funding": [ + { + "url": "https://spatie.be/open-source/support-us", + "type": "custom" + } + ], + "time": "2025-11-27T09:35:16+00:00" + }, { "name": "symfony/clock", "version": "v8.0.0", diff --git a/config/webhook-client.php b/config/webhook-client.php new file mode 100644 index 0000000..7a8460c --- /dev/null +++ b/config/webhook-client.php @@ -0,0 +1,77 @@ + [ + [ + /* + * This package supports multiple webhook receiving endpoints. If you only have + * one endpoint receiving webhooks, you can use 'default'. + */ + 'name' => 'gate', + + /* + * We expect that every webhook call will be signed using a secret. This secret + * is used to verify that the payload has not been tampered with. + */ + 'signing_secret' => env('GATE_WEBHOOK_SECRET'), + + /* + * The name of the header containing the signature. + */ + 'signature_header_name' => 'X-Gate-Signature', + + /* + * This class will verify that the content of the signature header is valid. + * + * It should implement \Spatie\WebhookClient\SignatureValidator\SignatureValidator + */ + 'signature_validator' => \App\Webhooks\GateSignatureValidator::class, + + /* + * This class determines if the webhook call should be stored and processed. + */ + 'webhook_profile' => \App\Webhooks\GateWebhookProfile::class, + + /* + * This class determines the response on a valid webhook call. + */ + 'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class, + + /* + * The classname of the model to be used to store webhook calls. The class should + * be equal or extend Spatie\WebhookClient\Models\WebhookCall. + */ + 'webhook_model' => \Spatie\WebhookClient\Models\WebhookCall::class, + + /* + * In this array, you can pass the headers that should be stored on + * the webhook call model when a webhook comes in. + * + * To store all headers, set this value to `*`. + */ + 'store_headers' => [ + 'X-Gate-Signature', + 'Content-Type', + ], + + /* + * The class name of the job that will process the webhook request. + * + * This should be set to a class that extends \Spatie\WebhookClient\Jobs\ProcessWebhookJob. + */ + 'process_webhook_job' => \App\Webhooks\ProcessGateWebhookJob::class, + ], + ], + + /* + * The integer amount of days after which models should be deleted. + * + * It deletes all records after 30 days. Set to null if no models should be deleted. + */ + 'delete_after_days' => 30, + + /* + * Should a unique token be added to the route name + */ + 'add_unique_token_to_route_name' => false, +]; diff --git a/database/migrations/2025_12_15_045715_create_webhook_calls_table.php b/database/migrations/2025_12_15_045715_create_webhook_calls_table.php new file mode 100644 index 0000000..7c957d2 --- /dev/null +++ b/database/migrations/2025_12_15_045715_create_webhook_calls_table.php @@ -0,0 +1,23 @@ +bigIncrements('id'); + + $table->string('name'); + $table->string('url', 512); + $table->json('headers')->nullable(); + $table->json('payload')->nullable(); + $table->text('exception')->nullable(); + + $table->timestamps(); + }); + } +}; diff --git a/routes/api.php b/routes/api.php index bd1cfb1..f607975 100644 --- a/routes/api.php +++ b/routes/api.php @@ -2,8 +2,6 @@ declare(strict_types=1); -use App\Http\Controllers\WebhookController; use Illuminate\Support\Facades\Route; -Route::post('/webhooks/gate', [WebhookController::class, 'gate']) - ->name('webhooks.gate'); +Route::webhooks('webhooks/gate', 'gate'); diff --git a/tests/Feature/WebhookGateTest.php b/tests/Feature/WebhookGateTest.php index 7c737b7..be6f2e9 100644 --- a/tests/Feature/WebhookGateTest.php +++ b/tests/Feature/WebhookGateTest.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use Spatie\WebhookClient\Models\WebhookCall; + describe('POST /api/webhooks/gate', function () { it('accepts valid gate certification payload', function () { $payload = [ @@ -21,10 +23,10 @@ $response = $this->postJson('/api/webhooks/gate', $payload); $response->assertSuccessful(); - $response->assertJson(['status' => 'accepted']); + $response->assertJson(['message' => 'ok']); }); - it('rejects payload missing required repository field', function () { + it('does not process payload missing required repository field', function () { $payload = [ 'sha' => 'abc123', 'verdict' => 'approved', @@ -32,11 +34,18 @@ $response = $this->postJson('/api/webhooks/gate', $payload); - $response->assertUnprocessable(); - $response->assertJsonValidationErrors(['repository']); + $response->assertSuccessful(); + + $this->assertDatabaseMissing('webhook_calls', [ + 'name' => 'gate', + ]); + + $this->assertDatabaseMissing('verb_events', [ + 'type' => 'App\Events\CertificationCompleted', + ]); }); - it('rejects payload with invalid verdict value', function () { + it('does not process payload with invalid verdict value', function () { $payload = [ 'repository' => 'conduit-ui/pr', 'sha' => 'abc123', @@ -45,8 +54,38 @@ $response = $this->postJson('/api/webhooks/gate', $payload); - $response->assertUnprocessable(); - $response->assertJsonValidationErrors(['verdict']); + $response->assertSuccessful(); + + $this->assertDatabaseMissing('webhook_calls', [ + 'name' => 'gate', + ]); + + $this->assertDatabaseMissing('verb_events', [ + 'type' => 'App\Events\CertificationCompleted', + ]); + }); + + it('stores webhook call in database', function () { + $payload = [ + 'repository' => 'conduit-ui/pr', + 'sha' => 'abc123def456', + 'verdict' => 'approved', + ]; + + $response = $this->postJson('/api/webhooks/gate', $payload); + + $response->assertSuccessful(); + + $this->assertDatabaseHas('webhook_calls', [ + 'name' => 'gate', + ]); + + $webhookCall = WebhookCall::first(); + expect($webhookCall->payload)->toMatchArray([ + 'repository' => 'conduit-ui/pr', + 'sha' => 'abc123def456', + 'verdict' => 'approved', + ]); }); it('stores certification event via Laravel Verbs', function () { @@ -69,4 +108,34 @@ 'type' => 'App\Events\CertificationCompleted', ]); }); + + it('does not process payload missing required sha field', function () { + $payload = [ + 'repository' => 'conduit-ui/pr', + 'verdict' => 'approved', + ]; + + $response = $this->postJson('/api/webhooks/gate', $payload); + + $response->assertSuccessful(); + + $this->assertDatabaseMissing('webhook_calls', [ + 'name' => 'gate', + ]); + }); + + it('does not process payload missing required verdict field', function () { + $payload = [ + 'repository' => 'conduit-ui/pr', + 'sha' => 'abc123', + ]; + + $response = $this->postJson('/api/webhooks/gate', $payload); + + $response->assertSuccessful(); + + $this->assertDatabaseMissing('webhook_calls', [ + 'name' => 'gate', + ]); + }); }); diff --git a/tests/Unit/GateSignatureValidatorTest.php b/tests/Unit/GateSignatureValidatorTest.php new file mode 100644 index 0000000..1434874 --- /dev/null +++ b/tests/Unit/GateSignatureValidatorTest.php @@ -0,0 +1,90 @@ + 'gate', + 'signing_secret' => '', + 'signature_header_name' => 'X-Gate-Signature', + 'signature_validator' => GateSignatureValidator::class, + 'webhook_profile' => \Spatie\WebhookClient\WebhookProfile\ProcessEverythingWebhookProfile::class, + 'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class, + 'webhook_model' => \Spatie\WebhookClient\Models\WebhookCall::class, + 'store_headers' => [], + 'process_webhook_job' => ProcessGateWebhookJob::class, + ]); + + expect($validator->isValid($request, $config))->toBeTrue(); + }); + + it('rejects requests with missing signature when secret is configured', function () { + $validator = new GateSignatureValidator; + $request = Request::create('/webhook', 'POST', [], [], [], [], '{"test": "data"}'); + $config = new WebhookConfig([ + 'name' => 'gate', + 'signing_secret' => 'my-secret-key', + 'signature_header_name' => 'X-Gate-Signature', + 'signature_validator' => GateSignatureValidator::class, + 'webhook_profile' => \Spatie\WebhookClient\WebhookProfile\ProcessEverythingWebhookProfile::class, + 'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class, + 'webhook_model' => \Spatie\WebhookClient\Models\WebhookCall::class, + 'store_headers' => [], + 'process_webhook_job' => ProcessGateWebhookJob::class, + ]); + + expect($validator->isValid($request, $config))->toBeFalse(); + }); + + it('rejects requests with invalid signature', function () { + $validator = new GateSignatureValidator; + $request = Request::create('/webhook', 'POST', [], [], [], [ + 'HTTP_X_GATE_SIGNATURE' => 'invalid-signature', + ], '{"test": "data"}'); + $config = new WebhookConfig([ + 'name' => 'gate', + 'signing_secret' => 'my-secret-key', + 'signature_header_name' => 'X-Gate-Signature', + 'signature_validator' => GateSignatureValidator::class, + 'webhook_profile' => \Spatie\WebhookClient\WebhookProfile\ProcessEverythingWebhookProfile::class, + 'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class, + 'webhook_model' => \Spatie\WebhookClient\Models\WebhookCall::class, + 'store_headers' => [], + 'process_webhook_job' => ProcessGateWebhookJob::class, + ]); + + expect($validator->isValid($request, $config))->toBeFalse(); + }); + + it('accepts requests with valid HMAC signature', function () { + $validator = new GateSignatureValidator; + $payload = '{"test": "data"}'; + $secret = 'my-secret-key'; + $validSignature = hash_hmac('sha256', $payload, $secret); + + $request = Request::create('/webhook', 'POST', [], [], [], [ + 'HTTP_X_GATE_SIGNATURE' => $validSignature, + ], $payload); + $config = new WebhookConfig([ + 'name' => 'gate', + 'signing_secret' => $secret, + 'signature_header_name' => 'X-Gate-Signature', + 'signature_validator' => GateSignatureValidator::class, + 'webhook_profile' => \Spatie\WebhookClient\WebhookProfile\ProcessEverythingWebhookProfile::class, + 'webhook_response' => \Spatie\WebhookClient\WebhookResponse\DefaultRespondsTo::class, + 'webhook_model' => \Spatie\WebhookClient\Models\WebhookCall::class, + 'store_headers' => [], + 'process_webhook_job' => ProcessGateWebhookJob::class, + ]); + + expect($validator->isValid($request, $config))->toBeTrue(); + }); +});