Проблема
Два процессора галереи не задают $permission и потому доступны любому аутентифицированному менеджеру, независимо от назначенных ему прав.
MODX\Revolution\Processors\ModelProcessor::checkPermissions():
return !empty($this->permission) ? $this->modx->hasPermission($this->permission) : true;
Пустое значение означает «пропустить всех». Текущая картина по каталогу src/Processors/Gallery/:
| Процессор |
$permission |
Generate.php |
msproductfile_generate |
GenerateAll.php |
msproductfile_generate |
GetList.php |
msproductfile_list |
Remove.php |
msproductfile_save |
RemoveAll.php |
msproductfile_save |
SetPreview.php |
msproductfile_save |
Update.php |
msproductfile_save |
Upload.php |
msproductfile_save |
Sort.php |
не задан |
Multiple.php |
не задан |
Sort.php — это drag-and-drop сортировка галереи из интерфейса менеджера. То есть пользователь, которому не выдано msproductfile_save, не может загрузить, удалить или изменить изображение, но может переставить порядок изображений в галерее любого товара. Это несогласованно и, судя по всему, недосмотр, а не решение: все восемь соседних процессоров права проверяют.
Периметр ограничен: попасть на процессор можно только через MODX-коннектор с активной manager-сессией, анонимный доступ исключён. То есть это не внешняя дыра, а отсутствие разграничения внутри менеджера — существенно для магазинов, где контент-редакторам выдают урезанный набор прав.
Как обнаружено
При ревью PR #643, который добавляет SortByName.php. Новый процессор как раз задаёт msproductfile_save явно — то есть сделан строже существующих соседей. Расхождение всплыло при сверке.
Предлагаемое решение
Проставить msproductfile_save обоим — это то же право, что у остальных изменяющих операций галереи, и оно уже объявлено в _build/elements/policies.php и policyTemplates.php.
Перед этим стоит проверить, что за операцию выполняет Multiple.php, и подобрать право по смыслу (если это массовая загрузка — msproductfile_save, если генерация превью — msproductfile_generate).
Заодно имеет смысл добавить тест, проверяющий, что у каждого процессора в src/Processors/ задан $permission — иначе следующий забытый процессор всплывёт так же случайно.
Сопутствующее: отсутствующие лексиконы
Обнаружено там же. Ключи ms3_gallery_err_ns и ms3_gallery_err_no_product используются в SortByName.php, Generate.php, GenerateAll.php, RemoveAll.php, SetPreview.php, Update.php и Upload.php, но не существуют ни в ru, ни в en. modLexicon::process() при отсутствии ключа возвращает сам ключ, поэтому пользователь при ошибке увидит ms3_gallery_err_ns вместо человекочитаемого текста.
Триггерится на edge-case (невалидный product_id), через обычный UI недостижимо, но починка тривиальная — добавить два ключа на двух языках.
Критерии приёмки
Проблема
Два процессора галереи не задают
$permissionи потому доступны любому аутентифицированному менеджеру, независимо от назначенных ему прав.MODX\Revolution\Processors\ModelProcessor::checkPermissions():Пустое значение означает «пропустить всех». Текущая картина по каталогу
src/Processors/Gallery/:$permissionGenerate.phpmsproductfile_generateGenerateAll.phpmsproductfile_generateGetList.phpmsproductfile_listRemove.phpmsproductfile_saveRemoveAll.phpmsproductfile_saveSetPreview.phpmsproductfile_saveUpdate.phpmsproductfile_saveUpload.phpmsproductfile_saveSort.phpMultiple.phpSort.php— это drag-and-drop сортировка галереи из интерфейса менеджера. То есть пользователь, которому не выданоmsproductfile_save, не может загрузить, удалить или изменить изображение, но может переставить порядок изображений в галерее любого товара. Это несогласованно и, судя по всему, недосмотр, а не решение: все восемь соседних процессоров права проверяют.Периметр ограничен: попасть на процессор можно только через MODX-коннектор с активной manager-сессией, анонимный доступ исключён. То есть это не внешняя дыра, а отсутствие разграничения внутри менеджера — существенно для магазинов, где контент-редакторам выдают урезанный набор прав.
Как обнаружено
При ревью PR #643, который добавляет
SortByName.php. Новый процессор как раз задаётmsproductfile_saveявно — то есть сделан строже существующих соседей. Расхождение всплыло при сверке.Предлагаемое решение
Проставить
msproductfile_saveобоим — это то же право, что у остальных изменяющих операций галереи, и оно уже объявлено в_build/elements/policies.phpиpolicyTemplates.php.Перед этим стоит проверить, что за операцию выполняет
Multiple.php, и подобрать право по смыслу (если это массовая загрузка —msproductfile_save, если генерация превью —msproductfile_generate).Заодно имеет смысл добавить тест, проверяющий, что у каждого процессора в
src/Processors/задан$permission— иначе следующий забытый процессор всплывёт так же случайно.Сопутствующее: отсутствующие лексиконы
Обнаружено там же. Ключи
ms3_gallery_err_nsиms3_gallery_err_no_productиспользуются вSortByName.php,Generate.php,GenerateAll.php,RemoveAll.php,SetPreview.php,Update.phpиUpload.php, но не существуют ни вru, ни вen.modLexicon::process()при отсутствии ключа возвращает сам ключ, поэтому пользователь при ошибке увидитms3_gallery_err_nsвместо человекочитаемого текста.Триггерится на edge-case (невалидный
product_id), через обычный UI недостижимо, но починка тривиальная — добавить два ключа на двух языках.Критерии приёмки
Sort.phpиMultiple.phpзадают осмысленное$permissionms3_gallery_err_nsиms3_gallery_err_no_productдобавлены вruиensrc/Processors/не остался без$permission