archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.53k stars 423 forks source link

Fortify Doesn't work with Tenancy Auth not getting user. #702

Closed jeremiahsherrill closed 2 years ago

jeremiahsherrill commented 3 years ago

Describe the bug

Setup route middleware with InitializeTenancyByDomain::class, 'web', 'universal', 'auth', 'verify', PreventAccessFromCentralDomains::class,

After login, session data is stored in both the central database and the tenant database, but it is not getting the auth user, returns null. So if it attempts to visit any route that requires verify, it just redirects back to {tenant}.{domain}/ route as an auth user doesn't exist. Spent the last couple days on discord, following instruction on there I was able to get registration, and login working great, everything stores, but so far what information on discord is kind of a jumbled mess and nobody can't quite figure out to make fortify work and if they have, they don't post their solution.

I know this laravel fortify issue has been talked about, and everyone always gets sent to discord, but since there is no set answer in discord we need to address this here. Fortify is integral part of the Laravel ecosystem. If we can't get this working correctly, what other auth system can we use that just works with tenancy. A multi-tenant system that doesn't auth, kind of pointless.

Steps to reproduce

Install laravel/fortify Install Tenancy Setup tenant routes Make sure to

use Stancl\Tenancy\Middleware\InitializeTenancyByDomain; use Stancl\Tenancy\Middleware\PreventAccessFromCentralDomains;

in the proper locations

Expected behavior

I create the tenant in the backend of the central app, create a default user, user points their domain to {tenant}.{domain}/myadmin and they are presented with a login screen that logs them in to their admin.

Your setup

stancl commented 3 years ago

I know this laravel fortify issue has been talked about, and everyone always gets sent to discord, but since there is no set answer in discord we need to address this here. Fortify is integral part of the Laravel ecosystem.

This is not a bug, a bug would be something that works differently than described in the docs.

The issue with Fortify is that it uses constructor DI which the docs say isn't supported.

In version 4 I might try to find a way to improve this, but currently everything is working as described, so this is still a support question.

You can try opening a PR in laravel/fortify to change all instances of controller constructor DI to route action DI. I don't think Fortify should have to change this to support other packages, but practically it's just a few lines of code so they might be open to it for better compatibility with the ecosystem.

If we can't get this working correctly, what other auth system can we use that just works with tenancy.

There are many auth systems. laravel/ui, Breeze, Jetstream, and many more. The first two should work with tenancy perfectly.

A multi-tenant system that doesn't auth, kind of pointless.

Nothing's stopping you from using auth. You just can't use constructor DI or packages that use constructor DI.

I personally wouldn't be using packages for this, I'd just make a simple Livewire component and implement auth as needed for each project individually.

jhoopes commented 2 years ago

Hey @stancl , yep, this is an issue with the constructor DI:

For anyone else, because we use the domain/subdomain tenancy initialization with sanctum and fortify, this is what we did to get around this issue for now.

Basically just need to override how Fortify binds the StatefulGuard class into the container through its own register function.

<?php declare(strict_types=1);

namespace App\Providers;

use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Http\Request;
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Str;
use Laravel\Fortify\Fortify;
use Illuminate\Support\ServiceProvider;
use Illuminate\Cache\RateLimiting\Limit;
use Illuminate\Support\Facades\RateLimiter;
use Stancl\Tenancy\Exceptions\NotASubdomainException;
use Stancl\Tenancy\Resolvers\DomainTenantResolver;
use Stancl\Tenancy\Tenancy;
use Exception;

class FortifyServiceProvider extends ServiceProvider
{

    /**
     * Register any application services.
     *
     * @return void
     */
    public function register()
    {
        $this->app->bind(StatefulGuard::class, function () {
            /** @var Request $request */
            $request = app('request');

            $tenantDomain = $request->getHost();
            if ($this->isSubdomain($request->getHost())) {
                $tenantDomain = $this->makeSubdomain($request->getHost());

                if($tenantDomain instanceof Exception) {
                    throw $tenantDomain;
                }
            }

            /** @var Tenancy $tenancy */
            $tenancy = app(Tenancy::class);
            /** @var DomainTenantResolver $tenantResolver */
            $tenantResolver = app(DomainTenantResolver::class);

            if($tenancy->initialized) {
                return Auth::guard(config('fortify.guard', null));
            }

            $tenancy->initialize(
                $tenantResolver->resolve(
                    $tenantDomain
                )
            );

            return Auth::guard(config('fortify.guard', null));
        });
    }

    /**
     * Bootstrap any application services.
     *
     * @return void
     */
    public function boot()
    {
        // usual fortify overrides
    }

    /** Copy tenancy functions for usage with building fortify guard */

    /**
     * The index of the subdomain fragment in the hostname
     * split by `.`. 0 for first fragment, 1 if you prefix
     * your subdomain fragments with `www`.
     *
     * @var int
     */
    public static int $subdomainIndex = 0;

