Remove DefaultAdmin things from Security and Member into the MemberAuthenticator, unifying and removing duplicate code.

sh.exe.stackdump

@@ -0,0 +1,4 @@
+Stack trace:
+Frame     Function  Args
+0081BF88  6106D69F (00000000, 01010000, 00000000, 00000190)
+End of stack trace

src/Forms/ConfirmedPasswordField.php

@@ -4,6 +4,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;
@@ -519,17 +520,20 @@ class ConfirmedPasswordField extends FormField
             }
 
             // With a valid user and password, check the password is correct
-            $checkResult = $member->checkPassword($this->currentPasswordValue);
+            $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHANGE_PASSWORD);
-            if (!$checkResult->isValid()) {
+            foreach($authenticators as $authenticator) {
-                $validator->validationError(
+                $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue);
-                    $name,
+                if (!$checkResult->isValid()) {
-                    _t(
+                    $validator->validationError(
-                        'SilverStripe\\Forms\\ConfirmedPasswordField.CURRENT_PASSWORD_ERROR',
+                        $name,
-                        "The current password you have entered is not correct."
+                        _t(
-                    ),
+                            'SilverStripe\\Forms\\ConfirmedPasswordField.CURRENT_PASSWORD_ERROR',
-                    "validation"
+                            "The current password you have entered is not correct."
-                );
+                        ),
-                return false;
+                        "validation"
+                    );
+                    return false;
+                }
             }
         }
 

src/Security/Member.php

