mirror of
https://github.com/archtechx/tenancy.git
synced 2026-06-20 22:54:05 +00:00
Correct DomainTenantResolver::isSubdomain() check (#1425)
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] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Samuel Štancl <samuel@archte.ch>
This commit is contained in:
parent
23b18c93a0
commit
ec06dcc52e
3 changed files with 24 additions and 2 deletions
|
|
@ -7,6 +7,7 @@ namespace Stancl\Tenancy\Resolvers;
|
||||||
use Illuminate\Database\Eloquent\Builder;
|
use Illuminate\Database\Eloquent\Builder;
|
||||||
use Illuminate\Database\Eloquent\Model;
|
use Illuminate\Database\Eloquent\Model;
|
||||||
use Illuminate\Database\Eloquent\Relations\Relation;
|
use Illuminate\Database\Eloquent\Relations\Relation;
|
||||||
|
use Illuminate\Support\Arr;
|
||||||
use Illuminate\Support\Str;
|
use Illuminate\Support\Str;
|
||||||
use Stancl\Tenancy\Contracts\Domain;
|
use Stancl\Tenancy\Contracts\Domain;
|
||||||
use Stancl\Tenancy\Contracts\SingleDomainTenant;
|
use Stancl\Tenancy\Contracts\SingleDomainTenant;
|
||||||
|
|
@ -58,7 +59,19 @@ class DomainTenantResolver extends Contracts\CachedTenantResolver
|
||||||
|
|
||||||
public static function isSubdomain(string $domain): bool
|
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
|
public function resolved(Tenant $tenant, mixed ...$args): void
|
||||||
|
|
|
||||||
|
|
@ -300,7 +300,7 @@ test('using different default route modes works with global domain identificatio
|
||||||
$exception = match ($middleware) {
|
$exception = match ($middleware) {
|
||||||
InitializeTenancyByDomain::class => TenantCouldNotBeIdentifiedOnDomainException::class,
|
InitializeTenancyByDomain::class => TenantCouldNotBeIdentifiedOnDomainException::class,
|
||||||
InitializeTenancyBySubdomain::class => NotASubdomainException::class,
|
InitializeTenancyBySubdomain::class => NotASubdomainException::class,
|
||||||
InitializeTenancyByDomainOrSubdomain::class => NotASubdomainException::class,
|
InitializeTenancyByDomainOrSubdomain::class => TenantCouldNotBeIdentifiedOnDomainException::class,
|
||||||
};
|
};
|
||||||
|
|
||||||
expect(fn () => $this->withoutExceptionHandling()->get('http://localhost/central-route'))->toThrow($exception);
|
expect(fn () => $this->withoutExceptionHandling()->get('http://localhost/central-route'))->toThrow($exception);
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ use Stancl\Tenancy\Database\Concerns\HasDomains;
|
||||||
use Stancl\Tenancy\Exceptions\NotASubdomainException;
|
use Stancl\Tenancy\Exceptions\NotASubdomainException;
|
||||||
use Stancl\Tenancy\Middleware\InitializeTenancyBySubdomain;
|
use Stancl\Tenancy\Middleware\InitializeTenancyBySubdomain;
|
||||||
use Stancl\Tenancy\Database\Models;
|
use Stancl\Tenancy\Database\Models;
|
||||||
|
use Stancl\Tenancy\Resolvers\DomainTenantResolver;
|
||||||
use function Stancl\Tenancy\Tests\pest;
|
use function Stancl\Tenancy\Tests\pest;
|
||||||
|
|
||||||
beforeEach(function () {
|
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');
|
->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
|
class SubdomainTenant extends Models\Tenant
|
||||||
{
|
{
|
||||||
use HasDomains;
|
use HasDomains;
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue