[feature/passwords] Do not pass phpbb_container to passwords manager

PHPBB3-11610

phpBB/config/passwords.yml

@@ -49,6 +49,5 @@ services:
         class: phpbb_passwords_manager
         arguments:
             - @config
-            - @service_container
             - @passwords.driver_collection
             - %passwords.algorithm%

phpBB/phpbb/passwords/driver/bcrypt.php

@@ -30,14 +30,6 @@ class phpbb_passwords_driver_bcrypt extends phpbb_passwords_driver_base
 		return self::PREFIX;
 	}
 
-	/**
-	* @inheritdoc
-	*/
-	public function get_type()
-	{
-		return get_class($this);
-	}
-
 	/**
 	* @inheritdoc
 	*/

phpBB/phpbb/passwords/driver/bcrypt_2y.php

@@ -30,14 +30,6 @@ class phpbb_passwords_driver_bcrypt_2y extends phpbb_passwords_driver_bcrypt
 		return self::PREFIX;
 	}
 
-	/**
-	* @inheritdoc
-	*/
-	public function get_type()
-	{
-		return get_class($this);
-	}
-
 	/**
 	* @inheritdoc
 	*/

phpBB/phpbb/passwords/driver/interface.php

@@ -33,13 +33,6 @@ interface phpbb_passwords_driver_interface
 	*/
 	public function get_prefix();
 
-	/**
-	* Returns the name of the hash type
-	*
-	* @return string	Hash type of driver
-	*/
-	public function get_type();
-
 	/**
 	* Hash the password
 	*

phpBB/phpbb/passwords/driver/phpass.php

@@ -29,12 +29,4 @@ class phpbb_passwords_driver_phpass extends phpbb_passwords_driver_salted_md5
 	{
 		return self::PREFIX;
 	}
-
-	/**
-	* @inheritdoc
-	*/
-	public function get_type()
-	{
-		return get_class($this);
-	}
 }

phpBB/phpbb/passwords/driver/salted_md5.php

@@ -30,14 +30,6 @@ class phpbb_passwords_driver_salted_md5 extends phpbb_passwords_driver_base
 		return self::PREFIX;
 	}
 
-	/**
-	* @inheritdoc
-	*/
-	public function get_type()
-	{
-		return get_class($this);
-	}
-
 	/**
 	* @inheritdoc
 	*/

phpBB/phpbb/passwords/helper.php

@@ -25,21 +25,14 @@ class phpbb_passwords_helper
 	*/
 	protected $manager;
 
-	/**
-	* @var phpbb_container
-	*/
-	protected $container;
-
 	/**
 	* Construct a phpbb_passwords_helper object
 	*
 	* @param phpbb_passwords_manager $manager Crypto manager object
-	* @param phpbb_container $container phpBB container object
 	*/
-	public function __construct($manager, $container)
+	public function __construct($manager)
 	{
 		$this->manager = $manager;
-		$this->container = $container;
 	}
 
 	/**
@@ -85,14 +78,22 @@ class phpbb_passwords_helper
 		$hash = $hash_settings[0];
 
 		// Put settings of current hash into data array
-		$stored_hash_type = $this->manager->get_hashing_algorithm($password_hash);
+		$stored_hash_type = $this->manager->detect_algorithm($password_hash);
 		$this->combine_hash_output($data, 'prefix', $stored_hash_type->get_prefix());
 		$this->combine_hash_output($data, 'settings', $stored_hash_type->get_settings_only($password_hash));
 
 		// Hash current hash with the defined types
 		foreach ($type as $cur_type)
 		{
-			$new_hash_type = $this->container->get($cur_type);
+			if (isset($this->manager->algorithms[$cur_type]))
+			{
+				$new_hash_type = $this->manager->algorithms[$cur_type];
+			}
+			else
+			{
+				return false;
+			}
+
 			$new_hash = $new_hash_type->hash(str_replace($stored_hash_type->get_settings_only($password_hash), '', $hash));
 			$this->combine_hash_output($data, 'prefix', $new_hash_type->get_prefix());
 			$this->combine_hash_output($data, 'settings', substr(str_replace('$', '\\', $new_hash_type->get_settings_only($new_hash, true)), 0));

phpBB/phpbb/passwords/manager.php

@@ -26,12 +26,19 @@ class phpbb_passwords_manager
 	protected $type = false;
 
 	/**
-	* Hashing algorithm types
+	* Hashing algorithm type map
+	* Will be used to map hash prefix to type
 	*/
 	protected $type_map = false;
 
 	/**
-	* Password convert flag. Password should be converted
+	* Service collection of hashing algorithms
+	* Needs to be public for passwords helper
+	*/
+	public $algorithms = false;
+
+	/**
+	* Password convert flag. Signals that password should be converted
 	*/
 	public $convert_flag = false;
 
