[Ibochkarev]

Описание

Продолжение #659/#666: анонимный фильтр групп ресурсов уже скрывает закрытые разделы, но авторизованный msCustomer для MODX всё ещё гость. Добавлена сущность msCustomerGroup со ссылкой на modUserGroup; видимость каталога считается через access_resource_groups и тот же SQL-контур, без второго реестра прав и без modUser на каждого покупателя.

Depends on #666 (база PR — ветка feat/issue-659-catalog-resource-groups).

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

• Исправление бага (non-breaking change)
• Новая функциональность (non-breaking change)
• Breaking change (изменение, ломающее обратную совместимость)
• Рефакторинг (без изменения функциональности)
• Документация
• Другое (опишите):

Связанные Issues

Closes #669

Depends on #666 / Refs #659

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

cd core/components/minishop3
php -l src/Services/Catalog/CatalogResourceGroupVisibility.php
php -l src/Services/Catalog/CustomerResourceGroupResolver.php
# exit 0

php tests/CatalogResourceGroupVisibilityTest.php
# OK, exit 0

php tests/CustomerResourceGroupResolverTest.php
# OK, exit 0

php tests/ProfileQuickUpdateForbiddenTest.php
php tests/CustomerPublicDtoTest.php
# OK, exit 0

composer test:smoke
# OK smoke tests (101), exit 0

После деплоя: phinx migrate (таблица ms3_customer_groups + customer_group_id).

• Ручное тестирование
• Автоматические тесты (composer ci:php / composer test, npm run lint:ci, composer stan / GitHub Actions CI)
• Тестирование на разных версиях PHP/MODX

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

• MiniShop3: feat/issue-669-customer-resource-groups поверх feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666
• MODX: n/a (unit/smoke)
• PHP: 8.4.23

Скриншоты (если применимо)

До	После
n/a	n/a

Чеклист

• Код соответствует стилю проекта
• Добавлены/обновлены комментарии в сложных местах
• Изменения не ломают существующую функциональность
• Лексиконы добавлены на двух языках (ru/en)
• PHPStan проходит без новых ошибок (composer stan / CI job PHPStan)
• ESLint проходит без ошибок (npm run lint:ci для Vue)
• Обновлён CHANGELOG.md (для значимых изменений)

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

MVP: одна группа на покупателя, без Vue UI, без чистки корзины при потере доступа. Manager API: /api/mgr/customer-groups + customer_group_id в PUT /customers/{id}. Web profile не отдаёт и не пишет customer_group_id. Blocked/inactive покупатель остаётся на anonymous-гейте. После merge #666 перенацелить PR на beta.

[Ibochkarev]

Подтянул фикс из #666 (anonymous principal = 0 + SQLite execution tests) в эту ветку. Protected-membership SQL для #669 использует ту же семантику.

[biz87]

