Merge pull request #7800 from open-sausages/pulls/4.1/you-shall-not-password--boom

Enhancement Updated PasswordValidator to fallback to config options (ss4)

src/Security/Member.php

@@ -1659,7 +1659,8 @@ class Member extends DataObject
 
         if (!$this->ID || $this->isChanged('Password')) {
             if ($this->Password && $validator) {
-                $valid->combineAnd($validator->validate($this->Password, $this));
+                $userValid = $validator->validate($this->Password, $this);
+                $valid->combineAnd($userValid);
             }
         }
 

src/Security/PasswordValidator.php

@@ -3,7 +3,9 @@
 namespace SilverStripe\Security;
 
 use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Dev\Deprecation;
 use SilverStripe\ORM\ValidationResult;
 
 /**
@@ -22,21 +24,59 @@ class PasswordValidator
 {
     use Injectable;
     use Configurable;
+    use Extensible;
 
     /**
      * @config
      * @var array
      */
-    private static $character_strength_tests = array(
+    private static $character_strength_tests = [
         'lowercase' => '/[a-z]/',
         'uppercase' => '/[A-Z]/',
         'digits' => '/[0-9]/',
         'punctuation' => '/[^A-Za-z0-9]/',
-    );
+    ];
-
-    protected $minLength, $minScore, $testNames, $historicalPasswordCount;
 
     /**
+     * @config
+     * @var integer
+     */
+    private static $min_length = null;
+
+    /**
+     * @config
+     * @var integer
+     */
+    private static $min_test_score = null;
+
+    /**
+     * @config
+     * @var integer
+     */
+    private static $historic_count = null;
+
+    /**
+     * @var integer
+     */
+    protected $minLength = null;
+
+    /**
+     * @var integer
+     */
+    protected $minScore = null;
+
+    /**
+     * @var array
+     */
+    protected $testNames = null;
+
+    /**
+     * @var integer
+     */
+    protected $historicalPasswordCount = null;
+
+    /**
+     * @deprecated 5.0
      * Minimum password length
      *
      * @param int $minLength
@@ -44,11 +84,12 @@ class PasswordValidator
      */
     public function minLength($minLength)
     {
-        $this->minLength = $minLength;
+        Deprecation::notice('5.0', 'Use ->setMinLength($value) instead.');
-        return $this;
+        return $this->setMinLength($minLength);
     }
 
     /**
+     * @deprecated 5.0
      * Check the character strength of the password.
      *
      * Eg: $this->characterStrength(3, array("lowercase", "uppercase", "digits", "punctuation"))
@@ -57,25 +98,122 @@ class PasswordValidator
      * @param array $testNames The names of the tests to perform
      * @return $this
      */
-    public function characterStrength($minScore, $testNames)
+    public function characterStrength($minScore, $testNames = null)
     {
-        $this->minScore = $minScore;
+
-        $this->testNames = $testNames;
+        Deprecation::notice(
-        return $this;
+            '5.0',
+            'Use ->setMinTestScore($score) and ->setTextNames($names) instead.'
+        );
+        return $this->setMinTestScore($minScore)
+            ->setTestNames($testNames);
     }
 
     /**
+     * @deprecated 5.0
      * Check a number of previous passwords that the user has used, and don't let them change to that.
      *
      * @param int $count
      * @return $this
      */
     public function checkHistoricalPasswords($count)
+    {
+        Deprecation::notice('5.0', 'Use ->setHistoricCount($value) instead.');
+        return $this->setHistoricCount($count);
+    }
+
+    /**
+     * @return integer
+     */
+    public function getMinLength()
+    {
+        if ($this->minLength !== null) {
+            return $this->minLength;
+        }
+        return $this->config()->get('min_length');
+    }
+
+    /**
+     * @param $minLength
+     * @return $this
+     */
+    public function setMinLength($minLength)
+    {
+        $this->minLength = $minLength;
+        return $this;
+    }
+
+    /**
+     * @return integer
+     */
+    public function getMinTestScore()
+    {
+        if ($this->minScore !== null) {
+            return $this->minScore;
+        }
+        return $this->config()->get('min_test_score');
+    }
+
+    /**
+     * @param $minScore
+     * @return $this
+     */
+    public function setMinTestScore($minScore)
+    {
+        $this->minScore = $minScore;
+        return $this;
+    }
+
+    /**
+     * @return array
+     */
+    public function getTestNames()
+    {
+        if ($this->testNames !== null) {
+            return $this->testNames;
+        }
+        return array_keys(array_filter($this->getTests()));
+    }
+
+    /**
+     * @param $testNames
+     * @return $this
+     */
+    public function setTestNames($testNames)
+    {
+        $this->testNames = $testNames;
+        return $this;
+    }
+
+    /**
+     * @return integer
+     */
+    public function getHistoricCount()
+    {
+        if ($this->historicalPasswordCount !== null) {
+            return $this->historicalPasswordCount;
+        }
+        return $this->config()->get('historic_count');
+    }
+
+    /**
+     * @param $count
+     * @return $this
+     */
+    public function setHistoricCount($count)
     {
         $this->historicalPasswordCount = $count;
         return $this;
     }
 
