feat(web-api): учитывать ACL групп ресурсов в публичном каталоге

[Ibochkarev]

Описание

Публичный Web API каталог (и добавление в корзину) для анонимных запросов скрывает товары и категории, входящие в группу ресурсов MODX с ACL «Доступ к группе ресурсов» для контекста запроса. Новая настройка ms3_web_catalog_respect_resource_groups (по умолчанию включена); также учитывается MODX access_resource_group_enabled.

Продуктовое решение по #659: позиция B (уважать resource groups), MVP только для anonymous — без маппинга msCustomer↔modUser.

Тип изменений

• Новая функциональность (non-breaking change)

Связанные Issues

Closes #659

Как это было протестировано?

cd core/components/minishop3
php -l src/Services/Catalog/CatalogResourceGroupVisibility.php
php -l src/Services/Product/ProductCatalogService.php
php -l src/Services/Category/CategoryCatalogService.php
php -l src/Services/Cart/CartItemManager.php
php tests/CatalogResourceGroupVisibilityTest.php
php tests/ProductCatalogServiceTest.php
php tests/CategoryCatalogServiceTest.php
./vendor/bin/phpunit tests/Unit/Services/Cart/CartItemManagerPureTest.php

• Автоматические тесты (smoke + PHPUnit unit)
• Ручное тестирование на сайте с реальной RG ACL

Конфигурация тестирования:

• MiniShop3: ветка feat/issue-659-catalog-resource-groups
• PHP: локальный CLI

Чеклист

• Код соответствует стилю проекта
• Лексиконы добавлены на двух языках (ru/en)
• Изменения не ломают существующую функциональность (выключатель настройки)
• PHPStan / полный composer ci:php — в CI
• CHANGELOG — при релизе (не в этом PR)

Дополнительные заметки

• Фильтр SQL NOT EXISTS по modResourceGroupResource + modAccessResourceGroup (без N+1 checkPolicy).
• Общий хелпер CatalogResourceGroupVisibility: product get/list/filters, category get/list/tree/breadcrumbs, CartItemManager::validateProduct.
• Вне scope MVP: principal-aware ACL через msCustomer, Delivery/Payment, сниппеты.

[biz87]

Спасибо — охват Web API проверил, он полный и аккуратный. Фильтр подключён везде, где нужно: product/get, list, filters, category/get, list, tree, хлебные крошки, фасетные агрегаты, добавление в корзину по id и галерея изображений из #598 (она прикрыта автоматически, потому что использует тот же findVisibleProduct). Категории идут через единственную точку входа createVisibleCategoriesQuery(), товары — через applyProductCatalogScope(), обойти нечем.

Реализация через NOT EXISTS на уровне запроса, а не checkPolicy() в цикле — правильное решение. Кстати, любопытная деталь: msProduct/msCategory наследуют modAccessibleObject, который умеет проверять политику сам, но делает это построчно после выборки, с отдельным запросом на объект. Так что переписать проверку на SQL было архитектурно оправдано.

Условие по контексту (context_key = :ctx OR context_key = '' OR context_key IS NULL) дословно совпадает с тем, что делает ядро. Выключатель работает: при выключенной настройке или при access_resource_group_enabled = false запрос идентичен прежнему. Существующие позиции в корзине не ломаются — гейт стоит только в validateProduct(), а он вызывается лишь из add(), тогда как change(), changeOption(), remove() и оформление заказа работают с уже сохранённым msOrderProduct.

Числа: мерж с beta не требуется (ветка уже на свежей), smoke 99/99, PHPUnit 304 теста / 757 assertions, WebApi 24/24, PHPStan из vendor/bin — 0 ошибок.

Но два момента нужно закрыть до вливания.

1. Ресурс с явным грантом анониму скрывается, хотя должен быть виден

CatalogResourceGroupVisibility::buildNotExistsSql() считает ресурс скрытым при любой строке в access_resource_groups для его группы и контекста.

Настоящая семантика другая. В MODX есть псевдогруппа «(anonymous)» с principal = 0 — она доступна в менеджере через usergroup/update?id=0 и является штатным способом открыть группу ресурсов анонимным посетителям. Для неавторизованного запроса ядро идёт именно за такими строками (modAccessResourceGroup::loadAttributes(), ветка userId = 0):

WHERE acl.principal_class = 'MODX\Revolution\modUserGroup'
  AND acl.principal = 0
  AND (acl.context_key = :context OR acl.context_key IS NULL OR acl.context_key = '')

