diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index 0bc6e8369..a6e706099 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -59,7 +59,7 @@ class ChangePasswordHandler extends RequestHandler * Handle the change password request * @todo this could use some spring cleaning * - * @return HTTPResponse|DBHTMLText + * @return array|HTTPResponse */ public function changepassword() { @@ -91,7 +91,10 @@ class ChangePasswordHandler extends RequestHandler ); // Subsequent request after the "first load with hash" (see previous if clause). - return $this->buildResponse($message); + return [ + 'Content' => $message, + 'Form' => $this->changePasswordForm() + ]; } if (Security::getCurrentUser()) { @@ -104,29 +107,32 @@ class ChangePasswordHandler extends RequestHandler ) . '

' ); - return $this->buildResponse($message); + return [ + 'Content' => $message, + 'Form' => $this->changePasswordForm() + ]; } // Show a friendly message saying the login token has expired if ($token !== null && $member && !$member->validateAutoLoginToken($token)) { - $customisedController = Controller::curr()->customise( - array( - 'Content' => DBField::create_field( - 'HTMLFragment', - _t( - 'SilverStripe\\Security\\Security.NOTERESETLINKINVALID', - '

The password reset link is invalid or expired.

' - . '

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

', - [ - 'link1' => $this->Link('lostpassword'), - 'link2' => $this->Link('login') - ] - ) + $message = [ + 'Content' => DBField::create_field( + 'HTMLFragment', + _t( + 'SilverStripe\\Security\\Security.NOTERESETLINKINVALID', + '

The password reset link is invalid or expired.

' + . '

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

