Merged revisions 52151 via svnmerge from

http://svn.silverstripe.com/open/modules/sapphire/branches/govtsecurity

........
  r52151 | sminnee | 2008-04-05 11:14:26 +1300 (Sat, 05 Apr 2008) | 1 line
  
  Lock users out after 5 failed log-ins
........


git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@53466 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Sam Minnee 2008-04-26 06:32:05 +00:00
parent eb60b67732
commit a1f8892c52
3 changed files with 178 additions and 32 deletions

View File

@ -28,6 +28,7 @@ class Member extends DataObject {
'PasswordEncryption' => "Enum('none', 'none')",
'Salt' => 'Varchar(50)',
'PasswordExpiry' => 'Date',
'LockedOutUntil' => 'Datetime',
'Locale' => 'Varchar(6)',
);
@ -84,6 +85,8 @@ class Member extends DataObject {
* By default, this is null, which means that passwords never expire
*/
protected static $password_expiry_days = null;
protected static $lock_out_after_incorrect_logins = null;
/**
* This method is used to initialize the static database members
@ -111,9 +114,18 @@ class Member extends DataObject {
* @return bool Returns TRUE if the passed password is valid, otherwise FALSE.
*/
public function checkPassword($password) {
$encryption_details = Security::encrypt_password($password, $this->Salt, $this->PasswordEncryption);
return ($this->Password === $encryption_details['password']);
// Only confirm that the password matches if the user isn't locked out
if(!$this->isLockedOut()) {
$encryption_details = Security::encrypt_password($password, $this->Salt, $this->PasswordEncryption);
return ($this->Password === $encryption_details['password']);
}
}
/**
* Returns true if this user is locked out
*/
public function isLockedOut() {
return $this->LockedOutUntil && time() < strtotime($this->LockedOutUntil);
}
/**
@ -141,6 +153,14 @@ class Member extends DataObject {
self::$password_expiry_days = $days;
}
/**
* Configure the security system to lock users out after this many incorrect logins
*/
static function lock_out_after_incorrect_logins($numLogins) {
self::$lock_out_after_incorrect_logins = $numLogins;
}
function isPasswordExpired() {
if(!$this->PasswordExpiry) return false;
return strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry);
@ -167,6 +187,15 @@ class Member extends DataObject {
Cookie::set('alc_enc', null);
Cookie::forceExpiry('alc_enc');
}
// Clear the incorrect log-in count
if(self::$lock_out_after_incorrect_logins) {
$failedLogins = Session::get('Member.FailedLogins');
$failedLogins[$this->Email] = 0;
Session::set('Member.FailedLogins', $failedLogins);
}
$this->LockedOutUntil = null;
$this->write();
}
@ -917,6 +946,25 @@ class Member extends DataObject {
return $valid;
}
/**
* Tell this member that someone made a failed attempt at logging in as them.
* This can be used to lock the user out temporarily if too many failed attempts are made.
*/
function registerFailedLogin() {
if(self::$lock_out_after_incorrect_logins) {
// Keep a tally of the number of failed log-ins so that we can lock people out
$failedLogins = Session::get('Member.FailedLogins');
if(!isset($failedLogins[$this->Email])) $failedLogins[$this->Email] = 0;
$failedLogins[$this->Email]++;
Session::set('Member.FailedLogins', $failedLogins);
if($failedLogins[$this->Email] >= self::$lock_out_after_incorrect_logins) {
$this->LockedOutUntil = date('Y-m-d H:i:s', time() + 15*60);
$this->write();
}
}
}
}

View File

