From 3abfaeecf30b845ac3a309f011bc460222a923b4 Mon Sep 17 00:00:00 2001 From: 0x20h Date: Sun, 5 Feb 2012 15:30:26 +0100 Subject: [PATCH] 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 b9ad118b9ca..0c77cb7aa1c 100644 --- a/lib/Cake/Controller/Component/Acl/PhpAcl.php +++ b/lib/Cake/Controller/Component/Acl/PhpAcl.php @@ -231,7 +231,7 @@ public function path($aco) { 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 @@ public function resolve($aco) { $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 @@ public function roles($aro) { } } - // 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 c5c06663dbb..870130af3ea 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 @@ public function setUp() { 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 @@ public function testAroAliases() { $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 @@ public function testAroAliases() { * @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 @@ public function testCheck() { $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 @@ public function testAcoResolve() { $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('/////')); } /**