@@ -47,21 +54,17 @@ class phpbb_passwords_manager
 	*/
 	protected $config;
 
-	/**
-	* phpBB compiled container
-	* @var service_container
-	*/
-	protected $container;
-
 	/**
 	* Construct a passwords object
 	*
 	* @param phpbb_config $config phpBB configuration
+	* @param phpbb_di_service_collection $hashing_algorithms Hashing driver
+	*			service collection
+	* @param string $default Default driver name
 	*/
-	public function __construct($config, $container, $hashing_algorithms, $default)
+	public function __construct($config, $hashing_algorithms, $default)
 	{
 		$this->config = $config;
-		$this->container = $container;
 		$this->type = $default;
 
 		$this->fill_type_map($hashing_algorithms);
@@ -77,9 +80,14 @@ class phpbb_passwords_manager
 	{
 		foreach ($hashing_algorithms as $algorithm)
 		{
+			if (!isset($this->algorithms[$algorithm->get_name()]))
+			{
+				$this->algorithms[$algorithm->get_name()] = $algorithm;
+			}
+
 			if (!isset($this->type_map[$algorithm->get_prefix()]))
 			{
-				$this->type_map[$algorithm->get_prefix()] = $algorithm;
+				$this->type_map[$algorithm->get_prefix()] = $algorithm->get_name();
 			}
 		}
 	}
@@ -91,18 +99,38 @@ class phpbb_passwords_manager
 	{
 		if ($this->helper === null)
 		{
-			$this->helper = new phpbb_passwords_helper($this, $this->container);
+			$this->helper = new phpbb_passwords_helper($this);
 		}
 	}
 
 	/**
-	* Get the hash type from the supplied hash
+	* Get the algorithm specified by a specific prefix
 	*
-	* @param string $hash Password hash that should be checked
+	* @param string $prefix Password hash prefix
 	*
 	* @return object The hash type object
 	*/
-	public function get_hashing_algorithm($hash)
+	protected function get_algorithm($prefix)
+	{
+		if (isset($this->type_map[$prefix]) && isset($this->algorithms[$this->type_map[$prefix]]))
+		{
+			return $this->algorithms[$this->type_map[$prefix]];
+		}
+		else
+		{
+			return false;
+		}
+	}
+
+	/**
+	* Detect the hash type of the supplied hash
+	*
+	* @param string $hash Password hash that should be checked
+	*
+	* @return object|bool The hash type object or false if the specified
+	*			type is not supported
+	*/
+	public function detect_algorithm($hash)
 	{
 		/*
 		* preg_match() will also show hashing algos like $2a\H$, which
@@ -112,7 +140,7 @@ class phpbb_passwords_manager
 		*/
 		if (!preg_match('#^\$([a-zA-Z0-9\\\]*?)\$#', $hash, $match))
 		{
-			return $this->type_map['$H$'];
+			return $this->get_algorithm('$H$');
 		}
 
 		// Be on the lookout for multiple hashing algorithms
@@ -123,32 +151,27 @@ class phpbb_passwords_manager
 			$return_ary = array();
 			foreach ($hash_types as $type)
 			{
-				if (isset($this->type_map["\${$type}\$"]))
+				// we do not support the same hashing
+				// algorithm more than once
+				if (isset($return_ary[$type]))
 				{
-					// we do not support the same hashing
+					return false;
-					// algorithm more than once
-					if (isset($return_ary[$type]))
-					{
-						return false;
-					}
-					$return_ary[$type] = $this->type_map["\${$type}\$"];
 				}
-				else
+
+				$return_ary[$type] = $this->get_algorithm("\${$type}\$");
+
+				if (empty($return_ary[$type]))
 				{
+
 					return false;
 				}
 			}
 			return $return_ary;
 		}
 
-		if (isset($this->type_map[$match[0]]))
+		// get_algorithm() will automatically return false if prefix
-		{
+		// is not supported
-			return $this->type_map[$match[0]];
+		return $this->get_algorithm($match[0]);
-		}
-		else
-		{
-			return false;
-		}
 	}
 
 	/**
@@ -169,7 +192,14 @@ class phpbb_passwords_manager
 			return $this->helper->combined_hash_password($password, $type);
 		}
 
-		$hashing_algorithm = $this->container->get($type);
+		if (isset($this->algorithms[$type]))
+		{
+			$hashing_algorithm = $this->algorithms[$type];
+		}
+		else
+		{
+			return false;
+		}
 
 		// Do not support 8-bit characters with $2a$ bcrypt
 		// Also see http://www.php.net/security/crypt_blowfish.php
@@ -181,7 +211,7 @@ class phpbb_passwords_manager
 			}
 		}
 
-		return $this->container->get($type)->hash($password);
+		return $hashing_algorithm->hash($password);
 	}
 
 	/**
@@ -195,7 +225,7 @@ class phpbb_passwords_manager
 	public function check_hash($password, $hash)
 	{
 		// First find out what kind of hash we're dealing with
-		$stored_hash_type = $this->get_hashing_algorithm($hash);
+		$stored_hash_type = $this->detect_algorithm($hash);
 		if ($stored_hash_type == false)
 		{
 			return false;

tests/passwords/manager_test.php

@@ -46,7 +46,7 @@ class phpbb_passwords_manager_test extends PHPUnit_Framework_TestCase
 		}
 
 		// Set up avatar manager
-		$this->manager = new phpbb_passwords_manager($config, $this->phpbb_container, $this->passwords_drivers, 'passwords.driver.bcrypt_2y');
+		$this->manager = new phpbb_passwords_manager($config, $this->passwords_drivers, 'passwords.driver.bcrypt_2y');
 	}
 
 	public function hash_password_data()
@@ -58,6 +58,7 @@ class phpbb_passwords_manager_test extends PHPUnit_Framework_TestCase
 				array('passwords.driver.bcrypt_2y', '2a', 60),
 				array('passwords.driver.bcrypt', '2a', 60),
 				array('passwords.driver.salted_md5', 'H', 34),
+				array('passwords.driver.foobar', '', false),
 			);
 		}
 		else
@@ -67,6 +68,7 @@ class phpbb_passwords_manager_test extends PHPUnit_Framework_TestCase
 				array('passwords.driver.bcrypt_2y', '2y', 60),
 				array('passwords.driver.bcrypt', '2a', 60),
 				array('passwords.driver.salted_md5', 'H', 34),
+				array('passwords.driver.foobar', '', false),
 			);
 		}
 	}
@@ -77,6 +79,12 @@ class phpbb_passwords_manager_test extends PHPUnit_Framework_TestCase
 	public function test_hash_password($type, $prefix, $length)
 	{
 		$password = $this->default_pw;
+
+		if (!$length)
+		{
+			$this->assertEquals(false, $hash = $this->manager->hash_password($password, $type));
+			return;
+		}
 		$time = microtime(true);
 
 		// Limit each test to 1 second