Skip to content
Permalink
Browse files

API Refactor Form request handling into FormRequestHandler

API Add HasRequestHandler interface
API Refactor Link() and url handling behaviour from Controller into RequestHandler
API RequestHandler classes now must define url_segment to have a default Link()
API Clean up redirectBack()
  • Loading branch information...
Damian Mooyman
Damian Mooyman committed Mar 2, 2017
1 parent f520a84 commit 0c41a97a8b1ecde2341122b49ebc58b9e6e2a9e0
Showing with 1,560 additions and 1,219 deletions.
  1. +2 −2 admin/code/CMSBatchActionHandler.php
  2. +26 −1 docs/en/04_Changelogs/4.0.0.md
  3. +3 −61 src/Control/Controller.php
  4. +0 −4 src/Control/Director.php
  5. +2 −0 src/Control/HTTPResponse.php
  6. +16 −0 src/Control/HasRequestHandler.php
  7. +133 −14 src/Control/RequestHandler.php
  8. +18 −0 src/Core/ClassInfo.php
  9. +114 −435 src/Forms/Form.php
  10. +510 −0 src/Forms/FormRequestHandler.php
  11. +8 −1 src/Forms/GridField/GridField.php
  12. +2 −2 src/Forms/GridField/GridFieldDetailForm.php
  13. +0 −5 src/Forms/GridField/GridFieldDetailForm_ItemRequest.php
  14. +3 −108 src/Security/CMSMemberLoginForm.php
  15. +93 −0 src/Security/CMSMemberLoginHandler.php
  16. +8 −102 src/Security/ChangePasswordForm.php
  17. +103 −0 src/Security/ChangePasswordHandler.php
  18. +3 −212 src/Security/MemberLoginForm.php
  19. +240 −0 src/Security/MemberLoginHandler.php
  20. +61 −91 src/Security/Security.php
  21. +1 −1 src/View/ViewableData.php
  22. +0 −2 tests/bootstrap/phpunit.php
  23. +10 −27 tests/php/Control/ControllerTest.php
  24. +2 −0 tests/php/Control/ControllerTest/AccessBaseController.php
  25. +2 −2 tests/php/Control/ControllerTest/AccessSecuredController.php
  26. +2 −2 tests/php/Control/ControllerTest/AccessWildcardSecuredController.php
  27. +1 −0 tests/php/Control/ControllerTest/ContainerController.php
  28. +1 −0 tests/php/Control/ControllerTest/HasAction.php
  29. +1 −0 tests/php/Control/ControllerTest/HasAction_Unsecured.php
  30. +2 −2 tests/php/Control/ControllerTest/IndexSecuredController.php
  31. +1 −0 tests/php/Control/ControllerTest/SubController.php
  32. +2 −0 tests/php/Control/ControllerTest/TestController.php
  33. +1 −0 tests/php/Control/ControllerTest/UnsecuredController.php
  34. +21 −15 tests/php/Control/DirectorTest.php
  35. +1 −0 tests/php/Control/DirectorTest/TestController.php
  36. +2 −0 tests/php/Control/RequestHandlingTest/AllowedController.php
  37. +4 −2 tests/php/Control/RequestHandlingTest/ControllerFormWithAllowedActions.php
  38. +1 −0 tests/php/Control/RequestHandlingTest/FieldController.php
  39. +4 −2 tests/php/Control/RequestHandlingTest/FormActionController.php
  40. +2 −11 tests/php/Control/RequestHandlingTest/FormWithAllowedActions.php
  41. +27 −0 tests/php/Control/RequestHandlingTest/FormWithAllowedActionsHandler.php
  42. +1 −0 tests/php/Control/RequestHandlingTest/TestController.php
  43. +2 −25 tests/php/Control/RequestHandlingTest/TestForm.php
  44. +41 −0 tests/php/Control/RequestHandlingTest/TestFormHandler.php
  45. +4 −4 tests/php/Forms/DropdownFieldTest.php
  46. +2 −6 tests/php/Forms/EmbedShortcodeProviderTest.php
  47. +3 −6 tests/php/Forms/FileFieldTest.php
  48. +1 −1 tests/php/Forms/FormFieldTest.php
  49. +6 −6 tests/php/Forms/FormScaffolderTest.php
  50. +15 −15 tests/php/Forms/FormSchemaTest.php
  51. +12 −12 tests/php/Forms/FormTest.php
  52. +2 −2 tests/php/Forms/GridField/GridFieldDeleteActionTest.php
  53. +1 −1 tests/php/Forms/GridField/GridFieldEditButtonTest.php
  54. +1 −1 tests/php/Forms/GridField/GridFieldPaginatorTest.php
  55. +3 −5 tests/php/Forms/GridField/GridFieldSortableHeaderTest.php
  56. +5 −6 tests/php/Forms/GridField/GridFieldTest.php
  57. +11 −7 tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent.php
  58. +3 −5 tests/php/Forms/GridField/GridField_URLHandlerTest/TestComponent_ItemRequest.php
  59. +4 −3 tests/php/Forms/GridField/GridField_URLHandlerTest/TestController.php
  60. +4 −4 tests/php/Forms/OptionsetFieldTest.php
  61. +6 −6 tests/php/Security/SecurityTest.php
