Quite often, I see Laravel developers using $request->all()
in Controller methods. It may be a security issue, let me show you why.
Why $request->all() is insecure
Let's imagine you have a regular registration form:
This form is submitted to this Controller method:
1public function store(StoreUserRequest $request) {2 User::create($request->all());3 4 return redirect()->route('dashboard');5}
We use a Form Request class with validation, so it doesn't look harmful, does it? It should save name, email, and password, right?
Notice: I know that the password should be encrypted, but in this article, let's assume the encryption is done somewhere else, like in Observer or Mutator.
Now, let's take a look at the $fillable
array in the User model.
1class User extends Authenticatable2{3 protected $fillable = [4 'name',5 'email',6 'password',7 'is_admin',8 ];
See that is_admin
column? It is used to assign the administrator role, and that field should be filled only by other administrators, in some other form than the registration, in a separate admin panel.
But what if I try to call that registration to submit by adding a hidden
field called is_admin
, directly from my browser, like Chrome dev tools, clicking Inspect?
Guess what: the is_admin
will be successfully saved, and I will successfully register myself as an administrator, without anyone's permission.
So, to "hack" the system, all I would need is to guess the non-visual database fields: it may be called is_admin
, it may be role_id
, just role
, or whatever else. Not that hard to write a script to automate trying all the possible options.
This is happening because $request->all()
doesn't filter or validate anything, it's just literally all()
.
So, what to do instead?
Option 1. Form Request and validated()
If you use the Form Request class for the validation, you have the rules()
method there:
1class StoreUserRequest extends FormRequest 2{ 3 public function rules() 4 { 5 return [ 6 'name' => ['required', 'string', 'max:255'], 7 'email' => ['required', 'email', 'string', 'max:255'], 8 'password' => ['nullable', 'string', 'confirmed', 'min:8'], 9 ];10 }11}
Then, in the Controller, you should use the request filtered after that validation. For that, just replace $request->all()
with $request->validated()
.
1public function store(StoreUserRequest $request) {2 User::create($request->validated());3 4 return redirect()->route('dashboard');5}
So, it will fill in only the fields that are present in the rules()
method.
Keep in mind, that in this case, you need to add all the needed fields into the rules()
array, even if it doesn't require a specific validation, just add them as nullable
or string
.
Option 2. $request->only()
Of course, another option is to specify the exact fields to be used. One of the options is to use $request->only()
:
1public function store(StoreUserRequest $request) {2 User::create($request->only('name', 'email', 'password'));3 4 return redirect()->route('dashboard');5}
Option 3. Field by Field
Finally, the good old way of specifying field by field. The code looks longer, but maybe more readable, with less "hidden" information.
1public function store(StoreUserRequest $request) {2 User::create([3 'name' => $request->name,4 'email' => $request->email,5 'password' => $request->password,6 ]);7 8 return redirect()->route('dashboard');9}
All three options above are valid, it's just your personal preference, which may also depend on the exact form or situation. The main thing is not to use $request->all()
, just forget about that method's existence, for your safety and security.
No comments or questions yet...