From 65de8c35e66c82a0cb21828912ca6ce3ba1e3e6b Mon Sep 17 00:00:00 2001 From: johnnyq Date: Mon, 27 Jul 2026 15:22:16 -0400 Subject: [PATCH] Prevent duplicate Stripe payment bookings and overlapping cron runs --- cron/cron.php | 8 +++++ guest/guest_pay_invoice_stripe.php | 11 ++++-- itflow-payment-idempotency-noschema.patch | 41 +++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 itflow-payment-idempotency-noschema.patch diff --git a/cron/cron.php b/cron/cron.php index 04d879d7..e1942c5c 100644 --- a/cron/cron.php +++ b/cron/cron.php @@ -8,6 +8,14 @@ if (php_sapi_name() !== 'cli') { die("This script must be run from the command line.\n"); } +// Only one run at a time. Autopay charges cards, so an overlapping run (the previous +// run still going when the next one fires) could bill the same invoice twice. The +// handle is held for the life of the process and released when it exits. +$cron_lock_handle = fopen(sys_get_temp_dir() . '/itflow_cron.lock', 'c'); +if ($cron_lock_handle === false || !flock($cron_lock_handle, LOCK_EX | LOCK_NB)) { + die("Cron is already running - exiting.\n"); +} + require_once "../config.php"; // Set Timezone diff --git a/guest/guest_pay_invoice_stripe.php b/guest/guest_pay_invoice_stripe.php index d7c8c07b..dcbe88af 100644 --- a/guest/guest_pay_invoice_stripe.php +++ b/guest/guest_pay_invoice_stripe.php @@ -223,8 +223,15 @@ if (isset($_GET['invoice_id'], $_GET['url_key']) && !isset($_GET['payment_intent exit(WORDING_PAYMENT_FAILED); } - // Update Invoice Status - mysqli_query($mysqli, "UPDATE invoices SET invoice_status = 'Paid' WHERE invoice_id = $invoice_id"); + // Claim the invoice - the conditional UPDATE is the lock, and the row lock it takes + // is what serialises concurrent requests carrying the same payment intent. A request + // that loses the race matches 0 rows and must not book the payment a second time. + mysqli_query($mysqli, "UPDATE invoices SET invoice_status = 'Paid' WHERE invoice_id = $invoice_id AND invoice_status NOT IN ('Draft', 'Paid', 'Cancelled')"); + if (mysqli_affected_rows($mysqli) !== 1) { + error_log("Stripe payment - invoice $invoice_id was already settled by a concurrent request; skipping duplicate booking of $pi_id"); + header('Location: //' . $config_base_url . '/guest/guest_view_invoice.php?invoice_id=' . $invoice_id . '&url_key=' . $invoice_url_key); + exit(); + } // Add Payment to History mysqli_query($mysqli, "INSERT INTO payments SET payment_date = '$pi_date', payment_amount = $pi_amount_paid, payment_currency_code = '$pi_currency', payment_account_id = $stripe_account, payment_method = 'Stripe', payment_reference = 'Stripe - $pi_id', payment_invoice_id = $invoice_id"); diff --git a/itflow-payment-idempotency-noschema.patch b/itflow-payment-idempotency-noschema.patch new file mode 100644 index 00000000..eb6c2d31 --- /dev/null +++ b/itflow-payment-idempotency-noschema.patch @@ -0,0 +1,41 @@ +diff --git a/cron/cron.php b/cron/cron.php +index 04d879d..e1942c5 100644 +--- a/cron/cron.php ++++ b/cron/cron.php +@@ -8,6 +8,14 @@ if (php_sapi_name() !== 'cli') { + die("This script must be run from the command line.\n"); + } + ++// Only one run at a time. Autopay charges cards, so an overlapping run (the previous ++// run still going when the next one fires) could bill the same invoice twice. The ++// handle is held for the life of the process and released when it exits. ++$cron_lock_handle = fopen(sys_get_temp_dir() . '/itflow_cron.lock', 'c'); ++if ($cron_lock_handle === false || !flock($cron_lock_handle, LOCK_EX | LOCK_NB)) { ++ die("Cron is already running - exiting.\n"); ++} ++ + require_once "../config.php"; + + // Set Timezone +diff --git a/guest/guest_pay_invoice_stripe.php b/guest/guest_pay_invoice_stripe.php +index d7c8c07..dcbe88a 100644 +--- a/guest/guest_pay_invoice_stripe.php ++++ b/guest/guest_pay_invoice_stripe.php +@@ -223,8 +223,15 @@ if (isset($_GET['invoice_id'], $_GET['url_key']) && !isset($_GET['payment_intent + exit(WORDING_PAYMENT_FAILED); + } + +- // Update Invoice Status +- mysqli_query($mysqli, "UPDATE invoices SET invoice_status = 'Paid' WHERE invoice_id = $invoice_id"); ++ // Claim the invoice - the conditional UPDATE is the lock, and the row lock it takes ++ // is what serialises concurrent requests carrying the same payment intent. A request ++ // that loses the race matches 0 rows and must not book the payment a second time. ++ mysqli_query($mysqli, "UPDATE invoices SET invoice_status = 'Paid' WHERE invoice_id = $invoice_id AND invoice_status NOT IN ('Draft', 'Paid', 'Cancelled')"); ++ if (mysqli_affected_rows($mysqli) !== 1) { ++ error_log("Stripe payment - invoice $invoice_id was already settled by a concurrent request; skipping duplicate booking of $pi_id"); ++ header('Location: //' . $config_base_url . '/guest/guest_view_invoice.php?invoice_id=' . $invoice_id . '&url_key=' . $invoice_url_key); ++ exit(); ++ } + + // Add Payment to History + mysqli_query($mysqli, "INSERT INTO payments SET payment_date = '$pi_date', payment_amount = $pi_amount_paid, payment_currency_code = '$pi_currency', payment_account_id = $stripe_account, payment_method = 'Stripe', payment_reference = 'Stripe - $pi_id', payment_invoice_id = $invoice_id");