Fixed bug #17490: After introducing the locking in #17289 no CSRF token will ever...
authorSteffen Kamper <info@sk-typo3.de>
Sat, 5 Feb 2011 21:31:41 +0000 (21:31 +0000)
committerSteffen Kamper <info@sk-typo3.de>
Sat, 5 Feb 2011 21:31:41 +0000 (21:31 +0000)
git-svn-id: https://svn.typo3.org/TYPO3v4/Core/branches/TYPO3_4-5@10391 709f56b5-9817-0410-a4d7-c38de5d9e867

ChangeLog
t3lib/formprotection/class.t3lib_formprotection_abstract.php
t3lib/formprotection/class.t3lib_formprotection_backendformprotection.php
tests/t3lib/formprotection/t3lib_formprotection_BackendFormProtectionTest.php
tests/t3lib/formprotection/t3lib_formprotection_InstallToolFormProtectionTest.php

index 363c35d..9564bf2 100755 (executable)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2011-02-05  Steffen Kamper  <steffen@typo3.org>
+
+       * Fixed bug #17490: After introducing the locking in #17289 no CSRF token will ever be deleted (Thanks to Helmut Hummel)
+
 2011-02-05  Steffen Gebert  <steffen@steffen-gebert.de>
 
        * Fixed bug #17374: implode() issues in Install Tool
