Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

add permissions capacity to RouteGuard and ControllerGuard #243

Closed
wants to merge 5 commits into from
Closed

add permissions capacity to RouteGuard and ControllerGuard #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d580bff
add permissions capacity to RouteGuard
jmleroux Jun 16, 2014
2819ed0
add permissions capacity to ControllerGuard
jmleroux Jun 16, 2014
2b40a1b
more tests
jmleroux Jun 16, 2014
68d4dce
$context parameter
jmleroux Jun 16, 2014
74e84a7
don't use roles and permissions for the same route
jmleroux Jun 16, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/ZfcRbac/Assertion/AssertionInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* @author Michaël Gallego <[email protected]>
* @author Aeneas Rekkas
* @author Daniel Gimenes <[email protected]>
* @author JM Leroux <[email protected]>
* @licence MIT
*/
interface AssertionInterface
Expand All @@ -36,8 +37,8 @@ interface AssertionInterface
* @TODO: for v3, update the interface to typehint to AuthorizationServiceInterface instead
*
* @param AuthorizationService $authorizationService
* @param mixed $context
* @param mixed|null $context
* @return bool
*/
public function assert(AuthorizationService $authorizationService);
public function assert(AuthorizationService $authorizationService, $context = null);
}
6 changes: 5 additions & 1 deletion src/ZfcRbac/Factory/ControllerGuardFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public function setCreationOptions(array $options)
*/
public function createService(ServiceLocatorInterface $serviceLocator)
{
/** @var \Zend\ServiceManager\ServiceManager $parentLocator */
$parentLocator = $serviceLocator->getServiceLocator();

/* @var \ZfcRbac\Options\ModuleOptions $moduleOptions */
Expand All @@ -58,7 +59,10 @@ public function createService(ServiceLocatorInterface $serviceLocator)
/* @var \ZfcRbac\Service\RoleService $roleService */
$roleService = $parentLocator->get('ZfcRbac\Service\RoleService');

$controllerGuard = new ControllerGuard($roleService, $this->options);
/* @var \ZfcRbac\Service\AuthorizationService $authorizationService */
$authorizationService = $parentLocator->get('ZfcRbac\Service\AuthorizationService');

$controllerGuard = new ControllerGuard($roleService, $authorizationService, $this->options);
$controllerGuard->setProtectionPolicy($moduleOptions->getProtectionPolicy());

return $controllerGuard;
Expand Down
8 changes: 7 additions & 1 deletion src/ZfcRbac/Factory/RouteGuardFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@
use Zend\ServiceManager\FactoryInterface;
use Zend\ServiceManager\MutableCreationOptionsInterface;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\ServiceManager\ServiceManager;
use ZfcRbac\Guard\RouteGuard;

/**
* Create a route guard
*
* @author Michaël Gallego <[email protected]>
* @author JM Leroux <[email protected]>
* @licence MIT
*/
class RouteGuardFactory implements FactoryInterface, MutableCreationOptionsInterface
Expand All @@ -50,6 +52,7 @@ public function setCreationOptions(array $options)
*/
public function createService(ServiceLocatorInterface $serviceLocator)
{
/** @var ServiceManager $parentLocator */
$parentLocator = $serviceLocator->getServiceLocator();

/* @var \ZfcRbac\Options\ModuleOptions $moduleOptions */
Expand All @@ -58,7 +61,10 @@ public function createService(ServiceLocatorInterface $serviceLocator)
/* @var \ZfcRbac\Service\RoleService $roleService */
$roleService = $parentLocator->get('ZfcRbac\Service\RoleService');

$routeGuard = new RouteGuard($roleService, $this->options);
/* @var \ZfcRbac\Service\AuthorizationService $authorizationService */
$authorizationService = $parentLocator->get('ZfcRbac\Service\AuthorizationService');

$routeGuard = new RouteGuard($roleService, $authorizationService, $this->options);
$routeGuard->setProtectionPolicy($moduleOptions->getProtectionPolicy());

return $routeGuard;
Expand Down
61 changes: 48 additions & 13 deletions src/ZfcRbac/Guard/ControllerGuard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
namespace ZfcRbac\Guard;

use Zend\Mvc\MvcEvent;
use ZfcRbac\Service\AuthorizationServiceInterface;
use ZfcRbac\Service\RoleService;

/**
Expand All @@ -41,6 +42,11 @@ class ControllerGuard extends AbstractGuard
*/
protected $roleService;

/**
* @var AuthorizationServiceInterface
*/
protected $authorizationService;

/**
* Controller guard rules
*
Expand All @@ -52,11 +58,16 @@ class ControllerGuard extends AbstractGuard
* Constructor
*
* @param RoleService $roleService
* @param array $rules
* @param AuthorizationServiceInterface $authorizationService
* @param array $rules
*/
public function __construct(RoleService $roleService, array $rules = [])
{
$this->roleService = $roleService;
public function __construct(
RoleService $roleService,
AuthorizationServiceInterface $authorizationService,
array $rules = []
) {
$this->roleService = $roleService;
$this->authorizationService = $authorizationService;
$this->setRules($rules);
}

Expand All @@ -79,17 +90,20 @@ public function setRules(array $rules)
$this->rules = [];

foreach ($rules as $rule) {
$controller = strtolower($rule['controller']);
$actions = isset($rule['actions']) ? (array) $rule['actions'] : [];
$roles = (array) $rule['roles'];
$controller = strtolower($rule['controller']);
$actions = isset($rule['actions']) ? (array)$rule['actions'] : [];
$roles = isset($rule['roles']) ? (array)$rule['roles'] : [];
$permissions = isset($rule['permissions']) ? (array)$rule['permissions'] : [];

if (empty($actions)) {
$this->rules[$controller][0] = $roles;
$this->rules[$controller][0]['roles'] = $roles;
$this->rules[$controller][0]['permissions'] = $permissions;
continue;
}

foreach ($actions as $action) {
$this->rules[$controller][strtolower($action)] = $roles;
$this->rules[$controller][strtolower($action)]['roles'] = $roles;
$this->rules[$controller][strtolower($action)]['permissions'] = $permissions;
}
}
}
Expand All @@ -113,17 +127,38 @@ public function isGranted(MvcEvent $event)
// if nothing is matched, we fallback to the protection policy logic

if (isset($this->rules[$controller][$action])) {
$allowedRoles = $this->rules[$controller][$action];
$allowedRoles = $this->rules[$controller][$action]['roles'];
$allowedPermissions = $this->rules[$controller][$action]['permissions'];
} elseif (isset($this->rules[$controller][0])) {
$allowedRoles = $this->rules[$controller][0];
$allowedRoles = $this->rules[$controller][0]['roles'];
$allowedPermissions = $this->rules[$controller][0]['permissions'];
} else {
return $this->protectionPolicy === self::POLICY_ALLOW;
}

if (in_array('*', $allowedRoles)) {
if (in_array('*', (array)$allowedRoles)) {
return true;
}

if (!empty($allowedRoles)) {
return $this->roleService->matchIdentityRoles($allowedRoles);
}

// 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('*', (array)$allowedPermissions)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me, that 'roles' => 'foo' override (AND operation) 'permission' => 'bar' you would actually need a setting for that. Take this config for example:

'My\Controller' => [
    'roles' => ['admin'],
    'permission' => ['mayAccess']
]

I think it is preferable to do something like this:

'My\Controller' => [
    'roles' => ['admin'],
    'permission' => ['mayAccess'],
    'operator' => GuardInterface::OR_OPERATOR
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like discussed in #238, it will be another PR.
I keep it simple first.

return true;
}

return $this->roleService->matchIdentityRoles($allowedRoles);
foreach ($allowedPermissions as $permission) {
if (!$this->authorizationService->isGranted($permission)) {
return false;
}
}

return true;
}
}
133 changes: 114 additions & 19 deletions src/ZfcRbac/Guard/RouteGuard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@

use Zend\Mvc\MvcEvent;
use ZfcRbac\Exception;
use ZfcRbac\Service\AuthorizationServiceInterface;
use ZfcRbac\Service\RoleService;

/**
* A route guard can protect a route or a hierarchy of routes (using simple wildcard pattern)
*
* @author Michaël Gallego <[email protected]>
* @author JM Leroux <[email protected]>
* @licence MIT
*/
class RouteGuard extends AbstractGuard
Expand All @@ -37,6 +39,11 @@ class RouteGuard extends AbstractGuard
*/
protected $roleService;

/**
* @var AuthorizationServiceInterface
*/
protected $authorizationService;

/**
* Route guard rules
*
Expand All @@ -50,11 +57,16 @@ class RouteGuard extends AbstractGuard
* Constructor
*
* @param RoleService $roleService
* @param array $rules
* @param AuthorizationServiceInterface $authorizationService
* @param array $rules
*/
public function __construct(RoleService $roleService, array $rules = [])
{
public function __construct(
RoleService $roleService,
AuthorizationServiceInterface $authorizationService,
array $rules = []
) {
$this->roleService = $roleService;
$this->authorizationService = $authorizationService;
$this->setRules($rules);
}

Expand All @@ -69,16 +81,51 @@ public function setRules(array $rules)
$this->rules = [];

foreach ($rules as $key => $value) {
if (is_int($key)) {
$routeRegex = $value;
$roles = [];
$result = $this->parseOneRule($key, $value);

$routePattern = $result['routePattern'];
$this->rules[$routePattern]['roles'] = $result['roles'];
$this->rules[$routePattern]['permissions'] = $result['permissions'];
}
}

/**
* @param string $key
* @param string|array $value
* @throws \InvalidArgumentException
* @return string[]
*/
private function parseOneRule($key, $value)
{
if (is_int($key)) {
$routePattern = $value;
$roles = [];
$permissions = [];
} else {
$routePattern = $key;
$roles = [];
$permissions = [];
if (isset($value['roles']) && isset($value['permissions'])) {
throw new \InvalidArgumentException("You cannot use roles AND permissions for a route.");
}
if (!isset($value['roles']) && !isset($value['permissions'])) {
$roles = (array)$value;
$permissions = [];
} else {
$routeRegex = $key;
$roles = (array) $value;
if (isset($value['roles'])) {
$roles = (array)$value['roles'];
}
if (isset($value['permissions'])) {
$permissions = (array)$value['permissions'];
}
}

$this->rules[$routeRegex] = $roles;
}

return [
'routePattern' => $routePattern,
'roles' => $roles,
'permissions' => $permissions,
];
}

/**
Expand All @@ -87,24 +134,72 @@ public function setRules(array $rules)
public function isGranted(MvcEvent $event)
{
$matchedRouteName = $event->getRouteMatch()->getMatchedRouteName();
$allowedRoles = null;

foreach (array_keys($this->rules) as $routeRule) {
if (fnmatch($routeRule, $matchedRouteName, FNM_CASEFOLD)) {
$allowedRoles = $this->rules[$routeRule];
break;
}
// check roles first
$allowedRoles = $this->getAllowedRoles($matchedRouteName);

if (in_array('*', (array)$allowedRoles)) {
return true;
}

if (!empty($allowedRoles)) {
return $this->roleService->matchIdentityRoles($allowedRoles);
}

// if no roles in rule, check permissions
$allowedPermissions = $this->getAllowedPermissions($matchedRouteName);

// If no rules apply, it is considered as granted or not based on the protection policy
if (null === $allowedRoles) {
if (null === $allowedPermissions) {
return $this->protectionPolicy === self::POLICY_ALLOW;
}

if (in_array('*', $allowedRoles)) {
if (in_array('*', (array)$allowedPermissions)) {
return true;
}

return $this->roleService->matchIdentityRoles($allowedRoles);
foreach ($allowedPermissions as $permission) {
if (!$this->authorizationService->isGranted($permission)) {
return false;
}
}

return true;
}

/**
* @param string $matchedRouteName
* @return array
*/
private function getAllowedRoles($matchedRouteName)
{
$allowedRoles = null;

foreach (array_keys($this->rules) as $routeRule) {
if (fnmatch($routeRule, $matchedRouteName, FNM_CASEFOLD)) {
$allowedRoles = $this->rules[$routeRule]['roles'];
break;
}
}

return $allowedRoles;
}

/**
* @param string $matchedRouteName
* @return array
*/
private function getAllowedPermissions($matchedRouteName)
{
$allowedPermissions = null;

foreach (array_keys($this->rules) as $routeRule) {
if (fnmatch($routeRule, $matchedRouteName, FNM_CASEFOLD)) {
$allowedPermissions = $this->rules[$routeRule]['permissions'];
break;
}
}

return $allowedPermissions;
}
}
4 changes: 4 additions & 0 deletions tests/ZfcRbacTest/Factory/ControllerGuardFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ public function testFactory()
'ZfcRbac\Service\RoleService',
$this->getMock('ZfcRbac\Service\RoleService', [], [], '', false)
);
$serviceManager->setService(
'ZfcRbac\Service\AuthorizationService',
$this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false)
);

$pluginManager = new GuardPluginManager();
$pluginManager->setServiceLocator($serviceManager);
Expand Down
4 changes: 4 additions & 0 deletions tests/ZfcRbacTest/Factory/GuardsFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public function testFactory()
'ZfcRbac\Service\RoleService',
$this->getMock('ZfcRbac\Service\RoleService', [], [], '', false)
);
$serviceManager->setService(
'ZfcRbac\Service\AuthorizationService',
$this->getMock('ZfcRbac\Service\AuthorizationServiceInterface', [], [], '', false)
);

$pluginManager->setServiceLocator($serviceManager);

Expand Down
Loading