From e4ca24c7e2dfc1f57952b11203d9131850a98b94 Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Fri, 6 Nov 2009 02:23:30 +0000
Subject: [PATCH] BUGFIX Legacy password hash migration in
 MemberAuthenticator::authenticate() which fixes the precision problems
 mentioned in #3004 when a user logs in

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@90950 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 security/MemberAuthenticator.php           | 31 ++++++++++++++-
 tasks/EncryptAllPasswordsTask.php          |  1 +
 tests/security/MemberAuthenticatorTest.php | 44 ++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 tests/security/MemberAuthenticatorTest.php

diff --git a/security/MemberAuthenticator.php b/security/MemberAuthenticator.php
index fe3bd88dc..f71446cec 100644
--- a/security/MemberAuthenticator.php
+++ b/security/MemberAuthenticator.php
@@ -8,6 +8,16 @@
  */
 class MemberAuthenticator extends Authenticator {
 
+	/**
+	 * @var Array Contains encryption algorithm identifiers.
+	 *  If set, will migrate to new precision-safe password hashing
+	 *  upon login. See http://open.silverstripe.org/ticket/3004.
+	 */
+	static $migrate_legacy_hashes = array(
+		'md5' => 'md5_v2.4', 
+		'sha1' => 'sha1_v2.4'
+	);
+
   /**
    * Method to authenticate an user
    *
@@ -27,7 +37,11 @@ class MemberAuthenticator extends Authenticator {
 	if(Security::check_default_admin($RAW_data['Email'], $RAW_data['Password'])) {
 		$member = Security::findAnAdministrator();
 	} else {
-		$member = DataObject::get_one("Member", "\"Email\" = '$SQL_user' AND \"Password\" IS NOT NULL");
+		$member = DataObject::get_one(
+			"Member", 
+			"\"Email\" = '$SQL_user' AND \"Password\" IS NOT NULL"
+		);
+		
 		if($member && ($member->checkPassword($RAW_data['Password']) == false)) { 
 			if($member->isLockedOut()) $isLockedOut = true;
 			$member->registerFailedLogin();
@@ -72,6 +86,21 @@ class MemberAuthenticator extends Authenticator {
 		$attempt->IP = Controller::curr()->getRequest()->getIP();
 		$attempt->write();
 	}
+	
+	// Legacy migration to precision-safe password hashes.
+	// A login-event with cleartext passwords is the only time
+	// when we can rehash passwords to a different hashing algorithm,
+	// bulk-migration doesn't work due to the nature of hashing.
+	// See PasswordEncryptor_LegacyPHPHash class.
+	if(
+		$member // only migrate after successful login
+		&& self::$migrate_legacy_hashes
+		&& array_key_exists($member->PasswordEncryption, self::$migrate_legacy_hashes)
+	) {
+		$member->Password = $RAW_data['Password'];
+		$member->PasswordEncryption = self::$migrate_legacy_hashes[$member->PasswordEncryption];
+		$member->write();
+	}
 
     if($member) {
 		Session::clear("BackURL");
diff --git a/tasks/EncryptAllPasswordsTask.php b/tasks/EncryptAllPasswordsTask.php
index 1fc11843b..b2724d8a9 100644
--- a/tasks/EncryptAllPasswordsTask.php
+++ b/tasks/EncryptAllPasswordsTask.php
@@ -55,6 +55,7 @@ class EncryptAllPasswordsTask extends DailyTask {
 			// automatically encrypted according to the settings, this will do all
 			// the work for us
 			$member->PasswordEncryption = $algo;
+			$member->forceChange();
 			$member->write();
 			
 			$this->debugMessage(sprintf('Encrypted credentials for member #%d;', $member->ID));
diff --git a/tests/security/MemberAuthenticatorTest.php b/tests/security/MemberAuthenticatorTest.php
new file mode 100644
index 000000000..6e6ad2615
--- /dev/null
+++ b/tests/security/MemberAuthenticatorTest.php
@@ -0,0 +1,44 @@
+<?php
+/**
+ * @package sapphire
+ * @subpackage tests
+ */
+class MemberAuthenticatorTest extends SapphireTest {
+	function testLegacyPasswordHashMigrationUponLogin() {
+		$member = new Member();
+		$member->Email = 'test@test.com';
+		$member->PasswordEncryption = "sha1";
+		$member->Password = "mypassword";
+		$member->write();
+		
+		$data = array(
+			'Email' => $member->Email,
+			'Password' => 'mypassword'
+		);
+		MemberAuthenticator::authenticate($data);
+		
+		$member = DataObject::get_by_id('Member', $member->ID);
+		$this->assertEquals($member->PasswordEncryption, "sha1_v2.4");
+		$this->assertTrue($member->checkPassword('mypassword'));
+	}
+	
+	function testNoLegacyPasswordHashMigrationOnIncompatibleAlgorithm() {
+		PasswordEncryptor::register('crc32', 'PasswordEncryptor_PHPHash("crc32")');
+		
+		$member = new Member();
+		$member->Email = 'test@test.com';
+		$member->PasswordEncryption = "crc32";
+		$member->Password = "mypassword";
+		$member->write();
+		
+		$data = array(
+			'Email' => $member->Email,
+			'Password' => 'mypassword'
+		);
+		MemberAuthenticator::authenticate($data);
+		
+		$member = DataObject::get_by_id('Member', $member->ID);
+		$this->assertEquals($member->PasswordEncryption, "crc32");
+		$this->assertTrue($member->checkPassword('mypassword'));
+	}
+}
\ No newline at end of file