APICHANGE: PasswordEncryptor::check() allows for more powerful password checking, deprecating PasswordEncryptor::compare()

security/Member.php

@@ -190,15 +190,8 @@ class Member extends DataObject implements TemplateGlobalProvider {
 	public function checkPassword($password) {
 		$result = $this->canLogIn();
 
-		$spec = Security::encrypt_password(
+		$e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
-			$password, 
+		if(!$e->check($this->Password, $password, $this->Salt, $this)) {
-			$this->Salt, 
-			$this->PasswordEncryption,
-			$this
-		);
-		$e = $spec['encryptor'];
-
-		if(!$e->compare($this->Password, $spec['password'])) {
 			$result->error(_t (
 				'Member.ERRORWRONGCRED',
 				'That doesn\'t seem to be the right e-mail address or password. Please try again.'

security/MemberPassword.php

@@ -42,13 +42,8 @@ class MemberPassword extends DataObject {
 	 * @return Boolean
 	 */	
 	function checkPassword($password) {
-		$spec = Security::encrypt_password(
+		$e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
-			$password, 
+		return $e->check($this->Password, $password, $this->Salt, $this->Member());
-			$this->Salt, 
-			$this->PasswordEncryption
-		);
-		$e = $spec['encryptor'];
-		return $e->compare($this->Password, $spec['password']);
 	}
 	
 	

security/PasswordEncryptor.php

@@ -109,10 +109,23 @@ abstract class PasswordEncryptor {
 	 * @param String $hash1
 	 * @param String $hash2
 	 * @return boolean
+	 *
+	 * @deprecated 3.0 - Use PasswordEncryptor::check() instead.
 	 */
 	function compare($hash1, $hash2) {
+		Deprecation::notice('3.0.0', 'PasswordEncryptor::compare() is deprecated, replaced by PasswordEncryptor::check().');
 		return ($hash1 === $hash2);
 	}
+
+	/**
+	 * This usually just returns a strict string comparison,
+	 * but is necessary for retain compatibility with password hashed
+	 * with flawed algorithms - see {@link PasswordEncryptor_LegacyPHPHash} and
+	 * {@link PasswordEncryptor_Blowfish}
+	 */
+	function check($hash, $password, $salt = null, $member = null) {
+		return $hash === $this->encrypt($password, $salt, $member);
+	}
 }
 
 /**
@@ -217,10 +230,18 @@ class PasswordEncryptor_LegacyPHPHash extends PasswordEncryptor_PHPHash {
 	}
 	
 	function compare($hash1, $hash2) {
+		Deprecation::notice('3.0.0', 'PasswordEncryptor::compare() is deprecated, replaced by PasswordEncryptor::check().');
+
 		// Due to flawed base_convert() floating poing precision, 
 		// only the first 10 characters are consistently useful for comparisons.
 		return (substr($hash1, 0, 10) === substr($hash2, 0, 10));
 	}
+
+	function check($hash, $password, $salt = null, $member = null) {
+		// Due to flawed base_convert() floating poing precision, 
+		// only the first 10 characters are consistently useful for comparisons.
+		return (substr($hash, 0, 10) === substr($this->encrypt($password, $salt, $member), 0, 10));
+	}
 }
 
 /**

tests/security/PasswordEncryptorTest.php

@@ -72,11 +72,11 @@ class PasswordEncryptorTest extends SapphireTest {
 		);
 	}
 	
-	function testEncryptorPHPHashCompare() {
+	function testEncryptorPHPHashCheck() {
 		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1'=>array('PasswordEncryptor_PHPHash'=>'sha1')));
 		$e = PasswordEncryptor::create_for_algorithm('test_sha1');
-		$this->assertTrue($e->compare(sha1('mypassword'), sha1('mypassword')));
+		$this->assertTrue($e->check(sha1('mypassword'), 'mypassword'));
-		$this->assertFalse($e->compare(sha1('mypassword'), sha1('mywrongpassword')));
+		$this->assertFalse($e->check(sha1('mypassword'), 'mywrongpassword'));
 	}
 	
 	/**
@@ -85,15 +85,16 @@ class PasswordEncryptorTest extends SapphireTest {
 	 * Handy command for reproducing via CLI on different architectures:
 	 * 	php -r "echo(base_convert(sha1('mypassword'), 16, 36));"
 	 */
-	function testEncryptorLegacyPHPHashCompare() {
+	function testEncryptorLegacyPHPHashCheck() {
 		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1legacy'=>array('PasswordEncryptor_LegacyPHPHash'=>'sha1')));
 		$e = PasswordEncryptor::create_for_algorithm('test_sha1legacy');
 		// precomputed hashes for 'mypassword' from different architectures
 		$amdHash = 'h1fj0a6m4o6k0sosks88oo08ko4gc4s';
 		$intelHash = 'h1fj0a6m4o0g04ocg00o4kwoc4wowws';
 		$wrongHash = 'h1fjxxxxxxxxxxxxxxxxxxxxxxxxxxx';
-		$this->assertTrue($e->compare($amdHash, $intelHash));
+		$this->assertTrue($e->check($amdHash, "mypassword"));
-		$this->assertFalse($e->compare($amdHash, $wrongHash));
+		$this->assertTrue($e->check($intelHash, "mypassword"));
+		$this->assertFalse($e->check($wrongHash, "mypassword"));
 	}
 }