From f54e9db8b914811d14e376a38469d5fb2f239a3a Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Sun, 16 Sep 2007 00:32:48 +0000 Subject: [PATCH] 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 | 94 +++++---- security/Member.php | 62 ++++-- security/MemberAuthenticator.php | 10 +- security/MemberLoginForm.php | 18 +- security/Security.php | 251 +++++++++++++++++++------ templates/email/ChangePasswordEmail.ss | 6 + templates/email/ForgotPasswordEmail.ss | 6 +- 7 files changed, 317 insertions(+), 130 deletions(-) create mode 100644 templates/email/ChangePasswordEmail.ss diff --git a/security/ChangePasswordForm.php b/security/ChangePasswordForm.php index 2a60054fb..464a00000 100755 --- a/security/ChangePasswordForm.php +++ b/security/ChangePasswordForm.php @@ -1,54 +1,84 @@ 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(); } } - + } + ?> \ No newline at end of file diff --git a/security/Member.php b/security/Member.php index 480c357c8..5a7d8c3d3 100644 --- a/security/Member.php +++ b/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'; } diff --git a/security/MemberAuthenticator.php b/security/MemberAuthenticator.php index 1aa347625..512df183e 100644 --- a/security/MemberAuthenticator.php +++ b/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; } diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index c0851881f..4c736e3a5 100644 --- a/security/MemberLoginForm.php +++ b/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"); - } } diff --git a/security/Security.php b/security/Security.php index 95b34e9ee..b26174f7a 100644 --- a/security/Security.php +++ b/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" => - "

Enter your e-mail address and we will send you a password

", - "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" => - "

Thank you, your password has been sent to '$email'.

", + 'Content' => + '

Enter your e-mail address and we will send you a link with ' . + 'which you can reset your password

', + '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' => + "

Thank you! The password reset link has been sent to '$email'.

", + )); + + //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' => + '

Please enter a new password.

', + 'Form' => $this->ChangePasswordForm(), + )); + + } elseif(Member::currentUser()) { + // let a logged in user change his password + $customisedController = $controller->customise(array( + 'Content' => '

You can change your password below.

', + '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' => + "

The password reset link is invalid or expired.

\n" . + '

You can request a new one here or change your password after you logged in.

')); + } 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' => + '

Please enter a new password.

', + 'Form' => $this->ChangePasswordForm(), + )); + + } elseif(Member::currentUser()) { + // let a logged in user change his password + $customisedController = $controller->customise(array( + 'Content' => '

You can change your password below.

', + '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' => + "

The password reset link is invalid or expired.

\n" . + '

You can request a new one here or change your password after you logged in.

')); + } 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, diff --git a/templates/email/ChangePasswordEmail.ss b/templates/email/ChangePasswordEmail.ss new file mode 100644 index 000000000..f28f48768 --- /dev/null +++ b/templates/email/ChangePasswordEmail.ss @@ -0,0 +1,6 @@ +

Hi $FirstName,

+ +

You changed your password for $BaseHref.
+ You can now use the following credentials to log in:

+

E-mail: $Email
+ Password: $CleartextPassword

diff --git a/templates/email/ForgotPasswordEmail.ss b/templates/email/ForgotPasswordEmail.ss index f8d577121..cd08ecde9 100755 --- a/templates/email/ForgotPasswordEmail.ss +++ b/templates/email/ForgotPasswordEmail.ss @@ -1,8 +1,4 @@

Hi $FirstName,

-

Here's your password for $BaseHref.

+

Here's is your password reset link for $BaseHref

-

-Email: $Email
-Password: $Password -

\ No newline at end of file