diff --git a/admin/code/CMSBatchActionHandler.php b/admin/code/CMSBatchActionHandler.php index 2c0676326..4d3db3127 100644 --- a/admin/code/CMSBatchActionHandler.php +++ b/admin/code/CMSBatchActionHandler.php @@ -101,9 +101,9 @@ class CMSBatchActionHandler extends RequestHandler ); } - public function Link() + public function Link($action = null) { - return Controller::join_links($this->parentController->Link(), $this->urlSegment); + return Controller::join_links($this->parentController->Link(), $this->urlSegment, $action, '/'); } /** diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index ab03f5f57..313e4e91a 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -1063,6 +1063,16 @@ now generally safer to use the default inherited config, where in the past you w * Falsey config values (null, 0, false, etc) can now replace non-falsey values. * Introduced new ModuleLoader manifest, which allows modules to be found via composer name. E.g. `$cms = ModuleLoader::instance()->getManifest()->getModule('silverstripe/cms')` +* Certain methods have been moved from `Controller` to `RequestHandler`: + * `Link` + * `redirect` + * `redirectBack` +* `RequestHandler` link and redirection behaviour has been enhanced slightly: + * `Link` now relies on the `url_segment` handler being provided for the class. + If left unset, this will raise an error. + * `getBackURL` and `getReturnReferer` have been added to safely inspect the current request + to see if there is a url to redirect back to. + #### General and Core Removed API @@ -1100,6 +1110,8 @@ now generally safer to use the default inherited config, where in the past you w [silverstripe-archive/bbcodeparser](https://github.com/silverstripe-archive/silverstripe-bbcodeparser) * Removed `ViewableData::ThemeDir`. Use `ThemeResourceLoader::findThemedResource` in conjunction with `SSViewer::get_themes` instead. * Removed `Config::FIRST_SET` and `Config::INHERITED` +* Removed `RequestHandler.require_allowed_actions`. This is now fixed to on and cannot be + disabled. #### General and Core Deprecated API @@ -1415,8 +1427,10 @@ handle field-level and form-level messages. This has the following properties: * `getMessageCastingHelper` retrieves the DBField cast to use for the appropriate message cast * `getSchemaMessage` encodes this message for form schema use in ReactJS. -`Form` methods have been changed: +`Form` behaviour methods have been changed: +* __construct() now allows a RequestHandler to be passed as a first argument, rather than a Controller. + in addition this argument is now optional. This allows forms to be constructed as a model only. * `validate` is replaced with `validationResult` instead, which returns a `ValidationResult` instance. This is no longer automatically persisted in the state by default, unless a redirection occurs. You can also save any response in the state by manually invoking `saveFormState` inside a custom @@ -1431,6 +1445,15 @@ handle field-level and form-level messages. This has the following properties: * `getAjaxErrorResponse` and `getRedirectReferer` created to simplify `getValidationErrorResponse` * `addErrorMessage` removed. Users can either use `sessionMessage` or `sessionError` to add a form level message, throw a ValidationException during submission, or add a custom validator. +* `Form` is no longer a `RequestHandler`, but implements the `HasRequestHandler` interface and returns + a `FormRequestHandler` instance from `getRequestHandler()`. the `Form` constructor no longer has + any mandatory parameters, and the first parameter allows a non-`Controller` `RequestHandler` to be + passed. Certain methods have been moved from `Form` to `FormRequestHandler`: + * `buttonClicked` + * `checkAccessAction` + * `handleField` + * `httpSubmission` + * `Link` `Validator` methods have changed: @@ -1490,6 +1513,8 @@ New `TimeField` methods replace `getConfig()` / `setConfig()` * `resetValidation` * `messageForForm` * `addErrorMessage` + * `testSubmission` + * `testAjaxSubmission` * Removed `Validator::requireField()` method. * Removed `ValidationResult` (see above for replacements): * `messageList` diff --git a/src/Control/Controller.php b/src/Control/Controller.php index 43fe21897..a521bc5c9 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -132,17 +132,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider $this->extend('onAfterInit'); } - /** - * Returns a link to this controller. Overload with your own Link rules if they exist. - * - * @param string $action Optional action - * @return string - */ - public function Link($action = null) - { - return Controller::join_links(ClassInfo::shortName($this), $action, '/'); - } - /** * {@inheritdoc} * @@ -646,56 +635,9 @@ class Controller extends RequestHandler implements TemplateGlobalProvider . "; now trying to direct to $url", E_USER_WARNING); return null; } - - // Attach site-root to relative links, if they have a slash in them - if ($url=="" || $url[0]=='?' || (substr($url, 0, 4) != "http" && $url[0] != "/" && strpos($url, '/') !== false)) { - $url = Director::baseURL() . $url; - } - - return $this->getResponse()->redirect($url, $code); - } - - /** - * Redirect back. Uses either the HTTP-Referer or a manually set request-variable called "BackURL". - * This variable is needed in scenarios where HTTP-Referer is not sent (e.g when calling a page by - * location.href in IE). If none of the two variables is available, it will redirect to the base - * URL (see {@link Director::baseURL()}). - * - * @uses redirect() - * - * @return bool|HTTPResponse - */ - public function redirectBack() - { - // Don't cache the redirect back ever - HTTP::set_cache_age(0); - - $url = null; - - // In edge-cases, this will be called outside of a handleRequest() context; in that case, - // redirect to the homepage - don't break into the global state at this stage because we'll - // be calling from a test context or something else where the global state is inappropraite - if ($this->getRequest()) { - if ($this->getRequest()->requestVar('BackURL')) { - $url = $this->getRequest()->requestVar('BackURL'); - } elseif ($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) { - $url = $this->getRequest()->getHeader('X-Backurl'); - } elseif ($this->getRequest()->getHeader('Referer')) { - $url = $this->getRequest()->getHeader('Referer'); - } - } - - if (!$url) { - $url = Director::baseURL(); - } - - // absolute redirection URLs not located on this site may cause phishing - if (Director::is_site_url($url)) { - $url = Director::absoluteURL($url, true); - return $this->redirect($url); - } else { - return false; - } + $response = parent::redirect($url, $code); + $this->setResponse($response); + return $response; } /** diff --git a/src/Control/Director.php b/src/Control/Director.php index 0d5efe396..8e995e7d7 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -424,10 +424,6 @@ class Director implements TemplateGlobalProvider { $rules = Director::config()->uninherited('rules'); - if (isset($_REQUEST['debug'])) { - Debug::show($rules); - } - foreach ($rules as $pattern => $controllerOptions) { if (is_string($controllerOptions)) { if (substr($controllerOptions, 0, 2) == '->') { diff --git a/src/Control/HTTPResponse.php b/src/Control/HTTPResponse.php index a7eb140ef..6e35aa9a0 100644 --- a/src/Control/HTTPResponse.php +++ b/src/Control/HTTPResponse.php @@ -4,6 +4,7 @@ namespace SilverStripe\Control; use InvalidArgumentException; use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\View\Requirements; @@ -12,6 +13,7 @@ use SilverStripe\View\Requirements; */ class HTTPResponse { + use Injectable; /** * @var array diff --git a/src/Control/HasRequestHandler.php b/src/Control/HasRequestHandler.php new file mode 100644 index 000000000..3c3343657 --- /dev/null +++ b/src/Control/HasRequestHandler.php @@ -0,0 +1,16 @@ +brokenOnConstruct = false; @@ -235,7 +233,11 @@ class RequestHandler extends ViewableData // empty rule ourselves, to prevent infinite loops. Also prevent further handling of controller // actions which return themselves to avoid infinite loops. $matchedRuleWasEmpty = $request->isEmptyPattern($match['rule']); - if ($this !== $result && !$matchedRuleWasEmpty && ($result instanceof RequestHandler)) { + if ($this !== $result && !$matchedRuleWasEmpty && ($result instanceof RequestHandler || $result instanceof HasRequestHandler)) { + // Expose delegated request handler + if ($result instanceof HasRequestHandler) { + $result = $result->getRequestHandler(); + } $returnValue = $result->handleRequest($request, $model); // Array results can be used to handle @@ -485,7 +487,7 @@ class RequestHandler extends ViewableData $isAllowed = false; } elseif ($allowedActions === null) { // If undefined, allow action based on configuration - $isAllowed = !Config::inst()->get('SilverStripe\\Control\\RequestHandler', 'require_allowed_actions'); + $isAllowed = false; } // If we don't have a match in allowed_actions, @@ -508,7 +510,6 @@ class RequestHandler extends ViewableData */ public function httpError($errorCode, $errorMessage = null) { - $request = $this->getRequest(); // Call a handler method such as onBeforeHTTPError404 @@ -527,7 +528,7 @@ class RequestHandler extends ViewableData * {@link handleAction()} or {@link handleRequest()} have been called, * which adds a reference to an actual {@link HTTPRequest} object. * - * @return HTTPRequest|NullHTTPRequest + * @return HTTPRequest */ public function getRequest() { @@ -546,4 +547,122 @@ class RequestHandler extends ViewableData $this->request = $request; return $this; } + + /** + * Returns a link to this controller. Overload with your own Link rules if they exist. + * + * @param string $action Optional action + * @return string + */ + public function Link($action = null) + { + // Check configured url_segment + $url = $this->config()->get('url_segment'); + if ($url) { + return Controller::join_links($url, $action, '/'); + } + + // no link defined by default + trigger_error( + 'Request handler '.get_class($this). ' does not have a url_segment defined. '. + 'Relying on this link may be an application error', + E_USER_WARNING + ); + return null; + } + + /** + * Redirect to the given URL. + * + * @param string $url + * @param int $code + * @return HTTPResponse + */ + public function redirect($url, $code = 302) + { + $url = Director::absoluteURL($url); + $response = new HTTPResponse(); + return $response->redirect($url, $code); + } + + /** + * Safely get the value of the BackURL param, if provided via querystring / posted var + * + * @return string + */ + public function getBackURL() + { + $request = $this->getRequest(); + if (!$request) { + return null; + } + $backURL = $request->requestVar('BackURL'); + // Fall back to X-Backurl header + if (!$backURL && $request->isAjax() && $request->getHeader('X-Backurl')) { + $backURL = $request->getHeader('X-Backurl'); + } + if (!$backURL) { + return null; + } + if (Director::is_site_url($backURL)) { + return $backURL; + } + return null; + } + + /** + * Returns the referer, if it is safely validated as an internal URL + * and can be redirected to. + * + * @internal called from {@see Form::getValidationErrorResponse} + * @return string|null + */ + public function getReturnReferer() + { + $referer = $this->getReferer(); + if ($referer && Director::is_site_url($referer)) { + return $referer; + } + return null; + } + + /** + * Get referer + * + * @return string + */ + public function getReferer() + { + $request = $this->getRequest(); + if (!$request) { + return null; + } + return $request->getHeader('Referer'); + } + + /** + * Redirect back. Uses either the HTTP-Referer or a manually set request-variable called "BackURL". + * This variable is needed in scenarios where HTTP-Referer is not sent (e.g when calling a page by + * location.href in IE). If none of the two variables is available, it will redirect to the base + * URL (see {@link Director::baseURL()}). + * + * @uses redirect() + * + * @return HTTPResponse + */ + public function redirectBack() + { + // Don't cache the redirect back ever + HTTP::set_cache_age(0); + + // Prefer to redirect to ?BackURL, but fall back to Referer header + // As a last resort redirect to base url + $url = $this->getBackURL() + ?: $this->getReturnReferer() + ?: Director::baseURL(); + + // Only direct to absolute urls + $url = Director::absoluteURL($url); + return $this->redirect($url); + } } diff --git a/src/Core/ClassInfo.php b/src/Core/ClassInfo.php index 863e36575..85a983520 100644 --- a/src/Core/ClassInfo.php +++ b/src/Core/ClassInfo.php @@ -340,4 +340,22 @@ class ClassInfo $reflection = new ReflectionClass($nameOrObject); return $reflection->getShortName(); } + + /** + * Helper to determine if the given object has a method + * + * @param object $object + * @param string $method + * @return bool + */ + public static function hasMethod($object, $method) + { + if (empty($object)) { + return false; + } + if (method_exists($object, $method)) { + return true; + } + return method_exists($object, 'hasMethod') && $object->hasMethod($method); + } } diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 031c6e430..567f34c1e 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -2,26 +2,22 @@ namespace SilverStripe\Forms; -use SilverStripe\Control\Controller; -use SilverStripe\Control\Director; +use SilverStripe\Control\HasRequestHandler; use SilverStripe\Control\HTTP; -use SilverStripe\Control\HTTPRequest; -use SilverStripe\Control\HTTPResponse; -use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RequestHandler; use SilverStripe\Control\Session; +use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Convert; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Deprecation; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBHTMLText; -use SilverStripe\ORM\SS_List; -use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\NullSecurityToken; use SilverStripe\Security\SecurityToken; use SilverStripe\View\SSViewer; +use SilverStripe\View\ViewableData; /** * Base class for all forms. @@ -64,10 +60,15 @@ use SilverStripe\View\SSViewer; * For example, the "URLSegment" field in a standard CMS form would be * accessible through "admin/EditForm/field/URLSegment/FieldHolder". */ -class Form extends RequestHandler +class Form extends ViewableData implements HasRequestHandler { use FormMessage; + /** + * Default form Name property + */ + const DEFAULT_NAME = 'Form'; + /** * Form submission data is URL encoded */ @@ -97,7 +98,9 @@ class Form extends RequestHandler protected $actions; /** - * @var Controller + * Parent (optional) request handler + * + * @var RequestHandler */ protected $controller; @@ -112,7 +115,8 @@ class Form extends RequestHandler protected $validator; /** - * @var callable {@see setValidationResponseCallback()} + * @see setValidationResponseCallback() + * @var callable */ protected $validationResponseCallback; @@ -127,7 +131,9 @@ class Form extends RequestHandler protected $strictFormMethodCheck = false; /** - * @var DataObject|null $record Populated by {@link loadDataFrom()}. + * Populated by {@link loadDataFrom()}. + * + * @var DataObject|null */ protected $record; @@ -135,7 +141,7 @@ class Form extends RequestHandler * Keeps track of whether this form has a default action or not. * Set to false by $this->disableDefaultAction(); * - * @var boolean + * @var bool */ protected $hasDefaultAction = true; @@ -167,11 +173,6 @@ class Form extends RequestHandler */ protected $template; - /** - * @var callable|null - */ - protected $buttonClickedFunc; - /** * Should we redirect the user back down to the * the form on validation errors rather then just the page @@ -191,7 +192,9 @@ class Form extends RequestHandler protected $securityToken = null; /** - * @var array $extraClasses List of additional CSS classes for the form tag. + * List of additional CSS classes for the form tag. + * + * @var array */ protected $extraClasses = array(); @@ -207,8 +210,10 @@ class Form extends RequestHandler protected $encType; /** - * @var array Any custom form attributes set through {@link setAttributes()}. + * Any custom form attributes set through {@link setAttributes()}. * Some attributes are calculated on the fly, so please use {@link getAttributes()} to access them. + * + * @var array */ protected $attributes = array(); @@ -217,12 +222,10 @@ class Form extends RequestHandler */ protected $validationExemptActions = array(); - private static $allowed_actions = array( - 'handleField', - 'httpSubmission', - 'forTemplate', - ); - + /** + * @config + * @var array + */ private static $casting = array( 'AttributesHTML' => 'HTMLFragment', 'FormAttributes' => 'HTMLFragment', @@ -236,14 +239,19 @@ class Form extends RequestHandler private $templateHelper = null; /** - * @ignore + * HTML ID for this form. + * + * @var string */ private $htmlID = null; /** - * @ignore + * Custom form action path, if not linking to itself. + * E.g. could be used to post to an external link + * + * @var string */ - private $formActionPath = false; + protected $formActionPath = false; /** * @var bool @@ -253,15 +261,20 @@ class Form extends RequestHandler /** * Create a new form, with the given fields an action buttons. * - * @param Controller $controller The parent controller, necessary to create the appropriate form action tag. + * @param RequestHandler $controller Optional parent request handler * @param string $name The method on the controller that will return this form object. * @param FieldList $fields All of the fields in the form - a {@link FieldList} of {@link FormField} objects. * @param FieldList $actions All of the action buttons in the form - a {@link FieldLis} of * {@link FormAction} objects * @param Validator|null $validator Override the default validator instance (Default: {@link RequiredFields}) */ - public function __construct($controller, $name, FieldList $fields, FieldList $actions, Validator $validator = null) - { + public function __construct( + RequestHandler $controller = null, + $name = self::DEFAULT_NAME, + FieldList $fields = null, + FieldList $actions = null, + Validator $validator = null + ) { parent::__construct(); $fields->setForm($this); @@ -269,13 +282,9 @@ class Form extends RequestHandler $this->fields = $fields; $this->actions = $actions; - $this->controller = $controller; + $this->setController($controller); $this->setName($name); - if (!$this->controller) { - user_error("$this->class form created without a controller", E_USER_ERROR); - } - // Form validation $this->validator = ($validator) ? $validator : new RequiredFields(); $this->validator->setForm($this); @@ -285,8 +294,7 @@ class Form extends RequestHandler // Check if CSRF protection is enabled, either on the parent controller or from the default setting. Note that // method_exists() is used as some controllers (e.g. GroupTest) do not always extend from Object. - if (method_exists($controller, 'securityTokenEnabled') || (method_exists($controller, 'hasMethod') - && $controller->hasMethod('securityTokenEnabled'))) { + if (ClassInfo::hasMethod($controller, 'securityTokenEnabled')) { $securityEnabled = $controller->securityTokenEnabled(); } else { $securityEnabled = SecurityToken::is_enabled(); @@ -297,18 +305,9 @@ class Form extends RequestHandler $this->setupDefaultClasses(); } - /** - * @var array - */ - private static $url_handlers = array( - 'field/$FieldName!' => 'handleField', - 'POST ' => 'httpSubmission', - 'GET ' => 'httpSubmission', - 'HEAD ' => 'httpSubmission', - ); - /** * Load form state from session state + * * @return $this */ public function restoreFormState() @@ -329,11 +328,14 @@ class Form extends RequestHandler /** * Flush persistant form state details + * + * @return $this */ public function clearFormState() { Session::clear("FormInfo.{$this->FormName()}.result"); Session::clear("FormInfo.{$this->FormName()}.data"); + return $this; } /** @@ -350,10 +352,12 @@ class Form extends RequestHandler * Store the given form data in the session * * @param array $data + * @return $this */ public function setSessionData($data) { Session::set("FormInfo.{$this->FormName()}.data", $data); + return $this; } /** @@ -374,6 +378,7 @@ class Form extends RequestHandler * Sets the ValidationResult in the session to be used with the next view of this form. * @param ValidationResult $result The result to save * @param bool $combineWithExisting If true, then this will be added to the existing result. + * @return $this */ public function setSessionValidationResult(ValidationResult $result, $combineWithExisting = false) { @@ -392,12 +397,19 @@ class Form extends RequestHandler // Serialise $resultData = $result ? serialize($result) : null; Session::set("FormInfo.{$this->FormName()}.result", $resultData); + return $this; } + /** + * Clear form message (and in session) + * + * @return $this + */ public function clearMessage() { $this->setMessage(null); $this->clearFormState(); + return $this; } /** @@ -467,175 +479,6 @@ class Form extends RequestHandler } } - /** - * Handle a form submission. GET and POST requests behave identically. - * Populates the form with {@link loadDataFrom()}, calls {@link validate()}, - * and only triggers the requested form action/method - * if the form is valid. - * - * @param HTTPRequest $request - * @return HTTPResponse - * @throws HTTPResponse_Exception - */ - public function httpSubmission($request) - { - // Strict method check - if ($this->strictFormMethodCheck) { - // Throws an error if the method is bad... - if ($this->formMethod != $request->httpMethod()) { - $response = Controller::curr()->getResponse(); - $response->addHeader('Allow', $this->formMethod); - $this->httpError(405, _t("Form.BAD_METHOD", "This form requires a ".$this->formMethod." submission")); - } - - // ...and only uses the variables corresponding to that method type - $vars = $this->formMethod == 'GET' ? $request->getVars() : $request->postVars(); - } else { - $vars = $request->requestVars(); - } - - // Ensure we only process saveable fields (non structural, readonly, or disabled) - $allowedFields = array_keys($this->Fields()->saveableFields()); - - // Populate the form - $this->loadDataFrom($vars, true, $allowedFields); - - // Protection against CSRF attacks - // @todo Move this to SecurityTokenField::validate() - $token = $this->getSecurityToken(); - if (! $token->checkRequest($request)) { - $securityID = $token->getName(); - if (empty($vars[$securityID])) { - $this->httpError(400, _t( - "Form.CSRF_FAILED_MESSAGE", - "There seems to have been a technical problem. Please click the back button, ". - "refresh your browser, and try again." - )); - } else { - // Clear invalid token on refresh - $this->clearFormState(); - $data = $this->getData(); - unset($data[$securityID]); - $this->setSessionData($data); - $this->sessionError(_t( - "Form.CSRF_EXPIRED_MESSAGE", - "Your session has expired. Please re-submit the form." - )); - - // Return the user - return $this->controller->redirectBack(); - } - } - - // Determine the action button clicked - $funcName = null; - foreach ($vars as $paramName => $paramVal) { - if (substr($paramName, 0, 7) == 'action_') { - // Break off querystring arguments included in the action - if (strpos($paramName, '?') !== false) { - list($paramName, $paramVars) = explode('?', $paramName, 2); - $newRequestParams = array(); - parse_str($paramVars, $newRequestParams); - $vars = array_merge((array)$vars, (array)$newRequestParams); - } - - // Cleanup action_, _x and _y from image fields - $funcName = preg_replace(array('/^action_/','/_x$|_y$/'), '', $paramName); - break; - } - } - - // If the action wasn't set, choose the default on the form. - if (!isset($funcName) && $defaultAction = $this->defaultAction()) { - $funcName = $defaultAction->actionName(); - } - - if (isset($funcName)) { - $this->setButtonClicked($funcName); - } - - // Permission checks (first on controller, then falling back to form) - if (// Ensure that the action is actually a button or method on the form, - // and not just a method on the controller. - $this->controller->hasMethod($funcName) - && !$this->controller->checkAccessAction($funcName) - // If a button exists, allow it on the controller - // buttonClicked() validates that the action set above is valid - && !$this->buttonClicked() - ) { - return $this->httpError( - 403, - sprintf('Action "%s" not allowed on controller (Class: %s)', $funcName, get_class($this->controller)) - ); - } elseif ($this->hasMethod($funcName) - && !$this->checkAccessAction($funcName) - // No checks for button existence or $allowed_actions is performed - - // all form methods are callable (e.g. the legacy "callfieldmethod()") - ) { - return $this->httpError( - 403, - sprintf('Action "%s" not allowed on form (Name: "%s")', $funcName, $this->name) - ); - } - - // Action handlers may throw ValidationExceptions. - try { - // Or we can use the Valiator attached to the form - $result = $this->validationResult(); - if (!$result->isValid()) { - return $this->getValidationErrorResponse($result); - } - - // First, try a handler method on the controller (has been checked for allowed_actions above already) - if ($this->controller->hasMethod($funcName)) { - return $this->controller->$funcName($vars, $this, $request); - } - - // Otherwise, try a handler method on the form object. - if ($this->hasMethod($funcName)) { - return $this->$funcName($vars, $this, $request); - } - - // Check for inline actions - if ($field = $this->checkFieldsForAction($this->Fields(), $funcName)) { - return $field->$funcName($vars, $this, $request); - } - } catch (ValidationException $e) { - // The ValdiationResult contains all the relevant metadata - $result = $e->getResult(); - $this->loadMessagesFrom($result); - return $this->getValidationErrorResponse($result); - } - - return $this->httpError(404); - } - - /** - * @param string $action - * @return bool - */ - public function checkAccessAction($action) - { - if (parent::checkAccessAction($action)) { - return true; - } - - $actions = $this->getAllActions(); - foreach ($actions as $formAction) { - if ($formAction->actionName() === $action) { - return true; - } - } - - // Always allow actions on fields - $field = $this->checkFieldsForAction($this->Fields(), $action); - if ($field && $field->checkAccessAction($action)) { - return true; - } - - return false; - } - /** * @return callable */ @@ -660,133 +503,6 @@ class Form extends RequestHandler return $this; } - - /** - * Returns the appropriate response up the controller chain - * if {@link validate()} fails (which is checked prior to executing any form actions). - * By default, returns different views for ajax/non-ajax request, and - * handles 'application/json' requests with a JSON object containing the error messages. - * Behaviour can be influenced by setting {@link $redirectToFormOnValidationError}, - * and can be overruled by setting {@link $validationResponseCallback}. - * - * @param ValidationResult $result - * @return HTTPResponse - */ - protected function getValidationErrorResponse(ValidationResult $result) - { - // Check for custom handling mechanism - $callback = $this->getValidationResponseCallback(); - if ($callback && $callbackResponse = call_user_func($callback, $result)) { - return $callbackResponse; - } - - // Check if handling via ajax - if ($this->getRequest()->isAjax()) { - return $this->getAjaxErrorResponse($result); - } - - // Prior to redirection, persist this result in session to re-display on redirect - $this->setSessionValidationResult($result); - $this->setSessionData($this->getData()); - - // Determine redirection method - if ($this->getRedirectToFormOnValidationError() && ($pageURL = $this->getRedirectReferer())) { - return $this->controller->redirect($pageURL . '#' . $this->FormName()); - } - return $this->controller->redirectBack(); - } - - /** - * Build HTTP error response for ajax requests - * - * @internal called from {@see Form::getValidationErrorResponse} - * @param ValidationResult $result - * @return HTTPResponse - */ - protected function getAjaxErrorResponse(ValidationResult $result) - { - // Ajax form submissions accept json encoded errors by default - $acceptType = $this->getRequest()->getHeader('Accept'); - if (strpos($acceptType, 'application/json') !== false) { - // Send validation errors back as JSON with a flag at the start - $response = new HTTPResponse(Convert::array2json($result->getMessages())); - $response->addHeader('Content-Type', 'application/json'); - return $response; - } - - // Send the newly rendered form tag as HTML - $this->loadMessagesFrom($result); - $response = new HTTPResponse($this->forTemplate()); - $response->addHeader('Content-Type', 'text/html'); - return $response; - } - - /** - * Get referrer to redirect back to and safely validates it - * - * @internal called from {@see Form::getValidationErrorResponse} - * @return string|null - */ - protected function getRedirectReferer() - { - $pageURL = $this->getRequest()->getHeader('Referer'); - if (!$pageURL) { - return null; - } - if (!Director::is_site_url($pageURL)) { - return null; - } - - // Remove existing pragmas - $pageURL = preg_replace('/(#.*)/', '', $pageURL); - return Director::absoluteURL($pageURL); - } - - /** - * Fields can have action to, let's check if anyone of the responds to $funcname them - * - * @param SS_List|array $fields - * @param callable $funcName - * @return FormField - */ - protected function checkFieldsForAction($fields, $funcName) - { - foreach ($fields as $field) { - /** @skipUpgrade */ - if (method_exists($field, 'FieldList')) { - if ($field = $this->checkFieldsForAction($field->FieldList(), $funcName)) { - return $field; - } - } elseif ($field->hasMethod($funcName) && $field->checkAccessAction($funcName)) { - return $field; - } - } - return null; - } - - /** - * Handle a field request. - * Uses {@link Form->dataFieldByName()} to find a matching field, - * and falls back to {@link FieldList->fieldByName()} to look - * for tabs instead. This means that if you have a tab and a - * formfield with the same name, this method gives priority - * to the formfield. - * - * @param HTTPRequest $request - * @return FormField - */ - public function handleField($request) - { - $field = $this->Fields()->dataFieldByName($request->param('FieldName')); - - if ($field) { - return $field; - } else { - // falling back to fieldByName, e.g. for getting tabs - return $this->Fields()->fieldByName($request->param('FieldName')); - } - } - /** * Convert this form into a readonly form */ @@ -906,6 +622,10 @@ class Form extends RequestHandler */ public function actionIsValidationExempt($action) { + // Non-actions don't bypass validation + if (!$action) { + return false; + } if ($action->getValidationExempt()) { return true; } @@ -1350,11 +1070,10 @@ class Form extends RequestHandler { if ($this->formActionPath) { return $this->formActionPath; - } elseif ($this->controller->hasMethod("FormObjectLink")) { - return $this->controller->FormObjectLink($this->name); - } else { - return Controller::join_links($this->controller->Link(), $this->name); } + + // Get action from request handler link + return $this->getRequestHandler()->Link(); } /** @@ -1406,9 +1125,9 @@ class Form extends RequestHandler } /** - * Get the controller. + * Get the controller or parent request handler. * - * @return Controller + * @return RequestHandler */ public function getController() { @@ -1416,15 +1135,14 @@ class Form extends RequestHandler } /** - * Set the controller. + * Set the controller or parent request handler. * - * @param Controller $controller - * @return Form + * @param RequestHandler $controller + * @return $this */ - public function setController($controller) + public function setController(RequestHandler $controller = null) { $this->controller = $controller; - return $this; } @@ -1537,21 +1255,19 @@ class Form extends RequestHandler */ public function validationResult() { - // Opportunity to invalidate via validator - $action = $this->buttonClicked(); - if ($action && $this->actionIsValidationExempt($action)) { + // Automatically pass if there is no validator, or the clicked button is exempt + // Note: Soft support here for validation with absent request handler + $handler = $this->getRequestHandler(); + $action = $handler ? $handler->buttonClicked() : null; + $validator = $this->getValidator(); + if (!$validator || $this->actionIsValidationExempt($action)) { return ValidationResult::create(); } // Invoke validator - if ($this->validator) { - $result = $this->validator->validate(); - $this->loadMessagesFrom($result); - return $result; - } - - // Successful result - return ValidationResult::create(); + $result = $validator->validate(); + $this->loadMessagesFrom($result); + return $result; } const MERGE_DEFAULT = 0; @@ -1765,6 +1481,7 @@ class Form extends RequestHandler $data = array(); if ($dataFields) { + /** @var FormField $field */ foreach ($dataFields as $field) { if ($field->getName()) { $data[$field->getName()] = $field->dataValue(); @@ -1830,7 +1547,7 @@ class Form extends RequestHandler $content = $this->forTemplate(); $this->IncludeFormTag = true; - $content .= "FormName . "_form_action\"" + $content .= "FormName() . "_form_action\"" . " value=\"" . $this->FormAction() . "\" />\n"; $content .= "FormName() . "\" />\n"; $content .= "FormMethod() . "\" />\n"; @@ -1858,53 +1575,6 @@ class Form extends RequestHandler return $template->process($custom); } - - /** - * Sets the button that was clicked. This should only be called by the Controller. - * - * @param callable $funcName The name of the action method that will be called. - * @return $this - */ - public function setButtonClicked($funcName) - { - $this->buttonClickedFunc = $funcName; - - return $this; - } - - /** - * @return FormAction - */ - public function buttonClicked() - { - $actions = $this->getAllActions(); - foreach ($actions as $action) { - if ($this->buttonClickedFunc === $action->actionName()) { - return $action; - } - } - - return null; - } - - /** - * Get a list of all actions, including those in the main "fields" FieldList - * - * @return array - */ - protected function getAllActions() - { - $fields = $this->fields->dataFields() ?: array(); - $actions = $this->actions->dataFields() ?: array(); - - $fieldsAndActions = array_merge($fields, $actions); - $actions = array_filter($fieldsAndActions, function ($fieldOrAction) { - return $fieldOrAction instanceof FormAction; - }); - - return $actions; - } - /** * Return the default button that should be clicked when another one isn't * available. @@ -2044,37 +1714,46 @@ class Form extends RequestHandler return $result; } - - ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// - // TESTING HELPERS - ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// + /** + * Current request handler, build by buildRequestHandler(), + * accessed by getRequestHandler() + * + * @var FormRequestHandler + */ + protected $requestHandler = null; /** - * Test a submission of this form. - * @param string $action - * @param array $data - * @return HTTPResponse the response object that the handling controller produces. You can interrogate this in - * your unit test. - * @throws HTTPResponse_Exception + * Get request handler for this form + * + * @return FormRequestHandler */ - public function testSubmission($action, $data) + public function getRequestHandler() { - $data['action_' . $action] = true; - - return Director::test($this->FormAction(), $data, Controller::curr()->getSession()); + if (!$this->requestHandler) { + $this->requestHandler = $this->buildRequestHandler(); + } + return $this->requestHandler; } /** - * Test an ajax submission of this form. + * Assign a specific request handler for this form * - * @param string $action - * @param array $data - * @return HTTPResponse the response object that the handling controller produces. You can interrogate this in - * your unit test. + * @param FormRequestHandler $handler + * @return $this */ - public function testAjaxSubmission($action, $data) + public function setRequestHandler(FormRequestHandler $handler) { - $data['ajax'] = 1; - return $this->testSubmission($action, $data); + $this->requestHandler = $handler; + return $this; + } + + /** + * Scaffold new request handler for this form + * + * @return FormRequestHandler + */ + protected function buildRequestHandler() + { + return FormRequestHandler::create($this); } } diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php new file mode 100644 index 000000000..8db8bd374 --- /dev/null +++ b/src/Forms/FormRequestHandler.php @@ -0,0 +1,510 @@ + 'handleField', + 'POST ' => 'httpSubmission', + 'GET ' => 'httpSubmission', + 'HEAD ' => 'httpSubmission', + ); + + /** + * Form model being handled + * + * @var Form + */ + protected $form = null; + + /** + * Build a new request handler for a given Form model + * + * @param Form $form + */ + public function __construct(Form $form) + { + $this->form = $form; + parent::__construct(); + + // Inherit parent controller request + $parent = $this->form->getController(); + if ($parent) { + $this->setRequest($parent->getRequest()); + } + } + + + /** + * Get link for this form + * + * @param string $action + * @return string + */ + public function Link($action = null) + { + // Forms without parent controller have no link; + // E.g. Submission handled via graphql + $controller = $this->form->getController(); + if (empty($controller)) { + return null; + } + + // Respect FormObjectLink() method + if ($controller->hasMethod("FormObjectLink")) { + return Controller::join_links( + $controller->FormObjectLink($this->form->getName()), + $action, + '/' + ); + } + + // Default form link + return Controller::join_links($controller->Link(), $this->form->getName(), $action, '/'); + } + + /** + * Handle a form submission. GET and POST requests behave identically. + * Populates the form with {@link loadDataFrom()}, calls {@link validate()}, + * and only triggers the requested form action/method + * if the form is valid. + * + * @param HTTPRequest $request + * @return HTTPResponse + * @throws HTTPResponse_Exception + */ + public function httpSubmission($request) + { + // Strict method check + if ($this->form->getStrictFormMethodCheck()) { + // Throws an error if the method is bad... + $allowedMethod = $this->form->FormMethod(); + if ($allowedMethod !== $request->httpMethod()) { + $response = Controller::curr()->getResponse(); + $response->addHeader('Allow', $allowedMethod); + $this->httpError(405, _t( + "Form.BAD_METHOD", + "This form requires a {method} submission", + ['method' => $allowedMethod] + )); + } + + // ...and only uses the variables corresponding to that method type + $vars = $allowedMethod === 'GET' + ? $request->getVars() + : $request->postVars(); + } else { + $vars = $request->requestVars(); + } + + // Ensure we only process saveable fields (non structural, readonly, or disabled) + $allowedFields = array_keys($this->form->Fields()->saveableFields()); + + // Populate the form + $this->form->loadDataFrom($vars, true, $allowedFields); + + // Protection against CSRF attacks + // @todo Move this to SecurityTokenField::validate() + $token = $this->form->getSecurityToken(); + if (! $token->checkRequest($request)) { + $securityID = $token->getName(); + if (empty($vars[$securityID])) { + $this->httpError(400, _t( + "Form.CSRF_FAILED_MESSAGE", + "There seems to have been a technical problem. Please click the back button, ". + "refresh your browser, and try again." + )); + } else { + // Clear invalid token on refresh + $this->form->clearFormState(); + $data = $this->form->getData(); + unset($data[$securityID]); + $this->form + ->setSessionData($data) + ->sessionError(_t( + "Form.CSRF_EXPIRED_MESSAGE", + "Your session has expired. Please re-submit the form." + )); + + // Return the user + return $this->redirectBack(); + } + } + + // Determine the action button clicked + $funcName = null; + foreach ($vars as $paramName => $paramVal) { + if (substr($paramName, 0, 7) == 'action_') { + // Break off querystring arguments included in the action + if (strpos($paramName, '?') !== false) { + list($paramName, $paramVars) = explode('?', $paramName, 2); + $newRequestParams = array(); + parse_str($paramVars, $newRequestParams); + $vars = array_merge((array)$vars, (array)$newRequestParams); + } + + // Cleanup action_, _x and _y from image fields + $funcName = preg_replace(array('/^action_/','/_x$|_y$/'), '', $paramName); + break; + } + } + + // If the action wasn't set, choose the default on the form. + if (!isset($funcName) && $defaultAction = $this->form->defaultAction()) { + $funcName = $defaultAction->actionName(); + } + + if (isset($funcName)) { + $this->setButtonClicked($funcName); + } + + // Permission checks (first on controller, then falling back to request handler) + $controller = $this->form->getController(); + if (// Ensure that the action is actually a button or method on the form, + // and not just a method on the controller. + $controller + && $controller->hasMethod($funcName) + && !$controller->checkAccessAction($funcName) + // If a button exists, allow it on the controller + // buttonClicked() validates that the action set above is valid + && !$this->buttonClicked() + ) { + return $this->httpError( + 403, + sprintf('Action "%s" not allowed on controller (Class: %s)', $funcName, get_class($controller)) + ); + } elseif (// No checks for button existence or $allowed_actions is performed - + // all form methods are callable (e.g. the legacy "callfieldmethod()") + $this->hasMethod($funcName) + && !$this->checkAccessAction($funcName) + ) { + return $this->httpError( + 403, + sprintf('Action "%s" not allowed on form request handler (Class: "%s")', $funcName, get_class($this)) + ); + } + + // Action handlers may throw ValidationExceptions. + try { + // Or we can use the Valiator attached to the form + $result = $this->form->validationResult(); + if (!$result->isValid()) { + return $this->getValidationErrorResponse($result); + } + + // First, try a handler method on the controller (has been checked for allowed_actions above already) + $controller = $this->form->getController(); + if ($controller && $controller->hasMethod($funcName)) { + return $controller->$funcName($vars, $this->form, $request); + } + + // Otherwise, try a handler method on the form request handler. + if ($this->hasMethod($funcName)) { + return $this->$funcName($vars, $this->form, $request); + } + + // Check for inline actions + $field = $this->checkFieldsForAction($this->form->Fields(), $funcName); + if ($field) { + return $field->$funcName($vars, $this->form, $request); + } + } catch (ValidationException $e) { + // The ValdiationResult contains all the relevant metadata + $result = $e->getResult(); + $this->form->loadMessagesFrom($result); + return $this->getValidationErrorResponse($result); + } + + // Determine if legacy form->allowed_actions is set + $legacyActions = $this->form->config()->get('allowed_actions'); + if ($legacyActions) { + throw new BadMethodCallException( + "allowed_actions are not valid on Form class " . get_class($this->form) . + ". Implement these in subclasses of " . get_class($this) . " instead" + ); + } + + return $this->httpError(404); + } + + /** + * @param string $action + * @return bool + */ + public function checkAccessAction($action) + { + if (parent::checkAccessAction($action)) { + return true; + } + + $actions = $this->getAllActions(); + foreach ($actions as $formAction) { + if ($formAction->actionName() === $action) { + return true; + } + } + + // Always allow actions on fields + $field = $this->checkFieldsForAction($this->form->Fields(), $action); + if ($field && $field->checkAccessAction($action)) { + return true; + } + + return false; + } + + + + /** + * Returns the appropriate response up the controller chain + * if {@link validate()} fails (which is checked prior to executing any form actions). + * By default, returns different views for ajax/non-ajax request, and + * handles 'application/json' requests with a JSON object containing the error messages. + * Behaviour can be influenced by setting {@link $redirectToFormOnValidationError}, + * and can be overruled by setting {@link $validationResponseCallback}. + * + * @param ValidationResult $result + * @return HTTPResponse + */ + protected function getValidationErrorResponse(ValidationResult $result) + { + // Check for custom handling mechanism + $callback = $this->form->getValidationResponseCallback(); + if ($callback && $callbackResponse = call_user_func($callback, $result)) { + return $callbackResponse; + } + + // Check if handling via ajax + if ($this->getRequest()->isAjax()) { + return $this->getAjaxErrorResponse($result); + } + + // Prior to redirection, persist this result in session to re-display on redirect + $this->form->setSessionValidationResult($result); + $this->form->setSessionData($this->form->getData()); + + // Determine redirection method + if ($this->form->getRedirectToFormOnValidationError()) { + return $this->redirectBackToForm(); + } + return $this->redirectBack(); + } + + /** + * Redirect back to this form with an added #anchor link + * + * @return HTTPResponse + */ + public function redirectBackToForm() + { + $pageURL = $this->getReturnReferer(); + if (!$pageURL) { + return $this->redirectBack(); + } + + // Add backURL and anchor + $pageURL = Controller::join_links( + $this->addBackURLParam($pageURL), + '#' . $this->form->FormName() + ); + + // Redirect + return $this->redirect($pageURL); + } + + /** + * Helper to add ?BackURL= to any link + * + * @param string $link + * @return string + */ + protected function addBackURLParam($link) + { + $backURL = $this->getBackURL(); + if ($backURL) { + return Controller::join_links($link, '?BackURL=' . urlencode($backURL)); + } + return $link; + } + + /** + * Build HTTP error response for ajax requests + * + * @internal called from {@see Form::getValidationErrorResponse} + * @param ValidationResult $result + * @return HTTPResponse + */ + protected function getAjaxErrorResponse(ValidationResult $result) + { + // Ajax form submissions accept json encoded errors by default + $acceptType = $this->getRequest()->getHeader('Accept'); + if (strpos($acceptType, 'application/json') !== false) { + // Send validation errors back as JSON with a flag at the start + $response = new HTTPResponse(Convert::array2json($result->getMessages())); + $response->addHeader('Content-Type', 'application/json'); + return $response; + } + + // Send the newly rendered form tag as HTML + $this->form->loadMessagesFrom($result); + $response = new HTTPResponse($this->form->forTemplate()); + $response->addHeader('Content-Type', 'text/html'); + return $response; + } + + /** + * Fields can have action to, let's check if anyone of the responds to $funcname them + * + * @param SS_List|array $fields + * @param callable $funcName + * @return FormField + */ + protected function checkFieldsForAction($fields, $funcName) + { + foreach ($fields as $field) { + /** @skipUpgrade */ + if (ClassInfo::hasMethod($field, 'FieldList')) { + if ($field = $this->checkFieldsForAction($field->FieldList(), $funcName)) { + return $field; + } + } elseif ($field->hasMethod($funcName) && $field->checkAccessAction($funcName)) { + return $field; + } + } + return null; + } + + /** + * Handle a field request. + * Uses {@link Form->dataFieldByName()} to find a matching field, + * and falls back to {@link FieldList->fieldByName()} to look + * for tabs instead. This means that if you have a tab and a + * formfield with the same name, this method gives priority + * to the formfield. + * + * @param HTTPRequest $request + * @return FormField + */ + public function handleField($request) + { + $field = $this->form->Fields()->dataFieldByName($request->param('FieldName')); + + if ($field) { + return $field; + } else { + // falling back to fieldByName, e.g. for getting tabs + return $this->form->Fields()->fieldByName($request->param('FieldName')); + } + } + + /** + * Sets the button that was clicked. This should only be called by the Controller. + * + * @param callable $funcName The name of the action method that will be called. + * @return $this + */ + public function setButtonClicked($funcName) + { + $this->buttonClickedFunc = $funcName; + return $this; + } + + /** + * Get instance of button which was clicked for this request + * + * @return FormAction + */ + public function buttonClicked() + { + $actions = $this->getAllActions(); + foreach ($actions as $action) { + if ($this->buttonClickedFunc === $action->actionName()) { + return $action; + } + } + return null; + } + + /** + * Get a list of all actions, including those in the main "fields" FieldList + * + * @return array + */ + protected function getAllActions() + { + $fields = $this->form->Fields()->dataFields(); + $actions = $this->form->Actions()->dataFields(); + + $fieldsAndActions = array_merge($fields, $actions); + $actions = array_filter($fieldsAndActions, function ($fieldOrAction) { + return $fieldOrAction instanceof FormAction; + }); + + return $actions; + } + + /** + * Processing that occurs before a form is executed. + * + * This includes form validation, if it fails, we throw a ValidationException + * + * This includes form validation, if it fails, we redirect back + * to the form with appropriate error messages. + * Always return true if the current form action is exempt from validation + * + * Triggered through {@link httpSubmission()}. + * + * + * Note that CSRF protection takes place in {@link httpSubmission()}, + * if it fails the form data will never reach this method. + * + * @return ValidationResult + */ + public function validationResult() + { + // Check if button is exempt, or if there is no validator + $action = $this->buttonClicked(); + $validator = $this->form->getValidator(); + if (!$validator || $this->form->actionIsValidationExempt($action)) { + return ValidationResult::create(); + } + + // Invoke validator + $result = $validator->validate(); + $this->form->loadMessagesFrom($result); + return $result; + } +} diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 4fa52cd36..0e1f958d7 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\GridField; +use SilverStripe\Control\HasRequestHandler; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; @@ -1032,7 +1033,13 @@ class GridField extends FormField return $result; } - if ($this !== $result && !$request->isEmptyPattern($rule) && is_object($result) && $result instanceof RequestHandler) { + if ($this !== $result && + !$request->isEmptyPattern($rule) && + ($result instanceof RequestHandler || $result instanceof HasRequestHandler) + ) { + if ($result instanceof HasRequestHandler) { + $result = $result->getRequestHandler(); + } $returnValue = $result->handleRequest($request, $model); if (is_array($returnValue)) { diff --git a/src/Forms/GridField/GridFieldDetailForm.php b/src/Forms/GridField/GridFieldDetailForm.php index fdfa83a42..13a40ed2b 100644 --- a/src/Forms/GridField/GridFieldDetailForm.php +++ b/src/Forms/GridField/GridFieldDetailForm.php @@ -3,13 +3,13 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\RequestHandler; use SilverStripe\Core\Extensible; use SilverStripe\ORM\DataModel; use SilverStripe\ORM\DataObject; use SilverStripe\Core\Object; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Injector\Injector; -use SilverStripe\Control\Controller; use SilverStripe\Forms\Validator; use SilverStripe\Forms\FieldList; use Closure; @@ -128,7 +128,7 @@ class GridFieldDetailForm implements GridField_URLHandler * * @param GridField $gridField * @param DataObject $record - * @param Controller $requestHandler + * @param RequestHandler $requestHandler * @return GridFieldDetailForm_ItemRequest */ protected function getItemRequestHandler($gridField, $record, $requestHandler) diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index 0ab9c10cc..571f8ef59 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -212,11 +212,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler } } - - // Caution: API violation. Form expects a Controller, but we are giving it a RequestHandler instead. - // Thanks to this however, we are able to nest GridFields, and also access the initial Controller by - // dereferencing GridFieldDetailForm_ItemRequest->getController() multiple times. See getToplevelController - // below. $form = new Form( $this, 'ItemEditForm', diff --git a/src/Security/CMSMemberLoginForm.php b/src/Security/CMSMemberLoginForm.php index b1b8d063d..d1f07dbab 100644 --- a/src/Security/CMSMemberLoginForm.php +++ b/src/Security/CMSMemberLoginForm.php @@ -18,9 +18,6 @@ use SilverStripe\Forms\FormAction; */ class CMSMemberLoginForm extends LoginForm { - - protected $authenticator_class = 'SilverStripe\\Security\\MemberAuthenticator'; - /** * Get link to use for external security actions * @@ -29,7 +26,7 @@ class CMSMemberLoginForm extends LoginForm */ public function getExternalLink($action = null) { - return Security::create()->Link($action); + return Security::singleton()->Link($action); } public function __construct(Controller $controller, $name) @@ -78,110 +75,8 @@ class CMSMemberLoginForm extends LoginForm parent::__construct($controller, $name, $fields, $actions); } - /** - * Try to authenticate the user - * - * @param array $data Submitted data - * @return Member Returns the member object on successful authentication - * or NULL on failure. - */ - public function performLogin($data) + protected function buildRequestHandler() { - $authenticator = $this->authenticator_class; - /** @var Member $member */ - $member = $authenticator::authenticate($data, $this); - if ($member) { - $member->logIn(isset($data['Remember'])); - return $member; - } - - $this->extend('authenticationFailed', $data); - return null; - } - - /** - * Login form handler method - * - * This method is called when the user clicks on "Log in" - * - * @param array $data Submitted data - * @return HTTPResponse - */ - public function dologin($data) - { - if ($this->performLogin($data)) { - return $this->logInUserAndRedirect($data); - } else { - // Find best url to redirect back to - $request = $this->controller->getRequest(); - $url = $request->getHeader('X-Backurl') - ?: $request->getHeader('Referer') - ?: $this->controller->Link('login'); - return $this->controller->redirect($url); - } - } - - /** - * Redirect the user to the change password form. - * - * @skipUpgrade - * @return HTTPResponse - */ - protected function redirectToChangePassword() - { - // Since this form is loaded via an iframe, this redirect must be performed via javascript - $changePasswordForm = new ChangePasswordForm($this->controller, 'ChangePasswordForm'); - $changePasswordForm->sessionMessage( - _t('Member.PASSWORDEXPIRED', 'Your password has expired. Please choose a new one.'), - 'good' - ); - - // Get redirect url - $changePasswordURL = $this->getExternalLink('changepassword'); - if ($backURL = $this->controller->getRequest()->requestVar('BackURL')) { - Session::set('BackURL', $backURL); - $changePasswordURL = Controller::join_links($changePasswordURL, '?BackURL=' . urlencode($backURL)); - } - $changePasswordURLATT = Convert::raw2att($changePasswordURL); - $changePasswordURLJS = Convert::raw2js($changePasswordURL); - $message = _t( - 'CMSMemberLoginForm.PASSWORDEXPIRED', - '

Your password has expired. Please choose a new one.

', - 'Message displayed to user if their session cannot be restored', - array('link' => $changePasswordURLATT) - ); - - // Redirect to change password page - $this->controller->getResponse()->setStatusCode(200); - $this->controller->getResponse()->setBody(<< - -$message - - -PHP - ); - return $this->controller->getResponse(); - } - - /** - * Send user to the right location after login - * - * @param array $data - * @return HTTPResponse - */ - protected function logInUserAndRedirect($data) - { - // Check password expiry - if (Member::currentUser()->isPasswordExpired()) { - // Redirect the user to the external password change form if necessary - return $this->redirectToChangePassword(); - } else { - // Link to success template - $url = $this->controller->Link('success'); - return $this->controller->redirect($url); - } + return CMSMemberLoginHandler::create($this); } } diff --git a/src/Security/CMSMemberLoginHandler.php b/src/Security/CMSMemberLoginHandler.php new file mode 100644 index 000000000..46bacb1ea --- /dev/null +++ b/src/Security/CMSMemberLoginHandler.php @@ -0,0 +1,93 @@ +performLogin($data)) { + return $this->logInUserAndRedirect($data); + } + + return $this->redirectBackToForm(); + } + + public function redirectBackToForm() + { + // Redirect back to form + $url = $this->addBackURLParam(CMSSecurity::singleton()->Link('login')); + return $this->redirect($url); + } + + /** + * Redirect the user to the change password form. + * + * @skipUpgrade + * @return HTTPResponse + */ + protected function redirectToChangePassword() + { + // Since this form is loaded via an iframe, this redirect must be performed via javascript + $changePasswordForm = ChangePasswordForm::create($this->form->getController(), 'ChangePasswordForm'); + $changePasswordForm->sessionMessage( + _t('Member.PASSWORDEXPIRED', 'Your password has expired. Please choose a new one.'), + 'good' + ); + + // Get redirect url + $changePasswordURL = $this->addBackURLParam(Security::singleton()->Link('changepassword')); + $changePasswordURLATT = Convert::raw2att($changePasswordURL); + $changePasswordURLJS = Convert::raw2js($changePasswordURL); + $message = _t( + 'CMSMemberLoginForm.PASSWORDEXPIRED', + '

Your password has expired. Please choose a new one.

', + 'Message displayed to user if their session cannot be restored', + array('link' => $changePasswordURLATT) + ); + + // Redirect to change password page + $response = HTTPResponse::create() + ->setBody(<< + +$message + + +PHP + ); + return $response; + } + + /** + * Send user to the right location after login + * + * @param array $data + * @return HTTPResponse + */ + protected function logInUserAndRedirect($data) + { + // Check password expiry + if (Member::currentUser()->isPasswordExpired()) { + // Redirect the user to the external password change form if necessary + return $this->redirectToChangePassword(); + } + + // Link to success template + $url = CMSSecurity::singleton()->Link('success'); + return $this->redirect($url); + } +} diff --git a/src/Security/ChangePasswordForm.php b/src/Security/ChangePasswordForm.php index c18233bf2..18565c803 100644 --- a/src/Security/ChangePasswordForm.php +++ b/src/Security/ChangePasswordForm.php @@ -2,30 +2,24 @@ namespace SilverStripe\Security; -use SilverStripe\Control\Controller; -use SilverStripe\Control\HTTPResponse; -use SilverStripe\Core\Convert; use SilverStripe\Control\Session; -use SilverStripe\Control\Director; -use SilverStripe\Control\HTTP; +use SilverStripe\Control\RequestHandler; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\PasswordField; use SilverStripe\Forms\FormAction; use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\Form; -use SilverStripe\ORM\ValidationResult; /** * Standard Change Password Form */ class ChangePasswordForm extends Form { - /** * Constructor * - * @param Controller $controller The parent controller, necessary to create the appropriate form action tag. + * @param RequestHandler $controller The parent controller, necessary to create the appropriate form action tag. * @param string $name The method on the controller that will return this form object. * @param FieldList|FormField $fields All of the fields in the form - a {@link FieldList} of * {@link FormField} objects. @@ -33,11 +27,7 @@ class ChangePasswordForm extends Form */ public function __construct($controller, $name, $fields = null, $actions = null) { - if (isset($_REQUEST['BackURL'])) { - $backURL = $_REQUEST['BackURL']; - } else { - $backURL = Session::get('BackURL'); - } + $backURL = $controller->getBackURL() ?: Session::get('BackURL'); if (!$fields) { $fields = new FieldList(); @@ -58,102 +48,18 @@ class ChangePasswordForm extends Form ); } - if (isset($backURL)) { - $fields->push(new HiddenField('BackURL', 'BackURL', $backURL)); + if ($backURL) { + $fields->push(new HiddenField('BackURL', false, $backURL)); } parent::__construct($controller, $name, $fields, $actions); } /** - * Change the password - * - * @param array $data The user submitted data - * @return HTTPResponse + * @return ChangePasswordHandler */ - public function doChangePassword(array $data) + protected function buildRequestHandler() { - if ($member = Member::currentUser()) { - // The user was logged in, check the current password - if (empty($data['OldPassword']) || !$member->checkPassword($data['OldPassword'])->isValid()) { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"), - "bad" - ); - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - } - - if (!$member) { - if (Session::get('AutoLoginHash')) { - $member = Member::member_from_autologinhash(Session::get('AutoLoginHash')); - } - - // The user is not logged in and no valid auto login hash is available - if (!$member) { - Session::clear('AutoLoginHash'); - return $this->controller->redirect($this->controller->Link('login')); - } - } - - // Check the new password - if (empty($data['NewPassword1'])) { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again"), - "bad" - ); - - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - - // Fail if passwords do not match - if ($data['NewPassword1'] !== $data['NewPassword2']) { - $this->clearMessage(); - $this->sessionMessage( - _t('Member.ERRORNEWPASSWORD', "You have entered your new password differently, try again"), - "bad" - ); - // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. - return $this->controller->redirect($this->controller->Link('changepassword')); - } - - // Check if the new password is accepted - $validationResult = $member->changePassword($data['NewPassword1']); - if (!$validationResult->isValid()) { - $this->setSessionValidationResult($validationResult); - return $this->controller->redirect($this->controller->Link('changepassword')); - } - - // Clear locked out status - $member->LockedOutUntil = null; - $member->FailedLoginCount = null; - $member->write(); - - if ($member->canLogIn()->isValid()) { - $member->logIn(); - } - - // TODO Add confirmation message to login redirect - Session::clear('AutoLoginHash'); - - if (!empty($_REQUEST['BackURL']) - // absolute redirection URLs may cause spoofing - && Director::is_site_url($_REQUEST['BackURL']) - ) { - $url = Director::absoluteURL($_REQUEST['BackURL']); - return $this->controller->redirect($url); - } else { - // Redirect to default location - the login form saying "You are logged in as..." - $redirectURL = HTTP::setGetVar( - 'BackURL', - Director::absoluteBaseURL(), - $this->controller->Link('login') - ); - return $this->controller->redirect($redirectURL); - } + return ChangePasswordHandler::create($this); } } diff --git a/src/Security/ChangePasswordHandler.php b/src/Security/ChangePasswordHandler.php new file mode 100644 index 000000000..902a97819 --- /dev/null +++ b/src/Security/ChangePasswordHandler.php @@ -0,0 +1,103 @@ +checkPassword($data['OldPassword'])->isValid() + )) { + $this->form->sessionMessage( + _t('Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"), + "bad" + ); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $this->redirectBackToForm(); + } + + if (!$member) { + if (Session::get('AutoLoginHash')) { + $member = Member::member_from_autologinhash(Session::get('AutoLoginHash')); + } + + // The user is not logged in and no valid auto login hash is available + if (!$member) { + Session::clear('AutoLoginHash'); + return $this->redirect($this->addBackURLParam(Security::singleton()->Link('login'))); + } + } + + // Check the new password + if (empty($data['NewPassword1'])) { + $this->form->sessionMessage( + _t('Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again"), + "bad" + ); + + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $this->redirectBackToForm(); + } + + // Fail if passwords do not match + if ($data['NewPassword1'] !== $data['NewPassword2']) { + $this->form->sessionMessage( + _t('Member.ERRORNEWPASSWORD', "You have entered your new password differently, try again"), + "bad" + ); + // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. + return $this->redirectBackToForm(); + } + + // Check if the new password is accepted + $validationResult = $member->changePassword($data['NewPassword1']); + if (!$validationResult->isValid()) { + $this->form->setSessionValidationResult($validationResult); + return $this->redirectBackToForm(); + } + + // Clear locked out status + $member->LockedOutUntil = null; + $member->FailedLoginCount = null; + $member->write(); + + if ($member->canLogIn()->isValid()) { + $member->logIn(); + } + + // TODO Add confirmation message to login redirect + Session::clear('AutoLoginHash'); + + // Redirect to backurl + $backURL = $this->getBackURL(); + if ($backURL) { + return $this->redirect($backURL); + } + + // Redirect to default location - the login form saying "You are logged in as..." + $url = Security::singleton()->Link('login'); + return $this->redirect($url); + } + + public function redirectBackToForm() + { + // Redirect back to form + $url = $this->addBackURLParam(CMSSecurity::singleton()->Link('changepassword')); + return $this->redirect($url); + } +} diff --git a/src/Security/MemberLoginForm.php b/src/Security/MemberLoginForm.php index 8a90af9e0..d04cf3696 100644 --- a/src/Security/MemberLoginForm.php +++ b/src/Security/MemberLoginForm.php @@ -2,13 +2,9 @@ namespace SilverStripe\Security; -use SilverStripe\Control\HTTPResponse; -use SilverStripe\Core\Convert; use SilverStripe\Control\Director; use SilverStripe\Control\Session; use SilverStripe\Control\Controller; -use SilverStripe\Control\Email\Email; -use SilverStripe\Dev\Debug; use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormAction; @@ -39,17 +35,6 @@ class MemberLoginForm extends LoginForm */ public $loggedInAsField = 'FirstName'; - protected $authenticator_class = 'SilverStripe\\Security\\MemberAuthenticator'; - - /** - * Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these - * remain valid actions regardless of the member login state. - * - * @var array - * @config - */ - private static $allowed_actions = array('dologin', 'logout'); - /** * Constructor * @@ -184,205 +169,11 @@ JS; return $this; } - /** - * Login form handler method - * - * This method is called when the user clicks on "Log in" - * - * @param array $data Submitted data + * @return MemberLoginHandler */ - public function dologin($data) + protected function buildRequestHandler() { - if ($this->performLogin($data)) { - $this->logInUserAndRedirect($data); - } else { - /** @skipUpgrade */ - if (array_key_exists('Email', $data)) { - Session::set('SessionForms.MemberLoginForm.Email', $data['Email']); - Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember'])); - } - - if (isset($_REQUEST['BackURL'])) { - $backURL = $_REQUEST['BackURL']; - } else { - $backURL = null; - } - - if ($backURL) { - Session::set('BackURL', $backURL); - } - - // Show the right tab on failed login - $loginLink = Director::absoluteURL($this->controller->Link('login')); - if ($backURL) { - $loginLink .= '?BackURL=' . urlencode($backURL); - } - $this->controller->redirect($loginLink . '#' . $this->FormName() .'_tab'); - } - } - - /** - * Login in the user and figure out where to redirect the browser. - * - * The $data has this format - * array( - * 'AuthenticationMethod' => 'MemberAuthenticator', - * 'Email' => 'sam@silverstripe.com', - * 'Password' => '1nitialPassword', - * 'BackURL' => 'test/link', - * [Optional: 'Remember' => 1 ] - * ) - * - * @param array $data - * @return HTTPResponse - */ - protected function logInUserAndRedirect($data) - { - Session::clear('SessionForms.MemberLoginForm.Email'); - Session::clear('SessionForms.MemberLoginForm.Remember'); - - if (Member::currentUser()->isPasswordExpired()) { - if (isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) { - Session::set('BackURL', $backURL); - } - /** @skipUpgrade */ - $cp = ChangePasswordForm::create($this->controller, 'ChangePasswordForm'); - $cp->sessionMessage( - _t('Member.PASSWORDEXPIRED', 'Your password has expired. Please choose a new one.'), - 'good' - ); - return $this->controller->redirect('Security/changepassword'); - } - - // Absolute redirection URLs may cause spoofing - if (!empty($_REQUEST['BackURL'])) { - $url = $_REQUEST['BackURL']; - if (Director::is_site_url($url)) { - $url = Director::absoluteURL($url); - } else { - // Spoofing attack, redirect to homepage instead of spoofing url - $url = Director::absoluteBaseURL(); - } - return $this->controller->redirect($url); - } - - // If a default login dest has been set, redirect to that. - if ($url = Security::config()->default_login_dest) { - $url = Controller::join_links(Director::absoluteBaseURL(), $url); - return $this->controller->redirect($url); - } - - // Redirect the user to the page where they came from - $member = Member::currentUser(); - if ($member) { - $firstname = Convert::raw2xml($member->FirstName); - if (!empty($data['Remember'])) { - Session::set('SessionForms.MemberLoginForm.Remember', '1'); - $member->logIn(true); - } else { - $member->logIn(); - } - - $message = _t('Member.WELCOMEBACK', "Welcome Back, {firstname}", array('firstname' => $firstname)); - Security::setLoginMessage($message, ValidationResult::TYPE_GOOD); - } - return Controller::curr()->redirectBack(); - } - - - /** - * Log out form handler method - * - * This method is called when the user clicks on "logout" on the form - * created when the parameter $checkCurrentUser of the - * {@link __construct constructor} was set to TRUE and the user was - * currently logged in. - */ - public function logout() - { - $s = new Security(); - $s->logout(); - } - - - /** - * Try to authenticate the user - * - * @param array $data Submitted data - * @return Member Returns the member object on successful authentication - * or NULL on failure. - */ - public function performLogin($data) - { - $member = call_user_func_array(array($this->authenticator_class, 'authenticate'), array($data, $this)); - if ($member) { - $member->LogIn(isset($data['Remember'])); - return $member; - } else { - $this->extend('authenticationFailed', $data); - return null; - } - } - - - /** - * Forgot password form handler method. - * Called when the user clicks on "I've lost my password". - * Extensions can use the 'forgotPassword' method to veto executing - * the logic, by returning FALSE. In this case, the user will be redirected back - * to the form without further action. It is recommended to set a message - * in the form detailing why the action was denied. - * - * @skipUpgrade - * @param array $data Submitted data - * @return HTTPResponse - */ - public function forgotPassword($data) - { - // Ensure password is given - if (empty($data['Email'])) { - $this->sessionMessage( - _t('Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'), - 'bad' - ); - - return $this->controller->redirect('Security/lostpassword'); - } - - // Find existing member - /** @var Member $member */ - $member = Member::get()->filter("Email", $data['Email'])->first(); - - // Allow vetoing forgot password requests - $results = $this->extend('forgotPassword', $member); - if ($results && is_array($results) && in_array(false, $results, true)) { - return $this->controller->redirect('Security/lostpassword'); - } - - if ($member) { - $token = $member->generateAutologinTokenAndStoreHash(); - - Email::create() - ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') - ->setData($member) - ->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject')) - ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) - ->setTo($member->Email) - ->send(); - - return $this->controller->redirect('Security/passwordsent/' . urlencode($data['Email'])); - } elseif ($data['Email']) { - // Avoid information disclosure by displaying the same status, - // regardless wether the email address actually exists - return $this->controller->redirect('Security/passwordsent/' . rawurlencode($data['Email'])); - } else { - $this->sessionMessage( - _t('Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'), - 'bad' - ); - - return $this->controller->redirect('Security/lostpassword'); - } + return MemberLoginHandler::create($this); } } diff --git a/src/Security/MemberLoginHandler.php b/src/Security/MemberLoginHandler.php new file mode 100644 index 000000000..698efe3af --- /dev/null +++ b/src/Security/MemberLoginHandler.php @@ -0,0 +1,240 @@ +performLogin($data)) { + return $this->logInUserAndRedirect($data); + } + + /** @skipUpgrade */ + if (array_key_exists('Email', $data)) { + Session::set('SessionForms.MemberLoginForm.Email', $data['Email']); + Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember'])); + } + + // Fail to login redirects back to form + return $this->redirectBackToForm(); + } + + /** + * Redirect to password recovery form + * + * @return HTTPResponse + */ + public function redirectToLostPassword() + { + $lostPasswordLink = Security::singleton()->Link('lostpassword'); + return $this->redirect($this->addBackURLParam($lostPasswordLink)); + } + + public function getReturnReferer() + { + // Home of login form is always this url + return Security::singleton()->Link('login'); + } + + /** + * Login in the user and figure out where to redirect the browser. + * + * The $data has this format + * array( + * 'AuthenticationMethod' => 'MemberAuthenticator', + * 'Email' => 'sam@silverstripe.com', + * 'Password' => '1nitialPassword', + * 'BackURL' => 'test/link', + * [Optional: 'Remember' => 1 ] + * ) + * + * @param array $data + * @return HTTPResponse + */ + protected function logInUserAndRedirect($data) + { + Session::clear('SessionForms.MemberLoginForm.Email'); + Session::clear('SessionForms.MemberLoginForm.Remember'); + + $member = Member::currentUser(); + if ($member->isPasswordExpired()) { + return $this->redirectToChangePassword(); + } + + // Absolute redirection URLs may cause spoofing + $backURL = $this->getBackURL(); + if ($backURL) { + return $this->redirect($backURL); + } + + // If a default login dest has been set, redirect to that. + $defaultLoginDest = Security::config()->get('default_login_dest'); + if ($defaultLoginDest) { + return $this->redirect($defaultLoginDest); + } + + // Redirect the user to the page where they came from + if ($member) { + if (!empty($data['Remember'])) { + Session::set('SessionForms.MemberLoginForm.Remember', '1'); + $member->logIn(true); + } else { + $member->logIn(); + } + + // Welcome message + $message = _t( + 'Member.WELCOMEBACK', + "Welcome Back, {firstname}", + ['firstname' => $member->FirstName] + ); + Security::setLoginMessage($message, ValidationResult::TYPE_GOOD); + } + + // Redirect back + return $this->redirectBack(); + } + + /** + * Log out form handler method + * + * This method is called when the user clicks on "logout" on the form + * created when the parameter $checkCurrentUser of the + * {@link __construct constructor} was set to TRUE and the user was + * currently logged in. + * + * @return HTTPResponse + */ + public function logout() + { + return Security::singleton()->logout(); + } + + /** + * Try to authenticate the user + * + * @param array $data Submitted data + * @return Member Returns the member object on successful authentication + * or NULL on failure. + */ + public function performLogin($data) + { + $member = call_user_func_array( + [$this->authenticator_class, 'authenticate'], + [$data, $this->form] + ); + if ($member) { + $member->LogIn(isset($data['Remember'])); + return $member; + } + + // No member, can't login + $this->extend('authenticationFailed', $data); + return null; + } + + /** + * Forgot password form handler method. + * Called when the user clicks on "I've lost my password". + * Extensions can use the 'forgotPassword' method to veto executing + * the logic, by returning FALSE. In this case, the user will be redirected back + * to the form without further action. It is recommended to set a message + * in the form detailing why the action was denied. + * + * @skipUpgrade + * @param array $data Submitted data + * @return HTTPResponse + */ + public function forgotPassword($data) + { + // Ensure password is given + if (empty($data['Email'])) { + $this->form->sessionMessage( + _t('Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'), + 'bad' + ); + return $this->redirectToLostPassword(); + } + + // Find existing member + /** @var Member $member */ + $member = Member::get()->filter("Email", $data['Email'])->first(); + + // Allow vetoing forgot password requests + $results = $this->extend('forgotPassword', $member); + if ($results && is_array($results) && in_array(false, $results, true)) { + return $this->redirectToLostPassword(); + } + + if ($member) { + $token = $member->generateAutologinTokenAndStoreHash(); + + Email::create() + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') + ->setData($member) + ->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject')) + ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) + ->setTo($member->Email) + ->send(); + } + + // Avoid information disclosure by displaying the same status, + // regardless wether the email address actually exists + $link = Controller::join_links( + Security::singleton()->Link('passwordsent'), + rawurlencode($data['Email']), + '/' + ); + return $this->redirect($this->addBackURLParam($link)); + } + + /** + * Invoked if password is expired and must be changed + * + * @skipUpgrade + * @return HTTPResponse + */ + protected function redirectToChangePassword() + { + $cp = ChangePasswordForm::create($this->form->getController(), 'ChangePasswordForm'); + $cp->sessionMessage( + _t('Member.PASSWORDEXPIRED', 'Your password has expired. Please choose a new one.'), + 'good' + ); + $changedPasswordLink = Security::singleton()->Link('changepassword'); + return $this->redirect($this->addBackURLParam($changedPasswordLink)); + } +} diff --git a/src/Security/Security.php b/src/Security/Security.php index a15381853..a19ce0fb6 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -2,17 +2,15 @@ namespace SilverStripe\Security; +use Page; use SilverStripe\CMS\Controllers\ContentController; -use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Session; use SilverStripe\Core\ClassInfo; -use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; -use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\TestOnly; use SilverStripe\Forms\EmailField; @@ -28,6 +26,7 @@ use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use SilverStripe\View\TemplateGlobalProvider; use Exception; +use SilverStripe\View\ViewableData_Customised; use Subsite; /** @@ -130,7 +129,7 @@ class Security extends Controller implements TemplateGlobalProvider * @var string * @config */ - private static $page_class = 'Page'; + private static $page_class = Page::class; /** * Default message set used in permission failures. @@ -192,18 +191,6 @@ class Security extends Controller implements TemplateGlobalProvider */ private static $robots_tag = 'noindex, nofollow'; - - /** - * Get location of word list file - * - * @deprecated 4.0 Use the "Security.word_list" config setting instead - */ - public static function get_word_list() - { - Deprecation::notice('4.0', 'Use the "Security.word_list" config setting instead'); - return self::config()->word_list; - } - /** * Enable or disable recording of login attempts * through the {@link LoginRecord} object. @@ -227,31 +214,6 @@ class Security extends Controller implements TemplateGlobalProvider */ static $database_is_ready = false; - /** - * Set location of word list file - * - * @deprecated 4.0 Use the "Security.word_list" config setting instead - * @param string $wordListFile Location of word list file - */ - public static function set_word_list($wordListFile) - { - Deprecation::notice('4.0', 'Use the "Security.word_list" config setting instead'); - self::config()->word_list = $wordListFile; - } - - /** - * Set the default message set used in permissions failures. - * - * @deprecated 4.0 Use the "Security.default_message_set" config setting instead - * @param string|array $messageSet - */ - public static function set_default_message_set($messageSet) - { - Deprecation::notice('4.0', 'Use the "Security.default_message_set" config setting instead'); - self::config()->default_message_set = $messageSet; - } - - /** * Register that we've had a permission failure trying to view the given page * @@ -377,13 +339,15 @@ class Security extends Controller implements TemplateGlobalProvider parent::init(); // Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options - if ($this->config()->frame_options) { - $this->getResponse()->addHeader('X-Frame-Options', $this->config()->frame_options); + $frameOptions = $this->config()->get('frame_options'); + if ($frameOptions) { + $this->getResponse()->addHeader('X-Frame-Options', $frameOptions); } // Prevent search engines from indexing the login page - if ($this->config()->robots_tag) { - $this->getResponse()->addHeader('X-Robots-Tag', $this->config()->robots_tag); + $robotsTag = $this->config()->get('robots_tag'); + if ($robotsTag) { + $this->getResponse()->addHeader('X-Robots-Tag', $robotsTag); } } @@ -473,6 +437,7 @@ class Security extends Controller implements TemplateGlobalProvider * - If it's false, the code calling logout() is * responsible for sending the user where-ever * they should go. + * @return HTTPResponse|null */ public function logout($redirect = true) { @@ -482,8 +447,9 @@ class Security extends Controller implements TemplateGlobalProvider } if ($redirect && (!$this->getResponse()->isFinished())) { - $this->redirectBack(); + return $this->redirectBack(); } + return null; } /** @@ -534,21 +500,24 @@ class Security extends Controller implements TemplateGlobalProvider */ protected function getResponseController($title) { - if (!class_exists('SilverStripe\\CMS\\Model\\SiteTree')) { + // Use the default setting for which Page to use to render the security page + $pageClass = $this->stat('page_class'); + if (!$pageClass || !class_exists($pageClass)) { return $this; } - // Use the default setting for which Page to use to render the security page - $pageClass = $this->stat('page_class'); - $tmpPage = new $pageClass; - $tmpPage->Title = $title; + // Create new instance of page holder + /** @var Page $holderPage */ + $holderPage = new $pageClass; + $holderPage->Title = $title; /** @skipUpgrade */ - $tmpPage->URLSegment = 'Security'; + $holderPage->URLSegment = 'Security'; // Disable ID-based caching of the log-in page by making it a random number - $tmpPage->ID = -1 * rand(1, 10000000); + $holderPage->ID = -1 * rand(1, 10000000); - $controllerClass = $tmpPage->getControllerName(); - $controller = $controllerClass::create($tmpPage); + $controllerClass = $holderPage->getControllerName(); + /** @var ContentController $controller */ + $controller = $controllerClass::create($holderPage); $controller->setDataModel($this->model); $controller->doInit(); return $controller; @@ -698,19 +667,20 @@ class Security extends Controller implements TemplateGlobalProvider return $response; } + $message = _t( + 'Security.NOTERESETPASSWORD', + 'Enter your e-mail address and we will send you a link with which you can reset your password' + ); + /** @var ViewableData_Customised $customisedController */ $customisedController = $controller->customise(array( - 'Content' => - '

