From 5f7fd38e5acd4355292a066e343d9b4d011c1db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 2 Jun 2025 03:43:47 +0200 Subject: [PATCH] [4.x] URL generation, request data identification improvements (#1357) * UrlGenerator: set defaults based on config; request data: move config to config file+resolver * Claude code adjustments * improve request data tests, simplify complex test in UrlGeneratorBootstrapperTest * url generator test: test changing tenant parameter name * request data identification: add tenant_model_column configuration * defaultParameterNames -> passQueryParameter * move comment * minor refactor in PathIdentificationTest, expand CLAUDE.md to include early identification section * Fix COLOR_FLAG * improve test name Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * TenancyUrlGenerator: add a check for queryParameterName being null Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix code style (php-cs-fixer) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] --- CLAUDE.md | 32 +++ assets/config.php | 9 +- .../Integrations/FortifyRouteBootstrapper.php | 33 ++- .../UrlGeneratorBootstrapper.php | 16 +- .../InitializeTenancyByRequestData.php | 38 ++- src/Overrides/TenancyUrlGenerator.php | 30 ++- src/Resolvers/RequestDataTenantResolver.php | 41 +++- t | 8 +- test | 8 +- .../FortifyRouteBootstrapperTest.php | 2 +- .../UrlGeneratorBootstrapperTest.php | 232 ++++++++++++++---- tests/PathIdentificationTest.php | 12 +- tests/RequestDataIdentificationTest.php | 105 +++++--- 13 files changed, 440 insertions(+), 126 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7061fcda..bb6dedac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ### Testing - `composer test` - Run tests without coverage using Docker +- `./test tests/TestFile.php` - Run an entire test file - `./t 'test name'` - Run a specific test +- You can append `-v` to get a full stack trace if a test fails due to an exception ### Code Quality - `composer phpstan` - Run PHPStan static analysis (level 8) @@ -75,6 +77,36 @@ All of these work as flags, i.e. middleware groups that are empty arrays with a - `universal` - Routes working in both contexts - `clone` - Tells route cloning logic to clone the route +### Early Identification + +**Early identification** ensures tenancy is initialized before controller instantiation, which is critical for certain scenarios. + +**When needed:** +- Controllers using constructor dependency injection +- Integration with packages that inject dependencies in constructors + +**The Problem:** +Laravel executes controller constructors and route model binding before route-level middleware runs, causing services to use central context instead of tenant context. + +**Solutions:** +1. **Avoid Constructor Injection** - Use method injection instead +2. **Laravel's Native Solution** - Use controllers that implement `HasMiddleware` interface +3. **Kernel Identification** - Add middleware to HTTP Kernel's global stack: + +```php +// In HttpKernel.php +protected $middleware = [ + \Stancl\Tenancy\Middleware\InitializeTenancyByDomain::class, + // other middleware... +]; +``` + +Note you also need to flag the route with the `'tenant'` middleware if default route mode (set in config) isn't set to TENANT. + +**Benefits:** +- Constructor dependency injection receives tenant-aware services +- Seamless integration with existing Laravel applications + ### Testing Environment Tests use Docker with MySQL/PostgreSQL/Redis. The `./test` script runs Pest tests inside containers with proper database isolation. diff --git a/assets/config.php b/assets/config.php index 9089974d..73becdee 100644 --- a/assets/config.php +++ b/assets/config.php @@ -119,7 +119,7 @@ return [ Resolvers\PathTenantResolver::class => [ 'tenant_parameter_name' => 'tenant', 'tenant_model_column' => null, // null = tenant key - 'tenant_route_name_prefix' => null, // null = 'tenant.' + 'tenant_route_name_prefix' => 'tenant.', 'allowed_extra_model_columns' => [], // used with binding route fields 'cache' => false, @@ -127,6 +127,13 @@ return [ 'cache_store' => null, // null = default ], Resolvers\RequestDataTenantResolver::class => [ + // Set any of these to null to disable that method of identification + 'header' => 'X-Tenant', + 'cookie' => 'tenant', + 'query_parameter' => 'tenant', + + 'tenant_model_column' => null, // null = tenant key + 'cache' => false, 'cache_ttl' => 3600, // seconds 'cache_store' => null, // null = default diff --git a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php index 05f3fa11..fb371d6a 100644 --- a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php +++ b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php @@ -8,6 +8,7 @@ use Illuminate\Config\Repository; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; /** * Allows customizing Fortify action redirects so that they can also redirect @@ -38,7 +39,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper * is handled in another way (TenancyUrlGenerator::$passTenantParameter for both, * UrlGeneratorBootstrapper:$addTenantParameterToDefaults for path identification). */ - public static bool $passTenantParameter = true; + public static bool $passTenantParameter = false; /** * Tenant route that serves as Fortify's home (e.g. a tenant dashboard route). @@ -47,12 +48,22 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper public static string|null $fortifyHome = 'tenant.dashboard'; /** - * Use default parameter names ('tenant' name and tenant key value) instead of the parameter name - * and column name configured in the path resolver config. + * Follow the query_parameter config instead of the tenant_parameter_name (path identification) config. * - * You want to enable this when using query string identification while having customized that config. + * This only has an effect when: + * - $passTenantParameter is enabled, and + * - the tenant_parameter_name config for the path resolver differs from the query_parameter config for the request data resolver. + * + * In such a case, instead of adding ['tenant' => '...'] to the route parameters (or whatever your tenant_parameter_name is if not 'tenant'), + * the query_parameter will be passed instead, e.g. ['team' => '...'] if your query_parameter config is 'team'. + * + * This is enabled by default because typically you will not need $passTenantParameter with path identification. + * UrlGeneratorBootstrapper::$addTenantParameterToDefaults is recommended instead when using path identification. + * + * On the other hand, when using request data identification (specifically query string) you WILL need to + * pass the parameter therefore you would use $passTenantParameter. */ - public static bool $defaultParameterNames = false; + public static bool $passQueryParameter = true; protected array $originalFortifyConfig = []; @@ -74,8 +85,14 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper protected function useTenantRoutesInFortify(Tenant $tenant): void { - $tenantParameterName = static::$defaultParameterNames ? 'tenant' : PathTenantResolver::tenantParameterName(); - $tenantParameterValue = static::$defaultParameterNames ? $tenant->getTenantKey() : PathTenantResolver::tenantParameterValue($tenant); + if (static::$passQueryParameter) { + // todo@tests + $tenantParameterName = RequestDataTenantResolver::queryParameterName(); + $tenantParameterValue = RequestDataTenantResolver::payloadValue($tenant); + } else { + $tenantParameterName = PathTenantResolver::tenantParameterName(); + $tenantParameterValue = PathTenantResolver::tenantParameterValue($tenant); + } $generateLink = function (string $redirect) use ($tenantParameterValue, $tenantParameterName) { return route($redirect, static::$passTenantParameter ? [$tenantParameterName => $tenantParameterValue] : []); @@ -89,7 +106,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper if (static::$fortifyHome) { // Generate the home route URL with the tenant parameter and make it the Fortify home route - $this->config->set('fortify.home', route(static::$fortifyHome, static::$passTenantParameter ? [$tenantParameterName => $tenantParameterValue] : [])); + $this->config->set('fortify.home', $generateLink(static::$fortifyHome)); } $this->config->set('fortify.redirects', $redirects); diff --git a/src/Bootstrappers/UrlGeneratorBootstrapper.php b/src/Bootstrappers/UrlGeneratorBootstrapper.php index b5289904..6c923d21 100644 --- a/src/Bootstrappers/UrlGeneratorBootstrapper.php +++ b/src/Bootstrappers/UrlGeneratorBootstrapper.php @@ -67,13 +67,15 @@ class UrlGeneratorBootstrapper implements TenancyBootstrapper $defaultParameters = $this->originalUrlGenerator->getDefaultParameters(); if (static::$addTenantParameterToDefaults) { - $defaultParameters = array_merge( - $defaultParameters, - [ - PathTenantResolver::tenantParameterName() => PathTenantResolver::tenantParameterValue($tenant), // path identification - 'tenant' => $tenant->getTenantKey(), // query string identification - ], - ); + $tenantParameterName = PathTenantResolver::tenantParameterName(); + + $defaultParameters = array_merge($defaultParameters, [ + $tenantParameterName => PathTenantResolver::tenantParameterValue($tenant), + ]); + + foreach (PathTenantResolver::allowedExtraModelColumns() as $column) { + $defaultParameters["$tenantParameterName:$column"] = $tenant->getAttribute($column); + } } $newGenerator->defaults($defaultParameters); diff --git a/src/Middleware/InitializeTenancyByRequestData.php b/src/Middleware/InitializeTenancyByRequestData.php index a4e8a9c2..d7a13e2c 100644 --- a/src/Middleware/InitializeTenancyByRequestData.php +++ b/src/Middleware/InitializeTenancyByRequestData.php @@ -18,9 +18,6 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware { use UsableWithEarlyIdentification; - public static string $header = 'X-Tenant'; - public static string $cookie = 'tenant'; - public static string $queryParameter = 'tenant'; public static ?Closure $onFail = null; public static bool $requireCookieEncryption = false; @@ -54,18 +51,19 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware protected function getPayload(Request $request): string|null { - if (static::$header && $request->hasHeader(static::$header)) { - $payload = $request->header(static::$header); - } elseif ( - static::$queryParameter && - $request->has(static::$queryParameter) - ) { - $payload = $request->get(static::$queryParameter); - } elseif (static::$cookie && $request->hasCookie(static::$cookie)) { - $payload = $request->cookie(static::$cookie); + $headerName = RequestDataTenantResolver::headerName(); + $queryParameterName = RequestDataTenantResolver::queryParameterName(); + $cookieName = RequestDataTenantResolver::cookieName(); + + if ($headerName && $request->hasHeader($headerName)) { + $payload = $request->header($headerName); + } elseif ($queryParameterName && $request->has($queryParameterName)) { + $payload = $request->get($queryParameterName); + } elseif ($cookieName && $request->hasCookie($cookieName)) { + $payload = $request->cookie($cookieName); if ($payload && is_string($payload)) { - $payload = $this->getTenantFromCookie($payload); + $payload = $this->getTenantFromCookie($cookieName, $payload); } } else { $payload = null; @@ -86,12 +84,12 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware return (bool) $this->getPayload($request); } - protected function getTenantFromCookie(string $cookie): string|null + protected function getTenantFromCookie(string $cookieName, string $cookieValue): string|null { // If the cookie looks like it's encrypted, we try decrypting it - if (str_starts_with($cookie, 'eyJpdiI')) { + if (str_starts_with($cookieValue, 'eyJpdiI')) { try { - $json = base64_decode($cookie); + $json = base64_decode($cookieValue); $data = json_decode($json, true); if ( @@ -100,9 +98,9 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware ) { // We can confidently assert that the cookie is encrypted. If this call were to fail, this method would just // return null and the cookie payload would get skipped. - $cookie = CookieValuePrefix::validate( - static::$cookie, - Crypt::decryptString($cookie), + $cookieValue = CookieValuePrefix::validate( + $cookieName, + Crypt::decryptString($cookieValue), Crypt::getAllKeys() ); } @@ -113,6 +111,6 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware return null; } - return $cookie; + return $cookieValue; } } diff --git a/src/Overrides/TenancyUrlGenerator.php b/src/Overrides/TenancyUrlGenerator.php index 4c6120a8..ed14d5b5 100644 --- a/src/Overrides/TenancyUrlGenerator.php +++ b/src/Overrides/TenancyUrlGenerator.php @@ -9,6 +9,7 @@ use Illuminate\Routing\UrlGenerator; use Illuminate\Support\Arr; use InvalidArgumentException; use Stancl\Tenancy\Resolvers\PathTenantResolver; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; /** * This class is used in place of the default UrlGenerator when UrlGeneratorBootstrapper is enabled. @@ -86,12 +87,22 @@ class TenancyUrlGenerator extends UrlGenerator public static array $overrides = []; /** - * Use default parameter names ('tenant' name and tenant key value) instead of the parameter name - * and column name configured in the path resolver config. + * Follow the query_parameter config instead of the tenant_parameter_name (path identification) config. * - * You want to enable this when using query string identification while having customized that config. + * This only has an effect when: + * - $passTenantParameterToRoutes is enabled, and + * - the tenant_parameter_name config for the path resolver differs from the query_parameter config for the request data resolver. + * + * In such a case, instead of adding ['tenant' => '...'] to the route parameters (or whatever your tenant_parameter_name is if not 'tenant'), + * the query_parameter will be passed instead, e.g. ['team' => '...'] if your query_parameter config is 'team'. + * + * This is enabled by default because typically you will not need $passTenantParameterToRoutes with path identification. + * UrlGeneratorBootstrapper::$addTenantParameterToDefaults is recommended instead when using path identification. + * + * On the other hand, when using request data identification (specifically query string) you WILL need to pass the parameter + * directly to route() calls, therefore you would use $passTenantParameterToRoutes to avoid having to do that manually. */ - public static bool $defaultParameterNames = false; + public static bool $passQueryParameter = true; /** * Override the route() method so that the route name gets prefixed @@ -175,11 +186,14 @@ class TenancyUrlGenerator extends UrlGenerator protected function addTenantParameter(array $parameters): array { if (tenant() && static::$passTenantParameterToRoutes) { - if (static::$defaultParameterNames) { - return array_merge($parameters, ['tenant' => tenant()->getTenantKey()]); - } else { - return array_merge($parameters, [PathTenantResolver::tenantParameterName() => PathTenantResolver::tenantParameterValue(tenant())]); + if (static::$passQueryParameter) { + $queryParameterName = RequestDataTenantResolver::queryParameterName(); + if ($queryParameterName !== null) { + return array_merge($parameters, [$queryParameterName => RequestDataTenantResolver::payloadValue(tenant())]); + } } + + return array_merge($parameters, [PathTenantResolver::tenantParameterName() => PathTenantResolver::tenantParameterValue(tenant())]); } else { return $parameters; } diff --git a/src/Resolvers/RequestDataTenantResolver.php b/src/Resolvers/RequestDataTenantResolver.php index 7ebc90ab..4d8b3277 100644 --- a/src/Resolvers/RequestDataTenantResolver.php +++ b/src/Resolvers/RequestDataTenantResolver.php @@ -20,7 +20,9 @@ class RequestDataTenantResolver extends Contracts\CachedTenantResolver { $payload = (string) $args[0]; - if ($payload && $tenant = tenancy()->find($payload, withRelations: true)) { + $column = static::tenantModelColumn(); + + if ($payload && $tenant = tenancy()->find($payload, $column, withRelations: true)) { return $tenant; } @@ -29,8 +31,43 @@ class RequestDataTenantResolver extends Contracts\CachedTenantResolver public function getPossibleCacheKeys(Tenant&Model $tenant): array { + // todo@tests return [ - $this->formatCacheKey($tenant->getTenantKey()), + $this->formatCacheKey(static::payloadValue($tenant)), ]; } + + public static function payloadValue(Tenant $tenant): string + { + return $tenant->getAttribute(static::tenantModelColumn()); + } + + public static function tenantModelColumn(): string + { + return config('tenancy.identification.resolvers.' . static::class . '.tenant_model_column') ?? tenancy()->model()->getTenantKeyName(); + } + + /** + * Returns the name of the header used for identification, or null if header identification is disabled. + */ + public static function headerName(): string|null + { + return config('tenancy.identification.resolvers.' . static::class . '.header'); + } + + /** + * Returns the name of the query parameter used for identification, or null if query parameter identification is disabled. + */ + public static function queryParameterName(): string|null + { + return config('tenancy.identification.resolvers.' . static::class . '.query_parameter'); + } + + /** + * Returns the name of the cookie used for identification, or null if cookie identification is disabled. + */ + public static function cookieName(): string|null + { + return config('tenancy.identification.resolvers.' . static::class . '.cookie'); + } } diff --git a/t b/t index 4fd5931c..36d2d391 100755 --- a/t +++ b/t @@ -1,3 +1,9 @@ #!/bin/bash -docker compose exec -e COLUMNS=$(tput cols) -T test vendor/bin/pest --color=always --no-coverage --filter "$@" +if [[ "${CLAUDECODE}" != "1" ]]; then + COLOR_FLAG="--colors=always" +else + COLOR_FLAG="--colors=never" +fi + +docker compose exec -e COLUMNS=$(tput cols) -T test vendor/bin/pest ${COLOR_FLAG} --no-coverage --filter "$@" diff --git a/test b/test index b8bd8fa0..0df8f63e 100755 --- a/test +++ b/test @@ -1,4 +1,10 @@ #!/bin/bash +if [[ "${CLAUDECODE}" != "1" ]]; then + COLOR_FLAG="--colors=always" +else + COLOR_FLAG="--colors=never" +fi + # --columns doesn't seem to work at the moment, so we're setting it using an environment variable -docker compose exec -e COLUMNS=$(tput cols) -T test vendor/bin/pest --colors=always "$@" +docker compose exec -e COLUMNS=$(tput cols) -T test vendor/bin/pest ${COLOR_FLAG} "$@" diff --git a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php index 1942a1c5..63f0f2a0 100644 --- a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php +++ b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php @@ -20,7 +20,7 @@ afterEach(function () { FortifyRouteBootstrapper::$passTenantParameter = true; FortifyRouteBootstrapper::$fortifyRedirectMap = []; FortifyRouteBootstrapper::$fortifyHome = 'tenant.dashboard'; - FortifyRouteBootstrapper::$defaultParameterNames = false; + FortifyRouteBootstrapper::$passQueryParameter = false; }); test('fortify route tenancy bootstrapper updates fortify config correctly', function() { diff --git a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php index 77d50073..39fcc475 100644 --- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php +++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php @@ -1,5 +1,6 @@ route('home'))->name('home'); - // Tenant route name prefix is 'tenant.' by default - Route::get('/tenant/home', fn () => route('tenant.home'))->name('tenant.home'); + config([ + 'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_route_name_prefix' => 'custom_prefix.', + ]); + + Route::get('/central/home', fn () => '')->name('home'); + Route::get('/tenant/home', fn () => '')->name('custom_prefix.home'); $tenant = Tenant::create(); - $centralRouteUrl = route('home'); - $tenantRouteUrl = route('tenant.home'); config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); tenancy()->initialize($tenant); // Route names don't get prefixed when TenancyUrlGenerator::$prefixRouteNames is false (default) - expect(route('home'))->toBe($centralRouteUrl); + expect(route('home'))->toBe('http://localhost/central/home'); - // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed with 'tenant.' automatically. + // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed automatically. TenancyUrlGenerator::$prefixRouteNames = true; - expect(route('home'))->toBe($tenantRouteUrl); + expect(route('home'))->toBe('http://localhost/tenant/home'); - // The 'tenant.home' route name doesn't get prefixed -- it is already prefixed with 'tenant.' - expect(route('tenant.home'))->toBe($tenantRouteUrl); + // The 'custom_prefix.home' route name doesn't get prefixed -- it is already prefixed with 'custom_prefix.' + expect(route('custom_prefix.home'))->toBe('http://localhost/tenant/home'); // Ending tenancy reverts route() behavior changes tenancy()->end(); - expect(route('home'))->toBe($centralRouteUrl); + expect(route('home'))->toBe('http://localhost/central/home'); }); -test('the route helper can receive the tenant parameter automatically', function ( - string $identification, - bool $addTenantParameterToDefaults, - bool $passTenantParameterToRoutes, -) { +test('path identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) { config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); - $appUrl = config('app.url'); - UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults; - - // When the tenant parameter isn't added to defaults, the tenant parameter has to be passed "manually" - // by setting $passTenantParameterToRoutes to true. This is only preferable with query string identification. - // With path identification, this ultimately doesn't have any effect - // if UrlGeneratorBootstrapper::$addTenantParameterToDefaults is true, - // but TenancyUrlGenerator::$passTenantParameterToRoutes can still be used instead. TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes; $tenant = Tenant::create(); - $tenantKey = $tenant->getTenantKey(); - Route::get('/central/home', fn () => route('home'))->name('home'); - - $tenantRoute = $identification === InitializeTenancyByPath::class ? "/{tenant}/home" : "/tenant/home"; - - Route::get($tenantRoute, fn () => route('tenant.home')) + Route::get('/{tenant}/home', fn () => tenant('id')) ->name('tenant.home') - ->middleware(['tenant', $identification]); + ->middleware([InitializeTenancyByPath::class]); tenancy()->initialize($tenant); - $expectedUrl = match (true) { - $identification === InitializeTenancyByRequestData::class && $passTenantParameterToRoutes => "{$appUrl}/tenant/home?tenant={$tenantKey}", - $identification === InitializeTenancyByRequestData::class => "{$appUrl}/tenant/home", // $passTenantParameterToRoutes is false - $identification === InitializeTenancyByPath::class && ($addTenantParameterToDefaults || $passTenantParameterToRoutes) => "{$appUrl}/{$tenantKey}/home", - $identification === InitializeTenancyByPath::class => null, // Should throw an exception -- route() doesn't receive the tenant parameter in this case - }; - - if ($expectedUrl === null) { + if (! $addTenantParameterToDefaults && ! $passTenantParameterToRoutes) { expect(fn () => route('tenant.home'))->toThrow(UrlGenerationException::class, 'Missing parameter: tenant'); } else { - expect(route('tenant.home'))->toBe($expectedUrl); + // If at least *one* of the approaches was used, the parameter will make its way to the route + expect(route('tenant.home'))->toBe("http://localhost/{$tenant->id}/home"); + pest()->get(route('tenant.home'))->assertSee($tenant->id); } -})->with([InitializeTenancyByPath::class, InitializeTenancyByRequestData::class]) - ->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults +})->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes +test('request data identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) { + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + + UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults; + TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes; + + $tenant = Tenant::create(); + + Route::get('/tenant/home', fn () => tenant('id')) + ->name('tenant.home') + ->middleware([InitializeTenancyByRequestData::class]); + + tenancy()->initialize($tenant); + + if ($passTenantParameterToRoutes) { + // Only $passTenantParameterToRoutes has an effect, defaults do not affect request data URL generation + expect(route('tenant.home'))->toBe("http://localhost/tenant/home?tenant={$tenant->id}"); + pest()->get(route('tenant.home'))->assertSee($tenant->id); + } else { + expect(route('tenant.home'))->toBe("http://localhost/tenant/home"); + expect(fn () => $this->withoutExceptionHandling()->get(route('tenant.home')))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); + } +})->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults + ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes + +test('changing request data query parameter and model column is respected by the url generator', function () { + config([ + 'tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class], + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => 'team', + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => 'slug', + ]); + + Tenant::$extraCustomColumns = ['slug']; + + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->unique(); + }); + + TenancyUrlGenerator::$passTenantParameterToRoutes = true; + + $tenant = Tenant::create(['slug' => 'acme']); + + Route::get('/tenant/home', fn () => tenant('id')) + ->name('tenant.home') + ->middleware([InitializeTenancyByRequestData::class]); + + tenancy()->initialize($tenant); + + expect(route('tenant.home'))->toBe("http://localhost/tenant/home?team=acme"); + pest()->get(route('tenant.home'))->assertSee($tenant->id); +}); + +test('setting extra model columns sets additional URL defaults', function () { + Tenant::$extraCustomColumns = ['slug']; + TenancyUrlGenerator::$passTenantParameterToRoutes = false; + UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true; + + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug']]); + + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->unique(); + }); + + Route::get('/{tenant}/foo/{user}', function (string $user) { + return tenant()->getTenantKey() . " $user"; + })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo'); + + Route::get('/{tenant:slug}/fooslug/{user}', function (string $user) { + return tenant()->getTenantKey() . " $user"; + })->middleware([InitializeTenancyByPath::class, 'web'])->name('fooslug'); + + $tenant = Tenant::create(['slug' => 'acme']); + + // In central context, no URL defaults are applied + expect(route('foo', [$tenant->getTenantKey(), 'bar']))->toBe("http://localhost/{$tenant->getTenantKey()}/foo/bar"); + pest()->get(route('foo', [$tenant->getTenantKey(), 'bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + tenancy()->end(); + + expect(route('fooslug', ['acme', 'bar']))->toBe('http://localhost/acme/fooslug/bar'); + pest()->get(route('fooslug', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + tenancy()->end(); + + // In tenant context, URL defaults are applied + tenancy()->initialize($tenant); + expect(route('foo', ['bar']))->toBe("http://localhost/{$tenant->getTenantKey()}/foo/bar"); + pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + + expect(route('fooslug', ['bar']))->toBe('http://localhost/acme/fooslug/bar'); + pest()->get(route('fooslug', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar'); +}); + +test('changing the tenant model column changes the default value for the tenant parameter', function () { + Tenant::$extraCustomColumns = ['slug']; + TenancyUrlGenerator::$passTenantParameterToRoutes = false; + UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true; + + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'slug']); + + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->unique(); + }); + + Route::get('/{tenant}/foo/{user}', function (string $user) { + return tenant()->getTenantKey() . " $user"; + })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo'); + + $tenant = Tenant::create(['slug' => 'acme']); + + // In central context, no URL defaults are applied + expect(route('foo', ['acme', 'bar']))->toBe("http://localhost/acme/foo/bar"); + pest()->get(route('foo', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + tenancy()->end(); + + // In tenant context, URL defaults are applied + tenancy()->initialize($tenant); + expect(route('foo', ['bar']))->toBe("http://localhost/acme/foo/bar"); + pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar'); +}); + +test('changing the tenant parameter name is respected by the url generator', function () { + Tenant::$extraCustomColumns = ['slug', 'slug2']; + TenancyUrlGenerator::$passTenantParameterToRoutes = false; + UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true; + + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team']); + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'slug']); + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug2']]); + + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->unique(); + $table->string('slug2')->unique(); + }); + + Route::get('/{team}/foo/{user}', function (string $user) { + return tenant()->getTenantKey() . " $user"; + })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo'); + + Route::get('/{team:slug2}/fooslug2/{user}', function (string $user) { + return tenant()->getTenantKey() . " $user"; + })->middleware([InitializeTenancyByPath::class, 'web'])->name('fooslug2'); + + $tenant = Tenant::create(['slug' => 'acme', 'slug2' => 'acme2']); + + // In central context, no URL defaults are applied + expect(route('foo', ['acme', 'bar']))->toBe("http://localhost/acme/foo/bar"); + pest()->get(route('foo', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + tenancy()->end(); + + expect(route('fooslug2', ['acme2', 'bar']))->toBe("http://localhost/acme2/fooslug2/bar"); + pest()->get(route('fooslug2', ['acme2', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + tenancy()->end(); + + // In tenant context, URL defaults are applied + tenancy()->initialize($tenant); + expect(route('foo', ['bar']))->toBe("http://localhost/acme/foo/bar"); + pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar'); + + expect(route('fooslug2', ['bar']))->toBe("http://localhost/acme2/fooslug2/bar"); + pest()->get(route('fooslug2', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar'); +}); + test('url generator can override specific route names', function() { config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); diff --git a/tests/PathIdentificationTest.php b/tests/PathIdentificationTest.php index 1df74092..79cd3816 100644 --- a/tests/PathIdentificationTest.php +++ b/tests/PathIdentificationTest.php @@ -35,6 +35,11 @@ beforeEach(function () { }); }); +afterEach(function () { + InitializeTenancyByPath::$onFail = null; + Tenant::$extraCustomColumns = []; +}); + test('tenant can be identified by path', function () { Tenant::create([ 'id' => 'acme', @@ -150,6 +155,7 @@ test('central route can have a parameter with the same name as the tenant parame config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team']); $tenantKey = Tenant::create()->getTenantKey(); + // The route is flagged as central (while using kernel identification) so the {team} parameter should not be used for tenancy initialization Route::get('/central/route/{team}/{a}/{b}', function ($team, $a, $b) { return "$a + $b + $team"; })->middleware('central')->name('central-route'); @@ -185,8 +191,6 @@ test('the tenant model column can be customized in the config', function () { $this->withoutExceptionHandling(); pest()->get('/acme/foo')->assertSee($tenant->getTenantKey()); expect(fn () => pest()->get($tenant->id . '/foo'))->toThrow(TenantCouldNotBeIdentifiedByPathException::class); - - Tenant::$extraCustomColumns = []; // static property reset }); test('the tenant model column can be customized in the route definition', function () { @@ -218,8 +222,6 @@ test('the tenant model column can be customized in the route definition', functi // Binding field defined pest()->get('/acme/bar')->assertSee($tenant->getTenantKey()); expect(fn () => pest()->get($tenant->id . '/bar'))->toThrow(TenantCouldNotBeIdentifiedByPathException::class); - - Tenant::$extraCustomColumns = []; // static property reset }); test('any extra model column needs to be whitelisted', function () { @@ -243,6 +245,4 @@ test('any extra model column needs to be whitelisted', function () { // After whitelisting the column it works config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug']]); pest()->get('/acme/foo')->assertSee($tenant->getTenantKey()); - - Tenant::$extraCustomColumns = []; // static property reset }); diff --git a/tests/RequestDataIdentificationTest.php b/tests/RequestDataIdentificationTest.php index f04d99a7..0f635bf1 100644 --- a/tests/RequestDataIdentificationTest.php +++ b/tests/RequestDataIdentificationTest.php @@ -2,9 +2,11 @@ declare(strict_types=1); +use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Route; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByRequestDataException; use Stancl\Tenancy\Middleware\InitializeTenancyByRequestData; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use Stancl\Tenancy\Tests\Etc\Tenant; use function Stancl\Tenancy\Tests\pest; @@ -15,45 +17,90 @@ beforeEach(function () { ], ]); - InitializeTenancyByRequestData::$header = 'X-Tenant'; - InitializeTenancyByRequestData::$cookie = 'X-Tenant'; - InitializeTenancyByRequestData::$queryParameter = 'tenant'; - - Route::middleware(['tenant', InitializeTenancyByRequestData::class])->get('/test', function () { + Route::middleware([InitializeTenancyByRequestData::class])->get('/test', function () { return 'Tenant id: ' . tenant('id'); }); }); -test('header identification works', function () { - $tenant = Tenant::create(); +test('header identification works', function (string|null $tenantModelColumn) { + if ($tenantModelColumn) { + Schema::table('tenants', function (Blueprint $table) use ($tenantModelColumn) { + $table->string($tenantModelColumn)->unique(); + }); + Tenant::$extraCustomColumns = [$tenantModelColumn]; + } - $this - ->withoutExceptionHandling() - ->withHeader('X-Tenant', $tenant->id) - ->get('test') - ->assertSee($tenant->id); -}); + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); -test('query parameter identification works', function () { - $tenant = Tenant::create(); + $tenant = Tenant::create($tenantModelColumn ? [$tenantModelColumn => 'acme'] : []); + $payload = $tenantModelColumn ? 'acme' : $tenant->id; - $this - ->withoutExceptionHandling() - ->get('test?tenant=' . $tenant->id) - ->assertSee($tenant->id); -}); + // Default header name + $this->withoutExceptionHandling()->withHeader('X-Tenant', $payload)->get('test')->assertSee($tenant->id); -test('cookie identification works', function () { - $tenant = Tenant::create(); + // Custom header name + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.header' => 'X-Custom-Tenant']); + $this->withoutExceptionHandling()->withHeader('X-Custom-Tenant', $payload)->get('test')->assertSee($tenant->id); - $this - ->withoutExceptionHandling() - ->withUnencryptedCookie('X-Tenant', $tenant->id) - ->get('test') - ->assertSee($tenant->id); -}); + // Setting the header to null disables header identification + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.header' => null]); + expect(fn () => $this->withoutExceptionHandling()->withHeader('X-Tenant', $payload)->get('test'))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); +})->with([null, 'slug']); -test('middleware throws exception when tenant data is not provided in the request', function () { +test('query parameter identification works', function (string|null $tenantModelColumn) { + if ($tenantModelColumn) { + Schema::table('tenants', function (Blueprint $table) use ($tenantModelColumn) { + $table->string($tenantModelColumn)->unique(); + }); + Tenant::$extraCustomColumns = [$tenantModelColumn]; + } + + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + + $tenant = Tenant::create($tenantModelColumn ? [$tenantModelColumn => 'acme'] : []); + $payload = $tenantModelColumn ? 'acme' : $tenant->id; + + // Default query parameter name + $this->withoutExceptionHandling()->get('test?tenant=' . $payload)->assertSee($tenant->id); + + // Custom query parameter name + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => 'custom_tenant']); + $this->withoutExceptionHandling()->get('test?custom_tenant=' . $payload)->assertSee($tenant->id); + + // Setting the query parameter to null disables query parameter identification + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => null]); + expect(fn () => $this->withoutExceptionHandling()->get('test?tenant=' . $payload))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); +})->with([null, 'slug']); + +test('cookie identification works', function (string|null $tenantModelColumn) { + if ($tenantModelColumn) { + Schema::table('tenants', function (Blueprint $table) use ($tenantModelColumn) { + $table->string($tenantModelColumn)->unique(); + }); + Tenant::$extraCustomColumns = [$tenantModelColumn]; + } + + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + + $tenant = Tenant::create($tenantModelColumn ? [$tenantModelColumn => 'acme'] : []); + $payload = $tenantModelColumn ? 'acme' : $tenant->id; + + // Default cookie name + $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test')->assertSee($tenant->id); + + // Custom cookie name + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.cookie' => 'custom_tenant_id']); + $this->withoutExceptionHandling()->withUnencryptedCookie('custom_tenant_id', $payload)->get('test')->assertSee($tenant->id); + + // Setting the cookie to null disables cookie identification + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.cookie' => null]); + expect(fn () => $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test'))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); +})->with([null, 'slug']); + +// todo@tests encrypted cookie + +test('an exception is thrown when no tenant data is provided in the request', function () { pest()->expectException(TenantCouldNotBeIdentifiedByRequestDataException::class); $this->withoutExceptionHandling()->get('test'); }); +