fix(acl): resolve renamed Resource policy for parallel groups

_build/test/Tests/Model/Security/modAccessPolicyTest.php

@@ -0,0 +1,124 @@
+<?php
+
+/*
+ * This file is part of the MODX Revolution package.
+ *
+ * Copyright (c) MODX, LLC
+ *
+ * For complete copyright and license information, see the COPYRIGHT and LICENSE
+ * files found in the top-level directory of this distribution.
+ *
+ * @package modx-test
+ */
+namespace MODX\Revolution\Tests\Model\Security;
+
+use MODX\Revolution\modAccessPolicy;
+use MODX\Revolution\modAccessPolicyTemplate;
+use MODX\Revolution\MODxTestCase;
+
+/**
+ * Tests related to modAccessPolicy core policy resolution.
+ *
+ * @package modx-test
+ * @subpackage modx
+ * @group Model
+ * @group Access
+ * @group modAccessPolicy
+ */
+class modAccessPolicyTest extends MODxTestCase
+{
+    /**
+     * Renamed core Resource policy must still resolve for parallel resource groups (#13831).
+     */
+    public function testGetPolicyFindsRenamedResourcePolicy()
+    {
+        /** @var modAccessPolicy|null $policy */
+        $policy = $this->modx->getObject(modAccessPolicy::class, [
+            'name' => modAccessPolicy::POLICY_RESOURCE,
+        ]);
+        if (!$policy instanceof modAccessPolicy) {
+            $this->markTestSkipped('Core Resource access policy is not installed.');
+        }
+
+        $originalName = $policy->get('name');
+        $policyId = (int)$policy->get('id');
+        $renamed = $originalName . '_renamed_13831';
+
+        $policy->set('name', $renamed);
+        $this->assertTrue($policy->save(), 'Failed to rename Resource policy for the test.');
+
+        try {
+            $this->assertNull(
+                $this->modx->getObject(modAccessPolicy::class, ['name' => modAccessPolicy::POLICY_RESOURCE]),
+                'Direct name lookup must fail after rename.'
+            );
+
+            $resolved = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
+            $this->assertInstanceOf(modAccessPolicy::class, $resolved);
+            $this->assertSame($policyId, (int)$resolved->get('id'));
+            $this->assertSame($renamed, $resolved->get('name'));
+        } finally {
+            $policy->set('name', $originalName);
+            $policy->save();
+        }
+    }
+
+    /**
+     * Unchanged core policy name still resolves via getPolicy().
+     */
+    public function testGetPolicyFindsResourcePolicyByName()
+    {
+        $resolved = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
+        if (!$resolved instanceof modAccessPolicy) {
+            $this->markTestSkipped('Core Resource access policy is not installed.');
+        }
+
+        $this->assertSame(modAccessPolicy::POLICY_RESOURCE, $resolved->get('name'));
+    }
+
+    /**
+     * Ambiguous ResourceTemplate (renamed core + duplicate) must fail closed.
+     */
+    public function testGetPolicyFailsClosedWhenMultiplePoliciesShareTemplate()
+    {
+        /** @var modAccessPolicy|null $policy */
+        $policy = $this->modx->getObject(modAccessPolicy::class, [
+            'name' => modAccessPolicy::POLICY_RESOURCE,
+        ]);
+        if (!$policy instanceof modAccessPolicy) {
+            $this->markTestSkipped('Core Resource access policy is not installed.');
+        }
+
+        $originalName = $policy->get('name');
+        $templateId = (int)$policy->get('template');
+        $duplicate = null;
+
+        $policy->set('name', $originalName . '_renamed_13831_ambiguous');
+        $this->assertTrue($policy->save());
+
+        try {
+            $duplicate = $this->modx->newObject(modAccessPolicy::class);
+            $duplicate->fromArray([
+                'name' => 'Resource Duplicate 13831',
+                'description' => 'Temporary duplicate for #13831 ambiguity test',
+                'parent' => 0,
+                'template' => $templateId,
+                'class' => '',
+                'data' => $policy->get('data'),
+                'lexicon' => $policy->get('lexicon'),
+            ]);
+            $this->assertTrue($duplicate->save());
+
+            $this->assertNull(
+                modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE),
+                'Multiple policies on ResourceTemplate must not silently pick one.'
+            );
+        } finally {
+            if ($duplicate instanceof modAccessPolicy && !$duplicate->isNew()) {
+                $duplicate->remove();
+            }
+            $policy->set('name', $originalName);
+            $policy->save();
+        }
+    }
+}

