Skip to content

Commit

Permalink
move PasswordValidator configuration into _config
Browse files Browse the repository at this point in the history
and out of _config.php
i.e. use Injector rather than proceedural boot time code, as
SilverStripe 4 supports this (where 3 did not).
Re-introduce some security level settings that were lost at some point
with no clear reason as to why.
  • Loading branch information
Dylan Wagstaff authored and robbieaverill committed Nov 8, 2018
1 parent 597980d commit a990d37
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 8 deletions.
8 changes: 0 additions & 8 deletions _config.php
Expand Up @@ -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
Expand All @@ -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'
Expand Down
23 changes: 23 additions & 0 deletions _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'
Expand Down
55 changes: 55 additions & 0 deletions 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());
}
}

0 comments on commit a990d37

Please sign in to comment.