diff --git a/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php b/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php index 9eb91679..1a614c7b 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php @@ -7,6 +7,7 @@ use MiniShop3\Router\HttpStatus; use MiniShop3\Router\Response; use MiniShop3\Services\Category\CategoryProductActionPermissions; +use MiniShop3\Services\Category\CategoryProductDocumentPolicy; use MiniShop3\Services\Category\CategoryProductScopeService; use MiniShop3\Services\Category\CategoryProductsListService; use MiniShop3\Services\FilterConfigManager; @@ -38,14 +39,9 @@ public function __construct(modX $modx) public function getList(array $params = []): array { $categoryId = (int) ($params['id'] ?? 0); - - if (!$categoryId) { - return Response::error('Category ID is required', HttpStatus::BAD_REQUEST)->getData(); - } - - $category = $this->modx->getObject(msCategory::class, $categoryId); - if (!$category) { - return Response::error('Category not found', HttpStatus::NOT_FOUND)->getData(); + $resolved = $this->requireCategoryWithView($categoryId); + if (is_array($resolved)) { + return $resolved; } $start = (int) ($params['start'] ?? 0); @@ -67,7 +63,8 @@ public function getList(array $params = []): array return Response::error('Category products list service is not available', 500)->getData(); } - $page = $listService->getPage( + $results = $this->collectVisibleListPage( + $listService, $categoryId, $params, $nested, @@ -78,9 +75,19 @@ public function getList(array $params = []): array $sortDir ); + $total = $this->countVisibleListResults( + $listService, + $categoryId, + $params, + $nested, + $gridFields, + (string) $sortBy, + $sortDir + ); + return Response::success([ - 'results' => $page['results'], - 'total' => $page['total'], + 'results' => $results, + 'total' => $total, ])->getData(); } @@ -93,6 +100,12 @@ public function getList(array $params = []): array */ public function getFilters(array $params = []): array { + $categoryId = (int) ($params['id'] ?? 0); + $resolved = $this->requireCategoryWithView($categoryId); + if (is_array($resolved)) { + return $resolved; + } + /** @var FilterConfigManager $filterConfigManager */ $filterConfigManager = $this->modx->services->get('ms3_filter_config'); @@ -118,15 +131,17 @@ public function sort(array $params = []): array $items = $params['items'] ?? []; $nested = $this->isNested($params); - if (!$categoryId) { - return Response::error('Category ID is required', HttpStatus::BAD_REQUEST)->getData(); - } - if (empty($items) || !is_array($items)) { return Response::error('Items array is required', HttpStatus::BAD_REQUEST)->getData(); } + $resolved = $this->requireCategoryWithView($categoryId); + if (is_array($resolved)) { + return $resolved; + } + $updated = 0; + $policyDenied = 0; $scope = $this->scopeService(); @@ -140,12 +155,27 @@ public function sort(array $params = []): array $product = $scope->findInCategory($categoryId, $productId, $nested); - if ($product) { - $product->set('menuindex', $menuindex); - if ($product->save()) { - $updated++; - } + if (!$product) { + continue; } + + if (!CategoryProductDocumentPolicy::isAllowedAll($product, CategoryProductDocumentPolicy::sortPolicies())) { + $policyDenied++; + $this->logDocumentPolicyDenied($product, CategoryProductDocumentPolicy::sortPolicies()); + continue; + } + + $product->set('menuindex', $menuindex); + if ($product->save()) { + $updated++; + } + } + + if ($updated === 0 && $policyDenied > 0) { + return Response::error( + 'Save permission denied for this document', + HttpStatus::FORBIDDEN + )->getData(); } return Response::success([ @@ -210,8 +240,11 @@ public function multiple(array $params = []): array return Response::error('No valid product IDs provided', HttpStatus::BAD_REQUEST)->getData(); } + $documentPolicies = CategoryProductActionPermissions::documentPoliciesForMethod($method); + $success = 0; $failed = 0; + $policyDenied = 0; $scope = $this->scopeService(); foreach ($ids as $id) { @@ -222,6 +255,15 @@ public function multiple(array $params = []): array continue; } + if ( + $documentPolicies !== null + && !CategoryProductDocumentPolicy::isAllowedAll($product, $documentPolicies) + ) { + $this->logDocumentPolicyDenied($product, $documentPolicies); + $policyDenied++; + continue; + } + // $method is already validated by CategoryProductActionPermissions::evaluate() // above (unknown → 400 before this loop); default is defensive/unreachable. $result = match ($method) { @@ -242,6 +284,13 @@ public function multiple(array $params = []): array } if ($success === 0) { + if ($policyDenied > 0) { + return Response::error( + 'Save permission denied for this document', + HttpStatus::FORBIDDEN + )->getData(); + } + return Response::error('No products were updated', HttpStatus::INTERNAL_SERVER_ERROR)->getData(); } @@ -303,6 +352,13 @@ public function publish(array $params = []): array $published = $product->get('published') ? 0 : 1; } + $policies = CategoryProductDocumentPolicy::policiesForPublish((bool) $published); + if ($denied = CategoryProductDocumentPolicy::denialResponseAll($product, $policies)) { + $this->logDocumentPolicyDenied($product, $policies); + + return $denied; + } + if (!$this->applyPublish($product, (bool) $published)) { return Response::error('Failed to update product', HttpStatus::INTERNAL_SERVER_ERROR)->getData(); } @@ -331,6 +387,216 @@ private function scopeService(): CategoryProductScopeService : new CategoryProductScopeService($this->modx); } + /** + * @return msCategory|array msCategory on success, error response array on failure + */ + private function requireCategoryWithView(int $categoryId): object|array + { + if (!$categoryId) { + return Response::error('Category ID is required', HttpStatus::BAD_REQUEST)->getData(); + } + + $category = $this->modx->getObject(msCategory::class, $categoryId); + if (!$category) { + return Response::error('Category not found', HttpStatus::NOT_FOUND)->getData(); + } + + if ($denied = CategoryProductDocumentPolicy::denialResponse( + $category, + CategoryProductDocumentPolicy::categoryViewPolicy() + )) { + $this->modx->log( + modX::LOG_LEVEL_WARN, + '[CategoryProductsController] Document view denied for category ' + . $categoryId + . ' (user id ' . (int) ($this->modx->user->get('id') ?? 0) . ')' + ); + + return $denied; + } + + return $category; + } + + /** + * @param list> $results + * @return list> + */ + private function filterListResultsByDocumentView(array $results, bool $nested): array + { + if ($results === []) { + return []; + } + + $productIds = array_values(array_filter(array_map( + static fn (array $row): int => (int) ($row['id'] ?? 0), + $results + ))); + + if ($productIds === []) { + return []; + } + + /** @var array $productsById */ + $productsById = []; + $collection = $this->modx->getCollection(msProduct::class, ['id:IN' => $productIds]); + foreach ($collection as $product) { + if ($product instanceof msProduct) { + $productsById[(int) $product->get('id')] = $product; + } + } + + /** @var array $parentsById */ + $parentsById = []; + if ($nested) { + $parentIds = array_values(array_unique(array_filter(array_map( + static fn (array $row): int => (int) ($row['parent'] ?? 0), + $results + )))); + if ($parentIds !== []) { + $parentCollection = $this->modx->getCollection(msCategory::class, ['id:IN' => $parentIds]); + foreach ($parentCollection as $parent) { + if ($parent instanceof msCategory) { + $parentsById[(int) $parent->get('id')] = $parent; + } + } + } + } + + $filtered = []; + foreach ($results as $row) { + $productId = (int) ($row['id'] ?? 0); + $product = $productsById[$productId] ?? null; + if (!$product instanceof msProduct) { + continue; + } + + if (CategoryProductDocumentPolicy::canViewInCategoryGridCached($product, $nested, $parentsById)) { + $filtered[] = $row; + } + } + + return $filtered; + } + + /** + * @param array> $gridFields + * @return list> + */ + private function collectVisibleListPage( + CategoryProductsListService $listService, + int $categoryId, + array $params, + bool $nested, + array $gridFields, + int $start, + int $limit, + string $sortBy, + string $sortDir, + ): array { + if ($limit <= 0) { + return []; + } + + $visible = []; + $scanOffset = 0; + $skipped = 0; + $batchSize = max($limit * 2, 20); + + while (count($visible) < $limit) { + $page = $listService->getPage( + $categoryId, + $params, + $nested, + $gridFields, + $scanOffset, + $batchSize, + $sortBy, + $sortDir + ); + + if ($page['results'] === []) { + break; + } + + $filtered = $this->filterListResultsByDocumentView($page['results'], $nested); + foreach ($filtered as $row) { + if ($skipped < $start) { + $skipped++; + continue; + } + + $visible[] = $row; + if (count($visible) >= $limit) { + break 2; + } + } + + $scanOffset += count($page['results']); + if (count($page['results']) < $batchSize) { + break; + } + } + + return $visible; + } + + /** + * @param array> $gridFields + */ + private function countVisibleListResults( + CategoryProductsListService $listService, + int $categoryId, + array $params, + bool $nested, + array $gridFields, + string $sortBy, + string $sortDir, + ): int { + $visible = 0; + $scanOffset = 0; + $batchSize = 200; + + while (true) { + $page = $listService->getPage( + $categoryId, + $params, + $nested, + $gridFields, + $scanOffset, + $batchSize, + $sortBy, + $sortDir + ); + + if ($page['results'] === []) { + break; + } + + $visible += count($this->filterListResultsByDocumentView($page['results'], $nested)); + $scanOffset += count($page['results']); + + if (count($page['results']) < $batchSize) { + break; + } + } + + return $visible; + } + + /** @param list $policies */ + private function logDocumentPolicyDenied(msProduct $product, array $policies): void + { + $this->modx->log( + modX::LOG_LEVEL_WARN, + '[CategoryProductsController] Document policy denied (' + . implode(',', $policies) + . ') for product ' + . (int) $product->get('id') + . ' (user id ' . (int) ($this->modx->user->get('id') ?? 0) . ')' + ); + } + private function denyWithoutPermission(string $permission): ?array { if ($this->modx->hasPermission($permission)) { diff --git a/core/components/minishop3/src/Services/Category/CategoryProductActionPermissions.php b/core/components/minishop3/src/Services/Category/CategoryProductActionPermissions.php index 265e451b..517fb8ae 100644 --- a/core/components/minishop3/src/Services/Category/CategoryProductActionPermissions.php +++ b/core/components/minishop3/src/Services/Category/CategoryProductActionPermissions.php @@ -4,9 +4,19 @@ use MiniShop3\Router\HttpStatus; -/** Maps category product bulk/multiple actions to MODX permissions (#378). */ +/** Maps category product bulk/multiple actions to MODX permissions (#378) and document policies (#445). */ final class CategoryProductActionPermissions { + /** @var array}> */ + private const ACTIONS = [ + 'publish' => ['permission' => 'msproduct_publish', 'document' => ['publish']], + 'unpublish' => ['permission' => 'msproduct_publish', 'document' => ['save', 'unpublish']], + 'delete' => ['permission' => 'msproduct_delete', 'document' => ['delete']], + 'undelete' => ['permission' => 'msproduct_delete', 'document' => ['save', 'undelete']], + 'show' => ['permission' => 'msproduct_save', 'document' => ['save']], + 'hide' => ['permission' => 'msproduct_save', 'document' => ['save']], + ]; + /** * Permissions that may authorize POST …/products/multiple (route gate). * @@ -19,12 +29,17 @@ public static function mutationPermissions(): array public static function forMethod(string $method): ?string { - return match ($method) { - 'publish', 'unpublish' => 'msproduct_publish', - 'delete', 'undelete' => 'msproduct_delete', - 'show', 'hide' => 'msproduct_save', - default => null, - }; + return self::ACTIONS[$method]['permission'] ?? null; + } + + /** + * Resource-level MODX policies required before mutating a product document. + * + * @return list|null null when method is unknown + */ + public static function documentPoliciesForMethod(string $method): ?array + { + return self::ACTIONS[$method]['document'] ?? null; } /** diff --git a/core/components/minishop3/src/Services/Category/CategoryProductDocumentPolicy.php b/core/components/minishop3/src/Services/Category/CategoryProductDocumentPolicy.php new file mode 100644 index 00000000..8698208d --- /dev/null +++ b/core/components/minishop3/src/Services/Category/CategoryProductDocumentPolicy.php @@ -0,0 +1,173 @@ + */ + public static function sortPolicies(): array + { + return [self::POLICY_SAVE]; + } + + /** @return list */ + public static function policiesForPublish(bool $published): array + { + return $published + ? [self::POLICY_PUBLISH] + : [self::POLICY_SAVE, self::POLICY_UNPUBLISH]; + } + + public static function isAllowed(object $resource, string $policy): bool + { + return self::evaluate($resource, $policy) === null; + } + + /** @param list $policies */ + public static function isAllowedAll(object $resource, array $policies): bool + { + return self::evaluateAll($resource, $policies) === null; + } + + /** + * @return array{status: int, message: string}|null null when allowed + */ + public static function evaluate(object $resource, string $policy): ?array + { + if (method_exists($resource, 'checkPolicy') && $resource->checkPolicy($policy)) { + return null; + } + + return [ + 'status' => HttpStatus::FORBIDDEN, + 'message' => self::messageForPolicy($policy), + ]; + } + + /** + * @param list $policies + * @return array{status: int, message: string}|null null when allowed + */ + public static function evaluateAll(object $resource, array $policies): ?array + { + foreach ($policies as $policy) { + $denied = self::evaluate($resource, $policy); + if ($denied !== null) { + return $denied; + } + } + + return null; + } + + public static function denialResponse(object $resource, string $policy): ?array + { + return self::toErrorResponse(self::evaluate($resource, $policy)); + } + + /** @param list $policies */ + public static function denialResponseAll(object $resource, array $policies): ?array + { + return self::toErrorResponse(self::evaluateAll($resource, $policies)); + } + + public static function canViewInCategoryGrid(modX $modx, msProduct $product, bool $nested): bool + { + if (!self::isAllowed($product, self::POLICY_VIEW)) { + return false; + } + + if (!$nested) { + return true; + } + + $parentId = (int) $product->get('parent'); + if ($parentId <= 0) { + return false; + } + + $parent = $modx->getObject(msCategory::class, $parentId); + if (!$parent instanceof msCategory) { + return false; + } + + return self::isAllowed($parent, self::POLICY_VIEW); + } + + /** + * @param array $parentsById + */ + public static function canViewInCategoryGridCached( + msProduct $product, + bool $nested, + array $parentsById, + ): bool { + if (!self::isAllowed($product, self::POLICY_VIEW)) { + return false; + } + + if (!$nested) { + return true; + } + + $parentId = (int) $product->get('parent'); + if ($parentId <= 0) { + return false; + } + + $parent = $parentsById[$parentId] ?? null; + if (!$parent instanceof msCategory) { + return false; + } + + return self::isAllowed($parent, self::POLICY_VIEW); + } + + public static function toErrorResponse(?array $evaluation): ?array + { + if ($evaluation === null) { + return null; + } + + return Response::error( + $evaluation['message'], + $evaluation['status'] + )->getData(); + } + + private static function messageForPolicy(string $policy): string + { + return match ($policy) { + self::POLICY_VIEW => 'View permission denied for this document', + self::POLICY_SAVE => 'Save permission denied for this document', + self::POLICY_PUBLISH => 'Publish permission denied for this document', + self::POLICY_DELETE => 'Delete permission denied for this document', + self::POLICY_UNPUBLISH => 'Unpublish permission denied for this document', + self::POLICY_UNDELETE => 'Undelete permission denied for this document', + default => 'Access denied for this document', + }; + } +} diff --git a/core/components/minishop3/tests/CategoryProductActionPermissionsTest.php b/core/components/minishop3/tests/CategoryProductActionPermissionsTest.php index 4aee8695..5c503bf9 100644 --- a/core/components/minishop3/tests/CategoryProductActionPermissionsTest.php +++ b/core/components/minishop3/tests/CategoryProductActionPermissionsTest.php @@ -102,4 +102,19 @@ $assertSame(true, CategoryProductActionPermissions::evaluate('show', $allowAll)['allowed'], 'show allowAll'); +foreach ([ + ['publish', ['publish']], + ['unpublish', ['save', 'unpublish']], + ['delete', ['delete']], + ['undelete', ['save', 'undelete']], + ['show', ['save']], + ['unknown', null], +] as [$method, $expected]) { + $assertSame( + $expected, + CategoryProductActionPermissions::documentPoliciesForMethod($method), + "documentPoliciesForMethod({$method})" + ); +} + fwrite(STDOUT, "OK: CategoryProductActionPermissionsTest\n"); diff --git a/core/components/minishop3/tests/CategoryProductDocumentPolicyTest.php b/core/components/minishop3/tests/CategoryProductDocumentPolicyTest.php new file mode 100644 index 00000000..eb538d44 --- /dev/null +++ b/core/components/minishop3/tests/CategoryProductDocumentPolicyTest.php @@ -0,0 +1,94 @@ +getObjectCalls[] = ['class' => $className, 'criteria' => $criteria]; + if ($className === msCategory::class) { + $categoryId = is_array($criteria) + ? (int) ($criteria['id'] ?? 0) + : (int) $criteria; + + if ($categoryId <= 0) { + return null; + } + + foreach ($this->categories as $row) { + if ((int) $row['id'] === $categoryId) { + return new StubMsCategory($row); + } + } + + // Category ids used as product parents / API path params (direct children grid). + foreach ($this->products as $product) { + if ((int) ($product['parent'] ?? 0) === $categoryId) { + return new StubMsCategory(['id' => $categoryId]); + } + } + + return null; + } + if ($className !== msProduct::class) { return null; } diff --git a/core/components/minishop3/tests/stubs/StubMsCategory.php b/core/components/minishop3/tests/stubs/StubMsCategory.php new file mode 100644 index 00000000..c42379ae --- /dev/null +++ b/core/components/minishop3/tests/stubs/StubMsCategory.php @@ -0,0 +1,41 @@ + */ + private array $data; + + /** @var array|null null = allow all policies */ + private ?array $policies; + + /** + * @param array $data + * @param array|null $policies null allows every checkPolicy(); map denies/allows per policy + */ + public function __construct(array $data, ?array $policies = null) + { + $this->data = $data; + $this->policies = $policies; + } + + public function get(string $key): mixed + { + return $this->data[$key] ?? null; + } + + public function checkPolicy(string $policy): bool + { + if ($this->policies === null) { + return true; + } + + return !empty($this->policies[$policy]); + } +} diff --git a/core/components/minishop3/tests/stubs/StubMsProduct.php b/core/components/minishop3/tests/stubs/StubMsProduct.php index 6187eff4..17cb9e12 100644 --- a/core/components/minishop3/tests/stubs/StubMsProduct.php +++ b/core/components/minishop3/tests/stubs/StubMsProduct.php @@ -12,10 +12,17 @@ class StubMsProduct /** @var array */ private array $data; - /** @param array $data */ - public function __construct(array $data) + /** @var array|null null = allow all policies */ + private ?array $policies; + + /** + * @param array $data + * @param array|null $policies null allows every checkPolicy(); map denies/allows per policy + */ + public function __construct(array $data, ?array $policies = null) { $this->data = $data; + $this->policies = $policies; } public function get(string $key): mixed @@ -32,4 +39,13 @@ public function save(): bool { return true; } + + public function checkPolicy(string $policy): bool + { + if ($this->policies === null) { + return true; + } + + return !empty($this->policies[$policy]); + } }