laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

Ignore fillable protection when passing 'Validated' data #2190

Open sileence opened 4 years ago

sileence commented 4 years ago

The problem with mass assigning attributes to Eloquent models since to arise from the following scenario:

new User($request->all())

If we are passing validated attributes, that shouldn't be a problem, or at least it's more explicit:

        $data = $request->validate([
            'name' => ['required'],
            'email' => ['required', 'email'],
            'password' => ['required', 'min:8'],
        ]);

        User::create($data);

But since both the validate and the all methods return an array, it's impossible for Eloquent to know how the data is passed from the "controller".

At the moment most Laravel developers seem to be divided between these 2 options:

  1. Use $fillable to set the attributes that can be mass assigned from the controller: the problem here is that sometimes the fillable attributes are context dependent: we'd like certain attributes to be fillable in one module of the app (i.e. the users profile) but different attributes in another module (i.e. the users module in the admin panel).

  2. Unguard all the attributes and be careful about what we pass to our modules: this is the approach I prefer, but I've found scenarios where one developer in the team forgets about this and leaves a potential vulnerability in the app.

Both have pros and cons.


I propose we implement a "Fillable" interface that can ignore the fillable protection.

An additional advantage of returning a "Validated" object from the validate method instead of a plain array is that we could add extra methods like "only", "except", "transform", to grab or change parts of the validated data, then pass it to multiple models, etc.

I've submitted a quick (and incomplete) proof of concept to the Laravel repository:

https://github.com/laravel/framework/pull/32379

I'd like to know what you think and if you consider this is an idea worth exploring further.

mingalevme commented 4 years ago

if you are sure that the $data is valid:

User::query()->make()->forceFill($data)->save();
sileence commented 4 years ago

In any case (new User)->forceFill($data)->save();

or if I recall correctly User::forceCreate($data)

rs-sliske commented 4 years ago

i think forceFill is a solution here if you want to bypass fillable fields restrictions

hicham-saddek commented 4 years ago

You can always add a new function inside a trait and add it to your models,something like ValidationTrait and would contain:

public function createWithValidation(array $data)
{
      $data = request()->validate($data);
      $this->attributes = array_merge($this->attributes, $data);
}

And then u can use it in you models like this use ValidationTrait;

And inside your controller you can now perform the following

User::createWithValidation($data);

in which $data would contain your validation rules and desired data to be inserted even if they are not required to have validation.

and now even with non validated data you still can be safe as you have your own rules inside the data attribute ... sure this can save you a line of code in your controller, but remember traits if they are used quite often they might result in conflicts so better if laravel ships it with its own Eloquent Model Class.

I hope i answered your question.