' . - _t( - 'Security.NOTERESETPASSWORD', - 'Enter your e-mail address and we will send you a link with which you can reset your password' - ) . - '

', + 'Content' => DBField::create_field('HTMLFragment', "

$message

"), 'Form' => $this->LostPasswordForm(), )); //Controller::$currentController = $controller; - return $customisedController->renderWith($this->getTemplatesFor('lostpassword')); + $result = $customisedController->renderWith($this->getTemplatesFor('lostpassword')); + + return $result; } @@ -757,21 +727,19 @@ class Security extends Controller implements TemplateGlobalProvider $email = Convert::raw2xml(rawurldecode($request->param('ID')) . '.' . $request->getExtension()); + $message = _t( + 'Security.PASSWORDSENTTEXT', + "Thank you! A reset link has been sent to '{email}', provided an account exists for this email" + . " address.", + array('email' => Convert::raw2xml($email)) + ); $customisedController = $controller->customise(array( 'Title' => _t( 'Security.PASSWORDSENTHEADER', "Password reset link sent to '{email}'", array('email' => $email) ), - 'Content' => - "

" - . _t( - 'Security.PASSWORDSENTTEXT', - "Thank you! A reset link has been sent to '{email}', provided an account exists for this email" - . " address.", - array('email' => $email) - ) - . "

