mirror of
https://github.com/coollabsio/coolify.git
synced 2026-09-26 01:10:30 -04:00
fix(webhooks): reject incomplete Stripe configuration
This commit is contained in:
@@ -46,3 +46,6 @@
|
||||
- Use the database as the correctness source for dynamic cron occurrences shared by multiple scheduler and Horizon nodes; Redis locks are load controls, not a durable execution ledger.
|
||||
- Give each schedule occurrence a unique database identity and make queue consumers claim it atomically before external work.
|
||||
- Keep pending occurrences recoverable across publisher interruptions, and define an explicit bounded policy for late or offline schedules.
|
||||
|
||||
## Fail closed at public webhook boundaries
|
||||
- Reject missing or blank secrets before signature verification, and return generic errors without logging secrets, signatures, or payloads.
|
||||
|
||||
@@ -4,17 +4,24 @@ namespace App\Http\Controllers\Webhook;
|
||||
|
||||
use App\Http\Controllers\Controller;
|
||||
use App\Jobs\StripeProcessJob;
|
||||
use Exception;
|
||||
use Illuminate\Http\Request;
|
||||
use Stripe\Exception\SignatureVerificationException;
|
||||
use Stripe\Webhook;
|
||||
use Throwable;
|
||||
|
||||
class Stripe extends Controller
|
||||
{
|
||||
public function events(Request $request)
|
||||
{
|
||||
try {
|
||||
$apiKey = config('subscription.stripe_api_key');
|
||||
$webhookSecret = config('subscription.stripe_webhook_secret');
|
||||
if (! is_string($apiKey) || trim($apiKey) === '' || ! is_string($webhookSecret) || trim($webhookSecret) === '') {
|
||||
auditLogWebhookFailure('stripe', 'stripe_not_configured');
|
||||
|
||||
return response('Invalid signature.', 400);
|
||||
}
|
||||
|
||||
$signature = $request->header('Stripe-Signature');
|
||||
$event = Webhook::constructEvent(
|
||||
$request->getContent(),
|
||||
@@ -24,14 +31,14 @@ class Stripe extends Controller
|
||||
StripeProcessJob::dispatch($event);
|
||||
|
||||
return response('Webhook received. Cool cool cool cool cool.', 200);
|
||||
} catch (SignatureVerificationException $e) {
|
||||
auditLogWebhookFailure('stripe', 'invalid_signature', [
|
||||
'error' => $e->getMessage(),
|
||||
]);
|
||||
} catch (SignatureVerificationException) {
|
||||
auditLogWebhookFailure('stripe', 'invalid_signature');
|
||||
|
||||
return response($e->getMessage(), 400);
|
||||
} catch (Exception $e) {
|
||||
return response($e->getMessage(), 400);
|
||||
return response('Invalid signature.', 400);
|
||||
} catch (Throwable) {
|
||||
auditLogWebhookFailure('stripe', 'invalid_payload');
|
||||
|
||||
return response('Invalid webhook.', 400);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\StripeProcessJob;
|
||||
use App\Models\Server;
|
||||
use App\Models\Subscription;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Testing\TestResponse;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
Queue::fake();
|
||||
config()->set('subscription.stripe_api_key', 'sk_test_configured');
|
||||
});
|
||||
|
||||
function stripeWebhookPayload(): string
|
||||
{
|
||||
return json_encode([
|
||||
'id' => 'evt_security_test',
|
||||
'type' => 'customer.subscription.updated',
|
||||
'data' => ['object' => [
|
||||
'id' => 'sub_test',
|
||||
'customer' => 'cus_test',
|
||||
'metadata' => ['team_id' => 999],
|
||||
'status' => 'unpaid',
|
||||
]],
|
||||
], JSON_THROW_ON_ERROR);
|
||||
}
|
||||
|
||||
function stripeSignature(string $payload, string $secret, ?int $timestamp = null): string
|
||||
{
|
||||
$timestamp ??= time();
|
||||
|
||||
return sprintf('t=%d,v1=%s', $timestamp, hash_hmac('sha256', $timestamp.'.'.$payload, $secret));
|
||||
}
|
||||
|
||||
function postStripeWebhook(string $payload, ?string $signature = null): TestResponse
|
||||
{
|
||||
$server = ['CONTENT_TYPE' => 'application/json'];
|
||||
if ($signature !== null) {
|
||||
$server['HTTP_STRIPE_SIGNATURE'] = $signature;
|
||||
}
|
||||
|
||||
return test()->call('POST', '/webhooks/payments/stripe/events', [], [], [], $server, $payload);
|
||||
}
|
||||
|
||||
test('missing null and blank webhook secrets fail closed without dispatching work', function (?string $secret) {
|
||||
config()->set('subscription.stripe_webhook_secret', $secret);
|
||||
$payload = stripeWebhookPayload();
|
||||
|
||||
postStripeWebhook($payload, stripeSignature($payload, trim((string) $secret)))
|
||||
->assertBadRequest()
|
||||
->assertContent('Invalid signature.');
|
||||
|
||||
Queue::assertNotPushed(StripeProcessJob::class);
|
||||
})->with([
|
||||
'null' => null,
|
||||
'empty' => '',
|
||||
'spaces' => ' ',
|
||||
]);
|
||||
|
||||
test('missing null and blank Stripe API keys disable the webhook', function (?string $apiKey) {
|
||||
config()->set('subscription.stripe_api_key', $apiKey);
|
||||
config()->set('subscription.stripe_webhook_secret', 'whsec_correct');
|
||||
$payload = stripeWebhookPayload();
|
||||
|
||||
postStripeWebhook($payload, stripeSignature($payload, 'whsec_correct'))
|
||||
->assertBadRequest()
|
||||
->assertContent('Invalid signature.');
|
||||
|
||||
Queue::assertNotPushed(StripeProcessJob::class);
|
||||
})->with([
|
||||
'null' => null,
|
||||
'empty' => '',
|
||||
'spaces' => ' ',
|
||||
]);
|
||||
|
||||
test('missing malformed invalid expired and wrong signatures fail closed', function (?string $signature) {
|
||||
config()->set('subscription.stripe_webhook_secret', 'whsec_correct');
|
||||
$payload = stripeWebhookPayload();
|
||||
|
||||
if ($signature === 'expired') {
|
||||
$signature = stripeSignature($payload, 'whsec_correct', time() - 301);
|
||||
} elseif ($signature === 'wrong') {
|
||||
$signature = stripeSignature($payload, 'whsec_wrong');
|
||||
}
|
||||
|
||||
postStripeWebhook($payload, $signature)
|
||||
->assertBadRequest()
|
||||
->assertContent('Invalid signature.');
|
||||
|
||||
Queue::assertNotPushed(StripeProcessJob::class);
|
||||
})->with([
|
||||
'missing' => null,
|
||||
'malformed' => 'not-a-stripe-signature',
|
||||
'invalid' => 't=123,v1=invalid',
|
||||
'expired' => 'expired',
|
||||
'wrong secret' => 'wrong',
|
||||
]);
|
||||
|
||||
test('an event cannot change state when Stripe is not configured', function () {
|
||||
config()->set('constants.coolify.self_hosted', true);
|
||||
config()->set('subscription.stripe_webhook_secret', null);
|
||||
$team = Team::factory()->create();
|
||||
$subscription = Subscription::create([
|
||||
'team_id' => $team->id,
|
||||
'stripe_subscription_id' => 'sub_existing',
|
||||
'stripe_customer_id' => 'cus_existing',
|
||||
'stripe_invoice_paid' => true,
|
||||
]);
|
||||
$server = Server::factory()->create(['team_id' => $team->id]);
|
||||
$server->settings()->update(['is_usable' => true, 'is_reachable' => true]);
|
||||
$payload = stripeWebhookPayload();
|
||||
|
||||
postStripeWebhook($payload, stripeSignature($payload, ''))->assertBadRequest();
|
||||
|
||||
expect($subscription->fresh()->stripe_subscription_id)->toBe('sub_existing')
|
||||
->and($subscription->fresh()->stripe_invoice_paid)->toBeTruthy()
|
||||
->and($server->fresh()->settings->is_usable)->toBeTruthy()
|
||||
->and($server->fresh()->settings->is_reachable)->toBeTruthy();
|
||||
Queue::assertNotPushed(StripeProcessJob::class);
|
||||
});
|
||||
|
||||
test('a valid signature with a malformed payload is rejected before dispatch', function () {
|
||||
config()->set('subscription.stripe_webhook_secret', 'whsec_correct');
|
||||
$payload = '{malformed';
|
||||
|
||||
postStripeWebhook($payload, stripeSignature($payload, 'whsec_correct'))
|
||||
->assertBadRequest()
|
||||
->assertContent('Invalid webhook.');
|
||||
|
||||
Queue::assertNotPushed(StripeProcessJob::class);
|
||||
});
|
||||
|
||||
test('a valid signed event is accepted in cloud and self-hosted modes', function (bool $selfHosted) {
|
||||
config()->set('constants.coolify.self_hosted', $selfHosted);
|
||||
config()->set('subscription.stripe_webhook_secret', 'whsec_correct');
|
||||
$payload = stripeWebhookPayload();
|
||||
|
||||
postStripeWebhook($payload, stripeSignature($payload, 'whsec_correct'))->assertSuccessful();
|
||||
|
||||
Queue::assertPushed(StripeProcessJob::class, 1);
|
||||
})->with([
|
||||
'cloud' => false,
|
||||
'self-hosted with Stripe configured' => true,
|
||||
]);
|
||||
Reference in New Issue
Block a user