Merged revisions 52121 via svnmerge from

http://svn.silverstripe.com/open/modules/sapphire/branches/govtsecurity

........
  r52121 | sminnee | 2008-04-03 22:04:33 +1300 (Thu, 03 Apr 2008) | 4 lines
  
  Added DataObject::validate() for specifying DataObject-level validators.
  Added DataObject::onAfterWrite(), a complement of DataObject::onBeforeWrite()
  Added password strength testing to security system
  Added password expiry to security system
........


git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@53465 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/ValidationResult.php

@@ -0,0 +1,99 @@
+<?php
+
+/**
+ * A class that combined as a boolean result with an optional list of error messages.
+ * This is used for returning validation results from validators
+ */
+class ValidationResult extends Object {
+	/**
+	 * Boolean - is the result valid or not
+	 */
+	protected $isValid;
+
+	
+	/**
+	 * Array of errors
+	 */
+	protected $errorList = array();
+
+	/**
+	 * Create a new ValidationResult.
+	 * By default, it is a successful result.	Call $this->error() to record errors.
+	 */
+	function __construct($valid = true, $message = null) {
+		$this->isValid = true;
+		if($message) $this->errorList[] = $message;
+	}
+	
+	/**
+	 * Record an error against this validation result,
+	 * @param $message The validation error message
+	 * @param $code An optional error code string, that can be accessed with {@link $this->codeList()}.
+	 */
+	function error($message, $code = null) {
+		$message = trim($message);
+		if(substr($message,-1) == '.') $message = substr($message,0,-1);
+		
+		$this->isValid = false;
+		
+		if($code) {
+			if(!is_numeric($code)) {
+				$this->errorList[$code] = $message;
+			} else {
+				user_error("ValidationResult::error() - Don't use a numeric code '$code'.  Use a string.  I'm going to ignore it.", E_USER_WARNING);
+				$this->errorList[$code] = $message;
+			}
+		} else {
+			$this->errorList[] = $message;
+		}
+	}
+	
+	/**
+	 * Returns true if the result is valid.
+	 */
+	function valid() {
+		return $this->isValid;
+	}
+	
+	/**
+	 * Get an array of errors
+	 */
+	function messageList() {
+		return $this->errorList;
+	}
+
+	/**
+	 * Get an array of error codes
+	 */
+	function codeList() {
+		$codeList = array();
+		foreach($this->errorList as $k => $v) if(!is_numeric($k)) $codeList[] = $k;
+		return $codeList;
+	}
+	
+	/**
+	 * Get the error message as a string.
+	 */
+	function message() {
+		return implode("; ", $this->errorList);
+	}
+	
+	/**
+	 * Get a starred list of all messages
+	 */
+	function starredList() {
+		return " * " . implode("\n * ", $this->errorList);
+	}
+	
+	/**
+	 * Combine this Validation Result with the ValidationResult given in other.
+	 * It will be valid if both this and the other result are valid.
+	 * This object will be modified to contain the new validation information.
+	 */
+	function combineAnd(ValidationResult $other) {
+		$this->isValid = $this->isValid && $other->valid();
+		$this->errorList = array_merge($this->errorList, $other->messageList());
+	}
+	
+	
+}

core/model/DataObject.php

@@ -432,11 +432,31 @@ class DataObject extends ViewableData implements DataObjectInterface {
 		foreach($this->record as $fieldName => $fieldVal)
 			$this->changed[$fieldName] = 1;
 	}
