-2

I am trying to create new accounts and have the signup details for each user enter into my membership database as encrypted - at the moment the actual password can be seen in the database. Why is this? I am using the form validation library and have included the md5 rule for the password field but it doesnt seem to have made a difference.

Controller:

 function register()
 {

$this->load->helper(array('form', 'url'));

$this->load->library('form_validation');


$this->form_validation->set_rules('username', 'Username', 'required|callback_usernameTaken|min_length[5]|max_length[12]|trim');
$this->form_validation->set_rules('password', 'Password', 'required|md5|trim');


$username = $this->input->post('username');
$password = $this->input->post('password');


if ( $this->form_validation->run()&& !$this->membership->usernameTaken($username)){

$this->membership->newUser($username, $password);
$this->session->set_userdata('status', 'OK');
$this->session->set_userdata('username', $username);
redirect('home');
 }


else 
   {

    $this->session->set_userdata('status', 'NOT_OK');
    $this->load->view('shared/header');
    $this->load->view('account/signuptitle');
    $this->load->view('account/signupview');
    $this->load->view('shared/footer');

  }

 }

Thanks again for the help

Model:

 function newUser($username, $password)
{
    $newMember = array ('username' => $username,
                        'password' => $password);
    $insert = $this->db->insert('membership', $newMember);

}



function usernameTaken($username)
{
    $this->db->select('*')->from ('membership')->where('username', $username);
    $query = $this->db->get();
    if ($query->num_rows > 0)
    {
        echo "<p>";
        echo 'username taken';
        echo "</p>";
        return true;
    }

   else{

    return false;

   }    
}
Frank_g
  • 21
  • 1
  • 1
  • 5

2 Answers2

2

The form validation rules validate the form values, it does not change them so if you want to encrypt your password, you would have to do that after the form has passed validation / before you add it to the database.

Apart from that md5 is not a very good way to encrypt your password and it does not return a boolean value (true or false), it returns a string which will always evaluate to true so it is of no use in the validation function.

jeroen
  • 91,079
  • 21
  • 114
  • 132
  • so what alternative would you suggest? – Frank_g Dec 29 '12 at 18:49
  • 1
    @Frank_g bcrypt probably, depending on what is available, see also: http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords – jeroen Dec 29 '12 at 18:53
0

You are assigning $username and $password before running the validation. set_rules method just sets the rules for validation. It doesn't run it. They are validated at $this->form_validation->run() method. So just place those assignment statements inside if block.

It is better to use trim first in set_rules method. i have used it as first rule. You are already checking callback_usernameTaken in set_rules method so no need to check it again in if statement. usernameTaken is better to be in controller than in model. so moved it there. And finally check out the changes i made to usernameTaken method. I have added $this->form_validation->set_message method call and interchanged true and false.

function register()
{

    $this->load->helper(array('form', 'url'));

    $this->load->library('form_validation');

    $this->form_validation->set_rules('username', 'Username', 'trim|required|callback_usernameTaken|min_length[5]|max_length[12]');
    $this->form_validation->set_rules('password', 'Password', 'trim|required|md5');

    if ( $this->form_validation->run() ){

        $username = $this->input->post('username');
        $password = $this->input->post('password');

        $this->membership->newUser($username, $password);
        $this->session->set_userdata('status', 'OK');
        $this->session->set_userdata('username', $username);
        redirect('home');
    }
    else {

        $this->session->set_userdata('status', 'NOT_OK');
        $this->load->view('shared/header');
        $this->load->view('account/signuptitle');
        $this->load->view('account/signupview');
        $this->load->view('shared/footer');
    }
}

function usernameTaken($username)
{
    $this->db->select('*')->from ('membership')->where('username', $username);
    $query = $this->db->get();
    if ($query->num_rows > 0)
    {
        $this->form_validation->set_message('usernameTaken', 'username taken');
        return false;
    }
    else{
        return true;
   }
}

CodeIgniter's form-validation class provides both form validation and data prepping features. http://ellislab.com/codeigniter/user-guide/libraries/form_validation.html So there is nothing wrong in using md5 in set_rules method. It will run md5 method and return the encrypted string.

And for better encryption use Mcrypt extension: php.net/manual/en/intro.mcrypt.php or crypt function: php.net/manual/en/function.crypt.php.

srikanth wgl
  • 18
  • 2
  • Do you mean put this in the if statement? - $username = $this->input->post('username'); $password = $this->input->post('password'); – Frank_g Dec 29 '12 at 19:34
  • Yes. Try it and let us know if it is worked. – srikanth wgl Dec 29 '12 at 19:37
  • And for better encryption use `Mcrypt` extension: http://php.net/manual/en/intro.mcrypt.php or `crypt` function: http://php.net/manual/en/function.crypt.php. – srikanth wgl Dec 29 '12 at 19:43
  • I tried this but now I get Message: Undefined variable: username. This is because I dont want an already taken username going into the database. Please see the if statement - I have removed the ! from it in an effort for the function usernameTaken will be taken into account. Please see my updated code for my model function. – Frank_g Dec 29 '12 at 20:09
  • I have edited my answer. It is better to use `trim` first in set_rules method. i have used it as first rule. You are already checking `callback_usernameTaken` in set_rules method so no need to check it again in if statement. `usernameTaken` is better to be in controller than in model. so moved it there. And finally check out the changes i made to `usernameTaken` method. I have added `$this->form_validation->set_message` method call and interchanged `true` and `false`. – srikanth wgl Dec 29 '12 at 20:44