Merge pull request #53 from creative-commoners/pulls/2.3/configure-pa…

…sswords

move PasswordValidator configuration into _config

_config.php

@@ -12,8 +12,6 @@
 use SilverStripe\Core\Environment;
 use SilverStripe\HybridSessions\HybridSession;
 use SilverStripe\i18n\i18n;
-use SilverStripe\Security\Member;
-use SilverStripe\Security\PasswordValidator;
 
 // set the system locale to en_GB. This also means locale dropdowns
 // and date formatting etc will default to this locale. Note there is no
@@ -25,12 +23,6 @@
     Environment::setEnv('WKHTMLTOPDF_BINARY', '/usr/local/bin/wkhtmltopdf');
 }
 
-// Configure password strength requirements
-$pwdValidator = new PasswordValidator();
-$pwdValidator->setMinTestScore(3);
-$pwdValidator->setTestNames(["lowercase", "uppercase", "digits", "punctuation"]);
-Member::set_password_validator($pwdValidator);
-
 // Automatically configure session key for activedr with hybridsessions module
 if (Environment::getEnv('CWP_INSTANCE_DR_TYPE')
     && Environment::getEnv('CWP_INSTANCE_DR_TYPE') === 'active'

_config/security.yml

@@ -1,3 +1,26 @@
+---
+Name: cwppasswordstrength
+---
+# In the case someone uses `new PasswordValidator` instead of Injector, provide some safe defaults through config.
+# Test names will not be set however, as it is not configurable.
+SilverStripe\Security\PasswordValidator:
+  min_length: 10
+  min_test_score: 3
+  historic_count: 6
+# Set strength tests and requirements in line with NZISM
+# Injector is used by default for Member password validation
+SilverStripe\Core\Injector\Injector:
+  SilverStripe\Security\PasswordValidator:
+    properties:
+      MinLength: 10
+      MinTestScore: 3
+      HistoricCount: 6
+      TestNames:
+        - lowercase
+        - uppercase
+        - digits
+        - punctuation
+
 ---
 Name: cwpsecurity
 After: '#canonicalurls'

tests/PasswordStrengthTest.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace CWP\Core\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Security\Member;
+
+/**
+ * Indeed it appears to only be testing config settings, however that isn't the main goal of this minor test suite. The
+ * goal is more to catch 'regressions' should someone alter the values, given that the minimums tested here are a
+ * requirement for compliance. The tests should still pass if passwords are strengthened with more checks or higher
+ * character limits, for example. The values were previously removed due to duplication. However on inspection I could
+ * not find where they were duplicated. I assume framework defaults - however I couldn't find where they were set there
+ * either. This is merely an extra layer of assurance.
+ *
+ * E.g. the TestNames have no default in the core, and are not configurable. I didn't look too hard at mid-method
+ * fallbacks, but it seemed a logical conclusion to add this back in via the use of Injector as seen in the
+ * _config/sercurity.yml section of this PR. To ensure this is set I run the test - not because it's not a config
+ * setting, but because it's also an Integration test - the PasswordValidator is always fetched via the way it's
+ * created in use (not directly with new or only with Injector via create).
+ *
+ * This is my justification for adding this wee test suite.
+ *
+ * @group integration
+ * @group compliance
+ */
+class PasswordStrengthTest extends SapphireTest
+{
+    public function testPasswordMinLength()
+    {
+        $passwordValidator = Member::password_validator();
+        $this->assertGreaterThanOrEqual(10, $passwordValidator->getMinLength());
+    }
+
+    public function testMinTestScore()
+    {
+        $passwordValidator = Member::password_validator();
+        $this->assertGreaterThanOrEqual(3, $passwordValidator->getMinTestScore());
+    }
+
+    public function testHistoricCheckCount()
+    {
+        $passwordValidator = Member::password_validator();
+        $this->assertGreaterThanOrEqual(6, $passwordValidator->getHistoricCount());
+    }
+
+    public function testTestNamesInclude()
+    {
+        $passwordValidator = Member::password_validator();
+        $this->assertContains('lowercase', $passwordValidator->getTestNames());
+        $this->assertContains('uppercase', $passwordValidator->getTestNames());
+        $this->assertContains('digits', $passwordValidator->getTestNames());
+        $this->assertContains('punctuation', $passwordValidator->getTestNames());
+    }
+}