Member.lock_out_delay_mins configurable, password security docs

docs/en/topics/security.md

@@ -407,13 +407,37 @@ configuration and test fixtures).
 You should therefore block access to all yaml files (extension .yml) by default, and white list only yaml files
 you need to serve directly.
 
-See [Apache](/installation/webserver) and [Nginx](/installation/nginx) installation documentation for details 
+See [Apache](/installation/webserver) and [Nginx](/installation/nginx) installation documentation for details  specific to your web server
-specific to your web server
+
+## Passwords
+
+SilverStripe stores passwords with a strong hashing algorithm (blowfish) by default
+(see [api:PasswordEncryptor]). It adds randomness to these hashes via
+salt values generated with the strongest entropy generators available on the platform
+(see [api:RandomGenerator]). This prevents brute force attacks with
+[Rainbow tables](http://en.wikipedia.org/wiki/Rainbow_table).
+
+Strong passwords are a crucial part of any system security.
+So in addition to storing the password in a secure fashion,
+you can also enforce specific password policies by configuring
+a [api:PasswordValidator]:
+
+	:::php
+	$validator = new PasswordValidator();
+	$validator->minLength(7);
+	$validator->checkHistoricalPasswords(6);
+	$validator->characterStrength('lowercase','uppercase','digits','punctuation');
+	Member::set_password_validator($validator);
+
+In addition, you can tighten password security with the following configuration settings:
+
+ * `Member.password_expiry_days`: Set the number of days that a password should be valid for.
+ * `Member.lock_out_after_incorrect_logins`: Number of incorrect logins after which
+    the user is blocked from further attempts for the timespan defined in `$lock_out_delay_mins`
+ * `Member.lock_out_delay_mins`: Minutes of enforced lockout after incorrect password attempts.
+ 		Only applies if `lock_out_after_incorrect_logins` is greater than 0.
 
 ##  Related
 
  * [http://silverstripe.org/security-releases/](http://silverstripe.org/security-releases/)
-
-## Links
-
  * [Best-practices for securing MySQL (securityfocus.com)](http://www.securityfocus.com/infocus/1726)

lang/en.yml

@@ -367,6 +367,7 @@ en:
     EMPTYNEWPASSWORD: 'The new password can''t be empty, please try again'
     ENTEREMAIL: 'Please enter an email address to get a password reset link.'
     ERRORLOCKEDOUT: 'Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in 20 minutes.'
+    ERRORLOCKEDOUT2: 'Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in {count} minutes.'
     ERRORNEWPASSWORD: 'You have entered your new password differently, try again'
     ERRORPASSWORDNOTMATCH: 'Your current password does not match, please try again'
     ERRORWRONGCRED: 'That doesn''t seem to be the right e-mail address or password. Please try again.'

security/Member.php

@@ -113,10 +113,19 @@ class Member extends DataObject implements TemplateGlobalProvider {
 
 	/**
 	 * @config
-	 * @var Int
+	 * @var Int Number of incorrect logins after which
+	 * the user is blocked from further attempts for the timespan 
+	 * defined in {@link $lock_out_delay_mins}. 
 	 */
 	private static $lock_out_after_incorrect_logins = null;
 
+	/**
+	 * @config
+	 * @var integer Minutes of enforced lockout after incorrect password attempts.
+	 * Only applies if {@link $lock_out_after_incorrect_logins} greater than 0.
+	 */
+	private static $lock_out_delay_mins = 15;
+	
 	/**
 	 * @config
 	 * @var String If this is set, then a session cookie with the given name will be set on log-in,
@@ -238,11 +247,15 @@ class Member extends DataObject implements TemplateGlobalProvider {
 		$result = new ValidationResult();
 
 		if($this->isLockedOut()) {
-			$result->error(_t (
+			$result->error(
-				'Member.ERRORLOCKEDOUT',
+				_t(
+					'Member.ERRORLOCKEDOUT2',
 					'Your account has been temporarily disabled because of too many failed attempts at ' .
-				'logging in. Please try again in 20 minutes.'
+					'logging in. Please try again in {count} minutes.',
-			));
+					null,
+					array('count' => $this->config()->lock_out_delay_mins)
+				)
+			);
 		}
 
 		$this->extend('canLogIn', $result);
@@ -1407,7 +1420,8 @@ class Member extends DataObject implements TemplateGlobalProvider {
 			$this->write();
 	
 			if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
-				$this->LockedOutUntil = date('Y-m-d H:i:s', time() + 15*60);
+				$lockoutMins = self::config()->lock_out_delay_mins;
+				$this->LockedOutUntil = date('Y-m-d H:i:s', time() + $lockoutMins*60);
 				$this->write();
 			}
 		}

tests/security/SecurityTest.php

@@ -250,61 +250,76 @@ class SecurityTest extends FunctionalTest {
 		i18n::set_locale('en_US');
 
 		Member::config()->lock_out_after_incorrect_logins = 5;
+		Member::config()->lock_out_delay_mins = 15;
 		
-		/* LOG IN WITH A BAD PASSWORD 7 TIMES */
+		// Login with a wrong password for more than the defined threshold
-
+		for($i = 1; $i <= Member::config()->lock_out_after_incorrect_logins+1; $i++) {
-		for($i=1;$i<=7;$i++) {
 			$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
 			$member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
 			
-			/* THE FIRST 4 TIMES, THE MEMBER SHOULDN'T BE LOCKED OUT */
+			if($i < Member::config()->lock_out_after_incorrect_logins) {
-			if($i < 5) {
+				$this->assertNull(
-				$this->assertNull($member->LockedOutUntil);
+					$member->LockedOutUntil,
+					'User does not have a lockout time set if under threshold for failed attempts'
+				);
 				$this->assertContains($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED'));
+			} else {
+				// Fuzzy matching for time to avoid side effects from slow running tests
+				$this->assertGreaterThan(
+					time() + 14*60, 
+					strtotime($member->LockedOutUntil),
+					'User has a lockout time set after too many failed attempts'
+				);
 			}
 
-			/* AFTER THAT THE USER IS LOCKED OUT FOR 15 MINUTES */
+			$msg = _t(
-
+				'Member.ERRORLOCKEDOUT2',
-			//(we check for at least 14 minutes because we don't want a slow running test to report a failure.)
+				'Your account has been temporarily disabled because of too many failed attempts at ' .
-			else {
+				'logging in. Please try again in {count} minutes.',
-				$this->assertGreaterThan(time() + 14*60, strtotime($member->LockedOutUntil));
+				null,
-			}
+				array('count' => Member::config()->lock_out_delay_mins)
-			
+			);			
-			if($i > 5) {
+			if($i > Member::config()->lock_out_after_incorrect_logins) {
-				$this->assertContains(_t('Member.ERRORLOCKEDOUT'), $this->loginErrorMessage());
+				$this->assertContains($msg, $this->loginErrorMessage());
-				// $this->assertTrue(false !== stripos($this->loginErrorMessage(), _t('Member.ERRORLOCKEDOUT')));
 			}
 		}
 		
-		/* THE USER CAN'T LOG IN NOW, EVEN IF THEY GET THE RIGHT PASSWORD */
-		
 		$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
-		$this->assertNull($this->session()->inst_get('loggedInAs'));
+		$this->assertNull(
-		
+			$this->session()->inst_get('loggedInAs'),
-		/* BUT, IF TIME PASSES, THEY CAN LOG IN */
+			'The user can\'t log in after being locked out, even with the right password'
+		);
 		
 		// (We fake this by re-setting LockedOutUntil)
 		$member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
 		$member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30);
 		$member->write();
-		
 		$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
-		$this->assertEquals($this->session()->inst_get('loggedInAs'), $member->ID);
+		$this->assertEquals(
+			$this->session()->inst_get('loggedInAs'), 
+			$member->ID,
+			'After lockout expires, the user can login again'
+		);
 		
 		// Log the user out
 		$this->session()->inst_set('loggedInAs', null);
 
-		/* NOW THAT THE LOCK-OUT HAS EXPIRED, CHECK THAT WE ARE ALLOWED 4 FAILED ATTEMPTS BEFORE LOGGING IN */
+		// Login again with wrong password, but less attempts than threshold
-
+		for($i = 1; $i < Member::config()->lock_out_after_incorrect_logins; $i++) {
-		$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
-		$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
-		$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
 			$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
+		}
 		$this->assertNull($this->session()->inst_get('loggedInAs'));
-		$this->assertTrue(false !== stripos($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED')));
+		$this->assertTrue(
+			false !== stripos($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED')),
+			'The user can retry with a wrong password after the lockout expires'
+		);
 		
 		$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
-		$this->assertEquals($this->session()->inst_get('loggedInAs'), $member->ID);
+		$this->assertEquals(
+			$this->session()->inst_get('loggedInAs'), 
+			$member->ID,
+			'The user can login successfully after lockout expires, if staying below the threshold'
+		);
 
 		i18n::set_locale($local);
 	}