[5.3] Allow boot() method on controllers

[tomschlick]

This is related to #14824 where not all the middleware executes before a controller is initiated which can lead to issues with auth, session, etc.

Adding a boot() method allows us to have BaseControllers that interact with "middlewarey" things without having to duplicate code / calls in each controller action.

[axyr]

Given the fact that, even before the release of 5.3 is official, there are raised some issues about using Controller::__construct() method to centralize and/or gather common data for the Controller, I think something like this should be added.

Yes, one could answer : "You should not use a constructor for auth" or "it is never supported, so don't expect it to work in this release", without giving an alternative, but my guess is that these closed issues will come back once 5.3 is official :

#14699
#14825
#14824

https://laracasts.com/discuss/channels/laravel/laravel53-controller-auth-problems
http://stackoverflow.com/search?q=laravel+auth+constructor

I guess a lot of devs use the constructor like that and there is a demand for a central place to have auth/session/etc available in all Controller methods, without the need to check them in every method separately.

So if the __construct() method is not the place to put that, this boot method seems a very viable option to me. Otherwise the documentation should provide a clear example on how to accomplish that.

This 'hack' seems to work fine though :

class MyController extends Controller 
{
    public function callAction($method, $parameters)
    {
        dd(app('auth')->user()); // this works

        return parent::callAction($method, $parameters);
    }
}

but this will probably gets a reply (in a next major release) like : its not supported or you should not do it like that.... ;)

[tomschlick]

Thanks for the feedback there. Validates that I'm not crazy for using this approach for a very large app with many base controllers to handle common variables and data calls.

[morloderex]

If am not mistaken you could inject the request class itself into the controller itself and get the user from there?

And then just store that in a property on a perent class.

[JosephSilber]

The whole point is that the user is not available yet. It's only bound into the request class after authorization runs.

[morloderex]

@JosephSilber was that comment to me?

[tomschlick]

It's not just the user but any other stuff you may need for a group of similar controllers. Being able to extend a parent BaseController is pretty common functionality to make that process easier.

5.3 breaks that ability with anything that uses session, auth, csrf, etc so this is the reasonable alternative in my mind.

[fernandobandeira]

@morloderex On 5.3 you can't access the user inside the construct anymore, also it's not supported to access any middleware inside the construct (since they are instantiated after the controller construct).

So with this method you can get the user and access some middlewares after the controller is constructed and the middlewares were instantiated. 👍

[GrahamCampbell]

You couldn't on 5.2 either. Not reliably.

[tomschlick]

I have been doing it reliably on a large 5.2 app that heavily utilizes extended controllers with tens of thousands of users per day. It's been working since 4.0 without issue until the change I pointed out in #14824 with cached controller objects.

[taylorotwell]

So why did it work on 5.2 but not on 5.3?

[JosephSilber]

It never worked in 5.2 for controller middleware. Only for route middleware. Since 5.3 consolidated both middleware into a single pipeline, you now can't rely on the route middleware either.

[tomschlick]

@taylorotwell the route middleware part worked previously because the controller was being re instantiated once all of the middleware was finished. In 5.3 the controller object is only created once then cached so it's __construct method never has access to data mutated by middleware.

This new proposed boot() method would work for all cases (route mw, controller mw) or anything else needing that base controller extension.

[eldair]

Hello,

Any news on this? Some of my apps also relly on controllers' constructor and this seems like a good solution.

[tomschlick]

Taylor's suggestion via some DM's I have had with him, was to use an event listener for the user to be logged in. He has some concerns about the testability of the boot() method in the long run as it basically sets up two constructors. The consumer has to be aware of calling the boot method before the object is usable.

The event listener idea kinda works but we need another event on the auth class to make that work. Right now the Illuminate\Auth\Events\Login event only fires when a user is logging in via credentials or is given a new session automatically via a remember cookie.

I have resorted to adding a custom event called Authenticated and putting it here via an custom auth guard that extends the default session guard.

Not ideal but it works for now to get me moving again on my 5.3 upgrade.

If that is a better solution than this boot() method, I can create a separate PR for the Illuminate\Auth\Events\Authenticated event.

[eldair]

In that case the user is still not available in controllers' constructors (and also constructors of any typehinted classes in them)?
I tried with existing event when user loggs in which didn't work since, either way, it doesn't fire before controller is construced, or I have it wrong?

