Framework: [5.0] route model binding runing before middleware

[alioygur]

Route model binding runing before middleware, this is bug.

[GrahamCampbell]

Pull request to fix?

[alioygur]

@GrahamCampbell, i have no idea that how is it gonna be resolved.

[alioygur]

@taylorotwell this is critical bug. please fix it.

[GrahamCampbell]

@alioygur This is NOT critical. Any bug that affects laravel 5 is low priority.

DO NOT USE LARAVEL 5 IN PRODUCTION

[taylorotwell]

Why would this be a bug?

[alioygur]

@Taylorotwell, I thinking this is bug. Because,

Imagine you use http basic authentication. (Stateless)

So, user authenticated at middleware (AuthenticatedWithBasic)

And you have a route and model binding, like this

// routes.php

$router->group(["prefix" => "me", "middleware" => "App\Http\Middleware\AuthenticatedWithBasic"], function($router) {

$router->resource("posts", "App\Http\Controllers\PostsController");

});

// routeserviceprovider@before

$router->bind("posts", function($post_id) use ($router){
return $router->user()->posts()->findOrFail($post_id);
});

So, I maybe do somethings with current logged in user order model binding. Currently I can not this because model binding running before middlewares.

I am sorry, for my bad English .
I am on mobile.

[taylorotwell]

I still don't understand.

On Tuesday, October 21, 2014, ali oygur [email protected] wrote:

@Taylorotwell https://github.com/Taylorotwell, I thinking this is bug.
Because,

Imagine you use http basic authentication. (Stateless)

So, user authenticated at middleware (AuthenticatedWithBasic)

And you have a route and model binding, like this

// routes.php

$router->group(["prefix" => "me", "middleware" => "App\Http\Middleware\AuthenticatedWithBasic"], function($router) {

$router->resource("posts", "App\Http\Controllers\PostsController");

});

// routeserviceprovider@before

$router->bind("posts", function($post_id) use ($router){
return $router->user()->posts()->findOrFail($post_id);
});

So, I maybe do somethings with current logged in user order model binding.
Currently I can not this because model binding running before middlewares.

I am sorry, for my bad English .
I am on mobile.

—
Reply to this email directly or view it on GitHub
https://github.com/laravel/framework/issues/6118#issuecomment-60001449.

[alioygur]

okey, just one question

SO, what I supposed to do if I try to get the current logged in user information while binding model.
Note: I am using http basic authentication.

/**
 * Called before routes are registered.
 *
 * Register any model bindings or pattern based filters.
 *
 * @param  Router  $router
 * @return void
 */
public function before(Router $router)
{
    $router->bind('resumes', function ($value, $route) {
        $user = app('auth')->user();
        $model = $user->resumes()->findOrFail($value);

    });
}

I am getting following error.

FatalErrorException in RouteServiceProvider.php line 43:
Error: Call to a member function resumes() on a non-object

because AuthenticatedWithBasic middleware did not run before model binding.

this is AuthenticatedWithBasicAuth file

<?php namespace App\Http\Middleware;

use Closure;
use Illuminate\Contracts\Auth\Guard;
use Illuminate\Contracts\Routing\Middleware;

class AuthenticatedWithBasicAuth implements Middleware {

    /**
     * The Guard implementation.
     *
     * @var Guard
     */
    protected $auth;

    /**
     * Create a new filter instance.
     *
     * @param  Guard  $auth
     * @return void
     */
    public function __construct(Guard $auth)
    {
        $this->auth = $auth;
    }

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @return mixed
     */
    public function handle($request, Closure $next)
    {
        return $this->auth->onceBasic() ?: $next($request);
    }

}

[JoostK]

So basically, in order to find out if the bound object belongs to the authenticated user, you will need access to the current user in the bind callback.

[alioygur]

@JoostK yep! you are right.

[robinmitra]

