From 4f48e61a2cbc6db5338872b6bdb45ef954ec1256 Mon Sep 17 00:00:00 2001 From: xboard <147830212+cedar2025@users.noreply.github.com> Date: Sun, 30 Aug 2026 04:50:07 +0800 Subject: [PATCH] fix(order): prevent duplicate refunds and redemptions Refs #1021 --- .../Controllers/V1/User/OrderController.php | 3 + app/Jobs/OrderHandleJob.php | 4 +- app/Services/GiftCardService.php | 27 ++- app/Services/OrderService.php | 123 ++++++++--- resources/lang/en-US.json | 1 + resources/lang/ru-RU.json | 1 + resources/lang/zh-CN.json | 1 + resources/lang/zh-TW.json | 1 + tests/Feature/Server/ServerHandshakeTest.php | 3 +- tests/TestCase.php | 9 + .../Services/OrderServiceConcurrencyTest.php | 193 ++++++++++++++++++ 11 files changed, 328 insertions(+), 38 deletions(-) create mode 100644 tests/TestCase.php create mode 100644 tests/Unit/Services/OrderServiceConcurrencyTest.php diff --git a/app/Http/Controllers/V1/User/OrderController.php b/app/Http/Controllers/V1/User/OrderController.php index 7b28128..779b715 100755 --- a/app/Http/Controllers/V1/User/OrderController.php +++ b/app/Http/Controllers/V1/User/OrderController.php @@ -126,6 +126,9 @@ class OrderController extends Controller if (!$order) { return $this->fail([400, __('Order does not exist or has been paid')]); } + if ($order->total_amount < 0) { + return $this->fail([400, __('Order amount is invalid')]); + } // free process if ($order->total_amount <= 0) { $orderService = new OrderService($order); diff --git a/app/Jobs/OrderHandleJob.php b/app/Jobs/OrderHandleJob.php index 960f659..1b98401 100644 --- a/app/Jobs/OrderHandleJob.php +++ b/app/Jobs/OrderHandleJob.php @@ -36,9 +36,7 @@ class OrderHandleJob implements ShouldQueue */ public function handle() { - $order = Order::where('trade_no', $this->tradeNo) - ->lockForUpdate() - ->first(); + $order = Order::where('trade_no', $this->tradeNo)->first(); if (!$order) return; $orderService = new OrderService($order); switch ($order->status) { diff --git a/app/Services/GiftCardService.php b/app/Services/GiftCardService.php index 8f14cbe..889bd43 100644 --- a/app/Services/GiftCardService.php +++ b/app/Services/GiftCardService.php @@ -15,8 +15,8 @@ use Illuminate\Support\Facades\Log; class GiftCardService { - protected readonly GiftCardCode $code; - protected readonly GiftCardTemplate $template; + protected GiftCardCode $code; + protected GiftCardTemplate $template; protected ?User $user = null; public function __construct(string $code) @@ -106,6 +106,23 @@ class GiftCardService } return DB::transaction(function () use ($options) { + $this->code = GiftCardCode::whereKey($this->code->id) + ->lockForUpdate() + ->first() + ?? throw new ApiException('兑换码不存在'); + + $this->template = GiftCardTemplate::whereKey($this->code->template_id) + ->lockForUpdate() + ->first() + ?? throw new ApiException('该礼品卡类型不存在'); + + $this->user = User::whereKey($this->user->id) + ->lockForUpdate() + ->first() + ?? throw new ApiException('用户信息未提供'); + + $this->validate(); + $actualRewards = $this->template->calculateActualRewards($this->user); if ($this->template->type === GiftCardTemplate::TYPE_MYSTERY) { @@ -198,7 +215,7 @@ class GiftCardService return null; } - $inviteUser = User::find($this->user->invite_user_id); + $inviteUser = User::lockForUpdate()->find($this->user->invite_user_id); if (!$inviteUser) { return null; } @@ -212,7 +229,9 @@ class GiftCardService if (isset($rewards['balance']) && $rewards['balance'] > 0) { $inviteBalance = intval($rewards['balance'] * $rate); if ($inviteBalance > 0) { - $userService->addBalance($inviteUser->id, $inviteBalance); + if (!$userService->addBalance($inviteUser->id, $inviteBalance)) { + throw new ApiException('邀请人余额发放失败'); + } $inviteRewards['balance'] = $inviteBalance; } } diff --git a/app/Services/OrderService.php b/app/Services/OrderService.php index 6651eaa..a7fbd55 100644 --- a/app/Services/OrderService.php +++ b/app/Services/OrderService.php @@ -33,6 +33,21 @@ class OrderService $this->order = $order; } + private function lockCurrentOrder(): ?Order + { + return Order::whereKey($this->order->id) + ->lockForUpdate() + ->first(); + } + + private function lockCurrentOrderWhenStatus(int $status): ?Order + { + return Order::whereKey($this->order->id) + ->where('status', $status) + ->lockForUpdate() + ->first(); + } + /** * Create an order from a request. * @@ -56,6 +71,15 @@ class OrderService HookManager::call('order.create.before', [$user, $plan, $period, $couponCode]); return DB::transaction(function () use ($user, $plan, $period, $couponCode, $userService) { + $user = User::lockForUpdate()->find($user->id); + if (!$user) { + throw new ApiException(__('The user does not exist')); + } + + if ($userService->isNotCompleteOrderByUserId($user->id)) { + throw new ApiException(__('You have an unpaid or pending order, please try again later or cancel it')); + } + $newPeriod = PlanService::getPeriodKey($period); $order = new Order([ @@ -79,7 +103,6 @@ class OrderService $orderService->handleUserBalance($user, $userService); } - // 余额抵扣后的 total_amount 才是用户实际支付金额，返利必须基于该金额计算。 $orderService->setInvite(user: $user); if (!$order->save()) { @@ -96,13 +119,19 @@ class OrderService public function open(): void { - $order = $this->order; - $plan = Plan::find($order->plan_id); + $openedOrder = DB::transaction(function () { + $order = $this->lockCurrentOrderWhenStatus(Order::STATUS_PROCESSING); + if (!$order) { + return null; + } - HookManager::call('order.open.before', $order); + $plan = Plan::find($order->plan_id); + if (!$plan) { + throw new \RuntimeException('订阅不存在'); + } + HookManager::call('order.open.before', $order); - DB::transaction(function () use ($order, $plan) { $this->user = User::lockForUpdate()->find($order->user_id); if ($order->surplus_credit) { @@ -131,10 +160,19 @@ class OrderService if (!$order->save()) { throw new \RuntimeException('订单信息保存失败'); } + + return $order; }); + if (!$openedOrder) { + return; + } + + $order = $openedOrder; + $this->order = $order; + $eventId = match ((int) $order->type) { - Order::STATUS_PROCESSING => admin_setting('new_order_event_id', 0), + Order::TYPE_NEW_PURCHASE => admin_setting('new_order_event_id', 0), Order::TYPE_RENEWAL => admin_setting('renew_order_event_id', 0), Order::TYPE_UPGRADE => admin_setting('change_order_event_id', 0), default => 0, @@ -287,16 +325,31 @@ class OrderService public function paid(string $callbackNo) { - $order = $this->order; - if ($order->status !== Order::STATUS_PENDING) - return true; - $order->status = Order::STATUS_PROCESSING; - $order->paid_at = time(); - $order->callback_no = $callbackNo; - if (!$order->save()) - return false; try { - OrderHandleJob::dispatchSync($order->trade_no); + [$order, $shouldDispatch] = DB::transaction(function () use ($callbackNo) { + $order = $this->lockCurrentOrder(); + if (!$order) { + throw new \RuntimeException('Order not found.'); + } + if ((int) $order->status !== Order::STATUS_PENDING) { + return [$order, false]; + } + + $order->status = Order::STATUS_PROCESSING; + $order->paid_at = time(); + $order->callback_no = $callbackNo; + if (!$order->save()) { + throw new \RuntimeException('Failed to save order status.'); + } + + return [$order, true]; + }); + + $this->order = $order; + + if ($shouldDispatch) { + OrderHandleJob::dispatchSync($order->trade_no); + } } catch (\Exception $e) { Log::error($e); return false; @@ -306,25 +359,37 @@ class OrderService public function cancel(): bool { - $order = $this->order; - HookManager::call('order.cancel.before', $order); try { - DB::beginTransaction(); - $order->status = Order::STATUS_CANCELLED; - if (!$order->save()) { - throw new \Exception('Failed to save order status.'); - } - if ($order->balance_amount) { - $userService = new UserService(); - if (!$userService->addBalance($order->user_id, $order->balance_amount)) { - throw new \Exception('Failed to add balance.'); + $cancelledOrder = DB::transaction(function () { + $order = $this->lockCurrentOrderWhenStatus(Order::STATUS_PENDING); + if (!$order) { + return null; } + + HookManager::call('order.cancel.before', $order); + + $order->status = Order::STATUS_CANCELLED; + if (!$order->save()) { + throw new \RuntimeException('Failed to save order status.'); + } + if ($order->balance_amount) { + $userService = new UserService(); + if (!$userService->addBalance($order->user_id, $order->balance_amount)) { + throw new \RuntimeException('Failed to add balance.'); + } + } + + return $order; + }); + + if (!$cancelledOrder) { + return false; } - DB::commit(); - HookManager::call('order.cancel.after', $order); + + $this->order = $cancelledOrder; + HookManager::call('order.cancel.after', $cancelledOrder); return true; } catch (\Exception $e) { - DB::rollBack(); Log::error($e); return false; } diff --git a/resources/lang/en-US.json b/resources/lang/en-US.json index 26d935d..2e8a33d 100644 --- a/resources/lang/en-US.json +++ b/resources/lang/en-US.json @@ -26,6 +26,7 @@ "No active subscription. Unable to use our provided Apple ID": "No active subscription. Unable to use our provided Apple ID", "Oops, there's a problem... Please refresh the page and try again later": "Oops, there's a problem... Please refresh the page and try again later", "Order does not exist": "Order does not exist", + "Order amount is invalid": "Order amount is invalid", "Order does not exist or has been paid": "Order does not exist or has been paid", "Payment failed. Please check your credit card information": "Payment failed. Please check your credit card information", "Payment gateway request failed": "Payment gateway request failed", diff --git a/resources/lang/ru-RU.json b/resources/lang/ru-RU.json index 9ad67a5..891ee5a 100644 --- a/resources/lang/ru-RU.json +++ b/resources/lang/ru-RU.json @@ -26,6 +26,7 @@ "No active subscription. Unable to use our provided Apple ID": "Нет активной подписки. Невозможно использовать предоставленный Apple ID", "Oops, there's a problem... Please refresh the page and try again later": "Ой, возникла проблема... Обновите страницу и попробуйте позже", "Order does not exist": "Заказ не существует", + "Order amount is invalid": "Некорректная сумма заказа", "Order does not exist or has been paid": "Заказ не существует или уже оплачен", "Payment failed. Please check your credit card information": "Ошибка оплаты. Проверьте данные карты", "Payment gateway request failed": "Ошибка запроса к платёжному шлюзу", diff --git a/resources/lang/zh-CN.json b/resources/lang/zh-CN.json index 0826fb1..9dff5af 100644 --- a/resources/lang/zh-CN.json +++ b/resources/lang/zh-CN.json @@ -22,6 +22,7 @@ "Withdrawal account": "提现账号", "Unsupported withdrawal": "不支持提现", "Order does not exist": "订单不存在", + "Order amount is invalid": "订单金额异常", "You have an unpaid or pending order, please try again later or cancel it": "您有未付款或开通中的订单，请稍后再试或将其取消", "This subscription has been sold out, please choose another subscription": "该订阅已售罄，请更换其它订阅", "This subscription cannot be renewed, please change to another subscription": "该订阅无法续费，请更换其它订阅", diff --git a/resources/lang/zh-TW.json b/resources/lang/zh-TW.json index c3c20b2..6046372 100644 --- a/resources/lang/zh-TW.json +++ b/resources/lang/zh-TW.json @@ -22,6 +22,7 @@ "Withdrawal account": "提現賬號", "Unsupported withdrawal": "不支持提現", "Order does not exist": "訂單不存在", + "Order amount is invalid": "訂單金額異常", "You have an unpaid or pending order, please try again later or cancel it": "您有未付款或開通中的訂單，請稍後再試或將其取消", "This subscription has been sold out, please choose another subscription": "該訂閱已售罄，請更換其它訂閱", "This subscription cannot be renewed, please change to another subscription": "該訂閱無法續費，請更換其它訂閱", diff --git a/tests/Feature/Server/ServerHandshakeTest.php b/tests/Feature/Server/ServerHandshakeTest.php index e2c3b32..0690f3b 100644 --- a/tests/Feature/Server/ServerHandshakeTest.php +++ b/tests/Feature/Server/ServerHandshakeTest.php @@ -6,7 +6,6 @@ use App\Models\Server; use App\Models\ServerMachine; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Bus; -use Illuminate\Support\Facades\Cache; use Tests\TestCase; class ServerHandshakeTest extends TestCase @@ -18,7 +17,7 @@ class ServerHandshakeTest extends TestCase parent::setUp(); config()->set('app.key', 'base64:' . base64_encode(str_repeat('a', 32))); - Cache::forever('admin_settings', [ + admin_setting([ 'server_token' => 'server-token', 'server_ws_enable' => 0, ]); diff --git a/tests/TestCase.php b/tests/TestCase.php new file mode 100644 index 0000000..539c7dd --- /dev/null +++ b/tests/TestCase.php @@ -0,0 +1,9 @@ +makeUser(['balance' => 0]); + $plan = $this->makePlan(); + $order = $this->makeOrder($user, $plan, [ + 'status' => Order::STATUS_PENDING, + 'balance_amount' => 100, + 'total_amount' => 1000, + ]); + + $firstStaleOrder = Order::findOrFail($order->id); + $secondStaleOrder = Order::findOrFail($order->id); + + $this->assertTrue((new OrderService($firstStaleOrder))->cancel()); + $this->assertFalse((new OrderService($secondStaleOrder))->cancel()); + $this->assertSame(100, User::findOrFail($user->id)->balance); + } + + public function test_open_only_applies_subscription_once_when_called_with_stale_order_models(): void + { + $user = $this->makeUser([ + 'balance' => 0, + 'expired_at' => 0, + 'transfer_enable' => 0, + 'u' => 1073741824, + 'd' => 0, + ]); + $plan = $this->makePlan(); + $order = $this->makeOrder($user, $plan, [ + 'status' => Order::STATUS_PROCESSING, + 'balance_amount' => 1100, + 'total_amount' => 0, + ]); + + $before = time(); + $firstStaleOrder = Order::findOrFail($order->id); + $secondStaleOrder = Order::findOrFail($order->id); + + (new OrderService($firstStaleOrder))->open(); + (new OrderService($secondStaleOrder))->open(); + + $user->refresh(); + $this->assertSame(1, $user->reset_count); + $this->assertSame(Order::STATUS_COMPLETED, Order::findOrFail($order->id)->status); + $this->assertLessThan($before + (45 * 86400), $user->expired_at); + } + + public function test_gift_card_redeem_only_grants_rewards_once_for_stale_code_models(): void + { + $user = $this->makeUser(['balance' => 0]); + $template = GiftCardTemplate::create([ + 'name' => 'race-test', + 'description' => 'race-test', + 'type' => GiftCardTemplate::TYPE_GENERAL, + 'status' => 1, + 'rewards' => ['balance' => 100], + 'admin_id' => 1, + 'created_at' => time(), + 'updated_at' => time(), + ]); + GiftCardCode::create([ + 'template_id' => $template->id, + 'code' => 'RACEUNITTEST', + 'status' => GiftCardCode::STATUS_UNUSED, + 'usage_count' => 0, + 'max_usage' => 1, + 'created_at' => time(), + 'updated_at' => time(), + ]); + + $firstService = (new GiftCardService('RACEUNITTEST'))->setUser(User::findOrFail($user->id)); + $secondService = (new GiftCardService('RACEUNITTEST'))->setUser(User::findOrFail($user->id)); + + $firstService->validate(); + $secondService->validate(); + $firstService->redeem(); + + try { + $secondService->redeem(); + $this->fail('The second stale gift card redeem should fail.'); + } catch (ApiException $exception) { + $this->assertNotSame('', $exception->getMessage()); + } + + $user->refresh(); + $code = GiftCardCode::where('code', 'RACEUNITTEST')->firstOrFail(); + $this->assertSame(100, $user->balance); + $this->assertSame(1, $code->usage_count); + $this->assertSame(1, $code->usages()->count()); + } + + public function test_checkout_rejects_negative_order_amount(): void + { + $user = $this->makeUser(['balance' => 0]); + Sanctum::actingAs($user); + + $order = $this->makeOrder($user, $this->makePlan(), [ + 'status' => Order::STATUS_PENDING, + 'total_amount' => -1, + ]); + + $response = $this->postJson('/api/v1/user/order/checkout', [ + 'trade_no' => $order->trade_no, + ]); + + $response->assertStatus(400); + $this->assertSame(Order::STATUS_PENDING, Order::findOrFail($order->id)->status); + $this->assertNull(User::findOrFail($user->id)->plan_id); + } + + private function makeUser(array $overrides = []): User + { + return User::create(array_merge([ + 'email' => 'race-test@example.com', + 'password' => 'password', + 'uuid' => '00000000-0000-0000-0000-000000000001', + 'token' => '0123456789abcdef0123456789abcdef', + 'balance' => 0, + 'commission_balance' => 0, + 'transfer_enable' => 0, + 'u' => 0, + 'd' => 0, + 'banned' => 0, + 'is_admin' => 0, + 'is_staff' => 0, + 'expired_at' => 0, + 'remind_expire' => 1, + 'remind_traffic' => 1, + 'created_at' => time(), + 'updated_at' => time(), + ], $overrides)); + } + + private function makePlan(array $overrides = []): Plan + { + return Plan::create(array_merge([ + 'group_id' => null, + 'transfer_enable' => 1111, + 'name' => 'Race Test Plan', + 'speed_limit' => null, + 'show' => 1, + 'sort' => 0, + 'renew' => 1, + 'prices' => [ + Plan::PERIOD_MONTHLY => 11, + ], + 'reset_traffic_method' => Plan::RESET_TRAFFIC_MONTHLY, + 'capacity_limit' => null, + 'sell' => 1, + 'device_limit' => null, + 'created_at' => time(), + 'updated_at' => time(), + ], $overrides)); + } + + private function makeOrder(User $user, Plan $plan, array $overrides = []): Order + { + return Order::create(array_merge([ + 'user_id' => $user->id, + 'plan_id' => $plan->id, + 'type' => Order::TYPE_NEW_PURCHASE, + 'period' => Plan::PERIOD_MONTHLY, + 'trade_no' => uniqid('race_', true), + 'total_amount' => 0, + 'balance_amount' => 0, + 'status' => Order::STATUS_PENDING, + 'commission_status' => 0, + 'commission_balance' => 0, + 'created_at' => time(), + 'updated_at' => time(), + ], $overrides)); + } +}