diff --git a/module/VuFind/config/module.config.php b/module/VuFind/config/module.config.php index 73c6cefe64dc7d01e255c31276a92b7afdf4bf19..d258c9b20bc48b4f62ca50cc4147b25316107c41 100644 --- a/module/VuFind/config/module.config.php +++ b/module/VuFind/config/module.config.php @@ -369,12 +369,12 @@ $config = [ 'VuFind\SMS\SMSInterface' => 'VuFind\SMS\Factory', 'VuFind\Solr\Writer' => 'VuFind\Solr\WriterFactory', 'VuFind\Tags' => 'VuFind\TagsFactory', + 'VuFind\Validator\Csrf' => 'VuFind\Validator\CsrfFactory', 'VuFindHttp\HttpService' => 'VuFind\Service\Factory::getHttp', 'VuFindSearch\Service' => 'VuFind\Service\Factory::getSearchService', 'Zend\Db\Adapter\Adapter' => 'VuFind\Service\Factory::getDbAdapter', 'Zend\Mvc\I18n\Translator' => 'VuFind\Service\Factory::getTranslator', 'Zend\Session\SessionManager' => 'VuFind\Session\ManagerFactory', - 'Zend\Validator\Csrf' => 'VuFind\Service\CsrfValidatorFactory', ], 'initializers' => [ 'VuFind\ServiceManager\ServiceInitializer', @@ -439,6 +439,7 @@ $config = [ 'VuFind\Translator' => 'Zend\Mvc\I18n\Translator', 'VuFind\WorldCatUtils' => 'VuFind\Connection\WorldCatUtils', 'VuFind\YamlReader' => 'VuFind\Config\YamlReader', + 'Zend\Validator\Csrf' => 'VuFind\Validator\Csrf', ], ], 'translator' => [], diff --git a/module/VuFind/src/VuFind/Auth/Manager.php b/module/VuFind/src/VuFind/Auth/Manager.php index 73b58bc5dea79f108cf5ec4677cd1b99fafe8ed8..67f64d0090ffc2cc40b0bc40d9f50b5745016c58 100644 --- a/module/VuFind/src/VuFind/Auth/Manager.php +++ b/module/VuFind/src/VuFind/Auth/Manager.php @@ -31,9 +31,9 @@ use VuFind\Cookie\CookieManager; use VuFind\Db\Row\User as UserRow; use VuFind\Db\Table\User as UserTable; use VuFind\Exception\Auth as AuthException; +use VuFind\Validator\Csrf; use Zend\Config\Config; use Zend\Session\SessionManager; -use Zend\Validator\Csrf; /** * Wrapper class for handling logged-in user in session. @@ -442,11 +442,15 @@ class Manager implements \ZfcRbac\Identity\IdentityProviderInterface * If no CSRF token currently exists, or should be regenerated, generates one. * * @param bool $regenerate Should we regenerate token? (default false) + * @param int $maxTokens The maximum number of tokens to store in the + * session. * * @return string */ - public function getCsrfHash($regenerate = false) + public function getCsrfHash($regenerate = false, $maxTokens = 5) { + // Reset token store if we've overflowed the limit: + $this->csrf->trimTokenList($maxTokens); return $this->csrf->getHash($regenerate); } @@ -553,11 +557,14 @@ class Manager implements \ZfcRbac\Identity\IdentityProviderInterface $this->getAuth()->preLoginCheck($request); // Validate CSRF for form-based authentication methods: - if (!$this->getAuth()->getSessionInitiator(null) - && !$this->csrf->isValid($request->getPost()->get('csrf')) - ) { - $this->getAuth()->resetState(); - throw new AuthException('authentication_error_technical'); + if (!$this->getAuth()->getSessionInitiator(null)) { + if (!$this->csrf->isValid($request->getPost()->get('csrf'))) { + $this->getAuth()->resetState(); + throw new AuthException('authentication_error_technical'); + } else { + // After successful token verification, clear list to shrink session: + $this->csrf->trimTokenList(0); + } } // Perform authentication: diff --git a/module/VuFind/src/VuFind/Auth/ManagerFactory.php b/module/VuFind/src/VuFind/Auth/ManagerFactory.php index 5d19dc29706d325526b532b3e06dc6097e812cb9..9b1436cb86e60f086a3cdf162faa24740e1d6f2f 100644 --- a/module/VuFind/src/VuFind/Auth/ManagerFactory.php +++ b/module/VuFind/src/VuFind/Auth/ManagerFactory.php @@ -84,7 +84,7 @@ class ManagerFactory implements FactoryInterface $sessionManager = $container->get('Zend\Session\SessionManager'); $pm = $container->get('VuFind\Auth\PluginManager'); $cookies = $container->get('VuFind\Cookie\CookieManager'); - $csrf = $container->get('Zend\Validator\Csrf'); + $csrf = $container->get('VuFind\Validator\Csrf'); // Build the object and make sure account credentials haven't expired: $manager = new $requestedName( diff --git a/module/VuFind/src/VuFind/Controller/MyResearchController.php b/module/VuFind/src/VuFind/Controller/MyResearchController.php index 4e2d8e5fb08e14b62db1521e2c822409a4c2675a..48d3c1b3c653ce64d688134c7187da09cbc0c443 100644 --- a/module/VuFind/src/VuFind/Controller/MyResearchController.php +++ b/module/VuFind/src/VuFind/Controller/MyResearchController.php @@ -1731,11 +1731,14 @@ class MyResearchController extends AbstractBase $view = $this->createViewModel(['accountDeleted' => false]); if ($this->formWasSubmitted('submit')) { - $csrf = $this->serviceLocator->get('Zend\Validator\Csrf'); + $csrf = $this->serviceLocator->get('VuFind\Validator\Csrf'); if (!$csrf->isValid($this->getRequest()->getPost()->get('csrf'))) { throw new \VuFind\Exception\BadRequest( 'error_inconsistent_parameters' ); + } else { + // After successful token verification, clear list to shrink session: + $this->csrf->trimTokenList(0); } $user->delete( $config->Authentication->delete_comments_with_user ?? true diff --git a/module/VuFind/src/VuFind/Validator/Csrf.php b/module/VuFind/src/VuFind/Validator/Csrf.php new file mode 100644 index 0000000000000000000000000000000000000000..21ddfd6c3d567370df374ef18e1355fffbe6ae27 --- /dev/null +++ b/module/VuFind/src/VuFind/Validator/Csrf.php @@ -0,0 +1,70 @@ +<?php +/** + * Extension of Zend\Validator\Csrf with token counting/clearing functions added. + * + * PHP version 7 + * + * Copyright (C) Villanova University 2018. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * @category VuFind + * @package Validator + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development Wiki + */ +namespace VuFind\Validator; + +/** + * Extension of Zend\Validator\Csrf with token counting/clearing functions added. + * + * @category VuFind + * @package Solr + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development Wiki + */ +class Csrf extends \Zend\Validator\Csrf +{ + /** + * How many tokens are currently stored in the session? + * + * @return int + */ + public function getTokenCount() + { + return count($this->getSession()->tokenList ?? []); + } + + /** + * Keep only the most recent N tokens. + * + * @param int $limit Number of tokens to keep. + * + * @return void + */ + public function trimTokenList($limit) + { + $session = $this->getSession(); + if ($limit < 1) { + // Reset the array if necessary: + $session->tokenList = []; + } elseif ($limit < $this->getTokenCount()) { + // Trim the array if necessary: + $session->tokenList + = array_slice($session->tokenList, -1 * $limit, null, true); + } + } +} diff --git a/module/VuFind/src/VuFind/Service/CsrfValidatorFactory.php b/module/VuFind/src/VuFind/Validator/CsrfFactory.php similarity index 95% rename from module/VuFind/src/VuFind/Service/CsrfValidatorFactory.php rename to module/VuFind/src/VuFind/Validator/CsrfFactory.php index fa38fa27299e3cdd09088df4f5cdb549ba60b6d3..e50365789b6d25980adf5e7bfddbdec4aa4c04fa 100644 --- a/module/VuFind/src/VuFind/Service/CsrfValidatorFactory.php +++ b/module/VuFind/src/VuFind/Validator/CsrfFactory.php @@ -21,13 +21,13 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * * @category VuFind - * @package Service + * @package Validator * @author Demian Katz <demian.katz@villanova.edu> * @author Ere Maijala <ere.maijala@helsinki.fi> * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License * @link https://vufind.org/wiki/development Wiki */ -namespace VuFind\Service; +namespace VuFind\Validator; use Interop\Container\ContainerInterface; use Zend\ServiceManager\Factory\FactoryInterface; @@ -36,7 +36,7 @@ use Zend\ServiceManager\Factory\FactoryInterface; * CSRF Validator factory. * * @category VuFind - * @package Service + * @package Validator * @author Demian Katz <demian.katz@villanova.edu> * @author Ere Maijala <ere.maijala@helsinki.fi> * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License @@ -44,7 +44,7 @@ use Zend\ServiceManager\Factory\FactoryInterface; * * @codeCoverageIgnore */ -class CsrfValidatorFactory implements FactoryInterface +class CsrfFactory implements FactoryInterface { /** * Create an object diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Auth/ManagerTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Auth/ManagerTest.php index 5eba35bc793c1ad76c52ee2ce1bad45bd78ec3c4..a27c81cdde738ca7ce5cc7896ee4ef68a1d627a7 100644 --- a/module/VuFind/tests/unit-tests/src/VuFindTest/Auth/ManagerTest.php +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Auth/ManagerTest.php @@ -514,7 +514,7 @@ class ManagerTest extends \VuFindTest\Unit\TestCase $pm = $this->getMockPluginManager(); } $cookies = new \VuFind\Cookie\CookieManager([]); - $csrf = new \Zend\Validator\Csrf( + $csrf = new \VuFind\Validator\Csrf( [ 'session' => new \Zend\Session\Container('csrf', $sessionManager), 'salt' => 'csrftest' diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Validator/CsrfTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Validator/CsrfTest.php new file mode 100644 index 0000000000000000000000000000000000000000..8c68d3ed90b6d42c47840b468c1dcab9117f165c --- /dev/null +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Validator/CsrfTest.php @@ -0,0 +1,91 @@ +<?php +/** + * CSRF Test Class + * + * PHP version 7 + * + * Copyright (C) Villanova University 2018. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * @category VuFind + * @package Tests + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development:testing:unit_tests Wiki + */ +namespace VuFindTest\Validator; + +use VuFind\Validator\Csrf; +use Zend\Session\Container; + +/** + * CSRF Test Class + * + * @category VuFind + * @package Tests + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development:testing:unit_tests Wiki + */ +class CsrfTest extends \PHPUnit\Framework\TestCase +{ + /** + * Test counting behavior. + * + * @return void + */ + public function testCounting() + { + $csrf = new Csrf(['session' => new Container('csrftest1')]); + $this->assertEquals(0, $csrf->getTokenCount()); + $csrf->getHash(); + $this->assertEquals(1, $csrf->getTokenCount()); + $csrf->getHash(true); + $this->assertEquals(2, $csrf->getTokenCount()); + } + + /** + * Test trimming behavior. + * + * @return void + */ + public function testTrimming() + { + $csrf = new Csrf(['session' => new Container('csrftest2')]); + // Try trimming an empty list: + $csrf->trimTokenList(5); + $this->assertEquals(0, $csrf->getTokenCount()); + + // Now populate the list: + $firstToken = $csrf->getHash(); + $secondToken = $csrf->getHash(true); + $thirdToken = $csrf->getHash(true); + $this->assertEquals(3, $csrf->getTokenCount()); + + // All tokens are valid now! + $this->assertTrue($csrf->isValid($firstToken)); + $this->assertTrue($csrf->isValid($secondToken)); + $this->assertTrue($csrf->isValid($thirdToken)); + + // Trim the list down to one: + $csrf->trimTokenList(1); + $this->assertEquals(1, $csrf->getTokenCount()); + + // Now only the latest token is valid: + $this->assertFalse($csrf->isValid($firstToken)); + $this->assertFalse($csrf->isValid($secondToken)); + $this->assertTrue($csrf->isValid($thirdToken)); + } +} diff --git a/themes/bootstrap3/templates/Auth/AbstractBase/login.phtml b/themes/bootstrap3/templates/Auth/AbstractBase/login.phtml index 1b42953515373d92232a37577cb4589b0cca4619..8f4b1ed2aecbdd2916a9e128c7aafe23e165073b 100644 --- a/themes/bootstrap3/templates/Auth/AbstractBase/login.phtml +++ b/themes/bootstrap3/templates/Auth/AbstractBase/login.phtml @@ -4,7 +4,7 @@ <form method="post" action="<?=$this->url('myresearch-home')?>" name="loginForm" class="form-login"> <?=$this->auth()->getLoginFields()?> <input type="hidden" name="auth_method" value="<?=$account->getAuthMethod()?>"> - <input type="hidden" name="csrf" value="<?=$this->escapeHtmlAttr($account->getCsrfHash(true))?>" /> + <input type="hidden" name="csrf" value="<?=$this->escapeHtmlAttr($account->getCsrfHash())?>" /> <div class="form-group"> <input class="btn btn-primary" type="submit" name="processLogin" value="<?=$this->transEsc('Login')?>"> <?php if ($account->supportsCreation()): ?> diff --git a/themes/bootstrap3/templates/myresearch/deleteaccount.phtml b/themes/bootstrap3/templates/myresearch/deleteaccount.phtml index c93eed2b00fc1c451cc9ea699c8e3fc584c8abf5..140d2fc16ece6ac77286c9da6dbac4a1c5d25a70 100644 --- a/themes/bootstrap3/templates/myresearch/deleteaccount.phtml +++ b/themes/bootstrap3/templates/myresearch/deleteaccount.phtml @@ -13,7 +13,7 @@ <form id="delete-account" method="post" action="<?=$this->url('myresearch-deleteaccount') ?>" name="deleteAccount"> <h3><?=$this->transEsc('delete_account_confirm'); ?></h3> <p><?=$this->translate('delete_account_description_html'); ?></p> - <input type="hidden" name="csrf" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash(true))?>" /> + <input type="hidden" name="csrf" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash())?>" /> <input id="delete-account-cancel" class="btn btn-primary" type="submit" name="reset" data-dismiss="modal" value="<?=$this->transEsc('confirm_dialog_no'); ?>"/> <input id="delete-account-submit" class="btn btn-primary" type="submit" name="submit" value="<?=$this->transEsc('confirm_dialog_yes'); ?>"/> </form> diff --git a/themes/bootstrap3/templates/myresearch/newpassword.phtml b/themes/bootstrap3/templates/myresearch/newpassword.phtml index d79c622e70ab96b35e0b96e17488b911046812ac..5ccd51db90ebc06aaa3019ff38a3a82ed36aa3a1 100644 --- a/themes/bootstrap3/templates/myresearch/newpassword.phtml +++ b/themes/bootstrap3/templates/myresearch/newpassword.phtml @@ -19,7 +19,7 @@ <div class="error"><?=$this->transEsc('recovery_user_not_found') ?></div> <?php else: ?> <form id="newpassword" class="form-new-password" action="<?=$this->url('myresearch-newpassword') ?>" method="post" data-toggle="validator" role="form"> - <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash(true))?>" name="csrf"/> + <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash())?>" name="csrf"/> <input type="hidden" value="<?=$this->escapeHtmlAttr($this->hash) ?>" name="hash"/> <input type="hidden" value="<?=$this->escapeHtmlAttr($this->username) ?>" name="username"/> <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth_method) ?>" name="auth_method"/>