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(