tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #40 from silverstripe-security/patch/3.4/ss-2017-002

Browse Source 
[SS-2017-002] FIX Lock out users who dont exist in the DB
This commit is contained in:
Daniel Hensby 2017-05-28 22:18:56 +01:00 committed by GitHub
commit f71efb5063
4 changed files with 88 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
security/Member.php
View File

@ -398,7 +398,35 @@ class Member extends DataObject implements TemplateGlobalProvider {
* Returns true if this user is locked out
*/
public function isLockedOut() {
return $this->LockedOutUntil && SS_Datetime::now()->Format('U') < strtotime($this->LockedOutUntil);
global $debug;
if ($this->LockedOutUntil && $this->dbObject('LockedOutUntil')->InFuture()) {
return true;
}
if ($this->config()->lock_out_after_incorrect_logins <= 0) {
return false;
}
$attempts = LoginAttempt::get()->filter($filter = array(
'Email' => $this->{static::config()->unique_identifier_field},
))->sort('Created', 'DESC')->limit($this->config()->lock_out_after_incorrect_logins);
if ($attempts->count() < $this->config()->lock_out_after_incorrect_logins) {
return false;
}
foreach ($attempts as $attempt) {
if ($attempt->Status === 'Success') {
return false;
}
}
$lockedOutUntil = $attempts->first()->dbObject('Created')->Format('U') + ($this->config()->lock_out_delay_mins * 60);
if (SS_Datetime::now()->Format('U') < $lockedOutUntil) {
return true;
}
return false;
}
/**
@ -1660,7 +1688,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
public function registerFailedLogin() {
if(self::config()->lock_out_after_incorrect_logins) {
// Keep a tally of the number of failed log-ins so that we can lock people out
$this->FailedLoginCount = $this->FailedLoginCount + 1;
++$this->FailedLoginCount;
if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
$lockoutMins = self::config()->lock_out_delay_mins;
@ -1679,6 +1707,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
if(self::config()->lock_out_after_incorrect_logins) {
// Forgive all past login failures
$this->FailedLoginCount = 0;
$this->LockedOutUntil = null;
$this->write();
}
}

10
security/MemberAuthenticator.php
View File

@ -70,6 +70,14 @@ class MemberAuthenticator extends Authenticator {
if($member && !$asDefaultAdmin) {
$result = $member->checkPassword($data['Password']);
$success = $result->valid();
} elseif (!$asDefaultAdmin) {
// spoof a login attempt
$member = Member::create();
$member->Email = $email;
$member->{Member::config()->unique_identifier_field} = $data['Password'] . '-wrong';
$member->PasswordEncryption = 'none';
$result = $member->checkPassword($data['Password']);
$member = null;
} else {
$result = new ValidationResult(false, _t('Member.ERRORWRONGCRED'));
}
@ -94,7 +102,7 @@ class MemberAuthenticator extends Authenticator {
* @param bool $success
*/
protected static function record_login_attempt($data, $member, $success) {
if(!Security::config()->login_recording) return;
if(!Security::config()->login_recording && !Member::config()->lock_out_after_incorrect_logins) return;
// Check email is valid
$email = isset($data['Email']) ? $data['Email'] : null;

41
tests/security/MemberAuthenticatorTest.php
View File

@ -180,6 +180,45 @@ class MemberAuthenticatorTest extends SapphireTest {
), $form);
$this->assertTrue(Member::default_admin()->isLockedOut());
$this->assertEquals(Member::default_admin()->LockedOutUntil, '2016-04-18 00:10:00');
$this->assertEquals('2016-04-18 00:10:00', Member::default_admin()->LockedOutUntil);
}
public function testNonExistantMemberGetsLoginAttemptRecorded()
{
Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1);
$email = 'notreal@example.com';
$this->assertFalse(Member::get()->filter(array('Email' => $email))->exists());
$this->assertCount(0, LoginAttempt::get());
$response = MemberAuthenticator::authenticate(array(
'Email' => $email,
'Password' => 'password',
));
$this->assertNull($response);
$this->assertCount(1, LoginAttempt::get());
$attempt = LoginAttempt::get()->first();
$this->assertEquals($email, $attempt->Email);
$this->assertEquals('Failure', $attempt->Status);
}
public function testNonExistantMemberGetsLockedOut()
{
Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1);
Config::inst()->update('Member', 'lock_out_delay_mins', 10);
$email = 'notreal@example.com';
$this->assertFalse(Member::get()->filter(array('Email' => $email))->exists());
$response = MemberAuthenticator::authenticate(array(
'Email' => $email,
'Password' => 'password'
));
$this->assertNull($response);
$member = new Member();
$member->Email = $email;
$this->assertTrue($member->isLockedOut());
$this->assertFalse($member->canLogIn()->valid());
}
}

18
tests/security/SecurityTest.php
View File

@ -398,6 +398,7 @@ class SecurityTest extends FunctionalTest {
public function testRepeatedLoginAttemptsLockingPeopleOut() {
$local = i18n::get_locale();
i18n::set_locale('en_US');
SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-05-22 00:00:00'));
Member::config()->lock_out_after_incorrect_logins = 5;
Member::config()->lock_out_delay_mins = 15;
@ -414,10 +415,9 @@ class SecurityTest extends FunctionalTest {
);
$this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
} else {
// Fuzzy matching for time to avoid side effects from slow running tests
$this->assertGreaterThan(
time() + 14*60,
strtotime($member->LockedOutUntil),
$this->assertEquals(
SS_Datetime::now()->Format('U') + (15 * 60),
$member->dbObject('LockedOutUntil')->Format('U'),
'User has a lockout time set after too many failed attempts'
);
}
@ -440,14 +440,12 @@ class SecurityTest extends FunctionalTest {
'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();
// Move into the future so we can login again
SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-06-22 00:00:00'));
$this->doTestLoginForm('testuser@example.com' , '1nitialPassword');
$this->assertEquals(
$this->session()->inst_get('loggedInAs'),
$member->ID,
$this->session()->inst_get('loggedInAs'),
'After lockout expires, the user can login again'
);
@ -467,8 +465,8 @@ class SecurityTest extends FunctionalTest {
$this->doTestLoginForm('testuser@example.com' , '1nitialPassword');
$this->assertEquals(
$this->session()->inst_get('loggedInAs'),
$member->ID,
$this->session()->inst_get('loggedInAs'),
'The user can login successfully after lockout expires, if staying below the threshold'
);