@ -28,25 +28,33 @@ class MemberAuthenticator extends Authenticator {
*/
public static function authenticate(array $RAW_data, Form $form = null) {
$SQL_user = Convert::raw2sql($RAW_data['Email']);
$isLockedOut = false;
// Default login (see Security::setDefaultAdmin())
if(Security::check_default_admin($RAW_data['Email'], $RAW_data['Password'])) {
$member = Security::findAnAdministrator();
} else {
$member = DataObject::get_one("Member", "Email = '$SQL_user' AND Password IS NOT NULL");
if($member && ($member->checkPassword($RAW_data['Password']) == false)) {
if($member && ($member->checkPassword($RAW_data['Password']) == false)) {
if($member->isLockedOut()) $isLockedOut = true;
$member->registerFailedLogin();
$member = null;
}
}
if($member) {
Session::clear("BackURL");
} else if(!is_null($form)) {
$form->sessionMessage(
} else if($isLockedOut) {
if($form) $form->sessionMessage(
_t('Member.ERRORLOCKEDOUT', "Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in 20 minutes."),
"bad"
);
} else {
if($form) $form->sessionMessage(
_t('Member.ERRORWRONGCRED', "That doesn't seem to be the right e-mail address or password. Please try again."),
"bad"
);
}
}
return $member;
}

View File

@ -11,18 +11,10 @@ class SecurityTest extends SapphireTest {
* Test that the login form redirects to the change password form after logging in with an expired password
*/
function testExpiredPassword() {
// BAD PASSWORDS ARE LOCKED OUT
$session = new Session(array());
$badResponse = Director::test('Security/login?executeForm=LoginForm', array(
'Email' => 'sam@silverstripe.com',
'Password' => 'badpassword',
'AuthenticationMethod' => 'MemberAuthenticator',
'action_dologin' => 1,
'BackURL' => 'test/link'),
$session
);
$badResponse = $this->doTestLoginForm('sam@silverstripe.com' , 'badpassword', $session);
$this->assertEquals(302, $badResponse->getStatusCode());
$this->assertRegExp('/Security\/login/', $badResponse->getHeader('Location'));
$this->assertNull($session->inst_get('loggedInAs'));
@ -30,14 +22,7 @@ class SecurityTest extends SapphireTest {
// UNEXPIRED PASSWORD GO THROUGH WITHOUT A HITCH
$session = new Session(array());
$goodResponse = Director::test('Security/login?executeForm=LoginForm', array(
'Email' => 'sam@silverstripe.com',
'Password' => '1nitialPassword',
'AuthenticationMethod' => 'MemberAuthenticator',
'action_dologin' => 1,
'BackURL' => 'test/link'),
$session
);
$goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword', $session);
$this->assertEquals(302, $goodResponse->getStatusCode());
$this->assertEquals(Director::baseURL() . 'test/link', $goodResponse->getHeader('Location'));
$this->assertEquals($this->idFromFixture('Member', 'test'), $session->inst_get('loggedInAs'));
@ -45,17 +30,122 @@ class SecurityTest extends SapphireTest {
// EXPIRED PASSWORDS ARE SENT TO THE CHANGE PASSWORD FORM
$session = new Session(array());
$expiredResponse = Director::test('Security/login?executeForm=LoginForm', array(
'Email' => 'expired@silverstripe.com',
'Password' => '1nitialPassword',
'AuthenticationMethod' => 'MemberAuthenticator',
'action_dologin' => 1,
'BackURL' => 'test/link'),
$session
);
$expiredResponse = $this->doTestLoginForm('expired@silverstripe.com' , '1nitialPassword', $session);
$this->assertEquals(302, $expiredResponse->getStatusCode());
$this->assertEquals(Director::baseURL() . 'Security/changepassword', $expiredResponse->getHeader('Location'));
$this->assertEquals($this->idFromFixture('Member', 'expiredpassword'), $session->inst_get('loggedInAs'));
}
function testRepeatedLoginAttemptsLockingPeopleOut() {
$session = new Session(array());
Member::lock_out_after_incorrect_logins(5);
// LOG IN WITH A BAD PASSWORD 7 TIMES
for($i=1;$i<=7;$i++) {
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
// THE FIRST 4 TIMES, THE MEMBER SHOULDN'T BE LOCKED OUT
if($i < 5) {
$this->assertNull($member->LockedOutUntil);
$this->assertTrue(false !== stripos($this->loginErrorMessage($session), "That doesn't seem to be the right e-mail address or password"));
}
// AFTER THAT THE USER IS LOCKED OUT FOR 15 MINUTES
//(we check for at least 14 minutes because we don't want a slow running test to report a failure.)
else {
$this->assertGreaterThan(time() + 14*60, strtotime($member->LockedOutUntil));
}
if($i > 5) {
$this->assertTrue(false !== stripos($this->loginErrorMessage($session), "Your account has been temporarily disabled"));
}
}
// THE USER CAN'T LOG IN NOW, EVEN IF THEY GET THE RIGHT PASSWORD
$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword', $session);
$this->assertNull($session->inst_get('loggedInAs'));
// BUT, IF TIME PASSES, THEY CAN LOG IN
// (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', $session);
$this->assertEquals($session->inst_get('loggedInAs'), $member->ID);
// Log the user out
$session->inst_set('loggedInAs', null);
// NOW THAT THE LOCK-OUT HAS EXPIRED, CHECK THAT WE ARE ALLOWED 4 FAILED ATTEMPTS BEFORE LOGGING IN
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->assertNull($session->inst_get('loggedInAs'));
$this->assertTrue(false !== stripos($this->loginErrorMessage($session), "That doesn't seem to be the right e-mail address or password"));
$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword', $session);
$this->assertEquals($session->inst_get('loggedInAs'), $member->ID);
}
function testAlternatingRepeatedLoginAttempts() {
$session = new Session(array());
Member::lock_out_after_incorrect_logins(3);
// ATTEMPTING LOG-IN TWICE WITH ONE ACCOUNT AND TWICE WITH ANOTHER SHOULDN'T LOCK ANYBODY OUT
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('noexpiry@silverstripe.com' , 'incorrectpassword', $session);
$this->doTestLoginForm('noexpiry@silverstripe.com' , 'incorrectpassword', $session);
$member1 = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
$member2 = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'noexpiry'));
$this->assertNull($member1->LockedOutUntil);
$this->assertNull($member2->LockedOutUntil);
// BUT, DOING AN ADDITIONAL LOG-IN WITH EITHER OF THEM WILL LOCK OUT, SINCE THAT IS THE 3RD FAILURE IN THIS SESSION
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword', $session);
$member1 = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test'));
$this->assertNotNull($member1->LockedOutUntil);
$this->doTestLoginForm('noexpiry@silverstripe.com' , 'incorrectpassword', $session);
$member2 = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'noexpiry'));
$this->assertNotNull($member2->LockedOutUntil);
}
/**
* Execute a log-in form using Director::test().
* Helper method for the tests above
*/
function doTestLoginForm($email, $password, &$session) {
return Director::test('Security/login?executeForm=LoginForm', array(
'Email' => $email,
'Password' => $password,
'AuthenticationMethod' => 'MemberAuthenticator',
'action_dologin' => 1,
'BackURL' => 'test/link'),
$session
);
}
/**
* Get the error message on the login form
*/
function loginErrorMessage($session) {
return $session->inst_get('FormInfo.MemberLoginForm_LoginForm.formError.message');
}
}