diff --git a/CHANGELOG.md b/CHANGELOG.md index 951501a788..52be2ac53f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +### Fixed + +- Order detail price and discount updates are now restricted to items of the current order + ## [2.11.6] 2026-09-11 ### Fixed diff --git a/front/order.form.php b/front/order.form.php index 02c9f5caef..d4a3d549c5 100644 --- a/front/order.form.php +++ b/front/order.form.php @@ -420,6 +420,7 @@ foreach ($data as $item) { $pluginOrderOrder_Item->updatePrice_taxfree([ 'item_id' => $item['id'], + 'orders_id' => (int) $_POST['plugin_order_orders_id'], 'price_taxfree' => $_POST['price_taxfree'], ]); } @@ -440,9 +441,10 @@ ); foreach ($data as $item) { $pluginOrderOrder_Item->updateDiscount([ - 'item_id' => $item['id'], - 'discount' => $_POST['discount'], - 'price' => $price + 'item_id' => $item['id'], + 'orders_id' => (int) $_POST['plugin_order_orders_id'], + 'discount' => $_POST['discount'], + 'price' => $price, ]); } } @@ -451,10 +453,13 @@ Html::back(); } else if (isset($_POST["update_detail_item"])) { $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); + $orders_id = (int) $_POST["plugin_order_orders_id"]; + if (isset($_POST['detail_price_taxfree'])) { foreach ($_POST['detail_price_taxfree'] as $item_id => $price) { $pluginOrderOrder_Item->updatePrice_taxfree([ 'item_id' => $item_id, + 'orders_id' => $orders_id, 'price_taxfree' => $price, ]); } @@ -463,9 +468,10 @@ if (isset($_POST['detail_discount'])) { foreach ($_POST['detail_discount'] as $item_id => $discount) { $pluginOrderOrder_Item->updateDiscount([ - 'item_id' => $item_id, - 'discount' => $discount, - 'price' => isset($_POST['detail_price_taxfree']) ? $_POST['detail_price_taxfree'][$item_id] : $_POST['detail_old_price_taxfree'][$item_id] + 'item_id' => $item_id, + 'orders_id' => $orders_id, + 'discount' => $discount, + 'price' => isset($_POST['detail_price_taxfree']) ? $_POST['detail_price_taxfree'][$item_id] : $_POST['detail_old_price_taxfree'][$item_id], ]); } } diff --git a/inc/order_item.class.php b/inc/order_item.class.php index d145b768dc..0d3d615b15 100644 --- a/inc/order_item.class.php +++ b/inc/order_item.class.php @@ -313,7 +313,7 @@ public function getPricesATI($priceHT, $taxes) } /** - * Check that this item belongs to the given order, to prevent cross-order IDOR on deletion. + * Check that this item belongs to the given order, to prevent cross-order updates and deletions. * * @param int $orders_id Order ID expected to own this item */ @@ -1592,16 +1592,19 @@ public function updatePrices($order_items_id) isset($this->input['price_taxfree']) || isset($this->input['plugin_order_ordertaxes_id']) ) { + $orders_id = (int) $this->fields['plugin_order_orders_id']; + $price_taxfree = $this->fields['price_taxfree']; $iterator = $this->queryRef( - $this->fields['plugin_order_orders_id'], + $orders_id, $this->fields['plugin_order_references_id'], - $this->fields['price_taxfree'], - $this->fields['discount'] + $price_taxfree, + $this->fields['discount'], ); foreach ($iterator as $item) { $this->updatePrice_taxfree([ 'item_id' => $item['id'], - 'price_taxfree' => $this->fields['price_taxfree'] + 'orders_id' => $orders_id, + 'price_taxfree' => $price_taxfree, ]); } } @@ -1907,7 +1910,9 @@ public function updateAnalyticNature($post) public function updatePrice_taxfree($post) { - $this->getFromDB($post['item_id']); + if (!$this->getFromDB($post['item_id']) || !$this->belongsToOrder((int) $post['orders_id'])) { + return; + } $input = $this->fields; $discount = $input['discount']; @@ -1927,7 +1932,9 @@ public function updatePrice_taxfree($post) public function updateDiscount($post) { - $this->getFromDB($post['item_id']); + if (!$this->getFromDB($post['item_id']) || !$this->belongsToOrder((int) $post['orders_id'])) { + return; + } $input = $this->fields; $plugin_order_ordertaxes_id = $input['plugin_order_ordertaxes_id']; diff --git a/tests/Units/OrderItemTest.php b/tests/Units/OrderItemTest.php index 77ecac636e..50fe1ca9b9 100644 --- a/tests/Units/OrderItemTest.php +++ b/tests/Units/OrderItemTest.php @@ -32,8 +32,12 @@ namespace GlpiPlugin\Order\Tests\Units; +use Computer; use DbTestCase; +use Entity; +use PluginOrderOrder; use PluginOrderOrder_Item; +use PluginOrderReference; final class OrderItemTest extends DbTestCase { @@ -52,4 +56,120 @@ public function testBelongsToOrderReturnsFalseForForeignOrder(): void $this->assertFalse($item->belongsToOrder(42)); } + + public function testUpdatePrice_taxfreeIgnoresItemFromAnotherOrder(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + [$order_item, $foreign_orders_id] = $this->createItemInOrderAndForeignOrder(); + + $order_item->updatePrice_taxfree([ + 'item_id' => $order_item->getID(), + 'orders_id' => $foreign_orders_id, + 'price_taxfree' => 999, + ]); + + $this->assertTrue($order_item->getFromDB($order_item->getID())); + $this->assertEquals(100, (float) $order_item->fields['price_taxfree']); + } + + public function testUpdatePrice_taxfreeAppliesToItemOfCurrentOrder(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + [$order_item] = $this->createItemInOrderAndForeignOrder(); + + $order_item->updatePrice_taxfree([ + 'item_id' => $order_item->getID(), + 'orders_id' => (int) $order_item->fields['plugin_order_orders_id'], + 'price_taxfree' => 999, + ]); + + $this->assertTrue($order_item->getFromDB($order_item->getID())); + $this->assertEquals(999, (float) $order_item->fields['price_taxfree']); + } + + public function testUpdateDiscountIgnoresItemFromAnotherOrder(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + [$order_item, $foreign_orders_id] = $this->createItemInOrderAndForeignOrder(); + + $order_item->updateDiscount([ + 'item_id' => $order_item->getID(), + 'orders_id' => $foreign_orders_id, + 'discount' => 50, + 'price' => 100, + ]); + + $this->assertTrue($order_item->getFromDB($order_item->getID())); + $this->assertEquals(0, (float) $order_item->fields['discount']); + } + + public function testUpdateDiscountAppliesToItemOfCurrentOrder(): void + { + global $CFG_GLPI; + $CFG_GLPI["event_loglevel"] = 0; + + $this->login(); + + [$order_item] = $this->createItemInOrderAndForeignOrder(); + + $order_item->updateDiscount([ + 'item_id' => $order_item->getID(), + 'orders_id' => (int) $order_item->fields['plugin_order_orders_id'], + 'discount' => 50, + 'price' => 100, + ]); + + $this->assertTrue($order_item->getFromDB($order_item->getID())); + $this->assertEquals(50, (float) $order_item->fields['discount']); + $this->assertEquals(50, (float) $order_item->fields['price_discounted']); + } + + /** @return array{0: PluginOrderOrder_Item, 1: int} */ + private function createItemInOrderAndForeignOrder(): array + { + $entities_id = getItemByTypeName(Entity::class, '_test_root_entity', true); + + $order = $this->createItem(PluginOrderOrder::class, [ + 'name' => 'Order test owner order', + 'entities_id' => $entities_id, + 'num_order' => mt_rand(), + 'order_date' => date('Y-m-d'), + ]); + + $foreign_order = $this->createItem(PluginOrderOrder::class, [ + 'name' => 'Order test foreign order', + 'entities_id' => $entities_id, + 'num_order' => mt_rand(), + 'order_date' => date('Y-m-d'), + ]); + + $reference = $this->createItem(PluginOrderReference::class, [ + 'name' => 'Order test reference', + 'entities_id' => $entities_id, + 'itemtype' => Computer::class, + ]); + + $order_item = $this->createItem(PluginOrderOrder_Item::class, [ + 'plugin_order_orders_id' => $order->getID(), + 'plugin_order_references_id' => $reference->getID(), + 'itemtype' => Computer::class, + 'items_id' => 0, + 'price_taxfree' => 100, + 'discount' => 0, + ]); + + return [$order_item, (int) $foreign_order->getID()]; + } }