CodeIgniter Forums
Split login fail condition in 2 conditions to cover “account inactive” case - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: General (https://forum.codeigniter.com/forumdisplay.php?fid=1)
+--- Forum: Lounge (https://forum.codeigniter.com/forumdisplay.php?fid=3)
+--- Thread: Split login fail condition in 2 conditions to cover “account inactive” case (/showthread.php?tid=69160)

Split login fail condition in 2 conditions to cover “account inactive” case - Ajax30 - 10-15-2017

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".

RE: Split login fail condition in 2 conditions to cover “account inactive” case - JayAdra - 10-15-2017

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.

RE: Split login fail condition in 2 conditions to cover “account inactive” case - Ajax30 - 10-15-2017

(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!