From 22239b4481918b09108d5e403971def266b1d64f Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 24 Oct 2010 20:26:31 -0400 Subject: [PATCH] Making the ability to use longer shared csrf tokens possible. This should make for fewer blackholed' requests when doing complicated javascript. --- cake/libs/controller/components/security.php | 25 +++++++++++++------ .../controller/components/security.test.php | 22 ++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/cake/libs/controller/components/security.php b/cake/libs/controller/components/security.php index c63b8ffda..e4db0ceb5 100644 --- a/cake/libs/controller/components/security.php +++ b/cake/libs/controller/components/security.php @@ -172,6 +172,16 @@ class SecurityComponent extends Component { */ public $csrfExpires = '+30 minutes'; +/** + * Controls whether or not CSRF tokens are use and burn. Set to false to not generate + * new tokens on each request. One token will be reused until it expires. This reduces + * the chances of users getting invalid requests because of token consumption. + * It has the side effect of making CSRF less secure, as tokens are reusable. + * + * @var boolean + */ + public $csrfUseOnce = true; + /** * Other components used by the Security component * @@ -677,16 +687,15 @@ class SecurityComponent extends Component { 'csrfTokens' => array() ); - if ($this->csrfCheck) { - $token['csrfTokens'][$authKey] = strtotime($this->csrfExpires); - } - + $tokenData = array(); if ($this->Session->check('_Token')) { $tokenData = $this->Session->read('_Token'); if (!empty($tokenData['csrfTokens'])) { - $token['csrfTokens'] += $tokenData['csrfTokens']; - $token['csrfTokens'] = $this->_expireTokens($token['csrfTokens']); + $token['csrfTokens'] = $this->_expireTokens($tokenData['csrfTokens']); } + } + if ($this->csrfCheck && ($this->csrfUseOnce || empty($tokenData['csrfTokens'])) ) { + $token['csrfTokens'][$authKey] = strtotime($this->csrfExpires); } $controller->request->params['_Token'] = $token; $this->Session->write('_Token', $token); @@ -705,7 +714,9 @@ class SecurityComponent extends Component { $token = $this->Session->read('_Token'); $requestToken = $controller->request->data('_Token.key'); if (isset($token['csrfTokens'][$requestToken]) && $token['csrfTokens'][$requestToken] >= time()) { - $this->Session->delete('_Token.csrfTokens.' . $requestToken); + if ($this->csrfUseOnce) { + $this->Session->delete('_Token.csrfTokens.' . $requestToken); + } return true; } return false; diff --git a/cake/tests/cases/libs/controller/components/security.test.php b/cake/tests/cases/libs/controller/components/security.test.php index e0506805c..71b7e51a1 100644 --- a/cake/tests/cases/libs/controller/components/security.test.php +++ b/cake/tests/cases/libs/controller/components/security.test.php @@ -1407,4 +1407,26 @@ DIGEST; $this->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed, 'fail() was not called.'); } + +/** + * test that csrfUseOnce = false works. + * + * @return void + */ + function testCsrfNotUseOnce() { + $this->Security->validatePost = false; + $this->Security->csrfCheck = true; + $this->Security->csrfUseOnce = false; + $this->Security->csrfExpires = '+10 minutes'; + + // Generate one token + $this->Security->startup($this->Controller); + $token = $this->Security->Session->read('_Token.csrfTokens'); + $this->assertEquals(1, count($token), 'Should only be one token.'); + + $this->Security->startup($this->Controller); + $token2 = $this->Security->Session->read('_Token.csrfTokens'); + $this->assertEquals(1, count($token2), 'Should only be one token.'); + $this->assertEquals($token, $token2, 'Tokens should not be different.'); + } }