', + [ + 'link1' => $this->link('lostpassword'), + 'link2' => $this->link('login') + ] ) ) - ); + ]; - return $customisedController->renderWith('changepassword'); + return [ + 'Content' => $message, + ]; } // Someone attempted to go to changepassword without token or being logged in @@ -139,21 +145,6 @@ class ChangePasswordHandler extends RequestHandler ); } - /** - * @param DBField $message - * @return DBHTMLText - */ - protected function buildResponse($message) - { - $customisedController = Controller::curr()->customise( - [ - 'Content' => $message, - 'Form' => $this->changePasswordForm() - ] - ); - - return $customisedController->renderWith(Security::singleton()->getTemplatesFor('changepassword')); - } /** * @param Member $member @@ -204,9 +195,10 @@ class ChangePasswordHandler extends RequestHandler * Change the password * * @param array $data The user submitted data + * @param ChangePasswordForm $form * @return HTTPResponse */ - public function doChangePassword(array $data) + public function doChangePassword(array $data, $form) { $member = Security::getCurrentUser(); // The user was logged in, check the current password @@ -215,12 +207,12 @@ class ChangePasswordHandler extends RequestHandler !$member->checkPassword($data['OldPassword'])->isValid() ) ) { - $this->form->sessionMessage( + $form->sessionMessage( _t( 'SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH', - "Your current password does not match, please try again" + 'Your current password does not match, please try again' ), - "bad" + 'bad' ); // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. @@ -242,12 +234,12 @@ class ChangePasswordHandler extends RequestHandler // Check the new password if (empty($data['NewPassword1'])) { - $this->form->sessionMessage( + $form->sessionMessage( _t( 'SilverStripe\\Security\\Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again" ), - "bad" + 'bad' ); // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. @@ -256,12 +248,12 @@ class ChangePasswordHandler extends RequestHandler // Fail if passwords do not match if ($data['NewPassword1'] !== $data['NewPassword2']) { - $this->form->sessionMessage( + $form->sessionMessage( _t( 'SilverStripe\\Security\\Member.ERRORNEWPASSWORD', - "You have entered your new password differently, try again" + 'You have entered your new password differently, try again' ), - "bad" + 'bad' ); // redirect back to the form, instead of using redirectBack() which could send the user elsewhere. @@ -271,7 +263,7 @@ class ChangePasswordHandler extends RequestHandler // Check if the new password is accepted $validationResult = $member->changePassword($data['NewPassword1']); if (!$validationResult->isValid()) { - $this->form->setSessionValidationResult($validationResult); + $form->setSessionValidationResult($validationResult); return $this->redirectBackToForm(); } @@ -303,10 +295,15 @@ class ChangePasswordHandler extends RequestHandler return $this->redirect($url); } + /** + * Something went wrong, go back to the changepassword + * + * @return HTTPResponse + */ public function redirectBackToForm() { // Redirect back to form - $url = $this->addBackURLParam(CMSSecurity::singleton()->Link('changepassword')); + $url = $this->addBackURLParam(Security::singleton()->Link('changepassword')); return $this->redirect($url); } diff --git a/src/Security/MemberAuthenticator/LostPasswordForm.php b/src/Security/MemberAuthenticator/LostPasswordForm.php new file mode 100644 index 000000000..395f93b73 --- /dev/null +++ b/src/Security/MemberAuthenticator/LostPasswordForm.php @@ -0,0 +1,45 @@ +authenticatorClass, 'lostPasswordForm', - new FieldList( - new EmailField('Email', _t('SilverStripe\\Security\\Member.EMAIL', 'Email')) - ), - new FieldList( - new FormAction( - 'forgotPassword', - _t('SilverStripe\\Security\\Security.BUTTONSEND', 'Send me the password reset link') - ) - ), + null, + null, false ); } @@ -164,21 +157,6 @@ class LostPasswordHandler extends RequestHandler return $this->redirect($this->addBackURLParam($lostPasswordLink)); } - /** - * Log out form handler method - * - * This method is called when the user clicks on "logout" on the form - * created when the parameter $checkCurrentUser of the - * {@link __construct constructor} was set to TRUE and the user was - * currently logged in. - * - * @return HTTPResponse - */ - public function logout() - { - return Security::singleton()->logout(); - } - /** * Forgot password form handler method. * Called when the user clicks on "I've lost my password". @@ -189,13 +167,14 @@ class LostPasswordHandler extends RequestHandler * * @skipUpgrade * @param array $data Submitted data + * @param LostPasswordForm $form * @return HTTPResponse */ - public function forgotPassword($data) + public function forgotPassword($data, $form) { // Ensure password is given if (empty($data['Email'])) { - $this->form->sessionMessage( + $form->sessionMessage( _t( 'SilverStripe\\Security\\Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.' @@ -220,17 +199,7 @@ class LostPasswordHandler extends RequestHandler if ($member) { $token = $member->generateAutologinTokenAndStoreHash(); - Email::create() - ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') - ->setData($member) - ->setSubject(_t( - 'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', - "Your password reset link", - 'Email subject' - )) - ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) - ->setTo($member->Email) - ->send(); + $this->sendEmail($member, $token); } // Avoid information disclosure by displaying the same status, @@ -243,4 +212,26 @@ class LostPasswordHandler extends RequestHandler return $this->redirect($this->addBackURLParam($link)); } + + /** + * Send the email to the member that requested a reset link + * @param Member $member + * @param string $token + * @return bool + */ + protected function sendEmail($member, $token) + { + /** @var Email $email */ + $email = Email::create() + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') + ->setData($member) + ->setSubject(_t( + 'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', + "Your password reset link", + 'Email subject' + )) + ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) + ->setTo($member->Email); + return $email->send(); + } } diff --git a/src/Security/MemberAuthenticator/MemberLoginForm.php b/src/Security/MemberAuthenticator/MemberLoginForm.php index 8f9c941dc..eb325cf45 100644 --- a/src/Security/MemberAuthenticator/MemberLoginForm.php +++ b/src/Security/MemberAuthenticator/MemberLoginForm.php @@ -74,6 +74,7 @@ class MemberLoginForm extends BaseLoginForm $checkCurrentUser = true ) { + $this->controller = $controller; $this->authenticator_class = $authenticatorClass; $customCSS = project() . '/css/member_login.css'; @@ -81,20 +82,17 @@ class MemberLoginForm extends BaseLoginForm Requirements::css($customCSS); } - if ($controller->request->getVar('BackURL')) { - $backURL = $controller->request->getVar('BackURL'); - } else { - $backURL = Session::get('BackURL'); - } - if ($checkCurrentUser && Security::getCurrentUser()) { // @todo find a more elegant way to handle this $logoutAction = Security::logout_url(); $fields = FieldList::create( - HiddenField::create("AuthenticationMethod", null, $this->authenticator_class, $this) + HiddenField::create('AuthenticationMethod', null, $this->authenticator_class, $this) ); $actions = FieldList::create( - FormAction::create("logout", _t('SilverStripe\\Security\\Member.BUTTONLOGINOTHER', "Log in as someone else")) + FormAction::create('logout', _t( + 'SilverStripe\\Security\\Member.BUTTONLOGINOTHER', + 'Log in as someone else' + )) ); } else { if (!$fields) { @@ -105,10 +103,6 @@ class MemberLoginForm extends BaseLoginForm } } - if (isset($backURL)) { - $fields->push(HiddenField::create('BackURL', 'BackURL', $backURL)); - } - // Reduce attack surface by enforcing POST requests $this->setFormMethod('POST', true); @@ -127,6 +121,12 @@ class MemberLoginForm extends BaseLoginForm */ protected function getFormFields() { + if ($this->controller->request->getVar('BackURL')) { + $backURL = $this->controller->request->getVar('BackURL'); + } else { + $backURL = Session::get('BackURL'); + } + $label = Member::singleton()->fieldLabel(Member::config()->unique_identifier_field); $fields = FieldList::create( HiddenField::create("AuthenticationMethod", null, $this->authenticator_class, $this), @@ -138,7 +138,7 @@ class MemberLoginForm extends BaseLoginForm ); $emailField->setAttribute('autofocus', 'true'); - if (Security::config()->remember_username) { + if (Security::config()->get('remember_username')) { $emailField->setValue(Session::get('SessionForms.MemberLoginForm.Email')); } else { // Some browsers won't respect this attribute unless it's added to the form @@ -160,6 +160,10 @@ class MemberLoginForm extends BaseLoginForm ); } + if (isset($backURL)) { + $fields->push(HiddenField::create('BackURL', 'BackURL', $backURL)); + } + return $fields; } diff --git a/src/Security/Security.php b/src/Security/Security.php index f028e68ba..b07b18fcb 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -214,7 +214,7 @@ class Security extends Controller implements TemplateGlobalProvider /** * @var Authenticator[] available authenticators */ - private static $authenticators = []; + private $authenticators = []; /** * @var Member Currently logged in user (if available) @@ -224,17 +224,17 @@ class Security extends Controller implements TemplateGlobalProvider /** * @return array */ - public static function getAuthenticators() + public function getAuthenticators() { - return self::$authenticators; + return $this->authenticators; } /** * @param array|Authenticator $authenticators */ - public static function setAuthenticators(array $authenticators) + public function setAuthenticators(array $authenticators) { - self::$authenticators = $authenticators; + $this->authenticators = $authenticators; } /** @@ -274,7 +274,7 @@ class Security extends Controller implements TemplateGlobalProvider */ protected function getAuthenticator($name = 'default') { - $authenticators = static::$authenticators; + $authenticators = $this->authenticators; if (isset($authenticators[$name])) { return $authenticators[$name]; @@ -291,7 +291,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public function getApplicableAuthenticators($service = Authenticator::LOGIN) { - $authenticators = static::$authenticators; + $authenticators = $this->authenticators; /** @var Authenticator $class */ foreach ($authenticators as $name => $class) { @@ -312,7 +312,7 @@ class Security extends Controller implements TemplateGlobalProvider */ public function hasAuthenticator($authenticator) { - $authenticators = static::$authenticators; + $authenticators = $this->authenticators; return !empty($authenticators[$authenticator]); }