+    /**
+     * @return array
+     */
+    public function getTests()
+    {
+        return $this->config()->get('character_strength_tests');
+    }
+
     /**
      * @param String $password
      * @param Member $member
@@ -85,69 +223,66 @@ class PasswordValidator
     {
         $valid = ValidationResult::create();
 
-        if ($this->minLength) {
+        $minLength = $this->getMinLength();
-            if (strlen($password) < $this->minLength) {
+        if ($minLength && strlen($password) < $minLength) {
-                $valid->addError(
+            $error = _t(
-                    _t(
+                'SilverStripe\\Security\\PasswordValidator.TOOSHORT',
-                        'SilverStripe\\Security\\PasswordValidator.TOOSHORT',
+                'Password is too short, it must be {minimum} or more characters long',
-                        'Password is too short, it must be {minimum} or more characters long',
+                ['minimum' => $this->minLength]
-                        ['minimum' => $this->minLength]
+            );
-                    ),
+
-                    'bad',
+            $valid->addError($error, 'bad', 'TOO_SHORT');
-                    'TOO_SHORT'
-                );
-            }
         }
 
-        if ($this->minScore) {
+        $minTestScore = $this->getMinTestScore();
-            $score = 0;
+        if ($minTestScore) {
-            $missedTests = array();
+            $missedTests = [];
-            foreach ($this->testNames as $name) {
+            $testNames = $this->getTestNames();
-                if (preg_match(self::config()->character_strength_tests[$name], $password)) {
+            $tests = $this->getTests();
-                    $score++;
+
-                } else {
+            foreach ($testNames as $name) {
-                    $missedTests[] = _t(
+                if (preg_match($tests[$name], $password)) {
-                        'SilverStripe\\Security\\PasswordValidator.STRENGTHTEST' . strtoupper($name),
+                    continue;
-                        $name,
-                        'The user needs to add this to their password for more complexity'
-                    );
                 }
+                $missedTests[] = _t(
+                    'SilverStripe\\Security\\PasswordValidator.STRENGTHTEST' . strtoupper($name),
+                    $name,
+                    'The user needs to add this to their password for more complexity'
+                );
             }
 
-            if ($score < $this->minScore) {
+            $score = count($this->testNames) - count($missedTests);
-                $valid->addError(
+            if ($score < $minTestScore) {
-                    _t(
+                $error = _t(
-                        'SilverStripe\\Security\\PasswordValidator.LOWCHARSTRENGTH',
+                    'SilverStripe\\Security\\PasswordValidator.LOWCHARSTRENGTH',
-                        'Please increase password strength by adding some of the following characters: {chars}',
+                    'Please increase password strength by adding some of the following characters: {chars}',
-                        ['chars' => implode(', ', $missedTests)]
+                    ['chars' => implode(', ', $missedTests)]
-                    ),
-                    'bad',
-                    'LOW_CHARACTER_STRENGTH'
                 );
+                $valid->addError($error, 'bad', 'LOW_CHARACTER_STRENGTH');
             }
         }
 
-        if ($this->historicalPasswordCount) {
+        $historicCount = $this->getHistoricCount();
+        if ($historicCount) {
             $previousPasswords = MemberPassword::get()
                 ->where(array('"MemberPassword"."MemberID"' => $member->ID))
                 ->sort('"Created" DESC, "ID" DESC')
-                ->limit($this->historicalPasswordCount);
+                ->limit($historicCount);
             /** @var MemberPassword $previousPassword */
             foreach ($previousPasswords as $previousPassword) {
                 if ($previousPassword->checkPassword($password)) {
-                    $valid->addError(
+                    $error =  _t(
-                        _t(
+                        'SilverStripe\\Security\\PasswordValidator.PREVPASSWORD',
-                            'SilverStripe\\Security\\PasswordValidator.PREVPASSWORD',
+                        'You\'ve already used that password in the past, please choose a new password'
-                            'You\'ve already used that password in the past, please choose a new password'
-                        ),
-                        'bad',
-                        'PREVIOUS_PASSWORD'
                     );
+                    $valid->addError($error, 'bad', 'PREVIOUS_PASSWORD');
                     break;
                 }
             }
         }
 
+        $this->extend('updateValidatePassword', $password, $member, $valid, $this);
+
         return $valid;
     }
 }

tests/php/Security/MemberTest.php

@@ -259,8 +259,8 @@ class MemberTest extends FunctionalTest
     public function testValidatePassword()
     {
         /**
- * @var Member $member
+         * @var Member $member
-*/
+        */
         $member = $this->objFromFixture(Member::class, 'test');
         $this->assertNotNull($member);