Skip to content

Commit 8561db1

Browse files
jimmynjimmyn
committed
fix: prevent race conditions in payment processing with atomic locks
- Created WC_Monei_Lock_Helper with atomic wp_cache_add() operations - Fixed broken lock in IPN webhook (was using set_transient which always overwrites) - Added lock mechanism to redirect verification - Ensured consistent lock keys (monei_payment_ + payment_id) across IPN and redirect - Increased lock timeout from 30s to 60s (aligned with Magento) - Added try-finally blocks for guaranteed lock release - Added PHPStan stubs for WordPress cache functions This prevents duplicate order processing when webhook and redirect arrive simultaneously. Verified working in production - idempotency check logs confirm no duplicate processing.
1 parent 9fb5bec commit 8561db1

File tree

5 files changed

+189
-106
lines changed

5 files changed

+189
-106
lines changed

‎class-woocommerce-gateway-monei.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ private function includes() {
139139
$container = ContainerProvider::getContainer();
140140
include_once 'includes/woocommerce-gateway-monei-core-functions.php';
141141
include_once 'includes/class-wc-monei-ipn.php';
142+
include_once 'includes/class-wc-monei-lock-helper.php';
142143
include_once 'includes/class-wc-monei-logger.php';
143144
include_once 'includes/class-wc-monei-payment-method-display.php';
144145

‎includes/class-wc-monei-ipn.php‎

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,12 @@ public function check_ipn_request() {
8686
}
8787

8888
// Acquire lock to prevent concurrent processing of the same payment.
89+
// IMPORTANT: Must use same lock key pattern as redirect verification to prevent race conditions.
8990
$payment_id = $payload['id'] ?? '';
90-
$lock_key = 'monei_ipn_' . md5( $payment_id );
91+
$lock_key = WC_Monei_Lock_Helper::get_payment_lock_key( $payment_id );
9192
$lock_value = wp_rand();
9293

93-
if ( ! $this->acquire_lock( $lock_key, $lock_value ) ) {
94+
if ( ! WC_Monei_Lock_Helper::acquire_lock( $lock_key, $lock_value ) ) {
9495
// Another process is handling this payment.
9596
$this->logging && WC_Monei_Logger::log( '[MONEI] Webhook already being processed [payment_id=' . $payment_id . ']', 'debug' );
9697
http_response_code( 200 );
@@ -114,7 +115,7 @@ public function check_ipn_request() {
114115
echo 'Bad Request';
115116
} finally {
116117
// Always release the lock.
117-
$this->release_lock( $lock_key, $lock_value );
118+
WC_Monei_Lock_Helper::release_lock( $lock_key, $lock_value );
118119
}
119120

120121
exit;
@@ -329,45 +330,4 @@ protected function log_ipn_request( $headers, $raw_body ) {
329330
$headers = implode( "\n", $headers );
330331
$this->logging && WC_Monei_Logger::log( 'IPN Request from ' . WC_Geolocation::get_ip_address() . ': ' . "\n\n" . $headers . "\n\n" . $raw_body . "\n", 'debug' );
331332
}
332-
333-
/**
334-
* Acquire a lock using WordPress transients.
335-
*
336-
* @param string $lock_key The lock key.
337-
* @param mixed $lock_value The lock value.
338-
* @param int $timeout Lock timeout in seconds (default 30).
339-
* @return bool True if lock acquired, false otherwise.
340-
*/
341-
private function acquire_lock( $lock_key, $lock_value, $timeout = 30 ) {
342-
// Try to set transient. If it already exists, add_transient returns false.
343-
$acquired = set_transient( $lock_key, $lock_value, $timeout );
344-
345-
if ( ! $acquired ) {
346-
// Transient already exists. Check if it's stale.
347-
$existing_value = get_transient( $lock_key );
348-
if ( false === $existing_value ) {
349-
// Transient expired between checks, try again.
350-
return set_transient( $lock_key, $lock_value, $timeout );
351-
}
352-
return false;
353-
}
354-
355-
return true;
356-
}
357-
358-
/**
359-
* Release a lock using WordPress transients.
360-
*
361-
* @param string $lock_key The lock key.
362-
* @param mixed $lock_value The lock value to verify ownership.
363-
* @return void
364-
*/
365-
private function release_lock( $lock_key, $lock_value ) {
366-
$existing_value = get_transient( $lock_key );
367-
368-
// Only delete if we own the lock.
369-
if ( $existing_value === $lock_value ) {
370-
delete_transient( $lock_key );
371-
}
372-
}
373333
}

‎includes/class-wc-monei-lock-helper.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
/**
3+
* MONEI Lock Helper
4+
* Provides locking mechanism to prevent concurrent payment processing
5+
*
6+
* @package MONEI
7+
* @since 6.5.0
8+
*/
9+
10+
if ( ! defined( 'ABSPATH' ) ) {
11+
exit; // Exit if accessed directly
12+
}
13+
14+
/**
15+
* Lock Helper Class
16+
* Provides shared locking functionality for payment processing
17+
*/
18+
class WC_Monei_Lock_Helper {
19+
20+
/**
21+
* Acquire a lock using WordPress object cache or transients.
22+
*
23+
* @param string $lock_key The lock key.
24+
* @param mixed $lock_value The lock value.
25+
* @param int $timeout Lock timeout in seconds (default 60).
26+
* @return bool True if lock acquired, false otherwise.
27+
*/
28+
public static function acquire_lock( $lock_key, $lock_value, $timeout = 60 ) {
29+
// Try wp_cache_add first (atomic, fails if key exists)
30+
// This requires object cache, but works with default WordPress cache
31+
$acquired = wp_cache_add( $lock_key, $lock_value, 'monei_locks', $timeout );
32+
33+
if ( ! $acquired ) {
34+
// Lock already held by another process
35+
// Check if it's stale by trying to get it
36+
$existing_value = wp_cache_get( $lock_key, 'monei_locks' );
37+
if ( false === $existing_value ) {
38+
// Cache expired between checks, try again
39+
return wp_cache_add( $lock_key, $lock_value, 'monei_locks', $timeout );
40+
}
41+
return false;
42+
}
43+
44+
return true;
45+
}
46+
47+
/**
48+
* Release a lock using WordPress object cache.
49+
*
50+
* @param string $lock_key The lock key.
51+
* @param mixed $lock_value The lock value to verify ownership.
52+
* @return void
53+
*/
54+
public static function release_lock( $lock_key, $lock_value ) {
55+
$existing_value = wp_cache_get( $lock_key, 'monei_locks' );
56+
57+
// Only delete if we own the lock.
58+
if ( $existing_value === $lock_value ) {
59+
wp_cache_delete( $lock_key, 'monei_locks' );
60+
}
61+
}
62+
63+
/**
64+
* Generate a consistent lock key for a payment ID.
65+
*
66+
* @param string $payment_id The MONEI payment ID.
67+
* @return string The lock key.
68+
*/
69+
public static function get_payment_lock_key( $payment_id ) {
70+
return 'monei_payment_' . $payment_id;
71+
}
72+
}

‎includes/class-wc-monei-redirect-hooks.php‎

Lines changed: 77 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -200,81 +200,96 @@ private function verify_and_complete_order( $order_id, $payment ) {
200200
return;
201201
}
202202

203-
// Check if payment was already processed (prevent duplicate processing)
204-
$processed_payment_id = $order->get_meta( '_monei_payment_id_processed', true );
205-
if ( $processed_payment_id === $payment->getId() ) {
206-
WC_Monei_Logger::log( sprintf( '[MONEI] Payment already processed via IPN [payment_id=%s, order_id=%s]', $payment->getId(), $order_id ), 'debug' );
203+
// Acquire lock to prevent race condition with IPN webhook.
204+
// IMPORTANT: Must use same lock key pattern as IPN to prevent race conditions.
205+
$lock_key = WC_Monei_Lock_Helper::get_payment_lock_key( $payment->getId() );
206+
$lock_value = wp_rand();
207+
208+
if ( ! WC_Monei_Lock_Helper::acquire_lock( $lock_key, $lock_value, 60 ) ) {
209+
WC_Monei_Logger::log( sprintf( '[MONEI] Order completion already in progress [order_id=%s, payment_id=%s]', $order_id, $payment->getId() ), 'debug' );
207210
return;
208211
}
209212

210-
/** @var string $payment_status */
211-
$payment_status = $payment->getStatus();
212-
$order_status = $order->get_status();
213+
try {
214+
// Check if payment was already processed (prevent duplicate processing)
215+
$processed_payment_id = $order->get_meta( '_monei_payment_id_processed', true );
216+
if ( $processed_payment_id === $payment->getId() ) {
217+
WC_Monei_Logger::log( sprintf( '[MONEI] Payment already processed via IPN [payment_id=%s, order_id=%s]', $payment->getId(), $order_id ), 'debug' );
218+
return;
219+
}
213220

214-
WC_Monei_Logger::log( sprintf( '[MONEI] Redirect verification [payment_id=%s, order_id=%s, payment_status=%s, order_status=%s]', $payment->getId(), $order_id, $payment_status, $order_status ), 'debug' );
221+
/** @var string $payment_status */
222+
$payment_status = $payment->getStatus();
223+
$order_status = $order->get_status();
215224

216-
// Only process if order is still pending/on-hold/failed and payment succeeded
217-
if ( ! in_array( $order_status, array( 'pending', 'on-hold', 'failed' ), true ) ) {
218-
WC_Monei_Logger::log( sprintf( '[MONEI] Order already processed, skipping [order_id=%s, status=%s]', $order_id, $order_status ), 'debug' );
219-
return;
220-
}
225+
WC_Monei_Logger::log( sprintf( '[MONEI] Redirect verification [payment_id=%s, order_id=%s, payment_status=%s, order_status=%s]', $payment->getId(), $order_id, $payment_status, $order_status ), 'debug' );
221226

222-
// If payment is SUCCEEDED or AUTHORIZED, complete the order
223-
if ( PaymentStatus::SUCCEEDED === $payment_status || PaymentStatus::AUTHORIZED === $payment_status ) {
224-
$amount = $payment->getAmount();
225-
$order_total = $order->get_total();
226-
227-
// Verify amounts match (with 1 cent exception for subscriptions)
228-
if ( ( (int) $amount !== monei_price_format( $order_total ) ) && ( 1 !== $amount ) ) {
229-
$order->update_status(
230-
'on-hold',
231-
sprintf(
232-
/* translators: 1: Order amount, 2: Payment amount */
233-
__( 'Validation error: Order vs. Payment amounts do not match (order: %1$s - received: %2$s).', 'monei' ),
234-
monei_price_format( $order_total ),
235-
$amount
236-
)
237-
);
238-
WC_Monei_Logger::log( sprintf( '[MONEI] Amount mismatch [order_id=%s, order_amount=%s, payment_amount=%s]', $order_id, monei_price_format( $order_total ), $amount ), 'error' );
227+
// Only process if order is still pending/on-hold/failed and payment succeeded
228+
if ( ! in_array( $order_status, array( 'pending', 'on-hold', 'failed' ), true ) ) {
229+
WC_Monei_Logger::log( sprintf( '[MONEI] Order already processed, skipping [order_id=%s, status=%s]', $order_id, $order_status ), 'debug' );
239230
return;
240231
}
241232

242-
// Mark payment as processed to prevent duplicate processing by IPN
243-
$order->update_meta_data( '_monei_payment_id_processed', $payment->getId() );
244-
$order->update_meta_data( '_payment_order_number_monei', $payment->getId() );
245-
$order->update_meta_data( '_payment_order_status_monei', $payment_status );
246-
$order->update_meta_data( '_payment_order_status_code_monei', $payment->getStatusCode() );
247-
$order->update_meta_data( '_payment_order_status_message_monei', $payment->getStatusMessage() );
248-
249-
// Store formatted payment method display
250-
$payment_method_display = $this->paymentMethodFormatter->get_payment_method_display_from_payment( $payment );
251-
if ( $payment_method_display ) {
252-
$order->update_meta_data( '_monei_payment_method_display', $payment_method_display );
253-
}
233+
// If payment is SUCCEEDED or AUTHORIZED, complete the order
234+
if ( PaymentStatus::SUCCEEDED === $payment_status || PaymentStatus::AUTHORIZED === $payment_status ) {
235+
$amount = $payment->getAmount();
236+
$order_total = $order->get_total();
237+
238+
// Verify amounts match (with 1 cent exception for subscriptions)
239+
if ( ( (int) $amount !== monei_price_format( $order_total ) ) && ( 1 !== $amount ) ) {
240+
$order->update_status(
241+
'on-hold',
242+
sprintf(
243+
/* translators: 1: Order amount, 2: Payment amount */
244+
__( 'Validation error: Order vs. Payment amounts do not match (order: %1$s - received: %2$s).', 'monei' ),
245+
monei_price_format( $order_total ),
246+
$amount
247+
)
248+
);
249+
WC_Monei_Logger::log( sprintf( '[MONEI] Amount mismatch [order_id=%s, order_amount=%s, payment_amount=%s]', $order_id, monei_price_format( $order_total ), $amount ), 'error' );
250+
return;
251+
}
254252

255-
if ( PaymentStatus::AUTHORIZED === $payment_status ) {
256-
$order->update_meta_data( '_payment_not_captured_monei', 1 );
257-
$order_note = __( 'Payment verified via redirect - <strong>Payment Authorized</strong>', 'monei' ) . '. <br><br>';
258-
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
259-
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
260-
$order->add_order_note( $order_note );
261-
$order->update_status( 'on-hold', __( 'Order On-Hold by MONEI', 'monei' ) );
262-
} else {
263-
// SUCCEEDED
264-
$order_note = __( 'Payment verified via redirect - <strong>Payment Completed</strong>', 'monei' ) . '. <br><br>';
265-
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
266-
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
267-
$order->add_order_note( $order_note );
268-
$order->payment_complete();
253+
// Mark payment as processed to prevent duplicate processing by IPN
254+
$order->update_meta_data( '_monei_payment_id_processed', $payment->getId() );
255+
$order->update_meta_data( '_payment_order_number_monei', $payment->getId() );
256+
$order->update_meta_data( '_payment_order_status_monei', $payment_status );
257+
$order->update_meta_data( '_payment_order_status_code_monei', $payment->getStatusCode() );
258+
$order->update_meta_data( '_payment_order_status_message_monei', $payment->getStatusMessage() );
259+
260+
// Store formatted payment method display
261+
$payment_method_display = $this->paymentMethodFormatter->get_payment_method_display_from_payment( $payment );
262+
if ( $payment_method_display ) {
263+
$order->update_meta_data( '_monei_payment_method_display', $payment_method_display );
264+
}
269265

270-
$payment_method_woo_id = $order->get_payment_method();
271-
if ( 'completed' === monei_get_settings( 'orderdo', $payment_method_woo_id ) ) {
272-
$order->update_status( 'completed', __( 'Order Completed by MONEI', 'monei' ) );
266+
if ( PaymentStatus::AUTHORIZED === $payment_status ) {
267+
$order->update_meta_data( '_payment_not_captured_monei', 1 );
268+
$order_note = __( 'Payment verified via redirect - <strong>Payment Authorized</strong>', 'monei' ) . '. <br><br>';
269+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
270+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
271+
$order->add_order_note( $order_note );
272+
$order->update_status( 'on-hold', __( 'Order On-Hold by MONEI', 'monei' ) );
273+
} else {
274+
// SUCCEEDED
275+
$order_note = __( 'Payment verified via redirect - <strong>Payment Completed</strong>', 'monei' ) . '. <br><br>';
276+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $payment->getId() . '. <br><br>';
277+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $payment->getStatusMessage();
278+
$order->add_order_note( $order_note );
279+
$order->payment_complete();
280+
281+
$payment_method_woo_id = $order->get_payment_method();
282+
if ( 'completed' === monei_get_settings( 'orderdo', $payment_method_woo_id ) ) {
283+
$order->update_status( 'completed', __( 'Order Completed by MONEI', 'monei' ) );
284+
}
273285
}
274-
}
275286

276-
$order->save();
277-
WC_Monei_Logger::log( sprintf( '[MONEI] Order completed via redirect verification [order_id=%s, payment_status=%s]', $order_id, $payment_status ), 'debug' );
287+
$order->save();
288+
WC_Monei_Logger::log( sprintf( '[MONEI] Order completed via redirect verification [order_id=%s, payment_status=%s]', $order_id, $payment_status ), 'debug' );
289+
}
290+
} finally {
291+
// Always release the lock.
292+
WC_Monei_Lock_Helper::release_lock( $lock_key, $lock_value );
278293
}
279294
}
280295
}

‎tests/phpstan-bootstrap.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,41 @@ function set_transient( $transient, $value, $expiration = 0 ) {
130130
}
131131
}
132132

133+
if ( ! function_exists( 'wp_cache_add' ) ) {
134+
/**
135+
* @param string $key
136+
* @param mixed $data
137+
* @param string $group
138+
* @param int $expire
139+
* @return bool
140+
*/
141+
function wp_cache_add( $key, $data, $group = '', $expire = 0 ) {
142+
return true;
143+
}
144+
}
145+
146+
if ( ! function_exists( 'wp_cache_get' ) ) {
147+
/**
148+
* @param string $key
149+
* @param string $group
150+
* @return mixed
151+
*/
152+
function wp_cache_get( $key, $group = '' ) {
153+
return false;
154+
}
155+
}
156+
157+
if ( ! function_exists( 'wp_cache_delete' ) ) {
158+
/**
159+
* @param string $key
160+
* @param string $group
161+
* @return bool
162+
*/
163+
function wp_cache_delete( $key, $group = '' ) {
164+
return true;
165+
}
166+
}
167+
133168
if ( ! function_exists( 'get_transient' ) ) {
134169
/**
135170
* @param string $transient

0 commit comments

Comments
 (0)