From 671b1e1c01044897c0eee900afe92d4ea78dad87 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 20 Feb 2025 12:33:35 +1300 Subject: [PATCH] NEW Form sudo mode --- composer.json | 1 + src/Forms/Form.php | 45 +++++++++++++++++ src/Forms/GridField/GridField.php | 35 ++++++++++++++ .../GridFieldDetailForm_ItemRequest.php | 8 ++++ src/Forms/GridField/GridFieldViewButton.php | 19 +++++++- src/Forms/SudoModePasswordField.php | 29 +++++++++++ src/Security/Group.php | 2 + src/Security/Member.php | 2 + .../SessionAuthenticationHandler.php | 5 -- src/Security/Permission.php | 2 + src/Security/PermissionRole.php | 2 + src/Security/PermissionRoleCode.php | 2 + src/Security/SudoMode/SudoModeService.php | 23 ++++++--- src/View/ViewableData.php | 15 ++++++ tests/php/Forms/FormTest.php | 48 +++++++++++++++++++ .../Security/SudoMode/SudoModeServiceTest.php | 6 +-- 16 files changed, 228 insertions(+), 16 deletions(-) create mode 100644 src/Forms/SudoModePasswordField.php diff --git a/composer.json b/composer.json index 97c5664e612..75c8e9be42c 100644 --- a/composer.json +++ b/composer.json @@ -72,6 +72,7 @@ }, "conflict": { "silverstripe/behat-extension": "<5.5", + "silverstripe/mfa": "<5.4", "egulias/email-validator": "^2", "oscarotero/html-parser": "<0.1.7", "symfony/process": "<5.3.7" diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 31493df977a..d34a9cb18bf 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms; use BadMethodCallException; +use SilverStripe\Admin\LeftAndMain; use SilverStripe\Control\Controller; use SilverStripe\Control\HasRequestHandler; use SilverStripe\Control\HTTPRequest; @@ -21,6 +22,10 @@ use SilverStripe\View\SSViewer; use SilverStripe\View\ViewableData; use SilverStripe\Dev\Deprecation; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; +use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest; +use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldViewButton; /** * Base class for all forms. @@ -317,6 +322,46 @@ public function __construct( $this->setupDefaultClasses(); } + /** + * Make the form require sudo mode, which will make the form readonly and add a sudo mode password field + * unless the current user previously activated sudo mode. + * + * Note that if the parent request handler for this form isn't LeftAndMain or GridFieldDetailForm_ItemRequest, + * sudo mode will not be required by this form. + */ + public function requireSudoMode(): void + { + // Check that the current request handler for the form is one that's used + // in an admin context where sudo mode makes sense + $classes = [ + LeftAndMain::class, + GridFieldDetailForm_ItemRequest::class, + ]; + $enableSudoMode = false; + foreach ($classes as $class) { + if (is_a($this->getController(), $class)) { + $enableSudoMode = true; + break; + } + } + if (!$enableSudoMode) { + return; + } + // Check if sudo mode is currently enabled + $service = Injector::inst()->get(SudoModeServiceInterface::class); + $session = $this->getRequest()->getSession(); + if ($service->check($session)) { + return; + } + // If sudo mode is not active, make the form readonly and add a sudo mode password field + $this->makeReadonly(); + $field = SudoModePasswordField::create(SudoModePasswordField::FIELD_NAME); + // Manually call setForm() to the field as the field list is being updated after the + // form is created, which is when setForm() is normally being created + $field->setForm($this); + $this->Fields()->unshift($field); + } + /** * @return bool */ diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 7745978887b..0d99b432861 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -28,6 +28,9 @@ use SilverStripe\ORM\SS_List; use SilverStripe\View\HTML; use SilverStripe\View\ViewableData; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; +use SilverStripe\ORM\DataObject; +use SilverStripe\Forms\GridField\GridFieldViewButton; /** * Displays a {@link SS_List} in a grid format. @@ -521,6 +524,31 @@ public function FieldHolder($properties = []) { $this->extend('onBeforeRenderHolder', $this, $properties); + // Set GridField to read-only if sudo mode is required for the DataObject being managed + // and sudo mode is not active + $sudoModeTransformation = false; + $modelClass = null; + try { + $modelClass = $this->getModelClass(); + } catch (LogicException) { + // noop - it's possible to have a gridfield with custom components that don't rely on columns + // from the records in the list. + } + if (is_a($modelClass, DataObject::class, true)) { + /** @var DataObject $obj */ + $obj = Injector::inst()->create($modelClass); + if ($obj->getRequireSudoMode()) { + $session = Controller::curr()?->getRequest()?->getSession(); + if ($session) { + $service = Injector::inst()->get(SudoModeServiceInterface::class); + if (!$service->check($session)) { + $this->performReadonlyTransformation(); + $sudoModeTransformation = true; + } + } + } + } + $columns = $this->getColumns(); $list = $this->getManipulatedList(); @@ -552,6 +580,13 @@ public function FieldHolder($properties = []) if ($item instanceof GridFieldPaginator) { $total = $item->getTotalItems(); } + if ($sudoModeTransformation) { + // Modify the GridFieldViewButton on any GridFields so that it doesn't suffix the view URL with 'view' + // This allows us to gracefully reload a form in readonly mode when sudo mode is activated + if ($item instanceof GridFieldViewButton) { + $item->setSuffixViewToUrl(false); + } + } } foreach ($content as $contentKey => $contentValue) { diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index 5421edc8b1a..0632c315cf0 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -322,7 +322,15 @@ public function ItemEditForm() if ($cb) { $cb($form, $this); } + $this->extend("updateItemEditForm", $form); + + // Check if the the record is a DataObject and if that DataObject requires sudo mode + // If so then require sudo mode for the item edit form + if (is_a($this->record, DataObject::class) && $this->record->getRequireSudoMode()) { + $form->requireSudoMode(); + } + return $form; } diff --git a/src/Forms/GridField/GridFieldViewButton.php b/src/Forms/GridField/GridFieldViewButton.php index a0efccd6a67..d31738eb099 100644 --- a/src/Forms/GridField/GridFieldViewButton.php +++ b/src/Forms/GridField/GridFieldViewButton.php @@ -13,6 +13,19 @@ */ class GridFieldViewButton extends AbstractGridFieldComponent implements GridField_ColumnProvider, GridField_ActionMenuLink { + private bool $suffixViewToUrl = true; + + public function getSuffixViewToUrl(): bool + { + return $this->suffixViewToUrl; + } + + public function setSuffixViewToUrl(bool $suffixViewToUrl): static + { + $this->suffixViewToUrl = $suffixViewToUrl; + return $this; + } + /** * @inheritdoc */ @@ -44,7 +57,11 @@ public function getExtraData($gridField, $record, $columnName) */ public function getUrl($gridField, $record, $columnName) { - $link = Controller::join_links($gridField->Link('item'), $record->ID, 'view'); + $parts = [$gridField->Link('item'), $record->ID]; + if ($this->getSuffixViewToUrl()) { + $parts[] = 'view'; + } + $link = Controller::join_links(...$parts); return $gridField->addAllStateToUrl($link); } diff --git a/src/Forms/SudoModePasswordField.php b/src/Forms/SudoModePasswordField.php new file mode 100644 index 00000000000..66f79e8317b --- /dev/null +++ b/src/Forms/SudoModePasswordField.php @@ -0,0 +1,29 @@ +setTitle(''); + $this->addExtraClass('SudoModePasswordField'); + } + + public function performReadonlyTransformation() + { + // Readonly transformation should not be applied to this field + // as this field is intended to be used on a form that has been set to read only mode + return $this; + } +} diff --git a/src/Security/Group.php b/src/Security/Group.php index dff5e81b84e..c708674186b 100755 --- a/src/Security/Group.php +++ b/src/Security/Group.php @@ -90,6 +90,8 @@ class Group extends DataObject 'Code' => true, 'Sort' => true, ]; + + private static bool $require_sudo_mode = true; public function getAllChildren() { diff --git a/src/Security/Member.php b/src/Security/Member.php index 217c474727f..e8573230f52 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -108,6 +108,8 @@ class Member extends DataObject //'AutoLoginHash' => Array('type'=>'unique', 'value'=>'AutoLoginHash', 'ignoreNulls'=>true) ]; + private static bool $require_sudo_mode = true; + /** * @config * @var boolean diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index 252bb749315..d18658adf23 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -82,11 +82,6 @@ public function logIn(Member $member, $persistent = false, HTTPRequest $request if (Member::config()->get('login_marker_cookie')) { Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0); } - - // Activate sudo mode on login so the user doesn't have to reauthenticate for sudo - // actions until the sudo mode timeout expires - $service = Injector::inst()->get(SudoModeServiceInterface::class); - $service->activate($session); } /** diff --git a/src/Security/Permission.php b/src/Security/Permission.php index ce57f81bbf3..d0e804c5a3b 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -100,6 +100,8 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl 'EDIT_PERMISSIONS' ]; + private static bool $require_sudo_mode = true; + /** * Check that the current member has the given permission. * diff --git a/src/Security/PermissionRole.php b/src/Security/PermissionRole.php index 5f96f877024..93c30304ca9 100644 --- a/src/Security/PermissionRole.php +++ b/src/Security/PermissionRole.php @@ -46,6 +46,8 @@ class PermissionRole extends DataObject private static $plural_name = 'Roles'; + private static bool $require_sudo_mode = true; + public function getCMSFields() { $fields = parent::getCMSFields(); diff --git a/src/Security/PermissionRoleCode.php b/src/Security/PermissionRoleCode.php index 8d7e2979bd0..4011785abed 100644 --- a/src/Security/PermissionRoleCode.php +++ b/src/Security/PermissionRoleCode.php @@ -23,6 +23,8 @@ class PermissionRoleCode extends DataObject ]; private static $table_name = "PermissionRoleCode"; + + private static bool $require_sudo_mode = true; private static $indexes = [ "Code" => true, diff --git a/src/Security/SudoMode/SudoModeService.php b/src/Security/SudoMode/SudoModeService.php index 74fa8547fe4..6a0e158f65b 100644 --- a/src/Security/SudoMode/SudoModeService.php +++ b/src/Security/SudoMode/SudoModeService.php @@ -4,11 +4,13 @@ use SilverStripe\Control\Session; use SilverStripe\Core\Config\Configurable; +use SilverStripe\Core\Extensible; use SilverStripe\ORM\FieldType\DBDatetime; class SudoModeService implements SudoModeServiceInterface { use Configurable; + use Extensible; /** * The lifetime that sudo mode authorization lasts for, in minutes. @@ -27,16 +29,18 @@ class SudoModeService implements SudoModeServiceInterface public function check(Session $session): bool { + $active = true; $lastActivated = $session->get(SudoModeService::SUDO_MODE_SESSION_KEY); - // Not activated at all if (!$lastActivated) { - return false; + // Not activated at all + $active = false; + } else { + // Activated within the last "lifetime" window + $nowTimestamp = DBDatetime::now()->getTimestamp(); + $active = $lastActivated > ($nowTimestamp - $this->getLifetime() * 60); } - - // Activated within the last "lifetime" window - $nowTimestamp = DBDatetime::now()->getTimestamp(); - - return $lastActivated > ($nowTimestamp - $this->getLifetime() * 60); + $this->extend('updateCheck', $active, $session); + return $active; } public function activate(Session $session): bool @@ -45,6 +49,11 @@ public function activate(Session $session): bool return true; } + public function deactivate(Session $session): void + { + $session->set(SudoModeService::SUDO_MODE_SESSION_KEY, null); + } + public function getLifetime(): int { return (int) static::config()->get('lifetime_minutes'); diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index 655b1c8f266..b53d6a3a6ae 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -78,6 +78,13 @@ class ViewableData implements IteratorAggregate */ private array $dynamicData = []; + /** + * Config of whether the model requires sudo mode to be active in order to be modified in admin + * Sudo mode is a security feature that requires the user to re-enter their password before + * making changes to the database. + */ + private static bool $require_sudo_mode = false; + // ----------------------------------------------------------------------------------------------------------------- /** @@ -261,6 +268,14 @@ public function hasDynamicData(string $field): bool return array_key_exists($field, $this->dynamicData); } + /** + * Whether the model requires sudo mode to be active in order to be modified in admin + */ + public function getRequireSudoMode(): bool + { + return static::config()->get('require_sudo_mode'); + } + /** * Returns true if a method exists for the current class which isn't private. * Also returns true for private methods if $this is ViewableData (not a subclass) diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 05ad1f2055f..80defa8a41f 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -33,6 +33,13 @@ use SilverStripe\Security\SecurityToken; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; +use SilverStripe\Forms\GridField\GridFieldDetailForm; +use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest; +use SilverStripe\Security\MemberAuthenticator\LostPasswordHandler; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\SudoModePasswordField; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; +use SilverStripe\Forms\ReadonlyField; class FormTest extends FunctionalTest { @@ -1263,6 +1270,47 @@ public function testRestoreFromState() ); } + public static function provideRequireSudoMode(): array + { + return [ + 'sudo-protected' => [ + 'class' => GridFieldDetailForm_ItemRequest::class, + 'expected' => true, + ], + 'not-sudo-protected' => [ + 'class' => LostPasswordHandler::class, + 'expected' => false, + ], + ]; + } + + /** + * @dataProvider provideRequireSudoMode + */ + public function testRequireSudoMode(string $class, bool $expected): void + { + $request = Controller::curr()->getRequest(); + if ($class === GridFieldDetailForm_ItemRequest::class) { + $handler = new GridFieldDetailForm_ItemRequest(null, null, null, null, null); + } elseif ($class === LostPasswordHandler::class) { + $handler = new LostPasswordHandler(''); + } + $handler->setRequest($request); + $form = new Form($handler, 'MyForm', new FieldList(new TextField('MyTextField'))); + $form->requireSudoMode(); + $fieldList = $form->Fields(); + $last = $fieldList->last(); + $this->assertSame('MyTextField', $last->getName()); + if ($expected) { + $this->assertSame(2, $fieldList->count()); + $this->assertSame(SudoModePasswordField::class, get_class($fieldList->first())); + $this->assertSame(ReadonlyField::class, get_class($last)); + } else { + $this->assertSame(1, $fieldList->count()); + $this->assertSame(TextField::class, get_class($last)); + } + } + protected function getStubForm() { return new Form( diff --git a/tests/php/Security/SudoMode/SudoModeServiceTest.php b/tests/php/Security/SudoMode/SudoModeServiceTest.php index 0dca89f6023..cadd478bee9 100644 --- a/tests/php/Security/SudoMode/SudoModeServiceTest.php +++ b/tests/php/Security/SudoMode/SudoModeServiceTest.php @@ -61,7 +61,7 @@ public function testActivateAndCheckImmediately() $this->assertTrue($this->service->check($this->session)); } - public function testSudoModeActivatesOnLogin() + public function testSudoModeNotActiveOnLogin() { // Sometimes being logged in carries over from other tests $this->logOut(); @@ -73,9 +73,9 @@ public function testSudoModeActivatesOnLogin() // Sudo mode should not be enabled automagically when nobody is logged in $this->assertFalse($service->check($session)); - // Ensure sudo mode is activated on login + // Ensure sudo mode is not automatically activated on login $this->logInWithPermission(); - $this->assertTrue($service->check($session)); + $this->assertFalse($service->check($session)); // Ensure sudo mode is not active after logging out $this->logOut();