diff --git a/_build/data/permissions/transport.policy.tpl.administrator.php b/_build/data/permissions/transport.policy.tpl.administrator.php index 333dff8762e..e4d63610692 100644 --- a/_build/data/permissions/transport.policy.tpl.administrator.php +++ b/_build/data/permissions/transport.policy.tpl.administrator.php @@ -10,21 +10,11 @@ use MODX\Revolution\modAccessPermission; $permissions = []; -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'about', - 'description' => 'perm.about_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'access_permissions', 'description' => 'perm.access_permissions_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'actions', - 'description' => 'perm.actions_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'change_password', 'description' => 'perm.change_password_desc', @@ -65,11 +55,6 @@ 'description' => 'perm.create_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'credits', - 'description' => 'perm.credits_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'customize_forms', 'description' => 'perm.customize_forms_desc', @@ -285,11 +270,6 @@ 'description' => 'perm.error_log_view_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'export_static', - 'description' => 'perm.export_static_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'file_create', 'description' => 'perm.file_create_desc', @@ -380,11 +360,6 @@ 'description' => 'perm.load_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'logout', - 'description' => 'perm.logout_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'mgr_log_view', 'description' => 'perm.mgr_log_view_desc', @@ -396,23 +371,23 @@ 'value' => true, ]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'menu_reports', - 'description' => 'perm.menu_reports_desc', + 'name' => 'menu_access', + 'description' => 'perm.menu_access_desc', 'value' => true, ]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'menu_security', - 'description' => 'perm.menu_security_desc', + 'name' => 'menu_media', + 'description' => 'perm.menu_media_desc', 'value' => true, ]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'menu_site', - 'description' => 'perm.menu_site_desc', + 'name' => 'menu_reports', + 'description' => 'perm.menu_reports_desc', 'value' => true, ]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'menu_support', - 'description' => 'perm.menu_support_desc', + 'name' => 'menu_site', + 'description' => 'perm.menu_site_desc', 'value' => true, ]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ @@ -420,11 +395,6 @@ 'description' => 'perm.menu_system_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'menu_tools', - 'description' => 'perm.menu_tools_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'menu_trash', 'description' => 'perm.menu_trash_desc', @@ -865,11 +835,6 @@ 'description' => 'perm.view_element_desc', 'value' => true, ]); -$permissions[] = $xpdo->newObject(modAccessPermission::class, [ - 'name' => 'view_eventlog', - 'description' => 'perm.view_eventlog_desc', - 'value' => true, -]); $permissions[] = $xpdo->newObject(modAccessPermission::class, [ 'name' => 'view_offline', 'description' => 'perm.view_offline_desc', diff --git a/_build/data/transport.core.accesspolicies.php b/_build/data/transport.core.accesspolicies.php index f63910e0fa3..94240859023 100644 --- a/_build/data/transport.core.accesspolicies.php +++ b/_build/data/transport.core.accesspolicies.php @@ -23,15 +23,15 @@ function jsonifyPermissions(array $permissions = []) { $corePermissions = [ modAccessPolicy::POLICY_RESOURCE => ['add_children', 'create', 'copy', 'delete', 'list', 'load', 'move', 'publish', 'remove', 'save', 'steal_lock', 'undelete', 'unpublish', 'view'], - modAccessPolicy::POLICY_ADMINISTRATOR => ['about', 'access_permissions', 'actions', 'change_password', 'change_profile', 'charsets', 'class_map', 'components', 'content_types', 'countries', 'create', 'credits', 'customize_forms', 'dashboards', 'database', 'database_truncate', 'delete_category', 'delete_chunk', 'delete_context', 'delete_document', 'delete_eventlog', 'delete_plugin', 'delete_propertyset', 'delete_role', 'delete_snippet', 'delete_static_resource', 'delete_symlink', 'delete_template', 'delete_tv', 'delete_user', 'delete_weblink', 'directory_chmod', 'directory_create', 'directory_list', 'directory_remove', 'directory_update', 'edit_category', 'edit_chunk', 'edit_context', 'edit_document', 'edit_locked', 'edit_plugin', 'edit_propertyset', 'edit_role', 'edit_snippet', 'edit_static_resource', 'edit_symlink', 'edit_template', 'edit_tv', 'edit_user', 'edit_weblink', 'element_tree', 'empty_cache', 'error_log_erase', 'error_log_view', 'events', 'export_static', 'file_create', 'file_list', 'file_manager', 'file_remove', 'file_tree', 'file_unpack', 'file_update', 'file_upload', 'file_view', 'flush_sessions', 'frames', 'help', 'home', 'language', 'languages', 'lexicons', 'list', 'load', 'logout', 'mgr_log_view', 'mgr_log_erase', 'menu_reports', 'menu_security', 'menu_site', 'menu_support', 'menu_system', 'menu_tools', 'menu_trash', 'menu_user', 'menus', 'messages', 'namespaces', 'new_category', 'new_chunk', 'new_context', 'new_document', 'new_document_in_root', 'new_plugin', 'new_propertyset', 'new_role', 'new_snippet', 'new_static_resource', 'new_symlink', 'new_template', 'new_tv', 'new_user', 'new_weblink', 'packages', 'policy_delete', 'policy_edit', 'policy_new', 'policy_save', 'policy_template_delete', 'policy_template_edit', 'policy_template_new', 'policy_template_save', 'policy_template_view', 'policy_view', 'property_sets', 'providers', 'publish_document', 'purge_deleted', 'remove', 'remove_locks', 'resource_duplicate', 'resource_quick_create', 'resource_quick_update', 'resource_tree', 'resourcegroup_delete', 'resourcegroup_edit', 'resourcegroup_new', 'resourcegroup_resource_edit', 'resourcegroup_resource_list', 'resourcegroup_save', 'resourcegroup_view', 'save', 'save_category', 'save_chunk', 'save_context', 'save_document', 'save_plugin', 'save_propertyset', 'save_role', 'save_snippet', 'save_template', 'save_tv', 'save_user', 'search', 'set_sudo', 'settings', 'source_delete', 'source_edit', 'source_save', 'source_view', 'sources', 'steal_locks', 'tree_show_element_ids', 'tree_show_resource_ids', 'undelete_document', 'unlock_element_properties', 'unpublish_document', 'usergroup_delete', 'usergroup_edit', 'usergroup_new', 'usergroup_save', 'usergroup_user_edit', 'usergroup_user_list', 'usergroup_view', 'view', 'view_category', 'view_chunk', 'view_context', 'view_document', 'view_element', 'view_eventlog', 'view_offline', 'view_plugin', 'view_propertyset', 'view_role', 'view_snippet', 'view_sysinfo', 'view_template', 'view_tv', 'view_unpublished', 'view_user', 'workspaces'], + modAccessPolicy::POLICY_ADMINISTRATOR => ['access_permissions', 'change_password', 'change_profile', 'charsets', 'class_map', 'components', 'content_types', 'countries', 'create', 'customize_forms', 'dashboards', 'database', 'database_truncate', 'delete_category', 'delete_chunk', 'delete_context', 'delete_document', 'delete_eventlog', 'delete_plugin', 'delete_propertyset', 'delete_role', 'delete_snippet', 'delete_static_resource', 'delete_symlink', 'delete_template', 'delete_tv', 'delete_user', 'delete_weblink', 'directory_chmod', 'directory_create', 'directory_list', 'directory_remove', 'directory_update', 'edit_category', 'edit_chunk', 'edit_context', 'edit_document', 'edit_locked', 'edit_plugin', 'edit_propertyset', 'edit_role', 'edit_snippet', 'edit_static_resource', 'edit_symlink', 'edit_template', 'edit_tv', 'edit_user', 'edit_weblink', 'element_tree', 'empty_cache', 'error_log_erase', 'error_log_view', 'events', 'file_create', 'file_list', 'file_manager', 'file_remove', 'file_tree', 'file_unpack', 'file_update', 'file_upload', 'file_view', 'flush_sessions', 'frames', 'help', 'home', 'language', 'languages', 'lexicons', 'list', 'load', 'mgr_log_view', 'mgr_log_erase', 'menu_access', 'menu_media', 'menu_reports', 'menu_site', 'menu_system', 'menu_trash', 'menu_user', 'menus', 'messages', 'namespaces', 'new_category', 'new_chunk', 'new_context', 'new_document', 'new_document_in_root', 'new_plugin', 'new_propertyset', 'new_role', 'new_snippet', 'new_static_resource', 'new_symlink', 'new_template', 'new_tv', 'new_user', 'new_weblink', 'packages', 'policy_delete', 'policy_edit', 'policy_new', 'policy_save', 'policy_template_delete', 'policy_template_edit', 'policy_template_new', 'policy_template_save', 'policy_template_view', 'policy_view', 'property_sets', 'providers', 'publish_document', 'purge_deleted', 'remove', 'remove_locks', 'resource_duplicate', 'resource_quick_create', 'resource_quick_update', 'resource_tree', 'resourcegroup_delete', 'resourcegroup_edit', 'resourcegroup_new', 'resourcegroup_resource_edit', 'resourcegroup_resource_list', 'resourcegroup_save', 'resourcegroup_view', 'save', 'save_category', 'save_chunk', 'save_context', 'save_document', 'save_plugin', 'save_propertyset', 'save_role', 'save_snippet', 'save_template', 'save_tv', 'save_user', 'search', 'set_sudo', 'settings', 'source_delete', 'source_edit', 'source_save', 'source_view', 'sources', 'steal_locks', 'tree_show_element_ids', 'tree_show_resource_ids', 'undelete_document', 'unlock_element_properties', 'unpublish_document', 'usergroup_delete', 'usergroup_edit', 'usergroup_new', 'usergroup_save', 'usergroup_user_edit', 'usergroup_user_list', 'usergroup_view', 'view', 'view_category', 'view_chunk', 'view_context', 'view_document', 'view_element', 'view_offline', 'view_plugin', 'view_propertyset', 'view_role', 'view_snippet', 'view_sysinfo', 'view_template', 'view_tv', 'view_unpublished', 'view_user', 'workspaces'], modAccessPolicy::POLICY_LOAD_ONLY => ['load'], modAccessPolicy::POLICY_LOAD_LIST_VIEW => ['load', 'list', 'view'], modAccessPolicy::POLICY_OBJECT => ['load', 'list', 'view', 'save', 'remove'], modAccessPolicy::POLICY_ELEMENT => ['add_children', 'create', 'delete', 'list', 'load', 'remove', 'save', 'view', 'copy'], - modAccessPolicy::POLICY_CONTENT_EDITOR => ['change_profile', 'class_map', 'countries', 'delete_document', 'delete_static_resource', 'delete_symlink', 'delete_weblink', 'edit_document', 'edit_static_resource', 'edit_symlink', 'edit_weblink', 'frames', 'help', 'home', 'language', 'list', 'load', 'logout', 'menu_reports', 'menu_site', 'menu_support', 'menu_tools', 'menu_user', 'new_document', 'new_static_resource', 'new_symlink', 'new_weblink', 'resource_duplicate', 'resource_tree', 'save_document', 'source_view', 'tree_show_resource_ids', 'view', 'view_document', 'view_template'], + modAccessPolicy::POLICY_CONTENT_EDITOR => ['change_profile', 'class_map', 'countries', 'delete_document', 'delete_static_resource', 'delete_symlink', 'delete_weblink', 'edit_document', 'edit_static_resource', 'edit_symlink', 'edit_weblink', 'frames', 'help', 'home', 'language', 'list', 'load', 'menu_reports', 'menu_site', 'menu_user', 'new_document', 'new_static_resource', 'new_symlink', 'new_weblink', 'resource_duplicate', 'resource_tree', 'save_document', 'source_view', 'tree_show_resource_ids', 'view', 'view_document', 'view_template'], modAccessPolicy::POLICY_MEDIA_SOURCE_ADMIN => ['create', 'copy', 'load', 'list', 'save', 'remove', 'view'], modAccessPolicy::POLICY_MEDIA_SOURCE_USER => ['load', 'list', 'view'], - modAccessPolicy::POLICY_DEVELOPER => ['about', 'change_password', 'change_profile', 'charsets', 'class_map', 'components', 'content_types', 'countries', 'create', 'credits', 'customize_forms', 'dashboards', 'database', 'delete_category', 'delete_chunk', 'delete_context', 'delete_document', 'delete_eventlog', 'delete_plugin', 'delete_propertyset', 'delete_role', 'delete_snippet', 'delete_template', 'delete_tv', 'delete_user', 'directory_chmod', 'directory_create', 'directory_list', 'directory_remove', 'directory_update', 'edit_category', 'edit_chunk', 'edit_context', 'edit_document', 'edit_locked', 'edit_plugin', 'edit_propertyset', 'edit_role', 'edit_snippet', 'edit_static_resource', 'edit_symlink', 'edit_template', 'edit_tv', 'edit_user', 'edit_weblink', 'element_tree', 'empty_cache', 'error_log_erase', 'error_log_view', 'export_static', 'file_create', 'file_list', 'file_manager', 'file_remove', 'file_tree', 'file_unpack', 'file_update', 'file_upload', 'file_view', 'frames', 'help', 'home', 'language', 'languages', 'lexicons', 'list', 'load', 'logout', 'mgr_log_view', 'mgr_log_erase', 'menu_reports', 'menu_site', 'menu_support', 'menu_system', 'menu_tools', 'menu_user', 'menus', 'messages', 'namespaces', 'new_category', 'new_chunk', 'new_context', 'new_document', 'new_document_in_root', 'new_plugin', 'new_propertyset', 'new_role', 'new_snippet', 'new_static_resource', 'new_symlink', 'new_template', 'new_tv', 'new_user', 'new_weblink', 'packages', 'property_sets', 'providers', 'publish_document', 'purge_deleted', 'remove', 'resource_duplicate', 'resource_quick_create', 'resource_quick_update', 'resource_tree', 'save', 'save_category', 'save_chunk', 'save_context', 'save_document', 'save_plugin', 'save_propertyset', 'save_snippet', 'save_template', 'save_tv', 'save_user', 'search', 'settings', 'source_delete', 'source_edit', 'source_save', 'source_view', 'sources', 'tree_show_element_ids', 'tree_show_resource_ids', 'undelete_document', 'unlock_element_properties', 'unpublish_document', 'view', 'view_category', 'view_chunk', 'view_context', 'view_document', 'view_element', 'view_eventlog', 'view_offline', 'view_plugin', 'view_propertyset', 'view_role', 'view_snippet', 'view_sysinfo', 'view_template', 'view_tv', 'view_unpublished', 'view_user', 'workspaces'], + modAccessPolicy::POLICY_DEVELOPER => ['change_password', 'change_profile', 'charsets', 'class_map', 'components', 'content_types', 'countries', 'create', 'customize_forms', 'dashboards', 'database', 'delete_category', 'delete_chunk', 'delete_context', 'delete_document', 'delete_eventlog', 'delete_plugin', 'delete_propertyset', 'delete_role', 'delete_snippet', 'delete_template', 'delete_tv', 'delete_user', 'directory_chmod', 'directory_create', 'directory_list', 'directory_remove', 'directory_update', 'edit_category', 'edit_chunk', 'edit_context', 'edit_document', 'edit_locked', 'edit_plugin', 'edit_propertyset', 'edit_role', 'edit_snippet', 'edit_static_resource', 'edit_symlink', 'edit_template', 'edit_tv', 'edit_user', 'edit_weblink', 'element_tree', 'empty_cache', 'error_log_erase', 'error_log_view', 'file_create', 'file_list', 'file_manager', 'file_remove', 'file_tree', 'file_unpack', 'file_update', 'file_upload', 'file_view', 'frames', 'help', 'home', 'language', 'languages', 'lexicons', 'list', 'load', 'mgr_log_view', 'mgr_log_erase', 'menu_access', 'menu_media', 'menu_reports', 'menu_site', 'menu_system', 'menu_user', 'menus', 'messages', 'namespaces', 'new_category', 'new_chunk', 'new_context', 'new_document', 'new_document_in_root', 'new_plugin', 'new_propertyset', 'new_role', 'new_snippet', 'new_static_resource', 'new_symlink', 'new_template', 'new_tv', 'new_user', 'new_weblink', 'packages', 'property_sets', 'providers', 'publish_document', 'purge_deleted', 'remove', 'resource_duplicate', 'resource_quick_create', 'resource_quick_update', 'resource_tree', 'save', 'save_category', 'save_chunk', 'save_context', 'save_document', 'save_plugin', 'save_propertyset', 'save_snippet', 'save_template', 'save_tv', 'save_user', 'search', 'settings', 'source_delete', 'source_edit', 'source_save', 'source_view', 'sources', 'tree_show_element_ids', 'tree_show_resource_ids', 'undelete_document', 'unlock_element_properties', 'unpublish_document', 'view', 'view_category', 'view_chunk', 'view_context', 'view_document', 'view_element', 'view_offline', 'view_plugin', 'view_propertyset', 'view_role', 'view_snippet', 'view_sysinfo', 'view_template', 'view_tv', 'view_unpublished', 'view_user', 'workspaces'], modAccessPolicy::POLICY_CONTEXT => ['load', 'list', 'view', 'save', 'remove', 'copy', 'view_unpublished'], modAccessPolicy::POLICY_HIDDEN_NAMESPACE => ['load' => false, 'list' => false, 'view' => true], ]; diff --git a/_build/data/transport.core.menus.php b/_build/data/transport.core.menus.php index a7ee89b2078..4fe3da7b3f7 100644 --- a/_build/data/transport.core.menus.php +++ b/_build/data/transport.core.menus.php @@ -94,7 +94,7 @@ [ 'text' => 'media', 'description' => '', - 'permissions' => 'file_manager', + 'permissions' => 'menu_media', 'action' => '', 'icon' => '', 'children' => [ @@ -174,7 +174,7 @@ [ 'text' => 'logout', 'description' => 'logout_desc', - 'permissions' => 'logout', + 'permissions' => '', 'action' => 'security/logout', 'handler' => 'MODx.logout(); return false;', ], @@ -186,7 +186,7 @@ [ 'text' => 'access', 'description' => '', - 'permissions' => 'access_permissions', + 'permissions' => 'menu_access', 'action' => '', 'icon' => '', 'children' => [ @@ -260,7 +260,7 @@ [ 'text' => 'admin', 'description' => '', - 'permissions' => 'settings', + 'permissions' => 'menu_system', 'action' => '', 'icon' => '', 'children' => [ @@ -292,7 +292,7 @@ [ 'text' => 'edit_menu', 'description' => 'edit_menu_desc', - 'permissions' => 'actions', + 'permissions' => 'menus', 'action' => 'system/action', ], // endregion @@ -355,7 +355,7 @@ [ 'text' => 'eventlog_viewer', 'description' => 'eventlog_viewer_desc', - 'permissions' => 'view_eventlog', + 'permissions' => 'error_log_view', 'action' => 'system/event', ], // endregion diff --git a/_build/test/Tests/Controllers/TopMenuAccessPolicyTest.php b/_build/test/Tests/Controllers/TopMenuAccessPolicyTest.php new file mode 100644 index 00000000000..53b9db3567e --- /dev/null +++ b/_build/test/Tests/Controllers/TopMenuAccessPolicyTest.php @@ -0,0 +1,234 @@ + 'menu_media', + 'access' => 'menu_access', + 'admin' => 'menu_system', + ]; + + private const MENUS_TRANSPORT = '_build/data/transport.core.menus.php'; + + private const ADMIN_TEMPLATE = '_build/data/permissions/transport.policy.tpl.administrator.php'; + + private const CORE_POLICIES = '_build/data/transport.core.accesspolicies.php'; + + private const UPGRADE_SCRIPT = 'setup/includes/upgrades/common/3.3.0-top-menu-access-policy.php'; + + private const UPGRADE_FUNCTIONS = 'setup/includes/upgrades/common/3.3.0-top-menu-access-policy.functions.php'; + + private function buildData(string $relativePath): string + { + return file_get_contents(MODX_BASE_PATH . $relativePath); + } + + private function loadUpgradeHelpers(): void + { + require_once MODX_BASE_PATH . self::UPGRADE_FUNCTIONS; + } + + private function assertMenuPermission(string $menuText, string $permission): void + { + $pattern = sprintf( + "/'text'\\s*=>\\s*'%s'[\\s\\S]*?'permissions'\\s*=>\\s*'%s'/", + $menuText, + $permission + ); + $this->assertMatchesRegularExpression($pattern, $this->buildData(self::MENUS_TRANSPORT)); + } + + private function assertMenuNotPermission(string $menuText, string $permission): void + { + $pattern = sprintf( + "/'text'\\s*=>\\s*'%s'[\\s\\S]*?'permissions'\\s*=>\\s*'%s'/", + $menuText, + $permission + ); + $this->assertDoesNotMatchRegularExpression($pattern, $this->buildData(self::MENUS_TRANSPORT)); + } + + public function testErrorLogMenuUsesErrorLogView() + { + $this->assertMenuPermission('eventlog_viewer', 'error_log_view'); + $this->assertMenuNotPermission('eventlog_viewer', 'view_eventlog'); + } + + public function testErrorLogControllerUsesErrorLogView() + { + $controller = file_get_contents(MODX_MANAGER_PATH . 'controllers/default/system/event.class.php'); + $this->assertStringContainsString("hasPermission('error_log_view')", $controller); + $this->assertStringNotContainsString("hasPermission('view_eventlog')", $controller); + } + + public function testMenusMenuAndControllerUseMenusPermission() + { + $this->assertMenuPermission('edit_menu', 'menus'); + $this->assertMenuNotPermission('edit_menu', 'actions'); + + $controller = file_get_contents(MODX_MANAGER_PATH . 'controllers/default/system/action.class.php'); + $this->assertStringContainsString("hasPermission('menus')", $controller); + $this->assertStringNotContainsString("hasPermission('actions')", $controller); + } + + public function testMenuProcessorsUseMenusPermission() + { + $processorDir = MODX_CORE_PATH . 'src/Revolution/Processors/System/Menu/'; + foreach (['Create.php', 'GetList.php', 'GetNodes.php', 'Remove.php', 'Sort.php', 'Update.php'] as $file) { + $path = $processorDir . $file; + $this->assertFileExists($path); + $contents = file_get_contents($path); + $this->assertTrue( + str_contains($contents, "hasPermission('menus')") || str_contains($contents, "\$permission = 'menus'"), + $file . ' must gate on menus' + ); + } + } + + public function testLogoutMenuHasNoPermissionGate() + { + $this->assertMenuPermission('logout', ''); + } + + public function testParentMenusUseDedicatedKeys() + { + foreach (self::PARENT_KEYS as $menuText => $permission) { + $this->assertMenuPermission($menuText, $permission); + } + $this->assertMenuPermission('file_browser', 'file_manager'); + $this->assertMenuPermission('system_settings', 'settings'); + $this->assertMenuPermission('acls', 'access_permissions'); + } + + public function testRetiredKeysRemovedFromAdministratorTemplate() + { + $template = $this->buildData(self::ADMIN_TEMPLATE); + foreach (self::RETIRED_KEYS as $key) { + $this->assertDoesNotMatchRegularExpression( + "/'name'\\s*=>\\s*'" . preg_quote($key, '/') . "'/", + $template, + $key . ' must be removed from AdministratorTemplate' + ); + } + foreach (['menus', 'error_log_view', 'menu_media', 'menu_access', 'menu_system'] as $key) { + $this->assertMatchesRegularExpression( + "/'name'\\s*=>\\s*'" . preg_quote($key, '/') . "'/", + $template + ); + } + } + + public function testRetiredKeysRemovedFromCorePolicyData() + { + $policies = $this->buildData(self::CORE_POLICIES); + foreach (self::RETIRED_KEYS as $key) { + $this->assertStringNotContainsString( + "'" . $key . "'", + $policies, + $key . ' must be removed from core policy data' + ); + } + foreach (['menus', 'error_log_view', 'menu_media', 'menu_access', 'menu_system'] as $key) { + $this->assertStringContainsString("'" . $key . "'", $policies); + } + } + + public function testMgrLogViewLexiconDocumentsReportsMenu() + { + $lexicon = file_get_contents(MODX_CORE_PATH . 'lexicon/en/permissions.inc.php'); + $this->assertStringContainsString( + "\$_lang['perm.mgr_log_view_desc'] = 'To view Manager actions under Reports (system/logs).';", + $lexicon + ); + $this->assertStringContainsString("perm.menu_media_desc", $lexicon); + $this->assertStringContainsString("perm.menu_access_desc", $lexicon); + } + + public function testUpgradeHelpersReplaceCompositeMenuPermissions() + { + $this->loadUpgradeHelpers(); + $this->assertSame( + 'error_log_view,custom_extra', + modxUpgrade330TopMenuReplacePermissionToken('view_eventlog,custom_extra', 'view_eventlog', 'error_log_view') + ); + $this->assertSame( + 'menu_media', + modxUpgrade330TopMenuReplacePermissionToken('file_manager', 'file_manager', 'menu_media') + ); + $this->assertSame( + '', + modxUpgrade330TopMenuReplacePermissionToken('logout', 'logout', '') + ); + } + + public function testUpgradeHelpersGrantParentKeysWithoutElevatingPageKeys() + { + $this->loadUpgradeHelpers(); + $migrated = modxUpgrade330TopMenuMigratePolicyData([ + 'file_manager' => true, + 'access_permissions' => true, + 'settings' => true, + 'view_eventlog' => true, + 'actions' => true, + 'about' => true, + 'menus' => false, + 'error_log_view' => false, + ]); + $this->assertTrue($migrated['menu_media']); + $this->assertTrue($migrated['menu_access']); + $this->assertTrue($migrated['menu_system']); + $this->assertTrue($migrated['file_manager']); + $this->assertTrue($migrated['access_permissions']); + $this->assertTrue($migrated['settings']); + $this->assertFalse($migrated['menus']); + $this->assertFalse($migrated['error_log_view']); + $this->assertArrayNotHasKey('view_eventlog', $migrated); + $this->assertArrayNotHasKey('actions', $migrated); + $this->assertArrayNotHasKey('about', $migrated); + } + + public function testUpgradeHelpersRespectExplicitParentDeny() + { + $this->loadUpgradeHelpers(); + $migrated = modxUpgrade330TopMenuMigratePolicyData([ + 'file_manager' => true, + 'menu_media' => false, + ]); + $this->assertFalse($migrated['menu_media']); + $this->assertTrue($migrated['file_manager']); + } + + public function testUpgradeScriptWiredInMysqlRunner() + { + $this->assertFileExists(MODX_BASE_PATH . self::UPGRADE_SCRIPT); + $this->assertFileExists(MODX_BASE_PATH . self::UPGRADE_FUNCTIONS); + $this->assertFileExists(MODX_BASE_PATH . 'setup/includes/upgrades/mysql/3.3.0-pl.php'); + $pl = file_get_contents(MODX_BASE_PATH . 'setup/includes/upgrades/mysql/3.3.0-pl.php'); + $this->assertStringContainsString('3.3.0-top-menu-access-policy.php', $pl); + $runner = file_get_contents(MODX_BASE_PATH . self::UPGRADE_SCRIPT); + $this->assertStringContainsString('3.3.0-top-menu-access-policy.functions.php', $runner); + $this->assertStringContainsString('modxUpgrade330TopMenuAccessPolicy($modx)', $runner); + } +} diff --git a/core/lexicon/en/permissions.inc.php b/core/lexicon/en/permissions.inc.php index 43d2ae252d5..e612258a24f 100644 --- a/core/lexicon/en/permissions.inc.php +++ b/core/lexicon/en/permissions.inc.php @@ -6,9 +6,7 @@ * @package modx * @subpackage lexicon */ -$_lang['perm.about_desc'] = 'The About page.'; -$_lang['perm.access_permissions_desc'] = 'Any Access Permission-related pages and actions.'; -$_lang['perm.actions_desc'] = 'The Actions page.'; +$_lang['perm.access_permissions_desc'] = 'Pages and actions under Access that use this key (Resource Groups, ACLs, Flush Permissions). The Access parent menu uses menu_access.'; $_lang['perm.add_children_desc'] = 'To add any Resources as children of the specified Resource or Elements to a Category.'; $_lang['perm.change_password_desc'] = 'User can change their user password.'; $_lang['perm.change_profile_desc'] = 'User can change their profile.'; @@ -19,7 +17,6 @@ $_lang['perm.copy_desc'] = 'The ability to copy an object.'; $_lang['perm.countries_desc'] = 'To view a list of countries.'; $_lang['perm.create_desc'] = 'Basic "create" access on new objects.'; -$_lang['perm.credits_desc'] = 'View the Credits page.'; $_lang['perm.customize_forms_desc'] = 'View and manage the Form Customization page.'; $_lang['perm.dashboards_desc'] = 'View and manage Custom Dashboards.'; $_lang['perm.database_desc'] = 'The System Info page.'; @@ -63,11 +60,10 @@ $_lang['perm.element_tree_desc'] = 'The ability to view the Elements Tree on the left nav.'; $_lang['perm.empty_cache_desc'] = 'To empty the site cache.'; $_lang['perm.error_log_erase_desc'] = 'To erase the error log.'; -$_lang['perm.error_log_view_desc'] = 'To view the error log.'; -$_lang['perm.export_static_desc'] = 'To export the site to static HTML.'; +$_lang['perm.error_log_view_desc'] = 'To view the Error Log under Reports (menu item, system/event page, and ErrorLog processors).'; $_lang['perm.file_create_desc'] = 'To create a file.'; $_lang['perm.file_list_desc'] = 'To list files within a given physical directory.'; -$_lang['perm.file_manager_desc'] = 'To use the file manager utility.'; +$_lang['perm.file_manager_desc'] = 'To use the Media Browser (and related file manager actions). The Media parent menu uses menu_media.'; $_lang['perm.file_remove_desc'] = 'To delete physical files.'; $_lang['perm.file_tree_desc'] = 'To view the Files Tree on the left nav.'; $_lang['perm.file_update_desc'] = 'To edit the content of physical files. WARNING: grants ability to execute arbitrary server-side code.'; @@ -83,18 +79,16 @@ $_lang['perm.lexicons_desc'] = 'To edit or view Lexicons and Internationalization.'; $_lang['perm.list_desc'] = 'Basic permission to "list" any object. List means to get a collection of objects.'; $_lang['perm.load_desc'] = 'Basic permission to "load" any object, or be able to return it as an instance at all.'; -$_lang['perm.logout_desc'] = 'To be able to logout as a user.'; -$_lang['perm.mgr_log_view_desc'] = 'To view the manager action log.'; +$_lang['perm.mgr_log_view_desc'] = 'To view Manager actions under Reports (system/logs).'; $_lang['perm.mgr_log_erase_desc'] = 'To clear the manager action log.'; +$_lang['perm.menu_access_desc'] = 'Show the main menu item "Access".'; +$_lang['perm.menu_media_desc'] = 'Show the main menu item "Media".'; $_lang['perm.menu_reports_desc'] = 'Show the main menu item "Reports".'; -$_lang['perm.menu_security_desc'] = 'Show the main menu item "Security".'; -$_lang['perm.menu_site_desc'] = 'Show the main menu item "Site".'; -$_lang['perm.menu_support_desc'] = 'Show the main menu item "Support".'; -$_lang['perm.menu_system_desc'] = 'Show the main menu item "System".'; -$_lang['perm.menu_tools_desc'] = 'Show the main menu item "Tools".'; +$_lang['perm.menu_site_desc'] = 'Show the main menu item "Content".'; +$_lang['perm.menu_system_desc'] = 'Show the main menu item "Gear" (System). Does not grant the System Settings page; that uses settings.'; $_lang['perm.menu_trash_desc'] = 'Show the main menu item "Trash Manager".'; $_lang['perm.menu_user_desc'] = 'Show the main menu item "User".'; -$_lang['perm.menus_desc'] = 'To edit or save any main Menu items.'; +$_lang['perm.menus_desc'] = 'To view and manage Gear → Menus (system/action page and Menu processors).'; $_lang['perm.messages_desc'] = 'To send or view any personal Messages.'; $_lang['perm.move_desc'] = 'Basic "move" access on any object.'; $_lang['perm.namespaces_desc'] = 'To edit or view Namespaces.'; @@ -156,7 +150,7 @@ $_lang['perm.save_user_desc'] = 'To save any Users.'; $_lang['perm.search_desc'] = 'To use the Search page.'; $_lang['perm.set_sudo_desc'] = 'To make any User sudo.'; -$_lang['perm.settings_desc'] = 'To view and edit any System Settings. WARNING: secrets such as API keys are commonly stored in System Settings. User may change uploadable file types, allowing execution of arbitrary code.'; +$_lang['perm.settings_desc'] = 'To view and edit System Settings. The Gear parent menu uses menu_system. WARNING: secrets such as API keys are commonly stored in System Settings. User may change uploadable file types, allowing execution of arbitrary code.'; $_lang['perm.events_desc'] = 'To view any System Events.'; $_lang['perm.source_delete_desc'] = 'To delete a Media Source.'; $_lang['perm.source_edit_desc'] = 'To edit a Media Source.'; @@ -185,7 +179,6 @@ $_lang['perm.view_context_desc'] = 'To view any Contexts.'; $_lang['perm.view_document_desc'] = 'To view any Resources.'; $_lang['perm.view_element_desc'] = 'To get a list of Elements or Element classes.'; -$_lang['perm.view_eventlog_desc'] = 'To view the Event Log.'; $_lang['perm.view_offline_desc'] = 'To be able to view the site when it is in offline status.'; $_lang['perm.view_plugin_desc'] = 'To view any Plugins.'; $_lang['perm.view_propertyset_desc'] = 'To view any Property Sets.'; diff --git a/manager/controllers/default/system/action.class.php b/manager/controllers/default/system/action.class.php index 65fc0a2d7b9..efbea935bb6 100644 --- a/manager/controllers/default/system/action.class.php +++ b/manager/controllers/default/system/action.class.php @@ -1,4 +1,5 @@ modx->hasPermission('actions'); + return $this->modx->hasPermission('menus'); } /** diff --git a/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.functions.php b/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.functions.php new file mode 100644 index 00000000000..4361e425e14 --- /dev/null +++ b/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.functions.php @@ -0,0 +1,182 @@ + $token) { + if ($token !== $from) { + continue; + } + if ($to === '') { + unset($tokens[$index]); + } else { + $tokens[$index] = $to; + } + $replaced = true; + } + if (!$replaced) { + return $permissions; + } + + return implode(',', array_values($tokens)); +} + +/** + * @param array $data + * @return array + */ +function modxUpgrade330TopMenuMigratePolicyData(array $data): array +{ + // Parent keys: preserve visibility for users who had the old shared child/page key. + $parentGrants = [ + 'menu_media' => 'file_manager', + 'menu_access' => 'access_permissions', + 'menu_system' => 'settings', + ]; + foreach ($parentGrants as $parentKey => $legacyKey) { + if (!empty($data[$legacyKey]) && !array_key_exists($parentKey, $data)) { + $data[$parentKey] = true; + } + } + + foreach ( + [ + 'view_eventlog', + 'actions', + 'logout', + 'about', + 'credits', + 'export_static', + 'menu_security', + 'menu_support', + 'menu_tools', + ] as $key + ) { + unset($data[$key]); + } + + return $data; +} + +/** + * @param list $definitions + */ +function modxUpgrade330TopMenuEnsureTemplatePermissions(modX $modx, array $definitions): void +{ + /** @var modAccessPolicyTemplate|null $adminTemplate */ + $adminTemplate = $modx->getObject(modAccessPolicyTemplate::class, [ + 'name' => 'AdministratorTemplate', + ]); + if (!$adminTemplate instanceof modAccessPolicyTemplate) { + return; + } + $templateId = (int)$adminTemplate->get('id'); + foreach ($definitions as $definition) { + $existing = $modx->getObject(modAccessPermission::class, [ + 'template' => $templateId, + 'name' => $definition['name'], + ]); + if ($existing instanceof modAccessPermission) { + continue; + } + $permission = $modx->newObject(modAccessPermission::class); + $permission->fromArray([ + 'template' => $templateId, + 'name' => $definition['name'], + 'description' => $definition['description'], + 'value' => true, + ], '', true, true); + $permission->save(); + } +} + +function modxUpgrade330TopMenuAccessPolicy(modX $modx): void +{ + $migrations = [ + ['text' => 'eventlog_viewer', 'from' => 'view_eventlog', 'to' => 'error_log_view'], + ['text' => 'edit_menu', 'from' => 'actions', 'to' => 'menus'], + ['text' => 'logout', 'from' => 'logout', 'to' => ''], + ['text' => 'media', 'from' => 'file_manager', 'to' => 'menu_media'], + ['text' => 'access', 'from' => 'access_permissions', 'to' => 'menu_access'], + ['text' => 'admin', 'from' => 'settings', 'to' => 'menu_system'], + ]; + + foreach ($migrations as $migration) { + /** @var modMenu|null $menu */ + $menu = $modx->getObject(modMenu::class, ['text' => $migration['text']]); + if (!$menu instanceof modMenu) { + continue; + } + $current = (string)$menu->get('permissions'); + $updated = modxUpgrade330TopMenuReplacePermissionToken( + $current, + $migration['from'], + $migration['to'] + ); + if ($updated === $current) { + continue; + } + $menu->set('permissions', $updated); + $menu->save(); + } + + modxUpgrade330TopMenuEnsureTemplatePermissions($modx, [ + ['name' => 'menu_media', 'description' => 'perm.menu_media_desc'], + ['name' => 'menu_access', 'description' => 'perm.menu_access_desc'], + ]); + + /** @var modAccessPolicy[] $policies */ + $policies = $modx->getCollection(modAccessPolicy::class); + foreach ($policies as $policy) { + $data = $policy->get('data'); + if (!is_array($data)) { + continue; + } + $migrated = modxUpgrade330TopMenuMigratePolicyData($data); + if ($migrated === $data) { + continue; + } + $policy->set('data', $migrated); + $policy->save(); + } + + $retiredKeys = [ + 'view_eventlog', + 'actions', + 'logout', + 'about', + 'credits', + 'export_static', + 'menu_security', + 'menu_support', + 'menu_tools', + ]; + /** @var modAccessPermission[] $permissions */ + $permissions = $modx->getCollection(modAccessPermission::class, [ + 'name:IN' => $retiredKeys, + ]); + foreach ($permissions as $permission) { + $permission->remove(); + } +} diff --git a/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.php b/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.php new file mode 100644 index 00000000000..9f42e4628cb --- /dev/null +++ b/setup/includes/upgrades/common/3.3.0-top-menu-access-policy.php @@ -0,0 +1,16 @@ +