fix(acl): Resource permission descriptions and class_map Resource Type UI

_build/test/Tests/Controllers/ResourceAccessPolicyTest.php

@@ -0,0 +1,106 @@
+<?php
+
+namespace MODX\Revolution\Tests\Controllers;
+
+use MODX\Revolution\MODxTestCase;
+
+/**
+ * Tests for Resource access policy accuracy (#14479)
+ *
+ * @package modx-test
+ * @group Controllers
+ */
+class ResourceAccessPolicyTest extends MODxTestCase
+{
+    public function testPublishDocumentLexiconIsPublishOnly()
+    {
+        $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+        $contents = file_get_contents($lexiconFile);
+        $this->assertStringContainsString(
+            "\$_lang['perm.publish_document_desc'] = 'To publish any Resource. Unpublishing uses unpublish_document.';",
+            $contents
+        );
+        $this->assertStringNotContainsString(
+            'To publish or unpublish any Resource.',
+            $contents
+        );
+    }
+
+    public function testEditLockedLexiconDescribesElementsNotResources()
+    {
+        $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+        $contents = file_get_contents($lexiconFile);
+        $this->assertStringContainsString('edit Elements that have the Locked checkbox', $contents);
+        $this->assertStringContainsString('Does not apply to Resource session locks', $contents);
+    }
+
+    public function testDeleteDocumentLexiconDescribesSoftDelete()
+    {
+        $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+        $contents = file_get_contents($lexiconFile);
+        $this->assertStringContainsString('soft-delete any Resource', $contents);
+        $this->assertStringContainsString('requires purge_deleted', $contents);
+    }
+
+    /**
+     * @dataProvider providerClassMapGatedFiles
+     */
+    public function testClassKeyFieldIsGatedOnClassMap(string $relativePath)
+    {
+        $file = MODX_MANAGER_PATH . $relativePath;
+        $this->assertFileExists($file);
+        $contents = file_get_contents($file);
+        $this->assertMatchesRegularExpression(
+            "/xtype:\\s*MODx\\.perm\\.class_map\\s*\\?\\s*'modx-combo-class-derivatives'\\s*:\\s*'hidden'/",
+            $contents,
+            $relativePath . ' must toggle Resource Type combo vs hidden via MODx.perm.class_map'
+        );
+    }
+
+    public function providerClassMapGatedFiles(): array
+    {
+        return [
+            ['assets/modext/widgets/resource/modx.panel.resource.js'],
+            ['assets/modext/widgets/resource/modx.window.resource.js'],
+            ['assets/modext/widgets/resource/modx.tree.resource.js'],
+        ];
+    }
+
+    public function testViewDocumentLexiconDistinguishesFromGetList()
+    {
+        $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+        $contents = file_get_contents($lexiconFile);
+        $this->assertStringContainsString('Resource/Get uses the generic view permission', $contents);
+        $this->assertStringContainsString('per-row object list', $contents);
+    }
+
+    public function testClassMapLexiconIsNotCreateTypeGate()
+    {
+        $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+        $contents = file_get_contents($lexiconFile);
+        $this->assertStringContainsString('Does not grant creating or changing Resource types', $contents);
+    }
+
+    public function testQuickCreateHasSingleClassKeyField()
+    {
+        $file = MODX_MANAGER_PATH . 'assets/modext/widgets/resource/modx.tree.resource.js';
+        $contents = file_get_contents($file);
+        $fnStart = strpos($contents, 'MODx.getQuickCreateResourceSettingsFields');
+        $this->assertNotFalse($fnStart);
+        $fnEnd = strpos($contents, 'MODx.getQRSettings', $fnStart);
+        $this->assertNotFalse($fnEnd);
+        $chunk = substr($contents, $fnStart, $fnEnd - $fnStart);
+        $this->assertStringContainsString('MODx.perm.class_map', $chunk);
+        $this->assertStringNotContainsString(
+            'id: `modx-${id}-class_key`',
+            $chunk,
+            'Pre-existing duplicate hidden class_key field must stay removed'
+        );
+        $this->assertStringContainsString('id: `modx-${id}-class-key`', $chunk);
+        $this->assertSame(
+            1,
+            substr_count($chunk, "name: 'class_key'"),
+            'Quick create settings must define class_key once'
+        );
+    }
+}

