Skip to content
This repository
Browse code

API CHANGE Use Config for registering default password encryptors

Using the config system for registering password encryptors
Remove the eval on password encryptor construction by using reflection
Throws deprecation messages when using static register / unregister
  • Loading branch information...
commit 0d031a5045243a7d0392d0d324df1d22072da1e7 1 parent aebbb10
Stig Lindqvist authored
6  _config.php
@@ -61,12 +61,6 @@
61 61
 	define('EMAIL_BOUNCEHANDLER_KEY', '1aaaf8fb60ea253dbf6efa71baaacbb3');
62 62
 }
63 63
 
64  
-PasswordEncryptor::register('none', 'PasswordEncryptor_None');
65  
-PasswordEncryptor::register('md5', 'PasswordEncryptor_LegacyPHPHash("md5")');
66  
-PasswordEncryptor::register('sha1','PasswordEncryptor_LegacyPHPHash("sha1")');
67  
-PasswordEncryptor::register('md5_v2.4', 'PasswordEncryptor_PHPHash("md5")');
68  
-PasswordEncryptor::register('sha1_v2.4','PasswordEncryptor_PHPHash("sha1")');
69  
-
70 64
 // Zend_Cache temp directory setting
71 65
 $_ENV['TMPDIR'] = TEMP_FOLDER; // for *nix
72 66
 $_ENV['TMP'] = TEMP_FOLDER; // for Windows
14  _config/PasswordEncryptor.yml
... ...
@@ -0,0 +1,14 @@
  1
+name: PasswordEncryptor
  2
+---
  3
+PasswordEncryptor:
  4
+  encryptors:
  5
+    none:
  6
+      PasswordEncryptor_None: 
  7
+    md5:
  8
+      PasswordEncryptor_LegacyPHPHash: md5
  9
+    sha1:
  10
+      PasswordEncryptor_LegacyPHPHash: sha1
  11
+    md5_v2.4:
  12
+      PasswordEncryptor_PHPHash: md5
  13
+    sha1_v2.4:
  14
+      PasswordEncryptor_PHPHash: sha1
23  security/PasswordEncryptor.php
@@ -22,7 +22,7 @@
22 22
 	 * @return Array Map of encryptor code to the used class.
23 23
 	 */
24 24
 	static function get_encryptors() {
25  
-		return self::$encryptors;
  25
+		return Config::inst()->get('PasswordEncryptor', 'encryptors');
26 26
 	}
27 27
 	
28 28
 	/**
@@ -36,6 +36,7 @@ static function get_encryptors() {
36 36
 	 * @param String $class Classname of a {@link PasswordEncryptor} subclass
37 37
 	 */
38 38
 	static function register($code, $class) {
  39
+		Deprecation::notice('3.0', 'Use the Config system to register Password encryptors');
39 40
 		self::$encryptors[$code] = $class;
40 41
 	}
41 42
 	
