Welcome Guest, Not a member yet? Register   Sign In
Iv created a basic auth using datamapper 1.8, Could expert look at code and offer suggestions?
#1

[eluser]Andy78[/eluser]
I'm creating my auth system for the website because 1.I'm using datamapper and there are no existing auth library's ready to go using that and 2. so that I fully understand it.

The site is a bare bones ci install with datamapper 1.8 added and the following functions working:

create user accounts
user account activation
Login/logout
reset password

The site also has a frontpage and an admin page which uses My_controller to check if the user is logged in and if their user_type is admin before allowing them access to it.

I'm still kinda a beginner programmer and what I'm asking for here is an expert to go through my code make sure that what I'm doing is secure and also offer suggestions on how this could be improved.

Iv attached the files here including the database .sql file. You will also need to be able to receive emails from localhost for user activation and password resetting.
#2

[eluser]Andy78[/eluser]
In case its not worth downloading the code and looking at it here is the main controllers:

Code:
<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class MY_Controller extends CI_Controller {
    
    function __construct()
    {
        parent::__construct();
        $this->is_logged_in();
    }  
    
    function is_logged_in()
    {
        $is_logged_in = $this->session->userdata('is_logged_in');
        $user_type = $this->session->userdata('user_type');
        
        //user is not logged in so redirect back to the admin login form
        if(!isset($is_logged_in) || $is_logged_in != true)
        {
            redirect('account/login');
        }
        
           //user is not an admin so do not log them in and redirect back to the admin login form
        if (!isset($user_type) || $user_type != 'admin')  
        {
              //$this->session->sess_destroy();
              $this->session->set_flashdata('error', 'You do not have permission to access this page.');
              redirect('account/login');
        }        

    }    
}

Code:
<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed');

class Admin extends MY_Controller {

    function __construct()
    {
        parent::__construct();
    }
      
    
    public function index()
    {
        
        $data['main_content'] = 'admin/dashboard';
        $this->load->view('admin/template', $data);
    }
    
    
}
#3

[eluser]Andy78[/eluser]
This is the biggy over two posts

Code:
class Account extends CI_Controller {

    function __construct()
    {
        parent::__construct();
    }
    
    //Handels the logins to the admin section of the site
    function login()
      {        
         //Standard CI validation used on login form
         $this->load->library('form_validation');
         $this->form_validation->set_rules('username', 'Username', 'required');
         $this->form_validation->set_rules('password', 'Password', 'required');
        
         if ($this->form_validation->run() == FALSE)
         {
            $data['username'] = $this->input->post('username');
            $data['error'] = '';
            $data['main_content'] = 'admin/login_form';
            $this->load->view('admin/template', $data);
         }
         else
         {
          
              // Create user object
              $u = new User();
              
              // Put user supplied data into user object
              $u->username = $this->input->post('username');
              $u->password = $this->input->post('password');
              
              
              if ($u->validate_credentials()) // if the user's credentials validated...
              {    
                $data = array(
                      'username' => $u->username,
                      'is_logged_in' => true,
                    'user_type' => $u->user_type
                  );
                
                $this->session->set_userdata($data);
                  
                if($u->user_type == 'admin')
                {
                       redirect('admin');
                }
                else
                {
                    redirect('frontpage');
                }
              }
              else // incorrect username or password
              {  
                $data['username'] = $u->username;
                $data['error'] = 'Incorrect username or password';//Show the custom login error message
                $data['main_content'] = 'admin/login_form';
                $this->load->view('admin/template', $data);
              }
                
        }
      }

Code:
//directs users to the signup form
     function signup()
     {
        $is_logged_in = $this->session->userdata('is_logged_in');
          //check to make sure user is not logged in when resetting the password
          if($is_logged_in == true)
          {
            redirect('frontpage');
          }
        
        $data['main_content'] = 'admin/signup_form';
        $data['errors'] = '';
        $this->load->view('admin/template', $data);
      }
    
     //logs out users by clearing the session
     function logout ()
     {
         $this->session->sess_destroy();
          redirect('admin');
        
     }
    
