Skip to content

Commit 4d1c156

Browse files
committed
fix(mgr-api): correct ACL list total and bulk 403 responses
Over-fetch visible rows for paginated grids, count ACL-visible total, batch-load products for view filter, and return 403 when bulk actions are denied solely by document policy.
1 parent a128cfa commit 4d1c156

2 files changed

Lines changed: 196 additions & 11 deletions

File tree

core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php

Lines changed: 167 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public function getList(array $params = []): array
6363
return Response::error('Category products list service is not available', 500)->getData();
6464
}
6565

66-
$page = $listService->getPage(
66+
$results = $this->collectVisibleListPage(
67+
$listService,
6768
$categoryId,
6869
$params,
6970
$nested,
@@ -74,11 +75,19 @@ public function getList(array $params = []): array
7475
$sortDir
7576
);
7677

77-
$results = $this->filterListResultsByDocumentView($page['results'], $nested);
78+
$total = $this->countVisibleListResults(
79+
$listService,
80+
$categoryId,
81+
$params,
82+
$nested,
83+
$gridFields,
84+
(string) $sortBy,
85+
$sortDir
86+
);
7887

7988
return Response::success([
8089
'results' => $results,
81-
'total' => $page['total'],
90+
'total' => $total,
8291
])->getData();
8392
}
8493

@@ -235,6 +244,7 @@ public function multiple(array $params = []): array
235244

236245
$success = 0;
237246
$failed = 0;
247+
$policyDenied = 0;
238248
$scope = $this->scopeService();
239249

240250
foreach ($ids as $id) {
@@ -250,7 +260,7 @@ public function multiple(array $params = []): array
250260
&& !CategoryProductDocumentPolicy::isAllowedAll($product, $documentPolicies)
251261
) {
252262
$this->logDocumentPolicyDenied($product, $documentPolicies);
253-
$failed++;
263+
$policyDenied++;
254264
continue;
255265
}
256266

@@ -274,6 +284,13 @@ public function multiple(array $params = []): array
274284
}
275285

276286
if ($success === 0) {
287+
if ($policyDenied > 0) {
288+
return Response::error(
289+
'Save permission denied for this document',
290+
HttpStatus::FORBIDDEN
291+
)->getData();
292+
}
293+
277294
return Response::error('No products were updated', HttpStatus::INTERNAL_SERVER_ERROR)->getData();
278295
}
279296