", + 'Content' => DBField::create_field('HTMLFragment', "

$message

"), 'Email' => $email )); @@ -844,31 +812,37 @@ class Security extends Controller implements TemplateGlobalProvider } elseif (Session::get('AutoLoginHash')) { // Subsequent request after the "first load with hash" (see previous if clause). $customisedController = $controller->customise(array( - 'Content' => - '

' . - _t('Security.ENTERNEWPASSWORD', 'Please enter a new password.') . - '

', + 'Content' => DBField::create_field( + 'HTMLFragment', + '

' . _t('Security.ENTERNEWPASSWORD', 'Please enter a new password.') . '

' + ), 'Form' => $this->ChangePasswordForm(), )); } elseif (Member::currentUser()) { // Logged in user requested a password change form. $customisedController = $controller->customise(array( - 'Content' => '

' - . _t('Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '

', + 'Content' => DBField::create_field( + 'HTMLFragment', + '

' . _t('Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '

' + ), 'Form' => $this->ChangePasswordForm())); } else { // Show friendly message if it seems like the user arrived here via password reset feature. if (isset($_REQUEST['m']) || isset($_REQUEST['t'])) { $customisedController = $controller->customise( - array('Content' => + array('Content' => DBField::create_field( + 'HTMLFragment', _t( 'Security.NOTERESETLINKINVALID', '

