From ac66a885a686bf5bf147aa6cb3c1261d0e130dac Mon Sep 17 00:00:00 2001 From: Christian Gahlert Date: Mon, 17 Sep 2012 18:46:50 +0200 Subject: [PATCH 1/9] Modified Zend Acl/Navigation to be extendable. --- src/Acl.php | 2 +- src/AclInterface.php | 58 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/AclInterface.php diff --git a/src/Acl.php b/src/Acl.php index 30eab0c..eb287e5 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -15,7 +15,7 @@ * @package Zend_Permissions * @subpackage Acl */ -class Acl +class Acl implements AclInterface { /** * Rule type: allow diff --git a/src/AclInterface.php b/src/AclInterface.php new file mode 100644 index 0000000..9d61e68 --- /dev/null +++ b/src/AclInterface.php @@ -0,0 +1,58 @@ +allow()). + * + * If a $privilege is not provided, then this method returns false if and only if the + * Role is denied access to at least one privilege upon the Resource. In other words, this + * method returns true if and only if the Role is allowed all privileges on the Resource. + * + * This method checks Role inheritance using a depth-first traversal of the Role registry. + * The highest priority parent (i.e., the parent most recently added) is checked first, + * and its respective parents are checked similarly before the lower-priority parents of + * the Role are checked. + * + * @param Role\RoleInterface|string $role + * @param Resource\ResourceInterface|string $resource + * @param string $privilege + * @return boolean + */ + public function isAllowed($role = null, $resource = null, $privilege = null); + +} \ No newline at end of file From 32b43d1cbd5014e9656e18917824245824a500ac Mon Sep 17 00:00:00 2001 From: Christian Gahlert Date: Mon, 17 Sep 2012 19:38:08 +0200 Subject: [PATCH 2/9] Fixed spaces/braces. --- src/AclInterface.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/AclInterface.php b/src/AclInterface.php index 9d61e68..d738ffe 100644 --- a/src/AclInterface.php +++ b/src/AclInterface.php @@ -1,4 +1,5 @@ Date: Mon, 17 Sep 2012 19:42:26 +0200 Subject: [PATCH 3/9] And removed another additional line feed... :( --- src/AclInterface.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/AclInterface.php b/src/AclInterface.php index d738ffe..b390aac 100644 --- a/src/AclInterface.php +++ b/src/AclInterface.php @@ -1,5 +1,4 @@ Date: Mon, 17 Sep 2012 20:01:33 +0200 Subject: [PATCH 4/9] Changed CRLF linefeeds to LF. Stupid netbeans! --- src/AclInterface.php | 112 +++++++++++++++++++++---------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/src/AclInterface.php b/src/AclInterface.php index b390aac..93beb4b 100644 --- a/src/AclInterface.php +++ b/src/AclInterface.php @@ -1,57 +1,57 @@ -allow()). - * - * If a $privilege is not provided, then this method returns false if and only if the - * Role is denied access to at least one privilege upon the Resource. In other words, this - * method returns true if and only if the Role is allowed all privileges on the Resource. - * - * This method checks Role inheritance using a depth-first traversal of the Role registry. - * The highest priority parent (i.e., the parent most recently added) is checked first, - * and its respective parents are checked similarly before the lower-priority parents of - * the Role are checked. - * - * @param Role\RoleInterface|string $role - * @param Resource\ResourceInterface|string $resource - * @param string $privilege - * @return boolean - */ - public function isAllowed($role = null, $resource = null, $privilege = null); +allow()). + * + * If a $privilege is not provided, then this method returns false if and only if the + * Role is denied access to at least one privilege upon the Resource. In other words, this + * method returns true if and only if the Role is allowed all privileges on the Resource. + * + * This method checks Role inheritance using a depth-first traversal of the Role registry. + * The highest priority parent (i.e., the parent most recently added) is checked first, + * and its respective parents are checked similarly before the lower-priority parents of + * the Role are checked. + * + * @param Role\RoleInterface|string $role + * @param Resource\ResourceInterface|string $resource + * @param string $privilege + * @return boolean + */ + public function isAllowed($role = null, $resource = null, $privilege = null); } \ No newline at end of file From df92e6d6c9611bbbb128ef10ba3e16a1af4a28f0 Mon Sep 17 00:00:00 2001 From: Jacob Kiers Date: Fri, 18 Jan 2013 11:14:43 +0000 Subject: [PATCH 5/9] Fix zendframework/zf2#3454. The fix involves forcing a loop through the parent resources when the current resource is 'TYPE_DENIED'. The loop stops when we are already at the top-level resource. --- src/Acl.php | 5 ++++- test/AclTest.php | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Acl.php b/src/Acl.php index ac99273..111ec8f 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -747,7 +747,10 @@ public function isAllowed($role = null, $resource = null, $privilege = null) if (null !== ($ruleType = $this->getRuleType($resource, null, $privilege))) { return self::TYPE_ALLOW === $ruleType; } elseif (null !== ($ruleTypeAllPrivileges = $this->getRuleType($resource, null, null))) { - return self::TYPE_ALLOW === $ruleTypeAllPrivileges; + $result = self::TYPE_ALLOW === $ruleTypeAllPrivileges; + if ($result || null === $resource) { + return $result; + } } // try next Resource diff --git a/test/AclTest.php b/test/AclTest.php index fd3e04e..4e8a7c0 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -1304,7 +1304,7 @@ public function testRemoveDenyWithNullResourceAppliesToAllResources() } /** - * @group ZF2-5434 + * @group ZF2-3454 */ public function testAclResourcePermissionsAreInheritedWithMultilevelResources() { From ec30bc846cc8787a6424a0fdd616fe4819cfcf1b Mon Sep 17 00:00:00 2001 From: Jacob Kiers Date: Fri, 18 Jan 2013 15:52:50 +0000 Subject: [PATCH 6/9] Ensure privilege escalation is not possible. With the previous version, a deny() on a lower-lever resource was not inherited when on a higher-lever resource an allow() was given. See the updated test for more an example. --- src/Acl.php | 22 +++++++++++++++++++++- test/AclTest.php | 8 ++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/Acl.php b/src/Acl.php index 111ec8f..9a7c73a 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -572,7 +572,9 @@ public function setRule($operation, $type, $roles = null, $resources = null, $resources = array(); foreach ($resourcesTemp as $resource) { if (null !== $resource) { - $resources[] = $this->getResource($resource); + $children = $this->getChildResources($this->getResource($resource)); + $resources = array_merge($resources, $children); + $resources[$resource] = $this->getResource($resource); } else { $resources[] = null; } @@ -659,6 +661,24 @@ public function setRule($operation, $type, $roles = null, $resources = null, return $this; } + /** + * Returns all child resources from the given resource. + * + * @param Resource\ResourceInterface|string $resource + * @return Resource\ResourceInterface[] + */ + protected function getChildResources(Resource\ResourceInterface $resource) + { + $id = $resource->getResourceId(); + $children = $this->resources[$id]['children']; + $return = array(); + foreach($children as $child) { + $return = $this->getChildResources($child); + $return[$child->getResourceId()] = $child; + } + return $return; + } + /** * Returns true if and only if the Role has access to the Resource * diff --git a/test/AclTest.php b/test/AclTest.php index 4e8a7c0..79beb9f 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -1306,18 +1306,22 @@ public function testRemoveDenyWithNullResourceAppliesToAllResources() /** * @group ZF2-3454 */ - public function testAclResourcePermissionsAreInheritedWithMultilevelResources() + public function testAclResourcePermissionsAreInheritedWithMultilevelResourcesAndDenyPolicy() { $this->_acl->addRole('guest'); $this->_acl->addResource('blogposts'); $this->_acl->addResource('feature', 'blogposts'); $this->_acl->addResource('post_1', 'feature'); + // Allow a guest to read feature posts and + // comment on everything except feature posts. $this->_acl->deny(); $this->_acl->allow('guest', 'feature', 'read'); + $this->_acl->allow('guest', null, 'comment'); + $this->_acl->deny('guest', 'feature', 'comment'); $this->assertFalse($this->_acl->isAllowed('guest', 'feature', 'write')); $this->assertTrue($this->_acl->isAllowed('guest', 'post_1', 'read')); + $this->assertFalse($this->_acl->isAllowed('guest', 'post_1', 'comment')); } - } From cfecd428e07b6b49faba67e8430274a17d15b3fe Mon Sep 17 00:00:00 2001 From: Jacob Kiers Date: Fri, 18 Jan 2013 18:32:26 +0100 Subject: [PATCH 7/9] Ensure all children inherit permissions. --- src/Acl.php | 10 +++++++--- test/AclTest.php | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Acl.php b/src/Acl.php index 9a7c73a..427a70d 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -669,13 +669,17 @@ public function setRule($operation, $type, $roles = null, $resources = null, */ protected function getChildResources(Resource\ResourceInterface $resource) { + $return = array(); $id = $resource->getResourceId(); + $children = $this->resources[$id]['children']; - $return = array(); foreach($children as $child) { - $return = $this->getChildResources($child); - $return[$child->getResourceId()] = $child; + $child_return = $this->getChildResources($child); + $child_return[$child->getResourceId()] = $child; + + $return = array_merge($return, $child_return); } + return $return; } diff --git a/test/AclTest.php b/test/AclTest.php index 79beb9f..b8788f5 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -1312,6 +1312,7 @@ public function testAclResourcePermissionsAreInheritedWithMultilevelResourcesAnd $this->_acl->addResource('blogposts'); $this->_acl->addResource('feature', 'blogposts'); $this->_acl->addResource('post_1', 'feature'); + $this->_acl->addResource('post_2', 'feature'); // Allow a guest to read feature posts and // comment on everything except feature posts. @@ -1322,6 +1323,9 @@ public function testAclResourcePermissionsAreInheritedWithMultilevelResourcesAnd $this->assertFalse($this->_acl->isAllowed('guest', 'feature', 'write')); $this->assertTrue($this->_acl->isAllowed('guest', 'post_1', 'read')); + $this->assertTrue($this->_acl->isAllowed('guest', 'post_2', 'read')); + $this->assertFalse($this->_acl->isAllowed('guest', 'post_1', 'comment')); + $this->assertFalse($this->_acl->isAllowed('guest', 'post_2', 'comment')); } } From 47deb0587dc04c29b761ed8d50ec5385c675fbd4 Mon Sep 17 00:00:00 2001 From: Philipp Dobrigkeit Date: Tue, 22 Jan 2013 12:36:33 +0100 Subject: [PATCH 8/9] Fixed CS? My local php-cs-fixer did not show any errors --- test/AclTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/AclTest.php b/test/AclTest.php index 499ba1b..1df99eb 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -1338,8 +1338,5 @@ public function testSetRuleWorksWithResourceInterface() { $this->_acl->addResource($resourceFoo); $this->_acl->setRule(Acl\Acl::OP_ADD, Acl\Acl::TYPE_ALLOW, $roleGuest, $resourceFoo); - - - } } From d2df3f240b7c92a98f2f03e2bf1d9dff46ac620b Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 22 Jan 2013 06:24:54 -0600 Subject: [PATCH 9/9] [zendframework/zf2#3513] CS fixes --- test/AclTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/AclTest.php b/test/AclTest.php index 1df99eb..d4b0317 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -1329,8 +1329,8 @@ public function testAclResourcePermissionsAreInheritedWithMultilevelResourcesAnd $this->assertFalse($this->_acl->isAllowed('guest', 'post_2', 'comment')); } - public function testSetRuleWorksWithResourceInterface() { - + public function testSetRuleWorksWithResourceInterface() + { $roleGuest = new Role\GenericRole('guest'); $this->_acl->addRole($roleGuest);