From d9e9e5f348b208f63bdc3dbe3d2f5016aaa3a8bd Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 11 Dec 2008 22:25:42 +0000 Subject: [PATCH] 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 | 28 +++++++++++- code/LeftAndMain.php | 94 ++++++++++++++++++++++++++------------- code/ReportAdmin.php | 29 ++++++++++++ code/SSReport.php | 12 +++++ tests/CMSMainTest.yml | 10 ++++- tests/LeftAndMainTest.php | 70 +++++++++++++++++++++++------ 6 files changed, 195 insertions(+), 48 deletions(-) diff --git a/code/CMSMenu.php b/code/CMSMenu.php index e5244e46..cabcda14 100644 --- a/code/CMSMenu.php +++ b/code/CMSMenu.php @@ -1,6 +1,7 @@ $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. * diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index a8d3ee0b..d3a4b7ac 100644 --- a/code/LeftAndMain.php +++ b/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')) { - $isAllowed = $this->alternateAccessCheck(); - - // Default security check for LeftAndMain sub-class permissions - } else { - $isAllowed = Permission::check("CMS_ACCESS_$this->class"); - if(!$isAllowed && $this->class == 'CMSMain') { - // When access /admin/, we should try a redirect to another part of the admin rather than be locked out - $menu = $this->MainMenu(); - if(($first = $menu->First()) && $first->Link) { - Director::redirect($first->Link); + if(!$this->canView()) { + // When access /admin/, we should try a redirect to another part of the admin rather than be locked out + $menu = $this->MainMenu(); + foreach($menu as $candidate) { + if( + $candidate->Link && + $candidate->Link != $this->Link() + && $candidate->MenuItem->controller + && singleton($candidate->MenuItem->controller)->canView() + ) { + return Director::redirect($candidate->Link); } } - } - - // Don't continue if there's already been a redirection request. - if(Director::redirected_to()) return; - - // Access failure! - if(!$isAllowed) { + + if(Member::currentUser()) { + Session::set("BackURL", null); + } + + // if no alternate menu items have matched, return a permission error $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; + return Security::permissionFailure($this, $messageSet); } - + + // 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) { - if(isset($menuItem->controller) && $this->hasMethod('alternateMenuDisplayCheck')) { - $isAllowed = $this->alternateMenuDisplayCheck($menuItem->controller); - } elseif(isset($menuItem->controller)) { - $isAllowed = Permission::check("CMS_ACCESS_" . $menuItem->controller); - } else { - $isAllowed = true; + $menuItems = CMSMenu::get_viewable_menu_items(); + if($menuItems) foreach($menuItems as $code => $menuItem) { + // alternate permission checks (in addition to LeftAndMain->canView()) + if( + isset($menuItem->controller) + && $this->hasMethod('alternateMenuDisplayCheck') + && !$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, diff --git a/code/ReportAdmin.php b/code/ReportAdmin.php index bc6c6e74..258098cd 100755 --- a/code/ReportAdmin.php +++ b/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. diff --git a/code/SSReport.php b/code/SSReport.php index 72f1568f..518567ee 100644 --- a/code/SSReport.php +++ b/code/SSReport.php @@ -68,6 +68,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 diff --git a/tests/CMSMainTest.yml b/tests/CMSMainTest.yml index 095b38ce..e7f712fc 100644 --- a/tests/CMSMainTest.yml +++ b/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 \ No newline at end of file diff --git a/tests/LeftAndMainTest.php b/tests/LeftAndMainTest.php index 9ac7c152..4997d731 100644 --- a/tests/LeftAndMainTest.php +++ b/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( - 'loggedInAs' => $this->idFromFixture('Member','admin') - )); - - // 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(); + $adminuser = $this->objFromFixture('Member','admin'); + $this->session()->inst_set('loggedInAs', $adminuser->ID); - $classes = ClassInfo::subclassesFor("LeftAndMain"); + $menuItems = singleton('CMSMain')->MainMenu(); 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('/]*>/i', $response->getBody(), "$link should contain tag"); $this->assertRegExp('/]*>/i', $response->getBody(), "$link should contain 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); } }