Feedback from Damian.

- Move the success and message to a validationresult
- Fix tests for validationresult return
- We need to clear the session in Test logOut method
- Rename to MemberAuthenticator and CMSMemberAuthenticator for consistency.
- Unify all to getCurrentUser on Security
- ChangePasswordHandler removed from Security
- Update SapphireTest for CMS login/logout
- Get the Member ID correctly, if it's an object.
- Only enable "remember me" when it's allowed.
- Add flag to disable password logging
- Remove Subsites coupling, give it an extension hook to disable itself
- Change cascadeLogInTo to cascadeInTo for the logout method logic naming
- Docblocks
- Basicauth config

_config.php

@@ -12,12 +12,6 @@ use SilverStripe\View\Parsers\ShortcodeParser;
  * Here you can make different settings for the Framework module (the core
  * module).
  *
- * For example you can register the authentication methods you wish to use
- * on your site, e.g. to register the OpenID authentication method type
- *
- * <code>
- * Authenticator::register_authenticator('OpenIDAuthenticator');
- * </code>
  */
 
 ShortcodeParser::get('default')

_config/security.yml

@@ -1,18 +1,16 @@
-SilverStripe\Security\MemberAuthenticator\LoginForm:
+---
+Name: coresecurity
+---
+SilverStripe\Security\MemberAuthenticator\MemberLoginForm:
   required_fields:
     - Email
     - Password
 
-SilverStripe\Security\Security:
-  authenticators:
-    default: SilverStripe\Security\MemberAuthenticator\Authenticator
-    cms: SilverStripe\Security\MemberAuthenticator\CMSAuthenticator
-
 SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\RequestProcessor:
     properties:
       filters:
-        - '%$SilverStripe\Security\AuthenticationRequestFilter'
+        - %$SilverStripe\Security\AuthenticationRequestFilter
   SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler:
     properties:
       SessionVariable: loggedInAs
@@ -20,14 +18,15 @@ SilverStripe\Core\Injector\Injector:
     properties:
       TokenCookieName: alc_enc
       DeviceCookieName: alc_device
-      CascadeLogInTo: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler
+      CascadeInTo: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler
   SilverStripe\Security\IdentityStore:
     class: SilverStripe\Security\AuthenticationRequestFilter
-
+  SilverStripe\Security\Security:
+    properties:
+      authenticators:
+        default: %$SilverStripe\Security\MemberAuthenticator\MemberAuthenticator
+        cms: %$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator
 SilverStripe\Security\AuthenticationRequestFilter:
   handlers:
     session: SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler
     alc: SilverStripe\Security\MemberAuthenticator\CookieAuthenticationHandler
-
-SilverStripe\Security\MemberAuthenticator\CMSSecurity:
-  reauth_enabled: true

src/Control/RequestHandler.php

@@ -296,6 +296,20 @@ class RequestHandler extends ViewableData
         return null;
     }
 
+    /**
+     * @param string $link
+     * @return string
+     */
+    protected function addBackURLParam($link)
+    {
+        $backURL = $this->getBackURL();
+        if ($backURL) {
+            return Controller::join_links($link, '?BackURL=' . urlencode($backURL));
+        }
+
+        return $link;
+    }
+
     /**
      * Given a request, and an action name, call that action name on this RequestHandler
      *

src/Core/Config/Config_ForClass.php

@@ -9,7 +9,7 @@ class Config_ForClass
     /**
      * @var string $class
      */
-    public $class;
+    protected $class;
 
     /**
      * @param string|object $class

src/Dev/FunctionalTest.php

@@ -106,8 +106,7 @@ class FunctionalTest extends SapphireTest
         // basis.
         BasicAuth::protect_entire_site(false);
 
-        $this->session()->inst_clear('loggedInAs');
+        $this->logOut();
-        Security::setCurrentUser(null);
 
         SecurityToken::disable();
     }
@@ -412,8 +411,7 @@ class FunctionalTest extends SapphireTest
             $member = $this->objFromFixture('SilverStripe\\Security\\Member', $member);
         }
 
-        $this->session()->inst_set('loggedInAs', $member->ID);
+        $this->logIn($member);
-        Security::setCurrentUser($member);
     }
 
     /**
@@ -422,10 +420,15 @@ class FunctionalTest extends SapphireTest
      */
     public function logOut()
     {
-        $this->session()->inst_set('loggedInAs', null);
+        $this->session()->inst_clear('loggedInAs');
         Security::setCurrentUser(null);
     }
 
+    public function logIn($member)
+    {
+        Security::setCurrentUser($member);
+    }
+
     /**
      * Use the draft (stage) site for testing.
      * This is helpful if you're not testing publication functionality and don't want "stage management" cluttering

src/Dev/SapphireTest.php

@@ -25,6 +25,7 @@ use SilverStripe\Core\Resettable;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\DataExtension;
 use SilverStripe\ORM\SS_List;
+use SilverStripe\Security\IdentityStore;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataModel;
@@ -1250,7 +1251,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase
 
             $this->cache_generatedMembers[$permCode] = $member;
         }
-        Security::setCurrentUser($member);
+        Injector::inst()->get(IdentityStore::class)->logIn($member);
         return $member->ID;
     }
 

src/Forms/FormRequestHandler.php

@@ -233,18 +233,18 @@ class FormRequestHandler extends RequestHandler
 
             // Otherwise, try a handler method on the form request handler.
             if ($this->hasMethod($funcName)) {
-                return $this->$funcName($vars, $this->form, $request);
+                return $this->$funcName($vars, $this->form, $request, $this);
             }
 
             // Otherwise, try a handler method on the form itself
             if ($this->form->hasMethod($funcName)) {
-                return $this->form->$funcName($vars, $this->form, $request);
+                return $this->form->$funcName($vars, $this->form, $request, $this);
             }
 
             // Check for inline actions
             $field = $this->checkFieldsForAction($this->form->Fields(), $funcName);
             if ($field) {
-                return $field->$funcName($vars, $this->form, $request);
+                return $field->$funcName($vars, $this->form, $request, $this);
             }
         } catch (ValidationException $e) {
             // The ValdiationResult contains all the relevant metadata

src/Security/AuthenticationRequestFilter.php

@@ -2,15 +2,16 @@
 
 namespace SilverStripe\Security;
 
-use Exception;
 use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Control\RequestFilter;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
+use SilverStripe\Dev\Debug;
 use SilverStripe\ORM\DataModel;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\ORM\ValidationException;
 
 class AuthenticationRequestFilter implements RequestFilter, IdentityStore
 {
@@ -18,10 +19,21 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
     use Configurable;
 
     /**
-     * @return array|IdentityStore[]
+     * @var array|AuthenticationHandler[]
+     */
+    protected $handlers;
+
+    /**
+     * This method currently uses a fallback as loading the handlers via YML has proven unstable
+     *
+     * @return array|AuthenticationHandler[]
      */
     protected function getHandlers()
     {
+        if (is_array($this->handlers)) {
+            return $this->handlers;
+        }
+
         return array_map(
             function ($identifier) {
                 return Injector::inst()->get($identifier);
@@ -30,24 +42,39 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
         );
     }
 
+    /**
+     * Set an associative array of handlers
+     *
+     * @param array|AuthenticationHandler[] $handlers
+     */
+    public function setHandlers($handlers)
+    {
+        $this->handlers = $handlers;
+    }
+
     /**
      * Identify the current user from the request
+     *
+     * @param HTTPRequest $request
+     * @param Session $session
+     * @param DataModel $model
+     * @return bool|void
+     * @throws HTTPResponse_Exception
      */
     public function preRequest(HTTPRequest $request, Session $session, DataModel $model)
     {
         try {
             /** @var AuthenticationHandler $handler */
-            foreach ($this->getHandlers() as $handler) {
+            foreach ($this->getHandlers() as $name => $handler) {
                 // @todo Update requestfilter logic to allow modification of initial response
                 // in order to add cookies, etc
-                $member = $handler->authenticateRequest($request, new HTTPResponse());
+                $member = $handler->authenticateRequest($request);
                 if ($member) {
-                    // @todo Remove the static coupling here
                     Security::setCurrentUser($member);
                     break;
                 }
             }
-        } catch (Exception $e) { // There's no valid exception currently. I would say AuthenticationException?
+        } catch (ValidationException $e) {
             throw new HTTPResponse_Exception(
                 "Bad log-in details: " . $e->getMessage(),
                 400
@@ -57,6 +84,11 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
 
     /**
      * No-op
+     *
+     * @param HTTPRequest $request
+     * @param HTTPResponse $response
+     * @param DataModel $model
+     * @return bool|void
      */
     public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model)
     {
@@ -65,11 +97,13 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
     /**
      * Log into the identity-store handlers attached to this request filter
      *
-     * @inherit
+     * @param Member $member
+     * @param bool $persistent
+     * @param HTTPRequest $request
+     * @return HTTPResponse|void
      */
-    public function logIn(Member $member, $persistent, HTTPRequest $request)
+    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
     {
-        // @todo Coupling here isn't ideal.
         $member->beforeMemberLoggedIn();
 
         foreach ($this->getHandlers() as $handler) {
@@ -78,7 +112,6 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
             }
         }
 
-        // @todo Coupling here isn't ideal.
         Security::setCurrentUser($member);
         $member->afterMemberLoggedIn();
     }
@@ -86,9 +119,10 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
     /**
      * Log out of all the identity-store handlers attached to this request filter
      *
-     * @inherit
+     * @param HTTPRequest $request
+     * @return HTTPResponse|void
      */
-    public function logOut(HTTPRequest $request)
+    public function logOut(HTTPRequest $request = null)
     {
         foreach ($this->getHandlers() as $handler) {
             if ($handler instanceof IdentityStore) {
@@ -96,7 +130,6 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
             }
         }
 
-        // @todo Coupling here isn't ideal.
         Security::setCurrentUser(null);
     }
 }

src/Security/Authenticator.php

@@ -7,6 +7,7 @@ use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Control\Controller;
 use SilverStripe\Forms\Form;
+use SilverStripe\ORM\ValidationResult;
 
 /**
  * Abstract base class for an authentication method
@@ -74,7 +75,8 @@ interface Authenticator
 
 
     /**
-     * @todo
+     * @param $link
+     * @return mixed
      */
     public function getLostPasswordHandler($link);
 
@@ -82,14 +84,8 @@ interface Authenticator
      * Method to authenticate an user.
      *
      * @param array $data Raw data to authenticate the user.
-     * @param string $message A variable to return an error message if authentication fails
+     * @param ValidationResult $result A validationresult which is either valid or contains the error message(s)
      * @return Member The matched member, or null if the authentication fails
      */
-    public function authenticate($data, &$message);
+    public function authenticate($data, &$result);
-
-    /**
-     * Return the keys that should be passed to authenticate()
-     * @return array
-     */
-//    public function getAuthenticateFields();
 }

src/Security/BasicAuth.php

@@ -7,13 +7,10 @@ use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Config\Configurable;
-use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Core\Injector\Injector;
 
-use SilverStripe\Security\MemberAuthenticator\Authenticator;
+use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
 
 /**
  * Provides an interface to HTTP basic authentication.
@@ -47,7 +44,7 @@ class BasicAuth
      * @var String Message that shows in the authentication box.
      * Set this value through {@link protect_entire_site()}.
      */
-    private static $entire_site_protected_message = "SilverStripe test website. Use your CMS login.";
+    private static $entire_site_protected_message = 'SilverStripe test website. Use your CMS login.';
 
     /**
      * Require basic authentication.  Will request a username and password if none is given.
@@ -63,9 +60,13 @@ class BasicAuth
      * @return bool|Member
      * @throws HTTPResponse_Exception
      */
-    public static function requireLogin(HTTPRequest $request, $realm, $permissionCode = null, $tryUsingSessionLogin = true)
+    public static function requireLogin(
-    {
+        HTTPRequest $request,
-        $isRunningTests = (class_exists('SilverStripe\\Dev\\SapphireTest', false) && SapphireTest::is_running_test());
+        $realm,
+        $permissionCode = null,
+        $tryUsingSessionLogin = true
+    ) {
+        $isRunningTests = (class_exists(SapphireTest::class, false) && SapphireTest::is_running_test());
         if (!Security::database_is_ready() || (Director::is_cli() && !$isRunningTests)) {
             return true;
         }
@@ -89,16 +90,21 @@ class BasicAuth
         $member = null;
 
         if ($request->getHeader('PHP_AUTH_USER') && $request->getHeader('PHP_AUTH_PW')) {
-            /** @var Authenticator $authenticator */
+            /** @var MemberAuthenticator $authenticator */
-            $authenticator = Injector::inst()->get(Authenticator::class);
+            $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::LOGIN);
 
+            foreach ($authenticators as $name => $authenticator) {
                 $member = $authenticator->authenticate([
                     'Email' => $request->getHeader('PHP_AUTH_USER'),
                     'Password' => $request->getHeader('PHP_AUTH_PW'),
-            ], $dummy);
+                ]);
+                if ($member instanceof Member) {
+                    break;
+                }
+            }
         }
 
-        if($member) {
+        if ($member instanceof Member) {
             Security::setCurrentUser($member);
         }
 
@@ -112,9 +118,19 @@ class BasicAuth
             $response->addHeader('WWW-Authenticate', "Basic realm=\"$realm\"");
 
             if ($request->getHeader('PHP_AUTH_USER')) {
-                $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ERRORNOTREC', "That username / password isn't recognised"));
+                $response->setBody(
+                    _t(
+                        'SilverStripe\\Security\\BasicAuth.ERRORNOTREC',
+                        "That username / password isn't recognised"
+                    )
+                );
             } else {
-                $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ENTERINFO', "Please enter a username and password."));
+                $response->setBody(
+                    _t(
+                        'SilverStripe\\Security\\BasicAuth.ENTERINFO',
+                        'Please enter a username and password.'
+                    )
+                );
             }
 
             // Exception is caught by RequestHandler->handleRequest() and will halt further execution
@@ -128,7 +144,12 @@ class BasicAuth
             $response->addHeader('WWW-Authenticate', "Basic realm=\"$realm\"");
 
             if ($request->getHeader('PHP_AUTH_USER')) {
-                $response->setBody(_t('SilverStripe\\Security\\BasicAuth.ERRORNOTADMIN', "That user is not an administrator."));
+                $response->setBody(
+                    _t(
+                        'SilverStripe\\Security\\BasicAuth.ERRORNOTADMIN',
+                        'That user is not an administrator.'
+                    )
+                );
             }
 
             // Exception is caught by RequestHandler->handleRequest() and will halt further execution
@@ -160,9 +181,9 @@ class BasicAuth
      */
     public static function protect_entire_site($protect = true, $code = 'ADMIN', $message = null)
     {
-        Config::modify()->set(self::class, 'entire_site_protected', $protect);
+        static::config()->set('entire_site_protected', $protect);
-        Config::modify()->set(self::class, 'entire_site_protected_code', $code);
+        static::config()->set('entire_site_protected_code', $code);
-        Config::modify()->set(self::class, 'entire_site_protected_message', $message);
+        static::config()->set('entire_site_protected_message', $message);
     }
 
     /**
@@ -174,7 +195,7 @@ class BasicAuth
      */
     public static function protect_site_if_necessary()
     {
-        $config = Config::forClass(BasicAuth::class);
+        $config = static::config();
         $request = Controller::curr()->getRequest();
         if ($config->get('entire_site_protected')) {
             /** @noinspection ExceptionsAnnotatingAndHandlingInspection */
@@ -182,7 +203,8 @@ class BasicAuth
                 $request,
                 $config->get('entire_site_protected_message'),
                 $config->get('entire_site_protected_code'),
-                false);
+                false
+            );
         }
     }
 }

src/Security/CMSMemberLoginForm.php

@@ -15,7 +15,7 @@ use SilverStripe\Security\Security;
 /**
  * Provides the in-cms session re-authentication form for the "member" authenticator
  */
-class CMSMemberLoginForm extends LoginForm
+class CMSMemberLoginForm extends MemberLoginForm
 {
 
     /**

src/Security/CMSSecurity.php

@@ -10,7 +10,7 @@ use SilverStripe\Control\Director;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\FieldType\DBField;
-use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator;
+use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator;
 use SilverStripe\View\Requirements;
 
 /**
@@ -44,7 +44,7 @@ class CMSSecurity extends Security
         Requirements::javascript(FRAMEWORK_ADMIN_DIR . '/client/dist/js/vendor.js');
     }
 
-    public function login($request, $service = Authenticator::CMS_LOGIN)
+    public function login($request = null, $service = Authenticator::CMS_LOGIN)
     {
         return parent::login($request, Authenticator::CMS_LOGIN);
     }
@@ -60,9 +60,9 @@ class CMSSecurity extends Security
         return parent::getAuthenticator($name);
     }
 
-    public static function getAuthenticators($service = Authenticator::CMS_LOGIN)
+    public function getApplicableAuthenticators($service = Authenticator::CMS_LOGIN)
     {
-        return parent::getAuthenticators($service);
+        return parent::getApplicableAuthenticators($service);
     }
 
     /**
@@ -97,7 +97,7 @@ class CMSSecurity extends Security
     public function getTitle()
     {
         // Check if logged in already
-        if (Member::currentUserID()) {
+        if (Security::getCurrentUser()) {
             return _t('SilverStripe\\Security\\CMSSecurity.SUCCESS', 'Success');
         }
 
@@ -174,19 +174,7 @@ PHP
             return false;
         }
 
-        /** @var [] $authenticators */
+        return count(Security::singleton()->getApplicableAuthenticators(Authenticator::CMS_LOGIN)) > 0;
-        $authenticators = Security::config()->get('authenticators');
-        foreach ($authenticators as $name => $authenticator) {
-            // Supported if at least one authenticator is supported
-            $authenticator = Injector::inst()->get($authenticator);
-            if (($authenticator->supportedServices() & Authenticator::CMS_LOGIN)
-                && Security::hasAuthenticator($name)
-            ) {
-                return true;
-            }
-        }
-
-        return false;
     }
 
     /**
@@ -197,7 +185,7 @@ PHP
     public function success()
     {
         // Ensure member is properly logged in
-        if (!Member::currentUserID() || !class_exists(AdminRootController::class)) {
+        if (!Security::getCurrentUser() || !class_exists(AdminRootController::class)) {
             return $this->redirectToExternalLogin();
         }
 

src/Security/IdentityStore.php

@@ -4,6 +4,7 @@ namespace SilverStripe\Security;
 
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Core\Config\Configurable;
 
 /**
  * Represents an authentication handler that can have identities logged into & out of it.
@@ -18,15 +19,15 @@ interface IdentityStore
      * @param Member $member The member to log in.
      * @param Boolean $persistent boolean If set to true, the login may persist beyond the current session.
      * @param HTTPRequest $request The request of the visitor that is logging in, to get, for example, cookies.
-     * @param HTTPResponse $response The response object to modify, if needed.
+     * @return HTTPResponse $response The response object to modify, if needed.
      */
-    public function logIn(Member $member, $persistent, HTTPRequest $request);
+    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null);
 
     /**
      * Log any logged-in member out of this identity store.
      *
      * @param HTTPRequest $request The request of the visitor that is logging out, to get, for example, cookies.
-     * @param HTTPResponse $response The response object to modify, if needed.
+     * @return HTTPResponse $response The response object to modify, if needed.
      */
-    public function logOut(HTTPRequest $request);
+    public function logOut(HTTPRequest $request = null);
 }

src/Security/Member.php

@@ -5,6 +5,7 @@ namespace SilverStripe\Security;
 use IntlDateFormatter;
 use SilverStripe\Admin\LeftAndMain;
 use SilverStripe\CMS\Controllers\CMSMain;
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\Cookie;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\Email\Email;
@@ -12,6 +13,8 @@ use SilverStripe\Control\Email\Mailer;
 use SilverStripe\Control\Session;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\Debug;
+use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Dev\TestMailer;
 use SilverStripe\Forms\ConfirmedPasswordField;
@@ -22,6 +25,7 @@ use SilverStripe\Forms\ListboxField;
 use SilverStripe\i18n\i18n;
 use SilverStripe\MSSQL\MSSQLDatabase;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
@@ -191,6 +195,12 @@ class Member extends DataObject
      */
     private static $password_expiry_days = null;
 
+    /**
+     * @config
+     * @var bool enable or disable logging of previously used passwords. See {@link onAfterWrite}
+     */
+    private static $password_logging_enabled = true;
+
     /**
      * @config
      * @var Int Number of incorrect logins after which
@@ -276,7 +286,7 @@ class Member extends DataObject
 
         // Find member
         /** @skipUpgrade */
-        $admin = Member::get()
+        $admin = static::get()
             ->filter('Email', Security::default_admin_username())
             ->first();
         if (!$admin) {
@@ -284,7 +294,7 @@ class Member extends DataObject
             // persistent logins in the database. See Security::setDefaultAdmin().
             // Set 'Email' to identify this as the default admin
             $admin = Member::create();
-            $admin->FirstName = _t(__CLASS__.'.DefaultAdminFirstname', 'Default Admin');
+            $admin->FirstName = _t(__CLASS__ . '.DefaultAdminFirstname', 'Default Admin');
             $admin->Email = Security::default_admin_username();
             $admin->write();
         }
@@ -323,14 +333,15 @@ class Member extends DataObject
 
         // Check a password is set on this member
         if (empty($this->Password) && $this->exists()) {
-            $result->addError(_t(__CLASS__.'.NoPassword', 'There is no password on this member.'));
+            $result->addError(_t(__CLASS__ . '.NoPassword', 'There is no password on this member.'));
+
             return $result;
         }
 
         $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
         if (!$e->check($this->Password, $password, $this->Salt, $this)) {
             $result->addError(_t(
-                __CLASS__.'.ERRORWRONGCRED',
+                __CLASS__ . '.ERRORWRONGCRED',
                 'The provided details don\'t seem to be correct. Please try again.'
             ));
         }
@@ -364,7 +375,7 @@ class Member extends DataObject
         if ($this->isLockedOut()) {
             $result->addError(
                 _t(
-                    __CLASS__.'.ERRORLOCKEDOUT2',
+                    __CLASS__ . '.ERRORLOCKEDOUT2',
                     'Your account has been temporarily disabled because of too many failed attempts at ' .
                     'logging in. Please try again in {count} minutes.',
                     null,
@@ -374,6 +385,7 @@ class Member extends DataObject
         }
 
         $this->extend('canLogIn', $result);
+
         return $result;
     }
 
@@ -382,11 +394,12 @@ class Member extends DataObject
      *
      * @return bool
      */
-    protected function isLockedOut()
+    public function isLockedOut()
     {
         if (!$this->LockedOutUntil) {
             return false;
         }
+
         return DBDatetime::now()->getTimestamp() < $this->dbObject('LockedOutUntil')->getTimestamp();
     }
 
@@ -416,17 +429,20 @@ class Member extends DataObject
         if (!$this->PasswordExpiry) {
             return false;
         }
+
         return strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry);
     }
 
     /**
-     * @deprecated Use Security::setCurrentUser() or IdentityStore::logIn()
+     * @deprecated 5.0.0 Use Security::setCurrentUser() or IdentityStore::logIn()
      *
-     * @param bool $remember If set to TRUE, the member will be logged in automatically the next time.
      */
     public function logIn()
     {
-        user_error("This method is deprecated and now only logs in for the current request", E_USER_WARNING);
+        Deprecation::notice(
+            '5.0.0',
+            'This method is deprecated and only logs in for the current request. Please use Security::setCurrentUser($user) or an IdentityStore'
+        );
         Security::setCurrentUser($this);
     }
 
@@ -478,53 +494,42 @@ class Member extends DataObject
      * has a database record of the same ID. If there is
      * no logged in user, FALSE is returned anyway.
      *
