From 5f5e1e73280c024c5aae1a4ae174be96078ad893 Mon Sep 17 00:00:00 2001 From: JM Leroux Date: Thu, 12 Jun 2014 21:01:25 +0200 Subject: [PATCH 01/20] Add route permissions guard --- .../Factory/RoutePermissionsGuardFactory.php | 67 ++++++++++++ .../Guard/RoutePermissionsGuard.php | 100 ++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php create mode 100644 tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php diff --git a/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php b/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php new file mode 100644 index 0000000..4fd08f5 --- /dev/null +++ b/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php @@ -0,0 +1,67 @@ + + * @licence MIT + */ +class RoutePermissionsGuardFactory implements FactoryInterface, MutableCreationOptionsInterface +{ + /** + * @var array + */ + protected $options = []; + + /** + * {@inheritDoc} + */ + public function setCreationOptions(array $options) + { + $this->options = $options; + } + + /** + * {@inheritDoc} + * @return RouteGuard + */ + public function createService(ServiceLocatorInterface $serviceLocator) + { + $parentLocator = $serviceLocator->getServiceLocator(); + + /* @var \ZfcRbac\Options\ModuleOptions $moduleOptions */ + $moduleOptions = $parentLocator->get('ZfcRbac\Options\ModuleOptions'); + + /* @var \ZfcRbac\Service\RoleService $roleService */ + $roleService = $parentLocator->get('ZfcRbac\Service\AuthorizationService'); + + $routeGuard = new RoutePermissionsGuard($roleService, $this->options); + $routeGuard->setProtectionPolicy($moduleOptions->getProtectionPolicy()); + + return $routeGuard; + } +} diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php new file mode 100644 index 0000000..d1c79ce --- /dev/null +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php @@ -0,0 +1,100 @@ + + * @licence MIT + */ +class RoutePermissionsGuard extends AbstractGuard +{ + use ProtectionPolicyTrait; + + /** + * @var RoleService + */ + protected $roleService; + + /** + * Route guard rules + * Those rules are an associative array that map a rule with one or multiple roles + * @var array + */ + protected $rules = []; + + public function __construct(AuthorizationService $roleService, array $rules = []) + { + $this->roleService = $roleService; + $this->setRules($rules); + } + + /** + * Set the rules (it overrides any existing rules) + * @param array $rules + * @return void + */ + public function setRules(array $rules) + { + $this->rules = []; + foreach ($rules as $key => $value) { + if (is_int($key)) { + $routeRegex = $value; + $permissions = []; + } else { + $routeRegex = $key; + $permissions = (array)$value; + } + $this->rules[$routeRegex] = $permissions; + } + } + + /** + * {@inheritDoc} + */ + public function isGranted(MvcEvent $event) + { + $matchedRouteName = $event->getRouteMatch()->getMatchedRouteName(); + $allowedPermissions = null; + + foreach (array_keys($this->rules) as $routeRule) { + if (fnmatch($routeRule, $matchedRouteName, FNM_CASEFOLD)) { + $allowedPermissions = $this->rules[$routeRule]; + break; + } + } + // If no rules apply, it is considered as granted or not based on the protection policy + if (null === $allowedPermissions) { + return $this->protectionPolicy === self::POLICY_ALLOW; + } + if (in_array('*', $allowedPermissions)) { + return true; + } + foreach ($allowedPermissions as $permission) { + if ($this->roleService->isGranted($permission)) { + return true; + } + } + return false; + } +} From b3dda6d0fec57c32139521cb8b478a29f993d700 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Fri, 13 Jun 2014 13:42:22 +0200 Subject: [PATCH 02/20] Moved to src + Docblock --- .../Factory/RoutePermissionsGuardFactory.php | 3 ++- .../ZfcRbac}/Guard/RoutePermissionsGuard.php | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) rename {tests/ZfcRbacTest => src/ZfcRbac}/Factory/RoutePermissionsGuardFactory.php (95%) rename {tests/ZfcRbacTest => src/ZfcRbac}/Guard/RoutePermissionsGuard.php (91%) diff --git a/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php b/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php similarity index 95% rename from tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php rename to src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php index 4fd08f5..734310b 100644 --- a/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactory.php +++ b/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php @@ -25,9 +25,10 @@ use ZfcRbac\Guard\RoutePermissionsGuard; /** - * Create a route guard + * Create a route guard for checking permissions * * @author Michaël Gallego + * @author JM Lerouxw * @licence MIT */ class RoutePermissionsGuardFactory implements FactoryInterface, MutableCreationOptionsInterface diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php similarity index 91% rename from tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php rename to src/ZfcRbac/Guard/RoutePermissionsGuard.php index d1c79ce..e409c44 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -24,7 +24,9 @@ /** * A route guard can protect a route or a hierarchy of routes (using simple wildcard pattern) + * * @author Michaël Gallego + * @author JM Leroux * @licence MIT */ class RoutePermissionsGuard extends AbstractGuard @@ -43,6 +45,10 @@ class RoutePermissionsGuard extends AbstractGuard */ protected $rules = []; + /** + * @param AuthorizationService $roleService + * @param array $rules + */ public function __construct(AuthorizationService $roleService, array $rules = []) { $this->roleService = $roleService; @@ -51,6 +57,7 @@ public function __construct(AuthorizationService $roleService, array $rules = [] /** * Set the rules (it overrides any existing rules) + * * @param array $rules * @return void */ @@ -59,11 +66,11 @@ public function setRules(array $rules) $this->rules = []; foreach ($rules as $key => $value) { if (is_int($key)) { - $routeRegex = $value; + $routeRegex = $value; $permissions = []; } else { - $routeRegex = $key; - $permissions = (array)$value; + $routeRegex = $key; + $permissions = (array) $value; } $this->rules[$routeRegex] = $permissions; } @@ -83,18 +90,22 @@ public function isGranted(MvcEvent $event) break; } } + // If no rules apply, it is considered as granted or not based on the protection policy if (null === $allowedPermissions) { return $this->protectionPolicy === self::POLICY_ALLOW; } + if (in_array('*', $allowedPermissions)) { return true; } + foreach ($allowedPermissions as $permission) { if ($this->roleService->isGranted($permission)) { return true; } } + return false; } } From 78f8e66b5373bd11f02cf5958dc71940d03cd233 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Fri, 13 Jun 2014 13:58:27 +0200 Subject: [PATCH 03/20] route permissions guard factory test --- .../RoutePermissionsGuardFactoryTest.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactoryTest.php diff --git a/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactoryTest.php b/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactoryTest.php new file mode 100644 index 0000000..6aee9e5 --- /dev/null +++ b/tests/ZfcRbacTest/Factory/RoutePermissionsGuardFactoryTest.php @@ -0,0 +1,62 @@ + 'role' + ]; + + $options = new ModuleOptions([ + 'identity_provider' => 'ZfcRbac\Identity\AuthenticationProvider', + 'guards' => [ + 'ZfcRbac\Guard\RoutePermissionsGuard' => $creationOptions + ], + 'protection_policy' => GuardInterface::POLICY_ALLOW, + ]); + + $serviceManager = new ServiceManager(); + $serviceManager->setService('ZfcRbac\Options\ModuleOptions', $options); + $serviceManager->setService( + 'ZfcRbac\Service\AuthorizationService', + $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false) + ); + + $pluginManager = new GuardPluginManager(); + $pluginManager->setServiceLocator($serviceManager); + + $factory = new RoutePermissionsGuardFactory(); + $routeGuard = $factory->createService($pluginManager); + + $this->assertInstanceOf('ZfcRbac\Guard\RoutePermissionsGuard', $routeGuard); + $this->assertEquals(GuardInterface::POLICY_ALLOW, $routeGuard->getProtectionPolicy()); + } +} From 8e75f52ec713bf50e932894d6269323d14222fda Mon Sep 17 00:00:00 2001 From: jmleroux Date: Fri, 13 Jun 2014 14:05:23 +0200 Subject: [PATCH 04/20] typo --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index e409c44..fdba3da 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -20,7 +20,6 @@ use Zend\Mvc\MvcEvent; use ZfcRbac\Exception; use ZfcRbac\Service\AuthorizationService; -use ZfcRbac\Service\RoleService; /** * A route guard can protect a route or a hierarchy of routes (using simple wildcard pattern) @@ -34,24 +33,24 @@ class RoutePermissionsGuard extends AbstractGuard use ProtectionPolicyTrait; /** - * @var RoleService + * @var AuthorizationService */ - protected $roleService; + protected $authorizationService; /** * Route guard rules - * Those rules are an associative array that map a rule with one or multiple roles + * Those rules are an associative array that map a rule with one or multiple permissions * @var array */ protected $rules = []; /** - * @param AuthorizationService $roleService + * @param AuthorizationService $authorizationService * @param array $rules */ - public function __construct(AuthorizationService $roleService, array $rules = []) + public function __construct(AuthorizationService $authorizationService, array $rules = []) { - $this->roleService = $roleService; + $this->authorizationService = $authorizationService; $this->setRules($rules); } @@ -101,7 +100,7 @@ public function isGranted(MvcEvent $event) } foreach ($allowedPermissions as $permission) { - if ($this->roleService->isGranted($permission)) { + if ($this->authorizationService->isGranted($permission)) { return true; } } From 9539831148c243227ea70b9694db75a02868255f Mon Sep 17 00:00:00 2001 From: jmleroux Date: Fri, 13 Jun 2014 15:15:22 +0200 Subject: [PATCH 05/20] change permissions guard priority to 10 --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index fdba3da..12d609b 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -32,6 +32,8 @@ class RoutePermissionsGuard extends AbstractGuard { use ProtectionPolicyTrait; + const EVENT_PRIORITY = -10; + /** * @var AuthorizationService */ From ec8321efa7c1958c8f180b318ac770f9e21ea944 Mon Sep 17 00:00:00 2001 From: JM Leroux Date: Fri, 13 Jun 2014 18:52:01 +0200 Subject: [PATCH 06/20] Add route permissions guard tests --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 8 +- .../Guard/RoutePermissionsGuardTest.php | 329 ++++++++++++++++++ 2 files changed, 333 insertions(+), 4 deletions(-) create mode 100644 tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index 12d609b..86ea1ea 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -19,7 +19,7 @@ use Zend\Mvc\MvcEvent; use ZfcRbac\Exception; -use ZfcRbac\Service\AuthorizationService; +use ZfcRbac\Service\AuthorizationServiceInterface; /** * A route guard can protect a route or a hierarchy of routes (using simple wildcard pattern) @@ -35,7 +35,7 @@ class RoutePermissionsGuard extends AbstractGuard const EVENT_PRIORITY = -10; /** - * @var AuthorizationService + * @var AuthorizationServiceInterface */ protected $authorizationService; @@ -47,10 +47,10 @@ class RoutePermissionsGuard extends AbstractGuard protected $rules = []; /** - * @param AuthorizationService $authorizationService + * @param AuthorizationServiceInterface $authorizationService * @param array $rules */ - public function __construct(AuthorizationService $authorizationService, array $rules = []) + public function __construct(AuthorizationServiceInterface $authorizationService, array $rules = []) { $this->authorizationService = $authorizationService; $this->setRules($rules); diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php new file mode 100644 index 0000000..a04e730 --- /dev/null +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -0,0 +1,329 @@ +getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false)); + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager->expects($this->once())->method('attach')->with(RouteGuard::EVENT_NAME); + $guard->attach($eventManager); + } + + /** + * We want to ensure an order for guards + */ + public function testAssertRouteGuardPriorityIsHigherThanControllerGuardPriority() + { + $this->assertTrue(RouteGuard::EVENT_PRIORITY > ControllerGuard::EVENT_PRIORITY); + } + + public function rulesConversionProvider() + { + return [ + // Simple string to array conversion + [ + 'rules' => [ + 'route' => 'permission1' + ], + 'expected' => [ + 'route' => ['permission1'] + ] + ], + // Array to array + [ + 'rules' => [ + 'route' => ['permission1', 'permission2'] + ], + 'expected' => [ + 'route' => ['permission1', 'permission2'] + ] + ], + // Traversable to array + [ + 'rules' => [ + 'route' => new \ArrayIterator(['permission1', 'permission2']) + ], + 'expected' => [ + 'route' => ['permission1', 'permission2'] + ] + ], + // Block a route for everyone + [ + 'rules' => [ + 'route' + ], + 'expected' => [ + 'route' => [] + ] + ], + ]; + } + + /** + * @dataProvider rulesConversionProvider + */ + public function testRulesConversions(array $rules, array $expected) + { + $roleService = $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false); + $routeGuard = new RoutePermissionsGuard($roleService, $rules); + $reflProperty = new \ReflectionProperty($routeGuard, 'rules'); + $reflProperty->setAccessible(true); + $this->assertEquals($expected, $reflProperty->getValue($routeGuard)); + } + + public function routeDataProvider() + { + return [ + // Assert basic one-to-one mapping with both policies + [ + 'rules' => ['adminRoute' => 'admin'], + 'matchedRouteName' => 'adminRoute', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['adminRoute' => 'admin'], + 'matchedRouteName' => 'adminRoute', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that policy changes result for non-specified route guards + [ + 'rules' => ['route' => 'admin'], + 'matchedRouteName' => 'anotherRoute', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['route' => 'admin'], + 'matchedRouteName' => 'anotherRoute', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that composed route work for both policies + [ + 'rules' => ['admin/dashboard' => 'admin'], + 'matchedRouteName' => 'admin/dashboard', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['admin/dashboard' => 'admin'], + 'matchedRouteName' => 'admin/dashboard', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that wildcard route work for both policies + [ + 'rules' => ['admin/*' => 'admin'], + 'matchedRouteName' => 'admin/dashboard', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['admin/*' => 'admin'], + 'matchedRouteName' => 'admin/dashboard', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that wildcard route does match (or not depending on the policy) if rules is after matched route name + [ + 'rules' => ['fooBar/*' => 'admin'], + 'matchedRouteName' => 'admin/fooBar/baz', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['fooBar/*' => 'admin'], + 'matchedRouteName' => 'admin/fooBar/baz', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that it can grant access with multiple rules + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route1', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route1', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that it can grant/deny access with multiple rules based on the policy + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route3', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route3', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert it can deny access if a permission does not have access + [ + 'rules' => ['route' => 'admin'], + 'matchedRouteName' => 'route', + 'identityPermissions' => [['admin', null, false], ['guest', null, true]], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['route' => 'admin'], + 'matchedRouteName' => 'route', + 'identityPermissions' => [['admin', null, false], ['guest', null, true]], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert wildcard in permission + [ + 'rules' => ['home' => '*'], + 'matchedRouteName' => 'home', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['home' => '*'], + 'matchedRouteName' => 'home', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + ]; + } + + /** + * @dataProvider routeDataProvider + */ + public function testRouteGranted( + array $rules, + $matchedRouteName, + array $identityPermissions, + $isGranted, + $protectionPolicy + ) { + $event = new MvcEvent(); + $routeMatch = new RouteMatch([]); + $routeMatch->setMatchedRouteName($matchedRouteName); + $event->setRouteMatch($routeMatch); + $authorizationService = $this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false); + $authorizationService->expects($this->any()) + ->method('isGranted') + ->will($this->returnValueMap($identityPermissions)); + $routeGuard = new RoutePermissionsGuard($authorizationService, $rules); + $routeGuard->setProtectionPolicy($protectionPolicy); + $this->assertEquals($isGranted, $routeGuard->isGranted($event)); + } + + public function testProperlyFillEventOnAuthorization() + { + $event = new MvcEvent(); + $routeMatch = new RouteMatch([]); + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $application->expects($this->never())->method('getEventManager')->will($this->returnValue($eventManager)); + $routeMatch->setMatchedRouteName('adminRoute'); + $event->setRouteMatch($routeMatch); + $event->setApplication($application); + $identity = $this->getMock('ZfcRbac\Identity\IdentityInterface'); + $identity->expects($this->any())->method('getRoles')->will($this->returnValue(['member'])); + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); + $identityProvider->expects($this->any())->method('getIdentity')->will($this->returnValue($identity)); + $roleProvider = new InMemoryRoleProvider(['member']); + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $routeGuard = new RouteGuard($roleService, [ + 'adminRoute' => 'member' + ]); + $routeGuard->onResult($event); + $this->assertEmpty($event->getError()); + $this->assertNull($event->getParam('exception')); + } + + public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() + { + $event = new MvcEvent(); + $routeMatch = new RouteMatch([]); + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $application->expects($this->once())->method('getEventManager')->will($this->returnValue($eventManager)); + $eventManager->expects($this->once())->method('trigger')->with(MvcEvent::EVENT_DISPATCH_ERROR); + $routeMatch->setMatchedRouteName('adminRoute'); + $event->setRouteMatch($routeMatch); + $event->setApplication($application); + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); + $identityProvider->expects($this->any())->method('getIdentityRoles')->will($this->returnValue('member')); + $roleProvider = new InMemoryRoleProvider(['member', 'guest']); + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $routeGuard = new RouteGuard($roleService, [ + 'adminRoute' => 'guest' + ]); + $routeGuard->onResult($event); + $this->assertTrue($event->propagationIsStopped()); + $this->assertEquals(RouteGuard::GUARD_UNAUTHORIZED, $event->getError()); + $this->assertInstanceOf('ZfcRbac\Exception\UnauthorizedException', $event->getParam('exception')); + } +} From 7e250380908a837aa749b8414773fe511c366b94 Mon Sep 17 00:00:00 2001 From: JM Leroux Date: Fri, 13 Jun 2014 19:39:52 +0200 Subject: [PATCH 07/20] Update documentation 04.Guards --- docs/04. Guards.md | 70 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/docs/04. Guards.md b/docs/04. Guards.md index 329cbec..1e9aea2 100644 --- a/docs/04. Guards.md +++ b/docs/04. Guards.md @@ -19,10 +19,14 @@ And here is a simple workflow with a route guard: ![Zend Framework workflow with guards](/docs/images/workflow-with-guards.png?raw=true) -Guards are not really aware of permissions (it does not make any sense) but rather only think about "roles". For +RouteGuard and ControllerGuard are not aware of permissions but rather only think about "roles". For instance, you may want to refuse access to each routes that begin by "admin/*" to all users that do not have the "admin" role. +If you want to protect a route for a set set of permissions, you must use RoutePermissionsGuard. For instance, +you may want to grant access to a route "post/delete" only to roles having the "delete" permission. +Note that in a RBAC system, a permission is linked to a role, not to a user. + Albeit simple to use, guards should not be the only protection in your application, and you should always also protect your service. The reason is that your business logic should be handled by your service. Protecting a given route or controller does not mean that the service cannot be access from elsewhere (another action for instance). @@ -57,7 +61,7 @@ deny policy is much more secure, but it needs much more configuration to work wi ## Built-in guards -ZfcRbac comes with two guards: RouteGuard and ControllerGuard. All guards must be added in the `guards` subkey: +ZfcRbac comes with three guards: RouteGuard, RoutePermissionsGuard and ControllerGuard. All guards must be added in the `guards` subkey: ```php return [ @@ -104,7 +108,7 @@ route to users that have the "guest" role (eg.: most likely unauthenticated user > The route pattern is not a regex. It only supports the wildcard (*) character, that replaces any segment. -You can also use the wildcard character for roles: +You can also use the wildcard character * for roles: ```php return [ @@ -151,6 +155,66 @@ return [ ``` +### RoutePermissionsGuard + +> The RoutePermissionsGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -10. + +The RoutePermissionsGuard allows to protect a route or a hierarchy of route. You must provide an array of "key" => "value", +where the key is a route pattern, and value an array of permissions names: + +```php +return [ + 'zfc_rbac' => [ + 'guards' => [ + 'ZfcRbac\Guard\RouteGuard' => [ + 'admin*' => ['admin'], + 'post/delete' => ['post.delete', 'admin'] + ] + ] + ] +]; +``` + +> Permissions are linked to roles, not to users + +Those rules grant access to all admin routes to roles that have the "admin" permission, and grant access to the +"post/delete" route to roles that have the "post.delete" or "admin" permissions. + +> The route pattern is not a regex. It only supports the wildcard (*) character, that replaces any segment. + +You can also use the wildcard character * for permissions: + +```php +return [ + 'zfc_rbac' => [ + 'guards' => [ + 'ZfcRbac\Guard\RouteGuard' => [ + 'home' => ['*'] + ] + ] + ] +]; +``` + +This rule grants access to the "home" route to anyone. + +Finally, you can also use an empty array to completly block a route, for maintenance purpose for example : + +```php +return [ + 'zfc_rbac' => [ + 'guards' => [ + 'ZfcRbac\Guard\RouteGuard' => [ + 'route_under_construction' => [] + ] + ] + ] +]; +``` + +This rule will be inaccessible. + + ### ControllerGuard The ControllerGuard allows to protect a controller. You must provide an array of array: From f20b7faf139c16154969a263497a5d8bf8bf9214 Mon Sep 17 00:00:00 2001 From: JM Leroux Date: Fri, 13 Jun 2014 19:53:51 +0200 Subject: [PATCH 08/20] Code format --- .../Guard/RoutePermissionsGuardTest.php | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index a04e730..2e9d260 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -35,9 +35,12 @@ class RoutePermissionsGuardTest extends \PHPUnit_Framework_TestCase { public function testAttachToRightEvent() { - $guard = new RoutePermissionsGuard($this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false)); $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); - $eventManager->expects($this->once())->method('attach')->with(RouteGuard::EVENT_NAME); + $eventManager->expects($this->once()) + ->method('attach') + ->with(RouteGuard::EVENT_NAME); + + $guard = new RoutePermissionsGuard($this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false)); $guard->attach($eventManager); } @@ -266,62 +269,87 @@ public function testRouteGranted( $isGranted, $protectionPolicy ) { - $event = new MvcEvent(); $routeMatch = new RouteMatch([]); $routeMatch->setMatchedRouteName($matchedRouteName); + + $event = new MvcEvent(); $event->setRouteMatch($routeMatch); + $authorizationService = $this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false); $authorizationService->expects($this->any()) ->method('isGranted') ->will($this->returnValueMap($identityPermissions)); + $routeGuard = new RoutePermissionsGuard($authorizationService, $rules); $routeGuard->setProtectionPolicy($protectionPolicy); + $this->assertEquals($isGranted, $routeGuard->isGranted($event)); } public function testProperlyFillEventOnAuthorization() { - $event = new MvcEvent(); - $routeMatch = new RouteMatch([]); - $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); - $application->expects($this->never())->method('getEventManager')->will($this->returnValue($eventManager)); + + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $application->expects($this->never()) + ->method('getEventManager') + ->will($this->returnValue($eventManager)); + + $routeMatch = new RouteMatch([]); $routeMatch->setMatchedRouteName('adminRoute'); + + $event = new MvcEvent(); $event->setRouteMatch($routeMatch); $event->setApplication($application); + $identity = $this->getMock('ZfcRbac\Identity\IdentityInterface'); $identity->expects($this->any())->method('getRoles')->will($this->returnValue(['member'])); + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); $identityProvider->expects($this->any())->method('getIdentity')->will($this->returnValue($identity)); + $roleProvider = new InMemoryRoleProvider(['member']); $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $routeGuard = new RouteGuard($roleService, [ 'adminRoute' => 'member' ]); $routeGuard->onResult($event); + $this->assertEmpty($event->getError()); $this->assertNull($event->getParam('exception')); } public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() { - $event = new MvcEvent(); - $routeMatch = new RouteMatch([]); - $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); - $application->expects($this->once())->method('getEventManager')->will($this->returnValue($eventManager)); - $eventManager->expects($this->once())->method('trigger')->with(MvcEvent::EVENT_DISPATCH_ERROR); + $eventManager->expects($this->once()) + ->method('trigger') + ->with(MvcEvent::EVENT_DISPATCH_ERROR); + + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $application->expects($this->once()) + ->method('getEventManager') + ->will($this->returnValue($eventManager)); + + $routeMatch = new RouteMatch([]); $routeMatch->setMatchedRouteName('adminRoute'); + + $event = new MvcEvent(); $event->setRouteMatch($routeMatch); $event->setApplication($application); + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); $identityProvider->expects($this->any())->method('getIdentityRoles')->will($this->returnValue('member')); + $roleProvider = new InMemoryRoleProvider(['member', 'guest']); $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $routeGuard = new RouteGuard($roleService, [ 'adminRoute' => 'guest' ]); $routeGuard->onResult($event); + $this->assertTrue($event->propagationIsStopped()); $this->assertEquals(RouteGuard::GUARD_UNAUTHORIZED, $event->getError()); $this->assertInstanceOf('ZfcRbac\Exception\UnauthorizedException', $event->getParam('exception')); From 8f783259623e9e9eae8c7f7645c640816fea2f6f Mon Sep 17 00:00:00 2001 From: JM Leroux Date: Fri, 13 Jun 2014 20:37:31 +0200 Subject: [PATCH 09/20] Update documentation 04.Guards --- docs/04. Guards.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/04. Guards.md b/docs/04. Guards.md index 1e9aea2..6fa2cc9 100644 --- a/docs/04. Guards.md +++ b/docs/04. Guards.md @@ -166,7 +166,7 @@ where the key is a route pattern, and value an array of permissions names: return [ 'zfc_rbac' => [ 'guards' => [ - 'ZfcRbac\Guard\RouteGuard' => [ + 'ZfcRbac\Guard\RoutePermissionsGuard' => [ 'admin*' => ['admin'], 'post/delete' => ['post.delete', 'admin'] ] @@ -188,7 +188,7 @@ You can also use the wildcard character * for permissions: return [ 'zfc_rbac' => [ 'guards' => [ - 'ZfcRbac\Guard\RouteGuard' => [ + 'ZfcRbac\Guard\RoutePermissionsGuard' => [ 'home' => ['*'] ] ] @@ -204,7 +204,7 @@ Finally, you can also use an empty array to completly block a route, for mainten return [ 'zfc_rbac' => [ 'guards' => [ - 'ZfcRbac\Guard\RouteGuard' => [ + 'ZfcRbac\Guard\RoutePermissionsGuard' => [ 'route_under_construction' => [] ] ] From 26ff9369a672498d6c243d194577bf2345319a26 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 14:19:17 +0200 Subject: [PATCH 10/20] docblock, comments and variable names --- src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php b/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php index 734310b..f5b99f7 100644 --- a/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php +++ b/src/ZfcRbac/Factory/RoutePermissionsGuardFactory.php @@ -47,7 +47,7 @@ public function setCreationOptions(array $options) } /** - * {@inheritDoc} + * @param \Zend\ServiceManager\AbstractPluginManager|ServiceLocatorInterface $serviceLocator * @return RouteGuard */ public function createService(ServiceLocatorInterface $serviceLocator) @@ -57,10 +57,10 @@ public function createService(ServiceLocatorInterface $serviceLocator) /* @var \ZfcRbac\Options\ModuleOptions $moduleOptions */ $moduleOptions = $parentLocator->get('ZfcRbac\Options\ModuleOptions'); - /* @var \ZfcRbac\Service\RoleService $roleService */ - $roleService = $parentLocator->get('ZfcRbac\Service\AuthorizationService'); + /* @var \ZfcRbac\Service\AuthorizationService $authorizationService */ + $authorizationService = $parentLocator->get('ZfcRbac\Service\AuthorizationService'); - $routeGuard = new RoutePermissionsGuard($roleService, $this->options); + $routeGuard = new RoutePermissionsGuard($authorizationService, $this->options); $routeGuard->setProtectionPolicy($moduleOptions->getProtectionPolicy()); return $routeGuard; From d9fa14d8e2b7887108ee8ec7b951611cd31da463 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 14:25:27 +0200 Subject: [PATCH 11/20] guard plugin manager for RoutePermissionsGuard --- src/ZfcRbac/Guard/GuardPluginManager.php | 5 +++-- tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ZfcRbac/Guard/GuardPluginManager.php b/src/ZfcRbac/Guard/GuardPluginManager.php index 83112cb..c21498e 100644 --- a/src/ZfcRbac/Guard/GuardPluginManager.php +++ b/src/ZfcRbac/Guard/GuardPluginManager.php @@ -35,8 +35,9 @@ class GuardPluginManager extends AbstractPluginManager * @var array */ protected $factories = [ - 'ZfcRbac\Guard\ControllerGuard' => 'ZfcRbac\Factory\ControllerGuardFactory', - 'ZfcRbac\Guard\RouteGuard' => 'ZfcRbac\Factory\RouteGuardFactory' + 'ZfcRbac\Guard\ControllerGuard' => 'ZfcRbac\Factory\ControllerGuardFactory', + 'ZfcRbac\Guard\RouteGuard' => 'ZfcRbac\Factory\RouteGuardFactory', + 'ZfcRbac\Guard\RoutePermissionsGuard' => 'ZfcRbac\Factory\RoutePermissionsGuardFactory', ]; /** diff --git a/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php b/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php index ec14497..a58b099 100644 --- a/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php +++ b/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php @@ -36,6 +36,12 @@ public function guardProvider() 'admin/*' => 'foo' ] ], + [ + 'ZfcRbac\Guard\RoutePermissionsGuard', + [ + 'post/delete' => 'post.delete' + ] + ], [ 'ZfcRbac\Guard\ControllerGuard', [ @@ -60,6 +66,10 @@ public function testCanCreateDefaultGuards($type, $options) 'ZfcRbac\Service\RoleService', $this->getMock('ZfcRbac\Service\RoleService', [], [], '', false) ); + $serviceManager->setService( + 'ZfcRbac\Service\AuthorizationService', + $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false) + ); $pluginManager = new GuardPluginManager(); $pluginManager->setServiceLocator($serviceManager); From 07c16ba32fa9ef24ca6445cc77c287add1225b90 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 16:24:43 +0200 Subject: [PATCH 12/20] Permission config is an AND condition --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 6 +- .../Guard/RoutePermissionsGuardTest.php | 84 +++++++++++++++++-- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index 86ea1ea..8140aa4 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -102,11 +102,11 @@ public function isGranted(MvcEvent $event) } foreach ($allowedPermissions as $permission) { - if ($this->authorizationService->isGranted($permission)) { - return true; + if (!$this->authorizationService->isGranted($permission)) { + return false; } } - return false; + return true; } } diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index 2e9d260..60e1b00 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -99,8 +99,8 @@ public function rulesConversionProvider() */ public function testRulesConversions(array $rules, array $expected) { - $roleService = $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false); - $routeGuard = new RoutePermissionsGuard($roleService, $rules); + $roleService = $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false); + $routeGuard = new RoutePermissionsGuard($roleService, $rules); $reflProperty = new \ReflectionProperty($routeGuard, 'rules'); $reflProperty->setAccessible(true); $this->assertEquals($expected, $reflProperty->getValue($routeGuard)); @@ -195,6 +195,16 @@ public function routeDataProvider() 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route2', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], [ 'rules' => [ 'route1' => 'admin', @@ -205,6 +215,16 @@ public function routeDataProvider() 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], + [ + 'rules' => [ + 'route1' => 'admin', + 'route2' => 'admin' + ], + 'matchedRouteName' => 'route2', + 'identityPermissions' => [['admin', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], // Assert that it can grant/deny access with multiple rules based on the policy [ 'rules' => [ @@ -230,17 +250,54 @@ public function routeDataProvider() [ 'rules' => ['route' => 'admin'], 'matchedRouteName' => 'route', - 'identityPermissions' => [['admin', null, false], ['guest', null, true]], + 'identityPermissions' => [ + ['admin', null, false], + ['guest', null, true] + ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_ALLOW ], [ 'rules' => ['route' => 'admin'], 'matchedRouteName' => 'route', - 'identityPermissions' => [['admin', null, false], ['guest', null, true]], + 'identityPermissions' => [ + ['admin', null, false], + ['guest', null, true] + ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], + // Assert it can deny access if one of a permission does not have access + [ + 'rules' => ['route' => ['admin', 'guest']], + 'matchedRouteName' => 'route', + 'identityPermissions' => [ + ['admin', null, true], + ['guest', null, true] + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['route' => ['admin', 'guest']], + 'matchedRouteName' => 'route', + 'identityPermissions' => [ + ['admin', null, true], + ['guest', null, false] + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['route' => ['admin', 'guest']], + 'matchedRouteName' => 'route', + 'identityPermissions' => [ + ['admin', null, false], + ['guest', null, true] + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_ALLOW + ], // Assert wildcard in permission [ 'rules' => ['home' => '*'], @@ -256,6 +313,21 @@ public function routeDataProvider() 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], + // Assert wildcard wins all + [ + 'rules' => ['home' => ['*', 'admin']], + 'matchedRouteName' => 'home', + 'identityPermissions' => [['admin', null, false]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => ['home' => ['*', 'admin']], + 'matchedRouteName' => 'home', + 'identityPermissions' => [['admin', null, false]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], ]; } @@ -309,7 +381,7 @@ public function testProperlyFillEventOnAuthorization() $identityProvider->expects($this->any())->method('getIdentity')->will($this->returnValue($identity)); $roleProvider = new InMemoryRoleProvider(['member']); - $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); $routeGuard = new RouteGuard($roleService, [ 'adminRoute' => 'member' @@ -343,7 +415,7 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() $identityProvider->expects($this->any())->method('getIdentityRoles')->will($this->returnValue('member')); $roleProvider = new InMemoryRoleProvider(['member', 'guest']); - $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); $routeGuard = new RouteGuard($roleService, [ 'adminRoute' => 'guest' From 7a834b29ae4b025a06819a6c0467a1f73e89f104 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 16:34:18 +0200 Subject: [PATCH 13/20] documentation 04.guards --- docs/04. Guards.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/04. Guards.md b/docs/04. Guards.md index 6fa2cc9..d96e94e 100644 --- a/docs/04. Guards.md +++ b/docs/04. Guards.md @@ -23,7 +23,7 @@ RouteGuard and ControllerGuard are not aware of permissions but rather only thin instance, you may want to refuse access to each routes that begin by "admin/*" to all users that do not have the "admin" role. -If you want to protect a route for a set set of permissions, you must use RoutePermissionsGuard. For instance, +If you want to protect a route for a set of permissions, you must use RoutePermissionsGuard. For instance, you may want to grant access to a route "post/delete" only to roles having the "delete" permission. Note that in a RBAC system, a permission is linked to a role, not to a user. From 255d622600bf32ae0293711b5890e478217f657f Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 16:38:14 +0200 Subject: [PATCH 14/20] documentation 04.guards --- docs/04. Guards.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/04. Guards.md b/docs/04. Guards.md index d96e94e..9cfcef4 100644 --- a/docs/04. Guards.md +++ b/docs/04. Guards.md @@ -168,13 +168,16 @@ return [ 'guards' => [ 'ZfcRbac\Guard\RoutePermissionsGuard' => [ 'admin*' => ['admin'], - 'post/delete' => ['post.delete', 'admin'] + 'post/manage' => ['post.update', 'post.delete'] ] ] ] ]; ``` +All permissions in a rule must be matched (it is an AND condition). In the previous example, one must have +```post.update``` **AND** ```post.delete``` permissions to access the ```post/manage``` route. + > Permissions are linked to roles, not to users Those rules grant access to all admin routes to roles that have the "admin" permission, and grant access to the From 7ecd16409e4d074606ad82f2fce0cd8c485d0bf4 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 16:45:09 +0200 Subject: [PATCH 15/20] tests : change permissions names in data provider --- .../Guard/RoutePermissionsGuardTest.php | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index 60e1b00..a541316 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -111,189 +111,189 @@ public function routeDataProvider() return [ // Assert basic one-to-one mapping with both policies [ - 'rules' => ['adminRoute' => 'admin'], + 'rules' => ['adminRoute' => 'post.edit'], 'matchedRouteName' => 'adminRoute', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['adminRoute' => 'admin'], + 'rules' => ['adminRoute' => 'post.edit'], 'matchedRouteName' => 'adminRoute', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], // Assert that policy changes result for non-specified route guards [ - 'rules' => ['route' => 'admin'], + 'rules' => ['route' => 'post.edit'], 'matchedRouteName' => 'anotherRoute', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['route' => 'admin'], + 'rules' => ['route' => 'post.edit'], 'matchedRouteName' => 'anotherRoute', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], // Assert that composed route work for both policies [ - 'rules' => ['admin/dashboard' => 'admin'], + 'rules' => ['admin/dashboard' => 'post.edit'], 'matchedRouteName' => 'admin/dashboard', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['admin/dashboard' => 'admin'], + 'rules' => ['admin/dashboard' => 'post.edit'], 'matchedRouteName' => 'admin/dashboard', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], // Assert that wildcard route work for both policies [ - 'rules' => ['admin/*' => 'admin'], + 'rules' => ['admin/*' => 'post.edit'], 'matchedRouteName' => 'admin/dashboard', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['admin/*' => 'admin'], + 'rules' => ['admin/*' => 'post.edit'], 'matchedRouteName' => 'admin/dashboard', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], // Assert that wildcard route does match (or not depending on the policy) if rules is after matched route name [ - 'rules' => ['fooBar/*' => 'admin'], + 'rules' => ['fooBar/*' => 'post.edit'], 'matchedRouteName' => 'admin/fooBar/baz', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['fooBar/*' => 'admin'], + 'rules' => ['fooBar/*' => 'post.edit'], 'matchedRouteName' => 'admin/fooBar/baz', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], // Assert that it can grant access with multiple rules [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route1', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route2', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route1', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route2', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], // Assert that it can grant/deny access with multiple rules based on the policy [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route3', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ 'rules' => [ - 'route1' => 'admin', - 'route2' => 'admin' + 'route1' => 'post.edit', + 'route2' => 'post.edit' ], 'matchedRouteName' => 'route3', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], // Assert it can deny access if a permission does not have access [ - 'rules' => ['route' => 'admin'], + 'rules' => ['route' => 'post.edit'], 'matchedRouteName' => 'route', 'identityPermissions' => [ - ['admin', null, false], - ['guest', null, true] + ['post.edit', null, false], + ['post.read', null, true] ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['route' => 'admin'], + 'rules' => ['route' => 'post.edit'], 'matchedRouteName' => 'route', 'identityPermissions' => [ - ['admin', null, false], - ['guest', null, true] + ['post.edit', null, false], + ['post.read', null, true] ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], // Assert it can deny access if one of a permission does not have access [ - 'rules' => ['route' => ['admin', 'guest']], + 'rules' => ['route' => ['post.edit', 'post.read']], 'matchedRouteName' => 'route', 'identityPermissions' => [ - ['admin', null, true], - ['guest', null, true] + ['post.edit', null, true], + ['post.read', null, true] ], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['route' => ['admin', 'guest']], + 'rules' => ['route' => ['post.edit', 'post.read']], 'matchedRouteName' => 'route', 'identityPermissions' => [ - ['admin', null, true], - ['guest', null, false] + ['post.edit', null, true], + ['post.read', null, false] ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['route' => ['admin', 'guest']], + 'rules' => ['route' => ['post.edit', 'post.read']], 'matchedRouteName' => 'route', 'identityPermissions' => [ - ['admin', null, false], - ['guest', null, true] + ['post.edit', null, false], + ['post.read', null, true] ], 'isGranted' => false, 'policy' => GuardInterface::POLICY_ALLOW @@ -302,29 +302,29 @@ public function routeDataProvider() [ 'rules' => ['home' => '*'], 'matchedRouteName' => 'home', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ 'rules' => ['home' => '*'], 'matchedRouteName' => 'home', - 'identityPermissions' => [['admin', null, true]], + 'identityPermissions' => [['post.edit', null, true]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], // Assert wildcard wins all [ - 'rules' => ['home' => ['*', 'admin']], + 'rules' => ['home' => ['*', 'post.edit']], 'matchedRouteName' => 'home', - 'identityPermissions' => [['admin', null, false]], + 'identityPermissions' => [['post.edit', null, false]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_ALLOW ], [ - 'rules' => ['home' => ['*', 'admin']], + 'rules' => ['home' => ['*', 'post.edit']], 'matchedRouteName' => 'home', - 'identityPermissions' => [['admin', null, false]], + 'identityPermissions' => [['post.edit', null, false]], 'isGranted' => true, 'policy' => GuardInterface::POLICY_DENY ], @@ -334,7 +334,7 @@ public function routeDataProvider() /** * @dataProvider routeDataProvider */ - public function testRouteGranted( + public function testRoutePermissionGranted( array $rules, $matchedRouteName, array $identityPermissions, From 2bb5a83438e83cea5e21488527dc572420848e6c Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 16:56:12 +0200 Subject: [PATCH 16/20] tests --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 2 +- .../Guard/RoutePermissionsGuardTest.php | 39 ++++++++----------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index 8140aa4..f52edbe 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -32,7 +32,7 @@ class RoutePermissionsGuard extends AbstractGuard { use ProtectionPolicyTrait; - const EVENT_PRIORITY = -10; + const EVENT_PRIORITY = -6; /** * @var AuthorizationServiceInterface diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index a541316..972b1a4 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -23,9 +23,6 @@ use ZfcRbac\Guard\GuardInterface; use ZfcRbac\Guard\RouteGuard; use ZfcRbac\Guard\RoutePermissionsGuard; -use ZfcRbac\Role\InMemoryRoleProvider; -use ZfcRbac\Service\RoleService; -use Rbac\Traversal\Strategy\RecursiveRoleIteratorStrategy; /** * @covers \ZfcRbac\Guard\AbstractGuard @@ -47,9 +44,10 @@ public function testAttachToRightEvent() /** * We want to ensure an order for guards */ - public function testAssertRouteGuardPriorityIsHigherThanControllerGuardPriority() + public function testAssertRoutePermissionsGuardPriorityRank() { - $this->assertTrue(RouteGuard::EVENT_PRIORITY > ControllerGuard::EVENT_PRIORITY); + $this->assertLessThan(RouteGuard::EVENT_PRIORITY, RoutePermissionsGuard::EVENT_PRIORITY); + $this->assertGreaterThan(ControllerGuard::EVENT_PRIORITY, RoutePermissionsGuard::EVENT_PRIORITY); } public function rulesConversionProvider() @@ -374,17 +372,14 @@ public function testProperlyFillEventOnAuthorization() $event->setRouteMatch($routeMatch); $event->setApplication($application); - $identity = $this->getMock('ZfcRbac\Identity\IdentityInterface'); - $identity->expects($this->any())->method('getRoles')->will($this->returnValue(['member'])); - - $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); - $identityProvider->expects($this->any())->method('getIdentity')->will($this->returnValue($identity)); - - $roleProvider = new InMemoryRoleProvider(['member']); - $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $authorizationService = $this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false); + $authorizationService->expects($this->once()) + ->method('isGranted') + ->with('post.edit') + ->will($this->returnValue(true)); - $routeGuard = new RouteGuard($roleService, [ - 'adminRoute' => 'member' + $routeGuard = new RoutePermissionsGuard($authorizationService, [ + 'adminRoute' => 'post.edit' ]); $routeGuard->onResult($event); @@ -411,14 +406,14 @@ public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() $event->setRouteMatch($routeMatch); $event->setApplication($application); - $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); - $identityProvider->expects($this->any())->method('getIdentityRoles')->will($this->returnValue('member')); - - $roleProvider = new InMemoryRoleProvider(['member', 'guest']); - $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + $authorizationService = $this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false); + $authorizationService->expects($this->once()) + ->method('isGranted') + ->with('post.edit') + ->will($this->returnValue(false)); - $routeGuard = new RouteGuard($roleService, [ - 'adminRoute' => 'guest' + $routeGuard = new RoutePermissionsGuard($authorizationService, [ + 'adminRoute' => 'post.edit' ]); $routeGuard->onResult($event); From ea027cf085d560807d7c333ebffef23adf7d46f6 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sat, 14 Jun 2014 17:00:25 +0200 Subject: [PATCH 17/20] RoutePermissionsGuard priority --- src/ZfcRbac/Guard/RoutePermissionsGuard.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ZfcRbac/Guard/RoutePermissionsGuard.php b/src/ZfcRbac/Guard/RoutePermissionsGuard.php index f52edbe..577a9c8 100644 --- a/src/ZfcRbac/Guard/RoutePermissionsGuard.php +++ b/src/ZfcRbac/Guard/RoutePermissionsGuard.php @@ -32,7 +32,7 @@ class RoutePermissionsGuard extends AbstractGuard { use ProtectionPolicyTrait; - const EVENT_PRIORITY = -6; + const EVENT_PRIORITY = -8; /** * @var AuthorizationServiceInterface From 6984b01c00ac8492ede6f0ce08a8a06fa63220e7 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Sun, 15 Jun 2014 00:24:37 +0200 Subject: [PATCH 18/20] revert rule_type --- tests/ZfcRbacTest/Guard/RouteGuardTest.php | 6 ++++-- tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/ZfcRbacTest/Guard/RouteGuardTest.php b/tests/ZfcRbacTest/Guard/RouteGuardTest.php index f3cad09..2844250 100644 --- a/tests/ZfcRbacTest/Guard/RouteGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RouteGuardTest.php @@ -23,6 +23,7 @@ use ZfcRbac\Guard\ControllerGuard; use ZfcRbac\Guard\GuardInterface; use ZfcRbac\Guard\RouteGuard; +use ZfcRbac\Guard\RoutePermissionsGuard; use ZfcRbac\Role\InMemoryRoleProvider; use ZfcRbac\Service\RoleService; use Rbac\Traversal\Strategy\RecursiveRoleIteratorStrategy; @@ -48,9 +49,10 @@ public function testAttachToRightEvent() /** * We want to ensure an order for guards */ - public function testAssertRouteGuardPriorityIsHigherThanControllerGuardPriority() + public function testAssertRouteGuardPriority() { - $this->assertTrue(RouteGuard::EVENT_PRIORITY > ControllerGuard::EVENT_PRIORITY); + $this->assertGreaterThan(RoutePermissionsGuard::EVENT_PRIORITY, RouteGuard::EVENT_PRIORITY); + $this->assertGreaterThan(ControllerGuard::EVENT_PRIORITY, RouteGuard::EVENT_PRIORITY); } public function rulesConversionProvider() diff --git a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php index 972b1a4..0de7740 100644 --- a/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php +++ b/tests/ZfcRbacTest/Guard/RoutePermissionsGuardTest.php @@ -44,7 +44,7 @@ public function testAttachToRightEvent() /** * We want to ensure an order for guards */ - public function testAssertRoutePermissionsGuardPriorityRank() + public function testAssertRoutePermissionsGuardPriority() { $this->assertLessThan(RouteGuard::EVENT_PRIORITY, RoutePermissionsGuard::EVENT_PRIORITY); $this->assertGreaterThan(ControllerGuard::EVENT_PRIORITY, RoutePermissionsGuard::EVENT_PRIORITY); @@ -244,7 +244,7 @@ public function routeDataProvider() 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], - // Assert it can deny access if a permission does not have access + // Assert it can deny access if the only permission does not have access [ 'rules' => ['route' => 'post.edit'], 'matchedRouteName' => 'route', @@ -265,7 +265,7 @@ public function routeDataProvider() 'isGranted' => false, 'policy' => GuardInterface::POLICY_DENY ], - // Assert it can deny access if one of a permission does not have access + // Assert it can deny access if one of the permission does not have access [ 'rules' => ['route' => ['post.edit', 'post.read']], 'matchedRouteName' => 'route', From 79264653e15f93e929110e303999ccad698556d8 Mon Sep 17 00:00:00 2001 From: jmleroux Date: Tue, 17 Jun 2014 00:26:48 +0200 Subject: [PATCH 19/20] ControllerPermissionsGuard --- .../ControllerPermissionsGuardFactory.php | 67 +++ .../Guard/ControllerPermissionsGuard.php | 141 +++++ src/ZfcRbac/Guard/GuardPluginManager.php | 8 +- .../ControllerPermissionsGuardFactoryTest.php | 62 +++ .../Guard/ControllerPermissionsGuardTest.php | 513 ++++++++++++++++++ .../Guard/GuardPluginManagerTest.php | 12 +- 6 files changed, 799 insertions(+), 4 deletions(-) create mode 100644 src/ZfcRbac/Factory/ControllerPermissionsGuardFactory.php create mode 100644 src/ZfcRbac/Guard/ControllerPermissionsGuard.php create mode 100644 tests/ZfcRbacTest/Factory/ControllerPermissionsGuardFactoryTest.php create mode 100644 tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php diff --git a/src/ZfcRbac/Factory/ControllerPermissionsGuardFactory.php b/src/ZfcRbac/Factory/ControllerPermissionsGuardFactory.php new file mode 100644 index 0000000..0ca7acf --- /dev/null +++ b/src/ZfcRbac/Factory/ControllerPermissionsGuardFactory.php @@ -0,0 +1,67 @@ + + * @licence MIT + */ +class ControllerPermissionsGuardFactory implements FactoryInterface, MutableCreationOptionsInterface +{ + /** + * @var array + */ + protected $options = []; + + /** + * {@inheritDoc} + */ + public function setCreationOptions(array $options) + { + $this->options = $options; + } + + /** + * @param \Zend\ServiceManager\AbstractPluginManager|ServiceLocatorInterface $serviceLocator + * @return RouteGuard + */ + public function createService(ServiceLocatorInterface $serviceLocator) + { + $parentLocator = $serviceLocator->getServiceLocator(); + + /* @var \ZfcRbac\Options\ModuleOptions $moduleOptions */ + $moduleOptions = $parentLocator->get('ZfcRbac\Options\ModuleOptions'); + + /* @var \ZfcRbac\Service\AuthorizationService $authorizationService */ + $authorizationService = $parentLocator->get('ZfcRbac\Service\AuthorizationService'); + + $guard = new ControllerPermissionsGuard($authorizationService, $this->options); + $guard->setProtectionPolicy($moduleOptions->getProtectionPolicy()); + + return $guard; + } +} diff --git a/src/ZfcRbac/Guard/ControllerPermissionsGuard.php b/src/ZfcRbac/Guard/ControllerPermissionsGuard.php new file mode 100644 index 0000000..41b4415 --- /dev/null +++ b/src/ZfcRbac/Guard/ControllerPermissionsGuard.php @@ -0,0 +1,141 @@ + + * @author JM Leroux + * @licence MIT + */ +class ControllerPermissionsGuard extends AbstractGuard +{ + use ProtectionPolicyTrait; + + /** + * Event priority + */ + const EVENT_PRIORITY = -15; + + /** + * @var AuthorizationServiceInterface + */ + protected $authorizationService; + + /** + * Controller guard rules + * + * @var array + */ + protected $rules = []; + + /** + * Constructor + * + * @param AuthorizationServiceInterface $authorizationService + * @param array $rules + */ + public function __construct(AuthorizationServiceInterface $authorizationService, array $rules = []) + { + $this->authorizationService = $authorizationService; + $this->setRules($rules); + } + + /** + * Set the rules + * + * A controller rule is made the following way: + * + * [ + * 'controller' => 'ControllerName', + * 'actions' => []/string + * 'roles' => []/string + * ] + * + * @param array $rules + * @return void + */ + public function setRules(array $rules) + { + $this->rules = []; + + foreach ($rules as $rule) { + $controller = strtolower($rule['controller']); + $actions = isset($rule['actions']) ? (array) $rule['actions'] : []; + $permissions = (array) $rule['permissions']; + + if (empty($actions)) { + $this->rules[$controller][0] = $permissions; + continue; + } + + foreach ($actions as $action) { + $this->rules[$controller][strtolower($action)] = $permissions; + } + } + } + + /** + * {@inheritDoc} + */ + public function isGranted(MvcEvent $event) + { + $routeMatch = $event->getRouteMatch(); + $controller = strtolower($routeMatch->getParam('controller')); + $action = strtolower($routeMatch->getParam('action')); + + // If no rules apply, it is considered as granted or not based on the protection policy + if (!isset($this->rules[$controller])) { + return $this->protectionPolicy === self::POLICY_ALLOW; + } + + // Algorithm is as follow: we first check if there is an exact match (controller + action), if not + // we check if there are rules set globally for the whole controllers (see the index "0"), and finally + // if nothing is matched, we fallback to the protection policy logic + + if (isset($this->rules[$controller][$action])) { + $allowedPermissions = $this->rules[$controller][$action]; + } elseif (isset($this->rules[$controller][0])) { + $allowedPermissions = $this->rules[$controller][0]; + } else { + return $this->protectionPolicy === self::POLICY_ALLOW; + } + + // If no rules apply, it is considered as granted or not based on the protection policy + if (empty($allowedPermissions)) { + return $this->protectionPolicy === self::POLICY_ALLOW; + } + + if (in_array('*', $allowedPermissions)) { + return true; + } + + foreach ($allowedPermissions as $permission) { + if (!$this->authorizationService->isGranted($permission)) { + return false; + } + } + + return true; + } +} diff --git a/src/ZfcRbac/Guard/GuardPluginManager.php b/src/ZfcRbac/Guard/GuardPluginManager.php index c21498e..dbb8573 100644 --- a/src/ZfcRbac/Guard/GuardPluginManager.php +++ b/src/ZfcRbac/Guard/GuardPluginManager.php @@ -27,6 +27,7 @@ * @method GuardInterface get($name) * * @author Michaël Gallego + * @author JM Leroux * @licence MIT */ class GuardPluginManager extends AbstractPluginManager @@ -35,9 +36,10 @@ class GuardPluginManager extends AbstractPluginManager * @var array */ protected $factories = [ - 'ZfcRbac\Guard\ControllerGuard' => 'ZfcRbac\Factory\ControllerGuardFactory', - 'ZfcRbac\Guard\RouteGuard' => 'ZfcRbac\Factory\RouteGuardFactory', - 'ZfcRbac\Guard\RoutePermissionsGuard' => 'ZfcRbac\Factory\RoutePermissionsGuardFactory', + 'ZfcRbac\Guard\ControllerGuard' => 'ZfcRbac\Factory\ControllerGuardFactory', + 'ZfcRbac\Guard\ControllerPermissionsGuard' => 'ZfcRbac\Factory\ControllerPermissionsGuardFactory', + 'ZfcRbac\Guard\RouteGuard' => 'ZfcRbac\Factory\RouteGuardFactory', + 'ZfcRbac\Guard\RoutePermissionsGuard' => 'ZfcRbac\Factory\RoutePermissionsGuardFactory', ]; /** diff --git a/tests/ZfcRbacTest/Factory/ControllerPermissionsGuardFactoryTest.php b/tests/ZfcRbacTest/Factory/ControllerPermissionsGuardFactoryTest.php new file mode 100644 index 0000000..0467410 --- /dev/null +++ b/tests/ZfcRbacTest/Factory/ControllerPermissionsGuardFactoryTest.php @@ -0,0 +1,62 @@ + 'permission' + ]; + + $options = new ModuleOptions([ + 'identity_provider' => 'ZfcRbac\Identity\AuthenticationProvider', + 'guards' => [ + 'ZfcRbac\Guard\ControllerPermissionsGuard' => $creationOptions + ], + 'protection_policy' => GuardInterface::POLICY_ALLOW, + ]); + + $serviceManager = new ServiceManager(); + $serviceManager->setService('ZfcRbac\Options\ModuleOptions', $options); + $serviceManager->setService( + 'ZfcRbac\Service\AuthorizationService', + $this->getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false) + ); + + $pluginManager = new GuardPluginManager(); + $pluginManager->setServiceLocator($serviceManager); + + $factory = new ControllerPermissionsGuardFactory(); + $guard = $factory->createService($pluginManager); + + $this->assertInstanceOf('ZfcRbac\Guard\ControllerPermissionsGuard', $guard); + $this->assertEquals(GuardInterface::POLICY_ALLOW, $guard->getProtectionPolicy()); + } +} diff --git a/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php b/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php new file mode 100644 index 0000000..8b7f9cb --- /dev/null +++ b/tests/ZfcRbacTest/Guard/ControllerPermissionsGuardTest.php @@ -0,0 +1,513 @@ +getMock('ZfcRbac\Service\AuthorizationService', [], [], '', false); + + return $authorizationService; + } + + public function testAttachToRightEvent() + { + $guard = new ControllerPermissionsGuard($this->getMockAuthorizationService()); + + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + $eventManager->expects($this->once()) + ->method('attach') + ->with(ControllerGuard::EVENT_NAME); + + $guard->attach($eventManager); + } + + public function rulesConversionProvider() + { + return [ + // Without actions + [ + 'rules' => [ + [ + 'controller' => 'MyController', + 'permissions' => 'post.manage' + ], + [ + 'controller' => 'MyController2', + 'permissions' => ['post.update', 'post.delete'] + ], + new \ArrayIterator([ + 'controller' => 'MyController3', + 'permissions' => new \ArrayIterator(['post.manage']) + ]) + ], + 'expected' => [ + 'mycontroller' => [0 => ['post.manage']], + 'mycontroller2' => [0 => ['post.update', 'post.delete']], + 'mycontroller3' => [0 => ['post.manage']] + ] + ], + // With one action + [ + 'rules' => [ + [ + 'controller' => 'MyController', + 'actions' => 'DELETE', + 'permissions' => 'permission1' + ], + [ + 'controller' => 'MyController2', + 'actions' => ['delete'], + 'permissions' => 'permission2' + ], + new \ArrayIterator([ + 'controller' => 'MyController3', + 'actions' => new \ArrayIterator(['DELETE']), + 'permissions' => new \ArrayIterator(['permission3']) + ]) + ], + 'expected' => [ + 'mycontroller' => [ + 'delete' => ['permission1'] + ], + 'mycontroller2' => [ + 'delete' => ['permission2'] + ], + 'mycontroller3' => [ + 'delete' => ['permission3'] + ], + ] + ], + // With multiple actions + [ + 'rules' => [ + [ + 'controller' => 'MyController', + 'actions' => ['EDIT', 'delete'], + 'permissions' => 'permission1' + ], + new \ArrayIterator([ + 'controller' => 'MyController2', + 'actions' => new \ArrayIterator(['edit', 'DELETE']), + 'permissions' => new \ArrayIterator(['permission2']) + ]) + ], + 'expected' => [ + 'mycontroller' => [ + 'edit' => ['permission1'], + 'delete' => ['permission1'] + ], + 'mycontroller2' => [ + 'edit' => ['permission2'], + 'delete' => ['permission2'] + ] + ] + ], + // Test that that if a rule is set globally to the controller, it does not override any + // action specific rule that may have been specified before + [ + 'rules' => [ + [ + 'controller' => 'MyController', + 'actions' => ['edit'], + 'permissions' => 'permission1' + ], + [ + 'controller' => 'MyController', + 'permissions' => 'permission2' + ] + ], + 'expected' => [ + 'mycontroller' => [ + 'edit' => ['permission1'], + 0 => ['permission2'] + ] + ] + ] + ]; + } + + /** + * @dataProvider rulesConversionProvider + */ + public function testRulesConversions(array $rules, array $expected) + { + $controllerGuard = new ControllerPermissionsGuard($this->getMockAuthorizationService(), $rules); + + $reflProperty = new \ReflectionProperty($controllerGuard, 'rules'); + $reflProperty->setAccessible(true); + + $this->assertEquals($expected, $reflProperty->getValue($controllerGuard)); + } + + public function controllerDataProvider() + { + return [ + // Test simple guard with both policies + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'permissions' => 'post.edit' + ] + ], + 'controller' => 'BlogController', + 'action' => 'edit', + 'identityPermissions' => [['post.edit', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'permissions' => 'post.edit' + ] + ], + 'controller' => 'BlogController', + 'action' => 'edit', + 'identityPermissions' => [['post.edit', null, true]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Test with multiple rules + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'read', + 'permissions' => 'post.read' + ], + [ + 'controller' => 'BlogController', + 'actions' => 'edit', + 'permissions' => 'post.edit' + ] + ], + 'controller' => 'BlogController', + 'action' => 'edit', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'read', + 'permissions' => 'post.read' + ], + [ + 'controller' => 'BlogController', + 'actions' => 'edit', + 'permissions' => 'post.edit' + ] + ], + 'controller' => 'BlogController', + 'action' => 'edit', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + // Test with multiple permissions. All must be authorized. + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'admin', + 'permissions' => ['post.update', 'post.delete'], + ], + ], + 'controller' => 'BlogController', + 'action' => 'admin', + 'identityPermissions' => [ + ['post.update', null, true], + ['post.delete', null, true], + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'admin', + 'permissions' => ['post.update', 'post.delete'], + ], + ], + 'controller' => 'BlogController', + 'action' => 'admin', + 'identityPermissions' => [ + ['post.update', null, false], + ['post.delete', null, true], + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'admin', + 'permissions' => ['post.update', 'post.delete'], + ], + ], + 'controller' => 'BlogController', + 'action' => 'admin', + 'identityPermissions' => [ + ['post.update', null, true], + ['post.delete', null, false], + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert that policy can deny unspecified rules + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'permissions' => 'post.edit' + ], + ], + 'controller' => 'CommentController', + 'action' => 'edit', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'permissions' => 'post.edit' + ], + ], + 'controller' => 'CommentController', + 'action' => 'edit', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Test assert policy can deny other actions from controller when only one is specified + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'edit', + 'permissions' => 'post.edit' + ], + ], + 'controller' => 'BlogController', + 'action' => 'read', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + [ + 'controller' => 'BlogController', + 'actions' => 'edit', + 'permissions' => 'post.edit' + ], + ], + 'controller' => 'BlogController', + 'action' => 'read', + 'identityPermissions' => [ + ['post.edit', null, true] + ], + 'isGranted' => false, + 'policy' => GuardInterface::POLICY_DENY + ], + // Assert wildcard in permissions + [ + 'rules' => [ + [ + 'controller' => 'IndexController', + 'permissions' => '*' + ] + ], + 'controller' => 'IndexController', + 'action' => 'index', + 'identityPermissions' => [['post.edit', null, false]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_ALLOW + ], + [ + 'rules' => [ + [ + 'controller' => 'IndexController', + 'permissions' => '*' + ] + ], + 'controller' => 'IndexController', + 'action' => 'index', + 'identityPermissions' => [['post.edit', null, false]], + 'isGranted' => true, + 'policy' => GuardInterface::POLICY_DENY + ], + ]; + } + + /** + * @dataProvider controllerDataProvider + */ + public function testControllerGranted( + array $rules, + $controller, + $action, + $identityPermissions, + $isGranted, + $protectionPolicy + ) { + $routeMatch = new RouteMatch([]); + $routeMatch->setParam('controller', $controller); + $routeMatch->setParam('action', $action); + + $authorizationService = $this->getMockAuthorizationService(); + $authorizationService->expects($this->any()) + ->method('isGranted') + ->will($this->returnValueMap($identityPermissions)); + + $controllerGuard = new ControllerPermissionsGuard($authorizationService, $rules); + $controllerGuard->setProtectionPolicy($protectionPolicy); + + $event = new MvcEvent(); + $event->setRouteMatch($routeMatch); + + $this->assertEquals($isGranted, $controllerGuard->isGranted($event)); + } + + public function testProperlyFillEventOnAuthorization() + { + $event = new MvcEvent(); + $routeMatch = new RouteMatch([]); + + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + + $application->expects($this->never()) + ->method('getEventManager') + ->will($this->returnValue($eventManager)); + + $routeMatch->setParam('controller', 'MyController'); + $routeMatch->setParam('action', 'edit'); + $event->setRouteMatch($routeMatch); + $event->setApplication($application); + + $identity = $this->getMock('ZfcRbac\Identity\IdentityInterface'); + $identity->expects($this->any())->method('getRoles')->will($this->returnValue(['member'])); + + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); + $identityProvider->expects($this->any()) + ->method('getIdentity') + ->will($this->returnValue($identity)); + + $roleProvider = new InMemoryRoleProvider([ + 'member' + ]); + + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + + $routeGuard = new ControllerGuard($roleService, [ + [ + 'controller' => 'MyController', + 'actions' => 'edit', + 'roles' => 'member' + ] + ]); + + $routeGuard->onResult($event); + + $this->assertEmpty($event->getError()); + $this->assertNull($event->getParam('exception')); + } + + public function testProperlySetUnauthorizedAndTriggerEventOnUnauthorization() + { + $event = new MvcEvent(); + $routeMatch = new RouteMatch([]); + + $application = $this->getMock('Zend\Mvc\Application', [], [], '', false); + $eventManager = $this->getMock('Zend\EventManager\EventManagerInterface'); + + $application->expects($this->once()) + ->method('getEventManager') + ->will($this->returnValue($eventManager)); + + $eventManager->expects($this->once()) + ->method('trigger') + ->with(MvcEvent::EVENT_DISPATCH_ERROR); + + $routeMatch->setParam('controller', 'MyController'); + $routeMatch->setParam('action', 'delete'); + + $event->setRouteMatch($routeMatch); + $event->setApplication($application); + + $identityProvider = $this->getMock('ZfcRbac\Identity\IdentityProviderInterface'); + $identityProvider->expects($this->any()) + ->method('getIdentityRoles') + ->will($this->returnValue('member')); + + $roleProvider = new InMemoryRoleProvider([ + 'member' + ]); + + $roleService = new RoleService($identityProvider, $roleProvider, new RecursiveRoleIteratorStrategy()); + + $routeGuard = new ControllerGuard($roleService, [ + [ + 'controller' => 'MyController', + 'actions' => 'edit', + 'roles' => 'member' + ] + ]); + + $routeGuard->onResult($event); + + $this->assertTrue($event->propagationIsStopped()); + $this->assertEquals(ControllerGuard::GUARD_UNAUTHORIZED, $event->getError()); + $this->assertInstanceOf('ZfcRbac\Exception\UnauthorizedException', $event->getParam('exception')); + } +} diff --git a/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php b/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php index a58b099..ff43d26 100644 --- a/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php +++ b/tests/ZfcRbacTest/Guard/GuardPluginManagerTest.php @@ -51,7 +51,17 @@ public function guardProvider() 'roles' => 'baz' ] ] - ] + ], + [ + 'ZfcRbac\Guard\ControllerPermissionsGuard', + [ + [ + 'controller' => 'Foo', + 'actions' => 'bar', + 'permissions' => 'baz' + ] + ] + ], ]; } From f26b666b4578fb7b5e794216b5552a4ea0adb46b Mon Sep 17 00:00:00 2001 From: jmleroux Date: Thu, 19 Jun 2014 12:20:24 +0200 Subject: [PATCH 20/20] Documentation for ControllerPermissionsGuard --- composer.json | 4 ++ docs/04. Guards.md | 53 +++++++++++++++++-- .../Guard/ControllerPermissionsGuard.php | 8 +-- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 45d85a3..aff14ed 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,10 @@ "name": "Michaël Gallego", "email": "mic.gallego@gmail.com", "homepage": "http://www.michaelgallego.fr" + }, + { + "name": "Jean-Marie Leroux", + "email": "jmleroux.pro@gmail.com" } ], "require": { diff --git a/docs/04. Guards.md b/docs/04. Guards.md index 9cfcef4..f61d15b 100644 --- a/docs/04. Guards.md +++ b/docs/04. Guards.md @@ -61,7 +61,14 @@ deny policy is much more secure, but it needs much more configuration to work wi ## Built-in guards -ZfcRbac comes with three guards: RouteGuard, RoutePermissionsGuard and ControllerGuard. All guards must be added in the `guards` subkey: +ZfcRbac comes with four guards, in order of priority : + +* RouteGuard : protect a set of routes based on the identity roles +* RoutePermissionsGuard : protect a set of routes based on roles permissions +* ControllerGuard : protect a controllers and/or actions based on the identity roles +* ControllerPermissionsGuard : protect a controllers and/or actions based on roles permissions + +All guards must be added in the `guards` subkey: ```php return [ @@ -103,6 +110,8 @@ return [ ]; ``` +> Only one role in a rule need to be matched (it is an OR condition). + Those rules grant access to all admin routes to users that have the "admin" role, and grant access to the "login" route to users that have the "guest" role (eg.: most likely unauthenticated users). @@ -157,7 +166,7 @@ return [ ### RoutePermissionsGuard -> The RoutePermissionsGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -10. +> The RoutePermissionsGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -8. The RoutePermissionsGuard allows to protect a route or a hierarchy of route. You must provide an array of "key" => "value", where the key is a route pattern, and value an array of permissions names: @@ -175,8 +184,10 @@ return [ ]; ``` -All permissions in a rule must be matched (it is an AND condition). In the previous example, one must have -```post.update``` **AND** ```post.delete``` permissions to access the ```post/manage``` route. +> All permissions in a rule must be matched (it is an AND condition). + +In the previous example, one must have ```post.update``` **AND** ```post.delete``` permissions +to access the ```post/manage``` route. > Permissions are linked to roles, not to users @@ -220,6 +231,8 @@ This rule will be inaccessible. ### ControllerGuard +> The RoutePermissionsGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -10. + The ControllerGuard allows to protect a controller. You must provide an array of array: ```php @@ -237,6 +250,8 @@ return [ ]; ``` +> Only one role in a rule need to be matched (it is an OR condition). + Those rules grant access to each actions of the MyController controller to users that have either the "guest" or "member" roles. @@ -285,7 +300,35 @@ return [ Those rules grant access to each actions of the controller to users that have the "member" role, but restrict the "delete" action to "admin" only. -> The ControllerGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -10. +### ControllerPermissionsGuard + +> The RoutePermissionsGuard listens to the `MvcEvent::EVENT_ROUTE` event with a priority of -13. + +The ControllerPermissionsGuard allows to protect a controller using permissions. You must provide an array of array: + +```php +return [ + 'zfc_rbac' => [ + 'guards' => [ + 'ZfcRbac\Guard\ControllerPermissionsGuard' => [ + [ + 'controller' => 'MyController', + 'permissions' => ['post.update', 'post.delete'] + ] + ] + ] + ] +]; +``` + +> All permissions in a rule must be matched (it is an AND condition). + +In the previous example, the user must have ```post.update``` **AND** ```post.delete``` permissions +to access each actions of the MyController controller. + +As for all other guards, you can use a wildcard (*) character for permissions. + +The configuration rules are the same as for ControllerGuard. ### Security notice diff --git a/src/ZfcRbac/Guard/ControllerPermissionsGuard.php b/src/ZfcRbac/Guard/ControllerPermissionsGuard.php index 41b4415..2239129 100644 --- a/src/ZfcRbac/Guard/ControllerPermissionsGuard.php +++ b/src/ZfcRbac/Guard/ControllerPermissionsGuard.php @@ -53,7 +53,7 @@ class ControllerPermissionsGuard extends AbstractGuard * Constructor * * @param AuthorizationServiceInterface $authorizationService - * @param array $rules + * @param array $rules */ public function __construct(AuthorizationServiceInterface $authorizationService, array $rules = []) { @@ -80,9 +80,9 @@ public function setRules(array $rules) $this->rules = []; foreach ($rules as $rule) { - $controller = strtolower($rule['controller']); - $actions = isset($rule['actions']) ? (array) $rule['actions'] : []; - $permissions = (array) $rule['permissions']; + $controller = strtolower($rule['controller']); + $actions = isset($rule['actions']) ? (array)$rule['actions'] : []; + $permissions = (array)$rule['permissions']; if (empty($actions)) { $this->rules[$controller][0] = $permissions;