From f6b7cf88893fdc5bc50f1c10d59696b41f924dc2 Mon Sep 17 00:00:00 2001 From: Christopher Joe Date: Fri, 27 Oct 2017 10:49:38 +1300 Subject: [PATCH] Feature disable current user from removing their admin permission --- behat.yml | 7 ++ lang/en.yml | 3 + src/Dev/Deprecation.php | 3 +- src/Forms/GridField/GridFieldDeleteAction.php | 4 +- .../GridField/GridFieldGroupDeleteAction.php | 91 +++++++++++++++++++ src/Security/Group.php | 6 ++ src/Security/Member_Validator.php | 31 +++++++ tests/behat/features/manage-users.feature | 22 ++++- tests/behat/features/profile.feature | 18 +++- .../features/security-permissions.feature | 2 +- tests/behat/src/CmsFormsContext.php | 87 +++++++++++++++++- 11 files changed, 265 insertions(+), 9 deletions(-) create mode 100644 src/Forms/GridField/GridFieldGroupDeleteAction.php diff --git a/behat.yml b/behat.yml index 4b65e29b8..faf1eea89 100644 --- a/behat.yml +++ b/behat.yml @@ -1,3 +1,10 @@ +# Run framework behat tests with this command (installed with silverstripe/installer) +# Note that framework behat tests require CMS module +# ========================================================================= # +# vendor/bin/selenium-server-standalone -Dwebdriver.firefox.bin="/Applications/Firefox31.app/Contents/MacOS/firefox-bin" +# vendor/bin/serve --bootstrap-file vendor/silverstripe/framework/tests/behat/serve-bootstrap.php +# vendor/bin/behat @framework +# ========================================================================= # default: suites: framework: diff --git a/lang/en.yml b/lang/en.yml index 6c6c78921..3bd8d7a94 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -85,6 +85,8 @@ en: Print: Print RelationSearch: 'Relation search' ResetFilter: Reset + SilverStripe\Forms\GridField\GridFieldGroupDeleteAction: + UnlinkSelfFailure: 'Cannot remove yourself from this group, you will lose admin rights' SilverStripe\Forms\GridField\GridFieldDeleteAction: DELETE_DESCRIPTION: Delete Delete: Delete @@ -247,6 +249,7 @@ en: other: '{count} Members' REMEMBERME: 'Remember me next time? (for {count} days on this device)' SINGULARNAME: Member + VALIDATIONADMINLOSTACCESS: 'Cannot remove all admin groups from your profile' SUBJECTPASSWORDCHANGED: 'Your password has been changed' SUBJECTPASSWORDRESET: 'Your password reset link' SURNAME: Surname diff --git a/src/Dev/Deprecation.php b/src/Dev/Deprecation.php index fbb350c7c..c7389defc 100644 --- a/src/Dev/Deprecation.php +++ b/src/Dev/Deprecation.php @@ -3,6 +3,7 @@ namespace SilverStripe\Dev; use SilverStripe\Control\Director; +use SilverStripe\Core\Environment; use SilverStripe\Core\Manifest\ClassLoader; use SilverStripe\Core\Manifest\Module; use SilverStripe\Core\Manifest\ModuleLoader; @@ -148,7 +149,7 @@ class Deprecation if (isset(self::$enabled)) { return self::$enabled; } - return getenv('SS_DEPRECATION_ENABLED') ?: true; + return Environment::getEnv('SS_DEPRECATION_ENABLED') ?: true; } /** diff --git a/src/Forms/GridField/GridFieldDeleteAction.php b/src/Forms/GridField/GridFieldDeleteAction.php index 0be50059f..d819c8905 100644 --- a/src/Forms/GridField/GridFieldDeleteAction.php +++ b/src/Forms/GridField/GridFieldDeleteAction.php @@ -119,6 +119,7 @@ class GridFieldDeleteAction implements GridField_ColumnProvider, GridField_Actio if (!$record->canEdit()) { return null; } + $title = _t('SilverStripe\\Forms\\GridField\\GridFieldDeleteAction.UnlinkRelation', "Unlink"); $field = GridField_FormAction::create( $gridField, @@ -128,7 +129,8 @@ class GridFieldDeleteAction implements GridField_ColumnProvider, GridField_Actio array('RecordID' => $record->ID) ) ->addExtraClass('btn btn--no-text btn--icon-md font-icon-link-broken grid-field__icon-action gridfield-button-unlink') - ->setAttribute('title', _t('SilverStripe\\Forms\\GridField\\GridFieldDeleteAction.UnlinkRelation', "Unlink")); + ->setAttribute('title', $title) + ->setAttribute('aria-label', $title); } else { if (!$record->canDelete()) { return null; diff --git a/src/Forms/GridField/GridFieldGroupDeleteAction.php b/src/Forms/GridField/GridFieldGroupDeleteAction.php new file mode 100644 index 000000000..3abe274b0 --- /dev/null +++ b/src/Forms/GridField/GridFieldGroupDeleteAction.php @@ -0,0 +1,91 @@ +groupID = $groupID; + parent::__construct(true); + } + + /** + * + * @param GridField $gridField + * @param DataObject $record + * @param string $columnName + * @return string the HTML for the column + */ + public function getColumnContent($gridField, $record, $columnName) + { + if ($this->canUnlink($record)) { + return parent::getColumnContent($gridField, $record, $columnName); + } + return null; + } + + /** + * Handle the actions and apply any changes to the GridField + * + * @param GridField $gridField + * @param string $actionName + * @param mixed $arguments + * @param array $data - form data + * @throws ValidationException + */ + public function handleAction(GridField $gridField, $actionName, $arguments, $data) + { + $record = $gridField->getList()->find('ID', $arguments['RecordID']); + + if (!$record || !$actionName == 'unlinkrelation' || $this->canUnlink($record)) { + parent::handleAction($gridField, $actionName, $arguments, $data); + return; + } + + throw new ValidationException( + _t(__CLASS__ . '.UnlinkSelfFailure', 'Cannot remove yourself from this group, you will lose admin rights') + ); + } + + /** + * @param $record - the record of the User to unlink with + * @return bool + */ + protected function canUnlink($record) + { + $currentUser = Security::getCurrentUser(); + if (($record instanceof Member && $record->ID === $currentUser->ID) + && Permission::checkMember($record, 'ADMIN') + ) { + $adminGroups = array_intersect( + $record->Groups()->column(), + Permission::get_groups_by_permission('ADMIN')->column() + ); + + if (count($adminGroups) === 1 && array_search($this->groupID, $adminGroups) !== false) { + return false; + } + } + return true; + } +} diff --git a/src/Security/Group.php b/src/Security/Group.php index 5ba3fc424..b57b1f7fb 100755 --- a/src/Security/Group.php +++ b/src/Security/Group.php @@ -11,8 +11,11 @@ use SilverStripe\Forms\GridField\GridField; use SilverStripe\Forms\GridField\GridFieldAddExistingAutocompleter; use SilverStripe\Forms\GridField\GridFieldButtonRow; use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor; +use SilverStripe\Forms\GridField\GridFieldDeleteAction; use SilverStripe\Forms\GridField\GridFieldDetailForm; use SilverStripe\Forms\GridField\GridFieldExportButton; +use SilverStripe\Forms\GridField\GridFieldGroupDeleteAction; +use SilverStripe\Forms\GridField\GridFieldPageCount; use SilverStripe\Forms\GridField\GridFieldPrintButton; use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig; @@ -150,6 +153,9 @@ class Group extends DataObject $config->addComponent(new GridFieldButtonRow('after')); $config->addComponents(new GridFieldExportButton('buttons-after-left')); $config->addComponents(new GridFieldPrintButton('buttons-after-left')); + $config->removeComponentsByType(GridFieldDeleteAction::class); + $config->addComponent(new GridFieldGroupDeleteAction($this->ID), GridFieldPageCount::class); + /** @var GridFieldAddExistingAutocompleter $autocompleter */ $autocompleter = $config->getComponentByType(GridFieldAddExistingAutocompleter::class); /** @skipUpgrade */ diff --git a/src/Security/Member_Validator.php b/src/Security/Member_Validator.php index aa8244573..5e7835b31 100644 --- a/src/Security/Member_Validator.php +++ b/src/Security/Member_Validator.php @@ -137,6 +137,37 @@ class Member_Validator extends RequiredFields } } + $currentUser = Security::getCurrentUser(); + $id = $data['ID']; + if ($id === $currentUser->ID && Permission::checkMember($currentUser, 'ADMIN')) { + $stillAdmin = true; + + if (!isset($data['DirectGroups'])) { + $stillAdmin = false; + } else { + $adminGroups = array_intersect( + $data['DirectGroups'], + Permission::get_groups_by_permission('ADMIN')->column() + ); + + if (count($adminGroups) === 0) { + $stillAdmin = false; + } + } + + if (!$stillAdmin) { + $this->validationError( + 'DirectGroups', + _t( + 'SilverStripe\\Security\\Member.VALIDATIONADMINLOSTACCESS', + 'Cannot remove all admin groups from your profile' + ), + 'required' + ); + } + } + + // Execute the validators on the extensions $results = $this->extend('updatePHP', $data, $this->form); diff --git a/tests/behat/features/manage-users.feature b/tests/behat/features/manage-users.feature index 58d689c61..c07ef0438 100644 --- a/tests/behat/features/manage-users.feature +++ b/tests/behat/features/manage-users.feature @@ -5,12 +5,30 @@ Feature: Manage users So that I can control access to the CMS Background: - Given a "member" "ADMIN" belonging to "ADMIN Group" with "Email"="admin@test.com" - And a "member" "Staff" belonging to "Staff Group" with "Email"="staffmember@test.com" + Given a "member" "ADMIN" belonging to "ADMIN group" with "Email"="ADMIN@example.org" + And the "member" "ADMIN" belonging to "ADMIN group2" + And a "member" "Staff" belonging to "Staff group" with "Email"="staffmember@test.com" And the "group" "ADMIN group" has permissions "Full administrative rights" + And the "group" "ADMIN group2" has permissions "Full administrative rights" And I am logged in with "ADMIN" permissions And I go to "/admin/security" + + Scenario: I cannot remove my admin access, but can remove myself from an admin group + When I click the "Groups" CMS tab + And I click "ADMIN group" in the "#Root_Groups" element + And I should see the "Unlink" button in the "Members" gridfield for the "ADMIN" row + Then I click "Groups" in the ".breadcrumbs-wrapper" element + And I click the "Groups" CMS tab + And I click "ADMIN group2" in the "#Root_Groups" element + And I should see the "Unlink" button in the "Members" gridfield for the "ADMIN" row + Then I click the "Unlink" button in the "Members" gridfield for the "ADMIN" row + And I should not see the "Unlink" button in the "Members" gridfield for the "ADMIN" row + Then I click "Groups" in the ".breadcrumbs-wrapper" element + And I click the "Groups" CMS tab + And I click "ADMIN group" in the "#Root_Groups" element + And I should not see the "Unlink" button in the "Members" gridfield for the "ADMIN" row + Scenario: I can list all users regardless of group When I click the "Users" CMS tab Then I should see "admin@test.com" in the "#Root_Users" element diff --git a/tests/behat/features/profile.feature b/tests/behat/features/profile.feature index 8c21656ec..89cae4e0c 100644 --- a/tests/behat/features/profile.feature +++ b/tests/behat/features/profile.feature @@ -5,11 +5,25 @@ Feature: Manage my own settings In order to streamline my CMS experience Background: - Given a "member" "Joe" belonging to "Admin Group" with "Email"="joe@test.com" and "Password"="secret" - And the "group" "Admin Group" has permissions "Full administrative rights" + Given a "member" "Joe" belonging to "Admin group" with "Email"="joe@test.com" and "Password"="secret" + And the "group" "Admin group" has permissions "Full administrative rights" + And the "member" "Joe" belonging to "Admin group2" + And the "group" "Admin group2" has permissions "Full administrative rights" And I log in with "joe@test.com" and "secret" And I go to "admin/myprofile" + Scenario: I cannot remove all my admin groups + When I click the "Admin group" option in the "DirectGroups" listbox + And I click the "Admin group2" option in the "DirectGroups" listbox + And I press the "Save" button + Then I should see "Cannot remove all admin groups from your profile" in the "#Form_EditForm" element + + Scenario: I can remove one of my admin groups + When I click the "Admin group" option in the "DirectGroups" listbox + And I press the "Save" button + Then I should see a "Saved" notice + And I should not see "Cannot remove all admin groups from your profile" in the "#Form_EditForm" element + Scenario: I can edit my personal details Given I fill in "First Name" with "Jack" And I fill in "Surname" with "Johnson" diff --git a/tests/behat/features/security-permissions.feature b/tests/behat/features/security-permissions.feature index ded5cb0f8..afb1ee04d 100644 --- a/tests/behat/features/security-permissions.feature +++ b/tests/behat/features/security-permissions.feature @@ -6,7 +6,7 @@ Feature: Manage Security Permissions for Groups Background: Given a "group" "test group" - And a "member" "ADMIN" belonging to "ADMIN Group" with "Email"="admin@test.com" + And a "member" "ADMIN" belonging to "ADMIN group" with "Email"="admin@test.com" And the "group" "ADMIN group" has permissions "Full administrative rights" And I am logged in with "ADMIN" permissions And I go to "/admin/security" diff --git a/tests/behat/src/CmsFormsContext.php b/tests/behat/src/CmsFormsContext.php index a5e41c041..6498e6cc7 100644 --- a/tests/behat/src/CmsFormsContext.php +++ b/tests/behat/src/CmsFormsContext.php @@ -95,7 +95,7 @@ class CmsFormsContext implements Context { $element = $this->getHtmlField($locator); $actual = $element->getValue(); - $regex = '/'.preg_quote($html, '/').'/ui'; + $regex = '/' . preg_quote($html, '/') . '/ui'; $failed = false; if (trim($negative)) { @@ -230,7 +230,7 @@ JS; */ public function iClickOnTheHtmlFieldButton($button) { - $xpath = "//*[@aria-label='".$button."']"; + $xpath = "//*[@aria-label='" . $button . "']"; $session = $this->getSession(); $element = $session->getPage()->find('xpath', $xpath); if (null === $element) { @@ -352,4 +352,87 @@ JS; // Destroy cookie to detach session $this->getMainContext()->getSession()->setCookie('PHPSESSID', null); } + + /** + * @When /^I should see the "([^"]*)" button in the "([^"]*)" gridfield for the "([^"]*)" row$/ + * @param string $buttonLabel + * @param string $gridFieldName + * @param string $rowName + */ + public function assertIShouldSeeTheGridFieldButtonForRow($buttonLabel, $gridFieldName, $rowName) + { + $button = $this->getGridFieldButton($gridFieldName, $rowName, $buttonLabel); + assertNotNull($button, sprintf('Button "%s" not found', $buttonLabel)); + } + + /** + * @When /^I should not see the "([^"]*)" button in the "([^"]*)" gridfield for the "([^"]*)" row$/ + * @param string $buttonLabel + * @param string $gridFieldName + * @param string $rowName + */ + public function assertIShouldNotSeeTheGridFieldButtonForRow($buttonLabel, $gridFieldName, $rowName) + { + $button = $this->getGridFieldButton($gridFieldName, $rowName, $buttonLabel); + assertNull($button, sprintf('Button "%s" not found', $buttonLabel)); + } + + /** + * @When /^I click the "([^"]*)" button in the "([^"]*)" gridfield for the "([^"]*)" row$/ + * @param string $buttonLabel + * @param string $gridFieldName + * @param string $rowName + */ + public function stepIClickTheGridFieldButtonForRow($buttonLabel, $gridFieldName, $rowName) + { + $button = $this->getGridFieldButton($gridFieldName, $rowName, $buttonLabel); + assertNotNull($button, sprintf('Button "%s" not found', $buttonLabel)); + + $button->click(); + } + + /** + * Finds a button in the gridfield row + * + * @param $gridFieldName + * @param $rowName + * @param $buttonLabel + * @return $button + */ + protected function getGridFieldButton($gridFieldName, $rowName, $buttonLabel) + { + $page = $this->getSession()->getPage(); + $gridField = $page->find('xpath', sprintf('//*[@data-name="%s"]', $gridFieldName)); + assertNotNull($gridField, sprintf('Gridfield "%s" not found', $gridFieldName)); + + $name = $gridField->find('xpath', sprintf('//*[count(*)=0 and contains(.,"%s")]', $rowName)); + if (!$name) { + return null; + } + + $button = $name->getParent()->find('xpath', sprintf('//*[@aria-label="%s"]', $buttonLabel)); + + return $button; + } + + /** + * @When /^I click the "([^"]*)" option in the "([^"]*)" listbox$/ + * @param $optionLabel + * @param $fieldName + */ + public function stepIClickTheListBoxOption($optionLabel, $fieldName) + { + $page = $this->getSession()->getPage(); + $listBox = $page->find('xpath', sprintf('//*[@name="%s[]"]', $fieldName)); + assertNotNull($listBox, sprintf('The listbox %s is not found', $fieldName)); + + $option = $listBox->getParent() + ->find('css', '.chosen-choices') + ->find('xpath', sprintf('//*[count(*)=0 and contains(.,"%s")]', $optionLabel)); + assertNotNull($option, sprintf('Option %s is not found', $optionLabel)); + + $button = $option->getParent()->find('css', 'a'); + + $button->click(); + } }