Skip to content

Commit

Permalink
Don't let every role inherit from default role. Filter empty aco paths
Browse files Browse the repository at this point in the history
  • Loading branch information
0x20h committed Feb 5, 2012
1 parent 4532659 commit 3abfaee
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 23 deletions.
8 changes: 2 additions & 6 deletions lib/Cake/Controller/Component/Acl/PhpAcl.php
View file
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)));
}

/**
Expand Down Expand Up @@ -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);
}

Expand Down
32 changes: 15 additions & 17 deletions lib/Cake/Test/Case/Controller/Component/Acl/PhpAclTest.php
View file
Expand Up @@ -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'));
}


Expand Down Expand Up @@ -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
Expand All @@ -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'));
Expand All @@ -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'));
Expand Down Expand Up @@ -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('/////'));
}

/**
Expand Down

0 comments on commit 3abfaee

Please sign in to comment.