@@ -101,9 +101,9 @@ public static function register($urlSegment, $batchActionClass, $recordClass = S
);
}
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, '/');
}
/**
@@ -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.
#### <a name="overview-general-removed"></a>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.
#### <a name="overview-general-deprecated"></a>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`
@@ -132,17 +132,6 @@ public function doInit()
$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 @@ public function redirect($url, $code = 302)
. "; 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;
}
/**
@@ -424,10 +424,6 @@ protected static function handleRequest(HTTPRequest $request, Session $session,
{
$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) == '->') {
@@ -4,6 +4,7 @@
use InvalidArgumentException;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\View\Requirements;
@@ -12,6 +13,7 @@
*/
class HTTPResponse
{
use Injectable;
/**
* @var array
@@ -0,0 +1,16 @@
<?php
namespace SilverStripe\Control;
/**
* Indicator for a class which cannot handle requests directly, but is able to
* generate a delegate for those requests.
*/
interface HasRequestHandler
{
/**
* @return RequestHandler
*/
public function getRequestHandler();
}
@@ -3,6 +3,7 @@
namespace SilverStripe\Control;
use InvalidArgumentException;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Object;
use SilverStripe\Dev\Debug;
@@ -47,6 +48,13 @@
*/
class RequestHandler extends ViewableData
{
/**
* Optional url_segment for this request handler
*
* @config
* @var string|null
*/
private static $url_segment = null;
/**
* @var HTTPRequest $request The request object that the controller was called with.
@@ -110,16 +118,6 @@ class RequestHandler extends ViewableData
*/
private static $allowed_actions = null;
/**
* @config
* @var boolean Enforce presence of $allowed_actions when checking acccess.
* Defaults to TRUE, meaning all URL actions will be denied.
* When set to FALSE, the controller will allow *all* public methods to be called.
* In most cases this isn't desireable, and in fact a security risk,
* since some helper methods can cause side effects which shouldn't be exposed through URLs.
*/
private static $require_allowed_actions = true;
public function __construct()
{
$this->brokenOnConstruct = false;
@@ -235,7 +233,11 @@ public function handleRequest(HTTPRequest $request, DataModel $model)
// 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 @@ public function checkAccessAction($action)
$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 @@ public function checkAccessAction($action)
*/
public function httpError($errorCode, $errorMessage = null)
{
$request = $this->getRequest();
// Call a handler method such as onBeforeHTTPError404
@@ -527,7 +528,7 @@ public function httpError($errorCode, $errorMessage = null)
* {@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 @@ public function setRequest($request)
$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);
}
}
Oops, something went wrong.

0 comments on commit 0c41a97

Please sign in to comment.
You can’t perform that action at this time.