diff --git a/_build/test/Tests/Model/Security/modAccessPolicyTest.php b/_build/test/Tests/Model/Security/modAccessPolicyTest.php new file mode 100644 index 00000000000..3789204cb70 --- /dev/null +++ b/_build/test/Tests/Model/Security/modAccessPolicyTest.php @@ -0,0 +1,124 @@ +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(); + } + } +} diff --git a/core/src/Revolution/Processors/Security/Group/Create.php b/core/src/Revolution/Processors/Security/Group/Create.php index 4070eb285c4..0cb80c4f539 100644 --- a/core/src/Revolution/Processors/Security/Group/Create.php +++ b/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; } diff --git a/core/src/Revolution/Processors/Security/ResourceGroup/Create.php b/core/src/Revolution/Processors/Security/ResourceGroup/Create.php index 6e1625c3a2f..ff2095f1743 100644 --- a/core/src/Revolution/Processors/Security/ResourceGroup/Create.php +++ b/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; } diff --git a/core/src/Revolution/modAccessPolicy.php b/core/src/Revolution/modAccessPolicy.php index 08ea91c3ec9..5d61c9b1485 100644 --- a/core/src/Revolution/modAccessPolicy.php +++ b/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 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. *