Skip to content

Commit 4f3628c

Browse files
jimmynjimmyn
committed
feat: enhance IPN webhook handler with enterprise-grade reliability
Align WooCommerce IPN callback handler with PrestaShop implementation for consistency and reliability across platforms. Security improvements: - Return proper HTTP status codes (401, 405, 400, 200) - Always log security violations (not conditional on debug mode) - Catch Throwable instead of Exception for comprehensive error handling - Add missing signature header validation with logging Reliability improvements: - Add transient-based concurrency protection with 30s timeout - Add idempotency check using order meta to prevent duplicate processing - Wrap payment processing in try-catch-finally for error handling - Add $order->save() to persist metadata changes New payment status handling: - Add PENDING status → sets order to on-hold - Add EXPIRED status → sets order to failed - Add REFUNDED status → sets order to refunded - Add PARTIALLY_REFUNDED status → adds order note with amount Critical bug fixes: - Fix exit() bug in amount validation (should be return) - Fix missing order metadata persistence Testing infrastructure: - Add PHPStan stubs for WordPress/WooCommerce functions - Add WC_Geolocation class stub - Configure PHPStan to ignore simplified stub return types - All code passes PHPStan Level 4 and PHPCS WordPress standards
1 parent 92f8094 commit 4f3628c

File tree

3 files changed

+284
-20
lines changed

3 files changed

+284
-20
lines changed

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

