uniqueValidator breaking login

First off, I’d like to say this framework is the best thing since php5.  I used to use cakephp but am now changing to yii, which I find waay better for my needs :).


Anyways,

I came across this problem and I'm inquiring what the most optimal solution would be.

I have a basic database user login/logout/registration system.

I naturally added the uniqueValidator to the username field so that new users can only create accounts with unique usernames.  Worked beautifully.

But now, the login action is broken.  Why?  Because the username that you would try to login with is obviously in the database already.

My first reaction was to try the ‘on’=>‘insert’ rule in the validation rules.  But then I realized that as far as the login action can tell, the login action does use an insert quarry (the login form does not supply an pk ID, so it thinks it’s an insert).

What would be the best way to go about this problem?

<?php


//My ruled as defined in the user model:


	public function rules()


	{


		return array(


			array('username, password', 'length', 'max'=>50),


			array('username', 'unique'),


			array('password', 'authenticatePass'),


			array('password_repeat', 'compare'),


			array('email', 'length', 'max'=>100),


			array('email', 'email'),


			array('username, password, group_id, email, password_repeat', 'required'),


			array('group_id', 'numerical', 'integerOnly'=>true),


		);


	}





//the login action


	public function actionLogin() {


		$user = new User;


		if (Yii::app()->request->isPostRequest) {


			// collect user input data


			if (isset($_POST['User']))


				$user->setAttributes($_POST['User']);


			// validate user input and redirect to previous page if valid


			if ($user->validate(array('username', 'password', 'rememberMe')))// ;


				$this->redirect(Yii::app()->user->returnUrl);


		}


		// display the login form


		$this->render('login',array('user'=>$user));


	}





?>

Feel free to criticize any of the above code btw, and in fact please do if you can.

Thanks and keep up the good work,

Jonah

The key here is to determine when to apply the unique validator. A quick workaround is to give a fake PK to the user instance (you don't use it anyway). You could also write an inline validator method to perform the unique check, but you still need to think about a way to identity that the model is being used for login purpose.

Because of this complexity, the yiic webapp is using a separate LoginForm model to collect login input, which I think is reasonable because attributes like "rememberMe" doesn't really belong to a User AR model.

Hmm… I just realized that the registration form is broken too because of the

array(‘password’, ‘authenticatePass’),
rule, as it tries to authenticate the username and password to the database on registration.  Seems the best solution would be to have a separate model for the login form.  But somehow that seems wrong to me, and as I go on I guess i’ll find out if it hinders anything.

What would be another good solution is if Yii had the ability to define custom 'on' options for the validation rules.

A proposal:

The 'insert' and 'update' options for 'on' could be changed to constants, eg:

array('password', , 'length', 'max'=>100, 'on'=>CValidator::INSERT)

But if you set the ‘on’ option to a string, it will be a custom rule set that can be invoked through an extra argument in CActiveRecord.validate()

For instance:



<?php


//user model


public function rules()


{


	return array(


		array('field_A', 'validatorName', 'on'=>CValidator::INSERT),


		array('field_B', 'validatorName', 'on'=>'login'),


		array('field_C', 'validatorName', 'on'=>'registration'),


		array('field_D', 'validatorName'),


		array('field_E', 'validatorName', 'on'=>array('x, z')),


		array('field_F', 'validatorName', 'notOn'=>'foo'), //note 'notOn' instead of 'on'


	);


}





//validate call: (assuming an UPDATE)


$user->validate(); //uses rule D and F


$user->validate(NULL, 'login'); //uses rule B, D and F


$user->validate(NULL, 'z'); //uses rule D, E and F


$user->validate(NULL, 'foo'); //uses rule D


Also maybe there could be a CValidator::FIND constant too? In AR there is not just 'inserts' and 'updates', but there are also 'selects'/'finds', as is a login form.

Or, instead of the invoking it with an argument in CActiveRecord.validate(), it could instead refer to a controller/action pair. eg:

array('field_C', 'validatorName', 'on'=>'controllerID.actionName'),

Just some brainstorming there, maybe it should be implanted into yii?

This sounds like a good idea. Will use your first approach. Thanks.

Great! I think it will be a great addition to Yii.  For now I guess I'll use two models however.

Now I'm running my app off the trunk but it does not seem to be working… not sure why but I think it's a bug in Yii



<?php


//rules


public function rules() {


	return array(


...


		array('username', 'unique', 'on' => 'register'),


...)


}





//controller action


$user->validate(array('username', 'password', 'rememberMe'), 'login')


Thanks!

Yet the unique rule is applied anyways.

I can't figure it out but I think the problem is in CModel::validate line 48:

<?php


				if(empty($on) || empty($validator->on) || is_array($validator->on) && in_array($on,$validator->on))


					$validator->validate($this,$attributes);

Also, perhaps instead of an array the scenarios could be defined with a comma separated list? That would mean less typing.

After some further examination i found that it is the empty($validator->on) that screws up.  For some reason the array array([0] => 'register') counts as empty.  Not sure why.  But replacing it with !count($validator->on) seemed to work.  So it looked like this after the fix:

<?php


            if(empty($on) || !count($validator->on) || is_array($validator->on) && in_array($on,$validator->on))


               $validator->validate($this,$attributes);

Although calling count() might not be the fastest solution…

Also the $on argument should be added to save() not just validate()

Thanks. Yes, it is a bug in checking the on property.

The 'on' property does allow you to enter a list of scenarios as a string.

I don't think save() method needs 'on' parameter because 'save' is only used for two scenarios: 'insert' and 'update'. If you need a different scenario for saving (very rare), you can call validate() followed by an insert() or update() call.

The reason I wanted the ‘on’ parameter on save() was for my registration form.  So my unique validation (checks if username is taken) does not run on the login form.  But I suppose I can call validate() and then call save(false) with validation turned off.

yeah, I understand your need. The reason that I am hesitant to add an 'on' parameter is because it would further complicate the signature of save(). Anyway, save() is a just wrapper of validate and insert/update.

I see your point. Another option would be to add a property to CActiveRecord that defines the scenario.

eg.

<?php


$user->scenario = 'register';


if ($user->save(true, $fields))


	$this->redirect(array('show'));


Just a thought.  It's not critical.