I'm in the same boat as @alioygur. Any possible solution to this issue yet? Otherwise, I'm afraid I would have to get rid of all my route model bindings sadly :(

[JoostK]

This shouldn't have been closed, delaying model binding until after the session data is available makes kind of sense.

[MrAtiebatie]

I came across the same issue. Imagine you have a admin panel where you can manage the users that can login into your website. As admin you want to be able to see users by browsing to admin/users/5 for example. Of course you only want admin's to be able to see that page so you put a middleware in your controller to redirect guests to the login page. In case that user 5 doesn't exist you see a nice 404 not found which is fine to me. BUT, when the guest browses to the admin/users/5 page, he doesn't get redirected to the login page but also sees the 404 not found page which is not fine with me. I hope you can fix this.

[taylorotwell]

None of that helps me without seeing actual code.

On Thu, Jan 15, 2015 at 4:28 AM, MrAtiebatie [email protected]
wrote:

I came across the same issue. Imagine you have a admin panel where you can
manage the users that can login into your website. As admin you want to be
able to see users by browsing to admin/users/5 for example. Of course you
only want admin's to be able to see that page so you put a middleware in
your controller to redirect guests to the login page. In case that user 5
doesn't exist you see a nice 404 not found which is fine to me. BUT, when
the guest browses to the admin/users/5 page, he doesn't get redirected to
the login page but also sees the 404 not found page which is not fine with
me. I hope you can fix this.

—
Reply to this email directly or view it on GitHub
https://github.com/laravel/framework/issues/6118#issuecomment-70066489.

[MrAtiebatie]

My code or framework code?

[taylorotwell]

Your code. Show me where the problems are actually happening.

On Thu, Jan 15, 2015 at 9:02 AM, MrAtiebatie [email protected]
wrote:

My code or framework code?

—
Reply to this email directly or view it on GitHub
https://github.com/laravel/framework/issues/6118#issuecomment-70097997.

[MrAtiebatie]

This is my (resource) controller constructor:

/**
 * Constructor
 * @param User $user
 */
function __construct(User $user)
{
    $this->user = $user;

    $this->middleware('guest');
}

This is my guest middleware:

/**
 * Handle an incoming request.
 *
 * @param  \Illuminate\Http\Request  $request
 * @param  \Closure  $next
 * @return mixed
 */
public function handle($request, Closure $next)
{
    if ($this->auth->guest())
    {
        return redirect('auth/login');
    }

    return $next($request);
}

And in routes.php:

 Route::resource('admin/users', 'Admin\UsersController');

I have tried the beforeFilter in the constructor and a group filter in routes.php but both still gave a 404 not found, browsing as guest.

[robinmitra]

Just to chime in, I was roughly trying to do something like the following, but it fails due to this issue:

Route model (or repository in my case) bindings in AppServiceProvider::register() were something like:

public function register()
{
    $repoAbstract = 'App\Contracts\Repositories\Order';
    $router->bind('order', function ($value) use ($repoAbstract) {
        $repo = App::make($repoAbstract);
        if ($model = $repo->find($value)) {
            return $repo;
        }

        throw new NotFoundHttpException('Repository ' . $repoAbstract . ' could not find the model!');
    });
}

And this is Order repository's find() method:

public function find($id)
{
    $model = $this->model
        ->where('id', $id)
        ->where('user_id', Auth::id())
        ->first();

    if ($model) {
        $this->setModel($model);
    }

    return $model;
}

The find() method in the repository expects the user to be authenticated, in order to only return orders that belong to that user, which is pretty standard stuff.

However, using route model (or repository in my case) bindings will fail for the above scenario, since the user hasn't yet been authenticated, which will cause the repository's find() method to fail.

As it stands right now, although I think route model bindings are pretty cool, I've had to disable all of them sadly, since they won't work for my scenario which is quite common frankly.

[landons]

I just ran into a similar issue, but it was pretty easy to get around with dependency injection:

$router->bind('project', function($id)
{
    App::bind('App\Contracts\Project', function() use($id)
    {
        if (($project = Project::findByClient(Auth::user(), $id))) {
            return $project;
        }

        throw new NotFoundHttpException;
    });

    return $id;
});

[GrahamCampbell]

This is not a bug, but a feature. I was talking to Taylor about this earlier today, not in relation to this though.

[vtalbot]

Could there be a third arguments to bind that defer the binding after the middlewares or an arguments to tell the binding to be done after a middleware or an array of middlewares?

[jordandobrev]

There is an easy fix for those who are stuck with this issue. Instead of using the bind method in the service provider, just replace the route parameter with the object using middleware.

public function handle($request, Closure $next)
{
    // some middleware logic ....
    if ($request->route()->hasParameter('user')) {
        $user = $this->app->make(UserRepository::class)->getByIdOrFail($request->route()->parameter('user'));
        $request->route()->setParameter('user', $user);
    }

    return $next($request);
}

Cheers ;)