+     * @deprecated Not needed anymore, as it returns Security::getCurrentUser();
+     *
      * @return boolean TRUE record found FALSE no record found
      */
     public static function logged_in_session_exists()
     {
-        if ($id = Member::currentUserID()) {
+        Deprecation::notice(
-            if ($member = DataObject::get_by_id(Member::class, $id)) {
+            '5.0.0',
-                if ($member->exists()) {
+            'This method is deprecated and now does not add value. Please use Security::getCurrentUser()'
+        );
+
+        if ($member = Security::getCurrentUser()) {
+            if ($member && $member->exists()) {
                 return true;
             }
         }
-        }
 
         return false;
     }
 
     /**
+     * @deprecated Use Security::setCurrentUser(null) or an IdentityStore
      * Logs this member out.
      */
     public function logOut()
     {
+        Deprecation::notice(
+            '5.0.0',
+            'This method is deprecated and now does not persist. Please use Security::setCurrentUser(null) or an IdenityStore'
+        );
+
         $this->extend('beforeMemberLoggedOut');
 
-        Session::clear("loggedInAs");
+        Injector::inst()->get(IdentityStore::class)->logOut(Controller::curr()->getRequest());
-        if (Member::config()->login_marker_cookie) {
-            Cookie::set(Member::config()->login_marker_cookie, null, 0);
-        }
-
-        Session::destroy();
-
-        $this->extend('memberLoggedOut');
-
-        // Clears any potential previous hashes for this member
-        RememberLoginHash::clear($this, Cookie::get('alc_device'));
-
-        Cookie::set('alc_enc', null); // // Clear the Remember Me cookie
-        Cookie::force_expiry('alc_enc');
-        Cookie::set('alc_device', null);
-        Cookie::force_expiry('alc_device');
-
-        // Switch back to live in order to avoid infinite loops when
-        // redirecting to the login screen (if this login screen is versioned)
-        Session::clear('readingMode');
-
-        $this->write();
-
         // Audit logging hook
-        $this->extend('memberLoggedOut');
+        $this->extend('afterMemberLoggedOut');
     }
 
     /**
@@ -548,6 +553,7 @@ class Member extends DataObject
 
         // We assume we have PasswordEncryption and Salt available here.
         $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
+
         return $e->encrypt($string, $this->Salt);
     }
 
@@ -590,6 +596,7 @@ class Member extends DataObject
     {
         $hash = $this->encryptWithUserSettings($autologinToken);
         $member = self::member_from_autologinhash($hash, false);
+
         return (bool)$member;
     }
 
@@ -605,13 +612,13 @@ class Member extends DataObject
     public static function member_from_autologinhash($hash, $login = false)
     {
         /** @var Member $member */
-        $member = Member::get()->filter([
+        $member = static::get()->filter([
             'AutoLoginHash'                => $hash,
             'AutoLoginExpired:GreaterThan' => DBDatetime::now()->getValue(),
         ])->first();
 
         if ($login && $member) {
-            $member->logIn();
+            Injector::inst()->get(IdentityStore::class)->logIn($member);
         }
 
         return $member;
@@ -625,11 +632,12 @@ class Member extends DataObject
      */
     public static function member_from_tempid($tempid)
     {
-        $members = Member::get()
+        $members = static::get()
             ->filter('TempIDHash', $tempid);
 
         // Exclude expired
         if (static::config()->get('temp_id_lifetime')) {
+            /** @var DataList|Member[] $members */
             $members = $members->filter('TempIDExpired:GreaterThan', DBDatetime::now()->getValue());
         }
 
@@ -640,6 +648,8 @@ class Member extends DataObject
      * Returns the fields for the member form - used in the registration/profile module.
      * It should return fields that are editable by the admin and the logged-in user.
      *
+     * @todo possibly move this to an extension
+     *
      * @return FieldList Returns a {@link FieldList} containing the fields for
      *                   the member form.
      */
@@ -660,6 +670,7 @@ class Member extends DataObject
 
 
         $this->extend('updateMemberFormFields', $fields);
+
         return $fields;
     }
 
@@ -672,7 +683,7 @@ class Member extends DataObject
     {
         $editingPassword = $this->isInDB();
         $label = $editingPassword
-            ? _t(__CLASS__.'.EDIT_PASSWORD', 'New Password')
+            ? _t(__CLASS__ . '.EDIT_PASSWORD', 'New Password')
             : $this->fieldLabel('Password');
         /** @var ConfirmedPasswordField $password */
         $password = ConfirmedPasswordField::create(
@@ -684,12 +695,13 @@ class Member extends DataObject
         );
 
         // If editing own password, require confirmation of existing
-        if ($editingPassword && $this->ID == Member::currentUserID()) {
+        if ($editingPassword && $this->ID == Security::getCurrentUser()->ID) {
             $password->setRequireExistingPassword(true);
         }
 
         $password->setCanBeEmpty(true);
         $this->extend('updateMemberPasswordField', $password);
+
         return $password;
     }
 
@@ -717,12 +729,17 @@ class Member extends DataObject
     /**
      * Returns the current logged in user
      *
-     * @deprecated use Security::getCurrentUser()
+     * @deprecated 5.0.0 use Security::getCurrentUser()
      *
      * @return Member
      */
     public static function currentUser()
     {
+        Deprecation::notice(
+            '5.0.0',
+            'This method is deprecated. Please use Security::getCurrentUser() or an IdentityStore'
+        );
+
         return Security::getCurrentUser();
     }
 
@@ -761,12 +778,17 @@ class Member extends DataObject
     /**
      * Get the ID of the current logged in user
      *
-     * @deprecated use Security::getCurrentUser()
+     * @deprecated 5.0.0 use Security::getCurrentUser()
      *
      * @return int Returns the ID of the current logged in user or 0.
      */
     public static function currentUserID()
     {
+        Deprecation::notice(
+            '5.0.0',
+            'This method is deprecated. Please use Security::getCurrentUser() or an IdentityStore'
+        );
+
         if ($member = Security::getCurrentUser()) {
             return $member->ID;
         } else {
@@ -774,7 +796,7 @@ class Member extends DataObject
         }
     }
 
-    /*
+    /**
      * Generate a random password, with randomiser to kick in if there's no words file on the
      * filesystem.
      *
@@ -788,16 +810,17 @@ class Member extends DataObject
             $words = file($words);
 
             list($usec, $sec) = explode(' ', microtime());
-            srand($sec + ((float) $usec * 100000));
+            mt_srand($sec + ((float)$usec * 100000));
 
-            $word = trim($words[rand(0, sizeof($words)-1)]);
+            $word = trim($words[random_int(0, count($words) - 1)]);
-            $number = rand(10, 999);
+            $number = random_int(10, 999);
 
             return $word . $number;
         } else {
-            $random = rand();
+            $random = mt_rand();
             $string = md5($random);
             $output = substr($string, 0, 8);
+
             return $output;
         }
     }
@@ -827,7 +850,7 @@ class Member extends DataObject
 
             if ($existingRecord) {
                 throw new ValidationException(_t(
-                    __CLASS__.'.ValidationIdentifierFailed',
+                    __CLASS__ . '.ValidationIdentifierFailed',
                     'Can\'t overwrite existing member #{id} with identical identifier ({name} = {value}))',
                     'Values in brackets show "fieldname = value", usually denoting an existing email address',
                     array(
@@ -841,6 +864,7 @@ class Member extends DataObject
 
         // We don't send emails out on dev/tests sites to prevent accidentally spamming users.
         // However, if TestMailer is in use this isn't a risk.
+        // @todo some developers use external tools, so emailing might be a good idea anyway
         if ((Director::isLive() || Injector::inst()->get(Mailer::class) instanceof TestMailer)
             && $this->isChanged('Password')
             && $this->record['Password']
@@ -850,7 +874,7 @@ class Member extends DataObject
                 ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail')
                 ->setData($this)
                 ->setTo($this->Email)
-                ->setSubject(_t(__CLASS__.'.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject'))
+                ->setSubject(_t(__CLASS__ . '.SUBJECTPASSWORDCHANGED', "Your password has been changed", 'Email subject'))
                 ->send();
         }
 
@@ -858,7 +882,7 @@ class Member extends DataObject
         // Note that this only works with cleartext passwords, as we can't rehash
         // existing passwords.
         if ((!$this->ID && $this->Password) || $this->isChanged('Password')) {
-            //reset salt so that it gets regenerated - this will invalidate any persistant login cookies
+            //reset salt so that it gets regenerated - this will invalidate any persistent login cookies
             // or other information encrypted with this Member's settings (see self::encryptWithUserSettings)
             $this->Salt = '';
             // Password was changed: encrypt the password according the settings
@@ -878,8 +902,8 @@ class Member extends DataObject
             // If we haven't manually set a password expiry
             if (!$this->isChanged('PasswordExpiry')) {
                 // then set it for us
-                if (self::config()->password_expiry_days) {
+                if (static::config()->get('password_expiry_days')) {
-                    $this->PasswordExpiry = date('Y-m-d', time() + 86400 * self::config()->password_expiry_days);
+                    $this->PasswordExpiry = date('Y-m-d', time() + 86400 * static::config()->get('password_expiry_days'));
                 } else {
                     $this->PasswordExpiry = null;
                 }
@@ -900,7 +924,7 @@ class Member extends DataObject
 
         Permission::reset();
 
-        if ($this->isChanged('Password')) {
+        if ($this->isChanged('Password') && static::config()->get('password_logging_enabled')) {
             MemberPassword::log($this);
         }
     }
@@ -924,6 +948,7 @@ class Member extends DataObject
             $password->delete();
             $password->destroy();
         }
+
         return $this;
     }
 
@@ -944,6 +969,7 @@ class Member extends DataObject
         // If there are no admin groups in this set then it's ok
         $adminGroups = Permission::get_groups_by_permission('ADMIN');
         $adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array();
+
         return count(array_intersect($ids, $adminGroupIDs)) == 0;
     }
 
@@ -1081,6 +1107,7 @@ class Member extends DataObject
             foreach ($format['columns'] as $col) {
                 $values[] = $this->getField($col);
             }
+
             return implode($format['sep'], $values);
         }
         if ($this->getField('ID') === 0) {
@@ -1124,7 +1151,8 @@ class Member extends DataObject
         }
 
         $sepSQL = Convert::raw2sql($format['sep'], true);
-        return "(".join(" $op $sepSQL $op ", $columnsWithTablename).")";
+
+        return "(" . join(" $op $sepSQL $op ", $columnsWithTablename) . ")";
     }
 
 
@@ -1195,6 +1223,7 @@ class Member extends DataObject
         if ($locale) {
             return $locale;
         }
+
         return i18n::get_locale();
     }
 
@@ -1271,7 +1300,7 @@ class Member extends DataObject
 
         // No groups, return all Members
         if (!$groupIDList) {
-            return Member::get()->sort(array('Surname'=>'ASC', 'FirstName'=>'ASC'))->map();
+            return static::get()->sort(array('Surname' => 'ASC', 'FirstName' => 'ASC'))->map();
         }
 
         $membersList = new ArrayList();
@@ -1281,6 +1310,7 @@ class Member extends DataObject
         }
 
         $membersList->removeDuplicates('ID');
+
         return $membersList->map();
     }
 
@@ -1335,7 +1365,7 @@ class Member extends DataObject
         }
 
         /** @skipUpgrade */
-        $members = Member::get()
+        $members = static::get()
             ->innerJoin("Group_Members", '"Group_Members"."MemberID" = "Member"."ID"')
             ->innerJoin("Group", '"Group"."ID" = "Group_Members"."GroupID"');
         if ($groupIDList) {
@@ -1395,12 +1425,12 @@ class Member extends DataObject
 
             $mainFields->replaceField('Locale', new DropdownField(
                 "Locale",
-                _t(__CLASS__.'.INTERFACELANG', "Interface Language", 'Language of the CMS'),
+                _t(__CLASS__ . '.INTERFACELANG', "Interface Language", 'Language of the CMS'),
                 i18n::getSources()->getKnownLocales()
             ));
             $mainFields->removeByName(static::config()->get('hidden_fields'));
 
-            if (! static::config()->get('lock_out_after_incorrect_logins')) {
+            if (!static::config()->get('lock_out_after_incorrect_logins')) {
                 $mainFields->removeByName('FailedLoginCount');
             }
 
@@ -1426,7 +1456,7 @@ class Member extends DataObject
                         ->setSource($groupsMap)
                         ->setAttribute(
                             'data-placeholder',
-                            _t(__CLASS__.'.ADDGROUP', 'Add group', 'Placeholder text for a dropdown')
+                            _t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown')
                         )
                 );
 
@@ -1465,21 +1495,22 @@ class Member extends DataObject
     {
         $labels = parent::fieldLabels($includerelations);
 
-        $labels['FirstName'] = _t(__CLASS__.'.FIRSTNAME', 'First Name');
+        $labels['FirstName'] = _t(__CLASS__ . '.FIRSTNAME', 'First Name');
-        $labels['Surname'] = _t(__CLASS__.'.SURNAME', 'Surname');
+        $labels['Surname'] = _t(__CLASS__ . '.SURNAME', 'Surname');
         /** @skipUpgrade */
-        $labels['Email'] = _t(__CLASS__.'.EMAIL', 'Email');
+        $labels['Email'] = _t(__CLASS__ . '.EMAIL', 'Email');
-        $labels['Password'] = _t(__CLASS__.'.db_Password', 'Password');
+        $labels['Password'] = _t(__CLASS__ . '.db_Password', 'Password');
-        $labels['PasswordExpiry'] = _t(__CLASS__.'.db_PasswordExpiry', 'Password Expiry Date', 'Password expiry date');
+        $labels['PasswordExpiry'] = _t(__CLASS__ . '.db_PasswordExpiry', 'Password Expiry Date', 'Password expiry date');
-        $labels['LockedOutUntil'] = _t(__CLASS__.'.db_LockedOutUntil', 'Locked out until', 'Security related date');
+        $labels['LockedOutUntil'] = _t(__CLASS__ . '.db_LockedOutUntil', 'Locked out until', 'Security related date');
-        $labels['Locale'] = _t(__CLASS__.'.db_Locale', 'Interface Locale');
+        $labels['Locale'] = _t(__CLASS__ . '.db_Locale', 'Interface Locale');
         if ($includerelations) {
             $labels['Groups'] = _t(
-                __CLASS__.'.belongs_many_many_Groups',
+                __CLASS__ . '.belongs_many_many_Groups',
                 'Groups',
                 'Security Groups this member belongs to'
             );
         }
+
         return $labels;
     }
 
@@ -1511,6 +1542,7 @@ class Member extends DataObject
         if ($this->ID == $member->ID) {
             return true;
         }
+
         //standard check
         return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin');
     }
@@ -1547,9 +1579,11 @@ class Member extends DataObject
         if ($this->ID == $member->ID) {
             return true;
         }
+
         //standard check
         return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin');
     }
+
     /**
      * Users can edit their own record.
      * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions
@@ -1582,10 +1616,11 @@ class Member extends DataObject
         // this is a hack because what this should do is to stop a user
         // deleting a member who has more privileges (e.g. a non-Admin deleting an Admin)
         if (Permission::checkMember($this, 'ADMIN')) {
-            if (! Permission::checkMember($member, 'ADMIN')) {
+            if (!Permission::checkMember($member, 'ADMIN')) {
                 return false;
             }
         }
+
         //standard check
         return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin');
     }
@@ -1644,7 +1679,7 @@ class Member extends DataObject
 
             if ($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
                 $lockoutMins = self::config()->lock_out_delay_mins;
-                $this->LockedOutUntil = date('Y-m-d H:i:s', DBDatetime::now()->getTimestamp() + $lockoutMins*60);
+                $this->LockedOutUntil = date('Y-m-d H:i:s', DBDatetime::now()->getTimestamp() + $lockoutMins * 60);
                 $this->FailedLoginCount = 0;
             }
         }

src/Security/MemberAuthenticator/CMSMemberAuthenticator.php

@@ -2,10 +2,11 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
+use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Authenticator as BaseAuthenticator;
 use SilverStripe\Security\Member;
 
-class CMSAuthenticator extends Authenticator
+class CMSMemberAuthenticator extends MemberAuthenticator
 {
 
     public function supportedServices()
@@ -15,11 +16,11 @@ class CMSAuthenticator extends Authenticator
 
     /**
      * @param array $data
-     * @param $message
+     * @param ValidationResult|null $result
-     * @param bool $success
+     * @param Member|null $member
      * @return Member
      */
-    protected function authenticateMember($data, &$message, &$success, $member = null)
+    protected function authenticateMember($data, &$result = null, $member = null)
     {
         // Attempt to identify by temporary ID
         if (!empty($data['tempid'])) {
@@ -30,9 +31,13 @@ class CMSAuthenticator extends Authenticator
             }
         }
 
-        return parent::authenticateMember($data, $message, $success, $member);
+        return parent::authenticateMember($data, $result, $member);
     }
 
+    /**
+     * @param string $link
+     * @return CMSLoginHandler
+     */
     public function getLoginHandler($link)
     {
         return CMSLoginHandler::create($link, $this);

src/Security/MemberAuthenticator/ChangePasswordForm.php

@@ -31,36 +31,51 @@ class ChangePasswordForm extends Form
         $backURL = $controller->getBackURL() ?: Session::get('BackURL');
 
         if (!$fields) {
-            $fields = new FieldList();
+            $fields = $this->getFormFields();
-
-            // Security/changepassword?h=XXX redirects to Security/changepassword
-            // without GET parameter to avoid potential HTTP referer leakage.
-            // In this case, a user is not logged in, and no 'old password' should be necessary.
-            if (Security::getCurrentUser()) {
-                $fields->push(new PasswordField("OldPassword", _t('SilverStripe\\Security\\Member.YOUROLDPASSWORD', "Your old password")));
-            }
-
-            $fields->push(new PasswordField("NewPassword1", _t('SilverStripe\\Security\\Member.NEWPASSWORD', "New Password")));
-            $fields->push(new PasswordField("NewPassword2", _t('SilverStripe\\Security\\Member.CONFIRMNEWPASSWORD', "Confirm New Password")));
         }
         if (!$actions) {
-            $actions = new FieldList(
+            $actions = $this->getFormActions();
-                new FormAction("doChangePassword", _t('SilverStripe\\Security\\Member.BUTTONCHANGEPASSWORD', "Change Password"))
-            );
         }
 
         if ($backURL) {
-            $fields->push(new HiddenField('BackURL', false, $backURL));
+            $fields->push(HiddenField::create('BackURL', false, $backURL));
         }
 
         parent::__construct($controller, $name, $fields, $actions);
     }
 
     /**
-     * @return ChangePasswordHandler
+     * @return FieldList
      */
-    protected function buildRequestHandler()
+    protected function getFormFields()
     {
-        return ChangePasswordHandler::create($this);
+        $fields = FieldList::create();
+
+        // Security/changepassword?h=XXX redirects to Security/changepassword
+        // without GET parameter to avoid potential HTTP referer leakage.
+        // In this case, a user is not logged in, and no 'old password' should be necessary.
+        if (Security::getCurrentUser()) {
+            $fields->push(PasswordField::create('OldPassword', _t('SilverStripe\\Security\\Member.YOUROLDPASSWORD', 'Your old password')));
+        }
+
+        $fields->push(PasswordField::create('NewPassword1', _t('SilverStripe\\Security\\Member.NEWPASSWORD', 'New Password')));
+        $fields->push(PasswordField::create('NewPassword2', _t('SilverStripe\\Security\\Member.CONFIRMNEWPASSWORD', 'Confirm New Password')));
+
+        return $fields;
+    }
+
+    /**
+     * @return FieldList
+     */
+    protected function getFormActions()
+    {
+        $actions = FieldList::create(
+            FormAction::create(
+                'doChangePassword',
+                _t('SilverStripe\\Security\\Member.BUTTONCHANGEPASSWORD', 'Change Password')
+            )
+        );
+
+        return $actions;
     }
 }

src/Security/MemberAuthenticator/ChangePasswordHandler.php

@@ -3,34 +3,226 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
+use SilverStripe\Control\Controller;
+use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
-use SilverStripe\Forms\FormRequestHandler;
+use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\ORM\FieldType\DBField;
+use SilverStripe\ORM\FieldType\DBHTMLText;
+use SilverStripe\Security\Authenticator;
+use SilverStripe\Security\CMSSecurity;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\IdentityStore;
 
-class ChangePasswordHandler extends FormRequestHandler
+class ChangePasswordHandler extends RequestHandler
 {
+    /**
+     * @var Authenticator
+     */
+    protected $authenticator;
+
+    /**
+     * @var string
+     */
+    protected $link;
+
+    /**
+     * @var array Allowed Actions
+     */
+    private static $allowed_actions = [
+        'changepassword',
+        'changePasswordForm',
+    ];
+
+    /**
+     * @var array URL Handlers. All should point to changepassword
+     */
+    private static $url_handlers = [
+        '' => 'changepassword',
+    ];
+
+    /**
+     * @param string $link The URL to recreate this request handler
+     * @param MemberAuthenticator $authenticator
+     */
+    public function __construct($link, MemberAuthenticator $authenticator)
+    {
+        $this->link = $link;
+        $this->authenticator = $authenticator;
+        parent::__construct();
+    }
+
+    /**
+     * Handle the change password request
+     * @todo this could use some spring cleaning
+     *
+     * @return HTTPResponse|DBHTMLText
+     */
+    public function changepassword()
+    {
+        $request = $this->getRequest();
+
+        // Extract the member from the URL.
+        /** @var Member $member */
+        $member = null;
+        if ($request->getVar('m') !== null) {
+            $member = Member::get()->filter(['ID' => (int)$request->getVar('m')])->first();
+        }
+        $token = $request->getVar('t');
+
+        // Check whether we are merely changin password, or resetting.
+        if ($token !== null && $member && $member->validateAutoLoginToken($token)) {
+            $this->setSessionToken($member, $token);
+
+            // Redirect to myself, but without the hash in the URL
+            return $this->redirect($this->link);
+        }
+
+        if (Session::get('AutoLoginHash')) {
+            $message = DBField::create_field(
+                'HTMLFragment',
+                '<p>' . _t(
+                    'SilverStripe\\Security\\Security.ENTERNEWPASSWORD',
+                    'Please enter a new password.'
+                ) . '</p>'
+            );
+
+            // Subsequent request after the "first load with hash" (see previous if clause).
+            return $this->buildResponse($message);
+        }
+
+        if (Security::getCurrentUser()) {
+            // Logged in user requested a password change form.
+            $message = DBField::create_field(
+                'HTMLFragment',
+                '<p>' . _t(
+                    'SilverStripe\\Security\\Security.CHANGEPASSWORDBELOW',
+                    'You can change your password below.'
+                ) . '</p>'
+            );
+
+            return $this->buildResponse($message);
+        }
+        // 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')
+                            ]
+                        )
+                    )
+                )
+            );
+
+            return $customisedController->renderWith('changepassword');
+        }
+
+        // Someone attempted to go to changepassword without token or being logged in
+        return Security::permissionFailure(
+            Controller::curr(),
+            _t(
+                'SilverStripe\\Security\\Security.ERRORPASSWORDPERMISSION',
+                'You must be logged in in order to change your password!'
+            )
+        );
+    }
+
+    /**
+     * @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
+     * @param string $token
+     */
+    protected function setSessionToken($member, $token)
+    {
+        // if there is a current member, they should be logged out
+        if ($curMember = Security::getCurrentUser()) {
+            /** @var LogoutHandler $handler */
+            Injector::inst()->get(IdentityStore::class)->logOut();
+        }
+
+        // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm.
+        Session::set('AutoLoginHash', $member->encryptWithUserSettings($token));
+    }
+
+    /**
+     * Return a link to this request handler.
+     * The link returned is supplied in the constructor
+     * @param null $action
+     * @return string
+     */
+    public function link($action = null)
+    {
+        if ($action) {
+            return Controller::join_links($this->link, $action);
+        }
+
+        return $this->link;
+    }
+
+    /**
+     * Factory method for the lost password form
+     *
+     * @skipUpgrade
+     * @return ChangePasswordForm Returns the lost password form
+     */
+    public function changePasswordForm()
+    {
+        return ChangePasswordForm::create(
+            $this,
+            'ChangePasswordForm'
+        );
+    }
+
     /**
      * Change the password
      *
      * @param array $data The user submitted data
      * @return HTTPResponse
      */