     //Used with the signup form to created new users.
     function registration()
     {        
        $this->load->library('email');//Load email library
        
        $is_logged_in = $this->session->userdata('is_logged_in');
          //check to make sure user is not logged in
          if($is_logged_in == true)
          {
            redirect('frontpage');
          }
        
        //Create user object
        $user = new User();
        $user->username = $this->input->post('username');
        $user->password = $this->input->post('password');
        
        //if you define a validation rule on a field called ‘confirm_password’, your Datamapper object must contain a property called ‘confirm_password’.
        $user->confirm_password = $this->input->post('confirm_password');  
        //set objects email equal to inputted email
        $user->email_address = $this->input->post('email_address');
        //Generate a random hash and store it to use in the user activation process.
        $user->activation_code = substr(sha1(mt_rand()),0,35);
        
        //Set up activation email settings
        $this->email->from('[email protected]', 'Admin');
        $this->email->to($this->input->post('email_address'));
        $this->email->subject('Complete Registration');
        //set email contents
        $message = "Hello $user->username, \n\nThank you for registering with with mysite.\n\n\nUsername : $user->username \n\nPassword : $user->password \n\n\nPlease click on the link in order to activate your account\n\n";
        $message .= site_url()."/account/activate/".$user->activation_code;
        
        $this->email->message($message);
              
            if($user->save())
            {
                $this->email->send();//send activation email
                $data['main_content'] = 'admin/activation_email';
                $this->load->view('admin/template', $data);
            }
            else
            {
                  $data['main_content'] = 'admin/signup_form';
                $data['errors'] = $user->error->string;
                $this->load->view('admin/template', $data);        
            }    
      }

Code:
//activates users via the link within the email
      function activate()
      {        
         // Create user object
         $user = new User();
         //Set user objects activation code equal to code in the email link
         $user->activation_code = $this->uri->segment(3);
              
         if($user->activate_user())
         {
            $data['main_content'] = 'admin/signup_successful';
            $this->load->view('admin/template', $data);
         }
         else
         {
            $data['main_content'] = 'admin/activation_failed';
            $this->load->view('admin/template', $data);
         }
      }
#4

[eluser]Andy78[/eluser]
the second part of the account controller:

Code:
//forgot password
      function forgot_password()
      {
          $this->load->library('email');//Load email library
          
          $is_logged_in = $this->session->userdata('is_logged_in');
          //check to make sure user is not logged in when resetting the password
          if($is_logged_in == true)
          {
            redirect('frontpage');
          }
          
          //Standard CI validation used on email form
          $this->load->library('form_validation');
          $this->form_validation->set_rules('email', 'Email address', 'trim|required|valid_email');
          //run form validation
          if ($this->form_validation->run() == FALSE)
          {
             $data['main_content'] = 'admin/forgot_password';
             $data['errors'] = '';
             $this->load->view('admin/template', $data);
          }
          else
          {
              //Create user object
              $user = new User();
              //set objects email equal to inputted email
              $user->email_address = $this->input->post('email');
                            
              //Check if user record with that email address exists
              if($user->reset_password())
              {                
                 //get user record with that email address
                 $user->where('email_address', $this->input->post('email'));
                 $user->get();
                 //create new password string
                 $user->password = $user->createRandomPassword();
                
                 //Set up reset passward email
                 $this->email->from('[email protected]', 'Admin');
                 $this->email->to($user->email_address);
                 $this->email->subject('Your passward has been reset');
                 //set email contents
                 $message = "Hello $user->username, \n\nAs you requested, your password has now been reset. Your new details are as follows:\n\n\nUsername : $user->username \n\nPassword : $user->password \n\n\nTo change your password, please visit this page:\n\n";
                 $message .= site_url()."/account/change_password/";
                 $this->email->message($message);  
                
                //Update the users password and send the email. Then load the success view
                 if($user->save())
                 {
                     $this->email->send();//send activation email
                     $data['main_content'] = 'admin/password_reset_successful';
                     $this->load->view('admin/template', $data);
                 }
                 //There was some kind of error updating the users records
                 else
                 {
                     $data['main_content'] = 'admin/forgot_password';
                     $data['errors'] = 'There was an error resetting your password!';
                     $this->load->view('admin/template', $data);
                 }          
                
                
                  
              }
              //User account does not exist or is not active
              else
              {
                 $data['main_content'] = 'admin/forgot_password';
                 $data['errors'] = 'There is no matching email address in our records!';
                 $this->load->view('admin/template', $data);
              }
              //$this->output->enable_profiler(TRUE);
          }
        
      }
        
}
#5

[eluser]Andy78[/eluser]
Finally here is the model
Code:
<?php
/**
* User Class
*
* Transforms users table into an object.
*
* @category    Models
* @author        Andrew Mclaughlan
*
*/
class User extends DataMapper {
  
       // Insert related models that Template can have just one of.
    var $has_one = array();

    // Insert related models that Template can have more than one of.
    var $has_many = array();

    // --------------------------------------------------------------------
    // Validation
    // --------------------------------------------------------------------
    var $validation = array(
    'username' => array(
        'label' => 'Username',
        'rules' => array('required', 'trim', 'unique', 'min_length' => 4, 'max_length' => 20)
    ),
    'password' => array(
        'label' => 'Password',
        'rules' => array('required', 'trim', 'min_length' => 6, 'encrypt')
    ),
    'confirm_password' => array( // accessed via $this->confirm_password
        'label' => 'Confirm Password',
        'rules' => array('encrypt', 'matches' => 'password')
    ),
    'email_address' => array(
        'label' => 'Email Address',
        'rules' => array('required', 'trim', 'unique', 'valid_email')
    )
);
      
