Skip to content

Commit

Permalink
Remove config repository dependency from webhook middleware
Browse files Browse the repository at this point in the history
Similarly to laravel#791, we can remove the `\Illuminate\Contracts\Config\Repository` in the `VerifyWebhookSignature`'s constructor. For consistency, the configuration is accessed with the `config()` helper like the rest of the code.

Since we don't need to mock the configuration repository anymore, the test has been moved to the integration suite, because we need to bootstrap the framework.

As a bonus, I have added tests for checking [replay attacks](https://stripe.com/docs/webhooks/signatures#replay-attacks) against the tolerance configuration setting.
  • Loading branch information
sebdesign committed Sep 30, 2019
1 parent 4a53b46 commit d2570e6
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 124 deletions.
23 changes: 2 additions & 21 deletions src/Http/Middleware/VerifyWebhookSignature.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,12 @@
namespace Laravel\Cashier\Http\Middleware;

use Closure;
use Illuminate\Contracts\Config\Repository as Config;
use Stripe\Exception\SignatureVerificationException;
use Stripe\WebhookSignature;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

class VerifyWebhookSignature
{
/**
* The configuration repository instance.
*
* @var \Illuminate\Contracts\Config\Repository
*/
protected $config;

/**
* Create a new middleware instance.
*
* @param \Illuminate\Contracts\Config\Repository $config
* @return void
*/
public function __construct(Config $config)
{
$this->config = $config;
}

/**
* Handle the incoming request.
*
Expand All @@ -43,8 +24,8 @@ public function handle($request, Closure $next)
WebhookSignature::verifyHeader(
$request->getContent(),
$request->header('Stripe-Signature'),
$this->config->get('cashier.webhook.secret'),
$this->config->get('cashier.webhook.tolerance')
config('cashier.webhook.secret'),
config('cashier.webhook.tolerance')
);
} catch (SignatureVerificationException $exception) {
throw new AccessDeniedHttpException($exception->getMessage(), $exception);
Expand Down
134 changes: 134 additions & 0 deletions tests/Integration/VerifyWebhookSignatureTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
<?php

namespace Laravel\Cashier\Tests\Feature;

use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Laravel\Cashier\Http\Middleware\VerifyWebhookSignature;
use Laravel\Cashier\Tests\TestCase;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

class VerifyWebhookSignatureTest extends TestCase
{
/**
* @var \Illuminate\Http\Request
*/
protected $request;

/**
* @var int
*/
protected $timestamp;

public function setUp(): void
{
parent::setUp();

config(['cashier.webhook.secret' => 'secret']);
config(['cashier.webhook.tolerance' => 300]);

$this->request = new Request([], [], [], [], [], [], 'Signed Body');
}

public function test_response_is_received_when_secret_matches()
{
$this->withTimestamp(time());
$this->withSignedSignature('secret');

$response = (new VerifyWebhookSignature())
->handle($this->request, function ($request) {
return new Response('OK');
});

$this->assertEquals('OK', $response->content());
}

public function test_response_is_received_when_timestamp_is_within_tolerance_zone()
{
$this->withTimestamp(time() - 300);
$this->withSignedSignature('secret');

$response = (new VerifyWebhookSignature())
->handle($this->request, function ($request) {
return new Response('OK');
});

$this->assertEquals('OK', $response->content());
}

public function test_app_aborts_when_timestamp_is_too_old()
{
$this->withTimestamp(time() - 301);
$this->withSignedSignature('secret');

$this->expectException(AccessDeniedHttpException::class);
$this->expectExceptionMessage('Timestamp outside the tolerance zone');

$response = (new VerifyWebhookSignature())
->handle($this->request, function ($request) {
});
}

public function test_app_aborts_when_timestamp_is_invalid()
{
$this->withTimestamp('invalid');
$this->withSignedSignature('secret');

$this->expectException(AccessDeniedHttpException::class);
$this->expectExceptionMessage('Unable to extract timestamp and signatures from header');

$response = (new VerifyWebhookSignature())
->handle($this->request, function ($request) {
});
}

public function test_app_aborts_when_secret_does_not_match()
{
$this->withTimestamp(time());
$this->withSignature('fail');

$this->expectException(AccessDeniedHttpException::class);
$this->expectExceptionMessage('No signatures found matching the expected signature for payload');

(new VerifyWebhookSignature())
->handle($this->request, function ($request) {
});
}

public function test_app_aborts_when_no_secret_was_provided()
{
$this->withTimestamp(time());
$this->withSignedSignature('');

$this->expectException(AccessDeniedHttpException::class);
$this->expectExceptionMessage('No signatures found matching the expected signature for payload');

(new VerifyWebhookSignature())
->handle($this->request, function ($request) {
});
}

public function withTimestamp($timestamp)
{
$this->timestamp = $timestamp;
}

public function withSignedSignature($secret)
{
return $this->withSignature(
$this->sign($this->request->getContent(), $secret)
);
}

public function withSignature($signature)
{
$this->request->headers->set('Stripe-Signature', 't='.$this->timestamp.',v1='.$signature);

return $this;
}

private function sign($payload, $secret)
{
return hash_hmac('sha256', $this->timestamp.'.'.$payload, $secret);
}
}
103 changes: 0 additions & 103 deletions tests/Unit/VerifyWebhookSignatureTest.php

This file was deleted.

0 comments on commit d2570e6

Please sign in to comment.