From 380be9f1fd713dbcee91f12f18060d6b3ff4819e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 25 Sep 2016 20:33:10 +0200 Subject: [PATCH] [ticket/14733] Make sure detect_algorithm() works correctly and add tests detect_algorithm() returned array() if an algorithm prefix was more than 2 characters long. This might have been invalid for other prefixes. In order to correctly cope with other prefixes, another check for a backslash in the prefix definitino has been added. This was discovered while writing the tests for the newly added interface. PHPBB3-14733 --- phpBB/phpbb/passwords/manager.php | 2 +- tests/passwords/manager_test.php | 50 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index 6c3ef4c477..fad76a9fe5 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -174,7 +174,7 @@ class manager // Be on the lookout for multiple hashing algorithms // 2 is correct: H\2a > 2, H\P > 2 - if (strlen($match[1]) > 2) + if (strlen($match[1]) > 2 && strpos($match[1], '\\') !== false) { $hash_types = explode('\\', $match[1]); $return_ary = array(); diff --git a/tests/passwords/manager_test.php b/tests/passwords/manager_test.php index dbe0341664..0410d7035f 100644 --- a/tests/passwords/manager_test.php +++ b/tests/passwords/manager_test.php @@ -344,4 +344,54 @@ class phpbb_passwords_manager_test extends \phpbb_test_case { $this->assertSame($expected, $this->driver_helper->string_compare($a, $b)); } + + public function data_driver_interface_driver() + { + return array( + array(false, false, false), + array(true, false, false), + array(true, true, true), + ); + } + + /** + * @dataProvider data_driver_interface_driver + */ + public function test_driver_interface_driver($use_new_interface, $needs_rehash, $expected) + { + if ($use_new_interface) + { + $test_driver = $this->getMock('\phpbb\passwords\driver\rehashable_driver_interface', array('needs_rehash', 'get_prefix', 'check', 'is_supported', 'is_legacy', 'hash', 'get_settings_only')); + $test_driver->method('needs_rehash') + ->willReturn($needs_rehash); + } + else + { + $test_driver = $this->getMock('\phpbb\passwords\driver\driver_interface', array('get_prefix', 'check', 'is_supported', 'is_legacy', 'hash', 'get_settings_only')); + } + $config = new \phpbb\config\config(array()); + + $test_driver->method('is_supported') + ->willReturn(true); + $test_driver->method('get_prefix') + ->willReturn('$test$'); + $test_driver->method('check') + ->with($this->anything()) + ->willReturn(true); + $passwords_drivers = array( + 'passwords.driver.foobar' => $test_driver, + 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $this->driver_helper, 10), + ); + // Set up another manager + $foobar_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $this->helper, array('passwords.driver.foobar')); + + $this->assertTrue($foobar_manager->check('foobar', '$test$somerandomstuff')); + $this->assertEquals($expected, $foobar_manager->convert_flag); + + // Should always return true in case a different driver is default + $foobar_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $this->helper, array('passwords.driver.bcrypt_2y', 'passwords.driver.foobar')); + + $this->assertTrue($foobar_manager->check('foobar', '$test$somerandomstuff')); + $this->assertTrue($foobar_manager->convert_flag); + } }