ENHANCEMENT Using LeftAndMain->canView() in LeftAndMain->init() - if current admin interface can't be viewed, iterate over remaining interfaces until we find a valid one. This only includes admin interfaces with a valid controller, so it should fix the obnoxious redirect to userhelp.silverstripe.com when a website-user tries to access the CMS.

ENHANCEMENT Added LeftAndMain->canView() to check for logged-in member and CMS_ACCESS_* permissions in a testable way
ENHANCEMENT Don't show "Reports" admin section if no subclasses of SSReport are found (or none of the existing subclasses returns a valid canView())
ENHANCEMENT Added CMSMenu::get_viewable_menu_items() and using it in LeftAndMain->MainMenu()

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/cms/branches/2.3@68460 467b73ca-7a2a-4603-9d3b-597d59a354a9

code/CMSMenu.php

@@ -1,6 +1,7 @@
 <?php
 /**
- * The object manages the main CMS menu
+ * The object manages the main CMS menu.
+ * See {@link LeftAndMain::init()} for example usage.
  * 
  * @package cms
  * @subpackage content
@@ -123,6 +124,31 @@ class CMSMenu extends Object implements Iterator, i18nEntityProvider
 		return self::$menu_items;
 	}
 	
+	/**
+	 * Get all menu items that the passed member can view.
+	 * Defaults to {@link Member::currentUser()}.
+	 * 
+	 * @param Member $member
+	 * @return array
+	 */
+	public static function get_viewable_menu_items($member = null) {
+		if(!$member && $member !== FALSE) {
+			$member = Member::currentUser();
+		}
+		
+		$viewableMenuItems = array();
+		$allMenuItems = self::get_menu_items();
+		if($allMenuItems) foreach($allMenuItems as $code => $menuItem) {
+			// exclude all items which have a controller to perform permission
+			// checks on
+			if($menuItem->controller && !singleton($menuItem->controller)->canView($member)) continue;
+			
+			$viewableMenuItems[$code] = $menuItem;
+		}
+		
+		return $viewableMenuItems;
+	}
+	
 	/**
 	 * Removes an existing item from the menu.
 	 *

code/LeftAndMain.php

@@ -64,11 +64,41 @@ class LeftAndMain extends Controller {
 		'themedcss' => array(),
 	);
 	
+	/**
+	 * @param Member $member
+	 * @return boolean
+	 */
+	function canView($member = null) {
+		if(!$member && $member !== FALSE) {
+			$member = Member::currentUser();
+		}
+		
+		// cms menus only for logged-in members
+		if(!$member) return false;
+		
+		// alternative decorated checks
+		if($this->hasMethod('alternateAccessCheck')) {
+			$alternateAllowed = $this->alternateAccessCheck();
+			if($alternateAllowed === FALSE) return false;
+		}
+			
+		// Default security check for LeftAndMain sub-class permissions
+		if(!Permission::checkMember($member, "CMS_ACCESS_$this->class")) {
+			return false;
+		}
+		
+		return true;
+	}
+	
 	/**
 	 * @uses LeftAndMainDecorator->init()
 	 * @uses LeftAndMainDecorator->accessedCMS()
+	 * @uses CMSMenu
+	 * @uses Director::set_site_mode()
 	 */
 	function init() {
+		parent::init();
+
 		Director::set_site_mode('cms');
 		
 		// set language
@@ -89,39 +119,38 @@ class LeftAndMain extends Controller {
 			Translatable::choose_site_lang(array_keys(i18n::get_existing_content_languages('SiteTree')));
 		}
 
-		parent::init();
-		
 		// Allow customisation of the access check by a decorator
-		if($this->hasMethod('alternateAccessCheck')) {
+		if(!$this->canView()) {
-			$isAllowed = $this->alternateAccessCheck();
+			// When access /admin/, we should try a redirect to another part of the admin rather than be locked out
-			
+			$menu = $this->MainMenu();
-		// Default security check for LeftAndMain sub-class permissions
+			foreach($menu as $candidate) {
-		} else {
+				if(
-			$isAllowed = Permission::check("CMS_ACCESS_$this->class");
+					$candidate->Link && 
-			if(!$isAllowed && $this->class == 'CMSMain') {
+					$candidate->Link != $this->Link() 
-				// When access /admin/, we should try a redirect to another part of the admin rather than be locked out
+					&& $candidate->MenuItem->controller 
-				$menu = $this->MainMenu();
+					&& singleton($candidate->MenuItem->controller)->canView()
-				if(($first = $menu->First()) && $first->Link) {
+				) {
-					Director::redirect($first->Link);
+					return Director::redirect($candidate->Link);
 				}
 			}
-		}
 			
-		// Don't continue if there's already been a redirection request.
+			if(Member::currentUser()) {
-		if(Director::redirected_to()) return;
+				Session::set("BackURL", null);
+			}
 			
-		// Access failure!		
+			// if no alternate menu items have matched, return a permission error
-		if(!$isAllowed) {
 			$messageSet = array(
 				'default' => _t('LeftAndMain.PERMDEFAULT',"Please choose an authentication method and enter your credentials to access the CMS."),
 				'alreadyLoggedIn' => _t('LeftAndMain.PERMALREADY',"I'm sorry, but you can't access that part of the CMS.  If you want to log in as someone else, do so below"),
 				'logInAgain' => _t('LeftAndMain.PERMAGAIN',"You have been logged out of the CMS.  If you would like to log in again, enter a username and password below."),
 			);
 
-			Security::permissionFailure($this, $messageSet);
+			return Security::permissionFailure($this, $messageSet);
-			return;
 		}
 
+		// Don't continue if there's already been a redirection request.
+		if(Director::redirected_to()) return;
+
 		// Audit logging hook
 		if(empty($_REQUEST['executeForm']) && !Director::is_ajax()) $this->extend('accessedCMS');
 		
@@ -344,17 +373,17 @@ class LeftAndMain extends Controller {
 
 		// Encode into DO set
 		$menu = new DataObjectSet();
-		foreach(singleton('CMSMenu') as $code => $menuItem) {
+		$menuItems = CMSMenu::get_viewable_menu_items();
-			if(isset($menuItem->controller) && $this->hasMethod('alternateMenuDisplayCheck')) {
+		if($menuItems) foreach($menuItems as $code => $menuItem) {
-				$isAllowed = $this->alternateMenuDisplayCheck($menuItem->controller);
+			// alternate permission checks (in addition to LeftAndMain->canView())
-			} elseif(isset($menuItem->controller)) {
+			if(
-				$isAllowed = Permission::check("CMS_ACCESS_" . $menuItem->controller);
+				isset($menuItem->controller) 
-			} else {
+				&& $this->hasMethod('alternateMenuDisplayCheck')
-				$isAllowed = true;
+				&& !$this->alternateMenuDisplayCheck($menuItem->controller)
+			) {
+				continue;
 			}
 
-			if(!$isAllowed) continue;
-
 			$linkingmode = "";
 			
 			if(strpos($this->Link(), $menuItem->url) !== false) {
@@ -381,6 +410,7 @@ class LeftAndMain extends Controller {
 			}
 			
 			$menu->push(new ArrayData(array(
+				"MenuItem" => $menuItem,
 				"Title" => Convert::raw2xml($title),
 				"Code" => $code,
 				"Link" => $menuItem->url,

code/ReportAdmin.php

@@ -41,6 +41,35 @@ class ReportAdmin extends LeftAndMain {
 		}
 	}
 	
+	/**
+	 * Does the parent permission checks, but also
+	 * makes sure that instantiatable subclasses of
+	 * {@link Report} exist. By default, the CMS doesn't
+	 * include any Reports, so there's no point in showing
+	 * 
+	 * @param Member $member
+	 * @return boolean
+	 */
+	function canView($member = null) {
+		if(!$member && $member !== FALSE) {
+			$member = Member::currentUser();
+		}
+		
+		if(!parent::canView($member)) return false;
+		
+		$hasViewableSubclasses = false;
+		$subClasses = array_values(ClassInfo::subclassesFor('SSReport'));
+		foreach($subClasses as $subclass) {
+			// Remove abstract classes and LeftAndMain
+			$classReflection = new ReflectionClass($subclass);
+			if($classReflection->isInstantiable() && $subclass != 'SSReport') {
+				if(singleton($subclass)->canView()) $hasViewableSubclasses = true;
+			}			
+		}
+		
+		return $hasViewableSubclasses;
+	}
+	
 	/**
 	 * Return a DataObjectSet of SSReport subclasses
 	 * that are available for use.

code/SSReport.php

@@ -69,6 +69,18 @@ class SSReport extends ViewableData {
 		return $fields;
 	}
 	
+	/**
+	 * @param Member $member
+	 * @return boolean
+	 */
+	function canView($member = null) {
+		if(!$member && $member !== FALSE) {
+			$member = Member::currentUser();
+		}
+		
+		return true;
+	}
+
 	/**
 	 * Return a field, such as a {@link ComplexTableField} that is
 	 * used to show and manipulate data relating to this report.

tests/CMSMainTest.yml

@@ -16,14 +16,20 @@ Group:
       Title: Administrators
    empty:
       Title: Empty Group
-        
+   assetsonly:
+      Title: assetsonly
 Member:
    admin:
       Email: admin@example.com
       Password: ZXXlkwecxz2390232233
       Groups: =>Group.admin
-   
+   assetsonlyuser:
+      Email: assetsonlyuser@test.com
+      Groups: =>Group.assetsonly
 Permission:
    admin:
       Code: ADMIN
       GroupID: =>Group.admin
+   assetsonly:
+      Code: CMS_ACCESS_AssetAdmin
+      GroupID: =>Group.assetsonly

tests/LeftAndMainTest.php

@@ -3,30 +3,32 @@
  * @package cms
  * @subpackage tests
  */
-class LeftAndMainTest extends SapphireTest {
+class LeftAndMainTest extends FunctionalTest {
 	static $fixture_file = 'cms/tests/CMSMainTest.yml';
 	
+	function setUp() {
+		parent::setUp();
+		
+		// @todo fix controller stack problems and re-activate
+		//$this->autoFollowRedirection = false;
+	}
+	
 	/**
 	 * Check that all subclasses of leftandmain can be accessed
 	 */
 	public function testLeftAndMainSubclasses() {
-		$session = new Session(array(
+		$adminuser = $this->objFromFixture('Member','admin');
-			'loggedInAs' => $this->idFromFixture('Member','admin')
+		$this->session()->inst_set('loggedInAs', $adminuser->ID);
-		)); 
 		
-		// This controller stuff is needed because LeftAndMain::MainMenu() inspects the current user's permissions
-		$controller = new Controller();
-		$controller->setSession($session);
-		$controller->pushCurrent();
 		$menuItems = singleton('CMSMain')->MainMenu();
-		$controller->popCurrent();
-		
-		$classes = ClassInfo::subclassesFor("LeftAndMain");
 		foreach($menuItems as $menuItem) {
 			$link = $menuItem->Link;
+			
+			// don't test external links
 			if(preg_match('/^https?:\/\//',$link)) continue;
 
-			$response = Director::test($link, null, $session);
+			$response = $this->get($link);
+			
 			$this->assertType('HTTPResponse', $response, "$link should return a response object");
 			$this->assertEquals(200, $response->getStatusCode(), "$link should return 200 status code");
 			// Check that a HTML page has been returned
@@ -34,6 +36,48 @@ class LeftAndMainTest extends SapphireTest {
 			$this->assertRegExp('/<head[^>]*>/i', $response->getBody(), "$link should contain <head> tag");
 			$this->assertRegExp('/<body[^>]*>/i', $response->getBody(), "$link should contain <body> tag");
 		}
+		
+		$this->session()->inst_set('loggedInAs', null);
+
+	}
+
+	function testCanView() {
+		$adminuser = $this->objFromFixture('Member', 'admin');
+		$assetsonlyuser = $this->objFromFixture('Member', 'assetsonlyuser');
+		
+		// anonymous user
+		$this->session()->inst_set('loggedInAs', null);
+		$menuItems = singleton('LeftAndMain')->MainMenu();
+		$this->assertEquals(
+			$menuItems->column('Code'),
+			array(),
+			'Without valid login, members cant access any menu entries'
+		);
+		
+		// restricted cms user
+		$this->session()->inst_set('loggedInAs', $assetsonlyuser->ID);
+		$menuItems = singleton('LeftAndMain')->MainMenu();
+		$this->assertEquals(
+			$menuItems->column('Code'),
+			array('AssetAdmin','Help'),
+			'Groups with limited access can only access the interfaces they have permissions for'
+		);
+		
+		// admin
+		$this->session()->inst_set('loggedInAs', $adminuser->ID);
+		$menuItems = singleton('LeftAndMain')->MainMenu();
+		$this->assertContains(
+			'CMSMain',
+			$menuItems->column('Code'),
+			'Administrators can access CMS'
+		);
+		$this->assertContains(
+			'AssetAdmin',
+			$menuItems->column('Code'),
+			'Administrators can access Assets'
+		);
+		
+		$this->session()->inst_set('loggedInAs', null);
 	}
 	
 }