По существу PR готов — проверил все девять пунктов, критичных проблем нет. Не вливаю только потому, что база (#666) уходит на доработку, и есть неразрешённый конфликт с #681. Подробности ниже.

Что подтвердилось

Реализован именно выбранный вариант. msCustomerGroup содержит name, user_group_id, active и метки времени — никаких списков групп ресурсов внутри модели. Видимость считается запросом к access_resource_groups по principal:IN с теми же классами принципалов, что и в анонимном гейте. То есть права остаются в одном месте, в MODX, а второго реестра не появилось — это было главным опасением в #669.

modUser не создаётся — проверено грепом по всем изменённым файлам.

Аноним идёт ровно прежним путём. При отсутствии токен-сервиса, отсутствующем или истёкшем токене, заблокированном покупателе, покупателе без группы, неактивной группе и user_group_id = 0 резолвер возвращает пустой массив, а buildNotExistsSql() становится алиасом buildVisibilitySql(..., []). Второй ветки логики для анонима нет, все четыре потребителя переведены на единый applyForRequest().

Существующие покупатели не ломаются. customer_group_id добавлен как int unsigned NULL без дефолта, существующие записи получают NULL, что трактуется как отсутствие группы — обычный каталог остаётся доступен. Покрыто тестом.

В публичный ответ ничего не утекает. И customer_group_id, и user_group_id в блок-листе CustomerPublicDto, оба проверены тестами — тот же whitelist-паттерн, что мы проверяли в #639.

Миграция и схема согласованы. hasTable/hasColumn без префикса, проверки перед addColumn/addIndex, создание через createObjectContainer — тот же паттерн, что в 20260518120000_create_option_groups_and_migrate.php. Схема и mysql-модели совпадают по типам, null, дефолтам и индексам. Зарезервированных слов MySQL 8 в новых колонках нет.

Роуты закрыты PermissionMiddleware с теми же правами msorder_*, что уже используются для существующего /customers — переиспользование конвенции, а не новое ослабление.

Отдельно отмечу решение по кэшу — оно лучше того, что я предполагал в issue. Резолв не кэшируется персистентно, а сегментом ключа кэша фасетов служит хеш от текущего набора разрешённых групп. При смене группы покупателя или правке ACL ключ меняется сам на следующем запросе, ручная инвалидация не нужна. Вопрос из #669 про «где хранить и когда сбрасывать» закрыт элегантно.

N+1 нет — фильтр остаётся условием запроса, а resolveAllowedResourceGroupIdsForCustomer() мемоизирован по паре «покупатель + контекст».

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

Что мешает влить прямо сейчас

База уходит на доработку. В #666 нашёлся баг: ресурс, состоящий в нескольких группах ресурсов, скрывается, даже если одна из групп открыта анониму — членство в MODX работает как ИЛИ, а SQL проверяет попарно внутри одной строки членства. Ваш buildProtectedMembershipExistsSql() выносит этот SQL дословно, то есть баг наследуется и распространяется ещё и на авторизованных покупателей через OR по разрешённым группам. Подробности в комментарии к #666.

Конфликт с #681. Вы оба стоите на ветке #666 и по-разному переписываете CartItemManager::validateProduct(): вы переводите на applyForRequest() с учётом группы покупателя, #681 — на isVisible(), который документирован как anonymous-only и списка разрешённых групп не принимает. Взять одну версию целиком нельзя ни в каком порядке: либо авторизованный покупатель увидит товар в каталоге, но не сможет положить его в корзину, либо потеряется то, ради чего #681 трогал этот метод.

То же и с самим CatalogResourceGroupVisibility: вы добавляете четвёртый параметр к apply() плюс applyForRequest() и buildVisibilitySql(), #681 добавляет buildWhereFragment() и isVisible() — написано независимо, друг про друга не знает.

Нужно ваше решение, как свести apply(), applyForRequest(), buildWhereFragment() и isVisible() в одну согласованную точку входа и в каком порядке мержить пару. Механически это не разруливается.

Мелочи, на усмотрение

• resolveAllowedIdsForRequest() каждый вызов заново резолвит токен и делает getObject(msCustomerToken::class, ...), мемоизация есть только глубже. При фасетном запросе с двумя десятками ключей это до двух десятков лишних точечных SELECT по индексированному полю. Не N+1 по каталогу, но кэшировать customerId по контексту на уровне резолвера дёшево.
• На ms3_customer_groups.user_group_id нет уникального индекса — несколько групп покупателей могут ссылаться на одну группу пользователей MODX. Возможно, так и задумано (разные сегменты на одну группу прав), но стоит записать это явно, чтобы следующий читатель не счёл упущением.

[Ibochkarev]

Сведено с #681 и пофикшенным #666.

Порядок мержа: #666 → #681 → #677 (база PR переключена на fix/issue-670-fenom-resource-groups).

API в одной точке входа

• apply($q, $alias, $ctx, $allowedIds = []) → через buildWhereFragment()
• applyForRequest() / appliesToCacheKeyForRequest() — токен → allowed ids (msCustomerGroup: связать покупателей с группами ресурсов MODX #669)
• buildWhereFragment() / isVisible() — Fenom (Классическая Fenom-витрина не учитывает группы ресурсов #670), опциональный $allowedIds
• CartItemManager::validateProduct() остаётся на applyForRequest() (каталог и корзина с одной политикой для авторизованного)

Multi-group (#666): buildProtectedMembershipExistsSql() наследует document-level OR: restricted на любой группе + нет principal=0 на любой группе документа (dg_anon.document = alias.id). Smoke + CatalogResourceGroupVisibilitySqlTest зелёные.

Мелочи из ревью

• resolveAllowedIdsForRequest() мемоизирует token → customer_id на инстансе резолвера
• в модели/миграции явно: user_group_id не уникален намеренно (несколько сегментов → один modUserGroup)

[Ibochkarev]

Уточнение по базе: GitHub не даёт сменить base у stacked PR (Cannot change the base branch because the pull request is part of a stack). База остаётся feat/issue-659-catalog-resource-groups (#666), tip ветки — поверх tip #681 (901fc0c7). После мержа #666 → #681 в #677 останется только коммит с customer groups. Mergeable сейчас зелёный.

[biz87]

Слой проверен, дизайн тот же, что одобрен в прошлый раз. Вливать можно только после исправления #666: buildVisibilitySql() и buildProtectedMembershipExistsSql() собирают IN (...) через principalClassInList(), и на MySQL с полным именем MODX\Revolution\modUserGroup строки ACL не находятся — закрытые ресурсы видны всем. Подробности и таблица сценариев на MySQL — в #666.

Ветку с разрешёнными группами я прогнал там же: с правильным квотированием покупатель с грантом видит ресурс в «закрытой A + разрешённой F» и «только в F», аноним — нет.

Резолвер ищет ACL через критерии xPDO (principal_class:IN), значения там биндятся — его эта ошибка не касается.

Что подтвердилось

• msCustomerGroup хранит только ссылку на modUserGroup, modUser не создаётся, аноним идёт ровно путём feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666: buildVisibilitySql(..., []) совпадает с buildNotExistsSql().
• Резолвер: просроченный токен, неактивный или заблокированный покупатель, неактивная группа, user_group_id = 0 — пустой набор. Ключи мемоизации включают покупателя и контекст.
• Кэш фасетов: разные наборы групп дают разные сегменты, аноним и покупатель без группы делят один.
• Корзина через applyForRequest() — та же политика, что в каталоге.
• Выставить себе группу через публичный API нельзя: customer_group_id вне PUBLIC_FIELDS, входит в SYSTEM_NON_EDITABLE_FIELDS, отсекается при обновлении профиля, регистрация его не читает.
• Маршруты /customer-groups под msorder_*; удаление группы сначала отвязывает покупателей.
• Миграция идемпотентна, schema.xml и модели совпадают, зарезервированных слов нет, номер 20260909120000 не пересекается с beta и Repair missing product_fields.section foreign key #713. PhinxSchemaLiveTest в CI прошёл.
• После мержа со свежей beta: smoke 104/104, PHPUnit 400, PHPStan без ошибок.

Замечания

1. Один список классов, как в ядре. В резолвере свой PRINCIPAL_CLASSES с коротким modUserGroup, а ядро учитывает только modUserGroup::class. После правки feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666 стоит держать одно определение и для класса видимости, и для резолвера — иначе резолвер будет выдавать разрешённые группы по старым строкам ACL, которые MODX не считает действующими.
2. Истёкшая блокировка. Резолвер возвращает пустой набор при is_blocked = 1, даже если blocked_until уже в прошлом. AuthManager::authenticate() в этом случае снимает блокировку. Покупатель с действующим токеном после окончания блокировки потеряет расширенный доступ до повторного входа. Ошибка в безопасную сторону, но комментарий «Match AuthManager gate» неточен.
3. Упрощения относительно ядра — записать явно. Резолвер считает грантом любую строку ACL, не глядя на содержимое политики (есть ли там view и load), и не учитывает authority роли, которую ядро сверяет для пользователей (mr.authority <= acl.authority) — у покупателя роли нет. Для штатных политик разницы нет, но это стоит записать в докблоке резолвера и в документации, чтобы администратор с нестандартной политикой не удивлялся.
4. Тесты. На CustomerGroupsController тестов нет. Ветка SQL с разрешёнными группами проверяется только сборкой строки и на данных не исполняется — хорошо бы добавить её в тот же MySQL-тест, о котором прошу в feat(web-api): учитывать ACL групп ресурсов в публичном каталоге #666.

Конфликт с #681

Порядок #666 → #681 → #677. После вливания #681 здесь будут конфликты в трёх файлах:

• CatalogResourceGroupVisibility.php — брать эту версию целиком, она надмножество;
• CartItemManager.php — конфликт только в комментарии, вызов applyForRequest() сливается сам;
• tests/CatalogResourceGroupVisibilityTest.php — объединить обе вставки.

[Ibochkarev]

@biz87 Спасибо. База подтянута с tip #666 (1e00fc92 — quote principal_class), поверх неё переложен только коммит customer groups.

Блокер MySQL: buildVisibilitySql / buildProtectedMembershipExistsSql / buildNotExistsSql принимают $quotedPrincipalClass от $modx->quote(modUserGroup::class), сравнение =, без principalClassInList().

Один список как в ядре: у резолвера убран свой PRINCIPAL_CLASSES с коротким modUserGroup — критерий principal_class => modUserGroup::class.

blocked_until: при is_blocked и истёкшем blocked_until группы снова резолвятся (как после AuthManager::authenticate()), без мутации флагов; комментарий и докблок обновлены.

Упрощения vs ядро: в докблоке резолвера явно: не смотрим policy view/load, не сравниваем authority.

Тесты:

• SQL unit: ветка с allowed groups (открывает закрытую / A+F / чужой id не открывает)
• live Modx: testAllowedResourceGroupOpensClosedProductForCustomer
• smoke CustomerGroupsControllerTest (валидации get/create/delete + пустой list)
• smoke резолвера: future vs expired blocked_until

Локально: visibility/resolver/controller smoke + PHPUnit SQL 13/13. Live MySQL — без DSN, жду CI.

[biz87]

Проверил. Квотирование principal_class исправлено, SQL фильтра на реальной MySQL отрабатывает правильно: 10 сценариев групп ресурсов, аноним и покупатель с переданными разрешёнными группами — ни одного расхождения. Упрощения относительно ядра записаны, customer_group_id в публичные поля не попадает. Но резолвер на реальной базе не находит ни одной разрешённой группы. Возвращаю.

Блокер: запрос резолвера не выполняется

В CustomerResourceGroupResolver::resolveAllowedResourceGroupIdsForUserGroups():

$c->where([
    'context_key' => $contextKey,
    'OR:context_key' => '',
    'OR:context_key:IS' => null,
]);

xPDO (xPDOQuery::parseConditions()) читает ключ из двух частей как поле:оператор, союз распознаётся только в ключе из трёх частей. 'OR:context_key' становится полем OR с оператором CONTEXT_KEY:

( `modAccessResourceGroup`.`context_key` = 'web' AND `modAccessResourceGroup`.`OR` CONTEXT_KEY '' OR `modAccessResourceGroup`.`context_key` IS NULL )

MySQL/MariaDB отвечают 1064 You have an error in your SQL syntax, execute() возвращает false, и метод молча отдаёт []. На сайте покупатель с группой никогда не получит доступ к закрытым группам — фича не работает. Ошибка в безопасную сторону, утечки нет.

Проверил на dev-базе (MariaDB), строки ACL созданы через modAccessResourceGroup, всё в транзакции с откатом: для группы с грантами в web, в пустом контексте и в mgr резолвер вернул [] во всех случаях.

Исправление — ключ из трёх частей:

'OR:context_key:=' => '',

С ним на тех же данных резолвер вернул ожидаемые группы для web (грант в web и в пустом контексте), для mgr, для нескольких групп пользователя; короткий modUserGroup не учитывается. Условие тогда совпадает с ядром: modAccessResourceGroup::loadAttributes() и modResource::findPolicy() берут (context_key = :context OR context_key IS NULL OR context_key = '').

Заодно: при false от prepare()/execute() стоит писать $modx->log(modX::LOG_LEVEL_ERROR, ...) с errorInfo() — сейчас ошибка SQL незаметна.

Тест, который это ловит

Юнит-тест резолвера мокает newQuery()/where(), а testAllowedResourceGroupOpensClosedProductForCustomer передаёт разрешённые группы в фильтр вручную — запрос резолвера не исполняется нигде. Нужен live-тест в tests/Modx/: modUserGroup → msCustomerGroup с user_group_id → msCustomer → modAccessResourceGroup на закрытую группу → resolveAllowedResourceGroupIdsForCustomer() возвращает эту группу, а товар из неё виден через apply() с результатом резолвера. Хорошо бы добавить строку ACL с пустым context_key.

Не блокер

• Комментарий «Align with AuthManager::authenticate()» в loadAllowedIdsForCustomer() неточен: при is_blocked = 1 и пустом blocked_until AuthManager снимает блокировку и пускает, а резолвер закрывает доступ. Направление безопасное, но формулировку стоит поправить.

Порядок

GitHub после вливания #666 сам перебазировал этот PR на beta, собственный diff не изменился. #681 остался на ветке #666 и против beta конфликтует — его нужно перебазировать отдельно. После вливания #681 здесь будут конфликты в CatalogResourceGroupVisibility.php, CartItemManager.php и tests/CatalogResourceGroupVisibilityTest.php, как разбирали раньше.

[Ibochkarev]

Починил.

Блокер. 'OR:context_key:=' => '' — ключ из трёх частей, как в ядре (loadAttributes / findPolicy). При prepare()/execute() === false пишется LOG_LEVEL_ERROR с errorInfo().

Тест. CatalogResourceGroupVisibilityLiveTest::testResolverQueryFindsWebAndEmptyContextAcls: modUserGroup → msCustomerGroup → msCustomer → ACL на web и пустой context_key, mgr не попадает. resolveAllowedResourceGroupIdsForCustomer() исполняет запрос. Товар открывается через apply() с результатом резолвера. Smoke ловит регресс двухчастного OR:context_key.

Не блокер. Комментарий про AuthManager: при is_blocked и пустом blocked_until резолвер по-прежнему закрывает доступ, флаги не трогает.

Live MySQL локально без DSN. Жду composer test:modx в CI.

[Ibochkarev]

Перебазировал на актуальный beta (после merge #681).

Конфликты по плану из ревью:

• CatalogResourceGroupVisibility.php — версия feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 целиком (надмножество) + class_key из Respect resource groups in Fenom storefront snippets #681 в isVisible()
• CartItemManager.php — комментарий; вызов applyForRequest() сохранён
• tests/CatalogResourceGroupVisibilityTest.php — объединены вставки Respect resource groups in Fenom storefront snippets #681 (innerJoin / class_key) и feat(web-api): msCustomerGroup и ACL групп ресурсов для авторизованных #677 (principal cache key)

Smoke visibility/resolver/controller + PHPUnit SQL 13/13 зелёные. Merge-tree против beta чистый.