CMS Login Handling

Move to canLogin in the authentication check. Protected isLockedOut

Enable login to be called with a different login service (CMSLogin), enabling CMS Log in. Seems the styling and/or output is still broken.

logOut could be managed from the Authenticator instead of the member

_config/security.yml

@@ -6,3 +6,7 @@ SilverStripe\Security\MemberAuthenticator\LoginForm:
 SilverStripe\Security\Security:
   authenticators:
     default: SilverStripe\Security\MemberAuthenticator\Authenticator
+    cms: SilverStripe\Security\MemberAuthenticator\CMSAuthenticator
+
+SilverStripe\Security\MemberAuthenticator\CMSSecurity:
+  reauth_enabled: true

src/Security/Authenticator.php

@@ -45,15 +45,10 @@ interface Authenticator
      * URL-handling methods may return an array [ "Form" => (form-object) ] which can then
      * be merged into a default controller.
      *
-     * @param $link The base link to use for this RequestHnadler
+     * @param string $link The base link to use for this RequestHnadler
      */
     public function getLoginHandler($link);
 
-    /**
-     * @todo
-     */
-    public function getCMSLoginHandler($link);
-
     /**
      * Return RequestHandler to manage the change-password process.
      *
@@ -63,7 +58,7 @@ interface Authenticator
      * URL-handling methods may return an array [ "Form" => (form-object) ] which can then
      * be merged into a default controller.
      *
-     * @param $link The base link to use for this RequestHnadler
+     * @param string $link The base link to use for this RequestHnadler
      */
     public function getChangePasswordHandler($link);
 
@@ -86,4 +81,13 @@ interface Authenticator
      * @return array
      */
 //    public function getAuthenticateFields();
+
+    /**
+     * Log the member out of this Authentication method.
+     *
+     * @param Member $member by reference, to allow for multiple actions on the member with a single write
+     * @return boolean|Member if logout was unsuccessfull, return true, otherwise, the member is returned
+     */
+    public function doLogOut(&$member);
+
 }

src/Security/BasicAuth.php

