We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 17750
    • 78 Posts
    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.

      • 11055 ☆ A M B ☆
      • 3,112 Posts
      would you add this to http://bugs.modx.com ?
        Rico
        Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
        MODx is great, but knowing how to use it well makes it perfect!

        www.virtudraft.com

        Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

        Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

        Maintainter/contributor of Babel

        Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
        • 25663 MODX Staff
        • 12,272 Posts
        Thanks so much for catching this Lyubomir. (See your PM inbox too, please. smiley )
          Ryan Thrash, MODX Co-Founder
          Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me