@@ -407,27 +424,166 @@ private function requireCategoryWithView(int $categoryId): object|array
407424
*/
408425
private function filterListResultsByDocumentView(array $results, bool $nested): array
409426
{
410-
$filtered = [];
427+
if ($results === []) {
428+
return [];
429+
}
411430

412-
foreach ($results as $row) {
413-
$productId = (int) ($row['id'] ?? 0);
414-
if ($productId <= 0) {
415-
continue;
431+
$productIds = array_values(array_filter(array_map(
432+
static fn (array $row): int => (int) ($row['id'] ?? 0),
433+
$results
434+
)));
435+
436+
if ($productIds === []) {
437+
return [];
438+
}
439+
440+
/** @var array<int, msProduct> $productsById */
441+
$productsById = [];
442+
$collection = $this->modx->getCollection(msProduct::class, ['id:IN' => $productIds]);
443+
foreach ($collection as $product) {
444+
if ($product instanceof msProduct) {
445+
$productsById[(int) $product->get('id')] = $product;
446+
}
447+
}
448+
449+
/** @var array<int, msCategory> $parentsById */
450+
$parentsById = [];
451+
if ($nested) {
452+
$parentIds = array_values(array_unique(array_filter(array_map(
453+
static fn (array $row): int => (int) ($row['parent'] ?? 0),
454+
$results
455+
))));
456+
if ($parentIds !== []) {
457+
$parentCollection = $this->modx->getCollection(msCategory::class, ['id:IN' => $parentIds]);
458+
foreach ($parentCollection as $parent) {
459+
if ($parent instanceof msCategory) {
460+
$parentsById[(int) $parent->get('id')] = $parent;
461+
}
462+
}
416463
}
464+
}
417465

418-
$product = $this->modx->getObject(msProduct::class, $productId);
466+
$filtered = [];
467+
foreach ($results as $row) {
468+
$productId = (int) ($row['id'] ?? 0);
469+
$product = $productsById[$productId] ?? null;
419470
if (!$product instanceof msProduct) {
420471
continue;
421472
}
422473

423-
if (CategoryProductDocumentPolicy::canViewInCategoryGrid($this->modx, $product, $nested)) {
474+
if (CategoryProductDocumentPolicy::canViewInCategoryGridCached($product, $nested, $parentsById)) {
424475
$filtered[] = $row;
425476
}
426477
}
427478

428479
return $filtered;
429480
}
430481

482+
/**
483+
* @param array<int, array<string, mixed>> $gridFields
484+
* @return list<array<string, mixed>>
485+
*/
486+
private function collectVisibleListPage(
487+
CategoryProductsListService $listService,
488+
int $categoryId,
489+
array $params,
490+
bool $nested,
491+
array $gridFields,
492+
int $start,
493+
int $limit,
494+
string $sortBy,
495+
string $sortDir,
496+
): array {
497+
if ($limit <= 0) {
498+
return [];
499+
}
500+
501+
$visible = [];
502+
$scanOffset = 0;
503+
$skipped = 0;
504+
$batchSize = max($limit * 2, 20);
505+
506+
while (count($visible) < $limit) {
507+
$page = $listService->getPage(
508+
$categoryId,
509+
$params,
510+
$nested,
511+
$gridFields,
512+
$scanOffset,
513+
$batchSize,
514+
$sortBy,
515+
$sortDir
516+
);
517+
518+
if ($page['results'] === []) {
519+
break;
520+
}
521+
522+
$filtered = $this->filterListResultsByDocumentView($page['results'], $nested);
523+
foreach ($filtered as $row) {
524+
if ($skipped < $start) {
525+
$skipped++;
526+
continue;
527+
}
528+
529+
$visible[] = $row;
530+
if (count($visible) >= $limit) {
531+
break 2;
532+
}
533+
}
534+
535+
$scanOffset += count($page['results']);
536+
if (count($page['results']) < $batchSize) {
537+
break;
538+
}
539+
}
540+
541+
return $visible;
542+
}
543+
544+
/**
545+
* @param array<int, array<string, mixed>> $gridFields
546+
*/
547+
private function countVisibleListResults(
548+
CategoryProductsListService $listService,
549+
int $categoryId,
550+
array $params,
551+
bool $nested,
552+
array $gridFields,
553+
string $sortBy,
554+
string $sortDir,
555+
): int {
556+
$visible = 0;
557+
$scanOffset = 0;
558+
$batchSize = 200;
559+
560+
while (true) {
561+
$page = $listService->getPage(
562+
$categoryId,
563+
$params,
564+
$nested,
565+
$gridFields,
566+
$scanOffset,
567+
$batchSize,
568+
$sortBy,
569+
$sortDir
570+
);
571+
572+
if ($page['results'] === []) {
573+
break;
574+
}
575+
576+
$visible += count($this->filterListResultsByDocumentView($page['results'], $nested));
577+
$scanOffset += count($page['results']);
578+
579+
if (count($page['results']) < $batchSize) {
580+
break;
581+
}
582+
}
583+
584+
return $visible;
585+
}
586+
431587
/** @param list<string> $policies */
432588
private function logDocumentPolicyDenied(msProduct $product, array $policies): void
433589
{

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,35 @@ public static function canViewInCategoryGrid(modX $modx, msProduct $product, boo
117117
return self::isAllowed($parent, self::POLICY_VIEW);
118118
}
119119

120+
/**
121+
* @param array<int, msCategory> $parentsById
122+
*/
123+
public static function canViewInCategoryGridCached(
124+
msProduct $product,
125+
bool $nested,
126+
array $parentsById,
127+
): bool {
128+
if (!self::isAllowed($product, self::POLICY_VIEW)) {
129+
return false;
130+
}
131+
132+
if (!$nested) {
133+
return true;
134+
}
135+
136+
$parentId = (int) $product->get('parent');
137+
if ($parentId <= 0) {
138+
return false;
139+
}
140+
141+
$parent = $parentsById[$parentId] ?? null;
142+
if (!$parent instanceof msCategory) {
143+
return false;
144+
}
145+
146+
return self::isAllowed($parent, self::POLICY_VIEW);
147+
}
148+
120149
public static function toErrorResponse(?array $evaluation): ?array
121150
{
122151
if ($evaluation === null) {

0 commit comments

Comments
 (0)