diff --git a/admin/code/CMSProfileController.php b/admin/code/CMSProfileController.php index 035bbc93b..f83abc29b 100644 --- a/admin/code/CMSProfileController.php +++ b/admin/code/CMSProfileController.php @@ -50,20 +50,20 @@ class CMSProfileController extends LeftAndMain { } public function canView($member = null) { - if(!$member && $member !== FALSE) $member = Member::currentUser(); - + if(!$member && $member !== false) $member = Member::currentUser(); + // cms menus only for logged-in members if(!$member) return false; - - // Only check for generic CMS permissions + + // Check they can access the CMS and that they are trying to edit themselves if( - !Permission::checkMember($member, "CMS_ACCESS_LeftAndMain") - && !Permission::checkMember($member, "CMS_ACCESS_CMSMain") + Permission::checkMember($member, "CMS_ACCESS") + && $member->ID === Member::currentUserID() ) { - return false; + return true; } - - return true; + + return false; } public function save($data, $form) { diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index d39b1f97e..af3d829f9 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -412,7 +412,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase { * Will collate all IDs form all fixtures if multiple fixtures are provided. * * @param string $className - * @return A map of fixture-identifier => object-id + * @return array A map of fixture-identifier => object-id */ protected function allFixtureIDs($className) { return $this->getFixtureFactory()->getIds($className); diff --git a/security/Permission.php b/security/Permission.php index 16498a529..a418b0a40 100644 --- a/security/Permission.php +++ b/security/Permission.php @@ -167,12 +167,21 @@ class Permission extends DataObject implements TemplateGlobalProvider { if(!is_array($code)) $code = array($code); if($arg == 'any') { + $adminImpliesAll = (bool)Config::inst()->get('Permission', 'admin_implies_all'); // Cache the permissions in memory if(!isset(self::$cache_permissions[$memberID])) { self::$cache_permissions[$memberID] = self::permissions_for_member($memberID); } foreach ($code as $permCode) { - if (substr($permCode, 0, 11) == 'CMS_ACCESS_') { + if ($permCode === 'CMS_ACCESS') { + foreach (self::$cache_permissions[$memberID] as $perm) { + //if they have admin rights OR they have an explicit access to the CMS then give permission + if (($adminImpliesAll && $perm == 'ADMIN') || substr($perm, 0, 11) === 'CMS_ACCESS_') { + return true; + } + } + } + elseif (substr($permCode, 0, 11) === 'CMS_ACCESS_') { //cms_access_leftandmain means access to all CMS areas $code[] = 'CMS_ACCESS_LeftAndMain'; break; @@ -180,7 +189,9 @@ class Permission extends DataObject implements TemplateGlobalProvider { } // if ADMIN has all privileges, then we need to push that code in - if(Config::inst()->get('Permission', 'admin_implies_all')) $code[] = "ADMIN"; + if($adminImpliesAll) { + $code[] = "ADMIN"; + } // Multiple $code values - return true if at least one matches, ie, intersection exists return (bool)array_intersect($code, self::$cache_permissions[$memberID]); diff --git a/tests/control/CMSProfileControllerTest.php b/tests/control/CMSProfileControllerTest.php index 9545a2c9a..2e62033c9 100644 --- a/tests/control/CMSProfileControllerTest.php +++ b/tests/control/CMSProfileControllerTest.php @@ -32,7 +32,7 @@ class CMSProfileControllerTest extends FunctionalTest { } public function testMemberEditsOwnProfile() { - $member = $this->objFromFixture('Member', 'user1'); + $member = $this->objFromFixture('Member', 'user3'); $this->session()->inst_set('loggedInAs', $member->ID); $response = $this->post('admin/myprofile/EditForm', array( @@ -46,9 +46,9 @@ class CMSProfileControllerTest extends FunctionalTest { 'Password[_ConfirmPassword]' => 'password', )); - $member = $this->objFromFixture('Member', 'user1'); + $member = $this->objFromFixture('Member', 'user3'); - $this->assertEquals($member->FirstName, 'JoeEdited', 'FirstName field was changed'); + $this->assertEquals('JoeEdited', $member->FirstName, 'FirstName field was changed'); } public function testExtendedPermissionsStopEditingOwnProfile() { diff --git a/tests/control/CMSProfileControllerTest.yml b/tests/control/CMSProfileControllerTest.yml index 4ab2d44f3..40795637a 100644 --- a/tests/control/CMSProfileControllerTest.yml +++ b/tests/control/CMSProfileControllerTest.yml @@ -1,27 +1,38 @@ Permission: - admin: - Code: ADMIN - cmsmain: - Code: CMS_ACCESS_LeftAndMain - leftandmain: - Code: CMS_ACCESS_CMSMain + admin: + Code: ADMIN + cmsmain: + Code: CMS_ACCESS_LeftAndMain + leftandmain: + Code: CMS_ACCESS_CMSMain + test: + Code: CMS_ACCESS_TestController + Group: - admins: - Title: Administrators - Permissions: =>Permission.admin - cmsusers: - Title: CMS Users - Permissions: =>Permission.cmsmain, =>Permission.leftandmain + admins: + Title: Administrators + Permissions: =>Permission.admin + cmsusers: + Title: CMS Users + Permissions: =>Permission.cmsmain, =>Permission.leftandmain + test: + Title: Test group + Permissions: =>Permission.test + Member: - admin: - FirstName: Admin - Email: admin@user.com - Groups: =>Group.admins - user1: - FirstName: Joe - Email: user1@user.com - Groups: =>Group.cmsusers - user2: - FirstName: Steve - Email: user2@user.com - Groups: =>Group.cmsusers + admin: + FirstName: Admin + Email: admin@user.com + Groups: =>Group.admins + user1: + FirstName: Joe + Email: user1@user.com + Groups: =>Group.cmsusers + user2: + FirstName: Steve + Email: user2@user.com + Groups: =>Group.cmsusers + user3: + FirstName: Files + Email: user3@example.com + Groups: =>Group.test diff --git a/tests/security/PermissionTest.php b/tests/security/PermissionTest.php index 7d63ccf72..fc659fdb4 100644 --- a/tests/security/PermissionTest.php +++ b/tests/security/PermissionTest.php @@ -14,7 +14,7 @@ class PermissionTest extends SapphireTest { } public function testGetCodesUngrouped() { - $codes = Permission::get_codes(null, false); + $codes = Permission::get_codes(false); $this->assertArrayHasKey('SITETREE_VIEW_ALL', $codes); } @@ -23,20 +23,25 @@ class PermissionTest extends SapphireTest { $this->assertTrue(Permission::checkMember($member, "SITETREE_VIEW_ALL")); } - public function testLeftAndMainAccessAll() { - //add user and group - $member = Member::create()->update(array( - 'FirstName' => 'Left', - 'Surname' => 'Main', - 'Email' => 'leftandmain@example.com', + public function testCMSAccess() { + $members = Member::get()->byIDs($this->allFixtureIDs('Member')); + foreach ($members as $member) { + $this->assertTrue(Permission::checkMember($member, 'CMS_ACCESS')); + } + + $member = new Member(); + $member->update(array( + 'FirstName' => 'No CMS', + 'Surname' => 'Access', + 'Email' => 'no-access@example.com', )); $member->write(); - $group = Group::create()->update(array( - 'Title' => 'LeftAndMain', - )); - $group->write(); - Permission::grant($group->ID, 'CMS_ACCESS_LeftAndMain'); - $group->DirectMembers()->add($member); + $this->assertFalse(Permission::checkMember($member, 'CMS_ACCESS')); + } + + public function testLeftAndMainAccessAll() { + //add user and group + $member = $this->objFromFixture('Member', 'leftandmain'); $this->assertTrue(Permission::checkMember($member, "CMS_ACCESS_MyAdmin")); $this->assertTrue(Permission::checkMember($member, "CMS_ACCESS_AssetAdmin")); diff --git a/tests/security/PermissionTest.yml b/tests/security/PermissionTest.yml index 58bc5fef3..f861e1d81 100644 --- a/tests/security/PermissionTest.yml +++ b/tests/security/PermissionTest.yml @@ -1,52 +1,63 @@ PermissionRole: - author: - Title: Author - access: - Title: Access Administrator + author: + Title: Author + access: + Title: Access Administrator PermissionRoleCode: - author1: - Role: =>PermissionRole.author - Code: CMS_ACCESS_MyAdmin - author2: - Role: =>PermissionRole.author - Code: CMS_ACCESS_AssetAdmin - access1: - Role: =>PermissionRole.access - Code: CMS_ACCESS_SecurityAdmin - access2: - Role: =>PermissionRole.access - Code: EDIT_PERMISSIONS + author1: + Role: =>PermissionRole.author + Code: CMS_ACCESS_MyAdmin + author2: + Role: =>PermissionRole.author + Code: CMS_ACCESS_AssetAdmin + access1: + Role: =>PermissionRole.access + Code: CMS_ACCESS_SecurityAdmin + access2: + Role: =>PermissionRole.access + Code: EDIT_PERMISSIONS + Member: - author: - FirstName: Test - Surname: Author - access: - FirstName: Test - Surname: Access Administrator - globalauthor: - FirstName: Test - Surname: Global Author + author: + FirstName: Test + Surname: Author + access: + FirstName: Test + Surname: Access Administrator + globalauthor: + FirstName: Test + Surname: Global Author + leftandmain: + FirstName: Left + Surname: AndMain + Email: leftandmain@example.com Group: - author: - Title: Authors - Members: =>Member.author - Roles: =>PermissionRole.author - access: - Title: Access Administrators + Authors - Members: =>Member.access - Roles: =>PermissionRole.access,=>PermissionRole.author - globalauthor: - Parent: =>Group.author - Title: Global Authors - Members: =>Member.globalauthor + author: + Title: Authors + Members: =>Member.author + Roles: =>PermissionRole.author + access: + Title: Access Administrators + Authors + Members: =>Member.access + Roles: =>PermissionRole.access,=>PermissionRole.author + globalauthor: + Parent: =>Group.author + Title: Global Authors + Members: =>Member.globalauthor + leftandmain: + Title: LeftAndMain + Members: =>Member.leftandmain Permission: - extra1: - Code: SITETREE_VIEW_ALL - Group: =>Group.author - globalauthor: - Code: SITETREE_EDIT_ALL - Group: =>Group.globalauthor + extra1: + Code: SITETREE_VIEW_ALL + Group: =>Group.author + globalauthor: + Code: SITETREE_EDIT_ALL + Group: =>Group.globalauthor + leftandmain: + Code: CMS_ACCESS_LeftAndMain + Group: =>Group.leftandmain