+	
+	/**
+	 * Validate the current object.
+	 *
+	 * By default, there is no validation - objects are always valid!  However, you can overload this method in your
+	 * DataObject sub-classes to specify custom validation.
+	 * 
+	 * Invalid objects won't be able to be written - a warning will be thrown and no write will occur.  onBeforeWrite()
+	 * and onAfterWrite() won't get called either.
+	 * 
+	 * It is expected that you call validate() in your own application to test that an object is valid before attempting
+	 * a write, and respond appropriately if it isnt'.
+	 * 
+	 * @return A {@link ValidationResult} object
+	 */
+	protected function validate() {
+		return new ValidationResult();
+	}
 
 	/**
 	 * Event handler called before writing to the database.
 	 * You can overload this to clean up or otherwise process data before writing it to the
 	 * database.  Don't forget to call parent::onBeforeWrite(), though!
+	 *
+	 * This called after {@link $this->validate()}, so you can be sure that your data is valid.
 	 */
 	protected function onBeforeWrite() {
 		$this->brokenOnWrite = false;
@@ -445,6 +465,17 @@ class DataObject extends ViewableData implements DataObjectInterface {
 		$this->extend('augmentBeforeWrite', $dummy);
 	}
 
+	/**
+	 * Event handler called after writing to the database.
+	 * You can overload this to act upon changes made to the data after it is written.
+	 * $this->changed will have a record
+	 * database.  Don't forget to call parent::onAfterWrite(), though!
+	 */
+	protected function onAfterWrite() {
+		$dummy = null;
+		$this->extend('augmentAfterWrite', $dummy);
+	}
+
 	/**
 	 * Used by onBeforeWrite() to ensure child classes call parent::onBeforeWrite()
 	 * @var boolean
@@ -516,6 +547,13 @@ class DataObject extends ViewableData implements DataObjectInterface {
 		$firstWrite = false;
 		$this->brokenOnWrite = true;
 		$isNewRecord = false;
+		
+		$valid = $this->validate();
+		if(!$valid->valid()) {
+			user_error("Validation error writing a $this->class object: " . $valid->message() . ".  Object not written.", E_USER_WARNING);
+			return false;
+		}
+		
 		$this->onBeforeWrite();
 		if($this->brokenOnWrite) {
 			user_error("$this->class has a broken onBeforeWrite() function.  Make sure that you call parent::onBeforeWrite().", E_USER_ERROR);
@@ -612,6 +650,8 @@ class DataObject extends ViewableData implements DataObjectInterface {
 				} else {
 					DataObjectLog::changedObject($this);
 				}
+				
+				$this->onAfterWrite();
 
 				$this->changed = null;
 			} elseif ( $showDebug ) {

security/ChangePasswordForm.php

@@ -37,7 +37,7 @@ class ChangePasswordForm extends Form {
 		}
 		if(!$actions) {
 			$actions = new FieldSet(
-				new FormAction("changePassword", _t('Member.BUTTONCHANGEPASSWORD', "Change Password"))
+				new FormAction("doChangePassword", _t('Member.BUTTONCHANGEPASSWORD', "Change Password"))
 			);
 		}
 
@@ -50,7 +50,7 @@ class ChangePasswordForm extends Form {
 	 *
 	 * @param array $data The user submitted data
 	 */
-	function changePassword(array $data) {
+	function doChangePassword(array $data) {
 		if($member = Member::currentUser()) {
 			// The user was logged in, check the current password
 			if($member->checkPassword($data['OldPassword']) == false) {
@@ -60,6 +60,7 @@ class ChangePasswordForm extends Form {
 					"bad"
 				);
 				Director::redirectBack();
+				return;
 			}
 		}
 
@@ -72,24 +73,26 @@ class ChangePasswordForm extends Form {
 			if(!$member) {
 				Session::clear('AutoLoginHash');
 				Director::redirect('loginpage');
+				return;
 			}
 		}
 
 		// Check the new password
 		if($data['NewPassword1'] == $data['NewPassword2']) {
-			$member->Password = $data['NewPassword1'];
-			$member->AutoLoginHash = null;
-			$member->write();
+			$isValid = $member->changePassword($data['NewPassword1']);
+			if($isValid->valid()) {
+				$this->clearMessage();
+				$this->sessionMessage(
+					_t('Member.PASSWORDCHANGED', "Your password has been changed, and a copy emailed to you."),
+					"good");
+				Session::clear('AutoLoginHash');
+				Director::redirect(Security::Link('login'));
 
-			$member->sendinfo('changePassword',
-												array('CleartextPassword' => $data['NewPassword1']));
-
-			$this->clearMessage();
-			$this->sessionMessage(
-				_t('Member.PASSWORDCHANGED', "Your password has been changed, and a copy emailed to you."),
-				"good");
-			Session::clear('AutoLoginHash');
-			Director::redirect(Security::Link('login'));
+			} else {
+				$this->clearMessage();
+				$this->sessionMessage(nl2br("We couldn't accept that password:\n" . $isValid->starredList()), "bad");
+				Director::redirectBack();
+			}
 
 		} else {
 			$this->clearMessage();

security/Member.php

@@ -27,7 +27,8 @@ class Member extends DataObject {
 		'BlacklistedEmail' => 'Boolean',
 		'PasswordEncryption' => "Enum('none', 'none')",
 		'Salt' => 'Varchar(50)',
-		'Locale' => 'Varchar(6)', 
+		'PasswordExpiry' => 'Date',
+		'Locale' => 'Varchar(6)',
 	);
 
 	static $belongs_many_many = array(
@@ -71,6 +72,19 @@ class Member extends DataObject {
 		'Email' => true,
 	);
 	
+	
+	/**
+	 * {@link PasswordValidator} object for validating user's password
+	 */
+	protected static $password_validator = null;
+	
+	
+	/**
+	 * The number of days that a password should be valid for.
+	 * By default, this is null, which means that passwords never expire
+	 */
+	protected static $password_expiry_days = null;
+	
 	/**
 	 * This method is used to initialize the static database members
 	 *
@@ -109,7 +123,27 @@ class Member extends DataObject {
 	 * quirky problems (such as using the Windmill 0.3.6 proxy).
 	 */
 	static function session_regenerate_id() {
-		session_regenerate_id(true);
+		if(!headers_sent()) session_regenerate_id(true);
+	}
+	
+	/**
+	 * Set a {@link PasswordValidator} object to use to validate member's passwords.
+	 */
+	static function set_password_validator($pv) {
+		self::$password_validator = $pv;
+	}
+
+	/**
+	 * Set the number of days that a password should be valid for.
+	 * Set to null (the default) to have passwords never expire.
+	 */
+	static function set_password_expiry($days) {
+		self::$password_expiry_days = $days;
+	}
+	
+	function isPasswordExpired() {
+		if(!$this->PasswordExpiry) return false;
+		return strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry);
 	}
 
 	/**
@@ -419,6 +453,7 @@ class Member extends DataObject {
 	 * If an email's filled out look for a record with the same email and if
 	 * found update this record to merge with that member.
 	 */
+
 	function onBeforeWrite() {
 		if($this->SetPassword) $this->Password = $this->SetPassword;
 
@@ -452,7 +487,8 @@ class Member extends DataObject {
 			isset($this->changed['Password']) && $this->changed['Password'] && $this->record['Password'] && 
 			Member::$notify_password_change) $this->sendInfo('changePassword');
 		
-		if(isset($this->changed['Password']) && $this->changed['Password']) {
+		// The test on $this->ID is used for when records are initially created
+		if(!$this->ID || (isset($this->changed['Password']) && $this->changed['Password'])) {
 			// Password was changed: encrypt the password according the settings
 			$encryption_details = Security::encrypt_password($this->Password);
 			$this->Password = $encryption_details['password'];
@@ -461,10 +497,28 @@ class Member extends DataObject {
 
 			$this->changed['Salt'] = true;
 			$this->changed['PasswordEncryption'] = true;
+			
+			// If we haven't manually set a password expiry
+			if(!isset($this->changed['PasswordExpiry']) || !$this->changed['PasswordExpiry']) {
+				// then set it for us
+				if(self::$password_expiry_days) {
+					$this->PasswordExpiry = date('Y-m-d', time() + 86400 * self::$password_expiry_days);
+				} else {
+					$this->PasswordExpiry = null;
+				}
+			}
 		}
 
 		parent::onBeforeWrite();
 	}
+	
+	function onAfterWrite() {
+		parent::onAfterWrite();
+
+		if(isset($this->changed['Password']) && $this->changed['Password']) {
+			MemberPassword::log($this);
+		}
+	}
 
 
 	/**
@@ -833,6 +887,36 @@ class Member extends DataObject {
 		
 		return Permission::check( 'ADMIN' );
 	}
+
+
+	/**
+	 * Validate this member object.
+	 */
+	function validate() {
+		$valid = parent::validate();
+		
+		if(!$this->ID || (isset($this->changed['Password']) && $this->changed['Password'])) {
+			if(self::$password_validator) {
+				$valid->combineAnd(self::$password_validator->validate($this->Password, $this));
+			}
+		}
+
+		return $valid;
+	}	
+	
+	function changePassword($password) {
+		$this->Password = $password;
+		$valid = $this->validate();
+		
+		if($valid->valid()) {
+			$this->AutoLoginHash = null;
+			$this->write();
+
+			$this->sendinfo('changePassword', array('CleartextPassword' => $password));
+		}
+		
+		return $valid;
+	}
 }
 
 
@@ -1347,7 +1431,6 @@ class Member_Validator extends RequiredFields {
 
 		return $js;
 	}
-
 }
 
 // Initialize the static DB variables to add the supported encryption

security/MemberLoginForm.php

@@ -99,8 +99,19 @@ class MemberLoginForm extends LoginForm {
 		if($this->performLogin($data)) {
 			Session::clear('SessionForms.MemberLoginForm.Email');
 			Session::clear('SessionForms.MemberLoginForm.Remember');
+			
+			if(Member::currentUser()->isPasswordExpired()) {
+				if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) {
+					Session::set('BackURL', $backURL);
+				}
 
-			if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) {
+				$cp = new ChangePasswordForm(null, 'ChangePasswordForm');
+				$cp->sessionMessage('Your password has expired.  Please choose a new one.', 'good');
+				
+				Director::redirect('Security/changepassword');
+				
+				
+			} else if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) {
 				Session::clear("BackURL");
 				Director::redirect($backURL);
 			} else {

security/MemberPassword.php

@@ -0,0 +1,39 @@
+<?php
+
+/**
+ * Keep track of users' previous passwords, so that we can check that new passwords aren't changed back to old ones.
+ */
+class MemberPassword extends DataObject {
+	static $db = array(
+		'Password' => 'Varchar',
+		'Salt' => 'Varchar',
+		'PasswordEncryption' => 'Varchar',
+	);
+	
+	static $has_one = array(
+		'Member' => 'Member',
+	);
+	
+	/**
+	 * Log a password change from the given member.
+	 * Call MemberPassword::log($this) from within Member whenever the password is changed.
+	 */
+	static function log($member) {
+		$record = new MemberPassword();
+		$record->MemberID = $member->ID;
+		$record->Password = $member->Password;
+		$record->PasswordEncryption = $member->PasswordEncryption;
+		$record->Salt = $member->Salt;
+		$record->write();
+	}
+	
+	/**
+	 * Check if the given password is the same as the one stored in this record
+	 */	
+	function checkPassword($password) {
+		$encryption_details = Security::encrypt_password($password, $this->Salt, $this->PasswordEncryption);
+		return ($this->Password === $encryption_details['password']);
+	}
+	
+	
+}

security/NZGovtPasswordValidator.php

@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * This {@link PasswordValidator} implements the NZ E-Government Guidelines for passwords
+ */
+class NZGovtPasswordValidator extends PasswordValidator {
+	function __construct() {
+		parent::__construct();
+		$this->minLength(7);
+		$this->checkHistoricalPasswords(6);
+		$this->characterStrength(3, array('lowercase','uppercase','digits','punctuation'));
+	}
+	
+}

security/PasswordValidator.php

@@ -0,0 +1,85 @@
+<?php
+
+/**
+ * This class represents a validator for member passwords.
+ * 
+ * <code>
+ * $pwdVal = new PasswordValidator();
+ * $pwdValidator->minLength(7);
+ * $pwdValidator->checkHistoricalPasswords(6);
+ * $pwdValidator->characterStrength('lowercase','uppercase','digits','punctuation');
+ * 
+ * Member::set_password_validator($pwdValidator);
+ * </code>
+ */
+class PasswordValidator extends Object {
+	static $character_strength_tests = array(
+		'lowercase' => '/[a-z]/',
+		'uppercase' => '/[A-Z]/',
+		'digits' => '/[0-9]/',
+		'punctuation' => '/[^A-Za-z0-9]/',
+	);
+	
+	protected $minLength, $minScore, $testNames, $historicalPasswordCount;
+
+	/**
+	 * Minimum password length
+	 */
+	function minLength($minLength) {
+		$this->minLength = $minLength;
+	}
+	
+	/**
+	 * Check the character strength of the password.
+	 *
+	 * Eg: $this->characterStrength(3, array("lowercase", "uppercase", "digits", "punctuation"))
+	 * 
+	 * @param $minScore The minimum number of character tests that must pass
+	 * @param $testNames The names of the tests to perform
+	 */
+	function characterStrength($minScore, $testNames) {
+		$this->minScore = $minScore;
+		$this->testNames = $testNames;
+	}
+	
+	/**
+	 * Check a number of previous passwords that the user has used, and don't let them change to that.
+	 */
+	function checkHistoricalPasswords($count) {
+		$this->historicalPasswordCount = $count;
+	}
+	
+	function validate($password, $member) {
+		$valid = new ValidationResult();
+		
+		if($this->minLength) {
+			if(strlen($password) < $this->minLength) $valid->error("Password is too short, it must be 7 or more characters long.", "TOO_SHORT");
+		}
+
+		if($this->minScore) {
+			$score = 0;
+			$missedTests = array();
+			foreach($this->testNames as $name) {
+				if(preg_match(self::$character_strength_tests[$name], $password)) $score++;
+				else $missedTests[] = $name;
+			}
+			
+			if($score < $this->minScore) {
+				$valid->error("You need to increase the strength of your passwords by adding some of the following characters: " . implode(", ", $missedTests), "LOW_CHARACTER_STRENGTH");
+			}
+		}
+		
+		if($this->historicalPasswordCount) {
+			$previousPasswords = DataObject::get("MemberPassword", "MemberID = $member->ID", "Created DESC, ID Desc", "", $this->historicalPasswordCount);
+			if($previousPasswords) foreach($previousPasswords as $previousPasswords) {
+				if($previousPasswords->checkPassword($password)) {
+					$valid->error("You've already used that password in the past, please choose a new password", "PREVIOUS_PASSWORD");
+					break;
+				}
+			}
+		}
+		
+		return $valid;
+	}
+	
+}

security/Security.php

@@ -172,8 +172,7 @@ class Security extends Controller {
 	 * Get the login form to process according to the submitted data
 	 */
 	protected function LoginForm() {
-		if(is_array($_REQUEST) && isset($_REQUEST['AuthenticationMethod']))
-		{
+		if(isset($this->requestParams['AuthenticationMethod'])) {
 			$authenticator = trim($_REQUEST['AuthenticationMethod']);
 
 			$authenticators = Authenticator::get_authenticators();

tests/security/MemberTest.php

@@ -0,0 +1,161 @@
+<?php
+
+class MemberTest extends SapphireTest {
+	static $fixture_file = 'sapphire/tests/security/MemberTest.yml';
+	
+	/**
+	 * Test that password changes are logged properly
+	 */
+	function testPasswordChangeLogging() {
+		Member::set_password_validator(null);
+
+		$member = $this->objFromFixture('Member', 'test');
+		$member->Password = "test1";
+		$member->write();
+
+		$member->Password = "test2";
+		$member->write();
+
+		$member->Password = "test3";
+		$member->write();
+		
+		$passwords = DataObject::get("MemberPassword", "MemberID = $member->ID", "Created DESC, ID DESC")->getIterator();
+		$this->assertNotNull($passwords);
+		$record = $passwords->rewind();
+		$this->assertTrue($record->checkPassword('test3'), "Password test3 not found in MemberRecord");
+
+		$record = $passwords->next();
+		$this->assertTrue($record->checkPassword('test2'), "Password test2 not found in MemberRecord");
+
+		$record = $passwords->next();
+		$this->assertTrue($record->checkPassword('test1'), "Password test1 not found in MemberRecord");
+
+		$record = $passwords->next();
+		$this->assertType('DataObject', $record);
+		$this->assertTrue($record->checkPassword('1nitialPassword'), "Password 1nitialPassword not found in MemberRecord");
+	}
+	
+	/**
+	 * Test that passwords validate against NZ e-government guidelines
+	 *  - don't allow the use of the last 6 passwords
+	 *  - require at least 3 of lowercase, uppercase, digits and punctuation
+	 *  - at least 7 characters long
+	 */
+	function testValidatePassword() {
+		$member = $this->objFromFixture('Member', 'test');
+		
+		Member::set_password_validator(new NZGovtPasswordValidator());
+
+		// BAD PASSWORDS
+		
+		$valid = $member->changePassword('shorty');
+		$this->assertFalse($valid->valid());
+		$this->assertContains("TOO_SHORT", $valid->codeList());
+
+		$valid = $member->changePassword('longone');
+		$this->assertNotContains("TOO_SHORT", $valid->codeList());
+		$this->assertContains("LOW_CHARACTER_STRENGTH", $valid->codeList());
+		$this->assertFalse($valid->valid());
+
+		$valid = $member->changePassword('w1thNumb3rs');
+		$this->assertNotContains("LOW_CHARACTER_STRENGTH", $valid->codeList());
+		$this->assertTrue($valid->valid());
+		
+		// Clear out the MemberPassword table to ensure that the system functions properly in that situation
+		DB::query("DELETE FROM MemberPassword");
+
+		// GOOD PASSWORDS
+		
+		$valid = $member->changePassword('withSym###Ls');
+		$this->assertNotContains("LOW_CHARACTER_STRENGTH", $valid->codeList());
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls2');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls3');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls4');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls5');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls6');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls7');
+		$this->assertTrue($valid->valid());
+
+		// CAN'T USE PASSWORDS 2-7, but I can use pasword 1
+
+		$valid = $member->changePassword('withSym###Ls2');
+		$this->assertFalse($valid->valid());
+		$this->assertContains("PREVIOUS_PASSWORD", $valid->codeList());
+
+		$valid = $member->changePassword('withSym###Ls5');
+		$this->assertFalse($valid->valid());
+		$this->assertContains("PREVIOUS_PASSWORD", $valid->codeList());
+
+		$valid = $member->changePassword('withSym###Ls7');
+		$this->assertFalse($valid->valid());
+		$this->assertContains("PREVIOUS_PASSWORD", $valid->codeList());
+		
+		$valid = $member->changePassword('withSym###Ls');
+		$this->assertTrue($valid->valid());
+		
+		// HAVING DONE THAT, PASSWORD 2 is now available from the list
+
+		$valid = $member->changePassword('withSym###Ls2');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls3');
+		$this->assertTrue($valid->valid());
+
+		$valid = $member->changePassword('withSym###Ls4');
+		$this->assertTrue($valid->valid());
+	}
+
+	/**
+	 * Test that the PasswordExpiry date is set when passwords are changed
+	 */
+	function testPasswordExpirySetting() {
+		Member::set_password_expiry(90);
+		
+		$member = $this->objFromFixture('Member', 'test');
+		$valid = $member->changePassword("Xx?1234234");
+		$this->assertTrue($valid->valid());
+		
+		$expiryDate = date('Y-m-d', time() + 90*86400);		
+		$this->assertEquals($expiryDate, $member->PasswordExpiry);
+
+		Member::set_password_expiry(null);
+		$valid = $member->changePassword("Xx?1234235");
+		$this->assertTrue($valid->valid());
+
+		$this->assertNull($member->PasswordExpiry);
+	}
+	
+	function testIsPasswordExpired() {
+		$member = $this->objFromFixture('Member', 'test');
+		$this->assertFalse($member->isPasswordExpired());
+
+		$member = $this->objFromFixture('Member', 'noexpiry');
+		$member->PasswordExpiry = null;
+		$this->assertFalse($member->isPasswordExpired());
+
+		$member = $this->objFromFixture('Member', 'expiredpassword');
+		$this->assertTrue($member->isPasswordExpired());
+		
+		// Check the boundary conditions
+		// If PasswordExpiry == today, then it's expired
+		$member->PasswordExpiry = date('Y-m-d');
+		$this->assertTrue($member->isPasswordExpired());
+
+		// If PasswordExpiry == tomorrow, then it's not
+		$member->PasswordExpiry = date('Y-m-d', time() + 86400);
+		$this->assertFalse($member->isPasswordExpired());
+		
+	}
+}

tests/security/MemberTest.yml

@@ -0,0 +1,18 @@
+Member:
+    test:
+        FirstName: Test
+        Surname: User
+        Email: sam@silverstripe.com
+        Password: 1nitialPassword
+        PasswordExpiry: 2030-01-01
+    expiredpassword:
+        FirstName: Test
+        Surname: User
+        Email: expired@silverstripe.com
+        Password: 1nitialPassword
+        PasswordExpiry: 2006-01-01
+    noexpiry:
+        FirstName: Test
+        Surname: User
+        Email: noexpiry@silverstripe.com
+        Password: 1nitialPassword

tests/security/SecurityTest.php

@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * Test the security class, including log-in form, change password form, etc
+ */
+class SecurityTest extends SapphireTest {
+	static $fixture_file = 'sapphire/tests/security/MemberTest.yml';
+	
+	
+	/**
+	 * Test that the login form redirects to the change password form after logging in with an expired password
+	 */
+	function testExpiredPassword() {
+		
+		// BAD PASSWORDS ARE LOCKED OUT
+		
+		$session = new Session(array());
+		$badResponse = Director::test('Security/login?executeForm=LoginForm', array(
+			'Email' => 'sam@silverstripe.com', 
+			'Password' => 'badpassword', 
+			'AuthenticationMethod' => 'MemberAuthenticator',
+			'action_dologin' => 1,
+			'BackURL' => 'test/link'),
+			$session
+		);
+		$this->assertEquals(302, $badResponse->getStatusCode());
+		$this->assertRegExp('/Security\/login/', $badResponse->getHeader('Location'));
+		$this->assertNull($session->inst_get('loggedInAs'));
+
+		// UNEXPIRED PASSWORD GO THROUGH WITHOUT A HITCH
+
+		$session = new Session(array());
+		$goodResponse = Director::test('Security/login?executeForm=LoginForm', array(
+			'Email' => 'sam@silverstripe.com', 
+			'Password' => '1nitialPassword', 
+			'AuthenticationMethod' => 'MemberAuthenticator',
+			'action_dologin' => 1,
+			'BackURL' => 'test/link'),
+			$session
+		);
+		$this->assertEquals(302, $goodResponse->getStatusCode());
+		$this->assertEquals(Director::baseURL() . 'test/link', $goodResponse->getHeader('Location'));
+		$this->assertEquals($this->idFromFixture('Member', 'test'), $session->inst_get('loggedInAs'));
+		
+		// EXPIRED PASSWORDS ARE SENT TO THE CHANGE PASSWORD FORM
+		
+		$session = new Session(array());
+		$expiredResponse = Director::test('Security/login?executeForm=LoginForm', array(
+			'Email' => 'expired@silverstripe.com', 
+			'Password' => '1nitialPassword', 
+			'AuthenticationMethod' => 'MemberAuthenticator',
+			'action_dologin' => 1,
+			'BackURL' => 'test/link'),
+			$session
+		);
+		$this->assertEquals(302, $expiredResponse->getStatusCode());
+		$this->assertEquals(Director::baseURL() . 'Security/changepassword', $expiredResponse->getHeader('Location'));
+		$this->assertEquals($this->idFromFixture('Member', 'expiredpassword'), $session->inst_get('loggedInAs'));
+	}
+	
+}