From ae6c47bf20be9a5e8360526ad8d967343580bf96 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 15 Oct 2024 12:04:46 +1300 Subject: [PATCH] NEW AdminController as superclass for /admin/* routed controllers --- code/AdminController.php | 359 ++++++++++ code/AdminRootController.php | 69 +- code/LeftAndMain.php | 619 ++++-------------- code/SudoModeController.php | 8 +- lang/en.yml | 8 + tests/php/AdminControllerTest.php | 236 +++++++ ...ndMainTest.yml => AdminControllerTest.yml} | 21 + .../DualPermissionAdminController.php | 14 + .../TestAdminController.php | 11 + tests/php/LeftAndMainTest.php | 210 +----- tests/php/SudoModeControllerTest.php | 2 +- 11 files changed, 827 insertions(+), 730 deletions(-) create mode 100644 code/AdminController.php create mode 100644 tests/php/AdminControllerTest.php rename tests/php/{LeftAndMainTest.yml => AdminControllerTest.yml} (62%) create mode 100644 tests/php/AdminControllerTest/DualPermissionAdminController.php create mode 100644 tests/php/AdminControllerTest/TestAdminController.php diff --git a/code/AdminController.php b/code/AdminController.php new file mode 100644 index 000000000..f47dd401d --- /dev/null +++ b/code/AdminController.php @@ -0,0 +1,359 @@ +" if no permissions are defined here. + * See {@link canView()} for more details on permission checks. + */ + private static string|array $required_permission_codes = []; + + /** + * The configuration passed to the supporting JS for each CMS section includes a 'name' key + * that by default matches the FQCN of the current class. This setting allows you to change + * the key if necessary (for example, if you are overloading CMSMain or another core class + * and want to keep the core JS - which depends on the core class names - functioning, you + * would need to set this to the FQCN of the class you are overloading). + * + * See getClientConfig() + */ + private static ?string $section_name = null; + + /** + * Get list of required permissions for accessing this controller. + * If false, no permission is required. + */ + public static function getRequiredPermissions(): array|string|false + { + if (static::class === AdminController::class) { + throw new BadMethodCallException('getRequiredPermissions should be called on a subclass'); + } + // If the user is accessing LeftAndMain directly, only generic permissions are required. + if (static::class === LeftAndMain::class) { + return 'CMS_ACCESS'; + } + $codes = static::config()->get('required_permission_codes'); + // allow explicit FALSE to disable subclass check + if ($codes === false) { + return false; + } + if ($codes) { + return $codes; + } + // Fallback if no explicit permission was declared + return 'CMS_ACCESS_' . static::class; + } + + public function canView($member = null) + { + if (!$member && $member !== false) { + $member = Security::getCurrentUser(); + } + + // don't allow unauthenticated users + if (!$member) { + return false; + } + + // alternative extended checks + if ($this->hasMethod('alternateAccessCheck')) { + $alternateAllowed = $this->alternateAccessCheck($member); + if ($alternateAllowed === false) { + return false; + } + } + + // Check for "Access to all CMS sections" permission + if (Permission::checkMember($member, 'CMS_ACCESS_LeftAndMain')) { + return true; + } + + // Check for permission to access this specific controller + $codes = static::getRequiredPermissions(); + // allow explicit FALSE to disable subclass check + if ($codes === false) { + return true; + } + foreach ((array) $codes as $code) { + if (!Permission::checkMember($member, $code)) { + return false; + } + } + + return true; + } + + public function Link($action = null) + { + // LeftAndMain methods have a top-level uri access + if (static::class === LeftAndMain::class) { + $segment = ''; + } else { + // Get url_segment + $segment = static::config()->get('url_segment'); + if (!$segment) { + throw new BadMethodCallException( + sprintf('AdminController subclasses (%s) must have url_segment', static::class) + ); + } + } + + $link = Controller::join_links( + AdminRootController::admin_url(), + $segment, + "$action" + ); + $this->extend('updateLink', $link); + return $link; + } + + /** + * Overloaded redirection logic to trigger a fake redirect on ajax requests. + * While this violates HTTP principles, its the only way to work around the + * fact that browsers handle HTTP redirects opaquely, no intervention via JS is possible. + * In isolation, that's not a problem - but combined with history.pushState() + * it means we would request the same redirection URL twice if we want to update the URL as well. + * See LeftAndMain.js for the required jQuery ajaxComplete handlers. + */ + public function redirect(string $url, int $code = 302): HTTPResponse + { + if ($this->getRequest()->isAjax()) { + $response = $this->getResponse(); + $response->addHeader('X-ControllerURL', $url); + if ($this->getRequest()->getHeader('X-Pjax') && !$response->getHeader('X-Pjax')) { + $response->addHeader('X-Pjax', $this->getRequest()->getHeader('X-Pjax')); + } + $newResponse = new LeftAndMain_HTTPResponse( + $response->getBody(), + $response->getStatusCode(), + $response->getStatusDescription() + ); + foreach ($response->getHeaders() as $k => $v) { + $newResponse->addHeader($k, $v); + } + $newResponse->setIsFinished(true); + $this->setResponse($newResponse); + // Actual response will be re-requested by client + return $newResponse; + } else { + return parent::redirect($url, $code); + } + } + + /** + * Returns configuration required by the client app + */ + public function getClientConfig(): array + { + // Allows the section name to be overridden in config + $name = static::config()->get('section_name') ?: static::class; + // Trim leading/trailing slash to make it easier to concatenate URL + // and use in routing definitions. + $url = trim($this->Link(), '/'); + $clientConfig = [ + 'name' => $name, + 'url' => $url, + 'reactRoutePath' => preg_replace('/^' . preg_quote(AdminRootController::admin_url(), '/') . '/', '', $url), + ]; + $this->extend('updateClientConfig', $clientConfig); + return $clientConfig; + } + + protected function init() + { + parent::init(); + + HTTPCacheControlMiddleware::singleton()->disableCache(); + + SSViewer::setRewriteHashLinksDefault(false); + ContentNegotiator::setEnabled(false); + + // set language + $member = Security::getCurrentUser(); + if (!empty($member->Locale)) { + i18n::set_locale($member->Locale); + } + + // Don't allow access if the request hasn't finished being handled and the user can't access this controller + if (!$this->canView() && !$this->getResponse()->isFinished()) { + // Allow subclasses and extensions to redirect somewhere more appropriate + $this->invokeWithExtensions('onInitPermissionFailure'); + + // If we're redirecting away, just let that happen + if ($this->getResponse()->isRedirect()) { + return; + } + + if (Security::getCurrentUser()) { + $this->getRequest()->getSession()->clear("BackURL"); + } + + // if no alternate menu items have matched, return a permission error + $messageSet = [ + 'default' => _t( + __CLASS__ . '.PERMDEFAULT', + "You must be logged in to access the administration area." + ), + 'alreadyLoggedIn' => _t( + __CLASS__ . '.PERMALREADY', + "I'm sorry, but you can't access that part of the CMS." + ), + 'logInAgain' => _t( + __CLASS__ . '.PERMAGAIN', + "You have been logged out of the CMS." + ), + ]; + + $this->suppressAdminErrorContext = true; + Security::permissionFailure($this, $messageSet); + return; + } + + // Don't continue if there's already been a redirection request. + if ($this->getResponse()->isRedirect()) { + return; + } + + $this->extend('onInit'); + + // Load the editor with original user themes before overwriting + // them with admin themes + $themes = HTMLEditorConfig::getThemes(); + if (empty($themes)) { + HTMLEditorConfig::setThemes(SSViewer::get_themes()); + } + + // Assign default cms theme and replace user-specified themes + // This ensures any templates rendered use appropriate templates and resources + // instead of the front-end ones + SSViewer::set_themes(LeftAndMain::config()->uninherited('admin_themes')); + + // Set the current reading mode + Versioned::set_stage(Versioned::DRAFT); + + // Set default reading mode to suppress ?stage=Stage querystring params in CMS + Versioned::set_default_reading_mode(Versioned::get_reading_mode()); + } + + /** + * Get a data value from JSON in body of the POST request, ensuring it exists + * Will only read from the root node of the JSON body + */ + protected function getPostedJsonValue(HTTPRequest $request, string $key): mixed + { + $data = json_decode($request->getBody(), true); + if (!array_key_exists($key, $data)) { + $this->jsonError(400); + } + return $data[$key]; + } + + /** + * Creates a successful json response + */ + protected function jsonSuccess(int $statusCode, ?array $data = null): HTTPResponse + { + if ($statusCode < 200 || $statusCode >= 300) { + throw new InvalidArgumentException("Status code $statusCode must be between 200 and 299"); + } + if (is_null($data)) { + $body = ''; + } else { + $body = json_encode($data, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + } + return $this->getResponse() + ->addHeader('Content-Type', 'application/json') + ->setStatusCode($statusCode) + ->setBody($body); + } + + /** + * Throw an error HTTPResponse encoded as json + * + * @throws HTTPResponse_Exception which interrupts request handling with the appropriate response + */ + protected function jsonError(int $errorCode, string $errorMessage = ''): void + { + // Build error from message + $error = [ + 'type' => 'error', + 'code' => $errorCode, + ]; + if ($errorMessage) { + $error['value'] = $errorMessage; + } else { + $messageDefault = match ($errorCode) { + 400 => 'Sorry, it seems there was something wrong with the request.', + 401 => 'Sorry, it seems you are not authorised to access this section or object.', + 403 => 'Sorry, it seems the action you were trying to perform is forbidden.', + 404 => 'Sorry, it seems you were trying to access a section or object that doesn\'t exist.', + 500 => 'Sorry, it seems there was an internal server error.', + 503 => 'Sorry, it seems the service is temporarily unavailable.', + default => 'Error', + }; + /** @phpstan-ignore translation.key (we need the key to be dynamic here) */ + $error['value'] = _t(__CLASS__ . ".ErrorMessage{$errorCode}", $messageDefault); + } + + // Support explicit error handling with status = error, or generic message handling + // with a message of type = error + $result = [ + 'status' => 'error', + 'errors' => [$error] + ]; + $response = HTTPResponse::create(json_encode($result), $errorCode) + ->addHeader('Content-Type', 'application/json'); + + // Call a handler method such as onBeforeHTTPError404 + $this->extend("onBeforeJSONError{$errorCode}", $request, $response); + + // Call a handler method such as onBeforeHTTPError, passing 404 as the first arg + $this->extend('onBeforeJSONError', $errorCode, $request, $response); + + // Throw a new exception + throw new HTTPResponse_Exception($response); + } +} diff --git a/code/AdminRootController.php b/code/AdminRootController.php index 32fade435..0a5a7c6f0 100644 --- a/code/AdminRootController.php +++ b/code/AdminRootController.php @@ -10,24 +10,41 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\View\TemplateGlobalProvider; +/** + * Controller in charge of managing AdminController routing. + * + * Adds AdminController classes to the Director routing rules and redirects to an + * appropriate controller if a user navigates directly to /admin + */ class AdminRootController extends Controller implements TemplateGlobalProvider { - /** * Fallback admin URL in case this cannot be infered from Director.rules + */ + private static string $url_base = 'admin'; + + /** + * The LeftAndMain child that will be used as the initial panel to display if none is selected (i.e. if you + * visit /admin) + */ + private static string $default_panel = SecurityAdmin::class; + + /** + * Holds an array of url_pattern => controller k/v pairs, the same as Director::rules. However this is built + * dynamically from introspecting on all the classes that derive from LeftAndMain. + * + * Don't access this directly - always access via the rules() accessor, which will build this array + * the first time it's accessed * - * @var string - * @config + * @internal */ - private static $url_base = 'admin'; + private static ?array $adminRules = null; /** * Convenience function to return the admin route config. * Looks for the {@link Director::$rules} for the current admin Controller. - * - * @return string */ - public static function get_admin_route() + public static function get_admin_route(): string { $rules = Director::config()->get('rules'); $adminRoute = array_search(__CLASS__, $rules ?? []); @@ -36,44 +53,22 @@ public static function get_admin_route() /** * Returns the root admin URL for the site with trailing slash - * - * @return string */ - public static function admin_url(string $action = '') + public static function admin_url(string $action = ''): string { return Controller::join_links(AdminRootController::get_admin_route(), $action); } - /** - * @var string - * @config - * The LeftAndMain child that will be used as the initial panel to display if none is selected (i.e. if you - * visit /admin) - */ - private static $default_panel = SecurityAdmin::class; - - /** - * @var array - * @internal - * - * Holds an array of url_pattern => controller k/v pairs, the same as Director::rules. However this is built - * dynamically from introspecting on all the classes that derive from LeftAndMain. - * - * Don't access this directly - always access via the rules() accessor below, which will build this array - * the first time it's accessed - */ - private static $adminRules = null; - /** * Gets a list of url_pattern => controller k/v pairs for each LeftAndMain derived controller */ - public static function rules() + public static function rules(): array { if (AdminRootController::$adminRules === null) { AdminRootController::$adminRules = []; // Map over the array calling add_rule_for_controller on each - $classes = CMSMenu::get_cms_classes(null, true, CMSMenu::URL_PRIORITY); + $classes = CMSMenu::get_cms_classes(AdminController::class, true, CMSMenu::URL_PRIORITY); array_map([__CLASS__, 'add_rule_for_controller'], $classes ?? []); } return AdminRootController::$adminRules; @@ -81,10 +76,8 @@ public static function rules() /** * Add the appropriate k/v pair to AdminRootController::$rules for the given controller. - * - * @param string $controllerClass Name of class */ - protected static function add_rule_for_controller($controllerClass) + protected static function add_rule_for_controller(string $controllerClass): void { $config = Config::forClass($controllerClass); $urlSegment = $config->get('url_segment'); @@ -118,7 +111,7 @@ public function handleRequest(HTTPRequest $request): HTTPResponse // Otherwise $rules = AdminRootController::rules(); foreach ($rules as $pattern => $controller) { - if (($arguments = $request->match($pattern, true)) !== false) { + if ($request->match($pattern, true) !== false) { /** @var LeftAndMain $controllerObj */ $controllerObj = Injector::inst()->create($controller); return $controllerObj->handleRequest($request); @@ -131,10 +124,10 @@ public function handleRequest(HTTPRequest $request): HTTPResponse } /** - * @return array Returns an array of strings of the method names of methods on the call that should be exposed + * Returns an array of strings of the method names of methods on the call that should be exposed * as global variables in the templates. */ - public static function get_template_global_variables() + public static function get_template_global_variables(): array { return [ 'adminURL' => 'admin_url' diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index 9eb920f7b..a55ea69de 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -2,19 +2,16 @@ namespace SilverStripe\Admin; -use BadMethodCallException; use InvalidArgumentException; use LogicException; use ReflectionClass; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\Admin\Navigator\SilverStripeNavigator; -use SilverStripe\Control\ContentNegotiator; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Control\PjaxResponseNegotiator; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; @@ -43,13 +40,11 @@ use SilverStripe\Model\List\SS_List; use SilverStripe\Core\Validation\ValidationException; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\PermissionProvider; use SilverStripe\Security\Security; use SilverStripe\Security\SecurityToken; use SilverStripe\SiteConfig\SiteConfig; -use SilverStripe\Versioned\Versioned; use SilverStripe\Model\ArrayData; use SilverStripe\View\Requirements; use SilverStripe\View\SSViewer; @@ -61,7 +56,7 @@ * This is essentially an abstract class which should be subclassed. * See {@link CMSMain} for a good example. */ -class LeftAndMain extends Controller implements PermissionProvider +class LeftAndMain extends AdminController implements PermissionProvider { /** @@ -77,20 +72,6 @@ class LeftAndMain extends Controller implements PermissionProvider */ private static $client_debugging = true; - /** - * The current url segment attached to the LeftAndMain instance - * - * @config - * @var string - */ - private static $url_segment = null; - - /** - * @config - * @var string Used by {@link AdminRootController} to augment Director route rules for sub-classes of LeftAndMain - */ - private static $url_rule = '/$Action/$ID/$OtherID'; - /** * @config * @var string @@ -109,12 +90,6 @@ class LeftAndMain extends Controller implements PermissionProvider */ private static $menu_priority = 0; - /** - * @config - * @var int - */ - private static $url_priority = 50; - /** * A subclass of {@link DataObject}. * @@ -187,18 +162,6 @@ class LeftAndMain extends Controller implements PermissionProvider */ private static $admin_themes = []; - /** - * 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. - * - * @config - * @var array - */ - private static $required_permission_codes; - /** * Namespace for session info, e.g. current record. * Defaults to the current class name, but can be amended to share a namespace in case @@ -281,16 +244,35 @@ class LeftAndMain extends Controller implements PermissionProvider private static $frame_options = 'SAMEORIGIN'; /** - * The configuration passed to the supporting JS for each CMS section includes a 'name' key - * that by default matches the FQCN of the current class. This setting allows you to change - * the key if necessary (for example, if you are overloading CMSMain or another core class - * and want to keep the core JS - which depends on the core class names - functioning, you - * would need to set this to the FQCN of the class you are overloading). + * The urls used for the links in the Help dropdown in the backend + * + * Set this to `false` via yml config if you do not want to show any help links * * @config - * @var string|null + * @var array */ - private static $section_name = null; + private static $help_links = [ + 'CMS User help' => 'https://userhelp.silverstripe.org/en/5', + 'Developer docs' => 'https://docs.silverstripe.org/en/5/', + 'Community' => 'https://www.silverstripe.org/', + 'Feedback' => 'https://www.silverstripe.org/give-feedback/', + ]; + + /** + * The href for the anchor on the Silverstripe logo + * + * @config + * @var string + */ + private static $application_link = '//www.silverstripe.org/'; + + /** + * The application name + * + * @config + * @var string + */ + private static $application_name = 'Silverstripe'; /** * @var PjaxResponseNegotiator @@ -302,6 +284,13 @@ class LeftAndMain extends Controller implements PermissionProvider */ protected $versionProvider; + /** + * Cached search filter instance for current search + * + * @var LeftAndMain_SearchFilter + */ + protected $searchFilterCache = null; + /** * Gets the combined configuration of all LeftAndMain subclasses required by the client app. * @@ -312,7 +301,7 @@ class LeftAndMain extends Controller implements PermissionProvider public function getCombinedClientConfig() { $combinedClientConfig = ['sections' => []]; - $cmsClassNames = CMSMenu::get_cms_classes(LeftAndMain::class, true, CMSMenu::URL_PRIORITY); + $cmsClassNames = CMSMenu::get_cms_classes(AdminController::class, true, CMSMenu::URL_PRIORITY); // append LeftAndMain to the list as well $cmsClassNames[] = LeftAndMain::class; @@ -336,40 +325,20 @@ public function getCombinedClientConfig() return json_encode($combinedClientConfig); } - /** - * Returns configuration required by the client app - * - * @return array - */ - public function getClientConfig() + public function getClientConfig(): array { - // Allows the section name to be overridden in config - $name = $this->config()->get('section_name'); - $url = trim($this->Link() ?? '', '/'); - - if (!$name) { - $name = static::class; - } - - $clientConfig = [ - // Trim leading/trailing slash to make it easier to concatenate URL - // and use in routing definitions. - 'name' => $name, - 'url' => $url, - 'reactRoutePath' => preg_replace('/^' . preg_quote(AdminRootController::admin_url(), '/') . '/', '', $url), - 'form' => [ + // Add WYSIWYG link form schema before extensions are applied + $this->beforeExtending('updateClientConfig', function (array &$clientConfig): void { + $clientConfig['form'] = [ 'EditorExternalLink' => [ 'schemaUrl' => $this->Link('methodSchema/Modals/EditorExternalLink'), ], 'EditorEmailLink' => [ 'schemaUrl' => $this->Link('methodSchema/Modals/EditorEmailLink'), ], - ], - ]; - - $this->extend('updateClientConfig', $clientConfig); - - return $clientConfig; + ]; + }); + return parent::getClientConfig(); } /** @@ -425,87 +394,9 @@ public function schema(HTTPRequest $request): HTTPResponse } /** - * Get a data value from JSON in body of the POST request, ensuring it exists - * Will only read from the root node of the JSON body - */ - protected function getPostedJsonValue(HTTPRequest $request, string $key): mixed - { - $data = json_decode($request->getBody(), true); - if (!array_key_exists($key, $data)) { - $this->jsonError(400); - } - return $data[$key]; - } - - /** - * Creates a successful json response - */ - protected function jsonSuccess(int $statusCode, ?array $data = null): HTTPResponse - { - if ($statusCode < 200 || $statusCode >= 300) { - throw new InvalidArgumentException("Status code $statusCode must be between 200 and 299"); - } - if (is_null($data)) { - $body = ''; - } else { - $body = json_encode($data, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); - } - return $this->getResponse() - ->addHeader('Content-Type', 'application/json') - ->setStatusCode($statusCode) - ->setBody($body); - } - - /** - * Return an error HTTPResponse encoded as json - * - * @param int $errorCode - * @param string $errorMessage - * @return HTTPResponse - * @throws HTTPResponse_Exception + * Get the form schema from a given method. + * The method must return a Form. */ - public function jsonError($errorCode, $errorMessage = null) - { - // Build error from message - $error = [ - 'type' => 'error', - 'code' => $errorCode, - ]; - if ($errorMessage) { - $error['value'] = $errorMessage; - } else { - $messageDefault = match ($errorCode) { - 400 => 'Sorry, it seems there was something wrong with the request.', - 401 => 'Sorry, it seems you are not authorised to access this section or object.', - 403 => 'Sorry, it seems the action you were trying to perform is forbidden.', - 404 => 'Sorry, it seems you were trying to access a section or object that doesn\'t exist.', - 500 => 'Sorry, it seems there was an internal server error.', - 503 => 'Sorry, it seems the service is temporarily unavailable.', - default => 'Error', - }; - /** @phpstan-ignore translation.key (we need the key to be dynamic here) */ - $error['value'] = _t(__CLASS__ . ".ErrorMessage{$errorCode}", $messageDefault); - } - - // Support explicit error handling with status = error, or generic message handling - // with a message of type = error - $result = [ - 'status' => 'error', - 'errors' => [$error] - ]; - $response = HTTPResponse::create(json_encode($result), $errorCode) - ->addHeader('Content-Type', 'application/json'); - - // Call a handler method such as onBeforeHTTPError404 - $this->extend("onBeforeJSONError{$errorCode}", $request, $response); - - // Call a handler method such as onBeforeHTTPError, passing 404 as the first arg - $this->extend('onBeforeJSONError', $errorCode, $request, $response); - - // Throw a new exception - throw new HTTPResponse_Exception($response); - } - public function methodSchema(HTTPRequest $request): HTTPResponse { $method = $request->param('Method'); @@ -574,258 +465,136 @@ protected function getSchemaResponse($schemaID, $form = null, ValidationResult $ } /** - * @param Member $member - * @return bool + * Try to redirect to an appropriate admin section if we can't access this one */ - public function canView($member = null) + public function onInitPermissionFailure(): void { - if (!$member && $member !== false) { - $member = Security::getCurrentUser(); - } - - // cms menus only for logged-in members - if (!$member) { - return false; - } - - // alternative extended checks - if ($this->hasMethod('alternateAccessCheck')) { - $alternateAllowed = $this->alternateAccessCheck($member); - if ($alternateAllowed === false) { - return false; - } - } - - // Check for "CMS admin" permission - if (Permission::checkMember($member, "CMS_ACCESS_LeftAndMain")) { - return true; - } - - // Check for LeftAndMain sub-class permissions - $codes = $this->getRequiredPermissions(); - if ($codes === false) { // allow explicit FALSE to disable subclass check - return true; - } - foreach ((array)$codes as $code) { - if (!Permission::checkMember($member, $code)) { - return false; + $menu = $this->MainMenu(); + foreach ($menu as $candidate) { + if ($candidate->Link && + $candidate->Link != $this->Link() + && $candidate->MenuItem->controller + && singleton($candidate->MenuItem->controller)->canView() + ) { + $this->redirect($candidate->Link); + return; } } - - return true; - } - - /** - * Get list of required permissions - * - * @return array|string|bool Code, array of codes, or false if no permission required - */ - public static function getRequiredPermissions() - { - $class = get_called_class(); - // If the user is accessing LeftAndMain directly, only generic permissions are required. - if ($class === LeftAndMain::class) { - return 'CMS_ACCESS'; - } - $code = Config::inst()->get($class, 'required_permission_codes'); - if ($code === false) { - return false; - } - if ($code) { - return $code; - } - return 'CMS_ACCESS_' . $class; + $this->suppressAdminErrorContext = true; } - /** - * @uses CMSMenu - */ protected function init() { - parent::init(); - - HTTPCacheControlMiddleware::singleton()->disableCache(); - - SSViewer::setRewriteHashLinksDefault(false); - ContentNegotiator::setEnabled(false); - - // set language - $member = Security::getCurrentUser(); - if (!empty($member->Locale)) { - i18n::set_locale($member->Locale); - } - - // Allow customisation of the access check by a extension - // Also all the canView() check to execute Controller::redirect() - if (!$this->canView() && !$this->getResponse()->isFinished()) { - // 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() - ) { - $this->redirect($candidate->Link); - return; - } + $this->beforeExtending('onInit', function () { + // Audit logging hook + if (empty($_REQUEST['executeForm']) && !$this->getRequest()->isAjax()) { + $this->extend('accessedCMS'); } + // Set the members html editor config if (Security::getCurrentUser()) { - $this->getRequest()->getSession()->clear("BackURL"); + HTMLEditorConfig::set_active_identifier(Security::getCurrentUser()->getHtmlEditorConfigForCMS()); } - // if no alternate menu items have matched, return a permission error - $messageSet = [ - 'default' => _t( - __CLASS__ . '.PERMDEFAULT', - "You must be logged in to access the administration area; please enter your credentials below." - ), - 'alreadyLoggedIn' => _t( - __CLASS__ . '.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( - __CLASS__ . '.PERMAGAIN', - "You have been logged out of the CMS. If you would like to log in again, enter a username and" - . " password below." - ), - ]; - - $this->suppressAdminErrorContext = true; - Security::permissionFailure($this, $messageSet); - return; - } - - // Don't continue if there's already been a redirection request. - if ($this->redirectedTo()) { - return; - } - - // Audit logging hook - if (empty($_REQUEST['executeForm']) && !$this->getRequest()->isAjax()) { - $this->extend('accessedCMS'); - } - - // Set the members html editor config - if (Security::getCurrentUser()) { - HTMLEditorConfig::set_active_identifier(Security::getCurrentUser()->getHtmlEditorConfigForCMS()); - } - - // Set default values in the config if missing. These things can't be defined in the config - // file because insufficient information exists when that is being processed - $htmlEditorConfig = HTMLEditorConfig::get_active(); - $htmlEditorConfig->setOption('language', TinyMCEConfig::get_tinymce_lang()); - $langUrl = TinyMCEConfig::get_tinymce_lang_url(); - if ($langUrl) { - $htmlEditorConfig->setOption('language_url', $langUrl); - } - - Requirements::customScript(" - window.ss = window.ss || {}; - window.ss.config = " . $this->getCombinedClientConfig() . "; - "); + // Set default values in the config if missing. These things can't be defined in the config + // file because insufficient information exists when that is being processed + $htmlEditorConfig = HTMLEditorConfig::get_active(); + $htmlEditorConfig->setOption('language', TinyMCEConfig::get_tinymce_lang()); + $langUrl = TinyMCEConfig::get_tinymce_lang_url(); + if ($langUrl) { + $htmlEditorConfig->setOption('language_url', $langUrl); + } - Requirements::javascript('silverstripe/admin: client/dist/js/vendor.js'); - Requirements::javascript('silverstripe/admin: client/dist/js/bundle.js'); + Requirements::customScript(" + window.ss = window.ss || {}; + window.ss.config = " . $this->getCombinedClientConfig() . "; + "); + + Requirements::javascript('silverstripe/admin: client/dist/js/vendor.js'); + Requirements::javascript('silverstripe/admin: client/dist/js/bundle.js'); + + // Bootstrap components + Requirements::javascript('silverstripe/admin: thirdparty/popper/popper.min.js'); + Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/util.js'); + Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/collapse.js'); + Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/tooltip.js'); + Requirements::customScript( + "window.jQuery('body').tooltip({ selector: '[data-toggle=tooltip]' });", + 'bootstrap.tooltip-boot' + ); - // Bootstrap components - Requirements::javascript('silverstripe/admin: thirdparty/popper/popper.min.js'); - Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/util.js'); - Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/collapse.js'); - Requirements::javascript('silverstripe/admin: thirdparty/bootstrap/js/dist/tooltip.js'); - Requirements::customScript( - "window.jQuery('body').tooltip({ selector: '[data-toggle=tooltip]' });", - 'bootstrap.tooltip-boot' - ); + Requirements::css('silverstripe/admin: client/dist/styles/bundle.css'); + Requirements::add_i18n_javascript('silverstripe/admin:client/lang'); + Requirements::add_i18n_javascript('silverstripe/admin:client/dist/moment-locales', false); - Requirements::css('silverstripe/admin: client/dist/styles/bundle.css'); - Requirements::add_i18n_javascript('silverstripe/admin:client/lang'); - Requirements::add_i18n_javascript('silverstripe/admin:client/dist/moment-locales', false); + if (LeftAndMain::config()->uninherited('session_keepalive_ping')) { + Requirements::javascript('silverstripe/admin: client/dist/js/LeftAndMain.Ping.js'); + } - if (LeftAndMain::config()->uninherited('session_keepalive_ping')) { - Requirements::javascript('silverstripe/admin: client/dist/js/LeftAndMain.Ping.js'); - } + // Custom requirements + $extraJs = $this->config()->get('extra_requirements_javascript'); - // Custom requirements - $extraJs = $this->config()->get('extra_requirements_javascript'); + if ($extraJs) { + foreach ($extraJs as $file => $config) { + if (is_numeric($file)) { + $file = $config; + $config = []; + } - if ($extraJs) { - foreach ($extraJs as $file => $config) { - if (is_numeric($file)) { - $file = $config; - $config = []; + Requirements::javascript($file, $config); } - - Requirements::javascript($file, $config); } - } - $extraI18n = $this->config()->get('extra_requirements_i18n'); - if ($extraI18n) { - foreach ($extraI18n as $dir => $return) { - if (is_numeric($dir)) { - $dir = $return; - $return = false; + $extraI18n = $this->config()->get('extra_requirements_i18n'); + if ($extraI18n) { + foreach ($extraI18n as $dir => $return) { + if (is_numeric($dir)) { + $dir = $return; + $return = false; + } + Requirements::add_i18n_javascript($dir, $return); } - Requirements::add_i18n_javascript($dir, $return); } - } - $extraCss = $this->config()->get('extra_requirements_css'); - - if ($extraCss) { - foreach ($extraCss as $file => $config) { - if (is_numeric($file)) { - $file = $config; - $config = []; - } - $media = null; - if (isset($config['media'])) { - $media = $config['media']; - unset($config['media']); - } - - Requirements::css($file, $media, $config); - } - } + $extraCss = $this->config()->get('extra_requirements_css'); - $extraThemedCss = $this->config()->get('extra_requirements_themedCss'); + if ($extraCss) { + foreach ($extraCss as $file => $config) { + if (is_numeric($file)) { + $file = $config; + $config = []; + } + $media = null; + if (isset($config['media'])) { + $media = $config['media']; + unset($config['media']); + } - if ($extraThemedCss) { - foreach ($extraThemedCss as $file => $config) { - if (is_numeric($file)) { - $file = $config; - $config = []; - } - $media = null; - if (isset($config['media'])) { - $media = $config['media']; - unset($config['media']); + Requirements::css($file, $media, $config); } - - Requirements::themedCSS($file, $media, $config); } - } - - $this->extend('onInit'); - // Load the editor with original user themes before overwriting - // them with admin themes - $themes = HTMLEditorConfig::getThemes(); - if (empty($themes)) { - HTMLEditorConfig::setThemes(SSViewer::get_themes()); - } + $extraThemedCss = $this->config()->get('extra_requirements_themedCss'); - // Assign default cms theme and replace user-specified themes - SSViewer::set_themes(LeftAndMain::config()->uninherited('admin_themes')); + if ($extraThemedCss) { + foreach ($extraThemedCss as $file => $config) { + if (is_numeric($file)) { + $file = $config; + $config = []; + } + $media = null; + if (isset($config['media'])) { + $media = $config['media']; + unset($config['media']); + } - // Set the current reading mode - Versioned::set_stage(Versioned::DRAFT); + Requirements::themedCSS($file, $media, $config); + } + } + }); - // Set default reading mode to suppress ?stage=Stage querystring params in CMS - Versioned::set_default_reading_mode(Versioned::get_reading_mode()); + // Run parent init and process the onInit extension hook + parent::init(); } public function afterHandleRequest() @@ -891,39 +660,6 @@ public function handleRequest(HTTPRequest $request): HTTPResponse return $response; } - /** - * Overloaded redirection logic to trigger a fake redirect on ajax requests. - * While this violates HTTP principles, its the only way to work around the - * fact that browsers handle HTTP redirects opaquely, no intervention via JS is possible. - * In isolation, that's not a problem - but combined with history.pushState() - * it means we would request the same redirection URL twice if we want to update the URL as well. - * See LeftAndMain.js for the required jQuery ajaxComplete handlers. - */ - public function redirect(string $url, int $code = 302): HTTPResponse - { - if ($this->getRequest()->isAjax()) { - $response = $this->getResponse(); - $response->addHeader('X-ControllerURL', $url); - if ($this->getRequest()->getHeader('X-Pjax') && !$response->getHeader('X-Pjax')) { - $response->addHeader('X-Pjax', $this->getRequest()->getHeader('X-Pjax')); - } - $newResponse = new LeftAndMain_HTTPResponse( - $response->getBody(), - $response->getStatusCode(), - $response->getStatusDescription() - ); - foreach ($response->getHeaders() as $k => $v) { - $newResponse->addHeader($k, $v); - } - $newResponse->setIsFinished(true); - $this->setResponse($newResponse); - // Actual response will be re-requested by client - return $newResponse; - } else { - return parent::redirect($url, $code); - } - } - public function index(HTTPRequest $request): HTTPResponse { return $this->getResponseNegotiator()->respond($request); @@ -940,41 +676,6 @@ public function ShowSwitchView() return false; } - - //------------------------------------------------------------------------------------------// - // Main controllers - - /** - * You should implement a Link() function in your subclass of LeftAndMain, - * to point to the URL of that particular controller. - * - * @param string $action - * @return string - */ - public function Link($action = null) - { - // LeftAndMain methods have a top-level uri access - if (static::class === LeftAndMain::class) { - $segment = ''; - } else { - // Get url_segment - $segment = $this->config()->get('url_segment'); - if (!$segment) { - throw new BadMethodCallException( - sprintf('LeftAndMain subclasses (%s) must have url_segment', static::class) - ); - } - } - - $link = Controller::join_links( - AdminRootController::admin_url(), - $segment, - "$action" - ); - $this->extend('updateLink', $link); - return $link; - } - /** * Get menu title for this section (translated) * @@ -1272,13 +973,6 @@ public function Breadcrumbs($unlinked = false) return $items; } - /** - * Cached search filter instance for current search - * - * @var LeftAndMain_SearchFilter - */ - protected $searchFilterCache = null; - /** * Gets the current search filter for this request, if available * @@ -1861,21 +1555,6 @@ public function SiteConfig() return class_exists(SiteConfig::class) ? SiteConfig::current_site_config() : null; } - /** - * The urls used for the links in the Help dropdown in the backend - * - * Set this to `false` via yml config if you do not want to show any help links - * - * @config - * @var array - */ - private static $help_links = [ - 'CMS User help' => 'https://userhelp.silverstripe.org/en/5', - 'Developer docs' => 'https://docs.silverstripe.org/en/5/', - 'Community' => 'https://www.silverstripe.org/', - 'Feedback' => 'https://www.silverstripe.org/give-feedback/', - ]; - /** * Returns help_links in a format readable by a template * @return ArrayList @@ -1901,14 +1580,6 @@ public function getHelpLinks() return ArrayList::create($formattedLinks); } - /** - * The href for the anchor on the Silverstripe logo - * - * @config - * @var string - */ - private static $application_link = '//www.silverstripe.org/'; - /** * @return string */ @@ -1917,14 +1588,6 @@ public function ApplicationLink() return $this->config()->get('application_link'); } - /** - * The application name - * - * @config - * @var string - */ - private static $application_name = 'Silverstripe'; - /** * Get the application name. * diff --git a/code/SudoModeController.php b/code/SudoModeController.php index 0a5376d5e..7140c3144 100644 --- a/code/SudoModeController.php +++ b/code/SudoModeController.php @@ -15,12 +15,10 @@ /** * Responsible for checking and verifying whether sudo mode is enabled */ -class SudoModeController extends LeftAndMain +class SudoModeController extends AdminController { private static string $url_segment = 'sudomode'; - private static bool $ignore_menuitem = true; - private static array $allowed_actions = [ 'check', 'activate', @@ -43,7 +41,7 @@ class SudoModeController extends LeftAndMain */ private static bool $required_permission_codes = false; - public function getClientConfig() + public function getClientConfig(): array { $request = Injector::inst()->get(HTTPRequest::class); @@ -91,7 +89,7 @@ public function activate(HTTPRequest $request): HTTPResponse return $this->jsonResponse([ 'result' => false, 'message' => _t(__CLASS__ . '.INVALID', 'Incorrect password'), - ]); + ], 400); } // Activate sudo mode and return successful result diff --git a/lang/en.yml b/lang/en.yml index a5b066f6d..0bad86de6 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -1,4 +1,12 @@ en: + SilverStripe\Admin\AdminController: + ErrorMessage: 'Sorry, it seems there was a {errorcode} error.' + ErrorMessage400: 'Sorry, it seems there was something wrong with the request.' + ErrorMessage401: 'Sorry, it seems you are not authorised to access this section or object.' + ErrorMessage403: 'Sorry, it seems the action you were trying to perform is forbidden.' + ErrorMessage404: "Sorry, it seems you were trying to access a section or object that doesn't exist." + ErrorMessage500: 'Sorry, it seems there was an internal server error.' + ErrorMessage503: 'Sorry, it seems the service is temporarily unavailable.' SilverStripe\Admin\CMSProfileController: CANTEDIT: "You don't have permission to do that" MENUTITLE: 'My Profile' diff --git a/tests/php/AdminControllerTest.php b/tests/php/AdminControllerTest.php new file mode 100644 index 000000000..e69868489 --- /dev/null +++ b/tests/php/AdminControllerTest.php @@ -0,0 +1,236 @@ + [ + 'fixtureName' => null, + 'allowed' => [], + ], + 'no-access user' => [ + 'fixtureName' => 'noaccessuser', + 'allowed' => [], + ], + 'restricted CMS user' => [ + 'fixtureName' => 'securityonlyuser', + 'allowed' => [ + CMSProfileController::class, + SecurityAdmin::class, + ], + ], + 'all cms sections user' => [ + 'fixtureName' => 'allcmssectionsuser', + 'allowed' => [ + TestAdminController::class, + DualPermissionAdminController::class, + CMSProfileController::class, + SecurityAdmin::class, + ], + ], + 'admin user' => [ + 'fixtureName' => 'admin', + 'allowed' => [ + TestAdminController::class, + DualPermissionAdminController::class, + CMSProfileController::class, + SecurityAdmin::class, + ], + ], + 'permission1 user (missing permission2)' => [ + 'fixtureName' => 'permission1user', + 'allowed' => [], + ], + 'permission2 user (missing permission1)' => [ + 'fixtureName' => 'permission2user', + 'allowed' => [], + ], + 'permission1 and 2 user user' => [ + 'fixtureName' => 'permission1and2user', + 'allowed' => [ + DualPermissionAdminController::class, + ], + ], + ]; + } + + #[DataProvider('provideCanView')] + public function testCanView(?string $fixtureName, array $allowed): void + { + $allControllers = [ + CMSProfileController::class, + SecurityAdmin::class, + TestAdminController::class, + DualPermissionAdminController::class, + ]; + $this->logOut(); + $user = null; + if ($fixtureName !== null) { + $user = $this->objFromFixture(Member::class, $fixtureName); + $this->logInAs($user); + } + + foreach ($allowed as $allowedClass) { + $this->assertTrue($allowedClass::singleton()->canView(), 'Tried to access ' . $allowedClass); + } + $disallowed = array_diff($allControllers, $allowed); + foreach ($disallowed as $disallowedClass) { + $this->assertFalse($disallowedClass::singleton()->canView(), 'Tried to access ' . $disallowedClass); + } + } + + public static function provideJsonSuccess(): array + { + return [ + [ + 'statusCode' => 201, + 'data' => null, + 'expectedBody' => '', + 'expectedException' => '', + ], + [ + 'statusCode' => 200, + 'data' => [], + 'expectedBody' => '[]', + 'expectedException' => '', + ], + [ + 'statusCode' => 200, + 'data' => [1, "two", 3.3], + 'expectedBody' => '[1,"two",3.3]', + 'expectedException' => '', + ], + [ + 'statusCode' => 200, + 'data' => ['foo' => 'bar', 'quotes' => '"something"', 'array' => [1, 2, 3]], + 'expectedBody' => '{"foo":"bar","quotes":"\"something\"","array":[1,2,3]}', + 'expectedException' => '', + ], + [ + 'statusCode' => 200, + 'data' => ['unicode' => ['one' => 'ōōō', 'two' => '℅℅℅', 'three' => '👍👍👍']], + 'expectedBody' => '{"unicode":{"one":"ōōō","two":"℅℅℅","three":"👍👍👍"}}', + 'expectedException' => '', + ], + [ + 'statusCode' => 199, + 'data' => [], + 'expectedBody' => '', + 'expectedException' => InvalidArgumentException::class, + ], + [ + 'statusCode' => 302, + 'data' => [], + 'expectedBody' => '', + 'expectedException' => InvalidArgumentException::class, + ], + ]; + } + + #[DataProvider('provideJsonSuccess')] + public function testJsonSuccess( + int $statusCode, + ?array $data, + string $expectedBody, + string $expectedException + ): void { + $controller = new TestAdminController(); + $refelectionObject = new ReflectionObject($controller); + $method = $refelectionObject->getMethod('jsonSuccess'); + $method->setAccessible(true); + if ($expectedException) { + $this->expectException($expectedException); + } + $response = $method->invoke($controller, $statusCode, $data); + $this->assertSame('application/json', $response->getHeader('Content-type')); + $this->assertSame($statusCode, $response->getStatusCode()); + $this->assertSame($expectedBody, $response->getBody()); + } + + public static function provideJsonError(): array + { + return [ + [ + 'statusCode' => 400, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems there was something wrong with the request.', + ], + [ + 'statusCode' => 401, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems you are not authorised to access this section or object.', + ], + [ + 'statusCode' => 403, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems the action you were trying to perform is forbidden.', + ], + [ + 'statusCode' => 404, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems you were trying to access a section or object that doesn\'t exist.', + ], + [ + 'statusCode' => 500, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems there was an internal server error.', + ], + [ + 'statusCode' => 503, + 'errorMessage' => '', + 'expectedValue' => 'Sorry, it seems the service is temporarily unavailable.', + ], + [ + 'statusCode' => 418, + 'errorMessage' => '', + 'expectedValue' => 'Error', + ], + [ + 'statusCode' => 400, + 'errorMessage' => 'Test custom error message', + 'expectedValue' => 'Test custom error message', + ], + ]; + } + + #[DataProvider('provideJsonError')] + public function testJsonError( + int $statusCode, + string $errorMessage, + ?string $expectedValue, + ): void { + $controller = new TestAdminController(); + $refelectionObject = new ReflectionObject($controller); + $method = $refelectionObject->getMethod('jsonError'); + $method->setAccessible(true); + $this->expectException(HTTPResponse_Exception::class); + $expectedMessage = json_encode((object) [ + 'status' => 'error', + 'errors' => [ + (object) [ + 'type' => 'error', + 'code' => $statusCode, + 'value' => $expectedValue, + ], + ], + ]); + $this->expectExceptionMessage($expectedMessage); + $method->invoke($controller, $statusCode, $errorMessage); + } +} diff --git a/tests/php/LeftAndMainTest.yml b/tests/php/AdminControllerTest.yml similarity index 62% rename from tests/php/LeftAndMainTest.yml rename to tests/php/AdminControllerTest.yml index 1b3bbfaa9..045bb34c0 100644 --- a/tests/php/LeftAndMainTest.yml +++ b/tests/php/AdminControllerTest.yml @@ -9,6 +9,10 @@ SilverStripe\Security\Group: Title: allcmssections rooteditusers: Title: rooteditusers + permission1: + Title: permission1 + permission2: + Title: permission2 SilverStripe\Security\Member: admin: Email: admin@example.com @@ -23,6 +27,17 @@ SilverStripe\Security\Member: rootedituser: Email: rootedituser@test.com Groups: =>SilverStripe\Security\Group.rooteditusers + noaccessuser: + Email: noaccessuser@test.com + permission1user: + Email: permission1user@test.com + Groups: =>SilverStripe\Security\Group.permission1 + permission2user: + Email: permission2user@test.com + Groups: =>SilverStripe\Security\Group.permission2 + permission1and2user: + Email: permission1and2user@test.com + Groups: =>SilverStripe\Security\Group.permission1,=>SilverStripe\Security\Group.permission2 SilverStripe\Security\Permission: admin: Code: ADMIN @@ -36,3 +51,9 @@ SilverStripe\Security\Permission: allcmssections2: Code: CMS_ACCESS_LeftAndMain GroupID: =>SilverStripe\Security\Group.rooteditusers + permission1: + Code: PERMISSION1 + GroupID: =>SilverStripe\Security\Group.permission1 + permission2: + Code: PERMISSION2 + GroupID: =>SilverStripe\Security\Group.permission2 diff --git a/tests/php/AdminControllerTest/DualPermissionAdminController.php b/tests/php/AdminControllerTest/DualPermissionAdminController.php new file mode 100644 index 000000000..34fece675 --- /dev/null +++ b/tests/php/AdminControllerTest/DualPermissionAdminController.php @@ -0,0 +1,14 @@ +objFromFixture(Member::class, 'admin'); - $this->logInAs($admin); + $this->logInWithPermission('ADMIN'); $response = $this->get('admin/security'); // Check css @@ -135,65 +128,6 @@ public function testLeftAndMainSubclasses() $this->assertMatchesRegularExpression('/]*>/i', $response->getBody(), "$link should contain tag"); } - public function testCanView() - { - $adminuser = $this->objFromFixture(Member::class, 'admin'); - $securityonlyuser = $this->objFromFixture(Member::class, 'securityonlyuser'); - $allcmssectionsuser = $this->objFromFixture(Member::class, 'allcmssectionsuser'); - - // anonymous user - $this->logOut(); - $this->resetMenu(); - $menuItems = LeftAndMain::singleton()->MainMenu(false); - $this->assertEquals( - $menuItems->column('Code'), - [], - 'Without valid login, members cant access any menu entries' - ); - - // restricted cms user - $this->logInAs($securityonlyuser); - $this->resetMenu(); - $menuItems = LeftAndMain::singleton()->MainMenu(false); - $menuItems = $menuItems->column('Code'); - sort($menuItems); - - $this->assertEquals( - [ - 'SilverStripe-Admin-CMSProfileController', - 'SilverStripe-Admin-SecurityAdmin' - ], - $menuItems, - 'Groups with limited access can only access the interfaces they have permissions for' - ); - - // all cms sections user - $this->logInAs($allcmssectionsuser); - $this->resetMenu(); - $menuItems = LeftAndMain::singleton()->MainMenu(false); - $this->assertContains( - 'SilverStripe-Admin-CMSProfileController', - $menuItems->column('Code'), - 'Group with CMS_ACCESS_SilverStripe\\Admin\\LeftAndMain permission can edit own profile' - ); - $this->assertContains( - 'SilverStripe-Admin-SecurityAdmin', - $menuItems->column('Code'), - 'Group with CMS_ACCESS_SilverStripe\\Admin\\LeftAndMain permission can access all sections' - ); - - // admin - $this->logInAs($adminuser); - $this->resetMenu(); - $menuItems = LeftAndMain::singleton()->MainMenu(false); - $this->assertContains( - 'SilverStripe-Admin-SecurityAdmin', - $menuItems->column('Code'), - 'Administrators can access Security Admin' - ); - - $this->logOut(); - } /** * Test that getHelpLinks transforms $help_links into the correct format @@ -255,8 +189,7 @@ public static function provideTestCMSVersionNumber() public function testValidationResult() { - $this->logInAs('admin'); - + $this->logInWithPermission('ADMIN'); $obj = MyTree::create(); $obj->write(); @@ -289,143 +222,4 @@ public function testValidationResult() $this->assertSame($result->messages[0]->fieldName, 'Content'); $this->assertSame($result->messages[0]->message, MyTree::INVALID_CONTENT_MESSAGE); } - - public static function provideJsonSuccess(): array - { - return [ - [ - 'statusCode' => 201, - 'data' => null, - 'expectedBody' => '', - 'expectedException' => '', - ], - [ - 'statusCode' => 200, - 'data' => [], - 'expectedBody' => '[]', - 'expectedException' => '', - ], - [ - 'statusCode' => 200, - 'data' => [1, "two", 3.3], - 'expectedBody' => '[1,"two",3.3]', - 'expectedException' => '', - ], - [ - 'statusCode' => 200, - 'data' => ['foo' => 'bar', 'quotes' => '"something"', 'array' => [1, 2, 3]], - 'expectedBody' => '{"foo":"bar","quotes":"\"something\"","array":[1,2,3]}', - 'expectedException' => '', - ], - [ - 'statusCode' => 200, - 'data' => ['unicode' => ['one' => 'ōōō', 'two' => '℅℅℅', 'three' => '👍👍👍']], - 'expectedBody' => '{"unicode":{"one":"ōōō","two":"℅℅℅","three":"👍👍👍"}}', - 'expectedException' => '', - ], - [ - 'statusCode' => 199, - 'data' => [], - 'expectedBody' => '', - 'expectedException' => InvalidArgumentException::class, - ], - [ - 'statusCode' => 302, - 'data' => [], - 'expectedBody' => '', - 'expectedException' => InvalidArgumentException::class, - ], - ]; - } - - #[DataProvider('provideJsonSuccess')] - public function testJsonSuccess( - int $statusCode, - ?array $data, - string $expectedBody, - string $expectedException - ): void { - $leftAndMain = new LeftAndMain(); - $refelectionObject = new ReflectionObject($leftAndMain); - $method = $refelectionObject->getMethod('jsonSuccess'); - $method->setAccessible(true); - if ($expectedException) { - $this->expectException($expectedException); - } - $response = $method->invoke($leftAndMain, $statusCode, $data); - $this->assertSame('application/json', $response->getHeader('Content-type')); - $this->assertSame($statusCode, $response->getStatusCode()); - $this->assertSame($expectedBody, $response->getBody()); - } - - public static function provideJsonError(): array - { - return [ - [ - 'statusCode' => 400, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems there was something wrong with the request.', - ], - [ - 'statusCode' => 401, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems you are not authorised to access this section or object.', - ], - [ - 'statusCode' => 403, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems the action you were trying to perform is forbidden.', - ], - [ - 'statusCode' => 404, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems you were trying to access a section or object that doesn\'t exist.', - ], - [ - 'statusCode' => 500, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems there was an internal server error.', - ], - [ - 'statusCode' => 503, - 'errorMessage' => null, - 'expectedValue' => 'Sorry, it seems the service is temporarily unavailable.', - ], - [ - 'statusCode' => 418, - 'errorMessage' => null, - 'expectedValue' => 'Error', - ], - [ - 'statusCode' => 400, - 'errorMessage' => 'Test custom error message', - 'expectedValue' => 'Test custom error message', - ], - ]; - } - - #[DataProvider('provideJsonError')] - public function testJsonError( - int $statusCode, - ?string $errorMessage, - ?string $expectedValue, - ): void { - $leftAndMain = new LeftAndMain(); - $refelectionObject = new ReflectionObject($leftAndMain); - $method = $refelectionObject->getMethod('jsonError'); - $method->setAccessible(true); - $this->expectException(HTTPResponse_Exception::class); - $expectedMessage = json_encode((object) [ - 'status' => 'error', - 'errors' => [ - (object) [ - 'type' => 'error', - 'code' => $statusCode, - 'value' => $expectedValue, - ], - ], - ]); - $this->expectExceptionMessage($expectedMessage); - $method->invoke($leftAndMain, $statusCode, $errorMessage); - } } diff --git a/tests/php/SudoModeControllerTest.php b/tests/php/SudoModeControllerTest.php index bd35f8162..320c505d6 100644 --- a/tests/php/SudoModeControllerTest.php +++ b/tests/php/SudoModeControllerTest.php @@ -88,7 +88,7 @@ public function testActivateFailsWithIncorrectPassword() 'Password' => 'wrongpassword!', ]); - $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(400, $response->getStatusCode()); $result = json_decode((string) $response->getBody(), true); $this->assertFalse($result['result'], 'Should have failed with incorrect password'); $this->assertEquals('Incorrect password', $result['message']);