API Update DefaultAdmin services

API Improve validation of authentication process

docs/en/04_Changelogs/4.0.0.md

@@ -1324,7 +1324,17 @@ After (`mysite/_config/config.yml`):
   * `MODULES_PATH` removed
   * `MODULES_DIR` removed
   * `SS_HOST` removed. Use `SS_BASE_URL` instead.
-  
+  * `Member::canLogIn()` now returns boolean. Use `Member::validateCanLogin()` to get a `ValidationResult`
+* `Security` methods deprecated:
+  * `has_default_admin` use `DefaultAdminService::hasDefaultAdmin()` instead
+  * `check_default_admin` use `DefaultAdminService::isDefaultAdminCredentials()` instead
+  * `default_admin_username` use `DefaultAdminService::getDefaultAdminUsername()` instead
+  * `default_admin_password` use `DefaultAdminService::getDefaultAdminPassword()` instead
+  * `setDefaultAdmin` use `DefaultAdminService::setDefaultAdmin()` instead
+  * `clearDefaultAdmin` use `DefaultAdminService::clearDefaultAdmin()` instead
+  * `findAnAdministrator` use `DefaultAdminService::findOrCreateDefaultAdmin()` instead
+* `Member` methods deprecated:
+  * `checkPassword`. Use Authenticator::checkPassword() instead
 
 #### <a name="overview-general-removed"></a>General and Core Removed API
 

src/Dev/Install/install.php5

@@ -15,6 +15,7 @@ use SilverStripe\Dev\Install\DatabaseConfigurationHelper;
 use SilverStripe\ORM\DatabaseAdmin;
 use SilverStripe\ORM\DB;
 use SilverStripe\Security\Security;
