Permalink
Browse files

ENHANCEMENT Added SecurityToken to wrap CSRF protection via "Security…

…ID" request parameter (from r113272)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@114525 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information...
1 parent ecaa735 commit 9ec31acacb5b23335ef64e1fcd70c4cecb937f4c @sminnee sminnee committed Dec 5, 2010
Showing with 588 additions and 34 deletions.
  1. +4 −0 dev/FunctionalTest.php
  2. +49 −34 forms/Form.php
  3. +273 −0 security/SecurityToken.php
  4. +117 −0 tests/forms/FormTest.php
  5. +145 −0 tests/security/SecurityTokenTest.php
View
@@ -70,9 +70,13 @@ function setUp() {
// Unprotect the site, tests are running with the assumption it's off. They will enable it on a case-by-case basis.
BasicAuth::protect_entire_site(false);
+
+ SecurityToken::disable();
}
function tearDown() {
+ SecurityToken::enable();
+
parent::tearDown();
unset($this->mainSession);
}
View
@@ -119,6 +119,11 @@ class Form extends RequestHandler {
protected $security = true;
+ /**
+ * @var SecurityToken
+ */
+ protected $securityToken = null;
+
/**
* HACK This is a temporary hack to allow multiple calls to includeJavascriptValidation on
* the validator (if one is present).
@@ -168,10 +173,12 @@ function __construct($controller, $name, FieldSet $fields, FieldSet $actions, $v
// 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'))) {
- $this->security = $controller->securityTokenEnabled();
+ $securityEnabled = $controller->securityTokenEnabled();
} else {
- $this->security = self::$default_security;
+ $securityEnabled = SecurityToken::is_enabled();
}
+
+ $this->securityToken = ($securityEnabled) ? new SecurityToken() : new NullSecurityToken();
}
static $url_handlers = array(
@@ -225,12 +232,9 @@ function httpSubmission($request) {
$this->loadDataFrom($vars, true);
// Protection against CSRF attacks
- if($this->securityTokenEnabled()) {
- $securityID = Session::get('SecurityID');
-
- if(!$securityID || !isset($vars['SecurityID']) || $securityID != $vars['SecurityID']) {
- $this->httpError(400, "SecurityID doesn't match, possible CSRF attack.");
- }
+ $token = $this->getSecurityToken();
+ if(!$token->checkRequest($request)) {
+ $this->httpError(400, "Security token doesn't match, possible CSRF attack.");
}
// Determine the action button clicked
@@ -430,27 +434,17 @@ function transformTo(FormTransformation $format) {
/**
- * Generate extra special fields - namely the SecurityID field
+ * Generate extra special fields - namely the security token field (if required).
*
* @return FieldSet
*/
public function getExtraFields() {
$extraFields = new FieldSet();
- if(!$this->fields->fieldByName('SecurityID') && $this->securityTokenEnabled()) {
- if(Session::get('SecurityID')) {
- $securityID = Session::get('SecurityID');
- } else {
- $generator = new RandomGenerator();
- $securityID = $generator->generateHash('sha1');
- Session::set('SecurityID', $securityID);
- }
-
- $securityField = new HiddenField('SecurityID', '', $securityID);
- $securityField->setForm($this);
- $extraFields->push($securityField);
- $this->securityTokenAdded = true;
- }
+ $token = $this->getSecurityToken();
+ $tokenField = $token->updateFieldSet($this->fields);
+ if($tokenField) $tokenField->setForm($this);
+ $this->securityTokenAdded = true;
// add the "real" HTTP method if necessary (for PUT, DELETE and HEAD)
if($this->FormMethod() != $this->FormHttpMethod()) {
@@ -859,7 +853,11 @@ function getLegend() {
* Processing that occurs before a form is executed.
* This includes form validation, if it fails, we redirect back
* to the form with appropriate error messages.
- * Triggered through {@link httpSubmission()} which is triggered
+ * 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 boolean
*/
function validate(){
if($this->validator){
@@ -1167,34 +1165,51 @@ function disableDefaultAction() {
}
/**
- * Disable the requirement of a SecurityID in the Form. This security protects
+ * Disable the requirement of a security token in the Form. This security protects
* against CSRF attacks, but you should disable this if you don't want to tie
* a form to a session - eg a search form.
*/
function disableSecurityToken() {
- $this->security = false;
+ $this->securityToken = new NullSecurityToken();
}
-
- private static $default_security = true;
-
/**
- * Disable security tokens for every form on this site.
+ * Disable security tokens for every form.
+ * Note that this doesn't apply to {@link SecurityToken}
+ * instances outside of the Form class, nor applies
+ * to existing form instances.
+ *
+ * See {@link enable_all_security_tokens()}.
+ *
+ * @deprecated 2.5 Use SecurityToken::disable()
*/
static function disable_all_security_tokens() {
- self::$default_security = false;
+ SecurityToken::disable();
}
/**
- * Returns true if security is enabled - that is if the SecurityID
+ * Returns true if security is enabled - that is if the security token
* should be included and checked on this form.
+ *
+ * @deprecated 2.5 Use Form->getSecurityToken()->isEnabled()
*
* @return bool
*/
function securityTokenEnabled() {
- return $this->security;
+ return $this->securityToken->isEnabled();
}
-
+
+ /**
+ * Returns the security token for this form (if any exists).
+ * Doesn't check for {@link securityTokenEnabled()}.
+ * Use {@link SecurityToken::inst()} to get a global token.
+ *
+ * @return SecurityToken|null
+ */
+ function getSecurityToken() {
+ return $this->securityToken;
+ }
+
/**
* Returns the name of a field, if that's the only field that the current controller is interested in.
* It checks for a call to the callfieldmethod action.
Oops, something went wrong.

0 comments on commit 9ec31ac

Please sign in to comment.