Merge branch '3.0.3' into 3.0

README.md

@@ -1,6 +1,6 @@
 ## SilverStripe Framework
 
-[![Build Status](https://secure.travis-ci.org/silverstripe/sapphire.png)](http://travis-ci.org/silverstripe/sapphire)
+[![Build Status](https://secure.travis-ci.org/silverstripe/sapphire.png?branch=3.0)](https://travis-ci.org/silverstripe/sapphire)
 
 PHP5 framework forming the base for the SilverStripe CMS ([http://silverstripe.org](http://silverstripe.org)). 
 Requires a [`silverstripe-installer`](http://github.com/silverstripe/silverstripe-installer) base project. Typically used alongside the [`cms`](http://github.com/silverstripe/silverstripe-cms) module.

docs/en/changelogs/rc/3.0.3-rc2.md

@@ -0,0 +1,44 @@
+# 3.0.3-rc2 (2012-11-16)
+
+## Overview
+
+3.0.3 provides security fixes, bugfixes and a number of minor enhancements since 3.0.2.
+
+Upgrading from 3.0.x should be a straightforward matter of dropping in the new release,
+with the exception noted below.
+
+## Upgrading
+
+Impact of the upgrade:
+
+* Reset password email links generated prior to 3.0.3 will cease to work.
+* Users who use the "remember me" login feature will have to log in again.
+
+API changes related to the below security patch:
+
+* `Member::generateAutologinHash` is deprecated. You can no longer get the autologin token from `AutoLoginHash` field in `Member`. Instead use the return value of the `Member::generateAutologinTokenAndStoreHash` and do not persist it.
+* `Security::getPasswordResetLink` now requires `Member` object as the first parameter. The password reset URL GET parameters have changed from only `h` (for hash) to `m` (for member ID) and `t` (for plaintext token).
+* `RandomGenerator::generateHash` will be deprecated with 3.1. Rename the function call to `RandomGenerator::randomToken`.
+
+### Security: Hash autologin tokens before storing in the database.
+
+Severity: Moderate
+
+Autologin tokens (remember me and reset password) are stored in the database as a plain text.
+If attacker obtained the database he would be able to gain access to accounts that have requested a password change, or have "remember me" enabled.
+
+## Changelog
+
+### API Changes
+
+ * 2012-11-16 [0dd97a3](https://github.com/silverstripe/sapphire/commit/0dd97a3) Form#loadDataFrom 2nd arg now sets how existing field data is merged with new data (Hamish Friedlander)
+ * 2012-11-08 [a8b0e44](https://github.com/silverstripe/sapphire/commit/a8b0e44) Hash autologin tokens before storing in the database. (Mateusz Uzdowski)
+
+### Bugfixes
+
+ * 2012-11-16 [7315be4](https://github.com/silverstripe/sapphire/commit/7315be4) default values from DataObject not showing in GridField details form (Hamish Friedlander)
+ * 2012-11-15 [78ab9d3](https://github.com/silverstripe/sapphire/commit/78ab9d3) Video embed from Add Media Feature no longer works (open #8033) (stojg)
+
+### Other
+
+ * 2012-11-09 [05a44e8](https://github.com/silverstripe/sapphire/commit/05a44e8) Correct branch for Travis build status image (Ingo Schommer)

forms/Form.php

@@ -1080,6 +1080,10 @@ class Form extends RequestHandler {
 		return true;
 	}
 
+	const MERGE_DEFAULT = 0;
+	const MERGE_CLEAR_MISSING = 1;
+	const MERGE_IGNORE_FALSEISH = 2;
+
 	/**
 	 * Load data from the given DataObject or array.
 	 * It will call $object->MyField to get the value of MyField.
@@ -1098,20 +1102,43 @@ class Form extends RequestHandler {
 	 * @uses FormField->setValue()
 	 * 
 	 * @param array|DataObject $data
-	 * @param boolean $clearMissingFields By default, fields which don't match
+	 * @param int $mergeStrategy
-	 *  a property or array-key of the passed {@link $data} argument are "left alone",
+	 *  For every field, {@link $data} is interogated whether it contains a relevant property/key, and
-	 *  meaning they retain any previous values (if present). If this flag is set to true,
+	 *  what that property/key's value is.
-	 *  those fields are overwritten with null regardless if they have a match in {@link $data}.
+	 *
+	 *  By default, if {@link $data} does contain a property/key, the fields value is always replaced by {@link $data}'s
+	 *  value, even if that value is null/false/etc. Fields which don't match any property/key in {@link $data} are
+	 *  "left alone", meaning they retain any previous value.
+	 *
+	 *  You can pass a bitmask here to change this behaviour.
+	 *
+	 *  Passing CLEAR_MISSING means that any fields that don't match any property/key in
+	 *  {@link $data} are cleared.
+	 *
+	 *  Passing IGNORE_FALSEISH means that any false-ish value in {@link $data} won't replace
+	 *  a field's value.
+	 *
+	 *  For backwards compatibility reasons, this parameter can also be set to === true, which is the same as passing
+	 *  CLEAR_MISSING
+	 *
 	 * @param $fieldList An optional list of fields to process.  This can be useful when you have a
 	 * form that has some fields that save to one object, and some that save to another.
 	 * @return Form
 	 */
-	public function loadDataFrom($data, $clearMissingFields = false, $fieldList = null) {
+	public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null) {
 		if(!is_object($data) && !is_array($data)) {
 			user_error("Form::loadDataFrom() not passed an array or an object", E_USER_WARNING);
 			return $this;
 		}
 
+		// Handle the backwards compatible case of passing "true" as the second argument
+		if ($mergeStrategy === true) {
+			$mergeStrategy = self::MERGE_CLEAR_MISSING;
+		}
+		else if ($mergeStrategy === false) {
+			$mergeStrategy = 0;
+		}
+
 		// if an object is passed, save it for historical reference through {@link getRecord()}
 		if(is_object($data)) $this->record = $data;
 
@@ -1126,36 +1153,49 @@ class Form extends RequestHandler {
 			// First check looks for (fieldname)_unchanged, an indicator that we shouldn't overwrite the field value
 			if(is_array($data) && isset($data[$name . '_unchanged'])) continue;
 
-			// get value in different formats
+			// Does this property exist on $data?
-			$hasObjectValue = false;
+			$exists = false;
-			if(
+			// The value from $data for this field
-				is_object($data) 
+			$val = null;
-				&& (
+
-					isset($data->$name)
+			if(is_object($data)) {
-					|| $data->hasMethod($name)
+				$exists = (
-					|| ($data->hasMethod('hasField') && $data->hasField($name))
+					isset($data->$name) ||
-				)
+					$data->hasMethod($name) ||
-			) {
+					($data->hasMethod('hasField') && $data->hasField($name))
-				// We don't actually call the method because it might be slow.  
+				);
-				// In a later release, relation methods will just return references to the query that should be
+
-				// executed, and so we will be able to safely pass the return value of the relation method to the
+				if ($exists) {
-				// first argument of setValue
+					$val = $data->__get($name);
-				$val = $data->__get($name);
+				}
-				$hasObjectValue = true;
+			}
-			} else if(strpos($name,'[') && is_array($data) && !isset($data[$name])) {
+			else if(is_array($data)){
-				// if field is in array-notation, we need to resolve the array-structure PHP creates from query-strings
+				if(array_key_exists($name, $data)) {
-				preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', urldecode(http_build_query($data)), $matches);
+					$exists = true;
-				$val = isset($matches[1]) ? $matches[1] : null;
+					$val = $data[$name];
-			} elseif(is_array($data) && array_key_exists($name, $data)) {
+				}
-				// else we assume its a simple keyed array
+				// If field is in array-notation we need to access nested data
-				$val = $data[$name];
+				else if(strpos($name,'[')) {
-			} else {
+					// First encode data using PHP's method of converting nested arrays to form data
-				$val = null;
+					$flatData = urldecode(http_build_query($data));
+					// Then pull the value out from that flattened string
+					preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', $flatData, $matches);
+
+					if (isset($matches[1])) {
+						$exists = true;
+						$val = $matches[1];
+					}
+				}
 			}
 
 			// save to the field if either a value is given, or loading of blank/undefined values is forced
-			if(isset($val) || $hasObjectValue || $clearMissingFields) {
+			if($exists){
-				// pass original data as well so composite fields can act on the additional information
+				if ($val != false || ($mergeStrategy & self::MERGE_IGNORE_FALSEISH) != self::MERGE_IGNORE_FALSEISH){
+					// pass original data as well so composite fields can act on the additional information
+					$field->setValue($val, $data);
+				}
+			}
+			else if(($mergeStrategy & self::MERGE_CLEAR_MISSING) == self::MERGE_CLEAR_MISSING){
 				$field->setValue($val, $data);
 			}
 		}

forms/gridfield/GridFieldDetailForm.php

@@ -325,9 +325,8 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 			$actions,
 			$this->component->getValidator()
 		);
-		if($this->record->ID !== 0) {
+
-			$form->loadDataFrom($this->record);
+		$form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT);
-		}
 
 		// TODO Coupling with CMS
 		$toplevelController = $this->getToplevelController();

security/Member.php

@@ -12,11 +12,11 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		'Surname' => 'Varchar',
 		'Email' => 'Varchar(256)', // See RFC 5321, Section 4.5.3.1.3.
 		'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.
@@ -322,9 +322,11 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		$this->NumVisit++;
 
 		if($remember) {
+			// Store the hash and give the client the cookie with the token.
 			$generator = new RandomGenerator();
-			$token = $generator->generateHash('sha1');
+			$token = $generator->randomToken('sha1');
-			$this->RememberLoginToken = $token;
+			$hash = $this->encryptWithUserSettings($token);
+			$this->RememberLoginToken = $hash;
 			Cookie::set('alc_enc', $this->ID . ':' . $token, 90, null, null, null, true);
 		} else {
 			$this->RememberLoginToken = null;
@@ -382,7 +384,8 @@ class Member extends DataObject implements TemplateGlobalProvider {
 			$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;
 			}
 
@@ -393,8 +396,10 @@ class Member extends DataObject implements TemplateGlobalProvider {
 				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');
-				Cookie::set('alc_enc', $member->ID . ':' . $member->RememberLoginToken, 90, null, null, false, true);
+				$hash = $member->encryptWithUserSettings($token);
+				$member->RememberLoginToken = $hash;
+				Cookie::set('alc_enc', $member->ID . ':' . $token, 90, null, null, false, true);
 
 				$member->NumVisit++;
 				$member->write();
@@ -425,27 +430,82 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		$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
+	 * Generate an auto login token which can be used to reset the password,
-	 *
+	 * at the same time hashing it and storing in the database.
-	 * This creates an auto login hash that can be used to reset the password.
 	 *
 	 * @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
 	 */
-	public 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 3.0
+	 */
+	public function generateAutologinHash($lifetime = 2) {
+		Deprecation::notice('3.0', 
+			'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.',
+			Deprecation::SCOPE_METHOD);
+
+		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;
 	}
 
 	/**
@@ -467,7 +527,6 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		return $member;
 	}
 
-
 	/**
 	 * Send signup, change password or forgot password informations to an user
 	 *

security/MemberLoginForm.php

@@ -258,12 +258,12 @@ JS
 		$member = DataObject::get_one('Member', "\"Email\" = '{$SQL_email}'");
 
 		if($member) {
-			$member->generateAutologinHash();
+			$token = $member->generateAutologinTokenAndStoreHash();
 
 			$e = Member_ForgotPasswordEmail::create();
 			$e->populateTemplate($member);
 			$e->populateTemplate(array(
-				'PasswordResetLink' => Security::getPasswordResetLink($member->AutoLoginHash)
+				'PasswordResetLink' => Security::getPasswordResetLink($member, $token)
 			));
 			$e->setTo($member->Email);
 			$e->send();

security/PasswordEncryptor.php

@@ -99,7 +99,7 @@ abstract class PasswordEncryptor {
 	 */
 	public function salt($password, $member = null) {
 		$generator = new RandomGenerator();
-		return substr($generator->generateHash('sha1'), 0, 50);
+		return substr($generator->randomToken('sha1'), 0, 50);
 	}
 	
 	/**
@@ -281,7 +281,7 @@ class PasswordEncryptor_Blowfish extends PasswordEncryptor {
 	 */
 	public function salt($password, $member = null) {
 		$generator = new RandomGenerator();
-		return sprintf('%02d', self::$cost) . '$' . substr($generator->generateHash('sha1'), 0, 22);
+		return sprintf('%02d', self::$cost) . '$' . substr($generator->randomToken('sha1'), 0, 22);
 	}
 
 	public function check($hash, $password, $salt = null, $member = null) {

security/RandomGenerator.php

@@ -60,13 +60,29 @@ 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
 	 */
-	public function generateHash($algorithm = 'whirlpool') {
+	public function randomToken($algorithm = 'whirlpool') {
 		return hash($algorithm, $this->generateEntropy());
 	}
+
+	/**
+	 * @deprecated 3.1
+	 */
+	public function generateHash($algorithm = 'whirlpool') {
+		Deprecation::notice('3.1',
+			'RandomGenerator::generateHash is deprecated because of a confusing name that hints the output is secure, '.
+			'while in fact it is just a random string. Use RandomGenerator::randomToken instead.',
+			Deprecation::SCOPE_METHOD);
+
+		return $this->randomToken($algorithm);
+	}
 }

security/Security.php

@@ -532,15 +532,20 @@ 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) {
+	public static function getPasswordResetLink($member, $autologinToken) {
-		$autoLoginHash = urldecode($autoLoginHash);
+		$autologinToken = urldecode($autologinToken);
 		$selfControllerClass = __CLASS__;
 		$selfController = new $selfControllerClass();
-		return $selfController->Link('changepassword') . "?h=$autoLoginHash";
+		return $selfController->Link('changepassword') . "?m={$member->ID}&t=$autologinToken";
 	}
 	
 	/**
@@ -567,15 +572,22 @@ class Security extends Controller {
 			$controller = $this;
 		}
 
-		// First load with hash: Redirect to same URL without hash to avoid referer leakage
+		// Extract the member from the URL.
-		if(isset($_REQUEST['h']) && Member::member_from_autologinhash($_REQUEST['h'])) {
+		$member = null;
-			// The auto login hash is valid, store it for the change password form.
+		if (isset($_REQUEST['m'])) {
-			// Temporary value, unset in ChangePasswordForm
+			$member = Member::get()->filter('ID', (int)$_REQUEST['m'])->First();
-			Session::set('AutoLoginHash', $_REQUEST['h']);
+		}
+
+		// 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>' . 
@@ -584,16 +596,16 @@ 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' =>
 						_t(

security/SecurityToken.php

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

tests/forms/FormTest.php

@@ -153,7 +153,7 @@ class FormTest extends FunctionalTest {
 		$captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails');
 		$team2 = $this->objFromFixture('FormTest_Team', 'team2');
 		$form->loadDataFrom($captainWithDetails);
-		$form->loadDataFrom($team2, true);
+		$form->loadDataFrom($team2, Form::MERGE_CLEAR_MISSING);
 		$this->assertEquals(
 			$form->getData(), 
 			array(
@@ -167,6 +167,34 @@ class FormTest extends FunctionalTest {
 		);
 	}
 
+	public function testLoadDataFromIgnoreFalseish() {
+		$form = new Form(
+			new Controller(),
+			'Form',
+			new FieldList(
+				new TextField('Biography', 'Biography', 'Custom Default')
+			),
+			new FieldList()
+		);
+
+		$captainNoDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails');
+		$captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainWithDetails');
+
+		$form->loadDataFrom($captainNoDetails, Form::MERGE_IGNORE_FALSEISH);
+		$this->assertEquals(
+			$form->getData(),
+			array('Biography' => 'Custom Default'),
+			'LoadDataFrom() doesn\'t overwrite fields when MERGE_IGNORE_FALSEISH set and values are false-ish'
+		);
+
+		$form->loadDataFrom($captainWithDetails, Form::MERGE_IGNORE_FALSEISH);
+		$this->assertEquals(
+			$form->getData(),
+			array('Biography' => 'Bio 1'),
+			'LoadDataFrom() does overwrite fields when MERGE_IGNORE_FALSEISH set and values arent false-ish'
+		);
+	}
+
 	public function testFormMethodOverride() {
 		$form = $this->getStubForm();
 		$form->setFormMethod('GET');

tests/security/MemberTest.php

@@ -624,6 +624,35 @@ class MemberTest extends FunctionalTest {
 		return $extensions;
 	}
 
+	public function testGenerateAutologinTokenAndStoreHash() {
+		$enc = new PasswordEncryptor_Blowfish();
+
+		$m = new Member();
+		$m->PasswordEncryption = 'blowfish';
+		$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_Blowfish();
+
+		$m1 = new Member();
+		$m1->PasswordEncryption = 'blowfish';
+		$m1->Salt = $enc->salt('123');
+		$m1Token = $m1->generateAutologinTokenAndStoreHash();
+
+		$m2 = new Member();
+		$m2->PasswordEncryption = 'blowfish';
+		$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.');
+	}
+
 }
 class MemberTest_ViewingAllowedExtension extends DataExtension implements TestOnly {
 

tests/security/RandomGeneratorTest.php

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

tests/security/SecurityTest.php

@@ -222,7 +222,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'));