-    public function doChangePassword(array $data, $form)
+    public function doChangePassword(array $data)
     {
         $member = Security::getCurrentUser();
         // The user was logged in, check the current password
         if ($member && (
                 empty($data['OldPassword']) ||
                 !$member->checkPassword($data['OldPassword'])->isValid()
-        )) {
+            )
+        ) {
             $this->form->sessionMessage(
-                _t('SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"),
+                _t(
+                    'SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH',
+                    "Your current password does not match, please try again"
+                ),
                 "bad"
             );
+
             // redirect back to the form, instead of using redirectBack() which could send the user elsewhere.
             return $this->redirectBackToForm();
         }
@@ -43,6 +235,7 @@ class ChangePasswordHandler extends FormRequestHandler
             // The user is not logged in and no valid auto login hash is available
             if (!$member) {
                 Session::clear('AutoLoginHash');
+
                 return $this->redirect($this->addBackURLParam(Security::singleton()->Link('login')));
             }
         }
@@ -50,7 +243,10 @@ class ChangePasswordHandler extends FormRequestHandler
         // Check the new password
         if (empty($data['NewPassword1'])) {
             $this->form->sessionMessage(
-                _t('SilverStripe\\Security\\Member.EMPTYNEWPASSWORD', "The new password can't be empty, please try again"),
+                _t(
+                    'SilverStripe\\Security\\Member.EMPTYNEWPASSWORD',
+                    "The new password can't be empty, please try again"
+                ),
                 "bad"
             );
 
@@ -61,9 +257,13 @@ class ChangePasswordHandler extends FormRequestHandler
         // Fail if passwords do not match
         if ($data['NewPassword1'] !== $data['NewPassword2']) {
             $this->form->sessionMessage(
-                _t('SilverStripe\\Security\\Member.ERRORNEWPASSWORD', "You have entered your new password differently, try again"),
+                _t(
+                    'SilverStripe\\Security\\Member.ERRORNEWPASSWORD',
+                    "You have entered your new password differently, try again"
+                ),
                 "bad"
             );
+
             // redirect back to the form, instead of using redirectBack() which could send the user elsewhere.
             return $this->redirectBackToForm();
         }
@@ -72,17 +272,20 @@ class ChangePasswordHandler extends FormRequestHandler
         $validationResult = $member->changePassword($data['NewPassword1']);
         if (!$validationResult->isValid()) {
             $this->form->setSessionValidationResult($validationResult);
+
             return $this->redirectBackToForm();
         }
 
         // Clear locked out status
         $member->LockedOutUntil = null;
         $member->FailedLoginCount = null;
+        // Clear the members login hashes
+        $member->AutoLoginHash = null;
+        $member->AutoLoginExpired = DBDatetime::create()->now();
         $member->write();
 
         if ($member->canLogIn()->isValid()) {
-            Injector::inst()->get(IdentityStore::class)
+            Injector::inst()->get(IdentityStore::class)->logIn($member, false, $this->getRequest());
-                ->logIn($member, false, $form->getRequestHandler()->getRequest());
         }
 
         // TODO Add confirmation message to login redirect
@@ -96,6 +299,7 @@ class ChangePasswordHandler extends FormRequestHandler
 
         // Redirect to default location - the login form saying "You are logged in as..."
         $url = Security::singleton()->Link('login');
+
         return $this->redirect($url);
     }
 
@@ -103,6 +307,7 @@ class ChangePasswordHandler extends FormRequestHandler
     {
         // Redirect back to form
         $url = $this->addBackURLParam(CMSSecurity::singleton()->Link('changepassword'));
+
         return $this->redirect($url);
     }
 }

src/Security/MemberAuthenticator/CookieAuthenticationHandler.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
+use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Security\Member;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface;
@@ -30,7 +31,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
     /**
      * @var IdentityStore
      */
-    private $cascadeLogInTo;
+    private $cascadeInTo;
 
     /**
      * Get the name of the cookie used to track this device
@@ -45,7 +46,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
     /**
      * Set the name of the cookie used to track this device
      *
-     * @param $deviceCookieName
+     * @param string $deviceCookieName
      * @return null
      */
     public function setDeviceCookieName($deviceCookieName)
@@ -66,8 +67,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
     /**
      * Set the name of the cookie used to store an login token
      *
-     * @param $tokenCookieName
+     * @param string $tokenCookieName
-     * @return null
      */
     public function setTokenCookieName($tokenCookieName)
     {
@@ -81,22 +81,23 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
      */
     public function getCascadeLogInTo()
     {
-        return $this->cascadeLogInTo;
+        return $this->cascadeInTo;
     }
 
     /**
      * Set the name of the cookie used to store an login token
      *
-     * @param $cascadeLogInTo
+     * @param IdentityStore $cascadeInTo
      * @return null
      */
-    public function setCascadeLogInTo(IdentityStore $cascadeLogInTo)
+    public function setCascadeLogInTo(IdentityStore $cascadeInTo)
     {
-        $this->cascadeLogInTo = $cascadeLogInTo;
+        $this->cascadeInTo = $cascadeInTo;
     }
 
     /**
-     * @inherit
+     * @param HTTPRequest $request
+     * @return null|Member
      */
     public function authenticateRequest(HTTPRequest $request)
     {
@@ -104,14 +105,14 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
         $deviceID = Cookie::get($this->getDeviceCookieName());
 
         // @todo Consider better placement of database_is_ready test
-        if (!$deviceID || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) {
+        if ($deviceID === null || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) {
-            return;
+            return null;
         }
 
         list($uid, $token) = explode(':', $uidAndToken, 2);
 
         if (!$uid || !$token) {
-            return;
+            return null;
         }
 
         /** @var Member $member */
@@ -144,11 +145,10 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
         }
 
         if ($member) {
-            if ($this->cascadeLogInTo) {
+            if ($this->cascadeInTo) {
                 // @todo look at how to block "regular login" triggers from happening here
                 // @todo deal with the fact that the Session::current_session() isn't correct here :-/
-                $this->cascadeLogInTo->logIn($member, false, $request);
+                $this->cascadeInTo->logIn($member, false, $request);
-                //\SilverStripe\Dev\Debug::message('here');
             }
 
             // @todo Consider whether response should be part of logIn() as well
@@ -168,20 +168,21 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
                 );
             }
 
-            return $member;
-
             // Audit logging hook
             $member->extend('memberAutoLoggedIn');
+
+            return $member;
         }
     }
 
     /**
-     * @inherit
+     * @param Member $member
+     * @param bool $persistent
+     * @param HTTPRequest $request
+     * @return HTTPResponse|void
      */
-    public function logIn(Member $member, $persistent, HTTPRequest $request)
+    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
     {
-        // @todo couple the cookies to the response object
-
         // Cleans up any potential previous hash for this member on this device
         if ($alcDevice = Cookie::get($this->getDeviceCookieName())) {
             RememberLoginHash::get()->filter('DeviceID', $alcDevice)->removeAll();
@@ -210,29 +211,39 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
                 null,
                 true
             );
-
-        // Clear a cookie for non-persistent log-ins
         } else {
-            $this->logOut($request);
+            // Clear a cookie for non-persistent log-ins
+            $this->clearCookies();
         }
     }
 
     /**
-     * @inherit
+     * @param HTTPRequest|null $request
+     * @return HTTPResponse|void
      */
-    public function logOut(HTTPRequest $request)
+    public function logOut(HTTPRequest $request = null)
     {
         $member = Security::getCurrentUser();
         if ($member) {
             RememberLoginHash::clear($member, Cookie::get('alc_device'));
         }
-        // @todo couple the cookies to the response object
+        $this->clearCookies();
 
+        if ($this->cascadeInTo) {
+            $this->cascadeInTo->logOut($request);
+        }
+
+        Security::setCurrentUser(null);
+    }
+
+    /**
+     * Clear the cookies set for the user
+     */
+    protected function clearCookies()
+    {
         Cookie::set($this->getTokenCookieName(), null);
         Cookie::set($this->getDeviceCookieName(), null);
         Cookie::force_expiry($this->getTokenCookieName());
         Cookie::force_expiry($this->getDeviceCookieName());
-
-        Security::setCurrentUser(null);
     }
 }

src/Security/MemberAuthenticator/LoginHandler.php

@@ -8,6 +8,7 @@ use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\Member;
 use SilverStripe\Core\Injector\Injector;
@@ -23,14 +24,14 @@ class LoginHandler extends RequestHandler
      */
     protected $authenticator;
 
+    /**
+     * @var array
+     */
     private static $url_handlers = [
         '' => 'login',
     ];
 
     /**
-     * Since the logout and dologin actions may be conditionally removed, it's necessary to ensure these
-     * remain valid actions regardless of the member login state.
-     *
      * @var array
      * @config
      */
@@ -40,23 +41,26 @@ class LoginHandler extends RequestHandler
         'logout',
     ];
 
+    /**
+     * @var string Called link on this handler
+     */
     private $link;
 
     /**
      * @param string $link The URL to recreate this request handler
-     * @param Authenticator $authenticator The
+     * @param MemberAuthenticator $authenticator The authenticator to use
      */
-    public function __construct($link, Authenticator $authenticator)
+    public function __construct($link, MemberAuthenticator $authenticator)
     {
         $this->link = $link;
         $this->authenticator = $authenticator;
-        parent::__construct($link, $this);
+        parent::__construct();
     }
 
     /**
      * Return a link to this request handler.
      * The link returned is supplied in the constructor
-     * @param null $action
+     * @param null|string $action
      * @return string
      */
     public function link($action = null)
