Multi-field validation callbacks, the "login" form problem, etc.

[eluser]marmida[/eluser]
I want to use a validation callback function in a login form to do the authentication check. I found at least a few threads like this (forgot to link the others, sorry) that make me believe that several people are encountering this problem and that the "login form" scenario is a pretty good, concrete case to discuss.

The problem usually is reduced to the need to validate multiple inputs within the callback, which is typically answered through the visibility of $this->validation from within the called function. This doesn't cut it for me for the following reasons:

1) validation status - I don't want the database check run if there are errors in either of the username or password fields, e.g. if the username isn't a valid e-mail address, I don't want to even try hitting the database. Currently no validation callback can know the status of any other validation's execution. (It occurs to me in writing this that you might be able to dig into $this->validation->_error_array from the callback)

2) order of execution - it doesn't seem like the order of execution of the validation functions is guaranteed beyond their incidental sequencing in the calls to set_rules() and set_fields(). So if I want to hack around it by wrapping both field's rules in a callback and then storing extra data in my controller (e.g. setting $this->_lastLoginValid = true once the validation callback figures the login is not empty and is a valid e-mail address), it still might break if the implementation of Validation::run() changes and password's validation gets fired before login.

3) error message ownership - the whole design of CI's validation is centered around fields owning rules, which doesn't apply to this type of form. Which field should show the "invalid username and password combination" error message: login or password? In either case, it looks awkward (cosmetically fixable, to be sure) and betrays an underlying problem that this rule in particular doesn't apply to any one field.

I have a implementation for "global" validations and incorporating these to be run after the individual field validations working in my project, but am not certain if this is a good way to go about it. Is there a plan to add something like this in a future release? Is there another robust way to add this functionality using the current validation lib?

[eluser]mycroes[/eluser]
I think the validator definately helps you to do what you want. I was thinking about the same problem plus that I didn't know how I was going to handle user authentication at all. No matter how you do your user authentication, you can first let the validator run. If the validator fails, display the form again, if the validator passes, do your own checking on the fields the validator generates for you ($this->validation->username or substitute username with whatever you need). Sure you're still checking some stuff by hand, but you don't have to do the trivial checks this way...

[eluser]Michael Wales[/eluser]
The validation helper is perfect for login forms. Here's a really quick login form, using ErkanaAuth (this code can easily be adapted to not use ErkanaAuth though):

Code:

function index() {
  // If the user is not logged in - send them to the form
  if (!$this->erkanaauth->try_session_login()) redirect('admin/login');
  $this->load->view('controlpanel');
}

function login() {
  // If the user is already logged in - send them to the control panel
  if ($this->erkanaauth->try_session_login()) redirect('admin/index');

  $this->load->library('validation');
  $rules['username'] = 'trim|required|callback__check_login';
  $rules['password'] = 'trim|required';
  $this->validation->set_rules($rules);
  $fields['username'] = 'username';
  $fields['password'] = 'password';
  $this->validation->set_fields($fields);

  if ($this->validation->run()) {
    redirect('admin/index');
  } else {
    $this->load->view('login');
  }
}

