API Hash autologin tokens before storing in the database.

Backported from 3.0, cc423c38fb.

security/Member.php

@@ -11,11 +11,11 @@ class Member extends DataObject {
 		'Surname' => 'Varchar',
 		'Email' => 'Varchar',
 		'Password' => 'Varchar(160)',
-		'RememberLoginToken' => 'Varchar(50)',
+		'RememberLoginToken' => 'Varchar(160)', // Note: this currently holds a hash, not a token.
 		'NumVisit' => 'Int',
 		'LastVisited' => 'SS_Datetime',
 		'Bounced' => 'Boolean', // Note: This does not seem to be used anywhere.
-		'AutoLoginHash' => 'Varchar(50)',
+		'AutoLoginHash' => 'Varchar(160)',
 		'AutoLoginExpired' => 'SS_Datetime',
 		// This is an arbitrary code pointing to a PasswordEncryptor instance,
 		// not an actual encryption algorithm.
@@ -327,9 +327,11 @@ class Member extends DataObject {
 		$this->NumVisit++;
 
 		if($remember) {
+			// Store the hash and give the client the cookie with the token.
 			$generator = new RandomGenerator();
-			$token = $generator->generateHash('sha1');
-			$this->RememberLoginToken = $token;
+			$token = $generator->randomToken('sha1');
+			$hash = $this->encryptWithUserSettings($token);
+			$this->RememberLoginToken = $hash;
 			Cookie::set('alc_enc', $this->ID . ':' . $token, 90, null, null, null, true);
 		} else {
 			$this->RememberLoginToken = null;
@@ -387,7 +389,8 @@ class Member extends DataObject {
 			$member = DataObject::get_one("Member", "\"Member\".\"ID\" = '$SQL_uid'");
 
 			// check if autologin token matches
-			if($member && (!$member->RememberLoginToken || $member->RememberLoginToken != $token)) {
+			$hash = $member->encryptWithUserSettings($token);
+			if($member && (!$member->RememberLoginToken || $member->RememberLoginToken != $hash)) {
 				$member = null;
 			}
 
@@ -398,7 +401,9 @@ class Member extends DataObject {
 				if(self::$login_marker_cookie) Cookie::set(self::$login_marker_cookie, 1, 0, null, null, false, true);
 				
 				$generator = new RandomGenerator();
-				$member->RememberLoginToken = $generator->generateHash('sha1');
+				$token = $generator->randomToken('sha1');
+				$hash = $member->encryptWithUserSettings($token);
+				$member->RememberLoginToken = $hash;
 				Cookie::set('alc_enc', $member->ID . ':' . $token, 90, null, null, false, true);
 
 				$member->NumVisit++;
@@ -430,27 +435,76 @@ class Member extends DataObject {
 		$this->extend('memberLoggedOut');
 	}
 
+	/**
+	 * Utility for generating secure password hashes for this member.
+	 */
+	public function encryptWithUserSettings($string) {
+		if (!$string) return null;
+
+		// If the algorithm or salt is not available, it means we are operating
+		// on legacy account with unhashed password. Do not hash the string.
+		if (!$this->PasswordEncryption) {
+			return $string;
+		}
+
+		// We assume we have PasswordEncryption and Salt available here.
+		$e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
+		return $e->encrypt($string, $this->Salt);
+
+	}
 
 	/**
-	 * Generate an auto login hash
-	 *
-	 * This creates an auto login hash that can be used to reset the password.
+	 * Generate an auto login token which can be used to reset the password,
+	 * at the same time hashing it and storing in the database.
 	 *
 	 * @param int $lifetime The lifetime of the auto login hash in days (by default 2 days)
 	 *
+	 * @returns string Token that should be passed to the client (but NOT persisted).
+	 *
 	 * @todo Make it possible to handle database errors such as a "duplicate key" error
 	 */
-	function generateAutologinHash($lifetime = 2) {
-
+	public function generateAutologinTokenAndStoreHash($lifetime = 2) {
 		do {
 			$generator = new RandomGenerator();
-			$hash = $generator->generateHash('sha1');
+			$token = $generator->randomToken();
+			$hash = $this->encryptWithUserSettings($token);
 		} while(DataObject::get_one('Member', "\"AutoLoginHash\" = '$hash'"));
 
 		$this->AutoLoginHash = $hash;
 		$this->AutoLoginExpired = date('Y-m-d', time() + (86400 * $lifetime));
 
 		$this->write();
+
+		return $token;
+	}
+
+	/**
+	 * @deprecated 2.4
+	 */
+	public function generateAutologinHash($lifetime = 2) {
+		user_error(
+			'Member::generateAutologinHash is deprecated - tokens are no longer saved directly into the database '.
+			'in plaintext. Use the return value of the Member::generateAutologinTokenAndHash to get the token '.
+			'instead.',
+			E_USER_ERROR);
+	}
+
+	/**
+	 * Check the token against the member.
+	 *
+	 * @param string $autologinToken
+	 *
+	 * @returns bool Is token valid?
+	 */
+	public function validateAutoLoginToken($autologinToken) {
+		$hash = $this->encryptWithUserSettings($autologinToken);
+
+		$member = DataObject::get_one(
+			'Member',
+			"\"AutoLoginHash\"='" . $hash . "' AND \"AutoLoginExpired\" > " . DB::getConn()->now()
+		);
+
+		return (bool)$member;
 	}
 
 	/**
@@ -469,7 +523,6 @@ class Member extends DataObject {
 		return $member;
 	}
 
-
 	/**
 	 * Send signup, change password or forgot password informations to an user
 	 *

security/MemberLoginForm.php

@@ -229,12 +229,12 @@ JS
 		$member = DataObject::get_one('Member', "\"Email\" = '{$SQL_email}'");
 
 		if($member) {
-			$member->generateAutologinHash();
+			$token = $member->generateAutologinTokenAndStoreHash();
 
 			$member->sendInfo(
 				'forgotPassword',
 				array(
-					'PasswordResetLink' => Security::getPasswordResetLink($member->AutoLoginHash)
+					'PasswordResetLink' => Security::getPasswordResetLink($member, $token)
 				)
 			);
 

security/PasswordEncryptor.php

@@ -91,7 +91,7 @@ abstract class PasswordEncryptor {
 	 */
 	function salt($password, $member = null) {
 		$generator = new RandomGenerator();
-		return substr($generator->generateHash('sha1'), 0, 50);
+		return substr($generator->randomToken('sha1'), 0, 50);
 	}
 	
 	/**

security/RandomGenerator.php

@@ -59,14 +59,24 @@ class RandomGenerator {
 	}
 
 	/**
-	 * Generates a hash suitable for manual session identifiers, CSRF tokens, etc.
+	 * Generates a random token that can be used for session IDs, CSRF tokens etc., based on
+	 * hash algorithms.
+	 *
+	 * If you are using it as a password equivalent (e.g. autologin token) do NOT store it 
+	 * in the database as a plain text but encrypt it with Member::encryptWithUserSettings.
 	 * 
 	 * @param String $algorithm Any identifier listed in hash_algos() (Default: whirlpool)
-	 *  If possible, choose a slow algorithm which complicates brute force attacks.
+	 *
 	 * @return String Returned length will depend on the used $algorithm
 	 */
-	function generateHash($algorithm = 'whirlpool') {
+	function randomToken($algorithm = 'whirlpool') {
 		return hash($algorithm, $this->generateEntropy());
 	}
 
+	/**
+	 * @deprecated 3.1
+	 */
+	public function generateHash($algorithm = 'whirlpool') {
+		return $this->randomToken($algorithm);
+	}
 }

security/Security.php

@@ -502,13 +502,18 @@ class Security extends Controller {
 
 
 	/**
-	 * Create a link to the password reset form
+	 * Create a link to the password reset form.
 	 *
-	 * @param string $autoLoginHash The auto login hash
+	 * GET parameters used:
+	 * - m: member ID
+	 * - t: plaintext token
+	 *
+	 * @param Member $member Member object associated with this link.
+	 * @param string $autoLoginHash The auto login token.
 	 */
-	public static function getPasswordResetLink($autoLoginHash) {
-		$autoLoginHash = urldecode($autoLoginHash);
-		return self::Link('changepassword') . "?h=$autoLoginHash";
+	public static function getPasswordResetLink($member, $autologinToken) {
+		$autologinToken = urldecode($autologinToken);
+		return self::Link('changepassword') . "?m={$member->ID}&t=$autologinToken";
 	}
 	
 	/**
@@ -531,15 +536,22 @@ class Security extends Controller {
 		$controller = new Page_Controller($tmpPage);
 		$controller->init();
 
-		// First load with hash: Redirect to same URL without hash to avoid referer leakage
-		if(isset($_REQUEST['h']) && Member::member_from_autologinhash($_REQUEST['h'])) {
-			// The auto login hash is valid, store it for the change password form.
-			// Temporary value, unset in ChangePasswordForm
-			Session::set('AutoLoginHash', $_REQUEST['h']);
+		// Extract the member from the URL.
+		$member = null;
+		if (isset($_REQUEST['m'])) {
+			$member = DataObject::get_by_id('Member', (int)$_REQUEST['m']);
+		}
+
+		// Check whether we are merely changin password, or resetting.
+		if(isset($_REQUEST['t']) && $member && $member->validateAutoLoginToken($_REQUEST['t'])) {
+			// On first valid password reset request redirect to the same URL without hash to avoid referrer leakage.
+
+			// Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm.
+			Session::set('AutoLoginHash', $member->encryptWithUserSettings($_REQUEST['t']));
 			
 			return $this->redirect($this->Link('changepassword'));
-		// Redirection target after "First load with hash"
 		} elseif(Session::get('AutoLoginHash')) {
+			// Subsequent request after the "first load with hash" (see previous if clause).
 			$customisedController = $controller->customise(array(
 				'Content' =>
 					'<p>' . 
@@ -548,15 +560,15 @@ class Security extends Controller {
 				'Form' => $this->ChangePasswordForm(),
 			));
 		} elseif(Member::currentUser()) {
-			// let a logged in user change his password
+			// Logged in user requested a password change form.
 			$customisedController = $controller->customise(array(
 				'Content' => '<p>' . _t('Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '</p>',
 				'Form' => $this->ChangePasswordForm()));
 
 		} else {
-			// show an error message if the auto login hash is invalid and the
+			// show an error message if the auto login token is invalid and the
 			// user is not logged in
-			if(isset($_REQUEST['h'])) {
+			if(!isset($_REQUEST['t']) || !$member) {
 				$customisedController = $controller->customise(
 					array('Content' =>
 						sprintf(

security/SecurityToken.php

@@ -206,7 +206,7 @@ class SecurityToken extends Object {
 	 */
 	protected function generate() {
 		$generator = new RandomGenerator();
-		return $generator->generateHash('sha1');
+		return $generator->randomToken('sha1');
 	}
 	
 }

tests/security/MemberTest.php

@@ -557,6 +557,35 @@ class MemberTest extends FunctionalTest {
 		);
 	}
 
+	public function testGenerateAutologinTokenAndStoreHash() {
+		$enc = new PasswordEncryptor_PHPHash('sha1');
+
+		$m = new Member();
+		$m->PasswordEncryption = 'sha1';
+		$m->Salt = $enc->salt('123');
+
+		$token = $m->generateAutologinTokenAndStoreHash();
+
+		$this->assertEquals($m->encryptWithUserSettings($token), $m->AutoLoginHash, 'Stores the token as ahash.');
+	}
+
+	public function testValidateAutoLoginToken() {
+		$enc = new PasswordEncryptor_PHPHash('sha1');
+
+		$m1 = new Member();
+		$m1->PasswordEncryption = 'sha1';
+		$m1->Salt = $enc->salt('123');
+		$m1Token = $m1->generateAutologinTokenAndStoreHash();
+
+		$m2 = new Member();
+		$m2->PasswordEncryption = 'sha1';
+		$m2->Salt = $enc->salt('456');
+		$m2Token = $m2->generateAutologinTokenAndStoreHash();
+		
+		$this->assertTrue($m1->validateAutoLoginToken($m1Token), 'Passes token validity test against matching member.');
+		$this->assertFalse($m2->validateAutoLoginToken($m1Token), 'Fails token validity test against other member.');
+	}
+
 	/**
 	 * Add the given array of member extensions as class names.
 	 * This is useful for re-adding extensions after being removed

tests/security/RandomGeneratorTest.php

@@ -14,14 +14,14 @@ class RandomGeneratorTest extends SapphireTest {
 	
 	function testGenerateHash() {
 		$r = new RandomGenerator();
-		$this->assertNotNull($r->generateHash());
-		$this->assertNotEquals($r->generateHash(), $r->generateHash());
+		$this->assertNotNull($r->randomToken());
+		$this->assertNotEquals($r->randomToken(), $r->randomToken());
 	}
 	
 	function testGenerateHashWithAlgorithm() {
 		$r = new RandomGenerator();
-		$this->assertNotNull($r->generateHash('md5'));
-		$this->assertNotEquals($r->generateHash(), $r->generateHash('md5'));
+		$this->assertNotNull($r->randomToken('md5'));
+		$this->assertNotEquals($r->randomToken(), $r->randomToken('md5'));
 	}
 	
 }

tests/security/SecurityTest.php

@@ -196,7 +196,12 @@ class SecurityTest extends FunctionalTest {
 		// Load password link from email
 		$admin = DataObject::get_by_id('Member', $admin->ID);
 		$this->assertNotNull($admin->AutoLoginHash, 'Hash has been written after lost password');
-		$response = $this->get('Security/changepassword/?h=' . $admin->AutoLoginHash);
+
+		// We don't have access to the token - generate a new token and hash pair.
+		$token = $admin->generateAutologinTokenAndStoreHash();
+
+		// Check.
+		$response = $this->get('Security/changepassword/?m='.$admin->ID.'&t=' . $token);
 		$this->assertEquals(302, $response->getStatusCode());
 		$this->assertEquals(Director::baseUrl() . 'Security/changepassword', $response->getHeader('Location'));