core/lexicon/en/permissions.inc.php

@@ -13,7 +13,7 @@
 $_lang['perm.change_password_desc'] = 'User can change their user password.';
 $_lang['perm.change_profile_desc'] = 'User can change their profile.';
 $_lang['perm.charsets_desc'] = 'To view a list of charsets.';
-$_lang['perm.class_map_desc'] = 'To view a list of classes in the Class Map.';
+$_lang['perm.class_map_desc'] = 'To view class lists used by Class Map selectors, including the Resource Type combo. Does not grant creating or changing Resource types; those still need new_document and the matching type key (new_weblink, new_symlink, new_static_resource).';
 $_lang['perm.components_desc'] = 'To view the Extras menu.';
 $_lang['perm.content_types_desc'] = 'The Content Types page.';
 $_lang['perm.copy_desc'] = 'The ability to copy an object.';
@@ -28,10 +28,10 @@
 $_lang['perm.delete_category_desc'] = 'To delete or remove any Categories.';
 $_lang['perm.delete_chunk_desc'] = 'To delete or remove any Chunks.';
 $_lang['perm.delete_context_desc'] = 'To delete or remove any Contexts.';
-$_lang['perm.delete_document_desc'] = 'To delete or remove any Resource.';
-$_lang['perm.delete_weblink_desc'] = 'To delete or remove any weblink, also requires delete_document permission.';
-$_lang['perm.delete_symlink_desc'] = 'To delete or remove any symlink, also requires delete_document permission.';
-$_lang['perm.delete_static_resource_desc'] = 'To delete or remove any static resource, also requires delete_document permission.';
+$_lang['perm.delete_document_desc'] = 'To soft-delete any Resource (mark as deleted). Permanent removal from the database requires purge_deleted.';
+$_lang['perm.delete_weblink_desc'] = 'To soft-delete any weblink; also requires delete_document.';
+$_lang['perm.delete_symlink_desc'] = 'To soft-delete any symlink; also requires delete_document.';
+$_lang['perm.delete_static_resource_desc'] = 'To soft-delete any static resource; also requires delete_document.';
 $_lang['perm.delete_eventlog_desc'] = 'To empty the Event Log.';
 $_lang['perm.delete_plugin_desc'] = 'To delete or remove any Plugins.';
 $_lang['perm.delete_propertyset_desc'] = 'To delete or remove any Property Sets.';
@@ -52,7 +52,7 @@
 $_lang['perm.edit_weblink_desc'] = 'To edit weblinks, also requires edit_document permission.';
 $_lang['perm.edit_symlink_desc'] = 'To edit symlinks, also requires edit_document permission.';
 $_lang['perm.edit_static_resource_desc'] = 'To edit static resources, also requires edit_document permission.';
-$_lang['perm.edit_locked_desc'] = 'Allows a user to override a lock and edit a locked Resource.';
+$_lang['perm.edit_locked_desc'] = 'To edit Elements that have the Locked checkbox set. Does not apply to Resource session locks; those use steal_locks and the Resource object policy steal_lock.';
 $_lang['perm.edit_plugin_desc'] = 'To edit any Plugins. <strong>WARNING: combined with save_plugin permission, grants ability to execute arbitrary server-side code.</strong>';
 $_lang['perm.edit_propertyset_desc'] = 'To edit any Property Sets.';
 $_lang['perm.edit_role_desc'] = 'To edit any Roles.';
@@ -101,18 +101,18 @@
 $_lang['perm.new_category_desc'] = 'To create a new Category.';
 $_lang['perm.new_chunk_desc'] = 'To create a new Chunk.';
 $_lang['perm.new_context_desc'] = 'To create a new Context.';
-$_lang['perm.new_document_desc'] = 'To create a new Resource.';
+$_lang['perm.new_document_desc'] = 'To create a new Resource (Document). Opening Create Resource requires this key; derivative types also need their type key.';
 $_lang['perm.new_document_in_root_desc'] = 'To be able to create a Resource at the root level.';
 $_lang['perm.new_plugin_desc'] = 'To create a new Plugin.';
 $_lang['perm.new_propertyset_desc'] = 'To create a new Property Set.';
 $_lang['perm.new_role_desc'] = 'To create a new Role.';
 $_lang['perm.new_snippet_desc'] = 'To create a new Snippet.';
