diff --git a/assets/config.php b/assets/config.php index 702af5b0..357c4b26 100644 --- a/assets/config.php +++ b/assets/config.php @@ -19,6 +19,11 @@ return [ // ], + 'central_domains' => [ + '127.0.0.1', + 'localhost', + ], + 'storage' => [ 'data_column' => 'data', @@ -232,6 +237,6 @@ return [ * Middleware pushed to the global middleware stack. */ 'global_middleware' => [ // todo get rid of this - Stancl\Tenancy\Middleware\InitializeTenancy::class, + // Stancl\Tenancy\Middleware\InitializeTenancy::class, ], ]; diff --git a/src/Contracts/Domain.php b/src/Contracts/Domain.php new file mode 100644 index 00000000..ba7d2e2f --- /dev/null +++ b/src/Contracts/Domain.php @@ -0,0 +1,13 @@ +domain)->first()) { if ($domain->getKey() !== $self->getKey()) { - throw new DomainsOccupiedByOtherTenantException; + throw new DomainOccupiedByOtherTenantException($self->domain); } } }; diff --git a/src/Exceptions/DomainOccupiedByOtherTenantException.php b/src/Exceptions/DomainOccupiedByOtherTenantException.php new file mode 100644 index 00000000..00d42f3e --- /dev/null +++ b/src/Exceptions/DomainOccupiedByOtherTenantException.php @@ -0,0 +1,15 @@ +tenancy->initialize( + $this->resolver->resolve(...$resolverArguments) + ); + } catch (TenantCouldNotBeIdentifiedException $e) { + $onFail = static::$onFail ?? function ($e) { + throw $e; + }; + + return $onFail($e, $request, $next); + } + + return $next($request); + } +} \ No newline at end of file diff --git a/src/Middleware/InitializeTenancy.php b/src/Middleware/InitializeTenancy.php deleted file mode 100644 index c8b3bd2f..00000000 --- a/src/Middleware/InitializeTenancy.php +++ /dev/null @@ -1,45 +0,0 @@ -onFail = $onFail ?? function ($e) { - throw $e; - }; - } - - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed - */ - public function handle($request, Closure $next) - { - if (tenancy()->initialized) { - return $next($request); - } - - if (! in_array($request->getHost(), config('tenancy.exempt_domains', []), true)) { - try { - tenancy()->init($request->getHost()); - } catch (TenantCouldNotBeIdentifiedException $e) { - return ($this->onFail)($e, $request, $next); - } - } - - return $next($request); - } -} diff --git a/src/Middleware/InitializeTenancyByDomain.php b/src/Middleware/InitializeTenancyByDomain.php new file mode 100644 index 00000000..24a1abb7 --- /dev/null +++ b/src/Middleware/InitializeTenancyByDomain.php @@ -0,0 +1,41 @@ +tenancy = $tenancy; + $this->resolver = $resolver; + } + + /** + * Handle an incoming request. + * + * @param \Illuminate\Http\Request $request + * @param \Closure $next + * @return mixed + */ + public function handle($request, Closure $next) + { + return $this->initializeTenancy( + $request, $next, $request->getHost() + ); + } +} diff --git a/src/Middleware/InitializeTenancyByPath.php b/src/Middleware/InitializeTenancyByPath.php index 7e5c92a8..3e4db04e 100644 --- a/src/Middleware/InitializeTenancyByPath.php +++ b/src/Middleware/InitializeTenancyByPath.php @@ -8,7 +8,7 @@ use Illuminate\Routing\Route; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Stancl\Tenancy\Tenancy; -class InitializeTenancyByPath +class InitializeTenancyByPath extends IdentificationMiddleware { /** @var Tenancy */ protected $tenancy; @@ -27,14 +27,15 @@ class InitializeTenancyByPath /** @var Route $route */ $route = $request->route(); + // todo test the behavior described by the comment // Only initialize tenancy if tenant is the first parameter // We don't want to initialize tenancy if the tenant is // simply injected into some route controller action. if ($route->parameterNames()[0] === 'tenant') { - $this->tenancy->initialize( - $this->resolver->resolve($route) + return $this->initializeTenancy( + $request, $next, $route ); - } + } // todo else case should probably throw exception about malformed route? or do we just leave that as the developer's responsibility? return $next($request); } diff --git a/src/Middleware/InitializeTenancyBySubdomain.php b/src/Middleware/InitializeTenancyBySubdomain.php new file mode 100644 index 00000000..2bad3c50 --- /dev/null +++ b/src/Middleware/InitializeTenancyBySubdomain.php @@ -0,0 +1,67 @@ +makeSubdomain($request->getHost()); + + // If a non-string, like a Response instance was returned + // from makeSubdomain() - due to NotASubDomainException + // being thrown, we abort by returning the value now. + if (! is_string($subdomain)) { + return $subdomain; + } + + return $this->initializeTenancy( + $request, $next, $subdomain + ); + } + + /** @return string|Response|mixed */ + protected function makeSubdomain(string $hostname) + { + $parts = explode('.', $hostname); + + // If we're on localhost or an IP address, then we're not visiting a subdomain. + if (in_array(count($parts), [1, 4])) { + $handle = static::$onInvalidSubdomain ?? function ($e) { + throw $e; + }; + + return $handle(new NotASubdomainException($hostname)); + } + + // todo should we verify that the subdomain belongs to one of our central domains? + // if yes, then write a test for it. + + return $parts[static::$subdomainIndex]; + } +} diff --git a/src/Middleware/PreventAccessFromTenantDomains.php b/src/Middleware/PreventAccessFromTenantDomains.php deleted file mode 100644 index 83afb31f..00000000 --- a/src/Middleware/PreventAccessFromTenantDomains.php +++ /dev/null @@ -1,75 +0,0 @@ -central404 = $central404 ?? function () { - return 404; - }; - } - - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed - */ - public function handle($request, Closure $next) - { - // If the route is universal, always let the request pass. - if ($this->routeHasMiddleware($request->route(), 'universal')) { - return $next($request); - } - - // If the domain is not in exempt domains, it's a tenant domain. - // Tenant domains can't have routes without tenancy middleware. - $isExemptDomain = in_array($request->getHost(), config('tenancy.exempt_domains')); - $isTenantDomain = ! $isExemptDomain; - - $isTenantRoute = $this->routeHasMiddleware($request->route(), 'tenancy'); - - if ($isTenantDomain && ! $isTenantRoute) { // accessing web routes from tenant domains - return redirect(config('tenancy.home_url')); - } - - if ($isExemptDomain && $isTenantRoute) { // accessing tenant routes on web domains - return ($this->central404)($request, $next); - } - - return $next($request); - } - - public static function routeHasMiddleware(Route $route, $middleware): bool - { - if (in_array($middleware, $route->middleware(), true)) { - return true; - } - - // Loop one level deep and check if the route's middleware - // groups have a `tenancy` middleware group inside them - $middlewareGroups = Router::getMiddlewareGroups(); - foreach ($route->gatherMiddleware() as $inner) { - if (! $inner instanceof Closure && isset($middlewareGroups[$inner]) && in_array($middleware, $middlewareGroups[$inner], true)) { - return true; - } - } - - return false; - } -} diff --git a/src/Resolvers/DomainTenantResolver.php b/src/Resolvers/DomainTenantResolver.php index bfba9c5f..b00c1cc4 100644 --- a/src/Resolvers/DomainTenantResolver.php +++ b/src/Resolvers/DomainTenantResolver.php @@ -2,6 +2,7 @@ namespace Stancl\Tenancy\Resolvers; +use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Contracts\TenantResolver; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; @@ -10,6 +11,7 @@ class DomainTenantResolver implements TenantResolver { public function resolve(...$args): Tenant { + /** @var Domain $domain */ $domain = config('tenancy.domain_model')::where('domain', $args[0])->first(); if ($domain) { @@ -18,4 +20,4 @@ class DomainTenantResolver implements TenantResolver throw new TenantCouldNotBeIdentifiedOnDomainException($domain); } -} \ No newline at end of file +} diff --git a/src/Tenancy.php b/src/Tenancy.php index afd063a1..bca42a5a 100644 --- a/src/Tenancy.php +++ b/src/Tenancy.php @@ -3,7 +3,7 @@ namespace Stancl\Tenancy; use Stancl\Tenancy\Contracts\TenancyBootstrapper; -use Stancl\Tenancy\Database\Models\Tenant; +use Stancl\Tenancy\Database\Models\Tenant; // todo contract class Tenancy { @@ -18,6 +18,11 @@ class Tenancy public function initialize(Tenant $tenant): void { + // todo the id is something that should be on the contract, with a method + if ($this->initialized && $this->tenant->id === $tenant->id) { + return; + } + $this->tenant = $tenant; $this->initialized = true; diff --git a/tests/v3/DomainTest.php b/tests/v3/DomainTest.php index efb72a1a..898f7be5 100644 --- a/tests/v3/DomainTest.php +++ b/tests/v3/DomainTest.php @@ -2,10 +2,12 @@ namespace Stancl\Tenancy\Tests\v3; +use Illuminate\Support\Facades\Route; use Stancl\Tenancy\Database\Models; use Stancl\Tenancy\Database\Models\Concerns\HasDomains; -use Stancl\Tenancy\Exceptions\DomainsOccupiedByOtherTenantException; +use Stancl\Tenancy\Exceptions\DomainOccupiedByOtherTenantException; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; +use Stancl\Tenancy\Middleware\InitializeTenancyByDomain; use Stancl\Tenancy\Resolvers\DomainTenantResolver; use Stancl\Tenancy\Tests\TestCase; @@ -15,6 +17,14 @@ class DomainTest extends TestCase { parent::setUp(); + Route::group([ + 'middleware' => InitializeTenancyByDomain::class, + ], function () { + Route::get('/foo/{a}/{b}', function ($a, $b) { + return "$a + $b"; + }); + }); + config(['tenancy.tenant_model' => Tenant::class]); } @@ -46,7 +56,7 @@ class DomainTest extends TestCase $tenant2 = Tenant::create(); - $this->expectException(DomainsOccupiedByOtherTenantException::class); + $this->expectException(DomainOccupiedByOtherTenantException::class); $tenant2->domains()->create([ 'domain' => 'foo.localhost', ]); @@ -61,29 +71,40 @@ class DomainTest extends TestCase } /** @test */ - public function tenancy_is_initialized_prior_to_controller_constructors() + public function tenant_can_be_identified_by_domain() { - // todo - $this->assertTrue(app('tenancy_was_initialized_in_constructor')); + $tenant = Tenant::create([ + 'id' => 'acme', + ]); + + $tenant->domains()->create([ + 'domain' => 'foo.localhost', + ]); + + $this->assertFalse(tenancy()->initialized); + + $this + ->get('http://foo.localhost/foo/abc/xyz') + ->assertSee('abc + xyz'); + $this->assertTrue(tenancy()->initialized); $this->assertSame('acme', tenant('id')); } + + /** @test */ + public function onfail_logic_can_be_customized() + { + InitializeTenancyByDomain::$onFail = function () { + return 'foo'; + }; + + $this + ->get('http://foo.localhost/foo/abc/xyz') + ->assertSee('foo'); + } } class Tenant extends Models\Tenant { use HasDomains; } - -class TestController -{ - public function __construct() - { - app()->instance('tenancy_was_initialized_in_constructor', tenancy()->initialized); - } - - public function index() - { - return 'foo'; - } -} \ No newline at end of file diff --git a/tests/v3/PathIdentificationTest.php b/tests/v3/PathIdentificationTest.php index cddc51f5..dc440b2b 100644 --- a/tests/v3/PathIdentificationTest.php +++ b/tests/v3/PathIdentificationTest.php @@ -10,13 +10,6 @@ use Stancl\Tenancy\Tests\TestCase; class PathIdentificationTest extends TestCase { - public function getEnvironmentSetup($app) - { - parent::getEnvironmentSetUp($app); - - config(['tenancy.global_middleware' => []]); - } - public function setUp(): void { parent::setUp(); @@ -28,8 +21,6 @@ class PathIdentificationTest extends TestCase Route::get('/foo/{a}/{b}', function ($a, $b) { return "$a + $b"; }); - - Route::get('/bar', [TestController::class, 'index']); }); } @@ -42,8 +33,7 @@ class PathIdentificationTest extends TestCase $this->assertFalse(tenancy()->initialized); - $this - ->get('/acme/foo/abc/xyz'); + $this->get('/acme/foo/abc/xyz'); $this->assertTrue(tenancy()->initialized); $this->assertSame('acme', tenant('id')); @@ -77,4 +67,16 @@ class PathIdentificationTest extends TestCase $this->assertFalse(tenancy()->initialized); } + + /** @test */ + public function onfail_logic_can_be_customized() + { + InitializeTenancyByPath::$onFail = function () { + return 'foo'; + }; + + $this + ->get('/acme/foo/abc/xyz') + ->assertSee('foo'); + } } diff --git a/tests/v3/SubdomainIdentificationTest.php b/tests/v3/SubdomainIdentificationTest.php deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/v3/SubdomainTest.php b/tests/v3/SubdomainTest.php new file mode 100644 index 00000000..38b3054e --- /dev/null +++ b/tests/v3/SubdomainTest.php @@ -0,0 +1,101 @@ + InitializeTenancyBySubdomain::class, + ], function () { + Route::get('/foo/{a}/{b}', function ($a, $b) { + return "$a + $b"; + }); + }); + + config(['tenancy.tenant_model' => Tenant::class]); + } + + /** @test */ + public function tenant_can_be_identified_by_subdomain() + { + $tenant = Tenant::create([ + 'id' => 'acme', + ]); + + $tenant->domains()->create([ + 'domain' => 'foo', + ]); + + $this->assertFalse(tenancy()->initialized); + + $this + ->get('http://foo.localhost/foo/abc/xyz') + ->assertSee('abc + xyz'); + + $this->assertTrue(tenancy()->initialized); + $this->assertSame('acme', tenant('id')); + } + + /** @test */ + public function onfail_logic_can_be_customized() + { + InitializeTenancyBySubdomain::$onFail = function () { + return 'foo'; + }; + + $this + ->get('http://foo.localhost/foo/abc/xyz') + ->assertSee('foo'); + } + + /** @test */ + public function localhost_is_not_a_valid_subdomain() + { + $this->expectException(NotASubdomainException::class); + + $this + ->withoutExceptionHandling() + ->get('http://localhost/foo/abc/xyz'); + } + + /** @test */ + public function ip_address_is_not_a_valid_subdomain() + { + $this->expectException(NotASubdomainException::class); + + $this + ->withoutExceptionHandling() + ->get('http://127.0.0.1/foo/abc/xyz'); + } + + /** @test */ + public function oninvalidsubdomain_logic_can_be_customized() + { + // in this case, we need to return a response instance + // since a string would be treated as the subdomain + InitializeTenancyBySubdomain::$onInvalidSubdomain = function () { + return response('foo custom invalid subdomain handler'); + }; + + $this + ->withoutExceptionHandling() + ->get('http://127.0.0.1/foo/abc/xyz') + ->assertSee('foo custom invalid subdomain handler'); + } +} + +class Tenant extends Models\Tenant +{ + use HasDomains; +}