[zot24]

Thanks @LPMadness good example!

[malhal]

Binding has always been less than ideal because it occurred before filters in 4, and now occurs before middleware in 5. For un-authorized requests it needlessly hits the database to find a record, only to fail authorisation afterwards, a complete drain on resources. There is no doubt model binding should be done as the very last thing in the chain of events. This might be one of those times where the design that best fits peoples needs doesn't align well with the framework's original technical design, personally I hate when that happens ;-)

[ont]

Just another possible solution. Declare middleware as global in app/Http/Kernel.php:

class Kernel extends HttpKernel {

    /**
     * The application's global HTTP middleware stack.
     *
     * @var array
     */
    protected $middleware = [
        'Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode',
        'Illuminate\Cookie\Middleware\EncryptCookies',
        'Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse',
        'Illuminate\Session\Middleware\StartSession',
        'Illuminate\View\Middleware\ShareErrorsFromSession',
        'App\Http\Middleware\VerifyCsrfToken',
        'App\Http\Middleware\YourAwesomeAuthMiddleware',   /// <<<< HERE
    ];
....

By some reason global middlewares are called before bindings.

[slaughter550]

@taylorotwell @GrahamCampbell I am not sure if this helps, but this is our use case which would currently benefit from making route bindings resolve after the request part of the middleware stack has executed.

We currently have several types of bindings in prod. Two of them look like this.

// This example pulls from the User class directly
Route::bind('userResourceKey', function ($value, $route) {
    return bind_obj('User', 'resource_key', $value);
});

and 

// This example pulls from the posts function on the User model
Route::bind('postResourceKey', function ($value, $route) {
    return bind_obj('posts', 'resource_key', $value, auth()->user());
});

the abbreviated version of the bind_obj function looks like this. Please keep in mind this was written in shorthand for the purposes of conveying a point.

function bind_obj($resource, $key, $value, $scope = null, $options = [])
{
    $obj = null;
    if ($scope) {
        $obj = $scope->$resource()->where($key, $value);
    } elseif (($resource = "App\Models\\$resource") && class_exists($resource)) {
        $obj = $resource::where($key, $value);
    }

    // more code to determine permissions and whether this is a protected path

    if ($obj && $obj = $obj->first()) {
        return $obj;
    } else {
        throw new NotFoundHttpException;
    }
}

The bind_obj function is responsible for resolving the issue of whether the user is already logged in once passed auth, for determining permissions. However, because our Laravel stack runs a restful api, we encounter this specific issue when the user is not logged in.

The request on an existing resource like https://api.example.com/users/{userResourceKey} returns a 403 as expected
The request on an also existing resource like https://api.example.com/posts/{postResourceKey} returns a 404, because the dependency on auth()->user() is null. This behavior is not desirable. It is desirable for the response code to be a 403.

Although the above problem is solvable with more logic, it gets trickier for different types of routes and whether they are protected and what their dependencies are. This in turn, leads to a lot of messy and duplicate logic all to determine whether a bind should actually run or not.

The next issue, and the biggest for us currently is for objects that are constructed like userResourceKey. It is not desirable in production for a user that is not authenticated to be able to make a http request which forces a DB action even though it is not required for that request.

I would expect them to hit the auth middleware first and have Laravel throw a 403 before going any further.

Hope this helps. Happy to answer any questions.

[simondubois]

Here is my contribution (Laravel 5.1.15).
I am trying to bind model to route parameters. For security reasons, only models related to the authenticated user should be bound.

<?php namespace App\Providers;

use Auth;
use Illuminate\Routing\Router;
use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider;

class RouteServiceProvider extends ServiceProvider
{
    public function boot(Router $router)
    {
        $router->bind('account', function($id) {
            if (Auth::check() === false) {
                return null;
            }

            return Auth::user()->accounts()->find($id);
        });

As nor session neither authentication are already loaded, Laravel seems to generate new session id on each request.

[slaughter550]

If it helps anyone, we ended up using the controller middleware, and doing this.

abstract class Controller extends BaseController
{
    use DispatchesCommands, ValidatesRequests;

    public function __construct()
    {
        $this->middleware('bind');
    }
}

where the middleware "bind" looks like this.

namespace App\Http\Middleware;

use Closure;
use App\Configs\Domain;

class BindRequests
{
    protected $binders = [];

    public function __construct()
    {
        $this->binders = Domain::getBindables();
    }

    public function handle($request, Closure $next)
    {
        foreach ($request->route()->parameters() as $key => $value) {
            if (isset($this->binders[$key])) {
                $boundObject = $this->performBinding($key, $value, $request->route());
                $request->route()->setParameter($key, $boundObject);
            }
        }

        return $next($request);
    }

    private function performBinding($key, $value, $route)
    {
        return call_user_func_array('routeBinder', $this->binders[$key] );
    }
}

[simondubois]

@slaughter550 Thanks for sharing, but $request->route() is null (Laravel 5.1).
Anyway, I remain with implicit controller routes and bind input parameter inside controller.
I just feel parameter binding is quiet useless for application with access control...

[ryanmortier]

I ran into this issue today as well. The Authenticate middleware should definitely be running before route model binding otherwise users do not get redirected to the signin page when hitting a URL that uses route model binding.

I don't believe this to be a bug in the way middleware works, just the way auth is implemented.

Here is how I worked around the issue, although it isn't the most elegant solution. I hate to modify auth in case I introduce a hole but I definitely needed auth to run before route model binding and I didn't want to stop using route model binding.

\App\Http\Middleware\Authenticate

    public function handle($request, Closure $next)
    {
        if ($this->auth->guest() && ! $request->is('signin')
                                 && ! $request->is('password/email')
                                 && ! $request->is('password/reset')
                                 && ! $request->is('password/reset/*'))
        {
            if ($request->ajax())
            {
                return response('Unauthorized.', 401);
            }
            else
            {
                return redirect()->guest('signin');
            }
        }

        return $next($request);
    }

App\Http\Kernel

    protected $middleware = [
        \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
        \App\Http\Middleware\EncryptCookies::class,
        \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
        \Illuminate\Session\Middleware\StartSession::class,
        \Illuminate\View\Middleware\ShareErrorsFromSession::class,
        \App\Http\Middleware\VerifyCsrfToken::class,
        \App\Http\Middleware\Authenticate::class,
    ];

Edit: Yeah, I didn't feel comfortable with this workaround so just ignore it. I guess I'll just stop using route-model-binding.

[andreygrehov]

@taylorotwell this is a bug. Once again: For un-authorized requests it needlessly hits the database to find a record, only to fail authorisation afterwards, a complete drain on resources. I can have sleep(100) in the resolution logic, which should not be even reached if I'm not authorized.

[JosephSilber]

I completely disagree that this is a bug. I believe it makes perfect sense for route model binding to be resolved before middleware; your middleware may want to access said model.

When using route model bindings, the idea is to never fiddle with the actual raw data in the route; it's as if the model is part of the actual route.

If you have to compare a user to a model, that should be done with authorization or some other way in the controller. This is not a bug, and definitely not a security issue.

For a real-world example of when it is actually very useful to resolve the model before the middleware, see the popular authorize middleware.

[andreygrehov]

@JosephSilber route model binding is a framework's feature, whereas authorization is, sort of speak, a higher level. The fact that authorize middleware is popular doesn't guarantee its architectural correctness. Now let's imaging a case - your database is very large, huge - millions of rows. You only have 2 clients, who are allowed to access this database. Because: a) operational costs are very high, b) you don't need more clients. The route model binding in this sample case is using a few parameters to find the required row/model (not just a simple primary key - and that's how you decided to model your database). Such query takes a good amount of time, because of the size of the database and lack of a simple way to access data by PK.

Now I am a bad guy who wants to kill your business, write a simple script that hits your API endpoint a thousand times a second. I don't have access to API, except the full url. In this case, model binding will execute the code to run thousand queries per second, making the CPU go to 100% of its use. Memory usage will jump to the top and the whole server will, presumably, go down. Now what will happen next?

1. You will receive a very nice bill from your hosting provider.
2. The only two of your clients are not able to do business with you anymore.

Maybe this is not a bug and I was wrong calling it so, but this is quite a big flaw in the architecture.

P.S. Keep in mind, I'm not talking about accessing a user object here.

[JosephSilber]

If you are really concerned about that, you should be rate-limiting your API at a layer prior to your actual application.

[andreygrehov]

@JosephSilber this is a work around, whereas I'm talking about architecture. What are the cons of my suggestion? I'm happy to agree I'm wrong if there are reasonable arguments.

[GrahamCampbell]

Or just don't use route model binding if you're concerned about performance...

[GrahamCampbell]

The only two of your clients are not able to do business with you anymore.

That's nothing to do with Laravel. To be frank, if you're code sucks, it's not our fault.

[GrahamCampbell]

you should be rate-limiting your API at a layer prior to your actual application.

If you are concerned about ddosing, you're defiantly going to want protection before the php layer.

CloudFlare offer a basic free service to put your app behind. Others are available too. :P

[andreygrehov]

@GrahamCampbell once again, what are the cons? Good framework should encourage to do a better job. I can of course do anything and not use bindings, but again, why would model binding exist at all then? If there is an auth middleware, why is it even allowed to do anything besides auth? That's what I don't understand.

[andreygrehov]

@GrahamCampbell well, that's not an answer, you could say "Go use Ruby" or "Find another job", essentially, this is the same as saying go buy CloudFlare solutions. I solved the problem by using the middleware, but that's the wrong path down the road. I don't know - it's simply wrong philosophy.

[GrahamCampbell]

You're free to choose ruby if it fits your needs better.

[malhal]

@GrahamCampbell please stop being childish. A lot of us want this problem fixed and you aren't helping. I think this is a pretty major design flaw, and that people are getting so passionate about it surely justifies it as so.

[GrahamCampbell]

I'm being totally serious, but the reality is that you need to just stop using route model binding because it doesn't fit your use case.

[GrahamCampbell]

Route model binding should definitely be done before the middlewares so the route parameter access is consistant.

You should note that you can actually have some middlewares that are processed BEFORE route model binding. The stuff registered in the kernel class should be processed before all the routing stuff has happened.

[malhal]

I sort of agree, because in the past when I've made use of cool Laravel features I usually turn them off as I need to do more advanced things. So it would be my guess if this was even fixed, we'd probably end up not using model binding for some other reason down the line.

[GrahamCampbell]

Alternatively, you could actually implement route model binding through a middleware. :)

[andreygrehov]

That's what I ended up doing. Original route model binding is a candidate for DDoS.

[GrahamCampbell]

Proper DDoS protection really belongs above the php level though, so I don't agree route model binding is really a target, especially since it is actually possible to bail out using a global middleware before route model binding has made any database queries.

[malhal]

Yeh I wouldn't use the DDoS argument for trying to get this fixed since there likely are a high number of other candidates, and remember all an attacker would need to do is log in with a fake account and then its the same problem. This is more about a design flaw, that plays with the mind of the coder who cares, who likely knows about trying to be as efficient with database comms as possible, and this problem is like a big fat middle finger in their face ;-)

[andreygrehov]

I agree that proper DDoS protection is out of PHP scope. But I still consider this architecture a bit awkward.

@malhal the thing is that an attacker does not need to log in at all in this case. He doesn't need to know any tokens, credentials, etc. Just the url. But I guess you understand that anyway.

[GrahamCampbell]

Doing your own route binding from a middleware isn't that bad tbh. Infact, I quite like that idea, and might even write one and publish it as a package. :)

[lanort]

Maybe this is not the smartest way but I just wrapped my bindings in the RouteServiceProvider.php in an if clause (App uses both: the check here and in the middleware)

if (Auth::check()) {
    // move the bindings here
}

EDIT: DOES NOT WORK. Oh gosh, does not work because Auth::check() will always return false at this stage.

[soee]

I think i have the same problem. Using model binding makes my global scope fail as i need to get in this scope authenticated user ID but it is always NULL in that case. So Laravel throws Trying to get property of non-object when calling Auth::user()->id in my global scope.

[websanova]

+1 for this fix. Auth should come before the route model binding. In my case I'm using JWT for auth, so definitely need the auth check first before route check...

In my case the api exposes certain parameters from a model based on privilege levels. So for instance a users profile only exposes an id and name to general public. But for the admins it exposes all fields.

[amitailanciano]

+1 since route binding comes first, the authenticated user doesn't exist in my user guard.

I feel like this is a pretty core use case -- @taylorotwell can you comment on why this issue was closed?

EDIT: Just read this: https://github.com/laravel/framework/issues/6118#issuecomment-149951364 however perhaps there ought to be some consideration into how this was implemented in the first place given that so many people encounter this use case.

It'd be great to see this functionality as part of the core route model binding feature, however it gets implemented.

Route::bind('event', function ($value, $route) {
    $event = EntityManager::getRepository(\App\Entities\Event::class)->findOneBy([
        'user' => auth('user')->user(),
        'id' => $value
    ]);

    if (!$event) {
        throw new NotFoundHttpException('Event entity not found');
    }

    return $event;
});

[malhal]

Another month gone by, anyone attempted to fix this show stopper?

[alioygur]

Laravel is Taylor's personnel project not an open source protect.
On Apr 30, 2016 12:39 AM, "Malcolm Hall" [email protected] wrote:

Another month gone by, anyone attempted to fix this show stopper?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/laravel/framework/issues/6118#issuecomment-215888607

[alioygur]

@rephole ben Go diline geçtim. Artık sikimde değil.

[neyaoz]

@alioygur Pire için yorgan yakılmaz, belki ölür bugün yarın.

[simondubois]

This blog post might help : https://murze.be/2016/05/using-current-user-route-model-binding/
Thanks @freekmurze

[stevebauman]

Just make sure the bindings or \Illuminate\Routing\Middleware\SubstituteBindings middleware is added after your authentication middleware.

[poisa]

I have another use case for this in multi-tenant sites. Let's say your model connects to your tenant db connection. This connection does not exist in the config file because it is dynamic; it depends on who is making the request. I need to authenticate the user and see which database tenant actually refers to. If model binding runs before my middleware, Laravel has no way to know which database to connect to.

@stevebauman's solution is quite right, alas it adds a bit of overhead in my particular design. But in my case, I removed the bindings middleware from all the groups, and added it manually to any route groups where I need model binding. It's more verbose that having this done by the framework, but still perfectly possible to achieve.

[devinfd]

This problem just bit me. I just learned that a user/attacker of my app was hitting an endpoint to see if records existed. Basically he was phishing the database. It wasn't really a DDOS attack because he was phishing each record manually so each request looked like normal. Long story short, once he learned which records did not exist he took that info to somebody else to complain about their work. I learned of this phishing from that third person that was concerned about how the attacker new which records were deleted. I'll spare you the details of why those records were deleted because the real issue seems to me that an UNAUTHENTICATED user was able to phish my database. THAT IS A SECURITY VULNERABILITY.

What is the actual reason that authentication does/can not happen before route model binding?

[GuidoHendriks]

@devinfd I think this issue has been resolved by now. Can't reproduce the behaviour you're talking about. You could try creating a new project and see if it still has this behaviour. If you find out it doesn't, it's probably in the configuration of your project.

If you believe you discovered a security vulnerability, you also shouldn't discuss it on Github, but email Taylor instead: https://laravel.com/docs/5.5/contributions#security-vulnerabilities

[devinfd]

@GuidoHendrik Thanks. My app is 5.5 and the bug does exists however I tried it on a fresh install of 5.5 and the bug did not exists. Long story short I discovered that the my app was using the old auth middleware 'auth' => \App\Http\Middleware\Authenticate::class. This was updated to 'auth' => \Illuminate\Auth\Middleware\Authenticate::class in 5.3 and I didn't catch that change. All is good now.

[garygreen]

@JosephSilber @taylorotwell I completely disagree that this use case should just be ignored - it's one of my real niggles with the framework at the moment; middlewares are fired after route model binding. This means that if you have a multi tenant application, or language-driven application, or global scopes, etc then your out of luck using the default built in route model binding:

$router->bind('account', function($id) {
   return Account::where('tenant_id', session('tenant_id'))->findOrFail();
});

Broke.

I can understand why the middlewares are fired after route model binding, but I don't believe it's outside the scope of the application to at least provide functionality to bind AFTER middlewares have fired. Something like:

Route::bindAfterMiddleware(function() {
   Route::bind('product', function($slug) {
      return Product::where('lang', session('lang_code', 'us'))->where('slug', $slug)->firstOrFail();
   });
});

Or for implicit route binding, hint which parameters should be bound after middlewares:

```php
Route::bindAfterMiddleware('product');
````

Or for binding after session middleware only:

```php
Route::bindAfterMiddleware('product', 'Illuminate\Session\Middleware\StartSession');
````

Something like that.

There really ought to be a way to cater for this without resorting to creating your own middleware to replace parameters in the url etc, otherwise it renders the whole route model binding feature quite useless in those cases. This would also help with any DDoS driven attacks based on urls as you could tell it to bind routes models AFTER throttler has run, etc.

Ref my suggestion: laravel/internals/issues/508

[malhal]

Loading in the models via binding before checking auth in middleware reminds me of Spectre!

[garygreen]

Loading in the models via binding before checking auth in middleware reminds me of Spectre!

Yah, it's definitely not ideal and I can totally understand the DDoS argument. Even if the throtter in Laravel would deny the request, your models have still already loaded and your more susceptible to a DDoS driven style attack on the database.

Like you said though, I wouldn't use that as an argument to fix this, though a fix would likely help with the DDoS issue too.

[devinfd]

@garygreen what Laravel version are you using? As I understand it, this was fixed in 5.3 however in order to receive that fix you must update the App\Http\Kernall $routeMiddleware auth key to use 'auth' => \Illuminate\Auth\Middleware\Authenticate::class

[garygreen]

@devinfd I think we're talking about different issues, this is nothing to do with auth. Route model binding is always fired before middlwares, even in Laravel 5.5.

[garygreen]

Nevermind, ignore me. I think since \Illuminate\Routing\Middleware\SubstituteBindings::class was added it just needs tweaking to be registered straight after session is initialised.

[jpuck]

Set Middleware Priority in App\Http\Kernel

For example, here I need my custom auth middleware to run first (before substitute bindings), so I unshift it onto the stack:

public function __construct(Application $app, Router $router)
{
    /**
     * Because we are using a custom authentication middleware,
     * we want to ensure it's executed early in the stack.
     */
    array_unshift($this->middlewarePriority, MyCustomApiAuthMiddleware::class);

    parent::__construct($app, $router);
}

Alternatively, you could override that entire priority structure if you needed explicit control (not recommended because you'll have to pay closer attention during upgrades to see if the framework changes). Specific to this issue is the SubstituteBindings class that handles route model binding, so just make sure your auth middleware comes sometime before that.

/**
 * The priority-sorted list of middleware.
 *
 * Forces the listed middleware to always be in the given order.
 *
 * @var array
 */
protected $middlewarePriority = [
    \App\Http\Middleware\MyCustomApiAuthMiddleware::class
    \Illuminate\Session\Middleware\StartSession::class,
    \Illuminate\View\Middleware\ShareErrorsFromSession::class,
    \Illuminate\Auth\Middleware\Authenticate::class,
    \Illuminate\Session\Middleware\AuthenticateSession::class,
    \Illuminate\Routing\Middleware\SubstituteBindings::class,
    \Illuminate\Auth\Middleware\Authorize::class,
];

[sebastiaanluca]

I rarely use(d) route binding, but for a new project I'm using https://github.com/Propaganistas/Laravel-FakeId which auto-resolves models from their fake ID and that kind of requires route binding. Of course, that's when I ran into this issue. Wanted to apply global scopes to those models _before they were resolved_, but Laravel does it the other way around. I can understand from a certain point of view, you want consistency. But sometimes that's a no-go security and usability-wise.

Here's my attempt at fixing the issue: https://gist.github.com/sebastiaanluca/d61e9e2bc874615f82cfd679dee8edce#gistcomment-2358542

Not a 100% covered and tested with this, but feedback welcome!

[denifelixe]

Good news come with laravel 5.3 above
as https://laravel.com/docs/5.3/upgrade said in Binding Substitution Middleware section

Route model binding is now accomplished using middleware. All applications should add the Illuminate\Routing\MiddlewareSubstituteBindings to your web middleware group in your app/Http/Kernel.php file

So now, the Route Model Binding is handled by Illuminate\Routing\MiddlewareSubstituteBindings in _web_ middleware group.

Since the _web_ middleware group always runs first, you can comment/remove this middleware in _web_ middleware group and add it to _routeMiddleware_

protected $middlewareGroups = [
        'web' => [
                ...
                ...
                // \Illuminate\Routing\Middleware\SubstituteBindings::class,
        ],
        ...
        ...
];

protected $routeMiddleware = [
        ...
        ...
        'bindings' => \Illuminate\Routing\Middleware\SubstituteBindings::class,
        ...
];

after doing that, you can add the 'bindings' middleware in specific order in your route.

e.g

Route::middleware(['auth', 'bindings'])->group(function () {

    Route::get('/', 'Home\HomeController@index');

});

in that example, 'auth' middleware run first before 'bindings' middleware.

but, in below example, 'bindings' middleware run first before 'auth' middleware :

Route::middleware(['bindings', 'auth'])->group(function () {

    Route::get('/', 'Home\HomeController@index');

});

Hope it helps 👍

[Stone624]

Adding a small tip to denifelixe's solution for this issue , I have a Route Group for Auth, Then Account Scopes (which sets scope for all models for the specific account type ie: only access owned resources, a custom middleware) , Then bindings :
Route::middleware(['auth','account_type_scope','bindings'])->group(function(){ // Routes Here // });

This is nice because the vast majority of my routes are within this group, And you can just add another group with just binding middleware bindings surrounding all other public routes as well.

If you have an existing and complex routes structure (where this isn't necessarily easy to refactor or possible), Try adding the middleware Construct Method of every controller, which did solve this issue for me as well. :

ModelController Extends Controller {
    public function __construct(){ $this->middleware('bindings'); }
}