laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.33k stars 10.96k forks source link

Auth::user() not working inside apply function in global scope #25611

Closed slavokozar closed 6 years ago

slavokozar commented 6 years ago

Description:

I was trying to create global scope dependent on a role of logged in user (typically, user with some acces rights based on role can see only users with lower access rights).

I user Auth::user() inside apply function of global scope and result was that my nginx server return http error 500. Not laravel error, but error directly from nginx after log time of loading...

However, it started to work, when I added Auth::user() as parameter to addGlobalScope :

 static::addGlobalScope(new UserVisibilityScope(Auth::user())); 

Steps To Reproduce:

Create global scope class and try to call $user = Auth::user() inside apply function

sisve commented 6 years ago

Can you provide a code snippet to reproduce the error? Preferably we can copy/paste to get the same problem.

It sounds like you're calling Auth::user() outside a request context, similar to how you cannot call it in a controller constructor, but without code it's hard to say for sure.

slavokozar commented 6 years ago

Hi there, thank you for response. Here is my scope:

class UserVisibilityScope implements Scope
{
    /**
     * Apply the scope to a given Eloquent query builder.
     *
     * @param  \Illuminate\Database\Eloquent\Builder $builder
     * @param  \Illuminate\Database\Eloquent\Model $model
     * @return void
     */
    public function apply(Builder $builder, Model $model)
    {
        $user = Auth::user();

        // some constrains based on the $user object
    }
}

I added this global scope in model User like this:

protected static function boot()
{
        parent::boot();
        static::addGlobalScope(new UserVisibilityScope);
}

Which caused error 500 reported after long loading of page directly from nginx.

However, when i modified method boot this way:

protected static function boot()
{
        parent::boot();
        static::addGlobalScope(new UserVisibilityScope(Auth::user()));
}

It started working!

staudenmeir commented 6 years ago

You do have use Auth; in the UserVisibilityScope file? How are you using the scope?

sisve commented 6 years ago

Your 500 error should contain more information we can use for debugging. If it doesn't check your logs for more details and stack traces.

kevinnio commented 6 years ago

@slavokozar Is it possible that you're running into a stack overflow issue?

The flow should be something like this:

  1. Auth::user() looks for a User model by running a query like this: User::where('id', session('user_id'))->first().
  2. UserVisibilityScope will be applied to this query since it's a global scope.
  3. While running UserVisibilityScope@apply(), Auth::user() gets called again.
  4. The whole thing is repeated until you reach a stack overflow.

On the other hand, if you call Auth::user() in the User::boot() method, its query would be resolved without applying UserVisibilityScope since this scope is not yet a global scope at that point of the execution.

slavokozar commented 6 years ago

@kevinnio I think that's exactly my case. Since I called Auth::user() in User:boot() method before applying my UserVisibilityScope result of Auth::user() is allready cached and is not queried again.

Thank you for explaining that for me.

DO you guys have idea how to solve this problem with a different approach? In my app i am querying list of users on many different places so I decided to use a global query.

staudenmeir commented 6 years ago

Can you post the actual UserVisibilityScope code?

slavokozar commented 6 years ago

No problem.

namespace App\Scopes;

use App\Classes\GroupRoles;
use App\Classes\SchoolRoles;
use App\Models\Users\Group;
use App\Models\Users\School;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Scope;
use Illuminate\Support\Facades\Auth;

class UserVisibilityScope implements Scope
{
    /**
     * Apply the scope to a given Eloquent query builder.
     *
     * @param  \Illuminate\Database\Eloquent\Builder $builder
     * @param  \Illuminate\Database\Eloquent\Model $model
     * @return void
     */
    public function apply(Builder $builder, Model $model)
    {
        $userObj = Auth::user();

        // admin can see anybody
        if (Auth::check() && !$userObj->isAdmin()) {

            // schools, in which logged user has admin or teacher role
            $schools = $userObj->schools()->wherePivotIn('role', [SchoolRoles::ADMIN, SchoolRoles::TEACHER])->pluck(School::TABLE_NAME . '.id');

            // groups, in which logged user has teacher role
            $groups = $userObj->groups()->wherePivot('role', GroupRoles::TEACHER)->pluck(Group::TABLE_NAME . '.id');

            $builder->where(function($query) use ($schools, $groups){
                if($schools->count() > 0)

                    $query->whereHas('schools', function ($query) use ($schools) {
                        $query->where(School::TABLE_NAME . '.id', $schools);
                    });

                if($groups->count() > 0)
                    $query->orWhereHas('groups', function ($query) use ($groups) {
                        $query->whereIn(Group::TABLE_NAME . '.id', $groups);
                    });
            });
        }
    }
}
slavokozar commented 6 years ago

Actually global scopes made me more problems, than solutions so I changed my scoles into local. Thank you guys, for explaining this problem to me. I will close this thread, because I think original problem was solved.

MahdiY commented 5 years ago

You can use hasUser method in Auth class.

class UserVisibilityScope implements Scope
{
    /**
     * Apply the scope to a given Eloquent query builder.
     *
     * @param  \Illuminate\Database\Eloquent\Builder $builder
     * @param  \Illuminate\Database\Eloquent\Model $model
     * @return void
     */
    public function apply(Builder $builder, Model $model)
    {
        if( Auth::hasUser() ) {

            $user = Auth::user();

            // some constrains based on the $user object
        }
    }
}
oludre commented 4 years ago

I faced this same issue on a laravel 5.8 setup. Fixed it by creating a middleware, add applicable scope to model and then apply middleware directly in controller.

As a n example, applying country scope to User model goes thus:

Create UserCountryScope.php middleware In the handle method, apply your scope

    public function handle($request, Closure $next)
    {
        if(!auth()->check()) {
            User::addGlobalScope('country', function (Builder $builder) {
                $country = session('country');
                $builder->where('country',$country);
            });
        }
        return $next($request);
    }

Now, in any controller you which to apply the scope (perhaps just for specific methods), specify the UserCountryScope middleware within the controllers constructor.

    public function __construct()
    {
        $this->middleware('userCountryScope')->only(['listings']); //This applies it only to listings method in this case
    }

Don't forget to register the alias in your applications route middleware in app/Http/Kernel.php

    protected $routeMiddleware = [
       ....
        'userCountryScope' => \App\Http\Middleware\UserCountryScope::class,
   ];

Applying scopes this way works best for me.

MaG21 commented 3 months ago

Solution use Auth::hasUser().

For anyone facing this problem, yes, using a global scope and calling auth->user() or Auth::user() inside the model User will cause a recursive call that will eventually fail because of memory (stack overflow).

This is what's happening

  1. Once laravel start processing a request, one of the first things laravel tries to do is to fetch and cache the user that belongs to the current session.
  2. Given that our User model has a global scope, this call unfortunately will also call the code in our global scope.
  3. If our global scope calls auth->user() it will make yet another query to get the current user.
  4. repeat step 2

How do we break this cycle ? Well first, know that Laravel will cache the value in auth->user() for obvious reasons, so we just need to prevent Laravel from using our global scope if it doesn't have the user for the current session already cached. There's a function called Auth::hasUser() it returns a boolean saying if Laravel has the current user cached or not.

So your global scope will now look like this


public function apply(Builder $builder, Model $model): void
{
    if (Auth::hasUser() === true) {
        // builder->where(), based on $user object
    }

}

Check out the PR for this here https://github.com/laravel/framework/pull/24518/files