Cleanup and RequestFilter refactor

_config/security.yml

@@ -1,16 +1,7 @@
 ---
-Name: coresecurity
+Name: coreauthentication
 ---
-SilverStripe\Security\MemberAuthenticator\MemberLoginForm:
-  required_fields:
-    - Email
-    - Password
-
 SilverStripe\Core\Injector\Injector:
-  SilverStripe\Control\RequestProcessor:
-    properties:
-      filters:
-        - %$SilverStripe\Security\AuthenticationRequestFilter
   SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler:
     properties:
       SessionVariable: loggedInAs
@@ -19,14 +10,26 @@ SilverStripe\Core\Injector\Injector:
       TokenCookieName: alc_enc
       DeviceCookieName: alc_device
       CascadeInTo: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler
-  SilverStripe\Security\IdentityStore:
-    class: SilverStripe\Security\AuthenticationRequestFilter
+  SilverStripe\Security\AuthenticationHandler:
+    class: SilverStripe\Security\RequestAuthenticationHandler
+    properties:
+      Handlers:
+        session: %$SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler
+        alc: %$SilverStripe\Security\MemberAuthenticator\CookieAuthenticationHandler
+---
+Name: coresecurity
+---
+SilverStripe\Core\Injector\Injector:
+  SilverStripe\Security\AuthenticationRequestFilter:
+    properties:
+      AuthenticationHandler: %$SilverStripe\Security\AuthenticationHandler
+  SilverStripe\Control\RequestProcessor:
+    properties:
+      filters:
+        - %$SilverStripe\Security\AuthenticationRequestFilter
   SilverStripe\Security\Security:
     properties:
-      authenticators:
+      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\IdentityStore: %$SilverStripe\Security\AuthenticationHandler

src/ORM/Connect/Database.php

@@ -490,6 +490,17 @@ abstract class Database
      */
     abstract public function datetimeDifferenceClause($date1, $date2);
 
+    /**
+     * String operator for concatenation of strings
+     *
+     * @return string
+     */
+    public function concatOperator()
+    {
+        // @todo Make ' + ' in mssql
+        return ' || ';
+    }
+
     /**
      * Returns true if this database supports collations
      *

src/Security/AuthenticationHandler.php

@@ -3,9 +3,7 @@
 namespace SilverStripe\Security;
 
 use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Control\HTTPResponse;
 use SilverStripe\ORM\ValidationException;
-use SilverStripe\Security\Member;
 
 /**
  * An AuthenticationHandler is responsible for providing an identity (in the form of a Member object) for
@@ -15,7 +13,7 @@ use SilverStripe\Security\Member;
  * request it should *not* attempt to redirect the visitor to a log-in from or 3rd party handler, as that
  * is the responsibiltiy of other systems.
  */
-interface AuthenticationHandler
+interface AuthenticationHandler extends IdentityStore
 {
     /**
      * Given the current request, authenticate the request for non-session authorization (outside the CMS).

src/Security/AuthenticationRequestFilter.php

@@ -2,54 +2,40 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\Control\HTTPResponse_Exception;
-use SilverStripe\Control\RequestFilter;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\HTTPResponse_Exception;
+use SilverStripe\Control\RequestFilter;
 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\DataModel;
 use SilverStripe\ORM\ValidationException;
 
-class AuthenticationRequestFilter implements RequestFilter, IdentityStore
+class AuthenticationRequestFilter implements RequestFilter
 {
-
     use Configurable;
 
     /**
-     * @var array|AuthenticationHandler[]
+     * @var AuthenticationHandler
      */
-    protected $handlers;
+    protected $authenticationHandler;
 
     /**
-     * This method currently uses a fallback as loading the handlers via YML has proven unstable
-     *
-     * @return array|AuthenticationHandler[]
+     * @return AuthenticationHandler
      */
-    protected function getHandlers()
+    public function getAuthenticationHandler()
     {
-        if (is_array($this->handlers)) {
-            return $this->handlers;
-        }
-
-        return array_map(
-            function ($identifier) {
-                return Injector::inst()->get($identifier);
-            },
-            static::config()->get('handlers')
-        );
+        return $this->authenticationHandler;
     }
 
     /**
-     * Set an associative array of handlers
-     *
-     * @param array|AuthenticationHandler[] $handlers
+     * @param AuthenticationHandler $authenticationHandler
+     * @return $this
      */
-    public function setHandlers($handlers)
+    public function setAuthenticationHandler(AuthenticationHandler $authenticationHandler)
     {
-        $this->handlers = $handlers;
+        $this->authenticationHandler = $authenticationHandler;
+        return $this;
     }
 
     /**
@@ -64,16 +50,9 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
     public function preRequest(HTTPRequest $request, Session $session, DataModel $model)
     {
         try {
-            /** @var AuthenticationHandler $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);
-                if ($member) {
-                    Security::setCurrentUser($member);
-                    break;
-                }
-            }
+            $this
+                ->getAuthenticationHandler()
+                ->authenticateRequest($request);
         } catch (ValidationException $e) {
             throw new HTTPResponse_Exception(
                 "Bad log-in details: " . $e->getMessage(),
@@ -93,43 +72,4 @@ class AuthenticationRequestFilter implements RequestFilter, IdentityStore
     public function postRequest(HTTPRequest $request, HTTPResponse $response, DataModel $model)
     {
     }
-
-    /**
-     * Log into the identity-store handlers attached to this request filter
-     *
-     * @param Member $member
-     * @param bool $persistent
-     * @param HTTPRequest $request
-     * @return HTTPResponse|void
-     */
-    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
-    {
-        $member->beforeMemberLoggedIn();
-
-        foreach ($this->getHandlers() as $handler) {
-            if ($handler instanceof IdentityStore) {
-                $handler->logIn($member, $persistent, $request);
-            }
-        }
-
-        Security::setCurrentUser($member);
-        $member->afterMemberLoggedIn();
-    }
-
-    /**
-     * Log out of all the identity-store handlers attached to this request filter
-     *
-     * @param HTTPRequest $request
-     * @return HTTPResponse|void
-     */
-    public function logOut(HTTPRequest $request = null)
-    {
-        foreach ($this->getHandlers() as $handler) {
-            if ($handler instanceof IdentityStore) {
-                $handler->logOut($request);
-            }
-        }
-
-        Security::setCurrentUser(null);
-    }
 }

