Skip to content

Commit 8c12e88

Browse files
authored
fix(mgr): scope category product inline-edit PUT to the category (IDOR) (#454)
Inline-edit in the Category Products grid sent PUT /api/mgr/product-data/{id} with no category binding, so a manager could edit another category's product by id (IDOR, complements #443 which scoped the mutations). - When category_id is in the body, the backend verifies msProduct.parent is in the same scope as the grid list filter (parent = categoryId, or nested descendants) via CategoryProductsListService::isProductInCategoryScope -> pure CategoryProductScopePolicy + CategoryTreeService (reused from #443). Out of scope -> 403. Fail-closed (missing product/service -> reject). - category_id / nested are stripped before updateProductData(). Without category_id the product-card (ProductDataFields) path is unchanged, gated by the route permission. - Vue inline-edit now always sends category_id + nested. - Adds CategoryProductScopePolicyTest and the ms3_err_product_not_in_category_scope lexicon (en/ru). Closes #444.
1 parent 027c364 commit 8c12e88

8 files changed

Lines changed: 161 additions & 11 deletions

File tree

core/components/minishop3/lexicon/en/default.inc.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@
176176
$_lang['ms3_err_order_load'] = 'Error loading order.';
177177
$_lang['ms3_err_order_num_lock'] = 'Could not acquire a lock to generate the order number. Please try again.';
178178
$_lang['ms3_err_order_num_save'] = 'Could not save the order number. Please try again.';
179+
$_lang['ms3_err_product_not_in_category_scope'] = 'Product is not in the scope of this category.';
179180
$_lang['ms3_err_status_nf'] = 'Status with this identifier not found.';
180181
$_lang['ms3_err_delivery_nf'] = 'Delivery method with this identifier not found.';
181182
$_lang['ms3_err_payment_nf'] = 'Payment method with this identifier not found.';

core/components/minishop3/lexicon/ru/default.inc.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@
176176
$_lang['ms3_err_order_load'] = 'Ошибка при загрузке заказа.';
177177
$_lang['ms3_err_order_num_lock'] = 'Не удалось получить блокировку для генерации номера заказа. Попробуйте ещё раз.';
178178
$_lang['ms3_err_order_num_save'] = 'Не удалось сохранить номер заказа. Попробуйте ещё раз.';
179+
$_lang['ms3_err_product_not_in_category_scope'] = 'Товар не входит в область этой категории.';
179180
$_lang['ms3_err_status_nf'] = 'Статус с таким идентификатором не найден.';
180181
$_lang['ms3_err_delivery_nf'] = 'Способ доставки с таким идентификатором не найден.';
181182
$_lang['ms3_err_payment_nf'] = 'Способ оплаты с таким идентификатором не найден.';

core/components/minishop3/src/Controllers/Api/ProductDataController.php

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,35 @@ public function update(array $params): Response
5757
return Response::error('Product ID is required', HttpStatus::BAD_REQUEST);
5858
}
5959

60-
$data = $this->getRequestData();
60+
$requestData = $this->getRequestData();
6161

