diff --git a/app/Http/Controllers/Api/PaymentWebhookController.php b/app/Http/Controllers/Api/PaymentWebhookController.php index 0e59c89..6a5146f 100755 --- a/app/Http/Controllers/Api/PaymentWebhookController.php +++ b/app/Http/Controllers/Api/PaymentWebhookController.php @@ -11,6 +11,8 @@ use App\Services\Payments\PurchaseProvisioner; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; +use RuntimeException; +use Throwable; final class PaymentWebhookController extends Controller { @@ -36,7 +38,7 @@ final class PaymentWebhookController extends Controller try { $verifier->verify($request, $raw); - } catch (\Throwable $e) { + } catch (Throwable $e) { report($e); return response()->json([ @@ -50,7 +52,7 @@ final class PaymentWebhookController extends Controller true, flags: JSON_THROW_ON_ERROR ); - } catch (\Throwable) { + } catch (Throwable) { return response()->json([ 'message' => 'Invalid JSON.', ], 400); @@ -76,16 +78,16 @@ final class PaymentWebhookController extends Controller ], 400); } + $payloadHash = hash('sha256', $raw); + try { /* - * Paddle can deliver several related webhook events at almost - * exactly the same time. + * Paddle can deliver several related events concurrently. * - * Laravel will automatically retry the entire transaction when - * MySQL reports a deadlock / serialization failure. - * - * Five attempts gives short-lived webhook contention time to - * resolve without weakening any database constraints. + * Laravel retries this complete transaction when MySQL reports + * a deadlock. Five attempts gives short-lived lock contention + * enough opportunity to resolve without acknowledging an event + * that was not actually processed. */ DB::transaction( function () use ( @@ -93,15 +95,14 @@ final class PaymentWebhookController extends Controller $externalID, $eventType, $payload, - $raw, + $payloadHash, $provisioner ): void { /* - * firstOrCreate protects against Paddle retrying the exact - * same event ID. + * Insert the event if this is its first delivery. * - * lockForUpdate prevents two workers handling the same - * webhook event concurrently after the row exists. + * Once present, lock the row so duplicate deliveries of + * the same event cannot provision concurrently. */ LicenseWebhookEvent::firstOrCreate( [ @@ -110,7 +111,7 @@ final class PaymentWebhookController extends Controller ], [ 'event_type' => $eventType, - 'payload_sha256' => hash('sha256', $raw), + 'payload_sha256' => $payloadHash, 'payload' => $payload, ] ); @@ -121,27 +122,15 @@ final class PaymentWebhookController extends Controller ->lockForUpdate() ->firstOrFail(); - /* - * Already successfully processed. - * - * Returning here makes duplicate delivery of the exact - * same Paddle event idempotent. - */ - if ($event->processed_at !== null) { + if ($event->processed_at) { return; } - /* - * An external event ID must never be reused with different - * content. - */ - $incomingHash = hash('sha256', $raw); - if (! hash_equals( (string) $event->payload_sha256, - $incomingHash + $payloadHash )) { - throw new \RuntimeException( + throw new RuntimeException( 'Webhook ID was reused with a different payload.' ); } @@ -152,31 +141,30 @@ final class PaymentWebhookController extends Controller $payload ); - $event->update([ - 'processed_at' => now(), - 'processing_error' => null, - ]); - } catch (\Throwable $e) { + $event->processed_at = now(); + $event->processing_error = null; + $event->save(); + } catch (Throwable $e) { /* - * This update is part of the same transaction. - * - * If processing fails and the transaction rolls back, - * Paddle can retry the webhook normally. + * This assignment is useful when the surrounding + * transaction eventually succeeds. If processing + * throws, the transaction itself is rolled back and + * Paddle will retry the event. */ - $event->update([ - 'processing_error' => mb_substr( - $e->getMessage(), - 0, - 4000 - ), - ]); + $event->processing_error = mb_substr( + $e->getMessage(), + 0, + 4000 + ); + + $event->save(); throw $e; } }, 5 ); - } catch (\Throwable $e) { + } catch (Throwable $e) { report($e); return response()->json([ @@ -189,15 +177,15 @@ final class PaymentWebhookController extends Controller } /* - * Delivery is intentionally outside the webhook transaction. + * Delivery deliberately happens outside the webhook transaction. * - * A temporary mail outage must not roll back an otherwise valid - * payment event. Pending outbox rows remain available for portal - * resend or a later webhook delivery attempt. + * A temporary SMTP failure must not undo a successfully processed + * payment event. The outbox retains anything that still needs + * delivery. */ try { $delivery->deliverPending(10); - } catch (\Throwable $e) { + } catch (Throwable $e) { report($e); } diff --git a/app/Services/Payments/PurchaseProvisioner.php b/app/Services/Payments/PurchaseProvisioner.php index 0eeca47..9217590 100755 --- a/app/Services/Payments/PurchaseProvisioner.php +++ b/app/Services/Payments/PurchaseProvisioner.php @@ -8,7 +8,9 @@ use App\Models\LicenseProduct; use App\Services\Accounts\PurchasedAccountProvisioner; use App\Services\Licensing\LicenseIssuer; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use RuntimeException; +use Throwable; final class PurchaseProvisioner { @@ -32,16 +34,24 @@ final class PurchaseProvisioner return; } - throw new RuntimeException('Unsupported payment provider.'); + throw new RuntimeException( + 'Unsupported payment provider.' + ); } private function provisionPaddle(array $event): void { $type = (string) ($event['event_type'] ?? ''); - $data = $event['data'] ?? []; + + $data = is_array($event['data'] ?? null) + ? $event['data'] + : []; if (str_starts_with($type, 'subscription.')) { - $this->syncPaddleSubscription($type, $data); + $this->syncPaddleSubscription( + $type, + $data + ); return; } @@ -53,11 +63,17 @@ final class PurchaseProvisioner } if ($type === 'transaction.refunded') { - $transactionID = (string) ($data['id'] ?? ''); + $transactionID = $this->nullableString( + $data['id'] ?? null + ); - if ($transactionID !== '') { - License::where('payment_provider', 'paddle') - ->where('external_order_id', $transactionID) + if ($transactionID !== null) { + License::query() + ->where('payment_provider', 'paddle') + ->where( + 'external_order_id', + $transactionID + ) ->whereNull('external_subscription_id') ->whereNull('revoked_at') ->update([ @@ -68,35 +84,67 @@ final class PurchaseProvisioner } } - private function syncPaddleSubscription(string $type, array $data): void - { - $subscriptionID = (string) ($data['id'] ?? ''); + private function syncPaddleSubscription( + string $type, + array $data + ): void { + $subscriptionID = $this->nullableString( + $data['id'] ?? null + ); - if ($subscriptionID === '') { + if ($subscriptionID === null) { throw new RuntimeException( 'Paddle subscription webhook has no subscription ID.' ); } /* - * Paddle webhook delivery order is not guaranteed. + * Multiple Paddle events for the same subscription can arrive + * simultaneously. * - * A transaction.completed event can arrive before the corresponding - * subscription event has created our local licence. Capture the - * transaction ID from the subscription payload as well whenever - * Paddle provides one. + * The unique index protects database integrity, but by itself it + * cannot stop two workers from both observing "no licence" before + * either worker inserts the subscription ID. + * + * A MySQL advisory lock serialises provisioning for one Paddle + * subscription while still allowing unrelated subscriptions to be + * processed concurrently. */ + $lockName = 'sonardesk:paddle:subscription:' + .$subscriptionID; + + $this->withMysqlLock( + $lockName, + function () use ( + $type, + $data, + $subscriptionID + ): void { + $this->syncLockedPaddleSubscription( + $type, + $data, + $subscriptionID + ); + } + ); + } + + private function syncLockedPaddleSubscription( + string $type, + array $data, + string $subscriptionID + ): void { $transactionID = $this->nullableString( $data['transaction_id'] ?? null ); $status = (string) ($data['status'] ?? ''); - $item = $this->firstRecurringItem( - is_array($data['items'] ?? null) - ? $data['items'] - : [] - ); + $items = is_array($data['items'] ?? null) + ? $data['items'] + : []; + + $item = $this->firstRecurringItem($items); $price = is_array($item['price'] ?? null) ? $item['price'] @@ -115,11 +163,20 @@ final class PurchaseProvisioner ); /* - * Subscription ID is the authoritative identity for recurring - * Paddle licences. + * The Paddle subscription ID is the authoritative identity for a + * recurring licence. + * + * lockForUpdate protects an existing licence while we mutate it. + * Creation of a missing licence is serialised by the advisory lock + * acquired by syncPaddleSubscription(). */ - $license = License::where('payment_provider', 'paddle') - ->where('external_subscription_id', $subscriptionID) + $license = License::query() + ->where('payment_provider', 'paddle') + ->where( + 'external_subscription_id', + $subscriptionID + ) + ->lockForUpdate() ->first(); if (! $license) { @@ -137,13 +194,10 @@ final class PurchaseProvisioner $data['customer_id'] ?? null ); - $customer = $this->paddle->customer($customerID) ?? []; + $customer = $this->paddle->customer( + $customerID + ) ?? []; - /* - * Use the transaction ID as the initial external order ID where - * available. If it is absent, transaction.completed can backfill - * it later. - */ [$license] = $this->issuer->issue( $product, $this->nullableString( @@ -164,19 +218,42 @@ final class PurchaseProvisioner true, ); - $this->accounts->ensure( - $license->customer_email, - $license->customer_name - ); + /* + * LicenseIssuer may return an existing licence if the + * transaction ID has already been seen. + * + * Re-read it with a row lock before attaching the Paddle + * subscription. + */ + $license = License::query() + ->whereKey($license->getKey()) + ->lockForUpdate() + ->firstOrFail(); - $license->external_subscription_id = $subscriptionID; + /* + * Do not attach this subscription to a licence already owned + * by a different subscription. + */ + if ( + $license->external_subscription_id !== null + && $license->external_subscription_id + !== $subscriptionID + ) { + throw new RuntimeException( + 'Paddle transaction resolved to a licence ' + .'belonging to another subscription.' + ); + } + + $license->external_subscription_id = + $subscriptionID; } elseif ( $priceID !== '' && $priceID !== $license->external_price_id ) { /* - * Upgrades/downgrades change the entitlement attached to the - * existing licence key. + * Upgrades and downgrades modify the entitlement attached to + * the existing licence key rather than issuing another key. */ $product = $this->mappedProduct( 'paddle', @@ -186,11 +263,15 @@ final class PurchaseProvisioner $license->license_product_id = $product->id; $license->edition = $product->edition; - $license->activation_limit = $product->activation_limit; - $license->max_major_version = $product->max_major_version; + $license->activation_limit = + $product->activation_limit; + $license->max_major_version = + $product->max_major_version; } - $periodEnd = $data['current_billing_period']['ends_at'] ?? null; + $periodEnd = + $data['current_billing_period']['ends_at'] + ?? null; $scheduled = $data['scheduled_change'] ?? null; @@ -200,14 +281,18 @@ final class PurchaseProvisioner : null; $license->external_customer_id = - $this->nullableString($data['customer_id'] ?? null) + $this->nullableString( + $data['customer_id'] ?? null + ) ?? $license->external_customer_id; - $this->backfillPaddleCustomer($license, $data); + $this->backfillPaddleCustomer( + $license, + $data + ); /* - * ensure() must be idempotent. Existing users must simply be - * returned rather than created again. + * Account provisioning is expected to be idempotent. */ $this->accounts->ensure( $license->customer_email, @@ -218,8 +303,12 @@ final class PurchaseProvisioner $license->external_price_id = $priceID; } - if ($transactionID && ! $license->external_order_id) { - $license->external_order_id = $transactionID; + if ( + $transactionID !== null + && ! $license->external_order_id + ) { + $license->external_order_id = + $transactionID; } if ($status !== '') { @@ -227,7 +316,8 @@ final class PurchaseProvisioner } if ($periodEnd) { - $license->current_period_ends_at = Carbon::parse($periodEnd); + $license->current_period_ends_at = + Carbon::parse($periodEnd); } $license->cancel_scheduled_at = $cancelAt @@ -235,41 +325,51 @@ final class PurchaseProvisioner : null; /* - * Entitlement handling. + * Subscription entitlement. */ - if (in_array($status, ['active', 'trialing'], true)) { + if (in_array( + $status, + ['active', 'trialing'], + true + )) { $license->revoked_at = null; $license->revoke_reason = null; - /* - * Active/trialing subscriptions remain valid through the - * current Paddle billing period. - */ $license->expires_at = $periodEnd ? Carbon::parse($periodEnd) : null; } elseif ($status === 'past_due') { /* - * Do not revoke immediately after the first failed renewal. - * Preserve Paddle's current billing period where possible. + * A failed renewal does not immediately destroy the + * entitlement. Preserve Paddle's current billing period when + * available and otherwise provide the existing grace period. */ $license->expires_at = $periodEnd ? Carbon::parse($periodEnd) - : ($license->expires_at ?? now()->addDays(7)); - } elseif (in_array($status, ['canceled', 'paused'], true)) { + : ( + $license->expires_at + ?? now()->addDays(7) + ); + } elseif (in_array( + $status, + ['canceled', 'paused'], + true + )) { $license->expires_at = now(); $license->revoked_at = now(); - $license->revoke_reason = $status === 'paused' - ? 'Subscription paused' - : 'Subscription canceled'; + $license->revoke_reason = + $status === 'paused' + ? 'Subscription paused' + : 'Subscription canceled'; } $license->save(); } - private function syncPaddleTransaction(array $data): void - { + private function syncPaddleTransaction( + array $data + ): void { $subscriptionID = $this->nullableString( $data['subscription_id'] ?? null ); @@ -279,28 +379,58 @@ final class PurchaseProvisioner ); /* - * Recurring transactions belong to an existing subscription. + * A recurring transaction belongs to its subscription. * - * The subscription event normally creates the licence. If the - * transaction event arrives afterwards we simply backfill the - * transaction/order ID. - * - * If transaction.completed arrives first, it does nothing here. - * The subsequent subscription event will obtain transaction_id - * directly from its payload where Paddle supplies it. + * Serialise this operation using the same lock as subscription + * provisioning. This prevents transaction.completed from updating + * the licence while a subscription event is still creating or + * modifying it. */ - if ($subscriptionID) { - if ($transactionID) { - License::where('payment_provider', 'paddle') - ->where( - 'external_subscription_id', - $subscriptionID - ) - ->whereNull('external_order_id') - ->update([ - 'external_order_id' => $transactionID, - ]); - } + if ($subscriptionID !== null) { + $lockName = 'sonardesk:paddle:subscription:' + .$subscriptionID; + + $this->withMysqlLock( + $lockName, + function () use ( + $subscriptionID, + $transactionID + ): void { + if ($transactionID === null) { + return; + } + + $license = License::query() + ->where( + 'payment_provider', + 'paddle' + ) + ->where( + 'external_subscription_id', + $subscriptionID + ) + ->lockForUpdate() + ->first(); + + /* + * transaction.completed may legitimately arrive before + * subscription.created. In that case there is nothing + * to update yet. The subscription event will create the + * licence and Paddle's transaction_id can populate the + * order ID. + */ + if (! $license) { + return; + } + + if (! $license->external_order_id) { + $license->external_order_id = + $transactionID; + + $license->save(); + } + } + ); return; } @@ -344,7 +474,9 @@ final class PurchaseProvisioner $data['customer_id'] ?? null ); - $customer = $this->paddle->customer($customerID) ?? []; + $customer = $this->paddle->customer( + $customerID + ) ?? []; [$license] = $this->issuer->issue( $product, @@ -372,8 +504,9 @@ final class PurchaseProvisioner ); } - private function provisionLemonSqueezy(array $event): void - { + private function provisionLemonSqueezy( + array $event + ): void { $type = (string) ( $event['meta']['event_name'] ?? '' @@ -393,13 +526,17 @@ final class PurchaseProvisioner if (in_array( $type, - ['order_refunded', 'subscription_expired'], + [ + 'order_refunded', + 'subscription_expired', + ], true )) { - License::where( - 'payment_provider', - 'lemon-squeezy' - ) + License::query() + ->where( + 'payment_provider', + 'lemon-squeezy' + ) ->where('external_order_id', $order) ->whereNull('revoked_at') ->update([ @@ -413,7 +550,10 @@ final class PurchaseProvisioner if (! in_array( $type, - ['order_created', 'subscription_created'], + [ + 'order_created', + 'subscription_created', + ], true )) { return; @@ -421,8 +561,14 @@ final class PurchaseProvisioner $product = $this->mappedProduct( 'lemon-squeezy', - (string) ($attributes['variant_id'] ?? ''), - (string) ($attributes['product_id'] ?? ''), + (string) ( + $attributes['variant_id'] + ?? '' + ), + (string) ( + $attributes['product_id'] + ?? '' + ), ); [$license] = $this->issuer->issue( @@ -451,7 +597,9 @@ final class PurchaseProvisioner License $license, array $data ): void { - $custom = is_array($data['custom_data'] ?? null) + $custom = is_array( + $data['custom_data'] ?? null + ) ? $data['custom_data'] : []; @@ -460,7 +608,9 @@ final class PurchaseProvisioner ?? $license->external_customer_id ); - $customer = $this->paddle->customer($customerID) ?? []; + $customer = $this->paddle->customer( + $customerID + ) ?? []; $name = $this->nullableString( $custom['customer_name'] @@ -485,10 +635,11 @@ final class PurchaseProvisioner $license->customer_email = $email; - LicenseDeliveryOutbox::where( - 'license_id', - $license->id - ) + LicenseDeliveryOutbox::query() + ->where( + 'license_id', + $license->id + ) ->whereNull('delivered_at') ->update([ 'recipient_email' => $email, @@ -502,26 +653,31 @@ final class PurchaseProvisioner string $productID ): LicenseProduct { /* - * Price IDs are authoritative. - * - * Monthly and annual Paddle prices can intentionally belong to the - * same Paddle product, therefore product ID alone cannot reliably - * identify the licence plan. + * Price IDs are authoritative because multiple billing intervals + * can belong to the same Paddle product. */ if ($priceID !== '') { - $product = LicenseProduct::where( - 'payment_provider', - $provider - ) - ->where('external_price_id', $priceID) + $product = LicenseProduct::query() + ->where( + 'payment_provider', + $provider + ) + ->where( + 'external_price_id', + $priceID + ) ->where('active', true) ->first(); } else { - $product = LicenseProduct::where( - 'payment_provider', - $provider - ) - ->where('external_product_id', $productID) + $product = LicenseProduct::query() + ->where( + 'payment_provider', + $provider + ) + ->where( + 'external_product_id', + $productID + ) ->where('active', true) ->first(); } @@ -536,8 +692,9 @@ final class PurchaseProvisioner return $product; } - private function firstRecurringItem(array $items): array - { + private function firstRecurringItem( + array $items + ): array { foreach ($items as $item) { if (! is_array($item)) { continue; @@ -550,11 +707,14 @@ final class PurchaseProvisioner $first = $items[0] ?? []; - return is_array($first) ? $first : []; + return is_array($first) + ? $first + : []; } - private function nullableString(mixed $value): ?string - { + private function nullableString( + mixed $value + ): ?string { if (! is_scalar($value)) { return null; } @@ -565,4 +725,62 @@ final class PurchaseProvisioner ? null : $value; } + + /** + * Execute a callback while holding a MySQL advisory lock. + * + * The lock is connection-scoped, so it must always be explicitly + * released in finally. A short timeout is sufficient because Paddle + * will retry failed webhook deliveries and the controller also retries + * database deadlocks. + */ + private function withMysqlLock( + string $name, + callable $callback + ): mixed { + /* + * MySQL advisory lock names have a finite length. Hashing also + * prevents provider supplied identifiers from creating unusual + * lock names. + */ + $lockName = 'sd:'.hash('sha256', $name); + + $row = DB::selectOne( + 'SELECT GET_LOCK(?, ?) AS acquired', + [ + $lockName, + 10, + ] + ); + + $acquired = (int) ( + $row->acquired + ?? 0 + ); + + if ($acquired !== 1) { + throw new RuntimeException( + 'Could not acquire Paddle subscription lock.' + ); + } + + try { + return $callback(); + } finally { + try { + DB::selectOne( + 'SELECT RELEASE_LOCK(?) AS released', + [$lockName] + ); + } catch (Throwable $e) { + /* + * Do not hide the original processing exception merely + * because releasing an advisory lock failed. MySQL also + * releases connection-owned locks when the connection + * closes. + */ + report($e); + } + } + } }