Jump to content


Photo

How Do I Check If A Row Exists In My Form Validation Rules?


  • Please log in to reply
6 replies to this topic

#1 RalphLeMouf

RalphLeMouf

    Advanced Member

  • Members
  • PipPipPip
  • 153 posts

Posted 05 December 2012 - 01:50 PM

Hey there. I am finalizing all of the errors on my forms. For example, on my login form where the user is prompted to enter their email and password, I have the errors working in order when the password and emails don’t match or it’s not a valid email, BUT if I were to type in an email that is NOT in the database, it only poses error password because it recognizes it as a valid email.
I would prefer not to write any new functions if possible and tie it into my existing login function.
Here is the model and controller for said module:

//MODEL


public function validate_home_login($data)
{
// TAKING THE DATA FROM THE MODEL AND CHECKING IT AGAINST THE STORED INFO IN THE DB
$query = $this->db->where($data)->get('users', '1');

if($query->row())
{
return $query->row();
$valid_email = $row->email;
}
}




//CONTROLLER


public function validate_credentials_login()
{
  $this->load->library('session');
  $this->load->helper(array('form','url'));
  $this->load->model('user_model', 'um');
  $this->load->library('encrypt');
  $this->load->library('form_validation');


  $this->form_validation->set_rules('email_login', 'Email', 'trim|required|valid_email|'); 
  $this->form_validation->set_rules('password_login', 'Password', 'trim|required');


  if ( $this->form_validation->run() === TRUE ) 
  {   
   $user = $this->um->validate_home_login(array('email' => $this->input->post('email_login')));

// if $user exists
   if ( $user ) {
// if the password is correct
    if ( $user->password == $this->encrypt->sha1( $user->salt .  $this->encrypt->sha1($this->input->post('password_login'))) && $user->email == $this->input->post('email_login') ) 
    {
     $this->session->set_userdata(array('email' => $this->input->post('email_login')));
     redirect('account/dashboard');

// if the password is incorrect
    }else{
        $this->form_validation->run() == FALSE;
  $data['password_error'] = 'The password field is invalid.';
    }

// if $user doesn't exist
   }else{
$this->form_validation->run() == FALSE;
$data['email_error'] = 'This email is invalid.';

}
  }

 $data['main_content'] = 'home/home_page';
 $this->load->view('includes/templates/home_page_template', $data);   

   }


thanks in advance

Edited by RalphLeMouf, 05 December 2012 - 01:52 PM.


#2 Christian F.

Christian F.

    Advanced Member

  • Staff Alumni
  • 3,106 posts
  • LocationNorway

Posted 05 December 2012 - 07:49 PM

You actually don't want to give any other message than "e-mail and/or password invalid", so I recommend removing the "invalid e-mail" error message. Reason for this is that if you tell an attacker that an e-mail they tried is valid, but the password is wrong, they can focus their efforts on attacking only the password. Instead of having to try to guess the right combinations of both.
The users know their e-mail, and password, and thus can easily check which of them was wrong.

I'd also like to give you a general advice on your coding style, to help make your scripts a bit more readable. That is to test for failure/error conditions, and exit early.
Unfortunately it seems a lot of people only test for success conditions, and thus ends up having to nest their code a lot more than what's strictly necessary. Something which makes the script harder to read, especially as it grows bigger, as you can't easily attach the error messages to the condition tests.

In addition to that it is not a good idea to manipulate user input in any form, this includes trimming it.
Passwords in particular are extremely bad to manipulate, as you're effectively decreasing their entropy, which makes them easier to crack. Who's to say that adding a space to the start or end of your password is a bad thing?

That said I've altered your code a bit, to give you an example of what I've outlined in this post. Also added a few comments to explain what I've done, as well as noting some things that should/needs to be done.
Spoiler


I've prefixed my comments with "CF:", and the todo-notices with.. Well, you can probably guess that one. :P

Edited by Christian F., 05 December 2012 - 07:50 PM.

Keeping it simple.

#3 RalphLeMouf

RalphLeMouf

    Advanced Member

  • Members
  • PipPipPip
  • 153 posts

Posted 06 December 2012 - 01:42 PM

Great feedback! I really appreciate it. As I'm still shaky and new with this and code-igniter all together, is there anyway you could re-write what I wrote the way you think it should be? I'm not trying to get you to write my code for me, I just think it would be easier for me to understand what your trying to get across rather than me putting it together based off of your recommendations and being usure of exactly what you mean.

thanks

#4 Christian F.

Christian F.

    Advanced Member

  • Staff Alumni
  • 3,106 posts
  • LocationNorway

Posted 06 December 2012 - 02:09 PM

Try clicking on the "Spoiler show" button. ;)

Edited by Christian F., 06 December 2012 - 02:09 PM.

Keeping it simple.

#5 RalphLeMouf

RalphLeMouf

    Advanced Member

  • Members
  • PipPipPip
  • 153 posts

Posted 06 December 2012 - 02:40 PM

well yeah I did. I tried that code chunk but it's posing errors

#6 Christian F.

Christian F.

    Advanced Member

  • Staff Alumni
  • 3,106 posts
  • LocationNorway

Posted 06 December 2012 - 02:53 PM

Of course it does, that's why I left the TODOs in there. Also, I've obviously not tested it as I don't have access to the rest of your code or your DB.
You'd just have to pick at the errors, and see if you can't clear them up. Paying close attention to what hey say, and if you find them difficult to understand; Search online. Should you still have problems, then post the error messages here, along with your updated code, and we should be able to help.
Keeping it simple.

#7 Psycho

Psycho

    Advanced Member

  • Gurus
  • 10,753 posts
  • LocationCanada

Posted 06 December 2012 - 03:24 PM

In addition to that it is not a good idea to manipulate user input in any form, this includes trimming it.
Passwords in particular are extremely bad to manipulate, as you're effectively decreasing their entropy, which makes them easier to crack. Who's to say that adding a space to the start or end of your password is a bad thing?


I agree with you on the basic statement of not manipulating input - except for trimming. I advocate always trimming input, unless there is a valid reason not to. And, of course, passwords are one of those exceptions. But, for "normal" inputs: username, emails, person name, address, phone, etc. leading and trailing spaces wound not make sense and the space bar is likely pressed as a mistake in those instance (the space bar is pressed 6 million times every second!). There isn't anything inherently wrong with not trimming, but that would require adding additional validation logic since you wouldn't want an input of nothing but spaces for things such as names, addresses, etc.
The quality of the responses received is directly proportional to the quality of the question asked.

I do not always test the code I provide, so there may be some syntax errors. In 99% of all cases I found the solution to your problem here: http://www.php.net




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users

Cheap Linux VPS from $5
SSD Storage, 30 day Guarantee
1 TB of BW, 100% Network Uptime

AlphaBit.com