From 7a055184b9c1c10cfe0c0b3591da7346d349da5f Mon Sep 17 00:00:00 2001 From: Artur Kyryliuk Date: Sun, 30 Aug 2026 01:10:29 +0200 Subject: [PATCH] fix(manager): for non-admin managers the Create link button should select allowed resource for link creation. --- core/functions/nodes.php | 25 ++- core/src/Legacy/Permissions.php | 186 +++++++++++++++--- core/src/Support/ResourceParentGuard.php | 105 ++++++++++ .../Unit/PermissionsDocumentAccessTest.php | 121 ++++++++++++ .../Unit/ResourceParentSelectionTest.php | 67 +++++++ .../Unit/Support/ResourceParentGuardTest.php | 52 +++++ core/vendor/composer/autoload_classmap.php | 1 + core/vendor/composer/autoload_static.php | 1 + manager/actions/mutate_content.dynamic.php | 25 +-- .../tests/tree-parent-guard-helper.test.js | 21 ++ .../media/script/tree-parent-guard-helper.js | 32 +++ manager/media/style/default/js/evo.js | 14 ++ manager/views/frame/1.blade.php | 2 + manager/views/frame/tree.blade.php | 1 + 14 files changed, 607 insertions(+), 46 deletions(-) create mode 100644 core/src/Support/ResourceParentGuard.php create mode 100644 core/tests/Unit/PermissionsDocumentAccessTest.php create mode 100644 core/tests/Unit/ResourceParentSelectionTest.php create mode 100644 core/tests/Unit/Support/ResourceParentGuardTest.php create mode 100644 manager/media/script/tests/tree-parent-guard-helper.test.js create mode 100644 manager/media/script/tree-parent-guard-helper.js diff --git a/core/functions/nodes.php b/core/functions/nodes.php index 9bef44b3e4..4b68329627 100644 --- a/core/functions/nodes.php +++ b/core/functions/nodes.php @@ -61,9 +61,12 @@ function makeHTML($indent, $parent, $expandAll, $hereid = '') } $mgrRole = (isset ($_SESSION['mgrRole']) && (string)$_SESSION['mgrRole'] === '1') ? '1' : '0'; - $docgrp_cond = $docgrp ? 'OR dg.document_group IN (' . $docgrp . ')' : ''; + // documents this user may work with, used to tell which nodes can take a new child + $usePermissions = $modx->getConfig('use_udperms'); + $accessibleDocuments = \EvolutionCMS\Legacy\Permissions::getAccessibleDocumentIds(); + + // $docgrp_cond = $docgrp ? 'OR dg.document_group IN (' . $docgrp . ')' : ''; $mgrRole = (int)$mgrRole; - $docgrp_cond = $docgrp_cond; $result = \EvolutionCMS\Models\SiteContent::query()->withTrashed()->select('site_content.id', 'site_content.pagetitle', 'longtitle', 'menutitle', 'parent', 'isfolder' @@ -123,6 +126,18 @@ function makeHTML($indent, $parent, $expandAll, $hereid = '') if ($mgrRole == 1 || $row['privatemgr'] == 0) { $row['hasAccess'] = 1; } + // reachability is wider than hasAccess: it also covers the private documents + // this user is a member of, which are legitimate places to create a resource in + $row['canAddChild'] = \EvolutionCMS\Support\ResourceParentGuard::nodeAcceptsChild( + \EvolutionCMS\Support\ResourceParentGuard::documentIsAccessible( + $mgrRole, + $usePermissions, + $row['privatemgr'], + $row['id'], + $accessibleDocuments + ), + $row['deleted'] + ) ? 1 : 0; $node = ''; $nodetitle = getNodeTitle($nodeNameSource, $row); $nodetitleDisplay = $nodetitle; @@ -243,7 +258,8 @@ function makeHTML($indent, $parent, $expandAll, $hereid = '') 'level' => $level, 'isPrivate' => 0, 'roles' => ($row['roles'] ? $row['roles'] : ''), - 'nomove' => 0 + 'nomove' => 0, + 'canAddChild' => $row['canAddChild'] ]; $ph = $data; @@ -665,6 +681,7 @@ function getTplSingleNode() data-private="[+isPrivate+]" data-roles="[+roles+]" data-nomove="[+nomove+]" + data-canaddchild="[+canAddChild+]" data-level="[+level+]" data-treepageclick="[+tree_page_click+]" [+contextmenu+] @@ -696,6 +713,7 @@ function getTplFolderNode() data-private="[+isPrivate+]" data-roles="[+roles+]" data-nomove="[+nomove+]" + data-canaddchild="[+canAddChild+]" data-level="[+level+]" data-icon-expanded="[+tree_plusnode+]" data-icon-collapsed="[+tree_minusnode+]" @@ -738,6 +756,7 @@ function getTplFolderNodeNotChildren() data-private="[+isPrivate+]" data-roles="[+roles+]" data-nomove="[+nomove+]" + data-canaddchild="[+canAddChild+]" data-level="[+level+]" data-icon-expanded="[+tree_plusnode+]" data-icon-collapsed="[+tree_minusnode+]" diff --git a/core/src/Legacy/Permissions.php b/core/src/Legacy/Permissions.php index 6fbf7a243d..1be905fe92 100644 --- a/core/src/Legacy/Permissions.php +++ b/core/src/Legacy/Permissions.php @@ -1,6 +1,8 @@ document; - $role = $this->role; + $document = (int) $this->document; - if ($role == 1) { - return true; // administrator - grant all document permissions + if (ResourceParentGuard::grantsWithoutLookup($this->role, $modx->getConfig('use_udperms'))) { + return true; // administrator, or permissions aren't in use } - if ($modx->getConfig('use_udperms') == 0 || $modx->getConfig('use_udperms') == "" || !isset($modx->config['use_udperms'])) { - return true; // permissions aren't in use - } - $parent = SiteContent::query()->find($this->document); - $parent = $parent->parent ?? null; - if ($document == 0 && $parent == null && $udperms_allowroot == 1) { - return true; - } // User is allowed to create new document in root - if (($this->duplicateDoc == true || $document == 0) && $parent == 0 && $udperms_allowroot == 0) { - return false; // deny duplicate || create new document at root if Allow Root is No + if ($document === 0) { + return static::rootIsAllowed(); // placing a resource in the site root } - // get document groups for current user - $docgrp = empty($_SESSION['mgrDocgroups']) ? '' : implode(' || dg.document_group = ', - $_SESSION['mgrDocgroups']); + if ($this->duplicateDoc) { + $source = SiteContent::withTrashed()->find($document); + if (ResourceParentGuard::duplicateBlockedAtRoot(static::allowRootSetting(), $source->parent ?? 0)) { + return false; // the duplicate would end up in the root + } + } /* Note: A document is flagged as private whenever the document group that it @@ -64,22 +58,150 @@ public function checkPermissions() are private to the manager users will not be private to web users if the document group is not assigned to a web user group and visa versa. */ - $permissionsok = false; // set permissions to false - - $query = SiteContent::query()->select('id'); - if(!empty($docgrp)){ - $query = $query->leftJoin('document_groups', 'site_content.id','=', 'document_groups.document') - ->where(function($q) use ($docgrp) { - $q->where('document_groups.document_group', $docgrp) - ->orWhere('site_content.privatemgr', 0); + return static::documentIsAccessible($document); + } + + /** + * The manager no longer exports settings as globals, so the setting is the authoritative + * source here; the legacy global is still honoured when something did define it. + * + * @return mixed + */ + public static function allowRootSetting() + { + global $udperms_allowroot; + + return isset($udperms_allowroot) ? $udperms_allowroot : evo()->getConfig('udperms_allowroot'); + } + + /** + * @return bool + */ + public static function rootIsAllowed() + { + return ResourceParentGuard::allowsRoot(static::allowRootSetting()); + } + + /** + * Document groups of the manager user of the current session. + * + * @return array + */ + public static function getManagerDocumentGroups() + { + return (isset($_SESSION['mgrDocgroups']) && is_array($_SESSION['mgrDocgroups'])) + ? $_SESSION['mgrDocgroups'] + : []; + } + + /** + * Ids of the private documents the manager user of the current session belongs to. + * Trashed documents are kept, so restoring one from the recycle bin stays possible. + * + * @return array + */ + public static function getAccessibleDocumentIds() + { + static $documents = null; + + if ($documents === null) { + $docgrp = static::getManagerDocumentGroups(); + $documents = empty($docgrp) + ? [] + : DocumentGroup::query()->whereIn('document_group', $docgrp) + ->pluck('document') + ->map(static function ($document) { + return (int) $document; + }) + ->all(); + } + + return $documents; + } + + /** + * Whether the manager user of the current session may work with the given document. + * + * @param int $document + * @return bool + */ + public static function documentIsAccessible($document) + { + $docgrp = static::getManagerDocumentGroups(); + + // withTrashed(), so publishing, restoring and moving a trashed document keep working + $query = SiteContent::withTrashed()->where('site_content.id', (int) $document); + + if (empty($docgrp)) { + $query->where('site_content.privatemgr', 0); + } else { + $query->leftJoin('document_groups', 'site_content.id', '=', 'document_groups.document') + ->where(function ($q) use ($docgrp) { + $q->where('site_content.privatemgr', 0) + ->orWhereIn('document_groups.document_group', $docgrp); }); - }else { - $query->where('privatemgr', 0); } - if ($query->count() > 0) { - $permissionsok = true; + + return $query->exists(); + } + + /** + * Whether the manager user of the current session may place a resource inside $parent. + * + * @param int $parent + * @return bool + */ + public static function canCreateIn($parent) + { + $udperms = new static(); + $udperms->user = evo()->getLoginUserID('mgr'); + $udperms->document = (int) $parent; + $udperms->role = $_SESSION['mgrRole'] ?? 0; + + return $udperms->checkPermissions(); + } + + /** + * First location the manager user of the current session may create resources in. + * + * @return int + */ + public static function getFirstAllowedParent() + { + if (static::canCreateIn(0)) { + return 0; } - return $permissionsok; + return ResourceParentGuard::pickDefaultParent(false, static::findFirstAccessibleDocument()); + } + + /** + * Topmost document of the tree the manager user of the current session can reach. + * + * @return int|null + */ + protected static function findFirstAccessibleDocument() + { + $docgrp = static::getManagerDocumentGroups(); + + $query = SiteContent::query()->select('site_content.id') + ->where('site_content.deleted', 0); + + if (empty($docgrp)) { + $query->where('site_content.privatemgr', 0); + } else { + $query->leftJoin('document_groups', 'site_content.id', '=', 'document_groups.document') + ->where(function ($q) use ($docgrp) { + $q->where('site_content.privatemgr', 0) + ->orWhereIn('document_groups.document_group', $docgrp); + }); + } + + $first = $query->orderBy('site_content.parent') + ->orderBy('site_content.menuindex') + ->orderBy('site_content.id') + ->first(); + + return $first === null ? null : (int) $first->id; } } diff --git a/core/src/Support/ResourceParentGuard.php b/core/src/Support/ResourceParentGuard.php new file mode 100644 index 0000000000..7175843976 --- /dev/null +++ b/core/src/Support/ResourceParentGuard.php @@ -0,0 +1,105 @@ +addConnection(['driver' => 'sqlite', 'database' => ':memory:', 'prefix' => '']); + $capsule->setAsGlobal(); + $capsule->bootEloquent(); + + $schema = Capsule::schema(); + $schema->dropIfExists('site_content'); + $schema->dropIfExists('document_groups'); + $schema->create('site_content', function ($table) { + $table->increments('id'); + $table->integer('parent')->default(0); + $table->string('pagetitle')->default(''); + $table->integer('menuindex')->default(0); + $table->integer('privatemgr')->default(0); + $table->integer('deleted')->default(0); + }); + $schema->create('document_groups', function ($table) { + $table->increments('id'); + $table->integer('document_group'); + $table->integer('document'); + }); + + Capsule::table('site_content')->insert([ + ['id' => 10, 'parent' => 0, 'menuindex' => 1, 'privatemgr' => 0, 'deleted' => 0, 'pagetitle' => 'public folder'], + ['id' => 11, 'parent' => 10, 'menuindex' => 0, 'privatemgr' => 0, 'deleted' => 0, 'pagetitle' => 'public child'], + ['id' => 20, 'parent' => 0, 'menuindex' => 0, 'privatemgr' => 1, 'deleted' => 0, 'pagetitle' => 'private folder'], + ['id' => 21, 'parent' => 20, 'menuindex' => 0, 'privatemgr' => 1, 'deleted' => 0, 'pagetitle' => 'private child'], + ['id' => 30, 'parent' => 0, 'menuindex' => 2, 'privatemgr' => 0, 'deleted' => 1, 'pagetitle' => 'trashed'], + ]); + Capsule::table('document_groups')->insert([ + ['document_group' => 5, 'document' => 20], + ['document_group' => 5, 'document' => 21], + ]); +} + +test('the document lookup is limited to the document being checked', function () { + bootPermissionsFixture(); + $_SESSION['mgrDocgroups'] = []; + + expect(Permissions::documentIsAccessible(10))->toBeTrue() + ->and(Permissions::documentIsAccessible(11))->toBeTrue() + // before the fix the group query counted every row and answered yes for anything + ->and(Permissions::documentIsAccessible(20))->toBeFalse() + ->and(Permissions::documentIsAccessible(21))->toBeFalse() + ->and(Permissions::documentIsAccessible(999))->toBeFalse(); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); + +test('private documents open up for the groups the user belongs to', function () { + bootPermissionsFixture(); + $_SESSION['mgrDocgroups'] = [5]; + + expect(Permissions::documentIsAccessible(20))->toBeTrue() + ->and(Permissions::documentIsAccessible(21))->toBeTrue() + ->and(Permissions::documentIsAccessible(10))->toBeTrue(); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); + +test('trashed documents stay reachable so undelete and publish keep their target', function () { + bootPermissionsFixture(); + $_SESSION['mgrDocgroups'] = []; + + expect(Permissions::documentIsAccessible(30))->toBeTrue(); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); + +test('the default parent is the topmost document the user can reach', function () { + bootPermissionsFixture(); + + // the private folder sorts first but is skipped for a user outside its group + $_SESSION['mgrDocgroups'] = []; + expect(PermissionsDocumentAccessProbe::firstAccessible())->toBe(10); + + // a member of group 5 gets that private folder, it sorts before the public one + $_SESSION['mgrDocgroups'] = [5]; + expect(PermissionsDocumentAccessProbe::firstAccessible())->toBe(20); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); + +test('trashed documents are never offered as a default parent', function () { + bootPermissionsFixture(); + $_SESSION['mgrDocgroups'] = []; + Capsule::table('site_content')->whereIn('id', [10, 11])->update(['deleted' => 1]); + + // 30 is trashed as well, so nothing public is left + expect(PermissionsDocumentAccessProbe::firstAccessible())->toBeNull(); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); + +test('a document in several groups is not returned twice by the join', function () { + bootPermissionsFixture(); + Capsule::table('document_groups')->insert([ + ['document_group' => 9, 'document' => 20], + ]); + $_SESSION['mgrDocgroups'] = [5, 9]; + + expect(Permissions::documentIsAccessible(20))->toBeTrue() + ->and(PermissionsDocumentAccessProbe::firstAccessible())->toBe(20); +})->skip(!extension_loaded('pdo_sqlite'), 'pdo_sqlite is required'); diff --git a/core/tests/Unit/ResourceParentSelectionTest.php b/core/tests/Unit/ResourceParentSelectionTest.php new file mode 100644 index 0000000000..3329aba92f --- /dev/null +++ b/core/tests/Unit/ResourceParentSelectionTest.php @@ -0,0 +1,67 @@ +toContain("evo()->getConfig('udperms_allowroot')") + // the document group lookup has to be limited to the document being checked + ->and($source)->toContain("->where('site_content.id', (int) \$document)") + ->and($source)->toContain("->orWhereIn('document_groups.document_group', \$docgrp)") + // trashed documents stay visible, otherwise undelete and publish lose their target + ->and($source)->toContain('SiteContent::withTrashed()->where(\'site_content.id\'') + // intval() as a Collection callback receives the key as its base argument + ->and($source)->not->toContain("->map('intval')"); +}); + +test('a new resource without a requested parent falls back to the first allowed location', function () use ($root) { + $source = file_get_contents($root . '/manager/actions/mutate_content.dynamic.php'); + + expect($source)->toContain('EvolutionCMS\Legacy\Permissions::getFirstAllowedParent()') + ->and($source)->toContain('EvolutionCMS\Legacy\Permissions::canCreateIn(') + // a restored form keeps the parent the user picked + ->and($source)->toContain("!isset(\$_REQUEST['pid']) && !isset(\$content['parent'])"); +}); + +test('resource tree nodes publish whether they accept a new child', function () use ($root) { + $source = file_get_contents($root . '/core/functions/nodes.php'); + + expect($source)->toContain('ResourceParentGuard::documentIsAccessible(') + ->and($source)->toContain('ResourceParentGuard::nodeAcceptsChild(') + ->and($source)->toContain("'canAddChild' => \$row['canAddChild']") + // single, folder and childless folder node templates + ->and(substr_count($source, 'data-canaddchild="[+canAddChild+]"'))->toBe(3) + // the protected styling and the hasAccess placeholder keep their old meaning + ->and($source)->toContain("if (\$mgrRole == 1 || \$row['privatemgr'] == 0) {"); +}); + +test('the site root node reports whether resources may be created in it', function () use ($root) { + $template = file_get_contents($root . '/manager/views/frame/tree.blade.php'); + + expect($template)->toContain('data-canaddchild="{{ \EvolutionCMS\Legacy\Permissions::canCreateIn(0) ? 1 : 0 }}"'); +}); + +test('default manager theme refuses parent and move targets the user may not manage', function () use ($root) { + $themeJs = file_get_contents($root . '/manager/media/style/default/js/evo.js'); + + expect($themeJs)->toContain('isBlockedParentTarget: function (node)') + ->and($themeJs)->toContain('w.modxTreeParentGuardHelper.isBlockedParentTarget(node)') + // both the parent picker of the resource form and the move screen are guarded + ->and(substr_count($themeJs, 'this.isBlockedParentTarget(el)'))->toBe(2) + ->and($themeJs)->toContain('alert(evo.lang.access_permission_parent_denied)'); +}); + +test('the manager frame loads the parent guard helper and its message', function () use ($root) { + $template = file_get_contents($root . '/manager/views/frame/1.blade.php'); + + expect($template)->toContain('media/script/tree-parent-guard-helper.js') + ->and($template)->toContain("'access_permission_parent_denied' => ManagerTheme::getLexicon('access_permission_parent_denied')"); +}); + +test('every shipped language defines the parent denied message', function () use ($root) { + foreach (glob($root . '/core/lang/*/global.php') as $file) { + expect(file_get_contents($file))->toContain('$_lang["access_permission_parent_denied"]'); + } +}); diff --git a/core/tests/Unit/Support/ResourceParentGuardTest.php b/core/tests/Unit/Support/ResourceParentGuardTest.php new file mode 100644 index 0000000000..9e7bd04bdf --- /dev/null +++ b/core/tests/Unit/Support/ResourceParentGuardTest.php @@ -0,0 +1,52 @@ +toBeTrue() + ->and(ResourceParentGuard::grantsWithoutLookup('1', '1'))->toBeTrue() + ->and(ResourceParentGuard::grantsWithoutLookup(2, 0))->toBeTrue() + ->and(ResourceParentGuard::grantsWithoutLookup(2, ''))->toBeTrue() + ->and(ResourceParentGuard::grantsWithoutLookup(2, null))->toBeTrue() + ->and(ResourceParentGuard::grantsWithoutLookup(2, 1))->toBeFalse() + ->and(ResourceParentGuard::grantsWithoutLookup(null, 1))->toBeFalse(); +}); + +test('the site root follows the allow root setting instead of an undefined global', function () { + expect(ResourceParentGuard::allowsRoot(1))->toBeTrue() + ->and(ResourceParentGuard::allowsRoot('1'))->toBeTrue() + ->and(ResourceParentGuard::allowsRoot(0))->toBeFalse() + ->and(ResourceParentGuard::allowsRoot('0'))->toBeFalse() + ->and(ResourceParentGuard::allowsRoot(''))->toBeFalse() + ->and(ResourceParentGuard::allowsRoot(null))->toBeFalse(); +}); + +test('duplicating a root level resource is blocked only while the root is denied', function () { + expect(ResourceParentGuard::duplicateBlockedAtRoot(0, 0))->toBeTrue() + ->and(ResourceParentGuard::duplicateBlockedAtRoot(0, null))->toBeTrue() + ->and(ResourceParentGuard::duplicateBlockedAtRoot(1, 0))->toBeFalse() + ->and(ResourceParentGuard::duplicateBlockedAtRoot(0, 12))->toBeFalse(); +}); + +test('document access combines the private flag with the document groups of the user', function () { + expect(ResourceParentGuard::documentIsAccessible(1, 1, 1, 7, []))->toBeTrue() + ->and(ResourceParentGuard::documentIsAccessible(2, 0, 1, 7, []))->toBeTrue() + ->and(ResourceParentGuard::documentIsAccessible(2, 1, 0, 7, []))->toBeTrue() + ->and(ResourceParentGuard::documentIsAccessible(2, 1, 1, 7, [3, 9]))->toBeFalse() + ->and(ResourceParentGuard::documentIsAccessible(2, 1, 1, 7, [3, 7, 9]))->toBeTrue() + ->and(ResourceParentGuard::documentIsAccessible(2, 1, '1', '7', [7]))->toBeTrue(); +}); + +test('only reachable and untrashed nodes accept a new child', function () { + expect(ResourceParentGuard::nodeAcceptsChild(1, 0))->toBeTrue() + ->and(ResourceParentGuard::nodeAcceptsChild('1', '0'))->toBeTrue() + ->and(ResourceParentGuard::nodeAcceptsChild(1, 1))->toBeFalse() + ->and(ResourceParentGuard::nodeAcceptsChild(0, 0))->toBeFalse(); +}); + +test('a new resource lands in the root only when the root is available', function () { + expect(ResourceParentGuard::pickDefaultParent(true, 42))->toBe(0) + ->and(ResourceParentGuard::pickDefaultParent(false, 42))->toBe(42) + ->and(ResourceParentGuard::pickDefaultParent(false, '42'))->toBe(42) + ->and(ResourceParentGuard::pickDefaultParent(false, null))->toBe(0); +}); diff --git a/core/vendor/composer/autoload_classmap.php b/core/vendor/composer/autoload_classmap.php index 3f6eeefb02..1fdf2e02fb 100644 --- a/core/vendor/composer/autoload_classmap.php +++ b/core/vendor/composer/autoload_classmap.php @@ -1406,6 +1406,7 @@ 'EvolutionCMS\\Support\\MoveDocumentTargetGuard' => $baseDir . '/src/Support/MoveDocumentTargetGuard.php', 'EvolutionCMS\\Support\\MysqlDumper' => $baseDir . '/src/Support/MysqlDumper.php', 'EvolutionCMS\\Support\\Paginate' => $baseDir . '/src/Support/Paginate.php', + 'EvolutionCMS\\Support\\ResourceParentGuard' => $baseDir . '/src/Support/ResourceParentGuard.php', 'EvolutionCMS\\Support\\SiteTimezone' => $baseDir . '/src/Support/SiteTimezone.php', 'EvolutionCMS\\Support\\SqliteDumper' => $baseDir . '/src/Support/SqliteDumper.php', 'EvolutionCMS\\Support\\SystemSettingPathNormalizer' => $baseDir . '/src/Support/SystemSettingPathNormalizer.php', diff --git a/core/vendor/composer/autoload_static.php b/core/vendor/composer/autoload_static.php index 7e5476db4b..1968cbd4d5 100644 --- a/core/vendor/composer/autoload_static.php +++ b/core/vendor/composer/autoload_static.php @@ -2083,6 +2083,7 @@ class ComposerStaticInit925fea465a58fa69f06ccf2629003e87 'EvolutionCMS\\Support\\MoveDocumentTargetGuard' => __DIR__ . '/../..' . '/src/Support/MoveDocumentTargetGuard.php', 'EvolutionCMS\\Support\\MysqlDumper' => __DIR__ . '/../..' . '/src/Support/MysqlDumper.php', 'EvolutionCMS\\Support\\Paginate' => __DIR__ . '/../..' . '/src/Support/Paginate.php', + 'EvolutionCMS\\Support\\ResourceParentGuard' => __DIR__ . '/../..' . '/src/Support/ResourceParentGuard.php', 'EvolutionCMS\\Support\\SiteTimezone' => __DIR__ . '/../..' . '/src/Support/SiteTimezone.php', 'EvolutionCMS\\Support\\SqliteDumper' => __DIR__ . '/../..' . '/src/Support/SqliteDumper.php', 'EvolutionCMS\\Support\\SystemSettingPathNormalizer' => __DIR__ . '/../..' . '/src/Support/SystemSettingPathNormalizer.php', diff --git a/manager/actions/mutate_content.dynamic.php b/manager/actions/mutate_content.dynamic.php index 30655f7c32..289f6dd01a 100644 --- a/manager/actions/mutate_content.dynamic.php +++ b/manager/actions/mutate_content.dynamic.php @@ -30,15 +30,11 @@ case 4: if(!$modx->hasPermission('new_document')) { $modx->webAlertAndQuit($_lang["error_no_privileges"]); - } elseif(isset($_REQUEST['pid']) && $_REQUEST['pid'] != '0') { - // check user has permissions for parent - $udperms = new EvolutionCMS\Legacy\Permissions(); - $udperms->user = $modx->getLoginUserID('mgr'); - $udperms->document = empty($_REQUEST['pid']) ? 0 : $_REQUEST['pid']; - $udperms->role = $_SESSION['mgrRole']; - if(!$udperms->checkPermissions()) { - $modx->webAlertAndQuit($_lang["access_permission_denied"]); - } + } + // check user has permissions for the requested parent + if(isset($_REQUEST['pid']) + && !EvolutionCMS\Legacy\Permissions::canCreateIn((int)get_by_key($_REQUEST, 'pid', 0, 'is_scalar'))) { + evo()->webAlertAndQuit($_lang["access_permission_parent_denied"]); } break; default: @@ -130,6 +126,12 @@ } } +// no parent was requested and there is none to restore: preselect the first location this +// user may create in, instead of the root, which non-administrators are often denied +if(evo()->getManagerApi()->action != 27 && !isset($_REQUEST['pid']) && !isset($content['parent'])) { + $_REQUEST['pid'] = EvolutionCMS\Legacy\Permissions::getFirstAllowedParent(); +} + // increase menu index if this is a new document if(!isset($_REQUEST['id'])) { if ($modx->getConfig('auto_menuindex')) { @@ -848,10 +850,11 @@ function evoRenderTvImageCheck(a) { $content['parent'] = 0; } if($parentlookup !== false && is_numeric($parentlookup)) { - $parentname = SiteContent::withTrashed()->select('pagetitle')->find($parentlookup)->pagetitle; - if(!$parentname) { + $parentDocument = SiteContent::withTrashed()->select('pagetitle')->find($parentlookup); + if(!$parentDocument) { $modx->webAlertAndQuit($_lang["error_no_parent"]); } + $parentname = $parentDocument->pagetitle; } ?> " onclick="enableParentSelection(!allowParentSelection);"> diff --git a/manager/media/script/tests/tree-parent-guard-helper.test.js b/manager/media/script/tests/tree-parent-guard-helper.test.js new file mode 100644 index 0000000000..74b6fd6fab --- /dev/null +++ b/manager/media/script/tests/tree-parent-guard-helper.test.js @@ -0,0 +1,21 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const helper = require('../tree-parent-guard-helper'); + +test('isBlockedParentTarget blocks nodes the user may not create in', () => { + assert.equal(helper.isBlockedParentTarget({ dataset: { canaddchild: '0' } }), true); +}); + +test('isBlockedParentTarget allows nodes the user may create in', () => { + assert.equal(helper.isBlockedParentTarget({ dataset: { canaddchild: '1' } }), false); +}); + +test('isBlockedParentTarget allows nodes rendered without the flag', () => { + assert.equal(helper.isBlockedParentTarget(null), false); + assert.equal(helper.isBlockedParentTarget({}), false); + assert.equal(helper.isBlockedParentTarget({ dataset: {} }), false); +}); + +test('acceptsChild keeps nodes selectable when a plugin leaves the placeholder unresolved', () => { + assert.equal(helper.acceptsChild({ dataset: { canaddchild: '[+canAddChild+]' } }), true); +}); diff --git a/manager/media/script/tree-parent-guard-helper.js b/manager/media/script/tree-parent-guard-helper.js new file mode 100644 index 0000000000..2bbfbeaf23 --- /dev/null +++ b/manager/media/script/tree-parent-guard-helper.js @@ -0,0 +1,32 @@ +(function (root, factory) { + var exported = factory(); + + if (typeof module === 'object' && module.exports) { + module.exports = exported; + } + + root.modxTreeParentGuardHelper = exported; +}(typeof globalThis !== 'undefined' ? globalThis : this, function () { + 'use strict'; + + function acceptsChild(targetAnchor) { + if (!targetAnchor || !targetAnchor.dataset) { + return true; + } + + var flag = parseInt(targetAnchor.dataset.canaddchild, 10); + + // nodes rendered without the flag, or with a placeholder a plugin left unresolved, + // stay selectable: the save processor is still the one having the final word + return isNaN(flag) || flag !== 0; + } + + function isBlockedParentTarget(targetAnchor) { + return !acceptsChild(targetAnchor); + } + + return { + acceptsChild: acceptsChild, + isBlockedParentTarget: isBlockedParentTarget + }; +})); diff --git a/manager/media/style/default/js/evo.js b/manager/media/style/default/js/evo.js index 2e62129a49..4f00e788d6 100644 --- a/manager/media/style/default/js/evo.js +++ b/manager/media/style/default/js/evo.js @@ -1085,6 +1085,10 @@ isBlockedDropTarget: function (target) { return !!(w.modxTreeDropGuardHelper && !w.modxTreeDropGuardHelper.canDropIntoTarget(target)); }, + // nodes the current user may not create or move a resource into + isBlockedParentTarget: function (node) { + return !!(w.modxTreeParentGuardHelper && w.modxTreeParentGuardHelper.isBlockedParentTarget(node)); + }, ondragenter: function (e) { if ( d.getElementById('node' + evo.tree.itemToChange) === (this.parentNode.closest('#node' + evo.tree.itemToChange) || this.parentNode) @@ -1390,6 +1394,11 @@ openfolder = parseInt(el.dataset.openfolder); title = title || el.dataset && el.dataset.titleEsc; if (tree.ca === 'move') { + if (this.isBlockedParentTarget(el)) { + e.preventDefault(); + alert(evo.lang.access_permission_parent_denied); + return; + } try { this.setSelectedByContext(id); w.main.setMoveValue(id, title); @@ -1428,6 +1437,11 @@ this.setSelected(id); } if (tree.ca === 'parent') { + if (this.isBlockedParentTarget(el)) { + e.preventDefault(); + alert(evo.lang.access_permission_parent_denied); + return; + } try { this.setSelectedByContext(id); w.main.setParent(id, title); diff --git a/manager/views/frame/1.blade.php b/manager/views/frame/1.blade.php index 7faf02e76e..047a4667b9 100644 --- a/manager/views/frame/1.blade.php +++ b/manager/views/frame/1.blade.php @@ -115,6 +115,7 @@ function jsIconMarkup($icon) { selectedObjectName: null }, lang: {{ js_json([ + 'access_permission_parent_denied' => ManagerTheme::getLexicon('access_permission_parent_denied'), 'already_deleted' => ManagerTheme::getLexicon('already_deleted'), 'cm_unknown_error' => ManagerTheme::getLexicon('cm_unknown_error'), 'collapse_tree' => ManagerTheme::getLexicon('collapse_tree'), @@ -215,6 +216,7 @@ function jsIconMarkup($icon) { window.tree = evo.tree; + @if ($modx->getConfig('show_picker')) diff --git a/manager/views/frame/tree.blade.php b/manager/views/frame/tree.blade.php index 9cb5a5bbcd..3f18988623 100644 --- a/manager/views/frame/tree.blade.php +++ b/manager/views/frame/tree.blade.php @@ -61,6 +61,7 @@ ?>
{{ $siteName }}