-$_lang['perm.new_static_resource_desc'] = 'To create a new Static Resource.';
-$_lang['perm.new_symlink_desc'] = 'To create a new SymLink.';
+$_lang['perm.new_static_resource_desc'] = 'To create a new Static Resource. Also requires new_document. Enforced on create and when changing Resource Type.';
+$_lang['perm.new_symlink_desc'] = 'To create a new SymLink. Also requires new_document. Enforced on create and when changing Resource Type.';
 $_lang['perm.new_template_desc'] = 'To create a new Template.';
 $_lang['perm.new_tv_desc'] = 'To create a new TV.';
 $_lang['perm.new_user_desc'] = 'To create a new User.';
-$_lang['perm.new_weblink_desc'] = 'To create a new WebLink.';
+$_lang['perm.new_weblink_desc'] = 'To create a new WebLink. Also requires new_document. Enforced on create and when changing Resource Type.';
 $_lang['perm.packages_desc'] = 'To use any Transport Packages in the Package Management system.';
 $_lang['perm.policy_delete_desc'] = 'To delete an Access Policy.';
 $_lang['perm.policy_edit_desc'] = 'To edit an Access Policy. <strong>WARNING: combined with policy_save permission, grants ability to escalate privileges, including security-related permissions.</strong>';
@@ -127,8 +127,8 @@
 $_lang['perm.property_sets_desc'] = 'To view and edit Properties and Property Sets.';
 $_lang['perm.providers_desc'] = 'To view and edit Providers across the site.';
 $_lang['perm.publish_desc'] = 'Basic "publish" access on any object.';
-$_lang['perm.publish_document_desc'] = 'To publish or unpublish any Resource.';
-$_lang['perm.purge_deleted_desc'] = 'To empty the Recycle Bin.';
+$_lang['perm.publish_document_desc'] = 'To publish any Resource. Unpublishing uses unpublish_document.';
+$_lang['perm.purge_deleted_desc'] = 'To permanently remove soft-deleted Resources from the Recycle Bin.';
 $_lang['perm.remove_desc'] = 'Basic permission to delete any object.';
 $_lang['perm.remove_locks_desc'] = 'To delete all existing Locks throughout the site.';
 $_lang['perm.resourcegroup_delete_desc'] = 'To delete a Resource Group.';
@@ -163,8 +163,8 @@
 $_lang['perm.source_save_desc'] = 'To create or save a Media Source.';
 $_lang['perm.source_view_desc'] = 'To view and list Media Sources.';
 $_lang['perm.sources_desc'] = 'To manage Media Sources and Media Source Types.';
-$_lang['perm.steal_lock_desc'] = 'To "steal", or delete, a lock from a Resource.';
-$_lang['perm.steal_locks_desc'] = 'To "steal" locks, overriding a current lock on a Resource.';
+$_lang['perm.steal_lock_desc'] = 'Resource object policy: allow stealing the session lock on that Resource. Also requires the manager permission steal_locks.';
+$_lang['perm.steal_locks_desc'] = 'Manager permission to steal Resource session locks (another user has the Resource open). Also requires the Resource object policy steal_lock on that Resource.';
 $_lang['perm.tree_show_resource_ids_desc'] = 'Show the IDs in the Resource tree.';
 $_lang['perm.tree_show_element_ids_desc'] = 'Show the IDs in the Elements tree.';
 $_lang['perm.undelete_desc'] = 'Basic permission to "undelete" any object.';
@@ -183,7 +183,7 @@
 $_lang['perm.view_category_desc'] = 'To view any Categories.';
 $_lang['perm.view_chunk_desc'] = 'To view any Chunks.';
 $_lang['perm.view_context_desc'] = 'To view any Contexts.';
-$_lang['perm.view_document_desc'] = 'To view any Resources.';
+$_lang['perm.view_document_desc'] = 'To open Resource Data and related Manager views (resource/data, site_schedule, tree preview). Resource/Get uses the generic view permission (and object view). Resource/GetList uses view plus per-row object list. Neither uses this key.';
 $_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.';