То есть если админ явно разрешил анониму, ресурс виден — даже когда для той же группы ресурсов есть ограничения для других групп пользователей.

Проверено эмпирически на живой базе, в транзакции с откатом:

1. Товар без ACL → SQL из PR отдаёт как видимый. Верно.
2. Добавлена строка principal = 1 (Administrator), товар в этой группе → SQL скрывает. Верно и ожидаемо.
3. Добавлена ещё одна строка, principal = 0 (anonymous), политика «Load Only» → SQL из PR по-прежнему скрывает товар, хотя ядро в этом случае считает его доступным.

Сценарий не выдуманный: группы ресурсов часто используют для сайта целиком, а не только для магазина, и связка «ограничения для одних групп плюс явный грант анониму» — обычная настройка. После обновления такие товары исчезнут из каталога молча: ни ошибки, ни записи в лог. Владелец узнает от покупателей.

Это как раз тот тип ошибки, который опаснее исходной утечки: потерять товары хуже, чем показать лишние, потому что утечка пассивна, а пропажа ломает продажи.

Починка — исключение для principal = 0 в условии. Заодно стоит добавить фильтр principal_class = 'MODX\Revolution\modUserGroup', как это делает modResource::findPolicy(). На практике сейчас не эксплуатируется, потому что процессоры создания ACL всегда проставляют этот класс, но пусть условие совпадает с ядром.

2. Тесты проверяют форму запроса, а не результат

tests/CatalogResourceGroupVisibilityTest.php делает строковые ассерты на сгенерированный SQL (str_contains($sql, 'NOT EXISTS')) и проверяет isEnabled() на моках getOption(). Ни один тест не исполняет этот SQL против реальных document_groups и access_resource_groups.

Именно поэтому случай с анонимом и не пойман — форма запроса правильная, результат неправильный.

Комментарий в tests/support/SqliteDraftCartHarnessTrait.php («catalog RG is covered elsewhere») не подтверждается: такого покрытия в диффе нет нигде.

Прошу интеграционный тест на четыре случая: ресурс без группы; группа без единой ACL-записи; группа с ACL только для чужой группы пользователей; группа с явным грантом анониму.

Отдельно — про масштаб, не к правке в этом PR

Классический Fenom-сниппет ms3_products.php строит листинг через ModxPro\PdoTools\Fetch сырым SQL, минуя ProductCatalogService и весь наш сервисный слой. Проверил грепом: ни один сниппет не обращается ни к ProductCatalogService, ни к новому CatalogResourceGroupVisibility.

Значит на обычной, не headless инсталляции — а это сегодня подавляющее большинство магазинов на MS3 — закрытые группы ресурсов по-прежнему не учитываются. Это не регрессия от твоего PR, поведение сниппета не менялось. Но означает, что #659 после вливания закрывается только для Web API.

Завёл отдельную задачу на классическую витрину — там своя работа со своими вопросами, встроить фильтр в pdoTools-путь одной строкой не выйдет. А в описании релиза надо будет честно написать, что Web API закрытые группы теперь уважает, а Fenom-витрина пока нет.

[Ibochkarev]

@biz87 Спасибо за ревью — оба пункта закрыл.

1. Грант анониму (principal = 0)

buildNotExistsSql() теперь:

• считает «закрывающей» только ACL с principal <> 0 и principal_class IN (modUserGroup / FQCN);
• если на ту же группу ресурсов есть явный грант principal = 0 — ресурс виден (кейс Administrator + anonymous Load Only).

2. Тесты на результат, не только на форму

Добавлен tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php: SQL исполняется против in-memory SQLite на четырёх сценариях (+ только anonymous grant).

./vendor/bin/phpunit tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php
# OK (6 tests, 9 assertions)

php tests/CatalogResourceGroupVisibilityTest.php
# OK

composer test:smoke
# OK (100)

Про масштаб Fenom/#670 — согласен, в scope этого PR не входит; в релизной заметке отметим Web API vs классическая витрина.

[biz87]

Первый пункт возврата закрыт по-настоящему, и это проверено не на моках.

Анонимный грант внутри одной группы теперь работает: SQL прогнан против реальной схемы dev-стенда, в транзакции с откатом, по шести сценариям. Ресурс без групп виден, ресурс в группе с ACL только для чужой группы скрыт, ресурс с явным грантом principal = 0 виден — как и требует ядро.

Второй пункт тоже закрыт: tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php действительно исполняет запрос против таблиц, а не сверяет текст. Это именно то, чего не хватало.

