From ec06dcc52e24859e3beb3486e60368fae087bacc Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 11 May 2026 14:26:06 +0200 Subject: [PATCH 1/2] Correct `DomainTenantResolver::isSubdomain()` check (#1425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a failing test for determining if a host is a subdomain, then fixed `DomainTenantResolver::isSubdomain()` (similar fix as in #1423) and a related assertion. Previously, while having `tenancy.identification.central_domains` set to e.g. `['site.com']`, the `isSubdomain()` check consider `tenantsite.com` a subdomain because it ends with `site.com`. Now, instead of the `endsWith()` check, the method checks if the passed domain is in the configured central domains. If it is, it returns `false`. Otherwise, loop through all the central domains and check if the passed domain matches any of the central domains prefixed with a dot (e.g. `tenant.site.com` would be considered a subdomain, `tenant.site.com` wouldn't). Because in InitializeTenancyByDomainOrSubdomain, if tenancy fails to initialize using a subdomain (before this PR's changes, e.g. `tenantsite.com` would be considered a subdomain, and `tenantsite` would be used for initializing tenancy), it'll catch the exception and use the whole domain for identification instead, this error will likely never be noticed in real-world usage. So this PR corrects the subdomain detection logic, but the real-world impact of that is negligible. > Note: The subdomain error catching logic in domainOrSubdomain ID MW was added in v4. If we applied this change in v3, it'd fix a real issue where domainOrSubdomain ID MW would just fail at the subdomain initialization, without attempting domain initialization after the failure. --------- Co-authored-by: github-actions[bot] Co-authored-by: Samuel Štancl --- src/Resolvers/DomainTenantResolver.php | 15 ++++++++++++++- tests/EarlyIdentificationTest.php | 2 +- tests/SubdomainTest.php | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Resolvers/DomainTenantResolver.php b/src/Resolvers/DomainTenantResolver.php index 9535cdf2..59ebb81f 100644 --- a/src/Resolvers/DomainTenantResolver.php +++ b/src/Resolvers/DomainTenantResolver.php @@ -7,6 +7,7 @@ namespace Stancl\Tenancy\Resolvers; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\Relation; +use Illuminate\Support\Arr; use Illuminate\Support\Str; use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\SingleDomainTenant; @@ -58,7 +59,19 @@ class DomainTenantResolver extends Contracts\CachedTenantResolver public static function isSubdomain(string $domain): bool { - return Str::endsWith($domain, config('tenancy.identification.central_domains')); + $centralDomains = Arr::wrap(config('tenancy.identification.central_domains')); + + if (in_array($domain, $centralDomains, true)) { + return false; + } + + foreach ($centralDomains as $centralDomain) { + if (Str::endsWith($domain, '.' . $centralDomain)) { + return true; + } + } + + return false; } public function resolved(Tenant $tenant, mixed ...$args): void diff --git a/tests/EarlyIdentificationTest.php b/tests/EarlyIdentificationTest.php index e6c08d26..4521b613 100644 --- a/tests/EarlyIdentificationTest.php +++ b/tests/EarlyIdentificationTest.php @@ -300,7 +300,7 @@ test('using different default route modes works with global domain identificatio $exception = match ($middleware) { InitializeTenancyByDomain::class => TenantCouldNotBeIdentifiedOnDomainException::class, InitializeTenancyBySubdomain::class => NotASubdomainException::class, - InitializeTenancyByDomainOrSubdomain::class => NotASubdomainException::class, + InitializeTenancyByDomainOrSubdomain::class => TenantCouldNotBeIdentifiedOnDomainException::class, }; expect(fn () => $this->withoutExceptionHandling()->get('http://localhost/central-route'))->toThrow($exception); diff --git a/tests/SubdomainTest.php b/tests/SubdomainTest.php index a7cc58ae..62e002f2 100644 --- a/tests/SubdomainTest.php +++ b/tests/SubdomainTest.php @@ -7,6 +7,7 @@ use Stancl\Tenancy\Database\Concerns\HasDomains; use Stancl\Tenancy\Exceptions\NotASubdomainException; use Stancl\Tenancy\Middleware\InitializeTenancyBySubdomain; use Stancl\Tenancy\Database\Models; +use Stancl\Tenancy\Resolvers\DomainTenantResolver; use function Stancl\Tenancy\Tests\pest; beforeEach(function () { @@ -108,6 +109,14 @@ test('we cant use a subdomain that doesnt belong to our central domains', functi ->get('http://foo.localhost/foo/abc/xyz'); }); +test('domain resolver correctly determines if string is a subdomain', function() { + config(['tenancy.identification.central_domains' => ['site.com', 'blog.site.com']]); + + expect(DomainTenantResolver::isSubdomain('blog.site.com'))->toBeFalse(); + expect(DomainTenantResolver::isSubdomain('tenant.site.com'))->toBeTrue(); + expect(DomainTenantResolver::isSubdomain('tenantsite.com'))->toBeFalse(); +}); + class SubdomainTenant extends Models\Tenant { use HasDomains; From da7eb94c07791e6c86397dac506d0997709d4965 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 12 May 2026 23:59:21 +0200 Subject: [PATCH 2/2] Remove redundant universal route check from PreventAccess MW (#1427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PreventAcessFromUnwantedDomains MW had the `tenancy()->routeIsUniversal($route)` check either for returning early, or it was a leftover from some older implementation, so I removed it. The middleware aborts if the `$this->accessingTenantRouteFromCentralDomain($request, $route) || $this->accessingCentralRouteFromTenantDomain($request, $route)` check passes. Meaning, **for the middleware to abort, the route has to be either in central or tenant mode**. When the route is in universal mode, the middleware will never reach `return $abortRequest()`. `return $next($request)` will always get reached, even when the `|| tenancy()->routeIsUniversal($route)` check is deleted from the previous condition, so that check was basically useless. Since the docblock for the class does mention the behavior for universal routes explicitly, we've instead added a comment documenting that things work this way. That's probably the most reasonable way to have this explicit behavior for universal routes easily understandable in this fairly complex logic without redundant code. Resolves #1418 --------- Co-authored-by: Samuel Štancl --- src/Middleware/PreventAccessFromUnwantedDomains.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Middleware/PreventAccessFromUnwantedDomains.php b/src/Middleware/PreventAccessFromUnwantedDomains.php index cdfa3b2c..7f628583 100644 --- a/src/Middleware/PreventAccessFromUnwantedDomains.php +++ b/src/Middleware/PreventAccessFromUnwantedDomains.php @@ -31,10 +31,12 @@ class PreventAccessFromUnwantedDomains { $route = tenancy()->getRoute($request); - if ($this->shouldBeSkipped($route) || tenancy()->routeIsUniversal($route)) { + if ($this->shouldBeSkipped($route)) { return $next($request); } + // If the route is universal, neither of these checks will pass and the logic will + // fall through to the $next($request) call at the end. if ($this->accessingTenantRouteFromCentralDomain($request, $route) || $this->accessingCentralRouteFromTenantDomain($request, $route)) { $abortRequest = static::$abortRequest ?? function () { abort(404);