Merge pull request #32 from tenancy-for-laravel/merge-3x-feb2024

Merge 3.x into master

.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

README.md

@@ -3,7 +3,7 @@
 </p>
 
 <p align="center">
-    <a href="https://laravel.com"><img alt="Laravel 9.x" src="https://img.shields.io/badge/laravel-9.x-red.svg"></a>
+    <a href="https://laravel.com"><img alt="Laravel 10.x" src="https://img.shields.io/badge/laravel-10.x-red.svg"></a>
     <a href="https://packagist.org/packages/stancl/tenancy"><img alt="Latest Stable Version" src="https://poser.pugx.org/stancl/tenancy/version"></a>
     <a href="https://github.com/stancl/tenancy/actions"><img alt="GitHub Actions CI status" src="https://github.com/stancl/tenancy/workflows/CI/badge.svg"></a>
     <a href="https://github.com/stancl/tenancy/blob/3.x/DONATIONS.md"><img alt="Donate" src="https://img.shields.io/badge/Donate-%3C3-red"></a>

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/',
         ],

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

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,
                 ]);
             });

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);
+            }
+        }
+    }
 }

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);
 });
 

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 () {

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;