• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Split login fail condition in 2 conditions to cover “account inactive” case

#1
I have made a Registration and Login application with Codeigniter 3.

When someone fills the *Registration form* and submits it successfully, the "active" column of the "users" table receives the value 0, as visible in the image bellow:

[Image: 6oeby.png]

Users will have to activate their accounts before being able to sign in.

The user_login() function inside the Usermodel:

   
PHP Code:
public function user_login($email$password$active) {
      $query $this->db->get_where('users', ['email' => $email'password' => md5($password), 'active' => 1]);
      return $query->row();
 


In the Signin.php controller I have the signin() method: 

 
PHP Code:
 public function signin()
      {  
      $this
->form_validation->set_rules('email''Email''required|trim|valid_email');
      $this->form_validation->set_rules('password''Password''required|trim');
      $this->form_validation->set_error_delimiters('<p class="error">''</p>');
      if ($this->form_validation->run())
      {
        $email $this->input->post('email');
        $password $this->input->post('password');
        $this->load->model('Usermodel');
        $current_user $this->Usermodel->user_login($email$password);
          // If we find a user
        if ($current_user) {
          // If the user found is active
          if ($current_user->active == 1) {
            $this->session->set_userdata(
             array(
              'user_id' => $current_user->id,
              'user_email' => $current_user->email,
              'user_first_name' => $current_user->fname,
              'user_active' => $current_user->active,
              'is_logged_in' => TRUE
              
)
             );
            redirect('home');  
          
} else {
            // If the user found is NOT active
            $this->session->set_flashdata("signin_failure""Your account has not been activated");
            redirect('signin'); 
          
}
        } else {
          // If we do NOT find a user
          $this->session->set_flashdata("signin_failure""Incorrect email or password");
          redirect('signin'); 
        
}
      }
      else
      {
       $this->load->view('signin');
     }
 


but there is a flaw in it because even when the email and password are correct, but the user is inactive, the message is: "Incorrect email or password" Instead of "Your account has not been activated".
Reply

#2
This is because your first if statement is:

PHP Code:
if ($current_user) { 

Which will return false for an inactive user, as your query is:

PHP Code:
$query $this->db->get_where('users', ['email' => $email'password' => md5($password), 'active' => 1]); 

Notice, the check for "active" => 1, meaning it won't return any records for inactive users.

So your first if statement returns false, hence going to the else clause which has:

PHP Code:
$this->session->set_flashdata("signin_failure""Incorrect email or password"); 

So you probably need to check if the user is active first, before checking if their username/password is correct.

I'd suggest splitting your "user_login" function into two distinct functions. One to check if the user is active, and one to test the user/pass combo.

Lastly, I noticed you're storing your password as md5 strings... this is a bad idea. It's not secure. Use bcrypt or similar.
Reply

#3
(10-15-2017, 06:43 AM)JayAdra Wrote: This is because your first if statement is:

PHP Code:
if ($current_user) { 

Which will return false for an inactive user, as your query is:

PHP Code:
$query $this->db->get_where('users', ['email' => $email'password' => md5($password), 'active' => 1]); 

Notice, the check for "active" => 1, meaning it won't return any records for inactive users.

So your first if statement returns false, hence going to the else clause which has:

PHP Code:
$this->session->set_flashdata("signin_failure""Incorrect email or password"); 

So you probably need to check if the user is active first, before checking if their username/password is correct.

I'd suggest splitting your "user_login" function into two distinct functions. One to check if the user is active, and one to test the user/pass combo.

Lastly, I noticed you're storing your password as md5 strings... this is a bad idea. It's not secure. Use bcrypt or similar.

It worke! Thx!
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2019 MyBB Group.