diff --git a/assets/config.php b/assets/config.php index b2001091..11ac739d 100644 --- a/assets/config.php +++ b/assets/config.php @@ -321,11 +321,12 @@ return [ /** * Filesystem tenancy config. Used by FilesystemTenancyBootstrapper. - * https://tenancyforlaravel.com/docs/v3/tenancy-bootstrappers/#filesystem-tenancy-boostrapper. + * https://v4.tenancyforlaravel.com/bootstrappers/filesystem */ 'filesystem' => [ /** - * Each disk listed in the 'disks' array will be suffixed by the suffix_base, followed by the tenant_id. + * Each disk listed in the 'disks' array will have its root + * suffixed by the suffix_base, followed by the tenant_id. */ 'suffix_base' => 'tenant', 'disks' => [ @@ -337,10 +338,18 @@ return [ /** * Use this for local disks. * - * See https://tenancyforlaravel.com/docs/v3/tenancy-bootstrappers/#filesystem-tenancy-boostrapper + * Customizes how the disk's root is scoped, instead of the default behavior + * which simply appends the tenant suffix to the original root. + * + * The overrides can reference the following placeholders: + * - %storage_path% -- the tenant's storage directory + * - %original_storage_path% -- the central storage directory. + * - %tenant% -- the tenant key. + * + * See https://v4.tenancyforlaravel.com/bootstrappers/filesystem */ 'root_override' => [ - // Disks whose roots should be overridden after storage_path() is suffixed. + // Disks whose roots should be overridden in tenant context. 'local' => '%storage_path%/app/', 'public' => '%storage_path%/app/public/', ], @@ -357,7 +366,8 @@ return [ * Use `php artisan tenants:link` to create a symbolic link from the tenant's storage to its public directory. */ 'url_override' => [ - // Note that the local disk you add must exist in the tenancy.filesystem.root_override config + // Note that the local disk you add must exist in the tenancy.filesystem.disks config, + // and it must have a non-falsy root (not null nor an empty string). 'public' => 'public-%tenant%', ], @@ -378,11 +388,7 @@ return [ /** * Should storage_path() be suffixed. * - * Note: Disabling this will likely break local disk tenancy. Only disable this if you're using an external file storage service like S3. - * - * For the vast majority of applications, this feature should be enabled. But in some - * edge cases, it can cause issues (like using Passport with Vapor - see #196), so - * you may want to disable this if you are experiencing these edge case issues. + * Only affects the storage_path() helper, other features use the tenant storage directory regardless. */ 'suffix_storage_path' => true, diff --git a/src/Actions/CreateStorageSymlinksAction.php b/src/Actions/CreateStorageSymlinksAction.php index 2de9e915..94917c10 100644 --- a/src/Actions/CreateStorageSymlinksAction.php +++ b/src/Actions/CreateStorageSymlinksAction.php @@ -47,6 +47,12 @@ class CreateStorageSymlinksAction mkdir($storagePath, 0777, true); } + // The public path of a prefixed disk includes the prefix, + // and its parent directories may not exist yet. + if (! is_dir($publicParent = dirname($publicPath))) { + mkdir($publicParent, 0777, true); + } + if ($relativeLink) { app()->make('files')->relativeLink($storagePath, $publicPath); } else { diff --git a/src/Actions/RemoveStorageSymlinksAction.php b/src/Actions/RemoveStorageSymlinksAction.php index 7b7b04d5..477a5c26 100644 --- a/src/Actions/RemoveStorageSymlinksAction.php +++ b/src/Actions/RemoveStorageSymlinksAction.php @@ -15,6 +15,16 @@ class RemoveStorageSymlinksAction { use DealsWithTenantSymlinks; + /** + * Should the directories created for nested symlinks be removed along with the symlink. + * + * Before enabling this, make sure you understand the removeLink() method + * and the high stakes of recursively removing parent directories (even if the logic should be sound). + * + * @see CreateStorageSymlinksAction + */ + public static bool $removeNestedDirectories = false; + /** * @param Tenant|Collection|LazyCollection $tenants */ @@ -32,12 +42,38 @@ class RemoveStorageSymlinksAction protected function removeLink(string $publicPath, Tenant $tenant): void { - if ($this->symlinkExists($publicPath)) { - event(new RemovingStorageSymlink($tenant)); + if (! $this->symlinkExists($publicPath)) { + return; + } - app()->make('files')->delete($publicPath); + $files = app()->make('files'); - event(new StorageSymlinkRemoved($tenant)); + event(new RemovingStorageSymlink($tenant)); + + $files->delete($publicPath); + + event(new StorageSymlinkRemoved($tenant)); + + if (! static::$removeNestedDirectories) { + return; + } + + $publicRoot = realpath(public_path()); + $directory = realpath(dirname($publicPath)); + + if ($publicRoot === false || $directory === false) { + return; + } + + // Remove the directories CreateStorageSymlinksAction created for the symlink + // until a non-empty one is reached. + while (str_starts_with($directory, rtrim($publicRoot, '/\\') . DIRECTORY_SEPARATOR) && $files->isEmptyDirectory($directory)) { + if (! @rmdir($directory)) { + // Stop the loop if the directory couldn't be removed + break; + } + + $directory = dirname($directory); } } } diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 1574ea4b..0a4f9992 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -128,10 +128,14 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper $scopedDisks = []; foreach ($this->app['config']['filesystems.disks'] as $name => $disk) { - if (isset($disk['driver'], $disk['disk']) - && $disk['driver'] === 'scoped' - && in_array($disk['disk'], $tenantDisks, true)) { + if (($disk['driver'] ?? null) !== 'scoped') { + continue; + } + + if (in_array(static::baseDiskName($name), $tenantDisks, true)) { $scopedDisks[] = $name; + } elseif (in_array($name, $tenantDisks, true)) { + throw new Exception("Disk [$name] uses the 'scoped' driver, so it has no root to make tenant-aware. List its base disk in tenancy.filesystem.disks instead."); } } @@ -140,6 +144,12 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper protected function diskRoot(string $disk, Tenant|false $tenant): void { + if ($this->app['config']["filesystems.disks.$disk.driver"] === 'scoped') { + // Skip scoped disks since they have no root to override + // (reachable when a scoped disk is listed in tenancy.filesystem.disks alongside its base disk). + return; + } + if ($tenant === false) { $this->app['config']["filesystems.disks.$disk.root"] = $this->originalDisks[$disk]['root']; @@ -176,7 +186,7 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper { $diskConfig = $this->app['config']["filesystems.disks.{$disk}"]; - if ($diskConfig['driver'] !== 'local' || $this->app['config']["tenancy.filesystem.url_override.{$disk}"] === null) { + if ($diskConfig['driver'] !== 'local' || ! $this->app['config']["tenancy.filesystem.url_override.{$disk}"]) { return; } @@ -327,4 +337,39 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper { return app(static::class)->originalStoragePath; } + + /** + * Get the storage path of the passed tenant (independent of the current context). + * + * Note that the returned path doesn't depend on suffix_storage_path. + * That config option only affects the storage_path() helper. + */ + public static function getTenantStoragePath(Tenant $tenant): string + { + $bootstrapper = app(static::class); + + return $bootstrapper->tenantStoragePath($bootstrapper->suffix($tenant)); + } + + /** + * Name of the disk whose root the passed disk uses. + * + * Disks using the 'scoped' driver have no root or url of their own -- they inherit those from their parent disk, + * which can be scoped as well, so only the final/base parent has to be tenant-aware. + * + * Returns null if the base disk has no name, i.e. when the disk is configured inline as an array. + */ + public static function baseDiskName(string $disk): string|null + { + while (config("filesystems.disks.$disk.driver") === 'scoped') { + if (! is_string($parent = config("filesystems.disks.$disk.disk"))) { + // Laravel allows configuring the parent disk inline as an array, and such a disk has no name + return null; + } + + $disk = $parent; + } + + return $disk; + } } diff --git a/src/Bootstrappers/LogChannelBootstrapper.php b/src/Bootstrappers/LogChannelBootstrapper.php index 293028e3..d25d9f5f 100644 --- a/src/Bootstrappers/LogChannelBootstrapper.php +++ b/src/Bootstrappers/LogChannelBootstrapper.php @@ -21,11 +21,8 @@ use Stancl\Tenancy\Contracts\Tenant; * Laravel's 'single' and 'daily' channels by default. To customize it, * see the property's docblock. * - * For the storage path channels to be scoped correctly: - * - this bootstrapper must run *after* FilesystemTenancyBootstrapper, - * since FilesystemTenancyBootstrapper adjusts storage_path() for the tenant - * - storage path suffixing has to be enabled (= config('tenancy.filesystem.suffix_storage_path') - * must be true), since the storage path suffix is what separates filesystem-based logs + * Note that since the tenant's storage path is resolved using FilesystemTenancyBootstrapper::getTenantStoragePath(), + * which is a public static method, FilesystemTenancyBootstrapper does not have to be enabled. * * For logging channels that are not filesystem-based, see the $channelOverrides logic. * @@ -40,14 +37,10 @@ class LogChannelBootstrapper implements TenancyBootstrapper /** * Logging channels whose path is built using storage_path() (e.g. Laravel's 'single' and 'daily'). * - * Channels included here will be configured to use tenant-specific storage paths - * created using storage_path() in the tenant context. Overrides in the $channelOverrides - * property take precedence over $storagePathChannels when a channel is included in both. + * Channels included here will be configured to use tenant-specific storage paths. * - * Requires FilesystemTenancyBootstrapper to run before this bootstrapper, - * and storage path suffixing to be enabled. - * - * @see Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper + * Overrides in the $channelOverrides property take precedence over + * $storagePathChannels when a channel is included in both. */ public static array $storagePathChannels = ['single', 'daily']; @@ -158,11 +151,12 @@ class LogChannelBootstrapper implements TenancyBootstrapper // The tenant log will be located at e.g. "storage/tenant{$tenantKey}/logs/laravel.log". $originalChannelPath = $this->config->get("logging.channels.{$channel}.path"); $centralStoragePath = FilesystemTenancyBootstrapper::getBoundCentralStoragePath(); + $tenantStoragePath = FilesystemTenancyBootstrapper::getTenantStoragePath($tenant); // The tenant log will inherit the segment that follows the storage path from the central channel path config. // For example, if a channel's path is configured to storage_path('logs/foo.log') (storage/logs/foo.log), - // the 'logs/foo.log' segment will be passed to storage_path() in the tenant context (storage/tenant123/logs/foo.log). - $this->config->set("logging.channels.{$channel}.path", storage_path(Str::after($originalChannelPath, $centralStoragePath))); + // the '/logs/foo.log' segment will be placed within the tenant storage path (so the log will be located at storage/tenant123/logs/foo.log). + $this->config->set("logging.channels.{$channel}.path", $tenantStoragePath . Str::after($originalChannelPath, rtrim($centralStoragePath, '/\\'))); } } } diff --git a/src/Concerns/DealsWithTenantSymlinks.php b/src/Concerns/DealsWithTenantSymlinks.php index 114eadb5..a53c2e51 100644 --- a/src/Concerns/DealsWithTenantSymlinks.php +++ b/src/Concerns/DealsWithTenantSymlinks.php @@ -7,15 +7,23 @@ namespace Stancl\Tenancy\Concerns; use Exception; use Stancl\Tenancy\Contracts\Tenant; +/** + * Requires FilesystemTenancyBootstrapper to be enabled, since the tenant symlinks + * point to the disk roots scoped by the bootstrapper. + * + * @see \Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper + */ trait DealsWithTenantSymlinks { /** - * Get all possible tenant symlinks, existing or not (array of ['public path' => 'storage path']). + * Get all possible tenant symlinks, existing or not (array of ['public path' => 'disk root with the disk's prefix appended']). + * + * Tenants can have a symlink for each local disk that is listed + * in both tenancy.filesystem.disks and tenancy.filesystem.url_override. * - * Tenants can have a symlink for each disk registered in the tenancy.filesystem.url_override config. * This is used for creating all possible tenant symlinks and removing all existing tenant symlinks. - * The same storage path can be symlinked to multiple public paths, which is why the public path - * is the Collection key. + * The same disk root can be symlinked to multiple public paths, which is why the public path + * is the array key. * * @return array */ @@ -23,20 +31,19 @@ trait DealsWithTenantSymlinks { $disks = config('filesystems.disks'); $urlOverrides = config('tenancy.filesystem.url_override'); - $rootOverrides = config('tenancy.filesystem.root_override'); $tenantKey = $tenant->getTenantKey(); - $tenantStoragePath = tenancy()->run($tenant, fn () => storage_path()); + $tenantDisks = tenancy()->run($tenant, fn () => config('filesystems.disks')); /** @var array $symlinks */ $symlinks = []; foreach ($urlOverrides as $disk => $publicPath) { - if (! isset($disks[$disk])) { + if (! $publicPath) { continue; } - if (! isset($rootOverrides[$disk])) { + if (! isset($disks[$disk])) { continue; } @@ -44,10 +51,25 @@ trait DealsWithTenantSymlinks throw new Exception("Disk $disk is not a local disk. Only local disks can be symlinked."); } - $publicPath = str_replace('%tenant%', (string) $tenantKey, $publicPath); - $storagePath = str_replace('%storage_path%', $tenantStoragePath, $rootOverrides[$disk]); + if (! in_array($disk, config('tenancy.filesystem.disks'), true)) { + // The bootstrapper only scopes disks listed in tenancy.filesystem.disks. + // Without that, the root stays central and the symlink of every tenant would point to it. + throw new Exception("Disk $disk is not tenant-aware. Add it to the tenancy.filesystem.disks config to make its root tenant-specific."); + } - $symlinks[public_path($publicPath)] = $storagePath; + $publicPath = str_replace('%tenant%', (string) $tenantKey, $publicPath); + $diskRoot = $tenantDisks[$disk]['root']; + + if ($prefix = trim($disks[$disk]['prefix'] ?? '', '/\\')) { + // Append the disk's prefix to the disk root + $diskRoot = rtrim($diskRoot, '/\\') . DIRECTORY_SEPARATOR . $prefix; + + // Append the same prefix to the public path. + // Storage::url() appends the disk's prefix to the url, so the prefix has to be in the public path as well. + $publicPath .= DIRECTORY_SEPARATOR . $prefix; + } + + $symlinks[public_path($publicPath)] = $diskRoot; } return $symlinks; diff --git a/src/Controllers/TenantAssetController.php b/src/Controllers/TenantAssetController.php index 243135ed..af222218 100644 --- a/src/Controllers/TenantAssetController.php +++ b/src/Controllers/TenantAssetController.php @@ -6,12 +6,25 @@ namespace Stancl\Tenancy\Controllers; use Closure; use Exception; +use Illuminate\Filesystem\LocalFilesystemAdapter; use Illuminate\Http\Request; use Illuminate\Routing\Controllers\HasMiddleware; use Illuminate\Routing\Controllers\Middleware; +use Illuminate\Support\Facades\Storage; +use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Throwable; +/** + * Serves files from app/public inside the tenant's storage directory, or from the root + * of the $publicDisk when the property is set. + * + * Requires FilesystemTenancyBootstrapper to be enabled, so that writes to the default + * public disk end up in the app/public directory within the *tenant's* storage, or so that the + * public disk set in the static property is similarly scoped. + * + * @see FilesystemTenancyBootstrapper + */ class TenantAssetController implements HasMiddleware { /** @@ -28,6 +41,19 @@ class TenantAssetController implements HasMiddleware */ public static array $middleware = []; + /** + * Disk the assets are served from. + * + * When null, the assets are served from app/public inside the tenant's storage directory. + * + * The disk has to be local, since the assets are read from the filesystem. Disks using the + * 'scoped' driver are supported as long as the disk they're based on uses the 'local' driver. + * + * The disk also has to be listed in tenancy.filesystem.disks -- for scoped disks, it's the + * disk they're based on that has to be listed there (since a scoped disk inherits its root). + */ + public static string|null $publicDisk = null; + public static function middleware() { return array_map( @@ -51,12 +77,53 @@ class TenantAssetController implements HasMiddleware ? (static::$headers)($request) : static::$headers; - return response()->file(storage_path("app/public/$path"), $headers); + return response()->file($this->assetRoot() . "/$path", $headers); } catch (Throwable) { abort(404); } } + /** + * Directory the assets are served from -- the root of the $publicDisk, or app/public + * inside the tenant's storage directory when no disk is configured. + * + * The tenant's storage directory is resolved using the FilesystemTenancyBootstrapper::getTenantStoragePath(). + */ + protected function assetRoot(): string + { + if (! tenant()) { + throw new Exception('Tenant assets can only be served in tenant context.'); + } + + if (static::$publicDisk) { + $disk = Storage::disk(static::$publicDisk); + + if (! $disk instanceof LocalFilesystemAdapter) { + throw new Exception('Disk [' . static::$publicDisk . '] is not a local disk. Only local disks can be used for serving assets.'); + } + + $baseDiskName = FilesystemTenancyBootstrapper::baseDiskName(static::$publicDisk); + + if ($baseDiskName === null) { + throw new Exception('Disk [' . static::$publicDisk . '] has an unnamed parent disk. Use a named parent disk listed in tenancy.filesystem.disks.'); + } + + if (! in_array($baseDiskName, config('tenancy.filesystem.disks'), true)) { + // FilesystemTenancyBootstrapper only scopes the roots of disks listed in tenancy.filesystem.disks. + // Without that, the root stays central and every tenant would be served the same directory. + throw new Exception("Disk [$baseDiskName] is not tenant-aware. Add it to the tenancy.filesystem.disks config to make its root tenant-specific."); + } + + // The full path is read from the resolved disk rather than from the disk's configured 'root', + // since the 'root' doesn't have to be the full path of every local disk: + // - disks using the 'scoped' driver have no 'root' in their config -- they inherit it from the parent disk + // - a disk's configured 'prefix' is a part of the full path as well. + return rtrim($disk->path(''), DIRECTORY_SEPARATOR); + } + + return FilesystemTenancyBootstrapper::getTenantStoragePath(tenant()) . '/app/public'; + } + /** * Prevent path traversal attacks. This is generally a non-issue on modern * webservers but it's still worth handling on the application level as well. @@ -67,18 +134,21 @@ class TenantAssetController implements HasMiddleware { $this->abortIf($path === null, 'Empty path'); - $allowedRoot = realpath(storage_path('app/public')); + $allowedRoot = realpath($this->assetRoot()); - // `storage_path('app/public')` doesn't exist, so it cannot contain files + // The asset root doesn't exist, so it cannot contain files $this->abortIf($allowedRoot === false, "Storage root doesn't exist"); + // realpath() ensures the directory exists and converts / to \ on Windows $attemptedPath = realpath("{$allowedRoot}/{$path}"); // User is attempting to access a nonexistent file $this->abortIf($attemptedPath === false, 'Accessing a nonexistent file'); - // User is attempting to access a file outside the $allowedRoot folder - $this->abortIf(! str($attemptedPath)->startsWith($allowedRoot), 'Accessing a file outside the storage root'); + // User is attempting to access a file outside the $allowedRoot folder. + // The trailing separator is needed so that sibling directories that + // start with the same name (e.g. app/public-private) aren't accepted. + $this->abortIf(! str($attemptedPath)->startsWith(rtrim($allowedRoot, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR), 'Accessing a file outside the storage root'); } /** @return void|never */ diff --git a/src/Jobs/DeleteTenantStorage.php b/src/Jobs/DeleteTenantStorage.php index 36a0d326..7245c4d0 100644 --- a/src/Jobs/DeleteTenantStorage.php +++ b/src/Jobs/DeleteTenantStorage.php @@ -10,8 +10,20 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\File; +use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; +/** + * Delete the tenant's storage directory. + * + * The directory is used by the FilesystemTenancyBootstrapper for: + * - scoped storage_path() when suffix_storage_path is enabled + * - scoped cache when enabled + * - scoped sessions when enabled + * - scoped disks when enabled + * + * @see FilesystemTenancyBootstrapper + */ class DeleteTenantStorage implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; @@ -22,17 +34,11 @@ class DeleteTenantStorage implements ShouldQueue public function handle(): void { - if (config('tenancy.filesystem.suffix_storage_path') === false) { - // Skip storage deletion if path suffixing is disabled - return; - } + $tenantStoragePath = FilesystemTenancyBootstrapper::getTenantStoragePath($this->tenant); + $centralStoragePath = FilesystemTenancyBootstrapper::getBoundCentralStoragePath(); - $centralStoragePath = tenancy()->central(fn () => storage_path()); - $tenantStoragePath = tenancy()->run($this->tenant, fn () => storage_path()); - - if ($tenantStoragePath === $centralStoragePath) { - // Check again to ensure the tenant storage path is distinct from the central storage path - // to avoid any accidental central storage path deletion + if (realpath($tenantStoragePath) === realpath($centralStoragePath)) { + // Never delete the central storage directory return; } diff --git a/tests/ActionTest.php b/tests/ActionTest.php index 93db0eb3..a885819d 100644 --- a/tests/ActionTest.php +++ b/tests/ActionTest.php @@ -4,6 +4,10 @@ declare(strict_types=1); use Illuminate\Support\Facades\Event; use Stancl\Tenancy\Events\TenancyEnded; +use Stancl\Tenancy\Events\StorageSymlinkCreated; +use Stancl\Tenancy\Events\StorageSymlinkRemoved; +use Stancl\Tenancy\Events\CreatingStorageSymlink; +use Stancl\Tenancy\Events\RemovingStorageSymlink; use Stancl\Tenancy\Database\Models\Tenant; use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; @@ -12,20 +16,33 @@ use Stancl\Tenancy\Actions\CreateStorageSymlinksAction; use Stancl\Tenancy\Actions\RemoveStorageSymlinksAction; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Illuminate\Support\Facades\File; +use Illuminate\Support\Facades\Storage; beforeEach(function () { Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + RemoveStorageSymlinksAction::$removeNestedDirectories = false; }); -test('create storage symlinks action works', function() { +afterEach(function () { + RemoveStorageSymlinksAction::$removeNestedDirectories = false; +}); + +test('create storage symlinks action works', function (string|null $rootOverride, bool $suffixStoragePath) { config([ 'tenancy.bootstrappers' => [ FilesystemTenancyBootstrapper::class, ], 'tenancy.filesystem.suffix_base' => 'tenant-', - 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', - 'tenancy.filesystem.url_override.public' => 'public-%tenant%' + // The disk root is suffixed regardless of the suffix_storage_path config + 'tenancy.filesystem.suffix_storage_path' => $suffixStoragePath, + 'tenancy.filesystem.root_override.public' => $rootOverride, + 'tenancy.filesystem.url_override' => [ + 'public' => 'public-%tenant%', + // Disks with a falsy url_override are skipped + 'local' => '', + ], ]); /** @var Tenant $tenant */ @@ -38,12 +55,50 @@ test('create storage symlinks action works', function() { expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeFalse(); expect(file_exists($publicPath))->toBeFalse(); + Storage::disk('public')->put('foo.txt', 'tenant file'); + + Event::fake([CreatingStorageSymlink::class, StorageSymlinkCreated::class]); + (new CreateStorageSymlinksAction)($tenant); - // The symlink exists and is valid - expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeTrue(); - expect(file_exists($publicPath))->toBeTrue(); - $this->assertEquals(storage_path("app/public/"), readlink($publicPath)); + Event::assertDispatched(CreatingStorageSymlink::class, fn (CreatingStorageSymlink $event) => $event->tenant->is($tenant)); + Event::assertDispatched(StorageSymlinkCreated::class, fn (StorageSymlinkCreated $event) => $event->tenant->is($tenant)); + + // The symlink exists and points to the directory the tenant's disk writes to + expect(is_link($publicPath))->toBeTrue(); + expect(readlink($publicPath))->toBe(config('filesystems.disks.public.root')); + expect(file_get_contents($publicPath . '/foo.txt'))->toBe('tenant file'); + + // The local disk is skipped because its url_override is '' -- no symlink is created at public_path('') + expect(is_link(public_path('')))->toBeFalse(); +})->with([ + 'default root_override' => ['%storage_path%/app/public/', true], + 'suffix_storage_path disabled' => ['%storage_path%/app/public/', false], + 'custom root_override' => ['%original_storage_path%/app/public/%tenant%/', true], + // Without a root_override, the bootstrapper suffixes the disk's central root + 'no root_override' => [null, true], +]); + +test('create storage symlinks action fails for disks that are not tenant-aware', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + // The public disk has both overrides configured, but it is not in + // tenancy.filesystem.disks, so the bootstrapper never scopes its root. + // Symlinking it would point every tenant's public path to the central disk root. + 'tenancy.filesystem.disks' => ['local'], + 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', + 'tenancy.filesystem.url_override.public' => 'public-%tenant%', + ]); + + /** @var Tenant $tenant */ + $tenant = Tenant::create(); + + expect(fn () => (new CreateStorageSymlinksAction)($tenant)) + ->toThrow(Exception::class, 'Disk public is not tenant-aware.'); + + expect(is_link(public_path('public-' . $tenant->getTenantKey())))->toBeFalse(); }); test('remove storage symlinks action works', function() { @@ -68,11 +123,25 @@ test('remove storage symlinks action works', function() { expect(is_link($publicPath = public_path("public-$tenantKey")))->toBeTrue(); expect(file_exists($publicPath))->toBeTrue(); + Event::fake([RemovingStorageSymlink::class, StorageSymlinkRemoved::class]); + (new RemoveStorageSymlinksAction)($tenant); + Event::assertDispatched(RemovingStorageSymlink::class, fn (RemovingStorageSymlink $event) => $event->tenant->is($tenant)); + Event::assertDispatched(StorageSymlinkRemoved::class, fn (StorageSymlinkRemoved $event) => $event->tenant->is($tenant)); + // The symlink doesn't exist expect(is_link($publicPath))->toBeFalse(); expect(file_exists($publicPath))->toBeFalse(); + + // Flush + Event::fake([RemovingStorageSymlink::class, StorageSymlinkRemoved::class]); + + // Nothing happens when there are no symlinks + (new RemoveStorageSymlinksAction)($tenant); + + Event::assertNotDispatched(RemovingStorageSymlink::class); + Event::assertNotDispatched(StorageSymlinkRemoved::class); }); test('removing tenant symlinks works even if the symlinks are invalid', function() { @@ -109,3 +178,123 @@ test('removing tenant symlinks works even if the symlinks are invalid', function expect(is_link($publicPath))->toBeFalse(); }); + +test('disks with a prefix are symlinked correctly', function (string $prefix) { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.filesystem.suffix_base' => 'tenant-', + 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', + 'tenancy.filesystem.url_override.public' => 'public-%tenant%', + 'filesystems.disks.public.prefix' => $prefix, + ]); + + /** @var Tenant $tenant */ + $tenant = Tenant::create(); + $tenantKey = $tenant->getTenantKey(); + + // possibleTenantSymlinks() trims the prefix, do the same here for the assertions to be accurate + $prefix = trim($prefix, '/'); + + tenancy()->initialize($tenant); + + Storage::disk('public')->put('foo.txt', 'tenant file'); + + (new CreateStorageSymlinksAction)($tenant); + + expect(Storage::disk('public')->url('foo.txt'))->toBe("http://localhost/public-{$tenantKey}/{$prefix}/foo.txt"); + expect(readlink(public_path("public-{$tenantKey}/{$prefix}")))->toBe(storage_path("app/public/{$prefix}")); + expect(file_get_contents(public_path("public-{$tenantKey}/{$prefix}/foo.txt")))->toBe('tenant file'); +})->with(['abc', 'abc/def', '/abc/def/']); + +test('symlinks of prefixed disks only expose the prefixed directory', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', + 'tenancy.filesystem.url_override.public' => 'public-%tenant%', + 'filesystems.disks.public.prefix' => 'abc', + ]); + + /** @var Tenant $tenant */ + $tenant = Tenant::create(); + $tenantKey = $tenant->getTenantKey(); + + tenancy()->initialize($tenant); + + Storage::disk('public')->put('foo.txt', 'tenant file'); + + // The disk cannot reach this file, neither should the symlink + File::put(storage_path('app/public/sibling.txt'), 'file next to the prefixed directory'); + + (new CreateStorageSymlinksAction)($tenant); + + expect(file_exists(public_path("public-{$tenantKey}/sibling.txt")))->toBeFalse(); + expect(file_get_contents(public_path("public-{$tenantKey}/abc/foo.txt")))->toBe('tenant file'); +}); + +test('removing a prefixed disk symlink removes the directories created for it', function () { + RemoveStorageSymlinksAction::$removeNestedDirectories = true; + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', + 'tenancy.filesystem.url_override.public' => 'public-%tenant%', + 'filesystems.disks.public.prefix' => 'abc/def', + ]); + + /** @var Tenant $tenant */ + $tenant = Tenant::create(); + $tenantKey = $tenant->getTenantKey(); + + tenancy()->initialize($tenant); + + Storage::disk('public')->put('foo.txt', 'tenant file'); + + (new CreateStorageSymlinksAction)($tenant); + + $symlink = public_path("public-{$tenantKey}/abc/def"); + $diskRoot = readlink($symlink); + + (new RemoveStorageSymlinksAction)($tenant); + + // The symlink and every directory created for it are deleted + expect(is_link($symlink))->toBeFalse(); + expect(file_exists(public_path("public-{$tenantKey}")))->toBeFalse(); + // public_path() itself is not deleted + expect(is_dir(public_path()))->toBeTrue(); + // The directory that the symlink points to is untouched + expect(file_get_contents($diskRoot . '/foo.txt'))->toBe('tenant file'); +}); + +test('non-empty directories are not removed with the symlink', function () { + RemoveStorageSymlinksAction::$removeNestedDirectories = true; + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.filesystem.root_override.public' => '%storage_path%/app/public/', + 'tenancy.filesystem.url_override.public' => 'public-%tenant%', + 'filesystems.disks.public.prefix' => 'abc/def', + ]); + + /** @var Tenant $tenant */ + $tenant = Tenant::create(); + $tenantKey = $tenant->getTenantKey(); + + tenancy()->initialize($tenant); + + (new CreateStorageSymlinksAction)($tenant); + + File::put(public_path("public-{$tenantKey}/abc/actual-file.txt"), 'foo'); + + (new RemoveStorageSymlinksAction)($tenant); + + expect(is_link(public_path("public-{$tenantKey}/abc/def")))->toBeFalse(); + expect(file_get_contents(public_path("public-{$tenantKey}/abc/actual-file.txt")))->toBe('foo'); +}); diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 785ffbc3..1066f872 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -146,6 +146,26 @@ test('links to storage disks with a configured root are suffixed if not overridd expect(storage_path())->toEqual($expectedStoragePath); }); +test('disks with a falsy url_override do not get their url overridden', function ($urlOverride) { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.filesystem.url_override.public' => $urlOverride, + ]); + + $tenant = Tenant::create(); + + $centralUrl = config('filesystems.disks.public.url'); + + tenancy()->initialize($tenant); + + expect(config('filesystems.disks.public.url'))->toBe($centralUrl); +})->with([ + 'empty string' => [''], + 'null' => [null], +]); + test('create and delete storage symlinks jobs work', function() { Event::listen( TenantCreated::class, @@ -185,63 +205,55 @@ test('create and delete storage symlinks jobs work', function() { $this->assertDirectoryDoesNotExist(public_path("public-$tenantKey")); }); -test('tenant storage gets deleted during tenant deletion when the DeletingTenant pipeline contains DeleteTenantStorage', function() { +test('tenant storage gets deleted during tenant deletion when the DeletingTenant pipeline contains DeleteTenantStorage', function (bool $bootstrapperEnabled) { Event::listen(DeletingTenant::class, JobPipeline::make([DeleteTenantStorage::class])->send(function (DeletingTenant $event) { return $event->tenant; })->shouldBeQueued(false)->toListener() ); + config([ + 'tenancy.bootstrappers' => $bootstrapperEnabled ? [FilesystemTenancyBootstrapper::class] : [], + ]); + $centralStoragePath = storage_path(); - tenancy()->initialize(Tenant::create()); + $tenantStoragePath = fn (Tenant $tenant) => $centralStoragePath . "/tenant{$tenant->getTenantKey()}"; - // FilesystemTenancyBootstrapper not enabled, - // tenant and central storage path is the same, - // the storage deletion will be skipped. - $tenantStoragePath = storage_path(); - expect($tenantStoragePath)->toBe($centralStoragePath); - expect(File::isDirectory($centralStoragePath))->toBeTrue(); - tenant()->delete(); + $tenant = Tenant::create(); + + File::ensureDirectoryExists($tenantStoragePath($tenant)); expect(File::isDirectory($centralStoragePath))->toBeTrue(); + expect(File::isDirectory($tenantStoragePath($tenant)))->toBeTrue(); - config([ - 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], - 'tenancy.filesystem.suffix_storage_path' => false, - ]); - - tenancy()->initialize(Tenant::create()); - - $tenantStoragePath = storage_path(); - - // FilesystemTenancyBootstrapper enabled, - // but tenant and central storage path is still the same - // because suffix_storage_path is false. - // The storage deletion will be skipped. - expect($tenantStoragePath)->toBe($centralStoragePath); - expect(File::isDirectory($centralStoragePath))->toBeTrue(); - tenant()->delete(); + $tenant->delete(); expect(File::isDirectory($centralStoragePath))->toBeTrue(); + expect(File::isDirectory($tenantStoragePath($tenant)))->toBeFalse(); +})->with([ + 'filesystem bootstrapper enabled' => true, + 'filesystem bootstrapper disabled' => false, +]); - config([ - 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], - 'tenancy.filesystem.suffix_storage_path' => true, - ]); +test('DeleteTenantStorage never deletes the central storage directory', function () { + $tenant = Tenant::create(); - tenancy()->initialize(Tenant::create()); - $tenantStoragePath = storage_path(); + $centralStoragePath = FilesystemTenancyBootstrapper::getBoundCentralStoragePath(); + $tenantStoragePath = FilesystemTenancyBootstrapper::getTenantStoragePath($tenant); - // FilesystemTenancyBootstrapper enabled, - // suffix_storage_path enabled, so the two paths are distinct. - // Tenant storage will be deleted. - expect($tenantStoragePath)->not()->toBe($centralStoragePath); - expect(File::isDirectory($tenantStoragePath))->toBeTrue(); + File::ensureDirectoryExists($centralStoragePath . '/app'); - tenant()->delete(); + // Make the tenant storage path a symlink to the central storage directory + File::deleteDirectory($tenantStoragePath); + symlink($centralStoragePath, $tenantStoragePath); + + expect(realpath($tenantStoragePath))->toBe(realpath($centralStoragePath)); + + (new DeleteTenantStorage($tenant))->handle(); - expect(File::isDirectory($tenantStoragePath))->toBeFalse(); expect(File::isDirectory($centralStoragePath))->toBeTrue(); + expect(File::isDirectory($centralStoragePath . '/app'))->toBeTrue(); + expect(is_link($tenantStoragePath))->toBeTrue(); }); test('the framework/cache directory is created when storage_path is scoped', function (bool $suffixStoragePath) { @@ -275,29 +287,106 @@ test('scoped disks are scoped per tenant', function () { 'disk' => 'public', 'prefix' => 'scoped_disk_prefix', ], + 'filesystems.disks.nested_disk' => [ + 'driver' => 'scoped', + 'disk' => 'scoped_disk', + 'prefix' => 'nested_disk_prefix', + ], ]); + foreach (['scoped_disk' => '', 'nested_disk' => '/nested_disk_prefix'] as $disk => $nested_prefix) { + $path = "app/public/scoped_disk_prefix{$nested_prefix}/foo.txt"; + + $tenant = Tenant::create(); + $centralFile = storage_path($path); + $tenantFile = storage_path("tenant{$tenant->id}/$path"); + + Storage::disk($disk)->put('foo.txt', 'central'); + expect(file_get_contents($centralFile))->toBe('central'); + + tenancy()->initialize($tenant); + + expect(Storage::disk($disk)->get('foo.txt'))->toBeNull(); + + Storage::disk($disk)->put('foo.txt', 'tenant'); + expect(file_get_contents($tenantFile))->toBe('tenant'); + + tenancy()->end(); + + expect(Storage::disk($disk)->get('foo.txt'))->toBe('central'); + expect(file_get_contents($centralFile))->toBe('central'); + expect(file_get_contents($tenantFile))->toBe('tenant'); + } +}); + +test('scoped disks based on a non-local disk are scoped per tenant', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'filesystems.disks.scoped_s3' => [ + 'driver' => 'scoped', + 'disk' => 's3', + 'prefix' => 'scoped_s3_prefix', + ], + 'tenancy.filesystem.disks' => ['s3'], // As long as the base disk (s3) is listed here, the scoped disk will be scoped + ]); + + expect(Storage::disk('s3')->path('foo.txt'))->toBe('foo.txt'); + expect(Storage::disk('scoped_s3')->path('foo.txt'))->toBe('scoped_s3_prefix/foo.txt'); + $tenant = Tenant::create(); - - Storage::disk('scoped_disk')->put('foo.txt', 'central'); - expect(Storage::disk('scoped_disk')->get('foo.txt'))->toBe('central'); - expect(file_get_contents(storage_path() . "/app/public/scoped_disk_prefix/foo.txt"))->toBe('central'); - tenancy()->initialize($tenant); - expect(Storage::disk('scoped_disk')->get('foo.txt'))->toBe(null); - Storage::disk('scoped_disk')->put('foo.txt', 'tenant'); - expect(file_get_contents(storage_path() . "/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); - expect(Storage::disk('scoped_disk')->get('foo.txt'))->toBe('tenant'); + expect(Storage::disk('s3')->path('foo.txt'))->toBe("tenant{$tenant->id}/foo.txt"); + expect(Storage::disk('scoped_s3')->path('foo.txt'))->toBe("tenant{$tenant->id}/scoped_s3_prefix/foo.txt"); tenancy()->end(); - expect(Storage::disk('scoped_disk')->get('foo.txt'))->toBe('central'); - Storage::disk('scoped_disk')->put('foo.txt', 'central2'); - expect(Storage::disk('scoped_disk')->get('foo.txt'))->toBe('central2'); + expect(Storage::disk('scoped_s3')->path('foo.txt'))->toBe('scoped_s3_prefix/foo.txt'); +}); - expect(file_get_contents(storage_path() . "/app/public/scoped_disk_prefix/foo.txt"))->toBe('central2'); - expect(file_get_contents(storage_path() . "/tenant{$tenant->id}/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); +test('adding a scoped disk to tenancy.filesystem.disks throws an exception if its base disk is not listed', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'filesystems.disks.foo' => [ + 'driver' => 'scoped', + 'disk' => 'public', + 'prefix' => 'foo', + ], + 'filesystems.disks.bar' => [ + 'driver' => 'scoped', + 'disk' => 'foo', + 'prefix' => 'bar', + ], + // There's no way for a scoped disk with an array parent to have the parent listed in tenancy.filesystem.disks + 'filesystems.disks.inline_base' => [ + 'driver' => 'scoped', + 'disk' => [ + 'driver' => 'local', + 'root' => storage_path('app/inline'), + ], + 'prefix' => 'inline_base', + ], + ]); + + $initializeTenancy = fn () => tenancy()->initialize(Tenant::create()); + + config(['tenancy.filesystem.disks' => ['foo']]); + expect($initializeTenancy)->toThrow(Exception::class, "Disk [foo] uses the 'scoped' driver, so it has no root to make tenant-aware."); + + config(['tenancy.filesystem.disks' => ['bar']]); + expect($initializeTenancy)->toThrow(Exception::class, "Disk [bar] uses the 'scoped' driver, so it has no root to make tenant-aware."); + + config(['tenancy.filesystem.disks' => ['inline_base']]); + expect($initializeTenancy)->toThrow(Exception::class, "Disk [inline_base] uses the 'scoped' driver, so it has no root to make tenant-aware."); + + config(['tenancy.filesystem.disks' => ['public', 'foo', 'bar']]); + expect($initializeTenancy)->not()->toThrow(Throwable::class); + + // No way to make the 'inline_base' disk work }); test('file cache stores get their paths scoped on bootstrap and restored back on revert', function () { diff --git a/tests/Bootstrappers/LogChannelBootstrapperTest.php b/tests/Bootstrappers/LogChannelBootstrapperTest.php index c37e7b54..009a0d66 100644 --- a/tests/Bootstrappers/LogChannelBootstrapperTest.php +++ b/tests/Bootstrappers/LogChannelBootstrapperTest.php @@ -9,7 +9,6 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\LogChannelBootstrapper; -use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Illuminate\Support\Facades\Log; afterEach($cleanup = function () { @@ -42,15 +41,6 @@ beforeEach(function () use ($cleanup) { }); test('storage path channels get tenant-specific paths by default', function () { - // Note that for LogChannelBootstrapper to change the paths correctly by default, - // the bootstrapper MUST run after FilesystemTenancyBootstrapper. - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - $centralStoragePath = storage_path(); $tenant = Tenant::create(); @@ -76,10 +66,6 @@ test('storage path channels get tenant-specific paths by default', function () { test('all channels included in a stack get processed correctly', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], @@ -176,30 +162,24 @@ test('channel config keys remain unchanged if the specified tenant override attr }); test('channel overrides take precedence over the default storage path channel updating logic', function () { + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'tenant1']); LogChannelBootstrapper::$storagePathChannels = ['single']; LogChannelBootstrapper::$channelOverrides = [ - 'single' => function (Tenant $tenant, array $channel) { - return array_merge($channel, ['path' => storage_path("logs/override-{$tenant->id}.log")]); + 'single' => function (Tenant $tenant, array $channel) use ($centralStoragePath) { + return array_merge($channel, ['path' => "{$centralStoragePath}/logs/override-{$tenant->id}.log"]); }, ]; tenancy()->initialize($tenant); // Should use channel override, not the storage path updating behavior - expect(config('logging.channels.single.path'))->toEndWith('storage/logs/override-tenant1.log'); + expect(config('logging.channels.single.path'))->toBe("{$centralStoragePath}/logs/override-tenant1.log"); }); test('channels are forgotten and re-resolved during bootstrap and revert', function () { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - $logManager = app('log'); $originalChannel = $logManager->channel('single'); $originalSinglePath = config('logging.channels.single.path'); @@ -229,14 +209,8 @@ test('channels are forgotten and re-resolved during bootstrap and revert', funct // Test real usage test('logs are written to tenant-specific files and do not leak between contexts', function () { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - - $centralLogPath = storage_path('logs/laravel.log'); + $centralStoragePath = storage_path(); + $centralLogPath = "{$centralStoragePath}/logs/laravel.log"; Log::channel('single')->info('central'); @@ -244,15 +218,11 @@ test('logs are written to tenant-specific files and do not leak between contexts [$tenant1, $tenant2] = [Tenant::create(['id' => 'tenant1']), Tenant::create(['id' => 'tenant2'])]; - tenancy()->runForMultiple([$tenant1, $tenant2], function (Tenant $tenant) use ($centralLogPath) { + tenancy()->runForMultiple([$tenant1, $tenant2], function (Tenant $tenant) use ($centralStoragePath) { Log::channel('single')->info($tenant->id); - $tenantLogPath = storage_path('logs/laravel.log'); - // The log gets saved to the tenant's storage directory (default behavior) - expect($tenantLogPath) - ->not()->toBe($centralLogPath) - ->toEndWith("storage/tenant{$tenant->id}/logs/laravel.log"); + $tenantLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; expect(file_get_contents($tenantLogPath)) ->toContain($tenant->id) @@ -268,14 +238,14 @@ test('logs are written to tenant-specific files and do not leak between contexts // Tenant log messages didn't leak to logs of other tenants tenancy()->initialize($tenant1); - expect(file_get_contents(storage_path('logs/laravel.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant1->id}/logs/laravel.log")) ->toContain('tenant1') ->not()->toContain('central') ->not()->toContain('tenant2'); tenancy()->initialize($tenant2); - expect(file_get_contents(storage_path('logs/laravel.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant2->id}/logs/laravel.log")) ->toContain('tenant2') ->not()->toContain('central') ->not()->toContain('tenant1'); @@ -285,9 +255,8 @@ test('logs are written to tenant-specific files and do not leak between contexts $tenant = Tenant::create(['id' => 'override-tenant']); LogChannelBootstrapper::$channelOverrides = [ - 'single' => function (Tenant $tenant, array $channel) { - // The tenant log path will be set to storage/tenantoverride-tenant/logs/custom-override-tenant.log - return array_merge($channel, ['path' => storage_path("logs/custom-{$tenant->id}.log")]); + 'single' => function (Tenant $tenant, array $channel) use ($centralStoragePath) { + return array_merge($channel, ['path' => "{$centralStoragePath}/tenant{$tenant->id}/logs/custom-{$tenant->id}.log"]); }, ]; @@ -296,21 +265,18 @@ test('logs are written to tenant-specific files and do not leak between contexts Log::channel('single')->info('tenant-override'); - expect(file_get_contents(storage_path('logs/custom-override-tenant.log')))->toContain('tenant-override'); + expect(file_get_contents("{$centralStoragePath}/tenantoverride-tenant/logs/custom-override-tenant.log"))->toContain('tenant-override'); }); test('stack logs are written to all configured channels with tenant-specific paths', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], ], ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'stack-tenant']); $today = now()->format('Y-m-d'); @@ -327,8 +293,8 @@ test('stack logs are written to all configured channels with tenant-specific pat // Tenant context stack log tenancy()->initialize($tenant); Log::channel('stack')->info('tenant'); - $tenantSingleLogPath = storage_path('logs/laravel.log'); - $tenantDailyLogPath = storage_path("logs/laravel-{$today}.log"); + $tenantSingleLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; + $tenantDailyLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel-{$today}.log"; expect(file_get_contents($tenantSingleLogPath))->toContain('tenant'); expect(file_get_contents($tenantDailyLogPath))->toContain('tenant'); @@ -351,18 +317,15 @@ test('stack logs are written to all configured channels with tenant-specific pat test('stack channels that include any configured channel are re-resolved', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.custom_stack' => [ 'driver' => 'stack', 'channels' => ['single'], ], ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'stack-tenant']); - $centralLogPath = storage_path('logs/laravel.log'); + $centralLogPath = "{$centralStoragePath}/logs/laravel.log"; $logManager = app('log'); @@ -387,7 +350,7 @@ test('stack channels that include any configured channel are re-resolved', funct ->toContain('central log message') ->not()->toContain('tenant log message'); - $tenantLogPath = storage_path('logs/laravel.log'); + $tenantLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; expect(file_exists($tenantLogPath))->toBeTrue(); expect(file_get_contents($tenantLogPath)) ->toContain('tenant log message'); @@ -454,10 +417,6 @@ test('slack channel uses correct webhook urls', function () { test('tenant logs inherit the path from the central log path config', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], @@ -466,6 +425,7 @@ test('tenant logs inherit the path from the central log path config', function ( 'logging.channels.daily.path' => storage_path('logs/daily/custom-name.log'), ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(); $today = now()->format('Y-m-d'); @@ -476,18 +436,17 @@ test('tenant logs inherit the path from the central log path config', function ( tenancy()->initialize($tenant); - // Tenant log is located at storage/tenantX/logs/custom-name.log Log::channel('stack')->info($tenant->id); // The filename from the central config is preserved in tenant context expect(config('logging.channels.single.path'))->toEndWith('logs/single/custom-name.log'); expect(config('logging.channels.daily.path'))->toEndWith('logs/daily/custom-name.log'); - expect(file_get_contents(storage_path('logs/single/custom-name.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant->id}/logs/single/custom-name.log")) ->toContain($tenant->id) ->not()->toContain('central'); - expect(file_get_contents(storage_path("logs/daily/custom-name-{$today}.log"))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant->id}/logs/daily/custom-name-{$today}.log")) ->toContain($tenant->id) ->not()->toContain('central'); }); diff --git a/tests/TenantAssetTest.php b/tests/TenantAssetTest.php index ef1cb41f..a4e169b7 100644 --- a/tests/TenantAssetTest.php +++ b/tests/TenantAssetTest.php @@ -17,6 +17,7 @@ use Stancl\Tenancy\Middleware\InitializeTenancyByRequestData; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper; use Stancl\Tenancy\Controllers\TenantAssetController; +use Stancl\Tenancy\Enums\RouteMode; use Stancl\Tenancy\Events\TenancyEnded; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Overrides\TenancyUrlGenerator; @@ -30,6 +31,7 @@ beforeEach(function () { TenancyUrlGenerator::$prefixRouteNames = false; TenancyUrlGenerator::$passTenantParameterToRoutes = true; TenantAssetController::$headers = []; + TenantAssetController::$publicDisk = null; /** @var CloneRoutesAsTenant $cloneAction */ $cloneAction = app(CloneRoutesAsTenant::class); @@ -39,6 +41,11 @@ beforeEach(function () { Event::listen(TenancyEnded::class, RevertToCentralContext::class); }); +afterEach(function () { + TenantAssetController::$headers = []; + TenantAssetController::$publicDisk = null; +}); + test('asset can be accessed using the url returned by the tenant asset helper', function () { config(['tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class]); @@ -65,6 +72,174 @@ test('asset can be accessed using the url returned by the tenant asset helper', expect($content)->toBe('bar'); }); +test('tenant assets are served even when the suffix_storage_path config is set to false', function () { + config([ + 'tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class, + 'tenancy.filesystem.suffix_storage_path' => false, + ]); + + // With suffix_storage_path disabled, storage_path() stays central in tenant context + $centralStoragePath = storage_path(); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + expect(storage_path())->toBe($centralStoragePath); + + $filename = 'testfile' . Str::random(8); + Storage::disk('public')->put($filename, 'bar'); + + $response = pest()->get(tenant_asset($filename), ['X-Tenant' => $tenant->id]); + + // The asset is served from the tenant's storage directory, not from the central storage path + $response->assertSuccessful(); + expect($response->getFile()->getPathname()) + ->toBe("$centralStoragePath/tenant{$tenant->id}/app/public/$filename"); +}); + +test('the disk used for serving tenant assets is configurable', function () { + config([ + 'tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class, + // This is tenancy's default override for the local disk's root, set it here for clarity + 'tenancy.filesystem.root_override.local' => '%storage_path%/app/', + ]); + + // The local disk's root is overridden to '%storage_path%/app/' (so it does not use 'app/public') + TenantAssetController::$publicDisk = 'local'; + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $filename = 'testfile' . Str::random(8); + Storage::disk('local')->put($filename, 'bar'); + $path = Storage::disk('local')->path($filename); + + $response = pest()->get(tenant_asset($filename), ['X-Tenant' => $tenant->id]); + + // The asset is served from the disk's root instead of 'app/public' + $response->assertSuccessful(); + expect($response->getFile()->getPathname())->toBe($path); +}); + +test('tenant asset controller throws when the configured disk is not local or not tenant-aware', function () { + config([ + 'tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class, + // Add a disk that uses the s3 driver (= non-local disk). + // Use dummy credentials so that the s3 disk can be resolved without throwing an AWS exception. + 'filesystems.disks.remote' => [ + 'driver' => 's3', + 'region' => 'us-east-1', + 'key' => 'key', + 'secret' => 'secret', + 'bucket' => 'bucket', + ], + 'filesystems.disks.scoped_remote' => [ + 'driver' => 'scoped', + 'disk' => 'remote', + 'prefix' => 'assets', + ], + // 'media' isn't tenant-aware (i.e. not included in tenancy.filesystem.disks) + 'filesystems.disks.media' => [ + 'driver' => 'local', + 'root' => storage_path('app/media'), + ], + 'filesystems.disks.scoped_media' => [ + 'driver' => 'scoped', + 'disk' => 'media', + 'prefix' => 'assets', + ], + 'filesystems.disks.inline_scoped_media' => [ + 'driver' => 'scoped', + // Laravel allows configuring the parent disk inline, but such a disk + // has no name, so it cannot be listed in tenancy.filesystem.disks (i.e. made tenant-aware) + 'disk' => [ + 'driver' => 'local', + 'root' => storage_path('app/media'), + ], + 'prefix' => 'assets', + ], + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $this->withoutExceptionHandling(); + + $expectedExceptions = [ + 'remote' => 'Disk [remote] is not a local disk.', + 'scoped_remote' => 'Disk [scoped_remote] is not a local disk.', + 'media' => 'Disk [media] is not tenant-aware.', + 'scoped_media' => 'Disk [media] is not tenant-aware.', + 'inline_scoped_media' => 'Disk [inline_scoped_media] has an unnamed parent disk.', + ]; + + foreach ($expectedExceptions as $publicDisk => $exceptionMessage) { + TenantAssetController::$publicDisk = $publicDisk; + + expect(fn () => pest()->get(tenant_asset('foo.txt'), ['X-Tenant' => $tenant->id])) + ->toThrow(Exception::class, $exceptionMessage); + } +}); + +test('tenant assets cannot be served in central context', function () { + config([ + 'tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class, + // Make the asset route skip tenant identification + 'tenancy.default_route_mode' => RouteMode::UNIVERSAL, + ]); + + $this->withoutExceptionHandling(); + + foreach ([null, 'local'] as $publicDisk) { + TenantAssetController::$publicDisk = $publicDisk; + + expect(fn () => pest()->get(tenant_asset('foo.txt'))) + ->toThrow(Exception::class, 'Tenant assets can only be served in tenant context.'); + } +}); + +test('tenant assets are served from the resolved root of the configured disk', function () { + config([ + 'tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class, + // A scoped disk has no configured root -- it inherits the root of its parent disk + // and appends its prefix to it, both of which happens when the disk is resolved. + 'filesystems.disks.scoped_disk' => [ + 'driver' => 'scoped', + 'disk' => 'public', + 'prefix' => 'scoped_disk_prefix', + ], + // A prefix is part of the disk's full path, so it has to be included in the asset root + 'filesystems.disks.prefixed' => [ + 'driver' => 'local', + 'root' => storage_path('app/media'), + 'prefix' => 'foo_prefix', + ], + 'tenancy.filesystem.disks' => ['local', 'public', 'prefixed'], + 'tenancy.filesystem.root_override.prefixed' => '%storage_path%/app/media/', + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + foreach ([ + 'scoped_disk' => 'app/public/scoped_disk_prefix', + 'prefixed' => 'app/media/foo_prefix', + ] as $publicDisk => $expectedRoot) { + TenantAssetController::$publicDisk = $publicDisk; + + $filename = 'testfile' . Str::random(8); + Storage::disk($publicDisk)->put($filename, 'bar'); + $path = Storage::disk($publicDisk)->path($filename); + + expect($path)->toBe(storage_path("{$expectedRoot}/$filename")); + + $response = pest()->get(tenant_asset($filename), ['X-Tenant' => $tenant->id]); + + $response->assertSuccessful(); + expect($response->getFile()->getPathname())->toBe($path); + } +}); + test('asset helper returns a link to tenant asset controller when asset url is null', function () { config(['app.asset_url' => null]); config(['tenancy.filesystem.asset_helper_override' => true]); @@ -148,8 +323,6 @@ test('TenantAssetController headers are configurable', function () { $response->assertSuccessful(); $response->assertHeader('X-Foo', 'Bar'); - - TenantAssetController::$headers = []; // reset static property }); test('global asset helper returns the same url regardless of tenancy initialization', function () { @@ -233,24 +406,27 @@ test('tenant asset controller returns a 404 when accessing a nonexistent file', ]); }); -test('test asset controller returns a 404 when accessing a file outside the storage root', function () { +test('tenant asset controller only serves files inside the asset root', function () { config(['tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class]); $tenant = Tenant::create(); - tenancy()->initialize($tenant); - $storageRoot = storage_path("app/public"); + Storage::disk('public')->put('photo.jpg', 'public file'); - if (! is_dir($storageRoot)) { - mkdir(storage_path("app/public"), recursive: true); - file_put_contents(storage_path('app/foo.txt'), 'bar'); - } + pest()->get(tenant_asset('photo.jpg'), ['X-Tenant' => $tenant->id])->assertSuccessful(); + + // Files outside the asset root + file_put_contents(storage_path('app/photo.jpg'), 'private file'); + + // This path starts with the asset root but is a sibling dir, not a child. Regression assertion + mkdir($siblingDirectory = storage_path('app/public-originals'), recursive: true); + file_put_contents($siblingDirectory . '/photo.jpg', 'private file'); $this->withoutExceptionHandling(); - pest()->expectExceptionMessage('Accessing a file outside the storage root'); // outside tests this is a 404 - pest()->get(tenant_asset('../foo.txt'), [ - 'X-Tenant' => $tenant->id, - ]); + foreach (['../photo.jpg', '../public-originals/photo.jpg'] as $path) { + expect(fn () => pest()->get(tenant_asset($path), ['X-Tenant' => $tenant->id])) + ->toThrow(Exception::class, 'Accessing a file outside the storage root'); // outside tests this is a 404 + } });