Scope Not being applied in middleware

[ghosh]

I added a ScopeRequestByUser middleware like so

public function handle($request, Closure $next)
{

    if ($request->ajax() || $request->wantsJson()) {
        Landlord::addTenant('user_id', Auth::guard('api')->id());
    }

    return $next($request);
}

Now this works perfectly when I add this to my global middleware stack like this.

protected $middleware = [
    \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
    \App\Http\Middleware\ScopeRequestByUser::class,
];

However, I need it to be applied only on the api middleware group. When I add to the group in the middleware file it does not work.

    protected $middlewareGroups = [
        'web' => [
          ...
        ],

        'api' => [
            'throttle:60,1',
            'bindings',
            \App\Http\Middleware\ScopeRequestByUser::class,
        ],
    ];

I even tried adding it ot the route middleware and then calling it both in the routes file and in the api middleware group. Both of them are not working.

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

Not sure, what is going wrong here.

[dschniepp]

@ghosh did you find a solution?

[ghosh]

@dschniepp Not really, I just ended up using it in the Global Middleware Stack. Hoping this issue can be resolved soon.

[hipsterjazzbo]

When you say, "does not work", what do you mean? Tenant are not scoped correctly? At a guess I'd say that the call to Auth::guard('api')->id() is returning null for some reason when you register your middleware non-globally?

[dschniepp]

When the middleware for setting the tenant will be applied in $middleware everything works as expected and the scoping will be run after that. But if the middleware will be registered in some sort of $routeMiddleware the scope will be applied before the middleware.

[ghosh]

I replaced the call to Landlord::addTenant('user_id', Auth::guard('api')->id()); by Log::info(Auth::guard('api')->id()); in the ScopeRequestByUser middleware and it successfully logs the user id.

This works regardless of weather the middleware is called in the global middleware or routeMiddleware. So the Auth::guard('api')->id() is definitely not returning null.

[hipsterjazzbo]

Maybe it's got something to do with one happening before package registration? I'm really stumped here. I'm using Landlord in probably 5 or 6 different middlewares of various types and it works fine.

[hipsterjazzbo]

This isn't the same issue as #33 is it?

[ghosh]

Hmm...Route Model Binding doesn't seem to be the issue.

Within my routes file, I have this:-

Route::get('/projects', 'ProjectController@all');

and the corresponding method in the controller:-

public function all() {
  return Project::all();
}

The above does not work.
However, if I return from within the routes file itself, it seems to work:-

Route::get('/projects', function () {
  return Project::all();
});

This works! Not sure whats happening here.

[torkiljohnsen]

I am having a similar issue. My problem seems to be the timing of when the tenant is being registered, because some models are being booted before the request is run through the middleware where the tenant is set.

From what I can tell, this is the order of things:

1. Kernel runs sendRequestThroughRouter
2. In this method, bootstrap() is run
3. Bootstrapping ends with registering providers, then booting them. In some cases, like if you register a model observer, the models will be booted now, which will attempt to set the global scope for tenant. At this point though, the middleware has not been run yet, so no tenant is registered, hence setting up the global scope here fails…
4. After this, the request is run through the middleware where you are setting your tenant. Any models booted after this point will work as expected. Models that were booted prematurely in the previous step will not have the global scope set since they can only be booted once.

[torkiljohnsen]

So, the real root of the problem was found finally.

We had set up observers in the boot method of a service provider, which caused implicated models to boot before middleware was run. There was also another issue that made the models boot prematurely. All resolved now :)

We also had to move the middleware we had set up to register the request tenant from the middlewareGroup to the global middleware in the kernel.

[dschniepp]

@torkiljohnsen but registering in a $middlewareGroup didn't work or?

[torkiljohnsen]

I haven't dug completely down through the call stack to see in what order things are happening in here, but the Kernel seems to be handling running the request through the global $middleware and then the router is responsible for running the request through the $routeMiddleware and $middlewareGroups.

