tonyair
/
silverstripe-framework
mirror of https://github.com/silverstripe/silverstripe-framework
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[SS-2017-002] FIX Lock out users who dont exist in the DB

This commit is contained in:
Daniel Hensby 2017-05-09 21:24:15 +01:00 committed by Damian Mooyman
parent 23a719e14e
commit 30986b4ea3
No known key found for this signature in database
GPG Key ID: 78B823A10DE27D1A
5 changed files with 130 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Security/LoginAttempt.php
View File

@ -24,6 +24,15 @@ use SilverStripe\ORM\DataObject;
*/
class LoginAttempt extends DataObject
{
/**
* Success status
*/
const SUCCESS = 'Success';
/**
* Failure status
*/
const FAILURE = 'Failure';
private static $db = array(
'Email' => 'Varchar(255)',
@ -32,7 +41,7 @@ class LoginAttempt extends DataObject
);
private static $has_one = array(
'Member' => 'SilverStripe\\Security\\Member', // only linked if the member actually exists
'Member' => Member::class, // only linked if the member actually exists
);
private static $table_name = "LoginAttempt";

42
src/Security/Member.php
View File

@ -350,13 +350,44 @@ class Member extends DataObject
*/
public function isLockedOut()
{
if (!$this->LockedOutUntil) {
/** @var DBDatetime $lockedOutUntilObj */
$lockedOutUntilObj = $this->dbObject('LockedOutUntil');
if ($lockedOutUntilObj->InFuture()) {
return true;
}
$maxAttempts = $this->config()->get('lock_out_after_incorrect_logins');
if ($maxAttempts <= 0) {
return false;
}
/** @var DBDatetime $lockedOutUntil */
$lockedOutUntil = $this->dbObject('LockedOutUntil');
return DBDatetime::now()->getTimestamp() < $lockedOutUntil->getTimestamp();
$idField = static::config()->get('unique_identifier_field');
$attempts = LoginAttempt::get()
->filter('Email', $this->{$idField})
->sort('Created', 'DESC')
->limit($maxAttempts);
if ($attempts->count() < $maxAttempts) {
return false;
}
foreach ($attempts as $attempt) {
if ($attempt->Status === 'Success') {
return false;
}
}
// Calculate effective LockedOutUntil
/** @var DBDatetime $firstFailureDate */
$firstFailureDate = $attempts->first()->dbObject('Created');
$maxAgeSeconds = $this->config()->get('lock_out_delay_mins') * 60;
$lockedOutUntil = $firstFailureDate->getTimestamp() + $maxAgeSeconds;
$now = DBDatetime::now()->getTimestamp();
if ($now < $lockedOutUntil) {
return true;
}
return false;
}
/**
@ -1652,7 +1683,7 @@ class Member extends DataObject
$lockOutAfterCount = self::config()->get('lock_out_after_incorrect_logins');
if ($lockOutAfterCount) {
// 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 >= $lockOutAfterCount) {
$lockoutMins = self::config()->get('lock_out_delay_mins');
@ -1672,6 +1703,7 @@ class Member extends DataObject
if (self::config()->get('lock_out_after_incorrect_logins')) {
// Forgive all past login failures
$this->FailedLoginCount = 0;
$this->LockedOutUntil = null;
$this->write();
}
}

6
src/Security/MemberAuthenticator/MemberAuthenticator.php
View File

@ -99,7 +99,7 @@ class MemberAuthenticator implements Authenticator
$member->registerFailedLogin();
}
} elseif ($member) {
$member->registerSuccessfulLogin();
$member->registerSuccessfulLogin();
} else {
// A non-existing member occurred. This will make the result "valid" so let's invalidate
$result->addError(_t(
@ -179,13 +179,13 @@ class MemberAuthenticator implements Authenticator
if ($success && $member) {
// successful login (member is existing with matching password)
$attempt->MemberID = $member->ID;
$attempt->Status = 'Success';
$attempt->Status = LoginAttempt::SUCCESS;
// Audit logging hook
$member->extend('authenticationSucceeded');
} else {
// Failed login - we're trying to see if a user exists with this email (disregarding wrong passwords)
$attempt->Status = 'Failure';
$attempt->Status = LoginAttempt::FAILURE;
if ($member) {
// Audit logging hook
$attempt->MemberID = $member->ID;

59
tests/php/Security/MemberAuthenticatorTest.php
View File

@ -11,6 +11,7 @@ use SilverStripe\ORM\ValidationResult;
use SilverStripe\Security\Authenticator;
use SilverStripe\Security\DefaultAdminService;
use SilverStripe\Security\IdentityStore;
use SilverStripe\Security\LoginAttempt;
use SilverStripe\Security\Member;
use SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator;
use SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm;
@ -205,4 +206,62 @@ class MemberAuthenticatorTest extends SapphireTest
$this->assertFalse($defaultAdmin->canLogin());
$this->assertEquals('2016-04-18 00:10:00', $defaultAdmin->LockedOutUntil);
}
public function testNonExistantMemberGetsLoginAttemptRecorded()
{
Security::config()->set('login_recording', true);
Member::config()
->set('lock_out_after_incorrect_logins', 1)
->set('lock_out_delay_mins', 10);
$email = 'notreal@example.com';
$this->assertFalse(Member::get()->filter(array('Email' => $email))->exists());
$this->assertCount(0, LoginAttempt::get());
$authenticator = new MemberAuthenticator();
$result = new ValidationResult();
$member = $authenticator->authenticate(
[
'Email' => $email,
'Password' => 'password',
],
Controller::curr()->getRequest(),
$result
);
$this->assertFalse($result->isValid());
$this->assertNull($member);
$this->assertCount(1, LoginAttempt::get());
$attempt = LoginAttempt::get()->first();
$this->assertEquals($email, $attempt->Email);
$this->assertEquals(LoginAttempt::FAILURE, $attempt->Status);
}
public function testNonExistantMemberGetsLockedOut()
{
Security::config()->set('login_recording', true);
Member::config()
->set('lock_out_after_incorrect_logins', 1)
->set('lock_out_delay_mins', 10);
$email = 'notreal@example.com';
$this->assertFalse(Member::get()->filter(array('Email' => $email))->exists());
$authenticator = new MemberAuthenticator();
$result = new ValidationResult();
$member = $authenticator->authenticate(
[
'Email' => $email,
'Password' => 'password',
],
Controller::curr()->getRequest(),
$result
);
$this->assertNull($member);
$this->assertFalse($result->isValid());
$member = new Member();
$member->Email = $email;
$this->assertTrue($member->isLockedOut());
$this->assertFalse($member->canLogIn());
}
}

41
tests/php/Security/SecurityTest.php
View File

@ -282,7 +282,7 @@ class SecurityTest extends FunctionalTest
Security::setCurrentUser($member);
/* Visit the Security/logout page with a test referer, but without a security token */
$response = $this->get(
$this->get(
Config::inst()->get(Security::class, 'logout_url'),
null,
['Referer' => Director::absoluteBaseURL() . 'testpage']
@ -494,8 +494,11 @@ class SecurityTest extends FunctionalTest
i18n::set_locale('en_US');
Member::config()->set('lock_out_after_incorrect_logins', 5);
Member::config()->set('lock_out_delay_mins', 15);
DBDatetime::set_mock_now('2017-05-22 00:00:00');
// Login with a wrong password for more than the defined threshold
/** @var Member $member */
$member = null;
for ($i = 1; $i <= 6; $i++) {
$this->doTestLoginForm('testuser@example.com', 'incorrectpassword');
/** @var Member $member */
@ -513,38 +516,36 @@ class SecurityTest extends FunctionalTest
)
);
} else {
// Fuzzy matching for time to avoid side effects from slow running tests
$this->assertGreaterThan(
time() + 14*60,
strtotime($member->LockedOutUntil),
// Lockout should be exactly 15 minutes from now
/** @var DBDatetime $lockedOutUntilObj */
$lockedOutUntilObj = $member->dbObject('LockedOutUntil');
$this->assertEquals(
DBDatetime::now()->getTimestamp() + (15 * 60),
$lockedOutUntilObj->getTimestamp(),
'User has a lockout time set after too many failed attempts'
);
}
}
$msg = _t(
'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2',
'Your account has been temporarily disabled because of too many failed attempts at ' .
'logging in. Please try again in {count} minutes.',
null,
array('count' => 15)
);
$this->assertHasMessage($msg);
$msg = _t(
'SilverStripe\\Security\\Member.ERRORLOCKEDOUT2',
'Your account has been temporarily disabled because of too many failed attempts at ' .
'logging in. Please try again in {count} minutes.',
null,
array('count' => 15)
);
$this->assertHasMessage($msg);
$this->doTestLoginForm('testuser@example.com', '1nitialPassword');
$this->assertNull(
$this->session()->get('loggedInAs'),
'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::class, $this->idFromFixture(Member::class, 'test'));
$member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30);
$member->write();
// Move into the future so we can login again
DBDatetime::set_mock_now('2017-06-22 00:00:00');
$this->doTestLoginForm('testuser@example.com', '1nitialPassword');
$this->assertEquals(
$this->session()->get('loggedInAs'),
$member->ID,
$this->session()->get('loggedInAs'),
'After lockout expires, the user can login again'
);