BUG FailedLoginCount reset

If you fail your maximum login attempts and are locked out, further failed login attempts add to your already existing FailedLoginCount as it is only reset if you log in successfully. This means that if you're locked out, then try again, one failure will automatically lock you out again, regardless of what you set your max limit to.

Example:

lock_out_after_incorrect_logins: 3
FailedLoginCount: 0

The user fails three login attempts.

lock_out_after_incorrect_logins: 3
FailedLoginCount: 3

The user is now locked out.

Lockout time passes.

The user fails their 4th login.

lock_out_after_incorrect_logins: 3
FailedLoginCount: 4

This will continue to happen until the user successfully logs in, without giving them the pre-defined amount of login attempts again due to this condition being met after every incorrect login:
```php
if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
```

FailedLoginTestCount Test Added
This commit is contained in:
Thomas Speak 2013-10-17 18:42:50 +01:00 committed by Tom Speak
parent 8c527eab40
commit d22ca62c6f
2 changed files with 48 additions and 1 deletions

View File

@ -1412,6 +1412,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) {
$lockoutMins = self::config()->lock_out_delay_mins;
$this->LockedOutUntil = date('Y-m-d H:i:s', time() + $lockoutMins*60);
$this->FailedLoginCount = 0;
$this->write();
}
}

View File

@ -7,7 +7,7 @@ class MemberTest extends FunctionalTest {
protected static $fixture_file = 'MemberTest.yml';
protected $orig = array();
protected $local = null;
protected $local = null;
protected $illegalExtensions = array(
'Member' => array(
@ -698,6 +698,52 @@ class MemberTest extends FunctionalTest {
);
}
public function testFailedLoginCount() {
$maxFailedLoginsAllowed = 3;
//set up the config variables to enable login lockouts
Config::nest();
Config::inst()->update('Member', 'lock_out_after_incorrect_logins', $maxFailedLoginsAllowed);
$member = $this->objFromFixture('Member', 'test');
$failedLoginCount = $member->FailedLoginCount;
for ($i = 1; $i < $maxFailedLoginsAllowed; ++$i) {
$member->registerFailedLogin();
$this->assertEquals(
++$failedLoginCount,
$member->FailedLoginCount,
'Failed to increment $member->FailedLoginCount'
);
$this->assertFalse(
$member->isLockedOut(),
"Member has been locked out too early"
);
}
//fail login until max login attempts is reached
$member->FailedLoginCount = 0;
for ($i = 0; $i < $maxFailedLoginsAllowed; ++$i) {
$member->registerFailedLogin();
}
//check to see if they've been locked out
$this->assertTrue(
$member->isLockedOut(),
'Member was not locked out when max logins met'
);
//after they're locked out, need to check FailedLoginCount was reset to 0
$this->assertEquals(
$member->FailedLoginCount,
0,
'Failed login count was not reset after lockout'
);
//test all done, unnest config
Config::unnest();
}
}
class MemberTest_ViewingAllowedExtension extends DataExtension implements TestOnly {