src/Security/Authenticator.php

@@ -2,12 +2,9 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\Core\Config\Configurable;
-use SilverStripe\Core\Extensible;
-use SilverStripe\Core\Injector\Injectable;
-use SilverStripe\Control\Controller;
-use SilverStripe\Forms\Form;
 use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Security\MemberAuthenticator\LoginHandler;
+use SilverStripe\Security\MemberAuthenticator\LogoutHandler;
 
 /**
  * Abstract base class for an authentication method
@@ -47,6 +44,7 @@ interface Authenticator
      * be merged into a default controller.
      *
      * @param string $link The base link to use for this RequestHandler
+     * @return LoginHandler
      */
     public function getLoginHandler($link);
 
@@ -56,7 +54,7 @@ interface Authenticator
      * The default URL of the RequestHandler should log the user out immediately and destroy the session.
      *
      * @param string $link The base link to use for this RequestHandler
-     * @return mixed
+     * @return LogoutHandler
      */
     public function getLogOutHandler($link);
 
@@ -75,7 +73,7 @@ interface Authenticator
 
 
     /**
-     * @param $link
+     * @param string $link
      * @return mixed
      */
     public function getLostPasswordHandler($link);
@@ -87,5 +85,5 @@ interface Authenticator
      * @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, &$result);
+    public function authenticate($data, &$result = null);
 }

src/Security/BasicAuth.php

