mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
API Form::setStrictFormMethodCheck() and strict argument to setFormMethod()
Thanks to @sminnee for getting this started
This commit is contained in:
parent
59be4a3be0
commit
14c59be85e
@ -339,6 +339,14 @@ or set on a form field instance via anyone of these methods:
|
||||
SilverStripe tries to protect users against *Cross-Site Request Forgery (CSRF)* by adding a hidden *SecurityID*
|
||||
parameter to each form. See [secure-development](/topics/security) for details.
|
||||
|
||||
In addition, you should limit forms to the intended HTTP verb (mostly `GET` or `POST`)
|
||||
to further reduce attack surface, by using `[api:Form->setStrictFormMethodCheck()]`.
|
||||
|
||||
:::php
|
||||
$myForm->setFormMethod('POST');
|
||||
$myForm->setStrictFormMethodCheck(true);
|
||||
$myForm->setFormMethod('POST', true); // alternative short notation
|
||||
|
||||
### Remove existing fields
|
||||
|
||||
If you want to remove certain fields from your subclass:
|
||||
|
@ -275,21 +275,14 @@ Some rules of thumb:
|
||||
|
||||
## Cross-Site Request Forgery (CSRF)
|
||||
|
||||
SilverStripe has built-in countermeasures against this type of identity theft for all form submissions. A form object
|
||||
will automatically contain a *SecurityID* parameter which is generated as a secure hash on the server, connected to the
|
||||
SilverStripe has built-in countermeasures against [CSRF](http://shiflett.org/articles/cross-site-request-forgeries) identity theft for all form submissions. A form object
|
||||
will automatically contain a `SecurityID` parameter which is generated as a secure hash on the server, connected to the
|
||||
currently active session of the user. If this form is submitted without this parameter, or if the parameter doesn't
|
||||
match the hash stored in the users session, the request is discarded.
|
||||
You can disable this behaviour through `[api:Form->disableSecurityToken()]`.
|
||||
|
||||
If you know what you're doing, you can disable this behaviour:
|
||||
|
||||
:::php
|
||||
$myForm->disableSecurityToken();
|
||||
|
||||
|
||||
See
|
||||
[http://shiflett.org/articles/cross-site-request-forgeries](http://shiflett.org/articles/cross-site-request-forgeries)
|
||||
|
||||
|
||||
It is also recommended to limit form submissions to the intended HTTP verb (mostly `GET` or `POST`)
|
||||
through `[api:Form->setStrictFormMethodCheck()]`.
|
||||
|
||||
## Casting user input
|
||||
|
||||
|
@ -66,6 +66,11 @@ class Form extends RequestHandler {
|
||||
|
||||
protected $formMethod = "post";
|
||||
|
||||
/**
|
||||
* @var boolean
|
||||
*/
|
||||
protected $strictFormMethodCheck = false;
|
||||
|
||||
protected static $current_action;
|
||||
|
||||
/**
|
||||
@ -239,7 +244,22 @@ class Form extends RequestHandler {
|
||||
* if the form is valid.
|
||||
*/
|
||||
public function httpSubmission($request) {
|
||||
// Strict method check
|
||||
if($this->strictFormMethodCheck) {
|
||||
|
||||
// Throws an error if the method is bad...
|
||||
if($this->formMethod != strtolower($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 vairables corresponding to that method type
|
||||
$vars = $this->formMethod == 'get' ? $request->getVars() : $request->postVars();
|
||||
} else {
|
||||
$vars = $request->requestVars();
|
||||
}
|
||||
|
||||
if(isset($funcName)) {
|
||||
Form::set_current_action($funcName);
|
||||
}
|
||||
@ -794,12 +814,38 @@ class Form extends RequestHandler {
|
||||
* Set the form method: GET, POST, PUT, DELETE.
|
||||
*
|
||||
* @param $method string
|
||||
* @param $strict If non-null, pass value to {@link setStrictFormMethodCheck()}.
|
||||
*/
|
||||
public function setFormMethod($method) {
|
||||
public function setFormMethod($method, $strict = null) {
|
||||
$this->formMethod = strtolower($method);
|
||||
if($strict !== null) $this->setStrictFormMethodCheck($strict);
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* If set to true, enforce the matching of the form method.
|
||||
*
|
||||
* This will mean two things:
|
||||
* - GET vars will be ignored by a POST form, and vice versa
|
||||
* - A submission where the HTTP method used doesn't match the form will return a 400 error.
|
||||
*
|
||||
* If set to false (the default), then the form method is only used to construct the default
|
||||
* form.
|
||||
*
|
||||
* @param $bool boolean
|
||||
*/
|
||||
public function setStrictFormMethodCheck($bool) {
|
||||
$this->strictFormMethodCheck = (bool)$bool;
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return boolean
|
||||
*/
|
||||
public function getStrictFormMethodCheck() {
|
||||
return $this->strictFormMethodCheck;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the form's action attribute.
|
||||
* This is build by adding an executeForm get variable to the parent controller's Link() value
|
||||
|
@ -331,6 +331,24 @@ class FormTest extends FunctionalTest {
|
||||
$this->assertEquals(200, $response->getStatusCode(), 'Submission suceeds with security token');
|
||||
}
|
||||
|
||||
public function testStrictFormMethodChecking() {
|
||||
$response = $this->get('FormTest_ControllerWithStrictPostCheck');
|
||||
$response = $this->get(
|
||||
'FormTest_ControllerWithStrictPostCheck/Form/?Email=test@test.com&action_doSubmit=1'
|
||||
);
|
||||
$this->assertEquals(405, $response->getStatusCode(), 'Submission fails with wrong method');
|
||||
|
||||
$response = $this->get('FormTest_ControllerWithStrictPostCheck');
|
||||
$response = $this->post(
|
||||
'FormTest_ControllerWithStrictPostCheck/Form',
|
||||
array(
|
||||
'Email' => 'test@test.com',
|
||||
'action_doSubmit' => 1
|
||||
)
|
||||
);
|
||||
$this->assertEquals(200, $response->getStatusCode(), 'Submission succeeds with correct method');
|
||||
}
|
||||
|
||||
public function testEnableSecurityToken() {
|
||||
SecurityToken::disable();
|
||||
$form = $this->getStubForm();
|
||||
@ -468,24 +486,7 @@ class FormTest_Controller extends Controller implements TestOnly {
|
||||
'SomeRequiredField'
|
||||
)
|
||||
);
|
||||
|
||||
// Disable CSRF protection for easier form submission handling
|
||||
$form->disableSecurityToken();
|
||||
|
||||
return $form;
|
||||
}
|
||||
|
||||
public function FormWithSecurityToken() {
|
||||
$form = new Form(
|
||||
$this,
|
||||
'FormWithSecurityToken',
|
||||
new FieldList(
|
||||
new EmailField('Email')
|
||||
),
|
||||
new FieldList(
|
||||
new FormAction('doSubmit')
|
||||
)
|
||||
);
|
||||
$form->disableSecurityToken(); // Disable CSRF protection for easier form submission handling
|
||||
|
||||
return $form;
|
||||
}
|
||||
@ -533,12 +534,40 @@ class FormTest_ControllerWithSecurityToken extends Controller implements TestOnl
|
||||
return $this->redirectBack();
|
||||
}
|
||||
|
||||
public function getViewer($action = null) {
|
||||
return new SSViewer('BlankPage');
|
||||
}
|
||||
}
|
||||
|
||||
Config::inst()->update('Director', 'rules', array(
|
||||
'FormTest_Controller' => 'FormTest_Controller'
|
||||
));
|
||||
class FormTest_ControllerWithStrictPostCheck extends Controller implements TestOnly {
|
||||
protected $template = 'BlankPage';
|
||||
|
||||
public function Link($action = null) {
|
||||
return Controller::join_links(
|
||||
'FormTest_ControllerWithStrictPostCheck',
|
||||
$this->request->latestParam('Action'),
|
||||
$this->request->latestParam('ID'),
|
||||
$action
|
||||
);
|
||||
}
|
||||
|
||||
public function Form() {
|
||||
$form = new Form(
|
||||
$this,
|
||||
'Form',
|
||||
new FieldList(
|
||||
new EmailField('Email')
|
||||
),
|
||||
new FieldList(
|
||||
new FormAction('doSubmit')
|
||||
)
|
||||
);
|
||||
$form->setFormMethod('POST');
|
||||
$form->setStrictFormMethodCheck(true);
|
||||
$form->disableSecurityToken(); // Disable CSRF protection for easier form submission handling
|
||||
|
||||
return $form;
|
||||
}
|
||||
|
||||
public function doSubmit($data, $form, $request) {
|
||||
$form->sessionMessage('Test save was successful', 'good');
|
||||
return $this->redirectBack();
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user