    protected function isSubdomain(string $hostname): bool
    {
        return Str::endsWith($hostname, config('tenancy.central_domains'));
    }

    /** @return Exception|string */
    protected function makeSubdomain(string $hostname)
    {
        $parts = explode('.', $hostname);

        $isLocalhost = count($parts) === 1;
        $isIpAddress = count(array_filter($parts, 'is_numeric')) === count($parts);

        // If we're on localhost or an IP address, then we're not visiting a subdomain.
        $isACentralDomain = in_array($hostname, config('tenancy.central_domains'), true);
        $notADomain = $isLocalhost || $isIpAddress;
        $thirdPartyDomain = ! Str::endsWith($hostname, config('tenancy.central_domains'));

        if ($isACentralDomain || $notADomain || $thirdPartyDomain) {
            return new NotASubdomainException($hostname);
        }

        return $parts[static::$subdomainIndex];
    }
}

Because some of the tenant resolving and domain resolving code isn't available to be called outside of their own respective classes, some of that code was copied into here. This doesn't feel the greatest, but, it is only generally called first when Fortify is building it's controller constructors. If tenancy is already initialized, it just continues on building up the guard without re-initializing.

Not sure if there's a better way other than modifying how Laravel's first party packages are doing things, but this at least makes it work on the sanctum, fortify side.

Edit: I should also note that this code is not accounting for 'universal' routes, so it's essentially binding fortify routes that build the statefull guard with DI to the tenant context vs the central one. So, another solution of sorts would need to be worked out if the fortify routes are needed in both central and tenant

stancl commented 2 years ago

Thanks a lot for sharing that @jhoopes. I wanted to write the usual explanation of why this is an issue with route level MW and that global MW solves this (but doesn't work well with non-domain identification), but I realized that in v4 it'd probably be best to add our own way of affecting the app. Something like middleware, but implemented a bit differently.

Basically we want to hook between routing and route action instantiation (controller constructor DI) and run tenancy there. I'll check the routing lifecycle in the Laravel codebase and find a way to implement this.

matt-breeden commented 2 years ago

I know this is closed, and don't expect anything from you until version 4 (crosses fingers), but I've been playing around and trying to get API auth to work with Sanctum and Tenancy. I noticed this in Laravel\Sanctum\SanctumServiceProvider and I wonder if it's the cause of a lot of these headaches:

$kernel->prependToMiddlewarePriority(EnsureFrontendRequestsAreStateful::class);

sicaboy commented 1 year ago

Thanks @jhoopes Do we have an official solution for this? When StatefulGuard DI is in any controller's __construct, it is reading session from central, even though the route is under tenant middleware.

stancl commented 1 year ago

The issue is pretty much what's described on the Early Identification page of v3 docs. We're in the process of finishing the early identification logic in the upcoming v4 version.

freziertz commented 11 months ago

Is there any schedule for version 4 release?

yusufkaracaburun commented 7 months ago

When can we expect version 4?

stancl commented 7 months ago

It's been mostly stable for a few months, we're working on adding some final features now. Hope to have everything done within Q1.

yusufkaracaburun commented 7 months ago

Is it possible get early access, because I really need it? I can always update it with the new features.

stancl commented 7 months ago

Yes, early access has been available for a long time. See the announcement on our Discord (you need to join first for the announcement link to work).

tahseen9 commented 3 months ago

TenancyServiceProvider should be loaded before fortify and jetstream

bootstrap/providers.php (This file is automatically generated by Laravel, just change the sequence as below) return [ App\Providers\TenancyServiceProvider::class, App\Providers\AppServiceProvider::class, App\Providers\FortifyServiceProvider::class, App\Providers\JetstreamServiceProvider::class, ];

I am using laravel 11.x with stancl/tenancy 3.8.3

sicaboy commented 3 months ago

@tahseen9 thanks, but modifying an auto-generated file is not an option for anyone

stancl commented 3 months ago

In L11 those files aren't autogenerated and can be modified (this was a bit unclear in early L11 iirc). Either way, v3 doesn't properly support Jetstream/Fortify.

tahseen9 commented 3 months ago

You are right, I had to do another change to run it properly, but i am not sure where its gonna break next.

according to L11 doc I did change the sequence

image

bootstrap/providers.php (This file is automatically generated by Laravel, just change the sequence as below) return [ App\Providers\TenancyServiceProvider::class, App\Providers\AppServiceProvider::class, App\Providers\FortifyServiceProvider::class, App\Providers\JetstreamServiceProvider::class, ];

and in my app service providers boot method:

public function boot(): void { $domainIsTenant = (new Domain)->where('domain', \request()->getHost());

    if($domainIsTenant->exists()){
        $tenancy = app(Tenancy::class);
        $tenantResolver = app(DomainTenantResolver::class);
        $tenancy->initialize(
            $tenantResolver->resolve(
                \request()->getHost()
            )
        );
    }
}

With this i am able to get the right db connection injected in Database Session Handler

image