@@ -43,29 +44,37 @@ static function register($code, $class) {
43 44
 	 * @param String $code Unique lookup.
44 45
 	 */
45 46
 	static function unregister($code) {
  47
+		Deprecation::notice('3.0', 'Use the Config system to unregister Password encryptors');
46 48
 		if(isset(self::$encryptors[$code])) unset(self::$encryptors[$code]);
47 49
 	}
48 50
 	
49 51
 	/**
50 52
 	 * @param String $algorithm
51  
-	 * @return PasswordEncryptor|Boolean Returns FALSE if class was not found
  53
+	 * @return PasswordEncryptor
  54
+	 * @throws PasswordEncryptor_NotFoundException
52 55
 	 */
53 56
 	static function create_for_algorithm($algorithm) {
54  
-		if(!isset(self::$encryptors[$algorithm])) {
  57
+		$encryptors = self::get_encryptors();
  58
+		if(!isset($encryptors[$algorithm])) {
55 59
 			throw new PasswordEncryptor_NotFoundException(
56 60
 				sprintf('No implementation found for "%s"', $algorithm)
57 61
 			);
58 62
 		}
59 63
 		
60  
-		$classWithArgs = self::$encryptors[$algorithm];
61  
-		$class = (($p = strpos($classWithArgs, '(')) !== false) ? substr($classWithArgs, 0, $p) : $classWithArgs;
  64
+		$class=key($encryptors[$algorithm]);
62 65
 		if(!class_exists($class)) {
63 66
 			throw new PasswordEncryptor_NotFoundException(
64 67
 				sprintf('No class found for "%s"', $class)
65 68
 			);
66  
-		}
67 69
 
68  
-		return eval("return new $classWithArgs;");
  70
+		}
  71
+		$refClass = new ReflectionClass($class);
  72
+		if(!$refClass->getConstructor()) {
  73
+			return new $class;
  74
+		}
  75
+		
  76
+		$arguments = $encryptors[$algorithm];
  77
+		return($refClass->newInstanceArgs($arguments));
69 78
 	}
70 79
 		
71 80
 	/**
3  tests/security/MemberAuthenticatorTest.php
@@ -30,8 +30,7 @@ function testLegacyPasswordHashMigrationUponLogin() {
30 30
 	}
31 31
 	
32 32
 	function testNoLegacyPasswordHashMigrationOnIncompatibleAlgorithm() {
33  
-		PasswordEncryptor::register('crc32', 'PasswordEncryptor_PHPHash("crc32")');
34  
-		
  33
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('crc32'=>array('PasswordEncryptor_PHPHash'=>'crc32')));
35 34
 		$field=Member::get_unique_identifier_field();
36 35
 		
37 36
 		$member = new Member();
44  tests/security/PasswordEncryptorTest.php
... ...
@@ -1,12 +1,26 @@
1 1
 <?php
2 2
 class PasswordEncryptorTest extends SapphireTest {
  3
+
  4
+	/**
  5
+	 *
  6
+	 * @var Config
  7
+	 */
  8
+	private $config = null;
  9
+
  10
+	public function setUp() {
  11
+		parent::setUp();
  12
+		$this->config = clone(Config::inst());
  13
+	}
  14
+
  15
+	public function tearDown() {
  16
+		parent::tearDown();
  17
+		Config::set_instance($this->config);
  18
+	}
  19
+
3 20
 	function testCreateForCode() {
4  
-		PasswordEncryptor::register('test', 'PasswordEncryptorTest_TestEncryptor');
  21
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test'=>array('PasswordEncryptorTest_TestEncryptor'=>null)));
5 22
 		$e = PasswordEncryptor::create_for_algorithm('test');
6  
-		$this->assertType(
7  
-			'PasswordEncryptorTest_TestEncryptor',
8  
-			$e
9  
-		);
  23
+		$this->assertType('PasswordEncryptorTest_TestEncryptor', $e );
10 24
 	}
11 25
 	
12 26
 	/**
@@ -17,25 +31,27 @@ function testCreateForCodeNotFound() {
17 31
 	}
18 32
 	
19 33
 	function testRegister() {
20  
-		PasswordEncryptor::register('test', 'PasswordEncryptorTest_TestEncryptor');
21  
-		$this->assertContains('test', array_keys(PasswordEncryptor::get_encryptors()));
22  
-		$this->assertContains('PasswordEncryptorTest_TestEncryptor', array_values(PasswordEncryptor::get_encryptors()));
  34
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test'=>array('PasswordEncryptorTest_TestEncryptor'=>null)));
  35
+		$encryptors = PasswordEncryptor::get_encryptors();
  36
+		$this->assertContains('test', array_keys($encryptors));
  37
+		$encryptor = $encryptors['test'];
  38
+		$this->assertContains('PasswordEncryptorTest_TestEncryptor', key($encryptor));
23 39
 	}
24 40
 	
25 41
 	function testUnregister() {
26  
-		PasswordEncryptor::register('test', 'PasswordEncryptorTest_TestEncryptor');
27  
-		PasswordEncryptor::unregister('test');
  42
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test'=>array('PasswordEncryptorTest_TestEncryptor'=>null)));
  43
+		Config::inst()->remove('PasswordEncryptor', 'encryptors', 'test');
28 44
 		$this->assertNotContains('test', array_keys(PasswordEncryptor::get_encryptors()));
29 45
 	}
30 46
 	
31 47
 	function testEncrytorPHPHashWithArguments() {
32  
-		PasswordEncryptor::register('test_md5', 'PasswordEncryptor_PHPHash("md5")');
  48
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_md5'=>array('PasswordEncryptor_PHPHash'=>'md5')));
33 49
 		$e = PasswordEncryptor::create_for_algorithm('test_md5');
34 50
 		$this->assertEquals('md5', $e->getAlgorithm());
35 51
 	}
36 52
 	
37 53
 	function testEncrytorPHPHash() {
38  
-		PasswordEncryptor::register('test_sha1', 'PasswordEncryptor_PHPHash("sha1")');
  54
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1'=>array('PasswordEncryptor_PHPHash'=>'sha1')));
39 55
 		$e = PasswordEncryptor::create_for_algorithm('test_sha1');
40 56
 		$password = 'mypassword';
41 57
 		$salt = 'mysalt';
@@ -46,7 +62,7 @@ function testEncrytorPHPHash() {
46 62
 	}
47 63
 	
48 64
 	function testEncrytorPHPHashCompare() {
49  
-		PasswordEncryptor::register('test_sha1', 'PasswordEncryptor_PHPHash("sha1")');
  65
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1'=>array('PasswordEncryptor_PHPHash'=>'sha1')));
50 66
 		$e = PasswordEncryptor::create_for_algorithm('test_sha1');
51 67
 		$this->assertTrue($e->compare(sha1('mypassword'), sha1('mypassword')));
52 68
 		$this->assertFalse($e->compare(sha1('mypassword'), sha1('mywrongpassword')));
@@ -59,7 +75,7 @@ function testEncrytorPHPHashCompare() {
59 75
 	 * 	php -r "echo(base_convert(sha1('mypassword'), 16, 36));"
60 76
 	 */
61 77
 	function testEncrytorLegacyPHPHashCompare() {
62  
-		PasswordEncryptor::register('test_sha1legacy', 'PasswordEncryptor_LegacyPHPHash("sha1")');
  78
+		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1legacy'=>array('PasswordEncryptor_LegacyPHPHash'=>'sha1')));
63 79
 		$e = PasswordEncryptor::create_for_algorithm('test_sha1legacy');
64 80
 		// precomputed hashes for 'mypassword' from different architectures
65 81
 		$amdHash = 'h1fj0a6m4o6k0sosks88oo08ko4gc4s';

0 notes on commit 0d031a5

Please sign in to comment.
Something went wrong with that request. Please try again.