CodeIgniter Forums

Full Version: Looking for little bit of advice on someting
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Hi all, first i'll say i am a newbie to Frameworks so this might seem like a pretty dumb question but i was just looking for a little bit of advice on something.

Currently i'm doing a project and have a "members" area so i have tried to develop a login/registration system. Its only basic at the moment as i've just started and so far everything appears to function as it should (i.e can register a user, login,  only a logged in member can view a certain page, if a users account status is a certain status then login fails and informs the user of whats wrong)

So, i was wondering if i was doing this the correct way for the login process (new to the whole controller/model style)

In my "members" controller i have the following which processes the login (this function is called via another function if form validation is successful)

I know theres 3rd party auth libraries but i prefer to do it myself if possible so i know whats what.

This is the main part of the controller i'd like (if possible) any advice on

PHP Code:
public function processLogin()
{
 
   if$this->uri->uri_string() == 'members/processLogin'show_404();
            
    
$this->load->model('user');
    
$valid_user=$this->user->validate();
        if(!
$valid_user) {
            
$DMData= array(
                    
'DisplayMsg'  => TRUE,
                    
'DisplayMsgType' => 'danger',
                    
'DisplayMsgContent' => $this->lang->line('members_invalid_login')
                    );
            
$this->session->set_flashdata($DMData);        
            
redirect('/members/index/');
    } else {
        
        
// User account is active so lets let them in
        
if ($valid_user['status'] == $this->config->item('account_active')) {
            
            
// Set session information
            
$this->session->set_userdata($valid_user);
            
            
$DMData= array(
                
'DisplayMsg'  => TRUE,
                
'DisplayMsgType' => 'success',
                
'DisplayMsgContent' => $this->lang->line('members_login_success')
            );
        
            
$this->session->set_flashdata($DMData);    
            
redirect('/members/upanel/');
        
        
// account is pending so lets tell them
        
} elseif ($valid_user['status'] == $this->config->item('account_pending')) {
            
$DMData= array(
                
'DisplayMsg'  => TRUE,
                
'DisplayMsgType' => 'danger',
                
'DisplayMsgContent' => $this->lang->line('members_account_pending')
            );
            
$this->session->set_flashdata($DMData);        
            
redirect('/members/index/');
        
        
// Account is suspended so lets tell them?    
        
} elseif ($valid_user['status'] == $this->config->item('account_suspended')) {
            
$DMData= array(
                
'DisplayMsg'  => TRUE,
                
'DisplayMsgType' => 'danger',
                
'DisplayMsgContent' => $this->lang->line('members_account_suspended')
            );
            
$this->session->set_flashdata($DMData);        
            
redirect('/members/index/');

            
// Unknown account status returned or closed account
        
} else {
            
$DMData= array(
                
'DisplayMsg'  => TRUE,
                
'DisplayMsgType' => 'danger',
                
'DisplayMsgContent' => $this->lang->line('members_account_unknown')
            );
            
$this->session->set_flashdata($DMData);        
            
redirect('/members/index/');
        }
    }



Then the main login check done in the model is:


PHP Code:
public function validate() {
    
// Data from the login form
    
$username=$this->input->post('username');
    
$password=$this->input->post('password');

    
// Get info for user and verify password is correct
    
    
$sql "SELECT * FROM users WHERE username = ?";
    
$query $this->db->query($sql, array($username));
    
    if(
$query->num_rows()==1){
        
$row=$query->row();
            
$data = array('pass' => $row->password
            
);
    
            
$hashedPass $data['pass'];
            
// Check if provided password matches the user
            
if (password_verify($password$hashedPass)) {
                
                
// Password matches
                
$UserActData = array (
                    
'userID' => $row->id,
                    
'userNAME' => $row->username,
                    
'status' => $row->status,
                    
'everified' => $row->email_verifed,
                    
'LoggedIn' => TRUE
                
);
                                                
                return 
$UserActData;
            } else {
                
// Password does NOT match
                
return false;
            }
    }
    
//User not found
    
return false;

 } 



So am i on the right path or completely wrong? haha
My advice is, unless you REALLY know what you're doing, do not design your own auth system.
(03-19-2016, 10:03 PM)albertleao Wrote: [ -> ]My advice is, unless you REALLY know what you're doing, do not design your own auth system.

Is that you're way of saying I've done something wrong or just general advice lol

Obviously you can't learn if you don't try doing something yourself Smile
(03-20-2016, 07:14 AM)CodieNewbie Wrote: [ -> ]
(03-19-2016, 10:03 PM)albertleao Wrote: [ -> ]My advice is, unless you REALLY know what you're doing, do not design your own auth system.

Is that you're way of saying I've done something wrong or just general advice lol

Obviously you can't learn  if you don't try doing something yourself Smile

My original comment was just some advice in general. 

Skimming through your code, I've noticed that you're breaking the MVC pattern by requesting the input data in your model. Generally speaking, it's bad practice to do that. Not only is it hard to scale, because once you have hundreds of models and controllers you'll have no idea where inputs are being handled, it also makes your models much harder to test.

Also, I can't see you password_verify() function so it's hard to say how that works or if you're doing something that would put you in danger of some SQL injection or if you're hashing your passwords in a good manner (md5 is not good practice)

While I understand the mentality of "If I build it, I know how it works", when it comes to security, you're much better off using a library where you're gaining from the collective knowledge of all the people who have contributed to the library. Based on your code, and your username being "CodieNewbie" I'd recommend using a proven library. You won't have to worry about hashing algorithms, sql injection, or any other things that you may not have done before. 

I'd recommend reading more about the benefits of the MVC pattern, why and how it's used. I'd also recommend reading up on unit testing and unit testing frameworks. Even if you decide not to unit test (which i recommend you do), understanding the concepts of testing will help wrap you head around how classes and functions should be written.
Just to add, there's nothing preventing you from using a 3rd party auth library and reading through the code so you know what is what. Most auth libraries have excellent documentation too so you're not left hanging.
Thanks for the tip on requesting input data in the model Smile

the "password_verify()" function is core php (PHP 5 >= 5.5.0) which verifies passwords created with "password_hash()" which uses the bcrypt algorithm by default.

As for auth i guess your right about using a 3rd party option.

Thanks again Smile
(03-20-2016, 09:14 AM)CodieNewbie Wrote: [ -> ]Thanks for the tip on requesting input data in the model Smile

the "password_verify()" function is core php (PHP 5 >= 5.5.0) which verifies passwords created with "password_hash()" which uses the bcrypt algorithm by default.

As for auth i guess your right about using a 3rd party option.

Thanks again Smile

Ahh right. It's good you are using those core functions. I've seen people try to roll their own hashing algorithms, and it usually ends up being terrible.

There are many auth libraries out there. I know Ion auth and Community auth are very popular amongst codeigniter devs. You can also find several auth libraries on Packagist. 

As for your model and controller structures, I like to keep the model as isolated as possible. My model functions either return an object, or return NULL so I can unit test everything. Form data always gets filtered in my controllers before touching any of my models, and on top of the form validation, I validate data in the model before putting anything in my database. The way you have your model working now is the definition of coding with 'side effects'

Though is is meant for another programming language (Erlang/Elixir), this is a very good article to read about side-effect: http://blog.jenkster.com/2015/12/what-is...mming.html
A bit off topic, apologies in advance.

(03-20-2016, 09:56 AM)albertleao Wrote: [ -> ]... this is a very good article to read about side-effect: http://blog.jenkster.com/2015/12/what-is...mming.html

Ooh, very interesting article. Sadly, I definitely write impure functions, and it has caused issues in the past.

However, not any more :-)

