Login Validation

I'm hoping someone is willing and can take a minute to look over this function and tell me what they think of it, if it can be improved or what not. I tried commenting out what the purpose of everything is. I would like to note that something isn't quite right with the time_remaining and what not because what's supposed to happen when its been passed the 10 minutes the users account clears out the failed logins and redirects to the index function but doesn't.

EDIT: I have redone my code and was hoping someone could tell me what they thought of it now?

```
'dl', 'theMsg' => 'This is a Blank Message Box...');

/***Your Coding Logic Here, Start/

if(!$this->session->userdata('logged_in'))
{
$bodyContent = "login";//which view file
}
else
{
redirect('cpanel/index');
}

$bodyType = "full";//type of template

/****Your Coding Logic Here, End/

//Double checks if any default variables have been changed, Start.
//If msgBoxMsgs array has anything in it, if so displays it in view, else does nothing.
if(count($msgBoxMsgs) !== 0)
{
$msgBoxes = $this->msgboxes->buildMsgBoxesOutput(array('display' => 'show', 'msgs' =>$msgBoxMsgs));
}
else
{
$msgBoxes = array('display' => 'none');
}

if($siteTitle == '')
{
$siteTitle = $this->metatags->SiteTitle(); //reads
}

//Double checks if any default variables have been changed, End.

$this->data['msgBoxes'] = $msgBoxes;
$this->data['cssPageAddons'] = $cssPageAddons;//if there is any additional CSS to add from above Variable this will send it to the view.
$this->data['jsPageAddons'] = $jsPageAddons;//if there is any addictional JS to add from the above variable this will send it to the view.
$this->data['metaAddons'] = $metaAddons;//if there is any addictional meta data to add fr

Okey, first of all; I had a hard time determining if this was a login script or a registration script even if you explicitly said it was a script containing login validation. If I was you I would review the code and try to clear up any vagueness, because I got a headache trying to sort your code out. Here we go anyway:

At line 6: Why do you use xss_clean() if you're not inserting the data into the database for later display to the user? Use remove_invisible_characters() instead.

By the looks of it, you never do any credential checks against the database before starting the log-in routine. This is prone to brute force attacks if you let your attackers check if the username already exists before giving an error message such as 'Wrong username/password'. I tried to figure out exactly where you did the login function, but I couldn't find it.

What exactly is $user_login_data? Shouldn't this information already exist in $user_data? I would recommend using an independent method such as getUserData() instead of returning a data object by a function that is supposed to check if a username exists.

Why do you load first name, last name, e-mail address and two passwords? At this point I was so confused I could barely keep track of the script's purpose. It would remind more of a registration form.

You should use cleaner variable names such as $user_data instead of $user_data[0] everywhere, it can quickly get messy if you decide to change things.

What is $users_statuses_id? I would call this $status_code or something similar. Since variables bound to an object reside within it, there is no point in calling them $user_some_info since the object should already state the purpose well enough by it's name: $user->some_info.

It's hard to know what functions such as genfunc->time_since() does without knowing the code. Though you seem to be checking seconds instead of minutes. if ($time_difference >= 10) should be if ($time_difference >= (10*60)).

Why do you use genfunc->getRealIpAddr(); at one place and $_SERVER['REMOTE_ADDR']; another?

Vague function names: loggedin->update_hacked_account(), genfunc->mailSomeone()? etc.

StackExchange Code Review Q#7501, answer score: 6