The whole point here is to have the middleware run (and tenant ID set) BEFORE we attempt to set the global scope in models (meaning before any model is booted).

In my case, that meant having to move the middleware to the global $middleware. Having it in the middleware handled by the router fails.

[torkiljohnsen]

Also: It seems like I will also have to rewrite some code to not use model observers, as that possibly makes models boot when the service providers are booted, before the request is pushed through the global middleware.

[dschniepp]

There seems to be a change the implementation between 1.x and 2.x, because 1.x works.

[torkiljohnsen]

We changed our approach a bit. The issue is with booting models before the middleware has run. Registering model observers will boot the model, since Model::observe will do new static;, which calls the constructor and boots the model.

So now I am registering observers like this:

Event::listen('eloquent.booted: ' . User::class, function () {
    User::observe(UserObserver::class);
});

This ensures that the observer isn't added until after the User model has been booted by some other process.

Also added an issue for this, can hopefully spark a discussion: laravel/framework#16715

[MrRio]

This is an issue I'm running up against. Where would I add this Event::listen code?

[torkiljohnsen]

In a service provider's register method for instance.

[r3donnx]

It does work, then I add middleware to the global $middleware. The only problem now, is that in global middleware I can't get authenticated user, it returns null. So I am unable to get users tenant_id.

[feralc]

My temporary solution (not the best) was to extend trait, and then I added the tenant after getting the TenantManager from laravel container.

namespace App\Traits;

use HipsterJazzbo\Landlord\BelongsToTenants as BaseBelongsToTenants;
use HipsterJazzbo\Landlord\TenantManager;
use Illuminate\Database\Eloquent\Model;

trait BelongsToTenants
{
    use BaseBelongsToTenants;

    public static function bootBelongsToTenants()
    {
        // Grab our singleton from the container
        static::$landlord = app(TenantManager::class);

        //My temporarily approach
        static::$landlord->addTenant('workspace_id', currentWorkspace()->id);

        // Add a global scope for each tenant this model should be scoped by.
        static::$landlord->applyTenantScopes(new static());

        // Add tenantColumns automatically when creating models
        static::creating(function (Model $model) {
            static::$landlord->newModel($model);
        });
    }

}

[hipsterjazzbo]

Here's my understanding:

You can't scope by Auth::user() in a global middleware, because the session's not started yet (at least in 5.3+).

Also, until just now, Landlord would accept a NULL user id and carry on happily, which is now fixed in 2.0.6.

Lastly, using Model Observers causes the model's boot sequence to happen before tenants have been added, although any queries done after this point should still work, as the tenants that are set are not actually applied to the model til a query is run.

Does that all seem accurate?

[Wipsly]

You can add

\App\Http\Middleware\EncryptCookies::class,
\Illuminate\Session\Middleware\StartSession::class,

To your global middleware and then Auth::user() is working.

But I still have a Problem. My middleware is working when i do the following

Landlord::addTenant('company_id', 1);

Then it works and I only get the user from company 1.

But when I do the following:

$tenant = Auth::user()->company->id;
Landlord::addTenant('company_id', $tenant);

It does not scope and shows all user. When i dd($tenant) it is 1.

[kmcluckie]

It's not only using model observers that cause the model's boot sequence to happen before tenants have been added - I was using dependency injection to instantiate a model repository, and that also caused the model to boot before the route middleware could run and add a tenant. If I added the middleware to the global list (as well as the session and cookie middlewares so I could access the logged in user) then the tenant would be set first. However, I don't want it set on all routes.

The solution was using the suggestion in #61. My middleware uses clearBootedModels just before setting the tenant id:

public function handle($request, Closure $next)
{
    $user = app('Dingo\Api\Auth\Auth')->user();

    \Illuminate\Database\Eloquent\Model::clearBootedModels();

    foreach($user->tenantIds() as $name => $id)
    {
        Landlord::addTenant($name, $id);
    }

    return $next($request);
}