From 2abb021efb01504c45d5b75b7e1dc2167ff4dc9a Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 5 Mar 2012 16:07:20 +0100 Subject: [PATCH] BUGFIX Restored old permission code model, broken due to new controller structure. Introduced LeftAndMain::$required_permission_codes as a way to control permissions independently of subclasses, and "cluster" multiple classes under a single code. --- admin/code/CMSProfileController.php | 1 + admin/code/LeftAndMain.php | 63 ++++++++++++++++++++++++----- admin/code/SecurityAdmin.php | 11 ++++- lang/en_US.php | 2 +- security/Group.php | 1 - 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/admin/code/CMSProfileController.php b/admin/code/CMSProfileController.php index 7d902b03c..c08172214 100644 --- a/admin/code/CMSProfileController.php +++ b/admin/code/CMSProfileController.php @@ -2,6 +2,7 @@ class CMSProfileController extends LeftAndMain { static $url_segment = 'myprofile'; + static $required_permission_codes = false; public function index($request) { $form = $this->Member_ProfileForm(); diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 40ab6ef25..2ad55c1ff 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -9,7 +9,7 @@ * @package cms * @subpackage core */ -class LeftAndMain extends Controller { +class LeftAndMain extends Controller implements PermissionProvider { /** * The 'base' url for CMS administration areas. @@ -83,6 +83,15 @@ class LeftAndMain extends Controller { 'BatchActionsForm', 'Member_ProfileForm', ); + + /** + * @var Array Codes which are required from the current user to view this controller. + * If multiple codes are provided, all of them are required. + * All CMS controllers require "CMS_ACCESS_LeftAndMain" as a baseline check, + * and fall back to "CMS_ACCESS_" if no permissions are defined here. + * See {@link canView()} for more details on permission checks. + */ + static $required_permission_codes; /** * Register additional requirements through the {@link Requirements} class. @@ -99,13 +108,10 @@ class LeftAndMain extends Controller { /** * @param Member $member - * * @return boolean */ 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; @@ -115,12 +121,18 @@ class LeftAndMain extends Controller { $alternateAllowed = $this->alternateAccessCheck(); if($alternateAllowed === FALSE) return false; } - - // Default security check for LeftAndMain sub-class permissions - if(!Permission::checkMember($member, "CMS_ACCESS_$this->class") && - !Permission::checkMember($member, "CMS_ACCESS_LeftAndMain")) { - return false; + + // Check for "CMS admin" permission + if(Permission::checkMember($member, "CMS_ACCESS_LeftAndMain")) return true; + + // Check for LeftAndMain sub-class permissions + $codes = array(); + $extraCodes = $this->stat('required_permission_codes'); + if($extraCodes !== false) { // allow explicit FALSE to disable subclass check + if($extraCodes) $codes = array_merge($codes, (array)$extraCodes); + else $codes[] = "CMS_ACCESS_$this->class"; } + foreach($codes as $code) if(!Permission::checkMember($member, $code)) return false; return true; } @@ -1313,6 +1325,37 @@ class LeftAndMain extends Controller { function Locale() { return DBField::create('DBLocale', $this->i18nLocale()); } + + function providePermissions() { + $perms = array( + "CMS_ACCESS_LeftAndMain" => array( + 'name' => _t('CMSMain.ACCESSALLINTERFACES', 'Access to all CMS sections'), + 'category' => _t('Permission.CMS_ACCESS_CATEGORY', 'CMS Access'), + 'help' => _t('CMSMain.ACCESSALLINTERFACESHELP', 'Overrules more specific access settings.'), + 'sort' => -100 + ) + ); + + // Add any custom ModelAdmin subclasses. Can't put this on ModelAdmin itself + // since its marked abstract, and needs to be singleton instanciated. + foreach(ClassInfo::subclassesFor('ModelAdmin') as $i => $class) { + if($class == 'ModelAdmin') continue; + if(ClassInfo::classImplements($class, 'TestOnly')) continue; + + $title = _t("{$class}.MENUTITLE", LeftAndMain::menu_title_for_class($class)); + $perms["CMS_ACCESS_" . $class] = array( + 'name' => sprintf(_t( + 'CMSMain.ACCESS', + "Access to '%s' section", + PR_MEDIUM, + "Item in permission selection identifying the admin section. Example: Access to 'Files & Images'" + ), $title, null), + 'category' => _t('Permission.CMS_ACCESS_CATEGORY', 'CMS Access') + ); + } + + return $perms; + } /** * Register the given javascript file as required in the CMS. diff --git a/admin/code/SecurityAdmin.php b/admin/code/SecurityAdmin.php index d01b29de8..021ee6be4 100755 --- a/admin/code/SecurityAdmin.php +++ b/admin/code/SecurityAdmin.php @@ -277,7 +277,16 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider { } function providePermissions() { + $title = _t("SecurityAdmin.MENUTITLE", LeftAndMain::menu_title_for_class($this->class)); return array( + "CMS_ACCESS_SecurityAdmin" => array( + 'name' => sprintf(_t('CMSMain.ACCESS', "Access to '%s' section"), $title), + 'category' => _t('Permission.CMS_ACCESS_CATEGORY', 'CMS Access'), + 'help' => _t( + 'SecurityAdmin.ACCESS_HELP', + 'Allow viewing, adding and editing users, as well as assigning permissions and roles to them.' + ) + ), 'EDIT_PERMISSIONS' => array( 'name' => _t('SecurityAdmin.EDITPERMISSIONS', 'Manage permissions for groups'), 'category' => _t('Permissions.PERMISSIONS_CATEGORY', 'Roles and access permissions'), @@ -287,7 +296,7 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider { 'APPLY_ROLES' => array( 'name' => _t('SecurityAdmin.APPLY_ROLES', 'Apply roles to groups'), 'category' => _t('Permissions.PERMISSIONS_CATEGORY', 'Roles and access permissions'), - 'help' => _t('SecurityAdmin.APPLY_ROLES_HELP', 'Ability to edit the roles assigned to a group. Requires the "Access to \'Security\' section" permission.'), + 'help' => _t('SecurityAdmin.APPLY_ROLES_HELP', 'Ability to edit the roles assigned to a group. Requires the "Access to \'Users\' section" permission.'), 'sort' => 0 ) ); diff --git a/lang/en_US.php b/lang/en_US.php index 1625f8676..a09a58066 100644 --- a/lang/en_US.php +++ b/lang/en_US.php @@ -716,7 +716,7 @@ $lang['en_US']['Security']['PASSWORDSENTHEADER'] = 'Password reset link sent to $lang['en_US']['Security']['PASSWORDSENTTEXT'] = 'Thank you! A reset link has been sent to \'%s\', provided an account exists for this email address.'; $lang['en_US']['SecurityAdmin']['ADDMEMBER'] = 'Add Member'; $lang['en_US']['SecurityAdmin']['APPLY_ROLES'] = 'Apply roles to groups'; -$lang['en_US']['SecurityAdmin']['APPLY_ROLES_HELP'] = 'Ability to edit the roles assigned to a group. Requires the "Access to \'Security\' section" permission.'; +$lang['en_US']['SecurityAdmin']['APPLY_ROLES_HELP'] = 'Ability to edit the roles assigned to a group. Requires the "Access to \'Users\' section" permission.'; $lang['en_US']['SecurityAdmin']['EDITPERMISSIONS'] = 'Manage permissions for groups'; $lang['en_US']['SecurityAdmin']['EDITPERMISSIONS_HELP'] = 'Ability to edit Permissions and IP Addresses for a group. Requires the "Access to \'Security\' section" permission.'; $lang['en_US']['SecurityAdmin']['GROUPNAME'] = 'Group name'; diff --git a/security/Group.php b/security/Group.php index 9bd01156f..f8c5daadd 100755 --- a/security/Group.php +++ b/security/Group.php @@ -431,7 +431,6 @@ class Group extends DataObject { $authorGroup->write(); Permission::grant($authorGroup->ID, 'CMS_ACCESS_CMSMain'); Permission::grant($authorGroup->ID, 'CMS_ACCESS_AssetAdmin'); - Permission::grant($authorGroup->ID, 'CMS_ACCESS_CommentAdmin'); Permission::grant($authorGroup->ID, 'CMS_ACCESS_ReportAdmin'); Permission::grant($authorGroup->ID, 'SITETREE_REORGANISE'); }