From c10ad55e50f402bf14eb2eb9bc424dd9a44dfc78 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 1 Feb 2018 10:55:50 +0100 Subject: [PATCH 01/31] Updated composer for release 2.6.0 --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 34917005..d9ed34ac 100644 --- a/composer.json +++ b/composer.json @@ -19,8 +19,8 @@ "prefer-stable": true, "extra": { "branch-alias": { - "dev-master": "2.5-dev", - "dev-develop": "2.6-dev" + "dev-master": "2.6-dev", + "dev-develop": "2.7-dev" } }, "autoload-dev": { From 1f8a4149e954951a63d6d4ea7e295373f438274c Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 1 Feb 2018 11:12:23 +0100 Subject: [PATCH 02/31] Updated CHANGELOG --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2161d1cf..6f6dfa1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. +## 2.6.1 - TBD + +### Added + +- Nothing. + +### Changed + +- Nothing. + +### Deprecated + +- Nothing. + +### Removed + +- Nothing. + +### Fixed + +- Nothing. + ## 2.6.0 - 2018-02-01 ### Added From ad2ca66fe415e6ead448bcac8aefb157c4447dc6 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 17:20:32 +0100 Subject: [PATCH 03/31] Refactoring RBAC for version 3.0.0 with BC breaks --- LICENSE.md | 2 +- composer.json | 2 +- composer.lock | 36 ++-- doc/book/examples.md | 4 +- doc/book/index.html | 2 +- doc/book/intro.md | 7 +- doc/book/methods.md | 61 +++---- phpunit.xml.dist | 12 +- src/AbstractIterator.php | 106 ------------ src/AbstractRole.php | 145 ---------------- src/Assertion/CallbackAssertion.php | 11 +- src/AssertionInterface.php | 8 +- src/Exception/ExceptionInterface.php | 2 +- src/Exception/InvalidArgumentException.php | 2 +- src/Exception/RuntimeException.php | 15 ++ src/Rbac.php | 125 +++++++------- src/Role.php | 182 ++++++++++++++++++++- src/RoleInterface.php | 31 ++-- test/Assertion/CallbackAssertionTest.php | 2 +- test/RbacTest.php | 83 ++++++---- test/RoleTest.php | 123 ++++++++++++++ test/TestAsset/RoleMustMatchAssertion.php | 24 +-- test/TestAsset/RoleTest.php | 13 +- test/TestAsset/SimpleFalseAssertion.php | 12 +- test/TestAsset/SimpleTrueAssertion.php | 12 +- test/bootstrap.php | 23 --- 26 files changed, 525 insertions(+), 520 deletions(-) delete mode 100644 src/AbstractIterator.php delete mode 100644 src/AbstractRole.php create mode 100644 src/Exception/RuntimeException.php create mode 100644 test/RoleTest.php delete mode 100644 test/bootstrap.php diff --git a/LICENSE.md b/LICENSE.md index dbb1b49c..67c914a6 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -1,4 +1,4 @@ -Copyright (c) 2005-2015, Zend Technologies USA, Inc. +Copyright (c) 2005-2018, Zend Technologies USA, Inc. All rights reserved. diff --git a/composer.json b/composer.json index d9ed34ac..59f8f5c3 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ } }, "require-dev": { - "phpunit/phpunit": "^5.7.15|| ^6.2.1", + "phpunit/phpunit": "^5.7.25 || ^6.4.4", "zendframework/zend-coding-standard": "~1.0.0" }, "scripts": { diff --git a/composer.lock b/composer.lock index dac7565f..8ff30eaf 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "2daefbdcd3df58ce5d4db6cd1e272273", + "content-hash": "f23d49d16a6176c8fc111967646915b3", "packages": [], "packages-dev": [ { @@ -362,16 +362,16 @@ }, { "name": "phpspec/prophecy", - "version": "1.7.3", + "version": "1.7.4", "source": { "type": "git", "url": "https://github.com/phpspec/prophecy.git", - "reference": "e4ed002c67da8eceb0eb8ddb8b3847bb53c5c2bf" + "reference": "9f901e29c93dae4aa77c0bb161df4276f9c9a1be" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpspec/prophecy/zipball/e4ed002c67da8eceb0eb8ddb8b3847bb53c5c2bf", - "reference": "e4ed002c67da8eceb0eb8ddb8b3847bb53c5c2bf", + "url": "https://api.github.com/repos/phpspec/prophecy/zipball/9f901e29c93dae4aa77c0bb161df4276f9c9a1be", + "reference": "9f901e29c93dae4aa77c0bb161df4276f9c9a1be", "shasum": "" }, "require": { @@ -383,7 +383,7 @@ }, "require-dev": { "phpspec/phpspec": "^2.5|^3.2", - "phpunit/phpunit": "^4.8.35 || ^5.7" + "phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.5" }, "type": "library", "extra": { @@ -421,7 +421,7 @@ "spy", "stub" ], - "time": "2017-11-24T13:59:53+00:00" + "time": "2018-02-11T18:49:29+00:00" }, { "name": "phpunit/php-code-coverage", @@ -674,16 +674,16 @@ }, { "name": "phpunit/phpunit", - "version": "6.5.5", + "version": "6.5.6", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/phpunit.git", - "reference": "83d27937a310f2984fd575686138597147bdc7df" + "reference": "3330ef26ade05359d006041316ed0fa9e8e3cefe" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/83d27937a310f2984fd575686138597147bdc7df", - "reference": "83d27937a310f2984fd575686138597147bdc7df", + "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/3330ef26ade05359d006041316ed0fa9e8e3cefe", + "reference": "3330ef26ade05359d006041316ed0fa9e8e3cefe", "shasum": "" }, "require": { @@ -754,7 +754,7 @@ "testing", "xunit" ], - "time": "2017-12-17T06:31:19+00:00" + "time": "2018-02-01T05:57:37+00:00" }, { "name": "phpunit/phpunit-mock-objects", @@ -862,21 +862,21 @@ }, { "name": "sebastian/comparator", - "version": "2.1.2", + "version": "2.1.3", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/comparator.git", - "reference": "11c07feade1d65453e06df3b3b90171d6d982087" + "reference": "34369daee48eafb2651bea869b4b15d75ccc35f9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/comparator/zipball/11c07feade1d65453e06df3b3b90171d6d982087", - "reference": "11c07feade1d65453e06df3b3b90171d6d982087", + "url": "https://api.github.com/repos/sebastianbergmann/comparator/zipball/34369daee48eafb2651bea869b4b15d75ccc35f9", + "reference": "34369daee48eafb2651bea869b4b15d75ccc35f9", "shasum": "" }, "require": { "php": "^7.0", - "sebastian/diff": "^2.0", + "sebastian/diff": "^2.0 || ^3.0", "sebastian/exporter": "^3.1" }, "require-dev": { @@ -922,7 +922,7 @@ "compare", "equality" ], - "time": "2018-01-12T06:34:42+00:00" + "time": "2018-02-01T13:46:46+00:00" }, { "name": "sebastian/diff", diff --git a/doc/book/examples.md b/doc/book/examples.md index 48cbc4d8..6a3649ba 100644 --- a/doc/book/examples.md +++ b/doc/book/examples.md @@ -83,7 +83,7 @@ Checking permission using `isGranted()` with a class implementing use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; -class AssertUserIdMatches implements AssertionInterface +class AssertUserRoleMatches implements AssertionInterface { protected $userId; protected $article; @@ -98,7 +98,7 @@ class AssertUserIdMatches implements AssertionInterface $this->article = $article; } - public function assert(Rbac $rbac) + public function assert(Rbac $rbac, $permission = null, $role = null) { if (! $this->article) { return false; diff --git a/doc/book/index.html b/doc/book/index.html index 07c8a528..4337842e 100644 --- a/doc/book/index.html +++ b/doc/book/index.html @@ -3,7 +3,7 @@

zend-permissions-rbac

- Provide and query Role-Based Access Controls for your application. + Provide and query Role-Based Access Controls (RBAC) for your application. 

$ composer require zendframework/zend-permissions-rbac
diff --git a/doc/book/intro.md b/doc/book/intro.md index ba94f30f..d5d830bc 100644 --- a/doc/book/intro.md +++ b/doc/book/intro.md @@ -1,8 +1,7 @@ # Introduction -zend-permissions-rbac provides a lightweight role-based access control (RBAC) -implementation based around PHP's `RecursiveIterator` and -`RecursiveIteratorIterator`. RBAC differs from access control lists (ACL) by +`zend-permissions-rbac` provides a lightweight [Role-based access control](https://it.wikipedia.org/wiki/Role-based_access_control) +(RBAC) implementation in PHP. RBAC differs from access control lists (ACL) by putting the emphasis on roles and their permissions rather than objects (resources). @@ -16,7 +15,7 @@ Thus, RBAC has the following model: - many to many relationship between **identities** and **roles**. - many to many relationship between **roles** and **permissions**. -- **roles** can have a parent role. +- **roles** can have parent and children roles (hierarchy of roles). ## Roles diff --git a/doc/book/methods.md b/doc/book/methods.md index c3d086c9..397d60f7 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -1,62 +1,39 @@ # Methods -## `Zend\Permissions\Rbac\AbstractIterator` - -The `AbstractIterator` is used as the basis for both the primary `Rbac` class -and the `AbstractRole`. - -Method signature | Description ------------------------------------ | ----------- -`current() : RoleInterface` | Return the current role instance. -`getChildren() : RecursiveIterator` | Returns a recursive iterator of all children of the current role. -`hasChildren() : bool` | Indicates if the current role has children. -`key() : int` | Index of the current role instance. -`next() : void` | Advance to the next role instance. -`rewind() : void` | Seek to the first item in the iterator. -`valid() : bool` | Is the current index valid? - -## `Zend\Permissions\Rbac\AbstractRole` - -The `AbstractRole` provides the base functionality required by the -`RoleInterface`, and is the foundation for the `Role` class. - -Method signature | Description ----------------------------------------------- | ----------- -`addChild(string|RoleInterface $child) : void` | Add a child role to the current instance. -`addPermission(string $name) : void` | Add a permission for the current role. -`getName() : string` | Retrieve the name assigned to this role. -`hasPermission(string $name) : bool` | Does the role have the given permission? -`setParent(RoleInterface $parent) : void` | Assign the provided role as the current role's parent. -`addParent(RoleInterface $parent) : Role` | Add a parent role to the current instance. -`getParent() null|RoleInterface|array` | Retrieve the current role's parent, or array of parents if more that one exists. +## `Zend\Permissions\Rbac\Role` + +The `Role` provides the base functionality required by the `RoleInterface`. + +Method signature | Description +------------------------------------------| ----------- +`__construct(string $name) : void` | Create a new instance with the provided name. +`getName() : string` | Retrieve the name assigned to this role. +`addPermission(string $name) : void` | Add a permission for the current role. +`hasPermission(string $name) : bool` | Does the role have the given permission? +`addChild(RoleInterface $child) : Role` | Add a child role to the current instance. +`getChildrens() : RoleInterface[]` | Get all the children roles. +`addParent(RoleInterface $parent) : Role` | Add a parent role to the current instance. +`getParents() : RoleInterface[]` | Get all the parent roles. ## `Zend\Permissions\Rbac\AssertionInterface` Custom assertions can be provided to `Rbac::isGranted()` (see below); such assertions are provided the `Rbac` instance on invocation. -Method signature | Description ---------------------------- | ----------- -`assert(Rbac $rbac) : bool` | Given an RBAC, determine if permission is granted. +Method signature | Description +--------------------------------------------------------------------------- | ----------- +`assert(Rbac $rbac, string $permission = null, RoleInterface $role = null)` | Given an RBAC, an optional permission, and optional role determine if permission is granted. ## `Zend\Permissions\Rbac\Rbac` `Rbac` is the object with which you will interact within your application in -order to query for permissions. It extends `AbstractIterator`. +order to query for permissions. Method signature | Description --------------------------------------------------------------------------- | ----------- `addRole(string|RoleInterface $child, array|RoleInterface $parents = null)` | Add a role to the RBAC. If `$parents` is non-null, the `$child` is also added to any parents provided. -`getRole(string|RoleInterface $role) : RoleInterface` | Recursively queries the RBAC for the given role, returning it if found, and raising an exception otherwise. +`getRole(string $role) : RoleInterface` | Get the role specified by name, raising an exception if not found. `hasRole(string|RoleInterface $role) : bool` | Recursively queries the RBAC for the given role, returning `true` if found, `false` otherwise. `getCreateMissingRoles() : bool` | Retrieve the flag that determines whether or not `$parent` roles are added automatically if not present when calling `addRole()`. `setCreateMissingRoles(bool $flag) : void` | Set the flag that determines whether or not `$parent` roles are added automatically if not present when calling `addRole()`. `isGranted(string|RoleInterface $role, string $permission, $assert = null)` | Determine if the role has the given permission. If `$assert` is provided and either an `AssertInterface` instance or callable, it will be queried before checking against the given role. - -## `Zend\Permissions\Rbac\Role` - -`Role` inherits from `AbstractRole` and `AbstractIterator`. - -Method signature | Description ----------------------------------- | ----------- -`__construct(string $name) : void` | Create a new instance with the provided name. diff --git a/phpunit.xml.dist b/phpunit.xml.dist index c709b112..09d524cd 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,7 +1,7 @@ @@ -20,14 +20,4 @@ ./src - - - - - - - diff --git a/src/AbstractIterator.php b/src/AbstractIterator.php deleted file mode 100644 index 2f11bf0b..00000000 --- a/src/AbstractIterator.php +++ /dev/null @@ -1,106 +0,0 @@ - - * Return the current element - * @link http://php.net/manual/en/iterator.current.php - * @return mixed Can return any type. - */ - public function current() - { - return $this->children[$this->index]; - } - - /** - * (PHP 5 >= 5.0.0)
- * Move forward to next element - * @link http://php.net/manual/en/iterator.next.php - * @return void Any returned value is ignored. - */ - public function next() - { - $this->index++; - } - - /** - * (PHP 5 >= 5.0.0)
- * Return the key of the current element - * @link http://php.net/manual/en/iterator.key.php - * @return int|null scalar on success, or null on failure. - */ - public function key() - { - return $this->index; - } - - /** - * (PHP 5 >= 5.0.0)
- * Checks if current position is valid - * @link http://php.net/manual/en/iterator.valid.php - * @return bool The return value will be casted to boolean and then evaluated. - * Returns true on success or false on failure. - */ - public function valid() - { - return isset($this->children[$this->index]); - } - - /** - * (PHP 5 >= 5.0.0)
- * Rewind the Iterator to the first element - * @link http://php.net/manual/en/iterator.rewind.php - * @return void Any returned value is ignored. - */ - public function rewind() - { - $this->index = 0; - } - - /** - * (PHP 5 >= 5.1.0)
- * Returns if an iterator can be created fot the current entry. - * @link http://php.net/manual/en/recursiveiterator.haschildren.php - * @return bool true if the current entry can be iterated over, otherwise returns false. - */ - public function hasChildren() - { - if ($this->valid() && ($this->current() instanceof RecursiveIterator)) { - return true; - } - - return false; - } - - /** - * (PHP 5 >= 5.1.0)
- * Returns an iterator for the current entry. - * @link http://php.net/manual/en/recursiveiterator.getchildren.php - * @return RecursiveIterator An iterator for the current entry. - */ - public function getChildren() - { - return $this->children[$this->index]; - } -} diff --git a/src/AbstractRole.php b/src/AbstractRole.php deleted file mode 100644 index 26b05ea4..00000000 --- a/src/AbstractRole.php +++ /dev/null @@ -1,145 +0,0 @@ -name; - } - - /** - * Add permission to the role. - * - * @param $name - * @return RoleInterface - */ - public function addPermission($name) - { - $this->permissions[$name] = true; - - return $this; - } - - /** - * Checks if a permission exists for this role or any child roles. - * - * @param string $name - * @return bool - */ - public function hasPermission($name) - { - if (isset($this->permissions[$name])) { - return true; - } - - $it = new RecursiveIteratorIterator($this, RecursiveIteratorIterator::CHILD_FIRST); - foreach ($it as $leaf) { - /** @var RoleInterface $leaf */ - if ($leaf->hasPermission($name)) { - return true; - } - } - - return false; - } - - /** - * Add a child. - * - * @param RoleInterface|string $child - * @return Role - */ - public function addChild($child) - { - if (is_string($child)) { - $child = new Role($child); - } - if (! $child instanceof RoleInterface) { - throw new Exception\InvalidArgumentException( - 'Child must be a string or implement Zend\Permissions\Rbac\RoleInterface' - ); - } - if (! in_array($child, $this->children, true)) { - $this->children[] = $child; - $child->setParent($this); - } - return $this; - } - - /** - * @deprecated deprecated since version 2.6.0, use addParent() instead - * - * @param RoleInterface $parent - * @return RoleInterface - */ - public function setParent($parent) - { - return $this->addParent($parent); - } - - /** - * @return null|RoleInterface|array - */ - public function getParent() - { - if (null === $this->parents) { - return; - } - if (1 === count($this->parents)) { - return $this->parents[0]; - } - return $this->parents; - } - - /** - * @param RoleInterface $parent - * @return RoleInterface - */ - public function addParent($parent) - { - if (! $parent instanceof RoleInterface) { - throw new Exception\InvalidArgumentException( - 'Parent must implement Zend\Permissions\Rbac\RoleInterface' - ); - } - if (null === $this->parents) { - $this->parents = []; - } - if (! in_array($parent, $this->parents, true)) { - $this->parents[] = $parent; - $parent->addChild($this); - } - return $this; - } -} diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index b3a64a1e..30ab2fd8 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/). * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ namespace Zend\Permissions\Rbac\Assertion; @@ -31,14 +31,9 @@ public function __construct($callback) } /** - * Assertion method - must return a boolean. - * - * Returns the result of the composed callback. - * - * @param Rbac $rbac - * @return bool + * {@inheritDoc} */ - public function assert(Rbac $rbac) + public function assert(Rbac $rbac, $permission = null, $role = null) { return (bool) call_user_func($this->callback, $rbac); } diff --git a/src/AssertionInterface.php b/src/AssertionInterface.php index cddff258..e9b6ed5d 100644 --- a/src/AssertionInterface.php +++ b/src/AssertionInterface.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ @@ -14,8 +14,10 @@ interface AssertionInterface /** * Assertion method - must return a boolean. * - * @param Rbac $rbac + * @param Rbac $rbac + * @param string $permission + * @param RoleInterface $role * @return bool */ - public function assert(Rbac $rbac); + public function assert(Rbac $rbac, $permission = null, $role = null); } diff --git a/src/Exception/ExceptionInterface.php b/src/Exception/ExceptionInterface.php index 431c4d5e..022e31be 100644 --- a/src/Exception/ExceptionInterface.php +++ b/src/Exception/ExceptionInterface.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index c2708ae4..f3caaea7 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ diff --git a/src/Exception/RuntimeException.php b/src/Exception/RuntimeException.php new file mode 100644 index 00000000..2aeab92c --- /dev/null +++ b/src/Exception/RuntimeException.php @@ -0,0 +1,15 @@ +createMissingRoles && ! $this->hasRole($parent)) { $this->addRole($parent); } - $this->getRole($parent)->addChild($child); + if (is_string($parent)) { + $parent = $this->getRole($parent); + } + $parent->addChild($role); } } - - $this->children[] = $child; - + $this->roles[$role->getName()] = $role; return $this; } /** - * Is a child with $name registered? + * Is a role registered? * - * @param \Zend\Permissions\Rbac\RoleInterface|string $objectOrName + * @param RoleInterface|string $role * @return bool */ - public function hasRole($objectOrName) + public function hasRole($role) { - try { - $this->getRole($objectOrName); - - return true; - } catch (Exception\InvalidArgumentException $e) { - return false; + if (! is_string($role) && ! $role instanceof RoleInterface) { + throw new Exception\InvalidArgumentException( + 'Role must be a string or implement Zend\Permissions\Rbac\RoleInterface' + ); + } + if (is_string($role)) { + return isset($this->roles[$role]); } + $roleName = $role->getName(); + return isset($this->roles[$roleName]) && + $this->roles[$roleName] === $role; } /** - * Get a child. + * Get a registered role by name * - * @param \Zend\Permissions\Rbac\RoleInterface|string $objectOrName + * @param string $roleName * @return RoleInterface * @throws Exception\InvalidArgumentException */ - public function getRole($objectOrName) + public function getRole($roleName) { - if (! is_string($objectOrName) && ! $objectOrName instanceof RoleInterface) { + if (! is_string($roleName)) { throw new Exception\InvalidArgumentException( - 'Expected string or implement \Zend\Permissions\Rbac\RoleInterface' + 'Role name must be a string' ); } - - if (is_object($objectOrName)) { - $requiredRole = $objectOrName->getName(); - } else { - $requiredRole = $objectOrName; - } - - $it = new RecursiveIteratorIterator($this, RecursiveIteratorIterator::CHILD_FIRST); - foreach ($it as $leaf) { - /** @var RoleInterface $leaf */ - if ($leaf->getName() == $requiredRole) { - return $leaf; - } + if (! isset($this->roles[$roleName])) { + throw new Exception\InvalidArgumentException(sprintf( + 'No role with name "%s" could be found', + $roleName + )); } - - throw new Exception\InvalidArgumentException(sprintf( - 'No role with name "%s" could be found', - is_object($objectOrName) ? $objectOrName->getName() : $objectOrName - )); + return $this->roles[$roleName]; } /** @@ -139,22 +135,27 @@ public function getRole($objectOrName) */ public function isGranted($role, $permission, $assert = null) { - $result = $this->getRole($role)->hasPermission($permission); - - if ($assert) { - if ($assert instanceof AssertionInterface) { - return $result && $assert->assert($this); - } - - if (is_callable($assert)) { - return $result && $assert($this); - } - - throw new Exception\InvalidArgumentException( - 'Assertions must be a Callable or an instance of Zend\Permissions\Rbac\AssertionInterface' - ); + if (! $this->hasRole($role)) { + throw new Exception\InvalidArgumentException(sprintf( + 'No role with name "%s" could be found', + is_object($role) ? $role->getName() : $role + )); } - - return $result; + if (is_string($role)) { + $role = $this->getRole($role); + } + $result = $role->hasPermission($permission); + if (false === $result || null === $assert) { + return $result; + } + if ($assert instanceof AssertionInterface) { + return $result && $assert->assert($this, $permission, $role); + } + if (is_callable($assert)) { + return $result && $assert($this, $permission, $role); + } + throw new Exception\InvalidArgumentException( + 'Assertions must be a Callable or an instance of Zend\Permissions\Rbac\AssertionInterface' + ); } } diff --git a/src/Role.php b/src/Role.php index 18114a41..eee25d7b 100644 --- a/src/Role.php +++ b/src/Role.php @@ -3,14 +3,34 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ namespace Zend\Permissions\Rbac; -class Role extends AbstractRole +class Role implements RoleInterface { + /** + * @var RoleInterface[] + */ + protected $childrens = []; + + /** + * @var RoleInterface[] + */ + protected $parents = []; + + /** + * @var string + */ + protected $name; + + /** + * @var array + */ + protected $permissions = []; + /** * @param string $name */ @@ -18,4 +38,162 @@ public function __construct($name) { $this->name = $name; } + + /** + * Get the name of the role. + * + * @return string + */ + public function getName() + { + return $this->name; + } + + /** + * Add permission to the role. + * + * @param $name + * @return RoleInterface + */ + public function addPermission($name) + { + if (! is_string($name)) { + throw new Exception\InvalidArgumentException( + 'The permission name must be a string' + ); + } + $this->permissions[$name] = true; + return $this; + } + + /** + * Checks if a permission exists for this role or any child roles. + * + * @param string $name + * @return bool + */ + public function hasPermission($name) + { + if (isset($this->permissions[$name])) { + return true; + } + foreach ($this->childrens as $child) { + if ($child->hasPermission($name)) { + return true; + } + } + return false; + } + + /** + * Add a child + * + * @param RoleInterface $child + * @return RoleInterface + */ + public function addChild(RoleInterface $child) + { + if (! $child instanceof RoleInterface) { + throw new Exception\InvalidArgumentException( + 'Child must implement Zend\Permissions\Rbac\RoleInterface' + ); + } + $childName = $child->getName(); + if ($this->hasAncestor($child)) { + throw new Exception\RuntimeException(sprintf( + "To prevent circular references, you cannot add role '%s' as child", + $childName + )); + } + if (! isset($this->childrens[$childName])) { + $this->childrens[$childName] = $child; + $child->addParent($this); + } + return $this; + } + + /** + * Check if a role is an ancestor + * + * @param RoleInterface $role + * @return bool + */ + protected function hasAncestor(RoleInterface $role) + { + if (isset($this->parents[$role->getName()])) { + return true; + } + foreach ($this->parents as $parent) { + if ($parent->hasAncestor($role)) { + return true; + } + } + return false; + } + + /** + * Get the children roles + * + * @return RoleInterface[] + */ + public function getChildrens() + { + return array_values($this->childrens); + } + + /** + * Add a parent role + * + * @param RoleInterface $parent + * @return RoleInterface + */ + public function addParent(RoleInterface $parent) + { + if (! $parent instanceof RoleInterface) { + throw new Exception\InvalidArgumentException( + 'Parent must implement Zend\Permissions\Rbac\RoleInterface' + ); + } + $parentName = $parent->getName(); + if ($this->hasDescendant($parent)) { + throw new Exception\RuntimeException(sprintf( + "To prevent circular references, you cannot add role '%s' as parent", + $parentName + )); + } + if (! isset($this->parents[$parentName])) { + $this->parents[$parentName] = $parent; + $parent->addChild($this); + } + return $this; + } + + /** + * Check if a role is a descendant + * + * @param RoleInterface $role + * @return bool + */ + protected function hasDescendant(RoleInterface $role) + { + if (isset($this->childrens[$role->getName()])) { + return true; + } + foreach ($this->childrens as $child) { + if ($child->hasDescendant($role)) { + return true; + } + } + return false; + } + + /** + * Get the parent roles + * + * @return RoleInterface[] + */ + public function getParents() + { + return array_values($this->parents); + } } diff --git a/src/RoleInterface.php b/src/RoleInterface.php index e789c53f..5c103818 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -3,15 +3,13 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ namespace Zend\Permissions\Rbac; -use RecursiveIterator; - -interface RoleInterface extends RecursiveIterator +interface RoleInterface { /** * Get the name of the role. @@ -39,19 +37,30 @@ public function hasPermission($name); /** * Add a child. * - * @param RoleInterface|string $child - * @return Role + * @param RoleInterface $child + * @return RoleInterface + */ + public function addChild(RoleInterface $child); + + /** + * Get the children roles. + * + * @return RoleInterface[] */ - public function addChild($child); + public function getChildrens(); /** - * @param RoleInterface $parent + * Add a parent. + * + * @param RoleInterface $parent * @return RoleInterface */ - public function setParent($parent); + public function addParent(RoleInterface $parent); /** - * @return null|RoleInterface|array + * Get the parent roles. + * + * @return RoleInterface[] */ - public function getParent(); + public function getParents(); } diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 43ddf3b5..639bef1a 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ namespace ZendTest\Permissions\Rbac\Assertion; diff --git a/test/RbacTest.php b/test/RbacTest.php index 8247ad69..a940ada9 100644 --- a/test/RbacTest.php +++ b/test/RbacTest.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ @@ -11,10 +11,8 @@ use PHPUnit\Framework\TestCase; use Zend\Permissions\Rbac; +use Zend\Permissions\Rbac\Exception; -/** - * @group Zend_Rbac - */ class RbacTest extends TestCase { /** @@ -86,6 +84,13 @@ public function testIsGrantedChildRoles() $this->assertEquals(false, $this->rbac->isGranted('bar', 'can.baz')); } + public function testGetRole() + { + $foo = new Rbac\Role('foo'); + $this->rbac->addRole($foo); + $this->assertEquals($foo, $this->rbac->getRole('foo')); + } + /** * @covers Zend\Permissions\Rbac\Rbac::hasRole() */ @@ -107,9 +112,10 @@ public function testHasRole() // check that the container do not have the string "baz" $this->assertFalse($this->rbac->hasRole('baz')); - // check that we can compare two different objects with same name + // check that 'snafu' role and $snafu are different $this->assertNotEquals($this->rbac->getRole('snafu'), $snafu); - $this->assertTrue($this->rbac->hasRole($snafu)); + $this->assertTrue($this->rbac->hasRole('snafu')); + $this->assertFalse($this->rbac->hasRole($snafu)); } public function testAddRoleFromString() @@ -131,6 +137,13 @@ public function testAddRoleFromClass() $this->assertInstanceOf('Zend\Permissions\Rbac\Role', $foo2); } + public function testAddRoleNotValid() + { + $foo = new \stdClass(); + $this->expectException(Exception\InvalidArgumentException::class); + $this->rbac->addRole($foo); + } + public function testAddRoleWithParentsUsingRbac() { $foo = new Rbac\Role('foo'); @@ -139,8 +152,8 @@ public function testAddRoleWithParentsUsingRbac() $this->rbac->addRole($foo); $this->rbac->addRole($bar, $foo); - $this->assertEquals($bar->getParent(), $foo); - $this->assertEquals($bar, $foo->getChildren()); + $this->assertEquals($bar->getParents(), [$foo]); + $this->assertEquals([$bar], $foo->getChildrens()); } @@ -150,10 +163,11 @@ public function testAddRoleWithAutomaticParentsUsingRbac() $bar = new Rbac\Role('bar'); $this->rbac->setCreateMissingRoles(true); + $this->assertTrue($this->rbac->getCreateMissingRoles()); $this->rbac->addRole($bar, $foo); - $this->assertEquals($bar->getParent(), $foo); - $this->assertEquals($bar, $foo->getChildren()); + $this->assertEquals($bar->getParents(), [$foo]); + $this->assertEquals([$bar], $foo->getChildrens()); } /** @@ -194,15 +208,15 @@ public function testAddMultipleParentRole() $viewerRole->addPermission('post.view'); $this->rbac->addRole($viewerRole, ['Editor', 'Manager']); - $this->assertEquals('Viewer', $editorRole->getChildren()->getName()); - $this->assertEquals('Viewer', $managerRole->getChildren()->getName()); + $this->assertEquals('Viewer', $editorRole->getChildrens()[0]->getName()); + $this->assertEquals('Viewer', $managerRole->getChildrens()[0]->getName()); $this->assertTrue($this->rbac->isGranted('Editor', 'post.view')); $this->assertTrue($this->rbac->isGranted('Manager', 'post.view')); - $this->assertEquals($viewerRole->getParent(), [ $editorRole, $managerRole ]); - $this->assertEquals($managerRole->getParent(), $adminRole); - $this->assertNull($editorRole->getParent()); - $this->assertNull($adminRole->getParent()); + $this->assertEquals($viewerRole->getParents(), [$editorRole, $managerRole]); + $this->assertEquals($managerRole->getParents(), [$adminRole]); + $this->assertEmpty($editorRole->getParents()); + $this->assertEmpty($adminRole->getParents()); } public function testAddParentRole() @@ -226,15 +240,29 @@ public function testAddParentRole() $viewerRole->addParent($managerRole); $this->rbac->addRole($viewerRole); - $this->assertEquals('Viewer', $editorRole->getChildren()->getName()); - $this->assertEquals('Viewer', $managerRole->getChildren()->getName()); + // Check roles hierarchy + $this->assertEquals([$viewerRole], $editorRole->getChildrens()); + $this->assertEquals([$viewerRole], $managerRole->getChildrens()); + $this->assertEquals($viewerRole->getParents(), [$editorRole, $managerRole]); + $this->assertEquals($managerRole->getParents(), [$adminRole]); + $this->assertEmpty($editorRole->getParents()); + $this->assertEmpty($adminRole->getParents()); + + // Check permissions $this->assertTrue($this->rbac->isGranted('Editor', 'post.view')); + $this->assertTrue($this->rbac->isGranted('Editor', 'post.edit')); + $this->assertTrue($this->rbac->isGranted('Viewer', 'post.view')); $this->assertTrue($this->rbac->isGranted('Manager', 'post.view')); - - $this->assertEquals($viewerRole->getParent(), [ $editorRole, $managerRole ]); - $this->assertEquals($managerRole->getParent(), $adminRole); - $this->assertNull($editorRole->getParent()); - $this->assertNull($adminRole->getParent()); + $this->assertTrue($this->rbac->isGranted('Administrator', 'post.view')); + $this->assertTrue($this->rbac->isGranted('Administrator', 'post.publish')); + $this->assertFalse($this->rbac->isGranted('Administrator', 'post.edit')); + $this->assertFalse($this->rbac->isGranted('Manager', 'post.edit')); + $this->assertFalse($this->rbac->isGranted('Viewer', 'post.edit')); + $this->assertFalse($this->rbac->isGranted('Viewer', 'post.publish')); + $this->assertFalse($this->rbac->isGranted('Viewer', 'user.manage')); + $this->assertFalse($this->rbac->isGranted('Editor', 'user.manage')); + $this->assertFalse($this->rbac->isGranted('Editor', 'post.publish')); + $this->assertFalse($this->rbac->isGranted('Manager', 'user.manage')); } public function testAddTwoChildRole() @@ -246,11 +274,8 @@ public function testAddTwoChildRole() $foo->addChild($bar); $foo->addChild($baz); - $this->assertEquals($foo, $bar->getParent()); - $this->assertEquals($bar, $foo->getChildren()); - $foo->next(); - $this->assertEquals($foo, $baz->getParent()); - $this->assertEquals($baz, $foo->getChildren()); + $this->assertEquals([$foo], $bar->getParents()); + $this->assertEquals([$bar, $baz], $foo->getChildrens()); } public function testAddSameParent() @@ -261,6 +286,6 @@ public function testAddSameParent() $foo->addParent($bar); $foo->addParent($bar); - $this->assertEquals($bar, $foo->getParent()); + $this->assertEquals([$bar], $foo->getParents()); } } diff --git a/test/RoleTest.php b/test/RoleTest.php new file mode 100644 index 00000000..04946d57 --- /dev/null +++ b/test/RoleTest.php @@ -0,0 +1,123 @@ +assertInstanceOf(RoleInterface::class, $foo); + } + + public function testGetName() + { + $foo = new Role('foo'); + $this->assertEquals('foo', $foo->getName()); + } + + public function testAddPermission() + { + $foo = new Role('foo'); + $this->assertInstanceOf(RoleInterface::class, $foo->addPermission('bar')); + $this->assertInstanceOf(RoleInterface::class, $foo->addPermission('baz')); + $this->assertTrue($foo->hasPermission('bar')); + $this->assertTrue($foo->hasPermission('baz')); + } + + public function testInvalidPermission() + { + $perm = new \stdClass(); + $foo = new Role('foo'); + $this->expectException(Exception\InvalidArgumentException::class); + $foo->addPermission($perm); + } + + public function testAddChild() + { + $foo = new Role('foo'); + $bar = new Role('bar'); + $baz = new Role('baz'); + + $this->assertInstanceOf(RoleInterface::class, $foo->addChild($bar)); + $this->assertInstanceOf(RoleInterface::class, $foo->addChild($baz)); + $this->assertEquals($foo->getChildrens(), [$bar, $baz]); + } + + public function testAddParent() + { + $foo = new Role('foo'); + $bar = new Role('bar'); + $baz = new Role('baz'); + + $this->assertInstanceOf(RoleInterface::class, $foo->addParent($bar)); + $this->assertInstanceOf(RoleInterface::class, $foo->addParent($baz)); + $this->assertEquals($foo->getParents(), [$bar, $baz]); + } + + public function testPermissionHierarchy() + { + $foo = new Role('foo'); + $foo->addPermission('foo.permission'); + + $bar = new Role('bar'); + $bar->addPermission('bar.permission'); + + $baz = new Role('baz'); + $baz->addPermission('baz.permission'); + + // create hierarchy bar -> foo -> baz + $foo->addParent($bar); + $foo->addChild($baz); + + $this->assertTrue($bar->hasPermission('bar.permission')); + $this->assertTrue($bar->hasPermission('foo.permission')); + $this->assertTrue($bar->hasPermission('baz.permission')); + + $this->assertFalse($foo->hasPermission('bar.permission')); + $this->assertTrue($foo->hasPermission('foo.permission')); + $this->assertTrue($foo->hasPermission('baz.permission')); + + $this->assertFalse($baz->hasPermission('foo.permission')); + $this->assertFalse($baz->hasPermission('bar.permission')); + $this->assertTrue($baz->hasPermission('baz.permission')); + } + + public function testCircleReferenceWithChild() + { + $foo = new Role('foo'); + $bar = new Role('bar'); + $baz = new Role('baz'); + $baz->addPermission('baz'); + + $foo->addChild($bar); + $bar->addChild($baz); + $this->expectException(Exception\RuntimeException::class); + $baz->addChild($foo); + } + + public function testCircleReferenceWithParent() + { + $foo = new Role('foo'); + $bar = new Role('bar'); + $baz = new Role('baz'); + $baz->addPermission('baz'); + + $foo->addParent($bar); + $bar->addParent($baz); + $this->expectException(Exception\RuntimeException::class); + $baz->addParent($foo); + } +} diff --git a/test/TestAsset/RoleMustMatchAssertion.php b/test/TestAsset/RoleMustMatchAssertion.php index 387f4715..ed997700 100644 --- a/test/TestAsset/RoleMustMatchAssertion.php +++ b/test/TestAsset/RoleMustMatchAssertion.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ @@ -13,29 +13,13 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; -/** - * @group Zend_Rbac - */ class RoleMustMatchAssertion implements AssertionInterface { /** - * @var AbstractRole - */ - protected $role; - - public function __construct(AbstractRole $role) - { - $this->role = $role; - } - - /** - * Assertion method - must return a boolean. - * - * @param Rbac $rbac - * @return bool + * {@inheritDoc} */ - public function assert(Rbac $rbac) + public function assert(Rbac $rbac, $permission = null, $role = null) { - return $this->role->getName() == 'foo'; + return $role->getName() === 'foo'; } } diff --git a/test/TestAsset/RoleTest.php b/test/TestAsset/RoleTest.php index b5cdf75d..6d71dd15 100644 --- a/test/TestAsset/RoleTest.php +++ b/test/TestAsset/RoleTest.php @@ -3,21 +3,14 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ namespace ZendTest\Permissions\Rbac\TestAsset; -use Zend\Permissions\Rbac\AbstractRole; +use Zend\Permissions\Rbac\Role; -class RoleTest extends AbstractRole +class RoleTest extends Role { - /** - * @param string $name - */ - public function __construct($name) - { - $this->name = $name; - } } diff --git a/test/TestAsset/SimpleFalseAssertion.php b/test/TestAsset/SimpleFalseAssertion.php index 0fb41bf2..63297c32 100644 --- a/test/TestAsset/SimpleFalseAssertion.php +++ b/test/TestAsset/SimpleFalseAssertion.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ @@ -12,18 +12,12 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; -/** - * @group Zend_Rbac - */ class SimpleFalseAssertion implements AssertionInterface { /** - * Assertion method - must return a boolean. - * - * @param Rbac $rbac - * @return bool + * {@inheritDoc} */ - public function assert(Rbac $rbac) + public function assert(Rbac $rbac, $permission = null, $role = null) { return false; } diff --git a/test/TestAsset/SimpleTrueAssertion.php b/test/TestAsset/SimpleTrueAssertion.php index 27af86bf..c0fc398f 100644 --- a/test/TestAsset/SimpleTrueAssertion.php +++ b/test/TestAsset/SimpleTrueAssertion.php @@ -3,7 +3,7 @@ * Zend Framework (http://framework.zend.com/) * * @link http://github.com/zendframework/zf2 for the canonical source repository - * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com) + * @copyright Copyright (c) 2005-2018 Zend Technologies USA Inc. (http://www.zend.com) * @license http://framework.zend.com/license/new-bsd New BSD License */ @@ -12,18 +12,12 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; -/** - * @group Zend_Rbac - */ class SimpleTrueAssertion implements AssertionInterface { /** - * Assertion method - must return a boolean. - * - * @param Rbac $rbac - * @return bool + * {@inheritDoc} */ - public function assert(Rbac $rbac) + public function assert(Rbac $rbac, $permission = null, $role = null) { return true; } diff --git a/test/bootstrap.php b/test/bootstrap.php deleted file mode 100644 index ec1cf911..00000000 --- a/test/bootstrap.php +++ /dev/null @@ -1,23 +0,0 @@ - Date: Tue, 13 Feb 2018 17:41:56 +0100 Subject: [PATCH 04/31] Updated CHANGELOG --- CHANGELOG.md | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f6dfa1b..72df3be0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,15 +2,22 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. -## 2.6.1 - TBD +## 3.0.0 - TBD ### Added -- Nothing. +- Check for circular references in role hierarchy when using `Role::addChild()` + and `Role::addParent()` functions. ### Changed -- Nothing. +- `Role::addChild(RoleInterface $child)`, accepts only `RoleInterface` parameter, + no string anymore. +- `Role::addParent(RoleInterface $parent)`, accepts only `RoleInterface` + parameter, no string anymore. +- `Zend\Permissions\Rbac\AssertionInterface`, added the optional parameters + `$permission` and `$role` in the `assert()` function, as follows: + `assert(Rbac $rbac, string $permission = null, RoleInterface $role = null)` ### Deprecated @@ -18,11 +25,19 @@ All notable changes to this project will be documented in this file, in reverse ### Removed -- Nothing. +- [AbstractIterator](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractIterator.php), + removed the class. The role hierarchy is not based anymore on `RecursiveIterator`. + +- [AbstractRole](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractRole.php), + removed the class. All the functions have been moved in `Zend\Permissions\Rbac\Role`. + +- `Role::setParent()`, use `Role::addParent()` instead. ### Fixed -- Nothing. +- [#30](https://github.com/zendframework/zend-permissions-rbac/issues/30), Fixed + circular references with the protected functions `Role::hasAncestor($role)` + used in `Role::addChild()` and `Role::hasDescendant($role)` in `Role::addParent()`. ## 2.6.0 - 2018-02-01 From 4768f64f99327e91d37db11afeb8f50b6de10ded Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 17:48:29 +0100 Subject: [PATCH 05/31] Updated README --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 958d7b95..fd6ba964 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,8 @@ [![Build Status](https://secure.travis-ci.org/zendframework/zend-permissions-rbac.svg?branch=master)](https://secure.travis-ci.org/zendframework/zend-permissions-rbac) [![Coverage Status](https://coveralls.io/repos/zendframework/zend-permissions-rbac/badge.svg?branch=master)](https://coveralls.io/r/zendframework/zend-permissions-rbac?branch=master) -Provides role-based access control (RBAC) permissions management. +Provides [Role-based access control](https://it.wikipedia.org/wiki/Role-based_access_control) +(RBAC) permissions management. - File issues at https://github.com/zendframework/zend-permissions-rbac - Documentation is at https://zendframework.github.io/zend-permissions-rbac/ From 725a782d0c611c65eca1e79416cb255de7dc8ca5 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 18:03:25 +0100 Subject: [PATCH 06/31] Fixing grammar --- doc/book/methods.md | 2 +- src/Role.php | 16 ++++++++-------- src/RoleInterface.php | 2 +- test/RbacTest.php | 14 +++++++------- test/RoleTest.php | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/doc/book/methods.md b/doc/book/methods.md index 397d60f7..bad69251 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -11,7 +11,7 @@ Method signature | Description `addPermission(string $name) : void` | Add a permission for the current role. `hasPermission(string $name) : bool` | Does the role have the given permission? `addChild(RoleInterface $child) : Role` | Add a child role to the current instance. -`getChildrens() : RoleInterface[]` | Get all the children roles. +`getChildren() : RoleInterface[]` | Get all the children roles. `addParent(RoleInterface $parent) : Role` | Add a parent role to the current instance. `getParents() : RoleInterface[]` | Get all the parent roles. diff --git a/src/Role.php b/src/Role.php index eee25d7b..18e18570 100644 --- a/src/Role.php +++ b/src/Role.php @@ -14,7 +14,7 @@ class Role implements RoleInterface /** * @var RoleInterface[] */ - protected $childrens = []; + protected $children = []; /** * @var RoleInterface[] @@ -77,7 +77,7 @@ public function hasPermission($name) if (isset($this->permissions[$name])) { return true; } - foreach ($this->childrens as $child) { + foreach ($this->children as $child) { if ($child->hasPermission($name)) { return true; } @@ -105,8 +105,8 @@ public function addChild(RoleInterface $child) $childName )); } - if (! isset($this->childrens[$childName])) { - $this->childrens[$childName] = $child; + if (! isset($this->children[$childName])) { + $this->children[$childName] = $child; $child->addParent($this); } return $this; @@ -136,9 +136,9 @@ protected function hasAncestor(RoleInterface $role) * * @return RoleInterface[] */ - public function getChildrens() + public function getChildren() { - return array_values($this->childrens); + return array_values($this->children); } /** @@ -176,10 +176,10 @@ public function addParent(RoleInterface $parent) */ protected function hasDescendant(RoleInterface $role) { - if (isset($this->childrens[$role->getName()])) { + if (isset($this->children[$role->getName()])) { return true; } - foreach ($this->childrens as $child) { + foreach ($this->children as $child) { if ($child->hasDescendant($role)) { return true; } diff --git a/src/RoleInterface.php b/src/RoleInterface.php index 5c103818..3ee214aa 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -47,7 +47,7 @@ public function addChild(RoleInterface $child); * * @return RoleInterface[] */ - public function getChildrens(); + public function getChildren(); /** * Add a parent. diff --git a/test/RbacTest.php b/test/RbacTest.php index a940ada9..d2855537 100644 --- a/test/RbacTest.php +++ b/test/RbacTest.php @@ -153,7 +153,7 @@ public function testAddRoleWithParentsUsingRbac() $this->rbac->addRole($bar, $foo); $this->assertEquals($bar->getParents(), [$foo]); - $this->assertEquals([$bar], $foo->getChildrens()); + $this->assertEquals([$bar], $foo->getChildren()); } @@ -167,7 +167,7 @@ public function testAddRoleWithAutomaticParentsUsingRbac() $this->rbac->addRole($bar, $foo); $this->assertEquals($bar->getParents(), [$foo]); - $this->assertEquals([$bar], $foo->getChildrens()); + $this->assertEquals([$bar], $foo->getChildren()); } /** @@ -208,8 +208,8 @@ public function testAddMultipleParentRole() $viewerRole->addPermission('post.view'); $this->rbac->addRole($viewerRole, ['Editor', 'Manager']); - $this->assertEquals('Viewer', $editorRole->getChildrens()[0]->getName()); - $this->assertEquals('Viewer', $managerRole->getChildrens()[0]->getName()); + $this->assertEquals('Viewer', $editorRole->getChildren()[0]->getName()); + $this->assertEquals('Viewer', $managerRole->getChildren()[0]->getName()); $this->assertTrue($this->rbac->isGranted('Editor', 'post.view')); $this->assertTrue($this->rbac->isGranted('Manager', 'post.view')); @@ -241,8 +241,8 @@ public function testAddParentRole() $this->rbac->addRole($viewerRole); // Check roles hierarchy - $this->assertEquals([$viewerRole], $editorRole->getChildrens()); - $this->assertEquals([$viewerRole], $managerRole->getChildrens()); + $this->assertEquals([$viewerRole], $editorRole->getChildren()); + $this->assertEquals([$viewerRole], $managerRole->getChildren()); $this->assertEquals($viewerRole->getParents(), [$editorRole, $managerRole]); $this->assertEquals($managerRole->getParents(), [$adminRole]); $this->assertEmpty($editorRole->getParents()); @@ -275,7 +275,7 @@ public function testAddTwoChildRole() $foo->addChild($baz); $this->assertEquals([$foo], $bar->getParents()); - $this->assertEquals([$bar, $baz], $foo->getChildrens()); + $this->assertEquals([$bar, $baz], $foo->getChildren()); } public function testAddSameParent() diff --git a/test/RoleTest.php b/test/RoleTest.php index 04946d57..3451feb1 100644 --- a/test/RoleTest.php +++ b/test/RoleTest.php @@ -53,7 +53,7 @@ public function testAddChild() $this->assertInstanceOf(RoleInterface::class, $foo->addChild($bar)); $this->assertInstanceOf(RoleInterface::class, $foo->addChild($baz)); - $this->assertEquals($foo->getChildrens(), [$bar, $baz]); + $this->assertEquals($foo->getChildren(), [$bar, $baz]); } public function testAddParent() From 4a5b6e66d3f021cd030d7845e358a9dbfc3393e1 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 18:38:17 +0100 Subject: [PATCH 07/31] Requiring PHP 7.1+ --- .travis.yml | 24 ++---------------------- composer.json | 10 +++++----- src/AssertionInterface.php | 2 +- src/Rbac.php | 13 ++++++------- src/Role.php | 23 +++++++++-------------- src/RoleInterface.php | 14 +++++++------- test/RoleTest.php | 2 +- 7 files changed, 31 insertions(+), 57 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9b5b4872..b89c98ee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,37 +18,17 @@ env: matrix: include: - - php: 5.6 - env: - - DEPS=lowest - - php: 5.6 - env: - - DEPS=locked - - LEGACY_DEPS="phpunit/phpunit" - - php: 5.6 - env: - - DEPS=latest - - TEST_COVERAGE=true - - php: 7 - env: - - DEPS=lowest - - php: 7 - env: - - DEPS=locked - - LEGACY_DEPS="phpunit/phpunit" - - CHECK_CS=true - - php: 7 - env: - - DEPS=latest - php: 7.1 env: - DEPS=lowest - php: 7.1 env: - DEPS=locked + - CHECK_CS=true - php: 7.1 env: - DEPS=latest + - TEST_COVERAGE=true - php: 7.2 env: - DEPS=lowest diff --git a/composer.json b/composer.json index 59f8f5c3..a8a82fc9 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "description": "provides a role-based access control management", "license": "BSD-3-Clause", "keywords": [ - "zf2", + "zf3", "Rbac" ], "homepage": "https://github.com/zendframework/zend-permissions-rbac", @@ -13,14 +13,14 @@ } }, "require": { - "php": "^5.6 || ^7.0" + "php": "^7.1" }, "minimum-stability": "dev", "prefer-stable": true, "extra": { "branch-alias": { - "dev-master": "2.6-dev", - "dev-develop": "2.7-dev" + "dev-master": "3.0-dev", + "dev-develop": "3.1-dev" } }, "autoload-dev": { @@ -29,7 +29,7 @@ } }, "require-dev": { - "phpunit/phpunit": "^5.7.25 || ^6.4.4", + "phpunit/phpunit": "^6.4.4", "zendframework/zend-coding-standard": "~1.0.0" }, "scripts": { diff --git a/src/AssertionInterface.php b/src/AssertionInterface.php index e9b6ed5d..6e0a8c83 100644 --- a/src/AssertionInterface.php +++ b/src/AssertionInterface.php @@ -19,5 +19,5 @@ interface AssertionInterface * @param RoleInterface $role * @return bool */ - public function assert(Rbac $rbac, $permission = null, $role = null); + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null); } diff --git a/src/Rbac.php b/src/Rbac.php index d4f0de58..28825ee9 100644 --- a/src/Rbac.php +++ b/src/Rbac.php @@ -28,17 +28,16 @@ class Rbac * @param bool $createMissingRoles * @return \Zend\Permissions\Rbac\Rbac */ - public function setCreateMissingRoles($createMissingRoles) + public function setCreateMissingRoles(bool $createMissingRoles): Rbac { $this->createMissingRoles = $createMissingRoles; - return $this; } /** * @return bool */ - public function getCreateMissingRoles() + public function getCreateMissingRoles(): bool { return $this->createMissingRoles; } @@ -51,7 +50,7 @@ public function getCreateMissingRoles() * @return self * @throws Exception\InvalidArgumentException */ - public function addRole($role, $parents = null) + public function addRole($role, $parents = null): Rbac { if (is_string($role)) { $role = new Role($role); @@ -86,7 +85,7 @@ public function addRole($role, $parents = null) * @param RoleInterface|string $role * @return bool */ - public function hasRole($role) + public function hasRole($role): bool { if (! is_string($role) && ! $role instanceof RoleInterface) { throw new Exception\InvalidArgumentException( @@ -108,7 +107,7 @@ public function hasRole($role) * @return RoleInterface * @throws Exception\InvalidArgumentException */ - public function getRole($roleName) + public function getRole(string $roleName): RoleInterface { if (! is_string($roleName)) { throw new Exception\InvalidArgumentException( @@ -133,7 +132,7 @@ public function getRole($roleName) * @throws Exception\InvalidArgumentException * @return bool */ - public function isGranted($role, $permission, $assert = null) + public function isGranted($role, string $permission, $assert = null): bool { if (! $this->hasRole($role)) { throw new Exception\InvalidArgumentException(sprintf( diff --git a/src/Role.php b/src/Role.php index 18e18570..e5c7c1ee 100644 --- a/src/Role.php +++ b/src/Role.php @@ -44,7 +44,7 @@ public function __construct($name) * * @return string */ - public function getName() + public function getName(): string { return $this->name; } @@ -55,13 +55,8 @@ public function getName() * @param $name * @return RoleInterface */ - public function addPermission($name) + public function addPermission(string $name): RoleInterface { - if (! is_string($name)) { - throw new Exception\InvalidArgumentException( - 'The permission name must be a string' - ); - } $this->permissions[$name] = true; return $this; } @@ -72,7 +67,7 @@ public function addPermission($name) * @param string $name * @return bool */ - public function hasPermission($name) + public function hasPermission(string $name): bool { if (isset($this->permissions[$name])) { return true; @@ -91,7 +86,7 @@ public function hasPermission($name) * @param RoleInterface $child * @return RoleInterface */ - public function addChild(RoleInterface $child) + public function addChild(RoleInterface $child): RoleInterface { if (! $child instanceof RoleInterface) { throw new Exception\InvalidArgumentException( @@ -118,7 +113,7 @@ public function addChild(RoleInterface $child) * @param RoleInterface $role * @return bool */ - protected function hasAncestor(RoleInterface $role) + protected function hasAncestor(RoleInterface $role): bool { if (isset($this->parents[$role->getName()])) { return true; @@ -136,7 +131,7 @@ protected function hasAncestor(RoleInterface $role) * * @return RoleInterface[] */ - public function getChildren() + public function getChildren(): array { return array_values($this->children); } @@ -147,7 +142,7 @@ public function getChildren() * @param RoleInterface $parent * @return RoleInterface */ - public function addParent(RoleInterface $parent) + public function addParent(RoleInterface $parent): RoleInterface { if (! $parent instanceof RoleInterface) { throw new Exception\InvalidArgumentException( @@ -174,7 +169,7 @@ public function addParent(RoleInterface $parent) * @param RoleInterface $role * @return bool */ - protected function hasDescendant(RoleInterface $role) + protected function hasDescendant(RoleInterface $role): bool { if (isset($this->children[$role->getName()])) { return true; @@ -192,7 +187,7 @@ protected function hasDescendant(RoleInterface $role) * * @return RoleInterface[] */ - public function getParents() + public function getParents(): array { return array_values($this->parents); } diff --git a/src/RoleInterface.php b/src/RoleInterface.php index 3ee214aa..e7a6090a 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -16,7 +16,7 @@ interface RoleInterface * * @return string */ - public function getName(); + public function getName(): string; /** * Add permission to the role. @@ -24,7 +24,7 @@ public function getName(); * @param $name * @return RoleInterface */ - public function addPermission($name); + public function addPermission(string $name): RoleInterface; /** * Checks if a permission exists for this role or any child roles. @@ -32,7 +32,7 @@ public function addPermission($name); * @param string $name * @return bool */ - public function hasPermission($name); + public function hasPermission(string $name): bool; /** * Add a child. @@ -40,14 +40,14 @@ public function hasPermission($name); * @param RoleInterface $child * @return RoleInterface */ - public function addChild(RoleInterface $child); + public function addChild(RoleInterface $child): RoleInterface; /** * Get the children roles. * * @return RoleInterface[] */ - public function getChildren(); + public function getChildren(): array; /** * Add a parent. @@ -55,12 +55,12 @@ public function getChildren(); * @param RoleInterface $parent * @return RoleInterface */ - public function addParent(RoleInterface $parent); + public function addParent(RoleInterface $parent): RoleInterface; /** * Get the parent roles. * * @return RoleInterface[] */ - public function getParents(); + public function getParents(): array; } diff --git a/test/RoleTest.php b/test/RoleTest.php index 3451feb1..87e69c0a 100644 --- a/test/RoleTest.php +++ b/test/RoleTest.php @@ -41,7 +41,7 @@ public function testInvalidPermission() { $perm = new \stdClass(); $foo = new Role('foo'); - $this->expectException(Exception\InvalidArgumentException::class); + $this->expectException(\TypeError::class); $foo->addPermission($perm); } From ae794911b7bda2f210dac110aa776620c0c1262d Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 18:46:27 +0100 Subject: [PATCH 08/31] Fixed composer JSON format --- composer.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/composer.json b/composer.json index 6a634eb8..a8a82fc9 100644 --- a/composer.json +++ b/composer.json @@ -19,13 +19,8 @@ "prefer-stable": true, "extra": { "branch-alias": { -<<<<<<< HEAD "dev-master": "3.0-dev", "dev-develop": "3.1-dev" -======= - "dev-master": "2.6-dev", - "dev-develop": "2.7-dev" ->>>>>>> zend-permissions-rbac/develop } }, "autoload-dev": { From 8c5bb1c46dde4488447264c271a83b5fefab0dfd Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Tue, 13 Feb 2018 18:53:52 +0100 Subject: [PATCH 09/31] Fixed CallbackAssertion using AssertionInterface --- src/Assertion/CallbackAssertion.php | 3 ++- test/TestAsset/RoleMustMatchAssertion.php | 3 ++- test/TestAsset/SimpleFalseAssertion.php | 3 ++- test/TestAsset/SimpleTrueAssertion.php | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index 30ab2fd8..70862584 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -11,6 +11,7 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Exception\InvalidArgumentException; use Zend\Permissions\Rbac\Rbac; +use Zend\Permissions\Rbac\RoleInterface; class CallbackAssertion implements AssertionInterface { @@ -33,7 +34,7 @@ public function __construct($callback) /** * {@inheritDoc} */ - public function assert(Rbac $rbac, $permission = null, $role = null) + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) { return (bool) call_user_func($this->callback, $rbac); } diff --git a/test/TestAsset/RoleMustMatchAssertion.php b/test/TestAsset/RoleMustMatchAssertion.php index ed997700..53e159e3 100644 --- a/test/TestAsset/RoleMustMatchAssertion.php +++ b/test/TestAsset/RoleMustMatchAssertion.php @@ -12,13 +12,14 @@ use Zend\Permissions\Rbac\AbstractRole; use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; +use Zend\Permissions\Rbac\RoleInterface; class RoleMustMatchAssertion implements AssertionInterface { /** * {@inheritDoc} */ - public function assert(Rbac $rbac, $permission = null, $role = null) + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) { return $role->getName() === 'foo'; } diff --git a/test/TestAsset/SimpleFalseAssertion.php b/test/TestAsset/SimpleFalseAssertion.php index 63297c32..481112c0 100644 --- a/test/TestAsset/SimpleFalseAssertion.php +++ b/test/TestAsset/SimpleFalseAssertion.php @@ -11,13 +11,14 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; +use Zend\Permissions\Rbac\RoleInterface; class SimpleFalseAssertion implements AssertionInterface { /** * {@inheritDoc} */ - public function assert(Rbac $rbac, $permission = null, $role = null) + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) { return false; } diff --git a/test/TestAsset/SimpleTrueAssertion.php b/test/TestAsset/SimpleTrueAssertion.php index c0fc398f..e62fced6 100644 --- a/test/TestAsset/SimpleTrueAssertion.php +++ b/test/TestAsset/SimpleTrueAssertion.php @@ -11,13 +11,14 @@ use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; +use Zend\Permissions\Rbac\RoleInterface; class SimpleTrueAssertion implements AssertionInterface { /** * {@inheritDoc} */ - public function assert(Rbac $rbac, $permission = null, $role = null) + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) { return true; } From 487b847f1ac7381d79913b9ad3a5a21afaea727f Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Wed, 14 Feb 2018 15:07:58 +0100 Subject: [PATCH 10/31] Included feedbacks from #34 + removed fluent interface + improved code coverage --- composer.json | 10 +- composer.lock | 127 ++++++++++----------- doc/book/examples.md | 8 +- src/Assertion/CallbackAssertion.php | 13 ++- src/AssertionInterface.php | 15 +-- src/Exception/ExceptionInterface.php | 8 +- src/Exception/InvalidArgumentException.php | 8 +- src/Exception/RuntimeException.php | 8 +- src/Rbac.php | 25 ++-- src/Role.php | 35 ++---- src/RoleInterface.php | 20 ++-- test/Assertion/CallbackAssertionTest.php | 23 +++- test/RbacTest.php | 30 ++++- test/RoleTest.php | 22 ++-- test/TestAsset/RoleMustMatchAssertion.php | 13 +-- test/TestAsset/RoleTest.php | 8 +- test/TestAsset/SimpleFalseAssertion.php | 13 +-- test/TestAsset/SimpleTrueAssertion.php | 13 +-- 18 files changed, 202 insertions(+), 197 deletions(-) diff --git a/composer.json b/composer.json index a8a82fc9..d86f871d 100644 --- a/composer.json +++ b/composer.json @@ -1,10 +1,12 @@ { "name": "zendframework/zend-permissions-rbac", - "description": "provides a role-based access control management", + "description": "Provides a role-based access control management", "license": "BSD-3-Clause", "keywords": [ - "zf3", - "Rbac" + "zendframework", + "zend-permssions-rbac", + "rbac", + "authorization" ], "homepage": "https://github.com/zendframework/zend-permissions-rbac", "autoload": { @@ -29,7 +31,7 @@ } }, "require-dev": { - "phpunit/phpunit": "^6.4.4", + "phpunit/phpunit": "^7.0.1", "zendframework/zend-coding-standard": "~1.0.0" }, "scripts": { diff --git a/composer.lock b/composer.lock index 8ff30eaf..1d1c775a 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "f23d49d16a6176c8fc111967646915b3", + "content-hash": "6a4f332dfcd3b2f4640b487e682bb496", "packages": [], "packages-dev": [ { @@ -425,40 +425,40 @@ }, { "name": "phpunit/php-code-coverage", - "version": "5.3.0", + "version": "6.0.1", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-code-coverage.git", - "reference": "661f34d0bd3f1a7225ef491a70a020ad23a057a1" + "reference": "f8ca4b604baf23dab89d87773c28cc07405189ba" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/661f34d0bd3f1a7225ef491a70a020ad23a057a1", - "reference": "661f34d0bd3f1a7225ef491a70a020ad23a057a1", + "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/f8ca4b604baf23dab89d87773c28cc07405189ba", + "reference": "f8ca4b604baf23dab89d87773c28cc07405189ba", "shasum": "" }, "require": { "ext-dom": "*", "ext-xmlwriter": "*", - "php": "^7.0", + "php": "^7.1", "phpunit/php-file-iterator": "^1.4.2", "phpunit/php-text-template": "^1.2.1", - "phpunit/php-token-stream": "^2.0.1", + "phpunit/php-token-stream": "^3.0", "sebastian/code-unit-reverse-lookup": "^1.0.1", "sebastian/environment": "^3.0", "sebastian/version": "^2.0.1", "theseer/tokenizer": "^1.1" }, "require-dev": { - "phpunit/phpunit": "^6.0" + "phpunit/phpunit": "^7.0" }, "suggest": { - "ext-xdebug": "^2.5.5" + "ext-xdebug": "^2.6.0" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "5.3.x-dev" + "dev-master": "6.0-dev" } }, "autoload": { @@ -484,7 +484,7 @@ "testing", "xunit" ], - "time": "2017-12-06T09:29:45+00:00" + "time": "2018-02-02T07:01:41+00:00" }, { "name": "phpunit/php-file-iterator", @@ -576,28 +576,28 @@ }, { "name": "phpunit/php-timer", - "version": "1.0.9", + "version": "2.0.0", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-timer.git", - "reference": "3dcf38ca72b158baf0bc245e9184d3fdffa9c46f" + "reference": "8b8454ea6958c3dee38453d3bd571e023108c91f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-timer/zipball/3dcf38ca72b158baf0bc245e9184d3fdffa9c46f", - "reference": "3dcf38ca72b158baf0bc245e9184d3fdffa9c46f", + "url": "https://api.github.com/repos/sebastianbergmann/php-timer/zipball/8b8454ea6958c3dee38453d3bd571e023108c91f", + "reference": "8b8454ea6958c3dee38453d3bd571e023108c91f", "shasum": "" }, "require": { - "php": "^5.3.3 || ^7.0" + "php": "^7.1" }, "require-dev": { - "phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.0" + "phpunit/phpunit": "^7.0" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "1.0-dev" + "dev-master": "2.0-dev" } }, "autoload": { @@ -612,7 +612,7 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sb@sebastian-bergmann.de", + "email": "sebastian@phpunit.de", "role": "lead" } ], @@ -621,33 +621,33 @@ "keywords": [ "timer" ], - "time": "2017-02-26T11:10:40+00:00" + "time": "2018-02-01T13:07:23+00:00" }, { "name": "phpunit/php-token-stream", - "version": "2.0.2", + "version": "3.0.0", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-token-stream.git", - "reference": "791198a2c6254db10131eecfe8c06670700904db" + "reference": "21ad88bbba7c3d93530d93994e0a33cd45f02ace" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-token-stream/zipball/791198a2c6254db10131eecfe8c06670700904db", - "reference": "791198a2c6254db10131eecfe8c06670700904db", + "url": "https://api.github.com/repos/sebastianbergmann/php-token-stream/zipball/21ad88bbba7c3d93530d93994e0a33cd45f02ace", + "reference": "21ad88bbba7c3d93530d93994e0a33cd45f02ace", "shasum": "" }, "require": { "ext-tokenizer": "*", - "php": "^7.0" + "php": "^7.1" }, "require-dev": { - "phpunit/phpunit": "^6.2.4" + "phpunit/phpunit": "^7.0" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "2.0-dev" + "dev-master": "3.0-dev" } }, "autoload": { @@ -670,20 +670,20 @@ "keywords": [ "tokenizer" ], - "time": "2017-11-27T05:48:46+00:00" + "time": "2018-02-01T13:16:43+00:00" }, { "name": "phpunit/phpunit", - "version": "6.5.6", + "version": "7.0.1", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/phpunit.git", - "reference": "3330ef26ade05359d006041316ed0fa9e8e3cefe" + "reference": "316555dbd0ed4097bbdd17c65ab416bf27a472e9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/3330ef26ade05359d006041316ed0fa9e8e3cefe", - "reference": "3330ef26ade05359d006041316ed0fa9e8e3cefe", + "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/316555dbd0ed4097bbdd17c65ab416bf27a472e9", + "reference": "316555dbd0ed4097bbdd17c65ab416bf27a472e9", "shasum": "" }, "require": { @@ -695,15 +695,15 @@ "myclabs/deep-copy": "^1.6.1", "phar-io/manifest": "^1.0.1", "phar-io/version": "^1.0", - "php": "^7.0", + "php": "^7.1", "phpspec/prophecy": "^1.7", - "phpunit/php-code-coverage": "^5.3", + "phpunit/php-code-coverage": "^6.0", "phpunit/php-file-iterator": "^1.4.3", "phpunit/php-text-template": "^1.2.1", - "phpunit/php-timer": "^1.0.9", - "phpunit/phpunit-mock-objects": "^5.0.5", + "phpunit/php-timer": "^2.0", + "phpunit/phpunit-mock-objects": "^6.0", "sebastian/comparator": "^2.1", - "sebastian/diff": "^2.0", + "sebastian/diff": "^3.0", "sebastian/environment": "^3.1", "sebastian/exporter": "^3.1", "sebastian/global-state": "^2.0", @@ -711,16 +711,12 @@ "sebastian/resource-operations": "^1.0", "sebastian/version": "^2.0.1" }, - "conflict": { - "phpdocumentor/reflection-docblock": "3.0.2", - "phpunit/dbunit": "<3.0" - }, "require-dev": { "ext-pdo": "*" }, "suggest": { "ext-xdebug": "*", - "phpunit/php-invoker": "^1.1" + "phpunit/php-invoker": "^2.0" }, "bin": [ "phpunit" @@ -728,7 +724,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "6.5.x-dev" + "dev-master": "7.0-dev" } }, "autoload": { @@ -754,33 +750,30 @@ "testing", "xunit" ], - "time": "2018-02-01T05:57:37+00:00" + "time": "2018-02-13T06:08:08+00:00" }, { "name": "phpunit/phpunit-mock-objects", - "version": "5.0.6", + "version": "6.0.0", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/phpunit-mock-objects.git", - "reference": "33fd41a76e746b8fa96d00b49a23dadfa8334cdf" + "reference": "e495e5d3660321b62c294d8c0e954d02d6ce2573" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/phpunit-mock-objects/zipball/33fd41a76e746b8fa96d00b49a23dadfa8334cdf", - "reference": "33fd41a76e746b8fa96d00b49a23dadfa8334cdf", + "url": "https://api.github.com/repos/sebastianbergmann/phpunit-mock-objects/zipball/e495e5d3660321b62c294d8c0e954d02d6ce2573", + "reference": "e495e5d3660321b62c294d8c0e954d02d6ce2573", "shasum": "" }, "require": { "doctrine/instantiator": "^1.0.5", - "php": "^7.0", + "php": "^7.1", "phpunit/php-text-template": "^1.2.1", "sebastian/exporter": "^3.1" }, - "conflict": { - "phpunit/phpunit": "<6.0" - }, "require-dev": { - "phpunit/phpunit": "^6.5" + "phpunit/phpunit": "^7.0" }, "suggest": { "ext-soap": "*" @@ -788,7 +781,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "5.0.x-dev" + "dev-master": "6.0.x-dev" } }, "autoload": { @@ -813,7 +806,7 @@ "mock", "xunit" ], - "time": "2018-01-06T05:45:45+00:00" + "time": "2018-02-01T13:11:13+00:00" }, { "name": "sebastian/code-unit-reverse-lookup", @@ -926,28 +919,29 @@ }, { "name": "sebastian/diff", - "version": "2.0.1", + "version": "3.0.0", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/diff.git", - "reference": "347c1d8b49c5c3ee30c7040ea6fc446790e6bddd" + "reference": "e09160918c66281713f1c324c1f4c4c3037ba1e8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/diff/zipball/347c1d8b49c5c3ee30c7040ea6fc446790e6bddd", - "reference": "347c1d8b49c5c3ee30c7040ea6fc446790e6bddd", + "url": "https://api.github.com/repos/sebastianbergmann/diff/zipball/e09160918c66281713f1c324c1f4c4c3037ba1e8", + "reference": "e09160918c66281713f1c324c1f4c4c3037ba1e8", "shasum": "" }, "require": { - "php": "^7.0" + "php": "^7.1" }, "require-dev": { - "phpunit/phpunit": "^6.2" + "phpunit/phpunit": "^7.0", + "symfony/process": "^2 || ^3.3 || ^4" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "2.0-dev" + "dev-master": "3.0-dev" } }, "autoload": { @@ -972,9 +966,12 @@ "description": "Diff implementation", "homepage": "https://github.com/sebastianbergmann/diff", "keywords": [ - "diff" + "diff", + "udiff", + "unidiff", + "unified diff" ], - "time": "2017-08-03T08:09:46+00:00" + "time": "2018-02-01T13:45:15+00:00" }, { "name": "sebastian/environment", @@ -1578,7 +1575,7 @@ "prefer-stable": true, "prefer-lowest": false, "platform": { - "php": "^5.6 || ^7.0" + "php": "^7.1" }, "platform-dev": [] } diff --git a/doc/book/examples.md b/doc/book/examples.md index 6a3649ba..249e87ac 100644 --- a/doc/book/examples.md +++ b/doc/book/examples.md @@ -80,6 +80,7 @@ Checking permission using `isGranted()` with a class implementing `Zend\Permissions\Rbac\AssertionInterface`: ```php +use App\Model\Article; use Zend\Permissions\Rbac\AssertionInterface; use Zend\Permissions\Rbac\Rbac; @@ -88,22 +89,21 @@ class AssertUserRoleMatches implements AssertionInterface protected $userId; protected $article; - public function __construct($userId) + public function __construct(string $userId) { $this->userId = $userId; } - public function setArticle($article) + public function setArticle(Article $article) { $this->article = $article; } - public function assert(Rbac $rbac, $permission = null, $role = null) + public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) { if (! $this->article) { return false; } - return ($this->userId === $this->article->getUserId()); } } diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index 70862584..723be666 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -1,11 +1,12 @@ callback, $rbac); } diff --git a/src/AssertionInterface.php b/src/AssertionInterface.php index 6e0a8c83..ce496a0f 100644 --- a/src/AssertionInterface.php +++ b/src/AssertionInterface.php @@ -1,23 +1,18 @@ createMissingRoles = $createMissingRoles; - return $this; } /** @@ -47,10 +46,10 @@ public function getCreateMissingRoles(): bool * * @param string|RoleInterface $child * @param array|RoleInterface|null $parents - * @return self + * @return void * @throws Exception\InvalidArgumentException */ - public function addRole($role, $parents = null): Rbac + public function addRole($role, $parents = null): void { if (is_string($role)) { $role = new Role($role); @@ -76,7 +75,6 @@ public function addRole($role, $parents = null): Rbac } } $this->roles[$role->getName()] = $role; - return $this; } /** @@ -109,11 +107,6 @@ public function hasRole($role): bool */ public function getRole(string $roleName): RoleInterface { - if (! is_string($roleName)) { - throw new Exception\InvalidArgumentException( - 'Role name must be a string' - ); - } if (! isset($this->roles[$roleName])) { throw new Exception\InvalidArgumentException(sprintf( 'No role with name "%s" could be found', @@ -129,8 +122,8 @@ public function getRole(string $roleName): RoleInterface * @param RoleInterface|string $role * @param string $permission * @param AssertionInterface|Callable|null $assert - * @throws Exception\InvalidArgumentException * @return bool + * @throws Exception\InvalidArgumentException */ public function isGranted($role, string $permission, $assert = null): bool { diff --git a/src/Role.php b/src/Role.php index e5c7c1ee..293720d3 100644 --- a/src/Role.php +++ b/src/Role.php @@ -1,12 +1,12 @@ permissions[$name] = true; - return $this; } /** @@ -84,15 +83,11 @@ public function hasPermission(string $name): bool * Add a child * * @param RoleInterface $child - * @return RoleInterface + * @return void + * @throws Exception\RuntimeException */ - public function addChild(RoleInterface $child): RoleInterface + public function addChild(RoleInterface $child): void { - if (! $child instanceof RoleInterface) { - throw new Exception\InvalidArgumentException( - 'Child must implement Zend\Permissions\Rbac\RoleInterface' - ); - } $childName = $child->getName(); if ($this->hasAncestor($child)) { throw new Exception\RuntimeException(sprintf( @@ -104,7 +99,6 @@ public function addChild(RoleInterface $child): RoleInterface $this->children[$childName] = $child; $child->addParent($this); } - return $this; } /** @@ -140,15 +134,11 @@ public function getChildren(): array * Add a parent role * * @param RoleInterface $parent - * @return RoleInterface + * @return void + * @throws Exception\RuntimeException */ - public function addParent(RoleInterface $parent): RoleInterface + public function addParent(RoleInterface $parent): void { - if (! $parent instanceof RoleInterface) { - throw new Exception\InvalidArgumentException( - 'Parent must implement Zend\Permissions\Rbac\RoleInterface' - ); - } $parentName = $parent->getName(); if ($this->hasDescendant($parent)) { throw new Exception\RuntimeException(sprintf( @@ -160,7 +150,6 @@ public function addParent(RoleInterface $parent): RoleInterface $this->parents[$parentName] = $parent; $parent->addChild($this); } - return $this; } /** diff --git a/src/RoleInterface.php b/src/RoleInterface.php index e7a6090a..c3721b54 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -1,12 +1,12 @@ assertFalse($rbac->isGranted($bar, 'can.foo', $roleNoMatch)); $this->assertTrue($rbac->isGranted($foo, 'can.foo', $roleMatch)); } + + public function testAssertWithCallable() + { + $rbac = new Rbac\Rbac(); + $foo = new Rbac\Role('foo'); + $foo->addPermission('can.foo'); + $rbac->addRole($foo); + + $callable = function ($rbac, $permission, $role) { + return true; + }; + $this->assertTrue($rbac->isGranted('foo', 'can.foo', $callable)); + $this->assertFalse($rbac->isGranted('foo', 'can.bar', $callable)); + } } diff --git a/test/RbacTest.php b/test/RbacTest.php index d2855537..4f46459d 100644 --- a/test/RbacTest.php +++ b/test/RbacTest.php @@ -1,12 +1,12 @@ assertEquals(false, $this->rbac->isGranted('bar', 'can.baz')); } + public function testIsGrantedWithInvalidRole() + { + $this->expectException(Exception\InvalidArgumentException::class); + $this->rbac->isGranted('foo', 'permission'); + } + public function testGetRole() { $foo = new Rbac\Role('foo'); @@ -118,6 +124,19 @@ public function testHasRole() $this->assertFalse($this->rbac->hasRole($snafu)); } + public function testHasRoleWithInvalidElement() + { + $role = new \stdClass(); + $this->expectException(Exception\InvalidArgumentException::class); + $this->rbac->hasRole($role); + } + + public function testGetUndefinedRole() + { + $this->expectException(Exception\InvalidArgumentException::class); + $this->rbac->getRole('foo'); + } + public function testAddRoleFromString() { $this->rbac->addRole('foo'); @@ -176,7 +195,8 @@ public function testAddRoleWithAutomaticParentsUsingRbac() public function testAddCustomChildRole() { $role = $this->getMockForAbstractClass(Rbac\RoleInterface::class); - $this->rbac->setCreateMissingRoles(true)->addRole($role, ['parent']); + $this->rbac->setCreateMissingRoles(true); + $this->rbac->addRole($role, 'parent'); $role->expects($this->any()) ->method('getName') diff --git a/test/RoleTest.php b/test/RoleTest.php index 87e69c0a..f96aeb37 100644 --- a/test/RoleTest.php +++ b/test/RoleTest.php @@ -1,12 +1,12 @@ assertInstanceOf(RoleInterface::class, $foo->addPermission('bar')); - $this->assertInstanceOf(RoleInterface::class, $foo->addPermission('baz')); + $foo->addPermission('bar'); + $foo->addPermission('baz'); + $this->assertTrue($foo->hasPermission('bar')); $this->assertTrue($foo->hasPermission('baz')); } @@ -51,8 +52,9 @@ public function testAddChild() $bar = new Role('bar'); $baz = new Role('baz'); - $this->assertInstanceOf(RoleInterface::class, $foo->addChild($bar)); - $this->assertInstanceOf(RoleInterface::class, $foo->addChild($baz)); + $foo->addChild($bar); + $foo->addChild($baz); + $this->assertEquals($foo->getChildren(), [$bar, $baz]); } @@ -62,8 +64,8 @@ public function testAddParent() $bar = new Role('bar'); $baz = new Role('baz'); - $this->assertInstanceOf(RoleInterface::class, $foo->addParent($bar)); - $this->assertInstanceOf(RoleInterface::class, $foo->addParent($baz)); + $foo->addParent($bar); + $foo->addParent($baz); $this->assertEquals($foo->getParents(), [$bar, $baz]); } diff --git a/test/TestAsset/RoleMustMatchAssertion.php b/test/TestAsset/RoleMustMatchAssertion.php index 53e159e3..65d72505 100644 --- a/test/TestAsset/RoleMustMatchAssertion.php +++ b/test/TestAsset/RoleMustMatchAssertion.php @@ -1,12 +1,12 @@ getName() === 'foo'; } diff --git a/test/TestAsset/RoleTest.php b/test/TestAsset/RoleTest.php index 6d71dd15..c72f8289 100644 --- a/test/TestAsset/RoleTest.php +++ b/test/TestAsset/RoleTest.php @@ -1,12 +1,12 @@ Date: Wed, 14 Feb 2018 15:37:24 +0100 Subject: [PATCH 11/31] Improving code coverage --- test/Assertion/CallbackAssertionTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 5f09e312..1563f3be 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -94,4 +94,16 @@ public function testAssertWithCallable() $this->assertTrue($rbac->isGranted('foo', 'can.foo', $callable)); $this->assertFalse($rbac->isGranted('foo', 'can.bar', $callable)); } + + public function testAssertWithInvalidValue() + { + $rbac = new Rbac\Rbac(); + $foo = new Rbac\Role('foo'); + $foo->addPermission('can.foo'); + $rbac->addRole($foo); + + $callable = new \stdClass(); + $this->expectException(Rbac\Exception\InvalidArgumentException::class); + $rbac->isGranted('foo', 'can.foo', $callable); + } } From c49aa62d2bbd68ee461c7d08cce7370e0ab5f077 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Mon, 19 Feb 2018 17:54:06 +0100 Subject: [PATCH 12/31] Updated with latest comments in #34 --- composer.json | 4 ++-- doc/book/methods.md | 2 +- src/Assertion/CallbackAssertion.php | 4 ++-- src/AssertionInterface.php | 2 +- src/Exception/CircularReferenceException.php | 15 +++++++++++++++ src/Rbac.php | 20 ++++++++++---------- src/Role.php | 20 ++++++++++---------- src/RoleInterface.php | 14 +++++++------- test/RoleTest.php | 4 ++-- test/TestAsset/RoleMustMatchAssertion.php | 2 +- test/TestAsset/SimpleFalseAssertion.php | 2 +- test/TestAsset/SimpleTrueAssertion.php | 2 +- 12 files changed, 53 insertions(+), 38 deletions(-) create mode 100644 src/Exception/CircularReferenceException.php diff --git a/composer.json b/composer.json index d86f871d..ee4a597f 100644 --- a/composer.json +++ b/composer.json @@ -21,8 +21,8 @@ "prefer-stable": true, "extra": { "branch-alias": { - "dev-master": "3.0-dev", - "dev-develop": "3.1-dev" + "dev-master": "2.6-dev", + "dev-develop": "2.7-dev" } }, "autoload-dev": { diff --git a/doc/book/methods.md b/doc/book/methods.md index bad69251..f6aacf70 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -22,7 +22,7 @@ assertions are provided the `Rbac` instance on invocation. Method signature | Description --------------------------------------------------------------------------- | ----------- -`assert(Rbac $rbac, string $permission = null, RoleInterface $role = null)` | Given an RBAC, an optional permission, and optional role determine if permission is granted. +`assert(Rbac $rbac, RoleInterface $role = null, string $permission = null)` | Given an RBAC, an optional permission, and optional role determine if permission is granted. ## `Zend\Permissions\Rbac\Rbac` diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index 723be666..bef4169f 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -35,8 +35,8 @@ public function __construct($callback) /** * {@inheritdoc} */ - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null): bool + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool { - return (bool) call_user_func($this->callback, $rbac); + return (bool) ($this->callback)($rbac, $role, $permission); } } diff --git a/src/AssertionInterface.php b/src/AssertionInterface.php index ce496a0f..bee9557d 100644 --- a/src/AssertionInterface.php +++ b/src/AssertionInterface.php @@ -14,5 +14,5 @@ interface AssertionInterface /** * Assertion method - must return a boolean. */ - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null): bool; + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool; } diff --git a/src/Exception/CircularReferenceException.php b/src/Exception/CircularReferenceException.php new file mode 100644 index 00000000..3e0e41d4 --- /dev/null +++ b/src/Exception/CircularReferenceException.php @@ -0,0 +1,15 @@ +createMissingRoles = $createMissingRoles; } @@ -36,7 +36,7 @@ public function setCreateMissingRoles(bool $createMissingRoles): void /** * @return bool */ - public function getCreateMissingRoles(): bool + public function getCreateMissingRoles() : bool { return $this->createMissingRoles; } @@ -49,7 +49,7 @@ public function getCreateMissingRoles(): bool * @return void * @throws Exception\InvalidArgumentException */ - public function addRole($role, $parents = null): void + public function addRole($role, $parents = null) : void { if (is_string($role)) { $role = new Role($role); @@ -83,7 +83,7 @@ public function addRole($role, $parents = null): void * @param RoleInterface|string $role * @return bool */ - public function hasRole($role): bool + public function hasRole($role) : bool { if (! is_string($role) && ! $role instanceof RoleInterface) { throw new Exception\InvalidArgumentException( @@ -94,8 +94,8 @@ public function hasRole($role): bool return isset($this->roles[$role]); } $roleName = $role->getName(); - return isset($this->roles[$roleName]) && - $this->roles[$roleName] === $role; + return isset($this->roles[$roleName]) + && $this->roles[$roleName] === $role; } /** @@ -105,7 +105,7 @@ public function hasRole($role): bool * @return RoleInterface * @throws Exception\InvalidArgumentException */ - public function getRole(string $roleName): RoleInterface + public function getRole(string $roleName) : RoleInterface { if (! isset($this->roles[$roleName])) { throw new Exception\InvalidArgumentException(sprintf( @@ -125,7 +125,7 @@ public function getRole(string $roleName): RoleInterface * @return bool * @throws Exception\InvalidArgumentException */ - public function isGranted($role, string $permission, $assert = null): bool + public function isGranted($role, string $permission, $assert = null) : bool { if (! $this->hasRole($role)) { throw new Exception\InvalidArgumentException(sprintf( @@ -141,10 +141,10 @@ public function isGranted($role, string $permission, $assert = null): bool return $result; } if ($assert instanceof AssertionInterface) { - return $result && $assert->assert($this, $permission, $role); + return $result && $assert->assert($this, $role, $permission); } if (is_callable($assert)) { - return $result && $assert($this, $permission, $role); + return $result && $assert($this, $role, $permission); } throw new Exception\InvalidArgumentException( 'Assertions must be a Callable or an instance of Zend\Permissions\Rbac\AssertionInterface' diff --git a/src/Role.php b/src/Role.php index 293720d3..b14d3b5f 100644 --- a/src/Role.php +++ b/src/Role.php @@ -55,7 +55,7 @@ public function getName(): string * @param $name * @return void */ - public function addPermission(string $name): void + public function addPermission(string $name) : void { $this->permissions[$name] = true; } @@ -66,7 +66,7 @@ public function addPermission(string $name): void * @param string $name * @return bool */ - public function hasPermission(string $name): bool + public function hasPermission(string $name) : bool { if (isset($this->permissions[$name])) { return true; @@ -86,11 +86,11 @@ public function hasPermission(string $name): bool * @return void * @throws Exception\RuntimeException */ - public function addChild(RoleInterface $child): void + public function addChild(RoleInterface $child) : void { $childName = $child->getName(); if ($this->hasAncestor($child)) { - throw new Exception\RuntimeException(sprintf( + throw new Exception\CircularReferenceException(sprintf( "To prevent circular references, you cannot add role '%s' as child", $childName )); @@ -107,7 +107,7 @@ public function addChild(RoleInterface $child): void * @param RoleInterface $role * @return bool */ - protected function hasAncestor(RoleInterface $role): bool + protected function hasAncestor(RoleInterface $role) : bool { if (isset($this->parents[$role->getName()])) { return true; @@ -125,7 +125,7 @@ protected function hasAncestor(RoleInterface $role): bool * * @return RoleInterface[] */ - public function getChildren(): array + public function getChildren() : array { return array_values($this->children); } @@ -137,11 +137,11 @@ public function getChildren(): array * @return void * @throws Exception\RuntimeException */ - public function addParent(RoleInterface $parent): void + public function addParent(RoleInterface $parent) : void { $parentName = $parent->getName(); if ($this->hasDescendant($parent)) { - throw new Exception\RuntimeException(sprintf( + throw new Exception\CircularReferenceException(sprintf( "To prevent circular references, you cannot add role '%s' as parent", $parentName )); @@ -158,7 +158,7 @@ public function addParent(RoleInterface $parent): void * @param RoleInterface $role * @return bool */ - protected function hasDescendant(RoleInterface $role): bool + protected function hasDescendant(RoleInterface $role) : bool { if (isset($this->children[$role->getName()])) { return true; @@ -176,7 +176,7 @@ protected function hasDescendant(RoleInterface $role): bool * * @return RoleInterface[] */ - public function getParents(): array + public function getParents() : array { return array_values($this->parents); } diff --git a/src/RoleInterface.php b/src/RoleInterface.php index c3721b54..a858971f 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -16,7 +16,7 @@ interface RoleInterface * * @return string */ - public function getName(): string; + public function getName() : string; /** * Add permission to the role. @@ -24,7 +24,7 @@ public function getName(): string; * @param $name * @return void */ - public function addPermission(string $name): void; + public function addPermission(string $name) : void; /** * Checks if a permission exists for this role or any child roles. @@ -32,7 +32,7 @@ public function addPermission(string $name): void; * @param string $name * @return bool */ - public function hasPermission(string $name): bool; + public function hasPermission(string $name) : bool; /** * Add a child. @@ -40,14 +40,14 @@ public function hasPermission(string $name): bool; * @param RoleInterface $child * @return void */ - public function addChild(RoleInterface $child): void; + public function addChild(RoleInterface $child) : void; /** * Get the children roles. * * @return RoleInterface[] */ - public function getChildren(): array; + public function getChildren() : array; /** * Add a parent. @@ -55,12 +55,12 @@ public function getChildren(): array; * @param RoleInterface $parent * @return void */ - public function addParent(RoleInterface $parent): void; + public function addParent(RoleInterface $parent) : void; /** * Get the parent roles. * * @return RoleInterface[] */ - public function getParents(): array; + public function getParents() : array; } diff --git a/test/RoleTest.php b/test/RoleTest.php index f96aeb37..9ba46379 100644 --- a/test/RoleTest.php +++ b/test/RoleTest.php @@ -106,7 +106,7 @@ public function testCircleReferenceWithChild() $foo->addChild($bar); $bar->addChild($baz); - $this->expectException(Exception\RuntimeException::class); + $this->expectException(Exception\CircularReferenceException::class); $baz->addChild($foo); } @@ -119,7 +119,7 @@ public function testCircleReferenceWithParent() $foo->addParent($bar); $bar->addParent($baz); - $this->expectException(Exception\RuntimeException::class); + $this->expectException(Exception\CircularReferenceException::class); $baz->addParent($foo); } } diff --git a/test/TestAsset/RoleMustMatchAssertion.php b/test/TestAsset/RoleMustMatchAssertion.php index 65d72505..0ce2086e 100644 --- a/test/TestAsset/RoleMustMatchAssertion.php +++ b/test/TestAsset/RoleMustMatchAssertion.php @@ -16,7 +16,7 @@ class RoleMustMatchAssertion implements AssertionInterface { - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null): bool + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool { return $role->getName() === 'foo'; } diff --git a/test/TestAsset/SimpleFalseAssertion.php b/test/TestAsset/SimpleFalseAssertion.php index fc242fd2..8ef14217 100644 --- a/test/TestAsset/SimpleFalseAssertion.php +++ b/test/TestAsset/SimpleFalseAssertion.php @@ -15,7 +15,7 @@ class SimpleFalseAssertion implements AssertionInterface { - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null): bool + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool { return false; } diff --git a/test/TestAsset/SimpleTrueAssertion.php b/test/TestAsset/SimpleTrueAssertion.php index 180336a1..8a9177d6 100644 --- a/test/TestAsset/SimpleTrueAssertion.php +++ b/test/TestAsset/SimpleTrueAssertion.php @@ -15,7 +15,7 @@ class SimpleTrueAssertion implements AssertionInterface { - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null): bool + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool { return true; } From 2f528dc604557441cab54194ed229ea998f17fd0 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:00:19 -0600 Subject: [PATCH 13/31] Consistent capitalization with acronym --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fd6ba964..aea66649 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![Build Status](https://secure.travis-ci.org/zendframework/zend-permissions-rbac.svg?branch=master)](https://secure.travis-ci.org/zendframework/zend-permissions-rbac) [![Coverage Status](https://coveralls.io/repos/zendframework/zend-permissions-rbac/badge.svg?branch=master)](https://coveralls.io/r/zendframework/zend-permissions-rbac?branch=master) -Provides [Role-based access control](https://it.wikipedia.org/wiki/Role-based_access_control) +Provides [Role-Based Access Control](https://it.wikipedia.org/wiki/Role-based_access_control) (RBAC) permissions management. - File issues at https://github.com/zendframework/zend-permissions-rbac From 3ac0f940e77146f0b14402acda81f445963f4a68 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:00:44 -0600 Subject: [PATCH 14/31] Corrects links in README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index aea66649..a1a49be4 100644 --- a/README.md +++ b/README.md @@ -6,5 +6,5 @@ Provides [Role-Based Access Control](https://it.wikipedia.org/wiki/Role-based_access_control) (RBAC) permissions management. -- File issues at https://github.com/zendframework/zend-permissions-rbac -- Documentation is at https://zendframework.github.io/zend-permissions-rbac/ +- File issues at https://github.com/zendframework/zend-permissions-rbac/issues +- Documentation is at https://docs.zendframework.com/zend-permissions-rbac/ From c2c47caf21215637685c77de27abf414075761e7 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:09:07 -0600 Subject: [PATCH 15/31] Rephrases 3.0.0 entries to refer to patches Rephrases all 3.0.0 entries to refer to the patch responsible for them (in all cases, #34), and updates the grammar used in each. --- CHANGELOG.md | 55 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72df3be0..b3ec7e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,18 +6,32 @@ All notable changes to this project will be documented in this file, in reverse ### Added -- Check for circular references in role hierarchy when using `Role::addChild()` - and `Role::addParent()` functions. +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) adds + checks for circular references in the role hierarchy when using the + `Role::addChild()` and `Role::addParent()` methods. ### Changed -- `Role::addChild(RoleInterface $child)`, accepts only `RoleInterface` parameter, - no string anymore. -- `Role::addParent(RoleInterface $parent)`, accepts only `RoleInterface` - parameter, no string anymore. -- `Zend\Permissions\Rbac\AssertionInterface`, added the optional parameters - `$permission` and `$role` in the `assert()` function, as follows: - `assert(Rbac $rbac, string $permission = null, RoleInterface $role = null)` +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) updates + the `Role::addChild(RoleInterface $child)` method to accept only a `RoleInterface` parameter; + strings are no longer accepted. + +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) updates + the `Role::addParent(RoleInterface $parent)` method to only accept a + `RoleInterface` parameter; strings are no longer accepted. + +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) updates + the `Zend\Permissions\Rbac\AssertionInterface`, adding two optional parameters + to the `assert()` definition and defining a return type, so that it now reads + as follows: + + ```php + public function assert( + Rbac $rbac, + string $permission = null, + RoleInterface $role = null + ) : bool + ``` ### Deprecated @@ -25,19 +39,26 @@ All notable changes to this project will be documented in this file, in reverse ### Removed -- [AbstractIterator](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractIterator.php), - removed the class. The role hierarchy is not based anymore on `RecursiveIterator`. +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) removes + support for PHP versions prior to 7.1. + +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) removes + the [AbstractIterator](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractIterator.php) + class. The role hierarchy no longer relies on a `RecursiveIterator`. -- [AbstractRole](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractRole.php), - removed the class. All the functions have been moved in `Zend\Permissions\Rbac\Role`. +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) removes + the [AbstractRole](https://github.com/zendframework/zend-permissions-rbac/blob/release-2.6.0/src/AbstractRole.php) + class. All its functions have been merged to the `Zend\Permissions\Rbac\Role` + class. -- `Role::setParent()`, use `Role::addParent()` instead. +- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) removes + the method `Role::setParent()`; use `Role::addParent()` instead. ### Fixed -- [#30](https://github.com/zendframework/zend-permissions-rbac/issues/30), Fixed - circular references with the protected functions `Role::hasAncestor($role)` - used in `Role::addChild()` and `Role::hasDescendant($role)` in `Role::addParent()`. +- [#30](https://github.com/zendframework/zend-permissions-rbac/issues/30) fixes + circular references within the used in `Role::addChild()` and + `Role::addParent()` algorithms. ## 2.6.0 - 2018-02-01 From 0e3c8c0fd5f1e2331ac9b68543f3c65722bf534d Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:10:17 -0600 Subject: [PATCH 16/31] Fixes example to match updated AssertionInterface definition --- doc/book/examples.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/examples.md b/doc/book/examples.md index 249e87ac..798e7acc 100644 --- a/doc/book/examples.md +++ b/doc/book/examples.md @@ -99,7 +99,7 @@ class AssertUserRoleMatches implements AssertionInterface $this->article = $article; } - public function assert(Rbac $rbac, string $permission = null, RoleInterface $role = null) + public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) { if (! $this->article) { return false; From 5c3c71148b295594853277d152d64901d27f4981 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:11:06 -0600 Subject: [PATCH 17/31] Consistent casing when presenting an acronym --- doc/book/intro.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/intro.md b/doc/book/intro.md index d5d830bc..214b5a49 100644 --- a/doc/book/intro.md +++ b/doc/book/intro.md @@ -1,6 +1,6 @@ # Introduction -`zend-permissions-rbac` provides a lightweight [Role-based access control](https://it.wikipedia.org/wiki/Role-based_access_control) +`zend-permissions-rbac` provides a lightweight [Role-Based Access Control](https://it.wikipedia.org/wiki/Role-based_access_control) (RBAC) implementation in PHP. RBAC differs from access control lists (ACL) by putting the emphasis on roles and their permissions rather than objects (resources). From 49ec38e40b4a98e0c5a72b168c916f18f3881034 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:11:37 -0600 Subject: [PATCH 18/31] s/children/child/ --- doc/book/intro.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/intro.md b/doc/book/intro.md index 214b5a49..980cc434 100644 --- a/doc/book/intro.md +++ b/doc/book/intro.md @@ -15,7 +15,7 @@ Thus, RBAC has the following model: - many to many relationship between **identities** and **roles**. - many to many relationship between **roles** and **permissions**. -- **roles** can have parent and children roles (hierarchy of roles). +- **roles** can have parent and child roles (hierarchy of roles). ## Roles From 6f44772c61e349c12fdafcf4873742bba527ab88 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:13:09 -0600 Subject: [PATCH 19/31] Rephrase description to reflect argument order. --- doc/book/methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/methods.md b/doc/book/methods.md index f6aacf70..2da8b44f 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -22,7 +22,7 @@ assertions are provided the `Rbac` instance on invocation. Method signature | Description --------------------------------------------------------------------------- | ----------- -`assert(Rbac $rbac, RoleInterface $role = null, string $permission = null)` | Given an RBAC, an optional permission, and optional role determine if permission is granted. +`assert(Rbac $rbac, RoleInterface $role = null, string $permission = null)` | Given an RBAC, an optional role, and an optional permission, determine if permission is granted. ## `Zend\Permissions\Rbac\Rbac` From 26fc89d27fa8ec889add7accd1deaafc5ba43964 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:14:50 -0600 Subject: [PATCH 20/31] Extend package-specific RuntimeException Since it exists... --- src/Exception/CircularReferenceException.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Exception/CircularReferenceException.php b/src/Exception/CircularReferenceException.php index 3e0e41d4..deba872f 100644 --- a/src/Exception/CircularReferenceException.php +++ b/src/Exception/CircularReferenceException.php @@ -9,7 +9,6 @@ namespace Zend\Permissions\Rbac\Exception; -class CircularReferenceException extends \RuntimeException implements - ExceptionInterface +class CircularReferenceException extends RuntimeException implements ExceptionInterface { } From b9098a8e40f6de8add7f92237a775558cfc906f6 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:17:43 -0600 Subject: [PATCH 21/31] Remove redundant docblock annotations If a param or return annotation does not provide information beyond the typing and name already present in the signature, they are redundant. --- src/RoleInterface.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/RoleInterface.php b/src/RoleInterface.php index a858971f..542f2cbd 100644 --- a/src/RoleInterface.php +++ b/src/RoleInterface.php @@ -13,32 +13,21 @@ interface RoleInterface { /** * Get the name of the role. - * - * @return string */ public function getName() : string; /** * Add permission to the role. - * - * @param $name - * @return void */ public function addPermission(string $name) : void; /** * Checks if a permission exists for this role or any child roles. - * - * @param string $name - * @return bool */ public function hasPermission(string $name) : bool; /** * Add a child. - * - * @param RoleInterface $child - * @return void */ public function addChild(RoleInterface $child) : void; @@ -51,9 +40,6 @@ public function getChildren() : array; /** * Add a parent. - * - * @param RoleInterface $parent - * @return void */ public function addParent(RoleInterface $parent) : void; From 2e8430fef1c2b94e1fa457057c9505784965e508 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:26:38 -0600 Subject: [PATCH 22/31] Make CallbackAssertion type-safe - Adds a `callable` typehint to the `$callback` argument of the constructor. - Decorates the callback in a closure to ensure type safety when invoked; no more casting! --- src/Assertion/CallbackAssertion.php | 21 +++++++------- test/Assertion/CallbackAssertionTest.php | 35 ++++++++++-------------- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index bef4169f..6d6d89b8 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -19,17 +19,18 @@ class CallbackAssertion implements AssertionInterface /** * @var callable */ - protected $callback; + private $callback; - /** - * @param callable $callback The assertion callback - */ - public function __construct($callback) + public function __construct(callable $callback) { - if (! is_callable($callback)) { - throw new InvalidArgumentException('Invalid callback provided; not callable'); - } - $this->callback = $callback; + // Cast callable to a closure to enforce type safety. + $this->callback = function ( + Rbac $rbac, + RoleInterface $role = null, + string $permission = null + ) use ($callback) : bool { + return $callback($rbac, $role, $permission); + }; } /** @@ -37,6 +38,6 @@ public function __construct($callback) */ public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool { - return (bool) ($this->callback)($rbac, $role, $permission); + return ($this->callback)($rbac, $role, $permission); } } diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 1563f3be..d2c4cf14 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -9,30 +9,24 @@ namespace ZendTest\Permissions\Rbac\Assertion; +use Closure; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; +use stdClass; use Zend\Permissions\Rbac; class CallbackAssertionTest extends TestCase { - /** - * Ensures constructor throws InvalidArgumentException if not callable is provided - */ - public function testConstructorThrowsExceptionIfNotCallable() - { - $this->expectException(Rbac\Exception\InvalidArgumentException::class); - $this->expectExceptionMessage('Invalid callback provided; not callable'); - new Rbac\Assertion\CallbackAssertion('I am not callable!'); - } - /** * Ensures callback is set in object */ - public function testCallbackIsSet() + public function testCallbackIsDecoratedAsClosure() { - $callback = function () { + $callback = function () { }; - $assert = new Rbac\Assertion\CallbackAssertion($callback); - $this->assertAttributeSame($callback, 'callback', $assert); + $assert = new Rbac\Assertion\CallbackAssertion($callback); + $this->assertAttributeNotSame($callback, 'callback', $assert); + $this->assertAttributeInstanceOf(Closure::class, 'callback', $assert); } /** @@ -40,13 +34,12 @@ public function testCallbackIsSet() */ public function testAssertMethodPassRbacToCallback() { - $rbac = new Rbac\Rbac(); - $that = $this; - $assert = new Rbac\Assertion\CallbackAssertion(function ($rbacArg) use ($that, $rbac) { - $that->assertSame($rbacArg, $rbac); + $rbac = new Rbac\Rbac(); + $assert = new Rbac\Assertion\CallbackAssertion(function ($rbacArg) use ($rbac) { + Assert::assertSame($rbacArg, $rbac); return false; }); - $foo = new Rbac\Role('foo'); + $foo = new Rbac\Role('foo'); $foo->addPermission('can.foo'); $rbac->addRole($foo); $this->assertFalse($rbac->isGranted($foo, 'can.foo', $assert)); @@ -63,7 +56,7 @@ public function testAssertMethod() $assertRoleMatch = function ($role) { return function ($rbac) use ($role) { - return $role->getName() == 'foo'; + return $role->getName() === 'foo'; }; }; @@ -102,7 +95,7 @@ public function testAssertWithInvalidValue() $foo->addPermission('can.foo'); $rbac->addRole($foo); - $callable = new \stdClass(); + $callable = new stdClass(); $this->expectException(Rbac\Exception\InvalidArgumentException::class); $rbac->isGranted('foo', 'can.foo', $callable); } From 0f792b25674d587df476e0e22498f30af3f89c50 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:27:21 -0600 Subject: [PATCH 23/31] Do not wrap if line is not over length limit --- src/Exception/InvalidArgumentException.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index b7e9aed5..759c2250 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -9,7 +9,6 @@ namespace Zend\Permissions\Rbac\Exception; -class InvalidArgumentException extends \InvalidArgumentException implements - ExceptionInterface +class InvalidArgumentException extends \InvalidArgumentException implements ExceptionInterface { } From 125f3aa2e406599f57db8c0f8551d8b70384c9ea Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:27:40 -0600 Subject: [PATCH 24/31] Do not wrap line if it does not exceed line limit --- src/Exception/RuntimeException.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Exception/RuntimeException.php b/src/Exception/RuntimeException.php index 597bdea4..afbcd277 100644 --- a/src/Exception/RuntimeException.php +++ b/src/Exception/RuntimeException.php @@ -9,7 +9,6 @@ namespace Zend\Permissions\Rbac\Exception; -class RuntimeException extends \RuntimeException implements - ExceptionInterface +class RuntimeException extends \RuntimeException implements ExceptionInterface { } From e0ced4c7eca286377e138642b366e89fece58c64 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:35:11 -0600 Subject: [PATCH 25/31] Consistency updates for Rbac class - Remove redundant `@param` and `@return` annotations. - Add descriptions for `@throws` annotations. - Remove unnecessary whitespace. - Add necessary whitespace. - Throw exceptions only from conditions. --- src/Rbac.php | 62 +++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/src/Rbac.php b/src/Rbac.php index 605699c7..e37f31d2 100644 --- a/src/Rbac.php +++ b/src/Rbac.php @@ -24,18 +24,11 @@ class Rbac */ protected $createMissingRoles = false; - /** - * @param bool $createMissingRoles - * @return void - */ public function setCreateMissingRoles(bool $createMissingRoles) : void { $this->createMissingRoles = $createMissingRoles; } - /** - * @return bool - */ public function getCreateMissingRoles() : bool { return $this->createMissingRoles; @@ -44,10 +37,10 @@ public function getCreateMissingRoles() : bool /** * Add a child. * - * @param string|RoleInterface $child - * @param array|RoleInterface|null $parents - * @return void - * @throws Exception\InvalidArgumentException + * @param string|RoleInterface $role + * @param null|array|RoleInterface $parents + * @throws Exception\InvalidArgumentException if $role is not a string or + * RoleInterface. */ public function addRole($role, $parents = null) : void { @@ -61,9 +54,7 @@ public function addRole($role, $parents = null) : void } if ($parents) { - if (! is_array($parents)) { - $parents = [$parents]; - } + $parents = is_array($parents) ? $parents : [$parents]; foreach ($parents as $parent) { if ($this->createMissingRoles && ! $this->hasRole($parent)) { $this->addRole($parent); @@ -74,6 +65,7 @@ public function addRole($role, $parents = null) : void $parent->addChild($role); } } + $this->roles[$role->getName()] = $role; } @@ -81,7 +73,6 @@ public function addRole($role, $parents = null) : void * Is a role registered? * * @param RoleInterface|string $role - * @return bool */ public function hasRole($role) : bool { @@ -90,9 +81,11 @@ public function hasRole($role) : bool 'Role must be a string or implement Zend\Permissions\Rbac\RoleInterface' ); } + if (is_string($role)) { return isset($this->roles[$role]); } + $roleName = $role->getName(); return isset($this->roles[$roleName]) && $this->roles[$roleName] === $role; @@ -101,9 +94,7 @@ public function hasRole($role) : bool /** * Get a registered role by name * - * @param string $roleName - * @return RoleInterface - * @throws Exception\InvalidArgumentException + * @throws Exception\InvalidArgumentException if role is not found. */ public function getRole(string $roleName) : RoleInterface { @@ -119,13 +110,12 @@ public function getRole(string $roleName) : RoleInterface /** * Determines if access is granted by checking the role and child roles for permission. * - * @param RoleInterface|string $role - * @param string $permission - * @param AssertionInterface|Callable|null $assert - * @return bool - * @throws Exception\InvalidArgumentException + * @param RoleInterface|string $role + * @param null|AssertionInterface|Callable $assertion + * @throws Exception\InvalidArgumentException if the role is not found. + * @throws Exception\InvalidArgumentException if the assertion is an invalid type. */ - public function isGranted($role, string $permission, $assert = null) : bool + public function isGranted($role, string $permission, $assertion = null) : bool { if (! $this->hasRole($role)) { throw new Exception\InvalidArgumentException(sprintf( @@ -133,21 +123,29 @@ public function isGranted($role, string $permission, $assert = null) : bool is_object($role) ? $role->getName() : $role )); } + if (is_string($role)) { $role = $this->getRole($role); } + $result = $role->hasPermission($permission); - if (false === $result || null === $assert) { + if (false === $result || null === $assertion) { return $result; } - if ($assert instanceof AssertionInterface) { - return $result && $assert->assert($this, $role, $permission); + + if (! $assertion instanceof AssertionInterface + && ! is_callable($assertion) + ) { + throw new Exception\InvalidArgumentException( + 'Assertions must be a Callable or an instance of Zend\Permissions\Rbac\AssertionInterface' + ); } - if (is_callable($assert)) { - return $result && $assert($this, $role, $permission); + + if ($assertion instanceof AssertionInterface) { + return $result && $assertion->assert($this, $role, $permission); } - throw new Exception\InvalidArgumentException( - 'Assertions must be a Callable or an instance of Zend\Permissions\Rbac\AssertionInterface' - ); + + // Callable assertion provided. + return $result && $assertion($this, $role, $permission); } } From def52906ff79001fe466cf5407214af0f6a9090f Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 21 Feb 2018 16:38:26 -0600 Subject: [PATCH 26/31] Fix method docblocks - Removes redundant `@param` and `@return` annotations. - Corrects `@throws` annotations. - Adds whitespace where necessary. - Removes whitespace where necessary. --- src/Role.php | 55 ++++++++++++++++++++-------------------------------- 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/src/Role.php b/src/Role.php index b14d3b5f..fa41aa21 100644 --- a/src/Role.php +++ b/src/Role.php @@ -31,29 +31,21 @@ class Role implements RoleInterface */ protected $permissions = []; - /** - * @param string $name - */ - public function __construct($name) + public function __construct(string $name) { $this->name = $name; } /** * Get the name of the role. - * - * @return string */ - public function getName(): string + public function getName() : string { return $this->name; } /** - * Add permission to the role. - * - * @param $name - * @return void + * Add a permission to the role. */ public function addPermission(string $name) : void { @@ -62,39 +54,37 @@ public function addPermission(string $name) : void /** * Checks if a permission exists for this role or any child roles. - * - * @param string $name - * @return bool */ public function hasPermission(string $name) : bool { if (isset($this->permissions[$name])) { return true; } + foreach ($this->children as $child) { if ($child->hasPermission($name)) { return true; } } + return false; } /** - * Add a child + * Add a child rold. * - * @param RoleInterface $child - * @return void - * @throws Exception\RuntimeException + * @throws Exception\CircularReferenceException */ public function addChild(RoleInterface $child) : void { $childName = $child->getName(); if ($this->hasAncestor($child)) { throw new Exception\CircularReferenceException(sprintf( - "To prevent circular references, you cannot add role '%s' as child", + 'To prevent circular references, you cannot add role "%s" as child', $childName )); } + if (! isset($this->children[$childName])) { $this->children[$childName] = $child; $child->addParent($this); @@ -102,26 +92,25 @@ public function addChild(RoleInterface $child) : void } /** - * Check if a role is an ancestor - * - * @param RoleInterface $role - * @return bool + * Check if a role is an ancestor. */ protected function hasAncestor(RoleInterface $role) : bool { if (isset($this->parents[$role->getName()])) { return true; } + foreach ($this->parents as $parent) { if ($parent->hasAncestor($role)) { return true; } } + return false; } /** - * Get the children roles + * Get all child roles * * @return RoleInterface[] */ @@ -131,21 +120,20 @@ public function getChildren() : array } /** - * Add a parent role + * Add a parent role. * - * @param RoleInterface $parent - * @return void - * @throws Exception\RuntimeException + * @throws Exception\CircularReferenceException */ public function addParent(RoleInterface $parent) : void { $parentName = $parent->getName(); if ($this->hasDescendant($parent)) { throw new Exception\CircularReferenceException(sprintf( - "To prevent circular references, you cannot add role '%s' as parent", + 'To prevent circular references, you cannot add role "%s" as parent', $parentName )); } + if (! isset($this->parents[$parentName])) { $this->parents[$parentName] = $parent; $parent->addChild($this); @@ -153,26 +141,25 @@ public function addParent(RoleInterface $parent) : void } /** - * Check if a role is a descendant - * - * @param RoleInterface $role - * @return bool + * Check if a role is a descendant. */ protected function hasDescendant(RoleInterface $role) : bool { if (isset($this->children[$role->getName()])) { return true; } + foreach ($this->children as $child) { if ($child->hasDescendant($role)) { return true; } } + return false; } /** - * Get the parent roles + * Get the parent roles. * * @return RoleInterface[] */ From b1c91bb23060db5dc72d946dc0b179e63cf35ea0 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 22 Feb 2018 18:16:04 +0100 Subject: [PATCH 27/31] Added migation guide + removed optionals in AssertionInterface --- CHANGELOG.md | 14 ++++----- doc/book/methods.md | 6 ++-- doc/book/migration/to-v3-0.md | 36 +++++++++++++++++++++++ mkdocs.yml | 3 +- src/Assertion/CallbackAssertion.php | 2 +- src/AssertionInterface.php | 2 +- test/TestAsset/RoleMustMatchAssertion.php | 2 +- test/TestAsset/SimpleFalseAssertion.php | 2 +- test/TestAsset/SimpleTrueAssertion.php | 2 +- 9 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 doc/book/migration/to-v3-0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ec7e93..963dfb40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,19 +17,15 @@ All notable changes to this project will be documented in this file, in reverse strings are no longer accepted. - [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) updates - the `Role::addParent(RoleInterface $parent)` method to only accept a - `RoleInterface` parameter; strings are no longer accepted. - -- [#34](https://github.com/zendframework/zend-permissions-rbac/pull/34) updates - the `Zend\Permissions\Rbac\AssertionInterface`, adding two optional parameters - to the `assert()` definition and defining a return type, so that it now reads - as follows: + the `Zend\Permissions\Rbac\AssertionInterface`, adding two parameters to the + `assert()` definition and defining a return type, so that it now reads as + follows: ```php public function assert( Rbac $rbac, - string $permission = null, - RoleInterface $role = null + RoleInterface $role = null, + string $permission ) : bool ``` diff --git a/doc/book/methods.md b/doc/book/methods.md index 2da8b44f..d3a3a2fa 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -20,9 +20,9 @@ Method signature | Description Custom assertions can be provided to `Rbac::isGranted()` (see below); such assertions are provided the `Rbac` instance on invocation. -Method signature | Description ---------------------------------------------------------------------------- | ----------- -`assert(Rbac $rbac, RoleInterface $role = null, string $permission = null)` | Given an RBAC, an optional role, and an optional permission, determine if permission is granted. +Method signature | Description +-------------------------------------------------------------------- | ----------- +`assert(Rbac $rbac, RoleInterface $role, string $permission) : bool` | Given an RBAC, a role, and a permission, determine if permission is granted. ## `Zend\Permissions\Rbac\Rbac` diff --git a/doc/book/migration/to-v3-0.md b/doc/book/migration/to-v3-0.md new file mode 100644 index 00000000..7818a9bc --- /dev/null +++ b/doc/book/migration/to-v3-0.md @@ -0,0 +1,36 @@ +# Upgrading to 3.0 + +If you upgrade from v2.X you will notice a few changes. The main change is the +`Zend\Permissiones\Rbac\AssertionInterface::assert` function definition. + +## AssertionInterface + +The new `assert` functions looks as follows: + +```php +public function assert(Rbac $rbac, RoleInterface $role, string $permission): bool; +``` + +In v2.X we had only the first parameter $rbac. In V3.0 we added `$role` and +`$permission` parameters. This will simplify the implementation of dynamic +assertion, using the Role and the permission information. For instance, imagine +you want to disable a specific permission `foo` for an `admin` role, you can +implement a simple function as follows: + +``` +public function assert(Rbac $rbac, RoleInterface $role, string $permission): bool +{ + return !($permission === 'foo' && $role->getName() === 'admin'); +} +``` + +## Removed Role::setParent() + +In v3.0 we removed the function `Role::setParent()` in favor of `Role::addParent()`. +This function is more consistent with the others function naming like +`Role::addChild()`. + +## Removed the support of string in Role::addChild() + +In v3.0 you cannot add a child using a role name as string. You can only add +a `RoleInterface` object. diff --git a/mkdocs.yml b/mkdocs.yml index a04995d9..59b3b516 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -6,8 +6,9 @@ pages: - Reference: - Methods: methods.md - Examples: examples.md + - Migration: + - 'v2.X to v3.0': migration/to-v3-0.md site_name: zend-permissions-rbac site_description: zend-permissions-rbac repo_url: 'https://github.com/zendframework/zend-permissions-rbac' copyright: 'Copyright (c) 2016 Zend Technologies USA Inc.' - diff --git a/src/Assertion/CallbackAssertion.php b/src/Assertion/CallbackAssertion.php index 6d6d89b8..d92d8a60 100644 --- a/src/Assertion/CallbackAssertion.php +++ b/src/Assertion/CallbackAssertion.php @@ -36,7 +36,7 @@ public function __construct(callable $callback) /** * {@inheritdoc} */ - public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool + public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool { return ($this->callback)($rbac, $role, $permission); } diff --git a/src/AssertionInterface.php b/src/AssertionInterface.php index bee9557d..fa6e329e 100644 --- a/src/AssertionInterface.php +++ b/src/AssertionInterface.php @@ -14,5 +14,5 @@ interface AssertionInterface /** * Assertion method - must return a boolean. */ - public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool; + public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool; } diff --git a/test/TestAsset/RoleMustMatchAssertion.php b/test/TestAsset/RoleMustMatchAssertion.php index 0ce2086e..2a2a6eec 100644 --- a/test/TestAsset/RoleMustMatchAssertion.php +++ b/test/TestAsset/RoleMustMatchAssertion.php @@ -16,7 +16,7 @@ class RoleMustMatchAssertion implements AssertionInterface { - public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool + public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool { return $role->getName() === 'foo'; } diff --git a/test/TestAsset/SimpleFalseAssertion.php b/test/TestAsset/SimpleFalseAssertion.php index 8ef14217..b30ab83d 100644 --- a/test/TestAsset/SimpleFalseAssertion.php +++ b/test/TestAsset/SimpleFalseAssertion.php @@ -15,7 +15,7 @@ class SimpleFalseAssertion implements AssertionInterface { - public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool + public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool { return false; } diff --git a/test/TestAsset/SimpleTrueAssertion.php b/test/TestAsset/SimpleTrueAssertion.php index 8a9177d6..5644e12d 100644 --- a/test/TestAsset/SimpleTrueAssertion.php +++ b/test/TestAsset/SimpleTrueAssertion.php @@ -15,7 +15,7 @@ class SimpleTrueAssertion implements AssertionInterface { - public function assert(Rbac $rbac, RoleInterface $role = null, string $permission = null) : bool + public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool { return true; } From 6d934ceceb33c46374fc42462e200dc3497b13c3 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 22 Feb 2018 13:06:22 -0600 Subject: [PATCH 28/31] Fixes assert() signature in CHANGELOG entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 963dfb40..afc74358 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ All notable changes to this project will be documented in this file, in reverse ```php public function assert( Rbac $rbac, - RoleInterface $role = null, + RoleInterface $role, string $permission ) : bool ``` From ee968a6f67819c8eb7e789e27ceedba0a8da9c73 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 22 Feb 2018 13:07:17 -0600 Subject: [PATCH 29/31] Fixes grammar of entry detailing circular reference fixes --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afc74358..5b58d2ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,8 +53,8 @@ All notable changes to this project will be documented in this file, in reverse ### Fixed - [#30](https://github.com/zendframework/zend-permissions-rbac/issues/30) fixes - circular references within the used in `Role::addChild()` and - `Role::addParent()` algorithms. + circular references within the `Role::addChild()` and `Role::addParent()` + algorithms. ## 2.6.0 - 2018-02-01 From 23275ecabdf5e2f18cbd4a6484b861aebe56b8c1 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 22 Feb 2018 13:24:31 -0600 Subject: [PATCH 30/31] Completes migration notes Adds a complete section on `RoleInterface` changes, incorporating the sections on `setParent` and `addChild` changes, and adding information on type hint changes, renaming of getParent to getParents, and addition of getChildren. --- doc/book/migration/to-v3-0.md | 87 +++++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/doc/book/migration/to-v3-0.md b/doc/book/migration/to-v3-0.md index 7818a9bc..cc30fcae 100644 --- a/doc/book/migration/to-v3-0.md +++ b/doc/book/migration/to-v3-0.md @@ -1,36 +1,85 @@ # Upgrading to 3.0 -If you upgrade from v2.X you will notice a few changes. The main change is the -`Zend\Permissiones\Rbac\AssertionInterface::assert` function definition. +If you upgrade from version 2 releases, you will notice a few changes. This +document details the changes + +## Minimum supported PHP version + +Version 3 drops support for PHP versions prior to PHP 7.1. ## AssertionInterface -The new `assert` functions looks as follows: +The primary change is the `Zend\Permissions\Rbac\AssertionInterface::assert()` +method definition. + +The new `assert` method has the following signature: ```php -public function assert(Rbac $rbac, RoleInterface $role, string $permission): bool; +namespace Zend\Permissions\Rbac; + +public function assert( + Rbac $rbac, + RoleInterface $role, + string $permission +) : bool ``` -In v2.X we had only the first parameter $rbac. In V3.0 we added `$role` and -`$permission` parameters. This will simplify the implementation of dynamic -assertion, using the Role and the permission information. For instance, imagine -you want to disable a specific permission `foo` for an `admin` role, you can -implement a simple function as follows: +The version 2 releases defined the method such that it only accepted a single +parameter, `Rbac $rbac`. Version 3 adds the `$role` and `$permission` +parameters. This simplifies implementation of dynamic assertions using the role +and the permission information. -``` -public function assert(Rbac $rbac, RoleInterface $role, string $permission): bool +For instance, imagine you want to disable a specific permission `foo` for an +`admin` role; you can implement that as follows: + +```php +public function assert(Rbac $rbac, RoleInterface $role, string $permission) : bool { - return !($permission === 'foo' && $role->getName() === 'admin'); + return ! ($permission === 'foo' && $role->getName() === 'admin'); } ``` -## Removed Role::setParent() +If you were previously implementing `AssertionInterface`, you will need to +update the `assert()` signature to match the changes in version 3. + +If you were creating assertions as PHP callables, you may continue to use the +existing signature; however, you may also expand them to accept the new +arguments should they assist you in creating more complex, dynamic assertions. + +## RoleInterface + +`Zend\Permissions\Rbac\RoleInterface` also received a number of changes, +including type hints and method name changes. + +### Type hints + +With the update to [PHP 7.1](#minimum-supported-php-version), we also updated +the `RoleInterface` to provide: + +- scalar type hints where applicable (`addPermission()` and `hasPermission()`). +- add return type hints (including scalar type hints) to all methods. + +You will need to examine the `RoleInterface` definitions to determine what +changes to make to your implementations. + +### setParent becomes addParent + +In version 3, we renamed the method `Role::setParent()` to `Role::addParent()`. +This naming is more consistent with other method names, such as +`Role::addChild()`, and also makes clear that more than one parent may be +provided to any given role. + +### getParent becomes getParents + +In line with the previous change, `getParent()` was also renamed to +`getParents()`, which returns an array of `RoleInterface` instances. + +### Removed support for string arguments in Role::addChild -In v3.0 we removed the function `Role::setParent()` in favor of `Role::addParent()`. -This function is more consistent with the others function naming like -`Role::addChild()`. +Version 3 no longer allows adding a child using a string role name; you may only +provide `RoleInterface` instances. -## Removed the support of string in Role::addChild() +### Adds getChildren -In v3.0 you cannot add a child using a role name as string. You can only add -a `RoleInterface` object. +Since roles may have multiple children, the method `getChildren()` was added; it +returns an array of `RoleInterface` instances. From 7a0dbf8007bcb8b74ceba1308686fd427137c3fd Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Thu, 1 Mar 2018 18:19:21 +0100 Subject: [PATCH 31/31] Added backslash for pipe in docs --- doc/book/methods.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/book/methods.md b/doc/book/methods.md index d3a3a2fa..c88113bc 100644 --- a/doc/book/methods.md +++ b/doc/book/methods.md @@ -31,9 +31,9 @@ order to query for permissions. Method signature | Description --------------------------------------------------------------------------- | ----------- -`addRole(string|RoleInterface $child, array|RoleInterface $parents = null)` | Add a role to the RBAC. If `$parents` is non-null, the `$child` is also added to any parents provided. +`addRole(string\|RoleInterface $child, array\|RoleInterface $parents = null)` | Add a role to the RBAC. If `$parents` is non-null, the `$child` is also added to any parents provided. `getRole(string $role) : RoleInterface` | Get the role specified by name, raising an exception if not found. -`hasRole(string|RoleInterface $role) : bool` | Recursively queries the RBAC for the given role, returning `true` if found, `false` otherwise. +`hasRole(string\|RoleInterface $role) : bool` | Recursively queries the RBAC for the given role, returning `true` if found, `false` otherwise. `getCreateMissingRoles() : bool` | Retrieve the flag that determines whether or not `$parent` roles are added automatically if not present when calling `addRole()`. `setCreateMissingRoles(bool $flag) : void` | Set the flag that determines whether or not `$parent` roles are added automatically if not present when calling `addRole()`. -`isGranted(string|RoleInterface $role, string $permission, $assert = null)` | Determine if the role has the given permission. If `$assert` is provided and either an `AssertInterface` instance or callable, it will be queried before checking against the given role. +`isGranted(string\|RoleInterface $role, string $permission, $assert = null)` | Determine if the role has the given permission. If `$assert` is provided and either an `AssertInterface` instance or callable, it will be queried before checking against the given role.