0

So I created a middleware to limit the data a connected user has access to by adding global scopes depending on some informations:

public function handle(Request $request, Closure $next)
    {
          if (auth()->user()?->organization_id) {
              User::addGlobalScope(new OrganizationScope(auth()->user()->organization));
          }

        return $next($request);
    }

The middleware is added to the 'auth.group' middleware group in Kernel.php which is used in web.php:

Route::middleware(['auth.group'])->group(function () {
  Route::resource('users', UserController::class);
});

Then in the controller, I would expect a user to get a 404 when trying to see a page of a user he has no rights to. But the $user is retrieved before the middleware applies the global scope!

public function show(User $user, Request $request) {
  // dd($user); // <= This actually contains the User model! It shouldn't, of course.
  // dd(User::find($user->id)); // <= null, as it should!
}

So, the dependency is apparently calculated before the middleware is applied. If I'm trying to move the middleware into the 'web' group in Kernel.php it's the same. And in the main $middleware array, the authenticated user's data is not available yet.

I found this discussion that seems to be on topic : https://github.com/laravel/framework/issues/44177 but the possible solutions (and Taylor's PR) seems to point to a solution in the controller itself. Not what I'm trying to do, or I can't see how to adapt it.

Before that I was applying the global scopes at the Model level, in the booted function (as shown in the docs). But I had lots of issues with that - namely, accessing a relationship from there to check what is allowed or not is problematic, as the relationship call will look for something in the Model itself, and said model is not ready (that's the point of the booted method, right...). For example, checking a relationship of the connected user on the User model has to be done with a direct query to the db, that will be ran every time the Model is called... Not good.

Anyway, I like the middleware approach as it is a clean way to deal with rights as well, I think. Any recommandation?

Jeremy Belolo
  • 4,319
  • 6
  • 44
  • 88

1 Answers1

0

Not a recommendation, just my opinion.

This issue is just because of that Laravel allow you add middleware in controller constructor, and that's why it calculate before midddleware in your case.

I agree that middleware is a clean way to deal with auth, but i also think that you are not completely doing auth in your middleware, for example if you create a new route will you need to add something auth action into your new controller or just add auth middleware to route?

If does needs add something to controller, that means your auth middleware is just put some permissions info into global scope and you are doing the auth in controller which i think it's not right.

Controller should be only control the view logic, and you should do full auth in your auth middleware, once the request passed into your controller function that means user passed your auth.

For some example, if you auth permissions like below, you can just add auth middleware to new route without any action in your controller when you trying to create new route.

public function handle(Request $request, Closure $next)
{
    if (auth()->user()->canView($request->route())) { // you should do full auth, not just add informations.
        return $next($request);
    }
    else 
        abort(404);
}
Charlie
  • 287
  • 11
  • Hello, and thanks, I appreciate you trying to answer the question. Although, since it's unrelated to auth but to user's rights to see some data, I don't think your answer is relevant. What could be relevant is using policies instead, but in my case it's data the user shouldn't even be aware exists, hence the use of global scopes. The middleware is simply fired after the dependency, and that's the real issue. I need to be able to modify the request in some way before any dependency is calculated, and globally for all the routes. – Jeremy Belolo Feb 02 '23 at 09:57
  • Oh... sorry thro... my auth words means of permissions not user identify, but what i want to express is that i don't think middleware should be a dependency. – Charlie Feb 02 '23 at 10:35
  • And i wander how you abort when user trying to see some data. Shouldn't they be abort in middleware? Why you set depending informations but not directly check permissions and abort in middleware? – Charlie Feb 02 '23 at 11:09