Repository navigation
Выравнивание ACL каталога на Fenom и hardening токенов - #757
Conversation
Member-aware resource-group visibility for storefront snippets matches Web API, revoked session tokens are not reused, query-string ms3_token no longer elevates ACL, reset denies inactive accounts, and unsafe snippet sortby parts are dropped.
Ibochkarev
added
bug
Replace ?? on $modx->resource with isset so nullCoalesce.initializedProperty does not fire on the typed modx property.
PHPStan flags isset()/?? on CatalogResourceGroupVisibility::$modx. Read resource into a local and guard with is_object instead.
|
Ветка сливается с актуальной Основное проверил на живом MODX, не только по коду. Развернул ветку на dev (включая сниппеты в БД), закрыл товар группой ресурсов, завёл покупателя с группой клиентов → группой пользователей MODX и сравнил с
Query-token elevation воспроизвёл на Гейт Возвращаю из-за сортировки. 1.
|
&sortby= |
beta |
PR |
|---|---|---|
pagetitle DESC, publishedon |
ORDER BY msProduct.pagetitle DESC, msProduct.publishedon |
так же |
RAND() |
ORDER BY RAND() |
ORDER BY msProduct.id |
FIELD(msProduct.id, 5, 3, 1) |
как есть | ORDER BY msProduct.id |
Случайная выдача товаров — ходовой приём для блоков «Смотрите также» и «Хиты», и ломается она молча: ни ошибки, ни записи в журнал, просто список внезапно отсортирован по id. То же самое с FIELD() для ручного порядка и любыми другими функциями.
Whitelist сам по себе нужен — &sortby=pagetitle; DROP TABLE x на beta уходит в pdoTools как есть, а в PR отбрасывается, это правильно. Но резать стоит по конструкции, а не по факту наличия скобки. Минимально: разрешить RAND() без аргументов; шире — белый список функций (RAND, FIELD, IFNULL, COALESCE, CAST) с проверкой, что внутри скобок только разрешённые поля, числа и строки.
2. Отброшенные части нигде не фиксируются
Помимо функций, drop-режим молча выбрасывает всё, что не поле ресурса, не поле msProductData, не объявлено через includeTVs / includeVendorFields / sortbyOptions и не имеет алиаса из ['msProduct', 'Data', 'Vendor']. Под это попадает сортировка по полю из пользовательского &leftJoin с собственным алиасом — распространённый приём на кастомных витринах. Когда все части отброшены, подставляется msProduct.id, и владелец сайта видит просто «сортировка сломалась».
Достаточно одной строки $modx->log(modX::LOG_LEVEL_WARN, ...) с перечислением отброшенного — тогда причина находится по журналу за минуту. Плюс имеет смысл добавлять в список разрешённых алиасов те, что пришли в &leftJoin / &innerJoin.
3. Member-каталог не работает на кэшируемых страницах после первого анонимного визита
Проверил на ms3.test страницей с кэшируемым [[msProducts]]:
| Порядок запросов | Результат |
|---|---|
| Аноним первым, затем покупатель | покупатель получает анонимный список; сниппет не выполняется вовсе |
| Покупатель первым (кэш пуст) | покупатель видит member-товар, страница в кэш не пишется |
| Аноним после покупателя | видит только публичный, кэширует анонимную версию |
| Покупатель после этого | снова анонимный список |
Со стороны безопасности всё правильно: member-HTML в общий ключ не попадает, утечки анонимам нет — cacheable=0 делает ровно то, что заявлено. Но MODX отдаёт кэш страницы до выполнения сниппетов, поэтому в обычной жизни (аноним заходит первым практически всегда) member-каталог на кэшируемой странице не работает никогда.
В лексиконе это описано как «предпочтительны вызовы [[!ms3_products]]». По факту это не предпочтение, а обязательное условие: без ! фича не работает. Предлагаю переформулировать прямо — и заодно упомянуть в документации и CHANGELOG, иначе на поддержку придут вопросы «включил настройку, а member ничего не видит».
Мелочь
disablePageCacheForMemberCatalog() покрыт только проверкой наличия метода в исходнике (CatalogResourceGroupVisibilityTest.php), а не утверждением про cacheable. Механизм сверил с ядром вручную, работает, но от будущих правок тест не защитит.
Allow RAND/FIELD/IFNULL/COALESCE/CAST in snippet drop mode, warn on dropped parts, accept leftJoin aliases, and state uncached snippets as required for member ACL on cacheable pages.
|
Адрес по комментарию ревью: 1.
|
Stop assigning a fresh array into the optional by-ref out-param up front; only write list<string> when the caller actually passed the argument.
* fix(snippets): вернуть сортировку по опциям и полям производителя Белый список sortby (#742/#757) ломал два рабочих сценария. Ключ sortbyOptions, совпадающий с колонкой msProductData (weight, color, size, price и другие), квалифицировался как Data.weight, после чего подстановка выражения опции давала Data.CAST(...) — битый SQL и пустая выдача. Документированный пример sortby=weight + sortbyOptions=weight:number переставал работать. Явно объявленные имена (sortbyOptions, includeTVs, includeVendorFields) теперь проверяются раньше колонок. Зарегистрированный дефолт includeVendorFields='*' попадал в белый список литералом vendor_*, поэтому sortby=vendor_name молча отбрасывался на обычном вызове [[msProducts]]. Звёздочка разворачивается в поля msVendor. * fix(snippets): тот же приоритет объявленных имён внутри аргументов функций Порядок проверок правился только в qualifySimplePart, а у аргументов разрешённых функций остался прежним: FIELD(color,'red','blue') с опцией color превращался в FIELD(Data.color,...) — то есть регрессия сохранялась для ручного порядка значений опции. Заодно исключаем vendor id из разворачивания includeVendorFields='*' — он и так не входит в выборку.
Описание
После v1.13.0-beta1 сниппеты витрины оставались на анонимном RG-гейте, а Web API уже поднимал member-каталог.
token/getотдавал отозванный токен из сессии,ResetPasswordменял пароль inactive-клиенту, catalog API принималms3_tokenиз query, а&sortby=на сниппете мог уходить в ORDER BY без whitelist.Этот PR выравнивает Fenom с Web API через
buildWhereFragmentForRequest/isVisibleForRequest, валидирует session-токен против БД, вешает TokenMiddleware наtoken/get, закрывает query-token elevation, проверяетisPasswordResetDeniedна reset, дропает небезопасные части sortby до inject menuindex/options, инвалидирует facet/page ACL cache при смене групп. Для member-запросов страница получаетcacheable=0, чтобы HTML не попал в общий ключ кэша ресурса.Тип изменений
Связанные Issues
Closes #755
Как это было протестировано?
composer ci:php/composer test,npm run lint:ci,composer stan/ GitHub Actions CI)Конфигурация тестирования:
fix/issue-755-catalog-acl-tokens(beta)Скриншоты (если применимо)
Не применимо (API / сниппеты / сервисы).
Чеклист
composer stan/ CI jobPHPStan)npm run lint:ciдля Vue)Дополнительные заметки
[[!ms3_products]]/[[!ms3_gallery]]при member RG ACL (описано в setting lexicon).