From 153152642c5565109cf1eb790781b6de1d347c7d Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 21:43:48 +0100 Subject: [PATCH 01/11] PHP configuration file base Acl implementation --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 516 ++++++++++++++++++ lib/Cake/Error/exceptions.php | 12 +- .../Controller/Component/Acl/PhpAclTest.php | 315 +++++++++++ lib/Cake/Test/test_app/Config/acl.php | 72 +++ 4 files changed, 912 insertions(+), 3 deletions(-) create mode 100644 lib/Cake/Controller/Component/Acl/PhpAcl.php create mode 100644 lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php create mode 100644 lib/Cake/Test/test_app/Config/acl.php diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php new file mode 100644 index 000000000..aad12ae19 --- /dev/null +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -0,0 +1,516 @@ +options = array( + 'policy' => self::DENY, + 'config' => APP . 'Config' . DS . 'acl.php', + ); + } +/** + * Phptialize method + * + * @param AclBase $component + * @return void + */ + public function initialize($Component) { + if (!empty($Component->settings['ini_acl'])) { + $this->options = array_merge($this->options, $Component->settings['ini_acl']); + } + + App::uses('PhpReader', 'Configure'); + $Reader = new PhpReader(dirname($this->options['config']) . DS); + $config = $Reader->read(basename($this->options['config'])); + $this->build($config); + $Component->Aco = $this->Aco; + $Component->Aro = $this->Aro; + } + + + public function build($config) { + if ($config instanceOf ConfigReaderInterface) { + $config = $config->read(basename($this->options['config'])); + } + + if (empty($config['roles'])) { + throw new AclException(__d('cake_dev','"roles" section not found in configuration.')); + } + + if (empty($config['rules']['allow']) && empty($config['rules']['deny'])) { + throw new AclException(__d('cake_dev','Neither "allow" nor "deny" rules were provided in configuration.')); + } + + $rules['allow'] = !empty($config['rules']['allow']) ? $config['rules']['allow'] : array(); + $rules['deny'] = !empty($config['rules']['deny']) ? $config['rules']['deny'] : array(); + $roles = !empty($config['roles']) ? $config['roles'] : array(); + $map = !empty($config['map']) ? $config['map'] : array(); + $alias = !empty($config['alias']) ? $config['alias'] : array(); + + $this->Aro = new PhpAro($roles, $map, $alias); + $this->Aco = new PhpAco($rules); + } + +/** + * No op method, allow cannot be done with PhpAcl + * + * @param string $aro ARO The requesting object identifier. + * @param string $aco ACO The controlled object identifier. + * @param string $action Action (defaults to *) + * @return boolean Success + */ + public function allow($aro, $aco, $action = "*") { + return $this->Aco->access($this->Aro->resolve($aro), $aco, $action, 'allow'); + } + +/** + * deny ARO access to ACO + * + * @param string $aro ARO The requesting object identifier. + * @param string $aco ACO The controlled object identifier. + * @param string $action Action (defaults to *) + * @return boolean Success + */ + public function deny($aro, $aco, $action = "*") { + return $this->Aco->access($this->Aro->resolve($aro), $aco, $action, 'deny'); + } + +/** + * No op method, inherit cannot be done with PhpAcl + * + * @param string $aro ARO The requesting object identifier. + * @param string $aco ACO The controlled object identifier. + * @param string $action Action (defaults to *) + * @return boolean Success + */ + public function inherit($aro, $aco, $action = "*") { + } + +/** + * Main ACL check function. Checks to see if the ARO (access request object) has access to the + * ACO (access control object). + * + * @param string $aro ARO + * @param string $aco ACO + * @param string $aco_action Action + * @return boolean true if access is granted, false otherwise + */ + public function check($aro, $aco, $aco_action = null) { + $allow = $this->options['policy']; + $prioritizedAros = $this->Aro->roles($aro); + + if ($aco_action) { + $aco .= (strpos($aco, '.') ? '.' : '/') . $aco_action; + } + + $path = $this->Aco->path($aco); + + if (empty($path)) { + return $allow; + } + + foreach ($path as $depth => $node) { + foreach ($prioritizedAros as $aros) { + if (!empty($node['allow'])) { + $allow = $allow || count(array_intersect($node['allow'], $aros)) > 0; + } + + if (!empty($node['deny'])) { + $allow = $allow && count(array_intersect($node['deny'], $aros)) == 0; + } + } + } + + return $allow; + } +} + +/** + * Access Control Object + * + */ +class PhpAco { + +/** + * holds internal ACO representation + * + * @var array + */ + protected $tree = array(); + +/** + * map modifiers for ACO paths to their respective PCRE pattern + * + * @var array + */ + public static $modifiers = array( + '*' => '.*', + ); + + public function __construct(array $rules = array()) { + foreach (array('allow', 'deny') as $type) { + if(empty($rules[$type])) { + $rules[$type] = array(); + } + } + + $this->build($rules['allow'], $rules['deny']); + } + +/** + * return path to the requested ACO with allow and deny rules for each level + * + * @return array + */ + public function path($aco) { + $aco = $this->resolve($aco); + $path = array(); + $level = 0; + $root = $this->tree; + $stack = array(array($root, 0)); + + while (!empty($stack)) { + list($root, $level) = array_pop($stack); + + if (empty($path[$level])) { + $path[$level] = array(); + } + + foreach ($root as $node => $elements) { + if (strpos($node, '*') === false && $node != $aco[$level]) { + continue; + } + + $pattern = '#^'.str_replace(array_keys(self::$modifiers), array_values(self::$modifiers), $node).'$#'; + + if ($node == $aco[$level] || preg_match($pattern, $aco[$level])) { + // merge allow/denies with $path of current level + foreach (array('allow', 'deny') as $policy) { + if (!empty($elements[$policy])) { + if (empty($path[$level][$policy])) { + $path[$level][$policy] = array(); + } + + $path[$level][$policy] = array_merge($path[$level][$policy], $elements[$policy]); + } + } + + // traverse + if (!empty($elements['children']) && isset($aco[$level + 1])) { + array_push($stack, array($elements['children'], $level + 1)); + } + } + } + } + + return $path; + } + + +/** + * allow/deny ARO access to ARO + * + * @return void + */ + public function access($aro, $aco, $action, $type = 'deny') { + $aco = $this->resolve($aco); + $depth = count($aco); + $root = $this->tree; + $tree = &$root; + + foreach ($aco as $i => $node) { + if (!isset($tree[$node])) { + $tree[$node] = array( + 'children' => array(), + ); + } + + if ($i < $depth - 1) { + $tree = &$tree[$node]['children']; + } else { + if (empty($tree[$node][$type])) { + $tree[$node][$type] = array(); + } + + $tree[$node][$type] = array_merge(is_array($aro) ? $aro : array($aro), $tree[$node][$type]); + } + } + + $this->tree = &$root; + } + +/** + * resolve given ACO string to a path + * + * @param string $aco ACO string + * @return array path + */ + public function resolve($aco) { + if (is_array($aco)) { + return array_map('strtolower', $aco); + } + + return array_map('trim', explode('/', ltrim(strtolower($aco), '/'))); + } + +/** + * build a tree representation from the given allow/deny informations for ACO paths + * + * @param array $allow ACO allow rules + * @param array $deny ACO deny rules + * @return void + */ + public function build(array $allow, array $deny = array()) { + $stack = array(); + $this->tree = array(); + $tree = array(); + $root = &$tree; + + foreach ($allow as $dotPath => $commaSeparatedAros) { + $aros = array_map('trim', explode(',', $commaSeparatedAros)); + $this->access($aros, $dotPath, null, 'allow'); + } + + foreach ($deny as $dotPath => $commaSeparatedAros) { + $aros = array_map('trim', explode(',', $commaSeparatedAros)); + $this->access($aros, $dotPath, null, 'deny'); + } + } + + +} + +/** + * Access Request Object + * + */ +class PhpAro { + +/** + * role to resolve to when a provided ARO is not listed in + * the internal tree + * + * @var string + */ + const DEFAULT_ROLE = 'Role/default'; + +/** + * map external identifiers. E.g. if + * + * array('User' => array('username' => 'jeff', 'role' => 'editor')) + * + * is passed as an ARO to one of the methods of AclComponent, PhpAcl + * will check if it can be resolved to an User or a Role defined in the + * configuration file. + * + * @var array + * @see app/Config/acl.php + */ + public $map = array( + 'User' => 'User/username', + 'Role' => 'User/role', + ); + +/** + * aliases to map + * + * @var array + */ + public $aliases = array(); + +/** + * internal ARO representation + * + * @var array + */ + protected $tree = array(); + + public function __construct(array $aro = array(), array $map = array(), array $aliases = array()) { + !empty($map) && $this->map = $map; + $this->aliases = $aliases; + $this->build($aro); + } + + +/** + * From the perspective of the given ARO, walk down the tree and + * collect all inherited AROs levelwise such that AROs from different + * branches with equal distance to the requested ARO will be collected at the same + * index. The resulting array will contain a prioritized list of (list of) roles ordered from + * the most distant AROs to the requested one itself. + * + * @param mixed $aro An ARO identifier + * @return array prioritized AROs + */ + public function roles($aro) { + $aros = array(); + $aro = $this->resolve($aro); + $stack = array(array($aro, 0)); + + while (!empty($stack)) { + list($element, $depth) = array_pop($stack); + $aros[$depth][] = $element; + + foreach ($this->tree as $node => $children) { + if (in_array($element, $children)) { + array_push($stack, array($node, $depth + 1)); + } + } + } + + // everybody inherits from the default role + if ($aro != self::DEFAULT_ROLE) { + $aros[]= array(self::DEFAULT_ROLE); + } + return array_reverse($aros); + } + + +/** + * resolve an ARO identifier to an internal ARO string using + * the internal mapping information. + * + * @param mixed $aro ARO identifier (User.jeff, array('User' => ...), etc) + * @return string internal aro string (e.g. User/jeff, Role/default) + */ + public function resolve($aro) { + foreach ($this->map as $aroGroup => $map) { + list ($model, $field) = explode('/', $map); + $mapped = ''; + + if (is_array($aro)) { + if (isset($aro['model']) && isset($aro['foreign_key']) && $aro['model'] == $aroGroup) { + $mapped = $aroGroup . '/' . $aro['foreign_key']; + } elseif (isset($aro[$model][$field])) { + $mapped = $aroGroup . '/' . $aro[$model][$field]; + } elseif (isset($aro[$field])) { + $mapped = $aroGroup . '/' . $aro[$field]; + } + } elseif (is_string($aro)) { + $aro = ltrim($aro, '/'); + + if (strpos($aro, '/') === false) { + $mapped = $aroGroup . '/' . $aro; + } else { + list($aroModel, $aroValue) = explode('/', $aro); + + $aroModel = Inflector::camelize($aroModel); + + if ($aroModel == $model || $aroModel == $aroGroup) { + $mapped = $aroGroup . '/' . $aroValue; + } + } + } + + if (isset($this->tree[$mapped])) { + return $mapped; + } + + // is there a matching alias defined (e.g. Role/1 => Role/admin)? + if (!empty($this->aliases[$mapped])) { + return $this->aliases[$mapped]; + } + + } + + return self::DEFAULT_ROLE; + } + + +/** + * adds a new ARO to the tree + * + * @param array $aro one or more ARO records + * @return void + */ + public function addRole(array $aro) { + foreach ($aro as $role => $inheritedRoles) { + if (!isset($this->tree[$role])) { + $this->tree[$role] = array(); + } + + if (!empty($inheritedRoles)) { + if (is_string($inheritedRoles)) { + $inheritedRoles = array_map('trim', explode(',', $inheritedRoles)); + } + + foreach ($inheritedRoles as $dependency) { + // detect cycles + $roles = $this->roles($dependency); + + if (in_array($role, Set::flatten($roles))) { + $path = ''; + + foreach ($roles as $roleDependencies) { + $path .= implode('|', (array)$roleDependencies) . ' -> '; + } + + trigger_error(__d('cake_dev', 'cycle detected when inheriting %s from %s. Path: %s', $role, $dependency, $path.$role)); + continue; + } + + if (!isset($this->tree[$dependency])) { + $this->tree[$dependency] = array(); + } + + $this->tree[$dependency][] = $role; + } + } + } + } + +/** + * adds one or more aliases to the internal map. Overwrites existing entries. + * + * @param array $alias alias from => to (e.g. Role/13 -> Role/editor) + * @return void + */ + public function addAlias(array $alias) { + $this->aliases = array_merge($this->aliases, $alias); + } + +/** + * build an ARO tree structure for internal processing + * + * @param array $aros array of AROs as key and their inherited AROs as values + * @return void + */ + public function build(array $aros) { + $this->tree = array(); + $this->addRole($aros); + } +} diff --git a/lib/Cake/Error/exceptions.php b/lib/Cake/Error/exceptions.php index 938711adf..f21d55698 100644 --- a/lib/Cake/Error/exceptions.php +++ b/lib/Cake/Error/exceptions.php @@ -226,7 +226,6 @@ class MissingActionException extends CakeException { parent::__construct($message, $code); } } - /** * Private Action exception - used when a controller action * starts with a `_`. @@ -355,7 +354,7 @@ class MissingDatasourceException extends CakeException { * @package Cake.Error */ class MissingTableException extends CakeException { - protected $_messageTemplate = 'Table %s for model %s was not found in datasource %s.'; + protected $_messageTemplate = 'Database table %s for model %s was not found.'; } /** @@ -385,6 +384,13 @@ class MissingPluginException extends CakeException { protected $_messageTemplate = 'Plugin %s could not be found.'; } +/** + * Exception class for AclComponent and Interface implementations. + * + * @package Cake.Error + */ +class AclException extends CakeException { } + /** * Exception class for Cache. This exception will be thrown from Cache when it * encounters an error. @@ -427,7 +433,7 @@ class ConfigureException extends CakeException { } /** * Exception class for Socket. This exception will be thrown from CakeSocket, CakeEmail, HttpSocket - * SmtpTransport, MailTransport and HttpResponse when it encounters an error. + * SmtpTransport and HttpResponse when it encounters an error. * * @package Cake.Error */ diff --git a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php new file mode 100644 index 000000000..ec43d7687 --- /dev/null +++ b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php @@ -0,0 +1,315 @@ +PhpAcl = new PhpAcl(); + $this->Acl = new AclComponent($Collection, array( + 'ini_acl' => array( + 'config' => CAKE . 'Test' . DS . 'test_app' . DS . 'Config'. DS . 'acl.php', + ), + )); + $this->Acl->adapter($this->PhpAcl); + } + + + public function testRoleInheritance() { + $roles = $this->Acl->Aro->roles('User/peter'); + $this->assertEquals(array('Role/default'), $roles[0]); + $this->assertEquals(array('Role/accounting'), $roles[1]); + $this->assertEquals(array('User/peter'), $roles[2]); + + $roles = $this->Acl->Aro->roles('hardy'); + $this->assertEquals(array('Role/default'), $roles[0]); + $this->assertEquals(array('Role/database_manager', 'Role/data_acquirer'), $roles[1]); + $this->assertEquals(array('Role/accounting', 'Role/data_analyst'), $roles[2]); + $this->assertEquals(array('Role/accounting_manager', 'Role/reports'), $roles[3]); + $this->assertEquals(array('User/hardy'), $roles[4]); + } + + public function testAddRole() { + $this->assertEquals(array(array(PhpAro::DEFAULT_ROLE)), $this->Acl->Aro->roles('foobar')); + $this->Acl->Aro->addRole(array('User/foobar' => 'Role/accounting')); + $this->assertEquals(array(array('Role/default'), array('Role/accounting'), array('User/foobar')), $this->Acl->Aro->roles('foobar')); + } + + public function testAroResolve() { + $map = $this->Acl->Aro->map; + $this->Acl->Aro->map = array( + 'User' => 'FooModel/nickname', + 'Role' => 'FooModel/role', + ); + + $this->assertEquals('Role/default', $this->Acl->Aro->resolve('Foo.bar')); + $this->assertEquals('User/hardy', $this->Acl->Aro->resolve('FooModel/hardy')); + $this->assertEquals('User/hardy', $this->Acl->Aro->resolve('hardy')); + $this->assertEquals('User/hardy', $this->Acl->Aro->resolve(array('FooModel' => array('nickname' => 'hardy')))); + $this->assertEquals('Role/admin', $this->Acl->Aro->resolve(array('FooModel' => array('role' => 'admin')))); + $this->assertEquals('Role/admin', $this->Acl->Aro->resolve('Role/admin')); + + $this->assertEquals('Role/admin', $this->Acl->Aro->resolve('admin')); + $this->assertEquals('Role/admin', $this->Acl->Aro->resolve('FooModel/admin')); + $this->assertEquals('Role/accounting', $this->Acl->Aro->resolve('accounting')); + + $this->assertEquals(PhpAro::DEFAULT_ROLE, $this->Acl->Aro->resolve('bla')); + $this->assertEquals(PhpAro::DEFAULT_ROLE, $this->Acl->Aro->resolve(array('FooModel' => array('role' => 'hardy')))); + } + + + public function testAroAliases() { + $this->Acl->Aro->map = array( + 'User' => 'User/username', + 'Role' => 'User/group_id', + ); + + $this->Acl->Aro->aliases = array( + 'Role/1' => 'Role/admin', + 'Role/24' => 'Role/accounting', + ); + + $user = array( + 'User' => array( + 'username' => 'unknown_user', + 'group_id' => '1', + ), + ); + // group/1 + $this->assertEquals('Role/admin', $this->Acl->Aro->resolve($user)); + // group/24 + $this->assertEquals('Role/accounting', $this->Acl->Aro->resolve('Role/24')); + $this->assertEquals('Role/accounting', $this->Acl->Aro->resolve('24')); + + // check department + $user = array( + 'User' => array( + 'username' => 'foo', + 'group_id' => '25', + ), + ); + + $this->Acl->Aro->addRole(array('Role/IT' => null)); + $this->Acl->Aro->addAlias(array('Role/25' => 'Role/IT')); + $this->Acl->allow('Role/IT', '/rules/debugging/*'); + + $this->assertEquals(array(array('Role/default'), array('Role/IT', )), $this->Acl->Aro->roles($user)); + $this->assertTrue($this->Acl->check($user, '/rules/debugging/stats/pageload')); + $this->assertTrue($this->Acl->check($user, '/rules/debugging/sql/queries')); + // Role/default is allowed users dashboard, so is Role/IT + $this->assertTrue($this->Acl->check($user, '/controllers/users/dashboard')); + + $this->assertFalse($this->Acl->check($user, '/controllers/invoices/send')); + // wee add an more specific entry for user foo to also inherit from Role/accounting + $this->Acl->Aro->addRole(array('User/foo' => 'Role/IT, Role/accounting')); + $this->assertTrue($this->Acl->check($user, '/controllers/invoices/send')); + } + +/** + * testPhpCheck method + * + * @return void + */ + public function testCheck() { + $this->assertTrue($this->Acl->check('db_manager_2', '/controllers/users/Dashboard')); + $this->assertTrue($this->Acl->check('jan', '/controllers/users/Dashboard')); + $this->assertTrue($this->Acl->check('some_unknown_role', '/controllers/users/Dashboard')); + $this->assertTrue($this->Acl->check('Role/admin', 'foo/bar')); + $this->assertTrue($this->Acl->check('role/admin', '/foo/bar')); + $this->assertTrue($this->Acl->check('jan', 'foo/bar')); + $this->assertTrue($this->Acl->check('user/jan', 'foo/bar')); + $this->assertTrue($this->Acl->check('Role/admin', 'controllers/bar')); + $this->assertTrue($this->Acl->check(array('User' => array('username' =>'jan')), '/controlers/bar/bll')); + $this->assertTrue($this->Acl->check('Role/database_manager', 'controllers/db/create')); + $this->assertTrue($this->Acl->check('User/db_manager_2', 'controllers/db/create')); + + // inheritance: hardy -> reports -> data_analyst -> database_manager + $this->assertTrue($this->Acl->check('User/hardy', 'controllers/db/create')); + $this->assertFalse($this->Acl->check('User/jeff', 'controllers/db/create')); + + $this->assertTrue($this->Acl->check('Role/database_manager', 'controllers/db/select')); + $this->assertTrue($this->Acl->check('User/db_manager_2', 'controllers/db/select')); + $this->assertFalse($this->Acl->check('User/jeff', 'controllers/db/select')); + + $this->assertTrue($this->Acl->check('Role/database_manager', 'controllers/db/drop')); + $this->assertTrue($this->Acl->check('User/db_manager_1', 'controllers/db/drop')); + $this->assertFalse($this->Acl->check('db_manager_2', 'controllers/db/drop')); + + $this->assertTrue($this->Acl->check('db_manager_2', 'controllers/invoices/edit')); + $this->assertFalse($this->Acl->check('database_manager', 'controllers/invoices/edit')); + $this->assertFalse($this->Acl->check('db_manager_1', 'controllers/invoices/edit')); + + // Role/manager is allowed /controllers/*/*_manager + $this->assertTrue($this->Acl->check('stan', 'controllers/invoices/manager_edit')); + $this->assertTrue($this->Acl->check('Role/manager', 'controllers/baz/manager_foo')); + $this->assertFalse($this->Acl->check('User/stan', 'custom/foo/manager_edit')); + $this->assertFalse($this->Acl->check('stan', 'bar/baz/manager_foo')); + $this->assertFalse($this->Acl->check('Role/accounting', 'bar/baz/manager_foo')); + $this->assertFalse($this->Acl->check('accounting', 'controllers/baz/manager_foo')); + + $this->assertTrue($this->Acl->check('User/stan', 'controllers/articles/edit')); + $this->assertTrue($this->Acl->check('stan', 'controllers/articles/add')); + $this->assertTrue($this->Acl->check('stan', 'controllers/articles/publish')); + $this->assertFalse($this->Acl->check('User/stan', 'controllers/articles/delete')); + $this->assertFalse($this->Acl->check('accounting', 'controllers/articles/edit')); + $this->assertFalse($this->Acl->check('accounting', 'controllers/articles/add')); + $this->assertFalse($this->Acl->check('role/accounting', 'controllers/articles/publish')); + } + + + public function testCheckIsCaseInsensitive() { + $this->assertTrue($this->Acl->check('hardy', 'controllers/forms/new')); + $this->assertTrue($this->Acl->check('Role/data_acquirer', 'controllers/forms/new')); + $this->assertTrue($this->Acl->check('hardy', 'controllers/FORMS/NEW')); + $this->assertTrue($this->Acl->check('Role/data_acquirer', 'controllers/FORMS/NEW')); + } + + + public function testAllow() { + $this->assertFalse($this->Acl->check('jeff', 'foo/bar')); + + $this->Acl->allow('jeff', 'foo/bar'); + + $this->assertTrue($this->Acl->check('jeff', 'foo/bar')); + $this->assertFalse($this->Acl->check('peter', 'foo/bar')); + $this->assertFalse($this->Acl->check('hardy', 'foo/bar')); + + $this->Acl->allow('Role/accounting', 'foo/bar'); + + $this->assertTrue($this->Acl->check('peter', 'foo/bar')); + $this->assertTrue($this->Acl->check('hardy', 'foo/bar')); + + $this->assertFalse($this->Acl->check('Role/reports', 'foo/bar')); + } + + + public function testDeny() { + $this->assertTrue($this->Acl->check('stan', 'controllers/baz/manager_foo')); + + $this->Acl->deny('stan', 'controllers/baz/manager_foo'); + + $this->assertFalse($this->Acl->check('stan', 'controllers/baz/manager_foo')); + $this->assertTrue($this->Acl->check('Role/manager', 'controllers/baz/manager_foo')); + $this->assertTrue($this->Acl->check('stan', 'controllers/baz/manager_bar')); + $this->assertTrue($this->Acl->check('stan', 'controllers/baz/manager_foooooo')); + } + + + public function testDenyRuleIsStrongerThanAllowRule() { + $this->assertFalse($this->Acl->check('peter', 'baz/bam')); + $this->Acl->allow('peter', 'baz/bam'); + $this->assertTrue($this->Acl->check('peter', 'baz/bam')); + $this->Acl->deny('peter', 'baz/bam'); + $this->assertFalse($this->Acl->check('peter', 'baz/bam')); + + $this->assertTrue($this->Acl->check('stan', 'controllers/reports/foo')); + // stan is denied as he's sales and sales is denied /controllers/*/delete + $this->assertFalse($this->Acl->check('stan', 'controllers/reports/delete')); + $this->Acl->allow('stan', 'controllers/reports/delete'); + $this->assertFalse($this->Acl->check('Role/sales', 'controllers/reports/delete')); + $this->assertTrue($this->Acl->check('stan', 'controllers/reports/delete')); + $this->Acl->deny('stan', 'controllers/reports/delete'); + $this->assertFalse($this->Acl->check('stan', 'controllers/reports/delete')); + + // there is already an equally specific deny rule that will win + $this->Acl->allow('stan', 'controllers/reports/delete'); + $this->assertFalse($this->Acl->check('stan', 'controllers/reports/delete')); + } + + + public function testInvalidConfigWithAroMissing() { + $this->setExpectedException( + 'AclException', + '"roles" section not found in configuration' + ); + $config = array('aco' => array('allow' => array('foo' => ''))); + $this->PhpAcl->build($config); + } + + + public function testInvalidConfigWithAcosMissing() { + $this->setExpectedException( + 'AclException', + 'Neither "allow" nor "deny" rules were provided in configuration.' + ); + + $config = array( + 'roles' => array('Role/foo' => null), + ); + + $this->PhpAcl->build($config); + } + + public function testAcoResolve() { + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo/bar')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo/bar')); + $this->assertEquals(array('foo', 'bar', 'baz'), $this->Acl->Aco->resolve('foo/bar/baz')); + $this->assertEquals(array('foo', '*-bar', '?-baz'), $this->Acl->Aco->resolve('foo/*-bar/?-baz')); + } + + public function testAroDeclarationContainsCycles() { + $config = array( + 'roles' => array( + 'Role/a' => null, + 'Role/b' => 'User/b', + 'User/a' => 'Role/a, Role/b', + 'User/b' => 'User/a', + + ), + 'rules' => array( + 'allow' => array( + '*' => 'Role/a', + ), + ), + ); + + $this->expectError('PHPUnit_Framework_Error', 'cycle detected' /* ... */); + $this->PhpAcl->build($config); + } + + +/** + * test that with policy allow, only denies count + */ + public function testPolicy() { + // allow by default + $this->Acl->settings['ini_acl']['policy'] = PhpAcl::ALLOW; + $this->PhpAcl->initialize($this->Acl); + + $this->assertTrue($this->Acl->check('Role/sales', 'foo')); + $this->assertTrue($this->Acl->check('Role/sales', 'controllers/bla/create')); + $this->assertTrue($this->Acl->check('Role/default', 'foo')); + // undefined user, undefined aco + $this->assertTrue($this->Acl->check('foobart', 'foo/bar')); + + // deny rule: Role.sales -> controllers.*.delete + $this->assertFalse($this->Acl->check('Role/sales', 'controllers/bar/delete')); + $this->assertFalse($this->Acl->check('Role/sales', 'controllers/bar', 'delete')); + } +} diff --git a/lib/Cake/Test/test_app/Config/acl.php b/lib/Cake/Test/test_app/Config/acl.php new file mode 100644 index 000000000..eb8fa562a --- /dev/null +++ b/lib/Cake/Test/test_app/Config/acl.php @@ -0,0 +1,72 @@ + null, + 'Role/data_acquirer' => null, + 'Role/accounting' => null, + 'Role/database_manager' => null, + 'Role/sales' => null, + 'Role/data_analyst' => 'Role/data_acquirer, Role/database_manager', + 'Role/reports' => 'Role/data_analyst', + 'Role/manager' => array( + 'Role/accounting', + 'Role/sales', + ), + 'Role/accounting_manager' => 'Role/accounting', + // managers + 'User/hardy' => 'Role/accounting_manager, Role/reports', + 'User/stan' => 'Role/manager', + // accountants + 'User/peter' => 'Role/accounting', + 'User/jeff' => 'Role/accounting', + // admins + 'User/jan' => 'Role/admin', + // database + 'User/db_manager_1' => 'Role/database_manager', + 'User/db_manager_2' => 'Role/database_manager', +); + +//------------------------------------- +// ACOs +//------------------------------------- +$config['rules']['allow'] = array( + '/*' => 'Role/admin', + '/controllers/*/manager_*' => 'Role/manager', + '/controllers/reports/*' => 'Role/sales', + '/controllers/invoices/*' => 'Role/accounting', + '/controllers/invoices/edit'=> 'User/db_manager_2', + '/controllers/db/*' => 'Role/database_manager', + '/controllers/*/add' => 'User/stan', + '/controllers/*/edit' => 'User/stan', + '/controllers/*/publish' => 'User/stan', + '/controllers/users/dashboard' => 'Role/default', + // test for case insensitivity + 'controllers/Forms/NEW' => 'Role/data_acquirer', +); +$config['rules']['deny'] = array( + // accountants and sales should not delete anything + '/controllers/*/delete' => 'Role/sales, Role/accounting', + '/controllers/db/drop' => 'User/db_manager_2', +); From 6ddf6dad369ef54b86e96505fdd31249a95b734f Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 21:50:03 +0100 Subject: [PATCH 02/11] reverting my mistake --- lib/Cake/Error/exceptions.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Error/exceptions.php b/lib/Cake/Error/exceptions.php index f21d55698..a224633e5 100644 --- a/lib/Cake/Error/exceptions.php +++ b/lib/Cake/Error/exceptions.php @@ -226,6 +226,7 @@ class MissingActionException extends CakeException { parent::__construct($message, $code); } } + /** * Private Action exception - used when a controller action * starts with a `_`. @@ -354,7 +355,7 @@ class MissingDatasourceException extends CakeException { * @package Cake.Error */ class MissingTableException extends CakeException { - protected $_messageTemplate = 'Database table %s for model %s was not found.'; + protected $_messageTemplate = 'Table %s for model %s was not found in datasource %s.'; } /** @@ -433,7 +434,7 @@ class ConfigureException extends CakeException { } /** * Exception class for Socket. This exception will be thrown from CakeSocket, CakeEmail, HttpSocket - * SmtpTransport and HttpResponse when it encounters an error. + * SmtpTransport, MailTransport and HttpResponse when it encounters an error. * * @package Cake.Error */ From ef5eead03806e9bf49d4e60452fca42992d38e2d Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 21:55:47 +0100 Subject: [PATCH 03/11] use more appropriate array key when passing options to the adapter --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 4 ++-- lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index aad12ae19..d62d27c91 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -49,8 +49,8 @@ class PhpAcl extends Object implements AclInterface { * @return void */ public function initialize($Component) { - if (!empty($Component->settings['ini_acl'])) { - $this->options = array_merge($this->options, $Component->settings['ini_acl']); + if (!empty($Component->settings['adapter'])) { + $this->options = array_merge($this->options, $Component->settings['adapter']); } App::uses('PhpReader', 'Configure'); diff --git a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php index ec43d7687..b139ddf93 100644 --- a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php +++ b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php @@ -33,7 +33,7 @@ class PhpAclTest extends CakeTestCase { $Collection = new ComponentCollection(); $this->PhpAcl = new PhpAcl(); $this->Acl = new AclComponent($Collection, array( - 'ini_acl' => array( + 'adapter' => array( 'config' => CAKE . 'Test' . DS . 'test_app' . DS . 'Config'. DS . 'acl.php', ), )); @@ -299,7 +299,7 @@ class PhpAclTest extends CakeTestCase { */ public function testPolicy() { // allow by default - $this->Acl->settings['ini_acl']['policy'] = PhpAcl::ALLOW; + $this->Acl->settings['adapter']['policy'] = PhpAcl::ALLOW; $this->PhpAcl->initialize($this->Acl); $this->assertTrue($this->Acl->check('Role/sales', 'foo')); From 95a41af9db130e4369ba8adfbe1426f5c81651b9 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 22:02:25 +0100 Subject: [PATCH 04/11] allow multiple roles for a rule to be specified as string or array --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 14 ++++++++++---- lib/Cake/Test/test_app/Config/acl.php | 12 ++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index d62d27c91..b9d9bb76b 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -300,13 +300,19 @@ class PhpAco { $tree = array(); $root = &$tree; - foreach ($allow as $dotPath => $commaSeparatedAros) { - $aros = array_map('trim', explode(',', $commaSeparatedAros)); + foreach ($allow as $dotPath => $aros) { + if (is_string($aros)) { + $aros = array_map('trim', explode(',', $aros)); + } + $this->access($aros, $dotPath, null, 'allow'); } - foreach ($deny as $dotPath => $commaSeparatedAros) { - $aros = array_map('trim', explode(',', $commaSeparatedAros)); + foreach ($deny as $dotPath => $aros) { + if (is_string($aros)) { + $aros = array_map('trim', explode(',', $aros)); + } + $this->access($aros, $dotPath, null, 'deny'); } } diff --git a/lib/Cake/Test/test_app/Config/acl.php b/lib/Cake/Test/test_app/Config/acl.php index eb8fa562a..8320c717e 100644 --- a/lib/Cake/Test/test_app/Config/acl.php +++ b/lib/Cake/Test/test_app/Config/acl.php @@ -20,7 +20,7 @@ // ------------------------------------- -// AROs +// Roles // ------------------------------------- $config['roles'] = array( 'Role/admin' => null, @@ -30,6 +30,7 @@ $config['roles'] = array( 'Role/sales' => null, 'Role/data_analyst' => 'Role/data_acquirer, Role/database_manager', 'Role/reports' => 'Role/data_analyst', + // allow inherited roles to be defined as an array or comma separated list 'Role/manager' => array( 'Role/accounting', 'Role/sales', @@ -49,7 +50,7 @@ $config['roles'] = array( ); //------------------------------------- -// ACOs +// Rules //------------------------------------- $config['rules']['allow'] = array( '/*' => 'Role/admin', @@ -67,6 +68,9 @@ $config['rules']['allow'] = array( ); $config['rules']['deny'] = array( // accountants and sales should not delete anything - '/controllers/*/delete' => 'Role/sales, Role/accounting', - '/controllers/db/drop' => 'User/db_manager_2', + '/controllers/*/delete' => array( + 'Role/sales', + 'Role/accounting' + ), + '/controllers/db/drop' => 'User/db_manager_2', ); From b5671eb7e043dcaaa35ef6e425ac55fa9f9a9a10 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 23:28:55 +0100 Subject: [PATCH 05/11] added default acl.php in app/Config --- app/Config/acl.php | 134 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 app/Config/acl.php diff --git a/app/Config/acl.php b/app/Config/acl.php new file mode 100644 index 000000000..635989384 --- /dev/null +++ b/app/Config/acl.php @@ -0,0 +1,134 @@ +Auth->authorize = array('Actions' => array('actionPath' => 'controllers/'),...) + * + * Now, when a user (i.e. jeff) authenticates successfully and requests a controller action (i.e. /invoices/edit) + * that is not allowed by default (e.g. via $this->Auth->allow('edit') in the Invoices controller) then AuthComponent + * will ask the configured ACL interface if access is granted. Under the assumptions 1. and 2. this will be + * done via a call to Acl->check() with + * + * array('User' => array('username' => 'jeff', 'group_id' => 4, ...)) + * + * as ARO and + * + * '/controllers/invoices/delete' + * + * as ACO. + * + * If the configured map looks like + * + * $config['map'] = array( + * 'User' => 'User/username', + * 'Role' => 'User/group_id', + * ); + * + * then PhpAcl will lookup if we defined a role like User/jeff. If that role is not found, PhpAcl will try to + * find a definition for Role/4. If the definition isn't found then a default role (Role/default) will be used to + * check rules for the given ACO. The search can be expanded by defining aliases in the alias configuration. + * E.g. if you want to use a more readable name than Role/4 in your definitions you can define an alias like + * + * $config['alias'] = array( + * 'Role/4' => 'Role/editor', + * ); + * + * In the roles configuration you can define roles on the lhs and inherited roles on the rhs: + * + * $config['roles'] = array( + * 'Role/admin' => null, + * 'Role/accountant' => null, + * 'Role/editor' => null, + * 'Role/manager' => 'Role/editor, Role/accountant', + * 'User/jeff' => 'Role/manager', + * ); + * + * In this example manager inherits all rules from editor and accountant. Role/admin doesn't inherit from any role. + * Lets define some rules: + * + * $config['rules'] = array( + * 'allow' => array( + * '*' => 'Role/admin', + * 'controllers/users/(dashboard|profile)' => 'Role/default', + * 'controllers/invoices/*' => 'Role/accountant', + * 'controllers/articles/*' => 'Role/editor', + * 'controllers/users/*' => 'Role/manager', + * 'controllers/invoices/delete' => 'Role/manager', + * ), + * 'deny' => array( + * 'controllers/invoices/delete' => 'Role/accountant, User/jeff', + * 'controllers/articles/(delete|publish)' => 'Role/editor', + * ), + * ); + * + * Ok, so as jeff inherits from Role/manager he's matched every rule that references User/jeff, Role/manager, + * Role/editor, Role/accountant and Role/default. However, for jeff, rules for User/jeff are more specific than + * rules for Role/manager, rules for Role/manager are more specific than rules for Role/editor and so on. + * This is important when allow and deny rules match for a role. E.g. Role/accountant is allowed + * controllers/invoices/* but at the same time controllers/invoices/delete is denied. But there is a more + * specific rule defined for Role/manager which is allowed controllers/invoices/delete. However, the most specific + * rule denies access to the delete action explicitly for User/jeff, so he'll be denied access to the resource. + * + * If we would remove the role definition for User/jeff, then jeff would be granted access as he would be resolved + * to Role/manager and Role/manager has an allow rule. + */ + +/** + * The role map defines how to resolve the user record from your application + * to the roles you defined in the roles configuration. + */ +$config['map'] = array( + 'User' => 'User/username', + 'Role' => 'User/group_id', +); + +/** + * define aliases to map your model information to + * the roles defined in your role configuration. + */ +$config['alias'] = array( + 'Role/4' => 'Role/editor', +); + +/** + * role configuration + */ +$config['roles'] = array( + 'Role/admin' => null, +); + +/** + * rule configuration + */ +$config['rules'] = array( + 'allow' => array( + '*' => 'Role/admin', + ), + 'deny' => array(), +); From bfaea78504f321831a0583d631ae524b5884a7bd Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 23:29:23 +0100 Subject: [PATCH 06/11] allow more elaborate regex rules --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 30 +++++++++----------- lib/Cake/Test/test_app/Config/acl.php | 4 +-- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index b9d9bb76b..6a4f81a84 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -43,9 +43,9 @@ class PhpAcl extends Object implements AclInterface { ); } /** - * Phptialize method - * - * @param AclBase $component + * Initialize method + * + * @param AclComponent $Component Component instance * @return void */ public function initialize($Component) { @@ -61,12 +61,13 @@ class PhpAcl extends Object implements AclInterface { $Component->Aro = $this->Aro; } - - public function build($config) { - if ($config instanceOf ConfigReaderInterface) { - $config = $config->read(basename($this->options['config'])); - } - +/** + * build and setup internal ACL representation + * + * @param array $config configuration array, see docs + * @return void + */ + public function build(array $config) { if (empty($config['roles'])) { throw new AclException(__d('cake_dev','"roles" section not found in configuration.')); } @@ -110,7 +111,7 @@ class PhpAcl extends Object implements AclInterface { } /** - * No op method, inherit cannot be done with PhpAcl + * No op method * * @param string $aro ARO The requesting object identifier. * @param string $aco ACO The controlled object identifier. @@ -118,6 +119,7 @@ class PhpAcl extends Object implements AclInterface { * @return boolean Success */ public function inherit($aro, $aco, $action = "*") { + return false; } /** @@ -192,7 +194,7 @@ class PhpAco { } /** - * return path to the requested ACO with allow and deny rules for each level + * return path to the requested ACO with allow and deny rules attached on each level * * @return array */ @@ -202,7 +204,7 @@ class PhpAco { $level = 0; $root = $this->tree; $stack = array(array($root, 0)); - + while (!empty($stack)) { list($root, $level) = array_pop($stack); @@ -211,10 +213,6 @@ class PhpAco { } foreach ($root as $node => $elements) { - if (strpos($node, '*') === false && $node != $aco[$level]) { - continue; - } - $pattern = '#^'.str_replace(array_keys(self::$modifiers), array_values(self::$modifiers), $node).'$#'; if ($node == $aco[$level] || preg_match($pattern, $aco[$level])) { diff --git a/lib/Cake/Test/test_app/Config/acl.php b/lib/Cake/Test/test_app/Config/acl.php index 8320c717e..faf283eff 100644 --- a/lib/Cake/Test/test_app/Config/acl.php +++ b/lib/Cake/Test/test_app/Config/acl.php @@ -59,9 +59,7 @@ $config['rules']['allow'] = array( '/controllers/invoices/*' => 'Role/accounting', '/controllers/invoices/edit'=> 'User/db_manager_2', '/controllers/db/*' => 'Role/database_manager', - '/controllers/*/add' => 'User/stan', - '/controllers/*/edit' => 'User/stan', - '/controllers/*/publish' => 'User/stan', + '/controllers/*/(add|edit|publish)' => 'User/stan', '/controllers/users/dashboard' => 'Role/default', // test for case insensitivity 'controllers/Forms/NEW' => 'Role/data_acquirer', From 98383389760b71ddda47ac3485fe810edd024017 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 23:34:35 +0100 Subject: [PATCH 07/11] docblocks --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 17 +++++++++++++++++ .../Controller/Component/Acl/PhpAclTest.php | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index 6a4f81a84..aee798558 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -1,4 +1,21 @@ Date: Fri, 13 Jan 2012 23:36:29 +0100 Subject: [PATCH 08/11] typo --- app/Config/acl.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Config/acl.php b/app/Config/acl.php index 635989384..83cfbc0f0 100644 --- a/app/Config/acl.php +++ b/app/Config/acl.php @@ -30,7 +30,7 @@ * 2. You configured AuthComponent to authorize actions via * $this->Auth->authorize = array('Actions' => array('actionPath' => 'controllers/'),...) * - * Now, when a user (i.e. jeff) authenticates successfully and requests a controller action (i.e. /invoices/edit) + * Now, when a user (i.e. jeff) authenticates successfully and requests a controller action (i.e. /invoices/delete) * that is not allowed by default (e.g. via $this->Auth->allow('edit') in the Invoices controller) then AuthComponent * will ask the configured ACL interface if access is granted. Under the assumptions 1. and 2. this will be * done via a call to Acl->check() with From c6624faf76ca09e4558a1b999ef74f59d2788271 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Fri, 13 Jan 2012 23:51:18 +0100 Subject: [PATCH 09/11] more clear test setup --- lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php index 74e268525..9727c4d0f 100644 --- a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php +++ b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php @@ -37,7 +37,6 @@ class PhpAclTest extends CakeTestCase { 'config' => CAKE . 'Test' . DS . 'test_app' . DS . 'Config'. DS . 'acl.php', ), )); - $this->Acl->adapter($this->PhpAcl); } @@ -300,7 +299,7 @@ class PhpAclTest extends CakeTestCase { public function testPolicy() { // allow by default $this->Acl->settings['adapter']['policy'] = PhpAcl::ALLOW; - $this->PhpAcl->initialize($this->Acl); + $this->Acl->adapter($this->PhpAcl); $this->assertTrue($this->Acl->check('Role/sales', 'foo')); $this->assertTrue($this->Acl->check('Role/sales', 'controllers/bla/create')); From 4532659fed6d27ee7ad9920a30198b7319aa25f8 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Wed, 18 Jan 2012 20:59:44 +0100 Subject: [PATCH 10/11] code cleanup, added some tests --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 27 +++++++----- .../Controller/Component/Acl/PhpAclTest.php | 41 +++++++++++++++++-- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index aee798558..b9ad118b9 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -148,12 +148,12 @@ class PhpAcl extends Object implements AclInterface { * @param string $aco_action Action * @return boolean true if access is granted, false otherwise */ - public function check($aro, $aco, $aco_action = null) { + public function check($aro, $aco, $aco_action = "*") { $allow = $this->options['policy']; $prioritizedAros = $this->Aro->roles($aro); - if ($aco_action) { - $aco .= (strpos($aco, '.') ? '.' : '/') . $aco_action; + if ($aco_action && $aco_action != "*") { + $aco .= '/' . $aco_action; } $path = $this->Aco->path($aco); @@ -202,7 +202,7 @@ class PhpAco { public function __construct(array $rules = array()) { foreach (array('allow', 'deny') as $type) { - if(empty($rules[$type])) { + if (empty($rules[$type])) { $rules[$type] = array(); } } @@ -230,7 +230,7 @@ class PhpAco { } foreach ($root as $node => $elements) { - $pattern = '#^'.str_replace(array_keys(self::$modifiers), array_values(self::$modifiers), $node).'$#'; + $pattern = '/^'.str_replace(array_keys(self::$modifiers), array_values(self::$modifiers), $node).'$/'; if ($node == $aco[$level] || preg_match($pattern, $aco[$level])) { // merge allow/denies with $path of current level @@ -299,7 +299,11 @@ class PhpAco { return array_map('strtolower', $aco); } - return array_map('trim', explode('/', ltrim(strtolower($aco), '/'))); + // strip multiple occurences of '/' + $aco = preg_replace('#/+#', '/', $aco); + // make case insensitive + $aco = ltrim(strtolower($aco), '/'); + return array_map('trim', explode('/', $aco)); } /** @@ -381,7 +385,10 @@ class PhpAro { protected $tree = array(); public function __construct(array $aro = array(), array $map = array(), array $aliases = array()) { - !empty($map) && $this->map = $map; + if (!empty($map)) { + $this->map = $map; + } + $this->aliases = $aliases; $this->build($aro); } @@ -415,7 +422,7 @@ class PhpAro { // everybody inherits from the default role if ($aro != self::DEFAULT_ROLE) { - $aros[]= array(self::DEFAULT_ROLE); + $aros[] = array(self::DEFAULT_ROLE); } return array_reverse($aros); } @@ -430,7 +437,7 @@ class PhpAro { */ public function resolve($aro) { foreach ($this->map as $aroGroup => $map) { - list ($model, $field) = explode('/', $map); + list ($model, $field) = explode('/', $map, 2); $mapped = ''; if (is_array($aro)) { @@ -447,7 +454,7 @@ class PhpAro { if (strpos($aro, '/') === false) { $mapped = $aroGroup . '/' . $aro; } else { - list($aroModel, $aroValue) = explode('/', $aro); + list($aroModel, $aroValue) = explode('/', $aro, 2); $aroModel = Inflector::camelize($aroModel); diff --git a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php index 9727c4d0f..c5c06663d 100644 --- a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php +++ b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php @@ -54,12 +54,14 @@ class PhpAclTest extends CakeTestCase { $this->assertEquals(array('User/hardy'), $roles[4]); } + public function testAddRole() { $this->assertEquals(array(array(PhpAro::DEFAULT_ROLE)), $this->Acl->Aro->roles('foobar')); $this->Acl->Aro->addRole(array('User/foobar' => 'Role/accounting')); $this->assertEquals(array(array('Role/default'), array('Role/accounting'), array('User/foobar')), $this->Acl->Aro->roles('foobar')); } + public function testAroResolve() { $map = $this->Acl->Aro->map; $this->Acl->Aro->map = array( @@ -82,7 +84,9 @@ class PhpAclTest extends CakeTestCase { $this->assertEquals(PhpAro::DEFAULT_ROLE, $this->Acl->Aro->resolve(array('FooModel' => array('role' => 'hardy')))); } - +/** + * test correct resolution of defined aliases + */ public function testAroAliases() { $this->Acl->Aro->map = array( 'User' => 'User/username', @@ -130,8 +134,9 @@ class PhpAclTest extends CakeTestCase { $this->assertTrue($this->Acl->check($user, '/controllers/invoices/send')); } + /** - * testPhpCheck method + * test check method * * @return void */ @@ -182,6 +187,9 @@ class PhpAclTest extends CakeTestCase { } +/** + * lhs of defined rules are case insensitive + */ public function testCheckIsCaseInsensitive() { $this->assertTrue($this->Acl->check('hardy', 'controllers/forms/new')); $this->assertTrue($this->Acl->check('Role/data_acquirer', 'controllers/forms/new')); @@ -190,6 +198,9 @@ class PhpAclTest extends CakeTestCase { } +/** + * allow should work in-memory + */ public function testAllow() { $this->assertFalse($this->Acl->check('jeff', 'foo/bar')); @@ -208,6 +219,9 @@ class PhpAclTest extends CakeTestCase { } +/** + * deny should work in-memory + */ public function testDeny() { $this->assertTrue($this->Acl->check('stan', 'controllers/baz/manager_foo')); @@ -220,6 +234,9 @@ class PhpAclTest extends CakeTestCase { } +/** + * test that a deny rule wins over an equally specific allow rule + */ public function testDenyRuleIsStrongerThanAllowRule() { $this->assertFalse($this->Acl->check('peter', 'baz/bam')); $this->Acl->allow('peter', 'baz/bam'); @@ -241,7 +258,10 @@ class PhpAclTest extends CakeTestCase { $this->assertFalse($this->Acl->check('stan', 'controllers/reports/delete')); } - + +/** + * test that an invalid configuration throws exception + */ public function testInvalidConfigWithAroMissing() { $this->setExpectedException( 'AclException', @@ -265,13 +285,28 @@ class PhpAclTest extends CakeTestCase { $this->PhpAcl->build($config); } +/** + * test resolving of ACOs + */ public function testAcoResolve() { $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo/bar')); $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo/bar')); $this->assertEquals(array('foo', 'bar', 'baz'), $this->Acl->Aco->resolve('foo/bar/baz')); $this->assertEquals(array('foo', '*-bar', '?-baz'), $this->Acl->Aco->resolve('foo/*-bar/?-baz')); + + $this->assertEquals(array('foo', 'bar', '[a-f0-9]{24}', '*_bla', 'bla'), $this->Acl->Aco->resolve('foo/bar/[a-f0-9]{24}/*_bla/bla')); + + // multiple slashes will be squashed to a single, then exploded + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo//bar')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('//foo//bar')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('/foo//bar')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('/foo // bar')); + $this->assertEquals(array(''), $this->Acl->Aco->resolve('/////')); } +/** + * test that declaring cyclic dependencies should give an error when building the tree + */ public function testAroDeclarationContainsCycles() { $config = array( 'roles' => array( From 3abfaeecf30b845ac3a309f011bc460222a923b4 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Sun, 5 Feb 2012 15:30:26 +0100 Subject: [PATCH 11/11] Don't let every role inherit from default role. Filter empty aco paths --- lib/Cake/Controller/Component/Acl/PhpAcl.php | 8 ++--- .../Controller/Component/Acl/PhpAclTest.php | 32 +++++++++---------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/lib/Cake/Controller/Component/Acl/PhpAcl.php b/lib/Cake/Controller/Component/Acl/PhpAcl.php index b9ad118b9..0c77cb7aa 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -231,7 +231,7 @@ class PhpAco { foreach ($root as $node => $elements) { $pattern = '/^'.str_replace(array_keys(self::$modifiers), array_values(self::$modifiers), $node).'$/'; - + if ($node == $aco[$level] || preg_match($pattern, $aco[$level])) { // merge allow/denies with $path of current level foreach (array('allow', 'deny') as $policy) { @@ -303,7 +303,7 @@ class PhpAco { $aco = preg_replace('#/+#', '/', $aco); // make case insensitive $aco = ltrim(strtolower($aco), '/'); - return array_map('trim', explode('/', $aco)); + return array_filter(array_map('trim', explode('/', $aco))); } /** @@ -420,10 +420,6 @@ class PhpAro { } } - // everybody inherits from the default role - if ($aro != self::DEFAULT_ROLE) { - $aros[] = array(self::DEFAULT_ROLE); - } return array_reverse($aros); } diff --git a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php index c5c06663d..870130af3 100644 --- a/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php +++ b/lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php @@ -42,23 +42,21 @@ class PhpAclTest extends CakeTestCase { public function testRoleInheritance() { $roles = $this->Acl->Aro->roles('User/peter'); - $this->assertEquals(array('Role/default'), $roles[0]); - $this->assertEquals(array('Role/accounting'), $roles[1]); - $this->assertEquals(array('User/peter'), $roles[2]); + $this->assertEquals(array('Role/accounting'), $roles[0]); + $this->assertEquals(array('User/peter'), $roles[1]); $roles = $this->Acl->Aro->roles('hardy'); - $this->assertEquals(array('Role/default'), $roles[0]); - $this->assertEquals(array('Role/database_manager', 'Role/data_acquirer'), $roles[1]); - $this->assertEquals(array('Role/accounting', 'Role/data_analyst'), $roles[2]); - $this->assertEquals(array('Role/accounting_manager', 'Role/reports'), $roles[3]); - $this->assertEquals(array('User/hardy'), $roles[4]); + $this->assertEquals(array('Role/database_manager', 'Role/data_acquirer'), $roles[0]); + $this->assertEquals(array('Role/accounting', 'Role/data_analyst'), $roles[1]); + $this->assertEquals(array('Role/accounting_manager', 'Role/reports'), $roles[2]); + $this->assertEquals(array('User/hardy'), $roles[3]); } public function testAddRole() { $this->assertEquals(array(array(PhpAro::DEFAULT_ROLE)), $this->Acl->Aro->roles('foobar')); $this->Acl->Aro->addRole(array('User/foobar' => 'Role/accounting')); - $this->assertEquals(array(array('Role/default'), array('Role/accounting'), array('User/foobar')), $this->Acl->Aro->roles('foobar')); + $this->assertEquals(array(array('Role/accounting'), array('User/foobar')), $this->Acl->Aro->roles('foobar')); } @@ -122,11 +120,11 @@ class PhpAclTest extends CakeTestCase { $this->Acl->Aro->addAlias(array('Role/25' => 'Role/IT')); $this->Acl->allow('Role/IT', '/rules/debugging/*'); - $this->assertEquals(array(array('Role/default'), array('Role/IT', )), $this->Acl->Aro->roles($user)); + $this->assertEquals(array(array('Role/IT', )), $this->Acl->Aro->roles($user)); $this->assertTrue($this->Acl->check($user, '/rules/debugging/stats/pageload')); $this->assertTrue($this->Acl->check($user, '/rules/debugging/sql/queries')); - // Role/default is allowed users dashboard, so is Role/IT - $this->assertTrue($this->Acl->check($user, '/controllers/users/dashboard')); + // Role/default is allowed users dashboard, but not Role/IT + $this->assertFalse($this->Acl->check($user, '/controllers/users/dashboard')); $this->assertFalse($this->Acl->check($user, '/controllers/invoices/send')); // wee add an more specific entry for user foo to also inherit from Role/accounting @@ -141,7 +139,6 @@ class PhpAclTest extends CakeTestCase { * @return void */ public function testCheck() { - $this->assertTrue($this->Acl->check('db_manager_2', '/controllers/users/Dashboard')); $this->assertTrue($this->Acl->check('jan', '/controllers/users/Dashboard')); $this->assertTrue($this->Acl->check('some_unknown_role', '/controllers/users/Dashboard')); $this->assertTrue($this->Acl->check('Role/admin', 'foo/bar')); @@ -152,6 +149,7 @@ class PhpAclTest extends CakeTestCase { $this->assertTrue($this->Acl->check(array('User' => array('username' =>'jan')), '/controlers/bar/bll')); $this->assertTrue($this->Acl->check('Role/database_manager', 'controllers/db/create')); $this->assertTrue($this->Acl->check('User/db_manager_2', 'controllers/db/create')); + $this->assertFalse($this->Acl->check('db_manager_2', '/controllers/users/Dashboard')); // inheritance: hardy -> reports -> data_analyst -> database_manager $this->assertTrue($this->Acl->check('User/hardy', 'controllers/db/create')); @@ -296,12 +294,12 @@ class PhpAclTest extends CakeTestCase { $this->assertEquals(array('foo', 'bar', '[a-f0-9]{24}', '*_bla', 'bla'), $this->Acl->Aco->resolve('foo/bar/[a-f0-9]{24}/*_bla/bla')); - // multiple slashes will be squashed to a single, then exploded + // multiple slashes will be squashed to a single, trimmed and then exploded $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('foo//bar')); - $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('//foo//bar')); - $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('/foo//bar')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('//foo///bar/')); + $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('/foo//bar//')); $this->assertEquals(array('foo', 'bar'), $this->Acl->Aco->resolve('/foo // bar')); - $this->assertEquals(array(''), $this->Acl->Aco->resolve('/////')); + $this->assertEquals(array(), $this->Acl->Aco->resolve('/////')); } /**