The password reset link is invalid or expired.

' . '

You can request a new one here or change your password after' . ' you logged in.

', - array('link1' => $this->Link('lostpassword'), 'link2' => $this->Link('login')) + [ + 'link1' => $this->Link('lostpassword'), + 'link2' => $this->Link('login') + ] ) - ) + )) ); } else { return self::permissionFailure( @@ -884,16 +858,12 @@ class Security extends Controller implements TemplateGlobalProvider /** * Factory method for the lost password form * + * @skipUpgrade * @return ChangePasswordForm Returns the lost password form */ public function ChangePasswordForm() { - /** @skipUpgrade */ - $formName = 'ChangePasswordForm'; - return Injector::inst()->createWithArgs( - 'SilverStripe\\Security\\ChangePasswordForm', - [ $this, $formName] - ); + return ChangePasswordForm::create($this, 'ChangePasswordForm'); } /** @@ -1092,7 +1062,7 @@ class Security extends Controller implements TemplateGlobalProvider { // Fall back to the default encryption algorithm if (!$algorithm) { - $algorithm = self::config()->password_encryption_algorithm; + $algorithm = self::config()->get('password_encryption_algorithm'); } $e = PasswordEncryptor::create_for_algorithm($algorithm); @@ -1221,7 +1191,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public static function login_url() { - return Controller::join_links(Director::baseURL(), self::config()->login_url); + return Controller::join_links(Director::baseURL(), self::config()->get('login_url')); } @@ -1234,7 +1204,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public static function logout_url() { - return Controller::join_links(Director::baseURL(), self::config()->logout_url); + return Controller::join_links(Director::baseURL(), self::config()->get('logout_url')); } /** @@ -1246,7 +1216,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public static function lost_password_url() { - return Controller::join_links(Director::baseURL(), self::config()->lost_password_url); + return Controller::join_links(Director::baseURL(), self::config()->get('lost_password_url')); } /** diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index c7d6d32d7..5f0704148 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -561,7 +561,7 @@ class ViewableData extends Object implements IteratorAggregate * @return string * @uses ClassInfo */ - public function CSSClasses($stopAtClass = 'SilverStripe\\View\\ViewableData') + public function CSSClasses($stopAtClass = self::class) { $classes = array(); $classAncestry = array_reverse(ClassInfo::ancestry($this->class)); diff --git a/tests/bootstrap/phpunit.php b/tests/bootstrap/phpunit.php index c465b67ef..fa4082694 100644 --- a/tests/bootstrap/phpunit.php +++ b/tests/bootstrap/phpunit.php @@ -3,8 +3,6 @@ // Bootstrap for running SapphireTests // Connect to database -use SilverStripe\Control\Tests\FakeController; -use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\DB; require_once __DIR__ . '/../../src/Core/Core.php'; diff --git a/tests/php/Control/ControllerTest.php b/tests/php/Control/ControllerTest.php index b40e1138f..87f272cd6 100644 --- a/tests/php/Control/ControllerTest.php +++ b/tests/php/Control/ControllerTest.php @@ -124,16 +124,6 @@ class ControllerTest extends FunctionalTest 'when called with an action in the URL' ); - Config::modify()->merge(RequestHandler::class, 'require_allowed_actions', false); - $response = $this->get("UnsecuredController/index"); - $this->assertEquals( - 200, - $response->getStatusCode(), - 'Access granted on index action without $allowed_actions on defining controller, ' . - 'when called with an action in the URL, and explicitly allowed through config' - ); - Config::modify()->merge(RequestHandler::class, 'require_allowed_actions', true); - $response = $this->get("UnsecuredController/method1"); $this->assertEquals( 403, @@ -142,16 +132,6 @@ class ControllerTest extends FunctionalTest 'when called without an action in the URL' ); - Config::modify()->merge(RequestHandler::class, 'require_allowed_actions', false); - $response = $this->get("UnsecuredController/method1"); - $this->assertEquals( - 200, - $response->getStatusCode(), - 'Access granted on action without $allowed_actions on defining controller, ' . - 'when called without an action in the URL, and explicitly allowed through config' - ); - Config::modify()->merge(RequestHandler::class, 'require_allowed_actions', true); - $response = $this->get("AccessBaseController/"); $this->assertEquals( 200, @@ -503,9 +483,9 @@ class ControllerTest extends FunctionalTest array('Referer' => $externalAbsoluteUrl) ); $this->assertEquals( - 200, - $response->getStatusCode(), - "Doesn't redirect on external URLs" + Director::absoluteBaseURL(), + $response->getHeader('Location'), + "Redirects back to home page on external url" ); } @@ -522,8 +502,10 @@ class ControllerTest extends FunctionalTest "Redirects on internal relative URLs" ); + // BackURL is internal link $internalAbsoluteUrl = Director::absoluteBaseURL() . '/some-url'; - $response = $this->get('TestController/redirectbacktest?BackURL=' . urlencode($internalAbsoluteUrl)); + $link = 'TestController/redirectbacktest?BackURL=' . urlencode($internalAbsoluteUrl); + $response = $this->get($link); $this->assertEquals($internalAbsoluteUrl, $response->getHeader('Location')); $this->assertEquals( 302, @@ -531,12 +513,13 @@ class ControllerTest extends FunctionalTest "Redirects on internal absolute URLs" ); + // Note that this test is affected by the prior ->get() $externalAbsoluteUrl = 'http://myhost.com/some-url'; $response = $this->get('TestController/redirectbacktest?BackURL=' . urlencode($externalAbsoluteUrl)); $this->assertEquals( - 200, - $response->getStatusCode(), - "Doesn't redirect on external URLs" + Director::absoluteURL($link), + $response->getHeader('Location'), + "If BackURL Is external link, fall back to last url (Referer)" ); } diff --git a/tests/php/Control/ControllerTest/AccessBaseController.php b/tests/php/Control/ControllerTest/AccessBaseController.php index a35d06ed5..ebff5b53a 100644 --- a/tests/php/Control/ControllerTest/AccessBaseController.php +++ b/tests/php/Control/ControllerTest/AccessBaseController.php @@ -9,6 +9,8 @@ class AccessBaseController extends Controller implements TestOnly { private static $allowed_actions = array(); + private static $url_segment = 'AccessBaseController'; + private static $extensions = [ AccessBaseControllerExtension::class ]; diff --git a/tests/php/Control/ControllerTest/AccessSecuredController.php b/tests/php/Control/ControllerTest/AccessSecuredController.php index befb5090a..00d12a4d2 100644 --- a/tests/php/Control/ControllerTest/AccessSecuredController.php +++ b/tests/php/Control/ControllerTest/AccessSecuredController.php @@ -2,11 +2,11 @@ namespace SilverStripe\Control\Tests\ControllerTest; -use SilverStripe\Control\Tests\ControllerTest; use SilverStripe\Dev\TestOnly; -class AccessSecuredController extends ControllerTest\AccessBaseController implements TestOnly +class AccessSecuredController extends AccessBaseController implements TestOnly { + private static $url_segment = 'AccessSecuredController'; private static $allowed_actions = array( "method1", // denied because only defined in parent diff --git a/tests/php/Control/ControllerTest/AccessWildcardSecuredController.php b/tests/php/Control/ControllerTest/AccessWildcardSecuredController.php index b8c01177f..21f661942 100644 --- a/tests/php/Control/ControllerTest/AccessWildcardSecuredController.php +++ b/tests/php/Control/ControllerTest/AccessWildcardSecuredController.php @@ -2,11 +2,11 @@ namespace SilverStripe\Control\Tests\ControllerTest; -use SilverStripe\Control\Tests\ControllerTest; use SilverStripe\Dev\TestOnly; -class AccessWildcardSecuredController extends ControllerTest\AccessBaseController implements TestOnly +class AccessWildcardSecuredController extends AccessBaseController implements TestOnly { + private static $url_segment = 'AccessWildcardSecuredController'; private static $allowed_actions = array( "*" => "ADMIN", // should throw exception diff --git a/tests/php/Control/ControllerTest/ContainerController.php b/tests/php/Control/ControllerTest/ContainerController.php index ef0b3909a..5aaa9cfed 100644 --- a/tests/php/Control/ControllerTest/ContainerController.php +++ b/tests/php/Control/ControllerTest/ContainerController.php @@ -7,6 +7,7 @@ use SilverStripe\Dev\TestOnly; class ContainerController extends Controller implements TestOnly { + private static $url_segment = 'ContainerController'; private static $allowed_actions = array( 'subcontroller', diff --git a/tests/php/Control/ControllerTest/HasAction.php b/tests/php/Control/ControllerTest/HasAction.php index 9ab3bcf0a..cb665f9aa 100644 --- a/tests/php/Control/ControllerTest/HasAction.php +++ b/tests/php/Control/ControllerTest/HasAction.php @@ -7,6 +7,7 @@ use SilverStripe\Dev\TestOnly; class HasAction extends Controller implements TestOnly { + private static $url_segment = 'HasAction'; private static $allowed_actions = array( 'allowed_action', diff --git a/tests/php/Control/ControllerTest/HasAction_Unsecured.php b/tests/php/Control/ControllerTest/HasAction_Unsecured.php index 6065976b3..109f7ba12 100644 --- a/tests/php/Control/ControllerTest/HasAction_Unsecured.php +++ b/tests/php/Control/ControllerTest/HasAction_Unsecured.php @@ -6,6 +6,7 @@ use SilverStripe\Dev\TestOnly; class HasAction_Unsecured extends HasAction implements TestOnly { + private static $url_segment = 'HasAction_Unsecured'; public function defined_action() { diff --git a/tests/php/Control/ControllerTest/IndexSecuredController.php b/tests/php/Control/ControllerTest/IndexSecuredController.php index 058754c01..cce27d9a5 100644 --- a/tests/php/Control/ControllerTest/IndexSecuredController.php +++ b/tests/php/Control/ControllerTest/IndexSecuredController.php @@ -2,11 +2,11 @@ namespace SilverStripe\Control\Tests\ControllerTest; -use SilverStripe\Control\Tests\ControllerTest; use SilverStripe\Dev\TestOnly; -class IndexSecuredController extends ControllerTest\AccessBaseController implements TestOnly +class IndexSecuredController extends AccessBaseController implements TestOnly { + private static $url_segment = 'IndexSecuredController'; private static $allowed_actions = array( "index" => "ADMIN", diff --git a/tests/php/Control/ControllerTest/SubController.php b/tests/php/Control/ControllerTest/SubController.php index c10c4dc52..8b87d1d02 100644 --- a/tests/php/Control/ControllerTest/SubController.php +++ b/tests/php/Control/ControllerTest/SubController.php @@ -7,6 +7,7 @@ use SilverStripe\Dev\TestOnly; class SubController extends Controller implements TestOnly { + private static $url_segment = 'SubController'; private static $allowed_actions = array( 'subaction', diff --git a/tests/php/Control/ControllerTest/TestController.php b/tests/php/Control/ControllerTest/TestController.php index 91130f275..ae967b8b4 100644 --- a/tests/php/Control/ControllerTest/TestController.php +++ b/tests/php/Control/ControllerTest/TestController.php @@ -10,6 +10,8 @@ use SilverStripe\Dev\TestOnly; */ class TestController extends Controller implements TestOnly { + private static $url_segment = 'TestController'; + public $Content = "default content"; private static $allowed_actions = array( diff --git a/tests/php/Control/ControllerTest/UnsecuredController.php b/tests/php/Control/ControllerTest/UnsecuredController.php index f579eacdd..06aef1208 100644 --- a/tests/php/Control/ControllerTest/UnsecuredController.php +++ b/tests/php/Control/ControllerTest/UnsecuredController.php @@ -7,6 +7,7 @@ use SilverStripe\Dev\TestOnly; class UnsecuredController extends Controller implements TestOnly { + private static $url_segment = 'UnsecuredController'; // Not defined, allow access to all // static $allowed_actions = array(); diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index 1e2b25afc..c2bc3a5ea 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -2,12 +2,17 @@ namespace SilverStripe\Control\Tests; +use SilverStripe\Control\Cookie_Backend; +use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\Tests\DirectorTest\TestController; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; use SilverStripe\Control\Director; use SilverStripe\Control\RequestProcessor; +use SilverStripe\Security\Security; /** * @todo test Director::alternateBaseFolder() @@ -116,7 +121,7 @@ class DirectorTest extends SapphireTest $rootURL = Director::protocolAndHost(); $_SERVER['REQUEST_URI'] = "$rootURL/mysite/sub-page/"; - Config::inst()->update('SilverStripe\\Control\\Director', 'alternate_base_url', '/mysite/'); + Director::config()->set('alternate_base_url', '/mysite/'); //test empty / local urls foreach (array('', './', '.') as $url) { @@ -176,7 +181,7 @@ class DirectorTest extends SapphireTest $rootURL = Director::protocolAndHost(); // relative base URLs - you should end them in a / - Config::inst()->update('SilverStripe\\Control\\Director', 'alternate_base_url', '/relativebase/'); + Director::config()->set('alternate_base_url', '/relativebase/'); $_SERVER['REQUEST_URI'] = "$rootURL/relativebase/sub-page/"; $this->assertEquals('/relativebase/', Director::baseURL()); @@ -187,7 +192,7 @@ class DirectorTest extends SapphireTest ); // absolute base URLs - you should end them in a / - Config::inst()->update('SilverStripe\\Control\\Director', 'alternate_base_url', 'http://www.example.org/'); + Director::config()->set('alternate_base_url', 'http://www.example.org/'); $_SERVER['REQUEST_URI'] = "http://www.example.org/sub-page/"; $this->assertEquals('http://www.example.org/', Director::baseURL()); $this->assertEquals('http://www.example.org/', Director::absoluteBaseURL()); @@ -250,7 +255,6 @@ class DirectorTest extends SapphireTest public function testIsRelativeUrl() { - $siteUrl = Director::absoluteBaseURL(); $this->assertFalse(Director::is_relative_url('http://test.com')); $this->assertFalse(Director::is_relative_url('https://test.com')); $this->assertFalse(Director::is_relative_url(' https://test.com/testpage ')); @@ -341,11 +345,11 @@ class DirectorTest extends SapphireTest $_COOKIE = array('somekey' => 'cookievalue'); $cookies = Injector::inst()->createWithArgs( - 'SilverStripe\\Control\\Cookie_Backend', + Cookie_Backend::class, array(array('somekey' => 'sometestcookievalue')) ); - $getresponse = Director::test( + Director::test( 'errorpage?somekey=sometestgetvalue', array('somekey' => 'sometestpostvalue'), null, @@ -387,10 +391,10 @@ class DirectorTest extends SapphireTest strtoupper($method), null, null, - Injector::inst()->createWithArgs('SilverStripe\\Control\\Cookie_Backend', array($fixture)) + Injector::inst()->createWithArgs(Cookie_Backend::class, array($fixture)) ); - $this->assertInstanceOf('SilverStripe\\Control\\HTTPResponse', $getresponse, 'Director::test() returns HTTPResponse'); + $this->assertInstanceOf(HTTPResponse::class, $getresponse, 'Director::test() returns HTTPResponse'); $this->assertEquals($fixture['somekey'], $getresponse->getBody(), 'Director::test() ' . $testfunction); } } @@ -402,6 +406,7 @@ class DirectorTest extends SapphireTest */ public function testRouteParams() { + /** @var HTTPRequest $request */ Director::test('en-nz/myaction/myid/myotherid', null, null, null, null, null, null, $request); $this->assertEquals( @@ -436,7 +441,7 @@ class DirectorTest extends SapphireTest public function testForceSSLOnSubPagesPattern() { - $_SERVER['REQUEST_URI'] = Director::baseURL() . Config::inst()->get('SilverStripe\\Security\\Security', 'login_url'); + $_SERVER['REQUEST_URI'] = Director::baseURL() . Config::inst()->get(Security::class, 'login_url'); $output = Director::forceSSL(array('/^Security/')); $this->assertEquals($output, 'https://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']); } @@ -454,7 +459,7 @@ class DirectorTest extends SapphireTest public function testForceSSLAlternateDomain() { - Config::inst()->update('SilverStripe\\Control\\Director', 'alternate_base_url', '/'); + Director::config()->set('alternate_base_url', '/'); $_SERVER['REQUEST_URI'] = Director::baseURL() . 'admin'; $output = Director::forceSSL(array('/^admin/'), 'secure.mysite.com'); $this->assertEquals($output, 'https://secure.mysite.com/admin'); @@ -565,6 +570,7 @@ class DirectorTest extends SapphireTest //test that hashes are ignored $url = "TestController/returnGetValue?somekey=key"; $hash = "#test"; + /** @var HTTPRequest $request */ $response = Director::test($url . $hash, null, null, null, null, null, null, $request); $this->assertFalse($response->isError()); $this->assertEquals('key', $response->getBody()); @@ -584,25 +590,25 @@ class DirectorTest extends SapphireTest $processor = new RequestProcessor(array($filter)); - Injector::inst()->registerService($processor, 'SilverStripe\\Control\\RequestProcessor'); + Injector::inst()->registerService($processor, RequestProcessor::class); - $response = Director::test('some-dummy-url'); + Director::test('some-dummy-url'); $this->assertEquals(1, $filter->preCalls); $this->assertEquals(1, $filter->postCalls); $filter->failPost = true; - $this->setExpectedException('SilverStripe\\Control\\HTTPResponse_Exception'); + $this->setExpectedException(HTTPResponse_Exception::class); - $response = Director::test('some-dummy-url'); + Director::test('some-dummy-url'); $this->assertEquals(2, $filter->preCalls); $this->assertEquals(2, $filter->postCalls); $filter->failPre = true; - $response = Director::test('some-dummy-url'); + Director::test('some-dummy-url'); $this->assertEquals(3, $filter->preCalls); diff --git a/tests/php/Control/DirectorTest/TestController.php b/tests/php/Control/DirectorTest/TestController.php index a8b076b7a..eda404f36 100644 --- a/tests/php/Control/DirectorTest/TestController.php +++ b/tests/php/Control/DirectorTest/TestController.php @@ -7,6 +7,7 @@ use SilverStripe\Dev\TestOnly; class TestController extends Controller implements TestOnly { + private static $url_segment = 'TestController'; private static $allowed_actions = array( 'returnGetValue', diff --git a/tests/php/Control/RequestHandlingTest/AllowedController.php b/tests/php/Control/RequestHandlingTest/AllowedController.php index 210912758..231cd45fb 100644 --- a/tests/php/Control/RequestHandlingTest/AllowedController.php +++ b/tests/php/Control/RequestHandlingTest/AllowedController.php @@ -10,6 +10,8 @@ use SilverStripe\Dev\TestOnly; */ class AllowedController extends Controller implements TestOnly { + private static $url_segment = 'AllowedController'; + private static $url_handlers = array( // The double-slash is need here to ensure that '$Action//$ID/$OtherID' => "handleAction", diff --git a/tests/php/Control/RequestHandlingTest/ControllerFormWithAllowedActions.php b/tests/php/Control/RequestHandlingTest/ControllerFormWithAllowedActions.php index 924e33e84..76f83faae 100644 --- a/tests/php/Control/RequestHandlingTest/ControllerFormWithAllowedActions.php +++ b/tests/php/Control/RequestHandlingTest/ControllerFormWithAllowedActions.php @@ -9,11 +9,13 @@ use SilverStripe\Forms\FormAction; class ControllerFormWithAllowedActions extends Controller implements TestOnly { + private static $url_segment = 'ControllerFormWithAllowedActions'; + private static $allowed_actions = array('Form'); /** - * @skipUpgrade -*/ + * @skipUpgrade + */ public function Form() { return new FormWithAllowedActions( diff --git a/tests/php/Control/RequestHandlingTest/FieldController.php b/tests/php/Control/RequestHandlingTest/FieldController.php index e111edd6d..282c6e9e5 100644 --- a/tests/php/Control/RequestHandlingTest/FieldController.php +++ b/tests/php/Control/RequestHandlingTest/FieldController.php @@ -14,6 +14,7 @@ use SilverStripe\Forms\FormAction; */ class FieldController extends Controller implements TestOnly { + private static $url_segment = 'FieldController'; private static $allowed_actions = array('TestForm'); diff --git a/tests/php/Control/RequestHandlingTest/FormActionController.php b/tests/php/Control/RequestHandlingTest/FormActionController.php index 39a902968..8bf499c79 100644 --- a/tests/php/Control/RequestHandlingTest/FormActionController.php +++ b/tests/php/Control/RequestHandlingTest/FormActionController.php @@ -14,6 +14,8 @@ class FormActionController extends Controller implements TestOnly { protected $template = 'BlankPage'; + private static $url_segment = 'FormActionController'; + private static $allowed_actions = array( 'controlleraction', 'Form', @@ -32,8 +34,8 @@ class FormActionController extends Controller implements TestOnly } /** - * @skipUpgrade -*/ + * @skipUpgrade + */ public function Form() { return new Form( diff --git a/tests/php/Control/RequestHandlingTest/FormWithAllowedActions.php b/tests/php/Control/RequestHandlingTest/FormWithAllowedActions.php index 13602fefd..d22287799 100644 --- a/tests/php/Control/RequestHandlingTest/FormWithAllowedActions.php +++ b/tests/php/Control/RequestHandlingTest/FormWithAllowedActions.php @@ -7,17 +7,8 @@ use SilverStripe\Forms\Form; class FormWithAllowedActions extends Form implements TestOnly { - private static $allowed_actions = array( - 'allowedformaction' => 1, - ); - - public function allowedformaction() + protected function buildRequestHandler() { - return 'allowedformaction'; - } - - public function disallowedformaction() - { - return 'disallowedformaction'; + return FormWithAllowedActionsHandler::create($this); } } diff --git a/tests/php/Control/RequestHandlingTest/FormWithAllowedActionsHandler.php b/tests/php/Control/RequestHandlingTest/FormWithAllowedActionsHandler.php new file mode 100644 index 000000000..389231840 --- /dev/null +++ b/tests/php/Control/RequestHandlingTest/FormWithAllowedActionsHandler.php @@ -0,0 +1,27 @@ + 1, + ); + + public function allowedformaction() + { + return 'allowedformaction'; + } + + public function disallowedformaction() + { + return 'disallowedformaction'; + } +} diff --git a/tests/php/Control/RequestHandlingTest/TestController.php b/tests/php/Control/RequestHandlingTest/TestController.php index a10c75cf5..45c2a460d 100644 --- a/tests/php/Control/RequestHandlingTest/TestController.php +++ b/tests/php/Control/RequestHandlingTest/TestController.php @@ -15,6 +15,7 @@ use SilverStripe\View\SSViewer; */ class TestController extends Controller implements TestOnly { + private static $url_segment = 'TestController'; private static $allowed_actions = array( 'method', diff --git a/tests/php/Control/RequestHandlingTest/TestForm.php b/tests/php/Control/RequestHandlingTest/TestForm.php index f264a8004..a39e1dd1b 100644 --- a/tests/php/Control/RequestHandlingTest/TestForm.php +++ b/tests/php/Control/RequestHandlingTest/TestForm.php @@ -10,31 +10,8 @@ use SilverStripe\Forms\Form; */ class TestForm extends Form implements TestOnly { - private static $url_handlers = array( - 'fields/$FieldName' => 'handleField', - "POST " => "handleSubmission", - "GET " => "handleGet", - ); - - // These are a different case from those in url_handlers to confirm that it's all case-insensitive - private static $allowed_actions = array( - 'handlesubmission', - 'handlefield', - 'handleget', - ); - - public function handleField($request) + protected function buildRequestHandler() { - return $this->Fields()->dataFieldByName($request->param('FieldName')); - } - - public function handleSubmission($request) - { - return "Form posted"; - } - - public function handleGet($request) - { - return "Get request on form"; + return TestFormHandler::create($this); } } diff --git a/tests/php/Control/RequestHandlingTest/TestFormHandler.php b/tests/php/Control/RequestHandlingTest/TestFormHandler.php new file mode 100644 index 000000000..eae42fc8f --- /dev/null +++ b/tests/php/Control/RequestHandlingTest/TestFormHandler.php @@ -0,0 +1,41 @@ + 'handleField', + "POST " => "handleSubmission", + "GET " => "handleGet", + ); + + // These are a different case from those in url_handlers to confirm that it's all case-insensitive + private static $allowed_actions = array( + 'handlesubmission', + 'handlefield', + 'handleget', + ); + + public function handleField($request) + { + return $this->form->Fields()->dataFieldByName($request->param('FieldName')); + } + + public function handleSubmission($request) + { + return "Form posted"; + } + + public function handleGet($request) + { + return "Get request on form"; + } +} diff --git a/tests/php/Forms/DropdownFieldTest.php b/tests/php/Forms/DropdownFieldTest.php index b26f20919..622141778 100644 --- a/tests/php/Forms/DropdownFieldTest.php +++ b/tests/php/Forms/DropdownFieldTest.php @@ -463,6 +463,9 @@ class DropdownFieldTest extends SapphireTest return $foundDisabled; } + /** + * @skipUpgrade + */ public function testValidation() { $field = DropdownField::create( @@ -475,10 +478,7 @@ class DropdownFieldTest extends SapphireTest ) ); $validator = new RequiredFields(); - /** - * @skipUpgrade -*/ - $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); + $form = new Form(null, 'Form', new FieldList($field), new FieldList(), $validator); $field->setValue("One"); $this->assertTrue($field->validate($validator)); $field->setName("TestNew"); //try changing name of field diff --git a/tests/php/Forms/EmbedShortcodeProviderTest.php b/tests/php/Forms/EmbedShortcodeProviderTest.php index c36c16678..67d11e511 100644 --- a/tests/php/Forms/EmbedShortcodeProviderTest.php +++ b/tests/php/Forms/EmbedShortcodeProviderTest.php @@ -29,9 +29,7 @@ class EmbedShortcodeProviderTest extends SapphireTest public function testYoutube() { - /** - * @var Webpage $result -*/ + /** @var Webpage $result */ $result = Embed::create(self::$test_youtube, array()); self::assertEquals($result->providerName, 'YouTube'); $embedded = EmbedShortcodeProvider::embedForTemplate($result); @@ -44,9 +42,7 @@ class EmbedShortcodeProviderTest extends SapphireTest public function testSoundcloud() { - /** - * @var Webpage $result -*/ + /** @var Webpage $result */ $result = Embed::create(self::$test_soundcloud, array()); self::assertEquals($result->providerName, 'SoundCloud'); $embedded = EmbedShortcodeProvider::embedForTemplate($result); diff --git a/tests/php/Forms/FileFieldTest.php b/tests/php/Forms/FileFieldTest.php index 33cf3bbb6..23b8038ab 100644 --- a/tests/php/Forms/FileFieldTest.php +++ b/tests/php/Forms/FileFieldTest.php @@ -14,12 +14,11 @@ class FileFieldTest extends FunctionalTest /** * Test a valid upload of a required file in a form. Error is set to 0, as the upload went well + * + * @skipUpgrade */ public function testUploadRequiredFile() { - /** - * @skipUpgrade -*/ $form = new Form( new Controller(), 'Form', @@ -42,12 +41,10 @@ class FileFieldTest extends FunctionalTest /** * Test different scenarii for a failed upload : an error occured, no files where provided + * @skipUpgrade */ public function testUploadMissingRequiredFile() { - /** - * @skipUpgrade -*/ $form = new Form( new Controller(), 'Form', diff --git a/tests/php/Forms/FormFieldTest.php b/tests/php/Forms/FormFieldTest.php index 97b442a86..4641750e1 100644 --- a/tests/php/Forms/FormFieldTest.php +++ b/tests/php/Forms/FormFieldTest.php @@ -362,7 +362,7 @@ class FormFieldTest extends SapphireTest { $field = new FormField('MyField', 'My Field'); $validator = new RequiredFields('MyField'); - $form = new Form(new Controller(), 'TestForm', new FieldList($field), new FieldList(), $validator); + $form = new Form(null, 'TestForm', new FieldList($field), new FieldList(), $validator); $form->validationResult(); $schema = $field->getSchemaState(); $this->assertEquals( diff --git a/tests/php/Forms/FormScaffolderTest.php b/tests/php/Forms/FormScaffolderTest.php index 05bfbf65d..3cbc2489a 100644 --- a/tests/php/Forms/FormScaffolderTest.php +++ b/tests/php/Forms/FormScaffolderTest.php @@ -37,7 +37,7 @@ class FormScaffolderTest extends SapphireTest { $article = new Article; $fields = $article->getCMSFields(); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom($article); $this->assertTrue( @@ -67,7 +67,7 @@ class FormScaffolderTest extends SapphireTest $article1 = $this->objFromFixture(Article::class, 'article1'); $fields = $article1->getCMSFields(); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom($article1); $this->assertNotNull( @@ -101,7 +101,7 @@ class FormScaffolderTest extends SapphireTest $article1 = $this->objFromFixture(Article::class, 'article1'); $fields = $article1->getCMSFields(); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom($article1); $this->assertNotNull( @@ -119,7 +119,7 @@ class FormScaffolderTest extends SapphireTest 'restrictFields' => array('Title') ) ); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom($article1); $this->assertNotNull( @@ -141,7 +141,7 @@ class FormScaffolderTest extends SapphireTest 'fieldClasses' => array('Title' => 'SilverStripe\\Forms\\HTMLEditor\\HTMLEditorField') ) ); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom($article1); $this->assertNotNull( @@ -157,7 +157,7 @@ class FormScaffolderTest extends SapphireTest public function testGetFormFields() { $fields = Article::singleton()->getFrontEndFields(); - $form = new Form(new Controller(), 'TestForm', $fields, new FieldList()); + $form = new Form(null, 'TestForm', $fields, new FieldList()); $form->loadDataFrom(singleton(Article::class)); $this->assertFalse($fields->hasTabSet(), 'getFrontEndFields() doesnt produce a TabSet by default'); diff --git a/tests/php/Forms/FormSchemaTest.php b/tests/php/Forms/FormSchemaTest.php index 1421ae86e..3073482f2 100644 --- a/tests/php/Forms/FormSchemaTest.php +++ b/tests/php/Forms/FormSchemaTest.php @@ -20,16 +20,16 @@ class FormSchemaTest extends SapphireTest public function testGetSchema() { - $form = new Form(new Controller(), 'TestForm', new FieldList(), new FieldList()); + $form = new Form(null, 'TestForm', new FieldList(), new FieldList()); $formSchema = new FormSchema(); $expected = [ 'name' => 'TestForm', 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'attributes' => [ 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'enctype' => 'application/x-www-form-urlencoded', 'target' => null, @@ -67,7 +67,7 @@ class FormSchemaTest extends SapphireTest public function testGetState() { - $form = new Form(new Controller(), 'TestForm', new FieldList(), new FieldList()); + $form = new Form(null, 'TestForm', new FieldList(), new FieldList()); $formSchema = new FormSchema(); $expected = [ 'id' => 'Form_TestForm', @@ -92,7 +92,7 @@ class FormSchemaTest extends SapphireTest { $fields = new FieldList(); $actions = new FieldList(); - $form = new Form(new Controller(), 'TestForm', $fields, $actions); + $form = new Form(null, 'TestForm', $fields, $actions); $form->sessionMessage('All saved', 'good'); $formSchema = new FormSchema(); $expected = [ @@ -122,7 +122,7 @@ class FormSchemaTest extends SapphireTest $fields = new FieldList(new TextField('Title')); $actions = new FieldList(); $validator = new RequiredFields('Title'); - $form = new Form(new Controller(), 'TestForm', $fields, $actions, $validator); + $form = new Form(null, 'TestForm', $fields, $actions, $validator); $form->loadDataFrom( [ 'Title' => null, @@ -159,10 +159,13 @@ class FormSchemaTest extends SapphireTest $this->assertJsonStringEqualsJsonString(json_encode($expected), json_encode($state)); } + /** + * @skipUpgrade + */ public function testGetNestedSchema() { $form = new Form( - new Controller(), + null, 'TestForm', new FieldList(new TextField("Name")), new FieldList( @@ -180,17 +183,14 @@ class FormSchemaTest extends SapphireTest ) ); $formSchema = new FormSchema(); - /** - * @skipUpgrade -*/ $expected = [ 'name' => 'TestForm', 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'attributes' => [ 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'enctype' => 'application/x-www-form-urlencoded', 'target' => null, @@ -392,7 +392,7 @@ class FormSchemaTest extends SapphireTest public function testSchemaValidation() { $form = new Form( - new Controller(), + null, 'TestForm', new FieldList( TextField::create("Name") @@ -409,12 +409,12 @@ class FormSchemaTest extends SapphireTest $expected = [ 'name' => 'TestForm', 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'attributes' => [ 'id' => 'Form_TestForm', - 'action' => 'Controller/TestForm', + 'action' => null, 'method' => 'POST', 'enctype' => 'application/x-www-form-urlencoded', 'target' => null, diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 03c951290..ac6db41ad 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -757,10 +757,13 @@ class FormTest extends FunctionalTest $this->assertEquals('bar', $attrs['foo']); } + /** + * @skipUpgrade + */ public function testButtonClicked() { $form = $this->getStubForm(); - $action = $form->buttonClicked(); + $action = $form->getRequestHandler()->buttonClicked(); $this->assertNull($action); $controller = new FormTest\TestController(); @@ -776,13 +779,10 @@ class FormTest extends FunctionalTest ) ); - $form->httpSubmission($request); - $button = $form->buttonClicked(); - $this->assertInstanceOf('SilverStripe\\Forms\\FormAction', $button); + $form->getRequestHandler()->httpSubmission($request); + $button = $form->getRequestHandler()->buttonClicked(); + $this->assertInstanceOf(FormAction::class, $button); $this->assertEquals('doSubmit', $button->actionName()); - /** - * @skipUpgrade -*/ $form = new Form( $controller, 'Form', @@ -799,9 +799,9 @@ class FormTest extends FunctionalTest ) ); - $form->httpSubmission($request); - $button = $form->buttonClicked(); - $this->assertInstanceOf('SilverStripe\\Forms\\FormAction', $button); + $form->getRequestHandler()->httpSubmission($request); + $button = $form->getRequestHandler()->buttonClicked(); + $this->assertInstanceOf(FormAction::class, $button); $this->assertEquals('doSubmit', $button->actionName()); } @@ -814,7 +814,7 @@ class FormTest extends FunctionalTest new FieldList(), new FieldList(new FormAction('actionName', 'Action')) ); - $this->assertTrue($form->checkAccessAction('actionName')); + $this->assertTrue($form->getRequestHandler()->checkAccessAction('actionName')); $form = new Form( $controller, @@ -822,7 +822,7 @@ class FormTest extends FunctionalTest new FieldList(new FormAction('inlineAction', 'Inline action')), new FieldList() ); - $this->assertTrue($form->checkAccessAction('inlineAction')); + $this->assertTrue($form->getRequestHandler()->checkAccessAction('inlineAction')); } public function testAttributesHTML() diff --git a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php index d10591298..603f00707 100644 --- a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php +++ b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php @@ -62,7 +62,7 @@ class GridFieldDeleteActionTest extends SapphireTest $this->list = new DataList(Team::class); $config = GridFieldConfig::create()->addComponent(new GridFieldDeleteAction()); $this->gridField = new GridField('testfield', 'testfield', $this->list, $config); - $this->form = new Form(new Controller(), 'mockform', new FieldList(array($this->gridField)), new FieldList()); + $this->form = new Form(null, 'mockform', new FieldList(array($this->gridField)), new FieldList()); } public function testDontShowDeleteButtons() @@ -185,7 +185,7 @@ class GridFieldDeleteActionTest extends SapphireTest $config = GridFieldConfig::create()->addComponent(new GridFieldDeleteAction(true)); $gridField = new GridField('testfield', 'testfield', $this->list, $config); - $form = new Form(new Controller(), 'mockform', new FieldList(array($this->gridField)), new FieldList()); + $form = new Form(null, 'mockform', new FieldList(array($this->gridField)), new FieldList()); $stateID = 'testGridStateActionField'; Session::set( diff --git a/tests/php/Forms/GridField/GridFieldEditButtonTest.php b/tests/php/Forms/GridField/GridFieldEditButtonTest.php index bbf4a81fd..b434738fb 100644 --- a/tests/php/Forms/GridField/GridFieldEditButtonTest.php +++ b/tests/php/Forms/GridField/GridFieldEditButtonTest.php @@ -57,7 +57,7 @@ class GridFieldEditButtonTest extends SapphireTest $this->list = new DataList(Team::class); $config = GridFieldConfig::create()->addComponent(new GridFieldEditButton()); $this->gridField = new GridField('testfield', 'testfield', $this->list, $config); - $this->form = new Form(new Controller(), 'mockform', new FieldList(array($this->gridField)), new FieldList()); + $this->form = new Form(null, 'mockform', new FieldList(array($this->gridField)), new FieldList()); } public function testShowEditLinks() diff --git a/tests/php/Forms/GridField/GridFieldPaginatorTest.php b/tests/php/Forms/GridField/GridFieldPaginatorTest.php index 5f64ee4ee..f88558946 100644 --- a/tests/php/Forms/GridField/GridFieldPaginatorTest.php +++ b/tests/php/Forms/GridField/GridFieldPaginatorTest.php @@ -59,7 +59,7 @@ class GridFieldPaginatorTest extends FunctionalTest new GridFieldPageCount('toolbar-header-right') ); $this->gridField = new GridField('testfield', 'testfield', $this->list, $config); - $this->form = new Form(new Controller(), 'mockform', new FieldList(array($this->gridField)), new FieldList()); + $this->form = new Form(null, 'mockform', new FieldList(array($this->gridField)), new FieldList()); } public function testThereIsPaginatorWhenMoreThanOnePage() diff --git a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php index 9fc9562db..0c4da52db 100644 --- a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php @@ -11,7 +11,6 @@ use SilverStripe\Forms\Tests\GridField\GridFieldSortableHeaderTest\Mom; use SilverStripe\ORM\DataList; use SilverStripe\Core\Convert; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Control\Controller; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor; @@ -32,6 +31,8 @@ class GridFieldSortableHeaderTest extends SapphireTest /** * Tests that the appropriate sortable headers are generated + * + * @skipUpgrade */ public function testRenderHeaders() { @@ -39,10 +40,7 @@ class GridFieldSortableHeaderTest extends SapphireTest // Generate sortable header and extract HTML $list = new DataList(Team::class); $config = new GridFieldConfig_RecordEditor(); - /** - * @skipUpgrade -*/ - $form = new Form(Controller::curr(), 'Form', new FieldList(), new FieldList()); + $form = new Form(null, 'Form', new FieldList(), new FieldList()); $gridField = new GridField('testfield', 'testfield', $list, $config); $gridField->setForm($form); $compontent = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class); diff --git a/tests/php/Forms/GridField/GridFieldTest.php b/tests/php/Forms/GridField/GridFieldTest.php index 86dbbc1c0..24e6ec44f 100644 --- a/tests/php/Forms/GridField/GridFieldTest.php +++ b/tests/php/Forms/GridField/GridFieldTest.php @@ -2,7 +2,6 @@ namespace SilverStripe\Forms\Tests\GridField; -use SilverStripe\Control\Controller; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\FieldList; @@ -415,7 +414,7 @@ class GridFieldTest extends SapphireTest // $obj = new GridField('testfield', 'testfield', ArrayList::create(), $config); // $id = 'testGridStateActionField'; // Session::set($id, array('grid'=>'', 'actionName'=>'jump')); - // $form = new Form(new Controller(), 'mockform', new FieldList(array($obj)), new FieldList()); + // $form = new Form(null, 'mockform', new FieldList(array($obj)), new FieldList()); // $request = new HTTPRequest('POST', 'url'); // $obj->gridFieldAlterAction(array('StateID'=>$id), $form, $request); } @@ -460,7 +459,7 @@ class GridFieldTest extends SapphireTest ) ); $field = new GridField('testfield', 'testfield', ArrayList::create(), $config); - $form = new Form(new Controller(), 'testform', new FieldList(array($field)), new FieldList()); + $form = new Form(null, 'testform', new FieldList(array($field)), new FieldList()); $this->assertContains( "
rightone\nrighttwo
left
", @@ -496,7 +495,7 @@ class GridFieldTest extends SapphireTest ) ); $field = new GridField('testfield', 'testfield', ArrayList::create(), $config); - $form = new Form(new Controller(), 'testform', new FieldList(array($field)), new FieldList()); + $form = new Form(null, 'testform', new FieldList(array($field)), new FieldList()); $this->assertContains( "
first\nsecond
", @@ -532,7 +531,7 @@ class GridFieldTest extends SapphireTest ) ); $field = new GridField('testfield', 'testfield', ArrayList::create(), $config); - $form = new Form(new Controller(), 'testform', new FieldList(array($field)), new FieldList()); + $form = new Form(null, 'testform', new FieldList(array($field)), new FieldList()); $this->setExpectedException('LogicException'); $field->FieldHolder(); @@ -573,7 +572,7 @@ class GridFieldTest extends SapphireTest $config = new GridFieldConfig(); $config->addComponent(new GridFieldDataColumns()); $obj = new GridField('testfield', 'testfield', $list, $config); - $form = new Form(new Controller(), 'mockform', new FieldList(array($obj)), new FieldList()); + $form = new Form(null, 'mockform', new FieldList(array($obj)), new FieldList()); $content = new CSSContentParser($obj->FieldHolder()); $members = $content->getBySelector('.ss-gridfield-item tr'); diff --git a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent.php b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent.php index 5d46c51fd..f2f22bdb4 100644 --- a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent.php +++ b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent.php @@ -7,6 +7,7 @@ use SilverStripe\Control\RequestHandler; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; +use SilverStripe\Forms\GridField\GridField; use SilverStripe\Forms\GridField\GridField_URLHandler; use SilverStripe\Forms\TextField; use SilverStripe\View\SSViewer; @@ -19,13 +20,16 @@ class TestComponent extends RequestHandler implements GridField_URLHandler private static $allowed_actions = array('Form', 'showform', 'testpage', 'handleItem'); + /** + * @var GridField + */ protected $gridField; + /** + * @skipUpgrade + */ public function getURLHandlers($gridField) { - /** - * @skipUpgrade -*/ return array( 'showform' => 'showform', 'testpage' => 'testpage', @@ -44,7 +48,7 @@ class TestComponent extends RequestHandler implements GridField_URLHandler ); } - public function Link() + public function Link($action = null) { return $this->gridField->Link(); } @@ -54,12 +58,12 @@ class TestComponent extends RequestHandler implements GridField_URLHandler return "" . SSViewer::get_base_tag("") . "" . $this->Form($gridField, $request)->forTemplate(); } + /** + * @skipUpgrade + */ public function Form($gridField, $request) { $this->gridField = $gridField; - /** - * @skipUpgrade -*/ return new Form( $this, 'Form', diff --git a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent_ItemRequest.php b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent_ItemRequest.php index 1f460a97d..e042dec8f 100644 --- a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent_ItemRequest.php +++ b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent_ItemRequest.php @@ -28,7 +28,7 @@ class TestComponent_ItemRequest extends RequestHandler parent::__construct(); } - public function Link() + public function Link($action = null) { return $this->link; } @@ -40,12 +40,10 @@ class TestComponent_ItemRequest extends RequestHandler public function Form() { - /** - * @skipUpgrade -*/ + /** @skipUpgrade */ return new Form( $this, - 'Form', + Form::DEFAULT_NAME, new FieldList( new TextField("Test") ), diff --git a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestController.php b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestController.php index 8d2401956..9329e4623 100644 --- a/tests/php/Forms/GridField/GridField_URLHandlerTest/TestController.php +++ b/tests/php/Forms/GridField/GridField_URLHandlerTest/TestController.php @@ -22,6 +22,10 @@ class TestController extends Controller implements TestOnly private static $allowed_actions = array('Form'); + /** + * @skipUpgrade + * @return Form + */ public function Form() { $gridConfig = GridFieldConfig::create(); @@ -30,9 +34,6 @@ class TestController extends Controller implements TestOnly $gridData = new ArrayList(); $gridField = new GridField('Grid', 'My grid', $gridData, $gridConfig); - /** - * @skipUpgrade -*/ return new Form( $this, 'Form', diff --git a/tests/php/Forms/OptionsetFieldTest.php b/tests/php/Forms/OptionsetFieldTest.php index c93226f75..98914013a 100644 --- a/tests/php/Forms/OptionsetFieldTest.php +++ b/tests/php/Forms/OptionsetFieldTest.php @@ -34,6 +34,9 @@ class OptionsetFieldTest extends SapphireTest ); } + /** + * @skipUpgrade + */ public function testValidation() { $field = OptionsetField::create( @@ -46,10 +49,7 @@ class OptionsetFieldTest extends SapphireTest ) ); $validator = new RequiredFields('Test'); - /** - * @skipUpgrade -*/ - $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); + $form = new Form(null, 'Form', new FieldList($field), new FieldList(), $validator); $field->setValue("One"); $this->assertTrue($field->validate($validator)); diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php index 54c31181d..6fd5ad3f0 100644 --- a/tests/php/Security/SecurityTest.php +++ b/tests/php/Security/SecurityTest.php @@ -389,8 +389,8 @@ class SecurityTest extends FunctionalTest $expiredResponse = $this->doTestLoginForm('expired@silverstripe.com', '1nitialPassword'); $this->assertEquals(302, $expiredResponse->getStatusCode()); $this->assertEquals( - '/Security/changepassword', - $expiredResponse->getHeader('Location') + Director::absoluteURL('Security/changepassword').'?BackURL=test%2Flink', + Director::absoluteURL($expiredResponse->getHeader('Location')) ); $this->assertEquals( $this->idFromFixture(Member::class, 'expiredpassword'), @@ -456,7 +456,10 @@ class SecurityTest extends FunctionalTest // Check. $response = $this->get('Security/changepassword/?m='.$admin->ID.'&t=' . $token); $this->assertEquals(302, $response->getStatusCode()); - $this->assertEquals(Director::baseUrl() . 'Security/changepassword', $response->getHeader('Location')); + $this->assertEquals( + Director::absoluteURL('Security/changepassword'), + Director::absoluteURL($response->getHeader('Location')) + ); // Follow redirection to form without hash in GET parameter $response = $this->get('Security/changepassword'); @@ -756,9 +759,6 @@ class SecurityTest extends FunctionalTest { $result = $this->session()->inst_get('FormInfo.MemberLoginForm_LoginForm.result'); if ($result) { - /** - * @var ValidationResult $resultObj -*/ return unserialize($result); } return null;