+use SilverStripe\Security\DefaultAdminService;
 
 /**
  * SilverStripe CMS Installer
@@ -1515,7 +1516,7 @@ PHP
 
 		// Create default administrator user and group in database
 		// (not using Security::setDefaultAdmin())
-		$adminMember = Security::findAnAdministrator();
+		$adminMember = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
 		$adminMember->Email = $config['admin']['username'];
 		$adminMember->Password = $config['admin']['password'];
 		$adminMember->PasswordEncryption = Security::config()->encryption_algorithm;

src/Forms/ConfirmedPasswordField.php

@@ -5,9 +5,7 @@ namespace SilverStripe\Forms;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataObjectInterface;
 use SilverStripe\Security\Authenticator;
-use SilverStripe\Security\Member;
 use SilverStripe\Security\Security;
-use SilverStripe\View\Requirements;
 
 /**
  * Two masked input fields, checks for matching passwords.
@@ -520,8 +518,8 @@ class ConfirmedPasswordField extends FormField
             }
 
             // With a valid user and password, check the password is correct
-            $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHANGE_PASSWORD);
+            $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD);
-            foreach($authenticators as $authenticator) {
+            foreach ($authenticators as $authenticator) {
                 $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue);
                 if (!$checkResult->isValid()) {
                     $validator->validationError(

src/ORM/DataObject.php

@@ -96,10 +96,11 @@ use stdClass;
  * @todo Add instance specific removeExtension() which undos loadExtraStatics()
  *  and defineMethods()
  *
- * @property integer ID ID of the DataObject, 0 if the DataObject doesn't exist in database.
+ * @property int $ID ID of the DataObject, 0 if the DataObject doesn't exist in database.
- * @property string ClassName Class name of the DataObject
+ * @property int $OldID ID of object, if deleted
- * @property string LastEdited Date and time of DataObject's last modification.
+ * @property string $ClassName Class name of the DataObject
- * @property string Created Date and time of DataObject creation.
+ * @property string $LastEdited Date and time of DataObject's last modification.
+ * @property string $Created Date and time of DataObject creation.
  */
 class DataObject extends ViewableData implements DataObjectInterface, i18nEntityProvider, Resettable
 {

src/Security/Authenticator.php

@@ -16,13 +16,36 @@ use SilverStripe\Security\MemberAuthenticator\LogoutHandler;
  */
 interface Authenticator
 {
-
+    /**
+     * Can log a user in
+     */
     const LOGIN = 1;
+
+    /**
+     * Can log user out
+     */
     const LOGOUT = 2;
+
+    /**
+     * Can change password (check + reset)
+     */
     const CHANGE_PASSWORD = 4;
+
+    /**
+     * Can modify password
+     */
     const RESET_PASSWORD = 8;
+
+    /**
+     * In-CMS authentication
+     */
     const CMS_LOGIN = 16;
 
+    /**
+     * Can check password is valid without logging the user in or modifying the password
+     */
+    const CHECK_PASSWORD = 32;
+
     /**
      * Returns the services supported by this authenticator
      *
@@ -86,4 +109,17 @@ interface Authenticator
      * @return Member The matched member, or null if the authentication fails
      */
     public function authenticate($data, &$result = null);
+
+    /**
+     * Check if the passed password matches the stored one (if the member is not locked out).
+     *
+     * Note, we don't return early, to prevent differences in timings to give away if a member
+     * password is invalid.
+     *
+     * @param Member $member
+     * @param string $password
+     * @param ValidationResult $result
+     * @return ValidationResult
+     */
+    public function checkPassword(Member $member, $password, ValidationResult $result = null);
 }

src/Security/DefaultAdminService.php

@@ -1,69 +1,91 @@
 <?php
 
-namespace SilverStripe\Security\Service;
+namespace SilverStripe\Security;
 
+use BadMethodCallException;
+use InvalidArgumentException;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\Extensible;
-use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Core\Injector\Injectable;
-use SilverStripe\Security\Group;
-use SilverStripe\Security\Member;
-use SilverStripe\Security\Permission;
 
+/**
+ * Provides access to the default admin
+ */
 class DefaultAdminService
 {
-
     use Extensible;
     use Configurable;
+    use Injectable;
 
     /**
      * @var bool
      */
-    protected static $has_default_admin = true;
+    protected static $has_default_admin = false;
 
     /**
      * @var string
      */
-    protected static $default_username;
+    protected static $default_username = null;
 
     /**
      * @var string
      */
-    protected static $default_password;
+    protected static $default_password = null;
+
+    public function __construct()
+    {
+        $this->constructExtensions();
+    }
 
     /**
      * Set the default admin credentials
      *
      * @param string $username
      * @param string $password
-     * @return bool
      */
     public static function setDefaultAdmin($username, $password)
     {
         // don't overwrite if already set
-        if (static::$default_username || static::$default_password) {
+        if (static::hasDefaultAdmin()) {
-            throw new \LogicException('Default admin is already set', 255);
+            throw new BadMethodCallException(
+                "Default admin already exists. Use clearDefaultAdmin() first."
+            );
+        }
+
+        if (empty($username) || empty($password)) {
+            throw new InvalidArgumentException("Default admin username / password cannot be empty");
         }
 
         static::$default_username = $username;
         static::$default_password = $password;
-        static::$has_default_admin = !empty($username) && !empty($password);
+        static::$has_default_admin = true;
-
-        return true;
     }
 
     /**
      * @return string The default admin username
+     * @throws BadMethodCallException Throws exception if there is no default admin
      */
     public static function getDefaultAdminUsername()
     {
+        if (!static::hasDefaultAdmin()) {
+            throw new BadMethodCallException(
+                "No default admin configured. Please call hasDefaultAdmin() before getting default admin username"
+            );
+        }
         return static::$default_username;
     }
 
     /**
      * @return string The default admin password
+     * @throws BadMethodCallException Throws exception if there is no default admin
      */
     public static function getDefaultAdminPassword()
     {
+        if (!static::hasDefaultAdmin()) {
+            throw new BadMethodCallException(
+                "No default admin configured. Please call hasDefaultAdmin() before getting default admin password"
+            );
+        }
         return static::$default_password;
     }
 
@@ -82,24 +104,20 @@ class DefaultAdminService
      */
     public static function clearDefaultAdmin()
     {
-        self::$default_username = null;
+        static::$has_default_admin = false;
-        self::$default_password = null;
+        static::$default_username = null;
+        static::$default_password = null;
     }
 
-
     /**
-     * @return null|Member
+     * @return Member|null
      */
     public function findOrCreateDefaultAdmin()
     {
-        $this->extend('beforeFindAdministrator');
+        $this->extend('beforeFindOrCreateDefaultAdmin');
 
         // Check if we have default admins
-        if (
+        if (!static::hasDefaultAdmin()) {
-            !static::$has_default_admin ||
-            empty(static::$default_username) ||
-            empty(static::$default_password)
-        ) {
             return null;
         }
 
@@ -137,29 +155,37 @@ class DefaultAdminService
                 ->add($admin);
         }
 
-        $this->extend('afterFindAnAdministrator');
+        $this->extend('afterFindOrCreateDefaultAdmin', $admin);
 
         return $admin;
     }
 
     /**
+     * Check if the user is a default admin.
+     * Returns false if there is no default admin.
+     *
+     * @param string $username
+     * @return bool
+     */
+    public static function isDefaultAdmin($username)
+    {
+        return static::hasDefaultAdmin()
+            && $username
+            && $username === static::getDefaultAdminUsername();
+    }
+
+    /**
+     * Check if the user credentials match the default admin.
+     * Returns false if there is no default admin.
+     *
      * @param string $username
      * @param string $password
-     * @return ValidationResult
+     * @return bool
      */
-    public function validateDefaultAdmin($username, $password)
+    public static function isDefaultAdminCredentials($username, $password)
     {
-        $result = new ValidationResult();
+        return static::isDefaultAdmin($username)
-        if (
+            && $password
-            static::$default_username === $username
+            && $password === static::getDefaultAdminPassword();
-            && static::$default_password === $password
-            && static::$has_default_admin
-        ) {
-            return $result;
-        }
-
-        $result->addError('No valid default admin found');
-
-        return $result;
     }
-}
+}

src/Security/Group.php

@@ -33,14 +33,14 @@ use SilverStripe\ORM\UnsavedRelationList;
 /**
  * A security group.
  *
- * @property string Title Name of the group
+ * @property string $Title Name of the group
- * @property string Description Description of the group
+ * @property string $Description Description of the group
- * @property string Code Group code
+ * @property string $Code Group code
- * @property string Locked Boolean indicating whether group is locked in security panel
+ * @property string $Locked Boolean indicating whether group is locked in security panel
- * @property int Sort
+ * @property int $Sort
  * @property string HtmlEditorConfig
  *
- * @property int ParentID ID of parent group
+ * @property int $ParentID ID of parent group
  *
  * @method Group Parent() Return parent group
  * @method HasManyList Permissions() List of group permissions

src/Security/Member.php

@@ -19,6 +19,8 @@ use SilverStripe\Forms\DropdownField;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\Forms\ListboxField;
+use SilverStripe\Forms\Tab;
+use SilverStripe\Forms\TabSet;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataList;
@@ -31,7 +33,6 @@ use SilverStripe\ORM\Map;
 use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\Service\DefaultAdminService;
 
 /**
  * The member class which represents the users of the system
@@ -260,89 +261,43 @@ class Member extends DataObject
     {
         parent::requireDefaultRecords();
         // Default groups should've been built by Group->requireDefaultRecords() already
-        $service = Injector::inst()->get(DefaultAdminService::class);
+        $service = DefaultAdminService::singleton();
-
         $service->findOrCreateDefaultAdmin();
     }
 
     /**
      * Get the default admin record if it exists, or creates it otherwise if enabled
      *
+     * @deprecated 4.0.0...5.0.0 Use DefaultAdminService::findOrCreateDefaultAdmin() instead
      * @return Member
      */
     public static function default_admin()
     {
-        // Check if set
+        Deprecation::notice('5.0', 'Use DefaultAdminService::findOrCreateDefaultAdmin() instead');
-        if (!Security::has_default_admin()) {
+        return DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
-            return null;
-        }
-
-        // Find or create ADMIN group
-        Group::singleton()->requireDefaultRecords();
-        $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
-
-        // Find member
-        /** @skipUpgrade */
-        $admin = static::get()
-            ->filter('Email', Security::default_admin_username())
-            ->first();
-        if (!$admin) {
-            // 'Password' is not set to avoid creating
-            // 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->Email = Security::default_admin_username();
-            $admin->write();
-        }
-
-        // Ensure this user is in the admin group
-        if (!$admin->inGroup($adminGroup)) {
-            // Add member to group instead of adding group to member
-            // This bypasses the privilege escallation code in Member_GroupSet
-            $adminGroup
-                ->DirectMembers()
-                ->add($admin);
-        }
-
-        return $admin;
     }
 
     /**
      * Check if the passed password matches the stored one (if the member is not locked out).
      *
-     * @param  string $password
+     * @deprecated 4.0.0...5.0.0 Use Authenticator::checkPassword() instead
+     *
+     * @param string $password
      * @return ValidationResult
      */
     public function checkPassword($password)
     {
-        $result = $this->canLogIn();
+        Deprecation::notice('5.0', 'Use Authenticator::checkPassword() instead');
 
-        // Short-circuit the result upon failure, no further checks needed.
+        // With a valid user and password, check the password is correct
-        if (!$result->isValid()) {
+        $result = ValidationResult::create();
-            return $result;
+        $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD);
+        foreach ($authenticators as $authenticator) {
+            $authenticator->checkPassword($this, $password, $result);
+            if (!$result->isValid()) {
+                break;
+            }
         }
-
-        // Allow default admin to login as self
-        if ($this->isDefaultAdmin() && Security::check_default_admin($this->Email, $password)) {
-            return $result;
-        }
-
-        // 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.'));
-
-            return $result;
-        }
-
-        $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
-        if (!$e->check($this->Password, $password, $this->Salt, $this)) {
-            $result->addError(_t(
-                __CLASS__ . '.ERRORWRONGCRED',
-                'The provided details don\'t seem to be correct. Please try again.'
-            ));
-        }
-
         return $result;
     }
 
