1
\$\begingroup\$

I have made my model global so that I can work only a model with several controllers. Is it good or bad practice? And according to this, is my login process secure?

Login check function in Controller:

public function login_auth(){
    $this->form_validation->set_rules('user_username','User Name','required|trim');
    $this->form_validation->set_rules('user_password','User Password','required|trim|trim');
    if($this->form_validation->run()==false){
        $this->login_panel();
    }
    else{
$this->load->model('Authentication_model');            
        $input_data = array(
            'user_username' => $this->input->post('user_username'),
            'user_password' => sha1($this->input->post('user_password' );
        $login_data = $this->Authentication_model->get_data_login('ac_users',$input_data,1)->row_array();
        if(empty($login_data)){
            $input_username = array(
                'user_username' => $this->input->post('user_username')
            );
            $login_username = $this->Authentication_model->get_data_login('ac_users',$input_username,1)->row_array();
            if(sizeof($login_username)>0){
                $this->update_login_attempts($login_username);
                $notification = array(
                    'sign' => 'alert alert-danger',
                    'msg' => 'Your input is wrong.'
                );
            }
            else if($login_username['max_login_attempt']>3){
                $this->update_login_attempts($login_username);
                $notification = array(
                   'sign' => 'alert alert-danger',
                   'msg' => 'Login attempts excedded.'
                );
                $captcha = array('captcha_show' => 'true');
                $this->create_session($captcha);
            }
            $this->error($notification);
            redirect('authentication/login_panel');
        }            
        else if(isset($login_data) && sizeof($login_data)>0){
            if($login_data['user_suspend']){
                $notification = array(
                    'sign' => 'alert alert-danger',
                    'msg' => 'Your Account Is Blocked.'
                );
                $this->error($notification);
                redirect('authentication/login_panel');
            }
            if($login_data['user_branch_id'])
                $branch_name = 'Main';
            else if($login_data['user_branch_id']==0)
                $branch_name = 'All';
            $loginData = array(
                'logged_in' => true,
                'user_name' => $login_data['user_first_name'].$login_data['user_last_name'],
                'user_id' => $login_data['user_id'],
                'branch_id' => $login_data['user_branch_id'],
                'user_group' => $login_data['user_group'],
                'branch_name' => ucfirst($branch_name)
            );
            $where = array(
                'user_id' => $login_data['user_id']
            );
            $update_data = array('user_date_last_login'=>date('Y-m-d g:i:s'));
            $this->Authentication_model->update_data('ac_users',$update_data,$where);
            $this->create_session($loginData);
            $notification = array(
                'sign' => 'alert alert-success',
                'msg' => 'You are logged in successfully'
            );
            $this->error($notification);
            redirect('master/index');
        }
        else{
            $notification = array(
                'sign' => 'alert alert-danger',
                'msg' => 'Your input is wrong.'
            );
            $this->error($notification);
            redirect('authentication/login_panel');
        }
    }
}

Login Model:

class Authentication_model extends CI_Model {

    public function insert_data($tbl='',$data=''){
        return $this->db->insert($tbl,$data);
    }
    public function get_data($tbl='',$where=''){
        if(!empty($where)){
            $this->db->where($where);
        }
        return $this->db->get($tbl);
    }
    public function get_data_limit($tbl='',$where='',$limit='',$offset=''){
        if(!empty($where)){
            $this->db->limit($limit,$offset);
            $this->db->where($where);
        }
        return $this->db->get($tbl);
    }    
    public function get_data_login($tbl='',$where='',$limit=''){
        if(!empty($where)){
            $this->db->limit($limit);
            $this->db->where($where);
        }
        return $this->db->get($tbl);
    }
    public function count_all_data($tbl='',$where=''){
        if(!empty($where)){
            $this->db->where($where);
        }
        return $this->db->count_all_results($tbl);
    }
    public function update_data($tbl='',$update_data='',$where=''){
        if(!empty($where)){
            $this->db->where($where);
        }
        return $this->db->update($tbl,$update_data);
    }
}
\$\endgroup\$
  • \$\begingroup\$ I think security is there but you need to use either a helper or library and it should configured to autoload and hence these login check function will be available throughout of your application and you can use it any where in your application. \$\endgroup\$ – shankar kumar Jul 7 '15 at 7:46
  • \$\begingroup\$ Why I will use helper/library for login check ?? I can check session->userdata('logged_in') true or false. Is not it ? Please review my model. \$\endgroup\$ – Kabir Hossain Jul 7 '15 at 7:51
  • \$\begingroup\$ I did not have doubt about that, I am saying that if you use helper or library then there is a feature for autoload in codeigniter and hence these function will be available in whole application. functions are correct. \$\endgroup\$ – shankar kumar Jul 7 '15 at 7:53
  • \$\begingroup\$ @shanusingh The author asked specifically for a review of his code/approach that he took. It would improve your answer if (in addition to your suggestions) you'd answer his questions and/or review his code. \$\endgroup\$ – Joeblade Jul 7 '15 at 8:00
  • \$\begingroup\$ @Joeblade : I have given a solutions for global model: what it means by that : means that you are writing model code in base model and then it will be extended by every model and hence I am saying specifically that instead of global model, we can use helper or libraries. if you want specific review of code then I am saying that function you written is good for add, edit and list but it can be optimize like you should use overloading and create one function for listing get_data and get_data_limit. you can inhance return data as well by retuning $this->db->result(); \$\endgroup\$ – shankar kumar Jul 7 '15 at 8:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.