Lines changed: 161 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,33 +39,80 @@ public function __construct( bool $logging = false ) {
3939
* @return void
4040
*/
4141
public function check_ipn_request() {
42+
// Enforce POST-only webhook endpoint.
4243
//phpcs:ignore WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
4344
if ( isset( $_SERVER['REQUEST_METHOD'] ) && ( 'POST' !== wc_clean( wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) ) ) {
44-
return;
45+
//phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
46+
WC_Monei_Logger::log( '[MONEI] Webhook received non-POST request: ' . wc_clean( wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) );
47+
http_response_code( 405 );
48+
header( 'Allow: POST' );
49+
header( 'Content-Type: text/plain; charset=utf-8' );
50+
echo 'Method Not Allowed';
51+
exit;
4552
}
4653

4754
$headers = $this->get_all_headers();
4855
$raw_body = file_get_contents( 'php://input' );
4956
$this->log_ipn_request( $headers, $raw_body );
5057

58+
// Check for signature header.
59+
if ( ! isset( $_SERVER['HTTP_MONEI_SIGNATURE'] ) ) {
60+
WC_Monei_Logger::log( '[MONEI] Webhook missing signature header from IP: ' . WC_Geolocation::get_ip_address() );
61+
http_response_code( 401 );
62+
header( 'Content-Type: text/plain; charset=utf-8' );
63+
echo 'Unauthorized';
64+
exit;
65+
}
66+
67+
$payload = null;
68+
5169
try {
52-
if ( ! isset( $_SERVER['HTTP_MONEI_SIGNATURE'] ) ) {
53-
return;
54-
}
5570
//phpcs:ignore WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
5671
$payload = $this->verify_signature_get_payload( $raw_body, wc_clean( wp_unslash( $_SERVER['HTTP_MONEI_SIGNATURE'] ) ) );
5772
$this->logging && WC_Monei_Logger::log( $payload, 'debug' );
73+
} catch ( Throwable $e ) {
74+
// Signature verification failed - this is a security issue, always log.
75+
WC_Monei_Logger::log( '[MONEI] Webhook signature verification failed: ' . $e->getMessage() );
76+
do_action( 'woocommerce_monei_handle_failed_ipn', $payload, $e );
77+
http_response_code( 401 );
78+
header( 'Content-Type: text/plain; charset=utf-8' );
79+
echo 'Unauthorized';
80+
exit;
81+
}
82+
83+
// Acquire lock to prevent concurrent processing of the same payment.
84+
$payment_id = $payload['id'] ?? '';
85+
$lock_key = 'monei_ipn_' . md5( $payment_id );
86+
$lock_value = wp_rand();
87+
88+
if ( ! $this->acquire_lock( $lock_key, $lock_value ) ) {
89+
// Another process is handling this payment.
90+
$this->logging && WC_Monei_Logger::log( '[MONEI] Webhook already being processed [payment_id=' . $payment_id . ']', 'debug' );
91+
http_response_code( 200 );
92+
header( 'Content-Type: text/plain; charset=utf-8' );
93+
echo 'OK';
94+
exit;
95+
}
96+
97+
try {
5898
$this->handle_valid_ipn( $payload );
5999
do_action( 'woocommerce_monei_handle_valid_ipn', $payload );
60100
http_response_code( 200 );
61-
exit();
62-
} catch ( Exception $e ) {
63-
do_action( 'woocommerce_monei_handle_failed_ipn', $payload, $e );
64-
$this->logging && WC_Monei_Logger::log( 'Failed IPN request: ' . $e->getMessage() );
65-
// Invalid signature
101+
header( 'Content-Type: text/plain; charset=utf-8' );
102+
echo 'OK';
103+
} catch ( Throwable $e ) {
104+
// Processing error - always log.
105+
WC_Monei_Logger::log( '[MONEI] Webhook processing error: ' . $e->getMessage() );
106+
do_action( 'woocommerce_monei_handle_processing_error', $payload, $e );
66107
http_response_code( 400 );
67-
exit();
108+
header( 'Content-Type: text/plain; charset=utf-8' );
109+
echo 'Bad Request';
110+
} finally {
111+
// Always release the lock.
112+
$this->release_lock( $lock_key, $lock_value );
68113
}
114+
115+
exit;
69116
}
70117

71118
/**
@@ -91,18 +138,38 @@ protected function handle_valid_ipn( $payload ) {
91138
return;
92139
}
93140

141+
// Check if this payment was already processed (idempotency check).
142+
$processed_payment_id = $order->get_meta( '_monei_payment_id_processed', true );
143+
if ( $processed_payment_id === $monei_id ) {
144+
// Payment already processed, skip to prevent duplicate processing.
145+
$this->logging && WC_Monei_Logger::log( '[MONEI] Payment already processed [payment_id=' . $monei_id . ', order_id=' . $order_id . ']', 'debug' );
146+
return;
147+
}
148+
94149
/**
95150
* Saving related information into order meta.
96151
*/
97152
$order->update_meta_data( '_payment_order_number_monei', $monei_id );
98153
$order->update_meta_data( '_payment_order_status_monei', $status );
99154
$order->update_meta_data( '_payment_order_status_code_monei', $status_code );
100155
$order->update_meta_data( '_payment_order_status_message_monei', $status_message );
156+
$order->update_meta_data( '_monei_payment_id_processed', $monei_id );
157+
$order->save();
158+
159+
if ( 'PENDING' === $status ) {
160+
// Payment is pending (e.g., Multibanco waiting for payment).
161+
$order_note = __( 'HTTP Notification received - <strong>Payment Pending</strong> ', 'monei' ) . '. <br><br>';
162+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $monei_id . '. <br><br>';
163+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $status_message;
164+
$order->add_order_note( $order_note );
165+
$order->update_status( 'on-hold', __( 'Payment pending confirmation', 'monei' ) );
166+
return;
167+
}
101168

102169
if ( 'FAILED' === $status ) {
103170
// Order failed.
104171
$order->add_order_note( __( 'HTTP Notification received - <strong>Payment Failed</strong> ', 'monei' ) . $status );
105-
$order->update_status( 'pending', 'Failed MONEI payment: ' . $status_message );
172+
$order->update_status( 'failed', 'Failed MONEI payment: ' . $status_message );
106173
return;
107174
}
108175

@@ -111,6 +178,16 @@ protected function handle_valid_ipn( $payload ) {
111178
$order->add_order_note( __( 'HTTP Notification received - <strong>Payment Cancelled</strong> ', 'monei' ) . $status );
112179
$message = __( 'Cancelled by MONEI: ', 'monei' ) . $status_message;
113180
$order->add_order_note( $message );
181+
$order->update_status( 'cancelled', $message );
182+
return;
183+
}
184+
185+
if ( 'EXPIRED' === $status ) {
186+
// Payment expired.
187+
$order->add_order_note( __( 'HTTP Notification received - <strong>Payment Expired</strong> ', 'monei' ) . $status );
188+
$message = __( 'Payment expired: ', 'monei' ) . $status_message;
189+
$order->add_order_note( $message );
190+
$order->update_status( 'failed', $message );
114191
return;
115192
}
116193

@@ -131,7 +208,7 @@ protected function handle_valid_ipn( $payload ) {
131208

132209
/**
133210
* If amounts don't match, we mark the order on-hold for manual validation.
134-
* 1 cent exception, for subscriptions when 0 sing ups are done.
211+
* 1 cent exception, for subscriptions when 0 sign ups are done.
135212
*/
136213
if ( ( (int) $amount !== monei_price_format( $order_total ) ) && ( 1 !== $amount ) ) {
137214
$order->update_status(
@@ -143,7 +220,7 @@ protected function handle_valid_ipn( $payload ) {
143220
monei_price_format( $order_total )
144221
)
145222
);
146-
exit;
223+
return;
147224
}
148225

149226
$order_note = __( 'HTTP Notification received - <strong>Payment Completed</strong> ', 'monei' ) . '. <br><br>';
@@ -157,18 +234,42 @@ protected function handle_valid_ipn( $payload ) {
157234
if ( 'completed' === monei_get_settings( 'orderdo', monei_get_option_key_from_order( $order ) ) ) {
158235
$order->update_status( 'completed', __( 'Order Completed by MONEI', 'monei' ) );
159236
}
237+
return;
238+
}
239+
240+
if ( 'REFUNDED' === $status ) {
241+
// Payment fully refunded.
242+
$order_note = __( 'HTTP Notification received - <strong>Payment Refunded</strong> ', 'monei' ) . '. <br><br>';
243+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $monei_id . '. <br><br>';
244+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $status_message;
245+
$order->add_order_note( $order_note );
246+
$order->update_status( 'refunded', __( 'Payment refunded by MONEI', 'monei' ) );
247+
return;
248+
}
249+
250+
if ( 'PARTIALLY_REFUNDED' === $status ) {
251+
// Payment partially refunded.
252+
$refunded_amount = $payload['refundedAmount'] ?? 0;
253+
$order_note = __( 'HTTP Notification received - <strong>Payment Partially Refunded</strong> ', 'monei' ) . '. <br><br>';
254+
$order_note .= __( 'MONEI Transaction id: ', 'monei' ) . $monei_id . '. <br><br>';
255+
$order_note .= __( 'Refunded amount: ', 'monei' ) . wc_price( $refunded_amount / 100 ) . '. <br><br>';
256+
$order_note .= __( 'MONEI Status Message: ', 'monei' ) . $status_message;
257+
$order->add_order_note( $order_note );
258+
// Note: WooCommerce doesn't have a built-in 'partially-refunded' status.
259+
// The order remains in its current status with a note about the partial refund.
260+
return;
160261
}
161262
}
162263

163264
/**
164265
* Verify signature, if all good returns payload.
165-
* Throws Exception if Signaturit not valid.
266+
* Throws Exception if signature is not valid.
166267
*
167-
* @param $request_body
168-
* @param $monei_signature
268+
* @param string $request_body The request body.
269+
* @param string $monei_signature The MONEI signature header.
169270
*
170271
* @return array
171-
* @throws \OpenAPI\Client\ApiException
272+
* @throws \Monei\ApiException
172273
*/
173274
protected function verify_signature_get_payload( $request_body, $monei_signature ) {
174275
$decoded_body = json_decode( $request_body );
@@ -182,7 +283,7 @@ protected function verify_signature_get_payload( $request_body, $monei_signature
182283
* getallheaders is only available for apache, we need a fallback in case of nginx or others,
183284
* http://php.net/manual/es/function.getallheaders.php
184285
*
185-
* @return array|false
286+
* @return array
186287
*/
187288
private function get_all_headers() {
188289
if ( ! function_exists( 'getallheaders' ) ) {
@@ -194,7 +295,7 @@ private function get_all_headers() {
194295
}
195296
return $headers;
196297
} else {
197-
return getallheaders();
298+
return getallheaders() ?: array();
198299
}
199300
}
200301

@@ -209,4 +310,45 @@ protected function log_ipn_request( $headers, $raw_body ) {
209310
$headers = implode( "\n", $headers );
210311
$this->logging && WC_Monei_Logger::log( 'IPN Request from ' . WC_Geolocation::get_ip_address() . ': ' . "\n\n" . $headers . "\n\n" . $raw_body . "\n", 'debug' );
211312
}
313+
314+
/**
315+
* Acquire a lock using WordPress transients.
316+
*
317+
* @param string $lock_key The lock key.
318+
* @param mixed $lock_value The lock value.
319+
* @param int $timeout Lock timeout in seconds (default 30).
320+
* @return bool True if lock acquired, false otherwise.
321+
*/
322+
private function acquire_lock( $lock_key, $lock_value, $timeout = 30 ) {
323+
// Try to set transient. If it already exists, add_transient returns false.
324+
$acquired = set_transient( $lock_key, $lock_value, $timeout );
325+
326+
if ( ! $acquired ) {
327+
// Transient already exists. Check if it's stale.
328+
$existing_value = get_transient( $lock_key );
329+
if ( false === $existing_value ) {
330+
// Transient expired between checks, try again.
331+
return set_transient( $lock_key, $lock_value, $timeout );
332+
}
333+
return false;
334+
}
335+
336+
return true;
337+
}
338+
339+
/**
340+
* Release a lock using WordPress transients.
341+
*
342+
* @param string $lock_key The lock key.
343+
* @param mixed $lock_value The lock value to verify ownership.
344+
* @return void
345+
*/
346+
private function release_lock( $lock_key, $lock_value ) {
347+
$existing_value = get_transient( $lock_key );
348+
349+
// Only delete if we own the lock.
350+
if ( $existing_value === $lock_value ) {
351+
delete_transient( $lock_key );
352+
}
353+
}
212354
}

‎phpstan.neon‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ parameters:
3939
- '#Access to an undefined property#'
4040
# WordPress actions can have return values (they're ignored by WordPress)
4141
- '#Action callback returns .+ but should not return anything\.#'
42+
# Bootstrap stubs can have simplified return types
43+
-
44+
message: '#never returns .+ so it can be removed from the return type#'
45+
path: tests/phpstan-bootstrap.php
4246

4347
# Performance optimizations
4448
parallel:

0 commit comments

Comments
 (0)