@@ -353,8 +308,17 @@ class Member extends DataObject
      */
     public function isDefaultAdmin()
     {
-        return Security::has_default_admin()
+        return DefaultAdminService::isDefaultAdmin($this->Email);
-            && $this->Email === Security::default_admin_username();
+    }
+
+    /**
+     * Check if this user can login
+     *
+     * @return bool
+     */
+    public function canLogin()
+    {
+        return $this->validateCanLogin()->isValid();
     }
 
     /**
@@ -363,12 +327,12 @@ class Member extends DataObject
      *
      * You can hook into this with a "canLogIn" method on an attached extension.
      *
+     * @param ValidationResult $result Optional result to add errors to
      * @return ValidationResult
      */
-    public function canLogIn()
+    public function validateCanLogin(ValidationResult $result = null)
     {
-        $result = ValidationResult::create();
+        $result = $result ?: ValidationResult::create();
-
         if ($this->isLockedOut()) {
             $result->addError(
                 _t(
@@ -397,7 +361,9 @@ class Member extends DataObject
             return false;
         }
 
-        return DBDatetime::now()->getTimestamp() < $this->dbObject('LockedOutUntil')->getTimestamp();
+        /** @var DBDatetime $lockedOutUntil */
+        $lockedOutUntil = $this->dbObject('LockedOutUntil');
+        return DBDatetime::now()->getTimestamp() < $lockedOutUntil->getTimestamp();
     }
 
     /**
@@ -1421,8 +1387,12 @@ class Member extends DataObject
     public function getCMSFields()
     {
         $this->beforeUpdateCMSFields(function (FieldList $fields) {
+            /** @var TabSet $rootTabSet */
+            $rootTabSet = $fields->fieldByName("Root");
+            /** @var Tab $mainTab */
+            $mainTab = $rootTabSet->fieldByName("Main");
             /** @var FieldList $mainFields */
-            $mainFields = $fields->fieldByName("Root")->fieldByName("Main")->getChildren();
+            $mainFields = $mainTab->getChildren();
 
             // Build change password field
             $mainFields->replaceField('Password', $this->getMemberPasswordField());
@@ -1482,7 +1452,7 @@ class Member extends DataObject
                 }
             }
 
-            $permissionsTab = $fields->fieldByName("Root")->fieldByName('Permissions');
+            $permissionsTab = $rootTabSet->fieldByName('Permissions');
             if ($permissionsTab) {
                 $permissionsTab->addExtraClass('readonly');
             }

src/Security/MemberAuthenticator/CMSMemberAuthenticator.php

@@ -6,6 +6,9 @@ use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Authenticator as BaseAuthenticator;
 use SilverStripe\Security\Member;
 
