Merge pull request #4543 from dhensby/pulls/members-can-view-themselves

FIX Members can access their own profiles in CMS

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) {

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);

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]);

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() {

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

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"));

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