function _check_login($username) {
  $this->load->helper('security');
  $password = dohash($this->input->post('password'));
  if ($this->erkanaauth->try_login(array('username'=>$username, 'password'=>$password)) {
    return TRUE;
  } else {
    $this->validation->set_message('_check_login', 'Incorrect login info');
    return FALSE;
  }
}

The only thing you really need to do is use a callback to check the login information. Unfortunately, callbacks only pass the value that calls them - but you can use the input class, the validation class, or the $_POST array to retrieve the other values after their sanitization and validation rules.

[eluser]mycroes[/eluser]
[quote author="walesmd" date="1195082632"]The validation helper is perfect for login forms. Here's a really quick login form, using ErkanaAuth (this code can easily be adapted to not use ErkanaAuth though):

Code:

...
function login() {
  // If the user is already logged in - send them to the control panel
  if ($this->erkanaauth->try_session_login()) redirect('admin/index');

  $this->load->library('validation');
  $rules['username'] = 'trim|required|callback__check_login';
  $rules['password'] = 'trim|required';
  $this->validation->set_rules($rules);
  $fields['username'] = 'username';
  $fields['password'] = 'password';
  $this->validation->set_fields($fields);

  if ($this->validation->run()) {
    redirect('admin/index');
  } else {
    $this->load->view('login');
  }
}

function _check_login($username) {
  $this->load->helper('security');
  $password = dohash($this->input->post('password'));
  if ($this->erkanaauth->try_login(array('username'=>$username, 'password'=>$password)) {
    return TRUE;
  } else {
    $this->validation->set_message('_check_login', 'Incorrect login info');
    return FALSE;
  }
}

The only thing you really need to do is use a callback to check the login information. Unfortunately, callbacks only pass the value that calls them - but you can use the input class, the validation class, or the $_POST array to retrieve the other values after their sanitization and validation rules.[/quote]

I have been looking at your code earlier today and what I really dislike is that you're neglecting all the validator can do with the password. The validator will trim, check if a password is given at all and whatever you want it to do, and your code just throws all that away... You can of course use the validator variable, but I didn't see any mention of sequences at all in the validator documentation, so you don't know if that variable has been processed yet.

That was just one point, the other point is that as the TS pointed out, he wants to only check username and password validity after the form is checked, that's only possible if you call your own functions after the validator has passed checking all variables.

Don't get me wrong, your code will work fine for a lot of people, I am just one person that isn't satisfied with it.
Regards,

Michael

[eluser]Michael Ekoka[/eluser]
Hello Marmida, I'm currently working on improving the validation library. I think validation is an essential part of database driven applications and I find that the current CI validation lib needs some steroids.

1) validation status - I don’t want the database check run if there are errors in either of the username or password fields, e.g. if the username isn’t a valid e-mail address, I don’t want to even try hitting the database.

You raise a good point there and I make note of it. I think what's missing is some kind of a rule flag that should be available for each field.
i.e. during validation run, if a field's rule fails a flag is raised, that contains the name of the failing rule for that field.

Code:

// if validation succeed for username we get this:
$validation->flags['username'] = '';
// if validation failed because the username is an invalid email address
$validation->flags['username'] = 'valid_email';

2) order of execution - it doesn’t seem like the order of execution of the validation functions is guaranteed beyond their incidental sequencing in the calls to set_rules() and set_fields(). So if I want to hack around it by wrapping both field’s rules in a callback and then storing extra data in my controller [...], it still might break if the implementation of Validation::run() changes and password’s validation gets fired before login.

I don't quite understand what you mean here. I have the sense that you still have some sort of control over the order of execution. To change the implementation of the run() method would mean to change the core of the validation class, in which case we would not be talking about the same validation.

3) error message ownership - the whole design of CI’s validation is centered around fields owning rules, which doesn’t apply to this type of form. Which field should show the “invalid username and password combination” error message: login or password? In either case, it looks awkward (cosmetically fixable, to be sure) and betrays an underlying problem that this rule in particular doesn’t apply to any one field.

The best way I've found to do this is to group related fields in forms:

Code:

<?php echo $this->validation->login_error ?>
<input name = 'login['username']' />
<input name = 'login['password']' />

I wanted to do something like this in my rules:

Code:

$rules['login[username]'] = 'trim|required';
$rules['login[password]'] = 'trim|required';
$rules['login'] = 'callback__do_login';
$fields['login'] = 'Login';

...

function _do_login($login){
    $username = $login['username'];
    $password = $login['password'];
    $this->validation->set_message('login','The username and/or password is incorrect');
    // this is probably where previous validation flags would be really handy
    // try to login
}

That's when I started bumping into the native Validation limitations and decided to hack through it.

[eluser]Michael Wales[/eluser]
No - the validator runs all of it's sanitization and applies it to the input class variables, validation class variables, and the $_POST array. Here's an easy way to check that - make a form with a text field, with rules of 'trim|required|md5'

In the view:

Code:

echo '"' . $this->input->post('field') . '"';
echo '<br />"' . $this->validation->field; . '"';
echo '<br />"' . $_POST['field'] . '"';

You will see they all echo the same thing - trimmed, md5'd, etc. So, we're not throwing anything away that the validation class offers us by referring to $this->input->post('password') in the callback function.

Checking username/password validity after the form is checked...