+/**
+ * Provides authentication for the user within the CMS
+ */
 class CMSMemberAuthenticator extends MemberAuthenticator
 {
 

src/Security/MemberAuthenticator/ChangePasswordHandler.php

@@ -200,11 +200,8 @@ class ChangePasswordHandler extends RequestHandler
     {
         $member = Security::getCurrentUser();
         // The user was logged in, check the current password
-        if ($member && (
+        $oldPassword = isset($data['OldPassword']) ? $data['OldPassword'] : null;
-                empty($data['OldPassword']) ||
+        if ($member && !$this->checkPassword($member, $oldPassword)) {
-                !$member->checkPassword($data['OldPassword'])->isValid()
-            )
-        ) {
             $form->sessionMessage(
                 _t(
                     'SilverStripe\\Security\\Member.ERRORPASSWORDNOTMATCH',
@@ -274,8 +271,10 @@ class ChangePasswordHandler extends RequestHandler
         $member->AutoLoginExpired = DBDatetime::create()->now();
         $member->write();
 
-        if ($member->canLogIn()->isValid()) {
+        if ($member->canLogIn()) {
-            Injector::inst()->get(IdentityStore::class)->logIn($member, false, $this->getRequest());
+            /** @var IdentityStore $identityStore */
+            $identityStore = Injector::inst()->get(IdentityStore::class);
+            $identityStore->logIn($member, false, $this->getRequest());
         }
 
         // TODO Add confirmation message to login redirect
@@ -305,4 +304,26 @@ class ChangePasswordHandler extends RequestHandler
 
         return $this->redirect($url);
     }
+
+    /**
+     * Check if password is ok
+     *
+     * @param Member $member
+     * @param string $password
+     * @return bool
+     */
+    protected function checkPassword($member, $password)
+    {
+        if (empty($password)) {
+            return false;
+        }
+        // With a valid user and password, check the password is correct
+        $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD);
+        foreach ($authenticators as $authenticator) {
+            if (!$authenticator->checkPassword($member, $password)->isValid()) {
+                return false;
+            }
+        }
+        return true;
+    }
 }

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -6,13 +6,13 @@ use InvalidArgumentException;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Session;
 use SilverStripe\Core\Extensible;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\LoginAttempt;
 use SilverStripe\Security\Member;
+use SilverStripe\Security\PasswordEncryptor;
 use SilverStripe\Security\Security;
-use SilverStripe\Security\Service\DefaultAdminService;
+use SilverStripe\Security\DefaultAdminService;
 
 /**
  * Authenticator for the default "member" method
@@ -28,7 +28,7 @@ class MemberAuthenticator implements Authenticator
     {
         // Bitwise-OR of all the supported services in this Authenticator, to make a bitmask
         return Authenticator::LOGIN | Authenticator::LOGOUT | Authenticator::CHANGE_PASSWORD
-            | Authenticator::RESET_PASSWORD;
+            | Authenticator::RESET_PASSWORD | Authenticator::CHECK_PASSWORD;
     }
 
     /**
@@ -62,26 +62,24 @@ class MemberAuthenticator implements Authenticator
     protected function authenticateMember($data, &$result = null, $member = null)
     {
         $email = !empty($data['Email']) ? $data['Email'] : null;
-        // Default success to false
+        $result = $result ?: ValidationResult::create();
-        $result = new ValidationResult();
 
         // Check default login (see Security::setDefaultAdmin())
-        $asDefaultAdmin = $email === DefaultAdminService::getDefaultAdminUsername();
+        $asDefaultAdmin = DefaultAdminService::isDefaultAdmin($email);
         if ($asDefaultAdmin) {
             // If logging is as default admin, ensure record is setup correctly
-            /** @var Member $member */
+            $member = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
-            $service = Injector::inst()->get(DefaultAdminService::class);
+            $member->validateCanLogin($result);
-            $member = $service->findOrCreateDefaultAdmin();
+            if ($result->isValid()) {
-            $validAdmin = $service->validateDefaultAdmin($email, $data['Password']);
+                // Check if default admin credentials are correct
-            $result = $member->canLogIn();
+                if (DefaultAdminService::isDefaultAdminCredentials($email, $data['Password'])) {
-            //protect against failed login
+                    return $member;
-            if ($validAdmin->isValid() && $result->isValid()) {
+                } else {
-                return $member;
+                    $result->addError(_t(
-            } else {
+                        'SilverStripe\\Security\\Member.ERRORWRONGCRED',
-                $result->addError(_t(
+                        "The provided details don't seem to be correct. Please try again."
-                    'SilverStripe\\Security\\Member.ERRORWRONGCRED',
+                    ));
-                    "The provided details don't seem to be correct. Please try again."
+                }
-                ));
             }
         }
 
@@ -97,7 +95,7 @@ class MemberAuthenticator implements Authenticator
 
         // Validate against member if possible
         if ($member && !$asDefaultAdmin) {
-            $result = $member->checkPassword($data['Password']);
+            $this->checkPassword($member, $data['Password'], $result);
         }
 
         // Emit failure to member and form (if available)
@@ -105,17 +103,15 @@ class MemberAuthenticator implements Authenticator
             if ($member) {
                 $member->registerFailedLogin();
             }
+        } elseif ($member) {
+            $member->registerSuccessfulLogin();
         } else {
-            if ($member) {
+            // A non-existing member occurred. This will make the result "valid" so let's invalidate
-                $member->registerSuccessfulLogin();
+            $result->addError(_t(
-            } else {
+                'SilverStripe\\Security\\Member.ERRORWRONGCRED',
-                // A non-existing member occurred. This will make the result "valid" so let's invalidate
+                "The provided details don't seem to be correct. Please try again."
-                $result->addError(_t(
+            ));
-                    'SilverStripe\\Security\\Member.ERRORWRONGCRED',
+            return null;
-                    "The provided details don't seem to be correct. Please try again."
-                ));
-                $member = null;
-            }
         }
 
         return $member;
@@ -128,12 +124,22 @@ class MemberAuthenticator implements Authenticator
      * password is invalid.
      *
      * @param Member $member
-     * @param  string $password
+     * @param string $password
+     * @param ValidationResult $result
      * @return ValidationResult
      */
-    public function checkPassword($member, $password)
+    public function checkPassword(Member $member, $password, ValidationResult $result = null)
     {
-        $result = $member->canLogIn();
+        // Check if allowed to login
+        $result = $member->validateCanLogin($result);
+        if (!$result->isValid()) {
+            return $result;
+        }
+
+        // Allow default admin to login as self
+        if (DefaultAdminService::isDefaultAdminCredentials($member->Email, $password)) {
+            return $result;
+        }
 
         // Check a password is set on this member
         if (empty($member->Password) && $member->exists()) {

src/Security/MemberPassword.php

@@ -7,10 +7,10 @@ use SilverStripe\ORM\DataObject;
 /**
  * Keep track of users' previous passwords, so that we can check that new passwords aren't changed back to old ones.
  *
- * @property string Password
+ * @property string $Password
- * @property string Salt
+ * @property string $Salt
- * @property string PasswordEncryption
+ * @property string $PasswordEncryption
- * @property int MemberID ID of the Member
+ * @property int $MemberID ID of the Member
  * @method Member Member() Owner of the password
  */
 class MemberPassword extends DataObject
@@ -21,9 +21,9 @@ class MemberPassword extends DataObject
         'PasswordEncryption' => 'Varchar(50)',
     );
 
-    private static $has_one = array(
+    private static $has_one = [
-        'Member' => 'SilverStripe\\Security\\Member'
+        'Member' => Member::class,
-    );
+    ];
 
     private static $table_name = "MemberPassword";
 
@@ -47,12 +47,12 @@ class MemberPassword extends DataObject
      * Check if the given password is the same as the one stored in this record.
      * See {@link Member->checkPassword()}.
      *
-     * @param String $password Cleartext password
+     * @param string $password Cleartext password
-     * @return Boolean
+     * @return bool
      */
     public function checkPassword($password)
     {
-        $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
+        $encryptor = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
-        return $e->check($this->Password, $password, $this->Salt, $this->Member());
+        return $encryptor->check($this->Password, $password, $this->Salt, $this->Member());
     }
 }

src/Security/Security.php

@@ -25,7 +25,7 @@ use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBHTMLText;
 use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\Service\DefaultAdminService;
+use SilverStripe\Security\DefaultAdminService;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\SSViewer;
 use SilverStripe\View\TemplateGlobalProvider;
@@ -270,7 +270,7 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public function getApplicableAuthenticators($service = Authenticator::LOGIN)
     {
-        $authenticators = $this->authenticators;
+        $authenticators = $this->getAuthenticators();
 
         /** @var Authenticator $authenticator */
         foreach ($authenticators as $name => $authenticator) {
@@ -279,6 +279,10 @@ class Security extends Controller implements TemplateGlobalProvider
             }
         }
 
+        if (empty($authenticators)) {
+            throw new LogicException('No applicable authenticators found');
+        }
+
         return $authenticators;
     }
 
@@ -942,21 +946,20 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @return Member
      *
-     * @deprecated 5.0.0 Please use DefaultAdminService::findOrCreateDefaultAdmin()
+     * @deprecated 4.0.0..5.0.0 Please use DefaultAdminService::findOrCreateDefaultAdmin()
      */
     public static function findAnAdministrator()
     {
         Deprecation::notice('5.0.0', 'Please use DefaultAdminService::findOrCreateDefaultAdmin()');
 
-        $service = Injector::inst()->get(DefaultAdminService::class);
+        $service = DefaultAdminService::singleton();
-
         return $service->findOrCreateDefaultAdmin();
     }
 
     /**
      * Flush the default admin credentials
      *
-     * @deprecated 5.0.0 Please use DefaultAdminService::clearDefaultAdmin()
+     * @deprecated 4.0.0..5.0.0 Please use DefaultAdminService::clearDefaultAdmin()
      */
     public static function clear_default_admin()
     {
@@ -978,13 +981,14 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $password The password (in cleartext)
      * @return bool True if successfully set
      *
-     * @deprecated 5.0.0 Please use DefaultAdminService::setDefaultAdmin($username, $password)
+     * @deprecated 4.0.0..5.0.0 Please use DefaultAdminService::setDefaultAdmin($username, $password)
      */
     public static function setDefaultAdmin($username, $password)
     {
         Deprecation::notice('5.0.0', 'Please use DefaultAdminService::setDefaultAdmin($username, $password)');
 
-        return DefaultAdminService::setDefaultAdmin($username, $password);
+        DefaultAdminService::setDefaultAdmin($username, $password);
+        return true;
     }
 
     /**
@@ -995,19 +999,20 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $password
      * @return bool
      *
-     * @deprecated 5.0.0
+     * @deprecated 4.0.0..5.0.0 Use DefaultAdminService::isDefaultAdminCredentials() instead
      */
     public static function check_default_admin($username, $password)
     {
-        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::validateDefaultAdmin($username, $password)');
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::isDefaultAdminCredentials($username, $password)');
 
-        $service = Injector::inst()->get(DefaultAdminService::class);
+        /** @var DefaultAdminService $service */
-
+        return DefaultAdminService::isDefaultAdminCredentials($username, $password);
-        return $service->validateDefaultAdmin($username, $password)->isValid();
     }
 
     /**
      * Check that the default admin account has been set.
+     *
+     * @deprecated 4.0.0..5.0.0 Use DefaultAdminService::hasDefaultAdmin() instead
      */
     public static function has_default_admin()
     {
@@ -1019,18 +1024,20 @@ class Security extends Controller implements TemplateGlobalProvider
     /**
      * Get default admin username
      *
+     * @deprecated 4.0.0..5.0.0 Use DefaultAdminService::getDefaultAdminUsername()
      * @return string
      */
     public static function default_admin_username()
     {
         Deprecation::notice('5.0.0', 'Please use DefaultAdminService::getDefaultAdminUsername()');
-        
+
         return DefaultAdminService::getDefaultAdminUsername();
     }
 
     /**
      * Get default admin password
      *
+     * @deprecated 4.0.0..5.0.0 Use DefaultAdminService::getDefaultAdminPassword()
      * @return string
      */
     public static function default_admin_password()
@@ -1074,16 +1081,16 @@ class Security extends Controller implements TemplateGlobalProvider
             $algorithm = self::config()->get('password_encryption_algorithm');
         }
 
-        $e = PasswordEncryptor::create_for_algorithm($algorithm);
+        $encryptor = PasswordEncryptor::create_for_algorithm($algorithm);
 
         // New salts will only need to be generated if the password is hashed for the first time
-        $salt = ($salt) ? $salt : $e->salt($password);
+        $salt = ($salt) ? $salt : $encryptor->salt($password);
 
         return array(
-            'password'  => $e->encrypt($password, $salt, $member),
+            'password'  => $encryptor->encrypt($password, $salt, $member),
             'salt'      => $salt,
             'algorithm' => $algorithm,
-            'encryptor' => $e
+            'encryptor' => $encryptor
         );
     }
 

src/conf/ConfigureFromEnv.php

@@ -46,16 +46,14 @@
  *  - SS_SEND_ALL_EMAILS_FROM: If you set this define, all emails will be send from this address.
  */
 
-use Monolog\Logger;
 use Monolog\Handler\StreamHandler;
+use Monolog\Logger;
 use Psr\Log\LoggerInterface;
 use SilverStripe\Control\Email\Email;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\Install\DatabaseAdapterRegistry;
 use SilverStripe\Security\BasicAuth;
-use SilverStripe\Security\Security;
+use SilverStripe\Security\DefaultAdminService;
-use SilverStripe\Security\Service\DefaultAdminService;
 
 global $database;
 

tests/php/Security/MemberAuthenticatorTest.php

@@ -5,6 +5,7 @@ namespace SilverStripe\Security\Tests;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\DataModel;
 use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator;
 use SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm;
@@ -16,7 +17,7 @@ use SilverStripe\Security\IdentityStore;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Security\Service\DefaultAdminService;
+use SilverStripe\Security\DefaultAdminService;
 
 class MemberAuthenticatorTest extends SapphireTest
 {
@@ -30,8 +31,14 @@ class MemberAuthenticatorTest extends SapphireTest
     {
         parent::setUp();
 
-        $this->defaultUsername = DefaultAdminService::getDefaultAdminUsername();
+        if (DefaultAdminService::hasDefaultAdmin()) {
-        $this->defaultPassword = DefaultAdminService::getDefaultAdminPassword();
+            $this->defaultUsername = DefaultAdminService::getDefaultAdminUsername();
+            $this->defaultPassword = DefaultAdminService::getDefaultAdminPassword();
+            DefaultAdminService::clearDefaultAdmin();
+        } else {
+            $this->defaultUsername = null;
+            $this->defaultPassword = null;
+        }
         DefaultAdminService::clearDefaultAdmin();
         DefaultAdminService::setDefaultAdmin('admin', 'password');
     }
@@ -39,21 +46,20 @@ class MemberAuthenticatorTest extends SapphireTest
     protected function tearDown()
     {
         DefaultAdminService::clearDefaultAdmin();
-        DefaultAdminService::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
+        if ($this->defaultUsername) {
+            DefaultAdminService::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
+        }
         parent::tearDown();
     }
 
     public function testCustomIdentifierField()
     {
+        Member::config()->set('unique_identifier_field', 'Username');
 
-        $origField = Member::config()->unique_identifier_field;
+        $label = Member::singleton()
-        Member::config()->unique_identifier_field = 'Username';
+            ->fieldLabel(Member::config()->get('unique_identifier_field'));
-
-        $label=singleton(Member::class)->fieldLabel(Member::config()->unique_identifier_field);
 
         $this->assertEquals($label, 'Username');
-
-        Member::config()->unique_identifier_field = $origField;
     }
 
     public function testGenerateLoginForm()
@@ -108,6 +114,7 @@ class MemberAuthenticatorTest extends SapphireTest
         $this->assertNotEmpty($tempID);
 
         // Test correct login
+        /** @var ValidationResult $message */
         $result = $authenticator->authenticate(
             array(
             'tempid' => $tempID,
@@ -145,6 +152,7 @@ class MemberAuthenticatorTest extends SapphireTest
         $authenticator = new MemberAuthenticator();
 
         // Test correct login
+        /** @var ValidationResult $message */
         $result = $authenticator->authenticate(
             array(
             'Email' => 'admin',
@@ -176,8 +184,8 @@ class MemberAuthenticatorTest extends SapphireTest
     {
         $authenticator = new MemberAuthenticator();
 
-        Config::inst()->update(Member::class, 'lock_out_after_incorrect_logins', 1);
+        Config::modify()->set(Member::class, 'lock_out_after_incorrect_logins', 1);
-        Config::inst()->update(Member::class, 'lock_out_delay_mins', 10);
+        Config::modify()->set(Member::class, 'lock_out_delay_mins', 10);
         DBDatetime::set_mock_now('2016-04-18 00:00:00');
 
         // Test correct login
@@ -188,7 +196,9 @@ class MemberAuthenticatorTest extends SapphireTest
             ]
         );
 
-        $this->assertFalse(Member::default_admin()->canLogin()->isValid());
+        $defaultAdmin = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
-        $this->assertEquals('2016-04-18 00:10:00', Member::default_admin()->LockedOutUntil);
+        $this->assertNotNull($defaultAdmin);
+        $this->assertFalse($defaultAdmin->canLogin());
+        $this->assertEquals('2016-04-18 00:10:00', $defaultAdmin->LockedOutUntil);
     }
 }

tests/php/Security/MemberTest.php

@@ -2,26 +2,27 @@
 
 namespace SilverStripe\Security\Tests;
 
+use SilverStripe\Control\Cookie;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\FunctionalTest;
-use SilverStripe\Control\Cookie;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
-use SilverStripe\Security\Member;
+use SilverStripe\ORM\ValidationException;
-use SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler;
-use SilverStripe\Security\Security;
-use SilverStripe\Security\MemberPassword;
 use SilverStripe\Security\Group;
-use SilverStripe\Security\Permission;
 use SilverStripe\Security\IdentityStore;
-use SilverStripe\Security\PasswordEncryptor_Blowfish;
+use SilverStripe\Security\Member;
-use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\Security\Member_Validator;
+use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
+use SilverStripe\Security\MemberAuthenticator\SessionAuthenticationHandler;
+use SilverStripe\Security\MemberPassword;
+use SilverStripe\Security\PasswordEncryptor_Blowfish;
+use SilverStripe\Security\Permission;
+use SilverStripe\Security\RememberLoginHash;
+use SilverStripe\Security\Security;
 use SilverStripe\Security\Tests\MemberTest\FieldsExtension;
-use SilverStripe\Control\HTTPRequest;
 
 class MemberTest extends FunctionalTest
 {
@@ -50,24 +51,13 @@ class MemberTest extends FunctionalTest
     {
         parent::setUp();
 
-        $this->orig['Member_unique_identifier_field'] = Member::config()->unique_identifier_field;
+        Member::config()->set('unique_identifier_field', 'Email');
-        Member::config()->unique_identifier_field = 'Email';
         Member::set_password_validator(null);
     }
 
-    protected function tearDown()
-    {
-        Member::config()->unique_identifier_field = $this->orig['Member_unique_identifier_field'];
-        parent::tearDown();
-    }
-
-
-
-    /**
-     * @expectedException \SilverStripe\ORM\ValidationException
-     */
     public function testWriteDoesntMergeNewRecordWithExistingMember()
     {
+        $this->expectException(ValidationException::class);
         $m1 = new Member();
         $m1->Email = 'member@test.com';
         $m1->write();
@@ -101,7 +91,7 @@ class MemberTest extends FunctionalTest
         $memberWithPassword->write();
         $this->assertEquals(
             $memberWithPassword->PasswordEncryption,
-            Security::config()->password_encryption_algorithm,
+            Security::config()->get('password_encryption_algorithm'),
             'Password encryption is set for new member records on first write (with setting "Password")'
         );
 
@@ -120,8 +110,7 @@ class MemberTest extends FunctionalTest
         $member->PasswordEncryption = 'sha1_v2.4';
         $member->write();
 
-        $origAlgo = Security::config()->password_encryption_algorithm;
+        Security::config()->set('password_encryption_algorithm', 'none');
-        Security::config()->password_encryption_algorithm = 'none';
 
         $member->Password = 'mynewpassword';
         $member->write();
@@ -130,10 +119,9 @@ class MemberTest extends FunctionalTest
             $member->PasswordEncryption,
             'sha1_v2.4'
         );
-        $result = $member->checkPassword('mynewpassword');
+        $auth = new MemberAuthenticator();
+        $result = $auth->checkPassword($member, 'mynewpassword');
         $this->assertTrue($result->isValid());
-
-        Security::config()->password_encryption_algorithm = $origAlgo;
     }
 
     public function testKeepsEncryptionOnEmptyPasswords()
@@ -150,16 +138,19 @@ class MemberTest extends FunctionalTest
             $member->PasswordEncryption,
             'sha1_v2.4'
         );
-        $result = $member->checkPassword('');
+        $auth = new MemberAuthenticator();
+        $result = $auth->checkPassword($member, '');
         $this->assertTrue($result->isValid());
     }
 
     public function testSetPassword()
     {
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $member->Password = "test1";
         $member->write();
-        $result = $member->checkPassword('test1');
+        $auth = new MemberAuthenticator();
+        $result = $auth->checkPassword($member, 'test1');
         $this->assertTrue($result->isValid());
     }
 
@@ -168,6 +159,7 @@ class MemberTest extends FunctionalTest
      */
     public function testPasswordChangeLogging()
     {
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);
         $member->Password = "test1";
@@ -179,7 +171,7 @@ class MemberTest extends FunctionalTest
         $member->Password = "test3";
         $member->write();
 
-        $passwords = DataObject::get("SilverStripe\\Security\\MemberPassword", "\"MemberID\" = $member->ID", "\"Created\" DESC, \"ID\" DESC")
+        $passwords = DataObject::get(MemberPassword::class, "\"MemberID\" = $member->ID", "\"Created\" DESC, \"ID\" DESC")
             ->getIterator();
         $this->assertNotNull($passwords);
         $passwords->rewind();
@@ -215,6 +207,7 @@ class MemberTest extends FunctionalTest
 
         $this->clearEmails();
 
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);
         $valid = $member->changePassword('32asDF##$$%%');
@@ -236,6 +229,7 @@ class MemberTest extends FunctionalTest
         $this->clearEmails();
         $this->autoFollowRedirection = false;
 
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);
 