@@ -85,6 +85,7 @@ class BasicAuth
 
         $member = null;
         if (isset($_SERVER['PHP_AUTH_USER']) && isset($_SERVER['PHP_AUTH_PW'])) {
+            /** @var Authenticator $authenticator */
             $authenticator = Injector::inst()->get(Authenticator::class);
 
             $member = $authenticator->authenticate([
@@ -151,9 +152,9 @@ class BasicAuth
      */
     public static function protect_entire_site($protect = true, $code = 'ADMIN', $message = null)
     {
-        Config::inst()->update('SilverStripe\\Security\\BasicAuth', 'entire_site_protected', $protect);
+        Config::inst()->update(self::class, 'entire_site_protected', $protect);
-        Config::inst()->update('SilverStripe\\Security\\BasicAuth', 'entire_site_protected_code', $code);
+        Config::inst()->update(self::class, 'entire_site_protected_code', $code);
-        Config::inst()->update('SilverStripe\\Security\\BasicAuth', 'entire_site_protected_message', $message);
+        Config::inst()->update(self::class, 'entire_site_protected_message', $message);
     }
 
     /**
@@ -165,7 +166,7 @@ class BasicAuth
      */
     public static function protect_site_if_necessary()
     {
-        $config = Config::forClass('SilverStripe\\Security\\BasicAuth');
+        $config = Config::forClass(BasicAuth::class);
         if ($config->entire_site_protected) {
             self::requireLogin($config->entire_site_protected_message, $config->entire_site_protected_code, false);
         }

src/Security/CMSMemberLoginForm.php

@@ -1,38 +1,30 @@
 <?php
 
-namespace SilverStripe\Security;
+namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Controller;
+use SilverStripe\Control\RequestHandler;
 use SilverStripe\Forms\CheckboxField;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\FormAction;
 use SilverStripe\Forms\HiddenField;
 use SilverStripe\Forms\LiteralField;
 use SilverStripe\Forms\PasswordField;
+use SilverStripe\Security\Security;
 
 /**
  * Provides the in-cms session re-authentication form for the "member" authenticator
  */
 class CMSMemberLoginForm extends LoginForm
 {
-    /**
-     * Get link to use for external security actions
-     *
-     * @param string $action Action
-     * @return string
-     */
-    public function getExternalLink($action = null)
-    {
-        return Security::singleton()->Link($action);
-    }
 
     /**
      * CMSMemberLoginForm constructor.
-     * @param Controller $controller
+     * @param RequestHandler $controller
      * @param string $authenticatorClass
      * @param FieldList $name
      */
-    public function __construct(Controller $controller, $authenticatorClass, $name)
+    public function __construct(RequestHandler $controller, $authenticatorClass, $name)
     {
         $this->controller = $controller;
 
@@ -42,7 +34,7 @@ class CMSMemberLoginForm extends LoginForm
 
         $actions = $this->getFormActions();
 
-        parent::__construct($controller, $name, $fields, $actions);
+        parent::__construct($controller, $authenticatorClass, $name, $fields, $actions);
     }
 
     /**
@@ -51,7 +43,7 @@ class CMSMemberLoginForm extends LoginForm
     public function getFormFields()
     {
         // Set default fields
-        $fields = new FieldList(
+        $fields = FieldList::create([
             HiddenField::create("AuthenticationMethod", null, $this->authenticator_class, $this),
             HiddenField::create('tempid', null, $this->controller->getRequest()->requestVar('tempid')),
             PasswordField::create("Password", _t('SilverStripe\\Security\\Member.PASSWORD', 'Password')),
@@ -63,9 +55,9 @@ class CMSMemberLoginForm extends LoginForm
                     _t('SilverStripe\\Security\\CMSMemberLoginForm.BUTTONFORGOTPASSWORD', "Forgot password?")
                 )
             )
-        );
+        ]);
 
-        if (Security::config()->autologin_enabled) {
+        if (Security::config()->get('autologin_enabled')) {
             $fields->push(CheckboxField::create(
                 "Remember",
                 _t('SilverStripe\\Security\\Member.REMEMBERME', "Remember me next time?")
@@ -88,8 +80,8 @@ class CMSMemberLoginForm extends LoginForm
         }
 
         // Make actions
-        $actions = new FieldList(
+        $actions = FieldList::create([
-            FormAction::create('dologin', _t('SilverStripe\\Security\\CMSMemberLoginForm.BUTTONLOGIN', "Log back in")),
+            FormAction::create('doLogin', _t('SilverStripe\\Security\\CMSMemberLoginForm.BUTTONLOGIN', "Log back in")),
             LiteralField::create(
                 'doLogout',
                 sprintf(
@@ -98,14 +90,20 @@ class CMSMemberLoginForm extends LoginForm
                     _t('SilverStripe\\Security\\CMSMemberLoginForm.BUTTONLOGOUT', "Log out")
                 )
             )
-        );
+        ]);
 
         return $actions;
     }
 
-    protected function buildRequestHandler()
+    /**
+     * Get link to use for external security actions
+     *
+     * @param string $action Action
+     * @return string
+     */
+    public function getExternalLink($action = null)
     {
-        return CMSMemberLoginHandler::create($this);
+        return Security::singleton()->Link($action);
     }
 
     /**

src/Security/CMSSecurity.php

@@ -4,11 +4,13 @@ namespace SilverStripe\Security;
 
 use SilverStripe\Admin\AdminRootController;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\Session;
 use SilverStripe\Core\Convert;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\Controller;
-use SilverStripe\Control\Session;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\FieldType\DBField;
+use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator;
 use SilverStripe\View\Requirements;
 
 /**
@@ -22,6 +24,7 @@ class CMSSecurity extends Security
     );
 
     private static $allowed_actions = array(
+        'login',
         'LoginForm',
         'success'
     );
@@ -41,12 +44,27 @@ class CMSSecurity extends Security
         Requirements::javascript(FRAMEWORK_ADMIN_DIR . '/client/dist/js/vendor.js');
     }
 
+    public function login($request, $service = Authenticator::CMS_LOGIN)
+    {
+        return parent::login($request, Authenticator::CMS_LOGIN);
+    }
+
     public function Link($action = null)
     {
         /** @skipUpgrade */
         return Controller::join_links(Director::baseURL(), "CMSSecurity", $action);
     }
 
+    protected function getAuthenticator($name = 'cms')
+    {
+        return parent::getAuthenticator($name);
+    }
+
+    public static function getAuthenticators($service = Authenticator::CMS_LOGIN)
+    {
+        return parent::getAuthenticators($service);
+    }
+
     /**
      * Get known logged out member
      *
@@ -57,6 +75,7 @@ class CMSSecurity extends Security
         if ($tempid = $this->getRequest()->requestVar('tempid')) {
             return Member::member_from_tempid($tempid);
         }
+
         return null;
     }
 
@@ -129,6 +148,7 @@ setTimeout(function(){top.location.href = "$loginURLJS";}, 0);
 PHP
         );
         $this->setResponse($response);
+
         return $response;
     }
 
@@ -142,19 +162,6 @@ PHP
         return parent::preLogin();
     }
 
-    public function GetLoginForms()
-    {
-        $forms = array();
-        $authenticators = Authenticator::get_authenticators();
-        foreach ($authenticators as $authenticator) {
-            // Get only CMS-supporting authenticators
-            if ($authenticator::supports_cms()) {
-                $forms[] = $authenticator::get_cms_login_form($this);
-            }
-        }
-        return $forms;
-    }
-
     /**
      * Determine if CMSSecurity is enabled
      *
@@ -163,28 +170,23 @@ PHP
     public static function enabled()
     {
         // Disable shortcut
-        if (!static::config()->reauth_enabled) {
+        if (!static::config()->get('reauth_enabled')) {
             return false;
         }
 
-        // Count all cms-supported methods
+        /** @var [] $authenticators */
-        $authenticators = Authenticator::get_authenticators();
+        $authenticators = Security::config()->get('authenticators');
-        foreach ($authenticators as $authenticator) {
+        foreach ($authenticators as $name => $authenticator) {
             // Supported if at least one authenticator is supported
-            if ($authenticator::supports_cms()) {
+            $authenticator = Injector::inst()->get($authenticator);
+            if (($authenticator->supportedServices() & Authenticator::CMS_LOGIN)
+                && Security::hasAuthenticator($name)
+            ) {
                 return true;
             }
         }
-        return false;
-    }
 
-    public function LoginForm()
+        return false;
-    {
-        $authenticator = $this->getAuthenticator('default');
-        if ($authenticator && $authenticator::supports_cms()) {
-            return $authenticator::get_cms_login_form($this);
-        }
-        user_error('Passed invalid authentication method', E_USER_ERROR);
     }
 
     /**
@@ -217,7 +219,7 @@ PHP
         $controller = $controller->customise(array(
             'Content' => _t(
                 'SilverStripe\\Security\\CMSSecurity.SUCCESSCONTENT',
-                '<p>Login success. If you are not automatically redirected '.
+                '<p>Login success. If you are not automatically redirected ' .
                 '<a target="_top" href="{link}">click here</a></p>',
                 'Login message displayed in the cms popup once a user has re-authenticated themselves',
                 array('link' => Convert::raw2att($backURL))

src/Security/Member.php

@@ -368,7 +368,7 @@ class Member extends DataObject implements TemplateGlobalProvider
                     'Your account has been temporarily disabled because of too many failed attempts at ' .
                     'logging in. Please try again in {count} minutes.',
                     null,
-                    array('count' => $this->config()->lock_out_delay_mins)
+                    array('count' => static::config()->get('lock_out_delay_mins'))
                 )
             );
         }
@@ -382,7 +382,7 @@ class Member extends DataObject implements TemplateGlobalProvider
      *
      * @return bool
      */
-    public function isLockedOut()
+    protected function isLockedOut()
     {
         if (!$this->LockedOutUntil) {
             return false;
@@ -499,7 +499,7 @@ class Member extends DataObject implements TemplateGlobalProvider
         $this->write();
 
         // Audit logging hook
-        $this->extend('memberLoggedIn');
+        $this->extend('afterMemberLoggedIn');
     }
 
     /**
@@ -626,40 +626,6 @@ class Member extends DataObject implements TemplateGlobalProvider
         }
     }
 
-    /**
-     * Logs this member out.
-     */
-    public function logOut()
-    {
-        $this->extend('beforeMemberLoggedOut');
-
-        Session::clear("loggedInAs");
-        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');
-    }
-
     /**
      * Utility for generating secure password hashes for this member.
      *
@@ -762,7 +728,7 @@ class Member extends DataObject implements TemplateGlobalProvider
             ->filter('TempIDHash', $tempid);
 
         // Exclude expired
-        if (static::config()->temp_id_lifetime) {
+        if (static::config()->get('temp_id_lifetime')) {
             $members = $members->filter('TempIDExpired:GreaterThan', DBDatetime::now()->getValue());
         }
 
@@ -788,7 +754,7 @@ class Member extends DataObject implements TemplateGlobalProvider
             i18n::getSources()->getKnownLocales()
         ));
 
-        $fields->removeByName(static::config()->hidden_fields);
+        $fields->removeByName(static::config()->get('hidden_fields'));
         $fields->removeByName('FailedLoginCount');
 
 
@@ -988,7 +954,7 @@ class Member extends DataObject implements TemplateGlobalProvider
         if ((Director::isLive() || Injector::inst()->get(Mailer::class) instanceof TestMailer)
             && $this->isChanged('Password')
             && $this->record['Password']
-            && $this->config()->notify_password_change
+            && static::config()->get('notify_password_change')
         ) {
             Email::create()
                 ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail')
@@ -1219,7 +1185,7 @@ class Member extends DataObject implements TemplateGlobalProvider
      */
     public function getTitle()
     {
-        $format = $this->config()->title_format;
+        $format = static::config()->get('title_format');
         if ($format) {
             $values = array();
             foreach ($format['columns'] as $col) {
@@ -1254,7 +1220,7 @@ class Member extends DataObject implements TemplateGlobalProvider
         $op = (DB::get_conn() instanceof MSSQLDatabase) ? " + " : " || ";
 
         // Get title_format with fallback to default
-        $format = static::config()->title_format;
+        $format = static::config()->get('title_format');
         if (!$format) {
             $format = [
                 'columns' => ['Surname', 'FirstName'],
@@ -1542,9 +1508,9 @@ class Member extends DataObject implements TemplateGlobalProvider
                 _t(__CLASS__.'.INTERFACELANG', "Interface Language", 'Language of the CMS'),
                 i18n::getSources()->getKnownLocales()
             ));
-            $mainFields->removeByName($this->config()->hidden_fields);
+            $mainFields->removeByName(static::config()->get('hidden_fields'));
 
-            if (! $this->config()->lock_out_after_incorrect_logins) {
+            if (! static::config()->get('lock_out_after_incorrect_logins')) {
                 $mainFields->removeByName('FailedLoginCount');
             }
 

src/Security/MemberAuthenticator/Authenticator.php

@@ -3,11 +3,12 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Controller;
+use SilverStripe\Control\Cookie;
 use SilverStripe\Control\Session;
-use SilverStripe\Forms\Form;
 use SilverStripe\ORM\ValidationResult;
 use InvalidArgumentException;
 use SilverStripe\Security\Authenticator as BaseAuthenticator;
+use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\LoginAttempt;
@@ -24,7 +25,7 @@ class Authenticator implements BaseAuthenticator
     {
         // Bitwise-OR of all the supported services, to make a bitmask
         return BaseAuthenticator::LOGIN | BaseAuthenticator::LOGOUT | BaseAuthenticator::CHANGE_PASSWORD
-            | BaseAuthenticator::RESET_PASSWORD | BaseAuthenticator::CMS_LOGIN;
+            | BaseAuthenticator::RESET_PASSWORD;
     }
 
     /**
@@ -50,39 +51,24 @@ class Authenticator implements BaseAuthenticator
     /**
      * Attempt to find and authenticate member if possible from the given data
      *
-     * @param array $data
+     * @param array $data Form submitted data
-     * @param Form $form
+     * @param $message
      * @param bool &$success Success flag
+     * @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)
+    protected function authenticateMember($data, &$message, &$success, $member = null)
     {
         // Default success to false
         $success = false;
-
+        $email = !empty($data['Email']) ? $data['Email'] : null ;
-        // Attempt to identify by temporary ID
+        
-        $member = null;
-        $email = null;
-        if (!empty($data['tempid'])) {
-            // Find user by tempid, in case they are re-validating an existing session
-            $member = Member::member_from_tempid($data['tempid']);
-            if ($member) {
-                $email = $member->Email;
-            }
-        }
-
-        // Otherwise, get email from posted value instead
-        /** @skipUpgrade */
-        if (!$member && !empty($data['Email'])) {
-            $email = $data['Email'];
-        }
-
         // 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->isLockedOut() && Security::check_default_admin($email, $data['Password']);
+            $success = $member->canLogin()->isValid() && Security::check_default_admin($email, $data['Password']);
             //protect against failed login
             if ($success) {
                 return $member;
@@ -92,8 +78,9 @@ class Authenticator implements BaseAuthenticator
         // Attempt to identify user by email
         if (!$member && $email) {
             // Find user by email
+            /** @var Member $member */
             $member = Member::get()
-                ->filter(Member::config()->unique_identifier_field, $email)
+                ->filter([Member::config()->get('unique_identifier_field') => $email])
                 ->first();
         }
 
@@ -120,7 +107,7 @@ class Authenticator implements BaseAuthenticator
                 $result->getMessages()
             ));
         } else {
-            if ($member) {
+            if ($member) { // How can success be true and member false?
                 $member->registerSuccessfulLogin();
             }
         }
@@ -137,7 +124,7 @@ class Authenticator implements BaseAuthenticator
      */
     protected function recordLoginAttempt($data, $member, $success)
     {
-        if (!Security::config()->login_recording) {
+        if (!Security::config()->get('login_recording')) {
             return;
         }
 
@@ -148,7 +135,7 @@ class Authenticator implements BaseAuthenticator
             throw new InvalidArgumentException("Bad email passed to MemberAuthenticator::authenticate(): $email");
         }
 
-        $attempt = new LoginAttempt();
+        $attempt = LoginAttempt::create();
         if ($success) {
             // successful login (member is existing with matching password)
             $attempt->MemberID = $member->ID;
@@ -198,8 +185,39 @@ class Authenticator implements BaseAuthenticator
         return LoginHandler::create($link, $this);
     }
 
-    public function getCMSLoginHandler($link)
+    /**
+     *
+     * @param Member $member
+     * @return bool|Member
+     */
+    public function doLogOut(&$member)
     {
-        return CMSMemberLoginHandler::create($controller, self::class, "LoginForm");
+        if($member instanceof Member) {
+            Session::clear("loggedInAs");
+            if (Member::config()->login_marker_cookie) {
+                Cookie::set(Member::config()->login_marker_cookie, null, 0);
+            }
+
+            Session::destroy();
+
+            // Clears any potential previous hashes for this member
+            RememberLoginHash::clear($member, 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');
+
+            // Log out unsuccessful. Useful for 3rd-party logins that return failure. Shouldn't happen
+            // on the default authenticator though.
+            if(Member::currentUserID()) {
+                return Member::currentUser();
+            }
+        }
+        return true;
     }
 }

src/Security/MemberAuthenticator/CMSAuthenticator.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace SilverStripe\Security\MemberAuthenticator;
+
+use SilverStripe\Security\Authenticator as BaseAuthenticator;
+use SilverStripe\Security\Member;
+
+class CMSAuthenticator extends Authenticator
+{
+
+    public function supportedServices()
+    {
+        return BaseAuthenticator::CMS_LOGIN;
+    }
+
+    /**
+     * @param array $data
+     * @param $message
+     * @param bool $success
+     * @return Member
+     */
+    protected function authenticateMember($data, &$message, &$success, $member = null)
+    {
+        // Attempt to identify by temporary ID
+        if (!empty($data['tempid'])) {
+            // Find user by tempid, in case they are re-validating an existing session
+            $member = Member::member_from_tempid($data['tempid']);
+            if ($member) {
+                $data['email'] = $member->Email;
+            }
+        }
+
+        return parent::authenticateMember($data, $message, $success, $member);
+    }
+
+    public function getLoginHandler($link)
+    {
+        return CMSLoginHandler::create($link, $this);
+    }
+
+}

src/Security/MemberAuthenticator/CMSLoginHandler.php

@@ -4,24 +4,26 @@ 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
 {
-    /**
+    private static $allowed_actions = [
-     * Login form handler method
+        'LoginForm'
-     *
+    ];
-     * This method is called when the user clicks on "Log in"
-     *
-     * @param array $data Submitted data
-     * @return HTTPResponse
-     */
-    public function dologin($data, $formHandler)
-    {
-        if ($this->performLogin($data)) {
-            return $this->logInUserAndRedirect($data);
-        }
 
-        return $this->redirectBackToForm();
+    /**
+     * Return the CMSMemberLoginForm form
+     */
+    public function loginForm()
+    {
+        return CMSMemberLoginForm::create(
+            $this,
+            get_class($this->authenticator),
+            'LoginForm'
+        );
     }
 
     public function redirectBackToForm()
@@ -75,10 +77,9 @@ PHP
     /**
      * Send user to the right location after login
      *
-     * @param array $data
      * @return HTTPResponse
      */
-    protected function logInUserAndRedirect($data, $formHandler)
+    protected function redirectAfterSuccessfulLogin()
     {
         // Check password expiry
         if (Member::currentUser()->isPasswordExpired()) {

src/Security/MemberAuthenticator/LoginForm.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Director;
+use SilverStripe\Control\RequestHandler;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\Controller;
 use SilverStripe\Forms\HiddenField;
@@ -49,7 +50,7 @@ class LoginForm extends BaseLoginForm
      * Constructor
      *
      * @skipUpgrade
-     * @param Controller $controller The parent controller, necessary to
+     * @param RequestHandler $controller The parent controller, necessary to
      *                               create the appropriate form action tag.
      * @param string $authenticatorClass Authenticator for this LoginForm
      * @param string $name The method on the controller that will return this

src/Security/MemberAuthenticator/LoginHandler.php

@@ -7,7 +7,6 @@ use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Session;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\MemberAuthenticator\Authenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\Security\Member;
 
@@ -91,7 +90,7 @@ class LoginHandler extends RequestHandler
      * This method is called when the user clicks on "Log in"
      *
      * @param array $data Submitted data
-     * @param LoginHandler $formHandler
+     * @param LoginForm $form
      * @return HTTPResponse
      */
     public function doLogin($data, $form)
@@ -223,7 +222,7 @@ class LoginHandler extends RequestHandler
      */
     public function performLogin($member, $data)
     {
-        $member->LogIn(isset($data['Remember']));
+        $member->logIn(isset($data['Remember']));
         return $member;
     }
     /**

src/Security/Security.php

@@ -238,13 +238,13 @@ class Security extends Controller implements TemplateGlobalProvider
         parent::init();
 
         // Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options
-        $frameOptions = $this->config()->get('frame_options');
+        $frameOptions = static::config()->get('frame_options');
         if ($frameOptions) {
             $this->getResponse()->addHeader('X-Frame-Options', $frameOptions);
         }
 
         // Prevent search engines from indexing the login page
-        $robotsTag = $this->config()->get('robots_tag');
+        $robotsTag = static::config()->get('robots_tag');
         if ($robotsTag) {
             $this->getResponse()->addHeader('X-Robots-Tag', $robotsTag);
         }
@@ -267,9 +267,9 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     protected function getAuthenticator($name)
     {
-        $authenticators = self::config()->authenticators;
+        $authenticators = self::config()->get('authenticators');
 
-        if (!$name) $name = 'default';
+        $name = $name ?: 'default';
 
         if (isset($authenticators[$name])) {
             return Injector::inst()->get($authenticators[$name]);
@@ -283,13 +283,20 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @return array Return an array of Authenticator objects
      */
-    public static function getAuthenticators()
+    public static function getAuthenticators($service = Authenticator::LOGIN)
     {
-        $authenticators = self::config()->authenticators;
+        $authenticators = self::config()->get('authenticators');
 
-        return array_map(function ($class) {
+        foreach($authenticators as $name => &$class) {
-            return Injector::inst()->get($class);
+            /** @var Authenticator $authenticator */
-        }, $authenticators);
+            $authenticator = Injector::inst()->get($class);
+            if($authenticator->supportedServices() & $service) {
+                $class = $authenticator;
+            } else {
+                unset($authenticators[$name]);
+            }
+        }
+        return $authenticators;
     }
 
     /**
@@ -421,22 +428,6 @@ class Security extends Controller implements TemplateGlobalProvider
         ));
     }
 
-    /**
-     * Get the login form to process according to the submitted data
-     *
-     * @return Form
-     * @throws Exception
-     */
-    public function LoginForm()
-    {
-        $authenticator = $this->getAuthenticator('default');
-        if ($authenticator) {
-            $handler = $authenticator->getLoginHandler($this->Link());
-            return $handler->handleRequest($this->request, DataModel::inst());
-        }
-        throw new Exception('Passed invalid authentication method');
-    }
-
     /**
      * Get the login forms for all available authentication methods
      *
@@ -480,6 +471,12 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Log the currently logged in user out
      *
+     * Logging out without ID-parameter in the URL, will log the user out of all applicable Authenticators.
+     *
+     * Adding an ID will only log the user out of that Authentication method.
+     *
+     * Logging out of Default will <i>always</i> completely log out the user.
+     *
      * @param bool $redirect Redirect the user back to where they came.
      *                       - If it's false, the code calling logout() is
      *                         responsible for sending the user where-ever
@@ -488,14 +485,43 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public function logout($redirect = true)
     {
+        $this->extend('beforeMemberLoggedOut');
+        $request = $this->getRequest();
         $member = Member::currentUser();
-        if ($member) {
+        // Reasoning for (now) to not go with a full LoginHandler call, is to not make it circular
-            $member->logOut();
+        // re-sending the request forward to the authenticator. In the case of logout, I think it would be
+        // overkill.
+        if (($name = $request->param('ID')) && self::hasAuthenticator($request->param('ID'))){
+            /** @var Authenticator $authenticator */
+            $authenticator = $this->getAuthenticator($request->param('ID'));
+            if($authenticator->doLogOut($member) !== true) {
+                $this->extend('failureMemberLoggedOut', $authenticator);
+                return $this->redirectBack();
+            }
+            $this->extend('successMemberLoggedOut', $authenticator);
+        } else {
+            $authenticators = static::getAuthenticators(Authenticator::LOGOUT);
+            /**
+             * @var string $name
+             * @var Authenticator $authenticator
+             */
+            foreach ($authenticators as $name => $authenticator) {
+                if ($authenticator->logOut($member) !== true) {
+                    $this->extend('failureMemberLoggedOut', $authenticator);
+                    // Break on first log out failure(?)
+                    return $this->redirectBack();
+                }
+                $this->extend('successMemberLoggedOut', $authenticator);
+            }
         }
+        // Member is successfully logged out. Write possible changes to the database.
+        $member->write();
+        $this->extend('afterMemberLoggedOut');
 
         if ($redirect && (!$this->getResponse()->isFinished())) {
             return $this->redirectBack();
         }
+
         return null;
     }
 
@@ -644,7 +670,7 @@ class Security extends Controller implements TemplateGlobalProvider
      * @return HTTPResponse|string Returns the "login" page as HTML code.
      * @throws HTTPResponse_Exception
      */
-    public function login($request)
+    public function login($request, $service = Authenticator::LOGIN)
     {
         // Check pre-login process
         if ($response = $this->preLogin()) {
@@ -654,19 +680,20 @@ class Security extends Controller implements TemplateGlobalProvider
         $link = $this->link("login");
 
         // Delegate to a single handler - Security/login/<authname>/...
-        if ($name = $request->param('ID')) {
+        if (($name = $request->param('ID')) && self::hasAuthenticator($request->param('ID'))) {
             $request->shift();
 
             $authenticator = $this->getAuthenticator($name);
-            if (!$authenticator) {
+            // @todo handle different Authenticator situations
-                throw new HTTPResponse_Exception(404, 'No authenticator "' . $name . '"');
+            if (!$authenticator->supportedServices() & $service) {
+                throw new HTTPResponse_Exception('Invalid Authenticator "' . $name . '" for login action', 418);
             }
 
             $authenticators = [ $name => $authenticator ];
 
         // Delegate to all of them, building a tabbed view - Security/login
         } else {
-            $authenticators = $this->getAuthenticators();
+            $authenticators = static::getAuthenticators($service);
         }
 
         $handlers = $authenticators;
@@ -746,7 +773,7 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     protected function delegateToHandler(RequestHandler $handler, $title, array $templates)
     {
-        $result = $handler->handleRequest($this->getRequest(), \SilverStripe\ORM\DataModel::inst());
+        $result = $handler->handleRequest($this->getRequest(), DataModel::inst());
 
         // Return the customised controller - used to render in a Form
         // Post requests are expected to be login posts, so they'll be handled downstairs
@@ -927,7 +954,7 @@ class Security extends Controller implements TemplateGlobalProvider
      * Factory method for the lost password form
      *
      * @skipUpgrade
-     * @return ChangePasswordForm Returns the lost password form
+     * @return MemberAuthenticator\ChangePasswordForm
      */
     public function ChangePasswordForm()
     {
@@ -1076,7 +1103,7 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function has_default_admin()
     {
-        return !empty(self::$default_username) && !empty(self::$default_password);
+        return !empty(self::$default_username) && !empty(self::$default_password) && (Director::get_environment_type() === 'dev');
     }
 
     /**

tests/php/Security/MemberAuthenticatorTest.php

@@ -5,6 +5,7 @@ namespace SilverStripe\Security\Tests;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Security\MemberAuthenticator\CMSAuthenticator;
 use SilverStripe\Security\PasswordEncryptor;
 use SilverStripe\Security\PasswordEncryptor_PHPHash;
 use SilverStripe\Security\Security;
@@ -89,7 +90,7 @@ class MemberAuthenticatorTest extends SapphireTest
      */
     public function testAuthenticateByTempID()
     {
-        $authenticator = new Authenticator();
+        $authenticator = new CMSAuthenticator();
 
         $member = new Member();
         $member->Email = 'test1@test.com';
@@ -186,7 +187,7 @@ class MemberAuthenticatorTest extends SapphireTest
             $dummy
         );
 
-        $this->assertTrue(Member::default_admin()->isLockedOut());
+        $this->assertFalse(Member::default_admin()->canLogin()->isValid());
         $this->assertEquals('2016-04-18 00:10:00', Member::default_admin()->LockedOutUntil);
     }
 }

tests/php/Security/MemberTest.php

@@ -1155,8 +1155,8 @@ class MemberTest extends FunctionalTest
                 'Failed to increment $member->FailedLoginCount'
             );
 
-            $this->assertFalse(
+            $this->assertTrue(
-                $member->isLockedOut(),
+                $member->canLogin()->isValid(),
                 "Member has been locked out too early"
             );
         }