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?<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');class Usermanagement extends CI_Controller { public function __construct(){    parent::__construct();}   public function index(){    //Config Defaults Start    $msgBoxMsgs = array();//msgType = dl, info, warn, note, msg    $cssPageAddons = '';//If you have extra CSS for this view append it here    $jsPageAddons = '';//If you have extra JS for this view append it here    $metaAddons = '';//Sometimes there is a need for additional Meta Data such in the case of Facebook addon's    $siteTitle = '';//alter only if you need something other than the default for this view.    //Config Defaults Start    //examples of how to use the message box system (css not included).    //$msgBoxMsgs[] = array('msgType' => '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 from the above variable this will send it to the view.    $this->data['pageMetaTags'] = $this->metatags->MetaTags();//defaults can be changed via models/metatags.php    $this->data['siteTitle'] = $siteTitle;//defaults can be changed via models/metatags.php    $this->data['bodyType'] = $bodyType;    $this->data['bodyContent'] = $bodyContent;    $this->load->view('usermanagement/index', $this->data);}function login(){    // Set validation rules for login form fields    $this->form_validation->set_rules('username', 'Username', 'trim|required|max_length[50]|xss_clean');    $this->form_validation->set_rules('password', 'Password', 'trim|required|max_length[12]|xss_clean');    // Verify login form was submitted    if ($this->form_validation->run() == FALSE)    {        // Form was not submitted        redirect('usermanagement/index');         }    else    {        // Create variables from login form submission        $username = $this->input->post('username');        $password = $this->input->post('password');        // Run function to check if username exists in database        $user_data = $this->loggedin->get_user_data($username);        // No username match in database        if($user_data == false)        {            echo json_encode(array('error' => 'yes', 'msg' => 'Username not found, check your username and try again.', 'flags' => 3));        }        else         {            // User was found in database            // Check account status of username            if ($user_data->users_statuses_id == 1)            {                echo json_encode(array('error' => 'yes', 'msg' => 'Account has not been authenticated. Check your email for verification instructions!', 'flags' => 3));                }            elseif ($user_data->users_statuses_id == 3)            {                echo json_encode(array('error' => 'yes', 'msg' => 'Account has been suspended!', 'flags' => 3));            }            elseif ($user_data->users_statuses_id == 4)            {                echo json_encode(array('error' => 'yes', 'msg' => 'Account has been banned!', 'flags' => 3));            }            elseif ($user_data->users_statuses_id == 5)            {                echo json_encode(array('error' => 'yes', 'msg' => 'Account has been deleted!', 'flags' => 3));            }            else            {                // Create a user_id variable from returned data                $user_id = $user_data->user_id;                $lock_date = $user_data->lock_date;                $current_time = time();                $failed_logins = $user_data->failed_logins;                // Check to see if account is currently locked                if ($lock_date !== 0000-00-00 00:00:00)                 {                     if((strtotime($lock_date) < $current_time && $failed_logins > 0))                     {                        $lock_date = strtotime($lock_date);                        // See how much time is remaining efore user can login again                        $time_difference = $this->genfunc->time_since($lock_date);                        // Time must be                        if ($time_difference <= 10)                         {                            // Has not been 10 minutes yet                            $time_remaining = 10 - $time_difference;                            echo json_encode(array('error' => 'yes', 'msg' => 'Your account is currently locked, we appologize for the inconvienence. You must wait ' .$time_remaining.' minutes before you can log in again!', 'flags' => 3));                        }                         else                         {                            // Clear the lock                            $this->loggedin->clear_login_attempts($user_id);                            echo json_encode(array('error' => 'no', 'msg' => 'Your account is now unlocked, you may now log in again!', 'flags' => 3));                                                     }                    }                                                                             }                else                {                       // Assign variables to returned data                     $passwordDB = $user_data->password;                    $passwordDB2 = $user_data->password2;                    $first_name = $user_data->first_name;                    $last_name = $user_data->last_name;                    $email = $user_data->email;                    $users_roles_id = $user_data->users_roles_id;                    $generated_password = $this->genfunc->reGenPassHash($password, $passwordDB2);                    $date_time = $this->genfunc->unixToMySQL($current_time);                    // Passwords match username in database                    if ($passwordDB == $generated_password)                     {                        // Run functions for user to update logins and clearing of failed attempts                        $this->loggedin->update_logins($user_id);                        $this->loggedin->clear_login_attempts($user_id);                         // Create variables about user and assign them to session for database entry                        $op_system = $this->genfunc->getUserOS();                        $user_ip = $this->genfunc->getRealIpAddr();                        $user_browser = $this->session->userdata('user_agent');                        $session_id = $this->session->userdata('session_id');                        $this->loggedin->insert_session($user_id, $session_id, $user_ip, $user_browser, $date_time);                        $this->session->set_userdata(array(                            'session_id' => $session_id,                            'logged_in' => TRUE,                            'user_id' => $user_id,                            'username' => $username,                            'users_roles_id' => $users_roles_id                        ));                        redirect('cpanel/index');                         }                    else                     {                        // Passwords didn't match in database                        // Take failed logins and compare it                         if ($failed_logins == 5)                         {                            // Retrieve IP Address of user trying to hack into account                            $hacker_ip_address = $this->genfunc->getRealIpAddr();                            $this->loggedin->update_hacked_account($user_id, $hacker_ip_address, $date_time);                            $my_domain_name = $this->genfunc->myDomainName();                            // Email user new registration account                            $sender_email = noreply@kansasoutlawwrestling.com;                            $reply_to = noreply@kansasoutlawwrestling.com;                            $recipient_email = $email;                             $email_subject = KOW Manager Account Locked;                            $email_body = 'Hello '.$first_name.' '.$last_name.' You, or someone using your account at '.$my_domain_name.', has attempted to hack into your account. If this is an error, ignore this email and you will be removed from our mailing list.<br /><br />Regards, '.$my_domain_name.' Team';                            $this->genfunc->mailSomeone($email, $sender_email, $email_subject, $email_body);                            echo json_encode(array('error' => 'yes', 'msg' => 'Your account is currently locked, we appologize for the inconvienence. This is a security messure implimented by to many failed login\'s! You must wait 10 minutes before you can login again!', 'flags' => 3));                             }                        else                        {                            // Upate amount of failed logins                            $this->loggedin->update_failed_logins($user_id);                            $chances_left = 5 - $failed_logins;                              echo json_encode(array('error' => 'yes', 'msg' => 'Invalid Username and Password combination! You have ' .$chances_left. ' chances left to login succesfully or the account will be locked!', 'flags' => 3));                             }                      }                                        }               }          }    }}function logout(){   $this->session->sess_destroy();   redirect('usermanagement/index');     }       }/* End of file usermanagement.php */ /* Location: ./application/controllers/usermanagement.php */ MODEL : <?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');class Loggedin extends CI_Model {/** * Loggedin::__construct() *  * @return */function __construct(){    parent::__construct();}/** * Loggedin::get_user_data() *  * @param string $username Username that was posted via login script * @return $query->row OR false depending on if a user was found in the database */public function get_user_data($username){    $this->db->select('*');    $this->db->from('users');    $this->db->join('users_logins', 'users.user_id = users_logins.user_id');    $this->db->where('users.username', $username);    $query = $this->db->get();       if ($query->num_rows() > 0)    {        return $query->row();    }    else    {        return false;    }}/** * Loggedin::clear_login_attempts() *  * @param int $user_id  $user_id of the account trying to be accessed */public function clear_login_attempts($user_id){    $this->db->set('lock_date', 'NULL');    $this->db->set('hacker_ip_address', 'NULL');    $this->db->set('failed_logins', '0');                                                    $this->db->where('user_id', $user_id);       $this->db->update('users_logins');         }/** * Loggedin::update_logins() *  * */public function update_logins($user_id){    $this->db->query('UPDATE users_logins SET number_of_logins = number_of_logins+1 WHERE user_id ='.$user_id);           }/** * Loggedin::insert_session() *  * @param int $user_id  $user_id of the account trying to be accessed */public function insert_session($user_id, $session_id, $user_ip, $user_browser, $current_time){    $data = array(       'user_id' => $user_id ,       'session_id' => $session_id,       'ip_address' => $user_ip,       'user_agent' => $user_browser,       'session_started' => $current_time                          );    $this->db->insert('users_logins_sessions', $data); }  /** * Loggedin::update_failed_logins() *  * @param int $user_id  $user_id of the account trying to be accessed */public function update_failed_logins($user_id){    $this->db->query('UPDATE users_logins SET failed_logins = failed_logins+1 WHERE user_id ='.$user_id);           } /** * Loggedin::update_hacked_account() *  *  */public function update_hacked_account($user_id, $hacker_ip_address, $current_time){    $this->db->set('lock_date', $current_time);    $this->db->set('hacker_ip_address', $hacker_ip_address);                                                  $this->db->where('user_id', $user_id);       $this->db->update('users_logins');   }   }/* End of file loggedin.php *//* Location: ./app/models/loggedin.php */EDIT 2:I have rewritten it all just to try and start over that way it'd be possible to accomplish all tasks in the correct logical order as well as with the smallest bit of code. I tend to fail at both of those things.Revised code:Controller: function login_submit(){    $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean');    $this->form_validation->set_rules('password', 'Password', 'trim|required|xss_clean');    $this->form_validation->set_rules('remember', 'Remember me', 'integer');    $user_id = $this->users->get_user_id_by_username($this->input->post('username'));    if ($user_id !== 0)    {        if ($this->kow_auth->is_max_login_attempts_exceeded($user_id))        {            echo json_encode(array('error' => 'yes', 'message' => 'Your account is currently locked, we appologize for the inconvienence. You must wait 10 minutes before you can login again!'));        }        else        {            $user_status = $this->users->get_user_status($user_id);            if ($user_status == 1)            {                echo json_encode(array('error' => 'yes', 'message' => 'Sorry you must verify your account before logging in!'));                                }            elseif ($user_status == 3)            {                echo json_encode(array('error' => 'yes', 'message' => 'Your account has been suspended!'));                      }            elseif ($user_status == 4)            {                echo json_encode(array('error' => 'yes', 'message' => 'Your account is currently banned!'));                      }             elseif ($user_status == 5)            {                echo json_encode(array('error' => 'yes', 'message' => 'Your account has been deleted!'));                 }            else            {            }                                                     }                   }     else     {        echo json_encode(array('error' => 'yes', 'message' => 'Incorrect username and password combination!'));    }            }Models: Users Model<?php if (!defined('BASEPATH')) exit('No direct script access allowed');/*** Users** This model represents user authentication data. It operates the following tables:* - user account data,* - user profiles** @package  Kow_auth* @author   Jeffrey Davidson*/class Users extends CI_Model{function __construct(){    parent::__construct();}function get_user_id_by_username($username){    $this->db->select('user_id');    $this->db->where('username', $username);    $query = $this->db->get('users');    if ($query->num_rows() > 0)    {        $row = $query->row();        return $row->user_id;    }     else    {        return 0;          }}function get_user_status($user_id){    $this->db->select('users_statuses_id ');    $this->db->where('user_id', $user_id);    $query = $this->db->get('users');    if ($query->num_rows() > 0)    {        $row = $query->row();        return $row->users_statuses_id;    }     else    {        return 0;          }}}Login Attempts Model<?php if (!defined('BASEPATH')) exit('No direct script access allowed');/** * Login_attempts * *  This model serves to watch on all attempts to login on the site * (to protect the site from brute-force attack to user database) * * @package Kow_auth * @author  Jeffrey Davidson */ class Login_attempts extends CI_Model{function __construct(){    parent::__construct();}function get_attempts_num($user_id){    $this->db->select('failed_logins');    $this->db->from('users_logins');    $this->db->where('user_id', $user_id);    $query = $this->db->get();    if ($query->num_rows() > 0)    {        $row = $query->row();        return $row->failed_logins;    }     else    {        return 0;          }}}Library:<?php if (!defined('BASEPATH')) exit('No direct script access allowed');/** * KOW Auth Library * Authentication Library for Code Igniter * @author Jeffrey Davidson * @version 1.0.0 * @copyright 2012 */class Kow_auth{function __construct(){    //assign the CI superglobal to $CI    $this->ci =& get_instance();             }function is_max_login_attempts_exceeded($user_id){    $this->ci->load->model('kow_auth/login_attempts');    $login_attempts = $this->ci->login_attempts->get_attempts_num($user_id);    if ($login_attempts >= 5)    {        return true;    }    else    {        return false;    }}} ?>
Login Validation
php;codeigniter
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.