As an even further off topic aside, I spent ages (and got very frustrated at times) with a strictly DRY approach and a logic layer before the database layer. However, since then, and rewriting bits of code or adding some functionality, it is been a pleasure updating my own code, almost for the first time ever. So fast compared to previous sites I have written. Pure functions, I am sure, will help that a lot too.

Thanks for sharing,

Paul.

PS As I feel guilty about being off topic, on the topic of auth libraries, I rolled my own using as much best practice as I could, and although it works great, and I have used in on many sites, in hindsight, I should have used an auth library. Especially one that is mature, well developed, survived criticism, hacking, abuse and even life in the wild. Whatever you produce on your own will not (unless you are of course an exceptional programmer already working in the security environment) come close to being as secure or robust as some of the great existing libraries, nor offer the complex features available in some of these libraries. The learning curve to use someone elses package is well worth the investment in time. However exciting and 'do-able' it seems, security for authorisation is simply a vast and complex subject all on it's own. Why re-invent the wheel? I did, and although I got something wheel shaped, I regret doing it and although I learned a lot in the process, I will not be doing it again.
The article I mentioned is talking about some lesser known programming languages like Erlang/Elixir which are functional. While functional programming doesn't really fit in php per say, you can learn a lot of things from it even though you won't get the full benefits. Erlang is a beast of a programming language and when used correctly can blow the top off of more common languages like PHP and such.

In conclusion: Write clean code, use proven libraries...