core/src/Revolution/Processors/Security/Group/Create.php

@@ -203,7 +203,7 @@ public function addManagerContextAccessViaWizard($adminPolicy)
     public function addContextAccessViaWizard(array $contexts)
     {
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Context']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_CONTEXT);
         if (!$policy) {
             return false;
         }
@@ -234,7 +234,7 @@ public function addResourceGroupsViaWizard($resourceGroupNames, array $contexts)
         $resourceGroupNames = array_unique($resourceGroupNames);
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Resource']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
         if (!$policy) {
             return false;
         }
@@ -283,7 +283,7 @@ public function addParallelResourceGroup(array $contexts)
         }
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Resource']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
         if (!$policy) {
             return false;
         }
@@ -315,7 +315,7 @@ public function addElementCategoriesViaWizard($categoryNames, array $contexts)
         $categoryNames = array_unique($categoryNames);
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Element']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_ELEMENT);
         if (!$policy) {
             return false;
         }

core/src/Revolution/Processors/Security/ResourceGroup/Create.php

@@ -120,7 +120,7 @@ protected function addAdminAccess(array $contexts = [])
         }
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Resource']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
         if (!$policy) {
             return false;
         }
@@ -150,7 +150,7 @@ protected function addAdminAccess(array $contexts = [])
     protected function addAnonymousAccess(array $contexts = [])
     {
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Load, List and View']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_LOAD_LIST_VIEW);
         if (!$policy) {
             return false;
         }
@@ -191,7 +191,7 @@ protected function addParallelUserGroup(array $contexts = [])
         }
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Resource']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
         if (!$policy) {
             return false;
         }
@@ -224,7 +224,7 @@ protected function addOtherUserGroups(array $userGroupNames = [], array $context
         $userGroupNames = array_unique($userGroupNames);
 
         /** @var modAccessPolicy $policy */
-        $policy = $this->modx->getObject(modAccessPolicy::class, ['name' => 'Resource']);
+        $policy = modAccessPolicy::getPolicy($this->modx, modAccessPolicy::POLICY_RESOURCE);
         if (!$policy) {
             return false;
         }

core/src/Revolution/modAccessPolicy.php

@@ -71,6 +71,58 @@ public function isCorePolicy(string $name): bool
         return in_array($name, static::getCorePolicies(), true);
     }
 
+    /**
+     * Core policies that map 1:1 to a core template (rename fallback only when unique).
+     *
+     * @return array<string, string> policy name => template name
+     */
+    private static function getUniqueCorePolicyTemplates(): array
+    {
+        return [
+            self::POLICY_RESOURCE => modAccessPolicyTemplate::TEMPLATE_RESOURCE,
+            self::POLICY_ELEMENT => modAccessPolicyTemplate::TEMPLATE_ELEMENT,
+            self::POLICY_CONTEXT => modAccessPolicyTemplate::TEMPLATE_CONTEXT,
+            self::POLICY_HIDDEN_NAMESPACE => modAccessPolicyTemplate::TEMPLATE_NAMESPACE,
+        ];
+    }
+
+    /**
+     * Resolve a uniquely-templated core Access Policy by its shipped name.
+     * If renamed and exactly one policy remains on that core template, return it.
+     * Ambiguous templates (multiple policies) fail closed with null.
+     *
+     * @param xPDO $xpdo
+     * @param string $name Core policy name (e.g. modAccessPolicy::POLICY_RESOURCE)
+     *
+     * @return static|null
+     */
+    public static function getPolicy(xPDO $xpdo, string $name): ?self
+    {
+        $policy = $xpdo->getObject(static::class, ['name' => $name]);
+        if ($policy) {
+            return $policy;
+        }
+
+        $templateName = static::getUniqueCorePolicyTemplates()[$name] ?? null;
+        if ($templateName === null) {
+            return null;
+        }
+
+        $template = $xpdo->getObject(modAccessPolicyTemplate::class, ['name' => $templateName]);
+        if (!$template) {
+            return null;
+        }
+
+        $policies = $xpdo->getCollection(static::class, [
+            'template' => $template->get('id'),
+        ]);
+        if (count($policies) !== 1) {
+            return null;
+        }
+
+        return reset($policies) ?: null;
+    }
+
     /**
      * Get the permissions for this access policy, in array format.
      *