    // Optionally, don't include a constructor if you don't need one.
    function __construct($id = NULL)
    {
        parent::__construct($id);
    }

    // Optionally, you can add post model initialisation code
    function post_model_init($from_cache = FALSE)
    {
    }
    
    //Validates the username and password
    function validate_credentials()
    {
       $username = $this->username;
       $password = $this->password;
       $active = 'active';
      
        // Get this users stored record via their username
        $this->where('username', $username);
        $this->where('user_status', $active); //Checks to make sure the account is active
        $this->get();
        
         // Did we find this user?
        if ($this->exists())        
        {
            //check the password
            if ( $this->password == sha1($this->salt . $password) )
            {
                return TRUE;
            }
          
        }
        else
        {
            // restore username for login field
            $this->username = $username;

            return FALSE;
        }  
    }
    
    //Checks if the activation code exists then sets the user_status to active
    function activate_user()
    {
        $activation_code = $this->activation_code;
        //find the user with the activation_code that matches the code in the link
        $this->where('activation_code', $activation_code)->get();
        //if the user exists
        if ($this->exists())        
        {
            $this->user_status = 'active'; //set the user status to active
            $this->activation_code = ''; //clear the activation code
            $this->save(); //update record as above
            return TRUE;
        }
        else
        {
            return FALSE;
        }  
        
    }
    
    //Reset the users password
    function reset_password()
    {
        $active = 'active';
        $this->where('email_address', $this->email_address);
        $this->where('user_status', $active); //Checks to make sure the account is active
        $this->get();
        
        if ($this->exists())
        {
            return true;    
        }
    }
  
    
    // Validation prepping function to encrypt passwords
    function _encrypt($field)
    {
        // Don't encrypt an empty string
        if (!empty($this->{$field}))
        {
            // Generate a random salt if empty
            if (empty($this->salt))
            {
                $this->salt = substr(sha1(mt_rand()),0,22);
            }

            $this->{$field} = sha1($this->salt . $this->{$field});
        }
    }
    
    //Generates a random password
    function createRandomPassword()
    {
        //The letter l (lowercase L) and the number 1 have been removed,
        // as they can be mistaken for each other.
        $chars = "abcdefghijkmnopqrstuvwxyz023456789";
    
        srand((double)microtime()*1000000);
    
        $i = 0;
    
        $pass = '' ;
    
        while ($i <= 7) {
    
            $num = rand() % 33;
    
            $tmp = substr($chars, $num, 1);
    
            $pass = $pass . $tmp;
    
            $i++;
    
        }
    
        return $pass;
    }

}

/* End of file name.php */
/* Location: ./application/models/user.php */
#6

[eluser]bEz[/eluser]
Explain the purpose of the reset_password() function you included in the model.
Code:
//Reset the users password
    function reset_password()
    {
        $active = 'active';
        $this->where('email_address', $this->email_address);
        $this->where('user_status', $active); //Checks to make sure the account is active
        $this->get();
        
        if ($this->exists())
        {
            return true;    
        }
    }

You referenced it in the controller with:
Code:
//Check if user record with that email address exists
              if($user->reset_password())
              {  
...
#7

[eluser]Andy78[/eluser]
Maybe I should have called it something different as it does not actually reset the password it just checks if a record in the users table exists with the supplied email address and if that record is set to active. If so it returns true and the controller carries on.
#8

[eluser]bEz[/eluser]
O Ok... just making sure you didn't get side-tracked, because it's quite misleading (although I knew what it actually did).

On another note, have you considered using a TINYINT or BOOLEAN value (or even a user_status_id from a User_Status TABLE) instead of a string value?
#9

[eluser]Andy78[/eluser]
I have, I'm just trying to keep it simple at the moment.

Basically I'm more interested in the security of what I'm doing. Will this provide a secure login system that will prevent unauthorised users from accessing the admin part of the site and will it be hack proof and so on.
Also looking to find the best way of implementing a remember me function into this.

Should most of what Iv put in the account controller be put into a library or is it ok where it is?

Could it be more streamlined and made more efficient?
#10

[eluser]m4ikel[/eluser]
Looks good to me, about your library question: It becomes usefull when your going to share auth logic throughout the application, for example you could store the login validation, account recovery and for example password generation process inside a separate library because I would guess users will also be able to change their password in other parts of the application.. keeping the hash generation at a centralized location will save you from a headache in the future.




Theme © iAndrew 2016 - Forum software by © MyBB