From 56dd4117ab7f6dd44bd6cba35065a3f70f477a3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 9 Nov 2024 20:48:45 +0100 Subject: [PATCH] Fix origin id w/ empty header & using full-hostname subdomain records This makes it possible to have Domain records in both `foo` and `foo.{centralDomain}` format when using the combined domain/subdomain identification middleware, or the origin header id mw which extends it. This commit also refactors some related logic. --- composer.json | 1 + src/Middleware/InitializeTenancyByDomain.php | 7 ++- .../InitializeTenancyByDomainOrSubdomain.php | 53 ++++++++++------ .../InitializeTenancyBySubdomain.php | 12 ++-- src/Resolvers/DomainTenantResolver.php | 6 ++ ...edDomainAndSubdomainIdentificationTest.php | 62 +++++++++---------- tests/OriginHeaderIdentificationTest.php | 53 ++++++++++++++++ 7 files changed, 131 insertions(+), 63 deletions(-) diff --git a/composer.json b/composer.json index 7bafdce4..37f0b918 100644 --- a/composer.json +++ b/composer.json @@ -71,6 +71,7 @@ "docker-m1": "ln -s docker-compose-m1.override.yml docker-compose.override.yml", "testbench-unlink": "rm ./vendor/orchestra/testbench-core/laravel/vendor", "testbench-link": "ln -s vendor ./vendor/orchestra/testbench-core/laravel/vendor", + "testbench-repair": "mkdir -p ./vendor/orchestra/testbench-core/laravel/storage/framework/sessions && mkdir -p ./vendor/orchestra/testbench-core/laravel/storage/framework/views && mkdir -p ./vendor/orchestra/testbench-core/laravel/storage/framework/cache", "coverage": "open coverage/phpunit/html/index.html", "phpstan": "vendor/bin/phpstan", "phpstan-pro": "vendor/bin/phpstan --pro", diff --git a/src/Middleware/InitializeTenancyByDomain.php b/src/Middleware/InitializeTenancyByDomain.php index b9e049a4..60f1d316 100644 --- a/src/Middleware/InitializeTenancyByDomain.php +++ b/src/Middleware/InitializeTenancyByDomain.php @@ -44,7 +44,12 @@ class InitializeTenancyByDomain extends IdentificationMiddleware */ public function requestHasTenant(Request $request): bool { - return ! in_array($this->getDomain($request), config('tenancy.identification.central_domains')); + $domain = $this->getDomain($request); + + // Mainly used with origin identification if the header isn't specified and e.g. universal routes are used + if (! $domain) return false; + + return ! in_array($domain, config('tenancy.identification.central_domains')); } public function getDomain(Request $request): string diff --git a/src/Middleware/InitializeTenancyByDomainOrSubdomain.php b/src/Middleware/InitializeTenancyByDomainOrSubdomain.php index 845e42b3..77b64271 100644 --- a/src/Middleware/InitializeTenancyByDomainOrSubdomain.php +++ b/src/Middleware/InitializeTenancyByDomainOrSubdomain.php @@ -8,8 +8,9 @@ use Closure; use Exception; use Illuminate\Http\Request; use Illuminate\Http\Response; -use Illuminate\Support\Str; use Stancl\Tenancy\Concerns\UsableWithEarlyIdentification; +use Stancl\Tenancy\Resolvers\DomainTenantResolver; +use Stancl\Tenancy\Contracts\TenantCouldNotBeIdentifiedException; class InitializeTenancyByDomainOrSubdomain extends InitializeTenancyBySubdomain { @@ -23,34 +24,46 @@ class InitializeTenancyByDomainOrSubdomain extends InitializeTenancyBySubdomain } $domain = $this->getDomain($request); + $subdomain = null; - if ($this->isSubdomain($domain)) { - $domain = $this->makeSubdomain($domain); + if (DomainTenantResolver::isSubdomain($domain)) { + $subdomain = $this->makeSubdomain($domain); - if ($domain instanceof Exception) { + if ($subdomain instanceof Exception) { $onFail = static::$onFail ?? function ($e) { throw $e; }; - return $onFail($domain, $request, $next); - } - - // If a Response instance was returned, we return it immediately. - // todo@samuel when does this execute? - if ($domain instanceof Response) { - return $domain; + return $onFail($subdomain, $request, $next); } } - return $this->initializeTenancy( - $request, - $next, - $domain - ); - } + try { + $this->tenancy->initialize( + $this->resolver->resolve($subdomain ?? $domain) + ); + } catch (TenantCouldNotBeIdentifiedException $e) { + if ($subdomain) { + try { + $this->tenancy->initialize( + $this->resolver->resolve($domain) + ); + } catch (TenantCouldNotBeIdentifiedException $e) { + $onFail = static::$onFail ?? function ($e) { + throw $e; + }; - protected function isSubdomain(string $hostname): bool - { - return Str::endsWith($hostname, config('tenancy.identification.central_domains')); + return $onFail($e, $request, $next); + } + } else { + $onFail = static::$onFail ?? function ($e) { + throw $e; + }; + + return $onFail($e, $request, $next); + } + } + + return $next($request); } } diff --git a/src/Middleware/InitializeTenancyBySubdomain.php b/src/Middleware/InitializeTenancyBySubdomain.php index 07a9c68d..ede50ab8 100644 --- a/src/Middleware/InitializeTenancyBySubdomain.php +++ b/src/Middleware/InitializeTenancyBySubdomain.php @@ -8,9 +8,9 @@ use Closure; use Exception; use Illuminate\Http\Request; use Illuminate\Http\Response; -use Illuminate\Support\Str; use Stancl\Tenancy\Concerns\UsableWithEarlyIdentification; use Stancl\Tenancy\Exceptions\NotASubdomainException; +use Stancl\Tenancy\Resolvers\DomainTenantResolver; class InitializeTenancyBySubdomain extends InitializeTenancyByDomain { @@ -57,20 +57,16 @@ class InitializeTenancyBySubdomain extends InitializeTenancyByDomain ); } - /** @return string|Response|Exception|mixed */ + /** @return string|Exception */ 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.identification.central_domains'), true); - $notADomain = $isLocalhost || $isIpAddress; - $thirdPartyDomain = ! Str::endsWith($hostname, config('tenancy.identification.central_domains')); + $thirdPartyDomain = ! DomainTenantResolver::isSubdomain($hostname); - if ($isACentralDomain || $notADomain || $thirdPartyDomain) { + if ($isACentralDomain || $isIpAddress || $thirdPartyDomain) { return new NotASubdomainException($hostname); } diff --git a/src/Resolvers/DomainTenantResolver.php b/src/Resolvers/DomainTenantResolver.php index 34fd4c4f..4d34811e 100644 --- a/src/Resolvers/DomainTenantResolver.php +++ b/src/Resolvers/DomainTenantResolver.php @@ -12,6 +12,7 @@ use Stancl\Tenancy\Contracts\SingleDomainTenant; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; use Stancl\Tenancy\Tenancy; +use Illuminate\Support\Str; class DomainTenantResolver extends Contracts\CachedTenantResolver { @@ -55,6 +56,11 @@ class DomainTenantResolver extends Contracts\CachedTenantResolver return $tenant; } + public static function isSubdomain(string $domain): bool + { + return Str::endsWith($domain, config('tenancy.identification.central_domains')); + } + public function resolved(Tenant $tenant, mixed ...$args): void { $this->setCurrentDomain($tenant, $args[0]); diff --git a/tests/CombinedDomainAndSubdomainIdentificationTest.php b/tests/CombinedDomainAndSubdomainIdentificationTest.php index f7201b3a..85f11182 100644 --- a/tests/CombinedDomainAndSubdomainIdentificationTest.php +++ b/tests/CombinedDomainAndSubdomainIdentificationTest.php @@ -6,62 +6,56 @@ use Illuminate\Support\Facades\Route; use Stancl\Tenancy\Database\Concerns\HasDomains; use Stancl\Tenancy\Middleware\InitializeTenancyByDomainOrSubdomain; use Stancl\Tenancy\Database\Models; +use Stancl\Tenancy\Tests\Etc\Tenant; beforeEach(function () { Route::group([ 'middleware' => InitializeTenancyByDomainOrSubdomain::class, ], function () { - Route::get('/foo/{a}/{b}', function ($a, $b) { - return "$a + $b"; + Route::get('/test', function () { + return tenant('id'); }); }); - - config(['tenancy.models.tenant' => CombinedTenant::class]); }); test('tenant can be identified by subdomain', function () { config(['tenancy.identification.central_domains' => ['localhost']]); - $tenant = CombinedTenant::create([ - 'id' => 'acme', - ]); - - $tenant->domains()->create([ - 'domain' => 'foo', - ]); + $tenant = Tenant::create(['id' => 'acme']); + $tenant->domains()->create(['domain' => 'foo']); expect(tenancy()->initialized)->toBeFalse(); - pest() - ->get('http://foo.localhost/foo/abc/xyz') - ->assertSee('abc + xyz'); - - expect(tenancy()->initialized)->toBeTrue(); - expect(tenant('id'))->toBe('acme'); + pest()->get('http://foo.localhost/test')->assertSee('acme'); }); test('tenant can be identified by domain', function () { config(['tenancy.identification.central_domains' => []]); - $tenant = CombinedTenant::create([ - 'id' => 'acme', - ]); - - $tenant->domains()->create([ - 'domain' => 'foobar.localhost', - ]); + $tenant = Tenant::create(['id' => 'acme']); + $tenant->domains()->create(['domain' => 'foobar.localhost']); expect(tenancy()->initialized)->toBeFalse(); - pest() - ->get('http://foobar.localhost/foo/abc/xyz') - ->assertSee('abc + xyz'); - - expect(tenancy()->initialized)->toBeTrue(); - expect(tenant('id'))->toBe('acme'); + pest()->get('http://foobar.localhost/test')->assertSee('acme'); }); -class CombinedTenant extends Models\Tenant -{ - use HasDomains; -} +test('domain records can be either in domain syntax or subdomain syntax', function () { + config(['tenancy.identification.central_domains' => ['localhost']]); + + $foo = Tenant::create(['id' => 'foo']); + $foo->domains()->create(['domain' => 'foo']); + + $bar = Tenant::create(['id' => 'bar']); + $bar->domains()->create(['domain' => 'bar.localhost']); + + expect(tenancy()->initialized)->toBeFalse(); + + // Subdomain format + pest()->get('http://foo.localhost/test')->assertSee('foo'); + + tenancy()->end(); + + // Domain format + pest()->get('http://bar.localhost/test')->assertSee('bar'); +}); diff --git a/tests/OriginHeaderIdentificationTest.php b/tests/OriginHeaderIdentificationTest.php index a32777da..83737f1f 100644 --- a/tests/OriginHeaderIdentificationTest.php +++ b/tests/OriginHeaderIdentificationTest.php @@ -38,6 +38,12 @@ test('origin identification works', function () { }); test('tenant routes are not accessible on central domains while using origin identification', function () { + $tenant = Tenant::create(); + + $tenant->domains()->create([ + 'domain' => 'foo', + ]); + pest() ->withHeader('Origin', 'localhost') ->post('home') @@ -54,3 +60,50 @@ test('onfail logic can be customized', function() { ->post('home') ->assertSee('onFail message'); }); + +test('origin identification can be used with universal routes', function () { + $tenant = Tenant::create(); + + $tenant->domains()->create([ + 'domain' => 'foo', + ]); + + Route::post('/universal', function () { + return response(tenant('id') ?? 'central'); + })->middleware([InitializeTenancyByOriginHeader::class, 'universal'])->name('universal'); + + pest() + ->withHeader('Origin', 'foo.localhost') + ->post('universal') + ->assertSee($tenant->id); + + tenancy()->end(); + + pest() + ->withHeader('Origin', 'localhost') + ->post('universal') + ->assertSee('central'); + + pest() + // no header + ->post('universal') + ->assertSee('central'); +}); + +test('origin identification can be used with both domains and subdomains', function () { + $foo = Tenant::create(); + $foo->domains()->create(['domain' => 'foo']); + + $bar = Tenant::create(); + $bar->domains()->create(['domain' => 'bar.localhost']); + + pest() + ->withHeader('Origin', 'foo.localhost') + ->post('home') + ->assertSee($foo->id); + + pest() + ->withHeader('Origin', 'bar.localhost') + ->post('home') + ->assertSee($bar->id); +});