I've been doing a lot of brainstorming on Yii validation and security…
First, there is a the safeAttributes() security layer. This makes sure that fields are not saved that shouldn't be saved. One way or another you define an array of fields that are safe (either in the model or as an argument of safeAttributes() or both). I usually define the fields in the argument instead of via the model as I found the safe attributes vary in scenario to scenario. For instance, I may have user creation page, and for admins, the isAdmin
attribute is safe, but for regular users accessing the page, isAdmin
is not safe.
The second security layer is the first argument of CModel::validate($attributes=null,$on=null)
This argument defines which attributes are validated (in most cases that means it defines which attributes are required which was the original reason for the implantation)
I found that in 95% cases you will want to pass the same value to safeAttributes() as validates(), as all fields that are required are obviously safe, and often all fields that are safe a required.
So I found myself doing a lot of this:
<?php
$fields = array('username', 'password', 'password_repeat', 'email');
$user->setAttributes($_POST['User'], $fields);
if ($user->validate($fields) && $user->save(false))
$this->redirect(...);
That's kind of redundant… I found another solution to defining required fields via validate()'s second argument. For instance:
<?php
///Model
public function rules() {
return array(
///...
array('username, password, password_repeat, email', 'required', 'on' => 'register'),
array('username, password', 'required', 'on' => 'login'),
);
}
//controller action
$fields = array('username', 'password', 'password_repeat', 'email');
$user->setAttributes($_POST['User'], $fields);
if ($user->validate(null, 'register') && $user->save(false)) //note the second argument of validate()
$this->redirect(...);
This way it knows the required fields through the 'on' parameter, but you still have to pass safe fields to setAttributes(), so that's even more redundant because you have to type out the attributes twice. But I liked the idea of the required fields being defined in the model instead of the controller with scenarios. It seems more correct as the controller should not really do that kind of thing. I thought it might be nice if you could also define the safe attributes in the model with scenarios in the rules() member function. Here's a set-up I thought up that I think would be nice in Yii:
<?php
//model rules
public function rules() {
return array(
///...
array('username, password, password_repeat, email', 'required', 'on' => 'register'),
array('about_me', 'safe', 'on' => 'register'),
);
}
//controller action
$user->setAttributes($_POST['User']);
if ($user->validate('register') && $user->save(false)) //note validate()'s first argument was removed, more info below
$this->redirect(...);
With this setup Yii would correctly know which attributes are required because of the scenario use (Yii can already do this). What I am introducing is the 'safe' validation rule. I would expect Yii to automatically know that all the fields defined in that rule are safe to assign massively through setAttributes(), and also the fields defined in the 'required' rule are safe (because if a attribute is required, it must be safe, right?). Going further, Yii could even realize that if any rules are defined for a field inside of the current scenario, it must be safe. So in the above case the 'about_me' attribute would be safe but not required, and 'username, password, password_repeat, email' are safe and required. It would be synonymous to doing the following right now:
<?php
$fields = array('username', 'password', 'password_repeat', 'email');
$user->setAttributes($_POST['User'], array_merge($fields, array('about_me')));
if ($user->validate($fields) && $user->save(false))
$this->redirect(...);
Only the new way is cleaner, and you don't have to define the fields in the controller, but instead in the model. Also it is not redundant as Yii would automatically know that 'required' fields are 'safe'. With the new way, if when I was developing a application and I decided to add a new field to the registration process, I would have to modify the model and the view, instead of the controller and the view, which makes more sense for that type of modification.
Tell me what you think, and if that makes sense…