Repository navigation
Sort.php и Multiple.php в галерее не проверяют права #660
Description
Проблема
Два процессора галереи не задают $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 недостижимо, но починка тривиальная — добавить два ключа на двух языках.
Критерии приёмки
-
Sort.phpиMultiple.phpзадают осмысленное$permission - Менеджер без соответствующего права получает отказ, а не молчаливое выполнение
- Ключи
ms3_gallery_err_nsиms3_gallery_err_no_productдобавлены вruиen - Есть тест, что ни один процессор в
src/Processors/не остался без$permission
Activity