Move LostPasswordHandler in to it's own class.

- Moved the Authenticators from statics to normal
- Moved MemberLoginForm methods to the getFormFields as they make more sense there
- Did some spring-cleaning on the LostPasswordHandler
- Removed the BuildResponse from ChangePasswordHandler after spring cleaning

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
                 ) . '</p>'
             );
 
-            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',
-                            '<p>The password reset link is invalid or expired.</p>'
-                            . '<p>You can request a new one <a href="{link1}">here</a> or change your password after'
-                            . ' you <a href="{link2}">logged in</a>.</p>',
-                            [
-                                'link1' => $this->Link('lostpassword'),
-                                'link2' => $this->Link('login')
-                            ]
-                        )
+            $message = [
+                'Content' => DBField::create_field(
+                    'HTMLFragment',
+                    _t(
+                        'SilverStripe\\Security\\Security.NOTERESETLINKINVALID',
+                        '<p>The password reset link is invalid or expired.</p>'
+                        . '<p>You can request a new one <a href="{link1}">here</a> or change your password after'
+                        . ' you <a href="{link2}">logged in</a>.</p>',
+                        [
+                            '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);
     }

src/Security/MemberAuthenticator/LostPasswordForm.php

@@ -0,0 +1,45 @@
+<?php
+
+
+namespace SilverStripe\Security\MemberAuthenticator;
+
+use SilverStripe\Forms\EmailField;
+use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\FormAction;
+
+/**
+ * Class LostPasswordForm handles the requests for lost password form generation
+ *
+ * We need the MemberLoginForm for the getFormFields logic.
+ */
+class LostPasswordForm extends MemberLoginForm
+{
+
+    /**
+     * Create a single EmailField form that has the capability
+     * of using the MemberLoginForm Authenticator
+     *
+     * @return FieldList
+     */
+    public function getFormFields()
+    {
+        return FieldList::create(
+            EmailField::create('Email', _t('SilverStripe\\Security\\Member.EMAIL', 'Email'))
+        );
+    }
+
+    /**
+     * Give the member a friendly button to push
+     *
+     * @return FieldList
+     */
+    public function getFormActions()
+    {
+        return FieldList::create(
+            FormAction::create(
+                'forgotPassword',
+                _t('SilverStripe\\Security\\Security.BUTTONSEND', 'Send me the password reset link')
+            )
+        );
+    }
+}

src/Security/MemberAuthenticator/LostPasswordHandler.php

@@ -135,19 +135,12 @@ class LostPasswordHandler extends RequestHandler
      */
     public function lostPasswordForm()
     {
-        return MemberLoginForm::create(
+        return LostPasswordForm::create(
             $this,
             $this->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 <i>$checkCurrentUser</i> 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();
+    }
 }

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;
     }
 

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]);
     }