diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 6870a17c..2aab2732 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,7 +1,7 @@ blank_issues_enabled: false contact_links: - name: Support Questions & Other - url: https://github.com/stancl/tenancy/blob/3.x/SUPPORT.md + url: https://archte.ch/discord about: 'If you have a question or need help using the package.' - name: Documentation Issue url: https://github.com/stancl/tenancy-docs/issues diff --git a/README.md b/README.md index 95fb7c60..8ce25809 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@

- Laravel 9.x + Laravel 10.x Latest Stable Version GitHub Actions CI status Donate diff --git a/assets/config.php b/assets/config.php index 50b53b0f..2c745c64 100644 --- a/assets/config.php +++ b/assets/config.php @@ -241,7 +241,7 @@ return [ * See https://tenancyforlaravel.com/docs/v3/tenancy-bootstrappers/#filesystem-tenancy-boostrapper */ 'root_override' => [ - // Disks whose roots should be overriden after storage_path() is suffixed. + // Disks whose roots should be overridden after storage_path() is suffixed. 'local' => '%storage_path%/app/', 'public' => '%storage_path%/app/public/', ], diff --git a/phpstan.neon b/phpstan.neon index ba269e5e..8a48a672 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -51,6 +51,10 @@ parameters: - '#Method Stancl\\Tenancy\\Tenancy::cachedResolvers\(\) should return array#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$tenancy#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$resolver#' + - + message: '#string\|false#' + paths: + - src/Controllers/TenantAssetController.php checkMissingIterableValueType: false checkGenericClassInNonGenericObjectType: false # later we may want to enable this diff --git a/src/Commands/MigrateFresh.php b/src/Commands/MigrateFresh.php index 7df75fb0..93b93e7c 100644 --- a/src/Commands/MigrateFresh.php +++ b/src/Commands/MigrateFresh.php @@ -20,6 +20,7 @@ class MigrateFresh extends BaseCommand parent::__construct(); $this->addOption('--drop-views', null, InputOption::VALUE_NONE, 'Drop views along with tenant tables.', null); + $this->addOption('--step', null, InputOption::VALUE_NONE, 'Force the migrations to be run so they can be rolled back individually.'); $this->setName('tenants:migrate-fresh'); } @@ -40,6 +41,7 @@ class MigrateFresh extends BaseCommand $this->components->task('Migrating', function () use ($tenant) { $this->callSilent('tenants:migrate', [ '--tenants' => [$tenant->getTenantKey()], + '--step' => $this->option('step'), '--force' => true, ]); }); diff --git a/src/Controllers/TenantAssetController.php b/src/Controllers/TenantAssetController.php index 980e5105..96fc6374 100644 --- a/src/Controllers/TenantAssetController.php +++ b/src/Controllers/TenantAssetController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Controllers; +use Exception; use Illuminate\Routing\Controllers\HasMiddleware; use Illuminate\Routing\Controllers\Middleware; use Symfony\Component\HttpFoundation\BinaryFileResponse; @@ -23,7 +24,7 @@ class TenantAssetController implements HasMiddleware // todo@docs this was renam */ public function __invoke(string $path = null): BinaryFileResponse { - abort_if($path === null, 404); + $this->validatePath($path); try { return response()->file(storage_path("app/public/$path")); @@ -31,4 +32,44 @@ class TenantAssetController implements HasMiddleware // todo@docs this was renam abort(404); } } + + /** + * 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. + * + * @throws \Symfony\Component\HttpKernel\Exception\HttpException + */ + protected function validatePath(string|null $path): void + { + $this->abortIf($path === null, 'Empty path'); + + $allowedRoot = realpath(storage_path('app/public')); + + // `storage_path('app/public')` doesn't exist, so it cannot contain files + $this->abortIf($allowedRoot === false, "Storage root doesn't exist"); + + $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'); + } + + /** @return void|never */ + protected function abortIf(bool $condition, string $exceptionMessage = ''): void + { + if ($condition) { + if (app()->runningUnitTests()) { + // Makes testing the cause of the failure in validatePath() easier + throw new Exception($exceptionMessage); + } else { + // We always use 404 to avoid leaking information about the cause of the error + // e.g. when someone is trying to access a nonexistent file outside of the allowed + // root folder, we don't want to let the user know whether such a file exists or not. + abort(404); + } + } + } } diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index 02a9c948..ad626427 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -295,11 +295,13 @@ test('migrate fresh command works', function () { test('run command with array of tenants works', function () { $tenantId1 = Tenant::create()->getTenantKey(); $tenantId2 = Tenant::create()->getTenantKey(); + $tenantId3 = Tenant::create()->getTenantKey(); Artisan::call('tenants:migrate-fresh'); pest()->artisan("tenants:run --tenants=$tenantId1 --tenants=$tenantId2 'foo foo --b=bar --c=xyz'") ->expectsOutputToContain('Tenant: ' . $tenantId1) ->expectsOutputToContain('Tenant: ' . $tenantId2) + ->doesntExpectOutput('Tenant: ' . $tenantId3) ->assertExitCode(0); }); diff --git a/tests/TenantAssetTest.php b/tests/TenantAssetTest.php index 3f8bd8e4..70ffebb4 100644 --- a/tests/TenantAssetTest.php +++ b/tests/TenantAssetTest.php @@ -145,11 +145,78 @@ test('test asset controller returns a 404 when no path is provided', function () tenancy()->initialize($tenant); + $this->withoutExceptionHandling(); + pest()->expectExceptionMessage('Empty path'); // outside tests this is a 404 + pest()->get(tenant_asset(null), [ 'X-Tenant' => $tenant->id, ])->assertNotFound(); }); +test('tenant asset controller returns a 404 when the storage root doesnt exist', function () { + config(['tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class]); + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $storageRoot = storage_path("app/public"); + + if (is_dir($storageRoot)) { + rmdir(storage_path("app/public")); + } + + $this->withoutExceptionHandling(); + pest()->expectExceptionMessage("Storage root doesn't exist"); // outside tests this is a 404 + + pest()->get(tenant_asset('foo.txt'), [ + 'X-Tenant' => $tenant->id, + ]); +}); + +test('tenant asset controller returns a 404 when accessing a nonexistent file', function () { + config(['tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class]); + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $storageRoot = storage_path("app/public"); + + if (! is_dir($storageRoot)) { + mkdir(storage_path("app/public"), recursive: true); + } + + $this->withoutExceptionHandling(); + pest()->expectExceptionMessage("Accessing a nonexistent file"); // outside tests this is a 404 + + pest()->get(tenant_asset('foo.txt'), [ + 'X-Tenant' => $tenant->id, + ]); +}); + +test('test asset controller returns a 404 when accessing a file outside the storage root', function () { + config(['tenancy.identification.default_middleware' => InitializeTenancyByRequestData::class]); + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $storageRoot = storage_path("app/public"); + + if (! is_dir($storageRoot)) { + mkdir(storage_path("app/public"), recursive: true); + file_put_contents(storage_path('app/foo.txt'), 'bar'); + } + + $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, + ]); +}); + function getEnvironmentSetUp($app) { $app->booted(function () { diff --git a/tests/UniversalRouteTest.php b/tests/UniversalRouteTest.php index ce10ccfe..59df82b7 100644 --- a/tests/UniversalRouteTest.php +++ b/tests/UniversalRouteTest.php @@ -8,12 +8,10 @@ use Illuminate\Routing\Route; use Stancl\Tenancy\Enums\RouteMode; use Stancl\Tenancy\Tests\Etc\Tenant; use Illuminate\Contracts\Http\Kernel; -use Stancl\Tenancy\Actions\CloneRoutesAsTenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Illuminate\Routing\Controller as BaseController; use Illuminate\Support\Facades\Route as RouteFacade; use Stancl\Tenancy\Tests\Etc\HasMiddlewareController; -use Stancl\Tenancy\Middleware\InitializeTenancyByPath; use Stancl\Tenancy\Middleware\IdentificationMiddleware; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use Stancl\Tenancy\Middleware\InitializeTenancyByDomain;