WebLogin snippet bug

54 Posts

dalekhin Reply #1, 19 years, 2 months ago

Hello all,
I am trying to setup login form for my site , and found intresting thing in Weblogin snippet:
file weblogin.processor.inc.php


    if($failedlogins>=$modx->config['failed_login_attempts'] && $blockeduntildate<time()) {    // blocked due to number of login errors, but get to try again
        $sql = "UPDATE $dbase.`".$table_prefix."web_user_attributes` SET failedlogincount='0', blockeduntil='".(time()-1)."' where internalKey=$internalKey";
        $ds = $modx->db->query($sql);
		return;
    }

.(time()-1).:

So it does mean , blocking will NOT work, because time will be set into past.

So , can anybody comment on this ?

Russian FileShare Service MODx powered

2,204 Posts

Dimmy Reply #2, 19 years, 2 months ago

it does work because when you look at the query it says blockeduntildate<time()) so if blocked until is smaler then curent time so the time until the block needs to be active is past, unblock user.

Dimmy

follow me on twitter: @dimmy01

54 Posts

dalekhin Reply #3, 19 years, 2 months ago

Hi!

Blocking looks like still does not work for me ( I use 0.9.6-RC3 )

May be here is the error:

        if($failedlogins>=$modx->config['failed_login_attempts']) { //increment the failed login counter, and block!
  $sql = "update $dbase.`".$table_prefix."web_user_attributes` SET failedlogincount='$failedlogins', blockeduntil='".(time()+($blocked_minutes*60))."' where internalKey=$internalKey";
            $ds = $modx->db->query($sql);
        } else { //increment the failed login counter
            $sql = "update $dbase.`".$table_prefix."web_user_attributes` SET failedlogincount='$failedlogins' where internalKey=$internalKey";
            $ds = $modx->db->query($sql);
        }

variable $blocked_minutes looks like unset.

It can be set before first $sql string
$blocked_minutes = $modx->config[’blocked_minutes’];

Russian FileShare Service MODx powered

54 Posts

dalekhin Reply #4, 19 years, 2 months ago

’remember me’ also doesnt work!

weblogin.processor.inc.php:


    if($rememberme) {
        $_SESSION['modx.web.session.cookie.lifetime']= intval($modx->config['session.cookie.lifetime']);
    } else {
        $_SESSION['modx.web.session.cookie.lifetime']= 0;
    }

it checks $rememberme variable which is set from form template. But it will be set only if register_global is on , which is unsafe and for default
php installation is off.

More safe way

 if(isset($_POST['rememberme'])) ....

Here is more safe and right way

add to weblogin snippet code:

$loginText = isset($logintext)? $logintext:’Login’;
$RemmeSet = isset($_POST[’rememberme’]) ? 1:0;
$logoutText = isset($logouttext)? $logouttext:’Logout’;

and in
weblogin.processor.inc.php:

    if($RemmeSet == 1) {
        $_SESSION['modx.web.session.cookie.lifetime']= intval($modx->config['session.cookie.lifetime']);
    } else {
        $_SESSION['modx.web.session.cookie.lifetime']= 0;
    }

Russian FileShare Service MODx powered

MODX Staff
10,725 Posts

opengeek Reply #5, 19 years, 2 months ago

Quote from: dalekhin at Jul 17, 2007, 07:16 PM

’remember me’ also doesnt work!

it checks $rememberme variable which is set from form template. But it will be set only if register_global is on , which is unsafe and for default
php installation is off.

Works fine here. And it has no dependency on register_global settings; the $rememberme variable is set on line 189 from the $_POST superglobal.

Jason Coward
Chief Architect @ MODX
http://www.jasoncoward.com | http://twitter.com/drumshaman | https://github.com/opengeek

54 Posts

dalekhin Reply #6, 19 years, 2 months ago

yes, sorry, here was my error. The thing confused me is cookie for domain, modxcms sets cookie for whole FQDN : www.site.com
but IMHO, its more correct to set cookies for domain only with dot: .site.com

Russian FileShare Service MODx powered

54 Posts

dalekhin Reply #7, 19 years, 1 month ago

still diggin weblogin

here is some code in weblogin.processor.inc.php for redirecting webuser if $refurl was set set in query:

    // redirect 
    if(isset($_REQUEST['refurl']) && !empty($_REQUEST['refurl'])) {
        // last accessed page
        $targetPageId= html_entity_decode($_REQUEST['refurl']);
        if (strpos($targetPageId, 'q=') !== false) {
            $urlPos = strpos($targetPageId, 'q=')+2;
            $alias = substr($targetPageId, $urlPos);
            $aliasLength = (strpos($alias, '&'))? strpos($alias, '&'): strlen($alias); 
            $alias = substr($alias, 0, $aliasLength);
            $url = $modx->config['base_url'] . $alias;
        } elseif ($targetPageId= array_search($targetPageId, $modx->documentListing)) {
            $url = $modx->makeUrl($targetPageId);
        } else {
            $url = $_REQUEST['refurl'];
        }
        $modx->sendRedirect($url);
    }

IMHO line

} elseif ($targetPageId= array_search($targetPageId, $modx->documentListing)) {

should be removed by:

} elseif ( array_search($targetPageId, $modx->documentListing)) {

btw may be here is my misunderstooding $modx->documentListing - is not yet documented sad

Russian FileShare Service MODx powered