Merge branch 'archtechx:3.x' into 3.x

.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

.gitignore

@@ -10,3 +10,4 @@ coverage/
 clover.xml
 tests/Etc/tmp/queuetest.json
 docker-compose.override.yml
+.DS_Store

src/Controllers/TenantAssetsController.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Controllers;
 
+use Exception;
 use Illuminate\Routing\Controller;
 use Throwable;
 
@@ -18,7 +19,7 @@ class TenantAssetsController extends Controller
 
     public function asset($path = null)
     {
-        abort_if($path === null, 404);
+        $this->validatePath($path);
 
         try {
             return response()->file(storage_path("app/public/$path"));
@@ -26,4 +27,43 @@ class TenantAssetsController extends Controller
             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);
+            }
+        }
+    }
 }

src/Middleware/InitializeTenancyByRequestData.php

@@ -6,6 +6,7 @@ namespace Stancl\Tenancy\Middleware;
 
 use Closure;
 use Illuminate\Http\Request;
+use Stancl\Tenancy\Contracts\TenantResolver;
 use Stancl\Tenancy\Resolvers\RequestDataTenantResolver;
 use Stancl\Tenancy\Tenancy;
 

tests/TenantAssetTest.php

@@ -134,11 +134,79 @@ class TenantAssetTest extends TestCase
         $tenant = Tenant::create();
 
         tenancy()->initialize($tenant);
-        $response = $this->get(tenant_asset(null), [
+
+        $this->withoutExceptionHandling();
+        $this->expectExceptionMessage('Empty path'); // outside tests this is a 404
+
+        $this->get(tenant_asset(null), [
             'X-Tenant' => $tenant->id,
         ]);
-
-        $response->assertNotFound();
     }
 
+    public function test_asset_controller_returns_a_404_when_the_storage_root_doesnt_exist()
+    {
+        TenantAssetsController::$tenancyMiddleware = InitializeTenancyByRequestData::class;
+
+        $tenant = Tenant::create();
+
+        tenancy()->initialize($tenant);
+
+        $storageRoot = storage_path("app/public");
+
+        if (is_dir($storageRoot)) {
+            rmdir(storage_path("app/public"));
+        }
+
+        $this->withoutExceptionHandling();
+        $this->expectExceptionMessage("Storage root doesn't exist"); // outside tests this is a 404
+
+        $this->get(tenant_asset('foo.txt'), [
+            'X-Tenant' => $tenant->id,
+        ]);
+    }
+
+    public function test_asset_controller_returns_a_404_when_accessing_a_nonexistent_file()
+    {
+        TenantAssetsController::$tenancyMiddleware = 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();
+        $this->expectExceptionMessage("Accessing a nonexistent file"); // outside tests this is a 404
+
+        $this->get(tenant_asset('foo.txt'), [
+            'X-Tenant' => $tenant->id,
+        ]);
+    }
+
+    public function test_asset_controller_returns_a_404_when_accessing_a_file_outside_the_storage_root()
+    {
+        TenantAssetsController::$tenancyMiddleware = 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();
+        $this->expectExceptionMessage('Accessing a file outside the storage root'); // outside tests this is a 404
+
+        $this->get(tenant_asset('../foo.txt'), [
+            'X-Tenant' => $tenant->id,
+        ]);
+    }
 }