62-
if (!$data) {
62+
if (!$requestData) {
63+
return Response::error('Invalid request data', HttpStatus::BAD_REQUEST);
64+
}
65+
66+
$categoryId = (int) ($requestData['category_id'] ?? 0);
67+
$nested = filter_var($requestData['nested'] ?? false, FILTER_VALIDATE_BOOLEAN);
68+
69+
$data = $requestData;
70+
unset($data['category_id'], $data['nested']);
71+
72+
if ($categoryId > 0) {
73+
/** @var \MiniShop3\Services\Category\CategoryProductsListService|null $listService */
74+
$listService = $this->modx->services->get('ms3_category_products_list');
75+
if (
76+
!$listService
77+
|| !$listService->isProductInCategoryScope($productId, $categoryId, $nested)
78+
) {
79+
$this->modx->lexicon->load('minishop3:default');
80+
81+
return Response::error(
82+
$this->modx->lexicon('ms3_err_product_not_in_category_scope'),
83+
HttpStatus::FORBIDDEN
84+
);
85+
}
86+
}
87+
88+
if ($data === []) {
6389
return Response::error('Invalid request data', HttpStatus::BAD_REQUEST);
6490
}
6591

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace MiniShop3\Services\Category;
6+
7+
/**
8+
* Pure scope rules for category products grid (same as CategoryProductsListService list filter).
9+
*/
10+
final class CategoryProductScopePolicy
11+
{
12+
/**
13+
* @param list<int> $descendantCategoryIds Child category IDs (recursive, excluding root)
14+
*/
15+
public static function isParentInScope(
16+
int $productParentId,
17+
int $categoryId,
18+
bool $nested,
19+
array $descendantCategoryIds
20+
): bool {
21+
if ($productParentId <= 0 || $categoryId <= 0) {
22+
return false;
23+
}
24+
25+
if (!$nested) {
26+
return $productParentId === $categoryId;
27+
}
28+
29+
$allowed = $descendantCategoryIds;
30+
$allowed[] = $categoryId;
31+
32+
return in_array($productParentId, $allowed, true);
33+
}
34+
}

core/components/minishop3/src/Services/Category/CategoryProductsListService.php

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,7 @@ private function buildProductListQuery(int $categoryId, array $params, bool $nes
144144
}
145145

146146
$c->where(['msProduct.class_key' => msProduct::class]);
147-
148-
if ($nested) {
149-
$categoryIds = $this->treeService()->productParentIds($categoryId, true);
150-
$c->where(['msProduct.parent:IN' => $categoryIds]);
151-
} else {
152-
$c->where(['msProduct.parent' => $categoryId]);
153-
}
147+
$c->where(['msProduct.parent:IN' => $this->getAllowedProductParentCategoryIds($categoryId, $nested)]);
154148

155149
if ($query !== '') {
156150
$c->where([
@@ -225,6 +219,41 @@ private function quoteOptionKeyForJoinCondition(string $key): string
225219
return str_replace("'", "''", $key);
226220
}
227221

222+
/**
223+
* Parent category IDs allowed for products in category grid scope (matches list filter).
224+
*
225+
* @return list<int>
226+
*/
227+
public function getAllowedProductParentCategoryIds(int $categoryId, bool $nested): array
228+
{
229+
if (!$nested) {
230+
return [$categoryId];
231+
}
232+
233+
$ids = $this->treeService()->getDescendantCategoryIds($categoryId);
234+
$ids[] = $categoryId;
235+
236+
return $ids;
237+
}
238+
239+
/**
240+
* Whether a product belongs to the category products grid scope (direct parent or nested tree).
241+
*/
242+
public function isProductInCategoryScope(int $productId, int $categoryId, bool $nested): bool
243+
{
244+
$product = $this->modx->getObject(msProduct::class, $productId);
245+
if (!$product) {
246+
return false;
247+
}
248+
249+
return CategoryProductScopePolicy::isParentInScope(
250+
(int) $product->get('parent'),
251+
$categoryId,
252+
$nested,
253+
$nested ? $this->treeService()->getDescendantCategoryIds($categoryId) : []
254+
);
255+
}
256+
228257
/**
229258
* @param list<string> $optionFieldNames Allowed option field names (whitelist)
230259
*
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
/**
4+
* Regression tests for category product scope policy (issue #444).
5+
*
6+
* Run: php tests/CategoryProductScopePolicyTest.php
7+
*/
8+
9+
declare(strict_types=1);
10+
11+
require __DIR__ . '/../vendor/autoload.php';
12+
13+
use MiniShop3\Services\Category\CategoryProductScopePolicy;
14+
15+
$fail = static function (string $message): never {
16+
fwrite(STDERR, "FAIL: {$message}\n");
17+
exit(1);
18+
};
19+
20+
$assertSame = static function ($expected, $actual, string $case) use ($fail): void {
21+
if ($actual !== $expected) {
22+
$fail($case . ': expected ' . var_export($expected, true) . ', got ' . var_export($actual, true));
23+
}
24+
};
25+
26+
$assertSame(true, CategoryProductScopePolicy::isParentInScope(10, 10, false, []), 'direct parent match');
27+
$assertSame(false, CategoryProductScopePolicy::isParentInScope(11, 10, false, []), 'direct parent mismatch');
28+
$assertSame(
29+
true,
30+
CategoryProductScopePolicy::isParentInScope(12, 10, true, [11, 12]),
31+
'nested child category'
32+
);
33+
$assertSame(
34+
true,
35+
CategoryProductScopePolicy::isParentInScope(10, 10, true, [11, 12]),
36+
'nested root category'
37+
);
38+
$assertSame(
39+
false,
40+
CategoryProductScopePolicy::isParentInScope(99, 10, true, [11, 12]),
41+
'nested foreign parent'
42+
);
43+
$assertSame(false, CategoryProductScopePolicy::isParentInScope(0, 10, false, []), 'invalid product parent');
44+
$assertSame(false, CategoryProductScopePolicy::isParentInScope(10, 0, false, []), 'invalid category id');
45+
46+
fwrite(STDOUT, "OK CategoryProductScopePolicyTest\n");
47+
exit(0);

vueManager/src/components/CategoryProductsGrid.vue

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ const {
8989
} = useCategoryProductsInlineEdit({
9090
products,
9191
referencePathsByKey,
92+
categoryId: computed(() => props.categoryId),
93+
nested,
9294
request,
9395
toast,
9496
_,

vueManager/src/composables/useCategoryProductsInlineEdit.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,19 @@ import {
1717
* @param {Object} deps
1818
* @param {import('vue').Ref} deps.products
1919
* @param {import('vue').Ref<Record<string, string>>} deps.referencePathsByKey Paths from GET grid-config (editor_references)
20+
* @param {import('vue').Ref<number>|number} deps.categoryId Category context for scoped inline edits
21+
* @param {import('vue').Ref<boolean>|boolean} deps.nested Include nested subcategories in scope
2022
* @param {Function} deps.request HTTP client (e.g. project request)
2123
* @param {{ add: Function }} deps.toast PrimeVue toast
2224
* @param {Function} deps._ Lexicon helper
2325
*/
2426
export function useCategoryProductsInlineEdit(deps) {
25-
const { products, referencePathsByKey, request, toast, _ } = deps
27+
const { products, referencePathsByKey, categoryId, nested, request, toast, _ } = deps
28+
29+
const resolveScopeContext = () => ({
30+
category_id: typeof categoryId === 'object' && categoryId !== null ? categoryId.value : categoryId,
31+
nested: Boolean(typeof nested === 'object' && nested !== null ? nested.value : nested),
32+
})
2633

2734
const editingCell = ref(null)
2835
const inlineEditValue = ref('')
@@ -204,7 +211,10 @@ export function useCategoryProductsInlineEdit(deps) {
204211
}
205212
inlineEditSaving.value = true
206213
try {
207-
const res = await request.put(`/api/mgr/product-data/${product.id}`, { [column.name]: value })
214+
const res = await request.put(`/api/mgr/product-data/${product.id}`, {
215+
[column.name]: value,
216+
...resolveScopeContext(),
217+
})
208218
const idx = products.value.findIndex(p => p.id === product.id)
209219
if (idx >= 0) {
210220
if (res && typeof res === 'object') {

0 commit comments

Comments
 (0)