| @@ -254,10 +254,20 @@ | ||
| 254 | 254 | |
| 255 | 255 | $transactionId = $transactionData['id']; |
| 256 | 256 | $oldTransaction = Transaction::find($transactionId); |
| 257 | 257 | |
| 258 | + if (!$oldTransaction) { | |
| 259 | + wp_send_json_error(['message' => __('Transaction not found.', 'fluentform')], 404); | |
| 260 | + } | |
| 261 | + | |
| 258 | 262 | $changingStatus = $oldTransaction->status != $transactionData['status']; |
| 259 | 263 | |
| 264 | + // Only a *changed* status is validated; a row may already hold one this build does not | |
| 265 | + // register, e.g. Pro's 'requires_review', and editing other fields must not be blocked. | |
| 266 | + if ($changingStatus && !isset(PaymentHelper::getPaymentStatuses()[$transactionData['status']])) { | |
| 267 | + wp_send_json_error(['message' => __('Invalid payment status.', 'fluentform')], 422); | |
| 268 | + } | |
| 269 | + | |
| 260 | 270 | $updateData = ArrayHelper::only($transactionData, [ |
| 261 | 271 | 'payer_name', |
| 262 | 272 | 'payer_email', |
| 263 | 273 | 'billing_address', |
| @@ -271,15 +281,21 @@ | ||
| 271 | 281 | Transaction::where('id', $transactionId)->update($updateData); |
| 272 | 282 | |
| 273 | 283 | // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in route registration |
| 274 | 284 | if ($subscriptionId) { |
| 275 | - $existingSubscription = Subscription::find($subscriptionId); | |
| 285 | + // Bind to the transaction's submission; submission_id comes from the row, not the request. | |
| 286 | + $existingSubscription = Subscription::where('id', $subscriptionId) | |
| 287 | + ->where('submission_id', $oldTransaction->submission_id) | |
| 288 | + ->first(); | |
| 276 | 289 | |
| 277 | 290 | $changedStatus = ArrayHelper::get($transactionData, 'status'); |
| 278 | 291 | |
| 279 | - $isStatusChanged = $existingSubscription->status != $changedStatus; | |
| 292 | + // Only mirror real subscription statuses; 'paid' here would block cancellation forever. | |
| 293 | + $isMappable = $existingSubscription | |
| 294 | + && isset(PaymentHelper::getSubscriptionStatuses()[$changedStatus]) | |
| 295 | + && $existingSubscription->status != $changedStatus; | |
| 280 | 296 | |
| 281 | - if ($isStatusChanged) { | |
| 297 | + if ($isMappable) { | |
| 282 | 298 | Subscription::where('id', $subscriptionId) |
| 283 | 299 | ->update([ |
| 284 | 300 | 'status' => $changedStatus, |
| 285 | 301 | 'updated_at' => current_time('mysql') |
| @@ -295,10 +311,10 @@ | ||
| 295 | 311 | } |
| 296 | 312 | }; |
| 297 | 313 | |
| 298 | 314 | if ( |
| 299 | - ($changingStatus && ($newStatus == 'refunded' || $newStatus == 'partial-refunded')) || | |
| 300 | - ($newStatus == 'partial-refunded' && ArrayHelper::get($transactionData, 'refund_amount')) | |
| 315 | + ($changingStatus && ($newStatus == 'refunded' || $newStatus == 'partially-refunded')) || | |
| 316 | + ($newStatus == 'partially-refunded' && ArrayHelper::get($transactionData, 'refund_amount')) | |
| 301 | 317 | ) { |
| 302 | 318 | $refundAmount = 0; |
| 303 | 319 | $refundNote = 'Refunded by Admin'; |
| 304 | 320 | |
| @@ -306,9 +322,9 @@ | ||
| 306 | 322 | // Handle refund here |
| 307 | 323 | $refundAmount = $oldTransaction->payment_total; |
| 308 | 324 | } else if ($newStatus == 'partially-refunded') { |
| 309 | 325 | $refundAmount = ArrayHelper::get($transactionData, 'refund_amount') * 100; |
| 310 | - $refundNote = ArrayHelper::get($transactionData, 'refund_note'); | |
| 326 | + $refundNote = ArrayHelper::get($transactionData, 'refund_note') ?: $refundNote; | |
| 311 | 327 | } |
| 312 | 328 | |
| 313 | 329 | if ($refundAmount) { |
| 314 | 330 | $baseProcessor->setSubmissionId($oldTransaction->submission_id); |
| @@ -314,8 +330,12 @@ | ||
| 314 | 330 | $baseProcessor->setSubmissionId($oldTransaction->submission_id); |
| 315 | 331 | |
| 316 | 332 | $submission = $baseProcessor->getSubmission(); |
| 317 | 333 | $baseProcessor->refund($refundAmount, $oldTransaction, $submission, $oldTransaction->payment_method, 'refund_' . time(), $refundNote); |
| 334 | + | |
| 335 | + // refund() derives the real status from the refunded total: an amount covering | |
| 336 | + // the whole charge is a full refund, whatever status was requested. | |
| 337 | + $newStatus = Transaction::find($transactionId)->status; | |
| 318 | 338 | } |
| 319 | 339 | |
| 320 | 340 | } |
| 321 | 341 | |
| @@ -321,10 +341,10 @@ | ||
| 321 | 341 | |
| 322 | 342 | if ($changingStatus) { |
| 323 | 343 | |
| 324 | 344 | if ($newStatus == 'paid' || $newStatus == 'pending' || $newStatus == 'processing') { |
| 325 | - // Delete All Refunds | |
| 326 | - Transaction::bySubmission($oldTransaction->submission_id)->refunds()->delete(); | |
| 345 | + // Delete All Refunds, recording them first | |
| 346 | + $this->recordAndRemoveRefundLedger($oldTransaction, $newStatus); | |
| 327 | 347 | } |
| 328 | 348 | |
| 329 | 349 | $baseProcessor->setSubmissionId($oldTransaction->submission_id); |
| 330 | 350 | $baseProcessor->changeSubmissionPaymentStatus($newStatus); |
| @@ -350,8 +370,57 @@ | ||
| 350 | 370 | 'message' => __('Successfully updated data', 'fluentform') |
| 351 | 371 | ], 200); |
| 352 | 372 | } |
| 353 | 373 | |
| 374 | + /** | |
| 375 | + * The record is the compensating control for an irreversible delete, so if it cannot be written the rows must survive. | |
| 376 | + */ | |
| 377 | + private function recordAndRemoveRefundLedger($oldTransaction, $newStatus) | |
| 378 | + { | |
| 379 | + $refunds = Transaction::bySubmission($oldTransaction->submission_id)->refunds()->get(); | |
| 380 | + | |
| 381 | + if (!count($refunds)) { | |
| 382 | + return; | |
| 383 | + } | |
| 384 | + | |
| 385 | + $ids = []; | |
| 386 | + $total = 0; | |
| 387 | + $records = []; | |
| 388 | + | |
| 389 | + foreach ($refunds as $refund) { | |
| 390 | + $ids[] = $refund->id; | |
| 391 | + $total += $refund->payment_total; | |
| 392 | + $records[] = '#' . $refund->id . ' (' . PaymentHelper::formatMoney($refund->payment_total, $refund->currency) . ')'; | |
| 393 | + } | |
| 394 | + | |
| 395 | + $description = sprintf( | |
| 396 | + /* translators: 1: previous status, 2: new status, 3: number of refund records, 4: formatted total, 5: the deleted records */ | |
| 397 | + __( | |
| 398 | + 'Payment status changed from %1$s to %2$s, which removed %3$d refund record(s) totalling %4$s: %5$s', | |
| 399 | + 'fluentform' | |
| 400 | + ), | |
| 401 | + $oldTransaction->status, | |
| 402 | + $newStatus, | |
| 403 | + count($refunds), | |
| 404 | + PaymentHelper::formatMoney($total, $oldTransaction->currency), | |
| 405 | + implode(', ', $records) | |
| 406 | + ); | |
| 407 | + | |
| 408 | + // Record first: if this write fails the rows are still here to try again. | |
| 409 | + do_action('fluentform/log_data', [ | |
| 410 | + 'parent_source_id' => $oldTransaction->form_id, | |
| 411 | + 'source_type' => 'submission_item', | |
| 412 | + 'source_id' => $oldTransaction->submission_id, | |
| 413 | + 'component' => 'Payment', | |
| 414 | + 'status' => 'info', | |
| 415 | + 'title' => __('Refund records deleted', 'fluentform'), | |
| 416 | + 'description' => $description, | |
| 417 | + ]); | |
| 418 | + | |
| 419 | + // Only the rows just recorded, so a refund added meanwhile is not destroyed unrecorded. | |
| 420 | + Transaction::whereIn('id', $ids)->delete(); | |
| 421 | + } | |
| 422 | + | |
| 354 | 423 | public function getStripeConnectConfig() |
| 355 | 424 | { |
| 356 | 425 | wp_send_json_success(ConnectConfig::getConnectConfig()); |
| 357 | 426 | } |
| @@ -408,40 +477,74 @@ | ||
| 408 | 477 | } |
| 409 | 478 | |
| 410 | 479 | // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in route registration |
| 411 | 480 | $transactionId = ArrayHelper::get($attributes, 'transaction_id', 0); |
| 412 | - // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in route registration | |
| 413 | - $submissionId = ArrayHelper::get($attributes, 'submission_id', 0); | |
| 414 | 481 | |
| 415 | - $oldTransaction = Transaction::find($transactionId); | |
| 482 | + // SECURITY (FINDING-29): bind the cancelled records to the AUTHORIZED SUBSCRIPTION itself, | |
| 483 | + // not merely its form. The submission is derived from the subscription (not the request), and | |
| 484 | + // the transaction must belong to that submission AND this subscription — so a per-form | |
| 485 | + // payment manager cannot flip an unrelated subscription's transaction/submission from the | |
| 486 | + // same form to cancelled. subscription_id is null on some legacy rows, so it is matched with | |
| 487 | + // a null-safe fallback while submission_id (1:1 with the subscription) does the hard binding. | |
| 488 | + $subscriptionSubmissionId = (int) $subscription->submission_id; | |
| 416 | 489 | |
| 417 | - $oldSubmission = Submission::find($submissionId); | |
| 490 | + $subscriptionTxnScope = function ($query) use ($subscription, $subscriptionSubmissionId) { | |
| 491 | + return $query | |
| 492 | + ->where('submission_id', $subscriptionSubmissionId) | |
| 493 | + ->where(function ($q) use ($subscription) { | |
| 494 | + $q->where('subscription_id', $subscription->id) | |
| 495 | + ->orWhereNull('subscription_id') | |
| 496 | + ->orWhere('subscription_id', 0); | |
| 497 | + }); | |
| 498 | + }; | |
| 418 | 499 | |
| 419 | - if ($oldTransaction && $oldSubmission) { | |
| 420 | - $isStatusNotCancelled = $oldTransaction->status !== 'cancelled' && $oldSubmission->payment_status !== 'cancelled'; | |
| 500 | + // FINDING-29 + review #243: derive the subscription's OWN transaction from the authorized | |
| 501 | + // scope. Prefer the request's transaction_id only when it resolves inside that scope; the id | |
| 502 | + // is optional in the admin UI, so a missing/stale/mismatched value must not leave the local | |
| 503 | + // transaction active while the submission and gateway are cancelled — fall back to the | |
| 504 | + // subscription's transaction. | |
| 505 | + $oldTransaction = $subscriptionTxnScope( | |
| 506 | + $transactionId ? Transaction::where('id', $transactionId) : Transaction::query() | |
| 507 | + )->first(); | |
| 421 | 508 | |
| 422 | - if ($isStatusNotCancelled) { | |
| 423 | - Transaction::where('id', $transactionId) | |
| 509 | + if (!$oldTransaction) { | |
| 510 | + $oldTransaction = $subscriptionTxnScope(Transaction::query())->first(); | |
| 511 | + } | |
| 512 | + | |
| 513 | + $oldSubmission = Submission::where('id', $subscriptionSubmissionId)->first(); | |
| 514 | + | |
| 515 | + // CORRECTNESS (review #243): cancel at the gateway FIRST — it holds the authoritative state. | |
| 516 | + // Only touch local records after it confirms, so a gateway failure never leaves us showing | |
| 517 | + // "cancelled" locally while the subscription keeps charging. | |
| 518 | + $response = (new PaymentManagement())->cancelSubscription($subscription); | |
| 519 | + | |
| 520 | + if (is_wp_error($response)) { | |
| 521 | + wp_send_json_error([ | |
| 522 | + 'message' => $response->get_error_code() . ' - ' . $response->get_error_message() | |
| 523 | + ], 423); | |
| 524 | + } | |
| 525 | + | |
| 526 | + // Gateway cancelled: reconcile each local record independently (so a partially-cancelled | |
| 527 | + // state is completed rather than skipped) and atomically. The transaction stays scoped to | |
| 528 | + // this subscription (FINDING-29), so an unrelated same-form transaction cannot be flipped. | |
| 529 | + $now = current_time('mysql'); | |
| 530 | + wpFluent()->transaction(function () use ($subscriptionTxnScope, $subscriptionSubmissionId, $oldTransaction, $oldSubmission, $now) { | |
| 531 | + if ($oldTransaction && $oldTransaction->status !== 'cancelled') { | |
| 532 | + $subscriptionTxnScope(Transaction::where('id', $oldTransaction->id)) | |
| 424 | 533 | ->update([ |
| 425 | - 'status' => 'cancelled', | |
| 426 | - 'updated_at' => current_time('mysql') | |
| 534 | + 'status' => 'cancelled', | |
| 535 | + 'updated_at' => $now | |
| 427 | 536 | ]); |
| 537 | + } | |
| 428 | 538 | |
| 429 | - Submission::where('id', $submissionId) | |
| 539 | + if ($oldSubmission && $oldSubmission->payment_status !== 'cancelled') { | |
| 540 | + Submission::where('id', $subscriptionSubmissionId) | |
| 430 | 541 | ->update([ |
| 431 | 542 | 'payment_status' => 'cancelled', |
| 432 | - 'updated_at' => current_time('mysql') | |
| 543 | + 'updated_at' => $now | |
| 433 | 544 | ]); |
| 434 | 545 | } |
| 435 | - } | |
| 436 | - | |
| 437 | - $response = (new PaymentManagement())->cancelSubscription($subscription); | |
| 438 | - | |
| 439 | - if(is_wp_error($response)) { | |
| 440 | - wp_send_json_error([ | |
| 441 | - 'message' => $response->get_error_code().' - '.$response->get_error_message() | |
| 442 | - ], 423); | |
| 443 | - } | |
| 546 | + }); | |
| 444 | 547 | |
| 445 | 548 | wp_send_json_success([ |
| 446 | 549 | 'message' => $response |
| 447 | 550 | ]); |
