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/.gitignore b/.gitignore index 5a5960b0..99545a46 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ tenant-schema-test.dump tests/Etc/tmp/queuetest.json docker-compose.override.yml .php-cs-fixer.cache + 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/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..3e0b68e5 100644 --- a/src/Controllers/TenantAssetController.php +++ b/src/Controllers/TenantAssetController.php @@ -23,7 +23,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 +31,43 @@ 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'); + } + + protected function abortIf($condition, $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/src/Resolvers/PathTenantResolver.php b/src/Resolvers/PathTenantResolver.php index d5742294..0f63e7c4 100644 --- a/src/Resolvers/PathTenantResolver.php +++ b/src/Resolvers/PathTenantResolver.php @@ -31,6 +31,14 @@ class PathTenantResolver extends Contracts\CachedTenantResolver throw new TenantCouldNotBeIdentifiedByPathException($id); } + public function resolved(Tenant $tenant, ...$args): void + { + /** @var Route $route */ + $route = $args[0]; + + $route->forgetParameter(static::$tenantParameterName); + } + public function getArgsForTenant(Tenant $tenant): array { return [ 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..81c0393d 100644 --- a/tests/UniversalRouteTest.php +++ b/tests/UniversalRouteTest.php @@ -417,4 +417,46 @@ class Controller extends BaseController { return tenant() ? 'Tenancy is initialized.' : 'Tenancy is not initialized.'; } + + /** @test */ + public function universal_route_works_when_middleware_is_inserted_via_controller_middleware() + { + Route::middlewareGroup('universal', []); + config(['tenancy.features' => [UniversalRoutes::class]]); + + Route::get('/foo', [UniversalRouteController::class, 'show']); + + $this->get('http://localhost/foo') + ->assertSuccessful() + ->assertSee('Tenancy is not initialized.'); + + $tenant = Tenant::create([ + 'id' => 'acme', + ]); + $tenant->domains()->create([ + 'domain' => 'acme.localhost', + ]); + + $this->get('http://acme.localhost/foo') + ->assertSuccessful() + ->assertSee('Tenancy is initialized.'); + } +} + +class UniversalRouteController +{ + public function getMiddleware() + { + return array_map(fn($middleware) => [ + 'middleware' => $middleware, + 'options' => [], + ], ['universal', InitializeTenancyByDomain::class]); + } + + public function show() + { + return tenancy()->initialized + ? 'Tenancy is initialized.' + : 'Tenancy is not initialized.'; + } }