Any reason not to use FormRequest for GET/DELETE requests?

[kohenkatz]

I have an application where I have to deal with some sloppy developers. We can't get rid of them, so I've been tasked with making the development process as idiot-proof as possible.

One problem these developers have is that they often forget to do input validation and user authorization. In the past, this has resulted in major problems with methods that allow junk input (e.g. invalid email addresses) and/or allow users to do things they do not actually have permission to do.

In order to make it harder for them to make these mistakes, I suggested that we use FormRequest as much as possible because that way all requests have automatic authorization and validation. We plan to use this structure:

app/
  Http/
    Api/
      Organizations/
        Controller.php
        CreateRequest.php
        DestroyRequest.php
        IndexRequest.php
        ShowRequest.php
        UpdateRequest.php
      ... // same for other resource controllers

It feels a little weird to do this for GET requests (i.e. index and show) and DELETE requests because those requests have no body. Validation can still be useful for these requests (query parameters for GET to include relations and for whether DELETE should be hard or soft), but it feels like there should be a nicer way to do it.

Also, when using model binding for a resource that doesn't have query parameters, the $request parameter and model parameter both need to be defined on the show method, but only one of them will be used. For example:

Organizations Controller

This uses both the injected model and the request:

public function show(Organization $organization, ShowRequest $request)
{
    if (Arr::has($request->input('include'), 'addresses')) {
        $organization->load('addresses');
    }

    // ...
}

Addresses Controller

This uses only the injected model, so IDEs will mark the $request as unused and prompt the developer to remove it:

public function show(Address $address, ShowRequest $request)

If developers follow that recommendation from the IDE, it will cause the authorization to be bypassed, which is what I'm trying to prevent.

---

I supposed I could say to use policies for authorization instead of FormRequests and use the $this->authorizeResource(...) method in the controller constructor, but that has two issues:

1. Some controllers have additional methods besides the regular resource routes. These are very easy to authorize using FormRequest injection, but they are not included when calling authorizeResource so they need to have manually-added validation.
2. I still want to use FormRequest for validation of POST/PUT/PATCH requests, and it will be confusing for the developers to have just return true in the authorize method of the FormRequest.

---

I know the best option would be "either educate the developers better or get rid of them and get good ones," but that is not currently a practical solution (and I'm not in charge of hiring anyway).

Given these constraints, is my original approach (using FormRequest everywhere) my best option? Or is there something else I should be doing?

[henzeb]

You could instruct your developers to extend their FormRequests your OrganisationFormRequest and implement/override the methods they need.

If you test your routes to always use a controller that extends OrganisationController, the ide will warn the developer if hey are not calling the parent constructor, and the tests will fail and block other ci steps if a controller is not using the OrganisationController.

[sethsandaru]

index & show don't have body, thus Laravel will validate the GET params and it is a good approach as well.

You don't want to see users hijacking your index by passing ?limit=9999999, do you?

Thus you will have the rules like:

public function rules(): void
{
    return [
        'limit' => 'nullable|integer|min:1|max:20',
        'keyword' => 'nullable|string',
    ];
}

Additionally, within the FormRequest's scope, you can create multiple getter methods to get the related Models of the Request, eg:

public function getOption(): ?Option
{
    return Option::find($this->input('option_id'));
}

So from your controller, simply hit the method to get the Model with the exact type

public function index(PostIndexRequest $request): JsonResponse
{
    $option = $request->getOption(); // Option type here, use it, IDE loves it.
    // ...
}

[henzeb]

He knows, but he wants to force the usage of FormRequest for authorization purposes even when there is no GET parameter the index or show is being used.