[taylorotwell]

This situation can be also be solved by adding a method to your controller that returns the value you were setting as a property before. If necessary, the method can cache the result to a protected property and return the same result each time on subsequent calls.

I want to avoid a boot method because it sets up a two-part constructor pattern which is typically considered an anti-pattern. In other words, when I new up an object, it isn't actually ready for use until I call some other arbitrary method (in this case "boot").

[tomschlick]

Yeah I forgot to mention that I solved most of it via a wrapper method like you're talking about @taylorotwell but there were still some cases where the event approach worked better (setting shared view data, setting the current module, breadcrumbs, etc) so some use that instead.

[axyr]

Can you provider a sample how you solved it? @tomschlick

[JayBizzle]

@tomschlick some example code for the Event solution would be much appreciated 👍

[tomschlick]

You can do so like this. However the authenticated event doesn't exist yet in the core. I have a PR for it with #14946

class BaseController extends Controller
{

    public function __construct()
    {
        Event::listen(Authenticated::class, function ($event) {
            $this->currentUser = $event->user;
            view()->share('currentUser', $this->currentUser);  // simple example
            // other code that may need to bootstrap stuff for this user in the base controller
        });
    }

}

[axyr]

This works for me. thanks.

[borutojisan]

ah...searched for the wrong keyword..no wonder cant find it XD... guess this could work

[sstringer]

@taylorotwell - with the deepest respect, I'd suggest you might want to rethink this if for no other reason than you may have to pave a cowpath, here. Given the number of comments I'm reading from people who are in the same boat as me, this appears to be a major show-stopper to the 5.2->5.3 upgrade for a non-trivial number of Laravel devs.

Intended or not, it was very much possible to call Auth::user() from the base controller in 5.2, and, right or wrong, devs like me built their app around that ability. Since it's completely broken in 5.3 and I have neither the time nor the budget to refactor my production app to account for this, I'm forced to downgrade back to 5.2 and stay there.

In a perfect world, I'd like to see a notional 5.3.1 that allows me to access Auth from the base controller like you can in 5.2.

As a side note, I'd love to learn from you the proper way this (setting app-wide Auth-based parameters) should be accomplished. I've been taught my whole OOP and MVC career two things: 1) avoid re-writing code as much as possible, and 2) logic like permission gates should be done at the controller level, not the view. Given this, putting Auth logic in the baseController class constructor would seem to me to be exactly the correct approach. Am I missing something? I'd love your input.

Many thanks,
Steve

[fernandobandeira]

Some issues related to this change:

Auth user: #14699
Session: #15048

[tomschlick]

If you haven't see it, Taylor added an option to fix this.

https://laravel.com/docs/5.3/upgrade#5.3-session-in-constructors

[gocanto]

I gotta try it tomorrow. I just really would like to know why this is better working this way than before. I really like laravel, and would like as always see a goos explanation about something like this matter. Thanks to you all

[JosephSilber]

It was necessary in order to allow route model binding to have access to the authorized user. Here's a little more information: https://josephsilber.com/posts/2016/07/10/authentication-improvements-in-laravel-5-3#route-model-binding-and-global-scopes

[gocanto]

@JosephSilber I still have an issue tho, and don't know if I am missing something. I do not have access to the logged user from any __construct implementation. For example, I have a a class and in it have this method; but there is not a session in there. Therefore, I can't retrieve the logged user.

My big question, why is it so important? I aware of the many explanations there are out there about this matter, but nobody has clarified the why of it. Also, I read what taylor posted in the docs, and still do not figure it out.

This is something that seems to be simple, I could access to the logged user from any __construct.

thanks in advance to you all.

[testingtoolsco]

Has anyone got solution for this? I am using Laravel 5.4 and still have the same issue, I tried to implement the ways suggested in some of the links referred in the comments, but it still doesn't work.... any help on this is greatly appreciated, as I am completely stuck and cannot go ahead on one of the app I am building :( this is sheer waste of time, I was loving laravel until now.. just this thing is making me think did I consider it right or not.

[JosephSilber]

@testingtoolsco I've written up a few strategies for dealing with this here: https://josephsilber.com/posts/2017/01/23/getting-current-user-in-laravel-controller-constructor

[w3spi5]

@JosephSilber Solution #4: Inline middleware works for me, thank you !