Implicit route model binding in validation methods

[lorisleiva]

Thanks for the super quick reply! I think that solves my issue, but there might be a secondary one.

In every case I've tested in my application, the $event parameter passed to the authorize function is always null. If I check the parameter in the handle function, it is correctly present. Just not in the authorize function!

Full code sample is below, and I am truly puzzled 🙂

class ShowEvent
{
    use AsAction;

    public function authorize(ActionRequest $request, Event $event): bool
    {
        dd($event->id);
        return Gate::check('view', $event);
    }

    public function handle(ActionRequest $request, Event $event): Response
    {
        return Inertia::render('Events/Show', [
            'event' => $event,
        ]);
    }

    public function asController(ActionRequest $request, Event $event): Response
    {
        return $this->handle($request, $event);
    }
}

In this case, dd just shows null.

Originally posted by @Celant in https://github.com/lorisleiva/laravel-actions/issues/137#issuecomment-939339628

---

@lorisleiva I suspect the issue here is that inspectAuthorization() in the ValidateActions concern uses $this->resolveAndCallMethod('authorize'), whereas the ControllerDecorator uses $this->resolveFromRouteAndCall().

The former uses the service container to resolve dependencies and then call the method, whereas the latter does the same but also includes $this->route->parametersWithoutNulls(), meaning handle gets the Event object from the route correctly but authorize doesn't.

I don't know enough about Laravel and this library to know whether it's as simple as just including the parametersWithoutNulls in the arguments passed to resolveAndCallMethod, or if some wider change is needed to support this.

If it is the former, I am happy to try and PR that. Otherwise, I'll need your help on this one. Additionally, It looks like the tests cover controllers with implicit route bindings, as well as authorization and validation, but not both. Maybe this is an edge case that requires a test case? 🙂

[lorisleiva]

Hey 👋 Sorry about the late reply.

Yeah I am aware of this limitation and Laravel Actions currently does not support implicit route model binding in the validation methods such as authorize and rules — hence the lack of test for it. I do realise now that in my previous example, I gave the wrong impression that this would work so sorry about that haha.

Currently, instead of using dependency injection to retrieve your event, you'd need to access it via the Request using the route method like so:

public function authorize(ActionRequest $request): bool
{
    return Gate::check('view', $request->route('event'));
}

Note that for the return value of that method to be an instance of App\Event, it needs to be dependency injected into the handle or asController method. Otherwise, you'll end up with the identifier of the event. Note that regular controllers and form requests behave exactly the same.

Now, I do realise this is not as elegant and intuitive as a direct dependency injection, and it even got me a few times. There are a few reasons that lead me to make that decision:

• In regular controllers, implicit model binding happen only once per request making it easier to understand and debug. When using form requests, the authorize and rules method will not resolve it again for you.
• If we had multiple resolutions of implicit model binding and each injected a different model, these models will be resolved in a certain order which could confuse users.
• I want to make sure Laravel Actions delegates to the same native implicit route model binding to be less intrusive and ensure that this feature is available if and only if they use the SubstituteBindings middleware.

Having said that, I haven't given up on this feature and if you can find a way to go around them, I would happily accept a PR for it. 😊

---

P.S.: I've taken the liberty to extract this in a new issue so it's more searchable for other users.

[Celant]

No worries!

Honestly, that's what I've been using as a workaround, but using $request->route()->parameter('event'); instead of the route method directly, but same-same really.

I think the reasoning for it not doing implicit binding makes total sense, and unfortunately I'm not proficient enough with Laravel to offer an alternative solution! I suspect at the very least, it may be worth some mention in the docs even just as a "gotcha" point, to help anyone else who might run into this as I have. Especially as the docs at a few points do have models passed into authorize via dependency injection, which is what threw me off!

[lorisleiva]

Oh thanks for mentioning it, I hadn't noticed! I'll fix the documentation.

I wouldn't want to just discard this feature though because I do believe it would improve the dev experience. I think a good middle ground would be to simply inject more data when resolving the validation methods from the container but there are two little issues with that:

• A. It won't work if the model you're injecting into the authorize method was not injected in the handle or asController method which can create confusion and force unused variables to be present in the main method.
• B. The logic will have to be different when using the WithAttributes traits because validation at this point could either come from a request or a set of attributes.

I'll just leave this issue open anyway as there might be a better solution out there and this issue is not closed in my mind. 😊

[vpratfr]

Hi,

We just had the same problem, and did not see any mention of that in the docs. So I'm glad I found this issue :)

Just an idea, I understand that you would not want to resolve the parameters twice, and that is a valid reason.

How about caching the result of what is resolved somewhere accessible by both the ControllerDecorator and the ValidateActions classes ?

Something like a ParameterResolutionCache, where you ask for an array of parameter values, and examine each of them :

• if already resolved, get value from the cache
• if not, resolve it using Laravel's default mechanism

I still haven't dove into your code to check if that would make sense. Do you have any idea if that would work?

[Floppy012]

Just got to this Issue after I tried something similar as shown in the docs: https://laravelactions.com/2.x/add-validation-to-controllers.html#authorization

If I understood, then that example from the docs will not work. Or am I doing something wrong?

[vpratfr]

Yes, in your case, $article will not be bound properly (null).

Instead you will have to do: $article = $request->route('article')

[EsdrasCaleb]

I m here because the same example we need update it to work or update the article example

[dnnp2011]

Also here because the docs led me astray ⬆️

[Wulfheart]

Closing due to inactivity. If you feel your issue is still relevant please open a new one with a link to a repository containing a minimal reproducible example.

[vpratfr]

afaik the bug still lives. No binding resolution occurs for rules and authorize methods (and other ones too).

See also #263

[Wulfheart]

@vpratfr Please provide a link to a repository containing a minimal reproducible example.

[vpratfr]

That sample code above (https://github.com/lorisleiva/laravel-actions/issues/140#issuecomment-1450973641) is rather clear. I don't think a whole repository would be required.

You just need to define an action with an authorize method accepting the model it is supposed to get from the route.

class MyAction {
  use AsController;

  public function authorize(Article $article) {
    // $article is not injected here
  }

  public function asController(Article $article) {
    // $article is properly injected here
  }
}

That is assuming a model class Article and a route bound to your action such as :

Route::get("/articles/{article}")->uses(MyAction::class);

That reproduces that particular issue for authorization. A quick look at the code reveals that only the main methods get injected (asController, asListener, asJob, etc.) and not the secondary methods that deal with side-features of the action aspect (authorization, validation, etc.).

[Wulfheart]

That sample code above (https://github.com/lorisleiva/laravel-actions/issues/140#issuecomment-1450973641) is rather clear. I don't think a whole repository would be required.

I am just lazy tbh and just want to clone something, execute some docker command and start.

[EsdrasCaleb]

Steps, 1- clone the base laravel project 2- create a controller to logged users 3- try to get any attribute from user object in the authorize method

[Wulfheart]

@EsdrasCaleb I know the steps but don't want to do these basic steps. It is more a philosophical question.