If you actually try the code above you will find the callback function is only called after all other rules for that field have been satisfied. Therefore, you won't get a "username is required" and an "invalid login" error at the same time. The only time you get dual errors is if password is left blank (invalid login and password required).

I see nothing wrong with the code I posted - I fulfills all requirements and is the standard way of processing forms of this nature within the CI community.
[/code]

[eluser]Michael Wales[/eluser]

1) validation status - I don’t want the database check run if there are errors in either of the username or password fields, e.g. if the username isn’t a valid e-mail address, I don’t want to even try hitting the database.

The database check won't run as long as valid_email precedes the callback function within the rules of that field.

The Validation class only checks for one fault per rule and outputs the error of that one fault.

[eluser]mycroes[/eluser]
[quote author="walesmd" date="1195088294"]

1) validation status - I don’t want the database check run if there are errors in either of the username or password fields, e.g. if the username isn’t a valid e-mail address, I don’t want to even try hitting the database.

The database check won't run as long as valid_email precedes the callback function within the rules of that field.

The Validation class only checks for one fault per rule and outputs the error of that one fault.[/quote]

I guess I wasn't clear enough, so I'll try to explain again with some code:

Code:

$rules['name'] = 'trim|required|exact_length[3]|callback__test';
$rules['password'] = 'trim|required|md5';


function _test($str) {
  echo $this->validation->password;
}

So what will this output? There's no documentation that says in what order the checks will execute, so we can have 2 cases:
1. first name is checked, during the check the _test function is called and the password isn't converted to md5 yet and the _test function will just print the text (password) entered by the user
2. password is checked first, after that name is checked and the _test function will output the md5 encrypted password.

See where I'm going? I didn't even think about your point that the $this->input->post('varname') was changed too, so my point of throwing stuff away the validator was doing was wrong, thanks for pointing that out. Still though, if you want to compare a entered password with the md5 encrypted password in a database you need to know if the validator already processed it and that's what missing. If we're able to set the order/sequence of checks (i.e. $rules = 'name(trim|required|exact_length[3])password(trim|required|md5)name(callback__test)') then this would be solved and the database will only be queried if all fields are valid.
Regards,

Michael

[eluser]marmida[/eluser]
[quote author="CodeIgniter" date="1195087787"]You raise a good point there and I make note of it. I think what's missing is some kind of a rule flag that should be available for each field.
i.e. during validation run, if a field's rule fails a flag is raised, that contains the name of the failing rule for that field.[/quote]

This is a possible solution. But it also encourages the use of callbacks to provide dependency and chaining for validations, and this I think is the central topic I'm bringing up - the current validation system does not support anything beyond the scope of a single field at a time, even if arguments are provided that relate to other fields. By using callbacks, it falls outside the scope of CI, which is fine.

An example and a counterproposal:

Say we have a form with zip and country fields. If the country is US, then the zip must be validated in US zip format. If the country is Canada, then the zip must be validated by Canadian zip code regex. If the country is anything else, we ignore zip entirely (because we're jerks).

I've worked with systems with encapsulated rules that would express this in a CI-translated way like:

Code:

$this->set_rules(array('zip' => 'trim|if[country==ID_USA]{valid_us_zip}else if[country==ID_CANADA]{valid_canada_zip}));

This is however a single rule that applies only to a single field and doesn't really touch on the subject of global validations across the entire form.

[quote author="CodeIgniter" date="1195087787"]

2) order of execution ...

I don't quite understand what you mean here. I have the sense that you still have some sort of control over the order of execution. To change the implementation of the run() method would mean to change the core of the validation class, in which case we would not be talking about the same validation.
[/quote]

I mean we're not guaranteed that validations assigned to fieldA run before those for fieldB and therefore using a hack like setting outside vars from inside a callback in fieldA is exactly that, a hack.

[eluser]marmida[/eluser]
[quote author="walesmd" date="1195088153"]The only time you get dual errors is if password is left blank (invalid login and password required).[/quote]

This is pretty much what I am looking to make impossible and why I'm suggesting this rule (there is a login that matches these two fields in the db) is (a) not tied to any particular form field and (b) needs to only be evaluated if the rest of the individual field validations clear. I agree with everything you're saying about your suggested solution, I'm just saying it's not as complete as I'd like.