diff --git a/src/Control/Middleware/AllowedHostsMiddleware.php b/src/Control/Middleware/AllowedHostsMiddleware.php index 8a7df32f4c7..7915dc8b139 100644 --- a/src/Control/Middleware/AllowedHostsMiddleware.php +++ b/src/Control/Middleware/AllowedHostsMiddleware.php @@ -2,6 +2,7 @@ namespace SilverStripe\Control\Middleware; +use InvalidArgumentException; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; @@ -12,14 +13,16 @@ class AllowedHostsMiddleware implements HTTPMiddleware { /** - * List of allowed hosts + * List of allowed hosts. + * Can be ['*'] to allow all hosts and disable the logged warning. * * @var array */ private $allowedHosts = []; /** - * @return array List of allowed Host header values + * @return array List of allowed Host header values. + * Note that both an empty array and ['*'] can be used to allow all hosts. */ public function getAllowedHosts() { @@ -30,14 +33,21 @@ public function getAllowedHosts() * Sets the list of allowed Host header values * Can also specify a comma separated list * + * Note that both an empty array and ['*'] can be used to allow all hosts. + * * @param array|string $allowedHosts * @return $this */ public function setAllowedHosts($allowedHosts) { - if (is_string($allowedHosts)) { + if ($allowedHosts === null) { + $allowedHosts = []; + } elseif (is_string($allowedHosts)) { $allowedHosts = preg_split('/ *, */', $allowedHosts ?? ''); } + if (count($allowedHosts) > 1 && in_array('*', $allowedHosts)) { + throw new InvalidArgumentException('The wildcard "*" cannot be used in conjunction with actual hosts.'); + } $this->allowedHosts = $allowedHosts; return $this; } @@ -51,6 +61,7 @@ public function process(HTTPRequest $request, callable $delegate) // check allowed hosts if ($allowedHosts + && $allowedHosts !== ['*'] && !Director::is_cli() && !in_array($request->getHeader('Host'), $allowedHosts ?? []) ) { diff --git a/src/Core/BaseKernel.php b/src/Core/BaseKernel.php index d3c0201b9f5..2c4cee151dd 100644 --- a/src/Core/BaseKernel.php +++ b/src/Core/BaseKernel.php @@ -10,6 +10,7 @@ use SilverStripe\Control\Director; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; +use SilverStripe\Control\Middleware\AllowedHostsMiddleware; use SilverStripe\Core\Cache\ManifestCacheFactory; use SilverStripe\Core\Config\ConfigLoader; use SilverStripe\Core\Config\CoreConfigFactory; @@ -221,6 +222,9 @@ protected function bootConfigs() { // After loading all other app manifests, include _config.php files $this->getModuleLoader()->getManifest()->activateConfig(); + + // Ensure everything is set up correctly + $this->validateConfiguration(); } /** @@ -362,6 +366,7 @@ public function activate() $this->getInjectorLoader() ->getManifest() ->registerService($this, Kernel::class); + return $this; } @@ -443,4 +448,27 @@ public function setThemeResourceLoader($themeResourceLoader) $this->themeResourceLoader = $themeResourceLoader; return $this; } + + /** + * Validate configuration of the application is in a good state, ready for use. + * + * This method can be used to warn developers of any misconfiguration, or configuration + * which is missing but should be set according to best practice. + * + * In some cases, this could be used to halt execution if configuration critical to operation + * has not been set. + */ + protected function validateConfiguration(): void + { + // Log a warning if allowed hosts hasn't been configured. + // This can include wildcard, but it must be explicitly set to ensure the developer is aware + // of the level of protection their application has against host header injection attacks. + $allowedHostsMiddleware = Injector::inst()->get(AllowedHostsMiddleware::class, true); + if (empty($allowedHostsMiddleware->getAllowedHosts())) { + Injector::inst()->get(LoggerInterface::class)->warning( + 'Allowed hosts has not been set. Your application could be vulnerable to host header injection attacks.' + . ' Either set the SS_ALLOWED_HOSTS environment variable or the AllowedHosts property on ' . AllowedHostsMiddleware::class + ); + } + } } diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 0fd406b567c..9dfe2aed586 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; @@ -22,6 +23,8 @@ use SilverStripe\View\SSViewer; use SilverStripe\Model\ModelData; use SilverStripe\Forms\Validation\Validator; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; +use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest; /** * Base class for all forms. @@ -320,6 +323,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 1e69f9e71ee..fefd52d14fd 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -16,7 +16,6 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormField; -use SilverStripe\Forms\GridField\FormAction\SessionStore; use SilverStripe\Forms\GridField\FormAction\StateStore; use SilverStripe\Model\List\ArrayList; use SilverStripe\ORM\DataList; @@ -25,6 +24,9 @@ use SilverStripe\Model\List\SS_List; use SilverStripe\View\HTML; use SilverStripe\Model\ModelData; +use SilverStripe\Security\SudoMode\SudoModeServiceInterface; +use SilverStripe\ORM\DataObject; +use SilverStripe\Forms\GridField\GridFieldViewButton; /** * Displays a {@link SS_List} in a grid format. @@ -526,6 +528,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(); @@ -557,6 +584,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 1ff678b2074..4accd756207 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -323,7 +323,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 58a9ad2d4f0..744ea293a47 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/Model/ModelData.php b/src/Model/ModelData.php index f99e4b9d78b..b9af69ccaca 100644 --- a/src/Model/ModelData.php +++ b/src/Model/ModelData.php @@ -65,6 +65,13 @@ class ModelData */ 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; + // ----------------------------------------------------------------------------------------------------------------- /** @@ -218,6 +225,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 ModelData (not a subclass) diff --git a/src/Security/Group.php b/src/Security/Group.php index 6b5a59f1b6a..c420e07a8ab 100755 --- a/src/Security/Group.php +++ b/src/Security/Group.php @@ -96,6 +96,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 8276a526ef4..87cdfff9612 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -115,6 +115,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 8d8e5ff69af..be10b4d5ef5 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 1c5d54b9163..235a850357e 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -102,6 +102,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 ccfc02fcaf7..b2dec04d7fc 100644 --- a/src/Security/PermissionRole.php +++ b/src/Security/PermissionRole.php @@ -48,6 +48,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 2383e926838..7f9a0c4df99 100644 --- a/src/Security/PermissionRoleCode.php +++ b/src/Security/PermissionRoleCode.php @@ -26,6 +26,8 @@ class PermissionRoleCode extends DataObject private static $table_name = "PermissionRoleCode"; private static bool $must_use_primary_db = true; + + 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/tests/php/Control/Middleware/AllowedHostsMiddlewareTest.php b/tests/php/Control/Middleware/AllowedHostsMiddlewareTest.php new file mode 100644 index 00000000000..8da267c64af --- /dev/null +++ b/tests/php/Control/Middleware/AllowedHostsMiddlewareTest.php @@ -0,0 +1,119 @@ + [ + 'allowedHosts' => [], + 'isCli' => true, + 'allowed' => true, + ], + 'cli ignores config' => [ + 'allowedHosts' => ['example.org'], + 'isCli' => true, + 'allowed' => true, + ], + 'HTTP allow all' => [ + 'allowedHosts' => [], + 'isCli' => false, + 'allowed' => true, + ], + 'HTTP allow all explicit' => [ + 'allowedHosts' => ['*'], + 'isCli' => false, + 'allowed' => true, + ], + 'HTTP allow explicit host' => [ + 'allowedHosts' => ['www.example.com'], + 'isCli' => false, + 'allowed' => true, + ], + 'HTTP allow explicit host multiple values' => [ + 'allowedHosts' => ['example.com', 'example.org', 'ww.example.org', 'www.example.com'], + 'isCli' => false, + 'allowed' => true, + ], + 'HTTP allow explicit host string' => [ + 'allowedHosts' => 'example.com,example.org,ww.example.org,www.example.com', + 'isCli' => false, + 'allowed' => true, + ], + 'HTTP host mismatch (missing subdomain)' => [ + 'allowedHosts' => ['example.com'], + 'isCli' => false, + 'allowed' => false, + ], + 'HTTP host mismatch (different tld)' => [ + 'allowedHosts' => ['example.org'], + 'isCli' => false, + 'allowed' => false, + ], + 'HTTP host mismatch multiple' => [ + 'allowedHosts' => ['example.org', 'www.example.org', 'example.com'], + 'isCli' => false, + 'allowed' => false, + ], + 'HTTP host mismatch string' => [ + 'allowedHosts' => 'example.org,www.example.org,example.com', + 'isCli' => false, + 'allowed' => false, + ], + ]; + } + + /** + * @dataProvider provideProcess + */ + public function testProcess(string|array $allowedHosts, bool $isCli, bool $allowed): void + { + $reflectionEnvironment = new ReflectionClass(Environment::class); + $origIsCli = $reflectionEnvironment->getStaticPropertyValue('isCliOverride'); + $reflectionEnvironment->setStaticPropertyValue('isCliOverride', $isCli); + + try { + $middleware = new AllowedHostsMiddleware(); + $middleware->setAllowedHosts($allowedHosts); + $request = new HTTPRequest('GET', '/'); + $request->addHeader('host', 'www.example.com'); + $defaultResponse = new HTTPResponse(); + + $result = $middleware->process($request, function () use ($defaultResponse) { + return $defaultResponse; + }); + + if ($allowed) { + $this->assertSame(200, $result->getStatusCode()); + $this->assertSame($defaultResponse, $result); + } else { + $this->assertSame(400, $result->getStatusCode()); + $this->assertNotSame($defaultResponse, $result); + } + } finally { + $reflectionEnvironment->setStaticPropertyValue('isCliOverride', $origIsCli); + } + } + + public function testProcessInvalidConfig(): void + { + $middleware = new AllowedHostsMiddleware(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The wildcard "*" cannot be used in conjunction with actual hosts.'); + + $middleware->setAllowedHosts(['*', 'www.example.com']); + } +} diff --git a/tests/php/Core/KernelTest.php b/tests/php/Core/KernelTest.php index afb5b37a7a7..2ad01bdca97 100644 --- a/tests/php/Core/KernelTest.php +++ b/tests/php/Core/KernelTest.php @@ -3,16 +3,20 @@ namespace SilverStripe\Core\Tests; use BadMethodCallException; +use Exception; +use Monolog\Logger; +use Psr\Log\LoggerInterface; +use ReflectionClass; use SilverStripe\Control\Director; +use SilverStripe\Control\Middleware\AllowedHostsMiddleware; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\ConfigLoader; use SilverStripe\Core\CoreKernel; +use SilverStripe\Core\Environment; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Core\Kernel; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Core\Environment; -use ReflectionClass; use SilverStripe\ORM\DB; use ReflectionObject; use SilverStripe\Core\Tests\KernelTest\TestFlushable; @@ -126,4 +130,59 @@ public function testImplementorsAreCalled() // reset the kernel Flush flag $kernel->boot(); } + + public function provideAllowedHostsWarning(): array + { + $scenarios = [ + [ + 'config' => [], + 'isCli' => true, + 'shouldLog' => true, + ], + [ + 'config' => ['*'], + 'isCli' => true, + 'shouldLog' => false, + ], + [ + 'config' => ['www.example.com', 'example.org'], + 'isCli' => true, + 'shouldLog' => false, + ], + ]; + // Test both in CLI and non-CLI context + foreach ($scenarios as $name => $scenario) { + $scenario['isCli'] = false; + $scenarios[$name . ' (non-CLI)'] = $scenario; + } + return $scenarios; + } + + /** + * @dataProvider provideAllowedHostsWarning + */ + public function testAllowedHostsWarning(array $config, bool $isCli, bool $shouldLog): void + { + // Prepare mock to check if a warning is logged or not + $mockLogger = $this->getMockBuilder(Logger::class)->setConstructorArgs(['testLogger'])->getMock(); + $expectLog = $shouldLog ? $this->once() : $this->never(); + $mockLogger->expects($expectLog)->method('warning'); + Injector::inst()->registerService($mockLogger, LoggerInterface::class); + + // Set the config in our middleware + $middleware = Injector::inst()->get(AllowedHostsMiddleware::class, true); + $middleware->setAllowedHosts($config); + + $reflectionEnvironment = new ReflectionClass(Environment::class); + $origIsCli = $reflectionEnvironment->getStaticPropertyValue('isCliOverride'); + $reflectionEnvironment->setStaticPropertyValue('isCliOverride', $isCli); + + try { + $kernel = Injector::inst()->get(Kernel::class); + $kernel->nest(); // $kernel is no longer current kernel + $kernel->boot(); + } finally { + $reflectionEnvironment->setStaticPropertyValue('isCliOverride', $origIsCli); + } + } } diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index e66e48e789a..c9bd79f4fdc 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -35,6 +35,13 @@ use SilverStripe\View\SSViewer; use PHPUnit\Framework\Attributes\DataProvider; use SilverStripe\Forms\EmailField; +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 { @@ -1258,6 +1265,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();