@@ -70,6 +74,8 @@ class LoginHandler extends RequestHandler
 
     /**
      * URL handler for the log-in screen
+     *
+     * @return array
      */
     public function login()
     {
@@ -80,10 +86,12 @@ class LoginHandler extends RequestHandler
 
     /**
      * Return the MemberLoginForm form
+     *
+     * @return MemberLoginForm
      */
     public function loginForm()
     {
-        return LoginForm::create(
+        return MemberLoginForm::create(
             $this,
             get_class($this->authenticator),
             'LoginForm'
@@ -96,28 +104,41 @@ class LoginHandler extends RequestHandler
      * This method is called when the user finishes the login flow
      *
      * @param array $data Submitted data
-     * @param LoginForm $form
+     * @param MemberLoginForm $form
      * @return HTTPResponse
      */
     public function doLogin($data, $form)
     {
         $failureMessage = null;
 
+        $this->extend('beforeLogin');
         // Successful login
-        if ($member = $this->checkLogin($data, $failureMessage)) {
+        if ($member = $this->checkLogin($data, $result)) {
             $this->performLogin($member, $data, $form->getRequestHandler()->getRequest());
+            // Allow operations on the member after successful login
+            $this->extend('afterLogin', $member);
 
             return $this->redirectAfterSuccessfulLogin();
         }
 
-        $form->sessionMessage($failureMessage, 'bad');
+        $this->extend('failedLogin');
+
+        $message = implode("; ", array_map(
+            function ($message) {
+                return $message['message'];
+            },
+            $result->getMessages()
+        ));
+
+        $form->sessionMessage($message, 'bad');
 
         // Failed login
 
         /** @skipUpgrade */
         if (array_key_exists('Email', $data)) {
+            $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled') === true);
             Session::set('SessionForms.MemberLoginForm.Email', $data['Email']);
-            Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember']));
+            Session::set('SessionForms.MemberLoginForm.Remember', $rememberMe);
         }
 
         // Fail to login redirects back to form
@@ -173,7 +194,7 @@ class LoginHandler extends RequestHandler
                 'Welcome Back, {firstname}',
                 ['firstname' => $member->FirstName]
             );
-            Security::setLoginMessage($message, ValidationResult::TYPE_GOOD);
+            Security::singleton()->setLoginMessage($message, ValidationResult::TYPE_GOOD);
         }
 
         // Redirect back
@@ -184,19 +205,16 @@ class LoginHandler extends RequestHandler
      * Try to authenticate the user
      *
      * @param array $data Submitted data
-     * @param string $message
+     * @param ValidationResult $result
      * @return Member Returns the member object on successful authentication
      *                or NULL on failure.
      */
-    public function checkLogin($data, &$message)
+    public function checkLogin($data, &$result)
     {
-        $message = null;
+        $member = $this->authenticator->authenticate($data, $result);
-        $member = $this->authenticator->authenticate($data, $message);
+        if ($member instanceof Member) {
-        if ($member) {
             return $member;
         }
-        // No member, can't login
-        $this->extend('authenticationFailed', $data);
 
         return null;
     }
@@ -212,8 +230,9 @@ class LoginHandler extends RequestHandler
      */
     public function performLogin($member, $data, $request)
     {
-        // @todo pass request/response
+        /** IdentityStore */
-        Injector::inst()->get(IdentityStore::class)->logIn($member, !empty($data['Remember']), $request);
+        $rememberMe = (isset($data['Remember']) && Security::config()->get('autologin_enabled'));
+        Injector::inst()->get(IdentityStore::class)->logIn($member, $rememberMe, $request);
 
         return $member;
     }
@@ -235,20 +254,4 @@ class LoginHandler extends RequestHandler
 
         return $this->redirect($this->addBackURLParam($changedPasswordLink));
     }
-
-
-    /**
-     * @todo copypaste from FormRequestHandler - refactor
-     * @param string $link
-     * @return string
-     */
-    protected function addBackURLParam($link)
-    {
-        $backURL = $this->getBackURL();
-        if ($backURL) {
-            return Controller::join_links($link, '?BackURL=' . urlencode($backURL));
-        }
-
-        return $link;
-    }
 }

src/Security/MemberAuthenticator/LogoutHandler.php

@@ -16,7 +16,6 @@ use SilverStripe\Security\Security;
  * The logout process destroys all traces of the member on the server (not the actual computer user
  * at the other end of the line, don't worry)
  *
- * @package SilverStripe\Security\MemberAuthenticator
  */
 class LogoutHandler extends RequestHandler
 {

src/Security/MemberAuthenticator/LostPasswordHandler.php

@@ -7,10 +7,13 @@ use SilverStripe\Control\Email\Email;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Forms\Form;
 use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\EmailField;
 use SilverStripe\Forms\FormAction;
+use SilverStripe\Security\IdentityStore;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
 use SilverStripe\Core\Convert;
@@ -21,8 +24,15 @@ use SilverStripe\ORM\FieldType\DBField;
  */
 class LostPasswordHandler extends RequestHandler
 {
+    /**
+     * Authentication class to use
+     * @var string
+     */
     protected $authenticatorClass = MemberAuthenticator::class;
 
+    /**
+     * @var array
+     */
     private static $url_handlers = [
         'passwordsent/$EmailAddress' => 'passwordsent',
         ''                           => 'lostpassword',
@@ -44,7 +54,7 @@ class LostPasswordHandler extends RequestHandler
     private $link = null;
 
     /**
-     * @param $link The URL to recreate this request handler
+     * @param string $link The URL to recreate this request handler
      */
     public function __construct($link)
     {
@@ -55,25 +65,29 @@ class LostPasswordHandler extends RequestHandler
     /**
      * Return a link to this request handler.
      * The link returned is supplied in the constructor
+     *
+     * @param string $action
      * @return string
      */
     public function link($action = null)
     {
         if ($action) {
             return Controller::join_links($this->link, $action);
-        } else {
-            return $this->link;
         }
+
+        return $this->link;
     }
 
     /**
      * URL handler for the initial lost-password screen
+     *
+     * @return array
      */
     public function lostpassword()
     {
 
         $message = _t(
-            'Security.NOTERESETPASSWORD',
+            'SilverStripe\\Security\\Security.NOTERESETPASSWORD',
             'Enter your e-mail address and we will send you a link with which you can reset your password'
         );
 
@@ -86,6 +100,8 @@ class LostPasswordHandler extends RequestHandler
     /**
      * Show the "password sent" page, after a user has requested
      * to reset their password.
+     *
+     * @return array
      */
     public function passwordsent()
     {
@@ -93,15 +109,15 @@ class LostPasswordHandler extends RequestHandler
         $email = Convert::raw2xml(rawurldecode($request->param('EmailAddress')) . '.' . $request->getExtension());
 
         $message = _t(
-            'Security.PASSWORDSENTTEXT',
+            'SilverStripe\\Security\\Security.PASSWORDSENTTEXT',
             "Thank you! A reset link has been sent to '{email}', provided an account exists for this email"
             . " address.",
-            [ 'email' => Convert::raw2xml($email) ]
+            ['email' => Convert::raw2xml($email)]
         );
 
         return [
             'Title'   => _t(
-                'Security.PASSWORDSENTHEADER',
+                'SilverStripe\\Security\\Security.PASSWORDSENTHEADER',
                 "Password reset link sent to '{email}'",
                 array('email' => $email)
             ),
@@ -119,17 +135,17 @@ class LostPasswordHandler extends RequestHandler
      */
     public function lostPasswordForm()
     {
-        return LoginForm::create(
+        return MemberLoginForm::create(
             $this,
             $this->authenticatorClass,
-            'LostPasswordForm',
+            'lostPasswordForm',
             new FieldList(
-                new EmailField('Email', _t('Member.EMAIL', 'Email'))
+                new EmailField('Email', _t('SilverStripe\\Security\\Member.EMAIL', 'Email'))
             ),
             new FieldList(
                 new FormAction(
                     'forgotPassword',
-                    _t('Security.BUTTONSEND', 'Send me the password reset link')
+                    _t('SilverStripe\\Security\\Security.BUTTONSEND', 'Send me the password reset link')
                 )
             ),
             false
@@ -144,12 +160,8 @@ class LostPasswordHandler extends RequestHandler
     public function redirectToLostPassword()
     {
         $lostPasswordLink = Security::singleton()->Link('lostpassword');
-        return $this->redirect($this->addBackURLParam($lostPasswordLink));
-    }
 
-    public function getReturnReferer()
+        return $this->redirect($this->addBackURLParam($lostPasswordLink));
-    {
-        return $this->link();
     }
 
     /**
@@ -167,29 +179,6 @@ class LostPasswordHandler extends RequestHandler
         return Security::singleton()->logout();
     }
 
-    /**
-     * Try to authenticate the user
-     *
-     * @param array $data Submitted data
-     * @return Member Returns the member object on successful authentication
-     *                or NULL on failure.
-     */
-    public function performLogin($data)
-    {
-        $member = call_user_func_array(
-            [$this->authenticator_class, 'authenticate'],
-            [$data, $this->form]
-        );
-        if ($member) {
-            $member->LogIn(isset($data['Remember']));
-            return $member;
-        }
-
-        // No member, can't login
-        $this->extend('authenticationFailed', $data);
-        return null;
-    }
-
     /**
      * Forgot password form handler method.
      * Called when the user clicks on "I've lost my password".
@@ -207,15 +196,20 @@ class LostPasswordHandler extends RequestHandler
         // Ensure password is given
         if (empty($data['Email'])) {
             $this->form->sessionMessage(
-                _t('Member.ENTEREMAIL', 'Please enter an email address to get a password reset link.'),
+                _t(
+                    'SilverStripe\\Security\\Member.ENTEREMAIL',
+                    'Please enter an email address to get a password reset link.'
+                ),
                 'bad'
             );
+
             return $this->redirectToLostPassword();
         }
 
         // Find existing member
+        $field = Member::config()->get('unique_identifier_field');
         /** @var Member $member */
-        $member = Member::get()->filter("Email", $data['Email'])->first();
+        $member = Member::get()->filter([$field => $data['Email']])->first();
 
         // Allow vetoing forgot password requests
         $results = $this->extend('forgotPassword', $member);
@@ -229,7 +223,11 @@ class LostPasswordHandler extends RequestHandler
             Email::create()
                 ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail')
                 ->setData($member)
-                ->setSubject(_t('Member.SUBJECTPASSWORDRESET', "Your password reset link", 'Email subject'))
+                ->setSubject(_t(
+                    'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET',
+                    "Your password reset link",
+                    'Email subject'
+                ))
                 ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token))
                 ->setTo($member->Email)
                 ->send();
@@ -242,18 +240,7 @@ class LostPasswordHandler extends RequestHandler
             rawurlencode($data['Email']),
             '/'
         );
+
         return $this->redirect($this->addBackURLParam($link));
     }
-
-    /**
-     * @todo copypaste from FormRequestHandler - refactor
-     */
-    protected function addBackURLParam($link)
-    {
-        $backURL = $this->getBackURL();
-        if ($backURL) {
-            return Controller::join_links($link, '?BackURL=' . urlencode($backURL));
-        }
-        return $link;
-    }
 }

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -6,7 +6,7 @@ use SilverStripe\Control\Controller;
 use SilverStripe\Control\Session;
 use SilverStripe\ORM\ValidationResult;
 use InvalidArgumentException;
-use SilverStripe\Security\Authenticator as BaseAuthenticator;
+use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\LoginAttempt;
@@ -14,62 +14,68 @@ use SilverStripe\Security\LoginAttempt;
 /**
  * Authenticator for the default "member" method
  *
- * @author Markus Lanthaler <markus@silverstripe.com>
+ * @author Sam Minnee <sam@silverstripe.com>
+ * @author Simon Erkelens <simonerkelens@silverstripe.com>
  */
-class Authenticator implements BaseAuthenticator
+class MemberAuthenticator implements Authenticator
 {
 
     public function supportedServices()
     {
-        // Bitwise-OR of all the supported services, to make a bitmask
+        // Bitwise-OR of all the supported services in this Authenticator, to make a bitmask
-        return BaseAuthenticator::LOGIN | BaseAuthenticator::LOGOUT | BaseAuthenticator::CHANGE_PASSWORD
+        return Authenticator::LOGIN | Authenticator::LOGOUT | Authenticator::CHANGE_PASSWORD
-            | BaseAuthenticator::RESET_PASSWORD;
+            | Authenticator::RESET_PASSWORD;
     }
 
     /**
-     * @inherit
+     * @param array $data
+     * @param null|ValidationResult $result
+     * @return null|Member
      */
-    public function authenticate($data, &$message)
+    public function authenticate($data, &$result = null)
     {
-        $success = null;
-
         // Find authenticated member
-        $member = $this->authenticateMember($data, $message, $success);
+        $member = $this->authenticateMember($data, $result);
 
         // Optionally record every login attempt as a {@link LoginAttempt} object
-        $this->recordLoginAttempt($data, $member, $success);
+        $this->recordLoginAttempt($data, $member, $result->isValid());
 
         if ($member) {
             Session::clear('BackURL');
         }
 
-        return $success ? $member : null;
+        return $result->isValid() ? $member : null;
     }
 
     /**
      * Attempt to find and authenticate member if possible from the given data
      *
      * @param array $data Form submitted data
-     * @param $message
+     * @param ValidationResult $result
-     * @param bool &$success Success flag
+     * @param Member|null This third parameter is used in the CMSAuthenticator(s)
-     * @param null|Member $member If the parent method already identified the member, it can be passed in
      * @return Member Found member, regardless of successful login
      */
-    protected function authenticateMember($data, &$message, &$success, $member = null)
+    protected function authenticateMember($data, &$result = null, $member = null)
     {
         // Default success to false
-        $success = false;
+        $email = !empty($data['Email']) ? $data['Email'] : null;
-        $email = !empty($data['Email']) ? $data['Email'] : null ;
+        $result = new ValidationResult();
 
         // Check default login (see Security::setDefaultAdmin())
         $asDefaultAdmin = $email === Security::default_admin_username();
         if ($asDefaultAdmin) {
             // If logging is as default admin, ensure record is setup correctly
             $member = Member::default_admin();
-            $success = $member->canLogin()->isValid() && Security::check_default_admin($email, $data['Password']);
+            $success = Security::check_default_admin($email, $data['Password']);
+            $result = $member->canLogIn();
             //protect against failed login
-            if ($success) {
+            if ($success && $result->isValid()) {
                 return $member;
+            } else {
+                $result->addError(_t(
+                    'SilverStripe\\Security\\Member.ERRORWRONGCRED',
+                    "The provided details don't seem to be correct. Please try again."
+                ));
             }
         }
 
@@ -85,28 +91,23 @@ class Authenticator implements BaseAuthenticator
         // Validate against member if possible
         if ($member && !$asDefaultAdmin) {
             $result = $member->checkPassword($data['Password']);
-            $success = $result->isValid();
-        } else {
-            $result = ValidationResult::create()->addError(_t(
-                'SilverStripe\\Security\\Member.ERRORWRONGCRED',
-                'The provided details don\'t seem to be correct. Please try again.'
-            ));
         }
 
         // Emit failure to member and form (if available)
-        if (!$success) {
+        if (!$result->isValid()) {
             if ($member) {
                 $member->registerFailedLogin();
             }
-            $message = implode("; ", array_map(
-                function ($message) {
-                    return $message['message'];
-                },
-                $result->getMessages()
-            ));
         } else {
-            if ($member) { // How can success be true and member false?
+            if ($member) {
                 $member->registerSuccessfulLogin();
+            } else {
+                // A non-existing member occurred. This will make the result "valid" so let's invalidate
+                $result->addError(_t(
+                    'SilverStripe\\Security\\Member.ERRORWRONGCRED',
+                    "The provided details don't seem to be correct. Please try again."
+                ));
+                $member = null;
             }
         }
 
@@ -119,6 +120,7 @@ class Authenticator implements BaseAuthenticator
      *
      * @param array $data
      * @param Member $member
+     * @param boolean $success
      */
     protected function recordLoginAttempt($data, $member, $success)
     {
@@ -134,7 +136,7 @@ class Authenticator implements BaseAuthenticator
         }
 
         $attempt = LoginAttempt::create();
-        if ($success) {
+        if ($success && $member) {
             // successful login (member is existing with matching password)
             $attempt->MemberID = $member->ID;
             $attempt->Status = 'Success';
@@ -160,7 +162,8 @@ class Authenticator implements BaseAuthenticator
     }
 
     /**
-     * @inherit
+     * @param $link
+     * @return LostPasswordHandler
      */
     public function getLostPasswordHandler($link)
     {
@@ -168,7 +171,8 @@ class Authenticator implements BaseAuthenticator
     }
 
     /**
-     * @inherit
+     * @param string $link
+     * @return ChangePasswordHandler
      */
     public function getChangePasswordHandler($link)
     {
@@ -176,7 +180,8 @@ class Authenticator implements BaseAuthenticator
     }
 
     /**
-     * @inherit
+     * @param string $link
+     * @return LoginHandler
      */
     public function getLoginHandler($link)
     {
@@ -184,7 +189,8 @@ class Authenticator implements BaseAuthenticator
     }
 
     /**
-     * @inherit
+     * @param string $link
+     * @return LogoutHandler
      */
     public function getLogoutHandler($link)
     {

src/Security/MemberAuthenticator/MemberLoginForm.php

@@ -31,7 +31,7 @@ use SilverStripe\View\Requirements;
  *    allowing extensions to "veto" execution by returning FALSE.
  *    Arguments: $member containing the detected Member record
  */
-class LoginForm extends BaseLoginForm
+class MemberLoginForm extends BaseLoginForm
 {
 
     /**
@@ -87,7 +87,7 @@ class LoginForm extends BaseLoginForm
             $backURL = Session::get('BackURL');
         }
 
-        if ($checkCurrentUser && Security::getCurrentUser() && Member::logged_in_session_exists()) {
+        if ($checkCurrentUser && Security::getCurrentUser()) {
             // @todo find a more elegant way to handle this
             $logoutAction = Security::logout_url();
             $fields = FieldList::create(
@@ -145,7 +145,7 @@ class LoginForm extends BaseLoginForm
             $this->setAttribute('autocomplete', 'off');
             $emailField->setAttribute('autocomplete', 'off');
         }
-        if (Security::config()->autologin_enabled) {
+        if (Security::config()->get('autologin_enabled')) {
             $fields->push(
                 CheckboxField::create(
                     "Remember",

src/Security/MemberAuthenticator/SessionAuthenticationHandler.php

@@ -3,22 +3,24 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Cookie;
+use SilverStripe\Control\HTTPResponse;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Security\Member;
 use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\Director;
-use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface;
+use SilverStripe\Security\AuthenticationHandler;
-use SilverStripe\ORM\ValidationException;
 use SilverStripe\Security\IdentityStore;
 
 /**
  * Authenticate a member pased on a session cookie
  */
-class SessionAuthenticationHandler implements AuthenticationHandlerInterface, IdentityStore
+class SessionAuthenticationHandler implements AuthenticationHandler, IdentityStore
 {
 
+    /**
+     * @var string
+     */
     private $sessionVariable;
 
     /**
@@ -35,7 +37,6 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id
      * Set the session variable name used to track member ID
      *
      * @param string $sessionVariable
-     * @return null
      */
     public function setSessionVariable($sessionVariable)
     {
@@ -44,12 +45,11 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id
 
     /**
      * @inherit
+     * @param HTTPRequest $request
+     * @return null|DataObject|Member
      */
     public function authenticateRequest(HTTPRequest $request)
     {
-        // @todo couple the session to a request object
-        // $session = $request->getSession();
-
         if ($id = Session::get($this->getSessionVariable())) {
             // If ID is a bad ID it will be treated as if the user is not logged in, rather than throwing a
             // ValidationException
@@ -61,17 +61,18 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id
 
     /**
      * @inherit
+     * @param Member $member
+     * @param bool $persistent
+     * @param HTTPRequest|null $request
+     * @return HTTPResponse|void
      */
-    public function logIn(Member $member, $persistent, HTTPRequest $request)
+    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
     {
-        // @todo couple the session to a request object
-        // $session = $request->getSession();
-
         static::regenerateSessionId();
         Session::set($this->getSessionVariable(), $member->ID);
 
         // This lets apache rules detect whether the user has logged in
-        // @todo make this a settign on the authentication handler
+        // @todo make this a setting on the authentication handler
         if (Member::config()->get('login_marker_cookie')) {
             Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0);
         }
@@ -82,7 +83,7 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id
      */
     protected static function regenerateSessionId()
     {
-        if (!Member::config()->session_regenerate_id) {
+        if (!Member::config()->get('session_regenerate_id')) {
             return;
         }
 
@@ -100,14 +101,13 @@ class SessionAuthenticationHandler implements AuthenticationHandlerInterface, Id
             @session_regenerate_id(true);
         }
     }
-    /**
-     * @inherit
-     */
-    public function logOut(HTTPRequest $request)
-    {
-        // @todo couple the session to a request object
-        // $session = $request->getSession();
 
+    /**
+     * @param HTTPRequest|null $request
+     * @return HTTPResponse|void
+     */
+    public function logOut(HTTPRequest $request = null)
+    {
         Session::clear($this->getSessionVariable());
     }
 }

src/Security/Permission.php

@@ -4,6 +4,7 @@ namespace SilverStripe\Security;
 
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Resettable;
+use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\i18n\i18nEntityProvider;
 use SilverStripe\ORM\DB;
@@ -131,10 +132,10 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
     public static function check($code, $arg = "any", $member = null, $strict = true)
     {
         if (!$member) {
-            if (!Member::currentUserID()) {
+            if (!Security::getCurrentUser()) {
                 return false;
             }
-            $member = Member::currentUserID();
+            $member = Security::getCurrentUser();
         }
 
         return self::checkMember($member, $code, $arg, $strict);
@@ -171,10 +172,9 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
     public static function checkMember($member, $code, $arg = "any", $strict = true)
     {
         if (!$member) {
-            $memberID = $member = Member::currentUserID();
+            $member = Security::getCurrentUser();
-        } else {
-            $memberID = (is_object($member)) ? $member->ID : $member;
         }
+        $memberID = ($member instanceof Member) ? $member->ID : $member;
 
         if (!$memberID) {
             return false;

src/Security/Security.php

@@ -13,29 +13,21 @@ use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\ClassInfo;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\TestOnly;
-use SilverStripe\Forms\EmailField;
-use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
-use SilverStripe\Forms\FormAction;
 use SilverStripe\ORM\ArrayList;
-use SilverStripe\ORM\Connect\Database;
 use SilverStripe\ORM\DataModel;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBField;
+use SilverStripe\ORM\FieldType\DBHTMLText;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\MemberAuthenticator\LogoutHandler;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\SSViewer;
 use SilverStripe\View\TemplateGlobalProvider;
-use Exception;
-use SilverStripe\View\ViewableData_Customised;
 use Subsite;
-use SilverStripe\Core\Injector\Injector;
 
 /**
  * Implements a basic security model
@@ -52,13 +44,10 @@ class Security extends Controller implements TemplateGlobalProvider
         'passwordsent',
         'changepassword',
         'ping',
-        'LoginForm',
-        'ChangePasswordForm',
-        'LostPasswordForm',
     );
 
     /**
-     * Default user name. Only used in dev-mode by {@link setDefaultAdmin()}
+     * Default user name. {@link setDefaultAdmin()}
      *
      * @var string
      * @see setDefaultAdmin()
@@ -66,7 +55,7 @@ class Security extends Controller implements TemplateGlobalProvider
     protected static $default_username;
 
     /**
-     * Default password. Only used in dev-mode by {@link setDefaultAdmin()}
+     * Default password. {@link setDefaultAdmin()}
      *
      * @var string
      * @see setDefaultAdmin()
@@ -80,7 +69,7 @@ class Security extends Controller implements TemplateGlobalProvider
      * @config
      * @var bool
      */
-    protected static $strict_path_checking = false;
+    private static $strict_path_checking = false;
 
     /**
      * The password encryption algorithm to use by default.
@@ -102,7 +91,7 @@ class Security extends Controller implements TemplateGlobalProvider
 
     /**
      * Determine if login username may be remembered between login sessions
-     * If set to false this will disable autocomplete and prevent username persisting in the session
+     * If set to false this will disable auto-complete and prevent username persisting in the session
      *
      * @config
      * @var bool
@@ -124,7 +113,7 @@ class Security extends Controller implements TemplateGlobalProvider
     private static $template = 'BlankPage';
 
     /**
-     * Template thats used to render the pages.
+     * Template that is used to render the pages.
      *
      * @var string
      * @config
@@ -163,7 +152,7 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @var string
      */
-    private static $login_url = "Security/login";
+    private static $login_url = 'Security/login';
 
     /**
      * The default logout URL
@@ -172,7 +161,7 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @var string
      */
-    private static $logout_url = "Security/logout";
+    private static $logout_url = 'Security/logout';
 
     /**
      * The default lost password URL
@@ -181,7 +170,7 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @var string
      */
-    private static $lost_password_url = "Security/lostpassword";
+    private static $lost_password_url = 'Security/lostpassword';
 
     /**
      * Value of X-Frame-Options header
@@ -212,7 +201,7 @@ class Security extends Controller implements TemplateGlobalProvider
      * @var boolean If set to TRUE or FALSE, {@link database_is_ready()}
      * will always return FALSE. Used for unit testing.
      */
-    protected static $force_database_is_ready = null;
+    protected static $force_database_is_ready;
 
     /**
      * When the database has once been verified as ready, it will not do the
@@ -223,19 +212,30 @@ class Security extends Controller implements TemplateGlobalProvider
     protected static $database_is_ready = false;
 
     /**
-     * @var array available authenticators
+     * @var Authenticator[] available authenticators
      */
-    protected static $authenticators = [];
+    private static $authenticators = [];
-
-    /**
-     * @var string Default authenticator
-     */
-    protected static $default_authenticator = MemberAuthenticator\Authenticator::class;
 
     /**
      * @var Member Currently logged in user (if available)
      */
-    private static $currentUser;
+    protected static $currentUser;
+
+    /**
+     * @return array
+     */
+    public static function getAuthenticators()
+    {
+        return self::$authenticators;
+    }
+
+    /**
+     * @param array|Authenticator $authenticators
+     */
+    public static function setAuthenticators(array $authenticators)
+    {
+        self::$authenticators = $authenticators;
+    }
 
     /**
      * @inheritdoc
@@ -268,18 +268,16 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Get the selected authenticator for this request
      *
-     * @param $name string The identifier of the authenticator in your config
+     * @param string $name The identifier of the authenticator in your config
      * @return Authenticator Class name of Authenticator
      * @throws LogicException
      */
-    protected function getAuthenticator($name)
+    protected function getAuthenticator($name = 'default')
     {
-        $authenticators = self::config()->get('authenticators');
+        $authenticators = static::$authenticators;
-
-        $name = $name ?: 'default';
 
         if (isset($authenticators[$name])) {
-            return Injector::inst()->get($authenticators[$name]);
+            return $authenticators[$name];
         }
 
         throw new LogicException('No valid authenticator found');
@@ -288,18 +286,16 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Get all registered authenticators
      *
+     * @param int $service The type of service that is requested
      * @return array Return an array of Authenticator objects
      */
-    public static function getAuthenticators($service = Authenticator::LOGIN)
+    public function getApplicableAuthenticators($service = Authenticator::LOGIN)
     {
-        $authenticators = self::config()->get('authenticators');
+        $authenticators = static::$authenticators;
 
-        foreach ($authenticators as $name => &$class) {
+        /** @var Authenticator $class */
-            /** @var Authenticator $authenticator */
+        foreach ($authenticators as $name => $class) {
-            $authenticator = Injector::inst()->get($class);
+            if (!($class->supportedServices() & $service)) {
-            if ($authenticator->supportedServices() & $service) {
-                $class = $authenticator;
-            } else {
                 unset($authenticators[$name]);
             }
         }
@@ -314,9 +310,10 @@ class Security extends Controller implements TemplateGlobalProvider
      * @return bool Returns TRUE if the authenticator is registered, FALSE
      *              otherwise.
      */
-    public static function hasAuthenticator($authenticator)
+    public function hasAuthenticator($authenticator)
     {
-        $authenticators = self::config()->get('authenticators');
+        $authenticators = static::$authenticators;
+
         return !empty($authenticators[$authenticator]);
     }
 
@@ -357,13 +354,18 @@ class Security extends Controller implements TemplateGlobalProvider
             $response = ($controller) ? $controller->getResponse() : new HTTPResponse();
             $response->setStatusCode(403);
             if (!static::getCurrentUser()) {
-                $response->setBody(_t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in'));
+                $response->setBody(
-                $response->setStatusDescription(_t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in'));
+                    _t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in')
-                // Tell the CMS to allow re-aunthentication
+                );
+                $response->setStatusDescription(
+                    _t('SilverStripe\\CMS\\Controllers\\ContentController.NOTLOGGEDIN', 'Not logged in')
+                );
+                // Tell the CMS to allow re-authentication
                 if (CMSSecurity::enabled()) {
                     $response->addHeader('X-Reauthenticate', '1');
                 }
             }
+
             return $response;
         }
 
@@ -407,8 +409,8 @@ class Security extends Controller implements TemplateGlobalProvider
                 $message = $messageSet['default'];
             }
 
-            Security::setLoginMessage($message, ValidationResult::TYPE_WARNING);
+            static::singleton()->setLoginMessage($message, ValidationResult::TYPE_WARNING);
-            $loginResponse = (new Security())->login(new HTTPRequest('GET', '/'));
+            $loginResponse = static::singleton()->login();
             if ($loginResponse instanceof HTTPResponse) {
                 return $loginResponse;
             }
@@ -422,7 +424,7 @@ class Security extends Controller implements TemplateGlobalProvider
             $message = $messageSet['default'];
         }
 
-        static::setLoginMessage($message, ValidationResult::TYPE_WARNING);
+        static::singleton()->setLoginMessage($message, ValidationResult::TYPE_WARNING);
 
         Session::set("BackURL", $_SERVER['REQUEST_URI']);
 
@@ -436,11 +438,17 @@ class Security extends Controller implements TemplateGlobalProvider
         ));
     }
 
-    public static function setCurrentUser($currentUser)
+    /**
+     * @param null|Member $currentUser
+     */
+    public static function setCurrentUser($currentUser = null)
     {
         self::$currentUser = $currentUser;
     }
 
+    /**
+     * @return null|Member
+     */
     public static function getCurrentUser()
     {
         return self::$currentUser;
@@ -449,18 +457,21 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Get the login forms for all available authentication methods
      *
+     * @deprecated 5.0.0 Now handled by {@link static::delegateToMultipleHandlers}
+     *
      * @return array Returns an array of available login forms (array of Form
      *               objects).
      *
-     * @todo Check how to activate/deactivate authentication methods
      */
     public function getLoginForms()
     {
+        Deprecation::notice('5.0.0', 'Now handled by delegateToMultipleHandlers');
+
         return array_map(
             function ($authenticator) {
-                return $authenticator->getLoginHandler($this->Link())->handleRequest($this->getRequest(), DataModel::inst());
+                return [$authenticator->getLoginHandler($this->Link())->loginForm()];
             },
-            Security::getAuthenticators()
+            $this->getApplicableAuthenticators()
         );
     }
 
@@ -507,11 +518,16 @@ class Security extends Controller implements TemplateGlobalProvider
         $member = static::getCurrentUser();
 
         if ($member) { // If we don't have a member, there's not much to log out.
-            /** @var Authenticator $authenticator */
+            /** @var array|Authenticator[] $authenticators */
-            $authenticator = $this->getAuthenticator('default'); // Always use the default authenticator to log out
+            $authenticators = $this->getApplicableAuthenticators(Authenticator::LOGOUT);
+
+            /** @var Authenticator[] $authenticator */
+            foreach ($authenticators as $name => $authenticator) {
                 $handler = $authenticator->getLogOutHandler(Controller::join_links($this->Link(), 'logout'));
-            $result = $this->delegateToHandler($handler, 'default', []);
+                $this->delegateToHandler($handler, $name);
-            if ($result !== true) {
+            }
+            // In the rare case, but plausible with e.g. an external IdentityStore, the user is not logged out.
+            if (static::getCurrentUser() !== null) {
                 $this->extend('failureMemberLoggedOut', $authenticator);
 
                 return $this->redirectBack();
@@ -558,10 +574,10 @@ class Security extends Controller implements TemplateGlobalProvider
         // This step is necessary in cases such as automatic redirection where a user is authenticated
         // upon landing on an SSL secured site and is automatically logged in, or some other case
         // where the user has permissions to continue but is not given the option.
-        if ($this->getRequest()->requestVar('BackURL')
+        if (!$this->getLoginMessage()
-            && !$this->getLoginMessage()
             && ($member = static::getCurrentUser())
             && $member->exists()
+            && $this->getRequest()->requestVar('BackURL')
         ) {
             return $this->redirectBack();
         }
@@ -590,20 +606,21 @@ class Security extends Controller implements TemplateGlobalProvider
         /** @skipUpgrade */
         $holderPage->URLSegment = 'Security';
         // Disable ID-based caching  of the log-in page by making it a random number
-        $holderPage->ID = -1 * rand(1, 10000000);
+        $holderPage->ID = -1 * random_int(1, 10000000);
 
         $controllerClass = $holderPage->getControllerName();
         /** @var ContentController $controller */
         $controller = $controllerClass::create($holderPage);
         $controller->setDataModel($this->model);
         $controller->doInit();
+
         return $controller;
     }
 
     /**
      * Combine the given forms into a formset with a tabbed interface
      *
-     * @param $forms
+     * @param array|Form[] $forms
      * @return string
      */
     protected function generateLoginFormSet($forms)
@@ -611,6 +628,7 @@ class Security extends Controller implements TemplateGlobalProvider
         $viewData = new ArrayData(array(
             'Forms' => new ArrayList($forms),
         ));
+
         return $viewData->renderWith(
             $this->getTemplatesFor('MultiAuthenticatorLogin')
         );
@@ -635,6 +653,7 @@ class Security extends Controller implements TemplateGlobalProvider
         if ($messageCast !== ValidationResult::CAST_HTML) {
             $message = Convert::raw2xml($message);
         }
+
         return sprintf('<p class="message %s">%s</p>', Convert::raw2att($messageType), $message);
     }
 
@@ -645,14 +664,14 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $messageType Message type. One of ValidationResult::TYPE_*
      * @param string $messageCast Message cast. One of ValidationResult::CAST_*
      */
-    public static function setLoginMessage(
+    public function setLoginMessage(
         $message,
         $messageType = ValidationResult::TYPE_WARNING,
         $messageCast = ValidationResult::CAST_TEXT
     ) {
-        Session::set("Security.Message.message", $message);
+        Session::set('Security.Message.message', $message);
-        Session::set("Security.Message.type", $messageType);
+        Session::set('Security.Message.type', $messageType);
-        Session::set("Security.Message.cast", $messageCast);
+        Session::set('Security.Message.cast', $messageCast);
     }
 
     /**
@@ -660,7 +679,7 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function clearLoginMessage()
     {
-        Session::clear("Security.Message");
+        Session::clear('Security.Message');
     }
 
 
@@ -670,37 +689,48 @@ class Security extends Controller implements TemplateGlobalProvider
      * For multiple authenticators, Security_MultiAuthenticatorLogin is used.
      * See getTemplatesFor and getIncludeTemplate for how to override template logic
      *
-     * @param $request
+     * @param null|HTTPRequest $request
+     * @param int $service
      * @return HTTPResponse|string Returns the "login" page as HTML code.
      * @throws HTTPResponse_Exception
      */
-    public function login($request, $service = Authenticator::LOGIN)
+    public function login($request = null, $service = Authenticator::LOGIN)
     {
         // Check pre-login process
         if ($response = $this->preLogin()) {
             return $response;
         }
+        $authName = null;
 
-        $link = $this->link("login");
+        if (!$request) {
+            $request = $this->getRequest();
+        }
+
+        if ($request && $request->param('ID')) {
+            $authName = $request->param('ID');
+        }
+
+        $link = $this->Link('login');
 
         // Delegate to a single handler - Security/login/<authname>/...
-        if (($name = $request->param('ID')) && self::hasAuthenticator($request->param('ID'))) {
+        if ($authName && $this->hasAuthenticator($authName)
+        ) {
+            if ($request) {
                 $request->shift();
+            }
+
+            $authenticator = $this->getAuthenticator($authName);
 
-            $authenticator = $this->getAuthenticator($name);
-            // @todo handle different Authenticator situations
             if (!$authenticator->supportedServices() & $service) {
-                throw new HTTPResponse_Exception('Invalid Authenticator "' . $name . '" for login action', 418);
+                throw new HTTPResponse_Exception('Invalid Authenticator "' . $authName . '" for login action', 418);
             }
 
-            $authenticators = [ $name => $authenticator ];
+            $handlers = [$authName => $authenticator];
-
-        // Delegate to all of them, building a tabbed view - Security/login
         } else {
-            $authenticators = static::getAuthenticators($service);
+            // Delegate to all of them, building a tabbed view - Security/login
+            $handlers = $this->getApplicableAuthenticators($service);
         }
 
-        $handlers = $authenticators;
         array_walk(
             $handlers,
             function (&$auth, $name) use ($link) {
@@ -721,9 +751,10 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * If a single handler is passed, delegateToHandler() will be called instead
      *
+     * @param array|RequestHandler[] $handlers
      * @param string $title The title of the form
      * @param array $templates
-     * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string
+     * @return array|HTTPResponse|RequestHandler|DBHTMLText|string
      */
     protected function delegateToMultipleHandlers(array $handlers, $title, array $templates)
     {
@@ -736,7 +767,7 @@ class Security extends Controller implements TemplateGlobalProvider
         // Process each of the handlers
         $results = array_map(
             function ($handler) {
-                return $handler->handleRequest($this->getRequest(), \SilverStripe\ORM\DataModel::inst());
+                return $handler->handleRequest($this->getRequest(), DataModel::inst());
             },
             $handlers
         );
@@ -754,7 +785,7 @@ class Security extends Controller implements TemplateGlobalProvider
         }
 
         if (!$forms) {
-            throw new \LogicException("No authenticators found compatible with a tabbed login");
+            throw new \LogicException('No authenticators found compatible with a tabbed login');
         }
 
         return $this->renderWrappedController(
@@ -770,11 +801,12 @@ class Security extends Controller implements TemplateGlobalProvider
      * Delegate to another RequestHandler, rendering any fragment arrays into an appropriate.
      * controller.
      *
+     * @param RequestHandler $handler
      * @param string $title The title of the form
      * @param array $templates
-     * @return array|HTTPResponse|RequestHandler|\SilverStripe\ORM\FieldType\DBHTMLText|string
+     * @return array|HTTPResponse|RequestHandler|DBHTMLText|string
      */
-    protected function delegateToHandler(RequestHandler $handler, $title, array $templates)
+    protected function delegateToHandler(RequestHandler $handler, $title, array $templates = [])
     {
         $result = $handler->handleRequest($this->getRequest(), DataModel::inst());
 
@@ -792,7 +824,7 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $title string The title to give the security page
      * @param array $fragments A map of objects to render into the page, e.g. "Form"
      * @param array $templates An array of templates to use for the render
-     * @return HTTPResponse|\SilverStripe\ORM\FieldType\DBHTMLText
+     * @return HTTPResponse|DBHTMLText
      */
     protected function renderWrappedController($title, array $fragments, array $templates)
     {
@@ -824,7 +856,7 @@ class Security extends Controller implements TemplateGlobalProvider
 
     public function basicauthlogin()
     {
-        $member = BasicAuth::requireLogin($this->getRequest(), "SilverStripe login", 'ADMIN');
+        $member = BasicAuth::requireLogin($this->getRequest(), 'SilverStripe login', 'ADMIN');
         static::setCurrentUser($member);
     }
 
@@ -835,12 +867,17 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public function lostpassword()
     {
-        $handler = $this->getAuthenticator('default')->getLostPasswordHandler(
+        $handlers = [];
-            Controller::join_links($this->link(), 'lostpassword')
+        $authenticators = $this->getApplicableAuthenticators(Authenticator::RESET_PASSWORD);
+        /** @var Authenticator $authenticator */
+        foreach ($authenticators as $authenticator) {
+            $handlers[] = $authenticator->getLostPasswordHandler(
+                Controller::join_links($this->Link(), 'lostpassword')
             );
+        }
 
-        return $this->delegateToHandler(
+        return $this->delegateToMultipleHandlers(
-            $handler,
+            $handlers,
             _t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password'),
             $this->getTemplatesFor('lostpassword')
         );
@@ -860,79 +897,18 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public function changepassword()
     {
-        $controller = $this->getResponseController(_t('SilverStripe\\Security\\Security.CHANGEPASSWORDHEADER', 'Change your password'));
+        /** @var array|Authenticator[] $authenticators */
-
+        $authenticators = $this->getApplicableAuthenticators(Authenticator::CHANGE_PASSWORD);
-        // if the controller calls Director::redirect(), this will break early
+        $handlers = [];
-        if (($response = $controller->getResponse()) && $response->isFinished()) {
+        foreach ($authenticators as $authenticator) {
-            return $response;
+            $handlers[] = $authenticator->getChangePasswordHandler($this->Link('changepassword'));
         }
 
-        // Extract the member from the URL.
+        return $this->delegateToMultipleHandlers(
-        /** @var Member $member */
+            $handlers,
-        $member = null;
+            _t('SilverStripe\\Security\\Security.CHANGEPASSWORDHEADER', 'Change your password'),
-        if (isset($_REQUEST['m'])) {
+            $this->getTemplatesFor('changepassword')
-            $member = Member::get()->filter('ID', (int)$_REQUEST['m'])->first();
-        }
-
-        // Check whether we are merely changin password, or resetting.
-        if (isset($_REQUEST['t']) && $member && $member->validateAutoLoginToken($_REQUEST['t'])) {
-            // On first valid password reset request redirect to the same URL without hash to avoid referrer leakage.
-
-            // if there is a current member, they should be logged out
-            if ($curMember = static::getCurrentUser()) {
-                /** @var LogoutHandler $handler */
-                $handler = $this->getAuthenticator('default')->getLogoutHandler($this->Link('logout'));
-                $handler->doLogOut($curMember);
-            }
-
-            // Store the hash for the change password form. Will be unset after reload within the ChangePasswordForm.
-            Session::set('AutoLoginHash', $member->encryptWithUserSettings($_REQUEST['t']));
-
-            return $this->redirect($this->Link('changepassword'));
-        } elseif (Session::get('AutoLoginHash')) {
-            // Subsequent request after the "first load with hash" (see previous if clause).
-            $customisedController = $controller->customise(array(
-                'Content' => DBField::create_field(
-                    'HTMLFragment',
-                    '<p>' . _t('SilverStripe\\Security\\Security.ENTERNEWPASSWORD', 'Please enter a new password.') . '</p>'
-                ),
-                'Form' => $this->ChangePasswordForm(),
-            ));
-        } elseif (static::getCurrentUser()) {
-            // Logged in user requested a password change form.
-            $customisedController = $controller->customise(array(
-                'Content' => DBField::create_field(
-                    'HTMLFragment',
-                    '<p>' . _t('SilverStripe\\Security\\Security.CHANGEPASSWORDBELOW', 'You can change your password below.') . '</p>'
-                ),
-                'Form' => $this->ChangePasswordForm()));
-        } else {
-            // Show friendly message if it seems like the user arrived here via password reset feature.
-            if (isset($_REQUEST['m']) || isset($_REQUEST['t'])) {
-                $customisedController = $controller->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')
-                            ]
-                        )
-                    ))
         );
-            } else {
-                return self::permissionFailure(
-                    $this,
-                    _t('SilverStripe\\Security\\Security.ERRORPASSWORDPERMISSION', 'You must be logged in in order to change your password!')
-                );
-            }
-        }
-
-        return $customisedController->renderWith($this->getTemplatesFor('changepassword'));
     }
 
     /**
@@ -949,21 +925,8 @@ class Security extends Controller implements TemplateGlobalProvider
     public static function getPasswordResetLink($member, $autologinToken)
     {
         $autologinToken = urldecode($autologinToken);
-        $selfControllerClass = __CLASS__;
-        /** @var static $selfController */
-        $selfController = new $selfControllerClass();
-        return $selfController->Link('changepassword') . "?m={$member->ID}&t=$autologinToken";
-    }
 
-    /**
+        return static::singleton()->Link('changepassword') . "?m={$member->ID}&t=$autologinToken";
-     * Factory method for the lost password form
-     *
-     * @skipUpgrade
-     * @return MemberAuthenticator\ChangePasswordForm
-     */
-    public function ChangePasswordForm()
-    {
-        return MemberAuthenticator\ChangePasswordForm::create($this, 'ChangePasswordForm');
     }
 
     /**
@@ -976,6 +939,7 @@ class Security extends Controller implements TemplateGlobalProvider
     public function getTemplatesFor($action)
     {
         $templates = SSViewer::get_templates_by_class(static::class, "_{$action}", __CLASS__);
+
         return array_merge(
             $templates,
             [
@@ -1002,12 +966,7 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function findAnAdministrator()
     {
-        // coupling to subsites module
+        static::singleton()->extend('beforeFindAdministrator');
-        $origSubsite = null;
-        if (is_callable('Subsite::changeSubsite')) {
-            $origSubsite = Subsite::currentSubsiteID();
-            Subsite::changeSubsite(0);
-        }
 
         /** @var Member $member */
         $member = null;
@@ -1015,19 +974,13 @@ class Security extends Controller implements TemplateGlobalProvider
         // find a group with ADMIN permission
         $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
 
-        if (is_callable('Subsite::changeSubsite')) {
-            Subsite::changeSubsite($origSubsite);
-        }
-
-        if ($adminGroup) {
-            $member = $adminGroup->Members()->First();
-        }
-
         if (!$adminGroup) {
             Group::singleton()->requireDefaultRecords();
             $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
         }
 
+        $member = $adminGroup->Members()->First();
+
         if (!$member) {
             Member::singleton()->requireDefaultRecords();
             $member = Permission::get_members_by_permission('ADMIN')->first();
@@ -1049,6 +1002,8 @@ class Security extends Controller implements TemplateGlobalProvider
                 ->add($member);
         }
 
+        static::singleton()->extend('afterFindAdministrator');
+
         return $member;
     }
 
@@ -1083,6 +1038,7 @@ class Security extends Controller implements TemplateGlobalProvider
 
         self::$default_username = $username;
         self::$default_password = $password;
+
         return true;
     }
 
@@ -1105,11 +1061,10 @@ class Security extends Controller implements TemplateGlobalProvider
 
     /**
      * Check that the default admin account has been set.
-     * @todo Check if we _actually_ only want this to work on dev
      */
     public static function has_default_admin()
     {
-        return !empty(self::$default_username) && !empty(self::$default_password) && (Director::get_environment_type() === 'dev');
+        return !empty(self::$default_username) && !empty(self::$default_password);
     }
 
     /**
@@ -1251,29 +1206,6 @@ class Security extends Controller implements TemplateGlobalProvider
         self::$force_database_is_ready = $isReady;
     }
 
-    /**
-     * Enable or disable recording of login attempts
-     * through the {@link LoginRecord} object.
-     *
-     * @deprecated 4.0 Use the "Security.login_recording" config setting instead
-     * @param boolean $bool
-     */
-    public static function set_login_recording($bool)
-    {
-        Deprecation::notice('4.0', 'Use the "Security.login_recording" config setting instead');
-        self::$login_recording = (bool)$bool;
-    }
-
-    /**
-     * @deprecated 4.0 Use the "Security.login_recording" config setting instead
-     * @return boolean
-     */
-    public static function login_recording()
-    {
-        Deprecation::notice('4.0', 'Use the "Security.login_recording" config setting instead');
-        return self::$login_recording;
-    }
-
     /**
      * @config
      * @var string Set the default login dest

src/View/ViewableData.php

@@ -312,7 +312,7 @@ class ViewableData implements IteratorAggregate
      */
     public function castingHelper($field)
     {
-        $specs = $this->config()->get('casting');
+        $specs = static::config()->get('casting');
         if (isset($specs[$field])) {
             return $specs[$field];
         }
@@ -329,7 +329,7 @@ class ViewableData implements IteratorAggregate
         }
 
         // Fall back to default_cast
-        $default = self::config()->get('default_cast');
+        $default = $this->config()->get('default_cast');
         if (empty($default)) {
             throw new Exception("No default_cast");
         }

tests/behat/features/login.feature

@@ -6,7 +6,7 @@ Feature: Log in
 
   Scenario: Bad login
     Given I log in with "bad@example.com" and "badpassword"
-    Then I will see a "error" log-in message
+    Then I should see "The provided details don't seem to be correct"
 
   Scenario: Valid login
     Given I am logged in with "ADMIN" permissions

tests/php/Forms/GridField/GridFieldEditButtonTest.php

@@ -64,7 +64,7 @@ class GridFieldEditButtonTest extends SapphireTest
     public function testShowEditLinks()
     {
         if (Security::getCurrentUser()) {
-            Security::getCurrentUser()->logOut();
+            Security::setCurrentUser(null);
         }
 
         $content = new CSSContentParser($this->gridField->FieldHolder());

tests/php/Security/BasicAuthTest.php

@@ -87,7 +87,7 @@ class BasicAuthTest extends FunctionalTest
 
         $_SERVER['PHP_AUTH_USER'] = 'user-in-mygroup@test.com';
         $_SERVER['PHP_AUTH_PW'] = 'test';
-        $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER);;
+        $response = Director::test('BasicAuthTest_ControllerSecuredWithPermission', null, $_SESSION, null, null, $_SERVER);
         $this->assertEquals(200, $response->getStatusCode(), 'Valid user with required permission has access');
 
         $_SERVER['PHP_AUTH_USER'] = $origUser;

tests/php/Security/MemberAuthenticatorTest.php

@@ -3,22 +3,18 @@
 namespace SilverStripe\Security\Tests;
 
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DataModel;
 use SilverStripe\ORM\FieldType\DBDatetime;
-use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Security\Authenticator;
-use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator;
+use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator;
-use SilverStripe\Security\PasswordEncryptor;
+use SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm;
-use SilverStripe\Security\PasswordEncryptor_PHPHash;
+use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\Member;
-use SilverStripe\Security\MemberAuthenticator\Authenticator;
+use SilverStripe\Security\MemberAuthenticator\MemberLoginForm;
-use SilverStripe\Security\MemberAuthenticator\LoginForm;
-use SilverStripe\Security\CMSMemberLoginForm;
 use SilverStripe\Security\IdentityStore;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\Form;
 use SilverStripe\Control\HTTPRequest;
 
 class MemberAuthenticatorTest extends SapphireTest
@@ -60,31 +56,31 @@ class MemberAuthenticatorTest extends SapphireTest
 
     public function testGenerateLoginForm()
     {
-        $authenticator = new Authenticator();
+        $authenticator = new MemberAuthenticator();
 
         $controller = new Security();
 
         // Create basic login form
         $frontendResponse = $authenticator
             ->getLoginHandler($controller->link())
-            ->handleRequest(new HTTPRequest('get', '/'), \SilverStripe\ORM\DataModel::inst());
+            ->handleRequest(new HTTPRequest('get', '/'), DataModel::inst());
 
         $this->assertTrue(is_array($frontendResponse));
         $this->assertTrue(isset($frontendResponse['Form']));
-        $this->assertTrue($frontendResponse['Form'] instanceof LoginForm);
+        $this->assertTrue($frontendResponse['Form'] instanceof MemberLoginForm);
     }
 
-    /* TO DO - reenable
     public function testGenerateCMSLoginForm()
     {
-        $authenticator = new Authenticator();
+        /** @var CMSMemberAuthenticator $authenticator */
+        $authenticator = new CMSMemberAuthenticator();
 
         // Supports cms login form
-        $this->assertTrue(MemberAuthenticator::supports_cms());
+        $this->assertGreaterThan(0, ($authenticator->supportedServices() & Authenticator::CMS_LOGIN));
-        $cmsForm = MemberAuthenticator::get_cms_login_form($controller);
+        $cmsHandler = $authenticator->getLoginHandler('/');
+        $cmsForm = $cmsHandler->loginForm();
         $this->assertTrue($cmsForm instanceof CMSMemberLoginForm);
     }
-    */
 
 
     /**
@@ -92,7 +88,7 @@ class MemberAuthenticatorTest extends SapphireTest
      */
     public function testAuthenticateByTempID()
     {
-        $authenticator = new CMSAuthenticator();
+        $authenticator = new CMSMemberAuthenticator();
 
         $member = new Member();
         $member->Email = 'test1@test.com';
@@ -105,7 +101,7 @@ class MemberAuthenticatorTest extends SapphireTest
         $this->assertEmpty($tempID);
 
         // If the user logs in then they have a temp id
-        Injector::inst()->get(IdentityStore::class)->logIn($member, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($member, true);
         $tempID = $member->TempIDHash;
         $this->assertNotEmpty($tempID);
 
@@ -120,7 +116,7 @@ class MemberAuthenticatorTest extends SapphireTest
 
         $this->assertNotEmpty($result);
         $this->assertEquals($result->ID, $member->ID);
-        $this->assertEmpty($message);
+        $this->assertTrue($message->isValid());
 
         // Test incorrect login
         $result = $authenticator->authenticate(
@@ -132,9 +128,10 @@ class MemberAuthenticatorTest extends SapphireTest
         );
 
         $this->assertEmpty($result);
+        $messages = $message->getMessages();
         $this->assertEquals(
             _t('SilverStripe\\Security\\Member.ERRORWRONGCRED', 'The provided details don\'t seem to be correct. Please try again.'),
-            $message
+            $messages[0]['message']
         );
     }
 
@@ -143,7 +140,7 @@ class MemberAuthenticatorTest extends SapphireTest
      */
     public function testDefaultAdmin()
     {
-        $authenticator = new Authenticator();
+        $authenticator = new MemberAuthenticator();
 
         // Test correct login
         $result = $authenticator->authenticate(
@@ -155,7 +152,7 @@ class MemberAuthenticatorTest extends SapphireTest
         );
         $this->assertNotEmpty($result);
         $this->assertEquals($result->Email, Security::default_admin_username());
-        $this->assertEmpty($message);
+        $this->assertTrue($message->isValid());
 
         // Test incorrect login
         $result = $authenticator->authenticate(
@@ -165,16 +162,17 @@ class MemberAuthenticatorTest extends SapphireTest
             ),
             $message
         );
+        $messages = $message->getMessages();
         $this->assertEmpty($result);
         $this->assertEquals(
             'The provided details don\'t seem to be correct. Please try again.',
-            $message
+            $messages[0]['message']
         );
     }
 
     public function testDefaultAdminLockOut()
     {
-        $authenticator = new Authenticator();
+        $authenticator = new MemberAuthenticator();
 
         Config::inst()->update(Member::class, 'lock_out_after_incorrect_logins', 1);
         Config::inst()->update(Member::class, 'lock_out_delay_mins', 10);
@@ -185,8 +183,7 @@ class MemberAuthenticatorTest extends SapphireTest
             [
                 'Email' => 'admin',
                 'Password' => 'wrongpassword'
-            ],
+            ]
-            $dummy
         );
 
         $this->assertFalse(Member::default_admin()->canLogin()->isValid());

tests/php/Security/MemberTest.php

@@ -11,7 +11,6 @@ use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Security\Member;
-use SilverStripe\Security\MemberAuthenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\MemberPassword;
 use SilverStripe\Security\Group;
@@ -875,7 +874,7 @@ class MemberTest extends FunctionalTest
     {
         $m1 = $this->objFromFixture(Member::class, 'grouplessmember');
 
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
         $hashes = RememberLoginHash::get()->filter('MemberID', $m1->ID);
         $this->assertEquals($hashes->count(), 1);
@@ -891,7 +890,7 @@ class MemberTest extends FunctionalTest
 */
         $m1 = $this->objFromFixture(Member::class, 'noexpiry');
 
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
         $this->assertNotNull($firstHash);
@@ -970,7 +969,7 @@ class MemberTest extends FunctionalTest
  * @var Member $m1
 */
         $m1 = $this->objFromFixture(Member::class, 'noexpiry');
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
         $this->assertNotNull($firstHash);
 
@@ -1029,10 +1028,10 @@ class MemberTest extends FunctionalTest
         $m1 = $this->objFromFixture(Member::class, 'noexpiry');
 
         // First device
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
         Cookie::set('alc_device', null);
         // Second device
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
         // Hash of first device
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
@@ -1105,7 +1104,7 @@ class MemberTest extends FunctionalTest
 
         // Logging out from any device when all login hashes should be removed
         RememberLoginHash::config()->update('logout_across_devices', true);
-        Injector::inst()->get(IdentityStore::class)->logIn($m1, true, new HTTPRequest('GET', '/'));
+        Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
         $response = $this->get('Security/logout', $this->session());
         $this->assertEquals(
             RememberLoginHash::get()->filter('MemberID', $m1->ID)->count(),
@@ -1423,17 +1422,17 @@ class MemberTest extends FunctionalTest
 
         /** @var Member $adminMember */
         $adminMember = $this->objFromFixture(Member::class, 'admin');
-        $memberID = Member::actAs($adminMember, function () {
+        $member = Member::actAs($adminMember, function () {
-            return Member::currentUserID();
+            return Security::getCurrentUser();
         });
-        $this->assertEquals($adminMember->ID, $memberID);
+        $this->assertEquals($adminMember->ID, $member->ID);
 
         // Check nesting
-        $memberID = Member::actAs($adminMember, function () {
+        $member = Member::actAs($adminMember, function () {
             return Member::actAs(null, function () {
-                return Member::currentUserID();
+                return Security::getCurrentUser();
             });
         });
-        $this->assertEmpty($memberID);
+        $this->assertEmpty($member);
     }
 }

tests/php/Security/SecurityTest.php

@@ -2,18 +2,16 @@
 
 namespace SilverStripe\Security\Tests;
 
-use PhpConsole\Auth;
+use SilverStripe\Dev\Debug;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\FieldType\DBClassName;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\LoginAttempt;
 use SilverStripe\Security\Member;
-use SilverStripe\Security\MemberAuthenticator;
+use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
 use SilverStripe\Security\Security;
-use SilverStripe\Security\Permission;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Dev\FunctionalTest;
@@ -48,13 +46,9 @@ class SecurityTest extends FunctionalTest
 
     protected function setUp()
     {
-        // This test assumes that MemberAuthenticator is present and the default
-        // $this->priorAuthenticators = Authenticator::get_authenticators();
-        // $this->priorDefaultAuthenticator = Authenticator::get_default_authenticator();
-
         // Set to an empty array of authenticators to enable the default
-        Config::modify()->set(Authenticator::class, 'authenticators', []);
+        Config::modify()->set(MemberAuthenticator::class, 'authenticators', []);
-        Config::modify()->set(Authenticator::class, 'default_authenticator', MemberAuthenticator::class);
+        Config::modify()->set(MemberAuthenticator::class, 'default_authenticator', MemberAuthenticator::class);
 
         // And that the unique identified field is 'Email'
         $this->priorUniqueIdentifierField = Member::config()->unique_identifier_field;
@@ -233,7 +227,7 @@ class SecurityTest extends FunctionalTest
         /* View the Security/login page */
         $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
 
-        $items = $this->cssParser()->getBySelector('#LoginForm_LoginForm input.action');
+        $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.action');
 
         /* We have only 1 input, one to allow the user to log in as someone else */
         $this->assertEquals(count($items), 1, 'There is 1 input, allowing the user to log in as someone else.');
@@ -242,7 +236,7 @@ class SecurityTest extends FunctionalTest
 
         /* Submit the form, using only the logout action and a hidden field for the authenticator */
         $response = $this->submitForm(
-            'LoginForm_LoginForm',
+            'MemberLoginForm_LoginForm',
             null,
             array(
                 'action_logout' => 1,
@@ -267,7 +261,7 @@ class SecurityTest extends FunctionalTest
         /* Attempt to get into the admin section */
         $response = $this->get(Config::inst()->get(Security::class, 'login_url'));
 
-        $items = $this->cssParser()->getBySelector('#LoginForm_LoginForm input.text');
+        $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.text');
 
         /* We have 2 text inputs - one for email, and another for the password */
         $this->assertEquals(count($items), 2, 'There are 2 inputs - one for email, another for password');
@@ -286,11 +280,11 @@ class SecurityTest extends FunctionalTest
         $this->get(Config::inst()->get(Security::class, 'login_url'));
         $items = $this
             ->cssParser()
-            ->getBySelector('#LoginForm_LoginForm #LoginForm_LoginForm_Email');
+            ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email');
         $this->assertEquals(1, count($items));
         $this->assertEmpty((string)$items[0]->attributes()->value);
         $this->assertEquals('off', (string)$items[0]->attributes()->autocomplete);
-        $form = $this->cssParser()->getBySelector('#LoginForm_LoginForm');
+        $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm');
         $this->assertEquals(1, count($form));
         $this->assertEquals('off', (string)$form[0]->attributes()->autocomplete);
 
@@ -300,11 +294,11 @@ class SecurityTest extends FunctionalTest
         $this->get(Config::inst()->get(Security::class, 'login_url'));
         $items = $this
             ->cssParser()
-            ->getBySelector('#LoginForm_LoginForm #LoginForm_LoginForm_Email');
+            ->getBySelector('#MemberLoginForm_LoginForm #MemberLoginForm_LoginForm_Email');
         $this->assertEquals(1, count($items));
         $this->assertEquals('myuser@silverstripe.com', (string)$items[0]->attributes()->value);
         $this->assertNotEquals('off', (string)$items[0]->attributes()->autocomplete);
-        $form = $this->cssParser()->getBySelector('#LoginForm_LoginForm');
+        $form = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm');
         $this->assertEquals(1, count($form));
         $this->assertNotEquals('off', (string)$form[0]->attributes()->autocomplete);
     }
@@ -482,11 +476,11 @@ class SecurityTest extends FunctionalTest
         Member::config()->lock_out_delay_mins = 15;
 
         // Login with a wrong password for more than the defined threshold
-        for ($i = 1; $i <= Member::config()->lock_out_after_incorrect_logins+1; $i++) {
+        for ($i = 1; $i <= (Member::config()->lock_out_after_incorrect_logins+1); $i++) {
             $this->doTestLoginForm('testuser@example.com', 'incorrectpassword');
             $member = DataObject::get_by_id(Member::class, $this->idFromFixture(Member::class, 'test'));
 
-            if ($i < Member::config()->lock_out_after_incorrect_logins) {
+            if ($i < Member::config()->get('lock_out_after_incorrect_logins')) {
                 $this->assertNull(
                     $member->LockedOutUntil,
                     'User does not have a lockout time set if under threshold for failed attempts'
@@ -505,7 +499,7 @@ class SecurityTest extends FunctionalTest
                     'User has a lockout time set after too many failed attempts'
                 );
             }
-
+        }
         $msg = _t(
             'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2',
             'Your account has been temporarily disabled because of too many failed attempts at ' .
@@ -513,10 +507,8 @@ class SecurityTest extends FunctionalTest
             null,
             array('count' => Member::config()->lock_out_delay_mins)
         );
-            if ($i > Member::config()->lock_out_after_incorrect_logins) {
         $this->assertHasMessage($msg);
-            }
+
-        }
 
         $this->doTestLoginForm('testuser@example.com', '1nitialPassword');
         $this->assertNull(
@@ -696,7 +688,7 @@ class SecurityTest extends FunctionalTest
         $this->get(Config::inst()->get(Security::class, 'login_url'));
 
         return $this->submitForm(
-            "LoginForm_LoginForm",
+            "MemberLoginForm_LoginForm",
             null,
             array(
                 'Email' => $email,
@@ -750,7 +742,7 @@ class SecurityTest extends FunctionalTest
      */
     protected function getValidationResult()
     {
-        $result = $this->session()->inst_get('FormInfo.LoginForm_LoginForm.result');
+        $result = $this->session()->inst_get('FormInfo.MemberLoginForm_LoginForm.result');
         if ($result) {
             return unserialize($result);
         }