For Login Code - Revolution version
File:
/core/model/modx/processors/security/login.php
needs few changes:
1) if the user login is incorrect, then the count of incorrect entries are counted twice
example: login incorrect (incorrect password) - in manager -> user security -> Failed logins: 2
again incorrect - in manager -> user security -> Failed logins: 4 ..etc.
Why is that?
in code line #136:
/* check if plugin authenticated the user */
if (!$rt || (is_array($rt) && !in_array(true, $rt))) {
/* check user password - local authentication */
if ($user->get('password') != md5($givenPassword)) {
$flc = ((int)$up->get('failedlogincount'))+1;
$up->set('failedlogincount',$flc);
$up->save();
return $modx->error->failure($modx->lexicon('login_username_password_incorrect'));
}
}
function RETURN is like external, and this code executes twice, and increments Failed logins +1 +1, t.e. two times
Solution:
/* check if plugin authenticated the user */
if (!$rt || (is_array($rt) && !in_array(true, $rt))) {
/* check user password - local authentication */
if ($user->get('password') != md5($givenPassword)) {
/* add session for count, when return function reload page */
/* if not session - value 0 */
if ($_SESSION['login_failed'] == NULL){
$_SESSION['login_failed'] = 0;
}
/* if it goes for the first time through this */
if ($_SESSION['login_failed'] == 0){
$flc = ((int)$up->get('failedlogincount'))+1;
$up->set('failedlogincount',$flc);
$up->save();
$_SESSION['login_failed'] ++; // increment session value
/* if it doesn't go for the first time through this (after reload of return function) */
}else{
$_SESSION['login_failed'] = 0; // zeroing session
}
return $modx->error->failure($modx->lexicon('login_username_password_incorrect'));
}
}
2) When the number of wrong logins is above the allowed in system settings (failed_login_attempts), then user is not blocked and only its written the time till the user is blocked - the current time() -1 sec, which is not logical. Line #91:
if ($up->get('failedlogincount') >= $modx->getOption('failed_login_attempts') && $up->get('blockeduntil') < time()) {
$up->set('failedlogincount', 0);
$up->set('blockeduntil', time() - 1);
$up->save();
}
Modified code:
if ($up->get('failedlogincount') >= $modx->getOption('failed_login_attempts') ) {
$up->set('failedlogincount', 0);
$up->set('blocked', 1);
$up->set('blockeduntil', time() + (60 * $modx->getOption('blocked_minutes')));
$up->save();
}
if ($up->get('blockeduntil') !=0 && $up->get('blockeduntil') < time()) {
$up->set('failedlogincount', 0);
$up->set('blocked', 0);
$up->set('blockeduntil', 0);
$up->save();
}
The first part blocks the user for the time set in system settings at "blocked_minutes", and the second part nulls all the blocks if the time for blocking has passed already.
3) It may be thought of this, as to send the user email, telling him why he is blocked and from which IP, and if its not him, he may contact the administrator - this way the security of login with be more strong. Also the message: "You have been blocked from the manager by an administrator." may be replaced with "You have been blocked from the manager by an administrator till xxx" , where xxx is the time till when the user is blocked.