Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #603 from willrossi/trac7296
FIX: ensure permissions_for_member() accounts for denied permissions
  • Loading branch information
chillu committed Jul 5, 2012
2 parents 64357a4 + 9babb01 commit 4b9ccab
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
22 changes: 15 additions & 7 deletions security/Permission.php
Expand Up @@ -230,31 +230,39 @@ public static function checkMember($member, $code, $arg = "any", $strict = true)

/**
* Get all the 'any' permission codes available to the given member.
* @return array();
*
* @return array
*/
public static function permissions_for_member($memberID) {
$groupList = self::groupList($memberID);

if($groupList) {
$groupCSV = implode(", ", $groupList);

// Raw SQL for efficiency
return array_unique(DB::query("
$allowed = array_unique(DB::query("
SELECT \"Code\"
FROM \"Permission\"
WHERE \"Type\" = " . self::GRANT_PERMISSION . " AND \"GroupID\" IN ($groupCSV)
UNION
SELECT \"Code\"
FROM \"PermissionRoleCode\" PRC
INNER JOIN \"PermissionRole\" PR ON PRC.\"RoleID\" = PR.\"ID\"
INNER JOIN \"Group_Roles\" GR ON GR.\"PermissionRoleID\" = PR.\"ID\"
WHERE \"GroupID\" IN ($groupCSV)
")->column());

} else {
return array();
$denied = array_unique(DB::query("
SELECT \"Code\"
FROM \"Permission\"
WHERE \"Type\" = " . self::DENY_PERMISSION . " AND \"GroupID\" IN ($groupCSV)
")->column());

return array_diff($allowed, $denied);
}

return array();
}


Expand Down
24 changes: 23 additions & 1 deletion tests/security/PermissionTest.php
@@ -1,6 +1,11 @@
<?php

/**
* @package framework
* @subpackage tests
*/
class PermissionTest extends SapphireTest {

static $fixture_file = 'PermissionTest.yml';

function testGetCodesGrouped() {
Expand Down Expand Up @@ -33,6 +38,23 @@ function testPermissionAreInheritedFromMultipleRoles() {
$this->assertTrue(Permission::checkMember($member, "EDIT_PERMISSIONS"));
$this->assertFalse(Permission::checkMember($member, "SITETREE_VIEW_ALL"));
}

function testPermissionsForMember() {
$member = $this->objFromFixture('Member', 'access');
$permissions = Permission::permissions_for_member($member->ID);
$this->assertEquals(4, count($permissions));
$this->assertTrue(in_array('CMS_ACCESS_MyAdmin', $permissions));
$this->assertTrue(in_array('CMS_ACCESS_AssetAdmin', $permissions));
$this->assertTrue(in_array('CMS_ACCESS_SecurityAdmin', $permissions));
$this->assertTrue(in_array('EDIT_PERMISSIONS', $permissions));

$group = $this->objFromFixture("Group", "access");

Permission::deny($group->ID, "CMS_ACCESS_MyAdmin");
$permissions = Permission::permissions_for_member($member->ID);
$this->assertEquals(3, count($permissions));
$this->assertFalse(in_array('CMS_ACCESS_MyAdmin', $permissions));
}

function testRolesAndPermissionsFromParentGroupsAreInherited() {
$member = $this->objFromFixture('Member', 'globalauthor');
Expand Down Expand Up @@ -76,5 +98,5 @@ function testHiddenPermissions(){

Permission::remove_from_hidden_permissions('CMS_ACCESS_LeftAndMain');
$this->assertContains('CMS_ACCESS_LeftAndMain', $permissionCheckboxSet->Field());
}
}
}
4 changes: 2 additions & 2 deletions tests/security/PermissionTest.yml
Expand Up @@ -3,7 +3,7 @@ PermissionRole:
Title: Author
access:
Title: Access Administrator

PermissionRoleCode:
author1:
Role: =>PermissionRole.author
Expand All @@ -28,7 +28,7 @@ Member:
globalauthor:
FirstName: Test
Surname: Global Author

Group:
author:
Title: Authors
Expand Down

0 comments on commit 4b9ccab

Please sign in to comment.