Skip to content

Sort.php и Multiple.php в галерее не проверяют права #660

Description

@biz87

Проблема

Два процессора галереи не задают $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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingpriority: mediumСредний приоритет

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions