mlanthaler: Newly implemented "I've lost my password" feature that works also with encrypted passwords (ticket #48).

There are some (cosmetically) things that should be fixed, but everything work as it should. 
Will fix those things after my vacation. 
(merged from branches/gsoc)


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

security/ChangePasswordForm.php

@@ -1,54 +1,84 @@
 <?php
 
- /**
-	* Standard Change Password Form
-	*/
-
-class ChangePasswordForm extends Form {	
+/**
+ * Standard Change Password Form
+ */
+class ChangePasswordForm extends Form {
 
 	function __construct($controller, $name, $fields = null, $actions = null) {
-		
 		if(!$fields) {
-			$fields = new FieldSet(
-				new EncryptField("OldPassword","Your old password"),
-				new EncryptField("NewPassword1", "New Password"),
-				new EncryptField("NewPassword2", "Confirm New Password")
-			);
+			$fields = new FieldSet();
+			if(Member::currentUser()) {
+				$fields->push(new EncryptField("OldPassword","Your old password"));
+			}
+
+			$fields->push(new EncryptField("NewPassword1", "New Password"));
+			$fields->push(new EncryptField("NewPassword2", "Confirm New Password"));
 		}
 		if(!$actions) {
 			$actions = new FieldSet(
 				new FormAction("changePassword", "Change Password")
 			);
 		}
-		
+
 		parent::__construct($controller, $name, $fields, $actions);
 	}
-	
-	function changePassword($data){
-		if($member = Member::currentUser()){
-			if($data['OldPassword'] != $member->Password){
+
+	/**
+	 * Change the password
+	 *
+	 * @param array $data The user submitted data
+	 */
+	function changePassword(array $data) {
+		if($member = Member::currentUser()) {
+			// The user was logged in, check the current password
+			if($member->checkPassword($data['OldPassword']) == false) {
 				$this->clearMessage();
-				$this->sessionMessage("Your current password does not match, please try again", "bad");
-				Director::redirectBack();
-			}else	if($data[NewPassword1] == $data[NewPassword2]){
-				$member->Password = $data[NewPassword1] ;
-				$member->sendinfo('changePassword');
-				$member->write();
-				$this->clearMessage();
-				$this->sessionMessage("Your password has been changed, and a copy emailed to you.", "good");
-				Director::redirectBack();
-			}
-			else{
-				$this->clearMessage();
-				$this->sessionMessage("Your have entered your new password differently, try again", "bad");
+				$this->sessionMessage(
+					"Your current password does not match, please try again", "bad");
 				Director::redirectBack();
 			}
 		}
-		else		{
-			Director::redirect('loginpage');
+
+		if(!$member) {
+			if(Session::get('AutoLoginHash')) {
+				$member = Member::autoLoginHash(Session::get('AutoLoginHash'));
+			}
+
+			// The user is not logged in and no valid auto login hash is available
+			if(!$member) {
+				Session::clear('AutoLoginHash');
+				Director::redirect('loginpage');
+			}
+		}
+
+
+		// Check the new password
+		if($data['NewPassword1'] == $data['NewPassword2']) {
+			$member->Password = $data['NewPassword1'];
+			$member->AutoLoginHash = null;
+			$member->write();
+
+			$member->sendinfo('changePassword',
+												array('CleartextPassword' => $data['NewPassword1']));
+
+			$this->clearMessage();
+			$this->sessionMessage(
+				"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(
+				"Your have entered your new password differently, try again",
+				"bad");
+			Director::redirectBack();
 		}
 	}
-	
+
 }
 
+
 ?>

security/Member.php

@@ -10,7 +10,7 @@ class Member extends DataObject {
 		'NumVisit' => "Int",
 		'LastVisited' => 'Datetime',
 		'Bounced' => 'Boolean',
-		'AutoLoginHash' => 'Varchar(10)',
+		'AutoLoginHash' => 'Varchar(30)',
 		'AutoLoginExpired' => 'Datetime',
 		'BlacklistedEmail' => 'Boolean',
 		'PasswordEncryption' => "Enum('none', 'none')",
@@ -30,6 +30,7 @@ class Member extends DataObject {
 
 	static $indexes = array(
 		'Email' => true,
+		'AutoLoginHash' => 'unique (AutoLoginHash)'
 	);
 
 
@@ -53,6 +54,21 @@ class Member extends DataObject {
 	}
 
 
+	/**
+	 * Check if the passed password matches the stored one
+	 *
+	 * @param string $password The clear text password to check
+	 * @return bool Returns TRUE if the passed password is valid, otherwise
+	 *              FALSE.
+	 */
+	public function checkPassword($password) {
+		$encryption_details = Security::encrypt_password($password, $this->Salt,
+			$this->PasswordEncryption);
+
+		return ($this->Password === $encryption_details['password']);
+	}
+
+
 	/**
 	 * Logs this member in
 	 *
@@ -131,30 +147,40 @@ class Member extends DataObject {
 	/**
 	 * Generate an auto login hash
 	 *
-	 * @todo This is relative insecure, check if we should fix it (Markus)
+	 * 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)
+	 *
+	 * @todo Make it possible to handle database errors such as a "duplicate
+	 *       key" error
 	 */
-	function generateAutologinHash() {
-		$linkHash = sprintf('%10d', time() );
+	function generateAutologinHash($lifetime = 2) {
 
-		while( DataObject::get_one( 'Member', "`AutoLoginHash`='$linkHash'" ) )
-			$linkHash = sprintf('%10d', abs( time() * rand( 1, 10 ) ) );
+		do {
+			$hash = substr(base_convert(md5(uniqid(mt_rand(), true)), 16, 36),
+										 0, 30);
+		} while(DataObject::get_one('Member', "`AutoLoginHash` = '$hash'"));
 
-		$this->AutoLoginHash = $linkHash;
-		$this->AutoLoginExpired = date('Y-m-d', time() + ( 60 * 60 * 24 * 14 ) );
+		$this->AutoLoginHash = $hash;
+		$this->AutoLoginExpired = date('Y-m-d', time() + (86400 * $lifetime));
 
 		$this->write();
 	}
 
 
 	/**
-	 * Log a member in with an auto login hash link
+	 * Return the member for the auto login hash
+	 *
+	 * @param bool $login Should the member be logged in?
 	 */
-	static function autoLoginHash($RAW_hash) {
+	static function autoLoginHash($RAW_hash, $login = false) {
 		$SQL_hash = Convert::raw2sql($RAW_hash);
 
-		$member = DataObject::get_one('Member',"`AutoLoginHash`='$SQL_hash' AND `AutoLoginExpired` > NOW()");
+		$member = DataObject::get_one('Member',"`AutoLoginHash`='" . $SQL_hash .
+																	"' AND `AutoLoginExpired` > NOW()");
 
-		if($member)
+		if($login && $member)
 			$member->logIn();
 
 		return $member;
@@ -166,8 +192,10 @@ class Member extends DataObject {
 	 *
 	 * @param string $type Information type to send ("signup",
 	 *                     "changePassword" or "forgotPassword")
+	 * @param array $data Additional data to pass to the email (can be used in
+	 *                    the template)
 	 */
-	function sendInfo($type = 'signup') {
+	function sendInfo($type = 'signup', $data = null) {
 		switch($type) {
 			case "signup":
 				$e = new Member_SignupEmail();
@@ -179,6 +207,12 @@ class Member extends DataObject {
 				$e = new Member_ForgotPasswordEmail();
 				break;
 		}
+
+		if(is_array($data)) {
+			foreach($data as $key => $value)
+				$e->$key = $value;
+		}
+
 		$e->populateTemplate($this);
 		$e->send();
 	}
@@ -1008,7 +1042,7 @@ class Member_ChangePasswordEmail extends Email_Template {
  */
 class Member_ForgotPasswordEmail extends Email_Template {
     protected $from = '';  // setting a blank from address uses the site's default administrator email
-    protected $subject = "Your password";
+    protected $subject = "Your password reset link";
     protected $ss_template = 'ForgotPasswordEmail';
     protected $to = '$Email';
 }

security/MemberAuthenticator.php

@@ -31,14 +31,8 @@ class MemberAuthenticator extends Authenticator {
     $member = DataObject::get_one("Member",
 			"Email = '$SQL_user' AND Password IS NOT NULL");
 
-		if($member) {
-			$encryption_details =
-				Security::encrypt_password($RAW_data['Password'], $member->Salt,
-																	 $member->PasswordEncryption);
-
-			// Check if the entered password is valid
-			if(($member->Password != $encryption_details['password']))
-			  $member = null;
+		if($member && ($member->checkPassword($RAW_data['Password']) == false)) {
+			$member = null;
 		}
 
 

security/MemberLoginForm.php

@@ -168,25 +168,25 @@ class MemberLoginForm extends LoginForm {
 	function forgotPassword($data) {
 		$SQL_data = Convert::raw2sql($data);
 
-		if($data['Email'] && $member = DataObject::get_one("Member",
-				"Member.Email = '$SQL_data[Email]'")) {
-			if(!$member->Password) {
-				$member->createNewPassword();
-				$member->write();
-			}
+		if(($data['Email']) && ($member = DataObject::get_one("Member",
+				"Member.Email = '$SQL_data[Email]'"))) {
+
+			$member->generateAutologinHash();
+
+			$member->sendInfo('forgotPassword', array('PasswordResetLink' =>
+				Security::getPasswordResetLink($member->AutoLoginHash)));
 
-			$member->sendInfo('forgotPassword');
 			Director::redirect('Security/passwordsent/' . urlencode($data['Email']));
 
 		} else if($data['Email']) {
 			$this->sessionMessage(
-				"Sorry, but I don't recognise the e-mail address. Maybe you need to sign up, or perhaps you used another e-mail address?",
+				"Sorry, but I don't recognise the e-mail address. Maybe you need " .
+					"to sign up, or perhaps you used another e-mail address?",
 				"bad");
 			Director::redirectBack();
 
 		} else {
 			Director::redirect("Security/lostpassword");
-
 		}
 	}
 

security/Security.php

@@ -119,7 +119,7 @@ class Security extends Controller {
   /**
 	 * Get the login form to process according to the submitted data
 	 */
-	function LoginForm() {
+	protected function LoginForm() {
 		if(is_array($_REQUEST) && isset($_REQUEST['AuthenticationMethod']))
 		{
 			$authenticator = trim($_REQUEST['AuthenticationMethod']);
@@ -142,7 +142,7 @@ class Security extends Controller {
 	 *
 	 * @todo Check how to activate/deactivate authentication methods
 	 */
-	function GetLoginForms()
+	protected function GetLoginForms()
 	{
 		$forms = array();
 
@@ -163,7 +163,7 @@ class Security extends Controller {
 	 * @param string $action Name of the action
 	 * @return string Returns the link to the given action
 	 */
-	function Link($action = null) {
+	public static function Link($action = null) {
 		return "Security/$action";
 	}
 
@@ -176,7 +176,7 @@ class Security extends Controller {
 	 *                           responsible for sending the user where-ever
 	 *                           they should go.
 	 */
-	function logout($redirect = true) {
+	public function logout($redirect = true) {
 		if($member = Member::currentUser())
 			$member->logOut();
 
@@ -190,7 +190,7 @@ class Security extends Controller {
 	 *
 	 * @return string Returns the "login" page as HTML code.
 	 */
-	function login() {
+	public function login() {
 		Requirements::javascript("jsparty/behaviour.js");
 		Requirements::javascript("jsparty/loader.js");
 		Requirements::javascript("jsparty/prototype.js");
@@ -245,53 +245,25 @@ class Security extends Controller {
 	/**
 	 * Show the "lost password" page
 	 *
-	 * @return string Returns the "lost password " page as HTML code.
+	 * @return string Returns the "lost password" page as HTML code.
 	 */
-	function lostpassword() {
-		Requirements::javascript("jsparty/prototype.js");
-		Requirements::javascript("jsparty/behaviour.js");
-		Requirements::javascript("jsparty/loader.js");
-		Requirements::javascript("jsparty/prototype_improvements.js");
-		Requirements::javascript("jsparty/scriptaculous/effects.js");
+	public function lostpassword() {
+		Requirements::javascript('jsparty/prototype.js');
+		Requirements::javascript('jsparty/behaviour.js');
+		Requirements::javascript('jsparty/loader.js');
+		Requirements::javascript('jsparty/prototype_improvements.js');
+		Requirements::javascript('jsparty/scriptaculous/effects.js');
 
 		$tmpPage = new Page();
-		$tmpPage->Title = "Lost Password";
-		$tmpPage->URLSegment = "Security";
+		$tmpPage->Title = 'Lost Password';
+		$tmpPage->URLSegment = 'Security';
 		$controller = new Page_Controller($tmpPage);
 
 		$customisedController = $controller->customise(array(
-			"Content" =>
-				"<p>Enter your e-mail address and we will send you a password</p>",
-			"Form" => $this->LostPasswordForm(),
-		));
-		
-		//Controller::$currentController = $controller;
-		return $customisedController->renderWith("Page");
-	}
-
-
-	/**
-	 * Show the "password sent" page
-	 *
-	 * @return string Returns the "password sent" page as HTML code.
-	 */
-	function passwordsent() {
-		Requirements::javascript("jsparty/behaviour.js");
-		Requirements::javascript("jsparty/loader.js");
-		Requirements::javascript("jsparty/prototype.js");
-		Requirements::javascript("jsparty/prototype_improvements.js");
-		Requirements::javascript("jsparty/scriptaculous/effects.js");
-
-		$tmpPage = new Page();
-		$tmpPage->Title = "Lost Password";
-		$tmpPage->URLSegment = "Security";
-		$controller = new Page_Controller($tmpPage);
-
-		$email = $this->urlParams['ID'];
-		$customisedController = $controller->customise(array(
-			"Title" => "Password sent to '$email'",
-			"Content" =>
-				"<p>Thank you, your password has been sent to '$email'.</p>",
+			'Content' =>
+				'<p>Enter your e-mail address and we will send you a link with ' .
+				'which you can reset your password</p>',
+			'Form' => $this->LostPasswordForm(),
 		));
 		
 		//Controller::$currentController = $controller;
@@ -304,13 +276,169 @@ class Security extends Controller {
 	 *
 	 * @return Form Returns the lost password form
 	 */
-	function LostPasswordForm() {
-		return new MemberLoginForm($this, "LostPasswordForm", new FieldSet(
-				new EmailField("Email", "Email address")
-			), new FieldSet(
-				new FormAction("forgotPassword", "Send me my password")
-			), false
-		);
+	public function LostPasswordForm() {
+		return new MemberLoginForm($this, 'LostPasswordForm',
+			new FieldSet(new EmailField('Email', 'E-mail address')),
+			new FieldSet(new FormAction('forgotPassword',
+																	'Send me the password reset link')),
+			false);
+	}
+
+
+	/**
+	 * Show the "password sent" page
+	 *
+	 * @return string Returns the "password sent" page as HTML code.
+	 */
+	public function passwordsent() {
+		Requirements::javascript('jsparty/behaviour.js');
+		Requirements::javascript('jsparty/loader.js');
+		Requirements::javascript('jsparty/prototype.js');
+		Requirements::javascript('jsparty/prototype_improvements.js');
+		Requirements::javascript('jsparty/scriptaculous/effects.js');
+
+		$tmpPage = new Page();
+		$tmpPage->Title = 'Lost Password';
+		$tmpPage->URLSegment = 'Security';
+		$controller = new Page_Controller($tmpPage);
+
+		$email = Convert::raw2xml($this->urlParams['ID']);
+		$customisedController = $controller->customise(array(
+			'Title' => "Password reset link sent to '$email'",
+			'Content' =>
+				"<p>Thank you! The password reset link has been sent to '$email'.</p>",
+		));
+		
+		//Controller::$currentController = $controller;
+		return $customisedController->renderWith("Page");
+	}
+
+
+	/**
+	 * Create a link to the password reset form
+	 *
+	 * @param string $autoLoginHash The auto login hash
+	 */
+	public static function getPasswordResetLink($autoLoginHash) {
+		$autoLoginHash = urldecode($autoLoginHash);
+		return self::Link('changepassword') . "?h=$autoLoginHash";
+	}
+
+
+	/**
+	 * Show the "change password" page
+	 *
+	 * @return string Returns the "change password" page as HTML code.
+	 */
+	public function changepassword() {
+		$tmpPage = new Page();
+		$tmpPage->Title = 'Change your password';
+		$tmpPage->URLSegment = 'Security';
+		$controller = new Page_Controller($tmpPage);
+
+		if(isset($_REQUEST['h']) && Member::autoLoginHash($_REQUEST['h'])) {
+			// The auto login hash is valid, store it for the change password form
+			Session::set('AutoLoginHash', $_REQUEST['h']);
+
+			$customisedController = $controller->customise(array(
+				'Content' =>
+					'<p>Please enter a new password.</p>',
+				'Form' => $this->ChangePasswordForm(),
+			));
+
+		} elseif(Member::currentUser()) {
+			// let a logged in user change his password
+			$customisedController = $controller->customise(array(
+				'Content' => '<p>You can change your password below.</p>',
+				'Form' => $this->ChangePasswordForm()));
+
+		} else {
+			// show an error message if the auto login hash is invalid and the
+			// user is not logged in
+			if(isset($_REQUEST['h'])) {
+				$customisedController = $controller->customise(array('Content' =>
+					"<p>The password reset link is invalid or expired.</p>\n" .
+						'<p>You can request a new one <a href="' .
+						$this->Link('lostpassword') .
+						'">here</a> or change your password after you <a href="' .
+						$this->link('login') . '">logged in</a>.</p>'));
+			} else {
+				self::permissionFailure($this, 'You must be logged in in order to change your password!');
+				die();
+			}
+		}
+
+		Controller::$currentController = $controller;
+		return $customisedController->renderWith('Page');
+	}
+
+
+	/**
+	 * Create a link to the password reset form
+	 *
+	 * @param string $autoLoginHash The auto login hash
+	 */
+	public static function getPasswordResetLink($autoLoginHash) {
+		$autoLoginHash = urldecode($autoLoginHash);
+		return self::Link('changepassword') . "?h=$autoLoginHash";
+	}
+
+
+	/**
+	 * Show the "change password" page
+	 *
+	 * @return string Returns the "change password" page as HTML code.
+	 */
+	public function changepassword() {
+		$tmpPage = new Page();
+		$tmpPage->Title = 'Change your password';
+		$tmpPage->URLSegment = 'Security';
+		$controller = new Page_Controller($tmpPage);
+
+		if(isset($_REQUEST['h']) && Member::autoLoginHash($_REQUEST['h'])) {
+			// The auto login hash is valid, store it for the change password form
+			Session::set('AutoLoginHash', $_REQUEST['h']);
+
+			$customisedController = $controller->customise(array(
+				'Content' =>
+					'<p>Please enter a new password.</p>',
+				'Form' => $this->ChangePasswordForm(),
+			));
+
+		} elseif(Member::currentUser()) {
+			// let a logged in user change his password
+			$customisedController = $controller->customise(array(
+				'Content' => '<p>You can change your password below.</p>',
+				'Form' => $this->ChangePasswordForm()));
+
+		} else {
+			// show an error message if the auto login hash is invalid and the
+			// user is not logged in
+			if(isset($_REQUEST['h'])) {
+				$customisedController = $controller->customise(array('Content' =>
+					"<p>The password reset link is invalid or expired.</p>\n" .
+						'<p>You can request a new one <a href="' .
+						$this->Link('lostpassword') .
+						'">here</a> or change your password after you <a href="' .
+						$this->link('login') . '">logged in</a>.</p>'));
+			} else {
+				self::permissionFailure($this, 'You must be logged in in order to change your password!');
+				die();
+			}
+		}
+
+		Controller::$currentController = $controller;
+		return $customisedController->renderWith('Page');
+	}
+
+
+	/**
+	 * Factory method for the lost password form
+	 *
+	 * @return Form Returns the lost password form
+	 */
+	public function ChangePasswordForm() {
+		return new ChangePasswordForm($this, 'ChangePasswordForm');
 	}
 
 
@@ -321,7 +449,7 @@ class Security extends Controller {
 	 * @return bool|Member Returns FALSE if authentication fails, otherwise
 	 *                     the member object
 	 */
-	static function authenticate($RAW_email, $RAW_password) {
+	public static function authenticate($RAW_email, $RAW_password) {
 		$SQL_email = Convert::raw2sql($RAW_email);
 		$SQL_password = Convert::raw2sql($RAW_password);
 
@@ -344,10 +472,9 @@ class Security extends Controller {
 	 * @return Member Returns a member object that has administrator
 	 *                privileges.
 	 */
-	static function findAnAdministrator($username = 'admin',
-																			$password = 'password') {
-		$permission = DataObject::get_one("Permission", "`Code` = 'ADMIN'");
-
+	static function findAnAdministrator($username = 'admin', $password = 'password') {
+		$permission = DataObject::get_one("Permission", "`Code` = 'ADMIN'", true, "ID");
+		
 		$adminGroup = null;
 		if($permission) $adminGroup = DataObject::get_one("Group", "`ID` = '{$permission->GroupID}'", true, "ID");
 		
@@ -387,7 +514,7 @@ class Security extends Controller {
 	 * @param $username String
 	 * @param $password String (Cleartext)
 	 */
-	static function setDefaultAdmin( $username, $password ) {
+	public static function setDefaultAdmin($username, $password) {
 		if( self::$username || self::$password )
 			return;
 
@@ -404,7 +531,7 @@ class Security extends Controller {
 	 * @param boolean $strictPathChecking To enable or disable strict patch
 	 *                                    checking.
 	 */
-	static function setStrictPathChecking($strictPathChecking) {
+	public static function setStrictPathChecking($strictPathChecking) {
 		self::$strictPathChecking = $strictPathChecking;
 	}
 
@@ -414,7 +541,7 @@ class Security extends Controller {
 	 *
 	 * @return boolean Status of strict path checking
 	 */
-	static function getStrictPathChecking() {
+	public static function getStrictPathChecking() {
 		return self::$strictPathChecking;
 	}
 
@@ -595,7 +722,7 @@ class Security extends Controller {
 	 *
 	 * To run this action, the user needs to have administrator rights!
 	 */
-	function encryptallpasswords() {
+	public function encryptallpasswords() {
 		// Only administrators can run this method
 		if(!Member::currentUser() || !Member::currentUser()->isAdmin()) {
 			Security::permissionFailure($this,

templates/email/ChangePasswordEmail.ss

@@ -0,0 +1,6 @@
+<p>Hi $FirstName,</p>
+
+<p>You changed your password for $BaseHref.<br />
+  You can now use the following credentials to log in:</p>
+<p>E-mail: $Email<br />
+  Password: $CleartextPassword</p>

templates/email/ForgotPasswordEmail.ss

@@ -1,8 +1,4 @@
 <p>Hi $FirstName,</p>
 
-<p>Here's your password for <a href="home/">$BaseHref</a>.</p>
+<p>Here's is your <a href="$PasswordResetLink">password reset link</a> for $BaseHref</p>
 
-<p>
-Email: $Email<br />
-Password: $Password
-</p>