index c5d12ae..8dbd4a8 100644 (file)
@@ -56,6 +56,20 @@ abstract class t3lib_formprotection_Abstract {
        protected $tokens = array();
 
        /**
+        * Tokens that have been added during this request.
+        *
+        * @var array<array>
+        */
+       protected $addedTokens = array();
+
+       /**
+        * Token ids of tokens that have been dropped during this request.
+        *
+        * @var array
+        */
+       protected $droppedTokenIds = array();
+
+       /**
         * Constructor. Makes sure existing tokens are read and available for
         * checking.
         */
@@ -123,6 +137,7 @@ abstract class t3lib_formprotection_Abstract {
                        'action' => $action,
                        'formInstanceName' => $formInstanceName,
                );
+               $this->addedTokens[$tokenId] = $this->tokens[$tokenId];
                $this->preventOverflow();
 
                return $tokenId;
@@ -219,10 +234,31 @@ abstract class t3lib_formprotection_Abstract {
        protected function dropToken($tokenId) {
                if (isset($this->tokens[$tokenId])) {
                        unset($this->tokens[$tokenId]);
+                       $this->droppedTokenIds[] = $tokenId;
                }
        }
 
        /**
+        * Persisting of tokens is only required, if tokens are
+        * deleted or added during this request.
+        *
+        * @return boolean
+        */
+       protected function isPersistingRequired() {
+               return !empty($this->droppedTokenIds) || !empty($this->addedTokens);
+       }
+
+       /**
+        * Reset the arrays of added or deleted tokens.
+        *
+        * @return void
+        */
+       protected function resetPersistingRequiredStatus() {
+               $this->droppedTokenIds = array();
+               $this->addedTokens = array();
+       }
+
+       /**
         * Checks whether the number of current tokens still is at most
         * $this->maximumNumberOfTokens.
         *
index 20cc07e..7dda931 100644 (file)
@@ -139,6 +139,19 @@ class t3lib_formprotection_BackendFormProtection extends t3lib_formprotection_Ab
        }
 
        /**
+        * Overrule the method in the absract class, because we can drop the
+        * whole locking procedure, which is done in persistTokens, if we
+        * simply want to delete all tokens.
+        *
+        * @see t3lib/formprotection/t3lib_formprotection_Abstract::clean()
+        */
+       public function clean() {
+               $this->tokens = array();
+               $this->backendUser->setAndSaveSessionData('formTokens', $this->tokens);
+               $this->resetPersistingRequiredStatus();
+       }
+
+       /**
         * Creates or displayes an error message telling the user that the submitted
         * form token is invalid.
         *
@@ -184,7 +197,10 @@ class t3lib_formprotection_BackendFormProtection extends t3lib_formprotection_Ab
        protected function updateTokens() {
                $this->backendUser->user = $this->backendUser->fetchUserSession(TRUE);
                $tokens = $this->retrieveTokens();
-               $this->tokens = array_merge($this->tokens, $tokens);
+               $this->tokens = array_merge($tokens, $this->addedTokens);
+               foreach ($this->droppedTokenIds as $tokenId) {
+                       unset($this->tokens[$tokenId]);
+               }
        }
 
        /**
@@ -194,12 +210,15 @@ class t3lib_formprotection_BackendFormProtection extends t3lib_formprotection_Ab
         * @return void
         */
        public function persistTokens() {
-               $lockObject = $this->acquireLock();
+               if ($this->isPersistingRequired()) {
+                       $lockObject = $this->acquireLock();
 
-               $this->updateTokens();
-               $this->backendUser->setAndSaveSessionData('formTokens', $this->tokens);
+                       $this->updateTokens();
+                       $this->backendUser->setAndSaveSessionData('formTokens', $this->tokens);
+                       $this->resetPersistingRequiredStatus();
 
-               $this->releaseLock($lockObject);
+                       $this->releaseLock($lockObject);
+               }
        }
 
        /**
index a622df5..4771ce8 100644 (file)
@@ -51,9 +51,10 @@ class t3lib_formprotection_BackendFormProtectionTest extends tx_phpunit_testcase
                        't3lib_beUserAuth',
                        array('getSessionData', 'setAndSaveSessionData')
                );
+               $GLOBALS['BE_USER']->user['uid'] = 1;
 
                $className = $this->createAccessibleProxyClass();
-               $this->fixture = new $className;
+               $this->fixture = $this->getMock($className, array('acquireLock', 'releaseLock'));
        }
 
        public function tearDown() {
@@ -84,6 +85,9 @@ class t3lib_formprotection_BackendFormProtectionTest extends tx_phpunit_testcase
                                '  public function createValidationErrorMessage() {' .
                                '    parent::createValidationErrorMessage();' .
                                '  }' .
+                               '  public function updateTokens() {' .
+                               '    return parent::updateTokens();' .
+                               '  }' .
                                '  public function retrieveTokens() {' .
                                '    return parent::retrieveTokens();' .
                                '  }' .
@@ -176,8 +180,9 @@ class t3lib_formprotection_BackendFormProtectionTest extends tx_phpunit_testcase
                $action = 'edit';
                $formInstanceName = '42';
 
-               $GLOBALS['BE_USER']->expects($this->once())->method('getSessionData')
-                       ->with('formTokens')->will($this->returnValue(array(
+               $GLOBALS['BE_USER']->expects($this->atLeastOnce())->method('getSessionData')
+                       ->with('formTokens')
+                       ->will($this->returnValue(array(
                                $tokenId => array(
                                        'formName' => $formName,
                                        'action' => $action,
@@ -185,7 +190,7 @@ class t3lib_formprotection_BackendFormProtectionTest extends tx_phpunit_testcase
                                ),
                        )));
 
-               $this->fixture->retrieveTokens();
+               $this->fixture->updateTokens();
 
                $this->assertTrue(
                        $this->fixture->validateToken($tokenId, $formName, $action,  $formInstanceName)
@@ -195,6 +200,37 @@ class t3lib_formprotection_BackendFormProtectionTest extends tx_phpunit_testcase
        /**
         * @test
         */
+       public function tokensStayDroppedAfterPersistingTokens() {
+               $tokenId = '51a655b55c54d54e5454c5f521f6552a';
+               $formName = 'foo';
+               $action = 'edit';
+               $formInstanceName = '42';
+
+               $GLOBALS['BE_USER']->expects($this->atLeastOnce())->method('getSessionData')
+                       ->will($this->returnValue(array(
+                               $tokenId => array(
+                                       'formName' => $formName,
+                                       'action' => $action,
+                                       'formInstanceName' => $formInstanceName,
+                               ),
+                       )));
+
+               $className = $this->createAccessibleProxyClass();
+
+               $this->fixture->updateTokens();
+
+               $this->fixture->validateToken($tokenId, $formName, $action,  $formInstanceName);
+
+               $this->fixture->persistTokens();
+
+               $this->assertFalse(
+                       $this->fixture->validateToken($tokenId, $formName, $action,  $formInstanceName)
+               );
+       }
+
+       /**
+        * @test
+        */
        public function persistTokensWritesTokensToSession() {
                $formName = 'foo';
                $action = 'edit';
index 68bf0a3..cd1983f 100644 (file)
@@ -83,7 +83,7 @@ class t3lib_formprotection_InstallToolFormProtectionTest extends tx_phpunit_test
                                '    parent::createValidationErrorMessage();' .
                                '  }' .
                                '  public function retrieveTokens() {' .
-                               '    return parent::retrieveTokens();' .
+                               '    return $this->tokens = parent::retrieveTokens();' .
                                '  }' .
                                '}'
                        );