Числа: мерж с актуальной beta чистый, smoke 100/100, PHPUnit 322 теста / 1397 assertions, WebApi 24/24, PHPStan по изменённым файлам — 0 ошибок.

Но на шестом сценарии — том самом, про множественное членство, — обнаружился баг того же класса.

Ресурс в нескольких группах скрывается, хотя должен быть виден

Сценарий: ресурс состоит в двух группах ресурсов. Группа A ограничена — есть ACL для какой-то группы пользователей, анонимного гранта нет. Группа B открыта анониму.

По семантике ядра такой ресурс виден. Членство работает как ИЛИ: достаточно, чтобы доступ давала хотя бы одна из групп. Это видно в modAccessibleObject::checkPolicy() — вложенные циклы идут по всем политикам и целям, при первом совпадении return true, а return false стоит только после исчерпания всех.

Текущий SQL даёт visible = FALSE. Воспроизведено на реальной MySQL.

Причина в корреляции. Внутренний NOT EXISTS, который ищет анонимный грант, привязан к dg.document_group, а не к dg.document. То есть проверка идёт попарно внутри одной строки членства: «есть ли в этой группе ограничение и есть ли в этой же группе грант анониму». А нужно на уровне документа: есть ли ограничение хоть в одной его группе, и есть ли анонимный грант хоть в одной.

Сценарий бытовой, не экзотический: товар лежит в общей открытой группе и заодно попал в какую-то закрытую — и пропадает с витрины. Это снова ложное скрытие, то есть молчаливая потеря товаров, которая хуже исходной утечки.

Тестов на этот случай нет ни в CatalogResourceGroupVisibilityTest.php, ни в новом SQL-тесте.

Почему это важно починить именно здесь

На ветке этого PR стоят два зависимых, и оба берут этот SQL как готовый:

• feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 выносит тело в buildProtectedMembershipExistsSql() дословно — баг наследуется и распространяется ещё и на авторизованных покупателей через OR по разрешённым группам;
• Respect resource groups in Fenom storefront snippets #681 оборачивает то же самое в buildWhereFragment() и isVisible().

Сейчас это одна правка в одном месте. После ребейза стека — три параллельные правки в трёх PR, которые придётся синхронизировать.

Направление правки: развязать анонимную проверку от привязки к конкретной строке document_groups. Считать два факта на уровне документа — «есть ли ограничение хоть в одной из его групп» и «есть ли анонимный грант хоть в одной из его групп» — и сравнивать уже их. Конкретную форму запроса оставляю тебе, это семантика ACL, а не механическая правка.

Второе, не блокирующее

CartItemManager::validateProduct() — новая защита от добавления в корзину скрытого товара по прямому id — не покрыта ни одним тестом.

tests/support/SqliteDraftCartHarnessTrait.php принудительно выставляет ms3_web_catalog_respect_resource_groups => false с комментарием, что в SQLite-харнессе нет ACL-таблиц. То есть под тестом этот код не исполнялся ни разу, и заявление «нельзя обойти через product id» подтверждено только комментарием в коде.

Понимаю, что поднять ACL-таблицы в SQLite-харнессе — отдельная работа. Но раз новый SQL-тест уже умеет создавать document_groups и access_resource_groups, возможно, туда же получится добавить и кейс корзины.

[Ibochkarev]

Исправлено в 8bd5f3e8 (ответ на комментарий).

Анонимный грант больше не коррелирует с dg.document_group той же строки членства. Предикат на уровне документа:

• NOT EXISTS — есть ли ограничение (principal ≠ 0) хоть в одной группе ресурса
• OR EXISTS — есть ли principal = 0 хоть в одной его группе

Сценарий «закрытая A + открытая анониму B» даёт visible. Две закрытые группы — по-прежнему скрывают.

Покрытие: testMultiGroupRestrictedPlusAnonymousIsVisible / testMultiGroupTwoRestrictedIsHidden в CatalogResourceGroupVisibilitySqlTest + smoke на OR EXISTS / dg_anon.document.

php tests/CatalogResourceGroupVisibilityTest.php  # OK
vendor/bin/phpunit tests/Unit/Catalog/CatalogResourceGroupVisibilitySqlTest.php  # 8 tests, 13 assertions

CartItemManager::validateProduct зовёт тот же apply(), multi-group там тоже закрыт. Отдельный cart-кейс с ACL в SQLite-харнессе не поднимал.

После мержа #666 — ребейз #677 / #681.