yii2-gii icon indicating copy to clipboard operation
yii2-gii copied to clipboard

Model Generate should encourage developers to remove excessive rules

Open krukru opened this issue 7 years ago • 1 comments

When generating a model from a database table using the Gii Model Generator, you get a nice list of rules depending on the data type of the column and so on.

This is very useful - however, it can also be very dangerous if the developer does not know how Model::load() works in junction with Model::rules().

Lets say your model has an attribute banned, or dateCreated or some other field that should never be set from the client-side using $model->load(). Gii of course cannot know this, so it generates a rule for these attributes. After Gii has generated the model and it's rules, the developers now thinks: "Great, I now have validation that dateCreated must be a datetime - just what I need", and leaves the rule - not knowing that such a rule must never be in rules(), at least not in the default scenario.

Later on, the developer calls $model->load(\Yii::$app->request->get()) and by doing so has made a big security error because anyone can inject the attributes dateCreated or banned.

I think that Gii should make this point more clear to the novice developer since the security implications can be critical.

Some of the ideas I think would be ok:

  • Each rule generated in rules() is initially commented out, this in a way forces the developer to check each rules and your actions are uncomment or delete rule
  • Leave a comment at the beginning of rules() to remove (or prefix with !) attributes that should not be settable by $model->load()

krukru avatar May 16 '17 16:05 krukru

Agree. I think a comment would do. No need to change what's actually generated.

samdark avatar May 16 '17 17:05 samdark