@@ -31,6 +31,7 @@ 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
@@ -259,7 +260,9 @@ class Member extends DataObject
     {
         parent::requireDefaultRecords();
         // Default groups should've been built by Group->requireDefaultRecords() already
-        static::default_admin();
+        $service = Injector::inst()->get(DefaultAdminService::class);
+
+        $service->findOrCreateDefaultAdmin();
     }
 
     /**

src/Security/MemberAuthenticator/CMSMemberAuthenticator.php

@@ -27,7 +27,7 @@ class CMSMemberAuthenticator extends MemberAuthenticator
             // 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;
+                $data['Email'] = $member->Email;
             }
         }
 

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -5,11 +5,14 @@ namespace SilverStripe\Security\MemberAuthenticator;
 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\Security;
+use SilverStripe\Security\Service\DefaultAdminService;
 
 /**
  * Authenticator for the default "member" method
@@ -19,6 +22,7 @@ use SilverStripe\Security\Security;
  */
 class MemberAuthenticator implements Authenticator
 {
+    use Extensible;
 
     public function supportedServices()
     {
@@ -53,23 +57,25 @@ class MemberAuthenticator implements Authenticator
      * @param array $data Form submitted data
      * @param ValidationResult $result
      * @param Member|null This third parameter is used in the CMSAuthenticator(s)
-     * @return Member Found member, regardless of successful login
+     * @return Member|null Found member, regardless of successful login
      */
     protected function authenticateMember($data, &$result = null, $member = null)
     {
-        // Default success to false
         $email = !empty($data['Email']) ? $data['Email'] : null;
+        // Default success to false
         $result = new ValidationResult();
 
         // Check default login (see Security::setDefaultAdmin())
-        $asDefaultAdmin = $email === Security::default_admin_username();
+        $asDefaultAdmin = $email === DefaultAdminService::getDefaultAdminUsername();
         if ($asDefaultAdmin) {
             // If logging is as default admin, ensure record is setup correctly
-            $member = Member::default_admin();
+            /** @var Member $member */
-            $success = Security::check_default_admin($email, $data['Password']);
+            $service = Injector::inst()->get(DefaultAdminService::class);
+            $member = $service->findOrCreateDefaultAdmin();
+            $validAdmin = $service->validateDefaultAdmin($email, $data['Password']);
             $result = $member->canLogIn();
             //protect against failed login
-            if ($success && $result->isValid()) {
+            if ($validAdmin->isValid() && $result->isValid()) {
                 return $member;
             } else {
                 $result->addError(_t(
@@ -82,9 +88,10 @@ class MemberAuthenticator implements Authenticator
         // Attempt to identify user by email
         if (!$member && $email) {
             // Find user by email
+            $identifierField = Member::config()->get('unique_identifier_field');
             /** @var Member $member */
             $member = Member::get()
-                ->filter([Member::config()->get('unique_identifier_field') => $email])
+                ->filter([$identifierField => $email])
                 ->first();
         }
 
@@ -114,6 +121,37 @@ class MemberAuthenticator implements Authenticator
         return $member;
     }
 
+    /**
+     * 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
+     * @return ValidationResult
+     */
+    public function checkPassword($member, $password)
+    {
+        $result = $member->canLogIn();
+
+        // Check a password is set on this member
+        if (empty($member->Password) && $member->exists()) {
+            $result->addError(_t(__CLASS__ . '.NoPassword', 'There is no password on this member.'));
+        }
+
+        $encryptor = PasswordEncryptor::create_for_algorithm($member->PasswordEncryption);
+        if (!$encryptor->check($member->Password, $password, $member->Salt, $member)) {
+            $result->addError(_t(
+                __CLASS__ . '.ERRORWRONGCRED',
+                'The provided details don\'t seem to be correct. Please try again.'
+            ));
+        }
+
+        return $result;
+    }
+
+
     /**
      * Log login attempt
      * TODO We could handle this with an extension
@@ -142,7 +180,7 @@ class MemberAuthenticator implements Authenticator
             $attempt->Status = 'Success';
 
             // Audit logging hook
-            $member->extend('authenticated');
+            $member->extend('authenticationSucceeded');
         } else {
             // Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
             $attempt->Status = 'Failure';

src/Security/Security.php

@@ -25,10 +25,10 @@ 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\View\ArrayData;
 use SilverStripe\View\SSViewer;
 use SilverStripe\View\TemplateGlobalProvider;
-use Subsite;
 
 /**
  * Implements a basic security model
@@ -47,22 +47,6 @@ class Security extends Controller implements TemplateGlobalProvider
         'ping',
     );
 
-    /**
-     * Default user name. {@link setDefaultAdmin()}
-     *
-     * @var string
-     * @see setDefaultAdmin()
-     */
-    protected static $default_username;
-
-    /**
-     * Default password. {@link setDefaultAdmin()}
-     *
-     * @var string
-     * @see setDefaultAdmin()
-     */
-    protected static $default_password;
-
     /**
      * If set to TRUE to prevent sharing of the session across several sites
      * in the domain.
@@ -957,57 +941,28 @@ class Security extends Controller implements TemplateGlobalProvider
      * purposes outside of any default credentials set through {@link Security::setDefaultAdmin()}.
      *
      * @return Member
+     *
+     * @deprecated 5.0.0 Please use DefaultAdminService::findOrCreateDefaultAdmin()
      */
     public static function findAnAdministrator()
     {
-        static::singleton()->extend('beforeFindAdministrator');
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::findOrCreateDefaultAdmin()');
 
-        /** @var Member $member */
+        $service = Injector::inst()->get(DefaultAdminService::class);
-        $member = null;
 
-        // find a group with ADMIN permission
+        return $service->findOrCreateDefaultAdmin();
-        $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
-
-        if (!$adminGroup) {
-            Group::singleton()->requireDefaultRecords();
-            $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
-        }
-
-        $member = $adminGroup->Members()->First();
-
-        if (!$member) {
-            Member::singleton()->requireDefaultRecords();
-            $member = Permission::get_members_by_permission('ADMIN')->first();
-        }
-
-        if (!$member) {
-            $member = Member::default_admin();
-        }
-
-        if (!$member) {
-            // Failover to a blank admin
-            $member = Member::create();
-            $member->FirstName = _t('SilverStripe\\Security\\Member.DefaultAdminFirstname', 'Default Admin');
-            $member->write();
-            // Add member to group instead of adding group to member
-            // This bypasses the privilege escallation code in Member_GroupSet
-            $adminGroup
-                ->DirectMembers()
-                ->add($member);
-        }
-
-        static::singleton()->extend('afterFindAdministrator');
-
-        return $member;
     }
 
     /**
      * Flush the default admin credentials
+     *
+     * @deprecated 5.0.0 Please use DefaultAdminService::clearDefaultAdmin()
      */
     public static function clear_default_admin()
     {
-        self::$default_username = null;
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::clearDefaultAdmin()');
-        self::$default_password = null;
+
+        DefaultAdminService::clearDefaultAdmin();
     }
 
 
@@ -1022,18 +977,14 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $username The user name
      * @param string $password The password (in cleartext)
      * @return bool True if successfully set
+     *
+     * @deprecated 5.0.0 Please use DefaultAdminService::setDefaultAdmin($username, $password)
      */
     public static function setDefaultAdmin($username, $password)
     {
-        // don't overwrite if already set
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::setDefaultAdmin($username, $password)');
-        if (self::$default_username || self::$default_password) {
-            return false;
-        }
 
-        self::$default_username = $username;
+        return DefaultAdminService::setDefaultAdmin($username, $password);
-        self::$default_password = $password;
-
-        return true;
     }
 
     /**
@@ -1043,14 +994,16 @@ class Security extends Controller implements TemplateGlobalProvider
      * @param string $username
      * @param string $password
      * @return bool
+     *
+     * @deprecated 5.0.0
      */
     public static function check_default_admin($username, $password)
     {
-        return (
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::validateDefaultAdmin($username, $password)');
-            self::$default_username === $username
+
-            && self::$default_password === $password
+        $service = Injector::inst()->get(DefaultAdminService::class);
-            && self::has_default_admin()
+
-        );
+        return $service->validateDefaultAdmin($username, $password)->isValid();
     }
 
     /**
@@ -1058,7 +1011,9 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function has_default_admin()
     {
-        return !empty(self::$default_username) && !empty(self::$default_password);
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::hasDefaultAdmin()');
+
+        return DefaultAdminService::hasDefaultAdmin();
     }
 
     /**
@@ -1068,7 +1023,9 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function default_admin_username()
     {
-        return self::$default_username;
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::getDefaultAdminUsername()');
+        
+        return DefaultAdminService::getDefaultAdminUsername();
     }
 
     /**
@@ -1078,7 +1035,9 @@ class Security extends Controller implements TemplateGlobalProvider
      */
     public static function default_admin_password()
     {
-        return self::$default_password;
+        Deprecation::notice('5.0.0', 'Please use DefaultAdminService::getDefaultAdminPassword()');
+
+        return DefaultAdminService::getDefaultAdminPassword();
     }
 
     /**

src/Security/Service/DefaultAdminService.php

@@ -0,0 +1,165 @@
+<?php
+
+namespace SilverStripe\Security\Service;
+
+use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Extensible;
+use SilverStripe\ORM\ValidationResult;
+use SilverStripe\Security\Group;
+use SilverStripe\Security\Member;
+use SilverStripe\Security\Permission;
+
+class DefaultAdminService
+{
+
+    use Extensible;
+    use Configurable;
+
+    /**
+     * @var bool
+     */
+    protected static $has_default_admin = true;
+
+    /**
+     * @var string
+     */
+    protected static $default_username;
+
+    /**
+     * @var string
+     */
+    protected static $default_password;
+
+    /**
+     * 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) {
+            throw new \LogicException('Default admin is already set', 255);
+        }
+
+        static::$default_username = $username;
+        static::$default_password = $password;
+        static::$has_default_admin = !empty($username) && !empty($password);
+
+        return true;
+    }
+
+    /**
+     * @return string The default admin username
+     */
+    public static function getDefaultAdminUsername()
+    {
+        return static::$default_username;
+    }
+
+    /**
+     * @return string The default admin password
+     */
+    public static function getDefaultAdminPassword()
+    {
+        return static::$default_password;
+    }
+
+    /**
+     * Check if there is a default admin
+     *
+     * @return bool
+     */
+    public static function hasDefaultAdmin()
+    {
+        return static::$has_default_admin;
+    }
+
+    /**
+     * Flush the default admin credentials
+     */
+    public static function clearDefaultAdmin()
+    {
+        self::$default_username = null;
+        self::$default_password = null;
+    }
+
+
+    /**
+     * @return null|Member
+     */
+    public function findOrCreateDefaultAdmin()
+    {
+        $this->extend('beforeFindAdministrator');
+
+        // Check if we have default admins
+        if (
+            !static::$has_default_admin ||
+            empty(static::$default_username) ||
+            empty(static::$default_password)
+        ) {
+            return null;
+        }
+
+        // Find or create ADMIN group
+        Group::singleton()->requireDefaultRecords();
+        $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
+
+        if (!$adminGroup) {
+            Group::singleton()->requireDefaultRecords();
+            $adminGroup = Permission::get_groups_by_permission('ADMIN')->first();
+        }
+
+        // Find member
+        /** @skipUpgrade */
+        $admin = Member::get()
+            ->filter('Email', static::getDefaultAdminUsername())
+            ->first();
+        // If no admin is found, create one
+        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 = static::getDefaultAdminUsername();
+            $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);
+        }
+
+        $this->extend('afterFindAnAdministrator');
+
+        return $admin;
+    }
+
+    /**
+     * @param string $username
+     * @param string $password
+     * @return ValidationResult
+     */
+    public function validateDefaultAdmin($username, $password)
+    {
+        $result = new ValidationResult();
+        if (
+            static::$default_username === $username
+            && static::$default_password === $password
+            && static::$has_default_admin
+        ) {
+            return $result;
+        }
+
+        $result->addError('No valid default admin found');
+
+        return $result;
+    }
+}

src/conf/ConfigureFromEnv.php

@@ -51,9 +51,11 @@ use Monolog\Handler\StreamHandler;
 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\Service\DefaultAdminService;
 
 global $database;
 
@@ -131,7 +133,7 @@ if ($defaultAdminUser = getenv('SS_DEFAULT_ADMIN_USERNAME')) {
             E_USER_ERROR
         );
     } else {
-        Security::setDefaultAdmin($defaultAdminUser, $defaultAdminPass);
+        DefaultAdminService::setDefaultAdmin($defaultAdminUser, $defaultAdminPass);
     }
 }
 if ($useBasicAuth = getenv('SS_USE_BASIC_AUTH')) {

tests/php/Security/MemberAuthenticatorTest.php

@@ -16,6 +16,7 @@ use SilverStripe\Security\IdentityStore;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Security\Service\DefaultAdminService;
 
 class MemberAuthenticatorTest extends SapphireTest
 {
@@ -29,15 +30,16 @@ class MemberAuthenticatorTest extends SapphireTest
     {
         parent::setUp();
 
-        $this->defaultUsername = Security::default_admin_username();
+        $this->defaultUsername = DefaultAdminService::getDefaultAdminUsername();
-        $this->defaultPassword = Security::default_admin_password();
+        $this->defaultPassword = DefaultAdminService::getDefaultAdminPassword();
-        Security::clear_default_admin();
+        DefaultAdminService::clearDefaultAdmin();
-        Security::setDefaultAdmin('admin', 'password');
+        DefaultAdminService::setDefaultAdmin('admin', 'password');
     }
 
     protected function tearDown()
     {
-        Security::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
+        DefaultAdminService::clearDefaultAdmin();
+        DefaultAdminService::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
         parent::tearDown();
     }
 
@@ -151,7 +153,7 @@ class MemberAuthenticatorTest extends SapphireTest
             $message
         );
         $this->assertNotEmpty($result);
-        $this->assertEquals($result->Email, Security::default_admin_username());
+        $this->assertEquals($result->Email, DefaultAdminService::getDefaultAdminUsername());
         $this->assertTrue($message->isValid());
 
         // Test incorrect login

tests/php/Security/SecurityDefaultAdminTest.php

@@ -2,10 +2,12 @@
 
 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;
 
 class SecurityDefaultAdminTest extends SapphireTest
 {
@@ -35,6 +37,7 @@ class SecurityDefaultAdminTest extends SapphireTest
 
     protected function tearDown()
     {
+        Security::clear_default_admin();
         Security::setDefaultAdmin($this->defaultUsername, $this->defaultPassword);
         Permission::reset();
         parent::tearDown();
@@ -72,15 +75,19 @@ class SecurityDefaultAdminTest extends SapphireTest
 
     public function testFindAnAdministratorWithoutDefaultAdmin()
     {
+        $service = Injector::inst()->get(DefaultAdminService::class);
         // Clear default admin
-        Security::clear_default_admin();
+        DefaultAdminService::clearDefaultAdmin();
 
         $adminMembers = Permission::get_members_by_permission('ADMIN');
         $this->assertEquals(0, $adminMembers->count());
 
-        $admin = Security::findAnAdministrator();
+        $admin = $service->findOrCreateDefaultAdmin();
 
-        $this->assertInstanceOf(Member::class, $admin);
+        $this->assertNull($admin);
+        // When clearing the admin, it will not re-instate it anymore
+        DefaultAdminService::setDefaultAdmin('admin', 'password');
+        $admin = $service->findAnAdministrator();
         $this->assertTrue(Permission::checkMember($admin, 'ADMIN'));
 
         // User should be blank