@@ -353,8 +347,9 @@ class MemberTest extends FunctionalTest
      */
     public function testPasswordExpirySetting()
     {
-        Member::config()->password_expiry_days = 90;
+        Member::config()->set('password_expiry_days', 90);
 
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);
         $valid = $member->changePassword("Xx?1234234");
@@ -363,7 +358,7 @@ class MemberTest extends FunctionalTest
         $expiryDate = date('Y-m-d', time() + 90*86400);
         $this->assertEquals($expiryDate, $member->PasswordExpiry);
 
-        Member::config()->password_expiry_days = null;
+        Member::config()->set('password_expiry_days', null);
         $valid = $member->changePassword("Xx?1234235");
         $this->assertTrue($valid->isValid());
 
@@ -372,6 +367,7 @@ class MemberTest extends FunctionalTest
 
     public function testIsPasswordExpired()
     {
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);
         $this->assertFalse($member->isPasswordExpired());
@@ -394,14 +390,13 @@ class MemberTest extends FunctionalTest
     }
     public function testInGroups()
     {
+        /** @var Member $staffmember */
         $staffmember = $this->objFromFixture(Member::class, 'staffmember');
-        $managementmember = $this->objFromFixture(Member::class, 'managementmember');
+        /** @var Member $ceomember */
-        $accountingmember = $this->objFromFixture(Member::class, 'accountingmember');
         $ceomember = $this->objFromFixture(Member::class, 'ceomember');
 
         $staffgroup = $this->objFromFixture(Group::class, 'staffgroup');
         $managementgroup = $this->objFromFixture(Group::class, 'managementgroup');
-        $accountinggroup = $this->objFromFixture(Group::class, 'accountinggroup');
         $ceogroup = $this->objFromFixture(Group::class, 'ceogroup');
 
         $this->assertTrue(
@@ -420,7 +415,9 @@ class MemberTest extends FunctionalTest
 
     public function testAddToGroupByCode()
     {
+        /** @var Member $grouplessMember */
         $grouplessMember = $this->objFromFixture(Member::class, 'grouplessmember');
+        /** @var Group $memberlessGroup */
         $memberlessGroup = $this->objFromFixture(Group::class, 'memberlessgroup');
 
         $this->assertFalse($grouplessMember->Groups()->exists());
@@ -434,6 +431,7 @@ class MemberTest extends FunctionalTest
         $grouplessMember->addToGroupByCode('somegroupthatwouldneverexist', 'New Group');
         $this->assertEquals($grouplessMember->Groups()->count(), 2);
 
+        /** @var Group $group */
         $group = DataObject::get_one(
             Group::class,
             array(
@@ -447,7 +445,9 @@ class MemberTest extends FunctionalTest
 
     public function testRemoveFromGroupByCode()
     {
+        /** @var Member $grouplessMember */
         $grouplessMember = $this->objFromFixture(Member::class, 'grouplessmember');
+        /** @var Group $memberlessGroup */
         $memberlessGroup = $this->objFromFixture(Group::class, 'memberlessgroup');
 
         $this->assertFalse($grouplessMember->Groups()->exists());
@@ -461,6 +461,7 @@ class MemberTest extends FunctionalTest
         $grouplessMember->addToGroupByCode('somegroupthatwouldneverexist', 'New Group');
         $this->assertEquals($grouplessMember->Groups()->count(), 2);
 
+        /** @var Group $group */
         $group = DataObject::get_one(Group::class, "\"Code\" = 'somegroupthatwouldneverexist'");
         $this->assertNotNull($group);
         $this->assertEquals($group->Code, 'somegroupthatwouldneverexist');
@@ -476,14 +477,20 @@ class MemberTest extends FunctionalTest
 
     public function testInGroup()
     {
+        /** @var Member $staffmember */
         $staffmember = $this->objFromFixture(Member::class, 'staffmember');
+        /** @var Member $managementmember */
         $managementmember = $this->objFromFixture(Member::class, 'managementmember');
+        /** @var Member $accountingmember */
         $accountingmember = $this->objFromFixture(Member::class, 'accountingmember');
+        /** @var Member $ceomember */
         $ceomember = $this->objFromFixture(Member::class, 'ceomember');
 
+        /** @var Group $staffgroup */
         $staffgroup = $this->objFromFixture(Group::class, 'staffgroup');
+        /** @var Group $managementgroup */
         $managementgroup = $this->objFromFixture(Group::class, 'managementgroup');
-        $accountinggroup = $this->objFromFixture(Group::class, 'accountinggroup');
+        /** @var Group $ceogroup */
         $ceogroup = $this->objFromFixture(Group::class, 'ceogroup');
 
         $this->assertTrue(
@@ -614,6 +621,7 @@ class MemberTest extends FunctionalTest
      */
     public function testName()
     {
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $member->setName('Test Some User');
         $this->assertEquals('Test Some User', $member->getName());
@@ -645,8 +653,11 @@ class MemberTest extends FunctionalTest
 
     public function testOnChangeGroups()
     {
+        /** @var Group $staffGroup */
         $staffGroup = $this->objFromFixture(Group::class, 'staffgroup');
+        /** @var Member $staffMember */
         $staffMember = $this->objFromFixture(Member::class, 'staffmember');
+        /** @var Member $adminMember */
         $adminMember = $this->objFromFixture(Member::class, 'admin');
         $newAdminGroup = new Group(array('Title' => 'newadmin'));
         $newAdminGroup->write();
@@ -685,7 +696,9 @@ class MemberTest extends FunctionalTest
      */
     public function testOnChangeGroupsByAdd()
     {
+        /** @var Member $staffMember */
         $staffMember = $this->objFromFixture(Member::class, 'staffmember');
+        /** @var Member $adminMember */
         $adminMember = $this->objFromFixture(Member::class, 'admin');
 
         // Setup new admin group
@@ -736,6 +749,7 @@ class MemberTest extends FunctionalTest
      */
     public function testOnChangeGroupsBySetIDList()
     {
+        /** @var Member $staffMember */
         $staffMember = $this->objFromFixture(Member::class, 'staffmember');
 
         // Setup new admin group
@@ -865,7 +879,7 @@ class MemberTest extends FunctionalTest
         $m2 = new Member();
         $m2->PasswordEncryption = 'blowfish';
         $m2->Salt = $enc->salt('456');
-        $m2Token = $m2->generateAutologinTokenAndStoreHash();
+        $m2->generateAutologinTokenAndStoreHash();
 
         $this->assertTrue($m1->validateAutoLoginToken($m1Token), 'Passes token validity test against matching member.');
         $this->assertFalse($m2->validateAutoLoginToken($m1Token), 'Fails token validity test against other member.');
@@ -873,12 +887,14 @@ class MemberTest extends FunctionalTest
 
     public function testRememberMeHashGeneration()
     {
+        /** @var Member $m1 */
         $m1 = $this->objFromFixture(Member::class, 'grouplessmember');
 
         Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
         $hashes = RememberLoginHash::get()->filter('MemberID', $m1->ID);
         $this->assertEquals($hashes->count(), 1);
+        /** @var RememberLoginHash $firstHash */
         $firstHash = $hashes->first();
         $this->assertNotNull($firstHash->DeviceID);
         $this->assertNotNull($firstHash->Hash);
@@ -893,6 +909,7 @@ class MemberTest extends FunctionalTest
 
         Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
+        /** @var RememberLoginHash $firstHash */
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
         $this->assertNotNull($firstHash);
 
@@ -966,11 +983,10 @@ class MemberTest extends FunctionalTest
 
     public function testExpiredRememberMeHashAutologin()
     {
-        /**
+        /** @var Member $m1 */
- * @var Member $m1
-*/
         $m1 = $this->objFromFixture(Member::class, 'noexpiry');
         Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
+        /** @var RememberLoginHash $firstHash */
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
         $this->assertNotNull($firstHash);
 
@@ -1026,6 +1042,7 @@ class MemberTest extends FunctionalTest
 
     public function testRememberMeMultipleDevices()
     {
+        /** @var Member $m1 */
         $m1 = $this->objFromFixture(Member::class, 'noexpiry');
 
         // First device
@@ -1035,10 +1052,12 @@ class MemberTest extends FunctionalTest
         Injector::inst()->get(IdentityStore::class)->logIn($m1, true);
 
         // Hash of first device
+        /** @var RememberLoginHash $firstHash */
         $firstHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->first();
         $this->assertNotNull($firstHash);
 
         // Hash of second device
+        /** @var RememberLoginHash $secondHash */
         $secondHash = RememberLoginHash::get()->filter('MemberID', $m1->ID)->last();
         $this->assertNotNull($secondHash);
 
@@ -1093,7 +1112,7 @@ class MemberTest extends FunctionalTest
 
         // Logging out from the second device - only one device being logged out
         RememberLoginHash::config()->update('logout_across_devices', false);
-        $response = $this->get(
+        $this->get(
             'Security/logout',
             $this->session(),
             null,
@@ -1110,7 +1129,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);
-        $response = $this->get('Security/logout', $this->session());
+        $this->get('Security/logout', $this->session());
         $this->assertEquals(
             RememberLoginHash::get()->filter('MemberID', $m1->ID)->count(),
             0
@@ -1152,6 +1171,7 @@ class MemberTest extends FunctionalTest
         //set up the config variables to enable login lockouts
         Member::config()->update('lock_out_after_incorrect_logins', $maxFailedLoginsAllowed);
 
+        /** @var Member $member */
         $member = $this->objFromFixture(Member::class, 'test');
         $failedLoginCount = $member->FailedLoginCount;
 
@@ -1165,7 +1185,7 @@ class MemberTest extends FunctionalTest
             );
 
             $this->assertTrue(
-                $member->canLogin()->isValid(),
+                $member->canLogin(),
                 "Member has been locked out too early"
             );
         }
@@ -1175,7 +1195,9 @@ class MemberTest extends FunctionalTest
     {
         // clear custom requirements for this test
         Member_Validator::config()->update('customRequired', null);
+        /** @var Member $memberA */
         $memberA = $this->objFromFixture(Member::class, 'admin');
+        /** @var Member $memberB */
         $memberB = $this->objFromFixture(Member::class, 'test');
 
         // create a blank form

tests/php/Security/SecurityDefaultAdminTest.php

@@ -2,19 +2,17 @@
 
 namespace SilverStripe\Security\Tests;
 
-use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Security\Security;
-use SilverStripe\Security\Permission;
-use SilverStripe\Security\Member;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Security\Service\DefaultAdminService;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\Permission;
+use SilverStripe\Security\DefaultAdminService;
 
 class SecurityDefaultAdminTest extends SapphireTest
 {
-
     protected $usesDatabase = true;
 
     protected $defaultUsername = null;
+
     protected $defaultPassword = null;
 
     protected function setUp()
@@ -28,34 +26,41 @@ class SecurityDefaultAdminTest extends SapphireTest
         }
         self::empty_temp_db();
 
-        $this->defaultUsername = Security::default_admin_username();
+        if (DefaultAdminService::hasDefaultAdmin()) {
-        $this->defaultPassword = Security::default_admin_password();
+            $this->defaultUsername = DefaultAdminService::getDefaultAdminUsername();
-        Security::clear_default_admin();
+            $this->defaultPassword = DefaultAdminService::getDefaultAdminPassword();
-        Security::setDefaultAdmin('admin', 'password');
+            DefaultAdminService::clearDefaultAdmin();
+        } else {
+            $this->defaultUsername = null;
+            $this->defaultPassword = null;
+        }
+        DefaultAdminService::setDefaultAdmin('admin', 'password');
         Permission::reset();
     }
 
     protected function tearDown()
     {
-        Security::clear_default_admin();
+        DefaultAdminService::clearDefaultAdmin();
-        Security::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
+        if ($this->defaultUsername) {
+            DefaultAdminService::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
+        }
         Permission::reset();
         parent::tearDown();
     }
 
     public function testCheckDefaultAdmin()
     {
-        $this->assertTrue(Security::has_default_admin());
+        $this->assertTrue(DefaultAdminService::hasDefaultAdmin());
         $this->assertTrue(
-            Security::check_default_admin('admin', 'password'),
+            DefaultAdminService::isDefaultAdminCredentials('admin', 'password'),
             'Succeeds with correct username and password'
         );
         $this->assertFalse(
-            Security::check_default_admin('wronguser', 'password'),
+            DefaultAdminService::isDefaultAdminCredentials('wronguser', 'password'),
             'Fails with incorrect username'
         );
         $this->assertFalse(
-            Security::check_default_admin('admin', 'wrongpassword'),
+            DefaultAdminService::isDefaultAdminCredentials('admin', 'wrongpassword'),
             'Fails with incorrect password'
         );
     }
@@ -65,33 +70,34 @@ class SecurityDefaultAdminTest extends SapphireTest
         $adminMembers = Permission::get_members_by_permission('ADMIN');
         $this->assertEquals(0, $adminMembers->count());
 
-        $admin = Security::findAnAdministrator();
+        $admin = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
 
         $this->assertInstanceOf(Member::class, $admin);
         $this->assertTrue(Permission::checkMember($admin, 'ADMIN'));
-        $this->assertEquals($admin->Email, Security::default_admin_username());
+        $this->assertEquals($admin->Email, DefaultAdminService::getDefaultAdminUsername());
+        $this->assertTrue(DefaultAdminService::isDefaultAdmin($admin->Email));
         $this->assertNull($admin->Password);
     }
 
     public function testFindAnAdministratorWithoutDefaultAdmin()
     {
-        $service = Injector::inst()->get(DefaultAdminService::class);
         // Clear default admin
+        $service = DefaultAdminService::singleton();
         DefaultAdminService::clearDefaultAdmin();
 
         $adminMembers = Permission::get_members_by_permission('ADMIN');
         $this->assertEquals(0, $adminMembers->count());
 
         $admin = $service->findOrCreateDefaultAdmin();
-
         $this->assertNull($admin);
+
         // When clearing the admin, it will not re-instate it anymore
         DefaultAdminService::setDefaultAdmin('admin', 'password');
-        $admin = $service->findAnAdministrator();
+        $admin = $service->findOrCreateDefaultAdmin();
         $this->assertTrue(Permission::checkMember($admin, 'ADMIN'));
 
-        // User should be blank
+        // User should have Email but no Password
-        $this->assertEmpty($admin->Email);
+        $this->assertEquals('admin', $admin->Email);
         $this->assertEmpty($admin->Password);
     }
 
@@ -100,11 +106,11 @@ class SecurityDefaultAdminTest extends SapphireTest
         $adminMembers = Permission::get_members_by_permission('ADMIN');
         $this->assertEquals(0, $adminMembers->count());
 
-        $admin = Member::default_admin();
+        $admin = DefaultAdminService::singleton()->findOrCreateDefaultAdmin();
-
         $this->assertInstanceOf(Member::class, $admin);
         $this->assertTrue(Permission::checkMember($admin, 'ADMIN'));
-        $this->assertEquals($admin->Email, Security::default_admin_username());
+        $this->assertEquals($admin->Email, DefaultAdminService::getDefaultAdminUsername());
+        $this->assertTrue(DefaultAdminService::isDefaultAdmin($admin->Email));
         $this->assertNull($admin->Password);
     }
 }

tests/php/Security/SecurityTest.php

@@ -31,14 +31,6 @@ class SecurityTest extends FunctionalTest
 
     protected $autoFollowRedirection = false;
 
-    protected $priorAuthenticators = array();
-
-    protected $priorDefaultAuthenticator = null;
-
-    protected $priorUniqueIdentifierField = null;
-
-    protected $priorRememberUsername = null;
-
     protected static $extra_controllers = [
         SecurityTest\NullController::class,
         SecurityTest\SecuredController::class,
@@ -50,9 +42,6 @@ class SecurityTest extends FunctionalTest
         Config::modify()->set(MemberAuthenticator::class, 'authenticators', []);
         Config::modify()->set(MemberAuthenticator::class, 'default_authenticator', MemberAuthenticator::class);
 
-        // And that the unique identified field is 'Email'
-        $this->priorUniqueIdentifierField = Member::config()->unique_identifier_field;
-        $this->priorRememberUsername = Security::config()->remember_username;
         /**
          * @skipUpgrade
          */
@@ -63,21 +52,6 @@ class SecurityTest extends FunctionalTest
         Config::modify()->merge('SilverStripe\\Control\\Director', 'alternate_base_url', '/');
     }
 
-    protected function tearDown()
-    {
-        // Restore selected authenticator
-
-        // MemberAuthenticator might not actually be present
-        // Config::modify()->set(Authenticator::class, 'authenticators', $this->priorAuthenticators);
-        // Config::modify()->set(Authenticator::class, 'default_authenticator', $this->priorDefaultAuthenticator);
-
-        // Restore unique identifier field
-        Member::config()->unique_identifier_field = $this->priorUniqueIdentifierField;
-        Security::config()->remember_username = $this->priorRememberUsername;
-
-        parent::tearDown();
-    }
-
     public function testAccessingAuthenticatedPageRedirectsToLoginForm()
     {
         $this->autoFollowRedirection = false;