@@ -9,7 +9,6 @@ use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Dev\SapphireTest;
-
 use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
 
 /**

src/Security/CMSSecurity.php

@@ -3,14 +3,12 @@
 namespace SilverStripe\Security;
 
 use SilverStripe\Admin\AdminRootController;
+use SilverStripe\Control\Controller;
+use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
 use SilverStripe\Core\Convert;
-use SilverStripe\Control\Director;
-use SilverStripe\Control\Controller;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\FieldType\DBField;
-use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator;
 use SilverStripe\View\Requirements;
 
 /**
@@ -194,7 +192,7 @@ PHP
         $backURLs = array(
             $this->getRequest()->requestVar('BackURL'),
             Session::get('BackURL'),
-            Director::absoluteURL(AdminRootController::config()->url_base, true),
+            Director::absoluteURL(AdminRootController::config()->get('url_base'), true),
         );
         $backURL = null;
         foreach ($backURLs as $backURL) {

src/Security/Group.php

@@ -4,24 +4,24 @@ namespace SilverStripe\Security;
 
 use SilverStripe\Admin\SecurityAdmin;
 use SilverStripe\Core\Convert;
-use SilverStripe\Forms\Form;
-use SilverStripe\Forms\GridField\GridFieldAddExistingAutocompleter;
-use SilverStripe\Forms\GridField\GridFieldDetailForm;
-use SilverStripe\Forms\TextField;
 use SilverStripe\Forms\DropdownField;
-use SilverStripe\Forms\TextareaField;
-use SilverStripe\Forms\Tab;
-use SilverStripe\Forms\TabSet;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\LiteralField;
-use SilverStripe\Forms\ListboxField;
-use SilverStripe\Forms\HiddenField;
-use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
-use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor;
+use SilverStripe\Forms\Form;
+use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\GridField\GridFieldAddExistingAutocompleter;
 use SilverStripe\Forms\GridField\GridFieldButtonRow;
+use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor;
+use SilverStripe\Forms\GridField\GridFieldDetailForm;
 use SilverStripe\Forms\GridField\GridFieldExportButton;
 use SilverStripe\Forms\GridField\GridFieldPrintButton;
-use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\HiddenField;
+use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
+use SilverStripe\Forms\ListboxField;
+use SilverStripe\Forms\LiteralField;
+use SilverStripe\Forms\Tab;
+use SilverStripe\Forms\TabSet;
+use SilverStripe\Forms\TextareaField;
+use SilverStripe\Forms\TextField;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataQuery;
@@ -29,7 +29,6 @@ use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
 use SilverStripe\ORM\ManyManyList;
 use SilverStripe\ORM\UnsavedRelationList;
-use SilverStripe\View\Requirements;
 
 /**
  * A security group.
@@ -95,6 +94,7 @@ class Group extends DataObject
         $doSet = new ArrayList();
 
         $children = Group::get()->filter("ParentID", $this->ID);
+        /** @var Group $child */
         foreach ($children as $child) {
             $doSet->push($child);
             $doSet->merge($child->getAllChildren());
@@ -159,7 +159,7 @@ class Group extends DataObject
             $detailForm = $config->getComponentByType(GridFieldDetailForm::class);
             $detailForm
                 ->setValidator(Member_Validator::create())
-                ->setItemEditFormCallback(function ($form, $component) use ($group) {
+                ->setItemEditFormCallback(function ($form) use ($group) {
                     /** @var Form $form */
                     $record = $form->getRecord();
                     $groupsField = $form->Fields()->dataFieldByName('DirectGroups');
@@ -369,9 +369,9 @@ class Group extends DataObject
     {
         $parent = $this;
         $items = [];
-        while (isset($parent) && $parent instanceof Group) {
+        while ($parent instanceof Group) {
             $items[] = $parent->ID;
-            $parent = $parent->Parent;
+            $parent = $parent->getParent();
         }
         return $items;
     }
@@ -395,12 +395,14 @@ class Group extends DataObject
             ->sort('"Sort"');
     }
 
+    /**
+     * @return string
+     */
     public function getTreeTitle()
     {
-        if ($this->hasMethod('alternateTreeTitle')) {
-            return $this->alternateTreeTitle();
-        }
-        return htmlspecialchars($this->Title, ENT_QUOTES);
+        $title = htmlspecialchars($this->Title, ENT_QUOTES);
+        $this->extend('updateTreeTitle', $title);
+        return $title;
     }
 
     /**

src/Security/GroupCsvBulkLoader.php

@@ -2,8 +2,8 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\ORM\DataObject;
 use SilverStripe\Dev\CsvBulkLoader;
+use SilverStripe\ORM\DataObject;
 
 /**
  * @todo Migrate Permission->Arg and Permission->Type values
@@ -15,12 +15,8 @@ class GroupCsvBulkLoader extends CsvBulkLoader
         'Code' => 'Code',
     );
 
-    public function __construct($objectClass = null)
+    public function __construct($objectClass = Group::class)
     {
-        if (!$objectClass) {
-            $objectClass = 'SilverStripe\\Security\\Group';
-        }
-
         parent::__construct($objectClass);
     }
 

src/Security/IdentityStore.php

@@ -4,7 +4,6 @@ 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.
@@ -19,7 +18,6 @@ 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.
-     * @return HTTPResponse $response The response object to modify, if needed.
      */
     public function logIn(Member $member, $persistent = false, HTTPRequest $request = null);
 
@@ -27,7 +25,6 @@ interface IdentityStore
      * 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.
-     * @return HTTPResponse $response The response object to modify, if needed.
      */
     public function logOut(HTTPRequest $request = null);
 }

src/Security/LoginForm.php

@@ -2,7 +2,6 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
 

src/Security/Member.php

@@ -3,19 +3,16 @@
 namespace SilverStripe\Security;
 
 use IntlDateFormatter;
+use InvalidArgumentException;
 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;
 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;
 use SilverStripe\Forms\DropdownField;
@@ -23,7 +20,6 @@ use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\Forms\ListboxField;
 use SilverStripe\i18n\i18n;
-use SilverStripe\MSSQL\MSSQLDatabase;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
@@ -31,13 +27,10 @@ use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\ManyManyList;
-use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\Map;
+use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\View\SSViewer;
-use SilverStripe\View\TemplateGlobalProvider;
-use DateTime;
 
 /**
  * The member class which represents the users of the system
@@ -60,6 +53,7 @@ use DateTime;
  * @property int $FailedLoginCount
  * @property string $DateFormat
  * @property string $TimeFormat
+ * @property string $SetPassword Pseudo-DB field for temp storage. Not emitted to DB
  */
 class Member extends DataObject
 {
@@ -482,9 +476,10 @@ class Member extends DataObject
     public function regenerateTempID()
     {
         $generator = new RandomGenerator();
+        $lifetime = self::config()->get('temp_id_lifetime');
         $this->TempIDHash = $generator->randomToken('sha1');
-        $this->TempIDExpired = self::config()->temp_id_lifetime
-            ? date('Y-m-d H:i:s', strtotime(DBDatetime::now()->getValue()) + self::config()->temp_id_lifetime)
+        $this->TempIDExpired = $lifetime
+            ? date('Y-m-d H:i:s', strtotime(DBDatetime::now()->getValue()) + $lifetime)
             : null;
         $this->write();
     }
@@ -756,7 +751,7 @@ class Member extends DataObject
      *
      * @param Member|null|int $member Member or member ID to log in as.
      * Set to null or 0 to act as a logged out user.
-     * @param $callback
+     * @param callable $callback
      */
     public static function actAs($member, $callback)
     {
@@ -837,7 +832,7 @@ class Member extends DataObject
         // If a member with the same "unique identifier" already exists with a different ID, don't allow merging.
         // Note: This does not a full replacement for safeguards in the controller layer (e.g. in a registration form),
         // but rather a last line of defense against data inconsistencies.
-        $identifierField = Member::config()->unique_identifier_field;
+        $identifierField = Member::config()->get('unique_identifier_field');
         if ($this->$identifierField) {
             // Note: Same logic as Member_Validator class
             $filter = [
@@ -890,7 +885,7 @@ class Member extends DataObject
                 $this->Password, // this is assumed to be cleartext
                 $this->Salt,
                 ($this->PasswordEncryption) ?
-                    $this->PasswordEncryption : Security::config()->password_encryption_algorithm,
+                    $this->PasswordEncryption : Security::config()->get('password_encryption_algorithm'),
                 $this
             );
 
@@ -1013,7 +1008,7 @@ class Member extends DataObject
         } elseif ($group instanceof Group) {
             $groupCheckObj = $group;
         } else {
-            user_error('Member::inGroup(): Wrong format for $group parameter', E_USER_ERROR);
+            throw new InvalidArgumentException('Member::inGroup(): Wrong format for $group parameter');
         }
 
         if (!$groupCheckObj) {
@@ -1081,10 +1076,17 @@ class Member extends DataObject
      */
     public static function set_title_columns($columns, $sep = ' ')
     {
+        Deprecation::notice('5.0', 'Use Member.title_format config instead');
         if (!is_array($columns)) {
             $columns = array($columns);
         }
-        self::config()->title_format = array('columns' => $columns, 'sep' => $sep);
+        self::config()->set(
+            'title_format',
+            [
+                'columns' => $columns,
+                'sep' => $sep
+            ]
+        );
     }
 
     //------------------- HELPER METHODS -----------------------------------//
@@ -1133,8 +1135,6 @@ class Member extends DataObject
      */
     public static function get_title_sql()
     {
-        // This should be abstracted to SSDatabase concatOperator or similar.
-        $op = (DB::get_conn() instanceof MSSQLDatabase) ? " + " : " || ";
 
         // Get title_format with fallback to default
         $format = static::config()->get('title_format');
@@ -1151,7 +1151,7 @@ class Member extends DataObject
         }
 
         $sepSQL = Convert::raw2sql($format['sep'], true);
-
+        $op = DB::get_conn()->concatOperator();
         return "(" . join(" $op $sepSQL $op ", $columnsWithTablename) . ")";
     }
 
@@ -1305,6 +1305,7 @@ class Member extends DataObject
 
         $membersList = new ArrayList();
         // This is a bit ineffective, but follow the ORM style
+        /** @var Group $group */
         foreach (Group::get()->byIDs($groupIDList) as $group) {
             $membersList->merge($group->Members());
         }
@@ -1332,7 +1333,7 @@ class Member extends DataObject
             return ArrayList::create()->map();
         }
 
-        if (!$groups || $groups->Count() == 0) {
+        if (count($groups) == 0) {
             $perms = array('ADMIN', 'CMS_ACCESS_AssetAdmin');
 
             if (class_exists(CMSMain::class)) {
@@ -1673,12 +1674,13 @@ class Member extends DataObject
      */
     public function registerFailedLogin()
     {
-        if (self::config()->lock_out_after_incorrect_logins) {
+        $lockOutAfterCount = self::config()->get('lock_out_after_incorrect_logins');
+        if ($lockOutAfterCount) {
             // Keep a tally of the number of failed log-ins so that we can lock people out
             $this->FailedLoginCount = $this->FailedLoginCount + 1;
 
-            if ($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
-                $lockoutMins = self::config()->lock_out_delay_mins;
+            if ($this->FailedLoginCount >= $lockOutAfterCount) {
+                $lockoutMins = self::config()->get('lock_out_delay_mins');
                 $this->LockedOutUntil = date('Y-m-d H:i:s', DBDatetime::now()->getTimestamp() + $lockoutMins * 60);
                 $this->FailedLoginCount = 0;
             }
@@ -1692,7 +1694,7 @@ class Member extends DataObject
      */
     public function registerSuccessfulLogin()
     {
-        if (self::config()->lock_out_after_incorrect_logins) {
+        if (self::config()->get('lock_out_after_incorrect_logins')) {
             // Forgive all past login failures
             $this->FailedLoginCount = 0;
             $this->write();

src/Security/MemberAuthenticator/CMSLoginHandler.php

@@ -5,7 +5,6 @@ namespace SilverStripe\Security\MemberAuthenticator;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Convert;
 use SilverStripe\Security\CMSSecurity;
-use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
 
 class CMSLoginHandler extends LoginHandler

src/Security/MemberAuthenticator/ChangePasswordForm.php

@@ -2,14 +2,14 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
-use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
+use SilverStripe\Control\Session;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\FormField;
-use SilverStripe\Forms\PasswordField;
-use SilverStripe\Forms\FormAction;
-use SilverStripe\Forms\HiddenField;
 use SilverStripe\Forms\Form;
+use SilverStripe\Forms\FormAction;
+use SilverStripe\Forms\FormField;
+use SilverStripe\Forms\HiddenField;
+use SilverStripe\Forms\PasswordField;
 use SilverStripe\Security\Security;
 
 /**

src/Security/MemberAuthenticator/ChangePasswordHandler.php

@@ -4,18 +4,16 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Controller;
-use SilverStripe\Control\RequestHandler;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\RequestHandler;
 use SilverStripe\Control\Session;
+use SilverStripe\Core\Injector\Injector;
 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\IdentityStore;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
-use SilverStripe\Security\IdentityStore;
 
 class ChangePasswordHandler extends RequestHandler
 {

src/Security/MemberAuthenticator/CookieAuthenticationHandler.php

@@ -2,20 +2,19 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
-use SilverStripe\Control\HTTPResponse;
-use SilverStripe\Security\Member;
+use SilverStripe\Control\Cookie;
 use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface;
+use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\Security\AuthenticationHandler;
 use SilverStripe\Security\IdentityStore;
+use SilverStripe\Security\Member;
 use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\Security\Security;
-use SilverStripe\ORM\FieldType\DBDatetime;
-use SilverStripe\Control\Cookie;
 
 /**
  * Authenticate a member pased on a session cookie
  */
-class CookieAuthenticationHandler implements AuthenticationHandlerInterface, IdentityStore
+class CookieAuthenticationHandler implements AuthenticationHandler
 {
 
     /**
@@ -47,11 +46,12 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
      * Set the name of the cookie used to track this device
      *
      * @param string $deviceCookieName
-     * @return null
+     * @return $this
      */
     public function setDeviceCookieName($deviceCookieName)
     {
         $this->deviceCookieName = $deviceCookieName;
+        return $this;
     }
 
     /**
@@ -68,10 +68,12 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
      * Set the name of the cookie used to store an login token
      *
      * @param string $tokenCookieName
+     * @return $this
      */
     public function setTokenCookieName($tokenCookieName)
     {
         $this->tokenCookieName = $tokenCookieName;
+        return $this;
     }
 
     /**
@@ -88,16 +90,17 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
      * Set the name of the cookie used to store an login token
      *
      * @param IdentityStore $cascadeInTo
-     * @return null
+     * @return $this
      */
     public function setCascadeLogInTo(IdentityStore $cascadeInTo)
     {
         $this->cascadeInTo = $cascadeInTo;
+        return $this;
     }
 
     /**
      * @param HTTPRequest $request
-     * @return null|Member
+     * @return Member
      */
     public function authenticateRequest(HTTPRequest $request)
     {
@@ -115,71 +118,65 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
             return null;
         }
 
+        // check if autologin token matches
         /** @var Member $member */
         $member = Member::get()->byID($uid);
+        if (!$member) {
+            return null;
+        }
+
+        $hash = $member->encryptWithUserSettings($token);
 
         /** @var RememberLoginHash $rememberLoginHash */
-        $rememberLoginHash = null;
-
-        // check if autologin token matches
-        if ($member) {
-            $hash = $member->encryptWithUserSettings($token);
-            $rememberLoginHash = RememberLoginHash::get()
-                ->filter(array(
-                    'MemberID' => $member->ID,
-                    'DeviceID' => $deviceID,
-                    'Hash'     => $hash
-                ))->first();
-
-            if (!$rememberLoginHash) {
-                $member = null;
-            } else {
-                // Check for expired token
-                $expiryDate = new \DateTime($rememberLoginHash->ExpiryDate);
-                $now = DBDatetime::now();
-                $now = new \DateTime($now->Rfc2822());
-                if ($now > $expiryDate) {
-                    $member = null;
-                }
-            }
+        $rememberLoginHash = RememberLoginHash::get()
+            ->filter(array(
+                'MemberID' => $member->ID,
+                'DeviceID' => $deviceID,
+                'Hash'     => $hash
+            ))->first();
+        if (!$rememberLoginHash) {
+            return null;
         }
 
-        if ($member) {
-            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->cascadeInTo->logIn($member, false, $request);
-            }
-
-            // @todo Consider whether response should be part of logIn() as well
-
-            // Renew the token
-            if ($rememberLoginHash) {
-                $rememberLoginHash->renew();
-                $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days');
-                Cookie::set(
-                    $this->getTokenCookieName(),
-                    $member->ID . ':' . $rememberLoginHash->getToken(),
-                    $tokenExpiryDays,
-                    null,
-                    null,
-                    false,
-                    true
-                );
-            }
-
-            // Audit logging hook
-            $member->extend('memberAutoLoggedIn');
-
-            return $member;
+        // Check for expired token
+        $expiryDate = new \DateTime($rememberLoginHash->ExpiryDate);
+        $now = DBDatetime::now();
+        $now = new \DateTime($now->Rfc2822());
+        if ($now > $expiryDate) {
+            return null;
         }
+
+        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->cascadeInTo->logIn($member, false, $request);
+        }
+
+        // @todo Consider whether response should be part of logIn() as well
+
+        // Renew the token
+        $rememberLoginHash->renew();
+        $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days');
+        Cookie::set(
+            $this->getTokenCookieName(),
+            $member->ID . ':' . $rememberLoginHash->getToken(),
+            $tokenExpiryDays,
+            null,
+            null,
+            false,
+            true
+        );
+
+        // Audit logging hook
+        $member->extend('memberAutoLoggedIn');
+
+        return $member;
     }
 
     /**
      * @param Member $member
      * @param bool $persistent
      * @param HTTPRequest $request
-     * @return HTTPResponse|void
      */
     public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
     {
@@ -218,8 +215,7 @@ class CookieAuthenticationHandler implements AuthenticationHandlerInterface, Ide
     }
 
     /**
-     * @param HTTPRequest|null $request
-     * @return HTTPResponse|void
+     * @param HTTPRequest $request
      */
     public function logOut(HTTPRequest $request = null)
     {

src/Security/MemberAuthenticator/LoginHandler.php

@@ -5,14 +5,14 @@ namespace SilverStripe\Security\MemberAuthenticator;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
-use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
+use SilverStripe\Control\Session;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Authenticator;
-use SilverStripe\Security\Security;
-use SilverStripe\Security\Member;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Security\IdentityStore;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\Security;
 
 /**
  * Handle login requests from MemberLoginForm

src/Security/MemberAuthenticator/LogoutHandler.php

@@ -2,13 +2,10 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
-use SilverStripe\Control\Cookie;
 use SilverStripe\Control\RequestHandler;
-use SilverStripe\Control\Session;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Security\IdentityStore;
 use SilverStripe\Security\Member;
-use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\Security\Security;
 
 /**

src/Security/MemberAuthenticator/LostPasswordHandler.php

@@ -5,19 +5,12 @@ namespace SilverStripe\Security\MemberAuthenticator;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Email\Email;
 use SilverStripe\Control\HTTPResponse;
-use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
-use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Convert;
 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\ORM\FieldType\DBField;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
-use SilverStripe\Core\Convert;
-use SilverStripe\ORM\FieldType\DBField;
 
 /**
  * Handle login requests from MemberLoginForm

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -2,14 +2,14 @@
 
 namespace SilverStripe\Security\MemberAuthenticator;
 
+use InvalidArgumentException;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Session;
 use SilverStripe\ORM\ValidationResult;
-use InvalidArgumentException;
 use SilverStripe\Security\Authenticator;
-use SilverStripe\Security\Security;
-use SilverStripe\Security\Member;
 use SilverStripe\Security\LoginAttempt;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\Security;
 
 /**
  * Authenticator for the default "member" method
@@ -162,7 +162,7 @@ class MemberAuthenticator implements Authenticator
     }
 
     /**
-     * @param $link
+     * @param string $link
      * @return LostPasswordHandler
      */
     public function getLostPasswordHandler($link)

src/Security/MemberAuthenticator/MemberLoginForm.php

@@ -5,20 +5,19 @@ namespace SilverStripe\Security\MemberAuthenticator;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\Control\Session;
-use SilverStripe\Control\Controller;
-use SilverStripe\Forms\HiddenField;
+use SilverStripe\Forms\CheckboxField;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\FormAction;
-use SilverStripe\Forms\TextField;
-use SilverStripe\Forms\PasswordField;
-use SilverStripe\Forms\CheckboxField;
+use SilverStripe\Forms\HiddenField;
 use SilverStripe\Forms\LiteralField;
+use SilverStripe\Forms\PasswordField;
 use SilverStripe\Forms\RequiredFields;
+use SilverStripe\Forms\TextField;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\Member;
-use SilverStripe\Security\Security;
-use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\Security\LoginForm as BaseLoginForm;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\RememberLoginHash;
+use SilverStripe\Security\Security;
 use SilverStripe\View\Requirements;
 
 /**
@@ -42,9 +41,14 @@ class MemberLoginForm extends BaseLoginForm
 
     /**
      * Required fields for validation
+     *
+     * @config
      * @var array
      */
-    private static $required_fields;
+    private static $required_fields = [
+        'Email',
+        'Password',
+    ];
 
     /**
      * Constructor

src/Security/MemberAuthenticator/SessionAuthenticationHandler.php

@@ -3,21 +3,17 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Cookie;
-use SilverStripe\Control\HTTPResponse;
-use SilverStripe\ORM\DataObject;
-use SilverStripe\Security\Member;
+use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\Session;
-use SilverStripe\Control\Director;
 use SilverStripe\Security\AuthenticationHandler;
-use SilverStripe\Security\IdentityStore;
+use SilverStripe\Security\Member;
 
 /**
  * Authenticate a member pased on a session cookie
  */
-class SessionAuthenticationHandler implements AuthenticationHandler, IdentityStore
+class SessionAuthenticationHandler implements AuthenticationHandler
 {
-
     /**
      * @var string
      */
@@ -44,27 +40,26 @@ class SessionAuthenticationHandler implements AuthenticationHandler, IdentitySto
     }
 
     /**
-     * @inherit
      * @param HTTPRequest $request
-     * @return null|DataObject|Member
+     * @return Member
      */
     public function authenticateRequest(HTTPRequest $request)
     {
-        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
-            return Member::get()->byID($id);
+        // If ID is a bad ID it will be treated as if the user is not logged in, rather than throwing a
+        // ValidationException
+        $id = Session::get($this->getSessionVariable());
+        if (!$id) {
+            return null;
         }
-
-        return null;
+        /** @var Member $member */
+        $member = Member::get()->byID($id);
+        return $member;
     }
 
     /**
-     * @inherit
      * @param Member $member
      * @param bool $persistent
-     * @param HTTPRequest|null $request
-     * @return HTTPResponse|void
+     * @param HTTPRequest $request
      */
     public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
     {
@@ -103,8 +98,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler, IdentitySto
     }
 
     /**
-     * @param HTTPRequest|null $request
-     * @return HTTPResponse|void
+     * @param HTTPRequest $request
      */
     public function logOut(HTTPRequest $request = null)
     {

src/Security/Member_GroupSet.php

@@ -109,7 +109,7 @@ class Member_GroupSet extends ManyManyList
     {
         $id = $this->getForeignID();
         if ($id) {
-            return DataObject::get_by_id('SilverStripe\\Security\\Member', $id);
+            return DataObject::get_by_id(Member::class, $id);
         }
     }
 }

src/Security/Member_Validator.php

@@ -2,8 +2,8 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\Forms\RequiredFields;
 use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest;
+use SilverStripe\Forms\RequiredFields;
 
 /**
  * Member Validator

src/Security/PasswordEncryptor.php

@@ -2,8 +2,8 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\Core\Config\Config;
 use ReflectionClass;
+use SilverStripe\Core\Config\Config;
 
 /**
  * Allows pluggable password encryption.

src/Security/Permission.php

@@ -4,12 +4,11 @@ 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;
-use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DB;
 use SilverStripe\ORM\SS_List;
 use SilverStripe\View\TemplateGlobalProvider;
 
@@ -459,7 +458,7 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
     /**
      * Returns all members for a specific permission.
      *
-     * @param $code String|array Either a single permission code, or a list of permission codes
+     * @param string|array $code Either a single permission code, or a list of permission codes
      * @return SS_List Returns a set of member that have the specified
      *                       permission.
      */

src/Security/PermissionCheckboxSetField.php

@@ -2,14 +2,13 @@
 
 namespace SilverStripe\Security;
 
+use InvalidArgumentException;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Forms\FormField;
-use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataObjectInterface;
-use SilverStripe\View\Requirements;
-use InvalidArgumentException;
+use SilverStripe\ORM\SS_List;
 
 /**
  * Shows a categorized list of available permissions (through {@link Permission::get_codes()}).

src/Security/RememberLoginHash.php

@@ -2,10 +2,10 @@
 
 namespace SilverStripe\Security;
 
-use SilverStripe\ORM\FieldType\DBDatetime;
-use SilverStripe\ORM\DataObject;
-use DateTime;
 use DateInterval;
+use DateTime;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\FieldType\DBDatetime;
 
 /**
  * Persists a token associated with a device for users who opted for the "Remember Me"
@@ -16,7 +16,8 @@ use DateInterval;
  * is discarded as well.
  *
  * @property string $DeviceID
- * @property string $RememberLoginHash
+ * @property string $ExpiryDate
+ * @property string $Hash
  * @method Member Member()
  */
 class RememberLoginHash extends DataObject

src/Security/RequestAuthenticationHandler.php

@@ -0,0 +1,83 @@
+<?php
+
+namespace SilverStripe\Security;
+
+use SilverStripe\Control\HTTPRequest;
+
+/**
+ * Core authentication handler / store
+ */
+class RequestAuthenticationHandler implements AuthenticationHandler
+{
+    /**
+     * @var AuthenticationHandler[]
+     */
+    protected $handlers = [];
+
+    /**
+     * This method currently uses a fallback as loading the handlers via YML has proven unstable
+     *
+     * @return AuthenticationHandler[]
+     */
+    protected function getHandlers()
+    {
+        return $this->handlers;
+    }
+
+    /**
+     * Set an associative array of handlers
+     *
+     * @param AuthenticationHandler[] $handlers
+     * @return $this
+     */
+    public function setHandlers(array $handlers)
+    {
+        $this->handlers = $handlers;
+        return $this;
+    }
+
+    public function authenticateRequest(HTTPRequest $request)
+    {
+        /** @var AuthenticationHandler $handler */
+        foreach ($this->getHandlers() as $name => $handler) {
+            // in order to add cookies, etc
+            $member = $handler->authenticateRequest($request);
+            if ($member) {
+                Security::setCurrentUser($member);
+                return;
+            }
+        }
+    }
+    /**
+     * Log into the identity-store handlers attached to this request filter
+     *
+     * @param Member $member
+     * @param bool $persistent
+     * @param HTTPRequest $request
+     */
+    public function logIn(Member $member, $persistent = false, HTTPRequest $request = null)
+    {
+        $member->beforeMemberLoggedIn();
+
+        foreach ($this->getHandlers() as $handler) {
+            $handler->logIn($member, $persistent, $request);
+        }
+
+        Security::setCurrentUser($member);
+        $member->afterMemberLoggedIn();
+    }
+
+    /**
+     * Log out of all the identity-store handlers attached to this request filter
+     *
+     * @param HTTPRequest $request
+     */
+    public function logOut(HTTPRequest $request = null)
+    {
+        foreach ($this->getHandlers() as $handler) {
+            $handler->logOut($request);
+        }
+
+        Security::setCurrentUser(null);
+    }
+}

src/Security/Security.php

@@ -2,25 +2,26 @@
 
 namespace SilverStripe\Security;
 
-use Page;
 use LogicException;
-use SilverStripe\CMS\Controllers\ContentController;
+use Page;
+use SilverStripe\CMS\Controllers\ModelAsController;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
-use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
+use SilverStripe\Control\Session;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Convert;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\Forms\Form;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataModel;
-use SilverStripe\ORM\DB;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBHTMLText;
 use SilverStripe\ORM\ValidationResult;
@@ -222,7 +223,7 @@ class Security extends Controller implements TemplateGlobalProvider
     protected static $currentUser;
 
     /**
-     * @return array
+     * @return Authenticator[]
      */
     public function getAuthenticators()
     {
@@ -230,16 +231,13 @@ class Security extends Controller implements TemplateGlobalProvider
     }
 
     /**
-     * @param array|Authenticator $authenticators
+     * @param Authenticator[] $authenticators
      */
     public function setAuthenticators(array $authenticators)
     {
         $this->authenticators = $authenticators;
     }
 
-    /**
-     * @inheritdoc
-     */
     protected function init()
     {
         parent::init();
@@ -257,9 +255,6 @@ class Security extends Controller implements TemplateGlobalProvider
         }
     }
 
-    /**
-     * @inheritdoc
-     */
     public function index()
     {
         return $this->httpError(404); // no-op
@@ -287,15 +282,15 @@ 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
+     * @return Authenticator[] Return an array of Authenticator objects
      */
     public function getApplicableAuthenticators($service = Authenticator::LOGIN)
     {
         $authenticators = $this->authenticators;
 
-        /** @var Authenticator $class */
-        foreach ($authenticators as $name => $class) {
-            if (!($class->supportedServices() & $service)) {
+        /** @var Authenticator $authenticator */
+        foreach ($authenticators as $name => $authenticator) {
+            if (!($authenticator->supportedServices() & $service)) {
                 unset($authenticators[$name]);
             }
         }
@@ -468,8 +463,10 @@ class Security extends Controller implements TemplateGlobalProvider
         Deprecation::notice('5.0.0', 'Now handled by delegateToMultipleHandlers');
 
         return array_map(
-            function ($authenticator) {
-                return [$authenticator->getLoginHandler($this->Link())->loginForm()];
+            function (Authenticator $authenticator) {
+                return [
+                    $authenticator->getLoginHandler($this->Link())->loginForm()
+                ];
             },
             $this->getApplicableAuthenticators()
         );
@@ -601,16 +598,14 @@ class Security extends Controller implements TemplateGlobalProvider
 
         // Create new instance of page holder
         /** @var Page $holderPage */
-        $holderPage = new $pageClass;
+        $holderPage = Injector::inst()->create($pageClass);
         $holderPage->Title = $title;
         /** @skipUpgrade */
         $holderPage->URLSegment = 'Security';
         // Disable ID-based caching  of the log-in page by making it a random number
         $holderPage->ID = -1 * random_int(1, 10000000);
 
-        $controllerClass = $holderPage->getControllerName();
-        /** @var ContentController $controller */
-        $controller = $controllerClass::create($holderPage);
+        $controller = ModelAsController::controller_for($holderPage);
         $controller->setDataModel($this->model);
         $controller->doInit();
 
@@ -713,8 +708,7 @@ class Security extends Controller implements TemplateGlobalProvider
         $link = $this->Link('login');
 
         // Delegate to a single handler - Security/login/<authname>/...
-        if ($authName && $this->hasAuthenticator($authName)
-        ) {
+        if ($authName && $this->hasAuthenticator($authName)) {
             if ($request) {
                 $request->shift();
             }
@@ -733,7 +727,7 @@ class Security extends Controller implements TemplateGlobalProvider
 
         array_walk(
             $handlers,
-            function (&$auth, $name) use ($link) {
+            function (Authenticator &$auth, $name) use ($link) {
                 $auth = $auth->getLoginHandler(Controller::join_links($link, $name));
             }
         );
@@ -766,7 +760,7 @@ class Security extends Controller implements TemplateGlobalProvider
 
         // Process each of the handlers
         $results = array_map(
-            function ($handler) {
+            function (RequestHandler $handler) {
                 return $handler->handleRequest($this->getRequest(), DataModel::inst());
             },
             $handlers
@@ -1200,6 +1194,8 @@ class Security extends Controller implements TemplateGlobalProvider
 
     /**
      * For the database_is_ready call to return a certain value - used for testing
+     *
+     * @param bool $isReady
      */
     public static function force_database_is_ready($isReady)
     {
@@ -1220,7 +1216,7 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Set to true to ignore access to disallowed actions, rather than returning permission failure
      * Note that this is just a flag that other code needs to check with Security::ignore_disallowed_actions()
-     * @param $flag True or false
+     * @param bool $flag True or false
      */
     public static function set_ignore_disallowed_actions($flag)
     {

src/Security/SecurityToken.php

@@ -2,11 +2,11 @@
 
 namespace SilverStripe\Security;
 
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\Session;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Injector\Injectable;
-use SilverStripe\Control\Session;
-use SilverStripe\Control\Controller;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\HiddenField;
 use SilverStripe\View\TemplateGlobalProvider;
@@ -61,11 +61,11 @@ class SecurityToken implements TemplateGlobalProvider
     protected $name = null;
 
     /**
-     * @param $name
+     * @param string $name
      */
     public function __construct($name = null)
     {
-        $this->name = ($name) ? $name : self::get_default_name();
+        $this->name = $name ?: self::get_default_name();
     }
 
     /**