diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 0a864fbd..1574ea4b 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -8,12 +8,16 @@ use Exception; use Illuminate\Foundation\Application; use Illuminate\Session\FileSessionHandler; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Str; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; class FilesystemTenancyBootstrapper implements TenancyBootstrapper { public array $originalDisks = []; + protected array $originalCachePaths = []; + protected array $originalCacheLockPaths = []; + protected string|null $originalSessionPath = null; public string|null $originalAssetUrl; public string $originalStoragePath; @@ -94,7 +98,7 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper protected function tenantStoragePath(string $suffix): string { - return $this->originalStoragePath . "/{$suffix}"; + return rtrim($this->originalStoragePath, '/\\') . DIRECTORY_SEPARATOR . $suffix; } protected function assetHelper(string|false $suffix): void @@ -161,7 +165,7 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper // This is executed if the disk is in tenancy.filesystem.disks but does NOT have a root_override // This behavior is used for disks like S3. $newRoot = $originalRoot - ? rtrim($originalRoot, '/') . '/' . $suffix + ? rtrim($originalRoot, '/\\') . '/' . $suffix : $suffix; } @@ -191,29 +195,88 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper return; } - $storagePath = $suffix - ? $this->tenantStoragePath($suffix) - : $this->originalStoragePath; - - $stores = array_filter($this->app['config']['tenancy.cache.stores'], function ($name) { - $store = $this->app['config']["cache.stores.{$name}"]; - - if ($store === null) { - return false; - } - - return $store['driver'] === 'file'; - }); + $stores = $suffix !== false + ? $this->app['config']['tenancy.cache.stores'] + : array_keys($this->originalCachePaths); foreach ($stores as $name) { - $path = $storagePath . '/framework/cache/data'; + $store = $this->app['config']["cache.stores.{$name}"]; + + // Only file stores have a path to scope. Skip stores that don't exist (null) or use another driver. + if ($store === null || $store['driver'] !== 'file') { + continue; + } + + if ($suffix !== false && ! isset($this->originalCachePaths[$name])) { + $this->originalCachePaths[$name] = $store['path']; + $this->originalCacheLockPaths[$name] = $store['lock_path'] ?? null; + } + + $path = $suffix ? $this->tenantScopedPath($this->originalCachePaths[$name], $suffix) : $this->originalCachePaths[$name]; + + // Unlike path, lock_path is optional -- if it's not set, FileStore::lock() falls back to path + // itself (see `$this->lockDirectory ?? $this->directory` in FileStore). Leave it null here rather + // than hardcoding it to $path ourselves, so a store that didn't configure a separate lock_path + // doesn't end up with one. + $lockPath = $this->originalCacheLockPaths[$name]; + if ($suffix && $lockPath !== null) { + $lockPath = $this->tenantScopedPath($lockPath, $suffix); + } + $this->app['config']["cache.stores.{$name}.path"] = $path; - $this->app['config']["cache.stores.{$name}.lock_path"] = $path; + $this->app['config']["cache.stores.{$name}.lock_path"] = $lockPath; /** @var \Illuminate\Cache\FileStore $store */ $store = $this->app['cache']->store($name)->getStore(); $store->setDirectory($path); - $store->setLockDirectory($path); + $store->setLockDirectory($lockPath); + } + } + + /** + * Scope a configured path (a cache store's path or lock_path, or the session path) + * to the tenant identified by $suffix. + */ + protected function tenantScopedPath(string $configuredPath, string $suffix): string + { + $configuredPath = $this->normalizePath($configuredPath); + $storagePath = $this->normalizePath($this->originalStoragePath); + + if (str_starts_with($configuredPath, $storagePath . DIRECTORY_SEPARATOR)) { + // Swap the central storage path prefix for the tenant's. + // For example, storage_path('framework/cache/data') becomes storage_path('tenant1/framework/cache/data'). + return str($configuredPath) + ->replaceFirst($storagePath, $this->tenantStoragePath($suffix)) + ->toString(); + } + + // Otherwise $configuredPath isn't necessarily storage_path()-based, so just append the + // suffix as a subdirectory, e.g. '/var/cache/foo' becomes '/var/cache/foo/tenant1'. + return $configuredPath . DIRECTORY_SEPARATOR . $suffix; + } + + /** + * Normalize the path to use the separator of the current OS. + * + * The separators are also deduplicated, with two exceptions: + * - if the path begins with \\ on Windows (i.e. a UNC path), the *leading* separators won't be deduplicated + * - if the path contains non-UTF-8 characters, the separators won't be deduplicated since Str::deduplicate() only supports UTF-8 strings) + */ + protected function normalizePath(string $path): string + { + $path = str_replace('/', DIRECTORY_SEPARATOR, $path); + + $uncPrefix = DIRECTORY_SEPARATOR === '\\' && str_starts_with($path, '\\\\') ? DIRECTORY_SEPARATOR : ''; + + if ($deduplicated = Str::deduplicate($path, DIRECTORY_SEPARATOR)) { + // On Windows, a path starting with two separators is a UNC path (e.g. '\\server\share'), + // so the leading separator that got collapsed by deduplicate() should be added back + // (only one \ will be kept, so we use one for the prefix). + return $uncPrefix . rtrim($deduplicated, DIRECTORY_SEPARATOR); + } else { + // Because deduplicate() only supports UTF-8 paths, paths with non-UTF-8 characters will not + // be deduplicated since deduplicate() returns an empty result with unsupported strings + return rtrim($path, DIRECTORY_SEPARATOR); } } @@ -223,12 +286,15 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper return; } + $originalPath = $this->originalSessionPath ?? $this->app['config']['session.files']; + $this->originalSessionPath = $originalPath; + $path = $suffix - ? $this->tenantStoragePath($suffix) . '/framework/sessions' - : $this->originalStoragePath . '/framework/sessions'; + ? $this->tenantScopedPath($originalPath, $suffix) + : $originalPath; if (! is_dir($path)) { - // Create tenant framework/sessions directory if it does not exist. + // Create tenant session directory if it does not exist. // We ignore errors due to TOCTOU race conditions, instead we check for success below. @mkdir($path, 0750, true); diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 4e834917..785ffbc3 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -1,6 +1,7 @@ toBe('central2'); expect(file_get_contents(storage_path() . "/tenant{$tenant->id}/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); }); + +test('file cache stores get their paths scoped on bootstrap and restored back on revert', function () { + $fooPath = storage_path('framework/cache/foo_file'); + $barPath = storage_path('framework/cache/bar_file'); + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); + + // Use separate 'foo_file'/'bar_file' stores rather than reconfiguring 'file'. + // TestCase::setUp() calls `cache:clear file`, which resolves the 'file' store and leaves it + // in the CacheManager with the default path. A later config() call can't mutate that, + // so in central context, cache would be written to 'storage/framework/cache/data' no matter how the config changes. + // The same applies to the other tests below that configure separate cache stores rather than using 'file'. + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file', 'bar_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + 'cache.stores.bar_file' => [ + 'driver' => 'file', + 'path' => $barPath, + ], + ]); + + Cache::store('foo_file')->put('key', 'central foo'); + Cache::store('bar_file')->put('key', 'central bar'); + + tenancy()->initialize(Tenant::create()); + + Cache::store('foo_file')->put('key', 'tenant foo'); + Cache::store('bar_file')->put('key', 'tenant bar'); + + // Each store uses its own scoped path. + // The stores don't read or overwrite each other's entries. + expect(Cache::store('foo_file')->get('key'))->toBe('tenant foo'); + expect(Cache::store('bar_file')->get('key'))->toBe('tenant bar'); + + Cache::store('bar_file')->flush(); + + // Only bar_file was flushed + expect(Cache::store('foo_file')->get('key'))->toBe('tenant foo'); + expect(Cache::store('bar_file')->get('key'))->toBeNull(); + + tenancy()->end(); + + // revert() points each store back at its original configured path + expect(Cache::store('foo_file')->get('key'))->toBe('central foo'); + expect(Cache::store('bar_file')->get('key'))->toBe('central bar'); +}); + +test('only file driver cache stores get scoped', function () { + $centralStoragePath = storage_path(); + $fooPath = storage_path('framework/cache/foo_file'); + File::deleteDirectory($fooPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + // Only stores that use the 'file' driver are scoped. + // The 'redis' store won't be scoped since it doesn't use the 'file' driver, + // and 'nonexistent_store' won't be scoped since it just doesn't exist. + 'tenancy.cache.stores' => ['foo_file', 'redis', 'nonexistent_store'], + ]); + + Cache::store('redis')->put('key', 'central'); + + tenancy()->initialize($tenant = Tenant::create()); + + // Only the file store's path gets scoped + expect(config('cache.stores.foo_file.path'))->toBe("{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo_file"); + expect(config('cache.stores.redis'))->not()->toHaveKey('path'); + expect(config('cache.stores.nonexistent_store'))->toBeNull(); + + // The 'redis' store doesn't use the file driver (no path to scope), + // so FilesystemTenancyBootstrapper skips it in scopeCache(). + // The central value is retained in the tenant context. + expect(Cache::store('redis')->get('key'))->toBe('central'); + + tenancy()->end(); +}); + +test('cache scoping can be toggled using the scope_cache config', function (bool $scopeCache) { + $fooPath = storage_path('framework/cache/foo_file'); + File::deleteDirectory($fooPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + 'tenancy.filesystem.scope_cache' => $scopeCache, + ]); + + Cache::store('foo_file')->put('key', 'central'); + + tenancy()->initialize(Tenant::create()); + + if ($scopeCache) { + expect(config('cache.stores.foo_file.path'))->not()->toBe($fooPath); + expect(Cache::store('foo_file')->get('key'))->toBe(null); + } else { + // The store keeps using its central path, so the cache is shared between contexts + expect(config('cache.stores.foo_file.path'))->toBe($fooPath); + expect(Cache::store('foo_file')->get('key'))->toBe('central'); + } + + Cache::store('foo_file')->put('key', 'written in tenant context'); + + tenancy()->end(); + + if ($scopeCache) { + expect(Cache::store('foo_file')->get('key'))->toBe('central'); + } else { + expect(Cache::store('foo_file')->get('key'))->toBe('written in tenant context'); + } +})->with([true, false]); + +test('scopeCache ignores changes to tenancy.cache.stores made in tenant context', function () { + $fooPath = storage_path('framework/cache/foo_file'); + $barPath = storage_path('framework/cache/bar_file'); + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + 'cache.stores.bar_file' => [ + 'driver' => 'file', + 'path' => $barPath, + ], + // Only foo_file is scoped during bootstrap() + 'tenancy.cache.stores' => ['foo_file'], + ]); + + Cache::store('foo_file')->put('key', 'central'); + Cache::store('bar_file')->put('key', 'central'); + + tenancy()->initialize(Tenant::create()); + + Cache::store('foo_file')->put('key', 'tenant'); + + // Mutate tenancy.cache.stores in tenant context (remove 'foo_file', add 'bar_file') + config(['tenancy.cache.stores' => ['bar_file']]); + // 'bar_file' wasn't in tenancy.cache.stores during bootstrap. + // No original path is captured for that store, so it continues to use its central path. + expect(Cache::store('bar_file')->get('key'))->toBe('central'); + Cache::store('bar_file')->put('key', 'written in tenant context'); + + tenancy()->end(); + + // revert() restores the paths for the stores that were scoped during bootstrap + expect(Cache::store('foo_file')->get('key'))->toBe('central'); + // 'bar_file' was never scoped, it still reads from the same path it was writing to in tenant context + expect(Cache::store('bar_file')->get('key'))->toBe('written in tenant context'); +}); + +test('a configured lock_path is scoped separately from path', function () { + // A store can configure path and lock_path as two different directories + $centralStoragePath = storage_path(); + $path = "{$centralStoragePath}/framework/cache/foo"; + $lockPath = "{$centralStoragePath}/framework/cache/foo_locks"; + + // The paths are hardcoded, so delete the directories before running the assertions. + // Cache::store('foo_file')->lock(...) below defaults to a lock that never expires, + // and this test never releases it, so a stale lock left over from + // a previous run would make lock() fail forever. + File::deleteDirectory($path); + File::deleteDirectory($lockPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + 'lock_path' => $lockPath, + ], + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $tenantPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo"; + $tenantLockPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo_locks"; + + // lock('foo')->get() acquires the lock and creates the lock file at $tenantLockPath + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + + expect(File::isDirectory($tenantLockPath))->toBeTrue(); + // The lock file was created at $tenantLockPath, not $tenantPath + expect(File::isDirectory($tenantPath))->toBeFalse(); + + tenancy()->end(); + + // The lock was only acquired in the tenant context, so in the central context, + // it's free (and the lock file doesn't exist in the central context). + expect(File::isDirectory($lockPath))->toBeFalse(); + // Acquire the lock in the central context + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + // The lock file was created at the original (central) lock_path + expect(File::isDirectory($lockPath))->toBeTrue(); +}); + +test('a file cache store without a configured lock_path defaults to using its scoped path for locks', function () { + // 'lock_path' is optional in the file store config. + // FileStore falls back to using 'path' for locks when 'lock_path' is not specified in the config (or set to null). + // scopeCache() respects the original config and leaves lock_path unset/null. + $centralStoragePath = storage_path(); + $path = "{$centralStoragePath}/framework/cache/foo"; + + // $path is hardcoded here. The directory created at $path in a previous run can persist, + // so delete the directory at $path first. + File::deleteDirectory($path); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + // 'lock_path' not set (same behavior as if it were set to null) + ], + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $tenantPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo"; + + // Initializing tenancy scopes the store's 'path', but respects the configured + // (unset) 'lock_path' and leaves it alone. + expect(config('cache.stores.foo_file.path'))->toBe($tenantPath); + expect(config('cache.stores.foo_file.lock_path'))->toBeNull(); + + // With no 'lock_path' configured, the lock file will be created in the scoped 'path' directory + expect(File::isDirectory($tenantPath))->toBeFalse(); + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + expect(File::isDirectory($tenantPath))->toBeTrue(); + + // The lock is still held in the tenant context, so acquiring it again fails + expect(Cache::store('foo_file')->lock('foo')->get())->toBeFalse(); + + tenancy()->end(); + + // The same 'foo' lock is free in central context. + // revert() points the 'path' back to the original. + expect(File::isDirectory($path))->toBeFalse(); + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + expect(File::isDirectory($path))->toBeTrue(); +}); + +test('a cache store using a path not based on storage_path() is scoped to a tenant subdirectory', function () { + // tenantScopedPath() has no central storage path prefix to swap for the tenant's here, + // so it appends the tenant suffix to $path instead. + $path = '/tmp/tenancy-cache-test'; + File::deleteDirectory($path); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + ], + ]); + + $tenant1 = Tenant::create(); + $tenant2 = Tenant::create(); + + tenancy()->initialize($tenant1); + Cache::store('foo_file')->put('key', 'tenant1'); + + tenancy()->initialize($tenant2); + expect(Cache::store('foo_file')->get('key'))->toBeNull(); + + tenancy()->initialize($tenant1); + expect(Cache::store('foo_file')->get('key'))->toBe('tenant1'); + + // Tenant's 'foo_file' cache directory is created inside $path, + // not at the tenant-scoped storage_path(). + expect(File::isDirectory("{$path}/tenant{$tenant1->id}"))->toBeTrue(); + expect(File::isDirectory(storage_path('framework/cache/data')))->toBeFalse(); + + tenancy()->end(); +}); diff --git a/tests/SessionSeparationTest.php b/tests/SessionSeparationTest.php index 6c7a8aa1..cb823afd 100644 --- a/tests/SessionSeparationTest.php +++ b/tests/SessionSeparationTest.php @@ -83,6 +83,57 @@ test('file sessions are separated', function (bool $scopeSessions) { } })->with([true, false]); +test('file sessions are separated when a custom session path is configured', function () { + $centralStoragePath = storage_path(); + $configuredSessionPath = "{$centralStoragePath}/framework/foo_sessions"; + + config([ + 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], + 'session.driver' => 'file', + 'session.files' => $configuredSessionPath, + ]); + + $sessionPath = fn () => invade(app('session')->driver()->getHandler())->path; + + expect($sessionPath())->toBe($configuredSessionPath); + + File::cleanDirectory($configuredSessionPath); // clean up the configured sessions dir from past test runs + + $tenant = Tenant::create(); + $tenantSessionPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/foo_sessions"; + + $tenant->enter(); + + // The configured path gets scoped to the tenant + expect($sessionPath())->toBe($tenantSessionPath); + // Initializing tenancy creates the tenant session dir + expect(is_dir($tenantSessionPath))->toBeTrue(); + + $tenant->leave(); + + // Session path reverts back to the original configured path + expect($sessionPath())->toBe($configuredSessionPath); + + // StartSession saves the session at the end of the request, so each request below creates a session file + Route::middleware([StartSession::class, InitializeTenancyByPath::class])->get('/{tenant}/foo', fn () => 'bar'); + Route::middleware(StartSession::class)->get('/central', fn () => 'bar'); + + expect(File::files($tenantSessionPath))->toHaveCount(0); + + // Visiting a tenant route should create the session file at the configured path scoped for the tenant + pest()->get("/{$tenant->id}/foo"); + + expect(File::files($tenantSessionPath))->toHaveCount(1); + expect(File::files($configuredSessionPath))->toHaveCount(0); + + // End tenancy to test the revert behavior (= the central session file gets created at the original configured path) + tenancy()->end(); + + pest()->get('/central'); + + expect(File::files($configuredSessionPath))->toHaveCount(1); +}); + test('redis sessions are separated using the redis bootstrapper', function (bool $bootstrappedEnabled) { config([ 'tenancy.bootstrappers' => $bootstrappedEnabled ? [RedisTenancyBootstrapper::class] : [],