manager/assets/modext/widgets/resource/modx.panel.resource.js

@@ -929,15 +929,17 @@ Ext.extend(MODx.panel.Resource, MODx.FormPanel, {
     },
 
     getSettingLeftFields: function(config) {
+        const classKey = config.record.class_key || 'MODX\\Revolution\\modDocument';
+
         return [{
-            xtype: 'modx-combo-class-derivatives',
+            xtype: MODx.perm.class_map ? 'modx-combo-class-derivatives' : 'hidden',
             fieldLabel: _('resource_type'),
             description: '<b>[[*class_key]]</b><br>',
             name: 'class_key',
             hiddenName: 'class_key',
             id: 'modx-resource-class-key',
             allowBlank: false,
-            value: config.record.class_key || 'MODX\\Revolution\\modDocument'
+            value: classKey
         }, {
             xtype: 'modx-combo-content-type',
             fieldLabel: _('resource_content_type'),

manager/assets/modext/widgets/resource/modx.tree.resource.js

@@ -1290,11 +1290,6 @@ MODx.getQuickCreateResourceSettingsFields = function(id, parentData) {
                 name: 'context_key',
                 id: `modx-${id}-context_key`,
                 value: parentData.context_key
-            }, {
-                xtype: 'hidden',
-                name: 'class_key',
-                id: `modx-${id}-class_key`,
-                value: parentData.class_key
             }, {
                 xtype: 'hidden',
                 name: 'publishedon',
@@ -1312,7 +1307,7 @@ MODx.getQuickCreateResourceSettingsFields = function(id, parentData) {
                 contextcmp: `modx-${id}-context_key`,
                 currentid: parentData.id || 0
             }, {
-                xtype: 'modx-combo-class-derivatives',
+                xtype: MODx.perm.class_map ? 'modx-combo-class-derivatives' : 'hidden',
                 fieldLabel: _('resource_type'),
                 description: '<b>[[*class_key]]</b><br>',
                 name: 'class_key',

manager/assets/modext/widgets/resource/modx.window.resource.js

@@ -3,19 +3,20 @@ MODx.window.CreateResource = function(config = {}) {
     const id = this.ident,
           requireAlias = parseInt(MODx.config.friendly_urls, 10) && !parseInt(MODx.config.automatic_alias, 10),
           aliasLength = parseInt(MODx.config.friendly_alias_max_length, 10) || 0,
+          classKey = config.record.class_key || 'MODX\\Revolution\\modDocument',
           resourceDetail = [
               {
                   columnWidth: requireAlias ? 0.33 : 0.5,
                   items: [
                       {
-                          xtype: 'modx-combo-class-derivatives',
+                          xtype: MODx.perm.class_map ? 'modx-combo-class-derivatives' : 'hidden',
                           fieldLabel: _('resource_type'),
                           description: MODx.expandHelp ? '' : _('resource_type_help'),
                           name: 'class_key',
                           hiddenName: 'class_key',
                           anchor: '100%',
                           allowBlank: false,
-                          value: config.record.class_key || 'MODX\\Revolution\\modDocument'
+                          value: classKey
                       }
                   ]
               }, {
@@ -35,8 +36,7 @@ MODx.window.CreateResource = function(config = {}) {
                       }
                   ]
               }
-          ]
-    ;
+          ];
     if (requireAlias) {
         resourceDetail.push({
             columnWidth: 0.34,
@@ -220,8 +220,7 @@ Ext.extend(MODx.combo.TemplatePicker, Ext.Panel, {
     loadItems: function(store, data) {
         const
             { value } = this,
-            items = []
-        ;
+            items = [];
         let category = '';
 
         Ext.each(data, function(record) {
@@ -260,8 +259,7 @@ Ext.extend(MODx.combo.TemplatePicker, Ext.Panel, {
                     fn: function(cmp) {
                         const
                             pickerValue = cmp.getValue(),
-                            record = pickerValue?.record
-                        ;
+                            record = pickerValue?.record;
                         if (record) {
                             this.fireEvent('select', record);
                         }